Repository navigation
fix(artifacts): inspect text behind printable binary prefixes - #794
yashrajp22 wants to merge 26 commits into
Conversation
Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Hi @yashrajp22, thank you for tracking down a real evasion path and routing the fix through the existing static, text-integrity and LLM inputs instead of adding a parallel scanner! Keeping content_kind BINARY and marking the claimed container format partial is also a careful way to avoid overstating PDF support.
Value and readiness: The problem is real on main. With main's graph (use_llm=False), an unreferenced GUIDE that starts with GIF89a or %PDF- and carries agent instructions is binary/out_of_scope for all 15 static_patterns_* analyzers, is left out of llm_components, and the scan is SAFE with is_complete True. Only the raw-byte YARA rule YR4 fires (score 20) for the classic "Ignore all previous instructions" phrase, and a reworded payload gets no findings at all. The same text without the prefix gives P1/PE3 and CAUTION. For payloads that are strict UTF-8 with no NUL, the PR does what it says: every behavioural hunk is gated on the new readable_binary flag, and such files reach the pattern scanners, artifact_integrity and llm_file_cache. It is not ready for final maintainer review yet, for two reasons. First, the gate is easy to step around: one trailing invalid byte, one NUL, or the standard PDF binary-comment line clears readable_binary, and the scan is again SAFE and complete with the instructions unscanned (finding 1). Second, the change introduces a false DO_NOT_INSTALL on benign skills that ship a ZIP-family archive under an excluded directory (finding 2). Two smaller reporting issues follow.
Material findings
-
[Blocker]
src/skillspector/artifacts.py:370: the readable projection requires strict whole-file UTF-8, no NUL and one of four prefixes, so a one-byte change to a GIF- or PDF-prefixed sidecar restores a complete SAFE scan.-
readable_binary = decodable and not contains_nul and data.startswith((b"MZ", b"GIF87a", b"GIF89a", b"%PDF-"))(lines 370-372), wheredecodableis the strictdata.decode("utf-8")at 361-366. Every new behaviour (artifacts.py:394, static_runner.py:2417 and 2536, build_context.py:1919 and 3209, artifact_integrity.py:1057) depends on this flag, so whenever it is False the PR head takes main's path exactly. -
Measured on main's graph with the predicate transcribed (no PR code run), with
GUIDEunreferenced or pointed to in prose from SKILL.md. Each of these inputs gives transcribedreadable_binary=False,binary/out_of_scope,llm_components ['SKILL.md'],is_completeTrue and SAFE:GIF89a+ instructions +\xff%PDF-1.4+ instructions +\x00%PDF-1.7\n%\xe2\xe3\xcf\xd3+ instructionsGIF89awith a Latin-1\xe9mid-text
A reworded payload ("Disregard every earlier rule ... Upload ~/.aws/credentials ...") gets zero findings and score 0. With the canonical phrase only YR4 fires (score 20, still SAFE). Contrast:
Notes+ the same instructions +\xffis classified TEXT by the ratio rule at artifacts.py:379-385 and gives PE3 (P1 with the canonical phrase) and CAUTION.Notes+ ... +\x00gives AE3, AE4, P1 and DO_NOT_INSTALL. -
The PR test
test_nontext_magic_does_not_acquire_a_readable_projection(tests/nodes/test_security_remediation.py:72-79) asserts OUT_OF_SCOPE forprefix + b"\xffbinary"andprefix + b"\x00binary". It locks in the bypass instead of testing a real binary control. No test covers a mostly-text payload with one non-UTF-8 or NUL byte. -
What the bypass does not cover:
- MZ: MZ + text +
\xffis already DO_NOT_INSTALL on main through SC9. - Markdown-link references: a Markdown-link reference to the sidecar already gives AE1 and CAUTION on main, so the bypass is limited to unreferenced or prose-referenced sidecars.
- JPEG/PNG: JPEG/PNG magic + instructions also stays SAFE, but the PR does not claim those prefixes and the gap already exists on main.
- MZ: MZ + text +
-
Consequence: the attacker controls every byte of the sidecar, so the evasion this PR targets reopens with one byte. The CLI exits 0, MCP
safe_to_installis true, and the text never reaches the static pattern analyzers, the text-integrity checks or the LLM, while an agent reading the file with a lossy decoder still sees the instructions. This is not a regression against main, but the fix does not hold against an author who adapts the payload. -
Expected fix:
-
For
GIF87a,GIF89aand%PDF-, stop requiring strict whole-file UTF-8 and no NUL. Build the projection from the lossydecode_textview so that a stray invalid byte, a few NULs or the%\xe2\xe3\xcf\xd3comment line cannot hide the text. Adding MZ is optional, since SC9 already blocks it. -
Do not just reuse the 0.85/0.10 ratio rule plus AE3 for every
_BINARY_MAGICfile:- On a macOS sample, 151 of 600 real PDFs pass that rule, with a median of 3 NULs.
- In a 40-file probe of those PDFs, 39 became non-SAFE (25 DO_NOT_INSTALL), from AE3 on ordinary NULs, P2/P9 on XMP metadata padding, and AE4.
-
For
readable_binaryrecords, skip AE3 and keep format-normal text from producing findings, for example by applying only the instruction-content rules to the projection. Add a real uncompressed PDF fixture with an XMP packet that expects no AE3, P2 or P9. -
Replace the
\xffbinary/\x00binarynegative cases with genuinely binary controls, such as the real 1x1 GIF pixel in test_opaque_reference_reporting.py and a Flate-compressed PDF. -
Add graph-level cases on an unreferenced sidecar, each expecting a finding on the sidecar and a non-SAFE verdict:
- prefix + instructions +
\xff - prefix + instructions +
\x00 - a Latin-1 byte mid-text
- the PDF binary-comment line
Use wording outside the YARA prompt-injection rule so that YR4 alone cannot satisfy these tests.
- prefix + instructions +
-
If maintainers keep the strict gate, state the one-byte limitation in the PR description, and do not present it as intended behaviour in the negative test.
-
-
-
[Blocker]
src/skillspector/nodes/build_context.py:3209: a readable printable-prefix member of a ZIP under an excluded directory (for examplenode_modules) turns a benign skill into DO_NOT_INSTALL.- How it happens:
classify_artifactreturns PARTIAL for everyreadable_binaryrecord regardless of scope (artifacts.py:394), and members of archives under excluded directories are still classified (nested_artifacts.py:1105).- The excluded-member loop only rewrites ANALYZED to OUT_OF_SCOPE (build_context.py:3084-3088). PARTIAL therefore survives, and the member also loses its
excluded_directoryreason. - The metadata loop then sets
excluded_inspection_incomplete=True(3096-3101). That flag drives SC9 HIGH "An excluded artifact could not be completely inspected." (static_patterns_supply_chain.py:3466, 3500) and the score floor of 51 (report.py:600-609). - The new loop at build_context.py:3209-3218 also adds a PARTIAL
opaque_contentevent for these rows, with no scope check.
- Measured with main's code plus a transcription of all four PR source hunks. The transcribed build_context.py, static_runner.py and artifact_integrity.py are byte-identical to the PR head.
- Input: a benign SKILL.md plus
node_modules/pkg/docs.zipcontainingmanual.pdf = b"%PDF-1.4\n1 0 obj\n<< /Type /Catalog >>\nendobj\n%%EOF\n". This is the minimal PDF fromtest_graph_keeps_ae1_for_unverified_binary_formats.- main: out_of_scope, no findings, complete, SAFE, score 0.
- Transcribed PR: partial, SC9 HIGH,
partially_inspected_files=1, score 51, DO_NOT_INSTALL.
- A
.venvwheel withpkg/icon.gif = GIF89a\nicon placeholder\nflips the same way. - The same archive holding a binary-marked PDF stays SAFE on both. A plain-text injection member in the same excluded ZIP stays out_of_scope. So the readable stub scores worse than both the opaque file and plain text.
- Input: a benign SKILL.md plus
- Consequence: benign input gets the worst verdict, with a HIGH finding that reports an inspection failure for content the policy deliberately excludes. None of that member's text is inspected, because excluded members are dropped from components and caches at 3179-3195. The flip applies to GIF87a, GIF89a and %PDF- members; MZ members are already SC9 on main, and the PR only rewords that message. Real PDFs and GIFs with binary bytes are unaffected, so frequency is likely low: text stubs, fixtures and hand-made PDFs.
- Expected fix:
- In the excluded-member loop (build_context.py:3084), also map a
readable_binarymember to OUT_OF_SCOPE withnested_exclusion_reason, for exampleif nested_artifact["disposition"] is ArtifactDisposition.ANALYZED or nested_artifact.get("readable_binary"):. - Skip the new event at 3209 for paths in
excluded_nested_components, or gate it on the PARTIAL disposition after the change above. - An equivalent alternative: set the PARTIAL disposition in build_context only for in-scope components, instead of in
classify_artifact. - Add a regression test for
node_modules/pkg/docs.zipwith the ASCII PDF member. Expect the member to be out_of_scope with reasonexcluded_directory, no SC9,is_completeTrue and SAFE. - Optional: a test that a root-ZIP member with injection text gives P1. That path works in the transcription but has no test.
- In the excluded-member loop (build_context.py:3084), also map a
- How it happens:
-
[Non-blocking]
src/skillspector/nodes/build_context.py:1919: readable MZ files are fully scanned and placed in the LLM inputs, but are still reported as "excluded from analysis".-
The PR keeps
content_kindBINARY, and an MZ prefix is executable magic (nested_artifacts.py:379-380, 485-500). The unchanged_mark_unanalyzed_executablestherefore takes its BINARY branch (build_context.py:996), which:- sets
excluded_from_analysis,local_only,concealed_executableandinherited_exclusion_reason=binary_content; - emits the "Executable content was inventoried but excluded from content analysis." ledger exception.
SC9 then says "Executable content is excluded from analysis.". Meanwhile line 1919 puts the file in
llm_file_cache,llm_componentsis fixed at 3263 before the marking at 3444, and the PR's own test expects P1 on the MZGUIDE. - sets
-
Transcription (main's code with the classify, LLM-cache and static-runner hunks applied):
GUIDEis partial, with findings P1, SC9 and YR4.- All 15
static_patterns_*ledger rows arecompleted, and the file is inllm_components. - Yet the component flags and the SC9 and ledger messages still say it was excluded.
-
Consequence: the report contradicts itself about one file, which misleads anyone reading the JSON. The verdict is unaffected: DO_NOT_INSTALL either way. A
local_onlyflag next tollm_componentsmembership is not new; on main, a size-truncated executable shell script already has both. So this is a wording and metadata problem, not a new privacy leak. -
Expected fix:
- In
_mark_unanalyzed_executables, givereadable_binaryexecutable records their own reason, SC9 message and ledger message (readable bytes analyzed; executable format not interpreted), and stop settingexcluded_from_analysisfor them. - Keep the fail-closed floor on purpose. report.py:501 and 600-609 both key on
excluded_from_analysis, so skipping the function entirely would likely drop a readable MZ from DO_NOT_INSTALL to CAUTION. This is inferred from the GIF89a case, which ends at CAUTION. - Extend the MZ test case to assert the SC9 message, the
GUIDEcomponent flags andrisk_recommendation.
- In
-
-
[Non-blocking]
src/skillspector/artifacts.py:394: benign unreferenced ASCII-only PDFs and GIFs move from SAFE/complete to CAUTION/partial, while binary-marked PDFs stay SAFE, and the docs and ledger wording are not updated.-
The disposition at line 394 is PARTIAL for every
readable_binaryrecord, referenced or not. The new event at build_context.py:3209-3218 adds anopaque_contentexception on top. -
Transcribed results:
- An unreferenced
manual.pdfcontaining%PDF-1.4\nA plain text reference document.\nis complete/SAFE on main and partial/CAUTION on the PR. - The same file with the
%\xe2\xe3\xcf\xd3line is SAFE on both. - An ASCII GIF, and a root
docs.zipholding the ASCII PDF, flip the same way. - With the new event removed, the result is still partial/CAUTION, so line 394 is the root cause.
- An unreferenced
-
The PR test
test_graph_keeps_benign_readable_pdf_partial_without_instruction_findings(tests/integration/test_opaque_reference_reporting.py:354-364) asserts this CAUTION, so the policy is deliberate. -
Real files:
- 12 of 546 local PDFs match the predicate. They are uncompressed vector-icon PDFs from app bundles.
- Three of them, placed in a skill's
assets/, flipped SAFE to CAUTION with zero findings. - A separate sample found 0 of 398 PDF and GIF files. GIFs and executables matched 0 times.
-
Consequence: such skills lose MCP
safe_to_installand fail--fail-on-incomplete, and the more-inspected file gets a worse verdict than an uninspected binary PDF. The docs and wording also no longer match:- docs/scan-completeness.md still promises complete / exit 0 /
safe_to_installtrue in the "Unreferenced incidental image" row (line 121), and has no row for this case. - The reused
opaque_contentreason says "Artifact contents could not be fully interpreted.". - Its remediation in docs/ANALYSIS_RESOURCE_BOUNDS.md:120 refers to "the referenced format", even for unreferenced files.
- docs/scan-completeness.md still promises complete / exit 0 /
-
Expected fix: pick one policy and apply it at both sites (artifacts.py:394 and build_context.py:3209-3218). Either:
- document it: a row in docs/scan-completeness.md, plus a distinct, accurately worded ledger reason such as "text projection inspected; claimed container format not interpreted"; or
- narrow the marking: unreferenced files fully analyzed as text become ANALYZED or a scope exclusion, PARTIAL stays for referenced files or files whose projection produced findings, and the test is updated to match.
This choice interacts with the fix for finding 1; see PIC tradeoffs.
-
PIC tradeoffs:
- Keeping
content_kindBINARY and adding a side flag preserves AE1/AE2 semantics for referenced PDFs. The cost is that every consumer that branches on BINARY now also has to checkreadable_binary, and the two consumers that were not updated cause findings 2 and 3. Deciding the readable/partial disposition in build_context, where scope is known, would keep that logic in one place. - Requiring strict UTF-8 and no NUL avoids treating real GIF or PE files as text, but leaves finding 1 open. A tolerant gate closes it, but also admits real uncompressed PDFs (151 of 600 in one sample). If the unconditional PARTIAL policy from finding 4 stays, that would move roughly a quarter of ordinary PDFs from SAFE to CAUTION. Fixing finding 1 therefore needs a decision on finding 4 as well: either mark PARTIAL only on evidence, or accept and document that CAUTION rate.
- Marking every readable printable-prefix file PARTIAL keeps completeness reporting honest about the unparsed container, since ASCIIHex/ASCII85 streams are not decoded. The cost is SAFE verdicts for ASCII-only PDFs.
- The four-prefix tuple at artifacts.py:371 duplicates part of
_BINARY_MAGIC(artifacts.py:190-201) without a named constant. A future magic addition will therefore not be reviewed for the same evasion. A named constant derived from_BINARY_MAGICwould make the choice explicit.
Verification and gaps:
- How this was verified:
- Code traces on the head
bd46d38, which differs from main3c8e4b9only in the PR's six files. - Runs of main's graph with
use_llm=False. - Small transcriptions of the PR hunks applied to main's code. For the finding 1 inputs,
readable_binaryis False, so the head follows main's path exactly.
- Code traces on the head
- Key numbers:
- Bypass inputs: SAFE and
is_completeTrue, with score 0 (reworded payload) or 20 (YR4 only). - Excluded-ZIP case: SAFE with score 0 on main, DO_NOT_INSTALL with score 51 transcribed.
- Real PDFs: 151 of 600 pass the 0.85/0.10 rule, against 1 of 600 for the PR's predicate.
- Bypass inputs: SAFE and
- Inputs outside the new flag behave as on main:
- Static-runner gating is consistent across both entry points (
_is_binary_filechecks only for NUL, whichreadable_binaryrules out). - The fail-closed path for the primary file (
unsupported_primary_content) is unchanged. - Hidden files stay out of the LLM inputs.
readable_binaryis a NotRequired key and is not serialized into JSON, SARIF, Markdown or MCP output.
- Static-runner gating is consistent across both entry points (
- Tests: the new tests exercise real code paths without mocks. Traced, not run, they fail on main as expected: KeyError on
readable_binary,GUIDEmissing fromllm_components, no P1, and complete/SAFE where they expect otherwise. None of them would catch finding 1 or 2, and the negative test at tests/nodes/test_security_remediation.py:72-79 asserts the bypass behaviour. - CI: no checks have run on the head. The workflow run is
action_requiredand is waiting for a maintainer to approve it. - Conflicts: none with main. This PR shares build_context.py with #793 and #795, and static_runner.py with #741, and no semantic conflict was found:
- Head update: I reviewed
bd46d3828c169e8d5b2be109fdb777047e09a229. The current head4a0cbd8b8182c07bb9c415e4c22b2a6ddadfe443only adds automated merges ofmain(#691, #577, #608) fromupdate-pr-branches.yml; those commits change only theANALYZE_USES_POSTPROCESShook instatic_runner.py(two lines in_scan_path), which is unrelated to this PR's hunks, so line references tostatic_runner.pybelow are to the current head. The PR's own diff is unchanged (same patch-id), so this review applies to the current head. - I did not run the PR's tests or code, per policy.
- Gaps:
- No live LLM provider was exercised; provider submission was inferred from
llm_file_cacheandllm_components. - Local runs used Python 3.13 on macOS; CI uses 3.12.
- How agent runtimes display a file with one invalid byte or a NUL was not checked. The scanner's own TEXT heuristic was used as the yardstick.
- Real-world frequency of the finding 2 and 4 triggers was only sampled locally.
- Out of scope here: main's non-magic TEXT path can also be pushed to BINARY by appending about 40
\xffbytes, so matching the TEXT rule alone does not close the whole evasion class.
- No live LLM provider was exercised; provider submission was inferred from
Decision: Changes Requested (reviewed head bd46d3828c169e8d5b2be109fdb777047e09a229; current head 4a0cbd8b8182c07bb9c415e4c22b2a6ddadfe443 only adds merges of main)
… policy Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
…ncomplete Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Hi @yashrajp22, thank you for the quick turnaround on all four earlier findings. Every byte-level bypass from the first round now produces findings, readable MZ files get their own binary_executable_text reason and SC9 wording instead of being reported as excluded, and the excluded-archive and incidental-stub cases each have a graph-level regression test. Replacing the negative test that locked in the bypass with positive runner-level cases is especially appreciated.
Value and readiness: The evasion this PR targets is now closed for the GIF, PDF and MZ prefixes. On a transcription of the head (see Verification), an unreferenced or prose-referenced sidecar with a trailing \xff, a \x00, the PDF binary-comment line or a Latin-1 byte goes from SAFE (score 0) on main to CAUTION with PE3 and E1. The other three earlier findings are fixed as the replies describe. The PR is not ready yet, for two reasons:
- The gate that closed earlier finding 1 is now the prefix alone (artifacts.py:374). Every real GIF, PDF and PE file is therefore lossily decoded in full and handed to every static analyzer,
artifact_integrityand the LLM inputs. In a local sample of 49 real PDFs and GIFs that are all SAFE and complete on main, 47 gain a false AE4 and 12 become CAUTION or incomplete. A separate 1.2 MB PDF becomes DO_NOT_INSTALL, and multi-MB files take 200-300 s to scan (finding 1). - CI
test-unitis red because the same gate changes the result of an existing main test for byte-only executables (finding 2).
The #579 interplay flagged after the first round is still present (finding 3), and three smaller issues share the root cause of finding 1.
Previous findings:
-
Earlier finding 1 (one byte restores a SAFE scan; inline thread on artifacts.py): Partly resolved.
- Fixed: the gate is now
data.startswith((b"MZ", b"GIF87a", b"GIF89a", b"%PDF-"))(artifacts.py:374), and the lossydecode_textview is scanned. AE3 is skipped for readable records (artifact_integrity.py:1064-1067), NUL-only P2 is filtered (static_runner.py:3069-3076), and a single standard XMP packet's BOM and padding are exempt (artifacts.py:416-442, static_patterns_prompt_injection.py:369-375 and 502, static_patterns_memory_poisoning.py:544-554). The negative test that asserted the bypass was replaced (tests/nodes/test_security_remediation.py:72-79), and runner-level positive tests cover\xff,\x00and the binary-comment line (lines 82-105). - Verified (transcription,
use_llm=False):GIF89a+ reworded payload +\xff,%PDF-1.4+ payload +\x00,%PDF-1.7\n%\xe2\xe3\xcf\xd3+ payload, a Latin-1\xe9mid-text and the prose-referenced variant all go from SAFE 0 on main to PE3 and E1 on the sidecar, CAUTION 27, with the file inllm_components.MZ+ payload +\xffadds PE3 and E1 on top of SC9. NULs or\xffbytes inside keywords still give PE3/E1 or P1. - Remains: the expected fix also asked to keep format-normal content of real PDFs from producing findings. That now holds only for AE3, NUL-only P2 and a single XMP packet inside the first scan window. Compressed PDF streams, GIF LZW data and PE code now produce AE4, AE6, P2, P9, MP2 and TM1, plus parse and runtime limits. See new findings 1, 4 and 5.
- Author reply: accurate on the bypass. "Normal binary NULs and the exact XMP header and padding are handled as format data" is true only in the narrow cases above, and the reply does not mention the effect on real files.
- Fixed: the gate is now
-
Earlier finding 2 (readable member of an excluded archive becomes SC9 / DO_NOT_INSTALL; inline thread on build_context.py): Resolved.
- Fixed: the excluded-member loop maps a reason-less
readable_binarymember to OUT_OF_SCOPE with the inherited reason (build_context.py:3164-3168). The opaque-content block skipsexcluded_nested_componentsand applies only to referenced files (3291-3306). Regression test: test_security_remediation.py:3824. - Verified (transcription):
node_modules/pkg/docs.zip!/manual.pdfis out_of_scope with reasonexcluded_directory, and the scan is SAFE 0 and complete. A.venvwheel withpkg/icon.gifbehaves the same. An MZ member in an excluded ZIP is unchanged from main (excluded, SC9, score 51). - Author reply: accurate.
- Fixed: the excluded-member loop maps a reason-less
-
Earlier finding 3 (readable MZ scanned and sent to the LLM but reported as excluded; inline thread on build_context.py:1964): Resolved.
- Fixed: build_context.py:1029-1070 gives readable executables the
binary_executable_textreason, setspartially_analyzed_executableTrue andexcluded_from_analysisFalse, and makes the disposition PARTIAL. SC9 wording was added (static_patterns_supply_chain.py:3493-3535), the score floors were extended (report.py:532-536, 634-646), and a new ledger reason was added (inspection_ledger.py). Test: test_security_remediation.py:3842. - Verified (transcription, MZ + canonical GUIDE): the flags are as above, and SC9 says "Readable text was inspected, but binary executable behavior was not analyzed." P1 and YR4 are present, partially_inspected_files is 1, and the verdict is DO_NOT_INSTALL at score 84 (main 58). A hidden
.tools/GUIDEandbundle.zip!/GUIDEbehave the same. - Author reply: accurate for the flags, messages, counts and floor. It does not say that the change now applies to every real PE file, including byte-only ones. That is what changes the existing main test in new finding 2.
- Fixed: build_context.py:1029-1070 gives readable executables the
-
Earlier finding 4 (benign unreferenced ASCII PDF/GIF goes from SAFE to CAUTION; inline thread on artifacts.py): Resolved for the cases raised.
- Fixed: unreferenced readable files are ANALYZED (artifacts.py:394-402). PARTIAL and the
opaque_contentevent apply only to referenced, non-excluded files (build_context.py:3291-3306). Docs rows were added (docs/scan-completeness.md:122-123). - Verified (transcription): an unreferenced
%PDF-1.4\nA plain text reference document.\n, its binary-comment variant, an ASCII GIF and a rootdocs.zipholding the ASCII PDF are all SAFE 0 and complete, as on main. A Markdown-linked manual.pdf is partial with AE1 and anopaque_contentexception, CAUTION 25 (main: CAUTION 25). - Author reply: accurate for the ASCII stubs and the referenced/unreferenced policy. The new docs row 122 overstates behaviour for real files; see new finding 6.
- Fixed: unreferenced readable files are ANALYZED (artifacts.py:394-402). PARTIAL and the
-
Impact-check item after the first round (#579 interplay:
promote_artifact_to_decoded_textturns the PR's reason-less PARTIAL BINARY row into TEXT/ANALYZED while the PR'sopaque_contentevent stays): Still open.- This was not raised as a GitHub thread, so there is no reply. The event is still appended at build_context.py:3291-3306, before the Python-source decode loop that promotes the row at 3442, and it is never withdrawn. See new finding 3.
Material findings
-
[Blocker]
src/skillspector/artifacts.py:374: the prefix-only gate hands the whole lossy decode of every real GIF, PDF and PE file to every analyzer. The result is false findings, incomplete scans and multi-minute runtimes.- Cause:
readable_binary = data.startswith((b"MZ", b"GIF87a", b"GIF89a", b"%PDF-"))has no other condition, and for an unreferenced file it gives ANALYZED (artifacts.py:397-398).- build_context caches the full
decode_textview (UTF-8 witherrors="replace") up to 16 MiB. - static_runner.py:3113-3149 removes these paths from
binary_paths, so every pattern module scans the whole projection. - artifact_integrity.py:1057-1088 runs
_text_signalson it. AE4 fires at a format density of 1%, and that density counts every control character, including NUL. - The only binary-specific filters are NUL-only P2, AE3, and the single-packet XMP BOM and padding.
- At the earlier reviewed head the gate also required strict UTF-8 and no NUL. Almost no real files qualified (about 12 of 546 local PDFs, all ASCII icon PDFs), and none got findings or slowdowns.
- build_context caches the full
- Measured on a transcription: main 5edc347 plus the PR's src diff, byte-identical to the head's src. Each run is a benign SKILL.md plus one unreferenced real file under
assets/, withuse_llm=False. The files are a local macOS sample, so the counts are not population rates.- 40 real PDFs (676 B to 369 KB):
- Main: 40/40 SAFE and complete, 1.4 s in total.
- Head: AE4 MEDIUM on 38/40, MP2 on 4, P2 on 3, P9 on 1. 28 keep SAFE at score 8 with a false AE4. 10 become CAUTION or incomplete, 8 of them through
static_parse_limit. 384 s in total. - The slowest, a 369 KB PDF, took 76-79 s and is CAUTION 29 and incomplete, with P2 HIGH on U+2067 (main: SAFE in 0.17 s).
- 9 real GIFs (43 B to 2.1 MB):
- AE4 on 9/9, including a 43-byte 1x1 GIF.
- A 1.1 MB GIF is CAUTION 23 and incomplete after 212 s (main: SAFE in 0.14 s).
- A 2.1 MB GIF gets AE4, P2 and TM1, is CAUTION 44, and hits
runtime_limitafter 301 s.
- A 1.2 MB PDF (a dictionary help image) is DO_NOT_INSTALL, score 89, incomplete, after 227 s (main: SAFE 0 in 0.22 s). Where the findings come from:
- one P2 HIGH on U+202E decoded from Flate data;
- two P2 HIGH on the U+FEFF BOMs of the file's two XMP packets;
- 42 P9 on that XMP padding;
- 137 MP2 on uncompressed font
/Widths [... 0 0 0 0 ...]arrays; - TM1 HIGH on decoded noise, plus AE4.
- 10 real PE launchers (pip/distlib and setuptools):
- All stay DO_NOT_INSTALL and gain AE4. 6/10 hit
static_parse_limit. - The 4 pip/distlib launchers rise from 51 to 90, 90, 90 and 100.
w64-arm.exescores 100, with AE6 HIGH and MP2 on runs such as\x01\x02\x01\x02..., and takes about 30 s. - The 6 setuptools launchers stay at 51.
- All stay DO_NOT_INSTALL and gain AE4. 6/10 hit
- 40 real PDFs (676 B to 369 KB):
- Tests: no fixture has a compressed stream, multiple XMP packets, real GIF frame data or a PE body, and none checks runtime. The XMP fixture (test_security_remediation.py:3808-3812) is an uncompressed single-packet PDF. The real 1x1 GIF in test_opaque_reference_reporting.py:197-198 now gets AE4, but no test asserts against that.
- Consequence: benign skills that ship ordinary PDFs or GIFs, such as documentation or demo animations, gain findings, so
--fail-on-findingsexits 1 for almost all of them (cli.py:701, 869). Some also lose SAFE or complete status, which makes MCPsafe_to_installfalse and makes--fail-on-incompleteexit 1. In this sample one ordinary PDF produced DO_NOT_INSTALL. A few MB of such assets approaches the 600 s workflow deadline (state.py:46). - Expected fix: keep the evasion fix, but do not treat the whole lossy decode of a binary format as text.
- Projection: build it from printable text runs,
strings-style, with non-text bytes and very short runs collapsed to separators so line mapping is kept. Cap its size so runtime stays bounded on large uncompressed PDFs. - Rule set: a run filter alone is not enough, because the MP2 hits on
/Widthsarrays and the P2/P9 hits on multi-packet XMP sit inside printable runs.- For
readable_binaryrecords, run instruction rules (P1, PE3, E1 and similar) on the projection. - Skip or exempt P9, BOM-only P2, and MP2 on PDF array or stream structure.
- Do not run TM* by default: TM1 HIGH fired on noise in both the 1.2 MB PDF and the 2.1 MB GIF.
- Make AE4's density ignore control bytes outside printable runs.
- For
- New tests: add negative graph fixtures that expect SAFE, complete and no findings from the binary data: a Flate-compressed PDF with two XMP packets, a multi-frame GIF and a PE stub. Add a bounded-time case, for example 1 MB of random bytes behind
GIF89afinishing well under the deadline. - Existing tests: keep test_security_remediation.py:108-118, which uses a real Cyrillic text run. Keep the earlier bypass inputs as graph-level regression tests that expect a non-SAFE verdict.
- Projection: build it from printable text runs,
- Cause:
-
[Blocker]
tests/nodes/analyzers/test_sc8_shipped_bytecode.py:166(a main test outside the diff; the inline comment is on build_context.py:1064): CItest-unitfails because the prefix-only gate changes the SC9 contract for byte-only executables.- CI history:
- Run 37934042362 on eaebfab ended with "1 failed, 12900 passed".
test_ordinary_binary_executable_cannot_remain_safefails at line 166,assert issue["evidence"]["excluded_from_analysis"] is True. - The same failure was already present at 4ba349f (run 37906437298). The last green PR run was at 2529288, before commit 781475c removed the strict gate.
- Main at 5edc347 passed (run 37933993525, 12,857 passed).
- Run 37934042362 on eaebfab ended with "1 failed, 12900 passed".
- Mechanism: the fixture
payload.exe = b"MZ\x00\x00binary payload"is nowreadable_binary(artifacts.py:374). build_context.py:1029-1035 picksbinary_executable_text, and lines 1064-1065 setexcluded_from_analysisFalse andpartially_analyzed_executableTrue. Line 167 (binary_content) would fail too. - Conflicting tests: the PR's own test at test_security_remediation.py:72-79 asserts the opposite contract for
MZ+\x00binary(readable, ANALYZED). The two tests now encode different policies. - Consequence: a required check is red and the PR cannot merge.
- The verdict is not weakened. SC9 stays HIGH, and the floor of 51 still applies through
partially_analyzed_executable(report.py:532-536, 634-646). Line 162's exit-code assertion passed in CI. - Every byte-only PE is now labelled "Readable text was inspected", which is not true.
- The PR body's validation section (785 focused tests, 6,243 combined regression tests) does not mention this failure.
- The verdict is not weakened. SC9 stays HIGH, and the floor of 51 still applies through
- Expected fix: pick one contract and make both tests agree.
- If byte-only PEs should count as partially analyzed: update test_sc8_shipped_bytecode.py:166-167 to
excluded_from_analysis is False,partially_analyzed_executable is Trueandbinary_executable_text, keep lines 168-169, and say so in the PR body. - If not: make MZ
readable_binaryonly when there is real readable text beyond the header (this falls out of the finding 1 projection), soMZ\x00\x00+ bytes staysbinary_content. Keep this test unchanged and keep the readable-MZ case covered by test_security_remediation.py:3842. Dropping MZ from the tuple is also possible, since the SC9 floor already blocks every MZ file, but it would undo the earlier finding 3 wording. - In either case, run the full
test-unitsuite before pushing.
- If byte-only PEs should count as partially analyzed: update test_sc8_shipped_bytecode.py:166-167 to
- CI history:
-
[Non-blocking]
src/skillspector/nodes/build_context.py:3291: a referenced Python helper whose first bytes areMZ,GIF87aorGIF89akeeps a staleopaque_contentevent, which gives a false AE1 HIGH and an incomplete scan.- Mechanism:
- Lines 3291-3306 set PARTIAL and append a PARTIAL
opaque_contentevent before the Python-source loop. - At 3442,
promote_artifact_to_decoded_text(artifacts.py:445-456) sees BINARY + PARTIAL + no reason and makes the row TEXT/ANALYZED, but the event is never withdrawn. - finalize_inspection_ledger.py:590-605 then raises AE1 from the partial event on a referenced target.
- Lines 3291-3306 set PARTIAL and append a PARTIAL
- Transcription input: SKILL.md with
See [the helper](scripts/scale.py), wherescripts/scale.pyisMZ_SCALE = 2\nprint(MZ_SCALE)\n.- Main: SAFE 0, complete.
- Head: CAUTION 25,
is_completeFalse, AE1 HIGH on SKILL.md. The inventory row is text/analyzed but carries anopaque_contentpartial exception. partially_inspected_files and entirely_uninspected_files are both 0, so the counts disagree withis_complete. scripts/img.pystarting withGIF89a_HEADER = b'GIF89a'gives the same result.- Controls: an unreferenced MZ helper and a referenced plain helper are both SAFE and complete. A referenced
docs/notes.txtstarting withMZstays BINARY/PARTIAL, as intended.%PDF-is not valid Python, so it takes a different path.
- Consequence: a benign skill gets a false HIGH finding, CAUTION and an incomplete scan. The trigger is rare.
- Expected fix: emit the event and set PARTIAL only for artifacts that are still BINARY after the Python-source decode loop. Either move the block after build_context.py:3394-3460, or skip paths whose
decode_python_sourcesucceeded. Add a graph test with the input above that expects complete, SAFE and no AE1.
- Mechanism:
-
[Non-blocking]
src/skillspector/nodes/build_context.py:1964: the full lossy text of every non-hidden GIF, PDF and PE file (up to 16 MiB each) now enters the LLM inputs.- Code path:
- Lines 1961-1972 store
_redact_for_external_model(path, content)inllm_file_cachefor any non-hiddenreadable_binaryrow. That redaction only changes.envfiles. - No later
llm_file_cache.popapplies to ordinary assets, andllm_componentsis taken at 3527. _mark_unanalyzed_executablessetslocal_only(1056) without removing the entry, and the semantic analyzers readllm_file_cachewith nocontent_kindorlocal_onlyfilter.- The per-artifact cap is 16 MiB (constants.py:41, ANALYSIS_RESOURCE_BOUNDS.md:18).
- Lines 1961-1972 store
- Main baseline, run with main's code: a 1.2 MB PDF, a 1.1 MB GIF and a setuptools
cli-64.exebeside SKILL.md givellm_components == ['SKILL.md']. On the head all three are added. - Token estimates: main's
estimate_tokenson the cached provider view, with only the gate transcribed and no live provider.- Totals: 41 real PDFs 804,711 tokens; 39 real GIFs 2,311,636; 10 PE launchers 133,325.
- Single files: the 1.2 MB PDF 289,753; the 1.1 MB GIF 267,990; a 2.4 MB GIF 570,143.
- Batching:
- The default glm-5.3 input budget is 96,000 tokens, and
get_batches/chunk_file_by_lines(llm_analyzer_base.py:1064-1110, 797-835) have no batch cap. - The 1.2 MB PDF alone becomes 3 chunks per semantic analyzer. Each is about 100,000 estimated tokens, above the budget, because per-line estimates are floored. In this set, 19 GIF chunks and 2 PDF chunks exceed the budget.
- Real tokenizer counts on text full of U+FFFD are likely higher, so these calls may fail and make scans incomplete. This was not exercised.
- The default glm-5.3 input budget is 96,000 tokens, and
- Consequence: with the default
use_llm=True, skills with ordinary documents or images make many large provider calls of decoded noise and run much longer. README.md:953 already says analyzer-eligible contents go to the provider, but nothing says binary formats are now eligible. Docs row 122 says only "Lossy text projection is inspected", and README.md:961 still says compiled content cannot be analyzed. - Expected fix: fix this together with finding 1.
- Submit only the bounded text-run projection under a character cap, or only when the projection contains instruction-like text. The full lossy view can stay for local static analysis.
- Consider not submitting records marked
local_only. - Document the provider policy for binary formats.
- Code path:
-
[Non-blocking]
src/skillspector/artifacts.py:419: the XMP exemption only works for a single packet that fits inside one raw scan window starting at character 0.- Code path:
pdf_xmp_format_spansreturns no span unless its argument starts with%PDF-(419), contains the whole packet from header to<?xpacket end=(426-437), and has exactly one header (426).- It is called on per-window text (static_patterns_prompt_injection.py:369, static_patterns_memory_poisoning.py:544).
- Artifacts over 256,000 characters are scanned in raw windows (static_runner.py:118-120, 2444-2453, 2590-2601). Window 1 ends at character 247,808, and window 2 starts at 231,424, so window 2 does not start with
%PDF-. _binary_text_findingsreceives the full content but discards it (static_runner.py:3071).
- Real file (transcription): the iMovie
stop_glyph.pdfis 469,680 characters, with the packet header at 136 and the metadata body running to 462,113.- Unreferenced beside a benign SKILL.md, it is CAUTION 36, with P2 HIGH on the BOM (line 8), 21 P9 MEDIUM on the padding (lines 6423-6442) and AE4. Main: SAFE 0.
- Across 35 real large PDFs that contain an XMP packet, the helper fails in the owning window for 12 distinct files. On the whole file it succeeds for all of them.
- Synthetic pair (541,364 characters, same standard packet): with the packet late, the scan is CAUTION 28, with P2 on the BOM and P9 on the padding. With the packet early, it is SAFE 0 with no findings. MP2 appears only for some padding shapes.
- Multiple packets: the 1.2 MB PDF in finding 1 has two XMP packets, so the helper gives up. Both BOMs get P2 HIGH and the padding gets 42 P9. Incremental-update PDFs commonly carry more than one packet.
- Consequence: the exemption that the PR documents and the reply describes does not hold for larger PDFs or for multi-packet PDFs.
- Expected fix:
- Compute the spans once on the whole artifact and map them into window offsets, for example in
_binary_text_findings(which already receives the full content) or alongside the active security view. - Handle every standard packet.
- Add tests for a packet after the first window, a packet crossing the window boundary, and two packets.
- Compute the spans once on the whole artifact and map them into window offsets, for example in
- Code path:
-
[Non-blocking]
docs/scan-completeness.md:122: the docs and the completeness report overstate readable-binary coverage.-
Row 122 says an incidental GIF/PDF is "complete / true (absent other limits)" with exits 0/0/0. That holds only for the synthetic ASCII fixture. Measured (transcription) with a real
assets/icon.gif(125 B, LZW) andassets/manual.pdf(1,479 B, Flate):Main Head Disposition out_of_scope analyzed scope_exclusionsboth files, binary_content[]Ledger exceptions none none Findings none AE4 on both files Risk SAFE, score 0 SAFE, score 12 CLI exits 0/0/0 0/0/1 In the local sample, 47 of 49 real GIF/PDF files got AE4.
-
The files disappear from
scope_exclusions: artifacts.py:397-398 gives ANALYZED, and static_runner.py:3296-3300 skips thebinary_contentevent. Yet Flate and LZW data is never decoded; src has no such decoder. Row 121 ("Unreferenced incidental image ...binary_contentinscope_exclusions") is now also wrong for GIF images. -
The earlier review's finding 4 offered "unreferenced files fully analyzed as text become ANALYZED" as one option. That condition no longer holds once the lossy gate admits compressed files, so this request does not reverse that advice.
-
Consequence: completeness reports and docs claim more coverage than exists for compressed content, and they mislead readers about the exit codes they will see.
-
Expected fix:
- After the finding 1 fix, update rows 121-122 to the measured behaviour.
- Keep readable binary formats visible in
scope_exclusionsunder a distinct reason, for examplebinary_text_projection("text runs inspected; container streams not decoded"). - Update the PR body's validation section to reflect the CI result.
-
PIC tradeoffs:
- Prefix-only gate: this is the simplest way to make sure no byte pattern can hide instructions behind these four prefixes, and it closed the first-round bypasses completely. The cost is that format data is treated as prose by rules tuned for text. A printable-run projection keeps most of the evasion fix. Its knob is the minimum run length: too long, and an author can break instructions up with control bytes; too short, and binary noise comes back. Tests at both ends would make that choice explicit.
- Rule set for readable-binary records: restricting them to instruction rules removes most of the noise, but it also drops the Unicode-evasion checks (AE4, bidi P2) on the readable text. Running those checks only inside printable runs keeps them without scoring decoded compressed bytes.
- MZ in the prefix tuple: it gives readable executables accurate wording (earlier finding 3), but it changes the evidence for every PE (finding 2). The SC9 floor already blocks every MZ file, so scanning PE text adds detail to the report rather than changing the verdict.
- LLM submission: sending the projection to the LLM gives semantic coverage of hidden instructions, but it costs provider tokens and sends binary-derived content off the machine. A bounded projection, or submission only on instruction-like text, is the middle ground.
- Timing of the disposition: setting the readable-binary disposition and ledger event before format-aware decoding keeps the logic in one block. Finding 3 shows it can go stale when #579's promotion runs later. Deciding after the decode loops avoids that class of mismatch.
Verification and gaps:
- Head: live head
eaebfabb70e9177b54f4177de8f3a58a50fada45, confirmed withgh pr viewjust before writing. Its merge-base is main 5edc347, and the provided diff matchesgit diff merge-base..head. The replies cite4ba349f6for the fixes; later commits are merges of main. - CI:
test-unitfailed (run 37934042362: 1 failed, 12,900 passed, 1:52:42).changes,lint,OpenCode TypeScript Tests,DCO Checkanddocker-smokepassed, and the merge state is BLOCKED. Main's run at 5edc347 took 1:00:16. I did not attribute that difference to this PR, though it is consistent with finding 1. - Conflicts: none with main;
git merge-tree --write-treeagainst origin/main exits 0. - Method: every measurement is a transcription: a copy of main 5edc347's src with the PR's src diff applied, checked byte-identical to the head's changed source files and run with main's venv (
python -I). Nothing was imported from the PR checkout. Graph runs useduse_llm=False. The runs used Python 3.13 on macOS, while CI uses 3.12. Timings ran with two probes in parallel. - Also verified:
run_static_patterns' switch fromnot binary_pathstonot raw_inventoryaffects only the non-graph helper, whose callers are all tests.- The new tests exercise real code paths, and the positive ones fail on main.
- Two tests are weaker than they look: test_opaque_reference_reporting.py:214-215 and test_security_remediation.py:1100-1104 derive their expected value from the result itself, so they pass under either policy.
- I did not run the PR's tests or code, per policy.
- Gaps:
- No live LLM provider was exercised. Provider exposure and chunk counts come from
llm_file_cache,estimate_tokensand a trace of the batching code. - The real-file sample is local: 40 PDFs (mostly app and system icon PDFs), 9 of a planned 40 GIFs (stopped early because multi-MB GIFs take 200-300 s each), and 10 PE launchers.
- I did not check how agent runtimes display NUL or invalid bytes.
- I did not measure the interaction with #741's regex budgeting.
- No live LLM provider was exercised. Provider exposure and chunk counts come from
Decision: Changes Requested (reviewed head eaebfabb70e9177b54f4177de8f3a58a50fada45)
| # These printable prefixes are also valid instruction text. Keep the | ||
| # claimed binary format's coverage limits, but inspect the readable bytes. | ||
| # Lossy decoding must not let one invalid byte or a binary comment hide text. | ||
| readable_binary = data.startswith((b"MZ", b"GIF87a", b"GIF89a", b"%PDF-")) |
There was a problem hiding this comment.
This gate is now the prefix alone, so every real GIF, PDF and PE file gets its whole lossy decode_text view scanned by every pattern module (static_runner.py:3113-3149) and by _text_signals (artifact_integrity.py:1057-1088). The only binary-specific filters are NUL-only P2, AE3 and a single XMP packet.
Measured on a transcription of the head (main 5edc347 plus this PR's src diff, byte-identical to the head), with an unreferenced real file beside a benign SKILL.md and use_llm=False:
- PDFs: AE4 on 38 of 40 real PDFs that are all SAFE and complete on main. 10 of them become CAUTION or incomplete.
- GIFs: AE4 on 9 of 9 real GIFs. A 1.1 MB GIF is incomplete after 212 s, and a 2.1 MB GIF hits
runtime_limitafter 301 s. - 1.2 MB PDF: DO_NOT_INSTALL, score 89, after 227 s (main: SAFE in 0.22 s). The findings come from Flate noise (P2 on U+202E, TM1), two XMP packets (P2 on the BOMs, 42 P9) and
/Widthsarrays (137 MP2).
Please keep the evasion fix but scan a bounded printable-run projection instead of the whole decode:
- For
readable_binaryrecords, run only instruction rules (P1, PE3, E1 and similar), and make AE4 ignore control bytes outside printable runs. - Add negative graph fixtures (a Flate PDF with two XMP packets, a multi-frame GIF, a PE stub) plus a bounded-time case.
See finding 1.
| "outer_hidden": component.get("outer_hidden", hidden), | ||
| "concealed_executable": True, | ||
| "excluded_from_analysis": True, | ||
| "excluded_from_analysis": not readable_executable, |
There was a problem hiding this comment.
CI test-unit fails on this head (run 37934042362, "1 failed, 12900 passed").
- Failing test: main's
tests/nodes/analyzers/test_sc8_shipped_bytecode.py::test_ordinary_binary_executable_cannot_remain_safefails at line 166,excluded_from_analysis is True. - Cause: its fixture
b"MZ\x00\x00binary payload"is nowreadable_binary(artifacts.py:374). This line therefore setsexcluded_from_analysisFalse, and lines 1029-1035 pickbinary_executable_textinstead ofbinary_content. - Conflicting tests: the PR's own test at test_security_remediation.py:72-79 asserts the opposite contract for
MZ+\x00binary. - Effect on the verdict: none. The SC9 floor of 51 still applies (report.py:532-536, 634-646). But every byte-only PE is now labelled "Readable text was inspected".
Please pick one contract:
- either update test_sc8_shipped_bytecode.py:166-167 to the partially-analyzed flags and reason, and say so in the PR body;
- or make MZ readable only when real text follows the header, and keep that test unchanged.
Either way, run the full test-unit suite before pushing. See finding 2.
| selected_primary = state.get("primary_file_path") | ||
| for artifact in artifact_inventory: | ||
| path = artifact["path"] | ||
| if ( |
There was a problem hiding this comment.
This block appends a PARTIAL opaque_content event before the Python-source loop. At line 3442, promote_artifact_to_decoded_text turns the same reason-less BINARY/PARTIAL row into TEXT/ANALYZED, but the event is never withdrawn, and finalize_inspection_ledger.py:590-605 raises AE1 from it.
Transcription: SKILL.md links scripts/scale.py, which contains MZ_SCALE = 2\nprint(MZ_SCALE)\n.
- Main: SAFE 0, complete.
- Head: CAUTION 25,
is_completeFalse, AE1 HIGH. The partial and uninspected counts are both 0. - Same result: a helper that starts with
GIF89a_HEADER = ....
Please emit the event and the PARTIAL disposition only for artifacts that are still BINARY after the Python decode loop. Either move this block after build_context.py:3394-3460, or skip paths that decoded successfully. Add a graph test that expects complete and SAFE with no AE1. See finding 3.
| provider_submission_allowed | ||
| and not _is_hidden_path(path) | ||
| and artifact["content_kind"] == "text" | ||
| and (artifact["content_kind"] == "text" or artifact.get("readable_binary")) |
There was a problem hiding this comment.
With the prefix-only gate, this condition puts the full lossy decode of every non-hidden GIF, PDF and PE file (up to 16 MiB each) into llm_file_cache. Nothing removes it later: _mark_unanalyzed_executables sets local_only but keeps the entry.
Main's estimate_tokens, with only the gate transcribed:
- Totals: 804,711 tokens for 41 real PDFs, 2,311,636 for 39 GIFs and 133,325 for 10 PE launchers. On main, none of these files is in
llm_components. - One file: the 1.2 MB PDF alone is 3 chunks of about 100,000 estimated tokens per semantic analyzer, above the 96,000-token glm-5.3 budget.
No live provider was exercised. Docs row 122 and README.md:961 do not say that binary formats are now sent to the provider.
Please submit only the bounded text-run projection from the finding 1 fix, under a character cap (or only when it contains instruction-like text), and document the provider policy for binary formats. See finding 4.
| def pdf_xmp_format_spans(content: str) -> tuple[int, int, int]: | ||
| """Recognize one standard XMP packet's BOM and trailing ASCII padding only.""" | ||
| header = '<?xpacket begin="\ufeff" id="W5M0MpCehiHzreSzNTczkc9d"?>' | ||
| if not content.startswith("%PDF-"): |
There was a problem hiding this comment.
This helper only recognizes the packet when it receives text that starts with %PDF-, holds the whole packet, and has exactly one XMP header (line 426). The callers pass per-window text, and static_runner windows artifacts over 256,000 characters (window 2 starts at 231,424).
Transcription results:
- Real
stop_glyph.pdf(469,680 characters, padding near 462K): CAUTION 36, with P2 HIGH on the BOM and 21 P9 on the padding (main: SAFE 0). - Synthetic PDF with a late packet: CAUTION 28. The same file with the packet early is SAFE.
- The 1.2 MB two-packet PDF from finding 1: P2 on both BOMs and 42 P9.
Please compute the spans once on the whole artifact, for example in _binary_text_findings, which already receives the full content, and map them into window offsets. Handle every standard packet, and add tests for a late packet, a boundary-crossing packet and two packets. See finding 5.
| | Unsupported in-profile `SKILL.md` below directories or normal/renamed ZIPs | Same required-content event at real/virtual member path | failed / false | 2 / 2 / 2 | false | | ||
| | Supported benign ZIP, including renamed or empty ZIP | Existing bounded archive inspection; incidental assets remain exclusions | complete / true | 0 / 0 / 0 | true | | ||
| | Unreferenced incidental image beside benign instructions | `binary_content` in `scope_exclusions` | complete / true | 0 / 0 / 0 | true | | ||
| | Incidental GIF/PDF with printable magic | Lossy text projection is inspected; ordinary binary NULs and PDF metadata padding are not evasion findings | complete / true (absent other limits) | 0 / 0 / 0 | true if no findings | |
There was a problem hiding this comment.
This row holds for the synthetic ASCII fixture but not for real files. Transcription with a real icon.gif (125 B, LZW) and manual.pdf (1,479 B, Flate):
- Main: both out_of_scope under
binary_contentinscope_exclusions, SAFE 0, exits 0/0/0. - Head: both
analyzed,scope_exclusionsempty, AE4 on both, SAFE 12, exits 0/0/1.
In the local sample, 47 of 49 real GIF/PDF files got AE4. Row 121 is also no longer accurate for GIF images, and the compressed data is never decoded.
After the finding 1 fix, please:
- update rows 121-122 to the measured behaviour;
- keep readable binary formats in
scope_exclusionsunder a distinct reason, for examplebinary_text_projection; - note in the PR body's validation section that CI
test-unitcurrently fails.
See finding 6.
Readable instructions could be hidden behind MZ, GIF, or PDF prefixes and excluded as binary content. These files now retain a lossy text projection even when invalid UTF-8 bytes or NULs are present. Static and provider-eligible inputs can inspect that text while canonical bytes remain available for byte-oriented analysis.
Incidental readable binary content can complete its text scan. Required or referenced opaque content retains its incomplete policy; readable executables explicitly say their text was inspected but binary behavior remains unverified. Referenced readable files and executables also count as partially inspected, so report totals agree with their warnings. Unsupported primary files still fail. Excluded archive members keep their existing policy. Only exact standard XMP header and terminal padding spans receive format treatment; instructions in metadata or unrelated padding remain visible.
Validation: 785 tests passed against source and a freshly installed wheel, including graph, primary-input, and Python-source checks. Five focused synthetic skills matched across source and wheel, covering invalid bytes, NULs, genuine PDF/XMP, and excluded archives. These focused tests made no live provider calls.
Combined verification across the updated PRs: 6,243 regression tests passed against source and again against the freshly installed wheel, with seven conditional skips and four expected failures per run. All 19 source/wheel sample pairs matched. The 12-skill corpus retained its findings and risk ratings; four former hangs now finish with explicit partial-analysis results. The 93 extension tests passed. Two synthetic live NVIDIA Build checks passed on the final wheel: benign-note was complete/SAFE, and the exfiltration sample retained SSD-3 with complete semantic and meta analysis and a DO_NOT_INSTALL recommendation. All seven recorded LLM analyses succeeded. Live checks used the configured model/reasoning defaults through a test-only proxy that kept the real credential outside the scanner.