feat: Anthropic WIF support in EngineAuthConfig and ClaudeEngine#35939
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
🧪 Test Quality Sentinel completed test quality analysis. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. |
There was a problem hiding this comment.
Pull request overview
Adds compiler/runtime support for Anthropic Workload Identity Federation (WIF) by extending engine auth configuration, emitting the required AWF auth environment variables, and teaching the Claude engine to skip static ANTHROPIC_API_KEY secret validation when WIF is configured.
Changes:
- Extend
EngineAuthConfigparsing and env emission to support an Anthropic auth provider and Anthropic WIF identifiers. - Update
ClaudeEnginesecret requirements + validation step generation to no-op whengithub-oidc+provider=anthropicis configured. - Add unit tests covering Anthropic WIF env mapping and the Claude WIF secret-validation skip behavior.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/engine.go | Adds Provider and Anthropic WIF fields; parses new frontmatter keys; emits AWF_AUTH_PROVIDER and AWF_AUTH_ANTHROPIC_* env vars. |
| pkg/workflow/claude_engine.go | Skips ANTHROPIC_API_KEY requirements and secret validation step when Anthropic WIF is active; adds isAnthropicWIF helper. |
| pkg/workflow/engine_config_test.go | Tests Anthropic WIF frontmatter mapping into EngineAuthConfig and emitted AWF env vars. |
| pkg/workflow/engine_helpers_secrets_test.go | Tests GetRequiredSecretNames behavior for Claude under Anthropic WIF (including MCP secrets). |
| pkg/workflow/secret_validation_test.go | Tests that Claude secret validation step is empty under Anthropic WIF. |
| pkg/workflow/claude_engine_test.go | Adds tests for isAnthropicWIF helper behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 6/6 changed files
- Comments generated: 1
| func TestIsAnthropicWIF(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| workflowData *WorkflowData | ||
| want bool | ||
| }{ |
|
We should really split engine from providers. Let's have: providers: This will allow to mix and match engines and providers. |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (228 new lines under 📄 Draft ADR committed:
📋 What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. ❓ Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /tdd and /grill-with-docs — requesting changes on one blocking issue (test formatting) and flagging three important correctness/design concerns.
📋 Key Themes & Highlights
Blocking
- Test formatting:
TestIsAnthropicWIFbody has no indentation —go fmt/make fmtwill fail CI immediately.
Correctness
- Anthropic env vars not gated on
provider: The fourAWF_AUTH_ANTHROPIC_*vars are emitted based solely on field presence, not onprovider == "anthropic". A misconfigured Azure+Anthropic-field workflow would silently leak Anthropic vars into the environment.
Design
- Generic frontmatter key names:
organization-id,service-account-id, etc. break the<provider>-prefix convention established by the Azure fields (azure-tenant-id, etc.). This will cause collisions when a future provider needs similar concepts. - Unvalidated
Providerstring: A typo silently emits a badAWF_AUTH_PROVIDERand quietly falls back to API key mode — consider validating against the known set. - Partial WIF edge case untested:
provider: anthropicwith no IDs skips API key validation but emits zero WIF vars; the firewall will reject this at runtime with no compiler guidance.
Positive Highlights
- ✅ Clean nil-guard chain in
isAnthropicWIF— all three nil paths are covered. - ✅ Good use of a dedicated helper rather than inlining the WIF check in each method.
- ✅ Thorough test coverage for the happy path and all
isAnthropicWIFboolean combinations. - ✅ PR description is exemplary — example frontmatter, env var list, and firewall-side context all in one place.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 1.8M
| } | ||
|
|
||
| func TestIsAnthropicWIF(t *testing.T) { | ||
| tests := []struct { |
There was a problem hiding this comment.
[/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)
})
}
}| config.Env["AWF_AUTH_PROVIDER"] = config.Auth.Provider | ||
| } | ||
| } | ||
| if config.Auth.AnthropicFederationRuleID != "" { |
There was a problem hiding this comment.
[/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 s, ok := authObj["azure-cloud"].(string); ok { | ||
| auth.AzureCloud = s | ||
| } | ||
| if s, ok := authObj["federation-rule-id"].(string); ok { |
There was a problem hiding this comment.
[/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| Audience string | ||
| Type string | ||
| Audience string | ||
| Provider string // "azure" or "anthropic" |
There was a problem hiding this comment.
[/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.
| } | ||
|
|
||
| secrets := engine.GetRequiredSecretNames(workflowData) | ||
|
|
There was a problem hiding this comment.
[/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.
There was a problem hiding this comment.
Two blockers must be fixed before merge
1. Schema not updated (pkg/parser/schemas/main_workflow_schema.json) — the engine.auth object schema has "additionalProperties": false and none of the five new Anthropic WIF fields are declared. Every workflow that uses Anthropic WIF will fail at gh aw compile time with a schema validation error. The Go-level parsing code is fine; the schema is the missing piece.
2. TestIsAnthropicWIF is un-indented (pkg/workflow/claude_engine_test.go) — the entire test body sits at column 0. make fmt (enforced by CI) rejects this immediately.
Other findings (non-blocking)
Missing compile-time validation of Anthropic WIF identity fields — when isAnthropicWIF is true the compiler skips secret validation but doesn't check that federation-rule-id, organization-id, service-account-id are non-empty. A partial configuration will compile cleanly and produce a cryptic runtime failure during OIDC token exchange. See inline comment.
provider string is unvalidated — an unknown value (e.g. provider: Anthropic) silently falls through to the non-WIF path and demands ANTHROPIC_API_KEY, with no user-visible error. A schema enum on the new provider field (fix #1 above) covers this automatically.
🔎 Code quality review by PR Code Quality Reviewer · sonnet46 2.6M
| } | ||
|
|
||
| func TestIsAnthropicWIF(t *testing.T) { | ||
| tests := []struct { |
There was a problem hiding this comment.
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,
},| // 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) { |
There was a problem hiding this comment.
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
}
🧪 Test Quality Sentinel Report✅ Test Quality Score: 80/100 — Excellent
📊 Metrics & Test Classification (5 tests analyzed)
Test Classification Details
Language SupportTests analyzed:
|
There was a problem hiding this comment.
✅ Test Quality Sentinel: 80/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). All 5 new tests enforce real behavioral contracts for the Anthropic WIF feature. Minor non-blocking warnings: missing assertion messages in 4 tests and a formatting issue in TestIsAnthropicWIF.
|
|
|
@copilot review all comments and reviews from copilot and github-actions actor. lint go. |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed and pushed in |
|
|
The compiler had no support for Anthropic Workload Identity Federation (WIF):
EngineAuthConfigwas Azure-only, andClaudeEngineunconditionally requiredANTHROPIC_API_KEY— causing WIF-configured workflows to fail secret validation before the agent runs. The firewall side already supports Anthropic WIF; this closes the compiler gap.Changes
EngineAuthConfig— new fields (pkg/workflow/engine.go)Providerstring ("azure"|"anthropic")AnthropicFederationRuleID,AnthropicOrganizationID,AnthropicServiceAccountID,AnthropicWorkspaceIDparseEngineAuthConfig— parse Anthropic frontmatterReads
provider,federation-rule-id,organization-id,service-account-id,workspace-idfrom theengine.authmap.applyEngineAuthEnv— emit Anthropic env varsWhen Anthropic WIF fields are set, emits
AWF_AUTH_PROVIDERand the fourAWF_AUTH_ANTHROPIC_*vars consumed by the firewall api-proxy.ClaudeEngine— branch on WIF auth (pkg/workflow/claude_engine.go)GetRequiredSecretNames: skipsANTHROPIC_API_KEYwhen WIF is active; MCP secrets still collected normallyGetSecretValidationStep: returns empty step (no-op) when WIF is activeisAnthropicWIFhelper:auth.Type == "github-oidc" && auth.Provider == "anthropic"Example frontmatter
With this config the compiler emits
AWF_AUTH_TYPE,AWF_AUTH_PROVIDER, and all fourAWF_AUTH_ANTHROPIC_*env vars, and skips theANTHROPIC_API_KEYsecret validation step entirely.