diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 0c984be..8d12687 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -1,27 +1,41 @@ -# This workflow will build a golang project -# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-go - name: Go on: push: - branches: [ "main" ] + branches: [main] pull_request: - branches: [ "main" ] + branches: [main] + +permissions: + contents: read jobs: - build: + test: + strategy: + matrix: + os: [ubuntu-latest, macos-latest] + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + - run: go vet ./... + - run: go test -race -count=1 -cover ./... + - run: python3 -m unittest discover -s scripts -p 'test_*.py' + - name: PID 1 and capability integration tests + if: runner.os == 'Linux' + run: | + docker pull alpine:3.21 + GONNER_DOCKER_TESTS=1 go test -count=1 -timeout=10m ./internal/integration + vulnerabilities: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - - name: Set up Go - uses: actions/setup-go@v5 + - uses: actions/setup-go@v5 with: go-version-file: go.mod - - - name: Build - run: go build -v ./... - - - name: Test - run: go test -v ./... + - run: go install golang.org/x/vuln/cmd/govulncheck@v1.8.0 + - run: govulncheck ./... + - name: Build and scan all release targets + run: bash scripts/check-artifacts.sh diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ae4524f..9529dc0 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -15,7 +15,31 @@ concurrency: cancel-in-progress: false jobs: + verify: + if: github.event.pull_request.merged == true + strategy: + matrix: + os: [ubuntu-latest, macos-latest] + runs-on: ${{ matrix.os }} + permissions: + contents: read + steps: + - uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.merge_commit_sha }} + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + - run: go vet ./... + - run: go test -race -count=1 -cover ./... + - run: python3 -m unittest discover -s scripts -p 'test_*.py' + - name: PID 1 and capability integration tests + if: runner.os == 'Linux' + run: | + docker pull alpine:3.21 + GONNER_DOCKER_TESTS=1 go test -count=1 -timeout=10m ./internal/integration release: + needs: verify if: github.event.pull_request.merged == true runs-on: ubuntu-latest @@ -40,17 +64,9 @@ jobs: - name: Determine release bump id: bump shell: bash - run: | - labels='${{ toJson(github.event.pull_request.labels.*.name) }}' - bump=patch - - if echo "$labels" | grep -q '"release:major"'; then - bump=major - elif echo "$labels" | grep -q '"release:minor"'; then - bump=minor - fi - - echo "level=$bump" >> "$GITHUB_OUTPUT" + env: + PR_LABELS_JSON: ${{ toJson(github.event.pull_request.labels.*.name) }} + run: python3 scripts/release_bump.py >> "$GITHUB_OUTPUT" - name: Compute next tag id: version @@ -91,17 +107,18 @@ jobs: - name: Create local release tag run: git tag "${{ steps.version.outputs.tag }}" "${{ github.event.pull_request.merge_commit_sha }}" - - name: Run tests - run: | - export PATH="$(go env GOPATH)/bin:$PATH" - task test - - name: Build release artifacts run: | export PATH="$(go env GOPATH)/bin:$PATH" task release (cd bin && shasum -a 256 gonner-* > checksums.txt) + - name: Check vulnerabilities before publishing + run: | + go install golang.org/x/vuln/cmd/govulncheck@v1.8.0 + govulncheck ./... + bash scripts/check-artifacts.sh bin + - name: Push release tag run: git push origin "${{ steps.version.outputs.tag }}" @@ -113,4 +130,4 @@ jobs: bin/* \ --target "${{ github.event.pull_request.merge_commit_sha }}" \ --title "${{ steps.version.outputs.tag }}" \ - --generate-notes \ No newline at end of file + --generate-notes diff --git a/.gitignore b/.gitignore index 31e57fa..b6613e6 100644 --- a/.gitignore +++ b/.gitignore @@ -23,4 +23,6 @@ coverage.txt # IDEs .vscode/ -.zed/ \ No newline at end of file +.zed/ +# Python release helper bytecode +__pycache__/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 6931147..aee8727 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,35 @@ All notable changes to this project are documented in this file. The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and the project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## v0.1.0 + +### Security + +- Resolve all 25 findings from the codebase audit, with permanent regression coverage and a resolution matrix in `docs/audits/2026-09-25-codebase-audit.md`. +- Validate log and PID paths through retained directory descriptors; reject unsafe ownership, writable ancestors, leaf symlinks, hardlinks, special files, and unsafe ACLs before workloads start. +- Resolve numeric users through account lookup, require a group for unknown UIDs, clear supplementary groups, and check Linux capabilities including `CAP_KILL` for cross-UID supervision. +- Raise the Go baseline to 1.26.7 and scan source plus all release binaries with pinned govulncheck v1.8.0 before publication. +- Parse release labels as JSON environment data instead of interpolating them into shell commands. + +### Fixed + +- Give every managed child exactly one wait owner; coordinate Linux PID 1 orphan reaping with the shared execution service used by main commands, precommands, and command conditions. +- Treat macOS groups containing only exited processes as stopped while preserving signal permission errors for live processes. +- Preserve oversized and unterminated output, drain pipes after child exit, share log sinks across instances, prevent backup collisions, and keep console output flowing after file errors. +- Propagate unavailable dependencies without hangs and allow any successfully started instance to satisfy the initial dependency gate. +- Report permanent failures with exit 1, cancel all work for critical failures including required precommands, and allow unrelated work to continue after noncritical failures. +- Correct restart counts, backoff stability tracking, and process status while waiting to restart. +- Validate TLS certificates before binding or starting workloads, propagate unexpected health-server failures, and protect uptime reads against races. +- Include pending and skipped critical processes in readiness and status, normalize bare ports in TCP conditions, and honor an explicit `gonner status --port 8089` over environment defaults. + +### Changed + +- **Migration required:** custom conditions implement `Evaluate(context.Context) (bool, error)`; command conditions inherit process credentials, environment, and working directory and terminate their group on cancellation or timeout. +- **Migration required:** strict path and privilege validation can reject previously accepted configurations; sequential forward dependencies are rejected. +- Oversized console lines are split into prefixed 64 KiB chunks. Raw files remain byte-exact, and legacy rotated files are preserved rather than pruned. +- Release publication requires native Linux and macOS verification, Docker PID 1 and privilege-boundary tests, and vulnerability scans. Binaries cover Linux/macOS on amd64 and arm64. +- See `docs/operations.md#upgrade-from-the-audited-baseline` before upgrading. + ## v0.0.3 ### Added diff --git a/README.md b/README.md index cf76e30..2a639bc 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ A lightweight, PID-1-aware process manager for containers. Single static Go binary, no runtime dependencies. Designed to be a Docker `ENTRYPOINT` that orchestrates multiple long-running services, but works equally well for local development. -[![Go Version](https://img.shields.io/badge/go-1.25%2B-blue)](go.mod) +[![Go Version](https://img.shields.io/badge/go-1.26.7%2B-blue)](go.mod) [![License](https://img.shields.io/badge/license-MIT-green)](LICENSE) --- @@ -44,7 +44,7 @@ A lightweight, PID-1-aware process manager for containers. Single static Go bina Download a prebuilt binary from [Releases](https://github.com/michaelishri/gonner/releases), or build from source: ```bash -# requires Go 1.25+ +# requires Go 1.26.7+ go install github.com/michaelishri/gonner/cmd/gonner@latest ``` diff --git a/Taskfile.yml b/Taskfile.yml index fbe9265..b1b8cb7 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -22,7 +22,7 @@ tasks: test: desc: Run tests with race detection and coverage cmds: - - go test ./... -v -race -cover + - go test ./... -v -race -count=1 -cover lint: desc: Run golangci-lint diff --git a/cmd/gonner/cmd/run.go b/cmd/gonner/cmd/run.go index 0b10e16..efacde0 100644 --- a/cmd/gonner/cmd/run.go +++ b/cmd/gonner/cmd/run.go @@ -2,8 +2,10 @@ package cmd import ( "context" + "errors" "fmt" "os" + "path/filepath" "github.com/spf13/cobra" @@ -11,6 +13,7 @@ import ( "github.com/michaelishri/gonner/internal/health" "github.com/michaelishri/gonner/internal/logging" "github.com/michaelishri/gonner/internal/runner" + "github.com/michaelishri/gonner/internal/safefile" ) var ( @@ -68,6 +71,8 @@ func runRun(_ *cobra.Command, _ []string) error { go sigHandler.Start() defer sigHandler.Stop() + var healthErr error + var healthDone chan struct{} // Start health endpoint if configured if port > 0 { opts := health.Options{ @@ -87,21 +92,48 @@ func runRun(_ *cobra.Command, _ []string) error { if err := healthSrv.Start(ctx); err != nil { return fmt.Errorf("failed to start health endpoint: %w", err) } + healthDone = make(chan struct{}) + go func() { + defer close(healthDone) + healthErr = <-healthSrv.Errors() + if healthErr != nil { + cancel() + } + }() + defer func() { cancel(); <-healthSrv.Done(); <-healthDone }() + } logging.Gonner("Gonner starting (PID %d)", os.Getpid()) - // Write PID file if configured. + // PID files use the same descriptor-relative safety policy as logs. if cfg.PIDFile != "" { - if err := os.WriteFile(cfg.PIDFile, []byte(fmt.Sprintf("%d\n", os.Getpid())), 0o644); err != nil { - logging.Gonner("warning: failed to write pidFile %s: %v", cfg.PIDFile, err) - } else { - defer os.Remove(cfg.PIDFile) + dir, err := safefile.OpenDirectory(filepath.Dir(cfg.PIDFile)) + if err != nil { + return fmt.Errorf("pidFile: %w", err) + } + defer dir.Close() + name := filepath.Base(cfg.PIDFile) + f, err := dir.OpenRegular(name, os.O_WRONLY|os.O_CREATE, 0644) + if err != nil { + return fmt.Errorf("pidFile: %w", err) + } + writeErr := f.Truncate(0) + if writeErr == nil { + _, writeErr = fmt.Fprintf(f, "%d\n", os.Getpid()) } + closeErr := f.Close() + if err := errors.Join(writeErr, closeErr); err != nil { + return fmt.Errorf("pidFile: %w", err) + } + defer dir.Remove(name) } - - // Run all processes (blocks until done) - if err := mgr.Run(ctx); err != nil { + err = mgr.Run(ctx) + cancel() + if healthDone != nil { + <-healthDone + } + if err = errors.Join(err, healthErr); err != nil { return fmt.Errorf("manager exited with error: %w", err) } diff --git a/cmd/gonner/cmd/shutdown_linux_test.go b/cmd/gonner/cmd/shutdown_linux_test.go index 4cdb37d..4f921a7 100644 --- a/cmd/gonner/cmd/shutdown_linux_test.go +++ b/cmd/gonner/cmd/shutdown_linux_test.go @@ -206,7 +206,7 @@ func TestRunShutdown(t *testing.T) { t.Fatal("supervisor did not finish shutdown") } elapsed := time.Since(start) - if waitErr != nil { + if (waitErr != nil) != tc.critical { t.Errorf("supervisor exit: %v\n%s", waitErr, &output) } stopped, err := os.ReadFile(filepath.Join(dir, "stopped")) @@ -302,3 +302,29 @@ func TestShutdownHelper(t *testing.T) { } os.Exit(0) } + +func TestInvalidTLSPreventsWorkloadStartup(t *testing.T) { + dir := t.TempDir() + marker := filepath.Join(dir, "should-not-run") + cfg := map[string]any{"health": map[string]any{"port": 8089, "tls": map[string]string{"certFile": "/gonner-missing-cert", "keyFile": "/gonner-missing-key"}}, "run": []map[string]any{{"name": "worker", "command": "touch " + shellQuote(marker)}}} + data, err := json.Marshal(cfg) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "gonner.json"), data, 0600); err != nil { + t.Fatal(err) + } + executable, err := os.Executable() + if err != nil { + t.Fatal(err) + } + command := exec.Command(executable, "-test.run=^TestShutdownHelper$", "--", "gonner") + command.Env = append(os.Environ(), "GONNER_SHUTDOWN_HELPER=1", "GONNER_SHUTDOWN_DIR="+dir, "GONNER_HEALTH_PORT=0") + output, err := command.CombinedOutput() + if err == nil || !strings.Contains(string(output), "loading health TLS certificate") { + t.Fatalf("exit=%v output=%s", err, output) + } + if _, err := os.Stat(marker); !os.IsNotExist(err) { + t.Fatal("workload ran after TLS startup failure") + } +} diff --git a/cmd/gonner/cmd/status.go b/cmd/gonner/cmd/status.go index 435e4bb..84c307f 100644 --- a/cmd/gonner/cmd/status.go +++ b/cmd/gonner/cmd/status.go @@ -54,8 +54,8 @@ type statusAPIResponse struct { } `json:"processes"` } -func runStatus(_ *cobra.Command, _ []string) error { - port := resolveStatusPort() +func runStatus(cmd *cobra.Command, _ []string) error { + port := resolveStatusPort(cmd.Flags().Changed("port")) scheme := "http" if statusTLS { scheme = "https" @@ -119,13 +119,14 @@ func runStatus(_ *cobra.Command, _ []string) error { // resolveStatusPort determines the status endpoint port. // CLI flag takes priority, then GONNER_HEALTH_PORT env var, then default. -func resolveStatusPort() int { +func resolveStatusPort(explicit bool) int { + if explicit { + return statusPort + } if envPort := os.Getenv("GONNER_HEALTH_PORT"); envPort != "" { var port int if _, err := fmt.Sscanf(envPort, "%d", &port); err == nil && port > 0 { - if statusPort == health.DefaultHealthPort { - return port - } + return port } } return statusPort diff --git a/cmd/gonner/cmd/status_test.go b/cmd/gonner/cmd/status_test.go new file mode 100644 index 0000000..04e56a2 --- /dev/null +++ b/cmd/gonner/cmd/status_test.go @@ -0,0 +1,26 @@ +package cmd + +import ( + "testing" + + "github.com/spf13/cobra" + + "github.com/michaelishri/gonner/internal/health" +) + +func TestExplicitDefaultStatusPortOverridesEnvironment(t *testing.T) { + old := statusPort + defer func() { statusPort = old }() + t.Setenv("GONNER_HEALTH_PORT", "9999") + command := &cobra.Command{} + command.Flags().IntVar(&statusPort, "port", health.DefaultHealthPort, "") + if got := resolveStatusPort(command.Flags().Changed("port")); got != 9999 { + t.Fatalf("env port=%d", got) + } + if err := command.Flags().Set("port", "8089"); err != nil { + t.Fatal(err) + } + if got := resolveStatusPort(command.Flags().Changed("port")); got != 8089 { + t.Fatalf("explicit port=%d", got) + } +} diff --git a/docs/architecture.md b/docs/architecture.md index f88b996..ab88e5f 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -49,10 +49,10 @@ To add a new condition type, implement `Condition` and call `condition.Register( ### `internal/runner` — lifecycle orchestration -- `manager.go` — top-level orchestrator. Evaluates conditions, builds the `[]*Process` set, drives `parallel` or `sequential` startup, gathers status for the health endpoint. +- `manager.go` — top-level orchestrator. Builds every configured instance at construction, performs preflight, evaluates conditions, drives `parallel` or `sequential` startup, gathers status for the health endpoint. - `process.go` — per-process state machine, restart loop, signal propagation, credential drop. - `backoff.go` — exponential backoff with ±10% jitter and post-stability reset. -- `signals.go` — `SignalHandler` (SIGTERM/SIGINT → shutdown, SIGHUP/USR* → forward) and `StartZombieReaper` (SIGCHLD-driven, PID-1 only). +- `signals.go` — `SignalHandler` (SIGTERM/SIGINT → shutdown, SIGHUP/USR* → forward). Signal registration is synchronous before workload startup. ### `internal/health` — HTTP endpoint @@ -61,30 +61,21 @@ To add a new condition type, implement `Condition` and call `condition.Register( ### `internal/logging` — multiplexed writer -- `writer.go` — per-process writer that fans out to stdout (prefixed) + log file (raw), with size-based rotation and optional gzip; also exposes `Gonner` (operational stderr) and `Recover` (panic-safe goroutines). +- `writer.go` — per-process prefix writers over a shared sink registry, with bounded streaming, exclusive size-based rotation and optional gzip; also exposes `Gonner` (operational stderr) and `Recover` (panic-safe goroutines). --- ## Concurrency model -- **Top context.** `cmd.run` creates a single cancellable `context.Context`. Cancellation propagates to: - - the signal handler (via the `cancelFn` it owns), - - each `Process.Run` (via `errgroup.WithContext`), - - the health server (graceful shutdown). -- **errgroup.** `Manager.Run` launches every process in an `errgroup.Group`. A returned error or context cancellation triggers each active command's graceful shutdown. The manager waits for all started lifecycles to finish, including when cancellation interrupts sequential startup. -- **Per-process goroutines.** Each instance has its own goroutine running `Process.Run`. Within that, the process spawns two more for stdout/stderr line scanning. -- **`readyCh` / `doneCh`.** Channels per process used by `dependsOn` waiters in `parallel` mode and the next-up sequencer in `sequential` mode. -- **Mutex.** `Process.mu` guards `currentCmd`, `cmdDone`, `startedAt`, `restarts`, `readyCh`. State itself is an `atomic.Value` so `State()` is lock-free. - ---- +- `Manager.Run` uses a cancellable context and a wait group. Permanent noncritical failures are collected without cancelling unrelated work; critical and infrastructure failures cancel the context. Every started lifecycle is joined before return. +- Each instance has stable first-success and lifecycle-done channels. Dependency and sequential gates watch every instance and terminate if all instances finish without starting. +- `Process.mu` protects its current PID, start time and successful restart count; state uses an atomic value. Manager uptime is mutex-protected and zero before `Run`. +- `internal/execution` owns main, precommand and probe execution without importing configuration or logging. Each child gets a process group and exactly one `exec.Cmd.Wait`. Explicit `os.Pipe` descriptors keep readers open after `Wait`, so output can drain before lifecycle completion. +- `internal/safefile` holds verified directory descriptors for logs and PID files. `internal/logging` shares descriptors, rotation counters and locks by canonical directory identity and leaf name. ## PID 1 reaping coordination -When `os.Getpid() == 1`, `StartZombieReaper` subscribes to `SIGCHLD`. On each signal it drains all currently-exited children via `wait4(-1, WNOHANG)`. Each reaped child's `WaitStatus` is stored in the package-level `ReapedStatuses` `sync.Map` keyed by PID. - -`Process.runCommand` calls `cmd.Wait()` as usual. If the reaper got there first, Go's `cmd.Wait` returns "wait: no child processes" (an error that is **not** an `*exec.ExitError`). At that point `runCommand` falls back to `ReapedStatuses.LoadAndDelete(pid)` to recover the real exit code. - -This avoids the classic "reaper-vs-exec.Cmd" race that plagues PID-1 supervisors written in Go. +On Linux PID 1, the execution service enumerates direct children through `/proc/self/task/*/children`. Startup fails if this discovery is unavailable. Reaping runs on `SIGCHLD` and at least once per second. A process-wide registry lock covers both `Start` plus registration and orphan selection plus PID-specific nonblocking `Wait4`. Registered children are always excluded: their sole wait owner is `exec.Cmd.Wait`. There is no wildcard wait or exit-status cache. The reaper stays active through workload cancellation and cleanup. Non-PID-1 Linux and Darwin do not start an orphan reaper. --- @@ -92,14 +83,14 @@ This avoids the classic "reaper-vs-exec.Cmd" race that plagues PID-1 supervisors 1. Trigger: SIGTERM/SIGINT received, **or** a critical process exits unexpectedly, **or** the outer context is cancelled. 2. `SignalHandler` (or `Process.onCriticalExit`) calls the top `cancelFn`. -3. The errgroup's context is cancelled; the manager marks itself as shutting down, and process lifecycles stop starting or restarting commands. -4. Each active command's custom `exec.Cmd.Cancel` handler performs graceful shutdown concurrently: +3. The manager context is cancelled; the manager marks itself as shutting down, and process lifecycles stop starting or restarting commands. +4. The shared execution service performs group shutdown concurrently for each active command: - send the configured `stopSignal` to the command's process group, - wait up to `stopTimeout` (per-process) or `shutdownTimeout` (global) for the group to exit, - escalate to SIGKILL for remaining group members, even if the original shell has already exited. - Both `commandsBefore` and main commands use this path. Each command has its own process group; its initial PID remains the group ID after the shell exits. Any descendants left in that group when a command exits normally are also stopped before the lifecycle advances. Descendants that create a separate session or process group are outside this group-based shutdown mechanism. + Both `commandsBefore` and main commands use this path. Command conditions use the same execution service but kill their group on probe cancellation or timeout. Each command has its own process group; its initial PID remains the group ID after the shell exits. Any descendants left in that group when a command exits normally are also stopped before the lifecycle advances. Descendants that create a separate session or process group are outside this group-based shutdown mechanism. 5. Health server's shutdown goroutine completes within 5s. -6. After all started process lifecycles finish shutdown and reap their direct children, `Manager.Run` returns; gonner exits with the errgroup's error (if any). +6. After all started process lifecycles finish shutdown and reap their direct children, `Manager.Run` returns; gonner exits 1 if any permanent or supervisor failure was recorded, otherwise 0. --- diff --git a/docs/audits/2026-09-25-codebase-audit.md b/docs/audits/2026-09-25-codebase-audit.md new file mode 100644 index 0000000..a35b8ce --- /dev/null +++ b/docs/audits/2026-09-25-codebase-audit.md @@ -0,0 +1,324 @@ +# Codebase bug and security audit — 2026-09-25 + +**Reviewed commit:** `a6720c79e4d703d4705233a29081303887f8d49c` + +**Audit branch:** `audit/codebase-bugs-security-2026-09-25` + +**Status:** Findings for review. No application fixes or implementation plans are included. + +The audit identified **25 findings: 6 high, 15 medium, and 4 low priority**. Four concern security boundaries or vulnerable builds (S1, S10, S11, S12). The remaining findings concern availability, data loss, lifecycle behavior, observability, or CLI correctness. Each numbered finding is independently referenceable for later planning; multiple reproductions of the same underlying issue are grouped together. + +## Scope and evidence + +Reviewed all production packages, their tests, CLI entry points, configuration examples, documentation, dependencies, build tasks, and both GitHub Actions workflows. The existing documentation supplies the behavioral specification and security model. The two review axes are kept separate below: **Standards** covers security and implementation correctness; **Spec** covers documented runtime behavior. + +Validation performed: + +- `go test -race -count=1 -cover ./...` — passed on Linux/amd64 with Go 1.26.0. Runner coverage was 59.2%; CLI coverage was 30.7%. Existing passing tests do not exercise the failure cases below. +- `go vet ./...` — passed. +- `CGO_ENABLED=0` builds for Linux and Darwin, each on amd64 and arm64 — passed. Darwin and arm64 binaries were compiled, not executed. +- Temporary focused Go tests, CLI/HTTP reproductions, and isolated Alpine 3.21 Docker containers — reproduced the application findings. Containers had no network access for the PID 1 and privilege tests. All temporary repository test files were removed afterward. +- `govulncheck` v1.8.0, database last modified `2026-09-24T20:07:49Z` — 18 symbol-level standard-library advisory IDs with the local Go 1.26.0 toolchain; **26** with the Go 1.25.0 toolchain selected by CI. See S12 and the appendix for interpretation. + +The review treats configuration and interpolated environment variables as trusted, as specified in [security.md](../security.md). Intentional `sh -c` execution, opt-in authentication/TLS, public probe endpoints, and configurable outbound port checks are not reported as vulnerabilities. No deployed service or published release binary was inspected, and no external exploit was attempted. Severity is an engineering assessment, not a CVSS score: **high** means a privilege boundary violation, exposed known vulnerability, or substantial lifecycle failure; **medium** means a material defect requiring a particular configuration or runtime condition; **low** means narrower correctness or observability impact. + +## Standards + +### S1 — A workload-controlled log symlink lets the root supervisor modify privileged files + +**High · Security · Reproduced across UIDs.** Locations: [writer.go](../../internal/logging/writer.go#L77), lines 77–90; rotation reopens the same path at line 161. + +`openLogFile` follows symlinks when opening a log and then changes permissions by pathname. If a lower-privileged workload can write the configured log directory, it can replace the log path with a symlink before startup or reopening. The supervisor subsequently appends that workload's output to the symlink target using the supervisor's privileges. The configuration itself does not need to be writable by the attacker. + +**Reproduction:** In an isolated container, create a root-owned `0600` file and a `0750` log directory owned by UID 65534. As UID 65534, verify direct writes to the root-owned file fail, then create `app.log` as a symlink to it. Run root gonner with `user: "65534"`, that `logFile`, and a command printing `AUDIT_APPEND`. Direct writing failed with permission denied; the target nevertheless acquired `AUDIT_APPEND` through gonner. A separate test also confirmed the target's mode is changed to `0600`. + +**Impact/preconditions:** Arbitrary privileged-file append and permission modification, potentially enabling privilege escalation depending on the target. Requires a supervisor with greater privileges and attacker control of the log directory/path. Default directories created and owned by the supervisor block that access provided the workload cannot replace an ancestor directory; an existing directory owned by the workload remains vulnerable even with mode `0750`. + +### S2 — One oversized log line stops draining output and can hang the workload + +**Medium · Availability · Reproduced.** Location: [writer.go](../../internal/logging/writer.go#L227), lines 227–234. + +The scanner stops permanently when a token exceeds 1 MiB. Its error is ignored, and the pipe remains open without a reader. Once further output fills the kernel pipe, the child blocks on writes while gonner waits for it to exit. This also affects workloads without `logFile` configured. The [troubleshooting guide](../troubleshooting.md) instead says oversized output is dropped up to the next newline. + +**Reproduction:** Run `head -c 16777216 /dev/zero; printf done > /tmp/completed` as a managed command in an isolated environment. The completion marker was never written; cancelling after 300 ms was required to terminate the blocked command. An `io.Pipe` test independently confirmed the scanner had returned while its producer remained blocked. The hang requires enough output to fill the remaining pipe capacity; lost output occurs as soon as scanning stops. + +### S3 — Multiple log rotations in one second overwrite earlier backups + +**Medium · Data loss · Reproduced with and without compression.** Locations: [writer.go](../../internal/logging/writer.go#L142), lines 142–145 and 198. + +Backup filenames contain only whole seconds. A second rotation within that second reuses the same filename: `Rename` replaces the previous plain backup, or gzip creation truncates the previous `.gz` file. This discards data even when retention is unlimited. + +**Reproduction:** Set `maxSizeMB: 1`, leave `maxBackups: 0`, and write three distinguishable 600 KiB chunks within one wall-clock second. Expected one active log plus two backups; observed one active log plus only one backup. Both `compress: false` and `compress: true` lost the first chunk. + +### S4 — Multi-instance processes lose output when their shared log rotates + +**Medium · Data loss · Reproduced.** Locations: [process.go](../../internal/runner/process.go#L139), lines 139–154; [writer.go](../../internal/logging/writer.go#L138), lines 138–161. + +Every instance opens an independent writer, file descriptor, mutex, and size counter for the same configured log path. When one writer rotates and compresses the file, other writers retain descriptors for the now-unlinked original file. Their writes can succeed while becoming inaccessible through any log filename. + +**Reproduction:** Open two writers for one path with a 1 MiB limit and compression enabled. Have writer A write two 600 KiB chunks, causing one rotation, then have writer B write a unique marker. B returned success, but the marker appeared in neither the active log nor the decompressed backup. This needs only one rotation and is independent of S3. It directly represents `instances: 2` with a shared `logFile`; separate process definitions sharing a path are also affected. + +### S5 — Successful commands can lose their final stdout/stderr + +**Medium · Data loss · Reproduced for main commands and precommands.** Locations: [process.go](../../internal/runner/process.go#L266), lines 266–270 and 356–384. + +Both command paths call `cmd.Wait()` before joining their pipe-reading goroutines. `Wait` closes the pipes when the command exits, even if a scanner is still processing earlier output. The subsequent scanner join cannot recover unread trailing bytes. This ordering violates the documented [StdoutPipe/StderrPipe contract](https://pkg.go.dev/os/exec#Cmd.StdoutPipe). + +**Reproduction:** Use a FIFO log destination whose reader pauses for 300 ms to simulate a slow sink. The child prints a 64 KiB line, sleeps 100 ms, prints `TAIL-MARKER`, and exits successfully. Both main-command and `commandsBefore` tests captured exactly 65,537 bytes and omitted the marker, although both command paths returned success. The line is below the scanner limit, so this is independent of S2. + +### S6 — Backup pruning deletes unrelated files with a matching prefix + +**Medium · Data loss · Reproduced.** Location: [writer.go](../../internal/logging/writer.go#L164), lines 164–188. + +Pruning considers every entry beginning with `.` to be a backup. It does not check the documented timestamp format or ownership of the file. Consequently another process's log or a sibling data file can count toward retention and be deleted. + +**Reproduction:** Create `app.log.1` containing unrelated data. Configure `app.log` with `maxSizeMB: 1` and `maxBackups: 1`, then write two 600 KiB chunks. The first rotation deleted `app.log.1`, even though it was not a gonner timestamp backup. Two explicitly configured logs such as `app.log` and `app.log.1` can therefore interfere with each other. + +### S7 — A log-file write failure silently suppresses stdout as well + +**Medium · Observability/data loss · Reproduced.** Location: [writer.go](../../internal/logging/writer.go#L108), lines 108–121 and 230–232. + +The file sink is written first. If it fails, `Write` returns before sending the same line to stdout, and `LineScanner` discards the error. A full disk therefore removes both the file record and the container's stdout record without an operational error for the lost line. + +**Reproduction:** Inject a directly opened `/dev/full` descriptor into a test writer, keeping stdout backed by an ordinary buffer, and scan a normal line. The file write returned ENOSPC internally; stdout received zero bytes. The test bypassed `NewWriter`, so it did not chmod or otherwise modify the device. + +### S8 — Startup conditions escape cancellation and descendant cleanup + +**Medium · Lifecycle/availability · Reproduced.** Locations: [commandsucceeds.go](../../internal/condition/commandsucceeds.go#L25), lines 25–30; [manager.go](../../internal/runner/manager.go#L52), lines 52–64. + +Command conditions use a context derived from `context.Background()`, and their default cancellation kills only the shell. They neither inherit supervisor cancellation nor use managed process-group cleanup. Condition evaluation also continues through the configuration without checking the manager context. + +**Reproductions:** A condition `(sleep 0.2; printf child > ) & wait`, with its test timeout shortened to 50 ms, returned false at timeout but its descendant subsequently wrote the marker. The production 10-second timeout uses the same cancellation path. Separately, with a normal `commandSucceeds: "sleep 2"` and `shutdownTimeout: "100ms"`, SIGTERM during evaluation took approximately 1.97 seconds to finish. Additional conditions can extend that delay. + +**Impact:** Timed-out conditions can retain resources or perform delayed side effects after evaluation returns. Startup shutdown can exceed the configured process timeout, and unmanaged condition descendants can survive while other workloads keep gonner alive. + +### S9 — Invalid TLS credentials leave startup successful and the listener unusable + +**Medium · Availability · Reproduced.** Location: [server.go](../../internal/health/server.go#L84), lines 84–112. + +Certificate loading happens asynchronously inside `ServeTLS`, after `Start` has returned success. A missing, unreadable, malformed, or mismatched credential can stop that goroutine without returning an error to the CLI. The raw listener also remains bound when certificate loading fails before the server starts serving. + +**Reproduction:** Start the health server with nonempty paths to nonexistent certificate/key files. `Start` returned nil, an HTTPS handshake timed out, and attempting to bind the address again failed with `address already in use`. The application proceeds with its workloads even though its configured probe/status endpoint cannot serve requests. This does not downgrade the server to plaintext; it strands the listener. + +### S10 — PR label text is evaluated as shell syntax in the release workflow + +**Medium · Release correctness / conditional security · Local reproduction.** Location: [release.yml](../../.github/workflows/release.yml#L44), line 44; workflow permissions at lines 10–11. + +The expression `labels='${{ toJson(github.event.pull_request.labels.*.name) }}'` pastes JSON directly into a single-quoted Bash string. JSON serialization does not escape apostrophes for Bash. A label such as `can't-release` breaks parsing; a crafted label can terminate the quoted string and execute shell commands. + +**Reproduction:** Locally substitute the JSON array containing `can't-release` into the workflow statement and run it with Bash: parsing exits with status 2. Substituting the label `x'; printf AUDIT_LABEL_EXECUTED; labels='` prints the marker with exit status 0. Only a harmless local marker was executed; GitHub was not modified. + +**Impact/preconditions:** The release job runs on a merged PR with `contents: write`. Exploitation requires the ability to introduce or attach the relevant repository label to a PR that gets merged. That permission path was not verified, so this is not claimed to be an unauthenticated fork-PR exploit. Ordinary apostrophes still cause a release failure. The mechanism matches GitHub's documented [script-injection boundary](https://docs.github.com/en/actions/reference/security/secure-use#script-injections). + +### S11 — Numeric user IDs select the wrong primary group + +**Medium · Privilege correctness/security · Reproduced across UIDs.** Location: [process.go](../../internal/runner/process.go#L538), lines 538–550. + +`applyCredential` looks up a numeric UID as a username. When that lookup fails, it assigns both UID and GID to the supplied number instead of looking up the account's primary group. This contradicts [the documented default](../security.md#3-drop-privileges) when `group` is omitted. + +**Reproduction:** In an isolated container, define `audituser` with UID 1234 and primary GID 2345, plus an unrelated group with GID 1234. Run the same `id` command under gonner first with `user: "1234"`, then with `user: "audituser"`, without a `group` override. The numeric form produced `uid=1234(audituser) gid=1234(unrelated)`; the named form correctly produced GID 2345. + +**Impact/preconditions:** Numeric-UID deployments can lose access to intended group-owned resources or gain access through an unrelated group. Requires root gonner, an omitted group override, and an account whose UID and primary GID differ. No automatic root-group escalation is claimed. + +### S12 — CI selects an unpatched Go toolchain with reachable security advisories + +**High · Build/dependency security · Scanner and source evidence; no exploit attempted.** Locations: [go.mod](../../go.mod#L3), line 3; [go.yml](../../.github/workflows/go.yml#L18), lines 18–21; [release.yml](../../.github/workflows/release.yml#L29), lines 29–32; [server.go](../../internal/health/server.go#L94), TLS serving. + +Both workflows use `go-version-file: go.mod`, whose directive specifies `go 1.25.0`. A patch-qualified directive selects that exact version, as documented by [setup-go](https://github.com/actions/setup-go/blob/main/docs/advanced-usage.md#using-the-go-version-file-input). It does not merely establish a minimum patched compiler for those workflows. + +**Evidence:** Running `GOTOOLCHAIN=go1.25.0 govulncheck -json ./...` with govulncheck v1.8.0 reported 26 distinct standard-library advisory IDs at symbol level. The optional TLS health server reaches affected TLS operations, including [GO-2026-4870](https://pkg.go.dev/vuln/GO-2026-4870), a TLS 1.3 KeyUpdate connection deadlock fixed in Go 1.25.9, and [GO-2026-6090](https://pkg.go.dev/vuln/GO-2026-6090), excessive post-handshake KeyUpdate processing fixed in Go 1.25.13. Their server-side exposure precedes application bearer-token authorization. + +**Qualification:** The 26 scanner results are not 26 proven exploits. Some need unsupported platforms or unused features; the appendix preserves the distinction. All symbol-level results were in the standard library, not third-party application modules. Published artifacts were not downloaded, so their embedded toolchain is not established. The defect here is the checked-in CI toolchain selection and the resulting exposure when vulnerable builds enable the affected features. The local Go 1.26.0 installation also returned 18 advisory IDs and is separately unpatched. + +### S13 — Health requests race with manager startup when reading uptime + +**Low · Concurrency/observability · Race detector reproduction.** Locations: [manager.go](../../internal/runner/manager.go#L44), lines 44 and 229–232; [run.go](../../cmd/gonner/cmd/run.go#L86), lines 86–104. + +`Run` writes the multiword `startedAt` value without synchronization, while `Uptime` reads it without synchronization. The CLI starts accepting health requests before invoking `Run`; `/status` and `/metrics` can therefore read it concurrently with startup. + +**Reproduction:** Start a goroutine repeatedly calling `Uptime` on a new manager, then call `Run` with a simple successful command. `go test -race` reported the write at `manager.go:44` racing with the read at `manager.go:231`. Existing health tests never run their manager concurrently. Impact is an actual data race and potentially invalid uptime reporting around startup; no process crash was demonstrated. + +## Spec + +### B1 — Failed or transitively skipped dependencies can leave startup blocked forever + +**High · Lifecycle · Reproduced.** Location: [manager.go](../../internal/runner/manager.go#L131), lines 131–144. + +Parallel dependency waiting observes only readiness and global cancellation. It never observes a dependency's terminal failure. In the skipped-dependency path, the process is marked skipped without running its lifecycle or closing its `Done` channel. Downstream dependants can therefore wait forever even though nothing can make their dependency ready. + +**Reproductions:** Define A with `commandsBefore: [{"command":"exit 7"}]`, and B with `dependsOn: ["A"]`. A becomes failed, B remains pending, and `Manager.Run` stays blocked with no live child. Separately, make A's condition false, B depend on A, and C depend on B. B becomes skipped but C remains pending forever. Both cases stopped only after explicit cancellation. The [architecture](../architecture.md#concurrency-model) describes readiness/done coordination and lifecycle completion. + +### B2 — Failure of a critical precommand does not shut down the other processes + +**High · Lifecycle · Reproduced.** Location: [process.go](../../internal/runner/process.go#L162), lines 162–171, compared with the main-command critical path at 197–203. + +A failed `commandsBefore` command marks its process failed and returns nil, bypassing critical cancellation. The [configuration reference](../configuration.md#run--process-definitions) promises full shutdown for critical failure, and [troubleshooting](../troubleshooting.md#container-exits-immediately) explicitly lists failed precommands as such a cause. + +**Reproduction:** Configure a critical process with precommand `exit 7` and another process running `exec sleep 30`. The critical process became failed, but `IsShuttingDown()` remained false and the background process continued running. This can leave a container serving from a partially initialized application after a required migration or setup step fails. + +### B3 — Multi-instance startup only waits for instance zero + +**Medium · Startup ordering · Reproduced in both modes.** Location: [manager.go](../../internal/runner/manager.go#L112), lines 112–120 and 139–144. + +Both startup paths inspect `procs[0]` instead of accepting any ready instance. [The specification](../configuration.md#multi-instance-processes) says any one running instance satisfies readiness for dependencies. + +**Reproduction:** Configure two instances whose precommand races to create a marker directory: the winner completes, the loser waits. When instance 1 wins, it runs while instance 0 remains starting; the dependent process remains pending despite a healthy dependency instance. This was observed in both parallel dependency waiting and sequential startup. A permanently blocked first instance prevents useful progress indefinitely. + +### B4 — Backoff downtime is mistaken for a stable running period + +**Medium · Restart policy · Reproduced in a real crash loop.** Location: [backoff.go](../../internal/runner/backoff.go#L34), lines 34–43. + +`RecordStart` resets the counter using time elapsed since the previous start. That interval includes the backoff wait itself. The [specified stability reset](../configuration.md#backoff) requires the process to have stayed running longer than `maxDelay`. + +**Reproduction:** Run `exit 7` with initial delay 10 ms, maximum 100 ms, multiplier 2, and 24 allowed retries. Observed start intervals included approximately `13, 20, 39, 74, 93, 101, 11, 22, 40, 79, 110, 12 ms`. Every process exited immediately, yet the delay repeatedly reset after reaching the cap. Persistent failures therefore periodically return to aggressive retry rates instead of remaining backed off. + +### B5 — A crashed process is reported as running throughout its restart delay + +**Medium · Status/metrics correctness · Reproduced.** Locations: [process.go](../../internal/runner/process.go#L389), lines 389–394 and 211–225; [manager.go](../../internal/runner/manager.go#L197), lines 197–200, aggregation of running instances. + +After a main command exits, the command pointer is cleared but the running state is retained unless group cleanup happened to change it. A noncritical process waiting to restart can consequently remain `running` while it has no PID. The [metrics contract](../operations.md#metrics-prometheus) describes currently running instances and whether a process is up. + +**Reproduction:** Run `exit 7` with automatic restart and a five-second initial delay. During the delay, status reported `status: "running"`, `runningInstances: 1`, and no PID. This can hide an outage from status consumers and `gonner_process_up` alerts. Critical processes currently take the shutdown path rather than automatic restart, so this reproduction is not a claim about critical readiness during backoff. + +### B6 — Restart counts include attempts that never happen + +**Low · Metrics correctness · Reproduced.** Location: [process.go](../../internal/runner/process.go#L211), lines 211–218. + +The counter increments before waiting and before checking the retry limit. It therefore includes the final rejected attempt and also counts a restart even if shutdown cancels its backoff. This differs from [the documented aggregate restart count](../operations.md#metrics-prometheus). + +**Reproduction:** Allow 24 retries for an immediately failing command. There were 24 actual restarts after the initial start, but the final reported counter was 25. During the very first backoff interval, it already reported one restart before a second command had started. + +### B7 — The PID 1 reaper still steals child exit statuses + +**High · Container lifecycle · Reproduced with gonner as PID 1.** Locations: [signals.go](../../internal/runner/signals.go#L118), lines 118–125; [process.go](../../internal/runner/process.go#L266), lines 266–280 and 360–373. + +The reaper's `wait4` and subsequent map store are not synchronized with `exec.Cmd.Wait` and its single map lookup. A main-command waiter can receive ECHILD before the status is stored. Precommands do not consult the map at all. This contradicts the [documented guarantee](../architecture.md#pid-1-reaping-coordination) that reaping is coordinated with command waiting. + +**Reproductions:** In a fresh Alpine container with gonner as the entrypoint, configure 32 instances, each with 20 precommands containing only `true`, followed by an ordinary main command. **28 of 32 instances** falsely failed in precommands with `waitid: no child processes`. Separately, run 300 instances of the main command `true`: three trials produced **1, 2, and 0** false main-command failures, respectively. A log entry from the reaper can follow the failed lookup, so the issue is timing-sensitive. + +**Impact:** Successful setup commands can prevent service startup; successful main commands can be restarted or treated as critical failures. Ordinary non-PID-1 tests do not activate the competing reaper. These are two manifestations of the same exit-status ownership problem. + +### B8 — A fatal critical-process failure exits gonner with success + +**High · Supervisor integration · Reproduced through the CLI.** Locations: [process.go](../../internal/runner/process.go#L197), lines 197–203; [manager.go](../../internal/runner/manager.go#L153), lines 153–157; [main.go](../../cmd/gonner/main.go#L11). + +Critical failure cancels the manager but returns nil. Shutdown of the other processes also returns nil, so the errgroup and CLI report success. The documented [systemd deployment](../deployment.md#systemd) uses `Restart=on-failure`, which will not restart a supervisor that exits with code 0. + +**Reproduction:** Run this configuration and inspect the gonner exit code: + +```json +{"run":[{"name":"essential","command":"exit 42","critical":true}]} +``` + +Gonner logged `CRITICAL ... triggering full shutdown` and then exited with **0**. The desired distinction is fatal workload failure versus operator-requested clean shutdown; preserving the exact child's exit code is a separate design choice. Docker `on-failure` policies are affected for the same reason. This is distinct from B2: main-command critical shutdown happens here, but its result is misreported. + +### B9 — Readiness succeeds before critical processes exist, and after they are skipped + +**Medium · Traffic routing/readiness · Reproduced over HTTP.** Locations: [run.go](../../cmd/gonner/cmd/run.go#L86), lines 86–104; [manager.go](../../internal/runner/manager.go#L52), lines 52–80 and 260–277. + +The health server starts before the manager evaluates conditions or constructs process instances. `Ready` checks only constructed instances. An empty instance list therefore looks like a configuration without critical processes. Critical configurations removed by false conditions remain absent from readiness checks permanently. [The public contract](../operations.md#get-ready) says all configured critical processes must have a running instance. + +**Reproductions:** Configure a critical sleeper gated by `commandSucceeds: "sleep 2"`, plus a noncritical sleeper, and query the loopback endpoint during condition evaluation. `/ready` returned **200 ready** while `/status` had `processes: null`. Change the critical condition to an unset environment variable: only the noncritical process started, but `/ready` still returned 200. Traffic can therefore be routed while a required service is absent. Skipped configurations are also missing from detailed status rather than appearing as skipped. + +### B10 — The documented container capability set prevents shutdown of dropped-UID children + +**Medium · Deployment/lifecycle · Reproduced in a container.** Locations: [deployment.md](../deployment.md#kubernetes), capability example at lines 110–114; [process.go](../../internal/runner/process.go#L422), lines 422–428. + +The Kubernetes example drops all capabilities and adds only `CHOWN`, `SETUID`, and `SETGID` for privilege dropping. After a child changes UID, the supervisor also needs permission to signal that other UID. Without `CAP_KILL`, group signaling returns EPERM; `stopProcessGroup` returns immediately, and the custom command cancellation cannot terminate the child. `cmd.Wait` can then block beyond the configured timeout. + +**Reproduction:** Run gonner in Docker with `--cap-drop ALL --cap-add CHOWN --cap-add SETUID --cap-add SETGID`, a child `exec sleep 30` under UID 65534, and `shutdownTimeout: "100ms"`. Send SIGTERM to gonner. After 600 ms the container was still running, having logged the shutdown request without stopping the child. A daemon-issued SIGKILL cleaned up the test container. This concerns the supplied deployment recipe and signal-permission handling, not containers retaining their normal capabilities. + +### B11 — The documented bare-port condition format always fails + +**Low · Conditional startup · Reproduced.** Location: [portopen.go](../../internal/condition/portopen.go#L18), lines 18–32 and 39. + +The [condition reference](../configuration.md#built-in-condition-types) permits `[host:]port` with localhost as the default. The constructor only adds a host if the string begins with `:`, so a bare numeric port reaches `net.DialTimeout` without the required address separator. The error becomes a false condition, silently skipping the workload. + +**Reproduction:** Listen on an ephemeral loopback TCP port. Use its decimal number as the `portOpen` value: the process is skipped. Use the same number with a leading colon: the process runs. The listener and every other configuration field were unchanged. + +### B12 — An explicit status port of 8089 loses to the environment override + +**Low · CLI correctness · Reproduced.** Location: [status.go](../../cmd/gonner/cmd/status.go#L121), lines 121–131. + +The resolver infers whether `--port` was supplied by comparing its value with the default. Supplying the default explicitly is indistinguishable from omitting the flag, violating [the documented CLI > environment precedence](../configuration.md#environment-variable-overrides). + +**Reproduction:** Serve a distinctive fake `/status` response on an ephemeral loopback port, export that number as `GONNER_HEALTH_PORT`, and run `gonner status --port 8089`. The command successfully printed the fake server's response from the environment-selected port. This can query the wrong instance despite an explicit operator choice. + +## Vulnerability scan appendix + +The following are the **26 distinct symbol-level advisory IDs** returned using Go 1.25.0. Fixed versions are the scanner's Go 1.25-series fixes, not a claim that that series is currently supported. To reproduce the scan without adding application dependencies, install `golang.org/x/vuln/cmd/govulncheck@v1.8.0` into a separate tools directory, then invoke its binary from the repository with `GOTOOLCHAIN=go1.25.0` and arguments `-json ./...`. + +| Advisory | Area reported | Fixed in Go 1.25 series | +|---|---|---| +| [GO-2025-4007](https://pkg.go.dev/vuln/GO-2025-4007) | X.509 name-constraint complexity | 1.25.3 | +| [GO-2025-4008](https://pkg.go.dev/vuln/GO-2025-4008) | TLS ALPN error content | 1.25.2 | +| [GO-2025-4009](https://pkg.go.dev/vuln/GO-2025-4009) | PEM parsing complexity | 1.25.2 | +| [GO-2025-4010](https://pkg.go.dev/vuln/GO-2025-4010) | URL IPv6 hostname validation | 1.25.2 | +| [GO-2025-4011](https://pkg.go.dev/vuln/GO-2025-4011) | ASN.1 memory exhaustion | 1.25.2 | +| [GO-2025-4012](https://pkg.go.dev/vuln/GO-2025-4012) | HTTP cookie parsing limits | 1.25.2 | +| [GO-2025-4013](https://pkg.go.dev/vuln/GO-2025-4013) | X.509 DSA certificate panic | 1.25.2 | +| [GO-2025-4155](https://pkg.go.dev/vuln/GO-2025-4155) | X.509 error formatting resources | 1.25.5 | +| [GO-2025-4175](https://pkg.go.dev/vuln/GO-2025-4175) | X.509 wildcard name constraints | 1.25.5 | +| [GO-2026-4337](https://pkg.go.dev/vuln/GO-2026-4337) | TLS session resumption | 1.25.7 | +| [GO-2026-4340](https://pkg.go.dev/vuln/GO-2026-4340) | TLS handshake encryption level | 1.25.6 | +| [GO-2026-4601](https://pkg.go.dev/vuln/GO-2026-4601) | URL IPv6 literal parsing | 1.25.8 | +| [GO-2026-4602](https://pkg.go.dev/vuln/GO-2026-4602) | `os.Root` file-info escape | 1.25.8 | +| [GO-2026-4870](https://pkg.go.dev/vuln/GO-2026-4870) | TLS 1.3 KeyUpdate deadlock | 1.25.9 | +| [GO-2026-4918](https://pkg.go.dev/vuln/GO-2026-4918) | HTTP/2 client settings loop | 1.25.10 | +| [GO-2026-4946](https://pkg.go.dev/vuln/GO-2026-4946) | X.509 policy-validation complexity | 1.25.9 | +| [GO-2026-4947](https://pkg.go.dev/vuln/GO-2026-4947) | X.509 chain-building work | 1.25.9 | +| [GO-2026-4971](https://pkg.go.dev/vuln/GO-2026-4971) | Windows network NUL-byte panic | 1.25.10 | +| [GO-2026-5026](https://pkg.go.dev/vuln/GO-2026-5026) | IDNA/Punycode validation | 1.25.13 | +| [GO-2026-5037](https://pkg.go.dev/vuln/GO-2026-5037) | X.509 hostname parsing complexity | 1.25.11 | +| [GO-2026-5039](https://pkg.go.dev/vuln/GO-2026-5039) | Text-protocol error escaping | 1.25.11 | +| [GO-2026-5856](https://pkg.go.dev/vuln/GO-2026-5856) | TLS ECH privacy behavior | 1.25.12 | +| [GO-2026-5972](https://pkg.go.dev/vuln/GO-2026-5972) | ASN.1 recursion depth | 1.25.13 | +| [GO-2026-6089](https://pkg.go.dev/vuln/GO-2026-6089) | Unencrypted HTTP/2 header timeout | 1.25.13 | +| [GO-2026-6090](https://pkg.go.dev/vuln/GO-2026-6090) | TLS post-handshake message processing | 1.25.13 | +| [GO-2026-6218](https://pkg.go.dev/vuln/GO-2026-6218) | URL path-resolution complexity | 1.25.13 | + +Manual applicability limits: gonner's release matrix does not include Windows; it does not use `os.Root`; it does not configure ECH; and it does not explicitly enable unencrypted HTTP/2. The corresponding scanner matches should not be promoted to confirmed application exposures without further evidence. Other advisories can depend on optional TLS, the outbound status client, certificates, or parsing paths. S12 is counted as **one build-security finding**, not 26 separate application defects. + +## Review disposition + +**Standards:** 13 findings (2 high, 10 medium, 1 low); greatest security impact is S1's conditional privileged-file modification, alongside S12's vulnerable CI toolchain. **Spec:** 12 findings (4 high, 5 medium, 3 low); B7 directly undermines PID 1 startup, while B1/B2/B8 break failure handling. All findings remain open for user review; no fixes, issue-tracker entries, PRs, or separate implementation plans were created. + +## Resolution matrix 2026-09-26 + +The evidence above describes the audited baseline and is retained unchanged. The remediation is implemented on `fix/codebase-audit-2026-09-26` for v0.1.0. Each finding below has a permanent regression check. This matrix records implementation validation; publication status is tracked in GitHub Releases. + +| Finding | Resolution | Permanent regression coverage | +|---|---|---| +| S1 | Verified directory descriptors, no-follow leaf opens, regular-file/single-link/ownership checks, descriptor chmod, ACL checks; the same protection covers PID files. | [safefile tests](../../internal/safefile/directory_test.go): `TestRejectLeafSymlinkHardlinkFIFOWithoutTouchingVictim`, `TestRejectWritableAncestorAndAllowTrustedSymlinks`, `TestStickyAncestorAndDescriptorSurviveRename`; platform ACL tests; [Docker](../../internal/integration/docker_linux_test.go) `/files` runs as root against workload-owned paths. | +| S2 | Stream arbitrarily long output in bounded 64 KiB chunks without discarding raw bytes. | [logging tests](../../internal/logging/regression_test.go): `TestOversizedAndUnterminatedOutputIsByteExact`; [execution tests](../../internal/execution/service_test.go): `TestLargeOutputDrainsWithoutDeadlock`. | +| S3 | Nanosecond/random backup names and exclusive publication prevent overwrites during rapid rotation. | Logging: `TestRapidRotationAndSharedSinkPreserveEveryByte`, with plain and compressed backups. | +| S4 | Shared path registry owns one descriptor, mutex, byte counter and reference count; conflicting settings fail preflight. | Logging: `TestRapidRotationAndSharedSinkPreserveEveryByte`, `TestSharedPathRejectsConflictingSettings`. | +| S5 | Explicit pipe descriptors outlive the sole child wait; drain to EOF after group cleanup. | Execution: `TestWaitDoesNotCloseUndrainedPipes` deliberately delays reading until after child exit. | +| S6 | Prune only reserved-format regular backups, excluding active configured log paths. | Logging: `TestPruningPreservesUnrelatedLegacyAndActivePaths`. | +| S7 | Both destinations are attempted; errors are rate-limited, future writes retry, and rotation failures retain a usable descriptor. | Logging: `TestFileErrorStillWritesConsoleAndRecovers`, `TestRotationFailureKeepsOpenSinkUsable`. | +| S8 | Context-aware conditions use the shared executor; command timeout/cancellation and normal completion clean up the process group. | [condition tests](../../internal/condition/new_conditions_test.go): `TestCommandConditionCancellationAndTimeout`; Docker `/conditions` checks normal and cancelled probes plus pre/main descendants. | +| S9 | Load certificates synchronously before binding; serving failures cancel supervision and propagate to exit status. | [health tests](../../internal/health/regression_test.go): `TestTLSValidationIsSynchronousAndDoesNotLeakListener`, `TestUnexpectedServeFailureIsReported`; [CLI tests](../../cmd/gonner/cmd/shutdown_linux_test.go): `TestInvalidTLSPreventsWorkloadStartup`. | +| S10 | Pass labels as environment JSON and choose exact labels in Python, with major > minor > patch precedence. | [release tests](../../scripts/test_release_bump.py): exact membership, priority, invalid shape and subprocess injection payload cases. | +| S11 | Numeric UID lookup uses the account's primary GID; unknown UID requires a group; validate IDs and clear supplementary groups. | Execution: `TestCredentialRejectsInvalidAndUnknownIDs`; Docker `/credentials` checks UID 2001 / GID 3001, override GID, unknown numeric UID and empty supplementary groups. | +| S12 | Require Go 1.26.7; pin govulncheck v1.8.0 and gate publication on source and all release-binary scans. | [CI](../../.github/workflows/go.yml), [release workflow](../../.github/workflows/release.yml), [artifact checks](../../scripts/check-artifacts.sh). | +| S13 | Guard manager start time and report zero uptime before Run. | [runner tests](../../internal/runner/regression_test.go): `TestReadinessIncludesPendingAndSkippedCriticalAndUptimeRace`, exercised with `-race`. | +| B1 | Stable first-start gates and exactly-once lifecycle completion propagate unavailable dependencies through the graph. | Runner: `TestFailedAndSkippedDependencyChainsTerminate`, `TestCriticalUnavailableDependencyIsFatal`, `TestReadyChannelSurvivesActualRestarts`. | +| B2 | Critical required-precommand failure cancels all work and returns an error. | Runner: `TestCriticalPrecommandFailureCancelsAndReturnsError` also checks `continueOnError`. | +| B3 | Both startup modes consider every instance; dependencies persist after a successful one-shot start. | Runner: `TestAnyInstanceStartGateAndShortLivedPrerequisite`, `TestDependencyRunsWhenInstanceZeroCannotStart`; [config tests](../../internal/config/validation_test.go): `TestSequentialRejectsForwardDependencies`. | +| B4 | Reset escalation from actual runtime at exit; exclude downtime and cleanup. | Runner: `TestBackoffExcludesDowntimeAndCleanup` uses virtual time; [backoff tests](../../internal/runner/backoff_test.go): `TestBackoff_RecordExit_ResetAfterStability`. | +| B5 | Clear PID/running status immediately after the direct-child wait; show `starting` during retry backoff. | Runner: `TestBackoffStatePIDAndCancelledRetryCount`. | +| B6 | Retry budget counts extra attempts; public restarts count successful launches after the first successful launch. | Runner: `TestRetryLimitCountsOnlySuccessfulRelaunches`, `TestBackoffStatePIDAndCancelledRetryCount`. | +| B7 | Managed PID registry synchronizes spawn/registration with PID-specific orphan reaping; remove wildcard waits and status cache. | Docker `/stress`: 3,584 short-lived command/probe/precommand shells, explicit nonzero exit checks and no remaining children; execution cancellation tests. | +| B8 | Permanent failures determine eventual exit 1; critical/supervisor failures cancel all; unrelated noncritical work continues. | Runner: `TestCriticalAndNoncriticalPermanentExitsReturnError`, `TestFailedAndSkippedDependencyChainsTerminate`; CLI `TestRunShutdown/critical_exit`; execution `TestLaterCancellationPreservesObservedExitFailure`. | +| B9 | Build every configured instance before Run; pending/skipped critical processes remain unready and visible. | Health: `TestReadyAndStatusIncludePendingAndSkippedCritical`; runner readiness/uptime test checks configured counts. | +| B10 | Check capabilities before any probe or command; include KILL in the deployment example and propagate signal errors. | Docker `/credentials` verifies cross-UID shutdown; `/missing-kill` verifies fail-fast startup without any workload or condition execution. | +| B11 | Normalize bare and colon-prefixed ports to loopback and use context-aware dialing. | Condition: `TestPortOpenBareAndColonPort`, including hostname and cancelled-context cases. | +| B12 | Resolve status port using Cobra flag presence, so explicit 8089 wins over the environment. | [CLI status tests](../../cmd/gonner/cmd/status_test.go): `TestExplicitDefaultStatusPortOverridesEnvironment`. | + +### Validation and remaining platform checks + +- Linux: `go test -race -count=1 -cover ./...`, `go vet ./...`, and the Python release-helper tests passed. +- Isolated Docker tests passed with a read-only filesystem, no network, temporary `/tmp`, explicit capabilities and the test process actually running as PID 1. They cover fast-exit stress, group cleanup, privilege drop, cross-UID signaling, missing capabilities and unsafe paths. Containers are removed after testing. +- Go 1.26.7 source scanning and binary scanning of Linux/Darwin × amd64/arm64 static builds with govulncheck v1.8.0 reported no vulnerabilities. +- Native macOS execution is unavailable in this Linux workspace. macOS tests, including the Darwin ACL regression, are configured in both PR CI and the release verification matrix; release publication depends on that matrix passing. Cross-compilation succeeded for both Darwin architectures. +- [Operations migration notes](../operations.md#upgrade-from-the-audited-baseline) document strict paths, numeric UID/group changes, capabilities, exit statuses, readiness, dependency semantics, console chunking, legacy backups and the context-aware condition API. No configuration keys, HTTP keys or lifecycle states were added. diff --git a/docs/configuration.md b/docs/configuration.md index f00eb75..35867bf 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -81,12 +81,12 @@ The health endpoint is **opt-in**. Set `health.port` to enable it. | `bindAddr` | string | `"0.0.0.0"` | Bind address. **Set to `"127.0.0.1"` for localhost-only.** | | `authToken` | string | — | If set, `/status` and `/metrics` require `Authorization: Bearer `. `/health` is always public. May be overridden by `GONNER_HEALTH_TOKEN` env var (preferred for secrets). | | `metrics` | bool | `false` | Enable Prometheus-compatible `/metrics`. | -| `tls` | object | — | TLS settings — `{ "certFile": "...", "keyFile": "..." }`. When set, the server speaks HTTPS with a minimum protocol version of TLS 1.2. | +| `tls` | object | — | TLS settings — `{ "certFile": "...", "keyFile": "..." }`. Certificates are loaded before binding or starting workloads; invalid or mismatched keys abort startup. When set, the server speaks HTTPS with a minimum protocol version of TLS 1.2. | Endpoints: - `GET /health` — unauthenticated **liveness** probe; returns `200 {"status":"healthy"}` while running, `503 {"status":"shutting_down"}` during shutdown. Use as a Docker `HEALTHCHECK` or Kubernetes liveness probe. -- `GET /ready` — unauthenticated **readiness** probe; returns `200 {"status":"ready"}` only when gonner is not shutting down **and** every `critical` process has a running instance, otherwise `503 {"status":"not_ready"}`. Use as a Kubernetes readiness probe. If no `critical` processes are defined, readiness tracks liveness. +- `GET /ready` — unauthenticated **readiness** probe; returns `200 {"status":"ready"}` only when gonner is not shutting down **and** every `critical` process has a running instance, otherwise `503 {"status":"not_ready"}`. Use as a Kubernetes readiness probe. Pending and condition-skipped critical processes return 503 and remain visible in status and metrics with their configured instance count. If no `critical` processes are defined, readiness tracks liveness. - `GET /status` — full per-process detail (uptime, PID, restart count, etc.). Authenticated if `authToken` is set. - `GET /metrics` — Prometheus text format. Only if `metrics: true`. Authenticated if `authToken` is set. @@ -102,17 +102,17 @@ Each entry defines a managed process. | `command` | string | required | Executed via `sh -c`. | | `workDir` | string | inherit | Working directory. | | `env` | object | — | Extra environment variables: `{ "KEY": "VALUE" }`. Merged on top of gonner's environment. | -| `user` | string | — | Username or numeric UID to drop to before exec. **Requires gonner to start as root.** | +| `user` | string | — | Username or numeric UID to drop to before exec. Uses the account's primary GID; unknown numeric UIDs require an explicit `group`. Requires root on macOS, or appropriate Linux capabilities. | | `group` | string | — | Group name or GID. Requires `user`. | -| `logFile` | string | — | Path to append raw process output. Parents are created (`0o750`). | +| `logFile` | string | — | Path to append raw process output. Parents are created (`0o750`) and must satisfy the [safe path policy](security.md#log-files). | | `logFileMode` | int | `0o600` | POSIX mode bits for the log file (e.g. `0o600`, `0o640`). | | `logRotate` | object | — | Size-based rotation. See [`logRotate`](#logrotate). | -| `autoRestart` | bool | `false` | Restart on non-zero exit (and on zero exit too if true? — no: clean exit ends the lifecycle). | -| `maxRetries` | int | `0` | Cap on restarts. `0` = unlimited (only applies if `autoRestart` is true). | +| `autoRestart` | bool | `false` | Restart on non-zero exit or failed launch. Clean exit ends the lifecycle. | +| `maxRetries` | int | `0` | Cap on extra launch attempts (failed launches also consume the budget). `0` = unlimited (only applies if `autoRestart` is true). | | `backoff` | object | defaults | See [`backoff`](#backoff). | | `instances` | int | `1` | Number of identical copies. Each instance has its own PID, log prefix, and restart counter. | -| `critical` | bool | `false` | Unexpected exit triggers full shutdown of the entire gonner process tree. | -| `dependsOn` | []string | `[]` | Process names that must be Running before this one starts. | +| `critical` | bool | `false` | Failed launch, non-zero exit or failed required precommand triggers full shutdown and exit 1. | +| `dependsOn` | []string | `[]` | Process names for which any instance must have started successfully at least once. | | `whenAll` | array | — | See [Conditions](#conditions-whenall--whenany). | | `whenAny` | array | — | See [Conditions](#conditions-whenall--whenany). | | `commandsBefore` | array | `[]` | Pre-start commands. See [`commandsBefore`](#commandsbefore). | @@ -125,7 +125,7 @@ A process moves through: `pending → starting → running → stopping → stop ### Multi-instance processes -When `instances > 1`, gonner spawns N goroutines, each with its own PID and restart counter. Log prefixes are suffixed with `/INDEX` (e.g. `[queue/3]`). A process is considered "running" for `dependsOn` resolution as soon as **any one** instance reaches `running`. +When `instances > 1`, gonner spawns N goroutines, each with its own PID and restart counter. Log prefixes are suffixed with `/INDEX` (e.g. `[queue/3]`). A dependency is satisfied permanently as soon as **any one** instance starts successfully, including a short-lived prerequisite. This is an initial-start gate, not ongoing availability monitoring. If every instance finishes without starting, waiting dependants become `skipped`; a critical dependant instead fails and stops the manager. Sequential mode uses the same any-instance gate, follows config order, and rejects forward `dependsOn` references. --- @@ -139,7 +139,7 @@ Exponential backoff applied between restart attempts. | `maxDelay` | duration | `"30s"` | Cap on the delay. | | `multiplier` | float | `2.0` | Applied after each restart. | -Jitter of ±10% is added to each delay to avoid thundering herds. The counter resets after the process has stayed running for longer than `maxDelay`. +Jitter of ±10% is added to each delay to avoid thundering herds. The counter resets when a process exits after actually running longer than `maxDelay`; restart waits and output draining do not count. During backoff the process is `starting`, with no PID or running instance. Public `restarts` counts successful launches after the initial successful launch. --- @@ -153,7 +153,7 @@ Sequentially executed pre-start commands. Their stdout/stderr stream to the same | `workDir` | string | parent's `workDir` | Working directory. | | `continueOnError` | bool | `false` | If `false`, a non-zero exit marks the process `failed` and the main command never starts. If `true`, a warning is logged and gonner continues. | -`commandsBefore` inherits the parent process's `env`, `user`, and `group`. +`commandsBefore` inherits the parent process's `env`, `user`, and `group`. A required precommand failure on a critical process triggers full shutdown. `continueOnError: true` keeps its existing meaning. --- @@ -167,7 +167,7 @@ Size-based log rotation. Applies only when `logFile` is set. | `maxBackups` | int | `0` (unlimited) | Max number of rotated files to retain. | | `compress` | bool | `false` | Gzip rotated files. | -Rotated files are named `.` (e.g. `app.log.20260310T120000Z`); compressed files get a `.gz` suffix. Backup pruning keeps the most recent `maxBackups` files lexicographically. +Rotated files use the reserved format `.gonner-YYYYMMDDTHHMMSS.nnnnnnnnnZ-<32 lowercase hex digits>.log`, optionally suffixed `.gz`. Names include nanoseconds and randomness and are published exclusively. Pruning only touches regular backups in this format, excluding configured active log paths. Legacy backups and other prefixed files remain untouched. Processes sharing a canonical log path share one sink and must specify identical mode and rotation settings. For long-term storage and search, consider shipping logs to your centralized logging stack instead of relying on rotation alone. @@ -175,7 +175,7 @@ For long-term storage and search, consider shipping logs to your centralized log ## Conditions: `whenAll` / `whenAny` -Conditions decide whether a process starts. They evaluate **once**, at startup (after env interpolation). +Conditions decide whether a process starts. They evaluate **once**, at startup, after configuration, credential and log-path preflight. Command conditions inherit their process's environment, working directory and credentials. All conditions receive the manager context; command probes have a 10s timeout and their whole process group is cleaned up on exit, timeout or cancellation. - `whenAll`: every condition must be true. - `whenAny`: at least one condition must be true. @@ -214,7 +214,7 @@ typos are caught before deployment. ### Adding custom conditions -Custom condition types can be registered programmatically (`condition.Register(name, factory)`) when embedding gonner as a library. +Custom condition types can be registered programmatically (`condition.Register(name, factory)`) when embedding gonner as a library. Implement `Evaluate(context.Context) (bool, error)` and respect cancellation. Evaluation helpers also take the context as their first argument. --- @@ -245,7 +245,7 @@ Queries a running gonner instance's `/status` endpoint and prints a formatted ta | Flag | Default | Description | |---|---|---| | `--host` | `127.0.0.1` | Host to query. | -| `--port`, `-p` | `8089` | Port to query (overridden by `GONNER_HEALTH_PORT`). | +| `--port`, `-p` | `8089` | Port to query. An explicitly supplied flag always overrides `GONNER_HEALTH_PORT`, including `--port 8089`. | | `--token`, `-t` | from `GONNER_HEALTH_TOKEN` | Bearer token for authenticated endpoints. | | `--tls` | `false` | Query over HTTPS instead of HTTP. | | `--insecure` | `false` | Skip TLS certificate verification (use with `--tls` for self-signed certs). | @@ -274,3 +274,7 @@ Prints version, commit hash, build date, and Go version. | `XDG_CONFIG_HOME` | Affects discovery priority 3. | Order of precedence for these settings: **CLI flag > environment variable > config file > built-in default**. + +## Exit status + +Gonner returns 1 for permanent process failures, exhausted retries, unavailable dependencies, and supervisor failures. A noncritical failure lets unrelated work continue and contributes to the eventual failure exit; a critical failure cancels all work. An intentional false condition alone is not a failure, including on a critical process, but leaves that critical process unready. Successful one-shot completion and operator shutdown return 0 unless a permanent failure was already recorded. diff --git a/docs/deployment.md b/docs/deployment.md index 356d3ff..c8918b9 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -30,7 +30,7 @@ ENTRYPOINT ["gonner"] ### Multi-stage build ```dockerfile -FROM golang:1.25-alpine AS build +FROM golang:1.26.7-alpine AS build WORKDIR /src COPY . . RUN CGO_ENABLED=0 go build -ldflags '-s -w' -o /out/gonner ./cmd/gonner @@ -111,7 +111,7 @@ spec: runAsNonRoot: false # gonner needs root for the zombie reaper if PID 1 capabilities: drop: ["ALL"] - add: ["CHOWN", "SETUID", "SETGID"] # if you use user/group privilege drop + add: ["CHOWN", "SETUID", "SETGID", "KILL"] # if you use user/group privilege drop ``` Match `terminationGracePeriodSeconds` to (`shutdownTimeout` + buffer). diff --git a/docs/operations.md b/docs/operations.md index 196d98d..a063204 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -132,12 +132,28 @@ Gonner's own operational events (startup, condition results, restarts, shutdown) ### Log lifecycle -- The line scanner buffers up to 1 MiB per line. -- Each `Write` is atomic per Writer (mutex-protected). -- Rotation closes and renames the file, optionally gzips it, prunes excess backups, then reopens. +- Output is streamed in chunks of at most 64 KiB. Oversized lines continue draining; every console chunk gets a prefix and newline. Raw files preserve every byte, including an absent final newline. +- Writers sharing a canonical path use one file descriptor, mutex and rotation counter. +- Rotation exclusively reserves a unique backup name and keeps the old descriptor usable if replacement fails. Compression uses a temporary file and publishes only after successful completion. +- Pruning ignores legacy backups, unrelated prefixes, special files and active configured log paths. +- A file error does not suppress console output or stop draining. Writes retry on subsequent chunks; diagnostics report the first error, then at most once per minute until recovery. +- Readers drain to EOF after a child exits. During shutdown, an escaped descendant retaining a pipe can cause forced closure at the shutdown deadline, with an explicit truncation diagnostic. ### Shipping logs For Docker, the docker logging driver receives both gonner's stdout and stderr — no extra wiring is needed for `docker logs`. For Kubernetes, the kubelet captures the same streams. -If you want to forward logs from the configured `logFile`, mount it onto a sidecar (Fluent Bit, Vector) and have that read & rotate independently. In that case set `logRotate: null` on gonner. +If you want to forward logs from the configured `logFile`, mount it onto a sidecar (Fluent Bit, Vector) and have that read the files without replacing gonner's active path. Keep rotation under gonner's ownership; use console collection if the collector needs to control rotation. + +## Upgrade from the audited baseline + +See [the audit resolution matrix](audits/2026-09-25-codebase-audit.md#resolution-matrix-2026-09-26) for regression coverage. Existing configuration keys, HTTP fields and lifecycle state names are unchanged. Review these behavior changes before upgrading: + +- Build with Go 1.26.7 or newer. CI scans source and all four static release artifacts with govulncheck v1.8.0 before publication. +- Keep log and PID paths owned by root or the supervisor UID, outside workload-controlled directories. Group/other writable paths are rejected except protected sticky ancestors. Log leaves must be single-link regular files; symlinks and devices are rejected. Use mode 0600 or 0640. +- Give unknown numeric users an explicit group. Known numeric users now use their real primary GID. Supplementary groups are cleared. Linux cross-UID supervision needs `SETUID`, `SETGID` and `KILL`; startup checks these before any command condition. +- Permanent noncritical failures keep unrelated work running but now produce eventual exit 1. Critical precommand failures stop everything unless `continueOnError` is set. +- A skipped critical process stays unready. Readiness includes configured critical processes from construction, and skipped processes remain in status and metrics. +- Sequential forward dependencies are rejected. Dependencies are satisfied by any successful initial start, including one-shot commands. +- Oversized console lines are split into prefixed 64 KiB chunks. Raw log files remain byte-exact. Legacy rotated files are retained and require separate retention management. +- Custom conditions must implement `Evaluate(context.Context) (bool, error)`; evaluation helpers take context first. Command probes run with their configured process credentials, environment and working directory and cannot outlive cancellation in the original process group. diff --git a/docs/security.md b/docs/security.md index 126e4fd..7362b6b 100644 --- a/docs/security.md +++ b/docs/security.md @@ -22,8 +22,8 @@ Gonner is not a sandbox. Use kernel features (Linux capabilities, seccomp, Kuber | Log file mode | `0o600` | Per-process override via `logFileMode`. | | Log directory mode | `0o750` | Created by gonner when the parent directory does not exist. | | Privilege drop | Opt-in (`user`/`group`) | Requires gonner to start as root. | -| Zombie reaping | PID 1 only | SIGCHLD-driven; coordinated with `exec.Cmd.Wait` via `ReapedStatuses`. Unclaimed statuses from orphaned descendants are pruned after 30s to bound memory. | -| Goroutine panics | Recovered | All long-running goroutines defer `logging.Recover`. | +| Zombie reaping | PID 1 only | SIGCHLD plus periodic discovery; only unregistered adopted children are reaped. Managed children have one `exec.Cmd.Wait` owner. | +| Child execution | Shared service | Main commands, precommands and probes use the same wait registry and group cleanup. | --- @@ -86,7 +86,11 @@ If your container's entrypoint runs as root (common with PID 1 + zombie reaping) ### 6. Constrain log file access -Default mode is `0o600` (owner read/write only). Override per-process via `logFileMode` when you ship logs to a shared collector — but never relax it past `0o640`. +Default mode is `0o600` (owner read/write only); `0o640` permits a collector group to read. Other-user write access is rejected. Log and PID paths must be owned by root or the supervisor UID, and every ancestor must resist replacement by workload UIDs. Protected sticky directories such as `/tmp` and trusted system symlinks are supported. Writable non-sticky ancestors, lower-UID-owned directories, leaf symlinks, hardlinks and special files are rejected before commands or probes run. Directories are retained by descriptor; opens, rotations and pruning stay relative to those descriptors. Permissions are changed on the validated file descriptor, never via the original pathname. + +Linux POSIX ACL write permissions are bounded by the mode mask; macOS extended ACL mutation grants are checked separately. Same-UID processes and privileged administrators remain inside the trust boundary. A supervisor and workload sharing a UID cannot be isolated by file ownership. + +Configured numeric users use account lookup for the primary GID. Unknown numeric UIDs need an explicit group, and inherited supplementary groups are cleared. Linux startup checks `CAP_SETGID` and, when changing UID, `CAP_SETUID` and `CAP_KILL`. These checks prevent starting a child the supervisor cannot later signal. If you write logs to a tmpfs / shared volume, the directory itself should be `0o750` or stricter. @@ -119,7 +123,7 @@ Please report security issues privately via [GitHub security advisories](https:/ | A03 — Injection | Commands run via `sh -c`, which is intentional. **Config is trusted.** Env interpolation pre-parse can change structure if untrusted vars are used. | | A04 — Insecure Design | Critical-process exit triggers full shutdown; failure modes prefer fail-fast. | | A05 — Security Misconfiguration | Strict defaults (private health, secure log perms). Misconfigurations surface via `gonner validate` warnings. | -| A06 — Vulnerable Components | Minimal deps (`cobra`, `errgroup`, `yaml.v3`). Run `go list -m -u all` to audit. | +| A06 — Vulnerable Components | Patched Go baseline; pinned vulnerability scans cover source and all release binaries before publication. Direct dependencies are `cobra`, `x/sys`, and `yaml.v3`. | | A07 — Auth Failures | Bearer-token comparison uses `crypto/subtle.ConstantTimeCompare`. | | A08 — Software & Data Integrity | Static binary builds with `CGO_ENABLED=0`; release binaries should be checksummed and signed. | | A09 — Logging Failures | Process and gonner logs are timestamped (RFC3339) and structured per-line. | diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index f59ffad..fd15546 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -105,7 +105,7 @@ The health server has built-in timeouts (`ReadHeaderTimeout=5s`, `Read/Write=15s ### Process output not appearing on stdout -- Per-line scanning is used. If your child writes binary or extremely long lines (> 1 MiB), the scanner drops to the next newline. Increase your line-flush frequency in the child if possible. +- Oversized output streams in 64 KiB chunks. Console chunks each get a prefix; raw log files retain every byte, including a missing final newline. Check gonner stderr for file errors or shutdown drain truncation. - `gonner.json` parse errors go to stderr, not stdout. ### Log file is empty diff --git a/go.mod b/go.mod index c919768..c01d688 100644 --- a/go.mod +++ b/go.mod @@ -1,10 +1,10 @@ module github.com/michaelishri/gonner -go 1.25.0 +go 1.26.7 require ( github.com/spf13/cobra v1.10.2 - golang.org/x/sync v0.20.0 + golang.org/x/sys v0.48.0 gopkg.in/yaml.v3 v3.0.1 ) diff --git a/go.sum b/go.sum index 9f3f3fe..f5ff8b0 100644 --- a/go.sum +++ b/go.sum @@ -7,8 +7,8 @@ github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiT github.com/spf13/pflag v1.0.9 h1:9exaQaMOCwffKiiiYk6/BndUBv+iRViNW+4lEMi0PvY= github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= -golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= -golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= +golang.org/x/sys v0.48.0 h1:bbX/i/6MgT9BVLM9RT1thmxL04yeTAhbEz4SyadbXoo= +golang.org/x/sys v0.48.0/go.mod h1:hNLxWAXmnKAxqDtdwIYC4bM9oQPEecfsnNMuSxOs3og= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/internal/condition/commandsucceeds.go b/internal/condition/commandsucceeds.go index 9fbae36..9373a68 100644 --- a/internal/condition/commandsucceeds.go +++ b/internal/condition/commandsucceeds.go @@ -2,32 +2,38 @@ package condition import ( "context" - "os/exec" + "errors" + "syscall" "time" + + "github.com/michaelishri/gonner/internal/execution" ) -// CommandSucceedsCondition runs a shell command and returns true if it exits 0. -// Format: the raw shell command string. A default 10s timeout is applied. type CommandSucceedsCondition struct { command string timeout time.Duration + service *execution.Service + spec execution.Spec } -// NewCommandSucceedsCondition creates a CommandSucceeds condition. func NewCommandSucceedsCondition(value string) Condition { - return &CommandSucceedsCondition{command: value, timeout: 10 * time.Second} + return &CommandSucceedsCondition{command: value, timeout: 10 * time.Second, service: execution.NewService()} } - -// Type returns "commandSucceeds". func (c *CommandSucceedsCondition) Type() string { return "commandSucceeds" } - -// Evaluate executes the command via sh -c and returns true if exit code is 0. -func (c *CommandSucceedsCondition) Evaluate() (bool, error) { - ctx, cancel := context.WithTimeout(context.Background(), c.timeout) +func (c *CommandSucceedsCondition) Evaluate(parent context.Context) (bool, error) { + ctx, cancel := context.WithTimeout(parent, c.timeout) defer cancel() - cmd := exec.CommandContext(ctx, "sh", "-c", c.command) - if err := cmd.Run(); err != nil { - return false, nil + spec := c.spec + spec.Command = c.command + spec.StopSignal = syscall.SIGKILL + spec.StopTimeout = time.Second + r := c.service.Run(ctx, spec) + var supervisor *execution.SupervisorError + if errors.As(r.Err, &supervisor) { + return false, r.Err + } + if parent.Err() != nil { + return false, parent.Err() } - return true, nil + return ctx.Err() == nil && r.Err == nil && r.ExitCode == 0, nil } diff --git a/internal/condition/condition.go b/internal/condition/condition.go index 7a61914..b8190bf 100644 --- a/internal/condition/condition.go +++ b/internal/condition/condition.go @@ -2,11 +2,15 @@ // for deciding whether a process should start. package condition +import ( + "context" +) + // Condition is the interface that all condition types must implement. type Condition interface { // Type returns the condition type name (e.g., "env", "fileExists"). Type() string // Evaluate returns true if the condition is met. - Evaluate() (bool, error) + Evaluate(ctx context.Context) (bool, error) } diff --git a/internal/condition/env.go b/internal/condition/env.go index 101844a..12acf97 100644 --- a/internal/condition/env.go +++ b/internal/condition/env.go @@ -1,6 +1,7 @@ package condition import ( + "context" "os" "strings" ) @@ -24,7 +25,10 @@ func (c *EnvCondition) Type() string { } // Evaluate checks the environment variable. -func (c *EnvCondition) Evaluate() (bool, error) { +func (c *EnvCondition) Evaluate(ctx context.Context) (bool, error) { + if err := ctx.Err(); err != nil { + return false, err + } if idx := strings.Index(c.raw, "="); idx > 0 { key := c.raw[:idx] expected := c.raw[idx+1:] diff --git a/internal/condition/env_test.go b/internal/condition/env_test.go index 95a88f1..6b68b13 100644 --- a/internal/condition/env_test.go +++ b/internal/condition/env_test.go @@ -1,6 +1,7 @@ package condition import ( + "context" "os" "testing" ) @@ -9,7 +10,7 @@ func TestEnvCondition_KeyValue_Match(t *testing.T) { t.Setenv("TEST_COND_KEY", "expected") cond := NewEnvCondition("TEST_COND_KEY=expected") - ok, err := cond.Evaluate() + ok, err := cond.Evaluate(context.Background()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -22,7 +23,7 @@ func TestEnvCondition_KeyValue_Mismatch(t *testing.T) { t.Setenv("TEST_COND_KEY", "other") cond := NewEnvCondition("TEST_COND_KEY=expected") - ok, err := cond.Evaluate() + ok, err := cond.Evaluate(context.Background()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -35,7 +36,7 @@ func TestEnvCondition_KeyValue_Unset(t *testing.T) { os.Unsetenv("TEST_COND_UNSET") cond := NewEnvCondition("TEST_COND_UNSET=value") - ok, err := cond.Evaluate() + ok, err := cond.Evaluate(context.Background()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -48,7 +49,7 @@ func TestEnvCondition_KeyOnly_Set(t *testing.T) { t.Setenv("TEST_COND_EXISTS", "anything") cond := NewEnvCondition("TEST_COND_EXISTS") - ok, err := cond.Evaluate() + ok, err := cond.Evaluate(context.Background()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -61,7 +62,7 @@ func TestEnvCondition_KeyOnly_Unset(t *testing.T) { os.Unsetenv("TEST_COND_ABSENT") cond := NewEnvCondition("TEST_COND_ABSENT") - ok, err := cond.Evaluate() + ok, err := cond.Evaluate(context.Background()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -74,7 +75,7 @@ func TestEnvCondition_KeyOnly_Empty(t *testing.T) { t.Setenv("TEST_COND_EMPTY", "") cond := NewEnvCondition("TEST_COND_EMPTY") - ok, err := cond.Evaluate() + ok, err := cond.Evaluate(context.Background()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -87,7 +88,7 @@ func TestEnvCondition_KeyValue_EmptyExpected(t *testing.T) { t.Setenv("TEST_COND_EMPTY_VAL", "") cond := NewEnvCondition("TEST_COND_EMPTY_VAL=") - ok, err := cond.Evaluate() + ok, err := cond.Evaluate(context.Background()) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/internal/condition/file.go b/internal/condition/file.go index ad7ccc9..f776540 100644 --- a/internal/condition/file.go +++ b/internal/condition/file.go @@ -1,6 +1,9 @@ package condition -import "os" +import ( + "context" + "os" +) // FileCondition checks whether a file or directory exists at a given path. type FileCondition struct { @@ -18,7 +21,10 @@ func (c *FileCondition) Type() string { } // Evaluate returns true if the path exists. -func (c *FileCondition) Evaluate() (bool, error) { +func (c *FileCondition) Evaluate(ctx context.Context) (bool, error) { + if err := ctx.Err(); err != nil { + return false, err + } _, err := os.Stat(c.path) if err == nil { return true, nil diff --git a/internal/condition/file_test.go b/internal/condition/file_test.go index 7958647..79c4ebb 100644 --- a/internal/condition/file_test.go +++ b/internal/condition/file_test.go @@ -1,6 +1,7 @@ package condition import ( + "context" "os" "path/filepath" "testing" @@ -15,7 +16,7 @@ func TestFileCondition_FileExists(t *testing.T) { cond := NewFileCondition(f) - ok, err := cond.Evaluate() + ok, err := cond.Evaluate(context.Background()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -27,7 +28,7 @@ func TestFileCondition_FileExists(t *testing.T) { func TestFileCondition_FileNotExists(t *testing.T) { cond := NewFileCondition("/nonexistent/path/file.txt") - ok, err := cond.Evaluate() + ok, err := cond.Evaluate(context.Background()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -40,7 +41,7 @@ func TestFileCondition_DirectoryExists(t *testing.T) { dir := t.TempDir() cond := NewFileCondition(dir) - ok, err := cond.Evaluate() + ok, err := cond.Evaluate(context.Background()) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/internal/condition/new_conditions_test.go b/internal/condition/new_conditions_test.go index 8b7cbe9..f1fee46 100644 --- a/internal/condition/new_conditions_test.go +++ b/internal/condition/new_conditions_test.go @@ -1,8 +1,10 @@ package condition import ( + "context" "net" "testing" + "time" ) func TestPortOpen_Open(t *testing.T) { @@ -13,7 +15,7 @@ func TestPortOpen_Open(t *testing.T) { defer l.Close() cond := NewPortOpenCondition(l.Addr().String()) - ok, err := cond.Evaluate() + ok, err := cond.Evaluate(context.Background()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -25,7 +27,7 @@ func TestPortOpen_Open(t *testing.T) { func TestPortOpen_Closed(t *testing.T) { // Use a port that's very likely unused (high random port). cond := NewPortOpenCondition("127.0.0.1:1@200ms") - ok, err := cond.Evaluate() + ok, err := cond.Evaluate(context.Background()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -43,7 +45,7 @@ func TestPortOpen_Type(t *testing.T) { func TestCommandSucceeds_True(t *testing.T) { cond := NewCommandSucceedsCondition("true") - ok, err := cond.Evaluate() + ok, err := cond.Evaluate(context.Background()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -54,7 +56,7 @@ func TestCommandSucceeds_True(t *testing.T) { func TestCommandSucceeds_False(t *testing.T) { cond := NewCommandSucceedsCondition("false") - ok, err := cond.Evaluate() + ok, err := cond.Evaluate(context.Background()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -69,3 +71,38 @@ func TestCommandSucceeds_Type(t *testing.T) { t.Errorf("Type(): got %q", cond.Type()) } } + +func TestPortOpenBareAndColonPort(t *testing.T) { + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer l.Close() + _, port, err := net.SplitHostPort(l.Addr().String()) + if err != nil { + t.Fatal(err) + } + for _, target := range []string{port, ":" + port, "localhost:" + port, l.Addr().String()} { + ok, err := NewPortOpenCondition(target).Evaluate(context.Background()) + if err != nil || !ok { + t.Errorf("%q: %v %v", target, ok, err) + } + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := NewPortOpenCondition(port).Evaluate(ctx); err != context.Canceled { + t.Fatalf("cancel: %v", err) + } +} +func TestCommandConditionCancellationAndTimeout(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := NewCommandSucceedsCondition("exec sleep 30").Evaluate(ctx); err != context.Canceled { + t.Fatalf("cancel: %v", err) + } + c := NewCommandSucceedsCondition("exec sleep 30").(*CommandSucceedsCondition) + c.timeout = 20 * time.Millisecond + if ok, err := c.Evaluate(context.Background()); err != nil || ok { + t.Fatalf("timeout: %v %v", ok, err) + } +} diff --git a/internal/condition/portopen.go b/internal/condition/portopen.go index ddf792a..0727f8e 100644 --- a/internal/condition/portopen.go +++ b/internal/condition/portopen.go @@ -1,7 +1,9 @@ package condition import ( + "context" "net" + "strconv" "strings" "time" ) @@ -24,6 +26,9 @@ func NewPortOpenCondition(value string) Condition { target = value[:idx] } } + if _, err := strconv.ParseUint(target, 10, 16); err == nil { + target = net.JoinHostPort("127.0.0.1", target) + } if strings.HasPrefix(target, ":") { target = "127.0.0.1" + target } @@ -35,11 +40,16 @@ func (c *PortOpenCondition) Type() string { return "portOpen" } // Evaluate attempts a TCP dial within the configured timeout. // Connection refused and timeouts both yield (false, nil) rather than an error. -func (c *PortOpenCondition) Evaluate() (bool, error) { - conn, err := net.DialTimeout("tcp", c.target, c.timeout) +func (c *PortOpenCondition) Evaluate(ctx context.Context) (bool, error) { + conn, err := (&net.Dialer{Timeout: c.timeout}).DialContext(ctx, "tcp", c.target) + if conn != nil { + defer conn.Close() + } + if ctx.Err() != nil { + return false, ctx.Err() + } if err != nil { return false, nil } - _ = conn.Close() return true, nil } diff --git a/internal/condition/registry.go b/internal/condition/registry.go index 47579e3..cac8d0e 100644 --- a/internal/condition/registry.go +++ b/internal/condition/registry.go @@ -1,14 +1,19 @@ package condition import ( + "context" "fmt" "sort" + "sync" + + "github.com/michaelishri/gonner/internal/execution" ) // ConditionFactory is a function that creates a Condition from a raw config value. type ConditionFactory func(value string) Condition // registry holds the mapping of condition type names to their factory functions. +var registryMu sync.RWMutex var registry = map[string]ConditionFactory{ "env": NewEnvCondition, "fileExists": NewFileCondition, @@ -18,18 +23,24 @@ var registry = map[string]ConditionFactory{ // Register adds a new condition type to the registry. func Register(typeName string, factory ConditionFactory) { + registryMu.Lock() + defer registryMu.Unlock() registry[typeName] = factory } // Known reports whether a condition type name is registered. func Known(typeName string) bool { + registryMu.RLock() + defer registryMu.RUnlock() _, ok := registry[typeName] return ok } // Create instantiates a Condition from a type name and raw value. func Create(typeName, value string) (Condition, error) { + registryMu.RLock() factory, ok := registry[typeName] + registryMu.RUnlock() if !ok { return nil, fmt.Errorf("unknown condition type: %q", typeName) } @@ -62,13 +73,19 @@ type condSpec struct { // EvaluateAll evaluates a list of conditions where all must be true (AND logic). // Returns true if the list is empty. -func EvaluateAll(conditions []map[string]string) (bool, string, error) { +func (e *Evaluator) EvaluateAll(ctx context.Context, conditions []map[string]string) (bool, string, error) { + if err := ctx.Err(); err != nil { + return false, "", err + } for _, spec := range flatten(conditions) { - cond, err := Create(spec.typeName, spec.value) + if err := ctx.Err(); err != nil { + return false, "", err + } + cond, err := e.create(spec.typeName, spec.value) if err != nil { return false, "", err } - ok, err := cond.Evaluate() + ok, err := cond.Evaluate(ctx) if err != nil { return false, "", fmt.Errorf("evaluating %s condition: %w", spec.typeName, err) } @@ -81,7 +98,10 @@ func EvaluateAll(conditions []map[string]string) (bool, string, error) { // EvaluateAny evaluates a list of conditions where at least one must be true (OR logic). // Returns true if the list is empty. -func EvaluateAny(conditions []map[string]string) (bool, string, error) { +func (e *Evaluator) EvaluateAny(ctx context.Context, conditions []map[string]string) (bool, string, error) { + if err := ctx.Err(); err != nil { + return false, "", err + } specs := flatten(conditions) if len(specs) == 0 { return true, "", nil @@ -89,11 +109,14 @@ func EvaluateAny(conditions []map[string]string) (bool, string, error) { var lastDesc string for _, spec := range specs { - cond, err := Create(spec.typeName, spec.value) + if err := ctx.Err(); err != nil { + return false, "", err + } + cond, err := e.create(spec.typeName, spec.value) if err != nil { return false, "", err } - ok, err := cond.Evaluate() + ok, err := cond.Evaluate(ctx) if err != nil { return false, "", fmt.Errorf("evaluating %s condition: %w", spec.typeName, err) } @@ -107,9 +130,12 @@ func EvaluateAny(conditions []map[string]string) (bool, string, error) { // ShouldRun evaluates both whenAll and whenAny conditions for a process. // Returns (true, "") if the process should run, or (false, reason) if skipped. -func ShouldRun(whenAll, whenAny []map[string]string) (bool, string, error) { +func (e *Evaluator) ShouldRun(ctx context.Context, whenAll, whenAny []map[string]string) (bool, string, error) { + if err := ctx.Err(); err != nil { + return false, "", err + } if len(whenAll) > 0 { - ok, reason, err := EvaluateAll(whenAll) + ok, reason, err := e.EvaluateAll(ctx, whenAll) if err != nil { return false, "", err } @@ -119,7 +145,7 @@ func ShouldRun(whenAll, whenAny []map[string]string) (bool, string, error) { } if len(whenAny) > 0 { - ok, reason, err := EvaluateAny(whenAny) + ok, reason, err := e.EvaluateAny(ctx, whenAny) if err != nil { return false, "", err } @@ -130,3 +156,38 @@ func ShouldRun(whenAll, whenAny []map[string]string) (bool, string, error) { return true, "", nil } + +// Evaluator injects the manager's execution service and process environment +// without changing factories for user-registered conditions. +type Evaluator struct { + service *execution.Service + spec execution.Spec +} + +func NewEvaluator(service *execution.Service, spec execution.Spec) *Evaluator { + return &Evaluator{service: service, spec: spec} +} +func (e *Evaluator) create(name, value string) (Condition, error) { + cond, err := Create(name, value) + if err != nil { + return nil, err + } + if c, ok := cond.(*CommandSucceedsCondition); ok { + // Preserve custom registered factories, and do not mutate objects a + // factory may share between evaluators. + copy := *c + copy.service = e.service + copy.spec = e.spec + return ©, nil + } + return cond, nil +} +func EvaluateAll(ctx context.Context, c []map[string]string) (bool, string, error) { + return NewEvaluator(execution.NewService(), execution.Spec{}).EvaluateAll(ctx, c) +} +func EvaluateAny(ctx context.Context, c []map[string]string) (bool, string, error) { + return NewEvaluator(execution.NewService(), execution.Spec{}).EvaluateAny(ctx, c) +} +func ShouldRun(ctx context.Context, all, any []map[string]string) (bool, string, error) { + return NewEvaluator(execution.NewService(), execution.Spec{}).ShouldRun(ctx, all, any) +} diff --git a/internal/condition/registry_test.go b/internal/condition/registry_test.go index 9cff29b..32aa3d2 100644 --- a/internal/condition/registry_test.go +++ b/internal/condition/registry_test.go @@ -1,6 +1,7 @@ package condition import ( + "context" "os" "path/filepath" "testing" @@ -44,7 +45,7 @@ func TestEvaluateAll_AllTrue(t *testing.T) { {"fileExists": f}, } - ok, _, err := EvaluateAll(conditions) + ok, _, err := EvaluateAll(context.Background(), conditions) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -61,7 +62,7 @@ func TestEvaluateAll_OneFalse(t *testing.T) { {"fileExists": "/nonexistent/path"}, } - ok, reason, err := EvaluateAll(conditions) + ok, reason, err := EvaluateAll(context.Background(), conditions) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -74,7 +75,7 @@ func TestEvaluateAll_OneFalse(t *testing.T) { } func TestEvaluateAll_Empty(t *testing.T) { - ok, _, err := EvaluateAll(nil) + ok, _, err := EvaluateAll(context.Background(), nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -92,7 +93,7 @@ func TestEvaluateAll_MultipleSameType(t *testing.T) { {"env": "TEST_ALL_M2=b"}, } - ok, _, err := EvaluateAll(conditions) + ok, _, err := EvaluateAll(context.Background(), conditions) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -109,7 +110,7 @@ func TestEvaluateAny_OneTrue(t *testing.T) { {"fileExists": os.TempDir()}, // exists } - ok, _, err := EvaluateAny(conditions) + ok, _, err := EvaluateAny(context.Background(), conditions) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -126,7 +127,7 @@ func TestEvaluateAny_NoneTrue(t *testing.T) { {"fileExists": "/nonexistent/path"}, } - ok, _, err := EvaluateAny(conditions) + ok, _, err := EvaluateAny(context.Background(), conditions) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -136,7 +137,7 @@ func TestEvaluateAny_NoneTrue(t *testing.T) { } func TestEvaluateAny_Empty(t *testing.T) { - ok, _, err := EvaluateAny(nil) + ok, _, err := EvaluateAny(context.Background(), nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -149,7 +150,7 @@ func TestShouldRun_BothMet(t *testing.T) { t.Setenv("TEST_SHOULD_A", "1") dir := t.TempDir() - ok, _, err := ShouldRun( + ok, _, err := ShouldRun(context.Background(), []map[string]string{{"env": "TEST_SHOULD_A=1"}}, []map[string]string{{"fileExists": dir}}, ) @@ -164,7 +165,7 @@ func TestShouldRun_BothMet(t *testing.T) { func TestShouldRun_WhenAllFails(t *testing.T) { os.Unsetenv("TEST_SHOULD_MISSING") - ok, reason, err := ShouldRun( + ok, reason, err := ShouldRun(context.Background(), []map[string]string{{"env": "TEST_SHOULD_MISSING=yes"}}, nil, ) @@ -180,7 +181,7 @@ func TestShouldRun_WhenAllFails(t *testing.T) { } func TestShouldRun_NilConditions(t *testing.T) { - ok, _, err := ShouldRun(nil, nil) + ok, _, err := ShouldRun(context.Background(), nil, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -203,7 +204,7 @@ func TestRegister_Custom(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - ok, err := cond.Evaluate() + ok, err := cond.Evaluate(context.Background()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -214,5 +215,24 @@ func TestRegister_Custom(t *testing.T) { type alwaysTrueCond struct{} -func (c *alwaysTrueCond) Type() string { return "alwaysTrue" } -func (c *alwaysTrueCond) Evaluate() (bool, error) { return true, nil } +func (c *alwaysTrueCond) Type() string { return "alwaysTrue" } +func (c *alwaysTrueCond) Evaluate(context.Context) (bool, error) { return true, nil } + +func TestEvaluatorPreservesCustomCommandFactory(t *testing.T) { + registryMu.RLock() + original := registry["commandSucceeds"] + registryMu.RUnlock() + Register("commandSucceeds", func(string) Condition { return &alwaysTrueCond{} }) + defer Register("commandSucceeds", original) + ok, _, err := ShouldRun(context.Background(), []map[string]string{{"commandSucceeds": "false"}}, nil) + if err != nil || !ok { + t.Fatalf("custom factory ignored: %v %v", ok, err) + } +} +func TestEmptyConditionsRespectCancellation(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, _, err := ShouldRun(ctx, nil, nil); err != context.Canceled { + t.Fatalf("cancelled evaluation: %v", err) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index 8ed49a6..f3967f8 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,7 +1,9 @@ // Package config defines the configuration structures and parsing for gonner. package config -import "time" +import ( + "time" +) // Config is the top-level gonner configuration. type Config struct { @@ -81,7 +83,8 @@ type ProcessConfig struct { LogRotate *LogRotateConfig `json:"logRotate,omitempty" yaml:"logRotate,omitempty"` // User drops privileges to this username (or numeric UID) before exec. - // Requires gonner to start as root. Linux/macOS only. + // Uses the account primary GID; unknown numeric UIDs require Group. + // Requires appropriate Linux capabilities or root on macOS. User string `json:"user,omitempty" yaml:"user,omitempty"` // Group sets the primary group (name or numeric GID) for the process. Requires User. @@ -109,7 +112,8 @@ type ProcessConfig struct { // Critical marks this process as critical — its unexpected exit triggers full shutdown. Critical bool `json:"critical,omitempty" yaml:"critical,omitempty"` - // DependsOn lists process names that must be running before this one starts. + // DependsOn lists process names for which any instance must have started + // successfully at least once before this one starts. DependsOn []string `json:"dependsOn,omitempty" yaml:"dependsOn,omitempty"` // WhenAll requires all conditions to be true for the process to start. diff --git a/internal/config/validation.go b/internal/config/validation.go index bce581c..cab3b5c 100644 --- a/internal/config/validation.go +++ b/internal/config/validation.go @@ -174,8 +174,15 @@ func ValidateWithWarnings(cfg *Config) (*ValidationResult, error) { } // Validate dependsOn references and detect cycles + order := make(map[string]int) + for i, p := range cfg.Run { + order[p.Name] = i + } for _, proc := range cfg.Run { for _, dep := range proc.DependsOn { + if cfg.Mode == "sequential" && names[dep] && order[dep] > order[proc.Name] { + errs = append(errs, fmt.Sprintf("%s: sequential dependsOn %q must appear earlier in run", proc.Name, dep)) + } if !names[dep] { errs = append(errs, fmt.Sprintf("%s: dependsOn references unknown process %q", proc.Name, dep)) } diff --git a/internal/config/validation_test.go b/internal/config/validation_test.go index 4672096..bc6e5c9 100644 --- a/internal/config/validation_test.go +++ b/internal/config/validation_test.go @@ -324,3 +324,15 @@ func TestValidate_ValidEnv(t *testing.T) { t.Errorf("expected no error, got: %v", err) } } + +func TestSequentialRejectsForwardDependencies(t *testing.T) { + cfg := &Config{Mode: "sequential", Run: []ProcessConfig{{Name: "first", Command: "true", DependsOn: []string{"later"}}, {Name: "later", Command: "true"}}} + cfg.ApplyDefaults() + if err := Validate(cfg); err == nil { + t.Fatal("accepted forward dependency") + } + cfg.Mode = "parallel" + if err := Validate(cfg); err != nil { + t.Fatal(err) + } +} diff --git a/internal/execution/capabilities_darwin.go b/internal/execution/capabilities_darwin.go new file mode 100644 index 0000000..68eb4f1 --- /dev/null +++ b/internal/execution/capabilities_darwin.go @@ -0,0 +1,14 @@ +package execution + +import ( + "fmt" + "os" + "syscall" +) + +func checkCapabilities(c *syscall.Credential) error { + if os.Geteuid() != 0 { + return fmt.Errorf("setting user/group requires root") + } + return nil +} diff --git a/internal/execution/capabilities_linux.go b/internal/execution/capabilities_linux.go new file mode 100644 index 0000000..329322c --- /dev/null +++ b/internal/execution/capabilities_linux.go @@ -0,0 +1,31 @@ +package execution + +import ( + "fmt" + "os" + "syscall" + + "golang.org/x/sys/unix" +) + +func checkCapabilities(c *syscall.Credential) error { + var data [2]unix.CapUserData + header := unix.CapUserHeader{Version: unix.LINUX_CAPABILITY_VERSION_3} + if err := unix.Capget(&header, &data[0]); err != nil { + return fmt.Errorf("reading capabilities: %w", err) + } + has := func(cap uint) bool { return data[cap/32].Effective&(1<<(cap%32)) != 0 } + // setgroups(empty) always needs SETGID, even when keeping the primary GID. + if !has(unix.CAP_SETGID) { + return fmt.Errorf("user/group requires CAP_SETGID to clear supplementary groups") + } + if c.Uid != uint32(os.Geteuid()) { + if !has(unix.CAP_SETUID) { + return fmt.Errorf("changing user requires CAP_SETUID") + } + if !has(unix.CAP_KILL) { + return fmt.Errorf("supervising another UID requires CAP_KILL") + } + } + return nil +} diff --git a/internal/execution/credential.go b/internal/execution/credential.go new file mode 100644 index 0000000..3b584dd --- /dev/null +++ b/internal/execution/credential.go @@ -0,0 +1,78 @@ +package execution + +import ( + "fmt" + "os/user" + "strconv" + "strings" + "syscall" +) + +func numeric(s string) bool { + return s != "" && strings.IndexFunc(s, func(r rune) bool { return r < '0' || r > '9' }) < 0 +} +func parseID(s string) (uint32, error) { + n, err := strconv.ParseUint(s, 10, 32) + if err != nil || n == 1<<32-1 { + return 0, fmt.Errorf("invalid UID/GID %q", s) + } + return uint32(n), nil +} + +// ResolveCredential uses the account's real primary group for numeric and named +// users. Unknown numeric UIDs require an explicit group. Supplementary groups +// are intentionally empty, and NoSetGroups is false. +func ResolveCredential(userSpec, groupSpec string) (*syscall.Credential, error) { + if userSpec == "" { + if groupSpec != "" { + return nil, fmt.Errorf("group requires user") + } + return nil, nil + } + var account *user.User + var err error + var uid, gid uint32 + if numeric(userSpec) { + uid, err = parseID(userSpec) + if err != nil { + return nil, err + } + account, err = user.LookupId(strconv.FormatUint(uint64(uid), 10)) + } else { + account, err = user.Lookup(userSpec) + } + if err != nil { + if !numeric(userSpec) || groupSpec == "" { + return nil, fmt.Errorf("resolving user %q (unknown numeric UID requires group): %w", userSpec, err) + } + } + if account != nil { + uid, err = parseID(account.Uid) + if err != nil { + return nil, err + } + gid, err = parseID(account.Gid) + if err != nil { + return nil, err + } + } + if groupSpec != "" { + g := groupSpec + if !numeric(g) { + grp, e := user.LookupGroup(g) + if e != nil { + return nil, fmt.Errorf("resolving group: %w", e) + } + g = grp.Gid + } + gid, err = parseID(g) + if err != nil { + return nil, err + } + } + cred := &syscall.Credential{Uid: uid, Gid: gid, Groups: []uint32{}} + if err := checkCapabilities(cred); err != nil { + return nil, err + } + return cred, nil +} diff --git a/internal/execution/reaper_darwin.go b/internal/execution/reaper_darwin.go new file mode 100644 index 0000000..3ca1df0 --- /dev/null +++ b/internal/execution/reaper_darwin.go @@ -0,0 +1,43 @@ +package execution + +import ( + "context" + "errors" + "fmt" + "syscall" + + "golang.org/x/sys/unix" +) + +const ( + darwinZombie = 5 // SZOMB in . + darwinExiting = 0x2000 // P_WEXIT in . +) + +func (s *Service) StartReaper(ctx context.Context) (func(), error) { return func() {}, nil } +func groupAlive(pid int) (bool, error) { + exited, err := groupSignalResult(pid, syscall.Kill(-pid, 0)) + return !exited && err == nil, err +} + +// Darwin's killpg can return EPERM for zombies and processes committed to exit. +// Confirm that all members are exiting before suppressing that error; +// permission failures for live members must still reach the supervisor. +func groupSignalResult(pid int, err error) (exited bool, signalErr error) { + if errors.Is(err, syscall.ESRCH) { + return true, nil + } + if errors.Is(err, syscall.EPERM) { + procs, inspectErr := unix.SysctlKinfoProcSlice("kern.proc.pgrp", pid) + if inspectErr != nil { + return false, errors.Join(err, fmt.Errorf("inspecting process group %d: %w", pid, inspectErr)) + } + for _, proc := range procs { + if proc.Proc.P_stat != darwinZombie && proc.Proc.P_flag&darwinExiting == 0 { + return false, fmt.Errorf("process group %d contains live PID %d (state %d, flags %#x): %w", pid, proc.Proc.P_pid, proc.Proc.P_stat, proc.Proc.P_flag, err) + } + } + return true, nil + } + return false, err +} diff --git a/internal/execution/reaper_darwin_test.go b/internal/execution/reaper_darwin_test.go new file mode 100644 index 0000000..10ee73e --- /dev/null +++ b/internal/execution/reaper_darwin_test.go @@ -0,0 +1,86 @@ +package execution + +import ( + "context" + "errors" + "os/exec" + "syscall" + "testing" + "time" + + "golang.org/x/sys/unix" +) + +func startDarwinGroup(t *testing.T, command string, args ...string) int { + t.Helper() + cmd := exec.Command(command, args...) + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + _ = cmd.Process.Kill() + _ = cmd.Wait() + }) + return cmd.Process.Pid +} + +func TestDarwinZombieGroupCleanup(t *testing.T) { + pid := startDarwinGroup(t, "true") + deadline := time.Now().Add(5 * time.Second) + for { + proc, err := unix.SysctlKinfoProc("kern.proc.pid", pid) + if err != nil { + t.Fatal(err) + } + if proc.Proc.P_stat == darwinZombie { + break + } + if time.Now().After(deadline) { + t.Fatal("child did not become a zombie") + } + time.Sleep(time.Millisecond) + } + // Deliberately leave the child unreaped until cleanup so the group remains + // present with only a zombie throughout each operation below. + if alive, err := groupAlive(pid); alive || err != nil { + t.Fatalf("zombie group: alive=%v, err=%v", alive, err) + } + if err := SignalGroup(pid, syscall.SIGKILL); err != nil { + t.Fatalf("signalling zombie group: %v", err) + } + if err := stopGroup(pid, syscall.SIGTERM, 50*time.Millisecond); err != nil { + t.Fatalf("stopping zombie group: %v", err) + } +} + +func TestDarwinLiveGroupPreservesSignalErrors(t *testing.T) { + pid := startDarwinGroup(t, "sleep", "30") + if alive, err := groupAlive(pid); !alive || err != nil { + t.Fatalf("live group: alive=%v, err=%v", alive, err) + } + // Exercise permission-denial handling without requiring privileged tests + // or signalling another user's processes. + if exited, err := groupSignalResult(pid, syscall.EPERM); exited || !errors.Is(err, syscall.EPERM) { + t.Fatalf("suppressed live-group permission error: exited=%v, err=%v", exited, err) + } + if err := SignalGroup(pid, syscall.Signal(-1)); !errors.Is(err, syscall.EINVAL) { + t.Fatalf("suppressed invalid signal: %v", err) + } +} + +func TestDarwinCancellationDuringExit(t *testing.T) { + for i := 0; i < 100; i++ { + ctx, cancel := context.WithCancel(context.Background()) + r := NewService().Run(ctx, Spec{ + Command: "exec sleep 30", + StopSignal: syscall.SIGKILL, + StopTimeout: time.Second, + OnStart: func(int) { cancel() }, + }) + cancel() + if r.Err != context.Canceled { + t.Fatalf("cancellation %d: %v", i, r.Err) + } + } +} diff --git a/internal/execution/reaper_linux.go b/internal/execution/reaper_linux.go new file mode 100644 index 0000000..786a20a --- /dev/null +++ b/internal/execution/reaper_linux.go @@ -0,0 +1,146 @@ +package execution + +import ( + "context" + "errors" + "fmt" + "os" + "os/signal" + "path/filepath" + "strconv" + "strings" + "syscall" + "time" +) + +func directChildren() ([]int, error) { + paths, err := filepath.Glob("/proc/self/task/*/children") + if err != nil { + return nil, err + } + if len(paths) == 0 { + return nil, fmt.Errorf("PID 1 requires /proc/self/task/*/children") + } + var pids []int + read := false + for _, path := range paths { + b, err := os.ReadFile(path) + if errors.Is(err, os.ErrNotExist) { + continue + } + if err != nil { + return nil, err + } + read = true + for _, s := range strings.Fields(string(b)) { + pid, e := strconv.Atoi(s) + if e != nil { + return nil, e + } + pids = append(pids, pid) + } + } + if !read { + return nil, fmt.Errorf("cannot discover PID 1 children") + } + return pids, nil +} +func reapOrphans() error { + children.Lock() + defer children.Unlock() + pids, err := directChildren() + if err != nil { + return err + } + for _, pid := range pids { + if _, managed := children.managed[pid]; managed { + continue + } + var status syscall.WaitStatus + _, err := syscall.Wait4(pid, &status, syscall.WNOHANG, nil) + if err != nil && !errors.Is(err, syscall.ECHILD) && !errors.Is(err, syscall.ESRCH) && !errors.Is(err, syscall.EINTR) { + return err + } + } + return nil +} + +// StartReaper returns a join function. Only adopted, unregistered children are +// reaped; os/exec retains exclusive ownership of every managed child status. +func (s *Service) StartReaper(ctx context.Context) (func(), error) { + if os.Getpid() != 1 { + return func() {}, nil + } + if _, err := directChildren(); err != nil { + return nil, err + } + ctx, cancel := context.WithCancel(ctx) + done := make(chan struct{}) + sig := make(chan os.Signal, 16) + signal.Notify(sig, syscall.SIGCHLD) + go func() { + defer close(done) + defer signal.Stop(sig) + ticker := time.NewTicker(time.Second) + defer ticker.Stop() + for { + if err := reapOrphans(); err != nil { + s.report(&SupervisorError{err}) + return + } + select { + case <-ctx.Done(): + if err := reapOrphans(); err != nil { + s.report(&SupervisorError{err}) + } + return + case <-sig: + case <-ticker.C: + } + } + }() + return func() { cancel(); <-done }, nil +} + +// Linux zombies must not consume the entire grace period: they cannot receive +// signals or hold pipes. PID 1's reaper will collect them independently. +func groupAlive(pgid int) (bool, error) { + exited, err := groupSignalResult(pgid, syscall.Kill(-pgid, 0)) + if exited || err != nil { + return false, err + } + paths, err := filepath.Glob("/proc/[0-9]*/stat") + if err != nil || len(paths) == 0 { + return true, nil + } + for _, path := range paths { + b, err := os.ReadFile(path) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + continue + } + return true, nil + } + end := strings.LastIndexByte(string(b), ')') + if end < 0 { + continue + } + f := strings.Fields(string(b[end+1:])) + if len(f) < 3 { + continue + } + g, _ := strconv.Atoi(f[2]) + if g == pgid && f[0] != "Z" && f[0] != "X" { + return true, nil + } + } + return false, nil +} + +// groupSignalResult distinguishes an exited group from a signal failure. +func groupSignalResult(_ int, err error) (exited bool, signalErr error) { + if errors.Is(err, syscall.ESRCH) { + return true, nil + } + return false, err +} diff --git a/internal/execution/service.go b/internal/execution/service.go new file mode 100644 index 0000000..23a8bc9 --- /dev/null +++ b/internal/execution/service.go @@ -0,0 +1,251 @@ +// Package execution owns child creation, waiting, group shutdown and pipe FDs. +// It has no dependency on configuration, conditions or logging. +package execution + +import ( + "context" + "errors" + "fmt" + "io" + "os" + "os/exec" + "sync" + "syscall" + "time" +) + +// The registry spans services so concurrent managers/probes cannot reap one +// another's children. Start and registration share the reaper's lock. +var children = struct { + sync.Mutex + managed map[int]*exec.Cmd +}{managed: make(map[int]*exec.Cmd)} + +type SupervisorError struct{ Err error } + +func (e *SupervisorError) Error() string { return "supervisor: " + e.Err.Error() } +func (e *SupervisorError) Unwrap() error { return e.Err } + +type Service struct { + Diagnostic func(string, ...any) + Errors chan error +} + +func NewService() *Service { return &Service{Errors: make(chan error, 1)} } +func (s *Service) diagnostic(f string, a ...any) { + if s.Diagnostic != nil { + s.Diagnostic(f, a...) + } +} +func (s *Service) report(err error) { + select { + case s.Errors <- err: + default: + } +} + +type Spec struct { + Command, Dir string + Env []string + Credential *syscall.Credential + StopSignal syscall.Signal + StopTimeout time.Duration + // Output must drain the reader even if an output destination fails. + Output func(io.Reader) error + OnStart func(int) + OnExit func(time.Duration) + OnStop func() +} +type Result struct { + Started bool + Cancelled bool + ExitCode int + Runtime time.Duration + Err error +} + +// Run calls exec.Cmd.Wait exactly once. The explicit pipe ownership ensures +// Wait never closes a reader while output is still being drained. +func (s *Service) Run(ctx context.Context, spec Spec) Result { + r := Result{ExitCode: -1} + if err := ctx.Err(); err != nil { + r.Err = err + r.Cancelled = true + return r + } + if spec.StopSignal == 0 { + spec.StopSignal = syscall.SIGTERM + } + if spec.StopTimeout <= 0 { + spec.StopTimeout = 30 * time.Second + } + cmd := exec.Command("sh", "-c", spec.Command) + cmd.Dir = spec.Dir + cmd.Env = spec.Env + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true, Credential: spec.Credential} + outR, outW, err := os.Pipe() + if err != nil { + r.Err = err + return r + } + defer outR.Close() + defer outW.Close() + errR, errW, err := os.Pipe() + if err != nil { + r.Err = err + return r + } + defer errR.Close() + defer errW.Close() + cmd.Stdout = outW + cmd.Stderr = errW + children.Lock() + if ctx.Err() != nil { + children.Unlock() + r.Err = ctx.Err() + r.Cancelled = true + return r + } + err = cmd.Start() + if err == nil { + children.managed[cmd.Process.Pid] = cmd + } + children.Unlock() + if err != nil { + r.Err = fmt.Errorf("starting command: %w", err) + return r + } + _ = outW.Close() + _ = errW.Close() + r.Started = true + start := time.Now() + pid := cmd.Process.Pid + if spec.OnStart != nil { + spec.OnStart(pid) + } + var drains sync.WaitGroup + var outputErr [2]error + for i, reader := range []*os.File{outR, errR} { + drains.Go(func() { + if spec.Output != nil { + outputErr[i] = spec.Output(reader) + } else { + _, outputErr[i] = io.Copy(io.Discard, reader) + } + }) + } + drainDone := make(chan struct{}) + go func() { drains.Wait(); close(drainDone) }() + var stopOnce sync.Once + var stopErr error + stop := func() { + stopOnce.Do(func() { + if spec.OnStop != nil { + spec.OnStop() + } + stopErr = stopGroup(pid, spec.StopSignal, spec.StopTimeout) + if stopErr != nil { + s.report(&SupervisorError{stopErr}) + } + }) + } + watchDone := make(chan struct{}) + watchExited := make(chan struct{}) + shutdownStarted := make(chan time.Time, 1) + go func() { + defer close(watchExited) + select { + case <-ctx.Done(): + shutdownStarted <- time.Now() + stop() + case <-watchDone: + } + }() + waitErr := cmd.Wait() + r.Cancelled = ctx.Err() != nil + r.Runtime = time.Since(start) + // Clear externally visible running/PID immediately, before group cleanup, + // output draining or backoff. Runtime excludes all of those activities. + if spec.OnExit != nil { + spec.OnExit(r.Runtime) + } + children.Lock() + if children.managed[pid] == cmd { + delete(children.managed, pid) + } + children.Unlock() + if cmd.ProcessState != nil { + r.ExitCode = cmd.ProcessState.ExitCode() + } + stop() + close(watchDone) + <-watchExited + select { + case <-drainDone: + case <-ctx.Done(): + // Cancellation bounds even escaped descendants retaining a pipe. Ordinary + // completion always drains to EOF. Report any forced truncation explicitly. + started := time.Now() + select { + case started = <-shutdownStarted: + default: + } + remaining := time.Until(started.Add(spec.StopTimeout)) + timer := time.NewTimer(remaining) + select { + case <-drainDone: + case <-timer.C: + s.diagnostic("Output drain exceeded shutdown deadline for PID %d; closing pipes (output may be truncated)", pid) + _ = outR.Close() + _ = errR.Close() + <-drainDone + } + timer.Stop() + } + r.Err = waitErr + if r.Cancelled { + r.Err = ctx.Err() + } + if stopErr != nil { + r.Err = errors.Join(r.Err, &SupervisorError{stopErr}) + } + for _, err := range outputErr { + if err != nil && !errors.Is(err, os.ErrClosed) { + r.Err = errors.Join(r.Err, &SupervisorError{fmt.Errorf("draining child output: %w", err)}) + } + } + return r +} +func stopGroup(pid int, sig syscall.Signal, timeout time.Duration) error { + if err := SignalGroup(pid, sig); err != nil { + return err + } + deadline := time.NewTimer(timeout) + defer deadline.Stop() + ticker := time.NewTicker(10 * time.Millisecond) + defer ticker.Stop() + for { + alive, err := groupAlive(pid) + if err != nil { + return err + } + if !alive { + return nil + } + select { + case <-ticker.C: + case <-deadline.C: + return SignalGroup(pid, syscall.SIGKILL) + } + } +} +func SignalGroup(pid int, sig syscall.Signal) error { + if pid <= 0 { + return nil + } + _, err := groupSignalResult(pid, syscall.Kill(-pid, sig)) + return err +} + +// ReportSignalError promotes a signal failure to a supervisor failure. +func (s *Service) ReportSignalError(err error) { s.report(&SupervisorError{err}) } diff --git a/internal/execution/service_test.go b/internal/execution/service_test.go new file mode 100644 index 0000000..b54447a --- /dev/null +++ b/internal/execution/service_test.go @@ -0,0 +1,86 @@ +package execution + +import ( + "bytes" + "context" + "io" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" +) + +func TestWaitDoesNotCloseUndrainedPipes(t *testing.T) { + service := NewService() + exited := make(chan struct{}) + var mu sync.Mutex + var data []byte + result := service.Run(context.Background(), Spec{Command: "printf '%04096d' 0", OnExit: func(time.Duration) { close(exited) }, Output: func(r io.Reader) error { + <-exited // intentionally do not read until cmd.Wait has returned + b, err := io.ReadAll(r) + mu.Lock() + data = append(data, b...) + mu.Unlock() + return err + }}) + if result.Err != nil { + t.Fatal(result.Err) + } + if string(data) != strings.Repeat("0", 4096) { + t.Fatalf("tail truncated: %d bytes", len(data)) + } +} +func TestLargeOutputDrainsWithoutDeadlock(t *testing.T) { + path := filepath.Join(t.TempDir(), "input") + input := bytes.Repeat([]byte("x"), 3*1024*1024+17) + if err := os.WriteFile(path, input, 0600); err != nil { + t.Fatal(err) + } + var mu sync.Mutex + var got []byte + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + result := NewService().Run(ctx, Spec{Command: "cat '" + path + "'", Output: func(r io.Reader) error { + b, err := io.ReadAll(r) + mu.Lock() + got = append(got, b...) + mu.Unlock() + return err + }}) + if result.Err != nil { + t.Fatal(result.Err) + } + if !bytes.Equal(input, got) { + t.Fatalf("got %d bytes, want %d", len(got), len(input)) + } +} +func TestCancellationWaitsForDirectChild(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + result := NewService().Run(ctx, Spec{Command: "exec sleep 30", OnStart: func(int) { cancel() }, StopTimeout: 50 * time.Millisecond}) + if result.Err != context.Canceled { + t.Fatalf("error=%v", result.Err) + } + children.Lock() + defer children.Unlock() + if len(children.managed) != 0 { + t.Fatal("managed child retained after wait") + } +} +func TestCredentialRejectsInvalidAndUnknownIDs(t *testing.T) { + for _, spec := range [][2]string{{"4294967295", "1"}, {"-1", "1"}, {"4294967296", "1"}, {"4000000000", ""}, {"", "1"}} { + if _, err := ResolveCredential(spec[0], spec[1]); err == nil { + t.Errorf("accepted %q", spec) + } + } +} + +func TestLaterCancellationPreservesObservedExitFailure(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + r := NewService().Run(ctx, Spec{Command: "exit 19", OnExit: func(time.Duration) { cancel() }}) + if r.Cancelled || r.ExitCode != 19 || r.Err == nil || r.Err == context.Canceled { + t.Fatalf("lost pre-cancellation failure: %+v", r) + } +} diff --git a/internal/health/regression_test.go b/internal/health/regression_test.go new file mode 100644 index 0000000..3c2d59d --- /dev/null +++ b/internal/health/regression_test.go @@ -0,0 +1,141 @@ +package health + +import ( + "context" + "crypto/ed25519" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "net" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strconv" + "strings" + "testing" + "time" + + "github.com/michaelishri/gonner/internal/config" + "github.com/michaelishri/gonner/internal/runner" +) + +func certificates(t *testing.T) (string, string) { + t.Helper() + pub, key, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + template := &x509.Certificate{SerialNumber: big.NewInt(1), Subject: pkix.Name{CommonName: "localhost"}, NotBefore: time.Now().Add(-time.Hour), NotAfter: time.Now().Add(time.Hour), DNSNames: []string{"localhost"}, KeyUsage: x509.KeyUsageDigitalSignature, ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}} + cert, err := x509.CreateCertificate(rand.Reader, template, template, pub, key) + if err != nil { + t.Fatal(err) + } + rawKey, err := x509.MarshalPKCS8PrivateKey(key) + if err != nil { + t.Fatal(err) + } + dir := t.TempDir() + certFile := filepath.Join(dir, "cert.pem") + keyFile := filepath.Join(dir, "key.pem") + if err := os.WriteFile(certFile, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: cert}), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(keyFile, pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: rawKey}), 0600); err != nil { + t.Fatal(err) + } + return certFile, keyFile +} +func TestTLSValidationIsSynchronousAndDoesNotLeakListener(t *testing.T) { + cert, key := certificates(t) + _, otherKey := certificates(t) + bad := filepath.Join(t.TempDir(), "bad.pem") + if err := os.WriteFile(bad, []byte("invalid"), 0600); err != nil { + t.Fatal(err) + } + for _, pair := range [][2]string{{"/gonner-missing-cert", key}, {bad, key}, {cert, otherKey}} { + port := freePort(t) + srv := NewServerWithOptions(Options{BindAddr: "127.0.0.1", Port: port, TLS: &config.TLSConfig{CertFile: pair[0], KeyFile: pair[1]}}, runner.NewManager(&config.Config{})) + if err := srv.Start(context.Background()); err == nil { + t.Fatal("invalid TLS accepted") + } + l, err := net.Listen("tcp", net.JoinHostPort("127.0.0.1", strconv.Itoa(port))) + if err != nil { + t.Fatalf("failed TLS leaked listener: %v", err) + } + _ = l.Close() + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + srv := NewServerWithOptions(Options{BindAddr: "127.0.0.1", Port: 0, TLS: &config.TLSConfig{CertFile: cert, KeyFile: key}}, runner.NewManager(&config.Config{})) + if err := srv.Start(ctx); err != nil { + t.Fatal(err) + } + client := &http.Client{Transport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}}, Timeout: time.Second} // test certificate + defer client.CloseIdleConnections() + resp, err := client.Get("https://" + srv.listener.Addr().String() + "/health") + if err != nil { + t.Fatal(err) + } + _ = resp.Body.Close() + if resp.StatusCode != 200 { + t.Fatal(resp.StatusCode) + } + cancel() + <-srv.Done() +} +func TestUnexpectedServeFailureIsReported(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + srv := NewServerWithOptions(Options{BindAddr: "127.0.0.1", Port: 0}, runner.NewManager(&config.Config{})) + if err := srv.Start(ctx); err != nil { + t.Fatal(err) + } + _ = srv.listener.Close() + select { + case err := <-srv.Errors(): + if err == nil { + t.Fatal("serve failure suppressed") + } + case <-time.After(time.Second): + t.Fatal("serve failure not reported") + } + <-srv.Done() +} +func TestReadyAndStatusIncludePendingAndSkippedCritical(t *testing.T) { + mgr := runner.NewManager(&config.Config{Run: []config.ProcessConfig{{Name: "critical", Command: "true", Critical: true, Instances: 2, WhenAll: []map[string]string{{"fileExists": "/gonner-missing"}}}, {Name: "other", Command: "exec sleep 30"}}, ShutdownTimeout: config.Duration(100 * time.Millisecond)}) + srv := NewServer(0, mgr) + check := func() { + t.Helper() + w := httptest.NewRecorder() + srv.handleReady(w, httptest.NewRequest("GET", "/ready", nil)) + if w.Code != 503 { + t.Fatalf("ready=%d", w.Code) + } + } + check() + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { done <- mgr.Run(ctx) }() + defer func() { cancel(); <-done }() + deadline := time.Now().Add(time.Second) + for mgr.Processes()[0].Status != runner.StateSkipped { + if time.Now().After(deadline) { + t.Fatal("critical condition did not resolve") + } + time.Sleep(time.Millisecond) + } + if mgr.IsShuttingDown() { + t.Fatal("intentional skip should not terminate unrelated work") + } + check() + w := httptest.NewRecorder() + srv.handleStatus(w, httptest.NewRequest("GET", "/status", nil)) + body := w.Body.String() + if !strings.Contains(body, `"status":"skipped"`) || !strings.Contains(body, `"instances":2`) { + t.Fatalf("status=%s", body) + } +} diff --git a/internal/health/server.go b/internal/health/server.go index 822f196..ad02ad0 100644 --- a/internal/health/server.go +++ b/internal/health/server.go @@ -8,6 +8,7 @@ import ( "fmt" "net" "net/http" + "strconv" "time" "github.com/michaelishri/gonner/internal/config" @@ -29,9 +30,12 @@ type Options struct { // Server is the HTTP health endpoint server. type Server struct { - opts Options - manager *runner.Manager - server *http.Server + opts Options + manager *runner.Manager + server *http.Server + listener net.Listener + errors chan error + done chan struct{} } // NewServer creates a new health server with default options. @@ -45,12 +49,23 @@ func NewServerWithOptions(opts Options, manager *runner.Manager) *Server { if opts.BindAddr == "" { opts.BindAddr = "0.0.0.0" } - return &Server{opts: opts, manager: manager} + return &Server{opts: opts, manager: manager, errors: make(chan error, 1), done: make(chan struct{})} } // Start starts the HTTP server in a goroutine. It shuts down gracefully when ctx is cancelled. // Returns an error if the server cannot bind to the configured address. func (s *Server) Start(ctx context.Context) error { + if err := ctx.Err(); err != nil { + return err + } + var tlsConfig *tls.Config + if s.opts.TLS != nil { + cert, err := tls.LoadX509KeyPair(s.opts.TLS.CertFile, s.opts.TLS.KeyFile) + if err != nil { + return fmt.Errorf("loading health TLS certificate: %w", err) + } + tlsConfig = &tls.Config{MinVersion: tls.VersionTLS12, Certificates: []tls.Certificate{cert}} + } mux := http.NewServeMux() mux.HandleFunc("/", s.handleRoot) mux.HandleFunc("/health", s.handleHealth) // public liveness probe @@ -60,7 +75,7 @@ func (s *Server) Start(ctx context.Context) error { mux.HandleFunc("/metrics", s.authMiddleware(s.handleMetrics)) } - addr := fmt.Sprintf("%s:%d", s.opts.BindAddr, s.opts.Port) + addr := net.JoinHostPort(s.opts.BindAddr, strconv.Itoa(s.opts.Port)) listener, err := net.Listen("tcp", addr) if err != nil { return fmt.Errorf("health endpoint failed to listen on %s: %w", addr, err) @@ -76,37 +91,32 @@ func (s *Server) Start(ctx context.Context) error { MaxHeaderBytes: 1 << 16, // 64 KiB } - // TLS hardening: require modern protocol versions when serving HTTPS. - if s.opts.TLS != nil { - s.server.TLSConfig = &tls.Config{MinVersion: tls.VersionTLS12} + s.server.TLSConfig = tlsConfig + if tlsConfig != nil { + listener = tls.NewListener(listener, tlsConfig) } - + s.listener = listener + serveDone := make(chan struct{}) go func() { - defer logging.Recover("health-server") - scheme := "http" - if s.opts.TLS != nil { - scheme = "https" - } - logging.Gonner("Health endpoint listening on %s://%s", scheme, addr) - - var serveErr error - if s.opts.TLS != nil { - serveErr = s.server.ServeTLS(listener, s.opts.TLS.CertFile, s.opts.TLS.KeyFile) - } else { - serveErr = s.server.Serve(listener) - } - if serveErr != nil && serveErr != http.ErrServerClosed { - logging.Gonner("Health server error: %v", serveErr) + defer close(serveDone) + defer close(s.errors) + logging.Gonner("Health endpoint listening on %s", listener.Addr()) + if err := s.server.Serve(listener); err != nil && err != http.ErrServerClosed { + s.errors <- fmt.Errorf("health server: %w", err) } }() - go func() { - defer logging.Recover("health-shutdown") - <-ctx.Done() - logging.Gonner("Shutting down health endpoint...") + defer close(s.done) + select { + case <-ctx.Done(): + case <-serveDone: + } shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - _ = s.server.Shutdown(shutdownCtx) + if err := s.server.Shutdown(shutdownCtx); err != nil { + _ = s.server.Close() + } + <-serveDone }() return nil @@ -128,3 +138,9 @@ func (s *Server) authMiddleware(next http.HandlerFunc) http.HandlerFunc { next(w, r) } } + +// Errors reports an unexpected serving failure; normal shutdown closes it. +func (s *Server) Errors() <-chan error { return s.errors } + +// Done is closed once the listener and active handlers have shut down. +func (s *Server) Done() <-chan struct{} { return s.done } diff --git a/internal/integration/docker_linux_test.go b/internal/integration/docker_linux_test.go new file mode 100644 index 0000000..5dcb804 --- /dev/null +++ b/internal/integration/docker_linux_test.go @@ -0,0 +1,300 @@ +//go:build linux + +package integration + +import ( + "bytes" + "context" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "sync" + "syscall" + "testing" + "time" + + "github.com/michaelishri/gonner/internal/condition" + "github.com/michaelishri/gonner/internal/config" + "github.com/michaelishri/gonner/internal/execution" + "github.com/michaelishri/gonner/internal/logging" + "github.com/michaelishri/gonner/internal/runner" +) + +func TestDockerPID1AndPrivilegeBoundaries(t *testing.T) { + if os.Getenv("GONNER_DOCKER_TESTS") != "1" { + t.Skip("set GONNER_DOCKER_TESTS=1; requires Docker and alpine:3.21") + } + dir := t.TempDir() + if err := os.Chmod(dir, 0755); err != nil { + t.Fatal(err) + } + binary := filepath.Join(dir, "integration.test") + build := exec.Command("go", "test", "-c", "-o", binary, ".") + build.Env = append(os.Environ(), "CGO_ENABLED=0") + if out, err := build.CombinedOutput(); err != nil { + t.Fatalf("build: %v\n%s", err, out) + } + for name, data := range map[string]string{"passwd": "root:x:0:0::/root:/bin/sh\nworker:x:2001:3001::/tmp:/bin/sh\n", "group": "root:x:0:\nprimary:x:3001:\noverride:x:3002:\n"} { + if err := os.WriteFile(filepath.Join(dir, name), []byte(data), 0644); err != nil { + t.Fatal(err) + } + } + for _, mode := range []string{"stress", "credentials", "missing-kill", "files", "conditions"} { + t.Run(mode, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) + defer cancel() + name := "gonner-audit-" + mode + "-" + strconv.FormatInt(time.Now().UnixNano(), 36) + t.Cleanup(func() { + cleanup, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + _ = exec.CommandContext(cleanup, "docker", "rm", "-f", name).Run() + }) + args := []string{"run", "--rm", "--name", name, "--network", "none", "--read-only", "--pids-limit", "2048", "--tmpfs", "/tmp:rw,nosuid,nodev,mode=1777", "--cap-drop", "ALL", "--cap-add", "SETUID", "--cap-add", "SETGID", "--cap-add", "CHOWN"} + if mode != "missing-kill" { + args = append(args, "--cap-add", "KILL") + } + args = append(args, "--mount", "type=bind,src="+dir+",dst=/fixture,readonly", "--mount", "type=bind,src="+filepath.Join(dir, "passwd")+",dst=/etc/passwd,readonly", "--mount", "type=bind,src="+filepath.Join(dir, "group")+",dst=/etc/group,readonly", "-e", "GONNER_CONTAINER_HELPER="+mode, "-e", "GOMAXPROCS=4", "alpine:3.21", "/fixture/integration.test", "-test.run=^TestContainerHelper$", "-test.v", "-test.timeout=150s") + out, err := exec.CommandContext(ctx, "docker", args...).CombinedOutput() + if err != nil { + t.Fatalf("container %s: %v\n%s", mode, err, out) + } + t.Logf("%s passed in isolated PID namespace", mode) + }) + } +} +func TestContainerHelper(t *testing.T) { + mode := os.Getenv("GONNER_CONTAINER_HELPER") + if mode == "" { + t.Skip("container helper") + } + if os.Getpid() != 1 { + t.Fatal("test must actually run as PID 1") + } + switch mode { + case "stress": + testStress(t) + case "credentials": + testCredentials(t) + case "missing-kill": + testMissingKill(t) + case "files": + testUnsafeFiles(t) + case "conditions": + testConditionGroups(t) + default: + t.Fatalf("unknown mode %q", mode) + } +} +func testStress(t *testing.T) { + // 512 command probes + 2,048 precommands + 1,024 main shells. Their fast + // exits compete with SIGCHLD delivery and adopted grandchildren. + cfg := &config.Config{ShutdownTimeout: config.Duration(100 * time.Millisecond)} + for i := 0; i < 256; i++ { + cfg.Run = append(cfg.Run, config.ProcessConfig{Name: fmt.Sprintf("p%d", i), Command: "true", Instances: 4, WhenAll: []map[string]string{{"commandSucceeds": "true"}, {"commandSucceeds": "true"}}, CommandsBefore: []config.PreCommand{{Command: "true"}, {Command: "true"}}}) + } + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Second) + defer cancel() + mgr := runner.NewManager(cfg) + if err := mgr.Run(ctx); err != nil { + t.Fatalf("managed exit status stolen: %v", err) + } + for _, p := range mgr.Processes() { + if p.Status != "stopped" || p.Instances != 4 { + t.Fatalf("unexpected status: %+v", p) + } + } + cfg = &config.Config{Run: []config.ProcessConfig{{Name: "bad-main", Command: "exit 23"}, {Name: "bad-pre", Command: "true", CommandsBefore: []config.PreCommand{{Command: "exit 19"}}}, {Name: "false-probe", Command: "exit 99", WhenAll: []map[string]string{{"commandSucceeds": "exit 27"}}}}} + err := runner.NewManager(cfg).Run(ctx) + if err == nil || !strings.Contains(err.Error(), "exit status 23") || !strings.Contains(err.Error(), "exit status 19") { + t.Fatalf("nonzero statuses lost: %v", err) + } + noChildren(t) +} +func testCredentials(t *testing.T) { + // A differing UID/GID proves numeric lookup uses passwd rather than uid=gid. + cred, err := execution.ResolveCredential("2001", "") + if err != nil { + t.Fatal(err) + } + if cred.Uid != 2001 || cred.Gid != 3001 { + t.Fatalf("credential=%+v", cred) + } + named, err := execution.ResolveCredential("worker", "override") + if err != nil || named.Gid != 3002 { + t.Fatalf("override=%+v %v", named, err) + } + unknown, err := execution.ResolveCredential("4000000000", "3001") + if err != nil || unknown.Gid != 3001 { + t.Fatalf("unknown UID with explicit GID: %+v %v", unknown, err) + } + if _, err := execution.ResolveCredential("4000000000", ""); err == nil { + t.Fatal("unknown UID without GID accepted") + } + if err := syscall.Setgroups([]int{3002}); err != nil { + t.Fatal(err) + } + var output bytes.Buffer + var mu sync.Mutex + service := execution.NewService() + r := service.Run(context.Background(), execution.Spec{Command: "id -u; id -g; id -G", Credential: cred, Output: func(r io.Reader) error { + b, err := io.ReadAll(r) + mu.Lock() + _, _ = output.Write(b) + mu.Unlock() + return err + }}) + if r.Err != nil || output.String() != "2001\n3001\n3001\n" { + t.Fatalf("credential result: %q %v", output.String(), r.Err) + } + ctx, cancel := context.WithCancel(context.Background()) + r = service.Run(ctx, execution.Spec{Command: "exec sleep 30", Credential: cred, StopTimeout: 100 * time.Millisecond, OnStart: func(int) { cancel() }}) + if r.Err != context.Canceled { + t.Fatalf("cross-UID shutdown: %v", r.Err) + } + noChildren(t) +} +func testMissingKill(t *testing.T) { + if _, err := execution.ResolveCredential("2001", ""); err == nil || !strings.Contains(err.Error(), "CAP_KILL") { + t.Fatalf("missing KILL not rejected: %v", err) + } + cfg := &config.Config{Run: []config.ProcessConfig{{Name: "probe", Command: "true", WhenAll: []map[string]string{{"commandSucceeds": "touch /tmp/probe-ran"}}}, {Name: "child", Command: "touch /tmp/child-ran; exec sleep 30", User: "2001"}}} + if err := runner.NewManager(cfg).Run(context.Background()); err == nil { + t.Fatal("manager accepted missing KILL") + } + for _, name := range []string{"probe-ran", "child-ran"} { + if _, err := os.Stat("/tmp/" + name); !os.IsNotExist(err) { + t.Fatal("workload started before privilege preflight") + } + } + noChildren(t) +} +func testUnsafeFiles(t *testing.T) { + if err := os.Mkdir("/tmp/logs", 0700); err != nil { + t.Fatal(err) + } + victim := "/tmp/victim" + if err := os.WriteFile(victim, []byte("private"), 0600); err != nil { + t.Fatal(err) + } + for _, kind := range []string{"symlink", "hardlink", "fifo", "device", "owned-dir", "writable-dir"} { + path := "/tmp/logs/" + kind + switch kind { + case "symlink": + if err := os.Symlink(victim, path); err != nil { + t.Fatal(err) + } + case "hardlink": + if err := os.Link(victim, path); err != nil { + t.Fatal(err) + } + case "fifo": + if err := syscall.Mkfifo(path, 0600); err != nil { + t.Fatal(err) + } + case "device": + path = "/dev/null" + case "owned-dir": + _ = os.Mkdir(path, 0700) + if err := os.Chown(path, 2001, 3001); err != nil { + t.Fatal(err) + } + path += "/app.log" + case "writable-dir": + _ = os.Mkdir(path, 0777) + _ = os.Chmod(path, 0777) + path += "/app.log" + } + if w, err := logging.NewWriter("root", path); err == nil { + _ = w.Close() + t.Fatalf("accepted unsafe %s", kind) + } + } + b, _ := os.ReadFile(victim) + st, _ := os.Stat(victim) + if string(b) != "private" || st.Mode().Perm() != 0600 { + t.Fatal("victim altered") + } + cfg := &config.Config{Run: []config.ProcessConfig{{Name: "probe", Command: "true", WhenAll: []map[string]string{{"commandSucceeds": "touch /tmp/unsafe-probe-ran"}}}, {Name: "unsafe", Command: "true", LogFile: "/tmp/logs/owned-dir/app.log"}}} + if err := runner.NewManager(cfg).Run(context.Background()); err == nil { + t.Fatal("unsafe log directory accepted") + } + if _, err := os.Stat("/tmp/unsafe-probe-ran"); !os.IsNotExist(err) { + t.Fatal("condition ran before path validation") + } +} +func testConditionGroups(t *testing.T) { + service := execution.NewService() + join, err := service.StartReaper(context.Background()) + if err != nil { + t.Fatal(err) + } + defer join() + for _, cancelled := range []bool{false, true} { + marker := fmt.Sprintf("/tmp/child-%v", cancelled) + command := "sleep 30 & echo $! > " + marker + ctx, cancel := context.WithCancel(context.Background()) + if cancelled { + command += "; wait" + go func() { + for { + if _, err := os.Stat(marker); err == nil { + cancel() + return + } + select { + case <-ctx.Done(): + return + case <-time.After(time.Millisecond): + } + } + }() + } + ok, _, err := condition.NewEvaluator(service, execution.Spec{}).ShouldRun(ctx, []map[string]string{{"commandSucceeds": command}}, nil) + cancel() + if cancelled { + if err != context.Canceled && !strings.Contains(fmt.Sprint(err), "context canceled") { + t.Fatalf("cancelled probe: %v", err) + } + } else if !ok || err != nil { + t.Fatalf("normal probe: %v %v", ok, err) + } + } + // Normal main and precommand exits also clean up background descendants. + cfg := &config.Config{ShutdownTimeout: config.Duration(50 * time.Millisecond), Run: []config.ProcessConfig{{Name: "orphans", Command: "sleep 30 & true", CommandsBefore: []config.PreCommand{{Command: "sleep 30 & true"}}}}} + if err := runner.NewManager(cfg).Run(context.Background()); err != nil { + t.Fatal(err) + } + noChildren(t) +} +func noChildren(t *testing.T) { + t.Helper() + deadline := time.Now().Add(3 * time.Second) + for { + paths, err := filepath.Glob("/proc/self/task/*/children") + if err != nil { + t.Fatal(err) + } + var live []string + for _, path := range paths { + b, err := os.ReadFile(path) + if os.IsNotExist(err) { + continue + } + if err != nil { + t.Fatal(err) + } + live = append(live, strings.Fields(string(b))...) + } + if len(live) == 0 { + return + } + if time.Now().After(deadline) { + t.Fatalf("unreaped or live descendants: %v", live) + } + time.Sleep(10 * time.Millisecond) + } +} diff --git a/internal/logging/regression_test.go b/internal/logging/regression_test.go new file mode 100644 index 0000000..8a12422 --- /dev/null +++ b/internal/logging/regression_test.go @@ -0,0 +1,201 @@ +package logging + +import ( + "bytes" + "compress/gzip" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "sync" + "testing" +) + +func secureTempDir(t *testing.T) string { + t.Helper() + dir := t.TempDir() + if err := os.Chmod(dir, 0700); err != nil { + t.Fatal(err) + } + return dir +} +func quietWriter(t *testing.T, opts Options) *Writer { + t.Helper() + w, err := NewWriterWithOptions(opts) + if err != nil { + t.Fatal(err) + } + w.stdout = io.Discard + t.Cleanup(func() { _ = w.Close() }) + return w +} +func TestOversizedAndUnterminatedOutputIsByteExact(t *testing.T) { + path := filepath.Join(secureTempDir(t), "output.log") + w := quietWriter(t, Options{ProcessName: "large", LogFilePath: path}) + var console bytes.Buffer + w.stdout = &console + input := bytes.Repeat([]byte("x"), 3*1024*1024+17) + input = append(input, []byte("\nlast line without newline")...) + if err := LineScanner(bytes.NewReader(input), w); err != nil { + t.Fatal(err) + } + got, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(got, input) { + t.Fatalf("raw length=%d want=%d", len(got), len(input)) + } + if n := strings.Count(console.String(), "[large]"); n != 50 { + t.Fatalf("chunk prefixes=%d want=50", n) + } +} +func TestRapidRotationAndSharedSinkPreserveEveryByte(t *testing.T) { + for _, compress := range []bool{false, true} { + t.Run(fmt.Sprint(compress), func(t *testing.T) { + dir := secureTempDir(t) + path := filepath.Join(dir, "shared.log") + const workers = 4 + const writes = 20 + chunkSize := 96 * 1024 + var wg sync.WaitGroup + for i := 0; i < workers; i++ { + w := quietWriter(t, Options{ProcessName: fmt.Sprint(i), LogFilePath: path, Rotate: &RotateOptions{MaxSizeMB: 1, Compress: compress}}) + wg.Go(func() { + data := bytes.Repeat([]byte{byte('A' + i)}, chunkSize) + for j := 0; j < writes; j++ { + if _, err := w.Write(data); err != nil { + t.Error(err) + return + } + } + }) + } + wg.Wait() + var counts [workers]int + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + if len(entries) < 5 { + t.Fatalf("only %d files after rapid rotation", len(entries)) + } + for _, entry := range entries { + f, err := os.Open(filepath.Join(dir, entry.Name())) + if err != nil { + t.Fatal(err) + } + var r io.Reader = f + var gz *gzip.Reader + if strings.HasSuffix(entry.Name(), ".gz") { + gz, err = gzip.NewReader(f) + if err != nil { + t.Fatal(err) + } + r = gz + } + data, err := io.ReadAll(r) + if err != nil { + t.Fatal(err) + } + if gz != nil { + _ = gz.Close() + } + _ = f.Close() + for _, b := range data { + if b < 'A' || b >= 'A'+workers { + t.Fatalf("unexpected byte %q", b) + } + counts[b-'A']++ + } + } + for i, n := range counts { + if n != writes*chunkSize { + t.Errorf("worker%d bytes=%d want=%d", i, n, writes*chunkSize) + } + } + }) + } +} +func TestSharedPathRejectsConflictingSettings(t *testing.T) { + dir := secureTempDir(t) + path := filepath.Join(dir, "same.log") + quietWriter(t, Options{ProcessName: "a", LogFilePath: path}) + _, err := NewWriterWithOptions(Options{ProcessName: "b", LogFilePath: filepath.Join(dir, ".", "same.log"), LogFileMode: 0640}) + if err == nil { + t.Fatal("accepted conflicting settings") + } +} +func TestPruningPreservesUnrelatedLegacyAndActivePaths(t *testing.T) { + dir := secureTempDir(t) + base := "app.log" + protected := []string{base + ".notes", base + ".20260101T000000Z", base + ".gonner-invalid.log", base + ".gonner-20000101T000000.000000000Z-" + strings.Repeat("0", 32) + ".log"} + for _, name := range protected { + if err := os.WriteFile(filepath.Join(dir, name), []byte("keep"), 0600); err != nil { + t.Fatal(err) + } + } + quietWriter(t, Options{ProcessName: "other", LogFilePath: filepath.Join(dir, protected[3])}) + w := quietWriter(t, Options{ProcessName: "a", LogFilePath: filepath.Join(dir, base), Rotate: &RotateOptions{MaxSizeMB: 1, MaxBackups: 1}}) + for i := 0; i < 6; i++ { + if _, err := w.Write(bytes.Repeat([]byte("x"), 700*1024)); err != nil { + t.Fatal(err) + } + } + for _, name := range protected { + b, err := os.ReadFile(filepath.Join(dir, name)) + if err != nil || string(b) != "keep" { + t.Errorf("protected %s changed: %q, %v", name, b, err) + } + } + entries, _ := os.ReadDir(dir) + if len(entries) != len(protected)+2 { + t.Fatalf("backup retention: %d entries", len(entries)) + } +} +func TestFileErrorStillWritesConsoleAndRecovers(t *testing.T) { + w := quietWriter(t, Options{ProcessName: "failure", LogFilePath: filepath.Join(secureTempDir(t), "app.log")}) + var console bytes.Buffer + w.stdout = &console + original := w.sink.file + closed, err := os.CreateTemp(t.TempDir(), "closed") + if err != nil { + t.Fatal(err) + } + _ = closed.Close() + w.sink.file = closed + if _, err := w.Write([]byte("visible\n")); err == nil { + t.Fatal("expected file failure") + } + if !strings.Contains(console.String(), "visible") { + t.Fatal("console lost output") + } + w.sink.file = original + if _, err := w.Write([]byte("recovered\n")); err != nil { + t.Fatal(err) + } + if !w.lastError.IsZero() { + t.Fatal("error state did not recover") + } +} +func TestRotationFailureKeepsOpenSinkUsable(t *testing.T) { + dir := secureTempDir(t) + path := filepath.Join(dir, "app.log") + w := quietWriter(t, Options{ProcessName: "a", LogFilePath: path, Rotate: &RotateOptions{MaxSizeMB: 1}}) + data := bytes.Repeat([]byte("x"), 700*1024) + if _, err := w.Write(data); err != nil { + t.Fatal(err) + } + saved := filepath.Join(dir, "saved.log") + if err := os.Rename(path, saved); err != nil { + t.Fatal(err) + } + if _, err := w.Write(data); err == nil { + t.Fatal("expected rotation failure") + } + b, err := os.ReadFile(saved) + if err != nil || len(b) != 2*len(data) { + t.Fatalf("open sink lost: %d %v", len(b), err) + } +} diff --git a/internal/logging/rotation_test.go b/internal/logging/rotation_test.go index a431780..3cb9f98 100644 --- a/internal/logging/rotation_test.go +++ b/internal/logging/rotation_test.go @@ -9,7 +9,7 @@ import ( ) func TestWriter_FilePermissions(t *testing.T) { - dir := t.TempDir() + dir := secureTempDir(t) logPath := filepath.Join(dir, "secure.log") w, err := NewWriterWithOptions(Options{ProcessName: "p", LogFilePath: logPath}) @@ -31,19 +31,14 @@ func TestWriter_FilePermissions(t *testing.T) { } func TestWriter_Rotation(t *testing.T) { - dir := t.TempDir() + dir := secureTempDir(t) logPath := filepath.Join(dir, "rotate.log") - w := &Writer{ - processName: "p", - stdout: io.Discard, - logFilePath: logPath, - logFileMode: 0o600, - rotate: &RotateOptions{MaxSizeMB: 1, MaxBackups: 2}, - } - if err := w.openLogFile(); err != nil { - t.Fatalf("new: %v", err) + w, err := NewWriterWithOptions(Options{ProcessName: "p", LogFilePath: logPath, Rotate: &RotateOptions{MaxSizeMB: 1, MaxBackups: 2}}) + if err != nil { + t.Fatal(err) } + w.stdout = io.Discard defer w.Close() // Write > 1 MB to trigger rotation. diff --git a/internal/logging/writer.go b/internal/logging/writer.go index 0023854..d735f8a 100644 --- a/internal/logging/writer.go +++ b/internal/logging/writer.go @@ -1,247 +1,300 @@ -// Package logging provides multiplexed log writers for gonner processes. +// Package logging multiplexes bounded output chunks to console and shared files. package logging import ( "bufio" "compress/gzip" + "crypto/rand" + "errors" "fmt" "io" "os" "path/filepath" + "regexp" "sort" "strings" "sync" "time" + + "github.com/michaelishri/gonner/internal/safefile" ) -// RotateOptions configures size-based log rotation. type RotateOptions struct { - MaxSizeMB int // rotate when file exceeds this size (0 = disabled) - MaxBackups int // number of rotated files to retain (0 = unlimited) - Compress bool // gzip rotated files + MaxSizeMB, MaxBackups int + Compress bool } - -// Options configures a Writer. type Options struct { - ProcessName string - LogFilePath string - LogFileMode os.FileMode // default 0o600 if 0 - Rotate *RotateOptions + ProcessName, LogFilePath string + LogFileMode os.FileMode + Rotate *RotateOptions } - -// Writer is a multiplexed writer that writes process output to both -// gonner's stdout (with prefix) and an optional log file (raw). type Writer struct { processName string stdout io.Writer - logFile *os.File - logFilePath string - logFileMode os.FileMode - rotate *RotateOptions - written int64 + sink *fileSink mu sync.Mutex + closed bool + lastError time.Time } - -// NewWriter creates a multiplexed writer with default settings. -// Kept for backwards compatibility; for new code, use NewWriterWithOptions. -func NewWriter(processName, logFilePath string) (*Writer, error) { - return NewWriterWithOptions(Options{ - ProcessName: processName, - LogFilePath: logFilePath, - }) +type fileSink struct { + mu sync.Mutex + dir *safefile.Directory + name, key string + file *os.File + mode os.FileMode + rotate RotateOptions + written int64 + refs int } -// NewWriterWithOptions creates a multiplexed writer using the provided options. +var sinks = struct { + sync.Mutex + files map[string]*fileSink +}{files: make(map[string]*fileSink)} +var activePaths sync.Map +var consoleMu sync.Mutex + +func NewWriter(name, path string) (*Writer, error) { + return NewWriterWithOptions(Options{ProcessName: name, LogFilePath: path}) +} func NewWriterWithOptions(opts Options) (*Writer, error) { + w := &Writer{processName: opts.ProcessName, stdout: os.Stdout} + if opts.LogFilePath == "" { + return w, nil + } mode := opts.LogFileMode if mode == 0 { - mode = 0o600 + mode = 0600 } - w := &Writer{ - processName: opts.ProcessName, - stdout: os.Stdout, - logFilePath: opts.LogFilePath, - logFileMode: mode, - rotate: opts.Rotate, + dir, err := safefile.OpenDirectory(filepath.Dir(opts.LogFilePath)) + if err != nil { + return nil, fmt.Errorf("log directory: %w", err) } - - if opts.LogFilePath != "" { - if err := w.openLogFile(); err != nil { - return nil, err - } + name := filepath.Base(opts.LogFilePath) + key := dir.Key + "/" + name + rotate := RotateOptions{} + if opts.Rotate != nil { + rotate = *opts.Rotate } - - return w, nil -} - -func (w *Writer) openLogFile() error { - dir := filepath.Dir(w.logFilePath) - if err := os.MkdirAll(dir, 0o750); err != nil { - return fmt.Errorf("creating log directory %s: %w", dir, err) + sinks.Lock() + defer sinks.Unlock() + if s := sinks.files[key]; s != nil { + _ = dir.Close() + if s.mode != mode || s.rotate != rotate { + return nil, fmt.Errorf("conflicting log settings for %s", opts.LogFilePath) + } + s.refs++ + w.sink = s + return w, nil } - - f, err := os.OpenFile(w.logFilePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, w.logFileMode) + f, err := dir.OpenRegular(name, os.O_WRONLY|os.O_CREATE|os.O_APPEND, mode) if err != nil { - return fmt.Errorf("opening log file %s: %w", w.logFilePath, err) - } - - // Enforce permissions even if the file existed. - if err := os.Chmod(w.logFilePath, w.logFileMode); err != nil { - Gonner("warning: chmod %s: %v", w.logFilePath, err) + _ = dir.Close() + return nil, fmt.Errorf("opening log: %w", err) } - - w.logFile = f - - if info, err := f.Stat(); err == nil { - w.written = info.Size() + st, err := f.Stat() + if err != nil { + _ = f.Close() + _ = dir.Close() + return nil, err } - - return nil + s := &fileSink{dir: dir, name: name, key: key, file: f, mode: mode, rotate: rotate, written: st.Size(), refs: 1} + sinks.files[key] = s + activePaths.Store(key, true) + w.sink = s + return w, nil } -// Write implements io.Writer. Each write is prefixed with timestamp and process name -// on stdout, and written raw to the log file. Performs rotation if configured. -func (w *Writer) Write(p []byte) (n int, err error) { +// Write always attempts both destinations. A file error never stops console +// output or future file retries. Callers must keep draining after write errors. +func (w *Writer) Write(p []byte) (int, error) { w.mu.Lock() defer w.mu.Unlock() - - if w.logFile != nil { - if err := w.maybeRotate(len(p)); err != nil { - Gonner("warning: log rotation failed for %s: %v", w.processName, err) - } - if _, werr := w.logFile.Write(p); werr != nil { - return 0, fmt.Errorf("writing to log file: %w", werr) - } - w.written += int64(len(p)) + if w.closed { + return 0, os.ErrClosed } - - ts := time.Now().UTC().Format(time.RFC3339) - prefix := fmt.Sprintf("[%s] [%s] ", ts, w.processName) - if _, err := fmt.Fprintf(w.stdout, "%s%s", prefix, p); err != nil { - return 0, fmt.Errorf("writing to stdout: %w", err) + var fileErr error + if w.sink != nil { + fileErr = w.sink.write(p) } - - return len(p), nil + consoleMu.Lock() + _, consoleErr := fmt.Fprintf(w.stdout, "[%s] [%s] %s", time.Now().UTC().Format(time.RFC3339), w.processName, p) + // Continuation chunks and unterminated final lines get a console newline; + // raw files receive exactly p, including its original lack of a newline. + if len(p) > 0 && p[len(p)-1] != '\n' { + _, err := fmt.Fprintln(w.stdout) + consoleErr = errors.Join(consoleErr, err) + } + consoleMu.Unlock() + err := errors.Join(fileErr, consoleErr) + now := time.Now() + if err != nil { + if w.lastError.IsZero() || now.Sub(w.lastError) >= time.Minute { + Gonner("Output error for %q (continuing to drain): %v", w.processName, err) + w.lastError = now + } + } else if !w.lastError.IsZero() { + Gonner("Output recovered for %q", w.processName) + w.lastError = time.Time{} + } + return len(p), err } - -// maybeRotate rotates the active log file if it would exceed the configured size. -// Called with the lock held. -func (w *Writer) maybeRotate(incoming int) error { - if w.rotate == nil || w.rotate.MaxSizeMB <= 0 || w.logFile == nil { - return nil +func (s *fileSink) write(p []byte) error { + s.mu.Lock() + defer s.mu.Unlock() + var rotationErr error + if s.rotate.MaxSizeMB > 0 && s.written > 0 && s.written+int64(len(p)) > int64(s.rotate.MaxSizeMB)*1024*1024 { + rotationErr = s.rotateFile() } - limit := int64(w.rotate.MaxSizeMB) * 1024 * 1024 - if w.written+int64(incoming) <= limit { - return nil + n, err := s.file.Write(p) + s.written += int64(n) + if err == nil && n != len(p) { + err = io.ErrShortWrite } - - if err := w.logFile.Close(); err != nil { + return errors.Join(rotationErr, err) +} +func (s *fileSink) rotateFile() error { + var nonce [16]byte + if _, err := rand.Read(nonce[:]); err != nil { return err } - - ts := time.Now().UTC().Format("20060102T150405Z") - rotated := w.logFilePath + "." + ts - if err := os.Rename(w.logFilePath, rotated); err != nil { + backup := fmt.Sprintf("%s.gonner-%s-%x.log", s.name, time.Now().UTC().Format("20060102T150405.000000000Z"), nonce) + // Link reserves the backup exclusively. Retain the original fd until its + // replacement is fully opened, and roll back if opening fails. + if err := s.dir.Link(s.name, backup); err != nil { return err } - - if w.rotate.Compress { - if err := gzipFile(rotated); err != nil { - Gonner("warning: gzip %s: %v", rotated, err) - } else { - _ = os.Remove(rotated) - } - } - - if w.rotate.MaxBackups > 0 { - w.pruneBackups() + if err := s.dir.Remove(s.name); err != nil { + _ = s.dir.Remove(backup) + return err } - - w.written = 0 - return w.openLogFile() -} - -func (w *Writer) pruneBackups() { - dir := filepath.Dir(w.logFilePath) - base := filepath.Base(w.logFilePath) - entries, err := os.ReadDir(dir) + next, err := s.dir.OpenRegular(s.name, os.O_WRONLY|os.O_CREATE|os.O_EXCL|os.O_APPEND, s.mode) if err != nil { - return - } - var matches []string - for _, e := range entries { - name := e.Name() - if name == base { - continue + if restoreErr := s.dir.Link(backup, s.name); restoreErr == nil { + _ = s.dir.Remove(backup) } - if strings.HasPrefix(name, base+".") { - matches = append(matches, filepath.Join(dir, name)) - } - } - sort.Strings(matches) - if len(matches) <= w.rotate.MaxBackups { - return + return err } - excess := matches[:len(matches)-w.rotate.MaxBackups] - for _, p := range excess { - _ = os.Remove(p) + old := s.file + s.file = next + s.written = 0 + closeErr := old.Close() + if s.rotate.Compress { + if err := s.compress(backup); err != nil { + return errors.Join(closeErr, err) + } } + return errors.Join(closeErr, s.pruneBackups()) } - -func gzipFile(path string) error { - in, err := os.Open(path) +func (s *fileSink) compress(name string) error { + in, err := s.dir.OpenRegular(name, os.O_RDONLY, s.mode) if err != nil { return err } defer in.Close() - - out, err := os.OpenFile(path+".gz", os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600) + temp := name + ".gz.tmp" + out, err := s.dir.OpenRegular(temp, os.O_WRONLY|os.O_CREATE|os.O_EXCL, s.mode) if err != nil { return err } - defer out.Close() - + defer s.dir.Remove(temp) gz := gzip.NewWriter(out) - if _, err := io.Copy(gz, in); err != nil { - _ = gz.Close() + _, copyErr := io.Copy(gz, in) + gzipErr := gz.Close() + closeErr := out.Close() + if err = errors.Join(copyErr, gzipErr, closeErr); err != nil { + return err + } + if err = s.dir.Link(temp, name+".gz"); err != nil { return err } - return gz.Close() + if err = s.dir.Remove(temp); err != nil { + return err + } + return s.dir.Remove(name) +} +func (s *fileSink) pruneBackups() error { + if s.rotate.MaxBackups <= 0 { + return nil + } + entries, err := s.dir.Entries() + if err != nil { + return err + } + pattern := regexp.MustCompile("^" + regexp.QuoteMeta(s.name) + `\.gonner-[0-9]{8}T[0-9]{6}\.[0-9]{9}Z-[0-9a-f]{32}\.log(\.gz)?$`) + var backups []string + for _, entry := range entries { + n := entry.Name() + if !pattern.MatchString(n) || !entry.Type().IsRegular() { + continue + } + if _, active := activePaths.Load(s.dir.Key + "/" + n); active { + continue + } + // Descriptor-relative validation excludes links, devices and untrusted files. + f, e := s.dir.OpenRegular(n, os.O_RDONLY, s.mode) + if e != nil { + continue + } + _ = f.Close() + backups = append(backups, n) + } + sort.Strings(backups) + for len(backups) > s.rotate.MaxBackups { + if err := s.dir.Remove(backups[0]); err != nil { + return err + } + backups = backups[1:] + } + return nil } - -// Close closes the underlying log file if one was opened. func (w *Writer) Close() error { w.mu.Lock() defer w.mu.Unlock() - if w.logFile != nil { - err := w.logFile.Close() - w.logFile = nil - return err + if w.closed { + return nil } - return nil + w.closed = true + if w.sink == nil { + return nil + } + s := w.sink + sinks.Lock() + defer sinks.Unlock() + s.refs-- + if s.refs > 0 { + return nil + } + delete(sinks.files, s.key) + activePaths.Delete(s.key) + s.mu.Lock() + defer s.mu.Unlock() + return errors.Join(s.file.Close(), s.dir.Close()) } -// LineScanner reads from a reader line-by-line and writes each line to the writer. -// This ensures log prefixes are applied cleanly per line rather than mid-line. -// It blocks until the reader is exhausted or an error occurs. -func LineScanner(r io.Reader, w *Writer) { - scanner := bufio.NewScanner(r) - scanner.Buffer(make([]byte, 64*1024), 1024*1024) - for scanner.Scan() { - line := scanner.Text() - _, _ = w.Write([]byte(line + "\n")) +// LineScanner preserves all bytes, using at most 64 KiB for each stream chunk. +// ReadSlice returns oversized lines in fragments instead of stopping the drain. +// Destination failures are reported by Writer; only read errors end a stream. +func LineScanner(r io.Reader, w *Writer) error { + reader := bufio.NewReaderSize(r, 64*1024) + for { + part, err := reader.ReadSlice('\n') + if len(part) > 0 { + _, _ = w.Write(part) + } + if err == nil || errors.Is(err, bufio.ErrBufferFull) { + continue + } + if errors.Is(err, io.EOF) { + return nil + } + return err } } - -// Gonner writes gonner's own operational messages to stderr. -func Gonner(format string, args ...interface{}) { - ts := time.Now().UTC().Format(time.RFC3339) +func Gonner(format string, args ...any) { msg := fmt.Sprintf(format, args...) - fmt.Fprintf(os.Stderr, "[%s] [gonner] %s\n", ts, msg) + fmt.Fprintf(os.Stderr, "[%s] [gonner] %s\n", time.Now().UTC().Format(time.RFC3339), strings.TrimSuffix(msg, "\n")) } - -// Recover is a panic recovery helper for long-running goroutines. -// Use as: defer logging.Recover("name"). func Recover(name string) { if r := recover(); r != nil { Gonner("PANIC in %s: %v", name, r) diff --git a/internal/logging/writer_test.go b/internal/logging/writer_test.go index 9bc4115..9f9aed8 100644 --- a/internal/logging/writer_test.go +++ b/internal/logging/writer_test.go @@ -34,7 +34,7 @@ func TestWriter_StdoutPrefix(t *testing.T) { } func TestWriter_LogFile(t *testing.T) { - dir := t.TempDir() + dir := secureTempDir(t) logPath := filepath.Join(dir, "test.log") w, err := NewWriter("proc", logPath) @@ -68,13 +68,13 @@ func TestWriter_NoLogFile(t *testing.T) { } defer w.Close() - if w.logFile != nil { + if w.sink != nil { t.Error("logFile should be nil when no path is given") } } func TestWriter_LogFileCreatesDirectory(t *testing.T) { - dir := t.TempDir() + dir := secureTempDir(t) logPath := filepath.Join(dir, "sub", "dir", "test.log") w, err := NewWriter("proc", logPath) diff --git a/internal/runner/backoff.go b/internal/runner/backoff.go index f09dd3c..b65d7cc 100644 --- a/internal/runner/backoff.go +++ b/internal/runner/backoff.go @@ -16,7 +16,6 @@ type Backoff struct { maxDelay time.Duration multiplier float64 attempt int - lastStart time.Time } // NewBackoff creates a Backoff from config. If cfg is nil, defaults are used. @@ -32,14 +31,12 @@ func NewBackoff(cfg *config.BackoffConfig) *Backoff { } } -// RecordStart records that the process has just started. -// If it was running longer than maxDelay, the backoff counter resets (stability). -func (b *Backoff) RecordStart() { - now := time.Now() - if !b.lastStart.IsZero() && now.Sub(b.lastStart) > b.maxDelay { +// RecordExit resets escalation only after actual stable runtime. Restart delay +// and post-exit cleanup must never count toward stability. +func (b *Backoff) RecordExit(runtime time.Duration) { + if runtime > b.maxDelay { b.attempt = 0 } - b.lastStart = now } // Next returns the delay to wait before the next restart attempt. @@ -72,5 +69,4 @@ func (b *Backoff) Wait(ctx context.Context) bool { // Reset resets the backoff counter. func (b *Backoff) Reset() { b.attempt = 0 - b.lastStart = time.Time{} } diff --git a/internal/runner/backoff_test.go b/internal/runner/backoff_test.go index 255f6da..3cac7ad 100644 --- a/internal/runner/backoff_test.go +++ b/internal/runner/backoff_test.go @@ -108,7 +108,7 @@ func TestBackoff_Reset(t *testing.T) { } } -func TestBackoff_RecordStart_ResetAfterStability(t *testing.T) { +func TestBackoff_RecordExit_ResetAfterStability(t *testing.T) { b := NewBackoff(&config.BackoffConfig{ InitialDelay: config.Duration(100 * time.Millisecond), MaxDelay: config.Duration(1 * time.Second), @@ -122,8 +122,7 @@ func TestBackoff_RecordStart_ResetAfterStability(t *testing.T) { currentAttempt := b.attempt // Simulate start a long time ago (stable process) - b.lastStart = time.Now().Add(-2 * time.Second) // > maxDelay - b.RecordStart() + b.RecordExit(2 * time.Second) // > maxDelay if b.attempt != 0 { t.Errorf("attempt should reset after stable period: got %d (was %d)", b.attempt, currentAttempt) diff --git a/internal/runner/manager.go b/internal/runner/manager.go index e97b6c8..2608949 100644 --- a/internal/runner/manager.go +++ b/internal/runner/manager.go @@ -2,159 +2,234 @@ package runner import ( "context" + "errors" "fmt" "os" + "reflect" "sync" "sync/atomic" "time" - "golang.org/x/sync/errgroup" - "github.com/michaelishri/gonner/internal/condition" "github.com/michaelishri/gonner/internal/config" + "github.com/michaelishri/gonner/internal/execution" "github.com/michaelishri/gonner/internal/logging" ) -// Manager orchestrates all managed processes. type Manager struct { cfg *config.Config processes []*Process mu sync.RWMutex startedAt time.Time - cancelFn context.CancelFunc shuttingDown atomic.Bool + service *execution.Service } -// NewManager creates a new process manager from a validated config. func NewManager(cfg *config.Config) *Manager { - return &Manager{ - cfg: cfg, + copyCfg := *cfg + copyCfg.Run = append([]config.ProcessConfig(nil), cfg.Run...) + copyCfg.ApplyDefaults() + m := &Manager{cfg: ©Cfg, service: execution.NewService()} + m.service.Diagnostic = logging.Gonner + for _, c := range m.cfg.Run { + for i := 0; i < c.Instances; i++ { + p := NewProcess(c, i, time.Duration(m.cfg.ShutdownTimeout), nil) + p.service = m.service + m.processes = append(m.processes, p) + } } + return m } -// Run starts all processes and blocks until all have exited or the context is cancelled. -func (m *Manager) Run(ctx context.Context) error { - ctx, cancel := context.WithCancel(ctx) - m.cancelFn = cancel +// waitStarted succeeds if ANY instance has ever started. It becomes unavailable +// only when ALL instances terminate before starting; it never polls live state. +func waitStarted(ctx context.Context, procs []*Process) bool { + cases := []reflect.SelectCase{{Dir: reflect.SelectRecv, Chan: reflect.ValueOf(ctx.Done())}} + for _, p := range procs { + cases = append(cases, reflect.SelectCase{Dir: reflect.SelectRecv, Chan: reflect.ValueOf(p.Ready())}, reflect.SelectCase{Dir: reflect.SelectRecv, Chan: reflect.ValueOf(p.Done())}) + } + remaining := len(procs) + for remaining > 0 { + // Prefer already-successful starts even when Done is also closed. + for _, p := range procs { + select { + case <-p.Ready(): + return true + default: + } + } + chosen, _, _ := reflect.Select(cases) + if chosen == 0 { + return false + } + if chosen%2 == 1 { + return true + } + cases[chosen].Chan = reflect.Value{} + remaining-- + } + for _, p := range procs { + select { + case <-p.Ready(): + return true + default: + } + } + return false +} +func (m *Manager) Run(parent context.Context) error { + ctx, cancel := context.WithCancel(parent) + defer cancel() + m.mu.Lock() + m.startedAt = time.Now() + m.mu.Unlock() defer func() { m.shuttingDown.Store(true) - cancel() + for _, p := range m.processes { + select { + case <-p.Done(): + default: + p.finish(StateStopped) + } + if p.logWriter != nil { + _ = p.logWriter.Close() + } + } }() - - m.startedAt = time.Now() - shutdownTimeout := time.Duration(m.cfg.ShutdownTimeout) - - // Start zombie reaper (no-op if not PID 1) - reaperDone := make(chan struct{}) - StartZombieReaper(reaperDone) - defer close(reaperDone) - - // Evaluate conditions and filter runnable processes - var runnableConfigs []config.ProcessConfig - for _, procCfg := range m.cfg.Run { - shouldRun, reason, err := condition.ShouldRun(procCfg.WhenAll, procCfg.WhenAny) + if err := config.Validate(m.cfg); err != nil { + return err + } + // All paths and credentials are checked before even command conditions. + for _, p := range m.processes { + if err := p.prepare(); err != nil { + p.finish(StateFailed) + return fmt.Errorf("preflight %q: %w", p.Name(), err) + } + } + // Keep the orphan reaper alive through cancellation and child cleanup. + joinReaper, err := m.service.StartReaper(context.Background()) + if err != nil { + return err + } + defer joinReaper() + var errorsMu sync.Mutex + var failures []error + record := func(err error) { if err != nil { - return fmt.Errorf("evaluating conditions for %q: %w", procCfg.Name, err) + errorsMu.Lock() + failures = append(failures, err) + errorsMu.Unlock() } - if !shouldRun { - logging.Gonner("Skipping %q: condition not met (%s)", procCfg.Name, reason) - continue + } + watchDone := make(chan struct{}) + watchExit := make(chan struct{}) + go func() { + defer close(watchExit) + select { + case err := <-m.service.Errors: + record(err) + cancel() + case <-watchDone: } - runnableConfigs = append(runnableConfigs, procCfg) + }() + var stopMonitorOnce sync.Once + stopMonitor := func() { stopMonitorOnce.Do(func() { close(watchDone); <-watchExit }) } + defer stopMonitor() + shutdownWatch := context.AfterFunc(ctx, func() { m.shuttingDown.Store(true) }) + defer shutdownWatch() + groups := make(map[string][]*Process) + for _, p := range m.processes { + groups[p.cfg.Name] = append(groups[p.cfg.Name], p) + p.onCriticalExit = cancel } - - if len(runnableConfigs) == 0 { - logging.Gonner("No processes to run after condition evaluation") - return nil + for _, c := range m.cfg.Run { + if ctx.Err() != nil { + break + } + p := groups[c.Name][0] + spec := p.spec("", c.WorkDir) + spec.Output = nil + ok, reason, err := condition.NewEvaluator(m.service, spec).ShouldRun(ctx, c.WhenAll, c.WhenAny) + if err != nil { + if ctx.Err() == nil { + record(fmt.Errorf("conditions for %q: %w", c.Name, err)) + } + cancel() + break + } + if !ok { + logging.Gonner("Skipping %q: %s", c.Name, reason) + for _, p := range groups[c.Name] { + p.finish(StateSkipped) + } + } } - - // Build process instances - // readyMap tracks which process name -> ready channels for dependsOn resolution - readyMap := make(map[string][]*Process) - - for _, procCfg := range runnableConfigs { - for i := 0; i < procCfg.Instances; i++ { - proc := NewProcess(procCfg, i, shutdownTimeout, cancel) - m.mu.Lock() - m.processes = append(m.processes, proc) - m.mu.Unlock() - readyMap[procCfg.Name] = append(readyMap[procCfg.Name], proc) + var wg sync.WaitGroup + launch := func(p *Process) { + select { + case <-p.Done(): + return + default: } + wg.Go(func() { + for _, dep := range p.cfg.DependsOn { + if !waitStarted(ctx, groups[dep]) { + if ctx.Err() != nil { + p.finish(StateStopped) + return + } + err := fmt.Errorf("process %q: dependency %q never started", p.Name(), dep) + if p.cfg.Critical { + p.finish(StateFailed) + record(err) + cancel() + } else { + p.finish(StateSkipped) + record(err) + } + return + } + } + if ctx.Err() != nil { + p.finish(StateStopped) + return + } + err := p.Run(ctx) + record(err) + var supervisor *execution.SupervisorError + if errors.As(err, &supervisor) { + cancel() + } + }) } - - logging.Gonner("Starting %d process(es) in %s mode", len(m.processes), m.cfg.Mode) - - g, gCtx := errgroup.WithContext(ctx) - stopShutdownWatch := context.AfterFunc(gCtx, func() { - m.shuttingDown.Store(true) - }) - defer stopShutdownWatch() - if m.cfg.Mode == "sequential" { - // Sequential: start each process config one at a time - startup: - for _, procCfg := range runnableConfigs { - if gCtx.Err() != nil { + for _, c := range m.cfg.Run { + if ctx.Err() != nil { break } - procCfg := procCfg // capture for closure - procs := readyMap[procCfg.Name] - - // Start all instances of this process - for _, proc := range procs { - proc := proc - g.Go(func() error { - defer logging.Recover(fmt.Sprintf("runner[%s]", proc.Name())) - return proc.Run(gCtx) - }) - } - - // Wait for at least one instance to be ready (or exit) before moving to next - if len(procs) > 0 { - select { - case <-procs[0].Ready(): - case <-procs[0].Done(): - // Process exited before becoming ready; continue to next - case <-gCtx.Done(): - // Already-started processes still need to finish shutdown. - break startup - } + for _, p := range groups[c.Name] { + launch(p) } + _ = waitStarted(ctx, groups[c.Name]) } } else { - // Parallel: start all processes with dependsOn coordination - for _, proc := range m.processes { - proc := proc - g.Go(func() error { - defer logging.Recover(fmt.Sprintf("runner[%s]", proc.Name())) - // Wait for dependencies - for _, depName := range proc.cfg.DependsOn { - depProcs, ok := readyMap[depName] - if !ok { - // Dependency was skipped by conditions — treat as unavailable - logging.Gonner("Dependency %q for %q was skipped — skipping %q", depName, proc.Name(), proc.Name()) - proc.state.Store(StateSkipped) - return nil - } - // Wait for at least one instance of the dependency to be ready - select { - case <-depProcs[0].Ready(): - case <-gCtx.Done(): - return nil - } - } - return proc.Run(gCtx) - }) + for _, p := range m.processes { + launch(p) } } - - // Each command handles cancellation using its configured stop signal and - // timeout. Join all lifecycles before returning, including during startup. - err := g.Wait() - m.shuttingDown.Store(true) - - logging.Gonner("All processes have exited") - return err + wg.Wait() + joinReaper() + stopMonitor() + // Join the error monitor before reading failures, without closing its channel + // twice. Drain any final infrastructure error before producing exit status. + select { + case err := <-m.service.Errors: + record(err) + default: + } + errorsMu.Lock() + defer errorsMu.Unlock() + return errors.Join(failures...) } // Processes returns info about all managed processes. @@ -228,6 +303,11 @@ func (m *Manager) Processes() []ProcessInfo { // Uptime returns how long the manager has been running. func (m *Manager) Uptime() time.Duration { + m.mu.RLock() + defer m.mu.RUnlock() + if m.startedAt.IsZero() { + return 0 + } return time.Since(m.startedAt) } @@ -277,14 +357,11 @@ func (m *Manager) Ready() bool { return true } -// ForwardSignal sends a signal to all running processes. +// ForwardSignal forwards operational signals and reports failures to Run. func (m *Manager) ForwardSignal(sig os.Signal) { - m.mu.RLock() - defer m.mu.RUnlock() - - for _, proc := range m.processes { - if proc.State() == StateRunning { - proc.ForwardSignal(sig) + for _, p := range m.processes { + if err := p.ForwardSignal(sig); err != nil { + m.service.ReportSignalError(err) } } } diff --git a/internal/runner/process.go b/internal/runner/process.go index fc127c0..b92f5b0 100644 --- a/internal/runner/process.go +++ b/internal/runner/process.go @@ -6,9 +6,6 @@ import ( "fmt" "io" "os" - "os/exec" - "os/user" - "strconv" "strings" "sync" "sync/atomic" @@ -16,6 +13,7 @@ import ( "time" "github.com/michaelishri/gonner/internal/config" + "github.com/michaelishri/gonner/internal/execution" "github.com/michaelishri/gonner/internal/logging" ) @@ -44,454 +42,224 @@ type ProcessInfo struct { Critical bool `json:"critical"` } -// Process manages a single process (or one instance of a multi-instance process). +// Process owns one configured instance and a stable initial-start gate. type Process struct { - cfg config.ProcessConfig - instanceID int - state atomic.Value // ProcessState - logWriter *logging.Writer - backoff *Backoff - restarts int - startedAt time.Time - currentCmd *exec.Cmd - cmdDone chan struct{} - shutdownTimeout time.Duration - mu sync.Mutex - - readyCh chan struct{} - doneCh chan struct{} - onCriticalExit func() + cfg config.ProcessConfig + instanceID int + state atomic.Value + logWriter *logging.Writer + service *execution.Service + credential *syscall.Credential + backoff *Backoff + shutdownTimeout time.Duration + mu sync.Mutex + restarts, launches, pid int + startedAt time.Time + cancel context.CancelFunc + readyCh, doneCh chan struct{} + readyOnce, doneOnce sync.Once + onCriticalExit func() } -// NewProcess creates a new Process instance. -func NewProcess( - cfg config.ProcessConfig, - instanceID int, - shutdownTimeout time.Duration, - onCriticalExit func(), -) *Process { - p := &Process{ - cfg: cfg, - instanceID: instanceID, - backoff: NewBackoff(cfg.Backoff), - shutdownTimeout: shutdownTimeout, - readyCh: make(chan struct{}), - doneCh: make(chan struct{}), - onCriticalExit: onCriticalExit, - } +func NewProcess(cfg config.ProcessConfig, id int, timeout time.Duration, onCriticalExit func()) *Process { + p := &Process{cfg: cfg, instanceID: id, shutdownTimeout: timeout, onCriticalExit: onCriticalExit, service: execution.NewService(), backoff: NewBackoff(cfg.Backoff), readyCh: make(chan struct{}), doneCh: make(chan struct{})} + p.service.Diagnostic = logging.Gonner p.state.Store(StatePending) return p } - -// Name returns the display name for this process (includes instance ID if needed). func (p *Process) Name() string { if p.cfg.Instances > 1 { return fmt.Sprintf("%s/%d", p.cfg.Name, p.instanceID) } return p.cfg.Name } - -// State returns the current state. -func (p *Process) State() ProcessState { - return p.state.Load().(ProcessState) +func (p *Process) State() ProcessState { return p.state.Load().(ProcessState) } + +// Ready is closed on the first successful launch and never replaced. +func (p *Process) Ready() <-chan struct{} { return p.readyCh } +func (p *Process) Done() <-chan struct{} { return p.doneCh } +func (p *Process) finish(state ProcessState) { + p.state.Store(state) + p.doneOnce.Do(func() { close(p.doneCh) }) } - -// Ready returns a channel that is closed when the process is running. -func (p *Process) Ready() <-chan struct{} { - p.mu.Lock() - defer p.mu.Unlock() - return p.readyCh -} - -// Done returns a channel that is closed when the process lifecycle is over. -func (p *Process) Done() <-chan struct{} { - return p.doneCh -} - -// Info returns current runtime info. func (p *Process) Info() ProcessInfo { p.mu.Lock() defer p.mu.Unlock() - - info := ProcessInfo{ - Name: p.Name(), - Status: p.State(), - Restarts: p.restarts, - Critical: p.cfg.Critical, - } - - if p.currentCmd != nil && p.currentCmd.Process != nil { - info.PID = p.currentCmd.Process.Pid - } - - if !p.startedAt.IsZero() && p.State() == StateRunning { + info := ProcessInfo{Name: p.Name(), Status: p.State(), PID: p.pid, Restarts: p.restarts, Critical: p.cfg.Critical} + if info.Status == StateRunning && !p.startedAt.IsZero() { info.Uptime = time.Since(p.startedAt).Truncate(time.Second).String() } - return info } -// Run executes the full process lifecycle: commandsBefore, start, monitor, restart. -// It blocks until the process terminates or the context is cancelled. -func (p *Process) Run(ctx context.Context) error { - defer logging.Recover(fmt.Sprintf("process[%s]", p.Name())) - defer close(p.doneCh) - name := p.Name() - - // Initialize log writer - logOpts := logging.Options{ - ProcessName: name, - LogFilePath: p.cfg.LogFile, - LogFileMode: os.FileMode(p.cfg.LogFileMode), - } - if p.cfg.LogRotate != nil { - logOpts.Rotate = &logging.RotateOptions{ - MaxSizeMB: p.cfg.LogRotate.MaxSizeMB, - MaxBackups: p.cfg.LogRotate.MaxBackups, - Compress: p.cfg.LogRotate.Compress, - } +// prepare resolves unsafe configuration before any conditions or commands run. +func (p *Process) prepare() error { + if p.logWriter != nil { + return nil } - w, err := logging.NewWriterWithOptions(logOpts) + cred, err := execution.ResolveCredential(p.cfg.User, p.cfg.Group) if err != nil { - p.state.Store(StateFailed) - return fmt.Errorf("creating log writer for %s: %w", name, err) + return err } - p.logWriter = w - defer p.logWriter.Close() - - // Run commandsBefore + p.credential = cred + opts := logging.Options{ProcessName: p.Name(), LogFilePath: p.cfg.LogFile, LogFileMode: os.FileMode(p.cfg.LogFileMode)} + if r := p.cfg.LogRotate; r != nil { + opts.Rotate = &logging.RotateOptions{MaxSizeMB: r.MaxSizeMB, MaxBackups: r.MaxBackups, Compress: r.Compress} + } + p.logWriter, err = logging.NewWriterWithOptions(opts) + return err +} +func (p *Process) spec(command, dir string) execution.Spec { + timeout := time.Duration(p.cfg.StopTimeout) + if timeout <= 0 { + timeout = p.shutdownTimeout + } + return execution.Spec{Command: command, Dir: dir, Env: p.buildEnv(), Credential: p.credential, StopSignal: parseSignal(p.cfg.StopSignal), StopTimeout: timeout, Output: func(r io.Reader) error { return logging.LineScanner(r, p.logWriter) }} +} +func (p *Process) fail(err error) error { + p.state.Store(StateFailed) + err = fmt.Errorf("process %q: %w", p.Name(), err) + logging.Gonner("%v", err) + if p.cfg.Critical && p.onCriticalExit != nil { + p.onCriticalExit() + } + return err +} +func (p *Process) Run(parent context.Context) error { + defer p.doneOnce.Do(func() { close(p.doneCh) }) + ownedLog := p.logWriter == nil + defer func() { + if ownedLog && p.logWriter != nil { + _ = p.logWriter.Close() + } + }() + if err := p.prepare(); err != nil { + return p.fail(err) + } + // Manager retains log writers until every lifecycle is joined, keeping all + // configured paths protected from pruning. Standalone callers own theirs. + ctx, cancel := context.WithCancel(parent) + defer cancel() + p.mu.Lock() + p.cancel = cancel + p.mu.Unlock() p.state.Store(StateStarting) if err := p.runCommandsBefore(ctx); err != nil { - if ctx.Err() != nil { + var supervisor *execution.SupervisorError + if (errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded)) && !errors.As(err, &supervisor) { p.state.Store(StateStopped) return nil } - p.state.Store(StateFailed) - logging.Gonner("Process %q failed during commandsBefore: %v", name, err) - return nil + return p.fail(err) } - + retries := 0 for { if ctx.Err() != nil { p.state.Store(StateStopped) return nil } - - exitCode, err := p.runCommand(ctx) - if ctx.Err() != nil { + p.state.Store(StateStarting) + spec := p.spec(p.cfg.Command, p.cfg.WorkDir) + spec.OnStart = func(pid int) { + p.mu.Lock() + p.pid = pid + p.startedAt = time.Now() + if p.launches > 0 { + p.restarts++ + } + p.launches++ + p.state.Store(StateRunning) + p.mu.Unlock() + p.readyOnce.Do(func() { close(p.readyCh) }) + } + spec.OnExit = func(runtime time.Duration) { + p.mu.Lock() + p.pid = 0 + p.state.Store(StateStarting) + p.mu.Unlock() + p.backoff.RecordExit(runtime) + } + spec.OnStop = p.markStopping + result := p.service.Run(ctx, spec) + var supervisor *execution.SupervisorError + if errors.As(result.Err, &supervisor) { + return p.fail(result.Err) + } + if result.Cancelled { p.state.Store(StateStopped) - logging.Gonner("Process %q stopped (shutdown)", name) return nil } - - if err != nil { - logging.Gonner("Process %q exited with error: %v", name, err) - } else if exitCode == 0 { - logging.Gonner("Process %q exited normally (code 0)", name) + if result.Err == nil && result.ExitCode == 0 { p.state.Store(StateStopped) return nil - } else { - logging.Gonner("Process %q exited with code %d", name, exitCode) } - - if p.cfg.Critical { - p.state.Store(StateFailed) - logging.Gonner("CRITICAL: Process %q failed — triggering full shutdown", name) - if p.onCriticalExit != nil { - p.onCriticalExit() - } - return nil + err := result.Err + if err == nil { + err = fmt.Errorf("exit status %d", result.ExitCode) } - - if !p.cfg.AutoRestart { - p.state.Store(StateFailed) - return nil + if p.cfg.Critical || !p.cfg.AutoRestart || (p.cfg.MaxRetries > 0 && retries >= p.cfg.MaxRetries) { + return p.fail(err) } - - p.mu.Lock() - p.restarts++ - p.mu.Unlock() - - if p.cfg.MaxRetries > 0 && p.restarts > p.cfg.MaxRetries { - p.state.Store(StateFailed) - logging.Gonner("Process %q exceeded max retries (%d)", name, p.cfg.MaxRetries) - return nil - } - - logging.Gonner("Restarting %q (attempt %d)...", name, p.restarts) + p.state.Store(StateStarting) + logging.Gonner("Restarting %q (retry %d)", p.Name(), retries+1) if !p.backoff.Wait(ctx) { p.state.Store(StateStopped) return nil } + // The retry budget counts extra launch attempts; public Restarts counts only + // successful launches after the first successful launch, in OnStart. + retries++ } } - -// runCommandsBefore executes pre-commands sequentially. -func (p *Process) runCommandsBefore(ctx context.Context) error { - for i, preCmd := range p.cfg.CommandsBefore { - if err := ctx.Err(); err != nil { - return err - } - logging.Gonner("Running commandsBefore[%d] for %q: %s", i, p.Name(), preCmd.Command) - - workDir := preCmd.WorkDir - if workDir == "" { - workDir = p.cfg.WorkDir - } - - cmd, err := p.newCommand(ctx, preCmd.Command, workDir) - if err != nil { - if preCmd.ContinueOnError { - logging.Gonner("commandsBefore[%d] for %q credential error (continuing): %v", i, p.Name(), err) - continue - } - return fmt.Errorf("commandsBefore[%d] credential: %w", i, err) - } - - stdout, _ := cmd.StdoutPipe() - stderr, _ := cmd.StderrPipe() - - if err := cmd.Start(); err != nil { - if preCmd.ContinueOnError { - logging.Gonner("commandsBefore[%d] for %q failed to start (continuing): %v", i, p.Name(), err) - continue - } - return fmt.Errorf("commandsBefore[%d] failed to start: %w", i, err) - } - - var wg sync.WaitGroup - wg.Go(func() { logging.LineScanner(stdout, p.logWriter) }) - wg.Go(func() { logging.LineScanner(stderr, p.logWriter) }) - - waitErr := cmd.Wait() - // The shell can exit before its descendants. Finish stopping the - // group before advancing to another command or closing its log writer. - _ = cmd.Cancel() - wg.Wait() - if err := ctx.Err(); err != nil { - return err - } - if err := waitErr; err != nil { - if preCmd.ContinueOnError { - logging.Gonner("commandsBefore[%d] for %q failed (continuing): %v", i, p.Name(), err) - continue - } - return fmt.Errorf("commandsBefore[%d] failed: %w", i, err) - } - - logging.Gonner("commandsBefore[%d] for %q completed", i, p.Name()) - } - return nil -} - -// runCommand executes the main command once and waits for it to exit. -// Returns the exit code and any error. -func (p *Process) runCommand(ctx context.Context) (int, error) { - cmd, err := p.newCommand(ctx, p.cfg.Command, p.cfg.WorkDir) - if err != nil { - return -1, fmt.Errorf("setting credential: %w", err) - } - - stdoutPipe, err := cmd.StdoutPipe() - if err != nil { - return -1, fmt.Errorf("creating stdout pipe: %w", err) - } - stderrPipe, err := cmd.StderrPipe() - if err != nil { - return -1, fmt.Errorf("creating stderr pipe: %w", err) - } - - // Refresh ready channel so dependants of this restart can re-arm if needed. - p.mu.Lock() - if p.readyCh == nil { - p.readyCh = make(chan struct{}) - } else { - select { - case <-p.readyCh: - // Was closed by previous start — make a fresh one. - p.readyCh = make(chan struct{}) - default: - } - } - p.mu.Unlock() - - if err := cmd.Start(); err != nil { - return -1, fmt.Errorf("starting command: %w", err) - } - +func (p *Process) markStopping() { p.mu.Lock() - p.currentCmd = cmd - p.cmdDone = make(chan struct{}) - p.startedAt = time.Now() - readyCh := p.readyCh - if ctx.Err() == nil { - p.state.Store(StateRunning) - } - p.mu.Unlock() - - p.backoff.RecordStart() - - // Signal readiness once. - select { - case <-readyCh: - default: - close(readyCh) - } - - logging.Gonner("Process %q started (PID %d)", p.Name(), cmd.Process.Pid) - - var wg sync.WaitGroup - wg.Add(2) - go func() { - defer wg.Done() - defer logging.Recover("stdout-scanner") - logging.LineScanner(stdoutPipe, p.logWriter) - }() - go func() { - defer wg.Done() - defer logging.Recover("stderr-scanner") - logging.LineScanner(stderrPipe, p.logWriter) - }() - - waitErr := cmd.Wait() - exitCode := 0 - pid := cmd.Process.Pid - - // PID-1 reaper may have stolen the wait — recover exit status if so. - if waitErr != nil { - var exitErr *exec.ExitError - if errors.As(waitErr, &exitErr) { - exitCode = exitErr.ExitCode() - } else if v, ok := ReapedStatuses.LoadAndDelete(pid); ok { - ws := v.(reapedStatus).status - exitCode = ws.ExitStatus() - if exitCode == 0 { - waitErr = nil - } - } else { - exitCode = -1 - } - } else { - // Clean exit; drop any stale reaper entry. - ReapedStatuses.Delete(pid) - } - - // Retain the original process group even if the shell has already exited. - // This also prevents background descendants from surviving a normal exit - // or being left behind when the main command restarts. Claim any reaped - // exit status above before cleanup can wait long enough for it to expire. - _ = cmd.Cancel() - wg.Wait() - - _, _ = io.Copy(io.Discard, stdoutPipe) - _, _ = io.Copy(io.Discard, stderrPipe) - - p.mu.Lock() - close(p.cmdDone) - p.currentCmd = nil - p.mu.Unlock() - - return exitCode, waitErr -} - -// newCommand gives both startup and main commands their own process group and -// replaces CommandContext's immediate Process.Kill with bounded group shutdown. -func (p *Process) newCommand(ctx context.Context, command, workDir string) (*exec.Cmd, error) { - cmd := exec.CommandContext(ctx, "sh", "-c", command) - cmd.Dir = workDir - cmd.Env = p.buildEnv() - cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} - if err := applyCredential(cmd, p.cfg.User, p.cfg.Group); err != nil { - return nil, err - } - - var stopOnce sync.Once - var stopErr error - cmd.Cancel = func() error { - stopOnce.Do(func() { - stopErr = p.stopProcessGroup(cmd.Process.Pid) - }) - return stopErr + defer p.mu.Unlock() + if p.pid != 0 { + p.state.Store(StateStopping) } - return cmd, nil } - -// stopProcessGroup waits for the entire group, not just its original shell. -// Setpgid makes the child's PID its PGID, which stays valid after the shell exits. -// Do not wait on cmdDone here: exec.Cmd.Wait itself waits for Cancel to return. -func (p *Process) stopProcessGroup(pgid int) error { - if err := syscall.Kill(-pgid, parseSignal(p.cfg.StopSignal)); err != nil { - if errors.Is(err, syscall.ESRCH) { - return os.ErrProcessDone +func (p *Process) runCommandsBefore(ctx context.Context) error { + for i, pre := range p.cfg.CommandsBefore { + if ctx.Err() != nil { + return ctx.Err() } - return err - } - - p.mu.Lock() - p.state.Store(StateStopping) - p.mu.Unlock() - - timeout := time.Duration(p.cfg.StopTimeout) - if timeout <= 0 { - timeout = p.shutdownTimeout - } - timer := time.NewTimer(timeout) - defer timer.Stop() - ticker := time.NewTicker(10 * time.Millisecond) - defer ticker.Stop() - - for { - if errors.Is(syscall.Kill(-pgid, 0), syscall.ESRCH) { - return nil + dir := pre.WorkDir + if dir == "" { + dir = p.cfg.WorkDir } - select { - case <-ticker.C: - case <-timer.C: - logging.Gonner("Process %q did not exit within %s, sending SIGKILL", p.Name(), timeout) - if err := syscall.Kill(-pgid, syscall.SIGKILL); err != nil && !errors.Is(err, syscall.ESRCH) { - return err + spec := p.spec(pre.Command, dir) + spec.OnStart = func(pid int) { p.mu.Lock(); p.pid = pid; p.mu.Unlock() } + spec.OnExit = func(time.Duration) { p.mu.Lock(); p.pid = 0; p.state.Store(StateStarting); p.mu.Unlock() } + spec.OnStop = p.markStopping + r := p.service.Run(ctx, spec) + if r.Err != nil { + var supervisor *execution.SupervisorError + if r.Cancelled || errors.As(r.Err, &supervisor) || !pre.ContinueOnError { + return fmt.Errorf("commandsBefore[%d]: %w", i, r.Err) } - return nil + logging.Gonner("commandsBefore[%d] for %q failed (continuing): %v", i, p.Name(), r.Err) } } + return nil } - -// Stop sends the configured stop signal to the process group, escalates to -// SIGKILL after the timeout, and waits for the command to be reaped. func (p *Process) Stop() { p.mu.Lock() - cmd := p.currentCmd - done := p.cmdDone + cancel := p.cancel p.mu.Unlock() - - if cmd == nil || cmd.Process == nil { - return + if cancel != nil { + cancel() + <-p.doneCh } - - _ = cmd.Cancel() - <-done } - -// ForwardSignal sends a signal to the process group. -func (p *Process) ForwardSignal(sig os.Signal) { +func (p *Process) ForwardSignal(sig os.Signal) error { p.mu.Lock() defer p.mu.Unlock() - cmd := p.currentCmd - - if cmd == nil || cmd.Process == nil { - return - } - - sysSignal, ok := sig.(syscall.Signal) + s, ok := sig.(syscall.Signal) if !ok { - return + return nil } - - _ = syscall.Kill(-cmd.Process.Pid, sysSignal) + return execution.SignalGroup(p.pid, s) } - -// buildEnv returns the environment for child processes. -// It inherits the current environment and overlays any per-process env vars. func (p *Process) buildEnv() []string { if len(p.cfg.Env) == 0 { return nil @@ -524,48 +292,3 @@ func parseSignal(name string) syscall.Signal { return syscall.SIGTERM } } - -// applyCredential sets cmd.SysProcAttr.Credential from a user/group name or UID/GID. -// No-op if userSpec is empty. Returns an error if lookup fails or current process is not root. -func applyCredential(cmd *exec.Cmd, userSpec, groupSpec string) error { - if userSpec == "" { - return nil - } - if os.Geteuid() != 0 { - return fmt.Errorf("dropping privileges to %q requires running as root", userSpec) - } - - var uid, gid uint64 - u, err := user.Lookup(userSpec) - if err != nil { - // Try numeric UID - n, errN := strconv.ParseUint(userSpec, 10, 32) - if errN != nil { - return fmt.Errorf("looking up user %q: %w", userSpec, err) - } - uid = n - gid = n - } else { - uid, _ = strconv.ParseUint(u.Uid, 10, 32) - gid, _ = strconv.ParseUint(u.Gid, 10, 32) - } - - if groupSpec != "" { - g, err := user.LookupGroup(groupSpec) - if err != nil { - n, errN := strconv.ParseUint(groupSpec, 10, 32) - if errN != nil { - return fmt.Errorf("looking up group %q: %w", groupSpec, err) - } - gid = n - } else { - gid, _ = strconv.ParseUint(g.Gid, 10, 32) - } - } - - if cmd.SysProcAttr == nil { - cmd.SysProcAttr = &syscall.SysProcAttr{} - } - cmd.SysProcAttr.Credential = &syscall.Credential{Uid: uint32(uid), Gid: uint32(gid)} - return nil -} diff --git a/internal/runner/regression_test.go b/internal/runner/regression_test.go new file mode 100644 index 0000000..b9ac643 --- /dev/null +++ b/internal/runner/regression_test.go @@ -0,0 +1,274 @@ +package runner + +import ( + "context" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "testing/synctest" + "time" + + "github.com/michaelishri/gonner/internal/config" +) + +func TestFailedAndSkippedDependencyChainsTerminate(t *testing.T) { + for _, skip := range []bool{false, true} { + t.Run(map[bool]string{false: "failed", true: "skipped"}[skip], func(t *testing.T) { + first := config.ProcessConfig{Name: "first", Command: "true", CommandsBefore: []config.PreCommand{{Command: "exit 7"}}} + if skip { + first.WhenAll = []map[string]string{{"fileExists": "/gonner-missing-condition-marker"}} + } + marker := filepath.Join(t.TempDir(), "unrelated") + mgr := NewManager(&config.Config{Run: []config.ProcessConfig{first, {Name: "second", Command: "true", DependsOn: []string{"first"}}, {Name: "third", Command: "true", DependsOn: []string{"second"}}, {Name: "unrelated", Command: "touch '" + marker + "'"}}}) + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + if err := mgr.Run(ctx); err == nil { + t.Fatal("unavailable dependency must fail eventual exit") + } + if ctx.Err() != nil { + t.Fatal("dependency chain hung until deadline") + } + if _, err := os.Stat(marker); err != nil { + t.Fatal("noncritical failure cancelled unrelated work") + } + for _, p := range mgr.processes { + select { + case <-p.Done(): + default: + t.Errorf("%s Done open", p.Name()) + } + } + if mgr.processes[1].State() != StateSkipped || mgr.processes[2].State() != StateSkipped { + t.Fatal("dependants not skipped") + } + }) + } +} +func TestCriticalPrecommandFailureCancelsAndReturnsError(t *testing.T) { + for _, continueOnError := range []bool{false, true} { + cmds := []config.ProcessConfig{{Name: "critical", Command: "true", Critical: true, CommandsBefore: []config.PreCommand{{Command: "exit 5", ContinueOnError: continueOnError}}}} + if !continueOnError { + cmds = append(cmds, config.ProcessConfig{Name: "other", Command: "exec sleep 30"}) + } + mgr := NewManager(&config.Config{ShutdownTimeout: config.Duration(100 * time.Millisecond), Run: cmds}) + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + err := mgr.Run(ctx) + cancel() + if (err == nil) != continueOnError { + t.Fatalf("continue=%v error=%v", continueOnError, err) + } + if !continueOnError && mgr.processes[0].State() != StateFailed { + t.Fatal("critical precommand not failed") + } + } +} +func TestCriticalUnavailableDependencyIsFatal(t *testing.T) { + mgr := NewManager(&config.Config{Run: []config.ProcessConfig{{Name: "missing", Command: "true", WhenAll: []map[string]string{{"fileExists": "/gonner-missing"}}}, {Name: "critical", Command: "true", Critical: true, DependsOn: []string{"missing"}}, {Name: "other", Command: "exec sleep 30"}}, ShutdownTimeout: config.Duration(100 * time.Millisecond)}) + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + if err := mgr.Run(ctx); err == nil || !strings.Contains(err.Error(), "never started") { + t.Fatalf("error=%v", err) + } + if ctx.Err() != nil { + t.Fatal("critical blocked dependency did not cancel") + } +} +func TestAnyInstanceStartGateAndShortLivedPrerequisite(t *testing.T) { + // Barriers deliberately keep instance zero pending. This exercises the exact + // shared gate used by parallel dependencies and sequential startup. + p0 := NewProcess(config.ProcessConfig{Name: "multi", Instances: 2}, 0, time.Second, nil) + p1 := NewProcess(config.ProcessConfig{Name: "multi", Instances: 2}, 1, time.Second, nil) + p1.readyOnce.Do(func() { close(p1.readyCh) }) + p1.finish(StateStopped) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + result := make(chan bool, 1) + go func() { result <- waitStarted(ctx, []*Process{p0, p1}) }() + select { + case ok := <-result: + if !ok { + t.Fatal("successful second instance ignored") + } + case <-time.After(time.Second): + t.Fatal("waited on instance zero") + } + // Ready must remain the same closed channel across every restart. + ready := p1.Ready() + p1.state.Store(StateStarting) + if ready != p1.Ready() { + t.Fatal("ready gate replaced") + } + if !waitStarted(ctx, []*Process{p1}) { + t.Fatal("one-shot prerequisite no longer satisfies gate") + } +} +func TestBackoffExcludesDowntimeAndCleanup(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + b := NewBackoff(&config.BackoffConfig{InitialDelay: config.Duration(time.Second), MaxDelay: config.Duration(4 * time.Second), Multiplier: 2}) + b.Next() + b.Next() + attempt := b.attempt + time.Sleep(time.Hour) // downtime/cleanup, not process runtime + b.RecordExit(time.Millisecond) + if b.attempt != attempt { + t.Fatal("downtime reset escalation") + } + b.RecordExit(5 * time.Second) + if b.attempt != 0 { + t.Fatal("stable runtime did not reset") + } + }) +} +func TestBackoffStatePIDAndCancelledRetryCount(t *testing.T) { + mgr := NewManager(&config.Config{Run: []config.ProcessConfig{{Name: "retry", Command: "exit 1", AutoRestart: true, Backoff: &config.BackoffConfig{InitialDelay: config.Duration(time.Hour), MaxDelay: config.Duration(time.Hour), Multiplier: 1}}}}) + p := mgr.processes[0] + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { done <- mgr.Run(ctx) }() + select { + case <-p.Ready(): + case <-time.After(time.Second): + cancel() + t.Fatal("did not start") + } + deadline := time.Now().Add(time.Second) + for { + info := p.Info() + if info.Status == StateStarting && info.PID == 0 { + if info.Restarts != 0 { + t.Fatal("counted unlaunched retry") + } + break + } + if time.Now().After(deadline) { + cancel() + t.Fatalf("stale running state: %+v", info) + } + time.Sleep(time.Millisecond) + } + cancel() + if err := <-done; err != nil { + t.Fatal(err) + } + if p.Info().Restarts != 0 { + t.Fatal("cancelled retry counted") + } +} +func TestRetryLimitCountsOnlySuccessfulRelaunches(t *testing.T) { + cfg := config.ProcessConfig{Name: "retry", Command: "exit 1", AutoRestart: true, MaxRetries: 2, Backoff: &config.BackoffConfig{InitialDelay: config.Duration(time.Millisecond), MaxDelay: config.Duration(time.Millisecond), Multiplier: 1}} + mgr := NewManager(&config.Config{Run: []config.ProcessConfig{cfg}}) + if err := mgr.Run(context.Background()); err == nil { + t.Fatal("retry exhaustion returned success") + } + if got := mgr.Processes()[0].Restarts; got != 2 { + t.Fatalf("restarts=%d want=2", got) + } + cfg.WorkDir = "/gonner-nonexistent-directory" + mgr = NewManager(&config.Config{Run: []config.ProcessConfig{cfg}}) + if err := mgr.Run(context.Background()); err == nil { + t.Fatal("failed starts returned success") + } + if got := mgr.Processes()[0].Restarts; got != 0 { + t.Fatalf("failed launches counted as restarts: %d", got) + } +} +func TestReadinessIncludesPendingAndSkippedCriticalAndUptimeRace(t *testing.T) { + mgr := NewManager(&config.Config{Run: []config.ProcessConfig{{Name: "critical", Command: "true", Critical: true, Instances: 2, WhenAll: []map[string]string{{"fileExists": "/gonner-missing"}}}}}) + if mgr.Ready() { + t.Fatal("pending critical was ready") + } + if mgr.Uptime() != 0 { + t.Fatal("pre-run uptime was nonzero") + } + var wg sync.WaitGroup + stop := make(chan struct{}) + wg.Go(func() { + for { + select { + case <-stop: + return + default: + _ = mgr.Uptime() + _ = mgr.Processes() + _ = mgr.Ready() + } + } + }) + if err := mgr.Run(context.Background()); err != nil { + t.Fatal(err) + } + close(stop) + wg.Wait() + procs := mgr.Processes() + if len(procs) != 1 || procs[0].Status != StateSkipped || procs[0].Instances != 2 { + t.Fatalf("skipped status=%+v", procs) + } + if mgr.Ready() { + t.Fatal("skipped critical ready") + } +} +func TestPreflightFinishesBeforeCommandConditions(t *testing.T) { + dir := t.TempDir() + marker := filepath.Join(dir, "ran") + mgr := NewManager(&config.Config{Run: []config.ProcessConfig{{Name: "first", Command: "true", WhenAll: []map[string]string{{"commandSucceeds": "touch '" + marker + "'"}}}, {Name: "unsafe", Command: "true", User: "4294967295", Group: "1"}}}) + if err := mgr.Run(context.Background()); err == nil { + t.Fatal("unsafe credentials accepted") + } + if _, err := os.Stat(marker); !os.IsNotExist(err) { + t.Fatal("condition ran before preflight completed") + } +} +func TestCriticalAndNoncriticalPermanentExitsReturnError(t *testing.T) { + for _, critical := range []bool{false, true} { + m := NewManager(&config.Config{Run: []config.ProcessConfig{{Name: "failed", Command: "exit 17", Critical: critical}}}) + if err := m.Run(context.Background()); err == nil { + t.Fatalf("critical=%v exit returned nil", critical) + } + } + m := NewManager(&config.Config{Run: []config.ProcessConfig{{Name: "one-shot", Command: "true", AutoRestart: true}}}) + if err := m.Run(context.Background()); err != nil { + t.Fatal(err) + } + if m.Processes()[0].Restarts != 0 { + t.Fatal("restarted successful one-shot") + } +} + +func TestDependencyRunsWhenInstanceZeroCannotStart(t *testing.T) { + for _, mode := range []string{"parallel", "sequential"} { + t.Run(mode, func(t *testing.T) { + marker := filepath.Join(t.TempDir(), "dependant") + m := NewManager(&config.Config{Mode: mode, Run: []config.ProcessConfig{{Name: "multi", Command: "true", Instances: 2}, {Name: "dependent", Command: "touch '" + marker + "'", DependsOn: []string{"multi"}}}}) + m.processes[0].cfg.WorkDir = "/gonner-missing-instance-zero" + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + if err := m.Run(ctx); err == nil { + t.Fatal("lost instance zero failure") + } + if ctx.Err() != nil { + t.Fatal("hung on instance zero") + } + if _, err := os.Stat(marker); err != nil { + t.Fatal("second instance did not release dependant") + } + }) + } +} +func TestReadyChannelSurvivesActualRestarts(t *testing.T) { + m := NewManager(&config.Config{Run: []config.ProcessConfig{{Name: "retry", Command: "false", AutoRestart: true, MaxRetries: 1, Backoff: &config.BackoffConfig{InitialDelay: config.Duration(time.Millisecond), MaxDelay: config.Duration(time.Millisecond), Multiplier: 1}}}}) + p := m.processes[0] + ready := p.Ready() + if err := m.Run(context.Background()); err == nil { + t.Fatal("expected exhaustion") + } + if p.Info().Restarts != 1 || p.Ready() != ready { + t.Fatal("first-start gate changed during restart") + } + select { + case <-ready: + default: + t.Fatal("first-start gate not closed") + } +} diff --git a/internal/runner/signals.go b/internal/runner/signals.go index b529c4d..d1a3eb5 100644 --- a/internal/runner/signals.go +++ b/internal/runner/signals.go @@ -3,34 +3,11 @@ package runner import ( "os" "os/signal" - "sync" "syscall" - "time" "github.com/michaelishri/gonner/internal/logging" ) -// ReapedStatuses records exit statuses for processes reaped by the zombie reaper -// before exec.Cmd.Wait could call wait4 itself. Indexed by PID. -// Process.runCommand consults this map when cmd.Wait returns ECHILD. -// -// Entries are reaped statuses for *all* descendants of gonner (PID 1 reaps -// orphaned grandchildren too). Only direct children that gonner waits on ever -// consume an entry, so untracked PIDs are pruned by age to bound memory use. -var ReapedStatuses sync.Map // map[int]reapedStatus - -// reapedStatusTTL is how long an unclaimed reaped status is retained before the -// reaper prunes it. Direct children are claimed almost immediately by -// Process.runCommand; anything older is an orphaned descendant nobody waits on. -const reapedStatusTTL = 30 * time.Second - -// reapedStatus pairs a wait status with the time it was recorded so stale, -// unclaimed entries (orphaned descendants) can be pruned. -type reapedStatus struct { - status syscall.WaitStatus - reapedAt time.Time -} - // SignalHandler manages signal trapping and forwarding for gonner. type SignalHandler struct { sigCh chan os.Signal @@ -42,19 +19,20 @@ type SignalHandler struct { // NewSignalHandler creates a signal handler that calls cancelFn on SIGTERM/SIGINT // and forwards SIGHUP/SIGUSR1/SIGUSR2 to child processes via forwardFn. func NewSignalHandler(cancelFn func(), forwardFn func(os.Signal)) *SignalHandler { - return &SignalHandler{ + s := &SignalHandler{ sigCh: make(chan os.Signal, 4), cancelFn: cancelFn, forwardFn: forwardFn, done: make(chan struct{}), } + signal.Notify(s.sigCh, syscall.SIGTERM, syscall.SIGINT, syscall.SIGHUP, syscall.SIGUSR1, syscall.SIGUSR2) + return s } // Start begins listening for signals. Call this in a goroutine. // SIGTERM/SIGINT trigger shutdown; SIGHUP/SIGUSR1/SIGUSR2 are forwarded to children. func (s *SignalHandler) Start() { defer logging.Recover("signal-handler") - signal.Notify(s.sigCh, syscall.SIGTERM, syscall.SIGINT, syscall.SIGHUP, syscall.SIGUSR1, syscall.SIGUSR2) for { select { @@ -85,63 +63,3 @@ func (s *SignalHandler) Stop() { close(s.done) } } - -// StartZombieReaper starts a goroutine that reaps zombie child processes. -// Only active when gonner is running as PID 1 (typical in a container). -// -// To avoid racing with os/exec's own wait4(pid) calls, the reaper records each -// reaped child's exit status in ReapedStatuses; Process.runCommand falls back -// to that map when cmd.Wait returns ECHILD ("wait: no child processes"). -// -// The goroutine runs until the done channel is closed. -func StartZombieReaper(done <-chan struct{}) { - if os.Getpid() != 1 { - logging.Gonner("Not PID 1 (pid=%d), skipping zombie reaper", os.Getpid()) - return - } - - logging.Gonner("Running as PID 1, starting zombie reaper") - - sigCh := make(chan os.Signal, 16) - signal.Notify(sigCh, syscall.SIGCHLD) - - go func() { - defer logging.Recover("zombie-reaper") - defer signal.Stop(sigCh) - - for { - select { - case <-done: - return - case <-sigCh: - // Drain all currently-exited children. - for { - var ws syscall.WaitStatus - pid, err := syscall.Wait4(-1, &ws, syscall.WNOHANG, nil) - if err != nil || pid <= 0 { - break - } - ReapedStatuses.Store(pid, reapedStatus{status: ws, reapedAt: time.Now()}) - logging.Gonner("Reaped child PID %d (exit %d)", pid, ws.ExitStatus()) - } - // Prune unclaimed statuses from orphaned descendants that no - // goroutine will ever wait on, bounding memory over time. - pruneReapedStatuses() - } - } - }() -} - -// pruneReapedStatuses removes unclaimed reaped statuses older than reapedStatusTTL. -// These accumulate when gonner (as PID 1) reaps orphaned descendants that none of -// its own goroutines wait on. Without pruning, the map would grow unbounded over -// the lifetime of a long-running container. -func pruneReapedStatuses() { - cutoff := time.Now().Add(-reapedStatusTTL) - ReapedStatuses.Range(func(key, value any) bool { - if rs, ok := value.(reapedStatus); ok && rs.reapedAt.Before(cutoff) { - ReapedStatuses.Delete(key) - } - return true - }) -} diff --git a/internal/safefile/acl_darwin.go b/internal/safefile/acl_darwin.go new file mode 100644 index 0000000..d258acc --- /dev/null +++ b/internal/safefile/acl_darwin.go @@ -0,0 +1,52 @@ +package safefile + +import ( + "encoding/binary" + "fmt" + "syscall" + "unsafe" +) + +// Darwin ACLs can grant rights beyond mode bits. Query the descriptor's extended +// security attributes; reject allow ACEs that grant mutation. Deny-only ACLs +// (common on home directories) are safe. No cgo or pathname lookup is involved. +// Layout: https://github.com/apple-oss-distributions/xnu/blob/main/bsd/sys/kauth.h +func checkACL(fd int) error { + attrs := struct { + Count, Reserved uint16 + Common, Volume, Dir, File, Fork uint32 + }{Count: 5, Common: 0x00400000} + var buf [8192]byte + _, _, errno := syscall.Syscall6(syscall.SYS_FGETATTRLIST, uintptr(fd), uintptr(unsafe.Pointer(&attrs)), uintptr(unsafe.Pointer(&buf[0])), uintptr(len(buf)), 0, 0) + if errno != 0 { + return fmt.Errorf("reading file ACL: %w", errno) + } + size := int(binary.LittleEndian.Uint32(buf[:4])) + if size < 12 || size > len(buf) { + return fmt.Errorf("invalid ACL attributes") + } + off := 4 + int(int32(binary.LittleEndian.Uint32(buf[4:8]))) + n := int(binary.LittleEndian.Uint32(buf[8:12])) + if n == 0 { + return nil + } + if off < 12 || n < 44 || off+n > size { + return fmt.Errorf("invalid ACL data") + } + count := binary.LittleEndian.Uint32(buf[off+36 : off+40]) + if count == 0xffffffff { + return nil + } + if count > uint32((n-44)/24) { + return fmt.Errorf("invalid ACL entry count") + } + for i := 0; i < int(count); i++ { + ace := buf[off+44+i*24:] + flags := binary.LittleEndian.Uint32(ace[16:20]) + rights := binary.LittleEndian.Uint32(ace[20:24]) + if flags&0xf == 1 && rights&((1<<2)|(1<<4)|(1<<5)|(1<<6)|(1<<8)|(1<<10)|(1<<12)|(1<<13)|(1<<21)|(1<<23)) != 0 { + return fmt.Errorf("ACL grants mutation rights") + } + } + return nil +} diff --git a/internal/safefile/acl_darwin_test.go b/internal/safefile/acl_darwin_test.go new file mode 100644 index 0000000..44474cb --- /dev/null +++ b/internal/safefile/acl_darwin_test.go @@ -0,0 +1,39 @@ +package safefile + +import ( + "os" + "os/exec" + "path/filepath" + "testing" +) + +func TestDarwinACLMutationGrantsAreRejected(t *testing.T) { + path := secureDir(t) + file := filepath.Join(path, "log") + if err := os.WriteFile(file, []byte("unchanged"), 0600); err != nil { + t.Fatal(err) + } + if out, err := exec.Command("chmod", "+a", "everyone allow write,append,delete", file).CombinedOutput(); err != nil { + t.Fatalf("setting ACL: %v %s", err, out) + } + dir, err := OpenDirectory(path) + if err != nil { + t.Fatal(err) + } + defer dir.Close() + if f, err := dir.OpenRegular("log", os.O_WRONLY|os.O_APPEND, 0600); err == nil { + _ = f.Close() + t.Fatal("ACL bypassed mode checks") + } + b, _ := os.ReadFile(file) + if string(b) != "unchanged" { + t.Fatal("file modified") + } + if out, err := exec.Command("chmod", "+a", "everyone allow add_file,delete_child", path).CombinedOutput(); err != nil { + t.Fatalf("setting directory ACL: %v %s", err, out) + } + if d, err := OpenDirectory(path); err == nil { + _ = d.Close() + t.Fatal("directory ACL mutation accepted") + } +} diff --git a/internal/safefile/acl_linux.go b/internal/safefile/acl_linux.go new file mode 100644 index 0000000..b1d1f42 --- /dev/null +++ b/internal/safefile/acl_linux.go @@ -0,0 +1,29 @@ +package safefile + +import ( + "errors" + "fmt" + + "golang.org/x/sys/unix" +) + +// POSIX ACL permissions are bounded by the group mode mask. checkDirectory +// and OpenRegular reject group/other write access; sticky ancestors protect +// trusted entries even when a POSIX ACL permits creation by other users. +// Rich/NFSv4 ACLs do not have the same mode-mask guarantee, so fail closed when +// one is present rather than guessing how its ordered allow/deny rules apply. +func checkACL(fd int) error { + for _, name := range []string{"system.nfs4_acl", "system.richacl"} { + n, err := unix.Fgetxattr(fd, name, nil) + if errors.Is(err, unix.ENODATA) || errors.Is(err, unix.ENOTSUP) { + continue + } + if err != nil { + return fmt.Errorf("checking %s: %w", name, err) + } + if n > 0 { + return fmt.Errorf("unsupported extended access ACL: %s", name) + } + } + return nil +} diff --git a/internal/safefile/acl_linux_test.go b/internal/safefile/acl_linux_test.go new file mode 100644 index 0000000..117a164 --- /dev/null +++ b/internal/safefile/acl_linux_test.go @@ -0,0 +1,41 @@ +package safefile + +import ( + "encoding/binary" + "errors" + "os" + "path/filepath" + "testing" + + "golang.org/x/sys/unix" +) + +func TestPOSIXACLWriteGrantIsRejected(t *testing.T) { + path := secureDir(t) + // Linux POSIX ACL v2: owner rwx, named UID 2001 wx, group none, + // effective mask wx, other none. A grant through the mask changes mode bits. + data := make([]byte, 4+5*8) + binary.LittleEndian.PutUint32(data, 2) + entries := [][3]uint32{{1, 7, 0xffffffff}, {2, 3, 2001}, {4, 0, 0xffffffff}, {16, 3, 0xffffffff}, {32, 0, 0xffffffff}} + for i, e := range entries { + b := data[4+i*8:] + binary.LittleEndian.PutUint16(b, uint16(e[0])) + binary.LittleEndian.PutUint16(b[2:], uint16(e[1])) + binary.LittleEndian.PutUint32(b[4:], e[2]) + } + f, err := os.Open(path) + if err != nil { + t.Fatal(err) + } + defer f.Close() + if err := unix.Fsetxattr(int(f.Fd()), "system.posix_acl_access", data, 0); err != nil { + if errors.Is(err, unix.ENOTSUP) { + t.Skip("filesystem has no POSIX ACL support") + } + t.Fatal(err) + } + if dir, err := OpenDirectory(filepath.Join(path, "logs")); err == nil { + _ = dir.Close() + t.Fatal("ACL write grant accepted") + } +} diff --git a/internal/safefile/directory.go b/internal/safefile/directory.go new file mode 100644 index 0000000..488544e --- /dev/null +++ b/internal/safefile/directory.go @@ -0,0 +1,205 @@ +// Package safefile opens supervisor-owned files without following leaf symlinks. +// All subsequent mutations are relative to a verified, retained directory fd. +package safefile + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "strings" + + "golang.org/x/sys/unix" +) + +type Directory struct { + file *os.File + Path string + Key string +} + +func trusted(uid uint32) bool { return uid == 0 || uid == uint32(os.Geteuid()) } + +// OpenDirectory permits root/current-user-owned ancestors, including sticky +// temporary directories. Symlinks in trusted ancestors are resolved explicitly; +// every target ancestor is checked again. A workload UID must not own this path. +func OpenDirectory(path string) (*Directory, error) { + abs, err := filepath.Abs(path) + if err != nil { + return nil, err + } + fd, err := unix.Open("/", unix.O_RDONLY|unix.O_DIRECTORY|unix.O_CLOEXEC, 0) + if err != nil { + return nil, err + } + defer func() { + if fd >= 0 { + _ = unix.Close(fd) + } + }() + parts := strings.Split(strings.TrimPrefix(abs, "/"), "/") + resolved := "/" + links := 0 + for len(parts) > 0 { + part := parts[0] + parts = parts[1:] + if part == "" || part == "." { + continue + } + if err := checkDirectory(fd); err != nil { + return nil, fmt.Errorf("unsafe directory %s: %w", resolved, err) + } + var st unix.Stat_t + err = unix.Fstatat(fd, part, &st, unix.AT_SYMLINK_NOFOLLOW) + if errors.Is(err, unix.ENOENT) { + if err = unix.Mkdirat(fd, part, 0750); err != nil && !errors.Is(err, unix.EEXIST) { + return nil, err + } + err = unix.Fstatat(fd, part, &st, unix.AT_SYMLINK_NOFOLLOW) + } + if err != nil { + return nil, err + } + if !trusted(st.Uid) { + return nil, fmt.Errorf("untrusted owner of %s", filepath.Join(resolved, part)) + } + if st.Mode&unix.S_IFMT == unix.S_IFLNK { + links++ + if links > 40 { + return nil, fmt.Errorf("too many directory symlinks") + } + buf := make([]byte, 4096) + n, e := unix.Readlinkat(fd, part, buf) + if e != nil { + return nil, e + } + if n == len(buf) { + return nil, fmt.Errorf("symlink target too long") + } + target := string(buf[:n]) + if filepath.IsAbs(target) { + _ = unix.Close(fd) + fd, err = unix.Open("/", unix.O_RDONLY|unix.O_DIRECTORY|unix.O_CLOEXEC, 0) + if err != nil { + return nil, err + } + resolved = "/" + } + parts = append(strings.Split(target, "/"), parts...) + continue + } + next, err := unix.Openat(fd, part, unix.O_RDONLY|unix.O_DIRECTORY|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0) + if err != nil { + return nil, err + } + _ = unix.Close(fd) + fd = next + resolved = filepath.Join(resolved, part) + } + if err := checkDirectory(fd); err != nil { + return nil, fmt.Errorf("unsafe directory %s: %w", resolved, err) + } + var st unix.Stat_t + if err := unix.Fstat(fd, &st); err != nil { + return nil, err + } + d := &Directory{file: os.NewFile(uintptr(fd), resolved), Path: resolved, Key: fmt.Sprintf("%d:%d", st.Dev, st.Ino)} + fd = -1 + return d, nil +} +func checkDirectory(fd int) error { + var st unix.Stat_t + if err := unix.Fstat(fd, &st); err != nil { + return err + } + if !trusted(st.Uid) { + return fmt.Errorf("directory is owned by UID %d", st.Uid) + } + if st.Mode&0022 != 0 && st.Mode&unix.S_ISVTX == 0 { + return fmt.Errorf("directory is writable by another user") + } + return checkACL(fd) +} +func (d *Directory) Close() error { return d.file.Close() } +func (d *Directory) FD() int { return int(d.file.Fd()) } +func validName(name string) bool { + return name != "" && name != "." && name != ".." && filepath.Base(name) == name +} + +// OpenRegular validates before chmod; O_NONBLOCK prevents a FIFO from hanging +// startup. O_NOFOLLOW and nlink==1 exclude symlink and hardlink targets. +func (d *Directory) OpenRegular(name string, flags int, mode os.FileMode) (*os.File, error) { + if !validName(name) { + return nil, fmt.Errorf("invalid file name %q", name) + } + if mode.Perm()&0022 != 0 { + return nil, fmt.Errorf("file mode %04o permits other users to write", mode.Perm()) + } + if err := checkDirectory(d.FD()); err != nil { + return nil, err + } + // Reject existing special files before opening them: even a nonblocking + // device open can have side effects. The descriptor is validated again below. + var before unix.Stat_t + if err := unix.Fstatat(d.FD(), name, &before, unix.AT_SYMLINK_NOFOLLOW); err == nil { + if before.Mode&unix.S_IFMT != unix.S_IFREG { + return nil, fmt.Errorf("not a regular file: %s", name) + } + } else if !errors.Is(err, unix.ENOENT) { + return nil, err + } + if flags&unix.O_TRUNC != 0 { + return nil, fmt.Errorf("truncate only after descriptor validation") + } + fd, err := unix.Openat(d.FD(), name, flags|unix.O_NOFOLLOW|unix.O_NONBLOCK|unix.O_CLOEXEC, uint32(mode.Perm())) + if err != nil { + return nil, err + } + f := os.NewFile(uintptr(fd), filepath.Join(d.Path, name)) + ok := false + defer func() { + if !ok { + _ = f.Close() + } + }() + var st unix.Stat_t + if err = unix.Fstat(fd, &st); err != nil { + return nil, err + } + if st.Mode&unix.S_IFMT != unix.S_IFREG || st.Nlink != 1 || !trusted(st.Uid) || st.Mode&0022 != 0 { + return nil, fmt.Errorf("unsafe file %s: require a trusted, singly linked regular file without other-user write access", name) + } + if err = checkACL(fd); err != nil { + return nil, err + } + if flags&(unix.O_WRONLY|unix.O_RDWR) != 0 { + if err = f.Chmod(mode.Perm()); err != nil { + return nil, err + } + } + ok = true + return f, nil +} +func (d *Directory) Remove(name string) error { + if !validName(name) { + return fmt.Errorf("invalid file name") + } + return unix.Unlinkat(d.FD(), name, 0) +} + +// Link publishes a name exclusively: an existing destination is never replaced. +func (d *Directory) Link(from, to string) error { + if !validName(from) || !validName(to) { + return fmt.Errorf("invalid file name") + } + return unix.Linkat(d.FD(), from, d.FD(), to, 0) +} +func (d *Directory) Entries() ([]os.DirEntry, error) { + fd, err := unix.Openat(d.FD(), ".", unix.O_RDONLY|unix.O_DIRECTORY|unix.O_CLOEXEC, 0) + if err != nil { + return nil, err + } + f := os.NewFile(uintptr(fd), d.Path) + defer f.Close() + return f.ReadDir(-1) +} diff --git a/internal/safefile/directory_test.go b/internal/safefile/directory_test.go new file mode 100644 index 0000000..49ce6e7 --- /dev/null +++ b/internal/safefile/directory_test.go @@ -0,0 +1,118 @@ +package safefile + +import ( + "os" + "path/filepath" + "testing" + + "golang.org/x/sys/unix" +) + +func secureDir(t *testing.T) string { + t.Helper() + path := t.TempDir() + if err := os.Chmod(path, 0700); err != nil { + t.Fatal(err) + } + return path +} +func TestRejectLeafSymlinkHardlinkFIFOWithoutTouchingVictim(t *testing.T) { + for _, kind := range []string{"symlink", "hardlink", "fifo"} { + t.Run(kind, func(t *testing.T) { + path := secureDir(t) + victim := filepath.Join(path, "victim") + if err := os.WriteFile(victim, []byte("unchanged"), 0400); err != nil { + t.Fatal(err) + } + target := filepath.Join(path, "log") + var err error + switch kind { + case "symlink": + err = os.Symlink(victim, target) + case "hardlink": + err = os.Link(victim, target) + case "fifo": + err = unix.Mkfifo(target, 0600) + } + if err != nil { + t.Fatal(err) + } + dir, err := OpenDirectory(path) + if err != nil { + t.Fatal(err) + } + defer dir.Close() + if f, err := dir.OpenRegular("log", os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600); err == nil { + _ = f.Close() + t.Fatal("unsafe file accepted") + } + b, err := os.ReadFile(victim) + if err != nil || string(b) != "unchanged" { + t.Fatal("victim changed") + } + st, _ := os.Stat(victim) + if st.Mode().Perm() != 0400 { + t.Fatal("victim mode changed") + } + }) + } +} +func TestRejectWritableAncestorAndAllowTrustedSymlinks(t *testing.T) { + path := secureDir(t) + unsafe := filepath.Join(path, "unsafe") + if err := os.Mkdir(unsafe, 0777); err != nil { + t.Fatal(err) + } + _ = os.Chmod(unsafe, 0777) + if dir, err := OpenDirectory(filepath.Join(unsafe, "child")); err == nil { + _ = dir.Close() + t.Fatal("accepted writable ancestor") + } + target := filepath.Join(path, "trusted") + if err := os.Mkdir(target, 0700); err != nil { + t.Fatal(err) + } + alias := filepath.Join(path, "alias") + if err := os.Symlink(target, alias); err != nil { + t.Fatal(err) + } + dir, err := OpenDirectory(alias) + if err != nil { + t.Fatal(err) + } + defer dir.Close() + f, err := dir.OpenRegular("log", os.O_CREATE|os.O_WRONLY, 0600) + if err != nil { + t.Fatal(err) + } + _ = f.Close() + if _, err := os.Stat(filepath.Join(target, "log")); err != nil { + t.Fatal(err) + } +} +func TestStickyAncestorAndDescriptorSurviveRename(t *testing.T) { + path := secureDir(t) + sticky := filepath.Join(path, "sticky") + if err := os.Mkdir(sticky, 0777); err != nil { + t.Fatal(err) + } + _ = os.Chmod(sticky, os.ModeSticky|0777) + logdir := filepath.Join(sticky, "logs") + dir, err := OpenDirectory(logdir) + if err != nil { + t.Fatal(err) + } + defer dir.Close() + moved := filepath.Join(sticky, "moved") + if err := os.Rename(logdir, moved); err != nil { + t.Fatal(err) + } + f, err := dir.OpenRegular("log", os.O_CREATE|os.O_WRONLY, 0600) + if err != nil { + t.Fatal(err) + } + _ = f.Close() + if _, err := os.Stat(filepath.Join(moved, "log")); err != nil { + t.Fatal(err) + } +} diff --git a/scripts/check-artifacts.sh b/scripts/check-artifacts.sh new file mode 100644 index 0000000..c340fc0 --- /dev/null +++ b/scripts/check-artifacts.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash +set -euo pipefail +# With an argument, scan the exact artifacts that will be published. +artifact_dir="${1:-$(mktemp -d)}" +if [ "$#" -eq 0 ]; then + trap 'rm -rf "$artifact_dir"' EXIT +fi +for target_os in linux darwin; do + for target_arch in amd64 arm64; do + artifact="$artifact_dir/gonner-$target_os-$target_arch" + if [ "$#" -eq 0 ]; then + GOOS="$target_os" GOARCH="$target_arch" CGO_ENABLED=0 go build -ldflags="-s -w" -o "$artifact" ./cmd/gonner + fi + govulncheck -mode=binary "$artifact" + done +done diff --git a/scripts/release_bump.py b/scripts/release_bump.py new file mode 100644 index 0000000..0961e6d --- /dev/null +++ b/scripts/release_bump.py @@ -0,0 +1,17 @@ +"""Read untrusted label data as JSON, never as shell source.""" +import json +import os + + +def release_bump(raw): + labels = json.loads(raw) + if not isinstance(labels, list) or not all(isinstance(label, str) for label in labels): + raise ValueError("labels must be a JSON array of strings") + for level in ("major", "minor"): + if "release:" + level in labels: + return level + return "patch" + + +if __name__ == "__main__": + print("level=" + release_bump(os.environ["PR_LABELS_JSON"])) diff --git a/scripts/test_release_bump.py b/scripts/test_release_bump.py new file mode 100644 index 0000000..1dfc163 --- /dev/null +++ b/scripts/test_release_bump.py @@ -0,0 +1,35 @@ +import json +import unittest +from release_bump import release_bump + + +class ReleaseBumpTests(unittest.TestCase): + def test_exact_membership_and_priority(self): + for labels, expected in [([], "patch"), (["release:minor"], "minor"), + (["release:minor", "release:major"], "major"), + (["xrelease:major", "release:minor suffix"], "patch")]: + self.assertEqual(release_bump(json.dumps(labels)), expected) + + def test_shell_and_newline_payloads_remain_data(self): + labels = ["'; touch /tmp/gonner-label-injected; #", "$(exit 99)", + "`exit 99`", "\nlevel=major", '"release:major"'] + self.assertEqual(release_bump(json.dumps(labels)), "patch") + + def test_rejects_non_array(self): + with self.assertRaises(ValueError): + release_bump('{"release:major": true}') + + def test_cli_treats_injection_payload_as_environment_data(self): + import os + from pathlib import Path + import subprocess + import sys + import tempfile + with tempfile.TemporaryDirectory() as directory: + marker = Path(directory) / "injected" + payload = "'; touch " + str(marker) + "; #" + environment = dict(os.environ, PR_LABELS_JSON=json.dumps([payload, "release:minor"])) + result = subprocess.run([sys.executable, str(Path(__file__).with_name("release_bump.py"))], + env=environment, text=True, capture_output=True, check=True) + self.assertEqual(result.stdout, "level=minor\n") + self.assertFalse(marker.exists())