Skip to content

feat: credentials sign & verify#384

Merged
sagarkhole4 merged 6 commits into
mainfrom
feat/credentials-sign-verify
May 13, 2026
Merged

feat: credentials sign & verify#384
sagarkhole4 merged 6 commits into
mainfrom
feat/credentials-sign-verify

Conversation

@sagarkhole4
Copy link
Copy Markdown
Contributor

@sagarkhole4 sagarkhole4 commented May 13, 2026

Summary by CodeRabbit

  • New Features
    • Added signature verification endpoint to validate signatures using public key credentials in multiple formats
    • Added credential signing endpoint supporting both JSON-LD credentials and raw data signing with optional storage capabilities
    • Enhanced credential handling with improved context and subject property support

Review Change Stack

@sagarkhole4 sagarkhole4 requested a review from sairanjit May 13, 2026 11:57
@sagarkhole4 sagarkhole4 self-assigned this May 13, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

Warning

Rate limit exceeded

@sagarkhole4 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 21 minutes and 5 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f9d906cc-7d94-4857-ad32-070039ac5edb

📥 Commits

Reviewing files that changed from the base of the PR and between 57bc7c9 and fcf9592.

📒 Files selected for processing (2)
  • src/controllers/agent/AgentController.ts
  • src/controllers/types.ts
📝 Walkthrough

Walkthrough

This PR adds two new endpoints to the Agent API: POST /agent/verify for signature verification and POST /agent/credential/sign for credential signing (JSON-LD or raw data). Supporting utilities include algorithm mapping, type constraints for credential structures, updated route registration, and OpenAPI documentation.

Changes

Verify and Sign Credential Endpoints

Layer / File(s) Summary
Supporting constants and type definitions
src/utils/constant.ts, src/controllers/types.ts
ALGORITHM_MAP maps cryptographic key types (ed25519, p256, secp256k1) to KMS algorithm names. CustomW3cJsonLdSignCredentialOptions constrains credential structure to require @context and credentialSubject. RegisterSchemaReturn union members are reformatted for readability.
Verify and credential sign endpoints
src/controllers/agent/AgentController.ts
verify endpoint validates signatures by converting publicKeyBase58 to JWK, mapping key type to KMS algorithm, and calling kms.verify. signCredential endpoint routes to JSON-LD or raw-data signing: JSON-LD signing normalizes options, forces ClaimFormat.LdpVc format, and optionally stores the credential; raw-data signing validates inputs, resolves kmsKeyId from DID records or documents with fallback derivation, and returns base64-encoded signature.
Route models and registration
src/routes/routes.ts
W3C credential schemas updated to use @context property. VerifyDataOptions and W3C JSON credential schemas added to route models. POST /agent/verify and POST /agent/credential/sign endpoints registered as protected Express routes with JWT authentication and parameter/body validation.
OpenAPI specification
src/routes/swagger.json
VerifyDataOptions and W3C-JWT credential schemas (W3cCredentialRecord, W3cJsonIssuer, W3cJsonCredentialSubject, SingleOrArray_W3cJsonCredentialSubject_, W3cJsonCredential) added to spec. W3C credential context field names updated from context to @context. POST /agent/verify and POST /agent/credential/sign endpoints documented with request and response schemas.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • credebl/agent-controller#368: Deactivates the same POST /agent/verify and POST /agent/credential/sign endpoints that this PR activates, creating potential conflict on the controller method implementations.
  • credebl/agent-controller#350: Updates RegisterSchemaReturn type in src/controllers/types.ts, overlapping with this PR's reformatting of the same union members.

Suggested labels

enhancement

Suggested reviewers

  • sairanjit
  • GHkrishna
  • RinkalBhojani

Poem

🐰 Two new paths emerge from the Agent's heart,
Verify signatures, credentials start,
With KMS keys and JSON-LD grace,
Raw data signs and finds its place,
The docs and routes now dance as one! 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: credentials sign & verify' directly and concisely captures the main changes: adding credential signing and verification endpoints to the AgentController.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/credentials-sign-verify

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

@ajile-in ajile-in changed the title Feat/credentials sign verify feat: credentials sign verify May 13, 2026
@ajile-in ajile-in changed the title feat: credentials sign verify feat: credentials sign & verify May 13, 2026
Copy link
Copy Markdown

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

🧹 Nitpick comments (1)
src/controllers/agent/AgentController.ts (1)

87-91: ⚡ Quick win

Extract the duplicated algorithmMap into a module-level constant.

The same key‑type → algorithm mapping appears verbatim in verify (lines 87‑91) and signCredential (lines 207‑211). Lift it to a file/module constant (or shared util) so the two endpoints can’t drift, and so adding a new key type is a one-line change.

const KEY_TYPE_TO_KMS_ALGORITHM: Record<string, string> = {
  ed25519: 'EdDSA',
  p256: 'ES256',
  secp256k1: 'ES256K',
}

Also applies to: 207-211

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/controllers/agent/AgentController.ts` around lines 87 - 91, Extract the
duplicated algorithmMap into a shared module-level constant (e.g.,
KEY_TYPE_TO_KMS_ALGORITHM) and replace the inline algorithmMap usages in both
the verify and signCredential functions with that constant; specifically create
a top-level exportable constant named KEY_TYPE_TO_KMS_ALGORITHM with the
ed25519/p256/secp256k1 mappings and update the references in
AgentController.verify and AgentController.signCredential to use
KEY_TYPE_TO_KMS_ALGORITHM so the mapping is defined in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cliAgent.ts`:
- Line 10: Restore the external imports for PolygonDidRegistrar,
PolygonDidResolver, and PolygonModule to the top-level import block (remove the
commented-out import) and delete the duplicate active import currently placed
later in the file; ensure all external package imports (including
PolygonDidRegistrar/PolygonDidResolver/PolygonModule) are grouped together
before local utility imports to satisfy the import/order rule and remove
dead/commented code.

In `@src/controllers/agent/AgentController.ts`:
- Around line 117-225: The signCredential method is too complex and uses `@Body`()
data: any plus a string union that collapses; refactor by extracting the JSON-LD
path into a private signJsonLd(request: Req, body:
CustomW3cJsonLdSignCredentialOptions, store: boolean) method, the raw-data path
into a private signRawData(request: Req, body: SignDataOptions) method, and the
kms resolution logic into a private resolveKmsKeyId(request: Req, body:
SignDataOptions): Promise<string> (move the dids.getCreatedDids/resolve and
fallback kmsKeyId logic there, keeping the getKmsKeyIdForVerifiacationMethod
call), then change the controller signature to use `@Query`('dataTypeToSign')
dataTypeToSign: 'rawData' | 'jsonLd' and `@Body`() body:
CustomW3cJsonLdSignCredentialOptions | SignDataOptions (a discriminated union)
and have the top-level signCredential simply dispatch to signJsonLd or
signRawData based on dataTypeToSign; preserve logging and error handling
unchanged.
- Around line 145-149: The handler currently returns a stored record when
storeCredential is true but a credential JSON otherwise, breaking the response
contract; change the code so it always returns the signed credential JSON (use
signedCredential.toJson()) and, if storeCredential is true, additionally include
the stored record id. Concretely: build the W3cCredentialRecord via
W3cCredentialRecord.fromCredential(signedCredential), call
request.agent.w3cCredentials.store({ record }), extract the resulting record id
(from the returned stored record or from the record object), and return a single
object such as { credential: signedCredential.toJson(), recordId: <id> } so both
paths share the same shape.
- Around line 1-27: There are two imports from '@credo-ts/core' and an unused
symbol; consolidate the two import statements into a single import from
'@credo-ts/core' in AgentController.ts, include all actually used symbols
(JsonTransformer, W3cJsonLdVerifiableCredential, TypedArrayEncoder, ClaimFormat,
W3cCredentialRecord, DidDocument, verkeyToPublicJwk,
getKmsKeyIdForVerifiacationMethod) and remove the unused
CustomW3cJsonLdSignCredentialOptions from the imported types list so the file
has one clean import for all required symbols.
- Around line 187-205: The fallback chain is assigning invalid identifiers (raw
public keys or DID fragments) to kmsKeyId which will fail when calling kms.sign;
change the logic in AgentController so that if
getKmsKeyIdForVerifiacationMethod(verificationMethod) returns undefined you
throw a BadRequestError (do not fall back to publicKeyBase58, idPart, or vmId),
and for the branch that currently sets kmsKeyId = rawData.publicKeyBase58
require a valid stored KMS keyId instead (either look up
didRecord.keys[0].kmsKeyId or call getKmsKeyIdForDid to map the verification
method to a stored kms key id) so kms.sign always receives a real KMS keyId.
- Line 19: The import getKmsKeyIdForVerifiacationMethod is invalid and must be
removed; replace the call to
getKmsKeyIdForVerifiacationMethod(verificationMethod) with one of the supported
patterns: 1) if you expect a KMS key ID from the agent, call the agent KMS API
(e.g., use agent.kms.* to look up or derive the key id from the verification
method) referencing the agent instance and verificationMethod variable, or 2) if
the key id is embedded in the JWK, extract verificationMethod.publicKeyJwk.kid
(or compute the kid per your JWK handling) and use that as derivedKeyId, or 3)
perform a DidRecord lookup to resolve the key and obtain its kid via the
agent/did resolver; remove the bad import line and update the code that assigns
derivedKeyId to use one of these supported approaches (use agent.kms methods,
the verificationMethod.publicKeyJwk.kid, or a did-record lookup depending on
intended behavior).

In `@src/routes/routes.ts`:
- Around line 4783-4788: The argsAgentController_signCredential schema currently
allows any string for dataTypeToSign which bypasses validation; update the
controller/type that defines dataTypeToSign to a strict union of the two
literals ("rawData" | "jsonLd") (remove the generic string alternative), then
regenerate the routes so argsAgentController_signCredential's dataTypeToSign
subSchemas only include the two enum values; locate the symbol
argsAgentController_signCredential and the controller method signCredential to
adjust the parameter type and re-run the codegen that emits
src/routes/routes.ts.

In `@src/routes/swagger.json`:
- Around line 9258-9262: The Swagger spec for the /agent/credential/sign
endpoint currently leaves the query parameter dataTypeToSign and the request
body schema untyped (empty {}); update the query param (dataTypeToSign) to an
enum of allowed values (e.g., "rawData", "jsonLd", and allow open string if
needed) and replace the empty request body schema with a proper component (e.g.,
SignCredentialRequest) that uses oneOf to discriminate between the JSON-LD
credential shape (reference existing JsonLdCredential) and the raw-data object
shape ({ data: string, publicKeyBase58?: string } with data required); ensure
the operation for /agent/credential/sign references this SignCredentialRequest
and marks the query param as required so the generated API matches the
controller behavior.
- Line 3253: Replace the empty object schema for VerifyDataOptions.keyType with
a reference to the existing KeyAlgorithm schema so the OpenAPI spec validates
allowed algorithms; locate the VerifyDataOptions definition (where "keyType": {}
appears) and change it to use a $ref to the KeyAlgorithm schema used elsewhere,
ensuring it matches the set of strings expected by AgentController.verifyData
(see usage around AgentController.ts line 103).

