fix(infra): serve the apt keyring dearmored, rename to moq-keyring.gpg#1611
Conversation
apt verifies repositories with gpgv, which only reads *binary* keyrings
and rejects ASCII armor. The published keyring was armored, so
`apt-get update` failed every night with:
NO_PUBKEY 8FBE8982FDD3FCA2
E: The repository 'https://apt.moq.dev stable InRelease' is not signed.
even though the key was correct -- `gpg --verify` accepts armor, so manual
checks looked fine, but gpgv chokes on the armor header
(`invalid packet (ctb=2d)`, ctb 0x2d = the '-' of "-----BEGIN").
- infra/README.md: export the key *binary* for apt and *armored* for rpm
instead of pushing the same armored file to both buckets (the root cause).
- apt/rpm workers: stop serving the keyring `immutable`/30d. It's a
fixed-name trust anchor whose bytes change on key rotation; immutable
would pin a stale/rotated copy for the whole cache window and block
recovery.
- rename moq-archive-keyring.gpg -> moq-keyring.gpg across docs and workers.
The keyring stays manually managed (publish.sh deliberately never touches
the trust anchor); this needs a one-time binary re-upload to R2, see
infra/README.md step 4.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WalkthroughThis PR renames the project's signing keyring artifact from 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
infra/apt/src/worker.ts (1)
40-47: ⚡ Quick winExtract the cache TTLs into named constants.
2592000,3600, and300make this policy harder to audit when the next cache tweak or key rotation lands. Pulling them into named constants keeps the intent obvious.♻️ Suggested refactor
+const IMMUTABLE_PACKAGE_CACHE_TTL_SECONDS = 60 * 60 * 24 * 30; +const KEYRING_CACHE_TTL_SECONDS = 60 * 60; +const METADATA_CACHE_TTL_SECONDS = 5 * 60; + function cacheControl(key: string): string { if (key.endsWith(".deb") || key.includes("/pool/")) { - return "public, max-age=2592000, immutable"; + return `public, max-age=${IMMUTABLE_PACKAGE_CACHE_TTL_SECONDS}, immutable`; } if (key === "moq-keyring.gpg") { - return "public, max-age=3600"; + return `public, max-age=${KEYRING_CACHE_TTL_SECONDS}`; } - return "public, max-age=300"; + return `public, max-age=${METADATA_CACHE_TTL_SECONDS}`; }As per coding guidelines, "Avoid using magic numbers; use named constants instead."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@infra/apt/src/worker.ts` around lines 40 - 47, Extract the numeric TTLs in cacheControl into named constants (e.g., DEB_POOL_TTL = 2592000, KEYRING_TTL = 3600, DEFAULT_TTL = 300) and replace the literal numbers in the return strings inside cacheControl(key: string) with those constants (e.g., `public, max-age=${DEB_POOL_TTL}, immutable`). Ensure the constants are defined near the top of the module and use clear names like DEB_POOL_TTL / KEYRING_TTL / DEFAULT_TTL to make intent obvious and simplify future changes.infra/rpm/src/worker.ts (1)
33-40: ⚡ Quick winExtract the cache TTLs into named constants.
Same issue here: the cache behavior is correct, but the raw TTL literals make future rotations and cache-policy edits harder to reason about.
♻️ Suggested refactor
+const IMMUTABLE_PACKAGE_CACHE_TTL_SECONDS = 60 * 60 * 24 * 30; +const KEYRING_CACHE_TTL_SECONDS = 60 * 60; +const METADATA_CACHE_TTL_SECONDS = 5 * 60; + function cacheControl(key: string): string { if (key.endsWith(".rpm")) { - return "public, max-age=2592000, immutable"; + return `public, max-age=${IMMUTABLE_PACKAGE_CACHE_TTL_SECONDS}, immutable`; } if (key === "moq-keyring.gpg") { - return "public, max-age=3600"; + return `public, max-age=${KEYRING_CACHE_TTL_SECONDS}`; } - return "public, max-age=300"; + return `public, max-age=${METADATA_CACHE_TTL_SECONDS}`; }As per coding guidelines, "Avoid using magic numbers; use named constants instead."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@infra/rpm/src/worker.ts` around lines 33 - 40, Extract the numeric TTLs in cacheControl into named constants (e.g., RPM_TTL = 2592000, MOQ_KEYRING_TTL = 3600, DEFAULT_TTL = 300) declared near the top of the module, then replace the raw literals in cacheControl with those constants when constructing the cache-control strings (e.g., `public, max-age=${RPM_TTL}, immutable` for RPMs). Keep the existing behavior (immutable for .rpm, shorter TTL for "moq-keyring.gpg", default otherwise) and ensure the constants are exported or documented if they will be reused elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@doc/setup/linux.md`:
- Around line 103-110: The verification snippet is missing steps to download the
key and Release files, so update the example to first fetch moq-keyring.gpg,
Release and Release.gpg (e.g., using curl -fsSLO against
https://apt.moq.dev/moq-keyring.gpg and the Release/Release.gpg URLs) before
running gpg --import moq-keyring.gpg and gpg --verify Release.gpg Release;
ensure the commands reference the same Release file and key filenames shown
(moq-keyring.gpg, Release, Release.gpg) so the example is self-contained and
reproducible.
---
Nitpick comments:
In `@infra/apt/src/worker.ts`:
- Around line 40-47: Extract the numeric TTLs in cacheControl into named
constants (e.g., DEB_POOL_TTL = 2592000, KEYRING_TTL = 3600, DEFAULT_TTL = 300)
and replace the literal numbers in the return strings inside cacheControl(key:
string) with those constants (e.g., `public, max-age=${DEB_POOL_TTL},
immutable`). Ensure the constants are defined near the top of the module and use
clear names like DEB_POOL_TTL / KEYRING_TTL / DEFAULT_TTL to make intent obvious
and simplify future changes.
In `@infra/rpm/src/worker.ts`:
- Around line 33-40: Extract the numeric TTLs in cacheControl into named
constants (e.g., RPM_TTL = 2592000, MOQ_KEYRING_TTL = 3600, DEFAULT_TTL = 300)
declared near the top of the module, then replace the raw literals in
cacheControl with those constants when constructing the cache-control strings
(e.g., `public, max-age=${RPM_TTL}, immutable` for RPMs). Keep the existing
behavior (immutable for .rpm, shorter TTL for "moq-keyring.gpg", default
otherwise) and ensure the constants are exported or documented if they will be
reused elsewhere.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fbfefabd-ebac-4fa8-af0c-5449ad873084
📒 Files selected for processing (7)
doc/setup/linux.mdinfra/README.mdinfra/apt/src/worker.tsinfra/rpm/publish.shinfra/rpm/src/worker.tsrs/moq-gst/README.mdrs/moq-relay/README.md
| - <https://apt.moq.dev/moq-keyring.gpg> | ||
| - <https://rpm.moq.dev/moq-keyring.gpg> | ||
|
|
||
| Verify the apt repository metadata signature manually: | ||
|
|
||
| ```bash | ||
| gpg --import moq-archive-keyring.gpg | ||
| gpg --import moq-keyring.gpg | ||
| gpg --verify Release.gpg Release |
There was a problem hiding this comment.
Make the verification example self-contained.
This block imports moq-keyring.gpg from the current directory, but the section never shows how to download the key or the Release files first. Readers following it verbatim will fail before they reach gpg --verify.
📝 Suggested doc fix
Verify the apt repository metadata signature manually:
```bash
+curl -fsSLO https://apt.moq.dev/moq-keyring.gpg
+curl -fsSLO https://apt.moq.dev/dists/stable/Release
+curl -fsSLO https://apt.moq.dev/dists/stable/Release.gpg
gpg --import moq-keyring.gpg
gpg --verify Release.gpg Release</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @doc/setup/linux.md around lines 103 - 110, The verification snippet is
missing steps to download the key and Release files, so update the example to
first fetch moq-keyring.gpg, Release and Release.gpg (e.g., using curl -fsSLO
against https://apt.moq.dev/moq-keyring.gpg and the Release/Release.gpg URLs)
before running gpg --import moq-keyring.gpg and gpg --verify Release.gpg
Release; ensure the commands reference the same Release file and key filenames
shown (moq-keyring.gpg, Release, Release.gpg) so the example is self-contained
and reproducible.
</details>
<!-- fingerprinting:phantom:poseidon:grasshopper -->
<!-- This is an auto-generated comment by CodeRabbit -->
1 participant
Problem
The nightly cross-language smoke run fails on the apt channel, deterministically:
The signing key is correct and
InReleaseis properly signed by it —gpg --verifyreports a good signature. The bug is encoding:aptverifies repositories withgpgv, which only reads binary keyrings and rejects ASCII armor. We publishedmoq-archive-keyring.gpgin armored form, so gpgv dies on the first byte:→ no usable key →
NO_PUBKEY. Dearmoring the exact same key to binary makes gpgv report "Good signature".Root cause:
infra/README.mdstep 4 exported the key withgpg --export --armorand pushed that same armored file to both the apt and rpm buckets. Armored is correct for rpm/dnf, but wrong for apt.Changes
gpg --export, no--armor) and armored for rpm, instead of one armored file for both. This is the actual fix.immutable, max-age=2592000. It's a fixed-name trust anchor whose bytes change on key rotation;immutablewould pin a stale/rotated/broken copy for the whole 30-day window and block recovery. Now a bounded 1h TTL (content-addressed.deb/.rpmblobs stay immutable).moq-archive-keyring.gpg→moq-keyring.gpgacross docs and workers (old name dropped).The keyring stays manually managed —
publish.shdeliberately never touches the trust anchor, so a key swap can't silently re-publish itself, and a rotation can keep old+new keys side by side (the key is shared with Maven, whose old artifacts stay signed by the old key).The renamed URL won't exist until the binary key is uploaded to R2 (see infra/README.md step 4):
Then
just infra apt deploy/just infra rpm deployfor the cache-header change. Existing installs that already fetched the broken armored file locally will need to re-run the install command.Verification
Reproduced both the failure (armored →
gpgv: invalid packet) and the fix (dearmored →gpgv: Good signature) locally against the liveapt.moq.devendpoint. Biome + remark lint pass on changed files.Note: the same nightly run also has an unrelated macOS failure (the C subscriber hitting the anonymous GitHub API rate limit) — tracked/fixed separately in the smoke repo.
🤖 Generated with Claude Code