fix: stabilization pass — twelve correctness fixes across core, fetch, cli and report - #99
Open
justin13888 wants to merge 33 commits into
Open
fix: stabilization pass — twelve correctness fixes across core, fetch, cli and report#99justin13888 wants to merge 33 commits into
justin13888 wants to merge 33 commits into
Conversation
A truncated manifest carried the cursor one byte past the end — `parse_object`
and `parse_array` advanced on `None` — and the next `skip_trivia` sliced
`bytes[len + 1..]`. `printf '{"dependencies":{'` aborted the process. A stray
closer inside the other kind of container reached `skip_scalar`, which breaks on
`}` and `]` without advancing, so `[}` spun forever. Both shapes arrive from a
half-written editor buffer.
The scan now stops at end of input without stepping past it, hands a mismatched
closer back to the frame that owns it, and guarantees each loop iteration
advances. `parse_string` keeps its cursor on a character boundary, so a
backslash before a multi-byte character no longer slices mid-UTF-8.
`unescape` decodes `\uXXXX`, including surrogate pairs: a generated manifest
writes `@scope\/pkg` and `@scope/pkg`, and the old pass yielded
`u0040scope/pkg`, which matches no dependency.
Escapes also mean the decoded value and the source span disagree byte for byte,
and both JS parsers add an offset found in the decoded value to a source offset.
The scanner now reports whether a string was escaped and those parsers withhold
the rewrite span when it was, so `--fix` cannot splice at a drifted offset.
`is_rewritable` checks the span width directly rather than inferring it from the
constraint, which is what its own doc comment already claimed.
…nt ones Six defects on the path that decides whether a dependency is current. An unparseable constraint became `UpdateAvailable`. npm-native ranges — `^1 || ^2`, `>=1.0.0 <2.0.0`, a `next` dist-tag — reach the Rust `semver` crate untranslated and all of them came back as "a newer version is waiting for you". A requirement nobody could read is now an error. An absent requirement still means `*`, which `VersionReq` rejects and so has to be spelled out, or a bare `numpy` would regress into that same error. `is_prerelease` matched substrings against a fixed marker list and was wrong in both directions: `1.0.0-M1` and `1.0.0-unstable.3` carry no listed marker and read as stable, while `1.2.3+build-rc` is a stable release whose build metadata reads `-rc` and was hidden. A version that parses as semver now answers for itself; the marker list stays for the versions that are not semver. PEP 440 orders `1.0 < 1.0.post1`, but a post-release was translated to the pre-release identifier `1.0.0-post.1`, which semver sorts *below* `1.0.0`, and `.post` was in the pre-release marker list on top of that. A project on `1.0` was told it was current and the post-release was filtered out. Post segments become build metadata, which sorts equal rather than below — the closest semver offers. A Hex constraint using `and`, or anything else `convert_clause` could not read, collapsed to the empty string, which `VersionReq` reads as `*`: a constraint that failed to translate matched every version and was always up to date. `and` is now semver's comma, and a failed translation is returned unchanged so it fails to parse and is reported. A union keeps the clause with the highest lower bound rather than whichever was written last — Hex does not require ascending order. Poetry's `^`/`~` handed their operand through verbatim while every other operator normalized theirs, so `^1.0.post1` produced a requirement that does not parse. Under semver's 0.x rules the leftmost non-zero component is the breaking axis, so `0.0.3 -> 0.0.4` was labelled a patch. On `0.0.z` nothing is. `+ 1` on a version component parsed straight from a manifest panicked in debug and wrapped in release; both bumps saturate. Three tests encoded these bugs as their expected values and are rewritten.
…verflow `Walker::visit` enumerates simple *paths*, not nodes. Cycles were already cut, but nothing bounded the number of distinct acyclic paths, and `tree --no-dedupe` sets `dedupe: false` with no depth limit. A ladder-shaped graph of n layers has 2^n of them, so a real lockfile never finished — and every appearance appends a node, so memory grew with it. The walk also recursed once per edge on the main thread, so a long enough chain overflowed the stack and aborted. A walk now carries an appearance budget (a million by default, far above any real forest) and a hard depth ceiling independent of `max_depth`. Both guards sit above `visitor.enter` so a stopped walk never leaves an `enter` without its `leave`. Stopping early is only safe if it is visible: `walk` returns `WalkStats`, `Tree` carries `truncated`, and the ASCII renderer says so and names the flags that narrow the tree. A truncated forest that stays quiet is a wrong answer wearing a complete one's clothes. `deps_of` indexed its edge table directly. It is public API on a crate whose stated audience is other tools holding indices from elsewhere, so an unknown index now yields an empty slice instead of a panic.
…ping Every gap here reported *nothing* rather than an error, which is the shape that gets trusted: a manifest with dependencies in it came back clean. `CargoTomlParser` visited three tables and never `[target.<predicate>.*]`, so a manifest whose only dependency sits under `[target.'cfg(unix)'.dependencies]` printed "0 dependencies — nothing to check". A correct reader for those tables already existed in `cargo_package.rs` and nothing called it; the parser that ships now collects them, with their own spans so `--fix` edits the right line. `package.json` skipped `overrides`, `resolutions`, and `pnpm.overrides` — the exact mechanism used to pin a vulnerable transitive dependency to a patched version. The pin was invisible, so a stale one could never be reported. Override keys carry paths and globs (`**/lodash`, `parent/child`) and may name a scope, so the package is the last segment, or the last two when the one before it is a scope; a nested `"."` key names the parent, not a new package. They get their own `DependencyKind::Override` rather than counting as direct dependencies of the manifest, which would inflate the inventory with packages it never asked for. `pyproject.toml` skipped PEP 518 `[build-system].requires` — Cargo's `[build-dependencies]` are read and these are the same thing — along with uv and PDM dev groups and pixi's tables. Poetry's multiple-constraint array-of-tables form fell through every branch and vanished; the first entry declaring a version now stands for the dependency, since markers are not evaluated here. PEP 508 permits the specifier in parentheses (`flask (>=2.0)`), the form PEP 621 metadata round-trips into. The parens ended up inside the constraint, which then failed to parse. npm records a workspace member twice — a versionless `node_modules/<name>` stub whose `resolved` is the member's path, and the member itself. Edges stopped at the stub, which declares no dependencies, so the member's whole subtree vanished; and because the stub has no version the edge was a bare name, which resolved to whichever candidate came first in document order — the stub, every time, since npm writes `node_modules/*` before `packages/*`. Edges now follow the link, and a stub is no longer classified as a registry install. A lockfileVersion 1 document keeps its graph under a tree this parser does not read, and returning an empty graph made "unsupported format" identical to "no dependencies". It is now an error, and `build_workspace_graph` degrades an unreadable lockfile to `UnreadableLockfile` over the manifest-derived graph rather than failing the command.
Three defects that all end the same way: a version list attributed to a package it does not belong to, reported with full confidence. The disk cache defaulted to *on*, pointed at the shared OS cache directory. Merely constructing a `Checker` therefore gave it write access to a location every other run on the machine reads. That is how this repository's own test suite poisoned real caches: several tests in `tests/checker.rs` build a checker against a mock registry without naming a cache directory, so their fabricated version lists were written to `~/.cache/dependable` under the real package names. A subsequent `dependable check` then reported `time 0.3.55` as up to date with a latest of `0.2.7` — the version this crate's own tests cite for RUSTSEC-2020-0071 — and offered `serde 1.2.0`, which does not exist. `--fix` writes `latest_available` into the manifest, so it would have written that version into a real `Cargo.toml`. The `pre-push` hook runs the suite, so every push re-poisoned the cache. The cache is now opted into. `disk_cache` is tri-state so an explicit choice wins regardless of builder order, and naming a directory opts in on its own — a caller that chose an isolated location means to use it. Nothing reaches the shared root without being asked. With `XDG_CACHE_HOME` pointed at a pristine directory, the suite now writes nothing there; it used to write eight entries. The disk-cache key named only the ecosystem, so a run against a private index or a mirror shared entries with the public registry. The entry's stored-name guard cannot catch this: the name matches, only the registry differs. Fetchers report their root and a non-default one is hashed into the key, leaving entries for default registries valid. The alternate-registry key loses its `::` separator, which is not a legal character in a Windows path component and made that key a directory name the cache could never create. `FetchedMap` was keyed by bare package name while the fetch tasks were deduplicated by `(cache_key, name)`. Two same-named packages from different registries in one manifest — `jsr:foo` and `npm:foo`, or a crate published to both crates.io and a private index — collapsed into one slot, and because the requests complete out of order, whichever finished last answered for both.
`dependable check --fail-on vulnerable` with the network down printed 25 fetch errors and exited 0. The scan failure was pushed onto `ManifestCheck::warnings`, the runner printed those warnings and then dropped them — `ManifestReport` had no field to carry them — so `exit_code` saw only per-result statuses, none of which were `Vulnerable`, because nothing had been asked. A CI gate that reports success on the run it could not perform is worse than no gate: it converts an outage into a green build. `ManifestCheck` now carries a typed `vulnerability_scan_failed` rather than only prose in `warnings`, because a caller has to act on it, not parse it. The CLI turns that plus a count of unresolved dependencies into `ScanIntegrity`, and a gate that cannot be answered from what the run established exits 2 naming the reason, rather than 0. The scope is the gate, not the tool: with no `--fail-on` the run still exits 0 and reports what it found, because nothing was promised. `--fail-on any` already fails on `DependencyStatus::Error`, so unresolved dependencies are not counted as a hole there — that is the gate working. `Vulnerable` and `Outdated` match specific statuses and skip errors entirely, which is where the hole was. `[policy]` had the same shape one level up. `check_policy_is_enforceable` proved the gate *could* run by inspecting the config, and the doc comment on `requires_cvss` claimed that made the gate non-vacuous — but a CVSS rule reads advisory lists, and a scan that never ran leaves those empty, which is exactly what "no advisories" looks like. A policy gating on severity now refuses to pass when the scan did not complete.
The CVSS gate skipped any result whose `advisories` list was empty. But `advisories` is *enrichment* — opt-in, and it degrades to a warning on failure — while `current_vulnerabilities` is the finding. A dependency that is known vulnerable, with enrichment off or failed, therefore passed `max_cvss` and `fail_on_severity` in complete silence: the exact package the gate exists to catch. `sarif.rs` already had the polarity right, treating `current_vulnerabilities` as authoritative and `advisories` as decoration. The gate now does too, and routes an unscored-but-known-vulnerable dependency through `unrated_advisories`, which is what that knob is for. Withdrawn advisories were scored and could fail a build. A retracted advisory is not a finding; the summary counts them separately and the HTML report flags them, and only the gate that fails the build was blind to it. They are filtered before scoring, and a live advisory beside a withdrawn one still fails. `major_distance` measured `0.1 -> 0.9` as 8 and `0.1 -> 1.0` as 1, because it counted 0.x on the minor axis and everything else on the major axis without reconciling them. Being further behind measured as being closer, so the moment upstream shipped `1.0` a project that had been failing `max_major_behind = 2` began to pass. Crossing out of 0.x now counts the crossing plus each major after it. The 0.x releases skipped on the way out are still not counted — only the two endpoints are available here, not the release list — so the measure is a lower bound once upstream crosses 1.0, and says so. A CVSS v4-only advisory carries no score (`cvss.rs` scores v3.0 and v3.1) and no band, so it is unrated and the default `unrated_advisories = "warn"` lets it pass a `max_cvss` gate. Scoring v4 needs the macro-vector lookup tables and changing the default is a product decision, so neither is done here; the behaviour is pinned by a test naming both, so whichever lands is a deliberate edit rather than silent drift, and `unrated_advisories = "fail"` closes the gap today.
`uri_for` fell through to `strip_prefix`'s error case for any manifest outside the report root and emitted the path verbatim. On Unix that is a bare path-absolute string, which GitHub code scanning rejects in a log carrying no `uriBaseId` — and this log deliberately omits one, because it would embed the developer's absolute path. On Windows it was worse: `encode_uri` percent-encodes every byte outside the URI-safe set, so the drive prefix became `C%3A/Users/...`, which names nothing at all. An absolute path outside the root now becomes a `file:` URI, where a drive prefix is legal and keeps its colon while the segments stay encoded — a space in a directory name is still a space. A path under the root is unchanged: relative, `/`-joined, encoded. A *relative* path outside the root also stays as it is; turning that into a `file:` URI would invent a base it never had. The existing test pinned `/elsewhere/Cargo.toml` as expected output and is rewritten. The drive-prefix case is Windows-only — elsewhere a backslash is an ordinary character and `C:\...` is a single relative component — so it is gated to the platform, where the CI matrix already runs the suite.
`load_config` ended in `.unwrap_or_default()`, so a file that was present but did not fit the schema became `Config::default()` — with `[global] fail_on` reset to `none`. One wrong-typed value anywhere in `.dependable.toml` therefore disarmed the CI gate and the run exited 0 with nothing on stderr. It is now an error, named by path. None of the config tables rejected unknown keys either, so `fail-on` with a hyphen was parsed, dropped, and left the gate off while looking configured. `[policy]` in the same file has always rejected its own typos and the CLI test suite asserted that the rest stayed lenient — that leniency was the hole, not a smaller version of the same safety, so the test asserting it is rewritten. The error names the offending key and the ones that would have worked. `--fail-on` could not override a config value. The guard was `args.fail_on != FailOn::None`, and `FailOn::None` is also clap's default, so an explicit `--fail-on none` was indistinguishable from the flag being absent: a config saying `fail_on = "any"` could not be turned off from the command line, in direct contradiction of the documented CLI-over-config precedence. The flag is an `Option`, which is how the neighbouring `--unstable` already got this right. `--include-ghsa` keeps OR-ing its layers, and now says why: it is a flag, so absence cannot be told from `false`, and it can only widen the scan. `--ecosystem` was accepted, advertised in `--help` as restricting the run, and read by nothing. Its help text also claimed V1 checks only Rust, which has not been true for nine ecosystems. Removed rather than left as a flag that looks like a filter and filters nothing. `--no-lock-file` said "Ignore `Cargo.lock`" across six lockfile formats.
`fix` is the only command that writes to the user's files and had the least defence around it — and no end-to-end test asserting the bytes it produces, only unit tests of the pure planner. It applied byte spans computed during the check to content read again afterwards, guarded only by a bounds test. That proves the span is inside the file, not that it still holds the constraint it was planned against. Anything that touched the manifest in between — an editor auto-save, a `cargo add` in another shell, a concurrent `fix` — shifted every later offset, and the splice landed on whatever now occupied them. Each edit now records the text it expects and the write is refused, with a message naming the line and telling the user to re-run, rather than corrupting the file and reporting success. A span running past its line is refused too; it used to be dropped and counted as applied. `fs::write` truncates before writing, so an interrupted write — a full disk, a crash, a Ctrl-C — left a manifest empty or half-written with no backup. The new contents go to a temporary file in the manifest's own directory, are flushed, and are renamed over the original, which is atomic. A read-only manifest now fails without having destroyed anything; `tempfile` moves from dev-dependencies to dependencies for it, and the original file's mode is preserved. A multi-manifest run wrote each manifest as it went and aborted on the first failure, leaving the tree half-rewritten — and since the report was printed after each write, the failing iteration also lost the record of what had already changed. Every manifest is planned before any is written. `fix` hardcoded `check_vuln: false`, so the `Vulnerable` arm in `plan_fixes` was unreachable and a vulnerable-but-current dependency could never be upgraded — though the docs sell `fix` as the remediation half of the tool. It also hardcoded `cache: true` with no way to bypass it, deciding what to write into a manifest from an hour-old cache. Both are now settings, with `--no-vuln` and `--no-cache` matching `check`. A new `tests/cli_fix.rs` covers the write path end to end: an unchanged manifest stays byte-identical, comments and formatting survive, `--dry-run` does not touch the file or its mtime, an unparseable manifest is left alone, no temporary file is left behind, and a read-only manifest is not truncated.
…package There was no retry, no backoff, no rate-limit handling and no connect timeout — one blanket 10-second request timeout, with `concurrency` defaulting to 20 against registries that rate-limit. A large monorepo reliably provokes 429s, and every failure was terminal. The cost was not just an unresolved dependency. A rate-limited package became `DependencyStatus::Error`, and `osv_query_for` excluded errored results from the vulnerability scan outright — even though the lockfile had already named the version and OSV needs nothing from the registry. A transient 429 therefore left a dependency silently *unaudited*, which `--fail-on vulnerable` then ignored. An errored result whose version is known from the lockfile is now scanned. `latest_compatible` is deliberately not a fallback there: an errored fetch has no version list behind it. Transient failures — 429, 5xx, timeouts, refused connections — are retried three times with exponential backoff, shared by the registry fetches and the OSV batch. A 404 is an answer and is not retried. The backoff is fixed rather than driven by `Retry-After`; the header is not carried on the error, which is noted where it matters. A connect timeout is separate from the total timeout so a black-holed host cannot spend the whole budget on a handshake. A short `querybatch` response left the unanswered slots empty — recorded as "no vulnerabilities" *and cached as clean* for ten minutes, so not even a retry in the same process could recover. One result per query is the API's contract, and a body that breaks it is an error. A malformed line in the sparse index was dropped. A wholly broken body was already caught downstream, but a *partially* corrupt one produced a plausible short version list — and if the newest release was among the dropped lines, the dependency reported up to date. Blank lines are still not corruption.
The instruction file still opened with "V1 scope is Rust / Crates.io only" and described four crates. There are five and ten ecosystems, and the missing crate — `dependable-report` — is the one holding the policy engine, SARIF, and HTML. `README.md` listed three crates and said CI runs on `main`; it runs on `master`. `SCOPE.md` called `dependable-report` a scaffold that "renders nothing yet and ships no user-visible command" while `report` is a default-on subcommand with end-to-end tests, and listed six items under "future work" that had all shipped — the persistent disk cache, alternate registries, `.npmrc` auth, `--fix` for JSON/YAML, Windows support, and `latest` resolution. `INTEGRATIONS.md` marked SARIF "Roadmap — V2" beside 1,268 lines implementing it. `README.md` and four places in the composite action's docs told users to pin `@v0.1.4`, a tag that does not exist — the docs were bumped ahead of the release. They now name `v0.1.3`, which does. Also removes the `dependable-report` binary. The crate is published, the binary was on by default, and its entire body printed "report rendering is not implemented yet" and exited 2 — so `cargo install dependable-report` handed the user a command that could only fail. The crate is a library; the reporting the CLI does goes through it as one. `release-plz.toml` named four crates in publish order and the real order is five, with `dependable-report` before `dependable`, which depends on it by version. Adds the exit-code table README never had, including the case this branch introduced: a gate that cannot be answered exits 2 rather than 0. And documents that `.dependable.toml` is now validated rather than silently falling back. Adds a scheduled `Live` workflow. Twenty `#[ignore]`d network tests existed and no workflow ran `mise run test:live`, so the whole registry and OSV surface — also the least-covered code in the workspace — was never exercised against a real API, and a registry changing a response shape would surface as a user's wrong answer. It is deliberately not on the PR gate: those tests fail for reasons a contributor cannot fix, and a gate that cries wolf gets ignored on the day it is right. Nine of them asserted only that a list was non-empty and now check the shape too.
Merge origin/master into the stabilization branch, keeping both the twelve correctness fixes on this branch and everything master shipped in the meantime (the Maven/Gradle ecosystem, and the wildcard guard in `fix`). Five conflict hunks across four files, plus two silent breaks the merge did not mark: - `semver/checker.rs` — both sides only added tests; both kept. The branch's comment claiming `1.x` is unparseable was wrong: the `semver` crate parses `x`/`X` wildcards, so wildcards stay real evaluations and reach the fix layer, which is exactly what master's reachability witness asserts. - `semver/normalize.rs` — the branch decides `is_prerelease` by parsing semver, master decides the JVM with Maven's tokenizer. Both kept, with the semver reading skipped for the JVM: `32.1.3-android` is semver with a pre-release segment but a build variant of a release under Maven's order. - `config.rs` — master's `JvmConfig` kept, with the branch's `deny_unknown_fields` applied to it and to `VulnConfig`, so no ecosystem block is exempt from validation. - `fix.rs` — both sides only added tests; both kept, with master's two adapted to the branch's fallible `plan_fixes`. - `check.rs` — master's `evaluated` test helper keyed the fetched map by name alone; rekeyed to the `(cache_key, name)` pair the branch introduced. - `runner.rs` — master's `run_list` used the infallible `load_config`; now propagates, like every other command, so an unreadable config cannot silently re-enable every ecosystem.
`uri_for` asked `Path::is_absolute`, which on Windows means "rooted *and* carrying a drive". `/elsewhere/Cargo.toml` is rooted with no drive, so the same manifest rendered as `file:///elsewhere/Cargo.toml` on Unix and as the bare path-absolute string `elsewhere/Cargo.toml` on Windows — the unresolvable form this fix exists to stop emitting. A rooted path is exactly as unresolvable without a base on one platform as on the other, and a SARIF log must not describe one manifest two ways depending on the machine that rendered it, so the question is now `Path::has_root`. On Unix the two are the same predicate and nothing changes; on Windows it adds only the rooted-but-drive-less case. A drive-relative path (`C:foo`) is rooted by neither and still stays relative, which is correct: it resolves against that drive's working directory.
This was referenced Sep 1, 2026
pnpm scopes an override to the parent that pulls the package in by joining the two with `>`: `"foo@2>bar"` forces a version onto `bar`. `override_name` split only on `/` and stripped a trailing `@…`, so the key resolved to `foo` — an unrelated package. The entry was then checked against `foo`'s version list, and `fix --all` offered to rewrite the pin on `bar` to `foo`'s newest release. The overridden package is the last `>`-separated segment; the selectors in front of it only say which parent the override applies to.
An `overrides`/`resolutions` entry is a version this manifest deliberately forces onto the resolved tree, very often a security pin holding a transitive dependency above a vulnerable release. `plan_fixes` treated it as an ordinary declaration, so `fix --all` rewrote it to the newest release and undid the pin. Checking an override and reporting that a newer version exists stays; rewriting it does not. The whole kind is declined rather than guessing which pins are safe to move.
`check_version` receives an already-translated constraint, and three of the four translators signal failure by returning the empty string: Maven and NuGet for an untranslatable version or a malformed interval, PEP 440 once every clause has been dropped. The checker's arm for an empty constraint reads that as "the author declared no range" and evaluates `*`, so `latest_compatible` became the newest release and the status became `up to date` — the most confident answer available for a requirement that was never understood, and one that disarms `--fail-on outdated` at every ecosystem that translates. `try_to_semver_constraint` tells the two apart by what went in: an empty result from a non-empty input is a failed translation and nothing else produces one. `check_version_for` translates and classifies together, so the distinction cannot be lost between the two calls, and reports the new `DependencyStatus::Undetermined` — a real package whose declared version this run could not read. It claims nothing about currency, is counted and rendered as itself, and `fix` never rewrites it.
`"overrides": { "semver": "$semver" }` is npm's documented way to force the
version this manifest already depends on. Nothing filtered it, so it reached the
version checker verbatim and came back `unparseable constraint: unexpected
character '$'` — a hard error on a valid manifest, which then took the
`--fail-on` gate with it.
The reference now resolves the way `csproj.rs` already handles `$(MSBuildProp)`:
where the named direct dependency exists, the override is checked against that
dependency's declared constraint, and its recorded span reports its position but
declines its width so nothing writes a version over the reference. Where the
manifest declares no such dependency, the entry is `PackageSource::Unresolved` —
a real package whose intended version cannot be read — reported as
`Undetermined` rather than as a parse error.
`gate_is_answerable` treated any `DependencyStatus::Error` as "the gate could not be answered", but `Error` also covers a permanent 404: a private or internal package, one served by a registry this run does not route to, a deleted package. One such dependency turned every `--fail-on` setting into exit 2, under a message that reads as transient, with no escape short of dropping the gate — and the shipped Action defaults to `--fail-on vulnerable`, so every repository with one unpublished internal package went from a passing step to a hard failure. The gate now asks about the run rather than about a dependency. `ManifestCheck` carries `registry_unreachable`, set when a lookup failed for any reason other than the package not existing — a timeout, a refused connection, a 5xx, an undecodable response — and that, with a vulnerability scan that did not complete, is what leaves a gate unanswerable. `--fail-on any` stays answerable through an unreachable registry, because it fails on the `Error` statuses that registry produces; the other settings skip errors and so still refuse to certify. A 404 stays visible in the table and in `--format json`, and a run with a gate armed now says on stderr how many dependencies were not found and were therefore not gated on.
`Config` carries `deny_unknown_fields`, so declaring `policy` only under the `report` feature made the absence of the field a rejection: a `--no-default-features` build exited 2 with "unknown field: found `policy`" on any config carrying a policy block, where it used to warn and run ungated. That left `warn_policy_ignored` and `has_policy_table` unreachable for the only case they exist for — the load they needed to survive failed first. The key is now declared in every build, typed as the policy schema only where the feature can read it and as an unread table otherwise. The feature gates what is done with the block, not whether it is a known key. `cli_policy` and `cli_sarif` assert enforcement and SARIF rendering, both of which the `report` feature builds, so they state that requirement instead of failing a `--no-default-features` run for the absence of code they never compiled. That a `[policy]` block still loads without the feature is asserted in `config::schema_tests`, which runs in both builds. The warning those builds print also loses the run of spaces a line continuation had left in it.
`MavenCentralFetcher` was the one `RegistryFetcher` that never overrode `registry_root`, because it landed on master after cache scoping was written and the merge had nothing to conflict with. It has a configurable `base_url` driven by `[jvm] registry`, so a run against `https://nexus.corp/...` with `--cache` wrote `com.google.guava:guava` under the bare `Maven` key, and a later default-registry run was served the mirror's version list — with `--fix` then splicing an internal-only version into the manifest. The name guard cannot catch it: the name matches. The test drives all nine default fetchers rather than the one that was missing, so a tenth implementation cannot repeat it, and asserts that a default registry still keeps the bare key so existing cache entries stay valid.
`flatten` discarded `Tree::truncated`, so only the ASCII renderer said a walk had run out of budget. `tree --no-dedupe --format json` on a graph that hits the appearance budget emitted a document byte-indistinguishable from a complete one — and the machine formats are exactly the ones whose consumer cannot eyeball the difference. JSON gains a top-level `truncated` boolean, always present so a consumer can require it rather than infer completeness from its absence; DOT gains a comment and a `dependable_truncated=true` graph attribute, so a tool reading the file sees it too. Both are additive. `DEFAULT_MAX_VISITS` and `MAX_WALK_DEPTH` keep their values and now record where each number comes from — an appearance budget bounded above by the largest real forests and below by what still looks like a bounded operation, and a stack budget rather than a graph property — including why neither is user-tunable.
…elling `absolute_file_uri` inserted a Windows path prefix unencoded after replacing backslashes, which is right only for a drive letter. A UNC path `\\server\share\repo\Cargo.toml` became `file://///server/share/...` rather than `file://server/share/...`, and the verbatim form `\\?\C:\repo\Cargo.toml` — which `std::fs::canonicalize` returns and `discover.rs` deliberately preserves — became `file:////?/C:/repo/Cargo.toml`, where the unencoded `?` opens a URI query and truncates the path at `file:////`. Separately, `join_components` dropped the prefix of a drive-relative path, so `C:crates\app\Cargo.toml` rendered as `crates/app/Cargo.toml` — the URI a path on any other drive produces. Each prefix form now has an explicit spelling: a drive (plain or verbatim) is a path segment whose colon survives, a UNC share (plain or verbatim) is an authority plus a first segment, and the verbatim and device namespaces are encoded segments. A drive-relative path keeps its drive as an encoded segment. The Windows test's drive-relative assertion pinned the dropped prefix as the expected result; it is corrected to the URI that preserves it, which restores information the renderer was losing rather than relaxing the check. The prefix decisions are also asserted directly, on every platform: `Path` parses a prefix only on Windows — which is how these forms went unnoticed — while `Prefix` itself is spellable everywhere.
This was referenced Sep 1, 2026
The Go module proxy protocol names both `404` and `410` as the not-found responses, and `410 Gone` is `proxy.golang.org`'s canonical answer for a module it will not serve — the private or internal path a repository excludes with `GOPRIVATE`. The `@v/list` handler treated only `404` that way, so a `410` became `FetchError::Status`, which marks the whole registry unreachable, which in turn leaves a `--fail-on` gate unanswerable. One private module therefore took a Go repository from a passing run to exit 2 under the shipped Action's default `--fail-on vulnerable` — the identical CI break the 404 carve-out was written to eliminate, unrepaired for Go. Both the list handler and the `@latest` fallback now read either status as absent. Falsified end to end by a new `cli_gate.rs`, which drives the real binary against a throwaway HTTP registry on loopback. The suite had no way to make a registry answer at all — the existing hermetic tests declare path dependencies so no fetch is ever built — which is why a defect about status codes could not be caught.
A `maven-metadata.xml` that parsed but carried no `<version>` elements returned `FetchError::NotFound`. That spelling is now load-bearing: a 404 is a per-dependency carve-out from the `--fail-on` gate, so an answered-but-empty document was silently exempted and the build was certified against a dependency nothing was ever established about. A 200 with no versions is not an authoritative "this artifact does not exist". A Nexus or Artifactory group repository whose upstream proxy is down serves exactly such a locally-merged document for an artifact that certainly does exist. It now has its own `FetchError::EmptyVersionList`, which leaves the registry unanswered rather than the package absent — non-transient, because the same document parses the same way next time. Corrects the unit test that pinned the opposite, and adds a CLI-level test over the loopback registry showing the run exit 2 with "the registry did not answer" rather than passing with a not-found note.
pnpm and Yarn both allow a version range in an override or resolution key, and a range can contain `>`. Splitting on every `>` read that comparator as pnpm's parent separator and cut the key inside its own range, so `"lodash@>=1.0.0"` named a package called `=1.0.0`, `"foo@>1.0.0"` named `1.0.0`, and `"a@>b"` named `b`. Each was then asked of the registry, which has none of them — and with a 404 no longer failing the gate, the run said nothing about it at all. The split is now bounded: a `>` separates a parent from the package it scopes only when it follows something that can end a name or a version. A comparator opens a clause, so it follows what opens one — the `@` that introduces the range, a clause delimiter, or another operator character. `"quux@1>bar@^2.1.0"` and `"@scope/pkg@1>@scope/other"` keep working, because a digit and a letter are not those. Covered at the helper, through the parser, and end to end against the loopback registry.
`"$"` and `"$a b"` are `$`-prefixed override values that name no usable dependency, and the reference guard rejected them as malformed — so they fell through to the version checker as literal constraints and hard-failed with `unparseable constraint: unexpected character '$'`, which is precisely the failure the reference form was added to prevent. Every `$`-prefixed override value is now read as a reference. One that resolves adopts the referenced dependency's constraint as before; one that does not — whether it names an undeclared dependency, names nothing, or is not a name at all — becomes `PackageSource::Unresolved` and is reported as `undetermined`, with nothing asked of any registry.
`*` is PEP 440's and Poetry's explicit "any version", and the most common way to write an unpinned dependency. It matches no operator and holds no numeric release, so every clause was dropped and the translation came back empty — and once an empty translation from a non-empty input meant "this constraint could not be read", `requests = "*"` was reported `undetermined` rather than `up to date`. That both excluded a real dependency from `--fail-on outdated` and flipped `--fail-on any` from pass to fail for any Poetry project with an unpinned dependency. A bare `*` clause now translates to `*`, as the NuGet and Maven translators already do for their own wildcards. Re-audited the other translators for constraints that legitimately translate to nothing, and pinned the result both ways: every ecosystem's "any version" spelling must translate, and the forms semver genuinely cannot express — a `!=` exclusion, an MSBuild property, Maven's `LATEST`/`RELEASE` — must keep coming back as failed translations.
`ScanIntegrity.unresolved` counted any `DependencyStatus::Error`, and the 404 carve-out then exempted that whole set from the `--fail-on` gate. But `Error` also covers a failure no registry ever produced: an unparseable constraint is rejected before anything is fetched. So a manifest declaring `"lodash": "^^^bogus"` passed `--fail-on vulnerable` with exit 0, under a note saying the dependency had not been found in its registry — a registry that was never asked. Two published, resolvable dependencies went unevaluated and the build was certified anyway. The provenance is now carried rather than re-derived from the message. `FetchError` already knows whether the registry answered "no such package", and each `CheckResult` records that as an `ErrorOrigin`: `NotFound` for the registry's own answer, `Unanswered` for a request that produced none, `Local` for a failure this run reached by itself. `CheckResult::new` records `Local` for an `Error`, because "we did not record where this came from" is not evidence a registry answered. `ScanIntegrity` splits accordingly. A 404 stays exempt and stays reported; a local evaluation failure makes the gate unanswerable exactly as it did before the carve-out existed, and the gate now names every reason it could not be honoured rather than a hand-written combination per pair. Corrects the round-one test that asserted the carve-out against a hand-written error *string*, which is why it could not catch this: it now asserts the provenance, and a CLI-level test over the loopback registry covers both halves.
`Undetermined` was made an honest status and then gated on by nothing and noted by nothing: a run that could not read two constraints printed a clean `--fail-on outdated`, said nothing on stderr, and read as "everything here is current" when two dependencies had never been evaluated at all. `check` now says on stderr how many dependencies it could not read a version out of, mirroring the not-found note exactly — silent for `--fail-on none` (nothing was gated on) and for `--fail-on any` (which already fails on the status). The status is deliberately *not* added to `--fail-on outdated`: that would change what the setting promises, which is a policy decision and not this repair. Both notes now respect `--quiet`, whose help says "Only print errors". A note about what was skipped is not an error, and the not-found note printed through it.
The message carried fourteen literal spaces mid-sentence — a source line wrapped inside a string literal, the same mistake already repaired in `warn_policy_ignored`. The advice it gives is the user's only way out of the error, so it is the message that can least afford to look broken.
`PackageSource::Unresolved` fell through to `"unknown"` in `list --format json`, so a consumer could not tell an npm `$name` override naming a dependency the manifest never declares from any other source the tool has no name for. It is a real, published package whose version this manifest does not state — a distinct fact, worth a distinct token.
`level_of` returned `None` for `Undetermined`, so it produced no GitHub Actions annotation at all while a plain `Error` at least got a notice. Both mean "this dependency was not checked", and the one that says the tool could not read the declared version is the one a pull request most needs to hear about.
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.
Twelve unshipped correctness fixes, brought up to date with
master(including theMaven/Gradle ecosystem and the
#87wildcard guard) by a merge, so the fixes keeptheir individual history and rationale.
The twelve fixes
bdd6c45fix(core)JSON scanunescapedid not decode\uXXXX, and escaped strings had their decoded offsets spliced against source spans.printf '{"dependencies":{'aborted the process,[}hung,@scope\/pkgresolved to nothing, and--fixcould splice at a drifted offset.1b13d61fix(core)version comparisonUpdateAvailable, substring-basedis_prerelease, PEP 440 post-releases sorting below their base, Hexandcollapsing to*, Poetry^/~skipping normalization,0.0.zbumps labelled patches, and a wrapping component bump.cc9df2bfix(core)bounded graph walkWalker::visitenumerated simple paths with no budget and recursed per edge.tree --no-dedupeon a ladder-shaped lockfile never finished, grew memory with it, and a long chain overflowed the stack. A truncated walk is now reported, not silently short.1a05147feat(core)missing dependency tables[target.*.dependencies], npmoverrides/resolutions/pnpm.overrides, PEP 518[build-system].requires, uv/PDM/pixi groups, Poetry's array-of-tables form, PEP 508 parenthesized specifiers, npm workspace-member stubs, and lockfileVersion 1.7ad8ea2fix(fetch)cache keyingFetchedMapwas keyed by bare name while fetch tasks were keyed by(cache_key, name).--fixwould have written into a realCargo.toml. Cache is now opt-in and correctly keyed.61dc6e2fix(cli)unanswerable gateexit_codesaw only per-result statuses.check --fail-on vulnerablewith the network down exited0. A gate that reports success on the run it could not perform converts an outage into a green build. It now exits2naming the reason.58b9ce4fix(report)policy gatesadvisorieslist (enrichment), ignoringcurrent_vulnerabilities(the finding); withdrawn advisories were scored;major_distancemeasured0.1 → 0.9as 8 and0.1 → 1.0as 1.max_cvssin silence, a retracted advisory could fail a build, and being further behind measured as being closer.0248ef8fix(report)SARIF URIsuri_foremitted paths outside the report root verbatim.uriBaseId, and on WindowsC:becameC%3A, naming nothing. Now afile:URI.b7c8ceafix(cli)safefixfs::writetruncating with no backup; multi-manifest runs writing as they went;check_vuln/cachehardcoded.--no-vuln/--no-cacheexist.45a9742fix(cli)config validationload_configended in.unwrap_or_default(); no table rejected unknown keys;--fail-oncould not override a config value;--ecosystemwas accepted and read by nothing.[global] fail_ontononeand disarmed the CI gate with nothing on stderr.--ecosystemis removed rather than left looking like a filter that filters nothing.bbd1c53fix(fetch)rate limitsErrorand was then excluded from the OSV scan; a shortquerybatchresponse was cached as clean; a malformed sparse-index line was dropped.--fail-on vulnerablethen ignored. Transient failures now retry with backoff; a 404 is an answer and is not retried.8e25de7docsSCOPE.mdcalled the shippeddependable-reporta scaffold,INTEGRATIONS.mdmarked SARIF "Roadmap — V2" beside its implementation, and the docs pinned a release tag that does not exist. Also removes thedependable-reportbinary (its whole body printed "not implemented yet" and exited 2) and adds a scheduledLiveworkflow for the twenty#[ignore]d network tests no workflow ran.How each conflict was resolved
Five hunks across four files, plus two breaks the merge did not mark.
crates/dependable-core/src/semver/checker.rs— both sides had only appended tests to the samemod tests. Both test blocks kept verbatim; nothing rejected. Separately, this branch's comment on the error arm claimed npm ranges including1.xland there. That is wrong — thesemvercrate parsesx/Xwildcards (parse.rs::wildcard) — and it contradicted master's newwildcard_constraint_is_reported_as_upgradable, the reachability witness proving thefixguard is not dead code. The comment is corrected to name only the ranges the crate has no dialect for (^1 || ^2,>=1.0.0 <2.0.0) and to say explicitly that wildcards stay real evaluations and reach the fix layer.crates/dependable-core/src/semver/normalize.rs— two hunks, one doc comment and one test block, but a real semantic collision underneath. This branch decidesis_prereleaseby parsing the version as semver and reading its pre-release segment; master decides the JVM with Maven's tokenizer and asserts!is_prerelease("32.1.3-android", Jvm)— a string that is semver with a pre-release segment but is a build variant of a release under Maven's order. Resolved by keeping both and ordering them: the semver reading is skipped when the ecosystem isJvm, and everything below it is master's code unchanged, so the JVM path is byte-for-byte the behaviour master shipped (universal marker list, thenmaven::is_prerelease) while every other ecosystem gets this branch's exact classification. Rejected: short-circuiting the JVM straight tomaven::is_prereleasebefore the universal marker list — it reads cleaner, but1.0.0-canaryon the JVM would have regressed from pre-release to release, sincecanaryis not one of Maven's qualifiers. Both doc comments were merged into one that states both exceptions and why each exists. Both test blocks kept verbatim.crates/dependable/src/config.rs— this branch turned#[serde(default)]into#[serde(default, deny_unknown_fields)]on every config struct; master addedJvmConfigwith the old attribute, and the hunk swallowedVulnConfig's attribute too. Master'sJvmConfigkept in full, withdeny_unknown_fieldsapplied to it and toVulnConfig. Rejected: taking this branch's side, which would have deletedJvmConfigoutright; and taking master's, which would have left[jvm]and[vulnerability]as the two blocks where a typo is still silently ignored — the exact hole45a9742closes.crates/dependable/src/fix.rs— both sides had only appended tests. Both blocks kept; master's two adapted to this branch's fallibleplan_fixes(.expect("the plan applies"), matching the neighbouring tests). The wildcard guard from #89 is untouched and verified present:is_wildcardand theif is_wildcard(rest) { return None; }block are inrewrite_constraint, anda_wildcard_dependency_is_left_untouched_by_fix,..._by_fix_all,rewrite_never_narrows_a_wildcard_to_a_pinandrewrite_declines_a_wildcard_wearing_a_stability_flagall pass.crates/dependable-fetch/src/check.rs(no conflict marker; a clean merge that did not compile) — master'sevaluatedtest helper inserted intoFetchedMapkeyed by bare name, which7ad8ea2changed to(cache_key, name). Rekeyed to the pair under aCACHE_KEYconstant. Test-only.crates/dependable/src/runner.rs(no conflict marker; a clean merge that did not compile) — master'srun_listcalledload_configas infallible, which45a9742made fallible. Now propagates withwith_context, exactly like the four other call sites.One fix the branch needed on top
787480dfix(report).0248ef8was never CI-tested — the branch had never beenpushed — and its two new SARIF URI assertions failed on
windows-latest.uri_forasked
Path::is_absolute, which on Windows means "rooted and carrying a drive", so/elsewhere/Cargo.tomlrendered asfile:///elsewhere/Cargo.tomlon Unix and as thebare path-absolute string on Windows — the unresolvable form the fix exists to stop
emitting. The predicate is now
Path::has_root: identical on Unix, and on Windows itadds only the rooted-but-drive-less case. A drive-relative path (
C:foo) is rooted byneither and still stays relative. No assertion was weakened; the two failing tests
pass as written, and the Windows-only test gained both boundary cases.
Judgement calls
is_prereleaseordering. The semver reading is skipped only forEcosystem::Jvm, chosen to preserve master's JVM behaviour exactly rather than to be the tidiest structure. Reverses if Maven'sPRERELEASE_QUALIFIERSgrows to cover the universal marker words, at which point the universal check becomes redundant for the JVM and the short-circuit is the better shape.deny_unknown_fieldsonJvmConfig. A[jvm]block with an unknown key is now a hard error, consistent with every other ecosystem block. Reverses if a key needs deprecating — add#[serde(alias)]rather than dropping the attribute.run_listnow fails on an unreadable config.dependable listis offline and read-only, so silently defaulting was defensible; but the file decides which ecosystems are enabled, and defaults enable all of them, so a broken config would makelistover-report. Reverses iflistis meant to stay usable with a broken config — restoreunwrap_or_defaultand print a warning.8e25de7changes five documented pins from@v0.1.4back to@v0.1.3, which reads as a regression against master. It is not:v0.1.4has no tag in this repository and the workspace version is0.1.3, so the docs were bumped ahead of a release that never happened and told users to pin something that does not resolve. Reverses at the next release — bump both together.--ecosystemremoved. A user-visible CLI flag disappears. It parsed, was advertised in--helpas restricting the run, and was read by nothing. Reverses if the filter is implemented, in which case the flag returns doing what it always claimed.Validation
Nothing from
masterwas dropped:git diff origin/master HEAD --name-statusreports exactly two deletions,crates/dependable-report/src/main.rsandcrates/dependable-report/tests/bin.rs, both from this branch's own8e25de7. The Gradle/Maven sources (parsers/gradle_catalog.rs,semver/maven.rs,registries/maven_central.rs) and thesample-kotlinfixture tree are all present.CI is green on all five checks, including
windows-latestandmacos-latest.Follow-up filed: #105 tracks the
--ecosystemremoval — both the open PRD requirement the flag was a placeholder for (now cheap to meet, sincecollect_manifestsalready takes an ecosystem predicate) and the fact that a breaking CLI change is carried by afix:commit, so release-plz will compute a patch bump for a release that changes the accepted arguments.Review repairs
An independent review of this branch built and ran the binary and confirmed two
critical and two high-severity defects, plus four lower ones. Each is reproduced
below as it behaved on the branch, then as it behaves now. Every repair carries a
test that fails without it.
1. CRITICAL — an unresolvable dependency turned every
--fail-ongate into exit 2gate_is_answerabletreated anyDependencyStatus::Erroras "the gate could not beanswered".
Erroralso covers a permanent 404: a private or internal package, oneserved by a registry this run does not route to, a deleted package.
Before, on a
package.jsonnaming one unpublished internal package beside a public one:The shipped Action defaults to
--fail-on vulnerable, so every consumer whoserepository contains one unpublished internal package went from a passing step to a
hard failure, under a message that reads as transient for a permanent condition and
with no escape short of dropping the gate.
Repair. The gate asks about the run, not about a dependency.
ManifestCheckgains
registry_unreachable, set infetch_allwhen a lookup fails for any reasonother than
FetchError::NotFound— a timeout, a refused connection, a 5xx, aresponse that would not decode. That, together with a vulnerability scan that did not
complete, is what leaves a gate unanswerable. A 404 is an answer and is treated as
one: it stays visible in the table and in
--format json, and a run with a gate armednow says on stderr how many dependencies were not found and therefore not gated on.
--fail-on anystays answerable through an unreachable registry, because it failson the
Errorstatuses that registry produces — the promise is kept, not missed. Theother settings match specific statuses and skip errors, which is where a run that
resolved nothing could still be reported clean, so they still refuse to certify.
After:
Tests.
runner::tests::a_package_the_registry_says_does_not_exist_does_not_break_the_gateis new and fails on the old rule.
unresolved_dependencies_cannot_pass_a_status_gatepinned the behaviour being removed and is replaced by
an_unreachable_registry_cannot_pass_a_status_gate, which asserts the half of theguard that has to survive — an unreachable registry still exits 2 under
--fail-on vulnerableand--fail-on outdated.a_failed_scan_cannot_pass_a_vulnerability_gateis untouched and still passes.2. CRITICAL —
fix --allrewrote a pnpm override selector from an unrelated packagepnpm scopes an override to a parent with
>:"foo@2>bar"pins bar.override_namesplit only on/and stripped a trailing@…, so the key resolved tofoo.Before, on
{ "pnpm": { "overrides": { "foo@2>bar": "3.0.0" } } }:That write would have produced
"foo@2>bar": "1.0.0"— silently downgrading a pin onbarto the newest release of an unrelated package.Repair, both halves.
>-separated segment, with its own@versionselector stripped. The selectors in front of it only say which parent the override
applies to.
DependencyKind::Overrideis excluded fromplan_fixesentirely. An override is aversion this manifest deliberately forces onto the resolved tree, very often a
security pin holding a transitive dependency above a vulnerable release. Reporting
that a newer version exists is useful; rewriting the pin to it defeats the reason the
entry was written. The whole kind is declined rather than guessing which pins are
safe to move.
After:
Tests.
package_json::tests::a_scoped_override_key_names_the_package_after_the_last_arrowcovers
"foo@2>bar","foo>bar","a>b>c","@scope/pkg@1>@scope/other"and plain"foo";a_scoped_pnpm_override_is_checked_as_the_package_it_pinsasserts the samethrough the parser.
fix::tests::fix_all_leaves_an_override_alonefails without thekind guard (it plans
["monolog", "minimist"]instead of["monolog"]) and asserts anon-override neighbour is still fixed, so it cannot pass by doing nothing.
3. HIGH — npm's documented
$nameoverride value hard-failed the run"overrides": { "semver": "$semver" }is npm's documented way to force the versionthis manifest already depends on. Nothing filtered it, so it reached the version
checker verbatim.
Before, on
{ "dependencies": { "semver": "^7.5.0" }, "overrides": { "semver": "$semver" } }:Three of this branch's own fixes composed into a new CI break on a valid manifest.
Repair. The reference resolves the way
csproj.rsalready handles$(MSBuildProp). Where the named direct dependency exists, the override is checkedagainst that dependency's declared constraint, and its recorded span reports its
position but declines its width — the same treatment an escaped JSON value gets — so
nothing can write a version over the reference. Where the manifest declares no such
dependency, the entry is the new
PackageSource::Unresolved: a real package whoseintended version cannot be read, reported as
Undetermined(see 4) rather than as aparse error. The
$form is recognised only inside an override map.After:
Tests.
a_dollar_override_resolves_to_the_dependency_it_names,a_dollar_override_naming_nothing_declared_is_unresolvable, anda_dollar_is_only_a_reference_inside_an_override.4. HIGH — a constraint that failed to translate was reported
UpToDatecheck_versionreceives an already-translated constraint, and three of the fourtranslators signal failure by returning the empty string:
maven_constraint_to_semver(an untranslatable version, orinterval_range(...) .unwrap_or_default()on a malformed interval),nuget_constraint_to_semver(the sametwo paths), and
pep440_constraint_to_semver(every clause dropped). The arm added foran empty constraint read that as "the author declared no range" and evaluated
*, solatest_compatiblebecame the newest release and the status became the most confidentanswer the tool has — reintroducing, at every ecosystem that translates, exactly the
failure mode
1b13d61exists to remove.Before, with
--fail-on outdated:version = "[4.0,4.9"version = "LATEST"<PackageReference Version="[12.0.0,13.0.0" />, latest 13.0.4 outside the intended rangerequests!=2.31.0Repair.
try_to_semver_constrainttells the two apart by what went in: an emptyresult from a non-empty input is a failed translation, and nothing else produces
one.
check_version_fortranslates and classifies together, so the distinction cannotbe lost between the two calls, and reports the new
DependencyStatus::Undetermined—a real package whose declared version this run could not read. It claims nothing about
currency, is counted and rendered as itself (table cell, totals line,
--format jsonsummary, and
dependable-report'sSummary), andfixnever rewrites it because itis not one of the four updatable statuses.
After, all four rows read
undetermined;--fail-on anyfails on them,--fail-on outdateddoes not claim they are current.Tests.
checker::tests::an_untranslatable_constraint_is_undetermined_not_up_to_datecarries one case per affected ecosystem (JVM interval, JVM
LATEST, NuGet interval,PEP 440
!=).an_absent_constraint_still_means_any_versionasserts the other half ofthe distinction across five ecosystems, and
a_translatable_constraint_is_still_evaluatedproves the guard is not simply refusing to answer.
5. MEDIUM — a
--no-default-featuresbuild hard-errored on any config carrying[policy]Configcarriesdeny_unknown_fields, so declaringpolicyonly under thereportfeature made the absence of the field a rejection.
Before:
dependable check --config <file>with[policy]\nmax_cvss = 7.0exited 2 onunknown field: foundpolicy``, where master warned and exited 0 — leavingwarn_policy_ignoredand `has_policy_table` unreachable for the only case they existfor, because the load they needed to survive failed first.
Repair. The key is declared in every build, typed as the policy schema only where
the feature can read it and as an unread table otherwise. The feature gates what is
done with the block, not whether it is a known key. The restored warning also loses
the run of spaces a line continuation had left in its text.
After: the same command prints
warning: … declares [policy], but this build has no report feature; the policy is not enforcedand exits 0.
Tests.
config::schema_tests::a_policy_block_loads_whether_or_not_this_build_enforces_itruns in both builds and fails without the fix;
an_undeclared_key_is_still_rejectedholds the other half of
deny_unknown_fieldsin place.tests/cli_policy.rsandtests/cli_sarif.rsassert policy enforcement and SARIFrendering, both of which the
reportfeature builds. They now state that requirement(
#![cfg(feature = "report")]) instead of failing a--no-default-featuresrun forthe absence of code they never compiled — a pre-existing condition, unrelated to the
defect: 11 failures before this branch touched anything.
--no-default-featuresnowbuilds clean and runs 133 passing tests.
6. MEDIUM — a JVM mirror's answers cached under Maven Central's key
MavenCentralFetcherwas the oneRegistryFetcherof ten that never overroderegistry_root, because it landed on master after cache scoping was written and themerge had nothing to conflict with. It has a configurable
base_urldriven by[jvm] registry, so a run againsthttps://nexus.corp/...with--cachewrotecom.google.guava:guavaunder the bareMavenkey; a later default-registry run wasserved the mirror's version list, and
--fixwould splice an internal-only versioninto the manifest. The name guard cannot catch it — the name matches.
Repair.
registry_rootreturns the configured base URL, matching the other nine.Test.
check::tests::every_default_fetcher_scopes_a_non_default_registrydrivesall nine default fetchers rather than the one that was missing, so a tenth
implementation cannot repeat it. Without the override it fails with
Jvm caches a mirror's answers under the public registry's key: left "Maven" right "Maven". It also asserts that a default registry still keeps the bare key, soexisting cache entries stay valid.
7. MEDIUM —
tree --format jsonand--format dotdropped the truncation flagflattencalledgraph.tree(opts)and discardedtree.truncated, so only the ASCIIrenderer said a walk had run out of budget.
tree --no-dedupe --format jsonon a graphthat hits
DEFAULT_MAX_VISITSemitted a document byte-indistinguishable from acomplete one — and the machine formats are the ones a consumer cannot eyeball.
Repair, an additive schema change. JSON gains a top-level
truncatedboolean,always present so a consumer can require it rather than infer completeness from its
absence. DOT gains a comment and a
dependable_truncated=truegraph attribute, so atool reading the file sees it too, not only a person.
DEFAULT_MAX_VISITSandMAX_WALK_DEPTHkeep their values and now record where eachnumber comes from — an appearance budget bounded above by the largest real forests and
below by what still looks like a bounded operation, and a stack budget rather than a
graph property — including why neither is user-tunable. No user-facing override is
added.
Tests.
output::tree::tests::every_format_reports_a_truncated_walkbuilds a chainpast the recursion ceiling, first asserting the fixture actually truncates so the rest
proves something, then checking all three renderers.
a_complete_walk_reports_itself_completepins the always-present half.8. LOW — the SARIF URI was still platform-dependent for UNC and verbatim prefixes
absolute_file_uriinserted a Windows prefix unencoded after replacingbackslashes, which is right only for a drive letter:
\\server\share\repo\Cargo.toml→file://///server/share/…, where the correctform is
file://server/share/….\\?\C:\repo\Cargo.toml— whatstd::fs::canonicalizereturns and whatdiscover.rs'ssimplified()deliberately preserves — →file:////?/C:/repo/Cargo.toml,where the unencoded
?opens a URI query and truncates the path atfile:////.C:foo\Cargo.tomllost its prefix injoin_components, rendering asfoo/Cargo.toml— the URI a path on any other drive produces.Repair. Each prefix form has an explicit spelling: a drive (plain or verbatim) is a
path segment whose colon survives; a UNC share (plain or verbatim) is an authority plus
a first segment; the verbatim and device namespaces are encoded segments. A
drive-relative path keeps its drive as an encoded segment (
C%3A/crates/app/Cargo.toml).Tests. The Windows-only test's drive-relative assertion pinned the dropped prefix
as the expected result. It is corrected to the URI that preserves it — restoring
information the renderer was losing, not relaxing the check — and gains UNC, verbatim
disk, and verbatim UNC cases. Because
Pathparses a prefix only on Windows, which ishow these forms went unnoticed, the prefix decisions are now also asserted directly by
every_windows_prefix_form_has_a_uri_spelling, which runs on every platform:Prefixitself is spellable everywhere.Decisions recorded
fixmust notedit a version the manifest deliberately forces onto the tree. Reverses if
fixgrows a way to distinguish a security pin from a stale one.
--allow-unresolvedflag is added; narrowing the condition removes the need for one.Reverses if a user needs a 404 to fail the build — that is what
--fail-on anyalready does.
Undeterminedis a newDependencyStatusvariant. The enum is#[non_exhaustive]and every internal match already carried a wildcard arm, so thisis additive; it is counted explicitly wherever
Erroris. It was preferred to reusingError, which means "the registry or the fetch failed" and would have made aperfectly readable manifest look broken.
--ecosystemremoval stands, tracked in feat(cli): implement --ecosystem rather than leaving it removed #105.run_liststaying fallible on an unreadable config stands — defaults enable everyecosystem, so
listwould over-report from a config it failed to read.DEFAULT_MAX_VISITSandMAX_WALK_DEPTHkeep their values, with the basis foreach number now documented where it is defined, and no user-facing override.
@v0.1.3stands — nov0.1.4tag exists.deny_unknown_fieldsonJvmConfigandVulnConfigstands.Breaking changes
dependable-reportbinary target is removed. Its entire body printed "notimplemented yet" and exited 2, so nothing depended on its behaviour — but
dependable-reportis a published crate, and a binary target disappearing from apublished crate is a break for anyone who installed or invoked it.
dependable check --ecosystemis removed (feat(cli): implement --ecosystem rather than leaving it removed #105).tree --format jsongains a required-shapetruncatedkey and--format dotgains a graph attribute. Additive: no existing key changes name, type, or meaning.
check --format json's summary gains anundeterminedcount, andUNDETERMINEDjoins the status tokens a result can carry. Additive in the same sense.
dependable-core:DependencyStatus::UndeterminedandPackageSource::Unresolvedare new variants of
#[non_exhaustive]enums;check_version_forandtry_to_semver_constraintare new exports. No existing signature changes.dependable-fetch:ManifestCheckgainsregistry_unreachable. The struct is#[non_exhaustive], so this is additive.Validation
FORCE_COLORin an interactive shell makes threetreetests fail spuriously bycolouring the labels they assert on; the gates above unset it, and CI does not set it.
Second review round
A re-review confirmed the four original defects fixed, and found that three of the four
repairs had introduced regressions of their own. All seven findings below are repaired,
each reproduced against a stub registry before the fix and re-run after.
The fixture that was missing
Both HIGH regressions were found by standing up a stub HTTP registry and driving the real
binary against it. The suite had no way to do that:
cli_policy.rsandcli_sarif.rsstay hermetic by declaring path dependencies, so no fetch is ever built and no status code
is ever seen — the opposite of what a gate keyed on the difference between "no such
package", "no answer", and "could not read the constraint" needs.
crates/dependable/tests/cli_gate.rsadds that fixture, in the shapecli_fix.rsestablished: a single-shot HTTP server on
127.0.0.1:0built onstd::net::TcpListener,no dev-dependency, carrying a status code and content type per route because these defects
are about status codes and about documents that are not JSON. Seven tests; every one of
them fails on the parent commit and passes here.
HIGH-1 — a Go
410 Gonehard-failed the whole gate410isproxy.golang.org's canonical answer for a module it will not serve, and theprotocol names it alongside
404as a not-found response. The@v/listhandler treatedonly
404that way, so a private module becameFetchError::Status→ registryunreachable → gate unanswerable. One private module took a Go repository from passing to
exit 2 under the shipped Action's default
--fail-on vulnerable— the identical break the404 carve-out was written to remove, unrepaired for Go.
Repaired at both the list handler and the
@latestfallback.Falsified by
a_go_module_the_proxy_answers_410_for_does_not_break_the_gate.HIGH-2 — the 404 carve-out exempted every error, including ones no registry saw
ScanIntegrity.unresolvedcounted anyDependencyStatus::Error, and the carve-outexempted that whole set. An unparseable constraint never reaches a registry, so two
published, resolvable dependencies went unevaluated while the gate certified the build and
the note blamed a registry that was never asked. This was the more dangerous of the pair,
because it made the gate lie.
The provenance is now carried rather than re-derived from a message string. Each
CheckResultrecords anErrorOriginbuilt from the typedFetchError:NotFoundforthe registry's own answer,
Unansweredfor a request that produced none,Localfor afailure this run reached by itself.
CheckResult::newrecordsLocalfor anError,because "no provenance recorded" is not evidence a registry answered.
ScanIntegritysplits into
unresolved(404s, exempt and reported) andunevaluated(local failures,which make the gate unanswerable as they did before the carve-out), and the gate now names
every reason it could not be honoured rather than a hand-written combination per pair.
The round-one test asserted the carve-out against a hand-written error string, which is
why it could not catch this; it now asserts the provenance.
Falsified by
a_dependency_this_run_could_not_evaluate_still_breaks_the_gate,an_unreadable_constraint_still_refuses_to_certify_the_build, and — for the half that hadto survive —
a_package_the_registry_answers_404_for_does_not_break_the_gate.HIGH-3 — a
>inside an override key's range was read as a pnpm parent separatorpnpm and Yarn both allow a version range in the key, and a range can contain
>.Splitting on every
>cut each key inside its own range, and given HIGH-2 the resulting404 was silently exempted rather than reported.
The split is bounded: a
>separates a parent from the package it scopes only when itfollows something that can end a name or a version. A comparator opens a clause, so it
follows what opens one — the
@that introduces the range, a clause delimiter, or anotheroperator character.
quux@1>bar@^2.1.0and@scope/pkg@1>@scope/otherkeep working.Falsified at the helper, through the parser, and by
an_override_key_carrying_a_range_is_checked_as_its_own_package.HIGH-4 — Poetry
"*"regressed fromup to datetoundetermined*matches no PEP 440 operator and holds no numeric release, so every clause was droppedand the translation came back empty — which round one's new failed-translation heuristic
read as a constraint nobody could parse.
*is PEP 440's and Poetry's explicit "anyversion" and the most common way to write an unpinned dependency.
A bare
*clause now translates to*, as the NuGet and Maven translators already do fortheir own wildcards. The other translators were re-audited and the result pinned in both
directions: every ecosystem's "any version" spelling must translate, and the forms semver
genuinely cannot express — a
!=exclusion, an MSBuild property, Maven'sLATEST/RELEASE— must keep coming back as failed translations.*was the only wrongone.
Falsified by
a_poetry_wildcard_resolves_instead_of_going_undetermined.MEDIUM-1 —
Undeterminedwas gated on by nothing and noted by nothingRound one made the status honest and stopped short of the consequence: a run that could
not read two constraints printed a clean
--fail-on outdatedand said nothing at all.The note mirrors the not-found note exactly — same silences, same shape.
Undeterminedisdeliberately not added to
--fail-on outdated: that changes what the setting promises,which is a policy decision beyond this PR, and
--fail-on anyalready fails on it. It islikewise left out of SARIF, which excludes
Errorfor the same recorded reason (a toolfailure is not a finding about the code). HIGH-4 was fixed first, so the note is not noisy
for every Poetry project.
Falsified by
a_dependency_whose_version_could_not_be_read_is_noted.MEDIUM-2 — a
200listing no versions was reported as a 404A
maven-metadata.xmlthat parses but carries no<version>elements returnedFetchError::NotFound, and that spelling is now load-bearing. A Nexus or Artifactory grouprepository whose upstream proxy is down serves exactly such a locally-merged document for
an artifact that certainly does exist, so it was silently exempted from the gate.
It now has its own
FetchError::EmptyVersionList, non-transient because the same documentparses the same way next time. The unit test that pinned the opposite is corrected.
Falsified by
a_metadata_document_listing_no_versions_is_not_exempt_from_the_gate.LOW findings
line wrapped inside a string literal, the same mistake
d5cc3a2repaired inwarn_policy_ignored. It is the message that can least afford to look broken, since theadvice it gives is the user's only way out of the error.
PackageSource::Unresolvedfell through to"unknown"inlist --format json; it now serializes as"unresolved", so a consumer can tell adangling
$nameoverride from any other unnamed source.level_ofreturnedNoneforUndetermined, producing no GitHub Actionsannotation where a plain
Errorat least got a notice. Both mean "this dependency wasnot checked"; both are now annotated, and
Undeterminedgets its own message ratherthan the bare status label.
"$"and"$a b"failed the reference guard and fell through to the checkeras literal constraints, hard-failing on the
$— precisely the failure the referenceform exists to prevent. Every
$-prefixed override value is now read as a reference;one that does not resolve becomes
Unresolved, as a dangling$namealready did.Decisions recorded
fixkeeps decliningDependencyKind::Overridewholesale. The reviewer is rightthat this is broader than the reported defect: a Yarn
resolutionsentry that is astale compatibility pin can no longer be advanced by
fix --all. But the tool cannottell a security pin from a stale one, and writing over a version the author deliberately
forced is the worse error. An opt-in is proposed in fix: let an override's forced version be advanced on purpose #111.
$nameoverride adopting the referenced dependency's constraint stands, includingthat the override row duplicates the dependency row in
checkoutput. The override isthe referenced constraint once resolved, so the row reports the version the manifest
actually forces; the duplication is the manifest saying the same thing twice.
registry_unreachablestays one boolean per manifest. Per-ecosystem granularity isa real improvement for a polyglot monorepo — a Go proxy timeout should not make an npm
manifest's answers unusable — but it is an improvement, not a defect repair. Filed as
check: make
registry_unreachableper-ecosystem rather than per-manifest #112;ErrorOrigin::Unansweredalready carries the per-dependency half of it.-q, whose help says "Only print errors". A noteabout what was skipped is not an error, and the not-found note printed through it.
Validation (second round)