Skip to content
Merged
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
1 change: 0 additions & 1 deletion internal/workflowfile/testdata/mixed_refs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ jobs:
- uses: ./local-action
- uses: docker://alpine:3.18
- uses: actions/cache/save@v4
- uses: ${{ matrix.action }}
- uses: owner/repo/.github/workflows/called.yml@v1
- uses: owner/repo/.github/workflows/called.yaml@main
- uses: actions/setup-node@v4
Expand Down
131 changes: 101 additions & 30 deletions internal/workflowfile/workflowfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,34 @@ func Parse(path string, content []byte) (*File, error) {
return nil, fmt.Errorf("parsing workflow YAML: %w", err)
}

var invalid []string
walkUses(&f.root, func(value string, _ bool) {
Comment thread
nodeselector marked this conversation as resolved.
if isUsesExpression(value) {
Comment thread
nodeselector marked this conversation as resolved.
invalid = append(invalid, strings.TrimSpace(value))
}
})
if len(invalid) > 0 {
return nil, usesExpressionErr(invalid)
}

return f, nil
}

// isUsesExpression reports whether a `uses:` value contains an expression.
// Malformed `$/…@ref` values are excluded so the more specific self
// repository diagnostic still wins.
func isUsesExpression(value string) bool {
value = strings.TrimSpace(value)
return strings.Contains(value, "${") && !SelfRepositoryRefHasVersion(value)
}

// usesExpressionErr reports an expression in `uses:`. GitHub Actions rejects
// the workflow outright at both step and job level (no context is in scope
// there), so parsing fails rather than skipping the line.
func usesExpressionErr(values []string) error {
return fmt.Errorf("`uses:` can't contain an expression: %s", strings.Join(values, ", "))
}

// RefScan is the classified result of walking a workflow's `uses:` values.
type RefScan struct {
// Refs are remote repository action references (owner/repo[/path]@ref).
Expand Down Expand Up @@ -88,10 +113,6 @@ func (f *File) ExtractActionRefs() RefScan {
}
return
}
if strings.Contains(value, "${") {
scan.Warnings = append(scan.Warnings, fmt.Sprintf("skipping unparseable uses: value %q (expressions are not supported)", value))
return
}
if IsSelfRepositoryAction(value) {
// Bare `$/…` is a legal self repository reference at both step level
// (action dir) and job level (reusable-workflow file): "this repo
Expand Down Expand Up @@ -212,8 +233,6 @@ func ScanSelfRepositoryActions(workflowPath string, actionRefs []string) SelfRep
seenInvalid[use] = true
scan.SelfRepositoryRefErrs = append(scan.SelfRepositoryRefErrs, use)
}
case strings.Contains(use, "${"):
scan.Warnings = append(scan.Warnings, fmt.Sprintf("skipping unparseable uses: value %q in %s (expressions are not supported)", use, current.ref))
case IsSelfRepositoryAction(use):
if !seenSelf[use] {
seenSelf[use] = true
Expand Down Expand Up @@ -375,37 +394,81 @@ func ExtractLocalCompositeRefs(workflowPath string, localPaths []string) ([]pars
return refs, warnings
}

// walkUses visits `uses:` values at the only places GitHub Actions honors
// them: `jobs.<id>.uses`, `jobs.<id>.steps[*].uses` in a workflow, and
// `runs.steps[*].uses` in a composite action definition. A `uses` key
// anywhere else (an env var, a `with:` input) is ordinary data, not a
// reference.
func walkUses(node *yaml.Node, fn func(value string, stepLevel bool)) {
walkUsesDepth(node, false, fn, 0)
root := resolveAlias(documentRoot(node))
walkStepUses(mapValue(root, "runs"), fn)

jobs := mapValue(root, "jobs")
if jobs == nil || jobs.Kind != yaml.MappingNode {
return
}
for i := 1; i < len(jobs.Content); i += 2 {
job := resolveAlias(jobs.Content[i])
if job == nil || job.Kind != yaml.MappingNode {
continue
}
if use := scalarValue(mapValue(job, "uses")); use != "" {
fn(use, false)
}
walkStepUses(job, fn)
}
}

func walkUsesDepth(node *yaml.Node, inStep bool, fn func(value string, stepLevel bool), depth int) {
if node == nil || depth > maxYAMLWalkDepth {
// walkStepUses visits the `uses:` of each step in owner's `steps:` sequence.
func walkStepUses(owner *yaml.Node, fn func(value string, stepLevel bool)) {
steps := mapValue(owner, "steps")
if steps == nil || steps.Kind != yaml.SequenceNode {
return
}
switch node.Kind {
case yaml.DocumentNode, yaml.SequenceNode:
for _, child := range node.Content {
walkUsesDepth(child, inStep, fn, depth+1)
for _, stepNode := range steps.Content {
step := resolveAlias(stepNode)
if step == nil || step.Kind != yaml.MappingNode {
continue
}
case yaml.MappingNode:
for i := 0; i < len(node.Content)-1; i += 2 {
key := node.Content[i]
value := node.Content[i+1]
childInStep := inStep
if key.Kind == yaml.ScalarNode {
switch key.Value {
case "uses":
if value.Kind == yaml.ScalarNode {
fn(value.Value, inStep)
}
case "steps":
childInStep = true
}
}
walkUsesDepth(value, childInStep, fn, depth+1)
if use := scalarValue(mapValue(step, "uses")); use != "" {
fn(use, true)
}
}
}

func documentRoot(node *yaml.Node) *yaml.Node {
if node != nil && node.Kind == yaml.DocumentNode && len(node.Content) > 0 {
return node.Content[0]
}
return node
}

// resolveAlias dereferences YAML anchors so an aliased job or step is walked
// like the node it points at.
func resolveAlias(node *yaml.Node) *yaml.Node {
for i := 0; node != nil && node.Kind == yaml.AliasNode && i < maxYAMLWalkDepth; i++ {
node = node.Alias
}
return node
}

func mapValue(node *yaml.Node, key string) *yaml.Node {
if node == nil || node.Kind != yaml.MappingNode {
return nil
}
for i := 0; i < len(node.Content)-1; i += 2 {
if node.Content[i].Kind == yaml.ScalarNode && node.Content[i].Value == key {
return resolveAlias(node.Content[i+1])
}
}
return nil
}

func scalarValue(node *yaml.Node) string {
if node == nil || node.Kind != yaml.ScalarNode {
return ""
}
return node.Value
}

// maxYAMLWalkDepth bounds recursion in walkYAMLNodes so a hostile or
Expand Down Expand Up @@ -517,11 +580,19 @@ func parseActionYAMLForUses(content []byte) ([]string, error) {
}

var uses []string
var invalid []string
for _, step := range action.Runs.Steps {
if step.Uses != "" {
switch {
case step.Uses == "":
case isUsesExpression(step.Uses):
invalid = append(invalid, strings.TrimSpace(step.Uses))
Comment thread
nodeselector marked this conversation as resolved.
default:
uses = append(uses, step.Uses)
}
}
if len(invalid) > 0 {
return nil, usesExpressionErr(invalid)
}
return uses, nil
}

Expand Down
76 changes: 74 additions & 2 deletions internal/workflowfile/workflowfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,32 @@ func TestExtractActionRefsMixed(t *testing.T) {

assert.Len(t, localPaths, 1)
assert.Equal(t, "./local-action", localPaths[0])
assert.Empty(t, warnings)
}

assert.Len(t, warnings, 1)
assert.Contains(t, warnings[0], "unparseable uses:")
func TestParseRejectsUsesExpression(t *testing.T) {
for name, content := range map[string]string{
"step level": `
on: push
jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: ${{ format('actions/checkout@v4') }}
`,
"job level": `
on: push
jobs:
call:
uses: owner/repo/.github/workflows/x.yml@${{ github.ref_name }}
`,
} {
t.Run(name, func(t *testing.T) {
_, err := Parse("wf.yml", []byte(content))
require.Error(t, err)
assert.Contains(t, err.Error(), "`uses:` can't contain an expression")
})
}
}

func TestExtractActionRefs_SelfRepositoryClassification(t *testing.T) {
Expand Down Expand Up @@ -382,3 +405,52 @@ func TestExtractLocalCompositeRefs_RejectsPathTraversal(t *testing.T) {
}
assert.True(t, sawRefusal, "expected refusal warning, got: %#v", warnings)
}

func TestParseIgnoresNonUsesKeys(t *testing.T) {
f, err := Parse("wf.yml", []byte(`
on: push
env:
uses: ${{ github.repository }}
jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
uses: ${{ github.ref }}
`))
require.NoError(t, err)
assert.Len(t, f.ExtractActionRefs().Refs, 1)
}

func TestParseRejectsAliasedUsesExpression(t *testing.T) {
_, err := Parse("wf.yml", []byte(`
on: push
x: &step
uses: ${{ matrix.action }}
jobs:
build:
runs-on: ubuntu-latest
steps:
- *step
`))
require.Error(t, err)
assert.Contains(t, err.Error(), "`uses:` can't contain an expression")
}

func TestScanSelfRepositoryActionsRejectsUsesExpression(t *testing.T) {
root := t.TempDir()
require.NoError(t, os.MkdirAll(filepath.Join(root, ".git"), 0o755))
require.NoError(t, os.MkdirAll(filepath.Join(root, "actions", "compo"), 0o755))
require.NoError(t, os.WriteFile(filepath.Join(root, "actions", "compo", "action.yml"), []byte(`
runs:
using: composite
steps:
- uses: actions/checkout@${{ inputs.ref }}
`), 0o644))

scan := ScanSelfRepositoryActions(filepath.Join(root, ".github", "workflows", "ci.yml"), []string{"$/actions/compo"})
require.Len(t, scan.Errors, 1)
assert.Contains(t, scan.Errors[0], "`uses:` can't contain an expression")
assert.Empty(t, scan.Refs)
}
9 changes: 5 additions & 4 deletions test/scenarios/catalog.yml
Original file line number Diff line number Diff line change
Expand Up @@ -414,8 +414,9 @@ scenarios:

- name: expression_in_uses
category: workflow_parsing
description: "Expression-based uses: (${{ }}) — unparseable, skipped with warning"
needs_token: true
description: "Expression-based uses: (${{ }}) — illegal in Actions, fails to parse"
needs_stub: true
tags: [stub]
fixtures:
workflows:
ci.yml:
Expand All @@ -429,8 +430,8 @@ scenarios:
- uses: ${{ matrix.action }}
- uses: actions/checkout@v4
expect:
exit: 0
lockfile_deps_cover_direct: true
exit: 2
output_contains: ["can't contain an expression"]

- name: local_action_skipped
category: workflow_parsing
Expand Down