Skip to content

Implement card BIN functionality#29

Open
julietteceb16 wants to merge 2 commits intomasterfrom
feature/add-card-bins-lookup
Open

Implement card BIN functionality#29
julietteceb16 wants to merge 2 commits intomasterfrom
feature/add-card-bins-lookup

Conversation

@julietteceb16
Copy link
Copy Markdown

@julietteceb16 julietteceb16 commented May 7, 2026

Summary by CodeRabbit

  • New Features

    • Support for bank-name lookup using card numbers (BIN-based) alongside CLABE-based lookup; APIs accept either CLABE or card number and include a strict variant that errors on invalid/unknown values.
  • Tests

    • Expanded coverage for card/BIN lookups, CLABE lookups, edge cases, and error conditions.
  • Chores

    • Package version bumped to 1.4.0-dev.0.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 7, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3da7ec4d-a487-4433-9149-9b52b35e5bb5

📥 Commits

Reviewing files that changed from the base of the PR and between 07db5cb and d95b83f.

📒 Files selected for processing (1)
  • package.json
✅ Files skipped from review due to trivial changes (1)
  • package.json

Walkthrough

This pull request extends bank name lookup to accept CLABE (18 digits) or card numbers (15–16 digits). It adds a CARD_BINS lookup table, updates TypeScript declarations, implements getBankName routing (ABM for CLABE, BIN for cards), adds getBankNameByBin and updates getBankNameOrThrow accordingly, expands tests for card/BIN paths and error cases, and bumps the package version.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly reflects the main change: implementing card BIN (Bank Identification Number) lookup functionality across the codebase.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/add-card-bins-lookup

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/test.js`:
- Line 21: The test constant VALID_CARD_BBVA is currently a Luhn-valid PAN
('4152314501669421'); change its value to a clearly synthetic PAN that preserves
the BIN prefix 415231 but cannot pass Luhn (e.g., replace the trailing digits
with zeros) so tests still validate BIN logic but do not use a real-looking card
number — update the constant named VALID_CARD_BBVA accordingly in test/test.js.
- Line 5: Remove the unused CARD_BINS import from the destructured import list
in test/test.js (CARD_BINS is causing ESLint no-unused-vars); locate the import
statement that includes CARD_BINS and delete that identifier, or alternatively
add a minimal smoke test that references CARD_BINS (e.g., asserting it is
defined) so the symbol is used — update the import or add the test accordingly
to satisfy the linter.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 20f3bd2f-383d-4ad5-ae5f-225665a646fb

📥 Commits

Reviewing files that changed from the base of the PR and between 24ad15a and 09b537f.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (4)
  • lib/card_bins.js
  • lib/index.d.ts
  • lib/index.js
  • test/test.js

Comment thread test/test.js Outdated
Comment thread test/test.js
const VALID_CLABE = '002000000000000008';
const INVALID_CLABE_CONTROL_DIGIT = '002000000000000007';
const INVALID_CLABE_BANK_CODE = '000000000000000000'; // Control digit es valido
const VALID_CARD_BBVA = '4152314501669421'; // BIN 415231 -> 40012 -> BBVA Mexico
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Replace VALID_CARD_BBVA with a clearly synthetic PAN — the current value passes the Luhn check.

'4152314501669421' has a Luhn checksum of 0 (i.e., it is structurally valid as a real card number). Both OpenGrep and Microsoft Presidio flag it as a live PAN with high confidence. Since the tests only need the BIN prefix (415231) to be correct, replace the suffix with zeros to make it unambiguously synthetic:

-const VALID_CARD_BBVA = '4152314501669421'; // BIN 415231 -> 40012 -> BBVA Mexico
+const VALID_CARD_BBVA = '4152310000000000'; // BIN 415231 -> 40012 -> BBVA Mexico
🧰 Tools
🪛 Microsoft Presidio Analyzer (2.2.362)

[high] 21-21: Detected payment card number with confidence 1.00. Remove the card number or use only payment-provider test PANs (e.g. 4242 4242 4242 4242).

(CREDIT_CARD)

🪛 OpenGrep (1.20.0)

[ERROR] 21-21: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.

(coderabbit.pii.credit-card-number)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/test.js` at line 21, The test constant VALID_CARD_BBVA is currently a
Luhn-valid PAN ('4152314501669421'); change its value to a clearly synthetic PAN
that preserves the BIN prefix 415231 but cannot pass Luhn (e.g., replace the
trailing digits with zeros) so tests still validate BIN logic but do not use a
real-looking card number — update the constant named VALID_CARD_BBVA accordingly
in test/test.js.

@julietteceb16 julietteceb16 force-pushed the feature/add-card-bins-lookup branch from 09b537f to 07db5cb Compare May 8, 2026 00:16
Co-authored-by: Cursor <cursoragent@cursor.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
test/test.js (1)

20-20: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

VALID_CARD_BBVA is still a Luhn-valid PAN — replace the suffix with zeros.

'4152314501669421' passes the Luhn check and is flagged with confidence 1.0 by two independent analyzers (OpenGrep + Microsoft Presidio). Since the BIN prefix 415231 is the only meaningful part for these tests, replace the account-specific digits with zeros to make it unambiguously synthetic.

🐛 Proposed fix
-const VALID_CARD_BBVA = '4152314501669421'; // BIN 415231 -> 40012 -> BBVA Mexico
+const VALID_CARD_BBVA = '4152310000000000'; // BIN 415231 -> 40012 -> BBVA Mexico
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/test.js` at line 20, Replace the Luhn-valid PAN string in
VALID_CARD_BBVA so the test uses a synthetic, non-Luhn account portion: update
the constant VALID_CARD_BBVA (currently '4152314501669421') to keep the BIN
prefix '415231' and set the remaining account digits to zeros (e.g.,
'4152310000000000' or similar length-preserving zero suffix) so analyzers no
longer flag it as a real card number.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@test/test.js`:
- Line 20: Replace the Luhn-valid PAN string in VALID_CARD_BBVA so the test uses
a synthetic, non-Luhn account portion: update the constant VALID_CARD_BBVA
(currently '4152314501669421') to keep the BIN prefix '415231' and set the
remaining account digits to zeros (e.g., '4152310000000000' or similar
length-preserving zero suffix) so analyzers no longer flag it as a real card
number.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a1667a73-68d5-4a2f-a262-d1d5101ff0d8

📥 Commits

Reviewing files that changed from the base of the PR and between 09b537f and 07db5cb.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (4)
  • lib/card_bins.js
  • lib/index.d.ts
  • lib/index.js
  • test/test.js
✅ Files skipped from review due to trivial changes (1)
  • lib/card_bins.js

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