mirror of
https://github.com/thedotmack/claude-mem.git
synced 2026-09-20 04:23:02 +08:00
Alex Newman
f03d918a47
`extractFromBash` calls shell-quote's `parse()` on every Bash PreToolUse
command to enrich the payload with `filePaths` for read commands. `parse()`
throws on substitutions it cannot resolve, the throw was not caught here, and
it reached the generic hook handler — which logs `Hook error:` and returns
BLOCKING_ERROR. A best-effort convenience field was turning an ordinary shell
command into a blocked tool call.
Reproduced against the version actually installed here, not just the 1.8.4 in
the report:
shell-quote 1.10.0
parse("cat ${}") -> throws Bad substitution: ${}
parse("echo hi && cat ${}") -> throws Bad substitution: ${}
The trigger is an *empty* substitution. `${FOO}`, `${#FOO}`, `${FOO:?err}` and
`${arr[@]}` all parse fine on 1.10.0, which is why the form matters: the guard
is needed on current dependencies, not only on the older one.
It also matters that `plugin/scripts/worker-service.cjs` — the file that ships —
carries a bundled copy containing the same `Bad substitution` throw, so the
behaviour does not depend on what a fresh `npm install` resolves.
The parse is now guarded and yields no paths on failure. Failing to enrich is
invisible; blocking the user's command is not.
Three tests, two of which fail without the guard (7 pass / 2 fail):
- an unparseable substitution yields [] instead of throwing
- the same when it rides alongside a genuinely readable file, so the failure
is attributable to tokenisation rather than to a bad path
- a command that parses still extracts its paths — the guard must not
swallow the feature it protects
`bun test tests/cli tests/hooks` — 116 passed.
The committed bundle is left alone: rebuilding it here diverges from the
committed artifact for unrelated reasons, so it should be regenerated by the
normal release build rather than by this PR.
Co-authored-by: Nguyen Thanh Dat <ntdat812@gmail.com>
86 lines
3.2 KiB
TypeScript
86 lines
3.2 KiB
TypeScript
import { afterEach, beforeEach, describe, expect, it } from 'bun:test';
|
|
import { mkdtempSync, rmSync, writeFileSync } from 'fs';
|
|
import { tmpdir } from 'os';
|
|
import { join } from 'path';
|
|
|
|
import { extractFilePaths } from '../../../src/cli/adapters/codex-file-context.js';
|
|
|
|
let tmpDir: string;
|
|
|
|
beforeEach(() => {
|
|
tmpDir = mkdtempSync(join(tmpdir(), 'codex-file-context-'));
|
|
writeFileSync(join(tmpDir, 'README.md'), 'readme');
|
|
writeFileSync(join(tmpDir, 'src.ts'), 'source');
|
|
writeFileSync(join(tmpDir, 'notes.txt'), 'notes');
|
|
});
|
|
|
|
afterEach(() => {
|
|
rmSync(tmpDir, { recursive: true, force: true });
|
|
});
|
|
|
|
describe('extractFilePaths', () => {
|
|
it('extracts existing files from Codex Bash read commands', () => {
|
|
const paths = extractFilePaths('Bash', {
|
|
command: 'cat README.md && head -n 20 src.ts && cat missing.md',
|
|
}, tmpDir);
|
|
|
|
expect(paths).toEqual(['README.md', 'src.ts']);
|
|
});
|
|
|
|
it('does not consume cat boolean flags as file arguments', () => {
|
|
const paths = extractFilePaths('Bash', {
|
|
command: 'cat -n README.md',
|
|
}, tmpDir);
|
|
|
|
expect(paths).toEqual(['README.md']);
|
|
});
|
|
|
|
it('ignores non-read Bash commands', () => {
|
|
const paths = extractFilePaths('Bash', {
|
|
command: 'rm README.md; echo src.ts',
|
|
}, tmpDir);
|
|
|
|
expect(paths).toEqual([]);
|
|
});
|
|
|
|
it('extracts MCP read tool path arrays', () => {
|
|
const paths = extractFilePaths('mcp__local_filesystem__read_file', {
|
|
paths: ['README.md', 'notes.txt', 'missing.txt'],
|
|
}, tmpDir);
|
|
|
|
expect(paths).toEqual(['README.md', 'notes.txt']);
|
|
});
|
|
|
|
it('extracts MCP exact read/view/cat tool names', () => {
|
|
expect(extractFilePaths('mcp__fs__read', { path: 'README.md' }, tmpDir)).toEqual(['README.md']);
|
|
expect(extractFilePaths('mcp__fs__view_files', { paths: ['README.md'] }, tmpDir)).toEqual(['README.md']);
|
|
});
|
|
|
|
it('ignores MCP tool names that only contain read verbs as a prefix', () => {
|
|
expect(extractFilePaths('mcp__fs__read_write', { path: 'README.md' }, tmpDir)).toEqual([]);
|
|
expect(extractFilePaths('mcp__server__readonly', { path: 'README.md' }, tmpDir)).toEqual([]);
|
|
});
|
|
|
|
// #3688: `parse` throws "Bad substitution" on `${}`. The throw escaped this
|
|
// best-effort enrichment and reached the generic hook handler, which answers
|
|
// BLOCKING_ERROR — so an ordinary shell command was blocked and the tool call
|
|
// discarded, to add a convenience field.
|
|
it('yields no paths instead of throwing on an unparseable substitution', () => {
|
|
expect(() => extractFilePaths('Bash', { command: 'cat ${}' }, tmpDir)).not.toThrow();
|
|
expect(extractFilePaths('Bash', { command: 'cat ${}' }, tmpDir)).toEqual([]);
|
|
});
|
|
|
|
it('yields no paths when the unparseable part rides alongside a real read', () => {
|
|
// The readable file is genuinely there, so this fails only because the
|
|
// command as a whole cannot be tokenised — not because the path is bad.
|
|
expect(
|
|
extractFilePaths('Bash', { command: 'cat README.md && cat ${}' }, tmpDir)
|
|
).toEqual([]);
|
|
});
|
|
|
|
it('still extracts paths from a command that parses', () => {
|
|
// The guard must not swallow the feature it protects.
|
|
expect(extractFilePaths('Bash', { command: 'cat README.md' }, tmpDir)).toEqual(['README.md']);
|
|
});
|
|
});
|