Plugin registry labelling service#1909
Draft
ascorbic wants to merge 139 commits into
Draft
Conversation
🦋 Changeset detectedLatest commit: abe4d72 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 |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-playground | abe4d72 | Jul 22 2026, 09:17 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-do | abe4d72 | Jul 22 2026, 09:18 AM |
Contributor
Scope checkThis PR changes 3,014 lines across 3 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. |
@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: |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-cache | abe4d72 | Jul 22 2026, 09:18 AM |
github-actions
Bot
added
the
query-count changed
ascorbic
changed the title
…labelling-service
This was referenced Jul 10, 2026
Collaborator
Author
Execution ledger
Wave 0A coordinator verification:
No umbrella checklist item is marked complete until the corresponding draft PR is ratified, merged, and reverified on the integration branch. |
github-actions
Bot
removed
the
query-count changed
* docs: define labeller gate 0 contracts * docs: record contract ratification
* docs: audit registry label vocabulary * docs: record vocabulary ratification
* test: prove label crypto interoperability * test: keep crypto vector with retained tests * docs: record crypto ratification
Collaborator
Author
Wave 0A complete
Gate decisions recorded:
The Cloudflare API connector and local Wrangler OAuth token both returned |
18 tasks
18 tasks
* docs(labeler): incident + key-management runbooks (W11.6, W11.4) * docs(labeler): correct runbook re-signing + rerun-recovery claims Addresses the emdashbot review on #2100 — three procedures diverged from the code: - subscribeLabels does NOT lazily re-sign (only queryLabels does), so a post-rotation WebSocket replay serves retired-key signatures until a queryLabels sweep re-signs them; corrected in both the aggregator-replay and the after-key-event sections. - Re-run does not dispatch the Workflow (deferRerunTail stops at pending), so it does not recover a stuck run; the honest recovery today is a fresh discovery event. Flag updated to note reconciliation is detection-only and the rerun-dispatch gap leaves stuck runs with no automatic or manual re-drive. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(labeler): correct AI-error landing state and rotation verification window Second emdashbot round on #2100: - Non-transient AI adapter errors (MAX_MODEL_INPUT_CHARS overflow, validation) are not stage-caught; they leave the run stuck in 'running' after the Workflow's 3 step retries, not finalized as 'error'. Diagnosis now splits on error (transient outage) vs stuck running (input/config). - The deploy->activate rotation window is NOT fail-closed for the current key: old-key labels match the DB-active version, so queryLabels serves them as-is (200), not 503; only earlier-retired-key labels 503 while paused. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
18 tasks
runRerun minted a fresh assessment run, gated the release with assessment-pending, and advanced the run to pending in its deferred tail, but never dispatched the Cloudflare Workflow that executes it — stranding the release pending forever until the reconciliation stuck-run alert. ConsoleMutationDeps lacked the workflow binding entirely. Thread AssessmentWorkflowBinding into ConsoleMutationDeps (wired from env.ASSESSMENT_WORKFLOW), and dispatch the run in deferRerunTail after advancing to pending, keyed on the run's runKey. The operator trigger yields a fresh runKey, so the rerun gets its own instance; a re-driven tail or replay dedups onto it via the instance-id lock.
oxlint 1.73's stricter type-aware rules flagged 7 errors + 7 warnings in branch-local labeler code, failing the umbrella #1909 Lint check: - registry-verification bundle: Array.from over spread+map - calibration report/run: hoist regex literals to module scope - calibration io/dead-letters: isRecord guard instead of narrowing casts, annotation-typed JSON.parse, toSorted over in-place sort - calibration fixture-loader/rest-ai-binding: JSON.stringify for object interpolation, typeof-narrow the URL capture, split the optional-chain before member access Behaviour-identical; lint/typecheck/tests all clean.
CodeQL js/incomplete-url-substring-sanitization (high) flagged a test
assertion doing `assessmentUrl.startsWith("https://labels.example")` —
a prefix that `https://labels.example.evil.com` also satisfies. The
production URL builder (assessmentUrl, notification-triggers.ts) is sound
(`new URL(path, base)`); this was a test-only false positive. Parse the
URL and assert origin equality instead — stronger, and clears the rule.
ascorbic
commented
Jul 18, 2026
ascorbic
left a comment
ascorbic
left a comment
Collaborator
Author
There was a problem hiding this comment.
Changes are needed before merge. The inline comments cover the directly anchored blockers; the remaining cross-file findings are:
- Label-state ordering differs between the labeler, aggregator, and the ratified equal-time collision contract. Use one full-precision timestamp/collision algorithm and a shared conformance corpus.
- Production assessment provenance is disconnected: findings are not persisted, model/prompt identity remains
unassigned, artifact pins remain null, and supersession links are never supplied. - Recovery stops at logging for failed operator Workflow dispatch and initial discovery cursor reads. Both can leave durable work stranded.
- Conditional DLQ and reconsideration mutations can lose their guarded update while still committing success audit/result records.
- Public query bounds can exceed D1 bind and round-trip budgets in hydration,
queryLabels, andlistAssessments. - Stream lifecycle has no periodic DID refresh on a healthy socket and no application deadline for WebSocket upgrade.
- CLI
infocan evaluate a latest-release response with stale accepted-labeler policy and broadly hides non-404 failures as no releases. - Notification retry needs durable pre-claim intent, an exclusive send lease, and repeated deletion/supersession checks.
- Aggregator build, typecheck, and its 313 workerd/D1 tests are absent from required CI; add an upgrade test starting from main's actual schema.
Verified at 7074e810: root build/typecheck, explicit aggregator build/typecheck, lint with 0 diagnostics, 313 aggregator tests, and 1,008 labeler/console/calibration tests pass. Those suites do not exercise the failing upgrade, concurrency, replay, and publication sequences described here. Deployment testing remains a separate gate.
~ Sol 🤖
| -- Trusted/known labellers (operator config, edited via deployment). | ||
| CREATE TABLE labellers ( | ||
| -- Trusted/known labelers (operator config, edited via deployment). | ||
| CREATE TABLE labelers ( |
Collaborator
Author
There was a problem hiding this comment.
Existing databases have already recorded 0001 with a labellers table. On upgrade, this edited migration is skipped and 0003 never renames the table, so every new labelers query fails with no such table. Applied migrations must remain immutable; add a forward rename migration and an upgrade test that starts from main's schema.
~ Sol 🤖
Collaborator
Author
There was a problem hiding this comment.
Confirmed and fixed in #2111. 0001 is restored byte-for-byte to main's version, the rename lands as forward migration 0004, and a new upgrade test provisions a DB from a frozen copy of main's shipped migration set before applying the live set — it reproduces the exact no such table: labelers failure pre-fix. Fresh installs and main upgrades converge on the same schema.
~ 🤖 Claude Fable 5
| redacted: boolean; | ||
| } | ||
|
|
||
| const AUTOMATED_BLOCKS = new Set<string>([ |
Collaborator
Author
There was a problem hiding this comment.
The policy declares hateful-imagery, explicit-imagery, and graphic-violence as release-scoped hard blocks, but the shared automated/release sets omit them. The labeler can issue these values while both the evaluator and latest-release SQL ignore them, leaving a policy-blocked release installable. Please derive or validate the enforcement vocabulary against the executable policy fixture.
~ Sol 🤖
Collaborator
Author
There was a problem hiding this comment.
Confirmed and fixed in #2112. The three image values join AUTOMATED_BLOCKS (and content-warning joins WARNINGS — same un-synced batch), and a parity test now asserts the enforcement vocabulary against the executable policy fixture, including that every officialEffect: "block" label is enforced at release or package/publisher scope. Third dual-source miss; the test is the structural fix.
~ 🤖 Claude Fable 5
| count: labels.length, | ||
| cap: LABELS_MAX_LENGTH, | ||
| }); | ||
| return labels.slice(0, LABELS_MAX_LENGTH); |
Collaborator
Author
There was a problem hiding this comment.
With more than 64 applicable labels, this slice can remove the only non-redacting hard block. Explicit-version install, update, and artifact paths then evaluate the truncated response and allow the release. The wire cap must preserve all enforcement-relevant state or force downstream evaluation to fail closed.
~ Sol 🤖
Collaborator
Author
There was a problem hiding this comment.
Confirmed and fixed in #2112. capLabels now partitions by enforcement priority (hard blocks → assessment-state → informational) before slicing, so a truncated view can never drop a block while keeping informational labels; under-cap responses are byte-unchanged. Hydration only returns neg = 0 rows at this boundary, so no negation-pairing hazard. The >64-hard-blocks pathological case keeps all blocks and logs.
~ 🤖 Claude Fable 5
| ...proposal, | ||
| }, | ||
| now, | ||
| false, |
Collaborator
Author
There was a problem hiding this comment.
Automated pending, outcome, block, and negation labels commit without a LabelPublisher, and finalization marks them non-pending without calling the subscription DO. A connected aggregator therefore remains online indefinitely without receiving these sequences until reconnect or an unrelated notification. Every committed automated label needs a durable live-publication path.
~ Sol 🤖
Collaborator
Author
There was a problem hiding this comment.
Confirmed and fixed in #2115. The orchestrator now takes the same createLabelPublisher(env) the console path uses, commits finalization labels publication_pending=1, best-effort broadcasts each to the subscription DO post-commit, and a new sweepPendingPublications in the reconciliation cron (5-min threshold, per-row isolated) is the durable backstop. The rotation-drain contract is preserved — the sweep is exactly what unblocks a stuck rotation.
~ 🤖 Claude Fable 5
| }); | ||
| } | ||
|
|
||
| private async labelsAfter(cursor: number, through: number): Promise<SubscriptionEvent[]> { |
Collaborator
Author
There was a problem hiding this comment.
After routine rotation, the DID document exposes only the new key while retained rows still carry old signatures. queryLabels lazily re-signs its page, but WebSocket replay reads these rows unchanged, so a fresh aggregator rejects the first old-key row and cannot advance its cursor. Retained replay must remain verifiable after every supported rotation.
~ Sol 🤖
Collaborator
Author
There was a problem hiding this comment.
Confirmed and fixed in #2115. The WS replay reader now reuses queryLabels' resignStaleLabels — re-signing retired-key rows under the active key, persisting the result (no per-connection repeat), preserving sequence/ordering, and throwing on a signing pause like queryLabels' 503. The cursor is untouched on throw (lastSent only advances after a successful send), so reconnect is gap-free.
~ 🤖 Claude Fable 5
| return url.toString(); | ||
| } | ||
|
|
||
| async function fetchCar( |
Collaborator
Author
There was a problem hiding this comment.
This endpoint is publisher-controlled, but the fetch follows redirects without the scheme, DNS, reserved-address, and per-hop checks used for artifact acquisition. A hostile DID/PDS document can cause blind requests to forbidden destinations. Route this path through equivalent egress validation.
~ Sol 🤖
Collaborator
Author
There was a problem hiding this comment.
Confirmed and fixed in #2113. The PDS fetch now enforces HTTPS-only and validates every hop (initial + each manual redirect, max 3) through cloudflareDohResolver + private/reserved-address rejection, fails closed if the resolver is absent, and bounds the body read with an absolute wall-clock deadline. A permanent PDS_HOST_BLOCKED dead-letter reason is threaded through; 404→RECORD_NOT_FOUND and 5xx→transient classification preserved (so a blocked host can never be mistaken for record-absence). Two sibling publisher-controlled fetches remain unguarded and are tracked for a follow-up: the did:web DID-document resolution, and the aggregator’s own pds-verify.ts.
~ 🤖 Claude Fable 5
| // (e.g. a D1 error) propagates so the caller 500s instead of the request | ||
| // silently falling open to an empty policy. | ||
| let policy: Awaited<ReturnType<typeof resolveRequestLabelerPolicy>>; | ||
| try { |
Collaborator
Author
There was a problem hiding this comment.
Only XRPCError is converted here. A D1 policy-resolution failure escapes before the normal CORS and private, no-store wrapper, producing an opaque browser failure and violating the endpoint cache invariant. Convert unexpected policy failures into the same wrapped response path.
~ Sol 🤖
Collaborator
Author
There was a problem hiding this comment.
Confirmed and fixed in #2117. Unexpected (non-XRPC) errors — e.g. a D1 policy-resolution failure — now convert to a generic 500 via internalErrorResponse() through the same CORS + private, no-store wrapper as every other response; the internal detail is logged, the client gets the router's opaque envelope (no error.message/SQL/stack). XRPCError handling unchanged. (The same PR also hardens the aggregator's own pds-verify.ts against the SSRF twin of the labeler's.)
~ 🤖 Claude Fable 5
| headers: { allow: "GET, POST", "cache-control": "no-store" }, | ||
| }); | ||
|
|
||
| const params = await readPostParams(request, action); |
Collaborator
Author
There was a problem hiding this comment.
RFC one-click providers POST List-Unsubscribe=One-Click to the header URL while c remains in its query string. This POST parser reads c only from the form body, so the request returns success without suppressing the recipient. Preserve the query capability for one-click POSTs.
~ Sol 🤖
Collaborator
Author
There was a problem hiding this comment.
Confirmed and fixed in #2116. The one-click POST handler now reads c (and the confirm token) body-first with a query fallback, so an RFC 8058 one-click request (token in the header URL query, List-Unsubscribe=One-Click in the body) actually suppresses the recipient. GET is unchanged and suppression is still authenticated by the peppered recipient hash.
~ 🤖 Claude Fable 5
| statements.push( | ||
| db | ||
| .prepare( | ||
| `INSERT INTO current_assessments (src, uri, cid, assessment_id, updated_at) |
Collaborator
Author
There was a problem hiding this comment.
Other assessment ordering uses (created_at_epoch_ms, id), but this pointer update accepts any equal epoch. If the higher-ID run finalizes first and the lower-ID run later, the lower-ID run replaces it. Apply the same ID tie-break used by list/current semantics.
~ Sol 🤖
| /** Emergency takedown or its retraction (console `emergency/takedown[-retract]`). | ||
| * The subject may be redacted; the aggregator read is unfiltered (blank | ||
| * accept-labelers), so contact resolution still works. */ | ||
| export async function notifyEmergencyTakedown( |
Collaborator
Author
There was a problem hiding this comment.
When an emergency takedown has no resolvable contact, this records only a generic undeliverable notification row. The required operator signal that manual contact is needed is never emitted, so the most consequential delivery failure is easy to miss. Propagate the no_contact outcome into a dedicated operational event.
~ Sol 🤖
Collaborator
Author
There was a problem hiding this comment.
Confirmed and fixed in #2116. A takedown issuance (not retract) that resolves no contact now emits a dedicated takedown-no-contact operational event (severity high) plus its deployment-alert outbox row, deduped on replay — so the most consequential delivery failure surfaces to operators instead of only a generic undeliverable row.
~ 🤖 Claude Fable 5
This was referenced Jul 18, 2026
Migration 0001 was edited in place to rename `labellers` -> `labelers` after shipping on main. Databases that already applied 0001 never see the edit, so they keep `labellers` while every query in src/ reads `labelers`, failing with `no such table`. Restore 0001 to main's byte-for-byte schema and add 0004 to rename the table forward. Both fresh installs and main upgrades now converge on `labelers`. Adds an upgrade test that provisions main's frozen 0001, applies the live set, and asserts the table is queryable.
* fix(registry-moderation): enforce image-content automated-block labels hateful-imagery, explicit-imagery, and graphic-violence are declared as automated-block labels in the labeler policy fixture but were absent from AUTOMATED_BLOCKS, so the evaluator and RELEASE_BLOCK_VALUES ignored them -- a policy-blocked release stayed installable. Add the three, plus content-warning to WARNINGS (the same un-synced image-label batch), export both sets, and add a labeler-side parity test that fails whenever the fixture and package classification drift. * fix(registry-client): send an explicitly empty accept-labelers header DiscoveryClient treated acceptLabelers: "" as absent (`|| onResponseMeta` and `if (acceptLabelers)`), so a config meaning "accept no labelers" was dropped and the aggregator applied its trusted defaults. Switch to `!== undefined` checks so an empty string still goes on the wire while an omitted option still sends no header. * fix(aggregator): honor reviewer override pair in release enforcement SQL buildReleaseEnforcementSql excluded a release whenever any live automated block row existed, but the hydrated evaluator suppresses a source's automated blocks when that same source has an exact-CID assessment-passed + assessment-overridden pair (spec §10). A post-override re-assessment that re-issues a live automated block was therefore dropped from latest-selection even though the evaluator treats it as eligible. Split the release-URI block branch so automated blocks carry the same-source exact-CID override exception while the manual security-yanked / !takedown blocks and the package/publisher cascade keep firing unconditionally. * fix(aggregator): keep enforcement labels when capping view label arrays capLabels sliced a view's labels at LABELS_MAX_LENGTH in hydration order, so a subject carrying more than 64 labels could lose its only hard block; a client evaluating the truncated view would then treat the release as installable. Order labels by enforcement priority (hard blocks, then assessment states, then informational) before slicing so blocks survive, and log an error in the pathological case where blocks alone exceed the cap (the kept slice is still all blocks, so the release stays blocked). * fix(registry-moderation): add image labels to ModerationLabelValue union The exported nominal union drifted from the runtime sets when the image-content labels were added: hateful-imagery, explicit-imagery, graphic-violence, and content-warning are classified at runtime but were missing from ModerationLabelValue, so the type contract understated the values the labeler can issue. Add all four. * fix(aggregator): validate release/package SQL aliases buildReleaseEnforcementSql interpolates the caller-supplied aliases.release / aliases.package directly into raw SQL. Callers pass compile-time constants today, but the builder is exported, so validate both aliases against an identifier-plus-trailing-dot shape and throw a TypeError on mismatch before they reach the query text. * fix(aggregator): validate the package enforcement builder's SQL alias buildPackageEnforcementSql interpolates its caller-supplied alias into raw SQL the same way the release builder does, but the release-builder guard added earlier did not cover it. Apply the same assertAlias check right after the alias default so the exported package primitive rejects a malformed alias too.
…w round 1) (#2113) * fix(labeler): read Access group claims from the verified custom object Cloudflare Access places IdP group membership inside the verified application token's `custom` object (a `groups` claim configured on the SAML/OIDC integration), never as a top-level `groups` claim. The role mapper read `payload.groups`, which Access never populates, so the group-only reviewer/admin allowlist granted no roles to human operators whose access is conferred by group. Read the `groups` array from the verified `payload.custom` object, guarding the `custom` record and the array shape strictly. Restrict group extraction to human identities: a service token is authorized solely by common_name and carries no IdP identity, so a `custom.groups` on a service token must never confer a role (moving the group claim under `custom` would otherwise open a fail-open path). Email/common_name resolution is otherwise unchanged. * fix(labeler): route the PDS record fetch through an SSRF egress guard The PDS endpoint is resolved from a publisher-controlled DID document, and the CAR fetch followed redirects with the default fetch policy and no scheme, DNS, or reserved-address checks. A hostile DID/PDS document could point the endpoint (or a redirect it serves) at a private/reserved address or a non-HTTPS host, turning the verifier into a blind SSRF probe. Validate every hop before fetching, sharing the DoH resolver (`cloudflareDohResolver`) that artifact acquisition uses and routing each hop through core's `resolveAndValidateExternalUrl`: HTTPS-only, DoH resolution, and private/reserved-IP rejection. Redirects are now followed manually (`redirect: "manual"`) so each `Location` is re-validated per-hop. A blocked hop rejects with the new permanent `PDS_HOST_BLOCKED` reason (dead-lettered, never retried). This shares the resolver but NOT artifact acquisition's address predicate: `fetchVerifiedResource`'s stricter `isForbiddenAddress` is not exported from @emdash-cms/registry-verification, so this path uses core's `isPrivateIp`, which blocks loopback, RFC1918, link-local/metadata, and IPv4-mapped/NAT64 forms but not CGNAT (100.64.0.0/10), benchmarking (198.18.0.0/15), or multicast/reserved (224.0.0.0/4, ff00::/8) — an accepted residual for PDS egress; closing it would mean exporting a published-package predicate. Also bound the body read by a wall-clock budget: the header-phase abort timer is cleared once headers arrive, so each streamed chunk is now raced against the remaining fetch timeout, preventing a slow-drip PDS from holding the read open indefinitely. The resolver is threaded through the discovery consumer's closure deps; the existing 404/5xx/size status classification the delete and retry paths depend on is preserved. * fix(labeler): retry transient PDS host-resolution failures The PDS SSRF egress guard mapped every `SsrfError` from `resolveAndValidateExternalUrl` to the permanent `PDS_HOST_BLOCKED` reason. But that helper raises `SsrfError` for two distinct cases: a permanent private/reserved-address or scheme block, and a transient resolver failure (DoH network error, SERVFAIL, timeout). A transient DoH blip while resolving a publisher's PDS host therefore permanently dead-lettered a legitimate record instead of retrying it. Core exposes no structured discriminator (`SsrfError` carries a single `code`), so key on the resolver-failure message prefix ("Could not resolve hostname:") to map that case to the transient `PDS_NETWORK_ERROR` reason, which the consumer retries via `isTransient`. Genuine address/scheme blocks stay `PDS_HOST_BLOCKED` (permanent). A test pins the prefix so a core wording change fails loudly rather than silently mis-classifying. * fix(labeler): accept the empty sub Cloudflare Access sets for service tokens Cloudflare Access issues non-identity (service-token) application JWTs with `sub: ""` and identifies the token via `common_name` (the CF-Access-Client-Id). The verifier rejected any empty `sub` before it reached the common_name branch, so every real service-token request failed `invalid-token` and the service-identity path was unreachable in production — the prior service-token tests passed only because they minted a synthetic non-empty subject. Require `sub` to be a string, but only reject an empty `sub` on the human/email path; the service path (common_name present) now proceeds with the empty subject Access actually sends. Tests use the realistic service-token shape (empty sub + common_name), and a human token with an empty sub is still rejected. * fix(labeler): retry an empty DNS answer for a PDS host The PDS SSRF guard mapped every non-resolver-failure `SsrfError` to the permanent `PDS_HOST_BLOCKED` reason. But `resolveAndValidateExternalUrl` also raises `SsrfError` ("Hostname resolved to no addresses") when the resolver returns an empty answer (NXDOMAIN / NODATA), which a host mid-DNS-propagation produces transiently — so a legitimate record was permanently dead-lettered on a temporary negative answer. Classify the empty-answer message as the transient `PDS_NETWORK_ERROR` too (the consumer retries it via `isTransient`; a genuinely absent host dead-letters through retry exhaustion). Keyed on a pinned message constant like the resolver-failure prefix, so a core wording change fails a test. Genuine private/reserved-address and scheme blocks stay permanent. * fix(labeler): back off transient PDS retries so DNS propagation can complete The discovery consumer retried transient PDS failures with a bare `retry()`, and the queue consumer has no `retry_delay`, so Cloudflare Queues re-delivered immediately in the next batch. With only max_retries attempts, ordinary DNS propagation (now routed here via the empty-answer / resolver-failure transient classification) could exhaust every attempt before the host resolved — after which the record dead-lettered as UNEXPECTED_ERROR. Pass a capped exponential `delaySeconds` (15s, doubling per delivery attempt, capped at 300s) when retrying a transient PDS failure, giving propagation time without an unbounded backlog. The delay applies only to the transient PDS retry path; permanent failures still dead-letter immediately with no retry, and the other retry paths (label issuance, workflow dispatch, DID resolution) are unchanged.
…ication-trust fixes (review round 1) (#2116) * fix(labeler): honor RFC 8058 one-click unsubscribe query param A one-click unsubscribe POST carries the recipient hash (c) in the List-Unsubscribe header URL's query string and only List-Unsubscribe=One-Click in the body. The POST parser read c from the form body only, so a one-click request returned success without suppressing the recipient. Read c (and the confirm token) from the body first, falling back to the query. * fix(labeler): resolve package contacts for release-subject notices A release subject's rkey is slug:version, but contactTargetFromUri passed the whole rkey as the package slug, so getPackage missed the package and its security[]/authors[] contacts were skipped in favor of the publisher-profile fallback. Parse the parent package slug (before the version delimiter) so a release notice reaches the package's contacts; a package rkey and a malformed rkey both degrade safely. * fix(labeler): alert operators when a takedown notice has no contact An emergency takedown issuance that resolves no publisher contact recorded only a generic undeliverable notification row, so the signal that manual outreach is needed was easy to miss. Emit a dedicated takedown-no-contact operational event (severity high) plus its operator-alert outbox row on that path. A takedown retract and a takedown that does resolve a contact emit nothing, and a deduped replay does not re-emit. * fix(labeler): gate double-opt-in bypass on trusted, current verification A publisher's verification claim upgraded a contact past double opt-in on the strength of any in-force claim. Verification claims are self-assertable — the aggregator indexes any issuer, including self-issued ones — so a publisher could name a victim address in its profile, self-issue a claim, and have an unconfirmed notice sent there. Upgrade only on a claim whose issuer is in the configured trust set, that is unexpired, and whose bound displayName still matches the publisher's current identity. No issuer is trusted by default, so every address falls back to double opt-in. The publisher view carries a current displayName but no handle (the aggregator's identity-event ingestion is unbuilt), so the binding checks displayName only; handle-binding is deferred until the view carries a handle. * fix(labeler): make takedown no-contact alert recoverable The takedown-no-contact operator alert was emitted only on the first pass's send outcome, after the undeliverable notifications row had committed. A transient failure of that fire-and-forget event write lost the alert permanently: a deferred replay dedups on the existing notifications row and never reached the emit again. Key the alert on its own operational_events row instead, and run the check on every takedown issuance (including the dedup replay path), so a replay after a failed emit recovers the signal while a normal replay does not duplicate it. * fix(labeler): scope unsubscribe query fallback, keep confirm body-only The one-click query fallback was applied to the confirm and not-me POST parsers too, so a scanner POSTing the original confirmation link URL (with c and t in the query) could confirm an address without the user submitting the form. Restrict the query fallback to the RFC 8058 one-click unsubscribe path; confirm and not-me require their capability from the POST body. * fix(labeler): validate release rkey shape; make no-contact alert dedup atomic Contact resolution stripped a subject rkey at the first colon without checking the collection or the canonical release rkey shape, so a malformed colon-bearing subject could resolve a DIFFERENT package's contacts. Only strip a canonical release record (release collection + well-formed slug:version); anything else keeps its rkey and degrades to the publisher tier. The takedown-no-contact alert used an existence-check-then-insert with no uniqueness guard, so two concurrent replays could double-emit. Add a UNIQUE(action_id, event_type) index (migration 0012) and make the insert idempotent (ON CONFLICT DO NOTHING), with the outbox gated on the event being written; concurrency and replay now converge to exactly one event. * fix(labeler): scope dedup index to takedown-no-contact; require semver Migration 0012 created a GLOBAL unique index on (action_id, event_type), which would fail at migration time on any existing DB holding historical duplicate rows of other event types. Make it PARTIAL (WHERE event_type = 'takedown-no-contact') and point the insert's ON CONFLICT target at that predicate, so it stays safe on historical data while still deduping the alert. The release-rkey parse validated only the slug and a non-empty suffix, so 'gallery:not-semver' still resolved the gallery package. Validate the complete canonical shape (slug + percent-decoded semver version, mirroring the aggregator's parseReleaseRkey) before stripping; anything else degrades to the publisher tier.
…nst SSRF (review round 1) (#2117) * fix(aggregator): wrap unexpected XRPC errors and harden PDS egress against SSRF Finding 17 (Sol): a non-XRPC failure resolving the labeler policy (e.g. a D1 error) escaped `handleXrpc` unwrapped, hitting workerd's bare 500 without the CORS or `private, no-store` headers every aggregator response carries. Convert unexpected errors to a generic 500 through the same wrapper, logging the internal detail but never leaking it to the client. XRPCError handling is unchanged. SSRF follow-up: `pds-verify.ts` fetched the publisher-controlled PDS endpoint (from a DID document) following redirects with no scheme/DNS/reserved-address validation and no body-read deadline. Route it through the shared hardened egress (`resolveAndValidateExternalUrl` + injected `cloudflareDohResolver`): HTTPS-only, per-hop re-resolution under `redirect: "manual"` with a bounded redirect count, a wall-clock deadline spanning redirects and the body read, and fail-closed when no resolver is injected. Blocks are reported as a new permanent `PDS_ADDRESS_BLOCKED` reason; existing 404/5xx/too-large/invalid-proof and the transient network classification are preserved. * fix(aggregator): retry PDS verification on resolver-infra failures, don't dead-letter `assertFetchableUrl` collapsed every `SsrfError` from `resolveAndValidateExternalUrl` into the permanent `PDS_ADDRESS_BLOCKED`, but that helper also throws `SsrfError` when the injected DNS resolver itself fails (DoH network error / SERVFAIL / timeout) — transient infrastructure, not a disallowed publisher address. A DoH blip therefore permanently dead-lettered legitimate records and risked mass dead-lettering during a resolver outage. `SsrfError.code` is the same constant for both cases, so key on the message prefix the helper uses only for the resolver-threw path (`Could not resolve hostname:`): map that to the transient `PDS_NETWORK_ERROR` (retried), and keep genuine address blocks — private/reserved IP, blocked host, scheme — as the permanent `PDS_ADDRESS_BLOCKED`. A test asserts the transient mapping so a future wording change in the shared core helper breaks loudly rather than silently mis-classifying. Also adds a redirect-hop-limit boundary test. * fix(aggregator): retry PDS verification on empty DNS answer, don't dead-letter An empty resolver answer (NXDOMAIN / NOERROR-NODATA / CNAME-only) makes `resolveAndValidateExternalUrl` throw `SsrfError("Hostname resolved to no addresses")`, which the classifier mapped to the permanent PDS_ADDRESS_BLOCKED — so a host mid-DNS-propagation lost the record to a dead-letter. Treat the empty-answer message as a transient resolution failure (PDS_NETWORK_ERROR, retried) alongside the existing resolver-threw case; a genuinely-gone host still dead-letters via retry exhaustion. Genuine private/reserved/blocked-host addresses stay permanent. Keyed on the shared helper's message as a module constant with a test, matching the resolver-failure-prefix approach. * fix(aggregator): delay re-delivery on transient DNS-resolution failure The records queue re-delivers immediately (no `retry_delay`), so classifying an empty DNS answer / resolver outage as transient only helped if the retry waited for propagation — otherwise all `max_retries` (5) burn in seconds and the record DLQs as UNEXPECTED_ERROR before DNS settles. `PdsVerificationError` now carries an optional `retryAfterSeconds`, set (to 60s) only on the transient DNS-resolution path; the consumer passes it as `retry({ delaySeconds })`. Other transient retries (network, 5xx, timeout) stay immediate. Delay strategy: fixed 60s. With max_retries: 5 that spreads retries across a ~5-minute propagation window. Chosen for reconciliation with labeler #2113.
…w round 1) (#2115) * fix(labeler): redrive the subscription-DO notify on console mutation replay A transient afterCommit failure after a label commits leaves the row publication_pending=1 and drops the live broadcast; the guard replay branch did not re-drive the notify, so a client retry never recovered it. The label-issue and emergency replay branches now re-fire the post-commit notify (keyed on the committed action), giving retries a cheap path back to a live broadcast. The reconciliation publication-pending sweep remains the durable backstop. Refs Sol finding 7. * fix(labeler): re-sign retired-key labels on WebSocket subscription replay After a routine key rotation the DID document publishes only the new key, but retained rows keep their old-key signatures; queryLabels re-signs lazily on read while the subscribeLabels replay path sent rows unchanged, so a fresh aggregator rejected the first old-key frame and could not advance. The replay reader now reuses queryLabels' lazy re-sign helper (exported from query-labels), bringing stale-key rows onto the active key before framing and persisting the result so the work is not repeated per connection. Sequence and ordering are preserved; a signing pause throws rather than serving an unverifiable frame. Refs Sol finding 5. * fix(labeler): publish automated assessment labels live, with a reconciliation backstop Automated pending/outcome/block/negation labels committed without ever notifying the subscription DO, so a connected aggregator received nothing until it reconnected or an unrelated notification arrived. The orchestrator now issues finalization labels publication_pending=1 and broadcasts each to the same subscription-DO publisher the console path uses, off the commit. A new publication-pending sweep in the reconciliation cron re-drives any notify that was dropped (assessment or console path alike) — the durable guarantee that a stranded row reaches subscribers and stops blocking the next key rotation. Refs Sol findings 4 and 7. * fix(labeler): guard the finalization CAS on signing state so a rotation pause can't strand a run If signing paused between finalization prep and the batch commit, the guarded label INSERTs no-oped while the unguarded assessment CAS still committed terminal state — leaving the run terminal with its pending-negation, outcome, and block labels missing, and the Workflow retry seeing terminal and never repairing them. The CAS now shares the same signing-state predicate as the issuance statements, so the whole db.batch is all-or-nothing: a mid-batch pause no-ops the CAS too, the run stays running, and the retry re-runs finalization after signing resumes. Refs Sol finding 6. * perf(labeler): index issued_labels for the publication-pending sweep The reconciliation sweep filtered issued_labels by publication_pending ordered by sequence with no covering index, full-scanning a monotonically growing table every cron tick — a D1 query-timeout there would strand pending rows and block the very rotation drain the sweep exists to unblock. Adds migration 0011 with a partial covering index on (sequence, cts) WHERE publication_pending = 1, and a query-plan guard so the sweep can't silently regress to a full scan. Follow-up to emdashbot review on #2115. * docs(labeler): drop the stale mid-batch signing-flip gap from finalize The F6 fix guards the finalization CAS on the same signing-state predicate as the label issuances, so the race-analysis comment's first bullet (claiming the CAS is unguarded and a flip could commit terminal state with labels suppressed) is no longer true. Replaces it with the now-closed description and keeps the two genuinely-remaining narrower gaps. Follow-up to emdashbot review on #2115. * fix(labeler): publish discovery-consumer labels live, with the sweep backstop The #4 fix gave the orchestrator path live publication but missed the discovery-consumer path: its initial assessment-pending issuance and its deletion negations committed publication_pending=0 with no subscription-DO broadcast, so connected subscribers never received them and the new reconciliation sweep (pending=1 only) could not recover them. Both issuances now take the same subscription-DO publisher the orchestrator uses — publication_pending=1 on commit plus a best-effort post-commit notify (a dropped broadcast never fails the discovery message; the sweep re-drives it). The rotation-drain contract is unchanged: these pending rows drain exactly like the orchestrator's. Follow-up to Sol review on #2115. * fix(labeler): negate a deleted run's pending label before cancelling it The delete path cancelled pending/running runs to terminal 'cancelled' before issuing their assessment-pending negations. If issuance was paused (signing rotation) the negation threw and the message retried, but on redelivery listNonTerminalAssessmentsForUri excluded the already-cancelled runs — so there was nothing left to negate, the message acked, and the active assessment-pending label survived a deleted release forever. Each run's negation is now issued before its terminal cancellation, so a paused/failed negation leaves the run non-terminal and re-discoverable on redelivery; a run is retired only after its negation commits, and the message cannot ack while any pending/running run is un-negated. No active assessment-pending survives an acked delete. Follow-up to Sol review on #2115. * fix(labeler): guard the finalization CAS on the subject not being tombstoned Finalization's isSubjectCurrent re-check is a separate read from its commit CAS, a TOCTOU: a delete that tombstoned the subject in between still let the CAS commit 'running -> outcome' plus positive outcome/block labels, then the delete's own cancel CAS conflicted and was swallowed — leaving live block labels on a deleted release the evaluator would still honor. The finalization CAS now carries a not-deleted predicate on the run's subject (same all-or-nothing batch idiom as the F6 signing guard), so a tombstone landing before the batch no-ops the CAS and every label gated on the outcome state; finalization retries and stales the run out, never labelling a deleted subject. Follow-up to Sol round-3 review on #2115. * fix(labeler): retry (never dead-letter+ack) a delete-path mutation failure The 'a negation throw prevents ack' invariant only held for recognized issuance-unavailable errors; an unexpected signing/D1 failure during the delete's tombstone/negate/cancel fell through to dead-letter+ack (the create-path policy), leaving the run non-terminal with its assessment-pending label live on a deleted subject forever. The delete handler now splits the absence-verification phase (classified like create: transient retries, forged/permanent dead-letters) from the mutation phase, which ALWAYS retries on failure so the label can never be left live on an acked delete. A genuinely permanent fault exhausts to the DLQ via max_retries — acceptable versus acking a stranded label. Follow-up to Sol round-3 review on #2115. * docs(labeler): correct the delete-mutation retry reference post-merge After merging the integration branch (which added #2113's transient-DNS retry backoff), the applyDiscoveryDelete docstring still said a failed negation propagates to classifyDiscoveryError. Since the Blocker-2 fix, the delete mutation phase has its own always-retry catch — update the comment to match. No behavior change. * fix(labeler): guard the initial pending-label issuance against a concurrent delete The create path committed a recreated run as pending and THEN issued its positive assessment-pending with no commit-time guard. A concurrent delete could, in the gap, tombstone the subject, negate + cancel the (still-positive-less) run, and ack; the create then committed its positive at a higher sequence, winning label reduction — a live pending label on a tombstoned, cancelled subject. (Queue invocations overlap; max_concurrency is unset.) The initial issuance now goes through buildIssuanceStatements gated atomically on BOTH the run still being pending AND the exact (uri,cid) subject still undeleted (the same guard idiom as Blocker 1's finalization CAS). On a guard miss the label no-ops: the issuance is obsolete, so the create neither publishes nor dispatches a Workflow for a label that never committed; a non-persist not explained by the guard (a signing flip) still retries. With this and Blocker 1, every automated- positive issuance site in the discovery consumer is now delete-guarded (the delete-path negation is negative and exempt). Follow-up to Sol round-4 review on #2115. * fix(labeler): guard the operator-rerun pending label against a concurrent delete The operator rerun issued its positive assessment-pending with no commit-time guard and while its run was still 'observed' (advance to 'pending' is deferred), opening two integrity races with a concurrent discovery-delete (queue + HTTP overlap; max_concurrency unset): (a) ordering: the rerun commits its run+pending after a delete tombstoned and snapshotted the non-terminal runs, so the delete never negates it and the positive survives on the tombstoned subject; (b) observed gap: the delete-path negation only fired for pending/running runs, so a delete that saw the rerun's 'observed' run cancelled it WITHOUT negating its already-live positive. Fix (a): gate the rerun's assessment-pending issuance on requireSubjectNotDeleted + requireAssessmentState (threaded through prepareAutomatedLabelIssuance); on a miss the label no-ops, assertIssuancePersisted aborts before the deferred tail, so nothing is published, dispatched, or advanced. Fix (b): the delete-path negation now fires for any non-terminal run that committed a positive pending (keyed on the committed positive, not lifecycle state) — no dangling negations, no interleaving where a cancelled/deleted run keeps a live positive. With Blocker 1 and the round-4 discovery guard, every positive assessment-pending issuance across the discovery and console paths is now delete-safe. Follow-up to adversary review on #2115. * fix(labeler): structural delete-generation guard closing the create/verify/rerun-vs-delete race class Adds a monotonic delete_generation to subjects (migration 0013). Every delete increments it; create/verify/rerun capture it before reading state/verifying and CAS-guard their subject-undelete, run creation, and label issuance on the generation not having advanced. A decision made before a delete is rejected obsolete; one that captured the post-delete generation still works (republish). Closes three race seams: (1) createSubject un-delete is generation-gated so a stale verify can't resurrect a deleted subject; (2) the delete cleanup now scans pending-bearing runs INCLUDING terminal stale and negates any committed positive; (3) the console rerun's run creation is generation-gated so no orphan observed run. Implementation checkpoint; barrier tests for each seam + a republish test follow. Refs Sol round-5 (systemic close). * test(labeler): barrier tests for the delete-generation close (three seams + republish) Seam 1: a stale verify (barrier during verify, full delete completes) cannot resurrect the tombstoned subject — createSubject's generation-guarded undelete no-ops, no run, no positive, no dispatch. Seam 2: the delete negates a terminal stale run's stranded positive (widened scan). Seam 3: a rerun after a concurrent tombstone leaves no orphan observed operator run. Plus a delete-then-republish test proving a new revision after a delete assesses cleanly (the generation does not over-block). Each verified failing pre-fix. Refs Sol round-5 (systemic close).
…try-moderation/verification packages The new (unpublished) registry-moderation and registry-verification packages accumulated a pile of per-change changesets; since they're initial releases, collapse them into a single initial-release changeset (net bump unchanged: both minor -> 0.1.0). Also drops two empty changesets. Changes to already- published packages (registry-lexicons, registry-client, emdash, admin, auth, plugin-types, plugin-cli) keep their individual changesets.
This was referenced Jul 21, 2026
…elope (#2167) * fix(aggregator): wrap syncGetRecord errors in the CORS + no-store envelope An unexpected error (e.g. a D1 failure in the record-blob fetch) escaped handleXrpc unwrapped, hitting workerd's bare 500 with no CORS or cache headers — the parallel gap #2117 left out of scope on the cacheable record-blob path. Extract the shared no-store+CORS wrapper (wrapDispatchError) so the sync and policy error paths stay in lockstep, add a context string for correct log attribution, and route syncGetRecord throws through it. The success 200 keeps its cacheable header; only errors get private, no-store. * docs(aggregator): drop redundant catch comment (wrapDispatchError docstring covers it)
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
emdash-bot | fa4d25b | Jul 21 2026, 12:09 PM |
…y, pointer tie-break (held review findings) (#2166) * fix(labeler): tie-break current-pointer update on assessment id The current-pointer upsert guard accepted any run at an equal created_at_epoch_ms, but list/current ordering is (created_at_epoch_ms, id). A later lower-id run could displace a higher-id current sharing a millisecond. Match the pointer guard to the ordering so a lower-id run cannot overwrite a higher-id current at an equal epoch. * fix(labeler): enforce automation kill-switch on in-flight runs An admin pausing automation only halted discovery; already-dispatched Workflow runs never re-read automation_state and finalization had no pause predicate, so in-flight runs kept issuing automated labels after a successful pause. Re-read the kill-switch on Workflow entry (halting before AI/network work) and fold a fail-closed automation-not-paused predicate into the finalization CAS. A pause landing mid-flight no-ops the whole batch, leaving the run running for the Workflow retry to resume after unpause -- matching the signing-pause precedent. * fix(labeler): make dead-letter retry durable before marking retried The dead-letter retry committed status='retried' before decoding the payload and delivering the re-drive to the queue. A failed or undecodable delivery stranded the row 'retried': health checks (which count only 'new') stopped seeing it and a later operator retry returned 409, with nothing ever re-driven. Enqueue the discovery job before committing 'retried', so the status flips only once the queue accepts the job. An undecodable payload (422) or a queue failure (503) leaves the row 'new' -- still counted and retryable. A duplicate enqueue under concurrent double-retry is absorbed by the consumer's runKey dedup. * docs(labeler): correct sendDiscoveryJob comment — awaited before commit, not deferred
18 tasks
…run resolves (#2168) Concurrent assessment runs for the same (uri,cid) all target the single assessment-pending label, so one run's finalization negation cleared the shared gate while a sibling run was still active — briefly exposing a stale pass. Gate the pending negation, in-batch, on no other non-terminal run for the subject (requireNoOtherActiveRun), so only the last run to resolve clears it. Make the delete cleanup's pending scan label-driven (any run holding a committed un-negated positive, at any state) so a decision run that now finalizes while still holding a positive is still negated on delete.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Establishes
feat/plugin-registry-labelling-serviceas the integration branch for the plugin registry labelling service described in RFC #694.This PR tracks the complete implementation without sub-issues. Implementation PRs should target
feat/plugin-registry-labelling-service; this umbrella PR remains draft until the production launch gates in the implementation plan are complete.The tracked planning documents are:
.opencode/plans/plugin-registry-labelling-service/spec.md.opencode/plans/plugin-registry-labelling-service/implementation-plan.mdRelated to #694.
Implementation tracker
Design and feasibility
security:yankedtosecurity-yankedcutover (docs: audit registry label vocabulary #1912)security:yankedproduction preflight (zero legacy rows confirmed; Branch A, no compatibility path)Shared protocol and policy
packages/registry-moderationand consumed by core/admin/registry-client/aggregator; a full-tree audit finds zero functionalsecurity:yankedcolon-form values remaining — only a stale comment in the already-applied historical migrationaggregator/migrations/0001_init.sql, left untouched for migration integrity; preflight confirmed zero persisted legacy rows so no dual-read compatibility path was needed — Branch A)Service foundation and signed distribution
apps/labeler, bindings, CI coverage, and real workerd/D1 tests (feat(registry): add labeler service core #1926)com.atproto.label.queryLabels(feat(registry): add labeler service core #1926)com.atproto.label.subscribeLabels(feat(registry): add label subscriptions #1931)NotSupportedfor deferred standard report intake (feat(registry): add labeler key rotation #1938)Aggregator and client enforcement
atproto-content-labelers(feat(registry): label-aware aggregator reads #1965)Automated assessment
registry-verification; W7 extends it from main)deterministic/dependencyorchestrator stages are removed (feat(labeler): code-AI and history stage wiring + dead-stage removal (slice A) #2090). Dissolves the scanner/SBOM-tooling selection gate — nothing to select)Operator and publisher surfaces
getPublisherread, with confirm/unsubscribe/not-me token endpoints in feat(labeler): publisher-notification token endpoints (W10.4 slice C) #2071 (W10.4 slice C))Production readiness
model_id/prompt_hashprovenance) and the operator-owned Cloudflare Email Sending onboarding + staging/prod provisioning)Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs; a workflow extracts catalogs on merge tomain.RFC #694 is the approved design source. Individual implementation PRs carry their applicable changesets and verification evidence. No admin translation catalogs are included.
AI-generated code disclosure
Screenshots / test output
Integrated gate on
2d6ad448after #1952 merged:pnpm buildpassespnpm typecheckand aggregator typecheck passTry 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/plugin-registry-labelling-service. Updated automatically when the playground redeploys.