feat: add live WebUI profile descriptions#63
Draft
Isla-Liu wants to merge 171 commits into
Draft
Conversation
…beros SSO Introduce a strategy pattern: the default ssh2-based transport is preserved byte-for-byte, while a new OpenSSH subprocess transport is available opt-in via --transport=openssh (or --kerberos shorthand). This adds GSSAPI/Kerberos authentication, missing from the underlying ssh2 library since 2015 (upstream issue mscdex/ssh2#333). New architecture: src/transports/types.ts ISshTransport interface, ExecResult, TransportConfig src/transports/ssh2.ts Ssh2Transport wraps existing SSHConnectionManager, execSshCommandWithConnection, ensureElevated src/transports/openssh.ts OpenSshTransport spawns ssh binary per exec; supports kerberos/key/password auth; SSH_ASKPASS helper for password; PTY-su state machine with random-nonce sentinel prompts for --suPassword src/transports/factory.ts createTransport(cfg) selects implementation src/utils/shell.ts sanitizeCommand / sanitizePassword / escapeCommandForShell (shared by both transports) New CLI flags: --transport=ssh2|openssh default ssh2 --kerberos implies --transport=openssh + GSSAPI --gssapiDelegateCredentials yes|no (default no) --knownHostsFile pinned known_hosts (openssh only) --strictHostKeyChecking yes|no|accept-new (default accept-new) Backward compatibility: - All 50 pre-existing tests pass unchanged - Default transport remains ssh2 with byte-for-byte preserved behavior - MCP tool contract unchanged: exec and sudo-exec keep {command, description?} - Legacy exports preserved (SSHConnectionManager, execSshCommandWithConnection, execSshCommand, sanitizeCommand, escapeCommandForShell) - Zero new runtime dependencies (uses Node built-ins only) Windows notes: - Win32-OpenSSH lacks ControlMaster (issue PowerShell/Win32-OpenSSH#1328); each exec spawns a fresh ssh.exe -- accept the latency trade-off - Kerberos via SSPI is automatic on domain-joined workstations - Password auth writes a .cmd askpass helper to %TEMP%\ssh-mcp-<pid>\ and passes the password through a per-process env var (never in argv) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
17 new unit tests cover: - classifyError: success, null exit, remote non-zero, all SSH-layer (exit 255) categories including Kerberos-specific failure modes (no credentials cache, ticket expired, GSSAPI failures, clock skew) - OpenSshTransport.buildArgs: kerberos / key / password arg construction, strictHostKeyChecking, knownHostsFile, ConnectTimeout scaling, PTY flag, port, ServerAlive keepalive No Docker or KDC required; all tests run offline in vitest. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rror Discovered during end-to-end validation against an AD-joined Ubuntu server: validateConfig computed `transport = config.transport ?? 'ssh2'` before applying the `--kerberos` implies `--transport=openssh` rule, so passing `--kerberos` without `--transport=openssh` falsely triggered two errors: "--kerberos requires --transport=openssh" "--knownHostsFile and --strictHostKeyChecking require --transport=openssh" Fix: distinguish `transportExplicit` (user-provided) from `transport` (effective after the kerberos implication), and only flag contradiction when the user explicitly passed `--transport=ssh2` alongside `--kerberos`. Also ignore manual-kerberos-test.mjs from the fork history (local scratch). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A single MCP server process can now serve multiple SSH targets, each with
its own auth mode, transport, and sudo/su passwords. Drop-in compatible
with legacy single-host --host/--user invocation.
CLI (new multi-host mode):
--ssh='{"name":"A","host":"a.example","user":"u","auth":"kerberos"}' \
--ssh='{"name":"B","host":"b.example","user":"u","auth":"key","keyPath":"..."}' \
--ssh='{"name":"C","host":"c.example","user":"u","auth":"password","password":"..."}'
Legacy single-host mode preserved unchanged (registered internally as
connection name "default"); legacy and multi-host flags are mutually
exclusive and validation rejects mixing.
MCP tool schema additions:
exec({command, description?, connectionName?})
sudo-exec({command, description?, connectionName?})
list-servers({}) -- new tool
connectionName is optional when only one server is configured
(the first registered becomes the default).
New module src/transports/registry.ts — lazy-init named transports,
concurrent init serialised per name, snapshot for list-servers.
Backward compatibility:
- All 67 existing tests pass unchanged (both integration and unit).
- Legacy src/index.ts exports (SSHConnectionManager, execSshCommand,
execSshCommandWithConnection, sanitizeCommand, escapeCommandForShell)
preserved.
- MCP tools without connectionName behave identically to single-host mode.
Bumped version to 2.1.0.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
resultToMcpContent threw on any non-empty stderr regardless of exit code, so sudo-exec reliably surfaced "Error (code 0):" even when sudo authenticated and the inner command exited cleanly. sudo -p "" -S writes a trailing newline to stderr after consuming the password, and many tools (curl/git/apt/dotnet) write progress to stderr while exiting 0. - src/index.ts: only throw when exitCode !== 0; on success, append substantive stderr to stdout under a [stderr] marker. Export the function so it can be unit-tested. - src/transports/ssh2.ts: tag remote_exit only when exit code is non-zero, matching openssh transport's classifyError. - test/result-mapper.test.ts: 8 unit tests covering the regression (exit 0 + stderr, exit 0 + whitespace stderr, non-zero with stderr, null exit code, category routing).
…tation
- src/audit/{types,redactor,store,rotator}: new audit module
- src/audit/__tests__: unit + integration coverage
- index.ts: wire audit hooks into exec + sudo-exec handlers
- tsconfig.json: include audit module
Acceptance per kanban t_67320e65 sibling review t_66e61c98 PASS.
Default audit dir: ~/.ssh-mcp on Linux/WSL, %USERPROFILE%/.ssh-mcp on Windows.
Rotation: 10MB size-roll + day-roll, retain 10 files.
Redactor: password/PEM/AWS/JWT/Bearer shapes from command/description/stdout/stderr.
Bounded stdout/stderr with truncated flag.
Write failure non-fatal unless strict-mode is configured.
…al audit seam Port the approval engine (engine/gate/manual/smart/yolo/types + barrel) with the build-from-config wiring (buildApprovalEngineFromConfig + dispatcher enqueue/resolve EventEmitter) onto pr/toml-config, and wire it into index.ts: exec/sudo-exec now call gateApproval() before the transport, and the boot path (main + test-mode) builds the production engine from [approval]/per-source TOML via setApprovalEngine. Audit-truth logging is an OPTIONAL seam (Decision D2): src/approval/audit-seam.ts loads src/audit/ via a computed specifier, so the branch builds and unit-passes even though src/audit/ is NOT on pr/toml-config. When the audit module is present it threads the real ApprovalDecision into a JSONL truth record; when absent it degrades to a no-op sink. Unit suites green: approval engine/gate/manual/smart/build-from-config + new audit-seam test + config loader/resolver. npm run build exit 0 (audit absent).
…gistry.profile() test
Port wip/per-source-approval (bda5839) onto pr/approval-engine and add the
registry.profile() coverage the WIP lacked.
Data path (ported from WIP):
- ServerConfig.description + approval?.mode (transports/types.ts)
- TransportRegistry.profile(name) read path returning ResolvedSource
- toml-loader projects out.description / out.approval + validates
source.default (boolean) and source.description (string)
- index.ts gate sites pass registry.profile(connectionName) so the engine
sees the per-source description + override (was { id } only)
New coverage:
- src/transports/__tests__/registry.test.ts (5 tests): profile() surfaces
per-source description+approval, override-free for plain sources, default
fallback, unknown-name throw, and an end-to-end loader->registry->engine
test asserting dc03 resolves to smart (description rides into the LLM
prompt body) while lab falls through to the global yolo default.
- toml-loader.test.ts: loader propagates per-source description + approval.
DATA path only — no web UI mutation endpoints (those are PR-8).
89/89 src/**/__tests__ unit suites green; npm run build exit 0. Only
host-dependent test/* suites fail (ECONNREFUSED 127.0.0.1:2222, no sandbox sshd).
…read surface (D2) # Conflicts: # src/index.ts
PR-7: runtime approval-mode switching with no restart and no TOML write-back (Decision D3 — in-memory override only). Engine: - ApprovalModeStore: net-new in-memory mutable mode store, three layers (live override > static TOML-seeded override > mutable global default), clear-to-reveal semantics, zero fs/path/toml surface. - ApprovalDispatcher: setGlobalMode / setProfileMode / getEffectiveMode / availableModes + mode-changed events. Sub-engines are built once and never torn down; decide() samples the effective mode exactly once, so a switch is atomic and an in-flight manual approval is race-free. A rejected switch (unarmed engine -> ModeUnavailableError) never mutates state or emits an event. - buildApprovalEngineFromConfig pre-arms manual when the WebUI is active and smart when the LLM is fully configured, so a live switch into either works without a restart — while keeping the gate-12 invariant tight (manual+no-WebUI still fatal). WebUI: - ModeController contract + routes: GET /api/approval-modes, PUT /api/profiles/:id/approval-mode (override + clear via mode:null), PUT /api/approval-mode (global). Unavailable mode -> 400 with the armed list; no controller -> 503. - SSE mode-changed broadcast; SseHub subscribes/unsubscribes the controller. - Static UI: per-profile mode <select>, optimistic switch with revert on failure, mode-changed SSE convergence across clients. index.ts wiring: live getApprovalMode lookup, buildWebUIModeController, static overrides seeded from perSourceApproval, gate ctx id aligned to the resolved profile name. Tests (net-new, 31): mode-store precedence/snapshot/in-memory-guard, dispatcher hot-swap + in-flight-race + pre-arm, WebUI routes + SSE. All 141 src unit suites green; build green.
# Conflicts: # src/index.ts # src/transports/types.ts
PR-8 / user target #4 UI+API half. Adds runtime per-source description editing that the approval engine re-reads on its next decision, with NO write-back to the TOML config (Decision D3: in-memory override only). - TransportRegistry: in-memory descriptionOverrides Map; profile()/list() resolve override > TOML; setDescription()/getEffectiveDescription()/ getDescriptionOverride(); clearing (null) reverts to the boot value. - WebUI: PUT /api/sources/:id/description route (set/blank/revert), 404 on unknown source, 400 on bad/over-long body, 503 when disabled. - SourceController contract + index.ts adapter (registry-backed, owns the source-updated fan-out); SSE source-updated broadcast wired into SseHub. - /api/profiles surfaces source_edit_enabled so the UI gates the editor. - Static UI: click-to-edit description cell (textarea + save/revert/cancel), source-updated SSE re-fetch; styles. - Engine re-read proven: a mid-session edit rides into the NEXT smart-mode LLM prompt. No-persistence proven: parsed config untouched, a fresh registry from the same TOML has no override, no fs/persist surface. Tests: +22 (11 registry description-override incl. engine-re-read + no-persistence guard; 11 webui description-edit route + SSE E2E). 169/169 src tests pass; tsc build clean.
Round-1 reviewer fixes for pr/kerberos-transport. All six findings from the R1 review are host-independent and covered by new unit tests. 1. [P1] resultToMcpContent: a successful exec (exitCode 0) is no longer turned into an MCP error by a non-empty stderr. With the default StrictHostKeyChecking=accept-new, OpenSSH prints "Warning: Permanently added ... to the list of known hosts." to stderr while exiting 0 on the first connection to a new host; this previously failed every first connect. stderr now only triggers an error when exitCode !== 0 and no more specific category applies. 2. [P2] buildTransportConfig / resolveAuthMode: restore password-over-key precedence (kerberos > password > key) and stop unconditionally reading the key file. A password config that still carries a stale --key no longer throws ENOENT before connecting; the key is read only when 'key' is the resolved auth mode on the ssh2 path. 3. [P2] runSuViaPty: large su command output is no longer truncated to the ~4KB regex scan window. A separate unbounded EXEC-state capture buffer preserves the full stream, matching runSsh. 4. [P2] runSuViaPty: the overall hard deadline now equals opts.timeoutMs instead of opts.timeoutMs + 30000, honoring the ExecOptions.timeoutMs hard-ceiling contract (types.ts). Handshake phases remain bounded by their own per-state timers. 5. [P2] prepareAskpassHelper: the Windows .cmd helper no longer uses bare `echo %VAR%` (which CMD re-parses, breaking passwords containing & | < > ^ %). It now reads the password verbatim via PowerShell $env:VAR, which never substitutes the value onto a command line. Extracted as the pure, testable renderAskpassHelper(). 6. [P2] execElevated (sudo mode): when only --suPassword is set (no sudo password), sudo-exec now runs the command as root via the su PTY instead of `sudo -n` as the unprivileged user, matching the ssh2 transport's persistent-root semantics for the documented --suPassword setup. Tests: +39 host-independent unit tests pass (test/index.unit.test.ts, test/openssh.unit.test.ts, test/openssh.suvia.unit.test.ts). tsc build clean. The 29 remaining suite failures are pre-existing environmental ECONNREFUSED 127.0.0.1:2222 live-sshd reds (unchanged). Refs Codex review threads 1-6 on PR #2.
…i-host # Conflicts: # src/index.ts
…g (PR #3) Round-1 reviewer findings (all P2), with unit tests: 1. registry.get(): wrap init() in try/finally so a rejected init (e.g. a transient connect timeout) is no longer cached. Previously the rejected initPromise stayed in the map and every later get() replayed the same rejection, so the connection could never retry without a process restart. 2. validateConfig() multi-host: add port, sudoPassword, suPassword to the rejected legacy-flag allowlist. In --ssh mode these are read only from each JSON, so a mixed invocation silently used the wrong port / ran sudo without the password instead of erroring. 3. registry.resolveName(): when connectionName is omitted and more than one server is configured (and no explicit setDefault()), throw instead of silently falling back to the first-registered host — a command could land on the wrong machine. A deliberate setDefault() re-enables the shortcut. 4. parseServerConfigJson(): reject knownHostsFile / strictHostKeyChecking when the resolved transport is ssh2 (which ignores both), mirroring the legacy single-host rule. Previously these were accepted and silently dropped — a host-key-enforcement downgrade. Tests: new test/registry.unit.test.ts (15) and test/multihost-config.unit.test.ts (15). tsc build clean; host-independent unit suite green.
added 30 commits
July 10, 2026 18:13
# Conflicts: # src/config/toml-loader.ts # src/index.ts
# Conflicts: # src/index.ts
…ave3/pr3b-t_24550551
…ower/pr9-t_d57769fb
…ower/pr10-t_d57769fb
…wer/pr11-t_d57769fb
…inal-lower/pr12-t_d57769fb
…-lower/pr13-t_d57769fb
…al-lower/pr13-t_d57769fb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add live in-memory editing of per-profile descriptions from the WebUI, building on the per-source metadata path and mode-switching UI.
Compatibility / safety
Description overrides are presentation metadata only and do not alter transport credentials or execution logic. They are runtime-scoped and reversible.
Test plan
Stack note
This PR is part of a staged stack from fork branches. GitHub displays cumulative diffs because fork branches cannot be used as upstream PR bases; merge/read in the order below.
pr/connection-name-guardpr/toml-configpr/audit-logpr/connection-name-toml-optout— builds on pr/connection-name-guard, pr/toml-configpr/approval-engine— builds on pr/toml-configpr/per-source-approval— builds on pr/approval-enginepr/webui-readonly— builds on pr/audit-log, pr/approval-enginepr/webui-manual-approval— builds on pr/webui-readonlypr/webui-mode-switch— builds on pr/webui-manual-approvalpr/webui-description-edit— builds on pr/per-source-approval, pr/webui-mode-switchpr/config-watch-reload— builds on pr/webui-description-editDependency notes
This PR builds on:
pr/per-source-approval,pr/webui-mode-switch.