|
| 1 | +# Sonar Fixes Implementation Plan |
| 2 | + |
| 3 | +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. |
| 4 | +
|
| 5 | +**Goal:** Clear all SonarCloud issues + pass Quality Gate. Fix hardcoded versions, missing dep verification, mutable collections, duplicate literals. |
| 6 | + |
| 7 | +**Architecture:** Move dep versions to `gradle/libs.versions.toml` catalog. Enable Gradle dependency verification (writes `gradle/verification-metadata.xml`) — kills both `text:S8569` vuln + LOW hotspot. Convert `mutableSetOf` builders to immutable `toSet()`. Extract repeated `"no package"` literal to constant in `PackageRules.kt`. |
| 8 | + |
| 9 | +**Tech Stack:** Gradle 9.4 Kotlin DSL, Gradle version catalogs, Gradle dependency verification, Kotlin 2.3. |
| 10 | + |
| 11 | +**Sonar issues fixed:** |
| 12 | +- `text:S8569` (vuln) — dep verification missing → enable verification-metadata |
| 13 | +- `text:S8569` build.gradle.kts:24 — kotlin-compiler-embeddable hardcoded → catalog |
| 14 | +- `kotlin:S6624` build.gradle.kts:27,28,29 — Spring/Hibernate hardcoded → catalog |
| 15 | +- `kotlin:S6524` ProxyRules.kt:139,140,146 — mutable collections → `toSet()` |
| 16 | +- `kotlin:S1192` PackageRules.kt:144 (+11 dup sites) — `"no package"` literal → constant |
| 17 | +- LOW hotspot — dep verification missing → resolved by verification-metadata |
| 18 | +- QG `new_security_rating=3` → resolved when vuln cleared |
| 19 | +- QG `new_security_hotspots_reviewed=0%` → mark hotspot Reviewed (manual or via API) |
| 20 | + |
| 21 | +--- |
| 22 | + |
| 23 | +## File Structure |
| 24 | + |
| 25 | +- Create: `gradle/libs.versions.toml` — version catalog |
| 26 | +- Create: `gradle/verification-metadata.xml` — generated dep verification (committed) |
| 27 | +- Modify: `build.gradle.kts` — switch deps to catalog refs |
| 28 | +- Modify: `src/main/kotlin/dev/protsenko/codeguard/rules/proxy/ProxyRules.kt` — immutable sets |
| 29 | +- Modify: `src/main/kotlin/dev/protsenko/codeguard/rules/packages/PackageRules.kt` — `NO_PACKAGE` const |
| 30 | + |
| 31 | +--- |
| 32 | + |
| 33 | +### Task 1: Version Catalog |
| 34 | + |
| 35 | +**Files:** |
| 36 | +- Create: `gradle/libs.versions.toml` |
| 37 | +- Modify: `build.gradle.kts:21-30` |
| 38 | + |
| 39 | +- [ ] **Step 1: Create catalog** |
| 40 | + |
| 41 | +`gradle/libs.versions.toml`: |
| 42 | +```toml |
| 43 | +[versions] |
| 44 | +konsist = "0.17.3" |
| 45 | +kotlin-compiler = "2.0.21" |
| 46 | +spring-boot = "4.0.5" |
| 47 | +hibernate-validator = "9.1.0.Final" |
| 48 | + |
| 49 | +[libraries] |
| 50 | +konsist = { module = "com.lemonappdev:konsist", version.ref = "konsist" } |
| 51 | +kotlin-compiler-embeddable = { module = "org.jetbrains.kotlin:kotlin-compiler-embeddable", version.ref = "kotlin-compiler" } |
| 52 | +spring-boot-starter-data-jpa = { module = "org.springframework.boot:spring-boot-starter-data-jpa", version.ref = "spring-boot" } |
| 53 | +spring-boot-starter-web = { module = "org.springframework.boot:spring-boot-starter-web", version.ref = "spring-boot" } |
| 54 | +hibernate-validator = { module = "org.hibernate.validator:hibernate-validator", version.ref = "hibernate-validator" } |
| 55 | +``` |
| 56 | + |
| 57 | +- [ ] **Step 2: Switch dependencies block** |
| 58 | + |
| 59 | +`build.gradle.kts` replace lines 21-30: |
| 60 | +```kotlin |
| 61 | +dependencies { |
| 62 | + api(libs.konsist) |
| 63 | + implementation(libs.kotlin.compiler.embeddable) |
| 64 | + |
| 65 | + testImplementation(kotlin("test")) |
| 66 | + testImplementation(libs.spring.boot.starter.data.jpa) |
| 67 | + testImplementation(libs.spring.boot.starter.web) |
| 68 | + testImplementation(libs.hibernate.validator) |
| 69 | +} |
| 70 | +``` |
| 71 | + |
| 72 | +- [ ] **Step 3: Verify build resolves** |
| 73 | + |
| 74 | +Run: `./gradlew dependencies --configuration testRuntimeClasspath -q | head -20` |
| 75 | +Expected: dep tree prints, no `Could not resolve` errors. |
| 76 | + |
| 77 | +- [ ] **Step 4: Run tests** |
| 78 | + |
| 79 | +Run: `./gradlew test` |
| 80 | +Expected: BUILD SUCCESSFUL. |
| 81 | + |
| 82 | +- [ ] **Step 5: Commit** |
| 83 | + |
| 84 | +```bash |
| 85 | +git add gradle/libs.versions.toml build.gradle.kts |
| 86 | +git commit -m "build: extract dep versions to version catalog" |
| 87 | +``` |
| 88 | + |
| 89 | +--- |
| 90 | + |
| 91 | +### Task 2: Dependency Verification |
| 92 | + |
| 93 | +**Files:** |
| 94 | +- Create: `gradle/verification-metadata.xml` |
| 95 | + |
| 96 | +- [ ] **Step 1: Generate metadata** |
| 97 | + |
| 98 | +Run: |
| 99 | +```bash |
| 100 | +./gradlew --write-verification-metadata sha256 help |
| 101 | +``` |
| 102 | +Expected: writes `gradle/verification-metadata.xml`. May take 1-3 min downloading checksums. |
| 103 | + |
| 104 | +- [ ] **Step 2: Inspect file head** |
| 105 | + |
| 106 | +Run: `head -20 gradle/verification-metadata.xml` |
| 107 | +Expected: `<verification-metadata>` root, `<components>` block with sha256 entries. |
| 108 | + |
| 109 | +- [ ] **Step 3: Validate verification mode** |
| 110 | + |
| 111 | +Edit `gradle/verification-metadata.xml`, ensure `<verify-metadata>true</verify-metadata>` and `<verify-signatures>false</verify-signatures>` present in `<configuration>` block (pgp signing not in scope). |
| 112 | + |
| 113 | +- [ ] **Step 4: Re-run build with verification** |
| 114 | + |
| 115 | +Run: `./gradlew clean test` |
| 116 | +Expected: BUILD SUCCESSFUL. Verification passes silently. |
| 117 | + |
| 118 | +- [ ] **Step 5: Commit** |
| 119 | + |
| 120 | +```bash |
| 121 | +git add gradle/verification-metadata.xml |
| 122 | +git commit -m "build: enable gradle dependency verification (sha256)" |
| 123 | +``` |
| 124 | + |
| 125 | +--- |
| 126 | + |
| 127 | +### Task 3: Immutable Sets in ProxyRules |
| 128 | + |
| 129 | +**Files:** |
| 130 | +- Modify: `src/main/kotlin/dev/protsenko/codeguard/rules/proxy/ProxyRules.kt:139-146` |
| 131 | + |
| 132 | +- [ ] **Step 1: Run existing tests for proxy rules** |
| 133 | + |
| 134 | +Run: `./gradlew test --tests "*Proxy*" -q` |
| 135 | +Expected: PASS. Establishes green baseline. |
| 136 | + |
| 137 | +- [ ] **Step 2: Replace mutable builders** |
| 138 | + |
| 139 | +Edit lines 139-146. Old: |
| 140 | +```kotlin |
| 141 | +val ownProxyNames = ownMethods.mapTo(mutableSetOf()) { it.n } |
| 142 | +val ownNames = functions.mapTo(mutableSetOf()) { it.name } |
| 143 | +``` |
| 144 | +New: |
| 145 | +```kotlin |
| 146 | +val ownProxyNames = ownMethods.mapTo(mutableSetOf()) { it.n }.toSet() |
| 147 | +val ownNames = functions.mapTo(mutableSetOf()) { it.name }.toSet() |
| 148 | +``` |
| 149 | + |
| 150 | +Old line 146: |
| 151 | +```kotlin |
| 152 | +val ambiguous = methods.keys.filterTo(mutableSetOf()) { name -> |
| 153 | +``` |
| 154 | +New: |
| 155 | +```kotlin |
| 156 | +val ambiguous: Set<String> = methods.keys.filterTo(mutableSetOf()) { name -> |
| 157 | +``` |
| 158 | + |
| 159 | +Then close the lambda and append `.toSet()`. Result: |
| 160 | +```kotlin |
| 161 | +val ambiguous: Set<String> = methods.keys.filter { name -> |
| 162 | + functions.any { it.name == name && it.proxyMethod() == null } |
| 163 | +}.toSet() |
| 164 | +``` |
| 165 | + |
| 166 | +(Replaces the `filterTo(mutableSetOf())` builder pattern with `filter { }.toSet()`.) |
| 167 | + |
| 168 | +- [ ] **Step 3: Run tests** |
| 169 | + |
| 170 | +Run: `./gradlew test --tests "*Proxy*" -q` |
| 171 | +Expected: PASS. |
| 172 | + |
| 173 | +- [ ] **Step 4: Commit** |
| 174 | + |
| 175 | +```bash |
| 176 | +git add src/main/kotlin/dev/protsenko/codeguard/rules/proxy/ProxyRules.kt |
| 177 | +git commit -m "refactor(proxy): use immutable sets in proxyModels" |
| 178 | +``` |
| 179 | + |
| 180 | +--- |
| 181 | + |
| 182 | +### Task 4: Extract `NO_PACKAGE` Constant |
| 183 | + |
| 184 | +**Files:** |
| 185 | +- Modify: `src/main/kotlin/dev/protsenko/codeguard/rules/packages/PackageRules.kt` |
| 186 | + |
| 187 | +- [ ] **Step 1: Locate insertion point** |
| 188 | + |
| 189 | +Run: `grep -n "^class\|^object\|^private const\|^const" src/main/kotlin/dev/protsenko/codeguard/rules/packages/PackageRules.kt | head` |
| 190 | +Expected: identifies top-level container. Add const at file top after imports OR inside enclosing object/class as `private const val NO_PACKAGE = "no package"`. |
| 191 | + |
| 192 | +- [ ] **Step 2: Add constant** |
| 193 | + |
| 194 | +Add (companion or top-level, matching codebase style): |
| 195 | +```kotlin |
| 196 | +private const val NO_PACKAGE = "no package" |
| 197 | +``` |
| 198 | + |
| 199 | +- [ ] **Step 3: Replace all 12 occurrences** |
| 200 | + |
| 201 | +In `PackageRules.kt` replace literal `"no package"` with `NO_PACKAGE` at lines 144, 172, 201, 231, 301, 327, 360, 395, 433, 469, 536, 563. |
| 202 | + |
| 203 | +Run after: `grep -c '"no package"' src/main/kotlin/dev/protsenko/codeguard/rules/packages/PackageRules.kt` |
| 204 | +Expected: `0`. |
| 205 | + |
| 206 | +- [ ] **Step 4: Run package tests** |
| 207 | + |
| 208 | +Run: `./gradlew test --tests "*Package*" -q` |
| 209 | +Expected: PASS. |
| 210 | + |
| 211 | +- [ ] **Step 5: Commit** |
| 212 | + |
| 213 | +```bash |
| 214 | +git add src/main/kotlin/dev/protsenko/codeguard/rules/packages/PackageRules.kt |
| 215 | +git commit -m "refactor(packages): extract NO_PACKAGE constant" |
| 216 | +``` |
| 217 | + |
| 218 | +--- |
| 219 | + |
| 220 | +### Task 5: Full Verification + Sonar Re-Scan |
| 221 | + |
| 222 | +**Files:** none |
| 223 | + |
| 224 | +- [ ] **Step 1: Run full pipeline** |
| 225 | + |
| 226 | +Run: `./gradlew clean test detektMain koverVerify koverXmlReport` |
| 227 | +Expected: BUILD SUCCESSFUL. |
| 228 | + |
| 229 | +- [ ] **Step 2: Trigger Sonar scan** |
| 230 | + |
| 231 | +Run: `./gradlew sonar` |
| 232 | +Expected: scan uploads. May still fail QG until hotspot reviewed (Task 6). |
| 233 | + |
| 234 | +- [ ] **Step 3: Inspect issues** |
| 235 | + |
| 236 | +Run: `./gradlew sonarReport` |
| 237 | +Expected: `=== Issues (open) ===` empty (no `[SEVERITY]` lines printed). |
| 238 | + |
| 239 | +If issues remain — read them, fix, re-run. Do not proceed to Task 6 until issue list empty. |
| 240 | + |
| 241 | +--- |
| 242 | + |
| 243 | +### Task 6: Mark Hotspot Reviewed |
| 244 | + |
| 245 | +**Files:** none (SonarCloud-side action) |
| 246 | + |
| 247 | +The single LOW hotspot (`Dependencies are not verified...`) is auto-resolved when `verification-metadata.xml` is committed (Task 2) — but SonarCloud may keep it `TO_REVIEW` until the next scan re-classifies. If still open after Task 5: |
| 248 | + |
| 249 | +- [ ] **Step 1: List open hotspots** |
| 250 | + |
| 251 | +Run: |
| 252 | +```bash |
| 253 | +curl -s -u "$SONAR_TOKEN:" \ |
| 254 | + "https://sonarcloud.io/api/hotspots/search?projectKey=NordCoderd_spring-boot-code-guard&status=TO_REVIEW&ps=100" \ |
| 255 | + -o /tmp/hotspots.json |
| 256 | +python3 -c "import json; d=json.load(open('/tmp/hotspots.json')); [print(h['key'],h.get('message')) for h in d['hotspots']]" |
| 257 | +``` |
| 258 | +Expected: prints hotspot keys still TO_REVIEW. If empty → done. |
| 259 | + |
| 260 | +- [ ] **Step 2: Mark as SAFE** |
| 261 | + |
| 262 | +For each `<key>` printed: |
| 263 | +```bash |
| 264 | +curl -s -u "$SONAR_TOKEN:" -X POST \ |
| 265 | + "https://sonarcloud.io/api/hotspots/change_status" \ |
| 266 | + -d "hotspot=<key>&status=REVIEWED&resolution=SAFE&comment=verification-metadata.xml committed" |
| 267 | +``` |
| 268 | +Expected: HTTP 204 (empty response body). |
| 269 | + |
| 270 | +- [ ] **Step 3: Re-fetch QG** |
| 271 | + |
| 272 | +Run: `./gradlew sonarReport` |
| 273 | +Expected: `"status":"OK"` in `=== Quality Gate ===` block. |
| 274 | + |
| 275 | +- [ ] **Step 4: Final commit (if any local changes)** |
| 276 | + |
| 277 | +If anything changed locally during scan-iteration: |
| 278 | +```bash |
| 279 | +git status |
| 280 | +git commit -am "chore: sonar quality gate green" |
| 281 | +``` |
| 282 | + |
| 283 | +--- |
| 284 | + |
| 285 | +## Notes |
| 286 | + |
| 287 | +- Dependency verification slows clean builds first time (downloads checksums for every transitive). Subsequent builds: zero overhead. |
| 288 | +- Adding new deps later requires `./gradlew --write-verification-metadata sha256 help` again to refresh checksums. |
| 289 | +- If a transitive lacks a checksum and verification fails, run the regenerate command — do NOT bypass with `--refresh-keys` or remove verification. |
| 290 | +- Version catalog accessor names: dashes in `[libraries]` keys become dots in Kotlin (e.g., `spring-boot-starter-web` → `libs.spring.boot.starter.web`). |
0 commit comments