Skip to content

fix(storage): don't query the encryption key registry when storage is off (unblocks backend:dev:saas) - #7265

Merged
ConnorYoh merged 1 commit into
mainfrom
claude/fix-saas-storage-encryption-probe
Aug 3, 2026
Merged

fix(storage): don't query the encryption key registry when storage is off (unblocks backend:dev:saas)#7265
ConnorYoh merged 1 commit into
mainfrom
claude/fix-saas-storage-encryption-probe

Conversation

@ConnorYoh

Copy link
Copy Markdown
Member

Description of Changes

Fixes a startup failure introduced by #7155 and reported against task backend:dev:saas.

What goes wrong

StorageProviderConfig.storageEncryptionState(...) is created on every startup, in every profile. When storage.encryption.enabled is false — the default, and what SaaS ships — the || short-circuit evaluates fileEncryptionKeyRepository.count(), a live query against file_encryption_keys:

if (writeEnabled || fileEncryptionKeyRepository.count() > 0) {   // <- always runs when the flag is off

That table only exists if ddl-auto=update managed to create it. When it cannot — permissions on a shared Supabase branch DB, concurrent DDL from several developers, schema ordering — ddl-auto logs and continues, so the situation used to be a warning nobody noticed. Now it is a query that throws during bean creation and takes the whole context down.

Two things make this sting in SaaS specifically: storage.enabled is false there, so before this feature nothing ever touched the table; and hibernate.default_schema=stirling_pdf means the table has to exist in a schema the app may not be able to create in.

There is a second exposure on the request path: suppressDirectDownloads() also counts (60s cached), so even a surviving boot could 500 on downloads.

Fix

  • The boot probe 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" rather than propagating; suppressDirectDownloads() fails safe by suppressing rather than issuing a presigned URL it cannot vouch for. Losing the direct-download fast path is recoverable; serving ciphertext is not.

Safety is unchanged, and that is the important part. The decorator is still installed unconditionally, so any blob carrying the SPDFEAR1 magic is still decrypted via lazy materialisation or fails loudly — the eager probe only ever bought earlier master-key verification. A node that can actually serve stored files has storage.enabled on by definition, which is exactly the node the drifted-node protection is for; that test now configures it that way, and a new test pins that the decorator remains installed even with storage off.

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. Full proprietary suite green apart from the pre-existing Windows-symlink FolderIdentitiesTest failure, which is environmental and unrelated.

Note on scope: deliberately minimal so it can land quickly. The Aikido findAll() code-quality finding lives in #7173 only (rotateMasterKey does not exist on main), so it is fixed there rather than here. #7173 will be rebased once this merges.


Checklist

General

Documentation

Translations (if applicable)

UI Changes (if applicable)

  • Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR)

Testing (if applicable)

  • I have run task check to verify linters, typechecks, and tests pass
  • I have tested my changes locally. Refer to the Testing Guide for more details.

@dosubot dosubot Bot added size:M This PR changes 30-99 lines ignoring generated files. Bugfix Pull requests that fix bugs labels Aug 3, 2026
@stirlingbot stirlingbot Bot added Java Pull requests that update Java code Test Testing-related issues or pull requests labels Aug 3, 2026
jbrunton96
jbrunton96 previously approved these changes Aug 3, 2026
…s 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.
@ConnorYoh
ConnorYoh force-pushed the claude/fix-saas-storage-encryption-probe branch from e302049 to 0576a27 Compare August 3, 2026 14:49
@dosubot dosubot Bot added size:S This PR changes 10-29 lines ignoring generated files. and removed size:M This PR changes 30-99 lines ignoring generated files. labels Aug 3, 2026
@ConnorYoh
ConnorYoh added this pull request to the merge queue Aug 3, 2026
Merged via the queue into main with commit 50bc4a7 Aug 3, 2026
40 checks passed
@ConnorYoh
ConnorYoh deleted the claude/fix-saas-storage-encryption-probe branch August 3, 2026 17:13
ConnorYoh added a commit that referenced this pull request Aug 4, 2026
Addresses the Aikido code-quality finding on #7155: rotateMasterKey() and
verifyMasterKey() loaded every key row and filtered in memory. Both now push
the predicate into the query (findByMasterKeyVersionLessThan /
countByMasterKeyVersionLessThan), so the rotation check costs a counted
query rather than a full table read.

The SaaS boot-probe fix that originally shared this commit landed on its own
in #7265, so only the rebase leftovers remain here: one assertion proving a
storage-disabled node resolves no master key at boot.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bugfix Pull requests that fix bugs Java Pull requests that update Java code size:S This PR changes 10-29 lines ignoring generated files. Test Testing-related issues or pull requests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants