Skip to content

Strict mode ignores an imported engine's supported-env-var-keys for engine.env secrets #75272

Open

Description

@dsfaccini

This issue was posted by Claude Code using claude-opus-5-5 on behalf of David.

Strict mode never reads an imported engine's behaviors.supported-env-var-keys, so a declared key in engine.env fails. Built-in engine: claude declares the same key and passes.

Reproduction

.github/workflows/shared/engine.md:

---
engine:
  id: my-engine
  display-name: My Engine
  behaviors:
    secret-strategy: universal-llm-consumer
    supported-env-var-keys: [ANTHROPIC_API_KEY]
    execution:
      command-name: my-engine
      step-name: Execute My Engine
---

.github/workflows/repro.md:

---
on: workflow_dispatch
permissions:
  contents: read
strict: true
imports: [shared/engine.md]
engine:
  id: my-engine
  model: anthropic/claude-sonnet-4
  env:
    ANTHROPIC_API_KEY: ${{ secrets.MY_ANTHROPIC_KEY }}
---
Say hello.

gh aw compile repro --no-emit --no-check-update on v1.0.1 and main f7b1cc2:

repro.md:7:1: error: strict mode: secrets detected in 'engine.env' section [...] Found: ${{ secrets.MY_ANTHROPIC_KEY }}.

Expected: it compiles. With strict: false, the lock already passes --exclude-env ANTHROPIC_API_KEY.

Root cause (f7b1cc2)

setupEngineAndImports runs validateEarlyEnvSecrets (pkg/workflow/strict_mode_env_validation.go) before resolveEngineFromIncludesAndImports registers imported definitions. getEngineBaseEnvVarKeys finds no engine and returns no allowlist. validateEarlyEngineSetting already skips imported engines for this reason, and #66593 defers engine.env for fallback-models.

Plan

  1. validateEarlyEnvSecrets: when frontmatterDeclaresImports, check only top-level env.
  2. validateWorkflowBuildContext: run that deferred check for imports too, on engine.env only (top-level warnings would repeat), wrapped in formatEngineSetupError (keeps the source position).
  3. Add TestCompileImportedEngineSupportedEnvVarKeysStrict.

Acceptance: the repro compiles. A key the definition does not declare still fails with the positioned error.

With the diff below the new test passes, as do the existing env-secret and fallback-model tests. Please adopt it through a core-team PR.

Related: #60858, #41465.

Diff against f7b1cc2
diff --git a/pkg/workflow/compiler_orchestrator_engine_test.go b/pkg/workflow/compiler_orchestrator_engine_test.go
index 06f4f49..3271cbb 100644
--- a/pkg/workflow/compiler_orchestrator_engine_test.go
+++ b/pkg/workflow/compiler_orchestrator_engine_test.go
@@ -1206,3 +1206,57 @@ engine: codex
 	assert.NotNil(t, result.agenticEngine)
 	assert.Equal(t, "codex", result.engineSetting)
 }
+
+// An imported engine definition registers after the early strict checks run, so its
+// supported-env-var-keys must still allow those keys to carry secrets in engine.env.
+func TestCompileImportedEngineSupportedEnvVarKeysStrict(t *testing.T) {
+	tests := []struct {
+		name      string
+		behaviors string
+		envKey    string
+		wantErr   string
+	}{
+		{name: "declared key", behaviors: "    supported-env-var-keys: [ANTHROPIC_API_KEY]\n", envKey: "ANTHROPIC_API_KEY"},
+		{name: "undeclared key", behaviors: "    supported-env-var-keys: [ANTHROPIC_API_KEY]\n", envKey: "OTHER_KEY", wantErr: "secrets detected in 'engine.env'"},
+		{name: "no declaration", envKey: "ANTHROPIC_API_KEY", wantErr: "secrets detected in 'engine.env'"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			dir := t.TempDir()
+			require.NoError(t, os.MkdirAll(filepath.Join(dir, "shared"), 0o755))
+			require.NoError(t, os.WriteFile(filepath.Join(dir, "shared", "engine.md"), []byte(`---
+engine:
+  id: harness-engine
+  display-name: Harness Engine
+  behaviors:
+    secret-strategy: universal-llm-consumer
+`+tt.behaviors+`    execution:
+      command-name: harness-engine
+      step-name: Execute Harness Engine
+---
+`), 0o600))
+			path := filepath.Join(dir, "workflow.md")
+			require.NoError(t, os.WriteFile(path, []byte(`---
+on: workflow_dispatch
+permissions:
+  contents: read
+strict: true
+imports:
+  - shared/engine.md
+engine:
+  id: harness-engine
+  model: anthropic/claude-sonnet-4
+  env:
+    `+tt.envKey+`: ${{ secrets.CUSTOM_KEY }}
+---
+Say hello.
+`), 0o600))
+			err := NewCompiler().CompileWorkflow(path)
+			if tt.wantErr == "" {
+				require.NoError(t, err)
+				return
+			}
+			require.ErrorContains(t, err, tt.wantErr)
+		})
+	}
+}
diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go
index 35a6df3..355241e 100644
--- a/pkg/workflow/compiler_orchestrator_workflow.go
+++ b/pkg/workflow/compiler_orchestrator_workflow.go
@@ -164,11 +164,13 @@ func (c *Compiler) validateWorkflowBuildContext(ctx *workflowBuildContext) error
 	if err := c.validateWorkflowEngineSettings(ctx.cleanPath, ctx.workflowData); err != nil {
 		return err
 	}
-	if cfg := ctx.workflowData.EngineConfig; cfg != nil && len(cfg.FallbackModels) > 0 {
+	// validateEarlyEnvSecrets defers engine.env in these cases: fallback provider aliases and
+	// imported engine definitions (and their supported-env-var-keys) are only known now.
+	if cfg := ctx.workflowData.EngineConfig; (cfg != nil && len(cfg.FallbackModels) > 0) || frontmatterDeclaresImports(ctx.frontmatter.Frontmatter) {
 		if err := c.withEffectiveStrictMode(ctx.frontmatter.Frontmatter, func() error {
-			return c.validateEnvSecretsWithModels(ctx.frontmatter.Frontmatter, ctx.workflowData.ModelMappings)
+			return c.validateEngineEnvSecretsWithModels(ctx.frontmatter.Frontmatter, ctx.workflowData.ModelMappings)
 		}); err != nil {
-			return err
+			return c.formatEngineSetupError(ctx, err)
 		}
 	}
 	return c.validateWorkflowToolConfigurations(ctx)
diff --git a/pkg/workflow/strict_mode_env_validation.go b/pkg/workflow/strict_mode_env_validation.go
index 88bc1c6..93aaf74 100644
--- a/pkg/workflow/strict_mode_env_validation.go
+++ b/pkg/workflow/strict_mode_env_validation.go
@@ -37,6 +37,10 @@ func (c *Compiler) validateEarlyEnvSecrets(frontmatter map[string]any) error {
 		// Provider aliases are not available until imports have been merged.
 		return c.validateEnvSecretsSection(frontmatter, "env", nil)
 	}
+	if frontmatterDeclaresImports(frontmatter) {
+		strictModeValidationLog.Print("Imports declared; deferring engine.env secret validation until imported engine definitions are registered")
+		return c.validateEnvSecretsSection(frontmatter, "env", nil)
+	}
 	return c.validateEnvSecrets(frontmatter)
 }
 
@@ -45,30 +49,28 @@ func (c *Compiler) validateEnvSecretsWithModels(frontmatter map[string]any, mode
 	if err := c.validateEnvSecretsSection(frontmatter, "env", nil); err != nil {
 		return err
 	}
+	return c.validateEngineEnvSecretsWithModels(frontmatter, models)
+}
 
-	// Check engine.env section when engine is in object format
-	if engineValue, exists := frontmatter["engine"]; exists {
-		if engineObj, ok := engineValue.(map[string]any); ok {
-			// Determine which env var keys may carry secrets: those that the engine itself
-			// requires (e.g. COPILOT_GITHUB_TOKEN for the copilot engine).
-			// The second return value is *EngineConfig (not an error); we only need the engine ID.
-			engineSetting, engineConfig, model := c.ExtractEngineConfig(frontmatter)
-			allowedEnvVarKeys := c.getEngineBaseEnvVarKeys(engineSetting)
-			if engineConfig != nil && len(engineConfig.FallbackModels) > 0 {
-				data := &WorkflowData{EngineConfig: engineConfig, Model: model, ModelMappings: models}
-				strictModeValidationLog.Printf("Adding fallback provider secret keys to engine.env allowlist for engine=%s", engineSetting)
-				for _, name := range fallbackProviderSecretNames(data) {
-					allowedEnvVarKeys[name] = struct{}{}
-				}
-			}
-
-			if err := c.validateEnvSecretsSection(engineObj, "engine.env", allowedEnvVarKeys); err != nil {
-				return err
-			}
+// validateEngineEnvSecretsWithModels checks the engine.env section when engine is in object format.
+func (c *Compiler) validateEngineEnvSecretsWithModels(frontmatter map[string]any, models map[string][]string) error {
+	engineObj, ok := frontmatter["engine"].(map[string]any)
+	if !ok {
+		return nil
+	}
+	// Determine which env var keys may carry secrets: those that the engine itself
+	// requires (e.g. COPILOT_GITHUB_TOKEN for the copilot engine).
+	// The second return value is *EngineConfig (not an error); we only need the engine ID.
+	engineSetting, engineConfig, model := c.ExtractEngineConfig(frontmatter)
+	allowedEnvVarKeys := c.getEngineBaseEnvVarKeys(engineSetting)
+	if engineConfig != nil && len(engineConfig.FallbackModels) > 0 {
+		data := &WorkflowData{EngineConfig: engineConfig, Model: model, ModelMappings: models}
+		strictModeValidationLog.Printf("Adding fallback provider secret keys to engine.env allowlist for engine=%s", engineSetting)
+		for _, name := range fallbackProviderSecretNames(data) {
+			allowedEnvVarKeys[name] = struct{}{}
 		}
 	}
-
-	return nil
+	return c.validateEnvSecretsSection(engineObj, "engine.env", allowedEnvVarKeys)
 }
 
 // getEngineBaseEnvVarKeys returns the set of env var key names that the named engine

Activity

  1. locked and limited conversation to collaborators on Nov 6, 2026
  2. unlocked this conversation on Nov 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions