From b9aef97d6bb9368cc4cecf4f753140adf51a5864 Mon Sep 17 00:00:00 2001 From: jettwang Date: Fri, 28 Aug 2026 23:54:14 +0800 Subject: [PATCH] feat: honor shebang, fix safety false positives, release v0.12.0 Match destructive commands in command position so read-only diagnostics are no longer blocked. Run script payloads under their shebang (or --shell), discover docker SQL roles from container env, and classify a missing remote psql/sqlite3 as config. Expand Windows CI to app, sshclient, and runtimepath. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/workflows/ci.yml | 15 +- AGENT.md | 26 +- CHANGELOG.md | 57 +++ README.md | 4 + SECURITY.md | 4 +- docs/agent-scripting.md | 1 + docs/mcp.md | 2 +- docs/security-guidelines.md | 13 +- docs/zh/agent-scripting.md | 1 + docs/zh/security-guidelines.md | 13 +- internal/app/agentmode_test.go | 16 +- internal/app/app_test.go | 2 +- internal/app/audit_test.go | 12 +- internal/app/config.go | 2 + internal/app/mcp.go | 7 + internal/app/mcp_test.go | 23 + internal/app/run.go | 21 +- internal/app/settings_test.go | 55 +- internal/app/sql.go | 94 +++- internal/app/sql_docker_test.go | 95 ++++ internal/app/testhome_test.go | 17 + internal/app/usage.go | 6 + internal/execution/executor.go | 2 +- internal/execution/payload.go | 54 +- internal/execution/payload_shebang_test.go | 103 ++++ internal/execution/validate.go | 5 +- internal/runtimepath/path_test.go | 4 + internal/skillinstall/install_test.go | 4 + internal/sqlsafe/credsource.go | 30 +- internal/sshclient/client.go | 58 ++- internal/sshclient/client_test.go | 8 +- internal/sshclient/runcommand_test.go | 2 +- internal/sshclient/testhome_test.go | 17 + internal/sshclient/validate.go | 121 ++--- internal/sshclient/validate_destructive.go | 476 ++++++++++++++++++ .../sshclient/validate_destructive_test.go | 153 ++++++ skills/sshx/SKILL.md | 24 +- tests/e2e/harness_test.go | 40 +- tests/e2e/run_e2e_test.go | 70 +++ 39 files changed, 1423 insertions(+), 234 deletions(-) create mode 100644 internal/app/sql_docker_test.go create mode 100644 internal/app/testhome_test.go create mode 100644 internal/execution/payload_shebang_test.go create mode 100644 internal/sshclient/testhome_test.go create mode 100644 internal/sshclient/validate_destructive.go create mode 100644 internal/sshclient/validate_destructive_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4d6e7f6..c14cf4f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -79,11 +79,16 @@ jobs: run: go mod download - name: Run unit tests (cross-platform packages) - # Full-suite Windows enablement is tracked in issue #50: several - # pre-existing app/plugin/skillinstall/sshclient tests assume POSIX - # permission and symlink semantics. Packages listed here must stay - # green; grow this list as tests are ported. - run: go test -short ./cmd/... ./internal/execution/... ./internal/keyringstore/... ./internal/sqlsafe/... ./pkg/... + # Windows is a first-class target, so the CLI surface (internal/app), + # the SSH core (internal/sshclient), and the runtime-root resolver run + # here too. Tests resolve the home directory through setTestHome so + # USERPROFILE is honored, and POSIX permission assertions are guarded + # by runtime.GOOS. + # + # Still excluded (tracked in issue #50): internal/plugin and + # internal/skillinstall assert symlink and permission semantics that + # need Windows equivalents, and tests/e2e has not been ported. + run: go test -short ./cmd/... ./internal/app/... ./internal/execution/... ./internal/keyringstore/... ./internal/runtimepath/... ./internal/sqlsafe/... ./internal/sshclient/... ./pkg/... - name: Vet run: go vet ./... diff --git a/AGENT.md b/AGENT.md index 9528383..1839a3e 100644 --- a/AGENT.md +++ b/AGENT.md @@ -243,9 +243,18 @@ Notes: ### CI (`.github/workflows/`) -- `ci.yml`: **Test** (ubuntu + macOS, Go 1.25.13, `-race -cover`), **Lint** - (golangci-lint), **Security Scan** (`gosec` plus `govulncheck`), **Analyze** - (CodeQL, Go). +- `ci.yml`: **Test** (ubuntu + macOS, Go 1.25.13, `-race -cover`), + **Test (windows-latest)** (`-short`, build + vet), **Lint** (golangci-lint), + **Security Scan** (`gosec` plus `govulncheck`), **Analyze** (CodeQL, Go), + and **E2E** (ubuntu + macOS). +- Windows is a first-class target, so its job covers `cmd`, `internal/app`, + `internal/execution`, `internal/keyringstore`, `internal/runtimepath`, + `internal/sqlsafe`, `internal/sshclient`, and `pkg`. When adding a test that + touches the home directory, use the package's `setTestHome` helper rather + than `t.Setenv("HOME", …)`: Go reads `USERPROFILE` on Windows. Guard POSIX + permission assertions with `runtime.GOOS != "windows"`. `internal/plugin`, + `internal/skillinstall`, and `tests/e2e` are still excluded pending Windows + symlink/permission equivalents (issue #50). - `release.yml`: builds release artifacts with Go 1.25.13 and bundles the matching Agent skill in every archive. @@ -281,10 +290,15 @@ tests. true only when the remote command starts with `sudo`, matching the exact form `sudoStdinCommand` can safely rewrite. Non-leading sudo inside shell wrappers or pipelines is left untouched. -5. **Command safety checks.** Destructive patterns (`rm -rf /`, `mkfs`, `dd`, +5. **Command safety checks.** Destructive operations (`rm -rf /`, `mkfs`, `dd`, fork bombs, `curl | sh`, critical file edits, shutdown/reboot) are blocked - unless `--force`/`-f` or `--no-safety-check` is given. Direct database - client execution (`psql`/`pgcli` in command position, including + unless `--force`/`-f` or `--no-safety-check` is given. Matching happens on + the token in **command position** after shell segmentation, never on the raw + command string: `last reboot -F`, `journalctl | grep -iE 'fail|halt'`, and + `iptables-save | grep -F ...` are reads and must stay allowed. A guardrail + that fires on reads trains the caller to pass `--force` reflexively, which + defeats its purpose — treat a new false positive as a bug. Direct database + client execution (`psql`/`pgcli`/`sqlite3` in command position, including `docker exec`, `sudo -u`, `sh -c`, `kubectl exec`, and pipe wrappers) is also blocked and redirected to `sshx sql`; availability probes such as `which psql` and `psql --version` stay allowed. The validator is a diff --git a/CHANGELOG.md b/CHANGELOG.md index 285ec70..e9f608b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,63 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.12.0] - 2026-08-28 + +### Changed + +- Windows CI now runs the CLI surface (`internal/app`), the SSH core + (`internal/sshclient`), and `internal/runtimepath` in addition to the + previously covered packages, taking the Windows matrix from 91 to roughly 320 + tests. Tests resolve the home directory through a portable helper so + `USERPROFILE` is honored, and POSIX permission assertions are guarded by + `runtime.GOOS`. `internal/plugin`, `internal/skillinstall`, and `tests/e2e` + remain excluded pending Windows symlink/permission equivalents (issue #50). + +### Added + +- `sshx_run` over MCP accepts `shell` for parity with the CLI `--shell`, and + script payloads sent over MCP follow their shebang like CLI payloads do. +- `sshx sql --docker=` now reads that container's environment for the + database role and name, so a TimescaleDB/Postgres image whose `POSTGRES_USER` + is not `postgres` no longer fails with `role "postgres" does not exist`. + `--db` and `--db-user` become optional in this form. Discovery is best-effort: + a container that cannot be inspected or exposes no credentials falls back to + the client defaults, and passing `--db-user` or `--db-password-key` disables + it. `--db-cred-from` keeps its stricter contract and still requires a password. +- `sshx run` script payloads now honor the script's shebang. A + `#!/usr/bin/env bash` payload runs under `bash -s --` instead of being piped + to `sh`, so bash-only constructs (`set -o pipefail`, arrays, `[[ ]]`) work + instead of failing remotely with `Illegal option -o pipefail`. `--shell=NAME` + overrides the shebang; supported interpreters are `sh`, `bash`, `zsh`, + `dash`, `ksh`, and `ash`. A payload declaring any other interpreter (for + example `python3`) is now rejected locally as `error_kind: config` with no + connection, instead of being silently executed by `sh`. The selected + interpreter appears as `action.script_runner` in dry-run plans and results. +- Safety-check recall now covers recursive removal of critical system + directories (`/etc`, `/usr`, `/var`, …), `rm --no-preserve-root`, + `wipefs -a`, `chown -R ... /`, LVM `pvremove`/`vgremove`/`lvremove`, + `zpool|zfs destroy`, `dd of=/dev/`, `systemctl kexec`, and destructive + commands nested inside `docker exec` / `docker compose exec`. + +### Fixed + +- A missing remote database client is now reported as `error_kind: config` + naming the binary, instead of the opaque + `database operation failed during execute with status 127` that required + decoding a shell convention to understand. +- Command safety checks no longer match dangerous keywords anywhere in the raw + command string. The command line is split into shell segments and only the + token in **command position** is judged, following `sudo`/`env`/`timeout` + wrappers, `sh -c` payloads, and `docker exec` into the command that actually + runs. Read-only diagnostics such as `last reboot -F`, + `journalctl | grep -iE 'fail|halt'`, `iptables-save | grep -F ...`, + `curl ... | sha256sum`, `fdisk -l /dev/sda`, `parted /dev/sdb print`, and + bare `wipefs /dev/sdb` are no longer blocked. Replaying 49 commands that a + real workload had blocked shows 48 were false positives; only `rm -rf /` + remains blocked, alongside the unchanged guarded-SQL client redirects. + `iptables` flag matching is now case-sensitive so `-F`/`-X` (flush / delete + chain) are distinguished from `-f`/`-x` (fragment / exact). + ## [0.11.0] - 2026-08-25 ### Added diff --git a/README.md b/README.md index 69a4e46..aa8e3ca 100644 --- a/README.md +++ b/README.md @@ -421,6 +421,10 @@ For PostgreSQL running in a production container, execute the database clients inside the container and resolve credentials from its environment: ```bash +# --docker alone reads the container environment for the role and database, +# so images whose POSTGRES_USER is not "postgres" work without --db-user. +sshx sql -h=prod --docker=pg-prod --json "SELECT count(*) FROM orders" + sshx sql -h=prod --docker=pg-prod \ --db-cred-from=docker:pg-prod --json \ "UPDATE users SET active=false WHERE id=42" diff --git a/SECURITY.md b/SECURITY.md index 941c301..607517c 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -6,9 +6,9 @@ We take security seriously. The following versions of SSHX are currently support | Version | Supported | | -------- | ------------------ | +| 0.12.x | :white_check_mark: | | 0.11.x | :white_check_mark: | -| 0.10.x | :white_check_mark: | -| < 0.10.0 | :x: | +| < 0.11.0 | :x: | Security updates are provided for the latest minor release and the previous minor release (N-1). Older lines do not receive patches; please upgrade. diff --git a/docs/agent-scripting.md b/docs/agent-scripting.md index 670dc09..7121217 100644 --- a/docs/agent-scripting.md +++ b/docs/agent-scripting.md @@ -15,6 +15,7 @@ cat ./check.sh | sshx run --target=prod-web --script-stdin --json - Selectors resolve configured hosts only. Use `--address=` for one literal address. - Script payloads are streamed on SSH stdin and are not reconstructed through shell joining. +- The script's `#!` line selects the interpreter, so a `#!/usr/bin/env bash` payload keeps bash semantics (`set -o pipefail`, arrays, `[[ ]]`). Use `--shell=NAME` to override it. Supported: `sh`, `bash`, `zsh`, `dash`, `ksh`, `ash`; any other interpreter is rejected as `error_kind: config` without connecting. The choice appears as `action.script_runner`. - Dry-run and results expose payload SHA-256 and byte length, not raw script contents. - Multi-target `--jsonl` streams `run_started`, per-target events, and `run_finished`. - Multi-target exit codes: `0` all succeeded, `1` partial/failed/skipped/uncertain, `255` request-level failure. diff --git a/docs/mcp.md b/docs/mcp.md index 942810a..d159856 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -33,7 +33,7 @@ Claude Desktop / generic MCP client entry: | Tool | Maps to | Notes | | --- | --- | --- | -| `sshx_run` | `sshx run --json` | Selectors, command or byte-preserving script, bounded fan-out, dry-run, force + bypass_reason | +| `sshx_run` | `sshx run --json` | Selectors, command or byte-preserving script (shebang or `shell` selects the interpreter), bounded fan-out, dry-run, force + bypass_reason | | `sshx_sql` | `sshx sql --json` | Guarded single-statement SQL via remote psql/sqlite3 | | `sshx_apply` | `sshx apply --json` | Guarded single-file replace; accepts `from_path` or inline `content` | | `sshx_inspect` | `sshx inspect --json` | Built-in capabilities and trusted local plugins | diff --git a/docs/security-guidelines.md b/docs/security-guidelines.md index b431e48..c980cd6 100644 --- a/docs/security-guidelines.md +++ b/docs/security-guidelines.md @@ -96,7 +96,18 @@ This boundary keeps password lookup, stdin injection, and audit metadata aligned ## Safety Checks Are Guardrails -`sshx` blocks common destructive patterns such as root deletion, disk formatting, shutdown or reboot commands, critical system file edits, fork bombs, and `curl | sh` style pipelines. +`sshx` blocks common destructive operations such as root deletion, disk formatting, shutdown or reboot commands, critical system file edits, fork bombs, and `curl | sh` style pipelines. + +Matching applies to the command actually being executed, not to any occurrence of a dangerous word. The command line is split into shell segments and only the token in command position is judged, so read-only diagnostics stay allowed: + +```bash +sshx -h=prod-web "last reboot -F | head -10" # allowed: reboot is an argument +sshx -h=prod-web "journalctl -u app | grep -iE 'fail|halt'" # allowed: halt is a grep pattern +sshx -h=prod-web "sudo iptables-save | grep -F 10.0.0.0/24" # allowed: a different binary +sshx -h=prod-web "sudo iptables -F" # blocked: flushes the ruleset +``` + +Wrappers are followed: `sudo`, `env`, `nohup`, `timeout`, `sh -c '...'`, and `docker exec ...` are all resolved to the command they ultimately run. That does not make untrusted commands safe. A command validator cannot understand every script, shell expansion, application-specific migration, or data-destruction path. diff --git a/docs/zh/agent-scripting.md b/docs/zh/agent-scripting.md index 14f577a..394bbdd 100644 --- a/docs/zh/agent-scripting.md +++ b/docs/zh/agent-scripting.md @@ -14,6 +14,7 @@ sshx run --target=prod-web --script-file=./check.sh --dry-run --json - 选择器只解析已配置主机；字面地址用 `--address=`，不能进入 group/tag 扩散。 - 脚本经 SSH stdin 原样传输，不经本地 `strings.Join` 拼装。 +- 脚本的 `#!` 行决定解释器，`#!/usr/bin/env bash` 会真正用 bash 执行（`set -o pipefail`、数组、`[[ ]]` 都可用）。可用 `--shell=NAME` 覆盖。支持 `sh`、`bash`、`zsh`、`dash`、`ksh`、`ash`；其他解释器在本地就以 `error_kind: config` 拒绝，不建立连接。最终解释器体现在 `action.script_runner`。 - dry-run/结果暴露 payload SHA-256 与字节数，默认不回传脚本全文。 - 多主机 `--jsonl` 输出 `run_started` / `target_*` / `run_finished`。 - 多主机退出码：`0` 全成功，`1` 部分失败/跳过/不确定，`255` 请求级失败。 diff --git a/docs/zh/security-guidelines.md b/docs/zh/security-guidelines.md index 01ac08a..5dfe255 100644 --- a/docs/zh/security-guidelines.md +++ b/docs/zh/security-guidelines.md @@ -96,7 +96,18 @@ sshx -h=prod-web "echo sudo" ## 安全检查只是护栏 -`sshx` 会拦截常见破坏性模式，例如删除根目录、格式化磁盘、关机重启、修改关键系统文件、fork bomb 和 `curl | sh` 这类管道。 +`sshx` 会拦截常见破坏性操作，例如删除根目录、格式化磁盘、关机重启、修改关键系统文件、fork bomb 和 `curl | sh` 这类管道。 + +判定对象是**真正要执行的命令**，而不是命令串里出现的危险词。sshx 先做 shell 分段，只判断处于命令位的 token，因此只读诊断不会被误拦： + +```bash +sshx -h=prod-web "last reboot -F | head -10" # 放行：reboot 是参数 +sshx -h=prod-web "journalctl -u app | grep -iE 'fail|halt'" # 放行：halt 是 grep 模式 +sshx -h=prod-web "sudo iptables-save | grep -F 10.0.0.0/24" # 放行：是另一个程序 +sshx -h=prod-web "sudo iptables -F" # 拦截：清空规则链 +``` + +包装器会被穿透解析：`sudo`、`env`、`nohup`、`timeout`、`sh -c '...'` 以及 `docker exec <容器> ...` 都会一路解析到最终执行的命令。 这并不代表不可信命令就安全了。命令校验器不可能理解所有脚本、shell 展开、应用迁移和业务数据删除路径。 diff --git a/internal/app/agentmode_test.go b/internal/app/agentmode_test.go index 31654f9..7671e22 100644 --- a/internal/app/agentmode_test.go +++ b/internal/app/agentmode_test.go @@ -130,7 +130,7 @@ func TestClassifyError(t *testing.T) { // before any network work, so it reports error_kind "blocked" (not "connect") // even though the host is never reachable. func TestRun_BlockedCommandShortCircuits(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + setTestHome(t, t.TempDir()) // 192.0.2.1 is RFC 5737 TEST-NET-1: if validation did not short-circuit, // the dial would block instead of returning instantly. result := runReportedJSON(t, []string{"sshx", "-h=192.0.2.1", "--json", "rm -rf /"}) @@ -146,7 +146,7 @@ func TestRun_BlockedCommandShortCircuits(t *testing.T) { } func TestRun_BlockedCommandJSONRedactsSecretLikeArguments(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + setTestHome(t, t.TempDir()) secretFragments := []string{"alpha", "bravo", "charlie", "delta"} result := runReportedJSON(t, []string{ "sshx", @@ -202,7 +202,7 @@ func TestRun_JSONConfigFailuresDoNotConnect(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + setTestHome(t, t.TempDir()) result := runReportedJSON(t, tt.args) if result["error_kind"] != "config" { t.Fatalf("expected error_kind=config, got %v", result["error_kind"]) @@ -305,7 +305,7 @@ func TestEmitCommandJSONContracts(t *testing.T) { } func TestRun_DryRunJSONDoesNotConnect(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + setTestHome(t, t.TempDir()) result := runDryRunJSON(t, []string{"sshx", "-h=192.0.2.1", "--dry-run", "--json", "uptime"}) if result["dry_run"] != true { @@ -329,7 +329,7 @@ func TestRun_DryRunJSONDoesNotConnect(t *testing.T) { } func TestRun_DryRunReportsBlockedCommand(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + setTestHome(t, t.TempDir()) result := runDryRunJSON(t, []string{"sshx", "-h=192.0.2.1", "--dry-run", "--json", "sudo rm -rf /"}) if result["valid"] != false { @@ -357,7 +357,7 @@ func TestRun_DryRunReportsBlockedCommand(t *testing.T) { } func TestRun_DryRunMissingHostDoesNotPlanConnection(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + setTestHome(t, t.TempDir()) result := runDryRunJSON(t, []string{"sshx", "--dry-run", "--json", "uptime"}) if result["valid"] != false { @@ -380,7 +380,7 @@ func TestRun_DryRunMissingHostDoesNotPlanConnection(t *testing.T) { func TestRun_DryRunResolvesNamedHostAndSudoKey(t *testing.T) { home := t.TempDir() - t.Setenv("HOME", home) + setTestHome(t, home) passwordKeyName := "prod-web-sudo" //nolint:gosec // G101: keyring key name used in a test, not secret material. err := SaveSettings(&Settings{ Key: "/keys/default.pem", @@ -429,7 +429,7 @@ func TestRun_DryRunResolvesNamedHostAndSudoKey(t *testing.T) { func TestRun_DryRunHostTestUsesConfiguredKeyAndPasswordKey(t *testing.T) { home := t.TempDir() - t.Setenv("HOME", home) + setTestHome(t, home) sudoKeyName := "prod-web-sudo" //nolint:gosec // G101: keyring key name used in a test, not secret material. sshKeyName := "prod-web-login" //nolint:gosec // G101: keyring key name used in a test, not secret material. err := SaveSettings(&Settings{ diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 831d5c8..9e9c0d9 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -115,7 +115,7 @@ func TestRun_ArgumentParsing(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + setTestHome(t, t.TempDir()) // Suppress output oldStdout := os.Stdout oldStderr := os.Stderr diff --git a/internal/app/audit_test.go b/internal/app/audit_test.go index b037a0a..b054d94 100644 --- a/internal/app/audit_test.go +++ b/internal/app/audit_test.go @@ -7,6 +7,7 @@ import ( "io" "os" "path/filepath" + "runtime" "strings" "testing" "time" @@ -15,7 +16,7 @@ import ( ) func TestRun_BlockedCommandWritesRedactedAuditEvent(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + setTestHome(t, t.TempDir()) auditDir := t.TempDir() command := "sudo rm -rf / password=orange --token purple" //nolint:gosec // test verifies redaction of credential-like arguments. @@ -193,7 +194,7 @@ func TestSQLAuditUsesRedactedStatementAndDigest(t *testing.T) { } func TestRun_DryRunDoesNotWriteAuditEvent(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + setTestHome(t, t.TempDir()) auditDir := filepath.Join(t.TempDir(), "audit") result := runDryRunJSON(t, []string{"sshx", "-h=192.0.2.1", "--audit-output=" + auditDir, "--dry-run", "--json", "uptime"}) @@ -206,7 +207,7 @@ func TestRun_DryRunDoesNotWriteAuditEvent(t *testing.T) { } func TestAuditRecorderRefreshRecordsExecutionContract(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + setTestHome(t, t.TempDir()) config := &sshclient.Config{ AuditEnabled: true, Host: "prod-web", @@ -336,7 +337,7 @@ func TestAuditEffectFlagsByModeAndAction(t *testing.T) { func TestWriteAuditEventUsesJSONLWithPrivatePermissions(t *testing.T) { home := t.TempDir() - t.Setenv("HOME", home) + setTestHome(t, home) config := &sshclient.Config{AuditEnabled: true} event := auditEvent{ SchemaVersion: auditSchemaVersion, @@ -357,7 +358,8 @@ func TestWriteAuditEventUsesJSONLWithPrivatePermissions(t *testing.T) { if err != nil { t.Fatalf("expected audit file at %s: %v", auditPath, err) } - if info.Mode().Perm() != 0o600 { + // Windows has no POSIX permission bits; Go reports 0666/0777 there. + if runtime.GOOS != "windows" && info.Mode().Perm() != 0o600 { t.Fatalf("expected audit file mode 0600, got %v", info.Mode().Perm()) } diff --git a/internal/app/config.go b/internal/app/config.go index bb49ef1..d72e511 100644 --- a/internal/app/config.go +++ b/internal/app/config.go @@ -514,6 +514,8 @@ func parseRunArgs(config *sshclient.Config, args []string) { case arg == "--script-stdin": config.ScriptStdin = true config.RunActionKind = "script" + case strings.HasPrefix(arg, "--shell="): + config.ScriptShell = strings.SplitN(arg, "=", 2)[1] case arg == "--sudo": config.RunUseSudo = true case strings.HasPrefix(arg, "--max-output-bytes="): diff --git a/internal/app/mcp.go b/internal/app/mcp.go index 9199c98..cfb51f3 100644 --- a/internal/app/mcp.go +++ b/internal/app/mcp.go @@ -145,6 +145,7 @@ type mcpRunInput struct { Address string `json:"address,omitempty" jsonschema:"Explicit single literal address (not for fan-out)."` Command string `json:"command,omitempty" jsonschema:"Remote command line. Exactly one of command or script is required."` Script string `json:"script,omitempty" jsonschema:"Byte-preserving script payload delivered over stdin. Exactly one of command or script is required."` + Shell string `json:"shell,omitempty" jsonschema:"Script interpreter override: sh, bash, zsh, dash, ksh, or ash. Defaults to the script's shebang, then sh."` TimeoutSecs int `json:"timeout_seconds,omitempty" jsonschema:"Remote execution timeout in seconds (default 60)."` Concurrency int `json:"concurrency,omitempty" jsonschema:"Bounded fan-out (default 4, hard max 32)."` FailureMode string `json:"failure_mode,omitempty" jsonschema:"continue or fail_fast (default continue)."` @@ -284,8 +285,14 @@ func buildRunArgs(in mcpRunInput) ([]string, string, error) { stdin := "" if hasScript { args = append(args, "--script-stdin") + if in.Shell != "" { + args = append(args, "--shell="+in.Shell) + } stdin = in.Script } else { + if in.Shell != "" { + return nil, "", fmt.Errorf("shell only applies to a script payload") + } args = append(args, "--", in.Command) } return args, stdin, nil diff --git a/internal/app/mcp_test.go b/internal/app/mcp_test.go index 8118d6e..2e5c59f 100644 --- a/internal/app/mcp_test.go +++ b/internal/app/mcp_test.go @@ -54,6 +54,29 @@ func TestBuildRunArgsScriptStdin(t *testing.T) { assertArgs(t, args, []string{"run", "--json", "--target=web-1", "--script-stdin"}) } +func TestBuildRunArgsScriptShell(t *testing.T) { + script := "#!/usr/bin/env bash\nset -o pipefail\n" + args, stdin, err := buildRunArgs(mcpRunInput{ + Targets: []string{"web-1"}, + Script: script, + Shell: "bash", + }) + if err != nil { + t.Fatalf("buildRunArgs: %v", err) + } + if stdin != script { + t.Fatalf("stdin = %q, want the byte-preserved script", stdin) + } + assertArgs(t, args, []string{"run", "--json", "--target=web-1", "--script-stdin", "--shell=bash"}) +} + +func TestBuildRunArgsRejectsShellWithoutScript(t *testing.T) { + _, _, err := buildRunArgs(mcpRunInput{Targets: []string{"web-1"}, Command: "uptime", Shell: "bash"}) + if err == nil { + t.Fatal("expected an error when shell is combined with a command payload") + } +} + func TestBuildRunArgsRequiresExactlyOnePayload(t *testing.T) { if _, _, err := buildRunArgs(mcpRunInput{Targets: []string{"a"}}); err == nil { t.Fatal("expected error when neither command nor script is set") diff --git a/internal/app/run.go b/internal/app/run.go index 7d458ed..68e9ba6 100644 --- a/internal/app/run.go +++ b/internal/app/run.go @@ -355,7 +355,6 @@ func buildRunRequest(config *sshclient.Config) (*execution.Request, *execution.P var payload *execution.Payload if req.Action.Kind == execution.ActionScript { - var err error if req.Action.ScriptFromStdin { p, loadErr := execution.LoadScriptStdin(os.Stdin, req.Limits.MaxPayloadBytes) if loadErr != nil { @@ -371,12 +370,30 @@ func buildRunRequest(config *sshclient.Config) (*execution.Request, *execution.P } req.Action.PayloadSHA256 = payload.SHA256 req.Action.PayloadBytes = payload.Size - _ = err + // An explicit --shell wins; otherwise a shebang selects the interpreter + // so a bash script keeps bash semantics (set -o pipefail, arrays, …) + // instead of being silently run by sh. + if runner := runnerFromConfig(config, payload); runner != "" { + req.Action.ScriptRunner = runner + } } return req, payload, nil } +// runnerFromConfig resolves the script interpreter: an explicit --shell first, +// then the payload's shebang when it names a supported shell. An unsupported +// shebang is reported rather than silently downgraded to sh. +func runnerFromConfig(config *sshclient.Config, payload *execution.Payload) string { + if s := strings.TrimSpace(config.ScriptShell); s != "" { + return s + } + if payload != nil && payload.Shebang != "" { + return payload.Shebang + } + return "" +} + func recordRunAudit(audit *auditRecorder, config *sshclient.Config, req *execution.Request, snap execution.TargetSnapshot, outcome execution.RunOutcome) { if audit == nil { return diff --git a/internal/app/settings_test.go b/internal/app/settings_test.go index c149e1d..a6b8700 100644 --- a/internal/app/settings_test.go +++ b/internal/app/settings_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "os" "path/filepath" + "runtime" "strings" "testing" ) @@ -11,15 +12,7 @@ import ( func TestLoadSettings_NotExist(t *testing.T) { // Create a temporary directory tmpDir := t.TempDir() - oldHome := os.Getenv("HOME") - t.Cleanup(func() { - if err := os.Setenv("HOME", oldHome); err != nil { - t.Logf("Warning: failed to restore HOME: %v", err) - } - }) - if err := os.Setenv("HOME", tmpDir); err != nil { - t.Fatalf("Failed to set HOME: %v", err) - } + setTestHome(t, tmpDir) settings, err := LoadSettings() if err != nil { @@ -42,15 +35,7 @@ func TestLoadSettings_NotExist(t *testing.T) { func TestSaveAndLoadSettings(t *testing.T) { // Create a temporary directory tmpDir := t.TempDir() - oldHome := os.Getenv("HOME") - t.Cleanup(func() { - if err := os.Setenv("HOME", oldHome); err != nil { - t.Logf("Warning: failed to restore HOME: %v", err) - } - }) - if err := os.Setenv("HOME", tmpDir); err != nil { - t.Fatalf("Failed to set HOME: %v", err) - } + setTestHome(t, tmpDir) // Create settings settings := &Settings{ @@ -84,7 +69,7 @@ func TestSaveAndLoadSettings(t *testing.T) { } if info, statErr := os.Stat(settingsDir); statErr != nil { t.Fatalf("Stat settings dir error = %v", statErr) - } else if perm := info.Mode().Perm(); perm != 0700 { + } else if perm := info.Mode().Perm(); runtime.GOOS != "windows" && perm != 0700 { t.Errorf("settings dir perm = %o, want 700", perm) } @@ -376,15 +361,7 @@ func TestJSONMarshaling(t *testing.T) { func TestSettingsPath(t *testing.T) { tmpDir := t.TempDir() - oldHome := os.Getenv("HOME") - t.Cleanup(func() { - if err := os.Setenv("HOME", oldHome); err != nil { - t.Logf("Warning: failed to restore HOME: %v", err) - } - }) - if err := os.Setenv("HOME", tmpDir); err != nil { - t.Fatalf("Failed to set HOME: %v", err) - } + setTestHome(t, tmpDir) settingsPath, err := GetSettingsPath() if err != nil { @@ -409,15 +386,7 @@ func TestSettingsPath(t *testing.T) { func TestSaveSettings_AtomicOverwrite(t *testing.T) { tmpDir := t.TempDir() - oldHome := os.Getenv("HOME") - t.Cleanup(func() { - if err := os.Setenv("HOME", oldHome); err != nil { - t.Logf("Warning: failed to restore HOME: %v", err) - } - }) - if err := os.Setenv("HOME", tmpDir); err != nil { - t.Fatalf("Failed to set HOME: %v", err) - } + setTestHome(t, tmpDir) first := &Settings{Hosts: []HostConfig{{Name: "a", Host: "10.0.0.1"}}} if err := SaveSettings(first); err != nil { @@ -456,22 +425,14 @@ func TestSaveSettings_AtomicOverwrite(t *testing.T) { if err != nil { t.Fatalf("Stat settings file error = %v", err) } - if perm := info.Mode().Perm(); perm != 0600 { + if perm := info.Mode().Perm(); runtime.GOOS != "windows" && perm != 0600 { t.Errorf("settings file perm = %o, want 600", perm) } } func TestSaveSettings_RenameFailureCleansTempFile(t *testing.T) { tmpDir := t.TempDir() - oldHome := os.Getenv("HOME") - t.Cleanup(func() { - if err := os.Setenv("HOME", oldHome); err != nil { - t.Logf("Warning: failed to restore HOME: %v", err) - } - }) - if err := os.Setenv("HOME", tmpDir); err != nil { - t.Fatalf("Failed to set HOME: %v", err) - } + setTestHome(t, tmpDir) settingsDir, err := GetSettingsDir() if err != nil { diff --git a/internal/app/sql.go b/internal/app/sql.go index 2c880e9..027fd5e 100644 --- a/internal/app/sql.go +++ b/internal/app/sql.go @@ -135,7 +135,9 @@ func HandleSQL(config *sshclient.Config, audit *auditRecorder) (err error) { // connecting; a hit avoids re-reading the production environment. var credSource sqlsafe.CredSource needExtract := false - if config.SQLCredFrom != "" { + credBestEffort := false + switch { + case config.SQLCredFrom != "": parsed, srcErr := sqlsafe.ParseCredSource(config.SQLCredFrom) if srcErr != nil { return run.fail("config", srcErr) @@ -151,6 +153,21 @@ func HandleSQL(config *sshclient.Config, audit *auditRecorder) (err error) { } else { needExtract = true } + case config.SQLDockerContainer != "" && config.SQLUser == "" && config.SQLPasswordKey == "": + // `--docker` already names the database container, so read its + // environment for the role and database instead of assuming a + // "postgres" superuser that many images never create. This is + // best-effort: if the container cannot be inspected or exposes no + // credentials, the client defaults still apply. + credSource = sqlsafe.CredSource{Kind: "docker", Container: config.SQLDockerContainer} + credBestEffort = true + run.credSource = credSource.String() + if creds, ok := lookupCredCache(config.Host, run.credSource); config.SQLCredCacheTTL > 0 && ok && !config.SQLCredRefresh { + run.applyCredentials(*creds) + run.credCache = "hit" + } else { + needExtract = true + } } client, cliErr := sshclient.NewSSHClient(config) @@ -164,7 +181,7 @@ func HandleSQL(config *sshclient.Config, audit *auditRecorder) (err error) { run.client = client if needExtract { - if credErr := run.resolveCredentials(credSource); credErr != nil { + if credErr := run.resolveCredentials(credSource, credBestEffort); credErr != nil { return credErr } } @@ -226,7 +243,9 @@ func validateSQLConfig(config *sshclient.Config) error { if config.SQLFile != "" { return fmt.Errorf("--db-file is only valid with --engine=sqlite") } - if config.SQLDatabase == "" && config.SQLCredFrom == "" { + // --docker names the database container, whose environment can supply the + // database name, so --db becomes optional in that form too. + if config.SQLDatabase == "" && config.SQLCredFrom == "" && config.SQLDockerContainer == "" { return fmt.Errorf("--db= is required") } if config.SQLDatabase != "" { @@ -330,21 +349,39 @@ func (r *sqlRun) applyCredentials(creds sqlsafe.Credentials) { // resolveCredentials reads the credential source on the remote host. The // extraction command carries no secret; its output does and is therefore // never logged, audited, or embedded in error messages. -func (r *sqlRun) resolveCredentials(source sqlsafe.CredSource) error { +// resolveCredentials reads the credential source on the remote host. When +// bestEffort is set the source was inferred from --docker rather than +// requested explicitly, so a container that cannot be inspected or exposes no +// password must not fail the run: the client defaults still apply. +func (r *sqlRun) resolveCredentials(source sqlsafe.CredSource, bestEffort bool) error { r.phase = "credentials" cmd, cmdErr := source.ExtractionCommand() if cmdErr != nil { + if bestEffort { + return nil + } return r.fail("config", cmdErr) } res, execErr := r.runRemote(sqlsafe.RemoteCommand{Command: cmd}) if execErr != nil || res.ExitCode != 0 { + if bestEffort { + logger.GetLogger().Info( + "Note: could not read %s for the database role; using client defaults", r.credSource) + return nil + } return r.fail("cred_source_failed", fmt.Errorf( "failed to read credential source %s (remote status %d)", r.credSource, res.ExitCode)) } - creds, parseErr := sqlsafe.ParseCredOutput(res.Stdout) - if parseErr != nil { - return r.fail("cred_source_failed", fmt.Errorf("credential source %s: %w", r.credSource, parseErr)) + var creds sqlsafe.Credentials + if bestEffort { + creds = sqlsafe.ParseCredIdentity(res.Stdout) + } else { + parsed, parseErr := sqlsafe.ParseCredOutput(res.Stdout) + if parseErr != nil { + return r.fail("cred_source_failed", fmt.Errorf("credential source %s: %w", r.credSource, parseErr)) + } + creds = parsed } r.applyCredentials(creds) if r.config.SQLDatabase != "" { @@ -353,7 +390,9 @@ func (r *sqlRun) resolveCredentials(source sqlsafe.CredSource) error { } } r.credCache = "resolved" - if r.config.SQLCredCacheTTL > 0 { + // Only an explicit source writes the cache: a best-effort result may lack + // a password and must not shadow a later --db-cred-from resolution. + if !bestEffort && r.config.SQLCredCacheTTL > 0 { if storeErr := storeCredCache(r.config.Host, r.credSource, creds, r.config.SQLCredCacheTTL); storeErr != nil { logger.GetLogger().Info("Note: credential cache not updated: %v", storeErr) } else { @@ -568,6 +607,15 @@ func (r *sqlRun) failWithExit(kind string, res sshclient.ExecResult, failErr err safeErr = fmt.Errorf("%s: %s", safeErr.Error(), redactSensitiveText(detail)) } } + // A missing client is a host configuration problem, not a statement + // failure. Reporting it as remote_exit 127 forces the caller to decode a + // shell convention to learn that psql/sqlite3 simply is not installed. + if missing, ok := missingDatabaseClient(res); ok { + kind = "config" + safeErr = fmt.Errorf( + "%s is not available on the remote host%s: install the client, or use --docker= to run it inside the database container", + missing, r.clientLocationHint()) + } r.recordAudit(res.ExitCode, kind, safeErr) if r.config.JSONOutput { result := r.baseResult() @@ -599,6 +647,36 @@ func firstNonEmptyLine(chunks ...string) string { return "" } +// databaseClientNames are the remote binaries sshx drives for each engine. +var databaseClientNames = []string{"psql", "sqlite3"} + +// missingDatabaseClient reports the client binary the remote shell could not +// find. Shells exit 127 for "command not found", so the exit code alone is +// ambiguous; the message is required to confirm it. +func missingDatabaseClient(res sshclient.ExecResult) (string, bool) { + if res.ExitCode != 127 { + return "", false + } + text := strings.ToLower(res.Stderr + "\n" + res.Stdout) + if !strings.Contains(text, "not found") && !strings.Contains(text, "no such file") { + return "", false + } + for _, name := range databaseClientNames { + if strings.Contains(text, name) { + return name, true + } + } + return "", false +} + +// clientLocationHint names where sshx looked for the client. +func (r *sqlRun) clientLocationHint() string { + if r.config != nil && r.config.SQLDockerContainer != "" { + return " or in container " + r.config.SQLDockerContainer + } + return "" +} + func (r *sqlRun) baseResult() sqlJSONResult { result := sqlJSONResult{ Host: r.config.Host, diff --git a/internal/app/sql_docker_test.go b/internal/app/sql_docker_test.go new file mode 100644 index 0000000..6c5c993 --- /dev/null +++ b/internal/app/sql_docker_test.go @@ -0,0 +1,95 @@ +package app + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/talkincode/sshx/internal/sshclient" +) + +// A remote shell exits 127 for "command not found". Reporting that verbatim as +// remote_exit forces the caller to decode a shell convention just to learn the +// client is not installed, so sshx classifies it as a config problem. +func TestMissingDatabaseClientClassification(t *testing.T) { + tests := []struct { + name string + res sshclient.ExecResult + wantName string + wantOK bool + }{ + { + name: "bash reports sqlite3 missing", + res: sshclient.ExecResult{ExitCode: 127, Stderr: "bash: line 1: sqlite3: command not found\n"}, + wantName: "sqlite3", + wantOK: true, + }, + { + name: "dash reports sqlite3 missing", + res: sshclient.ExecResult{ExitCode: 127, Stderr: "sh: 1: sqlite3: not found\n"}, + wantName: "sqlite3", + wantOK: true, + }, + { + name: "psql missing", + res: sshclient.ExecResult{ExitCode: 127, Stderr: "bash: psql: command not found\n"}, + wantName: "psql", + wantOK: true, + }, + { + name: "127 from the statement itself is not a missing client", + res: sshclient.ExecResult{ExitCode: 127, Stderr: "ERROR: relation \"users\" does not exist\n"}, + wantOK: false, + }, + { + name: "unrelated missing command is not attributed to a client", + res: sshclient.ExecResult{ExitCode: 127, Stderr: "bash: docker: command not found\n"}, + wantOK: false, + }, + { + name: "non-127 exit is a real statement failure", + res: sshclient.ExecResult{ExitCode: 3, Stderr: "psql: FATAL: role does not exist\n"}, + wantOK: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, ok := missingDatabaseClient(tt.res) + assert.Equal(t, tt.wantOK, ok) + if tt.wantOK { + assert.Equal(t, tt.wantName, got) + } + }) + } +} + +func TestClientLocationHint(t *testing.T) { + plain := &sqlRun{config: &sshclient.Config{}} + assert.Equal(t, "", plain.clientLocationHint()) + + inContainer := &sqlRun{config: &sshclient.Config{SQLDockerContainer: "tsdb"}} + assert.Equal(t, " or in container tsdb", inContainer.clientLocationHint()) +} + +// --docker names the database container, so --db may come from its +// environment; without --docker a database is still required. +func TestValidateSQLConfigDatabaseRequirement(t *testing.T) { + withDocker := &sshclient.Config{ + Mode: "sql", + Host: "db1", + SQLStatement: "SELECT 1", + SQLDockerContainer: "tsdb", + } + require.NoError(t, validateSQLConfig(withDocker)) + + withoutDocker := &sshclient.Config{ + Mode: "sql", + Host: "db1", + SQLStatement: "SELECT 1", + } + err := validateSQLConfig(withoutDocker) + require.Error(t, err) + assert.Contains(t, err.Error(), "--db= is required") +} diff --git a/internal/app/testhome_test.go b/internal/app/testhome_test.go new file mode 100644 index 0000000..1e8e022 --- /dev/null +++ b/internal/app/testhome_test.go @@ -0,0 +1,17 @@ +package app + +import ( + "runtime" + "testing" +) + +// setTestHome points the user home directory at dir for one test. Go resolves +// os.UserHomeDir() from HOME on Unix and USERPROFILE on Windows, so setting +// only HOME would leave Windows tests writing into the real user profile. +func setTestHome(t *testing.T, dir string) { + t.Helper() + t.Setenv("HOME", dir) + if runtime.GOOS == "windows" { + t.Setenv("USERPROFILE", dir) + } +} diff --git a/internal/app/usage.go b/internal/app/usage.go index b0807ad..ee21538 100644 --- a/internal/app/usage.go +++ b/internal/app/usage.go @@ -67,6 +67,12 @@ Run Contract (preferred for Agents): --all-hosts all configured hosts before tag filters --address=HOST explicit single literal address (not for fan-out) + Script payloads: + --script-file=PATH byte-preserving script from a local file + --script-stdin byte-preserving script from stdin + --shell=NAME interpreter override: sh, bash, zsh, dash, ksh, ash + (default: the script's #! line, else sh) + Limits / policy: --concurrency=N default 4, hard max 32 --failure-mode=continue|fail_fast default continue diff --git a/internal/execution/executor.go b/internal/execution/executor.go index 5381644..7a0db7b 100644 --- a/internal/execution/executor.go +++ b/internal/execution/executor.go @@ -458,7 +458,7 @@ func executeOne(ctx context.Context, opts RunOptions, target ResolvedTarget) Tar execErr = fmt.Errorf("%w: missing script payload", ErrConfig) } else { useSudo := req.Action.UseSudo - execRes, execErr = client.RunScript(opts.Payload.Bytes, useSudo) + execRes, execErr = client.RunScriptWithShell(opts.Payload.Bytes, req.Action.ScriptRunner, useSudo) } default: execErr = fmt.Errorf("%w: action kind %q not executable by run executor", ErrConfig, req.Action.Kind) diff --git a/internal/execution/payload.go b/internal/execution/payload.go index a6d27de..cf6ce92 100644 --- a/internal/execution/payload.go +++ b/internal/execution/payload.go @@ -1,11 +1,14 @@ package execution import ( + "bytes" "crypto/sha256" "encoding/hex" "fmt" "io" "os" + "path" + "strings" ) // Payload holds a byte-preserving script body and its digest metadata. @@ -13,6 +16,50 @@ type Payload struct { Bytes []byte SHA256 string Size int + // Shebang is the interpreter basename declared by a leading `#!` line, + // empty when the payload declares none. + Shebang string +} + +// supportedScriptRunners are POSIX-shell-family interpreters sshx can drive +// over stdin with ` -s --`. Other interpreters use different stdin +// conventions and are rejected rather than silently executed by sh. +var supportedScriptRunners = map[string]bool{ + "sh": true, "bash": true, "zsh": true, "dash": true, "ksh": true, "ash": true, +} + +// SupportedScriptRunner reports whether name can be used as a script runner. +func SupportedScriptRunner(name string) bool { + return supportedScriptRunners[name] +} + +// parseShebang returns the interpreter basename declared by a leading `#!` +// line. `#!/usr/bin/env bash` resolves to bash, `#!/bin/sh` to sh. +func parseShebang(data []byte) string { + if len(data) < 3 || data[0] != '#' || data[1] != '!' { + return "" + } + line := data[2:] + if idx := bytes.IndexAny(line, "\n\r"); idx >= 0 { + line = line[:idx] + } + fields := strings.Fields(string(line)) + if len(fields) == 0 { + return "" + } + interp := path.Base(fields[0]) + // `#!/usr/bin/env bash` — the real interpreter is the next word. Skip + // env's own NAME=value assignments and options. + if interp == "env" { + for _, f := range fields[1:] { + if strings.HasPrefix(f, "-") || strings.Contains(f, "=") { + continue + } + return path.Base(f) + } + return "" + } + return interp } // LoadScriptFile reads one local regular file as a script payload. @@ -66,8 +113,9 @@ func digestPayload(data []byte, maxBytes int) (Payload, error) { } sum := sha256.Sum256(data) return Payload{ - Bytes: data, - SHA256: hex.EncodeToString(sum[:]), - Size: len(data), + Bytes: data, + SHA256: hex.EncodeToString(sum[:]), + Size: len(data), + Shebang: parseShebang(data), }, nil } diff --git a/internal/execution/payload_shebang_test.go b/internal/execution/payload_shebang_test.go new file mode 100644 index 0000000..4b2518e --- /dev/null +++ b/internal/execution/payload_shebang_test.go @@ -0,0 +1,103 @@ +package execution + +import ( + "strings" + "testing" +) + +func TestParseShebang(t *testing.T) { + tests := []struct { + name string + body string + want string + }{ + {"no shebang", "echo hi\n", ""}, + {"absolute bash", "#!/bin/bash\nset -o pipefail\n", "bash"}, + {"absolute sh", "#!/bin/sh\nset -eu\n", "sh"}, + {"usr bin bash", "#!/usr/bin/bash\n", "bash"}, + {"env bash", "#!/usr/bin/env bash\nset -o pipefail\n", "bash"}, + {"env with option", "#!/usr/bin/env -S bash -e\n", "bash"}, + {"env with assignment", "#!/usr/bin/env LC_ALL=C bash\n", "bash"}, + {"env alone", "#!/usr/bin/env\n", ""}, + {"zsh", "#!/bin/zsh\n", "zsh"}, + {"python", "#!/usr/bin/env python3\nprint(1)\n", "python3"}, + {"shebang with args", "#!/bin/bash -e\n", "bash"}, + {"crlf line ending", "#!/bin/bash\r\necho hi\r\n", "bash"}, + {"only shebang line", "#!/bin/bash", "bash"}, + {"hash but not shebang", "# not a shebang\n", ""}, + {"too short", "#!", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := parseShebang([]byte(tt.body)); got != tt.want { + t.Errorf("parseShebang(%q) = %q, want %q", tt.body, got, tt.want) + } + }) + } +} + +func TestDigestPayloadCapturesShebang(t *testing.T) { + p, err := digestPayload([]byte("#!/usr/bin/env bash\nset -o pipefail\n"), DefaultMaxPayload) + if err != nil { + t.Fatal(err) + } + if p.Shebang != "bash" { + t.Errorf("Shebang = %q, want bash", p.Shebang) + } + if p.Size == 0 || p.SHA256 == "" { + t.Error("payload digest metadata missing") + } +} + +// A bash script must not be silently executed by sh: `set -o pipefail` fails +// with "Illegal option -o pipefail" under dash/ash, which is a confusing +// remote-exit failure rather than a clear local one. +func TestNormalizeRequestAcceptsShellFamilyRunners(t *testing.T) { + for _, runner := range []string{"sh", "bash", "zsh", "dash", "ksh", "ash"} { + t.Run(runner, func(t *testing.T) { + req := scriptRequest(runner) + if err := NormalizeRequest(req); err != nil { + t.Fatalf("runner %q should be accepted: %v", runner, err) + } + if req.Action.ScriptRunner != runner { + t.Errorf("ScriptRunner = %q, want %q", req.Action.ScriptRunner, runner) + } + }) + } +} + +func TestNormalizeRequestRejectsUnsupportedRunner(t *testing.T) { + req := scriptRequest("python3") + err := NormalizeRequest(req) + if err == nil { + t.Fatal("expected an error for an unsupported script runner") + } + if !strings.Contains(err.Error(), "unsupported script runner") { + t.Errorf("unexpected error: %v", err) + } +} + +func TestNormalizeRequestDefaultsRunnerToSh(t *testing.T) { + req := scriptRequest("") + if err := NormalizeRequest(req); err != nil { + t.Fatal(err) + } + if req.Action.ScriptRunner != ScriptRunnerSH { + t.Errorf("ScriptRunner = %q, want sh", req.Action.ScriptRunner) + } +} + +func scriptRequest(runner string) *Request { + return &Request{ + SchemaVersion: RequestSchemaVersion, + Targets: TargetSelector{Names: []string{"host-a"}}, + Action: ActionSpec{ + Kind: ActionScript, + Intent: IntentRead, + ScriptPath: "/tmp/script.sh", + ScriptRunner: runner, + }, + Policy: Policy{SafetyCheckEnabled: true}, + } +} diff --git a/internal/execution/validate.go b/internal/execution/validate.go index ab7700b..8c36ec0 100644 --- a/internal/execution/validate.go +++ b/internal/execution/validate.go @@ -97,8 +97,9 @@ func NormalizeRequest(req *Request) error { if req.Action.ScriptRunner == "" { req.Action.ScriptRunner = ScriptRunnerSH } - if req.Action.ScriptRunner != ScriptRunnerSH { - return fmt.Errorf("%w: unsupported script runner %q (required: sh)", ErrConfig, req.Action.ScriptRunner) + if !SupportedScriptRunner(req.Action.ScriptRunner) { + return fmt.Errorf("%w: unsupported script runner %q (supported: sh, bash, zsh, dash, ksh, ash)", + ErrConfig, req.Action.ScriptRunner) } } diff --git a/internal/runtimepath/path_test.go b/internal/runtimepath/path_test.go index 09c85f4..3970950 100644 --- a/internal/runtimepath/path_test.go +++ b/internal/runtimepath/path_test.go @@ -2,6 +2,7 @@ package runtimepath import ( "path/filepath" + "runtime" "testing" ) @@ -29,6 +30,9 @@ func TestRootDefaultsBelowUserHome(t *testing.T) { home := t.TempDir() t.Setenv(EnvHome, "") t.Setenv("HOME", home) + if runtime.GOOS == "windows" { + t.Setenv("USERPROFILE", home) + } root, err := Root() if err != nil { diff --git a/internal/skillinstall/install_test.go b/internal/skillinstall/install_test.go index bde3559..21c8e1c 100644 --- a/internal/skillinstall/install_test.go +++ b/internal/skillinstall/install_test.go @@ -4,6 +4,7 @@ import ( "errors" "os" "path/filepath" + "runtime" "testing" ) @@ -204,6 +205,9 @@ func TestInstallRejectsSymlinkedTargetDirectoryAndFile(t *testing.T) { func TestResolveDirDefaultAndHomeExpansion(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) + if runtime.GOOS == "windows" { + t.Setenv("USERPROFILE", home) + } defaultDir, err := ResolveDir("") if err != nil { diff --git a/internal/sqlsafe/credsource.go b/internal/sqlsafe/credsource.go index 490881e..58e8b04 100644 --- a/internal/sqlsafe/credsource.go +++ b/internal/sqlsafe/credsource.go @@ -102,7 +102,22 @@ var ( // ParseCredOutput extracts credentials from KEY=VALUE lines (docker inspect // env output or an env file). Discrete keys win over a connection URL. The // error never embeds raw output, which may contain unrelated secrets. +// ParseCredOutput parses KEY=VALUE lines into credentials and requires a +// password: an explicit --db-cred-from asked sshx to obtain one. func ParseCredOutput(output string) (Credentials, error) { + creds := ParseCredIdentity(output) + if creds.Password == "" { + return Credentials{}, fmt.Errorf( + "no database password found in credential source (looked for %s and a connection URL)", + strings.Join(credPasswordKeys, ", ")) + } + return creds, nil +} + +// ParseCredIdentity parses the same KEY=VALUE lines but tolerates a missing +// password. It backs best-effort discovery, where sshx only needs the role and +// database name and the server may well use trust or peer authentication. +func ParseCredIdentity(output string) Credentials { env := map[string]string{} for _, line := range strings.Split(output, "\n") { line = strings.TrimSpace(line) @@ -128,19 +143,12 @@ func ParseCredOutput(output string) (Credentials, error) { Host: firstEnv(env, credHostKeys), Port: firstEnv(env, credPortKeys), } - if creds.Password == "" { - if u := firstEnv(env, credURLKeys); u != "" { - if fromURL, err := parseDatabaseURL(u); err == nil { - merge(&creds, fromURL) - } + if u := firstEnv(env, credURLKeys); u != "" { + if fromURL, err := parseDatabaseURL(u); err == nil { + merge(&creds, fromURL) } } - if creds.Password == "" { - return Credentials{}, fmt.Errorf( - "no database password found in credential source (looked for %s and a connection URL)", - strings.Join(credPasswordKeys, ", ")) - } - return creds, nil + return creds } func firstEnv(env map[string]string, keys []string) string { diff --git a/internal/sshclient/client.go b/internal/sshclient/client.go index a067e4f..4701268 100644 --- a/internal/sshclient/client.go +++ b/internal/sshclient/client.go @@ -136,20 +136,24 @@ type Config struct { ReportedError string // Run-mode execution contract fields (Mode == "run"). - RequestID string - RunTargets []string - RunGroups []string - RunTags map[string]string - RunAllHosts bool - RunAddress string - RunActionKind string - RunIntent string - RunUseSudo bool - RunConcurrency int - FailureMode string - BypassReason string - ScriptFile string - ScriptStdin bool + RequestID string + RunTargets []string + RunGroups []string + RunTags map[string]string + RunAllHosts bool + RunAddress string + RunActionKind string + RunIntent string + RunUseSudo bool + RunConcurrency int + FailureMode string + BypassReason string + ScriptFile string + ScriptStdin bool + // ScriptShell overrides the interpreter used for --script-file / + // --script-stdin payloads. Empty means: follow the payload's shebang, or + // fall back to sh. + ScriptShell string JSONLOutput bool MaxOutputBytes int MaxPayloadBytes int @@ -679,12 +683,26 @@ func (c *SSHClient) RunCommand(capture bool) (ExecResult, error) { return result, fmt.Errorf("command failed: %w", runErr) } -// RunScript streams a trusted local collector to a fresh SSH session. The -// payload is never installed on the target. When useSudo is true, the password -// and script share stdin in that order: sudo consumes one line and sh consumes -// the remaining bytes. +// RunScript streams a trusted local collector to a fresh SSH session using the +// POSIX shell. The payload is never installed on the target. func (c *SSHClient) RunScript(payload []byte, useSudo bool) (ExecResult, error) { + return c.RunScriptWithShell(payload, "sh", useSudo) +} + +// RunScriptWithShell streams a trusted local script to a fresh SSH session and +// executes it with the named POSIX-shell-family interpreter. The payload is +// never installed on the target. When useSudo is true, the password and script +// share stdin in that order: sudo consumes one line and the shell consumes the +// remaining bytes. +func (c *SSHClient) RunScriptWithShell(payload []byte, shell string, useSudo bool) (ExecResult, error) { var result ExecResult + if shell == "" { + shell = "sh" + } + if !shellNames[shell] { + result.ExitCode = -1 + return result, fmt.Errorf("unsupported script shell %q", shell) + } if len(payload) == 0 { result.ExitCode = -1 return result, fmt.Errorf("collector payload is empty") @@ -705,10 +723,10 @@ func (c *SSHClient) RunScript(payload []byte, useSudo bool) (ExecResult, error) } defer func() { _ = session.Close() }() //nolint:errcheck // best-effort session teardown - command := "sh -s --" + command := shell + " -s --" stdin := bytes.NewReader(payload) if useSudo { - command = "sudo -S -p '' sh -s --" + command = "sudo -S -p '' " + shell + " -s --" stdin = bytes.NewReader(append(append([]byte(c.config.SudoPassword+"\n"), payload...), '\n')) } session.Stdin = stdin diff --git a/internal/sshclient/client_test.go b/internal/sshclient/client_test.go index ada35c7..feed0c7 100644 --- a/internal/sshclient/client_test.go +++ b/internal/sshclient/client_test.go @@ -304,7 +304,7 @@ func TestRemotePathJoinUsesSFTPSlashSeparator(t *testing.T) { func TestGetHostKeyCallbackAcceptsUnknownHost(t *testing.T) { home := t.TempDir() - t.Setenv("HOME", home) + setTestHome(t, home) cfg := &Config{AcceptUnknownHost: true} callback, err := getHostKeyCallback(cfg) require.NoError(t, err) @@ -322,7 +322,7 @@ func TestGetHostKeyCallbackAcceptsUnknownHost(t *testing.T) { func TestGetHostKeyCallbackStrictModeRejectsUnknownHost(t *testing.T) { home := t.TempDir() - t.Setenv("HOME", home) + setTestHome(t, home) cfg := &Config{} callback, err := getHostKeyCallback(cfg) require.NoError(t, err) @@ -342,7 +342,7 @@ func TestGetHostKeyCallbackStrictModeRejectsUnknownHost(t *testing.T) { func TestGetHostKeyCallbackRejectsChangedKnownHostKey(t *testing.T) { home := t.TempDir() - t.Setenv("HOME", home) + setTestHome(t, home) knownHostsPath := filepath.Join(home, ".ssh", "known_hosts") hostWithPort := net.JoinHostPort("changed-host", "22") remote := &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 22} @@ -362,7 +362,7 @@ func TestGetHostKeyCallbackRejectsChangedKnownHostKey(t *testing.T) { func TestGetHostKeyCallbackInsecureFallbackRequiresExplicitOptIn(t *testing.T) { home := t.TempDir() - t.Setenv("HOME", home) + setTestHome(t, home) knownHostsPath := filepath.Join(home, ".ssh") require.NoError(t, os.MkdirAll(knownHostsPath, 0o700)) diff --git a/internal/sshclient/runcommand_test.go b/internal/sshclient/runcommand_test.go index 89b5a81..fecfc85 100644 --- a/internal/sshclient/runcommand_test.go +++ b/internal/sshclient/runcommand_test.go @@ -170,7 +170,7 @@ func sendExitStatus(ch ssh.Channel, status uint32) { func dialTestClient(t *testing.T, host, port string) *SSHClient { t.Helper() - t.Setenv("HOME", t.TempDir()) + setTestHome(t, t.TempDir()) client, err := NewSSHClient(&Config{ Host: host, diff --git a/internal/sshclient/testhome_test.go b/internal/sshclient/testhome_test.go new file mode 100644 index 0000000..2ee0f8c --- /dev/null +++ b/internal/sshclient/testhome_test.go @@ -0,0 +1,17 @@ +package sshclient + +import ( + "runtime" + "testing" +) + +// setTestHome points the user home directory at dir for one test. Go resolves +// os.UserHomeDir() from HOME on Unix and USERPROFILE on Windows, so setting +// only HOME would leave Windows tests writing into the real user profile. +func setTestHome(t *testing.T, dir string) { + t.Helper() + t.Setenv("HOME", dir) + if runtime.GOOS == "windows" { + t.Setenv("USERPROFILE", dir) + } +} diff --git a/internal/sshclient/validate.go b/internal/sshclient/validate.go index bddf91c..93d279c 100644 --- a/internal/sshclient/validate.go +++ b/internal/sshclient/validate.go @@ -25,107 +25,30 @@ func (e *CommandBlockedError) Error() string { } // ValidateCommand performs a best-effort safety check against a small set of -// well-known destructive commands (for example "rm -rf /" or a fork bomb). +// well-known destructive operations (for example "rm -rf /" or a fork bomb). +// +// Matching happens on the token in *command position* after shell segmentation, +// not on the raw command string. That distinction matters: `last reboot -F`, +// `journalctl | grep -iE 'fail|halt'`, and `iptables-save | grep -F ...` are +// read-only and must not be blocked just because they contain a dangerous word. // // It is a guardrail to catch accidental mistakes, NOT a security boundary: the -// substring/keyword matching is trivially bypassed (casing, quoting, shell -// variables, alternate paths), so it must never be relied upon to sandbox -// untrusted input. +// matching is trivially bypassed (obfuscation, indirection, generated command +// strings), so it must never be relied upon to sandbox untrusted input. func ValidateCommand(command string) error { cmd := strings.TrimSpace(command) - cmdLower := strings.ToLower(cmd) - - dangerousExactPatterns := []struct { - pattern string - reason string - }{ - {" rm -rf / ", "Delete root directory"}, - {" rm -rf /$", "Delete root directory"}, - {" rm -rf /;", "Delete root directory"}, - {" rm -rf /&", "Delete root directory"}, - {" rm -rf /|", "Delete root directory"}, - {"rm -rf / ", "Delete root directory"}, - {"rm -rf /$", "Delete root directory"}, - {"rm -rf /;", "Delete root directory"}, - {"rm -rf /*", "Delete all files in root directory"}, - {"rm -rf ~", "Delete user home directory"}, - {"rm -rf ~/", "Delete user home directory"}, - {"rm -rf $home", "Delete $HOME directory"}, - {":(){:|:&};:", "Fork bomb"}, - {"> /etc/passwd", "Overwrite system password file"}, - {"> /etc/shadow", "Overwrite system shadow file"}, - {"dd if=/dev/zero", "Dangerous dd operation"}, - {"dd if=/dev/urandom", "Dangerous dd operation"}, + if cmd == "" { + return nil } - for _, pattern := range dangerousExactPatterns { - cmdWithSpaces := " " + cmdLower + " " - patternLower := strings.ToLower(pattern.pattern) - - if strings.HasSuffix(pattern.pattern, "$") { - patternLower = strings.TrimSuffix(patternLower, "$") - if strings.HasSuffix(cmdLower, patternLower) { - return &CommandBlockedError{Command: cmd, Reason: pattern.reason} - } - } else if strings.Contains(cmdWithSpaces, patternLower) { - return &CommandBlockedError{Command: cmd, Reason: pattern.reason} - } + // Fork bombs survive no useful tokenization, so match the literal shape + // after stripping whitespace. + if isForkBomb(cmd) { + return &CommandBlockedError{Command: cmd, Reason: "Fork bomb"} } - dangerousPatterns := []struct { - keywords []string - reason string - }{ - {[]string{"mkfs."}, "Format filesystem"}, - {[]string{"mkfs", "ext4"}, "Format filesystem"}, - {[]string{"mkfs", "ext3"}, "Format filesystem"}, - {[]string{"mkfs", "xfs"}, "Format filesystem"}, - {[]string{"fdisk", "/dev/"}, "Disk partition operation"}, - {[]string{"parted", "/dev/"}, "Disk partition operation"}, - {[]string{"mkswap", "/dev/"}, "Create swap partition"}, - {[]string{"shutdown"}, "System shutdown operation"}, - {[]string{"halt"}, "System halt operation"}, - {[]string{"poweroff"}, "System poweroff operation"}, - {[]string{"reboot"}, "System reboot operation"}, - {[]string{"init 0"}, "System shutdown (init 0)"}, - {[]string{"init 6"}, "System reboot (init 6)"}, - {[]string{"systemctl", "halt"}, "System halt operation"}, - {[]string{"systemctl", "poweroff"}, "System poweroff operation"}, - {[]string{"systemctl", "reboot"}, "System reboot operation"}, - {[]string{"curl", "| sh"}, "Download and execute script from network"}, - {[]string{"curl", "| bash"}, "Download and execute script from network"}, - {[]string{"curl", "|sh"}, "Download and execute script from network"}, - {[]string{"curl", "|bash"}, "Download and execute script from network"}, - {[]string{"wget", "| sh"}, "Download and execute script from network"}, - {[]string{"wget", "| bash"}, "Download and execute script from network"}, - {[]string{"wget", "|sh"}, "Download and execute script from network"}, - {[]string{"wget", "|bash"}, "Download and execute script from network"}, - {[]string{"chmod", "777", "/ "}, "Set root directory permissions to 777"}, - {[]string{"chmod", "777", "/$"}, "Set root directory permissions to 777"}, - {[]string{"chmod", "-r", "777", "/ "}, "Recursively set root directory permissions to 777"}, - {[]string{"chmod", "-r", "777", "/$"}, "Recursively set root directory permissions to 777"}, - {[]string{"iptables", "-f"}, "Flush firewall rules"}, - {[]string{"iptables", "-x"}, "Delete firewall chain"}, - } - - for _, pattern := range dangerousPatterns { - allMatch := true - for _, keyword := range pattern.keywords { - keywordLower := strings.ToLower(keyword) - if strings.HasSuffix(keyword, "$") { - keywordLower = strings.TrimSuffix(keywordLower, "$") - if !strings.HasSuffix(cmdLower, keywordLower) { - allMatch = false - break - } - } else if !strings.Contains(cmdLower, keywordLower) { - allMatch = false - break - } - } - if allMatch { - return &CommandBlockedError{Command: cmd, Reason: pattern.reason} - } + if reason, found := detectDestructiveCommand(cmd, 0); found { + return &CommandBlockedError{Command: cmd, Reason: reason} } if engine, client, found := detectGuardedDBClient(cmd, 0); found { @@ -143,6 +66,18 @@ func ValidateCommand(command string) error { return nil } +// isForkBomb matches the classic `:(){:|:&};:` shape regardless of spacing. +func isForkBomb(cmd string) bool { + var b strings.Builder + for _, r := range cmd { + if r == ' ' || r == '\t' || r == '\n' || r == '\r' { + continue + } + b.WriteRune(r) + } + return strings.Contains(b.String(), ":(){:|:&};:") +} + // CommandUsesSudo reports whether sshx can safely treat the command as a sudo // command for password auto-fill. Only a leading sudo command is supported, // because that is the only form sudoStdinCommand can rewrite without guessing at diff --git a/internal/sshclient/validate_destructive.go b/internal/sshclient/validate_destructive.go new file mode 100644 index 0000000..f64d053 --- /dev/null +++ b/internal/sshclient/validate_destructive.go @@ -0,0 +1,476 @@ +package sshclient + +import ( + "strings" +) + +// This file detects destructive commands by inspecting the token in *command +// position*, reusing the shell segmentation used by the guarded-SQL detector. +// +// The previous implementation matched keywords anywhere in the raw command +// string, which produced overwhelming false positives on ordinary read-only +// operations: `last reboot -F` matched "reboot", `journalctl | grep -iE +// 'fail|halt'` matched "halt", and `iptables-save | grep -F ...` matched the +// ("iptables", "-f") keyword pair. A guardrail that fires on reads trains the +// operator to reflexively pass --force, which defeats its purpose. +// +// Like ValidateCommand as a whole, this is a guardrail against accidental +// mistakes, NOT a security boundary. + +// destructiveRule matches the arguments of one command in command position. +// args excludes the command name itself. +type destructiveRule struct { + reason string + match func(args []string) bool +} + +// destructiveCommands maps a lowercased command basename to its rules. Rules +// are evaluated in order and the first match wins. `rm` and `mkfs.` +// are handled in matchDestructiveRules because they need a dynamic reason. +var destructiveCommands = map[string][]destructiveRule{ + "fdisk": {{reason: "Disk partition operation", match: partitionsDevice}}, + "sfdisk": {{reason: "Disk partition operation", match: partitionsDevice}}, + "parted": {{reason: "Disk partition operation", match: partitionsDevice}}, + "mkswap": {{reason: "Create swap partition", match: hasDevicePath}}, + "mkfs": {{reason: "Format filesystem", match: always}}, + "shutdown": {{reason: "System shutdown operation", match: always}}, + "halt": {{reason: "System halt operation", match: always}}, + "poweroff": {{reason: "System poweroff operation", match: always}}, + "reboot": {{reason: "System reboot operation", match: always}}, + "init": {{reason: "System shutdown (init 0)", match: initRunlevel("0")}, {reason: "System reboot (init 6)", match: initRunlevel("6")}}, + "systemctl": {{reason: "System halt operation", match: systemctlVerb("halt")}, {reason: "System poweroff operation", match: systemctlVerb("poweroff")}, {reason: "System reboot operation", match: systemctlVerb("reboot")}, {reason: "System kexec operation", match: systemctlVerb("kexec")}}, + "iptables": {{reason: "Flush firewall rules", match: iptablesFlag("-F", "--flush")}, {reason: "Delete firewall chain", match: iptablesFlag("-X", "--delete-chain")}}, + "ip6tables": {{reason: "Flush firewall rules", match: iptablesFlag("-F", "--flush")}, {reason: "Delete firewall chain", match: iptablesFlag("-X", "--delete-chain")}}, + "chmod": {{reason: "Set root directory permissions to 777", match: chmod777Root}}, + "chown": {{reason: "Recursively change ownership of the root directory", match: chownRoot}}, + "dd": {{reason: "Dangerous dd operation", match: ddDangerous}}, + "vgremove": {{reason: "Remove LVM volume group", match: always}}, + "lvremove": {{reason: "Remove LVM logical volume", match: always}}, + "pvremove": {{reason: "Remove LVM physical volume", match: always}}, + "wipefs": {{reason: "Erase filesystem signatures", match: wipefsDestructive}}, + "shred": {{reason: "Irreversibly shred a block device", match: hasDevicePath}}, + "zpool": {{reason: "Destroy ZFS pool", match: firstWordIs("destroy")}}, + "zfs": {{reason: "Destroy ZFS dataset", match: firstWordIs("destroy")}}, +} + +func always([]string) bool { return true } + +// tildeTargets and homeVarTargets are home-directory spellings whose recursive +// deletion is blocked. They are separated so the reported reason matches the +// spelling the caller actually used. +var tildeTargets = map[string]bool{ + "~": true, "~/": true, "~/*": true, +} + +var homeVarTargets = map[string]bool{ + "$home": true, "${home}": true, "$home/": true, "${home}/": true, + "$home/*": true, "${home}/*": true, +} + +// systemDirTargets are top-level system directories whose recursive removal +// breaks the host even though they are not literally "/". +var systemDirTargets = map[string]bool{ + "/bin": true, "/boot": true, "/etc": true, "/lib": true, "/lib64": true, + "/proc": true, "/sbin": true, "/sys": true, "/usr": true, "/var": true, +} + +// rmVerdict reports whether an `rm` invocation recursively targets the root, +// the user home, or a critical system directory. A recursive delete of any +// ordinary path (/tmp/x, /home/user/test, /var/log/app) is left alone. +func rmVerdict(args []string) (string, bool) { + recursive := false + noPreserveRoot := false + var targets []string + for _, a := range args { + lower := strings.ToLower(a) + switch { + case lower == "--no-preserve-root": + noPreserveRoot = true + case lower == "--recursive": + recursive = true + case strings.HasPrefix(a, "--"): + // other long option + case strings.HasPrefix(a, "-") && len(a) > 1: + if strings.Contains(lower[1:], "r") { + recursive = true + } + default: + targets = append(targets, a) + } + } + if !recursive { + return "", false + } + if noPreserveRoot { + return "Delete root directory", true + } + for _, t := range targets { + lower := strings.ToLower(t) + switch { + case t == "/" || t == "/.": + return "Delete root directory", true + case t == "/*": + return "Delete all files in root directory", true + case homeVarTargets[lower]: + return "Delete $HOME directory", true + case tildeTargets[lower]: + return "Delete user home directory", true + } + if systemDirTargets[strings.TrimSuffix(lower, "/")] { + return "Delete a critical system directory (" + t + ")", true + } + } + return "", false +} + +// readOnlyDiskArgs are arguments that make a partition tool report instead of +// modify. `-s` is deliberately absent: it means "print size" for fdisk but +// "script mode" for parted, where it enables unattended destructive edits. +var readOnlyDiskArgs = map[string]bool{ + "-l": true, "--list": true, "-d": true, "--dump": true, + "print": true, "p": true, "-v": true, "--version": true, +} + +// partitionsDevice blocks partition editors that name a device, while leaving +// the reporting forms (`fdisk -l /dev/sda`, `parted /dev/sdb print`) alone. +func partitionsDevice(args []string) bool { + for _, a := range args { + if readOnlyDiskArgs[strings.ToLower(a)] { + return false + } + } + return hasDevicePath(args) +} + +func hasDevicePath(args []string) bool { + for _, a := range args { + if strings.HasPrefix(a, "/dev/") { + return true + } + } + return false +} + +// wipefsDestructive blocks wipefs only when it actually erases. Invoked with +// just a device it prints the detected signatures, and -n/--no-act is a dry run. +func wipefsDestructive(args []string) bool { + if !hasDevicePath(args) { + return false + } + erases := false + for _, a := range args { + switch { + case a == "-n" || a == "--no-act": + return false + case a == "-a" || a == "--all" || a == "-o" || a == "--offset": + erases = true + case strings.HasPrefix(a, "--offset="): + erases = true + } + } + return erases +} + +func chownRoot(args []string) bool { + recursive := false + for _, a := range args { + lower := strings.ToLower(a) + if lower == "--recursive" { + recursive = true + continue + } + if strings.HasPrefix(a, "-") && !strings.HasPrefix(a, "--") && strings.Contains(lower[1:], "r") { + recursive = true + } + } + if !recursive { + return false + } + for _, a := range args { + if a == "/" { + return true + } + } + return false +} + +func firstWordIs(want string) func([]string) bool { + return func(args []string) bool { + for _, a := range args { + if strings.HasPrefix(a, "-") { + continue + } + return strings.EqualFold(a, want) + } + return false + } +} + +func initRunlevel(level string) func([]string) bool { + return func(args []string) bool { + for _, a := range args { + if strings.HasPrefix(a, "-") { + continue + } + return a == level + } + return false + } +} + +// systemctlVerb matches `systemctl `; it compares whole tokens so unit +// names such as reboot.target or a `systemctl status halt.service` query are +// not treated as a power action. +func systemctlVerb(verb string) func([]string) bool { + return func(args []string) bool { + for _, a := range args { + if strings.HasPrefix(a, "-") { + continue + } + return strings.EqualFold(a, verb) + } + return false + } +} + +// iptablesFlag matches iptables control flags. Matching is case-sensitive +// because iptables distinguishes -F (flush) from -f (fragment) and -X +// (delete chain) from -x (exact). +func iptablesFlag(flags ...string) func([]string) bool { + return func(args []string) bool { + for _, a := range args { + for _, f := range flags { + if a == f { + return true + } + } + } + return false + } +} + +func chmod777Root(args []string) bool { + has777 := false + rootTarget := false + for _, a := range args { + if a == "777" || a == "0777" { + has777 = true + continue + } + if a == "/" { + rootTarget = true + } + } + return has777 && rootTarget +} + +func ddDangerous(args []string) bool { + for _, a := range args { + lower := strings.ToLower(a) + if lower == "if=/dev/zero" || lower == "if=/dev/urandom" || lower == "if=/dev/random" { + return true + } + // Writing straight to a whole block device destroys its contents. + if v, ok := strings.CutPrefix(lower, "of=/dev/"); ok { + if v != "null" && v != "stdout" && v != "stderr" { + return true + } + } + } + return false +} + +// maxDestructiveDepth bounds recursion into nested shell payloads. +const maxDestructiveDepth = 6 + +// detectDestructiveCommand reports the reason a command is considered +// destructive, inspecting only tokens in command position. +func detectDestructiveCommand(command string, depth int) (string, bool) { + if depth > maxDestructiveDepth { + return "", false + } + segments := splitShellSegments(command) + sawDownloader := false + + for _, seg := range segments { + name, args, kind := commandInPosition(seg) + switch kind { + case positionShellStdin: + // `curl ... | sh` — a shell reading its script from the pipe. + if sawDownloader { + return "Download and execute script from network", true + } + case positionShellScript: + if reason, found := detectDestructiveCommand(args[0], depth+1); found { + return reason, true + } + case positionCommand: + if reason, found := matchDestructiveRules(name, args); found { + return reason, true + } + if name == "curl" || name == "wget" { + sawDownloader = true + } + } + if reason, found := redirectOverwrite(seg); found { + return reason, true + } + } + return "", false +} + +func matchDestructiveRules(name string, args []string) (string, bool) { + if name == "rm" { + return rmVerdict(args) + } + // mkfs. variants beyond the explicit table entries. + if strings.HasPrefix(name, "mkfs.") { + return "Format filesystem", true + } + rules, ok := destructiveCommands[name] + if !ok { + return "", false + } + for _, r := range rules { + if r.match == nil { + continue + } + if r.match(args) { + return r.reason, true + } + } + return "", false +} + +// redirectOverwrite detects `> /etc/passwd`-style truncation of critical +// identity files anywhere in a segment's tokens. +func redirectOverwrite(tokens []string) (string, bool) { + critical := map[string]string{ + "/etc/passwd": "Overwrite system password file", + "/etc/shadow": "Overwrite system shadow file", + } + for i, tok := range tokens { + var target string + switch { + case tok == ">" || tok == ">>": + if i+1 < len(tokens) { + target = tokens[i+1] + } + case strings.HasPrefix(tok, ">>"): + target = tok[2:] + case strings.HasPrefix(tok, ">"): + target = tok[1:] + } + if target == "" { + continue + } + if reason, ok := critical[strings.ToLower(target)]; ok { + return reason, true + } + } + return "", false +} + +type positionKind int + +const ( + positionNone positionKind = iota + positionCommand + // positionShellScript is `sh -c '