Skip to content

fix(ro): correct checksum alignment for 10-digit CUI numbers #145

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shayanzarei wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(ro): correct checksum alignment for 10-digit CUI numbers #145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/ro/cui.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ describe('ro/cui', () => {
expect(result).toEqual('185 472 90');
});

it('validate:18547290 (8 digits, padded to 10 for checksum)', () => {
const result = validate('18547290');

expect(result.isValid).toBe(true);
});

it('validate:185 472 90', () => {
const result = validate('185 472 90');

Expand All @@ -25,4 +31,11 @@ describe('ro/cui', () => {

expect(result.error).toBeInstanceOf(InvalidChecksum);
});

it('validate:18547291 (8 digits, invalid checksum after padding)', () => {
const result = validate('18547291');

expect(result.isValid).toBe(false);
expect(result.error).toBeInstanceOf(InvalidChecksum);
});
});
5 changes: 4 additions & 1 deletion src/ro/cui.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,10 @@ const impl: Validator = {
return { isValid: false, error: new exceptions.InvalidFormat() };
}

const [front, check] = strings.splitAt(value.padStart(9, '0'), -1);
// FIX: Pad to 10 total digits.
// This ensures that 'front' is always exactly 9 digits,
// perfectly matching the 9 weights provided.
Comment on lines +60 to +62
Copy link

@gemini-code-assist gemini-code-assist bot Feb 25, 2026

Choose a reason for hiding this comment

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

medium

While the comment is helpful, using prefixes like FIX: can be mistaken for temporary code markers (like TODO or FIXME) and may cause confusion for future maintainers. For long-term clarity, it's better to have a comment that explains the logic's purpose. I suggest rephrasing to focus on why the padding is necessary.

Suggested change
// FIX: Pad to 10 total digits.
// This ensures that 'front' is always exactly 9 digits,
// perfectly matching the 9 weights provided.
// Pad to 10 total digits to ensure the `front` part is always 9 digits.
// This correctly right-aligns numbers with fewer than 10 digits
// for the checksum calculation.

const [front, check] = strings.splitAt(value.padStart(10, '0'), -1);
Copy link

@gemini-code-assist gemini-code-assist bot Feb 25, 2026

Choose a reason for hiding this comment

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

security-high high

The Romanian CUI validation logic is fundamentally flawed and remains broken despite the alignment fix. The checksum calculation on line 72 (sum % 10) always evaluates to 0 because sum is defined as 10 * weightedSum(...) on line 65.

This creates a significant security bypass where the validator incorrectly marks any CUI ending in '0' as valid (provided it meets length and digit requirements) and rejects almost all valid CUIs that do not end in '0'.

To correctly implement the Romanian CUI algorithm, the logic should be: (Sum * 10) % 11, and if the result is 10, the check digit is 0. This can be achieved by changing the logic to ((weightedSum(...) * 10) % 11) % 10.


const sum =
10 *
Expand Down