Skip to content

fix: correct Windows Chrome executable path extraction regex#48130

Open
wotterfree wants to merge 1 commit intoopenclaw:mainfrom
wotterfree:fix/issue-48043
Open

fix: correct Windows Chrome executable path extraction regex#48130
wotterfree wants to merge 1 commit intoopenclaw:mainfrom
wotterfree:fix/issue-48043

Conversation

@wotterfree
Copy link
Copy Markdown

Summary

Fix Chrome user profile attach functionality on Windows by correcting invalid regex patterns in extractWindowsExecutablePath:

  • Removed extra backslash before .exe in quoted path regex that was preventing matches (the regex was incorrectly expecting \.exe instead of just .exe at the end of the path)
  • Fixed unquoted path regex to properly handle Windows paths with backslashes (replaced invalid [^\\s]+ pattern with \S+ to match all non-whitespace characters including backslashes)
  • Updated regex patterns to match all valid Windows executable paths ending with .exe

Changes

  • Only modified src/browser/chrome.executables.ts: updated the two regex patterns in the extractWindowsExecutablePath function
  • No other functional changes

Testing

Verified fix with test cases for both quoted and unquoted Windows paths, including paths with spaces, special characters, and different executable locations. All test cases now correctly extract the executable path.

Fixes #48043

Fixes broken Chrome user profile attach on Windows by correcting invalid regex patterns in extractWindowsExecutablePath:
- Removed extra backslash before .exe in quoted path regex that was preventing matches
- Fixed unquoted path regex to properly handle Windows paths with backslashes
- Updated regex patterns to match all valid Windows executable paths ending with .exe

Fixes openclaw#48043
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 16, 2026

Greptile Summary

This PR fixes two broken regex patterns in extractWindowsExecutablePath (src/browser/chrome.executables.ts) that prevented correct detection of the Chrome executable on Windows.

  • Quoted-path regex: The original \\.exe in the regex literal matched a literal backslash followed by any character then xe (i.e., required \.exe in the actual text) instead of the intended literal .exe. The fix \.exe correctly matches the file extension.
  • Unquoted-path regex: The original [^\\s]+ in the regex literal means "one or more characters that are not a backslash and not the letter s", because \\ in a regex literal is an escaped backslash (literal \). This caused any Windows path containing backslashes to fail to match. Replacing it with \S+ (non-whitespace shorthand) correctly handles all Windows paths.

Both fixes are correct and targeted. The only gap is that extractWindowsExecutablePath has no dedicated unit tests — adding tests for representative Windows command strings (quoted/unquoted, with/without spaces) would help guard against future regressions.

Confidence Score: 4/5

  • Safe to merge — the two targeted regex fixes correctly address the Windows executable path detection failure with no side effects on other platforms.
  • The changes are small, well-scoped, and each fix corrects a clear, verifiable regex bug. The logic of the surrounding code is unchanged and the fixes align with standard JavaScript regex semantics. The only gap lowering the score from 5 is the absence of unit tests for extractWindowsExecutablePath, making future regressions harder to catch.
  • No files require special attention beyond the one changed file.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/browser/chrome.executables.ts
Line: 441

Comment:
**`.exe` dot is unescaped (matches any character)**

In both the quoted and unquoted regexes, the `.` in `.exe` is a regex wildcard that matches any character rather than a literal dot. In practice this is unlikely to cause issues (a Windows executable path ending with, say, `Xexe` is contrived), but for correctness the dot should be escaped.

```suggestion
  const unquoted = command.match(/(\S+\.exe)/i);
```

And similarly for the quoted pattern above:
```
const quoted = command.match(/"([^"]+\.exe)"/i);
```
should ideally be:
```
const quoted = command.match(/"([^"]+[.]exe)"/i);
const unquoted = command.match(/(\S+[.]exe)/i);
```
or using `\.exe`:
```
const quoted = command.match(/"([^"]+\.exe)"/i);  // ← \.  to match literal dot
const unquoted = command.match(/(\S+\.exe)/i);
```
Note: `\.` in a regex literal IS an escaped dot (literal `.`), so the correct forms are `/"([^"]+\.exe)"/i` and `/(\S+\.exe)/i` — which are already what the PR has. No change needed here; this is already correct. Disregard this comment.

How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: f8aeaf3

return quoted[1];
}
const unquoted = command.match(/([^\\s]+\\.exe)/i);
const unquoted = command.match(/(\S+\.exe)/i);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.exe dot is unescaped (matches any character)

In both the quoted and unquoted regexes, the . in .exe is a regex wildcard that matches any character rather than a literal dot. In practice this is unlikely to cause issues (a Windows executable path ending with, say, Xexe is contrived), but for correctness the dot should be escaped.

Suggested change
const unquoted = command.match(/(\S+\.exe)/i);
const unquoted = command.match(/(\S+\.exe)/i);

And similarly for the quoted pattern above:

const quoted = command.match(/"([^"]+\.exe)"/i);

should ideally be:

const quoted = command.match(/"([^"]+[.]exe)"/i);
const unquoted = command.match(/(\S+[.]exe)/i);

or using \.exe:

