- 
                Notifications
    You must be signed in to change notification settings 
- Fork 65
merge: Sync changes from qa to main #1418
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ard workflow (#1301) Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com>
* feat: otel and signoz implementation Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * fix:resolve suggested comments Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * fix:resolve suggested for logging Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * feat: added flag for local setup Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * refactor: updated env sample and demo Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> --------- Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
* feat: single sign on Signed-off-by: bhavanakarwade <bhavana.karwade@ayanworks.com> * fix: rabbit suggestions Signed-off-by: bhavanakarwade <bhavana.karwade@ayanworks.com> * update: send verification mail template Signed-off-by: bhavanakarwade <bhavana.karwade@ayanworks.com> --------- Signed-off-by: bhavanakarwade <bhavana.karwade@ayanworks.com>
Signed-off-by: Sujit <sujit.sutar@ayanworks.com>
Signed-off-by: Sujit <sujit.sutar@ayanworks.com>
Signed-off-by: Sujit <sujit.sutar@ayanworks.com>
…hema_api Included organization name in all schema api for schema list
Merge latest develop branch into QA
Signed-off-by: Sujit <sujit.sutar@ayanworks.com>
Fix email not saving to DB and include connection label in proofs GET API
Signed-off-by: Sujit <sujit.sutar@ayanworks.com>
…edentials Added prisma relation for credentials and connections table
Fix Email Saving and Add Relations for Connection-Credential/Presentation Tables
* feat: added logic for updated routes for agent Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * refactor: uncommented required code Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * feat:added logic to get token from cloudwatch Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> * feat: update credo image in taskdef Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> * feat: change service name for testing Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> * chore: WIP Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> * debug: added logs for agent details Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * refactor:removed console logs Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * refactor:removed commented code Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * feat: added token for old wallet Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * refactor: cloud wallet urls and token changes Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * refactor: removed logs from shell scripts Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * refactor: resolve comment Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * refactor: resolve comments for getOrgAgentApiKey Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * refactor: agent service comments Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> --------- Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> Co-authored-by: Sahil Kamble <sahil.kamble@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
* fix: schema by id Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * fix: schema by id Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> --------- Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
* fix: schema by id Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * fix: schema by id Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * fix: duplicate URL creation Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * fix: added await in common method Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * fix: added logic for get token Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> * fix: polygon routes Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com> --------- Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
…r images (#1376) Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
merge: DEV to QA
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Sujit <sujit.sutar@ayanworks.com>
Fix crash on all schema view when no organization exists
Signed-off-by: bhavanakarwade <bhavana.karwade@ayanworks.com>
Signed-off-by: bhavanakarwade <bhavana.karwade@ayanworks.com>
Signed-off-by: bhavanakarwade <bhavana.karwade@ayanworks.com>
Signed-off-by: bhavanakarwade <bhavana.karwade@ayanworks.com>
Signed-off-by: bhavanakarwade <bhavana.karwade@ayanworks.com>
Signed-off-by: bhavanakarwade <bhavana.karwade@ayanworks.com>
* feat: script changes to spinup credo-controller -using docker Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> * typo error - agent spin-up issue Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> * removed unwanted values Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> --------- Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com>
feat/credo-spinup-script (#1417)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️  Outside diff range comments (8)
apps/agent-provisioning/src/agent-provisioning.service.ts (2)
43-75: Promise handling and error propagation are broken; secrets risk in logs
- You ignore exec’s err and throw inside the callback, which won’t reject the outer Promise reliably.
- Logging raw stdout/stderr may leak secrets (the script currently echoes WALLET_PASSWORD and prints API tokens).
Apply:
-const spinUpResponse: object = new Promise(async (resolve) => { - await exec(walletProvision, async (err, stdout, stderr) => { - this.logger.log(`shell script output: ${stdout}`); - if (stderr) { - this.logger.log(`shell script error: ${stderr}`); - } +const spinUpResponse: object = await new Promise((resolve, reject) => { + execFile(scriptPath, args, (err, stdout, stderr) => { + if (err) { + this.logger.error(`[walletProvision] script failed: ${stderr ?? err.message}`); + return reject(err); + } + // Avoid logging stdout wholesale; if needed, mask secrets here.Also mask/stop printing sensitive values in the shell scripts (see comments on docker_start_agent.sh and start_agent_ecs.sh).
50-67: Harden file reads and JSON parsingWrap JSON.parse in try/catch and include file paths in context; otherwise a malformed file leaves the Promise pending.
- agentEndPoint = await fs.readFileSync(agentEndpointPath, 'utf8'); - agentToken = await fs.readFileSync(agentTokenPath, 'utf8'); + try { + agentEndPoint = fs.readFileSync(agentEndpointPath, 'utf8'); + agentToken = fs.readFileSync(agentTokenPath, 'utf8'); + const endpointJson = JSON.parse(agentEndPoint); + const tokenJson = JSON.parse(agentToken); + resolve({ agentEndPoint: endpointJson.CONTROLLER_ENDPOINT, agentToken: tokenJson.token }); + } catch (e) { + return reject(new NotFoundException(`Invalid JSON in ${agentEndpointPath} or ${agentTokenPath}`)); + } - resolve({ - agentEndPoint: JSON.parse(agentEndPoint).CONTROLLER_ENDPOINT, - agentToken: JSON.parse(agentToken).token -});apps/agent-provisioning/AFJ/scripts/docker_start_agent.sh (4)
28-38: Stop printing secrets to stdoutWALLET_PASSWORD and WALLET_STORAGE_PASSWORD are echoed; logs end up in Nest logs via exec. Remove or mask them.
-echo "WALLET_PASSWORD: $WALLET_PASSWORD" ... -echo "WALLET_STORAGE_PASSWORD: $WALLET_STORAGE_PASSWORD" +echo "WALLET_PASSWORD: ******" +echo "WALLET_STORAGE_PASSWORD: ******"Also avoid echoing the extracted token later.
55-63: Ensure port-file directory existsPort files live under agent-provisioning/AFJ/port-file but the directory may not exist.
+# Ensure port-file directory exists +mkdir -p "$(dirname "$ADMIN_PORT_FILE")" "$(dirname "$INBOUND_PORT_FILE")" # Check if admin port file exists and if not, create and initialize it if [ ! -e "$ADMIN_PORT_FILE" ]; then
176-186: Avoid ${ROOT_PATH} in docker-compose unless guaranteed setIf ROOT_PATH is unset, compose may mount from “/”. Mount the known config path.
- - ${ROOT_PATH}:/agent-config + - ${PWD}/agent-provisioning/AFJ/agent-config:/agent-config
250-254: Write token to the expected folderPath should align with AFJ_AGENT_TOKEN_PATH (/agent-provisioning/AFJ/token/).
- cat <<EOF >${PWD}/token/${AGENCY}_${CONTAINER_NAME}.json + mkdir -p "${PWD}/agent-provisioning/AFJ/token" + cat <<EOF >${PWD}/agent-provisioning/AFJ/token/${AGENCY}_${CONTAINER_NAME}.jsonapps/agent-provisioning/AFJ/scripts/start_agent_ecs.sh (2)
114-147: Add apiKey to agent-config for parity with docker pathDocker script writes apiKey; ECS path omits it, causing divergence.
"adminPort": $ADMIN_PORT, "tenancy": $TENANT, - "schemaFileServerURL": "$SCHEMA_FILE_SERVER_URL" + "schemaFileServerURL": "$SCHEMA_FILE_SERVER_URL", + "apiKey": "$AGENT_API_KEY"
336-337: Undefined INTERNAL_IP in endpoints JSONINTERNAL_IP isn’t set. Use INBOUND_ENDPOINT or AGENT_ENDPOINT.
- "AGENT_ENDPOINT" : "${INTERNAL_IP}:${ADMIN_PORT}" + "AGENT_ENDPOINT" : "${AGENT_ENDPOINT}"
♻️ Duplicate comments (6)
apps/agent-provisioning/src/agent-provisioning.service.ts (1)
42-42: TESKDEFINITION_FAMILY → TASKDEFINITION_FAMILY rename is correctly appliedThe env var rename looks good here.
apps/agent-provisioning/AFJ/scripts/start_agent_ecs.sh (3)
312-312: Region now parameterizedUsing $AWS_PUBLIC_REGION for aws logs is correct.
284-286: Remove stray placeholder line breaking the scriptThis is executed as a command and will fail.
-.............................................................
198-206: Invalid container definition JSON (logConfiguration not closed)“ulimits” is nested inside logConfiguration. ECS will reject the task definition.
- "logConfiguration": { - "logDriver": "awslogs", - "options": { - "awslogs-group": "/ecs/$TASKDEFINITION_FAMILY", - "awslogs-create-group": "true", - "awslogs-region": "$AWS_PUBLIC_REGION", - "awslogs-stream-prefix": "ecs" - }, - "ulimits": [] + "logConfiguration": { + "logDriver": "awslogs", + "options": { + "awslogs-group": "/ecs/$TASKDEFINITION_FAMILY", + "awslogs-create-group": "true", + "awslogs-region": "$AWS_PUBLIC_REGION", + "awslogs-stream-prefix": "ecs" + } + }, + "ulimits": []apps/agent-service/src/agent-service.service.ts (2)
1120-1122: Fix wallet creation check (AND → OR)Current check always passes when partial object exists.
- if (!walletResponseDetails && !walletResponseDetails.id && !walletResponseDetails.token) { + if (!walletResponseDetails || !walletResponseDetails.id || !walletResponseDetails.token) { throw new InternalServerErrorException('Error while creating the wallet'); }
1785-1794: Branch always taken: compares enum instead of valueif (OrgAgentType.SHARED) is always truthy.
- let getApiKey; - if (OrgAgentType.SHARED) { + let getApiKey; + if (orgAgentTypeResult.agent === OrgAgentType.SHARED) { const platformAdminSpinnedUp = await this.agentServiceRepository.platformAdminAgent( CommonConstants.PLATFORM_ADMIN_ORG ); getApiKey = await this.commonService.decryptPassword(platformAdminSpinnedUp?.org_agents[0].apiKey); } else { getApiKey = getApiKeyResult?.value; }
🧹 Nitpick comments (7)
apps/agent-provisioning/src/agent-provisioning.service.ts (1)
85-92: Use non-blocking fs APIsaccessSync/readFileSync block the event loop. Prefer fs.promises.access and fs.promises.readFile.
apps/agent-provisioning/AFJ/scripts/docker_start_agent.sh (1)
106-106: Honor INBOUND_ENDPOINT if providedYou define INBOUND_ENDPOINT but never use it. Prefer it when non-empty; fallback to EXTERNAL_IP.
-AGENT_ENDPOINT="${PROTOCOL}://${EXTERNAL_IP}:${INBOUND_PORT}" +if [ -n "$INBOUND_ENDPOINT" ]; then + AGENT_ENDPOINT="${PROTOCOL}://${INBOUND_ENDPOINT}:${INBOUND_PORT}" +else + AGENT_ENDPOINT="${PROTOCOL}://${EXTERNAL_IP}:${INBOUND_PORT}" +fiapps/agent-provisioning/AFJ/scripts/start_agent_ecs.sh (1)
22-27: AGENT_API_KEY/AGENT_HOST args unusedEither use them (e.g., include apiKey in config; use AGENT_HOST for scp) or remove to reduce confusion.
Do you intend to scp the config to the EC2 host? If yes, un-comment the scp and ensure the “volumes.host.sourcePath” exists.
.env.demo (2)
171-171: Avoid shipping real-looking secrets in sampleKeep AGENT_API_KEY clearly placeholder and short. Consider AGENT_API_KEY='changeme-16-chars'.
150-169: dotenv lint nits (ordering/quotes)Purely stylistic; safe to ignore for demo unless your linters enforce them.
apps/agent-service/src/agent-service.service.ts (2)
622-638: Storing empty did/verkeyIntentional? If not, defer storing until DID is available.
978-980: constructUrl hardcodes write-did pathAcceptable, but you already import getAgentUrl elsewhere. Consider reusing it for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
- .env.demo(2 hunks)
- apps/agent-provisioning/AFJ/scripts/docker_start_agent.sh(3 hunks)
- apps/agent-provisioning/AFJ/scripts/start_agent.sh(4 hunks)
- apps/agent-provisioning/AFJ/scripts/start_agent_ecs.sh(7 hunks)
- apps/agent-provisioning/src/agent-provisioning.service.ts(1 hunks)
- apps/agent-service/src/agent-service.service.ts(22 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/agent-provisioning/AFJ/scripts/start_agent.sh
🧰 Additional context used
🧬 Code graph analysis (2)
apps/agent-service/src/agent-service.service.ts (2)
apps/agent-service/src/interface/agent-service.interface.ts (3)
IStoreOrgAgentDetails(177-201)
ICreateTenant(433-439)
WalletDetails(660-665)libs/common/src/common.utils.ts (1)
getAgentUrl(70-109)
apps/agent-provisioning/src/agent-provisioning.service.ts (2)
apps/agent-service/src/interface/agent-service.interface.ts (1)
IWalletProvision(144-164)apps/agent-provisioning/src/interface/agent-provisioning.interfaces.ts (1)
IWalletProvision(4-25)
🪛 Shellcheck (0.10.0)
apps/agent-provisioning/AFJ/scripts/docker_start_agent.sh
[warning] 20-20: INBOUND_ENDPOINT appears unused. Verify use (or export if used externally).
(SC2034)
apps/agent-provisioning/AFJ/scripts/start_agent_ecs.sh
[warning] 22-22: AGENT_API_KEY appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 23-23: AGENT_HOST appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 284-284: This is interpreted as a command name ending with '.'. Double check syntax.
(SC2288)
🪛 dotenv-linter (3.3.0)
.env.demo
[warning] 148-148: [UnorderedKey] The ISSUER key should go before the JWT_TOKEN_SECRET key
(UnorderedKey)
[warning] 152-152: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 153-153: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 154-154: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 155-155: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 155-155: [UnorderedKey] The OTEL_LOGS_OTLP_ENDPOINT key should go before the OTEL_SERVICE_NAME key
(UnorderedKey)
[warning] 156-156: [UnorderedKey] The OTEL_HEADERS_KEY key should go before the OTEL_LOGS_OTLP_ENDPOINT key
(UnorderedKey)
[warning] 157-157: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 157-157: [UnorderedKey] The OTEL_LOGGER_NAME key should go before the OTEL_LOGS_OTLP_ENDPOINT key
(UnorderedKey)
[warning] 158-158: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 158-158: [UnorderedKey] The HOSTNAME key should go before the IS_ENABLE_OTEL key
(UnorderedKey)
[warning] 161-161: [UnorderedKey] The APP_PROTOCOL key should go before the HOSTNAME key
(UnorderedKey)
[warning] 163-163: [UnorderedKey] The CREDEBL_CLIENT_ALIAS key should go before the HOSTNAME key
(UnorderedKey)
[warning] 164-164: [UnorderedKey] The CREDEBL_DOMAIN key should go before the HOSTNAME key
(UnorderedKey)
[warning] 165-165: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 165-165: [UnorderedKey] The CREDEBL_KEYCLOAK_MANAGEMENT_CLIENT_ID key should go before the HOSTNAME key
(UnorderedKey)
[warning] 165-165: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 166-166: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 166-166: [UnorderedKey] The CREDEBL_KEYCLOAK_MANAGEMENT_CLIENT_SECRET key should go before the HOSTNAME key
(UnorderedKey)
[warning] 166-166: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 171-171: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 189-189: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🪛 GitHub Check: CodeQL
apps/agent-provisioning/src/agent-provisioning.service.ts
[warning] 31-31: Shell command built from environment values
This shell command depends on an uncontrolled absolute path.
🔇 Additional comments (5)
.env.demo (1)
113-113: SCHEMA_FILE_SERVER_URL blank by defaultFine for demo; ensure non-empty in environments where schema fetching is required.
apps/agent-service/src/agent-service.service.ts (4)
425-426: AGENT_API_KEY now sourced from envGood. Ensure it’s set in all deploy envs; otherwise wallet spin-up may break.
1903-1937: Auto-provision tenant token path looks correctSolid flow to derive and persist tenant API key when missing for SHARED agents.
1058-1064: Log contains full URLFine for debug. Ensure no tokens are included.
529-540: No action needed: protocol is already handled
getBaseAgentTokencallsnormalizeUrlWithProtocol, which prependsprocess.env.API_GATEWAY_PROTOCOLwhen the endpoint lacks a scheme, ensuring a full URL is used.Likely an incorrect or invalid review comment.
| INBOUND_ENDPOINT=${16} | ||
| SCHEMA_FILE_SERVER_URL=${17} | ||
| AGENT_API_KEY=${18} | ||
| ADMIN_PORT_FILE="$PWD/agent-provisioning/AFJ/port-file/last-admin-port.txt" | ||
| INBOUND_PORT_FILE="$PWD/agent-provisioning/AFJ/port-file/last-inbound-port.txt" | ||
| ADMIN_PORT=8001 | ||
| INBOUND_PORT=9001 | ||
|  | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
AGENT_API_KEY added: ensure it’s used consistently
You accept AGENT_API_KEY but do not mask it in logs and do not validate presence.
Run-time check:
🏁 Script executed:
#!/bin/sh
if [ -z "$AGENT_API_KEY" ]; then echo "AGENT_API_KEY is empty"; exit 1; fiLength of output: 79
Validate and mask AGENT_API_KEY
- In apps/agent-provisioning/AFJ/scripts/docker_start_agent.sh, immediately after
 AGENT_API_KEY=${18}add a guard:if [ -z "$AGENT_API_KEY" ]; then echo "error: AGENT_API_KEY is required" >&2 exit 1 fi 
- Ensure no logs print the raw API key—mask it (e.g. echo "AGENT_API_KEY=****").
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 20-20: INBOUND_ENDPOINT appears unused. Verify use (or export if used externally).
(SC2034)
🤖 Prompt for AI Agents
In apps/agent-provisioning/AFJ/scripts/docker_start_agent.sh around lines 20 to
27, after the AGENT_API_KEY=${18} assignment add a guard that exits with a
non-zero status and writes an error to stderr if AGENT_API_KEY is empty, and
change any subsequent logging so the raw key is never printed (log a masked
value like "AGENT_API_KEY=****" instead); implement the guard and replace or
wrap any echo/printf that would output the key with a masked string.
| # Extract the token from the logs using sed | ||
| token=$(echo "$container_logs" | sed -nE 's/.*API Token: ([^ ]+).*/\1/p') | ||
| token=$(echo "$container_logs" | sed -nE 's/.*** API Key: ([^ ]+).*/\1/p') | ||
|  | ||
| # Print the extracted token | ||
| echo "Token: $token" | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix fragile token extraction
The sed regex with unescaped asterisks is unreliable. Extract by key name.
-token=$(echo "$container_logs" | sed -nE 's/.*** API Key: ([^ ]+).*/\1/p')
+token=$(printf "%s" "$container_logs" | awk -F'API Key: ' '/API Key:/ {print $2}' | awk '{print $1; exit}')Also avoid echoing the token.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/agent-provisioning/AFJ/scripts/docker_start_agent.sh around lines 231 to
235, the sed expression using unescaped asterisks is fragile and the script
prints the token to stdout; change extraction to reliably parse by the literal
key name and capture the following non-whitespace chunk (for example use grep
-oP 'API Key:\s*\K\S+' or awk -F'API Key: ' '{print $2}' | awk '{print $1}'),
assign that to the token variable, and remove the echo that prints the token so
it is not written to stdout or logs.
| "name": "$CONTAINER_NAME", | ||
| "image": "${AFJ_IMAGE_URL}", | ||
| "image": "${AFJ_VERSION}", | ||
| "cpu": 154, | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fail fast when AFJ_VERSION is unset
Empty image fields cause ECS failures.
-    "image": "${AFJ_VERSION}",
+    "image": "${AFJ_VERSION}",Add before building JSON:
: "${AFJ_VERSION:?AFJ_VERSION is required and must be a full image URI}"🤖 Prompt for AI Agents
In apps/agent-provisioning/AFJ/scripts/start_agent_ecs.sh around lines 156-158,
the container image field uses ${AFJ_VERSION} which can be empty and causes ECS
failures; add a shell fail-fast check before building the JSON such as a
parameter expansion that exits with a clear message when AFJ_VERSION is unset or
empty (i.e. use : "${AFJ_VERSION:?AFJ_VERSION is required and must be a full
image URI}") so the script fails early with a helpful error instead of producing
an empty image field.
| fi | ||
| done | ||
| # Describe the ECS service and filter by service name | ||
| service_description=$(aws ecs describe-services --service $SERVICE_NAME --cluster $CLUSTER_NAME --region $AWS_PUBLIC_REGION) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use --services (plural) in aws ecs describe-services
CLI requires “--services”.
-service_description=$(aws ecs describe-services --service $SERVICE_NAME --cluster $CLUSTER_NAME --region $AWS_PUBLIC_REGION)
+service_description=$(aws ecs describe-services --services "$SERVICE_NAME" --cluster "$CLUSTER_NAME" --region "$AWS_PUBLIC_REGION")📝 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.
| service_description=$(aws ecs describe-services --service $SERVICE_NAME --cluster $CLUSTER_NAME --region $AWS_PUBLIC_REGION) | |
| service_description=$(aws ecs describe-services --services "$SERVICE_NAME" --cluster "$CLUSTER_NAME" --region "$AWS_PUBLIC_REGION") | 
🤖 Prompt for AI Agents
In apps/agent-provisioning/AFJ/scripts/start_agent_ecs.sh around line 275, the
aws cli call uses the incorrect flag “--service”; change it to the required
plural “--services” and pass the SERVICE_NAME (and other variables) safely
(quoted) so the describe-services call succeeds; update the command to use
--services $SERVICE_NAME (preferably --services "$SERVICE_NAME") and ensure
consistency with other flags.
| if (agentType === AgentType.AFJ) { | ||
| // The wallet provision command is used to invoke a shell script | ||
| const walletProvision = `${process.cwd() + process.env.AFJ_AGENT_SPIN_UP} ${orgId} "${externalIp}" "${walletName}" "${walletPassword}" ${seed} ${webhookEndpoint} ${walletStorageHost} ${walletStoragePort} ${walletStorageUser} ${walletStoragePassword} ${containerName} ${protocol} ${tenant} ${credoImage} "${indyLedger}" ${inboundEndpoint} ${process.env.SCHEMA_FILE_SERVER_URL} ${process.env.AGENT_HOST} ${process.env.AWS_ACCOUNT_ID} ${process.env.S3_BUCKET_ARN} ${process.env.CLUSTER_NAME} ${process.env.TESKDEFINITION_FAMILY}`; | ||
| const walletProvision = `${process.cwd() + process.env.AFJ_AGENT_SPIN_UP} ${orgId} "${externalIp}" "${walletName}" "${walletPassword}" ${seed} ${webhookEndpoint} ${walletStorageHost} ${walletStoragePort} ${walletStorageUser} ${walletStoragePassword} ${containerName} ${protocol} ${tenant} ${credoImage} "${indyLedger}" ${inboundEndpoint} ${process.env.SCHEMA_FILE_SERVER_URL} ${process.env.AGENT_API_KEY} ${process.env.AGENT_HOST} ${process.env.AWS_ACCOUNT_ID} ${process.env.S3_BUCKET_ARN} ${process.env.CLUSTER_NAME} ${process.env.TASKDEFINITION_FAMILY}`; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid command injection: use execFile/spawn with args array and validate/escape inputs
Interpolating env/payload into a shell string invites injection and quoting bugs. Build an argv list and pass via execFile/spawn; also validate untrusted fields (walletName, walletPassword, seed, containerName, etc.). Additionally, resolve the script path from a trusted base to address the CodeQL “uncontrolled absolute path” warning.
Apply something like:
-const walletProvision = `${process.cwd() + process.env.AFJ_AGENT_SPIN_UP} ${orgId} "${externalIp}" "${walletName}" "${walletPassword}" ${seed} ${webhookEndpoint} ${walletStorageHost} ${walletStoragePort} ${walletStorageUser} ${walletStoragePassword} ${containerName} ${protocol} ${tenant} ${credoImage} "${indyLedger}" ${inboundEndpoint} ${process.env.SCHEMA_FILE_SERVER_URL} ${process.env.AGENT_API_KEY} ${process.env.AGENT_HOST} ${process.env.AWS_ACCOUNT_ID} ${process.env.S3_BUCKET_ARN} ${process.env.CLUSTER_NAME} ${process.env.TASKDEFINITION_FAMILY}`;
+import * as path from 'path';
+const scriptPath = path.resolve(process.cwd(), process.env.AFJ_AGENT_SPIN_UP ?? '');
+const args = [
+  A String(orgId), String(externalIp), String(walletName), String(walletPassword), String(seed),
+  String(webhookEndpoint), String(walletStorageHost), String(walletStoragePort), String(walletStorageUser),
+  String(walletStoragePassword), String(containerName), String(protocol), String(tenant), String(credoImage),
+  String(indyLedger), String(inboundEndpoint), String(process.env.SCHEMA_FILE_SERVER_URL ?? ''),
+  String(process.env.AGENT_API_KEY ?? ''), String(process.env.AGENT_HOST ?? ''), String(process.env.AWS_ACCOUNT_ID ?? ''),
+  String(process.env.S3_BUCKET_ARN ?? ''), String(process.env.CLUSTER_NAME ?? ''), String(process.env.TASKDEFINITION_FAMILY ?? '')
+];…and execute with:
import { spawn } from 'child_process';
const child = spawn(scriptPath, args, { stdio: 'pipe', shell: false });🤖 Prompt for AI Agents
In apps/agent-provisioning/src/agent-provisioning.service.ts around line 42, the
code builds a long shell command by interpolating env and payload values which
permits command injection and path traversal; instead, resolve the target script
path against a trusted base directory (e.g., path.resolve(TRUSTED_BASE,
process.env.AFJ_AGENT_SPIN_UP)) and verify it remains inside that base, then
construct an argv array with each parameter as a separate element (walletName,
walletPassword, seed, orgId, externalIp, webhookEndpoint, walletStorageHost,
walletStoragePort, walletStorageUser, walletStoragePassword, containerName,
protocol, tenant, credoImage, indyLedger, inboundEndpoint, schema server url,
agent api key, agent host, aws account id, s3 bucket arn, cluster name, taskdef
family) and call child_process.spawn/execFile with shell:false and stdio
appropriately; additionally validate and sanitize all untrusted inputs (length,
allowed chars, encoding) before adding them to the args array and avoid
concatenating any user/proc env into a single shell string.
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com>
fix: Badge verification page issue
There was a problem hiding this 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️  Outside diff range comments (2)
apps/api-gateway/src/agent-service/agent-service.controller.ts (2)
164-168: Harden input: use DTO-based validation and UUID parsing for orgIdThis route currently takes
IVerifySignature(interface → no runtime validation) and a raworgIdstring. Switch to the DTO used in Swagger and add the same pipes you apply in deleteWallet.[suggested fix — enable validation + strict orgId parsing]
- async verifysignature( - @Param('orgId') orgId: string, - @Body() data: IVerifySignature, - @Res() res: Response - ): Promise<Response> { + @UsePipes(new ValidationPipe({ whitelist: true, forbidNonWhitelisted: true })) + async verifysignature( + @Param( + 'orgId', + TrimStringParamPipe, + new ParseUUIDPipe({ + exceptionFactory: (): Error => { + throw new BadRequestException(ResponseMessages.organisation.error.invalidOrgId); + } + }) + ) + orgId: string, + @Body() data: VerifySignatureDto, + @Res() res: Response + ): Promise<Response> {Add imports at the top:
// from '@nestjs/common' import { UsePipes, ValidationPipe } from '@nestjs/common';If
agentService.verifysignature(...)expectsIVerifySignature,VerifySignatureDtoshould be structurally compatible; otherwise update the service signature accordingly.
331-337: Fix seed length check (should allow up to 32, not exactly 32)As written, non-32-length seeds are rejected, contradicting the message “at most 32”.
- if (createDidDto.seed && seedLength !== createDidDto.seed.length) { - this.logger.error(`seed must be at most 32 characters.`); + if (createDidDto.seed && createDidDto.seed.length > seedLength) { + this.logger.error(`seed must be at most 32 characters.`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
- apps/api-gateway/src/agent-service/agent-service.controller.ts(1 hunks)
| // @UseGuards(AuthGuard('jwt'), OrgRolesGuard) | ||
| // @Roles( | ||
| // OrgRoles.OWNER, | ||
| // OrgRoles.ADMIN, | ||
| // OrgRoles.HOLDER, | ||
| // OrgRoles.ISSUER, | ||
| // OrgRoles.SUPER_ADMIN, | ||
| // OrgRoles.MEMBER, | ||
| // OrgRoles.VERIFIER | ||
| // ) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Security regression: verify-signature endpoint lost auth/role enforcement
Commenting out @UseGuards(AuthGuard('jwt'), OrgRolesGuard) and @Roles(...) makes this endpoint publicly callable and orgId-enumerable. Unless intentionally public, this is a blocking security risk. Re-enable guards (or replace with the new guard you introduced elsewhere) to restore protection. If it is intended to be public, adjust Swagger (class-level @ApiBearerAuth() currently misleads) and add rate limiting/WAF.
[suggested fix — restore guards]
-  // @UseGuards(AuthGuard('jwt'), OrgRolesGuard)
-  // @Roles(
-  //   OrgRoles.OWNER,
-  //   OrgRoles.ADMIN,
-  //   OrgRoles.HOLDER,
-  //   OrgRoles.ISSUER,
-  //   OrgRoles.SUPER_ADMIN,
-  //   OrgRoles.MEMBER,
-  //   OrgRoles.VERIFIER
-  // )
+  @UseGuards(AuthGuard('jwt'), OrgRolesGuard)
+  @Roles(
+    OrgRoles.OWNER,
+    OrgRoles.ADMIN,
+    OrgRoles.HOLDER,
+    OrgRoles.ISSUER,
+    OrgRoles.SUPER_ADMIN,
+    OrgRoles.MEMBER,
+    OrgRoles.VERIFIER
+  )Run to confirm no other accidental public endpoints and whether SessionGuard is meant here:
🏁 Script executed:
#!/bin/bash
rg -nC2 '(verify-signature|@UseGuards|SessionGuard)' apps/api-gateway/src/agent-service/agent-service.controller.tsLength of output: 3293
Restore authentication and RBAC guards on verify-signature endpoint
The @Post('/orgs/:orgId/agents/verify-signature') handler has its @UseGuards(AuthGuard('jwt'), OrgRolesGuard) and @Roles(...) decorators commented out (lines 154–163), exposing it publicly. Re-enable those decorators—or apply the intended SessionGuard if that’s your new guard—to prevent unauthorized access.
🤖 Prompt for AI Agents
In apps/api-gateway/src/agent-service/agent-service.controller.ts around lines
154 to 163, the authentication and RBAC decorators for the POST
/orgs/:orgId/agents/verify-signature handler are commented out, leaving the
endpoint public; restore the appropriate guard and role decorators by
re-enabling @UseGuards(AuthGuard('jwt'), OrgRolesGuard) and the @Roles(...)
list, or replace them with the intended @UseGuards(SessionGuard, OrgRolesGuard)
and same @Roles(...) if SessionGuard is the new auth mechanism, ensuring the
endpoint enforces authentication and the original role checks.
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
Fixed resolve sonar cloud issues
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
fix:removed sonar cloud issue related changes
Sync Develop branch into QA
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️  Outside diff range comments (2)
apps/cloud-wallet/src/cloud-wallet.service.ts (1)
257-262: Fix condition: use OR, not ANDThis can miss invalid responses when object exists but id is falsy.
-if (!createCloudWalletResponse && !createCloudWalletResponse.id) { +if (!createCloudWalletResponse || !createCloudWalletResponse.id) {apps/agent-service/src/agent-service.service.ts (1)
1323-1331: Inject tenantId into SHARED credential-definition create URL
In apps/agent-service/src/agent-service.service.ts around line 1323, update as follows:- const url = `${payload.agentEndPoint}${CommonConstants.URL_SCHM_CREATE_CRED_DEF}`; + const url = `${payload.agentEndPoint}${CommonConstants.URL_SCHM_CREATE_CRED_DEF}`.replace( + '#', + `${payload.tenantId}` + );
♻️ Duplicate comments (16)
apps/issuance/src/issuance.service.ts (2)
284-295: Don’t drop schemaId update promises; also map schemaId per record, not from the first credential.
- Repos updates are fire-and-forget; errors get lost.
- Using payload.credentialData[0] applies the first schemaId to all records—wrong if batch mixes schemas.
Apply this refactor to await and map schemaId by index:
- const context = payload?.credentialData[0]?.credential?.['@context'] as string[]; - - if (Array.isArray(context) && context.includes(CommonConstants.W3C_SCHEMA_URL)) { - const filterData = context.filter((item) => CommonConstants.W3C_SCHEMA_URL !== item); - const [schemaId] = filterData; - results.forEach((record) => { - if (PromiseResult.FULFILLED === record.status && record?.value?.threadId) { - this.issuanceRepository.updateSchemaIdByThreadId(record?.value?.threadId, schemaId); - } - }); - } + const contextsByIndex = + payload?.credentialData?.map((cd) => cd?.credential?.['@context'] as string[] | undefined) ?? []; + const updates = results + .map((record, i) => ({ record, i })) + .filter(({ record }) => PromiseResult.FULFILLED === record.status && record?.value?.threadId) + .map(({ record, i }) => { + const ctx = contextsByIndex[i]; + if (!Array.isArray(ctx) || !ctx.includes(CommonConstants.W3C_SCHEMA_URL)) return null; + const [schemaId] = ctx.filter((c) => c !== CommonConstants.W3C_SCHEMA_URL); + return this.issuanceRepository.updateSchemaIdByThreadId(record.value.threadId, schemaId); + }) + .filter(Boolean) as Promise<unknown>[]; + const updateResults = await Promise.allSettled(updates); + const failed = updateResults.filter((r) => r.status === 'rejected').length; + if (failed) { + this.logger.warn(`[sendCredentialCreateOffer] schemaId updates failed: ${failed}/${updateResults.length}`); + }
1078-1091: Await schemaId update after email send.Ensure persistence completes and surface errors.
- if (isEmailSent) { - const w3cSchemaId = outOfBandIssuancePayload?.credentialFormats?.jsonld?.credential?.['@context'] as string[]; - if (w3cSchemaId?.includes(CommonConstants.W3C_SCHEMA_URL)) { - const filterData = w3cSchemaId.filter((item) => CommonConstants.W3C_SCHEMA_URL !== item); - const [schemaId] = filterData; - if (credentialCreateOfferDetails.response.credentialRequestThId) { - this.issuanceRepository.updateSchemaIdByThreadId( - credentialCreateOfferDetails.response.credentialRequestThId, - schemaId - ); - } - } - } + if (isEmailSent) { + const context = outOfBandIssuancePayload?.credentialFormats?.jsonld?.credential?.['@context'] as string[]; + if (Array.isArray(context) && context.includes(CommonConstants.W3C_SCHEMA_URL)) { + const [schemaId] = context.filter((item) => item !== CommonConstants.W3C_SCHEMA_URL); + if (credentialCreateOfferDetails.response.credentialRequestThId) { + await this.issuanceRepository.updateSchemaIdByThreadId( + credentialCreateOfferDetails.response.credentialRequestThId, + schemaId + ); + } + } + }apps/user/templates/user-email-template.ts (2)
10-12: Guard URL creation with try/catch, validate FRONT_END_URL, and preserve error details
new URL()runs before the try/catch; missing/invalid FRONT_END_URL will throw uncaught. Also the rethrow loses the original error context. Move URL construction inside try, validate scheme, and include the original error message.- const baseDomain = `${process.env.FRONT_END_URL}`; - const apiUrl = new URL('/verify-email-success', baseDomain); + try { + const baseDomain = process.env.FRONT_END_URL; + if (!baseDomain) { + throw new Error('FRONT_END_URL is not configured'); + } + const parsedBase = new URL(baseDomain); // validates absolute URL + if (!/^https?:$/.test(parsedBase.protocol)) { + throw new Error('FRONT_END_URL must include http(s) scheme'); + } + const apiUrl = new URL('/verify-email-success', parsedBase); @@ - try { + // html below... @@ - } catch (error) { - throw new Error('Error creating email verification template'); - } + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + throw new Error(`Error creating email verification template: ${message}`); + }Also applies to: 29-31, 79-81
14-14: Avoid double-encoding the email paramURLSearchParams encodes values;
encodeURIComponenthere producestest%2540example.com.- apiUrl.searchParams.append('email', encodeURIComponent(email)); + apiUrl.searchParams.append('email', email);libs/common/src/interfaces/user.interface.ts (1)
66-67: Use Date instead of string for timestamp fields (align with Prisma)Prisma returns Date for timestamp columns; typing as string causes runtime/type drift across services.
Apply this diff:
export interface IClientAliases { id: string; - createDateTime: string; - lastChangedDateTime: string; + createDateTime: Date; + lastChangedDateTime: Date; clientAlias: string; clientUrl: string; }apps/user/repositories/user.repository.ts (1)
151-157: Validate session expiry and return a typed valueMethod returns loose object and skips expiration; enforce expiry and return Session | null.
- async validateSession(sessionId: string): Promise<object> { - const session = await this.prisma.session.findUnique({ - where: { id: sessionId }, - include: { user: true } - }); - return session; - } + async validateSession(sessionId: string): Promise<session | null> { + const sess = await this.prisma.session.findUnique({ + where: { id: sessionId }, + include: { user: true } + }); + if (!sess) return null; + // Support new expiresAt (Date) or legacy expires (number) + if ((sess as any).expiresAt && new Date() > (sess as any).expiresAt) return null; + if ((sess as any).expires && Date.now() > Number((sess as any).expires)) return null; + return sess; + }apps/organization/src/organization.service.ts (3)
1341-1354: Same null-entry risk in rolesPayload mappingMirror the fix used above to avoid nulls and enforce strict matching.
- const rolesPayload: { roleId: string; name: string; idpRoleId: string }[] = matchedClientRoles.map( + const rolesPayload = matchedClientRoles.map( (clientRole: IClientRoles) => { let roleObj: { roleId: string; name: string; idpRoleId: string } = null; for (let index = 0; index < orgRoles.length; index++) { if (orgRoles[index].name === clientRole.name) { roleObj = { roleId: orgRoles[index].id, name: orgRoles[index].name, idpRoleId: clientRole.id }; break; } } return roleObj; } ); + if (rolesPayload.some((r) => !r)) { + throw new NotFoundException(ResponseMessages.organisation.error.orgRoleIdNotFound); + }
722-729: Await session cleanup before enforcing limitAvoid race with immediate session count; await deletion.
- this.userRepository.deleteInactiveSessions(orgRoleDetails['user'].id); + await this.userRepository.deleteInactiveSessions(orgRoleDetails['user'].id);
1219-1234: rolesPayload may contain null entries; validate or filter before useA missing match yields null and breaks downstream maps.
- const rolesPayload: { roleId: string; name: string; idpRoleId: string }[] = orgRoles.map((orgRole: IOrgRole) => { + const rolesPayload = orgRoles.map((orgRole: IOrgRole) => { let roleObj: { roleId: string; name: string; idpRoleId: string } = null; for (let index = 0; index < clientRolesList.length; index++) { if (clientRolesList[index].name === orgRole.name) { roleObj = { roleId: orgRole.id, name: orgRole.name, idpRoleId: clientRolesList[index].id }; break; } } return roleObj; }); + if (rolesPayload.some((r) => !r)) { + throw new NotFoundException(ResponseMessages.organisation.error.orgRoleIdNotFound); + }apps/user/src/user.service.ts (3)
115-116: Validate clientAlias before use to avoid runtime crash.getCredentialsByAlias calls alias.toUpperCase(); undefined/empty clientAlias will throw. Guard and 400.
- const { email, brandLogoUrl, platformName, clientAlias } = userEmailVerification; + const { email, brandLogoUrl, platformName, clientAlias } = userEmailVerification; + if (!clientAlias || typeof clientAlias !== 'string' || !clientAlias.trim()) { + throw new BadRequestException('clientAlias is required'); + } ... - const clientDetails = await getCredentialsByAlias(clientAlias); + const clientDetails = await getCredentialsByAlias(clientAlias);Also applies to: 138-138
453-459: Await session cleanup and parse SESSIONS_LIMIT safely.Unawaited deleteInactiveSessions can race; Number(env) can be NaN and bypass limit.
- // called seprate method to delete exp session - this.userRepository.deleteInactiveSessions(userData?.id); + // called separate method to delete expired sessions + await this.userRepository.deleteInactiveSessions(userData.id); const userSessionDetails = await this.userRepository.fetchUserSessions(userData?.id); - if (Number(process.env.SESSIONS_LIMIT) <= userSessionDetails?.length) { + const SESSIONS_LIMIT = toNumber(String(process.env.SESSIONS_LIMIT), { default: 5, min: 1 }); + if (SESSIONS_LIMIT <= (userSessionDetails?.length ?? 0)) { throw new BadRequestException(ResponseMessages.user.error.sessionLimitReached); }
571-574: Include new sessionId in refresh response for API consistency.A new session is created, but sessionId isn’t returned.
- return tokenResponse; + return { ...tokenResponse, sessionId: addSessionDetails.id };apps/cloud-wallet/src/cloud-wallet.service.ts (2)
155-160: Bug: stray “}” and missing encoding -> 404 riskRemove the extra brace and encode the ID (same issue noted earlier).
-const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${proofRecordId}}`; +const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${encodeURIComponent(proofRecordId)}`;
176-181: Malformed query string and stray “}”Avoid “/” before “?” and encode threadId; remove trailing brace.
-const threadParam = threadId ? `?threadId=${threadId}` : ''; -const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${threadParam}}`; +const threadParam = threadId ? `?threadId=${encodeURIComponent(threadId)}` : ''; +const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}${threadParam}`;apps/agent-service/src/agent-service.service.ts (2)
1120-1122: Fix wallet creation guard; current AND chain lets broken responses pass.The check only throws when all three are falsy. It should throw if any required field is missing. This was previously flagged.
- if (!walletResponseDetails && !walletResponseDetails.id && !walletResponseDetails.token) { + if (!walletResponseDetails || !walletResponseDetails.id || !walletResponseDetails.token) { throw new InternalServerErrorException('Error while creating the wallet'); } @@ - apiKey: walletResponseDetails.token + apiKey: walletResponseDetails.tokenAlso applies to: 1126-1127
1785-1795: Shared-agent branch always taken; condition uses enum constant instead of runtime value.The if (OrgAgentType.SHARED) is truthy regardless of agent type, causing wrong API key selection. Previously flagged.
- let getApiKey; - if (OrgAgentType.SHARED) { + let getApiKey; + if (orgAgentTypeResult.agent === OrgAgentType.SHARED) { const platformAdminSpinnedUp = await this.agentServiceRepository.platformAdminAgent( CommonConstants.PLATFORM_ADMIN_ORG ); - getApiKey = await this.commonService.decryptPassword(platformAdminSpinnedUp?.org_agents[0].apiKey); + const platformAdminApiKeyEnc = platformAdminSpinnedUp?.org_agents?.[0]?.apiKey; + if (!platformAdminApiKeyEnc) { + throw new NotFoundException(ResponseMessages.agent.error.apiKeyNotExist); + } + getApiKey = await this.commonService.decryptPassword(platformAdminApiKeyEnc); } else { getApiKey = getApiKeyResult?.value; }
🧹 Nitpick comments (25)
apps/user/templates/user-email-template.ts (4)
28-29: Fix poweredBy fallback to avoid overriding brandingCurrent logic makes poweredBy equal to platformName when provided, ignoring POWERED_BY. Prefer POWERED_BY when set, else fall back to platform.
- const poweredBy = platformName || process.env.POWERED_BY; + const poweredBy = process.env.POWERED_BY || platform;
43-43: Clean up non-standard/fragileattributes for email clients
Remove Gmail-specific attrs and invalid/fragile width/height values; rely on inline style.
- <img src="${logoUrl}" alt="${platform} logo" style="max-width:100px; background: white; padding: 5px;border-radius: 5px;" width="100%" height="fit-content" class="CToWUd" data-bit="iit"> + <img src="${logoUrl}" alt="${platform} logo" style="max-width:100px; background: white; padding: 5px; border-radius: 5px; height: auto;">
47-47: Fix invalid hex color
#00000is not valid; use#000000.- <div style="font-family: Montserrat; font-style: normal; font-weight: 500; - font-size: 15px; line-height: 24px;color: #00000;"> + <div style="font-family: Montserrat; font-style: normal; font-weight: 500; + font-size: 15px; line-height: 24px;color: #000000;">
68-68: Prevent “undefined” support email in production contentIf PUBLIC_PLATFORM_SUPPORT_EMAIL is not set, this renders “undefined”. Either default it or fail early.
Option A (default):
- For any assistance or questions while accessing your account, please do not hesitate to contact the support team at ${process.env.PUBLIC_PLATFORM_SUPPORT_EMAIL}. Our team will ensure a seamless onboarding experience for you. + For any assistance or questions while accessing your account, please do not hesitate to contact the support team at ${process.env.PUBLIC_PLATFORM_SUPPORT_EMAIL ?? ''}. Our team will ensure a seamless onboarding experience for you.Option B (fail fast): add a check near other env reads and throw with a clear message when missing.
libs/common/src/interfaces/user.interface.ts (1)
72-81: Consolidate verification email DTOs or document their distinct usageIVerificationEmail and IVerifyUserEmail overlap; either merge or clearly separate their purposes to avoid confusion at call sites.
apps/user/repositories/user.repository.ts (2)
55-61: Misleading log messageLog context says checkUserExist inside fetchClientAliases; update message to match method for easier triage.
- this.logger.error(`checkUserExist: ${JSON.stringify(error)}`); + this.logger.error(`fetchClientAliases: ${JSON.stringify(error)}`);
991-1007: Add DB index for session cleanup pathdeleteMany filters on userId + expiresAt; add a compound index (userId, expiresAt) in Prisma for efficiency at scale.
apps/organization/src/organization.service.ts (3)
726-728: Use cast helper for env numeric comparisonsNumber(process.env.SESSIONS_LIMIT) can be NaN; prefer toNumber helper with sensible default.
- if (Number(process.env.SESSIONS_LIMIT) <= userSessionDetails?.length) { + if (toNumber(`${process.env.SESSIONS_LIMIT}`, { default: 0 }) <= userSessionDetails?.length) { throw new BadRequestException(ResponseMessages.user.error.sessionLimitReached); }
1634-1636: Fix ESLint implicit-arrow-linebreakKeep returned expression on same line to satisfy rule.
- const deleteUserRolesPromises = keycloakUserIds.map((keycloakUserId) => - this.clientRegistrationService.deleteUserClientRoles(organizationDetails?.idpId, token, keycloakUserId) - ); + const deleteUserRolesPromises = keycloakUserIds.map((keycloakUserId) => + this.clientRegistrationService.deleteUserClientRoles(organizationDetails?.idpId, token, keycloakUserId) + );Alternatively:
- const deleteUserRolesPromises = keycloakUserIds.map((keycloakUserId) => - this.clientRegistrationService.deleteUserClientRoles(organizationDetails?.idpId, token, keycloakUserId) - ); + const deleteUserRolesPromises = keycloakUserIds.map((keycloakUserId) => { + return this.clientRegistrationService.deleteUserClientRoles(organizationDetails?.idpId, token, keycloakUserId); + });
1667-1674: Avoid confusing shadowing with sendEmail local variableRename to emailSent to avoid clashing with imported sendEmail symbol and improve readability.
- const sendEmail = await this.sendEmailForOrgInvitationsMember( + const emailSent = await this.sendEmailForOrgInvitationsMember( userDetails?.email, organizationDetails?.name, orgRoleNames ); ... - this.logger.log( - `email: ${userDetails.email}, orgName: ${organizationDetails?.name}, orgRoles: ${JSON.stringify(orgRoleNames)}, sendEmail: ${sendEmail}` + this.logger.log( + `email: ${userDetails.email}, orgName: ${organizationDetails?.name}, orgRoles: ${JSON.stringify(orgRoleNames)}, emailSent: ${emailSent}` );apps/user/src/user.service.ts (7)
71-71: Avoid cross-app imports; move utility to shared lib.Importing from apps/api-gateway couples services. Extract getCredentialsByAlias into a shared library (e.g., libs/auth or libs/common) and import from there.
91-105: Fix misleading log message.Change "In Create User" to "In getClientAliases" to aid diagnostics.
- this.logger.error(`In Create User : ${JSON.stringify(error)}`); + this.logger.error(`In getClientAliases : ${JSON.stringify(error)}`);
320-325: Deduplicate Keycloak user creation logic.createUser is called identically in both branches. Factor it out to a small helper to reduce divergence.
- try { - keycloakDetails = await this.clientRegistrationService.createUser( - userInfo, - process.env.KEYCLOAK_REALM, - token - ); - } catch (error) { + try { + keycloakDetails = await this.clientRegistrationService.createUser(userInfo, process.env.KEYCLOAK_REALM, token); + } catch (error) { throw new InternalServerErrorException('Error while registering user on keycloak'); }Also applies to: 333-339
513-524: Be resilient when decoding sessionId.Double decode + decryptPassword is brittle. Consider single decode with try/catch and a dedicated decrypt helper (not password-specific) to avoid semantic confusion.
- const onceDecoded = decodeURIComponent(sessionId); - const decodedSessionId = decodeURIComponent(onceDecoded); - const decryptedSessionId = await this.commonService.decryptPassword(decodedSessionId); + let decryptedSessionId: string; + try { + const decoded = decodeURIComponent(sessionId); + decryptedSessionId = await this.commonService.decrypt(decoded); + } catch { + throw new BadRequestException(ResponseMessages.user.error.invalidSession); + }
826-829: Log Supabase error only when present and avoid noisy error level.Currently always logs at error level. Log conditionally and downgrade to warn/debug.
- this.logger.error(`Supa Login Error::`, JSON.stringify(error)); + if (error) { + this.logger.warn(`Supabase login failed: ${error.message ?? error}`); + }
866-885: Prefer firstValueFrom over deprecated toPromise().Modern RxJS/NestJS recommend firstValueFrom.
+import { firstValueFrom } from 'rxjs'; ... - const getEcosystemConfigDetails = await this.userServiceProxy - .send(pattern, payload) - .toPromise() + const getEcosystemConfigDetails = await firstValueFrom( + this.userServiceProxy.send(pattern, payload) + ) .catch((error) => {
1326-1333: Align logout response with standardized messages and validate input.Return a ResponseMessages constant and validate sessions is a non-empty string array.
- if (logoutUserDto?.sessions) { + if (Array.isArray(logoutUserDto?.sessions) && logoutUserDto.sessions.length) { await this.userRepository.destroySession(logoutUserDto?.sessions); } - - return 'user logged out successfully'; + return ResponseMessages.user.success.logout;apps/cloud-wallet/src/cloud-wallet.service.ts (4)
101-111: Connection creation flow looks good; consider URL join + auth header helperLogic is sound. To reduce duplication and slash bugs across methods, add small helpers and use them here and elsewhere.
Add inside the class:
private joinUrl(base: string, path: string) { return `${base.replace(/\/+$/,'')}/${path.replace(/^\/+/,'')}`; } private authHeaders(token: string) { return { headers: { authorization: token } }; }Then:
-const url = `${agentEndpoint}${CommonConstants.URL_CONN_INVITE}`; -const createConnectionDetails = await this.commonService.httpPost(url, connectionPayload, { - headers: { authorization: decryptedApiKey } -}); +const url = this.joinUrl(agentEndpoint, CommonConstants.URL_CONN_INVITE); +const createConnectionDetails = await this.commonService.httpPost(url, connectionPayload, this.authHeaders(decryptedApiKey));
352-356: Accept offer: OK; apply URL-join helper when addedLooks fine; switch to joinUrl/authHeaders if you adopt the helper.
395-399: Create DID: OK; apply URL-join helper when addedAll good.
106-110: Deduplicate URL concat and headersLots of repeated string concat and header objects; helpers reduce error-prone copy/paste.
Apply the joinUrl/authHeaders helper suggested earlier across these call sites.
Also applies to: 130-130, 157-157, 179-179, 312-312, 355-355, 398-398, 435-435, 456-456, 487-487, 517-517, 539-539, 562-562, 585-585
apps/agent-service/src/agent-service.service.ts (4)
522-528: Avoid shadowing the socket parameter.Local const socket shadows the method parameter, increasing confusion and risk of emitting on the wrong instance.
- const socket = await this.initSocketConnection(`${process.env.SOCKET_HOST}`); + const localSocket = await this.initSocketConnection(`${process.env.SOCKET_HOST}`); - if (agentEndPoint && agentSpinupDto.clientSocketId) { - socket.emit('agent-spinup-process-completed', { clientId: agentSpinupDto.clientSocketId }); - socket.emit('did-publish-process-initiated', { clientId: agentSpinupDto.clientSocketId }); - socket.emit('invitation-url-creation-started', { clientId: agentSpinupDto.clientSocketId }); + if (agentEndPoint && agentSpinupDto.clientSocketId) { + localSocket.emit('agent-spinup-process-completed', { clientId: agentSpinupDto.clientSocketId }); + localSocket.emit('did-publish-process-initiated', { clientId: agentSpinupDto.clientSocketId }); + localSocket.emit('invitation-url-creation-started', { clientId: agentSpinupDto.clientSocketId }); }
606-611: Persisting empty strings for did/verkey; consider nulls and schema constraints.Storing '' for did/verkey can be semantically ambiguous and complicates “is set” checks. Prefer nulls unless DB constraints require non-null.
- const orgAgentTypeId = await this.agentServiceRepository.getOrgAgentTypeDetails(OrgAgentType.DEDICATED); + const orgAgentTypeId = await this.agentServiceRepository.getOrgAgentTypeDetails(OrgAgentType.DEDICATED); @@ - private _buildStoreOrgAgentData(payload: IStoreOrgAgentDetails, orgAgentTypeId: string): IStoreOrgAgentDetails { + private _buildStoreOrgAgentData(payload: IStoreOrgAgentDetails, orgAgentTypeId: string): IStoreOrgAgentDetails { return { - did: '', - verkey: '', + did: null, + verkey: null, isDidPublic: true,Also applies to: 622-637
978-980: Consider using getAgentUrl for DID/write too (for consistency).Keeps URL mapping uniform and reduces private helpers.
- private constructUrl(agentDetails): string { - return `${agentDetails.agentEndPoint}${CommonConstants.URL_AGENT_WRITE_DID}`; - } + private constructUrl(agentDetails): string { + // if CommonConstants.URL_AGENT_WRITE_DID is mapped in getAgentUrl + return `${agentDetails.agentEndPoint}${CommonConstants.URL_AGENT_WRITE_DID}`; + // Alternatively: + // return await getAgentUrl(agentDetails.agentEndPoint, CommonConstants.URL_AGENT_WRITE_DID); + }
1903-1937: Shared-tenant token backfill logic looks right; polish log message.Flow to fetch tenant token when shared agent has no apiKey is solid. The error log in getTenantToken mentions “org agent type”; update to reflect the method context.
- this.logger.error(`Error in getting org agent type : ${JSON.stringify(error)}`); + this.logger.error(`Error in getTenantToken : ${JSON.stringify(error)}`);Also applies to: 1944-1967
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
- apps/agent-service/src/agent-service.service.ts(22 hunks)
- apps/api-gateway/src/authz/guards/session.guard.ts(1 hunks)
- apps/api-gateway/src/tracer.ts(1 hunks)
- apps/api-gateway/src/user/utils/index.ts(1 hunks)
- apps/cloud-wallet/src/cloud-wallet.service.ts(18 hunks)
- apps/issuance/src/issuance.service.ts(7 hunks)
- apps/organization/src/organization.service.ts(41 hunks)
- apps/user/repositories/user.repository.ts(8 hunks)
- apps/user/src/user.service.ts(27 hunks)
- apps/user/templates/user-email-template.ts(2 hunks)
- libs/common/src/interfaces/user.interface.ts(1 hunks)
- libs/logger/src/logger.service.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/api-gateway/src/tracer.ts
- apps/api-gateway/src/authz/guards/session.guard.ts
- apps/api-gateway/src/user/utils/index.ts
- libs/logger/src/logger.service.ts
🧰 Additional context used
🧬 Code graph analysis (7)
apps/agent-service/src/agent-service.service.ts (3)
libs/common/src/response-messages/index.ts (1)
ResponseMessages(1-516)apps/agent-service/src/interface/agent-service.interface.ts (3)
IStoreOrgAgentDetails(177-201)
ICreateTenant(433-439)
WalletDetails(660-665)libs/common/src/common.utils.ts (1)
getAgentUrl(70-109)
apps/cloud-wallet/src/cloud-wallet.service.ts (4)
apps/api-gateway/src/cloud-wallet/cloud-wallet.controller.ts (1)
createConnection(327-343)apps/api-gateway/src/cloud-wallet/cloud-wallet.service.ts (1)
createConnection(20-24)apps/cloud-wallet/src/cloud-wallet.controller.ts (1)
createConnection(17-19)libs/common/src/interfaces/cloud-wallet.interface.ts (5)
ICreateConnection(250-266)
IConnectionInvitationResponse(304-309)
CloudWallet(175-190)
GetAllCloudWalletConnections(311-319)
IBasicMessageDetails(327-332)
apps/user/repositories/user.repository.ts (3)
libs/logger/src/logger.service.ts (1)
error(51-54)apps/user/interfaces/user.interface.ts (2)
ISession(184-195)
IUserDeletedActivity(236-244)apps/api-gateway/src/authz/authz.controller.ts (2)
refreshToken(298-307)
sessionDetails(193-211)
apps/organization/src/organization.service.ts (6)
libs/common/src/cast.helper.ts (1)
toNumber(58-76)libs/common/src/response-messages/index.ts (1)
ResponseMessages(1-516)apps/organization/interfaces/organization.interface.ts (2)
ICreateConnectionUrl(31-41)
IClientCredentials(12-15)libs/common/src/interfaces/interface.ts (1)
IAccessTokenData(10-17)apps/organization/repositories/organization.repository.ts (2)
getUser(427-439)
deleteOrg(835-893)libs/common/src/send-grid-helper-file.ts (1)
sendEmail(11-27)
apps/issuance/src/issuance.service.ts (1)
libs/common/src/common.utils.ts (1)
getAgentUrl(70-109)
apps/user/src/user.service.ts (6)
libs/common/src/response-messages/index.ts (1)
ResponseMessages(1-516)apps/api-gateway/src/user/utils/index.ts (1)
getCredentialsByAlias(33-46)libs/common/src/interfaces/user.interface.ts (2)
IVerificationEmail(72-81)
ISignInUser(1-9)apps/user/templates/user-email-template.ts (1)
URLUserEmailTemplate(1-83)apps/user/repositories/user.repository.ts (1)
getUserDetails(120-131)apps/user/interfaces/user.interface.ts (2)
ISessionDetails(204-208)
ISessions(258-260)
libs/common/src/interfaces/user.interface.ts (1)
apps/user/interfaces/user.interface.ts (4)
IVerifyUserEmail(173-176)
ISendVerificationEmail(51-56)
IOrgRole(22-26)
IOrganisation(27-38)
🪛 ESLint
apps/organization/src/organization.service.ts
[error] 1635-1635: Expected no linebreak before this expression.
(implicit-arrow-linebreak)
🔇 Additional comments (18)
apps/issuance/src/issuance.service.ts (2)
65-65: New shared utils import is correct—LGTM.All three imports are used and help centralize concerns.
187-187: Use centralized getAgentUrl—LGTM.Consistent URL resolution via common.utils reduces duplication and drift.
Also applies to: 384-384, 697-697, 839-839
apps/user/templates/user-email-template.ts (2)
16-22: LGTM: Optional params are appended only when providedAppending redirectTo/clientAlias conditionally is correct and avoids noise in links.
1-9: Signature change ripple-checkParameter order changed; verify all
getUserURLTemplate(…)call sites are updated to match the new signature.libs/common/src/interfaces/user.interface.ts (1)
8-8: Addition of sessionId on ISignInUser looks goodField is optional and non-breaking for consumers.
apps/user/repositories/user.repository.ts (1)
714-726: No change required:userIdis marked@uniquein the Prisma schema, sofindUniqueis appropriate.apps/organization/src/organization.service.ts (1)
1172-1182: Remove payload shape concern — theget-user-by-user-idhandler in apps/user/src/user.controller.ts (lines 139–140) is defined asfindUserByUserId(id: string), so passing the rawuserIdstring is correct.Likely an incorrect or invalid review comment.
apps/user/src/user.service.ts (2)
211-231: Email verification template wiring looks good.Passing redirectTo and clientAlias into the template aligns with alias-based flows.
501-507: Nice: returning sessionId with tokens.This improves client-side session tracking and parity with refresh flow (see note below to also include sessionId there).
apps/cloud-wallet/src/cloud-wallet.service.ts (5)
34-34: Import looks correctICreateConnection is used below; no issues.
309-315: Receive invitation: OKGood reuse of _commonCloudWalletInfo and standard auth header.
431-436: Get DID list: OKNo issues spotted.
507-521: Credential list: OKLooks consistent with connections list pattern.
193-216: All_commonCloudWalletInfoinvocations includeuserIdapps/agent-service/src/agent-service.service.ts (4)
1183-1196: Retry wrapper for DID/write looks good.Solid pragmatic retry until API returns didDocument/didDoc; scoped and minimal.
1249-1261: Correct tenant-aware schema URL construction.Using replace with tenantId is correct for shared agents here. Nice.
1571-1571: Good move to central getAgentUrl helper.Reduces local URL-calc drift and concentrates mapping logic.
Also applies to: 1631-1631
529-540: Ignore normalized endpoint change:getBaseAgentTokenalready handles protocol
getBaseAgentTokencallsnormalizeUrlWithProtocol(agentEndPoint), so passingagentDetails.agentEndPoint(host-only) is correct and will be prefixed internally. No change needed.Likely an incorrect or invalid review comment.
| apiKey: process.env.AGENT_API_KEY | ||
| }; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fail fast if AGENT_API_KEY env is missing.
Wallet provisioning will silently send an empty API key if the env is unset. Validate early and surface a clear error.
       tenant: agentSpinupDto.tenant || false,
-      apiKey: process.env.AGENT_API_KEY
+      apiKey: process.env.AGENT_API_KEY
     };
+    if (!walletProvisionPayload.apiKey) {
+      throw new BadRequestException(ResponseMessages.agent.error.notConfigured);
+    }📝 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.
| apiKey: process.env.AGENT_API_KEY | |
| }; | |
| tenant: agentSpinupDto.tenant || false, | |
| apiKey: process.env.AGENT_API_KEY | |
| }; | |
| if (!walletProvisionPayload.apiKey) { | |
| throw new BadRequestException(ResponseMessages.agent.error.notConfigured); | |
| } | 
🤖 Prompt for AI Agents
In apps/agent-service/src/agent-service.service.ts around lines 425-426, the
AGENT_API_KEY is used directly which allows an empty API key to be sent; add an
early validation that checks process.env.AGENT_API_KEY is present and non-empty
and if not, throw a clear Error (or log and process.exit(1)) with a message like
"Missing required environment variable AGENT_API_KEY" so provisioning fails fast
and surfaces the issue immediately.
| const url = `${payload.agentEndPoint}${CommonConstants.URL_SCHM_GET_SCHEMA_BY_ID}`.replace( | ||
| '#', | ||
| `${payload.payload.schemaId}` | ||
| ); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify tenant scoping on GET-by-ID URLs for shared agents.
These paths don’t inject tenantId; if the constants require /tenants/{tenantId}/..., calls will hit the wrong scope. Align with createSchema’s pattern or centralize via a helper.
Potential fix if constants contain two placeholders (tenantId, resourceId):
-        const url = `${payload.agentEndPoint}${CommonConstants.URL_SCHM_GET_SCHEMA_BY_ID}`.replace(
-          '#',
-          `${payload.payload.schemaId}`
-        );
+        const url = `${payload.agentEndPoint}${CommonConstants.URL_SCHM_GET_SCHEMA_BY_ID}`
+          .replace('#', String(payload.tenantId))
+          .replace('#', String(payload.payload.schemaId));and similarly for credential definition:
-        const url = `${payload.agentEndPoint}${CommonConstants.URL_SCHM_GET_CRED_DEF_BY_ID}`.replace(
-          '#',
-          `${payload.payload.credentialDefinitionId}`
-        );
+        const url = `${payload.agentEndPoint}${CommonConstants.URL_SCHM_GET_CRED_DEF_BY_ID}`
+          .replace('#', String(payload.tenantId))
+          .replace('#', String(payload.payload.credentialDefinitionId));Run this to confirm constant shapes:
Also applies to: 1355-1358
🏁 Script executed:
#!/bin/bash
# Inspect placeholders for shared schema/cred-def URL constants
rg -nC2 'URL_SCHM_(GET_SCHEMA_BY_ID|GET_CRED_DEF_BY_ID|CREATE_CRED_DEF)' libs/common/src/common.constant.tsLength of output: 434
🏁 Script executed:
#!/bin/bash
# Inspect createSchema and createCredDef usages for tenant scoping
rg -nC3 'URL_SCHM_CREATE_SCHEMA' apps/agent-service/src/agent-service.service.ts
rg -nC3 'URL_SCHM_CREATE_CRED_DEF' apps/agent-service/src/agent-service.service.tsLength of output: 1563
Include tenantId in shared‐agent URLs
Current constants only have a single # placeholder (for the resource ID) and the CREATE constants have no placeholder at all, so tenantId is never injected in either GET or CREATE calls. For example:
URL_SCHM_GET_SCHEMA_BY_ID      = '/anoncreds/schemas/#'
URL_SCHM_GET_CRED_DEF_BY_ID    = '/anoncreds/credential-definitions/#'
URL_SCHM_CREATE_SCHEMA         = '/anoncreds/schemas'
URL_SCHM_CREATE_CRED_DEF       = '/anoncreds/credential-definitions'– GET calls only replace the second # (resourceId)
– CREATE calls on shared agents call .replace('#', tenantId) against constants without any #, so it’s a no-op
Refactor both GET and CREATE flows to build scoped URLs (e.g. '/tenants/#/anoncreds/schemas/#') or centralize URL construction in a helper so that both tenantId and the resource ID are injected consistently. Also apply the same changes around lines 1355–1358.
🤖 Prompt for AI Agents
In apps/agent-service/src/agent-service.service.ts around lines 1290-1293 (and
similarly 1355-1358), the URL building for shared-agent endpoints only injects
the resourceId and never injects tenantId because the constants lack the tenant
placeholder; update the code to either (a) change the constants to include two
placeholders (e.g. '/tenants/#/anoncreds/schemas/#') and use ordered replacement
for tenantId then resourceId, or (b) create a small centralized helper
buildScopedUrl(basePath, tenantId, resourceId) that returns
`/tenants/${tenantId}${basePath}/${resourceId}` (or appropriate placement) and
replace all direct .replace calls for both GET and CREATE flows to call this
helper so both tenantId and resourceId are consistently injected; apply the same
refactor at lines ~1355-1358.
| const [baseWalletDetails, decryptedApiKey] = await this._commonCloudWalletInfo(userId); | ||
| const { agentEndpoint } = baseWalletDetails; | ||
|  | ||
| const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${proofRecordId}${CommonConstants.CLOUD_WALLET_ACCEPT_PROOF_REQUEST}${tenantId}`; | ||
| const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${proofRecordId}${CommonConstants.CLOUD_WALLET_ACCEPT_PROOF_REQUEST}`; | ||
| const proofAcceptRequestPayload = { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Encode path param and harden URL assembly
proofRecordId should be URL-encoded.
-const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${proofRecordId}${CommonConstants.CLOUD_WALLET_ACCEPT_PROOF_REQUEST}`;
+const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${encodeURIComponent(proofRecordId)}${CommonConstants.CLOUD_WALLET_ACCEPT_PROOF_REQUEST}`;📝 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.
| const [baseWalletDetails, decryptedApiKey] = await this._commonCloudWalletInfo(userId); | |
| const { agentEndpoint } = baseWalletDetails; | |
| const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${proofRecordId}${CommonConstants.CLOUD_WALLET_ACCEPT_PROOF_REQUEST}${tenantId}`; | |
| const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${proofRecordId}${CommonConstants.CLOUD_WALLET_ACCEPT_PROOF_REQUEST}`; | |
| const proofAcceptRequestPayload = { | |
| const [baseWalletDetails, decryptedApiKey] = await this._commonCloudWalletInfo(userId); | |
| const { agentEndpoint } = baseWalletDetails; | |
| const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${encodeURIComponent(proofRecordId)}${CommonConstants.CLOUD_WALLET_ACCEPT_PROOF_REQUEST}`; | |
| const proofAcceptRequestPayload = { | 
🤖 Prompt for AI Agents
In apps/cloud-wallet/src/cloud-wallet.service.ts around lines 127 to 131, the
code builds the request URL by concatenating agentEndpoint, constants and
proofRecordId directly which risks malformed URLs and injection; URL-encode
proofRecordId (e.g. use encodeURIComponent) and assemble the URL robustly
(ensure agentEndpoint/constant segments have/omit slashes consistently or use
the URL constructor) so the final URL is safe and well-formed before using it.
| const [baseWalletDetails, decryptedApiKey] = await this._commonCloudWalletInfo(userId); | ||
| const { agentEndpoint } = baseWalletDetails; | ||
|  | ||
| const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${tenantId}${ | ||
| threadId ? `?threadId=${threadId}` : '' | ||
| }`; | ||
|  | ||
| const threadParam = threadId ? `?threadId=${threadId}` : ''; | ||
| const url = `${agentEndpoint}${CommonConstants.CLOUD_WALLET_GET_PROOF_REQUEST}/${threadParam}}`; | ||
| const getProofById = await this.commonService.httpGet(url, { headers: { authorization: decryptedApiKey } }); | ||
| return getProofById; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Sanity check for dangling braces and malformed URLs
Automate a quick scan to catch similar issues.
Also applies to: 155-160
🏁 Script executed:
#!/bin/bash
# Find stray braces and "/?query" patterns in this service
rg -n '}}\)?;?$|/\?' apps/cloud-wallet/src/cloud-wallet.service.ts -n -C1Length of output: 75
🏁 Script executed:
#!/bin/bash
# Locate the cloud-wallet.service.ts file
fd cloud-wallet.service.tsLength of output: 132
🏁 Script executed:
#!/bin/bash
# Show the proof request URL construction in both services
rg -n 'CLOUD_WALLET_GET_PROOF_REQUEST' apps/cloud-wallet/src/cloud-wallet.service.ts -C3
rg -n 'CLOUD_WALLET_GET_PROOF_REQUEST' apps/api-gateway/src/cloud-wallet/cloud-wallet.service.ts -C3Length of output: 1624
Fix stray trailing braces in URL templates
- In apps/cloud-wallet/src/cloud-wallet.service.ts at line 157, the template ${proofRecordId}}has an extra}, resulting in a malformed URL.
- In the same file at line 179, ${threadParam}}likewise includes an unintended}, breaking the endpoint.
Remove the extra braces to restore correct URL construction.
🤖 Prompt for AI Agents
In apps/cloud-wallet/src/cloud-wallet.service.ts (around lines 157 and 176-181),
the template literal URL constructions contain stray closing braces (e.g.,
`${proofRecordId}}` and `${threadParam}}`) causing malformed endpoints; remove
the extra `}` from each template so they read `${proofRecordId}` and
`${threadParam}` respectively, then confirm the resulting URL concatenation is
correct and update any affected tests or callers if needed.
| const walletKey = await this.commonService.dataEncryption(createCloudWalletResponse.config.walletConfig.token); | ||
|  | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing await on API key encryption (stores a Promise)
Encrypt agentApiKey with await before constructing the DTO.
-const walletKey = await this.commonService.dataEncryption(createCloudWalletResponse.config.walletConfig.token);
+const walletKey = await this.commonService.dataEncryption(createCloudWalletResponse.config.walletConfig.token);
+const encryptedAgentApiKey = await this.commonService.dataEncryption(createCloudWalletResponse.token);
@@
-  agentApiKey: this.commonService.dataEncryption(createCloudWalletResponse.token),
+  agentApiKey: encryptedAgentApiKey,Also applies to: 280-281
🤖 Prompt for AI Agents
In apps/cloud-wallet/src/cloud-wallet.service.ts around lines 264-265 (and
similarly 280-281), the calls to this.commonService.dataEncryption are not
awaited so you end up storing a Promise instead of the encrypted string; change
those assignments to await the encryption calls (e.g., const walletKey = await
this.commonService.dataEncryption(...)) and likewise await encryption of
agentApiKey before you build the DTO so the DTO contains the resolved encrypted
values rather than Promises.
|  | ||
| async fetchSessionByRefreshToken(refreshToken: string): Promise<session> { | ||
| try { | ||
| const sessionDetails = await this.prisma.session.findFirst({ | ||
| where: { | ||
| refreshToken | ||
| } | ||
| }); | ||
| return sessionDetails; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Allow null when searching by refreshToken
findFirst may return null; fix return type accordingly.
-  async fetchSessionByRefreshToken(refreshToken: string): Promise<session> {
+  async fetchSessionByRefreshToken(refreshToken: string): Promise<session | null> {
     try {
       const sessionDetails = await this.prisma.session.findFirst({
         where: {
           refreshToken
         }
       });
       return sessionDetails;📝 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.
| async fetchSessionByRefreshToken(refreshToken: string): Promise<session> { | |
| try { | |
| const sessionDetails = await this.prisma.session.findFirst({ | |
| where: { | |
| refreshToken | |
| } | |
| }); | |
| return sessionDetails; | |
| } catch (error) { | |
| async fetchSessionByRefreshToken(refreshToken: string): Promise<session | null> { | |
| try { | |
| const sessionDetails = await this.prisma.session.findFirst({ | |
| where: { | |
| refreshToken | |
| } | |
| }); | |
| return sessionDetails; | |
| } catch (error) { | |
| // ... | |
| } | |
| } | 
🤖 Prompt for AI Agents
In apps/user/repositories/user.repository.ts around lines 977 to 985, the method
fetchSessionByRefreshToken assumes findFirst always returns a session but
findFirst can return null; change the return type from Promise<session> to
Promise<session | null>, update any related type imports if needed, and adjust
callers or add null checks where this method is used so they handle the possible
null result.
| let sendVerificationMail: boolean; | ||
|  | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid name shadowing and mutating input DTO; sanitize secrets handling.
- Boolean var sendVerificationMail shadows method name; rename for clarity.
- Avoid mutating input DTO; build a new object.
- Copying clientSecret into userEmailVerification risks leakage. Prefer referencing alias/tenant config at runtime rather than storing secrets per user.
-      let sendVerificationMail: boolean;
+      let isVerificationMailSent: boolean;
...
-        sendVerificationMail = await this.sendEmailForVerification({
+        isVerificationMailSent = await this.sendEmailForVerification({
           email,
           verificationCode: verifyCode,
           redirectUrl,
           clientId: clientDetails.clientId,
           brandLogoUrl,
           platformName,
           redirectTo: clientDetails.domain,
           clientAlias
         });
...
-      if (sendVerificationMail) {
-        const uniqueUsername = await this.createUsername(email, verifyCode);
-        userEmailVerification.username = uniqueUsername;
-        userEmailVerification.clientId = clientDetails.clientId;
-        userEmailVerification.clientSecret = clientDetails.clientSecret;
-        const resUser = await this.userRepository.createUser(userEmailVerification, verifyCode);
+      if (isVerificationMailSent) {
+        const uniqueUsername = await this.createUsername(email, verifyCode);
+        const createUserInput = {
+          ...userEmailVerification,
+          username: uniqueUsername,
+          clientId: clientDetails.clientId,
+          // Do NOT persist clientSecret on the user record; resolve by alias when needed.
+        };
+        const resUser = await this.userRepository.createUser(createUserInput, verifyCode);
         return resUser;
       }Also applies to: 153-162, 167-175
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const decodedToken: any = jwt.decode(tokenDetails?.access_token); | ||
| const expiresAt = new Date(decodedToken.exp * 1000); | ||
|  | ||
| const sessionData = { | ||
| id: decodedToken.sid, | ||
| sessionToken: tokenDetails?.access_token, | ||
| userId: userData?.id, | ||
| expires: tokenDetails?.expires_in, | ||
| refreshToken: tokenDetails?.refresh_token, | ||
| sessionType: SessionType.USER_SESSION, | ||
| expiresAt | ||
| }; | ||
|  | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard JWT decode and missing sid to prevent crashes/DB errors.
jwt.decode may return null and sid may be absent; ensure safe handling and generate a fallback id.
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any
-      const decodedToken: any = jwt.decode(tokenDetails?.access_token);
-      const expiresAt = new Date(decodedToken.exp * 1000);
+      const decodedToken = jwt.decode(tokenDetails.access_token) as jwt.JwtPayload | null;
+      if (!decodedToken || typeof decodedToken.exp !== 'number') {
+        throw new UnauthorizedException(ResponseMessages.user.error.invalidCredentials);
+      }
+      const expiresAt = new Date(decodedToken.exp * 1000);
+      const sessionId = typeof (decodedToken as any).sid === 'string' ? (decodedToken as any).sid : uuidv4();
...
-        id: decodedToken.sid,
+        id: sessionId,📝 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.
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | |
| const decodedToken: any = jwt.decode(tokenDetails?.access_token); | |
| const expiresAt = new Date(decodedToken.exp * 1000); | |
| const sessionData = { | |
| id: decodedToken.sid, | |
| sessionToken: tokenDetails?.access_token, | |
| userId: userData?.id, | |
| expires: tokenDetails?.expires_in, | |
| refreshToken: tokenDetails?.refresh_token, | |
| sessionType: SessionType.USER_SESSION, | |
| expiresAt | |
| }; | |
| // Decode and validate the JWT payload | |
| - // eslint-disable-next-line @typescript-eslint/no-explicit-any | |
| - const decodedToken: any = jwt.decode(tokenDetails?.access_token); | |
| const decodedToken = jwt.decode(tokenDetails.access_token) as jwt.JwtPayload | null; | |
| if (!decodedToken || typeof decodedToken.exp !== 'number') { | |
| throw new UnauthorizedException(ResponseMessages.user.error.invalidCredentials); | |
| } | |
| const expiresAt = new Date(decodedToken.exp * 1000); | |
| const sessionId = | |
| typeof (decodedToken as any).sid === 'string' | |
| ? (decodedToken as any).sid | |
| : uuidv4(); | |
| const sessionData = { | |
| id: sessionId, | |
| sessionToken: tokenDetails?.access_token, | |
| userId: userData?.id, | |
| expires: tokenDetails?.expires_in, | |
| refreshToken: tokenDetails?.refresh_token, | |
| sessionType: SessionType.USER_SESSION, | |
| expiresAt | |
| }; | 
🤖 Prompt for AI Agents
In apps/user/src/user.service.ts around lines 468 to 481,
jwt.decode(tokenDetails?.access_token) can return null and decodedToken.sid may
be missing which can crash or insert invalid DB records; before using
decodedToken, check it is not null and has numeric exp and a sid, compute
expiresAt only when exp exists (fallback to new Date(Date.now() +
(tokenDetails?.expires_in ?? 0) * 1000) if not), and generate a fallback id
(e.g., a UUID) when sid is absent; ensure the sessionData uses these guarded
values and import/use a UUID generator if needed so no undefined values are
written to the DB.
| this.logger.debug(`User details::;${JSON.stringify(userByKeycloakId)}`); | ||
| const tokenResponse = await this.clientRegistrationService.getAccessToken( | ||
| refreshToken, | ||
| userByKeycloakId?.['clientId'], | ||
| userByKeycloakId?.['clientSecret'] | ||
| ); | ||
| this.logger.debug(`tokenResponse::::${JSON.stringify(tokenResponse)}`); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove sensitive data from logs (tokens, secrets, PII).
Debug logs stringify user and token objects; these may include clientSecret, access/refresh tokens. Strip them.
-        this.logger.debug(`User details::;${JSON.stringify(userByKeycloakId)}`);
+        this.logger.debug('User details fetched for refresh flow.');
...
-        this.logger.debug(`tokenResponse::::${JSON.stringify(tokenResponse)}`);
+        this.logger.debug('Access token refreshed successfully.');📝 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.
| this.logger.debug(`User details::;${JSON.stringify(userByKeycloakId)}`); | |
| const tokenResponse = await this.clientRegistrationService.getAccessToken( | |
| refreshToken, | |
| userByKeycloakId?.['clientId'], | |
| userByKeycloakId?.['clientSecret'] | |
| ); | |
| this.logger.debug(`tokenResponse::::${JSON.stringify(tokenResponse)}`); | |
| // Do not log sensitive details | |
| this.logger.debug('User details fetched for refresh flow.'); | |
| const tokenResponse = await this.clientRegistrationService.getAccessToken( | |
| refreshToken, | |
| userByKeycloakId?.['clientId'], | |
| userByKeycloakId?.['clientSecret'] | |
| ); | |
| // Avoid logging tokens | |
| this.logger.debug('Access token refreshed successfully.'); | 
🤖 Prompt for AI Agents
In apps/user/src/user.service.ts around lines 531 to 537 the code logs full user
and token objects which may contain sensitive fields (clientSecret,
access/refresh tokens, PII); remove or redact sensitive fields before logging by
only logging non-sensitive identifiers (e.g., user id or username) or by
creating a sanitized object that excludes clientSecret and any token fields, and
update both debug statements to output the sanitized data or a minimal message
indicating success without secrets.
| if (Invitation.ACCEPTED === acceptRejectInvitation.status) { | ||
| const payload = {userId}; | ||
| const payload = { userId }; | ||
| const TotalOrgs = await this._getTotalOrgCount(payload); | ||
|  | ||
| if (TotalOrgs >= toNumber(`${process.env.MAX_ORG_LIMIT}`)) { | ||
| throw new BadRequestException(ResponseMessages.user.error.userOrgsLimit); | ||
| } | ||
| throw new BadRequestException(ResponseMessages.user.error.userOrgsLimit); | ||
| } | ||
| } | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Safely parse MAX_ORG_LIMIT to avoid NaN bypass.
If env is missing/invalid, toNumber may yield NaN and skip the limit check.
-        if (TotalOrgs >= toNumber(`${process.env.MAX_ORG_LIMIT}`)) {
+        const MAX_ORG_LIMIT = toNumber(String(process.env.MAX_ORG_LIMIT), { default: 5, min: 1 });
+        if (TotalOrgs >= MAX_ORG_LIMIT) {
           throw new BadRequestException(ResponseMessages.user.error.userOrgsLimit);
         }📝 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.
| if (Invitation.ACCEPTED === acceptRejectInvitation.status) { | |
| const payload = {userId}; | |
| const payload = { userId }; | |
| const TotalOrgs = await this._getTotalOrgCount(payload); | |
| if (TotalOrgs >= toNumber(`${process.env.MAX_ORG_LIMIT}`)) { | |
| throw new BadRequestException(ResponseMessages.user.error.userOrgsLimit); | |
| } | |
| throw new BadRequestException(ResponseMessages.user.error.userOrgsLimit); | |
| } | |
| } | |
| if (Invitation.ACCEPTED === acceptRejectInvitation.status) { | |
| const payload = { userId }; | |
| const TotalOrgs = await this._getTotalOrgCount(payload); | |
| const MAX_ORG_LIMIT = toNumber(String(process.env.MAX_ORG_LIMIT), { default: 5, min: 1 }); | |
| if (TotalOrgs >= MAX_ORG_LIMIT) { | |
| throw new BadRequestException(ResponseMessages.user.error.userOrgsLimit); | |
| } | |
| } | 
🤖 Prompt for AI Agents
In apps/user/src/user.service.ts around lines 1039 to 1046, the current check
uses toNumber(`${process.env.MAX_ORG_LIMIT}`) which can produce NaN if the env
var is missing/invalid and thus bypass the limit; update to safely parse and
validate the env value before comparing: parse MAX_ORG_LIMIT with
Number.parseInt (or Number) and a radix when appropriate, check
isNaN(parsedLimit) and either apply a sensible default or throw a configuration
error, then compare TotalOrgs against the validated parsedLimit (not against
NaN) so the limit check cannot be skipped.
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
fix:sonar cloud issues
Sync Develop branch into qa
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
♻️ Duplicate comments (8)
apps/user/src/user.service.ts (6)
115-116: Validate clientAlias and stop persisting clientSecret on user.
- Guard clientAlias before using getCredentialsByAlias; undefined will throw.
- Do not persist clientSecret in userEmailVerification; it’s highly sensitive and unnecessary. Persisting secrets is a security vulnerability.
- Avoid mutating the incoming DTO and rename boolean to avoid shadowing.
Apply:
@@ - const { email, brandLogoUrl, platformName, clientAlias } = userEmailVerification; + const { email, brandLogoUrl, platformName, clientAlias } = userEmailVerification; + if (!clientAlias || typeof clientAlias !== 'string' || !clientAlias.trim()) { + throw new BadRequestException('clientAlias is required'); + } @@ - let sendVerificationMail: boolean; + let isVerificationMailSent: boolean; @@ - const clientDetails = await getCredentialsByAlias(clientAlias); + const clientDetails = await getCredentialsByAlias(clientAlias); @@ - sendVerificationMail = await this.sendEmailForVerification({ + isVerificationMailSent = await this.sendEmailForVerification({ email, verificationCode: verifyCode, redirectUrl, clientId: clientDetails.clientId, brandLogoUrl, platformName, redirectTo: clientDetails.domain, clientAlias }); @@ - if (sendVerificationMail) { + if (isVerificationMailSent) { const uniqueUsername = await this.createUsername(email, verifyCode); - userEmailVerification.username = uniqueUsername; - userEmailVerification.clientId = clientDetails.clientId; - userEmailVerification.clientSecret = clientDetails.clientSecret; - const resUser = await this.userRepository.createUser(userEmailVerification, verifyCode); + const createUserInput = { + ...userEmailVerification, + username: uniqueUsername, + clientId: clientDetails.clientId + // Do NOT persist clientSecret on user records. + }; + const resUser = await this.userRepository.createUser(createUserInput, verifyCode); return resUser; }Also applies to: 138-138, 153-163, 167-175
453-459: Await session cleanup and parse SESSIONS_LIMIT safely.Unawaited deleteInactiveSessions races the limit check; Number(env) can be NaN and bypass the check.
- // called seprate method to delete exp session - this.userRepository.deleteInactiveSessions(userData?.id); + // called separate method to delete expired sessions + await this.userRepository.deleteInactiveSessions(userData.id); const userSessionDetails = await this.userRepository.fetchUserSessions(userData?.id); - if (Number(process.env.SESSIONS_LIMIT) <= userSessionDetails?.length) { + const SESSIONS_LIMIT = toNumber(String(process.env.SESSIONS_LIMIT), { default: 5, min: 1 }); + if (SESSIONS_LIMIT <= (userSessionDetails?.length ?? 0)) { throw new BadRequestException(ResponseMessages.user.error.sessionLimitReached); }
468-481: Guard JWT decode and missing sid; generate fallback session id.jwt.decode can return null; sid may be absent. Prevent crashes and DB writes with undefined ids.
- // eslint-disable-next-line @typescript-eslint/no-explicit-any - const decodedToken: any = jwt.decode(tokenDetails?.access_token); - const expiresAt = new Date(decodedToken.exp * 1000); + const decodedToken = jwt.decode(tokenDetails.access_token) as jwt.JwtPayload | null; + if (!decodedToken || typeof decodedToken.exp !== 'number') { + throw new UnauthorizedException(ResponseMessages.user.error.invalidCredentials); + } + const expiresAt = new Date(decodedToken.exp * 1000); + const sessionId = typeof (decodedToken as any)?.sid === 'string' ? (decodedToken as any).sid : uuidv4(); @@ - id: decodedToken.sid, + id: sessionId,
528-537: Stop decoding untrusted refreshToken claims; sanitize logs; include new sessionId in response.
- Don’t trust jwt.decode(refreshToken) to fetch user; first look up the session by refresh token, then load the user.
- Remove logs that print user/token objects.
- Include sessionId in the response for API consistency.
Apply:
- const data = jwt.decode(refreshToken) as jwt.JwtPayload; - const userByKeycloakId = await this.userRepository.getUserByKeycloakId(data?.sub); - this.logger.debug(`User details::;${JSON.stringify(userByKeycloakId)}`); + // Find session first; do not trust user-supplied JWT claims. + const existingSession = await this.userRepository.fetchSessionByRefreshToken(refreshToken); + if (!existingSession) { + throw new NotFoundException(ResponseMessages.user.error.userSeesionNotFound); + } + const userById = await this.userRepository.getUserById(existingSession.userId); - const tokenResponse = await this.clientRegistrationService.getAccessToken( - refreshToken, - userByKeycloakId?.['clientId'], - userByKeycloakId?.['clientSecret'] - ); - this.logger.debug(`tokenResponse::::${JSON.stringify(tokenResponse)}`); + const tokenResponse = await this.clientRegistrationService.getAccessToken( + refreshToken, + userById.clientId, + userById.clientSecret + ); + this.logger.debug('Access token refreshed successfully.'); @@ - const userAccountDetails = await this.userRepository.checkAccountDetails(userByKeycloakId?.['id']); + const userAccountDetails = await this.userRepository.checkAccountDetails(userById.id); @@ - const sessionDetails = await this.userRepository.fetchSessionByRefreshToken(refreshToken); - if (!sessionDetails) { - throw new NotFoundException(ResponseMessages.user.error.userSeesionNotFound); - } + const sessionDetails = existingSession; @@ - const deletePreviousSession = await this.userRepository.deleteSession(sessionDetails.id); + const deletePreviousSession = await this.userRepository.deleteSession(sessionDetails.id); @@ - return tokenResponse; + return { ...tokenResponse, sessionId: addSessionDetails.id };Also applies to: 545-569, 571-575
1028-1042: Safely parse MAX_ORG_LIMIT to prevent NaN bypass.Use a validated numeric with sensible defaults.
- if (TotalOrgs >= toNumber(`${process.env.MAX_ORG_LIMIT}`)) { + const MAX_ORG_LIMIT = toNumber(String(process.env.MAX_ORG_LIMIT), { default: 5, min: 1 }); + if (TotalOrgs >= MAX_ORG_LIMIT) { throw new BadRequestException(ResponseMessages.user.error.userOrgsLimit); }
766-783: Same concern in resetPassword: avoid per-user clientSecret.- const token = await this.clientRegistrationService.getManagementToken( - userData.clientId, - userData.clientSecret - ); + const clientDetails = await getCredentialsByAlias(userData.clientAlias ?? process.env.DEFAULT_CLIENT_ALIAS); + const token = await this.clientRegistrationService.getManagementToken( + clientDetails.clientId, + clientDetails.clientSecret + );apps/issuance/src/issuance.repository.ts (2)
139-169: Fix credIssued mapping; avoid .toLowerCase() on undefined.
- credIssued → offerSent is wrong; map to the final issued state.
- Normalize once; don’t call toLowerCase() on possibly undefined.
Apply:
- let stateInfo = null; - switch (issuedCredentialsSearchCriteria.search.toLowerCase()) { + let stateInfo: string | null = null; + const searchLc = (search ?? '').toLowerCase(); + switch (searchLc) { case IssuedCredentialStatus.offerSent.toLowerCase(): stateInfo = IssueCredentials.offerSent; break; case IssuedCredentialStatus.done.toLowerCase(): stateInfo = IssueCredentials.done; break; case IssuedCredentialStatus.abandoned.toLowerCase(): stateInfo = IssueCredentials.abandoned; break; case IssuedCredentialStatus.received.toLowerCase(): stateInfo = IssueCredentials.requestReceived; break; case IssuedCredentialStatus.proposalReceived.toLowerCase(): stateInfo = IssueCredentials.proposalReceived; break; case IssuedCredentialStatus.credIssued.toLowerCase(): - stateInfo = IssueCredentials.offerSent; + stateInfo = IssueCredentials.done; // or IssueCredentials.credentialIssued if that enum exists break; default: - break; + stateInfo = null; }If your enum has credentialIssued, switch to IssueCredentials.credentialIssued here. Want me to patch accordingly?
170-189: Prisma relation/state filters: use is/some and equals; reuse normalized search.
- For connections, use is (1:1) or some (1:n); current shape is invalid.
- Use equals when stateInfo is resolved; contains only for free-text.
- Reuse search and include schemaIdsMatched only when non-empty.
Apply:
const issuanceWhereClause: Prisma.credentialsWhereInput = { orgId, ...(schemaIds?.length ? { schemaId: { in: schemaIds } } : {}), - ...(!schemaIds?.length && issuedCredentialsSearchCriteria.search + ...(!schemaIds?.length && search ? { OR: [ - { connectionId: { contains: issuedCredentialsSearchCriteria.search, mode: 'insensitive' } }, - { schemaId: { contains: issuedCredentialsSearchCriteria.search, mode: 'insensitive' } }, - { schemaId: { in: schemaIdsMatched } }, - { - connections: { - theirLabel: { contains: issuedCredentialsSearchCriteria.search, mode: 'insensitive' } - } - }, - { state: { contains: stateInfo ?? issuedCredentialsSearchCriteria.search, mode: 'insensitive' } } + { connectionId: { contains: search, mode: 'insensitive' } }, + { schemaId: { contains: search, mode: 'insensitive' } }, + ...(schemaIdsMatched.length ? [{ schemaId: { in: schemaIdsMatched } }] : []), + { connections: { is: { theirLabel: { contains: search, mode: 'insensitive' } } } }, // if 1:n, replace is with some + ...(stateInfo + ? [{ state: { equals: stateInfo } }] + : [{ state: { contains: search, mode: 'insensitive' } }]) ] } : {}) };If connections is 1:n, replace is with some.
🧹 Nitpick comments (7)
apps/user/src/user.service.ts (5)
29-29: Avoid app-to-app imports; re-export shared utilities via a lib.Importing from apps/api-gateway in apps/user couples apps and risks circular deps. Prefer placing getCredentialsByAlias and shared interfaces in a libs/* package and import from there.
- import { getCredentialsByAlias } from 'apps/api-gateway/src/user/utils'; + // e.g., libs/auth-client/src/index.ts + import { getCredentialsByAlias } from '@credebl/auth-client';Also applies to: 52-52, 57-66, 71-71, 88-88
91-105: Fix misleading log message.Copy/paste log says “In Create User”. Use a specific message for getClientAliases.
- this.logger.error(`In Create User : ${JSON.stringify(error)}`); + this.logger.error(`In getClientAliases: ${JSON.stringify(error)}`);
482-499: Avoid mixing await with .then for clarity.- await this.userRepository.addAccountDetails(accountData).then(async (response) => { - const finalSessionData = { ...sessionData, accountId: response.id }; - addSessionDetails = await this.userRepository.createSession(finalSessionData); - }); + const response = await this.userRepository.addAccountDetails(accountData); + const finalSessionData = { ...sessionData, accountId: response.id }; + addSessionDetails = await this.userRepository.createSession(finalSessionData);
513-524: Double decode of sessionId is brittle.Decode once with a safe fallback instead of unconditional double decode.
- const onceDecoded = decodeURIComponent(sessionId); - const decodedSessionId = decodeURIComponent(onceDecoded); + const onceDecoded = decodeURIComponent(sessionId); + const decodedSessionId = onceDecoded.includes('%') ? decodeURIComponent(onceDecoded) : onceDecoded;
1306-1320: Rename getuserOrganizationByUserId to getUserOrganizationByUserId
Update the method signature in apps/user/src/user.service.ts (around line 1307) and its call site in apps/user/src/user.controller.ts (around line 264) to use consistent camelCase.apps/issuance/src/issuance.repository.ts (2)
190-205: Validate sort field; align return type vs select.
- sortField should be validated against an allowlist to avoid runtime errors.
- credentialExchangeId is selected but not part of the declared return type.
You can harden orderBy like:
const allowedSort: (keyof credentials)[] = ['createDateTime', 'state', 'schemaId', 'connectionId', 'createdBy', 'orgId']; const sortField = allowedSort.includes(issuedCredentialsSearchCriteria?.sortField as any) ? issuedCredentialsSearchCriteria.sortField : 'createDateTime'; const sortOrder = SortValue.DESC === issuedCredentialsSearchCriteria?.sortBy ? 'desc' : 'asc';And update orderBy to use sortField/sortOrder.
Optional: include credentialExchangeId in the method’s return type for clarity.
783-795: threadId is unique in your Prisma schema; update() will target at most one record but will throw a Prisma “record not found” error (P2025) if no match. If you’d rather no-op on a missing row, either catch P2025 explicitly or switch to updateMany() and check the returned count.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
- apps/issuance/src/issuance.repository.ts(6 hunks)
- apps/user/src/user.service.ts(27 hunks)
- apps/verification/src/repositories/verification.repository.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/verification/src/repositories/verification.repository.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/user/src/user.service.ts (5)
apps/api-gateway/src/user/utils/index.ts (1)
getCredentialsByAlias(33-46)libs/common/src/interfaces/user.interface.ts (2)
IVerificationEmail(72-81)
ISignInUser(1-9)apps/user/templates/user-email-template.ts (1)
URLUserEmailTemplate(1-83)apps/user/repositories/user.repository.ts (1)
getUserDetails(120-131)apps/api-gateway/src/authz/authz.controller.ts (1)
sessionDetails(193-211)
| const schemas = await this.prisma.schema.findMany({ | ||
| where: { | ||
| orgId, | ||
| ...(schemaIds?.length ? { schemaId: { in: schemaIds } } : {}), | ||
| ...(!schemaIds?.length && issuedCredentialsSearchCriteria.search | ||
| ? { | ||
| OR: [ | ||
| { connectionId: { contains: issuedCredentialsSearchCriteria.search, mode: 'insensitive' } }, | ||
| { schemaId: { contains: issuedCredentialsSearchCriteria.search, mode: 'insensitive' } } | ||
| ] | ||
| } | ||
| : {}) | ||
| name: { contains: issuedCredentialsSearchCriteria.search, mode: 'insensitive' } | ||
| }, | ||
| select: { schemaLedgerId: true } | ||
| }); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard search and avoid querying schemas with undefined; reuse a normalized variable.
Calling contains with an undefined search will throw and performs an unnecessary DB hit when no search is provided.
Apply:
-      const schemas = await this.prisma.schema.findMany({
-        where: {
-          name: { contains: issuedCredentialsSearchCriteria.search, mode: 'insensitive' }
-        },
-        select: { schemaLedgerId: true }
-      });
-
-      const schemaIdsMatched = schemas.map((s) => s.schemaLedgerId);
+      const search = issuedCredentialsSearchCriteria.search?.trim();
+      const schemaIdsMatched = search
+        ? (
+            await this.prisma.schema.findMany({
+              where: { name: { contains: search, mode: 'insensitive' } },
+              select: { schemaLedgerId: true }
+            })
+          ).map((s) => s.schemaLedgerId)
+        : [];📝 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.
| const schemas = await this.prisma.schema.findMany({ | |
| where: { | |
| orgId, | |
| ...(schemaIds?.length ? { schemaId: { in: schemaIds } } : {}), | |
| ...(!schemaIds?.length && issuedCredentialsSearchCriteria.search | |
| ? { | |
| OR: [ | |
| { connectionId: { contains: issuedCredentialsSearchCriteria.search, mode: 'insensitive' } }, | |
| { schemaId: { contains: issuedCredentialsSearchCriteria.search, mode: 'insensitive' } } | |
| ] | |
| } | |
| : {}) | |
| name: { contains: issuedCredentialsSearchCriteria.search, mode: 'insensitive' } | |
| }, | |
| select: { schemaLedgerId: true } | |
| }); | |
| const search = issuedCredentialsSearchCriteria.search?.trim(); | |
| const schemaIdsMatched = search | |
| ? ( | |
| await this.prisma.schema.findMany({ | |
| where: { name: { contains: search, mode: 'insensitive' } }, | |
| select: { schemaLedgerId: true } | |
| }) | |
| ).map((s) => s.schemaLedgerId) | |
| : []; | 
🤖 Prompt for AI Agents
In apps/issuance/src/issuance.repository.ts around lines 132 to 137, the call to
prisma.schema.findMany uses issuedCredentialsSearchCriteria.search directly
which may be undefined and will cause Prisma to throw and produce an unnecessary
DB query; normalize the search into a local variable (trim and check) and only
call findMany or include the contains clause when that normalized search is
non-empty (otherwise skip the query or use an empty where clause that avoids
contains). Ensure you reuse the normalized variable in the where condition so
undefined is never passed to contains.
| credDefId, | ||
| ...(schemaId ? { schemaId } : {}) | ||
| }, | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t overwrite credDefId with empty string on update.
When the webhook lacks credDefId, this will blank it out. Mirror the schemaId guard.
Apply:
         update: {
           lastChangedBy: organisationId,
           createDateTime: issueCredentialDto?.createDateTime,
           threadId: issueCredentialDto?.threadId,
           connectionId: issueCredentialDto?.connectionId,
           state: issueCredentialDto?.state,
-          credDefId,
-          ...(schemaId ? { schemaId } : {})
+          ...(credDefId ? { credDefId } : {}),
+          ...(schemaId ? { schemaId } : {})
         },📝 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.
| credDefId, | |
| ...(schemaId ? { schemaId } : {}) | |
| }, | |
| update: { | |
| lastChangedBy: organisationId, | |
| createDateTime: issueCredentialDto?.createDateTime, | |
| threadId: issueCredentialDto?.threadId, | |
| connectionId: issueCredentialDto?.connectionId, | |
| state: issueCredentialDto?.state, | |
| ...(credDefId ? { credDefId } : {}), | |
| ...(schemaId ? { schemaId } : {}) | |
| }, | 
🤖 Prompt for AI Agents
In apps/issuance/src/issuance.repository.ts around lines 271 to 273, the update
object always includes credDefId which can overwrite the stored value with an
empty string when the webhook omits it; change the spread to mirror the schemaId
guard so credDefId is only added when truthy (e.g., replace the unconditional
credDefId entry with a conditional spread like ...(credDefId ? { credDefId } :
{})) so missing values do not blank the existing field.
| async sendEmailForVerification(verificationEmailParameter: IVerificationEmail): Promise<boolean> { | ||
| try { | ||
| const { email, verificationCode, brandLogoUrl, platformName, redirectTo, clientAlias } = | ||
| verificationEmailParameter; | ||
| const platformConfigData = await this.prisma.platform_config.findMany(); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard missing platform config to avoid runtime crash.
platform_config[0] may be undefined; accessing emailFrom will throw. Validate and fail fast with a clear error.
-      const platformConfigData = await this.prisma.platform_config.findMany();
+      const platformConfigData = await this.prisma.platform_config.findMany();
+      const platformConfig = platformConfigData[0];
+      if (!platformConfig?.emailFrom) {
+        throw new InternalServerErrorException('Email sender configuration not found');
+      }
@@
-      emailData.emailFrom = platformConfigData[0].emailFrom;
+      emailData.emailFrom = platformConfig.emailFrom;Also applies to: 224-231
🤖 Prompt for AI Agents
In apps/user/src/user.service.ts around lines 211-215 (and similarly at
224-231), the code assumes platform_config[0] exists and accesses emailFrom
directly which can cause a runtime crash; guard against missing config by
validating the query result (e.g., check platform_config.length > 0 and that
platform_config[0].emailFrom is defined) and fail fast with a clear error or
return false; alternatively use findFirst to fetch a single config and handle a
null result, and update both occurrences to log/throw a descriptive error before
proceeding.
| let keycloakDetails = null; | ||
|  | ||
| const token = await this.clientRegistrationService.getManagementToken( | ||
| checkUserDetails.clientId, | ||
| checkUserDetails.clientSecret | ||
| ); | ||
| if (userInfo.isPasskey) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Don’t use per-user stored clientSecret; add holder role null-guard.
- Using user.clientId/clientSecret to get management tokens leaks secrets and couples users to SSO config. Resolve credentials at runtime via alias/tenant config.
- holderRoleData can be false when role not found, leading to .id access crash. Add guard.
Example adjustments:
-      const token = await this.clientRegistrationService.getManagementToken(
-        checkUserDetails.clientId,
-        checkUserDetails.clientSecret
-      );
+      // Prefer alias/tenant-based lookup; avoid user-level secrets.
+      const clientDetails = await getCredentialsByAlias(userDetails.clientAlias ?? process.env.DEFAULT_CLIENT_ALIAS);
+      const token = await this.clientRegistrationService.getManagementToken(
+        clientDetails.clientId,
+        clientDetails.clientSecret
+      );
@@
-      const holderRole = realmRoles.filter((role) => role.name === OrgRoles.HOLDER);
-      const holderRoleData = 0 < holderRole.length && holderRole[0];
+      const holderRole = realmRoles.filter((role) => role.name === OrgRoles.HOLDER);
+      const holderRoleData = holderRole.length > 0 ? holderRole[0] : null;
+      if (!holderRoleData?.id) {
+        throw new NotFoundException('Realm role HOLDER not found');
+      }Also applies to: 320-339, 344-345, 357-371, 375-383
🤖 Prompt for AI Agents
In apps/user/src/user.service.ts around lines 303-309 (and also apply same
changes to ranges 320-339, 344-345, 357-371, 375-383): currently the code calls
getManagementToken with per-user clientId/clientSecret and then assumes
holderRoleData exists; instead resolve the management client credentials from
the tenant/alias configuration (not from the user record) and pass those
credentials to getManagementToken, and add a null-guard before accessing
holderRoleData.id (e.g., if !holderRoleData throw or handle missing role) so the
code never dereferences a possibly-false value; apply these same two changes
(use tenant/alias creds + guard holderRoleData) to the other indicated blocks.
| const tokenDetails = await this.userRepository.getResetPasswordTokenDetails(userData.id, token); | ||
|  | ||
| if (!tokenDetails || (new Date() > tokenDetails.expiresAt)) { | ||
| if (!tokenDetails || new Date() > tokenDetails.expiresAt) { | ||
| throw new BadRequestException(ResponseMessages.user.error.invalidResetLink); | ||
| } | ||
|  | ||
| const decryptedPassword = await this.commonService.decryptPassword(password); | ||
| try { | ||
|  | ||
|  | ||
| const authToken = await this.clientRegistrationService.getManagementToken(userData.clientId, userData.clientSecret); | ||
| try { | ||
| const authToken = await this.clientRegistrationService.getManagementToken( | ||
| userData.clientId, | ||
| userData.clientSecret | ||
| ); | ||
| userData.password = decryptedPassword; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Reset-password flows still use stored user.clientSecret.
Resolve credentials via alias/tenant config rather than persisting clientSecret on user records. This reduces blast radius and secret sprawl.
-        const authToken = await this.clientRegistrationService.getManagementToken(
-          userData.clientId,
-          userData.clientSecret
-        );
+        const clientDetails = await getCredentialsByAlias(userData.clientAlias ?? process.env.DEFAULT_CLIENT_ALIAS);
+        const authToken = await this.clientRegistrationService.getManagementToken(
+          clientDetails.clientId,
+          clientDetails.clientSecret
+        );Also applies to: 710-716
🤖 Prompt for AI Agents
In apps/user/src/user.service.ts around lines 694-706 (and similarly 710-716),
the code uses userData.clientSecret from the persisted user record; instead,
resolve the client credentials from the tenant/alias configuration or a
centralized credential resolver (e.g., TenantConfigService or
ClientCredentialsStore) using userData.clientId before calling
getManagementToken, and pass that resolved secret into
clientRegistrationService.getManagementToken; remove reliance on
userData.clientSecret (and stop persisting it on the user record), and add a
clear fallback or error when no configured secret is found so the flow fails
safely.
* fix:credo-controller service spin up ECS ec2 Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> * WIP: invalid json issue for ecs.sh Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> * WIP: testing changes ecs.sh Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> * WIP: add efs to ecs.sh credo-spin-up script Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> * WIP: typo error Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> * WIP: typo error Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> * WIP: token extraction in ecs.sh Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> * update fargate.sh-credo spin up script Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> * fix: fargate.sh volume issue Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> * WIP fargate.sh issue Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> * WIP fargate.sh image issue Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> * added ARM arch to fargate.sh Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> * WIP inbound endpoint Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> * WIP fixed agent endpoint saved at end Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> * fix: fargate.sh inbound port to number from string Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com> --------- Signed-off-by: Sahil Kamble <sahil.kamble@ayanworks.com>
feat/credo-spinup-script (#1427)
| 
 | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️  Outside diff range comments (6)
apps/agent-provisioning/src/agent-provisioning.service.ts (2)
43-49: Handle child-process errors, add timeout/maxBuffer, and avoid logging secretsYou ignore err and treat any stderr as non-fatal. Add explicit reject on error/exit≠0, a timeout, larger maxBuffer, and be careful not to log sensitive data from stdout/stderr.
Apply:
-const spinUpResponse: object = new Promise(async (resolve) => { - await exec(walletProvision, async (err, stdout, stderr) => { - this.logger.log(`shell script output: ${stdout}`); - if (stderr) { - this.logger.log(`shell script error: ${stderr}`); - } +const spinUpResponse = new Promise<{ agentEndPoint: string; agentToken: string }>((resolve, reject) => { + execFile(scriptPath, args, { shell: false, timeout: 10 * 60_000, maxBuffer: 10 * 1024 * 1024 }, async (err, stdout, stderr) => { + if (stdout) this.logger.info('agent spin-up: stdout available'); + if (stderr) this.logger.warn('agent spin-up: stderr emitted'); + if (err) { + this.logger.error(`spin-up failed: ${err.message}`); + return reject(err); + }
50-67: Fix TOCTOU and rejection flow when files are absentThrowing inside the exec callback won’t be caught by the outer try/catch and can crash the process. Reject the promise instead, and simplify file existence checks by reading once in a try/catch.
Apply:
-const agentEndPointExists = await this.checkFileExistence(agentEndpointPath); -const agentTokenExists = await this.checkFileExistence(agentTokenPath); -let agentEndPoint; -let agentToken; -if (agentEndPointExists && agentTokenExists) { - this.logger.log('Both files exist'); - agentEndPoint = await fs.readFileSync(agentEndpointPath, 'utf8'); - agentToken = await fs.readFileSync(agentTokenPath, 'utf8'); -} else { - this.logger.log('One or both files do not exist'); - throw new NotFoundException(`${agentEndpointPath} or ${agentTokenPath} files do not exist `); -} +try { + const agentEndPoint = fs.readFileSync(agentEndpointPath, 'utf8'); + const agentToken = fs.readFileSync(agentTokenPath, 'utf8'); + return resolve({ + agentEndPoint: JSON.parse(agentEndPoint).CONTROLLER_ENDPOINT, + agentToken: JSON.parse(agentToken).token, + }); +} catch (e) { + return reject(new NotFoundException(`${agentEndpointPath} or ${agentTokenPath} not found/readable`)); +} - -resolve({ - agentEndPoint: JSON.parse(agentEndPoint).CONTROLLER_ENDPOINT, - agentToken: JSON.parse(agentToken).token -}); +// resolve is called in try blockapps/agent-provisioning/AFJ/scripts/start_agent_ecs.sh (1)
100-114: Use bash for [[ … ]] and regex matching, or rewrite to POSIX [ ]The script uses [[ … ]] but declares /bin/sh. Switch to bash or rewrite the tests to POSIX.
-#!/bin/sh +#!/usr/bin/env bash +set -euo pipefailapps/agent-provisioning/AFJ/scripts/fargate.sh (3)
1-1: Use bash; enable strict modeScript uses [[ … ]] and bashisms; switch shebang and add strict flags.
-#!/bin/sh +#!/usr/bin/env bash +set -euo pipefail
9-9: Typo in positional parameter: RANDOM_SEED=$5aThis appends a literal “a” to $5. Breaks seeds.
-RANDOM_SEED=$5a +RANDOM_SEED=$5
156-216: Container definitions JSON is malformed: duplicated/stray “ulimits” and bracesThere are two “ulimits” and an extra closing brace/comma that escapes the object. ECS will reject this.
Fix by keeping a single ulimits inside the container and properly closing logConfiguration and the container object:
"volumesFrom": [], - "ulimits": [], - "logConfiguration": { - "logDriver": "awslogs", - "options": { - "awslogs-create-group": "true", - "awslogs-group": "/ecs/$TASKDEFINITION_FAMILY", - "awslogs-region": "$AWS_PUBLIC_REGION", - "awslogs-stream-prefix": "ecs" -} - }, - "ulimits": [] + "ulimits": [], + "logConfiguration": { + "logDriver": "awslogs", + "options": { + "awslogs-create-group": "true", + "awslogs-group": "/ecs/$TASKDEFINITION_FAMILY", + "awslogs-region": "$AWS_PUBLIC_REGION", + "awslogs-stream-prefix": "ecs" + } + }Validate with jq before registering the task.
♻️ Duplicate comments (2)
apps/agent-provisioning/src/agent-provisioning.service.ts (1)
42-42: Stop building a shell command string; use execFile/spawn with an args array and a trusted script pathThis is vulnerable to command injection and trips CodeQL (“uncontrolled absolute path”). Resolve the script path from a trusted base, verify it, then pass each param as its own argv element. Also quote/normalize all env-driven values.
Apply along these lines:
-import { exec } from 'child_process'; +import { execFile } from 'child_process'; +import * as path from 'path'; -const walletProvision = `${process.cwd() + process.env.AFJ_AGENT_SPIN_UP} ${orgId} "${externalIp}" "${walletName}" "${walletPassword}" ${seed} ${webhookEndpoint} ${walletStorageHost} ${walletStoragePort} ${walletStorageUser} ${walletStoragePassword} ${containerName} ${protocol} ${tenant} ${credoImage} "${indyLedger}" ${inboundEndpoint} ${process.env.SCHEMA_FILE_SERVER_URL} ${process.env.AGENT_API_KEY} ${process.env.AWS_ACCOUNT_ID} ${process.env.S3_BUCKET_ARN} ${process.env.CLUSTER_NAME} ${process.env.TASKDEFINITION_FAMILY} ${process.env.ADMIN_TG_ARN} ${process.env.INBOUND_TG_ARN} ${process.env.FILESYSTEMID} ${process.env.ECS_SUBNET_ID} ${process.env.ECS_SECURITY_GROUP_ID}`; +const scriptPath = path.resolve(process.cwd(), process.env.AFJ_AGENT_SPIN_UP ?? ''); +// Optional: ensure scriptPath stays within a trusted base dir. +const args = [ + String(orgId), String(externalIp), String(walletName), String(walletPassword), String(seed), + String(webhookEndpoint), String(walletStorageHost), String(walletStoragePort), String(walletStorageUser), + String(walletStoragePassword), String(containerName), String(protocol), String(tenant), String(credoImage), + String(indyLedger), String(inboundEndpoint), String(process.env.SCHEMA_FILE_SERVER_URL ?? ''), + String(process.env.AGENT_API_KEY ?? ''), String(process.env.AWS_ACCOUNT_ID ?? ''), String(process.env.S3_BUCKET_ARN ?? ''), + String(process.env.CLUSTER_NAME ?? ''), String(process.env.TASKDEFINITION_FAMILY ?? ''), String(process.env.ADMIN_TG_ARN ?? ''), + String(process.env.INBOUND_TG_ARN ?? ''), String(process.env.FILESYSTEMID ?? ''), String(process.env.ECS_SUBNET_ID ?? ''), + String(process.env.ECS_SECURITY_GROUP_ID ?? '') +];apps/agent-provisioning/AFJ/scripts/start_agent_ecs.sh (1)
291-291: aws ecs describe-services flag: use --services (plural) and quote varsThe CLI requires --services. Also quote variables to avoid word-splitting.
-service_description=$(aws ecs describe-services --service $SERVICE_NAME --cluster $CLUSTER_NAME --region $AWS_PUBLIC_REGION) +service_description=$(aws ecs describe-services --services "$SERVICE_NAME" --cluster "$CLUSTER_NAME" --region "$AWS_PUBLIC_REGION")
🧹 Nitpick comments (12)
Dockerfiles/Dockerfile.agent-provisioning (2)
12-12: Install jq only where it's needed (drop from build stage).Unless your build steps call jq (they don’t appear to), installing it in the build stage only inflates the layer. Keep jq in the runtime image where the AFJ scripts use it.
Apply this minimal change:
@@ docker-compose \ - jq \
50-50: Runtime jq usage confirmed; pin image or package versions.
- Verified
jqis referenced inapps/agent-provisioning/AFJ/scripts/start_agent_ecs.shandfargate.sh. To reduce drift, pin thenode:18-alpinebase image to a digest and/or specify a fixed version forjqin theapk addcommand withinDockerfiles/Dockerfile.agent-provisioning.apps/agent-provisioning/src/agent-provisioning.service.ts (2)
69-75: Return a precise type instead of PromiseClarify the contract for callers and help static analysis.
-async walletProvision(payload: IWalletProvision): Promise<object> { +async walletProvision(payload: IWalletProvision): Promise<{ agentEndPoint: string; agentToken: string }> {
85-91: Use async fs APIs consistently or drop unnecessary awaitscheckFileExistence uses sync calls; either switch to fs/promises or remove awaits on sync functions to avoid misleading async semantics.
apps/agent-provisioning/AFJ/scripts/start_agent_ecs.sh (4)
303-323: Readiness check won’t work on ECS hosts; replace netstat/localhost probeThis is running off-host; netstat won’t see container ports. Prefer waiting for service stability and/or ALB target health.
- until [ "$n" -ge 6 ]; do - if netstat -tln | grep ${ADMIN_PORT} >/dev/null; then - AGENTURL="http://${EXTERNAL_IP}:${ADMIN_PORT}/agent" - agentResponse=$(curl -s -o /dev/null -w "%{http_code}" $AGENTURL) + until [ "$n" -ge 12 ]; do + svc_status=$(aws ecs describe-services --services "$SERVICE_NAME" --cluster "$CLUSTER_NAME" --region "$AWS_PUBLIC_REGION" | jq -r '.services[0].status') + agentResponse=$(curl -s -o /dev/null -w "%{http_code}" "http://${EXTERNAL_IP}:${ADMIN_PORT}/agent" || true)
330-336: Deriving task_id from events is brittle; use list-tasks/describe-tasksEvents text changes break parsing. Prefer the API.
-task_id=$(echo "$service_description" | jq -r ' - .services[0].events[] - | select(.message | test("has started 1 tasks")) - | .message - | capture("\(task (?<id>[^)]+)\)") - | .id -') +task_arn=$(aws ecs list-tasks --cluster "$CLUSTER_NAME" --service-name "$SERVICE_NAME" --region "$AWS_PUBLIC_REGION" --query 'taskArns[0]' --output text) +task_id="${task_arn##*/}"
361-371: Quote variables and prefer filter-log-events for token searchDefensive quoting and a single-pass filter reduces bandwidth.
- token=$(aws logs get-log-events \ - --log-group-name "$log_group" \ - --log-stream-name "$log_stream" \ - --region $AWS_PUBLIC_REGION \ - --query 'events[*].message' \ - --output text \ - | tr -d '\033' \ - | grep 'API Key:' \ - | sed -E 's/.*API Key:[[:space:]]*([a-zA-Z0-9._:-]*).*/\1/' \ - | head -n 1 -) + token=$(aws logs filter-log-events \ + --log-group-name "$log_group" \ + --log-stream-names "$log_stream" \ + --filter-pattern '"API Key:"' \ + --region "$AWS_PUBLIC_REGION" \ + --query 'events[-1].message' --output text \ + | sed -E 's/.*API Key:[[:space:]]*([a-zA-Z0-9._:-]*).*/\1/' | head -n1)
345-345: Typo: logstrem → log_streamCosmetic, but aids grepping.
-echo "logstrem=$log_stream" +echo "log_stream=$log_stream"apps/agent-provisioning/AFJ/scripts/fargate.sh (4)
257-303: Quote variables; ensure networkConfiguration values are correct; save JSON then validateSafer CLI invocation and easier troubleshooting.
-echo "$SERVICE_JSON" > service.json +echo "$SERVICE_JSON" > service.json && jq empty service.json
343-351: Use ECS list-tasks/describe-tasks for task_id, not event text parsingSame rationale as EC2 script.
-task_id=$(echo "$service_description" | jq -r ' - .services[0].events[] - | select(.message | test("has started 1 tasks")) - | .message - | capture("\(task (?<id>[^)]+)\)") - | .id -') +task_arn=$(aws ecs list-tasks --cluster "$CLUSTER_NAME" --service-name "$SERVICE_NAME" --region "$AWS_PUBLIC_REGION" --query 'taskArns[0]' --output text) +task_id="${task_arn##*/}"
375-385: Prefer filter-log-events and quote variables while scraping the tokenSame improvement as EC2 script.
- token=$(aws logs get-log-events \ - --log-group-name "$log_group" \ - --log-stream-name "$log_stream" \ - --region $AWS_PUBLIC_REGION \ - --query 'events[*].message' \ - --output text \ - | tr -d '\033' \ - | grep 'API Key:' \ - | sed -E 's/.*API Key:[[:space:]]*([a-zA-Z0-9._:-]*).*/\1/' \ - | head -n 1 -) + token=$(aws logs filter-log-events \ + --log-group-name "$log_group" \ + --log-stream-names "$log_stream" \ + --filter-pattern '"API Key:"' \ + --region "$AWS_PUBLIC_REGION" \ + --query 'events[-1].message' --output text \ + | sed -E 's/.*API Key:[[:space:]]*([a-zA-Z0-9._:-]*).*/\1/' | head -n1)
402-421: Minor: consolidate duplicate endpoint/token writes and reduce noisy logsAvoid writing token twice; keep one definitive write location.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Dockerfiles/Dockerfile.agent-provisioning(2 hunks)
apps/agent-provisioning/AFJ/scripts/fargate.sh(12 hunks)
apps/agent-provisioning/AFJ/scripts/start_agent_ecs.sh(9 hunks)
apps/agent-provisioning/src/agent-provisioning.service.ts(1 hunks)🧰 Additional context used
🧬 Code graph analysis (1)
apps/agent-provisioning/src/agent-provisioning.service.ts (3)
libs/logger/src/logger.interface.ts (1)
Logger(6-20)apps/agent-service/src/interface/agent-service.interface.ts (1)
IWalletProvision(144-164)apps/agent-provisioning/src/interface/agent-provisioning.interfaces.ts (1)
IWalletProvision(4-25)🪛 Shellcheck (0.10.0)
apps/agent-provisioning/AFJ/scripts/fargate.sh
[warning] 103-103: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 109-109: In POSIX sh, [[ ]] is undefined.
(SC3010)
[warning] 112-112: AGENT_ENDPOINT appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 315-315: This $? refers to echo/printf, not a previous command. Assign to variable to avoid it being overwritten.
(SC2320)
🪛 GitHub Check: CodeQL
apps/agent-provisioning/src/agent-provisioning.service.ts
[warning] 31-31: Shell command built from environment values
This shell command depends on an uncontrolled absolute path.🔇 Additional comments (3)
apps/agent-provisioning/AFJ/scripts/start_agent_ecs.sh (1)
159-211: Container definitions JSON validation passed
Validated with jq; JSON structure is correct.apps/agent-provisioning/AFJ/scripts/fargate.sh (2)
99-116: Portability: [[ … ]] isn’t POSIX sh; with bash shebang it’s fine. Otherwise rewrite to case/[ ]Ensure the shebang change above lands; else rework these tests.
221-246: Task definition: keep runtimePlatform/CPU/Memory consistent with launch typeYou set FARGATE; CPU/Memory must match allowed Fargate combos. Ensure these values are valid for your region/platform.
| "image": "${AFJ_VERSION}", | ||
| "cpu": 307, | ||
| "memory": 358, | ||
| "portMappings": [ | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fail fast if AFJ_VERSION is unset/invalid
Prevents empty image in task definition.
-AFJ_VERSION=${14}
+AFJ_VERSION=${14}
+: "${AFJ_VERSION:?AFJ_VERSION is required and must be a full image URI}"📝 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.
| "image": "${AFJ_VERSION}", | |
| "cpu": 307, | |
| "memory": 358, | |
| "portMappings": [ | |
| # assign the 14th argument to AFJ_VERSION | |
| AFJ_VERSION=${14} | |
| # fail fast if AFJ_VERSION is unset or empty | |
| : "${AFJ_VERSION:?AFJ_VERSION is required and must be a full image URI}" | 
🤖 Prompt for AI Agents
In apps/agent-provisioning/AFJ/scripts/fargate.sh around lines 162-165, the task
JSON is populated with "image": "${AFJ_VERSION}" but there's no validation,
which can produce an empty image field; add a fail-fast check before generating
the task definition that verifies AFJ_VERSION is set and non-empty (and
optionally matches a simple image pattern like containing a ':' or '@' if you
want stricter validation), print an explicit error to stderr if invalid, and
exit with a non-zero status so the script aborts instead of emitting a task
definition with an empty image.
| @@ -552,50 +312,52 @@ | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aws ecs describe-services: use --services (plural) and quote
Same as the EC2 script.
-service_description=$(aws ecs describe-services --service $SERVICE_NAME --cluster $CLUSTER_NAME --region $AWS_PUBLIC_REGION)
+service_description=$(aws ecs describe-services --services "$SERVICE_NAME" --cluster "$CLUSTER_NAME" --region "$AWS_PUBLIC_REGION")🤖 Prompt for AI Agents
In apps/agent-provisioning/AFJ/scripts/fargate.sh around line 305, the aws ecs
call uses the singular flag and unquoted variables; change --service to
--services and quote the variables so the command becomes --services
"$SERVICE_NAME" --cluster "$CLUSTER_NAME" (this also safely supports service
names with spaces or multiple services) and keep --region "$AWS_PUBLIC_REGION"
quoted for consistency.
|  | ||
| n=0 | ||
| until [ "$n" -ge 6 ]; do | ||
| if netstat -tln | grep ${ADMIN_PORT} >/dev/null; then | ||
|  | ||
| AGENTURL="http://${EXTERNAL_IP}:${ADMIN_PORT}/agent" | ||
| agentResponse=$(curl -s -o /dev/null -w "%{http_code}" $AGENTURL) | ||
|  | ||
| if [ "$agentResponse" = "200" ]; then | ||
| echo "Agent is running" && break | ||
| else | ||
| echo "Agent is not running" | ||
| n=$((n + 1)) | ||
| sleep "$sleep_time" | ||
| sleep 10 | ||
| fi | ||
| else | ||
| echo "No response from agent" | ||
| n=$((n + 1)) | ||
| sleep 10 | ||
| fi | ||
| done | ||
|  | ||
|  | ||
| done | ||
|  | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
$? misuse and off-host readiness probe
Capture exit code immediately; avoid netstat on control host; rely on ECS/ALB health or a direct HTTP probe to the LB.
-aws ecs create-service \
-    --cli-input-json file://service.json \
-    --region $AWS_PUBLIC_REGION 
+aws ecs create-service --cli-input-json file://service.json --region "$AWS_PUBLIC_REGION"
+create_rc=$?
 ...
-if [ $? -eq 0 ]; then
+if [ "$create_rc" -eq 0 ]; then
 ...
-    if netstat -tln | grep ${ADMIN_PORT} >/dev/null; then
+    # Poll service status instead of local netstat
+    svc_status=$(aws ecs describe-services --services "$SERVICE_NAME" --cluster "$CLUSTER_NAME" --region "$AWS_PUBLIC_REGION" | jq -r '.services[0].status')Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 315-315: This $? refers to echo/printf, not a previous command. Assign to variable to avoid it being overwritten.
(SC2320)
🤖 Prompt for AI Agents
In apps/agent-provisioning/AFJ/scripts/fargate.sh around lines 315 to 336, the
script misuses $? by checking it later and uses netstat on the control host to
detect container readiness; immediately save the previous command's exit code
into a variable (e.g., prev_exit=$?) before any other commands, remove the
netstat-based host check entirely, and replace the loop logic to directly probe
the agent HTTP endpoint (AGENTURL="http://${EXTERNAL_IP}:${ADMIN_PORT}/agent")
via curl, retrying up to 6 times with sleeps and breaking only when curl returns
HTTP 200; alternatively, if an ALB/ECS health check is preferred, query the load
balancer health API or rely on ECS task/ALB health status instead of netstat.
| AGENT_API_KEY=${18} | ||
| AWS_ACCOUNT_ID=${19} | ||
| S3_BUCKET_ARN=${20} | ||
| CLUSTER_NAME=${21} | ||
| TESKDEFINITION_FAMILY=${22} | ||
| TASKDEFINITION_FAMILY=${22} | ||
| ADMIN_TG_ARN=${23} | ||
| INBOUND_TG_ARN=${24} | ||
| FILESYSTEMID=${25} | ||
|  | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Arg mismatch with service caller: extra args ignored here
The service passes ECS_SUBNET_ID/ECS_SECURITY_GROUP_ID too; this script doesn’t read them. Confirm the intended spin-up path and align args to avoid future drift.
🏁 Script executed:
#!/bin/bash
# Show all call sites for AFJ_AGENT_SPIN_UP and compare arity
rg -nP 'AFJ_AGENT_SPIN_UP' -g '!**/node_modules/**'Length of output: 148
🏁 Script executed:
#!/bin/bash
# Search for AFJ_AGENT_SPIN_UP references across all files
rg -n "AFJ_AGENT_SPIN_UP" -C3 .
# Search for direct invocations of the start_agent_ecs.sh script
rg -n "start_agent_ecs\.sh" -C3 .Length of output: 2133
Consume ECS_SUBNET_ID and ECS_SECURITY_GROUP_ID in start_agent_ecs.sh
The TypeScript service passes ECS_SUBNET_ID and ECS_SECURITY_GROUP_ID as arguments 26/27 but the script only reads up to $25, so those values are ignored.
apps/agent-provisioning/AFJ/scripts/start_agent_ecs.sh (around lines 22–30): append
ECS_SUBNET_ID=${26}
ECS_SECURITY_GROUP_ID=${27}
🤖 Prompt for AI Agents
In apps/agent-provisioning/AFJ/scripts/start_agent_ecs.sh around lines 22 to 30,
the script stops reading positional parameters at ${25} so the ECS_SUBNET_ID and
ECS_SECURITY_GROUP_ID passed as args 26 and 27 are ignored; add two lines to
capture them as ECS_SUBNET_ID=${26} and ECS_SECURITY_GROUP_ID=${27} immediately
after the existing FILESYSTEMID assignment so the script consumes and uses the
subnet and security group values.
| if [ $? -eq 0 ]; then | ||
| echo "Service creation successful" | ||
| else | ||
| echo "Failed to create service" | ||
| exit 1 | ||
| fi | ||
|  | ||
| if [ $? -eq 0 ]; then | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
$? misuse after echo; capture status immediately
$? here refers to the previous echo/assignment, not the create-service call. Capture the status right after the command.
-aws ecs create-service \
-  --cli-input-json file://service.json \
-  --region $AWS_PUBLIC_REGION
-
-# Describe the ECS service and filter by service name
-service_description=$(aws ecs describe-services --service $SERVICE_NAME --cluster $CLUSTER_NAME --region $AWS_PUBLIC_REGION)
-
-# Check if the service creation was successful
-if [ $? -eq 0 ]; then
+aws ecs create-service \
+  --cli-input-json file://service.json \
+  --region "$AWS_PUBLIC_REGION"
+create_rc=$?
+
+service_description=$(aws ecs describe-services --services "$SERVICE_NAME" --cluster "$CLUSTER_NAME" --region "$AWS_PUBLIC_REGION")
+
+if [ "$create_rc" -eq 0 ]; then📝 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.
| if [ $? -eq 0 ]; then | |
| echo "Service creation successful" | |
| else | |
| echo "Failed to create service" | |
| exit 1 | |
| fi | |
| if [ $? -eq 0 ]; then | |
| aws ecs create-service \ | |
| --cli-input-json file://service.json \ | |
| --region "$AWS_PUBLIC_REGION" | |
| create_rc=$? | |
| service_description=$(aws ecs describe-services --services "$SERVICE_NAME" --cluster "$CLUSTER_NAME" --region "$AWS_PUBLIC_REGION") | |
| if [ "$create_rc" -eq 0 ]; then | |
| echo "Service creation successful" | |
| else | |
| echo "Failed to create service" | |
| exit 1 | |
| fi | |
| if [ $? -eq 0 ]; then | 
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 301-301: This $? refers to echo/printf, not a previous command. Assign to variable to avoid it being overwritten.
(SC2320)
🤖 Prompt for AI Agents
In apps/agent-provisioning/AFJ/scripts/start_agent_ecs.sh around lines 294 to
301, the script checks $? after an echo so it no longer reflects the exit status
of the service creation command; capture the exit status immediately after the
create-service command (e.g., status=$? or use the command || { echo ...; exit
1; }) and then test that stored variable (if [ $status -eq 0 ]; then ...) or use
the immediate logical check to ensure you are testing the create-service result
rather than the echo's exit status.
| const { | ||
| containerName, | ||
| externalIp, | ||
| orgId, | ||
| seed, | ||
| walletName, | ||
| walletPassword, | ||
| walletStorageHost, | ||
| walletStoragePassword, | ||
| walletStoragePort, | ||
| walletStorageUser, | ||
| webhookEndpoint, | ||
| agentType, | ||
| protocol, | ||
| credoImage, | ||
| tenant, | ||
| indyLedger, | ||
| inboundEndpoint | ||
| } = payload; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Validate and constrain payload before invoking provisioning scripts
Inputs like walletName, walletPassword, seed, containerName, protocol, tenant, inboundEndpoint etc. are used to drive infra. Add DTO validation (class-validator) and conservative whitelists/length caps to block bad characters and unsafe lengths.
🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 31-31: Shell command built from environment values
This shell command depends on an uncontrolled absolute path.
🤖 Prompt for AI Agents
In apps/agent-provisioning/src/agent-provisioning.service.ts around lines 21 to
39, the request payload is destructured and passed into provisioning logic
without validation; add a DTO class using class-validator and class-transformer,
validate payload at the service entry point, and reject invalid input before
running scripts. Create a ProvisionAgentDto listing the fields (containerName,
externalIp, orgId, seed, walletName, walletPassword, walletStorageHost,
walletStoragePassword, walletStoragePort, walletStorageUser, webhookEndpoint,
agentType, protocol, credoImage, tenant, indyLedger, inboundEndpoint) and apply
conservative decorators: allow only expected characters via
Matches/IsAlphanumeric or custom regex for names, hostnames and endpoints;
IsIP/IsFQDN for externalIp/walletStorageHost; IsPort for ports;
MinLength/MaxLength for seed, passwords and names (e.g. seed 16-128, passwords
8-128, names 1-64); whitelist tenant/agentType/protocol values with IsIn; ensure
inboundEndpoint/webhookEndpoint use IsUrl with allowed protocols or custom
regex; mark optional fields with IsOptional. In the service, run
validateSync/validate and throw a BadRequestError with collected validation
messages if invalid, and ensure any values passed to shell/infra are
escaped/sanitized or passed as environment variables rather than interpolated
into command strings.


What
Summary by CodeRabbit
New Features
Improvements
Chores