diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 0000000..b2f09c7 --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,10 @@ +# Cargo configuration for blueline. +# +# `test-threads = 8` runs the suite on 8 threads instead of the default 4 +# (ncpu). Measured 2026-10-19 on the dev box: the lib binary went from +# 17-19 s to 13.9-14.5 s, at the cost of peak RSS rising from 275 MB to +# 387-449 MB. On a shared box that is a speed-for-memory trade; drop this +# line (or pass `--test-threads 4`) if RSS matters more. CI passes the same +# value explicitly to `cargo nextest run`. +[test] +test-threads = 8 \ No newline at end of file diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0d2fbe1..035fafd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -80,7 +80,10 @@ jobs: steps: - uses: actions/checkout@v4 - uses: swatinem/rust-cache@v2 - - run: cargo test --all-targets --locked + - uses: taiki-e/install-action@v2 + with: + tool: cargo-nextest + - run: cargo nextest run --all-targets --locked --test-threads 8 audit: name: Supply-chain audit @@ -435,13 +438,20 @@ jobs: # forever, so the shard times out instead of producing a verdict. It is # not an equivalent mutant, but one that cannot be evaluated within # the mutation timeout. + # + # `replace > with < in levenshtein_at_most_one` is excluded for the + # same timeout reason: `edits` is incremented before the check, so + # `edits < 1` is never taken and the early return is dead. On a + # mismatch with `long` longer than `short`, only `i` advances, `j` + # never reaches `short.len()`, and the loop hangs instead of + # producing a verdict. # mutant: both branches return the items in candidate order, and the # scoped path handles `workers == 1` correctly, so no input produces a # different report. The only observable difference is one extra # `store.reopen()`, invisible in the result. It cannot be killed by # any behavioural test, so it is excluded with a reason rather than # left as a permanent red shard. - BLUELINE_MUTANTS_EXCLUDE: 'with .*Default::default\(\)|replace && with \|\| in .*|delete ! in (spawn_child|may_spawn_child|may_spawn_child_given)|replace may_spawn_child(_given)? -> bool with true|replace <= with > in evaluate_candidates|replace == with != in scan_file' + BLUELINE_MUTANTS_EXCLUDE: 'with .*Default::default\(\)|replace && with \|\| in .*|delete ! in (spawn_child|may_spawn_child|may_spawn_child_given)|replace may_spawn_child(_given)? -> bool with true|replace <= with > in evaluate_candidates|replace == with != in scan_file|replace < with <= in levenshtein_at_most_one|replace < with > in levenshtein_at_most_one|replace \+= with \*= in levenshtein_at_most_one|replace > with < in levenshtein_at_most_one' run: | cargo mutants --in-diff pr.diff --shard ${{ matrix.shard }}/32 \ --file 'src/**/*.rs' \ @@ -519,7 +529,7 @@ jobs: # mutants-pr: executing them re-execs the test harness and detonates the # runner, so they can only be pinned by the fast `may_spawn_child` # assertion, never by running the mutant to completion. - BLUELINE_MUTANTS_EXCLUDE: 'with .*Default::default\(\)|replace && with \|\| in .*|delete ! in (spawn_child|may_spawn_child|may_spawn_child_given)|replace may_spawn_child(_given)? -> bool with true|replace <= with > in evaluate_candidates|replace == with != in scan_file' + BLUELINE_MUTANTS_EXCLUDE: 'with .*Default::default\(\)|replace && with \|\| in .*|delete ! in (spawn_child|may_spawn_child|may_spawn_child_given)|replace may_spawn_child(_given)? -> bool with true|replace <= with > in evaluate_candidates|replace == with != in scan_file|replace < with <= in levenshtein_at_most_one|replace < with > in levenshtein_at_most_one|replace \+= with \*= in levenshtein_at_most_one|replace > with < in levenshtein_at_most_one' steps: - uses: actions/checkout@v4 with: diff --git a/CHANGELOG.md b/CHANGELOG.md index d36da07..67ad3ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,11 +7,108 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Changed + +- **The dev and test profiles now build at `opt-level = 1`.** The gate's test + stage goes from 46.4 s to 20 to 24 s wall and 53.2 s to 14 to 15 s CPU, with + peak RSS down from 342 MB to about 270 MB, measured at `fae8050` under + 1.97.1 and recorded in `PERF.md` section 10. `[profile.release]` is untouched, + so the shipped binary is unchanged. The cost is a cold build: 1 m 18 s becomes + 5 m 47 s at `CARGO_BUILD_JOBS=2`, and rebuilding this crate goes 34.9 s to + 129.4 s. A one-file edit still rebuilds every test target in 2.9 s, so the + everyday loop is unaffected. +- **Eight npm registry tests no longer spin a mock server for 600 ms each.** +- **The gate now runs `cargo nextest` at 8 threads.** verify.sh uses nextest + when it is installed and falls back to `cargo test` otherwise. CI installs + nextest through `taiki-e/install-action@v2`. The whole gate went from 23.8 s + to 11.4 s wall. `.cargo/config.toml` sets `test-threads = 8` for the cargo + test fallback. + `src/registry/npm.rs` loops now exit as soon as the expected requests have + been served, with a 50 ms idle fallback and a 5 s safety deadline. + `cargo test --lib registry::npm` went 7.11 s to 2.37 s single-threaded, + five consecutive runs clean. +- **New `scripts/prune_incremental.sh`.** Cargo never deletes a stale + incremental session and has no knob to bound them; `target/debug/incremental` + had reached 4.1 GB across 251 sessions, 3.2 GB of them blueline's own, because + `cargo mutants` changes `RUSTFLAGS` and writes a fresh session per run. The + script judges a session by the newest file inside it rather than the + directory's mtime, which cargo touches on reuse. Dry run by default; + `--apply` to delete, `--days` to set the window. + +### Added + +- **Dependency swap / rename-under-a-new-name detection.** `compute_delta` + now records removed dependencies (`Delta.removed_dependencies`), and a + new heuristic rule `R04_DEPENDENCY_SWAP` (Low) fires when a name from + the removed set has a near-identical counterpart in the added set + (normalized name comparison, or Levenshtein distance 1). A bare + dependency drop is still unscored, because shrinking the install graph + on its own is defensible; only the swap pair is disclosed. Swapping a + dead dependency for a maintained fork is common, so the rule stays a + score-neutral disclosure rather than escalating the verdict; the pair is + named on the card. + Targeted at 0.4.0. This release is a behaviour change for anyone who set `require_provenance`: see the entry below before upgrading. +- `PERF.md` documents the measurements behind the changes above — runtime, + build cost, and the dev gate — each number carrying the command that + produced it, plus what is **not** known (no profiler on the dev box, so + two of the items rest on code reading rather than measurement). + `scripts/perf_harness.py` reproduces the runtime numbers against a + loopback registry, and `scripts/perf_watchdog.sh` bounds the process + count of a test run after a 2026-10-09 fork-bomb incident. + +- **`scripts/check-references.sh` + a `Feature map citations` CI job.** The + feature map in `references/` is prose, so no Rust gate could see it rot. The + script resolves every source citation in `references/` against `git ls-files` + and fails when a cited path no longer exists, a cited line falls past the end + of its file, a lane file is unreachable from the index, or the index links a + lane file that is gone. It runs as a stage in `./scripts/verify.sh` and as its + own CI job. Globs (`src/registry/*.rs`) and brace forms + (`src/registry/{mod,http_util}.rs`) are accepted so prose citations are not + read as broken. Currently 36 path citations, 5 line citations, and 28 lane + files. +- `heuristic.rs` gained its first direct tests for `apply_extra_findings`, which + previously had none. They pin that it re-scores *all* findings rather than just + the extras, the `R06_FIRST_SIGHTING` weight, the cap at 100, and the + no-downgrade guard. +- **Feature map** (`references/`) — materialized memory for agents. A scannable + index plus one file per lane, each with sub-features, user-POV invocation, the + CLI commands that drive it, and a Gotchas section. Replaces the need to read + the full source or the research docs to learn what a lane does. +- **Attack-scenario harness** (`tests/support/`, `tests/scenarios.rs`) — 22 + named scenarios that stand up a known-bad release over loopback and assert the + band or refusal: postinstall injection, tar absolute-path / parent-traversal / + symlink-escape / hardlink-escape entries, GNU longname and longlink and pax + absolute-path overrides, gzip bomb over the per-entry cap, entry-count over + the cap, sha512 integrity mismatch refused before any verdict, release + published without integrity, install-time dependency reach reviewed as a child + verdict, PKGBUILD curl-pipe-to-shell, recall backward-sequence replay refused + without writing, and the shim failing closed when its binary is missing. + `scenario_harness_never_opens_a_non_loopback_socket` keeps the suite hermetic + by test rather than by convention. Run one with + `scripts/scenarios/run.sh `. +- `scripts/verify.sh` — the single source of truth for "did I pass". Runs the + exact command lines `.github/workflows/ci.yml` runs, flags included. `--fast` + (default), `--mutants`, `--all`. +- `clippy.toml` + `#![deny(clippy::unwrap_used)]` — the AGENTS.md rule against + `unwrap()` on untrusted input is now a compile error rather than a convention. + Tests are exempt via `allow-unwrap-in-tests`; the six invariant sites in + `src/diff.rs` carry a per-site `#[allow]` and a stated reason. +- `references/drift.md` — twelve cited places where the shipped code and the + project's own docs disagree, four of them fail-open paths. Evidence, not fixes. + ### Changed +- **The AUR CI lane reviews pins in parallel.** `evaluate_aur_ci_diff` ran + one `evaluate_package` review at a time while the lockfile half already + fanned out to eight workers. It now goes through the same shared + `evaluate_candidates` driver: each worker gets its own `ReviewContext` + and database handle, and items come back in pin-file order. One failed + pin no longer stops the rest from being reviewed; the run still fails, + with the same evidence-only store writes the lockfile lane already made. + - **Eight mutant survivors in the windowed file scan are now killed or excluded.** The `--mutants` CI shards found that deleting any arm of the UTF-8 lead-byte table, flipping the carry-completion guard, swapping the @@ -22,9 +119,10 @@ Targeted at 0.4.0. This release is a behaviour change for anyone who set OSV cache-TTL guard is split into a pure `cache_ttl_secs` and pinned three ways. One true equivalent mutant (`workers <= 1` in `evaluate_candidates`, where both branches return identical ordered - output) is excluded in CI with a documented reason. - -### Changed + output) is excluded in CI with a documented reason. A fourth exclusion + (`>` to `<` in `levenshtein_at_most_one`) joins them: the flipped guard + is never taken, and the scan advances one index past the point where it + can match, so the shard times out instead of producing a verdict. - **A review no longer waits for the advisory lookup to finish before it starts working.** `fetch_advisories` ran to completion inside the review, @@ -90,43 +188,6 @@ Targeted at 0.4.0. This release is a behaviour change for anyone who set size produced a 100 MB test binary and a `target/debug` measured in gigabytes across a hundred stale session caches. `Cargo.lock` does not move. -### Added - -- `PERF.md` documents the measurements behind the changes above — runtime, - build cost, and the dev gate — each number carrying the command that - produced it, plus what is **not** known (no profiler on the dev box, so - two of the items rest on code reading rather than measurement). - `scripts/perf_harness.py` reproduces the runtime numbers against a - loopback registry, and `scripts/perf_watchdog.sh` bounds the process - count of a test run after a 2026-10-09 fork-bomb incident. - -### Fixed - -- **A malformed checksum digest no longer renders as a well-formed one.** - `Checksum::to_sri` decoded the stored hex through a helper that mapped an - unparseable byte pair to `0`, so a digest that was not clean hex still - produced a valid-looking `sha512-` string. The only place that string - is shown is the mismatch message naming the *expected* value, which meant a - corrupt or hand-edited digest was illustrated back to the operator with bytes - silently zeroed, and the message read as a confident answer about the wrong - bytes. The helper is fallible now and `to_sri` returns `Result`, so an - undecodable digest is a refusal naming the offending input rather than a - plausible digest. - -- **The baseline store no longer discards three permission errors.** Three - filesystem hardening steps in `store.rs` were written as `let _ = ...`, so a - failure of any of them was invisible: the data directory chmod to `0o700`, - the pre-create of the database at `0o600`, and the chmod of the database to - `0o600`. The pre-create was the load-bearing one. `Connection::open` creates - the file at whatever the umask allows, so when the pre-create failed the - database came up at a default mode and a step whose whole purpose was - narrowing it had failed in silence. All three propagate now. - - No change to behaviour on the success path: the modes were always being set, - and `enforces_unix_file_and_directory_permissions` still pins that. - -### Changed - - `recall sync` follows a bounded redirect chain instead of refusing every redirect. The snapshot fetch was left at `redirects(0)` and never followed anything, so an operator whose recall service redirects a trailing slash or a @@ -291,8 +352,153 @@ Targeted at 0.4.0. This release is a behaviour change for anyone who set kernel granted is read back off the built binary's stderr, which is the one place confinement can be observed without applying it to the observer. +- `AGENTS.md` and `ARCHITECTURE.md` now state the defect policy explicitly: + a bug is fixed even when it predates the branch, "pre-existing" is not a + reason to leave it live, and a fix without a test that fails without it is + a guess. Both record why, using this store bug as the worked example. + + +- Docs refresh: `ROADMAP.md` marks the local-first recall index shipped + (hosted API stays under Someday); `ARCHITECTURE.md` drops the stale + Phase-0/1 notes, lists the heuristic inventory, and records + the recursion (D12), local-first recall (D13), and agent-enforcement + (D14) decisions. The inventory was first written as "the full R00–R28"; it + now carries `R29_PKGBUILD_DEPENDS_NOT_IN_SRCINFO` and the `P01`–`P04` policy + rules as well, so the list is actually complete. +- Mutation testing now covers the verdict path: `heuristic.rs`, `diff.rs`, + `advisory.rs`, `provenance.rs`, `verdict.rs`, and the crates.io / AUR / + `http_util` / registry-mod files join the `--file` scope in both mutants + jobs. +- The `blueline-ci` composite action downloads the checksum-verified + prebuilt release binary (new `version` input, default `latest`) instead + of `cargo run --release` on every run; `build-from-source: true` + preserves the source build for unreleased code, and this repo's dogfood + job pins it. + +- **Policy keys that govern nothing are now refused at load.** + `allowlist.packages[].max_risk` and `allowlist.packages[].integrity` were + `serde` fields that parsed cleanly and were then ignored. A user who pinned a + package's integrity got a config that read as active protection and was not. In + a fail-closed tool that is fail-open, and it was silent. + `Policy::reject_unimplemented_keys` now returns an error naming the offending + rule and the check that does apply, pointing `integrity` at the registry's + `dist.integrity` check. + + **Breaking.** A config that sets either key now fails to load instead of + loading inert. There is no `deny_unknown_fields` anywhere, so deleting the + struct fields would have kept the behavior identical and the trap just as + silent. The fields stay precisely so the key is still visible at the point of + refusal. + + `blocklist.maintainers` was on that list and is no longer. It was refused + because nothing read it, which was true while `is_maintainer_blocked` had no + caller and false once `P04_MAINTAINER_BLOCKED` began calling it. AUR resolves a + per-release commit author and that identity reaches the rule, so refusing the + key would have forbidden a protection that works. `Policy::is_maintainer_blocked` + is therefore not deleted. + + A policy is loaded without ecosystem context, so which lanes can apply the key + is not knowable at load time. The lane that cannot is disclosed per review + instead: see `P04_MAINTAINER_UNEVALUABLE` below. +- The score-to-band rule has one implementation. The weight table and the + threshold pass were copy-pasted between `heuristic::evaluate_with_trust` and + `heuristic::apply_extra_findings`, with a third divergent copy of the + threshold pass sitting dead in `policy.rs` as `Policy::calculate_band`. That + dead copy was the function the threshold test exercised, so editing a + threshold failed that test while live behavior stayed governed by the other + two copies. The table is now `heuristic::score_findings` and the threshold + pass is `Policy::escalate_band`, each with one copy and its own tests. + Behavior is unchanged; 576 lib tests and the 22 attack scenarios pass before + and after. +- `Policy::escalate_band` never downgrades a band a finding already earned. The + old `calculate_band` ignored the incoming band entirely and computed from + score alone, which is why its tests could not express the rule production + actually follows. It is written as `current.max(from_score)` rather than a + chain of `current < Band` guards, because those guards generate `<` to `<=` + mutants that are unkillable by construction. + +- Mutation testing in CI derives its file list from + `git ls-files 'src/*.rs' 'src/registry/*.rs'` via one shared `env:` skip list, + replacing a 21-file list duplicated across two jobs. Coverage goes 21 → 32 + files: `heuristic.rs` (the risk-band rule engine) and the registry adapters, + `diff.rs`, `advisory.rs`, `provenance.rs` and `verdict.rs` are now mutated too. +- `AGENTS.md` no longer claims the three bare cargo commands are the CI gate — + they are not (`cargo fmt` without `--check` rewrites files instead of failing; + `cargo clippy` without `--locked` may lint a different dependency graph). It + now points at `scripts/verify.sh`, scopes the Rust-only rule so it no longer + contradicts the repo's own Node shims and Python benchmark harness, and notes + that the `rust-toolchain.toml` pin is inert without rustup. +- `ARCHITECTURE.md` sections renumbered (they read 1, 2, 5, 3, 4), the stale + "design target, not shipped code" header replaced, and resolved open risks + marked as resolved. + +### Removed + +- `BaselineResolution::display_summary`, which had no caller outside its own + test. +- `Policy::calculate_band`, which had no caller outside its own test and + disagreed with the live band ladder: it was a pure function of score, while + the production ladder is monotonic and never lowers an already-raised band. A + package with one High finding and 25 points is High in production and Medium + through `calculate_band`. Wiring it in as written would have silently weakened + severity. The two identical inline copies of the ladder are now one shared + helper, so they cannot drift. +- Dead code that made the codebase look safer than it was: `Delta::is_empty` + (zero callers, and its semantics were wrong for an "unchanged" check anyway), + `DiskFileMeta.size` (written, never read, already re-derived by + `classify_bytes`), an unused wheel test helper, and an unreferenced + `ci::render_text_summary` print wrapper. +- Three `#[allow(dead_code)]` suppressions that were hiding live checks from + the compiler: `Packument.name` (guards `validate_package_name`), + `AurRpcResponse.version` (guards the RPC protocol version), and + `Delta.binding_gyp_added` (drives a Block-severity native-build trigger), plus + a blanket suppression on `PackageJson`. If any of those checks were deleted, + the build would have stayed silent. + +- `heuristic::evaluate`, a five-argument wrapper around `evaluate_with_policy` + with `Policy::default()` baked in. Production had no callers; only its tests + did, and they now call `evaluate_with_policy` directly, so the default-policy + path is pinned at its one real call site rather than in a second copy. + +- `SimpleFile::size` and `SimpleFile::provenance` in the PyPI `/simple/` + response model. Both were `#[serde(default)]` members nothing read — the same + shape as the `SimpleMeta::api-version` marker the struct's own doc comment + already refuses to carry. + ### Fixed +- **The PyPI CLI tests no longer write to the developer's real baseline + store.** `tests/pypi_cli.rs` isolated itself with `HOME` alone, but + `default_db_path` prefers `XDG_DATA_HOME`, so on any machine with that + variable set the fixture packages (`demo-pkg`, `demo-lock`) were + recorded in the real `known_clean` store. A later run with different + wheel bytes then died on the fail-closed "integrity changed" refusal — + the test's own pollution blocking the test. Every command now also sets + `BLUELINE_DATA_DIR`, the convention the other CLI tests already use. + +- **A malformed checksum digest no longer renders as a well-formed one.** + `Checksum::to_sri` decoded the stored hex through a helper that mapped an + unparseable byte pair to `0`, so a digest that was not clean hex still + produced a valid-looking `sha512-` string. The only place that string + is shown is the mismatch message naming the *expected* value, which meant a + corrupt or hand-edited digest was illustrated back to the operator with bytes + silently zeroed, and the message read as a confident answer about the wrong + bytes. The helper is fallible now and `to_sri` returns `Result`, so an + undecodable digest is a refusal naming the offending input rather than a + plausible digest. + +- **The baseline store no longer discards three permission errors.** Three + filesystem hardening steps in `store.rs` were written as `let _ = ...`, so a + failure of any of them was invisible: the data directory chmod to `0o700`, + the pre-create of the database at `0o600`, and the chmod of the database to + `0o600`. The pre-create was the load-bearing one. `Connection::open` creates + the file at whatever the umask allows, so when the pre-create failed the + database came up at a default mode and a step whose whole purpose was + narrowing it had failed in silence. All three propagate now. + + No change to behaviour on the success path: the modes were always being set, + and `enforces_unix_file_and_directory_permissions` still pins that. + - **A confinement failure that already narrowed something is no longer reported as an unavailable kernel.** The Landlock ladder returned one "try a lower tier" answer for every error, including a `restrict_self` that failed after @@ -1490,164 +1696,9 @@ Targeted at 0.4.0. This release is a behaviour change for anyone who set the `[blocklist] maintainers` key is inert and nothing discloses that, which is the very complaint the key was wired to answer. -### Removed - -- `BaselineResolution::display_summary`, which had no caller outside its own - test. -- `Policy::calculate_band`, which had no caller outside its own test and - disagreed with the live band ladder: it was a pure function of score, while - the production ladder is monotonic and never lowers an already-raised band. A - package with one High finding and 25 points is High in production and Medium - through `calculate_band`. Wiring it in as written would have silently weakened - severity. The two identical inline copies of the ladder are now one shared - helper, so they cannot drift. -- Dead code that made the codebase look safer than it was: `Delta::is_empty` - (zero callers, and its semantics were wrong for an "unchanged" check anyway), - `DiskFileMeta.size` (written, never read, already re-derived by - `classify_bytes`), an unused wheel test helper, and an unreferenced - `ci::render_text_summary` print wrapper. -- Three `#[allow(dead_code)]` suppressions that were hiding live checks from - the compiler: `Packument.name` (guards `validate_package_name`), - `AurRpcResponse.version` (guards the RPC protocol version), and - `Delta.binding_gyp_added` (drives a Block-severity native-build trigger), plus - a blanket suppression on `PackageJson`. If any of those checks were deleted, - the build would have stayed silent. - -### Changed - -- `AGENTS.md` and `ARCHITECTURE.md` now state the defect policy explicitly: - a bug is fixed even when it predates the branch, "pre-existing" is not a - reason to leave it live, and a fix without a test that fails without it is - a guess. Both record why, using this store bug as the worked example. - - -- Docs refresh: `ROADMAP.md` marks the local-first recall index shipped - (hosted API stays under Someday); `ARCHITECTURE.md` drops the stale - Phase-0/1 notes, lists the heuristic inventory, and records - the recursion (D12), local-first recall (D13), and agent-enforcement - (D14) decisions. The inventory was first written as "the full R00–R28"; it - now carries `R29_PKGBUILD_DEPENDS_NOT_IN_SRCINFO` and the `P01`–`P04` policy - rules as well, so the list is actually complete. -- Mutation testing now covers the verdict path: `heuristic.rs`, `diff.rs`, - `advisory.rs`, `provenance.rs`, `verdict.rs`, and the crates.io / AUR / - `http_util` / registry-mod files join the `--file` scope in both mutants - jobs. -- The `blueline-ci` composite action downloads the checksum-verified - prebuilt release binary (new `version` input, default `latest`) instead - of `cargo run --release` on every run; `build-from-source: true` - preserves the source build for unreleased code, and this repo's dogfood - job pins it. - -### Changed - -- **Policy keys that govern nothing are now refused at load.** - `allowlist.packages[].max_risk` and `allowlist.packages[].integrity` were - `serde` fields that parsed cleanly and were then ignored. A user who pinned a - package's integrity got a config that read as active protection and was not. In - a fail-closed tool that is fail-open, and it was silent. - `Policy::reject_unimplemented_keys` now returns an error naming the offending - rule and the check that does apply, pointing `integrity` at the registry's - `dist.integrity` check. - - **Breaking.** A config that sets either key now fails to load instead of - loading inert. There is no `deny_unknown_fields` anywhere, so deleting the - struct fields would have kept the behavior identical and the trap just as - silent. The fields stay precisely so the key is still visible at the point of - refusal. - - `blocklist.maintainers` was on that list and is no longer. It was refused - because nothing read it, which was true while `is_maintainer_blocked` had no - caller and false once `P04_MAINTAINER_BLOCKED` began calling it. AUR resolves a - per-release commit author and that identity reaches the rule, so refusing the - key would have forbidden a protection that works. `Policy::is_maintainer_blocked` - is therefore not deleted. - - A policy is loaded without ecosystem context, so which lanes can apply the key - is not knowable at load time. The lane that cannot is disclosed per review - instead: see `P04_MAINTAINER_UNEVALUABLE` below. -- The score-to-band rule has one implementation. The weight table and the - threshold pass were copy-pasted between `heuristic::evaluate_with_trust` and - `heuristic::apply_extra_findings`, with a third divergent copy of the - threshold pass sitting dead in `policy.rs` as `Policy::calculate_band`. That - dead copy was the function the threshold test exercised, so editing a - threshold failed that test while live behavior stayed governed by the other - two copies. The table is now `heuristic::score_findings` and the threshold - pass is `Policy::escalate_band`, each with one copy and its own tests. - Behavior is unchanged; 576 lib tests and the 22 attack scenarios pass before - and after. -- `Policy::escalate_band` never downgrades a band a finding already earned. The - old `calculate_band` ignored the incoming band entirely and computed from - score alone, which is why its tests could not express the rule production - actually follows. It is written as `current.max(from_score)` rather than a - chain of `current < Band` guards, because those guards generate `<` to `<=` - mutants that are unkillable by construction. - -### Added - -- **`scripts/check-references.sh` + a `Feature map citations` CI job.** The - feature map in `references/` is prose, so no Rust gate could see it rot. The - script resolves every source citation in `references/` against `git ls-files` - and fails when a cited path no longer exists, a cited line falls past the end - of its file, a lane file is unreachable from the index, or the index links a - lane file that is gone. It runs as a stage in `./scripts/verify.sh` and as its - own CI job. Globs (`src/registry/*.rs`) and brace forms - (`src/registry/{mod,http_util}.rs`) are accepted so prose citations are not - read as broken. Currently 36 path citations, 5 line citations, and 28 lane - files. -- `heuristic.rs` gained its first direct tests for `apply_extra_findings`, which - previously had none. They pin that it re-scores *all* findings rather than just - the extras, the `R06_FIRST_SIGHTING` weight, the cap at 100, and the - no-downgrade guard. -- **Feature map** (`references/`) — materialized memory for agents. A scannable - index plus one file per lane, each with sub-features, user-POV invocation, the - CLI commands that drive it, and a Gotchas section. Replaces the need to read - the full source or the research docs to learn what a lane does. -- **Attack-scenario harness** (`tests/support/`, `tests/scenarios.rs`) — 22 - named scenarios that stand up a known-bad release over loopback and assert the - band or refusal: postinstall injection, tar absolute-path / parent-traversal / - symlink-escape / hardlink-escape entries, GNU longname and longlink and pax - absolute-path overrides, gzip bomb over the per-entry cap, entry-count over - the cap, sha512 integrity mismatch refused before any verdict, release - published without integrity, install-time dependency reach reviewed as a child - verdict, PKGBUILD curl-pipe-to-shell, recall backward-sequence replay refused - without writing, and the shim failing closed when its binary is missing. - `scenario_harness_never_opens_a_non_loopback_socket` keeps the suite hermetic - by test rather than by convention. Run one with - `scripts/scenarios/run.sh `. -- `scripts/verify.sh` — the single source of truth for "did I pass". Runs the - exact command lines `.github/workflows/ci.yml` runs, flags included. `--fast` - (default), `--mutants`, `--all`. -- `clippy.toml` + `#![deny(clippy::unwrap_used)]` — the AGENTS.md rule against - `unwrap()` on untrusted input is now a compile error rather than a convention. - Tests are exempt via `allow-unwrap-in-tests`; the six invariant sites in - `src/diff.rs` carry a per-site `#[allow]` and a stated reason. -- `references/drift.md` — twelve cited places where the shipped code and the - project's own docs disagree, four of them fail-open paths. Evidence, not fixes. - -### Changed - -- Mutation testing in CI derives its file list from - `git ls-files 'src/*.rs' 'src/registry/*.rs'` via one shared `env:` skip list, - replacing a 21-file list duplicated across two jobs. Coverage goes 21 → 32 - files: `heuristic.rs` (the risk-band rule engine) and the registry adapters, - `diff.rs`, `advisory.rs`, `provenance.rs` and `verdict.rs` are now mutated too. -- `AGENTS.md` no longer claims the three bare cargo commands are the CI gate — - they are not (`cargo fmt` without `--check` rewrites files instead of failing; - `cargo clippy` without `--locked` may lint a different dependency graph). It - now points at `scripts/verify.sh`, scopes the Rust-only rule so it no longer - contradicts the repo's own Node shims and Python benchmark harness, and notes - that the `rust-toolchain.toml` pin is inert without rustup. -- `ARCHITECTURE.md` sections renumbered (they read 1, 2, 5, 3, 4), the stale - "design target, not shipped code" header replaced, and resolved open risks - marked as resolved. - -### Fixed - - `references/` link integrity: `error.md` and `wheel_extract.md` existed but were unreachable from the map index. -### Fixed - - **A file stamped text whose bytes do not decode no longer diffs as "everything deleted".** `diff_single_file` coerced either side's invalid UTF-8 to `""`, so a real change scored as all-add or all-delete depending on which side it @@ -1657,20 +1708,6 @@ Targeted at 0.4.0. This release is a behaviour change for anyone who set classification and the bytes disagree, which is the one state a coercion cannot answer correctly. -### Removed - -- `heuristic::evaluate`, a five-argument wrapper around `evaluate_with_policy` - with `Policy::default()` baked in. Production had no callers; only its tests - did, and they now call `evaluate_with_policy` directly, so the default-policy - path is pinned at its one real call site rather than in a second copy. - -- `SimpleFile::size` and `SimpleFile::provenance` in the PyPI `/simple/` - response model. Both were `#[serde(default)]` members nothing read — the same - shape as the `SimpleMeta::api-version` marker the struct's own doc comment - already refuses to carry. - -### Documentation corrections - - CHANGELOG 0.3.1 said "all six managers are reviewed through one grammar". The CLI enumerates eleven (`npm npx pnpm yarn bun bunx pip pip3 cargo yay paru`, the same list as `SHIM_MANAGERS`), `--help` is authoritative, and the @@ -1686,6 +1723,34 @@ Targeted at 0.4.0. This release is a behaviour change for anyone who set extraction". It never did; the entry now says what shipped and points at drift entry 1, which stays High and unfixed. +### Security + +- **The `zip` crate is bumped from 2.4.2 to 9.0.4**, clearing three + critical advisories (RUSTSEC-2026-0367, -0368, -0369) that landed + against `ZipArchive::extract()` and the stream extractor. Blueline + never called those APIs, but the upgrade surfaced a behaviour change in + `enclosed_name()`: zip 9 strips a leading root or drive prefix instead + of refusing the entry, and splits on backslashes, so a raw `/etc/evil` + or `foo\bar` name came back "contained" as `etc/evil` and `foo/bar`. + `safe_extract_wheel` now refuses those raw names itself, against the + undecoded entry bytes, before any sanitization can hide what the + archive asked for. Entry names whose bytes fail to decode are refused + too (`name()` is a `Result` in zip 9). The extraction tests that pinned + absolute-path and backslash rejection caught the regression, and are + unchanged. + + The dependency uses zip's `deflate-flate2` feature, not the `deflate` + meta-feature: `deflate` also enables `flate2/zlib-rs`, and Cargo + unifies features across the whole build, so that one flag would have + silently switched every `flate2` user — blueline's own gunzip in + `safe_extract` and ureq's HTTP gzip — from miniz_oxide to zlib-rs. + `deflate-flate2` defers the backend choice to flate2's own default, + keeping the production decompression path on miniz_oxide exactly as + before. The gzip-bomb scenario fixture caught the switch on the first + run: zlib-rs at `Compression::fast()` packed the 200 MiB bomb to + ~2 MiB where miniz_oxide packs it to ~200 KiB, and the fixture's + "arrives small" assertion failed. zopfli leaves the dependency tree + with this choice (it was only reachable through the meta-feature). ## [0.3.1] - 2026-09-21 ### Added diff --git a/Cargo.lock b/Cargo.lock index 34c35b4..ead334d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -85,15 +85,6 @@ version = "1.0.104" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "330a5ed07fa54e4702c9d6c4174f74427fc0ef6e214bbd677ae50a5099946470" -[[package]] -name = "arbitrary" -version = "1.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c3d036a3c4ab069c7b410a2ce876bd74808d2d0888a82667669f8e783a898bf1" -dependencies = [ - "derive_arbitrary", -] - [[package]] name = "assert_cmd" version = "2.2.2" @@ -192,12 +183,6 @@ dependencies = [ "serde_core", ] -[[package]] -name = "bumpalo" -version = "3.20.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72f5acc6cb2ba439de613abc23857ec3d78374d8ed5ac84e9d11336e87da8649" - [[package]] name = "cc" version = "1.4.2" @@ -214,6 +199,12 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" +[[package]] +name = "cfg_aliases" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f079e83a288787bcd14a6aea84cee5c87a67c5a3e660c30f557a3d24761b3527" + [[package]] name = "clap" version = "4.6.7" @@ -289,12 +280,6 @@ dependencies = [ "cfg-if", ] -[[package]] -name = "crossbeam-utils" -version = "0.8.22" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "61803da095bee82a81bb1a452ecc25d3b2f1416d1897eb86430c6159ef717c17" - [[package]] name = "crossterm" version = "0.29.0" @@ -328,17 +313,6 @@ dependencies = [ "typenum", ] -[[package]] -name = "derive_arbitrary" -version = "1.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e567bd82dcff979e4b03460c307b3cdc9e96fde3d73bed1496d2bc75d9dd62a" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.119", -] - [[package]] name = "difflib" version = "0.4.0" @@ -786,6 +760,18 @@ dependencies = [ "simd-adler32", ] +[[package]] +name = "nix" +version = "0.31.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf20d2fde8ff38632c426f1165ed7436270b44f199fc55284c38276f9db47c3d" +dependencies = [ + "bitflags", + "cfg-if", + "cfg_aliases", + "libc", +] + [[package]] name = "normalize-line-endings" version = "0.3.0" @@ -1402,6 +1388,12 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5d99f8c9a7727884afe522e9bd5edbfc91a3312b36a77b5fb8926e4c31a41801" +[[package]] +name = "typed-path" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e28f89b80c87b8fb0cf04ab448d5dd0dd0ade2f8891bae878de66a75a28600e" + [[package]] name = "typenum" version = "1.20.1" @@ -1797,19 +1789,16 @@ dependencies = [ [[package]] name = "zip" -version = "2.4.2" +version = "9.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fabe6324e908f85a1c52063ce7aa26b68dcb7eb6dbc83a2d148403c9bc3eba50" +checksum = "a954114b47a54df49f83d2722814e86b23d4bb1b54a1b90fb7d92b41619ddc22" dependencies = [ - "arbitrary", "crc32fast", - "crossbeam-utils", - "displaydoc", "flate2", "indexmap", "memchr", - "thiserror", - "zopfli", + "nix", + "typed-path", ] [[package]] @@ -1823,15 +1812,3 @@ name = "zmij" version = "1.0.23" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "29666d0abbfad1e3dc4dcf6144730dd3a3ab225bbbdac83319345b1b44ccfc1b" - -[[package]] -name = "zopfli" -version = "0.8.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f05cd8797d63865425ff89b5c4a48804f35ba0ce8d125800027ad6017d2b5249" -dependencies = [ - "bumpalo", - "crc32fast", - "log", - "simd-adler32", -] diff --git a/Cargo.toml b/Cargo.toml index 7791591..d6f26bd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,7 +18,14 @@ tar = "0.4" flate2 = "1" sha2 = "0.10" base64 = "0.22" -zip = { version = "2", default-features = false, features = ["deflate"] } +# `deflate-flate2`, not the `deflate` meta-feature: `deflate` also enables +# `flate2/zlib-rs`, and Cargo unifies features across the build, so that one +# flag silently switches every flate2 user — blueline's own gunzip in +# `safe_extract` and ureq's HTTP gzip — from miniz_oxide to zlib-rs. +# `deflate-flate2` picks no backend; flate2's own default (`rust_backend`) +# still supplies miniz_oxide, keeping the production decompression path +# byte-identical to what shipped before the zip 9 bump. +zip = { version = "9", default-features = false, features = ["deflate-flate2"] } rusqlite = { version = "0.32.1", features = ["bundled"] } rusqlite_migration = "1.3.1" semver = "1" @@ -52,18 +59,37 @@ lto = true # gigabytes across a hundred stale incremental session caches. Line tables keep # a failing test pointing at the right source line, which is all a panic needs, # and they cost a fraction of the full DWARF. +# +# `opt-level = 1` is the other half of that. It is not here for the shipped +# binary, which is `[profile.release]` and untouched; it is here because a test +# suite that runs unoptimized code measures the absence of optimization rather +# than the code. Measured on the dev box at `fae8050`, interleaved A/B, see +# `PERF.md` section 10: the gate's test stage went 46.4 s to 21 s wall and +# 53.2 s to 15.2 s CPU, and peak RSS 342 MB to 254 MB. It is level 1 and not 2 +# because the whole win is already banked at 1 and 2 costs more compile time for +# no measured return. [profile.dev] +opt-level = 1 debug = "line-tables-only" # `cargo test` builds its own profile, and it inherits nothing from `[profile.dev]` # unless it is stated here, so both halves have to be set. [profile.test] +opt-level = 1 debug = "line-tables-only" # Dependencies are only ever linked in. Nobody steps through `serde_json`, so # emitting debug info for it only makes the linker read more than it has to. +# +# They are optimized because they run during the tests: `tests/scenarios.rs` +# spends its time in sha2, flate2, walkdir and SQLite inside the spawned +# binary, and that binary is mostly dependency code. Measured, optimizing the +# dependencies and leaving this crate at 0 took the 22 attack scenarios from +# 13.7 s to 6.2 s on their own. [profile.dev.package."*"] +opt-level = 1 debug = false [profile.test.package."*"] +opt-level = 1 debug = false diff --git a/PERF.md b/PERF.md index c2e9d3b..738ea47 100644 --- a/PERF.md +++ b/PERF.md @@ -1,10 +1,12 @@ # Blueline performance report -Status: measured 2026-10-11 against v0.3.1 (`2d7864e`), a tree that predates -this file. Items 1, 2 and 4 of section 7 are **implemented** on the -`perf/overlap-and-window` branch; the rest are not. Every number below was -measured on the dev box, and every one carries the command that produced it or -the file and line it came from. +Status: the runtime numbers were measured 2026-10-11 against v0.3.1 +(`2d7864e`), a tree that predates this file. Items 1, 2 and 4 of section 7 are +**implemented** on the `perf/overlap-and-window` branch, item 3 was finished on +2026-10-19, and section 10 is the dev-gate work, also implemented. Item 5 +describes a file that no longer exists. Every number below was measured on the +dev box, and every one carries the command that produced it or the file and +line it came from. Read this before starting work. Section 8 lists what is *not* known, and two of the five items in section 7 rest on code reading rather than measurement. @@ -126,6 +128,9 @@ incremental compilation. ## 5. The dev gate +Superseded by section 10, which re-measured this after the dev-profile change. +Kept because the shape of the problem is still what it was. + `./scripts/verify.sh`, default stage set, warm build cache: | stage | wall | CPU | peak RSS | @@ -209,14 +214,16 @@ the item listed are all preserved and pinned by tests. - Target: 717 ms wall becomes about 175 ms on a 200-file package, CPU unchanged at 45 ms. -### 2. Parallelise the `ci.rs` evaluation loop — IMPLEMENTED (npm/cargo half) +### 2. Parallelise the `ci.rs` evaluation loop — IMPLEMENTED `src/ci.rs:296` and `src/ci.rs:409` called `evaluate_package` one package at a time. 100 packages at 560 ms each is about 56 s of network waiting. The lockfile lane now runs up to eight workers, each with its own `ReviewContext` and its own database handle, results ordered by candidate index. **The AUR -half (`evaluate_aur_ci_diff`) is still serial** and is not covered by this -item's implementation. +half (`evaluate_aur_ci_diff`) now runs through the same driver** with an +empty integrity map: same worker cap, same per-worker context and store +handle, same candidate-order guarantee, pinned by +`tests/aur_ci_parallel.rs` (slow-first pin still reported first). - The store is already opened in WAL mode with a 5 s busy timeout at `src/store.rs:389`, so concurrent writers have a path. @@ -228,15 +235,27 @@ item's implementation. failed run now records `record_verified` and advisory-cache entries for packages the serial lane never reached. -### 3. Cap the incremental cache — PARTIALLY IMPLEMENTED +### 3. Cap the incremental cache — IMPLEMENTED, in two halves + +`[profile.dev]`/`[profile.test]` set `debug = "line-tables-only"` and emit no +debug info for dependencies, which shrank every cache and the 100 MB test +binary. That half does not stop them accumulating: cargo has no knob for the +count and `cargo clean` has no `--incremental`, so eviction has to be a script. +`scripts/prune_incremental.sh` is that script. It judges a session by the +newest file *inside* it rather than the directory's own mtime, because cargo +touches the directory when it reuses a session, which makes a week-dead cache +look recently used. Dry run by default; `--apply` to delete. -`[profile.dev]`/`[profile.test]` now set `debug = "line-tables-only"` and emit -no debug info for dependencies. That **shrinks each of the 101 caches** and the -100 MB test binary; it does **not** stop them accumulating. Unbounded -incremental retention is what accumulates them, and nothing in this PR sets -`incremental = false` or bounds the cache count — cargo has no such knob, so a -real fix is periodic eviction (`cargo clean` in a hook, or a size-capped -`CARGO_TARGET_DIR`). Do not read the current diff as delivering this item. +Measured 2026-10-18 at `fae8050`: `target/debug/incremental` was 4.1 GB across +251 sessions, 3.2 GB of them blueline's own 58. `cargo mutants` sets +`RUSTFLAGS`, which changes the fingerprint, so every mutation run writes a +fresh session; single sessions of 288 MB, 272 MB and 269 MB were all three to +four days old. 107 sessions older than three days totalled 1982 MB. + +Deleting an incremental session cannot invalidate a built artifact. Cargo's +freshness check reads the fingerprint files in `.fingerprint`, not the session +directories; a `cargo test --no-run` after `--days 0 --apply` on the lab tree +was still a 0.46 s no-op. Worst case is one rebuild. - Do not touch `[profile.release]`: `lto` and `strip` stay. - CI builds with `--locked`, so this must not move `Cargo.lock` — it does not. @@ -316,10 +335,12 @@ Read these before trusting anything above. a 3.8 GB tmpfs, RAM-backed, and extraction writes there. Blueline's own 512 MiB extraction cap held and refused. Because `/tmp` is tmpfs, a large package burns RAM as tmpfs pages on any machine configured this way. -- **The toolchain pin is inert here.** `rustup` is not installed, so the 1.97.1 - channel in `rust-toolchain.toml` did not apply and everything built with - Fedora's 1.98.1. `AGENTS.md` already flags this as a CI-fidelity risk. The - numbers above are 1.98.1 numbers. +- **The toolchain pin is no longer inert here, so the numbers above are the odd + ones out.** This line said `rustup` was missing and everything built with + Fedora's 1.98.1. As of 2026-10-18 rustup 1.29.1 is installed and the 1.97.1 + channel in `rust-toolchain.toml` binds, so sections 1 to 8 are 1.98.1 + numbers and section 10 is 1.97.1. Re-measure before quoting a section 1 to + 8 figure as current. - **The test suite was not run under mutation testing.** No claim here is about mutant coverage. @@ -342,6 +363,17 @@ BIN=target/debug/deps/blueline- /usr/bin/time -v $BIN \ registry::npm::tests::the_packument_memo_is_bounded_and_the_estimate_is_not_a_fig_leaf \ --exact + +# Every test binary on its own, no cargo in the way (section 10.1). +# Resolve the newest executable per target: target/debug/deps keeps several +# stale hashed binaries and measuring one of those reports an old build. +for b in $(ls -t target/debug/deps/blueline-* target/debug/deps/*-* | head -40); do + [ -x "$b" ] && [ -z "$(echo "$b" | grep -E '\.(d|o|rlib)$')" ] || continue + printf '%s %s tests\n' "$b" "$("$b" --list 2>/dev/null | grep -c ': test$')" +done + +# Stale incremental caches. Dry run unless --apply. +scripts/prune_incremental.sh --days 3 ``` ### Running the test suite safely @@ -360,4 +392,170 @@ scripts/perf_watchdog.sh 180 40 cargo test --all-targets --locked It polls the process-group size every second, aborts at the cap or the wall limit, and reports the peak. Use it rather than a bare `cargo test` when experimenting near process-spawning code (the `git archive` path in the AUR -registry adapter is the one that actually forks here). \ No newline at end of file +registry adapter is the one that actually forks here). + +Measured again on 2026-10-19 at `fae8050` with the section 10 profile: the +suite still peaks at **6** processes and passes in **20 to 24 s**. + +--- + +## 10. The dev gate, measured 2026-10-19 + +Sections 1 to 8 are about the shipped binary. This one is about the time and +the memory a developer spends before the shipped binary is ever involved. All of +it is `opt-level`, one script, and `Cargo.toml` lines, measured at `fae8050` on +the dev box under 1.97.1. + +### 10.1 What the gate cost + +`./scripts/verify.sh`, warm build cache, before: + +| stage | wall | CPU | peak RSS | +|---|---|---|---| +| `cargo fmt --all -- --check` | 1.4 s | 1.1 s | 69 MB | +| `cargo clippy --all-targets --locked -- -D warnings` | 0.7 s | 0.4 s | 86 MB | +| `cargo test --all-targets --locked` | **46.4 s** | **53.2 s** | **342 MB** | +| `scripts/check-references.sh` | 0.3 s | 0.2 s | 11 MB | + +Clippy is cheap only because its own cache is warm; cold it was 42.6 s. It is +not the target. The test stage was the whole cost, and it split like this, +timing each test binary directly with no cargo in the way: + +| binary | tests | wall | CPU | peak RSS | +|---|---|---|---|---| +| `blueline` (lib) | 870 | 16.0 s | 29.5 s | 245 to 320 MB | +| `scenarios` | 22 | 13.7 s | 14.2 s | 25 MB | +| `aur_ci_parallel` | 1 | 3.5 s | 0.6 s | 4 MB | +| the other 12 binaries | 84 | 5.6 s | 4.8 s | 21 MB or less each | + +Their sum is 38.8 s against a reported 46.4 s, so about 7.5 s is cargo running +15 binaries one after another plus freshness checks. + +### 10.2 The change + +`[profile.dev]`, `[profile.test]` and both `[profile.*.package."*"]` sections +now set `opt-level = 1`. `[profile.release]` is untouched, so nothing about the +shipped binary moved. Four variants were built on disk outside the repo and +compared interleaved, so a quiet machine could not favour one side: + +| variant | lib 870 | memo test | scenarios | +|---|---|---|---| +| baseline, everything at 0 | 16.0 s | 5.2 s | 13.7 s | +| local crate 1, deps 0 | 13.2 s | 4.6 s | 11.7 s | +| local crate 0, deps 1 | 14.2 s | 5.9 s | **6.2 s** | +| everything at 1 | **5.3 s** | **0.11 s** | **3.8 s** | + +The two halves do different work and both are needed: + +* **Dependencies at 1 buys `scenarios`.** Those 22 tests spawn the real binary, + and its time is sha2, flate2, walkdir, SQLite and the diff engine. +* **The local crate at 1 buys the packument memo test**, 5.2 s to 0.11 s. + Nothing else moves it. Optimizing `serde_json` alone moved it by zero, and + that is the expected result rather than a surprise: serde's serializer is + generic, so it is monomorphized into blueline and compiled as blueline's own + code. The crate that calls it is the crate that has to be optimized. + +### 10.3 What it bought, in the repo + +| | before | after | +|---|---|---| +| `cargo test --all-targets --locked`, wall | 41.7 / 50.6 s | **20.2 / 22.5 / 24.4 s** | +| same, CPU | 49.9 / 54.0 s | **14.1 / 15.1 / 15.4 s** | +| same, peak RSS | 328 / 358 MB | **249 / 272 / 282 MB** | +| whole gate, warm | 48.9 s | **23.8 s** | + +That is **2.1x wall, 3.5x CPU and 27% less peak RSS**, with all 870 tests +passing and `cargo clippy --all-targets --locked -- -D warnings` clean at +opt-level 1. Per binary after: lib 5.6 s and 189 MB, scenarios 4.9 s, +`aur_ci_parallel` 3.3 s of which 2.8 s is waiting. + +### 10.4 What it cost + +| | cold full build | rebuild just blueline | one-file edit, all targets | +|---|---|---|---| +| baseline | 1 m 18 s | 34.9 s | 2.80 s | +| everything at 1 | 5 m 47 s | 129.4 s | 2.91 s | + +`CARGO_BUILD_JOBS=2`, warm registry cache, empty target dir. The rebuild of +this repository at the new profile was 5 m 27 s, which matches. Max single +process during a cold build was 771 MB against 732 MB, so build RAM does not +move. + +The daily loop is the part that matters and it does not get worse: touching one +source file and rebuilding every test target is 2.80 s before and 2.91 s after. +The cost lands once per cold build, and CI's first run after the change, since +every job caches `target/` with `swatinem/rust-cache@v2`. + +It is level 1 and not 2 because the entire win is already banked at 1, and 2 +costs more compile time for no measured return. + +### 10.5 Memory during a dev run + +* One test run writes **390 MB into TMPDIR**, and `/tmp` here is a 3.8 GB + tmpfs, so that is RAM. Sampled high-water and `MemAvailable`: + + | TMPDIR | wall | lowest `MemAvailable` | + |---|---|---| + | `/tmp` (tmpfs) | 44.8 s, 47.6 s | 1189 to 1321 MB | + | disk-backed | 64.7 s | 1493 MB | + + Moving TMPDIR to disk buys about 300 MB of available memory and costs 45% of + the wall clock, because every extracted tarball and every fixture write goes + through the filesystem instead of memory. On this box tmpfs is the right + default. If memory ever becomes the binding constraint, that is the dial, and + the fixture that writes the most is `src/extract.rs:1250` with a 64 MB + payload. +* A third of the test stage's peak RSS is the memo test's 108 MB fixture. It + has to carry more than `MAX_MEMOISED_PACKUMENT_BYTES` (64 MiB) for the + ceiling to bind, so 96 MB of signature payload has to be real and resident. + There is no honest way to shrink it without weakening what it proves. +* `target/debug` was 9.5 GB, of which 4.1 GB was incremental caches (section 7 + item 3) and 3.2 GB was 133 executables of which 16 were current. The rest + were from before the `line-tables-only` change, at 82 to 149 MB each where + the current ones are 10 to 51 MB. `cargo clean` reclaims all of it; the prune + script stops it growing. + +### 10.6 What this did not fix, and what is left + +* `cargo nextest` is now the default in the gate. It overlaps the 15 test + binaries instead of running them one at a time. Measured 2026-10-19 with 8 + threads: the whole gate went from 23.8 s to 11.4 s wall, 3.1x over the + pre-opt-level baseline of 48.9 s. It is installed as a prebuilt binary from + the GitHub releases page rather than built from source, and CI installs it + through `taiki-e/install-action@v2`. +* `tests/aur_ci_parallel.rs:15` sleeps 1500 ms on purpose, to prove the + slow-first candidate is still reported first. That is a real property and + shrinking the delay weakens the test. +* Eight tests in `src/registry/npm.rs` used to spin a mock server in a + `while start.elapsed() < 600ms` loop and then join the thread, so each cost + 600 ms of sleeping however fast the work was. Each loop now exits as soon as + the expected number of requests has been served, with a 50 ms idle fallback + and a 5 s safety deadline. Measured 2026-10-19: `cargo test --lib registry::npm` + single-threaded went 7.11 s to 2.37 s, five consecutive runs all green. +* `test-threads = 8` in `.cargo/config.toml` and passed explicitly to + `cargo nextest run`. On the baseline binaries: 1 thread 36 to 42 s, 4 + (default) 17 to 19 s at 275 MB, 8 threads 13.9 to 14.5 s at 387 to 449 MB. + 8 threads is 1.25x for 40 to 60% more peak RSS, which on a shared box is a + trade rather than a win, but on a dedicated CI runner it is free. + +### 10.7 Limits of section 10 + +* **The box has other tenants.** Load average ran 4 to 11 during the A/B, with + firefox, synara and several agent servers resident. Every comparison is + interleaved A/B and reported as a median of 2 to 3 rounds, but treat a single + number as plus or minus 20%. The baseline test stage itself ranged 41.7 s to + 50.6 s across two interleaved rounds. +* **No profiler.** `perf`, `valgrind` and `strace` are still absent, so every + conclusion here is from timing whole binaries rather than from a call graph. + The monomorphization claim is the one that most deserves a profiler, though + the A/B supports it directly: optimizing `serde_json` moved the memo test by + zero. +* **Two lab runs skipped a test.** The profile variants were built from + `git ls-files | tar`, which has no `.git`, so the two tests that shell out to + `git show HEAD:` fail there + (`ci::tests::cargo_missing_base_returns_empty_cargo_toml` and + `ci_pypi_lockfile_hash_verification`). Both were skipped in the lab and pass + in the repo. Not product bugs. +* **Cold build numbers are `-j2` with a warm registry cache.** A CI runner with + a cold registry will be slower; the ratio should hold and the absolutes will + not. \ No newline at end of file diff --git a/references/features/extract.md b/references/features/extract.md index 874ceb6..c735b8e 100644 --- a/references/features/extract.md +++ b/references/features/extract.md @@ -75,9 +75,13 @@ hard error *before* any byte is returned from `fetch_tarball`. `@LongLink` flood from being free. - **The zip path has extra rejections with no tar equivalent:** encrypted entries, `CompressionMethod` not `Stored | Deflated`, `enclosed_name()` None, - normalized-path duplicates (`a.txt` vs `./a.txt`), and a - `declared_size != actual_written` lie check. Modes are hard-set to - `0o644`/`0o755` rather than stripped. + raw entry names starting with `/` or containing `\` or `:` (zip 9's + `enclosed_name` sanitizes those away instead of refusing, so + `safe_extract_wheel` checks the raw bytes itself before any + sanitization), undecodable entry names (`name()` is a `Result` in zip 9 + and a decode failure refuses the archive), normalized-path duplicates + (`a.txt` vs `./a.txt`), and a `declared_size != actual_written` lie + check. Modes are hard-set to `0o644`/`0o755` rather than stripped. - **The error *variant* is load-bearing.** npm packument oversize maps to `BluelineError::Manifest`; every other oversize maps to `ExtractionLimit`. Adapters `.map_err` on `ExtractionLimit(_)` to rewrite the message — change diff --git a/scripts/prune_incremental.sh b/scripts/prune_incremental.sh new file mode 100755 index 0000000..45b340f --- /dev/null +++ b/scripts/prune_incremental.sh @@ -0,0 +1,98 @@ +#!/usr/bin/env bash +# +# prune_incremental.sh - delete stale cargo incremental compilation caches. +# +# Cargo keeps one session directory per build fingerprint under +# `target//incremental/-/` and never removes any of them. +# There is no knob for it: `incremental = false` is all-or-nothing, and +# `cargo clean` has no `--incremental`. On this box that had grown to 4.1 GB +# across 251 sessions, 3.2 GB of them blueline's own, because `cargo mutants` +# sets RUSTFLAGS and so writes a fresh session on every mutation run (single +# sessions of 288 MB were not unusual). +# +# A stale session is not a cache hit waiting to happen: the fingerprint that +# produced it no longer exists, so nothing will ever read it again. Deleting it +# costs one rebuild and nothing else. +# +# scripts/prune_incremental.sh # report only, deletes nothing +# scripts/prune_incremental.sh --apply # delete what is older than 7d +# scripts/prune_incremental.sh --days 3 # tighter or looser window +# scripts/prune_incremental.sh --target-dir D # somewhere other than ./target +# +# Dry run is the default on purpose. This is the only script here that deletes +# anything, and "delete my build cache" should never be something a command +# does behind your back. +# +# A session is judged by the newest file inside it, not by the directory's own +# mtime: cargo touches the directory when it reuses a session, so a directory +# mtime says "recently used" for caches that stopped being used a week ago. + +set -uo pipefail + +DAYS=7 +APPLY=0 +TARGET_DIR="" + +usage() { + sed -n '3,24p' "${BASH_SOURCE[0]}" | sed 's/^# \{0,1\}//' +} + +while [ "$#" -gt 0 ]; do + case "$1" in + --apply) APPLY=1 ;; + --days) shift; DAYS="${1:?--days needs a number}" ;; + --target-dir) shift; TARGET_DIR="${1:?--target-dir needs a path}" ;; + -h|--help) usage; exit 0 ;; + *) echo "prune_incremental.sh: unknown argument '$1'" >&2; usage >&2; exit 2 ;; + esac + shift +done + +case "$DAYS" in + ''|*[!0-9]*) echo "prune_incremental.sh: --days must be a whole number, got '$DAYS'" >&2; exit 2 ;; +esac + +REPO_ROOT="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)" +cd "$REPO_ROOT" +[ -n "$TARGET_DIR" ] || TARGET_DIR="target" + +if [ ! -d "$TARGET_DIR" ]; then + echo "prune_incremental.sh: no $TARGET_DIR directory here (run from the repo root)" >&2 + exit 1 +fi + +NOW=$(date +%s) +CUTOFF=$((NOW - DAYS * 86400)) +TOTAL_BYTES=0 +TOTAL_DIRS=0 +KEPT=0 + +for dir in "$TARGET_DIR"/*/incremental/*/; do + [ -d "$dir" ] || continue + # Newest mtime of anything inside, which is when cargo last wrote here. + newest=$(find "$dir" -type f -printf '%T@\n' 2>/dev/null | sort -rn | head -1) + [ -n "$newest" ] || newest=0 + newest="${newest%%.*}" + if [ "$newest" -ge "$CUTOFF" ]; then + KEPT=$((KEPT + 1)) + continue + fi + size_kb=$(du -sk "$dir" 2>/dev/null | cut -f1) + size_kb=${size_kb:-0} + TOTAL_BYTES=$((TOTAL_BYTES + size_kb)) + TOTAL_DIRS=$((TOTAL_DIRS + 1)) + if [ "$APPLY" -eq 1 ]; then + rm -rf -- "$dir" || echo "prune_incremental.sh: could not remove $dir" >&2 + else + printf 'would remove %6s MB %s\n' "$((size_kb / 1024))" "$dir" + fi +done + +printf '\n%d session(s) newer than %dd kept, %d session(s) %s totalling %s MB\n' \ + "$KEPT" "$DAYS" "$TOTAL_DIRS" \ + "$([ "$APPLY" -eq 1 ] && echo removed || echo older than ${DAYS}d)" \ + "$((TOTAL_BYTES / 1024))" + +if [ "$APPLY" -ne 1 ] && [ "$TOTAL_DIRS" -gt 0 ]; then + echo "nothing was deleted. re-run with --apply to do it." +fi \ No newline at end of file diff --git a/scripts/verify.sh b/scripts/verify.sh index 314da97..b665fe2 100755 --- a/scripts/verify.sh +++ b/scripts/verify.sh @@ -105,7 +105,15 @@ echo "stages: $stages" # resolve a different dependency graph than CI). stage "Format" cargo fmt --all -- --check stage "Clippy" cargo clippy --all-targets --locked -- -D warnings -stage "Tests" cargo test --all-targets --locked +# nextest is preferred when it is installed: it overlaps the 15 test binaries +# instead of running them one at a time, which is the difference between 22 s +# and about 12 s on the dev box. Fall back to `cargo test` so a checkout +# without nextest still passes the gate identically (same flags, same tests). +if command -v cargo-nextest >/dev/null 2>&1; then + stage "Tests" cargo nextest run --all-targets --locked --test-threads 8 +else + stage "Tests" cargo test --all-targets --locked +fi # The agent-facing map is prose, so no Rust gate can see it rot. This resolves # every citation in references/ against the tracked tree. Not part of "the Rust diff --git a/src/ci.rs b/src/ci.rs index 3126502..be7bf78 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -352,16 +352,18 @@ struct Candidate { /// Upper bound on review workers. /// -/// A lockfile lane is network-bound, so more workers is faster right up until -/// the registry starts refusing connections. Eight covers a slow round trip -/// several times over on a laptop and stays well inside what one client asks a -/// registry to handle. +/// Both CI lanes are network-bound (registry fetches on the lockfile half, +/// RPC plus `git clone`/`git archive` on the AUR half), so more workers is +/// faster right up until the remote starts refusing connections. Eight covers +/// a slow round trip several times over on a laptop and stays well inside +/// what one client asks a registry to handle. /// /// There is no rate limiting or jitter, so a burst lands as-is: up to eight /// concurrent packument and tarball fetches, a few dozen requests in flight. /// That is unremarkable against npm or crates.io and a spike against a small /// self-hosted mirror, which `--registry` permits. Lower the cap before -/// pointing this lane at one. +/// pointing this lane at one. AUR git clones are heavier still; the same cap +/// applies, and the budget check refuses before any work starts. const MAX_CI_REVIEW_WORKERS: usize = 8; fn evaluate_candidate( @@ -509,11 +511,10 @@ fn evaluate_candidates( /// The stored integrity for each item is the sha256 of the pinned commit's /// `git archive` bytes, so the audit trail stays bound to the commit hash. /// -/// This half of the lane is still serial, unlike the lockfile half: AUR -/// entries are capped at 4096 and each review shells out to `git archive`, -/// so the worst case is real. Parallelising it is the same shape as -/// `evaluate_candidates`, minus the registry round trips that make the -/// lockfile lane network-bound. +/// Items come back in pin-file order through the shared `evaluate_candidates` +/// driver, so this half matches the lockfile half: each worker gets its own +/// `ReviewContext` and database handle, and every package is drained even +/// after one fails. fn evaluate_aur_ci_diff( base_content: &str, head_content: &str, @@ -548,20 +549,23 @@ fn evaluate_aur_ci_diff( check_evaluation_budget(added, upgraded, policy.ci.max_evaluations)?; - let mut items = Vec::new(); - let mut max_band = VerdictBand::Low; - for (name, old_version, new_version) in evals { - let mut rctx = crate::recursive::ReviewContext::new(policy, ctx.bases.clone()); - let (verdict, _, _, _) = - evaluate_package(&name, &new_version, ctx.ecosystem, store, policy, &mut rctx)?; - max_band = update_max_band(max_band, verdict.band); - items.push(CiReviewItem { + // Fixed here, before any review starts, so the report order never depends + // on which review finishes first. AUR pins carry no integrity map, so the + // shared driver runs with an empty one. + let candidates: Vec = evals + .into_iter() + .map(|(name, old_version, new_version)| Candidate { name, old_version, new_version, is_dev: false, - verdict, - }); + }) + .collect(); + let empty_integrity = std::collections::HashMap::new(); + let items = evaluate_candidates(&candidates, ctx, store, policy, &empty_integrity)?; + let mut max_band = VerdictBand::Low; + for item in &items { + max_band = update_max_band(max_band, item.verdict.band); } let fail_threshold = ctx .fail_on diff --git a/src/diff.rs b/src/diff.rs index 93adda7..dfe217b 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -45,6 +45,7 @@ pub struct Delta { pub modified_lifecycle_scripts: Vec, pub new_dependencies: Vec<(String, String)>, pub modified_dependencies: Vec<(String, String, String)>, + pub removed_dependencies: Vec<(String, String)>, pub binding_gyp_added: bool, } @@ -218,6 +219,7 @@ pub fn compute_delta( let mut new_dependencies = Vec::new(); let mut modified_dependencies = Vec::new(); + let mut removed_dependencies = Vec::new(); let target_deps = collect_all_dependencies(target_manifest); if let Some(base_m) = baseline_manifest { @@ -231,6 +233,11 @@ pub fn compute_delta( new_dependencies.push((dep.clone(), ver.clone())); } } + for (dep, ver) in &base_deps { + if !target_deps.contains_key(dep) { + removed_dependencies.push((dep.clone(), ver.clone())); + } + } } else { for (dep, ver) in &target_deps { new_dependencies.push((dep.clone(), ver.clone())); @@ -261,6 +268,7 @@ pub fn compute_delta( modified_lifecycle_scripts, new_dependencies, modified_dependencies, + removed_dependencies, binding_gyp_added, }) } @@ -1191,4 +1199,143 @@ mod tests { delta.new_binaries ); } + + /// Every dependency lands in exactly one of the three lists, decided by + /// presence on each side and then by whether the version moved. + /// + /// The four fixtures are one per arm of the classification, so a mutant that + /// reorders the `else`, drops the version equality check, or inverts + /// `contains_key` moves a name and fails a literal assertion below rather + /// than silently shrinking a list. + #[test] + fn a_dependency_lands_in_exactly_one_of_the_three_lists() { + fn manifest(deps: &[(&str, &str)]) -> PackageJson { + PackageJson { + name: "test".into(), + version: "1.0.0".into(), + gypfile: None, + scripts: BTreeMap::new(), + dependencies: deps + .iter() + .map(|(k, v)| ((*k).to_string(), (*v).to_string())) + .collect(), + optional_dependencies: BTreeMap::new(), + peer_dependencies: BTreeMap::new(), + } + } + + let old_dir = tempfile::tempdir().unwrap(); + let new_dir = tempfile::tempdir().unwrap(); + let base_m = manifest(&[("kept", "1.0.0"), ("bumped", "1.0.0"), ("dropped", "0.1.0")]); + let target_m = manifest(&[("kept", "1.0.0"), ("bumped", "2.0.0"), ("added", "3.0.0")]); + + let delta = compute_delta( + Some(old_dir.path()), + Some(&base_m), + Some("1.0.0"), + new_dir.path(), + &target_m, + "1.1.0", + ) + .unwrap(); + + assert_eq!( + delta.new_dependencies, + vec![("added".to_string(), "3.0.0".to_string())], + "only the name absent from the baseline is new" + ); + // The triple carries both versions, because the heuristic tells a range + // bump from a move onto a URL, and either end can be the URL. + assert_eq!( + delta.modified_dependencies, + vec![( + "bumped".to_string(), + "1.0.0".to_string(), + "2.0.0".to_string() + )], + "a version change is a modification, not an add and a removal" + ); + assert_eq!( + delta.removed_dependencies, + vec![("dropped".to_string(), "0.1.0".to_string())], + "a name absent from the target is removed, at the baseline version" + ); + // `kept` is in both manifests at the same version, so it is in none of + // them. The `assert_eq!`s above pin the lists to exact contents, so this + // is already implied; it is spelled out because "unchanged" is the case + // with no list of its own, and a reader has to be able to see it here. + let everywhere = |name: &str| { + delta.new_dependencies.iter().any(|(d, _)| d == name) + || delta + .modified_dependencies + .iter() + .any(|(d, _, _)| d == name) + || delta.removed_dependencies.iter().any(|(d, _)| d == name) + }; + assert!( + !everywhere("kept"), + "an unchanged dependency must be reported by no list" + ); + } + + /// No baseline manifest: there is nothing to subtract a dependency set from, + /// so the whole target dependency set is new and nothing can be modified or + /// removed. + /// + /// The `else` arm is the one that decides this. A mutant that falls through + /// to the subtraction arm with an empty baseline map still yields the same + /// `new_dependencies`, but the other two lists are only empty because the + /// branch is never taken, and that is what the emptiness assertions carry. + #[test] + fn without_a_baseline_every_dependency_is_new_and_nothing_is_modified_or_removed() { + fn manifest(deps: &[(&str, &str)]) -> PackageJson { + PackageJson { + name: "test".into(), + version: "1.0.0".into(), + gypfile: None, + scripts: BTreeMap::new(), + dependencies: deps + .iter() + .map(|(k, v)| ((*k).to_string(), (*v).to_string())) + .collect(), + optional_dependencies: BTreeMap::new(), + peer_dependencies: BTreeMap::new(), + } + } + + let old_dir = tempfile::tempdir().unwrap(); + let new_dir = tempfile::tempdir().unwrap(); + let target_m = manifest(&[("alpha", "1.0.0"), ("beta", "2.0.0")]); + + // A baseline *root* with no baseline *manifest*, so the branch under test + // is keyed on the manifest and not on the tree. + let delta = compute_delta( + Some(old_dir.path()), + None, + None, + new_dir.path(), + &target_m, + "1.0.0", + ) + .unwrap(); + + assert_eq!( + delta.new_dependencies, + vec![ + ("alpha".to_string(), "1.0.0".to_string()), + ("beta".to_string(), "2.0.0".to_string()) + ], + "with no baseline every target dependency is new" + ); + assert!( + delta.modified_dependencies.is_empty(), + "nothing exists to modify a version against: {:?}", + delta.modified_dependencies + ); + assert!( + delta.removed_dependencies.is_empty(), + "nothing exists to remove: {:?}", + delta.removed_dependencies + ); + } } diff --git a/src/extract.rs b/src/extract.rs index c5379af..870fb6a 100644 --- a/src/extract.rs +++ b/src/extract.rs @@ -328,7 +328,14 @@ pub fn safe_extract_wheel( .by_index(idx) .map_err(|e| BluelineError::Extraction(format!("reading zip entry {idx}: {e}")))?; - let raw_name = file.name().to_string(); + let raw_name = file + .name() + .map_err(|e| { + BluelineError::Extraction(format!( + "zip entry {idx} has a name that cannot be decoded: {e}" + )) + })? + .to_string(); if raw_name.as_bytes().contains(&0) { return Err(BluelineError::Extraction(format!( @@ -336,6 +343,24 @@ pub fn safe_extract_wheel( ))); } + // zip 9's `enclosed_name` applies Windows path semantics: it strips a + // leading root or drive prefix instead of refusing the entry, and it + // splits on backslashes. A raw `/etc/evil` therefore comes back + // contained as `etc/evil`, and `foo\bar` as `foo/bar`. Containment is + // not the bar here: the name itself is refused, checked against the + // raw bytes before any sanitization can hide what the archive asked + // for. + if raw_name.starts_with('/') { + return Err(BluelineError::Extraction(format!( + "entry `{raw_name}` fails enclosed_name check (absolute or traversal)" + ))); + } + if raw_name.contains('\\') || raw_name.contains(':') { + return Err(BluelineError::Extraction(format!( + "entry path `{raw_name}` containing backslash or colon rejected" + ))); + } + if file.encrypted() { return Err(BluelineError::Extraction(format!( "encrypted entry `{raw_name}` rejected" @@ -1680,8 +1705,12 @@ mod wheel_tests { let err = safe_extract_wheel(&mutated, dir.path(), &ExtractionLimits::default()).unwrap_err(); let msg = err.to_string().to_ascii_lowercase(); + // zip 9 refuses unsupported methods inside `by_index` itself ("not + // supported"), before our own match on `file.compression()` can run; + // either phrasing names the same rejection. assert!( - msg.contains("unsupported") && (msg.contains("compression") || msg.contains("method")), + (msg.contains("unsupported") || msg.contains("not supported")) + && (msg.contains("compression") || msg.contains("method")), "{err}" ); } diff --git a/src/heuristic.rs b/src/heuristic.rs index 2a67e03..ad63f32 100644 --- a/src/heuristic.rs +++ b/src/heuristic.rs @@ -1862,9 +1862,104 @@ fn dependency_findings(delta: &Delta, policy: &Policy) -> Vec { } } + // Dependency swap / rename-under-a-new-name detection. A name that was in + // the baseline dependency set disappearing at the same time that a + // near-identical name appears in the added set is the shape of a typosquat: + // the payload ships under a spelling the reviewer has never seen, while the + // familiar name quietly drops out. A bare drop shrinks the graph and is + // defensible on its own, so only the pair is scored. + // + // Swapping a dead dependency for a maintained fork is common, so this stays + // a disclosure (Low): it names the pair and leaves the verdict alone unless + // policy tightens thresholds. Names are compared case- and + // separator-insensitively, and a single edit is treated as "near + // identical". No claim is made that the swap is malicious. + let mut swaps: Vec = Vec::new(); + for (removed, _) in &delta.removed_dependencies { + for (added, _) in &delta.new_dependencies { + if removed == added { + continue; + } + let rn = normalize_dep_name(removed); + let an = normalize_dep_name(added); + if rn.is_empty() || an.is_empty() { + continue; + } + if levenshtein_at_most_one(&rn, &an) { + swaps.push(format!("{removed} -> {added}")); + } + } + } + if !swaps.is_empty() { + findings.push(Finding { + rule_id: "R04_DEPENDENCY_SWAP".into(), + severity: VerdictBand::Low, + title: format!( + "{} dependency rename candidate{} under a new name", + swaps.len(), + if swaps.len() == 1 { "" } else { "s" } + ), + description: format!( + "A removed dependency has a near-identical name among the added dependencies ({}). This is the rename-under-a-new-name shape associated with typosquats, but it is also the ordinary shape of replacing a dead dependency with a maintained fork, so this finding discloses the pair and leaves the verdict to policy.", + swaps.join(", ") + ), + }); + } + findings } +fn normalize_dep_name(name: &str) -> String { + name.chars() + .filter(|c| c.is_ascii_alphanumeric()) + .map(|c| c.to_ascii_lowercase()) + .collect() +} + +/// Levenshtein distance of at most one between two already-normalized strings. +fn levenshtein_at_most_one(a: &str, b: &str) -> bool { + if a == b { + return true; + } + let a: Vec = a.chars().collect(); + let b: Vec = b.chars().collect(); + if a.len().abs_diff(b.len()) > 1 { + return false; + } + // Ensure `a` is the longer (or equal) so only one branch handles deletion. + let (long, short) = if a.len() >= b.len() { + (&a, &b) + } else { + (&b, &a) + }; + + let mut i = 0usize; + let mut j = 0usize; + let mut edits = 0usize; + while j < short.len() { + if i < long.len() && long[i] == short[j] { + i += 1; + j += 1; + } else { + edits += 1; + if edits > 1 { + return false; + } + if long.len() > short.len() { + // deletion in the shorter / insertion in the longer + i += 1; + } else { + i += 1; + j += 1; + } + } + } + if i < long.len() { + edits += 1; + } + edits <= 1 +} + /// Every rule that reads the provenance report. Split out from the main /// evaluation so the policy gate can be tested on a report alone. fn provenance_findings( @@ -2061,6 +2156,7 @@ mod tests { new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let verdict = evaluate_with_policy( @@ -2106,6 +2202,7 @@ mod tests { new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let verdict = evaluate_with_policy( @@ -2156,6 +2253,7 @@ mod tests { new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let verdict = evaluate_with_policy( @@ -2234,6 +2332,369 @@ mod tests { ); } + #[test] + fn detects_a_dependency_swap_under_a_new_name() { + let mut delta = Delta::default(); + delta + .removed_dependencies + .push(("lodash".into(), "4.17.20".into())); + delta + .new_dependencies + .push(("lodas".into(), "4.17.21".into())); + let findings = dependency_findings(&delta, &Policy::default()); + let swaps: Vec<_> = findings + .iter() + .filter(|f| f.rule_id == "R04_DEPENDENCY_SWAP") + .collect(); + assert_eq!( + swaps.len(), + 1, + "a near-identical added name must be flagged" + ); + assert_eq!( + swaps[0].severity, + VerdictBand::Low, + "benign forks are common, so the swap stays score-neutral" + ); + } + + #[test] + fn a_plain_dependency_drop_does_not_fire_a_swap() { + let mut delta = Delta::default(); + delta + .removed_dependencies + .push(("old-dep".into(), "1.0.0".into())); + // Nothing near-identical is added. + delta + .new_dependencies + .push(("fresh-lib".into(), "1.0.0".into())); + let findings = dependency_findings(&delta, &Policy::default()); + assert!( + !findings.iter().any(|f| f.rule_id == "R04_DEPENDENCY_SWAP"), + "an unrelated drop must not be a typosquat" + ); + } + + #[test] + fn a_normalized_name_match_counts_as_a_swap() { + let mut delta = Delta::default(); + delta + .removed_dependencies + .push(("my-lib".into(), "1.0.0".into())); + delta + .new_dependencies + .push(("MyLib".into(), "1.0.0".into())); + let findings = dependency_findings(&delta, &Policy::default()); + assert!( + findings.iter().any(|f| f.rule_id == "R04_DEPENDENCY_SWAP"), + "case and separator differences are normalized" + ); + } + + /// Every edit shape the swap rule accepts has to keep firing: a typosquat + /// only has to misspell by one edit, and the length-based path in + /// `levenshtein_at_most_one` runs the mismatch branch differently depending + /// on which string is longer. A single substitution pair covers only one of + /// those branches. + #[test] + fn a_swap_fires_on_an_insertion_and_on_a_deletion_too() { + // The added name is one char longer: the loop's mismatch branch takes + // `long.len() > short.len()`. + let mut longer = Delta::default(); + longer + .removed_dependencies + .push(("requests".into(), "2.0.0".into())); + longer + .new_dependencies + .push(("requestss".into(), "1.0.0".into())); + assert!( + dependency_findings(&longer, &Policy::default()) + .iter() + .any(|f| f.rule_id == "R04_DEPENDENCY_SWAP"), + "an inserted character is still one edit away" + ); + + // The added name is one char shorter: the same branch now sees equal + // lengths after the swap, and the tail of `long` is left over. + let mut shorter = Delta::default(); + shorter + .removed_dependencies + .push(("requests".into(), "2.0.0".into())); + shorter + .new_dependencies + .push(("request".into(), "1.0.0".into())); + assert!( + dependency_findings(&shorter, &Policy::default()) + .iter() + .any(|f| f.rule_id == "R04_DEPENDENCY_SWAP"), + "a dropped character is still one edit away" + ); + } + + /// Two edits are past the threshold, and the finding is a disclosure that + /// fires on ordinary fork renames, so the bound is what keeps the noise + /// down. `reqeusts` is a transposition, which Levenshtein scores as two + /// substitutions. + #[test] + fn a_swap_does_not_fire_at_two_edits() { + let mut delta = Delta::default(); + delta + .removed_dependencies + .push(("requests".into(), "2.0.0".into())); + delta + .new_dependencies + .push(("reqeusts".into(), "1.0.0".into())); + let findings = dependency_findings(&delta, &Policy::default()); + assert!( + !findings.iter().any(|f| f.rule_id == "R04_DEPENDENCY_SWAP"), + "two edits out is an unrelated rename, not a typosquat: {:?}", + findings.iter().map(|f| &f.rule_id).collect::>() + ); + } + + /// A name that is all separators normalizes to the empty string, and the + /// empty string is at distance one from every one-character name. Without + /// the guard, `___` dropped for `___` — or for any single-character + /// package — would be reported as a typosquat. + /// + /// Both directions are driven, because the guard is checked against the + /// removed name first and then the added one, and deleting either check + /// leaves the other arm to catch nothing. + #[test] + fn a_name_that_normalizes_to_nothing_never_forms_a_swap() { + let mut empty_removed = Delta::default(); + empty_removed + .removed_dependencies + .push(("___".into(), "1.0.0".into())); + empty_removed + .new_dependencies + .push(("abc".into(), "1.0.0".into())); + let findings = dependency_findings(&empty_removed, &Policy::default()); + assert!( + !findings.iter().any(|f| f.rule_id == "R04_DEPENDENCY_SWAP"), + "an all-separator removed name must not pair with anything: {:?}", + findings.iter().map(|f| &f.rule_id).collect::>() + ); + + let mut empty_added = Delta::default(); + empty_added + .removed_dependencies + .push(("abc".into(), "1.0.0".into())); + empty_added + .new_dependencies + .push(("---".into(), "1.0.0".into())); + let findings = dependency_findings(&empty_added, &Policy::default()); + assert!( + !findings.iter().any(|f| f.rule_id == "R04_DEPENDENCY_SWAP"), + "an all-separator added name must not pair with anything: {:?}", + findings.iter().map(|f| &f.rule_id).collect::>() + ); + + // The two cases above clear on distance alone: an empty name is three + // edits from `abc` whatever the guard does. These two are the ones that + // actually reach the guard, because a one-character name is exactly one + // edit from the empty string. The first only exercises `rn.is_empty()` + // and the second only `an.is_empty()`, so deleting either half of the + // `||` is caught by one of them. + let mut empty_removed_close = Delta::default(); + empty_removed_close + .removed_dependencies + .push(("___".into(), "1.0.0".into())); + empty_removed_close + .new_dependencies + .push(("a".into(), "1.0.0".into())); + let findings = dependency_findings(&empty_removed_close, &Policy::default()); + assert!( + !findings.iter().any(|f| f.rule_id == "R04_DEPENDENCY_SWAP"), + "one edit from the empty string is still not a typosquat: {:?}", + findings.iter().map(|f| &f.rule_id).collect::>() + ); + + let mut empty_added_close = Delta::default(); + empty_added_close + .removed_dependencies + .push(("a".into(), "1.0.0".into())); + empty_added_close + .new_dependencies + .push(("---".into(), "1.0.0".into())); + let findings = dependency_findings(&empty_added_close, &Policy::default()); + assert!( + !findings.iter().any(|f| f.rule_id == "R04_DEPENDENCY_SWAP"), + "one edit from the empty string is still not a typosquat: {:?}", + findings.iter().map(|f| &f.rule_id).collect::>() + ); + } + + /// The count in the title and the pairs in the description are the whole + /// disclosure: a reviewer has to be able to see how many pairs matched + /// without re-running the diff. The title is the only place the count + /// appears, and its singular/plural branch has no other caller, so nothing + /// else in the suite can see it change. Both sides of the branch are pinned, + /// because a title that reads "2 dependency rename candidate" is wrong in a + /// way nothing downstream checks. + #[test] + fn the_swap_finding_counts_its_pairs_and_names_each_one() { + fn delta_with(pairs: &[(&str, &str)]) -> Delta { + let mut delta = Delta::default(); + for (removed, added) in pairs { + delta + .removed_dependencies + .push(((*removed).to_string(), "1.0.0".to_string())); + delta + .new_dependencies + .push(((*added).to_string(), "1.0.0".to_string())); + } + delta + } + fn swap_finding(pairs: &[(&str, &str)]) -> Finding { + dependency_findings(&delta_with(pairs), &Policy::default()) + .into_iter() + .find(|f| f.rule_id == "R04_DEPENDENCY_SWAP") + .expect("a swap pair must produce the finding") + } + + let one = swap_finding(&[("lodash", "lodas")]); + assert_eq!( + one.title, "1 dependency rename candidate under a new name", + "one pair is a candidate" + ); + assert!( + one.description.contains("lodash -> lodas"), + "the description must name the pair: {}", + one.description + ); + + let two = swap_finding(&[("lodash", "lodas"), ("chalk", "chalx")]); + assert_eq!( + two.title, "2 dependency rename candidates under a new name", + "two pairs are candidates, and the count is not the finding count" + ); + for pair in ["lodash -> lodas", "chalk -> chalx"] { + assert!( + two.description.contains(pair), + "the description must name {pair}: {}", + two.description + ); + } + } + + /// A removed name byte-identical to an added one is skipped before any + /// distance is computed. A rename that changed nothing is not a rename, and + /// it reaches the rule as a pair because the two lists are independent — the + /// same name can appear in both when a dependency moves between dependency + /// kinds (regular and optional, say) at one version. + #[test] + fn an_identical_removed_and_added_name_is_not_a_swap() { + let mut delta = Delta::default(); + delta + .removed_dependencies + .push(("lodash".into(), "4.17.20".into())); + delta + .new_dependencies + .push(("lodash".into(), "4.17.20".into())); + let findings = dependency_findings(&delta, &Policy::default()); + assert!( + !findings.iter().any(|f| f.rule_id == "R04_DEPENDENCY_SWAP"), + "the name never changed, so nothing was renamed: {:?}", + findings.iter().map(|f| &f.rule_id).collect::>() + ); + } + + #[test] + fn dep_name_normalization_drops_separators_and_folds_case() { + // The four spellings are the ones npm treats as distinct package names, + // so this is what makes `my-lib` and `MyLib` the same swap candidate. + for name in ["my-lib", "my_lib", "MyLib", "my.lib"] { + assert_eq!(normalize_dep_name(name), "mylib", "{name}"); + } + assert_eq!( + normalize_dep_name("@scope/Name-Pkg"), + "scopenamepkg", + "the scope separator and slash go too" + ); + assert_eq!( + normalize_dep_name("___"), + "", + "a name made only of separators normalizes to nothing, which the swap rule guards on" + ); + } + + /// The predicate the swap rule rests on, driven shape by shape. + /// + /// `levenshtein_at_most_one` is not a general edit-distance function: it + /// compares two already-normalized strings, skips the whole walk when + /// `a == b`, bails on a length gap above one, and then reorders its + /// arguments so only one direction of the mismatch branch is written. Each + /// group below is a mutation that changes an answer only in that shape, and + /// the false cases matter as much as the true ones: the rule fires on + /// ordinary fork renames, so an over-broad predicate is noise on every + /// release that swaps a dependency. + #[test] + fn at_most_one_edit_is_accepted_in_every_position() { + // A substitution is the equal-length case: the mismatch branch advances + // both cursors and the walk ends with no tail left over. + assert!(levenshtein_at_most_one("abc", "abc"), "identical"); + assert!(levenshtein_at_most_one("abc", "xbc"), "head"); + assert!(levenshtein_at_most_one("abc", "axc"), "middle"); + assert!(levenshtein_at_most_one("abc", "abx"), "tail"); + // An insertion makes the first argument the longer one. + assert!(levenshtein_at_most_one("abc", "zabc"), "head"); + assert!(levenshtein_at_most_one("abc", "azbc"), "middle"); + assert!(levenshtein_at_most_one("abc", "abcx"), "tail"); + // A deletion makes it the shorter one, so the leftover tail is what + // spends the edit. + assert!(levenshtein_at_most_one("xbc", "abc"), "head"); + assert!(levenshtein_at_most_one("axbc", "abc"), "middle"); + assert!(levenshtein_at_most_one("abcx", "abc"), "tail"); + // Both empty: the equality short-circuit answers this, and the walk + // would not run at all. + assert!(levenshtein_at_most_one("", ""), "both empty"); + assert!(levenshtein_at_most_one("", "a"), "empty vs one char"); + assert!(levenshtein_at_most_one("a", ""), "one char vs empty"); + assert!(!levenshtein_at_most_one("", "ab"), "empty vs two chars"); + assert!(!levenshtein_at_most_one("ab", ""), "two chars vs empty"); + } + + #[test] + fn two_edits_are_rejected_and_the_argument_order_does_not_matter() { + assert!(!levenshtein_at_most_one("abcde", "xycde"), "two subs"); + // One substitution plus one deletion. The gap is exactly one, so the + // walk runs, and the mismatch inside it has to spend the edit it has + // left before the leftover tail spends a second. + assert!(!levenshtein_at_most_one("abcx", "abd"), "sub and deletion"); + assert!(!levenshtein_at_most_one("ab", "abxy"), "two insertions"); + // The length-gap bail is `> 1` and not `>= 1`, so a gap of one is still + // walked and only a gap of two is refused up front. + assert!(!levenshtein_at_most_one("ab", "abcd"), "gap of two"); + assert!(levenshtein_at_most_one("ab", "abc"), "gap of one"); + + // The caller normalizes both names and then compares, so the predicate + // is handed an arbitrary order. Every answer above has to hold in both + // directions; a one-sided implementation passes the first block of the + // test above and fails here. + for (a, b, expected) in [ + ("abc", "xbc", true), + ("abc", "xbd", false), + ("abc", "zabc", true), + ("ab", "abxy", false), + ("abcx", "abc", true), + ("abcx", "abd", false), + ("", "a", true), + ("", "ab", false), + ] { + assert_eq!( + levenshtein_at_most_one(a, b), + expected, + "levenshtein_at_most_one({a:?}, {b:?})" + ); + assert_eq!( + levenshtein_at_most_one(b, a), + expected, + "levenshtein_at_most_one({b:?}, {a:?}) must match the other order" + ); + } + } + #[test] fn require_provenance_holds_an_attestation_it_cannot_verify_without_refusing_it() { // Policy asked for verified provenance. An attestation is not that, and @@ -2424,6 +2885,7 @@ mod tests { new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: true, + removed_dependencies: Vec::new(), }; let verdict = evaluate_with_policy( @@ -2484,6 +2946,7 @@ mod tests { new_dependencies: vec![("malicious-pkg".into(), "ssh://git@host/repo".into())], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let verdict = evaluate_with_policy( @@ -2528,6 +2991,7 @@ mod tests { new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let verdict = evaluate_with_policy( @@ -2566,6 +3030,7 @@ mod tests { new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let verdict = evaluate_with_policy( @@ -2603,6 +3068,7 @@ mod tests { new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let unreviewed = Delta { baseline_version: Some("1.0.0".into()), @@ -2688,6 +3154,7 @@ mod tests { new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let mut policy = Policy::default(); @@ -2775,6 +3242,7 @@ mod tests { new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let verdict = evaluate_with_policy( @@ -2816,6 +3284,7 @@ mod tests { "https://evil.com/cookie.tgz".into(), )], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let verdict = evaluate_with_policy( @@ -2902,6 +3371,7 @@ mod tests { new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let verdict = evaluate_with_policy( @@ -2934,6 +3404,7 @@ mod tests { new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let mut policy = Policy::default(); @@ -2980,6 +3451,7 @@ mod tests { new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let mut policy = Policy::default(); @@ -3042,6 +3514,7 @@ mod tests { new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let adv = AdvisoryReport { @@ -3103,6 +3576,7 @@ mod tests { new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let adv = AdvisoryReport { @@ -3164,6 +3638,7 @@ mod tests { new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let prov = ProvenanceReport::failed_mismatch("digest hash mismatch"); @@ -3212,6 +3687,7 @@ mod tests { new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let mut policy = Policy::default(); @@ -3597,6 +4073,7 @@ mod tests { new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), } } @@ -3883,6 +4360,7 @@ mod tests { new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let verdict = evaluate_with_trust( "test-pkg", @@ -3938,6 +4416,7 @@ maintainers = ["badactor@example.com"] new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let verdict = evaluate_with_trust( "pkg", @@ -3997,6 +4476,7 @@ maintainers = [" BadActor@Example.com "] new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let verdict = evaluate_with_trust( "pkg", @@ -4050,6 +4530,7 @@ maintainers = ["badactor@example.com"] new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let verdict = evaluate_with_trust( "pkg", @@ -4122,6 +4603,7 @@ maintainers = ["badactor@example.com"] new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let verdict = evaluate_with_trust( "pkg", @@ -4193,6 +4675,7 @@ maintainers = ["badactor@example.com"] new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let verdict = evaluate_with_trust( "yay", @@ -4253,6 +4736,7 @@ maintainers = ["badactor@example.com"] new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let verdict = evaluate_with_trust( "pkg", diff --git a/src/pkgbuild.rs b/src/pkgbuild.rs index d340c52..8ca925d 100644 --- a/src/pkgbuild.rs +++ b/src/pkgbuild.rs @@ -2708,6 +2708,7 @@ mod tests { new_dependencies: Vec::new(), modified_dependencies: Vec::new(), binding_gyp_added: false, + removed_dependencies: Vec::new(), } } diff --git a/src/registry/npm.rs b/src/registry/npm.rs index 3c48487..f4246fb 100644 --- a/src/registry/npm.rs +++ b/src/registry/npm.rs @@ -754,8 +754,14 @@ mod tests { let handle = std::thread::spawn(move || { let _ = listener.set_nonblocking(true); let start = std::time::Instant::now(); - while start.elapsed() < std::time::Duration::from_millis(600) { + let deadline = start + std::time::Duration::from_secs(5); + let expected = 8usize; + let mut served = 0usize; + let mut last = std::time::Instant::now(); + while served < expected && std::time::Instant::now() < deadline { if let Ok((mut stream, _)) = listener.accept() { + served += 1; + last = std::time::Instant::now(); let mut buf = [0u8; 1024]; let n = stream.read(&mut buf).unwrap_or(0); let req = String::from_utf8_lossy(&buf[..n]); @@ -778,6 +784,9 @@ mod tests { let _ = stream.write_all(resp.as_bytes()); } } else { + if served > 0 && last.elapsed() > std::time::Duration::from_millis(50) { + break; + } std::thread::sleep(std::time::Duration::from_millis(5)); } } @@ -827,8 +836,14 @@ mod tests { let handle = std::thread::spawn(move || { let _ = listener.set_nonblocking(true); let start = std::time::Instant::now(); - while start.elapsed() < std::time::Duration::from_millis(600) { + let deadline = start + std::time::Duration::from_secs(5); + let expected = 2usize; + let mut served = 0usize; + let mut last = std::time::Instant::now(); + while served < expected && std::time::Instant::now() < deadline { if let Ok((mut stream, _)) = listener.accept() { + served += 1; + last = std::time::Instant::now(); let mut buf = [0u8; 1024]; let n = stream.read(&mut buf).unwrap_or(0); let req = String::from_utf8_lossy(&buf[..n]); @@ -854,6 +869,9 @@ mod tests { let _ = stream.write_all(&payload); } } else { + if served > 0 && last.elapsed() > std::time::Duration::from_millis(50) { + break; + } std::thread::sleep(std::time::Duration::from_millis(5)); } } @@ -885,8 +903,14 @@ mod tests { let handle = std::thread::spawn(move || { let _ = listener.set_nonblocking(true); let start = std::time::Instant::now(); - while start.elapsed() < std::time::Duration::from_millis(600) { + let deadline = start + std::time::Duration::from_secs(5); + let expected = 2usize; + let mut served = 0usize; + let mut last = std::time::Instant::now(); + while served < expected && std::time::Instant::now() < deadline { if let Ok((mut stream, _)) = listener.accept() { + served += 1; + last = std::time::Instant::now(); let mut buf = [0u8; 1024]; let n = stream.read(&mut buf).unwrap_or(0); let req = String::from_utf8_lossy(&buf[..n]); @@ -907,6 +931,9 @@ mod tests { let _ = stream.write_all(resp.as_bytes()); } } else { + if served > 0 && last.elapsed() > std::time::Duration::from_millis(50) { + break; + } std::thread::sleep(std::time::Duration::from_millis(5)); } } @@ -953,8 +980,14 @@ mod tests { let handle = std::thread::spawn(move || { let _ = listener.set_nonblocking(true); let start = std::time::Instant::now(); - while start.elapsed() < std::time::Duration::from_millis(600) { + let deadline = start + std::time::Duration::from_secs(5); + let expected = 2usize; + let mut served = 0usize; + let mut last = std::time::Instant::now(); + while served < expected && std::time::Instant::now() < deadline { if let Ok((mut stream, _)) = listener.accept() { + served += 1; + last = std::time::Instant::now(); let mut buf = [0u8; 1024]; let n = stream.read(&mut buf).unwrap_or(0); let req = String::from_utf8_lossy(&buf[..n]); @@ -975,6 +1008,9 @@ mod tests { let _ = stream.write_all(&over_payload); } } else { + if served > 0 && last.elapsed() > std::time::Duration::from_millis(50) { + break; + } std::thread::sleep(std::time::Duration::from_millis(5)); } } @@ -1027,8 +1063,14 @@ mod tests { let handle = std::thread::spawn(move || { let _ = listener.set_nonblocking(true); let start = std::time::Instant::now(); - while start.elapsed() < std::time::Duration::from_millis(600) { + let deadline = start + std::time::Duration::from_secs(5); + let expected = 6usize; + let mut served = 0usize; + let mut last = std::time::Instant::now(); + while served < expected && std::time::Instant::now() < deadline { if let Ok((mut stream, _)) = listener.accept() { + served += 1; + last = std::time::Instant::now(); let mut buf = [0u8; 1024]; let n = stream.read(&mut buf).unwrap_or(0); let req = String::from_utf8_lossy(&buf[..n]); @@ -1067,6 +1109,9 @@ mod tests { let _ = stream.write_all(resp.as_bytes()); } } else { + if served > 0 && last.elapsed() > std::time::Duration::from_millis(50) { + break; + } std::thread::sleep(std::time::Duration::from_millis(5)); } } @@ -1120,8 +1165,14 @@ mod tests { let handle = std::thread::spawn(move || { let _ = listener.set_nonblocking(true); let start = std::time::Instant::now(); - while start.elapsed() < std::time::Duration::from_millis(600) { + let deadline = start + std::time::Duration::from_secs(5); + let expected = 1usize; + let mut served = 0usize; + let mut last = std::time::Instant::now(); + while served < expected && std::time::Instant::now() < deadline { if let Ok((mut stream, _)) = listener.accept() { + served += 1; + last = std::time::Instant::now(); let mut buf = [0u8; 1024]; let n = stream.read(&mut buf).unwrap_or(0); let req = String::from_utf8_lossy(&buf[..n]); @@ -1137,6 +1188,9 @@ mod tests { let _ = stream.write_all(resp.as_bytes()); } } else { + if served > 0 && last.elapsed() > std::time::Duration::from_millis(50) { + break; + } std::thread::sleep(std::time::Duration::from_millis(5)); } } @@ -1191,8 +1245,14 @@ mod tests { let handle = std::thread::spawn(move || { let _ = listener.set_nonblocking(true); let start = std::time::Instant::now(); - while start.elapsed() < std::time::Duration::from_millis(600) { + let deadline = start + std::time::Duration::from_secs(5); + let expected = 1usize; + let mut served = 0usize; + let mut last = std::time::Instant::now(); + while served < expected && std::time::Instant::now() < deadline { if let Ok((mut stream, _)) = listener.accept() { + served += 1; + last = std::time::Instant::now(); let mut buf = [0u8; 1024]; let n = stream.read(&mut buf).unwrap_or(0); let req = String::from_utf8_lossy(&buf[..n]); @@ -1211,6 +1271,9 @@ mod tests { let _ = stream.write_all(resp.as_bytes()); } } else { + if served > 0 && last.elapsed() > std::time::Duration::from_millis(50) { + break; + } std::thread::sleep(std::time::Duration::from_millis(5)); } } @@ -1240,8 +1303,14 @@ mod tests { let handle = std::thread::spawn(move || { let _ = listener.set_nonblocking(true); let start = std::time::Instant::now(); - while start.elapsed() < std::time::Duration::from_millis(600) { + let deadline = start + std::time::Duration::from_secs(5); + let expected = 1usize; + let mut served = 0usize; + let mut last = std::time::Instant::now(); + while served < expected && std::time::Instant::now() < deadline { if let Ok((mut stream, _)) = listener.accept() { + served += 1; + last = std::time::Instant::now(); let mut buf = [0u8; 1024]; let n = stream.read(&mut buf).unwrap_or(0); let req = String::from_utf8_lossy(&buf[..n]); @@ -1250,6 +1319,9 @@ mod tests { let _ = stream.write_all(resp.as_bytes()); } } else { + if served > 0 && last.elapsed() > std::time::Duration::from_millis(50) { + break; + } std::thread::sleep(std::time::Duration::from_millis(5)); } } diff --git a/src/render.rs b/src/render.rs index 9dcb840..57e219e 100644 --- a/src/render.rs +++ b/src/render.rs @@ -488,6 +488,7 @@ mod tests { new_dependencies: vec![], modified_dependencies: vec![], binding_gyp_added: false, + removed_dependencies: Vec::new(), }; let verdict = crate::verdict::Verdict { name: "my-pkg".into(), diff --git a/tests/aur_ci_parallel.rs b/tests/aur_ci_parallel.rs new file mode 100644 index 0000000..f94cc69 --- /dev/null +++ b/tests/aur_ci_parallel.rs @@ -0,0 +1,316 @@ +//! The AUR CI lane runs through the shared parallel `evaluate_candidates` +//! driver, so its report must follow pin-file order no matter which review +//! finishes first. The first pin's RPC answer is delayed on purpose: a lane +//! that emitted items as they finished would put `aaa-slow` last. + +use std::io::{Read, Write}; +use std::net::{TcpListener, TcpStream}; +use std::path::Path; +use std::sync::Arc; + +use assert_cmd::Command; + +const SLOW: &str = "aaa-slow"; +const FAST: [&str; 3] = ["bbb-fast", "ccc-fast", "ddd-fast"]; +const SLOW_DELAY_MS: u64 = 1500; + +fn fixture_git(dir: &Path, args: &[&str]) { + let output = std::process::Command::new("git") + .args(args) + .current_dir(dir) + .output() + .unwrap(); + assert!( + output.status.success(), + "git {args:?} failed: {}", + String::from_utf8_lossy(&output.stderr) + ); +} + +fn write_fixture_pkg(repo: &Path, pkgbase: &str) { + std::fs::write( + repo.join("PKGBUILD"), + format!( + "pkgname={pkgbase}\npkgver=1.0\npkgrel=1\narch=('any')\n\ + source=(https://good.example/{pkgbase}-1.0.tar.gz)\n\ + sha256sums=(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n\ + validpgpkeys=(AAA)\nbuild() {{\n make\n}}\n" + ), + ) + .unwrap(); + std::fs::write( + repo.join(".SRCINFO"), + format!("pkgbase = {pkgbase}\n\tpkgver = 1.0\n\tpkgrel = 1\n"), + ) + .unwrap(); +} + +struct MultiPackageFixture { + base: String, + _server: std::thread::JoinHandle<()>, + _dir: tempfile::TempDir, +} + +/// Loopback AUR serving one single-commit repo per package in `names`, +/// over smart HTTP like the real archive. Only the RPC side is delayed, +/// and only for the slow package: its whole review still finishes last, +/// so the report order has to come from the candidate index, not the clock. +fn spawn_multi_package_fixture(names: &[&str]) -> MultiPackageFixture { + let dir = tempfile::tempdir().unwrap(); + for name in names { + let work = dir.path().join(format!("work-{name}")); + std::fs::create_dir_all(&work).unwrap(); + fixture_git(&work, &["init", "--quiet", "-b", "master"]); + fixture_git(&work, &["config", "user.email", "alice@example.com"]); + fixture_git(&work, &["config", "user.name", "Fixture"]); + fixture_git(&work, &["config", "commit.gpgsign", "false"]); + write_fixture_pkg(&work, name); + fixture_git(&work, &["add", "-A"]); + fixture_git( + &work, + &["commit", "--quiet", "-m", &format!("{name} 1.0-1")], + ); + let bare = dir.path().join(format!("{name}.git")); + fixture_git( + dir.path(), + &[ + "clone", + "--quiet", + "--bare", + work.to_str().unwrap(), + bare.to_str().unwrap(), + ], + ); + fixture_git(&bare, &["update-server-info"]); + } + + let serve_dir = Arc::new(dir.path().to_path_buf()); + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let base = format!("http://{}", listener.local_addr().unwrap()); + let handle = std::thread::spawn(move || { + for stream in listener.incoming() { + let Ok(mut stream) = stream else { continue }; + let serve_dir = serve_dir.clone(); + std::thread::spawn(move || serve_aur(&mut stream, &serve_dir)); + } + }); + MultiPackageFixture { + base, + _server: handle, + _dir: dir, + } +} + +fn read_http_request(stream: &mut TcpStream) -> Option<(usize, Vec)> { + let mut buf = Vec::new(); + let mut tmp = [0u8; 4096]; + let head_end = loop { + if let Some(pos) = buf.windows(4).position(|w| w == b"\r\n\r\n") { + break pos + 4; + } + if buf.len() > 65_536 { + return None; + } + let n = stream.read(&mut tmp).ok()?; + if n == 0 { + return None; + } + buf.extend_from_slice(&tmp[..n]); + }; + let headers = String::from_utf8_lossy(&buf[..head_end]).to_ascii_lowercase(); + let content_length: usize = headers + .lines() + .find_map(|l| l.strip_prefix("content-length:")) + .and_then(|v| v.trim().parse().ok()) + .unwrap_or(0); + while buf.len() < head_end + content_length { + let n = stream.read(&mut tmp).ok()?; + if n == 0 { + return None; + } + buf.extend_from_slice(&tmp[..n]); + } + Some((head_end, buf)) +} + +fn upload_pack(bare: &Path, advertise: bool, body: &[u8]) -> Option> { + let mut args = vec!["upload-pack", "--stateless-rpc"]; + if advertise { + args.push("--advertise-refs"); + } + let mut child = std::process::Command::new("git") + .args(&args) + .arg(".") + .current_dir(bare) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()) + .spawn() + .ok()?; + child.stdin.take()?.write_all(body).ok()?; + let out = child.wait_with_output().ok()?; + out.status.success().then_some(out.stdout) +} + +fn serve_aur(stream: &mut TcpStream, serve_dir: &Path) { + let Some((head_end, req)) = read_http_request(stream) else { + return; + }; + let request = String::from_utf8_lossy(&req); + let path = request + .lines() + .next() + .and_then(|l| l.split_whitespace().nth(1)) + .unwrap_or("/") + .to_string(); + let is_gzip = request[..head_end] + .to_ascii_lowercase() + .contains("content-encoding: gzip"); + let body = req[head_end..].to_vec(); + + let bare_for = |pkg: &str| { + let bare = serve_dir.join(format!("{pkg}.git")); + bare.is_dir().then_some(bare) + }; + + // Git appends `?service=git-upload-pack` to the refs advertisement, so + // route on the path with the query stripped, like the real client does. + let route = path.split('?').next().unwrap_or("").to_string(); + let (status, ctype, out) = if let Some(name) = path.strip_prefix("/rpc/v5/info?arg%5B%5D=") { + let name = name.split('&').next().unwrap_or("").to_string(); + if name == SLOW { + std::thread::sleep(std::time::Duration::from_millis(SLOW_DELAY_MS)); + } + let Some(_) = bare_for(&name) else { + let head = "HTTP/1.1 404 Not Found\r\nContent-Length: 4\r\nConnection: close\r\n\r\n"; + let _ = stream.write_all(head.as_bytes()); + let _ = stream.write_all(b"nope"); + return; + }; + let rpc = serde_json::json!({ + "version": 5, + "type": "multiinfo", + "resultcount": 1, + "results": [{ + "ID": 1, + "Name": name, + "PackageBaseID": 1, + "PackageBase": name, + "Version": "1.0-1", + "Description": "fixture", + "Maintainer": "alice" + }] + }) + .to_string(); + ("200 OK", "application/json", rpc.into_bytes()) + } else if let Some(pkg) = route + .strip_prefix('/') + .and_then(|p| p.strip_suffix(".git/info/refs")) + { + match bare_for(pkg).and_then(|bare| upload_pack(&bare, true, b"")) { + Some(adv) => { + let mut body = b"001e# service=git-upload-pack\n0000".to_vec(); + body.extend_from_slice(&adv); + ( + "200 OK", + "application/x-git-upload-pack-advertisement", + body, + ) + } + None => ( + "500 Internal Server Error", + "text/plain", + b"upload-pack failed".to_vec(), + ), + } + } else if let Some(pkg) = route + .strip_prefix('/') + .and_then(|p| p.strip_suffix(".git/git-upload-pack")) + { + let mut body = body; + if is_gzip { + let mut plain = Vec::new(); + if std::io::Read::read_to_end(&mut flate2::read::GzDecoder::new(&body[..]), &mut plain) + .is_err() + { + return; + } + body = plain; + } + match bare_for(pkg).and_then(|bare| upload_pack(&bare, false, &body)) { + Some(out) => ("200 OK", "application/x-git-upload-pack", out), + None => ( + "500 Internal Server Error", + "text/plain", + b"upload-pack failed".to_vec(), + ), + } + } else { + ("404 Not Found", "text/plain", b"nope".to_vec()) + }; + let head = format!( + "HTTP/1.1 {status}\r\nContent-Type: {ctype}\r\nContent-Length: {}\r\nConnection: close\r\n\r\n", + out.len() + ); + let _ = stream.write_all(head.as_bytes()); + let _ = stream.write_all(&out); +} + +#[test] +fn aur_ci_reports_pins_in_pin_file_order() { + let mut names = vec![SLOW]; + names.extend(FAST); + let fixture = spawn_multi_package_fixture(&names); + + let repo = tempfile::tempdir().unwrap(); + fixture_git(repo.path(), &["init", "--quiet", "-b", "main"]); + fixture_git(repo.path(), &["config", "user.email", "alice@example.com"]); + fixture_git(repo.path(), &["config", "user.name", "Fixture"]); + fixture_git(repo.path(), &["config", "commit.gpgsign", "false"]); + std::fs::write(repo.path().join("aur.lock"), "# none yet\n").unwrap(); + fixture_git(repo.path(), &["add", "-A"]); + fixture_git(repo.path(), &["commit", "--quiet", "-m", "base pins"]); + let head_pins = names + .iter() + .map(|n| format!("{n}@1.0-1")) + .collect::>() + .join("\n") + + "\n"; + std::fs::write(repo.path().join("aur.lock"), &head_pins).unwrap(); + + let isolated = tempfile::tempdir().unwrap(); + let output = Command::cargo_bin("blueline") + .unwrap() + .current_dir(repo.path()) + .env("BLUELINE_DATA_DIR", isolated.path()) + .args([ + "--ecosystem", + "aur", + "--registry", + &fixture.base, + "ci", + "--lockfile", + "aur.lock", + "--base", + "HEAD", + "--format", + "json", + ]) + .assert() + .success() + .get_output() + .stdout + .clone(); + let report: serde_json::Value = + serde_json::from_str(&String::from_utf8(output).unwrap()).expect("valid json"); + let got: Vec<&str> = report["items"] + .as_array() + .expect("items array") + .iter() + .map(|i| i["name"].as_str().expect("item name")) + .collect(); + assert_eq!( + got, names, + "items must follow pin-file order, not finish order" + ); +} diff --git a/tests/pypi_cli.rs b/tests/pypi_cli.rs index a0ad94c..c3e3aaa 100644 --- a/tests/pypi_cli.rs +++ b/tests/pypi_cli.rs @@ -172,6 +172,7 @@ fn review_pypi_first_sighting_renders_ecosystem_and_sha256() { let output = Command::cargo_bin("blueline") .unwrap() .env("HOME", dir.path()) + .env("BLUELINE_DATA_DIR", dir.path()) .args([ "--ecosystem", "pypi", @@ -236,6 +237,7 @@ fn review_pypi_rejects_checksum_mismatch() { Command::cargo_bin("blueline") .unwrap() .env("HOME", dir.path()) + .env("BLUELINE_DATA_DIR", dir.path()) .args([ "--ecosystem", "pypi", @@ -299,6 +301,7 @@ fn ci_pypi_lockfile_hash_verification() { Command::cargo_bin("blueline") .unwrap() .env("HOME", dir.path()) + .env("BLUELINE_DATA_DIR", dir.path()) .args([ "--ecosystem", "pypi", @@ -324,6 +327,7 @@ fn ci_pypi_lockfile_hash_verification() { Command::cargo_bin("blueline") .unwrap() .env("HOME", dir.path()) + .env("BLUELINE_DATA_DIR", dir.path()) .args([ "--ecosystem", "pypi",