diff --git a/src/main/app/start.ts b/src/main/app/start.ts index 1dbcbb28..4a077662 100644 --- a/src/main/app/start.ts +++ b/src/main/app/start.ts @@ -20,6 +20,7 @@ import { isAllowedWebviewUrl, } from "../security"; import { registerIpcHandlers } from "../ipc/register"; +import { setGatewayPromptParent } from "../gatewayPrompt"; import { showChatContextMenu } from "./context-menu"; import { buildMenu } from "./menu"; import { setupUpdater } from "./updater"; @@ -159,6 +160,10 @@ function createWindow(): void { }); mainWindow.on("ready-to-show", () => mainWindow?.show()); + + // Let mid-turn gateway sudo/secret prompts parent their modal to this window. + setGatewayPromptParent(() => mainWindow); + mainWindow.webContents.on("render-process-gone", (_event, details) => { console.error("[CRASH] Renderer process gone:", details.reason, details.exitCode); }); diff --git a/src/main/askpass.ts b/src/main/askpass.ts index 1c27a8bb..1d445576 100644 --- a/src/main/askpass.ts +++ b/src/main/askpass.ts @@ -117,10 +117,21 @@ exit 1 }; } -async function showPasswordDialog( +/** + * Show a hardened, modal password/secret prompt and resolve with the entered + * value (or null on cancel). CSP-locked (`default-src 'none'`), sandboxed, + * ephemeral data-URL — the value is never persisted. Reused by both the + * installer's sudo askpass bridge and the mid-turn gateway sudo/secret prompts + * (see `gatewayPrompt.ts`). `title` / `heading` default to the installer's + * wording so existing callers are unchanged. + */ +export async function showPasswordDialog( parent: BrowserWindow | null, prompt: string, + opts: { title?: string; heading?: string } = {}, ): Promise { + const title = opts.title ?? "Administrator Password Required"; + const heading = opts.heading ?? "The installer needs your password"; return new Promise((resolve) => { const win = new BrowserWindow({ width: 460, @@ -131,7 +142,7 @@ async function showPasswordDialog( minimizable: false, maximizable: false, fullscreenable: false, - title: "Administrator Password Required", + title, webPreferences: { preload: join(__dirname, "../preload/askpass.js"), nodeIntegration: false, @@ -173,7 +184,7 @@ async function showPasswordDialog( event.preventDefault(), ); - const html = buildDialogHtml(prompt); + const html = buildDialogHtml(prompt, heading); win.loadURL( "data:text/html;charset=UTF-8;base64," + Buffer.from(html).toString("base64"), @@ -181,12 +192,15 @@ async function showPasswordDialog( }); } -function buildDialogHtml(prompt: string): string { - const safePrompt = prompt - .replace(/&/g, "&") - .replace(//g, ">") - .replace(/"/g, """); +function buildDialogHtml(prompt: string, heading: string): string { + const esc = (s: string): string => + s + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """); + const safePrompt = esc(prompt); + const safeHeading = esc(heading); return ` @@ -203,7 +217,7 @@ function buildDialogHtml(prompt: string): string { button:hover { opacity:0.9; } -
The installer needs your password
+
${safeHeading}
${safePrompt}
diff --git a/src/main/gatewayPrompt.ts b/src/main/gatewayPrompt.ts new file mode 100644 index 00000000..dbb900e3 --- /dev/null +++ b/src/main/gatewayPrompt.ts @@ -0,0 +1,69 @@ +import type { BrowserWindow } from "electron"; +import { showPasswordDialog } from "./askpass"; + +/** + * Mid-turn gateway credential prompts (`sudo.request` / `secret.request`). + * + * Unlike `clarify.request` — which renders an inline card in the chat + * transcript — a sudo password or a secret value is sensitive and must NEVER + * land in scrollback. So these reuse the installer's hardened askpass modal + * (`showPasswordDialog`): CSP-locked `default-src 'none'`, sandboxed, ephemeral + * data-URL, the value never persisted. + * + * Gateway protocol (NousResearch/hermes-agent, tui_gateway/server.py), keyed by + * request_id: + * sudo.request {} -> sudo.respond { request_id, password } + * secret.request { prompt, env_var, ... } -> secret.respond { request_id, value } + * An empty answer is a safe "skip": the gateway treats secret.request as + * skipped and lets a terminal sudo prompt fail cleanly, so cancel maps to "". + */ + +let parentWindowGetter: () => BrowserWindow | null = () => null; + +/** Wire the provider that returns the window to parent the modal to. Called + * once from index.ts after the main window is created. */ +export function setGatewayPromptParent( + getter: () => BrowserWindow | null, +): void { + parentWindowGetter = getter; +} + +/** + * Prompt for the sudo password. Resolves with the password, or "" if the user + * cancels (safe skip — terminal sudo then fails cleanly rather than hanging). + */ +export async function promptSudoPassword(): Promise { + const parent = parentWindowGetter(); + const value = await showPasswordDialog( + parent, + "An agent command needs administrator (sudo) access to continue. " + + "Your password is sent only to the local sudo prompt and is never stored.", + { + title: "Administrator Password Required", + heading: "Hermes needs your sudo password", + }, + ); + return value ?? ""; +} + +/** + * Prompt for a named secret the agent requested (e.g. an API key it needs to + * store). Resolves with the value, or "" if the user cancels (safe skip — the + * gateway records the secret as skipped). + */ +export async function promptSecretValue( + envVar: string, + prompt: string, +): Promise { + const parent = parentWindowGetter(); + const detail = + (prompt && prompt.trim()) || + `The agent is requesting a value for ${envVar || "a secret"}.`; + const value = await showPasswordDialog(parent, detail, { + title: "Secret Required", + heading: envVar + ? `Hermes needs a value for ${envVar}` + : "Hermes needs a secret value", + }); + return value ?? ""; +} diff --git a/src/main/hermes.ts b/src/main/hermes.ts index 18824aec..f22c1992 100644 --- a/src/main/hermes.ts +++ b/src/main/hermes.ts @@ -45,6 +45,8 @@ import { getActiveProfileNameSync, } from "./utils"; import { getProfilePort } from "./gateway-ports"; +import { promptSudoPassword, promptSecretValue } from "./gatewayPrompt"; +import { getSecret } from "./secrets"; import { readModels } from "./models"; import { providerListSafe } from "./secrets"; import { HIDDEN_SUBPROCESS_OPTIONS } from "./process-options"; @@ -1984,14 +1986,63 @@ async function sendMessageViaTuiGateway( } if (event.type === "sudo.request" || event.type === "secret.request") { - // Out of scope for the inline-clarify change: a desktop sudo/secret prompt - // carries its own security-review surface and is a deliberate follow-up. - void client - .request("session.interrupt", { session_id: activeSessionId }, 5_000) - .catch(() => undefined); - finish( - `Hermes requested ${event.type.replace(".request", "")} input, but Hermes One does not yet expose that gateway dialog.`, - ); + const isSudo = event.type === "sudo.request"; + const requestId = + typeof event.payload?.request_id === "string" + ? event.payload.request_id + : ""; + if (!requestId) { + void client + .request("session.interrupt", { session_id: activeSessionId }, 5_000) + .catch(() => undefined); + finish( + `Hermes requested ${event.type.replace(".request", "")} input, but the gateway provided no request_id to answer.`, + ); + return; + } + // A sudo password / secret value is sensitive — collect it in the + // hardened askpass modal (never the chat transcript) and forward it to + // the gateway. Cancel maps to "" (a safe skip the gateway handles). + // + // For secret.request: try the configured security provider first. If the + // vault already holds the key, answer silently without prompting the user. + const payload = event.payload as + | { prompt?: string; env_var?: string } + | undefined; + const envVar = String(payload?.env_var ?? ""); + + // Vault-first resolution for secret.request: attempt a provider lookup + // before falling back to the interactive modal. sudo.request always needs + // an interactive password — no vault lookup applies. + const vaultValue = + !isSudo && envVar ? getSecret(envVar, profile) : null; + + const collect: Promise = + vaultValue != null + ? Promise.resolve(vaultValue) + : isSudo + ? promptSudoPassword() + : promptSecretValue(envVar, String(payload?.prompt ?? "")); + + void collect + .then((answer) => { + if (finished) return; // turn was cancelled while modal was open + const method = isSudo ? "sudo.respond" : "secret.respond"; + const params = isSudo + ? { request_id: requestId, password: answer } + : { request_id: requestId, value: answer }; + return client.request(method, params, 300_000); + }) + .catch((error) => { + const message = + error instanceof Error ? error.message : String(error); + if (!hasGatewayOutput) { + startApiFallback(message); + return; + } + finish(message); + }); + return; } }); diff --git a/tests/askpass-security.test.ts b/tests/askpass-security.test.ts index 91f4038b..d07a4653 100644 --- a/tests/askpass-security.test.ts +++ b/tests/askpass-security.test.ts @@ -6,6 +6,11 @@ import { ASKPASS_SUBMIT_CHANNEL } from "../src/shared/askpass"; const ROOT = join(__dirname, ".."); const askpassMainSrc = readFileSync(join(ROOT, "src/main/askpass.ts"), "utf-8"); const sudoCredsSrc = readFileSync(join(ROOT, "src/main/sudoCreds.ts"), "utf-8"); +const gatewayPromptSrc = readFileSync( + join(ROOT, "src/main/gatewayPrompt.ts"), + "utf-8", +); +const hermesSrc = readFileSync(join(ROOT, "src/main/hermes.ts"), "utf-8"); const askpassPreloadSrc = readFileSync( join(ROOT, "src/preload/askpass.ts"), "utf-8", @@ -92,3 +97,36 @@ describe("askpass Electron hardening", () => { expect(electronViteConfigSrc).toContain("src/preload/askpass.ts"); }); }); + +describe("gateway sudo/secret prompt handling", () => { + it("collects sudo/secret via the hardened askpass modal, not the chat transcript", () => { + // Reuses the shared hardened dialog rather than a bespoke window. + expect(gatewayPromptSrc).toContain( + 'import { showPasswordDialog } from "./askpass"', + ); + expect(gatewayPromptSrc).toContain("showPasswordDialog("); + // Sensitive values must never be rendered as a chat message / clarify card. + expect(gatewayPromptSrc).not.toMatch(/onClarify|ClarifyMessage|safeSend/); + }); + + it("never logs or persists the collected secret", () => { + expect(gatewayPromptSrc).not.toMatch(/console\.(log|warn|error|info)/); + expect(gatewayPromptSrc).not.toMatch(/writeFile|appendFile|setEnvValue/); + }); + + it("forwards answers with the gateway's exact respond shapes", () => { + // sudo.respond carries `password`; secret.respond carries `value`. + expect(hermesSrc).toContain('"sudo.respond"'); + expect(hermesSrc).toContain('"secret.respond"'); + expect(hermesSrc).toContain("password: answer"); + expect(hermesSrc).toContain("value: answer"); + // Both keyed by request_id, mirroring clarify.respond. + expect(hermesSrc).toContain("request_id: requestId, password"); + expect(hermesSrc).toContain("request_id: requestId, value"); + }); + + it("maps cancel to an empty answer (safe skip), never blocking the turn", () => { + // showPasswordDialog resolves null on cancel; the prompt helpers coerce to "". + expect(gatewayPromptSrc).toContain('return value ?? "";'); + }); +}); diff --git a/tests/ssh-remote.test.ts b/tests/ssh-remote.test.ts index bbf44819..e86dab18 100644 --- a/tests/ssh-remote.test.ts +++ b/tests/ssh-remote.test.ts @@ -33,18 +33,28 @@ const sshConfig: SshConfig = { function runWithHermesShim(command: string): Buffer { const home = mkdtempSync(join(tmpdir(), "hermes-ssh-cmd-home-")); - const bin = join(home, "bin"); - mkdirSync(bin, { recursive: true }); - const hermes = join(bin, "hermes"); + // Install the shim at a path buildRemoteHermesCmd PROBES BY ABSOLUTE PATH + // ($HOME/.local/bin/hermes), not just on PATH. The command runs under + // `bash -lc` (a login shell), which re-sources /etc/profile and RESETS PATH — + // so a shim reachable only via a prepended PATH entry is dropped, and the + // final `command -v hermes` fallback then finds either nothing (clean CI + // container → the test fails) or a real host hermes (dev box → the test + // passes for the wrong reason). Placing the shim at the probed absolute + // location makes the `[ -x $HOME/.local/bin/hermes ]` branch fire first, + // independent of login-shell PATH behavior and of whether a real hermes + // exists on the host. PATH is still prepended as belt-and-suspenders. + const localBin = join(home, ".local", "bin"); + mkdirSync(localBin, { recursive: true }); + const hermes = join(localBin, "hermes"); writeFileSync( hermes, [ "#!/usr/bin/env bash", 'if [ "$1" = "doctor" ]; then', - ' printf "doctor stderr preserved\\n" >&2', + ' printf "doctor stderr preserved\\\\n" >&2', " exit 0", "fi", - 'printf "%s\\0" "$@"', + 'printf "%s\\\\0" "$@"', "", ].join("\n"), ); @@ -53,7 +63,7 @@ function runWithHermesShim(command: string): Buffer { env: { ...process.env, HOME: home, - PATH: `${bin}:${process.env.PATH || ""}`, + PATH: `${localBin}:${process.env.PATH || ""}`, }, }); }