feat(registry): verify delegated releases independently - #2746
Conversation
🦋 Changeset detectedLatest commit: 0dc935c The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
review/needs-review
Scope checkThis PR changes 22,401 lines across 71 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
docs | 547e7f5 | Aug 28 2026, 11:12 PM |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds the isolated release-verifier Worker, direct-PDS record reads, and independent install/update verification, which is the right next step for the delegated-release stack. The architecture keeps moderation labels as metadata-only visibility inputs bound to exact signed CIDs, while the artifact, manifest, and provenance are verified independently — matching the design from the approved Discussion.
Static review found no security regressions, no SSRF bypasses in the new paths (PDS and provenance fetches both validate hosts), and no broken contracts in the consent flow. The admin strings are Lingui-wrapped, RTL-safe wrappers are used for CID/DID display, and the changed routes are authenticated (plugins:manage / plugins:read). The test coverage is extensive across core, admin, registry-verification, and release-service.
I do have one needs_fixing finding on the changeset: it publicly states that nonconforming bare-hexadecimal checksums are rejected, but the registry artifact proxy still accepts them via verifyRegistryArtifactChecksum. Two suggestions round out the review: a numbered module comment that violates the repo's comment discipline, and a latent footgun where DirectPdsClient caches a rejected publisher-resolution promise.
The test checklist in the PR description is taken at face value; I did not run the suite.
Findings
-
[needs fixing]
.changeset/verify-registry-bundles.md:26The changeset states: "Release records must contain a lowercase base32 multibase
sha2-256multihash. Existing releases produced by the EmDash plugin CLI already use this format; nonconforming bare hexadecimal checksums are rejected."That is only true for the install/update path. The registry artifact proxy (
packages/core/src/astro/routes/api/admin/plugins/registry/artifact.ts) still resolves artifact checksums throughverifyRegistryArtifactChecksum, which explicitly accepts a 64-character bare hex SHA-256 digest in addition to multibase multihashes. A release record whose image artifact uses bare hex will still verify and be served, so the blanket rejection claim is inaccurate.Either update the changeset to say that bare hex is rejected only during install/update/verification, or update the artifact proxy to require multibase checksums as well.
Release records must contain a lowercase base32 multibase `sha2-256` multihash for install, update, and verification. Existing releases produced by the EmDash plugin CLI already use this format; nonconforming bare hexadecimal checksums are rejected for plugin installs and updates. -
[suggestion]
packages/core/src/api/handlers/registry.ts:7-26This module comment enumerates the install flow with explicit numbers (
1.,2., …,11.). AGENTS.md says numbered comments are always wrong because they are brittle narrative that quickly goes stale when steps are reordered. The list was also re-numbered in this diff, which makes the fragility visible.Rewrite the header as prose paragraphs that describe the trust boundaries and sources of truth, without an ordered list. The numbered sequence belongs in the implementation, not the comment.
-
[suggestion]
packages/registry-client/src/direct-pds/index.ts:214this.#resolvedPublisher ??= this.#resolvePublisher()will store a rejected promise if the first DID-document resolution fails. Because a rejected promise is truthy, every subsequent read on the same client instance will await that same rejection rather than retrying resolution. The clients created byreadAuthoritativePackageReleaseare short-lived today, but this is still a latent footgun if the client is ever reused or if a transient PDS/TCP timeout makes the instance permanently unusable.Clear the cached promise on rejection so later reads can retry:
this.#resolvedPublisher ??= this.#resolvePublisher(); try { return await this.#resolvedPublisher; } catch (error) { this.#resolvedPublisher = undefined; throw error; }
github-actions
Bot
added
review/awaiting-author
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
22cf022 to
b38f41c
Compare
github-actions
Bot
added
review/needs-rereview
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-moderation
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
github-actions
Bot
added
the
query-count changed
97d3e4e to
2310136
Compare
github-actions
Bot
added
query-count changed
github-actions
Bot
removed
the
query-count changed
🔍 Adversarial review — stack #2766, layer 5 of 8Automated deep review of this layer's diff ( What the PR actually does + verdictIt adds an isolated Findings
PR description vs code
Test-coverage gaps
~ 🤖 Fable |
99343df to
adfb630
Compare
adfb630 to
9091ddb
Compare
9091ddb to
29a2bdc
Compare
github-actions
Bot
added
the
query-count changed
0b3fe34 to
29a2bdc
Compare
github-actions
Bot
removed
the
query-count changed
4d1c6cf to
547e7f5
Compare
github-actions
Bot
added
query-count changed
1 participant
What does this PR do?
Adds the isolated verifier Worker, shared artifact/manifest/provenance verification, authoritative direct-PDS record reads, release Workflow verification steps, and independent EmDash installer enforcement. Moderation labels remain metadata-only visibility inputs bound to exact signed profile or release CIDs; they never attest to plugin code.
This is PR 4 of 7. It retains the main verification commit and the artifact-proxy checksum compatibility follow-up separately. The stack merges as one unit and this branch is not deployable by itself.
Discussion: #1590
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pofiles are includedAI-generated code disclosure
Screenshots / test output
Core direct-PDS install conformance, admin, registry-client, registry-verification Node/workerd, release-service verification, and verifier Worker suites pass.