report closer line numbers (#15)

models/github_actions.go

@@ -92,6 +92,8 @@ type GithubActionsStep struct {
 	WithScript       string            `json:"with_script" yaml:"-"`
 	Line             int               `json:"line" yaml:"-"`
 	Action           string            `json:"action" yaml:"-"`
+
+	Lines map[string]int `json:"lines" yaml:"-"`
 }
 
 type GithubActionsMetadata struct {
@@ -169,6 +171,8 @@ type GithubActionsJob struct {
 	Steps             GithubActionsSteps           `json:"steps"`
 	ReferencesSecrets []string                     `json:"references_secrets" yaml:"-"`
 	Line              int                          `json:"line" yaml:"-"`
+
+	Lines map[string]int `json:"lines" yaml:"-"`
 }
 
 type GithubActionsWorkflow struct {
@@ -201,13 +205,27 @@ func (o *GithubActionsJobs) UnmarshalYAML(node *yaml.Node) error {
 		job := GithubActionsJob{
 			ID:   name,
 			Line: node.Content[i].Line,
+			Lines: map[string]int{
+				"start": node.Content[i].Line,
+			},
 		}
 		err := value.Decode(&job)
-
 		if err != nil {
 			return err
 		}
 
+		for j := 0; j < len(value.Content); j += 2 {
+			key := value.Content[j].Value
+			value := value.Content[j+1]
+
+			switch key {
+			case "runs-on":
+				job.Lines["runs_on"] = value.Line
+			case "if":
+				job.Lines[key] = value.Line
+			}
+		}
+
 		*o = append(*o, job)
 	}
 
@@ -375,7 +393,8 @@ func (o *GithubActionsEnvs) UnmarshalYAML(node *yaml.Node) error {
 func (o *GithubActionsStep) UnmarshalYAML(node *yaml.Node) error {
 	type Alias GithubActionsStep
 	t := Alias{
-		Line: node.Line,
+		Line:  node.Line,
+		Lines: map[string]int{"start": node.Line},
 	}
 	err := node.Decode(&t)
 	if err != nil {
@@ -384,12 +403,31 @@ func (o *GithubActionsStep) UnmarshalYAML(node *yaml.Node) error {
 
 	*o = GithubActionsStep(t)
 
-	for _, param := range o.With {
-		switch param.Name {
-		case "ref":
-			o.WithRef = param.Value
-		case "script":
-			o.WithScript = param.Value
+	for i := 0; i < len(node.Content); i += 2 {
+		key := node.Content[i].Value
+		value := node.Content[i+1]
+
+		switch key {
+		case "uses", "run", "if":
+			o.Lines[key] = value.Line
+		case "with":
+			if value.Kind != yaml.MappingNode {
+				continue
+			}
+
+			for j := 0; j < len(value.Content); j += 2 {
+				name := value.Content[j].Value
+				arg := value.Content[j+1]
+
+				switch name {
+				case "ref":
+					o.Lines["with_ref"] = arg.Line
+					o.WithRef = arg.Value
+				case "script":
+					o.Lines["with_script"] = arg.Line
+					o.WithScript = arg.Value
+				}
+			}
 		}
 	}
 

models/github_actions_test.go

@@ -38,20 +38,23 @@ func TestGithubActionsWorkflowJobs(t *testing.T) {
 			Expected: GithubActionsJob{
 				ID:     "build",
 				RunsOn: []string{"ubuntu-latest"},
+				Lines:  map[string]int{"start": 1, "runs_on": 1},
 			},
 		},
 		{
 			Input: `build: {runs-on: { group: runner-group, labels: [runner-label] }}`,
 			Expected: GithubActionsJob{
 				ID:     "build",
 				RunsOn: []string{"group:runner-group", "label:runner-label"},
+				Lines:  map[string]int{"start": 1, "runs_on": 1},
 			},
 		},
 		{
 			Input: `build: {runs-on: { labels: runner-label }}`,
 			Expected: GithubActionsJob{
 				ID:     "build",
 				RunsOn: []string{"label:runner-label"},
+				Lines:  map[string]int{"start": 1, "runs_on": 1},
 			},
 		},
 		{
@@ -143,6 +146,9 @@ func TestGithubActionsWorkflowJobs(t *testing.T) {
 		} else {
 			assert.Nil(t, err)
 			c.Expected.Line = 1
+			if c.Expected.Lines == nil {
+				c.Expected.Lines = map[string]int{"start": c.Expected.Line}
+			}
 			assert.Equal(t, c.Expected, jobs[0])
 		}
 	}
@@ -396,7 +402,7 @@ jobs:
 	assert.Equal(t, "Build", workflow.Events[3].Workflows[0])
 
 	assert.Equal(t, "build", workflow.Jobs[0].ID)
-	assert.Equal(t, 33, workflow.Jobs[0].Line)
+	assert.Equal(t, 33, workflow.Jobs[0].Lines["start"])
 	assert.Equal(t, "Build job", workflow.Jobs[0].Name)
 	assert.Equal(t, "ubuntu-latest", workflow.Jobs[0].RunsOn[0])
 	assert.Equal(t, "windows-latest", workflow.Jobs[0].RunsOn[1])
@@ -418,9 +424,11 @@ jobs:
 	assert.Equal(t, "git pull", workflow.Jobs[0].Steps[0].Run)
 	assert.Equal(t, "/tmp", workflow.Jobs[0].Steps[0].WorkingDirectory)
 	assert.Equal(t, "ref", workflow.Jobs[0].Steps[0].With[0].Name)
+	assert.Equal(t, 50, workflow.Jobs[0].Steps[0].Lines["with_ref"])
 	assert.Equal(t, "${{ github.head_ref }}", workflow.Jobs[0].Steps[0].With[0].Value)
 	assert.Equal(t, "${{ github.head_ref }}", workflow.Jobs[0].Steps[0].WithRef)
 	assert.Equal(t, "script", workflow.Jobs[0].Steps[0].With[1].Name)
+	assert.Equal(t, 51, workflow.Jobs[0].Steps[0].Lines["with_script"])
 	assert.Equal(t, "console.log(1)", workflow.Jobs[0].Steps[0].With[1].Value)
 	assert.Equal(t, "console.log(1)", workflow.Jobs[0].Steps[0].WithScript)
 	assert.Equal(t, "GITHUB_TOKEN", workflow.Jobs[0].Steps[0].Env[0].Name)
@@ -479,4 +487,5 @@ runs:
 	assert.Equal(t, "checkout", actionMetadata.Runs.Steps[0].ID)
 	assert.Equal(t, "ref", actionMetadata.Runs.Steps[0].With[0].Name)
 	assert.Equal(t, "koi", actionMetadata.Runs.Steps[0].With[0].Value)
+	assert.Equal(t, 17, actionMetadata.Runs.Steps[0].Lines["uses"])
 }

opa/rego/rules/if_always_true.rego

@@ -1,8 +1,8 @@
 # METADATA
 # title: If condition always evaluates to true
 # description: |-
-#   GitHub Actions expressions used in if condition of jobs or steps 
-#   must not contain extra characters or spaces. 
+#   GitHub Actions expressions used in if condition of jobs or steps
+#   must not contain extra characters or spaces.
 #   Otherwise, the condition is always true.
 # custom:
 #   level: error
@@ -31,7 +31,7 @@ always_true(cond) if {
 
 if_conditions[pkg.purl] contains {
 	"path": workflow.path,
-	"line": job.line,
+	"line": object.get(job.lines, "if", 0),
 	"job": job.id,
 } if {
 	pkg := input.packages[_]
@@ -44,7 +44,7 @@ if_conditions[pkg.purl] contains {
 
 if_conditions[pkg.purl] contains {
 	"path": workflow.path,
-	"line": step.line,
+	"line": object.get(step.lines, "if", 0),
 	"job": job.id,
 	"step": step_id,
 } if {
@@ -59,7 +59,7 @@ if_conditions[pkg.purl] contains {
 
 if_conditions[pkg.purl] contains {
 	"path": action.path,
-	"line": step.line,
+	"line": object.get(step.lines, "if", 0),
 	"step": step_id,
 } if {
 	pkg := input.packages[_]

opa/rego/rules/injection.rego

@@ -23,13 +23,13 @@ gh_injections(str) = {expr |
 	expr := regex.find_all_string_submatch_n("\\$\\{\\{\\s*([^}]+?)\\s*\\}\\}", match, 1)[0][1]
 }
 
-gh_step_injections(step) = gh_injections(step.with_script) if {
+gh_step_injections(step) = [gh_injections(step.with_script), step.lines.with_script] if {
 	startswith(step.uses, "actions/github-script@")
-} else = gh_injections(step.run)
+} else = [gh_injections(step.run), step.lines.run]
 
 results contains poutine.finding(rule, pkg.purl, {
 	"path": workflow.path,
-	"line": step.line,
+	"line": line,
 	"job": job.id,
 	"step": i,
 	"details": sprintf("Sources: %s", [concat(" ", exprs)]),
@@ -38,21 +38,21 @@ results contains poutine.finding(rule, pkg.purl, {
 	workflow = pkg.github_actions_workflows[_]
 	job := workflow.jobs[_]
 	step := job.steps[i]
-	exprs := gh_step_injections(step)
+	[exprs, line] := gh_step_injections(step)
 	count(exprs) > 0
 }
 
 results contains poutine.finding(rule, pkg.purl, {
 	"path": action.path,
-	"line": step.line,
+	"line": line,
 	"step": i,
 	"details": sprintf("Sources: %s", [concat(" ", exprs)]),
 }) if {
 	pkg = input.packages[_]
 	action := pkg.github_actions_metadata[_]
 	step := action.runs.steps[i]
 	action.runs.using == "composite"
-	exprs := gh_step_injections(step)
+	[exprs, line] := gh_step_injections(step)
 	count(exprs) > 0
 }
 

opa/rego/rules/job_all_secrets.rego

@@ -17,7 +17,7 @@ rule := poutine.rule(rego.metadata.chain())
 results contains poutine.finding(rule, pkg.purl, {
 	"path": workflow.path,
 	"job": job.id,
-	"line": job.line,
+	"line": job.lines.start,
 }) if {
 	pkg := input.packages[_]
 	workflow := pkg.github_actions_workflows[_]

opa/rego/rules/known_vulnerability.rego

@@ -29,7 +29,7 @@ step_advisory(step) = advisory if {
 
 results contains poutine.finding(rule, pkg.purl, {
 	"path": workflow.path,
-	"line": step.line,
+	"line": step.lines.uses,
 	"job": job.id,
 	"step": i,
 	"osv_id": advisory.osv_id,
@@ -44,7 +44,7 @@ results contains poutine.finding(rule, pkg.purl, {
 
 results contains poutine.finding(rule, pkg.purl, {
 	"path": action.path,
-	"line": step.line,
+	"line": step.lines.uses,
 	"step": i,
 	"osv_id": advisory.osv_id,
 	"details": sprintf("Package: %s", [advisory.package_name]),

opa/rego/rules/pr_runs_on_self_hosted.rego

@@ -1,7 +1,7 @@
 # METADATA
 # title: Pull Request Runs on Self-Hosted GitHub Actions Runner
 # description: |-
-#   This job runs on a self-hosted GitHub Actions runner in a workflow 
+#   This job runs on a self-hosted GitHub Actions runner in a workflow
 #   that is triggered by a pull request event.
 # custom:
 #   level: warning
@@ -16,7 +16,7 @@ rule := poutine.rule(rego.metadata.chain())
 results contains poutine.finding(rule, pkg.purl, {
 	"path": workflow.path,
 	"job": job.id,
-	"line": job.line,
+	"line": job.lines.runs_on,
 	"details": sprintf("runs-on: %s", [concat(", ", job.runs_on)]),
 }) if {
 	pkg := input.packages[_]

scanner/inventory_test.go

@@ -90,7 +90,7 @@ func TestFindings(t *testing.T) {
 				Job:     "build",
 				Path:    ".github/workflows/valid.yml",
 				Step:    "1",
-				Line:    19,
+				Line:    20,
 				Details: "Sources: github.head_ref",
 			},
 		},
@@ -101,7 +101,7 @@ func TestFindings(t *testing.T) {
 				Job:     "build",
 				Path:    ".github/workflows/valid.yml",
 				Step:    "7",
-				Line:    45,
+				Line:    46,
 				Details: "Sources: github.event.workflow_run.head_branch",
 			},
 		},
@@ -124,7 +124,7 @@ func TestFindings(t *testing.T) {
 				Job:     "build",
 				Step:    "5",
 				OsvId:   "GHSA-f9qj-7gh3-mhj4",
-				Line:    38,
+				Line:    39,
 				Details: "Package: kartverket/github-workflows/.github/workflows/run-terraform.yml",
 			},
 		},
@@ -136,7 +136,7 @@ func TestFindings(t *testing.T) {
 				Job:     "build",
 				Step:    "6",
 				OsvId:   "GHSA-f9qj-7gh3-mhj4",
-				Line:    42,
+				Line:    43,
 				Details: "Package: kartverket/github-workflows/.github/workflows/run-terraform.yml",
 			},
 		},
@@ -194,7 +194,7 @@ func TestFindings(t *testing.T) {
 			Meta: opa.FindingMeta{
 				Path:    ".github/workflows/valid.yml",
 				Job:     "build",
-				Line:    8,
+				Line:    9,
 				Details: "runs-on: self-hosted"},
 		},
 		{
@@ -211,7 +211,7 @@ func TestFindings(t *testing.T) {
 				Job:     "build",
 				Path:    ".github/workflows/valid.yml",
 				Step:    "8",
-				Line:    49,
+				Line:    50,
 				Details: "Sources: github.event.client_payload.foo",
 			},
 		},
@@ -251,6 +251,15 @@ func TestFindings(t *testing.T) {
 				Job:  "json",
 			},
 		},
+		{
+			RuleId: "if_always_true",
+			Purl:   purl,
+			Meta: opa.FindingMeta{
+				Path: "composite/action.yml",
+				Line: 17,
+				Step: "3",
+			},
+		},
 	}
 
 	assert.Equal(t, len(findings), len(results.Findings))

scanner/testdata/composite/action.yml

@@ -11,3 +11,8 @@ runs:
 
     # GHSA-4mgv-m5cm-f9h7
     - uses: hashicorp/[email protected]
+
+    - id: if-always-true
+      run: ls
+      if: |
+        ${{ github.event == 'not-checked' }}