fix(assets): wire configured AES key and harden vault crypto integrity #226
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/pms-188-assets-zero-key-crypto-integrity"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Asset secrets were encrypted under an all-zeros key because the router built AssetsService via the zero-key new() constructor. Wire the configured encryption_key through with_encryption_key and remove the zero-key new() so production paths cannot construct a zero-key service.
delete_asset now writes a 'deleted' asset_audit_log row inside its transaction so vault deletions are traceable; migration 042 drops the asset_audit_log.asset_id ON DELETE CASCADE so that audit row survives the asset it records. Migration 043 guards against orphaning secrets written under the old zero key: it RAISES fail-loud if any credential_vault / configuration_item rows exist at migrate time (none do on a wiped pre-go-live database, matching the migration 040 ruling), since AES-GCM ciphertext cannot be re-encrypted from SQL.
parse_encryption_key now hex-decodes a 64-char hex key, rejects an ambiguous all-hex 32-char string instead of silently taking it as raw bytes, and reports the supplied length in the wrong-length error.
bootstrap honors MOKOSH_AUTH_DATA_KEY_VERSION_PREV so a non-sequential AEAD key rotation can tag the previous key explicitly instead of always assuming current-1.
Recovery codes now keep all 16 base32 chars (split 8-8) to deliver the documented 80 bits of entropy; the previous code kept only 10 chars for 50 bits.
#PMS-188
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
Review of the PMS-188 work found delete_asset broken: it inserts an asset_audit_log row with action='deleted', but the action CHECK (migration 011) only allowed created/updated/synced/status_changed, and migration 042 dropped the cascade FK without widening the CHECK. Every DELETE /api/v1/assets/{id} therefore hit a check_violation on the audit insert and 500'd. No existing test covered asset deletion, so it went unnoticed. Widen the CHECK in migration 042 to include 'deleted'. Add delete_asset_writes_surviving_deleted_audit_row: deletes an asset (now 2xx), asserts the asset row is gone, asserts a 'deleted' audit row was written AND survives the delete (FK dropped), and asserts a second delete is 404 with no extra audit row (the failed tx rolls back). #PMS-188 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>