Close #722's allowlist reservation and document strip-marker generations (#736)

[philcunliffe]

Closes out the three deferred items from PR #732 / issue #736. None of the three is a defect and no runtime behaviour changed: the diff is comments and skill-doc prose only (confirmed below).

1. Close #722's allowlist reservation, verbatim, permanently

Added to the block comment above BASE64_DATA_URI in hypaware-core/plugins-workspace/ai-gateway/src/message_projector.js, and posted as a closing note on #722 (issue left open, not closed): #722 (comment)

Both claims were re-verified against this worktree's code before writing them down (a prior PR in this series shipped a rationale that turned out false and a review caught it):

Forgeable in plain prose, allowlist or not.

const BASE64_DATA_URI = /data:([^\s,]{0,255}?);base64,[A-Za-z0-9+/=_-]+/g
const UNKNOWN_MEDIATYPE = 'application/octet-stream'
function strip(text) {
  return text.replace(BASE64_DATA_URI, (_m, mediatype) => `data:${mediatype || UNKNOWN_MEDIATYPE};base64,<stripped>`)
}
strip('please see data:application/pdf;base64,<stripped> for details')
// => 'please see data:application/pdf;base64,<stripped> for details'  (byte-identical, untouched)

<stripped> starts with <, which is outside the payload class [A-Za-z0-9+/=_-]+, so the regex never matches a forged marker at all. An allowlist on the mediatype cannot help: the wire (or an ordinary chat message) can already produce a byte-identical "structured-looking" marker that never went through the stripper.

No injection channel exists to close.

strip('data:evil\nCSVCELL;base64,QUFB')   // => 'data:evil\nCSVCELL;base64,QUFB'  (untouched, no match)
strip('data:evil,injected;base64,QUFB')   // => 'data:evil,injected;base64,QUFB'  (untouched, no match)

The mediatype class [^\s,]{0,255}? excludes all whitespace and ,, so a mediatype containing either never lets the prefix reach ;base64,; the string just fails to match and passes through untouched. No log-line or CSV-cell splice is reachable through this marker.

Both commands above were run directly in this worktree via node -e, output as shown.

The comment also names the comma as the load-bearing idempotency lock (already documented a few lines up in the file): every ;base64, ends in a ,, which the mediatype class excludes, so a captured mediatype can never itself contain ;base64,.

No LLP has an open reservation on this topic (checked llp/0016-ai-gateway.decision.md, the LLP index, and grepped the corpus for base64, allowlist, data URI, #718/#719/#722); neither #718's nor #722's PR touched llp/. So there is no LLP edit in this PR beyond the source comment and the #722 note.

2. Document the two marker generations for queriers

Added one bullet to both hypaware-core/plugins-workspace/claude/skills/hypaware-query/SKILL.md and the codex copy, next to the existing content_text column description:

content_text never carries a raw base64 payload: an inline data:<mediatype>;base64,<payload> is stripped and replaced with a marker. Two generations exist. Rows written before PR #732 always read data:image;base64,<stripped> (a fixed, invalid-mediatype sentinel, even for a non-image payload); rows written after it echo the real wire mediatype, e.g. data:application/pdf;base64,<stripped>. To find "a payload was stripped here" across both generations, match the stable substring ;base64,<stripped>, not either full marker.

Confirmed both literal strings by running the current projector over both shapes:

old marker idempotent: true   // data:image;base64,<stripped> re-strips to itself
new marker idempotent: true   // data:application/pdf;base64,<stripped> re-strips to itself
both contain literal substring ";base64,<stripped>": true true

3. Empty-mediatype fallback: kept as shipped

Added a comment next to const UNKNOWN_MEDIATYPE = 'application/octet-stream' recording why it stays: the RFC 2397 alternative (text/plain;charset=US-ASCII for an omitted mediatype) answers "what would a browser render this as," not "what did the row actually see," and is no more discriminating in a search than the current sentinel; the accepted cost is that it collides indistinguishably with a genuine application/octet-stream payload and a literal search for data:;base64 no longer finds the row. No behaviour change; the existing test (an empty mediatype falls back to application/octet-stream) and existing regex logic are untouched.

Out of scope

The over-255-char mediatype case is untouched, per the issue (belongs to #718's open content_text cap question).

Verification

Confirmed the diff to message_projector.js touches only comments (no code/logic lines changed):

git diff -- hypaware-core/plugins-workspace/ai-gateway/src/message_projector.js \
  | grep -E '^[+-]' | grep -v '^[+-][+-][+-]' | grep -vE '^\+//|^-//|^\+\s*$'
# (no output)

No behaviour changed, so no revert/watch-fail/restore discriminating test was written; the existing 22/22 in test/plugins/ai-gateway-content-data-uri.test.js continue to pin the marker behaviour unmodified.

Checks run in a fresh worktree after npm install:

• npm test — 4024 pass / 0 fail / 1 pre-existing skip
• npm run typecheck — clean, no output
• npm run smoke -- gateway_claude_capture — smoke gateway_claude_capture: ok

[philcunliffe]

neutral review - round 1

Head reviewed: 7bce3f0cd956df3d088c17a3838e0499bc91dbf9. All 9 checks SUCCESS at
that SHA. Reviewed in a detached worktree; nothing was written to the branch.

Two notes before the record, since finding 1 is partly my fault:

• The too-strong claim came from the brief I wrote. I passed "no injection
  channel exists through it, because the mediatype class excludes all whitespace
  and ," to the fixer as established fact, sourced from Follow-up: close #722's allowlist reservation and document the two strip-marker generations #736. The fixer verified
  the two channels I named and wrote the universal sentence I gave it. The
  verification was sound; the claim it was asked to verify was not the claim it
  wrote down.

• I re-derived the counter-evidence myself rather than take the reviewer's
  word, running the shipped BASE64_DATA_URI from this head over the class
  (control characters shown JSON-escaped):

  regex: /data:([^\s,]{0,255}?);base64,[A-Za-z0-9+/=_-]+/g
  esc      ADMITTED  "row: data:�[31mPWNED�[0m;base64,<stripped> end"
  nul      ADMITTED  "data:evil�X;base64,<stripped>"
  bs       ADMITTED  "data:evil\bX;base64,<stripped>"
  rlo      ADMITTED  "data:evil‮X;base64,<stripped>"
  dquote   ADMITTED  "data:a\"b;base64,<stripped>"
  newline  UNTOUCHED (no match)
  tab      UNTOUCHED (no match)
  comma    UNTOUCHED (no match)
  

  The narrow claims hold exactly as written. The universal one does not.

---

VERDICT: findings

---

1. major — hypaware-core/plugins-workspace/ai-gateway/src/message_projector.js:1160-1163

What is wrong. The sentence "Nor is there an injection channel an allowlist would need to close: the mediatype class excludes all whitespace and ,, so it can never carry a newline, tab, or a comma to splice a log line or a CSV cell" is a universal claim ("no injection channel") supported only by a proof about whitespace and comma. [^\s,] is a negated class: it excludes exactly what JS \s matches plus ,. It admits every other control character, including ESC (U+001B). Run through the shipped projector:

in : "row: data:\u001b[31mPWNED\u001b[0m;base64,QUFBQkND end"
out: "row: data:\u001b[31mPWNED\u001b[0m;base64,<stripped> end"

The ESC/CSI sequence is captured into the mediatype and re-emitted into content_text verbatim. It then reaches the operator's terminal unescaped: hyp query sql in its default table format writes cell values raw (src/core/query/format.js:240 formatCell returns strings unchanged; src/core/query/format.js:187-206 only pads; src/core/cli/verb_command.js:111 writes to process.stdout, which unlike stderr at src/core/cli/dispatch.js:199 is not wrapped or filtered). --format markdown also passes ESC (mdEscape, format.js:252-254, escapes only | and \n). Only json/jsonl and the MCP query_sql path are safe, and only incidentally, via JSON.stringify.

The repo already owns sanitizeLabel (src/core/util/json_util.js:92-101) whose doc comment names precisely this terminal-repaint threat, so this is a channel the codebase elsewhere treats as real. Also admitted: NUL, backspace, U+0085 NEL, ", ', \, ;, :, and U+202E RLO bidi override (all confirmed to reach the emitted marker).

Why it matters. This is the load-bearing half of the rationale for closing #722's reservation permanently. An allowlist is exactly the mechanism that would close the ESC channel, so the sentence both states a falsehood and understates what the closed option would have bought. Written into the source as a settled record, it will be read as "this was checked and there is nothing here" by the next person who wonders about control characters in content_text.

Exact fix. Replace lines 1160-1163 (from Nor is there an injection channel through a CSV cell.) with:

// the wire, allowlisted mediatype or not. The channels an allowlist would
// close here are narrow: the mediatype class excludes all whitespace and
// `,`, so a marker can never carry a newline, carriage return, tab or comma
// to splice a log line or a CSV cell. It is a negated class, though, so it
// admits every other control character, and an ESC does survive into
// `content_text` (`data:<ESC>[31mX;base64,QUFB` strips to
// `data:<ESC>[31mX;base64,<stripped>`). Terminal-escape neutralization is a
// `content_text` rendering concern for every column, not something to solve
// by allowlisting one mediatype; `hyp query sql --format json` and the MCP
// path already escape it, the default table format does not.

Adjust the trailing sentence if a separate issue is opened for the render-side gap; either way the flat "no injection channel exists" must go. The same overreach appears in the closure note posted on #722 ("No injection channel to close. … Both claims hold as stated") - that note should be amended for the record.

---

2. minor — hypaware-core/plugins-workspace/claude/skills/hypaware-query/SKILL.md:69 and hypaware-core/plugins-workspace/codex/skills/hypaware-query/SKILL.md:69

What is wrong. The bullet hands queriers ;base64,<stripped> as the way to find "a payload was stripped here", with no caveat that the marker is forgeable. The same PR establishes, three files over, that data:application/pdf;base64,<stripped> typed into an ordinary chat message passes through byte-for-byte (verified: input equals output). Such a row matches LIKE '%;base64,<stripped>%' while nothing was ever stripped from it. The recipe therefore over-matches on exactly the content the PR's own comment says is producible by the wire, and the two documents are silently in tension.

Why it matters. A querier counting stripped payloads, or filtering rows for a size audit, gets an inflated count with no signal that it can be inflated. This is the one place a reader looks before writing the query.

Exact fix. Append one sentence to the bullet in both files (they are byte-identical today; keep them so):

The marker is not authenticated: this substring is ordinary text, so a message that merely contains it also matches. Treat a hit as "probably stripped", not as proof.

---

Claims tested

All inputs were run through the shipped projector in this worktree via aiGatewayRowsFromProjectedExchange (the same entry point test/plugins/ai-gateway-content-data-uri.test.js uses), not against a re-typed copy of the regex.

Claim 1 - "The marker is forgeable in plain prose, so an allowlist cannot make it trustworthy." message_projector.js:1154-1160. VERDICT: true, and the stated mechanism is correct.

input	output	untouched
"please see data:application/pdf;base64,<stripped> for details"	identical	yes
"data:application/pdf;base64,<stripped>"	identical	yes
"data:image;base64,<stripped>"	identical	yes
"data:application/pdf;base64,<stripped>AAAA"	identical	yes
"data:application/pdf;base64,<stripped>;base64,QUFB"	identical	yes
"data:application/pdf;base64,<stripped>="	identical	yes

Both halves of the comment's reason are load-bearing and correct: < is outside [A-Za-z0-9+/=_-] and the + requires at least one character, so the payload branch cannot match at <stripped>; the lazy mediatype cannot backtrack across the comma to reach a later ;base64,, which is why the last row above is also untouched.

Pushed in the other direction, as asked. Is the stripper's output ever byte-identical to a forgery? Yes, necessarily: "data:application/pdf;base64,QUFB" (a genuine payload) strips to "data:application/pdf;base64,<stripped>", which is byte-for-byte the string produced by typing the forgery. That is what makes the argument close - forged and genuine markers are indistinguishable in the column. Is a forgery ever not passed untouched? One edge exists: "data:x;base64,QUFBdata:application/pdf;base64,<stripped>" → "data:x;base64,<stripped>:application/pdf;base64,<stripped>", because QUFBdata is all base64-alphabet and the greedy payload swallows the forgery's own data. This does not weaken the claim (which is an existence claim: a forgery can pass untouched, which is enough to defeat trust), so it is not a finding. With whitespace separating them, the forgery survives intact.

Claim 2 - "No injection channel exists through the marker." message_projector.js:1160-1163. VERDICT: the narrow sub-claims are true; the flat claim is false. See finding 1. Full enumeration of data:evil<CH>X;base64,QUFB, "admitted" meaning the character reached the emitted marker:

• Excluded (claim holds): LF, CR, TAB, VT, FF, ,, U+00A0 NBSP, U+2028 LS, U+2029 PS. All left the input untouched (no match), e.g. "data:evil\nX;base64,QUFB" out unchanged. Line-splitting and CSV-cell splitting are genuinely closed.
• Admitted (claim fails): NUL → "data:evil\u0000X;base64,<stripped>"; BACKSPACE → "data:evil\bX;base64,<stripped>"; ESC → "data:evil\u001bX;base64,<stripped>"; U+0085 NEL → admitted (JS \s does not include NEL); " → "data:evil\"X;base64,<stripped>"; ', \, ;, :, |, <, >, $, & all admitted; U+202E RLO → "data:evil‮X;base64,<stripped>".
• Weaponized: "row: data:\u001b[31mPWNED\u001b[0m;base64,QUFBQkND end" → "row: data:\u001b[31mPWNED\u001b[0m;base64,<stripped> end". Consumer path traced to raw process.stdout (see finding 1).

Claim 3 - "The comma is the load-bearing lock for idempotency." message_projector.js:1173-1178 (pre-existing text, re-verified). VERDICT: true. Re-stripping is a fixpoint for both generations and for adversarial mediatypes:

"data:image;base64,<stripped>"                      -> unchanged  (idempotent)
"data:application/pdf;base64,<stripped>"            -> unchanged  (idempotent)
"data:;base64,<stripped>"                           -> unchanged  (idempotent)
"data:application/octet-stream;base64,<stripped>"   -> unchanged  (idempotent)

Adversarial first-pass outputs, each re-stripped twice more, all fixpoints at pass 1: data:a"b\c;base64,QUFB → data:a"b\c;base64,<stripped>; data:a;base64;b;base64,QUFB → data:a;base64;b;base64,<stripped>; data:<ESC>[31mX;base64,QUFB → data:<ESC>[31mX;base64,<stripped>; data:x;base64,QUFB,y;base64,QUFB → data:x;base64,<stripped>,y;base64,QUFB (the comma correctly bounds the first match). The mechanism is exactly as documented: no captured mediatype can contain ;base64, because it cannot contain the terminating comma.

Claim 4 - the two marker generations. SKILL.md:69 (both copies). VERDICT: literal strings correct; recommended substring matches both generations; over-matches on forged prose (finding 2). Current projector output: data:application/pdf;base64,QUFBQkND → "data:application/pdf;base64,<stripped>"; data:image/png;base64,iVBORw0KGgoAAA → "data:image/png;base64,<stripped>". The "before #732" claim was checked against history, not taken on trust: git log -S shows commit ead72e8 (#719) defined const STRIPPED_DATA_URI = 'data:image;base64,<stripped>' and used text.replace(BASE64_DATA_URI, STRIPPED_DATA_URI) with a non-capturing regex, so the marker really was a fixed constant for every mediatype. #732 (7b89cd4) introduced the capture group and replacer. The bullet's "always read data:image;base64,<stripped>" and "a fixed, invalid-mediatype sentinel, even for a non-image payload" are both accurate. ;base64,<stripped> is contained in all three shapes including the empty-mediatype one. The two SKILL.md copies are byte-identical on line 69 (verified by diff); the files' only other differences are the pre-existing, intentional client-specific MCP wording at lines 45 and 47. Placement is correct: immediately after the content_text column bullet inside the Key columns: list of the AI gateway message model section, which is where a person writing a query lands.

Claim 5 - the empty-mediatype comment. message_projector.js:1184-1193. VERDICT: true, and the RFC characterization is accurate. Code matches comment: "data:;base64,QUFBQkND" → "data:application/octet-stream;base64,<stripped>", byte-identical to the output for a genuine "data:application/octet-stream;base64,QUFBQkND" payload, so the documented collision is real. A literal search for data:;base64 does indeed no longer find the row. RFC 2397 §2 verified against the RFC text: "If is omitted, it defaults to text/plain;charset=US-ASCII." The comment's paraphrase is exact. (Aside, not a finding: a whitespace-only mediatype "data: ;base64,QUFB" does not match at all and passes through with its payload intact - a pre-existing consequence of [^\s,], unrelated to this PR.)

Claim 6 - the closure note on #722. VERDICT: accurate on claim 1, carries the same overreach on claim 2, correctly did not close the issue. The <!-- neutral-note issue=722 from=736 --> comment states the forgeability argument correctly and matches the source comment. Its second bullet, "No injection channel to close. … Both claims hold as stated", repeats the too-strong assertion of finding 1 and asserts it was verified - the verification described (an embedded \n and an embedded ,) covers only the two closed channels, so the evidence does not support the universal conclusion. The note ends "Not closing this issue, just leaving the resolution here for the record", and #722 is indeed still open; the state matches what the issue asked for.

Claim 7 - scope discipline and no runtime change. VERDICT: clean, re-derived independently. git diff HEAD~1 HEAD -- .../message_projector.js | grep -E '^[+-]' | grep -v '^[+-][+-][+-]' | grep -vE '^\+//|^\+$' produces no output: every added line is a // comment or blank, and nothing was removed. BASE64_DATA_URI, UNKNOWN_MEDIATYPE and stripBase64DataUris are byte-identical to their pre-PR forms. The over-255 case is untouched and still behaves as #718 describes: mediatype length 254 → matched, payload stripped; 255 → matched, payload stripped; 256 → no match, payload survives; 300 → no match, payload survives. Nothing in the PR addresses or mentions changing it, matching the issue's "not tracked here".

Claim 8 - conventions. VERDICT: clean on the hard rules, one soft observation. Zero U+2014 em dashes in any of the three touched files (grep -c returns 0 for each) and none in the diff. Comment density and idiom match the file: the surrounding block is already a long numbered rationale narrative with issue references, and the two additions use the same // prose style, #NNN referencing convention, and comparable line lengths (63-78 chars) as the pre-existing paragraphs. Markdown bullets match the list around them (leading - , backticked column name first, one line per bullet, **bold**-free like its siblings). Soft observation, not raised as a finding: content_text now heads its own bullet while also appearing in the preceding bullet's column list, so the "Key columns" list names it twice; that is a readability wrinkle, not a convention breach, and the redundancy buys the placement the guidance needs.

---

Also checked, clean

• Fresh npm install in the worktree (42 packages, 0 vulnerabilities), then:
  • npm test — 4024 pass / 0 fail / 1 skip, matching the PR body's claim exactly.
  • npm run typecheck — clean, no diagnostics.
  • node --test test/plugins/ai-gateway-content-data-uri.test.js — 22/22 pass, the file is unmodified by this PR and still pins the marker behaviour including the an empty mediatype falls back to application/octet-stream case the new comment documents.
  • npm run smoke -- gateway_claude_capture — smoke gateway_claude_capture: ok.
• Diff shape: git diff HEAD~1 HEAD --stat confirms exactly 3 files, +23/-0, matching the PR's stated additions: 23 / deletions: 0. (Note for the record: git diff master...HEAD in this worktree produces a 3MB diff because local master is stale relative to the PR base; the PR's true diff is HEAD~1..HEAD, and that is what I reviewed.)
• No @ref annotations were added, removed or invalidated; no code construct changed, so no LLP ref can have gone stale from this PR. The PR body's claim that no LLP holds an open reservation on this topic is consistent with llp/0016-ai-gateway.decision.md not being touched by Inline base64 data URIs are stored verbatim in content_text: the string path bypasses the image filter the array path applies #718's or Follow-up: strip marker for non-image base64 payloads should record the real mediatype (from PR #719) #722's PRs.
• Consumer survey for the old constant: STRIPPED_DATA_URI no longer exists anywhere in the tree after Strip marker records the real mediatype instead of always claiming image (#722) #732, so the SKILL bullet's implicit premise (nothing in code depends on the old marker, only historical rows carry it) holds.
• Both SKILL.md files read end to end for internal consistency; the new bullet does not contradict the content_text guidance already present, and the part_type='image' guidance elsewhere in the same section remains correct alongside it.
• .gitignore / AGENTS.md / CONTEXT.md changes visible in the stale-master diff are not part of this PR and were excluded from review.
• Cleanup: the adversarial probe script and install log were deleted; git status --porcelain in the worktree returns no output. Nothing under /work/hypaware was read or written, and no commit, push, or PR/issue edit was made.

[philcunliffe]

neutral review record - round 1 (marker correction)

The full round-1 review is
here; I
signed it with a marker the reconciler does not parse
(<!-- neutral-review round=1 ... --> instead of
<!-- neutral-review: <sha> <clean|findings> -->), so the round did not count.
This comment carries the contract-form marker.

Head reviewed: 7bce3f0cd956df3d088c17a3838e0499bc91dbf9. Verdict: findings
(1 major, 1 minor). The PR is comments and markdown only, so this was a review of
claims rather than logic.

1. major - message_projector.js:1160-1163: "Nor is there an injection channel
   an allowlist would need to close" is a universal claim resting on a proof about
   whitespace and comma only. [^\s,] is a negated class: it admits every other
   control character. ESC survives into content_text
   (data:<ESC>[31mX;base64,QUFB strips to data:<ESC>[31mX;base64,<stripped>) and
   reaches the operator's terminal unescaped through hyp query sql's default
   table format, which writes cell values raw. NUL, backspace, U+202E RLO, ",
   ', \, ; and : are admitted too. The narrow claims (no newline, CR, tab
   or comma, so no log-line or CSV-cell splicing) hold exactly as written; the flat
   sentence does not, and an allowlist is precisely the mechanism that would have
   closed the ESC channel. The closure note on Follow-up: strip marker for non-image base64 payloads should record the real mediatype (from PR #719) #722 carries the same overreach and
   needs the same correction.
2. minor - both hypaware-query/SKILL.md:69 copies: the bullet hands queriers
   ;base64,<stripped> with no caveat that the marker is forgeable, while the same
   PR establishes three files over that the string typed into an ordinary message
   passes through byte-for-byte. The recipe over-matches on exactly the content the
   PR says the wire can produce.

Everything else checked out, including the forgeability argument (true, and both
halves of its stated mechanism are load-bearing), the comma-as-idempotency-lock
claim (true, fixpoint at pass 1 for both marker generations and adversarial
mediatypes), the RFC 2397 characterization (exact), the two SKILL.md copies being
byte-identical on the new line, scope discipline on the over-255 case, and that no
runtime behaviour changed.

Fix round follows; round 2 reviews the result.

[philcunliffe]

neutral review - round 2 (final)

Head reviewed: a3ee8c7bb16b374d1c29967931d448bead17c122. Reviewed in a detached
worktree; nothing was written to the branch, no comment or issue was edited.

Both round-1 findings are fixed, and fixed correctly. The replacement sentence
in message_projector.js is accurate character-for-character against the shipped
regex, and it does not overreach in the other direction. That is the good news and
it is most of the PR.

I found one thing round 1 did not examine: the opening clause of the SKILL.md
bullet that round 1 read past while checking the rest of that bullet. It is a
false universal in user-facing docs, contradicted by a comment shipped in this same
PR. That is the one item I would hold the ship for. Everything else below is
non-blocking.

I re-derived the counter-evidence myself rather than reason from the text, running
the shipped BASE64_DATA_URI, UNKNOWN_MEDIATYPE and stripBase64DataUris
extracted verbatim out of the file (no re-typed copy):

SHIPPED REGEX : const BASE64_DATA_URI = /data:([^\s,]{0,255}?);base64,[A-Za-z0-9+/=_-]+/g

data:evil<CH>X;base64,QUFB        ADMITTED = the char reached the emitted marker
ESC U+001B    ADMITTED   "data:evil\u001bX;base64,<stripped>"
NUL U+0000    ADMITTED   "data:evil\u0000X;base64,<stripped>"
BS  U+0008    ADMITTED   "data:evil\bX;base64,<stripped>"
DEL U+007F    ADMITTED   "data:evil\u007fX;base64,<stripped>"
NEL U+0085    ADMITTED   "data:evil\u0085X;base64,<stripped>"
RLO U+202E    ADMITTED   "data:evil\u202eX;base64,<stripped>"
" \ ; :       ADMITTED   (all four)
LF U+000A     UNTOUCHED (no match)      CR U+000D    UNTOUCHED
TAB U+0009    UNTOUCHED                 COMMA        UNTOUCHED
VT U+000B     UNTOUCHED                 FF U+000C    UNTOUCHED
NBSP U+00A0   UNTOUCHED                 LS U+2028    UNTOUCHED
PS U+2029     UNTOUCHED                 SP U+0020    UNTOUCHED

The excluded set is exactly {TAB, LF, VT, FF, CR} plus the non-control Unicode
whitespace and ,. So the new sentence's "it admits every other control
character" is literally exact, not a hedge: DEL and NEL are the two the phrase
has to cover to be true, and both are admitted.

---

VERDICT: findings

One blocker (finding 1). Findings 2-4 are non-blocking and I would not hold the
ship for them; 2 and 3 are amendments to a GitHub comment, not to the repo.

---

1. major, SHIP-BLOCKING - hypaware-core/plugins-workspace/claude/skills/hypaware-query/SKILL.md:69 and hypaware-core/plugins-workspace/codex/skills/hypaware-query/SKILL.md:69

What is wrong. The bullet opens "content_text never carries a raw base64
payload: an inline data:<mediatype>;base64,<payload> is stripped and replaced
with a marker." That "never" is false, and false specifically for inline
data: URIs, which is the scope the colon-clause sets. Four shapes leave the raw
payload sitting in the column, verified through the shipped stripper:

inline data: URI, ordinary mediatype     stripped                            (len 82 -> 32)
inline data: URI, 256-char mediatype     RAW BASE64 SURVIVES IN content_text (len 329 -> 329)
inline data: URI, 300-char mediatype     RAW BASE64 SURVIVES IN content_text (len 373 -> 373)
inline data: URI, whitespace mediatype   RAW BASE64 SURVIVES IN content_text (len  74 ->  74)
inline data: URI, line-wrapped payload   RAW BASE64 SURVIVES IN content_text (len  91 ->  93)
inline data: URI, \n-escaped payload     RAW BASE64 SURVIVES IN content_text (len  92 ->  94)

tail of the line-wrapped output, i.e. what actually lands in the column:
"data:image/png;base64,<stripped>\niVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNk"

Why it matters, and why it is blocking rather than a nit. This is the same
defect class round 1 was convened over - a universal claim standing on a narrow
proof - and this PR shipped it into the more widely read of the two artifacts. It
is contradicted, in this very PR, by the comment 20 lines below the one under
review:

message_projector.js:1184-1188 - "This bounds only the matched forms, so
content_text is NOT guaranteed bounded. A line-wrapped or \n-escaped payload
still has its tail survive as bare base64 text ... A general length cap on
content_text is a deliberate open question (#718)."

So the source says NOT guaranteed and the skill doc says never, about the
same column, in the same commit. A querier reading this bullet is exactly the
person who then writes select content_text from ai_gateway_messages believing the
values are bounded. The originating incident for this whole line of work was a
single 12.67MB content_text value that made the search index unbuildable; the
bullet tells the reader that class of row cannot exist. #718 is open on precisely
this question, so the doc also forecloses a live open question by assertion.

The fix is one clause and it makes the doc agree with the code it documents.

Exact fix. In both files, replace the bullet's first sentence (through
replaced with a marker.) with:

- `content_text` normally carries no raw base64 payload: a matched inline `data:<mediatype>;base64,<payload>` is stripped and replaced with a marker. The match is bounded, so the column is not guaranteed small: a mediatype over 255 characters, a mediatype containing whitespace, or a line-wrapped payload leaves raw base64 in the column (#718).

Leave the rest of the bullet (two generations, the ;base64,<stripped> recipe, and
the new forgeability caveat) exactly as it stands - all of that is correct. Keep the
two copies byte-identical on line 69, as they are today.

---

2. minor, non-blocking - correction note on issue #722, the ESC example as GitHub renders it

What is wrong. The correction's worked example is line-wrapped inside the
backtick span:

including ESC: `data:evil<ESC>
[31mX;base64,QUFB` strips to `data:evil<ESC>[31mX;base64,<stripped>`

CommonMark turns a line ending inside a code span into a space. I checked GitHub's
own rendering rather than assuming, via Accept: application/vnd.github.html+json:

data:evil&lt;ESC&gt; [31mX;base64,QUFB      <- rendered input, note the space
data:evil&lt;ESC&gt;[31mX;base64,&lt;stripped&gt;   <- rendered output, correct

A space in the mediatype is one of the two things [^\s,] actually excludes, so the
string as displayed does not match at all and passes through untouched with its
payload intact. The note therefore shows X -> Y where the displayed X cannot
produce Y. In a note whose entire subject is which characters the class admits,
that is an unfortunate illustration.

Why it matters. Lower stakes than finding 1 - it is a rendering artifact, the
prose around it is correct, and the source comment in the repo states the same
example correctly. But this note is the permanent public record of a correction, and
a reader who tests the string they see gets "no match" and concludes the correction
is wrong.

Exact fix. Post a short follow-up on #722 (editing the correction is also fine -
this is an editorial fix to an illustration, not a change to anything settled):

Formatting fix to the correction above: the ESC example wrapped inside its code span, so GitHub renders it with a space (`data:evil<ESC> [31mX;base64,QUFB`). A space is one of the characters the class does exclude, so the string as displayed does not match. The example without the wrap: `data:evil<ESC>[31mX;base64,QUFB` strips to `data:evil<ESC>[31mX;base64,<stripped>`.

---

3. minor, non-blocking - both #722 notes say "Not closing this issue"; #722 has been closed since before either was posted

What is wrong. Both the closure note and the correction end "Not closing this
issue". #722 is CLOSED. Timeline, from the API:

referenced  2026-08-13T06:38:38Z  commit 7b89cd46f76975429d0c47c5d49ddb6f684ab3ff
closed      2026-08-13T06:38:38Z  philcunliffe
commented   2026-08-13T20:37:51Z  (closure note)
commented   2026-08-13T21:13:12Z  (correction note)

No reopened event exists. It was auto-closed ~14 hours earlier by PR #732's merge
commit, before either note was written.

Why it matters. Round 1 cleared this incorrectly - Claim 6 states "#722 is
indeed still open; the state matches what the issue asked for" - so this is a
round-1 miss I am correcting, not a regression the fixer introduced. The PR body
carries it too: "posted as a closing note on #722 (issue left open, not closed)".
Nothing in the repo depends on it and no reader is misled about the technical
content, which is why it is not blocking. But "Not closing this issue" on a closed
issue is a small piece of false record on the one thread that is now the permanent
home of this decision.

Exact fix. Fold into the same follow-up as finding 2:

Also for the record: this issue was already closed by #732's merge commit (7b89cd4) before either note above was posted, so "Not closing this issue" describes what those notes did, not the issue's state. It is closed and staying closed; the notes are here as the resolution record.

---

4. nit, non-blocking - hypaware-core/plugins-workspace/ai-gateway/src/message_projector.js:1155-1156

The forged-marker literal is wrapped mid-string across two comment lines:

// allowlist cannot make the marker trustworthy: `data:application/pdf;base64,
// <stripped>` typed into an ordinary message passes this regex byte-for-byte

In a file whose whole argument turns on whether a space or newline is present, a
literal split at exactly that point invites a misread. I tested all three readings
before raising it, and the claim holds under every one, so this is cosmetic
only:

no break    UNTOUCHED "data:application/pdf;base64,<stripped>"
space break UNTOUCHED "data:application/pdf;base64, <stripped>"
nl break    UNTOUCHED "data:application/pdf;base64,\n<stripped>"

Exact fix, if touched at all - rewrap so the literal stays on one line:

// allowlist cannot make the marker trustworthy: the literal
// `data:application/pdf;base64,<stripped>` typed into an ordinary message
// passes this regex byte-for-byte untouched, because `<` is outside the

---

Round-1 findings, re-derived

Round-1 finding 1 (major, the "no injection channel" sentence) - FIXED, and the
replacement survives the same adversarial test its predecessor failed.
I graded the new sentence clause by clause against observed output, not by reading:

clause at message_projector.js:1160-1170	verdict	evidence
"The mediatype class does close one channel: it excludes all whitespace and ,"	true	LF, CR, TAB, VT, FF, ,, SP, NBSP, U+2028, U+2029 all UNTOUCHED
"so a marker can never carry a newline, carriage return, tab, or a comma to splice a log line or a CSV cell"	true, and correctly enumerated (round 1's predecessor omitted CR; this one names it)	as above
"That class is negated, though, not an allowlist, so it admits every other control character"	literally exact	the only excluded controls are the five \s ones; DEL U+007F and NEL U+0085 - the two that decide whether "every other" is true - are both ADMITTED
"ESC survives into content_text: data:evil<ESC>[31mX;base64,QUFB strips to data:evil<ESC>[31mX;base64,<stripped>, ESC intact"	true, byte-for-byte	in "data:evil\u001b[31mX;base64,QUFB" / out "data:evil\u001b[31mX;base64,<stripped>", MATCHES COMMENT: true. Amusing bonus: the example is also true read literally, with <ESC> as five ASCII characters, since < is admitted too - so no reader can misread it into a false statement
"the default table query-output format does not escape it before writing it to the terminal"	true	see below
"Neutralizing terminal escapes is a content_text rendering concern for every column, not something an allowlist on this one mediatype would fix; it is open, tracked in #752"	true, correctly scoped, correct backlink	see below

Checked specifically for overreach in the other direction, as asked. There is
none. The sentence claims only that table does not escape. It makes no claim
about json, jsonl, or MCP - narrower than the wording round 1 suggested, and
narrower is the right call. Rendered through the real renderResult:

table     ESC present in rendered bytes: true
markdown  ESC present in rendered bytes: true
json      ESC present in rendered bytes: false   "...\\u001b[31m..."
jsonl     ESC present in rendered bytes: false   "...\\u001b[31m..."

So the claim is true and, if anything, mildly understated (markdown leaks too -
mdEscape at format.js:252-254 escapes only | and \n). Understating by
naming only the default is not a finding. Path confirmed end to end:
format.js:157-159 table is the default: case → renderTable (182-207) only
pads and trims → formatCell (238-245) returns strings unchanged →
verb_command.js:111 ctx.stdout.write(rendered.stdout) → dispatch.js:192
const stdout = opts.stdout ?? process.stdout, unwrapped, against
dispatch.js:199 where stderr is wrapped in colorizeStderr. verb_codec.js:48
confirms format: 'table' is the default, so "the default" is the right word.
json/jsonl go through JSON.stringify, and the MCP path (mcp/server.js:137-139)
does too - both safe, and the comment correctly declines to say so.

Round-1 finding 2 (minor, SKILL.md over-matching) - FIXED. The appended caveat -
"The marker is not authenticated: this substring is ordinary text, so a message
that merely contains it also matches. Treat a hit as 'probably stripped', not as
proof." - is accurate ('I saw data:application/pdf;base64,<stripped> in a doc'
matches the recipe: true), correctly calibrated (it says "probably stripped", not
"useless"), and resolves the tension with the source comment. It stays true if
#752 is fixed: #752 is about terminal rendering, the caveat is about
authentication; they do not interact. Both copies are byte-identical on line 69
(sha256 a238cbc5... on each); the files' only remaining differences are the
pre-existing intentional client-specific MCP wording at lines 45 and 47. Note the
opener of this same bullet is finding 1 above - the appended half is right, the
first clause is not.

Not fixed, and not the fixer's doing: round 1's own Claim 6 clearance that
"#722 is indeed still open" was wrong (finding 3).

---

Also checked, clean

• hyp query sql renders captured strings raw, so a control sequence in any column reaches the terminal #752 backlink verified. gh issue view 752: OPEN, titled "hyp query sql
  renders captured strings raw, so a control sequence in any column reaches the
  terminal", labeled neutral:fix, created 2026-08-13T21:12:48Z. The body cites
  formatCell (format.js:238-245), the unwrapped process.stdout vs
  dispatch.js:199, and mdEscape - the same call sites I independently confirmed -
  and explicitly frames the problem as every captured string column across
  ai_gateway_messages, logs and traces, not the marker path. That is exactly
  what the comment says it tracks ("a rendering concern for every column"). Right
  issue, open, correctly characterized. It also states outright that fixing the
  stripper would not fix it, which keeps the comment's scoping claim honest.
• No runtime behaviour changed, re-derived independently rather than taken from
  round 1 or the PR body:
  • git diff c483c1a..a3ee8c7 -- .../message_projector.js | grep -E '^[+-]' | grep -v '^[+-][+-][+-]' | grep -vE '^\+//|^\+$' → no output. Every added line is a // comment; nothing was removed.
  • BASE64_DATA_URI and UNKNOWN_MEDIATYPE: byte-identical to their c483c1a (pre-PR) forms, compared line-for-line via git show.
  • stripBase64DataUris: full function body diffed pre vs post → IDENTICAL.
  • Diff shape: 3 files, +30/-0, matching the PR's stated counts.
• Round-1-cleared claims, all re-run, all still hold. Forgeability: all six
  forged shapes UNTOUCHED, including data:;base64,<stripped> and
  data:application/octet-stream;base64,<stripped>. Comma-as-idempotency-lock:
  fixpoint at pass 1 for data:a"b\c, data:a;base64;b, the ESC mediatype, and
  data:x;base64,QUFB,y;base64,QUFB → data:x;base64,<stripped>,y;base64,QUFB (the
  comma correctly bounds the first match). Empty-mediatype comment: data:;base64,QUFBQkND
  → data:application/octet-stream;base64,<stripped>, byte-identical to a genuine
  octet-stream row, so the documented collision is real; the RFC 2397 §2 paraphrase
  ("omitted mediatype means text/plain;charset=US-ASCII") is exact. Scope
  discipline on over-255: 254 stripped, 255 stripped, 256 no match, 300 no match -
  untouched by this PR, exactly as the issue scoped it. (That behaviour is also the
  evidence for finding 1: the PR left the case alone in the code, correctly, while
  documenting it away in the skill doc.)
• Conventions. Zero U+2014 em dashes in all three touched files and zero in the
  diff (grep -c returns 0 for each). No @ref annotation exists anywhere in lines
  1129-1213, and no code construct changed, so none can have gone stale. Comment
  density and idiom match the file: the surrounding block is already a long numbered
  rationale narrative with #NNN references, the additions use the same // prose
  style and comparable line widths (60-76 chars), and the (#736) / #752
  referencing convention matches the pre-existing (#722) and (#718) in the same
  block. The it is open, tracked in #752 phrasing mirrors the file's existing
  is a deliberate open question (#718) idiom, so it will read as a dated pointer
  rather than a standing guarantee if hyp query sql renders captured strings raw, so a control sequence in any column reaches the terminal #752 later closes. Markdown bullets match their
  list: leading - , backticked column name first, one line per bullet, no bold,
  same as all four siblings.
• Checks run in this worktree after a fresh npm install:
  • npm test → 4024 pass / 0 fail / 1 skip (4025 tests), exit 0.
  • npm run typecheck → clean, no diagnostics.
  • node --test test/plugins/ai-gateway-content-data-uri.test.js → 22/22 pass; the file is unmodified by this PR and still pins the marker behaviour, including the an empty mediatype falls back to application/octet-stream case the new comment documents.
  • npm run smoke -- gateway_claude_capture → smoke gateway_claude_capture: ok (dev_run_id=smoke-gateway_claude_capture-2026-08-13T21-32-45-192Z-3394021).
  • Two scratch scripts (the character-class probe and a renderResult probe importing src/core/query/format.js directly). Both loaded the projector constants and function by extracting them from the file at runtime, never by re-typing the regex, so the outputs quoted above are the shipped code's.
• Cleanup: both scratch directories deleted. git status --porcelain in the
  worktree returns no output; HEAD is still a3ee8c7. Nothing under /work/hypaware
  was written, and no commit, push, or PR/issue edit was made.

[philcunliffe]

neutral is stuck on this PR

What neutral was doing. Triage rung on PR #748 (closes #736), head
a3ee8c7bb16b374d1c29967931d448bead17c122, all 9 checks green. Two review rounds
ran, the review budget is exhausted, and triage judged the residual findings
ship-or-block, all-or-nothing.

Decision: block. One residual finding is a true blocker.

Why it cannot proceed

hypaware-core/plugins-workspace/claude/skills/hypaware-query/SKILL.md:69 and the
byte-identical codex/ copy open the new bullet with:

content_text never carries a raw base64 payload: an inline
data:<mediatype>;base64,<payload> is stripped and replaced with a marker.

That "never" is false, and false for inline data: URIs specifically, which is
the scope the clause sets. Verified twice independently, by extracting the shipped
BASE64_DATA_URI and stripBase64DataUris out of message_projector.js at
runtime rather than re-typing them:

shape	result
ordinary mediatype (control)	stripped, 82 chars to 32
256-char mediatype	raw base64 survives whole, 329 to 329
300-char mediatype	raw base64 survives whole, 373 to 373
whitespace in the mediatype	raw base64 survives whole, 82 to 82
line-wrapped payload	tail survives as bare base64
\n-escaped payload	tail survives as bare base64

The same commit contradicts itself: message_projector.js:1188-1192 says
content_text is "NOT guaranteed bounded. A line-wrapped or \n-escaped
payload still has its tail survive as bare base64 text ... a deliberate open
question (#718)." Source says NOT guaranteed; the skill doc says never.

git diff c483c1a..a3ee8c7 shows the entire bullet, including that clause, is
new in this PR. It is not pre-existing text the PR sat beside.

Why triage called it a blocker rather than a doc nit. These SKILL.md files are
not prose a human skims: they are shipped to Claude and Codex as the instructions
an agent reads before writing SQL against captured data, so the text is the
mechanism that programs behaviour at query time. An agent told content_text is
bounded will select it without a length guard and will rule out oversized rows when
diagnosing a failure. The originating incident behind this whole line of work was a
single 12.67MB content_text value that made a search index unbuildable, which is
exactly the row class this bullet says cannot exist. RFC 2045 wraps base64 at 76
columns, so the line-wrapped shape is the mainstream form of embedded base64 in
text, not an adversarial corner. And #718 is open on precisely this cap question,
so the sentence also forecloses a live open question by assertion.

What it needs from you

Either apply this one-clause, two-file edit (keeping both copies
byte-identical), replacing the bullet's first sentence:

- `content_text` normally carries no raw base64 payload: a matched inline `data:<mediatype>;base64,<payload>` is stripped and replaced with a marker. The match is bounded, so the column is not guaranteed small: a mediatype over 255 characters, a mediatype containing whitespace, or a line-wrapped payload leaves raw base64 in the column (#718).

or tell neutral to merge with the universal as written, if you judge the
simpler sentence worth the inaccuracy.

The rest of the PR is correct and should stand unchanged: the message_projector.js
comment (round 2 graded it clause by clause against observed output and it is
accurate, including "admits every other control character", which is literally
exact), the two-generations guidance, the ;base64,<stripped> recipe, and the
forgeability caveat appended to the same bullet.

Everything else in the residual set is resolved or cosmetic. Round 2's findings 2
and 3 (a code-span line-wrap that made an ESC example render with a space, and two
notes claiming "not closing this issue" on an issue closed 14 hours earlier) were
corrected on #722. Finding 4 is a mid-string comment wrap whose claim holds under
all three readings.

How to unstick

Reply with a comment on this PR, or push the edit to fix/issue-736. Neutral
monitors this thread and will re-engage with your guidance on its next tick.