From 088eaea0cb7fc5e85301b3a36bfb4be68a309ccd Mon Sep 17 00:00:00 2001 From: Dotta Date: Wed, 11 Mar 2026 22:17:21 -0500 Subject: [PATCH] Redact current user in comments and token checks Co-Authored-By: Paperclip --- scripts/check-forbidden-tokens.mjs | 134 ++++++++++++------ server/src/__tests__/forbidden-tokens.test.ts | 77 ++++++++++ server/src/log-redaction.ts | 26 +++- server/src/services/activity-log.ts | 6 +- server/src/services/approvals.ts | 16 ++- server/src/services/issues.ts | 21 ++- 6 files changed, 225 insertions(+), 55 deletions(-) create mode 100644 server/src/__tests__/forbidden-tokens.test.ts diff --git a/scripts/check-forbidden-tokens.mjs b/scripts/check-forbidden-tokens.mjs index e94fd485..f34faae8 100644 --- a/scripts/check-forbidden-tokens.mjs +++ b/scripts/check-forbidden-tokens.mjs @@ -7,61 +7,109 @@ * working tree (not just staged changes). * * Token list: .git/hooks/forbidden-tokens.txt (one per line, # comments ok). - * If the file is missing, the check passes silently — other developers - * on the project won't have this list, and that's fine. + * If the file is missing, the check still uses the active local username when + * available. If username detection fails, the check degrades gracefully. */ import { execSync } from "node:child_process"; import { existsSync, readFileSync } from "node:fs"; +import os from "node:os"; import { resolve } from "node:path"; +import { fileURLToPath } from "node:url"; -const repoRoot = execSync("git rev-parse --show-toplevel", { encoding: "utf8" }).trim(); -const gitDir = execSync("git rev-parse --git-dir", { encoding: "utf8", cwd: repoRoot }).trim(); -const tokensFile = resolve(repoRoot, gitDir, "hooks/forbidden-tokens.txt"); - -if (!existsSync(tokensFile)) { - console.log(" ℹ Forbidden tokens list not found — skipping check."); - process.exit(0); +function uniqueNonEmpty(values) { + return Array.from(new Set(values.map((value) => value?.trim() ?? "").filter(Boolean))); } -const tokens = readFileSync(tokensFile, "utf8") - .split("\n") - .map((l) => l.trim()) - .filter((l) => l && !l.startsWith("#")); +export function resolveDynamicForbiddenTokens(env = process.env, osModule = os) { + const candidates = [env.USER, env.LOGNAME, env.USERNAME]; -if (tokens.length === 0) { - console.log(" ℹ Forbidden tokens list is empty — skipping check."); - process.exit(0); -} - -// Use git grep to search tracked files only (avoids node_modules, dist, etc.) -let found = false; - -for (const token of tokens) { try { - const result = execSync( - `git grep -in --no-color -- ${JSON.stringify(token)} -- ':!pnpm-lock.yaml' ':!.git'`, - { encoding: "utf8", cwd: repoRoot, stdio: ["pipe", "pipe", "pipe"] }, - ); - if (result.trim()) { - if (!found) { - console.error("ERROR: Forbidden tokens found in tracked files:\n"); - } - found = true; - // Print matches but DO NOT print which token was matched (avoids leaking the list) - const lines = result.trim().split("\n"); - for (const line of lines) { - console.error(` ${line}`); - } - } + candidates.push(osModule.userInfo().username); } catch { - // git grep returns exit code 1 when no matches — that's fine + // Some environments do not expose userInfo; env vars are enough fallback. } + + return uniqueNonEmpty(candidates); } -if (found) { - console.error("\nBuild blocked. Remove the forbidden token(s) before publishing."); - process.exit(1); -} else { - console.log(" ✓ No forbidden tokens found."); +export function readForbiddenTokensFile(tokensFile) { + if (!existsSync(tokensFile)) return []; + + return readFileSync(tokensFile, "utf8") + .split("\n") + .map((line) => line.trim()) + .filter((line) => line && !line.startsWith("#")); +} + +export function resolveForbiddenTokens(tokensFile, env = process.env, osModule = os) { + return uniqueNonEmpty([ + ...resolveDynamicForbiddenTokens(env, osModule), + ...readForbiddenTokensFile(tokensFile), + ]); +} + +export function runForbiddenTokenCheck({ + repoRoot, + tokens, + exec = execSync, + log = console.log, + error = console.error, +}) { + if (tokens.length === 0) { + log(" ℹ Forbidden tokens list is empty — skipping check."); + return 0; + } + + let found = false; + + for (const token of tokens) { + try { + const result = exec( + `git grep -in --no-color -- ${JSON.stringify(token)} -- ':!pnpm-lock.yaml' ':!.git'`, + { encoding: "utf8", cwd: repoRoot, stdio: ["pipe", "pipe", "pipe"] }, + ); + if (result.trim()) { + if (!found) { + error("ERROR: Forbidden tokens found in tracked files:\n"); + } + found = true; + const lines = result.trim().split("\n"); + for (const line of lines) { + error(` ${line}`); + } + } + } catch { + // git grep returns exit code 1 when no matches — that's fine + } + } + + if (found) { + error("\nBuild blocked. Remove the forbidden token(s) before publishing."); + return 1; + } + + log(" ✓ No forbidden tokens found."); + return 0; +} + +function resolveRepoPaths(exec = execSync) { + const repoRoot = exec("git rev-parse --show-toplevel", { encoding: "utf8" }).trim(); + const gitDir = exec("git rev-parse --git-dir", { encoding: "utf8", cwd: repoRoot }).trim(); + return { + repoRoot, + tokensFile: resolve(repoRoot, gitDir, "hooks/forbidden-tokens.txt"), + }; +} + +function main() { + const { repoRoot, tokensFile } = resolveRepoPaths(); + const tokens = resolveForbiddenTokens(tokensFile); + process.exit(runForbiddenTokenCheck({ repoRoot, tokens })); +} + +const isMainModule = process.argv[1] && resolve(process.argv[1]) === fileURLToPath(import.meta.url); + +if (isMainModule) { + main(); } diff --git a/server/src/__tests__/forbidden-tokens.test.ts b/server/src/__tests__/forbidden-tokens.test.ts new file mode 100644 index 00000000..0319311a --- /dev/null +++ b/server/src/__tests__/forbidden-tokens.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it, vi } from "vitest"; + +const { + resolveDynamicForbiddenTokens, + resolveForbiddenTokens, + runForbiddenTokenCheck, +} = await import("../../../scripts/check-forbidden-tokens.mjs"); + +describe("forbidden token check", () => { + it("derives username tokens without relying on whoami", () => { + const tokens = resolveDynamicForbiddenTokens( + { USER: "paperclip", LOGNAME: "paperclip", USERNAME: "pc" }, + { + userInfo: () => ({ username: "paperclip" }), + }, + ); + + expect(tokens).toEqual(["paperclip", "pc"]); + }); + + it("falls back cleanly when user resolution fails", () => { + const tokens = resolveDynamicForbiddenTokens( + {}, + { + userInfo: () => { + throw new Error("missing user"); + }, + }, + ); + + expect(tokens).toEqual([]); + }); + + it("merges dynamic and file-based forbidden tokens", async () => { + const fs = await import("node:fs"); + const os = await import("node:os"); + const path = await import("node:path"); + + const tokensFile = path.join(os.tmpdir(), `forbidden-tokens-${Date.now()}.txt`); + fs.writeFileSync(tokensFile, "# comment\npaperclip\ncustom-token\n"); + + try { + const tokens = resolveForbiddenTokens(tokensFile, { USER: "paperclip" }, { + userInfo: () => ({ username: "paperclip" }), + }); + + expect(tokens).toEqual(["paperclip", "custom-token"]); + } finally { + fs.unlinkSync(tokensFile); + } + }); + + it("reports matches without leaking which token was searched", () => { + const exec = vi + .fn() + .mockReturnValueOnce("server/file.ts:1:found\n") + .mockImplementation(() => { + throw new Error("not found"); + }); + const log = vi.fn(); + const error = vi.fn(); + + const exitCode = runForbiddenTokenCheck({ + repoRoot: "/repo", + tokens: ["paperclip", "custom-token"], + exec, + log, + error, + }); + + expect(exitCode).toBe(1); + expect(exec).toHaveBeenCalledTimes(2); + expect(error).toHaveBeenCalledWith("ERROR: Forbidden tokens found in tracked files:\n"); + expect(error).toHaveBeenCalledWith(" server/file.ts:1:found"); + expect(error).toHaveBeenCalledWith("\nBuild blocked. Remove the forbidden token(s) before publishing."); + }); +}); diff --git a/server/src/log-redaction.ts b/server/src/log-redaction.ts index 34af3bbd..07a28c58 100644 --- a/server/src/log-redaction.ts +++ b/server/src/log-redaction.ts @@ -8,6 +8,12 @@ interface CurrentUserRedactionOptions { homeDirs?: string[]; } +type CurrentUserCandidates = { + userNames: string[]; + homeDirs: string[]; + replacement: string; +}; + function isPlainObject(value: unknown): value is Record { if (typeof value !== "object" || value === null || Array.isArray(value)) return false; const proto = Object.getPrototypeOf(value); @@ -70,10 +76,24 @@ function defaultHomeDirs(userNames: string[]) { return uniqueNonEmpty(candidates); } +let cachedCurrentUserCandidates: CurrentUserCandidates | null = null; + +function getDefaultCurrentUserCandidates(): CurrentUserCandidates { + if (cachedCurrentUserCandidates) return cachedCurrentUserCandidates; + const userNames = defaultUserNames(); + cachedCurrentUserCandidates = { + userNames, + homeDirs: defaultHomeDirs(userNames), + replacement: CURRENT_USER_REDACTION_TOKEN, + }; + return cachedCurrentUserCandidates; +} + function resolveCurrentUserCandidates(opts?: CurrentUserRedactionOptions) { - const userNames = uniqueNonEmpty(opts?.userNames ?? defaultUserNames()); - const homeDirs = uniqueNonEmpty(opts?.homeDirs ?? defaultHomeDirs(userNames)); - const replacement = opts?.replacement?.trim() || CURRENT_USER_REDACTION_TOKEN; + const defaults = getDefaultCurrentUserCandidates(); + const userNames = uniqueNonEmpty(opts?.userNames ?? defaults.userNames); + const homeDirs = uniqueNonEmpty(opts?.homeDirs ?? defaults.homeDirs); + const replacement = opts?.replacement?.trim() || defaults.replacement; return { userNames, homeDirs, replacement }; } diff --git a/server/src/services/activity-log.ts b/server/src/services/activity-log.ts index 41cd2938..cdef68ec 100644 --- a/server/src/services/activity-log.ts +++ b/server/src/services/activity-log.ts @@ -1,6 +1,7 @@ import type { Db } from "@paperclipai/db"; import { activityLog } from "@paperclipai/db"; import { publishLiveEvent } from "./live-events.js"; +import { redactCurrentUserValue } from "../log-redaction.js"; import { sanitizeRecord } from "../redaction.js"; export interface LogActivityInput { @@ -17,6 +18,7 @@ export interface LogActivityInput { export async function logActivity(db: Db, input: LogActivityInput) { const sanitizedDetails = input.details ? sanitizeRecord(input.details) : null; + const redactedDetails = sanitizedDetails ? redactCurrentUserValue(sanitizedDetails) : null; await db.insert(activityLog).values({ companyId: input.companyId, actorType: input.actorType, @@ -26,7 +28,7 @@ export async function logActivity(db: Db, input: LogActivityInput) { entityId: input.entityId, agentId: input.agentId ?? null, runId: input.runId ?? null, - details: sanitizedDetails, + details: redactedDetails, }); publishLiveEvent({ @@ -40,7 +42,7 @@ export async function logActivity(db: Db, input: LogActivityInput) { entityId: input.entityId, agentId: input.agentId ?? null, runId: input.runId ?? null, - details: sanitizedDetails, + details: redactedDetails, }, }); } diff --git a/server/src/services/approvals.ts b/server/src/services/approvals.ts index 13e57c8a..53833044 100644 --- a/server/src/services/approvals.ts +++ b/server/src/services/approvals.ts @@ -2,9 +2,17 @@ import { and, asc, eq, inArray } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; import { approvalComments, approvals } from "@paperclipai/db"; import { notFound, unprocessable } from "../errors.js"; +import { redactCurrentUserText } from "../log-redaction.js"; import { agentService } from "./agents.js"; import { notifyHireApproved } from "./hire-hook.js"; +function redactApprovalComment(comment: T): T { + return { + ...comment, + body: redactCurrentUserText(comment.body), + }; +} + export function approvalService(db: Db) { const agentsSvc = agentService(db); const canResolveStatuses = new Set(["pending", "revision_requested"]); @@ -215,7 +223,8 @@ export function approvalService(db: Db) { eq(approvalComments.companyId, existing.companyId), ), ) - .orderBy(asc(approvalComments.createdAt)); + .orderBy(asc(approvalComments.createdAt)) + .then((comments) => comments.map(redactApprovalComment)); }, addComment: async ( @@ -224,6 +233,7 @@ export function approvalService(db: Db) { actor: { agentId?: string; userId?: string }, ) => { const existing = await getExistingApproval(approvalId); + const redactedBody = redactCurrentUserText(body); return db .insert(approvalComments) .values({ @@ -231,10 +241,10 @@ export function approvalService(db: Db) { approvalId, authorAgentId: actor.agentId ?? null, authorUserId: actor.userId ?? null, - body, + body: redactedBody, }) .returning() - .then((rows) => rows[0]); + .then((rows) => redactApprovalComment(rows[0])); }, }; } diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index a25d21fc..29995cd4 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -22,6 +22,7 @@ import { defaultIssueExecutionWorkspaceSettingsForProject, parseProjectExecutionWorkspacePolicy, } from "./execution-workspace-policy.js"; +import { redactCurrentUserText } from "../log-redaction.js"; const ALL_ISSUE_STATUSES = ["backlog", "todo", "in_progress", "in_review", "blocked", "done", "cancelled"]; @@ -88,6 +89,13 @@ type IssueUserContextInput = { updatedAt: Date | string; }; +function redactIssueComment(comment: T): T { + return { + ...comment, + body: redactCurrentUserText(comment.body), + }; +} + function sameRunLock(checkoutRunId: string | null, actorRunId: string | null) { if (actorRunId) return checkoutRunId === actorRunId; return checkoutRunId == null; @@ -1041,14 +1049,18 @@ export function issueService(db: Db) { .select() .from(issueComments) .where(eq(issueComments.issueId, issueId)) - .orderBy(desc(issueComments.createdAt)), + .orderBy(desc(issueComments.createdAt)) + .then((comments) => comments.map(redactIssueComment)), getComment: (commentId: string) => db .select() .from(issueComments) .where(eq(issueComments.id, commentId)) - .then((rows) => rows[0] ?? null), + .then((rows) => { + const comment = rows[0] ?? null; + return comment ? redactIssueComment(comment) : null; + }), addComment: async (issueId: string, body: string, actor: { agentId?: string; userId?: string }) => { const issue = await db @@ -1059,6 +1071,7 @@ export function issueService(db: Db) { if (!issue) throw notFound("Issue not found"); + const redactedBody = redactCurrentUserText(body); const [comment] = await db .insert(issueComments) .values({ @@ -1066,7 +1079,7 @@ export function issueService(db: Db) { issueId, authorAgentId: actor.agentId ?? null, authorUserId: actor.userId ?? null, - body, + body: redactedBody, }) .returning(); @@ -1076,7 +1089,7 @@ export function issueService(db: Db) { .set({ updatedAt: new Date() }) .where(eq(issues.id, issueId)); - return comment; + return redactIssueComment(comment); }, createAttachment: async (input: {