Skip to content

Comments

Task/mpd 5521 check for invalid claims#31

Merged
Matthew-Meeco merged 14 commits intomainfrom
task/MPD-5521-check-for-invalid-claims
May 28, 2025
Merged

Task/mpd 5521 check for invalid claims#31
Matthew-Meeco merged 14 commits intomainfrom
task/MPD-5521-check-for-invalid-claims

Conversation

@Matthew-Meeco
Copy link
Contributor

Addresses the following:

  1. The claim name, or key, as it would be used in a regular JWT payload. It MUST be a string and MUST NOT be _sd, ...,
  2. or a claim name existing in the object as a permanently disclosed claim. (Already handled by existing code)
  3. The same digest value MUST NOT appear more than once in the SD-JWT.

I modified the decoy tests to use a unique salt, as they were failing due to duplicate digests using the static salt.

@Matthew-Meeco Matthew-Meeco merged commit 20fb841 into main May 28, 2025
2 checks passed
@Matthew-Meeco Matthew-Meeco deleted the task/MPD-5521-check-for-invalid-claims branch May 28, 2025 06:48
@leikind
Copy link

leikind commented Jun 26, 2025

@CodiumAI-Agent /improve

@QodoAI-Agent
Copy link

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix disclosure key null check

The check for an invalid array element disclosure incorrectly treats undefined keys
as valid and non-null, causing valid disclosures to be rejected. Use a loose null
check to catch both null and undefined for disclosed.key.

src/helpers.ts [111-113]

-if (disclosed && disclosed.key !== null) {
+if (disclosed && disclosed.key != null) {
   throw new UnpackSDJWTError(`Invalid disclosure format for array element: expected 2 elements (salt, value)`);
 }
Suggestion importance[1-10]: 8

__

Why: The existing disclosed.key !== null check also rejects undefined keys, causing valid array disclosures to fail. Using != null ensures only actual keys (neither null nor undefined) trigger the error, restoring correct unpack behavior.

Medium
Use raw string for hashing

The test computes the digest over the Base64‐encoded JSON instead of the raw JSON
string, which does not match production logic. Hash the raw disclosure JSON
(rawDisclosureString) to produce the correct digest.

src/common.spec.ts [70]

-const conflictingDigest = testHasher(encodedDisclosureString);
+const conflictingDigest = testHasher(rawDisclosureString);
Suggestion importance[1-10]: 7

__

Why: The test currently hashes the Base64‐encoded disclosure string, but production logic hashes the raw JSON (rawDisclosureString). Hashing the raw JSON aligns the test with implementation and avoids mismatched digests.

Medium

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.

4 participants