Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
22 changes: 22 additions & 0 deletions pkg/cli/codemod_engine_env_secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,28 @@ func allowedEngineEnvSecretKeys(engineID string) map[string]bool {
) {
allowed[req.Name] = true
}
// Also include all secrets returned by the engine's GetRequiredSecretNames so that
// BYOK credentials (e.g. COPILOT_PROVIDER_API_KEY) are treated the same way as they
// are during compile-time strict-mode validation and are not removed by this codemod.
if engineID != "" {
registry := workflow.GetGlobalEngineRegistry()
if engine, err := registry.GetEngine(engineID); err == nil {
// Use a minimal WorkflowData so we get only the engine's unconditional secrets.
// GetRequiredSecretNames only adds extra secrets when non-nil MCP tools
// (ParsedTools.GitHub, ParsedTools.Playwright, etc.) are set, or when
// MCPScripts is populated. By passing empty Tools/ParsedTools we get just the
// base engine secrets without any optional/conditional ones.
minimalData := &workflow.WorkflowData{
Tools: map[string]any{},
ParsedTools: &workflow.ToolsConfig{},
}
for _, name := range engine.GetRequiredSecretNames(minimalData) {
allowed[name] = true
}
} else {
engineEnvSecretsCodemodLog.Printf("Could not look up engine '%s' for allowlist expansion: %v", engineID, err)
}
}
return allowed
}

Expand Down
56 changes: 56 additions & 0 deletions pkg/cli/codemod_engine_env_secrets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,62 @@ engine:
assert.Equal(t, content, result, "content should be unchanged")
})

t.Run("no-op when COPILOT_PROVIDER_API_KEY is used (BYOK)", func(t *testing.T) {
content := `---
on: workflow_dispatch
engine:
id: copilot
env:
COPILOT_PROVIDER_BASE_URL: ${{ vars.PROVIDER_BASE_URL }}
COPILOT_MODEL: ${{ vars.PROVIDER_MODEL }}
COPILOT_PROVIDER_API_KEY: ${{ secrets.PROVIDER_API_KEY }}
---
`
frontmatter := map[string]any{
"on": "workflow_dispatch",
"engine": map[string]any{
"id": "copilot",
"env": map[string]any{
"COPILOT_PROVIDER_BASE_URL": "${{ vars.PROVIDER_BASE_URL }}",
"COPILOT_MODEL": "${{ vars.PROVIDER_MODEL }}",
"COPILOT_PROVIDER_API_KEY": "${{ secrets.PROVIDER_API_KEY }}",
},
},
}

result, applied, err := codemod.Apply(content, frontmatter)
require.NoError(t, err, "codemod should not error for BYOK pattern")
assert.False(t, applied, "codemod should not apply: COPILOT_PROVIDER_API_KEY is a BYOK credential")
assert.Equal(t, content, result, "content should be unchanged")
})

t.Run("no-op when COPILOT_PROVIDER_BEARER_TOKEN is used (BYOK)", func(t *testing.T) {
content := `---
on: workflow_dispatch
engine:
id: copilot
env:
COPILOT_PROVIDER_BASE_URL: ${{ vars.PROVIDER_BASE_URL }}
COPILOT_PROVIDER_BEARER_TOKEN: ${{ secrets.PROVIDER_BEARER_TOKEN }}
---
`
frontmatter := map[string]any{
"on": "workflow_dispatch",
"engine": map[string]any{
"id": "copilot",
"env": map[string]any{
"COPILOT_PROVIDER_BASE_URL": "${{ vars.PROVIDER_BASE_URL }}",
"COPILOT_PROVIDER_BEARER_TOKEN": "${{ secrets.PROVIDER_BEARER_TOKEN }}",
},
},
}

result, applied, err := codemod.Apply(content, frontmatter)
require.NoError(t, err, "codemod should not error for BYOK bearer token pattern")
assert.False(t, applied, "codemod should not apply: COPILOT_PROVIDER_BEARER_TOKEN is a BYOK credential")
assert.Equal(t, content, result, "content should be unchanged")
})

t.Run("supports inline engine runtime.id for allowlist", func(t *testing.T) {
content := `---
on: workflow_dispatch
Expand Down
1 change: 0 additions & 1 deletion pkg/workflow/observability_otlp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,6 @@ func TestInjectOTLPConfig_MapHeaders(t *testing.T) {
})
}


// correctly parsed by ParseFrontmatterConfig.
func TestObservabilityConfigParsing_MapHeaders(t *testing.T) {
t.Run("map headers parsed as any type", func(t *testing.T) {
Expand Down