diff --git a/pkg/workflow/behavior_defined_engine.go b/pkg/workflow/behavior_defined_engine.go index 3c4797a674a..f986dec16bb 100644 --- a/pkg/workflow/behavior_defined_engine.go +++ b/pkg/workflow/behavior_defined_engine.go @@ -549,10 +549,19 @@ func parseEngineDefinitionFromJSON(engineJSON string) (*EngineDefinition, error) if err := json.Unmarshal([]byte(engineJSON), &engineData); err != nil { return nil, fmt.Errorf("failed to parse engine JSON: %w", err) } - if _, ok := engineData.(map[string]any); !ok { + dataMap, ok := engineData.(map[string]any) + if !ok { return nil, nil } - yamlBytes, err := yaml.Marshal(engineData) + // EngineDefinition.Auth expects a []AuthBinding sequence. If the auth field is + // an EngineAuthConfig mapping (e.g. Anthropic/Azure WIF-style auth), strip it before + // unmarshaling to avoid "mapping was used where sequence is expected". The + // mapping-style auth is handled separately by extractEngineConfigFromJSON via + // applyEngineAuthField. + if isEngineAuthConfigMapping(dataMap["auth"]) { + delete(dataMap, "auth") + } + yamlBytes, err := yaml.Marshal(dataMap) if err != nil { return nil, fmt.Errorf("failed to convert engine JSON to yaml: %w", err) } @@ -573,6 +582,15 @@ func parseEngineDefinitionFromJSON(engineJSON string) (*EngineDefinition, error) return &def, nil } +func isEngineAuthConfigMapping(auth any) bool { + authMap, ok := auth.(map[string]any) + if !ok { + return false + } + authType, ok := authMap["type"].(string) + return ok && authType == "github-oidc" +} + // deepCopyAny returns a fully independent copy of v for values produced by // yaml.Unmarshal into interface{}. The possible concrete types are: // nil, bool, int, float64, string, []any, and map[string]any. diff --git a/pkg/workflow/engine_includes_test.go b/pkg/workflow/engine_includes_test.go index 3e0e5849a52..f5b05427a50 100644 --- a/pkg/workflow/engine_includes_test.go +++ b/pkg/workflow/engine_includes_test.go @@ -786,3 +786,122 @@ imports: assert.Contains(t, lockStr, `GH_AW_INFO_ENGINE_ID: "auggie"`, "lock file should set engine ID to the imported definition") assert.Contains(t, lockStr, "AUGMENT_SESSION_AUTH: ${{ secrets.AUGMENT_SESSION_AUTH }}", "lock file should bind custom auth secrets from engine.auth") } + +// TestImportedEngineWithAnthropicWIFAuth is a regression test for the v0.82.10 regression +// where an imported engine definition with a mapping-style auth (Anthropic/Azure WIF) caused +// "mapping was used where sequence is expected" because EngineDefinition.Auth is []AuthBinding. +// The WIF auth mapping must be stripped before EngineDefinition unmarshaling and handled via +// the EngineConfig path (applyEngineAuthField), matching the behaviour of inline engine blocks. +func TestImportedEngineWithAnthropicWIFAuth(t *testing.T) { + tmpDir := testutil.TempDir(t, "test-wif-auth-import-*") + workflowsDir := filepath.Join(tmpDir, constants.GetWorkflowDir()) + sharedDir := filepath.Join(workflowsDir, "shared") + require.NoError(t, os.MkdirAll(sharedDir, 0755)) + + sharedContent := `--- +engine: + id: claude + auth: + type: github-oidc + provider: anthropic + federation-rule-id: fr_01ABC + organization-id: org_01XYZ + service-account-id: sa_01DEF + workspace-id: ws_01GHI +--- + +# Shared Anthropic WIF engine config +` + sharedFile := filepath.Join(sharedDir, "wif-engine.md") + require.NoError(t, os.WriteFile(sharedFile, []byte(sharedContent), 0644)) + + mainContent := `--- +name: Test Imported WIF Engine +on: + workflow_dispatch: +permissions: + contents: read + id-token: write +imports: + - shared/wif-engine.md +--- + +# Test Workflow +` + mainFile := filepath.Join(workflowsDir, "test-wif.md") + require.NoError(t, os.WriteFile(mainFile, []byte(mainContent), 0644)) + + compiler := NewCompiler() + err := compiler.CompileWorkflow(mainFile) + require.NoError(t, err, "compilation must succeed for imported engine definition with Anthropic WIF auth mapping") + + lockFile := filepath.Join(workflowsDir, "test-wif.lock.yml") + lockContent, err := os.ReadFile(lockFile) + require.NoError(t, err, "lock file should be created") + + lockStr := string(lockContent) + assert.Contains(t, lockStr, "AWF_AUTH_TYPE: github-oidc", "lock file must contain WIF auth type") + assert.Contains(t, lockStr, "AWF_AUTH_PROVIDER: anthropic", "lock file must contain WIF auth provider") + assert.Contains(t, lockStr, "AWF_AUTH_ANTHROPIC_FEDERATION_RULE_ID: fr_01ABC", "lock file must contain federation rule ID") + assert.Contains(t, lockStr, "AWF_AUTH_ANTHROPIC_ORGANIZATION_ID: org_01XYZ", "lock file must contain organization ID") + assert.Contains(t, lockStr, "AWF_AUTH_ANTHROPIC_SERVICE_ACCOUNT_ID: sa_01DEF", "lock file must contain service account ID") + assert.Contains(t, lockStr, "AWF_AUTH_ANTHROPIC_WORKSPACE_ID: ws_01GHI", "lock file must contain workspace ID") +} + +func TestImportedEngineWithAzureWIFAuth(t *testing.T) { + tmpDir := testutil.TempDir(t, "test-azure-wif-auth-import-*") + workflowsDir := filepath.Join(tmpDir, constants.GetWorkflowDir()) + sharedDir := filepath.Join(workflowsDir, "shared") + require.NoError(t, os.MkdirAll(sharedDir, 0755)) + + sharedContent := `--- +engine: + id: copilot + auth: + type: github-oidc + provider: azure + audience: https://cognitiveservices.azure.com + azure-tenant-id: tenant-id + azure-client-id: client-id + azure-scope: https://cognitiveservices.azure.com/.default + azure-cloud: public +--- + +# Shared Azure WIF engine config +` + sharedFile := filepath.Join(sharedDir, "azure-wif-engine.md") + require.NoError(t, os.WriteFile(sharedFile, []byte(sharedContent), 0644)) + + mainContent := `--- +name: Test Imported Azure WIF Engine +on: + workflow_dispatch: +permissions: + contents: read + id-token: write +imports: + - shared/azure-wif-engine.md +--- + +# Test Workflow +` + mainFile := filepath.Join(workflowsDir, "test-azure-wif.md") + require.NoError(t, os.WriteFile(mainFile, []byte(mainContent), 0644)) + + compiler := NewCompiler() + err := compiler.CompileWorkflow(mainFile) + require.NoError(t, err, "compilation must succeed for imported engine definition with Azure WIF auth mapping") + + lockFile := filepath.Join(workflowsDir, "test-azure-wif.lock.yml") + lockContent, err := os.ReadFile(lockFile) + require.NoError(t, err, "lock file should be created") + + lockStr := string(lockContent) + assert.Contains(t, lockStr, "AWF_AUTH_TYPE: github-oidc", "lock file must contain WIF auth type") + assert.Contains(t, lockStr, "AWF_AUTH_PROVIDER: azure", "lock file must contain WIF auth provider") + assert.Contains(t, lockStr, "AWF_AUTH_OIDC_AUDIENCE: https://cognitiveservices.azure.com", "lock file must contain OIDC audience") + assert.Contains(t, lockStr, "AWF_AUTH_AZURE_TENANT_ID: tenant-id", "lock file must contain Azure tenant ID") + assert.Contains(t, lockStr, "AWF_AUTH_AZURE_CLIENT_ID: client-id", "lock file must contain Azure client ID") + assert.Contains(t, lockStr, "AWF_AUTH_AZURE_SCOPE: https://cognitiveservices.azure.com/.default", "lock file must contain Azure scope") + assert.Contains(t, lockStr, "AWF_AUTH_AZURE_CLOUD: public", "lock file must contain Azure cloud") +} diff --git a/pkg/workflow/imported_engine_auth_integration_test.go b/pkg/workflow/imported_engine_auth_integration_test.go new file mode 100644 index 00000000000..1278940f1d5 --- /dev/null +++ b/pkg/workflow/imported_engine_auth_integration_test.go @@ -0,0 +1,109 @@ +//go:build integration + +package workflow + +import ( + "os" + "path/filepath" + "testing" + + "github.com/github/gh-aw/pkg/constants" + "github.com/github/gh-aw/pkg/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestImportedEngineWithAnthropicWIFAuthIntegration(t *testing.T) { + tmpDir := testutil.TempDir(t, "test-imported-wif-auth-*") + workflowsDir := filepath.Join(tmpDir, constants.GetWorkflowDir()) + sharedDir := filepath.Join(workflowsDir, "shared") + require.NoError(t, os.MkdirAll(sharedDir, 0755)) + + sharedContent := `--- +engine: + id: claude + auth: + type: github-oidc + provider: anthropic + federation-rule-id: fr_01ABC + organization-id: org_01XYZ + service-account-id: sa_01DEF + workspace-id: ws_01GHI +--- + +# Shared Anthropic WIF engine config +` + sharedFile := filepath.Join(sharedDir, "wif-engine.md") + require.NoError(t, os.WriteFile(sharedFile, []byte(sharedContent), 0644)) + + mainContent := `--- +name: Test Imported WIF Engine +on: + workflow_dispatch: +permissions: + contents: read + id-token: write +imports: + - shared/wif-engine.md +--- + +# Test Workflow +` + mainFile := filepath.Join(workflowsDir, "test-wif.md") + require.NoError(t, os.WriteFile(mainFile, []byte(mainContent), 0644)) + + compiler := NewCompiler() + require.NoError(t, compiler.CompileWorkflow(mainFile)) + + lockFile := filepath.Join(workflowsDir, "test-wif.lock.yml") + lockContent, err := os.ReadFile(lockFile) + require.NoError(t, err) + + lockStr := string(lockContent) + assert.Contains(t, lockStr, "AWF_AUTH_TYPE: github-oidc") + assert.Contains(t, lockStr, "AWF_AUTH_PROVIDER: anthropic") + assert.Contains(t, lockStr, "AWF_AUTH_ANTHROPIC_FEDERATION_RULE_ID: fr_01ABC") + assert.Contains(t, lockStr, "AWF_AUTH_ANTHROPIC_ORGANIZATION_ID: org_01XYZ") + assert.Contains(t, lockStr, "AWF_AUTH_ANTHROPIC_SERVICE_ACCOUNT_ID: sa_01DEF") + assert.Contains(t, lockStr, "AWF_AUTH_ANTHROPIC_WORKSPACE_ID: ws_01GHI") +} + +func TestImportedEngineWithMalformedAuthMappingStillFailsIntegration(t *testing.T) { + tmpDir := testutil.TempDir(t, "test-imported-malformed-auth-*") + workflowsDir := filepath.Join(tmpDir, constants.GetWorkflowDir()) + sharedDir := filepath.Join(workflowsDir, "shared") + require.NoError(t, os.MkdirAll(sharedDir, 0755)) + + sharedContent := `--- +engine: + id: claude + auth: + role: session + secret: ANTHROPIC_API_KEY +--- + +# Shared malformed auth config +` + sharedFile := filepath.Join(sharedDir, "bad-auth-engine.md") + require.NoError(t, os.WriteFile(sharedFile, []byte(sharedContent), 0644)) + + mainContent := `--- +name: Test Imported Invalid Engine Auth +on: + workflow_dispatch: +permissions: + contents: read +imports: + - shared/bad-auth-engine.md +--- + +# Test Workflow +` + mainFile := filepath.Join(workflowsDir, "test-invalid-auth.md") + require.NoError(t, os.WriteFile(mainFile, []byte(mainContent), 0644)) + + compiler := NewCompiler() + err := compiler.CompileWorkflow(mainFile) + require.Error(t, err) + assert.Contains(t, err.Error(), "mapping was used where sequence is expected") +}