Vulnerability fix changes#370
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 54 minutes and 54 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (20)
📝 WalkthroughWalkthroughThe diff updates container images and Dockerfile architectures, adds ESLint config, refactors many API controllers to use lodash safe getters, consolidates Postgres credentials and Sequelize initialization, adds HSTS headers to responses, bumps dependencies and tooling, and introduces Dependabot/workflow and documentation files. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (6)
api-service/postman-collection/Obsrv v2 apis.postman_collection.json (1)
7486-7486: Consider using Postman environment variables for sensitive data.For better security practices, the Postman collection should reference environment variables for sensitive values like webhook URLs rather than including them (even redacted) in the collection file.
Example:
"webhookUrl": "{{SLACK_WEBHOOK_URL}}"This approach:
- Keeps secrets out of version control entirely
- Allows different team members to use their own webhook URLs
- Makes it easier to rotate credentials without modifying the collection
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api-service/postman-collection/Obsrv` v2 apis.postman_collection.json at line 7486, Replace the hardcoded webhook URL value under the JSON object with key "config" -> "webhookUrl" so the Postman request uses an environment variable (e.g., SLACK_WEBHOOK_URL) instead of the literal/redacted URL; locate the request body containing "manager": "grafana", "name": "functional-metrics-slack", "type": "slack" and update the "webhookUrl" field to reference the environment variable placeholder so secrets are not stored in the collection.api-service/src/metrics/prometheus/index.ts (1)
28-28: Use standard header casing for HSTS.The HSTS header addition is a good security improvement. However, the header name should use the standard casing
Strict-Transport-Securityrather than all-uppercaseSTRICT-TRANSPORT-SECURITYfor consistency with HTTP conventions and other response handlers mentioned in the AI summary.♻️ Suggested fix
- res.setHeader('STRICT-TRANSPORT-SECURITY', 'max-age=31536000; includeSubDomains'); + res.setHeader('Strict-Transport-Security', 'max-age=31536000; includeSubDomains');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api-service/src/metrics/prometheus/index.ts` at line 28, The HSTS header is set using all-uppercase name; update the call to res.setHeader in this module (the response header middleware where res.setHeader('STRICT-TRANSPORT-SECURITY', ... ) is used) to use the standard header name 'Strict-Transport-Security' while keeping the value 'max-age=31536000; includeSubDomains' unchanged so casing matches HTTP conventions.Dockerfile (1)
1-10: Consider adding a non-root user for improved container security.The static analysis correctly flags that this container runs as root. While the base image change to ECR is appropriate, consider adding a non-root user for defense-in-depth, especially since this PR focuses on vulnerability fixes.
🔒 Proposed fix to run as non-root user
FROM --platform=linux/amd64 public.ecr.aws/docker/library/node:20-alpine RUN apk add libcrypto3=3.3.0-r2 libssl3=3.3.0-r2 RUN apk upgrade RUN mkdir -p /opt/obsrv-api-service WORKDIR /opt/obsrv-api-service/ COPY package*.json ./ RUN npm install COPY . . +RUN addgroup -g 1001 -S nodejs && adduser -S nodejs -u 1001 +USER nodejs EXPOSE 3000 CMD ["npm", "run", "start"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 1 - 10, Add a non-root user and switch to it in the Dockerfile to avoid running the container as root: create a dedicated user/group (e.g., obsrv), ensure the app directory (/opt/obsrv-api-service) is owned by that user (chown) before switching, install dependencies as root but run the app as the non-root user by adding a USER instruction before CMD; update any RUN/COPY steps that write to the app directory to maintain correct ownership and permissions so npm run start can execute without root privileges.api-service/eslint.config.js (1)
43-43: Ignore patterns may need trailing wildcards.The
dist/andnode_modules/patterns should use**to ensure all nested content is ignored.♻️ Suggested fix
{ - ignores: ['dist/', 'node_modules/', '**/*.d.ts'] + ignores: ['dist/**', 'node_modules/**', '**/*.d.ts'] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api-service/eslint.config.js` at line 43, The ignore patterns in the eslint config's ignores array currently use 'dist/' and 'node_modules/' which may not catch nested files; update the ignores array (the "ignores" entry in eslint.config.js) to use recursive glob patterns such as '**/dist/**' and '**/node_modules/**' (leave '**/*.d.ts' as-is) so all nested content is ignored.api-service/src/helpers/ResponseHandler.ts (1)
15-16: Centralize HSTS header in app-level middleware instead of repeating across response handlers.The header is currently set in five places within
ResponseHandler.ts(lines 15, 38, 51, 64, 70), plus inapi-service/src/metrics/prometheus/index.tsandapi-service/src/controllers/DataMetrics/DataMetricsController.ts. A single middleware inapi-service/src/app.tseliminates duplication and ensures all responses include HSTS without risk of missing future custom paths.♻️ Suggested approach
const app: Application = express(); + +app.use((_req, res, next) => { + res.setHeader("Strict-Transport-Security", "max-age=31536000; includeSubDomains"); + next(); +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api-service/src/helpers/ResponseHandler.ts` around lines 15 - 16, Duplicate HSTS header setting is scattered across ResponseHandler (calls that setHeader in multiple places), prometheus metrics code, and DataMetricsController; remove those per-response res.setHeader('STRICT-TRANSPORT-SECURITY' ...) calls (all occurrences in ResponseHandler methods that call res.setHeader, the metrics module in api-service/src/metrics/prometheus/index.ts, and DataMetricsController) and instead add a single app-level middleware in api-service/src/app.ts that sets the header for every response (e.g., in the Express app use a middleware that calls res.setHeader('Strict-Transport-Security','max-age=31536000; includeSubDomains') before next()). Ensure you update references to ResponseHandler methods (where res.setHeader was removed) so responses still use ResponseHandler.refactorResponse and status handling unchanged.api-service/src/controllers/NotificationChannel/Notification.ts (1)
50-50: Inconsistent request body access pattern.This line accesses
request.bodydirectly within_.get, whereas other handlers in this file use_.get(request, 'body'). For consistency with the refactoring pattern applied elsewhere (e.g., lines 14, 30, 111), consider:Proposed fix for consistency
- const { limit, filters, offset } = _.get(request.body, 'request', {}); + const { limit, filters, offset } = _.get(request, 'body.request', {});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api-service/src/controllers/NotificationChannel/Notification.ts` at line 50, The handler currently destructures with _.get(request.body, 'request', {}) which is inconsistent; change it to read the body via _.get(request, 'body', {}) first (or use _.get(request, 'body.request', {})) and then extract { limit, filters, offset } so it matches the other handlers in Notification (e.g., use the same _.get(request, 'body', ...) pattern); update the line that declares const { limit, filters, offset } accordingly in Notification.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api-service/Dockerfile`:
- Around line 20-25: The Docker build runs npm run build but never copies JSON
schema/config files into dist, so imports in DataMetricsController.ts (and
others) like DataMetricsValidationSchema.json, Constants.json,
SchemaMapping.json will be missing at runtime; update the Dockerfile to copy
source JSON files into the built output (e.g., after RUN npm run build add a
COPY or RUN command to copy src/**/*.json into dist/ or modify the earlier COPY
to include *.json from api-service/src into dist) and ensure the find ...
-delete step only removes .d.ts files and does not remove any JSON assets.
In `@api-service/eslint.config.js`:
- Around line 1-5: The ESLint config is using ESM imports/exports (imports: js,
typescript, typescriptParser; export default) while the repo is CommonJS; fix by
either renaming the file to eslint.config.mjs to force ESM parsing, or convert
the file to CommonJS: replace the import lines for js, typescript,
typescriptParser with const js = require('@eslint/js'), const typescript =
require('@typescript-eslint/eslint-plugin'), const typescriptParser =
require('@typescript-eslint/parser'), and replace export default [...] with
module.exports = [...]; choose one of the two approaches and update the file
accordingly.
In `@api-service/src/connections/databaseConnection.ts`:
- Around line 5-12: The code decodes the second credentials segment as base64
(Buffer.from(credentials[1], 'base64').toString('utf-8')) but
ConnectionsConfig.ts constructs credentials from the plain postgres_password,
causing wrong passwords; fix by stopping the incorrect base64 decode in
databaseConnection.ts and pass the raw password segment into Sequelize (use
credentials[1] instead of decodedCredentials in the new Sequelize call), or
alternatively make ConnectionsConfig.ts encode the password to base64 before
joining (update where credentials is built) so the existing Buffer.from call
becomes valid—choose one approach and update either the decodedCredentials usage
in databaseConnection.ts or the credentials construction in ConnectionsConfig.ts
accordingly.
In `@api-service/src/controllers/Alerts/Metric.ts`:
- Around line 13-14: The create handler currently spreads _.get(req, 'body')
into Metrics.create which allows an undefined body and results in { component:
undefined }; add an explicit guard in the create metric handler to validate the
request body (e.g., use _.get(req, 'body') and _.isEmpty or a null/undefined
check) and return a 400/validation error when the body is missing or empty, or
alternatively default to an empty object before spreading; update the call to
Metrics.create in this handler so it receives a validated payload (and ensure
component is present or validated) similar to the validation used in
updateMetricHandler and reference the Metrics.create invocation to locate the
change.
In `@api-service/src/controllers/Alerts/Silence.ts`:
- Around line 14-15: The handler currently does _.get(request, 'body') then
immediately destructures payload into startDate, endDate, alertId which yields a
TypeError when body is missing; change the code to guard for a missing/empty
payload (e.g. use _.get(request, 'body', {}) or explicitly check if payload is
truthy) and return an HTTP 400 with a clear message when the request body is
absent or required fields are missing; apply the same defensive check/failure
response in updateHandler where the payload is read (referencing payload,
startDate, endDate, alertId and updateHandler).
In `@api-service/src/controllers/NotificationChannel/Notification.ts`:
- Around line 14-18: The code uses _.get(request, "body") and then _.set on body
which can be undefined; ensure body is initialized before mutation to avoid
passing undefined to Notification.create. Replace the current pattern by
defaulting body to an object (e.g., const body = _.get(request, "body") || {})
or guard early, then set created_by/updated_by using the userID and call
Notification.create(body); update references in this function where _.get,
_.set, body, userID and Notification.create are used.
In `@command-service/Dockerfile`:
- Around line 1-12: The Dockerfile's kubectl clone uses a non-existent tag
"v1.35.1" which will make the git clone in the WORKDIR /build/kubernetes step
fail; update the git clone --depth 1 --branch v1.35.1
https://github.com/kubernetes/kubernetes.git . command to reference an existing
tag (e.g., v1.35.0) or replace the branch with the correct tag name you intend
to build, then verify the build target binary name ./cmd/kubectl remains
correct.
In `@command-service/requirements.txt`:
- Around line 1-2: Update the requirements to replace the old uvicorn version
with the latest (set uvicorn==0.42.0 or >=0.34.0) and add a minimum-safe h11 pin
(h11>=0.16.0) to prevent the CVE; ensure this change is applied alongside the
existing fastapi~=0.129.0 and starlette>=0.49.1 entries and optionally pin
urllib3==2.6.3 for consistency.
- Around line 1-2: The requirements file currently pins both fastapi~=0.129.0
and starlette>=0.49.1; remove the explicit "starlette>=0.49.1" entry and rely on
FastAPI's transitive constraint instead (keep "fastapi~=0.129.0"), and verify
there are no direct imports of the Starlette package in the codebase—if any
direct Starlette imports exist, update them to use FastAPI APIs or add Starlette
back only if truly required.
---
Nitpick comments:
In `@api-service/eslint.config.js`:
- Line 43: The ignore patterns in the eslint config's ignores array currently
use 'dist/' and 'node_modules/' which may not catch nested files; update the
ignores array (the "ignores" entry in eslint.config.js) to use recursive glob
patterns such as '**/dist/**' and '**/node_modules/**' (leave '**/*.d.ts' as-is)
so all nested content is ignored.
In `@api-service/postman-collection/Obsrv` v2 apis.postman_collection.json:
- Line 7486: Replace the hardcoded webhook URL value under the JSON object with
key "config" -> "webhookUrl" so the Postman request uses an environment variable
(e.g., SLACK_WEBHOOK_URL) instead of the literal/redacted URL; locate the
request body containing "manager": "grafana", "name":
"functional-metrics-slack", "type": "slack" and update the "webhookUrl" field to
reference the environment variable placeholder so secrets are not stored in the
collection.
In `@api-service/src/controllers/NotificationChannel/Notification.ts`:
- Line 50: The handler currently destructures with _.get(request.body,
'request', {}) which is inconsistent; change it to read the body via
_.get(request, 'body', {}) first (or use _.get(request, 'body.request', {})) and
then extract { limit, filters, offset } so it matches the other handlers in
Notification (e.g., use the same _.get(request, 'body', ...) pattern); update
the line that declares const { limit, filters, offset } accordingly in
Notification.ts.
In `@api-service/src/helpers/ResponseHandler.ts`:
- Around line 15-16: Duplicate HSTS header setting is scattered across
ResponseHandler (calls that setHeader in multiple places), prometheus metrics
code, and DataMetricsController; remove those per-response
res.setHeader('STRICT-TRANSPORT-SECURITY' ...) calls (all occurrences in
ResponseHandler methods that call res.setHeader, the metrics module in
api-service/src/metrics/prometheus/index.ts, and DataMetricsController) and
instead add a single app-level middleware in api-service/src/app.ts that sets
the header for every response (e.g., in the Express app use a middleware that
calls res.setHeader('Strict-Transport-Security','max-age=31536000;
includeSubDomains') before next()). Ensure you update references to
ResponseHandler methods (where res.setHeader was removed) so responses still use
ResponseHandler.refactorResponse and status handling unchanged.
In `@api-service/src/metrics/prometheus/index.ts`:
- Line 28: The HSTS header is set using all-uppercase name; update the call to
res.setHeader in this module (the response header middleware where
res.setHeader('STRICT-TRANSPORT-SECURITY', ... ) is used) to use the standard
header name 'Strict-Transport-Security' while keeping the value
'max-age=31536000; includeSubDomains' unchanged so casing matches HTTP
conventions.
In `@Dockerfile`:
- Around line 1-10: Add a non-root user and switch to it in the Dockerfile to
avoid running the container as root: create a dedicated user/group (e.g.,
obsrv), ensure the app directory (/opt/obsrv-api-service) is owned by that user
(chown) before switching, install dependencies as root but run the app as the
non-root user by adding a USER instruction before CMD; update any RUN/COPY steps
that write to the app directory to maintain correct ownership and permissions so
npm run start can execute without root privileges.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f6300d9f-1b93-40e2-9784-fb6ef927f693
⛔ Files ignored due to path filters (1)
command-service/.DS_Storeis excluded by!**/.DS_Store
📒 Files selected for processing (20)
Dockerfileapi-service/Dockerfileapi-service/eslint.config.jsapi-service/package.jsonapi-service/postman-collection/Obsrv v2 apis.postman_collection.jsonapi-service/src/configs/ConnectionsConfig.tsapi-service/src/connections/databaseConnection.tsapi-service/src/controllers/Alerts/Alerts.tsapi-service/src/controllers/Alerts/Metric.tsapi-service/src/controllers/Alerts/Silence.tsapi-service/src/controllers/DataMetrics/DataMetricsController.tsapi-service/src/controllers/DatasetRead/DatasetRead.tsapi-service/src/controllers/NotificationChannel/Notification.tsapi-service/src/controllers/UpdateQueryTemplate/UpdateTemplateController.tsapi-service/src/helpers/ResponseHandler.tsapi-service/src/metrics/prometheus/index.tsapi-service/tsconfig.jsoncommand-service/Dockerfilecommand-service/requirements.txtsystem-rules-ingestor/Dockerfile
| COPY api-service/tsconfig.json ./ | ||
| COPY api-service/src/ ./src/ | ||
| COPY api-service/@types/ ./@types/ | ||
| RUN npm run build | ||
| # Remove declaration files to avoid module resolution conflicts | ||
| RUN find dist -name "*.d.ts" -delete |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "JSON imports under api-service/src:"
rg -n "\\.json[\"']" api-service/src
echo
echo "api-service/package.json build script:"
python - <<'PY'
import json
with open("api-service/package.json") as f:
print(json.load(f)["scripts"]["build"])
PY
echo
echo "Relevant Dockerfile steps:"
sed -n '17,26p;43,48p' api-service/DockerfileRepository: Sanketika-Obsrv/obsrv-api-service
Length of output: 12569
🏁 Script executed:
cat api-service/tsconfig.jsonRepository: Sanketika-Obsrv/obsrv-api-service
Length of output: 11458
Copy JSON schema files into dist during the build stage.
api-service/src/controllers/DataMetrics/DataMetricsController.ts and 39+ other files import JSON schemas (e.g., DataMetricsValidationSchema.json, Constants.json, SchemaMapping.json). The build script at line 23 runs npm run build which compiles TypeScript with resolveJsonModule: true, but only copies package.json to dist/. The build does not copy schema/config JSON files from src/ into dist/, so those files will be missing at runtime when the compiled application tries to load them.
📦 Suggested fix
COPY api-service/tsconfig.json ./
COPY api-service/src/ ./src/
COPY api-service/@types/ ./@types/
RUN npm run build
+RUN find src -name "*.json" -not -path "*/node_modules/*" -exec sh -c 'for f do \
+ target="dist/${f#src/}"; \
+ mkdir -p "$(dirname "$target")"; \
+ cp "$f" "$target"; \
+done' sh {} +
# Remove declaration files to avoid module resolution conflicts
RUN find dist -name "*.d.ts" -deleteAlso applies to: lines 46-48
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api-service/Dockerfile` around lines 20 - 25, The Docker build runs npm run
build but never copies JSON schema/config files into dist, so imports in
DataMetricsController.ts (and others) like DataMetricsValidationSchema.json,
Constants.json, SchemaMapping.json will be missing at runtime; update the
Dockerfile to copy source JSON files into the built output (e.g., after RUN npm
run build add a COPY or RUN command to copy src/**/*.json into dist/ or modify
the earlier COPY to include *.json from api-service/src into dist) and ensure
the find ... -delete step only removes .d.ts files and does not remove any JSON
assets.
| import js from '@eslint/js'; | ||
| import typescript from '@typescript-eslint/eslint-plugin'; | ||
| import typescriptParser from '@typescript-eslint/parser'; | ||
|
|
||
| export default [ |
There was a problem hiding this comment.
Module type mismatch: ESM syntax with CommonJS package type.
The package.json declares "type": "commonjs", but this config uses ESM syntax (import/export). ESLint will fail to load this configuration.
Either rename the file to eslint.config.mjs to force ESM parsing, or convert to CommonJS syntax.
🔧 Option 1: Rename to `eslint.config.mjs` (preferred for ESLint 9)
Rename the file from eslint.config.js to eslint.config.mjs.
🔧 Option 2: Convert to CommonJS syntax
-import js from '@eslint/js';
-import typescript from '@typescript-eslint/eslint-plugin';
-import typescriptParser from '@typescript-eslint/parser';
+const js = require('@eslint/js');
+const typescript = require('@typescript-eslint/eslint-plugin');
+const typescriptParser = require('@typescript-eslint/parser');
-export default [
+module.exports = [
{
files: ['**/*.ts', '**/*.tsx'],
// ... rest unchanged
}
];📝 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.
| import js from '@eslint/js'; | |
| import typescript from '@typescript-eslint/eslint-plugin'; | |
| import typescriptParser from '@typescript-eslint/parser'; | |
| export default [ | |
| const js = require('@eslint/js'); | |
| const typescript = require('@typescript-eslint/eslint-plugin'); | |
| const typescriptParser = require('@typescript-eslint/parser'); | |
| module.exports = [ |
🧰 Tools
🪛 Biome (2.4.9)
[error] 1-1: Illegal use of an import declaration outside of a module
(parse)
[error] 2-2: Illegal use of an import declaration outside of a module
(parse)
[error] 3-3: Illegal use of an import declaration outside of a module
(parse)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api-service/eslint.config.js` around lines 1 - 5, The ESLint config is using
ESM imports/exports (imports: js, typescript, typescriptParser; export default)
while the repo is CommonJS; fix by either renaming the file to eslint.config.mjs
to force ESM parsing, or convert the file to CommonJS: replace the import lines
for js, typescript, typescriptParser with const js = require('@eslint/js'),
const typescript = require('@typescript-eslint/eslint-plugin'), const
typescriptParser = require('@typescript-eslint/parser'), and replace export
default [...] with module.exports = [...]; choose one of the two approaches and
update the file accordingly.
| const credentials = connectionConfig.postgres.credentials.split("::"); | ||
|
|
||
| export const sequelize = new Sequelize({ | ||
| database, password, username: username, dialect: "postgres", host, port: +port, pool: { | ||
| max: 2, | ||
| min: 1, | ||
| acquire: 30000, | ||
| idle: 10000 | ||
| } | ||
| }) | ||
| const decodedCredentials = Buffer.from(credentials[1], 'base64').toString('utf-8'); | ||
|
|
||
| export const sequelize = new Sequelize( | ||
| database, | ||
| credentials[0], | ||
| decodedCredentials, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Producer:"
sed -n '4,10p' api-service/src/configs/ConnectionsConfig.ts
echo
echo "Consumer:"
sed -n '4,12p' api-service/src/connections/databaseConnection.ts
echo
python - <<'PY'
import base64
raw = "postgres"
print("raw password:", raw)
print("base64-decoded bytes:", base64.b64decode(raw))
PYRepository: Sanketika-Obsrv/obsrv-api-service
Length of output: 813
🏁 Script executed:
cat -n api-service/src/configs/ConnectionsConfig.tsRepository: Sanketika-Obsrv/obsrv-api-service
Length of output: 1332
🏁 Script executed:
cat -n api-service/src/connections/databaseConnection.tsRepository: Sanketika-Obsrv/obsrv-api-service
Length of output: 1172
🏁 Script executed:
rg "credentials" --type ts -B 2 -A 2 -iRepository: Sanketika-Obsrv/obsrv-api-service
Length of output: 4936
Password must be base64-encoded when building credentials, or decoding must be removed from the consumer.
ConnectionsConfig.ts line 9 builds credentials from raw postgres_password env var with no encoding:
credentials: `${env['postgres_username'] || "postgres"}::${env['postgres_password'] || "postgres"}`
But databaseConnection.ts line 7 treats the second segment as base64 and decodes it:
const decodedCredentials = Buffer.from(credentials[1], 'base64').toString('utf-8');
Plain-text or default passwords will fail authentication. For example, "postgres" when decoded as base64 becomes corrupt binary data instead of the original password string.
Either base64-encode the password in ConnectionsConfig.ts, or use credentials[1] directly in databaseConnection.ts without decoding.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api-service/src/connections/databaseConnection.ts` around lines 5 - 12, The
code decodes the second credentials segment as base64
(Buffer.from(credentials[1], 'base64').toString('utf-8')) but
ConnectionsConfig.ts constructs credentials from the plain postgres_password,
causing wrong passwords; fix by stopping the incorrect base64 decode in
databaseConnection.ts and pass the raw password segment into Sequelize (use
credentials[1] instead of decodedCredentials in the new Sequelize call), or
alternatively make ConnectionsConfig.ts encode the password to base64 before
joining (update where credentials is built) so the existing Buffer.from call
becomes valid—choose one approach and update either the decodedCredentials usage
in databaseConnection.ts or the credentials construction in ConnectionsConfig.ts
accordingly.
| const component = _.get(req, 'body.component'); | ||
| const metricsBody = await Metrics.create({ ...(_.get(req, 'body')), component: component }); |
There was a problem hiding this comment.
Missing validation for undefined request body.
If _.get(req, 'body') returns undefined, spreading it in the object literal produces {}, so Metrics.create would receive { component: undefined }. Unlike updateMetricHandler which validates with _.isEmpty (line 42-43), this handler has no guard against a missing body.
Consider adding a default or validation:
Proposed fix
const component = _.get(req, 'body.component');
- const metricsBody = await Metrics.create({ ...(_.get(req, 'body')), component: component });
+ const requestBody = _.get(req, 'body', {});
+ const metricsBody = await Metrics.create({ ...requestBody, component });📝 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 component = _.get(req, 'body.component'); | |
| const metricsBody = await Metrics.create({ ...(_.get(req, 'body')), component: component }); | |
| const component = _.get(req, 'body.component'); | |
| const requestBody = _.get(req, 'body', {}); | |
| const metricsBody = await Metrics.create({ ...requestBody, component }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api-service/src/controllers/Alerts/Metric.ts` around lines 13 - 14, The
create handler currently spreads _.get(req, 'body') into Metrics.create which
allows an undefined body and results in { component: undefined }; add an
explicit guard in the create metric handler to validate the request body (e.g.,
use _.get(req, 'body') and _.isEmpty or a null/undefined check) and return a
400/validation error when the body is missing or empty, or alternatively default
to an empty object before spreading; update the call to Metrics.create in this
handler so it receives a validated payload (and ensure component is present or
validated) similar to the validation used in updateMetricHandler and reference
the Metrics.create invocation to locate the change.
| const payload = _.get(request,'body'); | ||
| const { startDate, endDate, alertId } = payload; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "_\.get\(request,\s*['\"]body['\"]\)" api-service/src/controllers/Alerts/Silence.ts
echo
sed -n '12,18p;73,84p' api-service/src/controllers/Alerts/Silence.tsRepository: Sanketika-Obsrv/obsrv-api-service
Length of output: 1370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get the full createHandler function (lines 12-30 should cover the try-catch)
echo "=== createHandler ==="
sed -n '12,35p' api-service/src/controllers/Alerts/Silence.ts
echo
echo "=== updateHandler ==="
# Get the full updateHandler function
sed -n '73,95p' api-service/src/controllers/Alerts/Silence.tsRepository: Sanketika-Obsrv/obsrv-api-service
Length of output: 2776
🏁 Script executed:
#!/bin/bash
set -euoelpipefail
# Get more lines to see the complete catch blocks
sed -n '12,45p' api-service/src/controllers/Alerts/Silence.tsRepository: Sanketika-Obsrv/obsrv-api-service
Length of output: 751
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get more lines to see the complete catch blocks
sed -n '12,45p' api-service/src/controllers/Alerts/Silence.tsRepository: Sanketika-Obsrv/obsrv-api-service
Length of output: 1718
Return 400 for missing request bodies; _.get without defaults still returns undefined.
_.get(request, "body") returns undefined when the body is absent. Line 15 immediately destructures it, and lines 82–83 dereference it, so a missing client request body throws a TypeError caught by the try-catch block and returns a 500 error instead of the correct 400.
🛠️ Suggested fix
- const payload = _.get(request,'body');
+ const payload = _.get(request, "body", {});
+ if (_.isEmpty(payload)) {
+ return next({ message: httpStatus[httpStatus.BAD_REQUEST], statusCode: httpStatus.BAD_REQUEST });
+ }
const { startDate, endDate, alertId } = payload;
@@
- const payload = _.get(request, 'body');
+ const payload = _.get(request, "body", {});
+ if (_.isEmpty(payload)) {
+ return next({ message: httpStatus[httpStatus.BAD_REQUEST], statusCode: httpStatus.BAD_REQUEST });
+ }Also applies to: line 76 in updateHandler.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api-service/src/controllers/Alerts/Silence.ts` around lines 14 - 15, The
handler currently does _.get(request, 'body') then immediately destructures
payload into startDate, endDate, alertId which yields a TypeError when body is
missing; change the code to guard for a missing/empty payload (e.g. use
_.get(request, 'body', {}) or explicitly check if payload is truthy) and return
an HTTP 400 with a clear message when the request body is absent or required
fields are missing; apply the same defensive check/failure response in
updateHandler where the payload is read (referencing payload, startDate,
endDate, alertId and updateHandler).
| const body = _.get(request, "body"); | ||
| const userID = (request as any)?.userID; | ||
| _.set(payload, "created_by", userID); | ||
| _.set(payload, "updated_by", userID); | ||
| const notificationBody = await Notification.create(payload); | ||
| _.set(body, "created_by", userID); | ||
| _.set(body, "updated_by", userID); | ||
| const notificationBody = await Notification.create(body); |
There was a problem hiding this comment.
Potential silent failure if request.body is undefined.
If _.get(request, "body") returns undefined, the subsequent _.set(body, ...) calls will not mutate anything meaningful—_.set on undefined simply returns undefined. The undefined body would then be passed to Notification.create, potentially causing unexpected database behavior.
Consider adding a guard or defaulting to an empty object:
Proposed fix
- const body = _.get(request, "body");
+ const body = _.get(request, "body", {});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api-service/src/controllers/NotificationChannel/Notification.ts` around lines
14 - 18, The code uses _.get(request, "body") and then _.set on body which can
be undefined; ensure body is initialized before mutation to avoid passing
undefined to Notification.create. Replace the current pattern by defaulting body
to an object (e.g., const body = _.get(request, "body") || {}) or guard early,
then set created_by/updated_by using the userID and call
Notification.create(body); update references in this function where _.get,
_.set, body, userID and Notification.create are used.
| # ---- Stage 1: Rebuild Go binaries with Go 1.25.7 to fix crypto/tls CVE ---- | ||
| FROM --platform=linux/amd64 public.ecr.aws/docker/library/golang:1.25.7-bookworm AS go-builder | ||
|
|
||
| # Rebuild kubectl v1.35.1 from source with patched Go toolchain | ||
| WORKDIR /build/kubernetes | ||
| RUN git clone --depth 1 --branch v1.35.1 https://github.com/kubernetes/kubernetes.git . && \ | ||
| CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -trimpath -ldflags="-s -w" -o /out/kubectl ./cmd/kubectl | ||
|
|
||
| # Rebuild helm v3.20.0 from source with patched Go toolchain | ||
| WORKDIR /build/helm | ||
| RUN git clone --depth 1 --branch v3.20.0 https://github.com/helm/helm.git . && \ | ||
| CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -trimpath -ldflags="-s -w" -o /out/helm ./cmd/helm |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that the specified versions exist
echo "=== Checking Go version availability ==="
curl -s "https://go.dev/dl/?mode=json&include=all" | jq -r '.[].version' | grep -E "^go1\.25" | head -5
echo "=== Checking kubectl/kubernetes tags ==="
git ls-remote --tags https://github.com/kubernetes/kubernetes.git | grep -E "v1\.35" | head -5
echo "=== Checking Helm tags ==="
git ls-remote --tags https://github.com/helm/helm.git | grep -E "v3\.20" | head -5Repository: Sanketika-Obsrv/obsrv-api-service
Length of output: 866
kubectl v1.35.1 does not exist—use v1.35.0 or verify the intended version.
Go 1.25.7 and Helm v3.20.0 are confirmed to exist. However, kubectl v1.35.1 was not found in the kubernetes repository tags; only v1.35.0 is available. The build will fail at the git clone step for kubectl. Either pin to v1.35.0 or verify the correct version if a patch release exists under a different tag name.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@command-service/Dockerfile` around lines 1 - 12, The Dockerfile's kubectl
clone uses a non-existent tag "v1.35.1" which will make the git clone in the
WORKDIR /build/kubernetes step fail; update the git clone --depth 1 --branch
v1.35.1 https://github.com/kubernetes/kubernetes.git . command to reference an
existing tag (e.g., v1.35.0) or replace the branch with the correct tag name you
intend to build, then verify the build target binary name ./cmd/kubectl remains
correct.
| fastapi~=0.129.0 | ||
| starlette>=0.49.1 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
`Confirm the latest published versions and compatibility constraints for:
- fastapi 0.129.0
- starlette 0.49.1
- urllib3 2.6.3
- uvicorn 0.20.0 compatibility with fastapi 0.129.x / starlette 0.49.x
Also check if uvicorn 0.20.0 pulls a vulnerable h11 version and what patched uvicorn version is recommended.`
💡 Result:
Latest published versions (as of 2026-04-02): 1. fastapi: 0.135.3 (2026-04-01), so 0.129.0 is outdated. 2. starlette: 1.0.0 (2026-03-22), so 0.49.1 is outdated. 3. urllib3: 2.6.3 (2026-01-07), this is the latest. 4. uvicorn: 0.42.0 (2026-03-16), so 0.20.0 is outdated. Compatibility: uvicorn 0.20.0 (from 2022) is compatible with fastapi 0.129.x / starlette 0.49.x as these versions align with the ASGI ecosystem at that time (starlette ~0.40-0.51 supported by fastapi then). Modern fastapi 0.135+ supports starlette >=0.46.0 (up to <1.0.0 earlier, now 1.0+), and uvicorn latest 0.42.0 works with them. h11 vulnerability: uvicorn requires h11>=0.8 (loose upper bound). Older installs could pull vulnerable h11<0.16.0 (CVE GHSA-vqfr-h8mv-ghfj, fixed in 0.16.0). uvicorn 0.20.0 itself does not pull a specific vulnerable version deterministically, but pre-2025 versions risk it due to pinning issues in ecosystem (e.g., httpcore). Recommended: upgrade to uvicorn >=0.34.0+ (post-2025 updates) or latest 0.42.0, and pin h11>=0.16.0 if needed. Recent PRs updated constraints.
Citations:
- 1: https://releasealert.dev/github/fastapi/fastapi
- 2: https://github.com/fastapi/fastapi/releases
- 3: https://fastapi.tiangolo.com/release-notes/
- 4: https://github.com/fastapi/fastapi/releases/tag/0.135.2
- 5: https://pypi.org/project/starlette/
- 6: https://starlette.dev/release-notes/
- 7: https://pypi.org/project/urllib3/
- 8: https://urllib3.readthedocs.io/en/stable/changelog.html
- 9: https://uvicorn.dev/release-notes/
- 10: https://pypi.python.org/pypi/uvicorn
- 11: https://pypi.org/project/uvicorn/0.40.0/
- 12: h11 dependency pinning causes weird cycle encode/httpcore#1007
- 13: https://raw.githubusercontent.com/Kludex/starlette/HEAD/pyproject.toml
- 14: https://github.com/fastapi/fastapi/blob/master/pyproject.toml
- 15: https://github.com/encode/uvicorn/blob/master/pyproject.toml
🏁 Script executed:
cat command-service/requirements.txtRepository: Sanketika-Obsrv/obsrv-api-service
Length of output: 434
Upgrade uvicorn from 0.20.0 to latest (0.42.0) to address outdated dependencies and potential h11 vulnerabilities.
fastapi~=0.129.0 and starlette>=0.49.1 are compatible with each other, but uvicorn==0.20.0 is significantly outdated (from 2022). This version may pull vulnerable h11<0.16.0 (CVE GHSA-vqfr-h8mv-ghfj). Upgrade uvicorn to >=0.34.0+ or latest 0.42.0, and consider pinning h11>=0.16.0. Note: urllib3==2.6.3 is current and compatible.
🧰 Tools
🪛 OSV Scanner (2.3.5)
[HIGH] 1-1: requests 2.9.2: undefined
(PYSEC-2018-28)
[HIGH] 1-1: requests 2.9.2: undefined
(PYSEC-2023-74)
[HIGH] 1-1: requests 2.9.2: Requests vulnerable to .netrc credentials leak via malicious URLs
[HIGH] 1-1: requests 2.9.2: Requests Session object does not verify requests after making first request with verify=False
[HIGH] 1-1: requests 2.9.2: Requests has Insecure Temp File Reuse in its extract_zipped_paths() utility function
[HIGH] 1-1: requests 2.9.2: Unintended leak of Proxy-Authorization header in requests
[HIGH] 1-1: requests 2.9.2: Insufficiently Protected Credentials in Requests
[CRITICAL] 1-1: h11 0.9.0: h11 accepts some malformed Chunked-Encoding bodies
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@command-service/requirements.txt` around lines 1 - 2, Update the requirements
to replace the old uvicorn version with the latest (set uvicorn==0.42.0 or
>=0.34.0) and add a minimum-safe h11 pin (h11>=0.16.0) to prevent the CVE;
ensure this change is applied alongside the existing fastapi~=0.129.0 and
starlette>=0.49.1 entries and optionally pin urllib3==2.6.3 for consistency.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether Starlette is imported directly anywhere (if not, remove explicit pin safely)
rg -n --type=py -C2 '\bfrom\s+starlette\b|\bimport\s+starlette\b'Repository: Sanketika-Obsrv/obsrv-api-service
Length of output: 59
🌐 Web query:
FastAPI 0.129.0 Starlette dependency requirements
💡 Result:
FastAPI 0.129.0 requires Starlette >=0.40.0,<1.0.0.
Citations:
- 1: https://fastapi.tiangolo.com/release-notes/
- 2: https://github.com/fastapi/fastapi/blob/master/pyproject.toml
- 3: ⬆ Bump fastapi from 0.128.1 to 0.128.3 fastapi/sqlmodel#1746
🏁 Script executed:
#!/bin/bash
# Check for any other references to Starlette in config/manifest files
rg -i starlette --type-list | head -20
rg -i starletteRepository: Sanketika-Obsrv/obsrv-api-service
Length of output: 534
🏁 Script executed:
#!/bin/bash
# Verify no other dependency manifests exist that might reference Starlette
fd -e toml -e txt -e lock | xargs grep -l -i "starlette\|pyproject\|setup.py" 2>/dev/null | head -10Repository: Sanketika-Obsrv/obsrv-api-service
Length of output: 109
Remove explicit Starlette pin—it's a FastAPI transitive dependency not directly used in your code.
Starlette is only consumed through FastAPI, which already constrains it to >=0.40.0,<1.0.0. The explicit starlette>=0.49.1 pin is redundant and creates resolver drift risk over time. Since there are no direct imports of Starlette in the codebase, rely on FastAPI's dependency management instead.
Suggested change
fastapi~=0.129.0
-starlette>=0.49.1📝 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.
| fastapi~=0.129.0 | |
| starlette>=0.49.1 | |
| fastapi~=0.129.0 |
🧰 Tools
🪛 OSV Scanner (2.3.5)
[HIGH] 1-1: requests 2.9.2: undefined
(PYSEC-2018-28)
[HIGH] 1-1: requests 2.9.2: undefined
(PYSEC-2023-74)
[HIGH] 1-1: requests 2.9.2: Requests vulnerable to .netrc credentials leak via malicious URLs
[HIGH] 1-1: requests 2.9.2: Requests Session object does not verify requests after making first request with verify=False
[HIGH] 1-1: requests 2.9.2: Requests has Insecure Temp File Reuse in its extract_zipped_paths() utility function
[HIGH] 1-1: requests 2.9.2: Unintended leak of Proxy-Authorization header in requests
[HIGH] 1-1: requests 2.9.2: Insufficiently Protected Credentials in Requests
[CRITICAL] 1-1: h11 0.9.0: h11 accepts some malformed Chunked-Encoding bodies
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@command-service/requirements.txt` around lines 1 - 2, The requirements file
currently pins both fastapi~=0.129.0 and starlette>=0.49.1; remove the explicit
"starlette>=0.49.1" entry and rely on FastAPI's transitive constraint instead
(keep "fastapi~=0.129.0"), and verify there are no direct imports of the
Starlette package in the codebase—if any direct Starlette imports exist, update
them to use FastAPI APIs or add Starlette back only if truly required.
vinodbhorge
force-pushed
the
Vulnerability-fixes
branch
from
1b9e951 to
ae6e831
Compare
1 participant
Summary by CodeRabbit
Chores
Refactor
Style
Documentation
Other