mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-23 09:07:46 +02:00
Fix watchdog decision authorization binding
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
eb7282e402
commit
600839b477
5 files changed
+285
-12
No files matched your search
@@ -20,6 +20,7 @@ import {
|
||||
heartbeatService,
|
||||
} from "../services/heartbeat.ts";
|
||||
import { recoveryService } from "../services/recovery/service.ts";
|
||||
import { getRunLogStore } from "../services/run-log-store.ts";
|
||||
|
||||
const mockAdapterExecute = vi.hoisted(() =>
|
||||
vi.fn(async () => ({
|
||||
@@ -92,7 +93,7 @@ describeEmbeddedPostgres("active-run output watchdog", () => {
|
||||
await tempDb?.cleanup();
|
||||
});
|
||||
|
||||
async function seedRunningRun(opts: { now: Date; ageMs: number; withOutput?: boolean }) {
|
||||
async function seedRunningRun(opts: { now: Date; ageMs: number; withOutput?: boolean; logChunk?: string }) {
|
||||
const companyId = randomUUID();
|
||||
const managerId = randomUUID();
|
||||
const coderId = randomUUID();
|
||||
@@ -161,6 +162,23 @@ describeEmbeddedPostgres("active-run output watchdog", () => {
|
||||
stdoutExcerpt: "OPENAI_API_KEY=sk-test-secret-value should not leak",
|
||||
logBytes: 0,
|
||||
});
|
||||
if (opts.logChunk) {
|
||||
const store = getRunLogStore();
|
||||
const handle = await store.begin({ companyId, agentId: coderId, runId });
|
||||
const logBytes = await store.append(handle, {
|
||||
stream: "stdout",
|
||||
chunk: opts.logChunk,
|
||||
ts: startedAt.toISOString(),
|
||||
});
|
||||
await db
|
||||
.update(heartbeatRuns)
|
||||
.set({
|
||||
logStore: handle.store,
|
||||
logRef: handle.logRef,
|
||||
logBytes,
|
||||
})
|
||||
.where(eq(heartbeatRuns.id, runId));
|
||||
}
|
||||
await db.update(issues).set({ executionRunId: runId }).where(eq(issues.id, issueId));
|
||||
return { companyId, managerId, coderId, issueId, runId, issuePrefix };
|
||||
}
|
||||
@@ -196,6 +214,34 @@ describeEmbeddedPostgres("active-run output watchdog", () => {
|
||||
expect(evaluations[0]?.description).not.toContain("sk-test-secret-value");
|
||||
});
|
||||
|
||||
it("redacts sensitive values from actual run-log evidence", async () => {
|
||||
const now = new Date("2026-04-22T20:00:00.000Z");
|
||||
const leakedJwt = "eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c";
|
||||
const leakedGithubToken = "ghp_1234567890abcdefghijklmnopqrstuvwxyz";
|
||||
const { companyId } = await seedRunningRun({
|
||||
now,
|
||||
ageMs: ACTIVE_RUN_OUTPUT_SUSPICION_THRESHOLD_MS + 60_000,
|
||||
logChunk: [
|
||||
"Authorization: Bearer live-bearer-token-value",
|
||||
`POST payload {"apiKey":"json-secret-value","token":"${leakedJwt}"}`,
|
||||
`GITHUB_TOKEN=${leakedGithubToken}`,
|
||||
].join("\n"),
|
||||
});
|
||||
const heartbeat = heartbeatService(db);
|
||||
|
||||
await heartbeat.scanSilentActiveRuns({ now, companyId });
|
||||
|
||||
const [evaluation] = await db
|
||||
.select()
|
||||
.from(issues)
|
||||
.where(and(eq(issues.companyId, companyId), eq(issues.originKind, "stale_active_run_evaluation")));
|
||||
expect(evaluation?.description).toContain("***REDACTED***");
|
||||
expect(evaluation?.description).not.toContain("live-bearer-token-value");
|
||||
expect(evaluation?.description).not.toContain("json-secret-value");
|
||||
expect(evaluation?.description).not.toContain(leakedJwt);
|
||||
expect(evaluation?.description).not.toContain(leakedGithubToken);
|
||||
});
|
||||
|
||||
it("raises critical stale-run evaluations and blocks the source issue", async () => {
|
||||
const now = new Date("2026-04-22T20:00:00.000Z");
|
||||
const { companyId, issueId } = await seedRunningRun({
|
||||
@@ -305,4 +351,146 @@ describeEmbeddedPostgres("active-run output watchdog", () => {
|
||||
evaluationIssueId,
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects agent watchdog decisions using issues not bound to the target run", async () => {
|
||||
const now = new Date("2026-04-22T20:00:00.000Z");
|
||||
const { companyId, managerId, coderId, runId, issuePrefix } = await seedRunningRun({
|
||||
now,
|
||||
ageMs: ACTIVE_RUN_OUTPUT_SUSPICION_THRESHOLD_MS + 60_000,
|
||||
});
|
||||
const heartbeat = heartbeatService(db);
|
||||
const recovery = recoveryService(db, { enqueueWakeup: vi.fn() });
|
||||
|
||||
const scan = await heartbeat.scanSilentActiveRuns({ now, companyId });
|
||||
const evaluationIssueId = scan.evaluationIssueIds[0];
|
||||
expect(evaluationIssueId).toBeTruthy();
|
||||
|
||||
const unrelatedIssueId = randomUUID();
|
||||
await db.insert(issues).values({
|
||||
id: unrelatedIssueId,
|
||||
companyId,
|
||||
title: "Assigned but unrelated",
|
||||
status: "todo",
|
||||
priority: "medium",
|
||||
assigneeAgentId: managerId,
|
||||
issueNumber: 20,
|
||||
identifier: `${issuePrefix}-20`,
|
||||
});
|
||||
|
||||
const otherRunId = randomUUID();
|
||||
const otherEvaluationIssueId = randomUUID();
|
||||
await db.insert(heartbeatRuns).values({
|
||||
id: otherRunId,
|
||||
companyId,
|
||||
agentId: coderId,
|
||||
status: "running",
|
||||
invocationSource: "assignment",
|
||||
triggerDetail: "system",
|
||||
startedAt: new Date(now.getTime() - ACTIVE_RUN_OUTPUT_SUSPICION_THRESHOLD_MS - 120_000),
|
||||
processStartedAt: new Date(now.getTime() - ACTIVE_RUN_OUTPUT_SUSPICION_THRESHOLD_MS - 120_000),
|
||||
lastOutputAt: null,
|
||||
lastOutputSeq: 0,
|
||||
lastOutputStream: null,
|
||||
contextSnapshot: {},
|
||||
logBytes: 0,
|
||||
});
|
||||
await db.insert(issues).values({
|
||||
id: otherEvaluationIssueId,
|
||||
companyId,
|
||||
title: "Other run evaluation",
|
||||
status: "todo",
|
||||
priority: "medium",
|
||||
assigneeAgentId: managerId,
|
||||
issueNumber: 21,
|
||||
identifier: `${issuePrefix}-21`,
|
||||
originKind: "stale_active_run_evaluation",
|
||||
originId: otherRunId,
|
||||
originFingerprint: `stale_active_run:${companyId}:${otherRunId}`,
|
||||
});
|
||||
|
||||
const attempts = [
|
||||
{ decision: "continue" as const, evaluationIssueId: unrelatedIssueId },
|
||||
{ decision: "dismissed_false_positive" as const, evaluationIssueId: unrelatedIssueId },
|
||||
{
|
||||
decision: "snooze" as const,
|
||||
evaluationIssueId: unrelatedIssueId,
|
||||
snoozedUntil: new Date(now.getTime() + 60 * 60 * 1000),
|
||||
},
|
||||
{ decision: "continue" as const, evaluationIssueId: otherEvaluationIssueId },
|
||||
];
|
||||
|
||||
for (const attempt of attempts) {
|
||||
await expect(
|
||||
recovery.recordWatchdogDecision({
|
||||
runId,
|
||||
actor: { type: "agent", agentId: managerId },
|
||||
reason: "malicious or stale binding",
|
||||
...attempt,
|
||||
}),
|
||||
).rejects.toMatchObject({ status: 403 });
|
||||
}
|
||||
|
||||
await db.update(issues).set({ status: "done" }).where(eq(issues.id, evaluationIssueId));
|
||||
await expect(
|
||||
recovery.recordWatchdogDecision({
|
||||
runId,
|
||||
actor: { type: "agent", agentId: managerId },
|
||||
decision: "continue",
|
||||
evaluationIssueId,
|
||||
reason: "closed evaluation should not authorize",
|
||||
}),
|
||||
).rejects.toMatchObject({ status: 403 });
|
||||
});
|
||||
|
||||
it("validates createdByRunId before storing watchdog decisions", async () => {
|
||||
const now = new Date("2026-04-22T20:00:00.000Z");
|
||||
const { companyId, managerId, runId } = await seedRunningRun({
|
||||
now,
|
||||
ageMs: ACTIVE_RUN_OUTPUT_SUSPICION_THRESHOLD_MS + 60_000,
|
||||
});
|
||||
const heartbeat = heartbeatService(db);
|
||||
const recovery = recoveryService(db, { enqueueWakeup: vi.fn() });
|
||||
|
||||
const scan = await heartbeat.scanSilentActiveRuns({ now, companyId });
|
||||
const evaluationIssueId = scan.evaluationIssueIds[0];
|
||||
expect(evaluationIssueId).toBeTruthy();
|
||||
|
||||
await expect(
|
||||
recovery.recordWatchdogDecision({
|
||||
runId,
|
||||
actor: { type: "agent", agentId: managerId },
|
||||
decision: "continue",
|
||||
evaluationIssueId,
|
||||
reason: "client supplied another agent run",
|
||||
createdByRunId: runId,
|
||||
}),
|
||||
).rejects.toMatchObject({ status: 403 });
|
||||
|
||||
const managerRunId = randomUUID();
|
||||
await db.insert(heartbeatRuns).values({
|
||||
id: managerRunId,
|
||||
companyId,
|
||||
agentId: managerId,
|
||||
status: "running",
|
||||
invocationSource: "assignment",
|
||||
triggerDetail: "system",
|
||||
startedAt: now,
|
||||
processStartedAt: now,
|
||||
lastOutputAt: now,
|
||||
lastOutputSeq: 1,
|
||||
lastOutputStream: "stdout",
|
||||
contextSnapshot: {},
|
||||
logBytes: 0,
|
||||
});
|
||||
|
||||
const decision = await recovery.recordWatchdogDecision({
|
||||
runId,
|
||||
actor: { type: "agent", agentId: managerId, runId: managerRunId },
|
||||
decision: "continue",
|
||||
evaluationIssueId,
|
||||
reason: "valid current actor run",
|
||||
createdByRunId: randomUUID(),
|
||||
});
|
||||
expect(decision.createdByRunId).toBe(managerRunId);
|
||||
});
|
||||
});
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { REDACTED_EVENT_VALUE, redactEventPayload, sanitizeRecord } from "../redaction.js";
|
||||
import { REDACTED_EVENT_VALUE, redactEventPayload, redactSensitiveText, sanitizeRecord } from "../redaction.js";
|
||||
|
||||
describe("redaction", () => {
|
||||
it("redacts sensitive keys and nested secret values", () => {
|
||||
@@ -63,4 +63,25 @@ describe("redaction", () => {
|
||||
safe: "value",
|
||||
});
|
||||
});
|
||||
|
||||
it("redacts common secret shapes from unstructured text", () => {
|
||||
const jwt = "eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c";
|
||||
const githubToken = "ghp_1234567890abcdefghijklmnopqrstuvwxyz";
|
||||
const input = [
|
||||
"Authorization: Bearer live-bearer-token-value",
|
||||
`payload {"apiKey":"json-secret-value"}`,
|
||||
`escaped {\\"apiKey\\":\\"escaped-json-secret\\"}`,
|
||||
`GITHUB_TOKEN=${githubToken}`,
|
||||
`session=${jwt}`,
|
||||
].join("\n");
|
||||
|
||||
const result = redactSensitiveText(input);
|
||||
|
||||
expect(result).toContain(REDACTED_EVENT_VALUE);
|
||||
expect(result).not.toContain("live-bearer-token-value");
|
||||
expect(result).not.toContain("json-secret-value");
|
||||
expect(result).not.toContain("escaped-json-secret");
|
||||
expect(result).not.toContain(githubToken);
|
||||
expect(result).not.toContain(jwt);
|
||||
});
|
||||
});
|
||||
@@ -1,6 +1,16 @@
|
||||
const SECRET_PAYLOAD_KEY_RE =
|
||||
/(api[-_]?key|access[-_]?token|auth(?:_?token)?|authorization|bearer|secret|passwd|password|credential|jwt|private[-_]?key|cookie|connectionstring)/i;
|
||||
const JWT_VALUE_RE = /^[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+(?:\.[A-Za-z0-9_-]+)?$/;
|
||||
const JWT_TEXT_RE = /\b[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}(?:\.[A-Za-z0-9_-]{8,})?\b/g;
|
||||
const OPENAI_KEY_TEXT_RE = /\bsk-[A-Za-z0-9_-]{12,}\b/g;
|
||||
const GITHUB_TOKEN_TEXT_RE = /\bgh[pousr]_[A-Za-z0-9_]{20,}\b/g;
|
||||
const AUTHORIZATION_BEARER_TEXT_RE = /(\bAuthorization\s*:\s*Bearer\s+)[^\s"'`]+/gi;
|
||||
const ENV_SECRET_ASSIGNMENT_TEXT_RE =
|
||||
/(\b[A-Za-z0-9_]*(?:TOKEN|KEY|SECRET|PASSWORD|PASSWD|AUTHORIZATION|JWT)[A-Za-z0-9_]*\s*=\s*)[^\s"'`]+/gi;
|
||||
const JSON_SECRET_FIELD_TEXT_RE =
|
||||
/((?:"|')?(?:api[-_]?key|access[-_]?token|auth(?:_?token)?|authorization|bearer|secret|passwd|password|credential|jwt|private[-_]?key|cookie|connectionstring)(?:"|')?\s*:\s*(?:"|'))[^"'`\r\n]+((?:"|'))/gi;
|
||||
const ESCAPED_JSON_SECRET_FIELD_TEXT_RE =
|
||||
/((?:\\")?(?:api[-_]?key|access[-_]?token|auth(?:_?token)?|authorization|bearer|secret|passwd|password|credential|jwt|private[-_]?key|cookie|connectionstring)(?:\\")?\s*:\s*(?:\\"))[^\\\r\n]+((?:\\"))/gi;
|
||||
export const REDACTED_EVENT_VALUE = "***REDACTED***";
|
||||
|
||||
function isPlainObject(value: unknown): value is Record<string, unknown> {
|
||||
@@ -57,3 +67,14 @@ export function redactEventPayload(payload: Record<string, unknown> | null): Rec
|
||||
if (!isPlainObject(payload)) return payload;
|
||||
return sanitizeRecord(payload);
|
||||
}
|
||||
|
||||
export function redactSensitiveText(input: string): string {
|
||||
return input
|
||||
.replace(AUTHORIZATION_BEARER_TEXT_RE, `$1${REDACTED_EVENT_VALUE}`)
|
||||
.replace(JSON_SECRET_FIELD_TEXT_RE, `$1${REDACTED_EVENT_VALUE}$2`)
|
||||
.replace(ESCAPED_JSON_SECRET_FIELD_TEXT_RE, `$1${REDACTED_EVENT_VALUE}$2`)
|
||||
.replace(ENV_SECRET_ASSIGNMENT_TEXT_RE, `$1${REDACTED_EVENT_VALUE}`)
|
||||
.replace(OPENAI_KEY_TEXT_RE, REDACTED_EVENT_VALUE)
|
||||
.replace(GITHUB_TOKEN_TEXT_RE, REDACTED_EVENT_VALUE)
|
||||
.replace(JWT_TEXT_RE, REDACTED_EVENT_VALUE);
|
||||
}
|
||||
@@ -2628,7 +2628,7 @@ export function agentRoutes(db: Db) {
|
||||
evaluationIssueId,
|
||||
reason,
|
||||
snoozedUntil,
|
||||
createdByRunId: typeof req.body?.createdByRunId === "string" ? req.body.createdByRunId : null,
|
||||
createdByRunId: req.actor.runId ?? null,
|
||||
});
|
||||
|
||||
res.json(row);
|
||||
|
||||
@@ -15,6 +15,7 @@ import { runningProcesses } from "../../adapters/index.js";
|
||||
import { forbidden, notFound } from "../../errors.js";
|
||||
import { logger } from "../../middleware/logger.js";
|
||||
import { redactCurrentUserText } from "../../log-redaction.js";
|
||||
import { redactSensitiveText } from "../../redaction.js";
|
||||
import { logActivity } from "../activity-log.js";
|
||||
import { budgetService } from "../budgets.js";
|
||||
import { instanceSettingsService } from "../instance-settings.js";
|
||||
@@ -62,8 +63,8 @@ type LatestIssueRun = Pick<
|
||||
> | null;
|
||||
|
||||
type WatchdogDecisionActor =
|
||||
| { type: "board"; userId?: string | null }
|
||||
| { type: "agent"; agentId?: string | null }
|
||||
| { type: "board"; userId?: string | null; runId?: string | null }
|
||||
| { type: "agent"; agentId?: string | null; runId?: string | null }
|
||||
| { type: "none" };
|
||||
|
||||
export type RunOutputSilenceSummary = {
|
||||
@@ -589,11 +590,7 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup })
|
||||
}
|
||||
|
||||
function redactWatchdogEvidenceText(value: string, currentUserRedactionOptions: Awaited<ReturnType<typeof getCurrentUserRedactionOptions>>) {
|
||||
return redactCurrentUserText(value, currentUserRedactionOptions)
|
||||
.replace(/\b(sk-[a-zA-Z0-9_-]{12,})\b/g, "[REDACTED_API_KEY]")
|
||||
.replace(/\b([A-Za-z0-9_]*TOKEN[A-Za-z0-9_]*\s*=\s*)[^\s]+/gi, "$1[REDACTED]")
|
||||
.replace(/\b([A-Za-z0-9_]*KEY[A-Za-z0-9_]*\s*=\s*)[^\s]+/gi, "$1[REDACTED]")
|
||||
.replace(/\b([A-Za-z0-9_]*SECRET[A-Za-z0-9_]*\s*=\s*)[^\s]+/gi, "$1[REDACTED]");
|
||||
return redactSensitiveText(redactCurrentUserText(value, currentUserRedactionOptions));
|
||||
}
|
||||
|
||||
function truncateEvidenceText(value: string, maxChars = 4000) {
|
||||
@@ -1029,13 +1026,25 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup })
|
||||
.limit(1);
|
||||
if (!run) throw notFound("Heartbeat run not found");
|
||||
|
||||
let evaluationIssue: { id: string; assigneeAgentId: string | null; companyId: string } | null = null;
|
||||
let evaluationIssue: {
|
||||
id: string;
|
||||
assigneeAgentId: string | null;
|
||||
companyId: string;
|
||||
originKind: string;
|
||||
originId: string | null;
|
||||
hiddenAt: Date | null;
|
||||
status: string;
|
||||
} | null = null;
|
||||
if (input.evaluationIssueId) {
|
||||
evaluationIssue = await db
|
||||
.select({
|
||||
id: issues.id,
|
||||
assigneeAgentId: issues.assigneeAgentId,
|
||||
companyId: issues.companyId,
|
||||
originKind: issues.originKind,
|
||||
originId: issues.originId,
|
||||
hiddenAt: issues.hiddenAt,
|
||||
status: issues.status,
|
||||
})
|
||||
.from(issues)
|
||||
.where(and(eq(issues.id, input.evaluationIssueId), eq(issues.companyId, run.companyId)))
|
||||
@@ -1047,11 +1056,45 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup })
|
||||
const assignedRecoveryOwner =
|
||||
input.actor.type === "agent" &&
|
||||
Boolean(input.actor.agentId) &&
|
||||
evaluationIssue !== null &&
|
||||
evaluationIssue.originKind === STALE_ACTIVE_RUN_EVALUATION_ORIGIN_KIND &&
|
||||
evaluationIssue.originId === run.id &&
|
||||
evaluationIssue.hiddenAt === null &&
|
||||
!["done", "cancelled"].includes(evaluationIssue.status) &&
|
||||
evaluationIssue?.assigneeAgentId === input.actor.agentId;
|
||||
if (!boardActor && !assignedRecoveryOwner) {
|
||||
throw forbidden("Only the board or the assigned recovery owner can record watchdog decisions");
|
||||
}
|
||||
|
||||
if (evaluationIssue && (
|
||||
evaluationIssue.originKind !== STALE_ACTIVE_RUN_EVALUATION_ORIGIN_KIND ||
|
||||
evaluationIssue.originId !== run.id
|
||||
)) {
|
||||
throw forbidden("Watchdog decision evaluation issue is not bound to the target run");
|
||||
}
|
||||
|
||||
if (input.actor.type === "agent" && !evaluationIssue) {
|
||||
throw forbidden("Agent watchdog decisions require the target evaluation issue");
|
||||
}
|
||||
|
||||
const createdByRunId = input.actor.type === "agent"
|
||||
? input.actor.runId ?? input.createdByRunId ?? null
|
||||
: input.actor.type === "board"
|
||||
? input.actor.runId ?? input.createdByRunId ?? null
|
||||
: null;
|
||||
if (createdByRunId) {
|
||||
const [creatorRun] = await db
|
||||
.select({ id: heartbeatRuns.id, companyId: heartbeatRuns.companyId, agentId: heartbeatRuns.agentId })
|
||||
.from(heartbeatRuns)
|
||||
.where(eq(heartbeatRuns.id, createdByRunId))
|
||||
.limit(1);
|
||||
const sameCompany = creatorRun?.companyId === run.companyId;
|
||||
const sameAgent = input.actor.type !== "agent" || creatorRun?.agentId === input.actor.agentId;
|
||||
if (!creatorRun || !sameCompany || !sameAgent) {
|
||||
throw forbidden("createdByRunId is not valid for this watchdog decision actor");
|
||||
}
|
||||
}
|
||||
|
||||
const [row] = await db
|
||||
.insert(heartbeatRunWatchdogDecisions)
|
||||
.values({
|
||||
@@ -1063,7 +1106,7 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup })
|
||||
reason: input.reason ?? null,
|
||||
createdByAgentId: input.actor.type === "agent" ? input.actor.agentId ?? null : null,
|
||||
createdByUserId: input.actor.type === "board" ? input.actor.userId ?? null : null,
|
||||
createdByRunId: input.createdByRunId ?? null,
|
||||
createdByRunId,
|
||||
})
|
||||
.returning();
|
||||
|
||||
|
||||
Reference in new issue
Block a user