feat: added x509 certificates logic with auth#346
Merged
tipusinghaw merged 2 commits intofeat/oidc-main-syncfrom Mar 4, 2026
Merged
feat: added x509 certificates logic with auth#346tipusinghaw merged 2 commits intofeat/oidc-main-syncfrom
tipusinghaw merged 2 commits intofeat/oidc-main-syncfrom
Conversation
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR extends the system with multi-tenant certificate retrieval capabilities and organizational token generation. It adds environment configuration variables, introduces a new REST controller for token and certificate endpoints, updates the certificate retrieval logic to be tenant-aware, and adds platform integration helpers for fetching tokens and certificates from external services. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthController
participant Platform
participant TrustService
Client->>AuthController: POST /v1/orgs/{orgId}/token<br/>(orgId, clientId, clientSecret)
AuthController->>Platform: Forward credentials to PLATFORM_BASE_URL
Platform-->>AuthController: Return OAuth token
AuthController-->>Client: Return OrgTokenResponse { token }
sequenceDiagram
participant Client
participant AuthController
participant Helpers as Helpers/oid4vc-agent
participant Platform
participant TrustService
Client->>AuthController: GET /test/dedicated-x509-certificates
AuthController->>Helpers: fetchDedicatedX509Certificates()
Helpers->>Platform: fetchPlatformToken(PLATFORM_DEDICATED_*_SECRET)
Platform-->>Helpers: OAuth token
Helpers->>TrustService: fetchTrustServiceCertificates(token, no ecosystems)
TrustService-->>Helpers: Certificate array
Helpers-->>AuthController: string[]
AuthController-->>Client: Certificate array
sequenceDiagram
participant Client
participant AuthController
participant Agent as getTrustedCerts
participant Helpers
participant Platform
participant TrustService
Client->>AuthController: GET /test/shared-agent-x509-certificates
AuthController->>Agent: fetchSharedAgentX509Certificates(tenantId)
Agent->>Agent: Resolve agent, determine type
Agent->>Helpers: fetchSharedAgentX509Certificates(tenantId)
Helpers->>Platform: fetchPlatformToken(PLATFORM_SHARED_*_SECRET)
Platform-->>Helpers: OAuth token
Helpers->>Agent: Fetch ecosystem IDs for tenantId
Agent-->>Helpers: ecosystemIds
Helpers->>TrustService: fetchTrustServiceCertificates(token, ecosystemIds)
TrustService-->>Helpers: Certificate array
Helpers-->>Agent: Filtered certificates
Agent-->>AuthController: string[]
AuthController-->>Client: Certificate array
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
src/routes/swagger.json (1)
2997-3012: HardenOrgTokenRequestagainst empty credentials.At Line [2999]-Line [3004], both fields allow empty strings. Add
minLength: 1to prevent trivially invalid requests hitting downstream auth.Diff suggestion
"OrgTokenRequest": { "properties": { "clientId": { - "type": "string" + "type": "string", + "minLength": 1 }, "clientSecret": { - "type": "string" + "type": "string", + "minLength": 1 } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/swagger.json` around lines 2997 - 3012, The schema OrgTokenRequest currently allows empty strings for clientId and clientSecret; update the JSON schema for OrgTokenRequest by adding "minLength": 1 to both the clientId and clientSecret property definitions so they cannot be empty, keeping the existing "required" array and "additionalProperties": false intact to enforce the constraint in the OrgTokenRequest object.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.sample:
- Line 45: The PLATFORM_BASE_URL entry currently includes an inline comment as
part of its value; update the .env line so PLATFORM_BASE_URL has only the URL
(or is empty) and move the explanatory comment to its own line prefixed with #
(e.g., add a separate comment line mentioning "CRED EBL BASE URL") so the env
value is not polluted by the comment; locate and edit the PLATFORM_BASE_URL line
to remove the inline comment and ensure only a valid URL or empty assignment
remains.
In `@src/controllers/auth/AuthController.ts`:
- Around line 38-45: The method is returning the raw upstream payload instead of
the declared OrgTokenResponse shape ({ token: string }); update the
AuthController method that calls axios.post<OrgTokenResponse> to explicitly
construct and return an object matching OrgTokenResponse (e.g., return { token:
response.data.token }) and ensure the function signature/return type is
OrgTokenResponse so callers and generated schemas receive the declared shape; if
upstream uses a different field name, map that field to token or validate its
presence before returning.
- Around line 46-63: Remove or protect the three temporary public test endpoints
that expose certificate/trust operations: the controller methods
testFetchDedicatedX509Certificates, testFetchSharedAgentX509Certificates, and
testGetTrustedCerts (which call fetchDedicatedX509Certificates,
fetchSharedAgentX509Certificates, and getTrustedCerts respectively). Either
delete these methods entirely before release or restrict access by applying your
authentication/authorization guard (e.g., add the appropriate `@UseGuards`(...)
and role-check decorator or middleware) and ensure only privileged users can
call them; update any tests or documentation that reference these routes
accordingly.
In `@src/routes/swagger.json`:
- Around line 6930-7005: The temporary "test" certificate endpoints
(operationId: TestFetchDedicatedX509Certificates,
TestFetchSharedAgentX509Certificates, TestGetTrustedCerts) expose sensitive
certificate/trust info with an empty security array; remove these routes from
the production swagger or protect them: either delete the /v1/orgs/test/* paths
entirely, or wrap them behind a non-production feature flag and add proper
security requirements (populate the "security" array with the required auth
scheme and scopes or an "internal" guard) and restrict to admin/internal orgs;
ensure the description "[TEMP]" is removed and that any remaining endpoints
validate caller identity before returning cert data.
- Around line 6888-6929: The /v1/orgs/{orgId}/token operation (operationId
"GetOrgToken", controller method getOrgToken) is unintentionally exposed;
re-enable authentication by restoring the previously commented-out security
decorator or by adding an operation-level security requirement in the OpenAPI
spec: reapply `@Security`('jwt', [SCOPES.UNPROTECTED]) (or the correct scope
array) on getOrgToken in the controller and ensure the swagger entry for "post"
under "/v1/orgs/{orgId}/token" includes a corresponding "security": [{"jwt":
[...]}] block so the endpoint is no longer unauthenticated.
In `@src/utils/helpers.ts`:
- Around line 108-110: The logs in helpers.ts are printing full
token/certificate payloads via console.log(`[${label}] token response data:`,
JSON.stringify(tokenResponse.data, ...)) which can leak sensitive auth/trust
data; remove or redact any logging of tokenResponse.data (and any similar
console.log of raw payloads elsewhere, e.g., the other occurrences around lines
139-141) and keep only non-sensitive metadata such as tokenResponse.status or a
sanitized summary (e.g., token length or masked fields) when using console.log
with the variables label and tokenResponse.
In `@src/utils/oid4vc-agent.ts`:
- Around line 259-261: The runtime is currently printing certificate contents
from getTrustedCerts via console.log of the certs array which can leak sensitive
trust anchors; remove those console.log statements and any direct serialization
of certs, and if you need telemetry keep only a non-sensitive metric such as the
number of certs using an application's logger (e.g., processLogger.info) or a
redacted/hashed indicator instead of the full certificate data; update
references in getTrustedCerts where certs and the two console.log calls appear
to use the safer alternative.
---
Nitpick comments:
In `@src/routes/swagger.json`:
- Around line 2997-3012: The schema OrgTokenRequest currently allows empty
strings for clientId and clientSecret; update the JSON schema for
OrgTokenRequest by adding "minLength": 1 to both the clientId and clientSecret
property definitions so they cannot be empty, keeping the existing "required"
array and "additionalProperties": false intact to enforce the constraint in the
OrgTokenRequest object.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.env.samplesrc/cliAgent.tssrc/controllers/auth/AuthController.tssrc/routes/routes.tssrc/routes/swagger.jsonsrc/utils/helpers.tssrc/utils/oid4vc-agent.ts
| {"reqSignature":{},"txn":{"data":{"data":{"alias":"lorica-identity-node1","blskey":"wUh24sVCQ8PHDgSb343g2eLxjD5vwxsrETfuV2sbwMNnYon9nhbaK5jcWTekvXtyiwxHxuiCCoZwKS97MQEAeC2oLbbMeKjYm212QwSnm7aKLEqTStXht35VqZvZLT7Q3mPQRYLjMGixdn4ocNHrBTMwPUQYycEqwaHWgE1ncDueXY","blskey_pop":"R2sMwF7UW6AaD4ALa1uB1YVPuP6JsdJ7LsUoViM9oySFqFt34C1x1tdHDysS9wwruzaaEFui6xNPqJ8eu3UBqcFKkoWhdsMqCALwe63ytxPwvtLtCffJLhHAcgrPC7DorXYdqhdG2cevdqc5oqFEAaKoFDBf12p5SsbbM4PYWCmVCb","client_ip":"35.225.220.151","client_port":"9702","node_ip":"35.224.26.110","node_port":"9701","services":["VALIDATOR"]},"dest":"k74ZsZuUaJEcB8RRxMwkCwdE5g1r9yzA3nx41qvYqYf"},"metadata":{"from":"Ex6hzsJFYzNJ7kzbfncNeU"},"type":"0"},"txnMetadata":{"seqNo":6,"txnId":"6880673ce4ae4a2352f103d2a6ae20469dd070f2027283a1da5e62a64a59d688"},"ver":"1"} | ||
| {"reqSignature":{},"txn":{"data":{"data":{"alias":"cysecure-itn","blskey":"GdCvMLkkBYevRFi93b6qaj9G2u1W6Vnbg8QhRD1chhrWR8vRE8x9x7KXVeUBPFf6yW5qq2JCfA2frc8SGni2RwjtTagezfwAwnorLhVJqS5ZxTi4pgcw6smebnt4zWVhTkh6ugDHEypHwNQBcw5WhBZcEJKgNbyVLnHok9ob6cfr3u","blskey_pop":"RbH9mY7M5p3UB3oj4sT1skYwMkxjoUnja8eTYfcm83VcNbxC9zR9pCiRhk4q1dJT3wkDBPGNKnk2p83vaJYLcgMuJtzoWoJAWAxjb3Mcq8Agf6cgQpBuzBq2uCzFPuQCAhDS4Kv9iwA6FsRnfvoeFTs1hhgSJVxQzDWMVTVAD9uCqu","client_ip":"35.169.19.171","client_port":"9702","node_ip":"54.225.56.21","node_port":"9701","services":["VALIDATOR"]},"dest":"4ETBDmHzx8iDQB6Xygmo9nNXtMgq9f6hxGArNhQ6Hh3u"},"metadata":{"from":"uSXXXEdBicPHMMhr3ddNF"},"type":"0"},"txnMetadata":{"seqNo":7,"txnId":"3c21718b07806b2f193b35953dda5b68b288efd551dce4467ce890703d5ba549"},"ver":"1"}` | ||
|
|
||
| PLATFORM_BASE_URL= #CREDEBL BASE URL |
There was a problem hiding this comment.
Fix .env assignment syntax for PLATFORM_BASE_URL.
The inline comment is currently part of the value, which can produce an invalid URL at runtime.
🧩 Suggested patch
-PLATFORM_BASE_URL= `#CREDEBL` BASE URL
+# CREDEBL BASE URL
+PLATFORM_BASE_URL=📝 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
| PLATFORM_BASE_URL= #CREDEBL BASE URL | |
| # CREDEBL BASE URL | |
| PLATFORM_BASE_URL= |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 45-45: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 45-45: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.env.sample at line 45, The PLATFORM_BASE_URL entry currently includes an
inline comment as part of its value; update the .env line so PLATFORM_BASE_URL
has only the URL (or is empty) and move the explanatory comment to its own line
prefixed with # (e.g., add a separate comment line mentioning "CRED EBL BASE
URL") so the env value is not polluted by the comment; locate and edit the
PLATFORM_BASE_URL line to remove the inline comment and ensure only a valid URL
or empty assignment remains.
Comment on lines
+38
to
+45
| const response = await axios.post<OrgTokenResponse>( | ||
| `${platformBaseUrl}/v1/orgs/${orgId}/token`, | ||
| { clientId: body.clientId, clientSecret: body.clientSecret }, | ||
| { headers: { 'Content-Type': 'application/json', accept: 'application/json' } }, | ||
| ) | ||
|
|
||
| return response.data | ||
| } |
There was a problem hiding this comment.
Return the declared OrgTokenResponse shape explicitly.
The method returns raw upstream payload, but the API contract declares { token: string }. This can break clients and generated schema expectations.
📦 Suggested patch
- const response = await axios.post<OrgTokenResponse>(
+ const response = await axios.post<{ token?: string; data?: { access_token?: string } }>(
`${platformBaseUrl}/v1/orgs/${orgId}/token`,
{ clientId: body.clientId, clientSecret: body.clientSecret },
{ headers: { 'Content-Type': 'application/json', accept: 'application/json' } },
)
-
- return response.data
+ const token = response.data.token ?? response.data.data?.access_token
+ if (!token) throw new BadRequestError('Token not found in platform response')
+ return { token }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/controllers/auth/AuthController.ts` around lines 38 - 45, The method is
returning the raw upstream payload instead of the declared OrgTokenResponse
shape ({ token: string }); update the AuthController method that calls
axios.post<OrgTokenResponse> to explicitly construct and return an object
matching OrgTokenResponse (e.g., return { token: response.data.token }) and
ensure the function signature/return type is OrgTokenResponse so callers and
generated schemas receive the declared shape; if upstream uses a different field
name, map that field to token or validate its presence before returning.
Comment on lines
+46
to
+63
| // TODO: Remove these test endpoints after manual testing is done | ||
| @Get('/test/dedicated-x509-certificates') | ||
| public async testFetchDedicatedX509Certificates(@Request() _request: Req): Promise<string[]> { | ||
| return fetchDedicatedX509Certificates() | ||
| } | ||
|
|
||
| @Get('/test/shared-agent-x509-certificates') | ||
| public async testFetchSharedAgentX509Certificates(@Request() _request: Req): Promise<string[]> { | ||
| return fetchSharedAgentX509Certificates() | ||
| } | ||
|
|
||
| /** | ||
| * [TEMP] Manually trigger getTrustedCerts to test agent type detection and trust list fetch | ||
| */ | ||
| @Get('/test/trusted-certs') | ||
| public async testGetTrustedCerts(@Request() _request: Req): Promise<string[]> { | ||
| return getTrustedCerts() | ||
| } |
There was a problem hiding this comment.
Protect or remove test certificate endpoints before release.
These endpoints are publicly reachable as declared and expose sensitive trust/certificate operations without authentication.
🛡️ Suggested patch
-import { Body, Controller, Get, Path, Post, Request, Route, Tags } from 'tsoa'
+import { Body, Controller, Get, Path, Post, Request, Route, Security, Tags } from 'tsoa'
@@
`@Get`('/test/dedicated-x509-certificates')
+ `@Security`('jwt', ['tenant', 'dedicated'])
public async testFetchDedicatedX509Certificates(`@Request`() _request: Req): Promise<string[]> {
return fetchDedicatedX509Certificates()
}
`@Get`('/test/shared-agent-x509-certificates')
+ `@Security`('jwt', ['tenant', 'dedicated'])
public async testFetchSharedAgentX509Certificates(`@Request`() _request: Req): Promise<string[]> {
return fetchSharedAgentX509Certificates()
}
@@
`@Get`('/test/trusted-certs')
+ `@Security`('jwt', ['tenant', 'dedicated'])
public async testGetTrustedCerts(`@Request`() _request: Req): Promise<string[]> {
return getTrustedCerts()
}🧰 Tools
🪛 ESLint
[error] 46-46: Insert ··
(prettier/prettier)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/controllers/auth/AuthController.ts` around lines 46 - 63, Remove or
protect the three temporary public test endpoints that expose certificate/trust
operations: the controller methods testFetchDedicatedX509Certificates,
testFetchSharedAgentX509Certificates, and testGetTrustedCerts (which call
fetchDedicatedX509Certificates, fetchSharedAgentX509Certificates, and
getTrustedCerts respectively). Either delete these methods entirely before
release or restrict access by applying your authentication/authorization guard
(e.g., add the appropriate `@UseGuards`(...) and role-check decorator or
middleware) and ensure only privileged users can call them; update any tests or
documentation that reference these routes accordingly.
Comment on lines
+6888
to
+6929
| "/v1/orgs/{orgId}/token": { | ||
| "post": { | ||
| "operationId": "GetOrgToken", | ||
| "responses": { | ||
| "200": { | ||
| "description": "Ok", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "$ref": "#/components/schemas/OrgTokenResponse" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "description": "Generate an organization token by forwarding credentials to the platform", | ||
| "tags": [ | ||
| "Auth" | ||
| ], | ||
| "security": [], | ||
| "parameters": [ | ||
| { | ||
| "in": "path", | ||
| "name": "orgId", | ||
| "required": true, | ||
| "schema": { | ||
| "type": "string" | ||
| } | ||
| } | ||
| ], | ||
| "requestBody": { | ||
| "required": true, | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "$ref": "#/components/schemas/OrgTokenRequest" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check swagger.json around line 6888-6929 ==="
sed -n '6880,6940p' src/routes/swagger.json
echo
echo "=== Search for GetOrgToken in route files ==="
fd -e ts | xargs rg -l 'GetOrgToken|routes|controller' 2>/dev/null | head -20
echo
echo "=== Locate routes file ==="
fd routes.ts | head -5
echo
echo "=== Locate auth controller ==="
fd AuthController.ts | head -5Repository: credebl/agent-controller
Length of output: 1783
🏁 Script executed:
#!/bin/bash
# Check for the specific endpoints in routes
rg -n '/v1/orgs/.*token|/v1/orgs/test/' src/routes/ -t ts 2>/dev/null | head -30Repository: credebl/agent-controller
Length of output: 385
🏁 Script executed:
#!/bin/bash
# Search in controllers for auth-related logic
fd AuthController.ts
rg -l 'orgToken|GetOrgToken|OrgTokenRequest' src/ -t tsRepository: credebl/agent-controller
Length of output: 166
🏁 Script executed:
#!/bin/bash
# Examine the route registration for GetOrgToken and auth middleware
sed -n '3640,3680p' src/routes/routes.tsRepository: credebl/agent-controller
Length of output: 2276
🏁 Script executed:
#!/bin/bash
# Check the other test endpoints
sed -n '3680,3760p' src/routes/routes.tsRepository: credebl/agent-controller
Length of output: 4509
🏁 Script executed:
#!/bin/bash
# Look at AuthController.ts to understand the handlers
wc -l src/controllers/auth/AuthController.tsRepository: credebl/agent-controller
Length of output: 109
🏁 Script executed:
#!/bin/bash
# Search for orgToken method in AuthController
rg -n 'orgToken|OrgToken' src/controllers/auth/AuthController.ts -A 5Repository: credebl/agent-controller
Length of output: 899
🏁 Script executed:
#!/bin/bash
# Check if there's any auth middleware applied to routes in routes.ts
rg -n 'middleware|auth|jwt|security|setAsyncId|agent_auth' src/routes/routes.ts | grep -i '3645\|3680\|3715\|3750\|middleware\|auth' | head -30Repository: credebl/agent-controller
Length of output: 3160
🏁 Script executed:
#!/bin/bash
# Check for any Security decorators or auth requirements in AuthController
head -30 src/controllers/auth/AuthController.tsRepository: credebl/agent-controller
Length of output: 931
🏁 Script executed:
#!/bin/bash
# Look for fetchMiddlewares definition to understand what it returns
rg -n 'fetchMiddlewares|export.*fetchMiddlewares' src/ -A 3 | head -40Repository: credebl/agent-controller
Length of output: 3162
🏁 Script executed:
#!/bin/bash
# Verify the test endpoints - check if they have any specific auth requirements
rg -n 'testFetchDedicatedX509Certificates|testFetchSharedAgentX509Certificates|testGetTrustedCerts' src/controllers/auth/AuthController.ts -B 2 -A 8Repository: credebl/agent-controller
Length of output: 831
🏁 Script executed:
#!/bin/bash
# Check if there's global auth middleware or if these endpoints are intentionally public
rg -n '"security".*\[\]' src/routes/swagger.json | wc -l
echo "---"
rg -n '"security".*jwt' src/routes/swagger.json | head -5Repository: credebl/agent-controller
Length of output: 73
Token issuance endpoint is unauthenticated and appears unintentionally so.
The /v1/orgs/{orgId}/token endpoint has no authentication despite handling sensitive credentials (clientSecret) and returning tokens. The commented-out @Security('jwt', [SCOPES.UNPROTECTED]) decorator on getOrgToken indicates this was considered but disabled, suggesting the current state may be unintentional. Remove the security decorator comment or reactivate authentication to match the sensitivity of this endpoint.
Note: The three test endpoints (/v1/orgs/test/*) are intentionally unprotected and marked with TODO comments for removal after testing—these are separate from the main concern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/swagger.json` around lines 6888 - 6929, The /v1/orgs/{orgId}/token
operation (operationId "GetOrgToken", controller method getOrgToken) is
unintentionally exposed; re-enable authentication by restoring the previously
commented-out security decorator or by adding an operation-level security
requirement in the OpenAPI spec: reapply `@Security`('jwt', [SCOPES.UNPROTECTED])
(or the correct scope array) on getOrgToken in the controller and ensure the
swagger entry for "post" under "/v1/orgs/{orgId}/token" includes a corresponding
"security": [{"jwt": [...]}] block so the endpoint is no longer unauthenticated.
Comment on lines
+6930
to
+7005
| "/v1/orgs/test/dedicated-x509-certificates": { | ||
| "get": { | ||
| "operationId": "TestFetchDedicatedX509Certificates", | ||
| "responses": { | ||
| "200": { | ||
| "description": "Ok", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "items": { | ||
| "type": "string" | ||
| }, | ||
| "type": "array" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "tags": [ | ||
| "Auth" | ||
| ], | ||
| "security": [], | ||
| "parameters": [] | ||
| } | ||
| }, | ||
| "/v1/orgs/test/shared-agent-x509-certificates": { | ||
| "get": { | ||
| "operationId": "TestFetchSharedAgentX509Certificates", | ||
| "responses": { | ||
| "200": { | ||
| "description": "Ok", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "items": { | ||
| "type": "string" | ||
| }, | ||
| "type": "array" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "tags": [ | ||
| "Auth" | ||
| ], | ||
| "security": [], | ||
| "parameters": [] | ||
| } | ||
| }, | ||
| "/v1/orgs/test/trusted-certs": { | ||
| "get": { | ||
| "operationId": "TestGetTrustedCerts", | ||
| "responses": { | ||
| "200": { | ||
| "description": "Ok", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "items": { | ||
| "type": "string" | ||
| }, | ||
| "type": "array" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "description": "[TEMP] Manually trigger getTrustedCerts to test agent type detection and trust list fetch", | ||
| "tags": [ | ||
| "Auth" | ||
| ], | ||
| "security": [], | ||
| "parameters": [] | ||
| } | ||
| }, |
There was a problem hiding this comment.
Remove or protect temporary test certificate endpoints before release.
Line [6930]-Line [7005] introduces test endpoints with empty security, and Line [6998] explicitly marks one as [TEMP]. This is a production-facing information disclosure risk for certificate/trust retrieval.
Diff suggestion (minimum mitigation)
"/v1/orgs/test/dedicated-x509-certificates": {
"get": {
@@
- "security": [],
+ "security": [
+ {
+ "jwt": [
+ "tenant",
+ "dedicated"
+ ]
+ }
+ ],
@@
"/v1/orgs/test/shared-agent-x509-certificates": {
"get": {
@@
- "security": [],
+ "security": [
+ {
+ "jwt": [
+ "tenant",
+ "dedicated"
+ ]
+ }
+ ],
@@
"/v1/orgs/test/trusted-certs": {
"get": {
@@
- "description": "[TEMP] Manually trigger getTrustedCerts to test agent type detection and trust list fetch",
+ "description": "Internal test endpoint. Do not expose in production.",
@@
- "security": [],
+ "security": [
+ {
+ "jwt": [
+ "tenant",
+ "dedicated"
+ ]
+ }
+ ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/swagger.json` around lines 6930 - 7005, The temporary "test"
certificate endpoints (operationId: TestFetchDedicatedX509Certificates,
TestFetchSharedAgentX509Certificates, TestGetTrustedCerts) expose sensitive
certificate/trust info with an empty security array; remove these routes from
the production swagger or protect them: either delete the /v1/orgs/test/* paths
entirely, or wrap them behind a non-production feature flag and add proper
security requirements (populate the "security" array with the required auth
scheme and scopes or an "internal" guard) and restrict to admin/internal orgs;
ensure the description "[TEMP]" is removed and that any remaining endpoints
validate caller identity before returning cert data.
src/utils/helpers.ts
Outdated
Comment on lines
+108
to
+110
| console.log(`[${label}] token response status:`, tokenResponse.status) | ||
| console.log(`[${label}] token response data:`, JSON.stringify(tokenResponse.data, null, 2)) | ||
|
|
There was a problem hiding this comment.
Remove raw token/certificate payload logging.
These logs can expose sensitive auth and trust data in application logs. Keep status/metadata logs only, not full payloads.
🔒 Suggested patch
- console.log(`[${label}] token response data:`, JSON.stringify(tokenResponse.data, null, 2))
+ // Avoid logging full token payloads
@@
- console.log(`[${label}] certificates response data:`, JSON.stringify(certResponse.data, null, 2))
+ // Avoid logging full certificate payloadsAlso applies to: 139-141
🧰 Tools
🪛 ESLint
[error] 108-108: Unexpected console statement.
(no-console)
[error] 109-109: Unexpected console statement.
(no-console)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/helpers.ts` around lines 108 - 110, The logs in helpers.ts are
printing full token/certificate payloads via console.log(`[${label}] token
response data:`, JSON.stringify(tokenResponse.data, ...)) which can leak
sensitive auth/trust data; remove or redact any logging of tokenResponse.data
(and any similar console.log of raw payloads elsewhere, e.g., the other
occurrences around lines 139-141) and keep only non-sensitive metadata such as
tokenResponse.status or a sanitized summary (e.g., token length or masked
fields) when using console.log with the variables label and tokenResponse.
Comment on lines
+259
to
+261
| // Remove this log after testing to avoid logging sensitive certificate information | ||
| console.log('[getTrustedCerts] fetched certificates count:', certs.length) | ||
| console.log("certs::::::::::::::::::::::::::", certs) |
There was a problem hiding this comment.
Do not log certificate contents in runtime paths.
certs values are being printed directly; this can leak trust anchors and internal certificate material into logs.
🔐 Suggested patch
-// Remove this log after testing to avoid logging sensitive certificate information
- console.log('[getTrustedCerts] fetched certificates count:', certs.length)
- console.log("certs::::::::::::::::::::::::::", certs)
+ console.log('[getTrustedCerts] fetched certificates count:', certs.length)🧰 Tools
🪛 ESLint
[error] 259-259: Insert ····
(prettier/prettier)
[error] 260-260: Unexpected console statement.
(no-console)
[error] 261-261: Unexpected console statement.
(no-console)
[error] 261-261: Replace "certs::::::::::::::::::::::::::" with 'certs::::::::::::::::::::::::::'
(prettier/prettier)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/oid4vc-agent.ts` around lines 259 - 261, The runtime is currently
printing certificate contents from getTrustedCerts via console.log of the certs
array which can leak sensitive trust anchors; remove those console.log
statements and any direct serialization of certs, and if you need telemetry keep
only a non-sensitive metric such as the number of certs using an application's
logger (e.g., processLogger.info) or a redacted/hashed indicator instead of the
full certificate data; update references in getTrustedCerts where certs and the
two console.log calls appear to use the safer alternative.
GHkrishna
requested changes
Mar 3, 2026
Comment on lines
+46
to
+61
| // TODO: Remove these test endpoints after manual testing is done | ||
| @Get('/test/dedicated-x509-certificates') | ||
| public async testFetchDedicatedX509Certificates(@Request() _request: Req): Promise<string[]> { | ||
| return fetchDedicatedX509Certificates() | ||
| } | ||
|
|
||
| @Get('/test/shared-agent-x509-certificates') | ||
| public async testFetchSharedAgentX509Certificates(@Request() _request: Req): Promise<string[]> { | ||
| return fetchSharedAgentX509Certificates() | ||
| } | ||
|
|
||
| /** | ||
| * [TEMP] Manually trigger getTrustedCerts to test agent type detection and trust list fetch | ||
| */ | ||
| @Get('/test/trusted-certs') | ||
| public async testGetTrustedCerts(@Request() _request: Req): Promise<string[]> { |
Contributor
Author
There was a problem hiding this comment.
We will remove this after testing on the instance
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
|
GHkrishna
approved these changes
Mar 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What