From 6afc0896baaef21c44e9c350a7d6d438a3463e63 Mon Sep 17 00:00:00 2001 From: Ruslan Strazhnyk Date: Sat, 26 Sep 2026 23:53:22 +0200 Subject: [PATCH] fix(agent): harden narrationDirective against secret leakage (Copilot #210) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot flagged HIGH-severity credential-exposure risk on PR #210: the `full` narration directive asked the subprocess agent to echo commands and output verbatim, which can carry API keys (argv `--token=`), bearer tokens (`Authorization:` headers), env-var values from a `printenv` call, connection strings, private keys, etc. Every mode (off/brief/full — including the "bogus"→brief fallback) now carries a shared REDACTION clause that: - names the secret classes to substitute (API keys, bearer/OAuth/session tokens, passwords, private keys, Authorization/Cookie header values, connection strings, --token=/--api-key=/--password=/--secret= flag values, AWS_*/GITHUB_TOKEN/ANTHROPIC_API_KEY/*_KEY/*_TOKEN/*_SECRET env-var values, webhook signing secrets), - specifies the substitution (``, not omission), - forbids quoting `env`/`printenv`/`railway variables` output or `.env*` file contents in the narration channel, - covers `off` mode too because "only narrate on failure" is exactly where secrets tend to appear (401 responses, curl -v dumps). Subprocess agents (Codex CLI, Antigravity, opencode-selected models) do not inherit qmax-code's own secret-handling rules from CLAUDE.md, so the guard is spelled out in the injected system prompt. TestNarrationDirectiveCarriesRedactionClause asserts each mode names the key secret classes by string — a future edit that thins the directive without replacing the guard fails loudly. --- internal/agent/cc_agent.go | 17 ++++++++++-- internal/agent/cc_agent_session_test.go | 37 +++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/internal/agent/cc_agent.go b/internal/agent/cc_agent.go index b3b360e..17c31ca 100644 --- a/internal/agent/cc_agent.go +++ b/internal/agent/cc_agent.go @@ -749,14 +749,25 @@ func outputStyleDirective(verbose bool) string { // snippet; "full" additionally asks for a quoted key output line after each // non-trivial tool call. Applied on top of OUTPUT MODE so a compact final // answer can still coexist with informative in-flight narration. +// +// Every mode carries the same REDACTION clause: whatever the model chooses +// to echo (command text, snippet, output line, error) must strip secrets +// before printing. Subprocess agents (Codex, Antigravity, opencode) do not +// inherit qmax-code's own CLAUDE.md secret-handling rules, so the guard is +// spelled out here to prevent narrated transcripts from leaking credentials. func narrationDirective(mode string) string { + // redaction is repeated in every mode because "off" still narrates on + // failure ("only narrate when a tool call fails"), and the failure output + // is exactly where secrets tend to appear (401 responses, curl -v dumps, + // env-var listings from a broken test). + const redaction = " REDACTION (applies to any text you narrate — command, snippet, output, error): before printing, replace secrets with ``. Treat as secret: API keys, bearer/OAuth/session tokens, passwords, private keys, `Authorization:` and `Cookie:` header values, connection strings (postgres://user:pass@…, redis://…), `--token=`/`--api-key=`/`--password=`/`--secret=` flag values, `AWS_*` / `GITHUB_TOKEN` / `ANTHROPIC_API_KEY` / other `*_KEY` / `*_TOKEN` / `*_SECRET` env-var values, webhook signing secrets, and anything the surrounding text calls a key/token/secret. When in doubt, redact. Never quote raw `env` / `printenv` / `railway variables` output or `.env*` file contents in narration." switch mode { case "off": - return "\n\nTOOL NARRATION: OFF — Chain tool calls without prose between them when the plan is obvious. Only narrate when a tool call fails or when direction changes." + return "\n\nTOOL NARRATION: OFF — Chain tool calls without prose between them when the plan is obvious. Only narrate when a tool call fails or when direction changes." + redaction case "full": - return "\n\nTOOL NARRATION: FULL — Before each non-trivial tool call, output one line quoting a redacted form of the command (```bash …```) or snippet about to be written; after the tool returns, quote a redacted key output line that mattered (test count, PR URL, error). Never include credentials, tokens, cookies, or other secrets in narration. Never chain 3+ silent tool calls." + return "\n\nTOOL NARRATION: FULL — Before each non-trivial tool call, output one line quoting the command (```bash …```) or snippet about to be written; after the tool returns, quote the key output line that mattered (test count, PR URL, error). Never chain 3+ silent tool calls." + redaction default: // "brief" and any unrecognized value - return "\n\nTOOL NARRATION: BRIEF — Before each non-trivial tool call, output one short line quoting the exact command or the snippet about to be written. Skip the preface for trivial reads. Never chain 3+ silent tool calls." + return "\n\nTOOL NARRATION: BRIEF — Before each non-trivial tool call, output one short line quoting the command or the snippet about to be written. Skip the preface for trivial reads. Never chain 3+ silent tool calls." + redaction } } diff --git a/internal/agent/cc_agent_session_test.go b/internal/agent/cc_agent_session_test.go index 6a88534..f5c2281 100644 --- a/internal/agent/cc_agent_session_test.go +++ b/internal/agent/cc_agent_session_test.go @@ -225,6 +225,43 @@ func TestNarrationDirectiveIncludesModeLabel(t *testing.T) { } } +// TestNarrationDirectiveCarriesRedactionClause guards the credential-leak +// mitigation Copilot flagged on #210: every mode (off/brief/full) must tell +// the subprocess agent to redact secrets before echoing commands, snippets, +// output lines, or errors into the transcript. If someone loosens the +// directive without replacing the guard, this test breaks loudly. +func TestNarrationDirectiveCarriesRedactionClause(t *testing.T) { + // Named secret classes the directive must call out by name. Missing any + // of these means the mitigation regressed and the release is unsafe. + mustName := []string{ + "REDACTION", + "", + "API keys", + "bearer", + "token", + "password", + "private key", + "Authorization:", + "Cookie:", + "connection string", + "--token=", + "--api-key=", + "env-var", + "webhook signing", + "env", // "raw `env` / `printenv` ..." + "printenv", + ".env", + } + for _, mode := range []string{"off", "brief", "full", "bogus"} { + got := narrationDirective(mode) + for _, want := range mustName { + if !strings.Contains(got, want) { + t.Errorf("narrationDirective(%q) missing redaction clause item %q", mode, want) + } + } + } +} + func TestCCAgentPromptIncludesNarrationDirective(t *testing.T) { a := NewCCAgent("bin", "", "high", "standard", false, "full", &api.SessionContext{}) if a.narrateToolCalls != "full" {