fix(core): reject bidi controls in benchmark evidence identity - #322
seonghobae wants to merge 11 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough실행 컨텍스트 필드 검증이 Unicode 줄·문단 구분자와 양방향 서식 제어 문자를 추가로 거부합니다. 관련 오류 문서, 테스트, 보안 문서와 changelog가 갱신되었습니다. 표시 가능한 Unicode 및 RTL 문자열은 계속 허용됩니다. Changes실행 컨텍스트 Unicode 검증
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The validation change, regression coverage, diagnostics, and documentation are aligned in the supplied evidence. No actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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
Do not treat static review as hosted CI or browser execution evidence. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
parent="ea92c326e2dc4e3daa869aff1266c10b05453e7d"
head="11b2c16422481eb674a90ae61e941a5b247c4024"
echo "== Commit identities =="
git show -s --format='parent: %P%nhead: %H%nsubject: %s' "$head"
git cat-file -e "${parent}^{commit}"
git cat-file -e "${head}^{commit}"
echo
echo "== Name/status delta =="
git diff --name-status "$parent" "$head"
echo
echo "== Stat =="
git diff --stat "$parent" "$head"
echo
echo "== Exact delta =="
git diff --find-renames --find-copies --no-ext-diff --unified=80 "$parent" "$head"
echo
echo "== Parent validator candidates =="
git grep -n -E 'validate_run_context_field|is_control|ControlledBenchmarkRunContext' "$parent" -- '*.rs' || true
echo
echo "== Head validator candidates =="
git grep -n -E 'validate_run_context_field|is_bidi_control|is_control|ControlledBenchmarkRunContext' "$head" -- '*.rs' || true
echo
echo "== Parent test candidates =="
git grep -n -E 'reasoning_configuration|ControlledBenchmarkRunContext|Arabic|Hebrew|Korean|Japanese|Spanish|Bidi' "$parent" -- '*test*.rs' '*.rs' || true
echo
echo "== Head test candidates =="
git grep -n -E 'reasoning_configuration|ControlledBenchmarkRunContext|Arabic|Hebrew|Korean|Japanese|Spanish|Bidi' "$head" -- '*test*.rs' '*.rs' || trueLength of output: 29189 🏁 Script executed: #!/bin/bash
set -euo pipefail
parent="ea92c326e2dc4e3daa869aff1266c10b05453e7d"
test_first="a02b1d6e3a2034944ba337973bc9d0385976907c"
head="11b2c16422481eb674a90ae61e941a5b247c4024"
echo "== Commit sequence =="
git show -s --format='%H%nparents: %P%nsubject: %s%n' "$parent" "$test_first" "$head"
echo "ancestry parent -> test-first:"
git merge-base --is-ancestor "$parent" "$test_first" && echo yes
echo "ancestry test-first -> head:"
git merge-base --is-ancestor "$test_first" "$head" && echo yes
echo "commits in requested range:"
git log --format='%H %P %s' --reverse "${parent}..${head}"
echo
echo "== Static whitespace diagnostics =="
git diff --check "$parent" "$head"
echo
echo "== Controlled benchmark public context and error definitions at head =="
git show "$head:crates/originweave-core/src/controlled_benchmark.rs" | nl -ba | sed -n '140,205p;430,545p;590,780p'
echo
echo "== Focused test file at head =="
git show "$head:crates/originweave-core/tests/controlled_benchmark_run_context.rs" | nl -ba | sed -n '1,235p'
echo
echo "== Rust workspace and lint configuration candidates =="
fd -HI -t f '^(Cargo\.toml|rustfmt\.toml|clippy\.toml)$' . -E .git -E target | sort | while IFS= read -r path; do
echo "--- $path"
sed -n '1,260p' "$path"
done
echo
echo "== Formatting availability and non-mutating formatting check =="
if command -v rustfmt >/dev/null 2>&1; then
rustfmt --version
git diff --no-ext-diff "$parent" "$head" -- \
crates/originweave-core/src/controlled_benchmark.rs \
crates/originweave-core/tests/controlled_benchmark_run_context.rs > /tmp/controlled-benchmark.patch
# Inspect the changed Rust files only. --check does not modify files.
rustfmt --edition 2021 --check \
crates/originweave-core/src/controlled_benchmark.rs \
crates/originweave-core/tests/controlled_benchmark_run_context.rs
else
echo "rustfmt unavailable"
fiLength of output: 35506
I found one documentation-semantic issue.
Other static checks passed:
|
|
@coderabbitai review Fresh exact-head review requested for Focus on the three-file bounded delta: complete Unicode |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base="ea92c326e2dc4e3daa869aff1266c10b05453e7d"
head="9175f992c81b21b5ac80db10afb6aa94ec6b3c97"
echo '== identities and range =='
git show -s --format='commit: %H%nparents: %P%nsubject: %s' "$base" "$head"
git merge-base --is-ancestor "$base" "$head" && echo 'base-is-ancestor: yes'
git log --format='%H %P %s' --reverse "${base}..${head}"
echo '== files and static whitespace =='
git diff --name-status "$base" "$head"
git diff --stat "$base" "$head"
git diff --check "$base" "$head"
echo '== exact delta =='
git diff --no-ext-diff --find-renames --find-copies --unified=100 "$base" "$head" -- \
crates/originweave-core/src/controlled_benchmark.rs \
crates/originweave-core/tests/controlled_benchmark_run_context.rs \
docs/doctoring/controlled-benchmark-unicode-identity-security.md
echo '== source map =='
ast-grep outline crates/originweave-core/src/controlled_benchmark.rs --items all --match 'ControlledBenchmarkRunContext|validate_run_context_field|is_bidi_control|ControlCharacterRunContext' || true
echo '== source/test focused evidence at head =='
git show "$head:crates/originweave-core/src/controlled_benchmark.rs" | nl -ba | sed -n '400,535p;620,735p'
git show "$head:crates/originweave-core/tests/controlled_benchmark_run_context.rs" | nl -ba | sed -n '1,270p'
echo '== doctoring document =='
git show "$head:docs/doctoring/controlled-benchmark-unicode-identity-security.md" | nl -ba
echo '== config candidates =='
fd -HI -t f '^(Cargo\.toml|rustfmt\.toml|clippy\.toml)$' . -E .git -E target | sort | while IFS= read -r p; do
echo "--- $p"
sed -n '1,240p' "$p"
done
echo '== all Bidi_Control-related changed-code locations =='
git grep -n -E 'Bidi_Control|bidi|bidirectional|is_bidi_control|ControlCharacterRunContext' "$head" -- \
'*.rs' 'docs/doctoring/controlled-benchmark-unicode-identity-security.md' || trueLength of output: 66131 🤖 web_search executed:
Length of output: 19786
I found one issue.
Static checks for the requested bounded contract found the following:
I did not run rustfmt, tests, Clippy, rustdoc, coverage, hosted CI, or browser execution. This is static review only. You are interacting with an AI system. |
|
@coderabbitai review Please review only current exact head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base="ea92c326e2dc4e3daa869aff1266c10b05453e7d"
head="e0f0fc662992f47c667026c62e5bf536323a3bc8"
echo '== Commit identity and ancestry =='
git show -s --format='base: %H%nbase parents: %P%nbase subject: %s' "$base"
git show -s --format='head: %H%nhead parents: %P%nhead subject: %s' "$head"
git merge-base --is-ancestor "$base" "$head" && echo 'base-is-ancestor: yes'
echo '-- commits in range --'
git log --format='%H %P %s' --reverse "${base}..${head}"
echo '== Changed files and whitespace =='
git diff --name-status "$base" "$head"
git diff --stat "$base" "$head"
git diff --check "$base" "$head"
echo '== Changed Rust/documentation structure =='
ast-grep outline crates/originweave-core/src/controlled_benchmark.rs --items all --match 'ControlledBenchmarkRunContext|ControlCharacterRunContext|validate_run_context_field|is_bidi_control' || true
ast-grep outline crates/originweave-core/tests/controlled_benchmark_run_context.rs --items all || true
echo '== Exact bounded delta =='
git diff --no-ext-diff --find-renames --find-copies --unified=100 "$base" "$head" -- \
crates/originweave-core/src/controlled_benchmark.rs \
crates/originweave-core/tests/controlled_benchmark_run_context.rs \
docs/doctoring/controlled-benchmark-unicode-identity-security.md
echo '== Current source: public error, validation, evaluation =='
git show "$head:crates/originweave-core/src/controlled_benchmark.rs" | nl -ba | sed -n '420,535p;620,775p'
echo '== Current test file =='
git show "$head:crates/originweave-core/tests/controlled_benchmark_run_context.rs" | nl -ba | sed -n '1,300p'
echo '== Current doctoring document =='
git show "$head:docs/doctoring/controlled-benchmark-unicode-identity-security.md" | nl -ba
echo '== Parent validator context =='
git show "$base:crates/originweave-core/src/controlled_benchmark.rs" | nl -ba | sed -n '620,735p'
echo '== Workspace style and lint configuration =='
fd -HI -t f '^(Cargo\.toml|rustfmt\.toml|clippy\.toml)$' . -E .git -E target | sort | while IFS= read -r p; do
echo "--- $p"
sed -n '1,220p' "$p"
done
echo '== Static macro and helper usage in changed Rust files =='
git diff --no-ext-diff -U0 "$base" "$head" -- \
crates/originweave-core/src/controlled_benchmark.rs \
crates/originweave-core/tests/controlled_benchmark_run_context.rs |
rg -n -C2 '(unwrap!|unwrap\(|expect\(|panic!|dbg!|todo!|println!|eprintln!)' || true
echo '== Non-mutating rustfmt check, if available =='
if command -v rustfmt >/dev/null 2>&1; then
rustfmt --version
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
git show "$head:crates/originweave-core/src/controlled_benchmark.rs" > "$tmpdir/controlled_benchmark.rs"
git show "$head:crates/originweave-core/tests/controlled_benchmark_run_context.rs" > "$tmpdir/controlled_benchmark_run_context.rs"
rustfmt --edition 2021 --check \
"$tmpdir/controlled_benchmark.rs" \
"$tmpdir/controlled_benchmark_run_context.rs"
else
echo 'rustfmt unavailable'
fiLength of output: 77219 🤖 web_search executed:
Length of output: 15333 🏁 Script executed: #!/bin/bash
set -euo pipefail
for url in \
'https://www.unicode.org/reports/tr39/tr39-34.html' \
'https://www.unicode.org/reports/tr9/tr9-51.html'
do
echo "== $url =="
page="$(mktemp)"
trap 'rm -f "$page"' EXIT
curl --fail --location --silent --show-error --max-time 30 "$url" -o "$page"
rg -in -m 20 -C1 '(unicode security mechanisms|unicode bidirectional algorithm|<th[^>]*>Version|<th[^>]*>Date|<th[^>]*>Revision|Revision 34|Revision 51|2026-08-27|2025-08-13)' "$page"
rm -f "$page"
trap - EXIT
doneLength of output: 3707
I found no defect in the requested bounded delta.
You are interacting with an AI system. |
|
@coderabbitai review Please review only current exact head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base="ea92c326e2dc4e3daa869aff1266c10b05453e7d"
prior="e0f0fc662992f47c667026c62e5bf536323a3bc8"
head="fcc49ab34f305ee844f596d8b4d56d1bc0e599ac"
echo '== Commit identities and ancestry =='
git show -s --format='base%n %H%n parents: %P%n subject: %s%n' "$base"
git show -s --format='prior%n %H%n parents: %P%n subject: %s%n' "$prior"
git show -s --format='head%n %H%n parents: %P%n subject: %s%n' "$head"
git merge-base --is-ancestor "$base" "$head" && echo 'base-is-ancestor-of-head: yes'
git merge-base --is-ancestor "$prior" "$head" && echo 'prior-is-ancestor-of-head: yes'
echo '-- commits in complete requested range --'
git log --reverse --format='%H %P %s' "${base}..${head}"
echo '-- commits after prior reviewed head --'
git log --reverse --format='%H %P %s' "${prior}..${head}"
echo '== Changed paths and whitespace =='
git diff --name-status "$base" "$head"
git diff --stat "$base" "$head"
git diff --check "$base" "$head"
echo '-- successor-only paths and diff --'
git diff --name-status "$prior" "$head"
git diff --check "$prior" "$head"
git diff --no-ext-diff --unified=60 "$prior" "$head" -- CHANGELOG.md
echo '== CHANGELOG.md at head =='
git show "$head:CHANGELOG.md" | nl -ba | sed -n '1,220p'
echo '== Complete bounded delta =='
git diff --no-ext-diff --find-renames --find-copies --unified=50 "$base" "$head" -- \
crates/originweave-core/src/controlled_benchmark.rs \
crates/originweave-core/tests/controlled_benchmark_run_context.rs \
docs/doctoring/controlled-benchmark-unicode-identity-security.md \
CHANGELOG.md
echo '== Validation and public diagnostic at head =='
git show "$head:crates/originweave-core/src/controlled_benchmark.rs" | nl -ba | sed -n '430,515p;635,750p'
echo '== Regression coverage at head =='
git show "$head:crates/originweave-core/tests/controlled_benchmark_run_context.rs" | nl -ba | sed -n '1,280p'
echo '== Doctoring document at head =='
git show "$head:docs/doctoring/controlled-benchmark-unicode-identity-security.md" | nl -ba
echo '== Scope scan for changed paths and authority-related terms =='
git diff --no-ext-diff -U0 "$base" "$head" | rg -n -C2 '^\+.*(threshold|registry|browser|workflow|sign|persist|provider|release|publish|tag|Bidi_Control|bidi|bidirectional)' || true
echo '== Rust formatting availability =='
if command -v rustfmt >/dev/null 2>&1; then
rustfmt --version
else
echo 'rustfmt unavailable'
fiLength of output: 123865
I found no defect in the requested scope.
You are interacting with an AI system. |
|
@coderabbitai review Please review exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base="ea92c326e2dc4e3daa869aff1266c10b05453e7d"
head="a4c8ceaf67a075ef483334802aacfc54cf502068"
echo '== Commit identity and ancestry =='
git show -s --format='base: %H%nbase parents: %P%nbase subject: %s%n' "$base"
git show -s --format='head: %H%nhead parents: %P%nhead subject: %s%n' "$head"
git merge-base --is-ancestor "$base" "$head" && echo 'base-is-ancestor-of-head: yes'
echo '-- commits in requested range --'
git log --reverse --format='%H %P %s' "${base}..${head}"
echo '== Changed paths, stats, and whitespace =='
git diff --name-status "$base" "$head"
git diff --stat "$base" "$head"
git diff --check "$base" "$head"
echo '== Changed source outlines =='
git show "$head:crates/originweave-core/src/controlled_benchmark.rs" > /tmp/controlled_benchmark.rs
git show "$head:crates/originweave-core/tests/controlled_benchmark_run_context.rs" > /tmp/controlled_benchmark_run_context.rs
ast-grep outline /tmp/controlled_benchmark.rs --items all --match 'ControlCharacterRunContext|validate_run_context_field|is_bidi_control|is_unicode_line_separator|evaluate_controlled_benchmark_suite_for_run' || true
ast-grep outline /tmp/controlled_benchmark_run_context.rs --items all || true
echo '== Complete bounded delta =='
git diff --no-ext-diff --find-renames --find-copies --unified=70 "$base" "$head" -- \
crates/originweave-core/src/controlled_benchmark.rs \
crates/originweave-core/tests/controlled_benchmark_run_context.rs \
docs/doctoring/controlled-benchmark-unicode-identity-security.md \
CHANGELOG.md
echo '== Current validation, diagnostic, and evaluation =='
git show "$head:crates/originweave-core/src/controlled_benchmark.rs" | nl -ba | sed -n '425,525p;620,790p'
echo '== Current regression coverage =='
git show "$head:crates/originweave-core/tests/controlled_benchmark_run_context.rs" | nl -ba | sed -n '1,360p'
echo '== Current doctoring and changelog =='
git show "$head:docs/doctoring/controlled-benchmark-unicode-identity-security.md" | nl -ba
git show "$head:CHANGELOG.md" | nl -ba | sed -n '1,160p'
echo '== Parent validation =='
git show "$base:crates/originweave-core/src/controlled_benchmark.rs" | nl -ba | sed -n '625,765p'
echo '== Authority-sensitive changed lines =='
git diff --no-ext-diff -U0 "$base" "$head" | rg -n -i -C2 '^\+.*(threshold|registry|browser|workflow|sign|persist|provider|release|publish|tag|bidi|line.separator|paragraph.separator|unicode)' || true
echo '== Static prohibited macros in changed Rust lines =='
git diff --no-ext-diff -U0 "$base" "$head" -- \
crates/originweave-core/src/controlled_benchmark.rs \
crates/originweave-core/tests/controlled_benchmark_run_context.rs |
rg -n -C2 '(unwrap!|unwrap\(|expect\(|panic!|dbg!|todo!|println!|eprintln!)' || true
echo '== Rust formatter availability =='
if command -v rustfmt >/dev/null 2>&1; then
rustfmt --version
else
echo 'rustfmt unavailable'
fiLength of output: 136792
I found no defect in the requested scope.
✅ Action performedReview finished.
|
|
Fresh follow-up finding is tracked separately as #323 rather than widening this PR into an unversioned Unicode catch-all. Current #322 exact |
seonghobae
added
bug
ChatGPT Codex Connector
1 participant
Scope
Stacked security repair for #237 / #203. This PR changes only OriginWeave-owned controlled-benchmark reproducibility identity admission plus its tests, doctoring, and changelog. It does not reinterpret browser-issued protocol identifiers, add release/signing authority, or duplicate #199/#201 evidence/release ownership.
Current exact head is
a4c8ceaf67a075ef483334802aacfc54cf502068. The branch still targetsfeat/controlled-benchmark-threshold-contract, but its recorded base is predecessor #237 exactea92c326e2dc4e3daa869aff1266c10b05453e7d.Implemented delta
The lane rejects all 12 Unicode
Bidi_Controlscalars plus U+2028 LINE SEPARATOR and U+2029 PARAGRAPH SEPARATOR in benchmark-owned reproducibility identity. Visible Korean/Japanese/Spanish and Arabic/Hebrew remain admissible. Admission still occurs before expected/observed byte equality. No normalization, confusable folding, ASCII-only restriction, browser-protocol normalization, benchmark-threshold change, signing/persistence authority, workflow mutation, or release authority is introduced.The exact head also verifies the broadened
ControlCharacterRunContextdiagnostic string incontrolled_benchmark_run_context.rs, so the child already semantically covers the error-display path that #237 later repaired for coverage.Current parent drift
#237 advanced ordinary-forward to
4d175467c550c969d1ad22e51016473c0a4da034after native CI exposed rustfmt and uncovered-Displaydefects. Comparing current #237 to this head yields 11 ahead / 1 behind / diverged with merge baseea92c326.... A fresh PR metadata recalculation reports this Draft PR mergeable, so there is no proven content conflict; the one-behind parent delta is still an explicit restack/adoption obligation.This is not a close condition and not permission to discard the 11 child commits. The missing parent delta is test-only; its exact predecessor diagnostic cannot be copied verbatim because this child intentionally broadens the diagnostic to include C0/C1, Unicode line/paragraph separators, and bidirectional formatting characters.
Foundation order remains authoritative: #237 must first obtain exact-head acceptance and normal protected integration. Then this branch must ordinary/non-force adopt/adapt the protected parent generation, preserving every valid child delta, and reacquire all execution/review/ruleset evidence on the resulting new exact head. Scheduled-agent policy forbids merging, so no merge commit, destructive rebase, or force push is performed during this maintenance run.
Existing evidence
Earlier Draft-policy CI/MV3 generations were skipped; the prior Ready probe was cancelled and is not acceptance evidence. The predecessor current-head static CodeRabbit review found no actionable issue, but it is not runner-backed repository GREEN and will not transfer after restack.
#324 remains the downstream Unicode-18 DICP successor and must stay intact until this lane is reconciled and accepted.
No force push, destructive rebase, self-approval, bypass, gate weakening, workflow/ruleset/secret mutation, protected-main merge, tag, publish, or release is authorized here.