Merge branch 'main' into fix/error-message

Author: richards-ensono

.github/workflows/trusted-sonarcloud-pr.yml

@@ -17,9 +17,10 @@ jobs:
       github.event.workflow_run.repository.full_name == github.repository
     runs-on: ubuntu-24.04
     concurrency:
-      # One group per PR lets a newer revision cancel stale analysis. The helper
-      # still binds every materialized object to this run's verified head SHA.
-      group: sonar-pr-${{ github.event.workflow_run.pull_requests[0].number }}
+      # GitHub can omit pull_requests for fork runs. Use the PR number when it is
+      # present and a stable fork-repository/branch identity otherwise so newer
+      # revisions cancel stale analysis without colliding with unrelated forks.
+      group: sonar-pr-${{ github.event.workflow_run.pull_requests[0].number || format('{0}-{1}', github.event.workflow_run.head_repository.id, github.event.workflow_run.head_branch) }}
       cancel-in-progress: true
     steps:
       # These helpers come only from protected main. Pull-request source is never
@@ -61,22 +62,54 @@ jobs:
              .event == $event and .repository.full_name == $repository and
              (.head_sha | test("^[0-9a-f]{40}$")) and
              (.head_repository.full_name | type == "string" and length > 0) and
-             (.pull_requests | type == "array" and length == 1)' <<<"$run" >/dev/null
+             (.head_repository.owner.login | type == "string" and length > 0) and
+             (.head_branch | type == "string" and length > 0) and
+             (.pull_requests | type == "array" and (length == 0 or length == 1))' <<<"$run" >/dev/null
 
-          pr_number="$(jq -er '.pull_requests[0].number | select(type == "number" and . > 0)' <<<"$run")"
           head_sha="$(jq -er '.head_sha' <<<"$run")"
           run_head_repository="$(jq -er '.head_repository.full_name' <<<"$run")"
+          head_owner="$(jq -er '.head_repository.owner.login' <<<"$run")"
+          head_ref="$(jq -er '.head_branch' <<<"$run")"
+          associated_pr_count="$(jq -er '.pull_requests | length' <<<"$run")"
+
+          if (( associated_pr_count == 1 )); then
+            pr_number="$(jq -er '.pull_requests[0].number | select(type == "number" and . > 0)' <<<"$run")"
+          else
+            candidates="$(gh api --method GET "repos/${repository}/pulls" \
+              -f state=open \
+              -f base="$expected_branch" \
+              -f head="${head_owner}:${head_ref}" \
+              -f per_page=2)"
+            pr_number="$(jq -er \
+              --arg repository "$repository" \
+              --arg branch "$expected_branch" \
+              --arg head_repository "$run_head_repository" \
+              --arg head_ref "$head_ref" \
+              --arg sha "$head_sha" \
+              'if length != 1 then
+                 error("expected exactly one open pull request for verified workflow run head")
+               elif (.[0].state != "open" or
+                     .[0].base.repo.full_name != $repository or .[0].base.ref != $branch or
+                     .[0].head.repo.full_name != $head_repository or .[0].head.ref != $head_ref or
+                     .[0].head.sha != $sha) then
+                 error("pull request candidate does not match verified workflow run head")
+               else
+                 .[0].number | select(type == "number" and . > 0)
+               end' <<<"$candidates")"
+          fi
+
           pull_request="$(gh api "repos/${repository}/pulls/${pr_number}")"
           jq -e \
             --arg repository "$repository" \
             --arg branch "$expected_branch" \
             --arg head_repository "$run_head_repository" \
+            --arg head_ref "$head_ref" \
             --arg sha "$head_sha" \
             --argjson pr_number "$pr_number" \
-            '.number == $pr_number and .base.repo.full_name == $repository and
-             .base.ref == $branch and .head.repo.full_name == $head_repository and
+            '.number == $pr_number and .state == "open" and
+             .base.repo.full_name == $repository and .base.ref == $branch and
+             .head.repo.full_name == $head_repository and .head.ref == $head_ref and
              .head.sha == $sha' <<<"$pull_request" >/dev/null
-          head_ref="$(jq -er '.head.ref | select(type == "string" and length > 0)' <<<"$pull_request")"
 
           artifacts="$(gh api "repos/${repository}/actions/runs/${RUN_ID}/artifacts?per_page=100")"
           artifact_id="$(jq -er --arg name "$expected_artifact" --arg sha "$head_sha" --argjson run_id "$RUN_ID" \

docs/ci-security.md

@@ -41,13 +41,13 @@ The `Lint and Test` workflow's **Advisory workflow policy feedback (not required
 
 ## Pull-request reporting
 
-The pull-request execution job is intentionally limited to `contents: read` and does not receive `SONAR_TOKEN`, a protected environment, or another privileged credential. It uploads the inert JUnit report for check publication and selects only `.coverage/out` and `.coverage/report-junit.xml` for a deterministic seven-day `sonar-reports-<run-id>-<attempt>` artifact. GitHub's artifact action strips their common `.coverage` parent, so the downloaded artifact contract is exactly two root-level regular files named `out` and `report-junit.xml`. After validating names, types, and size bounds, protected code normalizes those files under `reports/.coverage/` so the trusted scanner keeps its established report paths without broadening the accepted artifact surface. Missing coverage is a visible failed Sonar preparation result; it is never a silently skipped analysis.
+The pull-request execution job is intentionally limited to `contents: read` and does not receive `SONAR_TOKEN`, a protected environment, or another privileged credential. It uploads the inert JUnit report for check publication and selects only `.coverage/out` and `.coverage/report-junit.xml` for a deterministic seven-day `sonar-reports-<run-id>-<attempt>` artifact. GitHub's artifact action strips their common `.coverage` parent, so the downloaded artifact contract is exactly two root-level regular files named `out` and `report-junit.xml`. After validating names, types, and size bounds, protected code requires canonical repository-relative Go coverage records, prefixes each record path with the fixed `source/` scanner namespace used by API materialization, and normalizes both files under `reports/.coverage/`. This preserves the established report locations and aligns coverage keys with `analysis/source` without broadening the accepted artifact surface. Missing or malformed coverage is a visible failed Sonar preparation result; it is never a silently skipped analysis.
 
 ## Trusted SonarCloud pull-request analysis
 
 `Trusted SonarCloud pull-request analysis` is a protected-default-branch `workflow_run` workflow for completed `Lint and Test` pull-request runs. It is the only PR path that can receive `SONAR_TOKEN`. The workflow has only `contents: read` permission, uses no job container or services, restores or saves no cache, and supports both same-repository and fork pull requests without executing either source tree.
 
-Before any secret-bearing step, protected code resolves and binds the expected workflow and event, base repository and `main` branch, verified head repository (including fork identity), pull-request number, full current head SHA, run ID, run attempt, and exactly one unexpired `sonar-reports-<run-id>-<attempt>` artifact. The report download remains tied to that run and revision. Its protected validator accepts only the bounded regular coverage and JUnit files and rejects missing reports, oversized content, symlinks, special files, traversal-derived paths, and unexpected entries.
+Before any secret-bearing step, protected code resolves and binds the expected workflow and event, base repository and `main` branch, verified head repository (including fork identity), pull-request number, full current head SHA, run ID, run attempt, and exactly one unexpired `sonar-reports-<run-id>-<attempt>` artifact. GitHub may omit pull-request associations from a fork-originated Actions run. In that case, protected code queries only open pull requests targeting `Ensono/eirctl:main` by the verified fork owner and exact head branch, requires exactly one result, and revalidates its state, base repository/ref, head repository/ref, and immutable SHA against the run. Missing, ambiguous, closed, stale, or mismatched candidates fail closed before artifact download. Concurrency uses the event pull-request number when present and otherwise a stable workflow-run head-repository/branch identity, so unrelated forks do not collide while newer revisions cancel stale analysis. The report download remains tied to that run and revision. Its protected validator accepts only bounded regular UTF-8 coverage and JUnit files and rejects missing reports, malformed coverage modes or records, unsafe coverage paths, invalid encoding, oversized content, symlinks, special files, traversal-derived paths, and unexpected entries.
 
 Pull-request source is never passed to `actions/checkout`, `git checkout`, `git fetch`, an archive extractor, or another source action. After writing trusted `analysis/sonar-project.properties` outside the source root, the protected standard-library helper resolves the exact head commit and root tree through the verified head repository's Git Data API, requires a complete non-truncated recursive tree, validates every canonical path and mode, and retrieves each selected regular non-executable `.go` blob by its tree-recorded SHA. It verifies the API identity, declared and decoded size, and Git blob identity before making an exclusive `0644` write beneath the newly created `analysis/source` root. Symlinks, submodules, special or unknown entries, unsafe or duplicate paths, executable `.go` files, non-Go files, and changed PR heads fail closed or remain unmaterialized. The helper rechecks the current PR head after all writes.
 

internal/schema/github.go

@@ -46,49 +46,22 @@ func (events *GithubTriggerEvents) Has(name string) bool {
 // scalar event, a sequence of events, or a mapping containing event options.
 func (events *GithubTriggerEvents) UnmarshalYAML(node *yaml.Node) error {
 	*events = GithubTriggerEvents{configured: map[string]struct{}{}}
-	configure := func(name string, value *yaml.Node) error {
-		events.configured[name] = struct{}{}
-		if value == nil || value.Tag == "!!null" {
-			return nil
-		}
-		switch name {
-		case "push":
-			return value.Decode(&events.Push)
-		case "pull_request":
-			return value.Decode(&events.PullRequest)
-		case "pull_request_target":
-			return value.Decode(&events.PullRequestTarget)
-		case "issue_comment":
-			return value.Decode(&events.IssueComment)
-		case "repository_dispatch":
-			return value.Decode(&events.RepositoryDispatch)
-		case "schedule":
-			return value.Decode(&events.Schedule)
-		case "workflow_dispatch":
-			return value.Decode(&events.WorkflowDispatch)
-		case "workflow_run":
-			return value.Decode(&events.WorkflowRun)
-		default:
-			return nil
-		}
-	}
-
 	switch node.Kind {
 	case yaml.ScalarNode:
-		return configure(node.Value, nil)
+		return events.configure(node.Value, nil)
 	case yaml.SequenceNode:
 		for _, value := range node.Content {
 			if value.Kind != yaml.ScalarNode {
 				return fmt.Errorf("github trigger: expected an event name but got %v", value.Kind)
 			}
-			if err := configure(value.Value, nil); err != nil {
+			if err := events.configure(value.Value, nil); err != nil {
 				return err
 			}
 		}
 		return nil
 	case yaml.MappingNode:
 		for i := 0; i+1 < len(node.Content); i += 2 {
-			if err := configure(node.Content[i].Value, node.Content[i+1]); err != nil {
+			if err := events.configure(node.Content[i].Value, node.Content[i+1]); err != nil {
 				return fmt.Errorf("github trigger %s: %w", node.Content[i].Value, err)
 			}
 		}
@@ -98,6 +71,33 @@ func (events *GithubTriggerEvents) UnmarshalYAML(node *yaml.Node) error {
 	}
 }
 
+func (events *GithubTriggerEvents) configure(name string, value *yaml.Node) error {
+	events.configured[name] = struct{}{}
+	if value == nil || value.Tag == "!!null" {
+		return nil
+	}
+	switch name {
+	case "push":
+		return value.Decode(&events.Push)
+	case "pull_request":
+		return value.Decode(&events.PullRequest)
+	case "pull_request_target":
+		return value.Decode(&events.PullRequestTarget)
+	case "issue_comment":
+		return value.Decode(&events.IssueComment)
+	case "repository_dispatch":
+		return value.Decode(&events.RepositoryDispatch)
+	case "schedule":
+		return value.Decode(&events.Schedule)
+	case "workflow_dispatch":
+		return value.Decode(&events.WorkflowDispatch)
+	case "workflow_run":
+		return value.Decode(&events.WorkflowRun)
+	default:
+		return nil
+	}
+}
+
 // PushEvent represents a push event trigger configuration.
 type GithubPushEvent struct {
 	Branches       []string `json:"branches,omitempty" yaml:"branches,omitempty"`
@@ -226,21 +226,26 @@ func (job *GithubJob) UnmarshalYAML(node *yaml.Node) error {
 		return fmt.Errorf("github job: expected a mapping node but got %v", node.Kind)
 	}
 	values := yamlMapping(node)
-	decode := func(name string, target any) error {
-		value := values[name]
-		if value == nil {
-			return nil
-		}
-		if err := value.Decode(target); err != nil {
-			return fmt.Errorf("github job field %s: %w", name, err)
-		}
-		return nil
-	}
-
 	*job = GithubJob{configured: make(map[string]struct{}, len(values))}
 	for name := range values {
 		job.configured[name] = struct{}{}
 	}
+	if err := decodeGithubJobFields(values, job); err != nil {
+		return err
+	}
+	if err := decodeGithubJobNeeds(values["needs"], job); err != nil {
+		return err
+	}
+	if err := decodeGithubJobEnvironment(values["environment"], job); err != nil {
+		return err
+	}
+	if err := decodeGithubJobContainer(values["container"], job); err != nil {
+		return err
+	}
+	return validateGithubJobSteps(job.Steps)
+}
+
+func decodeGithubJobFields(values map[string]*yaml.Node, job *GithubJob) error {
 	for name, target := range map[string]any{
 		"name":            &job.Name,
 		"runs-on":         &job.RunsOn,
@@ -253,47 +258,70 @@ func (job *GithubJob) UnmarshalYAML(node *yaml.Node) error {
 		"services":        &job.Services,
 		"concurrency":     &job.Concurrency,
 	} {
-		if err := decode(name, target); err != nil {
-			return err
+		value := values[name]
+		if value == nil {
+			continue
+		}
+		if err := value.Decode(target); err != nil {
+			return fmt.Errorf("github job field %s: %w", name, err)
 		}
 	}
+	return nil
+}
 
-	if needs := values["needs"]; needs != nil {
-		switch needs.Kind {
-		case yaml.ScalarNode:
-			job.Needs = []string{needs.Value}
-		case yaml.SequenceNode:
-			if err := needs.Decode(&job.Needs); err != nil {
-				return fmt.Errorf("github job field needs: %w", err)
-			}
-		default:
-			return fmt.Errorf("github job field needs: expected a scalar or sequence but got %v", needs.Kind)
+func decodeGithubJobNeeds(needs *yaml.Node, job *GithubJob) error {
+	if needs == nil {
+		return nil
+	}
+	switch needs.Kind {
+	case yaml.ScalarNode:
+		job.Needs = []string{needs.Value}
+		return nil
+	case yaml.SequenceNode:
+		if err := needs.Decode(&job.Needs); err != nil {
+			return fmt.Errorf("github job field needs: %w", err)
 		}
+		return nil
+	default:
+		return fmt.Errorf("github job field needs: expected a scalar or sequence but got %v", needs.Kind)
 	}
+}
 
-	if environment := values["environment"]; environment != nil {
-		if environment.Kind == yaml.ScalarNode {
-			job.Environment = environment.Value
-		} else {
-			var configured struct {
-				Name string `yaml:"name"`
-			}
-			if err := environment.Decode(&configured); err != nil {
-				return fmt.Errorf("github job field environment: %w", err)
-			}
-			job.Environment = configured.Name
-		}
+func decodeGithubJobEnvironment(environment *yaml.Node, job *GithubJob) error {
+	if environment == nil {
+		return nil
+	}
+	if environment.Kind == yaml.ScalarNode {
+		job.Environment = environment.Value
+		return nil
+	}
+	var configured struct {
+		Name string `yaml:"name"`
 	}
+	if err := environment.Decode(&configured); err != nil {
+		return fmt.Errorf("github job field environment: %w", err)
+	}
+	job.Environment = configured.Name
+	return nil
+}
 
-	if container := values["container"]; container != nil && container.Tag != "!!null" {
-		job.Container = &GithubContainer{}
-		if container.Kind == yaml.ScalarNode {
-			job.Container.Image = container.Value
-		} else if err := container.Decode(job.Container); err != nil {
-			return fmt.Errorf("github job field container: %w", err)
-		}
+func decodeGithubJobContainer(container *yaml.Node, job *GithubJob) error {
+	if container == nil || container.Tag == "!!null" {
+		return nil
+	}
+	job.Container = &GithubContainer{}
+	if container.Kind == yaml.ScalarNode {
+		job.Container.Image = container.Value
+		return nil
+	}
+	if err := container.Decode(job.Container); err != nil {
+		return fmt.Errorf("github job field container: %w", err)
 	}
-	for index, step := range job.Steps {
+	return nil
+}
+
+func validateGithubJobSteps(steps []*GithubStep) error {
+	for index, step := range steps {
 		if step == nil {
 			return fmt.Errorf("github job step %d must be a mapping", index)
 		}

internal/schema/github_test.go

@@ -50,20 +50,26 @@ func TestGithubWorkflowUnmarshalTriggerForms(t *testing.T) {
 			if err := yaml.Unmarshal([]byte(contents), &workflow); err != nil {
 				t.Fatal(err)
 			}
-			for _, trigger := range tt.triggers {
-				if !workflow.On.Has(trigger) {
-					t.Errorf("On.Has(%q) = false", trigger)
-				}
-			}
-			if workflow.On.Has("workflow_run") {
-				if got := workflow.On.WorkflowRun.Workflows; len(got) != 1 || got[0] != "Lint and Test" {
-					t.Fatalf("WorkflowRun.Workflows = %#v", got)
-				}
-			}
+			assertGithubTriggers(t, workflow.On, tt.triggers)
 		})
 	}
 }
 
+func assertGithubTriggers(t *testing.T, events *GithubTriggerEvents, triggers []string) {
+	t.Helper()
+	for _, trigger := range triggers {
+		if !events.Has(trigger) {
+			t.Errorf("On.Has(%q) = false", trigger)
+		}
+	}
+	if !events.Has("workflow_run") {
+		return
+	}
+	if got := events.WorkflowRun.Workflows; len(got) != 1 || got[0] != "Lint and Test" {
+		t.Fatalf("WorkflowRun.Workflows = %#v", got)
+	}
+}
+
 func TestGithubJobUnmarshalPolicyFields(t *testing.T) {
 	contents := []byte(`on: [push]
 jobs: