Add input validation against malformed inputs

.claude/skills/add-command/SKILL.md

@@ -19,6 +19,7 @@ Before writing any code, understand what the command should do and whether the a
 3. **Does it need configuration?** (determines whether `PreRunE: initConfigDeferCreate(nil)` is needed)
 4. **Does it need authentication?** (determines whether auth flow is involved)
 5. **Does it need any new event types?** (e.g., a new kind of progress, a new status phase — if yes, use `/add-event` for each)
+6. **What user-supplied inputs does it accept?** (args, flags, config values — determines which `internal/validate` validators apply at the boundary)
 
 **Also ask context-specific questions** that aren't in this list but would help clarify behavior — e.g., edge cases ("what happens if no emulators are running?"), flags/arguments the command should accept, expected output format, whether it should be idempotent, etc.
 
@@ -48,6 +49,7 @@ Create `cmd/$ARGUMENTS.go` with:
   - Non-interactive: call domain function with `output.NewPlainSink(os.Stdout)`
 - If the command only groups subcommands (like `config`, `setup`, `snapshot`), call `requireSubcommand(cmd)` so a bare invocation shows help (exit 0) while an unknown subcommand exits non-zero
 - No business logic — only Cobra wiring, dependency creation, and output mode selection
+- Validate user-supplied args/flags where they are first accepted, via `internal/validate` — identifiers through `validate.ResourceName`, opaque secrets through loose checks like `validate.AuthToken`, paths/URLs through their normal parsers (`filepath`, `net/url`). If no validator fits, add one to `internal/validate` (with rule-code tests); never write an inline validation regexp
 - `Short`/`Long` help text: write each paragraph as one unbroken line (blank line between paragraphs); never hard-wrap a sentence across source lines. The help template (`cmd/help.go`) word-wraps to the terminal width at render time, and `lstk docs` reads the raw text — manual breaks fight both. Indent example/output blocks; the wrapper leaves indented lines untouched.
 
 ## Step 2: Create the domain logic package
@@ -96,6 +98,7 @@ Create `test/integration/<name>_test.go` with:
 - Use `requireDocker(t)` if Docker is needed
 - Use `cleanup()` and `t.Cleanup(cleanup)` for container state
 - Use `context.WithTimeout` for all tests
+- If the command accepts identifiers or free-form values, include malformed-input cases (control characters, `../` traversal, percent-encoding, shell metacharacters) asserting a clear rejection
 
 ## Telemetry
 

.claude/skills/review-pr/SKILL.md

@@ -48,6 +48,7 @@ Go through each changed file and check for violations. Flag only actual problems
 - [ ] Domain code never reads from stdin directly
 - [ ] Interactive input uses `UserInputRequestEvent` + `ResponseCh` pattern
 - [ ] Non-TTY mode fails early with a helpful error if input would be required
+- [ ] New user-supplied inputs (args, flags, config values) are validated at the boundary via `internal/validate`; no new inline validation regexp duplicating it (identifiers → `ResourceName`; opaque secrets → loose checks like `AuthToken`; paths/URLs → their existing parsers) and malformed-input cases are tested
 
 ### TUI (if UI changes)
 

CLAUDE.md

@@ -68,6 +68,7 @@ Notes:
   - `tracing/` - OpenTelemetry setup (`LSTK_OTEL=1`)
   - `ui/` - Bubble Tea views for interactive output
   - `update/` - Self-update logic: version check via GitHub API, binary/Homebrew/npm update paths, archive extraction; also detects multiple lstk installs on PATH (`FindInstalls`/`WarnMultipleInstalls`, warned on `lstk update` and the start-path update notification)
+  - `validate/` - Reusable input validators for user-supplied CLI values (pod names, env var names, auth tokens) rejecting malformed/hostile input (control chars, path traversal, percent-encoding, shell metacharacters)
   - `version/` - Version info
   - `volume/` - `lstk volume` domain logic
 
