Skip to content

fix: size and validate the CVC from the detected card brand - #1707

Merged
ArushKapoorJuspay merged 3 commits into
juspay:mainfrom
hossam-magdy:fix/brand-aware-cvc-length
Aug 12, 2026
Merged

fix: size and validate the CVC from the detected card brand#1707
ArushKapoorJuspay merged 3 commits into
juspay:mainfrom
hossam-magdy:fix/brand-aware-cvc-length

Conversation

@hossam-magdy

@hossam-magdy hossam-magdy commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

Fixes #1706.

In a bare card element every CVC rule runs against an empty brand, so the rules fall back to the default card pattern, which accepts 3 or 4 digits, and the input's maxLength was a literal 4. A 4-digit CVC on a Visa therefore passed the client and failed at the processor as an opaque cvv_invalid.

This derives a CVC-scoped brand and routes the CVC rules through it: the input's maxLength, the formatter, the range checks, and the CVC-to-ZIP auto-advance. A CVC already in the field when the brand arrives is re-judged against it, by the same rule a blur already uses, so narrowing invalidates it and widening makes it valid again. The value the user typed is never rewritten and never deleted.

The hook's cardBrand is left byte-identical to upstream. That is deliberate and is the whole reason the change is scoped this way: widening it would also put card_network on the wire in this mode, make checkIsCardSupported decline brands outside the merchant's configured card_networks, narrow maxCardLength, and wake the clear-expiry-and-CVC-on-brand-change effect. A PR about CVC length should change none of those, and this one changes none of them. Payment.res is untouched.

Three things reviewers should see explicitly

  • The individual cardCvc element is NOT fixed. It is a separate iframe whose local card number is always empty, so it cannot know the brand, and there is no association between a cardNumber and the cardCvc of the same form to route one to it. Its input keeps maxLength=4. Tightening only the submit-time gate was tried and dropped: it rejects input that the CVC iframe still shows as valid, and the error surfaces in a frame with no CVC field, which is worse than the status quo. Closing this properly looks like a design decision rather than something to invent inside a bugfix.
  • The two saved-card CVC inputs keep maxLength=4 on purpose. In those flows the scheme atom IS written, so the CVC brand is the saved card's scheme and the formatter already caps the value. Measured: typing 1234 into a saved Visa's CVC re-collect field yields 123, with the attribute still reading 4. The literal never binds, so converting it would be churn.
  • In the payment element the maxLength attribute does change (4 becomes 3 on a Visa), because that is where the brand was already available. The submitted value does not change, since the formatter already capped it. The only user-visible delta is that the browser now blocks a phantom 4th keystroke it previously accepted and discarded.

One pre-existing gap this does not widen, but worth knowing

emitIsFormReadyForSubmission only fires when all three of (isCardValid, isExpiryValid, isCVCValid) are Some, so any transition to None emits nothing and a previously-emitted true is never retracted. Upstream already reaches that state on its own: changeCVCNumber sets None on any invalid keystroke, so deleting a digit from an accepted CVC leaves the same stale true. Using the blur rule for the re-judge means this change reaches Some(false) rather than None when a CVC goes stale, so the signal retracts correctly here. Making the emit total is a change to the merchant-facing event contract and belongs in its own PR.

How did you test it?

Added two cypress assertions to 02-cards/01-card-validation.cy.ts, next to the existing per-brand formatting tests: the CVC maxlength is 3 for a Visa and 4 for an American Express. The first fails on main. Note the cypress job is skipped on fork PRs, so these have not run in CI here.

Beyond that, manually against a hosted sandbox with a real connector, driving the elements in a browser. The bare card element is not in any demo UI, so it was mounted directly to exercise it.

CVC maxlength:

before after
bare card element, no PAN 4 4
bare card element, Visa 4 3
bare card element, Amex 4 4
payment element, Visa / Amex 4 / 4 3 / 4
individual cardCvc element 4 4 (by design, see above)
saved-card CVC re-collect 4 4 (value already capped, see above)

Validity and submit, in the bare card element. These matter more than the attribute, because a wrong-length CVC used to be reported as valid:

Case Result
Amex PAN, CVC 123, blur invalid, and form readiness emits false
Visa PAN, CVC 123, blur valid, readiness true
Amex PAN, CVC 1234, blur valid, readiness true
CVC 1234 typed before the PAN, then a Visa PAN value 1234 kept, marked invalid

Checklist

  • I ran npm run re:build
  • I reviewed submitted code
  • I added unit tests for my changes where possible

@semanticdiff-com

semanticdiff-com Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review changes with  SemanticDiff

