From 00094ee251fdc3ea3402fecf9441eea2f4f76146 Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Fri, 8 May 2026 09:05:59 -0400 Subject: [PATCH 1/4] fix(record-host): require owner-signed enrollment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The recordHost.enroll endpoint accepted either an owner-signed call OR a self-attested authority call. The latter let any DID claim to be the authority for any space's URI and rebind that space's authority. Drop the self-attesting-authority branch. Require the caller to be the space owner (sa.issuer === parts.ownerDid). The owner is still free to designate any authority via body.authority — only the legitimacy of the caller changes. Updates the existing test that codified the old behavior to assert the new rejection, and adds a positive test for the canonical owner-driven split-deployment enrollment flow. --- packages/contrail-record-host/src/routes.ts | 6 +-- .../contrail/tests/spaces-enrollment.test.ts | 46 ++++++++++++++++--- 2 files changed, 41 insertions(+), 11 deletions(-) diff --git a/packages/contrail-record-host/src/routes.ts b/packages/contrail-record-host/src/routes.ts index 63fc951..25cd2ab 100644 --- a/packages/contrail-record-host/src/routes.ts +++ b/packages/contrail-record-host/src/routes.ts @@ -107,11 +107,9 @@ export function registerRecordHostRoutes( if (!parts) { return c.json({ error: "InvalidRequest", reason: "malformed-uri" }, 400); } - const callerIsOwner = sa.issuer === parts.ownerDid; - const callerIsAuthority = sa.issuer === body.authority; - if (!callerIsOwner && !callerIsAuthority) { + if (sa.issuer !== parts.ownerDid) { return c.json( - { error: "Forbidden", reason: "not-owner-or-authority" }, + { error: "Forbidden", reason: "not-owner" }, 403 ); } diff --git a/packages/contrail/tests/spaces-enrollment.test.ts b/packages/contrail/tests/spaces-enrollment.test.ts index 3b7ef5b..78cc8b0 100644 --- a/packages/contrail/tests/spaces-enrollment.test.ts +++ b/packages/contrail/tests/spaces-enrollment.test.ts @@ -127,7 +127,7 @@ describe("auto-enrollment via createSpace", () => { expect(((await reenroll.json()) as any).ok).toBe(true); }); - it("non-owner / non-authority callers cannot enroll", async () => { + it("non-owner callers cannot enroll", async () => { const app = await makeApp(); const create = await call(app, "POST", "/xrpc/test.enroll.space.createSpace", ALICE, {}); const uri = ((await create.json()) as any).space.uri; @@ -137,7 +137,7 @@ describe("auto-enrollment via createSpace", () => { authority: SERVICE_DID, }); expect(res.status).toBe(403); - expect((await res.json()).reason).toBe("not-owner-or-authority"); + expect((await res.json()).reason).toBe("not-owner"); }); }); @@ -369,10 +369,10 @@ describe("split deployment — authority and record host on separate apps", () = expect((await list.json()).reason).toBe("not-enrolled"); }); - it("the declared authority can enroll a space without the owner's involvement", async () => { - // Scenario: the authority service ('Contrail' in our naming) is a - // separate DID and acts on behalf of an owner. The authority can enroll - // because phase 5 accepts either owner-or-authority. + it("a third party cannot enroll a space by claiming to be the authority", async () => { + // Regression: the enroll handler used to accept either owner-signed + // OR authority-self-attested calls. That let any DID claim "I am the + // authority for ats:///..." and rebind the space. const authorityDb = createSqliteDatabase(":memory:"); const hostDb = createSqliteDatabase(":memory:"); const cfg: ContrailConfig = { @@ -392,11 +392,43 @@ describe("split deployment — authority and record host on separate apps", () = const create = await call(authorityApp, "POST", "/xrpc/test.split.space.createSpace", ALICE, {}); const uri = ((await create.json()) as any).space.uri; - // Authority service identifies itself via the JWT issuer matching its DID. + // SERVICE_DID self-attests as the authority for Alice's space. + // Must be rejected — only the owner can enroll. const enroll = await call(hostApp, "POST", "/xrpc/test.split.recordHost.enroll", SERVICE_DID, { spaceUri: uri, authority: SERVICE_DID, }); + expect(enroll.status).toBe(403); + expect((await enroll.json()).reason).toBe("not-owner"); + }); + + it("the owner can enroll their space designating a separate authority", async () => { + // Positive: the owner-signed path lets Alice point her space at the + // configured authority service (the canonical split-deployment flow). + const authorityDb = createSqliteDatabase(":memory:"); + const hostDb = createSqliteDatabase(":memory:"); + const cfg: ContrailConfig = { + namespace: "test.split", + collections: { message: { collection: "app.event.message" } }, + spaces: { + authority: { type: "tools.atmo.event.space", serviceDid: SERVICE_DID, signing: SIGNING }, + recordHost: {}, + }, + }; + const resolved = resolveConfig(cfg); + await initSchema(authorityDb, resolved); + await initSchema(hostDb, resolved); + + const authorityApp = buildAuthorityApp(authorityDb); + const hostApp = buildRecordHostApp(hostDb); + const create = await call(authorityApp, "POST", "/xrpc/test.split.space.createSpace", ALICE, {}); + const uri = ((await create.json()) as any).space.uri; + + const enroll = await call(hostApp, "POST", "/xrpc/test.split.recordHost.enroll", ALICE, { + spaceUri: uri, + authority: SERVICE_DID, + }); expect(enroll.status).toBe(200); + expect(((await enroll.json()) as any).ok).toBe(true); }); }); From be62232403137eed344abf923b183c75bcd9b41b Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Fri, 8 May 2026 09:44:26 -0400 Subject: [PATCH 2/4] fix(spaces): drop LocalBindingResolver from default composite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The default in-process credential verifier composed [Enrollment, Local], so a never-enrolled space still resolved its authority via LocalBindingResolver — collapsing the host-side consent layer the enrollment table is meant to provide. Default to enrollment-only. createLocalBindingResolver remains exported for explicit single-tenant wiring via options.credentialVerifier. --- packages/contrail-appview/src/core/spaces/router.ts | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/packages/contrail-appview/src/core/spaces/router.ts b/packages/contrail-appview/src/core/spaces/router.ts index 943f6a9..a0557f5 100644 --- a/packages/contrail-appview/src/core/spaces/router.ts +++ b/packages/contrail-appview/src/core/spaces/router.ts @@ -9,9 +9,7 @@ import { HostedAdapter } from "./adapter"; import { buildVerifier, createBindingCredentialVerifier, - createCompositeBindingResolver, createEnrollmentBindingResolver, - createLocalBindingResolver, createLocalKeyResolver, createServiceAuthMiddleware, } from "@atmo-dev/contrail-base"; @@ -71,18 +69,11 @@ export function registerSpacesRoutes( ); if (spacesConfig.recordHost) { - // Default in-process verifier: enrollment is the canonical binding - // source; Local-binding is a fallback for spaces created but not yet - // enrolled. Caller overrides via `options.credentialVerifier` to - // accept external authorities. const credentialVerifier = options.credentialVerifier ?? (authorityConfig.signing ? createBindingCredentialVerifier({ - bindings: createCompositeBindingResolver([ - createEnrollmentBindingResolver({ recordHost: adapter }), - createLocalBindingResolver({ authorityDid: authorityConfig.serviceDid }), - ]), + bindings: createEnrollmentBindingResolver({ recordHost: adapter }), keys: createLocalKeyResolver({ authorityDid: authorityConfig.serviceDid, publicKey: authorityConfig.signing.publicKey, From 428feead5f637e03666e60f9092d123bfaa87263 Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Fri, 8 May 2026 10:05:44 -0400 Subject: [PATCH 3/4] fix(spaces): schema-validate PDS-record declarations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PDS binding resolver trusted body.value.authority on any record returned for the space's URI, with only a startsWith('did:') sniff. A malformed or attacker-shaped record at the same NSID could rebind authority — record content alone, no signature verification. Validate before trusting: - $type must equal the URI's type (the space-type lexicon embeds the declaration fields per tools.atmo.space.declaration's contract) - createdAt must be a string - authority must match a strict did:plc | did:web regex Fail closed on any deviation. Adds three negative tests covering each deviation path. --- packages/contrail-base/src/spaces/binding.ts | 12 +++- .../contrail/tests/spaces-binding.test.ts | 66 +++++++++++++++++++ 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/packages/contrail-base/src/spaces/binding.ts b/packages/contrail-base/src/spaces/binding.ts index a81b95f..2457f4d 100644 --- a/packages/contrail-base/src/spaces/binding.ts +++ b/packages/contrail-base/src/spaces/binding.ts @@ -141,10 +141,16 @@ export function createPdsBindingResolver(args: { } if (!res.ok) return null; const body = (await res.json().catch(() => null)) as - | { value?: { authority?: unknown } } + | { value?: { $type?: unknown; authority?: unknown; createdAt?: unknown } } | null; - const authority = body?.value?.authority; - return typeof authority === "string" && authority.startsWith("did:") ? authority : null; + const value = body?.value; + if (!value) return null; + if (value.$type !== parts.type) return null; + if (typeof value.createdAt !== "string") return null; + const authority = value.authority; + if (typeof authority !== "string") return null; + if (!/^did:(plc|web):[a-zA-Z0-9._:%-]+(#[a-zA-Z0-9._-]+)?$/.test(authority)) return null; + return authority; }, }; } diff --git a/packages/contrail/tests/spaces-binding.test.ts b/packages/contrail/tests/spaces-binding.test.ts index 427b6f2..d121df4 100644 --- a/packages/contrail/tests/spaces-binding.test.ts +++ b/packages/contrail/tests/spaces-binding.test.ts @@ -158,6 +158,72 @@ describe("BindingResolver — PDS record", () => { }); expect(await r.resolveAuthority(SPACE_URI)).toBeNull(); }); + + it("returns null when the record's $type doesn't match the URI's type", async () => { + const fetch = mockFetch( + new Map([ + [ + "https://pds.test/xrpc/com.atproto.repo.getRecord", + { + value: { + $type: "com.attacker.fake.type", + authority: "did:web:authority.example", + createdAt: "2026-04-30T00:00:00Z", + }, + }, + ], + ]) + ); + const r = createPdsBindingResolver({ + resolver: mockResolver({ pdsEndpoint: "https://pds.test" }), + fetch, + }); + expect(await r.resolveAuthority(SPACE_URI)).toBeNull(); + }); + + it("returns null when createdAt is missing or non-string", async () => { + const fetch = mockFetch( + new Map([ + [ + "https://pds.test/xrpc/com.atproto.repo.getRecord", + { + value: { + $type: "com.example.event.space", + authority: "did:web:authority.example", + // createdAt deliberately omitted + }, + }, + ], + ]) + ); + const r = createPdsBindingResolver({ + resolver: mockResolver({ pdsEndpoint: "https://pds.test" }), + fetch, + }); + expect(await r.resolveAuthority(SPACE_URI)).toBeNull(); + }); + + it("returns null when authority isn't a well-formed DID", async () => { + const fetch = mockFetch( + new Map([ + [ + "https://pds.test/xrpc/com.atproto.repo.getRecord", + { + value: { + $type: "com.example.event.space", + authority: "did:fake!!://garbage", + createdAt: "2026-04-30T00:00:00Z", + }, + }, + ], + ]) + ); + const r = createPdsBindingResolver({ + resolver: mockResolver({ pdsEndpoint: "https://pds.test" }), + fetch, + }); + expect(await r.resolveAuthority(SPACE_URI)).toBeNull(); + }); }); describe("BindingResolver — DID-doc service entry", () => { From dffbd36cb8f8855d990cd14532cbefbe6b09c46f Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Fri, 8 May 2026 10:43:54 -0400 Subject: [PATCH 4/4] docs(auth): document binding-layer trust assumptions Two trust assumptions in the binding layer cannot be closed at the Contrail layer alone: - DID-doc service-entry rebind: deployments using createDidDocBindingResolver inherit PLC's rotation-key authorization model. Any rotation key on the owner's account can rewrite the #atproto_space_authority service entry. - PDS app password held for managed communities: community.provision and community.adopt hold unscoped app passwords that can write any record on the user-owned PDS, including the binding declaration read by createPdsBindingResolver. Both are constraints of their respective binding sources and onboarding flows, not bugs. Document each so operators picking a binding strategy can size them against their own threat model, with pointers to the canonical issues for the upstream/protocol-level work that would close them: #38 (DID-doc), #39 (PDS app password). --- docs/05-auth.md | 22 ++++++++++++++++++++++ docs/10-deployment-shapes.md | 9 +++++++++ 2 files changed, 31 insertions(+) diff --git a/docs/05-auth.md b/docs/05-auth.md index 7cb7578..6237fb8 100644 --- a/docs/05-auth.md +++ b/docs/05-auth.md @@ -190,6 +190,28 @@ No auth needed for: Public requests skip all verification middleware — no JWT parsing, no DID-doc fetch. Fast path. +## Trust assumptions + +Two trust assumptions in the binding layer cannot be closed at the Contrail layer alone. They are listed here so operators picking a binding strategy or onboarding flow can size them against their own threat model. + +### DID-doc binding path + +The record host can resolve a space's authority from three sources (see [Spaces § Discovery](./06-spaces.md#discovery--binding-resolution)): + +- **Local enrollment** — the host's own `record_host_enrollments` table, written via `recordHost.enroll`. Owner-signed; the host has full control over what's stored. +- **PDS record** — read from the owner's PDS at the space URI. +- **DID-doc service entry** — read from `service[id="#atproto_space_authority"]` on the owner's DID doc. + +The DID-doc path inherits PLC's authorization model: any rotation key on the owner's account can submit an update op rewriting the service entry. There is no per-entry signature or "this entry can only be edited by key X" constraint at the PLC layer. + +If the integrity of the space-authority binding needs to exceed what any one of the owner's rotation keys can already do, configure the host to resolve via local enrollment instead, or wait for an upstream signed-binding mechanism. Tracked upstream as [flo-bit/contrail#38](https://github.com/flo-bit/contrail/issues/38). + +### Contrail-held PDS app password + +Deployments running `community.provision` or `community.adopt` necessarily hold an ATProto app password for the user-owned PDS account so Contrail can write on the community's behalf. ATProto app passwords are unscoped at the PDS layer — they can write any record to the repo, including the `tools.atmo.space.declaration` record (or its embedded equivalent on a space-type record) that drives binding decisions in `createPdsBindingResolver`. + +This is an intentional consequence of the managed-community model, not a bug. Operators should treat stored app passwords with the same care as rotation keys, and reach for scoped app passwords (when ATProto adds them) or signed binding records to tighten the model further. Tracked upstream as [flo-bit/contrail#39](https://github.com/flo-bit/contrail/issues/39). + ## How the pieces fit A typical flow for a third-party app acting as a user in a space: diff --git a/docs/10-deployment-shapes.md b/docs/10-deployment-shapes.md index 25464ed..7b8675b 100644 --- a/docs/10-deployment-shapes.md +++ b/docs/10-deployment-shapes.md @@ -192,6 +192,15 @@ A deployment can act as the authority for spaces it owns *and* a record host for When in doubt, all-in-one. Splitting is for when you have a real operational reason to separate the two — different teams running them, different latency profiles, different scaling targets, different governance. +## Known trust assumptions + +Two assumptions in the binding layer cannot be closed at the Contrail layer alone, and which one applies depends on the shape you pick: + +- Deployments wiring `createDidDocBindingResolver` inherit PLC's rotation-key authorization model for the `#atproto_space_authority` service entry. Tracked as [flo-bit/contrail#38](https://github.com/flo-bit/contrail/issues/38). +- Deployments running `community.provision` or `community.adopt` hold an unscoped ATProto app password for each provisioned PDS account. Tracked as [flo-bit/contrail#39](https://github.com/flo-bit/contrail/issues/39). + +See [Auth § Trust assumptions](./05-auth.md#trust-assumptions) for the constraints in detail and what binding source to prefer when those assumptions don't fit your threat model. + ## What's not here - **Authority migration** — moving a space's authority from DID A to DID B. The architecture supports it (re-enroll on the host with the new authority binding) but no helper API yet.