Gateway: open config files without shell interpolation · Pull Request #57921 · openclaw/openclaw

ghost · 2026-03-30T20:21:18Z

Summary

switches config.openFile to a no-shell launch path
keeps platform-specific open behavior while treating the config path as data

Changes

replaced the shell-based launcher in src/gateway/server-methods/config.ts
added focused regression coverage in src/gateway/server-methods/config.test.ts
returns a generic client-facing open failure instead of raw process text

Validation

ran pnpm test -- src/gateway/server-methods/config.test.ts
ran pnpm build and reached an unrelated existing TypeScript failure in src/agents/skills/local-loader.ts

Notes

Windows now routes through powershell.exe with a quoted PowerShell string literal instead of cmd /c start
residual risk or follow-up: full repo build is currently blocked by the existing src/agents/skills/local-loader.ts TypeScript errors

Co-authored-by: peteryuqin <[email protected]>

greptile-apps · 2026-03-30T20:23:43Z

Greptile Summary

This PR correctly fixes a shell-injection vulnerability in config.openFile by replacing exec (which interpolated the config path through a shell) with execFile (which passes the path as a data argument). The platform-specific dispatch — open/xdg-open with the path as an argument on macOS/Linux, and powershell.exe Start-Process with a single-quote-escaped literal on Windows — is implemented correctly. Error responses now return a generic client-facing message while logging details server-side. The new test file provides good regression coverage.

Security fix is sound: execFile ensures the path is never interpreted by a shell on macOS and Linux; the PowerShell single-quoted literal correctly neutralises all special characters (including &, $(), etc.) on Windows, with ' doubled as the only required escape.
Minor: formatConfigOpenError contains a typeof error === "string" branch that is unreachable — execFile callbacks only produce Error instances — it can be dropped.
Minor: The new warn log embeds configPath without passing it through the existing sanitizeLookupPathForLog helper (which truncates at 120 chars), inconsistent with the pattern on line 342.

Confidence Score: 5/5

Safe to merge — the security fix is correct and all remaining findings are minor style/cleanup suggestions.
No P0 or P1 issues. Both open comments are P2: one is dead code in a helper function, the other is a missing path-sanitisation call in a log line. Neither affects runtime correctness, security, or data integrity.
No files require special attention.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/gateway/server-methods/config.ts
Line: 174-187

Comment:
**Unreachable `string` branch in `formatConfigOpenError`**

The `typeof error === "string"` branch is dead code in this context. `execFile`'s callback always receives an `Error`-subclass (`ExecFileException`) or `null` — it never passes a bare string. The function therefore always hits the `"message" in error` branch. Since the function is only called from one place with a value already cast to `ExecFileException`, this could simply be `err.message` (or the `object` branch alone if reuse is intended):

```suggestion
function formatConfigOpenError(error: unknown): string {
  if (
    typeof error === "object" &&
    error &&
    "message" in error &&
    typeof error.message === "string"
  ) {
    return error.message;
  }
  return String(error);
}
```

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

---

This is a comment left during a code review.
Path: src/gateway/server-methods/config.ts
Line: 573-575

Comment:
**Log path not guarded by `sanitizeLookupPathForLog`**

Elsewhere in this file (line 342) paths are passed through `sanitizeLookupPathForLog` before being embedded in log messages, which truncates them at 120 characters and strips control characters. The new `config.openFile` error log embeds the raw `configPath` without that guard. An unusually long or oddly-formed path would produce an unbounded log line:

```suggestion
      context?.logGateway?.warn(
        `config.openFile failed path=${sanitizeLookupPathForLog(configPath)}: ${formatConfigOpenError(err)}`,
      );
```

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

_{Reviews (2): Last reviewed commit: "Gateway: align config opener review fixe..." | Re-trigger Greptile}

ghost · 2026-03-30T20:28:31Z

Addressed the Greptile feedback on the latest head: the handler test is now platform-agnostic, and the error-path respond call again passes the explicit third undefined argument.

ghost · 2026-03-30T20:30:06Z

@greptile review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 914eb03b44

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

* Gateway: open config files without shell interpolation Co-authored-by: peteryuqin <[email protected]> * Gateway: align config opener review fixes * Gateway: tidy config opener logging * Gateway: simplify config opener error path * Gateway: cover Windows config opener test path * Gateway: use literal Windows config open path --------- Co-authored-by: peteryuqin <[email protected]>

Gateway: open config files without shell interpolation

5d6a96c

Co-authored-by: peteryuqin <[email protected]>

openclaw-barnacle Bot added gateway Gateway runtime size: S labels Mar 30, 2026

greptile-apps Bot reviewed Mar 30, 2026

View reviewed changes

Comment thread src/gateway/server-methods/config.test.ts

Comment thread src/gateway/server-methods/config.ts Outdated

Gateway: align config opener review fixes

914eb03

openclaw-barnacle Bot added size: M and removed size: S labels Mar 30, 2026

chatgpt-codex-connector Bot reviewed Mar 30, 2026

View reviewed changes

Comment thread src/gateway/server-methods/config.test.ts Outdated

mmaps added 2 commits March 30, 2026 20:59

Gateway: tidy config opener logging

9d0e51c

Gateway: simplify config opener error path

a73cbb3

openclaw-barnacle Bot added size: S and removed size: M labels Mar 30, 2026

mmaps added 2 commits March 30, 2026 21:02

Gateway: cover Windows config opener test path

564a988

Gateway: use literal Windows config open path

e16ee5e

drobison00 merged commit 5cc0bc9 into openclaw:main Mar 30, 2026
8 checks passed

clawsweeper Bot mentioned this pull request Apr 29, 2026

fix(gateway): use -FilePath instead of -LiteralPath in Start-Process [AI-assisted] #72153

Closed

4 tasks

clawsweeper Bot mentioned this pull request May 12, 2026

🛡️ [SECURITY] Zero-Dep Runtime Shield — Circuit Breaker, Anomaly Detection & 4 Critical Vuln Patches #52636

Closed

6 tasks

clawsweeper Bot mentioned this pull request Jul 9, 2026

fix(gateway): use truncateUtf16Safe for config path display truncation #102513

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Gateway: open config files without shell interpolation#57921

Gateway: open config files without shell interpolation#57921
drobison00 merged 6 commits into
mainfrom
unknown repository

ghost commented Mar 30, 2026

Uh oh!

greptile-apps Bot commented Mar 30, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

ghost commented Mar 30, 2026

Uh oh!

ghost commented Mar 30, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

ghost commented Mar 30, 2026

Summary

Changes

Validation

Notes

Uh oh!

greptile-apps Bot commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Uh oh!

Uh oh!

Uh oh!

ghost commented Mar 30, 2026

Uh oh!

ghost commented Mar 30, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

greptile-apps Bot commented Mar 30, 2026 •

edited

Loading