mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 07:12:00 +01:00
perf(issues): decide task reads for a list in one batch (#20858)
The task routes that filter several rows for one actor (summaries, lists, blockers, relation edges and the chat bootstrap) asked for one authorization decision per task, all at the same time. Each decision sent a task-row read and a copy of the task-privacy statement, which is more than 1 KB of SQL, so a request for 50 summaries sent 127 to 279 statements on every pool connection at once. The routes now ask the authorization service for all decisions in one batched read. It reads the task rows and applies the same privacy predicate to all of them in one statement, and reads membership, agent and run policy once. Within a batch, the authorizing user's membership is read once, and the boundary checks of a low-trust agent, which stay per task, run at most eight at a time. A request for 50 summaries now sends 22 to 28 statements (65 for a low-trust agent). Tests compare the batch with one check per task for 24 actors. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
This commit is contained in:
1 parent
c8ae02e5ce
commit
464e1de9b1
9 files changed
+738
-29
No files matched your search
@@ -597,6 +597,15 @@ authority once. Different task bindings must both pass. The batch is discarded
|
||||
after that delivery; later events still recheck grants, membership, task
|
||||
existence, and company boundaries.
|
||||
|
||||
Task routes that filter several rows for one actor (summaries, lists, blockers,
|
||||
relation edges, and the chat bootstrap) use one fresh authorization batch per
|
||||
call. The batch reads the task rows and applies the task-privacy predicate to
|
||||
all of them in one statement. It reads membership, agent, and run policy once.
|
||||
The number of statements does not grow with the number of tasks. The boundary
|
||||
checks of a low-trust agent (ancestry and mention grants) stay per task and run
|
||||
at most eight at a time. The batch is discarded after the call; the next
|
||||
request reads current grants, membership, and privacy again.
|
||||
|
||||
Browser progress events update cached live run status directly. A successful
|
||||
patch does not refetch task details, activity, or run history. Unknown runs
|
||||
retain the existing refresh fallback. Run transitions, activity events, and
|
||||
|
||||
@@ -0,0 +1,212 @@
|
||||
/**
|
||||
* Local proof for the batched task-read filter of `GET /companies/:id/issues/summaries`.
|
||||
*
|
||||
* The route reads up to 50 task summaries and then decides, task by task,
|
||||
* whether the actor may read each one. The pre-fix route asked for one
|
||||
* authorization decision per task at the same time, so a request for N tasks
|
||||
* sent N task-row reads and N copies of the >1 KB task-privacy statement (plus
|
||||
* N membership reads for a cloud tenant user) on every pool connection at
|
||||
* once. The fix asks for all decisions in one batched authorization read.
|
||||
*
|
||||
* The pre-fix route module is loaded from --baseline-ref and mounted beside
|
||||
* the working tree's route on the same database. The script fails unless both
|
||||
* return the same summaries for every actor.
|
||||
*
|
||||
* pnpm --filter @paperclipai/server exec tsx scripts/benchmark-issue-summaries-read-filter.ts --baseline-ref=<pre-fix commit>
|
||||
*
|
||||
* Optional sizes: IDS (default 50, the route maximum), NOISE_ISSUES (5000),
|
||||
* POOL (10), REQUESTS (40), CONCURRENT (8), WAVES (10).
|
||||
*/
|
||||
import assert from "node:assert/strict";
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { promises as fs } from "node:fs";
|
||||
import type { AddressInfo } from "node:net";
|
||||
import path from "node:path";
|
||||
import { performance } from "node:perf_hooks";
|
||||
import { fileURLToPath, pathToFileURL } from "node:url";
|
||||
import express from "express";
|
||||
import { drizzle } from "drizzle-orm/postgres-js";
|
||||
import { createDb, startEmbeddedPostgresTestDatabase, type Db } from "../../packages/db/src/index.js";
|
||||
import { ISSUE_SUMMARIES_MAX_IDS } from "../../packages/shared/src/index.js";
|
||||
import { errorHandler } from "../src/middleware/index.js";
|
||||
import { issueRoutes } from "../src/routes/issues.js";
|
||||
import type { AuthorizationActor } from "../src/services/authorization.js";
|
||||
import {
|
||||
seedIssueReadVisibilityFixture, type IssueReadVisibilityActorName,
|
||||
} from "../src/__tests__/helpers/issue-read-visibility-fixture.js";
|
||||
|
||||
const size = (name: string, fallback: number) => Number(process.env[name] ?? fallback);
|
||||
const IDS = size("IDS", ISSUE_SUMMARIES_MAX_IDS), NOISE_ISSUES = size("NOISE_ISSUES", 5_000), POOL = size("POOL", 10);
|
||||
const REQUESTS = size("REQUESTS", 40), CONCURRENT = size("CONCURRENT", 8), WAVES = size("WAVES", 10);
|
||||
for (const [name, value] of Object.entries({ IDS, NOISE_ISSUES, POOL, REQUESTS, CONCURRENT, WAVES })) {
|
||||
assert.ok(Number.isSafeInteger(value) && value > 0, `${name} must be a positive integer`);
|
||||
}
|
||||
assert.ok(IDS <= ISSUE_SUMMARIES_MAX_IDS, `IDS must be at most ${ISSUE_SUMMARIES_MAX_IDS}`);
|
||||
const baselineRef = process.argv.find(arg => arg.startsWith("--baseline-ref="))?.slice(15);
|
||||
assert.ok(baselineRef, "Pass an explicit pre-fix --baseline-ref=<commit>");
|
||||
const root = fileURLToPath(new URL("../../", import.meta.url));
|
||||
const git = (...args: string[]) => execFileSync("git", args, { cwd: root, encoding: "utf8", maxBuffer: 64 * 1024 * 1024 });
|
||||
const baselineRevision = git("rev-parse", "--verify", `${baselineRef}^{commit}`).trim();
|
||||
const routeFile = "server/src/routes/issues.ts";
|
||||
/** The actors measured: a cloud tenant person, a self-hosted person, agent runs and a low-trust agent. */
|
||||
const ACTORS = [
|
||||
"cloud tenant owner", "board operator", "agent run for operator", "agent authorized by operator",
|
||||
"low-trust agent bounded to a subtree",
|
||||
] as const satisfies readonly IssueReadVisibilityActorName[];
|
||||
type Arm = "baseline" | "treatment";
|
||||
|
||||
process.env.PAPERCLIP_ISSUE_PRIVACY_MODE = "enforce";
|
||||
const keepAlive = setInterval(() => {}, 1_000);
|
||||
let temporary: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | undefined;
|
||||
let db: Db | undefined;
|
||||
let baselineDir: string | undefined;
|
||||
const servers: Array<{ close: () => void }> = [];
|
||||
try {
|
||||
// A pre-fix copy of the route module. Its relative imports resolve to the
|
||||
// current tree. The fix also touches the authorization service, but only
|
||||
// inside a batched read, which the pre-fix route never asks for.
|
||||
baselineDir = await fs.mkdtemp(path.join(root, "server/scripts/.issue-summaries-baseline-"));
|
||||
const resolveImport = (specifier: string) => JSON.stringify(fileURLToPath(new URL(specifier, pathToFileURL(path.join(root, routeFile)))));
|
||||
const source = git("show", `${baselineRevision}:${routeFile}`)
|
||||
.replace(/from "(\.{1,2}\/[^"]+)"/g, (_match, specifier: string) => `from ${resolveImport(specifier)}`)
|
||||
.replace(/import\("(\.{1,2}\/[^"]+)"\)/g, (_match, specifier: string) => `import(${resolveImport(specifier)})`);
|
||||
await fs.writeFile(path.join(baselineDir, "issues.ts"), source);
|
||||
const baselineRoutes = (await import(pathToFileURL(path.join(baselineDir, "issues.ts")).href) as
|
||||
{ issueRoutes: typeof issueRoutes }).issueRoutes;
|
||||
const routes: Record<Arm, typeof issueRoutes> = { baseline: baselineRoutes, treatment: issueRoutes };
|
||||
|
||||
temporary = await startEmbeddedPostgresTestDatabase("issue-summaries-benchmark-");
|
||||
db = createDb(temporary.connectionString, { maxConnections: POOL });
|
||||
const sql = db.$client;
|
||||
const loadStart = performance.now();
|
||||
const f = await seedIssueReadVisibilityFixture(db, { publicCount: Math.max(IDS, 8) });
|
||||
// The rest of the company's task history, a third of it private to other people.
|
||||
await sql.unsafe(`insert into issues (id, company_id, title, status, visibility, privacy_root_issue_id, created_by_user_id)
|
||||
select id, $1::uuid, 'History ' || n, 'done', case when n % 3 = 0 then 'private' else 'open' end,
|
||||
case when n % 3 = 0 then id end, $3
|
||||
from (select gen_random_uuid() id, n from generate_series(1, $2::int) n) history`,
|
||||
[f.companyId, NOISE_ISSUES, f.strangerUserId]);
|
||||
await sql.unsafe("vacuum analyze");
|
||||
const loadMs = performance.now() - loadStart;
|
||||
|
||||
// What one screen asks for: mostly open tasks, private tasks the actor may and
|
||||
// may not read (roots and descendants), a blocked task, and IDs of another
|
||||
// company and of nothing, which must be omitted.
|
||||
const special = [
|
||||
...f.ownedPrivateIds, ...f.grantedPrivateIds, ...f.assignedPrivateIds, ...f.unreadablePrivateIds,
|
||||
...f.privateProjectIssueIds, ...f.blockedIds, ...f.foreignIds, ...f.missingIds,
|
||||
].slice(0, Math.floor(IDS / 2));
|
||||
const ids = [...special, ...f.publicIds].slice(0, IDS);
|
||||
assert.equal(new Set(ids).size, IDS);
|
||||
|
||||
// One HTTP server per arm and actor; the handler time is taken inside the process.
|
||||
const handlerMs: number[] = [];
|
||||
async function listen(arm: Arm, actor: AuthorizationActor, target: Db) {
|
||||
const app = express();
|
||||
app.use((req, res, next) => {
|
||||
const start = performance.now();
|
||||
res.on("finish", () => handlerMs.push(performance.now() - start));
|
||||
req.actor = { ...actor } as Express.Request["actor"];
|
||||
next();
|
||||
});
|
||||
app.use("/api", routes[arm](target, {} as never));
|
||||
app.use(errorHandler);
|
||||
const server = app.listen(0, "127.0.0.1");
|
||||
servers.push(server);
|
||||
await new Promise<void>((resolve, reject) => { server.once("listening", resolve); server.once("error", reject); });
|
||||
const url = `http://127.0.0.1:${(server.address() as AddressInfo).port}/api/companies/${f.companyId}/issues/summaries?ids=${ids.join(",")}`;
|
||||
return async () => {
|
||||
const response = await fetch(url);
|
||||
assert.equal(response.status, 200);
|
||||
return await response.json() as Array<{ id: string }>;
|
||||
};
|
||||
}
|
||||
const statements: string[] = [];
|
||||
const recorded = drizzle(sql, { schema: db._.fullSchema, logger: { logQuery: query => statements.push(query) } }) as unknown as Db;
|
||||
|
||||
const round = (value: number) => Number(value.toFixed(2));
|
||||
const stats = (samples: number[]) => {
|
||||
const sorted = [...samples].sort((a, b) => a - b);
|
||||
return { medianMs: round(sorted[Math.floor(sorted.length / 2)]!), p95Ms: round(sorted[Math.min(sorted.length - 1, Math.floor(sorted.length * 0.95))]!),
|
||||
maxMs: round(sorted.at(-1)!) };
|
||||
};
|
||||
/** Main-thread and process CPU spent while `run` executes. */
|
||||
async function measure(run: () => Promise<unknown>) {
|
||||
const loop = performance.eventLoopUtilization(), cpu = process.cpuUsage(), start = performance.now();
|
||||
handlerMs.length = 0;
|
||||
await run();
|
||||
const used = process.cpuUsage(cpu), busy = performance.eventLoopUtilization(loop);
|
||||
return { wallMs: performance.now() - start, cpuMs: (used.user + used.system) / 1000, mainThreadMs: busy.active, handlerMs: [...handlerMs] };
|
||||
}
|
||||
|
||||
const report: Record<string, unknown> = {};
|
||||
for (const name of ACTORS) {
|
||||
const actor = f.actors[name];
|
||||
const request = { baseline: await listen("baseline", actor, db), treatment: await listen("treatment", actor, db) };
|
||||
const counted = { baseline: await listen("baseline", actor, recorded), treatment: await listen("treatment", actor, recorded) };
|
||||
const bodies = { baseline: await request.baseline(), treatment: await request.treatment() };
|
||||
assert.deepEqual(bodies.treatment, bodies.baseline, `${name}: both versions must return the same summaries`);
|
||||
const returned = new Set(bodies.baseline.map(row => row.id));
|
||||
for (const id of [...f.unreadablePrivateIds, ...f.foreignIds, ...f.missingIds]) assert.ok(!returned.has(id), `${name}: ${id} must be omitted`);
|
||||
assert.ok(returned.size > 0 && returned.size < IDS, `${name}: the request must mix readable and unreadable tasks`);
|
||||
|
||||
const perArm: Record<Arm, Record<string, unknown>> = { baseline: {}, treatment: {} };
|
||||
for (const arm of ["baseline", "treatment"] as const) {
|
||||
statements.length = 0;
|
||||
await counted[arm]();
|
||||
const privacy = statements.filter(statement => /^select "id" from "issues" where \("issues"\."id" = \$1 and "issues"\."company_id" = \$2 and \(*exists/.test(statement));
|
||||
perArm[arm].statementsPerRequest = statements.length;
|
||||
perArm[arm].perTaskPrivacyStatements = privacy.length;
|
||||
perArm[arm].statementBytesPerRequest = statements.reduce((sum, statement) => sum + Buffer.byteLength(statement), 0);
|
||||
}
|
||||
for (const arm of ["baseline", "treatment"] as const) for (let warm = 0; warm < 5; warm++) await request[arm]();
|
||||
const samples: Record<Arm, { handler: number[]; cpu: number[]; main: number[]; wave: number[]; waveHandler: number[]; waveCpu: number[] }> = {
|
||||
baseline: { handler: [], cpu: [], main: [], wave: [], waveHandler: [], waveCpu: [] },
|
||||
treatment: { handler: [], cpu: [], main: [], wave: [], waveHandler: [], waveCpu: [] },
|
||||
};
|
||||
const blocks = 4, perBlock = Math.ceil(REQUESTS / blocks);
|
||||
for (let block = 0; block < blocks; block++) {
|
||||
for (const arm of (block % 2 === 0 ? ["baseline", "treatment"] : ["treatment", "baseline"]) as Arm[]) {
|
||||
const result = await measure(async () => { for (let index = 0; index < perBlock; index++) await request[arm](); });
|
||||
samples[arm].handler.push(...result.handlerMs);
|
||||
samples[arm].cpu.push(result.cpuMs / perBlock);
|
||||
samples[arm].main.push(result.mainThreadMs / perBlock);
|
||||
}
|
||||
}
|
||||
for (let wave = 0; wave < WAVES; wave++) {
|
||||
for (const arm of (wave % 2 === 0 ? ["baseline", "treatment"] : ["treatment", "baseline"]) as Arm[]) {
|
||||
const result = await measure(() => Promise.all(Array.from({ length: CONCURRENT }, () => request[arm]())));
|
||||
samples[arm].wave.push(result.wallMs);
|
||||
samples[arm].waveHandler.push(...result.handlerMs);
|
||||
samples[arm].waveCpu.push(result.cpuMs / CONCURRENT);
|
||||
}
|
||||
}
|
||||
const mean = (values: number[]) => round(values.reduce((sum, value) => sum + value, 0) / values.length);
|
||||
for (const arm of ["baseline", "treatment"] as const) {
|
||||
Object.assign(perArm[arm], {
|
||||
oneAtATime: { serverTime: stats(samples[arm].handler), processCpuMsPerRequest: mean(samples[arm].cpu),
|
||||
mainThreadBusyMsPerRequest: mean(samples[arm].main) },
|
||||
concurrent: { serverTime: stats(samples[arm].waveHandler), waveWallTime: stats(samples[arm].wave),
|
||||
processCpuMsPerRequest: mean(samples[arm].waveCpu) },
|
||||
});
|
||||
}
|
||||
report[name] = { summariesReturned: returned.size, idsRequested: IDS, ...perArm };
|
||||
}
|
||||
|
||||
console.log(JSON.stringify({
|
||||
baselineRevision, treatmentRevision: git("rev-parse", "HEAD").trim(),
|
||||
treatmentDirty: git("status", "--porcelain", "--", routeFile, "server/src/services").trim().length > 0,
|
||||
postgresVersion: (await sql.unsafe("select version()"))[0]!.version, node: process.version,
|
||||
fixture: { synthetic: true, idsPerRequest: IDS, companyTasks: f.companyIssueIds.length + NOISE_ISSUES, pool: POOL,
|
||||
requestsPerArm: REQUESTS, concurrentRequests: CONCURRENT, waves: WAVES, loadMs: Math.round(loadMs) },
|
||||
correctness: { sameSummariesForEveryActor: true },
|
||||
results: report,
|
||||
}, null, 2));
|
||||
} finally {
|
||||
try {
|
||||
for (const server of servers) server.close();
|
||||
await db?.$client.end({ timeout: 0 });
|
||||
await temporary?.cleanup();
|
||||
if (baselineDir) await fs.rm(baselineDir, { recursive: true, force: true });
|
||||
} finally { clearInterval(keepAlive); }
|
||||
}
|
||||
@@ -13,10 +13,16 @@ const mockIssueService = vi.hoisted(() => ({
|
||||
getById: vi.fn(),
|
||||
}));
|
||||
|
||||
const mockAccessService = vi.hoisted(() => ({
|
||||
decide: vi.fn(),
|
||||
hasPermission: vi.fn(),
|
||||
}));
|
||||
const mockAccessService = vi.hoisted(() => {
|
||||
const decide = vi.fn();
|
||||
return {
|
||||
decide,
|
||||
// The batched read asks the mocked decision once per task.
|
||||
decideReads: async (actor: unknown, resources: unknown[]) =>
|
||||
Promise.all(resources.map((resource) => decide({ actor, action: "issue:read", resource }))),
|
||||
hasPermission: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
const mockAgentService = vi.hoisted(() => ({
|
||||
list: vi.fn(),
|
||||
|
||||
@@ -0,0 +1,199 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { eq } from "drizzle-orm";
|
||||
import {
|
||||
agents, authUsers, companies, companyMemberships, heartbeatRuns, issueAccessGrants, issueRelations, issues,
|
||||
projectAccessMembers, projects, type Db,
|
||||
} from "@paperclipai/db";
|
||||
import type { AuthorizationActor } from "../../services/authorization.js";
|
||||
|
||||
/** Every actor type and membership role that can reach a task read. */
|
||||
export const ISSUE_READ_VISIBILITY_ACTORS = [
|
||||
"board owner", "board admin", "board operator", "board viewer", "board member", "board key operator",
|
||||
"cloud tenant owner", "cloud tenant viewer", "instance admin", "local implicit board", "board without membership",
|
||||
"agent key", "assignee agent key", "agent run for owner", "agent run for operator", "agent run for viewer",
|
||||
"agent run for owner, membership read from the database", "agent run for a user without membership",
|
||||
"agent authorized by operator", "agent authorized by owner for owner",
|
||||
"low-trust agent bounded to tasks", "low-trust agent bounded to a subtree",
|
||||
"agent of another company", "unauthenticated",
|
||||
] as const;
|
||||
export type IssueReadVisibilityActorName = (typeof ISSUE_READ_VISIBILITY_ACTORS)[number];
|
||||
|
||||
export type IssueReadVisibilityFixture = Awaited<ReturnType<typeof seedIssueReadVisibilityFixture>>;
|
||||
|
||||
/**
|
||||
* One company whose tasks cover every way a task read is allowed or denied:
|
||||
* open tasks, private tasks readable through creation, assignment, a grant or
|
||||
* a private ancestor, private tasks nobody in the fixture may read, tasks of a
|
||||
* private project, a hidden task, tasks of another company and missing IDs.
|
||||
* `publicCount` sizes the open group; the other groups are fixed.
|
||||
*/
|
||||
export async function seedIssueReadVisibilityFixture(db: Db, input: { publicCount?: number } = {}) {
|
||||
const companyId = randomUUID(), foreignCompanyId = randomUUID();
|
||||
const tag = companyId.replaceAll("-", "").slice(0, 6).toUpperCase();
|
||||
const foreignTag = foreignCompanyId.replaceAll("-", "").slice(0, 6).toUpperCase();
|
||||
await db.insert(companies).values([
|
||||
{ id: companyId, name: `Visibility ${companyId}`, issuePrefix: `V${tag}`, requireBoardApprovalForNewAgents: false },
|
||||
{ id: foreignCompanyId, name: `Foreign ${foreignCompanyId}`, issuePrefix: `F${foreignTag}`, requireBoardApprovalForNewAgents: false },
|
||||
]);
|
||||
|
||||
const roles = ["owner", "admin", "operator", "viewer", "member"] as const;
|
||||
const users = Object.fromEntries(roles.map((role) => [role, `user-${role}-${randomUUID()}`])) as Record<(typeof roles)[number], string>;
|
||||
const outsiderUserId = `user-outsider-${randomUUID()}`, strangerUserId = `user-stranger-${randomUUID()}`;
|
||||
const now = new Date();
|
||||
await db.insert(authUsers).values([...Object.values(users), outsiderUserId, strangerUserId].map((id) => ({
|
||||
id, name: id, email: `${id}@example.test`, emailVerified: true, createdAt: now, updatedAt: now,
|
||||
})));
|
||||
await db.insert(companyMemberships).values([
|
||||
...roles.map((role) => ({ companyId, principalType: "user", principalId: users[role], status: "active", membershipRole: role })),
|
||||
{ companyId, principalType: "user", principalId: strangerUserId, status: "active", membershipRole: "operator" },
|
||||
{ companyId: foreignCompanyId, principalType: "user", principalId: outsiderUserId, status: "active", membershipRole: "owner" },
|
||||
]);
|
||||
|
||||
const readerAgentId = randomUUID(), assigneeAgentId = randomUUID(), lowTrustAgentId = randomUUID();
|
||||
const lowTrustRootAgentId = randomUUID(), foreignAgentId = randomUUID();
|
||||
await db.insert(agents).values([
|
||||
{ id: readerAgentId, companyId, name: "Reader", role: "engineer", adapterType: "process" },
|
||||
{ id: assigneeAgentId, companyId, name: "Assignee", role: "engineer", adapterType: "process" },
|
||||
{ id: lowTrustAgentId, companyId, name: "Low trust", role: "engineer", adapterType: "process" },
|
||||
{ id: lowTrustRootAgentId, companyId, name: "Low trust root", role: "engineer", adapterType: "process" },
|
||||
{ id: foreignAgentId, companyId: foreignCompanyId, name: "Foreign", role: "engineer", adapterType: "process" },
|
||||
]);
|
||||
|
||||
const openProjectId = randomUUID(), privateProjectId = randomUUID();
|
||||
await db.insert(projects).values([
|
||||
{ id: openProjectId, companyId, name: "Open project" },
|
||||
{ id: privateProjectId, companyId, name: "Private project", visibility: "private" },
|
||||
]);
|
||||
await db.insert(projectAccessMembers).values({ companyId, projectId: privateProjectId, subjectType: "user", subjectId: users.admin });
|
||||
|
||||
const publicCount = input.publicCount ?? 6;
|
||||
const publicIds = Array.from({ length: publicCount }, () => randomUUID());
|
||||
const boundaryRootId = publicIds[0]!, boundaryChildId = publicIds[1]!;
|
||||
await db.insert(issues).values(publicIds.map((id, index) => ({
|
||||
id, companyId, title: `Open ${index}`, status: "todo",
|
||||
projectId: index % 3 === 2 ? openProjectId : null,
|
||||
parentId: index === 1 ? boundaryRootId : null,
|
||||
assigneeAgentId: index % 4 === 3 ? assigneeAgentId : null,
|
||||
createdByUserId: strangerUserId,
|
||||
})));
|
||||
|
||||
/** A private root and two generations below it. Descendants inherit privacy and are readable only through an ancestor. */
|
||||
async function privateTree(title: string, root: Partial<typeof issues.$inferInsert>) {
|
||||
const rootId = randomUUID(), childId = randomUUID(), grandchildId = randomUUID();
|
||||
await db.insert(issues).values({
|
||||
id: rootId, companyId, title: `${title} root`, status: "todo", visibility: "private", privacyRootIssueId: rootId,
|
||||
createdByUserId: strangerUserId, responsibleUserId: strangerUserId, ...root,
|
||||
});
|
||||
await db.insert(issues).values({
|
||||
id: childId, companyId, title: `${title} child`, status: "todo", parentId: rootId, visibility: "private",
|
||||
privacyRootIssueId: rootId, privacyParentIssueId: rootId, createdByUserId: strangerUserId,
|
||||
});
|
||||
await db.insert(issues).values({
|
||||
id: grandchildId, companyId, title: `${title} grandchild`, status: "todo", parentId: childId, visibility: "private",
|
||||
privacyRootIssueId: rootId, privacyParentIssueId: childId, createdByUserId: strangerUserId,
|
||||
});
|
||||
return [rootId, childId, grandchildId];
|
||||
}
|
||||
const ownedPrivateIds = await privateTree("Owner private", { createdByUserId: users.owner, responsibleUserId: users.owner });
|
||||
const grantedPrivateIds = await privateTree("Granted private", {});
|
||||
await db.insert(issueAccessGrants).values([
|
||||
{ issueId: grantedPrivateIds[0]!, subjectType: "user", subjectId: users.operator, source: "explicit" },
|
||||
{ issueId: grantedPrivateIds[0]!, subjectType: "agent", subjectId: readerAgentId, source: "explicit" },
|
||||
// A revoked grant must stay ineffective on both paths.
|
||||
{ issueId: grantedPrivateIds[0]!, subjectType: "user", subjectId: users.viewer, source: "explicit", revokedAt: now },
|
||||
]);
|
||||
const assignedPrivateIds = await privateTree("Assigned private", { assigneeAgentId });
|
||||
const unreadablePrivateIds = await privateTree("Stranger private", {});
|
||||
|
||||
const privateProjectIssueIds = [randomUUID(), randomUUID()];
|
||||
await db.insert(issues).values(privateProjectIssueIds.map((id, index) => ({
|
||||
id, companyId, title: `Private project ${index}`, status: "todo", projectId: privateProjectId, createdByUserId: strangerUserId,
|
||||
})));
|
||||
|
||||
const hiddenId = randomUUID();
|
||||
await db.insert(issues).values({ id: hiddenId, companyId, title: "Hidden", status: "todo", hiddenAt: now });
|
||||
|
||||
// Blocked open tasks, so the summaries route also filters their blockers.
|
||||
const blockedIds = [randomUUID(), randomUUID()];
|
||||
await db.insert(issues).values(blockedIds.map((id, index) => ({ id, companyId, title: `Blocked ${index}`, status: "blocked" })));
|
||||
await db.insert(issueRelations).values([
|
||||
{ companyId, issueId: publicIds[2 % publicCount]!, relatedIssueId: blockedIds[0]!, type: "blocks" as const },
|
||||
{ companyId, issueId: unreadablePrivateIds[0]!, relatedIssueId: blockedIds[0]!, type: "blocks" as const },
|
||||
{ companyId, issueId: ownedPrivateIds[1]!, relatedIssueId: blockedIds[1]!, type: "blocks" as const },
|
||||
]);
|
||||
|
||||
const foreignPrivateId = randomUUID();
|
||||
const foreignIds = [randomUUID(), foreignPrivateId];
|
||||
await db.insert(issues).values([
|
||||
{ id: foreignIds[0]!, companyId: foreignCompanyId, title: "Foreign open", status: "todo" },
|
||||
{ id: foreignPrivateId, companyId: foreignCompanyId, title: "Foreign private", status: "todo", visibility: "private",
|
||||
privacyRootIssueId: foreignPrivateId, createdByUserId: outsiderUserId },
|
||||
]);
|
||||
const missingIds = [randomUUID(), randomUUID()];
|
||||
|
||||
// Open tasks the task-bounded low-trust agent may read; the private one in its boundary stays unreadable.
|
||||
const boundedTaskIds = [2, 5, 7].map((index) => publicIds[index % publicCount]!);
|
||||
await db.update(agents).set({ permissions: { trustPreset: "low_trust_review",
|
||||
authorizationPolicy: { trustBoundary: { mode: "low_trust_review", companyId, issueIds: [...boundedTaskIds, grantedPrivateIds[1]!] } } },
|
||||
}).where(eq(agents.id, lowTrustAgentId));
|
||||
await db.update(agents).set({ permissions: { trustPreset: "low_trust_review",
|
||||
authorizationPolicy: { trustBoundary: { mode: "low_trust_review", companyId, rootIssueId: boundaryRootId } } },
|
||||
}).where(eq(agents.id, lowTrustRootAgentId));
|
||||
|
||||
const runId = randomUUID();
|
||||
await db.insert(heartbeatRuns).values({ id: runId, companyId, agentId: readerAgentId, status: "running", contextSnapshot: { issueId: publicIds[0]! } });
|
||||
|
||||
const board = (role: (typeof roles)[number], source: AuthorizationActor["source"] = "session"): AuthorizationActor => ({
|
||||
type: "board", source, userId: users[role], companyIds: [companyId],
|
||||
memberships: [{ companyId, membershipRole: role, status: "active" }], isInstanceAdmin: false,
|
||||
});
|
||||
const agent = (agentId: string, extra: Partial<AuthorizationActor> = {}): AuthorizationActor => ({
|
||||
type: "agent", agentId, companyId, source: "agent_key", ...extra,
|
||||
});
|
||||
const actors: Record<IssueReadVisibilityActorName, AuthorizationActor> = {
|
||||
"board owner": board("owner"),
|
||||
"board admin": board("admin"),
|
||||
"board operator": board("operator"),
|
||||
"board viewer": board("viewer"),
|
||||
"board member": board("member"),
|
||||
"board key operator": board("operator", "board_key"),
|
||||
"cloud tenant owner": board("owner", "cloud_tenant"),
|
||||
"cloud tenant viewer": board("viewer", "cloud_tenant"),
|
||||
"instance admin": { ...board("admin"), isInstanceAdmin: true },
|
||||
"local implicit board": { type: "board", source: "local_implicit", userId: "local-board", companyIds: [companyId], isInstanceAdmin: true },
|
||||
"board without membership": { type: "board", source: "session", userId: outsiderUserId, companyIds: [foreignCompanyId],
|
||||
memberships: [{ companyId: foreignCompanyId, membershipRole: "owner", status: "active" }], isInstanceAdmin: false },
|
||||
"agent key": agent(readerAgentId),
|
||||
"assignee agent key": agent(assigneeAgentId),
|
||||
"agent run for owner": agent(readerAgentId, { source: "agent_jwt", runId, onBehalfOfUserId: users.owner,
|
||||
onBehalfOfMemberships: [{ companyId, membershipRole: "owner", status: "active" }] }),
|
||||
"agent run for operator": agent(readerAgentId, { source: "agent_jwt", runId, onBehalfOfUserId: users.operator,
|
||||
onBehalfOfMemberships: [{ companyId, membershipRole: "operator", status: "active" }] }),
|
||||
"agent run for viewer": agent(readerAgentId, { source: "agent_jwt", runId, onBehalfOfUserId: users.viewer,
|
||||
onBehalfOfMemberships: [{ companyId, membershipRole: "viewer", status: "active" }] }),
|
||||
"agent run for owner, membership read from the database": agent(assigneeAgentId, { source: "agent_jwt", onBehalfOfUserId: users.owner }),
|
||||
"agent run for a user without membership": agent(readerAgentId, { source: "agent_jwt", runId, onBehalfOfUserId: outsiderUserId }),
|
||||
"agent authorized by operator": agent(readerAgentId, { authorizingUserId: users.operator }),
|
||||
"agent authorized by owner for owner": agent(assigneeAgentId, { onBehalfOfUserId: users.owner, authorizingUserId: users.owner,
|
||||
onBehalfOfMemberships: [{ companyId, membershipRole: "owner", status: "active" }] }),
|
||||
"low-trust agent bounded to tasks": agent(lowTrustAgentId),
|
||||
"low-trust agent bounded to a subtree": agent(lowTrustRootAgentId),
|
||||
"agent of another company": { type: "agent", agentId: foreignAgentId, companyId: foreignCompanyId, source: "agent_key" },
|
||||
"unauthenticated": { type: "none", source: "none" },
|
||||
};
|
||||
|
||||
const companyIssueIds = [
|
||||
...publicIds, ...ownedPrivateIds, ...grantedPrivateIds, ...assignedPrivateIds, ...unreadablePrivateIds,
|
||||
...privateProjectIssueIds, ...blockedIds, hiddenId,
|
||||
];
|
||||
return {
|
||||
companyId, foreignCompanyId, users, outsiderUserId, strangerUserId,
|
||||
readerAgentId, assigneeAgentId, lowTrustAgentId, lowTrustRootAgentId, foreignAgentId, runId,
|
||||
openProjectId, privateProjectId,
|
||||
publicIds, boundaryRootId, boundaryChildId, boundedTaskIds, ownedPrivateIds, grantedPrivateIds, assignedPrivateIds, unreadablePrivateIds,
|
||||
privateProjectIssueIds, blockedIds, hiddenId, foreignIds, missingIds, companyIssueIds,
|
||||
/** Every ID a caller might ask for, readable or not. */
|
||||
allIds: [...companyIssueIds, ...foreignIds, ...missingIds],
|
||||
actors,
|
||||
};
|
||||
}
|
||||
@@ -18,13 +18,19 @@ const mockIssueService = vi.hoisted(() => ({
|
||||
}));
|
||||
|
||||
const mockLogActivity = vi.hoisted(() => vi.fn(async () => undefined));
|
||||
const mockAccessService = vi.hoisted(() => ({
|
||||
decide: vi.fn(async ({ action }: { action: string }) => ({
|
||||
const mockAccessService = vi.hoisted(() => {
|
||||
const decide = vi.fn(async ({ action }: { action: string; actor?: unknown; resource?: unknown }) => ({
|
||||
action, allowed: action === "issue:read", reason: "allow_default", explanation: "Fixture task is readable",
|
||||
})),
|
||||
canUser: vi.fn(async () => false),
|
||||
hasPermission: vi.fn(async () => false),
|
||||
}));
|
||||
}));
|
||||
return {
|
||||
decide,
|
||||
// The batched read asks the mocked decision once per task.
|
||||
decideReads: async (actor: unknown, resources: unknown[]) =>
|
||||
Promise.all(resources.map((resource) => decide({ actor, action: "issue:read", resource }))),
|
||||
canUser: vi.fn(async () => false),
|
||||
hasPermission: vi.fn(async () => false),
|
||||
};
|
||||
});
|
||||
const mockHeartbeatService = vi.hoisted(() => ({
|
||||
wakeup: vi.fn(async () => undefined),
|
||||
reportRunActivity: vi.fn(async () => undefined),
|
||||
|
||||
@@ -0,0 +1,242 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import express from "express";
|
||||
import request from "supertest";
|
||||
import { inArray } from "drizzle-orm";
|
||||
import { drizzle } from "drizzle-orm/postgres-js";
|
||||
import { afterAll, beforeAll, expect, it } from "vitest";
|
||||
import { issues, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS } from "@paperclipai/db";
|
||||
import { ISSUE_SUMMARIES_MAX_IDS } from "@paperclipai/shared";
|
||||
import { issueRoutes } from "../routes/issues.js";
|
||||
import { errorHandler } from "../middleware/index.js";
|
||||
import { authorizationService, READ_DECISION_CONCURRENCY, type AuthorizationActor } from "../services/authorization.js";
|
||||
import { describeEmbeddedPostgres, useEmbeddedPostgres } from "./helpers/route-test-harness.js";
|
||||
import {
|
||||
ISSUE_READ_VISIBILITY_ACTORS, seedIssueReadVisibilityFixture, type IssueReadVisibilityActorName,
|
||||
type IssueReadVisibilityFixture,
|
||||
} from "./helpers/issue-read-visibility-fixture.js";
|
||||
|
||||
/** Actors that the company gate rejects before any task is read. The routes answer every other actor with 200. */
|
||||
const REJECTED_BY_COMPANY_GATE: Partial<Record<IssueReadVisibilityActorName, 401 | 403>> = {
|
||||
"board without membership": 403,
|
||||
"agent run for owner, membership read from the database": 403,
|
||||
"agent run for a user without membership": 403,
|
||||
"agent of another company": 403,
|
||||
"unauthenticated": 401,
|
||||
};
|
||||
|
||||
/** The statement one task-privacy check per task sends. */
|
||||
const PER_TASK_PRIVACY_STATEMENT = /^select "id" from "issues" where \("issues"\."id" = \$1 and "issues"\."company_id" = \$2 and exists/;
|
||||
|
||||
describeEmbeddedPostgres("batched task read filtering", () => {
|
||||
const ctx = useEmbeddedPostgres("paperclip-issue-read-batch-");
|
||||
const oldPrivacy = process.env.PAPERCLIP_ISSUE_PRIVACY_MODE;
|
||||
let f: IssueReadVisibilityFixture;
|
||||
beforeAll(async () => {
|
||||
process.env.PAPERCLIP_ISSUE_PRIVACY_MODE = "enforce";
|
||||
f = await seedIssueReadVisibilityFixture(ctx.db, { publicCount: 60 });
|
||||
}, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS);
|
||||
afterAll(() => {
|
||||
if (oldPrivacy === undefined) delete process.env.PAPERCLIP_ISSUE_PRIVACY_MODE;
|
||||
else process.env.PAPERCLIP_ISSUE_PRIVACY_MODE = oldPrivacy;
|
||||
});
|
||||
|
||||
type Row = Pick<typeof issues.$inferSelect, "id" | "companyId" | "projectId" | "parentId" | "assigneeAgentId" | "assigneeUserId" | "status">;
|
||||
async function rowsFor(ids: string[]): Promise<Row[]> {
|
||||
const rows = await ctx.db.select({
|
||||
id: issues.id, companyId: issues.companyId, projectId: issues.projectId, parentId: issues.parentId,
|
||||
assigneeAgentId: issues.assigneeAgentId, assigneeUserId: issues.assigneeUserId, status: issues.status,
|
||||
}).from(issues).where(inArray(issues.id, ids));
|
||||
const byId = new Map(rows.map((row) => [row.id, row]));
|
||||
return ids.flatMap((id) => byId.get(id) ?? []);
|
||||
}
|
||||
const resource = (row: Row, companyId = row.companyId) => ({
|
||||
type: "issue" as const, companyId, issueId: row.id, projectId: row.projectId, parentIssueId: row.parentId,
|
||||
assigneeAgentId: row.assigneeAgentId, assigneeUserId: row.assigneeUserId, status: row.status,
|
||||
});
|
||||
/** The reference: one authorization decision per task, as the routes asked before batching. */
|
||||
function decideEachTask(actor: AuthorizationActor, rows: Row[], companyId?: string) {
|
||||
return Promise.all(rows.map((row) => authorizationService(ctx.db).decide({
|
||||
actor: { ...actor }, action: "issue:read", resource: resource(row, companyId),
|
||||
scope: { issueId: row.id, projectId: row.projectId, parentIssueId: row.parentId,
|
||||
assigneeAgentId: row.assigneeAgentId, assigneeUserId: row.assigneeUserId },
|
||||
})));
|
||||
}
|
||||
const outcome = (decisions: Array<{ allowed: boolean; reason: string }>) => decisions.map(({ allowed, reason }) => ({ allowed, reason }));
|
||||
async function readableByBatch(actor: AuthorizationActor, ids: string[]) {
|
||||
const rows = await rowsFor(ids);
|
||||
const decisions = await authorizationService(ctx.db).decideReads({ ...actor }, rows.map((row) => resource(row)));
|
||||
return rows.filter((_, index) => decisions[index]!.allowed).map((row) => row.id);
|
||||
}
|
||||
|
||||
function app(actor: AuthorizationActor, db = ctx.db) {
|
||||
const result = express();
|
||||
result.use(express.json());
|
||||
result.use((req, _res, next) => { req.actor = { ...actor } as Express.Request["actor"]; next(); });
|
||||
result.use("/api", issueRoutes(db, {} as never));
|
||||
result.use(errorHandler);
|
||||
return result;
|
||||
}
|
||||
function recording() {
|
||||
const statements: string[] = [];
|
||||
const db = drizzle(ctx.db.$client, {
|
||||
schema: ctx.db._.fullSchema, logger: { logQuery: (query) => statements.push(query) },
|
||||
}) as unknown as typeof ctx.db;
|
||||
return { db, statements };
|
||||
}
|
||||
const summariesUrl = (companyId: string) => `/api/companies/${companyId}/issues/summaries`;
|
||||
async function summaryIds(actor: AuthorizationActor, ids: string[], db = ctx.db) {
|
||||
const found: string[] = [];
|
||||
for (let offset = 0; offset < ids.length; offset += ISSUE_SUMMARIES_MAX_IDS) {
|
||||
const res = await request(app(actor, db)).get(summariesUrl(f.companyId))
|
||||
.query({ ids: ids.slice(offset, offset + ISSUE_SUMMARIES_MAX_IDS).join(",") });
|
||||
if (res.status !== 200) return { status: res.status, ids: [] as string[], body: JSON.stringify(res.body) };
|
||||
found.push(...(res.body as Array<{ id: string }>).map((row) => row.id));
|
||||
}
|
||||
return { status: 200, ids: found, body: "" };
|
||||
}
|
||||
|
||||
it.each(ISSUE_READ_VISIBILITY_ACTORS)("decides every task read for %s exactly as one check per task does", async (name) => {
|
||||
const actor = f.actors[name];
|
||||
const rows = await rowsFor([...f.companyIssueIds, ...f.foreignIds]);
|
||||
expect(rows).toHaveLength(f.companyIssueIds.length + f.foreignIds.length);
|
||||
const batched = await authorizationService(ctx.db).decideReads({ ...actor }, rows.map((row) => resource(row)));
|
||||
expect(outcome(batched)).toEqual(outcome(await decideEachTask(actor, rows)));
|
||||
|
||||
// Tasks of another company and IDs that do not exist, asked for as this company's.
|
||||
const foreign = await rowsFor(f.foreignIds);
|
||||
const claimed: Row[] = [...foreign, ...f.missingIds.map((id) => ({
|
||||
id, companyId: f.companyId, projectId: null, parentId: null, assigneeAgentId: null, assigneeUserId: null, status: "todo",
|
||||
}))];
|
||||
const claimedBatch = await authorizationService(ctx.db).decideReads({ ...actor }, claimed.map((row) => resource(row, f.companyId)));
|
||||
expect(outcome(claimedBatch)).toEqual(outcome(await decideEachTask(actor, claimed, f.companyId)));
|
||||
});
|
||||
|
||||
it("keeps each kind of private task readable only by its principals", async () => {
|
||||
const privateIds = [...f.ownedPrivateIds, ...f.grantedPrivateIds, ...f.assignedPrivateIds, ...f.unreadablePrivateIds, ...f.privateProjectIssueIds];
|
||||
const readable = async (name: IssueReadVisibilityActorName) => (await readableByBatch(f.actors[name], privateIds)).sort();
|
||||
const sorted = (...groups: string[][]) => groups.flat().sort();
|
||||
expect(await readable("board owner")).toEqual(sorted(f.ownedPrivateIds));
|
||||
expect(await readable("cloud tenant owner")).toEqual(sorted(f.ownedPrivateIds));
|
||||
expect(await readable("board operator")).toEqual(sorted(f.grantedPrivateIds));
|
||||
expect(await readable("board admin")).toEqual(sorted(f.privateProjectIssueIds));
|
||||
// The viewer's grant is revoked and the member has none.
|
||||
expect(await readable("board viewer")).toEqual([]);
|
||||
expect(await readable("board member")).toEqual([]);
|
||||
expect(await readable("agent key")).toEqual(sorted(f.grantedPrivateIds));
|
||||
expect(await readable("assignee agent key")).toEqual(sorted(f.assignedPrivateIds));
|
||||
// An agent reads for a person only what both of them may read.
|
||||
expect(await readable("agent run for operator")).toEqual(sorted(f.grantedPrivateIds));
|
||||
expect(await readable("agent run for owner")).toEqual([]);
|
||||
expect(await readable("agent run for viewer")).toEqual([]);
|
||||
expect(await readable("instance admin")).toEqual(sorted(privateIds));
|
||||
expect(await readable("local implicit board")).toEqual(sorted(privateIds));
|
||||
for (const name of ["board without membership", "agent of another company", "unauthenticated"] as const) {
|
||||
expect(await readableByBatch(f.actors[name], f.companyIssueIds)).toEqual([]);
|
||||
}
|
||||
// A low-trust agent is denied open tasks outside its boundary, so its list is filtered row by row.
|
||||
const bounded = await readableByBatch(f.actors["low-trust agent bounded to a subtree"], f.publicIds);
|
||||
expect(bounded.sort()).toEqual([f.boundaryRootId, f.boundaryChildId].sort());
|
||||
});
|
||||
|
||||
it.each(ISSUE_READ_VISIBILITY_ACTORS)("returns %s the summaries one check per task would, omitting the rest", async (name) => {
|
||||
const actor = f.actors[name];
|
||||
const result = await summaryIds(actor, f.allIds);
|
||||
const rows = await rowsFor(f.companyIssueIds.filter((id) => id !== f.hiddenId));
|
||||
const decisions = await decideEachTask(actor, rows);
|
||||
const expected = rows.filter((_, index) => decisions[index]!.allowed).map((row) => row.id);
|
||||
const rejected = REJECTED_BY_COMPANY_GATE[name];
|
||||
expect(result.status).toBe(rejected ?? 200);
|
||||
if (rejected) {
|
||||
for (const id of f.allIds) expect(result.body).not.toContain(id);
|
||||
return;
|
||||
}
|
||||
expect(result.ids).toEqual(f.allIds.filter((id) => expected.includes(id)));
|
||||
expect(result.ids.length).toBeGreaterThan(0);
|
||||
for (const id of [...f.foreignIds, ...f.missingIds, f.hiddenId]) expect(result.ids).not.toContain(id);
|
||||
});
|
||||
|
||||
it.each([
|
||||
"low-trust agent bounded to tasks", "low-trust agent bounded to a subtree",
|
||||
] as const)("lists for %s the tasks one check per task would", async (name) => {
|
||||
const actor = f.actors[name];
|
||||
const res = await request(app(actor)).get(`/api/companies/${f.companyId}/issues`).query({ limit: 500 }).expect(200);
|
||||
const listed = (res.body as Array<{ id: string }>).map((row) => row.id).sort();
|
||||
const rows = await rowsFor(f.companyIssueIds.filter((id) => id !== f.hiddenId));
|
||||
const decisions = await decideEachTask(actor, rows);
|
||||
expect(listed).toEqual(rows.filter((_, index) => decisions[index]!.allowed).map((row) => row.id).sort());
|
||||
expect(listed.length).toBeGreaterThan(0);
|
||||
expect(listed.length).toBeLessThan(rows.length);
|
||||
});
|
||||
|
||||
/** Statements of one summaries request for 5 tasks and of one for 50, each mixing every kind of task. */
|
||||
async function summaryStatements(name: IssueReadVisibilityActorName) {
|
||||
const actor = f.actors[name];
|
||||
// Both requests mix open tasks, a project task and readable and unreadable private tasks.
|
||||
const mixed = [f.publicIds[2]!, f.ownedPrivateIds[0]!, f.grantedPrivateIds[2]!, f.unreadablePrivateIds[1]!, f.publicIds[3]!];
|
||||
const many = [...mixed, ...f.ownedPrivateIds, ...f.grantedPrivateIds, ...f.assignedPrivateIds, ...f.unreadablePrivateIds,
|
||||
...f.privateProjectIssueIds, ...f.foreignIds, ...f.missingIds, ...f.publicIds];
|
||||
const fifty = [...new Set(many)].slice(0, ISSUE_SUMMARIES_MAX_IDS);
|
||||
expect(fifty).toHaveLength(ISSUE_SUMMARIES_MAX_IDS);
|
||||
|
||||
const small = recording();
|
||||
const five = await summaryIds(actor, mixed, small.db);
|
||||
const large = recording();
|
||||
const all = await summaryIds(actor, fifty, large.db);
|
||||
expect(five.status).toBe(200);
|
||||
expect(all.status).toBe(200);
|
||||
expect(all.ids.length).toBeGreaterThan(five.ids.length);
|
||||
return { small: small.statements, large: large.statements, extraTasks: fifty.length - mixed.length };
|
||||
}
|
||||
|
||||
it.each([
|
||||
"board owner", "board viewer", "board key operator", "cloud tenant owner", "instance admin", "agent key",
|
||||
"assignee agent key", "agent run for operator", "agent authorized by operator",
|
||||
] as const)("sends the same number of statements for %s whether a summaries request names 5 tasks or 50", async (name) => {
|
||||
const { small, large } = await summaryStatements(name);
|
||||
expect(large.filter((statement) => PER_TASK_PRIVACY_STATEMENT.test(statement))).toEqual([]);
|
||||
expect(large.length).toBe(small.length);
|
||||
expect(large.length).toBeLessThanOrEqual(16);
|
||||
});
|
||||
|
||||
it.each([
|
||||
"low-trust agent bounded to tasks", "low-trust agent bounded to a subtree",
|
||||
] as const)("adds only the boundary checks of each further task for %s", async (name) => {
|
||||
const { small, large, extraTasks } = await summaryStatements(name);
|
||||
// The task rows and the privacy predicate come from the batch. What stays
|
||||
// per task is the boundary itself: its ancestry walk and its mention grant.
|
||||
expect(large.filter((statement) => PER_TASK_PRIVACY_STATEMENT.test(statement))).toEqual([]);
|
||||
expect(large.filter((statement) => /from "issues" where \(?"issues"\."id" = \$1/.test(statement))).toEqual([]);
|
||||
expect(large.length).toBeLessThanOrEqual(small.length + 2 * extraTasks);
|
||||
});
|
||||
|
||||
it("decides the per-task boundary checks of a batch a few at a time", async () => {
|
||||
let inFlight = 0, most = 0;
|
||||
const counting = new Proxy(ctx.db, {
|
||||
get(target, key) {
|
||||
if (key !== "execute") return Reflect.get(target, key);
|
||||
return (...args: Parameters<typeof target.execute>) => {
|
||||
most = Math.max(most, ++inFlight);
|
||||
return Promise.resolve(target.execute(...args)).finally(() => { inFlight -= 1; });
|
||||
};
|
||||
},
|
||||
});
|
||||
const actor = f.actors["low-trust agent bounded to a subtree"];
|
||||
const rows = await rowsFor(f.publicIds);
|
||||
const decisions = await authorizationService(counting).decideReads({ ...actor }, rows.map((row) => resource(row)));
|
||||
expect(rows.filter((_, index) => decisions[index]!.allowed).map((row) => row.id).sort())
|
||||
.toEqual([f.boundaryRootId, f.boundaryChildId].sort());
|
||||
// Each task outside the boundary walks its ancestry; they overlap, but never all at once.
|
||||
expect(most).toBeGreaterThan(1);
|
||||
expect(most).toBeLessThanOrEqual(READ_DECISION_CONCURRENCY);
|
||||
});
|
||||
|
||||
it("does not let a batch reuse another actor's answer", async () => {
|
||||
// Two different actors over the same tasks in one process, in both orders.
|
||||
const ids = [...f.ownedPrivateIds, ...f.grantedPrivateIds, randomUUID()];
|
||||
const owner = (await summaryIds(f.actors["board owner"], ids)).ids;
|
||||
const operator = (await summaryIds(f.actors["board operator"], ids)).ids;
|
||||
expect(owner).toEqual(f.ownedPrivateIds);
|
||||
expect(operator).toEqual(f.grantedPrivateIds);
|
||||
expect((await summaryIds(f.actors["board owner"], ids)).ids).toEqual(owner);
|
||||
});
|
||||
});
|
||||
@@ -38,11 +38,17 @@ const mockExecutionWorkspaceService = vi.hoisted(() => ({
|
||||
getStoredById: vi.fn(),
|
||||
}));
|
||||
|
||||
const mockAccessService = vi.hoisted(() => ({
|
||||
canUser: vi.fn(),
|
||||
decide: vi.fn(),
|
||||
hasPermission: vi.fn(),
|
||||
}));
|
||||
const mockAccessService = vi.hoisted(() => {
|
||||
const decide = vi.fn();
|
||||
return {
|
||||
canUser: vi.fn(),
|
||||
decide,
|
||||
// The batched read asks the mocked decision once per task.
|
||||
decideReads: async (actor: unknown, resources: unknown[]) =>
|
||||
Promise.all(resources.map((resource) => decide({ actor, action: "issue:read", resource }))),
|
||||
hasPermission: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
const mockAgentService = vi.hoisted(() => ({
|
||||
getById: vi.fn(),
|
||||
|
||||
@@ -5550,11 +5550,7 @@ export function issueRoutes(
|
||||
})
|
||||
.from(issueRows)
|
||||
.where(and(eq(issueRows.companyId, companyId), inArray(issueRows.id, uniqueIds)));
|
||||
const decisions = await Promise.all(rows.map(async (row) => ({
|
||||
id: row.id,
|
||||
allowed: (await decideIssueAccess(req, row, "issue:read")).allowed,
|
||||
})));
|
||||
return new Set(decisions.filter((decision) => decision.allowed).map((decision) => decision.id));
|
||||
return new Set((await filterIssuesForActor(req, rows)).map((row) => row.id));
|
||||
}
|
||||
|
||||
function lockedIssueStub(issue: Pick<IssueRelationIssueSummary, "id" | "identifier">): IssueLockedStub {
|
||||
@@ -5750,12 +5746,24 @@ export function issueRoutes(
|
||||
return decision !== true && decision.reason === "allow_visible_issue_write";
|
||||
}
|
||||
|
||||
/**
|
||||
* Keeps the tasks the actor may read. One batched authorization read decides
|
||||
* all of them, so the number of statements does not grow with the number of
|
||||
* tasks.
|
||||
*/
|
||||
async function filterIssuesForActor<
|
||||
T extends Parameters<typeof decideIssueAccess>[1],
|
||||
>(req: Request, rows: T[]) {
|
||||
const decisions = await Promise.all(
|
||||
rows.map((issue) => decideIssueAccess(req, issue, "issue:read")),
|
||||
);
|
||||
const decisions = await access.decideReads(req.actor, rows.map((issue) => ({
|
||||
type: "issue" as const,
|
||||
companyId: issue.companyId,
|
||||
issueId: issue.id,
|
||||
projectId: issue.projectId,
|
||||
parentIssueId: issue.parentId,
|
||||
assigneeAgentId: issue.assigneeAgentId,
|
||||
assigneeUserId: issue.assigneeUserId,
|
||||
status: issue.status,
|
||||
})));
|
||||
return rows.filter((_, index) => decisions[index]?.allowed);
|
||||
}
|
||||
|
||||
|
||||
@@ -783,6 +783,22 @@ type ReadAuthorizationSnapshot = {
|
||||
|
||||
type ReadAuthorizationResource = Extract<AuthorizationResource, { type: "issue" | "project" | "company" }>;
|
||||
|
||||
/** Per-resource decisions of one batched read that run at the same time. */
|
||||
export const READ_DECISION_CONCURRENCY = 8;
|
||||
|
||||
/** Maps in input order with bounded concurrency; stops starting work after the first failure. */
|
||||
async function mapWithConcurrency<T, R>(items: readonly T[], concurrency: number, mapper: (item: T) => Promise<R>): Promise<R[]> {
|
||||
const results = new Array<R>(items.length);
|
||||
let next = 0, failed = false;
|
||||
await Promise.all(Array.from({ length: Math.min(concurrency, items.length) }, async () => {
|
||||
while (!failed && next < items.length) {
|
||||
const index = next++;
|
||||
try { results[index] = await mapper(items[index]!); } catch (error) { failed = true; throw error; }
|
||||
}
|
||||
}));
|
||||
return results;
|
||||
}
|
||||
|
||||
export function authorizationService(db: Db | DbTransaction) {
|
||||
return createAuthorizationService(db);
|
||||
}
|
||||
@@ -850,10 +866,13 @@ function createAuthorizationService(db: Db | DbTransaction, readSnapshot?: ReadA
|
||||
const results: AuthorizationDecision[][] = [];
|
||||
for (let index = 0; index < batchActors.length; index++) {
|
||||
const batch = createAuthorizationService(db, snapshots[index]);
|
||||
const decisions: AuthorizationDecision[] = [];
|
||||
for (const resource of resources) decisions.push(await batch.decide({ actor: batchActors[index], resource,
|
||||
action: resource.type === "issue" ? "issue:read" : resource.type === "project" ? "project:read" : "company_scope:read" }));
|
||||
results.push(decisions);
|
||||
// The batch has already read what the resources share. Checks that stay
|
||||
// per resource (a low-trust boundary's ancestry and mention grants) run
|
||||
// a few at a time, so a long list neither waits on them one by one nor
|
||||
// takes every pool connection at once.
|
||||
results.push(await mapWithConcurrency(resources, READ_DECISION_CONCURRENCY, resource => batch.decide({
|
||||
actor: batchActors[index], resource,
|
||||
action: resource.type === "issue" ? "issue:read" : resource.type === "project" ? "project:read" : "company_scope:read" })));
|
||||
}
|
||||
return results;
|
||||
}
|
||||
@@ -2839,8 +2858,10 @@ function createAuthorizationService(db: Db | DbTransaction, readSnapshot?: ReadA
|
||||
|
||||
const companyId = companyIdForResource(input.resource);
|
||||
// External authority always uses current membership, without the request's
|
||||
// responsible-user memo or rollout exceptions for agent-owned grants.
|
||||
const snapshot = await loadResponsibleUserSnapshot(companyId, authorizingUserId);
|
||||
// responsible-user memo or rollout exceptions for agent-owned grants. A
|
||||
// batch of read decisions reads that membership from the database once.
|
||||
const snapshot = await batchRead(`responsible:${companyId}:${authorizingUserId}`,
|
||||
() => loadResponsibleUserSnapshot(companyId, authorizingUserId));
|
||||
const userDecision = snapshot.userExists && snapshot.activeMembership
|
||||
? await decideBase({
|
||||
...input,
|
||||
|
||||
Reference in new issue
Block a user