Skip to content

Commit c422dc8

Browse files
CopilotCopilot
authored
Reject setup-steps on activation and pre-activation jobs (#37441)
1 parent d25afa6 commit c422dc8

7 files changed

Lines changed: 108 additions & 16 deletions

File tree

‎.github/workflows/agentics-maintenance.yml‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,16 @@ jobs:
509509
- name: Build gh-aw
510510
run: make build
511511

512+
- name: Restore forecast report logs cache
513+
id: forecast_report_logs_cache
514+
uses: actions/cache/restore@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5
515+
with:
516+
path: ./.github/aw/logs
517+
key: ${{ runner.os }}-forecast-report-logs-${{ github.repository }}-${{ github.ref_name }}-${{ github.run_id }}
518+
restore-keys: |
519+
${{ runner.os }}-forecast-report-logs-${{ github.repository }}-
520+
${{ runner.os }}-forecast-report-logs-
521+
512522
- name: Generate forecast report
513523
id: generate_forecast_report
514524
shell: bash
@@ -532,6 +542,13 @@ jobs:
532542
exit 1
533543
fi
534544
545+
- name: Save forecast report logs cache
546+
if: ${{ always() }}
547+
uses: actions/cache/save@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5
548+
with:
549+
path: ./.github/aw/logs
550+
key: ${{ runner.os }}-forecast-report-logs-${{ github.repository }}-${{ github.ref_name }}-${{ github.run_id }}
551+
535552
- name: Generate forecast issue
536553
if: ${{ always() }}
537554
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0

‎docs/src/content/docs/reference/glossary.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,7 @@ Controls behavior when OTLP endpoint or header values resolve to empty at runtim
560560

561561
### Setup-Steps (`jobs.<job-id>.setup-steps`)
562562

563-
Steps injected immediately after the compiler-generated `actions/setup` step for a custom or built-in job. Defined under `jobs.<job-id>.setup-steps` in workflow frontmatter. When both a main workflow and an imported workflow define `setup-steps` for the same job, imported setup-steps run first. `setup-steps` remain distinct from `pre-steps` and are not merged across keys. See [Custom Jobs](/gh-aw/reference/steps-jobs/#jobs-and-steps).
563+
Steps injected immediately after the compiler-generated `actions/setup` step for a custom or built-in job. Defined under `jobs.<job-id>.setup-steps` in workflow frontmatter. When both a main workflow and an imported workflow define `setup-steps` for the same job, imported setup-steps run first. `setup-steps` remain distinct from `pre-steps` and are not merged across keys. `jobs.activation.setup-steps` and `jobs.pre_activation`/`jobs.pre-activation` `setup-steps` are refused at compile time because they can short-circuit protections. See [Custom Jobs](/gh-aw/reference/steps-jobs/#jobs-and-steps).
564564

565565
### Pre-Steps (`jobs.<job-id>.pre-steps`)
566566

‎docs/src/content/docs/reference/steps-jobs.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ jobs:
8080
| `continue-on-error` | Allow the workflow to continue if this job fails |
8181
| `container` | Docker container to run steps in |
8282
| `services` | Service containers (e.g. databases) |
83-
| `setup-steps` | Steps injected immediately after the compiler-generated `actions/setup` step for that job |
83+
| `setup-steps` | Steps injected immediately after the compiler-generated `actions/setup` step for that job (except `activation` and `pre_activation`, where compile fails) |
8484
| `pre-steps` | Steps injected after compiler setup steps and before checkout/`steps` in that job |
8585
| `steps` | List of steps — supports complete GitHub Actions step specification |
8686
| `uses` | Reusable workflow to call |
@@ -120,8 +120,8 @@ Use this map to see where compiler-inserted steps land for each job type.
120120

121121
| Job | Step order |
122122
|---|---|
123-
| `pre_activation` | `jobs.pre_activation.setup-steps` → compiler setup checkout/setup → `jobs.pre_activation.pre-steps` → built-in pre-activation steps |
124-
| `activation` | `jobs.activation.setup-steps` → compiler setup checkout/setup → `jobs.activation.pre-steps` → built-in activation steps |
123+
| `pre_activation` | `jobs.pre_activation.setup-steps` is refused at compile time to prevent short-circuiting protections; use `jobs.pre_activation.pre-steps` and `jobs.pre_activation.steps` |
124+
| `activation` | `jobs.activation.setup-steps` is refused at compile time to prevent short-circuiting protections; use `jobs.activation.pre-steps` |
125125
| `agent` | `jobs.agent.setup-steps` → compiler setup checkout/setup → `jobs.agent.pre-steps` → runtime path setup → top-level `pre-steps` → checkout/token/runtime/custom/agent steps |
126126
| `safe_outputs` | `jobs.safe_outputs.setup-steps` → compiler setup checkout/setup → `jobs.safe_outputs.pre-steps` → safe-outputs downloads/prep → GitHub App token minting → safe-output handlers/finalization |
127127
| `conclusion` | `jobs.conclusion.setup-steps` → compiler setup checkout/setup → `jobs.conclusion.pre-steps` → built-in conclusion steps (including GitHub App token minting when configured) |

‎pkg/parser/schemas/main_workflow_schema.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2455,7 +2455,7 @@
24552455
"$ref": "#/properties/jobs/additionalProperties/properties/steps"
24562456
}
24572457
],
2458-
"description": "Optional setup steps inserted after setup-injected steps and before any token-mint or checkout steps in this job. Uses the same schema as `steps`."
2458+
"description": "Optional setup steps inserted after setup-injected steps and before any token-mint or checkout steps in this job. Uses the same schema as `steps`. Not supported for jobs.activation, jobs.pre_activation, or jobs.pre-activation; compilation fails for those protected jobs because setup-steps could short-circuit protections."
24592459
},
24602460
"if": {
24612461
"type": "string",

‎pkg/workflow/compiler_builtin_presteps_test.go‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package workflow
55
import (
66
"os"
77
"path/filepath"
8+
"strings"
89
"testing"
910

1011
"github.com/github/gh-aw/pkg/testutil"
@@ -335,3 +336,50 @@ jobs:
335336
"- name: Generate GitHub App token",
336337
)
337338
}
339+
340+
func TestBuiltinJobsSetupStepsRejectedForActivationAndPreActivation(t *testing.T) {
341+
tests := []struct {
342+
name string
343+
jobKey string
344+
}{
345+
{name: "activation", jobKey: "activation"},
346+
{name: "pre-activation alias", jobKey: "pre-activation"},
347+
{name: "pre_activation", jobKey: "pre_activation"},
348+
}
349+
350+
for _, tt := range tests {
351+
t.Run(tt.name, func(t *testing.T) {
352+
tmpDir := testutil.TempDir(t, "reject-setup-steps-"+tt.jobKey)
353+
workflowContent := `---
354+
on:
355+
workflow_dispatch:
356+
engine: claude
357+
strict: false
358+
jobs:
359+
` + tt.jobKey + `:
360+
setup-steps:
361+
- name: blocked setup
362+
run: echo "blocked"
363+
---
364+
365+
# Reject setup-steps on protected jobs
366+
`
367+
368+
workflowFile := filepath.Join(tmpDir, "reject.md")
369+
if err := os.WriteFile(workflowFile, []byte(workflowContent), 0644); err != nil {
370+
t.Fatal(err)
371+
}
372+
373+
compiler := NewCompiler()
374+
err := compiler.CompileWorkflow(workflowFile)
375+
if err == nil {
376+
t.Fatalf("expected compile error for jobs.%s.setup-steps, got nil", tt.jobKey)
377+
}
378+
379+
want := "jobs." + tt.jobKey + ".setup-steps is not allowed"
380+
if !strings.Contains(err.Error(), want) {
381+
t.Fatalf("expected error containing %q, got: %v", want, err)
382+
}
383+
})
384+
}
385+
}

‎pkg/workflow/compiler_jobs.go‎

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,26 +1031,30 @@ func (c *Compiler) applyBuiltinJobPreSteps(data *WorkflowData) error {
10311031
}
10321032

10331033
for jobName, jobConfig := range data.Jobs {
1034-
targetJobName := jobName
1035-
if jobName == "pre-activation" {
1036-
targetJobName = string(constants.PreActivationJobName)
1037-
}
1038-
1039-
job, exists := c.jobManager.GetJob(targetJobName)
1040-
if !exists {
1041-
continue
1042-
}
1043-
10441034
configMap, ok := jobConfig.(map[string]any)
10451035
if !ok {
10461036
return fmt.Errorf("jobs.%s must be an object, got %T", jobName, jobConfig)
10471037
}
1038+
10481039
_, hasSetupSteps := configMap["setup-steps"]
10491040
_, hasPreSteps := configMap["pre-steps"]
1041+
if err := validateRestrictedBuiltinSetupSteps(jobName, hasSetupSteps); err != nil {
1042+
return err
1043+
}
10501044
if !hasSetupSteps && !hasPreSteps {
10511045
continue
10521046
}
10531047

1048+
targetJobName := jobName
1049+
if jobName == "pre-activation" {
1050+
targetJobName = string(constants.PreActivationJobName)
1051+
}
1052+
1053+
job, exists := c.jobManager.GetJob(targetJobName)
1054+
if !exists {
1055+
continue
1056+
}
1057+
10541058
var setupSteps []string
10551059
var preSteps []string
10561060
if hasSetupSteps {
@@ -1079,6 +1083,23 @@ func (c *Compiler) applyBuiltinJobPreSteps(data *WorkflowData) error {
10791083
return nil
10801084
}
10811085

1086+
func validateRestrictedBuiltinSetupSteps(jobName string, hasSetupSteps bool) error {
1087+
if !hasSetupSteps {
1088+
return nil
1089+
}
1090+
1091+
if jobName == string(constants.ActivationJobName) ||
1092+
jobName == string(constants.PreActivationJobName) ||
1093+
jobName == "pre-activation" {
1094+
return fmt.Errorf(
1095+
"jobs.%s.setup-steps is not allowed: setup-steps are refused for activation/pre-activation jobs because they can short-circuit protections",
1096+
jobName,
1097+
)
1098+
}
1099+
1100+
return nil
1101+
}
1102+
10821103
// insertSetupStepsAtStart places setup-steps at the start of the job so they
10831104
// run before any compiler-generated setup, checkout, or token-mint steps.
10841105
func insertSetupStepsAtStart(steps []string, setupSteps []string) []string {

‎pkg/workflow/compiler_pre_activation_job.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -596,8 +596,14 @@ func (c *Compiler) extractPreActivationCustomFields(jobs map[string]any) ([]stri
596596
}
597597

598598
for field := range configMap {
599+
if field == "setup-steps" {
600+
return nil, nil, fmt.Errorf(
601+
"jobs.%s.setup-steps is not allowed: setup-steps are refused for activation/pre-activation jobs because they can short-circuit protections",
602+
jobName,
603+
)
604+
}
599605
if !allowedFields[field] {
600-
return nil, nil, fmt.Errorf("jobs.%s: unsupported field '%s' - only 'steps' and 'outputs' are allowed", jobName, field)
606+
return nil, nil, fmt.Errorf("jobs.%s: unsupported field '%s' - only 'steps', 'outputs', and 'pre-steps' are allowed", jobName, field)
601607
}
602608
}
603609

0 commit comments

Comments
 (0)