M6: Fail closed on missing/unfetchable SHA256SUMS in the auto-updater #202

New Issue

2026-07-28T14:04:48Z

KofTwentyTwo commented

2026-07-28 14:04:48 +00:00

(Migrated from github.com)

Context

Story for milestone M6. Read docs/STORY-WORKFLOW.md first — it defines the process, gates, and patterns; this issue defines only the specific work. crates/nuncio-core/src/update.rs::UpdateEngine::download_and_verify (update.rs:226-266) is meant to reject an update package whose SHA-256 checksum doesn't match SHA256SUMS.txt. Verified in the tree: it fails OPEN in two distinct ways instead of failing closed:

if let Some(checksum_url) = &release_info.checksum_url {
    let sums_resp = self.client.get(checksum_url).send().await?;
    if sums_resp.status().is_success() {
        // ... verify hash, error on mismatch or filename-not-found ...
    }
    // <-- if sums_resp.status() is NOT success, nothing happens here: falls through
}
// <-- if release_info.checksum_url is None at all, this whole block is skipped
Ok(archive_bytes)

(update.rs:241-263, update.rs:265). Two concrete fail-open paths: (1) ReleaseInfo::checksum_url is Some only when the GitHub release happens to have a SHA256SUMS.txt asset attached (set in check_for_updates, update.rs:199-203) — if it's missing, download_and_verify verifies nothing and returns the downloaded archive bytes as-is; (2) even when checksum_url is present, if the HTTP GET for it returns a non-2xx status, the code silently skips verification instead of erroring. There is no signature verification (GPG/minisign) anywhere in this file — only the SHA-256 checksum path exists, and only checksum verification is in scope for this story (reconciles the reported "fail-open when SHA256SUMS is absent" defect; #162's cipher issue is unrelated to this file). apply_update (update.rs:269-313) calls download_and_verify and would silently apply an unverified binary swap in both fail-open cases above. Separately, crates/nunciod/src/main.rs:160-170 already gates the autonomous update-check loop off by default (AUTO_UPDATE_ENV_VAR, logged as "update verification is not yet safe") — that gate is correct and MUST remain off; this story does not touch main.rs.

Goal

Make download_and_verify fail closed: it must return an Err — never Ok(archive_bytes) — whenever the checksum cannot be positively verified, whether because no SHA256SUMS.txt asset exists on the release, the asset couldn't be fetched, or the fetched file doesn't contain an entry for the target archive. The only path to Ok must be an exact, matching SHA-256 hash. No behavior change to check_for_updates (reporting availability is not the security boundary; downloading/applying is).

Files to touch / patterns to mirror

crates/nuncio-core/src/update.rs:39-41 (UpdateError::ChecksumFileError) — reuse this variant (it already exists for "filename not found in SHA256SUMS.txt", update.rs:257-260) for the two new fail-closed paths, or add a distinct variant (e.g. ChecksumUnavailable(String)) if you want the three failure modes (missing asset / fetch failed / filename not found) to be distinguishable in logs — either is acceptable; pick one and be consistent.
crates/nuncio-core/src/update.rs:226-266 (download_and_verify) — restructure so:
1. If release_info.checksum_url is None, return Err immediately (do not download the archive at all, or download but never return it — prefer failing before the archive download to avoid wasted bandwidth, matching the existing code's ordering where the archive is downloaded first at update.rs:231-238; either ordering is acceptable as long as the final Ok path is unreachable without verification).
2. If the SHA256SUMS.txt GET (update.rs:242) returns a non-success status, return Err (today: silently falls through).
3. Keep the existing mismatch (update.rs:249-255) and filename-not-found (update.rs:256-261) error paths unchanged — they already fail closed correctly.
crates/nuncio-core/src/update.rs:513-663 (#[cfg(test)] mod tests) — the existing check_for_updates_with_wiremock test (update.rs:623-662) is the template for the new wiremock-based tests this story needs (GitHub API + asset download mocked via wiremock::MockServer, matching the project rule that "updater tests use wiremock for the GitHub API").

Implementation steps

Decide on the error-variant approach (reuse ChecksumFileError vs. add a new variant) and implement it.
Change download_and_verify so release_info.checksum_url.is_none() returns Err(..) — write a clear message like "no SHA256SUMS.txt asset found for this release; refusing to apply an unverified update".
Change the if sums_resp.status().is_success() branch (update.rs:243) so the else case (non-success HTTP status fetching SHA256SUMS.txt) also returns Err(..) with the HTTP status included in the message, instead of falling through silently.
Re-read the whole function afterward to confirm there is no remaining code path that reaches Ok(archive_bytes) (update.rs:265) without having gone through a successful hash comparison at update.rs:249.
Add wiremock-based tests per Acceptance Criteria, mirroring check_for_updates_with_wiremock (update.rs:623-662) for HTTP mocking style, and download_and_verify's existing (if any — verify; add if missing) success-path test for the "happy path" baseline to diff against.

Acceptance criteria

download_and_verify returns Err (never Ok) when release_info.checksum_url is None. Test: construct a ReleaseInfo with checksum_url: None, wiremock the archive download endpoint to succeed, call download_and_verify, assert Err.
download_and_verify returns Err when the SHA256SUMS.txt URL responds with a non-2xx status (e.g. wiremock a 404 for that path). Test asserts Err.
download_and_verify still returns Err on a hash mismatch (existing behavior — add an explicit wiremock test if one doesn't already exist covering this exact function, since the current test suite tests parse_sha256sums directly at update.rs:529-545 but does not appear to wiremock-test download_and_verify's mismatch path end-to-end — verify this gap and close it).
download_and_verify returns Ok(archive_bytes) only when the checksum matches exactly (positive test, wiremock both endpoints with a correct SHA-256 for known bytes computed via compute_sha256, update.rs:341-345).
No fabricated success: no path returns Ok without a verified matching checksum.
crates/nunciod/src/main.rs's autonomous-update gate (AUTO_UPDATE_ENV_VAR, currently off by default) is untouched by this story — confirm in the PR description that it remains disabled.
cargo verify green (fmt + clippy -D warnings all-targets + test-all), 0 ignored. Updater tests use wiremock for the simulated GitHub API / asset endpoints; no live network calls.

Definition of done & process

Follow docs/STORY-WORKFLOW.md: no fabricated data; comment convention (intent-only, no story/GH/phase refs in .rs/.proto); mock-only tests. Branch story/m6-updater-fail-closed off dev; PR to dev with the gate output pasted as evidence; the PO reviews before the issue is closed.

Dependencies / notes

Not a [PROTO] story. No overlap with other M6 stories' files (crates/nuncio-core/src/update.rs is untouched elsewhere in this milestone) — safe to run fully in parallel. Out of scope: adding cryptographic signature verification (GPG/minisign) on top of the SHA-256 checksum — the roadmap/backlog does not currently call for it; if the PO wants it, file a separate follow-up rather than expanding this story. Size: M.

## Context Story for milestone M6. **Read [`docs/STORY-WORKFLOW.md`](https://github.com/KofTwentyTwo/nuncio/blob/dev/docs/STORY-WORKFLOW.md) first** — it defines the process, gates, and patterns; this issue defines only the specific work. `crates/nuncio-core/src/update.rs::UpdateEngine::download_and_verify` (`update.rs:226-266`) is meant to reject an update package whose SHA-256 checksum doesn't match `SHA256SUMS.txt`. Verified in the tree: it fails OPEN in two distinct ways instead of failing closed: ```rust if let Some(checksum_url) = &release_info.checksum_url { let sums_resp = self.client.get(checksum_url).send().await?; if sums_resp.status().is_success() { // ... verify hash, error on mismatch or filename-not-found ... } // <-- if sums_resp.status() is NOT success, nothing happens here: falls through } // <-- if release_info.checksum_url is None at all, this whole block is skipped Ok(archive_bytes) ``` (`update.rs:241-263`, `update.rs:265`). Two concrete fail-open paths: (1) `ReleaseInfo::checksum_url` is `Some` only when the GitHub release happens to have a `SHA256SUMS.txt` asset attached (set in `check_for_updates`, `update.rs:199-203`) — if it's missing, `download_and_verify` verifies nothing and returns the downloaded archive bytes as-is; (2) even when `checksum_url` is present, if the HTTP GET for it returns a non-2xx status, the code silently skips verification instead of erroring. There is no signature verification (GPG/minisign) anywhere in this file — only the SHA-256 checksum path exists, and only checksum verification is in scope for this story (reconciles the reported "fail-open when SHA256SUMS is absent" defect; #162's cipher issue is unrelated to this file). `apply_update` (`update.rs:269-313`) calls `download_and_verify` and would silently apply an unverified binary swap in both fail-open cases above. Separately, `crates/nunciod/src/main.rs:160-170` already gates the autonomous update-check loop off by default (`AUTO_UPDATE_ENV_VAR`, logged as "update verification is not yet safe") — that gate is correct and MUST remain off; this story does not touch `main.rs`. ## Goal Make `download_and_verify` fail closed: it must return an `Err` — never `Ok(archive_bytes)` — whenever the checksum cannot be positively verified, whether because no `SHA256SUMS.txt` asset exists on the release, the asset couldn't be fetched, or the fetched file doesn't contain an entry for the target archive. The only path to `Ok` must be an exact, matching SHA-256 hash. No behavior change to `check_for_updates` (reporting availability is not the security boundary; downloading/applying is). ## Files to touch / patterns to mirror - `crates/nuncio-core/src/update.rs:39-41` (`UpdateError::ChecksumFileError`) — reuse this variant (it already exists for "filename not found in SHA256SUMS.txt", `update.rs:257-260`) for the two new fail-closed paths, or add a distinct variant (e.g. `ChecksumUnavailable(String)`) if you want the three failure modes (missing asset / fetch failed / filename not found) to be distinguishable in logs — either is acceptable; pick one and be consistent. - `crates/nuncio-core/src/update.rs:226-266` (`download_and_verify`) — restructure so: 1. If `release_info.checksum_url` is `None`, return `Err` immediately (do not download the archive at all, or download but never return it — prefer failing before the archive download to avoid wasted bandwidth, matching the existing code's ordering where the archive is downloaded first at `update.rs:231-238`; either ordering is acceptable as long as the final `Ok` path is unreachable without verification). 2. If the `SHA256SUMS.txt` GET (`update.rs:242`) returns a non-success status, return `Err` (today: silently falls through). 3. Keep the existing mismatch (`update.rs:249-255`) and filename-not-found (`update.rs:256-261`) error paths unchanged — they already fail closed correctly. - `crates/nuncio-core/src/update.rs:513-663` (`#[cfg(test)] mod tests`) — the existing `check_for_updates_with_wiremock` test (`update.rs:623-662`) is the template for the new wiremock-based tests this story needs (GitHub API + asset download mocked via `wiremock::MockServer`, matching the project rule that "updater tests use wiremock for the GitHub API"). ## Implementation steps 1. Decide on the error-variant approach (reuse `ChecksumFileError` vs. add a new variant) and implement it. 2. Change `download_and_verify` so `release_info.checksum_url.is_none()` returns `Err(..)` — write a clear message like "no SHA256SUMS.txt asset found for this release; refusing to apply an unverified update". 3. Change the `if sums_resp.status().is_success()` branch (`update.rs:243`) so the `else` case (non-success HTTP status fetching `SHA256SUMS.txt`) also returns `Err(..)` with the HTTP status included in the message, instead of falling through silently. 4. Re-read the whole function afterward to confirm there is no remaining code path that reaches `Ok(archive_bytes)` (`update.rs:265`) without having gone through a successful hash comparison at `update.rs:249`. 5. Add wiremock-based tests per Acceptance Criteria, mirroring `check_for_updates_with_wiremock` (`update.rs:623-662`) for HTTP mocking style, and `download_and_verify`'s existing (if any — verify; add if missing) success-path test for the "happy path" baseline to diff against. ## Acceptance criteria - [ ] `download_and_verify` returns `Err` (never `Ok`) when `release_info.checksum_url` is `None`. Test: construct a `ReleaseInfo` with `checksum_url: None`, wiremock the archive download endpoint to succeed, call `download_and_verify`, assert `Err`. - [ ] `download_and_verify` returns `Err` when the `SHA256SUMS.txt` URL responds with a non-2xx status (e.g. wiremock a `404` for that path). Test asserts `Err`. - [ ] `download_and_verify` still returns `Err` on a hash mismatch (existing behavior — add an explicit wiremock test if one doesn't already exist covering this exact function, since the current test suite tests `parse_sha256sums` directly at `update.rs:529-545` but does not appear to wiremock-test `download_and_verify`'s mismatch path end-to-end — verify this gap and close it). - [ ] `download_and_verify` returns `Ok(archive_bytes)` only when the checksum matches exactly (positive test, wiremock both endpoints with a correct SHA-256 for known bytes computed via `compute_sha256`, `update.rs:341-345`). - [ ] No fabricated success: no path returns `Ok` without a verified matching checksum. - [ ] `crates/nunciod/src/main.rs`'s autonomous-update gate (`AUTO_UPDATE_ENV_VAR`, currently off by default) is untouched by this story — confirm in the PR description that it remains disabled. - [ ] `cargo verify` green (fmt + clippy `-D warnings` all-targets + test-all), 0 ignored. Updater tests use `wiremock` for the simulated GitHub API / asset endpoints; no live network calls. ## Definition of done & process Follow [`docs/STORY-WORKFLOW.md`](https://github.com/KofTwentyTwo/nuncio/blob/dev/docs/STORY-WORKFLOW.md): no fabricated data; comment convention (intent-only, no story/GH/phase refs in `.rs`/`.proto`); mock-only tests. Branch `story/m6-updater-fail-closed` off `dev`; PR to `dev` with the gate output pasted as evidence; the PO reviews before the issue is closed. ## Dependencies / notes Not a [PROTO] story. No overlap with other M6 stories' files (`crates/nuncio-core/src/update.rs` is untouched elsewhere in this milestone) — safe to run fully in parallel. Out of scope: adding cryptographic signature verification (GPG/minisign) on top of the SHA-256 checksum — the roadmap/backlog does not currently call for it; if the PO wants it, file a separate follow-up rather than expanding this story. Size: M.

KofTwentyTwo commented

2026-07-30 17:56:01 +00:00

(Migrated from github.com)

Delivered via PR #242 (squash-merged to dev). download_and_verify now fails closed: the only path to Ok is an exact SHA-256 match. Absent SHA256SUMS.txt asset (checksum_url None) and non-2xx sums fetch both now return Err(ChecksumUnavailable) instead of silently returning unverified archive bytes; filename-not-found and hash mismatch already failed closed and remain so. Four wiremock tests cover each path. main.rs autonomous-update gate untouched (still off). Reconciles the fail-open-when-SHA256SUMS-absent defect.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#202