From 7dadeab331e9b432be529e709061007e5649fba0 Mon Sep 17 00:00:00 2001 From: roboomp Date: Mon, 22 Jun 2026 07:05:23 +0000 Subject: [PATCH] fix(coding-agent): hid secrets from advisor prompts Stopped restoring placeholders inside opaque assistant thinking blocks and threaded the configured secret obfuscator into advisor session-update prompts. Added regression coverage for thinking preservation and advisor prompt redaction. Fixes #3237 --- packages/coding-agent/CHANGELOG.md | 4 ++++ .../src/advisor/__tests__/advisor.test.ts | 23 +++++++++++++++++++ packages/coding-agent/src/advisor/runtime.ts | 7 +++++- .../coding-agent/src/secrets/obfuscator.ts | 12 +++------- .../coding-agent/src/session/agent-session.ts | 1 + .../test/secrets-obfuscator.test.ts | 11 ++++++--- 6 files changed, 45 insertions(+), 13 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 2e70c2bd9..d8436dac1 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed hide-secrets handling so advisor session updates are redacted before the advisor model sees them and opaque assistant thinking blocks are no longer deobfuscated. + ## [16.1.14] - 2026-06-22 ### Added diff --git a/packages/coding-agent/src/advisor/__tests__/advisor.test.ts b/packages/coding-agent/src/advisor/__tests__/advisor.test.ts index 303c7640f..5281de43b 100644 --- a/packages/coding-agent/src/advisor/__tests__/advisor.test.ts +++ b/packages/coding-agent/src/advisor/__tests__/advisor.test.ts @@ -5,6 +5,7 @@ import { createAdvisorMessageCard } from "../../modes/components/advisor-message import { getThemeByName } from "../../modes/theme/theme"; import { formatSessionHistoryMarkdown } from "../../session/session-history-format"; import { YieldQueue } from "../../session/yield-queue"; +import { SecretObfuscator } from "../../secrets/obfuscator"; import { ADVISOR_READONLY_TOOL_NAMES, AdviseTool, @@ -447,6 +448,28 @@ describe("advisor", () => { expect(promptInputs[0]).not.toContain("note"); }); + it("obfuscates session updates before prompting the advisor", async () => { + const secret = "ADVISOR_SECRET_TOKEN_123"; + const obfuscator = new SecretObfuscator([{ type: "plain", content: secret }]); + const placeholder = obfuscator.obfuscate(secret); + const promptInputs: string[] = []; + const agent = makeAgent(promptInputs); + const messages: AgentMessage[] = [{ role: "user", content: `token ${secret}`, timestamp: 1 } as AgentMessage]; + const host: AdvisorRuntimeHost = { + snapshotMessages: () => messages, + enqueueAdvice: () => {}, + obfuscator, + }; + const runtime = new AdvisorRuntime(agent, host); + + runtime.onTurnEnd(); + await Promise.resolve(); + + expect(promptInputs).toHaveLength(1); + expect(promptInputs[0]).toContain(placeholder); + expect(promptInputs[0]).not.toContain(secret); + }); + it("expands plan-mode context once, then collapses an unchanged re-injection", async () => { const promptInputs: string[] = []; const agent = makeAgent(promptInputs); diff --git a/packages/coding-agent/src/advisor/runtime.ts b/packages/coding-agent/src/advisor/runtime.ts index a3921ea74..ae8a5d446 100644 --- a/packages/coding-agent/src/advisor/runtime.ts +++ b/packages/coding-agent/src/advisor/runtime.ts @@ -2,6 +2,7 @@ import type { AgentMessage } from "@oh-my-pi/pi-agent-core"; import { estimateTokens } from "@oh-my-pi/pi-agent-core/compaction"; import { logger } from "@oh-my-pi/pi-utils"; import { formatSessionHistoryMarkdown, PRIMARY_CONTEXT_CUSTOM_TYPES } from "../session/session-history-format"; +import type { SecretObfuscator } from "../secrets/obfuscator"; /** Minimal slice of `Agent` the runtime drives — satisfied by pi-agent-core `Agent`. */ export interface AdvisorAgent { @@ -16,6 +17,8 @@ export interface AdvisorRuntimeHost { snapshotMessages(): AgentMessage[]; /** Surface one advice note to the primary (enqueues into the session YieldQueue). */ enqueueAdvice(note: string, severity?: "nit" | "concern" | "blocker"): void; + /** Redact primary transcript bytes before they reach the advisor model. */ + obfuscator?: SecretObfuscator; /** * Pre-prompt context maintenance for the advisor's own append-only context. * Promotes the advisor model to a larger sibling when its context nears the @@ -181,7 +184,9 @@ export class AdvisorRuntime { expandPrimaryContext: true, }); if (!md.trim()) return null; - return `### Session update\n\n${md}`; + const text = `### Session update\n\n${md}`; + const obfuscator = this.host.obfuscator; + return obfuscator?.hasSecrets() ? obfuscator.obfuscate(text) : text; } /** diff --git a/packages/coding-agent/src/secrets/obfuscator.ts b/packages/coding-agent/src/secrets/obfuscator.ts index d271b40bb..ef7d53741 100644 --- a/packages/coding-agent/src/secrets/obfuscator.ts +++ b/packages/coding-agent/src/secrets/obfuscator.ts @@ -259,9 +259,9 @@ export function deobfuscateAgentMessages(obfuscator: SecretObfuscator, messages: } /** - * Restore placeholders in assistant content: visible text, thinking text, and - * tool-call arguments/intent/rawBlock. Signatures and redacted-thinking bytes - * are opaque provider-replay data and pass through byte-identical. + * Restore placeholders in assistant content: visible text and tool-call + * arguments/intent/rawBlock. Thinking and signatures are opaque + * provider-replay/hidden-reasoning data and pass through byte-identical. */ export function deobfuscateAssistantContent( obfuscator: SecretObfuscator, @@ -276,12 +276,6 @@ export function deobfuscateAssistantContent( changed = true; return { ...block, text }; } - if (block.type === "thinking") { - const thinking = obfuscator.deobfuscate(block.thinking); - if (thinking === block.thinking) return block; - changed = true; - return { ...block, thinking }; - } if (block.type === "toolCall") { const args = deobfuscateToolArguments(obfuscator, block.arguments); const intent = block.intent === undefined ? undefined : obfuscator.deobfuscate(block.intent); diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index b25b30507..b06922695 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -1936,6 +1936,7 @@ export class AgentSession { snapshotMessages: () => this.agent.state.messages, enqueueAdvice, maintainContext: incomingTokens => this.#maintainAdvisorContext(incomingTokens), + obfuscator: this.#obfuscator, }); if (seedToCurrent) { this.#advisorRuntime.seedTo(this.agent.state.messages.length); diff --git a/packages/coding-agent/test/secrets-obfuscator.test.ts b/packages/coding-agent/test/secrets-obfuscator.test.ts index 5b84f6548..cf245e37d 100644 --- a/packages/coding-agent/test/secrets-obfuscator.test.ts +++ b/packages/coding-agent/test/secrets-obfuscator.test.ts @@ -252,7 +252,7 @@ describe("SecretObfuscator cross-turn cache stability", () => { }); describe("deobfuscateAgentMessages (display restore)", () => { - it("restores assistant content and model-generated summaries, leaving raw user text untouched", () => { + it("restores assistant text and tool calls while leaving raw user text and thinking untouched", () => { const secret = "DISPLAY_SECRET_TOKEN_123"; const obfuscator = new SecretObfuscator([{ type: "plain", content: secret }]); const placeholder = obfuscator.obfuscate(secret); @@ -302,11 +302,16 @@ describe("deobfuscateAgentMessages (display restore)", () => { const restored = deobfuscateAgentMessages(obfuscator, [userMsg, assistantMsg, branchSummary, compactionSummary]); - // Assistant text, thinking, and tool-call args/intent are restored to the real secret. + // Assistant text and tool-call args/intent are restored to the real secret. const restoredAssistant = restored[1] as AssistantMessage; const assistantJson = JSON.stringify(restoredAssistant.content); expect(assistantJson).toContain(secret); - expect(assistantJson).not.toContain(placeholder); + expect(assistantJson).not.toContain(`answer ${placeholder}`); + expect(assistantJson).not.toContain(`path ${placeholder}`); + expect(assistantJson).not.toContain(`intent ${placeholder}`); + // Opaque thinking is never walked: placeholder-shaped bytes survive unchanged. + expect(assistantJson).toContain(`reason ${placeholder}`); + expect(assistantJson).not.toContain(`reason ${secret}`); // Model-generated summaries are restored. expect((restored[2] as { summary: string }).summary).toBe(`branch ${secret}`); expect((restored[3] as { summary: string; shortSummary?: string }).summary).toBe(`compact ${secret}`);