feat(storage): encryption-at-rest ops — audit, admin kill switch, migration, key rotation (PR2) by ConnorYoh · Pull Request #7173 · Stirling-Tools/Stirling-PDF

ConnorYoh · 2026-07-27T14:14:25Z

Description of Changes

PR2 of the encrypt-at-rest initiative — PR1 was #7155 Makes the P1 crypto operable and compliance-credible: admins can see the feature's state, flip the kill switch over an API instead of raw SQL, encrypt the pre-existing plaintext backlog, rotate the master key, and every security-relevant event lands in the audit trail. No frontend — that's PR3.

What was changed

Audit events — new STORAGE_ENCRYPTION audit type, emitted through a small listener interface so the crypto classes stay plain objects: encrypt, decrypt (per-read events honour storage.encryption.auditReads, default on — HIPAA reviewers expect read audit; busy installs can disable), decrypt.denied (always), key.created/disabled/enabled, master.rotated, migration.completed, plus a plaintextExport marker whenever a plaintext copy of encrypted-at-rest content is served (with inline flag to distinguish in-app view from saved download).
Admin API /api/v1/admin/storage-encryption (hasRole('ADMIN')):
- GET /status — write/decrypt state, master-key fingerprint (SHA-256 prefix for backup verification, never key material), encrypted vs plaintext file counts, full key list with status history.
- POST /keys/{id}/disable / enable — the kill switch, now with active cache invalidation so revocation is immediate on the handling node (cross-node converges within the 60s cache TTL). Enable is restricted to DISABLED keys so two ACTIVE keys can't exist per scope.
- POST /migrate + GET /migrate/status — encrypt-existing job.
- POST /master/rotate — key material is never accepted over HTTP; keys come from config/env.
- Deliberately no delete endpoint — key material can be disabled but never destroyed through the API.
Encrypt-existing migration job — new writes are encrypted from the moment the flag is on; this converts the backlog. Crash-safe per file: store the encrypted copy under a NEW storage key → compare-and-swap the DB row → only then delete the old blob. A CAS miss (user replaced the file mid-run) discards the job's copy — the user's file always wins. Worst crash outcome is an orphaned blob, never a lost file; re-runs are idempotent (encryption_key_id IS NULL selection, cursor-paged so failures can't wedge the loop). Handles all three blobs per row (main/history/audit-log), runs on a throttled virtual thread, single-flight guarded.
Master-key rotation — cheap by design thanks to the P1 hierarchy: rotate re-wraps the handful of KEK rows, zero file I/O. New config stirling.security.fileEncryptionKeyPrevious (+env) gives unwrap a fallback during rotation, and stirling.security.fileEncryptionKeyVersion marks which master wrapped each row. Runbook: set new key primary + old as previous + bump version → restart (startup self-check passes via fallback, warns about pending rows) → POST /master/rotate → remove the previous key.
Shared state bean — StorageEncryptionState is built once and shared by the storage decorator and the admin API, so kill-switch cache invalidation hits the same caches the decorator reads.

Reviewer notes

The revoked→403 mapping promised for PR2 already landed in feat(storage): encryption at rest for stored files (per-team envelope encryption) #7155 after manual testing; this PR adds the matching decrypt.denied audit event.
19 new tests: audit emission (encrypt/decrypt/denied, legacy plaintext emits nothing), kill-switch immediacy (no TTL wait), rotation (previous-key fallback, re-wrap + cleanup, idempotent second call), migration (backlog encrypted byte-identical, CAS-miss discards own copy, per-file failure counting, concurrent-start rejection, write-disabled rejection), admin controller status/conflict/not-found paths.
Full proprietary suite: 2246/2247 green (the one failure is the pre-existing Windows-symlink FolderIdentitiesTest, unrelated).

ENCRYPTION_AT_REST_TEST_REPORT.html

ConnorYoh · 2026-07-31T15:25:49Z

Known limitation: the audit trail requires Enterprise, encryption itself is Pro.

AuditService drops all events unless the licence is Enterprise (runningEE is ENTERPRISE-only), but storage.encryption is gated at requireProOrEnterprise. So on a Pro licence files are encrypted exactly as designed, but none of this PR's audit events are recorded — no decrypt trail, no revocation record, no plaintext-export marker. Accepting that for now rather than widening the audit gate; we can revisit if a Pro customer needs audited encryption.

Made it impossible to hit by surprise (a930a46):

Startup logs a WARN when encryption is enabled without an Enterprise licence, stating that encryption is unaffected but the events will not be recorded.
devGuide/STORAGE_ENCRYPTION_AT_REST.md carries it as a callout at the top and in the Auditing section, framed around the compliance case (HIPAA/CMMC audit-logging requirements).
The settings template notes it next to auditReads.

Also in that commit, from a self-review pass over the ops layer:

Migration size fallback — a resource reporting an unknown content length (-1) now falls back to the plaintext size on the row instead of failing the file; the fallback previously only applied when contentLength() threw, so backends that legitimately return -1 would have failed whole classes of files.
New StoredFileMigrationQueriesDbTest — the migration's JPQL now runs against a real database: JOIN FETCH + cursor/page selection, and all three compare-and-swap updates including the stale-key no-match case and that secondary swaps leave encryptionKeyId alone. The service tests mock the repository wholesale, so none of these queries had ever executed; a wrong CAS predicate would have passed CI and silently skipped every file in production.
Documented the audit semantics precisely (a decrypt event means "authorised and opened", not "bytes fully read"; workflow-file downloads are not yet plaintextExport-marked) and the migration's operational limits (in-memory progress so migrate/status resets to IDLE after a restart, no cancel, per-node guard so trigger it on one node).

Remaining known-and-accepted, not fixed here: rotateMasterKey and /status do unbounded findAll scans and rotation is not atomic (recoverable and re-runnable, but a mid-way failure returns 500 without reporting partial progress). Happy to take those as follow-ups.

ConnorYoh · 2026-08-03T14:40:09Z

The SaaS fix from ab8438a is now also up as a standalone hotfix off main — #7265 — because the bug shipped in #7155 and is currently breaking task backend:dev:saas for people on main. That PR is deliberately minimal (probe gating + fail-safe registry reads only).

This PR keeps the Aikido findAll() fix, since both of those sites (verifyMasterKey's pending-row count and rotateMasterKey) are PR2-only code — rotateMasterKey doesn't exist on main. Once #7265 merges I'll rebase this branch onto it; the overlap is the same two files, so expect a small conflict resolution rather than anything structural.

… off (unblocks backend:dev:saas) (Stirling-Tools#7265) # Description of Changes Fixes a startup failure introduced by Stirling-Tools#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`: ```java 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 Stirling-Tools#7173 only (`rotateMasterKey` does not exist on main), so it is fixed there rather than here. Stirling-Tools#7173 will be rebased once this merges. --- ## Checklist ### General - [x] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [x] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md) (if applicable) - [ ] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable) - [x] I have performed a self-review of my own code - [x] My changes generate no new warnings ### Documentation - [ ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [ ] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### Translations (if applicable) - [ ] I ran [`scripts/counter_translation.py`](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/docs/counter_translation.md) ### 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) - [x] I have run `task check` to verify linters, typechecks, and tests pass - [x] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md#7-testing) for more details.

…ch, migration, master rotation PR2 of the encrypt-at-rest initiative, stacked on #7155. Makes the P1 crypto operable and compliance-credible (no frontend yet - PR3): - New STORAGE_ENCRYPTION audit type, emitted via a listener interface so crypto classes stay plain objects: encrypt, decrypt (behind storage.encryption.auditReads, default on), decrypt.denied, key lifecycle, master rotation, migration summary, plus a plaintextExport marker on downloads of encrypted files. - Admin API /api/v1/admin/storage-encryption (ROLE_ADMIN): status with master-key fingerprint + encrypted/plaintext counts, key disable/enable (kill switch, immediate on-node via cache invalidation), migrate + progress, master/rotate. No delete endpoint by design. - Encrypt-existing migration job: cursor-paged, throttled, crash-safe per file (store new blob -> CAS the row -> delete old); a CAS miss means a user replaced the file mid-run and their copy wins. Covers all three blobs per row (main/history/audit-log). - Master-key rotation: fileEncryptionKeyPrevious unwrap fallback + fileEncryptionKeyVersion; /master/rotate re-wraps KEK rows under the primary key (zero file I/O). Startup warns while rows remain on the old master. - StorageEncryptionState bean shares one key service between the storage decorator and the admin API so kill-switch invalidation hits the caches the decorator reads.

…core Rebase of the PR2 ops layer onto the #7155 review fixes: - StorageEncryptionState is now the lazy always-decorate holder from the review fixes, extended with the audit listener; admin API and migration keep sharing the decorator's instance so kill-switch cache invalidation still works. - FileEncryptionKeyService carries both the audit listener and the REQUIRES_NEW key-creation transaction; the admin controller uses the new materialise-on-demand API (409 with a clear message when key machinery cannot be initialised). - fileEncryptionKeyPrevious now goes through the same base64 + 32-byte validation as the primary key. - settings.yml.template documents storage.encryption.auditReads and the master-key rotation runbook.

…late Per review feedback on #7155: the template now carries a short pointer plus the back-up-your-key warning, and the detailed key setup, cluster requirements, encrypt-existing migration, kill switch and master-key rotation runbook live in devGuide/STORAGE_ENCRYPTION_AT_REST.md (indexed in devGuide/README.md).

- Log a startup warning when encryption at rest is enabled without an Enterprise licence: the audit subsystem is Enterprise-gated, so on Pro the encrypt/decrypt, revocation and plaintext-export events are silently dropped while encryption itself works normally. Documented in devGuide/STORAGE_ENCRYPTION_AT_REST.md and noted next to auditReads in the settings template. - Migration: a resource reporting an unknown content length (-1) now falls back to the plaintext size recorded on the row instead of failing the file; previously the fallback only applied when contentLength() threw. - Document the audit-event semantics (a decrypt event means 'authorised and opened', workflow downloads are not yet plaintextExport-marked) and the migration's operational limits (in-memory progress, no cancel, per-node guard). - New StoredFileMigrationQueriesDbTest exercises the migration's JPQL against a real database: the JOIN FETCH + cursor/page selection and all three compare-and-swap updates, including that a stale key does not match and that secondary swaps leave encryptionKeyId alone. The service tests mock the repository, so none of these queries had ever actually executed.

@PreAuthorize

The admin endpoints were only ever exercised as direct Java calls, so path mappings and response serialisation were unverified - a record field Jackson could not render would have passed every existing test and failed the first request. Adds MockMvc coverage of /status, disable/enable, migrate + migrate/status and master/rotate, asserting the JSON shape (UUIDs, Instants, enums, absent finishedAt) and the 404/409 mappings. Also pins two structural guarantees that standaloneSetup cannot enforce: the controller carries @PreAuthorize("hasRole('ADMIN')") (method security is enabled globally), and no handler maps a DELETE, which is what makes 'key material can be disabled but never destroyed' true by construction rather than by convention.

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.

Review follow-ups on the encryption-at-rest ops layer. One ACTIVE key per scope. Revoking a key does not stop the scope from storing new files: the next upload found no active key and minted one. Re-enabling the revoked key then produced two ACTIVE rows, which the unique constraint on (scope, key_version) permits. The consequence was not lost data - each blob pins its own key id - but a later revoke showed two active keys, so an operator who believed they had revoked a team's access had revoked only part of it. Enable now returns the key to ACTIVE only when its scope has no other active key, and to RETIRED otherwise: RETIRED unwraps existing content just the same, so read access is restored without a second key competing for new writes. The active-key lookup also orders by key version so selection is deterministic on every node even against a registry left in the old shape. Migration stops instead of churning. The write-flag-off guard threw per file, and that throw landed in the per-file catch, so every remaining file was still copied as plaintext and deleted again before failing - the exact churn the guard existed to prevent - and the run reported COMPLETED with N failures. Page walking now checks the flag per file and ends the run FAILED, leaving the backlog untouched. The per-file guard stays as a backstop for a flag that flips mid-store, where it now costs one file rather than all of them. Migration start is audited. Only completion was recorded, from the virtual thread, where the principal resolves to "system" - so who triggered a bulk re-encryption of every customer file was unrecoverable. The principal is captured on the request thread and attributed to both the start and completion events. Status no longer 500s on a registry that isn't there. /status read the key registry and the file counts unguarded, so on a storage-disabled install - where the table may not exist, the case PR1 guards for - an admin got a raw SQL error. It now refuses with 403 "Storage is disabled" before touching the database, matching FileStorageService, and reports 503 if the registry is unreadable while storage is on. Revocation reads consistently across paths. StorageKeyRevokedException extends IOException, so the workflow reads - which catch IOException or let it propagate - surfaced a deliberate revocation as 500 while My Files returned a clean 403 for the same blob and key. Both now share one translation helper. Tests: enable-after-upload comes back RETIRED and still decrypts, with one ACTIVE row; two active rows resolve to the same key on independent nodes; a flag flipped mid-run stops after one file with 29 untouched and nothing counted as failed; start and completion both name the admin; storage-disabled /status refuses without touching either repository; an unreadable registry is 503; revoked workflow reads are 403.

…eads Three gaps flagged in review, all in code that only matters when someone is auditing an incident. AuditingStorageEncryptionListener had no test at all. The provider test used a hand-rolled recording listener, so the production class - and with it storage.encryption.auditReads, the one knob a compliance reviewer actually turns - was never exercised. Now pinned both ways: reads off drops decrypt events and keeps encrypt, decrypt.denied and key.created. plaintextExport was untested. Its whole purpose is evidence that a decrypted copy left the platform, and nothing asserted it fires for encrypted files, stays silent for plaintext ones, or distinguishes an in-app view from a saved download. The migration only ran against LocalStorageProvider, whose resources are re-openable, so the size fallback written for one-shot S3 and database resources was never executed. Three cases now cover it: contentLength() refused falls back to the row's recorded size and still encrypts correctly; a wrong recorded size fails that file inside store() before the compare-and-swap, leaving the row pointing at its untouched plaintext blob; and no size available anywhere fails rather than guessing. Note the last is only reachable through a secondary blob - StoredFile.sizeBytes is primitive, so the main blob always has one.

- unwrap now reports the primary master key's failure with the previous key's attached as suppressed. A genuinely corrupt row was being diagnosed through the outgoing key's error message. - An invalid fileEncryptionKeyVersion is warned about rather than silently clamped, and startup warns when rows are wrapped by a version newer than the configured one: rotation only re-wraps rows below it, so a version set too low strands those rows permanently. - migrationService.status() returns Optional rather than null. - Documented that a compare-and-swap miss on a secondary blob abandons the whole file for the run; the next run picks it up from the top. - Replaced fully-qualified inline type names with imports, matching the rest of each file.

A half-finished master-key rotation could leave some scopes' files unreadable while the app looked healthy. verifyMasterKey() unwrapped one sampled row, so whether a mismatch was detected depended on which row the sample happened to hit: if rotation re-wrapped some rows and stopped, sampling a re-wrapped row passed and startup continued, with the rows left behind unreadable. The "still wrapped by the previous master key" warning was also gated on a previous key being configured, so it went silent in exactly the case that matters - the operator having removed it believing rotation had finished. Every row is now verified (the table holds one row per scope per rotation, so this is cheap) and startup refuses with the count and the first affected scope. Recovery is putting the outgoing key back, which is only possible while the operator still has it, so this needs to surface as a failed deploy rather than as unreadable files found later. The pending warning is unconditional. DISABLED rows are included: revocation is advertised as reversible, and a row that cannot be unwrapped would not come back on enable. The rotation runbook gains an explicit verify step before the outgoing key is removed.

A node still carrying only the outgoing master key cannot unwrap a row that has been re-wrapped under the new one, so running the rotate call before every node has both keys makes the lagging nodes fail on those scopes. Completes the rotation runbook this PR already rewrote.

ConnorYoh requested review from Frooodle, Ludy87 and jbrunton96 as code owners July 27, 2026 14:14

dosubot Bot added size:XXL This PR changes 1000+ lines ignoring generated files. enhancement New feature or request labels Jul 27, 2026

aikido-pr-checks Bot reviewed Jul 27, 2026

View reviewed changes

Comment thread ...ary/src/main/java/stirling/software/proprietary/storage/crypto/FileEncryptionKeyService.java Outdated

ConnorYoh mentioned this pull request Jul 31, 2026

feat(storage): encryption at rest for stored files (per-team envelope encryption) #7155

Merged

11 tasks

ConnorYoh force-pushed the claude/pdf-encryption-at-rest-p2 branch from 9d0ff6f to bd9d93a Compare July 31, 2026 09:48

Base automatically changed from claude/pdf-encryption-at-rest-e0c4a2 to main July 31, 2026 15:35

ConnorYoh force-pushed the claude/pdf-encryption-at-rest-p2 branch from b06eb8b to 5a5cca7 Compare July 31, 2026 15:50

stirlingbot Bot removed has conflicts Pull request has merge conflicts with the base branch Gradle Pull requests that update Gradle code labels Jul 31, 2026

reecebrowne previously approved these changes Aug 3, 2026

View reviewed changes

ConnorYoh dismissed reecebrowne’s stale review via ab8438a August 3, 2026 14:31

ConnorYoh mentioned this pull request Aug 3, 2026

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

Merged

11 tasks

stirlingbot Bot added the has conflicts Pull request has merge conflicts with the base branch label Aug 3, 2026

ConnorYoh added 4 commits August 4, 2026 10:59

ConnorYoh added 2 commits August 4, 2026 10:59

ConnorYoh force-pushed the claude/pdf-encryption-at-rest-p2 branch from ab8438a to 6f2b35b Compare August 4, 2026 10:03

stirlingbot Bot removed the has conflicts Pull request has merge conflicts with the base branch label Aug 4, 2026

ConnorYoh and others added 6 commits August 4, 2026 16:29

Merge branch 'main' into claude/pdf-encryption-at-rest-p2

f5cdd9d

reecebrowne approved these changes Aug 10, 2026

View reviewed changes

Frooodle approved these changes Aug 11, 2026

View reviewed changes

Frooodle added this pull request to the merge queue Aug 11, 2026

Merged via the queue into main with commit df170fd Aug 11, 2026
63 of 65 checks passed

Frooodle deleted the claude/pdf-encryption-at-rest-p2 branch August 11, 2026 13:02

ConnorYoh mentioned this pull request Aug 14, 2026

Encryption at rest: operator surface (P3A) #7501

Open

50 tasks

Navigation Menu

Uh oh!

feat(storage): encryption-at-rest ops — audit, admin kill switch, migration, key rotation (PR2) - #7173

feat(storage): encryption-at-rest ops — audit, admin kill switch, migration, key rotation (PR2)#7173
Frooodle merged 12 commits into
mainfrom
claude/pdf-encryption-at-rest-p2

ConnorYoh commented Jul 27, 2026 •

edited

Loading

Uh oh!

Uh oh!

ConnorYoh commented Jul 31, 2026

Uh oh!

ConnorYoh commented Aug 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

ConnorYoh commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of Changes

Uh oh!

Uh oh!

ConnorYoh commented Jul 31, 2026

Uh oh!

ConnorYoh commented Aug 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ConnorYoh commented Jul 27, 2026 •

edited

Loading