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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions