Skip to content

Commit e302049

Browse files
committed
fix(storage): do not query the encryption key registry when storage is off
Deployments that never enable storage - the SaaS flavour among them - query file_encryption_keys during bean creation, because the decrypt-only probe runs whenever storage.encryption.enabled is false. That table only exists if ddl-auto managed to create it, and ddl-auto logs and continues when it cannot, so a schema warning becomes a refusal to start. Reported against 'task backend:dev:saas', where storage.enabled is false and the schema is Supabase's stirling_pdf. - The boot probe now runs only when storage.enabled is true, so a deployment that does not use storage never touches the table. - Registry reads are wrapped: the boot probe degrades to 'no keys' instead of propagating, and suppressDirectDownloads() fails SAFE (suppresses) rather than handing out a presigned URL it cannot vouch for. Losing the direct-download fast path is recoverable; serving ciphertext is not. Safety is unchanged. The decorator is still installed unconditionally, so any blob carrying the SPDFEAR1 magic is decrypted via lazy materialisation or fails loudly; the probe only ever bought earlier master-key verification. A node that can actually serve stored files has storage.enabled by definition, which is why the drifted-node test now configures it that way. Tests: storage-disabled never calls count(); an unreadable registry still boots and still suppresses direct downloads; the decorator stays installed with storage off; storage-enabled still probes.
1 parent 7a5eb73 commit e302049

3 files changed

Lines changed: 104 additions & 6 deletions

File tree

app/proprietary/src/main/java/stirling/software/proprietary/storage/config/StorageProviderConfig.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,12 @@ public StorageEncryptionState storageEncryptionState(
7070
createKeyService(
7171
configuredFileEncryptionKey, clusterEnabled, requiresNew),
7272
fileEncryptionKeyRepository);
73-
if (writeEnabled || fileEncryptionKeyRepository.count() > 0) {
73+
// Only probe the key registry when storage is actually in use. Deployments that never
74+
// enable storage (the SaaS flavour among them) may not have the table at all, and a
75+
// startup query for a feature they do not use must not be able to stop them booting.
76+
boolean probeForExistingKeys =
77+
!writeEnabled && applicationProperties.getStorage().isEnabled();
78+
if (writeEnabled || (probeForExistingKeys && state.encryptedContentMayExist())) {
7479
state.initialiseEagerly();
7580
log.info(
7681
"Storage encryption at rest active (writes {})",

app/proprietary/src/main/java/stirling/software/proprietary/storage/crypto/StorageEncryptionState.java

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package stirling.software.proprietary.storage.crypto;
22

33
import java.time.Duration;
4+
import java.util.Optional;
45
import java.util.function.Supplier;
56

67
import lombok.extern.slf4j.Slf4j;
@@ -91,6 +92,9 @@ public void initialiseEagerly() {
9192
* The registry check is cached briefly, so installs that never enable encryption keep the S3
9293
* fast path. Per-file precision (redirecting plaintext blobs via {@code
9394
* StoredFile.encryptionKeyId}) is a deliberate follow-up.
95+
*
96+
* <p>If the registry cannot be read at all, this suppresses rather than delegating: losing the
97+
* direct-download fast path is recoverable, handing out ciphertext is not.
9498
*/
9599
public boolean suppressDirectDownloads() {
96100
if (writeEnabled || keyService != null) {
@@ -104,9 +108,40 @@ public boolean suppressDirectDownloads() {
104108
&& now - keysExistCheckedAtNanos < KEYS_EXIST_CACHE_TTL.toNanos()) {
105109
return keysExistCached;
106110
}
107-
keysExistCached = keyRepository.count() > 0;
111+
keysExistCached = countKeys().orElse(true /* unreadable registry: fail safe */);
108112
keysExistCheckedAtNanos = now;
109113
keysExistEverChecked = true;
110114
return keysExistCached;
111115
}
116+
117+
/**
118+
* True when key rows exist, empty when the registry could not be read.
119+
*
120+
* <p>The registry lives in whatever schema the deployment uses, and deployments that never
121+
* enable storage (the SaaS flavour, for one) may not have the table at all — {@code
122+
* ddl-auto=update} logs and continues when it cannot create it. So a failure here must never
123+
* propagate: it would turn a schema warning into a refusal to serve, or to start.
124+
*/
125+
private Optional<Boolean> countKeys() {
126+
try {
127+
return Optional.of(keyRepository.count() > 0);
128+
} catch (RuntimeException e) {
129+
log.warn(
130+
"Could not read the storage encryption key registry ({}). Treating direct"
131+
+ " downloads as unsafe; encrypted content is still decrypted on"
132+
+ " demand.",
133+
e.getMessage());
134+
return Optional.empty();
135+
}
136+
}
137+
138+
/**
139+
* Startup probe for decrypt-only mode: are there key rows even though writes are plaintext?
140+
* Returns false — rather than throwing — when the registry is unreadable, so a deployment that
141+
* never uses storage still starts. Safety is preserved because the decorator sniffs every
142+
* blob's header and materialises the key machinery lazily if it meets ciphertext.
143+
*/
144+
public boolean encryptedContentMayExist() {
145+
return countKeys().orElse(false);
146+
}
112147
}

app/proprietary/src/test/java/stirling/software/proprietary/storage/config/StorageProviderConfigTest.java

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,19 @@
44
import static org.assertj.core.api.Assertions.assertThatCode;
55
import static org.assertj.core.api.Assertions.assertThatThrownBy;
66
import static org.mockito.ArgumentMatchers.anyString;
7+
import static org.mockito.Mockito.atLeastOnce;
78
import static org.mockito.Mockito.doAnswer;
89
import static org.mockito.Mockito.doNothing;
910
import static org.mockito.Mockito.mock;
11+
import static org.mockito.Mockito.never;
12+
import static org.mockito.Mockito.verify;
1013
import static org.mockito.Mockito.when;
1114

1215
import java.util.Base64;
1316
import java.util.Optional;
1417

1518
import org.junit.jupiter.api.Test;
19+
import org.springframework.dao.InvalidDataAccessResourceUsageException;
1620
import org.springframework.transaction.PlatformTransactionManager;
1721

1822
import stirling.software.common.model.ApplicationProperties;
@@ -24,6 +28,7 @@
2428
import stirling.software.proprietary.storage.crypto.InMemoryKeyRepo;
2529
import stirling.software.proprietary.storage.crypto.StorageEncryptionState;
2630
import stirling.software.proprietary.storage.provider.StorageProvider;
31+
import stirling.software.proprietary.storage.repository.FileEncryptionKeyRepository;
2732
import stirling.software.proprietary.storage.repository.StoredFileBlobRepository;
2833

2934
/**
@@ -66,16 +71,18 @@ void decorator_writeEnabled_requiresLicenceAndSuppressesDirectDownloads() {
6671

6772
@Test
6873
void decorator_flagOffButKeysExist_decryptOnlyModeStillMaterialises() throws Exception {
69-
// Simulate the drifted-node case: another node already created keys.
70-
StorageProviderConfig seedCfg = newConfig("local", License.SERVER, true);
74+
// The drifted-node case: another node already created keys. Storage is enabled here
75+
// because a node that can actually serve stored files necessarily has it on - that is
76+
// precisely the node that must not hand out ciphertext.
77+
StorageProviderConfig seedCfg = newConfig("local", License.SERVER, true, true);
7178
StorageEncryptionState seedState = seedCfg.storageEncryptionState(MASTER, false, txManager);
7279
Team team = new Team();
7380
team.setId(1L);
7481
User owner = new User();
7582
owner.setTeam(team);
7683
seedState.keyService().activeKekForOwner(owner);
7784

78-
StorageProviderConfig cfg = newConfig("local", License.NORMAL, false);
85+
StorageProviderConfig cfg = newConfig("local", License.NORMAL, false, true);
7986
StorageEncryptionState state = cfg.storageEncryptionState(MASTER, false, txManager);
8087

8188
assertThat(cfg.storageProvider(state, Optional.empty()))
@@ -104,6 +111,52 @@ void encryption_enabled_wrongLengthKey_failsStartup() {
104111
.hasMessageContaining("32 bytes");
105112
}
106113

114+
// ---- deployments that do not use storage (the SaaS shape) ---------------------------
115+
116+
@Test
117+
void storageDisabled_neverQueriesTheKeyRegistry() {
118+
// The SaaS flavour ships storage.enabled=false and may not have the table at all. A
119+
// startup query for a feature it does not use must not happen, let alone fail the boot.
120+
StorageProviderConfig cfg = newConfig("local", License.NORMAL, false);
121+
StorageEncryptionState state = cfg.storageEncryptionState(MASTER, false, txManager);
122+
123+
assertThat(state.isWriteEnabled()).isFalse();
124+
verify(keyRepo.mock, never()).count();
125+
}
126+
127+
@Test
128+
void storageDisabled_decoratorStillInstalledSoCiphertextIsNeverServedRaw() {
129+
// Skipping the boot probe does not weaken safety: the decorator is still wrapped, so any
130+
// blob carrying the magic is decrypted (or fails loudly) via lazy materialisation.
131+
StorageProviderConfig cfg = newConfig("local", License.NORMAL, false);
132+
StorageEncryptionState state = cfg.storageEncryptionState(MASTER, false, txManager);
133+
134+
assertThat(cfg.storageProvider(state, Optional.empty()))
135+
.isInstanceOf(EncryptingStorageProvider.class);
136+
}
137+
138+
@Test
139+
void unreadableKeyRegistry_stillStartsAndFailsSafeOnDirectDownloads() {
140+
// Simulates the table being absent: ddl-auto=update logs and continues when it cannot
141+
// create it, so a query against it throws at runtime.
142+
FileEncryptionKeyRepository broken = mock(FileEncryptionKeyRepository.class);
143+
when(broken.count())
144+
.thenThrow(new InvalidDataAccessResourceUsageException("no such table"));
145+
StorageEncryptionState state = new StorageEncryptionState(false, () -> null, broken);
146+
147+
// Boot-time probe degrades to "no keys" rather than propagating...
148+
assertThat(state.encryptedContentMayExist()).isFalse();
149+
// ...but the request path fails safe: never hand out a presigned URL we cannot vouch for.
150+
assertThat(state.suppressDirectDownloads()).isTrue();
151+
}
152+
153+
@Test
154+
void storageEnabledWithoutEncryption_probesRegistry() {
155+
StorageProviderConfig cfg = newConfig("local", License.NORMAL, false, true);
156+
cfg.storageEncryptionState(MASTER, false, txManager);
157+
verify(keyRepo.mock, atLeastOnce()).count();
158+
}
159+
107160
// ---- backend licence gates (unchanged behaviour) ------------------------------------
108161

109162
@Test
@@ -161,9 +214,14 @@ void provider_unknown_throwsUnsupportedProvider_notLicense() {
161214

162215
private StorageProviderConfig newConfig(
163216
String provider, License license, boolean encryptionEnabled) {
217+
return newConfig(provider, license, encryptionEnabled, false);
218+
}
219+
220+
private StorageProviderConfig newConfig(
221+
String provider, License license, boolean encryptionEnabled, boolean storageEnabled) {
164222
ApplicationProperties props = new ApplicationProperties();
165223
props.getStorage().setProvider(provider);
166-
props.getStorage().setEnabled(false); // local-fallback path skips dir creation
224+
props.getStorage().setEnabled(storageEnabled);
167225
props.getStorage().getEncryption().setEnabled(encryptionEnabled);
168226
StoredFileBlobRepository repo = mock(StoredFileBlobRepository.class);
169227
LicenseKeyChecker checker = mock(LicenseKeyChecker.class);

0 commit comments

Comments
 (0)