feat(tools): Virgin.root <-> .ut corpus sync verifier (warn-only) (#675)#679
Conversation
Detects content drift between databases/Virgin.root and the .ut corpus at usertalk_scripts/Frontier.root/**/*.ut. The two are intended to stay in sync (the .ut tree is what reviewers see in PR diffs; the .root is the binary the kernel runs) but nothing mechanically enforced it. Agents and humans edit one, forget the other, and drift goes undetected until runtime. Modes: - default: incremental - verify only .ut files in the staged git diff. - --full: walk the entire corpus (~3058 in-scope .ut files). - --paths FILE [FILE ...]: verify named files explicitly. - --rewrite: overwrite drifted .ut files with kernel-canonical form (use for bulk cleanup; inspect diff before committing). Normalization applied to kernel output before byte-compare: 1. Decode JSON escapes (\r, \t, \n, \", \\, \u-escapes). 2. Decode MacRoman bytes -> UTF-8 (kernel emits raw MacRoman in JSON). 3. Substitute MacRoman comment markers: <<< -> //, >>> -> empty. 4. CR -> LF. 5. Strip outline structure markers around all-comment subtrees (matches langstripstructuremarkers behavior in script.newScriptObject). 6. Trim trailing LF symmetrically on both sides. Path translation: lexical FS->ODB mapping with bracket-quoting for segments that aren't valid UserTalk identifiers (e.g., #filters). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…mmit hook (#675) Top-level Makefile (new) proxies common workflows so contributors can `make build/unit/integration/verify-odb-sync` from the repo root without remembering whether each lives under `make -C tests` or as a tools/ script. tests/Makefile adds the same `verify-odb-sync` target (so `cd tests && make verify-odb-sync` works for contributors who already `cd tests` for `make test-integration`) plus a `test-odb-sync` target wired into `test-integration` that runs the positive+negative regression test for the verifier itself. Pre-commit hook bumps HOOK_VERSION 8 -> 9 and adds a Virgin.root <-> .ut sync check. WARN-ONLY for v9 because the corpus on develop carries ~12% historical drift (encoding artifacts from a Windows-1252-decoded-as-MacRoman export pass) that predates the verifier. Promoting to a hard gate requires a corpus cleanup pass, tracked separately. The hook prints to stderr but does not block the commit. Docs: CLAUDE.md Quick Reference lists the three invocation paths. docs/ODB_SCRIPT_EDITING.md #675 entry updated to reflect what landed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
P1 #2 (security): --rewrite refuses paths outside the resolved --corpus-root. New _path_within() helper uses os.path.commonpath for portability. Update help text to document the constraint. Mitigates CWE-22 / CWE-73 when --rewrite is called from automation with untrusted paths. P1 #3 (bar-raiser): ProtocolSession.get_script_body() has a per- request read timeout (30s default, FRONTIER_VERIFIER_TIMEOUT env override). New _readline_with_timeout() uses selectors + os.read on the fd directly (bufsize=0 Popen gives FileIO without read1). Persistent _read_buf preserves over-reads across calls. On timeout the subprocess is killed and RuntimeError raised; caller surfaces exit 2 (infra error). P1 #4 (security): pre-commit hook reads staged paths NUL-delimited into a bash array (git diff -z + while read -d ''), then passes them as proper quoted args. Eliminates the silent-bypass primitive that paths containing spaces or shell-glob characters would otherwise trigger. P2 #3 (bar-raiser): pre-commit hook prints a yellow NOTE line when frontier-cli is not built instead of silently eliding the check. P2 #8 (security): _quote_segment() rejects segments containing control bytes (0x00-0x1F + 0x7F). [ and ] are still permitted because the corpus uses literal brackets in some path segments (e.g., xml.rss.moduleDrivers/http[colon][slash][slash]backend). P1 #1 was diagnostically false: the structure-marker regex was NOT firing on bar-raiser's cited "false positive" files. The drift on those files is genuine kernel-vs-disk lexical difference (kernel emits a close-brace at end of body line; the .ut export pulled the close-brace to the start of the next comment line and added a semicolon on the prior line). This is real drift to address in the corpus cleanup follow-up, not a regex bug. No code change for this finding; documenting here so future readers do not try to re-fix it. Test results unchanged: unit 483/483 pass, integration 18 baseline failures preserved, verifier regression test 2/2 pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Round 2 of /gate review returned PASS WITH NOTES with three P2s. Addressing two; the third (unit tests for new helpers) is deferred as a follow-up since the shell-based regression test already covers happy paths. P2 #1: Three em-dashes in docstrings (lines 141, 187, 192 of verify_virgin_root_sync.py per the round-2 review citation, two of which were in the helper rationale). Project rule is ASCII-only in code comments. Replaced with " - ". P2 #2: _path_within() was purely lexical; safe because the single caller pre-resolved both sides, but the helper signature invited future misuse. Now resolves both arguments internally and catches OSError (broken symlinks during resolve). Updated the call site to pass raw Paths since the helper handles resolution. Verified _path_within edge cases manually: - subdirectory positive: True - identical path: True - prefix attack rejected (/tmp/ab vs /tmp/a): False - .. resolution: True - unrelated paths: False Deferred P2 #3 (unit tests for _path_within / _quote_segment control- byte rejection / _readline_with_timeout) for a follow-up. The shell regression test covers happy paths; the listed unit tests would add ~50 lines of pytest infrastructure and aren't blocking. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
/gate Review Round 2 @
|
| Finding | Status |
|---|---|
| P1 #1 (bar-raiser): structure-marker regex over-strip | Diagnostically false. The regex does NOT fire on the cited files. Drift is genuine kernel-vs-disk lexical difference. Re-confirmed by bar-raiser in round 2. |
P1 #2 (security): --rewrite containment |
Closed (4d836aba6) — _path_within() + resolve. Round 2 confirms. |
| P1 #3 (bar-raiser): no read timeout | Closed (4d836aba6) — _readline_with_timeout() + persistent _read_buf. Round 2 confirms. |
| P1 #4 (security): hook word-splitting | Closed (4d836aba6) — NUL-delimited paths + bash array. Round 2 confirms. |
| P2 #3 (bar-raiser): hook silent skip | Closed (4d836aba6) — yellow NOTE printed. Round 2 confirms. |
| P2 #8 (security): segment validation | Closed (4d836aba6) — control bytes rejected; ] permitted for legitimate corpus paths. Round 2 confirms. |
Round 2 new findings
| # | Sev | Finding | Status |
|---|---|---|---|
| 1 | P2 | Three em-dashes in docstrings (project rule is ASCII-only in code comments) | Fixed dc1633af5 |
| 2 | P2 | _path_within() is purely lexical; safe today but invites future misuse |
Fixed dc1633af5 — resolves internally now |
| 3 | P2 | No unit tests for _path_within / _quote_segment control-byte rejection / _readline_with_timeout |
Deferred — shell regression test covers happy paths; adding pytest unit tests is a separate follow-up |
Bar-raiser on warn-only landing
12.2% baseline drift (374/3058) is genuine pre-existing lexical corruption from the Windows-1252-as-MacRoman conversion era, not anything this PR can responsibly fix in-band. Shipping warn-only with a documented gate-flip predicate is the right call. Blocking on corpus cleanup would gate visibility behind a multi-week task and provides no incremental safety — reviewers already see the warning. APPROVE the warn-only position.
Security on the fix pass
All four round-1 findings are correctly closed. No new findings at >=70% confidence. Recommendation: merge-ready from the security side.
Final state after dc1633af5
- 0 P0
- 0 P1
- 1 P2 deferred (unit tests for new helpers — visible follow-up)
Branch is ready to merge.
/auto SummaryWhat went well:
What could improve:
Recommendations:
🤖 Generated with Claude Code /auto |
Summary
Adds
tools/verify_virgin_root_sync.py(+ thin.shwrapper) that detects content drift betweendatabases/Virgin.rootand the.utcorpus atusertalk_scripts/Frontier.root/**/*.ut. Fixes #675.The .ut corpus exists because git can't diff binary .root files. Both are tracked; nothing previously enforced sync between them.
What landed
tools/verify_virgin_root_sync.py— the verifier. Long-livedfrontier-cli --lock-opened-roots --protocol --skip-startupsession; per-.utscript/evalofstring(<dotted.path>); normalize kernel output; byte-compare against on-disk .ut. Modes: default (incremental — staged .ut files fromgit diff --cached),--full(entire corpus, 3058 in-scope files),--paths FILE ...(explicit list).--rewriteflag overwrites drifted .ut files with kernel-canonical form (opt-in cleanup tool).tools/verify_virgin_root_sync.sh— thin shell wrapper exec'ing the .py (matches the rest oftools/*.sh).Makefile(new top-level) — proxy targets:build/unit/integration/verify-odb-sync/help. Existing per-area Makefiles intests/andfrontier-cli/continue to own the real logic.tests/Makefile— addsverify-odb-syncproxy +test-odb-synctarget wired intotest-integration.tests/verify_odb_sync_test.sh— positive (known-good file matches) + negative (synthetic drift via--corpus-rootsandbox is detected, exit 1) regression tests.tools/hooks/pre-commit-integration-tests—HOOK_VERSION 8 -> 9; adds a sync-check section. Warn-only (prints drift to stderr, does NOT block the commit). Rationale below.CLAUDE.mdQuick Reference lists the three invocation paths;docs/ODB_SCRIPT_EDITING.mdupdates the ODB: pre-commit + CI verification that Virgin.root and .ut corpus stay in sync #675 entry.Normalization pipeline
The kernel emits MacRoman bytes inside JSON (NOT valid UTF-8 in the presence of high-bit chars), CR line endings,
«(MacRoman 0xC7) as the single-line comment marker, and outline structure markers around all-comments subtrees. The corpus is UTF-8, LF,//comments. The verifier applies:\r \t \n \" \\ \/ \uXXXX)mac_romancodec)«→//;»→ emptylangstripstructuremarkersinstall behavior)Why warn-only, not gating
The corpus on develop carries ~374 of 3058 files (12.2%) of drift that predates this verifier — primarily from the original
tools/convert_ut_encoding.shpass (commitfa468dde0) decoding MacRoman-encoded source bytes as Windows-1252, which produced systematic miscoding (MacRoman0xD5=’U+2019 was read as Windows-1252Õand re-encoded as UTF-8Õ). The kernel still emits the correct MacRoman characters.Reviewer (gate) feedback initially diagnosed some of this as my structure-marker regex over-stripping. Diagnostic finding: the regex is NOT firing on the cited files; the drift is genuine kernel-vs-disk lexical difference (the .ut export tool moved close-braces from end-of-body-line to start-of-next-comment-line, inserting semicolons). The lexical difference is consistent and reproducible across all 374 affected files. Re-engineering the historical export's transformations is out of scope for this PR.
Promoting from warn-only to hard gate requires a corpus cleanup pass (drift-by-rewrite using
verify_virgin_root_sync.sh --rewrite --fullthen inspecting the diff). Tracked for follow-up. The verifier landing today still provides value as a diagnostic + as--rewriteinfrastructure for the cleanup./gate review findings (all addressed)
P1 #1 (bar-raiser, structure-marker regex over-strip) — diagnostically incorrect. Verified the regex does NOT fire on the cited files; drift there is real kernel-vs-disk lexical difference (see normalization-pipeline rationale above). No code change; documented in commit
4d836aba6so future readers don't try to re-fix it.P1 #2 (security,
--rewritewrites attacker-controlled bytes to attacker-controlled paths) — Fixed in4d836aba6. New_path_within()helper usingos.path.commonpath.--rewriterefuses paths that don't resolve under--corpus-root(reasonrewrite refused: outside corpus root). Mitigates CWE-22 / CWE-73.P1 #3 (bar-raiser,
ProtocolSessionhas no read timeout) — Fixed in4d836aba6. New_readline_with_timeout()usingselectors+os.readon the fd; 30s default, overridable viaFRONTIER_VERIFIER_TIMEOUTenv. On timeout the subprocess is killed andRuntimeErrorraised; caller surfaces exit code 2 (infra error).P1 #4 (security, pre-commit hook word-splits paths) — Fixed in
4d836aba6. Hook now reads staged paths NUL-delimited viagit diff -z+while read -d ''into a bash array, then expands as"${UT_STAGED_PATHS[@]}". Eliminates the silent-bypass primitive for paths with spaces or shell globs.P2 #3 (bar-raiser, pre-commit silently skips when frontier-cli unbuilt) — Fixed in
4d836aba6. Hook now prints a yellow NOTE:"skipping ODB sync check (frontier-cli not built; run \make build`)"`.P2 #4 (bar-raiser,
parts[0]not quoted), P2 #5 (bar-raiser,VALUE_REfield-order coupling), P2 #6 (security, segment validation completeness) — P2 #8 (control-byte rejection) addressed; P2 #4 and P2 #5 deferred (current corpus has noparts[0]that needs quoting, and theVALUE_REorder assumption is documented in the class docstring).Issue cleanup
Test plan
./tools/run_headless_tests.sh— 483/483 passcd tests && make test-integration— 2154 pass, 18 baseline failures preserved (12 html., 6 tcp.), no new regressions./tools/verify_virgin_root_sync.sh --paths <known-good>.utreturns 0./tools/verify_virgin_root_sync.sh --corpus-root <sandbox> --paths <tampered>.ut --rewriterefuses outside corpus rootmake verify-odb-syncruns at repo rootcd tests && make verify-odb-syncruns from tests/bash -n)make helplists targets