fix(kyc): enforce gating across all capital-movement routes#245
Merged
mikewheeleer merged 2 commits intoMay 28, 2026
Conversation
- Closes Liquifact#222: requireKycForFunding was only wired to POST /api/invest/fund-invoice. Any other funding/settlement route could transfer capital without a KYC check. Changes: src/middleware/kycGating.js - FIX: smeId is now resolved ONLY from req.user.smeId (the JWT principal). Previously the fallback chain req.user.smeId || req.body?.smeId || req.params?.smeId allowed an authenticated caller to supply a verified SME's ID they do not own and bypass the identity check. Body/param values are now intentionally ignored. - req.kyc now includes smeId so downstream handlers can reference it without re-reading req.user. - Expanded JSDoc to document every gated endpoint and the security contract. src/routes/invoiceStateRoutes.js - POST /:id/link-escrow now requires requireKycForFunding (initiates escrow funding lifecycle — capital-movement entry point). - POST /:id/transition uses a new conditionalKycGate that applies requireKycForFunding only when targetState ∈ {funded, settled}; non-capital transitions (approved, rejected) are unaffected. - CAPITAL_MOVING_STATES set defined at module level for clarity. tests/kyc.gating.test.js - Full rewrite covering all gated routes and the anti-spoofing guarantee. - Sections: ConfigSchema regression, checkKycHealth, middleware unit tests (auth, smeId resolution, KYC status gate, req.kyc attachment), fund-invoice route, link-escrow route, transition route (conditional gating), and smeId spoofing via body/params. docs/compliance.md - Updated gated-endpoint inventory table. - Documented the anti-spoofing fix with before/after code. - Phase 1 roadmap updated to mark issue Liquifact#222 complete. - Version bumped to 1.1.0.
|
@Mrwicks00 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Contributor
Author
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.
fix(kyc): enforce gating across all capital-movement routes
Closes #222
Summary
requireKycForFundingwas documented to protect three endpoints but was only wired toPOST /api/invest/fund-invoice. This PR audits every money-moving route and applies the gate consistently, and also fixes a smeId spoofing vulnerability in the middleware itself.Problem
Two capital-movement routes were unprotected:
POST /api/invest/fund-invoicePOST /api/invoices/:id/link-escrowPOST /api/invoices/:id/transition→funded/settledAdditionally, the middleware's
smeIdresolution order was:An attacker with a valid JWT (even for an unverified SME) could supply a different, verified SME's ID in the request body and pass the KYC gate for an entity they do not own.
Changes
src/middleware/kycGating.jssmeIdis now resolved exclusively fromreq.user.smeId— the JWT claim set byauthenticateToken.req.body.smeIdandreq.params.smeIdare intentionally ignored during the KYC identity check.req.kycnow includessmeIdso downstream handlers can reference it without re-readingreq.user.src/routes/invoiceStateRoutes.jsPOST /:id/link-escrow— addedrequireKycForFundingdirectly. This endpoint initiates the escrow funding lifecycle and is a clear capital-movement entry point.POST /:id/transition— addedconditionalKycGate: KYC is enforced only whentargetState ∈ { funded, settled }. Non-capital transitions (approved,rejected) are unaffected and flow through without a KYC check.CAPITAL_MOVING_STATESset defined at module level for maintainability.tests/kyc.gating.test.jsFull rewrite covering all acceptance criteria from issue #222:
checkKycHealthreq.user/req.user.subsmeId; resolution from JWT onlyreq.kycsmeIdattached for downstreamPOST /api/invest/fund-invoicePOST /api/invoices/:id/link-escrowPOST /api/invoices/:id/transitiondocs/compliance.mdconditionalKycGatedescription for the transition endpoint.Security Notes
.env/ deployment secrets only.smeIdfix eliminates horizontal privilege escalation: an attacker who owns SME A cannot claim the KYC status of SME B by embedding SME B's ID in a request.extractTenantmiddleware upstream of the gate.Test Output
Checklist
security/kyc-gate-all-fundingfix(kyc): enforce gating across all capital-movement routessrc/middleware/kycGating.js— smeId spoofing fixsrc/routes/invoiceStateRoutes.js— gate onlink-escrowand capital transitionstests/kyc.gating.test.js— full rewrite with spoofing coveragedocs/compliance.md— updated gating table, anti-spoofing note, roadmapfund-invoice)