Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| fetchArgs.push("origin", checkout.ref); | ||
| await runGit(fetchArgs); |
There was a problem hiding this comment.
Addressed in the current branch: dynamic checkout refs are rejected when they start with -, git fetch passes refs after --, sparse-checkout patterns are rejected when option-like, and sparse patterns are passed after --.
|
@copilot checkouts should |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Runtime credential cleanup, expression consistency, and Git option/LFS handling contain unresolved correctness and security issues.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (4)
What changed in this PR
Adds runtime-selected checkout sets to the compiler, agent jobs, safe-output jobs, metadata, schema, and documentation.
Changes:
- Parses expression-valued checkout declarations.
- Implements validated runtime Git checkouts and manifest merging.
- Documents and tests dynamic checkout behavior.
| File | Description |
|---|---|
pkg/workflow/workflow_data.go |
Stores dynamic expressions. |
pkg/workflow/workflow_builder.go |
Collects local and imported expressions. |
pkg/workflow/unified_prompt_step.go |
Adds dynamic checkout guidance. |
pkg/workflow/frontmatter_types.go |
Extends parsed configuration. |
pkg/workflow/frontmatter_parsing.go |
Parses expression-valued checkout. |
pkg/workflow/dynamic_checkout.go |
Generates runtime checkout steps. |
pkg/workflow/dynamic_checkout_test.go |
Tests parsing and generation. |
pkg/workflow/compiler_yaml_checkout.go |
Emits agent checkout steps. |
pkg/workflow/compiler_safe_outputs_steps.go |
Emits safe-output checkout steps. |
pkg/parser/schemas/main_workflow_schema.json |
Allows checkout expressions. |
docs/src/content/docs/reference/checkout.md |
Documents dynamic checkout sets. |
actions/setup/js/dynamic_checkouts.test.cjs |
Tests runtime behavior. |
actions/setup/js/dynamic_checkouts.cjs |
Implements runtime Git checkout. |
actions/setup/js/build_checkout_manifest.test.cjs |
Tests manifest preservation. |
actions/setup/js/build_checkout_manifest.cjs |
Merges existing manifest entries. |
| .split(/\r?\n/) | ||
| .map(pattern => pattern.trim()) | ||
| .filter(Boolean); | ||
| await runGit(["-C", checkoutTarget, "sparse-checkout", "set", "--no-cone", ...patterns]); |
There was a problem hiding this comment.
Sparse-checkout patterns are now validated and passed after Git's option terminator. Addressed in 801b499.
| core.info(`Checking out ${checkout.repository} into ${checkout.path}`); | ||
| await runGit(cloneArgs); |
There was a problem hiding this comment.
Dynamic checkout clone, checkout, sparse checkout, and submodule operations now disable LFS smudging; only the explicit LFS branch pulls objects. Addressed in 801b499.
| }, | ||
| { | ||
| "type": "string", | ||
| "pattern": "^\\s*\\$\\{\\{[\\s\\S]+\\}\\}\\s*$", |
There was a problem hiding this comment.
Expression-valued dynamic checkouts are trimmed consistently during parsing and covered by a regression test. Addressed in 801b499.
| steps = append(steps, injectStepCondition( | ||
| c.generateDynamicCheckoutSteps(data.CheckoutExpressions, prCheckoutToken, true), | ||
| condition, | ||
| )...) |
There was a problem hiding this comment.
Dynamic checkout expressions referencing steps.* now fail compilation, preventing agent-job values from being re-evaluated in safe_outputs. Addressed in 801b499.
|
@copilot dynamic checkout should require a "allowed-repos" field that restricts the list of repo allowed to be checkout (gh action expression support too) |
|
🔎 PR Code Quality Reviewer is reviewing code quality for this pull request... |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"
- "proxy.golang.org"See Network Configuration for more information.
|
|
✅ Ponytail Reviewer completed successfully! Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"See Network Configuration for more information.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ Warning Firewall blocked 5 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"
- "o205451.ingest.us.sentry.io"
- "proxy.golang.org"
- "storage.googleapis.com"
- "sum.golang.org"See Network Configuration for more information.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "o205451.ingest.us.sentry.io"See Network Configuration for more information.
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Added compile-time validation: dynamic |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Ponytail pass focused strictly on deletable complexity in changed lines.
net: -7 lines possible.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
ab.chatgpt.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "ab.chatgpt.com"See Network Configuration for more information.
Generated by ✂️ Ponytail Reviewer for #63241 · codex · gpt53codex · 6.46 AIC · ⌖ 2.15 AIC · ⊞ 12.5K
Comment /ponytail to run again
| throw new Error(`failed to write dynamic checkout manifest: ${getErrorMessage(error)}`, { cause: error }); | ||
| } | ||
| return manifestPath; | ||
| } |
There was a problem hiding this comment.
L215: yagni: tri-branch submodule mode check (true/"true"/"recursive") plus conditional flag push. const recursive = checkout.submodules === "recursive"; if (checkout.submodules !== false) run once with optional --recursive.
There was a problem hiding this comment.
No change for this style-only simplification: the current explicit submodule handling keeps the accepted true, "true", and "recursive" modes easy to audit alongside the runtime validation.
| } | ||
|
|
||
| function credentialArgs(serverURL, token, maskSecret = value => core.setSecret(value)) { | ||
| if (!token) { |
There was a problem hiding this comment.
L85: stdlib: manual key-validation loop with Set membership. Object.keys(entry).every(k => supportedFields.has(k)) with one failure branch, fewer control lines.
There was a problem hiding this comment.
No change for this style-only simplification: the current explicit loop keeps the unsupported field name available for the precise validation error.
| } | ||
| } else { | ||
| fs.mkdirSync(candidate); | ||
| } |
There was a problem hiding this comment.
L123: shrink: duplicated token alias handling (github-token and token) every normalize call. Resolve once (const token = entry["github-token"] ?? entry.token ?? "") and stringify once.
There was a problem hiding this comment.
No change for this style-only simplification: the current normalization keeps the token aliases close to the returned checkout shape without changing behavior.
🏗️ ADR required — draft added for PR #63241I added a draft ADR at Evidence used
Inferred decision captured in the draft
Next actionPlease review and refine the draft ADR, especially the trade-offs and final wording, before merging this PR. Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"
- "proxy.golang.org"See Network Configuration for more information.
|
| if (checkout.ref) { | ||
| const fetchArgs = [...authArgs, "-C", checkoutTarget, "fetch", "--no-tags"]; | ||
| if (checkout.fetchDepth > 0) { | ||
| fetchArgs.push("--depth", String(checkout.fetchDepth)); | ||
| } | ||
| fetchArgs.push("origin", checkout.ref); | ||
| await runGit(fetchArgs); | ||
| await runGit(["-C", checkoutTarget, "checkout", "--force", "FETCH_HEAD"]); | ||
| } | ||
|
|
||
| if (checkout.sparseCheckout.trim()) { | ||
| const patterns = checkout.sparseCheckout | ||
| .split(/\r?\n/) | ||
| .map(pattern => pattern.trim()) | ||
| .filter(Boolean); | ||
| await runGit(["-C", checkoutTarget, "sparse-checkout", "set", "--no-cone", ...patterns]); |
There was a problem hiding this comment.
Addressed in the current branch: dynamic checkout refs are validated against leading - and passed to git fetch after --; sparse-checkout patterns are also validated and passed after --.
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 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "o205451.ingest.us.sentry.io"See Network Configuration for more information.
|
There was a problem hiding this comment.
Blocking runtime gaps remain in dynamic checkout validation, safe_outputs re-evaluation, and git argument hardening.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
o205451.ingest.us.sentry.io
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "o205451.ingest.us.sentry.io"See Network Configuration for more information.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 83.5 AIC · ⌖ 7.16 AIC · ⊞ 20.3K
Comment /review to run again
|
@copilot summarize the new syntax vs old syntax. Determine if it is confusing for an agent, update to make it more clear that some is static va dynamic |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Updated in Syntax summary:
checkout:
repos: ${{ fromJSON(inputs.checkouts) }}
allowed-repos:
- owner/allowed-repo
To reduce agent confusion, I updated the checkout reference, normative checkout spec, and schema description to explicitly say that top-level Validation: secret scan passed; |
|
Thanks for the contribution, Nice work covering validation edge cases like symlink workspace escapes and credential handling for safe-output jobs.
|
|
|
Verified current head |
|
|
No change since the last verification — head is still |
Reviewed all 20 listed threads. Each already has a substantive Copilot reply documenting the implemented fix or rationale; the current head contains those fixes, so no additional code change is needed. The branch is refreshed to |
|
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Merged current |
checkout:only accepted statically known objects or arrays, preventing workflows from selecting repositories at runtime. It now accepts an expression resolving to one checkout object or an array.Changes
Compiler
Runtime
Checkout metadata
Schema and documentation
checkout:.Run: https://github.com/github/gh-aw/actions/runs/36058136776
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
github.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.
Run: https://github.com/github/gh-aw/actions/runs/36060789465
Branch refresh requested by PR Sous Chef run https://github.com/github/gh-aw/actions/runs/36062966955
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
github.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.
✨ PR Review Safe Output Test - Run 36076891728
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
clients2.google.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.
Run: https://github.com/github/gh-aw/actions/runs/36092368943
Branch refresh requested by PR Sous Chef run https://github.com/github/gh-aw/actions/runs/36175468658
pr-sous-chefbranch refresh requested from run https://github.com/github/gh-aw/actions/runs/36221620757pr-sous-chefbranch refresh requested by https://github.com/github/gh-aw/actions/runs/36255219324pr-sous-chefBranch refresh requested by PR Sous Chef run: https://github.com/github/gh-aw/actions/runs/36260722205