Skip to content

fix(mercury): authoritative org-scoped Finance account binding preflight - #204

Draft
chitcommit wants to merge 3 commits into
mainfrom
fix/mercury-private-account-binding-preflight
Draft

chitcommit wants to merge 3 commits into
mainfrom
fix/mercury-private-account-binding-preflight

Conversation

@chitcommit

Copy link
Copy Markdown
Contributor

Existing Mercury reconciliation workstream — ChittyConnect #343 / ChittyFinance #203 / chittyconnect-finance #21/#22

The canonical chittyconnect-finance adapter requires a read-only exact provider-org/provider-account→Finance account binding resolver before any sync. ChittyFinance's private API exposes global source-owner reads but does not expose the corresponding account-binding contract. Previous Finance read-only audits found many Mercury-labeled accounts without external IDs, including ARIBIA LLC - MGMT. Account display names, mask suffixes, and shared DBAs must never be used as substitutes.

Changes

  • Add bounded SystemStorage.getMercuryAccountBindingCandidatesByExternalId() querying all tenants by the exact external Mercury account UUID and returning at most two candidates. Read-only; no auto-provision, no financial records changed.
  • Add GET /internal/chittyconnect/mercury/account-binding/:accountId behind the existing private service-binding hostname/caller gate. Verify exact org/account IDs, active tenant and exact canonical business identity, unique provider account binding, active Mercury institution, exact metadata mercuryOrganizationId / mercuryOrgId, and authorized tenant. On incomplete/missing/duplicate/conflicting data return 409/503 rather than a guessed match.
  • Mutation-test the guards: absent external ID, duplicate, wrong tenant, wrong organization, inactive/non-Mercury account, contradictory metadata, public access, storage exceptions; no secret/account-number disclosure.

Integration and deployment gates

This PR implements a read-only prerequisite; NOT a durable reconciliation consumer, Finance readiness/consume endpoint, completed ledger sync, or permission to write. Existing seven Mercury webhooks unchanged; CHIT/MNW registration remains blocked. Do not activate PR #343's org-scoped events until canonical provider reads, financial account binding, source ownership, write-executor idempotency, queue receipts, and governance are verified. No new queue/DB/schema/store/secret.

Finance operating constitution: document governs projections, fail closed on incomplete coverage, no hand-written apply SQL and operator-gate all production financial writes.

@coderabbitai

coderabbitai Bot commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@claude

claude Bot commented Oct 12, 2026

Copy link
Copy Markdown

Review of PR 204

Solid fail-closed design: read-only, no schema change, strict UUID validation, every binding dimension checked, and no internals leaked in error bodies. No blocking issues.

Suggestions

  1. Full tenant scan: storage.getTenants() loads every tenant per call just to find one id. A single-tenant lookup through SystemStorage would be cheaper. If the full list is deliberate, add a comment saying why.
  2. Weak test assertions: the alternatives loop only asserts status 409, so it can't tell which guard fired. Asserting on the error code would make the mutation tests meaningful.
  3. Institution not filtered in the query: a non-Mercury account sharing the UUID turns a valid binding into a duplicate 409. That is arguably desired fail-closed behaviour, so a comment documenting it would help.
  4. UUID regex duplicated in the route and in storage. Consider a shared constant. The regex is case-insensitive but eq is exact, so an upper-case request id gets 409 instead of a match. Decide whether to normalise.
  5. Types: any[] for tenants and candidates drops type safety from the storage return types.

Test gaps

  • No 400 validation tests (bad UUIDs, missing params, identity over 128 chars).
  • No tests for an inactive tenant, a canonical identity mismatch, or an unknown tenantId. Only the metadata: {} case is covered.
  • No test that mercuryOrganizationId and mercuryOrgId both present and equal is accepted.
  • No test that Mercury Bank is accepted as an institution value.
  • No test for array or non-object metadata.
  • No direct test of the storage method or its limit(2) behaviour.

Security
The route is behind the private gate and the public-host 404 is tested. The response carries only ids and the canonical name, and the MUST_NOT_LEAK assertions are good. No concerns.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant