From 827cdee761fc29d2e9986e628768f66a866dee86 Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Tue, 8 Sep 2026 14:23:04 +0200 Subject: [PATCH] Tests/docs: promote Soul's two it.fails, update the tripwire and fixtures Both caller-identity-soul.test.ts holes are now closed and promoted from it.fails to a plain it (both green): the caller-chosen completionCode is never redeemable, and an arbitrary returnTo origin is refused. The AppCaller-literal tripwire's expected forger list moves from ["private-command-plane.ts"] to [] now that the private plane constructs its caller through callerFromOpenedEnvelope instead of the literal; the "refuses to mint on refresh" test's wrongApp caller is built the same way, since a raw { appSlug } literal no longer satisfies the now-branded AppCaller type. access-plugin.test.ts's ghostlight completion-code assertion is updated: `completion.code` is minted independently of the attempt handle now, so the test documents that the code differs from the handle instead of asserting they match. app-registry.test.ts drops validateAppRegistration coverage (deleted) and seeds registered-app fixtures through InMemoryStore.seedRegisteredApp instead of the deleted store.registerApp. docs/service-contract.md documents the new 400 return_to_not_allowed failure mode. state/evidence.jsonl records this pass's decisions. Co-Authored-By: Claude Opus 5 --- docs/service-contract.md | 5 + state/evidence.jsonl | 1 + tests/access-plugin.test.ts | 9 +- tests/app-registry.test.ts | 152 ++++++----------------------- tests/caller-identity-soul.test.ts | 52 +++++----- 5 files changed, 75 insertions(+), 144 deletions(-) diff --git a/docs/service-contract.md b/docs/service-contract.md index 912cbf5..54df1a9 100644 --- a/docs/service-contract.md +++ b/docs/service-contract.md @@ -160,6 +160,11 @@ Failure modes: - `503 provider_not_configured` when the provider client id is missing - `400 untrusted_backend_callback` when `handoff.kind=backend_callback` and `callbackUrl` is not in that app's `GC_ACCESS_APP__BACKEND_CALLBACK_URLS` +- `400 return_to_not_allowed` when `returnTo`'s origin is not in that app's + `allowedReturnOrigins` (`src/app-profiles.ts`) -- `returnTo` is where the + browser handoff page postMessages the completion payload + (`src/browser-handoff.ts`), so this check runs for every caller, not only an + authenticated one - `401 app_auth_required` when the request body carries `entitlementPolicy` and the caller did not authenticate as that app via `x-heimdall-app-secret` (`resolveAppCaller`, `src/app-caller.ts`) -- this applies regardless of diff --git a/state/evidence.jsonl b/state/evidence.jsonl index dc1560c..3178947 100644 --- a/state/evidence.jsonl +++ b/state/evidence.jsonl @@ -34,3 +34,4 @@ {"ts": "2026-08-21T23:53:15+00:00", "type": "implementation", "status": "ok", "note": "Ghostlight private auth now validates refresh against exact persisted session custody and revision; logout atomically expires that session and advances its revision, preventing stale or racing refresh from resurrecting revoked access."} {"ts":"2026-08-22T02:01:00+00:00","type":"deployment","status":"accepted","note":"Yggdrasil runs Heimdall 1086aee01169bf60e8a492b2740db1c6f3e8cabf with CultLib 5cefa0db0079a8e3ee22f29d7b9e6e5aa60912a9. Odin b4f9a2e95f0b41cebdeddc49223781d1d3c7b42a returns Heimdall's provider, command boundary, Eve plugin, and transport profile under unique catalog keys. Ghostlight b515ca90c25573005a616244143803b37f2d06ec accepted canonical auth.begin through the Odin-resolved boundary; only advertised Discord and Heimdall navigation origins reached the browser."} {"ts": "2026-09-06T22:12:03+00:00", "type": "design", "status": "accepted", "note": "App slugs are no longer compile-time. AppSlug is a string; membership is a registry lookup, or isAppSlug shape validation where no store is reachable (the offline verifier). Capability rules are data (anyOf over Heimdall's own fact vocabulary), replacing a per-profile evaluateSharedCapabilities function that could not be accepted from a stranger. POST /v1/apps registers in RFC 7591 shape, gated on GC_ACCESS_APP_REGISTRATION_SECRET and closed until it is set. Built-in slugs are reserved so a registration cannot shadow a shipped app's audience. Motivation: a creator self-hosting Erycina alongside Heimdall would otherwise maintain a patched auth server to add one slug."} +{"ts": "2026-09-08T12:20:12+00:00", "type": "security", "status": "accepted", "note": "epiphany/heimdall-caller-identity, second Hands pass: closed the two Soul-proven holes. (1) auth-completions redeem code is now always store-minted (never handoff.attemptId); attemptId survives only as a separate correlation field on StoredAuthCompletion/CreateAuthCompletionInput, and a new store.consumeAuthCompletionByAttempt(appSlug, attemptId, at) lets the private command plane's completeAuth redeem by the handle it already owns instead of forging an HTTP self-call with completionCode=handle. (2) startOAuthFlow now refuses any returnTo whose origin is not in AppProfile.allowedReturnOrigins (repixelizer/streampixels/bifrost -> .gamecult.org, ghostlight -> yggdrasil.gamecult.org per its docs/app-profiles doc and existing test fixtures; a registered app's allowlist is derived from its stored redirectUris origins). AppCaller is now branded with an unexported Symbol (app-caller.ts); private-command-plane.ts's two { appSlug } literals are gone, replaced by app-caller.ts's callerFromOpenedEnvelope. Deleted: validateAppRegistration, AppRegistrationRequest, RegistrationProblem, AppRegistrationError, HeimdallStore.registerApp (writer). Kept: findRegisteredApp/listRegisteredApps/resolveAppProfile fallback -- AppSlug is a plain string (contracts.ts), registered_apps is a real extension point for a slug Heimdall was never rebuilt for, and isBuiltInApp's only production call site (listAppProfiles) depends on that fallback existing; a row is now provisioned by direct SQL (or InMemoryStore.seedRegisteredApp in tests), not by any app-facing writer. Deleted the two appSlug===\"ghostlight\" branches in private-command-plane.ts; the requirement is now AppProfile.requiredEntitlementPolicyKind, checked generically through resolveAppProfile. tsc --noEmit clean; vitest 94 passed/1 pre-existing-skip, only pre-existing failure is tests/odin-publication.test.ts (Cannot find module 'cultcache-ts', present on main too).", "branch": "epiphany/heimdall-caller-identity"} diff --git a/tests/access-plugin.test.ts b/tests/access-plugin.test.ts index 178432b..b5027d0 100644 --- a/tests/access-plugin.test.ts +++ b/tests/access-plugin.test.ts @@ -129,7 +129,14 @@ describe("Heimdall Eve access plugin", () => { url: `/v1/oauth/discord/callback?code=test-code&state=${encodeURIComponent(state ?? "")}`, }); expect(callback.statusCode, callback.body).toBe(201); - expect(callback.json().completion.code).toBe(handle); + // The completion code is always minted by the store now, never the + // attempt handle the caller supplied (that coupling was the + // caller-identity hole: a client-chosen attemptId used to become the + // redemption code verbatim). heimdall.auth.complete below redeems by + // handle through the store's own attempt-correlated lookup, not by + // knowing this code. + expect(callback.json().completion.code).toEqual(expect.any(String)); + expect(callback.json().completion.code).not.toBe(handle); const completeEnvelope = sealPrivateEnvelope({ appSlug: "ghostlight", diff --git a/tests/app-registry.test.ts b/tests/app-registry.test.ts index 05d1738..1187b32 100644 --- a/tests/app-registry.test.ts +++ b/tests/app-registry.test.ts @@ -1,25 +1,25 @@ import { beforeEach, describe, expect, it } from "vitest"; -import { - isBuiltInApp, - listAppProfiles, - resolveAppProfile, - validateAppRegistration, - type AppRegistrationRequest, -} from "../src/app-registry.js"; +import { isBuiltInApp, listAppProfiles, resolveAppProfile } from "../src/app-registry.js"; import { evaluateCapabilityRules } from "../src/capability-rules.js"; -import { type Provider } from "../src/contracts.js"; +import { type CapabilityDefinition } from "../src/capability-rules.js"; import { entitlementFacts, grantFacts, identityFacts } from "../src/facts.js"; import { InMemoryStore } from "../src/store/in-memory.js"; +import { type StoredRegisteredApp } from "../src/store/types.js"; -const request = (overrides: Partial = {}): AppRegistrationRequest => ({ +const registeredApp = (overrides: Partial = {}): StoredRegisteredApp => ({ slug: "erycina", displayName: "Erycina", + profileVersion: "2026-01-01", + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", identityProviders: ["discord"], - redirectUris: ["https://erycina.example/auth/callback"], + entitlementSources: [], + managedConnectionProviders: [], capabilities: [ { key: "viewer_access", mode: "shared", summary: "Signed-in viewer.", anyOf: [identityFacts.authenticated] }, - ], + ] as CapabilityDefinition[], + redirectUris: ["https://erycina.example/auth/callback"], ...overrides, }); @@ -30,98 +30,16 @@ beforeEach(() => { /** * There is no runtime registration authority anymore (deleted with the - * caller-identity cut: registerApp minted a client_secret nobody verified). - * `registered_apps` remains a profile store, so tests that need a resolvable - * profile write the row directly through the store primitive, the same way - * production code would seed one outside the deleted HTTP surface. + * caller-identity cut: registerApp minted a client_secret nobody verified, + * and validateAppRegistration/AppRegistrationRequest/RegistrationProblem/ + * AppRegistrationError went with it). `registered_apps` remains a profile + * store consulted below the built-in profiles; a real deployment provisions + * a row directly against Postgres, and `seedRegisteredApp` is the equivalent + * test/ops seam on the in-memory store. */ -async function seedRegisteredApp(overrides: Partial = {}, registeredAt = "2026-01-01T00:00:00.000Z") { - const req = request(overrides); - return store.registerApp({ - slug: req.slug, - displayName: req.displayName.trim(), - profileVersion: req.profileVersion?.trim() || registeredAt.slice(0, 10), - registeredAt, - identityProviders: req.identityProviders as Provider[], - entitlementSources: (req.entitlementSources ?? []) as Provider[], - managedConnectionProviders: (req.managedConnectionProviders ?? []) as Provider[], - capabilities: req.capabilities, - redirectUris: req.redirectUris, - }); -} - -describe("validateAppRegistration", () => { - it("accepts a well-formed registration", () => { - expect(validateAppRegistration(request())).toEqual([]); - }); - - it("refuses a slug that shadows a built-in app", () => { - // Otherwise a registration inherits a shipped app's audience, which is - // impersonation rather than registration. - const problems = validateAppRegistration(request({ slug: "bifrost" })); - expect(problems[0]!.reason).toMatch(/reserved by a built-in/); - }); - - it("refuses slugs that would be awkward as a JWT audience or storage key", () => { - for (const slug of ["A", "1app", "has space", "has/slash", ""]) { - expect(validateAppRegistration(request({ slug })).some((p) => p.field === "slug")).toBe(true); - } - }); - - it("refuses an unknown provider", () => { - const problems = validateAppRegistration(request({ identityProviders: ["myspace"] })); - expect(problems.some((p) => p.reason.includes("myspace"))).toBe(true); - }); - - it("requires at least one identity provider", () => { - expect(validateAppRegistration(request({ identityProviders: [] })).some((p) => p.field === "identityProviders")).toBe(true); - }); - - it("refuses a capability rule naming a fact Heimdall does not produce", () => { - // The registration gate. An app selects from Heimdall's vocabulary; it - // cannot invent a fact to grant itself something. - const problems = validateAppRegistration( - request({ - capabilities: [{ key: "admin", mode: "shared", summary: "x", anyOf: ["invented.superuser"] }], - }), - ); - expect(problems.some((p) => p.reason.includes("Unknown term"))).toBe(true); - }); - - describe("redirect URIs", () => { - it("requires at least one", () => { - expect(validateAppRegistration(request({ redirectUris: [] })).some((p) => p.field === "redirectUris")).toBe(true); - }); - - it("refuses a relative URI", () => { - expect(validateAppRegistration(request({ redirectUris: ["/callback"] }))[0]!.reason).toMatch(/absolute/); - }); - - it("refuses plain http off loopback, because that is where a code would leak", () => { - expect( - validateAppRegistration(request({ redirectUris: ["http://erycina.example/cb"] }))[0]!.reason, - ).toMatch(/https/); - }); - - it("allows http on loopback, for local development", () => { - expect(validateAppRegistration(request({ redirectUris: ["http://127.0.0.1:8080/cb"] }))).toEqual([]); - expect(validateAppRegistration(request({ redirectUris: ["http://localhost:8080/cb"] }))).toEqual([]); - }); - - it("refuses a fragment", () => { - expect(validateAppRegistration(request({ redirectUris: ["https://a.example/cb#x"] }))[0]!.reason).toMatch(/fragment/); - }); - }); - - it("reports every problem rather than stopping at the first", () => { - const problems = validateAppRegistration(request({ slug: "!!", displayName: "", identityProviders: [] })); - expect(problems.length).toBeGreaterThan(2); - }); -}); - describe("resolveAppProfile", () => { it("resolves a registered app", async () => { - await seedRegisteredApp(); + store.seedRegisteredApp(registeredApp()); const profile = await resolveAppProfile(store, "erycina"); expect(profile?.displayName).toBe("Erycina"); @@ -136,19 +54,9 @@ describe("resolveAppProfile", () => { }); it("a built-in cannot be shadowed even if a row exists for it", async () => { - // Registration refuses built-in slugs; this is the second place that holds, - // so a row written by some other path cannot hijack a shipped audience. - await store.registerApp({ - slug: "bifrost", - displayName: "Impostor", - profileVersion: "x", - registeredAt: "2026-01-01T00:00:00.000Z", - identityProviders: ["discord"], - entitlementSources: [], - managedConnectionProviders: [], - capabilities: [], - redirectUris: [], - }); + // The built-in lookup runs first; a row written by some other path + // cannot hijack a shipped audience. + store.seedRegisteredApp(registeredApp({ slug: "bifrost", displayName: "Impostor" })); expect((await resolveAppProfile(store, "bifrost"))?.displayName).toBe("Bifrost"); }); @@ -156,12 +64,14 @@ describe("resolveAppProfile", () => { describe("a registered profile evaluates like a built-in", () => { it("grants capabilities through the same evaluator", async () => { - await seedRegisteredApp({ - capabilities: [ - { key: "member_access", mode: "shared", summary: "member", anyOf: [entitlementFacts.appAccess, grantFacts.globalMember] }, - { key: "post_create", mode: "shared", summary: "post", anyOf: ["member_access"] }, - ], - }); + store.seedRegisteredApp( + registeredApp({ + capabilities: [ + { key: "member_access", mode: "shared", summary: "member", anyOf: [entitlementFacts.appAccess, grantFacts.globalMember] }, + { key: "post_create", mode: "shared", summary: "post", anyOf: ["member_access"] }, + ] as CapabilityDefinition[], + }), + ); const profile = await resolveAppProfile(store, "erycina"); expect(evaluateCapabilityRules(profile!.capabilities, new Set([grantFacts.globalMember]))).toEqual([ @@ -174,7 +84,7 @@ describe("a registered profile evaluates like a built-in", () => { describe("listAppProfiles", () => { it("lists built-ins alongside registered apps", async () => { - await seedRegisteredApp(); + store.seedRegisteredApp(registeredApp()); const slugs = (await listAppProfiles(store)).map((profile) => profile.slug); expect(slugs).toContain("bifrost"); @@ -182,7 +92,7 @@ describe("listAppProfiles", () => { }); it("lists each app once", async () => { - await seedRegisteredApp(); + store.seedRegisteredApp(registeredApp()); const slugs = (await listAppProfiles(store)).map((profile) => profile.slug); expect(new Set(slugs).size).toBe(slugs.length); diff --git a/tests/caller-identity-soul.test.ts b/tests/caller-identity-soul.test.ts index c0eaa77..3ebc000 100644 --- a/tests/caller-identity-soul.test.ts +++ b/tests/caller-identity-soul.test.ts @@ -11,7 +11,7 @@ import { encode } from "@msgpack/msgpack"; import { invokeCultNetOperation } from "cultnet-ts"; import { afterEach, describe, expect, it } from "vitest"; import { buildApp, refreshAppSession, startOAuthFlow } from "../src/app.js"; -import { secretMatches } from "../src/app-caller.js"; +import { callerFromOpenedEnvelope, secretMatches } from "../src/app-caller.js"; import { type HeimdallConfig } from "../src/config.js"; import { entitlementFacts } from "../src/facts.js"; import { type OAuthProviderRuntime } from "../src/oauth.js"; @@ -78,15 +78,15 @@ function mockDiscordRuntime(): OAuthProviderRuntime { } describe("caller identity: unauthenticated mint paths", () => { - // Item 1. The browser flow is public by design, but the completion code is - // chosen by whoever starts the flow (handoff.attemptId becomes the - // completion code verbatim, src/app.ts createAuthCompletion `code: - // handoff.attemptId`), and auth-completions/redeem takes no app secret. - // An unauthenticated party can therefore fix the code, get a victim to - // finish provider login on the authorization URL, and redeem the victim's - // signed access + refresh tokens. This is the pre-existing hole the cut - // did not close. - it.fails("a client-chosen attemptId must not be redeemable for signed tokens without app auth", async () => { + // Item 1 (closed). The completion code used to be chosen by whoever started + // the flow (handoff.attemptId became the completion code verbatim, + // src/app.ts createAuthCompletion `code: handoff.attemptId`), and + // auth-completions/redeem takes no app secret. An unauthenticated party + // could therefore fix the code, get a victim to finish provider login on + // the authorization URL, and redeem the victim's signed access + refresh + // tokens. The store now always mints the code itself; attemptId is kept + // only as a separate correlation field the browser may carry. + it("a client-chosen attemptId must not be redeemable for signed tokens without app auth", async () => { const app = await buildApp({ config: testConfig(), oauthRuntimes: { discord: mockDiscordRuntime() } }); resources.push(app); @@ -117,16 +117,19 @@ describe("caller identity: unauthenticated mint paths", () => { // still no x-heimdall-app-secret }); - // The invariant the cut claims. Today: 201 with accessToken + refreshToken. + // The invariant the cut claims: the caller-chosen code is never minted, + // so it can never be redeemed. expect(redeem.statusCode).not.toBe(201); expect(redeem.json().accessToken).toBeUndefined(); }); - // Same family: returnTo is only validated as `format: uri`. The success page - // postMessages the completion payload to new URL(returnTo).origin and links - // to returnTo with the code in the fragment (src/browser-handoff.ts). Nothing - // binds returnTo to an app-owned origin. - it.fails("an unauthenticated start must not accept an arbitrary returnTo origin", async () => { + // Item 2 (closed). returnTo used to be validated only as `format: uri`. The + // success page postMessages the completion payload to + // new URL(returnTo).origin and links to returnTo with the code in the + // fragment (src/browser-handoff.ts), so an unlisted origin was an + // exfiltration primitive. startOAuthFlow now refuses any returnTo whose + // origin is not in the app profile's allowedReturnOrigins. + it("an unauthenticated start must not accept an arbitrary returnTo origin", async () => { const app = await buildApp({ config: testConfig(), oauthRuntimes: { discord: mockDiscordRuntime() } }); resources.push(app); @@ -151,7 +154,7 @@ describe("caller identity: unauthenticated mint paths", () => { } as Parameters[3]); expect(nullCaller).toEqual({ statusCode: 401, body: { error: "app_auth_required" } }); - const wrongApp = await refreshAppSession(context, "repixelizer", { appSlug: "ghostlight" }, { + const wrongApp = await refreshAppSession(context, "repixelizer", callerFromOpenedEnvelope("ghostlight"), { refreshToken: "irrelevant", entitlementPolicy: policy, } as Parameters[3]); @@ -198,14 +201,19 @@ describe("caller identity: single authority", () => { expect(offenders).toEqual([]); }); - it("AppCaller literals are constructed only where identity was actually checked", () => { - // Item 3. `{ appSlug }` as an AppCaller argument must appear only in the - // private plane (after openPrivateEnvelope + sourceRuntimeId) and nowhere - // else; the HTTP path gets its caller from resolveAppCaller. + it("AppCaller literals are constructed nowhere; AppCaller is a branded type", () => { + // Item 3 (closed). `{ appSlug }` used to satisfy AppCaller structurally, + // so the private plane (after openPrivateEnvelope + sourceRuntimeId) + // constructed the literal directly — nothing but grep discipline stopped + // any other importer from doing the same. AppCaller is now branded with + // an unexported symbol, and the private plane gets its caller from + // app-caller.ts's callerFromOpenedEnvelope, the same way the HTTP path + // gets its caller from resolveAppCaller. No file may construct the + // literal at all anymore. const forgers = sources .filter(([name, text]) => name !== "app-caller.ts" && /(startOAuthFlow|refreshAppSession)\([^;]*\{\s*appSlug\s*\}/s.test(text)) .map(([name]) => name); - expect(forgers).toEqual(["private-command-plane.ts"]); + expect(forgers).toEqual([]); }); });