Keel-Linux/handbook

Fork 0

Decision note 0016: mirror layout, channels and rollback #10

Merged

marcos-mendez merged 4 commits from docs/decision-0016-mirror-layout into main

2026-09-29 00:56:47 -03:00

marcos-mendez commented

2026-09-28 02:17:44 -03:00

(Migrated from github.com)

Closes #7.

Decision note 0016, and only 0016: 0014 and 0015 are being written
concurrently, so the number was taken deliberately rather than as "the next
free one".

What it records

Blobs content addressed and never overwritten or deleted; releases immutable
and identified by release and revision; channels the only mutable object,
signed and carrying a timestamp and an expiry; rollback as moving a pointer;
an expired channel an error at the client.

The two pieces of reasoning the issue asked for are written out rather than
left in a commit message:

Why a release alone is not enough. Two publications of one release
overwrite each other, and with dated releases a rebuild for a security
update on the same day is exactly that case.
Why a signed pointer with an expiry is not optional. A stale or hostile
mirror freezes a client on a vulnerable release simply by not updating, and
without a timestamp inside the signature the client cannot tell that apart
from "nothing new".

Two things the note adds beyond the issue

Which key signs a channel, and what that key may say. The channel key is
the unattended one on the build host, today the staging key of decision 0011,
because re-signing before every expiry is daily and a design that needs the
maintainer at a terminal daily gets bypassed — by lengthening the expiry until
it means nothing. The note states the boundary plainly rather than implying a
guarantee we do not have: an online key closes the mirror, which is a
different machine holding no key, and does not close a compromised build host.
It also says explicitly that this extends 0011 rather than contradicting its
"no appliance may use the staging key" wording, because signing a pointer is a
strictly weaker claim than signing a release.

main. It stays the trunk and is always the latest; release branches are
cut only when the next Debian opens, and the forks keep master so they stay
mergeable upstream.

What the review added to the note

The note's argument stood; four things it did not argue are now in it, each
one found by the review attacking the design rather than the prose.

A ceiling on "until when". Unbounded, the narrow claim made for the
online key is untrue: one signature pins an appliance to a known
vulnerable revision for ever and the rollback refusal never fires, which
is the exact failure mode the note offers as the reason a person cannot be
in the loop daily. Thirty days, enforced by the client.
How the key is retired. The obvious answer — revoke and ship the
revocation — did not work, because both implementations accepted a revoked
key. And a thief can delay the keyring that retires them by freezing
appliances, which the ceiling bounds. The keyring travels through the
archive, not through the channel it protects.
Clock skew, in both directions, and what the client owes in return: a
message that does not misdiagnose a fleet-wide NTP failure as a stale
mirror.
The refresh has no delivery path. It writes on the build host and
nothing ships it, so the pointer clients read expires seven days after the
last publication, not the last refresh. Answered as the open question it
is, with the interim rule, and with a note that this must not become an
argument for a longer ceiling.

Also: the "one file, not a file beside a detached signature" rationale moved
into the layout section where it belongs, with the consequence that makes it
matter — clear signing dash-escapes, so the bytes on the mirror are not the
bytes signed, which is why a reader must take the body from the verifier and
never parse the file. And the coverage figure is gone: a note is not the
place for a number it cannot qualify, and that number was compatible with the
verifier never running.

Also here: the traps

Both in docs/traps.md, because each would have shipped a green check over
code that does not work.

The gpgv entry was wrong in the way this file exists to prevent, and
that is now the most useful thing in it. It named a function that did not
implement the fix it described, and its stated rule — exit status plus
VALIDSIG — is not sufficient: a revoked or expired key gives exit 0 and
VALIDSIG, withholding only GOODSIG. The entry now names the function
that does implement it, carries the measured table, records that
VALIDSIG's last field is the primary so pinning a primary accepts a
revoked subkey of it, and adds the relative---keyring case that produces
the same NO_PUBKEY as a wrong key. An entry recording a fix that is not
there costs more than a missing entry, so the entry says so.
The field-count trap already in the file was hit again in the same
work, in apt/lib/mirror.sh, so that entry now draws the lesson it did
not: if a format's last field can contain the separator, counting fields
is the bug.
In one local declaration bash expands every word before assigning any
of them, so local a="$1" b="$dir/$a" gives b the value a had outside
the function. It hit twice in the publisher, and the second time the
generated script died at an unbound name under set -u before printing the
refusal it existed to print. The adversarial test found it; the happy path
passed.

