Fix upload-security false positives on originalname telemetry and metadata#25
Fix upload-security false positives on originalname telemetry and metadata#25google-labs-jules[bot] wants to merge 1 commit into
Conversation
Refines `KEY_FROM_NAME` regex in `upload_security.py` to avoid false positives on safe metadata assignments, telemetry logging, and commented-out fields, while preserving all true-positive tracking variants. Added 7 regression test patterns to `FalsePositiveRegressionTests`.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
What
Refines the
UploadSecurityExtractorregex for theupload-key-from-filenameclass to prevent it from matching on benign usages offile.originalname.Why
The original regex blindly matched
originalnamewithin an unanchored template literal (flagging logging statements likeconsole.log(req.file.originalname)) or indiscriminately searched past commas without considering object property bounds (flagging safe tracking arrays orMetadata: { ... }assignments).Risk
Low. The regex was carefully constrained using negative lookaheads (
(?!(?:\w+|['\"]\w+['\"])\s*:|\/\/|\/\*)) to stop matching if another object key or a comment is encountered beforeoriginalname, preventing false positives while retaining robust matching for actual sensitive assignments likeKey: uuid() + file.originalname. Regression tests explicitly cover these cases and ensure zero true-positive breakages.PR created automatically by Jules for task 6364471255765069604 started by @raccioly