mirror of
https://github.com/hanzo-fi/payments.git
synced 2026-10-30 20:31:20 +00:00
903db79ab0
* fix(api): EN-1092 obfuscate IBAN on v2 bank-account forward response
The v2 POST /bank-accounts/{id}/forward handler was the only
bank-account-returning endpoint that built its response from the
decrypted bank account without calling Obfuscate(), echoing the full
IBAN and account number back to the API caller in clear. Every other
handler (v2 create/get/list and all v3) masks them first.
Call bankAccount.Obfuscate() before building the response, mirroring
handler_bank_accounts_get.go. This only mutates the in-memory copy used
for the response; it does not affect what is forwarded to the connector
or what is stored.
Constraint: Response shape must stay byte-compatible with the v2 API
Confidence: high
Scope-risk: narrow
* fix(api): EN-1092 stop recording connector config secrets in OTEL spans
The v2/v3 connector install and config-update handlers recorded the raw
request body as the span attribute "config". These bodies carry
credentials (apiKey for Adyen/Column/Increase/CurrencyCloud, secret for
Atlar). The DB encrypts the configs at rest, but the spans leaked them
in clear to the tracing backend.
Drop the "config" attribute in all four handlers. The provider /
connectorID attribute already set on each span is retained for
debuggability.
Rejected: Mask sensitive fields then trace | body is opaque bytes at the
handler layer and each provider names secrets differently, so masking
here is fragile and a new provider would silently leak
Confidence: high
Scope-risk: narrow
Directive: Do not re-add the raw request body to these spans
* fix(storage): EN-1092 encrypt open-banking tokens at rest
open_banking_access_tokens.access_token and
open_banking_connection_attempts.temporary_token were stored as plaintext
text columns, while every other secret in the schema is encrypted with
pgcrypto. These tokens grant access to customers' bank accounts.
Convert both columns to bytea and encrypt/decrypt them with
pgp_sym_encrypt / pgp_sym_decrypt and the existing configEncryptionKey,
mirroring connectors.config and bank_accounts. Migration 30 alters the
column types and backfills any existing plaintext in place (NULLs left
untouched). Writes encrypt via bun .Value(); reads decrypt via
ColumnExpr aliased back to the scanned field.
Added an integration test asserting the raw columns are ciphertext (not
plaintext) and decrypt back to the original value.
Constraint: Migration must encrypt existing rows in place using the
storage encryption key, so it is a Go migration receiving the key
Rejected: Static .sql migration | cannot inject the encryption key safely
Confidence: high
Scope-risk: moderate
Directive: temporary_token/access_token are bytea — always read via
pgp_sym_decrypt and write via pgp_sym_encrypt; never select the raw column
Not-tested: backfill over a large pre-existing plaintext dataset
* test(api): EN-1092 assert v2 forward response masks IBAN/account number
Covers the H8 fix: the forward handler now returns obfuscated IBAN and
account number. Expected masked values are derived from Obfuscate() so
the assertion cannot drift from the masking implementation; the backend
mock is handed independent copies since Obfuscate mutates in place.
Confidence: high
Scope-risk: narrow
* fix(models): EN-1092 mask short bank-account values instead of failing
Obfuscate() returned an error for IBANs shorter than 8 chars or account
numbers shorter than 5 chars. Since it runs on display-only fields and is
called after the data is already persisted/forwarded, that error surfaced
as a 500 — most notably in the v2 forward-to-connector endpoint, where it
fired only after BankAccountsForwardToConnector had already succeeded,
producing a misleading 500 for a completed side effect. Account numbers
have no minimum length in the create validation, so this was reachable
with stored data.
Fully mask values too short to keep both ends instead of erroring (the
most secure degradation). Output is unchanged for all previously-valid
inputs; only the former error paths now return a fully masked value. The
error return is kept for interface stability and is currently always nil.
Addresses PR #748 review (post-side-effect failure on short account details).
Constraint: must not leak raw values when masking degrades
Rejected: enforce a minimum length in create validation | would reject
account numbers banks legitimately issue, and not fix existing rows
Confidence: high
Scope-risk: narrow
* fix(storage): EN-1092 select attempt columns explicitly in UpdateStatus
OpenBankingConnectionAttemptsUpdateStatus re-selected the attempt via
.Model(&attempt) without an explicit column list. This worked because
bun derives the column list from the struct fields and excludes the
scanonly decrypted_temporary_token field (verified: UpdateStatus tests
pass), but it relied on that implicit behaviour and read no encrypted
column only by luck of the tag change.
Make the SELECT explicit like the get/list paths so it never references
the raw encrypted temporary_token column. The event payload does not need
the token, so it is intentionally omitted.
Addresses PR #748 review (keep temporary_token handling robust in
UpdateStatus).
Confidence: high
Scope-risk: narrow
---------
Co-authored-by: fabrice guery <[email protected]>
38 lines
1.0 KiB
Go
38 lines
1.0 KiB
Go
package migrations
|
|
|
|
import (
|
|
"context"
|
|
|
|
"github.com/uptrace/bun"
|
|
)
|
|
|
|
// encryptOpenBankingTokens converts the open-banking token columns to bytea and
|
|
// encrypts any existing plaintext values in place, mirroring the pgcrypto
|
|
// mechanism used for connector configs and bank account details.
|
|
func encryptOpenBankingTokens(ctx context.Context, db bun.IDB, encryptionKey string) error {
|
|
// access_token is NOT NULL, so every row holds a value to encrypt.
|
|
_, err := db.NewRaw(`
|
|
ALTER TABLE open_banking_access_tokens
|
|
ALTER COLUMN access_token TYPE bytea
|
|
USING pgp_sym_encrypt(access_token, ?, ?);
|
|
`, encryptionKey, encryptionOptions).Exec(ctx)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
|
|
// temporary_token is nullable, so leave NULLs untouched.
|
|
_, err = db.NewRaw(`
|
|
ALTER TABLE open_banking_connection_attempts
|
|
ALTER COLUMN temporary_token TYPE bytea
|
|
USING CASE
|
|
WHEN temporary_token IS NULL THEN NULL
|
|
ELSE pgp_sym_encrypt(temporary_token, ?, ?)
|
|
END;
|
|
`, encryptionKey, encryptionOptions).Exec(ctx)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
|
|
return nil
|
|
}
|