build(sonar): tune quality gate for false positives and fix genuine findings

build.gradle.kts

@@ -17,6 +17,35 @@ sonar {
         property("sonar.organization", "codeheadsystems")
         property("sonar.host.url", "https://sonarcloud.io")
         property("sonar.exclusions", "examples/**,clients/passkeys-browser/dist/**")
+
+        // Coverage exclusions: structural code with no meaningful logic to unit-test. The DynamoDB
+        // Enhanced Client item beans are mandatory mutable POJOs (no-arg ctor + getters/setters)
+        // driven entirely by the SDK's bean mapper, and module-info has no executable statements.
+        // Excluding them keeps the coverage signal honest instead of padding it with trivial tests.
+        property(
+            "sonar.coverage.exclusions",
+            "**/persistence/dynamodb/*Item.java,**/module-info.java",
+        )
+
+        // Documented false-positive suppressions (see docs/adr/0017). These are kept in the build
+        // (version-controlled, reviewed) rather than the SonarCloud UI so the rationale travels with
+        // the code. Real defects in these rules' domains are still caught: null discipline by Error
+        // Prone + JSpecify, and test effectiveness by the per-module JaCoCo floors plus mutation
+        // testing — these two rules are redundant with gates we already enforce.
+        property("sonar.issue.ignore.multicriteria", "jspecifyNullable,delegatedAssertions")
+        // S4449 demands JSR-305 `javax.annotation.Nullable`; the project standardizes on JSpecify
+        // (CONTRIBUTING.md §7), enforced at compile time by Error Prone. Two annotation systems for
+        // the same contract — we follow JSpecify, so this rule is noise here.
+        property("sonar.issue.ignore.multicriteria.jspecifyNullable.ruleKey", "java:S4449")
+        property("sonar.issue.ignore.multicriteria.jspecifyNullable.resourceKey", "**/*.java")
+        // S2699 ("tests should include assertions") cannot see across a call: the persistence and
+        // in-memory tests deliberately delegate their assertions to shared testkit `*Scenarios`
+        // classes so one suite runs against every backend. The assertions exist, just not inline.
+        property("sonar.issue.ignore.multicriteria.delegatedAssertions.ruleKey", "java:S2699")
+        property(
+            "sonar.issue.ignore.multicriteria.delegatedAssertions.resourceKey",
+            "**/*Test.java",
+        )
     }
 }
 

docs/adr/0017-sonarqube-cloud-static-analysis.md

@@ -40,7 +40,39 @@ Adopt **SonarQube Cloud** via the `org.sonarqube` Gradle plugin, driven from CI.
 - **Negative — fork PRs get no analysis.** GitHub withholds secrets from fork PRs, so the `sonar` job is skipped there. External contributors' branches are covered once merged to `main`; the standard trade-off for an open-source repo.
 - **Negative — heavier CI job.** The `sonar` job runs the full `build` (including the Docker-backed Testcontainers integration tests) so coverage exists when the scanner reads it. Acceptable on `ubuntu-latest`; can be optimized later by sharing coverage artifacts with the existing `build` job.
 
+## New-code quality gate and false-positive policy
+
+The default **Sonar way** gate scores the *new-code period* (Clean as You Code). On the very first
+analysis that period is the whole history, so the entire backlog is judged against strict new-code
+thresholds (80% coverage, A ratings, <3% duplication) — which is why the initial run reported red
+even though the project's *overall* numbers are healthy (≈88% coverage, ≈2% duplication). This is an
+onboarding artifact, not a regression.
+
+Two things keep the gate meaningful instead of noisy:
+
+- **New Code definition.** Set the project's New Code to **"Previous version"** in SonarCloud
+  (Administration → New Code). Because all modules share one `gradle.properties` version, each
+  release bounds a clean new-code window, and the gate then measures genuinely new changes rather
+  than the onboarding backlog. *This is a server-side setting and cannot live in the repo.*
+- **Documented false-positive suppressions live in `build.gradle.kts`**, not the SonarCloud UI, so
+  the rationale is version-controlled and reviewed:
+  - `java:S4449` (demands JSR-305 `javax.annotation.Nullable`) is suppressed project-wide — we
+    standardize on **JSpecify**, enforced by Error Prone (CONTRIBUTING.md §7).
+  - `java:S2699` ("tests should include assertions") is suppressed on `**/*Test.java` — the
+    persistence/in-memory suites delegate assertions to shared testkit `*Scenarios` classes so one
+    suite runs against every backend; the rule cannot see across the call. Test effectiveness is
+    already gated by the per-module JaCoCo floors and mutation testing.
+  - DynamoDB `*Item.java` beans and `module-info.java` are excluded from **coverage** — mandatory
+    structural code (SDK-driven POJOs / no executable statements) with nothing meaningful to test.
+
+Reliability findings flagged on intentional defensive null-guards (e.g. `S2583`/`S2589` where a
+framework can still inject null despite a `@NullMarked` package) are fixed *in code* by annotating
+the parameter `@Nullable` to match reality — see the Micronaut admin controller's `@Body` params —
+never by deleting the guard.
+
 ## Open follow-ups
 
