Add explicit Bank Frick personal IBAN routing#4384
Open
TaprootFreak wants to merge 7 commits into
Open
Conversation
This was referenced Jul 24, 2026
❌ TypeScript: 5 errors |
Collaborator
Author
|
Completed 4 full independent review passes. All findings were addressed; both final full-diff reviews report 0 findings and the latest CI is fully green. |
TaprootFreak
marked this pull request as ready for review
July 24, 2026 22:28
Review of the initial implementation found several ways the issuance path could create a second real account at the bank, strand a customer, or leak personal data. The bank's vIBAN API offers no idempotency key, so "did the bank already create one?" can only be answered from a listing - and an empty listing is not proof while another process is still working on the same intent. The request path is therefore strictly fail-closed: it never resets an intent and never retries after an empty recovery search. Reopening a stuck intent is now the sole responsibility of the reconciliation job, which additionally requires the intent to be older than the worst-case bank round trip before it treats an empty listing as proof. The one exception is a provider-proven "nothing was sent", which is a stronger guarantee than an empty search; that proof now rests on a signature-verified response, since the verifier previously ran for successful responses only. Deactivating a personal IBAN reopens its intent atomically, and account merges resolve deactivation, ownership and both sides' intents inside one transaction - a customer request could otherwise slip into the gap and issue a second real account without any bank-side ambiguity at all. The issuance event log no longer stores the customer IBAN in clear text; it references the record instead, and the migration refuses to drop the old columns while any row cannot be resolved. Customer-facing errors are short tokens, with the diagnostic detail kept in logs and the intent record, free of account numbers and raw provider text. The reconciliation job isolates failures per bank, alerts on chronically incomplete listings, and no longer reports a clean run when it could not evaluate what it found. Note: bank and personal IBAN are pinned on every new transaction request, so invoices and history reproduce the exact payment details of the quote. A later document request fails closed if that account was deactivated in the meantime, rather than silently substituting a different one.
The migration read its count via a numeric array index. The repository's guard against MSSQL bracket quoting matches any word characters inside brackets, so a plain JavaScript index tripped it. Destructuring keeps the logic identical without the bracket pattern; the guard itself stays as is, since it protects against syntax that would break on Postgres.
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.
Summary
personalIbanProvider: "Frick"buy payment-info request optionDatabase
transaction_request.bankIdandtransaction_request.virtualIbanIdwithout foreign keysvirtual_iban_issuance_intentfor cross-instance, crash-recoverable Frick vIBAN issuancevirtual_iban_issuance_eventas immutable audit history, referencing the virtual IBAN record rather than storing its IBANWhy the request path never retries
The bank's vIBAN API has no idempotency key (unlike its payment API). Whether a create call already
produced an account can therefore only be answered from a listing — and an empty listing is not
proof while another process is still working on the same intent. Building a retry on that
assumption produced a duplicate-account race in every variant we tried.
The request path is now strictly fail-closed: no reset, no retry after an empty recovery search.
Reopening a stuck intent is the sole responsibility of the hourly reconciliation job, which
additionally requires the intent to be older than the worst-case bank round trip (30 min, derived
from the HTTP timeout across create + activate + a possible re-authorisation) before it treats an
empty listing as proof.
The one retained exception is a provider-proven "nothing was sent" — a stronger guarantee than an
empty search. That proof now rests on a signature-verified response: the verifier previously ran
for successful responses only, so an unsigned error from anywhere on the path could have been read
as proof.
Residual risk, accepted deliberately: with no idempotency key, a sufficiently unlucky
interleaving can still leave a second real account at the bank.
docs/bank-frick-operations.mddocuments which interleaving, which alert surfaces it, and what to do about it.
Behaviour changes outside the selector path
Both are intentional and worth reviewing explicitly:
requests — that is what makes invoices and history reproduce the exact payment details of the
quote. If that account is deactivated before a later document request, the request now fails
closed instead of silently substituting a currently active one. A silent substitution is the
worse outcome: the customer may already have noted the original IBAN.
deliberately disjoint. No buy-scoped record exists in production (0 of 3053), and both feature
flags that would create one are off.
Fee determination for customers without the selector is unchanged from the merge base — the bank
override is passed only when the selector was explicitly set.
Validation
npm run format,npm run lint,npm run type-check,npm run buildnpm run test:frick:cov: 10 suites, 496 tests, 100% statements, branches, functions and lines — now including the issuance and provider logic, which was previously excludedAn authenticated sandbox round-trip remains a deployment verification step because it creates
external Bank Frick state. Endpoint DNS/TLS/routing was checked separately from the target hosts.
Rollout
The environment variable was renamed to
FRICK_VBAN_API_URL, which only this version reads — thepre-rename application treats the old name as "Frick is the generic EUR provider" and would issue
accounts to every KYC50+ customer unasked. DFXServer/server#965 sets the new name and pins the old
one empty, so the ordering is enforced technically rather than by convention.
Related: DFXswiss/services#1190 (consumer), DFXswiss/packages#191 (SDK types).