Skip to content

Agent-mode apply and rollback reject a manifest hash in uppercase hex that blob download accepts as valid #707

Description

[agent] Filed by the scheduled architecture audit routine (CLI and core). Register: discussion #560 register.

Kind: bug. Source: new finding; register C41. It is related to C17 (#706).

Problem (main @ 045d7ec)

The case policy for comparing a hash is implemented separately at each site, and the sites disagree:

  • Blob download is case-insensitive on purpose. blob_fetcher::blob_hash_matches uses eq_ignore_ascii_case. Its doc says the client's validator "accepts uppercase hex too — so a manifest … that uses uppercase would download byte-for-byte correct content and then be wrongly rejected by a case-sensitive comparison", and a regression test pins this. apply::is_valid_blob_hash and client::is_valid_sha256_hex both accept uppercase as well.
  • Agent-mode verification is case-sensitive. compute_git_sha256_from_bytes emits lowercase, and it is compared with ==/!=:
  • Vendored verification is split as well. vendor/verify.rs and vendor/pypi.rs use eq_ignore_ascii_case, while vendor/redownload.rs, bun_workspace.rs and bun_binary.rs use ==/!=.

The manifest loader doesn't normalize beforeHash/afterHash, so an uppercase hash travels unchanged to every one of these sites.

Proof by execution (temporary core integration test, run twice on 045d7ec, not committed):

  • Setup: a package file whose content matches beforeHash, and the before and after blobs present in blobs/ under the manifest's spelling.
  • Run 1 used the hashes as computed, and run 2 the same hashes uppercased:
lower: is_valid_blob_hash=true verify=Ready apply.success=true apply.err=None rollback_verify=Ready
UPPER: is_valid_blob_hash=true verify=HashMismatch apply.success=false apply.err=Some("Cannot apply patch: package/index.js - File hash does not match expected value") rollback_verify=HashMismatch

So the same patch, with the same bytes on disk and its blobs present, passes download and the blob-name check, then fails both apply and rollback verification.

Symptoms and impact

  • No open issue reports this yet.
  • The API emits lowercase, so only hand-edited or third-party manifests hit it today.
  • But the code explicitly supports uppercase in one place and breaks it in another. Each new comparison site picks a rule ad hoc: the vendored side already has both.

Proposed change

Pick one rule and enforce it at one boundary. Recommended: normalize at the boundary. Lowercase beforeHash/afterHash when the manifest is deserialized, or reject non-lowercase hex there. Then:

  • delete blob_hash_matches in favor of plain ==;
  • give digest one hex_eq used by the vendored sites, or normalize their pins at parse time the same way.

The alternative is a shared digest::hex_eq(a, b) used by every comparison above. That's more call sites, but no load-time change.

Size and scope

About 60 production lines across manifest/schema.rs (or the loader), blob_fetcher.rs, apply.rs, rollback.rs and the five vendored files listed. Out of scope: hosted-mode SRI/checksum pins, which have their own documented case policy in utils::digest (is_hex64_lower for Cargo).

Acceptance criteria

  • A regression test with an uppercase beforeHash/afterHash manifest: apply patches and rollback restores, or both refuse at load with a clear error (whichever rule is chosen, applied consistently).
  • No ==/!= between a computed digest and a manifest or pin hash remains outside the chosen helper or boundary. A grep or architecture test guards it.
  • test_blob_hash_matches_is_case_insensitive is updated or removed to match the chosen rule.
  • The existing apply, rollback and vendored verify tests stay green.

Dependencies

Easier after #706 (one utils::digest), but not blocked by it.

Activity

  1. mikolalysenko commented on Oct 3, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Triage: priority:p3 (cross-cutting core, no single ecosystem). Not a duplicate; no open or merged PR touches the case-sensitive == sites in apply.rs/rollback.rs on main @ 045d7ec. Related to #706 (digest helpers), but it's a separate defect: #706 makes no behavior change, and this issue is about the comparison rule. So they stay separate work items, and #707 is best fixed after #706.


    Generated by Claude Code

  2. mikolalysenko commented on Oct 5, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Re-checked on main @ 0d302dc (architecture audit, CLI and core): still present. The comparison sites are unchanged, but blob_fetcher.rs moved in #607, so here are fresh permalinks.

    The proposed change and acceptance criteria stand.


    Generated by Claude Code

  3. mikolalysenko commented on Oct 8, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Re-checked on main @ b762f41. The split still holds and the code has moved, so here are fresh permalinks.

    Agent-mode verify still compares exactly:

    The blob fetcher and vendored verify ignore case:


    Generated by Claude Code

  4. mikolalysenko commented on Oct 8, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Claiming this issue for the architecture refactor routine (highest leverage: the one eligible item whose fix fits the free boundary file manifest/schema.rs, so one place sets the hash case policy for apply, rollback, vendored and blob sites). Branch: arch-refactor/707-manifest-hash-case. Claim-ID: 2026-10-08T19:55:50Z-2a3f77

    Plan: lowercase beforeHash/afterHash when a manifest is deserialized (the "normalize at the boundary" option), plus a core regression test where an uppercase-hash manifest applies and rolls back. blob_hash_matches in blob_fetcher.rs stays for now because open PRs #1049 and #1041 change that file.


    Generated by Claude Code

  5. added
    arch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)
    and removed
    arch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)
    on Oct 8, 2026
  6. mikolalysenko commented on Oct 8, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Draft PR: #1163.


    Generated by Claude Code

  7. added 2 commits that reference this issue on Oct 8, 2026
    2388907
    c2b9a9b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent:claimedagent:triagedarch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)bugSomething isn't workingpriority:p3

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions