Skip to content

WEB-5004: attach referenced file contents to tool telemetry - #195

Open
pugazhendhi-m wants to merge 10 commits into
stagingfrom
WEB-5004
Open

WEB-5004: attach referenced file contents to tool telemetry#195
pugazhendhi-m wants to merge 10 commits into
stagingfrom
WEB-5004

Conversation

@pugazhendhi-m

@pugazhendhi-m pugazhendhi-m commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

What

Adds a uniform file_content field to the PreToolUse and PostToolUse/Stop telemetry payloads of all five coding-tool hooks (claude-code, cursor, codex, copilot, augment). For a file tool (Read/Write/Edit/MCP), the hook reads the referenced file and attaches:

file_content = [{ "path": ..., "content": ..., "truncated": <bool> }]

Uniform key + shape across every tool (single-file = 1-element list) so the gateway/backend can consume it identically.

How

  • Shared helpers (_cap_file_text, _resolve_and_read_file, _make_file_entry, _attach_file_content) are byte-identical across all 5 hooks (per-tool parity).
  • Write reuses the inline content (file may not exist on disk yet); Read/Edit read from disk, resolving relative paths against cwd (claude/cursor paths are already absolute).
  • 64KB per-file cap, truncate + per-entry truncated flag.
  • Wired at PRE (pre_tool_use_data.metadata) and POST (end-of-turn tool_use entries).

Safety / fail-open

  • Never raises or blocks the editor. Missing / binary (\x00) / non-UTF8 / permission-denied / directory / unresolvable-relative-path all silently skip.
  • os.path.isfile excludes fifos/devices/sockets (no hangs); read(cap+1) bounds huge files.
  • No new imports (only os) — no binary/unbound-hook.spec change needed.
  • Additive payload — backward compatible with the installed fleet.

Scope notes

  • Bash/shell excluded by design — only tools with a discrete file-path field.
  • No aggregate per-turn cap (per-file already bounded; the Stop exchange already carries content for Read/Write).

Follow-up (separate, gateway/backend — NOT this repo)

Gateway DLP must redact the new file_content field (not just prompt bodies), otherwise .env/keys read by the agent get stored server-side. Flagged by both /cso and /review as the one real governance item of always-on.

Testing

  • All 5 python3 -m py_compile clean.
  • Helper block md5-verified identical across all 5 files.
  • Behavioral tests pass for: abs/relative+cwd, truncation flag, binary/non-UTF8/missing/directory/permission-denied/None/empty/non-str path skips, write-inline, null-path guard, JSON-serializable output.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GrSjwjtXsfxcBpANpkqQg4


Note

High Risk
Hooks intentionally read and upload file text (including secrets) for leak detection, and Bash token heuristics can attach files from non-read commands; gateway DLP must redact file_content or sensitive data is stored server-side.

Overview
Adds a shared file_content sibling field ([{path, content, truncated}]) to PreToolUse metadata and end-of-turn tool-use payloads across augment, claude-code, cursor, codex, and copilot hooks.

File tools resolve paths with the turn cwd, reuse inline write text when present, otherwise read capped UTF-8 from disk (64KB/file, 128KB total, up to 5 files). Bash/shell commands also scan command tokens for existing files and attach the same shape; only /proc and /sys are excluded (symlinks dereferenced). Fail-open: binary, missing, or unreadable paths are skipped without blocking the editor.

Copilot threads cwd through transcript mapping; Codex enriches stop-event tool uses the same way. Claude Code separately replaces “latest session file” MCP resolution with a cwd→session JSON lookup.

Augment gains behavioral tests for Bash attachment, truncation, cwd, and exclusion guards; the helper block is duplicated identically in the other four hooks.

Reviewed by Cursor Bugbot for commit 3111950. Bugbot is set up for automated code reviews on this repo. Configure here.

Greptile Summary

This PR attaches a uniform file_content field — a list of {path, content, truncated} entries — to PreToolUse and PostToolUse/Stop telemetry payloads across all five coding-tool hooks (augment, claude-code, codex, copilot, cursor). The helper block is byte-identical across all five files, with a 64 KB per-file cap, 128 KB aggregate cap, and a 5-file limit per tool call. The implementation is fully fail-open: binary files, permission errors, /proc//sys paths, and symlinks into excluded directories all silently skip without blocking the editor.

  • Shared helper set (_cap_file_text, _resolve_existing_file, _make_file_entry, _append_file_entry, _attach_file_content, _attach_command_file_content, _extract_command_file_paths) duplicated verbatim into all five hook files; Write-style tools use inline content from the event, Read/Edit read from disk via realpath after exclusion checks.
  • Bash/shell support extracts file-path tokens from command strings via whitespace splitting, resolves each against the turn's cwd, and skips binaries (null-byte check), directories, and unreadable files.
  • claude-code side change replaces "glob for latest session JSON" MCP UUID resolution with _session_file_from_cwd, which derives the session file from the tool's cwd and validates it is within a known Claude Desktop support directory.

Confidence Score: 4/5

Safe to merge with one correctness gap in the Codex hook — all other hooks are clean.

The Codex stop-event handler uses a single cwd for every tool call extracted from the session transcript. If the agent's shell changed working directories mid-session, relative file-path tokens in earlier commands are resolved against the stop-event's project root rather than the directory those commands actually ran in — meaning a different file with the same relative name could be attached to the wrong tool call. All four other hooks and the helper implementation are correct and well-tested; the Codex gap is isolated to relative-path resolution in multi-directory sessions.

codex/hooks/unbound.py — the stop-event cwd enrichment loop at the bottom of process_stop_event.

Important Files Changed

Filename Overview
augment/hooks/test_augment_hooks.py Adds 10 new test cases covering Bash command file attachment, binary/missing file skipping, relative path resolution via cwd, truncation, /proc exclusion, symlink-into-proc exclusion, and inline content. Tests are thorough and cover the key safety edge cases.
augment/hooks/unbound.py Adds file-content telemetry helpers and wires them into pre-tool-use metadata and the post-tool-use exchange builder. Bash token scanning attaches files referenced in shell commands. Error handling is fully fail-open.
claude-code/hooks/unbound.py Adds the same file-content helper block; also replaces the "latest session JSON" glob-scan in _resolve_claude_code_session_connector with _session_file_from_cwd, which derives the session JSON from the cwd path and validates it within known Claude Desktop support directories.
codex/hooks/unbound.py Adds file-content helpers and enriches parsed transcript tool uses at Stop. Uses the Stop event's single cwd for all tool uses from the transcript, which can resolve relative paths against the wrong directory if the agent's shell changed directories mid-session.
copilot/hooks/unbound.py Adds file-content helpers and passes cwd through map_copilot_tool and build_exchange_from_transcript. File content is attached inside map_copilot_tool after the entry is built, covering both file-path and command-type tools.
cursor/unbound.py Adds file-content helpers and wires them into process_pre_tool_use, process_pre_tool_use_execution (Bash path), and all post-tool hooks in build_llm_exchange (beforeReadFile, postToolUse, afterFileEdit, afterShellExecution). afterMCPExecution is intentionally unchanged.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Hook event fires
PreToolUse / PostToolUse / Stop] --> B{Has file_path
in tool_input?}
    B -->|Yes| C[_attach_file_content
path + cwd + inline_content]
    B -->|No| D{Is Bash/shell
tool?}
    D -->|Yes| E[_extract_command_file_paths
split command on whitespace]
    D -->|No| Z[No file_content
attached]
    E --> F[_resolve_existing_file
for each token]
    F --> G{realpath within
excluded paths?}
    G -->|/proc or /sys| H[Skip — excluded]
    G -->|OK| I[_read_file_text
read up to 64 KB]
    I --> J{Binary?
null byte check}
    J -->|Yes| K[Skip]
    J -->|No| L[_cap_file_text
truncate + flag]
    C --> M[_make_file_entry
build dict]
    M --> N[_append_file_entry
check count/total caps
dedup by path]
    L --> N
    N --> O[target file_content
list attached as sibling]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Hook event fires
PreToolUse / PostToolUse / Stop] --> B{Has file_path
in tool_input?}
    B -->|Yes| C[_attach_file_content
path + cwd + inline_content]
    B -->|No| D{Is Bash/shell
tool?}
    D -->|Yes| E[_extract_command_file_paths
split command on whitespace]
    D -->|No| Z[No file_content
attached]
    E --> F[_resolve_existing_file
for each token]
    F --> G{realpath within
excluded paths?}
    G -->|/proc or /sys| H[Skip — excluded]
    G -->|OK| I[_read_file_text
read up to 64 KB]
    I --> J{Binary?
null byte check}
    J -->|Yes| K[Skip]
    J -->|No| L[_cap_file_text
truncate + flag]
    C --> M[_make_file_entry
build dict]
    M --> N[_append_file_entry
check count/total caps
dedup by path]
    L --> N
    N --> O[target file_content
list attached as sibling]
Loading

Comments Outside Diff (2)

  1. augment/hooks/unbound.py, line 2267-2290 (link)

    P1 security Bash token-scanning attaches files that the command never reads

    _extract_command_file_paths splits the command string on whitespace and checks every token against the filesystem. This means git add .env, rm -f secrets.json, or chmod 600 ~/.ssh/id_rsa would all attach the content of those sensitive files to telemetry — even though none of those commands read the file. The PR description states "Bash/shell excluded by design — only tools with a discrete file-path field," which directly contradicts the implementation. If Bash is intentionally in scope, the token heuristic should at minimum limit itself to tokens that follow read-oriented verbs (cat, less, head, tail, grep), not every command.

  2. augment/hooks/unbound.py, line 2230-2250 (link)

    P2 Silent error swallowing prevents any failure visibility

    Every helper (_make_file_entry, _append_file_entry, _attach_file_content, _attach_command_file_content, _extract_command_file_paths) catches all exceptions and returns silently. Per the team's logging standards, errors should be logged with context so failures can be reconstructed. If file attachment is silently failing for a subset of users or tools, there is currently no way to observe or diagnose it from logs alone. At minimum, a logging.debug or equivalent with the exception and relevant path should be emitted on each caught exception.

Reviews (6): Last reviewed commit: "WEB-5004: review polish — MCP/shell gati..." | Re-trigger Greptile

pugazhendhi-m and others added 2 commits July 6, 2026 16:09
Add a uniform `file_content` field to the PreToolUse and PostToolUse/Stop
payloads of all five coding-tool hooks (claude-code, cursor, codex, copilot,
augment). For a file tool (Read/Write/Edit/MCP) the hook reads the referenced
file and attaches `file_content = [{path, content, truncated}]`.

- Shared helpers (_cap_file_text, _resolve_and_read_file, _make_file_entry,
  _attach_file_content) are byte-identical across all five hooks.
- Write reuses the inline content (file may not exist on disk yet); Read/Edit
  read from disk, resolving relative paths against cwd.
- 64KB per-file cap with a per-entry `truncated` flag.
- Fail-open: missing/binary/non-UTF8/permission-denied/directory/unresolvable
  paths are silently skipped; no new imports; never raises or blocks the editor.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GrSjwjtXsfxcBpANpkqQg4
…ity)

The augment PostToolUse wiring attached file_content INSIDE tool_input
(canon_input); the other four tools attach it as a sibling on the tool_use
object. Align augment with them so tool_input stays clean and the key is uniform
across tools. Fixes test_posttooluse_canonicalization, which the augment suite
wasn't run against during the original change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GrSjwjtXsfxcBpANpkqQg4
Extend the file_content telemetry to shell/terminal commands, where no file
path is provided natively: parse the command string (or argv list), resolve
each token that is a real existing file (absolute path used directly, relative
built off cwd), read its text, and attach file_path + file_content to the
pre-tool metadata and the end-of-turn tool_use object.

- Uniform helper block, byte-identical across all 5 hooks; entry paths are
  always absolute; content only when readable UTF-8 text (binary/unreadable
  files are skipped entirely, no path-without-content).
- Caps: 64KB/file, 128KB total (UTF-8 bytes), 5 files, 256-token scan.
- Skip /proc and /sys pseudo-filesystem paths (process/kernel state).
- Fail-open everywhere: any read error (permission denied, missing, corrupt,
  binary) is skipped silently — never raises, never logs, never hits Sentry,
  never blocks the editor.
- No external libraries; docstrings kept to <=2 lines.

Verified the field passes through gateway -> Celery -> DB (stored as-is in
JSONFields) harmlessly; UI is unaffected (not consumed yet).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GrSjwjtXsfxcBpANpkqQg4
@pugazhendhi-m
pugazhendhi-m marked this pull request as ready for review July 6, 2026 13:18
@pugazhendhi-m
pugazhendhi-m requested a review from a team July 6, 2026 13:18
@vigneshsubbiah16

This comment was marked as resolved.

Comment thread augment/hooks/unbound.py
Comment thread augment/hooks/unbound.py
Comment thread codex/hooks/unbound.py
Comment thread augment/hooks/unbound.py
Comment thread claude-code/hooks/unbound.py
Comment thread augment/hooks/unbound.py
… POST

- _resolve_existing_file uses os.path.realpath (was abspath) so a symlink
  pointing into /proc or /sys can no longer bypass the exclusion guard.
- _append_file_entry checks the 128KB total cap including the new entry, so the
  aggregate never overshoots by one file.
- Gate the PostToolUse command file-content attach on tool_name == 'Bash' in
  claude-code and codex (was firing for any tool_use carrying a 'command' key);
  cursor/copilot/augment POST were already shell-gated.
- Tests: symlink-into-/proc exclusion; realpath-canonicalized path assertions.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GrSjwjtXsfxcBpANpkqQg4
@pugazhendhi-m

Copy link
Copy Markdown
Contributor Author

Addressing the two Greptile comments outside the diff:

  • Bash token over-capture (P1): intentional. Capturing files referenced by a command is the requested design (git add a.txt should attach a.txt even though git doesn't read it). We restrict on-device to real existing files only, skip /proc//sys (hardened against symlink bypass in d426296), and cap at 5 files / 64KB / 128KB. Broader secret redaction (.env, keys) is the gateway DLP follow-up tracked in the PR description.
  • Silent error swallowing (P2): intentional and required. Reading a referenced file failing (permission denied, missing, corrupt, binary) is normal on any device; per the design these must skip silently — no logging and no Sentry — to avoid flooding telemetry with expected non-errors. This is a deliberate deviation from the general logging standard for this specific best-effort read path.

vigneshsubbiah16

This comment was marked as resolved.

Comment thread augment/hooks/unbound.py Outdated
Comment thread augment/hooks/unbound.py
Comment thread cursor/unbound.py
Add an on-device guard so the hooks never read or attach well-known secret
files: SSH/TLS keys (.ssh//.aws//.gnupg/, id_rsa*, *.pem/*.key/*.p12/*.pfx),
dotenv (.env*), cloud/credential files (gcloud/kube/docker config, *credential*,
.npmrc/.netrc/.pgpass/.git-credentials/.pypirc/.boto/.databrickscfg), and
terraform (*.tfvars/*.tfstate).

- Critical fix: the guard now also runs on the INLINE-content path
  (_make_file_entry) — previously a Read's tool_response.content / a Write's
  tool_input.content shipped secrets even though the disk-read path was guarded.
- Symlinks to secret/proc paths are caught via realpath; _is_excluded_path is
  null-safe. Fail-open unchanged: excluded/unreadable files are silently skipped.

Directly addresses the "file content incl. secrets shipped before gateway DLP"
review finding — secrets no longer leave the device regardless of DLP status.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GrSjwjtXsfxcBpANpkqQg4
@pugazhendhi-m

This comment was marked as resolved.

Comment thread codex/hooks/unbound.py
Comment thread augment/hooks/unbound.py Outdated
vigneshsubbiah16

This comment was marked as resolved.

…ete parity

- _resolve_existing_file: a relative path now resolves ONLY against the tool
  turn's cwd, never the hook's own process cwd (an absolute path is used
  directly). Prevents reading a same-named file from the hook's working dir.
- _is_excluded_path: also skip *.env-suffixed and *.env.* files (production.env,
  app.env.local), not just leading-dot .env*.
- cursor postToolUse native-tool entries now attach file_content (uniform with
  beforeReadFile/afterFileEdit); no-op for deleted files.
- Tests for relative-cwd isolation and the *.env-suffix cases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GrSjwjtXsfxcBpANpkqQg4
Comment thread copilot/hooks/unbound.py
Comment thread copilot/hooks/unbound.py Outdated
vigneshsubbiah16

This comment was marked as resolved.

pugazhendhi-m and others added 2 commits July 6, 2026 20:28
…d /sys

Per product decision this is a data-leak-DETECTION feature: the hook should read
the content of ALL files an agent touches (including .env, keys, credentials)
so the platform can identify and prevent exfiltration; the gateway DLP layer
redacts secret values server-side. Simplify accordingly:

- _is_excluded_path now excludes only /proc and /sys (removed the on-device
  sensitive-file denylist entirely). Null-safe and cross-platform (Windows paths
  never match the /proc,/sys prefixes).
- Move the _MAX_FILE_CONTENT_* cap constants to the top of each hook file.
- Document intent with a one-line comment in _read_file_text.
- Align claude build_llm_exchange empty-string inline handling with augment.
- Update tests to assert the read-all / proc-sys-only behavior.

Fail-open unchanged; helper block byte-identical across all 5 hooks.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GrSjwjtXsfxcBpANpkqQg4
PreToolUse passed only tool_input.content as inline text; align with POST
map_copilot_tool which uses content OR file_text, so a Write via file_text
attaches its inline content instead of falling back to a disk read.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GrSjwjtXsfxcBpANpkqQg4

@vigneshsubbiah16 vigneshsubbiah16 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛡️ Automated Security Review (consensus)

3 findings — 1 high-confidence, 2 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.


🔴 HIGH — On-device sensitive-file guard missing; credentials ship in file_content

augment/hooks/unbound.py:2068 (identical in all 5 unbound.py hooks)

Impact: _is_excluded_path excludes only /proc and /sys; .env, ~/.ssh/id_rsa, .aws/credentials, *.pem, etc. are read and attached to Pre/Post telemetry. test_only_proc_sys_excluded_other_files_read asserts those paths are not excluded. This contradicts the thread claim (510500c/ea36efe) that on-device secret blocking was added and closes the stated precondition for shipping raw bodies pre-gateway-DLP.

Fix: Restore the sensitive-path guard on both disk-read (_resolve_existing_file) and inline (_make_file_entry) paths; add regression tests; remove or invert test_only_proc_sys_excluded_other_files_read.

Flagged by: Claude, Lead


🟡 TRIAGE — Misleading “prevent data leaks” comments on sensitive reads

augment/hooks/unbound.py:2094 (all 5 hooks); augment/hooks/test_augment_hooks.py:~510

Impact: # Read content of all files including sensitive files to identify and prevent data leaks and the matching test docstring describe a DLP/scan policy the hook does not implement (no on-device redaction; gateway DLP is an out-of-repo follow-up), which can mislead reviewers and operators about actual data handling.

Fix: Remove or rewrite comments/tests to state accurately: content is collected for gateway-side handling; on-device skips are limited to /proc//sys until the sensitive-path guard lands.

Flagged by: Claude


🟡 TRIAGE — Inline entries store normpath, not realpath

augment/hooks/unbound.py:2120 (all 5 hooks)

Impact: Write/inline file_content entries record _abspath() (normpath, symlinks not dereferenced) while disk reads store realpath; a symlink alias (e.g. ./notes~/.aws/credentials) can ship secret content under a benign-looking path, weakening future gateway path-based redaction/dedup.

Fix: Store os.path.realpath(abspath) on the inline branch, matching _resolve_existing_file.

Flagged by: Claude


Previously acknowledged (not re-flagged)

  • Bash token over-capture (git add .env, rm secrets.json, etc.) — intentional: attach files referenced by command tokens, not only read by the shell (pugazhendhi-m).
  • Raw file_content shipped before gateway DLP — accepted follow-up: feature purpose is DLP visibility; gateway redaction tracked separately (pugazhendhi-m; greptile acknowledged).
  • Silent exception swallowing on file reads — intentional fail-open; no logging/Sentry for expected skips (pugazhendhi-m).
  • Bash file_content read at exchange-assembly time — known best-effort limitation when later steps mutate files (pugazhendhi-m).
  • Stop-replay uses single event cwd — best-effort; relative paths may miss after cd (pugazhendhi-m).
  • apply_patch PreToolUse omits file_content — intentional; paths live in patch body, POST captures result (pugazhendhi-m).
  • Edit POST prefers full disk read over new_string snippet — intentional for completeness (pugazhendhi-m).

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head 1b6fce28 · 2026-07-06T15:02Z

Comment thread cursor/unbound.py
Comment thread cursor/unbound.py Outdated
Comment thread augment/hooks/unbound.py
Comment thread codex/hooks/unbound.py
An earlier rebase left claude-code/hooks/unbound.py with stale/old versions of
the MCP connector and script-hash helpers: _resolve_claude_code_session_connector
had reverted to a broad newest-session scan (losing staging's cwd-scoped lookup),
and _compute_script_hash / _hook_candidate_script / _HOOK_SCRIPT_* were missing
entirely while still being called (a latent runtime NameError on the MCP path).

Rebuilt the file from staging and re-applied only the file_content changes, so it
now differs from staging solely by this PR's feature. Helper block stays
byte-identical across all 5 hooks; 111 tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GrSjwjtXsfxcBpANpkqQg4
@pugazhendhi-m

Copy link
Copy Markdown
Contributor Author

Heads-up on commit aab96f2: an earlier rebase had left claude-code/hooks/unbound.py with stale MCP helpers — _resolve_claude_code_session_connector had reverted to a broad newest-session scan (losing staging's cwd-scoped lookup, flagged by Codex), and _compute_script_hash/_hook_candidate_script/_HOOK_SCRIPT_* were missing while still being called (a latent runtime NameError on the MCP path). Rebuilt the file from staging and re-applied only the file_content changes, so it now differs from staging solely by this PR's feature. All 5 hooks are back to differing from staging by file_content only.

…stency

- cursor: only scan a command for file_content on shell execution, not MCP
  (gate on mcp_server is None); postToolUse inline content uses `or None` so an
  empty string falls back to a disk read.
- codex: PreToolUse now sets metadata['file_path'] alongside file_content
  (uniform with the other tools).
- augment: MCP PostToolUse exchange now attaches file_content when the MCP
  tool_input carries a file_path/path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GrSjwjtXsfxcBpANpkqQg4

@vigneshsubbiah16 vigneshsubbiah16 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛡️ Automated Security Review (consensus)

2 findings — 1 high-confidence, 1 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.


🔴 HIGH — Cursor MCP pre-hook token-scans the MCP server id as a shell command

cursor/unbound.py:1110

Impact: process_pre_tool_use_execution calls _attach_command_file_content whenever event.get('command') is set, but the same helper serves beforeMCPExecution, where command holds the MCP server identifier—not a shell string. Tokens from that id can resolve to unrelated workspace files and attach their contents to MCP pre-tool telemetry.

Fix: Gate _attach_command_file_content on the shell path only (e.g. mcp_server is None / explicit Bash hook), matching the tool_name == 'Bash' gating already applied in claude-code and codex.

Flagged by: Cursor, Claude


🟡 TRIAGE — Copilot POST command attach is not Bash-gated

copilot/hooks/unbound.py:1453

Impact: map_copilot_tool calls _attach_command_file_content for any entry with a command field, not only Bash. Non-shell tools that carry a command argument could have workspace files token-scraped into Stop telemetry (same class of issue fixed for claude-code/codex PostToolUse in d426296).

Fix: Restrict the elif entry.get('command'): branch to Bash/canonical-shell tools only.

Flagged by: Lead


Previously acknowledged (not re-flagged)

  • Sensitive file contents ship to gateway before file_content DLP redaction — intentional for DLP visibility; gateway-side redaction is the tracked follow-up; final code/tests deliberately read .env, keys, and credentials (only /proc//sys excluded on-device).
  • Bash token over-capture (git add .env, etc.) — intentional; captures files referenced by commands, not only those read by them.
  • Silent exception swallowing on the file-read path — intentional fail-open; no logging/Sentry for expected skips.
  • Symlink /proc bypass, 128KB total cap overshoot, PostToolUse non-Bash command attach — fixed in d426296.
  • Relative path resolved against hook process cwd — fixed in ea36efe.
  • Bash content re-read at exchange assembly; Codex Stop single cwd for transcript replay — accepted best-effort limitations.
  • Cursor Delete postToolUse file_content; Copilot PRE file_text; Copilot apply_patch PRE paths — fixed, intentional out-of-scope, or graceful no-op per maintainer.
  • Edit inline prefers post-edit disk read over new_string — intentional.
  • Hardlink-to-secret residual — known/accepted low risk.
  • Semgrep chmod / SQL findings — pre-existing code outside this PR's file_content changes; not introduced by this diff.
  • Greptile on-device *.env/credential guard — superseded by the full-scan design codified in test_only_proc_sys_excluded_other_files_read; do not treat the reverted 510500c on-device guard as present in the merge commit.

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head aab96f23 · 2026-07-06T15:13Z

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 3111950. Configure here.

Comment thread copilot/hooks/unbound.py
_attach_file_content(metadata, file_path, event.get('cwd'),
tool_input.get('content') or tool_input.get('file_text'))
elif canonical == 'Bash' and tool_input.get('command'):
_attach_command_file_content(metadata, tool_input.get('command'), event.get('cwd'))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PRE Bash misses alternate keys

Medium Severity

Copilot PreToolUse only attaches file_content for Bash when tool_input has a command field, but the same handler already resolves the shell line via extract_command_for_pretool, which also reads input and text (e.g. send_to_terminal). Those runs send policy metadata without referenced-file attachments, while PostToolUse still attaches via the full command string.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3111950. Configure here.

@vigneshsubbiah16 vigneshsubbiah16 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛡️ Automated Security Review (consensus)

1 finding — 1 high-confidence, 0 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.


🔴 HIGH — Sensitive credential files are read and shipped in file_content (no on-device exclusion)

augment/hooks/unbound.py:2072 (byte-identical in all 5 hooks: claude-code/hooks/unbound.py, codex/hooks/unbound.py, copilot/hooks/unbound.py, cursor/unbound.py)

Impact: _is_excluded_path blocks only /proc and /sys; .env, ~/.ssh/id_rsa, .aws/credentials, *.pem, etc. are deliberately read and attached (up to 64 KB each) to pre/post telemetry — including via Bash token scanning (git add .env, chmod 600 ~/.ssh/id_rsa). Inline content (Read/Write responses) is gated the same way, so secrets in tool_input.content also ship. Until gateway DLP redacts file_content, credential plaintext is stored server-side.

Fix: Either restore the on-device sensitive-path guard described in the 510500c/ea36efe thread (_SENSITIVE_FILE_* lists, dir segments, *.env suffix/infix, inline-path enforcement in _make_file_entry) across all five hooks, or post an explicit PR comment retracting those claims so reviewers know only /proc//sys are blocked and gateway DLP is the sole mitigation. If full-file scanning for DLP is intentional (as test_only_proc_sys_excluded_other_files_read now asserts), document that clearly and confirm gateway DLP is deployed before fleet rollout.

Flagged by: Claude, Lead


Previously acknowledged (not re-flagged)

  • Bash token scraping attaches files the command never reads — intentional design; referenced paths (e.g. git add a.txt) are meant to attach (@pugazhendhi-m).
  • Silent exception swallowing on the file-read path — intentional fail-open; no logging/Sentry on expected skips (@pugazhendhi-m).
  • /proc//sys symlink bypass — fixed in d426296 via os.path.realpath (@pugazhendhi-m).
  • Codex Stop replay uses a single cwd — best-effort; transcript lacks per-call cwd, wrong-relative now skips rather than misreads (@pugazhendhi-m).
  • Bash file_content read at exchange-assembly time — known limitation; reflects post-turn disk state (@pugazhendhi-m).
  • apply_patch PRE paths / Copilot file_text PRE inline — patch-body parsing out of scope; file_text fixed in 1b6fce2 (@pugazhendhi-m).
  • Semgrep insecure-file-permissions / sqlalchemy-execute-raw-query hits — pre-existing code outside this diff's changes; not introduced by WEB-5004.

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head 3111950c · 2026-07-06T15:24Z

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants