fix(core): deflake the shell-registry fixtures, and share the display-strip helper

packages/core/src/services/backgroundShellRegistry.test.ts

@@ -347,6 +347,70 @@ describe('BackgroundShellRegistry', () => {
       expect(modelText).not.toContain(command);
     });
 
+    it('strips BIDI OVERRIDES from the OUTPUT TAIL — the biggest field', () => {
+      // The sibling test below asserts modelText-wide absence, which reads
+      // as whole-envelope coverage but is not: its fixture shell has no
+      // output file, so <output-tail> renders the canned unreadable form
+      // and is never exercised. The tail is the LARGEST
+      // attacker-controllable field — up to 8 KiB of a background shell's
+      // own output — and it renders through a different helper, which
+      // stripped C0/C1 but passed bidi overrides through verbatim
+      // (probe-verified). Newlines must survive the fix.
+      const reg = new BackgroundShellRegistry();
+      const callback = vi.fn();
+      reg.setNotificationCallback(callback);
+      // ALL NINE codepoints of both stripped ranges: pinning one per
+      // range let a one-character bound typo (0x202a→0x202b,
+      // 0x2066→0x2067) ship green (probe-verified).
+      const outputPath = makeOutputFile(
+        'line one\nharmless\u202A\u202B\u202C\u202D\u202Eevil\u2066\u2067\u2068\u2069 two\n',
+      );
+      reg.register(makeEntry({ shellId: 'tail-bidi', outputPath }));
+
+      reg.complete('tail-bidi', 0, 2000);
+
+      const [, modelText] = callback.mock.calls[0];
+      expect(modelText).toContain('<output-tail');
+      const bidi = '\u202A\u202B\u202C\u202D\u202E\u2066\u2067\u2068\u2069';
+      for (const ch of bidi) {
+        expect(modelText).not.toContain(ch);
+      }
+      // The tail keeps its line structure AND the text after the stripped
+      // characters survives — the strip must not eat \n or truncate at
+      // the first bidi marker.
+      expect(modelText).toContain('line one\nharmlessevil two</output-tail>');
+    });
+
+    it('strips BIDI OVERRIDES from the notification, not just C0/C1', () => {
+      // The shared helper this renders through removes U+202A-202E and
+      // U+2066-2069 as well as C0/C1 — the registry's own former copy did
+      // not. Those characters reorder how a path DISPLAYS without changing
+      // its bytes, so `/tmp/a<RLO>evil<PDI>/out.log` can render as
+      // something else entirely in a model-facing envelope. This pins the
+      // stronger behaviour that came with the shared helper.
+      const reg = new BackgroundShellRegistry();
+      const callback = vi.fn();
+      reg.setNotificationCallback(callback);
+      const outputPath = join(makeTempDir(), 'a\u202Eevil\u2069.log');
+      reg.register(
+        makeEntry({
+          shellId: 'bidi',
+          // command and cwd render through the same shared helper — pin
+          // them too, so a field-local bidi-blind sanitizer fails here.
+          command: 'cat \u202Efd\u2069.txt',
+          cwd: '/repo/\u202Efd\u2069',
+          outputPath,
+        }),
+      );
+
+      reg.complete('bidi', 0, 2000);
+
+      const [, modelText] = callback.mock.calls[0];
+      expect(modelText).toContain(expectedOutputFileElement(outputPath));
+      expect(modelText).not.toContain('\u202E');
+      expect(modelText).not.toContain('\u2069');
+    });
+
     it('escapes XML and strips display control characters on failure', () => {
       const reg = new BackgroundShellRegistry();
       const callback = vi.fn();
@@ -361,7 +425,7 @@ describe('BackgroundShellRegistry', () => {
         }),
       );
 
-      reg.fail('a&b', 'bad <thing>\x1B[31m', 2000);
+      reg.fail('a&b', 'bad <thing>\x1B[31m \u202Eevil\u2069', 2000);
 
       const [displayText, modelText] = callback.mock.calls[0];
       expect(displayText).toBe('Background shell "echo "<script>"" failed.');
@@ -370,7 +434,13 @@ describe('BackgroundShellRegistry', () => {
         '<command>echo &quot;&lt;script&gt;&quot;</command>',
       );
       expect(modelText).toContain('<cwd>/repo&amp;work</cwd>');
-      expect(modelText).toContain('<result>bad &lt;thing&gt;[31m</result>');
+      expect(modelText).toContain(
+        '<result>bad &lt;thing&gt;[31m evil</result>',
+      );
+      // The bidi pair in the fixture pins the fourth render site: <result>
+      // is the failed shell's error string and renders only on this path.
+      expect(modelText).not.toContain('\u202E');
+      expect(modelText).not.toContain('\u2069');
       // Assert the whole element, not just the tail: the temp prefix is
       // random but the escaping is what this test is about.
       expect(modelText).toContain(expectedOutputFileElement(outputPath));

packages/core/src/services/backgroundShellRegistry.ts

@@ -24,7 +24,10 @@ import type { TaskBase, TaskRegistration } from '../agents/tasks/types.js';
 import { atomicWriteFileSync } from '../utils/atomicFileWrite.js';
 import { createDebugLogger } from '../utils/debugLogger.js';
 import { todoWorkChainContext } from '../utils/promptIdContext.js';
-import { stripDisplayControlChars } from '../utils/terminalSafe.js';
+import {
+  isBidiControlChar,
+  stripDisplayControlChars,
+} from '../utils/terminalSafe.js';
 import { escapeXml } from '../utils/xml.js';
 
 const debugLogger = createDebugLogger('BACKGROUND_SHELLS');
@@ -42,6 +45,9 @@ function stripOutputControlChars(text: string): string {
     }
     if (code < 0x20) continue;
     if (code >= 0x80 && code <= 0x9f) continue;
+    // Same bidi set as the shared display helper, in its own loop only
+    // because the tail must keep \n and \r, which that helper strips.
+    if (isBidiControlChar(code)) continue;
     out += text[i];
   }
   return out;

packages/core/src/services/monitorRegistry.test.ts

@@ -663,6 +663,30 @@ describe('MonitorRegistry', () => {
     expect(modelText).not.toContain('\u0085');
   });
 
+  it('strips BIDI overrides from the streaming event <result>, not just the display line', () => {
+    // The display line is built from `safeEventLine`, but the model-facing
+    // <result> used to render from the raw event line — the one path that
+    // carries live untrusted process output (server logs, third-party
+    // stdout) to the model. Pin all nine codepoints of both stripped
+    // ranges so a one-character bound typo ships red.
+    const callback = vi.fn();
+    registry.setNotificationCallback(callback);
+    registry.register(createEntry());
+
+    registry.emitEvent(
+      'mon-1',
+      '/tmp/a\u202A\u202B\u202C\u202D\u202Eevil\u2066\u2067\u2068\u2069/out.log',
+    );
+
+    const [displayText, modelText] = callback.mock.calls[0] as [string, string];
+    expect(modelText).toContain('<result>/tmp/aevil/out.log</result>');
+    const bidi = '\u202A\u202B\u202C\u202D\u202E\u2066\u2067\u2068\u2069';
+    for (const ch of bidi) {
+      expect(modelText).not.toContain(ch);
+      expect(displayText).not.toContain(ch);
+    }
+  });
+
   it('propagates toolUseId in notification XML and meta', () => {
     const callback = vi.fn();
     registry.setNotificationCallback(callback);

packages/core/src/services/monitorRegistry.ts

@@ -517,7 +517,7 @@ export class MonitorRegistry {
       '<status>running</status>',
       `<event-count>${entry.eventCount}</event-count>`,
       `<summary>Monitor "${escapeXml(desc)}" emitted event #${entry.eventCount}.</summary>`,
-      `<result>${escapeXml(eventLine)}</result>`,
+      `<result>${escapeXml(safeEventLine)}</result>`,
       '</task-notification>',
     );
 

packages/core/src/utils/terminalSafe.ts

@@ -52,6 +52,20 @@ export function stripTerminalControlSequences(s: string): string {
   );
 }
 
+/**
+ * Whether a UTF-16 code unit is a Unicode bidirectional override /
+ * isolate codepoint: `\u202a-\u202e` (LRE / RLE / PDF / LRO / RLO) or
+ * `\u2066-\u2069` (LRI / RLI / FSI / PDI). These reorder how adjacent
+ * text renders without changing a byte — the "Trojan Source" class
+ * (CVE-2021-42574). The range set lives here so every display sanitizer
+ * strips from one home.
+ */
+export function isBidiControlChar(code: number): boolean {
+  return (
+    (code >= 0x202a && code <= 0x202e) || (code >= 0x2066 && code <= 0x2069)
+  );
+}
+
 /**
  * Strip C0 control characters (except TAB), C1 control characters, and
  * Unicode bidirectional override / isolate characters from a string
@@ -90,8 +104,7 @@ export function stripDisplayControlChars(text: string): string {
     }
     if (code < 0x20) continue;
     if (code >= 0x80 && code <= 0x9f) continue;
-    if (code >= 0x202a && code <= 0x202e) continue;
-    if (code >= 0x2066 && code <= 0x2069) continue;
+    if (isBidiControlChar(code)) continue;
     out += text[i];
   }
   return out;

packages/core/src/utils/xml.test.ts

@@ -10,8 +10,14 @@ import { escapeSystemReminderTags, escapeXml } from './xml.js';
 describe('xml utils', () => {
   describe('escapeXml', () => {
     it('escapes XML metacharacters for element and attribute contexts', () => {
-      expect(escapeXml(`a&b <tag attr="x">'y'</tag>`)).toBe(
-        'a&amp;b &lt;tag attr=&quot;x&quot;&gt;&apos;y&apos;&lt;/tag&gt;',
+      // TWO of each metacharacter: with a single `&`, a
+      // `.replace(/&/g, …)` → `.replace('&', …)` mutation shipped green
+      // across the whole package (measured: 19,546 tests passed), and a
+      // path holding two — a TMPDIR under `o&brien` with a basename like
+      // `out&err.log` — would then put a raw `&` into a model-facing XML
+      // envelope. The other four were already pinned globally; `&` was not.
+      expect(escapeXml(`a&b&c <tag attr="x">'y'</tag>`)).toBe(
+        'a&amp;b&amp;c &lt;tag attr=&quot;x&quot;&gt;&apos;y&apos;&lt;/tag&gt;',
       );
     });
   });