From 89e85377e3e30cae5a45734e86492baa03f4f9e4 Mon Sep 17 00:00:00 2001 From: Security Engineer Date: Mon, 14 Sep 2026 12:57:00 +0000 Subject: [PATCH 1/2] security(workspaces): scrub secrets on the workspace-operation write path (PEN-3205) `services/workspace-operations.ts` applied exactly one transform to captured command output: `redactCurrentUserText`, which is username censoring driven by `censorUsernameInLogs`. It is not a secret scrub, and `redactSensitiveText` appeared nowhere in that file or in `workspace-operation-log-store.ts`. That single sanitized-only chunk landed in all three durable sinks -- `stdoutExcerpt`, `stderrExcerpt`, and the log-store body. Why this is the PEN-2328 / PEN-2370 shape and not just "git output": `buildWorkspaceCommandEnv` (`services/workspace-runtime.ts:3658-3659`) builds the environment for operator-authored provision / cleanup / teardown / workspace-job commands as `{ ...process.env, ...buildAgentGitIdentityEnv(agent) }` -- the server process environment, wholesale. `recordWorkspaceCommandOperation` then captures that command's stdout and stderr straight into the excerpts and the log body. A command running under `set -x`, or any tool that dumps its environment on failure, wrote the server's environment into a durable company-readable channel with no secret scrub in the path. This is a write-path content defect, independent of PEN-3204: narrowing the read does not fix an unscrubbed write, and scrubbing the write does not by itself justify a standing company-wide read. Both halves are needed. Changes: - Extract `compactRunLogChunk` / `sanitizeRunLogChunkForStorage` out of `heartbeat.ts` into `services/log-chunk-sanitizer.ts`. `heartbeat.ts` already imports `workspace-operations.js`, so importing the sanitizer back from there would close a cycle -- which is why this path grew a weaker private transform in the first place. `heartbeat.ts` re-exports both so existing importers and tests keep their entry point. One definition, not two: a second sanitizer would be a second oracle, and the weaker one decides what a reviewer believes is covered. - Route captured chunks through the shared sanitizer ONCE, before the excerpt fork, so the excerpt columns and the log-store body cannot diverge. Scrubbing one and not the other is the half-control this replaces. - Censor `command` and `cwd` at write, consistent with the `metadata` beside them. A cwd under a censored user's home was masked in the log body and legible in the column next to it. `command` additionally gets the secret scrub -- an operator provision command can carry a credential as an inline argument. Null stays null and "" stays "": the transform applies to the value, it does not reclassify an empty command as an absent one. - Correct the `readLog` comment that claimed chunks were "sanitized before append-time storage". That overstated a username-censor-only transform, the same overstatement PEN-3139 corrected on the run-log path -- except here there was no secret scrub at all to overstate. - Apply the same username censoring on `GET /execution-workspaces/:id/workspace- operations` that the sibling list route in `routes/agents.ts` already applied. Both answer with the same historical rows including the excerpts, which `publicWorkspaceOperation` deliberately does NOT withhold, so censoring one route and not the other left the same bytes legible one URL over. New rows are censored at write now; this still covers rows stored before this commit. Tests fail first against the pre-fix path: the synthetic credential reaches both the excerpt column and the log-store body in the clear (3 of 5 assertions fail). The pass-through cases are the discriminator between scrubbing and blanking -- a fix that simply dropped the output would satisfy every `not.toContain` and fail those two. Scope checked and deliberately NOT changed here: `publicWorkspaceOperation` already masks `command`/`cwd` for unentitled viewers, and the excerpts are company-readable by a standing product decision (CTO Ruling F, PEN-3202). Narrowing that read is PEN-3204, not this row. Occurrence is unmeasured: the shape was unprotected, the rate is unknown, and no peer's operations were read. The backfill decision for already-stored rows is filed as PEN-3246 rather than silently skipped. Refs: PEN-3205, PEN-3246, PEN-3202, PEN-3153, PEN-3139 Signed-off-by: Security Engineer --- .../workspace-operation-secret-scrub.test.ts | 141 ++++++++++++++++++ server/src/routes/execution-workspaces.ts | 19 ++- server/src/services/heartbeat.ts | 33 +--- server/src/services/log-chunk-sanitizer.ts | 48 ++++++ server/src/services/workspace-operations.ts | 37 ++++- 5 files changed, 246 insertions(+), 32 deletions(-) create mode 100644 server/src/__tests__/workspace-operation-secret-scrub.test.ts create mode 100644 server/src/services/log-chunk-sanitizer.ts diff --git a/server/src/__tests__/workspace-operation-secret-scrub.test.ts b/server/src/__tests__/workspace-operation-secret-scrub.test.ts new file mode 100644 index 000000000000..ea7e1b5f81a0 --- /dev/null +++ b/server/src/__tests__/workspace-operation-secret-scrub.test.ts @@ -0,0 +1,141 @@ +import { describe, expect, it, vi, beforeEach } from "vitest"; + +// PEN-3205: the workspace-operation write path applied only `redactCurrentUserText` +// (username censoring) to captured command output, so a secret-shaped chunk reached +// BOTH durable sinks in the clear: the `stdoutExcerpt`/`stderrExcerpt` columns and the +// log-store body. The amplifier is `buildWorkspaceCommandEnv`, which hands operator +// commands `{ ...process.env }` wholesale — a command under `set -x`, or any tool that +// dumps its environment on failure, writes the server environment into a durable +// company-readable channel. +// +// These tests assert BOTH sinks on purpose. Scrubbing one and not the other is the +// exact half-control this row exists to close, and a test that only reads the excerpt +// would have passed against a fix that left the log body unscrubbed. + +const capturedAppends: { stream: string; chunk: string }[] = []; +const capturedUpdates: Record[] = []; +const capturedInserts: Record[] = []; + +vi.mock("../services/instance-settings.js", () => ({ + instanceSettingsService: () => ({ + getGeneral: async () => ({ censorUsernameInLogs: false }), + }), +})); + +vi.mock("../services/workspace-operation-log-store.js", () => ({ + getWorkspaceOperationLogStore: () => ({ + begin: async () => ({ store: "local_file", logRef: "test/log.ndjson" }), + append: async (_handle: unknown, event: { stream: string; chunk: string }) => { + capturedAppends.push({ stream: event.stream, chunk: event.chunk }); + }, + finalize: async () => ({ bytes: 0, sha256: "sha", compressed: false }), + read: async () => ({ content: "" }), + }), +})); + +const { workspaceOperationService } = await import("../services/workspace-operations.js"); + +function makeFakeDb() { + const thenableWhere = (payload: Record) => { + const row = { + id: "op-1", + companyId: "company-1", + phase: "provision", + status: "succeeded", + logCompressed: false, + startedAt: new Date(), + createdAt: new Date(), + updatedAt: new Date(), + ...payload, + }; + return { + returning: () => Promise.resolve([row]), + then: (resolve: (v: unknown) => unknown) => Promise.resolve([row]).then(resolve), + }; + }; + + return { + insert: () => ({ + values: async (payload: Record) => { + capturedInserts.push(payload); + }, + }), + update: () => ({ + set: (payload: Record) => { + capturedUpdates.push(payload); + return { where: () => thenableWhere(payload) }; + }, + }), + select: () => ({ from: () => ({ where: () => Promise.resolve([]) }) }), + } as never; +} + +const SECRET = "fake-pen3205-workspace-op-secret"; +const SECRET_CHUNK = `PAPERCLIP_SYNTHETIC_TOKEN=${SECRET}\nSAFE_ENV_NAME=visible\n`; + +async function runOperation(result: { stdout?: string; stderr?: string }) { + capturedAppends.length = 0; + capturedUpdates.length = 0; + capturedInserts.length = 0; + + const svc = workspaceOperationService(makeFakeDb()); + const recorder = svc.createRecorder({ companyId: "company-1" }); + await recorder.recordOperation({ + phase: "provision" as never, + command: "bash -lc 'set -x; env'", + cwd: "/workspace", + run: async () => ({ stdout: result.stdout ?? null, stderr: result.stderr ?? null }), + }); + + const finalUpdate = capturedUpdates.at(-1) ?? {}; + return { + stdoutExcerpt: (finalUpdate.stdoutExcerpt as string | null) ?? "", + stderrExcerpt: (finalUpdate.stderrExcerpt as string | null) ?? "", + logBody: capturedAppends.map((a) => a.chunk).join(""), + }; +} + +describe("workspace-operation captured output is secret-scrubbed at write time", () => { + beforeEach(() => { + capturedAppends.length = 0; + capturedUpdates.length = 0; + capturedInserts.length = 0; + }); + + it("scrubs a secret-shaped stdout chunk in the excerpt column", async () => { + const { stdoutExcerpt } = await runOperation({ stdout: SECRET_CHUNK }); + + expect(stdoutExcerpt).not.toContain(SECRET); + expect(stdoutExcerpt).toContain("PAPERCLIP_SYNTHETIC_TOKEN=***REDACTED***"); + }); + + it("scrubs the same chunk in the durable log-store body", async () => { + const { logBody } = await runOperation({ stdout: SECRET_CHUNK }); + + expect(logBody).not.toContain(SECRET); + expect(logBody).toContain("PAPERCLIP_SYNTHETIC_TOKEN=***REDACTED***"); + }); + + it("scrubs secret-shaped stderr too — the failure path is where env dumps land", async () => { + const { stderrExcerpt, logBody } = await runOperation({ stderr: SECRET_CHUNK }); + + expect(stderrExcerpt).not.toContain(SECRET); + expect(logBody).not.toContain(SECRET); + }); + + // Discriminates scrubbing from blanking: a fix that simply dropped the output + // would satisfy every "not.toContain" above. This is the control that fails it. + it("passes a non-secret chunk through unchanged in both sinks", async () => { + const benign = "Cloning into 'repo'...\nremote: Enumerating objects: 42, done.\n"; + const { stdoutExcerpt, logBody } = await runOperation({ stdout: benign }); + + expect(stdoutExcerpt).toBe(benign); + expect(logBody).toBe(benign); + }); + + it("keeps the non-secret line beside a scrubbed one", async () => { + const { stdoutExcerpt } = await runOperation({ stdout: SECRET_CHUNK }); + + expect(stdoutExcerpt).toContain("SAFE_ENV_NAME=visible"); + }); +}); diff --git a/server/src/routes/execution-workspaces.ts b/server/src/routes/execution-workspaces.ts index b7b026b3d298..4efe0239015e 100644 --- a/server/src/routes/execution-workspaces.ts +++ b/server/src/routes/execution-workspaces.ts @@ -13,6 +13,7 @@ import { import type { ExecutionWorkspace, ExecutionWorkspaceSummary, WorkspaceRuntimeDesiredState, WorkspaceRuntimeServiceStateMap } from "@paperclipai/shared"; import { validate } from "../middleware/validate.js"; import { accessService, executionWorkspaceService, heartbeatService, logActivity, workspaceOperationService } from "../services/index.js"; +import { instanceSettingsService } from "../services/instance-settings.js"; import { mergeExecutionWorkspaceConfig, readExecutionWorkspaceConfig } from "../services/execution-workspaces.js"; import { parseProjectExecutionWorkspacePolicy } from "../services/execution-workspace-policy.js"; import { readProjectWorkspaceRuntimeConfig } from "../services/project-workspace-runtime-config.js"; @@ -32,6 +33,7 @@ import { collectExecutionWorkspaceCommandPaths, } from "./workspace-command-authz.js"; import { assertCanManageExecutionWorkspaceRuntimeServices } from "./workspace-runtime-service-authz.js"; +import { redactCurrentUserValue } from "../log-redaction.js"; import { publicExecutionWorkspace, publicExecutionWorkspaces, @@ -51,6 +53,13 @@ export function executionWorkspaceRoutes(db: Db, opts: { pluginWorkerManager?: P const svc = executionWorkspaceService(db); const access = accessService(db); const workspaceOperationsSvc = workspaceOperationService(db); + const instanceSettings = instanceSettingsService(db); + + async function getCurrentUserRedactionOptions() { + return { + enabled: (await instanceSettings.getGeneral()).censorUsernameInLogs, + }; + } const heartbeat = heartbeatService(db, { pluginWorkerManager: opts.pluginWorkerManager, }); @@ -151,7 +160,15 @@ export function executionWorkspaceRoutes(db: Db, opts: { pluginWorkerManager?: P if (!(await assertExecutionWorkspaceReadAllowed(req, res, workspace.companyId))) return; const operations = await workspaceOperationsSvc.listForExecutionWorkspace(id); const viewer = await resolveWorkspaceRuntimeViewer(access, req, workspace.companyId); - res.json(publicWorkspaceOperations(operations, viewer)); + // PEN-3205: same username censoring as the sibling list route on `routes/agents.ts`. Both + // answer with the same historical `WorkspaceOperation` rows including `stdoutExcerpt` / + // `stderrExcerpt`, which `publicWorkspaceOperation` deliberately does NOT withhold, so + // censoring on one route and not the other left the same bytes legible one URL over. + // New rows are censored at write time as well; this still covers rows stored before that. + res.json(redactCurrentUserValue( + publicWorkspaceOperations(operations, viewer), + await getCurrentUserRedactionOptions(), + )); }); async function handleExecutionWorkspaceRuntimeCommand(req: Request, res: Response) { diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index f62d7e6a41a8..59d5cfedf662 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -571,7 +571,6 @@ const TERMINAL_RUN_STATUSES = new Set(["succeeded", "failed", "cancelled", "time import { serverVersion } from "../version.js"; const MAX_LIVE_LOG_CHUNK_BYTES = 8 * 1024; -const MAX_PERSISTED_LOG_CHUNK_CHARS = 64 * 1024; const MAX_RUN_EVENT_PAYLOAD_STRING_CHARS = 16 * 1024; const MAX_RUN_EVENT_PAYLOAD_ARRAY_ITEMS = 50; @@ -2145,7 +2144,6 @@ const activeRunExecutions = new Set(); // flips to "running"; this grace prevents reaping that startup race. Mirrors // the 5-min grace in cleanupManagedJobsWithoutRun. const ORPHANED_MANAGED_POD_REAP_GRACE_MS = 5 * 60 * 1000; -const INLINE_BASE64_IMAGE_DATA_RE = /("type":"image","source":\{"type":"base64","data":")([A-Za-z0-9+/=]{1024,})(")/g; const SESSION_ISOLATION_KEY_PARAM = "paperclipIsolationKey"; type RuntimeConfigSecretResolver = Pick< @@ -4116,30 +4114,13 @@ export function boundHeartbeatRunEventPayloadForStorage(payload: Record - `${prefix}[omitted base64 image data: ${data.length} chars]${suffix}`, - ); -} - -export function compactRunLogChunk(chunk: string, maxChars = MAX_PERSISTED_LOG_CHUNK_CHARS) { - const normalized = redactSensitiveText(redactInlineBase64ImageData(chunk)); - if (normalized.length <= maxChars) return normalized; - - const headChars = Math.max(0, Math.floor(maxChars * 0.6)); - const tailChars = Math.max(0, Math.floor(maxChars * 0.25)); - const omittedChars = Math.max(0, normalized.length - headChars - tailChars); - const marker = `\n[paperclip truncated run log chunk: omitted ${omittedChars} chars]\n`; - return `${normalized.slice(0, headChars)}${marker}${normalized.slice(normalized.length - tailChars)}`; -} - -export function sanitizeRunLogChunkForStorage( - chunk: string, - currentUserRedactionOptions: Parameters[1], - maxChars = MAX_PERSISTED_LOG_CHUNK_CHARS, -) { - return compactRunLogChunk(redactCurrentUserText(chunk, currentUserRedactionOptions), maxChars); -} +// PEN-3205: `compactRunLogChunk` / `sanitizeRunLogChunkForStorage` now live in +// `./log-chunk-sanitizer.js` so the workspace-operation write path can share the one +// definition (importing them from here would close a cycle — see that module's header). +// Imported for local use below and re-exported so existing importers and tests keep +// their current entry point. +import { compactRunLogChunk, sanitizeRunLogChunkForStorage } from "./log-chunk-sanitizer.js"; +export { compactRunLogChunk, sanitizeRunLogChunkForStorage }; const SYNTHETIC_KEEPALIVE_RUN_LOG_LINE_RE = /^\[paperclip\] keepalive\b.*\bjob\b.*\brunning \(\d+s since last output\)$/; diff --git a/server/src/services/log-chunk-sanitizer.ts b/server/src/services/log-chunk-sanitizer.ts new file mode 100644 index 000000000000..e0f5c918086c --- /dev/null +++ b/server/src/services/log-chunk-sanitizer.ts @@ -0,0 +1,48 @@ +import { redactCurrentUserText } from "../log-redaction.js"; +import { redactSensitiveText } from "../redaction.js"; + +/** + * Shared write-time sanitizer for captured command/agent output. + * + * PEN-3205: this used to live inside `heartbeat.ts`, which made it unreachable from + * `workspace-operations.ts` — `heartbeat.ts` already imports that module, so importing + * back would close a cycle. The workspace-operation write path therefore grew its own + * weaker transform (`redactCurrentUserText` alone, i.e. username censoring with no + * secret scrub) and persisted unscrubbed output to both the excerpt columns and the + * log-store body. + * + * Extracting it here rather than re-deriving an equivalent in the second caller is + * deliberate: two independent sanitizers would be two oracles that can silently drift, + * and the weaker one would decide what a reviewer believes is covered. `heartbeat.ts` + * re-exports these so its existing importers and tests keep their current entry points. + */ + +export const MAX_PERSISTED_LOG_CHUNK_CHARS = 64 * 1024; + +const INLINE_BASE64_IMAGE_DATA_RE = + /("type":"image","source":\{"type":"base64","data":")([A-Za-z0-9+/=]{1024,})(")/g; + +function redactInlineBase64ImageData(chunk: string) { + return chunk.replace(INLINE_BASE64_IMAGE_DATA_RE, (_match, prefix: string, data: string, suffix: string) => + `${prefix}[omitted base64 image data: ${data.length} chars]${suffix}`, + ); +} + +export function compactRunLogChunk(chunk: string, maxChars = MAX_PERSISTED_LOG_CHUNK_CHARS) { + const normalized = redactSensitiveText(redactInlineBase64ImageData(chunk)); + if (normalized.length <= maxChars) return normalized; + + const headChars = Math.max(0, Math.floor(maxChars * 0.6)); + const tailChars = Math.max(0, Math.floor(maxChars * 0.25)); + const omittedChars = Math.max(0, normalized.length - headChars - tailChars); + const marker = `\n[paperclip truncated run log chunk: omitted ${omittedChars} chars]\n`; + return `${normalized.slice(0, headChars)}${marker}${normalized.slice(normalized.length - tailChars)}`; +} + +export function sanitizeRunLogChunkForStorage( + chunk: string, + currentUserRedactionOptions: Parameters[1], + maxChars = MAX_PERSISTED_LOG_CHUNK_CHARS, +) { + return compactRunLogChunk(redactCurrentUserText(chunk, currentUserRedactionOptions), maxChars); +} diff --git a/server/src/services/workspace-operations.ts b/server/src/services/workspace-operations.ts index 1c0c13a824ff..a2848bdb99ab 100644 --- a/server/src/services/workspace-operations.ts +++ b/server/src/services/workspace-operations.ts @@ -6,6 +6,7 @@ import { asc, desc, eq, inArray, isNull, or, and } from "drizzle-orm"; import { notFound } from "../errors.js"; import { redactCurrentUserText, redactCurrentUserValue } from "../log-redaction.js"; import { instanceSettingsService } from "./instance-settings.js"; +import { sanitizeRunLogChunkForStorage } from "./log-chunk-sanitizer.js"; import { getWorkspaceOperationLogStore } from "./workspace-operation-log-store.js"; type WorkspaceOperationRow = typeof workspaceOperations.$inferSelect; @@ -123,7 +124,16 @@ export function workspaceOperationService(db: Db) { let stderrExcerpt = ""; const append = async (stream: "stdout" | "stderr" | "system", chunk: string | null | undefined) => { if (!chunk) return; - const sanitizedChunk = redactCurrentUserText(chunk, currentUserRedactionOptions); + // PEN-3205: this used to be `redactCurrentUserText` alone — username censoring, + // not a secret scrub — so captured command output reached BOTH durable sinks in + // the clear. It matters here specifically because `buildWorkspaceCommandEnv` + // hands operator provision/cleanup/teardown commands `{ ...process.env }` + // wholesale, so a command under `set -x`, or any tool that dumps its environment + // on failure, writes the server environment into a company-readable channel. + // Sanitize ONCE, before the excerpt fork, so the excerpt columns and the + // log-store body cannot diverge — scrubbing one and not the other is the exact + // half-control this replaced. + const sanitizedChunk = sanitizeRunLogChunkForStorage(chunk, currentUserRedactionOptions); if (stream === "stdout") stdoutExcerpt = appendExcerpt(stdoutExcerpt, sanitizedChunk); if (stream === "stderr") stderrExcerpt = appendExcerpt(stderrExcerpt, sanitizedChunk); await logStore.append(handle, { @@ -140,8 +150,18 @@ export function workspaceOperationService(db: Db) { heartbeatRunId: input.heartbeatRunId ?? null, issueId: input.issueId ?? null, phase: recordInput.phase, - command: recordInput.command ?? null, - cwd: recordInput.cwd ?? null, + // PEN-3205: `command` and `cwd` were stored raw while `metadata` beside them was + // censored, so a cwd under a censored user's home was masked in one column and + // legible in the next. `command` additionally gets the secret scrub: an operator + // provision command can carry a credential as an inline argument. + // Null/undefined still store null and "" still stores "" — the transform applies + // to the value, it does not reclassify an empty command as an absent one. + command: recordInput.command == null + ? null + : sanitizeRunLogChunkForStorage(recordInput.command, currentUserRedactionOptions), + cwd: recordInput.cwd == null + ? null + : redactCurrentUserText(recordInput.cwd, currentUserRedactionOptions), status: "running", logStore: handle.store, logRef: handle.logRef, @@ -253,8 +273,15 @@ export function workspaceOperationService(db: Db) { store: operation.logStore, logRef: operation.logRef, ...result, - // Workspace-operation log chunks are sanitized before append-time storage. - // Returning the stored chunk avoids another whole-string rewrite per poll. + // Workspace-operation log chunks are scrubbed for secret material AND + // username-censored before append-time storage, by the same + // `sanitizeRunLogChunkForStorage` the run-log path uses. Returning the stored chunk + // avoids another whole-string rewrite per poll. + // + // PEN-3205: before that fix this comment read "sanitized", which overstated a + // username-censor-only transform — the same overstatement PEN-3139 corrected on the + // run-log path. It is accurate as written now; if the append path above ever stops + // calling the shared sanitizer, correct this line in the same change. content: result.content, }; }, From 9feaa97d0a84edada8a8e0227787b14469094a8b Mon Sep 17 00:00:00 2001 From: Security Engineer Date: Mon, 14 Sep 2026 16:33:37 +0000 Subject: [PATCH 2/2] test(workspaces): stub instance settings on the withholding apps (PEN-3205) The workspace-operations route added in the parent commit reads `censorUsernameInLogs` per request, to apply the same username censoring `routes/agents.ts` already applied on the sibling list route. The PEN-2852 withholding apps mount `{}` as `db`, so the real `instanceSettingsService` reached a `getGeneral()` against a stub and the route answered 500 -- reddening both door-#15 tests. Typecheck was blind to it: the import is a direct module path, and the break is a runtime call through a stubbed `db`. Stub the service the way the other route tests in this suite already do, and default `censorUsernameInLogs` to OFF so the censor is inert underneath the withholding assertions -- those measure `publicWorkspaceOperation`, and a censor running beneath them could mask a sentinel and pass one for the wrong reason. That default would have left the route line itself uncovered, so cover it explicitly with a pair: excerpts are spread through `publicWorkspaceOperation` unmasked by design, so on this route the censor is the only control standing over `stdoutExcerpt`. The setting is the sole discriminator between the two cases -- removing the censor from the route fails the on-case (verified by mutation), and replacing it with blanket blanking fails the readability assertions. The home directory is read from `os.homedir()`, the same value `defaultHomeDirs` derives its module-cached candidates from, so the pair is deterministic on any runner without reaching into that cache. Signed-off-by: Security Engineer --- ...space-runtime-response-withholding.test.ts | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/server/src/__tests__/workspace-runtime-response-withholding.test.ts b/server/src/__tests__/workspace-runtime-response-withholding.test.ts index dc44022cb398..8922f0a2c63f 100644 --- a/server/src/__tests__/workspace-runtime-response-withholding.test.ts +++ b/server/src/__tests__/workspace-runtime-response-withholding.test.ts @@ -1,6 +1,7 @@ import express from "express"; import request from "supertest"; import { readFileSync } from "node:fs"; +import os from "node:os"; import { fileURLToPath } from "node:url"; import path from "node:path"; import { beforeEach, describe, expect, it, vi } from "vitest"; @@ -182,6 +183,24 @@ vi.mock("../routes/workspace-runtime-service-authz.js", () => ({ assertCanManageExecutionWorkspaceRuntimeServices: vi.fn(async () => undefined), })); +/** + * PEN-3205: the workspace-operations route now reads `censorUsernameInLogs` per request, to apply + * the username censoring the sibling list route in `routes/agents.ts` already applied. These apps + * mount `{}` as `db`, so the real service's `getGeneral()` throws and the route answers 500 — + * stubbing it is what keeps the withholding boundary below drivable at all. + * + * Default OFF so the censor is a no-op: the withholding assertions in this file measure + * `publicWorkspaceOperation`, and a censor running underneath them could mask a sentinel and make + * a withholding test pass for the wrong reason. The censor has its own coverage below, which + * turns it on explicitly and pairs it with the off-case as the discriminator. + */ +const mockInstanceGeneralSettings = vi.hoisted(() => ({ censorUsernameInLogs: false })); +vi.mock("../services/instance-settings.js", () => ({ + instanceSettingsService: () => ({ + getGeneral: async () => ({ ...mockInstanceGeneralSettings }), + }), +})); + function runtimeBlob(): Record { return { services: [{ name: "web", command: SECRET_SENTINEL, env: { TOKEN_FIXTURE: SECOND_SENTINEL } }], @@ -327,6 +346,7 @@ function createApp(mount: "execution-workspaces" | "projects") { describe("workspace runtime withholding boundary (PEN-2852)", () => { beforeEach(() => { vi.clearAllMocks(); + mockInstanceGeneralSettings.censorUsernameInLogs = false; decideAsUnprivilegedReader(); mockExecutionWorkspaceService.getById.mockResolvedValue(executionWorkspaceFixture()); mockExecutionWorkspaceService.list.mockResolvedValue([executionWorkspaceFixture()]); @@ -647,6 +667,51 @@ describe("workspace runtime withholding boundary (PEN-2852)", () => { expect(res.body[0].cwd).toBe(OPERATION_CWD_SENTINEL); }); + /** + * PEN-3205, read side. `publicWorkspaceOperation` masks `command`/`cwd`/`metadata` and spreads + * the rest, so `stdoutExcerpt` crosses this route UNMASKED by design — the username censor is + * the only control standing over it here, and `routes/agents.ts` was already applying it on + * the sibling list route while this one answered with a bare `res.json`. + * + * The home directory comes from `os.homedir()` rather than a literal because that is the same + * value `defaultHomeDirs` derives its (module-cached) candidate list from, so this is + * deterministic on any runner without reaching into that cache. The pair is the point: the + * setting is the sole discriminator between the two cases, so neither passes if the censor is + * dropped from the route, and neither passes if it is replaced by blanket blanking. + */ + it("censors the current user's home directory in the excerpt when the setting is on", async () => { + mockInstanceGeneralSettings.censorUsernameInLogs = true; + const homeDir = os.homedir(); + mockWorkspaceOperationService.listForExecutionWorkspace.mockResolvedValue([ + workspaceOperationFixture({ stdoutExcerpt: `cloned into ${homeDir}/checkout` }), + ]); + + const res = await request(createApp("execution-workspaces")).get( + "/api/execution-workspaces/workspace-1/workspace-operations", + ); + + expect(res.status).toBe(200); + expect(res.body[0].stdoutExcerpt).not.toContain(homeDir); + // Censored, not blanked: the surrounding line survives so the excerpt stays readable. + expect(res.body[0].stdoutExcerpt).toContain("cloned into "); + expect(res.body[0].stdoutExcerpt).toContain("/checkout"); + }); + + it("leaves the excerpt alone when the setting is off", async () => { + mockInstanceGeneralSettings.censorUsernameInLogs = false; + const homeDir = os.homedir(); + mockWorkspaceOperationService.listForExecutionWorkspace.mockResolvedValue([ + workspaceOperationFixture({ stdoutExcerpt: `cloned into ${homeDir}/checkout` }), + ]); + + const res = await request(createApp("execution-workspaces")).get( + "/api/execution-workspaces/workspace-1/workspace-operations", + ); + + expect(res.status).toBe(200); + expect(res.body[0].stdoutExcerpt).toBe(`cloned into ${homeDir}/checkout`); + }); + /** * Driven rather than source-pinned, at Ally's ask on the review of `024a330c`, and it is the * one that most deserved it: this exact handler is where the door was found. It masked