Release: develop -> main#4359
Merged
Merged
Conversation
…retained) (#4332) * feat(storage): cut over Azure Blob to provider-agnostic S3/MinIO storage Replace @azure/storage-blob with a provider-agnostic StorageService backed by S3StorageService (MinIO via @aws-sdk/client-s3), a MockStorageService for local dev, and a createStorageService() factory. blobUrl/blobName live on the base class so the URL shape stays reversible and host-independent: blobName() decodes both the legacy Azure-host and the new MinIO-host URLs to the same container-relative key, which keeps DB-persisted URLs valid across the migration. Add the GeBüV anchoring pipeline (Merkle + OpenTimestamps + WORM bucket provisioning) under storage/anchoring with the AddArchiveAnchoring migration, and record content hashes of KYC documents and EP2 settlement reports on upload as a best-effort side-booking (failures are logged, never roll back the upload). Swap the config azure.storage block for an s3 block, delete azure-storage.service.ts, and migrate all consumers (kyc-document, support-document, fiat-output-job) plus the BlobContent/Blob importers grown on develop to the new module. Restore KycDocumentService.toHostStableUrl on the storage-backed service via the shared StorageService.encodePath so served URLs stay byte-identical apart from the host. Also make the GwG compliance download (Handelsregisterauszug, Vollmacht) select KYC documents by their host-independent container-relative key instead of full-URL string equality, so pre-cutover documents whose stored Azure-host URL no longer equals the live MinIO-host URL are no longer silently dropped from the compliance ZIP. * test(compliance): prove GwG org documents survive the Azure to MinIO cutover Regression guard for the silent-data-loss blocker: when kyc_step.result holds a legacy Azure-host blob URL and the live KycFileBlob.url is regenerated from the MinIO backend, the fileDownloadConfig entries 11 (Handelsregisterauszug) and 12 (Vollmacht) must still select the document via the host-independent object key. Covers COMMERCIAL_REGISTER, LEGAL_ENTITY and AUTHORITY steps, percent-encoded file names, and the fail-closed behaviour when a URL is missing. * feat(storage): fail closed on WORM compliance-config gaps (GeBüV) Two fail-closed hardenings for the Azure->MinIO WORM cutover, so a misconfiguration surfaces as a loud error instead of silently under-protecting GeBüV-retention-relevant compliance records. - provision-bucket: reject RETENTION_YEARS below the GeBüV 10-year floor. COMPLIANCE-mode Object Lock retention is extend-only and irreversible, so a value of 1..9 would permanently under-retain WORM objects with no way to correct it. Keep the 11y default; a value >= 10 is required. - EP2 settlement sink: add StorageService.uploadWormBlob and route the per-merchant EP2 report write through it. The S3 implementation verifies the target bucket has Object Lock enabled (GetObjectLockConfiguration) before the first PUT and throws otherwise, since Object Lock cannot be retro-fitted onto an existing bucket. Verified once per container to keep the probe off the per-PUT hot path; the kyc path is unchanged. Add proving tests: the retention floor rejects 9 and accepts 10/11, and a WORM-sink write fails closed (no PUT) into a non-locked / unverifiable bucket while succeeding into a locked one. * style: apply prettier formatting to s3-storage spec * harden(anchoring): RFC 6962 Merkle tree, tamper-proof verification, persisted proofs, TOCTOU-safe recordHash - Merkle tree rebuilt per RFC 6962: 0x00 leaf / 0x01 node domain separation and the largest-power-of-two split, replacing the Bitcoin-style last-node duplication that made [a,b,c] and [a,b,c,c] hash to the same root (CVE-2012-2459). No production anchor exists yet (archive_batch is empty in every environment), so the format change is free now and would be irreversible after the first anchor. - verifyDocument derives the proof leaf from the supplied bytes, not the stored hash, so a tampered document no longer reports proofValid=true. Added a non-optional `verified` field (found && hashMatches && anchored && proofValid); `pending` stays separate. - Inclusion proofs are persisted (archive_file.merkleProof) and used verbatim; verifyDocument fails closed when a batch is set but the proof is missing, instead of live-reconstructing from sibling rows that may be pruned over the 11-year retention window. - recordHash uses a conditional IsNull() update and reloads fail-closed on race loss, closing the TOCTOU window against the nightly anchorPending run. - archive_file.bucket/name are `text`, not varchar(256): storage keys embed an unbounded, user-controlled file name; a long name made the recordHash insert throw and silently drop the document from anchoring. - merkle proof codec validates each sibling as a 32-byte hex digest before decoding. * harden(storage): WORM-verified KYC writes, anchor merge-copies, add reconcile safety net - KYC uploads go through uploadWormBlob (was uploadBlob), so a kyc bucket that does not enforce Object Lock is rejected fail-closed instead of silently accepting mutable compliance records. copyBlobs asserts Object Lock before copying, since an account-merge copy into the kyc bucket is a fresh WORM write. copyBlobs returns the destination keys so merge-copied documents get anchored the same way uploads are. - reconcileHashes cron backfills anchor gaps left by a failed best-effort recordHash. It lists keys only (no per-object HeadObject fan-out over 1.3M objects) and caps the backfill per run, logging loudly when capped, to avoid OOM and the cron timeout. Bucket enumeration is per-row fault-tolerant: one unparsable paymentLinksConfig no longer aborts the whole run, and the kyc bucket is always reconciled regardless of the EP2 lookup. - redundant per-method try/catch removed from the DfxCron jobs; the framework already logs. * harden(storage): fail-loud config, scoped provisioning creds, resilient compliance download - kycBlobKey throws on a blob URL without the container marker instead of silently returning the full URL, which would reinstate the exact host-equality bug the cutover filter fixes. To keep that fail-loud without taking down the whole GwG compliance export, the per-target filter in downloadUserData is wrapped: a single dirty step result is recorded in error_log.csv and skipped, the rest of the ZIP is still produced. - provision-bucket uses dedicated S3_ADMIN_* credentials (no fallback to the app keys, which cannot create buckets or set Object Lock), distinguishes 403 from 404 instead of guessing, and reads the Object Lock config back after applying it, failing closed on any mismatch. - .env.example documents the seven new S3_* / S3_ADMIN_* vars and drops the dead AZURE_STORAGE_CONNECTION_STRING; the still-used AZURE_* client vars stay. * fix(storage): harden anchoring pipeline and provisioning per review - provision-bucket: initialize Config via GetConfig() (was uninitialized -> crash) - s3-storage: assert Object Lock COMPLIANCE mode + retention floor, not just Enabled - anchoring: close anchorPending/recordHash TOCTOU by never overwriting sha256 on existing rows (per-file conditional update) - anchoring: recordHash fails closed (audit log + throw) on hash divergence instead of silent overwrite - fiat-output: collision-safe EP2 report filename (include entity id) - style: PascalCase enum values, typed OTS return, TypeORM object-syntax, merkleProof getter/setter * style: apply prettier formatting to storage/anchoring review fixes * refactor(storage): defer GeBueV anchoring pipeline to a separate PR The anchoring pipeline (Merkle/OTS/WORM-verify, Saeule 2) is decoupled from the storage cutover and moved to a dedicated hardened follow-up PR (branch feat/gebuev-anchoring). This PR now contains only the Azure Blob -> S3/MinIO storage cutover (Saeule 1). Removes the anchoring/ directory, its migration, and all ArchiveService/recordHash call sites; storage upload/copy paths unchanged. * harden(storage): standalone provisioning, TTL WORM-verify cache, negative WORM tests - provision-bucket: read S3 config directly from process.env (drops the src/config import chain so plain ts-node resolves, fail-loud on missing vars) - s3-storage: replace process-lifetime WORM-verify cache with a 5-min TTL so a later bucket-side retention weakening is re-detected (fail-closed on drift) - tests: WORM guard rejects GOVERNANCE mode, missing default retention, and sub-floor retention years * fix(storage): read S3 config via GetConfig() to avoid boot-order crash + cleanup - s3-storage: constructor reads GetConfig().s3 instead of the Config singleton, which is undefined until ConfigService runs; S3StorageService is eagerly constructed in KycDocumentService/SupportDocumentService (same boot-crash class guarded by config-bootstrap-order.spec) - add bootstrap-order regression test for the eager createStorageService path - remove dead s3Admin config block and orphaned Process.ARCHIVE_* enum values (anchoring moved to a separate PR) - provision-bucket: standalone WORM-provisioning header, DEFAULT_RETENTION_YEARS constant * fix(storage): validate S3 config lazily via Config singleton for boot-safety The previous GetConfig() approach broke the S3StorageService tests (they mock the Config singleton, not process.env) and was not repo-idiomatic. Instead follow the established boot-safe pattern (like FaucetRequestService and the base blobUrl): read Config.s3 lazily in a memoized client getter, not in the constructor. The constructor is now side-effect-free, so eagerly constructing S3StorageService in KycDocumentService/SupportDocumentService no longer risks a boot-order crash. Incomplete-config/trailing-slash validation now fails on the first storage call rather than at construction; tests and the factory JSDoc adapted accordingly. * test(storage): harden bootstrap-order env pin, fix stale factory-spec comment - config-bootstrap-order.spec: pin ENVIRONMENT to a non-LOC value so the factory constructs a real S3StorageService (avoids a vacuous pass if a local .env sets ENVIRONMENT=loc; CI already leaves it undefined) - storage.factory.spec: correct comment — Config.s3 is read lazily by the memoized client getter, not the side-effect-free constructor * feat(storage): reversible dual-write layer for the MinIO cutover Turn the storage cutover from a big-bang provider swap into a reversible, phased migration. Two independent switches drive the migration state so every step can be advanced and rolled back on its own: - STORAGE_WRITE_MODE (azure | dual | s3) and STORAGE_READ_SOURCE (azure | s3). - CompositeStorageService writes fail-closed to both backends in dual mode (read-source backend first) and reads only from the configured source, with no silent cross-store fallback. - AzureStorageService is restored as a StorageService implementation (the legacy backend during the migration); it gains listKeys, and copyBlobs now returns the target keys. - Config.storage is fail-loud: both env vars are required in non-LOC environments; invalid or unset values crash boot instead of silently picking a store. - The WORM retention guard gains an upper bound so a mistyped value can no longer lock the corpus for far longer than required. The storage consumers are untouched; the only wiring point is the factory. * fix(storage): fail loud when the Azure blob URL cannot be derived Config.azure.storage.url is parsed from the connection string's BlobEndpoint segment and can be undefined for a connection string without one. The Azure adapter now throws on first client use if the URL is missing, instead of silently producing a broken "undefined<container>/..." URL and persisting it into KYC/support/EP2 references. Also closes the review's test gaps: the composite dual-write copyBlobs S3-first and error paths, uploadWormBlob failing on the second backend, the missing-BlobEndpoint case, and the second forbidden storage combination. * test(storage): use a placeholder Azure hostname and add mock return types Genericize the DFX-looking Azure storage hostname in the dual-write test fixtures, and add explicit return types to the two Azure SDK mock helpers, per CONTRIBUTING. Test-only, no behaviour change. * test(storage): use a placeholder Azure hostname in the cutover regression tests Replace a production Azure storage account host with a generic placeholder in the host-independence fixtures. These tests assert host-independent object-key handling (blobName reversal, host-independent KYC blob matching), so the concrete host is irrelevant. Test-only, no behaviour change. * fix(storage): harden dual-write copy asymmetry and WORM object retention Two safety gaps found in review: - Dual-mode copyBlobs discarded the non-read-source result and succeeded silently when the two backends copied different object counts (e.g. a source prefix not yet backfilled to the S3 side), so an account merge could land in only one store. It now captures both results and logs the divergence loudly with the prefixes and counts. It still never throws: failing closed would break account merges during the migration window, and the mandatory reconciler gate heals the divergence before the read flip. - WORM writes relied solely on the bucket default retention, verified through a five-minute positive cache and never applied per object, so a bucket default weakened inside that window would let compliance records land unprotected. Every WORM PUT now carries per-object COMPLIANCE retention mirroring the bucket's validated default years (floor-clamped), and those validated years are cached with the probe so a cache hit can never write without a validated retention period. * fix(storage): probe Object Lock per WORM write and add a retention safety margin An explicit per-object retention overrides the bucket default, so it must never be shorter than what that default would produce. Two gaps remained: the validated retention years were served from a five-minute cache, so a raised bucket default was not mirrored inside that window, and the retain-until date was computed from client time, which latency or clock skew could push marginally earlier than a server-side computation. WORM paths now probe the live bucket configuration on every call. The guard is only used by the low-volume compliance paths, so one probe per write is the right trade-off, and a bucket-side weakening is no longer masked at all rather than only after a TTL. The computed retain-until date carries a one-day margin; over-retaining by a day is harmless because COMPLIANCE retention is extend-only. * docs(storage): record the accepted residual on per-object WORM retention Probing the bucket configuration and then writing leaves a sub-millisecond window in which a default RAISED between probe and PUT is not mirrored. That residual is deliberate: dropping the explicit per-object retention would reopen the dangerous case of a weakened default silently under-protecting compliance records, and pinning every record to the provisioning ceiling would irreversibly over-retain all of them. The worst case still satisfies the retention floor and stays extendable. Recorded in the code so the trade-off is not silently reversed later.
github-actions
Bot
requested review from
TaprootFreak and
davidleomay
as code owners
July 24, 2026 11:49
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automatic Release PR
This PR was automatically created after changes were pushed to develop.
Commits: 1 new commit(s)
Checklist