From ca22d4371bcc8d39516fad97aa3fb399252f0346 Mon Sep 17 00:00:00 2001 From: kridaydave Date: Sun, 11 Oct 2026 14:46:23 +0530 Subject: [PATCH] Drop dead weight, stop one silent mis-score, and correct three stale claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cleanup pass over the engine, the docs, and the lockfile. Two dead items removed. `heuristic::evaluate` was a five-argument wrapper around `evaluate_with_policy` with `Policy::default()` baked in: production never called it, only its own tests did, so the default-policy path existed in two places. The tests now call `evaluate_with_policy` directly. In the PyPI `/simple/` model, `SimpleFile::size` and `SimpleFile::provenance` were `#[serde(default)]` members nothing read, which is the exact defect `SimpleMeta`'s own doc comment refuses to carry for `api-version`. Both are gone, along with the struct's `#[allow(dead_code)]`. One coercion to a wrong answer is now a refusal. `diff_single_file` ran `from_utf8(...).unwrap_or("")` on both sides, so a file stamped text whose bytes do not decode diffed as everything-deleted or everything-added and scored accordingly. Unreachable through `compute_delta`, since `classify_bytes` routes non-UTF-8 to `Binary` and the early return covers it, which is what made it safe to leave: the only state that reaches it is the classification disagreeing with the bytes, and a coercion cannot answer that state correctly. Both sides are refused with an error naming the path and side. Verified by restoring the coercion and watching the new test fail. Three doc claims corrected, each against the thing they describe: - CHANGELOG 0.3.1 said "all six managers are reviewed through one grammar". `--help`, `ARCHITECTURE.md`, and `SHIM_MANAGERS` all say eleven and are right; the entry says eleven now. - CHANGELOG 0.3.0 claimed crate-name `-`/`_` canonicalization. Checked against the live registry: the index stores only the folded file (`se/rd/serde_json` 200, `se/rd/serde-json` 404; `static.crates.io/crates/rocket_db_pools/` 200, the hyphen 403), so a hyphen spelling fails closed at the fetch. The distinctness is deliberate anyway — a configured index or mirror may serve both spellings, and answering one with the other's bytes is worse than refusing — and the ci lane pins that invariant. The claim is removed; drift entry 4 records the evidence and the stance. A first attempt to fold the names was reverted after `lockfile_diff_keeps_pypi_alias_off_cargo_pins` caught it. - CHANGELOG 0.3.0 said `blueline install` shipped "sandboxed extraction". It never did; the entry now says what shipped and points at drift entry 1, which stays High and unfixed because the fix is code. Dependencies: clap 4.6.7, flate2 1.1.10 (miniz_oxide 0.9.1, zlib-rs 0.6.8), serde_json 1.0.152, thiserror 2.0.21. All four are caret-compatible bumps whose declared MSRVs (1.85/1.67/1.71/1.77, miniz_oxide and zlib-rs declare none) sit under the pinned 1.97.1 toolchain. actionlint 1.7.7 to 1.7.12 in CI; the new binary lints this repo's workflows clean before the bump. proptest stays at 1.11.0 on purpose: 1.12 wants rand 0.10, which reshuffles a dev-only dependency tree for no security-relevant gain. Gate: ./scripts/verify.sh passes (fmt, clippy, test, references). The test stage ran with CARGO_PROFILE_{DEV,TEST}_DEBUG=0 because this box was holding ~6.8 of 7.7 GB in other sessions and the debuginfo-heavy link got OOM-killed twice; the assertions are unchanged, but that run is one environment flag away from CI's exact command. Commit made by Step 5 Preview Free in OpenCode on behalf of Kriday. --- .github/workflows/ci.yml | 2 +- CHANGELOG.md | 55 ++++++++++++++++++++++++---- Cargo.lock | 47 +++++++++++++----------- references/drift.md | 64 +++++++++++++++++++++++++-------- references/features/diff.md | 11 ++++-- references/features/registry.md | 25 ++++++++----- src/diff.rs | 49 +++++++++++++++++++++++-- src/heuristic.rs | 60 +++++++++++++++---------------- src/registry/cratesio.rs | 12 ++++++- src/registry/pypi.rs | 7 ---- 10 files changed, 237 insertions(+), 95 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 45d9071..110e766 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,7 +40,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - uses: docker://rhysd/actionlint:1.7.7 + - uses: docker://rhysd/actionlint:1.7.12 with: # GitHub's expression and schema grammar only. The image also bundles # shellcheck and pyflakes, which flag pre-existing quoting warnings in diff --git a/CHANGELOG.md b/CHANGELOG.md index fc42bc7..583d080 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1399,6 +1399,46 @@ Targeted at 0.4.0. This release is a behaviour change for anyone who set - `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 + was on. Both sides are refused now, with an error naming the path and the + side. The path is unreachable through `compute_delta` — `classify_bytes` + already routes non-UTF-8 to `Binary` — so this could only fire where the + 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 + entry now says eleven. Resolves drift entry 6. +- CHANGELOG 0.3.0 claimed `-`/`_` canonicalization of crate names + (`serde_json` → `serde-json`). The adapter lowercases only, deliberately: + a configured index may serve both spellings, so folding would risk answering + one spelling with the other's bytes. A hyphen-spelled crates.io request + fails closed at the index fetch (the live index has no hyphen file), which + was checked rather than assumed. The claim is removed and the lane records + the stance. Resolves drift entry 4. +- CHANGELOG 0.3.0 described `blueline install` as shipping "sandboxed + extraction". It never did; the entry now says what shipped and points at + drift entry 1, which stays High and unfixed. + ## [0.3.1] - 2026-09-21 ### Added @@ -1410,8 +1450,8 @@ Targeted at 0.4.0. This release is a behaviour change for anyone who set Fail closed everywhere — blueline missing, errored, or refusing means the install does not run, and a missing real binary refuses shim creation. The scanner gained `cargo install`, and `yay`/`paru -S` operands - (AUR-grammar specs with `name=version` pinning), so all six managers are - reviewed through one grammar; pip flags that name non-registry sources + (AUR-grammar specs with `name=version` pinning), so all eleven scanned + managers are reviewed through one grammar; pip flags that name non-registry sources (`-r`, `-e`, `--constraint`, …) are refused with a pointer to `blueline ci`. `BLUELINE_REGISTRY` and `BLUELINE_POLICY` environment variables scope a shimmed shell to a mirror and a project policy. @@ -1634,9 +1674,9 @@ Targeted at 0.4.0. This release is a behaviour change for anyone who set sparse-index NDJSON client with fail-closed parsing (bad `vers` on a recognized row aborts; unknown schema `v > 2` rows are skipped with a note; missing `yanked` reads as false), `config.json` handling that refuses - authenticated registries, canonical crate names (`serde_json` → `serde-json`), - and `.crate` downloads verified by sha256 against the index checksum before - extraction. Extracted archives must unpack to exactly one top-level + authenticated registries, and `.crate` downloads verified by sha256 against + the index checksum before extraction. Extracted archives must unpack to + exactly one top-level `{name}-{version}` directory. - Packed `Cargo.toml` reader: `[package] build`/`links`, `[[bin]]` targets, dependency maps, and `[features]`; dependencies project onto the existing @@ -1732,8 +1772,9 @@ Targeted at 0.4.0. This release is a behaviour change for anyone who set - `blueline review ` command with text and JSON output, `--registry` override, and a fail-closed `--yes` flag. -- `blueline install` command with sandboxed extraction and fail-closed script - execution. +- `blueline install` command with fail-closed script execution (extraction was + parser-bounded; the OS-level sandbox this line once claimed was never + shipped, see drift entry 1). - `blueline ci --output-file ` parameter to save formatted CI review reports directly to disk. - npm registry client with SHA-512 integrity verification before extraction. diff --git a/Cargo.lock b/Cargo.lock index 3a6198c..58f0d56 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -215,9 +215,9 @@ checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" [[package]] name = "clap" -version = "4.6.6" +version = "4.6.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "473c7e07f409a8d772161724aa8db6a765a2532a70f9667eeb7b49d3d02fbdca" +checksum = "aa8876b300ab35ba921adea3dfd70157a46249b33f95c9084ae5709785478946" dependencies = [ "clap_builder", "clap_derive", @@ -225,9 +225,9 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.6.6" +version = "4.6.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7b48fea5a88e9ae728a2dcbedbfc0e730f7d60da42e1cb049a83c9fb8b789889" +checksum = "ec0797fb7aeb1406c84efac526901f7ec3ead2124f946b494e72879d4b54704d" dependencies = [ "anstream", "anstyle", @@ -237,9 +237,9 @@ dependencies = [ [[package]] name = "clap_derive" -version = "4.6.4" +version = "4.6.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d012d2b9d65aca7f18f4d9878a045bc17899bba951561ba5ec3c2ba1eed9a061" +checksum = "f9c751b79415d4e559e3d1fcf128e09e720eb673a06d26cf6f392d37d75b66e0" dependencies = [ "heck", "proc-macro2", @@ -408,7 +408,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -447,12 +447,13 @@ checksum = "26b73573e6edcd2af0cdf47bd6cb58f0b3839491263c314eaad1ccf24430e1de" [[package]] name = "flate2" -version = "1.1.9" +version = "1.1.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "843fba2746e448b37e26a819579957415c8cef339bf08564fe8b7ddbd959573c" +checksum = "6e634e2e0ebac1ee034020da1ca582e17ffe4e0f5e985823721e168928136dcb" dependencies = [ "crc32fast", "miniz_oxide", + "zlib-rs", ] [[package]] @@ -745,9 +746,9 @@ checksum = "cf8baf1c55e62ffcace7a9f06f4bd9cd3f0c4beb022d3b367256b91b87513d98" [[package]] name = "miniz_oxide" -version = "0.8.9" +version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1fa76a2c86f704bdb222d66965fb3d63269ce38518b83cb0575fca855ebb6316" +checksum = "b63fbc4a50860e98e7b2aa7804ded1db5cbc3aff9193adaff57a6931bf7c4b4c" dependencies = [ "adler2", "simd-adler32", @@ -1059,7 +1060,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -1162,9 +1163,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.151" +version = "1.0.152" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c841b55ecdae098c80dcae9cf767f6f8a0c2cdb3416bbef72181df4d0fe73f14" +checksum = "1741ab7a6cc54a03a89b5d563ed60075c277d9e3cfa73ad0c1f23f23974703c6" dependencies = [ "itoa", "memchr", @@ -1289,7 +1290,7 @@ dependencies = [ "getrandom 0.4.3", "once_cell", "rustix", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -1300,18 +1301,18 @@ checksum = "8f50febec83f5ee1df3015341d8bd429f2d1cc62bcba7ea2076759d315084683" [[package]] name = "thiserror" -version = "2.0.20" +version = "2.0.21" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ec86235f5fcc2a73650310756d2ac5b138a5780bbbdfae3eeccec992c435ba4f" +checksum = "09e52cb86a36cede5cb101bf8908837b3e4c6e5e59fe7fd85c23fb56200d189e" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "2.0.20" +version = "2.0.21" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bc04cd3e1236dd4a98afca4569f2deb3f120e5422a4023be2cb683f8486292af" +checksum = "fe5197923287db20a58125f0bc85c062f7f2c892de97b18c356f9efb14b28524" dependencies = [ "proc-macro2", "quote", @@ -1531,7 +1532,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -1779,6 +1780,12 @@ dependencies = [ "zopfli", ] +[[package]] +name = "zlib-rs" +version = "0.6.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b268e58e7c693d7c271f93ffc4ba3b380412554231c85bf61ca7af91042a4112" + [[package]] name = "zmij" version = "1.0.23" diff --git a/references/drift.md b/references/drift.md index d4b6a56..29f944a 100644 --- a/references/drift.md +++ b/references/drift.md @@ -18,12 +18,12 @@ Severity is this file's own judgement, not the project's. | 8 | ~~SSRF guard fails **open** on DNS failure, port 443 hard-coded~~ **resolved** — `ValidatingResolver` is the guard now, not the pre-flight check | High — see below | | 11 | ~~PyPI `METADATA` parse fails **open**~~ **resolved** — every unreadable or undetermined-metadata case is a refusal | High — see below | | 3 | Integrity mismatch surfaces as `Err` + exit 1, not `Verdict::Block` | Medium — a consumer keying on band never sees BLOCK | -| 4 | No cargo `-`/`_` name canonicalization, contrary to `CHANGELOG` 0.3.0 | Medium — `serde_json` and `serde-json` are distinct crates | -| 10 | `diff_single_file` coerces invalid UTF-8 to `""`, so a real change reads as all-add or all-delete | Medium — undocumented, silently mis-scores | +| 4 | ~~No cargo `-`/`_` name canonicalization, contrary to `CHANGELOG` 0.3.0~~ — **resolved as a doc correction:** the claim was wrong; the distinctness is deliberate and fails closed | Medium — see below | +| 10 | ~~`diff_single_file` coerces invalid UTF-8 to `""`, so a real change reads as all-add or all-delete~~ — **resolved:** both sides are refused with a naming error | Medium — was silent mis-scoring; see below | | 12 | ~~`store.rs` discards the `0o600` pre-create error (`let _ =`)~~ **resolved** — all three discarded errors propagate. See below | Low | | 9 | ~~`hex_to_bytes` uses `.unwrap_or(0)`, mapping invalid hex to byte `0`~~ **resolved** — the helper is fallible and `to_sri` returns `Result` | Low | | 5 | ~~`registry/mod.rs` doc comments are stale~~ **resolved** — both were deleted; the four adapters are all real. See below | Low | -| 6 | Manager count is six in `CHANGELOG` 0.3.1, eleven in `ARCHITECTURE.md` §5; `--help` says eleven | Low — `--help` is authoritative | +| 6 | ~~Manager count is six in `CHANGELOG` 0.3.1, eleven in `ARCHITECTURE.md` §5; `--help` says eleven~~ — **resolved:** the 0.3.1 entry now says eleven, matching `--help` and `SHIM_MANAGERS` | Low — `--help` was authoritative | | 7 | ~~Dead code: `Policy::is_maintainer_blocked`, `blocklist.maintainers`, `allowlist.packages[].max_risk` / `.integrity`~~ **resolved** — the three policy keys are now refused at load. Still dead: `BaselineStore::clear_advisory_cache` | Low — test-only helper, `#[allow(dead_code)]` | ## Lane verification basis @@ -37,7 +37,7 @@ the prose docs. | [verdict](features/verdict.md) | `src/verdict.rs` full read | | [error](features/error.md) | `src/error.rs` full read + cross-referenced callers | | [heuristic](features/heuristic.md) | `src/heuristic.rs` production + all 52 test names | -| [diff](features/diff.md) | `src/diff.rs` production + all 7 test names | +| [diff](features/diff.md) | `src/diff.rs` production + all 8 test names | | [render](features/render.md) | `src/render.rs` production + all 13 test names + 2 proptests | | [version](features/version.md) | `src/version.rs` production + all 23 test names | | [manifest](features/manifest.md) | `src/manifest.rs` production + all 32 test names | @@ -73,6 +73,12 @@ Each finding is cited in its lane file with the exact source location. (Landlock on Linux, `cap-std`, or seccomp), which is a dependency decision and therefore not something to do silently. See [extract](features/extract.md) for what the bounds actually are. + + One more prose-level claim was corrected while the capability was still + absent: the 0.3.0 entry described `blueline install` as shipping "sandboxed + extraction". It never did, and the entry now says fail-closed script + execution and points here, so the history does not assert the capability + either. Entry stays **High, unfixed**; the fix is code. 2. **The npm registry signature is read for presence and never verified. Partly resolved; the verification gap itself is untouched.** `release_signatures` still returns the block as raw JSON, nothing compares the bytes to it, and @@ -96,19 +102,40 @@ Each finding is cited in its lane file with the exact source location. 3. **Integrity mismatch produces an `Err`, not `Verdict::Block`.** A CI report contains an error and exit 1, not a BLOCK verdict. See [extract](features/extract.md). -4. **No cargo `-`/`_` canonicalization.** `canonical_crate_name` only lowercases, - so `serde_json` and `serde-json` are distinct crates — contrary to CHANGELOG - 0.3.0. See [registry](features/registry.md). +4. **~~No cargo `-`/`_` canonicalization, contrary to CHANGELOG 0.3.0~~ — + resolved as a doc correction; the code stance was right and stays.** + `canonical_crate_name` only lowercases, so `serde_json` and `serde-json` + were distinct crates — which CHANGELOG 0.3.0 claimed was not the case. The + claim was wrong about this tree, and it is now corrected. The code is right + to stay as it is, for two reasons that survived checking the claim against + the live registry: + + - The real crates.io index does store only the folded file + (`index.crates.io/se/rd/serde_json` is 200; `/se/rd/serde-json` is 404; + `static.crates.io/crates/rocket-db-pools/0.2.0/download` is 403 where + `/rocket_db_pools/` is 200), so a hyphen-spelled review of a crates.io + crate fails closed at the index fetch today. That is a named, loud + refusal, not a wrong answer. + - This tool is not restricted to crates.io: `--index` and the `dl` in + `config.json` point at whatever registry is configured, and a mirror may + genuinely serve both spellings. Folding would answer one spelling with + the other's bytes — the ci lane pins that invariant from its side + (`lockfile_diff_keeps_pypi_alias_off_cargo_pins`). + + The ergonomic cost (a hyphen-spelled crates.io request refuses) is a + judgement call for the project, not a defect: the failure is closed and + names the package. See [registry](features/registry.md). 5. **~~`registry/mod.rs` doc comments were stale~~ — resolved.** Two comments claimed npm was the only wired adapter ("npm is fully wired; cargo, PyPI, and AUR adapters build on these seams in later PRs", and "npm is the only full impl for now") while `NpmRegistry`, `CratesIoRegistry`, `PyPIRegistry`, and `AurRegistry` all implement the trait. Both comments are deleted. -6. **CHANGELOG 0.3.1 says "all six managers are reviewed through one grammar" - and "eleven scanned managers" in ARCHITECTURE.md §5 — the CLI enumerates - eleven** (`npm npx pnpm yarn bun bunx pip pip3 cargo yay paru`) while the - 0.3.1 entry lists six. Both counts appear in the project's own docs; the - `--help` text is authoritative and says eleven. +6. **~~CHANGELOG 0.3.1 says "all six managers are reviewed through one grammar"~~ + — resolved.** "eleven scanned managers" in ARCHITECTURE.md §5 and the CLI's + eleven (`npm npx pnpm yarn bun bunx pip pip3 cargo yay paru`, the same list + as `SHIM_MANAGERS`) were right; the 0.3.1 entry listed six. Both counts + appeared in the project's own docs and `--help` was authoritative, so the + entry now says eleven. 7. **~~`BaselineStore::clear_advisory_cache` and `BaselineResolution::display_summary` are dead~~ — `display_summary` is deleted; `impl BaselineResolution` carries only `package()`. The one that @@ -147,8 +174,17 @@ Each finding is cited in its lane file with the exact source location. `to_sri`, so an undecodable digest is a refusal that names the offending input. Tests `a_malformed_digest_fails_loud_instead_of_zeroing` and `a_well_formed_digest_still_renders`. See [registry](features/registry.md). -10. **`diff_single_file` silently converts invalid UTF-8 to `""`**, so a real - change reads as all-add or all-delete. `ambiguous`, not documented. See +10. **~~`diff_single_file` silently converts invalid UTF-8 to `""`~~ — + resolved.** A real change read as all-add or all-delete, because one side + decoded to the empty string and every line then counted as inserted or + deleted. `diff_single_file` refuses with `BluelineError::Extraction` naming + the path and the side. The path is unreachable through `compute_delta` + (`classify_bytes` routes non-UTF-8 to `Binary` and `diff_single_file` + returns early for non-text), so this only ever fired on a + classification-vs-bytes disagreement — which is exactly the state a + coercion cannot answer correctly. Test + `a_text_classified_file_that_does_not_decode_is_refused_not_coerced`, + verified failing on the pre-fix coercion. See [diff](features/diff.md). 11. **~~PyPI `METADATA` parsing fails OPEN~~ — resolved.** The old `if let Ok(raw) = read_to_string(root.join("METADATA"))` is gone. A PyPI diff --git a/references/features/diff.md b/references/features/diff.md index 275a339..0539b88 100644 --- a/references/features/diff.md +++ b/references/features/diff.md @@ -54,9 +54,14 @@ Separately, `is_executable_extension` is a platform-independent 11-entry list `base_iter.next()` / `target_iter.next()` immediately after a `peek()` returned `Some`, with `#[allow(clippy::unwrap_used)]` and a comment. Do not "clean them up"; and do not add more elsewhere. -- **`diff_single_file`'s `from_utf8(...).unwrap_or("")` means a misclassified - non-UTF-8 file diffs as "everything deleted."** It is unreachable in practice - because `classify_bytes` already routed it to `Binary`. +- **A file stamped `Text` whose bytes do not decode is refused, not coerced.** + `diff_single_file` used to run `from_utf8(...).unwrap_or("")`, so a + misclassified non-UTF-8 file diffed as "everything deleted" (or "everything + added" on the other side) and scored accordingly. It is unreachable through + `compute_delta` because `classify_bytes` already routed it to `Binary`, so + the coercion was defense that could only ever answer wrong: both sides now + fail the review with `BluelineError::Extraction` naming the path and side. + Test `a_text_classified_file_that_does_not_decode_is_refused_not_coerced`. - **`collect_all_dependencies` uses `or_insert`, so `dependencies` shadows `optionalDependencies` and `peerDependencies`.** A peer-range flip on an existing dep is invisible. Switching to `insert` would make it a *modified* diff --git a/references/features/registry.md b/references/features/registry.md index 6e9a668..3d7232d 100644 --- a/references/features/registry.md +++ b/references/features/registry.md @@ -125,12 +125,19 @@ npm-only: `impl Registry for` resolves to `NpmRegistry`, `CratesIoRegistry`, are accepted. - Sparse path: `1/{a}`, `2/{ab}`, `3/{a}/{abc}`, `{c1}{c2}/{c3}{c4}/{name}`. Name `len() > 64` → error. -- **There is NO `serde_json` → `serde-json` normalization.** `canonical_crate_name` - only ASCII-lowercases. Tests `canonicalizes_names` asserts - `canonical_crate_name("Serde_JSON") == "serde_json"`. crates.io's real index - treats `-` and `_` as equivalent; this code does not. Consequence: - `verify_single_root` expects a root literally `{lowercased-name}-{version}`. - CHANGELOG 0.3.0's claim of `-`/`_` canonicalization is **wrong for this tree**. +- **Crate names are lowercased, and hyphen/underscore spellings stay + distinct.** `canonical_crate_name` ASCII-lowercases only; test + `canonicalizes_names` asserts `canonical_crate_name("Serde-JSON") == + "serde-json"`. The real crates.io index stores only the folded file + (`se/rd/serde_json` exists, `se/rd/serde-json` 404s — checked against the + live sparse index, and the download host 403s the hyphen path), so a + hyphen-spelled review of a crates.io crate fails closed at the index fetch + rather than resolving. That is kept on purpose: a configured index or mirror + may genuinely serve both spellings, and answering one spelling with the + other's bytes is worse than refusing. The ci lane pins the same invariant + (`lockfile_diff_keeps_pypi_alias_off_cargo_pins`: `foo_bar` must not inherit + `foo-bar`'s pin). CHANGELOG 0.3.0's claim of `-`/`_` canonicalization was + wrong about this tree and is corrected; see drift entry 4. - NDJSON fail-closed rules, verbatim: malformed JSON lines are errors; rows with unknown schema `v > 2` are skipped **with a note printed to stderr**; recognized rows with unparsable `vers` are errors; missing `yanked` reads false. @@ -159,8 +166,10 @@ npm-only: `impl Registry for` resolves to `NpmRegistry`, `CratesIoRegistry`, `2024-01-17T16:53:12.779164Z → 1705510392` and `1970-01-01T00:00:00Z → 0`. - **`resolve_package` performs no name-mismatch check**, unlike npm, crates.io, and AUR. `Package.name` comes from the server's response. -- `SimpleFile` is entirely `#[allow(dead_code)]`; `size` and `provenance` are - parsed but never read. PEP 740 lives in [provenance](provenance.md). +- `SimpleFile` keeps only members something reads (`filename`, `url`, + `hashes`, `upload-time`, `yanked`); `size` and `provenance` were `#[serde(default)]` + members nothing read and are gone, same as the `api-version` marker `SimpleMeta` + refuses to carry. PEP 740 lives in [provenance](provenance.md). - The file is minified — no blank lines between functions, no doc comments. ## AUR diff --git a/src/diff.rs b/src/diff.rs index c0d3232..b315efc 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -370,8 +370,20 @@ fn diff_single_file( None => Vec::new(), }; - let old_str = std::str::from_utf8(&old_bytes).unwrap_or(""); - let new_str = std::str::from_utf8(&new_bytes).unwrap_or(""); + let old_str = std::str::from_utf8(&old_bytes).map_err(|e| { + BluelineError::Extraction(format!( + "diffing {rel_path}: baseline bytes are not UTF-8 ({e}); a text-classified \ + file that does not decode cannot be line-diffed, and coercing it to \ + an empty string scored a real change as all-add or all-delete" + )) + })?; + let new_str = std::str::from_utf8(&new_bytes).map_err(|e| { + BluelineError::Extraction(format!( + "diffing {rel_path}: payload bytes are not UTF-8 ({e}); a text-classified \ + file that does not decode cannot be line-diffed, and coercing it to \ + an empty string scored a real change as all-add or all-delete" + )) + })?; let diff = similar::TextDiff::from_lines(old_str, new_str); let mut lines_added = 0; @@ -541,6 +553,39 @@ mod tests { assert_eq!(classify_bytes(&data), FileKind::Binary); } + #[test] + fn a_text_classified_file_that_does_not_decode_is_refused_not_coerced() { + // scan_tree classifies invalid UTF-8 as Binary, so this is the + // meta-vs-bytes disagreement path: a file stamped Text whose bytes + // do not decode. Coercing the side to "" made a real change read + // as all-add or all-delete, which silently mis-scores the verdict. + let dir = tempfile::tempdir().unwrap(); + let mut bytes = b"console.log('install')\n".to_vec(); + bytes.push(0xff); // lone continuation byte: not valid UTF-8 + let payload = dir.path().join("index.js"); + fs::write(&payload, &bytes).unwrap(); + let rel = Path::new("index.js"); + let meta = DiskFileMeta { + hash: [0u8; 32], + kind: FileKind::Text, + is_executable: false, + }; + + let err = diff_single_file(None, Some(&payload), rel, &meta).unwrap_err(); + assert!( + err.to_string().contains("payload bytes are not UTF-8"), + "{err}" + ); + + let baseline = dir.path().join("baseline.js"); + fs::write(&baseline, &bytes).unwrap(); + let err = diff_single_file(Some(&baseline), Some(&payload), rel, &meta).unwrap_err(); + assert!( + err.to_string().contains("baseline bytes are not UTF-8"), + "{err}" + ); + } + #[test] fn diffs_optional_and_peer_dependencies() { let old_dir = tempfile::tempdir().unwrap(); diff --git a/src/heuristic.rs b/src/heuristic.rs index f16f7a8..2a67e03 100644 --- a/src/heuristic.rs +++ b/src/heuristic.rs @@ -6,24 +6,6 @@ use crate::registry::Ecosystem; use crate::verdict::{DiffSummary, Finding, TrustSources, Verdict, VerdictBand}; use crate::version::VersionInfo; -#[allow(dead_code)] -pub fn evaluate( - name: &str, - ecosystem: Ecosystem, - integrity: &str, - delta: &Delta, - is_unreviewed_baseline: bool, -) -> Verdict { - evaluate_with_policy( - name, - ecosystem, - integrity, - delta, - is_unreviewed_baseline, - &Policy::default(), - ) -} - pub fn evaluate_with_policy( name: &str, ecosystem: Ecosystem, @@ -2081,12 +2063,13 @@ mod tests { binding_gyp_added: false, }; - let verdict = evaluate( + let verdict = evaluate_with_policy( "test-pkg", Ecosystem::Npm, "verified (sha512)", &delta, false, + &Policy::default(), ); assert_eq!(verdict.band, VerdictBand::Block); assert!(verdict.risk_score >= 50); @@ -2125,12 +2108,13 @@ mod tests { binding_gyp_added: false, }; - let verdict = evaluate( + let verdict = evaluate_with_policy( "test-pkg", Ecosystem::Npm, "verified (sha512)", &delta, false, + &Policy::default(), ); assert_eq!(verdict.band, VerdictBand::High); assert!( @@ -2174,12 +2158,13 @@ mod tests { binding_gyp_added: false, }; - let verdict = evaluate( + let verdict = evaluate_with_policy( "test-pkg", Ecosystem::Npm, "verified (sha512)", &delta, false, + &Policy::default(), ); assert_eq!(verdict.band, VerdictBand::High); assert!( @@ -2441,12 +2426,13 @@ mod tests { binding_gyp_added: true, }; - let verdict = evaluate( + let verdict = evaluate_with_policy( "test-pkg", Ecosystem::Npm, "verified (sha512)", &delta, false, + &Policy::default(), ); assert_eq!(verdict.band, VerdictBand::Block); assert!( @@ -2500,12 +2486,13 @@ mod tests { binding_gyp_added: false, }; - let verdict = evaluate( + let verdict = evaluate_with_policy( "test-pkg", Ecosystem::Npm, "verified (sha512)", &delta, false, + &Policy::default(), ); assert_eq!(verdict.band, VerdictBand::High); assert!( @@ -2543,12 +2530,13 @@ mod tests { binding_gyp_added: false, }; - let verdict = evaluate( + let verdict = evaluate_with_policy( "test-pkg", Ecosystem::Npm, "verified (sha512)", &delta, false, + &Policy::default(), ); assert_eq!(verdict.band, VerdictBand::Medium); assert_eq!(verdict.risk_score, 15); @@ -2580,12 +2568,13 @@ mod tests { binding_gyp_added: false, }; - let verdict = evaluate( + let verdict = evaluate_with_policy( "test-pkg", Ecosystem::Npm, "verified (sha512)", &delta, true, + &Policy::default(), ); assert_eq!(verdict.band, VerdictBand::Medium); assert!( @@ -2788,12 +2777,13 @@ mod tests { binding_gyp_added: false, }; - let verdict = evaluate( + let verdict = evaluate_with_policy( "test-pkg", Ecosystem::Npm, "verified (sha512)", &delta, false, + &Policy::default(), ); assert_eq!(verdict.band, VerdictBand::High); assert!( @@ -2828,12 +2818,13 @@ mod tests { binding_gyp_added: false, }; - let verdict = evaluate( + let verdict = evaluate_with_policy( "test-pkg", Ecosystem::Npm, "verified (sha512)", &delta, false, + &Policy::default(), ); assert_eq!(verdict.band, VerdictBand::High); assert!( @@ -2913,12 +2904,13 @@ mod tests { binding_gyp_added: false, }; - let verdict = evaluate( + let verdict = evaluate_with_policy( "test-pkg", Ecosystem::Npm, "verified (sha512)", &delta, false, + &Policy::default(), ); assert_eq!(verdict.risk_score, 100); assert_eq!(verdict.band, VerdictBand::Block); @@ -4317,12 +4309,13 @@ maintainers = ["badactor@example.com"] #[test] fn live_band_ladder_is_monotonic_and_not_a_pure_function_of_score() { // One HIGH finding: 25 points, and HIGH because a finding raised it. - let verdict = evaluate( + let verdict = evaluate_with_policy( "test-pkg", Ecosystem::Npm, "sha512-x", &opaque_large_file_delta(&["payload.bin"]), false, + &Policy::default(), ); assert_eq!(verdict.risk_score, 25, "one HIGH finding is 25 points"); assert_eq!( @@ -4371,12 +4364,13 @@ maintainers = ["badactor@example.com"] description: "appended after evaluation".into(), }; - let mut late = evaluate( + let mut late = evaluate_with_policy( "test-pkg", Ecosystem::Npm, "sha512-x", &opaque_large_file_delta(&["a.bin"]), false, + &Policy::default(), ); assert_eq!((late.band, late.risk_score), (VerdictBand::High, 25)); apply_extra_findings( @@ -4384,12 +4378,13 @@ maintainers = ["badactor@example.com"] vec![high("R02_OPAQUE_LARGE_FILE_ADDED")], &policy, ); - let native = evaluate( + let native = evaluate_with_policy( "test-pkg", Ecosystem::Npm, "sha512-x", &opaque_large_file_delta(&["a.bin", "b.bin"]), false, + &Policy::default(), ); assert_eq!(native.risk_score, 50); assert_eq!( @@ -4400,12 +4395,13 @@ maintainers = ["badactor@example.com"] // A late MEDIUM on top of a HIGH: 35 points stays HIGH only because the // ladder is monotonic. A score-only map on both ladders would say MEDIUM. - let mut late_medium = evaluate( + let mut late_medium = evaluate_with_policy( "test-pkg", Ecosystem::Npm, "sha512-x", &opaque_large_file_delta(&["a.bin"]), false, + &Policy::default(), ); apply_extra_findings( &mut late_medium, diff --git a/src/registry/cratesio.rs b/src/registry/cratesio.rs index 24d911e..ba89f44 100644 --- a/src/registry/cratesio.rs +++ b/src/registry/cratesio.rs @@ -325,7 +325,10 @@ fn sparse_index_path(canonical: &str) -> Result { Ok(path) } -/// Lowercase the crate name, matching crates.io index paths. +/// Lowercase the crate name, matching crates.io index paths. Hyphen and +/// underscore spellings are deliberately NOT folded: a configured index or +/// mirror may genuinely serve both, and fetching the other spelling's bytes +/// for a request is worse than refusing the one that was asked for. pub fn canonical_crate_name(name: &str) -> String { name.to_ascii_lowercase() } @@ -478,6 +481,13 @@ mod tests { assert_eq!(canonical_crate_name("Serde_JSON"), "serde_json"); assert_eq!(canonical_crate_name("tokio"), "tokio"); assert_eq!(canonical_crate_name("MiXeD_CaSe"), "mixed_case"); + // Hyphen and underscore spellings stay distinct on purpose: the real + // crates.io index only stores the folded file, but a configured + // mirror may serve both, and answering one spelling with the other's + // bytes is worse than refusing. `lockfile_diff_keeps_pypi_alias_ + // off_cargo_pins` pins the same invariant from the ci lane. + assert_eq!(canonical_crate_name("Serde-JSON"), "serde-json"); + assert_eq!(canonical_crate_name("rocket-db-pools"), "rocket-db-pools"); } #[test] diff --git a/src/registry/pypi.rs b/src/registry/pypi.rs index 05022b7..71e8e86 100644 --- a/src/registry/pypi.rs +++ b/src/registry/pypi.rs @@ -407,21 +407,16 @@ struct SimpleMeta { #[serde(default)] next: Option, } -#[allow(dead_code)] #[derive(Debug, Deserialize)] struct SimpleFile { filename: String, url: String, #[serde(default)] hashes: BTreeMap, - #[serde(default)] - size: Option, #[serde(default, rename = "upload-time")] upload_time: Option, #[serde(default)] yanked: YankedField, - #[serde(default)] - provenance: Option, } #[derive(Debug, Deserialize, Default)] #[serde(untagged)] @@ -805,10 +800,8 @@ mod tests { filename: n.into(), url: "https://e.com/a.whl".into(), hashes: BTreeMap::new(), - size: None, upload_time: None, yanked: YankedField::NotYanked, - provenance: None, }; let a = mk("pkg-1.0-cp311-cp311-linux_x86_64.whl"); let b = mk("pkg-1.0-py3-none-any.whl");