Fix setup TUI hatch terminal handoff#69524
Conversation
🔒 Aisle Security AnalysisWe found 3 potential security issue(s) in this PR:
1. 🟠 Potential local file execution when relaunching TUI via `process.execPath` + relative entry
Description
If
In Node, the first non-flag argument is treated as the script path to execute. That makes the relaunch execute a file named Vulnerable code: function buildCurrentCliEntryArgs(): string[] {
const entry = process.argv[1]?.trim();
if (!entry) {
throw new Error("unable to relaunch TUI: current CLI entry path is unavailable");
}
return path.isAbsolute(entry) ? [entry] : [];
}
function buildTuiCliArgs(opts: TuiOptions): string[] {
const args = [...filterTuiExecArgv(process.execArgv), ...buildCurrentCliEntryArgs(), "tui"];
// ...
}RecommendationEnsure the respawn always invokes the intended CLI entry file when Safer options:
function buildCurrentCliEntryArgs(): string[] {
const entry = process.argv[1]?.trim();
if (!entry) {
throw new Error("unable to relaunch TUI: current CLI entry path is unavailable");
}
// Resolve relative paths against the current working directory.
return [path.isAbsolute(entry) ? entry : path.resolve(entry)];
}
Also consider setting an explicit 2. 🟠 Node child TUI inherits NODE_OPTIONS / execArgv enabling arbitrary code loading
Description
If OpenClaw is invoked in a context where environment variables or Node flags can be influenced by a less-trusted party (e.g., wrappers, service managers, Vulnerable code: const args = [...filterTuiExecArgv(process.execArgv), ...buildCurrentCliEntryArgs(), "tui"];
...
const env = ... ? { ...process.env, ... } : process.env;
...
const child = spawn(process.execPath, args, { stdio: "inherit", env });RecommendationScrub Node runtime injection vectors when spawning the child process.
Example hardening: function sanitizeChildEnv(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv {
const out = { ...env };
delete out.NODE_OPTIONS;
// consider also: delete out.NODE_PATH;
return out;
}
function filterSafeExecArgv(execArgv: readonly string[]): string[] {
const deniedPrefixes = ["--require", "--import", "--loader", "--experimental-loader", "--eval", "-e", "--inspect", "--inspect-brk", "--inspect-wait", "--inspect-port"];
return execArgv.filter((arg) => !deniedPrefixes.some((p) => arg === p || arg.startsWith(p + "=") ));
}
const args = [...filterSafeExecArgv(process.execArgv), ...buildCurrentCliEntryArgs(), "tui"];
const child = spawn(process.execPath, args, { stdio: "inherit", env: sanitizeChildEnv(process.env) });If you need some flags for debugging, gate them behind an explicit 3. 🟡 Gateway token/password exposed via child-process command-line arguments when relaunching TUI
Description
Why this is a problem:
Vulnerable code: appendOption(args, "--token", opts.token);
appendOption(args, "--password", opts.password);
...
spawn(process.execPath, args, { stdio: "inherit", env });RecommendationAvoid passing secrets in argv. Prefer one of:
Example (env-based) change: function buildTuiCliArgs(opts: TuiOptions): string[] {
const args = [...filterTuiExecArgv(process.execArgv), ...buildCurrentCliEntryArgs(), "tui"];
appendOption(args, "--url", opts.url);
// Do NOT append --token/--password
return args;
}
export async function launchTuiCli(opts: TuiOptions, launchOptions: TuiLaunchOptions = {}) {
const args = buildTuiCliArgs(opts);
const env = {
...process.env,
...(opts.token ? { OPENCLAW_GATEWAY_TOKEN: opts.token } : {}),
...(opts.password ? { OPENCLAW_GATEWAY_PASSWORD: opts.password } : {}),
};
spawn(process.execPath, args, { stdio: "inherit", env });
}Then update the Analyzed PR: #69524 at commit Last updated on: 2026-04-21T02:26:07Z |
Greptile SummaryThis PR fixes terminal state corruption during the onboarding "Hatch in TUI" flow by spawning a fresh
Confidence Score: 4/5Safe to merge after adding a try/finally around launchTuiCli so the post-restore runs on abnormal TUI exit. One P1 finding: the post-TUI terminal restore is skipped when the TUI subprocess crashes or exits with a signal. All other findings are P2 (execArgv debug-flag forwarding). The core approach is correct and the test updates are consistent. src/wizard/setup.finalize.ts — the try/finally gap around launchTuiCli Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/tui/tui-launch.ts
Line: 19
Comment:
**`--inspect` / debug flags forwarded to child process**
`process.execArgv` is spread into the child args. If the parent was started with `--inspect` or `--inspect-brk` for debugging, the child TUI process will try to bind the same debugger port, causing an `EADDRINUSE` failure. Consider filtering out inspector flags from `execArgv` before forwarding them.
```suggestion
const args = [...process.execArgv.filter(f => !f.startsWith("--inspect")), entry, "tui"];
```
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/wizard/setup.finalize.ts
Line: 424-435
Comment:
**Missing `try/finally` for post-TUI terminal restore**
`restoreTerminalState("post-setup tui", ...)` on line 434 is only reached when `launchTuiCli` resolves. If the TUI subprocess crashes or is killed by a signal, `launchTuiCli` rejects and the restore is skipped — leaving the terminal in whatever raw/alternate-mode state the TUI left behind for the wizard's remaining output. Wrapping `launchTuiCli` in a `try/finally` that calls `restoreTerminalState("post-setup tui", { resumeStdinIfPaused: true })` would ensure cleanup regardless of exit outcome.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: relaunch setup tui in a fresh proce..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4aeb81eeeb
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3669babf8f
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 459f1fc12d
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85726647f3
ℹ️ 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".
|
Merged via squash.
Thanks @shakkernerd! |
* fix: relaunch setup tui in a fresh process * fix: harden setup tui handoff * fix: preserve tui hatch exit flow * Revert "fix: preserve tui hatch exit flow" This reverts commit f4f119a. * fix: let setup tui resolve gateway auth * fix: support packaged tui relaunch * fix: pin setup tui gateway target * fix: preserve setup tui auth source
1 participant
Summary
openclaw tuichild process instead of reusing the onboarding processWhy
Choosing Hatch in TUI from onboarding could leave the setup prompt and TUI sharing the same terminal/input state. In that state, normal typing could inject raw terminal escape sequences into the TUI input area and the setup prompt could remain visually mixed with the TUI.
Before
The TUI could show raw terminal input such as
^[[104;1:3u...after hatch, even though the TUI had connected.After
The hatch flow starts a fresh TUI process, so the terminal belongs cleanly to the TUI and normal typing stays normal.
Verification
pnpm vitest run src/wizard/setup.finalize.test.ts src/wizard/setup.test.tspnpm checkmain