fix(safe_jobs): hoist expression env vars into step env: block to prevent run-script guardrail failure#32532
Conversation
…cripts into env blocks Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…to avoid run script inline expressions Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…orted slices Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
env: block to prevent run-script guardrail failure
|
@copilot secrets like github.token should never be written to output |
There was a problem hiding this comment.
Pull request overview
Fixes a compiler regression guardrail failure for safe-jobs when job env: values contain GitHub Actions expressions (e.g. ${{ github.token }}), by ensuring expressions are not inlined inside run: scripts in the compiled lock output.
Changes:
- Update
buildSafeJobsto split job env vars into expression-based vs literal, hoisting expression-based entries into the step-levelenv:block and referencing them via shell variables inrun:. - Sort env keys within both categories for deterministic compiled output.
- Add a unit test covering mixed expression + literal env var behavior.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/safe_jobs.go |
Hoists expression-based env vars into step env: and avoids ${{ ... }} in run: by echoing ${VAR} instead. |
pkg/workflow/safe_jobs_test.go |
Adds coverage to ensure expressions are not inlined in run: and are present in env:. |
.github/workflows/daily-model-inventory.lock.yml |
Regenerated lock file output reflecting the updated compilation behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 0
…T to prevent secret leaks Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed. Expression-based env vars (values containing |
|
@copilot review the safe_job code generation and the ensure secrets are never stored in outputs |
…tten to GITHUB_OUTPUT in compiled lock file Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Reviewed all code generation paths in Added |
|
@copilot review why the environment variables need to be written to the output |
… env vars directly into step env: blocks Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
They don't need to be. The original design wrote env vars to Removed all |
|
Commit pushed:
|
|
Caution agentic threat detected Smoke Test Results
Overall Status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
|
Smoke Test Codex 25953787851: ❌ FAIL Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment. Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
Smoke Test: Claude — Run §25953787843Core (1–12): ✅ all passed Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal! Run 25953787843.
Warning
Firewall blocked 6 domains
The following domains were blocked by the firewall during workflow execution:
accounts.google.comandroid.clients.google.comclients2.google.comcontentautofill.googleapis.comsafebrowsingohttpgateway.googleapis.comwww.google.com
To allow these domains, add them to the
network.allowedlist in your workflow frontmatter:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
💥 [THE END] — Illustrated by Smoke Claude · ● 3.9M
|
👋 Smoke test agent was here! Everything is running smoothly — tests pass, Copilot is awake, and the automation is humming along. 🤖✨ Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
Smoke Test: ✅ PASS
PR author: Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment. Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
There was a problem hiding this comment.
Quick smoke-test review pass. The refactor in safe_jobs.go looks clean — hoisting env vars directly into step env: blocks is simpler and safer than routing through $GITHUB_OUTPUT.
Warning
Firewall blocked 6 domains
The following domains were blocked by the firewall during workflow execution:
accounts.google.comandroid.clients.google.comclients2.google.comcontentautofill.googleapis.comsafebrowsingohttpgateway.googleapis.comwww.google.com
To allow these domains, add them to the
network.allowedlist in your workflow frontmatter:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
📰 BREAKING: Report filed by Smoke Copilot · ● 11.9M
| // All job-specific env vars (literal or expression-based) are injected with | ||
| // their original values. Nothing goes through $GITHUB_OUTPUT. | ||
| for key, value := range jobConfig.Env { | ||
| setupEnvVars[key] = value |
There was a problem hiding this comment.
Nice simplification — removing the intermediate $GITHUB_OUTPUT write reduces one step and avoids the shell-injection risk that came with echoing arbitrary env var values. Using runner.temp expression directly is cleaner.
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Please address the unresolved review thread about the unnecessary step.
|
|
Please post a short progress update after rerunning checks so maintainers can see the remaining blocker.
|
|
Please address unresolved review feedback and propose fixes. — pr-sous-chef (run: https://github.com/github/gh-aw/actions/runs/25956153961)
|
|
Quick nudge:
|
|
Could Copilot summarize the remaining blockers and the next step to finish this PR?
|
|
/q PR SOUS chef needs to avoid spamming the same pull request. Create a deterministic step that filters out PrS where the last comment is from pr sous chef |
|
🎩 Mission equipment ready! Q has optimized your workflow. Use wisely, 007! 🔫 |
Safe jobs with
env:values containing GitHub Actions expressions (e.g.GH_TOKEN: ${{ github.token }}) caused the compiler regression guardrail to fire on their own compiled output —microsoft/aspire'smilestone-changelog.mdwas the reproducer.Root cause
buildSafeJobsunconditionally inlined allenv:values into therun:script viaecho:validateNoGitHubExpressionsInRunScriptsFromParsed(whichgithub.tokendoes not pass) then rejected the compiled lock file.Fix
pkg/workflow/safe_jobs.go: Classify eachenv:entry as expression-based (${{present) or literal. Expression entries are emitted in the step'senv:block; therun:script references them as shell variables instead.Literal values continue to be written directly in the
run:script unchanged. Keys are sorted in both categories for deterministic output.pkg/workflow/safe_jobs_test.go: AddedTestBuildSafeJobsEnvExpressionHoistingcovering the mixed expression/literal case, asserting the expression never appears inline inrun:and that the shell variable reference andenv:block are both present.Changeset\n\n- Type: patch\n- Description: Hoist expression-based safe job environment variables into step
env:blocks so compiledrun:scripts no longer inline GitHub Actions expressions and trigger the guardrail.✨ PR Review Safe Output Test - Run 25953787843
Warning
Firewall blocked 6 domains
The following domains were blocked by the firewall during workflow execution:
accounts.google.comandroid.clients.google.comclients2.google.comcontentautofill.googleapis.comsafebrowsingohttpgateway.googleapis.comwww.google.comSee Network Configuration for more information.
pr-sous-chef: attempted branch update (run: https://github.com/github/gh-aw/actions/runs/25956153961)