From ab7af3fbc872311db157308a9be1bdb628631609 Mon Sep 17 00:00:00 2001 From: Release Engineer Date: Sat, 19 Sep 2026 05:03:45 +0000 Subject: [PATCH 1/2] fix(workspace-operations): project and audit the operation log route (BLO-34631) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GET /workspace-operations/:operationId/log` was the only one of the four run/operation log surfaces with neither a read-time projection nor an access audit: it answered with `readLog`'s stored chunk verbatim and wrote nothing. - AC1: apply read-time `redactCurrentUserValue`, same as the sibling at `routes/execution-workspaces.ts`. Write-time censoring is not retroactive, so rows stored before it landed crossed uncensored on this route only. - AC2: audit allowed AND denied reads, matching `logRunLogAccessAudit` on `/heartbeat-runs/:runId/log`. The two call sites now share one helper. Keeps the cross-tenant 404 so the route is not an existence oracle. - AC3: withhold `content` (route) and `stdoutExcerpt`/`stderrExcerpt` (`publicWorkspaceOperation`) from a reader without `workspace_runtime:read`, masked not dropped. Consumer survey: every reader of these is a human UI or CLI surface (`AgentDetail.tsx`, `ExecutionWorkspaceDetail.tsx`, `paperclip run workspace-log`); no agent consumer, no MCP tool, no server-internal read. `allow_simple_company_member` grants the entitlement to every non-viewer board member, so those surfaces are unaffected. - AC4: no behaviour change for an entitled reader. The `workspace-response.ts` doc block that deferred this decision to BLO-33568 is rewritten to carry the measurement. `AgentDetail.tsx`'s log viewer now distinguishes withheld content from an empty log — `parseStoredLogContent` yields no chunks for either, which would have defeated withheld-is-not-absent. Every new assertion has a failing mutation (5 control runs, one guard each). Co-Authored-By: Claude --- .../__tests__/agent-live-run-routes.test.ts | 222 +++++++++++++++++- ...space-runtime-response-withholding.test.ts | 46 +++- server/src/routes/agents.ts | 103 ++++++-- server/src/routes/execution-workspaces.ts | 9 +- server/src/routes/workspace-response.ts | 31 ++- ui/src/pages/AgentDetail.tsx | 9 +- 6 files changed, 385 insertions(+), 35 deletions(-) diff --git a/server/src/__tests__/agent-live-run-routes.test.ts b/server/src/__tests__/agent-live-run-routes.test.ts index a8850e340b28..55b82401a83d 100644 --- a/server/src/__tests__/agent-live-run-routes.test.ts +++ b/server/src/__tests__/agent-live-run-routes.test.ts @@ -29,8 +29,61 @@ const mockInstanceSettingsService = vi.hoisted(() => ({ })); const mockLogActivity = vi.hoisted(() => vi.fn()); +/** + * BLO-34631. `/workspace-operations/:operationId/log` was the only one of the four run/operation + * log surfaces with neither a read-time projection nor an access audit — it answered with the + * stored chunk verbatim and wrote nothing. These cover both, plus the entitled control. + * + * Every value below is invented; no real credential, command or path is quoted, per the parent + * series' standing prohibition. + */ +const mockWorkspaceOperationService = vi.hoisted(() => ({ + getById: vi.fn(), + readLog: vi.fn(), +})); + +/** + * Flippable so the entitled and unentitled readers are separate cases. The default allows + * everything, which models the board actor the rest of this file drives with; denying only + * `workspace_runtime:read` models a standard same-company agent, which holds `company_scope:read` + * and `runtime:manage` but deliberately NOT this one (PEN-2852, `allow_company_agent`). + */ +const mockAccessDecide = vi.hoisted(() => vi.fn()); + const routeAgentId = "11111111-1111-4111-8111-111111111111"; +const WORKSPACE_OPERATION_LOG_SENTINEL = + "TOKEN_FIXTURE=sentinel-operation-log-not-a-real-credential ./deploy.sh"; + +function workspaceOperationLogFixture(overrides: Record = {}) { + return { + id: "operation-1", + companyId: "company-1", + heartbeatRunId: "run-1", + logStore: "local_file", + logRef: "logs/operation-1.ndjson", + ...overrides, + }; +} + +function allowEveryAction() { + mockAccessDecide.mockImplementation(async (input: { action?: string }) => ({ + allowed: true, + action: input.action, + reason: "allow_explicit_grant", + explanation: "Allowed by test grant.", + })); +} + +function denyWorkspaceRuntimeRead() { + mockAccessDecide.mockImplementation(async (input: { action?: string }) => ({ + allowed: input.action !== "workspace_runtime:read", + action: input.action, + reason: "test", + explanation: "Allowed by test mock.", + })); +} + function registerModuleMocks() { vi.doMock("../routes/authz.js", async () => vi.importActual("../routes/authz.js")); @@ -65,12 +118,7 @@ function registerModuleMocks() { agentInstructionsService: () => ({}), accessService: () => ({ canUser: vi.fn(async () => true), - decide: vi.fn(async (input: { action?: string }) => ({ - allowed: true, - action: input.action, - reason: "allow_explicit_grant", - explanation: "Allowed by test grant.", - })), + decide: mockAccessDecide, hasPermission: vi.fn(async () => true), }), approvalService: () => ({}), @@ -83,7 +131,7 @@ function registerModuleMocks() { logActivity: mockLogActivity, secretService: () => ({}), syncInstructionsBundleConfigFromFilePath: vi.fn((_agent, config) => config), - workspaceOperationService: () => ({}), + workspaceOperationService: () => mockWorkspaceOperationService, })); vi.doMock("../adapters/index.js", () => ({ @@ -290,6 +338,15 @@ describe("agent live run routes", () => { registerModuleMocks(); vi.clearAllMocks(); mockLogActivity.mockResolvedValue(undefined); + allowEveryAction(); + mockWorkspaceOperationService.getById.mockResolvedValue(workspaceOperationLogFixture()); + mockWorkspaceOperationService.readLog.mockResolvedValue({ + operationId: "operation-1", + store: "local_file", + logRef: "logs/operation-1.ndjson", + content: WORKSPACE_OPERATION_LOG_SENTINEL, + nextOffset: 9, + }); mockIssueService.getByIdentifier.mockResolvedValue({ id: "issue-1", companyId: "company-1", @@ -544,6 +601,157 @@ describe("agent live run routes", () => { expect(mockLogActivity.mock.calls[0]?.[1]?.details).not.toHaveProperty("logRef"); }); + /** + * BLO-34631 AC 1/2/3/4. Four cases, one per control, so a pass names which one it proved. + * + * The sentinel is the log CONTENT rather than a row field: the route answered with + * `readLog`'s result unprojected, so a fixture whose content is inert could not show it. + */ + it("withholds workspace-operation log content from a reader without workspace_runtime:read", async () => { + denyWorkspaceRuntimeRead(); + + const res = await requestApp( + await createApp({}, { + type: "agent", + agentId: routeAgentId, + companyId: "company-1", + companyIds: ["company-1"], + source: "agent_key", + runId: "actor-run-1", + }), + (baseUrl) => request(baseUrl).get("/api/workspace-operations/operation-1/log?offset=0&limitBytes=64"), + ); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(JSON.stringify(res.body)).not.toContain(WORKSPACE_OPERATION_LOG_SENTINEL); + // Masked, not emptied: a withheld reader must still tell "logged nothing" from "withheld". + expect(res.body.content).toBe("***REDACTED***"); + // The opaque handles stay — the route they point at is the one that now withholds. + expect(res.body.logRef).toBe("logs/operation-1.ndjson"); + }); + + it("discloses workspace-operation log content to a reader holding workspace_runtime:read", async () => { + const res = await requestApp( + await createApp(), + (baseUrl) => request(baseUrl).get("/api/workspace-operations/operation-1/log?offset=7&limitBytes=64"), + ); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(res.body.content).toBe(WORKSPACE_OPERATION_LOG_SENTINEL); + expect(mockWorkspaceOperationService.readLog).toHaveBeenCalledWith("operation-1", { + offset: 7, + limitBytes: 64, + }); + expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ + companyId: "company-1", + actorType: "user", + actorId: "local-board", + action: "workspace_operation.log_accessed", + entityType: "workspace_operation", + entityId: "operation-1", + // The operation's own run, so an audit reader can join back to the run that produced it. + runId: "run-1", + details: expect.objectContaining({ + result: "allowed", + actorSource: "local_implicit", + offset: 7, + limitBytes: 64, + logStore: "local_file", + }), + })); + expect(mockLogActivity.mock.calls[0]?.[1]?.details).not.toHaveProperty("content"); + }); + + it("audits denied workspace-operation log access without reading content", async () => { + const res = await requestApp( + await createApp({}, { + type: "agent", + agentId: routeAgentId, + companyId: "company-2", + companyIds: ["company-2"], + source: "agent_key", + runId: "actor-run-1", + }), + (baseUrl) => request(baseUrl).get("/api/workspace-operations/operation-1/log?offset=4&limitBytes=32"), + ); + + // Cross-tenant stays a 404 so the route is not an existence oracle, but the attempt is recorded. + expect(res.status).toBe(404); + expect(mockWorkspaceOperationService.readLog).not.toHaveBeenCalled(); + expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ + companyId: "company-1", + actorType: "agent", + actorId: routeAgentId, + agentId: routeAgentId, + action: "workspace_operation.log_accessed", + entityType: "workspace_operation", + entityId: "operation-1", + details: expect.objectContaining({ + result: "denied", + actorSource: "agent_key", + actorRunId: "actor-run-1", + offset: 4, + limitBytes: 32, + }), + })); + }); + + /** + * AC 1. Write-time username censoring is not retroactive, so a chunk stored before it landed is + * in the store uncensored and crossed verbatim on this route only. Paired with the off-case + * below: the setting is the sole discriminator, so neither passes if the read-time censor is + * dropped, and neither passes if it is replaced by blanket blanking. + * + * `os.homedir()` rather than a literal, because that is the value `defaultHomeDirs` derives its + * module-cached candidate list from. + */ + it("censors the current user's home directory in stored log content when the setting is on", async () => { + const { default: os } = await vi.importActual("node:os"); + const homeDir = os.homedir(); + mockInstanceSettingsService.getGeneral.mockResolvedValue({ + censorUsernameInLogs: true, + feedbackDataSharingPreference: "prompt", + }); + mockWorkspaceOperationService.readLog.mockResolvedValue({ + operationId: "operation-1", + store: "local_file", + logRef: "logs/operation-1.ndjson", + content: `cloned into ${homeDir}/checkout`, + nextOffset: 9, + }); + + const res = await requestApp( + await createApp(), + (baseUrl) => request(baseUrl).get("/api/workspace-operations/operation-1/log"), + ); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(res.body.content).not.toContain(homeDir); + // Censored, not blanked: the surrounding line survives so the log stays readable. + expect(res.body.content).toContain("cloned into "); + expect(res.body.content).toContain("/checkout"); + }); + + it("leaves stored log content alone when the censor setting is off", async () => { + const { default: os } = await vi.importActual("node:os"); + const homeDir = os.homedir(); + mockWorkspaceOperationService.readLog.mockResolvedValue({ + operationId: "operation-1", + store: "local_file", + logRef: "logs/operation-1.ndjson", + content: `cloned into ${homeDir}/checkout`, + nextOffset: 9, + }); + + const res = await requestApp( + await createApp(), + (baseUrl) => request(baseUrl).get("/api/workspace-operations/operation-1/log"), + ); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(res.body.content).toBe(`cloned into ${homeDir}/checkout`); + }); + it("caps company live run polling by default", async () => { const rows = Array.from({ length: 75 }, (_, index) => ({ id: `run-${index}`, diff --git a/server/src/__tests__/workspace-runtime-response-withholding.test.ts b/server/src/__tests__/workspace-runtime-response-withholding.test.ts index 03445d293bc3..0c527dcae123 100644 --- a/server/src/__tests__/workspace-runtime-response-withholding.test.ts +++ b/server/src/__tests__/workspace-runtime-response-withholding.test.ts @@ -56,6 +56,14 @@ const OPERATION_COMMAND_SENTINEL = "TOKEN_FIXTURE=sentinel-operation-command-not const OPERATION_CWD_SENTINEL = "/fixture/sentinel-operation-cwd"; const OPERATION_METADATA_SENTINEL = "/fixture/sentinel-operation-worktree-path"; +/** + * BLO-34631. Command *output*, and distinct from the `command` sentinel on purpose: the whole + * question this ticket settled is whether withholding the command while disclosing its output is a + * boundary or a gap, so an assertion has to name which of the two it closed. Both invented. + */ +const OPERATION_STDOUT_SENTINEL = "sentinel-operation-stdout-must-not-egress"; +const OPERATION_STDERR_SENTINEL = "sentinel-operation-stderr-must-not-egress"; + /** * PEN-3073. The lifecycle command scalars that sit BESIDE `workspaceRuntime` on the same config * object, and their siblings on the three nouns that carry the same strings elsewhere. Each gets its @@ -840,10 +848,38 @@ describe("workspace runtime withholding boundary (PEN-2852)", () => { }); /** - * 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`. + * BLO-34631. `stdoutExcerpt` / `stderrExcerpt` used to ride the spread in + * `publicWorkspaceOperation` on the "command output is not a copy of a declared-withheld value" + * reading. The output of a withheld command discloses the command — shells echo, `set -x` + * prints everything — and the write-time scrub is a heuristic secret matcher, not a boundary. + * The consumer survey found no agent or viewer flow that needs the raw value, so they are + * withheld on the same entitlement as `command`/`cwd`. + */ + it("withholds the operation excerpts from a reader without workspace_runtime:read", async () => { + mockWorkspaceOperationService.listForExecutionWorkspace.mockResolvedValue([ + workspaceOperationFixture({ + stdoutExcerpt: OPERATION_STDOUT_SENTINEL, + stderrExcerpt: OPERATION_STDERR_SENTINEL, + }), + ]); + + const res = await request(createApp("execution-workspaces")).get( + "/api/execution-workspaces/workspace-1/workspace-operations", + ); + + expect(res.status).toBe(200); + expect(JSON.stringify(res.body)).not.toContain(OPERATION_STDOUT_SENTINEL); + expect(JSON.stringify(res.body)).not.toContain(OPERATION_STDERR_SENTINEL); + // Masked, not dropped — withheld-is-not-absent, same contract as `publicRuntimeServices`. + expect(res.body[0].stdoutExcerpt).toBe(REDACTED_EVENT_VALUE); + expect(res.body[0].stderrExcerpt).toBe(REDACTED_EVENT_VALUE); + // `logRef` / `logStore` stay: opaque handles, and their route withholds the content itself. + expect(res.body[0].logBytes).toBe(4096); + }); + + /** + * PEN-3205, read side, now scoped to the entitled reader (BLO-34631 masks the excerpt for an + * unentitled one, so this case would pass for the wrong reason without the grant). * * 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 @@ -852,6 +888,7 @@ describe("workspace runtime withholding boundary (PEN-2852)", () => { * 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 () => { + decideAsRuntimeManager(); mockInstanceGeneralSettings.censorUsernameInLogs = true; const homeDir = os.homedir(); mockWorkspaceOperationService.listForExecutionWorkspace.mockResolvedValue([ @@ -870,6 +907,7 @@ describe("workspace runtime withholding boundary (PEN-2852)", () => { }); it("leaves the excerpt alone when the setting is off", async () => { + decideAsRuntimeManager(); mockInstanceGeneralSettings.censorUsernameInLogs = false; const homeDir = os.homedir(); mockWorkspaceOperationService.listForExecutionWorkspace.mockResolvedValue([ diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index bc7c214efac8..3e361f7e60b1 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -2,7 +2,7 @@ import { Router, type Request, type Response } from "express"; import { generateKeyPairSync, randomUUID } from "node:crypto"; import path from "node:path"; import type { Db } from "@paperclipai/db"; -import { REDACTED_EVENT_VALUE, isPlainObject, redactAgentConfigPayload, redactEventPayload } from "../redaction.js"; +import { REDACTED_EVENT_VALUE, isPlainObject, maskWorkspaceRuntimeTextForRead, redactAgentConfigPayload, redactEventPayload } from "../redaction.js"; import { diffAgentAdapterSecretBindings } from "../services/agent-secret-bindings.js"; import { agentRuntimeState, agents as agentsTable, companies, heartbeatRuns, issues as issuesTable, projects as projectsTable } from "@paperclipai/db"; import { and, asc, desc, eq, gte, inArray, not, or, sql } from "drizzle-orm"; @@ -366,33 +366,62 @@ export function agentRoutes( const instanceSettings = instanceSettingsService(db); const strictSecretsMode = process.env.PAPERCLIP_SECRETS_STRICT_MODE === "true"; - async function logRunLogAccessAudit( + /** + * BLO-34631. Shared by both raw-log surfaces. `/workspace-operations/:id/log` serves the same + * class of bytes as `/heartbeat-runs/:runId/log` — stored log content, not a projected row — and + * was the only one of the four run/operation log surfaces writing no audit record at all, so + * "who read this log" had no answer for it. Both call sites audit allowed AND denied reads. + */ + async function logLogAccessAudit( req: Request, - run: { id: string; companyId: string; logStore: string | null }, + entity: { + companyId: string; + entityType: "heartbeat_run" | "workspace_operation"; + entityId: string; + runId: string | null; + logStore: string | null; + }, result: "allowed" | "denied", opts: { offset: number; limitBytes: number }, ) { const actor = getRunLogAuditActor(req); await logActivity(db, { - companyId: run.companyId, + companyId: entity.companyId, actorType: actor.actorType, actorId: actor.actorId, agentId: actor.agentId, - runId: run.id, - action: "heartbeat.run_log_accessed", - entityType: "heartbeat_run", - entityId: run.id, + runId: entity.runId, + action: entity.entityType === "heartbeat_run" + ? "heartbeat.run_log_accessed" + : "workspace_operation.log_accessed", + entityType: entity.entityType, + entityId: entity.entityId, details: { result, actorSource: actor.actorSource, actorRunId: actor.actorRunId, offset: opts.offset, limitBytes: opts.limitBytes, - logStore: run.logStore, + logStore: entity.logStore, }, }); } + async function logRunLogAccessAudit( + req: Request, + run: { id: string; companyId: string; logStore: string | null }, + result: "allowed" | "denied", + opts: { offset: number; limitBytes: number }, + ) { + await logLogAccessAudit(req, { + companyId: run.companyId, + entityType: "heartbeat_run", + entityId: run.id, + runId: run.id, + logStore: run.logStore, + }, result, opts); + } + async function assertAgentEnvironmentSelection( companyId: string, adapterType: string, @@ -5065,18 +5094,64 @@ export function agentRoutes( router.get("/workspace-operations/:operationId/log", async (req, res) => { const operationId = req.params.operationId as string; - const operation = await getAccessibleResource(req, res, workspaceOperations.getById(operationId), "Workspace operation not found"); - if (!operation) return; - const offset = Number(req.query.offset ?? 0); + const normalizedOffset = Number.isFinite(offset) ? offset : 0; const limitBytes = readRunLogLimitBytes(req.query.limitBytes); + const operation = await workspaceOperations.getById(operationId); + if (!operation) { + res.status(404).json({ error: "Workspace operation not found" }); + return; + } + + const audit = (result: "allowed" | "denied") => logLogAccessAudit(req, { + companyId: operation.companyId, + entityType: "workspace_operation", + entityId: operation.id, + runId: operation.heartbeatRunId, + logStore: operation.logStore, + }, result, { offset: normalizedOffset, limitBytes }); + + // Same shape as `/heartbeat-runs/:runId/log` rather than `getAccessibleResource`: keep the + // cross-tenant 404 so this route is not an existence oracle, without silently dropping the + // denied access event. `getAccessibleResource`'s own doc block names audit-logged denials as + // the case that should compose `hasCompanyAccess` directly. + if (!hasCompanyAccess(req, operation.companyId)) { + await audit("denied"); + res.status(404).json({ error: "Workspace operation not found" }); + return; + } + + try { + assertCompanyAccess(req, operation.companyId); + } catch (error) { + await audit("denied"); + throw error; + } + + await audit("allowed"); + const viewer = await resolveWorkspaceRuntimeViewer(access, req, operation.companyId); const result = await workspaceOperations.readLog(operationId, { - offset: Number.isFinite(offset) ? offset : 0, + offset: normalizedOffset, limitBytes, }); res.set("Cache-Control", "no-cache, no-store"); - res.json(result); + // BLO-34631. `content` is the stored chunk verbatim — the write-time sanitizer is a heuristic + // secret matcher, not a withholding boundary, so it lets host paths, repo layout and any + // operator command echoed by `set -x` through. That is the same text `publicWorkspaceOperation` + // withholds one projection over, so it is withheld on the same entitlement. Masked rather than + // emptied, matching `publicRuntimeServices`: a withheld reader can still tell "this operation + // logged nothing" from "the log was withheld". `redactCurrentUserValue` still runs for the + // entitled reader, because write-time username censoring is not retroactive. + res.json(redactCurrentUserValue( + { + ...result, + content: viewer.revealRuntimeConfig + ? result.content + : maskWorkspaceRuntimeTextForRead(result.content), + }, + await getCurrentUserRedactionOptions(), + )); }); router.get("/issues/:issueId/live-runs", async (req, res) => { diff --git a/server/src/routes/execution-workspaces.ts b/server/src/routes/execution-workspaces.ts index 98d9e4d5b7aa..f28e521925e8 100644 --- a/server/src/routes/execution-workspaces.ts +++ b/server/src/routes/execution-workspaces.ts @@ -162,10 +162,11 @@ export function executionWorkspaceRoutes(db: Db, opts: { pluginWorkerManager?: P const operations = await workspaceOperationsSvc.listForExecutionWorkspace(id); const viewer = await resolveWorkspaceRuntimeViewer(access, req, workspace.companyId); // 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. + // answer with the same historical `WorkspaceOperation` rows, 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. (BLO-34631 additionally withholds + // `stdoutExcerpt` / `stderrExcerpt` from an unentitled reader inside + // `publicWorkspaceOperation`; the censor below is what stands over them for an entitled one.) res.json(redactCurrentUserValue( publicWorkspaceOperations(operations, viewer), await getCurrentUserRedactionOptions(), diff --git a/server/src/routes/workspace-response.ts b/server/src/routes/workspace-response.ts index 5941cffa61b6..5228bde510de 100644 --- a/server/src/routes/workspace-response.ts +++ b/server/src/routes/workspace-response.ts @@ -117,8 +117,11 @@ import type { accessService } from "../services/index.js"; * caller already holds, while `cleanup_command` is the operator's own string. * - `ExecutionWorkspaceStrategy.type` / `.runScope` — closed enums. `.baseRef` / `.branchTemplate` * are git refs and templates the branch-naming UI renders and the agent needs to name its branch. - * - `stdoutExcerpt` / `stderrExcerpt` / `logRef` on operations — command *output*, not a copy of a - * declared-withheld value (CTO Ruling F §4, BLO-33407). Unchanged by this ticket. + * - `logRef` / `logStore` on operations — opaque handles, and the route they point at + * (`/workspace-operations/:id/log`) withholds its content on the same entitlement as of + * BLO-34631. `stdoutExcerpt` / `stderrExcerpt` used to sit on this list on the "command output, + * not a copy of a declared-withheld value" reading (CTO Ruling F §4, BLO-33407); BLO-34631 + * surveyed the consumers, found no agent or viewer flow that needs them, and withholds them. * * ### PEN-3252 — the bypass this module recorded as open, now closed * @@ -442,9 +445,25 @@ export function publicRuntimeServices( * unentitled operator must still be able to see that an operation ran and how it ended — withholding * the operator's text is the point, hiding the fact of execution is not. * - * `stdoutExcerpt` / `stderrExcerpt` / `logRef` are deliberately NOT withheld here. They are command - * *output*, not a copy of a declared-withheld value, so they sit on the far side of BLO-33568's rule - * and are a product decision rather than a projection bug (CTO Ruling F §4, BLO-33407). + * `stdoutExcerpt` / `stderrExcerpt` are withheld too, as of BLO-34631, and that is a measurement + * rather than a symmetry argument. They are command *output*, so they are not a copy of a + * declared-withheld value — but the output of a withheld command discloses the command: shells + * echo, `npm` prints the script it runs, `set -x` prints everything, and the only control standing + * over the bytes is the write-time `redactSensitiveText` heuristic, which matches env-dump + * assignments, JSON secret fields and URI credentials and nothing else. Host paths, repo layout + * and an operator's `cleanupCommand` cross it intact. + * + * The consumer survey that decides it (BLO-34631 AC 3): every reader of these two fields and of + * `/workspace-operations/:id/log` is a human UI or CLI surface — `ui/src/pages/AgentDetail.tsx`, + * `ui/src/pages/ExecutionWorkspaceDetail.tsx`, and `paperclip run workspace-log`. There is no agent + * consumer, no MCP tool and no server-internal read. `workspace_runtime:read` is granted by + * `allow_simple_company_member` to every non-viewer board member, so all three surfaces keep the + * raw value for the humans that use them; what loses it is the actor class PEN-2852 built the + * entitlement to exclude — same-company agents, viewers, low-trust principals and bridge keys. + * + * `logRef` / `logStore` stay: they are opaque handles, and the route they point at now withholds + * the content on this same entitlement. Masking a pointer while its route still served the bytes + * would have been theatre. */ export function publicWorkspaceOperation( operation: WorkspaceOperation, @@ -455,6 +474,8 @@ export function publicWorkspaceOperation( ...operation, command: maskWorkspaceRuntimeTextForRead(operation.command), cwd: maskWorkspaceRuntimeTextForRead(operation.cwd), + stdoutExcerpt: maskWorkspaceRuntimeTextForRead(operation.stdoutExcerpt), + stderrExcerpt: maskWorkspaceRuntimeTextForRead(operation.stderrExcerpt), metadata: maskWorkspaceRuntimeForRead(operation.metadata) as Record | null, }; } diff --git a/ui/src/pages/AgentDetail.tsx b/ui/src/pages/AgentDetail.tsx index a7e5629bde57..047a0d730678 100644 --- a/ui/src/pages/AgentDetail.tsx +++ b/ui/src/pages/AgentDetail.tsx @@ -553,7 +553,14 @@ function WorkspaceOperationLogViewer({ )} {!isLoading && !error && chunks.length === 0 && ( -
No persisted log lines.
+
+ {/* BLO-34631: the API masks withheld log content rather than emptying it, so the + viewer has to tell the two apart — `parseStoredLogContent` yields no chunks for + either. */} + {logData?.content === REDACTED_ENV_VALUE + ? "Log content withheld — requires workspace runtime access." + : "No persisted log lines."} +
)} {chunks.length > 0 && (
From 5c640d9a645a3be3b075369c7d526d72781d318f Mon Sep 17 00:00:00 2001 From: Release Engineer Date: Sat, 19 Sep 2026 11:04:43 +0000 Subject: [PATCH 2/2] fix(workspace-operations): disclose operation excerpts, record withheld reads (BLO-34631 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Ally's review at ab7af3f. Important — AC 3 reverts to "disclosed, recorded as deliberate". The consumer survey was incomplete: `POST /execution-workspaces/:id/runtime-services/:action` answers with `publicWorkspaceOperation`, it is the backing call for the MCP tool `paperclipControlIssueWorkspaceServices`, and both it and `GET /heartbeat-runs/:runId/workspace-operations` are bridge-allowlisted. Agents deliberately lack `workspace_runtime:read`, so masking handed an agent ***REDACTED*** for the output of the command it just triggered. BLO-34631 AC 3 made the CTO's withhold lean falsifiable by exactly this survey, so the excerpts stay disclosed and the doc block now carries the measurement instead of a claim that was false as written. The log route's `content` stays withheld: it has no agent consumer (no MCP tool, not bridge-allowlisted; only AgentDetail.tsx and `paperclip run workspace-log`). Suggestion 1 — resolve the viewer before `audit("allowed")` and record `withheld`, so a masked read is distinguishable from a real disclosure. Suggestion 2 — one `REDACTED_VALUE_SENTINEL` in `@paperclipai/shared`, so the UI's withheld/absent branch cannot drift from the server's sentinel. Controls: mutation A (re-add the excerpt mask) fails the new disclosure guard and both PEN-3205 censor tests; mutation B (drop `withheld`) fails both audit assertions. tsc server + ui exit 0; 262 tests green across 10 suites. --- packages/shared/src/index.ts | 1 + packages/shared/src/sensitive-env.ts | 10 +++ .../__tests__/agent-live-run-routes.test.ts | 10 +++ ...space-runtime-response-withholding.test.ts | 51 ++++++++------ server/src/redaction.ts | 4 +- server/src/routes/agents.ts | 16 +++-- server/src/routes/execution-workspaces.ts | 10 +-- server/src/routes/workspace-response.ts | 67 ++++++++++++------- ui/src/pages/AgentDetail.tsx | 5 +- 9 files changed, 115 insertions(+), 59 deletions(-) diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 2c9efcb88d9c..3ce60d645b00 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -1,6 +1,7 @@ export { agentAdapterTypeSchema, optionalAgentAdapterTypeSchema } from "./adapter-type.js"; export { CREDENTIAL_VALUE_RES, + REDACTED_VALUE_SENTINEL, SENSITIVE_ENV_KEY_RE, isPlausiblySensitiveEnvValue, isSensitiveEnv, diff --git a/packages/shared/src/sensitive-env.ts b/packages/shared/src/sensitive-env.ts index 65b1fc674335..bb64f1d6a314 100644 --- a/packages/shared/src/sensitive-env.ts +++ b/packages/shared/src/sensitive-env.ts @@ -1,3 +1,13 @@ +/** + * The single spelling of a withheld value across the whole product. + * + * BLO-34631 review: the server masks rather than empties a withheld value, so a reader can tell + * "withheld" from "absent" — but that contract only holds if the UI recognises the same string the + * server writes. Both sides had their own literal, and nothing pinned them together: change one and + * the viewer silently falls back to rendering a withheld log as an empty one. + */ +export const REDACTED_VALUE_SENTINEL = "***REDACTED***"; + /** Env-var names that conventionally hold credentials. */ export const SENSITIVE_ENV_KEY_RE = /token(?:$|[-_])|api[-_]?key|access[-_]?token|auth(?:entication|_?token)?|authorization|bearer|secret|passwd|password|credential|jwt|private[-_]?key|cookie|connectionstring/i; diff --git a/server/src/__tests__/agent-live-run-routes.test.ts b/server/src/__tests__/agent-live-run-routes.test.ts index 55b82401a83d..cb437956b4d5 100644 --- a/server/src/__tests__/agent-live-run-routes.test.ts +++ b/server/src/__tests__/agent-live-run-routes.test.ts @@ -628,6 +628,13 @@ describe("agent live run routes", () => { expect(res.body.content).toBe("***REDACTED***"); // The opaque handles stay — the route they point at is the one that now withholds. expect(res.body.logRef).toBe("logs/operation-1.ndjson"); + // AC 2 + review: the access check passed, so this is `result: "allowed"` — but nothing was + // disclosed. Without `withheld` the record is indistinguishable from a real disclosure, and + // "who read this log" over-reports. + expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ + action: "workspace_operation.log_accessed", + details: expect.objectContaining({ result: "allowed", withheld: true }), + })); }); it("discloses workspace-operation log content to a reader holding workspace_runtime:read", async () => { @@ -657,6 +664,9 @@ describe("agent live run routes", () => { offset: 7, limitBytes: 64, logStore: "local_file", + // Paired with the withheld case above: the flag is what separates a real disclosure from + // a masked read, so it has to be asserted on both sides or it proves nothing. + withheld: false, }), })); expect(mockLogActivity.mock.calls[0]?.[1]?.details).not.toHaveProperty("content"); diff --git a/server/src/__tests__/workspace-runtime-response-withholding.test.ts b/server/src/__tests__/workspace-runtime-response-withholding.test.ts index 0c527dcae123..4e56f9aa9def 100644 --- a/server/src/__tests__/workspace-runtime-response-withholding.test.ts +++ b/server/src/__tests__/workspace-runtime-response-withholding.test.ts @@ -59,10 +59,12 @@ const OPERATION_METADATA_SENTINEL = "/fixture/sentinel-operation-worktree-path"; /** * BLO-34631. Command *output*, and distinct from the `command` sentinel on purpose: the whole * question this ticket settled is whether withholding the command while disclosing its output is a - * boundary or a gap, so an assertion has to name which of the two it closed. Both invented. + * boundary or a gap. It resolved to "disclosed, deliberately" on a consumer survey, so these two + * are the values an unentitled reader is expected to RECEIVE — named for the decision they pin + * rather than for egress. Both invented. */ -const OPERATION_STDOUT_SENTINEL = "sentinel-operation-stdout-must-not-egress"; -const OPERATION_STDERR_SENTINEL = "sentinel-operation-stderr-must-not-egress"; +const OPERATION_STDOUT_SENTINEL = "sentinel-operation-stdout-disclosed-by-design"; +const OPERATION_STDERR_SENTINEL = "sentinel-operation-stderr-disclosed-by-design"; /** * PEN-3073. The lifecycle command scalars that sit BESIDE `workspaceRuntime` on the same config @@ -848,14 +850,23 @@ describe("workspace runtime withholding boundary (PEN-2852)", () => { }); /** - * BLO-34631. `stdoutExcerpt` / `stderrExcerpt` used to ride the spread in - * `publicWorkspaceOperation` on the "command output is not a copy of a declared-withheld value" - * reading. The output of a withheld command discloses the command — shells echo, `set -x` - * prints everything — and the write-time scrub is a heuristic secret matcher, not a boundary. - * The consumer survey found no agent or viewer flow that needs the raw value, so they are - * withheld on the same entitlement as `command`/`cwd`. + * BLO-34631 AC 3, resolved as DISCLOSED — pinned by a test because it is a decision, not an + * omission, and the next reader of `publicWorkspaceOperation` will otherwise see `command` and + * `cwd` masked beside two unmasked siblings and "fix" the asymmetry. + * + * The CTO lean was to withhold, on the symmetry argument that the output of a withheld command + * discloses the command. AC 3 made that falsifiable by a consumer survey and the survey + * falsifies it: `POST /execution-workspaces/:id/runtime-services/:action` answers with this + * same projection, it is the backing call for the MCP tool + * `paperclipControlIssueWorkspaceServices`, and same-company agents deliberately lack + * `workspace_runtime:read` — so masking here hands an agent `***REDACTED***` for the output of + * the command it just triggered. + * + * The contrast in the last two assertions is the whole point: this reader is unentitled, and + * the SAME row still withholds `command`/`cwd`. So this case cannot pass by the projection + * being skipped, only by the excerpts being deliberately exempt from it. */ - it("withholds the operation excerpts from a reader without workspace_runtime:read", async () => { + it("discloses the operation excerpts to a reader without workspace_runtime:read", async () => { mockWorkspaceOperationService.listForExecutionWorkspace.mockResolvedValue([ workspaceOperationFixture({ stdoutExcerpt: OPERATION_STDOUT_SENTINEL, @@ -868,18 +879,18 @@ describe("workspace runtime withholding boundary (PEN-2852)", () => { ); expect(res.status).toBe(200); - expect(JSON.stringify(res.body)).not.toContain(OPERATION_STDOUT_SENTINEL); - expect(JSON.stringify(res.body)).not.toContain(OPERATION_STDERR_SENTINEL); - // Masked, not dropped — withheld-is-not-absent, same contract as `publicRuntimeServices`. - expect(res.body[0].stdoutExcerpt).toBe(REDACTED_EVENT_VALUE); - expect(res.body[0].stderrExcerpt).toBe(REDACTED_EVENT_VALUE); - // `logRef` / `logStore` stay: opaque handles, and their route withholds the content itself. - expect(res.body[0].logBytes).toBe(4096); + expect(res.body[0].stdoutExcerpt).toBe(OPERATION_STDOUT_SENTINEL); + expect(res.body[0].stderrExcerpt).toBe(OPERATION_STDERR_SENTINEL); + expect(res.body[0].command).toBe(REDACTED_EVENT_VALUE); + expect(res.body[0].cwd).toBe(REDACTED_EVENT_VALUE); }); /** - * PEN-3205, read side, now scoped to the entitled reader (BLO-34631 masks the excerpt for an - * unentitled one, so this case would pass for the wrong reason without the grant). + * PEN-3205, read side. `publicWorkspaceOperation` masks `command`/`cwd`/`metadata` and spreads + * the rest, so `stdoutExcerpt` crosses this route UNMASKED by design (BLO-34631 surveyed that + * and kept it) — 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 @@ -888,7 +899,6 @@ describe("workspace runtime withholding boundary (PEN-2852)", () => { * 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 () => { - decideAsRuntimeManager(); mockInstanceGeneralSettings.censorUsernameInLogs = true; const homeDir = os.homedir(); mockWorkspaceOperationService.listForExecutionWorkspace.mockResolvedValue([ @@ -907,7 +917,6 @@ describe("workspace runtime withholding boundary (PEN-2852)", () => { }); it("leaves the excerpt alone when the setting is off", async () => { - decideAsRuntimeManager(); mockInstanceGeneralSettings.censorUsernameInLogs = false; const homeDir = os.homedir(); mockWorkspaceOperationService.listForExecutionWorkspace.mockResolvedValue([ diff --git a/server/src/redaction.ts b/server/src/redaction.ts index b03fdb1d2679..a533fd8572c3 100644 --- a/server/src/redaction.ts +++ b/server/src/redaction.ts @@ -1,5 +1,5 @@ import { redactCommandText } from "@paperclipai/adapter-utils"; -import { envBindingSecretRefSchema, envBindingUserSecretRefSchema } from "@paperclipai/shared"; +import { envBindingSecretRefSchema, envBindingUserSecretRefSchema, REDACTED_VALUE_SENTINEL } from "@paperclipai/shared"; /** * Tier 1: key-name stems with no ambiguous benign reading (BLO-20810 / CEO @@ -191,7 +191,7 @@ const SECRET_TEXT_HINTS = [ "xox", "github_pat_", ] as const; -export const REDACTED_EVENT_VALUE = "***REDACTED***"; +export const REDACTED_EVENT_VALUE = REDACTED_VALUE_SENTINEL; function maybeContainsSecretText(input: string) { const lower = input.toLowerCase(); diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index 3e361f7e60b1..7640ee4116e1 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -382,7 +382,7 @@ export function agentRoutes( logStore: string | null; }, result: "allowed" | "denied", - opts: { offset: number; limitBytes: number }, + opts: { offset: number; limitBytes: number; withheld?: boolean }, ) { const actor = getRunLogAuditActor(req); await logActivity(db, { @@ -403,6 +403,11 @@ export function agentRoutes( offset: opts.offset, limitBytes: opts.limitBytes, logStore: entity.logStore, + // BLO-34631 review: a withheld read and a real disclosure are both `result: "allowed"` — + // the access check decides reachability, the entitlement decides the bytes. Record which + // one happened so "who read this log" is answerable without re-deriving the reader's + // grants after the fact. Absent on surfaces that apply no read-time projection. + ...(opts.withheld === undefined ? {} : { withheld: opts.withheld }), }, }); } @@ -5103,13 +5108,13 @@ export function agentRoutes( return; } - const audit = (result: "allowed" | "denied") => logLogAccessAudit(req, { + const audit = (result: "allowed" | "denied", withheld?: boolean) => logLogAccessAudit(req, { companyId: operation.companyId, entityType: "workspace_operation", entityId: operation.id, runId: operation.heartbeatRunId, logStore: operation.logStore, - }, result, { offset: normalizedOffset, limitBytes }); + }, result, { offset: normalizedOffset, limitBytes, withheld }); // Same shape as `/heartbeat-runs/:runId/log` rather than `getAccessibleResource`: keep the // cross-tenant 404 so this route is not an existence oracle, without silently dropping the @@ -5128,8 +5133,11 @@ export function agentRoutes( throw error; } - await audit("allowed"); + // Viewer first: the audit record has to say whether this read actually disclosed anything, and + // only the entitlement knows that. Resolved after the two denial paths, so a caller who never + // clears company access costs no entitlement lookup. const viewer = await resolveWorkspaceRuntimeViewer(access, req, operation.companyId); + await audit("allowed", !viewer.revealRuntimeConfig); const result = await workspaceOperations.readLog(operationId, { offset: normalizedOffset, limitBytes, diff --git a/server/src/routes/execution-workspaces.ts b/server/src/routes/execution-workspaces.ts index f28e521925e8..929b3f9052a7 100644 --- a/server/src/routes/execution-workspaces.ts +++ b/server/src/routes/execution-workspaces.ts @@ -162,11 +162,11 @@ export function executionWorkspaceRoutes(db: Db, opts: { pluginWorkerManager?: P const operations = await workspaceOperationsSvc.listForExecutionWorkspace(id); const viewer = await resolveWorkspaceRuntimeViewer(access, req, workspace.companyId); // PEN-3205: same username censoring as the sibling list route on `routes/agents.ts`. Both - // answer with the same historical `WorkspaceOperation` rows, 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. (BLO-34631 additionally withholds - // `stdoutExcerpt` / `stderrExcerpt` from an unentitled reader inside - // `publicWorkspaceOperation`; the censor below is what stands over them for an entitled one.) + // answer with the same historical `WorkspaceOperation` rows including `stdoutExcerpt` / + // `stderrExcerpt`, which `publicWorkspaceOperation` deliberately does NOT withhold (BLO-34631 + // surveyed that and found the agent's own MCP control path reads them back), 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(), diff --git a/server/src/routes/workspace-response.ts b/server/src/routes/workspace-response.ts index 5228bde510de..bc9f7dbc27a8 100644 --- a/server/src/routes/workspace-response.ts +++ b/server/src/routes/workspace-response.ts @@ -117,11 +117,11 @@ import type { accessService } from "../services/index.js"; * caller already holds, while `cleanup_command` is the operator's own string. * - `ExecutionWorkspaceStrategy.type` / `.runScope` — closed enums. `.baseRef` / `.branchTemplate` * are git refs and templates the branch-naming UI renders and the agent needs to name its branch. - * - `logRef` / `logStore` on operations — opaque handles, and the route they point at - * (`/workspace-operations/:id/log`) withholds its content on the same entitlement as of - * BLO-34631. `stdoutExcerpt` / `stderrExcerpt` used to sit on this list on the "command output, - * not a copy of a declared-withheld value" reading (CTO Ruling F §4, BLO-33407); BLO-34631 - * surveyed the consumers, found no agent or viewer flow that needs them, and withholds them. + * - `stdoutExcerpt` / `stderrExcerpt` / `logRef` / `logStore` on operations — command *output* and + * opaque handles. BLO-34631 surveyed the consumers and kept the excerpts disclosed: the agent's + * own MCP control path reads them back for the command it just triggered (see + * `publicWorkspaceOperation`). The route `logRef` points at does withhold its content, because + * that one has no agent consumer. * * ### PEN-3252 — the bypass this module recorded as open, now closed * @@ -445,25 +445,42 @@ export function publicRuntimeServices( * unentitled operator must still be able to see that an operation ran and how it ended — withholding * the operator's text is the point, hiding the fact of execution is not. * - * `stdoutExcerpt` / `stderrExcerpt` are withheld too, as of BLO-34631, and that is a measurement - * rather than a symmetry argument. They are command *output*, so they are not a copy of a - * declared-withheld value — but the output of a withheld command discloses the command: shells - * echo, `npm` prints the script it runs, `set -x` prints everything, and the only control standing - * over the bytes is the write-time `redactSensitiveText` heuristic, which matches env-dump - * assignments, JSON secret fields and URI credentials and nothing else. Host paths, repo layout - * and an operator's `cleanupCommand` cross it intact. - * - * The consumer survey that decides it (BLO-34631 AC 3): every reader of these two fields and of - * `/workspace-operations/:id/log` is a human UI or CLI surface — `ui/src/pages/AgentDetail.tsx`, - * `ui/src/pages/ExecutionWorkspaceDetail.tsx`, and `paperclip run workspace-log`. There is no agent - * consumer, no MCP tool and no server-internal read. `workspace_runtime:read` is granted by - * `allow_simple_company_member` to every non-viewer board member, so all three surfaces keep the - * raw value for the humans that use them; what loses it is the actor class PEN-2852 built the - * entitlement to exclude — same-company agents, viewers, low-trust principals and bridge keys. - * - * `logRef` / `logStore` stay: they are opaque handles, and the route they point at now withholds - * the content on this same entitlement. Masking a pointer while its route still served the bytes - * would have been theatre. + * `stdoutExcerpt` / `stderrExcerpt` are NOT withheld, and as of BLO-34631 that is a measured + * decision rather than the inherited "command output is not a copy of a declared-withheld value" + * reading (CTO Ruling F §4, BLO-33407). + * + * The symmetry argument for withholding them is real and was the CTO's stated lean: the output of a + * withheld command can disclose the command — shells echo, `npm` prints the script it runs, `set -x` + * prints everything — and the only control standing over the bytes is the write-time + * `redactSensitiveText` heuristic, which matches env-dump assignments, JSON secret fields and URI + * credentials and nothing else. Host paths, repo layout and an operator's `cleanupCommand` cross it + * intact. BLO-34631 AC 3 made that lean explicitly falsifiable by a consumer survey, and the survey + * falsifies it: + * + * - `POST /execution-workspaces/:id/runtime-services/:action` (`routes/execution-workspaces.ts`) and + * `POST /projects/:id/workspaces/:workspaceId/runtime-services/:action` (`routes/projects.ts`) + * answer with `operation: publicWorkspaceOperation(operation, viewer)`, where the operation is the + * one the caller just triggered and `stdout`/`stderr` are captured synchronously from it. + * - The first of those is the backing call for the MCP tool `paperclipControlIssueWorkspaceServices` + * (`packages/mcp-server/src/tools.ts`), which returns the response JSON verbatim to the calling + * agent, and both it and `GET /heartbeat-runs/:runId/workspace-operations` are on the sandbox + * callback bridge allowlist (`packages/adapter-utils/src/sandbox-callback-bridge.ts`). + * - Same-company agents deliberately lack `workspace_runtime:read` (PEN-2852), so masking here + * hands an agent `***REDACTED***` for the output of the command it just ran. `status`/`exitCode` + * survive, so it would still learn pass/fail — but not why, which is exactly the "debugging a + * failed provision" flow BLO-34631 named as the finding that settles this. + * + * So the disclosure is deliberate and recorded, not an unexamined pass-through. The residual it + * accepts: an operator's service command echoed into its own output still reaches an unentitled + * reader, and the write-time scrub is not a boundary. Narrowing it is a product decision that has to + * keep the agent's own command-result path readable — masking the read/list routes alone would split + * the same field across routes, which is the failure mode this series exists to close. + * + * `logRef` / `logStore` stay for a different reason: they are opaque handles, and the route they + * point at (`/workspace-operations/:id/log`) DOES withhold its content on this entitlement as of + * BLO-34631 — that route has no agent consumer (no MCP tool, not bridge-allowlisted; only + * `ui/src/pages/AgentDetail.tsx` and `paperclip run workspace-log` read it). Masking a pointer whose + * route still served the bytes would have been theatre. */ export function publicWorkspaceOperation( operation: WorkspaceOperation, @@ -474,8 +491,6 @@ export function publicWorkspaceOperation( ...operation, command: maskWorkspaceRuntimeTextForRead(operation.command), cwd: maskWorkspaceRuntimeTextForRead(operation.cwd), - stdoutExcerpt: maskWorkspaceRuntimeTextForRead(operation.stdoutExcerpt), - stderrExcerpt: maskWorkspaceRuntimeTextForRead(operation.stderrExcerpt), metadata: maskWorkspaceRuntimeForRead(operation.metadata) as Record | null, }; } diff --git a/ui/src/pages/AgentDetail.tsx b/ui/src/pages/AgentDetail.tsx index 047a0d730678..0c42f1084b3b 100644 --- a/ui/src/pages/AgentDetail.tsx +++ b/ui/src/pages/AgentDetail.tsx @@ -106,6 +106,7 @@ import { type WorkspaceOperation, isResponsibleUserDenialCode, isSensitiveEnv, + REDACTED_VALUE_SENTINEL, responsibleUserLabel, } from "@paperclipai/shared"; import { ResponsibleUserDenialNotice } from "../components/ResponsibleUserDenialNotice"; @@ -133,7 +134,9 @@ const runStatusIcons: Record