Conversation
📝 WalkthroughWalkthroughImplements JWT auth and role guards, adds Zod request validation and shared validators, centralizes error handling, generates an OpenAPI spec and mounts Swagger UI at /api/docs, wires validation and JSDoc into auth/admin/user routes, and updates CI/infra/dependencies. ChangesAPI authentication, validation, and documentation
Sequence Diagram(s)sequenceDiagram
participant Client
participant authenticate
participant requireRole
participant RouteHandler
participant errorHandler
Client->>authenticate: HTTP request with Authorization: Bearer <token>
authenticate->>authenticate: extract & verify JWT
alt token valid
authenticate->>RouteHandler: next() with req.user
RouteHandler->>requireRole: route may invoke requireRole
requireRole->>requireRole: check allowed roles
alt role allowed
requireRole->>RouteHandler: next() -> handler executes
else forbidden
requireRole->>Client: 403 FORBIDDEN
end
else token expired
authenticate->>Client: 401 TOKEN_EXPIRED
else invalid token
authenticate->>Client: 401 INVALID_TOKEN
end
Note over errorHandler: Catches thrown errors and maps Zod/JWT/generic errors to structured JSON responses
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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: 6
🧹 Nitpick comments (1)
backend/src/index.ts (1)
26-26: ⚡ Quick winGate Swagger UI exposure in production.
Line 26 mounts
/api/docsfor all environments; consider disabling or protecting it in production to reduce endpoint discovery surface.🔐 Minimal env-based guard
-app.use('/api/docs', swaggerUi.serve, swaggerUi.setup(swaggerSpec)); +if (process.env.NODE_ENV !== 'production') { + app.use('/api/docs', swaggerUi.serve, swaggerUi.setup(swaggerSpec)); +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/index.ts` at line 26, The Swagger UI route is always mounted via app.use('/api/docs', swaggerUi.serve, swaggerUi.setup(swaggerSpec)); — wrap that mount in an environment-based guard (e.g., check process.env.NODE_ENV !== 'production' or require process.env.ENABLE_SWAGGER === 'true') or protect it with an auth middleware so swaggerUi.setup/serve are not exposed in production; update the code around the app.use call to only register swaggerUi.serve and swaggerUi.setup(swaggerSpec) when the guard passes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/pnpm-workspace.yaml`:
- Line 4: Remove the allowlist entry for '`@scarf/scarf`' under allowBuilds (or
set it to false) so install-time lifecycle scripts aren't permitted;
alternatively, if you must keep the entry, ensure Scarf's telemetry is
explicitly disabled during install by setting scarfSettings.enabled=false or
SCARF_ANALYTICS=false when building (update the allowBuilds configuration and
any CI/build env config referencing '`@scarf/scarf`' accordingly).
In `@backend/src/middleware/authenticate.ts`:
- Around line 32-33: The middleware currently treats a missing JWT secret
(process.env.JWT_ACCESS_SECRET) as an INVALID_TOKEN client error when calling
jwt.verify (seen in the jwt.verify(token, process.env.JWT_ACCESS_SECRET!) usage
in authenticate.ts); change this to throw or forward a 500-class server error
when the secret is absent instead of returning 401/INVALID_TOKEN. Do the same
for the other verify usage around the refresh/second-verify block (lines 54-58
equivalent) so both jwt.verify calls check for a missing secret first and
return/forward an internal server error with a clear message before attempting
verification.
- Around line 32-45: The JWT payload returned by jwt.verify is being cast and
assigned directly to req.user without verifying the presence and types of
user_id, role, campus_id, and email; add a runtime check after jwt.verify (in
the authenticate middleware) that confirms payload is an object and that
payload.user_id and payload.campus_id and payload.email are non-empty strings
and payload.role is one of 'student'|'security'|'admin'; if the check fails,
treat it as an invalid token (return/throw the same error flow used for
INVALID_TOKEN) instead of attaching req.user, otherwise assign the validated
values to req.user.
In `@backend/src/routes/admin/users.ts`:
- Around line 73-103: The OpenAPI POST requestBody schema in the admin users
route currently lists a single static required array (email, firstName,
lastName, role, campusId) but the API enforces role-dependent required fields;
update the OpenAPI schema for the POST endpoint (the requestBody in the
admin/users route) to express those conditional requirements—for example replace
the flat schema with a oneOf (or discriminator) composed of role-specific
schemas for role=student (require studentNumber), role=security/administrative
(require employeeId), and role=admin (if different), or otherwise document the
conditional validation clearly in the schema description so generated clients
know which fields are required per role; ensure the role property is the
discriminator or included in each subschema so runtime validation matches the
docs.
In `@backend/src/routes/auth.ts`:
- Around line 7-32: Update the OpenAPI blocks for the auth routes to match
validator constraints and current runtime behavior: change the example email
domain in the /api/auth/login, /api/auth/refresh and /api/auth/logout schemas to
use the `@myseneca.ca` domain (as enforced by backend/src/validators/auth.ts), and
replace or augment the documented success responses with a 501 response note for
these endpoints to reflect the current "not implemented" handlers until real
implementations exist; ensure the route summaries (/api/auth/login,
/api/auth/refresh, /api/auth/logout) and their request/response schemas are
adjusted accordingly so docs and runtime remain consistent.
In `@backend/src/validators/users.ts`:
- Around line 64-67: The current isActive schema uses z.string().transform((v)
=> v === 'true') which coerces any string to false; replace it with an explicit
allowed-values schema such as z.enum(['true','false']).transform(v => v ===
'true').optional() (or z.union([z.literal('true'),
z.literal('false')]).transform(...).optional()) so inputs other than "true" or
"false" fail validation; update the isActive definition to use that enum/union
and keep the .optional() behavior.
---
Nitpick comments:
In `@backend/src/index.ts`:
- Line 26: The Swagger UI route is always mounted via app.use('/api/docs',
swaggerUi.serve, swaggerUi.setup(swaggerSpec)); — wrap that mount in an
environment-based guard (e.g., check process.env.NODE_ENV !== 'production' or
require process.env.ENABLE_SWAGGER === 'true') or protect it with an auth
middleware so swaggerUi.setup/serve are not exposed in production; update the
code around the app.use call to only register swaggerUi.serve and
swaggerUi.setup(swaggerSpec) when the guard passes.
🪄 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: d34b3ece-f6c6-483e-bb62-838632ec7e70
⛔ Files ignored due to path filters (1)
backend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
.github/workflows/ci.yml.gitignorebackend/package.jsonbackend/pnpm-workspace.yamlbackend/src/index.tsbackend/src/middleware/authenticate.tsbackend/src/middleware/errorHandler.tsbackend/src/middleware/requireRole.tsbackend/src/routes/admin/users.tsbackend/src/routes/auth.tsbackend/src/routes/users.tsbackend/src/swagger.tsbackend/src/validators/auth.tsbackend/src/validators/shared.tsbackend/src/validators/users.ts
Comment on lines
+73
to
+103
| * requestBody: | ||
| * required: true | ||
| * content: | ||
| * application/json: | ||
| * schema: | ||
| * type: object | ||
| * required: [email, firstName, lastName, role, campusId] | ||
| * properties: | ||
| * email: | ||
| * type: string | ||
| * example: student@myseneca.ca | ||
| * firstName: | ||
| * type: string | ||
| * lastName: | ||
| * type: string | ||
| * role: | ||
| * type: string | ||
| * enum: [student, security, admin] | ||
| * campusId: | ||
| * type: string | ||
| * format: uuid | ||
| * studentNumber: | ||
| * type: integer | ||
| * example: 123456789 | ||
| * employeeId: | ||
| * type: string | ||
| * example: EMP00000001 | ||
| * phone: | ||
| * type: string | ||
| * example: '4161234567' | ||
| * responses: |
There was a problem hiding this comment.
Document role-dependent required fields in POST schema.
Line 79 defines a static required set, but this API uses role-specific constraints; generated clients may send payloads that pass docs but fail runtime validation.
📘 Suggested OpenAPI adjustment
* application/json:
* schema:
-* type: object
-* required: [email, firstName, lastName, role, campusId]
-* properties:
+* oneOf:
+* - type: object
+* required: [email, firstName, lastName, role, campusId, studentNumber]
+* properties:
+* role:
+* type: string
+* enum: [student]
+* studentNumber:
+* type: integer
+* - type: object
+* required: [email, firstName, lastName, role, campusId, employeeId]
+* properties:
+* role:
+* type: string
+* enum: [security, admin]
+* employeeId:
+* type: string🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/routes/admin/users.ts` around lines 73 - 103, The OpenAPI POST
requestBody schema in the admin users route currently lists a single static
required array (email, firstName, lastName, role, campusId) but the API enforces
role-dependent required fields; update the OpenAPI schema for the POST endpoint
(the requestBody in the admin/users route) to express those conditional
requirements—for example replace the flat schema with a oneOf (or discriminator)
composed of role-specific schemas for role=student (require studentNumber),
role=security/administrative (require employeeId), and role=admin (if
different), or otherwise document the conditional validation clearly in the
schema description so generated clients know which fields are required per role;
ensure the role property is the discriminator or included in each subschema so
runtime validation matches the docs.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/routes/admin/users.ts (1)
24-26:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winOpenAPI
isActivetype does not match validator expectation.The OpenAPI spec declares
isActiveastype: boolean, butlistUsersQuerySchemaexpects string literals"true"or"false". Query parameters arrive as strings in HTTP, and the validator explicitly uses.enum(['true', 'false']). API clients generated from this spec may send boolean values that fail validation.📝 Suggested fix
- in: query name: isActive schema: - type: boolean + type: string + enum: [true, false]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/routes/admin/users.ts` around lines 24 - 26, The OpenAPI spec for the query param `isActive` doesn't match the validator `listUsersQuerySchema`: OpenAPI declares `isActive` as boolean while the validator expects string literals `"true"`/`"false"`. Update the OpenAPI `isActive` declaration to type: string and include enum: ['true','false'] (or otherwise mirror the validator) so the spec matches `listUsersQuerySchema` and generated clients follow the same string enum semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@backend/src/routes/admin/users.ts`:
- Around line 24-26: The OpenAPI spec for the query param `isActive` doesn't
match the validator `listUsersQuerySchema`: OpenAPI declares `isActive` as
boolean while the validator expects string literals `"true"`/`"false"`. Update
the OpenAPI `isActive` declaration to type: string and include enum:
['true','false'] (or otherwise mirror the validator) so the spec matches
`listUsersQuerySchema` and generated clients follow the same string enum
semantics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5543264d-58ed-4c2c-93c4-410c37566fb3
📒 Files selected for processing (5)
backend/pnpm-workspace.yamlbackend/src/middleware/authenticate.tsbackend/src/routes/admin/users.tsbackend/src/routes/auth.tsbackend/src/validators/users.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/routes/admin/users.ts`:
- Around line 29-30: The OpenAPI schema for the isActive parameter currently
uses unquoted booleans (enum: [true, false]) while the validator expects string
literals (z.enum(['true','false'])); update the enum to use quoted strings
(enum: ['true', 'false']) in the route's parameter/schema for isActive so the
OpenAPI type: string matches the runtime validator z.enum(['true','false']) and
the values align.
🪄 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: b8451b3f-551c-4eb5-b305-62f55e4ad409
📒 Files selected for processing (1)
backend/src/routes/admin/users.ts
Comment on lines
+29
to
+30
| * type: string | ||
| * enum: [true, false] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how the current JSDoc YAML block is typed when parsed.
python - <<'PY'
import re
from pathlib import Path
import subprocess, sys
try:
import yaml
except Exception:
subprocess.check_call([sys.executable, "-m", "pip", "install", "-q", "pyyaml"])
import yaml
path = Path("backend/src/routes/admin/users.ts")
lines = path.read_text().splitlines()
start = next(i for i, l in enumerate(lines) if "`@openapi`" in l)
end = next(i for i in range(start + 1, len(lines)) if lines[i].strip() == "*/")
yaml_lines = []
for l in lines[start + 1:end]:
m = re.match(r"\s*\*\s?(.*)$", l)
if m:
yaml_lines.append(m.group(1))
doc = yaml.safe_load("\n".join(yaml_lines))
params = doc["/api/admin/users"]["get"]["parameters"]
is_active = next(p for p in params if p.get("name") == "isActive")
enum_vals = is_active["schema"]["enum"]
print("enum values:", enum_vals)
print("enum value types:", [type(v).__name__ for v in enum_vals])
print("EXPECTED after fix: ['true', 'false'] and ['str', 'str']")
PYRepository: 86unj/Foundit
Length of output: 176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the isActive OpenAPI JSDoc fragment
echo "---- OpenAPI JSDoc (isActive) ----"
rg -n "isActive" backend/src/routes/admin/users.ts
echo
# Print around the enum lines mentioned (best-effort)
nl -ba backend/src/routes/admin/users.ts | sed -n '15,45p'
echo
# Verify runtime validator expectation for isActive
echo "---- Validator for isActive ----"
rg -n "isActive" backend/src/validators/users.ts
echo
nl -ba backend/src/validators/users.ts | sed -n '40,95p'Repository: 86unj/Foundit
Length of output: 183
🏁 Script executed:
#!/bin/bash
# Verify how the current JSDoc YAML block is typed when parsed.
python - <<'PY'
import re
from pathlib import Path
import subprocess, sys
try:
import yaml
except Exception:
subprocess.check_call([sys.executable, "-m", "pip", "install", "-q", "pyyaml"])
import yaml
path = Path("backend/src/routes/admin/users.ts")
lines = path.read_text().splitlines()
start = next(i for i, l in enumerate(lines) if "`@openapi`" in l)
end = next(i for i in range(start + 1, len(lines)) if lines[i].strip() == "*/")
yaml_lines = []
for l in lines[start + 1:end]:
m = re.match(r"\s*\*\s?(.*)$", l)
if m:
yaml_lines.append(m.group(1))
doc = yaml.safe_load("\n".join(yaml_lines))
params = doc["/api/admin/users"]["get"]["parameters"]
is_active = next(p for p in params if p.get("name") == "isActive")
enum_vals = is_active["schema"]["enum"]
print("enum values:", enum_vals)
print("enum value types:", [type(v).__name__ for v in enum_vals])
PYRepository: 86unj/Foundit
Length of output: 119
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- backend/src/routes/admin/users.ts (isActive OpenAPI snippet) ----"
cat -n backend/src/routes/admin/users.ts | sed -n '15,45p'
echo
echo "---- backend/src/validators/users.ts (validator for isActive) ----"
rg -n "isActive" backend/src/validators/users.ts || true
echo
cat -n backend/src/validators/users.ts | sed -n '50,90p'Repository: 86unj/Foundit
Length of output: 2207
Quote isActive enum literals to match the string validator and OpenAPI type
In backend/src/routes/admin/users.ts, enum: [true, false] is parsed as booleans, but the parameter schema declares type: string and the runtime validator expects string literals via z.enum(['true', 'false']). Quote the enum values.
📘 Proposed fix
* schema:
* type: string
-* enum: [true, false]
+* enum: ['true', 'false']📝 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
| * type: string | |
| * enum: [true, false] | |
| * type: string | |
| * enum: ['true', 'false'] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/routes/admin/users.ts` around lines 29 - 30, The OpenAPI schema
for the isActive parameter currently uses unquoted booleans (enum: [true,
false]) while the validator expects string literals (z.enum(['true','false']));
update the enum to use quoted strings (enum: ['true', 'false']) in the route's
parameter/schema for isActive so the OpenAPI type: string matches the runtime
validator z.enum(['true','false']) and the values align.
This was referenced May 25, 2026
This was referenced Jun 11, 2026
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
2 participants
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
authenticate/requireRole/errorHandlerandmiddlewarevalidate()/validateQuery()extracted tovalidators/shared.tsSummary by CodeRabbit
New Features
Bug Fixes / Improvements
Chores