Redact secret_request values from transcripts, pushes and memory (#1531)

electron/memory-record.js

@@ -2,6 +2,7 @@
 
 
 const { createMemoryProxy } = require("./memory-proxy.js");
+const threadSecrets = require("./threadSecrets.js");
 
 const TITLE_MAX = 80;
 const BODY_TEXT_MAX = 1200;
@@ -121,7 +122,8 @@ async function recordRunOutcome(args, deps = {}) {
       });
     }
 
-    await proxy.store(payload);
+    // The body quotes the run's final text, which may echo a secret (#1531).
+    await proxy.store(threadSecrets.redact(String(thread.id || ""), payload));
   } catch {
     // Silent no-op: memory down, network error, bad payload — never affect runs.
   }

electron/runner.js

@@ -15,6 +15,7 @@ const { createAskRuns } = require("./runner-ask-runs.js");
 const { createTurnSetup } = require("./runner-turn-setup.js");
 const { createSubagents } = require("./runner-subagents.js");
 const { createSessionRecording } = require("./runner-session-recording.js");
+const threadSecrets = require("./threadSecrets.js");
 const { createClaudeSessions } = require("./runner-claude-sessions.js");
 const { createVerifyGate } = require("./runner-verify-gate.js");
 const { createQuotaWait } = require("./runner-quota-wait.js");
@@ -1285,7 +1286,7 @@ function createRunner(opts) {
       }
     }
     // Session mirror: user + event immediately; assistant/tool at terminal.
-    recordSessionOnAppend(threadId, role, text);
+    recordSessionOnAppend(threadId, role, threadSecrets.redact(threadId, text));
     return msg.id;
   }
 

electron/store.js

@@ -4,6 +4,7 @@ const fs = require("node:fs");
 const path = require("node:path");
 const { randomUUID } = require("node:crypto");
 const { getDefaultSecrets } = require("./secrets.js");
+const threadSecrets = require("./threadSecrets.js");
 const {
   splitMessagesByThread,
   stringifyStore,
@@ -2073,7 +2074,10 @@ class Store {
    */
   appendMessage(threadId, message) {
     const list = this.getMessages(threadId).slice();
-    list.push(message);
+    // Every runner/adapter message write lands here or in updateMessage, and
+    // the renderer push, summaries and session transcript read back from the
+    // store, so redacting here covers them all (#1531 secret_request).
+    list.push(threadSecrets.redact(threadId, message));
     this.setMessages(threadId, list);
     this.updateThread(threadId, {}, { touch: true });
   }
@@ -2311,7 +2315,9 @@ class Store {
     const list = this.getMessages(threadId).slice();
     const idx = list.findIndex((m) => m.id === messageId);
     if (idx < 0) return null;
-    list[idx] = { ...list[idx], ...patch };
+    // Streams patch the whole accumulated text, so a value split across
+    // chunks is caught once both halves have arrived.
+    list[idx] = { ...list[idx], ...threadSecrets.redact(threadId, patch) };
     this.setMessages(threadId, list);
     return list[idx];
   }

electron/test/secret-request.test.js

@@ -9,6 +9,7 @@
 const { describe, it, afterEach } = require("node:test");
 const assert = require("node:assert/strict");
 const fs = require("node:fs");
+const http = require("node:http");
 const os = require("node:os");
 const path = require("node:path");
 const { pathToFileURL } = require("node:url");
@@ -132,6 +133,117 @@ describe("secret_request (#1531)", () => {
     assert.equal(threadSecrets.withEnv(thread.id, undefined), undefined);
   });
 
+  it("redacts a printed secret from messages, pushes, the store and the transcript", async () => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "coder-secret-"));
+    const userData = path.join(tmpDir, "userData");
+    fs.mkdirSync(userData);
+    const VALUE = "sk_live_51HxTestSecretValue";
+    // The agent does what the issue warns about: `echo $STRIPE_API_KEY`.
+    const agent = path.join(tmpDir, "agent.js");
+    fs.writeFileSync(
+      agent,
+      "process.stdout.write('key is '+process.env.STRIPE_API_KEY+' ok');",
+    );
+    delete process.env.CODER_SIMULATE;
+    process.env.CODER_AGENT_CMD = `${process.execPath} ${agent}`;
+
+    // Fake memory server: captures what the session transcript queue sends.
+    const transcript = [];
+    const server = http.createServer((req, res) => {
+      let body = "";
+      req.on("data", (c) => (body += c));
+      req.on("end", () => {
+        transcript.push(body);
+        res.setHeader("content-type", "application/json");
+        res.end("{}");
+      });
+    });
+    await new Promise((r) => server.listen(0, "127.0.0.1", r));
+    const { port } = /** @type {import("node:net").AddressInfo} */ (server.address());
+    fs.writeFileSync(
+      path.join(userData, "memory-server.json"),
+      JSON.stringify({ port, token: "t" }),
+    );
+
+    let threadId = "";
+    try {
+      const store = new Store(path.join(userData, "coder-store.json"));
+      const core = await import(
+        pathToFileURL(path.join(__dirname, "../../core/dist/index.js")).href
+      );
+      const pushes = [];
+      runner = createRunner({
+        store,
+        core,
+        pushFn: (_ch, payload) => pushes.push(JSON.stringify(payload)),
+        tickMs: 15,
+        userDataPath: userData,
+        getMemoryStatus: () => ({ running: true, adopted: false, port }),
+      });
+      const repo = path.join(tmpDir, "app");
+      fs.mkdirSync(repo);
+      execFileSync("git", ["init"], { cwd: repo, stdio: "ignore" });
+      const project = await services.addProject(store, repo);
+      const thread = services.createThread(store, { projectId: project.id, title: "Redact" });
+      threadId = thread.id;
+
+      runner.requestSecret({ threadId: thread.id, name: "STRIPE_API_KEY" });
+      const card = store.getThread(thread.id).pendingSecret;
+      runner.answerSecret({ threadId: thread.id, requestId: card.id, value: VALUE });
+      await waitFor(() => store.getThread(thread.id).status === "done");
+
+      // A tool call whose result carries the value, written the way the
+      // adapters do: append, then patch the result in.
+      const toolId = "tool-msg-1";
+      store.appendMessage(thread.id, {
+        id: toolId,
+        role: "tool",
+        text: "",
+        createdAt: Date.now(),
+        tool: { id: "t1", name: "Bash", input: { command: `echo ${VALUE}` }, done: false },
+      });
+      store.updateMessage(thread.id, toolId, {
+        tool: { id: "t1", name: "Bash", input: { command: `echo ${VALUE}` }, output: `${VALUE}\n`, done: true },
+      });
+      runner.refreshDetail(thread.id);
+
+      await runner.flushTranscripts();
+      store.saveNow();
+
+      const msgs = JSON.stringify(store.getMessages(thread.id));
+      assert.ok(msgs.includes("key is [secret:STRIPE_API_KEY] ok"), "assistant text redacted");
+      assert.ok(msgs.includes('"output":"[secret:STRIPE_API_KEY]\\n"'), "tool output redacted");
+      assert.ok(!msgs.includes(VALUE), "messages");
+
+      const pushed = pushes.join("\n");
+      assert.ok(pushed.includes("[secret:STRIPE_API_KEY]"), "pushed detail redacted");
+      assert.ok(!pushed.includes(VALUE), "pushed detail");
+
+      const sent = transcript.join("\n");
+      assert.ok(sent.includes("[secret:STRIPE_API_KEY]"), "transcript redacted");
+      assert.ok(!sent.includes(VALUE), "transcript");
+
+      assert.deepEqual(filesContaining(userData, VALUE), [], "files under userData");
+      const storeFiles = filesContaining(userData, "[secret:STRIPE_API_KEY]");
+      assert.ok(storeFiles.length > 0, "store file holds the placeholder");
+    } finally {
+      threadSecrets.clear(threadId);
+      await new Promise((r) => server.close(r));
+    }
+  });
+
+  it("redact skips values shorter than 6 chars and leaves other threads alone", () => {
+    threadSecrets.set("redact-a", "SHORT", "abc12");
+    threadSecrets.set("redact-a", "LONG", "abcdef");
+    const msg = { text: "abc12 abcdef", tool: { output: ["xabcdefx"] } };
+    assert.deepEqual(threadSecrets.redact("redact-a", msg), {
+      text: "abc12 [secret:LONG]",
+      tool: { output: ["x[secret:LONG]x"] },
+    });
+    assert.equal(threadSecrets.redact("redact-b", msg), msg);
+    threadSecrets.clear("redact-a");
+  });
+
   it("declining drops the card without starting a turn", async () => {
     tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "coder-secret-"));
     const store = new Store(path.join(tmpDir, "coder-store.json"));

electron/threadSecrets.js

@@ -39,4 +39,38 @@ function withEnv(threadId, base) {
   return s ? /** @type {T} */ ({ ...(base || {}), ...s }) : base;
 }
 
-module.exports = { NAME_RE, set, clear, withEnv };
+/** Shorter values would redact ordinary words ("true", "admin"). */
+const MIN_REDACT_LEN = 6;
+
+/**
+ * Replace this thread's secret values with `[secret:NAME]` in every string of
+ * a JSON-shaped value (message, patch, text). Returns `value` itself when the
+ * thread has no redactable secrets, so the common path allocates nothing.
+ * @template T
+ * @param {string} threadId
+ * @param {T} value
+ * @returns {T}
+ */
+function redact(threadId, value) {
+  const s = byThread.get(threadId);
+  if (!s) return value;
+  // Longest first, so a secret containing another redacts whole.
+  const pairs = Object.entries(s)
+    .filter(([, v]) => typeof v === "string" && v.length >= MIN_REDACT_LEN)
+    .sort((a, b) => b[1].length - a[1].length);
+  if (!pairs.length) return value;
+  /** @param {unknown} v @returns {unknown} */
+  const walk = (v) => {
+    if (typeof v === "string") {
+      return pairs.reduce((t, [name, secret]) => t.replaceAll(secret, `[secret:${name}]`), v);
+    }
+    if (Array.isArray(v)) return v.map(walk);
+    if (v && typeof v === "object") {
+      return Object.fromEntries(Object.entries(v).map(([k, x]) => [k, walk(x)]));
+    }
+    return v;
+  };
+  return /** @type {T} */ (walk(value));
+}
+
+module.exports = { NAME_RE, set, clear, withEnv, redact };