fix(protocols): adjudicate a 16-item audit — 11 fixes with mutation evidence - #251
Merged
Conversation
…y over-deletion
`rule()` warned only when a substitution removed more than
OVER_DELETION_WARN_BYTES. That guard was added for an unbounded-regex
over-deletion defect (199 bytes in, 18 out, two caller security rules gone),
and it can only fire when the text SHRINKS.
a7-freeform is the one rule that rewrites a bare token, and it GROWS:
FREEFORM (8 bytes) -> free-form (9 bytes), so `removed` is -1 per hit and no
byte threshold can ever see it. Measured damage while it was silent:
CHECK (kind IN ('FREEFORM','STRUCTURED')) -> ('free-form','STRUCTURED')
if (mode === FREEFORM) { parse(); } -> mode === free-form
The first rewrites a string literal inside a data contract; the second is not
valid code. Both were silent because neither shrinks the prompt.
The regex is deliberately NOT narrowed. The rule exists because a live-bisected
(deterministic 7/7) content policy blocks the request when that token is
present, and the live A/B showed BOTH fragments must change or the block
persists. Narrowing needs a fresh A/B against a NON-DETERMINISTIC policy that
cannot be run here — a6-cline-obj already ships default-OFF for that reason.
So the deliverable is visibility, not suppression.
The growth direction counts OCCURRENCES rather than bytes. A byte threshold is
the wrong instrument twice over: a one-byte-per-hit rewrite never reaches a
useful figure, while an ordinary identity rewrite (a clause replaced by a longer
sentence) exceeds any small figure on every well-behaved request, training
operators to ignore the line. What distinguishes a7-freeform is that it is a
global BARE-TOKEN rule, so it can hit an unbounded number of times in text it
was never aimed at.
The deletion branch is byte-for-byte unchanged, so the W2 guard it was written
for still holds.
Residual, stated because it is not covered: a SINGLE-hit rewrite of caller
content is still silent. `if (mode === FREEFORM)` has one occurrence, which is
indistinguishable by count from one legitimate hit of the trigger phrase.
Separating them needs context sniffing, which is the same kind of shape guess
this file refuses to make about narrowing.
Mutation spec: 6 mutations, 5 CAUGHT + 1 documented survivor. The survivor
(dropping `re.global`) is undrivable today because no non-global rule carries a
capture group, and that premise is itself pinned by a test rather than left
implicit.
Also repairs two anchors in identity-neutralize-paragraph-bound.json that the
rule() edit broke; the mutation they carry (byte counting -> char counting) is
preserved verbatim.
…paths
Adjudicated a 16-item audit list whose author stated they had verified none of
it (4 of their earlier hypotheses were already disproved and 1 named a file that
does not exist). 11 items are fixed here, 3 were judged real-but-not-worth-fixing
with reasons, and 2 were already covered. Four of the reports were wrong about
their own mechanism and are corrected below.
These five source files are committed together because three mutation specs bind
them: terminal-event-guard spans responses/messages/gemini, retry-after-route-parity
spans messages/gemini/server, and bash-prefix-repair spans chat/messages. Splitting
by file would leave a spec's anchors unresolved and the docs-consistency guard red
at that commit.
BASH PREFIX REPAIR (item 7) — the report said "expands to a longer command". It
is worse and different: `startsWith` alone permitted MID-TOKEN expansion, so the
gateway INVERTED the model's decision rather than extending it.
rm -i -> rm -if /data (interactive became force)
deploy --dry -> deploy --dry-run=false --prod
Five holes closed, each measured: token boundary; no new command (COMMAND_CHAINING_RE);
negation masking reused from the NLU layer rather than a second copy; provenance —
the source was any trailing user message, so a <tool_result> carrier made TOOL
OUTPUT the command source; and guessed boundaries — the backtick-free pattern
captured to end-of-line, so "run rm -rf varlog happened in the report" became argv
with no metacharacter for the chaining check to see.
DOCUMENT PROVENANCE — a fifth door, found after the others closed. Anthropic
separates an attachment from the caller's words at the BLOCK level; this layer
joined both into one message, after which nothing downstream could tell them
apart. Measured: a document reading "run `npm install --force --unsafe-perm`"
made the repair serve those flags on top of the model's `npm install`. Decoded
document text is now marked at translation and blanked in the repair. Blanking
rather than skipping the message is load-bearing — attachment and instruction
share one message.
RESPONSE STORE (items 9, 10) — the report named an unreachable branch: `commit`
returns early on `!chainable`, so putResponse's `!callerKey` line can never fire
from its only call site, and that line is the only one bumping _stats.rejected.
Both fixes are LATENT guards, and the tests say so rather than implying a live
defect. Retrieval no longer walks past the last user turn (measured: GET returned
"It is 4." when the last question was "And 3+3?"), and a refusal to store is now
logged instead of surfacing as a 404 one round trip later.
POST-TOOL TEXT (item 16) — text after a tool call merged into the FIRST output
item, so a client reassembling the turn placed all text before the call it
followed. The message item was a singleton. Sealing at tool-item creation gives
post-tool text its own item and makes output_item.done ascend without reordering
finish(). The store deliberately keeps the whole turn's text: it persists one
OpenAI-shaped message whose content is a single string, so segmenting it would
drop the pre-tool text from the next chained turn.
USAGE (item 15) — the report claimed a four-protocol divergence. Measured, the
routes agree on real values; Anthropic's message_start pre-fill is spec-required
and its delta corrects it. The real defect is one-directional: Responses ASSERTED
input_tokens:0 when the upstream reported nothing, which is false for any request
that reached a model and silently under-bills a relay. Now omitted, matching what
the Gemini exit already did. An explicit zero from the upstream is still reported.
Items 2-6 (terminal event guard, three translators, Gemini array-mode JSON,
Retry-After across two layers) carry their own specs and comments.
W3 — README:307 already states that tenant isolation rests on the response id's
~90 bits and not on the client-asserted scope. That paragraph had no test. Verified
the premise it corrects: under a shared API key two callers sending the same `user`
derive a BYTE-IDENTICAL callerKey. Four assertions now pin the id's width and
randomness, so narrowing it cannot silently make the documentation false.
Every fix carries mutation evidence including an over-reach control. Gate: 3691
pass / 0 fail, stable across two runs; 212 anchors, all unique.
scripts/mutate-verify-dirty.mjs exists for the window where a fix is still uncommitted — exactly when mutate-verify.mjs refuses to run, and refuses for a good reason. Its guard 1 restores with `git checkout HEAD -- src/`, which on a dirty tree DESTROYS the uncommitted fix sitting next to the mutation; every later mutation then measures "fix missing" rather than "mutation applied", and that failure is indistinguishable from the mutation being caught. Ledger round 4 recorded it; round 8 hit it again. So the file header states, line by line, what replaces each protection guard 1 bought: a cp backup taken after the baseline and before the first write (equal strength, and only the statement ORDER enforces that); a byte-for-byte comparison after restore (STRONGER — cp is not authoritative and must not be assumed to have succeeded); no clean-tree precondition (WEAKER, and the entire point); and handlers on SIGINT/SIGTERM/SIGHUP/uncaughtException. The one gap that is NOT closed is named rather than glossed: `kill -9` runs no handler, so the tree is left MUTATED while the backups sit in os.tmpdir(). That is why every run prints `BACKUP <file> -> <path>` before touching anything — those lines are the manual recovery path. This was not hypothetical: a run hung this round, was killed with -9, and left responses.js at `removed > 0`. The printed path is what restored it. .gitignore: `scripts/*` is ignore-by-default with a per-file allowlist, so a new script is invisible to git until it gets its own `!` line — `ls` shows it, `git status` does not. This harness was written, tested and referenced in a handoff before anyone noticed; it surfaced only because `git status` still reported the same file count. Now commented beside the rule. AUDIT-LEDGER round 15, five sections. The load-bearing one: a SURVIVING mutation has three causes with OPPOSITE correct responses — (a) an earlier gate rejected the fixture, (b) no fixture satisfies a conjunction, (c) the line is live but undrivable. All three occurred this round. (a) and (b) mean "add a fixture"; (c) means "do not, and pin the premise instead". Reading them as one thing either leaves a real hole or pads the mutation count. Section 5 is an erratum against myself: I reported a "contributors page / multilingual" problem list three times, and three of its four numbers were wrong (three READMEs -> two, and genuinely bilingual; 47 ledger records -> 53; badge links broken -> correct). They were cross-session numbers I quoted without retesting, and the last item proposed work that was already finished. A single `shasum README*.md` refuted three of them.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
中文 TL;DR
判定了一份 16 条的审计清单（提交者自陈没有复核任何一条，而同一轮里已有 4 条被证否、1 条是它编造的文件名）。结果：修了 11 条、3 条判定成立但附理由不修、2 条已被上轮覆盖。四条报告弄错了自己的机制，正文里逐条更正。
三个提交按 spec 的真实依赖切分，不是按文件——
terminal-event-guard绑 responses/messages/gemini、retry-after-route-parity绑 messages/gemini/server、bash-prefix-repair绑 chat/messages，所以那五个源文件是一个连通分量，按文件拆会留下 anchor 指向不存在代码的红提交。每个提交都在独立 worktree 里实测过门禁。门禁 3691 pass / 0 fail（跨两次稳定），212 个 anchor 全部唯一，dependencies 仍为空。
最重的一条：item 7 是语义反转，不是"扩写"
报告说
repairToolCallArguments会"按前缀把命令扩写成更长的命令"。实测比这严重一个量级——startsWith允许中段扩写，于是网关推翻了模型的决定而不是延长它：五个洞逐个实测封掉：token 边界、不引入新命令（
COMMAND_CHAINING_RE）、否定遮蔽（复用 NLU 层的maskNonActionableRegions而不是长第二份拷贝）、来源可信（原本读任意尾部 user 消息，于是<tool_result>载体让工具输出决定执行什么）、以及猜测的边界（裸模式正则捕获到行尾，run rm -rf varlog happened in the report整段进 argv 且无元字符可查）。封完之后发现的第五道门
Anthropic 在块级别区分附件与调用方原话，而翻译层把两者 join 进同一条消息，此后下游无法区分。实测：一份写着
run `npm install --force --unsafe-perm`的 document，让 repair 在模型选的npm install上追加了那两个 flag。修法是在翻译时标记已解码的 document 文本，在 repair 里遮蔽那些区间。遮蔽而非跳过整条消息是承重的——附件和真实指令共处一条，跳整条会把指令一起丢掉。
知情保留的残留：
ls -la→ls -la /etc/shadow仍会扩写，因为它与合法的node -p→node -p "1+1"形状逐字节同构，差别只在用户消息是指令还是粘贴内容——形状上不可判定。处置是可观测（命中记 warn + 记扩写前后），不是猜一个辩护不了的启发式。加一个假判据比留着它更糟，因为它会制造"已经防住了"的错觉。四条报告弄错了机制
item 9 报告点的分支不可达：
commit在!isResponseStoreEnabled() || !chainable早返，而chainable = hasPerUserScope(callerKey)在 callerKey 空时必 false，所以putResponse的!callerKey从唯一调用点永不触发——而那行正是唯一 bump_stats.rejected的地方。所以 item 9/10 定性为潜伏守卫，测试注释里明说这不是真实请求走得到的路径，不假装可达。item 10 修的是根（
storedResponseBody不越过最后一条 user）而不是堵那个调用点，因为调用点的不可达性是会变的前提，而"检索只认本轮的答案"不会变。实测旧行为：GET 返回"It is 4."而最后一个问题是"And 3+3?"，status报 completed。item 15 报告说"仅 Anthropic 做本地估算，其余三条报 0，计费中继按不同出口得出不同数字"。实测：有真值时四出口一致，Anthropic 的
message_start预填是规范要求且 delta 会修正。真缺陷窄得多且单向——上游沉默时 Responses 断言input_tokens: 0，对一个确实消耗了 prompt 的请求是假的，且静默少计费。现在省略该字段，与 Gemini 出口已有的做法一致。关键区分：上游明确报 0 是数据，上游沉默不是（缓存命中会合法报 0 completion tokens）。item 12/13/14 判定成立但不修，理由写在各自位置。item 12 报告归因
Atomics.wait，实测那条只在 EPERM/EBUSY 的 rename 重试里可达（POSIX 同文件系统 rename 不返回 EBUSY，那是 Windows 形态），真正阻塞事件循环的是writeFileSyncDurable里的fsync——照报告去改会改错地方。item 16：post-tool 文本
msgId在构造函数里铸一次、messageStarted一旦置位永不复位，于是工具调用之后的文本被并回第一个 item：在工具 item 创建时封段，于是
output_item.done自然升序，不需要重排finish()。store 刻意存全量文本：它持久化的是一条 OpenAI 形状的 assistant 消息、content是单个字符串，承载不了"文本-工具-文本"顺序，分段存会让链上下一轮丢掉工具前的文本。丢文本比丢顺序更糟，这条由一条突变钉住。W3：文档已对，但那个事实没有守卫
README:307已经写实了——它明说"这句原本写'租户间不可互读'，而那高估了保证的来源"，并指出真机密是 response id 的 ~90 bit 熵、"别把user当成访问控制用"。但那段说明依赖的事实没有任何测试钉住。我先验证了它要更正的前提：共享 API key 下，两个不同 caller（不同 IP、不同 UA）发同一个
user得到逐字节相同的 callerKey。所以 id 是唯一的机密。四条断言现在钉住它的宽度与随机性——若genResponseId被改窄，README 会静默变成假话，而这是本仓库反复栽的形状。验证口径
每条修复都带突变证据，含 over-reach 对照（让修复无条件生效的那条突变），因为没有它，"没修过头"这件事从未被测试过。
一条突变起初 SURVIVED，诊断值得记：把
COMMAND_CHAINING_RE收窄成/[&|]/活了下来，不是护栏有洞——抓它的 fixture 无反引号无句末标点，被新加的bounded门在链式检查之前就拒了，于是; > < $从来没承重过。补三条带反引号的 fixture 后 CAUGHT。这三种成因（更早的门 / 未满足的合取 / 活但不可驱动）处置相反，已写进AUDIT-LEDGER第十五轮。并发跑门禁时计数不可信：
--test-force-exit会在 stdout flush 完成前切断，而子进程仍退 0。同一棵树（shasum 逐字节相同）三次读数 3685 → 3687 → 3687。所以EXIT=0加跨次稳定才是证据，单次读数不是。Test plan
npm run test:release：3691 pass / 0 fail，跨两次一致mutate-verify-dirty.mjs，全部如声明1d5c416→ 3612/0（156 anchor）、b6e7dc9→ 3691/0（212 anchor）secret-scanEXIT=0，git diff --check干净，dependencies仍为空