Repository navigation
Agent-mode apply and rollback reject a manifest hash in uppercase hex that blob download accepts as valid #707
Description
Activity
- addedbugSomething isn't workingSomething isn't workingarch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)Filed by a scheduled architecture audit routine (see the architecture review discussion)
on Oct 3, 2026 mikolalysenko commented
on Oct 3, 2026 CollaboratorAuthorMore actions[agent] Triage:
priority:p3(cross-cutting core, no single ecosystem). Not a duplicate; no open or merged PR touches the case-sensitive==sites inapply.rs/rollback.rsonmain@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
mikolalysenko commented
on Oct 5, 2026 CollaboratorAuthorMore actions[agent] Re-checked on main @
0d302dc(architecture audit, CLI and core): still present. The comparison sites are unchanged, butblob_fetcher.rsmoved in #607, so here are fresh permalinks.- Download still compares case-insensitively:
blob_hash_matchesblob_fetcher.rs#L522-L524. - Agent mode still compares with exact
==/!=: apply at L267 and L1063, and rollback at L145, L198 and L293.
The proposed change and acceptance criteria stand.
Generated by Claude Code
- Download still compares case-insensitively:
mikolalysenko commented
on Oct 8, 2026 CollaboratorAuthorMore actions[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:
- apply:
apply.rs#L267andapply.rs#L1194 - rollback:
rollback.rs#L160androllback.rs#L213
The blob fetcher and vendored verify ignore case:
Generated by Claude Code
- apply:
mikolalysenko commented
on Oct 8, 2026 CollaboratorAuthorMore actions[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-2a3f77Plan: lowercase
beforeHash/afterHashwhen 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_matchesinblob_fetcher.rsstays for now because open PRs #1049 and #1041 change that file.
Generated by Claude Code
- addedarch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)Filed by a scheduled architecture audit routine (see the architecture review discussion)and removedarch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)Filed by a scheduled architecture audit routine (see the architecture review discussion)
on Oct 8, 2026 mikolalysenko commented
on Oct 8, 2026 CollaboratorAuthorMore actions- added 2 commits that reference this issue
on Oct 8, 2026
[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_fetcher::blob_hash_matchesuseseq_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_hashandclient::is_valid_sha256_hexboth accept uppercase as well.compute_git_sha256_from_bytesemits lowercase, and it is compared with==/!=:verify_file_patch, and the patched-bytes check;vendor/verify.rsandvendor/pypi.rsuseeq_ignore_ascii_case, whilevendor/redownload.rs,bun_workspace.rsandbun_binary.rsuse==/!=.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):beforeHash, and the before and after blobs present inblobs/under the manifest's spelling.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
Proposed change
Pick one rule and enforce it at one boundary. Recommended: normalize at the boundary. Lowercase
beforeHash/afterHashwhen the manifest is deserialized, or reject non-lowercase hex there. Then:blob_hash_matchesin favor of plain==;digestonehex_eqused 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.rsand the five vendored files listed. Out of scope: hosted-mode SRI/checksum pins, which have their own documented case policy inutils::digest(is_hex64_lowerfor Cargo).Acceptance criteria
beforeHash/afterHashmanifest:applypatches androllbackrestores, or both refuse at load with a clear error (whichever rule is chosen, applied consistently).==/!=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_insensitiveis updated or removed to match the chosen rule.apply,rollbackand vendored verify tests stay green.Dependencies
Easier after #706 (one
utils::digest), but not blocked by it.