From 4abbfefb49b748e9d8365461137b9b9a0dd4dfa7 Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Tue, 1 Sep 2026 11:30:06 +0100 Subject: [PATCH] fix(oauth-provider): resolve space type declarations with proof intact MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `resolveSpaceDeclaration` routed through @atcute/lexicon-resolver, whose final step validates the fetched lexicon document against @atcute/lexicon-doc. That library's def-type whitelist has no `type: "space"`, so a correctly published space declaration threw InvalidLexiconSchemaError (invalid_literal at .defs.main.type) and the consent UI showed "could not resolve space type declaration" for every space scope. The space path now replicates the resolver's authenticated steps in-process — DID document -> PDS endpoint, fetch the record proof CAR, and verify the commit signature via @atcute/repo — then reads defs.main off the proof-verified record and validates it against the local LexiconSpace shape. Full proof verification is preserved; it never falls back to an unauthenticated getRecord. The permission-set `resolve` path is unchanged. A block comment documents the workaround and its removal condition (upstream lexicon-doc gaining the `space` def type). Adds @atcute/crypto and @atcute/repo as runtime deps (they dedupe to the instances the resolver already uses) and @atproto/repo + @atproto/crypto as devDeps for building real signed proof-CAR fixtures. New tests prove a valid space declaration resolves and that wrong-key / tampered records still fail. --- .changeset/space-declaration-resolution.md | 5 + packages/oauth-provider/package.json | 4 + .../oauth-provider/src/permission-sets.ts | 135 +++++++++- .../test/permission-sets.test.ts | 233 ++++++++++++++++++ plans/in-progress/spaces.md | 9 + pnpm-lock.yaml | 12 + 6 files changed, 394 insertions(+), 4 deletions(-) create mode 100644 .changeset/space-declaration-resolution.md create mode 100644 packages/oauth-provider/test/permission-sets.test.ts diff --git a/.changeset/space-declaration-resolution.md b/.changeset/space-declaration-resolution.md new file mode 100644 index 0000000..ae6295e --- /dev/null +++ b/.changeset/space-declaration-resolution.md @@ -0,0 +1,5 @@ +--- +"@getcirrus/oauth-provider": patch +--- + +Fix `space:` scope resolution in the consent UI. `@atcute/lexicon-resolver` validates fetched lexicon documents against `@atcute/lexicon-doc`, whose def-type whitelist has no `type: "space"`, so a correctly published space type declaration failed with `InvalidLexiconSchemaError` and the consent screen showed "could not resolve space type declaration" for every space scope. `resolveSpaceDeclaration` now replicates the resolver's authenticated steps itself — resolve the DID document, fetch the record proof CAR, and verify the commit signature — then validates `defs.main` against the local space shape, keeping full proof verification while tolerating the unknown def type. The workaround can be removed once upstream `@atcute/lexicon-doc` gains the `space` def type. diff --git a/packages/oauth-provider/package.json b/packages/oauth-provider/package.json index be55d84..6fa0c67 100644 --- a/packages/oauth-provider/package.json +++ b/packages/oauth-provider/package.json @@ -20,10 +20,12 @@ "check": "publint && attw --pack --ignore-rules=cjs-resolves-to-esm" }, "dependencies": { + "@atcute/crypto": "^2.3.0", "@atcute/identity": "^1.1.0", "@atcute/identity-resolver": "^1.1.3", "@atcute/lexicon-resolver": "^0.1.6", "@atcute/lexicons": "^1.2.6", + "@atcute/repo": "^0.1.1", "@atproto/oauth-scopes": "0.0.0-spaces-alpha-20260818163953", "@atproto/oauth-types": "^0.6.3", "@atproto/syntax": "^0.4.2", @@ -31,6 +33,8 @@ }, "devDependencies": { "@arethetypeswrong/cli": "^0.18.2", + "@atproto/crypto": "^0.4.5", + "@atproto/repo": "^0.8.12", "@cloudflare/workers-types": "^4.20251225.0", "publint": "^0.3.16", "tsdown": "^0.18.3", diff --git a/packages/oauth-provider/src/permission-sets.ts b/packages/oauth-provider/src/permission-sets.ts index 87a7df9..efd6c51 100644 --- a/packages/oauth-provider/src/permission-sets.ts +++ b/packages/oauth-provider/src/permission-sets.ts @@ -13,16 +13,30 @@ * `@atcute/lexicon-resolver` (DNS-based authority + AT-URI schema fetch). */ +import { + getPublicKeyFromDidController, + P256PublicKey, + Secp256k1PublicKey, + type PublicKey, +} from "@atcute/crypto"; +import { + getAtprotoVerificationMaterial, + getPdsEndpoint, +} from "@atcute/identity"; +import type { DidDocumentResolver } from "@atcute/identity-resolver"; import { DohJsonLexiconAuthorityResolver, LexiconSchemaResolver, } from "@atcute/lexicon-resolver"; -import type { DidDocumentResolver } from "@atcute/identity-resolver"; -import type { Nsid } from "@atcute/lexicons/syntax"; +import { isNsid, type AtprotoDid, type Nsid } from "@atcute/lexicons/syntax"; +import { verifyRecord } from "@atcute/repo"; import type { LexiconPermissionSet } from "@atproto/oauth-scopes"; export type { LexiconPermissionSet }; +/** Collection that lexicon schema records (permission sets, spaces) live in. */ +const LEXICON_SCHEMA_COLLECTION = "com.atproto.lexicon.schema"; + /** * A lexicon space type declaration (`type: 'space'`), per the permissioned * data proposal. Mirrors `@atproto/oauth-scopes`'s internal `LexiconSpace` @@ -80,6 +94,7 @@ export interface CreateAtcutePermissionSetResolverOptions { export function createAtcutePermissionSetResolver( opts: CreateAtcutePermissionSetResolverOptions, ): PermissionSetResolver { + const fetchImpl = opts.fetch ?? globalThis.fetch; const authority = new DohJsonLexiconAuthorityResolver({ dohUrl: opts.dohUrl, fetch: opts.fetch, @@ -96,6 +111,92 @@ export function createAtcutePermissionSetResolver( ?.main as { type?: string } | undefined; }; + /** + * Resolve a lexicon record's `defs.main` with the SAME proof guarantees as + * {@link LexiconSchemaResolver.resolve} — resolve the DID document, find the + * PDS, fetch the record as a proof CAR, and verify the commit signature + * against the key in the DID document — but WITHOUT its final + * whole-document validation. + * + * WORKAROUND: `@atcute/lexicon-resolver` finishes by parsing the fetched + * document with `@atcute/lexicon-doc`, whose def-type whitelist has no + * `type: "space"` (as of lexicon-doc 2.2.0). A correctly published + * spaces-alpha type declaration therefore throws `InvalidLexiconSchemaError` + * (invalid_literal at `.defs.main.type`) and never returns, so the consent + * UI shows "could not resolve space type declaration" for every space + * scope. Replicating steps 1–3 here keeps full proof verification on the + * space path while letting us validate `defs.main` against our own + * {@link LexiconSpace} shape instead. + * + * Deliberately NOT an unauthenticated `com.atproto.repo.getRecord`: the + * signature proof is part of the lexicon-resolution contract. + * + * Remove this and route the space path back through + * `LexiconSchemaResolver.resolve()` once lexicon-doc gains the `space` def + * type (upstream: https://github.com/mary-ext/atcute). + */ + const resolveVerifiedMain = async ( + did: AtprotoDid, + nsid: Nsid, + ): Promise => { + // Step 1: DID document → PDS service endpoint. + const didDocument = await opts.didDocumentResolver.resolve(did); + const pdsEndpoint = getPdsEndpoint(didDocument); + if (!pdsEndpoint) { + throw new Error(`no atproto PDS in DID document; did=${did}`); + } + + // Step 2: fetch the lexicon record as a proof CAR. + const url = new URL("/xrpc/com.atproto.sync.getRecord", pdsEndpoint); + url.searchParams.set("did", did); + url.searchParams.set("collection", LEXICON_SCHEMA_COLLECTION); + url.searchParams.set("rkey", nsid); + const response = await fetchImpl(url, { + headers: { accept: "application/vnd.ipld.car" }, + }); + if (!response.ok) { + throw new Error( + `failed to fetch lexicon record; nsid=${nsid}; status=${response.status}`, + ); + } + const carBytes = new Uint8Array(await response.arrayBuffer()); + + // Step 3: verify the record's commit signature against the DID + // document's atproto key — identical to LexiconSchemaResolver's proof + // check, so this path is no weaker than the permission-set path. + const material = getAtprotoVerificationMaterial(didDocument); + if (!material) { + throw new Error( + `DID document has no atproto verification material; did=${did}`, + ); + } + const found = getPublicKeyFromDidController(material); + const publicKey: PublicKey = + found.type === "p256" + ? await P256PublicKey.importRaw(found.publicKeyBytes) + : await Secp256k1PublicKey.importRaw(found.publicKeyBytes); + const verified = await verifyRecord({ + did, + collection: LEXICON_SCHEMA_COLLECTION, + rkey: nsid, + publicKey, + carBytes, + }); + + // Sanity-check the record envelope (mirrors the resolver) before + // trusting `defs.main`. + const raw = verified.record; + if ( + typeof raw !== "object" || + raw === null || + (raw as { $type?: unknown }).$type !== LEXICON_SCHEMA_COLLECTION || + (raw as { id?: unknown }).id !== nsid + ) { + throw new Error(`invalid lexicon schema record; nsid=${nsid}`); + } + return (raw as { defs?: Record }).defs?.main; + }; + return { async resolve(nsid) { const main = await resolveMain(nsid); @@ -103,8 +204,34 @@ export function createAtcutePermissionSetResolver( return main as unknown as LexiconPermissionSet; }, async resolveSpaceDeclaration(nsid) { - const main = await resolveMain(nsid); - if (!main || main.type !== "space") return null; + const did = await authority.resolve(nsid); + const main = await resolveVerifiedMain(did, nsid); + if ( + typeof main !== "object" || + main === null || + (main as { type?: unknown }).type !== "space" + ) { + // Not a space type declaration (e.g. a permission set). The + // caller treats null as "no space metadata", not an error. + return null; + } + // Validate against the local LexiconSpace shape. The record itself + // is already proof-verified above; this only guards the fields the + // consent UI and grant-time collection defaulting rely on. + const decl = main as Record; + if (typeof decl.name !== "string") { + throw new Error( + `space type declaration is missing a name; nsid=${nsid}`, + ); + } + if ( + !Array.isArray(decl.collections) || + !decl.collections.every((c) => isNsid(c)) + ) { + throw new Error( + `space type declaration has invalid collections; nsid=${nsid}`, + ); + } return main as unknown as LexiconSpace; }, }; diff --git a/packages/oauth-provider/test/permission-sets.test.ts b/packages/oauth-provider/test/permission-sets.test.ts new file mode 100644 index 0000000..f6b188f --- /dev/null +++ b/packages/oauth-provider/test/permission-sets.test.ts @@ -0,0 +1,233 @@ +import { Secp256k1Keypair } from "@atproto/crypto"; +import { + MemoryBlockstore, + Repo, + WriteOpAction, + getRecords, +} from "@atproto/repo"; +import type { DidDocumentResolver } from "@atcute/identity-resolver"; +import type { Nsid } from "@atcute/lexicons/syntax"; +import { beforeAll, describe, expect, it } from "vitest"; +import { createAtcutePermissionSetResolver } from "../src/permission-sets.js"; + +/** + * These tests cover the spaces-alpha workaround in `resolveSpaceDeclaration`: + * `@atcute/lexicon-resolver` can't parse a `type: "space"` lexicon document + * (its def-type whitelist has no "space"), so the resolver replicates the + * fetch + signature-proof steps itself. The fixtures below build a real + * proof CAR with `@atproto/repo` (exactly what `com.atproto.sync.getRecord` + * returns) and feed it through a mocked network so we exercise the true + * `@atcute/repo` verification path — not a stub. + */ + +const LEXICON_COLLECTION = "com.atproto.lexicon.schema"; +const NSID = "earth.cirrus.check.space" as Nsid; +const AUTHORITY_DID = "did:web:pds.example"; +const PDS_ENDPOINT = "https://pds.example/"; +const DOH_URL = "https://doh.example/dns-query"; + +const SPACE_RECORD = { + $type: LEXICON_COLLECTION, + lexicon: 1, + id: NSID, + defs: { + main: { + type: "space", + name: "Cirrus Check", + collections: ["earth.cirrus.check.item", "earth.cirrus.check.result"], + }, + }, +}; + +/** Build the signed proof CAR a PDS would return for `record`. */ +async function buildProofCar( + keypair: Secp256k1Keypair, + record: Record, +): Promise { + const storage = new MemoryBlockstore(); + const repo = await Repo.create(storage, AUTHORITY_DID, keypair, [ + { + action: WriteOpAction.Create, + collection: LEXICON_COLLECTION, + rkey: NSID, + record, + }, + ]); + const chunks: Uint8Array[] = []; + for await (const chunk of getRecords(storage, repo.cid, [ + { collection: LEXICON_COLLECTION, rkey: NSID }, + ])) { + chunks.push(chunk); + } + const total = chunks.reduce((n, c) => n + c.length, 0); + const car = new Uint8Array(total); + let offset = 0; + for (const chunk of chunks) { + car.set(chunk, offset); + offset += chunk.length; + } + return car; +} + +/** A DID document exposing `publicKeyMultibase` as its `#atproto` key. */ +function didDocument(publicKeyMultibase: string) { + return { + id: AUTHORITY_DID, + verificationMethod: [ + { + id: `${AUTHORITY_DID}#atproto`, + type: "Multikey", + controller: AUTHORITY_DID, + publicKeyMultibase, + }, + ], + service: [ + { + id: `${AUTHORITY_DID}#atproto_pds`, + type: "AtprotoPersonalDataServer", + serviceEndpoint: PDS_ENDPOINT, + }, + ], + }; +} + +function dohResponse(): Response { + return new Response( + JSON.stringify({ + Status: 0, + TC: false, + RD: true, + RA: true, + AD: false, + CD: false, + Question: [{ name: `_lexicon.check.cirrus.earth`, type: 16 }], + Answer: [ + { + name: `_lexicon.check.cirrus.earth`, + type: 16, + TTL: 300, + data: `did=${AUTHORITY_DID}`, + }, + ], + }), + { headers: { "content-type": "application/dns-json" } }, + ); +} + +/** + * Wire up the resolver with a fetch mock that answers the DoH authority + * lookup and the `com.atproto.sync.getRecord` CAR fetch, plus a DID-document + * resolver returning `didDoc`. + */ +function makeResolver(car: Uint8Array, publicKeyMultibase: string) { + const didDocumentResolver: DidDocumentResolver = { + resolve: async () => + didDocument(publicKeyMultibase) as unknown as Awaited< + ReturnType + >, + }; + const fetch: typeof globalThis.fetch = async (input) => { + const url = new URL( + typeof input === "string" || input instanceof URL ? input : input.url, + ); + if (url.pathname === "/xrpc/com.atproto.sync.getRecord") { + return new Response(car, { + headers: { "content-type": "application/vnd.ipld.car" }, + }); + } + if (url.searchParams.get("type") === "TXT") { + return dohResponse(); + } + throw new Error(`unexpected fetch: ${url}`); + }; + return createAtcutePermissionSetResolver({ + dohUrl: DOH_URL, + didDocumentResolver, + fetch, + }); +} + +let keypair: Secp256k1Keypair; +let multikey: string; + +beforeAll(async () => { + keypair = await Secp256k1Keypair.create(); + multikey = keypair.did().slice("did:key:".length); +}); + +describe("resolveSpaceDeclaration", () => { + it('resolves a proof-verified `type: "space"` declaration', async () => { + const car = await buildProofCar(keypair, SPACE_RECORD); + const resolver = makeResolver(car, multikey); + + const decl = await resolver.resolveSpaceDeclaration!(NSID); + + expect(decl).not.toBeNull(); + expect(decl?.type).toBe("space"); + expect(decl?.name).toBe("Cirrus Check"); + expect(decl?.collections).toEqual([ + "earth.cirrus.check.item", + "earth.cirrus.check.result", + ]); + }); + + it("rejects an unproven record signed by a different key", async () => { + const car = await buildProofCar(keypair, SPACE_RECORD); + // DID document advertises someone else's key, so the commit signature + // won't verify — proving we didn't drop proof verification. + const attacker = await Secp256k1Keypair.create(); + const resolver = makeResolver(car, attacker.did().slice("did:key:".length)); + + await expect(resolver.resolveSpaceDeclaration!(NSID)).rejects.toThrow( + /signature verification failed/, + ); + }); + + it("rejects a tampered proof CAR", async () => { + const car = await buildProofCar(keypair, SPACE_RECORD); + const last = car.length - 1; + car[last] = (car[last] ?? 0) ^ 0xff; + const resolver = makeResolver(car, multikey); + + await expect(resolver.resolveSpaceDeclaration!(NSID)).rejects.toThrow(); + }); + + it("returns null for a non-space declaration", async () => { + const permissionSet = { + $type: LEXICON_COLLECTION, + lexicon: 1, + id: NSID, + defs: { + main: { + type: "permission-set", + permissions: [{ type: "repo", collection: "app.bsky.feed.post" }], + }, + }, + }; + const car = await buildProofCar(keypair, permissionSet); + const resolver = makeResolver(car, multikey); + + expect(await resolver.resolveSpaceDeclaration!(NSID)).toBeNull(); + }); + + it("rejects a space declaration with non-NSID collections", async () => { + const bad = { + $type: LEXICON_COLLECTION, + lexicon: 1, + id: NSID, + defs: { + main: { + type: "space", + name: "Broken", + collections: ["not a valid nsid"], + }, + }, + }; + const car = await buildProofCar(keypair, bad); + const resolver = makeResolver(car, multikey); + + await expect(resolver.resolveSpaceDeclaration!(NSID)).rejects.toThrow( + /invalid collections/, + ); + }); +}); diff --git a/plans/in-progress/spaces.md b/plans/in-progress/spaces.md index 31dd77a..2b1b419 100644 --- a/plans/in-progress/spaces.md +++ b/plans/in-progress/spaces.md @@ -76,6 +76,15 @@ User stories S1–S9 all have automated coverage (54 engine/protocol tests, `runInDurableObject` instead. - `@atproto/oauth-scopes@alpha` does not re-export `LexiconSpace`; oauth-provider defines the shape locally. +- `@atcute/lexicon-resolver` can't resolve a space type declaration: + it validates the fetched doc against `@atcute/lexicon-doc`, whose + def-type whitelist has no `type: "space"`, so a valid declaration + throws `InvalidLexiconSchemaError` (invalid_literal at + `.defs.main.type`). `permission-sets.ts`'s `resolveSpaceDeclaration` + replicates the resolver's DID-doc → PDS → proof-CAR → signature-verify + steps itself, then validates `defs.main` against the local shape — + full proof verification, tolerant parse. Remove once upstream + lexicon-doc learns the `space` def type. - Deleted spaces read as `SpaceNotFound`, never-written spaces as `RepoNotFound`; `getSpaceCredential` alone answers `SpaceDeleted` (tombstone retained in the space DO). diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0d5df90..7dc76be 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -164,6 +164,9 @@ importers: packages/oauth-provider: dependencies: + '@atcute/crypto': + specifier: ^2.3.0 + version: 2.4.4 '@atcute/identity': specifier: ^1.1.0 version: 1.1.4 @@ -176,6 +179,9 @@ importers: '@atcute/lexicons': specifier: ^1.2.6 version: 1.3.0 + '@atcute/repo': + specifier: ^0.1.1 + version: 0.1.4 '@atproto/oauth-scopes': specifier: 0.0.0-spaces-alpha-20260818163953 version: 0.0.0-spaces-alpha-20260818163953 @@ -192,6 +198,12 @@ importers: '@arethetypeswrong/cli': specifier: ^0.18.2 version: 0.18.2 + '@atproto/crypto': + specifier: ^0.4.5 + version: 0.4.5 + '@atproto/repo': + specifier: ^0.8.12 + version: 0.8.12 '@cloudflare/workers-types': specifier: ^4.20251225.0 version: 4.20260207.0