Skip to content

feat: add built-in secure sudo password prompt#2101

Open
uiYzzi wants to merge 3 commits into
MoonshotAI:mainfrom
uiYzzi:feat/sudo-askpass
Open

feat: add built-in secure sudo password prompt#2101
uiYzzi wants to merge 3 commits into
MoonshotAI:mainfrom
uiYzzi:feat/sudo-askpass

Conversation

@uiYzzi

@uiYzzi uiYzzi commented Jul 23, 2026

Copy link
Copy Markdown

Related Issue

Resolve #2090 (also addresses the pain point reported in #249)

cc @sailist @liruifengv — would appreciate a review, especially on the interaction-kernel redaction and the new REST/WS surface.

Problem

When the agent's Bash tool runs a command that invokes sudo, the command fails because stdin is closed and there is no TTY to read the password from. Users have to run privileged commands manually in another terminal, or build their own SUDO_ASKPASS hacks. See the linked issue for details.

What changed

A built-in secure sudo password prompt, for both the TUI and the web UI:

  • Askpass channel (both engines): the Bash tool injects SUDO_ASKPASS (+ a per-session 128-bit token and the triggering command) into spawned commands on macOS/Linux. The per-session helper dir (mode 0700) holds helper.sh/helper.mjs and a unix socket; sudo auto-invokes the helper when it needs a password (no TTY is present), the helper authenticates over the socket, and the engine issues one password request per connection. Deep session dirs that would exceed the 104-byte sun_path cap fall back to a short session-keyed dir under the OS temp dir.
  • TUI (agent-corenode-sdkapps/kimi-code): new requestPassword reverse RPC mirroring the approval flow, rendered as a masked () input dialog showing the sudo prompt and the triggering command. The submitted value is never trimmed, so whitespace-containing passwords work.
  • Web (agent-core-v2kap-serverapps/kimi-web): new password interaction kind surfaced as event.password.requested/resolved + GET/POST /api/v1/sessions/{id}/passwords + pending_passwords in the snapshot, rendered as a masked PasswordCard (top priority over question/approval cards, since sudo is blocking a running process).
  • Security properties: the password flows only client → in-memory resolution → socket → helper stdout → sudo. It never enters the model context, wire.jsonl (the interaction kernel journals a redacted {cancelled} response and broadcasts only that shape), the transcript, WS events, REST envelopes/snapshots, request logs, or telemetry. Helper connections that disconnect mid-prompt auto-cancel. Cancelling the prompt makes the helper exit 1, so sudo fails with its normal error — identical to today's UX. Commands containing sudo still go through the normal permission approval flow.
  • Config: [sudo_askpass] enabled = false in config.toml disables the feature (default enabled; no-op on Windows). One config shape works for both engines.
  • Docs: config-files.md (en/zh) new [sudo_askpass] section; reference/tools.md (en/zh) Bash section documents the behavior.

Known consideration for reviewers: as with any askpass-based design (e.g. Cursor's), the token travels in the spawned command's environment, so a malicious command could present a phishing prompt via the socket — the dialog always shows the sudo prompt text and the triggering command to help users judge. The token only authenticates same-user local connections, and no password is ever exposed to the model either way.

Testing

  • New unit/integration tests across all touched packages, including fake-sudo end-to-end tests (a temp sudo on PATH that invokes $SUDO_ASKPASS) proving submit and cancel paths through the real Bash tool layer, socket roundtrips with the real spawned helper, token rejection, journaling redaction, WS/REST shapes, reducer/mapper coverage, and dialog masking assertions. No real sudo is used in tests.
  • Full suites green: agent-core 4003, agent-core-v2 4001, kap-server 723, node-sdk 234, kimi-code 2388, kimi-web 664, klient + protocol suites; typechecks clean across all touched packages and consumers (acp-adapter, vscode).

Checklist

  • I have read the CONTRIBUTING document.
  • I have linked a related issue, or explained the problem above.
  • I have added tests that prove my feature works.
  • Ran gen-changesets skill, or this PR needs no changeset.
  • Ran gen-docs skill, or this PR needs no doc update.

@changeset-bot

changeset-bot Bot commented Jul 23, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 77a2baf

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@moonshot-ai/kimi-code Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 95973403c8

ℹ️ 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".

Comment thread packages/agent-core-v2/src/os/backends/node-local/tools/bash.ts
Comment thread packages/agent-core-v2/src/session/sudoAskpass/sudoAskpassService.ts Outdated
uiYzzi added a commit to uiYzzi/kimi-code that referenced this pull request Jul 23, 2026
sudo 1.9+ only consults SUDO_ASKPASS when askpass mode is active, so
env injection alone never triggered the password prompt — plain sudo
invocations still failed with the no-TTY error (spotted in review on
PR MoonshotAI#2101).

Both engines now write a bin/sudo shim (exec real-sudo -A "$@") into
the per-session askpass dir and prepend it to the spawned command's
PATH. -S/--stdin callers pass through without -A (sudo rejects that
combination); -n keeps its non-interactive semantics; an explicit
caller-side -A is harmless. Real-sudo smoke tests (cancel-only, no
credentials) plus -n/-A/-S flag tests cover the behavior, and the
fake-sudo e2e tests now mimic sudo 1.9's -A requirement.

Also folds the new agent-core-v2 inline comments into file headers
per the package's header-only comment convention.
uiYzzi added a commit to uiYzzi/kimi-code that referenced this pull request Jul 23, 2026
sudo 1.9+ only consults SUDO_ASKPASS when askpass mode is active, so
env injection alone never triggered the password prompt — plain sudo
invocations still failed with the no-TTY error (spotted in review on
PR MoonshotAI#2101).

Both engines now write a bin/sudo shim (exec real-sudo -A "$@") into
the per-session askpass dir and prepend it to the spawned command's
PATH. -S/--stdin callers pass through without -A (sudo rejects that
combination); -n keeps its non-interactive semantics; an explicit
caller-side -A is harmless. Real-sudo smoke tests (cancel-only, no
credentials) plus -n/-A/-S flag tests cover the behavior, and the
fake-sudo e2e tests now mimic sudo 1.9's -A requirement.

Also folds the new agent-core-v2 inline comments into file headers
per the package's header-only comment convention.
@uiYzzi
uiYzzi force-pushed the feat/sudo-askpass branch from 2eeb4d7 to fd6d8b8 Compare July 23, 2026 10:18
uiYzzi added 2 commits July 23, 2026 19:04
When a Bash tool command invokes sudo and a password is required, the
Bash tool injects a per-session askpass helper (SUDO_ASKPASS) into the
spawned environment. sudo invokes the helper, which authenticates over
a per-session unix socket (0700 dir, random 128-bit token) and triggers
a masked local password prompt in the connected client:

- TUI: masked input dialog via a new requestPassword reverse RPC
  (agent-core SDKAPI -> node-sdk -> TUI reverse-rpc controller)
- Web: new 'password' interaction kind in agent-core-v2, surfaced by
  kap-server as event.password.requested/resolved + REST
  /sessions/{id}/passwords, rendered as a masked PasswordCard in
  kimi-web

The password flows only client -> in-memory resolution -> socket ->
helper stdout -> sudo. It never enters the model context, wire.jsonl
(journaled responses are redacted at the interaction kernel), the
transcript, WS broadcasts, logs, or telemetry.

Enabled by default on macOS/Linux; disable with
[sudo_askpass] enabled = false in config.toml. Commands containing
sudo still go through the normal permission approval flow.

Resolve MoonshotAI#2090
sudo 1.9+ only consults SUDO_ASKPASS when askpass mode is active, so
env injection alone never triggered the password prompt — plain sudo
invocations still failed with the no-TTY error (spotted in review on
PR MoonshotAI#2101).

Both engines now write a bin/sudo shim (exec real-sudo -A "$@") into
the per-session askpass dir and prepend it to the spawned command's
PATH. -S/--stdin callers pass through without -A (sudo rejects that
combination); -n keeps its non-interactive semantics; an explicit
caller-side -A is harmless. Real-sudo smoke tests (cancel-only, no
credentials) plus -n/-A/-S flag tests cover the behavior, and the
fake-sudo e2e tests now mimic sudo 1.9's -A requirement.

Also folds the new agent-core-v2 inline comments into file headers
per the package's header-only comment convention.
@uiYzzi
uiYzzi force-pushed the feat/sudo-askpass branch from fd6d8b8 to 9d793a2 Compare July 23, 2026 11:14
@sailist

sailist commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Thanks for putting this together — it's a lot of work, and it covers both the TUI and Web UI across both engines.

We're not going to merge this right now, for a few reasons. One of them is agent-core-v2: its layering rules and design principles haven't been written down yet (no skills, no AGENTS.md), so it's really easy for outside contributions to break the structure without realizing it. This PR hits exactly that — the dependency between os and sudo askpass goes the wrong way. os is the lowest layer and shouldn't know about anything above it, but os/backends/node-local/tools/bash.ts imports ISessionSudoAskpassService from the session layer. It should be the other way around. Beyond v2, a feature touching both engines plus new REST/WS surface is also something we'd want to design and land carefully on our own schedule.

That said, we're keeping this PR open rather than closing it — once v2 settles down and we've written up the guidelines, we may come back to it for reference or pick it up directly. The feature itself is genuinely useful, and the sudo-stdin pain (#2090, #249) is on our radar. If we still haven't shipped something like this by then, please bug us about it. ❤️

…ayer

The Bash tool (os/backends, L6) imported ISessionSudoAskpassService from
the session domain (L7) — an upward dependency against the layering
rules, surfaced during review on PR MoonshotAI#2101. The env-provider contract
now lives in os/interface (L1) as ISudoAskpassEnvProvider; the
Session-scope sudoAskpass channel implements and registers it there, so
every import flows downward and the layer-check exception is removed.
@uiYzzi

uiYzzi commented Jul 24, 2026

Copy link
Copy Markdown
Author

Thanks for the detailed feedback — totally understand holding this until v2's guidelines are written down.

The layering point was spot-on: os/backends importing ISessionSudoAskpassService from the session domain was an upward dependency. Reworked in 77a2baf: the env-provider contract now lives in os/interface (L1) as ISudoAskpassEnvProvider, and the Session-scope sudoAskpass channel implements and registers it there — every import now flows downward, and the layer-check exception I had added is removed (lint:domain clean, full v2 suite green).

No expectation on timing from my side — feel free to pick this up whenever it fits your schedule, or mine it for reference. Happy to keep it rebased in the meantime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] 内置安全的 sudo 密码输入提示 / Built-in secure sudo password prompt (like Cursor)

2 participants