From 4aef1d94b9d3ada046d568d30d77756af59353bc Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 19 Sep 2026 19:18:48 -0700 Subject: [PATCH 01/10] fix(security): approval surfaces show reordering and invisible characters as escapes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #1029, found in #1026's review. Trojan Source (CVE-2021-42574): Unicode bidi overrides and isolates reorder the glyphs a browser draws without changing the bytes a shell runs, so a prompt-injected model can propose a command that READS as harmless and RUNS as something else — the canonical demonstration makes a destructive command appear to sit inside a comment. Zero-width characters hide text outright, and a lone CR scrolls the dangerous half of a line out of a
. Nothing in src/
neutralised any of it, and the permission modal is frequently the only
place a command is ever shown, so what it renders IS the user's
evidence.

One helper, src/shared/text/visibleControls.ts, renders each such code
point as a visible marker (⟨U+202E RLO⟩). It ESCAPES rather than strips:
the approval must show what will actually run, and deleting the
characters would make the modal disagree with the command the provider
executes — a second, quieter lie. Its table names what each character
does, since "it is in a range" is not a reason a reader can check.

Applied to every approval surface: Claude's permission prompt and trust
dialog, Codex's approval modal and trust dialog, OpenCode's permission
subject and command, and Grok's permission title.

Tests: the helper's contract (the canonical attack string, isolates,
zero-width, a lone CR versus a real CRLF, and several non-Latin scripts
left untouched), plus the REAL OpenCode permission modal driven from its
recorded 1.18.30 ask with only the command replaced by the attack shape.
The modal test fails without the change.

Co-Authored-By: Claude Opus 5 (1M context) 
---
 .../claude/renderer/PermissionPromptModal.tsx |  3 +-
 .../claude/renderer/TrustDialogModal.tsx      |  3 +-
 .../codex/renderer/CodexApprovalModal.tsx     |  3 +-
 .../conditions/CodexTrustDialogModal.tsx      |  3 +-
 .../grok/renderer/conditions/views.tsx        |  3 +-
 .../opencodePermissionView.renderer.test.tsx  | 21 +++++
 .../opencode/renderer/conditions/views.tsx    |  5 +-
 src/shared/text/visibleControls.test.ts       | 42 ++++++++++
 src/shared/text/visibleControls.ts            | 84 +++++++++++++++++++
 9 files changed, 160 insertions(+), 7 deletions(-)
 create mode 100644 src/shared/text/visibleControls.test.ts
 create mode 100644 src/shared/text/visibleControls.ts

diff --git a/src/providers/claude/renderer/PermissionPromptModal.tsx b/src/providers/claude/renderer/PermissionPromptModal.tsx
index 2b40d8943..fa8448767 100644
--- a/src/providers/claude/renderer/PermissionPromptModal.tsx
+++ b/src/providers/claude/renderer/PermissionPromptModal.tsx
@@ -5,6 +5,7 @@ import {
   DialogDescription,
   DialogTitle,
 } from '@renderer/components/ui/dialog'
+import { withVisibleControls } from '@shared/text/visibleControls'
 
 type PermissionPromptState = {
   title?: string
@@ -62,7 +63,7 @@ export function PermissionPromptModal({ state, onSend }: Props) {
         
{state.command && (
-              {state.command}
+              {withVisibleControls(state.command)}
             
)} {state.options && state.options.length > 0 && ( diff --git a/src/providers/claude/renderer/TrustDialogModal.tsx b/src/providers/claude/renderer/TrustDialogModal.tsx index 68cba4e89..19caa6201 100644 --- a/src/providers/claude/renderer/TrustDialogModal.tsx +++ b/src/providers/claude/renderer/TrustDialogModal.tsx @@ -5,6 +5,7 @@ import { DialogDescription, DialogTitle, } from '@renderer/components/ui/dialog' +import { withVisibleControls } from '@shared/text/visibleControls' // WHY the modal takes intent callbacks instead of an onSend(bytes) writer: // this component used to write a bare '\r' for accept, assuming Claude Code @@ -48,7 +49,7 @@ export function TrustDialogModal({ state, onAccept, onDecline }: Props) {

Claude Code is about to access:

{state.workspace && (
-              {state.workspace}
+              {withVisibleControls(state.workspace)}
             
)}

diff --git a/src/providers/codex/renderer/CodexApprovalModal.tsx b/src/providers/codex/renderer/CodexApprovalModal.tsx index 94afb6350..5560c2370 100644 --- a/src/providers/codex/renderer/CodexApprovalModal.tsx +++ b/src/providers/codex/renderer/CodexApprovalModal.tsx @@ -1,4 +1,5 @@ import { useCallback, useEffect, useRef, useState } from 'react' +import { withVisibleControls } from '@shared/text/visibleControls' // CodexApprovalPane — inline approval prompt rendered inside the pane, // matching how Codex's TUI draws it in the bottom pane. @@ -146,7 +147,7 @@ export function CodexApprovalModal({ approval, onSend, interactionActive }: Prop {command && (

$ - {command} + {withVisibleControls(command)}
)} diff --git a/src/providers/codex/renderer/conditions/CodexTrustDialogModal.tsx b/src/providers/codex/renderer/conditions/CodexTrustDialogModal.tsx index f40e6a023..8a2af49d9 100644 --- a/src/providers/codex/renderer/conditions/CodexTrustDialogModal.tsx +++ b/src/providers/codex/renderer/conditions/CodexTrustDialogModal.tsx @@ -6,6 +6,7 @@ import { DialogDescription, DialogTitle, } from '@renderer/components/ui/dialog' +import { withVisibleControls } from '@shared/text/visibleControls' type Props = { state: { workspace?: string } | null @@ -60,7 +61,7 @@ export function CodexTrustDialogModal({ state, actions, dispatch }: Props) {

Codex is about to work in:

{state.workspace && (
-              {state.workspace}
+              {withVisibleControls(state.workspace)}
             
)}

diff --git a/src/providers/grok/renderer/conditions/views.tsx b/src/providers/grok/renderer/conditions/views.tsx index e0f18cd7d..b6e9bc822 100644 --- a/src/providers/grok/renderer/conditions/views.tsx +++ b/src/providers/grok/renderer/conditions/views.tsx @@ -31,6 +31,7 @@ import { DialogDescription, DialogTitle, } from '@renderer/components/ui/dialog' +import { withVisibleControls } from '@shared/text/visibleControls' // Per-provider kind→state binding: eraseRegistry checks the registry literal // against this, so filing a view under the wrong kind is a compile error. @@ -134,7 +135,7 @@ export const grokPermissionView = defineView< Grok is requesting permission {state.title ? ( <> - {' '}for {state.title} + {' '}for {withVisibleControls(state.title)} ) : null} . diff --git a/src/providers/opencode/renderer/conditions/opencodePermissionView.renderer.test.tsx b/src/providers/opencode/renderer/conditions/opencodePermissionView.renderer.test.tsx index 4079fea5f..640fa86fa 100644 --- a/src/providers/opencode/renderer/conditions/opencodePermissionView.renderer.test.tsx +++ b/src/providers/opencode/renderer/conditions/opencodePermissionView.renderer.test.tsx @@ -81,6 +81,27 @@ describe('opencode permission modal on a recorded 1.18.30 ask', () => { expect(subject.className).toMatch(/max-h-/) }) + it('renders a bidi override as a visible escape, so the command cannot lie about itself (#1029)', () => { + // Trojan Source, CVE-2021-42574: U+202E reorders the glyphs a browser + // draws without changing the bytes the shell runs, so a prompt-injected + // model can make a destructive command read as a harmless one. This modal + // is frequently the only place the command is shown, so what it renders + // IS the user's evidence. DERIVED from the recording: only the command + // changes, to the canonical attack shape. + const rec = recording() + const spoofed = 'rm -rf ~/work \u202E# this is fine\u202C' + for (const { event } of rec.sse) { + if (event.type === 'permission.asked') event.properties = { ...event.properties, metadata: { command: spoofed } } + } + mount(permissionStateFrom(rec)) + const rendered = screen.getByText((_, element) => element?.tagName === 'PRE' && (element.textContent ?? '').includes('rm -rf')) + expect(rendered.textContent).toContain('⟨U+202E RLO⟩') + expect(rendered.textContent).toContain('⟨U+202C PDF⟩') + // The override itself must not survive into the DOM, or the browser + // reorders the line exactly as the attack intends. + expect(rendered.textContent).not.toContain('\u202E') + }) + it('shows the command behind a default-permission external_directory ask, not just the directory', () => { // #1026 review: OpenCode's DEFAULT rules allow bash and ask only for // external_directory, so for most users this is THE shell-command diff --git a/src/providers/opencode/renderer/conditions/views.tsx b/src/providers/opencode/renderer/conditions/views.tsx index 84e19aeeb..f7098cc94 100644 --- a/src/providers/opencode/renderer/conditions/views.tsx +++ b/src/providers/opencode/renderer/conditions/views.tsx @@ -30,6 +30,7 @@ import { DialogDescription, DialogTitle, } from '@renderer/components/ui/dialog' +import { withVisibleControls } from '@shared/text/visibleControls' // Per-provider kind→state binding (see CodexStateByKind for the rationale // — eraseRegistry checks the registry literal against this, so filing a @@ -167,7 +168,7 @@ export const opencodePermissionView = defineView< `python3 -c` legible; the height cap keeps the buttons on screen. */}

-              {state.title}
+              {withVisibleControls(state.title)}
             
) : ( @@ -187,7 +188,7 @@ export const opencodePermissionView = defineView< <>

Command:

-                {command}
+                {withVisibleControls(command)}
               
) diff --git a/src/shared/text/visibleControls.test.ts b/src/shared/text/visibleControls.test.ts new file mode 100644 index 000000000..2994a14b6 --- /dev/null +++ b/src/shared/text/visibleControls.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it } from 'vitest' + +import { containsInvisibleControls, withVisibleControls } from './visibleControls' + +// #1029. The threat is Trojan Source (CVE-2021-42574): text that a browser +// draws in a different order than a shell executes. + +describe('visible controls in text a user is asked to approve', () => { + it('escapes the bidi override that makes a destructive command read as a comment', () => { + // The canonical attack: everything after the override is drawn + // right-to-left, so `rm -rf ~/work` appears to be inside the comment. + const spoofed = 'rm -rf ~/work ‮# this is fine‬' + const shown = withVisibleControls(spoofed) + expect(shown).toBe('rm -rf ~/work ⟨U+202E RLO⟩# this is fine⟨U+202C PDF⟩') + expect(shown).not.toContain('‮') + }) + + it('escapes isolates, marks and zero-width characters, which hide text rather than reorder it', () => { + expect(withVisibleControls('a⁦b⁩c')).toBe('a⟨U+2066 LRI⟩b⟨U+2069 PDI⟩c') + expect(withVisibleControls('git​push')).toBe('git⟨U+200B ZWSP⟩push') + expect(withVisibleControls('sudo')).toBe('⟨U+FEFF BOM⟩sudo') + }) + + it('escapes a lone carriage return, which scrolls the rest of a line out of view', () => { + expect(withVisibleControls('echo safe\rrm -rf /')).toBe('echo safe⟨U+000D CR⟩rm -rf /') + // A real line ending is not an attack. + expect(withVisibleControls('echo safe\r\nrm -rf /')).toBe('echo safe\r\nrm -rf /') + }) + + it('leaves ordinary text alone, including every non-Latin script', () => { + for (const text of ['npm run build', 'grep -R "café" .', 'echo "日本語"', 'echo "العربية"', 'tab\there\nnewline']) { + expect(withVisibleControls(text)).toBe(text) + expect(containsInvisibleControls(text)).toBe(false) + } + }) + + it('reports whether anything was hidden, for a caller that wants to warn', () => { + expect(containsInvisibleControls('rm -rf ~/work ‮#ok')).toBe(true) + expect(containsInvisibleControls('echo safe\rrm -rf /')).toBe(true) + expect(containsInvisibleControls('echo "safe"')).toBe(false) + }) +}) diff --git a/src/shared/text/visibleControls.ts b/src/shared/text/visibleControls.ts new file mode 100644 index 000000000..6c4ff8779 --- /dev/null +++ b/src/shared/text/visibleControls.ts @@ -0,0 +1,84 @@ +/** + * Make invisible and reordering Unicode visible, for any text a user is asked + * to APPROVE (#1029, found in #1026's review). + * + * THE ATTACK (Trojan Source, CVE-2021-42574): bidirectional overrides and + * isolates reorder the glyphs a browser draws without changing the bytes a + * shell runs, so a prompt-injected model can propose a command that READS as + * harmless and RUNS as something else — the canonical demonstration makes a + * destructive command appear to be inside a comment. Zero-width characters do + * the same job by hiding text outright, and a lone `\r` can scroll the + * dangerous half of a line out of view in a `
`. The permission modal is
+ * frequently the only place the command is ever shown, so what it renders IS
+ * the user's evidence.
+ *
+ * WHY escape rather than strip: the approval has to show what will actually
+ * run. Deleting the characters would make the modal disagree with the command
+ * the provider executes — a second, quieter lie. `⟨U+202E⟩` keeps the string
+ * faithful, makes the anomaly obvious, and stays copy-pasteable as a report.
+ *
+ * WHY not a regex over "suspicious ranges" at each call site: there are three
+ * separate families (bidi controls, isolates, invisible formatting) plus the
+ * deprecated ones, and any surface that forgot one would silently be the weak
+ * link. One table, one helper, every approval surface.
+ *
+ * NOT for transcript bodies or agent prose. Reordering in a model's answer is
+ * cosmetic; reordering in the command you are about to authorise is the bug.
+ */
+
+/**
+ * Every code point that can reorder or hide neighbouring text.
+ *
+ * Sources: Unicode 15 Bidirectional Algorithm (explicit formatting), the
+ * General_Category=Cf class for the invisible formatting characters, and
+ * CVE-2021-42574's published set. Each entry below says what it does, because
+ * "it is in a range" is not a reason a future reader can check.
+ */
+const VISIBLE_CONTROL_CODE_POINTS = new Map([
+  // Explicit bidi embedding/override — the Trojan Source core.
+  [0x202a, 'LRE'], [0x202b, 'RLE'], [0x202c, 'PDF'], [0x202d, 'LRO'], [0x202e, 'RLO'],
+  // Bidi isolates: the modern replacement, same reordering power.
+  [0x2066, 'LRI'], [0x2067, 'RLI'], [0x2068, 'FSI'], [0x2069, 'PDI'],
+  // Implicit marks: weaker, but still flip the order of adjacent runs.
+  [0x200e, 'LRM'], [0x200f, 'RLM'], [0x061c, 'ALM'],
+  // Invisible: hide or join text without reordering it.
+  [0x200b, 'ZWSP'], [0x200c, 'ZWNJ'], [0x200d, 'ZWJ'], [0xfeff, 'BOM'],
+  [0x2060, 'WJ'], [0x00ad, 'SHY'],
+  // Deprecated shaping/format controls that still render as nothing.
+  [0x206a, 'ISS'], [0x206b, 'ASS'], [0x206c, 'IAFS'], [0x206d, 'AAFS'],
+  [0x206e, 'NADS'], [0x206f, 'NODS'],
+])
+
+/** A lone CR can push the rest of a line out of view in a terminal-styled
+ *  block; a real CRLF is left alone because it is just a line ending. */
+const LONE_CARRIAGE_RETURN = /\r(?!\n)/g
+
+export function containsInvisibleControls(text: string): boolean {
+  if (LONE_CARRIAGE_RETURN.test(text)) {
+    LONE_CARRIAGE_RETURN.lastIndex = 0
+    return true
+  }
+  for (const character of text) {
+    if (VISIBLE_CONTROL_CODE_POINTS.has(character.codePointAt(0) ?? -1)) return true
+  }
+  return false
+}
+
+/**
+ * The text with every reordering or invisible control replaced by a visible
+ * marker: `⟨U+202E RLO⟩`. Ordinary text, including real newlines, tabs and
+ * every non-Latin script, is returned unchanged.
+ */
+export function withVisibleControls(text: string): string {
+  let out = ''
+  for (const character of text) {
+    const code = character.codePointAt(0) ?? -1
+    const name = VISIBLE_CONTROL_CODE_POINTS.get(code)
+    if (name) {
+      out += `⟨U+${code.toString(16).toUpperCase().padStart(4, '0')} ${name}⟩`
+      continue
+    }
+    out += character
+  }
+  return out.replace(LONE_CARRIAGE_RETURN, '⟨U+000D CR⟩')
+}

From 27ae4b36d0196d179d8a832a3e9f1081df89f70f Mon Sep 17 00:00:00 2001
From: Julius Olsson 
Date: Sat, 19 Sep 2026 19:42:43 -0700
Subject: [PATCH 02/10] fix(security): escape by Unicode's own invisibility
 rule, and on every authorisation surface
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Codex review of #1049, six findings:

1. High. The hand-picked table was still short by variation selectors,
   tag characters, Mongolian and Khmer controls, the invisible math
   operators and the combining grapheme joiner — each of which makes two
   different commands render identically (review reproduced `./check.sh`
   and `./check.sh` executing different files). The rule is now
   Unicode's `Default_Ignorable_Code_Point`, the property that MEANS
   "renders as nothing", with readable names kept for the characters a
   report will actually contain.
2. High. The persistent grant was still spoofable: OpenCode's `always`
   patterns and permission name, Codex's clickable options and reason,
   and Claude's title and option labels all rendered raw. Those fields
   describe the authorisation itself, independently of the command.
3. Medium. "CRLF is just a line ending" is a Windows text assumption,
   and this is a command: bash reads `./check.sh\r\n` as a filename
   ending in CR, and the review executed that file. Every CR is escaped
   now; LF is untouched.
4. Medium. Native consent dialogs were outside the protection —
   workflow-source approval (repository-controlled name and identity)
   and extension install consent (attacker-chosen manifest fields).
5. Medium. Destructive confirmation lists showed model-controlled titles
   and paths raw, so a reordered title misrepresents WHICH session a
   bulk close is about to terminate.
6. Medium. The queued-prompt preview and its dialog, which #1029 names
   explicitly. The escape happens before truncation, so a marker cannot
   be cut in half.

Combining accents stay untouched: they render, and escaping every
combining mark would make ordinary prose unreadable.

Tests: the widened rule (a variation selector, CGJ, Mongolian, Khmer, an
invisible operator, a tag character, a musical control), CRLF, ordinary
accents left alone, and the OpenCode `always` scope escaped in the real
recorded modal.

Co-Authored-By: Claude Opus 5 (1M context) 
---
 src/main/ipc/extensions.ts                    |   6 +-
 src/main/workflows/createWorkflowService.ts   |   9 +-
 .../claude/renderer/PermissionPromptModal.tsx |   4 +-
 .../codex/renderer/CodexApprovalModal.tsx     |   4 +-
 .../opencodePermissionView.renderer.test.tsx  |  15 +++
 .../opencode/renderer/conditions/views.tsx    |   4 +-
 .../workspace/ui/CloseConfirmationDialog.tsx  |   5 +-
 .../workspace/ui/CloseOldAgentsModal.tsx      |   7 +-
 .../tile-tree/TileLeaf/QueueStrip.tsx         |   9 +-
 src/shared/text/visibleControls.test.ts       |  37 +++++-
 src/shared/text/visibleControls.ts            | 111 +++++++++++-------
 11 files changed, 146 insertions(+), 65 deletions(-)

diff --git a/src/main/ipc/extensions.ts b/src/main/ipc/extensions.ts
index a2dad042e..27321ae4d 100644
--- a/src/main/ipc/extensions.ts
+++ b/src/main/ipc/extensions.ts
@@ -17,6 +17,7 @@ import type {
   ExtensionInstallResult,
   ExtensionListEntry,
 } from '@shared/types/extensions.js'
+import { withVisibleControls } from '@shared/text/visibleControls.js'
 
 // The capability-consent dialog, shared by both install paths (GitHub + local
 // folder). A blocking, OS-native dialog on purpose: granting an extension
@@ -73,9 +74,10 @@ function consentPromptFor(evt: IpcMainInvokeEvent, source: string): ConsentPromp
       // one thing the dialog did not show. `manifest.name` is attacker-chosen and
       // only length-bounded, so it is presented as a claim about an identity
       // (`id`), never as the identity itself.
-      message: `Install ${manifest.id} from ${source}?`,
+      // Every interpolated field here is attacker-chosen (#1049 review).
+      message: `Install ${withVisibleControls(manifest.id)} from ${withVisibleControls(source)}?`,
       detail:
-        `"${manifest.name}" wants these capabilities:\n\n${detail}\n\n` +
+        `"${withVisibleControls(manifest.name)}" wants these capabilities:\n\n${withVisibleControls(detail)}\n\n` +
         `${canWrite ? 'It can change project files.' : 'It cannot change project files.'} ` +
         `It has no network access. ` +
         `Install it only if you trust ${source}.`,
diff --git a/src/main/workflows/createWorkflowService.ts b/src/main/workflows/createWorkflowService.ts
index f6737d5d1..8b91df1a4 100644
--- a/src/main/workflows/createWorkflowService.ts
+++ b/src/main/workflows/createWorkflowService.ts
@@ -13,6 +13,7 @@ import { ElectronWorkflowWorkerLauncher } from '@main/workflows/ElectronWorkflow
 import { resolveClaudeAgentType } from '@main/workflows/ClaudeAgentTypeResolver.js'
 import { prepareGitWorkflowWorktree } from '@main/workflows/GitWorkflowWorktree.js'
 import { WorkflowSourceApprovalStore } from '@main/workflows/WorkflowSourceApprovalStore.js'
+import { withVisibleControls } from '@shared/text/visibleControls.js'
 
 export async function createWorkflowService(options: {
   isCodexCliUpdateReserved?: () => boolean
@@ -68,9 +69,13 @@ export async function createWorkflowService(options: {
       const result = await dialog.showMessageBox({
         type: 'warning',
         title: 'Approve workflow source',
-        message: `Allow ${source.workflowName} to run agents?`,
+        // Repository-controlled, so it is escaped (#1049 review): the hash
+        // binds the grant to exact bytes, but the IDENTITY beside it is what
+        // the user reads, and a reordered one can describe a different source
+        // than the bytes being approved.
+        message: `Allow ${withVisibleControls(source.workflowName)} to run agents?`,
         detail: [
-          `Source: ${source.canonicalIdentity}`,
+          `Source: ${withVisibleControls(source.canonicalIdentity)}`,
           `SHA-256: ${source.sourceHash}`,
           '',
           'This approval applies only to these exact bytes. Editing the workflow will ask again.',
diff --git a/src/providers/claude/renderer/PermissionPromptModal.tsx b/src/providers/claude/renderer/PermissionPromptModal.tsx
index fa8448767..5ba378bde 100644
--- a/src/providers/claude/renderer/PermissionPromptModal.tsx
+++ b/src/providers/claude/renderer/PermissionPromptModal.tsx
@@ -47,7 +47,7 @@ export function PermissionPromptModal({ state, onSend }: Props) {
           
!
- {title} + {withVisibleControls(title)} Review the requested tool and choose whether Claude may continue. @@ -73,7 +73,7 @@ export function PermissionPromptModal({ state, onSend }: Props) { key={`${option.key}:${option.label}`} className={index === state.selectedIndex ? 'text-ink' : undefined} > - {option.key}. {option.label} + {option.key}. {withVisibleControls(option.label)}
))}
diff --git a/src/providers/codex/renderer/CodexApprovalModal.tsx b/src/providers/codex/renderer/CodexApprovalModal.tsx index 5560c2370..89aa63398 100644 --- a/src/providers/codex/renderer/CodexApprovalModal.tsx +++ b/src/providers/codex/renderer/CodexApprovalModal.tsx @@ -139,7 +139,7 @@ export function CodexApprovalModal({ approval, onSend, interactionActive }: Prop {/* Reason — parsed from the screen's "Reason: " line */} {approval.reason && (
- Reason: {approval.reason} + Reason: {withVisibleControls(approval.reason)}
)} @@ -165,7 +165,7 @@ export function CodexApprovalModal({ approval, onSend, interactionActive }: Prop ›{' '} - {i + 1}. {opt} + {i + 1}. {withVisibleControls(opt)} ({DEFAULT_HINTS[i] ?? ''}) ))} diff --git a/src/providers/opencode/renderer/conditions/opencodePermissionView.renderer.test.tsx b/src/providers/opencode/renderer/conditions/opencodePermissionView.renderer.test.tsx index 640fa86fa..f3c3a9e2e 100644 --- a/src/providers/opencode/renderer/conditions/opencodePermissionView.renderer.test.tsx +++ b/src/providers/opencode/renderer/conditions/opencodePermissionView.renderer.test.tsx @@ -102,6 +102,21 @@ describe('opencode permission modal on a recorded 1.18.30 ask', () => { expect(rendered.textContent).not.toContain('\u202E') }) + it('escapes the persistent grant\'s scope too, since that is what "Allow always" authorises (#1049 review)', () => { + // The command is only half of the decision: "Allow always covers " + // describes what the grant will keep allowing, for this agent and its + // subagents. A reordered pattern misdescribes that scope. + const rec = recording() + for (const { event } of rec.sse) { + if (event.type === 'permission.asked') { + event.properties = { ...event.properties, metadata: { command: 'ls -1' }, pattern: ['ls \u202E rm -rf *'] } + } + } + mount(permissionStateFrom(rec)) + const always = screen.getByText(/Allow always covers/) + expect(always.textContent).not.toContain('\u202E') + }) + it('shows the command behind a default-permission external_directory ask, not just the directory', () => { // #1026 review: OpenCode's DEFAULT rules allow bash and ask only for // external_directory, so for most users this is THE shell-command diff --git a/src/providers/opencode/renderer/conditions/views.tsx b/src/providers/opencode/renderer/conditions/views.tsx index f7098cc94..a0991a780 100644 --- a/src/providers/opencode/renderer/conditions/views.tsx +++ b/src/providers/opencode/renderer/conditions/views.tsx @@ -220,11 +220,11 @@ export const opencodePermissionView = defineView< ) : ( <> - Allow always covers {permission ? <>{permission}{' '} : null} + Allow always covers {permission ? <>{withVisibleControls(permission)}{' '} : null} {always.map((pattern, index) => ( {index > 0 ? ', ' : ''} - {pattern} + {withVisibleControls(pattern)} ))}{' '} for this agent and its subagents until this agent restarts. diff --git a/src/renderer/src/features/workspace/ui/CloseConfirmationDialog.tsx b/src/renderer/src/features/workspace/ui/CloseConfirmationDialog.tsx index 6a49047cf..4ce72d1f7 100644 --- a/src/renderer/src/features/workspace/ui/CloseConfirmationDialog.tsx +++ b/src/renderer/src/features/workspace/ui/CloseConfirmationDialog.tsx @@ -15,6 +15,7 @@ import { subscribeToCloseConfirmation, } from '@renderer/workspace/closeConfirmationBroker' import type { PendingCloseConfirmation } from '@renderer/workspace/closeConfirmationBroker' +import { withVisibleControls } from '@shared/text/visibleControls' /** * The confirmation the close paths await before ending anything. @@ -78,7 +79,9 @@ export function CloseConfirmationDialog() { key={target.sessionId} className="flex items-center justify-between border-b border-border/40 px-2 py-1 text-xs last:border-b-0" > - {target.title} + {/* The title is model-controlled (#1049 review): a reordered + one misrepresents WHICH session is about to be killed. */} + {withVisibleControls(target.title)} {target.live ? ( working diff --git a/src/renderer/src/features/workspace/ui/CloseOldAgentsModal.tsx b/src/renderer/src/features/workspace/ui/CloseOldAgentsModal.tsx index 681886cb3..00698c995 100644 --- a/src/renderer/src/features/workspace/ui/CloseOldAgentsModal.tsx +++ b/src/renderer/src/features/workspace/ui/CloseOldAgentsModal.tsx @@ -29,6 +29,7 @@ import { resolveTabSessions } from '@renderer/workspace/queries' import type { SessionId, Tab } from '@renderer/workspace/types' import type { Workspace } from '@renderer/workspace/workspaceStore' import type { Entry } from '@shared/types/transcript' +import { withVisibleControls } from '@shared/text/visibleControls' type Props = { open: boolean @@ -641,11 +642,13 @@ export function CloseOldAgentsModal({ open, workspace, onClose }: Props) {
+ {/* Same rule as the close confirmation: these identify + what a bulk close is about to terminate (#1049). */}
- {row.title} + {withVisibleControls(row.title)}
- {tabIndexLabel(row.tabIndex)} · {row.tabTitle} · {row.cwd} + {tabIndexLabel(row.tabIndex)} · {withVisibleControls(row.tabTitle)} · {withVisibleControls(row.cwd)}
diff --git a/src/renderer/src/workspace/tile-tree/TileLeaf/QueueStrip.tsx b/src/renderer/src/workspace/tile-tree/TileLeaf/QueueStrip.tsx index d779e15e6..a3051faad 100644 --- a/src/renderer/src/workspace/tile-tree/TileLeaf/QueueStrip.tsx +++ b/src/renderer/src/workspace/tile-tree/TileLeaf/QueueStrip.tsx @@ -13,6 +13,7 @@ import { } from '@renderer/components/ui/dialog' import { PagedTextViewer } from '@renderer/lib/text/PagedTextViewer' import { useEffect, useId, useMemo, useState } from 'react' +import { withVisibleControls } from '@shared/text/visibleControls' // The browsing surface must stay cheap even when somebody pastes a whole // design document as their next prompt. CSS clipping alone still leaves the @@ -23,7 +24,11 @@ const PREVIEW_SCAN_CHARACTERS = 320 const PREVIEW_CHARACTERS = 180 function queuedPromptPreview(content: string): string { - const scanned = content.slice(0, PREVIEW_SCAN_CHARACTERS) + // Escaped BEFORE truncation (#1029): a queued prompt is text the user is + // about to send on their own authority, and #1049's review found both this + // preview and the dialog below showing reordering controls raw. Escaping + // first also means the marker itself cannot be cut in half by the slice. + const scanned = withVisibleControls(content.slice(0, PREVIEW_SCAN_CHARACTERS)) // Preserve line boundaries because they are the only cheap hint that a // queued item contains pasted instructions or code. Horizontal whitespace // is normalized so an indented block cannot make the compact lane look @@ -96,7 +101,7 @@ function QueuedPromptDialog({
{message ? ( ) : null} diff --git a/src/shared/text/visibleControls.test.ts b/src/shared/text/visibleControls.test.ts index 2994a14b6..2343833e8 100644 --- a/src/shared/text/visibleControls.test.ts +++ b/src/shared/text/visibleControls.test.ts @@ -21,14 +21,41 @@ describe('visible controls in text a user is asked to approve', () => { expect(withVisibleControls('sudo')).toBe('⟨U+FEFF BOM⟩sudo') }) - it('escapes a lone carriage return, which scrolls the rest of a line out of view', () => { + it('escapes EVERY carriage return, including one inside a CRLF', () => { + // "CRLF is just a line ending" is a Windows text assumption, and this is + // a command: bash reads `./check.sh\r\n` as an instruction to run a file + // whose name ends in CR, and #1049's review executed that file instead of + // the intended one. A line feed is left alone — it is the break the
+    // already shows.
     expect(withVisibleControls('echo safe\rrm -rf /')).toBe('echo safe⟨U+000D CR⟩rm -rf /')
-    // A real line ending is not an attack.
-    expect(withVisibleControls('echo safe\r\nrm -rf /')).toBe('echo safe\r\nrm -rf /')
+    expect(withVisibleControls('./check.sh\r\n')).toBe('./check.sh⟨U+000D CR⟩\n')
+    expect(containsInvisibleControls('./check.sh\r\n')).toBe(true)
   })
 
-  it('leaves ordinary text alone, including every non-Latin script', () => {
-    for (const text of ['npm run build', 'grep -R "café" .', 'echo "日本語"', 'echo "العربية"', 'tab\there\nnewline']) {
+  it('escapes every character Unicode defines as invisible, not a hand-picked list', () => {
+    // The first version enumerated the bidi and zero-width families and was
+    // still short by these, each of which makes two different commands render
+    // identically. Reproduced in review: `./check.sh` and `./check.sh\uFE0F`
+    // execute different files.
+    for (const [text, expected] of [
+      ['./check.sh\uFE0F', './check.sh⟨U+FE0F⟩'],            // variation selector 16
+      ['git\u034Fpush', 'git⟨U+034F CGJ⟩push'],               // combining grapheme joiner
+      ['rm\u180E -rf', 'rm⟨U+180E⟩ -rf'],                     // Mongolian vowel separator
+      ['a\u17B4b', 'a⟨U+17B4⟩b'],                             // Khmer inherent vowel
+      ['x\u2062y', 'x⟨U+2062 INVISIBLE TIMES⟩y'],             // invisible math operator
+      ['sudo\u{E0041}', 'sudo⟨U+E0041⟩'],                     // tag character
+      ['a\u{1D173}b', 'a⟨U+1D173⟩b'],                         // musical format control
+    ] as const) {
+      expect(withVisibleControls(text)).toBe(expected)
+      expect(containsInvisibleControls(text)).toBe(true)
+    }
+  })
+
+  it('leaves ordinary text alone, including every non-Latin script and ordinary accents', () => {
+    // Combining accents are NOT blanket-escaped: they render, and treating
+    // every combining mark as an attack would make ordinary prose unreadable
+    // (#1049 review).
+    for (const text of ['npm run build', 'grep -R "café" .', 'grep -R "cafe\u0301" .', 'echo "日本語"', 'echo "العربية"', 'tab\there\nnewline']) {
       expect(withVisibleControls(text)).toBe(text)
       expect(containsInvisibleControls(text)).toBe(false)
     }
diff --git a/src/shared/text/visibleControls.ts b/src/shared/text/visibleControls.ts
index 6c4ff8779..a5e81a11b 100644
--- a/src/shared/text/visibleControls.ts
+++ b/src/shared/text/visibleControls.ts
@@ -6,79 +6,100 @@
  * isolates reorder the glyphs a browser draws without changing the bytes a
  * shell runs, so a prompt-injected model can propose a command that READS as
  * harmless and RUNS as something else — the canonical demonstration makes a
- * destructive command appear to be inside a comment. Zero-width characters do
- * the same job by hiding text outright, and a lone `\r` can scroll the
- * dangerous half of a line out of view in a `
`. The permission modal is
- * frequently the only place the command is ever shown, so what it renders IS
- * the user's evidence.
+ * destructive command appear to be inside a comment. Invisible characters do
+ * the same job by hiding a difference rather than reordering one: `./check.sh`
+ * and `./check.sh` look identical and execute different files
+ * (reproduced in #1049's review). A carriage return hides one the same way:
+ * a Unix shell treats `./check.sh\r` as a filename whose last byte is CR.
  *
  * WHY escape rather than strip: the approval has to show what will actually
  * run. Deleting the characters would make the modal disagree with the command
- * the provider executes — a second, quieter lie. `⟨U+202E⟩` keeps the string
- * faithful, makes the anomaly obvious, and stays copy-pasteable as a report.
+ * the provider executes — a second, quieter lie. `⟨U+202E RLO⟩` keeps the
+ * string faithful, makes the anomaly obvious, and stays copy-pasteable as a
+ * report.
  *
- * WHY not a regex over "suspicious ranges" at each call site: there are three
- * separate families (bidi controls, isolates, invisible formatting) plus the
- * deprecated ones, and any surface that forgot one would silently be the weak
- * link. One table, one helper, every approval surface.
+ * WHY the rule is "Unicode says this is invisible" rather than a hand-picked
+ * list: the first version enumerated the bidi and zero-width families and was
+ * still short by variation selectors, tag characters, Mongolian and Khmer
+ * controls, and the invisible math operators — each of which makes two
+ * different commands render identically. `Default_Ignorable_Code_Point` is
+ * the property Unicode defines for exactly "renders as nothing", so it is the
+ * rule, with readable names for the ones a reader will actually meet.
  *
  * NOT for transcript bodies or agent prose. Reordering in a model's answer is
- * cosmetic; reordering in the command you are about to authorise is the bug.
+ * cosmetic; reordering in the thing you are about to authorise is the bug.
  */
 
-/**
- * Every code point that can reorder or hide neighbouring text.
- *
- * Sources: Unicode 15 Bidirectional Algorithm (explicit formatting), the
- * General_Category=Cf class for the invisible formatting characters, and
- * CVE-2021-42574's published set. Each entry below says what it does, because
- * "it is in a range" is not a reason a future reader can check.
- */
-const VISIBLE_CONTROL_CODE_POINTS = new Map([
-  // Explicit bidi embedding/override — the Trojan Source core.
+/** Readable names for the characters a report is likely to contain. */
+const NAMED: ReadonlyMap = new Map([
+  [0x00ad, 'SHY'], [0x034f, 'CGJ'], [0x061c, 'ALM'],
+  [0x200b, 'ZWSP'], [0x200c, 'ZWNJ'], [0x200d, 'ZWJ'], [0x200e, 'LRM'], [0x200f, 'RLM'],
   [0x202a, 'LRE'], [0x202b, 'RLE'], [0x202c, 'PDF'], [0x202d, 'LRO'], [0x202e, 'RLO'],
-  // Bidi isolates: the modern replacement, same reordering power.
+  [0x2060, 'WJ'], [0x2061, 'FUNCTION APPLICATION'], [0x2062, 'INVISIBLE TIMES'],
+  [0x2063, 'INVISIBLE SEPARATOR'], [0x2064, 'INVISIBLE PLUS'],
   [0x2066, 'LRI'], [0x2067, 'RLI'], [0x2068, 'FSI'], [0x2069, 'PDI'],
-  // Implicit marks: weaker, but still flip the order of adjacent runs.
-  [0x200e, 'LRM'], [0x200f, 'RLM'], [0x061c, 'ALM'],
-  // Invisible: hide or join text without reordering it.
-  [0x200b, 'ZWSP'], [0x200c, 'ZWNJ'], [0x200d, 'ZWJ'], [0xfeff, 'BOM'],
-  [0x2060, 'WJ'], [0x00ad, 'SHY'],
-  // Deprecated shaping/format controls that still render as nothing.
   [0x206a, 'ISS'], [0x206b, 'ASS'], [0x206c, 'IAFS'], [0x206d, 'AAFS'],
   [0x206e, 'NADS'], [0x206f, 'NODS'],
+  [0xfeff, 'BOM'],
 ])
 
-/** A lone CR can push the rest of a line out of view in a terminal-styled
- *  block; a real CRLF is left alone because it is just a line ending. */
-const LONE_CARRIAGE_RETURN = /\r(?!\n)/g
+/**
+ * Unicode's `Default_Ignorable_Code_Point` ranges (DerivedCoreProperties, 15
+ * through 18 — the review compared them and found no later additions).
+ * Anything here is defined to render as nothing.
+ */
+const DEFAULT_IGNORABLE: ReadonlyArray = [
+  [0x00ad, 0x00ad], [0x034f, 0x034f], [0x061c, 0x061c],
+  [0x115f, 0x1160], [0x17b4, 0x17b5], [0x180b, 0x180f],
+  [0x200b, 0x200f], [0x202a, 0x202e], [0x2060, 0x206f],
+  [0x3164, 0x3164], [0xfe00, 0xfe0f], [0xfeff, 0xfeff],
+  [0xffa0, 0xffa0], [0xfff0, 0xfff8],
+  [0x1bca0, 0x1bca3], [0x1d173, 0x1d17a],
+  [0xe0000, 0xe0fff],
+]
+
+function invisible(code: number): boolean {
+  return DEFAULT_IGNORABLE.some(([from, to]) => code >= from && code <= to)
+}
+
+/**
+ * A carriage return is escaped WHEREVER it appears, including in a CRLF.
+ *
+ * WHY (#1049 review): "CRLF is just a line ending" is a Windows text
+ * assumption, and this text is a command. Bash reads `./check.sh\r\n` as an
+ * instruction to run a file whose name ends in CR — the review executed the
+ * CR-suffixed file instead of the intended one. A line feed is left alone: it
+ * genuinely is the line break the `
` already shows.
+ */
+const CARRIAGE_RETURN = /\r/g
+
+function marker(code: number): string {
+  const name = NAMED.get(code)
+  const hex = code.toString(16).toUpperCase().padStart(4, '0')
+  return name ? `⟨U+${hex} ${name}⟩` : `⟨U+${hex}⟩`
+}
 
 export function containsInvisibleControls(text: string): boolean {
-  if (LONE_CARRIAGE_RETURN.test(text)) {
-    LONE_CARRIAGE_RETURN.lastIndex = 0
+  if (CARRIAGE_RETURN.test(text)) {
+    CARRIAGE_RETURN.lastIndex = 0
     return true
   }
   for (const character of text) {
-    if (VISIBLE_CONTROL_CODE_POINTS.has(character.codePointAt(0) ?? -1)) return true
+    if (invisible(character.codePointAt(0) ?? -1)) return true
   }
   return false
 }
 
 /**
- * The text with every reordering or invisible control replaced by a visible
- * marker: `⟨U+202E RLO⟩`. Ordinary text, including real newlines, tabs and
- * every non-Latin script, is returned unchanged.
+ * The text with every invisible or reordering character replaced by a visible
+ * marker: `⟨U+202E RLO⟩`, or `⟨U+FE0F⟩` for one with no short name. Ordinary
+ * text, including line feeds, tabs and every script, is returned unchanged.
  */
 export function withVisibleControls(text: string): string {
   let out = ''
   for (const character of text) {
     const code = character.codePointAt(0) ?? -1
-    const name = VISIBLE_CONTROL_CODE_POINTS.get(code)
-    if (name) {
-      out += `⟨U+${code.toString(16).toUpperCase().padStart(4, '0')} ${name}⟩`
-      continue
-    }
-    out += character
+    out += invisible(code) ? marker(code) : character
   }
-  return out.replace(LONE_CARRIAGE_RETURN, '⟨U+000D CR⟩')
+  return out.replace(CARRIAGE_RETURN, '⟨U+000D CR⟩')
 }

From c8fd18db38290cf1b8d36dc7f094f78b47bd4d89 Mon Sep 17 00:00:00 2001
From: Julius Olsson 
Date: Sat, 19 Sep 2026 20:26:24 -0700
Subject: [PATCH 03/10] fix(approvals): close the six approval surfaces the
 re-review found still raw
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Round-2 review. The Unicode table itself checked out against
DerivedCoreProperties (15/17/18, 4,174 code points, no gaps), but coverage
did not:

- OpenCode's WILDCARD grant branch (`always: ['*']`) rendered the permission
  name raw. That branch describes the broadest grant we offer, so its scope
  line is the most profitable thing to spoof; the earlier test exercised the
  pattern branch beside it.
- Codex trimmed BEFORE escaping, and `.trim()` removes CR — so `./check.sh\r`,
  the exact trick this work exists to expose, still rendered as `./check.sh`.
- A single-target close confirmation shows no target list, so its summary is
  the only identity the user sees before authorising a kill. It was raw.
- Bulk close rendered project labels and directory paths raw beside the
  checkboxes that choose what dies.
- Grok rendered provider-supplied option labels and approved plan content raw.
- The extension consent dialog escaped `source` in its heading and then
  interpolated it raw into "Install it only if you trust …".

Three new tests, each failing first: the wildcard grant scope (recorded
1.18.30 ask, replayed through the real dispatcher), the Codex carriage return,
and the single-target close summary.

Co-Authored-By: Claude Opus 5 (1M context) 
---
 src/main/ipc/extensions.ts                    |  5 ++++-
 .../codex/renderer/CodexApprovalModal.tsx     |  8 ++++++-
 .../grok/renderer/conditions/views.tsx        |  9 ++++++--
 .../opencodePermissionView.renderer.test.tsx  | 22 +++++++++++++++++++
 .../opencode/renderer/conditions/views.tsx    |  6 ++++-
 ...inlineConditionOwnership.renderer.test.tsx | 20 +++++++++++++++++
 .../CloseConfirmationDialog.renderer.test.tsx | 17 ++++++++++++++
 .../workspace/ui/CloseConfirmationDialog.tsx  |  6 ++++-
 .../workspace/ui/CloseOldAgentsModal.tsx      |  9 ++++++--
 9 files changed, 94 insertions(+), 8 deletions(-)

diff --git a/src/main/ipc/extensions.ts b/src/main/ipc/extensions.ts
index 27321ae4d..a3afcb396 100644
--- a/src/main/ipc/extensions.ts
+++ b/src/main/ipc/extensions.ts
@@ -80,7 +80,10 @@ function consentPromptFor(evt: IpcMainInvokeEvent, source: string): ConsentPromp
         `"${withVisibleControls(manifest.name)}" wants these capabilities:\n\n${withVisibleControls(detail)}\n\n` +
         `${canWrite ? 'It can change project files.' : 'It cannot change project files.'} ` +
         `It has no network access. ` +
-        `Install it only if you trust ${source}.`,
+        // The same value, twice, and the second one was raw: a source string
+        // with a bidi override could therefore spoof the sentence that carries
+        // the whole trust decision (#1049 re-review).
+        `Install it only if you trust ${withVisibleControls(source)}.`,
     }
     const result = win
       ? await dialog.showMessageBox(win, options)
diff --git a/src/providers/codex/renderer/CodexApprovalModal.tsx b/src/providers/codex/renderer/CodexApprovalModal.tsx
index 89aa63398..3aa37d020 100644
--- a/src/providers/codex/renderer/CodexApprovalModal.tsx
+++ b/src/providers/codex/renderer/CodexApprovalModal.tsx
@@ -90,7 +90,13 @@ export function CodexApprovalModal({ approval, onSend, interactionActive }: Prop
 
   if (!approval) return null
 
-  const command = approval.command.join(' ').trim()
+  // Escape BEFORE trimming, never after. `./check.sh\r` is a filename whose
+  // last byte is CR — the exact trick #1049 exists to expose — and `.trim()`
+  // removes CR, so trimming first deleted the evidence and left two different
+  // commands rendering identically (#1049 re-review). After escaping, the CR
+  // is the visible text `⟨U+000D CR⟩`, which trim leaves alone, and ordinary
+  // surrounding whitespace is still tidied.
+  const command = withVisibleControls(approval.command.join(' ')).trim()
 
   return (
     
- {action.label} + {/* Provider-supplied, and it is the text the user reads to decide + WHICH grant they are giving (#1049 re-review). */} + {withVisibleControls(action.label)} ) })} @@ -184,7 +186,10 @@ export const grokPlanApprovalView = defineView< {state.planContent ? (
-            {state.planContent}
+            {/* The plan is the thing being approved; a reordering override in
+                it misrepresents what the user is authorising (#1049
+                re-review). */}
+            {withVisibleControls(state.planContent)}
           
) : (

Grok is waiting for plan approval.

diff --git a/src/providers/opencode/renderer/conditions/opencodePermissionView.renderer.test.tsx b/src/providers/opencode/renderer/conditions/opencodePermissionView.renderer.test.tsx index f3c3a9e2e..589055a30 100644 --- a/src/providers/opencode/renderer/conditions/opencodePermissionView.renderer.test.tsx +++ b/src/providers/opencode/renderer/conditions/opencodePermissionView.renderer.test.tsx @@ -117,6 +117,28 @@ describe('opencode permission modal on a recorded 1.18.30 ask', () => { expect(always.textContent).not.toContain('\u202E') }) + it('escapes the WILDCARD grant scope, the broadest one we offer (#1049 re-review)', () => { + // `always: ['*']` renders a different branch — "every + // request" — and that branch was left unescaped while the pattern branch + // beside it was fixed. It is also the worst one to lose: the wildcard is + // the broadest grant in the modal, so the permission name is the only + // thing telling the user what they are signing away. + const rec = recording() + for (const { event } of rec.sse) { + if (event.type === 'permission.asked') { + // The recorded payload's own field names: `permission` and `always` + // sit beside `metadata`, and the dispatcher folds the whole payload + // into the state's metadata. `always: ['*']` is the wildcard shape + // OpenCode really sends for edit/write/MCP asks. + event.properties = { ...event.properties, permission: 'bash \u202E harmless', always: ['*'] } + } + } + mount(permissionStateFrom(rec)) + const always = screen.getByText(/Allow always covers/) + expect(always.textContent).not.toContain('\u202E') + expect(always.textContent).toContain('U+202E') + }) + it('shows the command behind a default-permission external_directory ask, not just the directory', () => { // #1026 review: OpenCode's DEFAULT rules allow bash and ask only for // external_directory, so for most users this is THE shell-command diff --git a/src/providers/opencode/renderer/conditions/views.tsx b/src/providers/opencode/renderer/conditions/views.tsx index a0991a780..5e6e324b2 100644 --- a/src/providers/opencode/renderer/conditions/views.tsx +++ b/src/providers/opencode/renderer/conditions/views.tsx @@ -215,7 +215,11 @@ export const opencodePermissionView = defineView< {always.includes('*') ? ( <> Allow always covers{' '} - every {permission ? {permission} : 'such'} request{' '} + {/* The wildcard branch names the SCOPE the grant will cover, + so it is the one a spoofed permission name would misstate + most profitably — `*` is the broadest grant we offer + (#1049 re-review found this branch unescaped). */} + every {permission ? {withVisibleControls(permission)} : 'such'} request{' '} from this agent and its subagents until this agent restarts. ) : ( diff --git a/src/providers/shared/renderer/conditions/inlineConditionOwnership.renderer.test.tsx b/src/providers/shared/renderer/conditions/inlineConditionOwnership.renderer.test.tsx index d1241b931..f3c566ab2 100644 --- a/src/providers/shared/renderer/conditions/inlineConditionOwnership.renderer.test.tsx +++ b/src/providers/shared/renderer/conditions/inlineConditionOwnership.renderer.test.tsx @@ -107,3 +107,23 @@ describe('pane-local condition keyboard ownership', () => { expect(request).toHaveBeenCalledOnce() }) }) + +describe('what an approval modal SHOWS is part of the decision (#1049)', () => { + it('keeps a trailing carriage return visible instead of trimming it away', async () => { + // `./check.sh\r` is a filename whose last byte is CR: a shell runs a + // DIFFERENT file than `./check.sh`, and the two render identically. The + // first fix escaped the command but trimmed it first, and `.trim()` + // removes CR — so the modal went on showing the safe-looking name for the + // command Codex would actually run (#1049 re-review). + const { CodexApprovalModal } = await import('@providers/codex/renderer/CodexApprovalModal') + render( + undefined)} + interactionActive={false} + />, + ) + const strip = screen.getByRole('group', { name: 'Codex command approval options' }) + expect(strip.textContent).toContain('U+000D CR') + }) +}) diff --git a/src/renderer/src/features/workspace/ui/CloseConfirmationDialog.renderer.test.tsx b/src/renderer/src/features/workspace/ui/CloseConfirmationDialog.renderer.test.tsx index 6af128a79..07d098c18 100644 --- a/src/renderer/src/features/workspace/ui/CloseConfirmationDialog.renderer.test.tsx +++ b/src/renderer/src/features/workspace/ui/CloseConfirmationDialog.renderer.test.tsx @@ -64,6 +64,23 @@ it('asks about one working session without a list or a count', () => { expect(screen.getByRole('button', { name: 'Close' })).toBeInTheDocument() }) +it('escapes the summary, the ONLY identity a single-target close shows (#1049 re-review)', () => { + // With one target the list below never renders, so the summary — which + // embeds the session's own title — is the whole of what the user reads + // before authorising a kill. A title carrying a bidi override could name a + // different session than the one that dies. + render() + act(() => { + void requestCloseConfirmation({ + required: true, reason: 'running', targets: [worker], + summary: 'staging \u202Eprod deploy is still working. Close it anyway?', + }) + }) + const description = screen.getByText(/is still working/) + expect(description.textContent).not.toContain('\u202E') + expect(description.textContent).toContain('U+202E') +}) + it('offers exactly two answers — no narrower or wider scope than the list shown', () => { // The regression guard for the deleted branch: a third button means some // close path has grown a second scope again, and the user is once more diff --git a/src/renderer/src/features/workspace/ui/CloseConfirmationDialog.tsx b/src/renderer/src/features/workspace/ui/CloseConfirmationDialog.tsx index 4ce72d1f7..713d9af6f 100644 --- a/src/renderer/src/features/workspace/ui/CloseConfirmationDialog.tsx +++ b/src/renderer/src/features/workspace/ui/CloseConfirmationDialog.tsx @@ -69,7 +69,11 @@ export function CloseConfirmationDialog() { ? 'Kill this session permanently?' : 'Close these sessions?'} - {request?.summary} + {/* The summary embeds the session's own title, and for a SINGLE + target the escaped target list below never renders — so this line + is the only identity the user is shown before authorising a kill + (#1049 re-review). */} + {request ? withVisibleControls(request.summary) : null} {request && request.targets.length > 1 ? ( diff --git a/src/renderer/src/features/workspace/ui/CloseOldAgentsModal.tsx b/src/renderer/src/features/workspace/ui/CloseOldAgentsModal.tsx index 00698c995..451c31d66 100644 --- a/src/renderer/src/features/workspace/ui/CloseOldAgentsModal.tsx +++ b/src/renderer/src/features/workspace/ui/CloseOldAgentsModal.tsx @@ -586,11 +586,16 @@ export function CloseOldAgentsModal({ open, workspace, onClose }: Props) { {/* The Dispatch vocabulary (A · title), so this picker names projects the way the index does. Worktrees appear below as directories inside the project. */} + {/* Escaped like the session rows below: these two + lines are what the user reads to decide WHICH + project they are ticking for a bulk close, and both + come from titles and paths an agent can influence + (#1049 re-review). */} - {project.label} + {withVisibleControls(project.label)} - {project.directories.join(' · ')} + {withVisibleControls(project.directories.join(' · '))} From 2a41e8b4ab86aacc1b2eca45d78453d82cc87d7f Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 19 Sep 2026 20:43:45 -0700 Subject: [PATCH 04/10] fix(approvals): escape every remaining surface the user authorises from MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-3 sweep. The six named gaps were fixed; this closes the thirteen the reviewer's full sweep then found. All the same defect: text an agent or a repository authors, rendered raw in the sentence the user reads before granting something. P1: Root Agent Code Management named its recipient raw — a title like `Trusted auditor` renders as a different, trusted agent immediately before the broadest grant this app offers. P2: Claude's interactive question picker (header, question, option labels and descriptions — display only, the resolver payload and the transcript keep the original bytes); the editor's delete and close-with-unsaved-changes dialogs, where `report.txt` is a different file; the installed-skill install and update review, where a repository supplies descriptions, source paths and file lists; bulk provider switch and the provider-switch picker; rewind. P3: new-agent project choice, project merge, the OpenCode and Grok question bodies (reject-only today), and Claude's resume prompt, whose age and token strings are scraped from its screen between two anchors. A regression test covers the P1. `tsc -b` clean; the pre-existing imageAttachment corpus failure is unrelated (it fails identically on main). Co-Authored-By: Claude Opus 5 (1M context) --- .../claude/renderer/ResumePromptModal.tsx | 7 ++++++- .../ask-user-question/AskUserQuestionRow.tsx | 15 ++++++++++---- .../grok/renderer/conditions/views.tsx | 2 +- .../opencode/renderer/conditions/views.tsx | 5 ++++- .../features/editor/ui/ConfirmCloseDialog.tsx | 8 ++++++-- .../editor/ui/ConfirmDeleteDialog.tsx | 8 ++++++-- .../ui/AgentCodeInstalledSkillsRow.tsx | 16 ++++++++++----- .../workspace/ui/BulkProviderSwitchModal.tsx | 11 ++++++---- .../workspace/ui/MergeProjectTabsModal.tsx | 11 ++++++---- .../workspace/ui/NewAgentInDialog.tsx | 3 ++- .../ui/ProviderSwitchPickerModal.tsx | 5 ++++- .../workspace/ui/RewindToPromptModal.tsx | 7 +++++-- ...tManagementConfirmDialog.renderer.test.tsx | 20 +++++++++++++++++++ .../ui/RootManagementConfirmDialog.tsx | 10 ++++++++-- 14 files changed, 98 insertions(+), 30 deletions(-) diff --git a/src/providers/claude/renderer/ResumePromptModal.tsx b/src/providers/claude/renderer/ResumePromptModal.tsx index 1dae85dc8..42f3505b7 100644 --- a/src/providers/claude/renderer/ResumePromptModal.tsx +++ b/src/providers/claude/renderer/ResumePromptModal.tsx @@ -1,4 +1,5 @@ import { useCallback, useEffect, useRef, useState } from 'react' +import { withVisibleControls } from '@shared/text/visibleControls' type Props = { prompt: { @@ -104,7 +105,11 @@ export function ResumePromptModal({ prompt, onSend, interactionActive }: Props) outline-none ">
- This session is {prompt.sessionAgeText ?? 'older'} old and {prompt.tokenCountText ?? 'many'} tokens. + {/* Scraped from Claude's own screen between two anchors, so the + parser accepts whatever sits there — including controls that + reorder the size and age this decision is made on (#1049 + re-review). */} + This session is {withVisibleControls(prompt.sessionAgeText ?? 'older')} old and {withVisibleControls(prompt.tokenCountText ?? 'many')} tokens.
diff --git a/src/providers/claude/renderer/components/ask-user-question/AskUserQuestionRow.tsx b/src/providers/claude/renderer/components/ask-user-question/AskUserQuestionRow.tsx index 6a4de5218..72daebd50 100644 --- a/src/providers/claude/renderer/components/ask-user-question/AskUserQuestionRow.tsx +++ b/src/providers/claude/renderer/components/ask-user-question/AskUserQuestionRow.tsx @@ -22,6 +22,7 @@ import { useAnswerSubmissionStore, useAnsweredViaMessageStore, } from '@providers/claude/renderer/components/ask-user-question/answeredViaMessageStore' +import { withVisibleControls } from '@shared/text/visibleControls' // Native in-feed renderer for Claude Code's `AskUserQuestion` tool. // @@ -408,12 +409,18 @@ export function AskUserQuestionRow({
{q.header ? ( - {q.header} + {/* DISPLAY only — the resolver still sends the option's own + bytes, and the transcript keeps the original text. What + is escaped is what the user READS before clicking, which + an agent authors: `Run ./check.sh` and + `Run ./check.sh` are different commands that render + identically (#1049 re-review). */} + {withVisibleControls(q.header)} ) : null} {q.question ? (
- {q.question} + {withVisibleControls(q.question)}
) : null}
@@ -453,9 +460,9 @@ export function AskUserQuestionRow({ {q.multiSelect ? (isSelected ? '[x]' : '[ ]') : isSelected ? '(*)' : `${oi + 1}.`} - {opt.label} + {withVisibleControls(opt.label)} {opt.description ? ( - {opt.description} + {withVisibleControls(opt.description)} ) : null} diff --git a/src/providers/grok/renderer/conditions/views.tsx b/src/providers/grok/renderer/conditions/views.tsx index e2178c2de..1aaa780f2 100644 --- a/src/providers/grok/renderer/conditions/views.tsx +++ b/src/providers/grok/renderer/conditions/views.tsx @@ -160,7 +160,7 @@ export const grokQuestionView = defineView< {state.text ? (
-            {state.text}
+            {withVisibleControls(state.text)}
           
) : (

Grok is waiting for a response.

diff --git a/src/providers/opencode/renderer/conditions/views.tsx b/src/providers/opencode/renderer/conditions/views.tsx index 5e6e324b2..f3c6b7c80 100644 --- a/src/providers/opencode/renderer/conditions/views.tsx +++ b/src/providers/opencode/renderer/conditions/views.tsx @@ -259,7 +259,10 @@ export const opencodeQuestionView = defineView< // height, and Escape and outside-click are disabled, so a long // question must never push the only button (Reject) off-screen.
-            {state.text}
+            {/* Reject-only today, so no affirmative grant hangs off it — but
+                it is still a provider-authored question the user answers, and
+                the escape costs nothing (#1049 re-review). */}
+            {withVisibleControls(state.text)}
           
) : (

OpenCode is waiting for a response.

diff --git a/src/renderer/src/features/editor/ui/ConfirmCloseDialog.tsx b/src/renderer/src/features/editor/ui/ConfirmCloseDialog.tsx index 330d84b61..e433b4dc2 100644 --- a/src/renderer/src/features/editor/ui/ConfirmCloseDialog.tsx +++ b/src/renderer/src/features/editor/ui/ConfirmCloseDialog.tsx @@ -7,6 +7,7 @@ import { DialogHeader, DialogTitle, } from '@renderer/components/ui/dialog' +import { withVisibleControls } from '@shared/text/visibleControls' type Props = { fileName: string @@ -45,9 +46,12 @@ export function ConfirmCloseDialog({ {deleted ? 'File deleted on disk' : 'Unsaved changes'} + {/* Which file is saved, discarded or recreated is the whole + decision, and an invisible character makes two names identical + (#1049 re-review). */} {deleted - ? `“${fileName}” no longer exists on disk. Its in-memory copy is still safe here. Recreate it before closing?` - : `“${fileName}” has unsaved changes. Save before closing?`} + ? `“${withVisibleControls(fileName)}” no longer exists on disk. Its in-memory copy is still safe here. Recreate it before closing?` + : `“${withVisibleControls(fileName)}” has unsaved changes. Save before closing?`} {error ? (

diff --git a/src/renderer/src/features/editor/ui/ConfirmDeleteDialog.tsx b/src/renderer/src/features/editor/ui/ConfirmDeleteDialog.tsx index fa0e1ecd2..b30c9beb9 100644 --- a/src/renderer/src/features/editor/ui/ConfirmDeleteDialog.tsx +++ b/src/renderer/src/features/editor/ui/ConfirmDeleteDialog.tsx @@ -7,6 +7,7 @@ import { DialogHeader, DialogTitle, } from '@renderer/components/ui/dialog' +import { withVisibleControls } from '@shared/text/visibleControls' type Props = { path: string @@ -26,7 +27,10 @@ export function ConfirmDeleteDialog({ path, dirtyPaths, onCancel, onConfirm }: P Delete from disk? - “{path}” will be permanently deleted. + {/* `report.txt` and `report.txt` are different files and + render identically; this dialog authorises deleting one of + them (#1049 re-review). */} + “{withVisibleControls(path)}” will be permanently deleted. {dirtyCount > 0 ? ` ${dirtyCount} open unsaved ${dirtyCount === 1 ? 'file is' : 'files are'} inside it; confirming will discard those edits.` : ' This action cannot be undone in Agent Code.'} @@ -36,7 +40,7 @@ export function ConfirmDeleteDialog({ path, dirtyPaths, onCancel, onConfirm }: P

{dirtyPaths.map(dirtyPath => (
- {dirtyPath} + {withVisibleControls(dirtyPath)}
))}
diff --git a/src/renderer/src/features/settings/ui/AgentCodeInstalledSkillsRow.tsx b/src/renderer/src/features/settings/ui/AgentCodeInstalledSkillsRow.tsx index 74244e225..a6a6449e3 100644 --- a/src/renderer/src/features/settings/ui/AgentCodeInstalledSkillsRow.tsx +++ b/src/renderer/src/features/settings/ui/AgentCodeInstalledSkillsRow.tsx @@ -23,6 +23,7 @@ import type { AgentCodeInstalledSkillsSnapshot, AgentCodeInstalledSkillUpdateResult, } from '@shared/types/agentCodeInstalledSkills.js' +import { withVisibleControls } from '@shared/text/visibleControls' const HEALTH_LABELS: Record = { disabled: 'Disabled', @@ -463,10 +464,15 @@ function DiscoveryReview({ function CandidateDetails({ candidate }: { candidate: AgentCodeInstalledSkillCandidate }) { return (
+ {/* The skill NAME is validated ASCII, but nothing else here is: the + description, the source path and the file list all come from the + repository being installed, and this panel is the review the user + approves. `scripts/check.sh` and `scripts/check.sh` are + different files that render identically (#1049 re-review). */}
{candidate.name}
-
{candidate.description}
+
{withVisibleControls(candidate.description)}
- {candidate.source.path || 'repository root'} · {candidate.files.length} files · {formatBytes(candidate.totalBytes)} + {withVisibleControls(candidate.source.path) || 'repository root'} · {candidate.files.length} files · {formatBytes(candidate.totalBytes)}
{candidate.warnings.length > 0 ? (
    @@ -477,7 +483,7 @@ function CandidateDetails({ candidate }: { candidate: AgentCodeInstalledSkillCan Review package files
      {candidate.files.map(file => ( -
    • {file.executable ? 'executable · ' : ''}{file.path} · {formatBytes(file.bytes)}
    • +
    • {file.executable ? 'executable · ' : ''}{withVisibleControls(file.path)} · {formatBytes(file.bytes)}
    • ))}
    @@ -489,7 +495,7 @@ function UpdateReviewPanel({ review }: { review: UpdateReview }) { return ( <>
    -
    {review.candidate.source.repositoryUrl}
    +
    {withVisibleControls(review.candidate.source.repositoryUrl)}
    New commit {review.candidate.source.resolvedCommit.slice(0, 12)}
    @@ -506,7 +512,7 @@ function ChangeList({ title, paths }: { title: string; paths: string[] }) { return (
    {title} · {paths.length}
    - {paths.length > 0 ?
      {paths.map(path =>
    • {path}
    • )}
    : null} + {paths.length > 0 ?
      {paths.map(path =>
    • {withVisibleControls(path)}
    • )}
    : null}
    ) } diff --git a/src/renderer/src/features/workspace/ui/BulkProviderSwitchModal.tsx b/src/renderer/src/features/workspace/ui/BulkProviderSwitchModal.tsx index 4a2bd2393..3d30abcf9 100644 --- a/src/renderer/src/features/workspace/ui/BulkProviderSwitchModal.tsx +++ b/src/renderer/src/features/workspace/ui/BulkProviderSwitchModal.tsx @@ -29,6 +29,7 @@ import { deriveProviderExhaustion } from '@shared/usage/exhaustion' import { estimateLiveEntriesBytes } from '@renderer/session-runtime/liveEntryWindow' import { isLimitIdle } from '@renderer/workspace/hook/actions/providerSwitchCore' import { useGlobalToast } from '@renderer/ui/GlobalToast' +import { withVisibleControls } from '@shared/text/visibleControls' // Switch Agents modal — bulk provider switch + remembered-batch return. // @@ -881,11 +882,13 @@ export function BulkProviderSwitchModal({ open, workspace, onClose }: Props) { {/* The Dispatch vocabulary (A · title), so this picker names projects the way the index does. Worktrees appear below as directories inside the project. */} + {/* Which projects the batch switch or compact will + touch (#1049 re-review). */} - {project.label} + {withVisibleControls(project.label)} - {project.directories.join(' · ')} + {withVisibleControls(project.directories.join(' · '))} @@ -935,9 +938,9 @@ export function BulkProviderSwitchModal({ open, workspace, onClose }: Props) {
    -
    {row.cwdBase}
    +
    {withVisibleControls(row.cwdBase)}
    - {tabIndexLabel(row.tabIndex)} · {row.tabTitle} · {row.cwd} + {tabIndexLabel(row.tabIndex)} · {withVisibleControls(row.tabTitle)} · {withVisibleControls(row.cwd)}
    diff --git a/src/renderer/src/features/workspace/ui/MergeProjectTabsModal.tsx b/src/renderer/src/features/workspace/ui/MergeProjectTabsModal.tsx index f02d18d7b..ea8a2314c 100644 --- a/src/renderer/src/features/workspace/ui/MergeProjectTabsModal.tsx +++ b/src/renderer/src/features/workspace/ui/MergeProjectTabsModal.tsx @@ -10,6 +10,7 @@ import { DialogTitle, } from '@renderer/components/ui/dialog' import type { TabId } from '@renderer/workspace/types' +import { withVisibleControls } from '@shared/text/visibleControls' export type MergeTabOption = { id: TabId @@ -119,8 +120,10 @@ function MergeDraft({ tabs, initialTargetId, onCancel, onConfirm }: Omit + {/* Source and destination of a merge that moves every agent + (#1049 re-review). */} {tabs.map(tab => ( - + ))}
    @@ -148,8 +151,8 @@ function MergeDraft({ tabs, initialTargetId, onCancel, onConfirm }: Omit - {tab.label} - {tab.directories.join(' · ')} + {withVisibleControls(tab.label)} + {withVisibleControls(tab.directories.join(' · '))} {tab.sessionCount} @@ -160,7 +163,7 @@ function MergeDraft({ tabs, initialTargetId, onCancel, onConfirm }: Omit {sources.length === 0 || !target ? 'Tick at least one tab to merge.' - : `${sources.length} tab${sources.length === 1 ? '' : 's'}, ${movedCount} agent${movedCount === 1 ? '' : 's'} move to ${target.label}.`} + : `${sources.length} tab${sources.length === 1 ? '' : 's'}, ${movedCount} agent${movedCount === 1 ? '' : 's'} move to ${withVisibleControls(target.label)}.`}
diff --git a/src/renderer/src/features/workspace/ui/NewAgentInDialog.tsx b/src/renderer/src/features/workspace/ui/NewAgentInDialog.tsx index 164378229..5c39f721f 100644 --- a/src/renderer/src/features/workspace/ui/NewAgentInDialog.tsx +++ b/src/renderer/src/features/workspace/ui/NewAgentInDialog.tsx @@ -18,6 +18,7 @@ import { import { AGENT_PROVIDER_CHOICES } from '@renderer/workspace/providerChoices' import type { TabId } from '@renderer/workspace/types' import type { Workspace } from '@renderer/workspace/workspaceStore' +import { withVisibleControls } from '@shared/text/visibleControls' type Props = { open: boolean @@ -277,7 +278,7 @@ export function NewAgentInDialog({ open, workspace, onClose }: Props) { {/* Same "A · title" vocabulary as the Dispatch index and the row-project picker, so a project has one name everywhere. */}
- {`${project.label} · ${project.title}`} + {withVisibleControls(`${project.label} · ${project.title}`)}
{project.disabledReason ? (
{project.disabledReason}
diff --git a/src/renderer/src/features/workspace/ui/ProviderSwitchPickerModal.tsx b/src/renderer/src/features/workspace/ui/ProviderSwitchPickerModal.tsx index 038ca3936..78d6b9cd0 100644 --- a/src/renderer/src/features/workspace/ui/ProviderSwitchPickerModal.tsx +++ b/src/renderer/src/features/workspace/ui/ProviderSwitchPickerModal.tsx @@ -18,6 +18,7 @@ import { import type { Workspace } from '@renderer/workspace/workspaceStore' import type { SessionId } from '@renderer/workspace/types' import { isAgentProviderKind } from '@shared/types/providerKind' +import { withVisibleControls } from '@shared/text/visibleControls' type Props = { open: boolean @@ -121,7 +122,9 @@ export function ProviderSwitchPickerModal({ Switch Provider
-
Current: {currentLabel}{cwdBase ? ` · ${cwdBase}` : ''}
+ {/* Names the conversation being moved to another provider + (#1049 re-review). */} +
Current: {withVisibleControls(currentLabel)}{cwdBase ? ` · ${withVisibleControls(cwdBase)}` : ''}
Choose where this conversation should continue.
diff --git a/src/renderer/src/features/workspace/ui/RewindToPromptModal.tsx b/src/renderer/src/features/workspace/ui/RewindToPromptModal.tsx index 985519382..4f6084891 100644 --- a/src/renderer/src/features/workspace/ui/RewindToPromptModal.tsx +++ b/src/renderer/src/features/workspace/ui/RewindToPromptModal.tsx @@ -15,6 +15,7 @@ import { PromptList } from '@renderer/features/conversations/ui/PromptList' import type { Workspace } from '@renderer/workspace/workspaceStore' import type { SessionId } from '@renderer/workspace/types' import { resumableProviderSessionId } from '@renderer/workspace/providerSessionIdentity' +import { withVisibleControls } from '@shared/text/visibleControls' // RewindToPromptModal — picker for the rewind-to-prompt flow. // @@ -153,8 +154,10 @@ export function RewindToPromptModal({ Rewind to Prompt
-
{meta.kind ?? DEFAULT_PROVIDER} · {cwdBase}
-
{meta.cwd}
+ {/* Names the session whose history is about to be discarded + back to a chosen prompt (#1049 re-review). */} +
{meta.kind ?? DEFAULT_PROVIDER} · {withVisibleControls(cwdBase)}
+
{withVisibleControls(meta.cwd)}
diff --git a/src/renderer/src/features/workspace/ui/RootManagementConfirmDialog.renderer.test.tsx b/src/renderer/src/features/workspace/ui/RootManagementConfirmDialog.renderer.test.tsx index f555849ed..62d3b3435 100644 --- a/src/renderer/src/features/workspace/ui/RootManagementConfirmDialog.renderer.test.tsx +++ b/src/renderer/src/features/workspace/ui/RootManagementConfirmDialog.renderer.test.tsx @@ -33,6 +33,26 @@ describe('Root Agent Code Management confirmation', () => { expect(onConfirm).toHaveBeenCalledOnce() }) + it('escapes the recipient identity, the broadest grant in the app (#1049 re-review)', () => { + // This dialog hands ONE agent authority over the whole application. The + // label and directory are its own title and cwd, which an agent can set: + // `Trusted auditor\u200B` renders exactly like a different, trusted agent. + render( + , + ) + const dialog = screen.getByRole('dialog') + expect(dialog.textContent).not.toContain('\u200B') + expect(dialog.textContent).not.toContain('\u202E') + expect(dialog.textContent).toContain('U+200B ZWSP') + expect(dialog.textContent).toContain('U+202E RLO') + }) + it('treats cancel as a decline that grants nothing', () => { const onCancel = vi.fn() const onConfirm = vi.fn() diff --git a/src/renderer/src/features/workspace/ui/RootManagementConfirmDialog.tsx b/src/renderer/src/features/workspace/ui/RootManagementConfirmDialog.tsx index b888b07c9..e51f7b25d 100644 --- a/src/renderer/src/features/workspace/ui/RootManagementConfirmDialog.tsx +++ b/src/renderer/src/features/workspace/ui/RootManagementConfirmDialog.tsx @@ -9,6 +9,7 @@ import { DialogHeader, DialogTitle, } from '@renderer/components/ui/dialog' +import { withVisibleControls } from '@shared/text/visibleControls' type Props = { open: boolean @@ -62,8 +63,13 @@ export function RootManagementConfirmDialog({ Enable Root Agent Code Management?
-
{agentLabel}
-
{description}
+ {/* The label and directory of the agent about to be granted + application-wide management authority. A zero-width + character in a session title hides a different identity + behind a familiar-looking one, immediately before the + broadest grant this app offers (#1049 re-review). */} +
{withVisibleControls(agentLabel)}
+
{withVisibleControls(description)}
From 42cb518a22d1bd50a9728122aa4d6ac8a4db7772 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 19 Sep 2026 21:19:10 -0700 Subject: [PATCH 05/10] fix(approvals): escape the skill review's remaining repository-authored text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-4 review, two more in the same panel. A repository supplies its own default branch name, its notices and its warnings, and all three sit in the lines a user reads before approving an install, an update or an Enable: `main` reads as `main`, and a warning about `scripts/check.sh` reads as one about `scripts/check.sh` while the collapsed file list beside it already showed the real name. The installed skill's description and source URL are escaped for the same reason — they are the identity next to Enable, Disable and Remove. Co-Authored-By: Claude Opus 5 (1M context) --- .../ui/AgentCodeInstalledSkillsRow.tsx | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/renderer/src/features/settings/ui/AgentCodeInstalledSkillsRow.tsx b/src/renderer/src/features/settings/ui/AgentCodeInstalledSkillsRow.tsx index a6a6449e3..1a40bcab5 100644 --- a/src/renderer/src/features/settings/ui/AgentCodeInstalledSkillsRow.tsx +++ b/src/renderer/src/features/settings/ui/AgentCodeInstalledSkillsRow.tsx @@ -436,14 +436,18 @@ function DiscoveryReview({ return ( <>
-
{discovery.repositoryUrl}
+ {/* The repository, the ref and the notices are all attacker-chosen: + a default branch named `main` reads as `main` in the line + the user approves an installation from (#1049 re-review). Only the + resolved commit is a hash we computed. */} +
{withVisibleControls(discovery.repositoryUrl)}
- {discovery.requestedRefType === 'branch' ? 'Branch' : 'Tag'} {discovery.requestedRef} + {discovery.requestedRefType === 'branch' ? 'Branch' : 'Tag'} {withVisibleControls(discovery.requestedRef)} {' · '}commit {discovery.resolvedCommit.slice(0, 12)}
{discovery.notices.map(notice => ( -
{notice}
+
{withVisibleControls(notice)}
))} {discovery.candidates.map(candidate => (
@@ -476,7 +480,11 @@ function CandidateDetails({ candidate }: { candidate: AgentCodeInstalledSkillCan
{candidate.warnings.length > 0 ? (
    - {candidate.warnings.map(warning =>
  • {warning}
  • )} + {/* A warning names the file it is warning ABOUT, so an unescaped + one can describe `scripts/check.sh` while meaning a different + file — the collapsed list below escapes it, this line did not + (#1049 re-review). */} + {candidate.warnings.map(warning =>
  • {withVisibleControls(warning)}
  • )}
) : null}
@@ -542,8 +550,10 @@ function InstalledSkillRow({
{skill.name}
-
{skill.description}
-
{skill.source.skillUrl}
+ {/* Beside Enable / Disable / Remove: the identity the user acts on + (#1049 re-review). */} +
{withVisibleControls(skill.description)}
+
{withVisibleControls(skill.source.skillUrl)}
{HEALTH_LABELS[skill.health]} · commit {skill.source.resolvedCommit.slice(0, 12)} · {skill.files.length} files · {formatBytes(skill.totalBytes)}
@@ -563,7 +573,7 @@ function InstalledSkillRow({
{skill.warnings.length > 0 ? (
    - {skill.warnings.map(warning =>
  • {warning}
  • )} + {skill.warnings.map(warning =>
  • {withVisibleControls(warning)}
  • )}
) : null} From 356752436f500d875ff937e9ae92e5a77ea205d8 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 19 Sep 2026 21:37:40 -0700 Subject: [PATCH 06/10] fix(approvals): escape the conventions target path shown beside Replace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-5 review. The path comes from the provider's own config resolution — CLAUDE_CONFIG_DIR=~/.claude makes a different directory read as ~/.claude — and it identifies the file the buttons beside it reveal and OVERWRITE. Both the installation row and the confirm() text are escaped; every action still uses the target id. Co-Authored-By: Claude Opus 5 (1M context) --- .../settings/ui/AgentCodeConventionsEditorModal.tsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/renderer/src/features/settings/ui/AgentCodeConventionsEditorModal.tsx b/src/renderer/src/features/settings/ui/AgentCodeConventionsEditorModal.tsx index 295420ea9..a7e652aee 100644 --- a/src/renderer/src/features/settings/ui/AgentCodeConventionsEditorModal.tsx +++ b/src/renderer/src/features/settings/ui/AgentCodeConventionsEditorModal.tsx @@ -15,6 +15,7 @@ import { type AgentCodeConventionsMutationResult, type AgentCodeConventionsSnapshot, } from '@shared/types/agentCodeConventions.js' +import { withVisibleControls } from '@shared/text/visibleControls' type Props = { open: boolean @@ -268,7 +269,13 @@ export function AgentCodeConventionsEditorModal({
Installations
{shownSnapshot.targets.map(target => (
- {target.displayPath || target.id} · {target.state} + {/* The path comes from the provider's own config + resolution — `CLAUDE_CONFIG_DIR=~/.claude` makes + a different directory read as `~/.claude` (#1049 + re-review) — and it is what identifies the file the + buttons beside it reveal and OVERWRITE. Display only: + every action still uses `target.id`. */} + {withVisibleControls(target.displayPath || target.id)} · {target.state} {(target.state === 'conflict' || target.state === 'retired') ? ( <> @@ -277,7 +284,7 @@ export function AgentCodeConventionsEditorModal({ type="button" className="rounded-control border border-danger px-1.5 py-0.5 text-danger" onClick={() => { - if (!window.confirm(`Replace the reviewed file at ${target.displayPath}?`)) return + if (!window.confirm(`Replace the reviewed file at ${withVisibleControls(target.displayPath)}?`)) return const next = [ ...overwriteApprovals.filter(value => value.targetId !== target.id), { targetId: target.id, expectedConflictFingerprint: target.conflictFingerprint! }, From bd62a432d2fca5d66c58f051d2e1cec0b2a14fbe Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 19 Sep 2026 22:02:28 -0700 Subject: [PATCH 07/10] fix(approvals): escape key-vault names and skill deployment paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-6 sweep. Key names and provider names are user-typed and the validator permits invisible characters, so `production` and `production` coexist and read identically — in the list and in the confirmation that destroys a provider's whole key set. Skill deployment paths sit beside the controls that reveal, remove and forget them, and CLAUDE_CONFIG_DIR can make a different directory read as ~/.claude. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/features/key-vault/ui/KeyVaultModal.tsx | 11 +++++++++-- .../features/settings/ui/AgentCodeCustomSkillsRow.tsx | 3 ++- .../settings/ui/AgentCodeInstalledSkillsRow.tsx | 4 +++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/renderer/src/features/key-vault/ui/KeyVaultModal.tsx b/src/renderer/src/features/key-vault/ui/KeyVaultModal.tsx index c9a330275..da8fa71f0 100644 --- a/src/renderer/src/features/key-vault/ui/KeyVaultModal.tsx +++ b/src/renderer/src/features/key-vault/ui/KeyVaultModal.tsx @@ -14,6 +14,7 @@ import { deliverTextToSession } from '@renderer/features/session-text-delivery/d import { commandTargetSessionId } from '@renderer/workspace/hook/selectors/commandTargetSessionId' import { useWorkspaceLayoutContext } from '@renderer/workspace/WorkspaceContext' import type { KeyVaultKey, KeyVaultStatus } from '@shared/types/keyVault' +import { withVisibleControls } from '@shared/text/visibleControls' // API Key Vault modal (#831). Revealed plaintext lives only in this // component's ephemeral state — the VAULT never persists it — and is @@ -341,7 +342,13 @@ export function KeyVaultModal() { )} {entry.isDirectory && isExpanded && ( diff --git a/src/renderer/src/features/goal-loop/GoalLoopPane.tsx b/src/renderer/src/features/goal-loop/GoalLoopPane.tsx index 95bf77fd8..b4577d4ca 100644 --- a/src/renderer/src/features/goal-loop/GoalLoopPane.tsx +++ b/src/renderer/src/features/goal-loop/GoalLoopPane.tsx @@ -4,6 +4,7 @@ import { GOAL_LOOP_MAX_CONTINUATIONS_CEILING } from '@shared/types/goalLoop' import { useAgentTerminalOwnerVisible } from '@renderer/workspace/terminal/AgentTerminalOwnership' import type { GoalLoopControlAction, GoalLoopState } from '@shared/types/goalLoop' import { dismissGoalLoop, useGoalLoopView } from './viewState' +import { withVisibleControls } from '@shared/text/visibleControls' const PHASE_LABEL: Record = { active: 'active', paused: 'paused', ended: 'ended', @@ -120,7 +121,9 @@ export function GoalLoopPane({ sessionId }: { sessionId: string }) { onMouseDown={event => event.stopPropagation()} onClick={event => event.stopPropagation()} > - Goal loop · {PHASE_LABEL[loop.phase]} · {describe(loop)} · {loop.goal} + {/* The goal is agent-authored and sits beside Resume, Raise cap and Stop + — the controls that grant it more turns (#1049 re-review). */} + Goal loop · {PHASE_LABEL[loop.phase]} · {describe(loop)} · {withVisibleControls(loop.goal)} {loop.phase === 'active' && } {loop.phase === 'paused' && } @@ -138,9 +141,9 @@ export function GoalLoopPane({ sessionId }: { sessionId: string }) { {strip}

Goal loop · {PHASE_LABEL[loop.phase]}{loop.phase === 'paused' ? ` · ${loop.pauseReason}` : ''}

-

{loop.goal}

+

{withVisibleControls(loop.goal)}

{describe(loop)} continuations · started {loop.startedAt}

- {loop.completionSummary &&

{loop.endReason}: {loop.completionSummary}

} + {loop.completionSummary &&

{loop.endReason}: {withVisibleControls(loop.completionSummary)}

}
{loop.phase === 'active' && } {loop.phase === 'paused' && } diff --git a/src/renderer/src/features/key-vault/ui/KeyVaultModal.tsx b/src/renderer/src/features/key-vault/ui/KeyVaultModal.tsx index da8fa71f0..8a7a73fff 100644 --- a/src/renderer/src/features/key-vault/ui/KeyVaultModal.tsx +++ b/src/renderer/src/features/key-vault/ui/KeyVaultModal.tsx @@ -310,7 +310,10 @@ export function KeyVaultModal() { onClick={() => { setSelectedProviderId(provider.id); setKeyForm(null); setProviderRename(null) }} title={provider.name} > - {provider.name} + {/* The row the Delete below acts on: escaped here too, so + the list and the confirmation agree (#1049 + re-review). */} + {withVisibleControls(provider.name)} ))} (
- {key.name} + {withVisibleControls(key.name)} ••••{key.hint} {revealed.has(key.id) && ( // WHY no `title` attribute here, and why it wraps diff --git a/src/renderer/src/features/path-picker/ui/PathInput.tsx b/src/renderer/src/features/path-picker/ui/PathInput.tsx index 515404fd2..5f5298c9e 100644 --- a/src/renderer/src/features/path-picker/ui/PathInput.tsx +++ b/src/renderer/src/features/path-picker/ui/PathInput.tsx @@ -1,5 +1,6 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import type { CSSProperties } from 'react' +import { withVisibleControls } from '@shared/text/visibleControls' // PathInput — the path picker's path-with-completion input. // @@ -290,7 +291,12 @@ export function PathInput({ {s.isDirectory ? '▸' : '·'} - {s.name} + {/* Choosing the working directory IS the trust decision for + Codex (ensureCodexProjectTrust runs on it before spawn), + so `project` and `project` must not read the same + here (#1049 re-review). The completion still inserts the + real name. */} + {withVisibleControls(s.name)} {s.isDirectory ? '/' : ''}
diff --git a/src/renderer/src/features/path-picker/ui/PathPickerModal.tsx b/src/renderer/src/features/path-picker/ui/PathPickerModal.tsx index 7bfd1686b..d7398e4de 100644 --- a/src/renderer/src/features/path-picker/ui/PathPickerModal.tsx +++ b/src/renderer/src/features/path-picker/ui/PathPickerModal.tsx @@ -17,6 +17,7 @@ import { ConversationRow } from '@renderer/features/conversations/ui/Conversatio // picker renders, so a session looks the same in both places and the label, // provenance and ordering decisions live in main, not here. import type { Conversation } from '@shared/conversations/types' +import { withVisibleControls } from '@shared/text/visibleControls' // PathPickerModal — modal that asks the user for a working directory // when they press ⌘T (or click the + button in the tab bar). @@ -386,7 +387,8 @@ export function PathPickerModal({ ) : pendingCreatePath ? ( Will create:{' '} - {pendingCreatePath} + {/* The directory this is about to make and then trust. */} + {withVisibleControls(pendingCreatePath)} ) : ( diff --git a/src/renderer/src/features/prompt-templates/ui/PromptTemplateManagerPane.tsx b/src/renderer/src/features/prompt-templates/ui/PromptTemplateManagerPane.tsx index 9ae72b8be..630c07e10 100644 --- a/src/renderer/src/features/prompt-templates/ui/PromptTemplateManagerPane.tsx +++ b/src/renderer/src/features/prompt-templates/ui/PromptTemplateManagerPane.tsx @@ -2,6 +2,7 @@ import type { ReactNode } from 'react' import { Button } from '@renderer/components/ui/button' import type { PromptTemplate } from '@renderer/features/prompt-templates/types' +import { withVisibleControls } from '@shared/text/visibleControls' type Props = { templates: PromptTemplate[] @@ -125,7 +126,7 @@ function TemplateRow({
-
{preview}
+ {/* Same as the custom-skill preview: this is what Save & Enable + writes into every provider's skills directory (#1049 + re-review). */} +
{withVisibleControls(preview)}
) : ( <> diff --git a/src/renderer/src/features/settings/ui/AgentCodeConventionsRow.tsx b/src/renderer/src/features/settings/ui/AgentCodeConventionsRow.tsx index b75c957c6..d5419c0a9 100644 --- a/src/renderer/src/features/settings/ui/AgentCodeConventionsRow.tsx +++ b/src/renderer/src/features/settings/ui/AgentCodeConventionsRow.tsx @@ -11,6 +11,7 @@ import type { AgentCodeConventionsSnapshot, } from '@shared/types/agentCodeConventions.js' import { AgentCodeConventionsEditorModal } from './AgentCodeConventionsEditorModal' +import { withVisibleControls } from '@shared/text/visibleControls' // See docs/design/agent-code-conventions.md. This row displays main-owned // desired state and deployment health; it must never persist a shadow toggle. @@ -166,7 +167,9 @@ export function AgentCodeConventionsRow() { onClick={() => void window.api.revealAgentCodeConventionsTarget(target.id)} className="min-w-0 truncate text-right text-control-fg hover:text-ink" > - {target.displayPath || target.state} · {target.state} + {/* The deployment path beside this row's own controls + (#1049 re-review). */} + {withVisibleControls(target.displayPath || target.state)} · {target.state}
))} diff --git a/src/renderer/src/features/settings/ui/AgentCodeCustomSkillsRow.tsx b/src/renderer/src/features/settings/ui/AgentCodeCustomSkillsRow.tsx index a74f49cd2..c25a92a54 100644 --- a/src/renderer/src/features/settings/ui/AgentCodeCustomSkillsRow.tsx +++ b/src/renderer/src/features/settings/ui/AgentCodeCustomSkillsRow.tsx @@ -371,7 +371,11 @@ function AgentCodeCustomSkillsModal({ Generated SKILL.md preview
-
{preview}
+ {/* The generated file, read immediately before Save & + Enable deploys it to every provider's skills directory. + Display only — the editor model and the saved bytes are + untouched (#1049 re-review). */} +
{withVisibleControls(preview)}
) : (