From 7b36f34e32b0c778a29b870f032d4cc12a70dda6 Mon Sep 17 00:00:00 2001 From: itelo Date: Mon, 24 Aug 2026 16:30:00 -0300 Subject: [PATCH] feat(harness): extend .env guard to pi + redact broad-grep leaks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to ENG-2925 (#57), completing ENG-2926. - pi harness: wrap the built-in Read/Edit/Write/Ls/Find/Grep tools so a call whose input targets a secret file (.env, …) is refused before it runs — parity with the anthropic PreToolUse deny. Reuses toolInputTouchesSecret; pi's path fields (path/glob/dir/directory) are now covered too. - anthropic harness: add a PostToolUse hook that strips .env-sourced lines from Grep output via updatedToolOutput. The PreToolUse deny only fires when a tool *names* a secret path, so a repo-wide grep (path ".") could still surface .env lines; this closes that residual. - secret-paths.ts: add redactSecretGrepLines + the pi path-field keys, with tests. --- src/lib/steps/harness/anthropic.ts | 28 ++++++++++++- src/lib/steps/harness/pi.ts | 46 +++++++++++++++++++++- src/lib/steps/harness/secret-paths.test.ts | 43 ++++++++++++++++++-- src/lib/steps/harness/secret-paths.ts | 24 +++++++++-- 4 files changed, 130 insertions(+), 11 deletions(-) diff --git a/src/lib/steps/harness/anthropic.ts b/src/lib/steps/harness/anthropic.ts index bbed7ba..02a942b 100644 --- a/src/lib/steps/harness/anthropic.ts +++ b/src/lib/steps/harness/anthropic.ts @@ -1,6 +1,9 @@ import { type HookCallback, query } from '@anthropic-ai/claude-agent-sdk' -import { toolInputTouchesSecret } from './secret-paths.js' +import { + redactSecretGrepLines, + toolInputTouchesSecret, +} from './secret-paths.js' import type { Harness, HarnessRunStepArgs, StepRunResult } from './types.js' // The official Seam MCP (same server the seam-plugin wires up). @@ -40,6 +43,25 @@ const denySecretFileAccess: HookCallback = async (input) => { return {} } +// The PreToolUse deny only fires when a tool *names* a secret path; a repo-wide +// grep (path ".") still scans `.env` and returns its lines. Strip those from the +// grep output before it reaches the model. Only the string form of the result is +// handled — a shape the redactor can't parse is left untouched. +const redactSecretsFromGrepOutput: HookCallback = async (input) => { + if (input.hook_event_name !== 'PostToolUse') return {} + if (input.tool_name !== 'Grep' || typeof input.tool_response !== 'string') { + return {} + } + const redacted = redactSecretGrepLines(input.tool_response) + if (redacted === input.tool_response) return {} + return { + hookSpecificOutput: { + hookEventName: 'PostToolUse', + updatedToolOutput: redacted, + }, + } +} + // The control harness: drives the integration with the Claude Agent SDK. export const anthropicHarness: Harness = { name: 'anthropic', @@ -76,9 +98,11 @@ export const anthropicHarness: Harness = { // reviews the result as a git diff afterward. Read/search tools and the // docs MCP are read-only, so nothing destructive runs unattended. permissionMode: 'acceptEdits', - // Block reads/writes of the developer's .env / secret files. + // Block reads/writes of the developer's .env / secret files, and strip + // any secret lines a broad grep still surfaces from them. hooks: { PreToolUse: [{ hooks: [denySecretFileAccess] }], + PostToolUse: [{ hooks: [redactSecretsFromGrepOutput] }], }, mcpServers: { // Wired exactly like the seam-plugin: mcp-remote bridges to the hosted diff --git a/src/lib/steps/harness/pi.ts b/src/lib/steps/harness/pi.ts index c8e1da7..540669b 100644 --- a/src/lib/steps/harness/pi.ts +++ b/src/lib/steps/harness/pi.ts @@ -1,6 +1,7 @@ import type { ToolDefinition } from '@earendil-works/pi-coding-agent' import { createJiti } from 'jiti' +import { toolInputTouchesSecret } from './secret-paths.js' import type { Harness, HarnessRunStepArgs, StepRunResult } from './types.js' // The official Seam MCP — same server the anthropic harness and the seam-plugin @@ -127,14 +128,14 @@ export const piHarness: Harness = { // Each factory returns a differently-parameterised ToolDefinition; widen to // the general type createAgentSession accepts (the per-tool schema variance // is irrelevant to the session, which treats them uniformly). - const customTools = [ + const customTools = guardSecretFileTools([ createReadToolDefinition(cwd), createEditToolDefinition(cwd), createWriteToolDefinition(cwd), createLsToolDefinition(cwd), createFindToolDefinition(cwd), createGrepToolDefinition(cwd), - ] as unknown as ToolDefinition[] + ] as unknown as ToolDefinition[]) const { session } = await createAgentSession({ model, @@ -201,6 +202,47 @@ export const piHarness: Harness = { }, } +// pi's built-in file tools ship no secret-file guard, so wrap each one: a call +// whose input targets a secret file (.env, …) is refused before it runs — parity +// with the anthropic harness's PreToolUse deny. Only `execute` is overridden; +// every other field (schema, renderers) is preserved by the spread. +type PiToolExecute = ( + toolCallId: string, + params: unknown, + signal: AbortSignal | undefined, + onUpdate: unknown, + ctx: unknown, +) => Promise + +function guardSecretFileTools(tools: ToolDefinition[]): ToolDefinition[] { + return tools.map((tool) => { + const runOriginal = ( + tool as unknown as { execute: PiToolExecute } + ).execute.bind(tool) + const execute: PiToolExecute = async ( + toolCallId, + params, + signal, + onUpdate, + ctx, + ) => { + if (toolInputTouchesSecret(params)) { + return { + content: [ + { + type: 'text', + text: 'Reading .env / secret files is blocked. Load SEAM_API_KEY from the runtime environment instead.', + }, + ], + details: {}, + } + } + return runOriginal(toolCallId, params, signal, onUpdate, ctx) + } + return { ...tool, execute } as unknown as ToolDefinition + }) +} + function readRole(message: unknown): string | undefined { const role = (message as { role?: unknown }).role return typeof role === 'string' ? role : undefined diff --git a/src/lib/steps/harness/secret-paths.test.ts b/src/lib/steps/harness/secret-paths.test.ts index 6af12bd..5e5340f 100644 --- a/src/lib/steps/harness/secret-paths.test.ts +++ b/src/lib/steps/harness/secret-paths.test.ts @@ -1,6 +1,10 @@ import { expect, test } from 'vitest' -import { isSecretFilePath, toolInputTouchesSecret } from './secret-paths.js' +import { + isSecretFilePath, + redactSecretGrepLines, + toolInputTouchesSecret, +} from './secret-paths.js' test('isSecretFilePath: blocks .env and its variants', () => { expect(isSecretFilePath('.env')).toBe(true) @@ -25,13 +29,44 @@ test('isSecretFilePath: allows ordinary files', () => { }) test('toolInputTouchesSecret: detects the path across tool input fields', () => { - expect(toolInputTouchesSecret({ file_path: '.env' })).toBe(true) // Read/Edit/Write - expect(toolInputTouchesSecret({ path: './.env.local' })).toBe(true) // Grep/Glob/Ls - expect(toolInputTouchesSecret({ glob: '.env' })).toBe(true) // Grep glob + expect(toolInputTouchesSecret({ file_path: '.env' })).toBe(true) // SDK Read/Edit/Write + expect(toolInputTouchesSecret({ path: './.env.local' })).toBe(true) // pi Read / SDK Grep + expect(toolInputTouchesSecret({ glob: '.env' })).toBe(true) // Grep/Find glob + expect(toolInputTouchesSecret({ dir: 'config/.env.staging' })).toBe(true) // pi Ls expect(toolInputTouchesSecret({ file_path: '.env.example' })).toBe(false) expect(toolInputTouchesSecret({ file_path: 'src/app.ts' })).toBe(false) }) +test('redactSecretGrepLines: drops lines sourced from a secret file', () => { + const output = [ + 'src/config.ts:12:const url = process.env.DATABASE_URL', + '.env:1:DATABASE_URL=postgres://user:pw@host/db', + 'config/.env.production:3:STRIPE_KEY=sk_live_abc', + 'src/app.ts:5:import { Seam } from "seam"', + ].join('\n') + const redacted = redactSecretGrepLines(output) + expect(redacted).toBe( + [ + 'src/config.ts:12:const url = process.env.DATABASE_URL', + 'src/app.ts:5:import { Seam } from "seam"', + ].join('\n'), + ) + expect(redacted).not.toContain('sk_live_abc') + expect(redacted).not.toContain('postgres://') +}) + +test('redactSecretGrepLines: keeps .env.example lines and is a no-op when clean', () => { + const clean = ['src/a.ts:1:x', '.env.example:1:SEAM_API_KEY='].join('\n') + expect(redactSecretGrepLines(clean)).toBe(clean) +}) + +test('redactSecretGrepLines: handles files-with-matches (bare path) output', () => { + const output = ['src/config.ts', '.env', 'src/app.ts'].join('\n') + expect(redactSecretGrepLines(output)).toBe( + ['src/config.ts', 'src/app.ts'].join('\n'), + ) +}) + test('toolInputTouchesSecret: safe on non-object / empty input', () => { expect(toolInputTouchesSecret(null)).toBe(false) expect(toolInputTouchesSecret(undefined)).toBe(false) diff --git a/src/lib/steps/harness/secret-paths.ts b/src/lib/steps/harness/secret-paths.ts index f973ff2..c4bef29 100644 --- a/src/lib/steps/harness/secret-paths.ts +++ b/src/lib/steps/harness/secret-paths.ts @@ -4,14 +4,17 @@ // model context. `.env.example` is the value-less template the wizard writes, so // it stays readable. -// Tool-input fields that carry a filesystem path across the built-in tools: -// Read/Edit/Write use `file_path`, Grep/Glob/Ls use `path`, Grep also takes a -// `glob`, and notebook tools use `notebook_path`. +// Tool-input fields that carry a filesystem path across the built-in tools of +// both harnesses. Claude Agent SDK: Read/Edit/Write use `file_path`, Grep/Glob +// use `path` + `glob`, notebooks use `notebook_path`. pi: Read/Grep/Find use +// `path`, Grep/Find also take `glob`, Ls uses `dir`/`directory`. const PATH_BEARING_KEYS = [ 'file_path', 'path', 'notebook_path', 'glob', + 'dir', + 'directory', ] as const // True for `.env` and `.env.*` at any directory depth, except `.env.example`. @@ -33,3 +36,18 @@ export function toolInputTouchesSecret(toolInput: unknown): boolean { } return false } + +// Drop lines a grep surfaced from a secret file. The PreToolUse deny only fires +// when a tool *names* a secret path, so a repo-wide grep (path ".") still scans +// `.env` and returns its matching lines; this strips them from the output before +// it reaches the model. Each grep line is prefixed with its source file +// (`path:line:text` in content mode, or a bare `path` in files-with-matches +// mode), so the leading token is the file to test. +export function redactSecretGrepLines(output: string): string { + const lines = output.split('\n') + const kept = lines.filter((line) => { + const leadingPath = line.split(':', 1)[0] ?? line + return !isSecretFilePath(leadingPath) + }) + return kept.length === lines.length ? output : kept.join('\n') +}