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.
There was a problem hiding this comment.
Verified addressed in 801b499/2b83bd4: sparse-checkout patterns are validated and passed after Git’s -- option terminator. Local validation (make fmt, make lint, make test-unit, make test, make recompile) passes.
| 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.
There was a problem hiding this comment.
Verified addressed in 801b499: clone, checkout, sparse-checkout, and submodule operations run with GIT_LFS_SKIP_SMUDGE=1, and LFS objects are fetched only by the explicit lfs: true pull path. Local validation passes.
| }, | ||
| { | ||
| "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.
There was a problem hiding this comment.
Verified addressed in 801b499: dynamic checkout expressions are trimmed consistently during parsing and covered by TestParseFrontmatterConfigDynamicCheckoutTrimsExpression. Local validation passes.
| 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.
There was a problem hiding this comment.
Verified addressed in 801b499: dynamic checkout expressions referencing steps.* are rejected at compile time, avoiding safe_outputs re-evaluation of agent-job-local step outputs. Local validation passes.
|
@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
|
|
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 |
|
@copilot There is still forward progress needed on this PR. Please continue with the remaining unresolved review items, newest concerns first:
There are no newly listed failed checks in the compact queue, so the priority is clearing the open review feedback and getting the PR back to a maintainer-ready state.
|
…URL validation 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>
Tightened the dynamic checkout git argument hardening in
Tests added in |
|
@copilot There is still forward progress needed on this PR. Please continue with the remaining unresolved review items, newest first:
Please also refresh the branch as needed and run the
|
Completed the finishing pass. The four linked review concerns are already addressed on this branch ( |
|
|
Final cleanup pass complete. The four linked
No code changes were needed. The branch is clean, includes current base |
|
@copilot Review cleanup remains on this PR. Please run the Also refresh the branch if GitHub still reports it behind base after the queued update-branch attempt from this sous-chef run.
|
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
Branch refresh requested by PR Sous Chef run https://github.com/github/gh-aw/actions/runs/36280932883
Run: https://github.com/github/gh-aw/actions/runs/36291473147
pr-sous-chefRun: https://github.com/github/gh-aw/actions/runs/36296973970
pr-sous-chefRun: https://github.com/github/gh-aw/actions/runs/36307747288
pr-sous-chefRun: https://github.com/github/gh-aw/actions/runs/36310278424
Run: https://github.com/github/gh-aw/actions/runs/36333017097
pr-sous-chefbranch refresh requested from run https://github.com/github/gh-aw/actions/runs/36334311673