mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 01:41:42 +02:00
fix(server): share repository source reads across retained readers
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
d660c420ef
commit
be0d39b8d6
5 files changed
+92
-28
No files matched your search
@@ -1,14 +1,14 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { mkdtemp, rm } from "node:fs/promises";
|
||||
import { createHash, randomUUID } from "node:crypto";
|
||||
import { mkdtemp, realpath, rm } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import { and, eq, inArray, sql } from "drizzle-orm";
|
||||
import { drizzle } from "drizzle-orm/postgres-js";
|
||||
import { agents, authUsers, companies, companyMemberships, createDb, executionWorkspaces, heartbeatRuns, issueAccessGrants,
|
||||
import { agents, authUsers, companies, companyMemberships, createDb, executionWorkspaces, executionWorkspaceRepositoryAccess, heartbeatRuns, issueAccessGrants,
|
||||
issues, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS } from "@paperclipai/db";
|
||||
import { authorizationService, executionWorkspaceReadSqlCondition, type AuthorizationActor } from "../services/authorization.js";
|
||||
import { assertExecutionWorkspaceRootReadAccess } from "../services/execution-workspace-repositories.js";
|
||||
import { executionWorkspaceRepositoryService, assertExecutionWorkspaceRootReadAccess } from "../services/execution-workspace-repositories.js";
|
||||
import { assertTaskWorkspaceAccess } from "../services/task-workspace-source-access.js";
|
||||
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js";
|
||||
|
||||
@@ -133,6 +133,35 @@ const support = await getEmbeddedPostgresTestSupport();
|
||||
}
|
||||
});
|
||||
|
||||
it("shares source loading across retained readers during actual repository admission", async () => {
|
||||
const cwd = await realpath(await mkdtemp(path.join(tmpdir(), "multi-reader-admission-")));
|
||||
const rootKey = createHash("sha256").update(cwd).digest("hex");
|
||||
const workspaceIds = Array.from({ length: 40 }, () => randomUUID());
|
||||
const taskIds = workspaceIds.map(() => randomUUID()), runIds = taskIds.slice(0, 10).map(() => randomUUID());
|
||||
await db.insert(executionWorkspaces).values(workspaceIds.map(id => ({ id, companyId, cwd,
|
||||
name: "Historical root", mode: "shared_workspace", strategyType: "operator_directory" })));
|
||||
await db.insert(issues).values(taskIds.map((id, i) => ({ id, companyId, title: "Source", assigneeAgentId: agentId, executionWorkspaceId: workspaceIds[i] })));
|
||||
await db.insert(heartbeatRuns).values(runIds.map((id, i) => ({ id, companyId, agentId, responsibleUserId: userId,
|
||||
status: "succeeded", contextSnapshot: { issueId: taskIds[i] } })));
|
||||
await db.insert(executionWorkspaceRepositoryAccess).values(runIds.map((scopeId, i) => ({ companyId, rootKey,
|
||||
canonicalCwd: cwd, kind: "reader", scopeId, issueId: taskIds[i], agentId, responsibleUserId: userId })));
|
||||
try {
|
||||
const statements: string[] = [];
|
||||
const recorded = drizzle(db.$client, { schema: db._.fullSchema,
|
||||
logger: { logQuery: query => statements.push(query) } }) as unknown as typeof db;
|
||||
await executionWorkspaceRepositoryService(recorded).prepareForAdmission({ companyId, cwd,
|
||||
issueId: taskIds[0], workspaceId: workspaceIds[0], agentId, runId: runIds[0], responsibleUserId: userId });
|
||||
expect(statements.filter(query => query.includes('from "execution_workspaces"')
|
||||
|| query.includes('from "issues"')).length).toBeLessThan(50);
|
||||
} finally {
|
||||
await db.delete(executionWorkspaceRepositoryAccess).where(and(eq(executionWorkspaceRepositoryAccess.companyId, companyId), eq(executionWorkspaceRepositoryAccess.rootKey, rootKey)));
|
||||
await db.delete(heartbeatRuns).where(inArray(heartbeatRuns.id, runIds));
|
||||
await db.delete(issues).where(inArray(issues.id, taskIds));
|
||||
await db.delete(executionWorkspaces).where(inArray(executionWorkspaces.id, workspaceIds));
|
||||
await rm(cwd, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
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") {
|
||||
|
||||
@@ -752,9 +752,19 @@ function createAuthorizationService(db: Db | DbTransaction, readSnapshot?: ReadA
|
||||
}
|
||||
|
||||
async function decideReads(actor: AuthorizationActor, resources: ReadAuthorizationResource[]): Promise<AuthorizationDecision[]> {
|
||||
const snapshot: ReadAuthorizationSnapshot = { issues: new Map(), projects: new Map(), memo: new Map() };
|
||||
return (await decideReadsForActors([actor], resources))[0];
|
||||
}
|
||||
|
||||
async function decideReadsForActors(actors: AuthorizationActor[], resources: ReadAuthorizationResource[]): Promise<AuthorizationDecision[][]> {
|
||||
if (!actors.length) return [];
|
||||
const snapshots: ReadAuthorizationSnapshot[] = actors.map(() => ({ issues: new Map(), projects: new Map(), memo: new Map() }));
|
||||
// Do not carry a previous request's responsible-user snapshot into this batch.
|
||||
const batchActor: ResponsibleUserActorWithMemo = { ...actor, __responsibleUserSnapshotMemo: undefined };
|
||||
const batchActors: ResponsibleUserActorWithMemo[] = actors.map(actor => ({ ...actor, __responsibleUserSnapshotMemo: undefined }));
|
||||
// Privacy depends on the two principals, while trust can differ by run.
|
||||
// Share SQL privacy evaluation only; every run still gets its own decision.
|
||||
const privacyKey = (actor: AuthorizationActor) => JSON.stringify([actor.type, actor.agentId, actor.userId, actor.onBehalfOfUserId]);
|
||||
const privacyActors = [...new Map(batchActors.map(actor => [privacyKey(actor), actor])).values()];
|
||||
const privacyIndexes = batchActors.map(actor => privacyActors.findIndex(peer => privacyKey(peer) === privacyKey(actor)));
|
||||
const issueIds = [...new Set(resources.flatMap(resource => resource.type === "issue" && resource.issueId ? [resource.issueId] : []))];
|
||||
const projectIds = new Set(resources.flatMap(resource => resource.type === "project" && resource.projectId ? [resource.projectId]
|
||||
: resource.type === "issue" && resource.projectId ? [resource.projectId] : []));
|
||||
@@ -765,23 +775,32 @@ function createAuthorizationService(db: Db | DbTransaction, readSnapshot?: ReadA
|
||||
status: issues.status, executionPolicy: issues.executionPolicy, originKind: issues.originKind, originId: issues.originId,
|
||||
visibility: issues.visibility, privacyRootIssueId: issues.privacyRootIssueId,
|
||||
responsibleUserId: issues.responsibleUserId, createdByUserId: issues.createdByUserId,
|
||||
readable: issuePrivacyCondition(batchActor),
|
||||
readable: sql<boolean[]>`jsonb_build_array(${sql.join(privacyActors.map(actor => issuePrivacyCondition(actor)), sql`, `)})`,
|
||||
}).from(issues).where(inArray(issues.id, issueIds.slice(offset, offset + 500)));
|
||||
for (const row of rows) { snapshot.issues.set(row.id, row); if (row.projectId) projectIds.add(row.projectId); }
|
||||
for (const row of rows) {
|
||||
for (let index = 0; index < snapshots.length; index++) snapshots[index].issues.set(row.id, { ...row, readable: row.readable[privacyIndexes[index]] });
|
||||
if (row.projectId) projectIds.add(row.projectId);
|
||||
}
|
||||
}
|
||||
const ids = [...projectIds];
|
||||
for (let offset = 0; offset < ids.length; offset += 500) {
|
||||
const rows = await db.select({ id: projects.id, companyId: projects.companyId,
|
||||
executionWorkspacePolicy: projects.executionWorkspacePolicy, visibility: projects.visibility,
|
||||
readable: projectPrivacyCondition(batchActor),
|
||||
readable: sql<boolean[]>`jsonb_build_array(${sql.join(privacyActors.map(actor => projectPrivacyCondition(actor)), sql`, `)})`,
|
||||
}).from(projects).where(inArray(projects.id, ids.slice(offset, offset + 500)));
|
||||
for (const row of rows) snapshot.projects.set(row.id, row);
|
||||
for (const row of rows) for (let index = 0; index < snapshots.length; index++) {
|
||||
snapshots[index].projects.set(row.id, { ...row, readable: row.readable[privacyIndexes[index]] });
|
||||
}
|
||||
}
|
||||
const batch = createAuthorizationService(db, snapshot);
|
||||
const decisions: AuthorizationDecision[] = [];
|
||||
for (const resource of resources) decisions.push(await batch.decide({ actor: batchActor, resource,
|
||||
action: resource.type === "issue" ? "issue:read" : resource.type === "project" ? "project:read" : "company_scope:read" }));
|
||||
return decisions;
|
||||
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);
|
||||
}
|
||||
return results;
|
||||
}
|
||||
|
||||
async function isInstanceAdmin(userId: string | null | undefined): Promise<boolean> {
|
||||
@@ -2824,6 +2843,7 @@ function createAuthorizationService(db: Db | DbTransaction, readSnapshot?: ReadA
|
||||
return {
|
||||
decide,
|
||||
decideReads,
|
||||
decideReadsForActors,
|
||||
projectDiscoveryCandidateIds,
|
||||
decidePrincipalGrant,
|
||||
};
|
||||
|
||||
@@ -10,7 +10,7 @@ import { prepareWorkspaceRepositorySchema, type PrepareWorkspaceRepository, type
|
||||
import { withDirectoryPublicationLock } from "@paperclipai/adapter-utils/workspace-restore-merge";
|
||||
import { conflict, forbidden, notFound } from "../errors.js";
|
||||
import { parseNativeExecutionInput } from "../vendor/paperclip-runner/index.js";
|
||||
import { assertTaskWorkspacesReadAccess, assertTaskWorkspaceAccess, assertTaskWorkspaceSourceProjectAccess } from "./task-workspace-source-access.js";
|
||||
import { assertTaskWorkspacesReadAccessForActors, assertTaskWorkspacesReadAccess, assertTaskWorkspaceAccess, assertTaskWorkspaceSourceProjectAccess } from "./task-workspace-source-access.js";
|
||||
import { authorizationService, canActorReadExecutionWorkspace, executionWorkspaceReadSqlCondition, issueReadSqlCondition, projectReadSqlCondition, type AuthorizationActor } from "./authorization.js";
|
||||
import { LOW_TRUST_ISSUE_ANCESTRY_MAX_DEPTH, resolveCoreTrustPreset } from "./trust-preset-resolver.js";
|
||||
import { taskWorkspaceSelectableCondition } from "./task-workspace-selection.js";
|
||||
@@ -256,11 +256,13 @@ async function retainGitMetadataOwners(tx: DbTransaction, input: {
|
||||
const closure = await resolveRepositoryRootSources(tx, { companyId: input.companyId, cwd: input.commonRoot, inspectCurrentAliases: true });
|
||||
const workspaceIds = new Set([...closure.sourceIds, ...input.workspaceIds]);
|
||||
const projectIds = new Set([...closure.sourceProjectIds, ...input.projectIds]);
|
||||
const readerActors: AuthorizationActor[] = [];
|
||||
for (const reader of closure.overlappingEntries.filter(entry => entry.kind === "reader")) {
|
||||
const { actor } = await retainedRootReader(tx, input.companyId, reader);
|
||||
await assertTaskWorkspacesReadAccess(tx, actor, input.companyId, workspaceIds);
|
||||
readerActors.push(actor);
|
||||
await assertRootProjectAccess(tx, actor, input.companyId, projectIds);
|
||||
}
|
||||
await assertTaskWorkspacesReadAccessForActors(tx, readerActors, input.companyId, workspaceIds);
|
||||
for (const [kind, ids] of [["source", workspaceIds], ["project_source", projectIds]] as const) {
|
||||
for (const scopeId of ids) if (!closure.entries.some(entry => entry.kind === kind && entry.scopeId === scopeId)) {
|
||||
await tx.insert(executionWorkspaceRepositoryAccess).values({ companyId: input.companyId,
|
||||
@@ -349,12 +351,14 @@ export async function retainIsolatedWorkspaceSourceAccess(db: Db, input: {
|
||||
await assertTaskWorkspacesReadAccess(tx, input.actor, input.companyId, target.sourceIds);
|
||||
await assertRootProjectAccess(tx, input.actor, input.companyId, target.sourceProjectIds,
|
||||
{ sharedProjectIds: target.sharedProjectIds, issueId: input.issueId });
|
||||
const readerActors: AuthorizationActor[] = [];
|
||||
for (const reader of target.overlappingEntries.filter(entry => entry.kind === "reader")) {
|
||||
const { actor, issue } = await retainedRootReader(tx, input.companyId, reader);
|
||||
readerActors.push(actor);
|
||||
await assertRootProjectTrust(tx, { companyId: input.companyId, actor, issueId: issue.id, projectIds: target.sourceProjectIds });
|
||||
await assertTaskWorkspacesReadAccess(tx, actor, input.companyId, target.sourceIds);
|
||||
await assertRootProjectAccess(tx, actor, input.companyId, target.sourceProjectIds);
|
||||
}
|
||||
await assertTaskWorkspacesReadAccessForActors(tx, readerActors, input.companyId, target.sourceIds);
|
||||
} catch (error) { failure ??= error; }
|
||||
if (!failure && verifiedCopyProjectIds) {
|
||||
for (const scopeId of verifiedCopyProjectIds) if (!existing.some(entry => entry.kind === "isolated_copy" && entry.scopeId === scopeId)) {
|
||||
@@ -538,6 +542,11 @@ export function executionWorkspaceRepositoryService(db: Db) {
|
||||
const ownAdmissionOnly = input.preserveOwnAdmission && input.reader && input.sourceWorkspaceId
|
||||
&& sourceIds.size === 1 && sourceIds.has(input.sourceWorkspaceId)
|
||||
&& readers.every(reader => reader.issueId === input.reader!.issueId);
|
||||
// Acquisition policy depends on the source project, not on each historical
|
||||
// workspace. Revalidate each distinct project for every retained reader.
|
||||
const sourceProjects = sourceIds.size ? await tx.selectDistinct({ projectId: executionWorkspaces.projectId }).from(executionWorkspaces).where(and(
|
||||
eq(executionWorkspaces.companyId, input.companyId), inArray(executionWorkspaces.id, [...sourceIds]))) : [];
|
||||
const sourceActors: AuthorizationActor[] = [];
|
||||
for (const reader of readers) {
|
||||
const { actor, issue } = await retainedRootReader(tx, input.companyId, reader);
|
||||
await assertRootProjectTrust(tx, { companyId: input.companyId, projectIds: sourceProjectIds, actor, issueId: issue.id,
|
||||
@@ -552,11 +561,7 @@ export function executionWorkspaceRepositoryService(db: Db) {
|
||||
type: "board", userId: actor.onBehalfOfUserId, ignoreInstanceAdmin: true,
|
||||
}) : undefined));
|
||||
if (!ownSource) throw forbidden("Task workspace access is no longer available");
|
||||
} else await assertTaskWorkspacesReadAccess(tx, actor, input.companyId, sourceIds);
|
||||
// Acquisition policy depends on the source project, not on each historical
|
||||
// workspace. Revalidate each distinct project for every retained reader.
|
||||
const sourceProjects = sourceIds.size ? await tx.selectDistinct({ projectId: executionWorkspaces.projectId }).from(executionWorkspaces).where(and(
|
||||
eq(executionWorkspaces.companyId, input.companyId), inArray(executionWorkspaces.id, [...sourceIds]))) : [];
|
||||
} else sourceActors.push(actor);
|
||||
for (const sourceWorkspace of sourceProjects.length ? sourceProjects : [null]) {
|
||||
if (await requiresAnonymousAcquisition(tx, { issue, workspace: sourceWorkspace, agentId: reader.agentId!, runId: reader.scopeId })) {
|
||||
await reserveRootPolicy(tx, input.companyId, canonicalCwd, "anonymous_only");
|
||||
@@ -567,8 +572,9 @@ export function executionWorkspaceRepositoryService(db: Db) {
|
||||
if (!input.issueId) throw forbidden("Workspace write authorization requires a task");
|
||||
await assertRootProjectTrust(tx, { ...input, projectIds: sourceProjectIds, actor: input.actor });
|
||||
await assertRootProjectAccess(tx, input.actor, input.companyId, sourceProjectIds, { sharedProjectIds, issueId: input.issueId });
|
||||
await assertTaskWorkspacesReadAccess(tx, input.actor, input.companyId, sourceIds);
|
||||
sourceActors.push(input.actor);
|
||||
}
|
||||
await assertTaskWorkspacesReadAccessForActors(tx, sourceActors, input.companyId, sourceIds);
|
||||
for (const [kind, ids] of [["source", sharedSourceIds], ["isolated_source", isolatedSourceIds], ["project_source", sourceProjectIds],
|
||||
["shared_project_source", sharedProjectIds], ["isolated_project_source", isolatedProjectIds]] as const) {
|
||||
for (const scopeId of ids) {
|
||||
|
||||
@@ -66,10 +66,17 @@ export async function assertTaskWorkspaceAccess(
|
||||
* batch. Keep the single-workspace write path's assignment checks separate. */
|
||||
export async function assertTaskWorkspacesReadAccess(
|
||||
db: Db | DbTransaction, actor: AuthorizationActor, companyId: string, workspaceIds: Iterable<string>,
|
||||
) {
|
||||
return assertTaskWorkspacesReadAccessForActors(db, [actor], companyId, workspaceIds);
|
||||
}
|
||||
|
||||
export async function assertTaskWorkspacesReadAccessForActors(
|
||||
db: Db | DbTransaction, actors: AuthorizationActor[], companyId: string, workspaceIds: Iterable<string>,
|
||||
) {
|
||||
const ids = [...new Set(workspaceIds)];
|
||||
if (!ids.length) return;
|
||||
const privacy = await executionWorkspaceReadSqlCondition(db, actor);
|
||||
if (!ids.length || !actors.length) return;
|
||||
const privacyActors = [...new Map(actors.map(actor => [JSON.stringify({ ...actor, runId: undefined }), actor])).values()];
|
||||
const privacy = and(...await Promise.all(privacyActors.map(actor => executionWorkspaceReadSqlCondition(db, actor))));
|
||||
const projectIds = new Set<string>(), sourceIds = new Set<string>();
|
||||
let needsCompanyRead = false;
|
||||
for (let offset = 0; offset < ids.length; offset += 500) {
|
||||
@@ -105,8 +112,8 @@ export async function assertTaskWorkspacesReadAccess(
|
||||
...[...sourceIds].map(issueId => ({ type: "issue" as const, companyId, issueId })),
|
||||
...(needsCompanyRead ? [{ type: "company" as const, companyId }] : []),
|
||||
];
|
||||
const decisions = await authorizationService(db).decideReads(actor, resources);
|
||||
for (let index = 0; index < decisions.length; index++) {
|
||||
const batches = await authorizationService(db).decideReadsForActors(actors, resources);
|
||||
for (const decisions of batches) for (let index = 0; index < decisions.length; index++) {
|
||||
if (decisions[index].allowed) continue;
|
||||
throw forbidden(resources[index].type === "issue"
|
||||
? "Task workspace retains files outside this actor's authorization boundary"
|
||||
|
||||
Reference in new issue
Block a user