In `@src/utils/openbadges-context.json`:
- Line 4: The `@base` entry in openbadges-context.json is set to
"http://localhost/base/" and must be removed or replaced to avoid resolving
relative IRIs to localhost in production; edit the openbadges-context.json file
to either delete the "@base" key entirely (if you do not rely on relative IRIs)
or replace its value with your canonical production base URL (e.g.,
"https://example.org/base/"), then scan for uses of "@base" or any relative IRIs
in credential-generation code to ensure they resolve correctly and update any
tests or fixtures that expect the localhost base.
- Around line 8-72: The openbadges-context.json file contains invalid localhost
"@base" and incomplete/mismatched mappings (see keys "OpenBadgeCredential",
"Achievement", "awardedDate" etc.), and must be removed or replaced: either
delete this unused file or replace its contents with a direct reference to the
official OpenBadges 3.0.3 context URL
(https://purl.imsglobal.org/spec/ob/v3p0/context-3.0.3.json) and update any code
that imports this file to use the official URL; if you need extensions, create a
small context that composes the official context (no "@base":
"http://localhost/base/") and document the additions, ensuring date/numeric
types and container mappings follow the official schema.

---

Nitpick comments:
In `@src/controllers/agent/AgentController.ts`:
- Around line 87-91: Extract the duplicated algorithmMap into a shared
module-level constant (e.g., KEY_TYPE_TO_KMS_ALGORITHM) and replace the inline
algorithmMap usages in both the verify and signCredential functions with that
constant; specifically create a top-level exportable constant named
KEY_TYPE_TO_KMS_ALGORITHM with the ed25519/p256/secp256k1 mappings and update
the references in AgentController.verify and AgentController.signCredential to
use KEY_TYPE_TO_KMS_ALGORITHM so the mapping is defined in one place.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8bb6c27f-bd41-4a21-8ef4-e2ba27cf0adc

📥 Commits

Reviewing files that changed from the base of the PR and between ca1d777 and 14196a5.

📒 Files selected for processing (5)
  • src/cliAgent.ts
  • src/controllers/agent/AgentController.ts
  • src/routes/routes.ts
  • src/routes/swagger.json
  • src/utils/openbadges-context.json

Comment thread src/cliAgent.ts Outdated
import type { IndyVdrPoolConfig } from '@credo-ts/indy-vdr'

import { PolygonDidRegistrar, PolygonDidResolver, PolygonModule } from '@ayanworks/credo-polygon-w3c-module'
// import { PolygonDidRegistrar, PolygonDidResolver, PolygonModule } from '@ayanworks/credo-polygon-w3c-module'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Restore import to its original location and remove commented code.

The Polygon module import was moved from the top-level external imports (line 10, now commented) to line 87, which violates the ESLint import/order rule. External package imports should be grouped together at the top of the file, before local utility imports. The commented-out import at line 10 should also be removed to avoid leaving dead code.

📦 Proposed fix

Remove the commented import and move the active import back to the top:

-// import { PolygonDidRegistrar, PolygonDidResolver, PolygonModule } from '@ayanworks/credo-polygon-w3c-module'
+import { PolygonDidRegistrar, PolygonDidResolver, PolygonModule } from '@ayanworks/credo-polygon-w3c-module'
 import {
   AnonCredsDidCommCredentialFormatService,

And remove the duplicate import at line 87:

   getX509CertsByUrl,
 } from './utils/oid4vc-agent'
-import { PolygonDidRegistrar, PolygonDidResolver, PolygonModule } from '@ayanworks/credo-polygon-w3c-module'
 
 export type Transports = 'ws' | 'http'

Also applies to: 87-87

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cliAgent.ts` at line 10, Restore the external imports for
PolygonDidRegistrar, PolygonDidResolver, and PolygonModule to the top-level
import block (remove the commented-out import) and delete the duplicate active
import currently placed later in the file; ensure all external package imports
(including PolygonDidRegistrar/PolygonDidResolver/PolygonModule) are grouped
together before local utility imports to satisfy the import/order rule and
remove dead/commented code.

Comment thread src/controllers/agent/AgentController.ts Outdated
Comment thread src/controllers/agent/AgentController.ts Outdated
Comment thread src/controllers/agent/AgentController.ts Outdated
Comment thread src/controllers/agent/AgentController.ts
Comment thread src/routes/routes.ts
Comment thread src/routes/swagger.json
"publicKeyBase58": {
"type": "string"
},
"keyType": {},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify expected type for verify input in controller/routes
rg -n -C3 'VerifyDataOptions|/agent/verify|keyType|Verify\(' src/controllers src/routes

Repository: credebl/agent-controller

Length of output: 23581


🏁 Script executed:

rg -n "KeyAlgorithm|Curve" src/routes/swagger.json | head -30

Repository: credebl/agent-controller

Length of output: 213


🏁 Script executed:

sed -n '10,50p' src/routes/swagger.json

Repository: credebl/agent-controller

Length of output: 929


🏁 Script executed:

sed -n '3115,3135p' src/routes/swagger.json

Repository: credebl/agent-controller

Length of output: 393


Use a proper schema for keyType in VerifyDataOptions.

Line 3253 defines keyType as {}, which provides no validation for a security-critical field that determines the cryptographic algorithm. The controller expects a string matching one of the supported key algorithms (line 103 in AgentController.ts). Use a reference to the existing KeyAlgorithm schema instead.

Suggested patch
-					"keyType": {},
+					"keyType": {
+						"$ref": "#/components/schemas/KeyAlgorithm"
+					},
📝 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.

Suggested change
"keyType": {},
"keyType": {
"$ref": "#/components/schemas/KeyAlgorithm"
},
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/routes/swagger.json` at line 3253, Replace the empty object schema for
VerifyDataOptions.keyType with a reference to the existing KeyAlgorithm schema
so the OpenAPI spec validates allowed algorithms; locate the VerifyDataOptions
definition (where "keyType": {} appears) and change it to use a $ref to the
KeyAlgorithm schema used elsewhere, ensuring it matches the set of strings
expected by AgentController.verifyData (see usage around AgentController.ts line
103).

Comment thread src/routes/swagger.json
Comment thread src/utils/openbadges-context.json Outdated
Comment thread src/utils/openbadges-context.json Outdated
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
@sagarkhole4 sagarkhole4 force-pushed the feat/credentials-sign-verify branch from 287340f to 7847dbd Compare May 13, 2026 12:19
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
…oller

Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Copy link
Copy Markdown

@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

♻️ Duplicate comments (1)
src/controllers/agent/AgentController.ts (1)

178-231: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Do not treat a public key or DID fragment as a KMS keyId.

Line 186 returns publicKeyBase58 as the kmsKeyId, and Lines 221-226 fall back to publicKeyBase58 / #fragment / verificationMethod.id when lookup fails. kms.sign() can only use a stored private-key reference; a public key or DID fragment is not enough to sign. This makes the publicKeyBase58 + keyType path non-functional and turns lookup failures into late runtime errors instead of a clean 4xx.

♻️ Safer failure mode
   private async resolveKmsKeyId(request: Req, body: SignDataOptions): Promise<string> {
     const hasDidOrMethod = body.did || body.method
     const hasPublicKey = body.publicKeyBase58 && body.keyType
     if (!hasDidOrMethod && !hasPublicKey) {
       throw new BadRequestError('Either (did or method) OR (publicKeyBase58 and keyType) must be provided.')
     }

     if (!hasDidOrMethod) {
-      return body.publicKeyBase58
+      throw new BadRequestError('Raw-data signing requires a DID/method that resolves to a stored KMS key.')
     }
@@
-      const publicKeyBase58 = (verificationMethod as any).publicKeyBase58
-      const vmId = verificationMethod.id || ''
-      const idPart = vmId.includes('#') ? vmId.split('#')[1] : undefined
-
-      kmsKeyId = (derivedKeyId || publicKeyBase58 || idPart || vmId) as string
+      if (!derivedKeyId) {
+        throw new BadRequestError('Unable to resolve a stored kmsKeyId for the verification method.')
+      }
+      kmsKeyId = derivedKeyId

Verify that keyId is sourced from stored KMS identifiers elsewhere in the repo and that this method is the only path feeding publicKeyBase58 through unchanged. Expected result: other call sites resolve kmsKeyId from records/DID state, while this method is the outlier.

#!/bin/bash
set -euo pipefail

echo "=== Raw-data signing path ==="
sed -n '164,231p' src/controllers/agent/AgentController.ts

echo
echo "=== KMS sign call sites and key-id sources ==="
rg -nP --type=ts -C2 '\bkms\.sign\s*\(|\bkmsKeyId\b' src

echo
echo "=== DID-record based key-id lookups ==="
rg -nP --type=ts -C2 'didRecord\.keys\[[0-9]+\]\.kmsKeyId|getKmsKeyIdFor' src
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/controllers/agent/AgentController.ts` around lines 178 - 231,
resolveKmsKeyId currently treats a raw publicKeyBase58 or DID fragment as a KMS
keyId (e.g., returning body.publicKeyBase58 on no DID, and falling back to
publicKeyBase58/#fragment/verificationMethod.id), which is invalid for kms.sign;
change resolveKmsKeyId so it only returns actual stored KMS identifiers: if
body.publicKeyBase58+keyType is provided without a DID, reject with
BadRequestError; when a DID is provided, first try to read
didRecord.keys[].kmsKeyId and return it; if caller supplied publicKeyBase58,
match it against didRecord.keys[*].publicKeyBase58 (or equivalent) and return
the associated kmsKeyId; if no matching stored kmsKeyId can be found (including
after getKmsKeyIdForVerifiacationMethod), throw BadRequestError describing no
usable KMS keyId rather than returning a public key or fragment. Ensure
references in this function (resolveKmsKeyId, didRecord.keys,
getKmsKeyIdForVerifiacationMethod) are used to locate and implement these
checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/controllers/agent/AgentController.ts`:
- Around line 1-8: The import list in AgentController.ts is missing trailing
commas causing Prettier/Validate failures; update the import statements that
reference AgentInfo, AgentToken, SafeW3cJsonLdVerifyCredentialOptions,
CustomW3cJsonLdSignCredentialOptions, SignDataOptions, and VerifyDataOptions to
include the required trailing commas (and likewise fix the second import block
around lines 10-19) so the formatter passes—locate the import blocks and add
trailing commas after the last identifier on the affected lines.

In `@src/controllers/types.ts`:
- Around line 467-474: The new type alias CustomW3cJsonLdSignCredentialOptions
is misformatted and failing CI Prettier checks; run the project's Prettier
formatter (or your editor's format command) and reflow the declaration for
consistency with surrounding types (ensure proper spacing/commas, brace
placement, and consistent indentation for the nested credential:
Omit<W3cCredential, 'context' | 'credentialSubject'> block and the '@context'
and credentialSubject lines), and apply the same formatting fix to the related
type aliases referencing SingleOrArray and JsonObject so the file passes
lint/format checks.

In `@src/utils/constant.ts`:
- Around line 33-37: The ALGORITHM_MAP keys must match the lowercased keyType
produced by verify() and signRawData() (which yield values like "p-256" etc.),
so update ALGORITHM_MAP to include the hyphenated lowercase EC curve spellings
(e.g., "p-256": "ES256", "p-384": "ES384", "p-521": "ES512") and also add any
common alternates ("p256", "p384", "p521") if you want backward compatibility;
modify the ALGORITHM_MAP constant accordingly so lookups from
verify()/signRawData() return the correct JOSE algorithms.

---

Duplicate comments:
In `@src/controllers/agent/AgentController.ts`:
- Around line 178-231: resolveKmsKeyId currently treats a raw publicKeyBase58 or
DID fragment as a KMS keyId (e.g., returning body.publicKeyBase58 on no DID, and
falling back to publicKeyBase58/#fragment/verificationMethod.id), which is
invalid for kms.sign; change resolveKmsKeyId so it only returns actual stored
KMS identifiers: if body.publicKeyBase58+keyType is provided without a DID,
reject with BadRequestError; when a DID is provided, first try to read
didRecord.keys[].kmsKeyId and return it; if caller supplied publicKeyBase58,
match it against didRecord.keys[*].publicKeyBase58 (or equivalent) and return
the associated kmsKeyId; if no matching stored kmsKeyId can be found (including
after getKmsKeyIdForVerifiacationMethod), throw BadRequestError describing no
usable KMS keyId rather than returning a public key or fragment. Ensure
references in this function (resolveKmsKeyId, didRecord.keys,
getKmsKeyIdForVerifiacationMethod) are used to locate and implement these
checks.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dda0ba05-7db9-4fdb-839f-4aa97b43a8b5

📥 Commits

Reviewing files that changed from the base of the PR and between 14196a5 and 57bc7c9.

📒 Files selected for processing (5)
  • src/controllers/agent/AgentController.ts
  • src/controllers/types.ts
  • src/routes/routes.ts
  • src/routes/swagger.json
  • src/utils/constant.ts
✅ Files skipped from review due to trivial changes (1)
  • src/routes/routes.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/routes/swagger.json

Comment thread src/controllers/agent/AgentController.ts
Comment thread src/controllers/types.ts
Comment thread src/utils/constant.ts
Comment on lines +33 to +37
export const ALGORITHM_MAP: Record<string, string> = {
ed25519: 'EdDSA',
p256: 'ES256',
secp256k1: 'ES256K',
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Normalize the EC curve spellings in ALGORITHM_MAP.

verify() and signRawData() lowercase keyType before lookup, but the rest of this module uses P-256 / P-384 / P-521. Those inputs become p-256 / p-384 / p-521, which miss this map and get forwarded as raw curve names instead of JOSE algs. Valid EC requests will fail unless callers know to send the special p256 spelling.

♻️ Minimal fix
 export const ALGORITHM_MAP: Record<string, string> = {
   ed25519: 'EdDSA',
   p256: 'ES256',
+  'p-256': 'ES256',
+  'p-384': 'ES384',
+  'p-521': 'ES512',
   secp256k1: 'ES256K',
 }
📝 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.

Suggested change
export const ALGORITHM_MAP: Record<string, string> = {
ed25519: 'EdDSA',
p256: 'ES256',
secp256k1: 'ES256K',
}
export const ALGORITHM_MAP: Record<string, string> = {
ed25519: 'EdDSA',
p256: 'ES256',
'p-256': 'ES256',
'p-384': 'ES384',
'p-521': 'ES512',
secp256k1: 'ES256K',
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/constant.ts` around lines 33 - 37, The ALGORITHM_MAP keys must
match the lowercased keyType produced by verify() and signRawData() (which yield
values like "p-256" etc.), so update ALGORITHM_MAP to include the hyphenated
lowercase EC curve spellings (e.g., "p-256": "ES256", "p-384": "ES384", "p-521":
"ES512") and also add any common alternates ("p256", "p384", "p521") if you want
backward compatibility; modify the ALGORITHM_MAP constant accordingly so lookups
from verify()/signRawData() return the correct JOSE algorithms.

Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
@sonarqubecloud
Copy link
Copy Markdown

@sagarkhole4 sagarkhole4 merged commit 3116c1a into main May 13, 2026
9 checks passed
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.

2 participants