fix(hooks): add bash-file-read-guard to warn agents against cat/head/tail (fixes #2096)#2771
fix(hooks): add bash-file-read-guard to warn agents against cat/head/tail (fixes #2096)#2771MoerAI wants to merge 1 commit intocode-yeongyu:devfrom
Conversation
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f801f6c28
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const WARNING_MESSAGE = "Prefer the Read tool over `cat`/`head`/`tail` for reading file contents. The Read tool provides line numbers and hash anchors for precise editing." | ||
|
|
||
| const FILE_READ_PATTERNS = [ | ||
| /^\s*cat\s+(?!-)[^\s|&;]+\s*$/, |
There was a problem hiding this comment.
Warn on numbered
cat reads too
The cat pattern on this line rejects every flag via (?!-), so a valid single-file read like cat -n src/file.ts or cat --number src/file.ts never triggers the warning. I checked cat --help, which documents -n, --number, and that form is especially relevant here because agents often use it to approximate line-numbered reads. As written, the new guard still misses one of the most useful cat variants for bypassing Read.
Useful? React with 👍 / 👎.
| /^\s*head\s+(-n\s+\d+\s+)?(?!-)[^\s|&;]+\s*$/, | ||
| /^\s*tail\s+(-n\s+\d+\s+)?(?!-)[^\s|&;]+\s*$/, |
There was a problem hiding this comment.
Accept compact
head/tail count syntax
These regexes only recognize the spaced -n 20 file form, so valid commands like head -20 src/file.ts, head -n20 src/file.ts, tail -50 src/file.ts, or tail -n50 src/file.ts slip through without any warning. I checked head --help and tail --help; both accept --lines=[-+]NUM, which covers the compact forms. That leaves a common way of reading file prefixes/suffixes via Bash completely unguarded.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
2 issues found across 5 files
Confidence score: 3/5
- There is moderate regression risk in
src/hooks/bash-file-read-guard.ts: thehead/tailregex only accepts spaced-n <num>forms and misses common valid variants like-n200or-200, which can cause legitimate commands to be misclassified or skipped. - The
catpattern’s(?!-)lookahead appears to block all flags, socat -n/cat --numbercan bypass the guard behavior; with high confidence, this is a concrete functional gap in command filtering. - Given two medium-severity issues concentrated in core guard logic, this is not a merge-blocker but carries noticeable behavior risk until patterns are tightened.
- Pay close attention to
src/hooks/bash-file-read-guard.ts- flag parsing and lookahead logic may allow unintended bypasses and miss valid command forms.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/hooks/bash-file-read-guard.ts">
<violation number="1" location="src/hooks/bash-file-read-guard.ts:8">
P2: The `(?!-)` negative lookahead rejects every flag, so `cat -n src/file.ts` and `cat --number src/file.ts` silently bypass the guard. Since `cat -n` is used by agents specifically to get line-numbered output (approximating the Read tool), it should trigger the warning. Consider allowing known display flags like `-n`/`--number`/`-A` before the filename.</violation>
<violation number="2" location="src/hooks/bash-file-read-guard.ts:9">
P2: Regex patterns for `head` and `tail` miss common valid flag variations like `-n200` or `-200`. The pattern `(-n\s+\d+\s+)?` requires a space between `-n` and the number and expects explicit `-n`, missing valid syntax like `head -n10 file` or `head -10 file`. This allows some file read commands to bypass the guard.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| const FILE_READ_PATTERNS = [ | ||
| /^\s*cat\s+(?!-)[^\s|&;]+\s*$/, | ||
| /^\s*head\s+(-n\s+\d+\s+)?(?!-)[^\s|&;]+\s*$/, |
There was a problem hiding this comment.
P2: Regex patterns for head and tail miss common valid flag variations like -n200 or -200. The pattern (-n\s+\d+\s+)? requires a space between -n and the number and expects explicit -n, missing valid syntax like head -n10 file or head -10 file. This allows some file read commands to bypass the guard.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/bash-file-read-guard.ts, line 9:
<comment>Regex patterns for `head` and `tail` miss common valid flag variations like `-n200` or `-200`. The pattern `(-n\s+\d+\s+)?` requires a space between `-n` and the number and expects explicit `-n`, missing valid syntax like `head -n10 file` or `head -10 file`. This allows some file read commands to bypass the guard.</comment>
<file context>
@@ -0,0 +1,44 @@
+
+const FILE_READ_PATTERNS = [
+ /^\s*cat\s+(?!-)[^\s|&;]+\s*$/,
+ /^\s*head\s+(-n\s+\d+\s+)?(?!-)[^\s|&;]+\s*$/,
+ /^\s*tail\s+(-n\s+\d+\s+)?(?!-)[^\s|&;]+\s*$/,
+]
</file context>
| const WARNING_MESSAGE = "Prefer the Read tool over `cat`/`head`/`tail` for reading file contents. The Read tool provides line numbers and hash anchors for precise editing." | ||
|
|
||
| const FILE_READ_PATTERNS = [ | ||
| /^\s*cat\s+(?!-)[^\s|&;]+\s*$/, |
There was a problem hiding this comment.
P2: The (?!-) negative lookahead rejects every flag, so cat -n src/file.ts and cat --number src/file.ts silently bypass the guard. Since cat -n is used by agents specifically to get line-numbered output (approximating the Read tool), it should trigger the warning. Consider allowing known display flags like -n/--number/-A before the filename.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/bash-file-read-guard.ts, line 8:
<comment>The `(?!-)` negative lookahead rejects every flag, so `cat -n src/file.ts` and `cat --number src/file.ts` silently bypass the guard. Since `cat -n` is used by agents specifically to get line-numbered output (approximating the Read tool), it should trigger the warning. Consider allowing known display flags like `-n`/`--number`/`-A` before the filename.</comment>
<file context>
@@ -0,0 +1,44 @@
+const WARNING_MESSAGE = "Prefer the Read tool over `cat`/`head`/`tail` for reading file contents. The Read tool provides line numbers and hash anchors for precise editing."
+
+const FILE_READ_PATTERNS = [
+ /^\s*cat\s+(?!-)[^\s|&;]+\s*$/,
+ /^\s*head\s+(-n\s+\d+\s+)?(?!-)[^\s|&;]+\s*$/,
+ /^\s*tail\s+(-n\s+\d+\s+)?(?!-)[^\s|&;]+\s*$/,
</file context>
|
Re: issues identified by cubic. P2 (head/tail regex misses -n200 and -200 forms): Valid edge case. The regex currently matches P2 (cat (?!-) blocks all flags, cat -n bypasses): This is intentional. The guard targets simple file-reading patterns ( |
Summary
cat/head/tailfor file reading instead of the built-in Read toolProblem
Agents (particularly Gemini models) frequently use bash commands like
cat src/file.ts,head -n 200 src/file.tsto read files instead of the Read tool. This bypasses the hash-anchored line IDs that the Read tool provides, leading to less precise edits and missed context.Fix
Created a new
bash-file-read-guardhook (tool.execute.before) that:cat <file>,head [-n N] <file>,tail [-n N] <file>)cat file | grep pattern) which have legitimate usesdisabled_hooks: ["bash-file-read-guard"]Changes
src/hooks/bash-file-read-guard.tssrc/hooks/index.tssrc/plugin/hooks/create-tool-guard-hooks.tssrc/config/schema/hooks.tssrc/plugin/tool-execute-before.tsFixes #2096
Summary by cubic
Add a
bash-file-read-guardhook that warns oncat/head/tailfile reads and guides agents to use the Read tool for hash-anchored line IDs, improving edit precision without blocking commands. Fixes #2096.cat/head/tailfile reads and warns to use the Read tool.tool.execute.before; added to the hook schema and exports.disabled_hooks: ["bash-file-read-guard"].Written for commit 2f801f6. Summary will update on new commits.