Skip to content

feat: Implement Type Metadata utility functions#33

Merged
vijayshiyani merged 8 commits intomainfrom
feat/type-metadata-handling
May 30, 2025
Merged

feat: Implement Type Metadata utility functions#33
vijayshiyani merged 8 commits intomainfrom
feat/type-metadata-handling

Conversation

@vijayshiyani
Copy link
Contributor

@vijayshiyani vijayshiyani commented May 28, 2025

  • extractEmbeddedTypeMetadata() utility function to extract and decode Type Metadata documents embedded in the vctm unprotected header of SD-JWT VCs
  • fetchTypeMetadataFromUrl() utility function to fetch and optionally verify Type Metadata from URLs specified in the vct claim, with integrity validation support

Copy link

@Matthew-Meeco Matthew-Meeco left a comment

Choose a reason for hiding this comment

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

Seems fine, just a few comments to review please.

@vijayshiyani vijayshiyani marked this pull request as ready for review May 30, 2025 04:12
…h errors

- Remove algorithmPrefixes from fetchTypeMetadataFromUrl options
- Throw SDJWTVCError instead of console.warn for better error handling
- Add validation for algorithm prefix in vct#integrity claims
- Update tests to reflect new error-throwing behavior
- Remove related algorithmPrefixes tests
Copy link

@Matthew-Meeco Matthew-Meeco left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the errors

@vijayshiyani vijayshiyani merged commit 9488b88 into main May 30, 2025
2 checks passed
@leikind
Copy link

leikind commented Jun 26, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Error Handling Consistency

The extractEmbeddedTypeMetadata and fetchTypeMetadataFromUrl functions introduce new error handling logic and custom error messages. The reviewer should verify that all error cases are handled as intended, that error messages are clear and actionable, and that no unexpected exceptions can leak through, especially in edge cases (e.g., malformed headers, network failures, or integrity check mismatches).

export function extractEmbeddedTypeMetadata(sdJwtVC: JWT): TypeMetadata[] | null {
  const parts = sdJwtVC.split(SD_JWT_FORMAT_SEPARATOR);
  const jws = parts[0];

  try {
    const protectedHeader = decodeProtectedHeader(jws);

    // Check if header is valid and has vctm
    if (!protectedHeader || typeof protectedHeader !== 'object') {
      return null;
    }

    const vctm = (protectedHeader as any).vctm;
    if (!vctm) {
      return null;
    }

    if (!Array.isArray(vctm)) {
      throw new SDJWTVCError('vctm in unprotected header must be an array');
    }

    return vctm.map((doc: string) => {
      try {
        return JSON.parse(base64decode(doc)) as TypeMetadata;
      } catch (_: any) {
        throw new SDJWTVCError(`Failed to decode base64url vctm entry: ${doc}. Error: Invalid base64url string`);
      }
    });
  } catch (e: any) {
    // Re-throw specific errors, treat other errors as 'not present'
    if (e instanceof SDJWTVCError) {
      throw e;
    }
    return null;
  }
}

/**
 * Fetches and optionally verifies Type Metadata from a URL specified in the vct claim.
 * @param sdJwtPayload The decoded SD-JWT payload.
 * @param options Optional parameters, including a hasher for integrity checking.
 * @returns A Promise that resolves to the TypeMetadata object, or null if not found or invalid.
 * @throws An error if integrity check fails or if fetching/parsing encounters critical issues.
 */
export async function fetchTypeMetadataFromUrl(
  sdJwtPayload: SDJWTPayload,
  options?: { hasher?: SDJWTHasher },
): Promise<TypeMetadata | null> {
  const DEFAULT_ALGORITHM_PREFIXES = ['sha256-', 'sha384-', 'sha512-']; // as per https://www.w3.org/TR/sri-2/#integrity-metadata-description

  const vct = sdJwtPayload.vct;

  if (typeof vct !== 'string' || !vct.startsWith('https://') || !isValidUrl(vct)) {
    // vct is not a string or not an HTTPS URL, so no metadata to fetch from here.
    return null;
  }

  try {
    const response = await fetch(vct);
    if (!response.ok) {
      throw new SDJWTVCError(`Failed to fetch Type Metadata from ${vct}: ${response.status} ${response.statusText}`);
    }

    const rawContent = await response.text();

    const integrityClaimValue = sdJwtPayload['vct#integrity'] as string | undefined;

    if (integrityClaimValue && options?.hasher) {
      const calculatedHash = await Promise.resolve(options.hasher(rawContent));

      // Extract hash from integrity claim, handling algorithm prefixes properly
      const matchingPrefix = DEFAULT_ALGORITHM_PREFIXES.find((prefix) => integrityClaimValue.startsWith(prefix));

      if (!matchingPrefix) {
        throw new SDJWTVCError(
          `Invalid algorithm prefix in vct#integrity claim: ${integrityClaimValue}. Expected one of: ${DEFAULT_ALGORITHM_PREFIXES.join(', ')}`,
        );
      }

      const expectedHash = integrityClaimValue.substring(matchingPrefix.length);

      if (calculatedHash !== expectedHash) {
        throw new SDJWTVCError(
          `Type Metadata integrity check failed for ${vct}. Expected hash ${expectedHash} (derived from ${integrityClaimValue}), got ${calculatedHash}.`,
        );
      }
    }

    try {
      const typeMetadata = JSON.parse(rawContent);
      return typeMetadata as TypeMetadata;
    } catch (parseError: any) {
      throw new SDJWTVCError(`Failed to parse Type Metadata from ${vct} as JSON: ${parseError.message}`);
    }
  } catch (error: any) {
    if (error instanceof SDJWTVCError) {
      throw error;
    }
    throw new SDJWTVCError(`Error fetching Type Metadata from ${vct}: ${error.message}`);
  }
}
Integrity Check Algorithm Prefixes

The integrity check in fetchTypeMetadataFromUrl only accepts certain hash algorithm prefixes (sha256-, sha384-, sha512-). The reviewer should confirm that this restriction is appropriate for all intended use cases and that it does not inadvertently reject valid integrity claims that may use other algorithms or formats in the future.

export async function fetchTypeMetadataFromUrl(
  sdJwtPayload: SDJWTPayload,
  options?: { hasher?: SDJWTHasher },
): Promise<TypeMetadata | null> {
  const DEFAULT_ALGORITHM_PREFIXES = ['sha256-', 'sha384-', 'sha512-']; // as per https://www.w3.org/TR/sri-2/#integrity-metadata-description

  const vct = sdJwtPayload.vct;

  if (typeof vct !== 'string' || !vct.startsWith('https://') || !isValidUrl(vct)) {
    // vct is not a string or not an HTTPS URL, so no metadata to fetch from here.
    return null;
  }

  try {
    const response = await fetch(vct);
    if (!response.ok) {
      throw new SDJWTVCError(`Failed to fetch Type Metadata from ${vct}: ${response.status} ${response.statusText}`);
    }

    const rawContent = await response.text();

    const integrityClaimValue = sdJwtPayload['vct#integrity'] as string | undefined;

    if (integrityClaimValue && options?.hasher) {
      const calculatedHash = await Promise.resolve(options.hasher(rawContent));

      // Extract hash from integrity claim, handling algorithm prefixes properly
      const matchingPrefix = DEFAULT_ALGORITHM_PREFIXES.find((prefix) => integrityClaimValue.startsWith(prefix));

      if (!matchingPrefix) {
        throw new SDJWTVCError(
          `Invalid algorithm prefix in vct#integrity claim: ${integrityClaimValue}. Expected one of: ${DEFAULT_ALGORITHM_PREFIXES.join(', ')}`,
        );
      }

      const expectedHash = integrityClaimValue.substring(matchingPrefix.length);

      if (calculatedHash !== expectedHash) {
        throw new SDJWTVCError(
          `Type Metadata integrity check failed for ${vct}. Expected hash ${expectedHash} (derived from ${integrityClaimValue}), got ${calculatedHash}.`,
        );
      }
    }

    try {
      const typeMetadata = JSON.parse(rawContent);
      return typeMetadata as TypeMetadata;
    } catch (parseError: any) {
      throw new SDJWTVCError(`Failed to parse Type Metadata from ${vct} as JSON: ${parseError.message}`);
    }
  } catch (error: any) {
    if (error instanceof SDJWTVCError) {
      throw error;
    }
    throw new SDJWTVCError(`Error fetching Type Metadata from ${vct}: ${error.message}`);
  }
}
TypeMetadata Interface Flexibility

The TypeMetadata interface allows arbitrary additional properties via an index signature. The reviewer should ensure that this flexibility is intentional and does not introduce ambiguity or weaken type safety for consumers of this interface.

export interface TypeMetadata {
  vct?: string;
  name?: string;
  description?: string;
  extends?: string; // URI
  display?: Array<Record<string, any>>;
  claims?: Array<Record<string, any>>;
  /**
   * OPTIONAL. An embedded JSON Schema document describing the structure of the Verifiable Credential.
   * MUST NOT be used if schema_uri is present.
   */
  schema?: Record<string, any>;
  /**
   * OPTIONAL. A URL pointing to a JSON Schema document.
   * MUST NOT be used if schema is present.
   */
  schema_uri?: string;

  /**
   * OPTIONAL. integrity metadata for vct, extends, schema_uri, and similar URIs.
   * Value MUST be an "integrity metadata" string per W3C.SRI.
   */
  'schema_uri#integrity'?: string;
  'vct#integrity'?: string;
  'extends#integrity'?: string;

  [key: string]: any;
}

@Meeco Meeco deleted a comment from QodoAI-Agent Jun 27, 2025
@leikind
Copy link

leikind commented Jun 27, 2025

===

@leikind
Copy link

leikind commented Jun 27, 2025

PR Code Suggestions ✨

Latest suggestions up to 0197b6f

CategorySuggestion                                                                                                                                    Impact
Possible issue
Extract vctm from unprotected header

The function is extracting vctm from the protected header, but according to the
SD-JWT VC specification, vctm should be in the unprotected header. This will cause
the function to always return null for valid SD-JWT VCs with embedded type metadata.

src/util.ts [155-162]

-const protectedHeader = decodeProtectedHeader(jws);
-
-// Check if header is valid and has vctm
-if (!protectedHeader || typeof protectedHeader !== 'object') {
+// Parse the JWS header to get the unprotected header
+const jwsParts = jws.split('.');
+if (jwsParts.length < 2) {
   return null;
 }
 
-const vctm = (protectedHeader as any).vctm;
+let unprotectedHeader;
+try {
+  unprotectedHeader = JSON.parse(base64decode(jwsParts[0]));
+} catch {
+  return null;
+}
 
+// Check if header is valid and has vctm
+if (!unprotectedHeader || typeof unprotectedHeader !== 'object') {
+  return null;
+}
+
+const vctm = unprotectedHeader.vctm;
+
Suggestion importance[1-10]: 10

__

Why: This is a critical bug. The function is extracting vctm from the protected header using decodeProtectedHeader, but according to SD-JWT VC specification, vctm should be in the unprotected header. This will cause the function to always return null for valid SD-JWT VCs.

High
General
Improve error message accuracy

The error message assumes all decoding failures are due to invalid base64url, but
JSON parsing can also fail. The error message should be more accurate to help with
debugging.

src/util.ts [171-177]

 return vctm.map((doc: string) => {
   try {
-    return JSON.parse(base64decode(doc)) as TypeMetadata;
-  } catch (_: any) {
-    throw new SDJWTVCError(`Failed to decode base64url vctm entry: ${doc}. Error: Invalid base64url string`);
+    const decoded = base64decode(doc);
+    return JSON.parse(decoded) as TypeMetadata;
+  } catch (error: any) {
+    throw new SDJWTVCError(`Failed to decode base64url vctm entry: ${doc}. Error: ${error.message}`);
   }
 });
Suggestion importance[1-10]: 4

__

Why: The suggestion correctly identifies that the error message assumes all failures are base64url decoding issues, when JSON parsing can also fail. However, the impact is minor as it only affects error message clarity.

Low
Remove unnecessary Promise wrapping

The hasher function may not return a Promise, so wrapping it in Promise.resolve is
unnecessary and could mask synchronous errors. Call the hasher directly and handle
any thrown errors appropriately.

src/util.ts [218]

 if (integrityClaimValue && options?.hasher) {
-  const calculatedHash = await Promise.resolve(options.hasher(rawContent));
+  const calculatedHash = options.hasher(rawContent);
Suggestion importance[1-10]: 3

__

Why: The Promise.resolve wrapper is unnecessary since the hasher function is likely synchronous. However, this is a minor optimization that doesn't affect functionality significantly.

Low

Previous suggestions

Suggestions up to commit 0197b6f
CategorySuggestion                                                                                                                                    Impact
Possible issue
Quote reserved keyword property

Escape the reserved keyword extends by quoting it as a string property key to avoid
TypeScript syntax errors.

src/types.ts [89]

-extends?: string; // URI
+'extends'?: string; // URI
Suggestion importance[1-10]: 8

__

Why: The property extends is a reserved TypeScript keyword and must be quoted ('extends') to avoid syntax errors.

Medium
Validate URL with URL constructor

Replace the undefined isValidUrl utility with a built-in URL constructor check to
validate the vct string and ensure HTTPS protocol.

src/util.ts [202-205]

-if (typeof vct !== 'string' || !vct.startsWith('https://') || !isValidUrl(vct)) {
-  // vct is not a string or not an HTTPS URL, so no metadata to fetch from here.
+try {
+  const url = new URL(vct);
+  if (url.protocol !== 'https:') {
+    return null;
+  }
+} catch {
   return null;
 }
Suggestion importance[1-10]: 7

__

Why: The undefined isValidUrl would cause a compile error; using the URL constructor covers both type and protocol validation without extra imports.

Medium
General
Distinguish decode and parse errors

Separate the base64url decoding step from JSON parsing to provide distinct error
messages for decoding and parsing failures.

src/util.ts [172-178]

 return vctm.map((doc: string) => {
+  let decoded: string;
   try {
-    return JSON.parse(base64decode(doc)) as TypeMetadata;
-  } catch (_: any) {
-    throw new SDJWTVCError(`Failed to decode base64url vctm entry: ${doc}. Error: Invalid base64url string`);
+    decoded = base64decode(doc);
+  } catch (e: any) {
+    throw new SDJWTVCError(`Failed to decode base64url vctm entry: ${doc}. Error: ${e.message}`);
+  }
+  try {
+    return JSON.parse(decoded) as TypeMetadata;
+  } catch (e: any) {
+    throw new SDJWTVCError(`Failed to parse JSON for vctm entry: ${doc}. Error: ${e.message}`);
   }
 });
Suggestion importance[1-10]: 5

__

Why: Separating base64 decoding from JSON parsing yields clearer, specific error messages and improves debuggability.

Low
Suggestions up to commit 0197b6f
CategorySuggestion                                                                                                                                    Impact
Possible issue
Inline URL validation

Replace the undefined isValidUrl check with a URL constructor validation. This
ensures the code validates vct as a well-formed HTTPS URL without relying on an
external helper.

src/util.ts [202-204]

-if (typeof vct !== 'string' || !vct.startsWith('https://') || !isValidUrl(vct)) {
+if (typeof vct !== 'string' || !vct.startsWith('https://')) {
+  return null;
+}
+try {
+  new URL(vct);
+} catch {
   return null;
 }
Suggestion importance[1-10]: 9

__

Why: The code references isValidUrl which isn't imported causing a potential runtime error; using the built-in URL constructor ensures proper HTTPS URL validation.

High
Improve base64url decoding

Use Buffer.from(..., 'base64url') for proper base64url decoding and include the
actual error message in the exception. This prevents failures on valid base64url
inputs and improves debugging.

src/util.ts [171-177]

 return vctm.map((doc: string) => {
   try {
-    return JSON.parse(base64decode(doc)) as TypeMetadata;
-  } catch (_: any) {
-    throw new SDJWTVCError(`Failed to decode base64url vctm entry: ${doc}. Error: Invalid base64url string`);
+    const decoded = Buffer.from(doc, 'base64url').toString('utf-8');
+    return JSON.parse(decoded) as TypeMetadata;
+  } catch (e: any) {
+    throw new SDJWTVCError(`Failed to decode base64url vctm entry: ${doc}. Error: ${e.message}`);
   }
 });
Suggestion importance[1-10]: 6

__

Why: Using Buffer.from(..., 'base64url') guarantees correct base64url decoding and surfacing the actual error message aids debugging.

Low
Suggestions up to commit 0197b6f
CategorySuggestion                                                                                                                                    Impact
Possible issue
Use URL constructor for validation

Remove the undefined isValidUrl dependency and instead validate the URL using the
built-in URL constructor. This prevents runtime errors and correctly identifies
invalid URLs.

src/util.ts [202-205]

-if (typeof vct !== 'string' || !vct.startsWith('https://') || !isValidUrl(vct)) {
-  // vct is not a string or not an HTTPS URL, so no metadata to fetch from here.
+if (typeof vct !== 'string' || !vct.startsWith('https://')) {
+  return null;
+}
+try {
+  new URL(vct);
+} catch {
   return null;
 }
Suggestion importance[1-10]: 9

__

Why: The code references an undefined isValidUrl, causing a runtime error; validating with new URL(vct) is a correct, built-in replacement that fixes this critical bug.

High
Manual header decode

Replace the call to decodeProtectedHeader, which expects a full JWS, with a manual
base64url decode and JSON.parse of the header. This ensures the vctm field is
reliably extracted without throwing on incomplete inputs.

src/util.ts [155-160]

-const protectedHeader = decodeProtectedHeader(jws);
-
-// Check if header is valid and has vctm
-if (!protectedHeader || typeof protectedHeader !== 'object') {
+let headerObj: any;
+try {
+  headerObj = JSON.parse(base64decode(jws));
+} catch {
   return null;
 }
 
+// Check if header is valid and has vctm
+if (!headerObj || typeof headerObj !== 'object') {
+  return null;
+}
+
Suggestion importance[1-10]: 2

__

Why: Replacing decodeProtectedHeader is unnecessary and risks losing the advantages of a well-tested library parser, as the manual decode duplicates behavior and may break future header parsing logic.

Low

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants