fix(security): restore canonical CSV control-prefix boundary - #1177
fix(security): restore canonical CSV control-prefix boundary#1177seonghobae wants to merge 3 commits into
Conversation
|
Warning Review limit reachedNext included review available in 43 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: Team Run ID: 📒 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 |
|
@coderabbitai review Please review exact head |
🧩 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}'
doneLength 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
No actionable findings.
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.
|
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@314ddeae7b775a4957594b599358c8255617eb2eand 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:escapeCsvFieldfails 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
a9c2913f9ea52ec3f654b88a902b3ee10a9a4c6crestores the C0/full-width regression contract on the clean protected base while production is still the narrower protected implementation.1157ba9739cef0b72810cc02a95e91b1902f376crestores the production predicate.f9875aed9403d6372297d107f78b2f7a0807ec10records 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.
Merge gate
Keep Draft until 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 the protected-context contract is satisfiable. No self-approval, force-push, destructive rebase, duplicate local CodeQL scanner, bypass, or gate weakening.