feat(chat): handle gateway sudo.request / secret.request via hardened modal#606
feat(chat): handle gateway sudo.request / secret.request via hardened modal#6060xr00tf3rr3t wants to merge 3 commits into
Conversation
Greptile SummaryThis PR wires up the gateway's
Confidence Score: 3/5Safe to merge after adding a The sudo/secret collect-and-forward chain fires unconditionally once the modal resolves, even if the turn was cancelled or errored while the modal was open. The src/main/hermes.ts — the sudo/secret event handler around the Important Files Changed
Reviews (1): Last reviewed commit: "feat(chat): handle gateway sudo.request ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| void collect | ||
| .then((answer) => { | ||
| 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); | ||
| }) |
There was a problem hiding this comment.
finished guard before forwarding credentials to the gateway. If cancel() or finish() is called while the askpass modal is still open (e.g. the user aborts the chat turn), finished becomes true but the modal stays open with no way to dismiss it programmatically. When the user eventually submits the modal, collect resolves and the .then() fires unconditionally — forwarding the sudo password or secret value to the gateway on a session that has already been torn down. The clarify path avoids this by clearing pendingClarify in finish()/cancel(), making stale responses a no-op; the same protection is missing here.
| void collect | |
| .then((answer) => { | |
| 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); | |
| }) | |
| void collect | |
| .then((answer) => { | |
| if (finished) return; | |
| 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); | |
| }) |
| export async function promptSecretValue( | ||
| envVar: string, | ||
| prompt: string, | ||
| ): Promise<string> { | ||
| 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 ?? ""; | ||
| } |
There was a problem hiding this comment.
envVar used verbatim in the modal heading without sanitisation. envVar comes directly from event.payload.env_var in the gateway event stream — an untrusted, agent-controlled string. It flows into the heading option and is rendered as <div class="title">${safeHeading}</div> in buildDialogHtml, which does HTML-escape it, so the XSS risk in the rendered DOM is mitigated. However, the raw value also appears in the title option, which is used as the OS-level window title (win.title), where HTML escaping is irrelevant. A length cap and simple printable-chars validation on envVar would close the spoofing surface entirely.
… modal
The stock build dead-ends sudo.request / secret.request the same way clarify
used to — it interrupts the turn with "Hermes One does not yet expose that
gateway dialog," so any agent command needing sudo or a secret value can't
proceed from the desktop.
Unlike clarify (an inline transcript card), a sudo password / secret value is
sensitive and must never land in chat scrollback. So this reuses the installer's
existing hardened askpass modal (showPasswordDialog): CSP-locked
default-src 'none', sandboxed, contextIsolation, ephemeral data-URL, value never
persisted. showPasswordDialog is parameterized with title/heading (defaults
unchanged for the installer caller) and exported.
- gatewayPrompt.ts: promptSudoPassword() / promptSecretValue(envVar, prompt)
wrap the modal; a new setGatewayPromptParent() lets index.ts hand in the main
window. Cancel maps to "" — a safe skip the gateway already handles
(secret.request -> {skipped:true}; sudo lets terminal sudo fail cleanly).
- hermes.ts: the sudo/secret stream handler collects via the modal and forwards
with the gateway's exact shapes — sudo.respond {request_id, password} and
secret.respond {request_id, value} (verified against tui_gateway/server.py).
- askpass-security.test.ts: +4 cases pinning modal reuse, no-log/no-persist,
the exact respond shapes, and cancel->skip.
Stacks on the inline-clarify PR (fathah#604). Suite 1052 pass (+4); typecheck, lint
(--quiet), build all green.
…quest - Guard the .then() callback with a check so credentials are not forwarded to a dead gateway session if the turn was cancelled while the askpass modal was open (Greptile finding, confidence 3/5). - For secret.request: try getSecret(env_var) from the configured security provider first. If the vault already holds the key, answer silently without prompting the user. Fall back to the interactive modal only when the vault has no value for that key. - sudo.request always requires an interactive password — no vault lookup.
0xr00tf3rr3t
force-pushed
the
feat/inline-sudo-secret
branch
from
fcc44c7 to
c07534e
Compare
|
Addressed Greptile's finding + added security provider integration: 1. 2. Vault-first secret resolution — for Rebased onto current main. |
…tainer tests/ssh-remote.test.ts wrote its fake `hermes` shim into $HOME/bin and made it reachable only by prepending that dir to PATH. The command under test (buildRemoteHermesCmd) runs under `bash -lc` — a LOGIN shell that re-sources /etc/profile and resets PATH — and probes a list of ABSOLUTE venv paths before falling back to `command -v hermes`. In a clean CI container (node:22-bookworm) there is no real `hermes` anywhere and the prepended PATH entry does not survive the login shell, so `command -v hermes` finds nothing and the command exits 1 with "hermes CLI not found" — 4 failing cases. On a dev box the same test passed for the WRONG reason: `command -v hermes` resolved a real host hermes. Fix is test-only: install the shim at $HOME/.local/bin/hermes — a path buildRemoteHermesCmd probes BY ABSOLUTE PATH ([ -x $HOME/.local/bin/hermes ]) before the PATH-dependent fallback. The shim is now hit deterministically, independent of login-shell PATH behavior and of whether a real hermes exists on the host. PATH is still prepended as belt-and-suspenders. No production code changes. Verified GREEN in the real CI image via `forgejo-runner exec` on node:22-bookworm: tests/ssh-remote.test.ts 17/17 pass (was 4 failing). Typecheck clean on the tracked surface.
1 participant
Summary
sudo.request/secret.requestprompts instead of dead-ending the turn with "Hermes One does not yet expose that gateway dialog" (the same dead-end feat(chat): render gateway clarify.request as an inline card #604 removed forclarify.request)default-src 'none',script-src 'none', sandboxed,contextIsolation, ephemeral data-URL) and never rendered into the chat transcript or persistedshowPasswordDialogis parameterized withtitle/heading(defaults unchanged, so the installer's existing call is untouched) and exported;gatewayPrompt.tswraps it aspromptSudoPassword()/promptSecretValue(envVar, prompt)sudo.respond { request_id, password }andsecret.respond { request_id, value }— and Cancel maps to an empty answer, a safe skip the gateway already handles (secret.request→{ skipped: true }; sudo -S -p '' lets the terminal prompt fail cleanly), so the turn never wedgesWhy a modal, not an inline card
clarify.request(#604) is a non-sensitive question → inline transcript card. A sudo password or secret must never land in scrollback, so this reuses the installer's already-hardened askpass dialog rather than introducing a new surface. Same trust boundary the app already uses to collect the installer's sudo password.Testing
npm run typechecknpm run buildnpm run lint -- --quiet(no new problems from this change; one pre-existing error insrc/main/ssh-remote.tsis untouched here)git diff --checknpm test— 1052 passing (+4 newaskpass-security.test.tscases: modal reuse, no-log/no-persist, exact respond shapes, cancel→skip)Note
Stacked on #604 (inline clarify card) — that branch is its base, so the diff shows the clarify commits too until #604 merges. The only new commit here is the top one (
feat(chat): handle gateway sudo.request / secret.request via hardened modal). Happy to rebase ontomainonce #604 lands.