Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 31 additions & 4 deletions packages/host-service/src/terminal/clean-shell-env.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { type ChildProcess, spawn } from "node:child_process";
import * as os from "node:os";

const SHELL_ENV_TIMEOUT_MS = 8_000;
const CACHE_TTL_MS = 60_000;
const DELIMITER = "__SUPERSET_SHELL_ENV__";
const DIAGNOSTIC_OUTPUT_LIMIT = 200;

const SHELL_BOOTSTRAP_KEYS = [
"HOME",
Expand All @@ -29,7 +31,7 @@ const COMMON_MACOS_PATHS = [
"/usr/local/sbin",
];

function augmentPathForMacOS(
export function augmentPathForMacOS(
env: Record<string, string>,
platform: NodeJS.Platform = process.platform,
): void {
Expand Down Expand Up @@ -90,18 +92,32 @@ function parseEnvOutput(stdout: string): Record<string, string> {
return result;
}

function truncateForDiagnostics(value: string): string {
const trimmed = value.trim();
if (trimmed.length <= DIAGNOSTIC_OUTPUT_LIMIT) return trimmed;
return `${trimmed.slice(0, DIAGNOSTIC_OUTPUT_LIMIT)}…`;
}

function spawnCleanShellEnv(): Promise<Record<string, string>> {
return new Promise((resolve, reject) => {
const shell = resolveShellForEnv();
const env = buildMinimalEnv();
const command = `echo -n "${DELIMITER}"; command env; echo -n "${DELIMITER}"; exit`;

// Anchor at $HOME so the snapshot shell doesn't inherit a cwd
// host-service has no control over. Tools called from interactive
// rc files — brew is the recurring offender (#4025) — abort when
// pwd isn't readable to the invoking user, and Electron helpers
// can land at /private/var/... or similar at launch.
const cwd = env.HOME || os.homedir();

let child: ChildProcess;
try {
child = spawn(shell, ["-i", "-l", "-c", command], {
detached: true,
stdio: ["ignore", "pipe", "pipe"],
env,
cwd,
});
} catch (error) {
return reject(
Expand Down Expand Up @@ -139,6 +155,7 @@ function spawnCleanShellEnv(): Promise<Record<string, string>> {
child.on("close", (code, signal) => {
clearTimeout(timeout);

const stdout = Buffer.concat(stdoutBuffers).toString("utf8");
const stderr = Buffer.concat(stderrBuffers).toString("utf8").trim();
if (stderr) {
console.debug("[terminal-clean-shell-env] stderr:", stderr);
Expand All @@ -147,15 +164,25 @@ function spawnCleanShellEnv(): Promise<Record<string, string>> {
if (code !== 0 && code !== null) {
return reject(
new Error(
`Shell ${shell} exited with code ${code}${signal ? `, signal ${signal}` : ""}`,
`Shell ${shell} exited with code ${code}${signal ? `, signal ${signal}` : ""}` +
(stderr ? ` stderr=${truncateForDiagnostics(stderr)}` : "") +
(stdout ? ` stdout=${truncateForDiagnostics(stdout)}` : ""),
),
);
}

try {
resolve(parseEnvOutput(Buffer.concat(stdoutBuffers).toString("utf8")));
resolve(parseEnvOutput(stdout));
} catch (error) {
reject(error);
const detail = error instanceof Error ? error.message : String(error);
reject(
new Error(
`${detail} (shell=${shell}` +
` stdout=${truncateForDiagnostics(stdout)}` +
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Remove shell stdout/stderr from thrown probe errors

The new error construction now embeds truncated stdout/stderr from the shell probe, and resolveTerminalBaseEnv() logs that message verbatim on fallback. Because the probe command runs env, a failing shell startup can leak user-exported secrets (for example tokens from shell rc files) into host-service logs; this is a security regression introduced by this commit’s diagnostics path.

Useful? React with 👍 / 👎.

(stderr ? ` stderr=${truncateForDiagnostics(stderr)}` : "") +
")",
),
);
}
});

Expand Down
17 changes: 16 additions & 1 deletion packages/host-service/src/terminal/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export {
import fs from "node:fs";
import os from "node:os";
import {
augmentPathForMacOS,
clearStrictShellEnvCache,
getStrictShellEnvironment,
} from "./clean-shell-env";
Expand Down Expand Up @@ -53,11 +54,25 @@ function snapshotStringEnv(
/**
* Resolve the shell-derived terminal base env inside the host-service process.
* Desktop main should not construct or own this snapshot.
*
* Falls back to a process.env snapshot if the user's login shell can't be
* probed — crashing host-service startup over a degraded PTY env strands
* users on v2. v1 desktop main does the same in apps/desktop shell-env.ts.
*/
export async function resolveTerminalBaseEnv(): Promise<
Record<string, string>
> {
return getStrictShellEnvironment();
try {
return await getStrictShellEnvironment();
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
console.warn(
`[host-service] Shell env snapshot failed, falling back to process.env: ${message}`,
);
const fallback = snapshotStringEnv(process.env);
augmentPathForMacOS(fallback);
return fallback;
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/src/terminal-link-parsing/link-parsing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ enum RegexPathConstants {
PathPrefix = "(?:\\.\\.?|\\~|file:\\/\\/)",
PathSeparatorClause = "\\/",
// '":; are allowed in paths but they are often separators so ignore them
// Also disallow \\ to prevent a catastropic backtracking case #24795
// Also disallow \\ to prevent a catastrophic backtracking case #24795
ExcludedPathCharactersClause = "[^\\0<>\\?\\s!`&*()'\";:\\\\]",
ExcludedStartPathCharactersClause = "[^\\0<>\\?\\s!`&*()\\[\\]'\";:\\\\]",

Expand Down
Loading