add models, utils->( responseFormat, securityutils ), scripts ->initsql - #4
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an Express API with an auth domain (models, repositories, services, controllers, routes), response/error utilities, validation and auth middlewares, Mongo/Postgres/RabbitMQ connection wiring and graceful shutdown, a Postgres metrics table and trigger, Docker/Docker‑Compose files, ESM/module and path aliasing, and a set of config/runtime tweaks. ChangesAPI service + persistence schema
Core application logic & auth feature
Middleware, logging, and connectivity
Runtime / deployment / scripts
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant MongoDB as MongoDB(rgba(100,150,250,0.5))
participant Postgres as Postgres(rgba(100,250,150,0.5))
participant RabbitMQ as RabbitMQ(rgba(250,150,100,0.5))
Client->>Server: startServer()
Server->>MongoDB: mongo.connect()
MongoDB-->>Server: connected
Server->>Postgres: postgres.testConnection()
Postgres-->>Server: connection OK
Server->>RabbitMQ: rabbitmq.connect()
RabbitMQ-->>Server: channel ready
Server-->>Client: HTTP server listening
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/package.json`:
- Around line 7-10: The dev script references nodemon but package.json lacks
nodemon in devDependencies; add "nodemon" to package.json's devDependencies
(e.g., under the existing "scripts" object) or run npm install --save-dev
nodemon so the "dev" script ("dev": "nodemon src/server.js") works reliably;
update package.json's devDependencies entry accordingly and ensure package-lock
is updated.
In `@server/src/shared/models/apikey.js`:
- Around line 16-21: The schema currently stores raw API secrets in the keyValue
field; change it to store a public identifier plus a one-way hash instead:
replace or augment keyValue with something like keyId (public lookup) and
keyHash (the hashed secret), add a pre-save hook on the ApiKey model to hash the
secret using a strong KDF (e.g., bcrypt/argon2 or PBKDF2) before persisting, and
update the authentication path to fetch by keyId and verify using the
corresponding compare function (bcrypt.compare / timing-safe hash compare)
rather than comparing raw values; also plan a migration to rotate or re-hash
existing raw secrets if any remain.
In `@server/src/shared/models/user.js`:
- Around line 66-76: The schema has a typo: the field is named "ole" but other
logic (the clientId.required function and the role index) reference "role",
causing incorrect validation and a broken index; rename the "ole" field to
"role" (or consistently update all dependent checks to "ole") so the enum,
default, clientId.required function and any index use the same field name (e.g.,
update the field definition from ole to role and keep clientId.required's
this.role check and the role index intact).
- Around line 107-120: The pre-save middleware on userSchema is broken because
it uses an arrow function (so this.isModified and this.password are undefined)
and bcrypt is never imported; change the middleware to a normal function
declaration (e.g., function(next) or async function(next)) so this refers to the
document, ensure you require/import bcrypt at the top of the file (e.g., const
bcrypt = require('bcrypt') or import bcrypt from 'bcrypt'), and keep the
existing try/catch and next(error) behavior in the userSchema.pre("save", ...)
handler so passwords are hashed before persistence.
- Around line 38-63: The password field currently uses top-level
validator/message instead of Mongoose's validate option—wrap those into
validate: { validator, message } on the password schema entry; replace
SecurityUtils.validatePassword calls with the actual exported passwordValidate
(import it from the utility module) and import bcrypt at top for hashing; fix
the schema field name typo from ole to role and update any references (clientId
conditional and index) to use role so role-based logic and indexes work; and
change the pre-save hook to a regular function (schema.pre('save', async
function(next) { ... })) so this is bound to the document during bcrypt hashing.
In `@server/src/shared/utils/responseFormat.js`:
- Around line 32-40: The paginated static method can produce NaN/Infinity for
totalPages when limit is missing or non-positive; update the
ResponseFormat.paginated function to validate/guard the incoming page/limit
values (e.g., ensure limit is a positive integer or coerce to a safe default
like 1, and if total is 0 set totalPages to 0) before computing totalPages, and
compute totalPages using a safe expression (for example only call
Math.ceil(total / limit) after confirming limit > 0; otherwise set totalPages to
0 or Math.max(1, ... ) as appropriate) so pagination metadata is always finite
and valid.
In `@server/src/shared/utils/securityUtils.js`:
- Around line 3-13: The PASSWORD_REQUIREMENTS.minLength uses parseInt on
process.env.PASSWORD_MIN_LENGTH which can produce NaN and silently disable
length checks; update the PASSWORD_REQUIREMENTS initialization in
securityUtils.js to parse with radix 10 and validate the result (e.g., use
Number.isFinite or Number.isInteger and ensure >= 0) falling back to the default
(8) when the env value is missing or invalid; keep the same symbol name
PASSWORD_REQUIREMENTS and only change how minLength is derived so downstream
checks using password.length < PASSWORD_REQUIREMENTS.minLength behave correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 02b9e511-d3e7-43be-90f2-66f0f4b8a687
📒 Files selected for processing (10)
server/package.jsonserver/scripts/init_postgres.sqlserver/src/server.jsserver/src/shared/config/mongo.jsserver/src/shared/models/apihits.jsserver/src/shared/models/apikey.jsserver/src/shared/models/client.jsserver/src/shared/models/user.jsserver/src/shared/utils/responseFormat.jsserver/src/shared/utils/securityUtils.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
server/src/services/auth/repositories/baseRepository.js (1)
1-26:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing export makes this class unusable outside the file.
There is no export after the class definition, so other modules cannot import
BaseRepository(Line 26 currently just closes the class).Suggested fix
class BaseRepository { @@ } + +export default BaseRepository;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/services/auth/repositories/baseRepository.js` around lines 1 - 26, The BaseRepository class is declared but not exported, so modules cannot import it; export the class by adding an export (e.g., export default BaseRepository or module.exports = BaseRepository) after the class declaration so other files can import BaseRepository; ensure the chosen export style matches the project's module system (ESM vs CommonJS) and update any imports that reference BaseRepository accordingly.server/src/shared/config/rabbitmq.js (1)
17-27:⚠️ Potential issue | 🟠 Major | ⚡ Quick winConcurrent callers can get a
nullchannel after a failed connect.If the first
connect()attempt throws, callers waiting in this branch wake up and returnthis.channel, which is stillnull. That masks the startup failure and pushes the breakage downstream.Suggested fix
class RabbitMQConnection { constructor() { this.connection = null; this.channel = null; this.isConnecting = false; + this.connectPromise = null; } async connect() { if (this.connection) { logger.info("RabbitMQ connection already exists"); return this.channel; } - if (this.isConnecting) { - logger.info("RabbitMQ is already connecting..."); - await new Promise((resolve) => { - const checkInterval = setInterval(() => { - if (!this.isConnecting) { - clearInterval(checkInterval); - resolve(); - } - }, 100); - }); - return this.channel; + if (this.connectPromise) { + logger.info("RabbitMQ is already connecting..."); + return this.connectPromise; } - try { + this.connectPromise = (async () => { this.isConnecting = true; - this.connection = await amqp.connect(config.rabbitmq.uri); - this.channel = await this.connection.createChannel(); + try { + this.connection = await amqp.connect(config.rabbitmq.uri); + this.channel = await this.connection.createChannel(); - const deadLetterQueue = `${config.rabbitmq.queue}.dlq`; - await this.channel.assertQueue(deadLetterQueue, { durable: true }); + const deadLetterQueue = `${config.rabbitmq.queue}.dlq`; + await this.channel.assertQueue(deadLetterQueue, { durable: true }); - const normalQueue = `${config.rabbitmq.queue}`; - await this.channel.assertQueue(normalQueue, { - durable: true, - arguments: { - "x-dead-letter-exchange": "", - "x-dead-letter-routing-key": deadLetterQueue, - }, - }); - logger.info("RabbitMQ connected, Queue: ", config.rabbitmq.queue); + const normalQueue = `${config.rabbitmq.queue}`; + await this.channel.assertQueue(normalQueue, { + durable: true, + arguments: { + "x-dead-letter-exchange": "", + "x-dead-letter-routing-key": deadLetterQueue, + }, + }); + logger.info("RabbitMQ connected, Queue: ", config.rabbitmq.queue); - this.connection.on("close", () => { - logger.warn("RabbitMQ connection closed"); - this.connection = null; - this.channel = null; - this.isConnecting = false; - }); + this.connection.on("close", () => { + logger.warn("RabbitMQ connection closed"); + this.connection = null; + this.channel = null; + this.isConnecting = false; + }); - this.connection.on("error", () => { - logger.error("RabbitMQ connection error"); - this.connection = null; - this.channel = null; - }); + this.connection.on("error", () => { + logger.error("RabbitMQ connection error"); + this.connection = null; + this.channel = null; + }); - this.isConnecting = false; - return this.channel; - } catch (error) { - this.isConnecting = false; - logger.error("RabbitMq failed to Connect", error); - throw error; - } + return this.channel; + } catch (error) { + logger.error("RabbitMq failed to Connect", error); + throw error; + } finally { + this.isConnecting = false; + this.connectPromise = null; + } + })(); + + return this.connectPromise; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/shared/config/rabbitmq.js` around lines 17 - 27, The concurrent-wait branch in connect() uses this.isConnecting and returns this.channel after waiting, but if the initial connect failed callers will wake and get a null channel; fix by recording the failure inside connect() (e.g., set this.connectError in the catch and ensure this.isConnecting is cleared in finally) and, in the waiting loop after clearInterval, check if this.channel is null and if so throw the recorded this.connectError (or a new descriptive Error) instead of returning null; update references to this.isConnecting, this.channel and connect() accordingly.
🧹 Nitpick comments (2)
server/src/services/auth/repositories/baseRepository.js (1)
14-20: ⚡ Quick winKeep
BaseRepositorygeneric; move user-specific finders toUserRepository.
findByUsernameandfindByEmailare domain-specific and couple this base class to user fields. Prefer a generic finder in base (e.g.,findOne(criteria)) and keep username/email methods in a concrete user repository. This aligns with the user model fields inserver/src/shared/models/user.js:1-119.Possible direction
- async findByUsername(username) { - throw new Error("Method not implemented"); - } - - async findByEmail(email) { + async findOne(criteria) { throw new Error("Method not implemented"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/services/auth/repositories/baseRepository.js` around lines 14 - 20, BaseRepository currently defines user-specific methods findByUsername and findByEmail which couples the base class to the user domain; replace those with a generic finder and move the user-specific logic into the concrete UserRepository. In BaseRepository, remove or stop throwing for findByUsername/findByEmail and instead add a generic async findOne(criteria) (or similar) to accept arbitrary query objects; then implement async findByUsername(username) and async findByEmail(email) in UserRepository to call findOne({ username }) / findOne({ email }) and return the user. Update any callers to use UserRepository's methods or findOne as appropriate and ensure method names referenced are BaseRepository.findOne and UserRepository.findByUsername/UserRepository.findByEmail.server/docker-compose.yml (1)
83-83: ⚡ Quick winAdd MongoDB to
api-appstartup dependencies.Line 83 configures
MONGO_URI, but Line 106-110 only gates on PostgreSQL and RabbitMQ. Add MongoDB independs_onto reduce startup race/flakiness.♻️ Proposed fix
api-app: depends_on: + mongo: + condition: service_started postgres: condition: service_healthy rabbitmq: condition: service_healthyAlso applies to: 106-110
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/docker-compose.yml` at line 83, The api-app service sets MONGO_URI but its depends_on only includes PostgreSQL and RabbitMQ, causing startup races; update the api-app docker-compose service definition (the service block that contains the MONGO_URI environment entry) to include the mongo service in its depends_on list alongside the existing postgres and rabbitmq entries so Docker waits for MongoDB before starting api-app.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/docker-compose.yml`:
- Line 8: The compose file contains hardcoded plaintext credentials (e.g.,
POSTGRES_PASSWORD and embedded basic-auth in AMQP/connection URLs) — replace
these literals with environment variables or Docker secrets and stop embedding
creds inside URLs; update the POSTGRES_PASSWORD entry and any connection strings
that currently include user:pass@host to reference placeholders like
${POSTGRES_PASSWORD} or a secret name, add corresponding entries to an .env or
secrets block, and adjust any services that consume AMQP/DB URLs (the AMQP URL
and DB connection entries) to read credentials from those env vars/secrets
instead of inline values so secrets can be rotated and managed externally.
- Around line 112-113: The healthcheck currently uses curl in the healthcheck
test (test: ["CMD", "curl", "-f", "http://localhost:5000/health"]) but the
node:18-alpine image doesn't include curl; update either the Dockerfile to
install curl (e.g., add apk add --no-cache curl) or change the docker-compose
healthcheck to use a tool present in the image (e.g., wget) or a Node-based
check (invoking node to GET /health), ensuring you update the healthcheck test
entry and/or Dockerfile installation so the health probe succeeds.
In `@server/Dockerfile`:
- Around line 1-10: The image currently runs as root; make /app writable by and
switch to the existing node user so the container runs non-root. After
creating/copying files and the logs directory (WORKDIR /app and RUN mkdir -p
logs), ensure ownership is changed to node (e.g., chown -R node:node /app and
chown node:node logs) and then add USER node before the CMD (CMD
["node","src/server.js"]) so the process runs as the node user.
In `@server/src/server.js`:
- Around line 60-62: The imported error handler (named errorhandler) is never
registered so errors bypass your JSON formatter; after the existing 404
middleware (the app.use((req, res) => {...}) block) register the imported error
handler by calling app.use with the imported symbol (e.g.,
app.use(errorhandler)) so it is mounted as the final error-handling middleware
and will receive thrown AppError instances and route errors.
- Around line 42-58: The root handler currently registered with app.use("/")
intercepts all methods and subpaths; change it to an exact GET route so other
endpoints and the 404 handler can run: replace the app.use("/") registration
with app.get("/", ...) (keeping the ResponseFormat.success body and status) so
only GET / is handled and other routes like the auth/ingest/analytics handlers
or the 404 middleware are reachable.
- Around line 91-105: The gracefulShutdown handler closes the HTTP server but
does not wait for server.close() to finish before closing DBs, causing in-flight
requests to fail; modify gracefulShutdown (the function named gracefulShutdown)
to wrap server.close(...) in a Promise, await that Promise (and handle errors
from server.close) before calling mongo.disconnect(), postgres.close(), and
rabbitmq.close(), and preserve the existing logger.info/logger.error calls and
process.exit behavior so databases are closed only after the HTTP server
callback has run.
In `@server/src/services/auth/repositories/baseRepository.js`:
- Around line 2-4: The BaseRepository constructor accepts a model without
validation so misconfigured repositories fail later; update the constructor
(constructor in BaseRepository / baseRepository.js) to validate the incoming
model and fail fast by throwing a clear error if model is null/undefined or not
the expected shape (e.g., not an object or missing key methods like find/create
if applicable). Set a descriptive error (e.g., "BaseRepository requires a valid
model") and only assign this.model when the check passes so invalid setup is
caught immediately.
In `@server/src/shared/config/index.js`:
- Around line 5-10: The readInt function currently allows malformed strings
because Number.parseInt tolerates trailing non-digits; update readInt to
validate the input string strictly before parsing (e.g., require it matches an
integer regex like /^[+-]?\d+$/ or equivalent) and only then parse and return
the integer, otherwise throw the existing Invalid integer error; keep the
existing fallback behavior for null/empty values and reference the readInt
helper to locate the change.
In `@server/src/shared/middlewares/errorHandler.js`:
- Around line 4-7: The error handler is reading the status from req.statusCode
which is incorrect; update the errorhandler middleware to use err.statusCode
(falling back to 500) so custom AppError status codes are preserved — locate the
errorhandler function in errorHandler.js and replace the reference to
req.statusCode with err.statusCode, keeping the existing defaults for message
and errors and leaving the rest of the name-based overrides intact.
In `@server/src/shared/models/apikey.js`:
- Around line 102-105: The default expiry computation inside the model's default
function for API key expiration uses parseInt unsafely; update it to use
Number.parseInt(..., 10), validate the parsed value with
Number.isFinite(expiryDays) && expiryDays > 0, and if validation fails fall back
to 365 days before computing the Date; reference the API_KEY_EXPIRY_DAYS env var
and the default: () => { ... } function so you replace the raw parseInt usage
with this validated parsing and fallback.
---
Outside diff comments:
In `@server/src/services/auth/repositories/baseRepository.js`:
- Around line 1-26: The BaseRepository class is declared but not exported, so
modules cannot import it; export the class by adding an export (e.g., export
default BaseRepository or module.exports = BaseRepository) after the class
declaration so other files can import BaseRepository; ensure the chosen export
style matches the project's module system (ESM vs CommonJS) and update any
imports that reference BaseRepository accordingly.
In `@server/src/shared/config/rabbitmq.js`:
- Around line 17-27: The concurrent-wait branch in connect() uses
this.isConnecting and returns this.channel after waiting, but if the initial
connect failed callers will wake and get a null channel; fix by recording the
failure inside connect() (e.g., set this.connectError in the catch and ensure
this.isConnecting is cleared in finally) and, in the waiting loop after
clearInterval, check if this.channel is null and if so throw the recorded
this.connectError (or a new descriptive Error) instead of returning null; update
references to this.isConnecting, this.channel and connect() accordingly.
---
Nitpick comments:
In `@server/docker-compose.yml`:
- Line 83: The api-app service sets MONGO_URI but its depends_on only includes
PostgreSQL and RabbitMQ, causing startup races; update the api-app
docker-compose service definition (the service block that contains the MONGO_URI
environment entry) to include the mongo service in its depends_on list alongside
the existing postgres and rabbitmq entries so Docker waits for MongoDB before
starting api-app.
In `@server/src/services/auth/repositories/baseRepository.js`:
- Around line 14-20: BaseRepository currently defines user-specific methods
findByUsername and findByEmail which couples the base class to the user domain;
replace those with a generic finder and move the user-specific logic into the
concrete UserRepository. In BaseRepository, remove or stop throwing for
findByUsername/findByEmail and instead add a generic async findOne(criteria) (or
similar) to accept arbitrary query objects; then implement async
findByUsername(username) and async findByEmail(email) in UserRepository to call
findOne({ username }) / findOne({ email }) and return the user. Update any
callers to use UserRepository's methods or findOne as appropriate and ensure
method names referenced are BaseRepository.findOne and
UserRepository.findByUsername/UserRepository.findByEmail.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3c374066-7e36-4219-9f87-22ccd7e2b758
⛔ Files ignored due to path filters (1)
server/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
server/Dockerfileserver/docker-compose.ymlserver/jsconfig.jsonserver/package.jsonserver/src/server.jsserver/src/services/auth/repositories/baseRepository.jsserver/src/shared/config/index.jsserver/src/shared/config/logger.jsserver/src/shared/config/mongo.jsserver/src/shared/config/rabbitmq.jsserver/src/shared/middlewares/errorHandler.jsserver/src/shared/models/apikey.jsserver/src/shared/models/user.jsserver/src/shared/utils/appError.js
✅ Files skipped from review due to trivial changes (2)
- server/package.json
- server/src/shared/config/logger.js
Comment on lines
+102
to
+105
| default: () => { | ||
| const days = parseInt(process.env.API_KEY_EXPIRY_DAYS || "365"); | ||
| return new Date(Date.now() + days * 24 * 60 * 60 * 1000); | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify expiry parsing has finite/positive guard
rg -n -C3 'API_KEY_EXPIRY_DAYS|parseInt|Number\.isFinite|parsed > 0' server/src/shared/models/apikey.jsRepository: NazimRiyadh/Owlpi
Length of output: 358
Harden API_KEY_EXPIRY_DAYS parsing to prevent invalid expiration dates.
The parseInt() call on line 103 lacks validation and can produce NaN (if the env var is set to a non-numeric string) or non-positive values (if set to "0" or negative numbers). This results in invalid dates that break the intended expiration behavior. Additionally, parseInt() without a radix parameter is deprecated and can misparse strings like those starting with "0x".
Use Number.parseInt() with radix 10, validate the result with Number.isFinite() and a positive check, and fall back to a safe default (365) when validation fails.
Suggested fix
expiresAt: {
type: Date,
default: () => {
- const days = parseInt(process.env.API_KEY_EXPIRY_DAYS || "365");
+ const parsed = Number.parseInt(
+ process.env.API_KEY_EXPIRY_DAYS || "365",
+ 10,
+ );
+ const days = Number.isFinite(parsed) && parsed > 0 ? parsed : 365;
return new Date(Date.now() + days * 24 * 60 * 60 * 1000);
},
index: true,
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| default: () => { | |
| const days = parseInt(process.env.API_KEY_EXPIRY_DAYS || "365"); | |
| return new Date(Date.now() + days * 24 * 60 * 60 * 1000); | |
| }, | |
| default: () => { | |
| const parsed = Number.parseInt( | |
| process.env.API_KEY_EXPIRY_DAYS || "365", | |
| 10, | |
| ); | |
| const days = Number.isFinite(parsed) && parsed > 0 ? parsed : 365; | |
| return new Date(Date.now() + days * 24 * 60 * 60 * 1000); | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/shared/models/apikey.js` around lines 102 - 105, The default
expiry computation inside the model's default function for API key expiration
uses parseInt unsafely; update it to use Number.parseInt(..., 10), validate the
parsed value with Number.isFinite(expiryDays) && expiryDays > 0, and if
validation fails fall back to 365 days before computing the Date; reference the
API_KEY_EXPIRY_DAYS env var and the default: () => { ... } function so you
replace the raw parseInt usage with this validated parsing and fallback.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (3)
server/docker-compose.yml (2)
8-8:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove hardcoded credentials from compose env and URLs.
Plaintext secrets are committed in multiple places (Line 8, Line 40-41, Line 62-63, Line 90, Line 92), including embedded basic-auth in
RABBITMQ_URL(Line 92). This is a security leak and blocks safe credential rotation.Proposed change
postgres: environment: POSTGRES_DB: api_monitoring_system POSTGRES_USER: postgres - POSTGRES_PASSWORD: password + POSTGRES_PASSWORD: ${POSTGRES_PASSWORD} rabbitmq: environment: - RABBITMQ_DEFAULT_USER: rabbitmq - RABBITMQ_DEFAULT_PASS: password - RABBITMQ_DEFAULT_VHOST: api_monitoring_system + RABBITMQ_DEFAULT_USER: ${RABBITMQ_DEFAULT_USER} + RABBITMQ_DEFAULT_PASS: ${RABBITMQ_DEFAULT_PASS} + RABBITMQ_DEFAULT_VHOST: ${RABBITMQ_DEFAULT_VHOST} pgadmin: environment: - PGADMIN_DEFAULT_EMAIL: admin@example.com - PGADMIN_DEFAULT_PASSWORD: admin + PGADMIN_DEFAULT_EMAIL: ${PGADMIN_DEFAULT_EMAIL} + PGADMIN_DEFAULT_PASSWORD: ${PGADMIN_DEFAULT_PASSWORD} api-app: environment: PG_HOST: postgres PG_PORT: 5432 PG_DATABASE: api_monitoring_system PG_USER: postgres - PG_PASSWORD: password - RABBITMQ_URL: amqp://rabbitmq:password@rabbitmq:5672/api_monitoring_system + PG_PASSWORD: ${PG_PASSWORD} + RABBITMQ_URL: ${RABBITMQ_URL}Also applies to: 40-41, 62-63, 90-92
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/docker-compose.yml` at line 8, Replace all hardcoded secrets and embedded basic-auth strings in the compose file (e.g., POSTGRES_PASSWORD, any POSTGRES_USER/POSTGRES_DB entries, DB URLs on lines around 40-41 and 62-63, and the RABBITMQ_URL/BASIC_AUTH entry) with references to environment variables or Docker secrets (e.g., use ${POSTGRES_PASSWORD}, ${RABBITMQ_URL} or docker secrets) and remove inline credentials from URLs; update service env declarations to read from a .env or secrets manager and ensure any BASIC_AUTH is built at runtime from secure values rather than being committed in the compose file.
112-113:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHealthcheck may fail if
curlis missing in the app image.The healthcheck command uses
curl(Line 113). Ifserver/Dockerfiledoesn’t install it, container health will stay failing even when the app is up.#!/bin/bash set -euo pipefail echo "== healthcheck command ==" rg -n 'healthcheck|curl|/health' server/docker-compose.yml -C2 echo "== Dockerfile base image and curl install check ==" rg -n '^\s*FROM\s+' server/Dockerfile rg -n 'apk add|apt-get install|microdnf install|curl' server/Dockerfile -C2 || trueIf no
curlinstall is found, switch to a Node-based probe or add curl explicitly inserver/Dockerfile.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/docker-compose.yml` around lines 112 - 113, The healthcheck uses curl (healthcheck.test invoking "curl" against /health) and will fail if the app image doesn't include curl; either add an explicit curl install to the image build (use the image's package manager in the Dockerfile and install curl) or replace the docker-compose healthcheck test with a probe that will always be present in the image (e.g., a small Node-based one-liner that requests /health or another available binary), ensuring you update the healthcheck.test entry accordingly.server/src/shared/models/user.js (1)
33-55:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winPassword policy validator is not wired to Mongoose validation.
At Line 37+,
validator/messageare defined at the field root. In Mongoose, custom validation must be undervalidate, otherwise password policy checks can be skipped.Suggested fix
password: { type: String, required: true, minlength: 8, - validator: function (password) { - if ( - this.isModified("password") && - password && - !password.startsWith("$2a$") - ) { - const validation = securityUtils.passwordValidate(password); - return validation.success; - } - return true; - }, - message: function (props) { - if (props.value && !props.value.startsWith("$2a$")) { - const validation = securityUtils.passwordValidate(props.value); - return validation.errors.join(". "); - } - return "Password validation failed"; + validate: { + validator: function (password) { + if ( + this.isModified("password") && + password && + !password.startsWith("$2a$") + ) { + const validation = securityUtils.passwordValidate(password); + return validation.success; + } + return true; + }, + message: function (props) { + if (props.value && !props.value.startsWith("$2a$")) { + const validation = securityUtils.passwordValidate(props.value); + return validation.errors.join(". "); + } + return "Password validation failed"; + }, }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/shared/models/user.js` around lines 33 - 55, The password field's custom validator and message are defined at the field root instead of under Mongoose's validate option, so the password policy is not enforced; update the password schema entry to use validate: { validator: function(password) { ... }, message: function(props) { ... } } (keeping the existing this.isModified("password") check and calls to securityUtils.passwordValidate) so Mongoose runs the custom validator; ensure the validator remains a non-arrow function to preserve this binding and preserve minlength/required settings.
🧹 Nitpick comments (2)
server/package.json (1)
36-46: MovenodemontodevDependencies.
nodemonis currently in runtime dependencies but only used by the dev script. Moving it to devDependencies reduces the production surface.Proposed change
"dependencies": { "amqplib": "^1.0.3", "bcryptjs": "^3.0.3", "cookie-parser": "^1.4.7", "cors": "^2.8.6", "dotenv": "^17.4.2", "express": "^5.2.1", "express-rate-limit": "^8.4.1", "helmet": "^8.1.0", "jsonwebtoken": "^9.0.3", "mongoose": "^9.6.1", - "nodemon": "^3.1.14", "pg": "^8.20.0", "uuid": "^14.0.0", "winston": "^3.19.0" }, "devDependencies": { + "nodemon": "^3.1.14", "@types/bcryptjs": "^2.4.6", "@types/cors": "^2.8.19", "@types/express": "^5.0.6", "@types/jsonwebtoken": "^9.0.10" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/package.json` around lines 36 - 46, The package.json lists "nodemon" under "dependencies" but it is only used for development; remove the "nodemon": "^3.1.14" entry from the "dependencies" section and add the same "nodemon": "^3.1.14" entry to the "devDependencies" section so the dev script continues to work while keeping production deps minimal; update package.json accordingly and verify existing scripts still reference nodemon correctly (no code changes required beyond the dependency move).server/src/shared/middlewares/errorHandler.js (1)
9-30: ⚡ Quick winLog after error normalization so status codes are accurate.
Line [11] currently logs
statusCodebefore Line [17]-[30] remaps it, so observability can record500while the response is400/401/409.Suggested fix
- logger.error("Error occurred:", { - message: err.message, - statusCode, - stack: err.stack, - path: req.path, - method: req.method, - }); - if (err.name === "ValidationError") { statusCode = 400; message = "Validation Error"; errors = Object.values(err.errors).map((e) => e.message); @@ statusCode = 401; message = "Token expired"; } + + logger.error("Error occurred:", { + message: err.message, + statusCode, + stack: err.stack, + path: req.path, + method: req.method, + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/shared/middlewares/errorHandler.js` around lines 9 - 30, The logger.error call is executed before the error normalization logic so it logs the pre-normalized statusCode; move or duplicate the log to occur after the normalization block that checks err.name (ValidationError, MongoServerError code 11000, JsonWebTokenError, TokenExpiredError) so the final statusCode/message reflect the mapped values; specifically update the error handler so logger.error(...) (the call that logs err.message, statusCode, stack, path, method) runs after the branches that set statusCode, message, and errors (or call logger.error again after normalization) to ensure observability records the normalized statusCode.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/src/services/auth/controllers/authController.js`:
- Around line 26-30: The auth cookie writes currently omit sameSite and the
cookie clear call doesn't mirror options; update every res.cookie usage in
authController.js (the places that set "authToken") to include sameSite (e.g.,
config.cookie.sameSite or 'Strict'/'Lax' per policy) alongside httpOnly, secure,
and maxAge, and update the res.clearCookie("authToken") call to pass the
identical options object (httpOnly, secure, maxAge if needed, and sameSite) so
clearing reliably matches the original cookie attributes.
- Line 87: In the authController.js login response, the success message string
"User logged in Successfullly" contains a typo; update the literal to "User
logged in successfully" (fix spelling and casing) inside the login
handler/function in the authController (look for the login or signIn method that
returns this message) so the response reads correctly.
- Around line 46-52: The registration handler currently reads role from req.body
when constructing userData (see the destructuring "const { username, email,
password, role }" and the userData object) which allows privilege escalation; to
fix, stop honoring the incoming role by hardcoding role:
APPLICATION_ROLES.CLIENT_VIEWER in userData (remove role from the destructure
and do not pass through req.body.role) before calling AuthService.register, or
alternatively enforce the tighter schema change by replacing isValidRole() with
isValidClientRole() in the validation layer (authSchema) so only client roles
are allowed; ensure AuthService.register receives only the safe client role.
In `@server/src/services/auth/repositories/userRepository.js`:
- Around line 67-69: The catch block inside the findAll() function incorrectly
logs "Error finding user by email"; update the logger.error call in findAll() to
a correct, descriptive message like "Error finding all users" (keeping the
existing error object parameter) so the log reflects the operation (reference:
findAll(), logger.error).
In `@server/src/services/auth/routes/authRoutes.js`:
- Around line 42-44: Replace the unauthenticated GET route with an authenticated
POST: change authRouter.get("/logout", requestLogger, authController.logout) to
authRouter.post("/logout", <authenticationMiddleware>, requestLogger,
(req,res,next) => authController.logout(req,res,next)). Ensure you use the
existing authentication middleware (e.g., ensureAuthenticated / requireAuth) so
logout is protected, keep authController.logout as the handler, and update any
clients/tests to call POST /logout instead of GET.
In `@server/src/services/auth/services/authService.js`:
- Around line 51-54: The auth service is returning full user objects (including
the password hash) from functions like onboard/register/login (the return blocks
around user/token at lines shown), so strip the sensitive field before
returning: create a sanitizedUser by omitting the password property (or
explicitly selecting allowed fields) and return that sanitized object with the
token instead of the raw user; update all similar return sites (e.g., the blocks
at lines ~51-54, ~84-87, ~115-118) to use the sanitizedUser.
- Around line 38-41: The onboarding gate currently uses
this.userRepository.findAll(), which checks any user rather than super-admins;
replace that call with a repository query scoped to the super-admin role (e.g.
use or add a method like userRepository.findByRole('super_admin') or
findAllByRole/findOneByRole) and ensure the check considers super-admins
regardless of active/inactive state (or explicitly includes inactive if
required) before throwing AppError in the auth service onboarding logic.
In `@server/src/services/auth/validate/authSchema.js`:
- Around line 10-26: The request validation allows password minLength 6 while
the User model enforces 8; update the password validators in authSchema.js—both
the top-level schema's password and the registrationSchema.password—to use
minLength: 8 so API validation matches server/src/shared/models/user.js and
prevents invalid payloads from passing request validation.
In `@server/src/shared/middlewares/authrorize.js`:
- Around line 13-25: The middleware incorrectly calls next() without returning,
causing double-dispatch when allowedRoles is empty; update the authorize
middleware so the empty-role guard returns immediately (e.g., change if
(allowedRoles.length === 0) { next(); } to return after calling next()) and
ensure the subsequent role-check block (which uses
allowedRoles.includes(req.user.role) and returns 403) is only reached when the
guard doesn't short-circuit, preventing multiple next()/response calls.
- Around line 26-27: The catch block currently converts all unexpected errors
into a 403 via res.status(403).json(ResponseFormat.error("Forbidden", 403)); —
instead only send 403 for known authorization failures and let other errors
propagate to centralized error handling: detect/handle explicit auth errors
where appropriate, and in the generic catch call next(error) (or return
res.status(500).json(ResponseFormat.error("Internal Server Error", 500)) if your
stack doesn't use next), removing the unconditional res.status(403) path so
internal/runtime failures are not masked; update the catch in the authorize
middleware accordingly.
In `@server/src/shared/middlewares/validate.js`:
- Around line 14-17: The required-field check in the validate middleware treats
whitespace-only strings as present; update the logic in the function handling
rules.required (the block using variable value) to treat strings that are empty
after trimming as missing by using a trimmedValue (or checking typeof value ===
'string' && value.trim() === '') alongside the existing undefined/null checks;
adjust subsequent validation to use the trimmedValue where appropriate so
blank-only strings are rejected for required fields.
---
Duplicate comments:
In `@server/docker-compose.yml`:
- Line 8: Replace all hardcoded secrets and embedded basic-auth strings in the
compose file (e.g., POSTGRES_PASSWORD, any POSTGRES_USER/POSTGRES_DB entries, DB
URLs on lines around 40-41 and 62-63, and the RABBITMQ_URL/BASIC_AUTH entry)
with references to environment variables or Docker secrets (e.g., use
${POSTGRES_PASSWORD}, ${RABBITMQ_URL} or docker secrets) and remove inline
credentials from URLs; update service env declarations to read from a .env or
secrets manager and ensure any BASIC_AUTH is built at runtime from secure values
rather than being committed in the compose file.
- Around line 112-113: The healthcheck uses curl (healthcheck.test invoking
"curl" against /health) and will fail if the app image doesn't include curl;
either add an explicit curl install to the image build (use the image's package
manager in the Dockerfile and install curl) or replace the docker-compose
healthcheck test with a probe that will always be present in the image (e.g., a
small Node-based one-liner that requests /health or another available binary),
ensuring you update the healthcheck.test entry accordingly.
In `@server/src/shared/models/user.js`:
- Around line 33-55: The password field's custom validator and message are
defined at the field root instead of under Mongoose's validate option, so the
password policy is not enforced; update the password schema entry to use
validate: { validator: function(password) { ... }, message: function(props) {
... } } (keeping the existing this.isModified("password") check and calls to
securityUtils.passwordValidate) so Mongoose runs the custom validator; ensure
the validator remains a non-arrow function to preserve this binding and preserve
minlength/required settings.
---
Nitpick comments:
In `@server/package.json`:
- Around line 36-46: The package.json lists "nodemon" under "dependencies" but
it is only used for development; remove the "nodemon": "^3.1.14" entry from the
"dependencies" section and add the same "nodemon": "^3.1.14" entry to the
"devDependencies" section so the dev script continues to work while keeping
production deps minimal; update package.json accordingly and verify existing
scripts still reference nodemon correctly (no code changes required beyond the
dependency move).
In `@server/src/shared/middlewares/errorHandler.js`:
- Around line 9-30: The logger.error call is executed before the error
normalization logic so it logs the pre-normalized statusCode; move or duplicate
the log to occur after the normalization block that checks err.name
(ValidationError, MongoServerError code 11000, JsonWebTokenError,
TokenExpiredError) so the final statusCode/message reflect the mapped values;
specifically update the error handler so logger.error(...) (the call that logs
err.message, statusCode, stack, path, method) runs after the branches that set
statusCode, message, and errors (or call logger.error again after normalization)
to ensure observability records the normalized statusCode.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1e5166ce-fda1-4aeb-896d-5aa2c5366928
📒 Files selected for processing (19)
server/Dockerfileserver/docker-compose.ymlserver/jsconfig.jsonserver/package.jsonserver/src/server.jsserver/src/services/auth/controllers/authController.jsserver/src/services/auth/dependencies/authDependency.jsserver/src/services/auth/repositories/baseRepository.jsserver/src/services/auth/repositories/userRepository.jsserver/src/services/auth/routes/authRoutes.jsserver/src/services/auth/services/authService.jsserver/src/services/auth/validate/authSchema.jsserver/src/shared/constants/roles.jsserver/src/shared/middlewares/authenticate.jsserver/src/shared/middlewares/authrorize.jsserver/src/shared/middlewares/errorHandler.jsserver/src/shared/middlewares/reqLogger.jsserver/src/shared/middlewares/validate.jsserver/src/shared/models/user.js
✅ Files skipped from review due to trivial changes (2)
- server/Dockerfile
- server/src/shared/constants/roles.js
🚧 Files skipped from review as they are similar to previous changes (1)
- server/src/server.js
Comment on lines
+46
to
+52
| const { username, email, password, role } = req.body; | ||
| const userData = { | ||
| username, | ||
| email, | ||
| password, | ||
| role: role || APPLICATION_ROLES.CLIENT_VIEWER, | ||
| }; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify whether AuthService.register enforces/overrides incoming role
fd -i 'authService.js'
echo "=== register implementation(s) ==="
rg -nP --type=js '\bregister\s*\(' -C4
echo "=== auth service file content (inspect role handling) ==="
cat -n server/src/services/auth/services/authService.js | sed -n '1,260p'Repository: NazimRiyadh/Owlpi
Length of output: 6744
🏁 Script executed:
# Check the auth routes to confirm the /register endpoint is public
cat -n server/src/services/auth/routes/authRoutes.js | sed -n '1,40p'Repository: NazimRiyadh/Owlpi
Length of output: 1561
🏁 Script executed:
# Search for APPLICATION_ROLES definition
rg -n 'APPLICATION_ROLES|const.*ROLES' --type=js -A3 | head -50Repository: NazimRiyadh/Owlpi
Length of output: 2335
🏁 Script executed:
# Check the user model/schema for role field constraints
fd -i 'user\.js' -o 'userSchema' -o 'userModel' | head -10Repository: NazimRiyadh/Owlpi
Length of output: 201
🏁 Script executed:
# Search for role validation in middleware or validators
rg -n 'role.*valid|validateRole|roleValidator' --type=jsRepository: NazimRiyadh/Owlpi
Length of output: 43
🏁 Script executed:
# Find and inspect the registrationSchema
fd -i 'authSchema.js' -o 'schema.js' | head -5Repository: NazimRiyadh/Owlpi
Length of output: 199
🏁 Script executed:
# Read the authSchema to see registration validation
rg -n 'registrationSchema' -A20 --type=jsRepository: NazimRiyadh/Owlpi
Length of output: 4421
🏁 Script executed:
# Find user model/schema files
fd -i 'user' --type=f | grep -iE '(model|schema)' | head -10Repository: NazimRiyadh/Owlpi
Length of output: 94
🏁 Script executed:
cat -n server/src/shared/models/user.js | head -100Repository: NazimRiyadh/Owlpi
Length of output: 3423
Privilege escalation via role parameter on public /register endpoint
The registrationSchema validation accepts super_admin role via isValidRole(), which includes all roles. Combined with the public /register endpoint and no role override in AuthService.register(), an attacker can escalate to super_admin during registration.
Fix: Restrict registration to client roles by replacing isValidRole() with isValidClientRole() in the schema (lines 27-33 of server/src/services/auth/validate/authSchema.js), or hardcode role: APPLICATION_ROLES.CLIENT_VIEWER in the controller.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/services/auth/controllers/authController.js` around lines 46 - 52,
The registration handler currently reads role from req.body when constructing
userData (see the destructuring "const { username, email, password, role }" and
the userData object) which allows privilege escalation; to fix, stop honoring
the incoming role by hardcoding role: APPLICATION_ROLES.CLIENT_VIEWER in
userData (remove role from the destructure and do not pass through
req.body.role) before calling AuthService.register, or alternatively enforce the
tighter schema change by replacing isValidRole() with isValidClientRole() in the
validation layer (authSchema) so only client roles are allowed; ensure
AuthService.register receives only the safe client role.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/src/server.js`:
- Around line 55-59: The endpoints map advertises routes that are not mounted
(ingest: "/api/hit" and analytics: "/api/analytics"), causing 404s; either
remove those keys from the endpoints object or actually mount the corresponding
routers (e.g., register ingestRouter and analyticsRouter with
app.use("/api/hit", ingestRouter) and app.use("/api/analytics",
analyticsRouter)); update the endpoints object to match what is mounted
(currently authRouter is mounted via authRouter) so advertised routes and
mounted routers (authRouter, ingestRouter, analyticsRouter) stay in sync.
- Around line 102-123: There is an extraneous closing brace/token after the
gracefulShutdown function that breaks parsing; remove the stray "};" following
the gracefulShutdown declaration so the function ends correctly and the rest of
the file (including startServer and subsequent code) can be parsed;
specifically, locate the gracefulShutdown definition (uses logger.info,
server.close, mongo.disconnect, postgres.close, rabbitmq.close) and delete the
unmatched closing brace so only the intended function termination and any
required single semicolon remain.
- Around line 18-22: The current global CORS setup uses origin: true with
credentials: true which reflects any Origin and bypasses the API-key allowlist;
replace this by implementing per-request origin validation: remove the blanket
app.use(cors(... origin: true ...)) and instead supply a dynamic origin callback
to the cors middleware (or a small middleware executed before CORS) that
retrieves the API key for the request (e.g., the existing API key lookup
function in server/src/shared/models/apikey.js or a helper like
getApiKeyForRequest) and checks the request Origin against that API key's
allowedOrigins array; only return true (or the origin) when the origin matches
the allowedOrigins, otherwise reject by returning false or sending a 403, and
preserve credentials: true only when the origin is allowed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+55
to
+59
| endpoints: { | ||
| health: "/health", | ||
| auth: "/api/auth", | ||
| ingest: "/api/hit", | ||
| analytics: "/api/analytics", |
Contributor
There was a problem hiding this comment.
Don't advertise routes that this server never mounts.
Lines 58-59 publish /api/hit and /api/analytics, but this file only mounts authRouter on Line 67. Those links currently lead clients to 404s.
Suggested fix
endpoints: {
health: "/health",
auth: "/api/auth",
- ingest: "/api/hit",
- analytics: "/api/analytics",
},Also applies to: 67-67
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/server.js` around lines 55 - 59, The endpoints map advertises
routes that are not mounted (ingest: "/api/hit" and analytics:
"/api/analytics"), causing 404s; either remove those keys from the endpoints
object or actually mount the corresponding routers (e.g., register ingestRouter
and analyticsRouter with app.use("/api/hit", ingestRouter) and
app.use("/api/analytics", analyticsRouter)); update the endpoints object to
match what is mounted (currently authRouter is mounted via authRouter) so
advertised routes and mounted routers (authRouter, ingestRouter,
analyticsRouter) stay in sync.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Chores