feat: add ssh tool for remote command execution over SSH - #192
Conversation
New tools/ssh container tool, mirroring tools/kubectl-readonly and tools/github's shape: a fixed user@host allowlist as the authorization boundary, a read-only remote-command allowlist, and a strict plain-argument charset (the remote command reaches the target's login shell via ssh, unlike the local-spawn-only tools, so this matters more here). Wires it into charts/community-components (Tool/ServiceAccount templates, values.yaml defaults) and .github/workflows/release.yml's build matrix, and enables it in values-production.yaml against the homelab hosts pulled from ~/.ssh/config, using a temporary personal-key credential pending a dedicated scoped key. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fg8b9pPWm91nLbDnB6ECJh
# Conflicts: # .github/workflows/release.yml # package-lock.json
Splits target resolution into two independently optional inputs instead of one bundled SSH_ALLOWED_HOSTS shape: SSH_CONFIG (ssh_config-shaped Host/HostName/User/Port blocks, parsed in the new sshconfig.ts, for alias resolution like ~/.ssh/config already does) and SSH_ALLOWED_HOSTS (now optional -- restricts resolved targets when set, no-ops when unset). At least one must be configured, enforced at startup, since neither would mean no boundary on what this tool dials. allowlist.ts shrinks to just the remote-command allowlist; target.ts (new) owns resolution/restriction and is decoupled from config.ts via its own narrow TargetResolutionConfig interface so it stays independently testable. Updates the Tool CR template's fail guard, values.yaml, README, .env.example, and run.sh to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fg8b9pPWm91nLbDnB6ECJh
Replaces the flat SSH_ALLOWED_HOSTS list with a copy of the operator's own ~/.ssh/config Host aliases (home, bastion, db1-3, kube0-8, printcam, airvinyl, airbuddy) via the new SSH_CONFIG feature, and sets allowedHosts to empty -- the allowlist is disabled for now, per request, with sshConfig's own closed Host list as the sole boundary in the meantime. No new public key needs installing on any target: the credential is still the operator's own id_rsa, already authorized everywhere below for their interactive access. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fg8b9pPWm91nLbDnB6ECJh
|
🤖 Starting work on this now. Watch live or take over the session here: https://claude.ai/code/session_012RcHQ8DZay6svwBFRZMwhY |
There was a problem hiding this comment.
Solid, well-documented tool that closely mirrors the existing tool shape, and the local-vs-remote-shell reasoning behind the charset guard is exactly right. Three things to address before merge, in priority order:
- The read-only guarantee doesn't hold —
ipandfindare allowlisted with no subcommand/flag restriction, soip link set … down,ip route del …, andfind … -delete/-fprintall pass validation (inline onallowlist.ts). - CI is red — the
validatejob fails because the two new templates aren't enabled invalues-ci-all.yaml(inline ontool-ssh.yaml). - Production ships with no real target boundary — with
allowedHostsempty, the sshConfig alias list isn't enforced; literaluser@hosttargets bypass it (inline onvalues-production.yaml).
The remaining CI checks (Node, Go) are green as of this review.
| "w", | ||
| "ss", | ||
| "netstat", | ||
| "ip", |
There was a problem hiding this comment.
ip and find break the "strictly read-only" guarantee. Unlike systemctl/docker, neither is subcommand-restricted, and their state-changing forms fit SAFE_TOKEN, so these all pass validateCommand (verified against this logic):
ip link set eth0 down,ip route del default,ip addr add 10.0.0.9/24 dev eth0find /tmp/x -delete,find / -name foo -fprint /etc/cron.d/x
With the production admin key (values-production.yaml) this is remote write / network-DoS / arbitrary-file-write, not diagnostics.
🤖 Prompt to fix this with an AI agent
In imaustink/agent-controller on PR #192, tools/ssh/src/allowlist.ts allowlists `ip` and `find` with no subcommand/flag restriction, so state-changing invocations (`ip link set … down`, `ip route add/del`, `ip addr add/del`, `find … -delete`, `find … -fprint/-fls/-fprintf`, `find … -exec`) pass validateCommand and violate the tool's read-only guarantee. Restrict `ip` to read-only forms only (allow object + `show`/`list`; reject `add`/`del`/`set`/`change`/`replace`/`flush`), and either drop `find` from the allowlist or reject its writing/execution action flags anywhere in argv (-delete, -exec, -execdir, -ok, -okdir, -fprint, -fprintf, -fls). Add tests in allowlist.test.ts asserting each of those write forms is rejected with BlockedCommandError. Do not change unrelated behavior.
| @@ -0,0 +1,55 @@ | |||
| {{- if .Values.sshTool.enabled }} | |||
There was a problem hiding this comment.
This PR turns CI red. .github/workflows/validate-crds.yml renders the chart with values-ci-all.yaml and asserts every file under templates/ appears in the output. tool-ssh.yaml and serviceaccount-ssh.yaml render nothing there because sshTool is disabled by default and values-ci-all.yaml was not updated to enable it, so the "Assert every template was actually rendered" step exits 1 (this is the failing validate check on the PR).
🤖 Prompt to fix this with an AI agent
In imaustink/agent-controller on PR #192, the validate-crds CI job fails because charts/community-components/templates/tool-ssh.yaml and serviceaccount-ssh.yaml render nothing under charts/community-components/values-ci-all.yaml. Add a `sshTool` block to values-ci-all.yaml with `enabled: true` and a non-empty `sshConfig` (a throwaway Host block, e.g. HostName ci-host.example.invalid / User ci) so the tool-ssh.yaml `fail` guard passes and both templates render. Verify `helm template charts/community-components -f charts/community-components/values-ci-all.yaml` includes both `# Source:` lines. Do not change unrelated behavior.
| enabled: true | ||
| image: registry.kurpuis.com:5000/ssh:latest | ||
| serviceAccountName: ssh-tool | ||
| allowedHosts: "" |
There was a problem hiding this comment.
With allowedHosts empty, the sshConfig Host list is not an enforced boundary. resolveTarget falls back to the caller's literal user@host when no Host block matches (tools/ssh/src/target.ts:91; intended per the test at target.test.ts:62). So callers aren't limited to the aliases below — root@192.168.1.59 … (note: a different user than the ubuntu this file maps kube0 to) or any literal user@host resolves and is dialed, with known_hosts pinning as the only remaining restriction. The comment above claims "sshConfig's own Host list is the only boundary," but that list restricts neither the host set nor the user. Combined with the temporary personal admin key, populating allowedHosts (which pins user+host+port) is the only thing that actually bounds this.
🤖 Prompt to fix this with an AI agent
In imaustink/agent-controller on PR #192, charts/community-components/values-production.yaml enables sshTool with allowedHosts: "" and relies on the sshConfig alias list as the boundary, but resolveTarget (tools/ssh/src/target.ts) dials any literal user@host that isn't a configured alias, so the alias list is not an authorization boundary. Set allowedHosts to a comma-separated user@host list derived from the sshConfig Host blocks in this file (e.g. austinkurpuis@166.113.38.187, ubuntu@192.168.1.59, …, pi@airvinyl.local, pi@192.168.1.218) so the resolved user+host+port must match an explicit entry. Do not change the sshConfig block or unrelated behavior.
|
All three inline comments anchored to the correct file and line. Review is posted. SummaryI reviewed PR #192 ( Findings (most severe first):
✅ Opened pull request: imaustink/agent-controller#192 |
- allowlist.ts: restrict ip to a read-only object+action allowlist (addr/route/link/... + show/list/get, ip's own default action) and reject find's write/exec primaries (-delete/-exec[dir]/-ok[dir]/ -fprint/-fprintf/-fls) wherever they appear in argv -- both previously passed validateCommand unrestricted, breaking the strictly-read-only guarantee. Adds tests for the exact attack forms from review (ip link set down, ip route del, find -delete/-exec/ -fprint). - values-ci-all.yaml: enables sshTool with a throwaway sshConfig so tool-ssh.yaml/serviceaccount-ssh.yaml actually render under CI's full-catalog validation; verified against the exact validate-crds.yml assertion plus a real kind-cluster server-side dry-run apply. - values-production.yaml: populates allowedHosts, derived from the sshConfig Host list -- resolveTarget falls back to any literal user@host when no alias matches, so with allowedHosts empty the sshConfig list was restricting nothing at all (any host reachable with this key, as any user, would resolve and dial). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fg8b9pPWm91nLbDnB6ECJh
Adds SSH_ALLOWED_COMMANDS as a third independent, optional input (alongside SSH_ALLOWED_HOSTS/SSH_CONFIG for target resolution): unset keeps the curated read-only default; a comma-separated list substitutes a custom allowed-command set; "*" disables the command/subcommand allowlist entirely for a deployment that has decided read-only diagnostics aren't enough and accepts the risk. The remote-shell-injection charset check in allowlist.ts is unconditional in every mode, including "*" -- wide open means "any command, no shell injection", not "no restriction at all". The Tool CR's description/allowedRoles now reflect which mode is active (reader for the default, writer for "*"). Sets allowedCommands: "*" in values-production.yaml per explicit request, against the operator's own homelab hosts they're comfortable an agent could break -- documented inline as a decision not to copy elsewhere. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fg8b9pPWm91nLbDnB6ECJh
|
Addressed all three findings:
Separately (not from this review — a follow-up request): added a third independent, optional input, All 48 tests passing, typecheck/build/helm-lint/helm-template clean, and a real kind-cluster dry-run apply verified locally. |
allowedCommands: "*" gives the ssh tool write access to anything it reaches -- kube0-8 and db1-3 are the actual cluster/database nodes this repo runs on, too much blast radius to leave wide open with a personal admin key backing it. Removed both from sshConfig and allowedHosts; kept home/bastion/printcam/airvinyl/airbuddy. Added console (192.168.1.83, user admin) instead. Its host key isn't in SSH_KNOWN_HOSTS yet -- it was unreachable (ssh-keyscan timeout) when regenerating the Secret prerequisite; noted inline with the ssh-keyscan command to add it once it's back on the network. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fg8b9pPWm91nLbDnB6ECJh
console's DHCP lease moved it to 192.168.1.71. Verified via ssh-keyscan that the ed25519/rsa/ecdsa host keys at the new address match what's already recorded for it under its console.local mDNS name, so this is the same box, not a different host to re-vet from scratch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NynLp3kMJ5vMdvHDv3opGy
`ssh -i ~/.ssh/id_rsa admin@192.168.1.71` returns "Permission denied (publickey)" -- unlike home/bastion/printcam/airvinyl/airbuddy, id_rsa is not in admin's authorized_keys on this box. The prior "no new public key needs installing" claim didn't hold for console; noted inline so the ssh tool's console calls failing at the auth step isn't mistaken for an allowlist bug. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fg8b9pPWm91nLbDnB6ECJh
Summary
tools/ssh, a container Tool that runs a remote command over SSH, mirroringtools/kubectl-readonly/tools/github's shape (allowlist + config + spawn wrapper + messaging + redaction, with tests).tools/ssh/src/target.ts,src/allowlist.ts,src/sshconfig.ts):SSH_ALLOWED_HOSTS-- an explicituser@host[:port]allowlist restricting which resolved targets may be dialed at all.SSH_CONFIG-- ssh_config(5)-shapedHost/HostName/User/Portblocks for alias resolution (e.g.kube0->ubuntu@192.168.1.59), the same way an operator's own~/.ssh/configworks. At least one ofSSH_ALLOWED_HOSTS/SSH_CONFIGis required at startup.SSH_ALLOWED_COMMANDS-- unset keeps the curated read-only diagnostic default (df,ps,journalctl,systemctl status,docker ps/logs/inspect,ip addr show, ...;systemctl/dockerrestricted to read-only subcommands,ip/findreject their write/exec forms specifically); a comma-separated list substitutes a custom set;"*"disables the command allowlist entirely for a deployment that accepts that risk.^[A-Za-z0-9._\-/:=@,]+$) is unconditional in every mode, including"*"-- OpenSSH hands the remote argv to the target's own login shell unless it forces a fixed command, so this is what actually prevents;/|/`/$(...)injection regardless of which top-level command is allowed.charts/community-components(Tool/ServiceAccounttemplates,values.yamldefaults, afailguard requiring at least one target-resolution input,values-ci-all.yamlfor full-catalog CI validation) and.github/workflows/release.yml's build matrix.values-production.yamlagainst the operator'shome/bastion/console/printcam/airvinyl/airbuddyhomelab boxes (aliased via a copy of~/.ssh/config, withallowedHostspopulated from the same list as the actual enforced boundary) -- deliberately excludes the kube0-8/db1-3 cluster nodes, sinceallowedCommands: "*"gives this tool write access to anything it reaches and those are this repo's own production infrastructure. Uses the operator's ownid_rsaas a temporary shared credential pending rotation to a dedicated, scoped key (flagged inline in the values file andtools/ssh/README.md's "Choosing a credential" section).Review history
kindcluster +kubectl apply --dry-run=server(not justhelm template):ip/findallowed unrestricted write/exec forms despite the "read-only" claim; CI was red becausevalues-ci-all.yamlnever enabledsshTool; andallowedHostsbeing empty meant thesshConfigalias list wasn't actually an enforced boundary (any literaluser@hostwould resolve).console's IP changed (192.168.1.83->.71, confirmed same box via host-key fingerprint match) mid-review; updated everywhere. Itsid_rsaauthorization is still outstanding on the operator's end -- flagged inline, not blocking this PR.Test plan
npm run typecheck --workspace=sshnpm run test --workspace=ssh(48/48 passing acrossallowlist.test.ts/sshconfig.test.ts/target.test.ts)npm run build --workspace=sshhelm lint charts/community-componentshelm templateagainstvalues.yamldefaults (disabled), a custom host list,SSH_CONFIG-only, both together, andvalues-production.yamlas-is -- all render cleanly; thefailguard rejects neither-input-set.helm template community-components -f values-ci-all.yaml, verified againstvalidate-crds.yml's literal "every template rendered" assertion.kindcluster +kubectl apply --dry-run=serveragainst the full rendered catalog (CEL validation included) -- passes for thesshTool CR.ToolRunagainst a homelab host oncessh-tool-secretsis created (command in thevalues-production.yamlcomment) andconsole's key authorization is resolved.🤖 Generated with Claude Code
https://claude.ai/code/session_01Fg8b9pPWm91nLbDnB6ECJh