fix(security): commit board-key allow audit with its mutation
Stage the allow disposition in a request-scoped context and flush it into the first transaction that performs a domain mutation, so an authorized mutation and its audit record commit or roll back together. Denials and read-only allows stay immediately durable, and a mutation that runs outside any transaction is recorded and logged as uncoupled. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta
cryppadotta
parent
4daa9af530
commit
8cb500359e
|
|
@ -174,6 +174,7 @@ import { apiCompression } from "./middleware/api-compression.js";
|
|||
import { chatWebhookBodyParser } from "./middleware/chat-webhook-body.js";
|
||||
import { createChatWebhookDiagnostics } from "./services/chat-webhook-diagnostics.js";
|
||||
import { boardKeyAuthorizationMiddleware } from "./security/board-key-route-registry.js";
|
||||
import { instrumentDbForBoardKeyAudit } from "./security/board-key-audit-coupling.js";
|
||||
|
||||
type UiMode = "none" | "static" | "vite-dev";
|
||||
const FEEDBACK_EXPORT_FLUSH_INTERVAL_MS = 5_000;
|
||||
|
|
@ -564,6 +565,11 @@ export async function createApp(
|
|||
// REPLACES whatever actor the request otherwise resolved to, and only on
|
||||
// the one endpoint it authorizes (see the middleware for the contract).
|
||||
app.use(cloudControlMiddleware());
|
||||
// In-place, idempotent instrumentation: the board-key gate stages its allow
|
||||
// disposition and the handle commits it inside the transaction of the
|
||||
// mutation it authorizes. Instrumenting the handle itself keeps object
|
||||
// identity, so `dbOrTx === db` checks in services are unaffected.
|
||||
instrumentDbForBoardKeyAudit(db);
|
||||
app.use(boardKeyAuthorizationMiddleware(db));
|
||||
app.use("/api/auth", authRoutes(db));
|
||||
if (opts.betterAuthHandler) {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,358 @@
|
|||
import { randomUUID } from "node:crypto";
|
||||
import express from "express";
|
||||
import request from "supertest";
|
||||
import { eq, sql } from "drizzle-orm";
|
||||
import { pgTable, text, uuid } from "drizzle-orm/pg-core";
|
||||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import {
|
||||
boardApiKeyAuthorizationEvents,
|
||||
createDb,
|
||||
getEmbeddedPostgresTestSupport,
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
type Db,
|
||||
type EmbeddedPostgresTestDatabase,
|
||||
} from "@paperclipai/db";
|
||||
import { errorHandler } from "../middleware/error-handler.js";
|
||||
import {
|
||||
BOARD_KEY_AUDIT_COUPLINGS,
|
||||
createBoardKeyAuditContext,
|
||||
instrumentDbForBoardKeyAudit,
|
||||
runWithBoardKeyAuditContext,
|
||||
settleBoardKeyAuditContext,
|
||||
stageBoardKeyAllowAudit,
|
||||
type BoardKeyAuditValues,
|
||||
} from "./board-key-audit-coupling.js";
|
||||
import { boardKeyAuthorizationMiddleware } from "./board-key-route-registry.js";
|
||||
|
||||
/**
|
||||
* Stand-in for any board-key-protected domain table. Using a dedicated table
|
||||
* keeps the regression focused on the audit/mutation boundary itself instead of
|
||||
* the constraints of whichever real table a route happens to write.
|
||||
*/
|
||||
const probe = pgTable("board_key_atomicity_probe", {
|
||||
id: uuid("id").primaryKey().defaultRandom(),
|
||||
value: text("value").notNull(),
|
||||
});
|
||||
|
||||
const support = await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = support.supported ? describe : describe.skip;
|
||||
|
||||
describeEmbeddedPostgres("board-key allow audit / mutation atomicity", () => {
|
||||
let database: EmbeddedPostgresTestDatabase;
|
||||
let db: Db;
|
||||
// Independent connection pool: reads through it can only observe committed
|
||||
// rows, which is how the test proves the audit row lives inside the domain
|
||||
// transaction rather than in a separate one.
|
||||
let outside: Db;
|
||||
|
||||
beforeAll(async () => {
|
||||
database = await startEmbeddedPostgresTestDatabase("paperclip-board-key-atomicity-");
|
||||
db = instrumentDbForBoardKeyAudit(createDb(database.connectionString));
|
||||
outside = createDb(database.connectionString);
|
||||
await outside.execute(sql`
|
||||
CREATE TABLE IF NOT EXISTS board_key_atomicity_probe (
|
||||
id uuid PRIMARY KEY DEFAULT gen_random_uuid(),
|
||||
value text NOT NULL
|
||||
)
|
||||
`);
|
||||
}, 240_000);
|
||||
|
||||
afterAll(async () => {
|
||||
await database?.cleanup();
|
||||
});
|
||||
|
||||
function stagedAllow(overrides: Partial<BoardKeyAuditValues> = {}): BoardKeyAuditValues {
|
||||
return {
|
||||
boardApiKeyId: randomUUID(),
|
||||
ownerUserId: randomUUID(),
|
||||
tokenPrefix: "pcp_board_atomic",
|
||||
action: "issues:write",
|
||||
classification: "company",
|
||||
authoritativeCompanyId: null,
|
||||
authoritativeResourceType: "company",
|
||||
authoritativeResourceId: null,
|
||||
decision: "allow",
|
||||
reason: "authorized",
|
||||
requestId: randomUUID(),
|
||||
runId: null,
|
||||
details: {},
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
async function committedAudit(boardApiKeyId: string) {
|
||||
return await outside
|
||||
.select({
|
||||
decision: boardApiKeyAuthorizationEvents.decision,
|
||||
reason: boardApiKeyAuthorizationEvents.reason,
|
||||
details: boardApiKeyAuthorizationEvents.details,
|
||||
})
|
||||
.from(boardApiKeyAuthorizationEvents)
|
||||
.where(eq(boardApiKeyAuthorizationEvents.boardApiKeyId, boardApiKeyId));
|
||||
}
|
||||
|
||||
async function committedProbe(value: string) {
|
||||
return await outside.select({ value: probe.value }).from(probe).where(eq(probe.value, value));
|
||||
}
|
||||
|
||||
it("commits the allow disposition inside the transaction of the mutation it authorizes", async () => {
|
||||
const values = stagedAllow();
|
||||
const marker = `commit-${randomUUID()}`;
|
||||
const context = createBoardKeyAuditContext();
|
||||
|
||||
await runWithBoardKeyAuditContext(context, async () => {
|
||||
stageBoardKeyAllowAudit(values);
|
||||
// Staging alone must never be durable: that is the non-atomic behaviour
|
||||
// this coupling replaces.
|
||||
expect(await committedAudit(values.boardApiKeyId)).toHaveLength(0);
|
||||
await db.transaction(async (tx) => {
|
||||
await tx.insert(probe).values({ value: marker });
|
||||
// Still inside the transaction: neither the mutation nor its
|
||||
// disposition is visible to another connection yet.
|
||||
expect(await committedProbe(marker)).toHaveLength(0);
|
||||
expect(await committedAudit(values.boardApiKeyId)).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
expect(context.flushed).toBe(true);
|
||||
expect(await committedProbe(marker)).toHaveLength(1);
|
||||
expect(await committedAudit(values.boardApiKeyId)).toEqual([
|
||||
{
|
||||
decision: "allow",
|
||||
reason: "authorized",
|
||||
details: { coupling: BOARD_KEY_AUDIT_COUPLINGS.transaction },
|
||||
},
|
||||
]);
|
||||
}, 60_000);
|
||||
|
||||
it("rolls the authorized mutation back when the audit write fails at the boundary", async () => {
|
||||
// Forced failure exactly at the audit/mutation boundary: `action` is NOT
|
||||
// NULL, so the coupled audit insert aborts the domain transaction.
|
||||
const values = stagedAllow({ action: null as unknown as string });
|
||||
const marker = `audit-failure-${randomUUID()}`;
|
||||
const context = createBoardKeyAuditContext();
|
||||
|
||||
await expect(runWithBoardKeyAuditContext(context, async () => {
|
||||
stageBoardKeyAllowAudit(values);
|
||||
await db.transaction(async (tx) => {
|
||||
await tx.insert(probe).values({ value: marker });
|
||||
});
|
||||
})).rejects.toThrow();
|
||||
|
||||
expect(context.flushed).toBe(false);
|
||||
expect(await committedProbe(marker)).toHaveLength(0);
|
||||
expect(await committedAudit(values.boardApiKeyId)).toHaveLength(0);
|
||||
|
||||
// Settlement must not paper over the failure with a durable allow either.
|
||||
await settleBoardKeyAuditContext(db, context, 500);
|
||||
expect(await committedAudit(values.boardApiKeyId)).toHaveLength(0);
|
||||
}, 60_000);
|
||||
|
||||
it("keeps no durable allow disposition when the protected mutation rolls back", async () => {
|
||||
const values = stagedAllow();
|
||||
const marker = `rollback-${randomUUID()}`;
|
||||
const context = createBoardKeyAuditContext();
|
||||
|
||||
await expect(runWithBoardKeyAuditContext(context, async () => {
|
||||
stageBoardKeyAllowAudit(values);
|
||||
await db.transaction(async (tx) => {
|
||||
await tx.insert(probe).values({ value: marker });
|
||||
throw new Error("controller failed after mutating");
|
||||
});
|
||||
})).rejects.toThrow("controller failed after mutating");
|
||||
|
||||
expect(context.flushed).toBe(false);
|
||||
expect(await committedProbe(marker)).toHaveLength(0);
|
||||
expect(await committedAudit(values.boardApiKeyId)).toHaveLength(0);
|
||||
}, 60_000);
|
||||
|
||||
it("does not resurrect an allow disposition when a rolled-back mutation is swallowed", async () => {
|
||||
const values = stagedAllow();
|
||||
const marker = `swallowed-${randomUUID()}`;
|
||||
const context = createBoardKeyAuditContext();
|
||||
|
||||
await runWithBoardKeyAuditContext(context, async () => {
|
||||
stageBoardKeyAllowAudit(values);
|
||||
await expect(db.transaction(async (tx) => {
|
||||
await tx.insert(probe).values({ value: marker });
|
||||
throw new Error("swallowed");
|
||||
})).rejects.toThrow("swallowed");
|
||||
});
|
||||
|
||||
// The handler reports success, but nothing committed, so there is no
|
||||
// mutation for an `authorized` record to describe.
|
||||
await settleBoardKeyAuditContext(db, context, 200);
|
||||
expect(await committedProbe(marker)).toHaveLength(0);
|
||||
expect(await committedAudit(values.boardApiKeyId)).toHaveLength(0);
|
||||
}, 60_000);
|
||||
|
||||
it("records the disposition of an authorized request that mutates nothing", async () => {
|
||||
const values = stagedAllow();
|
||||
const context = createBoardKeyAuditContext();
|
||||
|
||||
await runWithBoardKeyAuditContext(context, async () => {
|
||||
stageBoardKeyAllowAudit(values);
|
||||
await db.select({ value: probe.value }).from(probe).limit(1);
|
||||
});
|
||||
await settleBoardKeyAuditContext(db, context, 204);
|
||||
|
||||
expect(await committedAudit(values.boardApiKeyId)).toEqual([
|
||||
{
|
||||
decision: "allow",
|
||||
reason: "authorized",
|
||||
details: { coupling: BOARD_KEY_AUDIT_COUPLINGS.noMutation },
|
||||
},
|
||||
]);
|
||||
}, 60_000);
|
||||
|
||||
it("does not treat a read-only raw statement as an uncoupled mutation", async () => {
|
||||
const values = stagedAllow();
|
||||
const context = createBoardKeyAuditContext();
|
||||
|
||||
await runWithBoardKeyAuditContext(context, async () => {
|
||||
stageBoardKeyAllowAudit(values);
|
||||
await db.execute(sql`SELECT 1`);
|
||||
});
|
||||
await settleBoardKeyAuditContext(db, context, 200);
|
||||
|
||||
expect(context.untransacted).toBe(false);
|
||||
expect(await committedAudit(values.boardApiKeyId)).toEqual([
|
||||
{
|
||||
decision: "allow",
|
||||
reason: "authorized",
|
||||
details: { coupling: BOARD_KEY_AUDIT_COUPLINGS.noMutation },
|
||||
},
|
||||
]);
|
||||
}, 60_000);
|
||||
|
||||
it("couples a raw mutating statement issued inside a transaction", async () => {
|
||||
const values = stagedAllow();
|
||||
const marker = `raw-${randomUUID()}`;
|
||||
const context = createBoardKeyAuditContext();
|
||||
|
||||
await runWithBoardKeyAuditContext(context, async () => {
|
||||
stageBoardKeyAllowAudit(values);
|
||||
await db.transaction(async (tx) => {
|
||||
await tx.execute(sql`INSERT INTO board_key_atomicity_probe (value) VALUES (${marker})`);
|
||||
});
|
||||
});
|
||||
|
||||
expect(context.flushed).toBe(true);
|
||||
expect(await committedProbe(marker)).toHaveLength(1);
|
||||
expect(await committedAudit(values.boardApiKeyId)).toEqual([
|
||||
{
|
||||
decision: "allow",
|
||||
reason: "authorized",
|
||||
details: { coupling: BOARD_KEY_AUDIT_COUPLINGS.transaction },
|
||||
},
|
||||
]);
|
||||
}, 60_000);
|
||||
|
||||
it("marks a mutation that ran outside any transaction as uncoupled", async () => {
|
||||
const values = stagedAllow();
|
||||
const marker = `untransacted-${randomUUID()}`;
|
||||
const context = createBoardKeyAuditContext();
|
||||
|
||||
await runWithBoardKeyAuditContext(context, async () => {
|
||||
stageBoardKeyAllowAudit(values);
|
||||
await db.insert(probe).values({ value: marker });
|
||||
});
|
||||
await settleBoardKeyAuditContext(db, context, 201);
|
||||
|
||||
expect(context.untransacted).toBe(true);
|
||||
expect(await committedProbe(marker)).toHaveLength(1);
|
||||
expect(await committedAudit(values.boardApiKeyId)).toEqual([
|
||||
{
|
||||
decision: "allow",
|
||||
reason: "authorized",
|
||||
details: { coupling: BOARD_KEY_AUDIT_COUPLINGS.untransacted },
|
||||
},
|
||||
]);
|
||||
}, 60_000);
|
||||
|
||||
describe("through the live request pipeline", () => {
|
||||
const ownerId = randomUUID();
|
||||
|
||||
function createApp(keyId: string, handler: express.RequestHandler) {
|
||||
const app = express();
|
||||
app.use((req, _res, next) => {
|
||||
(req as unknown as { id: string }).id = randomUUID();
|
||||
(req as unknown as { actor: Record<string, unknown> }).actor = {
|
||||
type: "board",
|
||||
source: "board_key",
|
||||
userId: ownerId,
|
||||
keyId,
|
||||
boardKeyOwnerId: ownerId,
|
||||
boardKeyPrefix: "pcp_board_atomic",
|
||||
boardKeyScope: null,
|
||||
boardKeyLegacyUnrestricted: true,
|
||||
isInstanceAdmin: true,
|
||||
companyIds: [],
|
||||
memberships: [],
|
||||
};
|
||||
next();
|
||||
});
|
||||
app.use(boardKeyAuthorizationMiddleware(db));
|
||||
app.post("/api/companies", handler);
|
||||
app.use(errorHandler);
|
||||
return app;
|
||||
}
|
||||
|
||||
it("couples the gate decision to a controller transaction across the middleware chain", async () => {
|
||||
const marker = `pipeline-commit-${randomUUID()}`;
|
||||
const keyId = randomUUID();
|
||||
const app = createApp(keyId, async (_req, res) => {
|
||||
await db.transaction(async (tx) => {
|
||||
await tx.insert(probe).values({ value: marker });
|
||||
});
|
||||
res.status(201).json({ ok: true });
|
||||
});
|
||||
|
||||
const response = await request(app).post("/api/companies").send({});
|
||||
expect(response.status).toBe(201);
|
||||
expect(await committedProbe(marker)).toHaveLength(1);
|
||||
expect(await committedAudit(keyId)).toEqual([
|
||||
{
|
||||
decision: "allow",
|
||||
reason: "authorized",
|
||||
details: { coupling: BOARD_KEY_AUDIT_COUPLINGS.transaction },
|
||||
},
|
||||
]);
|
||||
}, 60_000);
|
||||
|
||||
it("leaves no allow disposition when the controller transaction fails", async () => {
|
||||
const marker = `pipeline-rollback-${randomUUID()}`;
|
||||
const keyId = randomUUID();
|
||||
const app = createApp(keyId, async (_req, _res) => {
|
||||
await db.transaction(async (tx) => {
|
||||
await tx.insert(probe).values({ value: marker });
|
||||
throw new Error("controller failed");
|
||||
});
|
||||
});
|
||||
|
||||
const response = await request(app).post("/api/companies").send({});
|
||||
expect(response.status).toBe(500);
|
||||
// Give the response-completion settlement a chance to run.
|
||||
await new Promise((resolve) => setTimeout(resolve, 100));
|
||||
expect(await committedProbe(marker)).toHaveLength(0);
|
||||
expect(await committedAudit(keyId)).toHaveLength(0);
|
||||
}, 60_000);
|
||||
|
||||
it("refuses to commit the mutation when the coupled audit write fails", async () => {
|
||||
const marker = `pipeline-audit-failure-${randomUUID()}`;
|
||||
// `board_api_key_id` is a uuid column, so this key identity makes the
|
||||
// coupled audit insert fail inside the controller's own transaction.
|
||||
const app = createApp("not-a-uuid", async (_req, res) => {
|
||||
await db.transaction(async (tx) => {
|
||||
await tx.insert(probe).values({ value: marker });
|
||||
});
|
||||
res.status(201).json({ ok: true });
|
||||
});
|
||||
|
||||
const response = await request(app).post("/api/companies").send({});
|
||||
expect(response.status).toBe(500);
|
||||
await new Promise((resolve) => setTimeout(resolve, 100));
|
||||
expect(await committedProbe(marker)).toHaveLength(0);
|
||||
}, 60_000);
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,300 @@
|
|||
import { AsyncLocalStorage } from "node:async_hooks";
|
||||
import { boardApiKeyAuthorizationEvents, type Db } from "@paperclipai/db";
|
||||
import { logger } from "../middleware/logger.js";
|
||||
|
||||
/**
|
||||
* Board-key allow dispositions must be atomic with the mutation they authorize.
|
||||
*
|
||||
* The central gate runs as request middleware, so it cannot see the transaction
|
||||
* a controller will open later. Writing the allow row in the middleware leaves
|
||||
* an `authorized` record behind whenever the controller rolls back or the
|
||||
* process dies mid-request, and gives a committed mutation no durable
|
||||
* disposition when the audit insert itself fails after the mutation.
|
||||
*
|
||||
* Instead the gate *stages* the allow disposition in a request-scoped context
|
||||
* and the instrumented database handle flushes it into the first transaction
|
||||
* that performs a domain mutation. From that point the disposition and the
|
||||
* mutation share one transaction: they commit together or roll back together.
|
||||
*
|
||||
* Denials are unaffected — they abort the request, so there is no mutation to
|
||||
* couple them to and they stay immediately durable.
|
||||
*/
|
||||
|
||||
export type BoardKeyAuditValues = typeof boardApiKeyAuthorizationEvents.$inferInsert;
|
||||
|
||||
/**
|
||||
* Fixed allowlist for the `details.coupling` audit field. The value records how
|
||||
* the durable allow disposition was tied to the request's effects so incident
|
||||
* reconstruction can tell an atomic record from a degraded one.
|
||||
*/
|
||||
export const BOARD_KEY_AUDIT_COUPLINGS = {
|
||||
/** Written inside the same transaction as the first domain mutation. */
|
||||
transaction: "transaction",
|
||||
/** The authorized request completed without mutating anything. */
|
||||
noMutation: "no_mutation",
|
||||
/** A mutation ran outside any transaction, so it could not be coupled. */
|
||||
untransacted: "untransacted",
|
||||
/** No request context was active (direct gate invocation, not a live route). */
|
||||
detached: "detached",
|
||||
} as const;
|
||||
|
||||
export type BoardKeyAuditCoupling =
|
||||
(typeof BOARD_KEY_AUDIT_COUPLINGS)[keyof typeof BOARD_KEY_AUDIT_COUPLINGS];
|
||||
|
||||
export type BoardKeyAuditContext = {
|
||||
/** Allow disposition awaiting a domain transaction to commit with. */
|
||||
pending: BoardKeyAuditValues | null;
|
||||
/** True once the staged disposition committed inside a domain transaction. */
|
||||
flushed: boolean;
|
||||
/** A domain mutation was issued at least once during this request. */
|
||||
mutationAttempted: boolean;
|
||||
/** A domain mutation was issued outside any transaction. */
|
||||
untransacted: boolean;
|
||||
/** Settlement already ran; further mutation marks are gate-owned writes. */
|
||||
settled: boolean;
|
||||
};
|
||||
|
||||
const store = new AsyncLocalStorage<BoardKeyAuditContext>();
|
||||
|
||||
// Mutating entry points on both the database handle and a transaction client.
|
||||
// `execute` covers raw SQL, which is also how several services mutate.
|
||||
const MUTATION_METHODS = ["insert", "update", "delete", "execute"] as const;
|
||||
const INSTRUMENTED = Symbol.for("paperclip.boardKeyAuditInstrumented");
|
||||
|
||||
// Raw SQL is the one entry point that can be read-only, so classify it instead
|
||||
// of reporting every `execute` as a mutation. Anything unrecognised counts as a
|
||||
// mutation: over-reporting keeps a disposition attached, under-reporting loses
|
||||
// one.
|
||||
const READ_ONLY_STATEMENT = /^\s*(?:select|show|explain|table|values)\b/i;
|
||||
|
||||
function rawStatementMutates(args: readonly unknown[]): boolean {
|
||||
const chunks = (args[0] as { queryChunks?: unknown[] } | null | undefined)?.queryChunks;
|
||||
if (!Array.isArray(chunks)) return true;
|
||||
let text = "";
|
||||
for (const chunk of chunks) {
|
||||
if (typeof chunk === "string") text += chunk;
|
||||
else {
|
||||
const parts = (chunk as { value?: unknown } | null)?.value;
|
||||
if (Array.isArray(parts)) text += parts.join("");
|
||||
}
|
||||
if (text.trim().length > 0) break;
|
||||
}
|
||||
return text.trim().length === 0 ? true : !READ_ONLY_STATEMENT.test(text);
|
||||
}
|
||||
|
||||
export function createBoardKeyAuditContext(): BoardKeyAuditContext {
|
||||
return {
|
||||
pending: null,
|
||||
flushed: false,
|
||||
mutationAttempted: false,
|
||||
untransacted: false,
|
||||
settled: false,
|
||||
};
|
||||
}
|
||||
|
||||
export function runWithBoardKeyAuditContext<T>(
|
||||
context: BoardKeyAuditContext,
|
||||
run: () => T,
|
||||
): T {
|
||||
return store.run(context, run);
|
||||
}
|
||||
|
||||
/**
|
||||
* Stage an allow disposition for transactional coupling. Returns false when no
|
||||
* request context is active, in which case the caller must fall back to an
|
||||
* immediate insert so the disposition is never silently dropped.
|
||||
*/
|
||||
export function stageBoardKeyAllowAudit(values: BoardKeyAuditValues): boolean {
|
||||
const context = store.getStore();
|
||||
if (!context) return false;
|
||||
context.pending = values;
|
||||
context.flushed = false;
|
||||
return true;
|
||||
}
|
||||
|
||||
function markMutation(context: BoardKeyAuditContext | undefined, transactional: boolean) {
|
||||
// Only requests carrying an uncoupled allow disposition need tracking, and
|
||||
// gate-owned settlement writes must never count as domain mutations.
|
||||
if (!context || !context.pending || context.settled) return;
|
||||
context.mutationAttempted = true;
|
||||
if (!transactional) context.untransacted = true;
|
||||
}
|
||||
|
||||
type MutatingClient = Record<string | symbol, unknown> & {
|
||||
transaction?: (fn: (tx: unknown) => unknown, config?: unknown) => unknown;
|
||||
};
|
||||
|
||||
/**
|
||||
* Replace a client's mutating methods with hooked versions. The replacements are
|
||||
* non-enumerable so the client keeps the shape callers and drizzle expect.
|
||||
*/
|
||||
function shadowMutationMethods(
|
||||
client: object,
|
||||
onMutation: (method: string, args: readonly unknown[]) => void,
|
||||
) {
|
||||
const target = client as MutatingClient;
|
||||
for (const method of MUTATION_METHODS) {
|
||||
const original = target[method];
|
||||
if (typeof original !== "function") continue;
|
||||
const bound = (original as (...args: unknown[]) => unknown).bind(client);
|
||||
Object.defineProperty(target, method, {
|
||||
configurable: true,
|
||||
enumerable: false,
|
||||
writable: true,
|
||||
value: (...args: unknown[]) => {
|
||||
if (method !== "execute" || rawStatementMutates(args)) onMutation(method, args);
|
||||
return bound(...args);
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Shadow the mutating methods of a transaction client so the staged allow
|
||||
* disposition is dispatched on the same connection immediately before the
|
||||
* first domain mutation. The client is created per transaction and discarded
|
||||
* afterwards, so in-place instrumentation stays scoped to this request.
|
||||
*
|
||||
* Nested transactions (savepoints) share the outermost client's flush, so a
|
||||
* savepoint never needs a disposition of its own. Residual: a mutation issued
|
||||
* only inside a savepoint dispatches the disposition within that savepoint, so
|
||||
* rolling the savepoint back while the outer transaction commits would drop it.
|
||||
* Nothing in the server opens nested transactions today; a caller that starts
|
||||
* doing so must flush at the outer level instead.
|
||||
*/
|
||||
function instrumentTransactionClient<T extends object>(
|
||||
client: T,
|
||||
onMutation: () => void,
|
||||
): T {
|
||||
shadowMutationMethods(client, onMutation);
|
||||
const target = client as unknown as MutatingClient;
|
||||
const nested = target.transaction;
|
||||
if (typeof nested === "function") {
|
||||
const boundNested = nested.bind(client);
|
||||
Object.defineProperty(target, "transaction", {
|
||||
configurable: true,
|
||||
enumerable: false,
|
||||
writable: true,
|
||||
value: (fn: (tx: unknown) => unknown, config?: unknown) =>
|
||||
boundNested((inner: unknown) => fn(instrumentTransactionClient(inner as object, onMutation)), config),
|
||||
});
|
||||
}
|
||||
return client;
|
||||
}
|
||||
|
||||
/**
|
||||
* Shadow `transaction` and the direct mutating methods on the process-wide
|
||||
* database handle. The handle object is returned unchanged (same identity, so
|
||||
* existing `dbOrTx === db` checks keep working) and every hook is inert unless
|
||||
* a board-key request has staged an allow disposition.
|
||||
*/
|
||||
export function instrumentDbForBoardKeyAudit<T extends Db>(db: T): T {
|
||||
const target = db as unknown as MutatingClient;
|
||||
if (target[INSTRUMENTED] === true) return db;
|
||||
Object.defineProperty(target, INSTRUMENTED, { value: true, enumerable: false });
|
||||
|
||||
shadowMutationMethods(db, () => markMutation(store.getStore(), false));
|
||||
|
||||
const originalTransaction = target.transaction;
|
||||
if (typeof originalTransaction === "function") {
|
||||
const boundTransaction = originalTransaction.bind(db);
|
||||
Object.defineProperty(target, "transaction", {
|
||||
configurable: true,
|
||||
enumerable: false,
|
||||
writable: true,
|
||||
value: (fn: (tx: unknown) => unknown, config?: unknown) => {
|
||||
const context = store.getStore();
|
||||
const pending = context && context.pending && !context.flushed ? context.pending : null;
|
||||
if (!context || !pending) return boundTransaction(fn, config);
|
||||
return runCoupledTransaction(boundTransaction, fn, config, context, pending);
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
return db;
|
||||
}
|
||||
|
||||
async function runCoupledTransaction(
|
||||
boundTransaction: (fn: (tx: unknown) => unknown, config?: unknown) => unknown,
|
||||
fn: (tx: unknown) => unknown,
|
||||
config: unknown,
|
||||
context: BoardKeyAuditContext,
|
||||
pending: BoardKeyAuditValues,
|
||||
) {
|
||||
let flush: Promise<unknown> | null = null;
|
||||
const result = await boundTransaction(async (tx: unknown) => {
|
||||
// Captured before instrumentation so the audit insert does not re-enter the
|
||||
// mutation hook it is triggered from.
|
||||
const rawInsert = (tx as {
|
||||
insert: (table: unknown) => {
|
||||
values: (values: unknown) => { execute?: () => Promise<unknown> } & PromiseLike<unknown>;
|
||||
};
|
||||
}).insert.bind(tx);
|
||||
const instrumented = instrumentTransactionClient(tx as object, () => {
|
||||
markMutation(context, true);
|
||||
if (flush) return;
|
||||
const query = rawInsert(boardApiKeyAuthorizationEvents).values({
|
||||
...pending,
|
||||
details: { ...(pending.details ?? {}), coupling: BOARD_KEY_AUDIT_COUPLINGS.transaction },
|
||||
});
|
||||
// Dispatched, not awaited, so the audit statement is queued on this
|
||||
// transaction's connection ahead of the mutation that triggered it.
|
||||
flush = typeof query.execute === "function" ? query.execute() : Promise.resolve(query);
|
||||
// The transaction wrapper awaits `flush` before committing; this guard
|
||||
// only keeps a rejection from surfacing as an unhandled rejection first.
|
||||
flush.catch(() => {});
|
||||
});
|
||||
const value = await fn(instrumented);
|
||||
// A failed audit insert aborts the transaction, so the mutation cannot
|
||||
// commit without its disposition.
|
||||
if (flush) await flush;
|
||||
return value;
|
||||
}, config);
|
||||
if (flush) context.flushed = true;
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the request's allow disposition once the response is complete.
|
||||
*
|
||||
* Nothing is written when a transactional mutation rolled back or the request
|
||||
* failed: there is no committed effect to attest, and a durable `allow` in that
|
||||
* situation is exactly the false association this coupling exists to prevent. A
|
||||
* durable allow therefore means "this mutation was authorized and committed";
|
||||
* denials remain logged in full regardless of outcome.
|
||||
*/
|
||||
export async function settleBoardKeyAuditContext(
|
||||
db: Db,
|
||||
context: BoardKeyAuditContext,
|
||||
statusCode: number,
|
||||
): Promise<void> {
|
||||
if (context.settled) return;
|
||||
context.settled = true;
|
||||
const pending = context.pending;
|
||||
if (!pending || context.flushed) return;
|
||||
|
||||
const succeeded = statusCode >= 200 && statusCode < 400;
|
||||
if (!succeeded) return;
|
||||
if (context.mutationAttempted && !context.untransacted) return;
|
||||
|
||||
const coupling = context.untransacted
|
||||
? BOARD_KEY_AUDIT_COUPLINGS.untransacted
|
||||
: BOARD_KEY_AUDIT_COUPLINGS.noMutation;
|
||||
if (context.untransacted) {
|
||||
logger.error(
|
||||
{ boardApiKeyId: pending.boardApiKeyId, action: pending.action },
|
||||
"Board-key mutation ran outside a transaction; allow disposition could not be coupled atomically",
|
||||
);
|
||||
}
|
||||
try {
|
||||
await db.insert(boardApiKeyAuthorizationEvents).values({
|
||||
...pending,
|
||||
details: { ...(pending.details ?? {}), coupling },
|
||||
});
|
||||
} catch (err) {
|
||||
logger.error(
|
||||
{ err, boardApiKeyId: pending.boardApiKeyId, action: pending.action },
|
||||
"Failed to persist settled board-key allow disposition",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
@ -12,6 +12,10 @@ import {
|
|||
import { BOARD_API_KEY_SCOPE_PRESETS } from "@paperclipai/shared";
|
||||
import { HttpError } from "../errors.js";
|
||||
import { boardAuthService, hashBearerToken } from "../services/board-auth.js";
|
||||
import {
|
||||
createBoardKeyAuditContext,
|
||||
runWithBoardKeyAuditContext,
|
||||
} from "./board-key-audit-coupling.js";
|
||||
import { authorizeBoardKey, lookupBoardKeyRoute } from "./board-key-route-registry.js";
|
||||
|
||||
const TOKEN = "pcp_board_authorization_matrix";
|
||||
|
|
@ -92,7 +96,10 @@ function createLiveAuthorityDb() {
|
|||
return { db, state, key, companyId, ownerId, audit };
|
||||
}
|
||||
|
||||
function requestFor(authentication: Awaited<ReturnType<ReturnType<typeof boardAuthService>["authenticateBoardApiKey"]>>) {
|
||||
function requestFor(
|
||||
authentication: Awaited<ReturnType<ReturnType<typeof boardAuthService>["authenticateBoardApiKey"]>>,
|
||||
method = "POST",
|
||||
) {
|
||||
if (!authentication.ok) throw new Error("Expected successful board-key authentication");
|
||||
const { key, scopeConfig, access } = authentication;
|
||||
const companyIds = scopeConfig
|
||||
|
|
@ -100,6 +107,7 @@ function requestFor(authentication: Awaited<ReturnType<ReturnType<typeof boardAu
|
|||
: access.companyIds;
|
||||
return {
|
||||
id: randomUUID(),
|
||||
method,
|
||||
actor: {
|
||||
type: "board",
|
||||
source: "board_key",
|
||||
|
|
@ -121,26 +129,47 @@ async function expectDenied(run: Promise<unknown>, status: number) {
|
|||
}
|
||||
|
||||
describe("board-key effective authority", () => {
|
||||
it("audits an allow before the protected side effect runs", async () => {
|
||||
it("stages a mutating allow for its transaction instead of auditing ahead of the mutation", async () => {
|
||||
const { db, companyId, audit } = createLiveAuthorityDb();
|
||||
const authentication = await boardAuthService(db).authenticateBoardApiKey(TOKEN);
|
||||
const req = requestFor(authentication);
|
||||
const metadata = lookupBoardKeyRoute("POST", `/api/companies/${companyId}/issues`);
|
||||
const sideEffect = vi.fn();
|
||||
const context = createBoardKeyAuditContext();
|
||||
|
||||
await authorizeBoardKey(
|
||||
await runWithBoardKeyAuditContext(context, () => authorizeBoardKey(
|
||||
db,
|
||||
req,
|
||||
metadata.action,
|
||||
async () => ({ companyId, resourceType: "company", resourceId: companyId }),
|
||||
metadata,
|
||||
);
|
||||
expect(audit.at(-1)).toMatchObject({ decision: "allow", action: "issues:write" });
|
||||
sideEffect();
|
||||
expect(sideEffect).toHaveBeenCalledOnce();
|
||||
));
|
||||
|
||||
// Nothing durable yet: the disposition commits with the mutation it
|
||||
// authorizes, so a rolled-back controller leaves no `authorized` record.
|
||||
expect(audit).toHaveLength(0);
|
||||
expect(context.pending).toMatchObject({ decision: "allow", action: "issues:write" });
|
||||
});
|
||||
|
||||
it("fails closed before side effects when the allow audit cannot be persisted", async () => {
|
||||
it("audits a read-only allow immediately because it has no mutation to couple to", async () => {
|
||||
const { db, companyId, audit } = createLiveAuthorityDb();
|
||||
const authentication = await boardAuthService(db).authenticateBoardApiKey(TOKEN);
|
||||
const req = requestFor(authentication, "GET");
|
||||
const metadata = lookupBoardKeyRoute("GET", `/api/companies/${companyId}/issues`);
|
||||
const context = createBoardKeyAuditContext();
|
||||
|
||||
await runWithBoardKeyAuditContext(context, () => authorizeBoardKey(
|
||||
db,
|
||||
req,
|
||||
metadata.action,
|
||||
async () => ({ companyId, resourceType: "company", resourceId: companyId }),
|
||||
metadata,
|
||||
));
|
||||
|
||||
expect(context.pending).toBeNull();
|
||||
expect(audit.at(-1)).toMatchObject({ decision: "allow", action: "issues:read" });
|
||||
});
|
||||
|
||||
it("fails closed before side effects when a detached allow audit cannot be persisted", async () => {
|
||||
const { db, companyId } = createLiveAuthorityDb();
|
||||
const authentication = await boardAuthService(db).authenticateBoardApiKey(TOKEN);
|
||||
const req = requestFor(authentication);
|
||||
|
|
@ -148,6 +177,8 @@ describe("board-key effective authority", () => {
|
|||
db.insert = vi.fn(() => ({ values: vi.fn().mockRejectedValue(new Error("audit unavailable")) }));
|
||||
const sideEffect = vi.fn();
|
||||
|
||||
// No request context, so the gate cannot couple the disposition to a
|
||||
// transaction and must persist it before the caller proceeds.
|
||||
try {
|
||||
await authorizeBoardKey(
|
||||
db,
|
||||
|
|
|
|||
|
|
@ -38,6 +38,13 @@ import {
|
|||
import { isUuidLike, type BoardPermissionKey, type PermissionKey } from "@paperclipai/shared";
|
||||
import { HttpError, forbidden, notFound } from "../errors.js";
|
||||
import { logger } from "../middleware/logger.js";
|
||||
import {
|
||||
BOARD_KEY_AUDIT_COUPLINGS,
|
||||
createBoardKeyAuditContext,
|
||||
runWithBoardKeyAuditContext,
|
||||
settleBoardKeyAuditContext,
|
||||
stageBoardKeyAllowAudit,
|
||||
} from "./board-key-audit-coupling.js";
|
||||
|
||||
export type BoardKeyRouteClassification =
|
||||
| "company"
|
||||
|
|
@ -623,7 +630,7 @@ async function auditDecision(
|
|||
reason: string,
|
||||
) {
|
||||
if (req.actor.source !== "board_key" || !req.actor.keyId || !req.actor.boardKeyOwnerId) return;
|
||||
await db.insert(boardApiKeyAuthorizationEvents).values({
|
||||
const values = {
|
||||
boardApiKeyId: req.actor.keyId,
|
||||
ownerUserId: req.actor.boardKeyOwnerId,
|
||||
tokenPrefix: req.actor.boardKeyPrefix ?? null,
|
||||
|
|
@ -636,8 +643,21 @@ async function auditDecision(
|
|||
reason,
|
||||
requestId: typeof req.id === "string" ? req.id : null,
|
||||
runId: isUuidLike(req.actor.runId) ? req.actor.runId : null,
|
||||
details: {},
|
||||
});
|
||||
details: {} as Record<string, string | boolean | null>,
|
||||
};
|
||||
// An allow on an unsafe method authorizes a mutation, so its disposition is
|
||||
// staged and committed inside that mutation's transaction. Denials abort the
|
||||
// request and safe methods have no mutation to couple to, so both stay
|
||||
// immediately durable.
|
||||
if (decision === "allow" && !SAFE_METHODS.has(req.method) && stageBoardKeyAllowAudit(values)) return;
|
||||
if (decision === "allow" && !SAFE_METHODS.has(req.method)) {
|
||||
logger.warn(
|
||||
{ boardApiKeyId: req.actor.keyId, action: metadata.action },
|
||||
"Board-key allow audited without a request context; disposition is not transactionally coupled",
|
||||
);
|
||||
values.details = { coupling: BOARD_KEY_AUDIT_COUPLINGS.detached };
|
||||
}
|
||||
await db.insert(boardApiKeyAuthorizationEvents).values(values);
|
||||
}
|
||||
|
||||
async function denyBoardKey(
|
||||
|
|
@ -746,23 +766,36 @@ export async function authorizeBoardKey(
|
|||
}
|
||||
|
||||
export function boardKeyAuthorizationMiddleware(db: Db): RequestHandler {
|
||||
return async (req, _res, next) => {
|
||||
return async (req, res, next) => {
|
||||
if (req.actor.source !== "board_key") {
|
||||
next();
|
||||
return;
|
||||
}
|
||||
const metadata = lookupBoardKeyRoute(req.method, req.originalUrl);
|
||||
try {
|
||||
await authorizeBoardKey(
|
||||
db,
|
||||
req,
|
||||
metadata.action,
|
||||
() => resolveAuthoritativeResource(db, metadata),
|
||||
metadata,
|
||||
);
|
||||
next();
|
||||
} catch (err) {
|
||||
next(err);
|
||||
}
|
||||
const context = createBoardKeyAuditContext();
|
||||
// A staged allow disposition that never reached a domain transaction is
|
||||
// resolved once the response is complete: recorded when the request
|
||||
// succeeded without a rollback, dropped otherwise.
|
||||
const settle = (statusCode: number) => {
|
||||
void settleBoardKeyAuditContext(db, context, statusCode);
|
||||
};
|
||||
res.on("finish", () => settle(res.statusCode));
|
||||
res.on("close", () => settle(res.writableEnded ? res.statusCode : 0));
|
||||
// The context stays active across the downstream handler chain, so the
|
||||
// instrumented database handle can couple the disposition to its mutation.
|
||||
await runWithBoardKeyAuditContext(context, async () => {
|
||||
try {
|
||||
await authorizeBoardKey(
|
||||
db,
|
||||
req,
|
||||
metadata.action,
|
||||
() => resolveAuthoritativeResource(db, metadata),
|
||||
metadata,
|
||||
);
|
||||
next();
|
||||
} catch (err) {
|
||||
next(err);
|
||||
}
|
||||
});
|
||||
};
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue