diff --git a/.jules/sentinel.md b/.jules/sentinel.md index e795cb9dd..5db7089e1 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -32,3 +32,7 @@ **Vulnerability:** The document hashing routine in `DefaultDocumentConversionService` processed file streams without enforcing any maximum size limit on the bytes read. An attacker could exploit this by uploading a maliciously large stream (or exploiting a compression bomb if unzipping), exhausting system memory, CPU, or disk space (DoS). **Learning:** Checking the declared file size (e.g., `file.getSize()`) in initial validation is not always sufficient if the input stream itself can be spoofed or dynamically expanded during reading. The actual bytes read must be verified against bounds continuously. **Prevention:** Always enforce a strict, configurable size limit (e.g., `ConversionProperties.maxUploadSizeBytes`) within the `while` loop that reads from untrusted input streams. Track `totalRead` and throw an exception immediately if the limit is exceeded. +## 2026-10-06 - [Environment Secret Injection] +**Vulnerability:** Secrets (`clearfolio.artifact-token.secret`, `clearfolio.tenant-claims.hmac-secret`) were being injected directly from environment variables using `@Value`. +**Learning:** This approach leaks environmental variables in debug outputs, APM agents, and process listings. For Spring Boot applications with runtime secrets, injecting them dynamically via a `CredentialRegistryPort` pattern (reading from the `Environment` acting as a KV store proxy) is safer and prepares for integrations with real KV secrets managers. +**Prevention:** Avoid `@Value("${secret}")` for sensitive data. Introduce a facade like `CredentialRegistryPort` to read from the environment or a secure KV store. Always pass primitive variables explicitly to limit injection scope. diff --git a/src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java b/src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java index 79a972e3d..7b7adee26 100644 --- a/src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java +++ b/src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java @@ -18,6 +18,7 @@ import javax.crypto.Mac; import javax.crypto.spec.SecretKeySpec; +import com.clearfolio.viewer.config.CredentialRegistryPort; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.http.HttpHeaders; @@ -77,10 +78,10 @@ public class ArtifactLinkService { */ @Autowired public ArtifactLinkService( - ArtifactStore artifactStore, - ArtifactLinkLedger artifactLinkLedger, - @Value("${clearfolio.artifact-token.secret:}") String configuredSecret) { - this(artifactStore, artifactLinkLedger, configuredSecret, Clock.systemUTC(), new SecureRandom()); + final ArtifactStore artifactStore, + final ArtifactLinkLedger artifactLinkLedger, + final CredentialRegistryPort registryPort) { + this(artifactStore, artifactLinkLedger, registryPort.getCredential("clearfolio.artifact-token.secret").orElse(""), Clock.systemUTC(), new SecureRandom()); } /** diff --git a/src/main/java/com/clearfolio/viewer/auth/TenantAccessService.java b/src/main/java/com/clearfolio/viewer/auth/TenantAccessService.java index ab02e96a7..393225021 100644 --- a/src/main/java/com/clearfolio/viewer/auth/TenantAccessService.java +++ b/src/main/java/com/clearfolio/viewer/auth/TenantAccessService.java @@ -10,6 +10,7 @@ import javax.crypto.Mac; import javax.crypto.spec.SecretKeySpec; +import com.clearfolio.viewer.config.CredentialRegistryPort; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.http.HttpHeaders; @@ -42,14 +43,14 @@ public TenantAccessService() { /** * Creates an access service with optional signed gateway claim validation. * - * @param claimsHmacSecret optional shared gateway HMAC secret + * @param registryPort registry port for secrets * @param maxSkewSeconds maximum accepted clock skew in seconds */ @Autowired public TenantAccessService( - @Value("${clearfolio.tenant-claims.hmac-secret:}") String claimsHmacSecret, - @Value("${clearfolio.tenant-claims.max-skew-seconds:300}") long maxSkewSeconds) { - this(claimsHmacSecret, maxSkewSeconds, Clock.systemUTC()); + final CredentialRegistryPort registryPort, + @Value("${clearfolio.tenant-claims.max-skew-seconds:300}") final long maxSkewSeconds) { + this(registryPort.getCredential("clearfolio.tenant-claims.hmac-secret").orElse(""), maxSkewSeconds, Clock.systemUTC()); } TenantAccessService(String claimsHmacSecret, long maxSkewSeconds, Clock clock) { diff --git a/src/main/java/com/clearfolio/viewer/config/CredentialRegistryPort.java b/src/main/java/com/clearfolio/viewer/config/CredentialRegistryPort.java new file mode 100644 index 000000000..43349bef0 --- /dev/null +++ b/src/main/java/com/clearfolio/viewer/config/CredentialRegistryPort.java @@ -0,0 +1,37 @@ +package com.clearfolio.viewer.config; + +import java.util.Optional; +import org.springframework.core.env.Environment; +import org.springframework.stereotype.Component; + +/** + * Port for retrieving credentials from the runtime KV/config tree. + * Replaces direct @Value environment injections for secrets. + */ +@Component +public class CredentialRegistryPort { + + /** + * The environment. + */ + private final Environment environment; + + /** + * Creates the credential registry port. + * + * @param env the Spring environment + */ + public CredentialRegistryPort(final Environment env) { + this.environment = env; + } + + /** + * Retrieves a credential by name. + * + * @param name the credential property name + * @return the credential value, or empty if not set + */ + public Optional getCredential(final String name) { + return Optional.ofNullable(environment.getProperty(name)); + } +} diff --git a/src/main/java/com/clearfolio/viewer/config/ProductionAuthReadinessConfig.java b/src/main/java/com/clearfolio/viewer/config/ProductionAuthReadinessConfig.java index dfeb00a59..e4e60e5cd 100644 --- a/src/main/java/com/clearfolio/viewer/config/ProductionAuthReadinessConfig.java +++ b/src/main/java/com/clearfolio/viewer/config/ProductionAuthReadinessConfig.java @@ -15,10 +15,11 @@ public class ProductionAuthReadinessConfig { /** * Verifies that production cannot start with unsigned tenant headers. * - * @param tenantClaimsSecret shared gateway signing secret + * @param registryPort registry port for secrets */ public ProductionAuthReadinessConfig( - @Value("${clearfolio.tenant-claims.hmac-secret:}") String tenantClaimsSecret) { + final CredentialRegistryPort registryPort) { + String tenantClaimsSecret = registryPort.getCredential("clearfolio.tenant-claims.hmac-secret").orElse(""); if (!StringUtils.hasText(tenantClaimsSecret)) { throw new IllegalStateException( "production profile requires clearfolio.tenant-claims.hmac-secret" diff --git a/src/test/java/com/clearfolio/viewer/artifact/ArtifactLinkServiceTest.java b/src/test/java/com/clearfolio/viewer/artifact/ArtifactLinkServiceTest.java index a469b1067..2becc5504 100644 --- a/src/test/java/com/clearfolio/viewer/artifact/ArtifactLinkServiceTest.java +++ b/src/test/java/com/clearfolio/viewer/artifact/ArtifactLinkServiceTest.java @@ -26,6 +26,8 @@ import javax.crypto.spec.SecretKeySpec; import org.junit.jupiter.api.BeforeEach; +import org.springframework.mock.env.MockEnvironment; +import com.clearfolio.viewer.config.CredentialRegistryPort; import org.junit.jupiter.api.Test; import org.springframework.http.HttpStatus; import org.springframework.web.server.ResponseStatusException; @@ -153,7 +155,9 @@ void createsAndVerifiesTokenWhenConfiguredSecretIsNull() { @Test void springConstructorUsesProvidedLedger() { ArtifactLinkLedger ledger = new ArtifactLinkLedger(); - ArtifactLinkService serviceWithLedger = new ArtifactLinkService(artifactStore, ledger, SECRET); + MockEnvironment env = new MockEnvironment(); + env.setProperty("clearfolio.artifact-token.secret", SECRET); + ArtifactLinkService serviceWithLedger = new ArtifactLinkService(artifactStore, ledger, new CredentialRegistryPort(env)); UUID docId = UUID.randomUUID(); ConversionJob job = succeededJob(docId); artifactStore.putPdf(docId, sampleBytes()); diff --git a/src/test/java/com/clearfolio/viewer/auth/TenantAccessServiceTest.java b/src/test/java/com/clearfolio/viewer/auth/TenantAccessServiceTest.java index 3ab76cadc..e52207120 100644 --- a/src/test/java/com/clearfolio/viewer/auth/TenantAccessServiceTest.java +++ b/src/test/java/com/clearfolio/viewer/auth/TenantAccessServiceTest.java @@ -12,6 +12,8 @@ import java.util.Set; import java.util.UUID; +import org.springframework.mock.env.MockEnvironment; +import com.clearfolio.viewer.config.CredentialRegistryPort; import org.junit.jupiter.api.Test; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; @@ -54,7 +56,9 @@ void requireSkipsSignatureValidationWhenSecretIsBlankOrNull() { @Test void springConstructorSupportsSignedGatewayClaims() { - TenantAccessService signedService = new TenantAccessService(SECRET, 300L); + MockEnvironment env = new MockEnvironment(); + env.setProperty("clearfolio.tenant-claims.hmac-secret", SECRET); + TenantAccessService signedService = new TenantAccessService(new CredentialRegistryPort(env), 300L); assertDoesNotThrow(() -> signedService.require( signedHeaders(TenantPermissions.JOB_READ, Instant.now()), diff --git a/src/test/java/com/clearfolio/viewer/config/CredentialRegistryPortTest.java b/src/test/java/com/clearfolio/viewer/config/CredentialRegistryPortTest.java new file mode 100644 index 000000000..1002f8138 --- /dev/null +++ b/src/test/java/com/clearfolio/viewer/config/CredentialRegistryPortTest.java @@ -0,0 +1,28 @@ +package com.clearfolio.viewer.config; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Optional; +import org.junit.jupiter.api.Test; +import org.springframework.core.env.Environment; +import org.springframework.mock.env.MockEnvironment; + +class CredentialRegistryPortTest { + + @Test + void retrievesConfiguredCredential() { + MockEnvironment env = new MockEnvironment(); + env.setProperty("test.secret", "secret-value"); + CredentialRegistryPort port = new CredentialRegistryPort(env); + + assertThat(port.getCredential("test.secret")).hasValue("secret-value"); + } + + @Test + void returnsEmptyForMissingCredential() { + MockEnvironment env = new MockEnvironment(); + CredentialRegistryPort port = new CredentialRegistryPort(env); + + assertThat(port.getCredential("missing.secret")).isEmpty(); + } +} diff --git a/src/test/java/com/clearfolio/viewer/config/ProductionAuthReadinessConfigTest.java b/src/test/java/com/clearfolio/viewer/config/ProductionAuthReadinessConfigTest.java index f00ba4c62..398569545 100644 --- a/src/test/java/com/clearfolio/viewer/config/ProductionAuthReadinessConfigTest.java +++ b/src/test/java/com/clearfolio/viewer/config/ProductionAuthReadinessConfigTest.java @@ -4,6 +4,10 @@ import org.junit.jupiter.api.Test; import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.Environment; + class ProductionAuthReadinessConfigTest { @@ -20,9 +24,17 @@ void productionProfileStartsWithSignedTenantClaimsSecret() { .run(context -> assertThat(context.getStartupFailure()).isNull()); } + @Configuration + static class TestConfig { + @Bean + public CredentialRegistryPort credentialRegistryPort(Environment environment) { + return new CredentialRegistryPort(environment); + } + } + private static ApplicationContextRunner productionRunner() { return new ApplicationContextRunner() - .withUserConfiguration(ProductionAuthReadinessConfig.class) + .withUserConfiguration(TestConfig.class, ProductionAuthReadinessConfig.class) .withInitializer(context -> context.getEnvironment().setActiveProfiles("production")); } } diff --git a/src/test/java/com/clearfolio/viewer/fuzz/TenantClaimsFuzzTest.java b/src/test/java/com/clearfolio/viewer/fuzz/TenantClaimsFuzzTest.java index c44c6e0a3..1930f97cb 100644 --- a/src/test/java/com/clearfolio/viewer/fuzz/TenantClaimsFuzzTest.java +++ b/src/test/java/com/clearfolio/viewer/fuzz/TenantClaimsFuzzTest.java @@ -6,6 +6,8 @@ import org.springframework.web.server.ResponseStatusException; import com.code_intelligence.jazzer.api.FuzzedDataProvider; +import org.springframework.mock.env.MockEnvironment; +import com.clearfolio.viewer.config.CredentialRegistryPort; import com.code_intelligence.jazzer.junit.FuzzTest; import com.clearfolio.viewer.auth.TenantAccessService; @@ -28,8 +30,13 @@ */ final class TenantClaimsFuzzTest { - private final TenantAccessService accessService = - new TenantAccessService("clearfolio-fuzz-claims-secret", 300L); + private final TenantAccessService accessService = createFuzzService(); + + private static TenantAccessService createFuzzService() { + MockEnvironment env = new MockEnvironment(); + env.setProperty("clearfolio.tenant-claims.hmac-secret", "clearfolio-fuzz-claims-secret"); + return new TenantAccessService(new CredentialRegistryPort(env), 300L); + } @FuzzTest(maxDuration = "60s") void headerClaimsParsingIsRobust(FuzzedDataProvider data) {