fix: convert base64 encoding/decoding to utf8#135
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR removes explicit type annotations from encryption utility functions and updates test assertions to validate standard base64 encoding instead of base64url encoding. Changes span the test file, encryption helper, and utility functions with adjustments to JSDoc comments alongside signature modifications. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/__tests__/open-attestation/encrypt-decrypt.test.ts`:
- Around line 124-128: The test for encodeDocument/decodeDocument doesn't
exercise the '+'/'/' alphabet difference because using 'hello' produces only
ASCII letters; update the test around encodeDocument and decodeDocument to
either assert the exact expected base64 output for a known input (e.g., assert
encodeDocument('hello') === 'aGVsbG8=') or use an input whose UTF-8 bytes will
produce '+' or '/' when base64-encoded (for example a Buffer with bytes that map
to those characters) so the test truly verifies standard base64 vs base64url
behavior and still round-trips with decodeDocument.
In `@src/open-attestation/utils.ts`:
- Around line 40-53: The decoder decodeDocument must remain backward-compatible
with legacy base64url payloads emitted under ENCRYPTION_PARAMETERS.version
"OPEN-ATTESTATION-TYPE-1"; change decodeDocument to first normalize base64url to
standard base64 (replace '-'->'+', '_'->'/', add padding as needed) before
calling forge.util.decode64 so it accepts both alphabets (or attempt decode64
and fall back to URL-to-standard normalization), ensuring decryptString
continues to handle existing type-1 ciphertexts without bumping the wire-format
version; reference functions: decodeDocument, encodeDocument and the
ENCRYPTION_PARAMETERS.version "OPEN-ATTESTATION-TYPE-1".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b26275f0-b75a-4a4d-b291-1c1494e91088
📒 Files selected for processing (3)
src/__tests__/open-attestation/encrypt-decrypt.test.tssrc/open-attestation/encrypt.tssrc/open-attestation/utils.ts
| it('encodeDocument should return standard base64 (matches @govtechsg/oa-encryption / tradetrust-cli)', () => { | ||
| const input = 'hello'; | ||
| const encoded = encodeDocument(input); | ||
| // encodeURIComponent encodes +, /, = — so this only passes when output is truly query-safe (base64url). | ||
| expect(encodeURIComponent(encoded)).toBe(encoded); | ||
| expect(encoded).toMatch(base64Regex); | ||
| expect(decodeDocument(encoded)).toBe(input); |
There was a problem hiding this comment.
This case doesn't really prove standard base64 vs base64url.
'hello' encodes to aGVsbG8=, so this assertion never exercises the + / / alphabet difference. Use an input whose UTF-8 bytes produce one of those characters, or assert the exact expected encoding for a known value.
Suggested test update
it('encodeDocument should return standard base64 (matches `@govtechsg/oa-encryption` / tradetrust-cli)', () => {
- const input = 'hello';
+ const input = '🦄';
const encoded = encodeDocument(input);
- expect(encoded).toMatch(base64Regex);
+ expect(encoded).toBe('8J+mhA==');
expect(decodeDocument(encoded)).toBe(input);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('encodeDocument should return standard base64 (matches @govtechsg/oa-encryption / tradetrust-cli)', () => { | |
| const input = 'hello'; | |
| const encoded = encodeDocument(input); | |
| // encodeURIComponent encodes +, /, = — so this only passes when output is truly query-safe (base64url). | |
| expect(encodeURIComponent(encoded)).toBe(encoded); | |
| expect(encoded).toMatch(base64Regex); | |
| expect(decodeDocument(encoded)).toBe(input); | |
| it('encodeDocument should return standard base64 (matches `@govtechsg/oa-encryption` / tradetrust-cli)', () => { | |
| const input = '🦄'; | |
| const encoded = encodeDocument(input); | |
| expect(encoded).toBe('8J+mhA=='); | |
| expect(decodeDocument(encoded)).toBe(input); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/__tests__/open-attestation/encrypt-decrypt.test.ts` around lines 124 -
128, The test for encodeDocument/decodeDocument doesn't exercise the '+'/'/'
alphabet difference because using 'hello' produces only ASCII letters; update
the test around encodeDocument and decodeDocument to either assert the exact
expected base64 output for a known input (e.g., assert encodeDocument('hello')
=== 'aGVsbG8=') or use an input whose UTF-8 bytes will produce '+' or '/' when
base64-encoded (for example a Buffer with bytes that map to those characters) so
the test truly verifies standard base64 vs base64url behavior and still
round-trips with decodeDocument.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
## [2.11.1](v2.11.0...v2.11.1) (2026-03-06) ### Bug Fixes * convert base64 encoding/decoding to utf8 ([#135](#135)) ([f8b9cec](f8b9cec))
|
🎉 This PR is included in version 2.11.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
3 participants
Summary
What is the background of this pull request?
Changes
Issues
What are the related issues or stories?
Summary by CodeRabbit
Release Notes
Tests
Changes
Documentation