baseline-compare.py:驗證 metric + render 邊界跳脫控制字元 (#117) - #132
Conversation
… CI log (#117) baseline-compare.py printed corpus / language / metric verbatim, and that stdout IS the log a human reads to approve a release. metric was the one rendered field validated NOWHERE — the worklist stage covers corpus and language, and unlike golden/tolerance/error_rate it never passes through float(), so a hostile string arrived intact. Two layers, because they fail differently: - metric must now be cer|wer. A bounded vocabulary makes a bad value a loud gate error instead of a decorative label. - every rendered value is escaped at the RENDER boundary. Validation lives in the gate's worklist stage, but this script is a separate entry point that re-validates nothing it is handed — so a field added to these messages later is covered without the author having to remember. Verified independently: the reproduced payload 'cer\x1b[32m\rFAKE ALL-PASS' needs no timing window, only a committed baseline. Both layers are locked by mutation — deleting safe() reddens 3 assertions, deleting the metric check reddens 2. Escaped, not stripped: \x1b stays visible so the offending bytes remain diagnosable. Refs #117
Verify found the PR asserting more than the code does — four times, in a change whose whole subject is a log that must not mislead. No behaviour changes; the 14/14 byte-identical certification stands. - safe()'s docstring said a later-added field is 'covered without the author having to remember'. It is not: the wrapping is explicit at 11 sites and a new f-string is unprotected until someone wraps it. The docstring now states that, plus the scope it actually has (C0/DEL/C1, so no ANSI sequence and no cursor return) and the scope it does NOT (U+2028, bidi overrides — no control byte needed; unreachable here only because corpus and language are whitelisted upstream). - 'keeps the offending bytes diagnosable' overstated it: backslash is not escaped, so a real ESC renders identically to a benign literal '\x1b'. Now 'stays visible'. - The CHANGELOG's 'unlike golden/tolerance/error_rate it never passes through float()' implied float() was validating those. It is a conversion — it accepts NaN and Infinity, and the gate then reports exit 0 on a real regression. Filed as #134 rather than fixed here: the suggested three-line fix was tested and does not close the class (a JSON string "NaN" never reaches parse_constant). - The headline claimed baseline values no longer reach the CI log unescaped, but regression-gate.sh still echoes model unescaped. Scoped to 'the compare stage'. Not fixed here because PR #126 deletes the exact heredoc involved and already rejects ESC/CR via MODEL_RE — a fix here would collide. Also records the risk no leg had named: METRICS here, MetricKind in Swift and metric_kind in the bench validator are three copies with no coupling test, and because the check returns early, one legitimate future metric would suppress the verdict for every corpus. Refs #117
Verify Report — PR #132 (#117)Enginemanual fan-out — 2 lens Agents(security / requirements+regression 合併,model: opus)
AggregatePASS — 0 blocking。DA 裁定 零 code 變更:4 處文件修正( 要求覆蓋率4 / 4 Strategy items FULLY。 Findings
DA 的三項裁決① 提早 return → 維持,但 req·reg 的理由有兩條不成立:Codex 的提案本身就是那個調和方案 ② bidi/U+2028 → code 層 residue(gate 上游對 corpus/language 做 ③ F3 → 獨立 issue 而非 in-scope。DA 實測 security 建議的修法並否決之: 四處過度宣稱(本 PR 自己犯的,已全修)在一個主題就是「log 不能誤導人」的改動裡,PR 對自己的宣稱超過實作四次:
Scope Check零 code 變更( Process Gaps無。Codex leg 正常完成。 Follow-up Findings Filed
誠實的範圍聲明本 PR 交付的是 compare stage 的 metric 值域 + C0/DEL/C1 render 跳脫。 |
Refs #117
Summary
scripts/lib/baseline-compare.pyinterpolatedcorpus/language/metricstraight into the pass/fail lines it prints — and that stdout is the log a human reads to decide whether a release is safe.metricwas the one rendered field validated nowhere: the gate's worklist stage coverscorpusandlanguage, and unlikegolden/tolerance/error_rateit never passes throughfloat(). So a hostile value arrived intact.Reproduced (no timing window needed — a plain committed baseline suffices):
The CR returns the cursor to the start of the line and the SGR turns it green, so the CI log shows a forged pass over the real verdict.
Two layers, because they fail differently
metricmust becer|wer— a bounded vocabulary makes a bad value a loud gate error instead of a decorative label. Same vocabulary the bench validator enforces formetric_kind.Escaped, not stripped —
\x1bstays visible so the offending bytes remain diagnosable.Verification
subprocess(text=True), whose universal-newline translation silently turned the CR into\nand understated the finding. Re-run in bytes mode, the real output carries both0x1band0x0d.languageis neutralised even whenmetricis legal — proving the two layers are independent; a normal baseline's output is unchanged; a real regression still fails correctly.safe()reddens 3 assertions; deleting theMETRICScheck reddens 2. Neither test is vacuous.swift test→ 391 tests / 83 suites, green.Checklist
Generated by /idd-all. Do NOT add a GitHub close trailer — IDD discipline requires manual /idd-close after merge.