From 24c4eaffa0e22e6030c25b536322f21795d2d599 Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Thu, 9 Jul 2026 07:24:03 +0200 Subject: [PATCH 1/2] =?UTF-8?q?fix(security):=20enforce=20host=E2=86=94tok?= =?UTF-8?q?en=20binding=20before=20sending=20stored=20token=20(#30)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit authedSession paired resolveConfig() (CT_HOST env precedence) with a token read that discarded the stored host, so a CT_HOST override sent the stored login token — as a URL query param — to a foreign host, leaking it into that server's access logs even on a failed login. Compare the stored host against the resolved host BEFORE any network call and refuse on mismatch, naming both hosts. An explicit CT_LOGINTOKEN env token has no stored binding and is exempt. The token still rides as a query param: the handshake requires it (an Authorization header yields a null CSRF token and breaks writes, per ctClient's header comment) — documented at the call site. --- src/api/ctClient.ts | 5 +++ src/api/session.ts | 32 ++++++++++++++-- tests/session.test.ts | 88 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 tests/session.test.ts diff --git a/src/api/ctClient.ts b/src/api/ctClient.ts index 09a2f97..edf65ca 100644 --- a/src/api/ctClient.ts +++ b/src/api/ctClient.ts @@ -48,6 +48,11 @@ export class CtClient { /** Run the login-token handshake and cache the session cookie + CSRF token. */ async authenticate(loginToken: string): Promise { + // The token rides as a URL query param (it lands in the server's access logs). This is + // unavoidable for this token class: the handshake above is documented to require the + // `login_token` query param — an `Authorization` header yields a null CSRF token and breaks + // writes. The token↔host binding enforced in `authedSession` (issue #30) makes this safe by + // guaranteeing the token is only ever sent to the host it was captured against. const url = `${this.config.host}/api/whoami?login_token=${encodeURIComponent(loginToken)}`; const res = await fetchWithRetry( url, diff --git a/src/api/session.ts b/src/api/session.ts index 074d01c..e292da1 100644 --- a/src/api/session.ts +++ b/src/api/session.ts @@ -3,20 +3,46 @@ * Centralised so every command fails the same friendly way when logged out. */ import { CtClient, type WhoAmI } from "./ctClient.js"; -import { readToken } from "../auth/tokenStore.js"; -import { resolveConfig } from "../config.js"; +import { readCredentials } from "../auth/tokenStore.js"; +import { normalizeHost, resolveConfig } from "../config.js"; export interface AuthedSession { client: CtClient; me: WhoAmI; } +/** + * Build an authenticated {@link CtClient} from the stored credentials. + * + * Enforces the host↔token binding BEFORE any network call: the stored token is + * bound to the host it was captured against (`tokenStore` stores them together). + * If the resolved host (which gives `CT_HOST` precedence) differs from the host + * the stored token belongs to, we refuse — otherwise the secret would be sent + * (as a `login_token` URL query param) to a foreign server and land in its logs, + * even on a failed login. See issue #30. + * + * An explicit `CT_LOGINTOKEN` env token carries no stored-host binding, so the + * caller owns pairing it with the intended `CT_HOST` — no check applies there. + */ export async function authedSession(): Promise { const config = await resolveConfig(); - const token = await readToken(); + const envToken = process.env.CT_LOGINTOKEN?.trim(); + const stored = await readCredentials(); + const token = envToken || stored?.token; if (!token) { throw new Error("Not logged in. Run `ct auth login --host --token ` first."); } + + // Only the *stored* token is host-bound. Refuse to send it to a different host. + if (!envToken && stored && normalizeHost(stored.host) !== config.host) { + throw new Error( + `Refusing to send the stored login token: it belongs to ${normalizeHost(stored.host)}, ` + + `but the resolved host is ${config.host} (from CT_HOST). ` + + `Run \`ct auth login --host ${config.host} --token \` for that host, ` + + `or unset CT_HOST to use ${normalizeHost(stored.host)}.`, + ); + } + const client = new CtClient(config); const me = await client.authenticate(token); return { client, me }; diff --git a/tests/session.test.ts b/tests/session.test.ts new file mode 100644 index 0000000..55fde7b --- /dev/null +++ b/tests/session.test.ts @@ -0,0 +1,88 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +// Mock the token store so the Keychain is never touched and we control the stored host/token. +const { readCredentials, readStoredHost } = vi.hoisted(() => ({ + readCredentials: vi.fn<() => Promise<{ host: string; token: string } | null>>(), + readStoredHost: vi.fn<() => Promise>(async () => null), +})); +vi.mock("../src/auth/tokenStore.js", () => ({ readCredentials, readStoredHost })); + +import { authedSession } from "../src/api/session.js"; + +const savedHost = process.env.CT_HOST; +const savedToken = process.env.CT_LOGINTOKEN; + +function restoreEnv(): void { + if (savedHost === undefined) delete process.env.CT_HOST; + else process.env.CT_HOST = savedHost; + if (savedToken === undefined) delete process.env.CT_LOGINTOKEN; + else process.env.CT_LOGINTOKEN = savedToken; +} + +describe("authedSession host↔token binding (issue #30)", () => { + beforeEach(() => { + readCredentials.mockReset(); + delete process.env.CT_HOST; + delete process.env.CT_LOGINTOKEN; + }); + afterEach(() => { + vi.restoreAllMocks(); + restoreEnv(); + }); + + it("refuses to send the stored token to a CT_HOST-overridden host — zero network calls", async () => { + readCredentials.mockResolvedValue({ host: "https://prod.church.tools", token: "prod-secret" }); + process.env.CT_HOST = "https://staging.church.tools"; + const fetchSpy = vi.spyOn(globalThis, "fetch"); + + await expect(authedSession()).rejects.toThrow( + /belongs to https:\/\/prod\.church\.tools.*resolved host is https:\/\/staging\.church\.tools/s, + ); + // The token must never reach the wire: no fetch at all, so it can't land in a foreign log. + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("names both hosts in the refusal so the user can act", async () => { + readCredentials.mockResolvedValue({ host: "https://prod.church.tools", token: "prod-secret" }); + process.env.CT_HOST = "https://staging.church.tools"; + + await expect(authedSession()).rejects.toThrow( + /ct auth login --host https:\/\/staging\.church\.tools/, + ); + }); + + it("tolerates a trailing slash mismatch (host is normalized before comparison)", async () => { + readCredentials.mockResolvedValue({ host: "https://prod.church.tools/", token: "prod-secret" }); + process.env.CT_HOST = "https://prod.church.tools"; + const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response(JSON.stringify({ data: { id: 1 } }), { + status: 200, + headers: { "content-type": "application/json", "set-cookie": "SESSION=abc" }, + }), + ); + + // Matching (modulo trailing slash) host proceeds to the network handshake. + await authedSession().catch(() => {}); // csrf fetch may follow; we only assert the token was sent to the right host + const firstUrl = String(fetchSpy.mock.calls[0]?.[0] ?? ""); + expect(firstUrl).toContain("https://prod.church.tools/api/whoami"); + expect(firstUrl).toContain("login_token=prod-secret"); + }); + + it("skips the binding check for an explicit CT_LOGINTOKEN env token (no stored binding)", async () => { + readCredentials.mockResolvedValue({ host: "https://prod.church.tools", token: "prod-secret" }); + process.env.CT_HOST = "https://staging.church.tools"; + process.env.CT_LOGINTOKEN = "explicit-env-token"; + const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response(JSON.stringify({ data: { id: 1 } }), { + status: 200, + headers: { "content-type": "application/json", "set-cookie": "SESSION=abc" }, + }), + ); + + await authedSession().catch(() => {}); + // The env token is sent to the env host; the stored prod token is never used. + const firstUrl = String(fetchSpy.mock.calls[0]?.[0] ?? ""); + expect(firstUrl).toContain("https://staging.church.tools/api/whoami"); + expect(firstUrl).toContain("login_token=explicit-env-token"); + }); +}); From fd420fc005bbd9b40d3995dd1a466879f53429c6 Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Thu, 9 Jul 2026 07:31:17 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix(permissions):=20resolve=20scope=20again?= =?UTF-8?q?st=20desired=E2=88=AAstate,=20re-resolve=20after=20execute=20(#?= =?UTF-8?q?29,=20#33=20item=203)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two halves of one fix, sharing a re-resolution point: #29 — a config declaring a group AND a grant scoped to it could never be planned or applied: resolveScope threw for any key absent from state, and it runs (via desiredTuples → buildPermissionPlan) BEFORE executePlan creates anything. Now scope keys resolve against DESIRED ∪ STATE: a key declared in the config but not yet created is valid and renders as pending (`scope=[ (created this apply)]`) instead of aborting even read-only `ct plan`. Truly unknown keys (neither declared nor in state) still hard-fail. #33 item 3 — buildPermissionPlan resolved dataIds from PRE-apply state, so a group recreated in the same apply had its grant PUT with the old dangling id. Every scoped tuple now retains its symbolic scopeKey; applyPermissionPlan re-resolves each against the POST-execute state just before writing, so grants always carry fresh ids. A pending tuple that reaches the writer un-resolved is refused rather than silently written as a global grant. buildPermissionPlan gains the desired resources (for the declared-group set); apply passes post-execute state to applyPermissionPlan. --- docs/permissions.md | 36 +++++++---- src/commands/apply.ts | 6 +- src/commands/plan.ts | 2 +- src/permissions/apply.ts | 16 ++++- src/permissions/grants.ts | 31 +++++++++- src/permissions/plan.ts | 24 ++++++-- src/permissions/render.ts | 7 ++- src/permissions/scope.ts | 59 +++++++++++++++--- tests/permission-bootstrap.test.ts | 99 ++++++++++++++++++++++++++++++ tests/permission-plan.test.ts | 6 +- tests/permission-scope.test.ts | 19 +++++- 11 files changed, 267 insertions(+), 38 deletions(-) create mode 100644 tests/permission-bootstrap.test.ts diff --git a/docs/permissions.md b/docs/permissions.md index 6bd7bbd..29f36e2 100644 --- a/docs/permissions.md +++ b/docs/permissions.md @@ -82,18 +82,30 @@ The two DSL functions manage two different ChurchTools "domain types," and A scoped grant's `scope: [...]` is a list of **logical keys of groups managed by this tool** (declared via `ct.group` or adopted into state) — not raw -ChurchTools ids. At plan time each key is resolved to that group's `dataId` -via the state file; a key that isn't a managed group throws: - -``` -Scope key "kids_area" does not resolve to a managed group. Declare/adopt it, -or use a group already under management. -``` - -This is a deliberate constraint (see `src/permissions/scope.ts`): scope -targets must be tool-visible so `ct plan` can show what a grant actually -resolves to, and so renaming/re-keying a group doesn't silently orphan a -grant's scope. +ChurchTools ids. Each key is resolved against **desired ∪ state** +(`src/permissions/scope.ts`): + +- A key already in state resolves to that group's `dataId`. +- A key **declared in this config but not yet created** resolves to a *pending* + target: the plan renders it as `scope=[ (created this apply)]`, and its + real `dataId` is filled in at apply time — so a config can declare a group AND + a grant scoped to it and still plan/apply in one run (no bootstrap deadlock). +- A key that is neither in state nor declared throws: + + ``` + Scope key "kids_area" does not resolve to a managed group. Declare/adopt it, + or use a group already under management. + ``` + +The requirement that scope targets be tool-visible is deliberate: so `ct plan` +can show what a grant resolves to, and so renaming/re-keying a group doesn't +silently orphan a grant's scope. + +**Re-resolution at apply time.** Every scoped tuple retains its symbolic scope +key. Immediately before grants are written (after the resource tier has run), +each key is re-resolved against the post-execute state. This means a group +*created* or *recreated* in the same apply always gets its grant written with +its fresh `dataId`, never a pending placeholder or a stale, dangling id. ## Domain rules (validated, throw on violation) diff --git a/src/commands/apply.ts b/src/commands/apply.ts index adb6c7c..897e020 100644 --- a/src/commands/apply.ts +++ b/src/commands/apply.ts @@ -97,7 +97,7 @@ export function applyCommand(): Command { const { client } = await authedSession(); const { plan, actual, fetchErrors } = await buildPlan(client, state, desired, { configDir }); - const { items: permItems, fetchErrors: permFetchErrors } = await buildPermissionPlan(client, state, permissions); + const { items: permItems, fetchErrors: permFetchErrors } = await buildPermissionPlan(client, state, permissions, desired); const allFetchErrors = [...fetchErrors, ...permFetchErrors]; if (allFetchErrors.length > 0) { @@ -156,7 +156,9 @@ export function applyCommand(): Command { return; } - const permResult = await applyPermissionPlan(permItems, client); + // Re-resolve scope dataIds against the POST-execute state (executePlan has upserted every + // created/recreated group's real id) so grants are never written with a stale/pending id. + const permResult = await applyPermissionPlan(permItems, client, state); if (permResult.granted > 0 || permResult.deleted > 0) { success(`Permissions applied: ${permResult.granted} granted, ${permResult.deleted} deleted.`); } diff --git a/src/commands/plan.ts b/src/commands/plan.ts index 214d836..a452110 100644 --- a/src/commands/plan.ts +++ b/src/commands/plan.ts @@ -32,7 +32,7 @@ export function planCommand(): Command { const { client } = await authedSession(); const { plan, fetchErrors } = await buildPlan(client, state, desired, { configDir }); - const { items: permItems, fetchErrors: permFetchErrors } = await buildPermissionPlan(client, state, permissions); + const { items: permItems, fetchErrors: permFetchErrors } = await buildPermissionPlan(client, state, permissions, desired); if (opts.json) { out({ plan, permissions: permItems }); } else { diff --git a/src/permissions/apply.ts b/src/permissions/apply.ts index 238d8e3..cabbd36 100644 --- a/src/permissions/apply.ts +++ b/src/permissions/apply.ts @@ -5,19 +5,33 @@ * as belt-and-suspenders atop the structural-only registry. */ import type { CtClient } from "../api/ctClient.js"; +import type { State } from "../state/state.js"; import { assertNotPeople } from "../engine/guard.js"; import type { PermissionPlanItem } from "./plan.js"; import type { GrantTuple } from "./grants.js"; +import { reresolveTuple } from "./scope.js"; function body(t: GrantTuple): Record { + if (t.pending) { + // A pending tuple's dataId is unknown until it is re-resolved against post-execute state. + // Reaching here means re-resolution was skipped — refuse rather than emit a silent GLOBAL grant. + throw new Error(`Grant scoped to "${t.scopeKey}" was not re-resolved before apply — refusing to write it without a dataId.`); + } const b: Record = { authId: t.authId, type: t.type }; if (t.dataId.length) b.dataId = t.dataId; // omit when unscoped return b; } +/** + * Apply a permission plan. When `state` is provided, each scoped grant's dataId is RE-RESOLVED + * against it just before the PUT — `state` here is the POST-execute state (executePlan has upserted + * every created/recreated group), so grants are always written with fresh ids and a group created in + * the same apply gets its real id (#29, #33.3). Without `state`, tuples are written as-is. + */ export async function applyPermissionPlan( items: PermissionPlanItem[], client: Pick, + state?: State, ): Promise<{ granted: number; deleted: number }> { let granted = 0; let deleted = 0; @@ -25,7 +39,7 @@ export async function applyPermissionPlan( const path = `/permissions/${item.domainType}/${item.domainId}`; assertNotPeople(path); for (const t of item.diff.toPut) { - await client.request("PUT", path, body(t)); + await client.request("PUT", path, body(state ? reresolveTuple(t, state) : t)); granted++; } for (const t of item.diff.toDelete) { diff --git a/src/permissions/grants.ts b/src/permissions/grants.ts index 2e6655d..3f00520 100644 --- a/src/permissions/grants.ts +++ b/src/permissions/grants.ts @@ -5,14 +5,39 @@ */ export type DomainType = "group_role" | "group_type_role"; -export interface GrantTuple { authId: number; dataId: number[]; type: "grant" | "revoke" } +export interface GrantTuple { + authId: number; + dataId: number[]; + type: "grant" | "revoke"; + /** + * For a scoped grant: the symbolic scope key this tuple was resolved from. Retained so the + * dataId can be RE-RESOLVED against post-execute state at apply time — which fixes both a grant + * scoped to a group created in the same apply (its dataId is `pending` at plan time) and a stale + * dataId after a scope-target group is recreated (#29, #33.3). Absent on unscoped grants and on + * actual tuples read back from ChurchTools. + */ + scopeKey?: string; + /** + * True when `scopeKey` names a group DECLARED in this config but not yet created (absent from + * state at plan time). Its real dataId is unknown until `executePlan` runs, so the plan renders + * it as pending and it always diffs into `toPut`. Cleared once re-resolved at apply time. + */ + pending?: boolean; +} export interface RawPermission { authId: number; dataId: number | null; type: "grant" | "revoke"; domainId: number; isInherited?: boolean; meta?: { modifiedPid?: number }; } -export function tupleKey(t: { authId: number; dataId: number[]; type: string }): string { - return `${t.type}:${t.authId}:${[...t.dataId].sort((a, b) => a - b).join(",")}`; +/** + * Identity key for set reconciliation. A pending tuple has no resolved dataId yet, so key it by + * its symbolic scope key instead — that keeps it distinct from an unscoped grant (`dataId: []`) + * and guarantees it can never collide with an actual row (actuals never carry a scopeKey), so it + * always lands in `toPut`. + */ +export function tupleKey(t: { authId: number; dataId: number[]; type: string; scopeKey?: string; pending?: boolean }): string { + const scope = t.pending && t.scopeKey != null ? `pending:${t.scopeKey}` : [...t.dataId].sort((a, b) => a - b).join(","); + return `${t.type}:${t.authId}:${scope}`; } export function normalizeActual(rows: RawPermission[]): GrantTuple[] { diff --git a/src/permissions/plan.ts b/src/permissions/plan.ts index 5d94616..761631c 100644 --- a/src/permissions/plan.ts +++ b/src/permissions/plan.ts @@ -7,6 +7,7 @@ import type { CtClient } from "../api/ctClient.js"; import { CtApiError } from "../api/ctClient.js"; import type { State } from "../state/state.js"; +import type { DesiredResource } from "../engine/types.js"; import { resolveAuthId } from "./catalog.js"; import { resolveScope } from "./scope.js"; import { normalizeActual, diffGrants, type GrantTuple, type GrantDiff, type DomainType, type RawPermission } from "./grants.js"; @@ -21,8 +22,12 @@ export interface PermissionPlanItem { key: string; domainType: DomainType; domai * scalar read shape, a scoped grant `{right, scope:[a,b]}` becomes TWO single-dataId tuples, * not one two-element tuple. */ -export function desiredTuples(p: DesiredPermission, state: State): GrantTuple[] { - return p.grants.flatMap((g) => { +export function desiredTuples( + p: DesiredPermission, + state: State, + declaredGroupKeys: ReadonlySet = new Set(), +): GrantTuple[] { + return p.grants.flatMap((g): GrantTuple[] => { const name = typeof g === "string" ? g : g.right; const entry = resolveAuthId(name); if (p.domainType === "group_type_role" && entry.authId >= 10000) { @@ -41,15 +46,24 @@ export function desiredTuples(p: DesiredPermission, state: State): GrantTuple[] if (entry.scopeField == null) { throw new Error(`${p.domainType} "${p.key}": "${name}" is not a scoped right (no scopeField) — remove "scope" or use a scoped right.`); } - return resolveScope(g.scope, state).map((id) => ({ authId: entry.authId, dataId: [id], type: "grant" as const })); + // Retain the symbolic scopeKey on every scoped tuple so its dataId is re-resolved against + // post-execute state at apply time. `id === null` means the group is declared but not yet + // created (pending); it renders in the plan and always diffs into toPut (#29, #33.3). + return resolveScope(g.scope, state, declaredGroupKeys).map(({ key, id }) => + id === null + ? { authId: entry.authId, dataId: [], type: "grant" as const, scopeKey: key, pending: true } + : { authId: entry.authId, dataId: [id], type: "grant" as const, scopeKey: key }, + ); }); } export async function buildPermissionPlan( - client: Pick, state: State, permissions: DesiredPermission[], + client: Pick, state: State, permissions: DesiredPermission[], desired: DesiredResource[] = [], ): Promise<{ items: PermissionPlanItem[]; fetchErrors: string[] }> { const items: PermissionPlanItem[] = []; const fetchErrors: string[] = []; + // Keys declared as groups in the config — valid scope targets even before they are created. + const declaredGroupKeys = new Set(desired.filter((r) => r.type === "group").map((r) => r.key)); // one bulk fetch per distinct domainType const byType = new Map(); for (const dt of new Set(permissions.map((p) => p.domainType))) { @@ -65,7 +79,7 @@ export async function buildPermissionPlan( const all = byType.get(p.domainType); if (all == null) continue; // fetch failed for this domainType — recorded above const actual = normalizeActual(all.filter((r) => r.domainId === p.domainId)); - items.push({ key: p.key, domainType: p.domainType, domainId: p.domainId, diff: diffGrants(desiredTuples(p, state), actual) }); + items.push({ key: p.key, domainType: p.domainType, domainId: p.domainId, diff: diffGrants(desiredTuples(p, state, declaredGroupKeys), actual) }); } return { items, fetchErrors }; } diff --git a/src/permissions/render.ts b/src/permissions/render.ts index fb1e9be..94cef56 100644 --- a/src/permissions/render.ts +++ b/src/permissions/render.ts @@ -7,7 +7,12 @@ import type { PermissionPlanItem } from "./plan.js"; import type { GrantTuple } from "./grants.js"; function fmtTuple(t: GrantTuple): string { - const scope = t.dataId.length ? ` scope=[${t.dataId.join(",")}]` : ""; + let scope = ""; + if (t.pending && t.scopeKey != null) { + scope = ` scope=[${t.scopeKey} (created this apply)]`; + } else if (t.dataId.length) { + scope = ` scope=[${t.dataId.join(",")}]`; + } return `authId=${t.authId}${scope} (${t.type})`; } diff --git a/src/permissions/scope.ts b/src/permissions/scope.ts index cf788db..6ac5a44 100644 --- a/src/permissions/scope.ts +++ b/src/permissions/scope.ts @@ -1,14 +1,59 @@ import type { State } from "../state/state.js"; +import type { GrantTuple } from "./grants.js"; -/** Resolve scope logical keys to sorted group dataIds. MVP: scope keys must be managed groups. */ -export function resolveScope(scopeKeys: string[], state: State): number[] { - const ids: number[] = []; +/** One resolved scope key: `id` is the state group id, or `null` when the group is declared in the config but not yet created (pending). */ +export interface ScopeResolution { key: string; id: number | null } + +/** + * Resolve scope logical keys against DESIRED ∪ STATE. + * + * A key that names a managed group in state resolves to its id. A key that names a group DECLARED + * in this config but not yet in state (`declaredGroupKeys`) resolves to `null` (pending) — its id + * is only known after the resource tier applies, so it is re-resolved at apply time (see + * {@link reresolveTuple}). This is what lets a config declare a group AND a grant scoped to it and + * still plan/apply in one run (#29). A key that is neither in state nor declared stays a hard error. + * + * Resolved (in-state) keys sort ascending by id — ChurchTools reads scoped grants back one row per + * dataId, so a stable order keeps multi-scope grants idempotent. Pending keys follow, sorted by key. + */ +export function resolveScope( + scopeKeys: string[], + state: State, + declaredGroupKeys: ReadonlySet = new Set(), +): ScopeResolution[] { + const resolved: ScopeResolution[] = []; + const pending: ScopeResolution[] = []; for (const key of scopeKeys) { const m = state.resources[key]; - if (!m || m.type !== "group") { - throw new Error(`Scope key "${key}" does not resolve to a managed group. Declare/adopt it, or use a group already under management.`); + if (m && m.type === "group") { + resolved.push({ key, id: m.id }); + } else if (declaredGroupKeys.has(key)) { + pending.push({ key, id: null }); + } else { + throw new Error( + `Scope key "${key}" does not resolve to a managed group. Declare/adopt it, or use a group already under management.`, + ); } - ids.push(m.id); } - return ids.sort((a, b) => a - b); + resolved.sort((a, b) => (a.id as number) - (b.id as number)); + pending.sort((a, b) => a.key.localeCompare(b.key)); + return [...resolved, ...pending]; +} + +/** + * Re-resolve a scoped grant tuple's dataId against the current (post-execute) state, using the + * symbolic scopeKey retained on the tuple. Fixes stale ids after a recreate and fills in the id of + * a group created in the same apply. Unscoped tuples (no scopeKey) pass through unchanged. Throws + * if the scope key no longer resolves to a managed group — which should be impossible once the + * resource tier has applied, so it signals a real inconsistency rather than being silently skipped. + */ +export function reresolveTuple(t: GrantTuple, state: State): GrantTuple { + if (t.scopeKey == null) return t; + const m = state.resources[t.scopeKey]; + if (!m || m.type !== "group") { + throw new Error( + `Scope key "${t.scopeKey}" did not resolve to a managed group after apply — cannot write its grant with a valid dataId.`, + ); + } + return { ...t, dataId: [m.id], pending: false }; } diff --git a/tests/permission-bootstrap.test.ts b/tests/permission-bootstrap.test.ts new file mode 100644 index 0000000..6ee8c2e --- /dev/null +++ b/tests/permission-bootstrap.test.ts @@ -0,0 +1,99 @@ +/** + * Scope-resolution bootstrap + stale-id-after-recreate (#29, #33 item 3). + * + * Exercises the REAL build → execute → apply sequence with a mock client, so the + * shared re-resolution point is proven end-to-end: a scope key naming a group + * created (or recreated) in the same apply is resolved to that group's fresh id. + */ +import { describe, it, expect, vi } from "vitest"; +import { buildPermissionPlan, desiredTuples } from "../src/permissions/plan.js"; +import { applyPermissionPlan } from "../src/permissions/apply.js"; +import { renderPermissionPlan } from "../src/permissions/render.js"; +import { executePlan } from "../src/engine/execute.js"; +import { emptyState, type State } from "../src/state/state.js"; +import type { Plan, DesiredResource } from "../src/engine/types.js"; +import type { DesiredPermission } from "../src/permissions/types.js"; +import type { CtClient } from "../src/api/ctClient.js"; + +const HOST = "https://eqrm.church.tools"; + +// churchgroup:view group — authId 1104, scoped (scopeField cdb_gruppe). +const scopedPerm: DesiredPermission = { + key: "gtr", + domainType: "group_type_role", + domainId: 8, + grants: [{ right: "churchgroup:view group", scope: ["kids"] }], +}; +const desiredKidsGroup: DesiredResource[] = [ + { type: "group", key: "kids", fields: { name: "Kids" }, dependsOn: [] }, +]; + +/** A mock client: GET /permissions/* returns no existing rows; POST /groups mints `newId`. */ +function mockClient(newId: number) { + const calls: { method: string; path: string; body?: unknown }[] = []; + const request = vi.fn(async (method: string, path: string, body?: unknown) => { + calls.push({ method, path, body }); + if (method === "POST" && path === "/groups") return { id: newId }; + return {}; + }); + const get = vi.fn(async () => [] as unknown[]); + return { client: { request, get } as unknown as CtClient, calls }; +} + +const createKidsPlan: Plan = { + items: [{ type: "group", key: "kids", id: null, action: "create", changes: [{ field: "name", from: undefined, to: "Kids" }] }], +}; + +describe("scope bootstrap: declare group + grant scoped to it in one config (#29)", () => { + it("plans cleanly from empty state — no throw, grant rendered as pending", async () => { + const { client } = mockClient(555); + const state = emptyState(HOST); + const { items, fetchErrors } = await buildPermissionPlan(client, state, [scopedPerm], desiredKidsGroup); + expect(fetchErrors).toEqual([]); + // The scoped grant survives planning and lands in toPut as a pending tuple. + expect(items[0]?.diff.toPut).toEqual([{ authId: 1104, dataId: [], type: "grant", scopeKey: "kids", pending: true }]); + // Read-only `ct plan` renders instead of aborting, and labels the pending scope. + expect(renderPermissionPlan(items)).toContain("kids (created this apply)"); + }); + + it("applies cleanly in ONE run — grant PUT carries the id the group create returned", async () => { + const { client, calls } = mockClient(555); + const state = emptyState(HOST); + const { items } = await buildPermissionPlan(client, state, [scopedPerm], desiredKidsGroup); + + await executePlan(createKidsPlan, { client, state, statePath: "unused", save: async () => {} }); + expect(state.resources.kids?.id).toBe(555); // executePlan upserted the real id + + const res = await applyPermissionPlan(items, client, state); + expect(res.granted).toBe(1); + const put = calls.find((c) => c.method === "PUT" && c.path === "/permissions/group_type_role/8"); + expect(put?.body).toEqual({ authId: 1104, type: "grant", dataId: [555] }); // the FRESH id, not a pending placeholder + }); +}); + +describe("stale id after recreate (#33 item 3)", () => { + it("re-resolves the scope dataId against post-execute state — grant PUT carries the NEW id", async () => { + const { client, calls } = mockClient(777); + // Pre-apply state has kids under a stale id 100 (it vanished from CT and will be recreated). + const state: State = { version: 1, host: HOST, resources: { + kids: { type: "group", id: 100, key: "kids", fields: { name: "Kids" }, adoptedAt: "t", updatedAt: "t" }, + }}; + const { items } = await buildPermissionPlan(client, state, [scopedPerm], desiredKidsGroup); + // At plan time the tuple resolves to the OLD id 100… + expect(items[0]?.diff.toPut).toEqual([{ authId: 1104, dataId: [100], type: "grant", scopeKey: "kids" }]); + + // …then a recreate mints id 777 in state. + await executePlan(createKidsPlan, { client, state, statePath: "unused", save: async () => {} }); + expect(state.resources.kids?.id).toBe(777); + + await applyPermissionPlan(items, client, state); + const put = calls.find((c) => c.method === "PUT" && c.path === "/permissions/group_type_role/8"); + expect(put?.body).toEqual({ authId: 1104, type: "grant", dataId: [777] }); // NOT the stale 100 + }); +}); + +describe("truly-unknown scope key still hard-fails (#29 acceptance)", () => { + it("throws the clear error when a key is neither in state nor declared", () => { + expect(() => desiredTuples(scopedPerm, emptyState(HOST), new Set())).toThrow(/scope key "kids" does not resolve/i); + }); +}); diff --git a/tests/permission-plan.test.ts b/tests/permission-plan.test.ts index eb2d112..83b74e8 100644 --- a/tests/permission-plan.test.ts +++ b/tests/permission-plan.test.ts @@ -16,7 +16,7 @@ describe("desiredTuples", () => { ]}, state); expect(tuples).toEqual([ { authId: 1113, dataId: [], type: "grant" }, - { authId: 1104, dataId: [42], type: "grant" }, + { authId: 1104, dataId: [42], type: "grant", scopeKey: "kids_area" }, // scoped tuples retain their symbolic key for re-resolution ]); }); @@ -39,8 +39,8 @@ describe("desiredTuples", () => { ]}, state); // resolveScope sorts resolved dataIds ascending (7 < 42), independent of scope-key order. expect(tuples).toEqual([ - { authId: 1104, dataId: [7], type: "grant" }, - { authId: 1104, dataId: [42], type: "grant" }, + { authId: 1104, dataId: [7], type: "grant", scopeKey: "other" }, + { authId: 1104, dataId: [42], type: "grant", scopeKey: "kids_area" }, ]); expect(tuples.every((t) => t.dataId.length <= 1)).toBe(true); }); diff --git a/tests/permission-scope.test.ts b/tests/permission-scope.test.ts index 155f7bc..7923a7c 100644 --- a/tests/permission-scope.test.ts +++ b/tests/permission-scope.test.ts @@ -8,10 +8,23 @@ const state: State = { version: 1, host: "h", resources: { }}; describe("resolveScope", () => { - it("maps managed group keys to sorted ids", () => { - expect(resolveScope(["other", "kids_area"], state)).toEqual([7, 42]); + it("maps managed group keys to resolutions sorted by id", () => { + expect(resolveScope(["other", "kids_area"], state)).toEqual([ + { key: "other", id: 7 }, + { key: "kids_area", id: 42 }, + ]); }); - it("throws for a key that is not a managed group", () => { + it("resolves a declared-but-not-yet-created group key to a pending (null id) resolution", () => { + expect(resolveScope(["kids"], state, new Set(["kids"]))).toEqual([{ key: "kids", id: null }]); + }); + it("orders resolved (in-state, by id) before pending (by key)", () => { + expect(resolveScope(["pending_b", "kids_area", "pending_a"], state, new Set(["pending_a", "pending_b"]))).toEqual([ + { key: "kids_area", id: 42 }, + { key: "pending_a", id: null }, + { key: "pending_b", id: null }, + ]); + }); + it("throws for a key that is neither in state nor declared", () => { expect(() => resolveScope(["nope"], state)).toThrow(/scope key "nope"/i); }); });