Skip to content

security(csv): harden spreadsheet formula export boundary - #570

Draft
seonghobae wants to merge 6 commits into
developmentalfrom
sentinel/fix-csv-injection-2898109622445415200
Draft

security(csv): harden spreadsheet formula export boundary#570
seonghobae wants to merge 6 commits into
developmentalfrom
sentinel/fix-csv-injection-2898109622445415200

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 3, 2026

Copy link
Copy Markdown

Current exact authority

  • protected base: developmental@722fb505d384405dd5e2e757cc4bf4371a957e54
  • exact head: b8dc9f8a7b97827a4ba61db814e8de3533ec91c1
  • compare: ahead-only (ahead_by=5, behind_by=0), merge base exactly protected developmental
  • lifecycle: open / Draft / mergeable

The underlying risk is valid: session CSV export embeds values that may be attacker/user controlled, and spreadsheet applications can interpret formula-leading cell values. This lane treats the defect as CSV/formula-injection prevention at the human-opened spreadsheet boundary. It does not retain the earlier unconditional HIGH/RCE claim because actual impact depends on the spreadsheet application, version, configuration, warnings and user interaction.

Current production contract

csvField() neutralizes formula-leading =, +, -, @, C0/whitespace-prefixed forms and the explicitly selected full-width =+-@ variants by prepending a single quote before normal CSV quoting. Existing comma/double-quote/CR/LF encoding remains the RFC-4180-style field boundary.

OWASP's current CSV Injection guidance identifies untrusted CSV/spreadsheet exports as the relevant sink and warns that spreadsheet behavior varies. OWASP WSTG WSTG-INPV-21 likewise treats impact as spreadsheet/client dependent. ASVS 5.0.0 v5.0.0-1.2.10 requires CSV escaping plus single-quote neutralization for formula-leading special characters including =, +, -, @, TAB and NUL.

Primary references:

TDD / evidence repair in this lineage

The owner-generated predecessor 6a6aa83b2b80e4f8a7c8a346c54e3527abe30db5 added the sanitizer and a test file, but the test duplicated its own local csvField() implementation. That test could remain GREEN even if the production export boundary regressed, so it was not causal product evidence.

  • RED/evidence repair 8dea52107f7bf2bae7b09ef76e7dbdbdf8c9a3cb: removes the copied sanitizer from the test and instead imports the real buildSessionsCsv boundary. The regression constructs one realistic session projection and exercises benign values, CSV quoting, = + - @, TAB/CR/LF, leading-space/control variants, full-width =+-@, and comma/quote/CRLF cell-breakout payloads through the production row builder.
  • minimal GREEN 8908388b0e072e8ffbc0438a9ace1d715954beb3: exports the existing pure buildSessionsCsv function so the regression can execute the canonical product boundary. No CSV runtime semantics, authorization, query or response behavior is changed by this seam.
  • test-framing repair/current head b8dc9f8a7b97827a4ba61db814e8de3533ec91c1: the first integration assertion initially split a quoted CRLF field with a naive row delimiter. The current test keeps exact-row compatibility on a framing-safe benign payload and verifies embedded CRLF separately as a quoted-field containment case.

The current effective delta is three files only: .jules/sentinel.md, the real-boundary regression, and route.ts. The branch is an ordinary ahead-only descendant; no force update or destructive rebase was used.

Promotion boundary

Fresh exact-head workflows have materialized but are non-passing: Dependency Review is pending and CI, Security Scan, SAST Semgrep, OSV/OSV-Scanner PR and Scorecard are queued. No predecessor workflow/review result transfers to b8dc9f8....

Keep Draft until this unchanged exact head has terminal applicable tests/build/lint/typecheck/security/dependency evidence, zero valid unresolved findings, and then-live qualifying review/protected-branch admission. Do not use no-op retriggers, self-approval, administrator bypass, force-push, gate weakening, or fabricated spreadsheet-execution evidence.

@google-labs-jules

Copy link
Copy Markdown

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae
seonghobae marked this pull request as draft September 3, 2026 21:04
@seonghobae seonghobae changed the title 🛡️ Sentinel: [HIGH] 세션 익스포트의 CSV Injection 취약점 수정 security(csv): harden spreadsheet formula export boundary Sep 3, 2026

Copy link
Copy Markdown
Author

@jules Please continue this existing Draft branch in place from exact head 83c030429924f358c9e7962348d5a556047900c4; do not create another PR.

Implement the RED→GREEN acceptance now recorded in the PR body. Start with benign tests through the actual session CSV export/build path for ordinary values; = + - @; TAB/CR/LF and leading-space/control variants; quote/comma/CRLF cell-breakout attempts; and an explicit JA/full-width =+-@ contract. Preserve RFC 4180 quote escaping and visible data semantics. Then make the smallest causal sanitizer change required by those tests.

Do not preserve the old unconditional HIGH/RCE claim: impact is spreadsheet/application dependent, and current OWASP guidance warns that no universal sanitizer is reliable across all spreadsheet/save-reopen workflows. If you choose an Excel-specific mitigation, document that product target and the data-mutating trade-off rather than generalizing it.

Run applicable pnpm focused/full tests, lint/typecheck/build and keep Draft until exact-head hosted gates/review are terminal. Adopt intervening valid deltas; normal descendant only. No force-push, destructive rebase, no-op retrigger, self-approval or gate weakening.

@google-labs-jules

Copy link
Copy Markdown

@jules Please continue this existing Draft branch in place from exact head 83c030429924f358c9e7962348d5a556047900c4; do not create another PR.

Implement the RED→GREEN acceptance now recorded in the PR body. Start with benign tests through the actual session CSV export/build path for ordinary values; = + - @; TAB/CR/LF and leading-space/control variants; quote/comma/CRLF cell-breakout attempts; and an explicit JA/full-width =+-@ contract. Preserve RFC 4180 quote escaping and visible data semantics. Then make the smallest causal sanitizer change required by those tests.

Do not preserve the old unconditional HIGH/RCE claim: impact is spreadsheet/application dependent, and current OWASP guidance warns that no universal sanitizer is reliable across all spreadsheet/save-reopen workflows. If you choose an Excel-specific mitigation, document that product target and the data-mutating trade-off rather than generalizing it.

Run applicable pnpm focused/full tests, lint/typecheck/build and keep Draft until exact-head hosted gates/review are terminal. Adopt intervening valid deltas; normal descendant only. No force-push, destructive rebase, no-op retrigger, self-approval or gate weakening.

Acknowledged. I have updated the branch in place per your instructions, applying OWASP's recommended mitigation of prepending a single quote before potential spreadsheet formula triggers (=, +, -, @, \t, \r, and full-width variants) and verified against the tests you requested.

@seonghobae seonghobae added bug Something isn't working priority: medium Normal-priority or P2 work type: bug Defect or incorrect behavior labels Sep 7, 2026 — with ChatGPT Codex Connector
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority: medium Normal-priority or P2 work type: bug Defect or incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant