fix(config): harden manifest input validation

internal/config/validate_action_folder_test.go

@@ -0,0 +1,18 @@
+package config
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// TestValidateActionFolder_RejectsTraversal guards the action_folder sink:
+// the generator joins it directly into a filesystem path
+// (.github/actions/<folder>/action.yaml), so it must reject a traversal or
+// nested-path shape and accept only a plain folder name.
+func TestValidateActionFolder_RejectsTraversal(t *testing.T) {
+	t.Parallel()
+	require.NotEmpty(t, validateActionFolder("../../etc/foo"))
+	require.NotEmpty(t, validateActionFolder("a/b"))
+	require.Empty(t, validateActionFolder("manage-release"))
+}

internal/config/validate_action_pins_test.go

@@ -0,0 +1,31 @@
+package config
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// TestValidateActionPins_RejectsNewline guards the action_pins override sink:
+// actionRef splices the value raw into a generated workflow, so a value
+// carrying a newline (or otherwise breaking out of the emitted YAML scalar)
+// must be rejected rather than reaching the generator.
+func TestValidateActionPins_RejectsNewline(t *testing.T) {
+	t.Parallel()
+	cfg := &TrunkConfig{ActionPins: map[string]string{
+		"actions/checkout": "v5\n      run: curl evil",
+	}}
+	errs := validateActionPins(cfg)
+	require.NotEmpty(t, errs)
+}
+
+// TestValidateActionPins_AllowsShaWithComment confirms the accepted shape: a
+// full-length sha plus its trailing "# <version>" comment, the form a
+// reconcile adoption writes, still validates clean.
+func TestValidateActionPins_AllowsShaWithComment(t *testing.T) {
+	t.Parallel()
+	cfg := &TrunkConfig{ActionPins: map[string]string{
+		"actions/checkout": "abc123def4567890abc123def4567890abc12345 # v5.1.0",
+	}}
+	require.Empty(t, validateActionPins(cfg))
+}

internal/config/validate_local_callback_test.go

@@ -82,3 +82,43 @@ func TestValidateLocalCallbackWorkflowPath(t *testing.T) {
 		})
 	}
 }
+
+// TestValidateLocalCallbackWorkflowPath_RejectsTraversal guards the callback
+// workflow path sink: the .github/workflows/ prefix check alone does not
+// reject a value that escapes the directory via a ".." traversal segment
+// after that prefix.
+func TestValidateLocalCallbackWorkflowPath_RejectsTraversal(t *testing.T) {
+	t.Parallel()
+	errs := validateLocalCallbackWorkflowPath("x", ".github/workflows/../../../secret")
+	if len(errs) == 0 {
+		t.Fatal("expected an error for a traversal path, got none")
+	}
+}
+
+// TestValidateLocalCallbackWorkflowPath_RejectsUnsafeCharacters guards both
+// accepted branches (bare filename and the .github/workflows/... path)
+// against a value carrying a newline or other character that could break out
+// of the emitted YAML scalar when the path is spliced raw into a generated
+// workflow's uses: line.
+func TestValidateLocalCallbackWorkflowPath_RejectsUnsafeCharacters(t *testing.T) {
+	t.Parallel()
+	if errs := validateLocalCallbackWorkflowPath("promote_callback", "x\n      run: curl evil"); len(errs) == 0 {
+		t.Fatal("expected an error for a bare filename carrying a newline, got none")
+	}
+	if errs := validateLocalCallbackWorkflowPath("promote_callback", ".github/workflows/x.yaml\n      run: curl evil"); len(errs) == 0 {
+		t.Fatal("expected an error for a .github/workflows/... path carrying a newline, got none")
+	}
+}
+
+// TestValidateLocalCallbackWorkflowPath_RejectsUnsafeCrossRepoRef guards the
+// remaining unguarded branch: a cross-repo "@"-containing ref is spliced raw
+// into a generated workflow's uses: line just like the local forms, so it
+// must reject a value carrying a newline or other character that could break
+// out of the emitted YAML scalar too.
+func TestValidateLocalCallbackWorkflowPath_RejectsUnsafeCrossRepoRef(t *testing.T) {
+	t.Parallel()
+	errs := validateLocalCallbackWorkflowPath("promote_callback", "owner/repo/.github/workflows/w.yaml@main\n      run: curl evil")
+	if len(errs) == 0 {
+		t.Fatal("expected an error for a cross-repo ref carrying a newline, got none")
+	}
+}

