fix: avoid shell-escaping special characters except quotes by lifeizhou-ap · Pull Request #7242 · block/goose

lifeizhou-ap · 2026-02-16T04:52:28Z

Summary

Fixed #6816 with the display issue.

Replace shell-quote's quote() with a simple quoting function for displaying extension commands in the UI. The shell quote function does more escape which is not necessary for display eg: "@" -> "//@". Keep shell-quote's parse() for command parsing on user input.

Type of Change

Testing

Unit test and manual testing

Screenshots/Demos (for UX changes)

Before:

After:

…scape the whole command line for extension

Copilot

Pull request overview

This PR fixes a Windows-specific bug where extensions failed to load due to shell-quote's escaping behavior. The shell-quote library was escaping special characters like @ to \@, which broke Windows command execution for npm packages like @upstash/context7-mcp.

Changes:

Removed the shell-quote npm dependency and its usage
Implemented custom splitCmdAndArgs and combineCmdAndArgs functions for parsing and formatting command strings
Added tests covering the specific @ character case and roundtrip behavior

Reviewed changes

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

Show a summary per file

File	Description
ui/desktop/src/components/settings/extensions/utils.ts	Removed shell-quote dependency, implemented custom parsing/quoting logic that doesn't escape special characters unnecessarily
ui/desktop/src/components/settings/extensions/utils.test.ts	Added tests for @ character handling and roundtrip conversion
ui/desktop/src/components/settings/extensions/subcomponents/ExtensionList.tsx	Updated to use new combineCmdAndArgs function for display
ui/desktop/package.json	Removed shell-quote dependency
ui/desktop/package-lock.json	Removed shell-quote package lock entries

Files not reviewed (1)

ui/desktop/package-lock.json: Language not supported

Copilot · 2026-02-16T04:57:52Z

ui/desktop/src/components/settings/extensions/utils.ts

+    } else {
+      current += ch;
+    }
  }


The parser silently accepts unclosed quotes. If a user enters npx -y "@upstash/context7-mcp (with unclosed double quote), everything after the opening quote becomes part of a single argument, which may not be the intended behavior. Consider detecting unclosed quotes and either throwing an error or treating the quote as a literal character.

Suggested change

}

}

// If we reach the end of the string while still inside a quote, treat quotes as literals

// by falling back to a simple whitespace-based split.

if (inSingle || inDouble) {

const fallbackParts = trimmed.split(/\s+/);

return { cmd: fallbackParts[0] || '', args: fallbackParts.slice(1) };

}

Copilot · 2026-02-16T04:57:52Z

ui/desktop/src/components/settings/extensions/utils.ts

