Skip to content

fix(release): read checksum manifests the way coreutils writes them - #1330

Merged
justinjoy merged 1 commit into
mainfrom
fix/1315-manifest-unescape
Sep 4, 2026
Merged

fix(release): read checksum manifests the way coreutils writes them#1330
justinjoy merged 1 commit into
mainfrom
fix/1315-manifest-unescape

Conversation

@justinjoy

Copy link
Copy Markdown
Collaborator

Closes #1315. Stacked on #1314 (fix/1311-checksum-escaping) — review that first.

The bug

verify-release.sh rejected any archive whose name coreutils escapes, before comparing a
single hash:

checksum manifest does not name back\slash.tar.gz: back\\slash.tar.gz

GNU coreutils escapes exactly three characters in a name — \\\, newline → \n,
carriage return → \r — and prefixes the whole line with a literal \.

The escape set came from scanning all 254 legal filename bytes through sha256sum, not
from documentation.
My first version had only two: I'd derived the set from the two cases
the issue happened to name, and missed CR. The reviewer caught it by running the scan I
should have run — an incomplete set treated as complete, which is the same defect as the bug
being fixed, reproduced inside the fix. Tab, other control bytes and UTF-8 are not escaped;
b3sum escapes the same three with the same marker.

A third failure had nothing to do with escaping. The name was read as awk '$2', and
coreutils does not escape spaces, so has space.tar.gz was truncated to has. The name is
now taken as everything after the two-character separator, which is also correct for binary
mode (hash *name).

Why escaping the expected name, not unescaping the manifest's

The issue suggested unescaping. That direction is ambiguous, and I only found out by
measuring it: a file genuinely named a\nb.tar.gz is written a\\nb.tar.gz, and replacing
\\ then \n in sequence corrupts it to a<newline>b.tar.gz. Escaping is one-way and
has no such case — which is why a\nb.tar.gz is one of the regression cases rather than an
incidental one.

Fail-closed, verified

The name check only selects which manifest line is trusted; the hash is recomputed
independently. Reviewer confirmed no input produces a false "verified" — corrupt archive with
an escaped name, a forged name field carrying another file's hash, multi-line manifests with a
wrong first line, empty and hash-only manifests are all rejected. It also checked that
escaping stays injective across all three rules, so a manifest for an LF-named file is
correctly rejected against a CR-named archive.

Tests

Five cases in scripts/ci/test-release-roundtrip.sh, built from real sha256sum/b3sum
output — a hand-written unescaped manifest would pin the parsing without exercising the
escaping condition at all.

variant suite result
this PR 0 FAIL
minus the \r rule 1 FAIL — carriage return only
minus the \n rule 1 FAIL — newline only
minus the \\ rule 2 FAIL — backslash, literal \n
backslash rule moved last 2 FAIL — newline, carriage return
unfixed verify-release.sh 5 FAIL — all five

Each rule is independently pinned, and the ordering the comment asserts is executable
rather than prose. Verified on real bash 3.2.57 (the macOS floor) as well as 5.3, with no
temp-directory leftovers.

This also makes #1311's leading-backslash strip in hash_of reachable for the first time and
pins it, so the comment declaring it deliberately unpinned is replaced.

Left out deliberately

Local: 306 Ok / 0 Fail / 12 Skipped, serialized, dedicated build dir.

verify-release.sh rejected any archive whose name coreutils escapes, before
comparing a single hash:

    checksum manifest does not name back\slash.tar.gz: back\\slash.tar.gz

GNU coreutils escapes exactly three characters in a name -- backslash as `\\`,
newline as `\n`, carriage return as `\r` -- and prefixes the whole line with a
literal `\`. The set was determined by scanning all 254 legal filename bytes
through sha256sum, not from documentation: tab, other control bytes and UTF-8
are not escaped. b3sum escapes the same three with the same marker.

A third failure had nothing to do with escaping. The name was read as awk's
`$2`, and coreutils does not escape spaces, so `has space.tar.gz` was truncated
to `has` and reported as a name mismatch. The name is now taken as everything
after the two-character separator, which is also correct for binary mode.

Compare against the ESCAPED form of the expected name rather than unescaping the
manifest's. Unescaping is ambiguous in the wrong direction: a file genuinely
named `a\nb.tar.gz` is written `a\\nb.tar.gz`, and replacing `\\` then `\n` in
sequence turns it into a newline. Measured -- the naive form corrupts that name.
Escaping is one-way and has no such case, which is why `a\nb.tar.gz` is a
regression case rather than an incidental one.

Every path still fails closed. The name check only selects which manifest line
is trusted; the hash is recomputed independently, so no name-check outcome can
produce a false "verified".

This makes #1311's leading-backslash strip in hash_of reachable for the first
time and pins it, so the comment declaring it deliberately unpinned is replaced.

Refs #1315
@justinjoy
justinjoy force-pushed the fix/1315-manifest-unescape branch from 329ce3e to 72efd1b Compare September 3, 2026 14:29
@justinjoy
justinjoy merged commit f066746 into main Sep 4, 2026
26 checks passed
@justinjoy
justinjoy deleted the fix/1315-manifest-unescape branch September 4, 2026 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

verify-release.sh: manifest reading does not unescape, so an escaped archive name is rejected as a name mismatch

1 participant