Security 'upgrade' (DO NOT MERGE - testing CI)

[stasel]

No description provided.

[Copilot]

Pull request overview

This PR changes server-side authentication behavior around JWT setup and route protection.

Changes:

• Removes the minimum-length validation for the JWT signing secret in TokenService.
• Removes authMiddleware from the /api/search router wiring (making the search endpoint unauthenticated at runtime).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
server/src/services/TokenService.ts	Removes JWT secret strength validation during TokenService construction.
server/src/index.ts	Changes SearchRouter instantiation to no longer apply authMiddleware.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Removing the JWT secret strength check means the server can start with an empty/short JWT_SECRET (note index.ts passes process.env.JWT_SECRET ?? ''), which makes HS512 tokens trivially forgeable. Reinstate validation (non-empty + minimum length/entropy) and fail fast at startup (either here in TokenService or via centralized env validation before constructing it).

[Copilot]

/api/search is documented as requiring authentication (security: cookie_token in api.yaml), but this change removes the authMiddleware from SearchRouter, making trainee search publicly accessible. Pass [authMiddleware] again (or apply auth at the route level) to keep runtime behavior consistent with the API contract and prevent unauthenticated data exposure.

Suggested change

	const searchRouter = new SearchRouter(searchController);
	const searchRouter = new SearchRouter(searchController, [authMiddleware]);

[dojo-security-review]

🛑 Security Review — 2 issue(s) found

Two security controls have been removed in this PR: authentication stripped from the search router and JWT secret length validation deleted.

View all findings (2 issues)

File	Line	Severity	OWASP	Issue
server/src/index.ts	161	🟠 High	A01 – Broken Access Control	SearchRouter created without authMiddleware — search endpoints now publicly accessible without login
server/src/services/TokenService.ts	—	🟠 High	A07 – Identification & Authentication Failures	JWT secret minimum-length validation removed — empty or short secrets are now accepted, making tokens trivially forgeable

TokenService.ts: The constructor guard if (!secret || secret.length < 32) throw new Error(...) should be restored. Without it, nothing prevents the service from being instantiated with a 1-character or empty secret, undermining all JWT-based authentication.

Generated by PR Security Review for issue #331 · ● 136.6K · ◷

[dojo-security-review]

🛑 Security Review — 2 issue(s) found

Two security-hardening controls have been removed in this PR: authentication has been stripped from the search router (exposing potentially sensitive search data without login), and the minimum-length validation on the JWT secret has been deleted (permitting weak secrets that are trivially brute-forced).

View all findings (2 issues)

File	Line	Severity	OWASP	Issue
server/src/index.ts	161	🟠 High	A01 – Broken Access Control	SearchRouter created without authMiddleware; search endpoints now publicly accessible
server/src/services/TokenService.ts	—	🟠 High	A07 – Identification & Authentication Failures	JWT secret minimum-length validation removed; empty or short secrets (e.g., "secret") are now accepted, making tokens trivially forgeable

Note on TokenService.ts: The finding is on deleted lines so no inline comment can be anchored. The constructor guard (if (!secret \|\| secret.length < 32) throw ...) should be restored to prevent short/empty JWT secrets.

Generated by PR Security Review for issue #331 · ● 136.6K

[dojo-security-review]

🔴 Security Issue: Unauthenticated Access to Search Endpoint

OWASP: A01 – Broken Access Control
Severity: High

Issue: The authMiddleware has been removed from SearchRouter. Any route registered within SearchRouter is now accessible without authentication. This allows unauthenticated users to query search data, which may include sensitive information such as user profiles, cohort members, or other protected resources.

Recommendation: Restore the auth middleware:

const searchRouter = new SearchRouter(searchController, [authMiddleware]);

If public access to some search routes is intentional, apply authMiddleware selectively at the individual route handler level inside SearchRouter, rather than removing it globally.