fix(notification): close the static-analysis findings on the merged tree
Checkstyle and SpotBugs run in the module's `check` task, not in `test`, so these only surfaced once the platform was verified against the merged tree. - MissingSwitchDefault on the admin runtime-state switch. The enum is exhaustive, so the default is unreachable today; it throws rather than falling through, so a state added later fails loudly instead of silently leaving the runtime in whatever state it already had. - ConstantName on the two audit loggers: the checkstyle pattern allows `log`, `logger` or UPPER_SNAKE, and this class needs two named sinks. - DMI_RANDOM_USED_ONLY_ONCE in three Web Push fixtures. A fresh SecureRandom per call re-seeds from the OS every time, which on a constrained CI runner can block on entropy. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
92744c57de
commit
c1ee1d9dd9
+3
@@ -293,6 +293,9 @@ public final class NotificationAdminServiceImpl implements NotificationAdminServ
|
|||||||
case DEGRADED -> runtime.markDegraded(command.reason());
|
case DEGRADED -> runtime.markDegraded(command.reason());
|
||||||
case THROTTLED -> runtime.markThrottled();
|
case THROTTLED -> runtime.markThrottled();
|
||||||
case AUTHENTICATION_FAILED -> runtime.markAuthenticationFailed(command.reason());
|
case AUTHENTICATION_FAILED -> runtime.markAuthenticationFailed(command.reason());
|
||||||
|
// Unreachable while the enum is exhaustive; present so a state added later fails loudly here
|
||||||
|
// rather than silently leaving the runtime in whatever state it was already in.
|
||||||
|
default -> throw new IllegalStateException("unhandled provider runtime state");
|
||||||
}
|
}
|
||||||
|
|
||||||
AdminOperationResult result =
|
AdminOperationResult result =
|
||||||
|
|||||||
+5
-5
@@ -22,13 +22,13 @@ import org.slf4j.LoggerFactory;
|
|||||||
public final class LoggingNotificationAudit
|
public final class LoggingNotificationAudit
|
||||||
implements NotificationAuditPort, NotificationSecurityAuditPort {
|
implements NotificationAuditPort, NotificationSecurityAuditPort {
|
||||||
|
|
||||||
private static final Logger audit = LoggerFactory.getLogger("notification.audit");
|
private static final Logger AUDIT = LoggerFactory.getLogger("notification.audit");
|
||||||
private static final Logger security = LoggerFactory.getLogger("notification.security");
|
private static final Logger SECURITY = LoggerFactory.getLogger("notification.security");
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void record(NotificationAuditEvent event) {
|
public void record(NotificationAuditEvent event) {
|
||||||
Objects.requireNonNull(event, "event");
|
Objects.requireNonNull(event, "event");
|
||||||
audit.info(
|
AUDIT.info(
|
||||||
"action={} actor={} reason={} operationId={} occurredAt={} attributes={}",
|
"action={} actor={} reason={} operationId={} occurredAt={} attributes={}",
|
||||||
event.action(),
|
event.action(),
|
||||||
event.actorRef(),
|
event.actorRef(),
|
||||||
@@ -43,7 +43,7 @@ public final class LoggingNotificationAudit
|
|||||||
Objects.requireNonNull(profileId, "profileId");
|
Objects.requireNonNull(profileId, "profileId");
|
||||||
// The payload is deliberately absent: a forged callback must not get its content into the log
|
// The payload is deliberately absent: a forged callback must not get its content into the log
|
||||||
// just by being rejected.
|
// just by being rejected.
|
||||||
security.warn(
|
SECURITY.warn(
|
||||||
"event=callback_signature_rejected providerProfile={} reason={}",
|
"event=callback_signature_rejected providerProfile={} reason={}",
|
||||||
profileId.value(),
|
profileId.value(),
|
||||||
reasonCode);
|
reasonCode);
|
||||||
@@ -52,7 +52,7 @@ public final class LoggingNotificationAudit
|
|||||||
@Override
|
@Override
|
||||||
public void callbackRejectedByLimit(ProviderProfileId profileId, String reasonCode) {
|
public void callbackRejectedByLimit(ProviderProfileId profileId, String reasonCode) {
|
||||||
Objects.requireNonNull(profileId, "profileId");
|
Objects.requireNonNull(profileId, "profileId");
|
||||||
security.warn(
|
SECURITY.warn(
|
||||||
"event=callback_rejected_by_limit providerProfile={} reason={}",
|
"event=callback_rejected_by_limit providerProfile={} reason={}",
|
||||||
profileId.value(),
|
profileId.value(),
|
||||||
reasonCode);
|
reasonCode);
|
||||||
|
|||||||
+4
-1
@@ -41,6 +41,9 @@ class WebPushCryptoTest {
|
|||||||
private static final Clock CLOCK =
|
private static final Clock CLOCK =
|
||||||
Clock.fixed(Instant.parse("2026-08-14T00:00:00Z"), ZoneOffset.UTC);
|
Clock.fixed(Instant.parse("2026-08-14T00:00:00Z"), ZoneOffset.UTC);
|
||||||
private static final URI ENDPOINT = URI.create("https://push.example.com/send/abc123");
|
private static final URI ENDPOINT = URI.create("https://push.example.com/send/abc123");
|
||||||
|
// One instance: a fresh SecureRandom per call re-seeds from the OS each time, which is slower
|
||||||
|
// and, on a constrained CI runner, can block on entropy.
|
||||||
|
private static final SecureRandom RANDOM = new SecureRandom();
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void payloadIsEncryptedForTheSubscriptionAndDecryptsBackToThePlaintext() throws Exception {
|
void payloadIsEncryptedForTheSubscriptionAndDecryptsBackToThePlaintext() throws Exception {
|
||||||
@@ -221,7 +224,7 @@ class WebPushCryptoTest {
|
|||||||
|
|
||||||
private static byte[] authSecret() {
|
private static byte[] authSecret() {
|
||||||
byte[] secret = new byte[16];
|
byte[] secret = new byte[16];
|
||||||
new SecureRandom().nextBytes(secret);
|
RANDOM.nextBytes(secret);
|
||||||
return secret;
|
return secret;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+7
-3
@@ -35,6 +35,10 @@ class WebPushProviderAdapterTest {
|
|||||||
private static final Clock CLOCK =
|
private static final Clock CLOCK =
|
||||||
Clock.fixed(Instant.parse("2026-08-14T00:00:00Z"), ZoneOffset.UTC);
|
Clock.fixed(Instant.parse("2026-08-14T00:00:00Z"), ZoneOffset.UTC);
|
||||||
|
|
||||||
|
// One instance: a fresh SecureRandom per call re-seeds from the OS each time, which is slower
|
||||||
|
// and, on a constrained CI runner, can block on entropy.
|
||||||
|
private static final SecureRandom RANDOM = new SecureRandom();
|
||||||
|
|
||||||
private final ProviderFaultHarness harness = new ProviderFaultHarness();
|
private final ProviderFaultHarness harness = new ProviderFaultHarness();
|
||||||
private final ContactPointProtector protector =
|
private final ContactPointProtector protector =
|
||||||
new AesGcmContactPointProtector(SecurityFixtures.keys());
|
new AesGcmContactPointProtector(SecurityFixtures.keys());
|
||||||
@@ -115,7 +119,7 @@ class WebPushProviderAdapterTest {
|
|||||||
|
|
||||||
@Test
|
@Test
|
||||||
void payloadEncryptionRoundTripsThroughTheSubscriptionKeys() {
|
void payloadEncryptionRoundTripsThroughTheSubscriptionKeys() {
|
||||||
var encryptor = new Rfc8291Aes128GcmEncryptor(new SecureRandom());
|
var encryptor = new Rfc8291Aes128GcmEncryptor(RANDOM);
|
||||||
var subscription = subscription();
|
var subscription = subscription();
|
||||||
|
|
||||||
var encrypted =
|
var encrypted =
|
||||||
@@ -135,7 +139,7 @@ class WebPushProviderAdapterTest {
|
|||||||
return new WebPushNotificationProviderAdapter(
|
return new WebPushNotificationProviderAdapter(
|
||||||
new JdkNotificationHttpGateway(Duration.ofSeconds(2)),
|
new JdkNotificationHttpGateway(Duration.ofSeconds(2)),
|
||||||
new WebPushRequestMapper(
|
new WebPushRequestMapper(
|
||||||
new Rfc8291Aes128GcmEncryptor(new SecureRandom()),
|
new Rfc8291Aes128GcmEncryptor(RANDOM),
|
||||||
// Signing needs a PKCS#8 EC key, which VapidJwtSignerTest covers; this keeps the
|
// Signing needs a PKCS#8 EC key, which VapidJwtSignerTest covers; this keeps the
|
||||||
// transport test about TTL, encryption and status mapping.
|
// transport test about TTL, encryption and status mapping.
|
||||||
(endpoint, signingKey, publicKey) -> "vapid t=stub-token, k=" + publicKey,
|
(endpoint, signingKey, publicKey) -> "vapid t=stub-token, k=" + publicKey,
|
||||||
@@ -163,7 +167,7 @@ class WebPushProviderAdapterTest {
|
|||||||
generator.initialize(new ECGenParameterSpec("secp256r1"));
|
generator.initialize(new ECGenParameterSpec("secp256r1"));
|
||||||
KeyPair pair = generator.generateKeyPair();
|
KeyPair pair = generator.generateKeyPair();
|
||||||
byte[] authSecret = new byte[16];
|
byte[] authSecret = new byte[16];
|
||||||
new SecureRandom().nextBytes(authSecret);
|
RANDOM.nextBytes(authSecret);
|
||||||
return new WebPushSubscriptionValue(
|
return new WebPushSubscriptionValue(
|
||||||
URI.create(harness.baseUri() + "/push/subscription-1"),
|
URI.create(harness.baseUri() + "/push/subscription-1"),
|
||||||
Rfc8291Aes128GcmEncryptor.encodePoint((ECPublicKey) pair.getPublic()),
|
Rfc8291Aes128GcmEncryptor.encodePoint((ECPublicKey) pair.getPublic()),
|
||||||
|
|||||||
+6
-2
@@ -61,6 +61,10 @@ import java.util.Optional;
|
|||||||
*/
|
*/
|
||||||
public final class ContractAdapters {
|
public final class ContractAdapters {
|
||||||
|
|
||||||
|
// One instance: a fresh SecureRandom per call re-seeds from the OS each time, which is slower
|
||||||
|
// and, on a constrained CI runner, can block on entropy.
|
||||||
|
private static final SecureRandom RANDOM = new SecureRandom();
|
||||||
|
|
||||||
private static final Clock CLOCK =
|
private static final Clock CLOCK =
|
||||||
Clock.fixed(Instant.parse("2026-08-14T00:00:00Z"), ZoneOffset.UTC);
|
Clock.fixed(Instant.parse("2026-08-14T00:00:00Z"), ZoneOffset.UTC);
|
||||||
|
|
||||||
@@ -186,7 +190,7 @@ public final class ContractAdapters {
|
|||||||
new WebPushNotificationProviderAdapter(
|
new WebPushNotificationProviderAdapter(
|
||||||
new JdkNotificationHttpGateway(Duration.ofSeconds(2)),
|
new JdkNotificationHttpGateway(Duration.ofSeconds(2)),
|
||||||
new WebPushRequestMapper(
|
new WebPushRequestMapper(
|
||||||
new Rfc8291Aes128GcmEncryptor(new SecureRandom()),
|
new Rfc8291Aes128GcmEncryptor(RANDOM),
|
||||||
// Signing needs a PKCS#8 EC key, which WebPushCryptoTest covers; the suites here
|
// Signing needs a PKCS#8 EC key, which WebPushCryptoTest covers; the suites here
|
||||||
// are about transport and evidence semantics.
|
// are about transport and evidence semantics.
|
||||||
(endpoint, signingKey, publicKey) -> "vapid t=stub-token, k=" + publicKey,
|
(endpoint, signingKey, publicKey) -> "vapid t=stub-token, k=" + publicKey,
|
||||||
@@ -240,7 +244,7 @@ public final class ContractAdapters {
|
|||||||
generator.initialize(new ECGenParameterSpec("secp256r1"));
|
generator.initialize(new ECGenParameterSpec("secp256r1"));
|
||||||
KeyPair pair = generator.generateKeyPair();
|
KeyPair pair = generator.generateKeyPair();
|
||||||
byte[] authSecret = new byte[16];
|
byte[] authSecret = new byte[16];
|
||||||
new SecureRandom().nextBytes(authSecret);
|
RANDOM.nextBytes(authSecret);
|
||||||
return new WebPushSubscriptionValue(
|
return new WebPushSubscriptionValue(
|
||||||
URI.create(harness.baseUri() + "/push/subscription-1"),
|
URI.create(harness.baseUri() + "/push/subscription-1"),
|
||||||
Rfc8291Aes128GcmEncryptor.encodePoint((ECPublicKey) pair.getPublic()),
|
Rfc8291Aes128GcmEncryptor.encodePoint((ECPublicKey) pair.getPublic()),
|
||||||
|
|||||||
Reference in New Issue
Block a user