fix(#24): redact marker must have no shell metacharacters

src/sanitize.js

@@ -31,18 +31,30 @@ const _repoRoot = (() => {
   } catch { return '/root/WindsurfAPI'; }
 })();
 
-// Placeholder chosen as a parenthesised natural-language phrase because
-// every punctuation-only marker we tried has been re-used by the model
-// as a real path in later turns:
-//   ./tail            → LLM Reads ./src/main.py → ENOENT → loops
-//   [internal]        → LLM runs `ls [internal]` → ENOENT → loops
-//   <redacted-path>   → LLM passes it as file_path arg to Read/Bash →
-//                       ENOENT (Linux) or Errno 22 on Windows → loops
-// A multi-word parenthesised phrase cannot be tokenised as a path or
-// identifier, so clients skip it instead of trying to resolve. Verified
-// with the drift probe (scripts/_agent_drift_probe.py) that previously
-// crashed because sonnet kept calling read_file('<redacted-path>').
-const REDACTED_PATH = '(internal path redacted)';
+// Placeholder chosen as a plain-ASCII, multi-word phrase with NO shell
+// metacharacters. Every previous marker broke at least one consumer:
+//   ./tail                    → LLM Reads ./src/main.py → ENOENT → loops
+//   [internal]                → LLM runs `ls [internal]` → ENOENT → loops
+//   <redacted-path>           → LLM passes to Read/Bash → ENOENT (Linux) /
+//                               Errno 22 (Windows) → loops
+//   (internal path redacted)  → zsh parses `cd (internal path redacted)`
+//                               as glob-qualifier syntax → cryptic
+//                               "unknown file attribute: i" error → Opus
+//                               gets confused and stops calling tools
+// The marker must satisfy three constraints at once:
+//   1. Contains no character that any mainstream shell parses specially
+//      — excludes `( ) [ ] { } < > | & ; $ \` " ' \\ * ?` and whitespace
+//      inside a paired delimiter.
+//   2. Does not look like a path or identifier so the model does not
+//      reuse it as a file_path / cd target on later turns.
+//   3. Reads as descriptive prose when it appears in sanitized output.
+// A plain multi-word phrase with a trailing period satisfies all three:
+// if a client ever does embed it in shell, the words become separate
+// argv entries and `cd` / `Read` fails with a clean, recoverable error
+// (too many arguments / ENOENT once) instead of the cryptic zsh glob
+// qualifier error. Verified with the drift probe
+// (scripts/_agent_drift_probe.py).
+const REDACTED_PATH = 'redacted internal path';
 
 const PATTERNS = [
   [/\/tmp\/windsurf-workspace(?:\/[^\s"'`<>)}\],*;]*)?/g, REDACTED_PATH],

test/sanitize.test.js

@@ -2,29 +2,30 @@ import { describe, it } from 'node:test';
 import assert from 'node:assert/strict';
 import { sanitizeText, PathSanitizeStream, sanitizeToolCall } from '../src/sanitize.js';
 
-// Leaked Windsurf paths are redacted to the angle-bracketed marker
-// `(internal path redacted)`. Shell / file APIs won't try to resolve that string,
-// and downstream LLMs don't tokenize it as a path (see sanitize.js header
-// for the history — ./tail and [internal] both caused read-loop regressions).
+// Leaked Windsurf paths are redacted to the multi-word prose marker
+// `redacted internal path`. The marker MUST contain no shell metacharacter
+// — earlier `(internal path redacted)` broke zsh (glob-qualifier syntax on
+// `(…)` → `unknown file attribute: i`). See sanitize.js header for the
+// full history of markers that regressed.
 
 describe('sanitizeText', () => {
   it('redacts /tmp/windsurf-workspace paths', () => {
-    assert.equal(sanitizeText('/tmp/windsurf-workspace/src/index.js'), '(internal path redacted)');
+    assert.equal(sanitizeText('/tmp/windsurf-workspace/src/index.js'), 'redacted internal path');
   });
 
   it('redacts bare /tmp/windsurf-workspace', () => {
-    assert.equal(sanitizeText('/tmp/windsurf-workspace'), '(internal path redacted)');
+    assert.equal(sanitizeText('/tmp/windsurf-workspace'), 'redacted internal path');
   });
 
   it('redacts per-account workspace paths', () => {
     assert.equal(
       sanitizeText('/home/user/projects/workspace-abc12345/package.json'),
-      '(internal path redacted)'
+      'redacted internal path'
     );
   });
 
   it('redacts /opt/windsurf', () => {
-    assert.equal(sanitizeText('/opt/windsurf/language_server'), '(internal path redacted)');
+    assert.equal(sanitizeText('/opt/windsurf/language_server'), 'redacted internal path');
   });
 
   it('leaves normal text unchanged', () => {
@@ -35,7 +36,7 @@ describe('sanitizeText', () => {
   it('handles multiple patterns in one string', () => {
     const input = 'Editing /tmp/windsurf-workspace/a.js and /opt/windsurf/bin';
     const result = sanitizeText(input);
-    assert.equal(result, 'Editing (internal path redacted) and (internal path redacted)');
+    assert.equal(result, 'Editing redacted internal path and redacted internal path');
   });
 
   it('returns non-strings unchanged', () => {
@@ -50,7 +51,7 @@ describe('PathSanitizeStream', () => {
     const stream = new PathSanitizeStream();
     const out = stream.feed('/tmp/windsurf-workspace/file.js is here');
     const rest = stream.flush();
-    assert.equal(out + rest, '(internal path redacted) is here');
+    assert.equal(out + rest, 'redacted internal path is here');
   });
 
   it('handles path split across chunks', () => {
@@ -59,7 +60,7 @@ describe('PathSanitizeStream', () => {
     result += stream.feed('Look at /tmp/windsurf');
     result += stream.feed('-workspace/config.yaml for details');
     result += stream.flush();
-    assert.equal(result, 'Look at (internal path redacted) for details');
+    assert.equal(result, 'Look at redacted internal path for details');
   });
 
   it('handles partial prefix at buffer end', () => {
@@ -68,7 +69,7 @@ describe('PathSanitizeStream', () => {
     result += stream.feed('path is /tmp/win');
     result += stream.feed('dsurf-workspace/x.js done');
     result += stream.flush();
-    assert.equal(result, 'path is (internal path redacted) done');
+    assert.equal(result, 'path is redacted internal path done');
   });
 
   it('flushes clean text immediately', () => {
@@ -82,17 +83,39 @@ describe('sanitizeToolCall', () => {
   it('sanitizes argumentsJson paths', () => {
     const tc = { name: 'Read', argumentsJson: '{"path":"/tmp/windsurf-workspace/f.js"}' };
     const result = sanitizeToolCall(tc);
-    assert.equal(result.argumentsJson, '{"path":"(internal path redacted)"}');
+    assert.equal(result.argumentsJson, '{"path":"redacted internal path"}');
   });
 
   it('sanitizes input object string values', () => {
     const tc = { name: 'Read', input: { file_path: '/home/user/projects/workspace-abc12345/src/x.ts' } };
     const result = sanitizeToolCall(tc);
-    assert.equal(result.input.file_path, '(internal path redacted)');
+    assert.equal(result.input.file_path, 'redacted internal path');
   });
 
   it('returns null/undefined unchanged', () => {
     assert.equal(sanitizeToolCall(null), null);
     assert.equal(sanitizeToolCall(undefined), undefined);
   });
 });
+
+describe('REDACTED_PATH marker shape (shell-safety regression)', () => {
+  // The marker is emitted verbatim into model-facing text. Models
+  // sometimes echo it back inside a shell command (e.g. `cd <marker>`).
+  // If the marker contains any character the shell parses specially, the
+  // resulting command fails with a cryptic error instead of a clean
+  // ENOENT / too-many-arguments, and the model derails (issue: zsh
+  // `unknown file attribute: i` after parens redaction).
+  const marker = sanitizeText('/tmp/windsurf-workspace');
+
+  it('contains no shell metacharacters', () => {
+    const banned = /[()\[\]{}<>|&;$`\\"'*?]/;
+    assert.ok(!banned.test(marker), `marker must not contain shell metachars: got ${JSON.stringify(marker)}`);
+  });
+
+  it('is not shaped like a path or identifier (multi-word, no slashes, has spaces)', () => {
+    assert.ok(!marker.includes('/'), 'marker must not contain / (looks like a path)');
+    assert.ok(!marker.includes('\\'), 'marker must not contain \\ (looks like a Windows path)');
+    assert.ok(marker.includes(' '), 'marker must contain whitespace so it cannot be a single identifier / file arg');
+    assert.ok(marker.split(/\s+/).filter(Boolean).length >= 2, 'marker must be multi-word');
+  });
+});