Skip to content

fix(artifacts): inspect text behind printable binary prefixes - #794

Merged
yashrajp22 merged 55 commits into
mainfrom
yashraj/fix-printable-magic-evasion-20261007
Oct 16, 2026
Merged

yashrajp22 merged 55 commits into
mainfrom
yashraj/fix-printable-magic-evasion-20261007

Conversation

@yashrajp22

@yashrajp22 yashrajp22 commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Printable MZ, GIF and PDF prefixes no longer hide readable instructions. Static instruction checks and eligible provider input use a printable-text projection capped at 32,768 characters per file, with findings mapped back to the original content. XMP formatting is handled before scan windows, and valid Python source takes precedence over a binary-looking prefix.

Incidental unreferenced assets keep an explicit binary_text_projection scope exclusion, including when they reach the projection cap. Generic Unicode-density and mixed-script noise in binary data no longer produces AE4; instruction-obfuscation checks (AE6), credential access, exfiltration and prompt-injection detection remain active. Referenced files, actual resource failures and unverified executable behavior retain their incomplete-analysis warnings. Readable executables are marked partially analyzed rather than wholly excluded, while SC9 and its blocking risk floor remain.

The projection does not decode compressed content or inspect text beyond its cap. The cap applies per file, so multiple assets can add provider input. Hidden and nested content stays local-only. The branch includes the merged OMS changes from #795, preserving both projection paths and the existing bounded provider excerpts.

Validation at b134113e: 1,101 selected unit/integration tests passed from source and a fresh installed wheel, including the OMS and opaque-reference suites. Eight full-skill cases ran on the original and fixed versions in both modes (32 CLI scans): the original benign PDF/GIF false positives reproduced, the fixed assets stayed SAFE and complete, and malicious instruction, referenced-file and executable controls retained their expected findings. Twelve additional sample directories ran in both modes/revisions (48 CLI scans), with source/wheel agreement and unchanged findings, verdicts and completion status. Coverage counts include the merged OMS inspection. Lint passed. These checks were offline (--no-llm); live-provider behavior and the entire repository test suite were not rerun locally.

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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

  1. [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), where decodable is the strict data.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 GUIDE unreferenced or pointed to in prose from SKILL.md. Each of these inputs gives transcribed readable_binary=False, binary/out_of_scope, llm_components ['SKILL.md'], is_complete True and SAFE:

      • GIF89a + instructions + \xff
      • %PDF-1.4 + instructions + \x00
      • %PDF-1.7\n%\xe2\xe3\xcf\xd3 + instructions
      • GIF89a with a Latin-1 \xe9 mid-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 + \xff is classified TEXT by the ratio rule at artifacts.py:379-385 and gives PE3 (P1 with the canonical phrase) and CAUTION. Notes + ... + \x00 gives 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 for prefix + b"\xffbinary" and prefix + 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 + \xff is 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.
    • 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_install is 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, GIF89a and %PDF-, stop requiring strict whole-file UTF-8 and no NUL. Build the projection from the lossy decode_text view so that a stray invalid byte, a few NULs or the %\xe2\xe3\xcf\xd3 comment 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_MAGIC file:

        • 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_binary records, 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/\x00binary negative 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.

      • 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.

  2. [Blocker] src/skillspector/nodes/build_context.py:3209: a readable printable-prefix member of a ZIP under an excluded directory (for example node_modules) turns a benign skill into DO_NOT_INSTALL.

    • How it happens:
      • classify_artifact returns PARTIAL for every readable_binary record 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_directory reason.
      • 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_content event 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.zip containing manual.pdf = b"%PDF-1.4\n1 0 obj\n<< /Type /Catalog >>\nendobj\n%%EOF\n". This is the minimal PDF from test_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 .venv wheel with pkg/icon.gif = GIF89a\nicon placeholder\n flips 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.
    • 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_binary member to OUT_OF_SCOPE with nested_exclusion_reason, for example if 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.zip with the ASCII PDF member. Expect the member to be out_of_scope with reason excluded_directory, no SC9, is_complete True 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.
  3. [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_kind BINARY, and an MZ prefix is executable magic (nested_artifacts.py:379-380, 485-500). The unchanged _mark_unanalyzed_executables therefore takes its BINARY branch (build_context.py:996), which:

      • sets excluded_from_analysis, local_only, concealed_executable and inherited_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_components is fixed at 3263 before the marking at 3444, and the PR's own test expects P1 on the MZ GUIDE.

    • Transcription (main's code with the classify, LLM-cache and static-runner hunks applied):

      • GUIDE is partial, with findings P1, SC9 and YR4.
      • All 15 static_patterns_* ledger rows are completed, and the file is in llm_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_only flag next to llm_components membership 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, give readable_binary executable records their own reason, SC9 message and ledger message (readable bytes analyzed; executable format not interpreted), and stop setting excluded_from_analysis for 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 GUIDE component flags and risk_recommendation.
  4. [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_binary record, referenced or not. The new event at build_context.py:3209-3218 adds an opaque_content exception on top.

    • Transcribed results:

      • An unreferenced manual.pdf containing %PDF-1.4\nA plain text reference document.\n is complete/SAFE on main and partial/CAUTION on the PR.
      • The same file with the %\xe2\xe3\xcf\xd3 line is SAFE on both.
      • An ASCII GIF, and a root docs.zip holding 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.
    • 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_install and 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_install true in the "Unreferenced incidental image" row (line 121), and has no row for this case.
      • The reused opaque_content reason 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.
    • 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_kind BINARY 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 check readable_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_MAGIC would make the choice explicit.

Verification and gaps:

  • How this was verified:
    • Code traces on the head bd46d38, which differs from main 3c8e4b9 only 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_binary is False, so the head follows main's path exactly.
  • Key numbers:
    • Bypass inputs: SAFE and is_complete True, 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.
  • Inputs outside the new flag behave as on main:
    • Static-runner gating is consistent across both entry points (_is_binary_file checks only for NUL, which readable_binary rules out).
    • The fail-closed path for the primary file (unsupported_primary_content) is unchanged.
    • Hidden files stay out of the LLM inputs.
    • readable_binary is a NotRequired key and is not serialized into JSON, SARIF, Markdown or MCP output.
  • Tests: the new tests exercise real code paths without mocks. Traced, not run, they fail on main as expected: KeyError on readable_binary, GUIDE missing from llm_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_required and 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:
    • #793's reserved-path FAILED guards run before the new loop, and FAILED dominates.
    • #795 changes a separate OMS code path.
    • #741 only touches regex budgeting.
  • Head update: I reviewed bd46d3828c169e8d5b2be109fdb777047e09a229. The current head 4a0cbd8b8182c07bb9c415e4c22b2a6ddadfe443 only adds automated merges of main (#691, #577, #608) from update-pr-branches.yml; those commits change only the ANALYZE_USES_POSTPROCESS hook in static_runner.py (two lines in _scan_path), which is unrelated to this PR's hunks, so line references to static_runner.py below 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_cache and llm_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 \xff bytes, so matching the TEXT rule alone does not close the whole evasion class.

Decision: Changes Requested (reviewed head bd46d3828c169e8d5b2be109fdb777047e09a229; current head 4a0cbd8b8182c07bb9c415e4c22b2a6ddadfe443 only adds merges of main)

Comment thread src/skillspector/artifacts.py Outdated
Comment thread src/skillspector/nodes/build_context.py Outdated
Comment thread src/skillspector/nodes/build_context.py
Comment thread src/skillspector/artifacts.py Outdated
github-actions Bot and others added 19 commits October 8, 2026 22:45
… 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 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SkillSpector Review]

@yashrajp22 thank you for closing the binary-prefix bypass itself: every evasion case from the first round now produces findings.

Value and readiness

This is a status re-review at 1061b831. The PR's own diff is byte-identical to eaebfabb, which I reviewed on 2026-10-12 (same patch-id). Every commit since then is an automated merge of main. Both blockers are still open, so the PR is not ready for final review.

Material findings

Prior findings from my 2026-10-12 review:

  1. Blocker: the readable_binary gate checks only the prefix (artifacts.py:374), so every analyzer gets the full lossy decode of every real GIF, PDF and PE file. The result is false AE4/P2/P9/MP2/TM1 findings, incomplete scans, and 200-300 s runtimes. Still open.
  2. Blocker: test-unit is red. Main's test_sc8_shipped_bytecode.py:166 expects excluded_from_analysis for byte-only MZ. Still open.
    • That test is unchanged on main.
    • #749 added another byte-only MZ fixture (test_skill_script_exemption_fails_closed). It asserts only concealed_executable and an SC9 finding, and both hold under either contract, so it does not change the choice you need to make.
  3. Non-blocking: a stale opaque_content event on MZ- or GIF-prefixed Python helpers (build_context.py:3391). Still open.
  4. Non-blocking: the full lossy text of binary formats goes into the LLM inputs (build_context.py:2064). Still open.
  5. Non-blocking: the XMP exemption works only for a single packet in the first window (artifacts.py:419). Still open.
  6. Non-blocking: docs/scan-completeness.md:122 overstates coverage. Still open.

New: none. Since 5edc347c, #749, #802, #816 and #824 changed build_context.py, inspection_ledger.py, report.py and static_patterns_supply_chain.py. None of them touched the code this PR edits: _mark_unanalyzed_executables, _read_file_cache's provider check, the SC9 message chain or the SC9 score floors. The merge with main is clean.

PIC tradeoffs

Unchanged from my last review:

  • the projection and rule set for readable-binary records;
  • whether MZ belongs in the prefix tuple;
  • the LLM submission policy for binary formats.

Verification and gaps

  • I compared the patch-ids of eaebfabb and 1061b831. git merge-tree against main 8b993d32 is clean. I also reviewed the changes on main to the PR's files since 5edc347c.
  • CI: test-unit failed on eaebfabb (run 37934042362) and on 4ba349f6. Every later head is action_required. The PR code is unchanged, so I expect the same failure once a maintainer approves the workflow run.
  • All six threads from my last review are open with no reply. @mohgupta-ship-it approved on 10-09. This was a static review; I did not run the PR's code.

Decision: Changes Requested (reviewed head 1061b831a51a4ccbf4dca47f787584574b1dfc70)

@mohgupta-ship-it

Copy link
Copy Markdown
Member

lgtm merging

@mohgupta-ship-it
mohgupta-ship-it dismissed rng1995’s stale review October 16, 2026 06:51

lgtm merging, review comments addressed

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SkillSpector Review]

Hi @yashrajp22, thank you for the 10-14 rework. Bounding readable binaries to an instruction-text projection and allow-listing the rules was the right direction. The evasion this PR targets is closed, and the 200-300 s runtimes are gone.

Value and readiness

This is a re-review of 36e050ac and 92ade330 at head 149801b0. Blocker 2 is fixed and blocker 1 is mostly fixed. I'm requesting changes for one thing: on real benign PDFs and GIFs, the PR still gives worse results than main. A skill that ships an ordinary PDF in assets/ goes from SAFE and complete to CAUTION and incomplete. That would surface on many real skills, so we're holding this PR out of the current release. Everything else below is non-blocking.

Material findings

  1. Blocker, new in the fix commits: reaching the projection cap marks ordinary assets partial. build_context.py:3679-3680 sets partial whenever binary_instruction_view truncates at BINARY_TEXT_MAX_CHARS = 32_768 (artifacts.py:418), and static_runner.py:2364 adds SIZE_LIMIT. Compressed PDF streams easily produce more than 32K printable characters, so this fires on unreferenced incidental assets:

    Real file, unreferenced in assets/ PR head main
    209 KB PDF (2 XMP packets) CAUTION 0, partial (size_limit) SAFE 0, complete
    470 KB PDF CAUTION 0, partial SAFE 0, complete
    1.2 MB PDF CAUTION 8, AE4, partial SAFE 0, complete
    2.9 MB PDF CAUTION 8, AE4, partial SAFE 0, complete
    2.1 MB animated GIF SAFE 8, AE4, complete SAFE 0, complete
    29 KB PDF, 2.3 KB GIF SAFE 0, complete SAFE 0, complete

    In a sample of real PDFs over 100 KB, 7 of 16 hit the cap, and one 49 KB PDF did too. The three projection analyzers then report as degraded.
    Suggested fix: when an unreferenced incidental asset hits the cap, record a scope note, not PARTIAL. Alternatively, go partial only when the projection contains instruction-like text.

  2. Blocker (rest of prior finding 1): AE4 still runs on the projection. artifact_integrity.py:1058-1072 feeds binary_instruction_view(...) into _text_signals. Compressed bytes decode into short mixed Latin/Cyrillic/Greek tokens (e.g. Rл), which trip the mixed-script check. In a signal-level count over 76 real files, AE4 fired on 18/36 PDFs and 8/38 GIFs (0/2 PE).
    Suggested fix: skip the mixed-script and density signals for the projection, or require multi-character script runs.

  3. Tests: the real_binary_structure fixture compresses bytes(range(256))*4096, which shrinks to almost nothing. It never reaches the cap or produces the noise above. A test with a real compressed PDF and GIF over 32K projected characters would cover both blockers.

Prior findings

  • Blocker 2 (SC8 / test-unit): fixed. test_sc8_shipped_bytecode.py:166-168 now expects partially_analyzed_executable / binary_executable_text. The new contract is consistent, and the SC9 floor of 51 still holds. Please mention this contract change in the PR body.
  • Stale opaque_content on MZ/GIF-prefixed Python: fixed. Helpers named MZ_SCALE and GIF89a_HEADER scan SAFE and complete.
  • Full lossy text to the LLM: fixed. Only the capped projection is sent. Residual: the cap is per file, so every readable GIF/PDF/PE now joins llm_components. Nine real assets added about 171K characters of mostly noise; main sends none.
  • XMP exemption: fixed. All packets are found across the whole file.
  • Docs: partly fixed. docs/scan-completeness.md:122 says incidental GIF/PDF is "complete / true", which doesn't match finding 1 for real PDFs. The docs also don't say that AE4/AE6 run on the projection.

Verification

  • Static-only CLI scans (--no-llm) of real files on the PR head and on main, as tabled above. I reproduced the 209 KB and 2.9 MB PDF rows independently.
  • The evasion stays closed. GIF89a/GIF87a/%PDF-1.7 prefixes followed by injection and an id_rsa | curl line produce DNI 68 (P1, PE3, E1). The MZ variant produces DNI 100. Instructions in a real GIF comment block, appended to a real PDF or appended to a real PE are all caught. main reports SAFE 20 on the GIF/PDF cases.
  • Tests: 1364 targeted unit tests pass. The one failure was a wall-clock timing test (test_dense_directory_discovery_and_cache_complete_with_modest_real_elapsed_time) that also fails on main under load and passes when rerun. The integration-marked test_opaque_reference_reporting.py passes (56). CI test-unit on this head is still running.

Heads-up: this PR conflicts with #795 in _scan_all_views_detailed (static_runner.py) and in build_context. Both add a new early branch or loop at the same spot. A merge that keeps both (the readable_binary branch first, then #795's runtime check and OMS projection) passes both PRs' tests.


Decision: Changes Requested (reviewed head 149801b0be4ce043d2ba931806b0364c4f1508ed)

Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
@yashrajp22

Copy link
Copy Markdown
Collaborator Author

Thanks for the detailed examples! I fixed both remaining issues in b134113e.

  • Ordinary PDF/GIF assets no longer become incomplete just because their printable-text view reaches the cap. The report still says that the remaining text and binary content are outside the inspected scope.
  • Binary noise no longer triggers the generic Unicode warning. Hidden-instruction checks still run, and the malicious PDF/GIF cases still detect prompt overrides, credential access and uploads.
  • Added larger valid PDF/GIF fixtures, including ZIP cases and exact-cap checks. Referenced files, real cache limits and executables still keep their incomplete-analysis warnings.
  • Resolved the conflict with merged fix(ingest): inspect unverified OMS bundle contents #795 and tested both scanning paths together. I also updated the docs and PR description, including the partially inspected executable behavior.

All 1,101 selected tests passed from source and a fresh installed wheel. The 32 targeted CLI scans matched expectations, and 48 further sample scans agreed between source and wheel without changes to findings or verdicts. These were offline checks; no live LLM was called.

Please take another look when you have a chance. Thanks!

@yashrajp22
yashrajp22 requested a review from rng1995 October 16, 2026 09:17
@yashrajp22

Copy link
Copy Markdown
Collaborator Author

All review comments addressed

@yashrajp22
yashrajp22 merged commit 3a8ee68 into main Oct 16, 2026
6 checks passed
@yashrajp22
yashrajp22 deleted the yashraj/fix-printable-magic-evasion-20261007 branch October 16, 2026 15:03
yashrajp22 added a commit that referenced this pull request Oct 16, 2026
…on-20261007

fix(artifacts): inspect text behind printable binary prefixes

(cherry picked from commit 3a8ee68)
Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants