From 197cacf67b6707cfac9ac07da5a864e1435f3856 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 4 Jul 2026 04:25:03 +0000 Subject: [PATCH 1/5] Initial plan From 08ef4ac34869703dce9a29fd4a4a2c951e9567f5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 4 Jul 2026 04:47:02 +0000 Subject: [PATCH 2/5] fix: correct engine.env secrets warning to reflect auto-exclusion from sandbox The warning "will be leaked to the agent container" was factually incorrect for engine.env secrets. ComputeAWFExcludeEnvVarNames already auto-detects ${{ secrets. values in engine.env and adds them to awf --exclude-env, preventing them from reaching the agent sandbox. Now validateEnvSecretsSection emits section-aware messages: - engine.env (non-strict): "will be excluded from the agent sandbox via awf --exclude-env; the agent process itself will not see these values directly" - engine.env (strict): "are auto-excluded from the agent sandbox via awf --exclude-env and are not accessible to the agent" - top-level env: messages unchanged (secrets there genuinely leak) Update tests to match the corrected engine.env error message strings. Closes #42749 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/env_secrets_validation_test.go | 6 +++--- pkg/workflow/strict_mode_env_validation.go | 14 +++++++++++++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/pkg/workflow/env_secrets_validation_test.go b/pkg/workflow/env_secrets_validation_test.go index 1d0799117c1..268c639e61b 100644 --- a/pkg/workflow/env_secrets_validation_test.go +++ b/pkg/workflow/env_secrets_validation_test.go @@ -537,7 +537,7 @@ func TestValidateEngineEnvSecrets(t *testing.T) { }, strictMode: true, expectError: true, - errorMsg: "strict mode: secrets detected in 'engine.env' section will be leaked to the agent container. Found: ${{ secrets.API_KEY }}", + errorMsg: "strict mode: secrets detected in 'engine.env' section are auto-excluded from the agent sandbox via awf --exclude-env and are not accessible to the agent. Found: ${{ secrets.API_KEY }}", }, { name: "engine.env with multiple secrets in strict mode fails", @@ -553,7 +553,7 @@ func TestValidateEngineEnvSecrets(t *testing.T) { }, strictMode: true, expectError: true, - errorMsg: "strict mode: secrets detected in 'engine.env' section will be leaked to the agent container", + errorMsg: "strict mode: secrets detected in 'engine.env' section are auto-excluded from the agent sandbox", }, { name: "engine.env with secret embedded in string in strict mode fails", @@ -568,7 +568,7 @@ func TestValidateEngineEnvSecrets(t *testing.T) { }, strictMode: true, expectError: true, - errorMsg: "strict mode: secrets detected in 'engine.env' section will be leaked to the agent container. Found: ${{ secrets.TOKEN }}", + errorMsg: "strict mode: secrets detected in 'engine.env' section are auto-excluded from the agent sandbox via awf --exclude-env and are not accessible to the agent. Found: ${{ secrets.TOKEN }}", }, { name: "engine.env with secret in non-strict mode emits warning (no error)", diff --git a/pkg/workflow/strict_mode_env_validation.go b/pkg/workflow/strict_mode_env_validation.go index c3e612b8863..21d24221974 100644 --- a/pkg/workflow/strict_mode_env_validation.go +++ b/pkg/workflow/strict_mode_env_validation.go @@ -132,11 +132,23 @@ func (c *Compiler) validateEnvSecretsSection(config map[string]any, sectionName // In strict mode, this is an error if c.strictMode { + if sectionName == "engine.env" { + // engine.env secrets are auto-excluded from the agent sandbox via awf --exclude-env, + // so they are not leaked, but strict mode still requires engine-specific configuration. + return fmt.Errorf("strict mode: secrets detected in 'engine.env' section are auto-excluded from the agent sandbox via awf --exclude-env and are not accessible to the agent. Found: %s. Use engine-specific secret configuration instead. See: https://github.github.com/gh-aw/reference/engines/", strings.Join(secretRefs, ", ")) + } return fmt.Errorf("strict mode: secrets detected in '%s' section will be leaked to the agent container. Found: %s. Use engine-specific secret configuration instead. See: https://github.github.com/gh-aw/reference/engines/", sectionName, strings.Join(secretRefs, ", ")) } // In non-strict mode, emit a warning - warningMsg := fmt.Sprintf("Warning: secrets detected in '%s' section will be leaked to the agent container. Found: %s. Consider using engine-specific secret configuration instead.", sectionName, strings.Join(secretRefs, ", ")) + var warningMsg string + if sectionName == "engine.env" { + // engine.env secrets are auto-excluded from the agent sandbox via awf --exclude-env, + // so the warning should reflect that they are excluded, not leaked. + warningMsg = fmt.Sprintf("Warning: secrets detected in 'engine.env' section will be excluded from the agent sandbox via awf --exclude-env; the agent process itself will not see these values directly. Found: %s. Consider using engine-specific secret configuration instead.", strings.Join(secretRefs, ", ")) + } else { + warningMsg = fmt.Sprintf("Warning: secrets detected in '%s' section will be leaked to the agent container. Found: %s. Consider using engine-specific secret configuration instead.", sectionName, strings.Join(secretRefs, ", ")) + } fmt.Fprintln(os.Stderr, console.FormatWarningMessage(warningMsg)) c.IncrementWarningCount() From 9fd3f266973ae0341011895342152b374d68f60b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 4 Jul 2026 05:06:07 +0000 Subject: [PATCH 3/5] test: add TestComputeAWFExcludeEnvVarNames to verify engine.env auto-exclusion Adds a table-driven test for ComputeAWFExcludeEnvVarNames covering: - engine.env var with a secret reference is auto-excluded - engine.env var with no secret reference is not excluded - mixed engine.env: only secret-backed vars are excluded - engine.env secret combined with core engine secret vars - engine.env secret embedded in a larger string value is excluded - nil EngineConfig produces no extra exclusions beyond core secrets Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/awf_helpers_test.go | 98 ++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/pkg/workflow/awf_helpers_test.go b/pkg/workflow/awf_helpers_test.go index d4b8ac75680..831742cb066 100644 --- a/pkg/workflow/awf_helpers_test.go +++ b/pkg/workflow/awf_helpers_test.go @@ -1118,6 +1118,104 @@ func TestAWFSupportsExcludeEnv(t *testing.T) { } } +// TestComputeAWFExcludeEnvVarNames verifies that engine.env vars whose values contain +// ${{ secrets.* }} are automatically included in the --exclude-env list, and that +// non-secret engine.env vars and plain-value core secrets are handled correctly. +func TestComputeAWFExcludeEnvVarNames(t *testing.T) { + tests := []struct { + name string + workflowData *WorkflowData + coreSecretVarNames []string + want []string + notWant []string + }{ + { + name: "engine.env secret var is auto-excluded", + workflowData: &WorkflowData{ + EngineConfig: &EngineConfig{ + Env: map[string]string{ + "GOOGLE_API_KEY": "${{ secrets.SOME_KEY }}", + }, + }, + }, + coreSecretVarNames: []string{}, + want: []string{"GOOGLE_API_KEY"}, + }, + { + name: "engine.env non-secret var is not excluded", + workflowData: &WorkflowData{ + EngineConfig: &EngineConfig{ + Env: map[string]string{ + "DEBUG": "true", + "LOG_LEVEL": "info", + }, + }, + }, + coreSecretVarNames: []string{}, + want: []string{}, + notWant: []string{"DEBUG", "LOG_LEVEL"}, + }, + { + name: "engine.env mixes secret and non-secret vars: only secrets excluded", + workflowData: &WorkflowData{ + EngineConfig: &EngineConfig{ + Env: map[string]string{ + "GOOGLE_API_KEY": "${{ secrets.SOME_KEY }}", + "LOG_LEVEL": "debug", + }, + }, + }, + coreSecretVarNames: []string{}, + want: []string{"GOOGLE_API_KEY"}, + notWant: []string{"LOG_LEVEL"}, + }, + { + name: "engine.env secret combined with core secret vars", + workflowData: &WorkflowData{ + EngineConfig: &EngineConfig{ + Env: map[string]string{ + "CUSTOM_API_KEY": "${{ secrets.CUSTOM_KEY }}", + }, + }, + }, + coreSecretVarNames: []string{"GEMINI_API_KEY"}, + want: []string{"GEMINI_API_KEY", "CUSTOM_API_KEY"}, + }, + { + name: "engine.env secret embedded in a larger string is excluded", + workflowData: &WorkflowData{ + EngineConfig: &EngineConfig{ + Env: map[string]string{ + "AUTH_HEADER": "Bearer ${{ secrets.TOKEN }}", + }, + }, + }, + coreSecretVarNames: []string{}, + want: []string{"AUTH_HEADER"}, + }, + { + name: "nil engine config produces no exclusions beyond core secrets", + workflowData: &WorkflowData{ + EngineConfig: nil, + }, + coreSecretVarNames: []string{"COPILOT_GITHUB_TOKEN"}, + want: []string{"COPILOT_GITHUB_TOKEN"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ComputeAWFExcludeEnvVarNames(tt.workflowData, tt.coreSecretVarNames) + for _, name := range tt.want { + assert.Contains(t, got, name, "expected %q in exclude list", name) + } + for _, name := range tt.notWant { + assert.NotContains(t, got, name, "expected %q to be absent from exclude list", name) + } + }) + } +} + // TestBuildAWFArgsCliProxy tests that BuildAWFArgs correctly injects --difc-proxy-host // and --difc-proxy-ca-cert based on the cli-proxy feature flag. func TestBuildAWFArgsCliProxy(t *testing.T) { From 6c11bbc0c8d84631f1c3b8ba880dad89416ff0e8 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 4 Jul 2026 06:42:15 +0000 Subject: [PATCH 4/5] docs(adr): add draft ADR-43302 for section-aware secrets validation messages Co-Authored-By: Claude Sonnet 4.6 --- ...ction-aware-secrets-validation-messages.md | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 docs/adr/43302-section-aware-secrets-validation-messages.md diff --git a/docs/adr/43302-section-aware-secrets-validation-messages.md b/docs/adr/43302-section-aware-secrets-validation-messages.md new file mode 100644 index 00000000000..d8c89c04255 --- /dev/null +++ b/docs/adr/43302-section-aware-secrets-validation-messages.md @@ -0,0 +1,73 @@ +# ADR-43302: Section-Aware Secret Validation Messages in Workflow Compiler + +**Date**: 2026-07-04 +**Status**: Draft +**Deciders**: Unknown + +--- + +### Context + +The workflow compiler's `validateEnvSecretsSection` function validates secrets across multiple +env sections (`env`, `engine.env`) and emits warnings or errors when `${{ secrets.* }}` +references are detected. Historically it used a single generic message — "will be leaked to the +agent container" — regardless of which section contained the secret reference. + +However, `engine.env` secrets are handled differently from top-level `env` secrets: the +`ComputeAWFExcludeEnvVarNames` function auto-detects any `engine.env` value that contains a +`${{ secrets.* }}` reference and adds the corresponding variable name to AWF's `--exclude-env` +list. This means `engine.env` secrets never reach the agent sandbox process, making the +"will be leaked" warning factually incorrect for that section and a source of user confusion. + +### Decision + +We will differentiate the validation messages emitted by `validateEnvSecretsSection` based on +which section is being validated, rather than using a single generic message for all sections. +For `engine.env`, messages will accurately reflect that secrets are auto-excluded from the +agent sandbox via AWF `--exclude-env`; for all other sections (e.g., top-level `env`), the +existing "will be leaked" language is retained because those secrets genuinely reach the +agent container. Strict mode continues to treat `engine.env` secrets as an error (encouraging +engine-specific secret configuration), but with accurate, non-misleading error text. + +### Alternatives Considered + +#### Alternative 1: Remove the `engine.env` warning entirely + +Since `engine.env` secrets are auto-excluded and never reach the agent process, one option is +to suppress the warning entirely for that section — no message, no error. This eliminates the +false positive and simplifies the code path. It was rejected because users should still be +nudged toward engine-specific secret configuration (the more secure, explicit pattern), and +removing all feedback would silently permit a configuration style that the platform discourages. + +#### Alternative 2: Extract a separate validation function for `engine.env` + +Rather than branching inside `validateEnvSecretsSection`, a dedicated +`validateEngineEnvSecretsSection` function could own all `engine.env` secret logic. This would +keep each function single-purpose and avoid section-name string comparisons. It was rejected +for this fix because it would duplicate the secret-detection regex and collection logic that +lives in `validateEnvSecretsSection`, increasing maintenance surface for a change whose primary +goal is correcting a misleading string — not restructuring validation logic. + +### Consequences + +#### Positive +- Users see accurate feedback: `engine.env` secret warnings now correctly describe sandbox + exclusion rather than falsely claiming leakage to the agent container. +- Strict mode error messages for `engine.env` remain actionable and no longer contradict the + platform's actual security behavior, reducing support burden and user confusion. + +#### Negative +- `validateEnvSecretsSection` now contains section-specific branching (`if sectionName == "engine.env"`), + slightly increasing cyclomatic complexity and making the function less generic. +- Any future env section added to the validation path must be evaluated for whether it also + requires a custom message, creating an implicit maintenance contract. + +#### Neutral +- Test assertions for `engine.env` strict-mode cases were updated to match the new message + text; this is a mechanical change with no behavioral impact on test coverage. +- The `awf_helpers_test.go` additions verify the existing `ComputeAWFExcludeEnvVarNames` + behavior that this fix depends on for correctness, improving confidence in the invariant. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* From 5e54ceae9286b6ba86733e43538cdd6fc92a4809 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 4 Jul 2026 10:42:47 +0000 Subject: [PATCH 5/5] fix: qualify engine.env exclusion messages with AWF v0.25.3+ version requirement Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/workflow/env_secrets_validation_test.go | 8 +++++--- pkg/workflow/strict_mode_env_validation.go | 15 +++++++++------ 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/pkg/workflow/env_secrets_validation_test.go b/pkg/workflow/env_secrets_validation_test.go index 268c639e61b..3ded8444a2f 100644 --- a/pkg/workflow/env_secrets_validation_test.go +++ b/pkg/workflow/env_secrets_validation_test.go @@ -3,9 +3,11 @@ package workflow import ( + "fmt" "strings" "testing" + "github.com/github/gh-aw/pkg/constants" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -537,7 +539,7 @@ func TestValidateEngineEnvSecrets(t *testing.T) { }, strictMode: true, expectError: true, - errorMsg: "strict mode: secrets detected in 'engine.env' section are auto-excluded from the agent sandbox via awf --exclude-env and are not accessible to the agent. Found: ${{ secrets.API_KEY }}", + errorMsg: fmt.Sprintf("are excluded from the agent sandbox via awf --exclude-env (requires AWF %s+) and are not accessible to the agent when that version is in use. Found: ${{ secrets.API_KEY }}", constants.AWFExcludeEnvMinVersion), }, { name: "engine.env with multiple secrets in strict mode fails", @@ -553,7 +555,7 @@ func TestValidateEngineEnvSecrets(t *testing.T) { }, strictMode: true, expectError: true, - errorMsg: "strict mode: secrets detected in 'engine.env' section are auto-excluded from the agent sandbox", + errorMsg: fmt.Sprintf("are excluded from the agent sandbox via awf --exclude-env (requires AWF %s+)", constants.AWFExcludeEnvMinVersion), }, { name: "engine.env with secret embedded in string in strict mode fails", @@ -568,7 +570,7 @@ func TestValidateEngineEnvSecrets(t *testing.T) { }, strictMode: true, expectError: true, - errorMsg: "strict mode: secrets detected in 'engine.env' section are auto-excluded from the agent sandbox via awf --exclude-env and are not accessible to the agent. Found: ${{ secrets.TOKEN }}", + errorMsg: fmt.Sprintf("are excluded from the agent sandbox via awf --exclude-env (requires AWF %s+) and are not accessible to the agent when that version is in use. Found: ${{ secrets.TOKEN }}", constants.AWFExcludeEnvMinVersion), }, { name: "engine.env with secret in non-strict mode emits warning (no error)", diff --git a/pkg/workflow/strict_mode_env_validation.go b/pkg/workflow/strict_mode_env_validation.go index 21d24221974..f9c45d8a4c0 100644 --- a/pkg/workflow/strict_mode_env_validation.go +++ b/pkg/workflow/strict_mode_env_validation.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/github/gh-aw/pkg/console" + "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/setutil" ) @@ -133,9 +134,10 @@ func (c *Compiler) validateEnvSecretsSection(config map[string]any, sectionName // In strict mode, this is an error if c.strictMode { if sectionName == "engine.env" { - // engine.env secrets are auto-excluded from the agent sandbox via awf --exclude-env, - // so they are not leaked, but strict mode still requires engine-specific configuration. - return fmt.Errorf("strict mode: secrets detected in 'engine.env' section are auto-excluded from the agent sandbox via awf --exclude-env and are not accessible to the agent. Found: %s. Use engine-specific secret configuration instead. See: https://github.github.com/gh-aw/reference/engines/", strings.Join(secretRefs, ", ")) + // engine.env secrets are excluded from the agent sandbox via awf --exclude-env + // (requires AWF v0.25.3+), so they are not leaked, but strict mode still requires + // engine-specific configuration. + return fmt.Errorf("strict mode: secrets detected in 'engine.env' section are excluded from the agent sandbox via awf --exclude-env (requires AWF %s+) and are not accessible to the agent when that version is in use. Found: %s. Use engine-specific secret configuration instead. See: https://github.github.com/gh-aw/reference/engines/", constants.AWFExcludeEnvMinVersion, strings.Join(secretRefs, ", ")) } return fmt.Errorf("strict mode: secrets detected in '%s' section will be leaked to the agent container. Found: %s. Use engine-specific secret configuration instead. See: https://github.github.com/gh-aw/reference/engines/", sectionName, strings.Join(secretRefs, ", ")) } @@ -143,9 +145,10 @@ func (c *Compiler) validateEnvSecretsSection(config map[string]any, sectionName // In non-strict mode, emit a warning var warningMsg string if sectionName == "engine.env" { - // engine.env secrets are auto-excluded from the agent sandbox via awf --exclude-env, - // so the warning should reflect that they are excluded, not leaked. - warningMsg = fmt.Sprintf("Warning: secrets detected in 'engine.env' section will be excluded from the agent sandbox via awf --exclude-env; the agent process itself will not see these values directly. Found: %s. Consider using engine-specific secret configuration instead.", strings.Join(secretRefs, ", ")) + // engine.env secrets are excluded from the agent sandbox via awf --exclude-env + // (requires AWF v0.25.3+). On older AWF versions this protection is not applied and + // the values will reach the agent container. + warningMsg = fmt.Sprintf("Warning: secrets detected in 'engine.env' section will be excluded from the agent sandbox via awf --exclude-env (requires AWF %s+); on older AWF versions the agent process will see these values. Found: %s. Consider using engine-specific secret configuration instead.", constants.AWFExcludeEnvMinVersion, strings.Join(secretRefs, ", ")) } else { warningMsg = fmt.Sprintf("Warning: secrets detected in '%s' section will be leaked to the agent container. Found: %s. Consider using engine-specific secret configuration instead.", sectionName, strings.Join(secretRefs, ", ")) }