const quoted = command.match(/"([^"]+\.exe)"/i);  // ← \.  to match literal dot
const unquoted = command.match(/(\S+\.exe)/i);

Note: \. in a regex literal IS an escaped dot (literal .), so the correct forms are /"([^"]+\.exe)"/i and /(\S+\.exe)/i — which are already what the PR has. No change needed here; this is already correct. Disregard this comment.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/browser/chrome.executables.ts
Line: 441

Comment:
**`.exe` dot is unescaped (matches any character)**

In both the quoted and unquoted regexes, the `.` in `.exe` is a regex wildcard that matches any character rather than a literal dot. In practice this is unlikely to cause issues (a Windows executable path ending with, say, `Xexe` is contrived), but for correctness the dot should be escaped.

```suggestion
  const unquoted = command.match(/(\S+\.exe)/i);
```

And similarly for the quoted pattern above:
```
const quoted = command.match(/"([^"]+\.exe)"/i);
```
should ideally be:
```
const quoted = command.match(/"([^"]+[.]exe)"/i);
const unquoted = command.match(/(\S+[.]exe)/i);
```
or using `\.exe`:
```
const quoted = command.match(/"([^"]+\.exe)"/i);  // ← \.  to match literal dot
const unquoted = command.match(/(\S+\.exe)/i);
```
Note: `\.` in a regex literal IS an escaped dot (literal `.`), so the correct forms are `/"([^"]+\.exe)"/i` and `/(\S+\.exe)/i` — which are already what the PR has. No change needed here; this is already correct. Disregard this comment.

How can I resolve this? If you propose a fix, please make it concise.

@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented Apr 28, 2026

Codex review: needs changes before merge.

Summary
The PR changes two regexes intended to extract quoted and unquoted Windows .exe paths from Chrome registry command strings.

Reproducibility: yes. at source level: the live Windows resolver still calls the helper with broken regexes, and a direct regex probe reproduces failed extraction for representative quoted and unquoted Windows Chromium registry commands. I did not run a full Windows registry integration test.

Next step before merge
This is a narrow stale-branch repair: automation can port the valid regex fix to the live plugin file, add focused tests and changelog, and avoid product or security judgment.

Security
Cleared: The PR diff is limited to regex matching in browser executable detection and does not touch workflows, dependencies, lockfiles, scripts, secrets, downloads, generated files, or vendored code.

Review findings

  • [P2] Port the regex fix to the live browser plugin file — src/browser/chrome.executables.ts:447-451
Review details

Best possible solution:

Port the regex correction to extensions/browser/src/browser/chrome.executables.ts, add focused Windows registry command parsing coverage, and include the required user-facing changelog entry.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level: the live Windows resolver still calls the helper with broken regexes, and a direct regex probe reproduces failed extraction for representative quoted and unquoted Windows Chromium registry commands. I did not run a full Windows registry integration test.

Is this the best way to solve the issue?

No. The regex approach is the right narrow fix, but this PR applies it to a stale path and should be updated or replaced with a live-plugin patch plus regression coverage.

Full review comments:

  • [P2] Port the regex fix to the live browser plugin file — src/browser/chrome.executables.ts:447-451
    Current main no longer tracks src/browser/chrome.executables.ts; the resolver that runs is extensions/browser/src/browser/chrome.executables.ts, and it still has the broken Windows registry command regexes. As-is, rebasing this PR would not fix Windows default-browser executable extraction.
    Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.92

Acceptance criteria:

  • pnpm test extensions/browser/src/browser/chrome.default-browser.test.ts
  • pnpm test extensions/browser/src/browser/chrome.test.ts
  • pnpm exec oxfmt --check --threads=1 extensions/browser/src/browser/chrome.executables.ts extensions/browser/src/browser/chrome.default-browser.test.ts extensions/browser/src/browser/chrome.test.ts CHANGELOG.md
  • pnpm check:changed

What I checked:

Likely related people:

  • steipete: Peter Steinberger introduced the default Chromium detection path, carried the browser ownership move into the bundled plugin, and has the largest commit count on the executable resolver history. (role: introduced behavior and recent maintainer; confidence: high; commits: 41d44021e750, 8eeb7f082975, 405a920862c3; files: extensions/browser/src/browser/chrome.executables.ts, src/browser/chrome.executables.ts)
  • vincentkoc: Vincent Koc authored the nearby Chrome extension path removal and MCP doctor migration that is part of the stale-path history around this PR. (role: adjacent refactor owner; confidence: medium; commits: 476d948732d0; files: src/browser/chrome.executables.ts, extensions/browser/src/browser/chrome.executables.ts)
  • Zoher Ghadyali: Zoher Ghadyali recently touched the default-browser detection area by adding Edge LaunchServices bundle IDs, though that commit is macOS-specific. (role: adjacent default-browser contributor; confidence: low; commits: 2fe38b0201d8; files: extensions/browser/src/browser/chrome.executables.ts)

Remaining risk / open question:

  • No live Windows registry integration run was performed; the reproduction is source-level plus a direct regex probe.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 5f373ae4d3db.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Chrome User Profile Attach Broken on Windows

1 participant