Harden gh aw env update with strict input validation and non-mutating preview (#35358)

Author: Copilot

.github/workflows/daily-spdd-spec-planner.lock.yml

@@ -39,6 +39,9 @@ For max effective tokens, precedence is:
 2. `GH_AW_DEFAULT_MAX_EFFECTIVE_TOKENS`
 3. Built-in compiler default
 
+A negative `GH_AW_DEFAULT_MAX_EFFECTIVE_TOKENS` disables AWF token steering and
+omits the budget limit when frontmatter does not set `max-effective-tokens`.
+
 For default timeout-minutes, precedence is:
 
 1. `timeout-minutes` in workflow frontmatter

docs/src/content/docs/reference/compiler-enterprise-environment-controls.md

@@ -231,8 +231,12 @@ gh aw env update defaults.yml --scope org --org MY_ORG
 ```
 
 `gh aw env update` shows a confirmation preview before applying changes.
-Pass `--yes` to skip the prompt in automation. Set a field to `null` to delete
-the corresponding variable from the target scope.
+Pass `--yes` to skip the prompt in automation, or `--dry-run` to preview
+without changing any variables. Set a field to `null` to delete the
+corresponding variable from the target scope. Unknown YAML keys are rejected,
+`default_max_turns` / `default_timeout_minutes` must be positive integers, and
+`default_max_effective_tokens` must be a non-zero integer (negative values
+disable token steering and budget enforcement).
 
 3. If you compile workflows in CI, pass compiler-read defaults into
 the compiler process environment (for example via `${{ vars.* }}`):

docs/src/content/docs/reference/cost-management.md

@@ -6,6 +6,7 @@ import (
 	"net/url"
 	"os"
 	"os/exec"
+	"strconv"
 	"strings"
 
 	"github.com/github/gh-aw/pkg/console"
@@ -146,7 +147,7 @@ func newDefaultsGetCommand() *cobra.Command {
 
 func newDefaultsUpdateCommand() *cobra.Command {
 	var scope, repo, org, enterprise string
-	var yes bool
+	var yes, dryRun bool
 
 	cmd := &cobra.Command{
 		Use:   "update [file]",
@@ -161,7 +162,7 @@ func newDefaultsUpdateCommand() *cobra.Command {
 			if err != nil {
 				return err
 			}
-			return defaultsUpdateFromFile(target, inputFile, yes)
+			return defaultsUpdateFromFile(target, inputFile, yes, dryRun)
 		},
 	}
 
@@ -170,6 +171,7 @@ func newDefaultsUpdateCommand() *cobra.Command {
 	cmd.Flags().StringVar(&org, "org", "", "Target organization (required for --scope org unless inferable from --repo/current repo)")
 	cmd.Flags().StringVar(&enterprise, "enterprise", "", "Target enterprise slug (required for --scope ent)")
 	cmd.Flags().BoolVarP(&yes, "yes", "y", false, "Skip confirmation prompt")
+	cmd.Flags().BoolVar(&dryRun, "dry-run", false, "Preview updates without applying any changes")
 	_ = cmd.MarkFlagRequired("scope")
 	return cmd
 }
@@ -200,18 +202,26 @@ func defaultsGetToFile(target defaultsTarget, outputFile string) error {
 	return nil
 }
 
-func defaultsUpdateFromFile(target defaultsTarget, inputFile string, skipConfirmation bool) error {
+func defaultsUpdateFromFile(target defaultsTarget, inputFile string, skipConfirmation, dryRun bool) error {
 	data, err := os.ReadFile(inputFile)
 	if err != nil {
 		return fmt.Errorf("failed to read defaults file %q: %w", inputFile, err)
 	}
 
-	var file defaultsFile
-	if err := yaml.Unmarshal(data, &file); err != nil {
-		return fmt.Errorf("failed to parse defaults file %q: %w", inputFile, err)
+	file, err := defaultsParseFile(inputFile, data)
+	if err != nil {
+		return err
+	}
+	if err := defaultsValidateFile(&file); err != nil {
+		return err
 	}
 
 	changes := defaultsBuildUpdateChanges(&file)
+	if dryRun {
+		renderDefaultsUpdatePreview(target, inputFile, changes)
+		fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Dry-run mode enabled; no variables were changed."))
+		return nil
+	}
 	if err := confirmDefaultsUpdate(target, inputFile, changes, skipConfirmation, console.ConfirmAction); err != nil {
 		return err
 	}
@@ -232,6 +242,60 @@ func defaultsUpdateFromFile(target defaultsTarget, inputFile string, skipConfirm
 	return nil
 }
 
+func defaultsParseFile(inputFile string, data []byte) (defaultsFile, error) {
+	var file defaultsFile
+	if err := yaml.UnmarshalWithOptions(data, &file, yaml.DisallowUnknownField()); err != nil {
+		return defaultsFile{}, fmt.Errorf("failed to parse defaults file %q: %w", inputFile, err)
+	}
+	return file, nil
+}
+
+func defaultsValidateFile(file *defaultsFile) error {
+	var validationErrors []string
+
+	validateNonZeroInt := func(field string, value *string) {
+		if value == nil {
+			return
+		}
+		trimmed := strings.TrimSpace(*value)
+		parsed, err := strconv.ParseInt(trimmed, 10, 64)
+		if err != nil || parsed == 0 {
+			validationErrors = append(validationErrors, fmt.Sprintf("%s must be a non-zero integer when set", field))
+		}
+	}
+	validatePositiveInt := func(field string, value *string) {
+		if value == nil {
+			return
+		}
+		trimmed := strings.TrimSpace(*value)
+		parsed, err := strconv.ParseInt(trimmed, 10, 64)
+		if err != nil || parsed <= 0 {
+			validationErrors = append(validationErrors, fmt.Sprintf("%s must be a positive integer when set", field))
+		}
+	}
+	validateNonEmpty := func(field string, value *string) {
+		if value == nil {
+			return
+		}
+		if strings.TrimSpace(*value) == "" {
+			validationErrors = append(validationErrors, fmt.Sprintf("%s cannot be empty when set", field))
+		}
+	}
+
+	validateNonZeroInt("default_max_effective_tokens", file.DefaultMaxEffectiveTokens)
+	validatePositiveInt("default_max_turns", file.DefaultMaxTurns)
+	validatePositiveInt("default_timeout_minutes", file.DefaultTimeoutMinutes)
+	validateNonEmpty("default_detection_model", file.DefaultDetectionModel)
+	validateNonEmpty("default_model_copilot", file.DefaultModelCopilot)
+	validateNonEmpty("default_model_claude", file.DefaultModelClaude)
+	validateNonEmpty("default_model_codex", file.DefaultModelCodex)
+
+	if len(validationErrors) > 0 {
+		return fmt.Errorf("invalid defaults file: %s", strings.Join(validationErrors, "; "))
+	}
+	return nil
+}
+
 func defaultsBuildUpdateChanges(file *defaultsFile) []defaultsUpdateChange {
 	changes := make([]defaultsUpdateChange, 0, len(defaultsBindings))
 	for _, binding := range defaultsBindings {
@@ -261,17 +325,7 @@ func confirmDefaultsUpdate(
 	skipConfirmation bool,
 	confirmAction func(title, affirmative, negative string) (bool, error),
 ) error {
-	fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Defaults update preview:"))
-	fmt.Fprint(os.Stderr, console.RenderStruct(defaultsUpdatePreview{
-		Scope:  target.scope,
-		Target: target.displayName(),
-		File:   inputFile,
-		Fields: len(changes),
-	}))
-	fmt.Fprintln(os.Stderr)
-	fmt.Fprintln(os.Stderr)
-	fmt.Fprint(os.Stderr, console.RenderStruct(defaultsUpdateRows(changes)))
-	fmt.Fprintln(os.Stderr)
+	renderDefaultsUpdatePreview(target, inputFile, changes)
 
 	if skipConfirmation {
 		fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Skipping confirmation because --yes was provided."))
@@ -292,6 +346,20 @@ func confirmDefaultsUpdate(
 	return nil
 }
 
+func renderDefaultsUpdatePreview(target defaultsTarget, inputFile string, changes []defaultsUpdateChange) {
+	fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Defaults update preview:"))
+	fmt.Fprint(os.Stderr, console.RenderStruct(defaultsUpdatePreview{
+		Scope:  target.scope,
+		Target: target.displayName(),
+		File:   inputFile,
+		Fields: len(changes),
+	}))
+	fmt.Fprintln(os.Stderr)
+	fmt.Fprintln(os.Stderr)
+	fmt.Fprint(os.Stderr, console.RenderStruct(defaultsUpdateRows(changes)))
+	fmt.Fprintln(os.Stderr)
+}
+
 func defaultsUpdateRows(changes []defaultsUpdateChange) []defaultsUpdateRow {
 	rows := make([]defaultsUpdateRow, 0, len(changes))
 	for _, change := range changes {

pkg/cli/env_command.go

@@ -33,6 +33,7 @@ func TestNewEnvCommand(t *testing.T) {
 	assert.True(t, hasUpdate, "env command should include update subcommand")
 	require.NotNil(t, updateCmd)
 	assert.NotNil(t, updateCmd.Flags().Lookup("yes"))
+	assert.NotNil(t, updateCmd.Flags().Lookup("dry-run"))
 }
 
 func TestResolveDefaultsTarget(t *testing.T) {
@@ -120,6 +121,41 @@ func TestDefaultsFileYAMLNullDelete(t *testing.T) {
 	})
 }
 
+func TestDefaultsParseFileDisallowsUnknownFields(t *testing.T) {
+	_, err := defaultsParseFile("defaults.yml", []byte("default_max_turns: \"42\"\ndefault_model_copliot: gpt-5-mini\n"))
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "default_model_copliot")
+}
+
+func TestDefaultsValidateFile(t *testing.T) {
+	t.Run("accepts valid values", func(t *testing.T) {
+		err := defaultsValidateFile(&defaultsFile{
+			DefaultMaxEffectiveTokens: strPtr("-1"),
+			DefaultMaxTurns:           strPtr("12"),
+			DefaultTimeoutMinutes:     strPtr("30"),
+			DefaultDetectionModel:     strPtr("claude-sonnet-4.6"),
+			DefaultModelCopilot:       strPtr("gpt-5-mini"),
+			DefaultModelClaude:        strPtr("claude-haiku-4.5"),
+			DefaultModelCodex:         strPtr("gpt-5.4-mini"),
+		})
+		require.NoError(t, err)
+	})
+
+	t.Run("rejects invalid numeric and empty model values", func(t *testing.T) {
+		err := defaultsValidateFile(&defaultsFile{
+			DefaultMaxEffectiveTokens: strPtr("0"),
+			DefaultMaxTurns:           strPtr("abc"),
+			DefaultTimeoutMinutes:     strPtr("0"),
+			DefaultModelCopilot:       strPtr("   "),
+		})
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "default_max_effective_tokens must be a non-zero integer when set")
+		assert.Contains(t, err.Error(), "default_max_turns must be a positive integer when set")
+		assert.Contains(t, err.Error(), "default_timeout_minutes must be a positive integer when set")
+		assert.Contains(t, err.Error(), "default_model_copilot cannot be empty when set")
+	})
+}
+
 func TestDefaultsTargetEndpoints(t *testing.T) {
 	repoTarget := defaultsTarget{scope: defaultsScopeRepo, repoOwner: "github", repoName: "gh-aw"}
 	orgTarget := defaultsTarget{scope: defaultsScopeOrg, org: "github"}

pkg/cli/env_command_test.go

@@ -36,6 +36,11 @@ func TestResolveDefaultMaxEffectiveTokens(t *testing.T) {
 		t.Setenv(DefaultMaxEffectiveTokens, "424242")
 		assert.Equal(t, int64(424242), ResolveDefaultMaxEffectiveTokens(10))
 	})
+
+	t.Run("negative value overrides fallback", func(t *testing.T) {
+		t.Setenv(DefaultMaxEffectiveTokens, "-1")
+		assert.Equal(t, int64(-1), ResolveDefaultMaxEffectiveTokens(10))
+	})
 }
 
 func TestBuildModelOverrideExpression(t *testing.T) {