+  return [cmd, ...args]
+    .map((a) => {
+      if (!a.includes(' ')) return a;
+      if (a.includes('"')) return `'${a}'`;


The quoting logic has a flaw: if an argument contains both a space and a double quote, it wraps the arg in single quotes. However, if that arg also contains a single quote (e.g., it's "quoted"), the round-trip through combineCmdAndArgs and splitCmdAndArgs will fail because splitCmdAndArgs will end the quote at the apostrophe. Consider implementing escape sequences for quotes, or using a more robust approach that can handle all combinations of special characters.

Suggested change

if (a.includes('"')) return `'${a}'`;

if (a.includes('"')) {

// If the argument contains both single and double quotes, escape double quotes and use double quotes outside

if (a.includes("'")) {

const escaped = a.replace(/"/g, '\\"');

return `"${escaped}"`;

}

// Safe to wrap in single quotes when there are no embedded single quotes

return `'${a}'`;

}

Copilot · 2026-02-16T04:57:52Z

ui/desktop/src/components/settings/extensions/utils.ts

 export function splitCmdAndArgs(str: string): { cmd: string; args: string[] } {
  const trimmed = str.trim();
-  if (!trimmed) {
-    return { cmd: '', args: [] };
+  if (!trimmed) return { cmd: '', args: [] };
+
+  const parts: string[] = [];
+  let current = '';
+  let inSingle = false;
+  let inDouble = false;
+
+  for (const ch of trimmed) {
+    if (inSingle) {
+      if (ch === "'") inSingle = false;
+      else current += ch;
+    } else if (inDouble) {
+      if (ch === '"') inDouble = false;
+      else current += ch;
+    } else if (ch === "'") {
+      inSingle = true;
+    } else if (ch === '"') {
+      inDouble = true;
+    } else if (/\s/.test(ch)) {
+      if (current) {
+        parts.push(current);
+        current = '';
+      }
+    } else {
+      current += ch;
+    }
  }
+  if (current) parts.push(current);

-  const parsed = parseShellQuote(trimmed);
-  const words = parsed.filter((item): item is string => typeof item === 'string').map(String);
-
-  const cmd = words[0] || '';
-  const args = words.slice(1);
-
-  return {
-    cmd,
-    args,
-  };
+  return { cmd: parts[0] || '', args: parts.slice(1) };
 }


The new quote parsing logic lacks test coverage for edge cases like unclosed quotes (e.g., npx -y "@upstash/context7-mcp), arguments with both single and double quotes (e.g., it's "quoted"), and empty quotes. Given that shell-quote was replaced with custom parsing, these edge cases should be explicitly tested to prevent regressions.

…our to escape the whole command line for extension" This reverts commit 2a742f5.

* main: fix: handle reasoning_content for Kimi/thinking models (#7252) feat: sandboxing for macos (#7197) fix(otel): use monotonic_counter prefix and support temporality env var (#7234) Streaming markdown (#7233) Improve compaction messages to enable better post-compaction agent behavior (#7259) fix: avoid shell-escaping special characters except quotes (#7242)

* origin/main: docs: playwright CLI skill tutorial (#7261) install node in goose dir (#7220) fix: relax test_basic_response assertion for providers returning reasoning_content (#7249) fix: handle reasoning_content for Kimi/thinking models (#7252) feat: sandboxing for macos (#7197) fix(otel): use monotonic_counter prefix and support temporality env var (#7234) Streaming markdown (#7233) Improve compaction messages to enable better post-compaction agent behavior (#7259) fix: avoid shell-escaping special characters except quotes (#7242) fix: use dynamic port for Tetrate auth callback server (#7228) docs: removing LLM Usage admonitions (#7227) feat(otel): respect standard OTel env vars for exporter selection (#7144) fix: fork session (#7219) Bump version numbers for 1.24.0 release (#7214) Move platform extensions into their own folder (#7210) fix: ignore deprecated skills extension (#7139) # Conflicts: # Cargo.lock # Cargo.toml

…led-extensions-cmd * 'main' of github.com:block/goose: (24 commits) Set up direnv and update flake inputs (#6526) fix: restore subagent tool call notifications after summon refactor (#7243) fix(ui): preserve server config values on partial provider config save (#7248) fix(claude-code): allow goose to run inside a Claude Code session (#7232) fix(openai): route gpt-5 codex via responses and map base paths (#7254) feat: add GoosePlatform to AgentConfig and MCP initialization (#6931) Fix copied over (#7270) feat(gemini-cli): add streaming support via stream-json events (#7244) fix: filter models without tool support from recommended list (#7198) fix(google): handle more thoughtSignature vagaries during streaming (#7204) docs: playwright CLI skill tutorial (#7261) install node in goose dir (#7220) fix: relax test_basic_response assertion for providers returning reasoning_content (#7249) fix: handle reasoning_content for Kimi/thinking models (#7252) feat: sandboxing for macos (#7197) fix(otel): use monotonic_counter prefix and support temporality env var (#7234) Streaming markdown (#7233) Improve compaction messages to enable better post-compaction agent behavior (#7259) fix: avoid shell-escaping special characters except quotes (#7242) fix: use dynamic port for Tetrate auth callback server (#7228) ...

removed dependency on shell quote as it has unexpected behaviour to e…

2a742f5

…scape the whole command line for extension

Copilot AI review requested due to automatic review settings February 16, 2026 04:52

Copilot started reviewing on behalf of lifeizhou-ap February 16, 2026 04:53 View session

Copilot AI reviewed Feb 16, 2026

View reviewed changes

lifeizhou-ap added 2 commits February 16, 2026 16:50

Revert "removed dependency on shell quote as it has unexpected behavi…

e272ebb

…our to escape the whole command line for extension" This reverts commit 2a742f5.

avoid shell-escaping special characters except quotes

6aead6d

lifeizhou-ap changed the title ~~fix: removed dependency on shell quote as it has unexpected behaviour to escape the whole command line for extension~~ fix: avoid shell-escaping special characters except quotes Feb 16, 2026

DOsinga approved these changes Feb 16, 2026

View reviewed changes

lifeizhou-ap added this pull request to the merge queue Feb 16, 2026

Merged via the queue into main with commit b9c3ce2 Feb 16, 2026
19 of 20 checks passed

lifeizhou-ap deleted the lifei/extension-config-escape-char branch February 16, 2026 09:20

github-actions bot mentioned this pull request Feb 17, 2026

chore(release): release version 1.25.0 (minor) #7263

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: avoid shell-escaping special characters except quotes#7242

fix: avoid shell-escaping special characters except quotes#7242
lifeizhou-ap merged 3 commits intomainfrom
lifei/extension-config-escape-char

lifeizhou-ap commented Feb 16, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 16, 2026

Uh oh!

Copilot AI Feb 16, 2026

Uh oh!

Copilot AI Feb 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

-  }
+  }
+  // If we reach the end of the string while still inside a quote, treat quotes as literals
+  // by falling back to a simple whitespace-based split.
+  if (inSingle || inDouble) {
+    const fallbackParts = trimmed.split(/\s+/);
+    return { cmd: fallbackParts[0] || '', args: fallbackParts.slice(1) };
+  }

-      if (a.includes('"')) return `'${a}'`;
+      if (a.includes('"')) {
+        // If the argument contains both single and double quotes, escape double quotes and use double quotes outside
+        if (a.includes("'")) {
+          const escaped = a.replace(/"/g, '\\"');
+          return `"${escaped}"`;
+        }
+        // Safe to wrap in single quotes when there are no embedded single quotes
+        return `'${a}'`;
+      }

Conversation

lifeizhou-ap commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Type of Change

Testing

Screenshots/Demos (for UX changes)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

lifeizhou-ap commented Feb 16, 2026 •

edited

Loading