implements OT tracing with Jaeger UI - #184
Open
jacksayshi wants to merge 42 commits into
Open
Conversation
…tures The User model in prisma/schema.prisma gained emailVerified and image fields (BetterAuth), but the two mock User fixtures in auth.service.test.ts were never updated, breaking tsc --noEmit and therefore npm run build. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
migrate:dev pointed at ./src/prisma/schema.prisma and ./src/prisma.config.ts, neither of which exists; the real files are ./prisma/schema.prisma and ./prisma.config.ts. Also add a typecheck script so CI can gate on tsc --noEmit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
npm test (vitest/esbuild) never type-checks, so a broken tsc/build stayed green in CI. Add Typecheck and Build steps between Prisma generation and the test run so CI actually catches type errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Advisor/001 verification baseline
Fails closed at import time if JWT_SECRET is missing or too short, instead of silently falling back to the string 'undefined'.
Replaces String(process.env.JWT_SECRET) + dead undefined-guards in auth.middleware.ts, login.service.ts, and auth.service.ts with the validated JWT_SECRET import. Removes the auth-bypass risk where a missing secret would previously sign/verify against the literal string 'undefined'.
config/env.ts now throws at import time when JWT_SECRET is absent, so every test file that transitively imports it needs the variable seeded before the test module graph loads. Wires vitest setupFiles to a new setup-env.ts (fake, non-production value only), removes the now-dead runtime env mutation from login.service.test.ts's beforeEach, and updates its by-value jwt.sign assertion to match.
Covers present/valid, unset, too-short, and the never-log-the-value property directly against the exported helper.
Advisor/002 harden jwt secret
159 timeline backend
The rbac branch added the Role enum and code reading user.role, but the User model itself never got the column, so the generated Prisma client had no role property. This caused the two CI typecheck errors in authorize.middleware.ts and auth.repository.ts. - Add role Role @default(REPORTER) to model User - Add migration creating the Role enum and user.role column - Add role to the User fixtures in auth.service.test.ts Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
adds rbac middleware
Update project status to reflect new timeline
- auth.repository.ts: findById now uses a Prisma select that omits passwordHash instead of fetching and reattaching it. - /auth/user now requires the existing Bearer-token authMiddleware instead of reading a JWT from the query string; auth.service.ts gains getUserById(userId) (the middleware already verifies the token), replacing the token-parsing getUserByToken. - Mobile AuthContext sends the token via an Authorization header instead of ?token=<jwt> in the URL, and gates the query on authToken being present so it no longer fires with token=null. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Stop /auth/user from leaking password hashes and tokens in URLs
PR #166 added the Role model and put requirePermission("update:issue_status") in front of PATCH /issues/:issueId/status, but POST /issues/:issueId/update also writes the issue status via TimelineController.postUpdate and was left behind authMiddleware alone. Any authenticated REPORTER could move any issue to RESOLVED/CLOSED through it, which defeats the gate on the sibling route. Gate the timeline route with the same permission, and add the tests that should have shipped with the role work: the authorize middleware had none, so neither the 403 path nor the fail-closed behaviour on a repository error was covered. Also drops the stale "add user restrictions here" TODO in IssueService, which is now handled a layer up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fix(backend): close status-change bypass and cover the RBAC gate
The global error handler only recognized AppError instances, so plain
object-literal throws in issue.service.ts (e.g. { status: 404, message:
'Issue not found' }) fell through to a generic 500 "Internal Server
Error", hiding real status codes from clients. Also removes the double
res.status(error.statusCode) + next(error) pattern in login.controller.ts,
which threw when statusCode was undefined.
- error.middleware.ts now also honors a numeric status/statusCode on any
thrown object, defaulting to 500 only for truly unknown errors.
- issue.service.ts's 4 object-literal throws are now AppError instances.
- login.controller.ts's catch block just forwards to next(error).
- error_handling.test.ts gains 4 cases covering AppError, {status}, and
{statusCode} shapes, plus confirms unknown errors still stay opaque.
Make error middleware honor service error status codes
- Replace wildcard CORS origin with an ALLOWED_ORIGINS allowlist (env-configurable, falls back to localhost dev origin, still permits no-Origin requests for the mobile app).
- Add a stricter auth rate limiter and apply it to the BetterAuth mount, which previously ran unthrottled ahead of the general limiter registration.
- Remove dead `app.use('/api/issues/upvote', authMiddleware)` line, registered after the issue routes so it never executed; real upvote routes already carry authMiddleware.
Add a zod-backed validateBody middleware and createIssueSchema, and wire them into POST /api/issues. Strips unknown keys (e.g. attacker-supplied userId) and enforces types/bounds on title, coordinates, images, etc.
…ED_ORIGINS Cover valid body, missing/short title, non-URL images, out-of-range latitude, and unknown-key stripping (mass-assignment protection). Document the new ALLOWED_ORIGINS env var in .env.example.
Fix by clearing `isLoading` immediately when the mount-time token read returns null: with no token there is no user to fetch, and the correct state is "not logged in, not loading." The existing flow for stored tokens is unchanged.
mobile: exit auth loading state when no stored token exists
Advisor/006 api hardening
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
4 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.
feat: Open Telemetry Tracing with Jaeger UIresolves (partly) : #171
Description
Adds OpenTelemetry distributed tracing to the backend so we can see where request time is actually going.
The backend is instrumented with the OpenTelemetry Node SDK plus auto-instrumentations, which traces incoming HTTP requests, outgoing HTTP calls, and Postgres queries without any per-route code changes. Spans are exported over OTLP/HTTP to a Jaeger all-in-one container added to docker-compose.yml, so traces are viewable in the Jaeger UI at http://localhost:16686/.
Tracing is loaded via Node's --import flag in the dev script.
Files Added
Files Modified
To Test:
Expected: a trace for GET /api/issues/nearby with nested child spans for the PostGIS query, showing the duration breakdown per span.