Skip to content

chore(build): bind spotbugs:check + dependency-check:check to mvn verify (RAN-24)#72

Closed
aksOps wants to merge 2 commits into
mainfrom
chore/ran-24-bind-quality-gates
Closed

chore(build): bind spotbugs:check + dependency-check:check to mvn verify (RAN-24)#72
aksOps wants to merge 2 commits into
mainfrom
chore/ran-24-bind-quality-gates

Conversation

@aksOps
Copy link
Copy Markdown
Contributor

@aksOps aksOps commented Apr 24, 2026

Summary

Restores CI enforcement of the SpotBugs and OWASP dependency-check quality gates by binding their check goals to the verify phase. Previously both plugins were declared in pom.xml without <executions>, so mvn clean verify (the command run by .github/workflows/ci-java.yml) silently skipped them — which is how the 12 SpotBugs findings cleaned up in RAN-23 accumulated on main undetected.

Closes RAN-24. Depends on RAN-23 (PR #71, merged) — without those fixes, this PR's gate would fail on the merge.

Changes

  • pom.xml:
    • spotbugs-maven-plugin: bind check to verify (id spotbugs-verify).
    • dependency-check-maven: bind check to verify (id dependency-check-verify), failBuildOnCVSS=7, suppression file pointer.
    • New <dependency-check.skip> property (default false); override locally with -Ddependency-check.skip=true to skip the ~1GB NVD download.
  • dependency-check-suppressions.xml (new): empty baseline stub with policy comment ("every entry MUST have a CVE id, a date, and a one-line rationale").
  • .github/workflows/ci-java.yml: one-line comment documenting that verify is now the enforced gate.

Verification

Against origin/main HEAD 8f1ce18:

mvn -B clean verify -DskipTests=true -Ddependency-check.skip=true

Result: BUILD SUCCESS, exit 0.

[INFO] --- spotbugs-maven-plugin:4.9.8.3:check (spotbugs-verify) @ code-iq ---
[INFO] BugInstance size is 0
[INFO] Error size is 0
[INFO] No errors/warnings found

[INFO] --- dependency-check-maven:12.2.0:check (dependency-check-verify) @ code-iq ---
[INFO] Skipping dependency-check

Both new executions fire by their declared id (spotbugs-verify, dependency-check-verify), proving the bindings are wired correctly. The dependency-check Skipping line confirms the new <dependency-check.skip> property override works.

Test plan

  • CI on this PR runs mvn clean verify -B and the spotbugs+depcheck executions appear in the build log.
  • CI is green (no new SpotBugs findings post-RAN-23, no >=CVSS 7 dependency CVEs).
  • After merge, future regressions in either gate fail main CI immediately.

🤖 Generated with Claude Code

aksOps and others added 2 commits April 24, 2026 21:37
…ify (RAN-24)

Both quality plugins were declared without `<executions>` so `mvn clean verify`
(the command run by .github/workflows/ci-java.yml) silently skipped them. This
is how 12 SpotBugs findings (RAN-23) accumulated on main without CI noticing.

Adds:
- spotbugs-maven-plugin: bind `check` goal to `verify` phase.
- dependency-check-maven: bind `check` goal to `verify` phase, with
  `failBuildOnCVSS=7`, suppression file pointer, and a `<dependency-check.skip>`
  property (default false) so local devs can opt out of the slow NVD download
  via `-Ddependency-check.skip=true`.
- dependency-check-suppressions.xml: empty baseline stub with policy comment.
- ci-java.yml comment documenting that `verify` is the enforced gate.

Verification (against origin/main HEAD 8f1ce18):
  mvn -B clean verify -DskipTests=true -Ddependency-check.skip=true
  exit=0
  spotbugs-maven-plugin:check (spotbugs-verify): BugInstance size is 0
  dependency-check-maven:check (dependency-check-verify): Skipping dependency-check

Closes RAN-24.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
Per reviewer feedback on #72: the OWASP dependency-check binding added in
15b5e6c red-lines every CI run because the public NVD 2.0 API rate-limits
anonymous callers (HTTP 429), which corrupts the partial H2 NVD store and
trips NPEs in the plugin's worker pool. Root cause is infra, not a real
CVE finding, but the failing binding still breaks the build - the opposite
of "CI enforces them."

Path B (reviewer's recommendation): keep the SpotBugs binding (already
green on this branch), drop the dep-check binding + skip property from
this PR, and track the NVD secret + cache work in a follow-up.

Removes from pom.xml:
  - `<dependency-check.skip>` property and comment
  - `<configuration>` + `<executions>` blocks on dependency-check-maven
    (the plugin declaration itself stays for the follow-up to re-enable)

Updates .github/workflows/ci-java.yml comment to reference only the
SpotBugs gate.

Keeps dependency-check-suppressions.xml so the next PR can turn the gate
on without re-introducing the stub.

Verified locally (`mvn -B clean compile spotbugs:check ...`):
  [INFO] --- spotbugs-maven-plugin:4.9.8.3:check (default-cli) @ code-iq ---
  [INFO] BugInstance size is 0
  [INFO] BUILD SUCCESS

Full `mvn clean verify` on this branch pre-edits produced
`Tests run: 3395, Failures: 0, Errors: 0, Skipped: 31` before the dep-check
step failed (per CI run 24913046002); removing that step is the only CI
delta this commit introduces.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 12ba17748d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread pom.xml
Comment on lines +405 to +409
<executions>
<execution>
<id>spotbugs-verify</id>
<phase>verify</phase>
<goals><goal>check</goal></goals>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Bind dependency-check check goal to verify

This change only adds a verify execution for SpotBugs, but .github/workflows/ci-java.yml runs mvn clean verify -B as the sole gate and the dependency-check-maven plugin still has no <executions> block, so OWASP dependency scanning is never invoked in CI. That means high-CVSS dependency vulnerabilities can still merge undetected even though this commit is intended to enforce that gate.

Useful? React with 👍 / 👎.

@sonarqubecloud
Copy link
Copy Markdown

@aksOps aksOps enabled auto-merge (squash) April 25, 2026 13:18
@aksOps
Copy link
Copy Markdown
Contributor Author

aksOps commented Apr 25, 2026

Superseded by RAN-46 (#74) + RAN-26 (#68). Both spotbugs:check and dependency-check:check are now bound to mvn verify on main with full NVD cache + retry infra and 5 TechLead-signed suppressions. Not force-pushing — the rebase would land an empty/contradictory diff. Closing per RAN-24 thread (board-armed auto-merge superseded). Follow-up tracked in RAN-42 (NVD_API_KEY secret provisioning only).

@aksOps aksOps closed this Apr 25, 2026
auto-merge was automatically disabled April 25, 2026 13:36

Pull request was closed

@aksOps aksOps deleted the chore/ran-24-bind-quality-gates branch April 26, 2026 05:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant