security(bridge): ENG-279 audit findings — 3 issues, branch blocked until resolved#314
Draft
patoo0x wants to merge 5 commits intofeature/bridge-integrationfrom
Draft
security(bridge): ENG-279 audit findings — 3 issues, branch blocked until resolved#314patoo0x wants to merge 5 commits intofeature/bridge-integrationfrom
patoo0x wants to merge 5 commits intofeature/bridge-integrationfrom
Conversation
…ore merge Critical: amount not validated before Bridge API call High: fake placeholder email in customer creation (ENG-278) High: no error if external account not found in withdrawal Webhook sig verification, GraphQL auth, and feature flag gating all pass. Branch blocked until 3 items resolved.
… guard
fix(initiateKyc): use real email from Kratos identity (IdentityRepository)
instead of the `${account.id}@flash.app` placeholder.
Falls back to placeholder with a warning log if Kratos lookup fails or
identity has no email — maintains KYC flow without hard failure.
fix(verifyBridgeSignature): remove silent JSON.stringify fallback for rawBody.
If req.rawBody is not set (body-parser misconfiguration), the middleware now
returns HTTP 500 with a descriptive error instead of silently bypassing
signature verification. The standalone webhook server already sets rawBody
via express.json({ verify }) — this guard makes misconfiguration loud.
Refs: ENG-279
- Add BridgeDepositAddress collection + repo for active crypto receive addresses - Remove chain-specific bridgeTronAddress from Account schema/type - Resolve ETH USDT receive address through IBEX and persist in BridgeDepositAddress - Update Bridge virtual account creation and withdrawal flows to read from repo - Add generic bridge deposit primitive to domain layer This replaces the hardcoded Tron field with a pre-launch extensible model.
- Add unit coverage for BridgeDepositAddress repository - Verify active address reuse, deactivation + insert, and deactivate helper - Verify Bridge service reuses existing deposit address, creates/stores a new ETH USDT address when missing, and uses stored address for withdrawals
- Update IBEX client createCryptoReceiveInfo wrapper to use documented + + request shape. - Switch ETH USDT receive creation to the documented receive-info API. - Update IBEX crypto-receive webhook to resolve deposits via the new BridgeDepositAddress collection and accept ethereum/erc20 payloads. - Refresh bridge integration docs to remove Tron-specific language.
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
2 participants
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.
Bridge Integration Security Audit (ENG-279)
Full findings in
docs/bridge-integration/SECURITY-AUDIT.md.🔴 Critical
Amount not validated before Bridge API call
initiateWithdrawalpasses rawamount: stringdirectly to Bridge with no validation. AllowsNaN,-100, exponential notation. Fix: parse + validate >= 1.0 before API call.🟡 High (x2)
Fake email in customer creation (ENG-278)
${account.id}@flash.appplaceholder email sent to Bridge KYC. KYC emails undeliverable, ToS violation. Fix: use real email from Kratos identity.External account ownership not enforced in withdrawal
If
externalAccountIdnot found in user's accounts, transfer is attempted anyway. Should early-return error iftargetAccount === undefined.✅ Passes
BridgeConfig.enabledfeature flag — correctBottom line
Branch not ready to merge. 3 targeted fixes needed. Items 1 and 3 are < 1 hour. Item 2 (ENG-278) needs Kratos identity lookup — pattern already exists in
business-account-upgrade-request.ts.