diff --git a/AGENTS.md b/AGENTS.md index 0944f79..ba0eccc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -25,23 +25,26 @@ go vet ./... # vet - `internal/output/output.go` — colored/emoji output with `--no-color` flag support - `internal/output/output_test.go` — ANSI stripping verification - `internal/args/args.go` — argument validation (enum, glob, regex) +- `internal/trust/trust.go` — direnv-style trust store; content-hashed gate over the local config -Tests: `cmd/root_test.go`, `internal/{config,checker,version,output,args}/*_test.go` +Tests: `cmd/root_test.go`, `internal/{config,checker,version,output,args,trust}/*_test.go` ## Working conventions - Verbs are defined in YAML, not hardcoded. Adding a new verb means editing config, not code. - Config schema: - - `shell_options: "set -euo pipefail"` — prepended to all shell scripts (multiline `cmd` and all `cmds` items) + - `shell_options: "set -euo pipefail"` — prepended to all shell scripts (every `cmd` and all `cmds` items) - `commands..{cmd, cmds, env, description, arguments[]}` — `cmd` is a string, `cmds` is a list of strings, `env` is a map of environment variables; arguments are objects with `name`, optional `values` (enum), optional `match` (glob or regex), optional `exclude` (list of disallowed values) - `tools..{min_version, max_version, version_cmd, download_url}` — pre-flight checks run before every verb - Arguments after the verb are mapped positionally to `arguments` entries and expanded into `${name}` placeholders in `cmd` or `cmds` -- `cmd` supports multiline YAML block scalars (`|`): if single-line, runs as a command; if multi-line, runs as a shell script via `sh -c` +- `cmd` runs via `sh -c` (single-line and multiline block scalars alike), so quoting, pipes, and shell operators work; multiline blocks run as a shell script - `cmds` is a list of commands; each item runs via `sh -c`, so shell features (variables, subshells, pipes) work; multi-line items also run as shell scripts - `env` sets environment variables for the command execution; merged with current environment -- `match` is auto-detected: contains `*` or `?` → glob (checks files on disk); otherwise → regex (full string match, auto-anchored) +- `match` is auto-detected: contains `*` or `?` → glob (checks files on disk); otherwise → regex (full string match, anchored as `^(?:pattern)$` so top-level alternation stays bound) - Glob matching accepts full path, basename, basename without extension, or directory name - `exclude` filters out values from glob matches and rejects them during validation; excluded values are hidden from help output - `ugo check` runs tool checks and prints status for each tool - Version comparison uses `golang.org/x/mod/semver`; `version_cmd` output is scanned for a semver pattern - Running a verb without required arguments (or with invalid args) prints the error then the help, then exits +- Security model: config (global + local-from-CWD) and argument/prompt values are trusted; `cmd`/`cmds` run via `sh -c` and `${name}` values are expanded as unquoted shell text. Constrain untrusted args with `values`/`match`. Sensitive prompt values are masked in uGo's output only — they still reach the shell (visible in `ps`, and to `set -x`). See README "Security". +- Trust gate: the local (CWD) config must be trusted before any verb or `check` executes. Trust is content-hashed (path + SHA-256) in `~/.config//trust.json`; editing the config revokes it. `PersistentPreRunE` prompts on a TTY; `--trust` records trust without prompting (CI/CD); non-interactive + untrusted aborts. `help`/`version` are never gated. The global config is implicitly trusted. diff --git a/README.md b/README.md index b439270..c5d1d50 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,7 @@ uGo loads configuration from two locations and merges them (local overrides glob ### Shell options -Set `shell_options` to prepend shell flags to all commands that run via `sh -c` (multiline `cmd` and all `cmds` items): +Set `shell_options` to prepend shell flags to all commands that run via `sh -c` (every `cmd` — single-line and multiline — and all `cmds` items): ```yaml shell_options: "set -euo pipefail" @@ -119,7 +119,7 @@ tools: ```yaml commands: : - cmd: "" # string: runs directly (single-line) or as shell script (multi-line) + cmd: "" # string: runs via "sh -c", so quoting, pipes, and && work cmds: # list: each item runs via sh -c (supports shell features) - "echo ${arg}" - | @@ -216,7 +216,16 @@ Arguments support three validation modes: Glob vs regex is auto-detected: if the pattern contains `*` or `?` it's treated as a file glob. -### Multiline commands +### Command execution + +Every `cmd` runs via `sh -c`, so shell features — quoting, embedded whitespace, pipes (`|`), and operators (`&&`, `||`, redirects) — behave as written: + +```yaml +commands: + plan: + cmd: terraform workspace select "${workspace}" && terraform plan + description: "Select a workspace and plan" +``` **`cmd` with multiline** — runs the entire block as a shell script via `sh -c`: @@ -335,6 +344,58 @@ $ ugo check ❌ Tool checks failed ``` +## Security + +uGo's job is to run commands you have configured, so **the configuration and the argument values you pass are trusted inputs**. A few properties are worth understanding: + +### Configuration is loaded from the working directory + +uGo reads `./.yaml` from the current directory and merges it over your global config. Running a verb — or `ugo check` — executes the `version_cmd` of each configured tool found on your `$PATH`, and runs the verb's `cmd`/`cmds` via `sh -c`. Changing into a directory and running a verb therefore runs *that directory's* configuration, the same way `make`, `npm run`, or a `./go` script would. + +To guard against running an unfamiliar repository's config, uGo gates the **local** config behind a trust prompt (see [Trusting a directory](#trusting-a-directory)). The global config (`~/.config//config.yaml`) is user-owned and always trusted. `ugo --help` and `ugo version` never execute anything from the config, so they are safe to run anywhere. + +### Trusting a directory + +The first time you run a verb (or `ugo check`) in a directory with a local config, uGo asks before executing anything: + +```bash +$ ugo build + + ⚠️ /home/me/project/ugo.yaml is not trusted. + Running a verb here will execute the commands defined in this file. + Trust it? [y/N]: +``` + +Answering `y` records the config as trusted and runs it; anything else aborts without executing. Trust is **content-addressed**: uGo stores the path together with a SHA-256 of the file's contents in `~/.config//trust.json`. If the config is later edited (e.g. a `git pull` changes it), trust is automatically revoked and you are prompted again. + +For non-interactive use (CI/CD), pass `--trust` to skip the prompt and record the config as trusted: + +```bash +ugo --trust build +``` + +When no terminal is attached and `--trust` is not given, uGo refuses to run rather than executing an untrusted config silently. To revoke trust, delete the relevant entry (or the whole file) from `~/.config//trust.json`. + +### Argument and prompt values are expanded as shell text + +`${name}` placeholders are substituted into the command string *before* it is handed to `sh -c`, and the values are **not** shell-quoted. A value like `foo; rm -rf ~` placed in an unconstrained argument runs as written. + +If a verb can receive values from an untrusted or external source (CI variables, webhooks, etc.), constrain those arguments: + +- `values: [...]` — accept only an explicit set (exact match), or +- `match: ""` — validate against a fully-anchored regex, or an on-disk glob. + +Arguments with neither are accepted verbatim. Unresolved `${name}` placeholders (a typo, or a deliberate `$HOME`) are passed through and expanded by the shell. + +### Secrets + +`sensitive: true` masks a prompt's value **in uGo's own output only** — the `🚀` line shows `********`. The real value is still expanded into the command, which means: + +- it is passed to `sh -c`, so it can be visible in the process list (`ps`, `/proc`) to other local users while the command runs; and +- enabling shell tracing through `shell_options` (e.g. `set -x`) echoes the expanded command — including the secret — to stderr. + +Prefer passing secrets through the environment — reference a `${prompt}` from an `env:` value and use `"$VAR"` in the command, rather than interpolating the secret directly into `cmd` — and avoid `set -x` when handling sensitive prompts. + ## Colored Output uGo uses UTF-8 icons and colors for status output. Use `--no-color` to disable: diff --git a/cmd/root.go b/cmd/root.go index e51eb24..f5485b2 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -3,8 +3,11 @@ package cmd import ( "bufio" "fmt" + "io" "os" "os/exec" + "path/filepath" + "sort" "strings" "github.com/spf13/cobra" @@ -12,6 +15,7 @@ import ( "github.com/PyratLabs/ugo/internal/checker" "github.com/PyratLabs/ugo/internal/config" "github.com/PyratLabs/ugo/internal/output" + "github.com/PyratLabs/ugo/internal/trust" "golang.org/x/term" ) @@ -21,6 +25,7 @@ var ( binaryName string appCfg *config.Config noColor bool + trustFlag bool ) func RootCmd() *cobra.Command { @@ -49,7 +54,18 @@ Local config overrides global config for the same verb names.`, CompletionOptions: cobra.CompletionOptions{DisableDefaultCmd: true}, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { output.SetNoColor(noColor) - if cmd.Name() == "check" || cmd.Name() == "help" || cmd.Name() == "version" { + // help and version never execute anything from the config. + if cmd.Name() == "help" || cmd.Name() == "version" { + return nil + } + // Everything else (verbs and check) can run config-defined + // commands, so the local config must be trusted first. + if err := enforceTrust(); err != nil { + output.CheckFail(err.Error()) + os.Exit(1) + } + // check does its own tool checking in its Run. + if cmd.Name() == "check" { return nil } return runToolChecks() @@ -57,6 +73,7 @@ Local config overrides global config for the same verb names.`, } root.PersistentFlags().BoolVar(&noColor, "no-color", false, "disable color output") + root.PersistentFlags().BoolVar(&trustFlag, "trust", false, "trust this directory's config without prompting (for CI/CD)") root.SetFlagErrorFunc(func(c *cobra.Command, err error) error { return fmt.Errorf("unknown flag: %s\nRun '%s help' for usage", err.Error(), binaryName) }) @@ -116,7 +133,13 @@ func printToolStatus(tools map[string]config.Tool, issues []checker.Issue) { issueMap[i.Tool] = i } + names := make([]string, 0, len(tools)) for name := range tools { + names = append(names, name) + } + sort.Strings(names) + + for _, name := range names { if issue, ok := issueMap[name]; ok { for _, e := range issue.Errors { if strings.HasPrefix(e, "version:") { @@ -154,9 +177,91 @@ func runToolChecks() error { return nil } +// enforceTrust gates execution of config-defined commands behind the trust +// store, prompting on os.Stdin or honoring --trust. It is thin glue over +// trustGate so the latter stays free of globals and easy to test. +func enforceTrust() error { + _, localPath := config.ConfigPaths(binaryName) + storePath, err := trust.DefaultStorePath(binaryName) + if err != nil { + return err + } + interactive := term.IsTerminal(int(os.Stdin.Fd())) + return trustGate(localPath, storePath, bufio.NewReader(os.Stdin), os.Stderr, trustFlag, interactive) +} + +// trustGate decides whether the local config may be executed. It returns nil to +// allow execution or an error explaining why it is blocked. allow corresponds +// to --trust; interactive reports whether prompting is possible. +func trustGate(localPath, storePath string, in *bufio.Reader, out io.Writer, allow, interactive bool) error { + // Only the working-directory config is gated; the global config is + // user-owned and implicitly trusted. + if !fileExists(localPath) { + return nil + } + + absPath, err := filepath.Abs(localPath) + if err != nil { + return err + } + hash, err := trust.HashFile(localPath) + if err != nil { + return fmt.Errorf("reading %s: %w", localPath, err) + } + + store, err := trust.Load(storePath) + if err != nil { + return fmt.Errorf("loading trust store: %w", err) + } + + status := store.Status(absPath, hash) + if status == trust.Trusted { + return nil + } + + // --trust: skip the prompt and record trust (best-effort, so a read-only + // home in CI/CD doesn't fail the run). + if allow { + if err := store.Trust(absPath, hash); err != nil { + fmt.Fprintf(out, " warning: could not record trust for %s: %v\n", absPath, err) + } + return nil + } + + if !interactive { + return fmt.Errorf("%s is not trusted; re-run with --trust to allow it (e.g. in CI/CD)", localPath) + } + + fmt.Fprintln(out) + if status == trust.Changed { + fmt.Fprintf(out, " ⚠️ %s has changed since it was last trusted.\n", absPath) + } else { + fmt.Fprintf(out, " ⚠️ %s is not trusted.\n", absPath) + } + fmt.Fprintln(out, " Running a verb here will execute the commands defined in this file.") + fmt.Fprint(out, " Trust it? [y/N]: ") + + line, _ := in.ReadString('\n') + switch strings.ToLower(strings.TrimSpace(line)) { + case "y", "yes": + if err := store.Trust(absPath, hash); err != nil { + return fmt.Errorf("recording trust: %w", err) + } + fmt.Fprintf(out, " trusted %s\n\n", absPath) + return nil + default: + return fmt.Errorf("%s not trusted; aborting", localPath) + } +} + +func fileExists(path string) bool { + info, err := os.Stat(path) + return err == nil && !info.IsDir() +} + func buildCommand(name string, def config.Command) *cobra.Command { c := &cobra.Command{ - Use: buildUse(name, def.Arguments, def.Prompts), + Use: buildUse(name, def.Arguments), Short: def.Description, Long: buildLong(def.Arguments, def.Prompts), RunE: func(cmd *cobra.Command, args []string) error { @@ -190,7 +295,7 @@ func buildLong(arguments []config.Argument, prompts []config.Prompt) string { b.WriteString("(no files found)") } } else { - b.WriteString(fmt.Sprintf("^%s$", arg.Match)) + b.WriteString(fmt.Sprintf("^(?:%s)$", arg.Match)) } default: b.WriteString("(no validation)") @@ -217,7 +322,7 @@ func buildLong(arguments []config.Argument, prompts []config.Prompt) string { return b.String() } -func buildUse(name string, arguments []config.Argument, prompts []config.Prompt) string { +func buildUse(name string, arguments []config.Argument) string { if len(arguments) == 0 { return name } @@ -264,7 +369,10 @@ func executeCommand(cmd *cobra.Command, name string, def config.Command, values } } - // Prompt if no value from env var + // Prompt if there is still no value. Note a set-but-empty from_env_var + // (e.g. TOKEN="") intentionally falls through to the interactive + // prompt — this matches the documented "unset or empty" behavior, so + // an empty env var cannot be used to supply an empty answer. if value == "" { var err error if p.Sensitive { @@ -344,30 +452,26 @@ func executeCmdString(name, cmdStr string, vars map[string]string, env map[strin displayVars := output.MaskedVars(vars, sensitiveNames) display := expandVars(cmdStr, displayVars) + if strings.TrimSpace(expanded) == "" { + output.CommandSuccess(name) + return nil + } + + // Single-line and multiline cmd both run via "sh -c" so that quoting, + // embedded whitespace, and shell operators (&&, |, redirects) behave as + // written rather than being split on whitespace into argv. if strings.Contains(expanded, "\n") { output.CommandRunning(name, "shell script") - if err := runShellScript(expanded, env, shellOpts); err != nil { - output.CommandFail(name) - if exitErr, ok := err.(*exec.ExitError); ok { - os.Exit(exitErr.ExitCode()) - } - os.Exit(1) - } } else { - parts := strings.Fields(expanded) - if len(parts) == 0 { - output.CommandSuccess(name) - return nil - } - output.CommandRunning(name, display) - if err := runCommand(expanded, env); err != nil { - output.CommandFail(name) - if exitErr, ok := err.(*exec.ExitError); ok { - os.Exit(exitErr.ExitCode()) - } - os.Exit(1) + } + + if err := runShellScript(expanded, env, shellOpts); err != nil { + output.CommandFail(name) + if exitErr, ok := err.(*exec.ExitError); ok { + os.Exit(exitErr.ExitCode()) } + os.Exit(1) } output.CommandSuccess(name) @@ -396,19 +500,6 @@ func expandEnv(env map[string]string, vars map[string]string) map[string]string return expanded } -func runCommand(cmdStr string, env map[string]string) error { - parts := strings.Fields(cmdStr) - if len(parts) == 0 { - return nil - } - command := exec.Command(parts[0], parts[1:]...) - command.Stdout = os.Stdout - command.Stderr = os.Stderr - command.Stdin = os.Stdin - applyEnv(command, env) - return command.Run() -} - func runShellScript(script string, env map[string]string, shellOpts string) error { if shellOpts != "" { script = shellOpts + "\n" + script diff --git a/cmd/root_test.go b/cmd/root_test.go index 3791fdd..738c760 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -1,6 +1,7 @@ package cmd import ( + "bufio" "bytes" "os" "path/filepath" @@ -15,7 +16,6 @@ func TestBuildUse(t *testing.T) { name string verb string arguments []config.Argument - prompts []config.Prompt want string }{ { @@ -42,26 +42,13 @@ func TestBuildUse(t *testing.T) { arguments: []config.Argument{}, want: "test", }, - { - name: "prompts only", - verb: "login", - prompts: []config.Prompt{{Name: "password", Description: "Enter password", Sensitive: true}}, - want: "login", - }, - { - name: "arguments and prompts", - verb: "deploy", - arguments: []config.Argument{{Name: "env"}}, - prompts: []config.Prompt{{Name: "token", Description: "API token", Sensitive: true}}, - want: "deploy ", - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := buildUse(tt.verb, tt.arguments, tt.prompts) + got := buildUse(tt.verb, tt.arguments) if got != tt.want { - t.Errorf("buildUse(%q, %v, %v) = %q, want %q", tt.verb, tt.arguments, tt.prompts, got, tt.want) + t.Errorf("buildUse(%q, %v) = %q, want %q", tt.verb, tt.arguments, got, tt.want) } }) } @@ -217,52 +204,20 @@ func TestBuildCommand(t *testing.T) { } func TestRootCmdExecute(t *testing.T) { - oldArgs := os.Args - defer func() { os.Args = oldArgs }() - - dir := t.TempDir() - configPath := filepath.Join(dir, "execgo.yaml") - if err := os.WriteFile(configPath, []byte(` + out := runVerb(t, "execgo", ` commands: hello: cmd: echo hello description: "Say hello" -`), 0644); err != nil { - t.Fatal(err) - } - - os.Args = []string{"execgo"} - os.Chdir(dir) - - root := RootCmd() - - // Capture stdout since the command writes to os.Stdout - oldStdout := os.Stdout - r, w, _ := os.Pipe() - os.Stdout = w - - root.SetArgs([]string{"hello"}) - if err := root.Execute(); err != nil { - t.Fatalf("Execute() error = %v", err) - } - - w.Close() - var out bytes.Buffer - out.ReadFrom(r) - os.Stdout = oldStdout +`, "hello") - if !strings.Contains(out.String(), "hello") { - t.Errorf("output = %q, want to contain %q", out.String(), "hello") + if !strings.Contains(out, "hello") { + t.Errorf("output = %q, want to contain %q", out, "hello") } } func TestRootCmdExecuteMultiline(t *testing.T) { - oldArgs := os.Args - defer func() { os.Args = oldArgs }() - - dir := t.TempDir() - configPath := filepath.Join(dir, "multigo.yaml") - if err := os.WriteFile(configPath, []byte(` + out := runVerb(t, "multigo", ` commands: multi: cmd: | @@ -270,44 +225,17 @@ commands: echo step2 echo step3 description: "Run multiple commands" -`), 0644); err != nil { - t.Fatal(err) - } - - os.Args = []string{"multigo"} - os.Chdir(dir) - - root := RootCmd() - - oldStdout := os.Stdout - r, w, _ := os.Pipe() - os.Stdout = w - - root.SetArgs([]string{"multi"}) - if err := root.Execute(); err != nil { - t.Fatalf("Execute() error = %v", err) - } +`, "multi") - w.Close() - var out bytes.Buffer - out.ReadFrom(r) - os.Stdout = oldStdout - - output := out.String() for _, want := range []string{"step1", "step2", "step3"} { - if !strings.Contains(output, want) { - t.Errorf("output = %q, want to contain %q", output, want) + if !strings.Contains(out, want) { + t.Errorf("output = %q, want to contain %q", out, want) } } } func TestRootCmdExecuteCmdsList(t *testing.T) { - oldArgs := os.Args - defer func() { os.Args = oldArgs }() - - dir := t.TempDir() - configPath := filepath.Join(dir, "cmdsgo.yaml") - if err := os.WriteFile(configPath, []byte(` + out := runVerb(t, "cmdsgo", ` commands: deploy: cmds: @@ -315,44 +243,17 @@ commands: - echo "step 2" - echo "step 3" description: "Run multiple commands" -`), 0644); err != nil { - t.Fatal(err) - } - - os.Args = []string{"cmdsgo"} - os.Chdir(dir) - - root := RootCmd() - - oldStdout := os.Stdout - r, w, _ := os.Pipe() - os.Stdout = w - - root.SetArgs([]string{"deploy"}) - if err := root.Execute(); err != nil { - t.Fatalf("Execute() error = %v", err) - } - - w.Close() - var out bytes.Buffer - out.ReadFrom(r) - os.Stdout = oldStdout +`, "deploy") - output := out.String() for _, want := range []string{"step 1", "step 2", "step 3"} { - if !strings.Contains(output, want) { - t.Errorf("output = %q, want to contain %q", output, want) + if !strings.Contains(out, want) { + t.Errorf("output = %q, want to contain %q", out, want) } } } func TestRootCmdExecuteCmdsMultiline(t *testing.T) { - oldArgs := os.Args - defer func() { os.Args = oldArgs }() - - dir := t.TempDir() - configPath := filepath.Join(dir, "cmdsmulti.yaml") - if err := os.WriteFile(configPath, []byte(` + out := runVerb(t, "cmdsmulti", ` commands: script: cmds: @@ -360,33 +261,11 @@ commands: echo "line1" echo "line2" description: "Run multi-line script" -`), 0644); err != nil { - t.Fatal(err) - } - - os.Args = []string{"cmdsmulti"} - os.Chdir(dir) - - root := RootCmd() - - oldStdout := os.Stdout - r, w, _ := os.Pipe() - os.Stdout = w - - root.SetArgs([]string{"script"}) - if err := root.Execute(); err != nil { - t.Fatalf("Execute() error = %v", err) - } - - w.Close() - var out bytes.Buffer - out.ReadFrom(r) - os.Stdout = oldStdout +`, "script") - output := out.String() for _, want := range []string{"line1", "line2"} { - if !strings.Contains(output, want) { - t.Errorf("output = %q, want to contain %q", output, want) + if !strings.Contains(out, want) { + t.Errorf("output = %q, want to contain %q", out, want) } } } @@ -415,7 +294,7 @@ func TestBuildLong(t *testing.T) { arguments: []config.Argument{ {Name: "service", Match: "[a-z][a-z0-9-]+"}, }, - want: []string{"^[a-z][a-z0-9-]+$"}, + want: []string{"^(?:[a-z][a-z0-9-]+)$"}, }, { name: "no validation", @@ -462,6 +341,219 @@ func TestBuildLong(t *testing.T) { } } +// runVerb writes configYAML to a temp .yaml, builds the root +// command, runs the given args, and returns captured stdout. It sandboxes HOME +// (so the trust store and global config live in a temp dir) and passes --trust +// so the local config is accepted without prompting. +func runVerb(t *testing.T, binaryName, configYAML string, args ...string) string { + t.Helper() + + t.Setenv("HOME", t.TempDir()) + + oldArgs := os.Args + defer func() { os.Args = oldArgs }() + + dir := t.TempDir() + configPath := filepath.Join(dir, binaryName+".yaml") + if err := os.WriteFile(configPath, []byte(configYAML), 0644); err != nil { + t.Fatal(err) + } + + oldWd, _ := os.Getwd() + defer func() { _ = os.Chdir(oldWd) }() + + os.Args = []string{binaryName} + if err := os.Chdir(dir); err != nil { + t.Fatal(err) + } + + root := RootCmd() + + oldStdout := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + + root.SetArgs(append([]string{"--trust"}, args...)) + execErr := root.Execute() + + w.Close() + var buf bytes.Buffer + buf.ReadFrom(r) + os.Stdout = oldStdout + + if execErr != nil { + t.Fatalf("Execute() error = %v", execErr) + } + return buf.String() +} + +// containsLine reports whether any line of s, trimmed of surrounding +// whitespace, equals want. Used to assert on a command's actual output +// rather than the echoed "running" display line. +func containsLine(s, want string) bool { + for line := range strings.SplitSeq(s, "\n") { + if strings.TrimSpace(line) == want { + return true + } + } + return false +} + +// trustFixture writes a local config and returns its path plus a fresh +// (empty) trust store path. +func trustFixture(t *testing.T) (localPath, storePath string) { + t.Helper() + dir := t.TempDir() + localPath = filepath.Join(dir, "ugo.yaml") + if err := os.WriteFile(localPath, []byte("commands:\n build:\n cmd: echo hi\n"), 0644); err != nil { + t.Fatal(err) + } + storePath = filepath.Join(t.TempDir(), "trust.json") + return localPath, storePath +} + +func gate(localPath, storePath, answer string, allow, interactive bool) (string, error) { + var out bytes.Buffer + in := bufio.NewReader(strings.NewReader(answer)) + err := trustGate(localPath, storePath, in, &out, allow, interactive) + return out.String(), err +} + +func TestTrustGate(t *testing.T) { + t.Run("no local config is allowed", func(t *testing.T) { + missing := filepath.Join(t.TempDir(), "nope.yaml") + storePath := filepath.Join(t.TempDir(), "trust.json") + if _, err := gate(missing, storePath, "", false, false); err != nil { + t.Errorf("expected nil for absent local config, got %v", err) + } + }) + + t.Run("--trust bypasses prompt and records trust", func(t *testing.T) { + localPath, storePath := trustFixture(t) + if _, err := gate(localPath, storePath, "", true /*allow*/, false); err != nil { + t.Fatalf("--trust should allow, got %v", err) + } + // A subsequent non-interactive run must now pass without --trust. + if _, err := gate(localPath, storePath, "", false, false); err != nil { + t.Errorf("config should be trusted after --trust, got %v", err) + } + }) + + t.Run("non-interactive untrusted is blocked", func(t *testing.T) { + localPath, storePath := trustFixture(t) + _, err := gate(localPath, storePath, "", false, false /*interactive*/) + if err == nil || !strings.Contains(err.Error(), "--trust") { + t.Errorf("expected a blocking error mentioning --trust, got %v", err) + } + }) + + t.Run("interactive yes trusts and persists", func(t *testing.T) { + localPath, storePath := trustFixture(t) + out, err := gate(localPath, storePath, "y\n", false, true) + if err != nil { + t.Fatalf("expected trust granted, got %v", err) + } + if !strings.Contains(out, "Trust it?") { + t.Errorf("expected a prompt, got %q", out) + } + if _, err := gate(localPath, storePath, "", false, false); err != nil { + t.Errorf("config should be trusted after yes, got %v", err) + } + }) + + t.Run("interactive no aborts", func(t *testing.T) { + localPath, storePath := trustFixture(t) + for _, answer := range []string{"n\n", "\n", "nope\n"} { + _, err := gate(localPath, storePath, answer, false, true) + if err == nil || !strings.Contains(err.Error(), "aborting") { + t.Errorf("answer %q: expected abort error, got %v", answer, err) + } + } + }) + + t.Run("editing a trusted config re-prompts", func(t *testing.T) { + localPath, storePath := trustFixture(t) + if _, err := gate(localPath, storePath, "", true, false); err != nil { + t.Fatalf("initial trust: %v", err) + } + + // Modify the config: trust must be revoked (Changed), so a + // non-interactive run is blocked again. + if err := os.WriteFile(localPath, []byte("commands:\n build:\n cmd: echo PWNED\n"), 0644); err != nil { + t.Fatal(err) + } + if _, err := gate(localPath, storePath, "", false, false); err == nil { + t.Error("expected a changed config to be blocked until re-trusted") + } + + // Re-prompt should mention that it changed. + out, err := gate(localPath, storePath, "y\n", false, true) + if err != nil { + t.Fatalf("re-trust: %v", err) + } + if !strings.Contains(out, "changed") { + t.Errorf("expected 'changed' notice on re-prompt, got %q", out) + } + }) +} + +func TestRootCmdTrustFlag(t *testing.T) { + oldArgs := os.Args + defer func() { os.Args = oldArgs }() + os.Args = []string{"trustflaggo"} + os.Chdir(t.TempDir()) + + root := RootCmd() + if flag := root.PersistentFlags().Lookup("trust"); flag == nil { + t.Fatal("expected --trust flag") + } +} + +func TestRootCmdExecuteSingleLineQuoting(t *testing.T) { + // Regression: a single-line cmd must preserve quoted whitespace instead of + // collapsing it during whitespace tokenization (strings.Fields). + out := runVerb(t, "qgo", ` +commands: + spaces: + cmd: echo "hello world" + description: "Quoted whitespace" +`, "spaces") + + if !containsLine(out, "hello world") { + t.Errorf("expected an output line %q, got:\n%s", "hello world", out) + } +} + +func TestRootCmdExecuteSingleLineShellOperators(t *testing.T) { + // Regression: shell operators in a single-line cmd must be interpreted by + // the shell, not passed as literal arguments to the first word. + t.Run("&& chains", func(t *testing.T) { + out := runVerb(t, "andgo", ` +commands: + chain: + cmd: echo first && echo second + description: "Operator chain" +`, "chain") + + if !containsLine(out, "first") || !containsLine(out, "second") { + t.Errorf("expected output lines %q and %q, got:\n%s", "first", "second", out) + } + }) + + t.Run("pipes", func(t *testing.T) { + out := runVerb(t, "pipego", ` +commands: + pipe: + cmd: echo "hello world" | tr ' ' '_' + description: "Pipeline" +`, "pipe") + + if !containsLine(out, "hello_world") { + t.Errorf("expected an output line %q, got:\n%s", "hello_world", out) + } + }) +} + func TestExpandEnv(t *testing.T) { vars := map[string]string{ "env": "prod", diff --git a/go.mod b/go.mod index 3830062..db39e51 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/spf13/viper v1.21.0 go.yaml.in/yaml/v3 v3.0.4 golang.org/x/mod v0.35.0 + golang.org/x/term v0.43.0 ) require ( @@ -24,6 +25,5 @@ require ( github.com/spf13/pflag v1.0.10 // indirect github.com/subosito/gotenv v1.6.0 // indirect golang.org/x/sys v0.44.0 // indirect - golang.org/x/term v0.43.0 // indirect golang.org/x/text v0.28.0 // indirect ) diff --git a/go.sum b/go.sum index 8eac48f..d4143df 100644 --- a/go.sum +++ b/go.sum @@ -52,8 +52,6 @@ go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/mod v0.35.0 h1:Ww1D637e6Pg+Zb2KrWfHQUnH2dQRLBQyAtpr/haaJeM= golang.org/x/mod v0.35.0/go.mod h1:+GwiRhIInF8wPm+4AoT6L0FA1QWAad3OMdTRx4tFYlU= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.29.0 h1:TPYlXGxvx1MGTn2GiZDhnjPA9wZzZeGKHHmKhHYvgaU= -golang.org/x/sys v0.29.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.44.0 h1:ildZl3J4uzeKP07r2F++Op7E9B29JRUy+a27EibtBTQ= golang.org/x/sys v0.44.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.43.0 h1:S4RLU2sB31O/NCl+zFN9Aru9A/Cq2aqKpTZJ6B+DwT4= diff --git a/internal/args/args.go b/internal/args/args.go index c395c76..c159be8 100644 --- a/internal/args/args.go +++ b/internal/args/args.go @@ -4,6 +4,7 @@ import ( "fmt" "path/filepath" "regexp" + "slices" "github.com/PyratLabs/ugo/internal/config" ) @@ -33,19 +34,15 @@ func Validate(arg config.Argument, value string) error { } func validateExclude(name string, exclude []string, actual string) error { - for _, v := range exclude { - if v == actual { - return fmt.Errorf("argument '%s': value %q is not allowed", name, actual) - } + if slices.Contains(exclude, actual) { + return fmt.Errorf("argument '%s': value %q is not allowed", name, actual) } return nil } func validateEnum(name string, values []string, actual string) error { - for _, v := range values { - if v == actual { - return nil - } + if slices.Contains(values, actual) { + return nil } return fmt.Errorf("argument '%s': value %q not in allowed values: %s", name, actual, values) } @@ -67,7 +64,11 @@ func validateMatch(name string, pattern string, actual string) error { return fmt.Errorf("argument '%s': no file matching pattern %q found for value %q", name, pattern, actual) } - pattern = "^" + pattern + "$" + // Wrap in a non-capturing group before anchoring so the ^ and $ bind the + // whole pattern. Without the group, a top-level alternation like "dev|prod" + // would anchor as "^dev|prod$" = "(^dev)|(prod$)", letting a value such as + // "dev; rm -rf ~" satisfy the "^dev" branch and bypass validation. + pattern = "^(?:" + pattern + ")$" re, err := regexp.Compile(pattern) if err != nil { return fmt.Errorf("argument '%s': invalid regex pattern %q: %w", name, pattern, err) diff --git a/internal/args/args_test.go b/internal/args/args_test.go index 07df682..b3de8ec 100644 --- a/internal/args/args_test.go +++ b/internal/args/args_test.go @@ -338,6 +338,34 @@ func TestRegexFullMatch(t *testing.T) { } } +func TestRegexAlternationIsFullyAnchored(t *testing.T) { + // A top-level alternation must anchor as a whole, i.e. "^(?:dev|prod)$", + // not "^dev|prod$" (= "^dev" OR "prod$"). Otherwise a value like + // "dev; rm -rf ~" satisfies the "^dev" branch and slips past validation + // straight into the shell. + arg := config.Argument{ + Name: "env", + Match: "dev|prod", + } + + for _, valid := range []string{"dev", "prod"} { + if err := Validate(arg, valid); err != nil { + t.Errorf("Validate(%q) = %v, want nil", valid, err) + } + } + + for _, bad := range []string{ + "dev; rm -rf ~", // prefix-matches the "dev" branch + "xprod", // suffix-matches the "prod" branch + "development", + "prod-extra", + } { + if err := Validate(arg, bad); err == nil { + t.Errorf("Validate(%q) = nil, want error (alternation must be fully anchored)", bad) + } + } +} + func TestGlobMatches(t *testing.T) { t.Run("file glob returns basenames without ext", func(t *testing.T) { dir := t.TempDir() diff --git a/internal/checker/checker.go b/internal/checker/checker.go index 712a3bb..fd08326 100644 --- a/internal/checker/checker.go +++ b/internal/checker/checker.go @@ -3,6 +3,7 @@ package checker import ( "fmt" "os/exec" + "sort" "strings" "github.com/PyratLabs/ugo/internal/config" @@ -19,7 +20,16 @@ type Issue struct { func CheckTools(tools map[string]config.Tool) []Issue { var issues []Issue - for name, tool := range tools { + // Iterate in sorted order so results (and the printed check output) are + // deterministic rather than following Go's randomized map iteration. + names := make([]string, 0, len(tools)) + for name := range tools { + names = append(names, name) + } + sort.Strings(names) + + for _, name := range names { + tool := tools[name] var errs []string if _, err := exec.LookPath(name); err != nil { diff --git a/internal/checker/checker_test.go b/internal/checker/checker_test.go index 24dcd6a..fab8274 100644 --- a/internal/checker/checker_test.go +++ b/internal/checker/checker_test.go @@ -3,6 +3,7 @@ package checker import ( "os" "path/filepath" + "slices" "testing" "github.com/PyratLabs/ugo/internal/config" @@ -78,14 +79,7 @@ func TestCheckTools(t *testing.T) { if len(issues) != 1 { t.Fatalf("expected 1 issue, got %d", len(issues)) } - found := false - for _, e := range issues[0].Errors { - if e == "nonexistent-tool-xyz is not installed, download at: https://example.com/download" { - found = true - break - } - } - if !found { + if !slices.Contains(issues[0].Errors, "nonexistent-tool-xyz is not installed, download at: https://example.com/download") { t.Errorf("expected download URL in error, got: %v", issues[0].Errors) } }) @@ -110,6 +104,29 @@ func TestCheckTools(t *testing.T) { t.Errorf("expected no issues, got %d", len(issues)) } }) + + t.Run("issues are returned in sorted, deterministic order", func(t *testing.T) { + // All three are missing, so each yields one issue. Map iteration in Go + // is randomized, so a stable result must come from explicit sorting. + tools := map[string]config.Tool{ + "zzz-missing-tool": {}, + "aaa-missing-tool": {}, + "mmm-missing-tool": {}, + } + want := []string{"aaa-missing-tool", "mmm-missing-tool", "zzz-missing-tool"} + + for range 20 { + issues := CheckTools(tools) + if len(issues) != len(want) { + t.Fatalf("expected %d issues, got %d", len(want), len(issues)) + } + for j, w := range want { + if issues[j].Tool != w { + t.Errorf("issues[%d].Tool = %q, want %q", j, issues[j].Tool, w) + } + } + } + }) } func TestHasErrors(t *testing.T) { diff --git a/internal/config/config.go b/internal/config/config.go index dd734b9..d0c7a1b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -2,6 +2,7 @@ package config import ( "fmt" + "maps" "os" "path/filepath" "strings" @@ -126,7 +127,7 @@ func rereadEnvMaps(v *viper.Viper, cfg *Config) error { return nil // Ignore read errors, env will be empty } - var raw map[string]interface{} + var raw map[string]any if err := yaml.Unmarshal(data, &raw); err != nil { return nil // Ignore parse errors } @@ -136,13 +137,13 @@ func rereadEnvMaps(v *viper.Viper, cfg *Config) error { return nil } - commands, ok := commandsRaw.(map[string]interface{}) + commands, ok := commandsRaw.(map[string]any) if !ok { return nil } for name, cmdRaw := range commands { - cmdMap, ok := cmdRaw.(map[string]interface{}) + cmdMap, ok := cmdRaw.(map[string]any) if !ok { continue } @@ -152,7 +153,7 @@ func rereadEnvMaps(v *viper.Viper, cfg *Config) error { continue } - envMap, ok := envRaw.(map[string]interface{}) + envMap, ok := envRaw.(map[string]any) if !ok { continue } @@ -179,20 +180,16 @@ func mergeConfigs(global, local *Config) *Config { Tools: make(map[string]Tool), } - for name, cmd := range global.Commands { - merged.Commands[name] = cmd - } - - for name, cmd := range local.Commands { - merged.Commands[name] = cmd - } - - for name, tool := range global.Tools { - merged.Tools[name] = tool - } + // Local entries override global ones with the same key. + maps.Copy(merged.Commands, global.Commands) + maps.Copy(merged.Commands, local.Commands) + maps.Copy(merged.Tools, global.Tools) + maps.Copy(merged.Tools, local.Tools) - for name, tool := range local.Tools { - merged.Tools[name] = tool + // Local shell_options overrides global; otherwise inherit global. + merged.ShellOptions = global.ShellOptions + if local.ShellOptions != "" { + merged.ShellOptions = local.ShellOptions } return merged diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 9cf2f46..e07601e 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -63,6 +63,27 @@ tools: } }) + t.Run("parses shell_options", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + if err := os.WriteFile(path, []byte(` +shell_options: "set -euo pipefail" +commands: + plan: + cmd: echo plan +`), 0644); err != nil { + t.Fatal(err) + } + + cfg, err := loadConfigFile(path) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.ShellOptions != "set -euo pipefail" { + t.Errorf("ShellOptions = %q, want %q", cfg.ShellOptions, "set -euo pipefail") + } + }) + t.Run("invalid yaml", func(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "config.yaml") @@ -145,6 +166,32 @@ func TestMergeConfigs(t *testing.T) { t.Error("expected tool 'make' from local") } }) + + t.Run("shell_options is preserved through merge", func(t *testing.T) { + tests := []struct { + name string + global string + local string + wantShellOpts string + }{ + {"local only", "", "set -euo pipefail", "set -euo pipefail"}, + {"global only", "set -e", "", "set -e"}, + {"local overrides global", "set -e", "set -euo pipefail", "set -euo pipefail"}, + {"neither set", "", "", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + global := &Config{ShellOptions: tt.global} + local := &Config{ShellOptions: tt.local} + + merged := mergeConfigs(global, local) + if merged.ShellOptions != tt.wantShellOpts { + t.Errorf("merged.ShellOptions = %q, want %q", merged.ShellOptions, tt.wantShellOpts) + } + }) + } + }) } func TestConfigPaths(t *testing.T) { diff --git a/internal/trust/trust.go b/internal/trust/trust.go new file mode 100644 index 0000000..f53c9c1 --- /dev/null +++ b/internal/trust/trust.go @@ -0,0 +1,105 @@ +// Package trust implements a direnv-style trust store. Local configuration is +// loaded from the working directory, so before uGo executes anything defined in +// it the config must be trusted. Trust is content-addressed: a config is keyed +// by its absolute path and the SHA-256 of its contents, so editing a trusted +// config revokes trust until it is granted again. +package trust + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "os" + "path/filepath" +) + +// Status describes the trust state of a config file. +type Status int + +const ( + // Unknown means the config path has never been trusted. + Unknown Status = iota + // Changed means the path was trusted before but its contents have changed. + Changed + // Trusted means the path is trusted and its contents match. + Trusted +) + +// Store is the on-disk trust database, mapping an absolute config path to the +// hex SHA-256 of the contents that were trusted. +type Store struct { + path string + entries map[string]string +} + +// Load reads the trust store at path. A missing or empty file yields an empty +// store rather than an error. +func Load(path string) (*Store, error) { + s := &Store{path: path, entries: map[string]string{}} + + data, err := os.ReadFile(path) + if os.IsNotExist(err) { + return s, nil + } + if err != nil { + return nil, err + } + if len(data) == 0 { + return s, nil + } + if err := json.Unmarshal(data, &s.entries); err != nil { + return nil, err + } + return s, nil +} + +// Status reports whether configPath with the given content hash is trusted. +func (s *Store) Status(configPath, hash string) Status { + stored, ok := s.entries[configPath] + switch { + case !ok: + return Unknown + case stored == hash: + return Trusted + default: + return Changed + } +} + +// Trust records configPath+hash as trusted and persists the store. +func (s *Store) Trust(configPath, hash string) error { + s.entries[configPath] = hash + return s.save() +} + +func (s *Store) save() error { + if err := os.MkdirAll(filepath.Dir(s.path), 0o755); err != nil { + return err + } + data, err := json.MarshalIndent(s.entries, "", " ") + if err != nil { + return err + } + // 0600: this is per-user security state, not shared config. + return os.WriteFile(s.path, data, 0o600) +} + +// HashFile returns the hex-encoded SHA-256 of the file at path. +func HashFile(path string) (string, error) { + data, err := os.ReadFile(path) + if err != nil { + return "", err + } + sum := sha256.Sum256(data) + return hex.EncodeToString(sum[:]), nil +} + +// DefaultStorePath returns the trust store location for a binary: +// ~/.config//trust.json, alongside the global config. +func DefaultStorePath(binaryName string) (string, error) { + home, err := os.UserHomeDir() + if err != nil { + return "", err + } + return filepath.Join(home, ".config", binaryName, "trust.json"), nil +} diff --git a/internal/trust/trust_test.go b/internal/trust/trust_test.go new file mode 100644 index 0000000..0096ad8 --- /dev/null +++ b/internal/trust/trust_test.go @@ -0,0 +1,129 @@ +package trust + +import ( + "os" + "path/filepath" + "testing" +) + +func writeFile(t *testing.T, path, content string) { + t.Helper() + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatal(err) + } +} + +func TestHashFile(t *testing.T) { + dir := t.TempDir() + a := filepath.Join(dir, "a.yaml") + b := filepath.Join(dir, "b.yaml") + writeFile(t, a, "commands: {}\n") + writeFile(t, b, "commands: {}\n") + + ha, err := HashFile(a) + if err != nil { + t.Fatalf("HashFile(a): %v", err) + } + if ha == "" { + t.Fatal("expected a non-empty hash") + } + + hb, _ := HashFile(b) + if ha != hb { + t.Errorf("identical content should hash equal: %q vs %q", ha, hb) + } + + writeFile(t, b, "commands: {}\n# changed\n") + hb2, _ := HashFile(b) + if hb2 == ha { + t.Error("changed content should produce a different hash") + } + + if _, err := HashFile(filepath.Join(dir, "missing")); err == nil { + t.Error("expected error hashing a missing file") + } +} + +func TestStoreStatus(t *testing.T) { + store := filepath.Join(t.TempDir(), "trust.json") + s, err := Load(store) + if err != nil { + t.Fatalf("Load: %v", err) + } + + const path = "/projects/app/ugo.yaml" + + if got := s.Status(path, "hash1"); got != Unknown { + t.Errorf("Status(new) = %v, want Unknown", got) + } + + if err := s.Trust(path, "hash1"); err != nil { + t.Fatalf("Trust: %v", err) + } + + if got := s.Status(path, "hash1"); got != Trusted { + t.Errorf("Status(same hash) = %v, want Trusted", got) + } + if got := s.Status(path, "hash2"); got != Changed { + t.Errorf("Status(different hash) = %v, want Changed", got) + } + if got := s.Status("/other/ugo.yaml", "hash1"); got != Unknown { + t.Errorf("Status(other path) = %v, want Unknown", got) + } +} + +func TestStorePersistence(t *testing.T) { + store := filepath.Join(t.TempDir(), "nested", "dir", "trust.json") + + s, err := Load(store) + if err != nil { + t.Fatalf("Load (missing): %v", err) + } + if err := s.Trust("/projects/app/ugo.yaml", "abc123"); err != nil { + t.Fatalf("Trust: %v", err) + } + + // Trust must create parent directories and persist to disk. + if _, err := os.Stat(store); err != nil { + t.Fatalf("trust store not written: %v", err) + } + + // A fresh load must see the recorded entry. + reloaded, err := Load(store) + if err != nil { + t.Fatalf("Load (existing): %v", err) + } + if got := reloaded.Status("/projects/app/ugo.yaml", "abc123"); got != Trusted { + t.Errorf("after reload Status = %v, want Trusted", got) + } +} + +func TestLoadMissingFile(t *testing.T) { + s, err := Load(filepath.Join(t.TempDir(), "does-not-exist.json")) + if err != nil { + t.Fatalf("Load of missing file should not error: %v", err) + } + if got := s.Status("/x", "h"); got != Unknown { + t.Errorf("empty store Status = %v, want Unknown", got) + } +} + +func TestLoadCorruptFile(t *testing.T) { + store := filepath.Join(t.TempDir(), "trust.json") + writeFile(t, store, "{ not valid json") + if _, err := Load(store); err == nil { + t.Error("expected error loading corrupt trust store") + } +} + +func TestDefaultStorePath(t *testing.T) { + t.Setenv("HOME", "/home/tester") + got, err := DefaultStorePath("myproj") + if err != nil { + t.Fatalf("DefaultStorePath: %v", err) + } + want := filepath.Join("/home/tester", ".config", "myproj", "trust.json") + if got != want { + t.Errorf("DefaultStorePath = %q, want %q", got, want) + } +} diff --git a/internal/version/version.go b/internal/version/version.go index 9fb2f62..c8cd2dd 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -1,16 +1,29 @@ package version import ( + "context" "fmt" "os/exec" "regexp" "strings" + "time" "golang.org/x/mod/semver" ) var versionRe = regexp.MustCompile(`v?(\d+\.\d+\.\d+)`) +// versionCmdTimeout bounds how long a tool's version command may run. Version +// checks are pre-flight — they run before every verb — so a hung version +// command must not hang uGo. It is a var (not a const) so tests can shorten it. +var versionCmdTimeout = 10 * time.Second + +// versionCmdWaitDelay bounds the extra wait, after the timeout fires, for any +// descendant processes that still hold the command's output pipes open. Without +// it a version command that backgrounds a child would block I/O indefinitely +// even after its own process is killed. +var versionCmdWaitDelay = 1 * time.Second + // ExtractVersion finds the first semver-compatible version string in output func ExtractVersion(output string) string { match := versionRe.FindStringSubmatch(output) @@ -33,7 +46,18 @@ func Check(name string, versionCmd string, minVersion string, maxVersion string) return "", fmt.Errorf("empty version command for %s", name) } - out, err := exec.Command(parts[0], parts[1:]...).Output() + ctx, cancel := context.WithTimeout(context.Background(), versionCmdTimeout) + defer cancel() + + cmd := exec.CommandContext(ctx, parts[0], parts[1:]...) + cmd.WaitDelay = versionCmdWaitDelay + + // CombinedOutput captures stdout and stderr so tools that report their + // version on stderr (e.g. "java -version") are still parsed. + out, err := cmd.CombinedOutput() + if ctx.Err() == context.DeadlineExceeded { + return "", fmt.Errorf("%s: version command timed out after %s", name, versionCmdTimeout) + } if err != nil { return "", fmt.Errorf("%s: failed to run version command: %w", name, err) } diff --git a/internal/version/version_test.go b/internal/version/version_test.go index c1e8bf9..eb4b5fd 100644 --- a/internal/version/version_test.go +++ b/internal/version/version_test.go @@ -5,6 +5,7 @@ import ( "path/filepath" "strings" "testing" + "time" ) func TestExtractVersion(t *testing.T) { @@ -123,3 +124,41 @@ func TestCheck(t *testing.T) { }) } } + +func TestCheckReadsStderr(t *testing.T) { + // Many tools (e.g. "java -version") print their version to stderr. + tmp := t.TempDir() + scriptPath := filepath.Join(tmp, "stderr-version") + if err := os.WriteFile(scriptPath, []byte("#!/bin/sh\necho \"stderr-tool v2.3.4\" >&2\n"), 0755); err != nil { + t.Fatal(err) + } + + got, err := Check("stderr-tool", scriptPath, "2.0.0", "3.0.0") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "v2.3.4" { + t.Errorf("Check() version = %q, want %q", got, "v2.3.4") + } +} + +func TestCheckTimeout(t *testing.T) { + tmp := t.TempDir() + scriptPath := filepath.Join(tmp, "slow-version") + if err := os.WriteFile(scriptPath, []byte("#!/bin/sh\nsleep 5\necho v1.0.0\n"), 0755); err != nil { + t.Fatal(err) + } + + orig := versionCmdTimeout + versionCmdTimeout = 100 * time.Millisecond + defer func() { versionCmdTimeout = orig }() + + start := time.Now() + _, err := Check("slow-tool", scriptPath, "", "") + if elapsed := time.Since(start); elapsed > 3*time.Second { + t.Errorf("Check() took %s, expected it to abort well before the script's 5s sleep", elapsed) + } + if err == nil || !strings.Contains(err.Error(), "timed out") { + t.Errorf("Check() error = %v, want a timeout error", err) + } +}