fix(server): final-review polish — encryption-gate wording, all-field sentinel guard, README/e2e notes

server/src/app-config/README.md

@@ -68,9 +68,15 @@ PORT, HOST, TRUST_PROXY, SESSION_DURATION(_REMEMBER), MCP_SESSION_TTL,
 MCP_MAX_SESSION_PER_USER, MCP_SSE_KEEPALIVE, TREK_PLUGIN_RPC_*/LOG_*/MAX_RSS_MB,
 TREK_PLUGIN_REGISTRY_URL, TREK_WIKI_DIR*, TREK_PLACE_PHOTO_DIR, BACKUP_*,
 TRANSIT_API_URL, LOG_LEVEL*, ALLOW_INTERNAL_NETWORK*, DEFAULT_LANGUAGE,
-TREK_DB_FILE, TREK_DB_JOURNAL_MODE, TREK_DB_SYNCHRONOUS, ENCRYPTION_KEY.
+TREK_DB_FILE, TREK_DB_JOURNAL_MODE, TREK_DB_SYNCHRONOUS, ENCRYPTION_KEY**.
 (* frozen today because the consuming module captures it at import; tests that
 override these set them at file top, before the SUT import.)
+(** ENCRYPTION_KEY is boot-stable only for the cipher key material itself —
+`src/config.ts` resolves and freezes it at process start, per the exemption
+below. `security.encryptionKeySet` (the storage admin-config encryption gate)
+is a *different* read of the same variable: `deriveSecurity()` reads it live
+via `readEnv()`/`RuntimeEnvService`, so a value set after boot is observed on
+the next call — it does not retroactively unlock the frozen cipher.)
 
 **Tombstoned** (boot-aborting since the admin-config slice — configure storage
 in the admin UI / `data/storage-config.json` instead): `TREK_S3_ENDPOINT`,

server/src/nest/storage/storage-admin.service.ts

@@ -11,6 +11,7 @@ import {
 import { StorageService } from './storage.service';
 import { SEED_CONFIG_PATH } from './storage-paths';
 import {
+  assertNoMaskSentinels,
   decryptBackendSecrets,
   encryptionGateError,
   encryptStorageSecrets,
@@ -62,6 +63,10 @@ export class StorageAdminService {
   /** Full-document replace of the two settings rows. Throws StorageBackendError on any refusal. */
   applyConfig(config: StorageConfig): void {
     const unmasked = unmaskStorageConfig(config, this.storedBackendsRow());
+    // unmask only resolves the secret fields it knows about; a mask sentinel
+    // submitted in a non-secret field would otherwise pass through untouched
+    // and get persisted verbatim as garbage-in.
+    assertNoMaskSentinels(unmasked);
     const plaintext = listPlaintextSecrets(unmasked);
     if (plaintext.length > 0 && !this.env.env().security.encryptionKeySet) {
       throw new StorageBackendError(encryptionGateError(plaintext[0]!));

server/src/nest/storage/storage-secrets.ts

@@ -49,10 +49,19 @@ export function listPlaintextSecrets(config: StorageConfig): PlaintextSecret[] {
   return found;
 }
 
-/** The seed file has no mask round-trip — a literal mask sentinel is always a mistake. */
+/**
+ * No literal mask sentinel may ever become a stored value, in ANY option
+ * field — not just the fields the type registry marks `secret`. Secret
+ * fields are already covered structurally (`unmaskStorageConfig` throws
+ * when a mask has no stored counterpart); this closes the same hole for a
+ * mask sentinel submitted in a non-secret field, which would otherwise be
+ * persisted verbatim as garbage-in. Also the seed-file guard: the seed file
+ * has no mask round-trip, so a literal mask sentinel there is always a
+ * mistake.
+ */
 export function assertNoMaskSentinels(config: StorageConfig): void {
   for (const backend of config.backends) {
-    for (const field of storageSecretFields(backend.type)) {
+    for (const field of Object.keys(backend.options as Record<string, unknown>)) {
       if ((backend.options as Record<string, unknown>)[field] === MASKED_SETTING_VALUE) {
         throw new StorageBackendError(
           `backend '${backend.name}' field '${field}' is the mask sentinel — a mask can never become a stored value`,
@@ -62,11 +71,11 @@ export function assertNoMaskSentinels(config: StorageConfig): void {
   }
 }
 
-/** Fail-closed refusal text: the silent JWT-derived fallback key is not accepted for credentials. */
+/** Fail-closed refusal text: the implicit key persisted in the data directory is not accepted for credentials — it rides inside backups. */
 export function encryptionGateError(secret: PlaintextSecret): string {
   return (
     `backend '${secret.backend}' has a plaintext secret '${secret.field}' but ENCRYPTION_KEY is not set — ` +
-    'set ENCRYPTION_KEY explicitly to save credentialed storage backends (the JWT-derived fallback key is not accepted)'
+    'set ENCRYPTION_KEY explicitly to save credentialed storage backends (the implicit key persisted in the data directory is not accepted: it rides inside backups)'
   );
 }
 

server/tests/e2e/storage-admin.e2e.test.ts

@@ -87,6 +87,12 @@ describe('Storage admin e2e (real auth + admin guard + managed guard + temp SQLi
     server = app.getHttpServer();
   });
 
+  // Clears the persisted rows only — NOT the registry's in-memory last-good
+  // snapshot (StorageRegistryService keeps state across requests within one
+  // `app`, and this suite builds `app` once in beforeAll). A test added after
+  // STORE2E-004 that expects to see fresh defaults must call GET (which
+  // renders the registry's live snapshot) rather than assume the deleted rows
+  // reset it — the registry only re-reads app_settings on `reload()`/init.
   beforeEach(() => {
     db.exec("DELETE FROM app_settings WHERE key LIKE 'storage.%'");
     db.exec('DELETE FROM audit_log');
@@ -99,6 +105,10 @@ describe('Storage admin e2e (real auth + admin guard + managed guard + temp SQLi
 
   it('STORE2E-001 401 without a session', async () => {
     expect((await request(server).get('/api/admin/storage')).status).toBe(401);
+    // PUT stays behind the same guard chain — pins that the guards are declared
+    // class-level (JwtAuthGuard + AdminGuard on the controller), not re-added
+    // per handler where a future handler could forget them.
+    expect((await request(server).put('/api/admin/storage').send({ backends: [], categories: {} })).status).toBe(401);
   });
 
   it('STORE2E-002 403 for a non-admin', async () => {

server/tests/unit/nest/storage/storage-admin.service.test.ts

@@ -206,6 +206,18 @@ describe('StorageAdminService.applyConfig', () => {
     expect(registry.resolve('backups').backendName).toBe('backups-local'); // live state untouched
   });
 
+  it('STORADM-018 a mask sentinel in a NON-secret field throws and persists nothing', () => {
+    const { service, uploadsRoot } = makeService();
+    const before = readRow(BACKENDS_KEY);
+    expect(() =>
+      service.applyConfig(configWith(uploadsRoot, {
+        backends: [{ name: 'off-box', type: 's3', options: { ...S3_OPTIONS, accessKeyId: MASKED_SETTING_VALUE } }],
+        categories: {},
+      })),
+    ).toThrow("backend 'off-box' field 'accessKeyId' is the mask sentinel — a mask can never become a stored value");
+    expect(readRow(BACKENDS_KEY)).toBe(before);
+  });
+
   it('STORADM-017 persists both rows in ONE transaction and reloads once', () => {
     const { service, registry, uploadsRoot } = makeService();
     const txSpy = vi.spyOn(db, 'transaction');

server/tests/unit/nest/storage/storage-secrets.test.ts

@@ -54,7 +54,7 @@ describe('encryptionGateError', () => {
     expect(message).toContain('ENCRYPTION_KEY');
     expect(message).toContain("'off-box'");
     expect(message).toContain("'secretAccessKey'");
-    expect(message).toContain('fallback');
+    expect(message).toContain('not accepted');
   });
 });
 
@@ -104,10 +104,20 @@ describe('unmaskStorageConfig', () => {
 });
 
 describe('assertNoMaskSentinels', () => {
-  it('throws when a seed-style config carries the mask sentinel', () => {
+  it('throws when a seed-style config carries the mask sentinel in a secret field', () => {
     expect(() => assertNoMaskSentinels(s3Config(MASKED_SETTING_VALUE))).toThrow('mask');
     expect(() => assertNoMaskSentinels(s3Config('sk'))).not.toThrow();
   });
+
+  it('throws for a mask sentinel in a NON-secret field too — the check is all-field, not secret-only', () => {
+    const config: StorageConfig = {
+      backends: [{ name: 'off-box', type: 's3', options: { ...S3_OPTIONS, accessKeyId: MASKED_SETTING_VALUE } }],
+      categories: {},
+    };
+    expect(() => assertNoMaskSentinels(config)).toThrow(
+      "backend 'off-box' field 'accessKeyId' is the mask sentinel — a mask can never become a stored value",
+    );
+  });
 });
 
 describe('decryptBackendSecrets', () => {