π‘οΈ Sentinel: [HIGH] Fix CSV formula injection null byte bypass - #1014
π‘οΈ Sentinel: [HIGH] Fix CSV formula injection null byte bypass#1014seonghobae wants to merge 1 commit into
Conversation
|
π 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. |
|
Warning Review limit reachedNext included review available in 6 minutes. View limit detailsLimit details: Youβve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: βοΈ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: β Files ignored due to path filters (1)
π Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Closing this as a competing/superseded CSV-security writer lane after fresh exact comparison.
No code is transferred from this branch because the security behavior is already strictly subsumed by #941. Dependency-security and lockfile authority remain #783-owned; no suppression or generated-lock rewrite belongs in this leaf lane. |
Understood. Acknowledging that this work is superseded by #941 and the PR is closed. Stopping work on this task. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| // Prevent CSV formula injection by prefixing problematic leading characters with a single quote | ||
| if (/^[\s\uFEFF\xA0]*[=+\-@\t\r\n]/.test(value)) { | ||
| // eslint-disable-next-line no-control-regex | ||
| if (/^[\s\uFEFF\xA0]*[=+\-@\t\r\n\x00]/.test(value)) { |
There was a problem hiding this comment.
π‘ Incomplete export Security Notes
Export changes require Security Notes covering boundaries, validation, safe failure, logging/privacy, and tests. The existing note omits several required review points.
Prompt for agents
The change touches CSV export behavior in apps/desktop/src/lib/export.ts. AGENTS.md and docs/security/app-security.md require Security Notes for export changes that cover untrusted inputs, trust boundaries, validation or allowlists, safe failure, logging/privacy impact, and test points. Add that evidence in the PR implementation summary or update the relevant implementation documentation before merge.
Was this helpful? React with π or π to provide feedback.
| // Prevent CSV formula injection by prefixing problematic leading characters with a single quote | ||
| if (/^[\s\uFEFF\xA0]*[=+\-@\t\r\n]/.test(value)) { | ||
| // eslint-disable-next-line no-control-regex | ||
| if (/^[\s\uFEFF\xA0]*[=+\-@\t\r\n\x00]/.test(value)) { |
There was a problem hiding this comment.
π Info: Only leading null bytes are neutralized
The regex /^[\s\uFEFF\xA0]*[=+\-@\t\r\n\x00]/ flags a null byte only at the field start. A payload like x\x00=cmd, where the null byte separates benign text from the trigger, is not prefixed. Consistent with the prior leading-trigger model, but interior null bytes remain unhandled.
Was this helpful? React with π or π to provide feedback.
1 participant
π¨ Severity: HIGH
π‘ Vulnerability: CSV Formula Injection can be bypassed using a null byte (
\x00).π― Impact: Attackers can execute arbitrary commands on a user's machine if the user opens the exported CSV file in spreadsheet software like Excel that ignores the null byte.
π§ Fix: Added the null byte (
\x00) to the regex used for escaping CSV formula trigger characters.β Verification: Ran
npm run test --workspace @bandscope/desktopto verify the added test cases passed and./scripts/harness/quickcheck.shto ensure all checks passed.PR created automatically by Jules for task 9564197493898694460 started by @seonghobae