-- If we want the quality gate to block merges, wire the Sonar status check into branch protection.
-- If CI time becomes a concern, have the `sonar` job download JaCoCo artifacts from the `build` job instead of rebuilding.
+- Once the New Code definition is set and the gate is green, optionally wire the Sonar status check
+  into branch protection to block merges.
+- If CI time becomes a concern, have the `sonar` job download JaCoCo artifacts from the `build` job
+  instead of rebuilding.

pk-auth-micronaut/src/main/java/com/codeheadsystems/pkauth/micronaut/PkAuthAdminController.java

@@ -30,6 +30,7 @@
 import io.micronaut.http.annotation.Produces;
 import io.micronaut.scheduling.TaskExecutors;
 import io.micronaut.scheduling.annotation.ExecuteOn;
+import org.jspecify.annotations.Nullable;
 
 /**
  * Admin controller mounting the brief §6.9 endpoints under {@code /auth/admin/**}. All
@@ -76,7 +77,9 @@
   /** Renames the credential identified by its base64url-encoded id. */
   @Patch("/credentials/{credentialId}")
   public HttpResponse<?> renameCredential(
-      HttpRequest<?> request, @PathVariable String credentialId, @Body RenameCredential body) {
+      HttpRequest<?> request,
+      @PathVariable String credentialId,
+      @Body @Nullable RenameCredential body) {
     UserHandle actor = PkAuthJwtAuthenticationFilter.attachedUserHandle(request);
     if (actor == null) return HttpResponse.status(HttpStatus.UNAUTHORIZED);
     CredentialId id = CredentialId.fromB64Url(credentialId);
@@ -111,15 +114,15 @@
 
   @Post("/email/start-verification")
   public HttpResponse<?> startEmailVerification(
-      HttpRequest<?> request, @Body StartEmailVerification body) {
+      HttpRequest<?> request, @Body @Nullable StartEmailVerification body) {
     UserHandle actor = PkAuthJwtAuthenticationFilter.attachedUserHandle(request);
     if (actor == null) return HttpResponse.status(HttpStatus.UNAUTHORIZED);
     return map(adminService.startEmailVerification(actor, actor, body == null ? "" : body.email()));
   }
 
   /** Unauthenticated. */
   @Post("/email/complete-verification")
-  public HttpResponse<?> finishEmailVerification(@Body FinishEmailVerification body) {
+  public HttpResponse<?> finishEmailVerification(@Body @Nullable FinishEmailVerification body) {
     return toMicronaut(
         AdminResponseMapper.toResponse(
             adminService.finishEmailVerification(body == null ? "" : body.token()),
@@ -128,15 +131,15 @@
 
   @Post("/phone/start-verification")
   public HttpResponse<?> startPhoneVerification(
-      HttpRequest<?> request, @Body StartPhoneVerification body) {
+      HttpRequest<?> request, @Body @Nullable StartPhoneVerification body) {
     UserHandle actor = PkAuthJwtAuthenticationFilter.attachedUserHandle(request);
     if (actor == null) return HttpResponse.status(HttpStatus.UNAUTHORIZED);
     return map(adminService.startPhoneVerification(actor, actor, body == null ? "" : body.phone()));
   }
 
   @Post("/phone/complete-verification")
   public HttpResponse<?> finishPhoneVerification(
-      HttpRequest<?> request, @Body FinishPhoneVerification body) {
+      HttpRequest<?> request, @Body @Nullable FinishPhoneVerification body) {
     UserHandle actor = PkAuthJwtAuthenticationFilter.attachedUserHandle(request);
     if (actor == null) return HttpResponse.status(HttpStatus.UNAUTHORIZED);
     return map(

pk-auth-testkit/src/main/java/com/codeheadsystems/pkauth/testkit/CeremonyScenarios.java

@@ -65,8 +65,9 @@ public void registrationThenAssertionBumpsSignCount() {
 
     Optional<CredentialRecord> stored = singleCredentialFor(handle);
     assertThat(stored).isPresent();
-    assertThat(stored.get().signCount()).isZero();
-    CredentialId credentialId = stored.get().credentialId();
+    CredentialRecord credential = stored.orElseThrow();
+    assertThat(credential.signCount()).isZero();
+    CredentialId credentialId = credential.credentialId();
 
     AssertionResult first = assertOnce(handle);
     assertThat(first)