From 60b00df4c3e20b15302606c330cfc17de4118e51 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Sat, 15 Aug 2026 22:44:38 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[security?= =?UTF-8?q?=20improvement]=20Remove=20environment=20variable=20fallbacks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit YAML 설정 및 Java `@Value` 어노테이션에서 환경 변수 폴백 구문(`:`)을 제거하여 런타임 비밀키가 빈 문자열로 잘못 주입되는 것을 방지하고 KV 레지스트리 마운트 방식을 강제합니다. --- .jules/sentinel.md | 4 ++++ .../viewer/artifact/ArtifactLinkService.java | 16 ++++++++-------- .../viewer/auth/TenantAccessService.java | 6 +++--- .../config/ProductionAuthReadinessConfig.java | 3 ++- .../viewer/controller/ArtifactController.java | 2 +- .../viewer/controller/ConversionController.java | 2 +- src/main/resources/application-buyer-demo.yml | 4 ++-- .../ProductionAuthReadinessConfigTest.java | 2 +- 8 files changed, 22 insertions(+), 17 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index e795cb9dd..df158ff24 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-08-15 - Remove Environment Variable Fallbacks for Secrets +**Vulnerability:** Runtime secrets were injected from environment variables using placeholders with empty fallbacks, violating the org rule to use a KV registry. +**Learning:** This bypasses strict mounting rules and can silently default to empty strings, which could lead to weak or missing security signatures. +**Prevention:** Remove environment variable fallback syntax from YAML configs and Java `@Value` annotations, enforcing the Spring Boot configtree mounting mechanism. diff --git a/src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java b/src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java index 79a972e3d..e1caf58bb 100644 --- a/src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java +++ b/src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java @@ -77,9 +77,9 @@ public class ArtifactLinkService { */ @Autowired public ArtifactLinkService( - ArtifactStore artifactStore, - ArtifactLinkLedger artifactLinkLedger, - @Value("${clearfolio.artifact-token.secret:}") String configuredSecret) { + final ArtifactStore artifactStore, + final ArtifactLinkLedger artifactLinkLedger, + @Value("${clearfolio.artifact-token.secret}") final String configuredSecret) { this(artifactStore, artifactLinkLedger, configuredSecret, Clock.systemUTC(), new SecureRandom()); } @@ -90,13 +90,13 @@ public ArtifactLinkService( * @param configuredSecret optional deployment secret */ public ArtifactLinkService( - ArtifactStore artifactStore, - String configuredSecret) { + final ArtifactStore artifactStore, + final String configuredSecret) { this(artifactStore, new ArtifactLinkLedger(), configuredSecret, Clock.systemUTC(), new SecureRandom()); } ArtifactLinkService( - ArtifactStore artifactStore, + final ArtifactStore artifactStore, String configuredSecret, Clock clock, SecureRandom secureRandom) { @@ -104,8 +104,8 @@ public ArtifactLinkService( } ArtifactLinkService( - ArtifactStore artifactStore, - ArtifactLinkLedger artifactLinkLedger, + final ArtifactStore artifactStore, + final ArtifactLinkLedger artifactLinkLedger, String configuredSecret, Clock clock, SecureRandom 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..22a696099 100644 --- a/src/main/java/com/clearfolio/viewer/auth/TenantAccessService.java +++ b/src/main/java/com/clearfolio/viewer/auth/TenantAccessService.java @@ -47,12 +47,12 @@ public TenantAccessService() { */ @Autowired public TenantAccessService( - @Value("${clearfolio.tenant-claims.hmac-secret:}") String claimsHmacSecret, - @Value("${clearfolio.tenant-claims.max-skew-seconds:300}") long maxSkewSeconds) { + @Value("${clearfolio.tenant-claims.hmac-secret}") final String claimsHmacSecret, + @Value("${clearfolio.tenant-claims.max-skew-seconds:300}") final long maxSkewSeconds) { this(claimsHmacSecret, maxSkewSeconds, Clock.systemUTC()); } - TenantAccessService(String claimsHmacSecret, long maxSkewSeconds, Clock clock) { + TenantAccessService(final String claimsHmacSecret, final long maxSkewSeconds, final Clock clock) { this.claimsHmacSecret = clean(claimsHmacSecret); this.maxSkewSeconds = Math.max(0L, maxSkewSeconds); this.clock = clock; diff --git a/src/main/java/com/clearfolio/viewer/config/ProductionAuthReadinessConfig.java b/src/main/java/com/clearfolio/viewer/config/ProductionAuthReadinessConfig.java index dfeb00a59..86f77a24f 100644 --- a/src/main/java/com/clearfolio/viewer/config/ProductionAuthReadinessConfig.java +++ b/src/main/java/com/clearfolio/viewer/config/ProductionAuthReadinessConfig.java @@ -18,7 +18,8 @@ public class ProductionAuthReadinessConfig { * @param tenantClaimsSecret shared gateway signing secret */ public ProductionAuthReadinessConfig( - @Value("${clearfolio.tenant-claims.hmac-secret:}") String tenantClaimsSecret) { + @Value("${clearfolio.tenant-claims.hmac-secret}") + final String tenantClaimsSecret) { if (!StringUtils.hasText(tenantClaimsSecret)) { throw new IllegalStateException( "production profile requires clearfolio.tenant-claims.hmac-secret" diff --git a/src/main/java/com/clearfolio/viewer/controller/ArtifactController.java b/src/main/java/com/clearfolio/viewer/controller/ArtifactController.java index 881b98f37..e21470756 100644 --- a/src/main/java/com/clearfolio/viewer/controller/ArtifactController.java +++ b/src/main/java/com/clearfolio/viewer/controller/ArtifactController.java @@ -57,7 +57,7 @@ public class ArtifactController { public ArtifactController( DocumentConversionService conversionService, ArtifactStore artifactStore, - ArtifactLinkService artifactLinkService, + final ArtifactLinkService artifactLinkService, TenantAccessService tenantAccessService) { this.conversionService = conversionService; this.artifactStore = artifactStore; diff --git a/src/main/java/com/clearfolio/viewer/controller/ConversionController.java b/src/main/java/com/clearfolio/viewer/controller/ConversionController.java index efe6f7562..3979df9f7 100644 --- a/src/main/java/com/clearfolio/viewer/controller/ConversionController.java +++ b/src/main/java/com/clearfolio/viewer/controller/ConversionController.java @@ -73,7 +73,7 @@ public class ConversionController { public ConversionController( DocumentConversionService conversionService, TenantAccessService tenantAccessService, - ArtifactLinkService artifactLinkService, + final ArtifactLinkService artifactLinkService, ArtifactStore artifactStore, @Value("${spring.codec.max-in-memory-size:262144B}") DataSize maxInMemorySize) { this.conversionService = conversionService; diff --git a/src/main/resources/application-buyer-demo.yml b/src/main/resources/application-buyer-demo.yml index 4e4df4091..2cd0f3732 100644 --- a/src/main/resources/application-buyer-demo.yml +++ b/src/main/resources/application-buyer-demo.yml @@ -23,9 +23,9 @@ viewer: clearfolio: artifact-token: - secret: ${CLEARFOLIO_ARTIFACT_TOKEN_SECRET:} + secret: ${CLEARFOLIO_ARTIFACT_TOKEN_SECRET} tenant-claims: - hmac-secret: ${CLEARFOLIO_TENANT_CLAIMS_HMAC_SECRET:} + hmac-secret: ${CLEARFOLIO_TENANT_CLAIMS_HMAC_SECRET} max-skew-seconds: ${CLEARFOLIO_TENANT_CLAIMS_MAX_SKEW_SECONDS:300} artifact-link-ledger: path: ${CLEARFOLIO_ARTIFACT_LINK_LEDGER_PATH:} diff --git a/src/test/java/com/clearfolio/viewer/config/ProductionAuthReadinessConfigTest.java b/src/test/java/com/clearfolio/viewer/config/ProductionAuthReadinessConfigTest.java index f00ba4c62..dd4357c8c 100644 --- a/src/test/java/com/clearfolio/viewer/config/ProductionAuthReadinessConfigTest.java +++ b/src/test/java/com/clearfolio/viewer/config/ProductionAuthReadinessConfigTest.java @@ -9,7 +9,7 @@ class ProductionAuthReadinessConfigTest { @Test void productionProfileFailsWithoutSignedTenantClaimsSecret() { - productionRunner().run(context -> assertThat(context.getStartupFailure()) + productionRunner().withPropertyValues("clearfolio.tenant-claims.hmac-secret=").run(context -> assertThat(context.getStartupFailure()) .hasRootCauseMessage("production profile requires clearfolio.tenant-claims.hmac-secret")); }