Skip to content

Commit fdac5c8

Browse files
committed
fix(bootstrap): round-3 reviewer findings on PR #74 (RAN-47)
Four blockers raised on `1dad7e7`: 1. `<failOnError>false</failOnError>` weakens the CVE gate (silent pass on feed/DB failure). Replaced with the right mitigation: - Keep failOnError at default true (gate is hard again) - Add `<dataDirectory>` so the H2 NVD cache lives at a stable path - Add `<nvdApiKey>${env.NVD_API_KEY}</nvdApiKey>` so a configured secret drives the authenticated NVD endpoint (drastically lower throttle/5xx) - ci-java.yml: actions/cache for the NVD data dir keyed on run + restore so the H2 cache is incrementally updated rather than rebuilt every PR - ci-java.yml: pass `NVD_API_KEY: ${{ secrets.NVD_API_KEY }}` (no-op when the secret is unset; configured under RAN-42) The fail-open path is gone. CVE findings AND scanner-operational failures both red the build. 2. `rollback.md` documented `git revert -m 1 <merge-sha>` for squash merges. Squash merges are single-parent; `-m` only applies to true multi-parent merge commits. Replaced with `git revert <squash-sha>` plus a one-line explanation of when `-m 1` is correct. 3. `CLAUDE.md` Deploy section claimed `release-java.yml` is triggered by `vX.Y.Z` tag pushes. Reality (and now the runbook in `release.md` §3) is `workflow_dispatch`-only — the workflow itself creates the release commit and pushes the signed annotated tag. Updated CLAUDE.md to match. 4. `engineering-standards.md` §1 quality-gate table promised an OSV-Scanner gate "weekly cron + on PR" that no workflow in this PR implements. Dropped the row and added a "Planned, not yet enforced" footnote pointing at RAN-42. The table now reflects what is actually wired: OWASP Dependency-Check on every PR is the single CVE gate. Refs RAN-47.
1 parent 1dad7e7 commit fdac5c8

5 files changed

Lines changed: 49 additions & 16 deletions

File tree

.github/workflows/ci-java.yml

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,25 @@ jobs:
1818
distribution: 'temurin'
1919
java-version: '25'
2020
cache: 'maven'
21-
- run: mvn clean verify -B
21+
# Cache the OWASP Dependency-Check NVD data directory across runs so the
22+
# CVE gate does not need to re-download the full feed on every PR.
23+
# `key` is unique per run (forces a save on every run), `restore-keys`
24+
# falls back to the most recent prior cache so the H2 DB is incrementally
25+
# updated rather than rebuilt.
26+
- uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5
27+
with:
28+
path: ~/.m2/repository/org/owasp/dependency-check-data
29+
key: dependency-check-${{ runner.os }}-${{ github.run_id }}
30+
restore-keys: |
31+
dependency-check-${{ runner.os }}-
32+
- name: Build + verify (jacoco 85% + SpotBugs + dependency-check)
33+
env:
34+
# When the NVD_API_KEY secret is unset, dependency-check falls back
35+
# to the unauthenticated NVD endpoint (rate-limited but functional
36+
# once the cache is warm). Provisioning the secret is tracked under
37+
# RAN-42.
38+
NVD_API_KEY: ${{ secrets.NVD_API_KEY }}
39+
run: mvn -B -ntp clean verify
2240
- uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v4.6.2
2341
if: always()
2442
with:

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ bean for code paths that haven't been ported yet.
431431

432432
## Deploy
433433

434-
codeiq's deploy surface is **Maven Central + GitHub Releases** (per RAN-46 AC #10 ruling, option a). The single Java JAR (with the React UI bundled inside) is published via `.github/workflows/beta-java.yml` (`workflow_dispatch` → Sonatype Central beta + GitHub pre-release) and `.github/workflows/release-java.yml` (`vX.Y.Z` tag → Sonatype Central GA + GitHub Release). There is no static-CDN frontend, no hosted backend, no VPS — codeiq runs on the developer's machine. See [`shared/runbooks/release.md`](shared/runbooks/release.md) and [`shared/runbooks/engineering-standards.md`](shared/runbooks/engineering-standards.md) §7.1.
434+
codeiq's deploy surface is **Maven Central + GitHub Releases** (per RAN-46 AC #10 ruling, option a). The single Java JAR (with the React UI bundled inside) is published via two `workflow_dispatch`-only workflows: `.github/workflows/beta-java.yml` (manual beta cut → Sonatype Central beta + GitHub pre-release) and `.github/workflows/release-java.yml` (manual GA cut with a `version` input → the workflow builds a GPG-signed release commit on a detached HEAD, deploys from that exact tree, then creates and pushes a GPG-signed annotated `vX.Y.Z` tag pointing at the release commit + a GitHub Release). There is no static-CDN frontend, no hosted backend, no VPS — codeiq runs on the developer's machine. See [`shared/runbooks/release.md`](shared/runbooks/release.md) and [`shared/runbooks/engineering-standards.md`](shared/runbooks/engineering-standards.md) §7.1.
435435

436436
## Updating This File
437437

pom.xml

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -407,18 +407,26 @@
407407
<artifactId>dependency-check-maven</artifactId>
408408
<version>${owasp.dependency-check.version}</version>
409409
<configuration>
410-
<!-- Fail build on High/Critical CVEs (CVSS >= 7) per security.md -->
410+
<!-- Hard gate: build fails on High/Critical CVEs (CVSS >= 7)
411+
per security.md AND on operational failures of the
412+
scanner itself. failOnError stays at the default (true)
413+
so a silent NVD-feed degradation does NOT pass as a
414+
green build (Reviewer round-3 finding #1). -->
411415
<failBuildOnCVSS>7</failBuildOnCVSS>
412-
<!-- Do NOT fail the build on NVD-feed download/connection
413-
hiccups (the upstream NVD API is rate-limited and
414-
occasionally returns 5xx, which would otherwise red-X
415-
every PR). The gate still fails on real CVE findings
416-
via failBuildOnCVSS above; transient NVD outages just
417-
skip the analysis for that run.
418-
RAN-42 tracks making the gate fully robust (NVD API
419-
key, cached data, etc.); this is the minimal compromise
420-
to satisfy RAN-46 AC #5 without breaking PRs. -->
421-
<failOnError>false</failOnError>
416+
<!-- Stable on-disk cache directory. ci-java.yml caches
417+
this path between runs (actions/cache), which is the
418+
primary mitigation for transient NVD-feed/H2 issues:
419+
after the first successful run on a fresh runner, the
420+
cache holds a pre-populated H2 DB and incremental
421+
updates are short. -->
422+
<dataDirectory>${user.home}/.m2/repository/org/owasp/dependency-check-data</dataDirectory>
423+
<!-- Use NVD_API_KEY when available (drastically reduces
424+
throttle / 5xx rates from the NVD API). The build
425+
falls back to the unauthenticated path when the
426+
secret is unset; that path is rate-limited but works
427+
once the cache is warm. RAN-42 tracks the secret
428+
provisioning + a dedicated nightly NVD-refresh job. -->
429+
<nvdApiKey>${env.NVD_API_KEY}</nvdApiKey>
422430
</configuration>
423431
<executions>
424432
<!-- Bind dependency-check:check into the verify phase so

shared/runbooks/engineering-standards.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,14 @@ The rule of last resort: **`/home/dev/.claude/rules/*.md` wins.** This file does
1414
| JaCoCo coverage | ≥ 85% line, ≥ 75% branch (project-wide, post-exclusions) | `jacoco-maven-plugin` rule in `pom.xml` | Block merge |
1515
| SonarCloud Quality Gate | `Passed` (`Sonar way` profile + 80% new-code coverage) | `ci-java.yml` | Block merge |
1616
| SpotBugs | Zero High/Critical findings; `spotbugs-exclude.xml` justified per-entry | `mvn spotbugs:check` | Block merge |
17-
| OWASP Dependency-Check | No High/Critical CVEs (`failBuildOnCVSS=7`); Medium tracked | `mvn dependency-check:check` (CI nightly + on `release-java.yml`) | Block release |
18-
| OSV-Scanner | Clean, or every finding has a justification commit on file | weekly cron + on PR | Block release on High/Critical |
17+
| OWASP Dependency-Check | No High/Critical CVEs (`failBuildOnCVSS=7`); Medium tracked | `mvn -B -ntp clean verify` (the `dependency-check:check` execution is bound to the `verify` phase in `pom.xml`); `ci-java.yml` runs on every PR + push to `main` | Block merge |
1918
| OpenSSF Scorecard | Best-effort; no hard score floor; `Pinned-Dependencies` is a soft target | `scorecard.yml` (push to `main` + weekly) | Surface in security tab; do **not** gate merge |
2019
| Signed commits | Every commit on `main` must verify | Branch protection + `gh api ... /commits/{sha}/check-runs` | Block merge |
2120

2221
Coverage exclusions are enumerated in `pom.xml` `<jacoco>` config — only generated ANTLR sources, the `application/` Spring Boot main, and pure data records are excluded. Adding to that list requires TechLead sign-off.
2322

23+
**Planned, not yet enforced:** OSV-Scanner as a second-source CVE feed (cross-checks OWASP Dependency-Check against the OSV / GitHub Advisory Database). Tracked under [RAN-42](/RAN/issues/RAN-42); will land as `.github/workflows/osv-scanner.yml` and a row added to the table above. Until then OSV is **not** part of the gate — only OWASP Dependency-Check is.
24+
2425
---
2526

2627
## 2. Code style

shared/runbooks/rollback.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,13 @@ If you are not sure, default to §2 and revert.
2525
```bash
2626
git fetch origin main
2727
git switch -c revert/<short-name> origin/main
28-
git revert -m 1 <merge-sha> # for squash merges, target the squash commit
28+
29+
# Squash-merged PRs land as a single commit with one parent. Plain `git revert`
30+
# applies — do NOT pass `-m`, which only makes sense for true (multi-parent)
31+
# merge commits. Use `-m 1` only if `main` ever carries an actual merge commit
32+
# (which the squash-merge-only branch protection should prevent).
33+
git revert <squash-merge-sha>
34+
2935
git push -u origin revert/<short-name>
3036
gh pr create --base main --fill --title "revert: <short-name>" --label "type:revert"
3137
gh pr merge --squash --auto

0 commit comments

Comments
 (0)