fix(scanner): scan single-file targets; fail on unusable ones - #90
Conversation
Local discovery used Path.rglob as its only mechanism, which yields the contents of a directory and nothing else. A target that was a regular file walked nothing, recorded no error, and exited 0 — so a malicious .pt named directly on the command line reported 'No AI models found' and passed clean, while the same file scanned via its parent directory was correctly CRITICAL. The README documents that exact form for both --strict and --lint. A nonexistent path failed the same way: empty SBOM, exit 0, indistinguishable from a genuinely clean repo. Handle each local target shape explicitly. A single file is now dispatched through the same extension ladder as the walk, extracted into _dispatch_local_file so the two cannot drift. Missing paths, broken symlinks, and files no scanner claims record a structured target_error and exit 1; --no-fail-on-risk does not suppress it, since that flag governs risk findings rather than a broken target. An empty directory is still a clean scan at exit 0, and the directory walk still skips non-model files silently. 'No AI models found' is now withheld when nothing was examined — it is a claim about the target's contents.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a4c85653b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ext in PYTORCH_EXTENSIONS: | ||
| self.artifacts.append(self._inspect_pytorch(full_path)) | ||
| elif ext == SAFETENSORS_EXTENSION: | ||
| self.artifacts.append(self._inspect_safetensors(full_path)) |
There was a problem hiding this comment.
Propagate named-file inspector failures
When a supported single-file target cannot be read or parsed, each inspector catches the exception and returns an artifact containing error, but this dispatcher appends that artifact without adding anything to self.errors. Consequently the CLI still exits 0—for example, an unreadable unreadable.pt produces an UNKNOWN artifact and a malformed SafeTensors header can produce a LOW artifact—so the new unusable-target gate remains fail-open for precisely the supported files it is intended to protect. Record returned inspector errors in self.errors (or let them propagate into a target/parse error) before considering the file successfully dispatched.
Useful? React with 👍 / 👎.
The bug
Local target discovery used
Path.rglobas its only mechanism.rglobyields the contents of a directory and nothing else, so a target that was a regular file walked nothing, recorded no error, and exited0:Same file, opposite verdicts. Reproduced in all three modes — default,
--strict,--lint.The README documents exactly that invocation for both flagship security features:
aisbom scan model.pkl --strictandaisbom scan model.pt --lint. The two examples a security-conscious user is most likely to copy were the two that silently did nothing.A nonexistent path failed the same way — empty-but-valid SBOM, exit
0, indistinguishable from a genuinely clean repo. A typo'd path in CI turned the gate green permanently.The fix
Each local target shape is now handled explicitly:
_dispatch_local_fileso the two paths cannot drift apart again. Works for every supported format, not just PyTorch.target_errorand exit1.--no-fail-on-riskdoes not suppress it — that flag governs risk findings, not a broken target.No AI models foundis withheld when nothing was examined. It is a claim about the target's contents, so it must not appear when the target was never opened.Deliberately unchanged: an empty directory is still a clean scan at exit
0, and the directory walk still skips non-model files silently. Skipping aREADME.mdinside a tree is correct; being handed one as the target is a failed instruction.Exit-code contract
012--no-fail-on-risk)Only the
1row is new for local targets; it matches what the remote fetch-failure path already does.Verification
--no-fail-on-riskstill clearing a real CRITICAL), which must pass both before and after.mainshows exactly one change — the missing-target case goingexit 0→exit 1with its new message. Every other captured surface is byte-identical:--help/--version/info,scanin terminal/markdown/JSON/SPDX form,--strict,--lint,--no-fail-on-risk,diff, the unknown-flag error path, and the emitted CycloneDX and SPDX documents.cve-2025-1889-nonstandard-extensionshares theNo AI models foundsymptom but is an extension-matching gap, not this bug, and its verdict did not move.binaries.ymlnever runs on PRs: single malicious file →2, nonexistent →1, empty dir →0, fixtures dir →2, andposix.systemstill detected inside the bundle.Behavior change to be aware of
A path that previously exited
0while scanning nothing now exits1. Any pipeline passing a file path was getting no scanning at all before this change, so nothing that genuinely worked is affected — but a pipeline that was silently green on a bad path will now fail, which is the point.Closes the local-target side of this gap; the remote fetch-failure equivalent was fixed previously.