Detect and block secrets in env section including sub-expressions#16722
Detect and block secrets in env section including sub-expressions#16722
Conversation
- Add validateEnvSecrets function to detect secrets in env section - In strict mode: raise error when secrets are detected - In non-strict mode: emit warning when secrets are detected - Add comprehensive test suite with 16+ test cases - Manually verified with test workflows Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot also detect sub expressions |
Enhanced ExtractSecretsFromValue to detect secrets in complex GitHub Actions
expressions like:
- ${{ github.workflow && secrets.TOKEN }}
- ${{ (github.actor || secrets.HIDDEN) }}
- ${{ !secrets.PRIVATE_KEY && github.workflow }}
Uses regex to find all ${{...}} expressions, then searches for secrets.*
pattern within each expression. Handles multiple secrets in same expression.
Added comprehensive test coverage for sub-expression cases in both
secret_extraction_test.go and env_secrets_validation_test.go.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Enhanced secret detection to catch sub-expressions like The implementation now uses regex to find all Commit: 44fdfc6 |
There was a problem hiding this comment.
Pull request overview
Adds compilation-time detection of ${{ secrets.* }} usage in the workflow top-level env section to prevent secrets leaking into the agent container, including secrets embedded inside complex GitHub Actions expressions.
Changes:
- Added
validateEnvSecrets()to error in strict mode and warn in non-strict mode when secrets are found in top-levelenv. - Enhanced secret extraction to detect
secrets.*inside${{ ... }}sub-expressions. - Added/expanded tests covering strict vs non-strict behavior and sub-expression patterns.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/strict_mode_validation.go | Introduces env secret validation with strict error / non-strict warning behavior. |
| pkg/workflow/secret_extraction.go | Updates extraction logic to find secrets inside ${{ ... }} blocks and complex expressions. |
| pkg/workflow/compiler_orchestrator_engine.go | Integrates env secret validation into the compilation flow. |
| pkg/workflow/secret_extraction_test.go | Adds test cases for secret detection in sub-expressions. |
| pkg/workflow/env_secrets_validation_test.go | Adds comprehensive tests for env secret validation in strict and non-strict modes. |
Comments suppressed due to low confidence (2)
pkg/workflow/strict_mode_validation.go:266
validateEnvSecretsonly type-assertsenvasmap[string]any. In this codebaseenvis sometimes stored asmap[string]stringinside amap[string]any(e.g.,FrontmatterConfig.ToMap()setsresult["env"] = fc.Env). In those cases this validation will be silently skipped and secrets can still leak. Handle bothmap[string]anyandmap[string]string(and/or normalize via a helper) so validation is applied consistently.
// Parse env as map[string]string
envMap, ok := envValue.(map[string]any)
if !ok {
strictModeValidationLog.Printf("Env section is not a map, skipping validation")
return nil
}
pkg/workflow/strict_mode_validation.go:287
secretRefsis built by iterating a map (secrets), so the order is nondeterministic and duplicate expressions can appear (e.g.,${{ secrets.KEY1 && secrets.KEY2 }}will append the same expression twice). This makes the emitted error/warning unstable and harder to read. Consider de-duplicating expressions (set) and sorting beforestrings.Join, or alternatively listing the secret names (map keys) instead of the expression values.
// Build list of secret references found
var secretRefs []string
for _, secretExpr := range secrets {
secretRefs = append(secretRefs, secretExpr)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Find all ${{ ... }} expressions in the value | ||
| // Pattern matches from ${{ to }} allowing nested content | ||
| exprPattern := regexp.MustCompile(`\$\{\{[^}]+\}\}`) | ||
| expressions := exprPattern.FindAllString(value, -1) | ||
|
|
||
| // For each expression, check if it contains secrets.VARIABLE_NAME | ||
| // This handles both simple cases like "${{ secrets.TOKEN }}" | ||
| // and complex sub-expressions like "${{ github.workflow && secrets.TOKEN }}" | ||
| secretPattern := regexp.MustCompile(`secrets\.([A-Z_][A-Z0-9_]*)`) | ||
| for _, expr := range expressions { |
There was a problem hiding this comment.
exprPattern := regexp.MustCompile(${{[^}]+}}) will stop at the first } inside the expression, so it can fail to match valid ${{ ... }} blocks that contain } characters (e.g., JSON literals/strings like fromJSON('{"a":1}')). That can cause secret references inside those expressions to be missed. Consider switching to a non-greedy DOTALL pattern (e.g., (?s)\$\{\{.*?\}\}) or a small scanner that finds the next }} after ${{.
Also, both regexes are compiled on every call; they should be package-level precompiled vars like secretExprPattern to avoid repeated compilation.
Secrets defined in the workflow
envsection are leaked to the agent container. The compiler now detects${{ secrets.* }}expressions in environment variables and enforces proper secret handling, including secrets embedded in complex sub-expressions.Changes
Validation logic (
pkg/workflow/strict_mode_validation.go)validateEnvSecrets()using existingExtractSecretsFromMaputilityEnhanced secret detection (
pkg/workflow/secret_extraction.go)ExtractSecretsFromValue()to detect secrets in complex GitHub Actions sub-expressions${{...}}expressions, then searches forsecrets.*patterns within each${{ github.workflow && secrets.TOKEN }}${{ (github.actor || secrets.HIDDEN) }}${{ !secrets.PRIVATE_KEY && github.workflow }}${{ secrets.KEY1 && secrets.KEY2 }}Integration (
pkg/workflow/compiler_orchestrator_engine.go)Test coverage (
pkg/workflow/env_secrets_validation_test.go,pkg/workflow/secret_extraction_test.go)Example
Strict mode:
Non-strict mode:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.