feat: spec-driven engine.env allowlist via GetSupportedEnvVarKeys; remove strict: false from smoke workflows#41465
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…ove strict: false from smoke workflows Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
| // all BYOK provider variables that may carry secret values. | ||
| func (e *CopilotEngine) GetSupportedEnvVarKeys() []string { | ||
| return []string{ | ||
| "COPILOT_GITHUB_TOKEN", |
There was a problem hiding this comment.
Done — added constants.CopilotGitHubToken = "COPILOT_GITHUB_TOKEN" to pkg/constants/engine_constants.go and updated GetSupportedEnvVarKeys (and the EngineOptions table) to use it. Commit: feat: extract API key string literals into named constants
| // supports as defined in the AWF specification. | ||
| func (e *CodexEngine) GetSupportedEnvVarKeys() []string { | ||
| return []string{ | ||
| "CODEX_API_KEY", |
There was a problem hiding this comment.
Done — added constants.CodexAPIKey = "CODEX_API_KEY" and constants.OpenAIAPIKey = "OPENAI_API_KEY" to pkg/constants/engine_constants.go and updated GetSupportedEnvVarKeys (and the EngineOptions table) to use them.
| // supports as defined in the AWF specification. | ||
| func (e *ClaudeEngine) GetSupportedEnvVarKeys() []string { | ||
| return []string{ | ||
| "ANTHROPIC_API_KEY", |
There was a problem hiding this comment.
Done — added constants.AnthropicAPIKey = "ANTHROPIC_API_KEY" to pkg/constants/engine_constants.go and updated GetSupportedEnvVarKeys (and the EngineOptions table) to use it.
Add CopilotGitHubToken, AnthropicAPIKey, CodexAPIKey, OpenAIAPIKey, GeminiAPIKey, and AntigravityAPIKey constants to engine_constants.go. Replace string literals in copilot_engine.go, claude_engine.go, codex_engine.go, and the EngineOptions table in engine_constants.go. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot merge main and recompile |
There was a problem hiding this comment.
Pull request overview
This PR makes strict-mode engine.env secret allowlisting spec-driven by introducing SecurityProvider.GetSupportedEnvVarKeys() (instead of deriving an allowlist from GetRequiredSecretNames with minimal workflow data), and then removes strict: false workarounds from the smoke workflows now that strict mode validates cleanly.
Changes:
- Added
GetSupportedEnvVarKeys() []stringtoSecurityProvider(with aBaseEnginedefault) and implemented it across all engines; strict-mode env secret validation now uses this method. - Centralized primary engine secret names into constants and added
CopilotProviderWireAPIto the Copilot BYOK env var constants. - Removed
strict: falsefrom smoke workflows and regenerated their.lock.ymloutputs with strict enabled.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/strict_mode_env_validation.go | Switch env-secret allowlist source to engine-declared supported env var keys. |
| pkg/workflow/agentic_engine.go | Extend SecurityProvider with GetSupportedEnvVarKeys and add a BaseEngine default. |
| pkg/workflow/copilot_engine.go | Implement Copilot supported env var keys (including BYOK vars). |
| pkg/workflow/claude_engine.go | Implement Claude supported env var keys. |
| pkg/workflow/codex_engine.go | Implement Codex supported env var keys. |
| pkg/workflow/gemini_engine.go | Implement Gemini supported env var keys. |
| pkg/workflow/antigravity_engine.go | Implement Antigravity supported env var keys. |
| pkg/workflow/opencode_engine.go | Implement OpenCode supported env var keys. |
| pkg/workflow/crush_engine.go | Implement Crush supported env var keys. |
| pkg/workflow/pi_engine.go | Implement Pi supported env var keys. |
| pkg/constants/engine_constants.go | Add/export canonical secret-name constants; add CopilotProviderWireAPI. |
| .github/workflows/smoke-claude.md | Remove strict: false from smoke workflow frontmatter. |
| .github/workflows/smoke-claude.lock.yml | Regenerate compiled workflow with strict enabled metadata/env. |
| .github/workflows/smoke-codex.md | Remove strict: false from smoke workflow frontmatter. |
| .github/workflows/smoke-codex.lock.yml | Regenerate compiled workflow with strict enabled metadata/env. |
| .github/workflows/smoke-copilot.md | Remove strict: false from smoke workflow frontmatter. |
| .github/workflows/smoke-copilot.lock.yml | Regenerate compiled workflow with strict enabled metadata/env. |
| .github/workflows/smoke-copilot-arm.md | Remove strict: false from smoke workflow frontmatter. |
| .github/workflows/smoke-copilot-arm.lock.yml | Regenerate compiled workflow with strict enabled metadata/env. |
| .github/workflows/smoke-copilot-aoai-apikey.md | Remove strict: false from smoke workflow frontmatter. |
| .github/workflows/smoke-copilot-aoai-apikey.lock.yml | Regenerate compiled workflow with strict enabled metadata/env. |
| .github/workflows/smoke-copilot-aoai-entra.md | Remove strict: false from smoke workflow frontmatter. |
| .github/workflows/smoke-copilot-aoai-entra.lock.yml | Regenerate compiled workflow with strict enabled metadata/env. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 23/23 changed files
- Comments generated: 6
| return []string{ | ||
| "COPILOT_GITHUB_TOKEN", | ||
| "ANTHROPIC_API_KEY", | ||
| "CODEX_API_KEY", | ||
| "OPENAI_API_KEY", | ||
| } | ||
| } |
| return []string{ | ||
| "COPILOT_GITHUB_TOKEN", | ||
| "ANTHROPIC_API_KEY", | ||
| "CODEX_API_KEY", | ||
| "OPENAI_API_KEY", | ||
| } | ||
| } |
| return []string{ | ||
| "COPILOT_GITHUB_TOKEN", | ||
| "ANTHROPIC_API_KEY", | ||
| "CODEX_API_KEY", | ||
| "OPENAI_API_KEY", | ||
| } | ||
| } |
| func (e *CopilotEngine) GetSupportedEnvVarKeys() []string { | ||
| return []string{ | ||
| constants.CopilotGitHubToken, | ||
| constants.CopilotProviderBaseURL, | ||
| constants.CopilotProviderAPIKey, |
…-false-smoke-workflows # Conflicts: # .github/workflows/smoke-codex.lock.yml # .github/workflows/smoke-copilot-aoai-entra.lock.yml # .github/workflows/smoke-copilot.lock.yml Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done — merged |
|
@copilot run pr-finisher skill |
…es; add WIRE_API test Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done. Addressed all |
|
``
|
The engine.env secret allowlist in strict-mode validation was derived dynamically by calling
GetRequiredSecretNameswith a minimalWorkflowData, which is fragile and conflates "runtime-required secrets" with "spec-supported env var keys". Smoke workflows hadstrict: falseas a workaround that's no longer needed.New interface method:
GetSupportedEnvVarKeys() []stringAdded to
SecurityProvider; replaces theGetRequiredSecretNames(minimalData)hack ingetEngineBaseEnvVarKeys(). Each engine now declares its static, spec-driven allowlist independently of runtime secret resolution:BaseEngineprovides a default returning[]string{}. All engines implement it: Claude (ANTHROPIC_API_KEY), Codex (CODEX_API_KEY,OPENAI_API_KEY), Gemini (GEMINI_API_KEY), Antigravity (ANTIGRAVITY_API_KEY), and the universal engines (OpenCode, Crush, Pi) with their full multi-provider key sets.Constant addition
CopilotProviderWireAPI = "COPILOT_PROVIDER_WIRE_API"added toengine_constants.go— used in smoke workflows but was missing from the spec.Smoke workflow cleanup
Removed
strict: falsefrom all 6 smoke workflows (smoke-claude,smoke-codex,smoke-copilot,smoke-copilot-aoai-apikey,smoke-copilot-aoai-entra,smoke-copilot-arm). All 251 workflows recompile cleanly under strict mode.