Skip to content

Fix non-deterministic secret replacement in workflows with shared fallback expressions#33441

Merged
pelikhan merged 3 commits into
mainfrom
copilot/investigate-compiler-bug
May 20, 2026
Merged

Fix non-deterministic secret replacement in workflows with shared fallback expressions#33441
pelikhan merged 3 commits into
mainfrom
copilot/investigate-compiler-bug

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 20, 2026

When two imported workflows (e.g. shared/datadog.md + shared/mcp/datadog.md) both reference the same fallback secret expression, the compiled lock.yml produces different env var names across runs.

Root cause

ExtractSecretsFromValue("${{ secrets.DD_APPLICATION_KEY || secrets.DD_APP_KEY }}") returns a map[string]string with two keys (DD_APPLICATION_KEY, DD_APP_KEY) sharing the same expression value. Three replacement functions iterated over this map with Go's randomized range — whichever key came first consumed the expression via strings.ReplaceAll; the other was a no-op. Winner was random per run.

# lock.yml would non-deterministically emit either:
"DD_APPLICATION_KEY": "${DD_APPLICATION_KEY}"
# or
"DD_APPLICATION_KEY": "${DD_APP_KEY}"

Fix

  • ReplaceSecretsWithEnvVars, ReplaceSecretsWithBashVars, ReplaceTemplateExpressionsWithEnvVars: replace bare for k := range secrets with the existing sortedMapKeys helper, ensuring stable iteration order.
    Since 'L' (76) < '_' (95), DD_APPLICATION_KEY sorts before DD_APP_KEY and consistently wins — matching the primary-secret-first semantics of the || expression.

  • Regression tests: TestReplaceSecretsWithEnvVars_FallbackExpressionDeterminism and TestReplaceSecretsWithBashVars_FallbackExpressionDeterminism each call the affected function 50 times with a shared-expression map and assert identical output every run.

Copilot AI and others added 3 commits May 20, 2026 03:37
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title Fix non-deterministic env var generation when importing multiple workflows with shared fallback secrets Fix non-deterministic secret replacement in workflows with shared fallback expressions May 20, 2026
Copilot AI requested a review from pelikhan May 20, 2026 03:46
@pelikhan pelikhan marked this pull request as ready for review May 20, 2026 03:47
Copilot AI review requested due to automatic review settings May 20, 2026 03:47
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes non-deterministic secret/env expression replacement caused by randomized Go map iteration when multiple keys share the same fallback expression, ensuring stable compiled lock.yml output across runs.

Changes:

  • Iterates over secret/env maps in deterministic (sorted) key order in replacement helpers.
  • Adds regression tests that repeatedly run replacements to assert stable output for shared fallback expressions.
Show a summary per file
File Description
pkg/workflow/secret_extraction.go Uses sortedMapKeys when replacing secrets/env expressions to ensure deterministic output when multiple keys map to the same expression.
pkg/workflow/secret_extraction_test.go Adds determinism regression tests for fallback secret expressions in both env-var and bash-var replacement functions.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

@github-actions github-actions Bot mentioned this pull request May 20, 2026
@pelikhan pelikhan merged commit e38c582 into main May 20, 2026
4 checks passed
@pelikhan pelikhan deleted the copilot/investigate-compiler-bug branch May 20, 2026 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants