feat!: authenticate the cache, contain config paths, report what was not scanned (1.3.0) - #39
Merged
Merged
Conversation
…was not scanned (1.3.0) The scanned tree is untrusted input. This release closes the paths by which a repository could weaken its own scan, and makes the places the scanner does not look visible instead of silent. BREAKING: a scan that loads zero rules, a coverage rule with no usable report, and a discovered config whose rules/source_roots/include paths resolve outside the config root are now exit-2 errors rather than clean exits. CoverageReport gained a public field. - cache entries are authenticated per cache directory, so a .siloscan committed into a repository is ignored rather than trusted (unix mode binding plus location binding; NTFS alternate data stream on Windows) - config path containment resolves symlinks, closing the lexical bypass - project ignore matchers are rooted at the file that declares them, so results no longer depend on the process working directory - anchor=config consults the project's own ignore files, restoring module/root scan interchangeability for shared baselines - ignored file and directory counts in JSON, SARIF and human output - terminal sanitizer also escapes Unicode bidi controls - stderr writes tolerate a broken pipe instead of panicking to 101 - SARIF URIs are percent-encoded and columns are character-based
The cache salt was folded together from /dev/urandom, a RandomState-keyed hash of hard-coded tuple constants, the process id, the wall clock and the directory path. CodeQL flagged it (rust/hard-coded-cryptographic-value) and the shape was wrong: values this file spells out reached a value used as a salt, and the non-unix path had nothing but them behind it. The operating system is now the only source. generate_salt reads SALT_LEN bytes from /dev/urandom and returns an Option: no device, a short read, or a platform without one means there is no salt at all. No salt is written, no entry authenticates, every lookup is a miss and the scan runs cold. A cold scan is a correct scan; a guessable salt is a forgeable tag, so that is the direction to fail in. Drops os_random_bytes along with the pid, clock, counter and path padding a fully random salt never needed, and builds both generate_salt and unhex by accumulating bytes rather than overwriting a zeroed array.
The salt came from /dev/urandom, which exists on unix and nowhere else, so Windows - a shipped release target - could never write one and its cache was permanently cold. Take the bytes from getrandom instead: it asks each target for the random source that target has, and std exposes no OS random API on stable, so this is a dependency or it is nothing. The fail-closed contract is unchanged. A source that errors is None, and None is no salt, no write, and a miss on every entry. The buffer is uninitialized rather than zeroed and the salt is built from what the call returns, so no value spelled in this file can reach an authentication tag. The platform provenance checks are untouched: an owner-only mode on unix, an alternate data stream on Windows, nothing trusted anywhere else. The Windows branch is reachable again now that a salt can exist there.
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
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.
Closes the confirmed blocker and twelve concerns from the v1.2.0 production gate review, plus three blockers and nine minors the adversarial re-review found in the fixes themselves. Cut as 1.3.0: new exit-2 conditions and a new public field are not patch-safe.
The blocker
A repository could commit a poisoned .siloscan/cache and a fresh clone would scan clean over a live credential - exit 0, no warning, nothing. Content-hash keying does not help, because the attacker's entry key legitimately matches the file. Cache entries are now authenticated per cache directory: a salt bound to the cache location, an authentication tag over the whole stored entry, and a mismatched or foreign tag is a miss (a real scan), never a trusted zero. Reproduced in its strongest form during verification - forging a valid tag under the attacker's own salt, proven to suppress the finding in their checkout - and defeated in a fresh clone by location binding alone.
On Windows the salt lives in an NTFS alternate data stream, which no archive or git checkout carries; unsupported platforms trust no salt and scan cold. The Windows runtime path is type-checked cross-target but not executed here - failure direction is a cold cache, never a false clean.
Trust boundary
Saying where it did not look
Interop and robustness
Verification