Changed Files
File Status
  cypress-tests/cypress/e2e/02-cards/01-card-validation.cy.ts  0% smaller
  src/CardUtils.res Unsupported file format
  src/Components/CardFields.res Unsupported file format
  src/Hooks/CommonCardProps.res Unsupported file format
  src/SingleLineCardPayment.res Unsupported file format

@hossam-magdy
hossam-magdy marked this pull request as draft July 26, 2026 01:21
@XyneSpaces

Copy link
Copy Markdown

[should-fix] The new messageParentWindow call in src/RenderPaymentMethods.res posts the detected card brand using the default targetOrigin="*". Please pass the validated parent origin from the keys atom instead, so this brand message is only delivered to the expected merchant origin.

// src/RenderPaymentMethods.res
let {parentURL} = Recoil.useRecoilValueFromAtom(keys)
...
switch paymentType {
| CardNumberElement =>
  messageParentWindow(
    [("cardBrandForSiblings", detectedBrand->JSON.Encode.string)],
    ~targetOrigin=parentURL,
  )
| _ => ()
}

@XyneSpaces

Copy link
Copy Markdown

[should-fix] src/RenderPaymentMethods.res: new cardBrandForSiblings postMessage uses a wildcard origin

messageParentWindow defaults to targetOrigin="*". Other postMessage callers in the codebase (e.g., InputField.res) pass ~targetOrigin=parentURL from the keys atom. Broadcast the brand update with the validated parent origin instead of *.

Also confirm that Elements.res fan-out via iframePostMessage targets a constrained origin (it currently routes through GlobalVars.targetOrigin) and is not "*".

@hossam-magdy
hossam-magdy force-pushed the fix/brand-aware-cvc-length branch from 43c192b to 7176706 Compare July 26, 2026 03:16
@hossam-magdy
hossam-magdy force-pushed the fix/brand-aware-cvc-length branch from 7176706 to 27d10fb Compare July 26, 2026 11:48
@hossam-magdy hossam-magdy changed the title fix: size the CVC input from the card brand instead of a fixed 4 fix: size and validate the CVC from the detected card brand Jul 26, 2026
@hossam-magdy

Copy link
Copy Markdown
Contributor Author

Thanks for both notes (#1707 (comment) and #1707 (comment)).

Both point at src/RenderPaymentMethods.res posting a cardBrandForSiblings message, which was in the revision that was on this PR when you commented. That approach (publishing the brand from the cardNumber frame and relaying it to its siblings) has since been dropped entirely and the branch force-pushed. The current head touches four files and adds no postMessage, no cross-frame message and no origin-bearing call at all, so there is nothing left here to constrain.

On the second half, about the Elements.res fan-out: the answer is the opposite of the confirmation you were after.

That is existing behaviour rather than something this PR introduces, so I have deliberately kept it out of a CVC-length change. Happy to open a separate issue for it if that would be useful.

@hossam-magdy
hossam-magdy marked this pull request as ready for review July 26, 2026 12:21
@hossam-magdy
hossam-magdy force-pushed the fix/brand-aware-cvc-length branch from 27d10fb to 4fbd304 Compare July 26, 2026 12:33
@hossam-magdy
hossam-magdy force-pushed the fix/brand-aware-cvc-length branch from 4fbd304 to 0839518 Compare July 26, 2026 12:35
Comment thread src/Hooks/CommonCardProps.res
@aritro2002

Copy link
Copy Markdown
Contributor
Screen.Recording.2026-07-27.at.4.02.38.pm.mov

I noticed that when I focus on the CVC input and then blur it, the colour does not change to red.

@aritro2002

Copy link
Copy Markdown
Contributor

Could you also please attach videos showing the behaviour before and after your changes?

@hossam-magdy hossam-magdy left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Re: this comment

Thanks for the catch. Reproduced and fixed in 0aa3dae.

The cause is the auto-advance rather than the blur. Completing the expiry focuses the CVC (CommonCardProps.res#L211), and focusing clears that field's validity (InputField.res#L72), so the verdict went without the user editing anything. It now auto-advances only when the CVC is empty.

Submit stayed blocked throughout (Payment.res#L56), so the defect was the missing red only.

Comment thread src/Hooks/CommonCardProps.res Outdated
@hossam-magdy

Copy link
Copy Markdown
Contributor Author

Any suggestions how to move this PR forward / make a decision on it? … cc: @aritro2002 @Shivam25092001

@Shivam25092001

Copy link
Copy Markdown
Contributor

@AbhishekChorotiya @sakksham7 could you please help us with the review here.

aritro2002
aritro2002 previously approved these changes Aug 2, 2026
@ArushKapoorJuspay
ArushKapoorJuspay merged commit a1f6f54 into juspay:main Aug 12, 2026
6 of 8 checks passed
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.

CVC input accepts 4 digits regardless of card brand

5 participants