Skip to content

feat(release-service): add passkey approvals - #2745

Open
ascorbic wants to merge 3 commits into
feat/drs-review-02-authority-admissionfrom
feat/drs-review-03-approvals
Open

feat(release-service): add passkey approvals#2745
ascorbic wants to merge 3 commits into
feat/drs-review-02-authority-admissionfrom
feat/drs-review-03-approvals

Conversation

@ascorbic

Copy link
Copy Markdown
Collaborator

What does this PR do?

Adds required-user-verification passkey primitives, typed challenge contexts, approver-sharded Durable Object storage, digest-bound approval and rejection, credential counters and revocation, replay protection, and current signed-profile membership checks.

This is PR 3 of 7 and depends on publisher authority and admission. The stack merges as one unit and this branch is not deployable by itself.

Discussion: #1590

Type of change

  • Bug fix
  • Feature (requires maintainer-approved Discussion)
  • Refactor (no behavior change)
  • Translation
  • Documentation
  • Performance improvement
  • Tests
  • Chore (dependencies, CI, tooling)

Checklist

  • I have read CONTRIBUTING.md
  • pnpm typecheck passes
  • pnpm lint passes
  • pnpm test passes (or targeted tests for my change)
  • pnpm format has been run
  • I have added/updated tests for my changes (if applicable)
  • User-visible strings in the admin UI are wrapped for translation (not applicable in this layer)
  • I have added and reviewed the user-facing changeset for @emdash-cms/auth
  • New features link to an approved Discussion: RFC: Attested Automated Publishing #1590

AI-generated code disclosure

  • This PR includes AI-generated code — model/tool: OpenAI Codex (GPT-5)

Screenshots / test output

The auth package and approver, passkey, approval-digest, invalidation, decision-route, and session suites pass.

@changeset-bot

changeset-bot Bot commented Aug 27, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 4e43b4d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 17 packages
Name Type
@emdash-cms/auth Minor
emdash Minor
@emdash-cms/auth-atproto Patch
@emdash-cms/release-service Patch
@emdash-cms/cloudflare Minor
@emdash-cms/sandbox-workerd Patch
@emdash-cms/fixture-perf-site Patch
@emdash-cms/perf-demo-site Patch
@emdash-cms/cache-demo-site Patch
@emdash-cms/do-demo-site Patch
@emdash-cms/do-solo-demo-site Patch
@emdash-cms/admin Minor
@emdash-cms/blocks Minor
@emdash-cms/gutenberg-to-portable-text Minor
@emdash-cms/x402 Minor
create-emdash Minor
@emdash-cms/plugin-embeds Patch

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

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 27, 2026

Copy link
Copy Markdown

🚀 Deploying Preview to Cloudflare 🚀

Preview URL: https://feat-drs-review-03-approvals.try.emdashcms.com, https://feat-drs-review-03-approvals-emdash-playground.emdash-cms.workers.dev (commit 4e43b4d)

This URL reflects your latest Preview deployment

Preview Deployments by commit

Status Deployment URL Commit Updated (UTC) See this deployment's details
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://bb77a23b.try.emdashcms.com, https://bb77a23b-emdash-playground.emdash-cms.workers.dev 4e43b4d 2026-08-28T23:22:22.754Z Visit the dashboard ↗
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://8a75de58.try.emdashcms.com, https://8a75de58-emdash-playground.emdash-cms.workers.dev c7e14b7 2026-08-28T21:31:18.865Z Visit the dashboard ↗
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://77bf4e5b.try.emdashcms.com, https://77bf4e5b-emdash-playground.emdash-cms.workers.dev e906805 2026-08-28T15:16:29.460Z Visit the dashboard ↗
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://541f143e.try.emdashcms.com, https://541f143e-emdash-playground.emdash-cms.workers.dev 12f6af8 2026-08-28T11:19:28.943Z Visit the dashboard ↗
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://363d0bc0.try.emdashcms.com, https://363d0bc0-emdash-playground.emdash-cms.workers.dev 4f5c3a7 2026-08-28T10:24:27.679Z Visit the dashboard ↗
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://3b1c5b66.try.emdashcms.com, https://3b1c5b66-emdash-playground.emdash-cms.workers.dev d0647ee 2026-08-27T15:09:59.185Z Visit the dashboard ↗

@github-actions

Copy link
Copy Markdown
Contributor

Scope check

This PR changes 6,816 lines across 41 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.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 27, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
docs 4e43b4d Aug 28 2026, 11:11 PM

@github-actions

Copy link
Copy Markdown
Contributor

Overlapping PRs

This PR modifies files that are also changed by other open PRs:

This may cause merge conflicts or duplicated work. A maintainer will coordinate.

@pkg-pr-new

pkg-pr-new Bot commented Aug 28, 2026

Copy link
Copy Markdown

Open in StackBlitz

@emdash-cms/admin

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/admin@2745

@emdash-cms/auth

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/auth@2745

@emdash-cms/auth-atproto

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/auth-atproto@2745

@emdash-cms/blocks

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/blocks@2745

@emdash-cms/cloudflare

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/cloudflare@2745

@emdash-cms/contentful-to-portable-text

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/contentful-to-portable-text@2745

emdash

npm i https://pkg.pr.new/emdash-cms/emdash@2745

create-emdash

npm i https://pkg.pr.new/emdash-cms/emdash/create-emdash@2745

@emdash-cms/gutenberg-to-portable-text

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/gutenberg-to-portable-text@2745

@emdash-cms/plugin-cli

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-cli@2745

@emdash-cms/plugin-types

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-types@2745

@emdash-cms/registry-client

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/registry-client@2745

@emdash-cms/registry-lexicons

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/registry-lexicons@2745

@emdash-cms/registry-moderation

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/registry-moderation@2745

@emdash-cms/registry-verification

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/registry-verification@2745

@emdash-cms/sandbox-workerd

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/sandbox-workerd@2745

@emdash-cms/x402

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/x402@2745

@emdash-cms/plugin-ai-moderation

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-ai-moderation@2745

@emdash-cms/plugin-atproto

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-atproto@2745

@emdash-cms/plugin-audit-log

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-audit-log@2745

@emdash-cms/plugin-color

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-color@2745

@emdash-cms/plugin-embeds

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-embeds@2745

@emdash-cms/plugin-field-kit

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-field-kit@2745

@emdash-cms/plugin-forms

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-forms@2745

@emdash-cms/plugin-webhook-notifier

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-webhook-notifier@2745

commit: 4e43b4d

@ascorbic
ascorbic force-pushed the feat/drs-review-03-approvals branch from 4f5c3a7 to 12f6af8 Compare August 28, 2026 11:15
@ascorbic

Copy link
Copy Markdown
Collaborator Author

🔍 Adversarial review — stack #2766, layer 4 of 8

Automated deep review of this layer's diff (feat/drs-review-02-authority-admission...feat/drs-review-03-approvals). Findings ranked most-severe first; confidence is CONFIRMED (full code path read) or PLAUSIBLE (strong suspicion, path partially read). Nothing was auto-fixed.

What the PR actually does + verdict

The PR adds a passkey-approval realm to the release service: an approver identity OAuth flow that reuses the confidential-client custody plumbing with a new approver_identity purpose and stores encrypted OAuth state in a new approver-sharded Durable Object (ApproverDurableObject, keyed by approver DID via getByName); cookie-based approver sessions (__Host- cookies, hashed tokens, session epochs, CSRF via origin check + custom header + session-bound double-submit hash); credential enrolment/list/revoke routes; and digest-bound approve/reject decisions. In @emdash-cms/auth it adds an opt-in userVerification config (enforced from authenticatorData flags, not just requested), typed versioned challenge contexts whose SHA-256 digest is embedded in the challenge itself, an AtomicChallengeStore consume path, and a tightened counter rule (nonzero→zero now rejected). The service pins userVerification: "required", binds each approval challenge to (approver DID, publisher DID, intent ULID, evidence digest, approval digest, decision) both in DO row metadata and in the signed challenge context, consumes challenges atomically before verification, CAS-commits counter updates and decision receipts in one DO transaction with idempotency-key replay, and applies the decision to the publisher DO via generation-guarded transition carrying the approval digest. Membership in the publisher's package-profile approver list is checked at challenge issuance and re-checked twice at consumption, with the profile CID pinned to the admission-time value. Overall quality is high — the WebAuthn ceremony is correct (type, exact origin, rpIdHash, UP, UV, stored-algorithm signature over authData || sha256(clientDataJSON), atomic single-use challenges, fail-closed ordering), the DO stores are transactional with strict input validation, and tests use real ES256 signatures rather than stubs. The findings below are mostly at the trust-boundary and operational edges, not in the core ceremony.

Findings

  • [medium] apps/release-service/src/approvals/authority.ts:337-366 — the "current signed-profile membership" check trusts the PDS's self-reported cid string from com.atproto.repo.getRecord; the returned record value is never cryptographically verified against that CID (fetchVerifiedResource in packages/registry-verification/src/fetch.ts does SSRF/limit guarding only, no digest/CID verification, unlike the blob path which uses multihashFromBlobCid). Failure scenario: a compromised or malicious PDS (the publisher's hosting provider) serves a modified releasePolicy.approvers list containing an attacker DID while echoing the pinned evidence.profileCid, defeating both the PROFILE_CHANGED pin and the membership check — the exact tampering the CID pin exists to catch. Confidence: CONFIRMED that no verification occurs; PLAUSIBLE exploitation (requires PDS-side compromise, which is inside this system's stated threat model of independent approver authority).
  • [medium] apps/release-service/src/oauth/routes.ts (handleApproverIdentityAuthorize) + approver-do/store.ts (MAX_ACTIVE_IDENTITY_TRANSACTIONS = 20) — unauthenticated, targeted login lockout: anyone who scripts Origin/X-EmDash-Request headers can start OAuth flows naming a victim approver's DID; each fills one of the 20 identity_transactions slots (≤10 min each, trivially replenished), after which the victim's own authorize attempts fail with IDENTITY_TRANSACTION_LIMIT_REACHEDOAUTH_AUTHORIZATION_FAILED. Existing sessions survive, but a sustained loop blocks new approver logins — relevant because approvals are time-boxed (intent.expiresAt), so blocking login can run out the approval clock. The publisher flow from layer 02 shares the pattern; this PR newly exposes it for the approver realm. CONFIRMED mechanism.
  • [low] apps/release-service/src/approvals/passkeys.ts (DurableApproverChallengeStore.set) vs approver-do/store.ts #validChallenge — the auth package computes expiresAt = workerNow + 5min while the DO validates expiresAt - doNow > 5min with zero tolerance against its own clock; any case where the DO's clock trails the worker's snapshot makes createChallenge throw ApproverStoreError, which mapApprovalError in decision-routes.ts does not handle (unlike mapApproverError in approvals/routes.ts) — it re-throws to the top-level catch as a 500. Intermittent, environment-dependent begin-decision failures. PLAUSIBLE.
  • [low] packages/auth/src/passkey/authenticate.ts / register.ts (isAtomicChallengeStore) — atomic mode is duck-typed on the presence of a consume method. Any existing consumer of this published package whose custom ChallengeStore happens to expose a consume member silently changes behavior on upgrade: get/delete are no longer called and consume(challenge) is invoked with whatever semantics that method has. Backwards-compatibility hazard in a published API that an explicit option flag would avoid. CONFIRMED behavior, PLAUSIBLE impact.
  • [low] packages/core/src/auth/challenge-store.ts changed (published package emdash) with no changeset — the PR's only changeset covers @emdash-cms/auth, violating the stated repo rule "changeset added if a published package changed". The change is small (persist ChallengeData.context in the data column) but it is a schema-usage/behavior change in a published package. CONFIRMED. Note also the change is currently inert-by-construction: core's store is non-atomic, and context-bound challenges refuse non-atomic stores at verification, so a context stored via core's store can never verify.
  • [low] apps/release-service/wrangler.jsoncApproverDurableObject was added to the already-existing v1 migration tag instead of a new v2 new_sqlite_classes entry. Any environment already deployed from layers 01–02 has v1 applied; the edited tag won't re-run, so the new class is never migrated there and deploys fail or the binding is unusable. Harmless only if no environment deploys before the stack merges. CONFIRMED config risk.
  • [low] apps/release-service/src/approver-do/store.ts (commitVerifiedDecision, commitCredentialUse) — a detected COUNTER_REGRESSION (the canonical cloned-authenticator signal) returns a generic 409 and writes no audit event; the same is true when @emdash-cms/auth throws invalid_signature_counter. For a system that otherwise audits every challenge/credential event, the single highest-signal security event is invisible. Behavior is safe (decision rejected, challenge burned) but undetectable. CONFIRMED.
  • [info] packages/auth/src/passkey/register.ts — attestation is "none" and any other attestation format is explicitly ignored ("we just ignore it and trust the credential"). This is a commented, pre-existing decision in the base package; it means enrolment authenticity for release approvers rests entirely on the approver session + UV, which is defensible but worth stating in the PR rather than inheriting silently.

Things specifically hunted and confirmed clean: challenge double-spend (atomic consumeChallenge marks consumed_at inside transactionSync before any verification; burn-on-mismatch), cross-context and cross-approver challenge use (kind check, per-shard storage, six-field sameApprovalContext including approver DID and decision, digest-of-context embedded in the signed challenge), UV actually verified from flags (tested with 0x01 vs 0x05), algorithm taken from the stored credential (no attacker-chosen alg), counter CAS with CREDENTIAL_STATE_CHANGED on interleaving and zero-counter authenticators still allowed, revocation immediate in the only shard that holds the credential plus invalidation of all outstanding challenges, revoked credential IDs permanently unreusable (CREDENTIAL_EXISTS includes revoked rows), membership checked at consumption (twice) not just issuance, rejection digest-bound identically to approval, the recovery/replay path unable to mint any decision that wasn't previously passkey-verified with exact identity match, and evidence decode requiring exact canonical JSON round-trip plus digest recomputation. The ctx.id.name alarm concern is resolved by the runtime as of March 2026 (name now persisted for getByName objects; historical issue workerd#2240).

PR description vs code

  • "Current signed-profile membership checks" overstates the code: nothing validates a signature or verifies the record against its CID; membership is checked against a PDS-served JSON record pinned only by a PDS-asserted CID string (finding 1). Every other claim (required-UV primitives, typed contexts, approver-sharded DO storage, digest-bound approve/reject, counters, revocation, replay protection, generation-bound application) matches the code as verified above.
  • The changeset claim is accurate for @emdash-cms/auth but the PR also changes the published emdash core package without one.

Test-coverage gaps

  • No cross-shard replay test: an assertion/challenge minted for approver A replayed via approver B's routes (protection is by construction; a route-level test would pin it).
  • The recovery path — decision committed in the approver DO but publisher transition never applied (crash window) — is untested; the replay test only covers replay after a successful transition.
  • completeApprovalDecision outcomes CREDENTIAL_STATE_CHANGED/COUNTER_REGRESSION/DECISION_CONFLICT are tested only at the store layer (commitCredentialUse), not through the passkey completion path or routes.
  • No RS256 (−257) approval ceremony test in the release service (ES256 only; RS256 covered only in the auth package's generic tests).
  • invalidateApprovalChallenges has no production caller at this layer (presumably wired by a later layer); its test exercises the helper directly, so nothing proves intents actually invalidate challenges on cancellation/expiry end-to-end at this layer.
  • No test for the challenge-TTL boundary (expiresAt - now == MAX_CHALLENGE_MS) or for mapApprovalError's handling of ApproverStoreError from the begin path (currently a 500).

~ 🤖 Fable

@ascorbic
ascorbic force-pushed the feat/drs-review-03-approvals branch from 12f6af8 to e906805 Compare August 28, 2026 15:11
@ascorbic
ascorbic force-pushed the feat/drs-review-03-approvals branch from e906805 to 29b62c5 Compare August 28, 2026 21:15
@ascorbic
ascorbic force-pushed the feat/drs-review-03-approvals branch from 29b62c5 to c7e14b7 Compare August 28, 2026 21:19
@ascorbic
ascorbic force-pushed the feat/drs-review-03-approvals branch from c7e14b7 to 4e43b4d Compare August 28, 2026 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant