feat(labeler): verified artifact acquisition (W7.2)#2069
Conversation
Resolves a verified release's package artifact from the preferred source, fetches it under the shared SSRF-hardened controls, verifies the bytes against the signed checksum, and unpacks the canonical bundle into the analysis file set. Composes @emdash-cms/registry-verification (guarded fetch, multihash verification, bundle validation) and adds the labeler's mirror-first source preference, declared-URL fallback, and acquisition-category classification. Every failure is classified so that transport and capability faults on unverified bytes retry into assessment-error and never a public label, while only checksum/coordinate integrity faults and content-policy bundle rejections become deterministic blocking findings (spec §9.4). Non-UTF-8 code files are rejected rather than silently dropped, keeping the analyzed set a superset of the executable set. Mirror-ready seam: v1 ships no mirror binding, so every acquisition resolves via the publisher's declared URL. Extends validatePluginBundle with a validated file inventory (files) so consumers extract analysis inputs without re-parsing the bundle.
🦋 Changeset detectedLatest commit: 3811016 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
Scope checkThis PR changes 1,115 lines across 10 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 |
emdash-playground | 3811016 | Jul 16 2026, 08:55 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-do | 3811016 | Jul 16 2026, 08:55 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-cache | 3811016 | Jul 16 2026, 08:55 AM |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the right change for W7.2: it builds the labeler’s artifact-acquisition stage on top of the existing @emdash-cms/registry-verification package rather than reimplementing fetch/checksum/validation, adds a clean mirror-first seam for W0.6, and keeps the §9.4 security invariant that transport/capability faults on unverified bytes must not become public block labels. The failure-classification taxonomy, the mirror CHECKSUM_MISMATCH fallback, and the orchestrator adapter all line up, and the bundle inventory extension to validatePluginBundle is additive with a proper minor changeset.
I checked the acquisition code against the VerificationErrorCode set, traced the orchestrator integration (including how StageTransientError is thrown for retryable failures and how permanent findings map onto the policy resolver), and verified the bundle changes and AGENTS.md conventions: the changeset is present and well-worded, no admin UI/i18n/SQL changes were introduced, and the dependency additions are scoped to the labeler app and registry-verification. Test coverage looks comprehensive.
The only clear issue is a misleading docstring that says an oversized artifact is "classified as an invalid bundle," while the actual code (and the PR’s stated §9.4 intent) classifies it as retryable/transient. There is also minor comment imprecision around whether the mirror key is content-addressed. The unrelated pnpm-lock.yaml snapshot churn is noted but not flagged as a defect since I cannot verify whether it was intentional deduplication from adding modern-tar/registry-verification to the workspace.
| * Fetch budgets for a declared-URL artifact download. The byte budget equals | ||
| * the compressed-bundle cap, so an artifact larger than any valid bundle is | ||
| * rejected during streaming (the earliest point) and classified as an invalid | ||
| * bundle. Timeouts bound a slow or stalled origin. | ||
| */ |
There was a problem hiding this comment.
[needs fixing] The docstring for the declared-URL fetch limits says an artifact that hits maxBytes is "classified as an invalid bundle." That contradicts both the implementation and spec §9.4: RESOURCE_SIZE_EXCEEDED from fetchVerifiedResource is in TRANSIENT_CODES, so the orchestrator retries and finalizes it as assessment-error — it never becomes a public block label. A misleading security-level comment is a real risk because a future maintainer could rely on it.
| * Fetch budgets for a declared-URL artifact download. The byte budget equals | |
| * the compressed-bundle cap, so an artifact larger than any valid bundle is | |
| * rejected during streaming (the earliest point) and classified as an invalid | |
| * bundle. Timeouts bound a slow or stalled origin. | |
| */ | |
| /** | |
| * Fetch budgets for a declared-URL artifact download. The byte budget equals | |
| * the compressed-bundle cap, so an artifact larger than any valid bundle is | |
| * rejected during streaming (the earliest point) and classified as a transient | |
| * failure (retry → assessment-error), never as a public block label per | |
| * spec §9.4. Timeouts bound a slow or stalled origin. | |
| */ |
| /** | ||
| * Deterministic mirror object key derived from release coordinates. The mirror | ||
| * is a content-addressed copy of the exact signed artifact, so the key is | ||
| * stable across acquisitions and independent of the declared URL. | ||
| */ | ||
| export function mirrorObjectKey(target: AcquisitionTarget): string { |
There was a problem hiding this comment.
[suggestion] The comment calls the mirror "a content-addressed copy" but the key below is derived from release coordinates (slug/version/artifactId), not from the artifact’s checksum. This is harmless today but confusing: if a future mirror actually uses content-addressing, the comment will be wrong; if it stays coordinate-based, the comment should say so.
| /** | |
| * Deterministic mirror object key derived from release coordinates. The mirror | |
| * is a content-addressed copy of the exact signed artifact, so the key is | |
| * stable across acquisitions and independent of the declared URL. | |
| */ | |
| export function mirrorObjectKey(target: AcquisitionTarget): string { | |
| /** | |
| * Deterministic mirror object key derived from release coordinates. The mirror | |
| * stores the exact signed artifact, so the key is stable across acquisitions | |
| * and independent of the declared URL. | |
| */ |
github-actions
Bot
added
review/awaiting-author
…h §9.4 classification
github-actions
Bot
added
review/needs-rereview
There was a problem hiding this comment.
This remains the right shape for W7.2: it composes @emdash-cms/registry-verification instead of reimplementing fetch/checksum/validation, adds a clean mirror-first seam, and keeps the §9.4 invariant that transport/capability faults on unverified bytes do not become public block labels. The bundle inventory extension is additive with a proper minor changeset, and the orchestrator integration tests cover the happy path, integrity-failure, and transient-exhaustion flows.
The previous review's two findings are both addressed: the oversized-artifact comment now correctly calls the pre-checksum abort transient, and the mirrorObjectKey comment no longer implies content-addressing.
One new inaccuracy remains: the crossCheckCoordinates docstring claims a malformed declared checksum is a "permanent integrity fault," but the code maps the decodeMultihash failure (INVALID_MULTIHASH / UNSUPPORTED_MULTIHASH) to TRANSIENT_CODES, the PR description says malformed/unsupported declared checksums are retryable assessment-error, and artifact-acquisition.test.ts has a passing test that asserts exactly that transient classification. The implementation is correct and safe; the comment is the only thing out of sync.
AGENTS.md conventions otherwise check out: the changeset is present and user-facing, there are no admin UI / i18n / RTL / SQL / API-route changes, dependency additions are scoped, and the new files inventory export is additive. Test coverage is comprehensive.
| /** Cross-checks the pinned coordinates and the declared checksum before any | ||
| * network work: a drift between what discovery pinned and what the release | ||
| * declares, or a malformed declared checksum, is a permanent integrity fault. */ |
There was a problem hiding this comment.
[suggestion] This docstring is still misleading in the same way the prior review flagged: it says a malformed declared checksum is a "permanent integrity fault," but the implementation passes decodeMultihash errors to classifyFailure, where INVALID_MULTIHASH/UNSUPPORTED_MULTIHASH live in TRANSIENT_CODES and map to kind: "transient" / assessment-error. The PR description and the test "treats a malformed declared checksum as transient, never a public block" both agree the classification is transient, so the comment contradicts the code.
| /** Cross-checks the pinned coordinates and the declared checksum before any | |
| * network work: a drift between what discovery pinned and what the release | |
| * declares, or a malformed declared checksum, is a permanent integrity fault. */ | |
| /** Cross-checks the pinned coordinates and the declared checksum before any | |
| * network work: a drift between what discovery pinned and what the release | |
| * declares is a permanent integrity fault. A malformed or unsupported declared | |
| * checksum is a transient record/capability fault (spec §9.4), not tampering, | |
| * and is classified via the decode error. */ |
github-actions
Bot
added
review/awaiting-author
…lformed-checksum classification
github-actions
Bot
added
review/needs-rereview
There was a problem hiding this comment.
This is the right shape for W7.2: it composes @emdash-cms/registry-verification for fetch/checksum/bundle validation, adds a clean mirror-first seam, and keeps the §9.4 invariant that transport/capability faults on unverified bytes do not become public block labels.
The only outstanding issue from the previous review — the crossCheckCoordinates docstring incorrectly characterizing a malformed declared checksum as a permanent integrity fault — is now fixed. The comment distinguishes coordinate drift (permanent COORDINATE_MISMATCH) from malformed/unsupported declared checksums (decodeMultihash → INVALID_MULTIHASH/UNSUPPORTED_MULTIHASH, mapped to TRANSIENT_CODES and retried as assessment-error), which matches both the implementation and the passing tests.
I re-checked every changed hunk and traced the new code against the composed verification package:
- Classification is sound:
MIRROR_MISS/TRANSIENT_CODES/URL_POLICY_CODESall retry;INTEGRITY_CODESproduceartifact-integrity-failure; everything else from verified bytes maps toinvalid-bundle. buildCodeFileSetcorrectly rejects non-UTF-8 files with code extensions asNON_UTF8_CODE_FILE/invalid-bundle, so a lenient runtime cannot execute something the code AI never sees.- SSRF hardening remains delegated to
fetchVerifiedResource(HTTPS-only, IP-literal/private-host rejection, redirect validation). - Mirror
CHECKSUM_MISMATCHfalls through to the authoritative declared URL; mirror binding errors are treated as misses, never authoritative failures. - The bundle inventory addition in
registry-verificationis additive, has a minor changeset, and is exported through the public barrel. - Tests cover happy path, integrity mismatch, transport/policy failures, mirror fallback, non-UTF-8 executable rejection, and orchestrator integration.
No AGENTS.md conventions are violated: the changeset is present and user-facing, there are no admin UI/i18n/RTL/SQL/API-route changes, dependency additions are scoped, and there are no drive-by refactors.
Approving.
github-actions
Bot
added
review/approved
7c28520
into
feat/plugin-registry-labelling-service
1 participant
What does this PR do?
Implements verified artifact acquisition (plan W7.2, spec §9.3/§9.4) for the labeler pipeline. Given a verified release, it resolves the package artifact from the preferred source, fetches it under
@emdash-cms/registry-verification's SSRF-hardened controls, verifies the bytes against the signed checksum, and unpacks the canonical bundle into the code-analysis file set.The module composes the W7.1 verification package rather than reimplementing any of it — guarded fetch, multihash verification, and bundle validation are the shared foundation. It adds the labeler-specific layer on top: mirror-first source preference, declared-URL fallback, and classification of every failure into the four ratified acquisition categories.
The classification is the security-load-bearing part. It enforces the §9.4 discipline that a transport or capability fault on unverified bytes must never become a public block label:
assessment-error. No public label.CHECKSUM_MISMATCH, coordinate drift) and content-policy bundle rejections (decompression bomb, structurally-invalid archive, non-UTF-8 code file) become deterministic blocking findings.Non-UTF-8 code files are rejected as
invalid-bundlerather than silently dropped, so the set the AI analyzes stays a superset of the set that actually executes (a lenient runtime would still run a file the strict decoder discards).Mirror-ready seam: v1 ships no mirror binding, so the mirror source always misses and every acquisition resolves via the publisher's declared URL. Injecting a future aggregator mirror binding activates mirror-first ordering with no contract change.
Also extends
validatePluginBundlewith a validated file inventory (files) so consumers extract analysis inputs without re-parsing the bundle.Part of the plugin-registry labelling service (RFC #694, umbrella #1909). Targets the integration branch.
Type of change
Covered by the approved RFC #694 umbrella.
Checklist
pnpm typecheckpassespnpm lintpasses (zero diagnostics in changed files; pre-existing baseline unchanged)pnpm testpasses (labeler workerd 596, registry-verification node 94)pnpm formathas been runregistry-verificationminor for thefilesinventory; the labeler app is unpublishedAI-generated code disclosure
Screenshots / test output
Classification table validated against every
VerificationErrorCodethe composed package can emit; +20 acquisition tests (18 acquisition, 2 orchestrator) plus the pre-existing suites. An Opus adversarial pass caught three misclassifications (two of which produced public block labels on unverified/capability faults, one enshrined in a passing test) before this PR; all fixed and covered by new tests.Try this PR
Open a fresh playground →
A full working EmDash site, deployed from this branch. Each visit gets its own session-scoped sandbox: no login needed and no shared state. Try the admin, edit content, hit the public site.
Tracks
feat/labeler-artifact-acquisition. Updated automatically when the playground redeploys.