@@ -182,6 +183,7 @@ The release job (`.github/workflows/ci.yml`) builds the npm packages with `gorel
 - Never print directly to stdout/stderr (e.g., `fmt.Fprintf(os.Stderr, …)`). For user-facing output, emit events through `output.Sink`. For internal diagnostics, use `log.Logger`. If neither is available (e.g., during logger setup), return errors to the caller and let them decide.
 - Don't deprecate commands with Cobra's `Deprecated` field: it prints the notice raw to `os.Stderr` (bypassing `output.Sink`) and silently hides the command from `--help` and generated `lstk docs`. Remove the old command outright instead; if a transition period is genuinely needed, keep the command visible and emit the deprecation notice through the sink.
 - Do not call `config.Get()` from domain/business-logic packages. Instead, extract the values you need at the command boundary (`cmd/`) and pass them as explicit function arguments. This keeps domain functions testable without requiring Viper/config initialization.
+- Validate user/agent-supplied values where they are first accepted (the command boundary, or the domain parser that owns the format) via `internal/validate` — never an inline one-off regexp. Route by value class: identifier-shaped values (pod names, resource names) → `validate.ResourceName`; opaque secrets → only loose malformed-ness checks (`validate.AuthToken` style — no charset restriction); paths and URLs → their existing parsers (`filepath`, `net/url`), not `ResourceName`. If no validator fits, add one to `internal/validate` with rule-code tests instead of forking rules locally — parallel validators for the same value class drift (the pod-name rules forked exactly this way before #293 re-unified them).
 
 # Shell Completion
 

cmd/root.go

@@ -24,6 +24,7 @@ import (
 	"github.com/localstack/lstk/internal/tracing"
 	"github.com/localstack/lstk/internal/ui"
 	"github.com/localstack/lstk/internal/update"
+	"github.com/localstack/lstk/internal/validate"
 	"github.com/localstack/lstk/internal/version"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
@@ -260,6 +261,15 @@ func Execute(ctx context.Context) error {
 			resolvedToken = token
 		}
 	}
+	// Trim surrounding whitespace: env-injected tokens (e.g. CI secrets) commonly
+	// carry a trailing newline. Then reject clearly malformed tokens before they
+	// reach the platform API, telemetry, or the container environment.
+	resolvedToken = strings.TrimSpace(resolvedToken)
+	if err := validate.AuthToken(resolvedToken); err != nil {
+		err = fmt.Errorf("invalid auth token: %w", err)
+		fmt.Fprintf(os.Stderr, "Error: %v\n", err)
+		return err
+	}
 	cfg.AuthToken = resolvedToken
 	tel.SetAuthToken(resolvedToken)
 

internal/config/config.go

@@ -9,6 +9,7 @@ import (
 	"regexp"
 	"strings"
 
+	"github.com/localstack/lstk/internal/validate"
 	"github.com/pelletier/go-toml/v2"
 	"github.com/spf13/viper"
 )
@@ -184,5 +185,24 @@ func Get() (*Config, error) {
 			return nil, fmt.Errorf("invalid container config: %w", err)
 		}
 	}
+	if err := validateNamedEnvs(cfg.Env); err != nil {
+		return nil, err
+	}
 	return &cfg, nil
 }
+
+// validateNamedEnvs rejects malformed variables defined in the top-level [env.*]
+// config sections before they are injected into a container's environment.
+func validateNamedEnvs(envs map[string]map[string]string) error {
+	for name, vars := range envs {
+		for key, value := range vars {
+			if err := validate.EnvVarName(key); err != nil {
+				return fmt.Errorf("invalid variable in [env.%s]: %w", name, err)
+			}
+			if err := validate.NoControlChars("value for "+key, value); err != nil {
+				return fmt.Errorf("invalid variable in [env.%s]: %w", name, err)
+			}
+		}
+	}
+	return nil
+}

internal/config/env_validation_test.go

@@ -0,0 +1,52 @@
+package config
+
+import "testing"
+
+func TestValidateNamedEnvs(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name    string
+		envs    map[string]map[string]string
+		wantErr bool
+	}{
+		{"nil", nil, false},
+		{"empty", map[string]map[string]string{}, false},
+		{
+			name: "valid",
+			envs: map[string]map[string]string{
+				"debug": {"ls_log": "trace", "debug": "1"},
+				"ci":    {"services": "s3,sqs"},
+			},
+			wantErr: false,
+		},
+		{
+			name:    "control char in value",
+			envs:    map[string]map[string]string{"bad": {"debug": "1\x00"}},
+			wantErr: true,
+		},
+		{
+			name:    "hyphen in key",
+			envs:    map[string]map[string]string{"bad": {"my-key": "1"}},
+			wantErr: true,
+		},
+		{
+			name:    "equals in key",
+			envs:    map[string]map[string]string{"bad": {"a=b": "1"}},
+			wantErr: true,
+		},
+		{
+			name:    "key starts with digit",
+			envs:    map[string]map[string]string{"bad": {"1var": "1"}},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			err := validateNamedEnvs(tt.envs)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("validateNamedEnvs() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}

internal/snapshot/destination.go

@@ -7,9 +7,10 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
-	"regexp"
 	"strings"
 	"time"
+
+	"github.com/localstack/lstk/internal/validate"
 )
 
 // ErrHomeNotSet is returned when a path needs "~" expansion but no home directory was provided.
@@ -24,8 +25,6 @@ var (
 	// ErrCredentialsInS3URL is returned when an s3:// ref embeds credential query
 	// params. Credentials must come from the environment or --profile, never the URL.
 	ErrCredentialsInS3URL = errors.New("do not put credentials in the s3:// URL; use AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY or --profile")
-
-	validPodName = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9_-]*$`)
 )
 
 const (
@@ -70,8 +69,8 @@ func IsS3Ref(ref string) bool {
 // ValidatePodName validates a user-supplied pod name (the identity of a snapshot
 // on a remote), using the same rules as pod: refs.
 func ValidatePodName(name string) error {
-	if !validPodName.MatchString(name) {
-		return fmt.Errorf("invalid pod name %q: use letters, digits, hyphens, and underscores only, starting with a letter or digit", name)
+	if err := validate.ResourceName("pod name", name); err != nil {
+		return fmt.Errorf("invalid pod name %q: %w", name, err)
 	}
 	return nil
 }

internal/snapshot/destination_test.go

@@ -178,6 +178,16 @@ func TestParseSource(t *testing.T) {
 			input:   "pod:-bad",
 			wantErr: "invalid pod name",
 		},
+		{
+			name:    "pod: percent encoding rejected",
+			input:   "pod:staging%2Fpod",
+			wantErr: "invalid pod name",
+		},
+		{
+			name:    "pod: shell metacharacters rejected",
+			input:   "pod:a;rm",
+			wantErr: "invalid pod name",
+		},
 
 		// --- remote schemes ---
 		{
@@ -522,6 +532,21 @@ func TestParseDestination(t *testing.T) {
 			wantKind:    snapshot.KindPod,
 			wantPodName: "ci_test-underscore",
 		},
+		{
+			name:    "pod: percent encoding rejected",
+			input:   "pod:staging%2Fpod",
+			wantErr: "invalid pod name",
+		},
+		{
+			name:    "pod: embedded query rejected",
+			input:   "pod:abc?fields=name",
+			wantErr: "invalid pod name",
+		},
+		{
+			name:    "pod: shell metacharacters rejected",
+			input:   "pod:a;rm",
+			wantErr: "invalid pod name",
+		},
 
 		// --- unknown schemes ---
 		{

internal/validate/validate.go

@@ -0,0 +1,128 @@
+// Package validate provides reusable, deterministic validators for user-supplied
+// CLI inputs. It exists to make the CLI a safe target for AI agents and scripts,
+// which can produce malformed or hostile input — control characters, path
+// traversal, percent-encoding, embedded query parameters, shell metacharacters —
+// in ways humans rarely do.
+//
+// Validators return an *Error carrying a machine-classifiable Rule so callers can
+// surface a precise, stable reason (and, in JSON output mode, a stable error
+// code) instead of a generic "invalid input" message. Error() returns the bare
+// reason so it composes cleanly when wrapped with caller context.
+package validate
+
+import (
+	"fmt"
+	"regexp"
+	"strings"
+	"unicode"
+)
+
+// Rule classifies why a value was rejected. The values are stable and intended to
+// be surfaced as machine-readable error codes.
+const (
+	RuleEmpty        = "empty"
+	RuleControlChars = "control_chars"
+	RuleEncoding     = "encoding"
+	RuleTraversal    = "traversal"
+	RuleEmbedded     = "embedded"
+	RuleMetachars    = "metachars"
+	RuleFormat       = "format"
+	RuleRange        = "range"
+)
+
+type Error struct {
+	Field string
+	Rule  string
+	Msg   string
+}
+
+func (e *Error) Error() string { return e.Msg }
+
+func newError(field, rule, msg string) *Error {
+	return &Error{Field: field, Rule: rule, Msg: msg}
+}
+
+// containsControlChars reports whether s contains any control character other
+// than tab, newline, or carriage return.
+func containsControlChars(s string) bool {
+	for _, r := range s {
+		if r == '\t' || r == '\n' || r == '\r' {
+			continue
+		}
+		if unicode.IsControl(r) {
+			return true
+		}
+	}
+	return false
+}
+
+func NoControlChars(field, value string) error {
+	if containsControlChars(value) {
+		return newError(field, RuleControlChars, "contains control characters")
+	}
+	return nil
+}
+
+var envVarKeyRegexp = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_]*$`)
+
+// EnvVarName validates an environment variable name (the key of a KEY=VALUE pair).
+func EnvVarName(name string) error {
+	if !envVarKeyRegexp.MatchString(name) {
+		return newError("env", RuleFormat, fmt.Sprintf("env key %q contains invalid characters", name))
+	}
+	return nil
+}
+
+var resourceNameRegexp = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9_-]*$`)
+
+// shellMetaChars are characters that enable command injection if an identifier is
+// ever interpolated into a shell. The slash, question mark, and hash are handled
+// separately as embedded path/query characters and are not repeated here.
+const shellMetaChars = ";&|$\x60<>(){}[]!*'~\\\""
+
+// ResourceName validates an opaque resource identifier such as a Cloud Pod name.
+// It runs ordered deny-checks so the most specific reason wins, then a strict
+// allow-list. The deny-checks exist to give precise, machine-classifiable
+// feedback; the allow-list alone would reject every invalid value.
+func ResourceName(field, value string) error {
+	switch {
+	case value == "":
+		return newError(field, RuleEmpty, "must not be empty")
+	case containsControlChars(value):
+		return newError(field, RuleControlChars, "contains control characters")
+	case strings.Contains(value, "%"):
+		return newError(field, RuleEncoding, "contains percent-encoding (pass the decoded value)")
+	case strings.Contains(value, ".."):
+		return newError(field, RuleTraversal, "contains a path traversal sequence (..)")
+	case strings.ContainsAny(value, "/?#"):
+		return newError(field, RuleEmbedded, "contains path or query characters (/, ?, #)")
+	case strings.ContainsAny(value, shellMetaChars):
+		return newError(field, RuleMetachars, "contains shell metacharacters")
+	case !resourceNameRegexp.MatchString(value):
+		return newError(field, RuleFormat, "use letters, digits, hyphens, and underscores only, starting with a letter or digit")
+	}
+	return nil
+}
+
+// AuthToken validates a LocalStack auth token. The character set is intentionally
+// not restricted — tokens are opaque — so only clearly malformed values are
+// rejected: control characters, embedded whitespace, or an implausible length. An
+// empty token is allowed (it means none is set). Callers should TrimSpace first,
+// since environment injection (e.g. CI secrets) commonly appends a trailing newline.
+func AuthToken(value string) error {
+	if value == "" {
+		return nil
+	}
+	for _, r := range value {
+		if unicode.IsControl(r) {
+			return newError("auth token", RuleControlChars, "contains control characters")
+		}
+		if unicode.IsSpace(r) {
+			return newError("auth token", RuleFormat, "contains whitespace")
+		}
+	}
+	if len(value) > 1024 {
+		return newError("auth token", RuleRange, "is implausibly long (over 1024 characters)")
+	}
+	return nil
+}