Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions internal/cli/upload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1746,11 +1746,11 @@ func TestRunUploadEmitsReusableInputFailuresAsActionableFailingSteps(t *testing.
if len(pipeline.Steps) != 1 || !isGeneratedFailureCommand(pipeline.Steps[0].Command) {
t.Fatalf("reusable input failure pipeline = %#v", pipeline.Steps)
}
primary := `Reusable workflow input "target" uses an unsupported needs expression. Use exactly needs.<job>.outputs.<name> for a string input.`
primary := `Reusable workflow input "target" uses a needs expression in an unsupported form. Pass the whole value as exactly ${{ needs.<job>.outputs.<name> }}, with nothing around it. Only string inputs can take a needs value, and Buildkite resolves it before the called job runs, so the reference has to be the entire value rather than part of a larger expression. If you need a computed input from job outputs, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.`
detail := `Reusable-workflow input "target" is not statically resolvable: unsupported compile-time context "needs"`
message := string(failureArtifactForStep(pipeline.Steps[0].Plugins, runner.uploaded, "messages"))
annotation := string(failureArtifactForStep(pipeline.Steps[0].Plugins, runner.uploaded, "annotations"))
if !strings.Contains(message, primary) || !strings.Contains(message, "detail: "+detail) || !strings.Contains(annotation, "<strong>Reusable workflow input &#34;target&#34; uses an unsupported needs expression.</strong>") || !strings.Contains(annotation, "Use exactly needs.&lt;job&gt;.outputs.&lt;name&gt;") || !strings.Contains(annotation, strings.ReplaceAll(detail, `"`, "&#34;")) || len(pipeline.Steps[0].Notify) != 1 || strings.Contains(pipeline.Steps[0].Notify[0].GitHubCheck.Output.Summary, "<h2") || !strings.Contains(pipeline.Steps[0].Notify[0].GitHubCheck.Output.Summary, "Reusable workflow input &#34;target&#34;") {
if !strings.Contains(message, primary) || !strings.Contains(message, "detail: "+detail) || !strings.Contains(annotation, "<strong>Reusable workflow input &#34;target&#34; uses a needs expression in an unsupported form.</strong>") || !strings.Contains(annotation, "Pass the whole value as exactly ${{ needs.&lt;job&gt;.outputs.&lt;name&gt; }}") || !strings.Contains(annotation, "github.com/buildkite/buildkite-gha") || !strings.Contains(annotation, strings.ReplaceAll(detail, `"`, "&#34;")) || len(pipeline.Steps[0].Notify) != 1 || strings.Contains(pipeline.Steps[0].Notify[0].GitHubCheck.Output.Summary, "<h2") || !strings.Contains(pipeline.Steps[0].Notify[0].GitHubCheck.Output.Summary, "Reusable workflow input &#34;target&#34;") {
t.Fatalf("reusable input failure output = message %q, annotation %q, pipeline %#v", message, annotation, pipeline.Steps[0])
}
}
Expand Down
48 changes: 48 additions & 0 deletions internal/compiler/compiler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1858,6 +1858,54 @@ jobs:
}
})

t.Run("unsupported needs input form", func(t *testing.T) {
repository := t.TempDir()
path := writeWorkflow(t, repository, "caller.yml", `on: push
jobs:
prepare:
runs-on: ubuntu-latest
outputs:
target: ${{ steps.value.outputs.target }}
steps:
- id: value
run: echo target=test >> "$GITHUB_OUTPUT"
call:
needs: prepare
uses: ./.github/workflows/reusable.yml
with:
target: prefix-${{ needs.prepare.outputs.target }}
`)
writeWorkflow(t, repository, "reusable.yml", "on:\n workflow_call:\n inputs:\n target:\n type: string\njobs:\n test:\n runs-on: ubuntu-latest\n steps:\n - run: true\n")
_, err := Compile(path, readFile(t, path), readFile(t, smokePath("events", "push.json")))
if err == nil {
t.Fatal("Compile() error = nil, want unsupported needs input finding")
}
var finding *ProcessingFinding
wantMessage := `Reusable workflow input "target" uses a needs expression in an unsupported form. Pass the whole value as exactly ${{ needs.<job>.outputs.<name> }}, with nothing around it. Only string inputs can take a needs value, and Buildkite resolves it before the called job runs, so the reference has to be the entire value rather than part of a larger expression. If you need a computed input from job outputs, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.`
wantDetail := `Reusable-workflow input "target" is not statically resolvable: unsupported compile-time context "needs"`
if !errors.As(err, &finding) || finding.Message != wantMessage || finding.Detail != wantDetail || finding.Path != "./.github/workflows/caller.yml" || finding.Line != 14 || finding.Column != 15 || finding.Job != "call" {
t.Fatalf("Compile() finding = %#v", finding)
}
})

t.Run("value unavailable before jobs run", func(t *testing.T) {
repository := t.TempDir()
path := writeWorkflow(t, repository, "caller.yml", `on: push
jobs:
call:
uses: ./.github/workflows/reusable.yml
with:
target: ${{ steps.prepare.outputs.target }}
`)
writeWorkflow(t, repository, "reusable.yml", "on:\n workflow_call:\n inputs:\n target:\n type: string\njobs:\n test:\n runs-on: ubuntu-latest\n steps:\n - run: true\n")
_, err := Compile(path, readFile(t, path), readFile(t, smokePath("events", "push.json")))
var finding *ProcessingFinding
wantMessage := `Reusable workflow input "target" uses a value that is unavailable before jobs run. Replace it with a literal or an expression that does not depend on job results.`
if err == nil || !errors.As(err, &finding) || finding.Message != wantMessage {
t.Fatalf("Compile() finding = %#v", finding)
}
})

t.Run("call condition", func(t *testing.T) {
repository := t.TempDir()
path := writeWorkflow(t, repository, "caller.yml", "on: push\njobs:\n call:\n if: github.ref == 'refs/heads/main'\n uses: ./.github/workflows/reusable.yml\n")
Expand Down
20 changes: 18 additions & 2 deletions internal/compiler/reusable.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,10 +308,26 @@ func (resolver *reusableResolver) resolve(ctx context.Context, current reusableW
}
calleeSource, source, err := resolver.loadReusableWorkflow(ctx, current, call.Uses)
if err != nil {
var finding *ProcessingFinding
if errors.As(err, &finding) {
attributed := *finding
attributed.Path, attributed.Line, attributed.Column, attributed.Job = path, call.Span.Start.Line, call.Span.Start.Column, job.ID
attributed.Err = locatedJobWrappedError(path, job, call.Span.Start.Line, call.Span.Start.Column, "", finding.Err)
return reusableResolution{}, &attributed
}
return reusableResolution{}, locatedJobWrappedError(path, job, call.Span.Start.Line, call.Span.Start.Column, "", err)
}
if (call.InheritSecrets || len(call.Secrets) != 0) && calleeSource.identity.kind != "workspace" {
return reusableResolution{}, locatedJobError(path, job, call.Span.Start.Line, call.Span.Start.Column, "secret forwarding is supported only for repository-local reusable workflows")
message := fmt.Sprintf("A secrets: map cannot forward secrets to a workflow in another repository. Reusable workflow %q is outside this repository, so no secrets were forwarded. Reference each secret by name in the jobs of that workflow, or copy the workflow into this repository's .github/workflows and use a secrets: map with a ./ call. If you need explicit secret mappings across repositories, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.", calleeSource.displayPath)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: The first remediation in both messages still leaves the remote jobs without secret authority. Calls with no forwarding produce an empty restricted binding set in resolveCallSecretAuthority, so requiredSecrets drops direct ${{ secrets.NAME }} references—the existing TestCompileDoesNotGrantUninheritedReusableWorkflowSecrets covers this behavior. A user following this guidance receives an empty value at runtime. Please remove that option, or name buildkite-agent secret get NAME explicitly if that is the intended job-level mechanism.

if call.InheritSecrets {
message = fmt.Sprintf("secrets: inherit cannot forward secrets to a workflow in another repository. Reusable workflow %q is outside this repository, so no secrets were forwarded. Reference each secret by name in the jobs of that workflow, or copy the workflow into this repository's .github/workflows and use secrets: inherit with a ./ call. If you need secrets: inherit across repositories, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.", calleeSource.displayPath)
}
return reusableResolution{}, &ProcessingFinding{
Stage: StageGraph, Code: CodeGraphInvalid, Category: "compatibility",
Path: path, Line: call.Span.Start.Line, Column: call.Span.Start.Column, Job: job.ID,
Message: message,
Err: locatedJobError(path, job, call.Span.Start.Line, call.Span.Start.Column, message),
}
}
if cycle := resolver.cycle(calleeSource); cycle != "" {
return reusableResolution{}, locatedJobError(path, job, call.Span.Start.Line, call.Span.Start.Column, "reusable-workflow cycle detected: "+cycle)
Expand Down Expand Up @@ -750,7 +766,7 @@ func resolveCallInputs(path string, job workflow.Job, call *workflow.ReusableWor
if need, _, ok := deferredNeedReference(text); ok {
message = fmt.Sprintf("Reusable workflow input %q references job %q, but the call does not list it in needs. Add %q to the reusable-workflow call's needs.", name, need, need)
} else if strings.Contains(err.Error(), `unsupported compile-time context "needs"`) {
message = fmt.Sprintf("Reusable workflow input %q uses an unsupported needs expression. Use exactly needs.<job>.outputs.<name> for a string input.", name)
message = fmt.Sprintf("Reusable workflow input %q uses a needs expression in an unsupported form. Pass the whole value as exactly ${{ needs.<job>.outputs.<name> }}, with nothing around it. Only string inputs can take a needs value, and Buildkite resolves it before the called job runs, so the reference has to be the entire value rather than part of a larger expression. If you need a computed input from job outputs, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.", name)
}
return reusableInputs{}, &ProcessingFinding{
Stage: StageGraph, Code: CodeGraphInvalid, Category: "compatibility",
Expand Down
79 changes: 59 additions & 20 deletions internal/compiler/reusable_remote_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,26 +181,30 @@ jobs:
}
}

func TestCompileRejectsSecretInheritanceIntoRemoteReusableWorkflows(t *testing.T) {
func TestCompileRejectsSecretForwardingIntoRemoteReusableWorkflows(t *testing.T) {
for _, test := range []struct {
name string
caller string
remote string
name string
caller string
remote string
wantMessage string
}{
{
name: "direct remote call",
caller: "on: push\njobs:\n call:\n uses: owner/workflows/.github/workflows/ci.yml@v1\n secrets: inherit\n",
remote: "on: workflow_call\njobs:\n test:\n runs-on: ubuntu-latest\n steps: [{run: true}]\n",
name: "inherited remote call",
caller: "on: push\njobs:\n call:\n uses: owner/workflows/.github/workflows/ci.yml@v1\n secrets: inherit\n",
remote: "on: workflow_call\njobs:\n test:\n runs-on: ubuntu-latest\n steps: [{run: true}]\n",
wantMessage: `secrets: inherit cannot forward secrets to a workflow in another repository. Reusable workflow "owner/workflows/.github/workflows/ci.yml@v1" is outside this repository, so no secrets were forwarded. Reference each secret by name in the jobs of that workflow, or copy the workflow into this repository's .github/workflows and use secrets: inherit with a ./ call. If you need secrets: inherit across repositories, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.`,
},
{
name: "explicit remote call",
caller: "on: push\njobs:\n call:\n uses: owner/workflows/.github/workflows/ci.yml@v1\n secrets:\n token: ${{ secrets.SOURCE }}\n",
remote: "on:\n workflow_call:\n secrets:\n token:\njobs:\n test:\n runs-on: ubuntu-latest\n steps: [{run: true}]\n",
name: "explicit remote call",
caller: "on: push\njobs:\n call:\n uses: owner/workflows/.github/workflows/ci.yml@v1\n secrets:\n token: ${{ secrets.SOURCE }}\n",
remote: "on:\n workflow_call:\n secrets:\n token:\njobs:\n test:\n runs-on: ubuntu-latest\n steps: [{run: true}]\n",
wantMessage: `A secrets: map cannot forward secrets to a workflow in another repository. Reusable workflow "owner/workflows/.github/workflows/ci.yml@v1" is outside this repository, so no secrets were forwarded. Reference each secret by name in the jobs of that workflow, or copy the workflow into this repository's .github/workflows and use a secrets: map with a ./ call. If you need explicit secret mappings across repositories, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.`,
},
{
name: "local path within remote repository",
caller: "on: push\njobs:\n call:\n uses: owner/workflows/.github/workflows/ci.yml@v1\n",
remote: "on: workflow_call\njobs:\n nested:\n uses: ./.github/workflows/nested.yml\n secrets: inherit\n",
name: "local path within remote repository",
caller: "on: push\njobs:\n call:\n uses: owner/workflows/.github/workflows/ci.yml@v1\n",
remote: "on: workflow_call\njobs:\n nested:\n uses: ./.github/workflows/nested.yml\n secrets: inherit\n",
wantMessage: `secrets: inherit cannot forward secrets to a workflow in another repository. Reusable workflow "owner/workflows/.github/workflows/nested.yml@v1" is outside this repository, so no secrets were forwarded. Reference each secret by name in the jobs of that workflow, or copy the workflow into this repository's .github/workflows and use secrets: inherit with a ./ call. If you need secrets: inherit across repositories, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.`,
},
} {
t.Run(test.name, func(t *testing.T) {
Expand All @@ -212,8 +216,12 @@ func TestCompileRejectsSecretInheritanceIntoRemoteReusableWorkflows(t *testing.T
options := defaultOptions()
options.RepositorySource = MemoizeRepositorySource(newFakeReusableRepositorySource(t, map[string]string{"owner/workflows": remoteRoot}))
_, err := CompileWithOptions(callerPath, readFile(t, callerPath), pushEvent(t), options)
if err == nil || !strings.Contains(err.Error(), "secret forwarding is supported only for repository-local reusable workflows") {
t.Fatalf("CompileWithOptions() error = %v, want remote inheritance rejection", err)
var finding *ProcessingFinding
if err == nil || !errors.As(err, &finding) {
t.Fatalf("CompileWithOptions() error = %v, want remote forwarding finding", err)
}
if finding.Message != test.wantMessage || finding.Detail != "" || finding.Path != "./.github/workflows/caller.yml" || finding.Line != 4 || finding.Column != 11 || finding.Job != "call" {
t.Fatalf("CompileWithOptions() finding = %#v", finding)
}
})
}
Expand Down Expand Up @@ -446,11 +454,15 @@ func TestCompileRemoteReusableWorkflowLimitsAndDiagnostics(t *testing.T) {
})

for _, test := range []struct {
name string
uses string
want string
name string
uses string
want string
wantMessage string
}{
{name: "dynamic", uses: "${{ inputs.workflow }}", want: "runtime-dependent"},
{
name: "dynamic", uses: "${{ inputs.workflow }}", want: "reusable workflow path cannot be an expression",
wantMessage: `Reusable workflow path cannot be an expression. "${{ inputs.workflow }}" is only known once the build is running, and the workflow file has to be read before that. Name the file directly, for example ./.github/workflows/ci.yml, or org/shared/.github/workflows/ci.yml@v1. If you need a computed workflow path, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.`,
},
{name: "nested path", uses: "owner/repo/.github/workflows/nested/ci.yml@v1", want: "directly under .github/workflows"},
{name: "wrong directory", uses: "owner/repo/workflows/ci.yml@v1", want: "directly under .github/workflows"},
{name: "non YAML", uses: "owner/repo/.github/workflows/ci.json@v1", want: "must end in .yml or .yaml"},
Expand All @@ -465,6 +477,12 @@ func TestCompileRemoteReusableWorkflowLimitsAndDiagnostics(t *testing.T) {
if err == nil || !strings.Contains(err.Error(), test.want) || len(fake.references()) != 0 {
t.Fatalf("CompileWithOptions() error/calls = %v / %#v, want %q before source access", err, fake.references(), test.want)
}
if test.wantMessage != "" {
var finding *ProcessingFinding
if !errors.As(err, &finding) || finding.Message != test.wantMessage || finding.Detail != "" || finding.Path != "./.github/workflows/dynamic.yml" || finding.Line != 4 || finding.Column != 11 || finding.Job != "call" {
t.Fatalf("CompileWithOptions() finding = %#v", finding)
}
}
})
}

Expand All @@ -475,9 +493,30 @@ func TestCompileRemoteReusableWorkflowLimitsAndDiagnostics(t *testing.T) {
options := defaultOptions()
options.RepositorySource = MemoizeRepositorySource(fake)
_, err := CompileWithOptions(callerPath, readFile(t, callerPath), event, options)
if err == nil || !strings.Contains(err.Error(), `public reusable workflow "owner/private/.github/workflows/ci.yml@v1" was not found or is not public`) {
if err == nil || !strings.Contains(err.Error(), `public reusable workflow "owner/private/.github/workflows/ci.yml@v1" could not be read`) {
t.Fatalf("CompileWithOptions() error = %v, want non-enumerating source error", err)
}
var finding *ProcessingFinding
wantMessage := `Reusable workflow could not be read. "owner/private/.github/workflows/ci.yml@v1" is either private or does not exist. Only public workflows can be called across repositories. Check the path, or copy the workflow into this repository's .github/workflows and call it with a ./ path. If you need private cross-repository calls, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.`
if !errors.As(err, &finding) || finding.Message != wantMessage || finding.Detail != "" || finding.Path != "./.github/workflows/private.yml" || finding.Line != 4 || finding.Column != 11 || finding.Job != "call" {
t.Fatalf("CompileWithOptions() finding = %#v", finding)
}
})

t.Run("missing workflow in public repository", func(t *testing.T) {
callerPath := writeWorkflow(t, callerRoot, "missing.yml", "on: push\njobs:\n call:\n uses: owner/public/.github/workflows/absent.yml@v1\n")
publicRoot := t.TempDir()
options := defaultOptions()
options.RepositorySource = MemoizeRepositorySource(newFakeReusableRepositorySource(t, map[string]string{"owner/public": publicRoot}))
_, err := CompileWithOptions(callerPath, readFile(t, callerPath), event, options)
if err == nil || !strings.Contains(err.Error(), `public reusable workflow "owner/public/.github/workflows/absent.yml@v1" could not be read`) {
t.Fatalf("CompileWithOptions() error = %v, want non-enumerating missing workflow error", err)
}
var finding *ProcessingFinding
wantMessage := `Reusable workflow could not be read. "owner/public/.github/workflows/absent.yml@v1" is either private or does not exist. Only public workflows can be called across repositories. Check the path, or copy the workflow into this repository's .github/workflows and call it with a ./ path. If you need private cross-repository calls, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.`
if !errors.As(err, &finding) || finding.Message != wantMessage || finding.Detail != "" || finding.Path != "./.github/workflows/missing.yml" || finding.Line != 4 || finding.Column != 11 || finding.Job != "call" {
t.Fatalf("CompileWithOptions() finding = %#v", finding)
}
})

t.Run("cancellation", func(t *testing.T) {
Expand Down
Loading