fix(analyzer): distinguish format-only opaque references in AE1 - #597
chrisknvidia wants to merge 5 commits into
Conversation
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
|
Independent verification of
One note on that table: The case I was actually worried aboutA suppression rule trades false positives for possible false negatives, so I built the adversarial fixture rather than only re-running my own benign one.
AE1 still fires and the scan blocks. The suppression is genuinely narrow — it holds only for a uniquely inventoried target whose limitations are solely What I could not confirmI tried to exercise the ScopeAgreed with leaving the replacement rule and the coverage-denominator question out. Worth restating for anyone reading #596 later: this PR does not change the completeness block, so the Thanks for the quick turnaround on this. |
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
rng1995
left a comment
There was a problem hiding this comment.
[P2] Keep actively used opaque targets out of format-only suppression (src/skillspector/nodes/finalize_inspection_ledger.py:322-329). This condition is broader than the passive-asset scope because it never examines reference use. At this exact head, a real skill containing Run [the helper](assets/payload.asset). and a 1 KiB non-UTF-8 blob produces no issues; --no-llm --format json --fail-on-findings exits 0 with score 0, LOW/CAUTION, 50% coverage, and only a nonfatal opaque_content exception. Thus an actively invoked unknown-format payload loses the only AE1 signal. Please restrict the exception to references proven passive or allowlisted non-executable formats, or retain AE1/a replacement finding for active or unknown-format references. Recognized executable, misleading-extension, and oversized cases otherwise fail closed correctly.
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
|
@rng1995 The AE1 suppression is now limited to known passive formats; unknown opaque targets such as actively invoked |
A passive binary or opaque file referenced from
SKILL.mdcould produce a HIGH AE1 analysis-evasion finding even when the inspection ledger said its only limitation was the unsupported content format. This change propagates ledgerreason_codeevidence into AE1 and suppresses that finding only when the resolved target is uniquely inventoried as binary/opaque and every canonical limitation isbinary_contentoropaque_content.For a skill with one passive PNG, the reported result changes from an AE1 finding to no finding with
LOWseverity, while the scan remainsCAUTION, 50% complete, and incomplete. The coverage denominator, ledger exceptions, MCPsafe_to_install=false, and--fail-on-incompletebehavior do not change.The all-reasons requirement is the safety boundary. Failed or skipped inspection, missing or malformed ledger evidence, truncation, size/read/text failures, mixed or unknown reasons, conflicting inventory/reference state, executable content, and duplicate target records all retain AE1. DEX and Lua bytecode signatures are now recognized consistently with the existing executable-content path so UTF-8-decodable bytecode cannot enter the format-only exception.
Refs #596.
Validation
Exact pushed head:
2f59a5bd9010392762610580119531a504d3e3e1types-PyYAMLstub.skillspector:ae1-finaland passed the repository Docker smoke, including a real GitHub URL scan.LOW / CAUTION, coverage 50.0%/20.0%/11.1%, default exit 0, and strict-incomplete exit 1. DEX, Lua bytecode, and executable-bit PNG each retained AE1 + SC9,HIGH / DO_NOT_INSTALL, and exit 1. Benign text beginning withdex\nremained fully analyzed andSAFE.383476c1a1f92a045e775c26fe5e5f33a39f44c5e49c36a897d56e681933f387with no correctness or security blocker.The broad non-provider integration command reported 55 passed, 2 skipped, 2 failed. Both failures were unchanged live-agent tests: the standalone Codex CLI rejected the configured model/cache in this environment. Static product scans used
--no-llm; no live LLM/provider success was established. Local Docker coverage used Linux/ARM64, and the exact-head hosted Docker build/smoke passed on Ubuntu/x86_64. Windows and live-provider success remain unverified.Scope boundaries
This PR does not implement the separate replacement/use-sensitive policy. A passive opaque asset invoked in command text may still qualify for the format-only exception when no executable suffix, magic signature, or executable bit identifies it. Unknown executable formats without recognized suffix or magic remain a residual. Completeness and the denominator remain unchanged by design.
Additional Test GURU verification
A second exact-head verification pass exercised the feature through the real graph, installed wheel, CLI renderers, MCP, and concurrent requests:
No P0 or P1 defect was found. Two pre-existing/future-route P2 limitations were observed: replaying the already-committed finalizer output is not idempotent, although the production DAG invokes it once; and standard angle-bracket Markdown destinations containing spaces are treated as missing references, which remains fail-closed. Neither changes the security decision for #596.
Live NVIDIA provider requests were attempted without exposing credentials. The repository default z-ai/glm-5.2 returned HTTP 410 because the endpoint reports it as end-of-life; available-model overrides returned HTTP 401 in this environment. Those attempts failed closed and preserved the static AE1/SC9 contracts, but a successful live-LLM scan was not established. Windows and remote HTTP transport remain unverified; hosted Ubuntu/x86_64 CI and local macOS/Linux ARM64 paths are covered.