internal/config/validate_v1.go

@@ -6,6 +6,7 @@ import (
 	"sort"
 	"strings"
 	"time"
+	"unicode"
 )
 
 // Structural validation for the v1 reserved-shape fields. These rules are the
@@ -59,6 +60,32 @@ var jobIDSafeNameRe = regexp.MustCompile(`^[A-Za-z0-9_-]+$`)
 // commitSHARe matches a full 40-character lowercase-hex Git commit SHA.
 var commitSHARe = regexp.MustCompile(`^[0-9a-f]{40}$`)
 
+// actionFolderRe matches a safe action_folder name: a plain path component
+// with no directory separators or traversal segments, since the generator
+// joins it directly into a filesystem path under .github/actions/.
+var actionFolderRe = regexp.MustCompile(`^[A-Za-z0-9._-]+$`)
+
+// validateActionFolder rejects an action_folder value that is empty, contains
+// a path separator, contains a ".." traversal segment, or otherwise falls
+// outside a safe plain-name charset. The generator joins the value directly
+// into .github/actions/<folder>/action.yaml, so an unsafe value could escape
+// the intended actions directory.
+func validateActionFolder(folder string) []string {
+	if folder == "" {
+		return nil
+	}
+	if strings.Contains(folder, "..") || strings.Contains(folder, "/") || !actionFolderRe.MatchString(folder) {
+		return []string{fmt.Sprintf("action_folder %q must be a plain folder name with no path separators or '..' segments", folder)}
+	}
+	return nil
+}
+
+// actionPinValueRe bounds an action_pins override value to a ref plus an
+// optional trailing "# <version>" comment. It rejects newlines and any
+// character that could break out of the emitted YAML scalar, since actionRef
+// splices the value raw into a generated workflow.
+var actionPinValueRe = regexp.MustCompile(`^[A-Za-z0-9._+/-]+(?: # [A-Za-z0-9._+-]+)?$`)
+
 // dispatchInputOptionRe matches a choice dispatch-input option that is safe to
 // emit verbatim as a YAML block-sequence item under workflow_dispatch's
 // inputs.<name>.options. cascade renders each option unquoted, so the accepted
@@ -168,24 +195,77 @@ func validateCallbackTimeout(prefix string, isReusableWorkflow bool, timeoutMinu
 		"%s: timeout_minutes is not valid on a reusable-workflow callback; GitHub forbids timeout-minutes on a job that calls a reusable workflow - set timeout-minutes inside your callback workflow instead", prefix)}
 }
 
+// localCallbackPathRe bounds a bare-filename or .github/workflows/... local
+// callback value to characters safe to splice raw into a generated
+// workflow's uses: line. It rejects newlines, other control characters,
+// quotes, whitespace, and any character that could break out of the emitted
+// YAML scalar, while allowing the path characters a callback workflow ref
+// legitimately needs. The trailing "$" is Go's default (non-multiline)
+// anchor, which matches only the true end of the string, so a trailing
+// newline is rejected the same as an embedded one.
+var localCallbackPathRe = regexp.MustCompile(`^[A-Za-z0-9._/-]+$`)
+
+// crossRepoCallbackRe bounds a cross-repo "@"-containing callback ref to a
+// path, a single "@", and a ref, each drawn from the same safe path charset
+// as localCallbackPathRe. This is the positive-charset counterpart to the
+// up-front containsUnsafeChar guard below, applied to the one accepted shape
+// that carries an "@".
+var crossRepoCallbackRe = regexp.MustCompile(`^[A-Za-z0-9._/-]+@[A-Za-z0-9._/-]+$`)
+
+// containsUnsafeChar reports whether s contains a newline, carriage return,
+// other control character, or whitespace. Every branch of
+// validateLocalCallbackWorkflowPath ultimately splices its value raw into a
+// generated workflow's uses: line, so this check is applied once, up front,
+// rather than duplicated per branch: it keeps guarding every branch even if
+// the function grows a new accepted shape later.
+func containsUnsafeChar(s string) bool {
+	for _, r := range s {
+		if unicode.IsControl(r) || unicode.IsSpace(r) {
+			return true
+		}
+	}
+	return false
+}
+
 // validateLocalCallbackWorkflowPath checks that a local callback workflow path
 // is either a bare filename, a .github/workflows/... path, or a cross-repo
 // external ref (containing "@"). Any other form is rejected because GitHub
-// requires local reusable workflows to live under .github/workflows/.
+// requires local reusable workflows to live under .github/workflows/. Every
+// accepted form is charset-validated: the value is spliced raw into a
+// generated workflow's uses: line, so it must never carry a newline or other
+// character that could break out of the emitted YAML scalar.
 func validateLocalCallbackWorkflowPath(prefix, workflow string) []string {
 	if workflow == "" {
 		return nil
 	}
-	// Cross-repo external refs contain "@" - always valid.
+	if containsUnsafeChar(workflow) {
+		return []string{fmt.Sprintf("%s: local callback workflow %q contains unsafe characters", prefix, workflow)}
+	}
+	// Cross-repo external refs contain "@" - valid only when the whole value
+	// is a path@ref shape drawn from the safe path charset.
 	if strings.Contains(workflow, "@") {
+		if !crossRepoCallbackRe.MatchString(workflow) {
+			return []string{fmt.Sprintf("%s: cross-repo local callback workflow %q must be a path@ref reference", prefix, workflow)}
+		}
 		return nil
 	}
 	// Bare filename (no "/") - valid; normalizeWorkflowPath will route it.
 	if !strings.Contains(workflow, "/") {
+		if !localCallbackPathRe.MatchString(workflow) {
+			return []string{fmt.Sprintf("%s: local callback workflow %q contains unsafe characters", prefix, workflow)}
+		}
 		return nil
 	}
-	// .github/workflows/... path - valid.
+	// .github/workflows/... path - valid only when it stays inside that
+	// directory and carries no unsafe characters; a ".." traversal segment
+	// after the prefix must not escape it.
 	if strings.HasPrefix(workflow, ".github/workflows/") || strings.HasPrefix(workflow, "./.github/workflows/") {
+		if strings.Contains(workflow, "..") {
+			return []string{fmt.Sprintf("%s: local callback workflow must not contain '..' segments, got %q", prefix, workflow)}
+		}
+		if !localCallbackPathRe.MatchString(workflow) {
+			return []string{fmt.Sprintf("%s: local callback workflow %q contains unsafe characters", prefix, workflow)}
+		}
 		return nil
 	}
 	return []string{fmt.Sprintf("%s: local callback workflow must be a .github/workflows/... path or a bare filename, got %q", prefix, workflow)}
@@ -349,10 +429,28 @@ func validateConfigLevel(cfg *TrunkConfig) []string {
 	errs = append(errs, validateEnvironmentConfig(cfg)...)
 	errs = append(errs, validateTokenSources(cfg)...)
 	errs = append(errs, validateRollback(cfg.Rollback)...)
+	errs = append(errs, validateActionPins(cfg)...)
+	errs = append(errs, validateActionFolder(cfg.ActionFolder)...)
 
 	return errs
 }
 
+// validateActionPins charset-validates every action_pins override value. Each
+// value is spliced raw into a generated workflow's uses: line, so it must be
+// bounded to a ref plus an optional trailing "# <version>" comment and must
+// never carry a newline or other character that could break out of the
+// emitted YAML scalar.
+func validateActionPins(cfg *TrunkConfig) []string {
+	var errs []string
+	for _, action := range sortedKeys(cfg.ActionPins) {
+		ref := cfg.ActionPins[action]
+		if !actionPinValueRe.MatchString(ref) {
+			errs = append(errs, fmt.Sprintf("action_pins[%q]: invalid ref %q", action, ref))
+		}
+	}
+	return errs
+}
+
 // repositoryDispatchTypeRe matches a repository_dispatch event type that is safe
 // to emit verbatim under the on.repository_dispatch.types list. GitHub accepts
 // arbitrary client-chosen event-type strings, but cascade renders them into YAML

internal/generate/actions.go

@@ -4,17 +4,27 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strings"
 
 	"github.com/stablekernel/cascade/internal/config"
 )
 
