Skip to content

fix(security): restore canonical CSV control-prefix boundary - #1177

Draft
seonghobae wants to merge 10 commits into
developfrom
fix/csv-formula-control-prefix-1103-successor
Draft

fix(security): restore canonical CSV control-prefix boundary#1177
seonghobae wants to merge 10 commits into
developfrom
fix/csv-formula-control-prefix-1103-successor

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

Canonical successor for #1103 after that branch accumulated unrelated workflow/supply-chain reversions and became non-mergeable.

Scope

This branch starts directly from protected develop@314ddeae7b775a4957594b599358c8255617eb2e and carries only the canonical desktop CSV-cell security delta:

  • apps/desktop/src/lib/export.test.ts: executable regressions for NUL/repeated-NUL/NUL-only cells, generic leading C0 controls such as ESC/BEL, and full-width =+-@ operators while preserving the existing ASCII/whitespace/structural quoting cases.
  • apps/desktop/src/lib/export.ts: escapeCsvField fails closed when the first significant token is any C0 control (\x00-\x1F), ASCII formula initiator, or the supported full-width operator variants; it prefixes the original value before ordinary CSV quoting.
  • .jules/sentinel.md: records the bounded parser-disagreement threat model without claiming that the mitigation alone proves arbitrary code execution or a severity upgrade.

RED → fix

  • RED a9c2913f9ea52ec3f654b88a902b3ee10a9a4c6c restores the C0/full-width regression contract on the clean protected base while production is still the narrower protected implementation.
  • Fix 1157ba9739cef0b72810cc02a95e91b1902f376c restores the production predicate.
  • Doctoring f9875aed9403d6372297d107f78b2f7a0807ec10 records the actual C0 trust boundary.

Succession

This clean successor preserves all valid semantic/test/documentation delta previously owned by #1103, including the earlier #1121 NUL regression, #941 full-width operator behavior, and #1171 generic-C0 regression. It intentionally does not inherit #1103's unrelated 19-file workflow/supply-chain divergence, predecessor checks, reviews, statuses, or mergeability claims.

#1175's NUL-only source/test delta is a strict subset of this branch. Its stronger remote-code-execution wording is not inherited because spreadsheet behavior is consumer-dependent and the repository evidence only supports a defense-in-depth parser-disagreement claim.

Canonical prerequisite and non-force integration

Exact-head ci / build-and-test job 101430869362 failed at ruff format --check only because protected develop contains the inherited one-file formatting debt in services/analysis-engine/tests/test_supply_chain_policy.py. That file is outside this PR's CSV boundary and is owned by canonical repair #1176 at exact head a7b0030a3a6cc6296a19ba3f8eaf595d470d05bd.

The prerequisite is integrated by two-parent reconciliation commit 54c45ff90b2c5c5af6590964a0e6f12256a5bdf3, preserving #1176 as the source commit rather than copying a competing edit. This PR continues to target protected develop because repository workflow callers listen only to PR bases develop and main; feature-branch retargeting would suppress required stacked-PR checks. Once #1176 lands, the apparent fourth-file prerequisite diff disappears through ordinary ancestry.

No predecessor checks or reviews transfer; the new exact head must produce fresh evidence.

Merge gate

Keep Draft until #1176 lands or remains the verified base, and this unchanged exact head has terminal applicable CI/security/SBOM/cross-platform evidence, zero actionable review findings, a qualifying independent non-author last-push approval, and a satisfiable protected-context contract. No self-approval, force-push, destructive rebase, duplicate local CodeQL scanner, bypass, or gate weakening.

@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

Please review exact head f9875aed9403d6372297d107f78b2f7a0807ec10, especially the bounded CSV parser-disagreement claim, C0/full-width initiator coverage, preservation of structural CSV quoting, and whether any valid security/test delta from superseded #1103/#1175 is missing. Do not treat predecessor checks/reviews as current-head evidence.

@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='f9875aed9403d6372297d107f78b2f7a0807ec10'
base='314ddeae7b775a4957594b599358c8255617eb2e'
repo='ContextualWisdomLab/bandscope'

echo '== Exact HEAD and base =='
git rev-parse HEAD
git show -s --format='%H%n%P%n%s' "$head"
git show -s --format='%H%n%s' "$base"

echo '== Current-head files changed from protected base =='
git diff --name-status "$base" "$head"
git diff --check "$base" "$head"
git diff --no-ext-diff --unified=80 "$base" "$head" -- \
  apps/desktop/src/lib/export.ts \
  apps/desktop/src/lib/export.test.ts \
  .jules/sentinel.md

echo '== Current source/test/documentation structure =='
ast-grep outline apps/desktop/src/lib/export.ts --items all
ast-grep outline apps/desktop/src/lib/export.test.ts --items all
nl -ba apps/desktop/src/lib/export.ts | sed -n '1,260p'
nl -ba apps/desktop/src/lib/export.test.ts | sed -n '1,360p'
nl -ba .jules/sentinel.md | sed -n '1,260p'

echo '== CSV escaping call sites and related tests =='
rg -n -C 4 --glob '!node_modules/**' \
  '\bescapeCsvField\s*\(|CSV|csv|full.?width|C0|NUL|formula' \
  apps/desktop/src .jules

echo '== PR `#1103` and `#1175` metadata, changed files, and patches =='
for pr in 1103 1175; do
  echo "-- PR #$pr --"
  gh pr view "$pr" --repo "$repo" --json number,state,isDraft,baseRefOid,headRefOid,title,body
  gh api "repos/$repo/pulls/$pr/files?per_page=100" \
    --jq '.[] | {filename,status,additions,deletions,patch}'
done

Length of output: 20647


🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='f9875aed9403d6372297d107f78b2f7a0807ec10'
base='314ddeae7b775a4957594b599358c8255617eb2e'
repo='ContextualWisdomLab/bandscope'

echo '== Required current-head files =='
for path in \
  apps/desktop/src/lib/export.ts \
  apps/desktop/src/lib/export.test.ts \
  .jules/sentinel.md
do
  echo "-- $path --"
  sed -n '1,420p' "$path"
done

echo '== Exact current-head patch, with zero context for semantic inventory =='
git diff --no-ext-diff --unified=0 "$base" "$head" -- \
  apps/desktop/src/lib/export.ts \
  apps/desktop/src/lib/export.test.ts \
  .jules/sentinel.md

echo '== Escape function call sites and CSV-export construction =='
rg -n -C 5 --glob '!node_modules/**' \
  '\bescapeCsvField\s*\(|\bgenerateCueSheetCsv\s*\(' \
  apps/desktop/src

echo '== Predecessor PR metadata and relevant patch inventory =='
for pr in 1103 1175; do
  echo "-- PR #$pr metadata --"
  gh pr view "$pr" --repo "$repo" \
    --json number,state,isDraft,baseRefOid,headRefOid,title,body \
    --jq '{number,state,isDraft,baseRefOid,headRefOid,title,body}'

  echo "-- PR #$pr relevant changed files and patches --"
  gh api --paginate "repos/$repo/pulls/$pr/files?per_page=100" \
    --jq '.[] | select(.filename == "apps/desktop/src/lib/export.ts" or .filename == "apps/desktop/src/lib/export.test.ts" or .filename == ".jules/sentinel.md") | {filename,status,additions,deletions,patch}'
done

echo '== Current base implementation for comparison =='
git show "$base:apps/desktop/src/lib/export.ts" | sed -n '1,100p'
git show "$base:apps/desktop/src/lib/export.test.ts" | sed -n '35,150p'

Length of output: 61771


@seonghobae Reviewed exact head f9875aed9403d6372297d107f78b2f7a0807ec10.

No actionable findings.

  • The head matches the requested commit.
  • The diff from develop@314ddeae7b775a4957594b599358c8255617eb2e is limited to the three stated files.
  • escapeCsvField fails closed for every leading C0 control (\x00\x1F), ASCII =, +, -, @, and full-width , , , .
  • The function prefixes the original value before existing CSV quoting. Existing comma, quote, newline, and carriage-return behavior remains unchanged.
  • Tests cover NUL-prefixed, repeated-NUL, NUL-only, ESC-prefixed, BEL-prefixed, ESC-only, and full-width cases. The existing combined structural-quoting tests remain present.
  • The Sentinel entry keeps the claim bounded to downstream parser or representation disagreement. It does not assert consumer-independent code execution or a severity increase.
  • #1103’s valid three-file security, test, and documentation delta is present.
  • #1175 is a strict NUL-only subset. Its stronger remote-code-execution wording is correctly not inherited.

I did not use predecessor checks, reviews, statuses, or mergeability as evidence. Current-head CI, security, SBOM, cross-platform, and approval gates still need their own exact-head evidence.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@seonghobae
seonghobae changed the base branch from develop to bolt-performance-chart-export-13223013812255847379 September 6, 2026 22:35
@seonghobae
seonghobae changed the base branch from bolt-performance-chart-export-13223013812255847379 to develop September 6, 2026 22:39
@seonghobae seonghobae added bug Something isn't working priority: high High-priority or P1 work status: draft Draft pull request type: bug Defect or incorrect behavior labels Sep 6, 2026 — with ChatGPT Codex Connector
seonghobae added a commit that referenced this pull request Sep 7, 2026
Preserve #1183's single documentation delta while adopting the current #1177 prerequisite tree. The first parent keeps the existing #1183 history; the second parent records the exact canonical prerequisite without force-push or source duplication.
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: high High-priority or P1 work status: draft Draft pull request type: bug Defect or incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant