fix(ssh): harden logging and execution cleanup

[appleboy]

Summary

Debug mode previously dumped SSH/proxy credentials and forwarded environment values. A host failure could also leave workers blocked sending errors after Exec returned, and closed output channels could keep the stream reader spinning.

• Keep godump and render a display-only configuration copy: redact non-empty SSH/proxy keys, passwords, and passphrases while preserving other settings and distinguishing unset credentials. Log forwarded environment values as [REDACTED] while preserving the actual remote values.
• Return one result per host. Stop subsequent hosts on a synchronous failure; wait for all started parallel hosts before returning the first reported error, wrapped with the host name.
• Disable closed stdout, stderr, and error channels. Preserve remote errors and return once on timeout.
• Add deterministic testing/synctest regressions and document the changed failure and logging behavior.

Related issues

GitHub/Jira: N/A; no issue was supplied.

Architecture / flow

flowchart TD
    A["main.go: run — dump redacted Config copy"] --> B["plugin.go: Exec / execHosts"]
    B --> C{"Sync?"}
    C -->|Yes| D["Run hosts in order; stop on failure"]
    C -->|No| E["Start workers; buffer at most one error per host"]
    D --> F["exec: redact debug environment values; start SSH"]
    E --> F
    F --> G["readStream: disable closed channels; return one result"]
    G --> H["execHosts: finish started workers; return first error or success"]
    style A fill:#dbeafe
    style B fill:#dbeafe
    style D fill:#dbeafe
    style E fill:#dbeafe
    style F fill:#dbeafe
    style G fill:#dbeafe
    style H fill:#dbeafe

Loading

AI authorship

• AI was used: OpenAI Codex (GPT-6).
• AI-authored changes: main.go, plugin.go, plugin_regression_test.go, plugin_test.go, DOCS.md.
• Human line-by-line reviewed: None — not yet reviewed by a human.

Change classification

• Core change: shared SSH execution, concurrent worker lifetime, and credential logging.
• Leaf change

Plan reference

Scope: fix credential disclosure in debug metadata, blocked error reporting, and closed-channel busy loops. IPv6 parsing, concurrency limits, and general output synchronization are outside this change.

Verification

Setup

Check out fix/ssh-execution-safety and run commands from the repository root. Local checks require Go 1.26.8. The complete suite additionally needs Docker, IPv6, and an Alpine SSH fixture; it must not run make ssh-server on the host OS.

The following reproduces the isolated environment used for the full test run. It copies the checkout rather than relying on host directory mounts. Docker must expose its Unix socket to the container and support host-gateway.

docker create --name drone-ssh-pr-check \
  --sysctl net.ipv6.conf.all.disable_ipv6=0 \
  --add-host host.docker.internal:host-gateway \
  -e TESTCONTAINERS_HOST_OVERRIDE=host.docker.internal \
  -v /var/run/docker.sock:/var/run/docker.sock \
  golang:1.26.8-alpine sh -ec '
    apk add --no-cache git make bash build-base sudo openssh openrc
    cd /work
    make ssh-server
    for attempt in 1 2 3 4 5; do
      if ssh-keyscan localhost >/dev/null 2>&1; then break; fi
      sleep 1
    done
    go test -race -v -count=1 -timeout 10m ./...
  '
docker cp . drone-ssh-pr-check:/work
docker start -a drone-ssh-pr-check
docker inspect --format '{{.State.ExitCode}}' drone-ssh-pr-check
docker rm drone-ssh-pr-check

Readiness is a successful ssh-keyscan localhost; the test completion signals are PASS, package ok, and container exit code 0. Inspect verbose output for skips as well as failures. The testcontainers cases create disposable SSH containers through the Docker socket. Cleanup removes the test runner; testcontainers removes its SSH fixtures. The completed verification run cleaned up its runner.

Automated checks

All checks below ran against the files included in this PR.

Command / location	Behavior covered and expected success	Status	Observed result
go test -race -v -count=1 -timeout 10m ./... in the Alpine fixture	Full suite, real SSH/proxy/IPv6, environment forwarding, sudo/PTY, timeouts, and regressions; no skips or races	Passed	Package ok in 15.606s; all cases passed, no skips or race reports
make lint at repository root	Static checks	Passed	0 issues
make fmt-check at repository root	Formatting	Passed	No formatting diff
go vet ./... at repository root	Go analysis	Passed	No issues
go build -o /tmp/drone-ssh-pr-4641 . at repository root	CLI builds	Passed	Exit 0
git diff --staged --check before commit	Patch whitespace	Passed	No errors

Behavioral scenarios

The focused tests below require no SSH service or Docker. From the checked-out repository root, run each command with go test -race -v -count=1 -timeout 60s -run '<pattern>' ./..., substituting the pattern from the table. Each row is Passed as part of the full run above; expected assertions and observed outcomes are recorded separately.

Step / pattern	Starting state and expected result	Status / observed result
1. ^(TestRunDebugRedactsCredentials|TestConfigRedactedPreservesOriginal|TestDebugRedactsEnvironment)$	Synthetic SSH/proxy credentials and a forwarded environment value; captured debug logs redact all six credential fields and forwarded values, retain non-sensitive settings, preserve unset fields, and leave the actual connection configuration unchanged	Passed; all three tests passed
2. ^TestExecHosts	Controlled workers: an early failure, a blocked worker, and success. Parallel execution waits for released workers and preserves the first failure; synchronous execution never visits the host after a failure; success visits every host	Passed; all three tests passed; synctest completed with no stranded goroutines
3. ^TestReadStream	Controlled channels close before completion. The reader blocks rather than spins, retains the error after the error channel closes, and returns one result for timeout	Passed; both tests and all timeout/error subcases passed
4. ^TestExecMultipleConnectionFailures$	Two connections to loopback port 0 fail; execution returns a host-prefixed error after both workers finish	Passed

For the actual remote environment-value preservation scenario, the complete Docker run executes TestEnvOutput and TestAllEnvs: original values (including quotes and whitespace) must reach the remote script while debug metadata is redacted. Passed; both assertions passed. The focused tests clean up their own temporary files and environment variables.

Security check

• No real secrets in the diff; new credential strings are test fixtures.
• Configuration dumps use a redacted copy; credential values and environment debug values are masked.
• Input-validation and authorization rules are unchanged; no new permission boundary is introduced.
• Script text, remote stdout/stderr, and underlying SSH error messages are not globally scrubbed. Scripts that contain or print secrets can still expose them; this limitation is documented.

Risk and rollback

• Parallel failures now return only after all started hosts finish or time out, so failure reporting can take longer than before. This does not cancel remote commands on other hosts.
• Debug ENV formatting changes, and host errors gain a host prefix. Error wrapping preserves errors.Is / errors.As behavior.
• Rollback: revert the PR commits. No schema or data migration is involved; reverting also restores the original logging and worker-lifecycle defects.

Reviewer guide

Request two or more reviewers, including the module owner. Read main.go:run and plugin.go:exec, readStream, and execHosts line by line, especially timeout/error ordering and worker completion. Review Config.redacted and its regression assertions to confirm debug rendering preserves the actual connection configuration; check the documented compatibility changes. No human line-by-line review has been recorded yet.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.