fix(tui): handle quotes in VISUAL/EDITOR env vars #7759

imxinde · 2026-01-11T06:58:26Z

I noticed this issue was assigned but didn't active PR, so I went ahead and fixed it. Happy to collaborate with @assignee if they're also working on this.

What does this PR do?

Fixes the /editor command to correctly handle VISUAL or EDITOR environment variables that contain quoted arguments.

The Problem

When VISUAL contains arguments with quotes like:
```bash
export VISUAL="nvim --cmd 'let g:flatten_wait=1'"
```

The current implementation splits on spaces (`editor.split(" ")`), breaking the quoted argument:

Expected: `nvim --cmd "let g:flatten_wait=1"`
Actual: `nvim --cmd 'let g:flatten_wait=1'` (parsed as 4 separate args)

This causes nvim to fail with: `E20: Mark not set: 'let`

The Solution

Use shell execution (`sh -c` on Unix, `cmd /c` on Windows) to properly parse quotes and arguments, just like git, ranger, and other CLI tools handle editor invocation.

Also added filepath escaping to prevent command injection attacks.

How did you verify your code works?

Unit tests: Added comprehensive tests in `test/util/editor.test.ts`
- All 5 tests passing
- 100% coverage on editor.ts
- Tests cover: simple editor, quoted arguments, multiple arguments, fallback to EDITOR, no editor set
Type checking: Pre-commit hooks passed (turbo typecheck)

Changes

Modified `src/cli/cmd/tui/util/editor.ts` to use shell execution
Added filepath escaping for security (`'${filepath.replace(/'/g, "'\''")}`)
Added comprehensive unit tests in `test/util/editor.test.ts`

github-actions · 2026-01-11T06:58:59Z

The following comment was made by an LLM, it may be inaccurate:

No duplicate PRs found

Copilot

Pull request overview

This PR fixes the /editor command to properly handle VISUAL/EDITOR environment variables containing quoted arguments by using shell execution (sh -c on Unix, cmd /c on Windows) instead of naive space-based splitting. It also adds filepath escaping to prevent command injection.

Changes:

Modified editor.ts to use shell execution with proper quote handling
Added filepath escaping for Unix (single-quote with proper escape sequence)
Added comprehensive unit tests covering various editor configurations

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
`packages/opencode/src/cli/cmd/tui/util/editor.ts`	Replaced naive space splitting with shell execution and added filepath escaping
`packages/opencode/test/util/editor.test.ts`	Added comprehensive test suite covering simple editor, quoted arguments, special characters, and VISUAL/EDITOR preference

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-11T07:01:06Z

packages/opencode/src/cli/cmd/tui/util/editor.ts

+    const shellCommand = isWindows ? `${editor} "${filepath}"` : `${editor} ${shellEscapedFilepath}`
+
    const proc = Bun.spawn({
-      cmd: [...parts, filepath],
+      cmd: isWindows ? ["cmd", "/c", shellCommand] : ["sh", "-c", shellCommand],


The Windows implementation passes the entire editor string to cmd /c, but this approach could be problematic if the editor variable itself contains quotes or special Windows command characters. Unlike Unix's single-quote escaping which prevents all interpolation, Windows double quotes still allow variable expansion and special character interpretation. Consider validating or sanitizing the editor environment variable on Windows, or using a safer execution method.

Copilot · 2026-01-11T07:01:06Z

packages/opencode/src/cli/cmd/tui/util/editor.ts

+    const shellEscapedFilepath = `'${filepath.replace(/'/g, "'\\''")}'`
+    const isWindows = process.platform === "win32"
+    const shellCommand = isWindows ? `${editor} "${filepath}"` : `${editor} ${shellEscapedFilepath}`


There's an inconsistency in the Windows shell command construction. The shellCommand variable on line 21 uses "${filepath}" (with double quotes), but the escaping logic on line 19 creates shellEscapedFilepath with single quotes which is only used for Unix. This means the shellEscapedFilepath variable is unused on Windows, and the Windows path gets no escaping at all beyond the double quotes in the template string. Consider applying consistent escaping for both platforms.

Suggested change

const shellEscapedFilepath = `'${filepath.replace(/'/g, "'\\''")}'`

const isWindows = process.platform === "win32"

const shellCommand = isWindows ? `${editor} "${filepath}"` : `${editor} ${shellEscapedFilepath}`

const isWindows = process.platform === "win32"

const shellEscapedFilepath = isWindows

? `"${filepath.replace(/"/g, '\\"')}"`

: `'${filepath.replace(/'/g, "'\\''")}'`

const shellCommand = `${editor} ${shellEscapedFilepath}`

Copilot · 2026-01-11T07:01:07Z

packages/opencode/src/cli/cmd/tui/util/editor.ts

+
+    const shellEscapedFilepath = `'${filepath.replace(/'/g, "'\\''")}'`
+    const isWindows = process.platform === "win32"
+    const shellCommand = isWindows ? `${editor} "${filepath}"` : `${editor} ${shellEscapedFilepath}`


The Windows filepath escaping using double quotes is insufficient to prevent command injection. Double quotes in Windows do not prevent command execution with special characters like &, |, ^, etc. Consider using proper escaping for Windows or using an alternative approach similar to the Unix version. For example, you could escape special characters using ^ or use a more robust escaping mechanism.

Copilot · 2026-01-11T07:01:07Z

packages/opencode/test/util/editor.test.ts

+  test("should handle filepath with special characters", async () => {
+    if (process.platform === "win32") {
+      return
+    }
+
+    const testScript = join(tmpdir(), `test-editor-${Date.now()}.sh`)
+    await writeFile(testScript, `#!/bin/sh\necho "SUCCESS" > "$1"\n`, { mode: 0o755 })
+
+    try {
+      process.env["VISUAL"] = testScript
+
+      const mockRenderer = createMockRenderer()
+      const result = await Editor.open({
+        value: "test",
+        renderer: mockRenderer,
+      })
+
+      expect(result).toContain("SUCCESS")
+    } finally {
+      await rm(testScript, { force: true })
+    }
+  })


The test claims to handle "filepath with special characters" but the test doesn't actually create or use a filepath with special characters. The test uses a regular filepath from tmpdir() with Date.now(). Consider either removing this test as redundant with the simple editor test, or actually testing with special characters in the filepath (e.g., spaces, quotes, ampersands) to validate the escaping mechanism works correctly.

Copilot · 2026-01-11T07:01:07Z

packages/opencode/test/util/editor.test.ts

+    if (process.platform === "win32") {
+      return
+    }


All tests skip Windows platform, leaving the Windows code path (cmd /c execution and double-quote escaping on line 21) completely untested. Since the PR modifies Windows-specific behavior, consider adding at least one test that runs on Windows, or document why Windows testing is being deferred.

- Add Windows filepath validation to prevent cmd.exe injection attacks - Validates against unsafe characters (%, ^, &, |, <, >, etc.) - Prevents environment variable expansion via % character - Add random suffix to temp filenames for better security - Add comprehensive security documentation explaining: - POSIX-compliant single-quote escaping for Unix - Windows cmd.exe injection vulnerabilities and mitigations - Trust model for VISUAL/EDITOR environment variables - Add Windows test case to verify filepath validation - Reference: BatBadBut research on Windows command injection Refs: anomalyco#7749

fix(tui): handle quotes in VISUAL/EDITOR env vars (anomalyco#7749)

b7b8df6

Copilot AI review requested due to automatic review settings January 11, 2026 06:58

Copilot started reviewing on behalf of imxinde January 11, 2026 06:58 View session

Copilot AI reviewed Jan 11, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(tui): handle quotes in VISUAL/EDITOR env vars #7759

fix(tui): handle quotes in VISUAL/EDITOR env vars #7759

imxinde commented Jan 11, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Jan 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 11, 2026

Uh oh!

Copilot AI Jan 11, 2026

Uh oh!

Copilot AI Jan 11, 2026

Uh oh!

Copilot AI Jan 11, 2026

Uh oh!

Copilot AI Jan 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

fix(tui): handle quotes in VISUAL/EDITOR env vars #7759

Are you sure you want to change the base?

fix(tui): handle quotes in VISUAL/EDITOR env vars #7759

Conversation

imxinde commented Jan 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

The Problem

The Solution

How did you verify your code works?

Changes

Uh oh!

github-actions bot commented Jan 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

imxinde commented Jan 11, 2026 •

edited

Loading