From a8eb4fe0b518aff9584ab771d21878b5ce0b7615 Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Sun, 3 May 2026 14:45:22 +0100 Subject: [PATCH] ML-157: task research and plan --- ...event-pi-from-accessing-sensitive-files.md | 212 ++++++++++++++++++ ...event-pi-from-accessing-sensitive-files.md | 207 +++++++++++++++++ 2 files changed, 419 insertions(+) create mode 100644 backlog/docs/doc-2 - Implementation-Analysis-Prevent-pi-from-accessing-sensitive-files.md create mode 100644 backlog/tasks/ml-157 - Prevent-pi-from-accessing-sensitive-files.md diff --git a/backlog/docs/doc-2 - Implementation-Analysis-Prevent-pi-from-accessing-sensitive-files.md b/backlog/docs/doc-2 - Implementation-Analysis-Prevent-pi-from-accessing-sensitive-files.md new file mode 100644 index 00000000..88354b2e --- /dev/null +++ b/backlog/docs/doc-2 - Implementation-Analysis-Prevent-pi-from-accessing-sensitive-files.md @@ -0,0 +1,212 @@ +--- +id: doc-2 +title: 'Implementation Analysis: Prevent pi from accessing sensitive files' +type: other +created_date: '2026-05-03 13:32' +--- +# Implementation Analysis: Prevent pi from accessing sensitive files + +## Summary + +The task requires a **declarative** way to intercept pi tool calls that would access sensitive files (e.g., `.env`, secrets, credentials, API keys). Pi's extension system provides the `tool_call` event hook, which fires **before** any tool executes and supports blocking. + +## Sensitive File Categories (for this project) + +| Category | Patterns | Examples | +|----------|----------|----------| +| Environment secrets | `.env`, `.env.*`, `.envrc` | `.env`, `.env.production` | +| Key files | `*.pem`, `*.key`, `*.key.pub` | SSH keys | +| Secret/credential files | `*secret*`, `*credential*`, `*credentials*` | `secrets.yml`, `credentials.json` | +| Config dirs | `.ssh/`, `.aws/`, `.gnupg/` | SSH config, AWS credentials | +| Production config | Any path known to contain secrets | `rel/`, production env vars | +| Pi session files | `~/.pi/agent/sessions/` | Session JSONL files (contain tool outputs) | + +## Available Pi Extension Mechanisms + +### `tool_call` event (primary approach) + +The `tool_call` event fires after `tool_execution_start` and **before** the tool executes. It can block execution: + +```typescript +pi.on("tool_call", async (event, ctx) => { + if (event.toolName === "read" && isSensitivePath(event.input.path)) { + return { block: true, reason: "Sensitive file blocked" }; + } +}); +``` + +**Key capabilities:** +- `event.toolName` — identifies the tool (`read`, `bash`, `grep`, `find`, `ls`, `write`, `edit`) +- `event.input` — tool parameters (mutable, can be patched) +- `isToolCallEventType("read", event)` — type-narrows for specific tools +- Return `{ block: true, reason: "..." }` to prevent execution +- `ctx.hasUI` — check if interactive (for notifications) +- `ctx.ui.notify(...)` — show warning in UI + +### Tools that can access sensitive files + +| Tool | Danger | Input fields to check | +|------|--------|----------------------| +| `read` | **High** — reads file content into LLM context | `input.path` | +| `bash` | **High** — can `cat`, `grep`, `curl` any file | `input.command` (regex match) | +| `grep` | **High** — searches file contents | `input.path` | +| `find` | **Low** — lists filenames only | `input.path` (directory listing) | +| `ls` | **Low** — lists directory contents | `input.path` | +| `write` | **Medium** — could overwrite sensitive files | `input.path` | +| `edit` | **Medium** — could modify sensitive files | `input.path` | + +### `tool_result` event (secondary approach) + +Fires **after** execution. Can modify results but NOT block. Useful for sanitization, but harder to get right: + +```typescript +pi.on("tool_result", async (event, ctx) => { + // Modify event.content or event.details + return { content: [...], details: {...} }; +}); +``` + +### Full tool override (heavyweight approach) + +Register a replacement for `read`/`bash` with the same name. Requires reimplementing built-in functionality: + +```typescript +pi.registerTool({ + name: "read", + // ... must implement full read logic +}); +``` + +## Implementation Routes + +### Route A: `tool_call` event interception (RECOMMENDED) + +**Description:** Single extension listening on `tool_call`, checking paths/commands against a configuration file of protected patterns. Blocks matching calls. + +**Implementation:** +1. Create `.pi/extensions/sensitive-file-guard.ts` (or `index.ts` in a directory) +2. Create `.pi/sensitive-paths.json` — declarative config listing protected patterns +3. In `tool_call` handler: + - For `read`, `grep`, `write`, `edit`: check `input.path` against patterns + - For `bash`: regex-scan `input.command` for sensitive paths + - For `find`, `ls`: check directory paths + - Block if match, notify in UI + +**Sample config file (`.pi/sensitive-paths.json`):** +```json +{ + "blocked_paths": [ + ".env", + ".env.*", + "*.pem", + "*.key", + "*secret*", + "*credential*", + "*credentials*", + ".ssh/", + ".aws/", + ".gnupg/", + "config/*.secret.exs" + ], + "blocked_commands": [ + "cat .env", + "cat ~/.ssh", + "printenv", + "cat ~/.aws" + ] +} +``` + +**Pros:** +- Minimal code (~60 lines of TypeScript) +- Declarative configuration file (easy to audit and update) +- Works with ALL tools (read, bash, grep, find, ls, write, edit) +- No reimplementation of built-in tools +- Inherits built-in rendering (syntax highlighting, diffs, etc.) +- Easy to extend with new patterns +- Fails safe (blocks before any file access) + +**Cons:** +- Can't sanitize partial output (but blocking prevents all access) +- Regex-based command scanning for bash has edge cases +- `find`/`ls` only blocked at directory level, not per-file + +**Code size estimate:** ~80 lines TypeScript + ~20 lines JSON config + +--- + +### Route B: Full tool override (read + bash) + +**Description:** Override the `read` and `bash` tools with custom implementations that include path checking. Delegates to original implementation for allowed paths. + +**Implementation:** +1. Create `.pi/extensions/sensitive-file-guard/` directory +2. Override `read` tool: check paths, delegate allowed reads to original implementation +3. Override `bash` tool: scan commands, delegate allowed commands to original implementation +4. Optionally override `grep`, `find`, `ls`, `write`, `edit` + +**Pros:** +- Full control (can log, can sanitize output, can modify behavior) +- No event overhead per tool call +- Can add audit logging + +**Cons:** +- Must reimplement or re-wrap built-in tool logic (~200+ lines) +- Must maintain compatibility with pi updates +- More complex testing +- Risk of subtle behavior differences vs built-in +- Lose or must reimplement built-in rendering + +**Code size estimate:** ~300+ lines TypeScript + +--- + +### Route C: Hybrid — `tool_call` + `tool_result` + config file + +**Description:** Route A plus `tool_result` sanitization for cases where partial access is allowed. + +**Implementation:** +Same as Route A, plus a `tool_result` handler that scans output for sensitive patterns and redacts them. + +**Pros:** +- Most comprehensive protection +- Can allow partial access with sanitization + +**Cons:** +- Most complex +- `tool_result` sanitization is error-prone (false positives/negatives) +- Redacted content still consumes context tokens +- Hard to get right for all output formats + +**Code size estimate:** ~150 lines TypeScript + +--- + +## Recommendation: Route A + +Route A (pure `tool_call` event interception with declarative config) is the **simplest viable approach** that meets all requirements: + +1. **Declarative** ✓ — JSON config file lists protected patterns +2. **Comprehensive** ✓ — Covers all tools that can access files +3. **Simple** ✓ — ~80 lines of code, easy to review and maintain +4. **Safe** ✓ — Blocks BEFORE any file access, cannot fail open +5. **Minimal** ✓ — No reimplementation needed, no dependency on pi internals + +## Architectural Impact + +| Touchpoint | Impact | +|------------|--------| +| `.pi/extensions/` | New extension file added | +| `.pi/sensitive-paths.json` | New config file (declarative rules) | +| `tool_call` event | New subscriber, no breaking changes | +| Built-in tools | Unchanged (event interception is non-invasive) | +| `ctx.ui` | Used for notification only (no blocking dependency) | +| Other extensions | Compatible — tool_call handlers chain in load order | + +## Open Questions + +1. **Config format:** JSON vs YAML? JSON is simpler (no dependencies), YAML is more readable. +2. **Per-project vs global:** Should this be project-local (`.pi/`) or global (`~/.pi/agent/`)? +3. **Command scanning depth:** For bash, how aggressively should we scan? Regex on the command string has edge cases (e.g., `cat $HOME/.env`). +4. **Logging:** Should blocked attempts be logged for audit? +5. **Non-interactive mode:** Should sensitive access be blocked silently or with error in print/JSON modes? diff --git a/backlog/tasks/ml-157 - Prevent-pi-from-accessing-sensitive-files.md b/backlog/tasks/ml-157 - Prevent-pi-from-accessing-sensitive-files.md new file mode 100644 index 00000000..70e8c817 --- /dev/null +++ b/backlog/tasks/ml-157 - Prevent-pi-from-accessing-sensitive-files.md @@ -0,0 +1,207 @@ +--- +id: ML-157 +title: Prevent pi from accessing sensitive files +status: To Do +assignee: [] +created_date: '2026-05-03 13:30' +updated_date: '2026-05-03 13:44' +labels: [] +dependencies: [] +references: + - 'backlog://document/doc-2' +--- + +## Description + + +To prevent the pi harness from accidentally reading and sending sensitive data to the LLM, we need a declarative way to intercept problematic commands that interact with sensitive files that for example contain secrets. + + +## Acceptance Criteria + +- [ ] #1 Pi cannot read `.env` files via the `read` tool — access is blocked with a notification in interactive mode +- [ ] #2 Pi cannot read files matching `*secret*` or `*credential*` patterns via the `read` tool +- [ ] #3 Pi cannot `cat .env` or `grep` inside `.ssh/` or `.aws/` via the `bash` tool +- [ ] #4 Non-interactive mode (`pi -p`) reports blocked sensitive file access as an error rather than silently failing +- [ ] #5 Normal file access (source code, test files, config examples like `.env.example`) is unaffected +- [ ] #6 The set of blocked paths is declared in `.pi/sensitive-paths.json` — adding/removing patterns does not require code changes +- [ ] #7 The extension loads correctly at pi startup and chains with existing extensions (MCP adapter) without conflicts + + +## Implementation Plan + + +## Implementation Plan + +### 1. Objective Alignment + +The problem: pi's `read`, `bash`, `grep`, `find`, `ls`, `write`, and `edit` tools can access any file on disk, including secrets (`.env`, API keys, SSH keys, credentials). When a file is read, its content is sent to the LLM provider, potentially leaking secrets to a third party. + +The solution: a pi extension that intercepts `tool_call` events before execution, checks the target paths against a declarative JSON config of blocked patterns, and blocks the call if it matches. This prevents any file content from reaching the LLM — the block happens before the tool executes, so no data is ever read from disk. + +### 2. Simplicity and Alternatives Considered + +**Chosen: Route A — `tool_call` event interception with JSON config** + +This is the simplest viable approach: +- ~80 lines of TypeScript + a JSON config file +- Uses pi's built-in `tool_call` event hook (no tool reimplementation) +- Inherits built-in rendering and behavior for all tools +- Fail-safe: blocks BEFORE file access, cannot leak data + +**Rejected alternatives:** + +- **Route B (full tool override):** Requires reimplementing `read`, `bash`, `grep`, etc. (~300+ lines). Must maintain compatibility with pi's internal tool interfaces. Risk of subtle behavior differences. Overkill for a blocking gate. + +- **Route C (tool_call + tool_result sanitization):** `tool_result` sanitization is error-prone — redacting secrets from arbitrary output is the same class of problem as sanitizing LLM output. Blocking upfront is safer and simpler. + +- **Filesystem permissions (e.g., `chmod`):** Would break the application itself (Phoenix needs to read `.env`). Only pi needs to be restricted, not all processes. + +- **`.gitignore`-based approach:** `.gitignore` controls what's tracked, not what's read. Many sensitive files are intentionally tracked (`.env.example`) or excluded from git for size (not sensitivity). + +### 3. Completeness and Sequencing + +**Step 1: Create blocked paths configuration** + +File: `.pi/sensitive-paths.json` + +Content: JSON object with `blocked_paths` (glob-like patterns for path matching) and `blocked_command_patterns` (regex patterns for bash command scanning). + +Initial patterns cover: +- `.env` files: `.env`, `.env.*`, `.envrc` +- Key files: `*.pem`, `*.key`, `*.key.pub` +- Secret/credential files: paths containing `secret`, `credential`, `credentials` +- Sensitive directories: `.ssh/`, `.aws/`, `.gnupg/` +- Pi session files (contain tool outputs): `~/.pi/agent/sessions/` + +Verification: File exists at `.pi/sensitive-paths.json` and is valid JSON. + +**Step 2: Create the extension** + +File: `.pi/extensions/sensitive-file-guard.ts` + +Implementation: +1. Read `.pi/sensitive-paths.json` at extension load time (synchronous, using `readFileSync`) +2. Compile `blocked_paths` entries into a regex matcher (glob → regex conversion) +3. Subscribe to `tool_call` event +4. Handler logic: + - For `read`, `grep`, `write`, `edit`, `find`, `ls`: check `event.input.path` against the compiled blocked path regexes. Resolve relative paths against `ctx.cwd`. + - For `bash`: check `event.input.command` against `blocked_command_patterns` regexes AND scan for blocked paths in the command string + - If match found and `ctx.hasUI` is true: show a warning notification via `ctx.ui.notify()` + - If match found: return `{ block: true, reason: "Blocked sensitive path: " }` (error message for non-interactive mode) + - If no match: return `undefined` (allow execution) + +Verification: +- Start pi in this project, ask it to read `.env.example` (should work — this is NOT a secret, it's the example) +- Ask pi to read `.env` (should be blocked with a notification) +- Ask pi to run `cat .env` via bash (should be blocked) +- Ask pi to run `ls .` (should work — listing the project root is fine) +- Run `pi -p "read .env"` (non-interactive mode — should report error) + +**Step 3: Verify non-interactive mode behavior** + +The extension must handle `ctx.hasUI === false` (print mode, JSON mode) by returning a descriptive error reason string, so the blocked access is reported to stdout rather than silently swallowed. + +Verification: +```bash +pi -p "read the .env file" 2>&1 | grep -i "blocked" +# Should produce output indicating the access was blocked +``` + +**Step 4: Verify other tools are unaffected** + +Ask pi to: +- Read a normal source file (e.g., `lib/music_library.ex`) +- Edit a normal file +- Run `mix test` +- Search with grep for normal code patterns + +All should work as usual. + +Verification: All normal pi operations are unaffected. + +### 4. Verifiability + +Each step includes concrete verification instructions above. Overall verification suite: + +| Test Case | Tool | Target | Expected | +|-----------|------|--------|----------| +| Read .env | read | `.env` | Blocked | +| Read .env.production | read | `.env.production` | Blocked | +| Read .env.example | read | `.env.example` | **Allowed** (example file) | +| Read secrets file | read | `config/secrets.yml` | Blocked | +| Cat .env via bash | bash | `cat .env` | Blocked | +| Grep in .ssh | grep | `~/.ssh/config` | Blocked | +| Ls in .aws | ls | `~/.aws/` | Blocked | +| Write to .env | write | `.env` | Blocked | +| Edit .env | edit | `.env` | Blocked | +| Read normal .ex file | read | `lib/music_library.ex` | Allowed | +| Run mix test | bash | `mix test` | Allowed | +| Non-interactive read .env | read | `.env` (print mode) | Error reported | + +### 5. Architecture Impact Analysis + +| Touchpoint | Impact | +|------------|--------| +| `.pi/extensions/sensitive-file-guard.ts` | **New file** — extension entry point | +| `.pi/sensitive-paths.json` | **New file** — declarative blocked patterns config | +| `tool_call` event | New subscriber, chains with existing extensions (e.g., MCP adapter) | +| Built-in tools | **Unchanged** — interception is non-invasive | +| Phoenix/Ecto/Oban | **No impact** — pi-only concern, not part of the Elixir application | +| Production infrastructure | **No impact** — this is developer-local tooling, not server configuration | +| Other pi extensions (MCP adapter) | Compatible — `tool_call` handlers chain in load order. The guard runs alongside, not instead of. | + +**No migration or deprecation path needed** — this is a net-new capability. + +**Rollback:** Delete `.pi/extensions/sensitive-file-guard.ts` and `.pi/sensitive-paths.json` to remove all protections. No cleanup needed. + +### 6. Performance Profile + +- **Runtime complexity:** `O(p + c)` where `p` is number of blocked path patterns (~15) and `c` is number of blocked command patterns (~5). Each check is a single regex test. Total per tool call: <1ms. +- **Memory footprint:** Patterns are compiled once at extension load time. Regex objects and config: ~5KB. +- **Database queries:** None. This is a filesystem-level guard. +- **N+1 risks:** None. No database interaction. +- **Latency:** Negligible. The guard adds <1ms to each tool call, imperceptible next to tool execution time (often 100ms+ for file I/O). + +### 7. Benchmarking Requirements + +**No benchmarks needed.** The guard is a simple gate with constant-time pattern matching: +- O(20) regex tests per tool call +- No I/O beyond the initial config read (done once at startup) +- No allocations beyond the return value object +- Performance profile is self-evident from the code structure + +If future patterns grow to hundreds of entries, a trie-based matcher could be considered, but the current scale (~15 patterns) does not warrant it. + +### 8. Cost Profile + +**Zero cost.** The guard: +- Uses no third-party APIs +- Consumes no paid resources +- Runs entirely locally in the pi process +- No additional compute, storage, or services required + +### 9. Production Infrastructure Steps + +**No production changes required.** This change: + +- Is **developer-local tooling** (pi configuration files in `.pi/`) +- Does not affect the Elixir application, Phoenix server, database, or any deployed infrastructure +- Is not deployed to production servers +- Does not require environment variables, service provisioning, DNS changes, or firewall rules + +**Rollout:** Each developer places the extension and config in their local `.pi/` directory. No server-side deployment. + +**Rollback:** Remove the two files locally. No server-side action needed. + +### 10. Documentation Updates + +**`docs/project-conventions.md`** — Add a section "Pi Security Configuration" documenting: +- The existence of the sensitive file guard extension +- The location and format of `.pi/sensitive-paths.json` +- How to add or remove blocked patterns +- The behavior in interactive vs. non-interactive modes +- How to temporarily disable (comment out patterns, or start pi with `--no-extensions`) + +**No other documentation files need updates.** This is a pi-level concern, not an application architecture concern. +