Redact current user in comments and token checks

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-11 22:17:21 -05:00
parent b1bf09970f
commit 088eaea0cb
6 changed files with 225 additions and 55 deletions
--- 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();
 }
--- a/server/src/tests/forbidden-tokens.test.ts
+++ 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.");
+  });
+});
--- 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<string, unknown> {
  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 };
 }

--- 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,
    },
  });
 }
--- 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<T extends { body: string }>(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]));
    },
  };
 }
--- 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<T extends { body: string }>(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: {