Conversation
…er call
createSubaccount only checked accountType, so a signed-in session could
reserve any unclaimed CPF: the claim, the provider subaccount and the
unique tax-hash row were all created before anything looked at taxId. A
non-string taxId also threw a TypeError (500) and "abc" normalized to ""
and bound sha256("").
Require a non-empty name (max 255, the company_name column width) and a
checksum-valid CPF for INDIVIDUAL or CNPJ for COMPANY, in every
environment, and reject with 400 ahead of the controller.
…subaccount newKyc and the KYB API submission forwarded the request body to the provider without comparing its tax id to the one reserved at createSubaccount. The provider approves whoever the documents belong to, so an account could end up Approved with a stored taxReference that differs from the KYC'd identity. Reject with 400 before any provider call when the normalized taxIdNumber (individual) or taxIdentificationNumberTin (company) does not hash to the record's taxReferenceHash. Formatted equivalents pass.
The tax-keyed BRL lookups answer 403 for a tax id held by another profile and createSubaccount answers 409, which lets any signed-in session test whether a CPF is registered. After five such distinct hits in 24 hours a principal gets 429 for further tax ids. Own and unregistered tax ids never count, so a partner onboarding many customers from one profile and status polling are unaffected. State is in memory per API instance. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…he 429 limit Replace the checksum-invalid CPF in the managed-profile example with a valid synthetic one, describe the 400 cases (name/taxId validation, taxIdNumber and taxIdentificationNumberTin mismatch) and add the shared 429 response for the six tax-keyed BR operations to the OpenAPI source, the regenerated types and the fiat-corridors guide.
createSubaccount reserves a CPF/CNPJ for the first authenticated caller, and the real owner then gets a 409 with no self-service recovery. The runbook gives read-first, verify-then-change steps to release an unapproved claim: the provider_customers and kyc_cases rows, the tax-hash-keyed financial_operations claim that would otherwise block the owner's retry, and the orphaned provider subaccount.
createSubaccount is a reserving flow, and invariant 5 and its checklist line claimed CPF validation at ramp registration, which the code does not do. Restate invariant 5 around validation before the claim, extend invariants 18 and 36, add invariants 48-49 (claim bound to the verified identity, distinct-tax-id limiter), squatting/enumeration/identity-swap threat rows and checklist lines, register the accepted residual risk as RISK-026 (full fix: exclusivity only on approval), and record the limiter in the api-surface spec.
6 of 9 tasks
✅ Deploy Preview for vortexfi canceled.
|
✅ Deploy Preview for vrtx-dashboard canceled.
|
✅ Deploy Preview for vortex-sandbox ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Concurrent requests can bypass the probe cap, unrelated conflicts can consume it, and accepted names are not normalized before provider submission.
Review effort: Balanced
Findings: 1
Open (3)
What changed in this PR
Hardens BRL onboarding against invalid tax-ID claims, identity swaps, and enumeration while documenting residual squatting risk and operator recovery.
Changes:
- Validates subaccount names and CPF/CNPJ checksums.
- Binds KYC/KYB submissions to the claimed tax ID.
- Adds a distinct-tax-ID probe limiter, tests, API documentation, and release runbook.
| File | Description |
|---|---|
docs/security-spec/RISK-REGISTER.md |
Records residual BRL tax-ID risk. |
docs/security-spec/07-operations/api-surface.md |
Specifies limiter behavior and coverage. |
docs/security-spec/05-integrations/brla.md |
Defines validation and binding invariants. |
docs/README.md |
Indexes the operator runbook. |
docs/operations-brl-tax-id-claim-release.md |
Documents claim-release operations. |
docs/api/pages/14-managed-profiles.md |
Uses a checksum-valid CPF example. |
docs/api/pages/09-fiat-corridors.md |
Documents validation and rate limiting. |
docs/api/openapi/vortex.openapi.json |
Updates schemas and responses. |
docs/api/openapi/vortex.openapi.d.ts |
Regenerates OpenAPI declarations. |
apps/api/src/api/routes/v1/brla.route.ts |
Mounts the limiter on tax-keyed routes. |
apps/api/src/api/middlewares/validators.ts |
Adds subaccount input validation. |
apps/api/src/api/middlewares/validators.test.ts |
Tests validation boundaries. |
apps/api/src/api/middlewares/distinctTaxIdLimiter.ts |
Implements per-principal probe limiting. |
apps/api/src/api/middlewares/distinctTaxIdLimiter.test.ts |
Tests limiter semantics and mounting. |
apps/api/src/api/controllers/brla.controller.ts |
Enforces KYC/KYB tax-ID binding. |
apps/api/src/api/controllers/brla.controller.test.ts |
Adds controller regression coverage. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+53
to
+55
| const hits = foreignHits.get(principal); | ||
| if (hits) dropExpired(hits, Date.now()); | ||
| if (hits && !hits.has(taxIdHash) && hits.size >= MAX_FOREIGN_TAX_IDS) { |
Comment on lines
+16
to
+20
| function answeredForeignTaxId(req: Request, res: Response): boolean { | ||
| return ( | ||
| res.statusCode === httpStatus.FORBIDDEN || | ||
| (res.statusCode === httpStatus.CONFLICT && req.method === "POST" && req.path === "/createSubaccount") | ||
| ); |
Comment on lines
+374
to
+379
| if (typeof name !== "string" || name.trim().length === 0 || name.trim().length > SUBACCOUNT_NAME_MAX_LENGTH) { | ||
| res.status(httpStatus.BAD_REQUEST).json({ | ||
| error: `name must be a non-empty string of at most ${SUBACCOUNT_NAME_MAX_LENGTH} characters.` | ||
| }); | ||
| return; | ||
| } |
The in-memory limiter only partially bounded tax-id enumeration (per instance, reset on deploy, bypassable with fresh sign-ups and URL variants) while blocking a capped partner's own customers. Validation and KYC binding close the squatting issues; the enumeration residual stays under RISK-026.
ebma
changed the title
The validator bounds the trimmed name to 255 characters, but the controller passed the raw value to Avenia, so a 255-character name with surrounding spaces exceeded the documented limit at the provider.
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
Lean mitigation for Brazilian tax-ID squatting, found while reviewing the gold app (#1388 follow-ups). The full fix (a CPF becomes exclusive only once the provider approves it) was deliberately not chosen; it stays open under the new RISK-026.
createSubaccountonly checkedaccountType, so any signed-in session could bind any string:"abc"bound the hash of an empty string, a missingtaxIdthrew a 500, and an unclaimed valid CPF became exclusive to whoever sent it first. It now requires a non-emptynameof at most 255 characters and a checksum-valid CPF (INDIVIDUAL) or CNPJ (COMPANY), in every environment including sandbox, before any provider call or write.newKycforwarded the submission without comparingtaxIdNumberwith the CPF claimed on the subaccount, so an account could end up approved for one identity while Vortex stored another CPF.newKycand the KYB level-1 API submission now answer400before any provider call unless the submitted tax ID hashes to the claimed one.docs/operations-brl-tax-id-claim-release.md(thefinancial_operationsclaim must go too, or the owner's retry still gets 409).400cases, a checksum-valid example CPF instead of12345678901),05-integrations/brla.md(invariant 5 claimed CPF validation that did not exist; new invariant 48, threat rows),RISK-REGISTER.md(RISK-026).Notes for review
429after five). It was dropped: review found thecreateSubaccount409count could be bypassed with a path variant (/createSubaccount/), the cap also blocked the caller's own tax IDs, and it was per instance only. Whether a tax ID is registered to another profile therefore stays observable through the403/409answers, bounded only by the global per-IP rate limit. This residual is recorded under RISK-026.newKyc, KYB level-1). The hosted KYB flow (initiateKybLevel1) and the provider's approval are not compared with the claimed tax ID, because Avenia subaccount creation sends onlyaccountTypeandname. Also recorded under RISK-026.countryTaxResidenceis notBRA; a company with a foreign TIN now gets400. Fail-closed on purpose (confirmed), because gating on the country would reopen the swap.400instead of silently claiming the wrong one. A client-side checksum there would be a nicer follow-up (gold gets one in Gold: faster landing, sturdier KYC polling, CPF check and key cleanup #1392).Test plan
name/taxId, formatted valid CPF. Controller: malformed bodies never reach the provider, the operation claim or the DB.newKyc/ KYB: mismatching, non-string and formatted-equivalent tax IDs.