Skip to content

Analyze fork-sonar.yml untrusted-fork-checkout finding (Sonar S7631) against existing fork-CI security gates #232

Description

@AndreasIgel

Summary

SonarCloud reports new-code Security rating = C on main (failing the quality gate) from a single githubactions:S7631 finding:

.github/workflows/fork-sonar.yml:68Make sure that no untrusted code is executed from a fork.

This is an analysis task, not a quick fix: the naive remediation (stop checking out the fork source) interacts with the other fork-CI security gates already in place, so we need to analyse the trade-offs before changing anything.

The finding

fork-sonar.yml runs on workflow_run in the privileged base-repo context with SONAR_TOKEN. It checks out the fork PR head:

- name: Check out fork PR source (from the base repo's PR ref)
  uses: actions/checkout@... # v7.0.1
  with:
    ref: refs/pull/${{ steps.meta.outputs.pr_number }}/head

and later runs mvn ... sonar:sonar on that checkout. Because Maven parses the fork-controlled pom.xml (and could load .mvn/extensions.xml, build extensions, or plugin config), untrusted code could execute while the Sonar token is present. That is the classic "pwn-request" pattern S7631 flags.

Existing mitigations (the reason this needs analysis, not a blind fix)

  • The workflow is gated by the fork-ci GitHub Environment with Required reviewers — a maintainer must approve each run. (Comment in the file: "Do NOT remove that environment gate.")
  • The unprivileged Java CI with Maven workflow does the build/test/generated-source check; this workflow is documented to restore analysis inputs, not rebuild/retest fork code with the token.
  • PR metadata is extracted from the fork artifact with strict validation to prevent $GITHUB_OUTPUT injection.

So the risk is already reduced, and any change must not weaken these gates or the codecov/dependency-review paths that share the same fork-CI design.

Analysis to perform

  1. Confirm whether running mvn sonar:sonar on the checked-out fork pom.xml can actually execute fork-controlled code despite the required-reviewer gate (extensions, plugin executions, wrapper scripts).
  2. Evaluate remediation options and their interaction with the other gates:
    • (a) Restore source from the trusted artifact instead of checking out refs/pull/N/head: have the unprivileged build package the analysed source into sonar-analysis-inputs, and here restore it alongside the already-restored compiled classes + jacoco — eliminating the untrusted checkout entirely (matches this workflow's "restore, don't rebuild" design). Verify Sonar still maps issues to source lines.
    • (b) Run the scanner without invoking Maven on the fork pom (e.g. sonar-scanner CLI against restored classes + reports), removing pom execution.
    • (c) Accept + mark "safe" in SonarCloud given the required-reviewer gate — document the justification; requires Sonar project admin.
  3. Recommend one option, considering safety, reproducibility, and not regressing the fork-CI / codecov / dependency-review security model.

Acceptance criteria

  • Documented analysis + decision.
  • Chosen remediation implemented (or an explicit, justified "mark safe") such that SonarCloud new-code Security rating = A without weakening the fork-CI required-reviewer gate or the other fork-safe workflows.
  • Validated against a real fork PR run.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions