From 445feaa7aadc40b7c09ad31eba94e862f1c600c4 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 14 Oct 2026 16:55:34 -0500 Subject: [PATCH] fix(server): index retained workspace task authorization (#16334) ## Thinking Path > - Paperclip manages AI agents and their work. > - Before an agent starts, repository admission checks access to retained workspace files. > - Admission holds a company provenance lock while it checks each source and reader. > - The retained-task query cast the indexed task ID to text. PostgreSQL could evaluate privacy across company history before it matched one source ID. > - Repeated slow checks kept the lock for hours and blocked other agent launches. > - This change looks up the canonical source UUID through the existing primary key before it checks privacy. > - The query does less work and keeps the same access decisions. ## Linked Issues or Issue Description **What happened?** Nine runs appeared active but had no provider process or log file. Read-only database inspection found a transaction holding the company repository-provenance lock for more than ten hours. Eight other transactions waited for that lock. The owner repeatedly ran workspace authorization queries. The company had about 13,400 tasks and 8,500 workspaces. **Expected behavior** A retained-source check should find the referenced task before it evaluates that task's privacy. Unrelated task history should not multiply the privacy work for every source and reader. **Steps to reproduce** 1. Run the new `workspace-authorization-query.test.ts` against authorization code from `5c7a025032`. 2. The real PostgreSQL fixture has 1,200 unrelated tasks and one retained source. The regression fails with 2,402 recursive privacy traversals, above the bound of 100. The other five access tests pass. 3. Apply this change and run the same test. All six tests pass. Related: Refs #16330 adds workspace timing evidence. This PR fixes a measured SQL plan. It does not change those tracing boundaries. ## What Changed - Compare the UUID task column with a guarded canonical UUID from the retained-source JSON key. - Reuse `canonicalExecutionUuid`. Invalid and noncanonical keys still match nothing and cannot cause a UUID parse error. - Add real PostgreSQL coverage for query work, missing and foreign sources, malformed and uppercase IDs, both agent and sponsor grants, and grant revocation. - Add a reproducible baseline comparison and document its scope in `doc/DATABASE.md`. ## Verification - Red: the unchanged query fails the new work-bound regression. Five access cases pass. - Green: 101 tests pass across the new query suite, workspace binding, and the production privacy review suite. - Full `pnpm -r typecheck` and `pnpm build`: pass. Dependencies use the repository's pinned pnpm 9.15.4. - All current-head CI checks pass, including the full server, chat, workspace, serialized, Runner, and eight E2E test lanes. Build, typecheck, canary, and security checks pass. Head: `ad907f6e14c2d773fa8c20692e5ccc10215c9297`. - Greptile: 5/5 on that head, with no actionable findings or unresolved review threads. The branch is mergeable. - The local full `pnpm test:run` was started, then stopped after a concurrent disposable-database check hit the macOS shared-memory limit. This is not counted as a local full-suite pass. The benchmark passed when rerun alone; the complete suite passed in CI. - `git diff --check`: pass. The branch has no lockfile or workflow changes. Synthetic SQL comparison on PostgreSQL 18.1, with 13,429 tasks and one retained source: | Query | Median | Five measured samples | Recursive privacy traversals | | --- | --- | --- | --- | | Baseline `5c7a025032` | 42.57 ms | 42.57, 47.71, 44.51, 41.29, 41.44 ms | 26,860 | | Fixed | 2.37 ms | 2.76, 2.35, 2.37, 2.11, 3.33 ms | 4 | Both versions return identical complete rows. The benchmark alternates variants after two warmup rounds. It loads the baseline authorization module with current dependencies; the UUID helper is unchanged. The measured gain is about 18x for this SQL fixture. The final committed benchmark also passed a second isolated run: 55.44 ms baseline and 3.26 ms fixed, with identical complete rows (about 17x). Two read-only queries on the affected database also returned identical complete rows. One changed from 129 ms to 7 ms; a workspace with 49 retained sources changed from 576 ms to 11 ms. These are single paired measurements, not endpoint or startup benchmarks. Reproduce: ```sh pnpm exec vitest run server/src/__tests__/workspace-authorization-query.test.ts server/src/__tests__/task-workspace-binding.test.ts server/src/__tests__/privacy-production-review.test.ts pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-workspace-authorization.ts --baseline-ref=5c7a025032 pnpm -r typecheck pnpm test:run pnpm build ``` ## Risks - A direct UUID cast could broaden or break access for malformed keys. The existing canonical guard and regression cases preserve the former text-match behavior. - Planner choices can differ across PostgreSQL versions and data sets. The regression bounds real privacy work rather than elapsed time or one exact plan shape. - Admission still checks every retained source and reader under its existing lock. This removes a measured query cost; it does not claim a bound for the full admission process. - No schema migration, permission change, lock removal, or live deployment is included. Recovery of the affected runs remains unverified until rollout. ## Model Used OpenAI GPT-6 through Codex, with tool use and code execution. The exact served model variant and context-window size are not exposed to this task. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip --- doc/DATABASE.md | 18 ++++ scripts/benchmark-workspace-authorization.ts | 74 +++++++++++++++ .../workspace-authorization-query.test.ts | 90 +++++++++++++++++++ server/src/services/authorization.ts | 3 +- 4 files changed, 184 insertions(+), 1 deletion(-) create mode 100644 scripts/benchmark-workspace-authorization.ts create mode 100644 server/src/__tests__/workspace-authorization-query.test.ts diff --git a/doc/DATABASE.md b/doc/DATABASE.md index 4ae992bc87e..8af04cbd37b 100644 --- a/doc/DATABASE.md +++ b/doc/DATABASE.md @@ -45,6 +45,24 @@ pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-run-history.ts - The fixture has 1,400 synthetic runs with 192 KiB contexts. Both queries return the same 200 complete rows. This benchmark excludes HTTP and redaction latency. +Workspace reads also compare retained task IDs as guarded, canonical UUIDs. +This lets PostgreSQL check each source task by primary key before it evaluates +task privacy. Casting the indexed task ID to text can instead evaluate privacy +across company history for each workspace. Repository admission repeats these +reads while holding its company provenance lock, so that plan can delay other +agent launches. Missing, malformed, noncanonical, and foreign-company source +IDs still deny access. + +To compare this query on a disposable fixture with 13,429 tasks: + +```sh +pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-workspace-authorization.ts --baseline-ref=5c7a025032 +``` + +The benchmark loads the baseline authorization module with current dependencies, +compares complete workspace rows, and prints five warm samples and query plans. +It measures SQL execution, not the full repository admission or agent startup. + ## 1. Embedded PostgreSQL — zero config If you don't set `DATABASE_URL`, the server automatically starts an embedded PostgreSQL instance and manages a local data directory. diff --git a/scripts/benchmark-workspace-authorization.ts b/scripts/benchmark-workspace-authorization.ts new file mode 100644 index 00000000000..c7a40efc2d3 --- /dev/null +++ b/scripts/benchmark-workspace-authorization.ts @@ -0,0 +1,74 @@ +import assert from "node:assert/strict"; +import { randomUUID } from "node:crypto"; +import { execFileSync } from "node:child_process"; +import fs from "node:fs/promises"; +import path from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { createRequire } from "node:module"; +const { and, eq } = createRequire(new URL("../server/package.json", import.meta.url))("drizzle-orm"); +import { agents, authUsers, companies, companyMemberships, createDb, executionWorkspaces, issues, + startEmbeddedPostgresTestDatabase, type Db } from "../packages/db/src/index.js"; +import * as authorization from "../server/src/services/authorization.js"; + +const baselineRef = process.argv.find(arg => arg.startsWith("--baseline-ref="))?.slice(15) ?? "HEAD"; +const baselineRevision = execFileSync("git", ["rev-parse", `${baselineRef}^{commit}`], { encoding: "utf8" }).trim(); +process.env.PAPERCLIP_ISSUE_PRIVACY_MODE = "enforce"; +const keepAlive = setInterval(() => {}, 1000); +let database: Awaited> | undefined; +let db: Db | undefined; +let baselineDir: string | undefined; +try { + database = await startEmbeddedPostgresTestDatabase("workspace-authorization-benchmark-"); + db = createDb(database.connectionString); + baselineDir = await fs.mkdtemp(fileURLToPath(new URL("../server/scripts/.workspace-auth-baseline-", import.meta.url))); + const original = new URL("../server/src/services/authorization.ts", import.meta.url); + const source = execFileSync("git", ["show", `${baselineRevision}:server/src/services/authorization.ts`], { encoding: "utf8" }); + const rewritten = source.replace(/from "(\.{1,2}\/[^\"]+)"/g, + (_match, specifier: string) => `from ${JSON.stringify(fileURLToPath(new URL(specifier, original)))}`); + const baselinePath = path.join(baselineDir, "authorization.ts"); + await fs.writeFile(baselinePath, rewritten); + const baseline = await import(pathToFileURL(baselinePath).href) as typeof authorization; + const companyId = randomUUID(), agentId = randomUUID(), userId = randomUUID(), workspaceId = randomUUID(); + await db.insert(companies).values({ id: companyId, name: "Synthetic workspace authorization", issuePrefix: "BENCH" }); + await db.insert(authUsers).values({ id: userId, name: "Sponsor", email: `${userId}@example.test`, emailVerified: true, createdAt: new Date(), updatedAt: new Date() }); + await db.insert(agents).values({ id: agentId, companyId, name: "Worker", adapterType: "process" }); + await db.insert(companyMemberships).values({ companyId, principalType: "user", principalId: userId, status: "active", membershipRole: "operator" }); + await db.insert(executionWorkspaces).values({ id: workspaceId, companyId, name: "Shared source", mode: "shared_workspace", strategyType: "operator_directory" }); + await db.$client.unsafe(`insert into issues (company_id, title) select $1, 'Historical task ' || n from generate_series(1, 13428) n`, [companyId]); + const [sourceIssue] = await db.insert(issues).values({ companyId, title: "Retained source after unrelated history" }).returning({ id: issues.id }); + await db.update(executionWorkspaces).set({ sourceIssueId: sourceIssue.id }).where(eq(executionWorkspaces.id, workspaceId)); + await db.$client.unsafe("ANALYZE"); + const actor: authorization.AuthorizationActor = { type: "agent", source: "agent_jwt", agentId, companyId, onBehalfOfUserId: userId }; + const queries = await Promise.all([baseline, authorization].map(async module => db!.select().from(executionWorkspaces).where(and( + eq(executionWorkspaces.id, workspaceId), eq(executionWorkspaces.companyId, companyId), + await module.executionWorkspaceReadSqlCondition(db!, actor), + )).toSQL())); + const samples: number[][] = [[], []]; + let expected: unknown; + for (let n = 0; n < 7; n++) { + for (const variant of n % 2 ? [1, 0] : [0, 1]) { + const query = queries[variant]; + const start = performance.now(); + const rows = await db.$client.unsafe(query.sql, query.params as never[]).values(); + if (n >= 2) samples[variant].push(performance.now() - start); + assert.equal(rows.length, 1); + if (expected === undefined) expected = rows; + else assert.deepEqual(rows, expected); + } + } + const results = []; + for (const [variant, query] of queries.entries()) { + const [plan] = await db.$client.unsafe(`EXPLAIN (ANALYZE, BUFFERS, FORMAT JSON) ${query.sql}`, query.params as never[]); + results.push({ variant: variant ? "treatment" : "baseline", samplesMs: samples[variant], + medianMs: [...samples[variant]].sort((a, b) => a - b)[2], sql: query.sql, params: query.params, plan: plan["QUERY PLAN"] }); + } + console.log(JSON.stringify({ baselineRevision, tasks: 13429, retainedSources: 1, equalCompleteRows: true, + fixtureIds: { companyId, agentId, userId, workspaceId }, + postgres: (await db.$client.unsafe("select version()"))[0].version, results }, null, 2)); +} finally { + try { + await db?.$client.end({ timeout: 0 }); + await database?.cleanup(); + if (baselineDir) await fs.rm(baselineDir, { recursive: true, force: true }); + } finally { clearInterval(keepAlive); } +} diff --git a/server/src/__tests__/workspace-authorization-query.test.ts b/server/src/__tests__/workspace-authorization-query.test.ts new file mode 100644 index 00000000000..ce66a9c7a19 --- /dev/null +++ b/server/src/__tests__/workspace-authorization-query.test.ts @@ -0,0 +1,90 @@ +import { randomUUID } from "node:crypto"; +import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; +import { and, eq, sql } from "drizzle-orm"; +import { agents, authUsers, companies, companyMemberships, createDb, executionWorkspaces, issueAccessGrants, + issues, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS } from "@paperclipai/db"; +import { executionWorkspaceReadSqlCondition, type AuthorizationActor } from "../services/authorization.js"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; + +const support = await getEmbeddedPostgresTestSupport(); +(support.supported ? describe : describe.skip)("workspace authorization query", () => { + let database: Awaited>; + let db: ReturnType; + const companyId = randomUUID(), agentId = randomUUID(), userId = randomUUID(), workspaceId = randomUUID(); + const sourceId = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"; + const actor: AuthorizationActor = { type: "agent", source: "agent_jwt", companyId, agentId, onBehalfOfUserId: userId }; + + beforeAll(async () => { + vi.stubEnv("PAPERCLIP_ISSUE_PRIVACY_MODE", "enforce"); + database = await startEmbeddedPostgresTestDatabase("workspace-authorization-query-"); + db = createDb(database.connectionString); + await db.insert(companies).values({ id: companyId, name: "Workspace authorization", issuePrefix: "WAQ" }); + await db.insert(authUsers).values({ id: userId, name: "Sponsor", email: `${userId}@example.test`, emailVerified: true, createdAt: new Date(), updatedAt: new Date() }); + await db.insert(agents).values({ id: agentId, companyId, name: "Worker", adapterType: "process" }); + await db.insert(companyMemberships).values({ companyId, principalType: "user", principalId: userId, status: "active", membershipRole: "operator" }); + await db.insert(executionWorkspaces).values({ id: workspaceId, companyId, name: "Retained files", mode: "shared_workspace", strategyType: "operator_directory" }); + await db.$client.unsafe(`insert into issues (company_id, title) select $1, 'Unrelated history ' || n from generate_series(1, 1200) n`, [companyId]); + await db.insert(issues).values({ id: sourceId, companyId, title: "Retained source" }); + await db.$client.unsafe("ANALYZE"); + }, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS); + afterAll(async () => { + vi.unstubAllEnvs(); + await db?.$client.end({ timeout: 0 }); + await database?.cleanup(); + }); + + async function query() { + return db.select().from(executionWorkspaces).where(and(eq(executionWorkspaces.id, workspaceId), + eq(executionWorkspaces.companyId, companyId), await executionWorkspaceReadSqlCondition(db, actor))).toSQL(); + } + async function read() { + const q = await query(); + return db.$client.unsafe(q.sql, q.params as never[]); + } + async function retain(id: string) { + // Seed historical/corrupt provenance in this disposable database. Normal + // writes have a trigger that deliberately preserves the previous keys. + await db.transaction(async tx => { + await tx.execute(sql`set local session_replication_role = replica`); + await tx.update(executionWorkspaces).set({ metadata: { _issuePrivacySources: { [id]: true } } }) + .where(eq(executionWorkspaces.id, workspaceId)); + }); + } + + it("checks retained task IDs without evaluating privacy for unrelated company history", async () => { + await retain(sourceId); + expect(await read()).toHaveLength(1); + const q = await query(); + const [result] = await db.$client.unsafe(`EXPLAIN (ANALYZE, FORMAT JSON) ${q.sql}`, q.params as never[]); + type Plan = { "Node Type": string; "Actual Loops": number; Plans?: Plan[] }; + function privacyTraversals(plan: Plan): number { + return (plan["Node Type"] === "Recursive Union" ? plan["Actual Loops"] : 0) + + (plan.Plans ?? []).reduce((sum, child) => sum + privacyTraversals(child), 0); + } + // A single source should not make admission authorize 1,200 unrelated tasks + // for both the agent and its responsible user under the company-wide lock. + expect(privacyTraversals(result["QUERY PLAN"][0].Plan)).toBeLessThan(100); + }); + + it.each(["missing", "malformed", "uppercase", "foreign"])("denies a %s retained source without throwing a UUID parse error", async kind => { + const id = kind === "malformed" ? "not-a-uuid" : kind === "uppercase" ? sourceId.toUpperCase() : randomUUID(); + if (kind === "foreign") { + const [foreign] = await db.insert(companies).values({ name: "Foreign", issuePrefix: "OTHER" }).returning(); + await db.insert(issues).values({ id, companyId: foreign.id, title: "Foreign source" }); + } + await retain(id); + expect(await read()).toHaveLength(0); + }); + + it("requires both principals and rechecks revoked grants for retained private files", async () => { + await retain(sourceId); + await db.update(issues).set({ visibility: "private", privacyRootIssueId: sourceId }).where(eq(issues.id, sourceId)); + expect(await read()).toHaveLength(0); + await db.insert(issueAccessGrants).values({ issueId: sourceId, subjectType: "agent", subjectId: agentId, source: "explicit" }); + expect(await read()).toHaveLength(0); + const [grant] = await db.insert(issueAccessGrants).values({ issueId: sourceId, subjectType: "user", subjectId: userId, source: "explicit" }).returning(); + expect(await read()).toHaveLength(1); + await db.update(issueAccessGrants).set({ revokedAt: new Date() }).where(eq(issueAccessGrants.id, grant.id)); + expect(await read()).toHaveLength(0); + }); +}); diff --git a/server/src/services/authorization.ts b/server/src/services/authorization.ts index 9b68990080e..e78eae1b013 100644 --- a/server/src/services/authorization.ts +++ b/server/src/services/authorization.ts @@ -2,6 +2,7 @@ import { and, eq, inArray, isNull, or, sql, type SQL } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; import { withHumanDirectedWork } from "./human-directed-work.js"; import { hasOwnerChatInstructionAuthority } from "./owner-chat-instruction-authority.js"; +import { canonicalExecutionUuid } from "./execution-identity.js"; import { agents, approvals, @@ -2769,7 +2770,7 @@ export async function executionWorkspaceReadSqlCondition(db: Db | DbTransaction, select 1 from ${projects} where ${projects.id} = ${executionWorkspaces.projectId} and ${projects.companyId} = ${executionWorkspaces.companyId} and ${projectCondition})) and not exists (select 1 from jsonb_object_keys(coalesce(${executionWorkspaces.metadata}->'_issuePrivacySources', '{}'::jsonb)) source(id) - where not exists (select 1 from ${issues} where ${issues.id}::text = source.id + where not exists (select 1 from ${issues} where ${issues.id} = ${canonicalExecutionUuid(sql`source.id`)} and ${issues.companyId} = ${executionWorkspaces.companyId} and ${issueCondition})) and not exists (select 1 from ${issues} where ${issues.companyId} = ${executionWorkspaces.companyId}