Client: Keel-Linux/keel#34 (landed first, on purpose)
Publisher: Keel-Linux/apt#14

Closes #7. Decision note **0016**, and only 0016: 0014 and 0015 are being written concurrently, so the number was taken deliberately rather than as "the next free one". ## What it records Blobs content addressed and never overwritten or deleted; releases immutable and identified by release *and revision*; channels the only mutable object, signed and carrying a timestamp and an expiry; rollback as moving a pointer; an expired channel an error at the client. The two pieces of reasoning the issue asked for are written out rather than left in a commit message: - **Why a release alone is not enough.** Two publications of one release overwrite each other, and with dated releases a rebuild for a security update on the same day is exactly that case. - **Why a signed pointer with an expiry is not optional.** A stale or hostile mirror freezes a client on a vulnerable release simply by not updating, and without a timestamp inside the signature the client cannot tell that apart from "nothing new". ## Two things the note adds beyond the issue **Which key signs a channel, and what that key may say.** The channel key is the unattended one on the build host, today the staging key of decision 0011, because re-signing before every expiry is daily and a design that needs the maintainer at a terminal daily gets bypassed — by lengthening the expiry until it means nothing. The note states the boundary plainly rather than implying a guarantee we do not have: an online key closes the **mirror**, which is a different machine holding no key, and does not close a compromised build host. It also says explicitly that this extends 0011 rather than contradicting its "no appliance may use the staging key" wording, because signing a pointer is a strictly weaker claim than signing a release. **`main`.** It stays the trunk and is always the latest; release branches are cut only when the next Debian opens, and the forks keep `master` so they stay mergeable upstream. ## What the review added to the note The note's argument stood; four things it did not argue are now in it, each one found by the review attacking the design rather than the prose. - **A ceiling on "until when".** Unbounded, the narrow claim made for the online key is untrue: one signature pins an appliance to a known vulnerable revision for ever and the rollback refusal never fires, which is the exact failure mode the note offers as the reason a person cannot be in the loop daily. Thirty days, enforced by the client. - **How the key is retired.** The obvious answer — revoke and ship the revocation — did not work, because both implementations accepted a revoked key. And a thief can delay the keyring that retires them by freezing appliances, which the ceiling bounds. The keyring travels through the archive, not through the channel it protects. - **Clock skew**, in both directions, and what the client owes in return: a message that does not misdiagnose a fleet-wide NTP failure as a stale mirror. - **The refresh has no delivery path.** It writes on the build host and nothing ships it, so the pointer clients read expires seven days after the last *publication*, not the last refresh. Answered as the open question it is, with the interim rule, and with a note that this must not become an argument for a longer ceiling. Also: the "one file, not a file beside a detached signature" rationale moved into the layout section where it belongs, with the consequence that makes it matter — clear signing dash-escapes, so the bytes on the mirror are not the bytes signed, which is why a reader must take the body from the verifier and never parse the file. And the coverage figure is gone: a note is not the place for a number it cannot qualify, and that number was compatible with the verifier never running. ## Also here: the traps Both in `docs/traps.md`, because each would have shipped a green check over code that does not work. - **The gpgv entry was wrong in the way this file exists to prevent**, and that is now the most useful thing in it. It named a function that did not implement the fix it described, and its stated rule — exit status plus `VALIDSIG` — is not sufficient: a revoked or expired key gives exit 0 and `VALIDSIG`, withholding only `GOODSIG`. The entry now names the function that does implement it, carries the measured table, records that `VALIDSIG`'s last field is the primary so pinning a primary accepts a revoked subkey of it, and adds the relative-`--keyring` case that produces the same `NO_PUBKEY` as a wrong key. An entry recording a fix that is not there costs more than a missing entry, so the entry says so. - **The field-count trap already in the file was hit again** in the same work, in `apt/lib/mirror.sh`, so that entry now draws the lesson it did not: if a format's last field can contain the separator, counting fields is the bug. - **In one `local` declaration bash expands every word before assigning any of them**, so `local a="$1" b="$dir/$a"` gives `b` the value `a` had outside the function. It hit twice in the publisher, and the second time the generated script died at an unbound name under `set -u` before printing the refusal it existed to print. The adversarial test found it; the happy path passed. ## Related - Client: Keel-Linux/keel#34 (landed first, on purpose) - Publisher: Keel-Linux/apt#14

marcos-mendez commented

2026-09-28 04:58:01 -03:00

(Migrated from github.com)

Reviewed alongside Keel-Linux/keel#34 and Keel-Linux/apt#14. Measured means run here on gpgv 2.4.7 / Debian 13.7 against those two branches.

On the question of whether the note argues the design or merely describes it: it argues. "Why the revision, and not the release alone" names the case a date cannot distinguish. "Why a signed pointer with an expiry is not optional" states the freeze attack and why a warning would not do. "Which key signs a channel" does the hardest job in the note — it reaches the uncomfortable conclusion, gives the operational reason (a daily passphrase gets bypassed by lengthening the expiry), states the boundary rather than implying a guarantee, and then reconciles itself with 0011 instead of quietly contradicting it. "What this does not decide" is three real open questions and not a formality. That is a decision note.

The findings below are about what it does not argue, and about docs/traps.md.

HIGH — `docs/traps.md`: the gpgv entry records a fix that is not in the code it names

The Fix paragraph says the output is read "only after the exit status and a [GNUPG:] VALIDSIG <fpr> line naming an accepted key have both been accepted", and names two implementations: keel.layers.signature.verify_bytes, and channel_verified in apt/lib/channel.sh.

channel_verified is apt/lib/channel.sh:165-177. It runs

if gpgv --keyring "$ring" --output "$out" "$file" 2>/dev/null; then

— exit status, and nothing else. No --status-fd, no VALIDSIG, no fingerprint. The trap file states as done a thing the code it cites does not do. apt/lib/release.sh:243-245 is the function that actually implements the described fix, and it is not the one named.

That is the failure mode this file exists to prevent, in this file. It also means a reader who trusts the entry will not look at channel_verified again.

HIGH — the gpgv entry's fix is not sufficient anywhere, including where it is implemented

"Exit status and a VALIDSIG line" does not establish that a signature is good. Measured, gpgv 2.4.7:

signature by	exit	status lines
good key	0	`GOODSIG`, `VALIDSIG`
revoked primary	0	`REVKEYSIG`, `VALIDSIG`
revoked signing subkey, live primary	0	`REVKEYSIG`, `VALIDSIG` — and `VALIDSIG`'s last field is the primary fingerprint
expired key	0	`KEYEXPIRED`, `EXPKEYSIG`, `VALIDSIG`

stderr says Good signature from in all four. End to end on keel#34: a pointer signed by a key that is revoked in the very keyring passed to --channel-keyring, with --channel-signer pinned to its fingerprint, gives exit 0 and fetches the layer. Same for an expired key.

So the entry records two properties of gpgv that shaped the design, and omits the two that break it. Given the standard this file sets — "each would have shipped a green check over code that does not work" — these belong in it more than the two that are there. The accurate rule is "require a GOODSIG line", because gpgv emits GOODSIG only when the key is neither revoked nor expired and substitutes REVKEYSIG/EXPKEYSIG otherwise.

This is not academic for this project: apt/keys/keel-archive-keyring.asc already carries sub D276B62C2BD16F4E … r, a signing subkey that has already been revoked once and rotated (keel-wordpress/COVERAGE.md:134-163). A third gpgv fact worth the same paragraph: --keyring with a relative path is resolved against $GNUPGHOME, not the working directory, and a keyring it cannot find produces NO_PUBKEY — the same message as a wrong key, and the same message as the armored-keyring case the entry already records.

MEDIUM — "and until when. That is all" understates the online key

Lines 99-102 are the narrow claim the design rests on, and line 116-118 concedes the weaker half: "whoever holds the online key can keep re-signing a stale pointer." Two things are missing from that concession.

First, they need not keep doing it. Nothing bounds expires_at at either end. Measured on keel#34: a pointer signed now with expires_at 2126-01-01T00:00:00Z is accepted, exit 0. On apt#14, KEEL_CHANNEL_TTL_DAYS=100000000 produces expires_at 275817-06-11 and exits 0 silently. So one signature from the online key pins an appliance to a known-vulnerable, already-staged revision indefinitely, and CHANNEL_ROLLBACK never fires because the revision does not go backwards.

Second, that is precisely the failure mode the note itself names two paragraphs earlier — "by lengthening the expiry until it means nothing, which is worse than not having one" — offered as the reason a person must not be in the loop daily. The design identifies the lever, removes the human who would pull it, and then leaves the lever unguarded and unlogged for the unattended key. A client-side ceiling on expires_at - signed_at costs two comparisons and turns "forever" into "N days".

While there: nothing refuses a signed_at in the future either (measured: signed_at 2076-01-01 accepted).

MEDIUM — the note does not say how the channel key is retired

It puts a key online and argues well for why. It does not say what happens when that key is stolen, which for an unattended key on a build host is the event to plan for. The answer the reader will assume — revoke it, ship the revocation in the keyring — does not work, per the HIGH above. And the project has already done exactly this manoeuvre once, for the archive signing subkey.

A decision note that moves a key online owes a paragraph on retirement: revoke, rotate, and how appliances learn — including the awkward part, that the stolen key can delay the keyring that would retire it by freezing appliances on an old revision.

MEDIUM — nothing on clock skew

The expiry is checked client-side as now >= expires_at against the appliance's own clock, and the note does not mention the clock once. Both directions matter and neither is obvious:

clock ahead: every pointer expires, exit 19 on every channel pull, and the message says "the mirror is not being updated, or is holding this instance back" — it never prints now and never mentions the local clock. The appliance is stalled, not bricked (--release/--rev and the flat layout still work), but a fleet-wide NTP failure sends the operator to the mirror.
clock behind: dead pointers still look fresh and the freeze attack the expiry exists to stop succeeds. An LXC container restored from a snapshot is exactly that case.

One paragraph: the expiry is only as good as the clock, here is what each direction looks like, and here is why we accept it.

MEDIUM — "seven days, refreshed daily" describes an operational property the implementation does not have

Lines 120-128. On apt#14, keel-channel refresh writes $KEEL_RELEASE_ROOT/channels on the build host; the only thing that installs a pointer on the public host is keel-publish-mirror, which requires a mandatory release date and an operator-opened tunnel. There is no timer, unit or script that ships a refreshed pointer. So the pointer clients read expires seven days after the last manual full release publication, and "six consecutive failures of the refresh are survivable and a seventh is not" measures a refresh no client ever sees. Either the note should say publication is manual, or apt#14 should grow a delivery path — I have raised it there as the finding that needs a design answer rather than a patch.

LOW

The "one clear-signed file, not a file beside a detached signature" argument — two files are two mutable objects a mirror could skew — lives in keel/docs/layers.md and not in the note. It is the right call and the reason given is right (and it is the same reason apt made InRelease), but it is a decision about the mirror layout and this is the mirror-layout note. Worth a sentence beside "The layout", with the consequence: the bytes on the mirror are not the bytes signed, because clearsign dash-escapes and does not sign trailing whitespace, which is why the client must read gpgv's --output and never the file. Both implementations get this right; neither the note nor the traps entry says why it matters.
Line 36-37, "The flat names stay, as hard links to the blobs, until their removal is its own change." On apt#14 that hard link is ln -f over an unvalidated tarball field and is one of two ways a published blob can currently be overwritten. Not the note's fault, but it is the note's sentence that makes "blobs are immutable" and "flat names are hard links" sound compatible, and today they are not.
Line 176-177 quotes "971 tests, 100 percent line and branch coverage". Reproduced exactly, and it is real (Miss=0, BrPart=0). It is also, on that branch, compatible with the CLI plumbing of --channel-signer being deleted on both paths with no test noticing, and with the whole gate passing green at 97% when gpgv is absent. A decision note is not the place for a coverage figure it cannot qualify; either drop it or say what it does and does not establish.
docs/traps.md already carries "A path with a space in it breaks a whitespace comparison", whose fix is to key on a tab and to count paths with an internal space before trusting a count. apt/lib/mirror.sh:133 is awk 'NF == 3 …' over MANIFEST lines, which silently drops any path containing a space from the digest check while the file is still fetched and installed. Pre-existing, but it is the same trap, and the file it is recorded in is the file this PR is editing.

What is right and worth keeping as written

Line 112-118, naming the threat the expiry closes as the mirror, a different machine holding no key, and stating plainly that it does not close a compromised build host. Refusing to imply a guarantee you do not have is the hardest thing to do in a note like this and it is done well.
Line 104-110, reconciling with 0011 explicitly rather than letting a future reader find the contradiction. "Signing a channel is a strictly weaker claim than signing a release: it selects among artefacts a person already stood behind" is the sentence the whole decision turns on, and it is a good one — my only argument with it is that "and until when" is not weak while it is unbounded.
Line 130-139, the ordering argument. It is right, and the client does read both layouts: a flat pull on keel#34 behaves exactly as before.
Line 149-157, and particularly "When a blob may be deleted. Never, by this note" with the reachability criterion attached. That is the right shape for a deferred decision.
The local trap entry is accurate, correctly diagnosed, gives the two-statement fix, and says which test found it and that the happy path passed. I swept every shell file in every Keel repository and worktree with a quote-aware scanner validated against the known-bad shapes: zero remaining occurrences. The entry did its job.

Warning. The note itself is sound and should land close to as written; the two HIGHs are both in docs/traps.md. A traps file that names the wrong implementation, and whose stated fix does not hold, costs more than a missing entry — the next reader stops checking. Fix the gpgv entry to name apt/lib/release.sh:243-245, to say that channel_verified still needs the same treatment, and to record that exit 0 plus VALIDSIG is produced for revoked and expired keys, so the rule is GOODSIG. Then the MEDIUMs: a retirement paragraph, a TTL ceiling, a clock-skew paragraph, and either a caveat on "refreshed daily" or a delivery path in apt#14.

Reviewed alongside Keel-Linux/keel#34 and Keel-Linux/apt#14. **Measured** means run here on gpgv 2.4.7 / Debian 13.7 against those two branches. **On the question of whether the note argues the design or merely describes it: it argues.** "Why the revision, and not the release alone" names the case a date cannot distinguish. "Why a signed pointer with an expiry is not optional" states the freeze attack and why a warning would not do. "Which key signs a channel" does the hardest job in the note — it reaches the uncomfortable conclusion, gives the operational reason (a daily passphrase gets bypassed by lengthening the expiry), states the boundary rather than implying a guarantee, and then reconciles itself with 0011 instead of quietly contradicting it. "What this does not decide" is three real open questions and not a formality. That is a decision note. The findings below are about what it does not argue, and about `docs/traps.md`. --- ## HIGH — `docs/traps.md`: the gpgv entry records a fix that is not in the code it names The **Fix** paragraph says the output is read *"only after the exit status **and** a `[GNUPG:] VALIDSIG <fpr>` line naming an accepted key have both been accepted"*, and names two implementations: `keel.layers.signature.verify_bytes`, and **`channel_verified` in `apt/lib/channel.sh`**. `channel_verified` is `apt/lib/channel.sh:165-177`. It runs ```sh if gpgv --keyring "$ring" --output "$out" "$file" 2>/dev/null; then ``` — exit status, and nothing else. No `--status-fd`, no `VALIDSIG`, no fingerprint. The trap file states as done a thing the code it cites does not do. `apt/lib/release.sh:243-245` is the function that actually implements the described fix, and it is not the one named. That is the failure mode this file exists to prevent, in this file. It also means a reader who trusts the entry will not look at `channel_verified` again. ## HIGH — the gpgv entry's fix is not sufficient anywhere, including where it *is* implemented "Exit status **and** a VALIDSIG line" does not establish that a signature is good. **Measured**, gpgv 2.4.7: | signature by | exit | status lines | | --- | --- | --- | | good key | 0 | `GOODSIG`, `VALIDSIG` | | **revoked primary** | **0** | `REVKEYSIG`, `VALIDSIG` | | **revoked signing subkey**, live primary | **0** | `REVKEYSIG`, `VALIDSIG` — and `VALIDSIG`'s last field is the *primary* fingerprint | | **expired key** | **0** | `KEYEXPIRED`, `EXPKEYSIG`, `VALIDSIG` | stderr says `Good signature from` in all four. End to end on keel#34: a pointer signed by a key that is revoked in the very keyring passed to `--channel-keyring`, with `--channel-signer` pinned to its fingerprint, gives exit 0 and fetches the layer. Same for an expired key. So the entry records two properties of gpgv that shaped the design, and omits the two that break it. Given the standard this file sets — *"each would have shipped a green check over code that does not work"* — these belong in it more than the two that are there. The accurate rule is "require a `GOODSIG` line", because gpgv emits `GOODSIG` only when the key is neither revoked nor expired and substitutes `REVKEYSIG`/`EXPKEYSIG` otherwise. This is not academic for this project: `apt/keys/keel-archive-keyring.asc` already carries `sub D276B62C2BD16F4E … r`, a signing subkey that has already been revoked once and rotated (`keel-wordpress/COVERAGE.md:134-163`). A third gpgv fact worth the same paragraph: `--keyring` with a relative path is resolved against `$GNUPGHOME`, not the working directory, and a keyring it cannot find produces `NO_PUBKEY` — the same message as a wrong key, and the same message as the armored-keyring case the entry already records. --- ## MEDIUM — "and until when. That is all" understates the online key Lines 99-102 are the narrow claim the design rests on, and line 116-118 concedes the weaker half: *"whoever holds the online key can keep re-signing a stale pointer."* Two things are missing from that concession. First, they need not keep doing it. Nothing bounds `expires_at` at either end. **Measured** on keel#34: a pointer signed now with `expires_at 2126-01-01T00:00:00Z` is accepted, exit 0. On apt#14, `KEEL_CHANNEL_TTL_DAYS=100000000` produces `expires_at 275817-06-11` and exits 0 silently. So one signature from the online key pins an appliance to a known-vulnerable, already-staged revision indefinitely, and `CHANNEL_ROLLBACK` never fires because the revision does not go backwards. Second, that is precisely the failure mode the note itself names two paragraphs earlier — *"by lengthening the expiry until it means nothing, which is worse than not having one"* — offered as the reason a person must not be in the loop daily. The design identifies the lever, removes the human who would pull it, and then leaves the lever unguarded and unlogged for the unattended key. A client-side ceiling on `expires_at - signed_at` costs two comparisons and turns "forever" into "N days". While there: nothing refuses a `signed_at` in the future either (**measured**: `signed_at 2076-01-01` accepted). ## MEDIUM — the note does not say how the channel key is retired It puts a key online and argues well for why. It does not say what happens when that key is stolen, which for an unattended key on a build host is the event to plan for. The answer the reader will assume — revoke it, ship the revocation in the keyring — **does not work**, per the HIGH above. And the project has already done exactly this manoeuvre once, for the archive signing subkey. A decision note that moves a key online owes a paragraph on retirement: revoke, rotate, and how appliances learn — including the awkward part, that the stolen key can delay the keyring that would retire it by freezing appliances on an old revision. ## MEDIUM — nothing on clock skew The expiry is checked client-side as `now >= expires_at` against the appliance's own clock, and the note does not mention the clock once. Both directions matter and neither is obvious: - clock ahead: every pointer expires, exit 19 on every channel pull, and the message says *"the mirror is not being updated, or is holding this instance back"* — it never prints `now` and never mentions the local clock. The appliance is stalled, not bricked (`--release/--rev` and the flat layout still work), but a fleet-wide NTP failure sends the operator to the mirror. - clock behind: dead pointers still look fresh and the freeze attack the expiry exists to stop succeeds. An LXC container restored from a snapshot is exactly that case. One paragraph: the expiry is only as good as the clock, here is what each direction looks like, and here is why we accept it. ## MEDIUM — "seven days, refreshed daily" describes an operational property the implementation does not have Lines 120-128. On apt#14, `keel-channel refresh` writes `$KEEL_RELEASE_ROOT/channels` **on the build host**; the only thing that installs a pointer on the public host is `keel-publish-mirror`, which requires a mandatory release date and an operator-opened tunnel. There is no timer, unit or script that ships a refreshed pointer. So the pointer clients read expires seven days after the last *manual full release publication*, and "six consecutive failures of the refresh are survivable and a seventh is not" measures a refresh no client ever sees. Either the note should say publication is manual, or apt#14 should grow a delivery path — I have raised it there as the finding that needs a design answer rather than a patch. --- ## LOW - The "one clear-signed file, not a file beside a detached signature" argument — *two files are two mutable objects a mirror could skew* — lives in `keel/docs/layers.md` and not in the note. It is the right call and the reason given is right (and it is the same reason apt made `InRelease`), but it is a decision about the mirror layout and this is the mirror-layout note. Worth a sentence beside "The layout", with the consequence: the bytes on the mirror are not the bytes signed, because clearsign dash-escapes and does not sign trailing whitespace, which is why the client must read gpgv's `--output` and never the file. Both implementations get this right; neither the note nor the traps entry says why it matters. - Line 36-37, *"The flat names stay, as hard links to the blobs, until their removal is its own change."* On apt#14 that hard link is `ln -f` over an unvalidated `tarball` field and is one of two ways a published blob can currently be overwritten. Not the note's fault, but it is the note's sentence that makes "blobs are immutable" and "flat names are hard links" sound compatible, and today they are not. - Line 176-177 quotes "971 tests, 100 percent line and branch coverage". Reproduced exactly, and it is real (`Miss=0`, `BrPart=0`). It is also, on that branch, compatible with the CLI plumbing of `--channel-signer` being deleted on both paths with no test noticing, and with the whole gate passing green at 97% when gpgv is absent. A decision note is not the place for a coverage figure it cannot qualify; either drop it or say what it does and does not establish. - `docs/traps.md` already carries *"A path with a space in it breaks a whitespace comparison"*, whose fix is to key on a tab and to count paths with an internal space before trusting a count. `apt/lib/mirror.sh:133` is `awk 'NF == 3 …'` over MANIFEST lines, which silently drops any path containing a space from the digest check while the file is still fetched and installed. Pre-existing, but it is the same trap, and the file it is recorded in is the file this PR is editing. --- ## What is right and worth keeping as written - Line 112-118, naming the threat the expiry closes as *the mirror, a different machine holding no key*, and stating plainly that it does not close a compromised build host. Refusing to imply a guarantee you do not have is the hardest thing to do in a note like this and it is done well. - Line 104-110, reconciling with 0011 explicitly rather than letting a future reader find the contradiction. "Signing a channel is a strictly weaker claim than signing a release: it selects among artefacts a person already stood behind" is the sentence the whole decision turns on, and it is a good one — my only argument with it is that "and until when" is not weak while it is unbounded. - Line 130-139, the ordering argument. It is right, and the client does read both layouts: a flat pull on keel#34 behaves exactly as before. - Line 149-157, and particularly "When a blob may be deleted. **Never, by this note**" with the reachability criterion attached. That is the right shape for a deferred decision. - The `local` trap entry is accurate, correctly diagnosed, gives the two-statement fix, and says which test found it and that the happy path passed. I swept every shell file in every Keel repository and worktree with a quote-aware scanner validated against the known-bad shapes: **zero remaining occurrences**. The entry did its job. --- **Warning.** The note itself is sound and should land close to as written; the two HIGHs are both in `docs/traps.md`. A traps file that names the wrong implementation, and whose stated fix does not hold, costs more than a missing entry — the next reader stops checking. Fix the gpgv entry to name `apt/lib/release.sh:243-245`, to say that `channel_verified` still needs the same treatment, and to record that exit 0 plus `VALIDSIG` is produced for revoked and expired keys, so the rule is `GOODSIG`. Then the MEDIUMs: a retirement paragraph, a TTL ceiling, a clock-skew paragraph, and either a caveat on "refreshed daily" or a delivery path in apt#14.

marcos-mendez commented

2026-09-29 00:54:01 -03:00

(Migrated from github.com)

25a6456 and 9f52bf7: the note and the gpgv trap now describe what apt#14 actually does after its fixes, the rollback section says how a revision is verified, the em dashes are gone, and the status line separates the maintainer's decision (layout, expiry) from the choice of channel key, which is key custody under BRIEF section 11 and still needs the maintainer.

25a6456 and 9f52bf7: the note and the gpgv trap now describe what apt#14 actually does after its fixes, the rollback section says how a revision is verified, the em dashes are gone, and the status line separates the maintainer's decision (layout, expiry) from the choice of channel key, which is key custody under BRIEF section 11 and still needs the maintainer.