Skip to content

feat: added encrypt and decrypt oa functionlities#131

Merged
rongquan1 merged 9 commits intomainfrom
feat/TT-907-encrypt-decrypt-oa-functions
Feb 26, 2026
Merged

feat: added encrypt and decrypt oa functionlities#131
rongquan1 merged 9 commits intomainfrom
feat/TT-907-encrypt-decrypt-oa-functions

Conversation

@manishdex25
Copy link
Copy Markdown
Contributor

@manishdex25 manishdex25 commented Feb 24, 2026

Summary

What is the background of this pull request?

Changes

  • What are the changes made in this pull request?
  • Change this and that, etc...

Issues

What are the related issues or stories?

Summary by CodeRabbit

  • New Features

    • Client-side document encrypt/decrypt APIs with optional custom keys, URL-safe encoded outputs, key generation, encode/decode utilities, and a structured encryption result in the public API.
  • Tests

    • Comprehensive tests for round-trip encrypt/decrypt, Unicode and large documents, key handling, encoding/decoding, and error cases.
  • Chores

    • Added a cryptography library dependency and corresponding type definitions.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 24, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds AES‑GCM client-side encryption/decryption for open-attestation: new encryptString/decryptString, encoding/key utilities, IEncryptionResults type, tests, and node-forge dependency; functions exported from the package public API. (≤50 words)

Changes

Cohort / File(s) Summary
Dependencies
package.json
Added runtime dependency node-forge and dev dependency @types/node-forge.
Utilities & Parameters
src/open-attestation/utils.ts
New ENCRYPTION_PARAMETERS, generateEncryptionKey, encodeDocument, decodeDocument using node-forge (base64url and hex helpers).
Encryption
src/open-attestation/encrypt.ts
New encryptString(document, key?): input validation, key/IV handling, AES‑GCM encryption, returns IEncryptionResults (cipherText/iv/tag base64, key hex, type).
Decryption
src/open-attestation/decrypt.ts
New decryptString({ cipherText, tag, iv, key, type }): validates version/type, converts encodings, performs AES‑GCM decryption and decodes plaintext.
Types & Exports
src/open-attestation/types.ts, src/open-attestation/index.ts
Added IEncryptionResults interface and re-exported encryptString/decryptString from the module index.
Tests
src/__tests__/open-attestation/encrypt-decrypt.test.ts
New comprehensive tests covering encryption/decryption round-trips, unicode/large documents, key handling, error cases, and encode/decode integrity.

Sequence Diagram(s)

sequenceDiagram
  participant Client as Client
  participant API as encryptString / decryptString
  participant Utils as encodeDocument / decodeDocument
  participant Forge as node-forge (AES‑GCM)

  rect rgba(200,200,255,0.5)
  Client->>API: call encryptString(document, [key])
  API->>Utils: encodeDocument(document)
  API->>Forge: generate IV / create cipher / encrypt
  Forge-->>API: cipherText, iv, tag
  API-->>Client: IEncryptionResults (cipherText, iv, tag, key, type)
  end

  rect rgba(200,255,200,0.5)
  Client->>API: call decryptString(IEncryptionResults)
  API->>Forge: create decipher with iv, tag, key
  Forge-->>API: plaintext bytes or error
  API->>Utils: decodeDocument(plaintext)
  API-->>Client: decrypted document string
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hop through bytes beneath the moonlight,
I nibble keys and tuck them tight,
AES‑GCM hums, the papers keep,
I guard the secrets while you sleep,
Hop, encrypt, then off I flight.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is entirely incomplete; it contains only the template structure with placeholder text and no substantive information about background, actual changes, or related issues. Fill in all template sections with concrete details: provide background/rationale, list actual code changes and affected modules, and reference related issues or stories (e.g., TT-907).
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title partially describes the changeset by mentioning encryption/decryption functionality, but contains a typo ('functionlities' instead of 'functionalities') and lacks specificity about the scope or impact of these new features.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/TT-907-encrypt-decrypt-oa-functions

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 and usage tips.

Copy link
Copy Markdown
Contributor

@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: 3

🧹 Nitpick comments (5)
src/__tests__/open-attestation/encrypt-decrypt.test.ts (1)

70-84: Dead code: encryptionResults = encryptString('hello world') is unused in this test.

Line 71 re-assigns encryptionResults but the result is never read before the subsequent expect(() => encryptString(...)).toThrow(...) calls. It can be removed.

♻️ Proposed fix
   test('should throw error if input is not a string', () => {
-    encryptionResults = encryptString('hello world');
     expect(() =>
       encryptString(
🤖 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 70 - 84,
Remove the dead assignment to encryptionResults in the test: delete the line
"encryptionResults = encryptString('hello world');" inside the 'should throw
error if input is not a string' test (or replace it with a call/assert if the
intent was to use the result); ensure only the two expect(() =>
encryptString(...)).toThrow(...) assertions remain so the test does not contain
unused variables.
src/open-attestation/encrypt.ts (1)

20-33: LGTM — no key-length validation when key is provided.

If a caller passes an incorrectly-sized hex string (e.g., 32 hex chars = 128-bit key when 256-bit is expected), node-forge will silently create a decipher with the wrong key size, producing garbage output. Consider asserting key.length === ENCRYPTION_PARAMETERS.keyLength / 4 (64 chars for 256-bit).

🛡️ Suggested guard in `makeCipher`
 const makeCipher = (encryptionKey: string = generateEncryptionKey()) => {
+  const expectedHexLen = ENCRYPTION_PARAMETERS.keyLength / 4;
+  if (encryptionKey.length !== expectedHexLen) {
+    throw new Error(`Key must be ${expectedHexLen} hex characters (${ENCRYPTION_PARAMETERS.keyLength} bits)`);
+  }
   const iv = generateIv();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/open-attestation/encrypt.ts` around lines 20 - 33, The makeCipher
function must validate a provided encryptionKey's hex length before using it:
ensure when encryptionKey is passed (in makeCipher) that encryptionKey.length
=== ENCRYPTION_PARAMETERS.keyLength / 4 (e.g., 64 chars for a 256-bit key) and
throw a clear Error if it mismatches; reference ENCRYPTION_PARAMETERS.keyLength
to compute the expected hex length and perform this check at the top of
makeCipher so incorrect-size keys are rejected instead of producing garbage
output from forge.cipher.createCipher.
package.json (1)

134-134: Consider native crypto over node-forge for new encryption code.

node-forge adds a non-trivial runtime dependency. Since the project already requires Node ≥ 20 (which ships the Web Crypto API natively) and modern browsers also expose globalThis.crypto.subtle, the same AES-GCM operations could be implemented without this extra package. If browser compatibility via the ESM build is a concern, globalThis.crypto.subtle covers all evergreen browsers.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@package.json` at line 134, The dependency "node-forge" should be removed and
replaced by using the native Web Crypto API (globalThis.crypto.subtle) for
AES‑GCM and related crypto operations; update package.json to drop "node-forge"
and refactor any code that imports or references "node-forge" (search for
occurrences of "node-forge", AES‑GCM helper functions, or usage sites) to use
SubtleCrypto methods (generateKey, encrypt, decrypt, importKey, exportKey) and
ensure key/IV handling and encoding/decoding are adapted for the Web Crypto API
and Node ≥20 compatibility.
src/open-attestation/decrypt.ts (1)

19-19: Use ENCRYPTION_PARAMETERS.algorithm instead of the hardcoded string.

encrypt.ts uses ENCRYPTION_PARAMETERS.algorithm when creating the cipher, but decrypt.ts hardcodes 'AES-GCM'. If the algorithm constant were ever updated, the decipher would silently diverge.

♻️ Proposed fix
-  const decipher = forge.cipher.createDecipher('AES-GCM', keyBytestring);
+  const decipher = forge.cipher.createDecipher(ENCRYPTION_PARAMETERS.algorithm, keyBytestring);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/open-attestation/decrypt.ts` at line 19, Replace the hardcoded algorithm
string in the decipher creation with the shared constant so decryption stays in
sync: in decrypt.ts change the call that uses
forge.cipher.createDecipher('AES-GCM', keyBytestring) to use
ENCRYPTION_PARAMETERS.algorithm (the same constant used in encrypt.ts). Ensure
ENCRYPTION_PARAMETERS is imported or available in this module and use
ENCRYPTION_PARAMETERS.algorithm when invoking forge.cipher.createDecipher
(mirroring how encrypt.ts uses ENCRYPTION_PARAMETERS.algorithm).
src/open-attestation/utils.ts (1)

16-22: ENCRYPTION_PARAMETERS is mutable; freeze it.

Any module-level consumer could accidentally reassign a property (e.g., ENCRYPTION_PARAMETERS.tagLength = 64). Object.freeze prevents silent misconfiguration.

♻️ Proposed fix
-export const ENCRYPTION_PARAMETERS = {
+export const ENCRYPTION_PARAMETERS = Object.freeze({
   algorithm: 'AES-GCM' as const,
   keyLength: 256,
   ivLength: 96,
   tagLength: 128,
   version: 'OPEN-ATTESTATION-TYPE-1',
-};
+});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/open-attestation/utils.ts` around lines 16 - 22, ENCRYPTION_PARAMETERS is
currently a mutable object and should be made immutable to prevent accidental
runtime reassignments; change its definition so the exported
ENCRYPTION_PARAMETERS is frozen with Object.freeze (apply freeze to the
top-level object and, if needed for nested objects, recursively freeze) so
consumers cannot modify properties like tagLength, and keep the exported
constant name ENCRYPTION_PARAMETERS and its existing fields (algorithm,
keyLength, ivLength, tagLength, version) intact.
🤖 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`:
- Line 56: The test currently hardcodes an encryption key hex literal (seen in
encrypt-decrypt.test.ts) which triggers Gitleaks; replace both hardcoded
occurrences with a programmatically generated key instead (e.g., call and import
a utility like generateEncryptionKey() or use Node's crypto to derive a hex key
inside the test), update the test to use that generated value where
encryptionKey is referenced (and assert behavior remains the same), and remove
the literal so CI Gitleaks no longer flags the test; alternatively, if
generation is not feasible, add a Gitleaks allow-list comment on those specific
lines, but prefer generating via generateEncryptionKey()/crypto in the test
harness.
- Around line 115-119: The test for encodeDocument is using encodeURI which does
not catch '+' '/' '=' from standard base64, so update the test or
implementation: either change the assertion in encrypt-decrypt.test.ts to use
encodeURIComponent(encoded) === encoded to verify query-safe characters, or
change encodeDocument implementation to produce URL-safe base64 (replace
'+'→'-', '/'→'_', trim or replace '=' padding) and assert that output matches
that URL-safe form; reference the encodeDocument function and the failing test
to locate and apply the fix.
- Around line 13-15: The base64Regex includes an invalid trailing alternative
`{1}===` and should be fixed to only allow at most two '=' padding characters
(keep the `{3}=`? NO — use the valid trailing patterns `{3}=` is also wrong per
comment; replace the final alternatives so the only valid endings are `{3}=` and
`{2}==` corrected to the RFC-accurate patterns of 3 data chars + one '=' and 2
data chars + '=='); remove the `{1}===` alternative in base64Regex (identifier:
base64Regex). For encryptionKeyRegex, avoid the ast-grep ReDoS false positive by
replacing the dynamic RegExp constructor with a static regex literal using the
computed fixed quantifier value derived from ENCRYPTION_PARAMETERS.keyLength/4
(identifier: encryptionKeyRegex and ENCRYPTION_PARAMETERS.keyLength) — e.g.,
replace new RegExp(...) with a literal like /^[0-9a-f]{<N>}$/ where <N> is the
evaluated integer, which preserves behaviour and silences the warning.

---

Nitpick comments:
In `@package.json`:
- Line 134: The dependency "node-forge" should be removed and replaced by using
the native Web Crypto API (globalThis.crypto.subtle) for AES‑GCM and related
crypto operations; update package.json to drop "node-forge" and refactor any
code that imports or references "node-forge" (search for occurrences of
"node-forge", AES‑GCM helper functions, or usage sites) to use SubtleCrypto
methods (generateKey, encrypt, decrypt, importKey, exportKey) and ensure key/IV
handling and encoding/decoding are adapted for the Web Crypto API and Node ≥20
compatibility.

In `@src/__tests__/open-attestation/encrypt-decrypt.test.ts`:
- Around line 70-84: Remove the dead assignment to encryptionResults in the
test: delete the line "encryptionResults = encryptString('hello world');" inside
the 'should throw error if input is not a string' test (or replace it with a
call/assert if the intent was to use the result); ensure only the two expect(()
=> encryptString(...)).toThrow(...) assertions remain so the test does not
contain unused variables.

In `@src/open-attestation/decrypt.ts`:
- Line 19: Replace the hardcoded algorithm string in the decipher creation with
the shared constant so decryption stays in sync: in decrypt.ts change the call
that uses forge.cipher.createDecipher('AES-GCM', keyBytestring) to use
ENCRYPTION_PARAMETERS.algorithm (the same constant used in encrypt.ts). Ensure
ENCRYPTION_PARAMETERS is imported or available in this module and use
ENCRYPTION_PARAMETERS.algorithm when invoking forge.cipher.createDecipher
(mirroring how encrypt.ts uses ENCRYPTION_PARAMETERS.algorithm).

In `@src/open-attestation/encrypt.ts`:
- Around line 20-33: The makeCipher function must validate a provided
encryptionKey's hex length before using it: ensure when encryptionKey is passed
(in makeCipher) that encryptionKey.length === ENCRYPTION_PARAMETERS.keyLength /
4 (e.g., 64 chars for a 256-bit key) and throw a clear Error if it mismatches;
reference ENCRYPTION_PARAMETERS.keyLength to compute the expected hex length and
perform this check at the top of makeCipher so incorrect-size keys are rejected
instead of producing garbage output from forge.cipher.createCipher.

In `@src/open-attestation/utils.ts`:
- Around line 16-22: ENCRYPTION_PARAMETERS is currently a mutable object and
should be made immutable to prevent accidental runtime reassignments; change its
definition so the exported ENCRYPTION_PARAMETERS is frozen with Object.freeze
(apply freeze to the top-level object and, if needed for nested objects,
recursively freeze) so consumers cannot modify properties like tagLength, and
keep the exported constant name ENCRYPTION_PARAMETERS and its existing fields
(algorithm, keyLength, ivLength, tagLength, version) intact.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6f00437 and da6cbd5.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (8)
  • package.json
  • src/__tests__/fixtures/sample-oa-document.json
  • src/__tests__/open-attestation/encrypt-decrypt.test.ts
  • src/open-attestation/decrypt.ts
  • src/open-attestation/encrypt.ts
  • src/open-attestation/index.ts
  • src/open-attestation/types.ts
  • src/open-attestation/utils.ts

Comment thread src/__tests__/open-attestation/encrypt-decrypt.test.ts Outdated
Comment thread src/__tests__/open-attestation/encrypt-decrypt.test.ts Outdated
Comment thread src/__tests__/open-attestation/encrypt-decrypt.test.ts
Copy link
Copy Markdown
Contributor

@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 (3)
src/__tests__/open-attestation/encrypt-decrypt.test.ts (3)

52-65: ⚠️ Potential issue | 🟡 Minor

Avoid hardcoded hex key literals in tests.

These fixtures will trigger secret scanning. Generate a key at runtime (e.g., generateEncryptionKey() or crypto.randomBytes(32)).

🛡️ Proposed fix
 import {
   encryptString,
   decryptString,
   ENCRYPTION_PARAMETERS,
   encodeDocument,
   decodeDocument,
+  generateEncryptionKey,
 } from '../..';
@@
-      const encryptionKey = '35fb46ca758889669f38c83d2f159b0f5a320b5a97387a9eaecb5652d15e0e3d';
+      const encryptionKey = generateEncryptionKey();
@@
-      const encryptionKey = '35fb46ca758889669f38c83d2f159b0f5a320b5a97387a9eaecb5652d15e0e3d';
+      const encryptionKey = generateEncryptionKey();

Verification:

#!/bin/bash
# Find any remaining hardcoded key literals and confirm helper availability.
rg -n "35fb46ca758889669f38c83d2f159b0f5a320b5a97387a9eaecb5652d15e0e3d" -g '*.ts'
rg -n "generateEncryptionKey" src/open-attestation/index.ts src/open-attestation/utils.ts

Also applies to: 89-93

🤖 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 52 - 65,
The test currently uses a hardcoded hex encryption key literal which triggers
secret scanning; replace the literal with a runtime-generated key (call
generateEncryptionKey() or crypto.randomBytes(32) and hex-encode it) and use
that variable for both the encryptString(encryptionKey) call and the final
equality assertion; update the two occurrences (the test at lines ~52-65 and the
duplicate at ~89-93) to remove the literal and assert encryptionResults.key ===
generatedKey instead, keeping the other expectations (cipherText, iv, tag, type)
unchanged.

12-15: ⚠️ Potential issue | 🟡 Minor

Fix invalid base64 padding branch and make encryptionKeyRegex literal.

The {1}=== padding branch is invalid base64, and the dynamic RegExp can be a literal since the key length is fixed (256-bit → 64 hex chars). This also avoids the tool warning.

🐛 Proposed fix
-const base64Regex =
-  /^(?:[a-zA-Z0-9+/]{4})*(?:|(?:[a-zA-Z0-9+/]{3}=)|(?:[a-zA-Z0-9+/]{2}==)|(?:[a-zA-Z0-9+/]{1}===))$/;
-const encryptionKeyRegex = new RegExp(`^[0-9a-f]{${ENCRYPTION_PARAMETERS.keyLength / 4}}$`);
+const base64Regex =
+  /^(?:[a-zA-Z0-9+/]{4})*(?:[a-zA-Z0-9+/]{3}=|[a-zA-Z0-9+/]{2}==)?$/;
+const encryptionKeyRegex = /^[0-9a-f]{64}$/;

Verification:

#!/bin/bash
# Confirm keyLength is fixed (so the literal remains correct).
rg -n "keyLength" src/open-attestation/utils.ts
🤖 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 12 - 15,
The base64Regex includes an invalid padding branch (`{1}===`) and should be
corrected to only allow valid padding lengths, so update base64Regex to remove
the `{1}===` branch (ensure it matches groups of 4 with optional `3=` or `2==`
endings only); also replace the dynamic RegExp construction for
encryptionKeyRegex with a literal that matches 64 hex chars (since
ENCRYPTION_PARAMETERS.keyLength is fixed to 256 bits) by changing
encryptionKeyRegex to a literal like /^[0-9a-f]{64}$/ to eliminate the warning
and hardcode the expected key length.

124-127: ⚠️ Potential issue | 🟡 Minor

encodeURI doesn’t verify URL‑safe base64.

encodeURI leaves +, /, and = unescaped, so this assertion always passes for standard base64. Either assert standard base64 with base64Regex, or switch to URL‑safe base64 (and then use encodeURIComponent).

🐛 Proposed fix (align with current implementation)
-    it('encodeDocument should return url safe characters only', () => {
+    it('encodeDocument output contains only standard base64 characters', () => {
       const input = '🦄😱|certificate|证书|sijil|प्रमाणपत्र';
       const encoded = encodeDocument(input);
-      expect(encodeURI(encoded)).toBe(encoded);
+      expect(encoded).toMatch(base64Regex);
     });

Verification:

#!/bin/bash
# Demonstrate encodeURI vs encodeURIComponent behavior.
node -e "console.log(encodeURI('+/='), encodeURIComponent('+/='))"
🤖 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 -
127, The test incorrectly uses encodeURI to assert URL-safe base64; update the
test for encodeDocument to assert the actual expected encoding format: either
(preferred) validate the output against a standard base64 regex (e.g.,
base64Regex) if encodeDocument returns standard base64, or change the
implementation/test to produce URL-safe base64 and then assert with
encodeURIComponent; specifically modify the test in encrypt-decrypt.test.ts to
replace expect(encodeURI(encoded)).toBe(encoded) with an assertion that encoded
matches the chosen regex or call encodeURIComponent(encoded) when switching to
URL-safe base64, referencing the encodeDocument function and the test case name
'encodeDocument should return url safe characters only'.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/__tests__/open-attestation/encrypt-decrypt.test.ts`:
- Around line 52-65: The test currently uses a hardcoded hex encryption key
literal which triggers secret scanning; replace the literal with a
runtime-generated key (call generateEncryptionKey() or crypto.randomBytes(32)
and hex-encode it) and use that variable for both the
encryptString(encryptionKey) call and the final equality assertion; update the
two occurrences (the test at lines ~52-65 and the duplicate at ~89-93) to remove
the literal and assert encryptionResults.key === generatedKey instead, keeping
the other expectations (cipherText, iv, tag, type) unchanged.
- Around line 12-15: The base64Regex includes an invalid padding branch
(`{1}===`) and should be corrected to only allow valid padding lengths, so
update base64Regex to remove the `{1}===` branch (ensure it matches groups of 4
with optional `3=` or `2==` endings only); also replace the dynamic RegExp
construction for encryptionKeyRegex with a literal that matches 64 hex chars
(since ENCRYPTION_PARAMETERS.keyLength is fixed to 256 bits) by changing
encryptionKeyRegex to a literal like /^[0-9a-f]{64}$/ to eliminate the warning
and hardcode the expected key length.
- Around line 124-127: The test incorrectly uses encodeURI to assert URL-safe
base64; update the test for encodeDocument to assert the actual expected
encoding format: either (preferred) validate the output against a standard
base64 regex (e.g., base64Regex) if encodeDocument returns standard base64, or
change the implementation/test to produce URL-safe base64 and then assert with
encodeURIComponent; specifically modify the test in encrypt-decrypt.test.ts to
replace expect(encodeURI(encoded)).toBe(encoded) with an assertion that encoded
matches the chosen regex or call encodeURIComponent(encoded) when switching to
URL-safe base64, referencing the encodeDocument function and the test case name
'encodeDocument should return url safe characters only'.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between da6cbd5 and 64a4f0b.

📒 Files selected for processing (1)
  • src/__tests__/open-attestation/encrypt-decrypt.test.ts

Copy link
Copy Markdown
Contributor

@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: 1

🤖 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`:
- Line 13: The base64Regex currently allows the empty string because it uses *
for the leading quartet group; update the base64Regex constant (base64Regex) so
it requires at least one base64 quartet (e.g., change the repeated group from *
to + or otherwise assert non-empty) and keep the optional padding branch for
final 2–3 char fragments; ensure cipherText/iv/tag assertions use the updated
base64Regex so empty strings are rejected.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 64a4f0b and fa97d1c.

📒 Files selected for processing (2)
  • src/__tests__/open-attestation/encrypt-decrypt.test.ts
  • src/open-attestation/utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/open-attestation/utils.ts

Comment thread src/__tests__/open-attestation/encrypt-decrypt.test.ts Outdated
Copy link
Copy Markdown
Contributor

@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

🧹 Nitpick comments (1)
src/open-attestation/utils.ts (1)

56-62: Consider normalizing decode failures with a domain-specific error.

Right now malformed input will bubble low-level decode errors. Wrapping it provides a stable API contract for callers.

♻️ Proposed refactor
 export const decodeDocument = (encoded: string): string => {
-  let normalized = encoded.replace(/-/g, '+').replace(/_/g, '/');
-  const pad = normalized.length % 4;
-  if (pad) normalized += '='.repeat(4 - pad);
-  const decoded = forge.util.decode64(normalized);
-  return forge.util.decodeUtf8(decoded);
+  try {
+    let normalized = encoded.replace(/-/g, '+').replace(/_/g, '/');
+    const pad = normalized.length % 4;
+    if (pad) normalized += '='.repeat(4 - pad);
+    const decoded = forge.util.decode64(normalized);
+    return forge.util.decodeUtf8(decoded);
+  } catch {
+    throw new Error('Invalid base64/base64url document payload');
+  }
 };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/open-attestation/utils.ts` around lines 56 - 62, The decodeDocument
helper currently lets low-level forge decode errors propagate; wrap the body of
decodeDocument in a try/catch, catch any exception from forge.util.decode64 or
forge.util.decodeUtf8 and throw a domain-specific error (e.g., new
Error("DocumentDecodeError: failed to decode document") or a custom
DocumentDecodeError) that includes a brief context message and, if possible, the
original error as a cause or included detail; this keeps the API contract stable
while preserving original error info for debugging.
🤖 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/open-attestation/utils.ts`:
- Around line 16-22: ENCRYPTION_PARAMETERS is exported as a regular mutable
object; make it immutable by freezing it before export so downstream code cannot
mutate crypto settings. Update the declaration of ENCRYPTION_PARAMETERS in
src/open-attestation/utils.ts to create the same object but call Object.freeze
(and freeze any nested objects if present) and preserve the existing types
(e.g., keep the 'as const' or use a readonly type assertion) so the exported
symbol ENCRYPTION_PARAMETERS is deeply immutable at runtime while retaining its
type information.
- Around line 29-34: Validate the keyLengthInBits parameter in
generateEncryptionKey: ensure it's an integer and one of the supported AES key
sizes (e.g., 128, 192, 256) before calling forge.random.getBytesSync; if
invalid, throw a clear RangeError (or fallback to
ENCRYPTION_PARAMETERS.keyLength) with a descriptive message referencing the
expected values, and update any callers that rely on generateEncryptionKey to
handle the thrown error as needed; reference the generateEncryptionKey function
and ENCRYPTION_PARAMETERS.keyLength to locate the change.

---

Nitpick comments:
In `@src/open-attestation/utils.ts`:
- Around line 56-62: The decodeDocument helper currently lets low-level forge
decode errors propagate; wrap the body of decodeDocument in a try/catch, catch
any exception from forge.util.decode64 or forge.util.decodeUtf8 and throw a
domain-specific error (e.g., new Error("DocumentDecodeError: failed to decode
document") or a custom DocumentDecodeError) that includes a brief context
message and, if possible, the original error as a cause or included detail; this
keeps the API contract stable while preserving original error info for
debugging.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fa97d1c and 053b419.

📒 Files selected for processing (1)
  • src/open-attestation/utils.ts

Comment thread src/open-attestation/utils.ts Outdated
Comment on lines +16 to +22
export const ENCRYPTION_PARAMETERS = {
algorithm: 'AES-GCM' as const,
keyLength: 256, // Key length in bits
ivLength: 96, // IV length in bits: NIST suggests 12 bytes
tagLength: 128, // GCM authentication tag length in bits, see link above for explanation
version: 'OPEN-ATTESTATION-TYPE-1', // Type 1 using the above params without compression
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Freeze exported encryption parameters to prevent runtime mutation.

ENCRYPTION_PARAMETERS is exported as a mutable object. Any downstream mutation can silently alter crypto behavior at runtime.

🔒 Proposed fix
-export const ENCRYPTION_PARAMETERS = {
-  algorithm: 'AES-GCM' as const,
+export const ENCRYPTION_PARAMETERS = Object.freeze({
+  algorithm: 'AES-GCM',
   keyLength: 256, // Key length in bits
   ivLength: 96, // IV length in bits: NIST suggests 12 bytes
   tagLength: 128, // GCM authentication tag length in bits, see link above for explanation
   version: 'OPEN-ATTESTATION-TYPE-1', // Type 1 using the above params without compression
-};
+} as const);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/open-attestation/utils.ts` around lines 16 - 22, ENCRYPTION_PARAMETERS is
exported as a regular mutable object; make it immutable by freezing it before
export so downstream code cannot mutate crypto settings. Update the declaration
of ENCRYPTION_PARAMETERS in src/open-attestation/utils.ts to create the same
object but call Object.freeze (and freeze any nested objects if present) and
preserve the existing types (e.g., keep the 'as const' or use a readonly type
assertion) so the exported symbol ENCRYPTION_PARAMETERS is deeply immutable at
runtime while retaining its type information.

Comment thread src/open-attestation/utils.ts
manishdex25 and others added 5 commits February 26, 2026 08:05
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…:TrustVC/trustvc into feat/TT-907-encrypt-decrypt-oa-functions
@sonarqubecloud
Copy link
Copy Markdown

@manishdex25 manishdex25 self-assigned this Feb 26, 2026
@rongquan1 rongquan1 merged commit 5949581 into main Feb 26, 2026
13 checks passed
@rongquan1 rongquan1 deleted the feat/TT-907-encrypt-decrypt-oa-functions branch February 26, 2026 02:52
nghaninn pushed a commit that referenced this pull request Feb 26, 2026
## [2.10.0](v2.9.1...v2.10.0) (2026-02-26)

### Features

* added encrypt and decrypt oa functionlities ([#131](#131)) ([5949581](5949581))
@tradetrustimda
Copy link
Copy Markdown

🎉 This PR is included in version 2.10.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants