fix(audit): a scan that could not run is not a scan that found nothing - #1547
Merged
Conversation
Ndevu12
added
the
pin-bump-deferred
`--verify` is the operator asking for the harder look. Every failure of the content scan was answered with the empty marker list a clean scan returns, so the run fell back to the weaker finding — whose own advice is to run `--verify`. The operator was told to run the thing they had just run, over a scan that never happened, and the rotation verdict counted the module as settled. Measured: on an interpreter without one of the engine's dependencies the import raises and every module reads as scanned-and-clean. The scan now reports three states rather than two. A blocked one keeps the module at its `info` grade for what was actually observed and adds the hole as its own item, because something found and a look that did not happen are different claims and the report groups them differently — folding both into one `unknown` made the run headline "no findings" over a module that looks modified. The hole withholds the rotation all-clear, since a scan that did not run cannot say there is no live foothold, and it is one item for the run with each distinct cause named once rather than a copy per module. A scan that ran and came back clean now says so, instead of repeating the advice to run the scan that had just cleared it. `KeyboardInterrupt` is no longer swallowed here: this runs once per module, so catching it cost one interrupt per file to stop an audit and everything the run went on to report carried scans nobody performed. `run_probe` already states that rule for the layer above. The test that needs the engine now skips with a reason where it cannot load, rather than failing as though the tool were broken.
The first version of this fix only closed the exception channel. The engine almost never raises — it reports a tree it could not fully read IN its verdict, and its own docstring says the caller must not render that as clean. Reading the marker list alone turned every one of those into "scanned, found nothing", including the two that return before the scanner runs at all: a directory too large to scan, and one holding a pipe or device the scan must not open. That made this worse than what it replaced. The old sentence claimed nothing; the new one affirmatively stated a clean scan. And the second case is reachable: something that has already appended to a bundle module can create one pipe in that directory and buy a printed statement that a content scan found no worm markers, with the credential-rotation all-clear intact. Only a scan that read the whole directory and found no markers clears a module now. Anything else keeps the module unsettled and carries the scan's own reason through to the operator. The sibling consumer in `host_artifacts` already reads every field of that verdict; this asks the same questions rather than a narrower one. The tests could not have caught it: they drove the clean path with a `Mock(markers=[])`, which answers that one attribute and stubs away the fields the verdict uses to say it did not finish. They use a stand-in that mirrors the real shape, held to it by a check that runs wherever the engine can be imported.
The round on the previous commit found the class surviving below the content scan and beside it. A module the host would not let this read answered exactly as one that was read and held nothing. It then fell through with no finding, no count and no mention anywhere in the report — the tier that decides whether the content scan runs at all, saying nothing when it could not decide. Two files with a byte-identical payload, one readable and one not, produced one finding. It is now its own grade, and it withholds the rotation all-clear: MEASURED first, 22477 modules under 44 enumerated roots on an ordinary host, none unreadable and none anything but a regular file. The bounds this tool chooses for itself stay an `info` note — a location the host refused is a different claim. Reading a file that is not a regular file has no timeout, so anything able to append to an application's module could also stop the audit by creating a pipe named like one beside it. The engine guards exactly this before its own read; this side did not. `scanned_clean` is CONFIRMED-only — the engine drops its weaker tiers before answering — so "found no worm markers" was narrower than an operator reads it. The sibling attaches a caution to that same field for that reason, and the previous commit claimed to ask the same questions while dropping it. The sibling itself had both defects this branch exists to fix. It threw away the reason a content scan failed, so `--verify` over an engine that cannot load printed output byte-identical to not passing the flag — advice to run the command just run — and kept the rotation verdict at "safe pending a check". And it described a directory where the scanner never ran as one where a content scan of the rest found no markers. The test that pinned the unreadable case asserted no findings at all. Its contract is that the run survives; the silence was how that happened to be expressed, and it made the silence the spec. Also: the `DirVerdict` stand-in added last commit was missing the derived read the other consumer asks for first, and the check holding it to the real shape compared only fields. It compares the public surface.
Measured against `origin/main` for the same files: comment-plus-docstring to code had gone from 0.12 to 0.25 in app_bundle.py, doc lines from 26 to 83, and the test file had picked up docstrings where the convention is a name that says it. What is left carries a measurement, a rejected alternative or a trap. The narrative each one was retelling is in the commit that made the change, which is where it belongs.
Three constants carried a comment each, which is a name that failed. Two of them said which surface the id belongs to — so the name says it now: `APP_BUNDLE_SCAN_BLOCKED_ID`, `APP_BUNDLE_MODULE_UNREADABLE_ID`, `HOST_ARTIFACT_SCAN_BLOCKED_ID`. The third restated a measurement the function that builds the finding already carries. Two comments sat above a local, which reads the same way. Both were about the branch under them rather than the value, and moved into the function that applies the rule.
…assertion CI, on the modules this host cannot import. `test_the_lone_indicator_path_keeps_its_finding` is the spec for the thing this branch changes: an aborted scan must not take the weak finding with it. It still does not — the finding is kept — but the run now also reports the scan that did not happen, so the exact-list assertion no longer holds. It asserts the contract it is named for, plus the item, rather than a list that forbids adding one. The stand-in equivalence check compared `dir()`, and a field given a default here to keep the fixtures short becomes a class attribute the real verdict does not have. It compares the fields and the properties, which is where `has_markers` actually went missing.
Ndevu12
force-pushed
the
fix/audit-unrunnable-verify-scan
branch
from
September 2, 2026 10:44
10e252b to
c22ff8f
Compare
Ndevu12
marked this pull request as ready for review
September 2, 2026 10:59
…uld not read The third round measured the asymmetry this branch had created. Locking a module made the run fail closed; locking the DIRECTORY holding it made it fail open — `os.walk` with no `onerror` drops the whole subtree, no finding and no count. Anything able to write a module can chmod its directory, so the new counter bought nothing against the case it was for while handing an attacker a cheap way to raise a gate. Both directions answer the same way now, and so does a base that exists and will not list, which `Path.glob` reports as a base holding nothing. The finding NAMES what could not be read. A count alone raises a gate with no way to act on it, which is alarm fatigue against a gate whose whole purpose is to be believed; the sibling unknown-tier finding names its locations for that reason. A dangling link is no longer a hole. Nothing is behind it to read, which is what the engine states for the same case, and Electron cache trees churn `.js` while an audit runs. A bound reached on the last module of a directory never ran the inner break, so it was reported as nothing — measured at roughly one truncation in five on a real application tree. Also: one of my own tests pinned the dangling-link grade the round refuted, and a `sed` rename silently did nothing because BSD `sed` has no `\b` — the reference count it printed was of matches it had not made.
CI. The placeholder for a walk error carrying no filename was `"?"`, which is the render vocabulary's unknown-severity marker — a string literal leading with the glyph a reader decodes as severity, which `test_no_surface_hardcodes_a_glyph_in_MARKER_POSITION` exists to stop drifting. The root being walked is both allowed and more use: it names where the hole is instead of standing in for a name. Found by CI, not locally, because a repo-wide contract test scans every surface file and so is affected by any string literal added anywhere — which the module suites I had been running cannot show.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
saw audit --verifyis the operator asking for the harder look on a module that alreadylooks modified. Every failure of that scan was answered with the empty marker list a clean
scan returns, so the run fell back to the weaker finding — whose own advice is to run
--verify. The operator was told to run the thing they had just run, over a scan that neverhappened, and the rotation verdict counted the module as settled.
Measured on an interpreter missing one of the engine's dependencies: the import raises and
every module reads as scanned-and-clean.
What changes for an operator
modules it did not settle.
scan that did not happen cannot say there is no live foothold.
to run the scan that had just cleared it.
runs normally.
Shape of the fix
The hole is a separate item from the observation. Folding both into one
unknownmade therun headline read "no findings" over a module that looks modified — the renderer treats
unknownas the absence of a look, not as something found, and this is both. It is one itemper run with each distinct cause named once, not a copy per module.
KeyboardInterruptis no longer caught here. This runs once per module, so swallowing itcost one interrupt per file to stop an audit, and everything the run went on to report
carried scans nobody performed. The layer above already states that rule.
Verification
test_app_bundle, 10 mutations against the new pins, all killed — includingthe two that only showed up under mutation: a clean scan withholding its own negative
result, and the per-run item repeating one cause once per module inside a single item.
remediation now meets outright rather than joining the ratchet.
instead of failing as though the tool were broken.