Fixed two spots that failed toward the permissive answer

CHANGELOG.md

@@ -10,6 +10,31 @@ and this project adheres to [Semantic Versioning](https://semver.org/).
 Targeted at 0.4.0. This release is a behaviour change for anyone who set
 `require_provenance`: see the entry below before upgrading.
 
+### 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-<base64>` 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

references/drift.md

@@ -20,8 +20,8 @@ Severity is this file's own judgement, not the project's.
 | 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 |
-| 12 | `store.rs` discards the `0o600` pre-create error (`let _ =`) | Low — the DB may be more permissive than intended |
-| 9 | `hex_to_bytes` uses `.unwrap_or(0)`, mapping invalid hex to byte `0` | Low — fails toward "valid-looking" |
+| 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 |
 | 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)]` |
@@ -141,8 +141,12 @@ Each finding is cited in its lane file with the exact source location.
    a name that resolved publicly for the check and privately for the
    connection no longer gets through. Only the configured base is exempt.
    See [registry](features/registry.md).
-9. **`registry/mod.rs::hex_to_bytes` uses `.unwrap_or(0)`**, silently mapping
-   invalid hex to byte `0`.
+9. **~~`registry/mod.rs::hex_to_bytes` uses `.unwrap_or(0)`~~ — resolved.** The
+   helper maps an unparseable byte pair to `0`, silently mapping invalid hex to
+   byte `0`. It is fallible now and `hex_to_bytes` is the only caller of
+   `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
     [diff](features/diff.md).
@@ -154,4 +158,12 @@ Each finding is cited in its lane file with the exact source location.
     refusals name which of those failed. Test
     `a_pypi_archive_whose_metadata_cannot_be_read_is_refused`. See
     [review](features/review.md).
-12. **`store.rs` silently discards the `0o600` pre-create error** (`let _ =`).
+12. **~~`store.rs` silently discards the `0o600` pre-create error~~ — resolved.**
+    Three hardening steps were `let _ = ...`: the data-directory chmod to
+    `0o700`, the pre-create of the database at `0o600`, and the database chmod
+    to `0o600`. All three propagate now. The pre-create was the load-bearing
+    one, because `Connection::open` creates the file at whatever the umask
+    allows, so a failed pre-create meant a database at a default mode and a
+    narrowing step that had failed in silence. Test
+    `an_unopenable_database_path_refuses_at_the_pre_create`. See
+    [store](features/store.md).

references/features/registry.md

@@ -73,9 +73,14 @@ RegistryLimits::default()
   max_redirects       = 5
 ```
 
-**`hex_to_bytes` uses `.unwrap_or(0)`** — an invalid hex pair silently becomes
-byte `0` instead of erroring. The only non-fail-closed spot in `mod.rs`,
-reachable only via `to_sri()` on an already-validated `value_hex`.
+**`hex_to_bytes` is fallible** — an invalid hex pair is a refusal naming the
+offending digest, not byte `0`. `to_sri` returns `Result` for this reason: the
+decoder cannot produce a well-formed SRI string from a digest it could not read,
+and the only place `to_sri` renders is the mismatch message naming the expected
+value, where a silently zeroed byte put the wrong bytes in a confident sentence.
+Reachable only through direct `Checksum` construction, since `Checksum::parse`
+validates its hex before storing it. Tests `a_malformed_digest_fails_loud_
+instead_of_zeroing` and `a_well_formed_digest_still_renders`.
 
 **`Checksum::parse` takes the FIRST recognized token of a whitespace-separated
 list**, so npm's multi-algorithm `dist.integrity` resolves deterministically.

references/features/store.md

@@ -71,10 +71,14 @@ Audit `action` values actually written (free-form `&str`, not an enum):
   `_new` copies** (PKs change to composites) and only `ALTER`s `audit_log`. The
   advisory expiry index is dropped with the old table and must be recreated
   after the rename.
-- **Filesystem hardening is `#[cfg(unix)]` and mostly best-effort.** Parent dir
-  `0o700`, DB `0o600`, symlink re-checked *before* and *after*
-  `Connection::open`; `set_permissions` and the pre-create `OpenOptions` both
-  use `let _ =`, so they are not fail-closed.
+- **Filesystem hardening is `#[cfg(unix)` and fail-closed on every step.** Parent
+  dir `0o700`, DB `0o600`, symlink re-checked *before* and *after*
+  `Connection::open`; `set_permissions` and the pre-create `OpenOptions` all
+  propagate their errors. The pre-create is the one that matters, because
+  `Connection::open` creates the file at whatever the umask allows, so a
+  swallowed pre-create failure meant a database at a default mode and a
+  narrowing step that failed in silence. Test
+  `an_unopenable_database_path_refuses_at_the_pre_create`.
 - **`AuditEntry` has `row.get::<_, i64>(5)? as u32`** — an unchecked narrowing
   cast. And `hit_count`/`slsa_level` use `.max(0) as u*`, which coerces a
   negative corrupted value to `0` rather than erroring.

src/registry/mod.rs

@@ -117,13 +117,19 @@ impl Checksum {
 
     /// npm SRI form (`sha512-<base64>`), used when talking to npm-shaped
     /// registries and preserved for backwards-compatible display.
-    pub fn to_sri(&self) -> String {
+    ///
+    /// Fallible because decoding the digest is. The old form mapped an
+    /// unparseable hex pair to `0`, so a malformed digest rendered as a
+    /// well-formed SRI string and the only place an operator ever saw it was a
+    /// mismatch message naming the expected value, which then illustrated the
+    /// wrong bytes without saying so.
+    pub fn to_sri(&self) -> Result<String, BluelineError> {
         match self.alg {
-            ChecksumAlg::Sha512 => format!(
+            ChecksumAlg::Sha512 => Ok(format!(
                 "sha512-{}",
-                base64::engine::general_purpose::STANDARD.encode(hex_to_bytes(&self.value_hex))
-            ),
-            ChecksumAlg::Sha256 => self.to_display(),
+                base64::engine::general_purpose::STANDARD.encode(hex_to_bytes(&self.value_hex)?)
+            )),
+            ChecksumAlg::Sha256 => Ok(self.to_display()),
         }
     }
 
@@ -137,9 +143,23 @@ pub fn hex_encode(bytes: &[u8]) -> String {
     bytes.iter().map(|b| format!("{b:02x}")).collect()
 }
 
-fn hex_to_bytes(hex: &str) -> Vec<u8> {
+/// Decode a lowercase hex digest into bytes. Fail closed on anything that is
+/// not clean hex, rather than mapping an unparseable pair to `0`, which would
+/// produce a digest that still decodes and still compares as a mismatch later.
+fn hex_to_bytes(hex: &str) -> Result<Vec<u8>, BluelineError> {
+    if !hex.len().is_multiple_of(2) {
+        return Err(BluelineError::Verification(format!(
+            "digest `{hex}` has an odd number of hex digits"
+        )));
+    }
     (0..hex.len() / 2)
-        .map(|i| u8::from_str_radix(&hex[2 * i..2 * i + 2], 16).unwrap_or(0))
+        .map(|i| {
+            u8::from_str_radix(&hex[2 * i..2 * i + 2], 16).map_err(|_| {
+                BluelineError::Verification(format!(
+                    "digest `{hex}` contains a non-hexadecimal pair at byte {i}"
+                ))
+            })
+        })
         .collect()
 }
 
@@ -321,11 +341,53 @@ mod tests {
     #[test]
     fn sri_round_trips_through_display() {
         let c = Checksum::parse(TEST_SRI).unwrap();
-        assert_eq!(c.to_sri(), TEST_SRI);
+        assert_eq!(c.to_sri().unwrap(), TEST_SRI);
         let again = Checksum::parse(&c.to_display()).unwrap();
         assert_eq!(again, c);
     }
 
+    /// The old `hex_to_bytes` mapped an unparseable hex pair to `0`, so a
+    /// malformed digest rendered as a well-formed SRI string. The only place it
+    /// surfaced was a mismatch message naming the expected value, where the
+    /// zeroed bytes illustrated the wrong answer without saying so.
+    #[test]
+    fn a_malformed_digest_fails_loud_instead_of_zeroing() {
+        let mut bad = Checksum::parse(TEST_SRI).unwrap();
+        bad.value_hex = TEST_HEX[..126].to_string();
+        bad.value_hex.push_str("zz");
+
+        let err = bad
+            .to_sri()
+            .expect_err("a non-hexadecimal digest must not render as an SRI string");
+        let msg = err.to_string();
+        assert!(
+            msg.contains("zz") && msg.contains("non-hexadecimal"),
+            "the refusal must name the offending digest, got: {msg}"
+        );
+
+        // And an odd-length one, which the old `len() / 2` truncated silently.
+        let mut odd = Checksum::parse(TEST_SRI).unwrap();
+        odd.value_hex.pop();
+        let err = odd
+            .to_sri()
+            .expect_err("an odd-length digest must not be silently truncated");
+        assert!(
+            err.to_string().contains("odd number of hex digits"),
+            "got: {}",
+            err
+        );
+    }
+
+    /// The honest path still round-trips: this is what would have caught a fix
+    /// that made every digest refuse.
+    #[test]
+    fn a_well_formed_digest_still_renders() {
+        let c = Checksum::parse(TEST_HEX).unwrap();
+        let sri = c.to_sri().unwrap();
+        assert!(sri.starts_with("sha512-"), "got {sri}");
+        assert_eq!(Checksum::parse(&sri).unwrap(), c);
+    }
+
     #[test]
     fn picks_sha512_from_multi_algorithm_list() {
         let c = Checksum::parse(&format!("sha1-YWJjZA== {TEST_SRI}")).unwrap();

src/registry/npm.rs

@@ -128,12 +128,16 @@ impl NpmRegistry {
         match &pkg.integrity {
             Some(expected) if expected.alg == ChecksumAlg::Sha512 => {
                 if !expected.value_hex.eq_ignore_ascii_case(&computed.value_hex) {
+                    // Both digests are produced by `Checksum::parse` or
+                    // `hex_encode`, so neither can fail here; `?` keeps it loud
+                    // if that ever stops being true rather than rendering a
+                    // digest with bytes silently zeroed.
                     return Err(BluelineError::Verification(format!(
                         "{}@{}: tarball sha512 mismatch (expected {}, got {})",
                         pkg.name,
                         pkg.version,
-                        expected.to_sri(),
-                        computed.to_sri()
+                        expected.to_sri()?,
+                        computed.to_sri()?
                     )));
                 }
             }

src/store.rs

@@ -355,7 +355,12 @@ impl BaselineStore {
                     use std::os::unix::fs::PermissionsExt;
                     let mut perms = meta.permissions();
                     perms.set_mode(0o700);
-                    let _ = fs::set_permissions(parent, perms);
+                    fs::set_permissions(parent, perms).map_err(|e| {
+                        BluelineError::Store(format!(
+                            "setting permissions on {}: {e}",
+                            parent.display()
+                        ))
+                    })?;
                 }
             }
             #[cfg(not(unix))]
@@ -376,12 +381,22 @@ impl BaselineStore {
                 ));
             }
             use std::os::unix::fs::OpenOptionsExt;
-            let _ = fs::OpenOptions::new()
+            // Pre-create the file at 0o600 so it is never briefly world-readable
+            // before SQLite opens it: `Connection::open` would otherwise create
+            // it at whatever the umask allows, and the chmod after open cannot
+            // close a window it did not measure. Failure is fatal rather than
+            // ignored, because the step below creates the file anyway, so an
+            // ignored error here is exactly the permission the mode was chosen
+            // to avoid.
+            fs::OpenOptions::new()
                 .write(true)
                 .create(true)
                 .truncate(false)
                 .mode(0o600)
-                .open(path);
+                .open(path)
+                .map_err(|e| {
+                    BluelineError::Store(format!("creating {} at mode 0o600: {e}", path.display()))
+                })?;
         }
 
         let mut conn = rusqlite::Connection::open(path)
@@ -1003,7 +1018,7 @@ mod tests {
                 Ecosystem::Npm,
                 "pkg",
                 "1.0.0",
-                &Checksum::parse(&ck("one").to_sri()).unwrap(),
+                &Checksum::parse(&ck("one").to_sri().unwrap()).unwrap(),
             )
             .unwrap();
         assert_eq!(
@@ -1334,7 +1349,7 @@ mod tests {
             migrations.to_latest(&mut conn).unwrap();
             conn.execute(
                 "INSERT INTO known_clean (name, version, integrity, clean) VALUES ('legacy', '1.0.0', ?1, 1)",
-                rusqlite::params![legacy_checksum.to_sri()],
+                rusqlite::params![legacy_checksum.to_sri().unwrap()],
             )
             .unwrap();
             conn.execute(
@@ -1365,7 +1380,7 @@ mod tests {
         assert_eq!(clean.len(), 1);
         assert_eq!(clean[0].0.to_string(), "1.0.0");
         // The legacy SRI value survives verbatim and normalizes to the same content.
-        assert_eq!(clean[0].1, legacy_checksum.to_sri());
+        assert_eq!(clean[0].1, legacy_checksum.to_sri().unwrap());
         assert_eq!(Checksum::parse(&clean[0].1).unwrap(), legacy_checksum);
 
         assert!(
@@ -1495,6 +1510,35 @@ mod tests {
         assert_eq!(file_meta.permissions().mode() & 0o777, 0o600);
     }
 
+    /// The pre-create at 0o600 is load-bearing, and its error used to be
+    /// discarded with `let _ =`. Dropping it silently was the defect: the step
+    /// below creates the file anyway, so an ignored failure simply meant SQLite
+    /// made it at the umask default, which is what the mode exists to prevent.
+    #[test]
+    #[cfg(unix)]
+    fn an_unopenable_database_path_refuses_at_the_pre_create() {
+        let dir = tempfile::tempdir().unwrap();
+        // A directory at the path the database should occupy: everything up to
+        // the pre-create succeeds, which is what isolates that arm. The old
+        // `let _ =` swallowed this and let SQLite fail later with a message that
+        // named no mode and no reason to suspect the permissions.
+        let db_path = dir.path().join("t.db");
+        fs::create_dir(&db_path).unwrap();
+
+        let err = BaselineStore::open_at(&db_path).err().expect(
+            "a database path that cannot be opened must refuse, not fall through to SQLite",
+        );
+        let msg = err.to_string();
+        assert!(
+            msg.contains("at mode 0o600"),
+            "the refusal must come from the pre-create and name the mode it wanted, got: {msg}"
+        );
+        assert!(
+            msg.contains("t.db"),
+            "the refusal must name the path it could not use, got: {msg}"
+        );
+    }
+
     #[test]
     fn advisory_cache_roundtrip_and_clear() {
         let dir = tempfile::tempdir().unwrap();

tests/cli.rs

@@ -247,7 +247,8 @@ fn full_flow_json_output() {
     assert_eq!(
         blueline::registry::Checksum::parse(displayed)
             .unwrap()
-            .to_sri(),
+            .to_sri()
+            .unwrap(),
         integrity
     );
     assert_eq!(json["ecosystem"], "npm");
@@ -272,7 +273,7 @@ fn full_flow_json_output() {
     // The witness is stored in the canonical display form; it must normalize
     // to the same digest content as the SRI the fixture served.
     let stored = blueline::registry::Checksum::parse(&integrity_row).unwrap();
-    assert_eq!(stored.to_sri(), integrity);
+    assert_eq!(stored.to_sri().unwrap(), integrity);
     assert_eq!(clean, 0, "review records evidence, never a clean blessing");
 }