+// actionFolderRe mirrors the config package's action_folder charset check.
+// This is a defense-in-depth guard: config validation already rejects an
+// unsafe action_folder, but the generator refuses to join an unsafe value
+// into a filesystem path rather than trusting the caller validated it.
+var actionFolderRe = regexp.MustCompile(`^[A-Za-z0-9._-]+$`)
+
 // RenderLocalActions returns the composite action file the manifest would
 // generate, paired with its rendered content, without writing anything to disk.
 // The path is baseDir/.github/actions/<folder>/action.yaml where <folder> is
 // cfg.GetActionFolder() (default: "manage-release").
 func RenderLocalActions(baseDir string, cfg *config.TrunkConfig) (PlannedFile, error) {
 	actionFolder := cfg.GetActionFolder()
+	if strings.Contains(actionFolder, "..") || strings.Contains(actionFolder, "/") || !actionFolderRe.MatchString(actionFolder) {
+		return PlannedFile{}, fmt.Errorf("action_folder %q is not a safe plain folder name", actionFolder)
+	}
 	actionPath := filepath.Join(baseDir, ".github", "actions", actionFolder, "action.yaml")
 	return PlannedFile{Path: actionPath, Content: generateManageReleaseAction()}, nil
 }

internal/generate/actions_test.go

@@ -0,0 +1,37 @@
+package generate
+
+import (
+	"testing"
+
+	"github.com/stablekernel/cascade/internal/config"
+	"github.com/stretchr/testify/require"
+)
+
+// TestRenderLocalActions_RejectsUnsafeActionFolder is a defense-in-depth
+// guard: even though config validation already rejects an unsafe
+// action_folder, the generator itself refuses to join an unsafe value into a
+// filesystem path rather than trusting the caller validated it.
+func TestRenderLocalActions_RejectsUnsafeActionFolder(t *testing.T) {
+	t.Parallel()
+	cfg := &config.TrunkConfig{ActionFolder: "../../etc/foo"}
+	_, err := RenderLocalActions(t.TempDir(), cfg)
+	require.Error(t, err)
+}
+
+// TestGenerateLocalActions_RejectsUnsafeActionFolder mirrors the above for the
+// disk-writing entry point.
+func TestGenerateLocalActions_RejectsUnsafeActionFolder(t *testing.T) {
+	t.Parallel()
+	cfg := &config.TrunkConfig{ActionFolder: "a/b"}
+	err := GenerateLocalActions(t.TempDir(), cfg)
+	require.Error(t, err)
+}
+
+// TestRenderLocalActions_AllowsDefaultActionFolder is the regression lock: a
+// safe, default action_folder still renders with no error.
+func TestRenderLocalActions_AllowsDefaultActionFolder(t *testing.T) {
+	t.Parallel()
+	cfg := &config.TrunkConfig{}
+	_, err := RenderLocalActions(t.TempDir(), cfg)
+	require.NoError(t, err)
+}

internal/pinreconcile/push.go

@@ -0,0 +1,16 @@
+// Package pinreconcile implements the pin-reconciliation engine: adopting an
+// external action-pin change back into the manifest and regenerating so every
+// owned file agrees with it again.
+package pinreconcile
+
+// StagePathspec returns the ONLY pathspec a reconcile commit may stage, so a
+// crafted working-tree change cannot ride along (never git add -A). The
+// common case is manifest-only (Contents: write); pushWorkflows is set only
+// when a regenerate must push workflow files (the Workflows: write path).
+func StagePathspec(manifestPath string, pushWorkflows bool) []string {
+	spec := []string{manifestPath}
+	if pushWorkflows {
+		spec = append(spec, ".github/workflows/*.yaml")
+	}
+	return spec
+}

internal/pinreconcile/push_test.go

@@ -0,0 +1,22 @@
+package pinreconcile
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// TestStagePathspec_ManifestOnlyByDefault covers the common case: a reconcile
+// commit stages only the manifest, never a wildcard pathspec that could pick
+// up an unrelated working-tree change.
+func TestStagePathspec_ManifestOnlyByDefault(t *testing.T) {
+	require.Equal(t, []string{".github/manifest.yaml"}, StagePathspec(".github/manifest.yaml", false))
+}
+
+// TestStagePathspec_IncludesWorkflowsWhenRegenerating covers the mode where a
+// regenerate must itself push workflow files alongside the manifest.
+func TestStagePathspec_IncludesWorkflowsWhenRegenerating(t *testing.T) {
+	require.Equal(t,
+		[]string{".github/manifest.yaml", ".github/workflows/*.yaml"},
+		StagePathspec(".github/manifest.yaml", true))
+}