AI-driven round of code fixes

.github/workflows/ci.yml

@@ -8,7 +8,7 @@ on:
     branches: [master]
 
 permissions:
-  contents: write
+  contents: read
 
 jobs:
   test:
@@ -62,12 +62,14 @@ jobs:
     if: startsWith(github.ref, 'refs/tags/v')
     needs: build
     runs-on: ubuntu-latest
+    permissions:
+      contents: write
     steps:
       - uses: actions/download-artifact@v8
         with:
           path: dist
           merge-multiple: true
-      - uses: softprops/action-gh-release@v3
+      - uses: softprops/action-gh-release@3d0d9888cb7fd7b750713d6e236d1fcb99157228 # v3.0.2
         with:
           files: dist/*
           generate_release_notes: true

Makefile

@@ -30,7 +30,7 @@ $(TARGETS): go.mod
 	make $(platform)
 
 test: deps
-	$(GO) test
+	$(GO) test -race ./...
 
 fmt:
 	$(GO) fmt

README.md

@@ -91,6 +91,16 @@ GLOBAL OPTIONS:
    --version, -v                                              print the version
 ```
 
+## Exit codes
+
+The wrapped command's exit code is passed through unchanged. ``env-aws-params``
+itself uses:
+
+- ``125`` — invalid usage or a Parameter Store error
+- ``126`` — the command was found but could not be started
+- ``127`` — the command was not found
+- ``128+N`` — the command was killed by signal ``N`` (e.g. ``143`` for ``SIGTERM``)
+
 ## Building from source
 
 This project uses [Go modules](https://go.dev/blog/using-go-modules) and requires Go 1.26+.
@@ -99,9 +109,3 @@ This project uses [Go modules](https://go.dev/blog/using-go-modules) and require
 go mod download
 go build
 ```
-
-Or build the Docker image locally:
-
-```bash
-docker build -t env-aws-params .
-```

data.go

@@ -1,10 +1,12 @@
 package main
 
 import (
-	"fmt"
+	"maps"
 	"regexp"
-	"sort"
+	"slices"
 	"strings"
+
+	log "github.com/sirupsen/logrus"
 )
 
 var InvalidPattern = regexp.MustCompile(`[^a-zA-Z0-9_]`)
@@ -25,20 +27,31 @@ func MergeEnvVars(ssmVars []string, environ []string) []string {
 }
 
 func BuildEnvVars(parameters map[string]string, sanitize bool, strip bool, upcase bool) []string {
-	var vars []string
-
-	for k, v := range parameters {
+	// Transform in sorted parameter order so collisions resolve deterministically.
+	values := make(map[string]string, len(parameters))
+	sources := make(map[string]string, len(parameters))
+	for _, name := range slices.Sorted(maps.Keys(parameters)) {
+		key := name
 		if sanitize {
-			k = InvalidPattern.ReplaceAllString(k, "_")
+			key = InvalidPattern.ReplaceAllString(key, "_")
 		}
 		if strip {
-			k = InvalidPattern.ReplaceAllString(k, "")
+			key = InvalidPattern.ReplaceAllString(key, "")
 		}
 		if upcase {
-			k = strings.ToUpper(k)
+			key = strings.ToUpper(key)
+		}
+		if prev, ok := sources[key]; ok {
+			log.Warnf("Parameters %q and %q both map to %s; keeping the value of %q", prev, name, key, name)
 		}
-		vars = append(vars, fmt.Sprintf("%s=%s", k, v))
+		sources[key] = name
+		values[key] = parameters[name]
+	}
+
+	vars := make([]string, 0, len(values))
+	for key, value := range values {
+		vars = append(vars, key+"="+value)
 	}
-	sort.Strings(vars)
+	slices.Sort(vars)
 	return vars
 }

data_test.go

@@ -110,6 +110,16 @@ func TestBuildEnvVarsUpperSanitize(t *testing.T) {
 	AssertEqual(t, envVars, expectation)
 }
 
+func TestBuildEnvVarsCollisionKeepsLastSortedParameter(t *testing.T) {
+	params := map[string]string{
+		"db-host": "from-dash",
+		"db_host": "from-underscore",
+	}
+
+	envVars := BuildEnvVars(params, true, false, true)
+	AssertEqual(t, envVars, []string{"DB_HOST=from-underscore"})
+}
+
 func TestBuildEnvVarsUpperStrip(t *testing.T) {
 	var params map[string]string
 

go.mod

@@ -2,6 +2,8 @@ module env-aws-params
 
 go 1.26
 
+toolchain go1.26.5
+
 require (
 	github.com/aws/aws-sdk-go-v2 v1.41.7
 	github.com/aws/aws-sdk-go-v2/config v1.32.17

main.go

@@ -8,22 +8,27 @@ import (
 	"os"
 	"os/exec"
 	"strings"
+	"syscall"
 
 	log "github.com/sirupsen/logrus"
 	"github.com/urfave/cli/v3"
 	"golang.org/x/sync/errgroup"
 )
 
-var VersionString string
+var VersionString = "dev"
 
 func main() {
+	// Stop flag parsing at the wrapped command, so its own flags
+	// (e.g. sh -c) are passed through instead of rejected.
+	stopOnFirstArg := 1
 	cmd := &cli.Command{
-		Name:      "env-aws-params",
-		Usage:     "Application entry-point that injects SSM Parameter Store values as Environment Variables",
-		UsageText: "env-aws-params [global options] -p prefix command [command arguments]",
-		Version:   VersionString,
-		Flags:     cliFlags(),
-		Action:    action,
+		Name:         "env-aws-params",
+		Usage:        "Application entry-point that injects SSM Parameter Store values as Environment Variables",
+		UsageText:    "env-aws-params [global options] -p prefix command [command arguments]",
+		Version:      VersionString,
+		Flags:        cliFlags(),
+		Action:       action,
+		StopOnNthArg: &stopOnFirstArg,
 	}
 	if err := cmd.Run(context.Background(), os.Args); err != nil {
 		log.Fatal(err)
@@ -38,16 +43,15 @@ func action(ctx context.Context, cmd *cli.Command) error {
 		log.SetOutput(io.Discard)
 	}
 
-	code, err := validateArgs(cmd.NArg(), cmd.Bool("sanitize"), cmd.Bool("strip"))
-	if code > 0 {
-		return cli.Exit(errorPrefix(err), code)
+	if err := validateArgs(cmd.NArg(), cmd.Bool("sanitize"), cmd.Bool("strip")); err != nil {
+		return cli.Exit(errorPrefix(err), 125)
 	}
 
 	var envVars []string
 	if len(cmd.StringSlice("prefix")) > 0 {
 		params, err := getParameters(ctx, cmd)
 		if err != nil {
-			return cli.Exit(errorPrefix(err), -1)
+			return cli.Exit(errorPrefix(err), 125)
 		}
 
 		envVars = BuildEnvVars(
@@ -76,9 +80,16 @@ func action(ctx context.Context, cmd *cli.Command) error {
 	if err := RunCommand(args.First(), args.Tail(), envVars); err != nil {
 		var exitErr *exec.ExitError
 		if errors.As(err, &exitErr) {
-			return cli.Exit(errorPrefix(err), exitErr.ExitCode())
+			code := exitErr.ExitCode()
+			if status, ok := exitErr.Sys().(syscall.WaitStatus); ok && status.Signaled() {
+				code = 128 + int(status.Signal())
+			}
+			return cli.Exit(errorPrefix(err), code)
+		}
+		if errors.Is(err, exec.ErrNotFound) || errors.Is(err, os.ErrNotExist) {
+			return cli.Exit(errorPrefix(err), 127)
 		}
-		return cli.Exit(errorPrefix(err), 128)
+		return cli.Exit(errorPrefix(err), 126)
 	}
 	return nil
 }
@@ -170,14 +181,14 @@ func getParameters(ctx context.Context, cmd *cli.Command) (map[string]string, er
 	return values, nil
 }
 
-func validateArgs(nargs int, sanitize, strip bool) (int, error) {
+func validateArgs(nargs int, sanitize, strip bool) error {
 	if nargs == 0 {
-		return 1, errors.New("command not specified")
+		return errors.New("command not specified")
 	}
 
 	if sanitize && strip {
-		return 2, errors.New("--sanitize and --strip are mutually exclusive behaviors")
+		return errors.New("--sanitize and --strip are mutually exclusive behaviors")
 	}
 
-	return 0, nil
+	return nil
 }

main_test.go

@@ -6,41 +6,25 @@ import (
 )
 
 func TestValidateArgsNoPrefixIsValid(t *testing.T) {
-	code, err := validateArgs(1, false, false)
-	if code != 0 {
-		t.Fatalf("expected code to be 0, got %v", code)
-	}
-	if err != nil {
+	if err := validateArgs(1, false, false); err != nil {
 		t.Fatalf("expected err to be nil, got %v", err)
 	}
 }
 
 func TestValidateArgsMissingCommand(t *testing.T) {
-	code, err := validateArgs(0, false, false)
-	if code != 1 {
-		t.Fatalf("expected code to be 1, got %v", code)
-	}
-	if err == nil {
+	if err := validateArgs(0, false, false); err == nil {
 		t.Fatalf("expected err to be set, got nil")
 	}
 }
 
 func TestValidateArgsStripAndSanitize(t *testing.T) {
-	code, err := validateArgs(1, true, true)
-	if code != 2 {
-		t.Fatalf("expected code to be 2, got %v", code)
-	}
-	if err == nil {
+	if err := validateArgs(1, true, true); err == nil {
 		t.Fatalf("expected err to be set, got nil")
 	}
 }
 
 func TestValidateArgsValid(t *testing.T) {
-	code, err := validateArgs(1, false, true)
-	if code != 0 {
-		t.Fatalf("expected code to be 0, got %v", code)
-	}
-	if err != nil {
+	if err := validateArgs(1, false, true); err != nil {
 		t.Fatalf("expected err to be nil, got %v", err)
 	}
 }

runner.go

@@ -23,26 +23,30 @@ func RunCommand(command string, args []string, envVars []string) error {
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
 
+	// Signals arriving before Start queue up in the channel and are forwarded
+	// once the child is running.
+	sigc := make(chan os.Signal, 32)
+	signal.Notify(sigc)
+
 	if err := cmd.Start(); err != nil {
 		return err
 	}
 
 	log.Infof("PID %v running %s %s", cmd.Process.Pid, cmd.Path, strings.Join(args, " "))
 
-	sigc := make(chan os.Signal, 1)
-	signal.Notify(sigc,
-		syscall.SIGHUP,
-		syscall.SIGINT,
-		syscall.SIGTERM,
-		syscall.SIGQUIT)
 	go func() {
-		sigv := <-sigc
-		killErr := syscall.Kill(-os.Getpid(), sigv.(syscall.Signal))
-		log.WithFields(log.Fields{
-			"err":    killErr,
-			"pid":    -cmd.Process.Pid,
-			"signal": sigv,
-		}).Info("Caught signal, sent to child")
+		for sigv := range sigc {
+			// SIGCHLD is for the wrapper itself; SIGURG is used by the Go runtime.
+			if sigv == syscall.SIGCHLD || sigv == syscall.SIGURG {
+				continue
+			}
+			sigErr := cmd.Process.Signal(sigv)
+			log.WithFields(log.Fields{
+				"err":    sigErr,
+				"pid":    cmd.Process.Pid,
+				"signal": sigv,
+			}).Debug("Forwarded signal to child")
+		}
 	}()
 
 	return cmd.Wait()