Stop executing fork-controlled Maven in the privileged fork Sonar workflow (#232)

[devin-ai-integration]

Summary

Fixes the SonarCloud S7631 finding on .github/workflows/fork-sonar.yml (new-code Security rating C) by removing every execution of fork-controlled content from the privileged workflow_run job, while keeping the fork-ci required-reviewer gate and the artifact-based design unchanged.

Analysis (issue java-helpers#232, step 1)

The finding is real, not a false positive: the old workflow checked out refs/pull/N/head and ran mvn ... sonar-maven-plugin:sonar --file pom.xml with SONAR_TOKEN in the environment. Even with -DskipTests, Maven on a fork tree executes code the fork controls:

• .mvn/extensions.xml / <build><extensions> — arbitrary jars loaded into the Maven process
• .mvn/maven.config / .mvn/jvm.config — arbitrary flags, e.g. -javaagent:
• pom.xml plugin bindings — any plugin/execution bound to phases run by the Sonar goal (it forks a build), or <pluginRepositories> pointing at attacker-hosted plugins
• mvnw wrapper scripts (not used today, but trivially added by a PR)

The required-reviewer gate reduces likelihood but not impact: a maintainer approving a plausible-looking PR cannot review .mvn/ config or plugin coordinates for exfiltration intent. The rule flags the pattern for exactly that reason.

Options considered (step 2)

Option	Verdict
(a) Ship sources in the artifact, no checkout at all	Sonar PR analysis needs SCM data to compute changed lines / blame; without .git it warns SCM provider autodetection failed and reports 0 new lines, so PR decoration and the new-code gate become meaningless. Also duplicates data already present in refs/pull/N/head. Rejected.
(b) Standalone scanner, no Maven on the fork pom	Eliminates all code-execution vectors above; keeps SCM info; fits the "restore, don't rebuild" design. Chosen.
(c) Mark "safe" in SonarCloud	Would leave a genuine pwn-request path in place and depend on the human gate alone. Rejected.

Key insight: the problem was never checking out the fork tree (that only writes files), it was running a build tool on it. The fix therefore separates the two.

Implementation (step 3)

fork-sonar.yml now:

checkout            → base repo default branch (trusted), fetch-depth 0
mvn (trusted pom)   → -pl core install; -pl processor dependency:build-classpath
                      (excludes io.github.java-helpers so no stale/self jars)
git fetch           → refs/pull/N/head ; assert FETCH_HEAD == pr_head_sha from artifact
git checkout        → --detach FETCH_HEAD  (files + SCM history only, nothing run)
restore             → classes, test-classes, jacoco xml from artifact
write               → $RUNNER_TEMP/sonar-project.properties (mirrors pom <properties>)
sonarqube-scan-action → -Dproject.settings=$RUNNER_TEMP/... -Dsonar.pullrequest.*

Why each piece matters:

• Maven only ever sees the trusted pom.xml: it runs before the fork tree exists in the workspace. The classpath it produces is what gives the Java analyzer full type resolution; a dependency added by the fork PR is simply unresolved (analysis degrades, nothing is downloaded on the fork's behalf).
• pr_head_sha is now validated (^[0-9a-f]{40}$) and asserted against refs/pull/N/head, so the analysed sources match the artifact's bytecode even if the PR was force-pushed between the CI run and the approval.
• project.settings outside the workspace — with it set, the scanner ignores any sonar-project.properties in the fork tree. This matters because that file can set sonar.scanner.javaExePath (i.e. "run this binary"), which would otherwise be a new fork-controlled execution vector.
• maven.yml additionally ships core/processor target/test-classes so the standalone scanner gets sonar.java.test.binaries (previously provided implicitly by the Maven plugin).

Sonar properties duplicated from the root pom.xml are called out in a comment and in docs/CONTRIBUTING.md; the docs section now records the security model and this decision.

Validation

• actionlint clean on both workflows.
• Classpath resolution command verified locally against main (30 jars, no simple-builders-* self-references).
• Not yet validated against a real fork PR run — that requires a PR from a fork after this lands on main (workflow_run uses the base-branch workflow file) plus a maintainer approval in the fork-ci environment. Please check the SonarCloud PR decoration shows changed lines and coverage on the first such run.

Closes java-helpers#232.

Link to Devin session: https://app.devin.ai/sessions/d691231fba0842a4b842b275ca4b35ed
Open in Devin Desktop: https://app.devin.ai/desktop/session/d691231fba0842a4b842b275ca4b35ed?variant=devin
Requested by: @AndreasIgel

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[AndreasIgel]

moved to java-helpers#285