fix(llm): defang any chat-template control token, not an enumerated few (#3183) - #3198
fix(llm): defang any chat-template control token, not an enumerated few (#3183)#3198abhay-codes07 wants to merge 1 commit into
Conversation
…d few (Graphify-Labs#3183) _neutralise_injection_sentinels listed six specific <|...|> markers, so <|start_header_id|>/<|end_header_id|>/<|eot_id|> (Llama 3), <|endofprompt|>, and whatever the next template calls its turns passed through to the model intact, and [SYSTEM]/[/SYSTEM] were not covered at all. A hostile file in an audited repository could use the unlisted forms to forge a turn boundary inside the untrusted_source block. The <|...|> FORM itself is the hazard - no legitimate source construct needs an intact one - so the pattern now matches it generically (<\|[A-Za-z0-9_.\-]{1,64}\|>), and [SYSTEM] joins [INST]. Defanging still only inserts a zero-width space, so the text stays human-readable in the graph and nothing is deleted; ordinary uses of | and brackets (bitset<|N|+1>, a || b, arr[SYS]) are untouched.
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Graphify review — findings
Broadens the injection-sentinel matcher to defang any <|token|> chat-template marker generically (up to 64 chars) rather than the six previously enumerated, so Llama 3's <|start_header_id|>/<|eot_id|>, <|endofprompt|>, and future turn markers no longer pass through intact, and extends [/?INST] to also cover [/?SYSTEM]. Neutralisation still only inserts a zero-width space, leaving text human-readable and deleting nothing. Adds tests asserting control tokens never survive wrapping while ordinary code and prose (a | b, std::bitset<|N|+1>, arr[SYS]) stay untouched.
Worth a look
- Generic <|...|> pattern may match legitimate '<|x|>' constructs in ordinary text —
graphify/llm.py:566· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 787 functions depend on the 178 functions this change touches.
Health — this change adds coupling hotspots:
- new:
deduplicate_entities()— 63 callers, 21 callees - new:
build_merge()— 53 callers, 13 callees - new:
extract_files_direct()— 17 callers, 20 callees - new:
extract_corpus_parallel()— 26 callers, 11 callees - new:
_call_claude_cli()— 31 callers, 9 callees - new:
dispatch_command()— 2 callers, 122 callees - new:
_extract_with_adaptive_retry()— 22 callers, 10 callees - new:
_call_llm()— 11 callers, 18 callees - …and 16 more — each is listed as a finding
Verification — 787 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 491 function(s) in the blast radius were not formally verified this run
· 24 more finding(s) on lines outside this diff (see the check run).
Closes #3183.
The problem, and a correction to the report
First the correction: the issue's quoted code is not what ships.
_neutralise_injection_sentinelslives ingraphify/llm.py(notextract.py:142), is regex-based (not twostr.replacecalls), and already covered<|im_end|>and<|endoftext|>among six enumerated<|…|>markers, plus<<SYS>>/[INST]and heading-style "### system:" lines.The residual gap is real, though: the
<|…|>coverage was an enumerated list, so<|start_header_id|>/<|end_header_id|>/<|eot_id|>(Llama 3),<|endofprompt|>, and whatever the next template calls its turn markers passed through to the model intact — and[SYSTEM]/[/SYSTEM]were not covered at all. A hostile file in an audited repository could use the unlisted forms to forge a turn boundary inside the<untrusted_source>block.The change
Match the form, not a list: any
<|token|>(<\|[A-Za-z0-9_.\-]{1,64}\|>) is defanged — no legitimate source construct needs an intact one — and[SYSTEM]joins[INST]. Everything else about the mechanism is unchanged: defanging inserts a zero-width space after the first character, so nothing is deleted, byte content stays reviewable against the block's sha256 stamp, and the text remains human-readable in the graph. Ordinary uses of the characters (a || b,bitset<|N|+1>,arr[SYS],(|x|, |y|)) are untouched — the token charset and length bound keep them out.Tests
tests/test_injection_sentinel_coverage.py— 28 tests: every previously-enumerated marker plus the Llama-3 headers,<|endofprompt|>, FIM/return markers and[SYSTEM]variants never survive intact (and nothing is deleted); case variants; six ordinary-code/prose strings pass through byte-identical; a full Llama-3 turn forgery wrapped by_wrap_untrustedreaches the prompt with no intact control token and exactly one real closing tag; and defanged text stays readable. With the fix reverted, 10 fail (the enumerated set and the benign cases rightly keep passing). The LLM suites are unchanged (178 passed together); the full suite matches the freshv8(0.9.52) baseline.