security

[AndrewHanasiro]

Proposal

• Update docker
• REmove k6 hardcoded password

Summary by CodeRabbit

• Bug Fixes

  • Ensure JSON content-type headers are set for login, token refresh, and MFA endpoints
  • Return empty 200 responses for password reset flows on success

• Infrastructure

  • Upgraded Node.js runtime; container now copies package file and exposes port 5000

• Tests

  • Load test updated to use a generated secure password constant

• Chores

  • CI workflow and project badges updated (no functional app changes)

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Updated Docker base image to node:24.10.0-slim, added JSON content-type headers to several HTTP routes, changed reset-password endpoints to return empty 200 responses, replaced hardcoded passwords in k6 load tests with a generated hashed password, and adjusted CI workflow and README badges.

Changes

Cohort / File(s)	Summary
Docker infrastructure Dockerfile	Upgraded base image from node:22.12.0-slim to node:24.10.0-slim across all multi-stage phases; final stage now copies package.json and adds EXPOSE 5000.
HTTP route content-type headers src/presentation/http/routes/login.route.ts, src/presentation/http/routes/mfa.route.ts	Added explicit Content-Type: application/json headers via res.writeHead() before sending JSON responses for POST /login, GET /refresh/:token, and POST /code.
Reset password response behavior src/presentation/http/routes/reset_password.route.ts	POST /forget and POST /recover now await core.reset.forget() / core.reset.recover() but return empty 200 responses instead of returning those methods' payloads.
Load testing configuration test/loading/k6.js	Added crypto imports (hash, randomUUID); created a module-scoped password using hash('sha512', randomUUID()); replaced hardcoded per-user and login payload passwords with the generated constant.
Continuous Integration workflow .github/workflows/continuous_integration.yml	Removed commented npm test step; replaced SonarCloud action sonarsource/sonarcloud-github-action@master with SonarSource/sonarqube-scan-action@v6; removed Code Climate scan step.
Documentation / Badges README.md	Removed CodeClimate Test Coverage badge and updated Codacy badge URL/parameters.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Verify callers/clients of reset_password endpoints do not expect a JSON payload on success.
• Check for conflicts or duplicate header setting with any global response middleware after adding Content-Type via res.writeHead().
• Confirm the generated SHA-512 hashed UUID password in test/loading/k6.js meets backend password validation/format expectations.
• Validate Docker final-stage additions (copying package.json, EXPOSE 5000) align with runtime/startup scripts.
• Ensure CI SonarQube action replacement runs with expected inputs/secrets.

Poem

🐰 I hopped through Docker, found Node twenty-four,
I nudged response headers and shut an old door,
I hid plain passwords beneath hashed moonlight,
CI badges polished, tests prance in the night,
A little rabbit cheers — code snug and bright! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 2 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check	❓ Inconclusive	The title 'security' is vague and overly generic, failing to clearly describe the specific changes made across multiple files and systems.	Use a more descriptive title that highlights the main changes, such as 'Update Node.js base image and improve security headers' or 'Upgrade dependencies and remove hardcoded passwords'.
Description check	❓ Inconclusive	The description provides only two brief bullet points that partially address the changes but omit significant alterations like header additions, response body changes, and workflow modifications.	Expand the description to include all substantive changes: Docker upgrade, k6 password removal, JSON content-type headers, empty response bodies, and CI/CD workflow updates.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feature/security

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 77ccd89 and 49a7178.

📒 Files selected for processing (1)

• .github/workflows/continuous_integration.yml (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• .github/workflows/continuous_integration.yml

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Codacy Static Code Analysis

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

test/loading/k6.js (1)

1-55: Fix invalid crypto import and make admin password configurable

Two blocking issues:

1. Crypto import will fail in k6

   • k6 doesn't recognize 'crypto' as a module. k6-specific modules use the 'k6/' prefix (e.g., 'k6/crypto', 'k6/http').
   • Even if the import somehow resolved, k6's k6/crypto module exports createHash, not hash.
   • This will prevent the script from running.

2. Random per-run password won't match backend's seeded admin

   • You generate password = hash('sha512', randomUUID()) at script load time, then use it to log in as admin@authplus.com.
   • Unless the backend is seeded with the exact same random value, the login will fail and the entire load test will not execute.

Recommended fix:

• Replace the invalid crypto import. If you need hashing, use import { hash } from 'k6/crypto' (or use k6/encoding for base64).
• Source the admin password from configuration (e.g., k6 environment variable) instead of generating it randomly. This ensures the backend's seeded password and the script's login credentials are synchronized.

🧹 Nitpick comments (3)

src/presentation/http/routes/mfa.route.ts (1)

94-95: Prefer Express helpers over writeHead for JSON responses

Using res.writeHead(200, { 'Content-Type': 'application/json' }) on an Express Response is non‑idiomatic and redundant, since res.status(200).json(credential) (or just res.json(credential)) will set the correct status and Content-Type for you. It also avoids setting status/headers twice in the same handler.

Consider simplifying to:

-    res.writeHead(200, { 'Content-Type': 'application/json' })
-    res.status(200).send(credential)
+    res.status(200).json(credential)

This will also keep the pattern consistent with other routes.

Dockerfile (1)

1-17: Node image upgrade looks fine; consider a bit more container hardening

The upgrade to node:24.10.0-slim across all stages and copying package.json into the deploy stage make sense and align with port 5000 being used elsewhere.

For additional security/operational hardening (optional here), consider:

• Running the app as a non‑root user in the final stage.
• Installing only production dependencies in the deploy stage (or pruning dev deps before copying).
• Pinning the base image by digest (node:24.10.0-slim@sha256:...) to reduce supply‑chain risk.

Also double‑check that your runtime (e.g., compose, k8s) still starts the container correctly with Node 24.

src/presentation/http/routes/login.route.ts (1)

35-36: Avoid writeHead on Express responses; use res.json/res.send instead

Calling res.writeHead(200, { 'Content-Type': 'application/json' }) and then res.status(200).send(resp) sets status/headers twice and mixes low‑level Node APIs with Express helpers. Express will already set the correct JSON content type when you send an object.

You can simplify to:

-    const resp = await core.login.login(email, password)
-    res.writeHead(200, { 'Content-Type': 'application/json' })
-    res.status(200).send(resp)
+    const resp = await core.login.login(email, password)
+    res.status(200).json(resp)

and similarly in the refresh route:

-    const resp = await core.token.refresh(token)
-    res.writeHead(200, { 'Content-Type': 'application/json' })
-    res.status(200).send(resp)
+    const resp = await core.token.refresh(token)
+    res.status(200).json(resp)

This keeps the handlers clearer and avoids low‑level header manipulation.

Also applies to: 51-52

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cea5cc4 and 75ca88f.

📒 Files selected for processing (5)

• Dockerfile (1 hunks)
• src/presentation/http/routes/login.route.ts (2 hunks)
• src/presentation/http/routes/mfa.route.ts (1 hunks)
• src/presentation/http/routes/reset_password.route.ts (2 hunks)
• test/loading/k6.js (3 hunks)

🧰 Additional context used

🧬 Code graph analysis (4)

src/presentation/http/routes/reset_password.route.ts (5)

src/core/usecases/reset_password.usecase.ts (3)

• recover (39-50)
• forget (29-37)
• ResetPasswordUseCase (18-51)

test/unit/usecases/reset_password.usecase.test.ts (2)

• newPassword (64-101)
• hash (15-102)

test/integration/http/reset_password.route.test.ts (1)

• employeePassword (117-127)

src/core/usecases/driver/recover_password.driver.ts (1)

• RecoverPassword (1-3)

src/core/usecases/driver/forget_password.driver.ts (1)

• ForgetPassword (1-3)

src/presentation/http/routes/login.route.ts (1)

test/integration/http/login.route.test.ts (3)

• responseLogin (181-203)
• response (85-95)
• casual (141-179)

test/loading/k6.js (4)

test/unit/usecases/user.usecase.test.ts (1)

• newName (170-212)

test/unit/services/password.services.test.ts (2)

• name (15-22)
• it (6-30)

src/core/services/password.service.ts (1)

• PasswordService (4-19)

test/fixtures/generators.ts (1)

• passwordGenerator (39-51)

src/presentation/http/routes/mfa.route.ts (2)

test/integration/http/mfa.route.test.ts (1)

• response (81-91)

src/core/usecases/mfa_code.usecase.ts (1)

• MFACode (24-69)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Codacy Static Code Analysis

🔇 Additional comments (1)

src/presentation/http/routes/reset_password.route.ts (1)

31-32: Returning empty 200s for password reset looks good; verify client expectations

Changing both /password/forget and /password/recover to:

await core.reset.forget(email)
res.status(200).send()

await core.reset.recover(password, hash)
res.status(200).send()

aligns with the core contracts (Promise<void>) and avoids leaking any extra information in the response body, which is a nice security/UX improvement.

Just make sure there are no frontend or external consumers relying on a non‑empty response body from these endpoints; if any do, they’ll need to be adjusted to treat “200 with empty body” as success.

Also applies to: 57-58

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud