Skip to content

Feature/merge costrae#182

Open
Askhz wants to merge 11 commits into
devfrom
feature/merge-costrae
Open

Feature/merge costrae#182
Askhz wants to merge 11 commits into
devfrom
feature/merge-costrae

Conversation

@Askhz

@Askhz Askhz commented Mar 20, 2026

Copy link
Copy Markdown
Contributor

Issue for this PR

Closes #

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Please provide a description of the issue, the changes you made to fix it, and why they work. It is expected that you understand why your changes work and if you do not understand why at least say as much so a maintainer knows how much to value the PR.

If you paste a large clearly AI generated description here your PR may be IGNORED or CLOSED!

How did you verify your code works?

Screenshots / recordings

If this is a UI change, please include a screenshot or recording.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

If you do not follow this template your PR will be automatically rejected.

})
.catch(() => { })
}
const restore = last ? bashPath(last).replace(/"/g, '\\"') : ""

Check failure

Code scanning / CodeQL

Incomplete string escaping or encoding High

This does not escape backslash characters in the input.

Copilot Autofix

AI 4 months ago

In general, when embedding an arbitrary string into a shell command inside double quotes, you must escape both the double quotes themselves and backslashes (and, if desired, other metacharacters) so that the shell interprets the content literally. Relying on .replace for only one character leads to incomplete escaping: a sequence like \" or \ at the end of the string can be interpreted in unexpected ways.

The best targeted fix here is to update the construction of restore on line 964 so that it escapes backslashes before escaping double quotes. We can do this by chaining two .replace calls with global regexes: first replace(/\\/g, '\\\\') to turn each \ into \\, then replace(/"/g, '\\"') to safely embed into a double-quoted string. This preserves existing functionality (double quotes are still escaped) while adding the missing protection. No external libraries are needed; we can use built-in JavaScript string replacement with regular expressions.

Concretely, in packages/opencode/src/tool/bash.ts, change the line

const restore = last ? bashPath(last).replace(/"/g, '\\"') : ""

to

const restore = last
  ? bashPath(last)
      .replace(/\\/g, "\\\\")
      .replace(/"/g, '\\"')
  : ""

This ensures any backslashes in the path are preserved correctly when the string is passed to cd "${restore}" ..., preventing malformed commands or potential injection arising from unescaped backslashes.

Suggested changeset 1
packages/opencode/src/tool/bash.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/opencode/src/tool/bash.ts b/packages/opencode/src/tool/bash.ts
--- a/packages/opencode/src/tool/bash.ts
+++ b/packages/opencode/src/tool/bash.ts
@@ -961,7 +961,11 @@
               })
               .catch(() => { })
           }
-          const restore = last ? bashPath(last).replace(/"/g, '\\"') : ""
+          const restore = last
+            ? bashPath(last)
+                .replace(/\\/g, "\\\\")
+                .replace(/"/g, '\\"')
+            : ""
           const note = restore ? ` (restored working directory: ${last})` : ""
           if (restore) {
             await next
EOF
@@ -961,7 +961,11 @@
})
.catch(() => { })
}
const restore = last ? bashPath(last).replace(/"/g, '\\"') : ""
const restore = last
? bashPath(last)
.replace(/\\/g, "\\\\")
.replace(/"/g, '\\"')
: ""
const note = restore ? ` (restored working directory: ${last})` : ""
if (restore) {
await next
Copilot is powered by AI and may make mistakes. Always verify output.
.catch(() => { })
}
if (last) {
const restore = bashPath(last).replace(/"/g, '\\"')

Check failure

Code scanning / CodeQL

Incomplete string escaping or encoding High

This does not escape backslash characters in the input.

Copilot Autofix

AI 4 months ago

In general, when embedding arbitrary strings into shell commands, avoid hand-rolled escaping. Prefer using a proper quoting routine or a well-tested library that correctly handles all special characters, including backslashes and quotes. If you must escape manually, ensure that backslashes are escaped before quotes so that an input like \" doesn’t turn into an unescaped quote in the final string.

For this specific case, the smallest fix that preserves existing behavior is to extend the current replacement chain so that it also escapes backslashes. A correct order is: first replace each backslash \ with a double backslash \\, then escape double quotes. Doing it in this order prevents newly added backslashes from interfering with subsequent escaping of quotes. Concretely, in packages/opencode/src/tool/bash.ts, at line 1175, change:

const restore = bashPath(last).replace(/"/g, '\\"')

to:

const restore = bashPath(last).replace(/\\/g, "\\\\").replace(/"/g, '\\"')

This keeps the rest of the logic intact and ensures both backslashes and double quotes are escaped before the value is interpolated into the cd command. No new imports or helpers are needed.

Suggested changeset 1
packages/opencode/src/tool/bash.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/opencode/src/tool/bash.ts b/packages/opencode/src/tool/bash.ts
--- a/packages/opencode/src/tool/bash.ts
+++ b/packages/opencode/src/tool/bash.ts
@@ -1172,7 +1172,7 @@
               .catch(() => { })
           }
           if (last) {
-            const restore = bashPath(last).replace(/"/g, '\\"')
+            const restore = bashPath(last).replace(/\\/g, "\\\\").replace(/"/g, '\\"')
             await next
               .run({
                 command: `cd "${restore}" 2>/dev/null || true`,
EOF
@@ -1172,7 +1172,7 @@
.catch(() => { })
}
if (last) {
const restore = bashPath(last).replace(/"/g, '\\"')
const restore = bashPath(last).replace(/\\/g, "\\\\").replace(/"/g, '\\"')
await next
.run({
command: `cd "${restore}" 2>/dev/null || true`,
Copilot is powered by AI and may make mistakes. Always verify output.
}
const needs = state.cwd ? state.cwd !== expected : true
if (needs) {
const restore = bashPath(expected).replace(/"/g, '\\"')

Check failure

Code scanning / CodeQL

Incomplete string escaping or encoding High

This does not escape backslash characters in the input.

Copilot Autofix

AI 4 months ago

In general, when interpolating untrusted or variable paths into shell commands, avoid manual string concatenation and escaping where possible; instead, use APIs that take argument arrays, or use a robust escaping function that correctly handles all relevant meta-characters (including backslashes). If you must escape manually for a double‑quoted string in a POSIX shell, you need to escape \, ", `, and $ at a minimum.

The single best minimal fix here, without changing overall behavior, is to replace the current .replace(/"/g, '\\"') with an escaping function that properly escapes backslashes first and then double quotes (and optionally other special characters) so the result can safely be embedded inside double quotes. Since we must only modify the shown snippet, we can implement a small helper function right above its first use within this file, and use it to compute restore. For example, define escapeForDoubleQuotes that replaces each backslash \ with \\ and each double quote " with \". Then update line 1238 so that restore is computed as escapeForDoubleQuotes(bashPath(expected)). No new imports are needed; we can implement the helper using built-in string methods.

Concretely:

  • In packages/opencode/src/tool/bash.ts, near the ensureReady function (before its definition or at least before line 1238), add a local helper:
const escapeForDoubleQuotes = (s: string): string =>
  s.replace(/\\/g, "\\\\").replace(/"/g, '\\"')
  • Replace line 1238 so that it calls this helper instead of calling .replace directly. The rest of the ensureReady logic remains unchanged.
Suggested changeset 1
packages/opencode/src/tool/bash.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/opencode/src/tool/bash.ts b/packages/opencode/src/tool/bash.ts
--- a/packages/opencode/src/tool/bash.ts
+++ b/packages/opencode/src/tool/bash.ts
@@ -1201,6 +1201,9 @@
           return { ok: true, cwd: normalizeWin(text), error: "" }
         }
 
+        const escapeForDoubleQuotes = (s: string): string =>
+          s.replace(/\\/g, "\\\\").replace(/"/g, '\\"')
+
         const ensureReady = async () => {
           const state = { session: null as BashSession | null, notice: "", cwd: "", error: "" }
           const loop = { count: 0 }
@@ -1235,7 +1238,7 @@
           }
           const needs = state.cwd ? state.cwd !== expected : true
           if (needs) {
-            const restore = bashPath(expected).replace(/"/g, '\\"')
+            const restore = escapeForDoubleQuotes(bashPath(expected))
             await state.session
               .run({
                 command: `cd "${restore}" 2>/dev/null || cd "${restore}" || exit 1`,
EOF
@@ -1201,6 +1201,9 @@
return { ok: true, cwd: normalizeWin(text), error: "" }
}

const escapeForDoubleQuotes = (s: string): string =>
s.replace(/\\/g, "\\\\").replace(/"/g, '\\"')

const ensureReady = async () => {
const state = { session: null as BashSession | null, notice: "", cwd: "", error: "" }
const loop = { count: 0 }
@@ -1235,7 +1238,7 @@
}
const needs = state.cwd ? state.cwd !== expected : true
if (needs) {
const restore = bashPath(expected).replace(/"/g, '\\"')
const restore = escapeForDoubleQuotes(bashPath(expected))
await state.session
.run({
command: `cd "${restore}" 2>/dev/null || cd "${restore}" || exit 1`,
Copilot is powered by AI and may make mistakes. Always verify output.
Comment thread packages/opencode/src/tool/lint.ts Fixed
@Askhz
Askhz force-pushed the feature/merge-costrae branch from adef4b0 to befef7c Compare March 20, 2026 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants