fix(workspace-operations): project and audit the operation log route (BLO-34631)

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,

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;

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<string, unknown> = {}) {
+  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,167 @@ 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");
+    // 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 () => {
+    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",
+        // 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");
+  });
+
+  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<typeof import("node:os")>("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<typeof import("node:os")>("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}`,

server/src/__tests__/workspace-runtime-response-withholding.test.ts

@@ -56,6 +56,16 @@ 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. 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-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
  * object, and their siblings on the three nouns that carry the same strings elsewhere. Each gets its
@@ -839,11 +849,48 @@ describe("workspace runtime withholding boundary (PEN-2852)", () => {
       expect(res.body[0].cwd).toBe(OPERATION_CWD_SENTINEL);
     });
 
+    /**
+     * 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("discloses the operation excerpts to 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(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. `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 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

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();