Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions pkg/workflow/claude_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,22 @@ func (e *ClaudeEngine) GetAPMTarget() string {
return "claude"
}

// GetRequiredSecretNames returns the list of secrets required by the Claude engine
// This includes ANTHROPIC_API_KEY and optionally MCP_GATEWAY_API_KEY and mcp-scripts secrets
// GetRequiredSecretNames returns the list of secrets required by the Claude engine.
// When Anthropic WIF (github-oidc + provider=anthropic) is configured, no static API key
// is needed and only common MCP secrets are returned.
func (e *ClaudeEngine) GetRequiredSecretNames(workflowData *WorkflowData) []string {
if isAnthropicWIF(workflowData) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No validation that required Anthropic WIF fields are non-empty when WIF is active — runtime failure will be cryptic.

When isAnthropicWIF returns true, the compiler silently skips secret validation and emits env vars. But all four Anthropic WIF fields (federation-rule-id, organization-id, service-account-id, workspace-id) are optional at parse time. A workflow with only type: github-oidc + provider: anthropic and none of the other fields will compile cleanly but fail at runtime in the OIDC token exchange with no useful error message traced back to frontmatter.

💡 Suggested fix

Add a compile-time check in GetSecretValidationStep (or a new Validate() method called from the compiler) that returns an error if WIF is active but the mandatory identity fields are missing:

func validateAnthropicWIF(workflowData *WorkflowData) error {
    auth := workflowData.EngineConfig.Auth
    var missing []string
    if auth.AnthropicFederationRuleID == "" {
        missing = append(missing, "federation-rule-id")
    }
    if auth.AnthropicOrganizationID == "" {
        missing = append(missing, "organization-id")
    }
    if auth.AnthropicServiceAccountID == "" {
        missing = append(missing, "service-account-id")
    }
    if len(missing) > 0 {
        return fmt.Errorf("engine.auth provider=anthropic requires: %s", strings.Join(missing, ", "))
    }
    return nil
}

return collectCommonMCPSecrets(workflowData)
}
return append([]string{"ANTHROPIC_API_KEY"}, collectCommonMCPSecrets(workflowData)...)
}

// GetSecretValidationStep returns the secret validation step for the Claude engine.
// Returns an empty step if custom command is specified.
// Returns an empty step if custom command is specified or if Anthropic WIF is configured.
func (e *ClaudeEngine) GetSecretValidationStep(workflowData *WorkflowData) GitHubActionStep {
if isAnthropicWIF(workflowData) {
return GitHubActionStep{}
}
return BuildDefaultSecretValidationStep(
workflowData,
[]string{"ANTHROPIC_API_KEY"},
Expand All @@ -67,6 +74,16 @@ func (e *ClaudeEngine) GetSecretValidationStep(workflowData *WorkflowData) GitHu
)
}

// isAnthropicWIF returns true when the workflow is configured to use Anthropic
// Workload Identity Federation (github-oidc auth type with provider=anthropic).
func isAnthropicWIF(workflowData *WorkflowData) bool {
if workflowData == nil || workflowData.EngineConfig == nil || workflowData.EngineConfig.Auth == nil {
return false
}
auth := workflowData.EngineConfig.Auth
return auth.Type == "github-oidc" && auth.Provider == "anthropic"
}

func (e *ClaudeEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHubActionStep {
claudeLog.Printf("Generating installation steps for Claude engine: workflow=%s", workflowData.Name)

Expand Down
69 changes: 69 additions & 0 deletions pkg/workflow/claude_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -932,3 +932,72 @@ func TestClaudeEngineWithExpressionVersion(t *testing.T) {
t.Errorf("Expression should NOT be embedded directly in npm install command, got:\n%s", installStep)
}
}

func TestIsAnthropicWIF(t *testing.T) {
tests := []struct {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] Test body is entirely unindented — go fmt will reject this and fail CI.

Every line inside TestIsAnthropicWIF is missing its leading tab. Run make fmt to fix.

💡 Expected shape after formatting
func TestIsAnthropicWIF(t *testing.T) {
	tests := []struct {
		name         string
		workflowData *WorkflowData
		want         bool
	}{
		// ...
	}
	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			got := isAnthropicWIF(tt.workflowData)
			assert.Equal(t, tt.want, got)
		})
	}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TestIsAnthropicWIF body is entirely un-indented — make fmt will fail and block CI.

Every line inside the function (struct definition, table entries, loop) is at column 0 instead of being tab-indented. gofmt requires consistent indentation; this causes the make fmt check to fail immediately.

💡 Suggested fix

Run gofmt -w pkg/workflow/claude_engine_test.go (or make fmt) to auto-fix the indentation. The test logic itself is correct — it just wasn't formatted before committing.

Example of what the first few lines should look like after formatting:

func TestIsAnthropicWIF(t *testing.T) {
	tests := []struct {
		name         string
		workflowData *WorkflowData
		want         bool
	}{
		{
			name:         "nil workflowData returns false",
			workflowData: nil,
			want:         false,
		},

name string
workflowData *WorkflowData
want bool
}{
{
name: "nil workflowData returns false",
workflowData: nil,
want: false,
},
{
name: "nil EngineConfig returns false",
workflowData: &WorkflowData{},
want: false,
},
{
name: "nil Auth returns false",
workflowData: &WorkflowData{
EngineConfig: &EngineConfig{},
},
want: false,
},
{
name: "github-oidc without provider returns false",
workflowData: &WorkflowData{
EngineConfig: &EngineConfig{
Auth: &EngineAuthConfig{Type: "github-oidc"},
},
},
want: false,
},
{
name: "github-oidc with azure provider returns false",
workflowData: &WorkflowData{
EngineConfig: &EngineConfig{
Auth: &EngineAuthConfig{Type: "github-oidc", Provider: "azure"},
},
},
want: false,
},
{
name: "static type with anthropic provider returns false",
workflowData: &WorkflowData{
EngineConfig: &EngineConfig{
Auth: &EngineAuthConfig{Type: "static", Provider: "anthropic"},
},
},
want: false,
},
{
name: "github-oidc with anthropic provider returns true",
workflowData: &WorkflowData{
EngineConfig: &EngineConfig{
Auth: &EngineAuthConfig{Type: "github-oidc", Provider: "anthropic"},
},
},
want: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := isAnthropicWIF(tt.workflowData)
assert.Equal(t, tt.want, got)
})
}
}
51 changes: 49 additions & 2 deletions pkg/workflow/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,19 @@ type EngineConfig struct {
// EngineAuthConfig represents engine.auth frontmatter settings that map to
// AWF_AUTH_* environment variables consumed by the AWF API proxy sidecar.
type EngineAuthConfig struct {
Type string
Audience string
Type string
Audience string
Provider string // "azure" or "anthropic"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] Provider is an unvalidated free-form string — a typo like "anthropic2" emits AWF_AUTH_PROVIDER=anthropic2 without any warning, and isAnthropicWIF silently falls back to requiring ANTHROPIC_API_KEY, which may confuse the user.

Consider validating Provider against a known set ("azure", "anthropic") in parseEngineAuthConfig and surfacing a compiler warning (or error) for unknown values, consistent with how other enum-like fields are handled in the codebase.

// Azure WIF fields
AzureTenantID string
AzureClientID string
AzureScope string
AzureCloud string
// Anthropic WIF fields
AnthropicFederationRuleID string
AnthropicOrganizationID string
AnthropicServiceAccountID string
AnthropicWorkspaceID string
}

// NetworkPermissions represents network access permissions for workflow execution
Expand Down Expand Up @@ -607,6 +614,9 @@ func parseEngineAuthConfig(authObj map[string]any) *EngineAuthConfig {
if s, ok := authObj["audience"].(string); ok {
auth.Audience = s
}
if s, ok := authObj["provider"].(string); ok {
auth.Provider = s
}
if s, ok := authObj["azure-tenant-id"].(string); ok {
auth.AzureTenantID = s
}
Expand All @@ -619,6 +629,18 @@ func parseEngineAuthConfig(authObj map[string]any) *EngineAuthConfig {
if s, ok := authObj["azure-cloud"].(string); ok {
auth.AzureCloud = s
}
if s, ok := authObj["federation-rule-id"].(string); ok {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/grill-with-docs] The frontmatter keys organization-id, service-account-id, and workspace-id are generic and not namespaced to Anthropic. A future provider with similar concepts would collide.

Consider prefixing them (anthropic-organization-id, anthropic-federation-rule-id, etc.) or nesting them under a provider-specific sub-key, consistent with how Azure fields use the azure- prefix throughout this file.

💡 Contrast with Azure naming

Existing Azure keys: azure-tenant-id, azure-client-id, azure-scope, azure-cloud — all namespaced.

New Anthropic keys: federation-rule-id, organization-id, service-account-id, workspace-id — un-namespaced.

Applying the same convention:

engine-auth:
  type: github-oidc
  provider: anthropic
  anthropic-federation-rule-id: fr_01ABC
  anthropic-organization-id: org_01XYZ
  anthropic-service-account-id: sa_01DEF
  anthropic-workspace-id: ws_01GHI

auth.AnthropicFederationRuleID = s
}
if s, ok := authObj["organization-id"].(string); ok {
auth.AnthropicOrganizationID = s
}
if s, ok := authObj["service-account-id"].(string); ok {
auth.AnthropicServiceAccountID = s
}
if s, ok := authObj["workspace-id"].(string); ok {
auth.AnthropicWorkspaceID = s
}
return auth
}

Expand Down Expand Up @@ -663,6 +685,31 @@ func applyEngineAuthEnv(config *EngineConfig) {
config.Env["AWF_AUTH_AZURE_CLOUD"] = config.Auth.AzureCloud
}
}
if config.Auth.Provider != "" {
if _, exists := config.Env["AWF_AUTH_PROVIDER"]; !exists {
config.Env["AWF_AUTH_PROVIDER"] = config.Auth.Provider
}
}
if config.Auth.AnthropicFederationRuleID != "" {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] Anthropic env vars are emitted independently of Provider — they'll appear even when provider: azure if someone accidentally provides an organization-id in the frontmatter.

Wrap all four AWF_AUTH_ANTHROPIC_* emissions inside the provider == "anthropic" guard (or a dedicated if isAnthropicProvider block) to prevent cross-provider env var leakage.

💡 Suggested fix
if config.Auth.Provider == "anthropic" {
    if config.Auth.AnthropicFederationRuleID != "" {
        if _, exists := config.Env["AWF_AUTH_ANTHROPIC_FEDERATION_RULE_ID"]; !exists {
            config.Env["AWF_AUTH_ANTHROPIC_FEDERATION_RULE_ID"] = config.Auth.AnthropicFederationRuleID
        }
    }
    // ... same for other Anthropic fields
}

A test demonstrating provider: azure does not emit any AWF_AUTH_ANTHROPIC_* vars would lock this in.

if _, exists := config.Env["AWF_AUTH_ANTHROPIC_FEDERATION_RULE_ID"]; !exists {
config.Env["AWF_AUTH_ANTHROPIC_FEDERATION_RULE_ID"] = config.Auth.AnthropicFederationRuleID
}
}
if config.Auth.AnthropicOrganizationID != "" {
if _, exists := config.Env["AWF_AUTH_ANTHROPIC_ORGANIZATION_ID"]; !exists {
config.Env["AWF_AUTH_ANTHROPIC_ORGANIZATION_ID"] = config.Auth.AnthropicOrganizationID
}
}
if config.Auth.AnthropicServiceAccountID != "" {
if _, exists := config.Env["AWF_AUTH_ANTHROPIC_SERVICE_ACCOUNT_ID"]; !exists {
config.Env["AWF_AUTH_ANTHROPIC_SERVICE_ACCOUNT_ID"] = config.Auth.AnthropicServiceAccountID
}
}
if config.Auth.AnthropicWorkspaceID != "" {
if _, exists := config.Env["AWF_AUTH_ANTHROPIC_WORKSPACE_ID"]; !exists {
config.Env["AWF_AUTH_ANTHROPIC_WORKSPACE_ID"] = config.Auth.AnthropicWorkspaceID
}
}
}

// parseRequestShape converts a raw request config map (from engine.provider.request) into
Expand Down
34 changes: 34 additions & 0 deletions pkg/workflow/engine_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,40 @@ func TestExtractEngineConfig_EngineEnvTakesPrecedenceOverEngineAuth(t *testing.T
assert.Equal(t, "from-engine-env", config.Env["AWF_AUTH_OIDC_AUDIENCE"])
}

func TestExtractEngineConfig_AnthropicWIFMapsToAWFEnv(t *testing.T) {
compiler := NewCompiler()
_, config := compiler.ExtractEngineConfig(map[string]any{
"engine": map[string]any{
"id": "claude",
"auth": map[string]any{
"type": "github-oidc",
"provider": "anthropic",
"federation-rule-id": "fr_01ABC",
"organization-id": "org_01XYZ",
"service-account-id": "sa_01DEF",
"workspace-id": "ws_01GHI",
},
},
})

assert.NotNil(t, config)
if assert.NotNil(t, config.Auth) {
assert.Equal(t, "github-oidc", config.Auth.Type)
assert.Equal(t, "anthropic", config.Auth.Provider)
assert.Equal(t, "fr_01ABC", config.Auth.AnthropicFederationRuleID)
assert.Equal(t, "org_01XYZ", config.Auth.AnthropicOrganizationID)
assert.Equal(t, "sa_01DEF", config.Auth.AnthropicServiceAccountID)
assert.Equal(t, "ws_01GHI", config.Auth.AnthropicWorkspaceID)
}

assert.Equal(t, "github-oidc", config.Env["AWF_AUTH_TYPE"])
assert.Equal(t, "anthropic", config.Env["AWF_AUTH_PROVIDER"])
assert.Equal(t, "fr_01ABC", config.Env["AWF_AUTH_ANTHROPIC_FEDERATION_RULE_ID"])
assert.Equal(t, "org_01XYZ", config.Env["AWF_AUTH_ANTHROPIC_ORGANIZATION_ID"])
assert.Equal(t, "sa_01DEF", config.Env["AWF_AUTH_ANTHROPIC_SERVICE_ACCOUNT_ID"])
assert.Equal(t, "ws_01GHI", config.Env["AWF_AUTH_ANTHROPIC_WORKSPACE_ID"])
}

func TestCompileWorkflowWithExtendedEngine(t *testing.T) {
// Create temporary directory for test files
tmpDir := testutil.TempDir(t, "extended-engine-test")
Expand Down
39 changes: 39 additions & 0 deletions pkg/workflow/engine_helpers_secrets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,45 @@ func TestGetRequiredSecretNames_Claude(t *testing.T) {
assert.Contains(t, secrets, "ANTHROPIC_API_KEY")
assert.Contains(t, secrets, "MCP_GATEWAY_API_KEY")
})

t.Run("WIF: skips ANTHROPIC_API_KEY when github-oidc+anthropic configured", func(t *testing.T) {
workflowData := &WorkflowData{
Tools: map[string]any{},
ParsedTools: &ToolsConfig{},
EngineConfig: &EngineConfig{
Auth: &EngineAuthConfig{
Type: "github-oidc",
Provider: "anthropic",
},
},
}

secrets := engine.GetRequiredSecretNames(workflowData)

assert.NotContains(t, secrets, "ANTHROPIC_API_KEY")
})

t.Run("WIF: still includes MCP secrets when WIF and MCP both configured", func(t *testing.T) {
workflowData := &WorkflowData{
Tools: map[string]any{
"github": map[string]any{},
},
ParsedTools: &ToolsConfig{
GitHub: &GitHubToolConfig{},
},
EngineConfig: &EngineConfig{
Auth: &EngineAuthConfig{
Type: "github-oidc",
Provider: "anthropic",
},
},
}

secrets := engine.GetRequiredSecretNames(workflowData)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] Missing edge case: WIF active but no WIF fields populated (only type: github-oidc + provider: anthropic, none of the four IDs set). In this state the compiler skips ANTHROPIC_API_KEY and emits no AWF_AUTH_ANTHROPIC_* vars — the firewall will likely reject the request at runtime with a confusing error.

A test asserting either (a) this configuration is rejected at compile time, or (b) the expected runtime behaviour is explicitly documented, would prevent a silent footgun for users migrating to WIF.

assert.NotContains(t, secrets, "ANTHROPIC_API_KEY")
assert.Contains(t, secrets, "MCP_GATEWAY_API_KEY")
})
}

// TestGetRequiredSecretNames_Codex tests CodexEngine.GetRequiredSecretNames
Expand Down
17 changes: 17 additions & 0 deletions pkg/workflow/secret_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,23 @@ func TestClaudeEngineHasSecretValidation(t *testing.T) {
}
}

func TestClaudeEngineWIFSkipsSecretValidation(t *testing.T) {
engine := NewClaudeEngine()
workflowData := &WorkflowData{
EngineConfig: &EngineConfig{
Auth: &EngineAuthConfig{
Type: "github-oidc",
Provider: "anthropic",
},
},
}

step := engine.GetSecretValidationStep(workflowData)
if len(step) != 0 {
t.Errorf("Expected empty secret validation step for Anthropic WIF, got %d entries", len(step))
}
}

func TestCopilotEngineHasSecretValidation(t *testing.T) {
engine := NewCopilotEngine()
workflowData := &WorkflowData{}
Expand Down
Loading