Codebase Concerns

Analysis Date: 2026-05-11

TODO / FIXME / HACK / XXX Comments

A repo-wide grep for TODO, FIXME, HACK, and XXX markers across apps/web/src/** and packages/** returned zero hits in source code. The team appears to track follow-ups in PRs/issues rather than inline. Two referenced issues remain anchored in inline comments:

• packages/pdf/src/hooks/use-register-fonts.ts:22 — references issue #2986 (CJK glyph-level font fallback).
• packages/pdf/src/hooks/use-register-fonts.ts:103 — references issue #2986 again for CJK textkit substitution.

These are stable design references, not unresolved debt — included for traceability only.

The remaining inline-comment "concerns" found during exploration sit in the Anti-debt narrative below, derived from code shape rather than comment markers.

High Severity

Security: SMTP-disabled fallback logs full email bodies (incl. verification/reset links)

When SMTP_HOST / SMTP_USER / SMTP_PASS / SMTP_FROM are not all set, the email transport logs the entire payload — including text and html bodies — to stdout.

• File: packages/email/src/transport.ts:59-66

console.info("SMTP not configured; skipping email send.", {
    to: payload.to,
    subject: payload.subject,
    text: payload.text,
    html: payload.html,
});

Impact:

• Password reset, email verification, and email-change confirmation URLs (which are credential-equivalent bearer tokens) are written to server logs whenever SMTP is not fully configured.
• In any shared-log / log-shipping environment (Docker, Kubernetes, journald, cloud logging) this is a credential leak.
• The README and AGENTS.md:101 describe this as a dev convenience; nothing prevents an operator from running production with partial SMTP config.

Fix approach:

1. Add a server-startup assertion that, if NODE_ENV === "production", isSmtpEnabled() must be true.
2. Redact text / html from the console.info call — log only to and subject.
3. Document the production SMTP requirement in .env.example alongside AUTH_SECRET.

Security: rate limiting is silently disabled in non-production

Both the oRPC rate-limit middleware and Better Auth's rate-limit config gate on process.env.NODE_ENV === "production". Anything that does not set NODE_ENV=production at runtime (default node invocation, custom Docker entrypoints that forget to set it, self-hosters running pnpm start without NODE_ENV) runs with all rate limiting disabled, including:

• /sign-in/email, /sign-up/email, password-reset, OAuth register/authorize/token (packages/auth/src/config.ts:30, :69-78, :249-251, :388-390)
• Resume password verification, AI calls, PDF export, storage uploads/deletes, resume mutations (packages/api/src/middleware/rate-limit/index.ts:5, :75)

Impact: brute-force-friendly. A self-hosted deployment that omits NODE_ENV=production is wide open on auth and resume-password endpoints.

Fix approach:

• Either default isRateLimitEnabled = true and provide a RATE_LIMIT_DISABLED opt-out env var, or assert NODE_ENV === "production" at boot when not running tests.

Security: rate limiter is in-process memory only — broken under horizontal scaling

MemoryRatelimiter is used for every oRPC rate limit (packages/api/src/middleware/rate-limit/index.ts:59-66). Better Auth's rateLimit and apiKey.rateLimit blocks (packages/auth/src/config.ts:248-251, :387-391) similarly do not configure a distributed store.

Impact: Running >1 Node instance behind a load balancer multiplies the effective rate limit by the instance count. Brute-force attacks bypass the limit by retrying until they hit a different replica.

Fix approach: Swap MemoryRatelimiter for a Redis-backed ratelimiter (the package supports it via @orpc/experimental-ratelimit) once a redis dependency is acceptable. Until then, document the single-instance constraint in the deployment docs.

Security: file upload accepts arbitrary MIME with image processing disabled

uploadFile (packages/api/src/routers/storage.ts:42-71) only runs the sharp image-processing pipeline for files where isImageFile(file.type) returns true based on the client-supplied file.type, and even that pipeline is skipped if FLAG_DISABLE_IMAGE_PROCESSING=true (packages/api/src/services/storage.ts:96-101).

Impact:

• A client can claim content-type: text/html (or any non-image MIME) on a 10 MB upload, and the file is stored verbatim into uploads/{userId}/pictures/... with the user-claimed content type.
• Served back from apps/web/src/routes/uploads/$userId.$.tsx. X-Content-Type-Options: nosniff is set (:159) and the storage route forces application/octet-stream only for .pdf (:39, :148-153), so a stored HTML body could be served as HTML if the inferred extension matches. The picture key always ends in .jpeg (storage.ts:60), which mitigates this in the picture path, but there is no MIME allow-list enforced at upload time.
• With FLAG_DISABLE_IMAGE_PROCESSING on, the same applies to genuine images (no resize/strip-metadata path), so EXIF data is preserved unredacted.

Fix approach:

1. Add a strict allow-list at the router boundary (packages/api/src/routers/storage.ts:9) — z.file().mime(["image/png", "image/jpeg", "image/webp", "image/gif"]) plus magic-byte validation.
2. Reject upload if claimed MIME does not match the sharp-detected MIME; do not fall back to client claim.
3. Surface a startup warning when FLAG_DISABLE_IMAGE_PROCESSING=true so it is not enabled in production unknowingly.

Tech Debt: web app has near-zero test coverage

• apps/web/src has 11 test files across ~224 source files (~5%) — find apps/web/src -name "*.test.*".
• apps/web/src/routes has 1 test file across 125 route files (apps/web/src/routes/builder/$resumeId/-components/donation-toast.test.tsx).
• Total repo: ~93 test files / ~385 source files (~24%). Most coverage lives in packages/ui, packages/utils, packages/pdf/src/templates/shared, and packages/api (5 tests).

Impact:

• Refactors to routes, builder shell, sidebar forms, resume preview wiring, MCP tools, and uploads handler land without a regression net.
• Particularly thin: apps/web/src/routes/api/** (rpc, auth, openapi, mcp, uploads) and apps/web/src/routes/builder/$resumeId/**.

Fix approach: Phase-by-phase, add server-handler integration tests for api/health.ts, api/auth.$.ts (registration-validation path), uploads/$userId.$.tsx (path traversal), and at least smoke tests around the builder store in apps/web/src/components/resume/builder-resume-draft.ts.

Medium Severity

Fragile: PDF.js / canvas SSR boundary is enforced by convention only

The SSR/CSR split for the resume preview is hand-maintained:

• apps/web/src/components/resume/preview.tsx:14 returns null until useIsClient() resolves and then lazy-loads the browser bundle.
• apps/web/src/components/resume/preview.browser.tsx:2 imports @react-pdf/renderer (pdf).
• apps/web/src/components/resume/pdf-canvas.tsx:1, :4, :9 imports PDF.js types/runtime and sets a module-level GlobalWorkerOptions.workerSrc.
• apps/web/src/routes/templates/$.tsx:1 imports PDFViewer at the top level but the route component itself bails on !isClient. Top-level import means the bundle reaches the SSR chunk; correctness relies on the import being tree-shaken away when SSR runs.
• apps/web/src/routes/dashboard/resumes/-components/cards/resume-thumbnail.tsx uses dynamic await import("pdfjs-dist") — a different convention from pdf-canvas.tsx's static import.
• SSR mode hints: apps/web/src/routes/builder/$resumeId/index.tsx:4 (ssr: false), apps/web/src/routes/$username/$slug.tsx (ssr: "data-only").

Impact: A future contributor adding a static PDF.js import inside a SSR-rendered route component will break SSR with a window is not defined error at build/run time. There is no lint rule or build-time guard enforcing this.

Fix approach:

1. Document the boundary explicitly at the top of apps/web/src/components/resume/preview.tsx and pdf-canvas.tsx (currently only described in AGENTS.md:35).
2. Consider a Vite SSR-externals config that aborts the build if pdfjs-dist / @react-pdf/renderer is reachable from an SSR-eligible route.
3. Switch apps/web/src/routes/templates/$.tsx:1 to a dynamic import("@react-pdf/renderer") inside the useIsClient() branch for consistency with the rest of the preview pipeline.

Fragile: getStorageService() is captured at module load in router

packages/api/src/routers/storage.ts:7 calls getStorageService() at module top level. Because apps/web/src/routes/api/rpc.$.ts constructs a new RPCHandler on every request, the router's storage reference is fixed for the lifetime of the Node process.

Impact: Switching backends (e.g. flipping S3 vars on at runtime) requires a process restart, which is normally fine — but the singleton is also held inside Nitro's HMR boundary in dev, so config changes during pnpm dev need a full restart (not just a save).

Fix approach: Call getStorageService() inside each handler instead, or invalidate the cached service when env values change.

Fragile: RPCHandler instantiated per-request

apps/web/src/routes/api/rpc.$.ts:8-16 creates a new RPCHandler (with plugins) for every incoming request. Each instance re-walks the router tree and re-constructs plugin pipelines.

Impact: Measurable per-request cost on high-RPS dashboards; not catastrophic but unnecessary.

Fix approach: Move new RPCHandler(...) out of the handler and reuse a module-level instance; only getLocale() should run per-request.

Performance: PDF preview regenerates entire PDF on every change

apps/web/src/components/resume/preview.browser.tsx:103-131 debounces PDF generation by 100ms and calls pdf(resumeDocument).toBlob() on every resume change. For multi-page resumes with images, this re-renders the full document and re-loads it into PDF.js.

Impact: Builder feels sluggish on slower hardware when typing into fields; CPU spikes on each keystroke after debounce.

Mitigation in place: UPDATE_DEBOUNCE_MS = 100, crossfade between staged/active layers (:20-80).

Fix approach: Long-term, switch to incremental rendering or page-level memoization keyed by section hash. Short-term, raise debounce to 200–300 ms while typing.

Performance: font registration cost

packages/pdf/src/hooks/use-register-fonts.ts:18, :86 keeps a module-level registeredFontVariants Set keyed by family:weight:style. Per-resume registration calls Font.register once per (family × weight × italic × CJK-fallback) combination, with web-font fetches resolved through getWebFontSource. This Set never expires — if many resumes with different typography are previewed in one session, the registered-font count grows for the page lifetime.

Impact: Memory grows in the builder for sessions that switch typography frequently. Not a leak in the GC sense, but bounded only by the size of the registered-font universe.

Fix approach: Acceptable for current usage. Document the cap and reconsider if typography switching becomes more common.

Performance: large source files hint at oversized modules

Top offenders by line count (excluding generated/test fixtures):

assistant.tsx (1088 lines) and sections.tsx (900 lines) are particularly likely to accumulate further complexity without splitting. rich-input.tsx (789 lines) is the rich-text editor — likely justifies its size but has zero direct tests.

Fix approach: Split assistant.tsx along tool boundaries; extract per-section renderers from sections.tsx if any individual section grows further.

Security: verifyPassword rate limit keyed by username:slug:ip

packages/api/src/middleware/rate-limit/index.ts:77-83 keys the resume-password limiter on resume-password:{username}:{slug}:{clientKey} where clientKey is the client IP. The window/max is 5 attempts per 10 minutes (packages/utils/src/rate-limit.ts:43). This is reasonable, but the global Better Auth global rule for /two-factor/verify-otp is also 5 per 600s. An attacker on a botnet (different IPs) is not blocked at the resource level — each IP gets its own 5/10min budget against the same resume.

Impact: Limited but present brute-force surface on password-protected public resumes.

Fix approach: Add a per-resume global cap on top of the per-IP limit (e.g. 50/hour per username:slug regardless of IP).

Security: as string cast on password hash

packages/api/src/services/resume.ts:487 casts resume.password to string after a isNotNull(schema.resume.password) WHERE clause. The cast is correct in context, but if a future refactor drops the isNotNull guard the cast silently allows null through bcrypt.compare.

Fix approach: Replace as string with a runtime if (!resume.password) throw new ORPCError(...) check.

Security: trust of TRUSTED_IP_HEADERS is unconditional

packages/utils/src/rate-limit.ts:1-7 defines a list of trusted IP headers (CF-Connecting-IP, True-Client-IP, X-Forwarded-For, etc.) that the rate limiter and Better Auth (packages/auth/src/config.ts:276) honour from any caller.

Impact: If the app is deployed without a proxy that strips client-supplied versions of these headers, any client can spoof their rate-limit identity by setting X-Forwarded-For: 1.2.3.4.

Fix approach: Document that operators must terminate at a trusted proxy (Cloudflare, nginx, Caddy) that strips inbound X-Forwarded-For / X-Real-IP. Optionally, add a TRUST_PROXY env flag and only honour those headers when set.

Fragile: cachedTransport in email/transport.ts ignores env mutation

packages/email/src/transport.ts:21-37 caches the nodemailer transport on first use. If SMTP creds change at runtime (e.g. credential rotation in a deployed instance), the cached transport keeps using the stale credentials until the process restarts.

Fix approach: Detect cred changes and rebuild, or document the restart-on-rotation behaviour.

Storage gotcha: statistics cache is filesystem-bound even with S3 configured

packages/api/src/services/statistics.ts:21-52 caches user/resume/star counts as files in getLocalDataDirectory(env.LOCAL_STORAGE_PATH) regardless of whether S3 is enabled.

Impact: When S3 is configured and LOCAL_STORAGE_PATH is on ephemeral storage (e.g. container scratch), the cache is recreated on every redeploy — meaning a cold start always queries the DB / GitHub API rather than re-using the cache. Also breaks horizontal scaling — each replica has its own cache file.

Fix approach: Cache via the configured storage service (getStorageService) instead of raw fs, or move to an in-memory + TTL cache.

Low Severity

Tech Debt: generated file you must not edit

apps/web/src/routeTree.gen.ts (983 lines, eslint-disable, @ts-nocheck at top) is auto-generated by TanStack Router. It is correctly excluded from Biome (biome.json:14) and noted in AGENTS.md:31.

Action required of contributors: Never hand-edit. Regenerate by running pnpm dev (TanStack tooling watches apps/web/src/routes/**).

Tech Debt: dev workflow — pnpm check is write-capable

package.json:20 defines "check": "biome check --write --unsafe .". Running pnpm check will modify files. The lefthook pre-commit (lefthook.yml:7-9) runs the same command on staged files only, and stage_fixed: true re-stages them.

Impact: Surprise file modifications when a contributor runs pnpm check expecting a non-mutating audit.

Fix approach: Add a parallel pnpm check:ci (no --write) for inspection, and document the distinction (already covered in AGENTS.md:103).

Tech Debt: dev gotcha — drizzle-kit does not auto-load .env

packages/db/drizzle.config.ts:8 reads process.env.DATABASE_URL || "". The @reactive-resume/env package auto-loads .env via dotenv (packages/env/src/server.ts:9-11), but drizzle-kit runs as a separate process that does not import @reactive-resume/env.

Impact: Fresh pnpm db:migrate silently fails with an empty connection string unless DATABASE_URL is exported in the shell. Documented in AGENTS.md:58, :79-80.

Fix approach: Either import the env package from drizzle.config.ts to inherit the .env load, or wrap the script with a one-liner that exports DATABASE_URL.

Tech Debt: Nitro plugin auto-runs migrations on dev/prod boot

apps/web/plugins/1.migrate.ts:23-40 runs migrations on every Nitro startup. This is convenient but couples app-boot health to migration health.

Impact:

• A bad migration takes down the whole web service on boot, not just future migration runs.
• Two app instances starting concurrently both call migrate(); Drizzle's migrations table uses transactions to avoid duplicate apply, but the race adds startup latency.
• For prod self-hosters, there is no "boot-without-migrate" knob.

Fix approach: Gate on an env flag (e.g. RUN_MIGRATIONS_ON_BOOT=true, default true) and document running pnpm db:migrate separately in zero-downtime deploys.

Tech Debt: S3 toggle is all-or-nothing

packages/api/src/services/storage.ts:336 and apps/web/plugins/2.storage.ts:7: storage backend selection requires all three of S3_ACCESS_KEY_ID, S3_SECRET_ACCESS_KEY, S3_BUCKET to be set. Setting two of three silently falls back to local storage.

Impact: Operator misconfigures S3 (e.g. forgets S3_BUCKET), app silently writes to local FS — uploads disappear on next container restart on ephemeral disks.

Fix approach: Treat "any S3 var set" as "S3 intended" — throw on partial config rather than silently downgrading.

Security: no global CSP / X-Frame-Options on HTML responses

A repo-wide grep for Content-Security-Policy, X-Frame-Options, Strict-Transport-Security finds them only in the uploads route (apps/web/src/routes/uploads/$userId.$.tsx:159-165) and the schema JSON route (apps/web/src/routes/schema[.]json.ts). The HTML shell (apps/web/src/routes/__root.tsx) sets no CSP, no HSTS, no Permissions-Policy, no Referrer-Policy.

Impact:

• No clickjacking protection on the resume builder or public resume pages — they can be framed by any origin.
• No CSP means an XSS through rich-text rendering (packages/utils/src/sanitize.*, packages/pdf/src/templates/shared/rich-text-html.ts) has no defence-in-depth.

Fix approach: Add a Nitro response hook that sets Content-Security-Policy, Strict-Transport-Security, Referrer-Policy: strict-origin-when-cross-origin, X-Content-Type-Options: nosniff, and X-Frame-Options: SAMEORIGIN on all HTML responses. Validate that the CSP allows pdf.worker.min.mjs, @react-pdf/renderer font fetches, and the configured S3/SeaweedFS origin.

Security: OAuth dynamic-client registration allows unauthenticated callers

packages/auth/src/config.ts:395-401 enables allowDynamicClientRegistration: true and allowUnauthenticatedClientRegistration: true on the oauthProvider plugin. The comment (:397-399) explicitly states this is required for MCP onboarding (RFC 7591) and that the phishing vector is closed by the redirect-URI allowlist in hooks.before (:253-271) and apps/web/src/routes/api/auth.$.ts:97-111.

Impact: Anyone can register an OAuth client. The protection depends entirely on the redirect-URI allowlist correctness in parseAllowedHostList(env.OAUTH_DYNAMIC_CLIENT_REDIRECT_HOSTS) and isAllowedOAuthRedirectUri.

Fix approach: Already mitigated in code; the residual risk is operator misconfiguration of OAUTH_DYNAMIC_CLIENT_REDIRECT_HOSTS. Add a startup warning when OAUTH_DYNAMIC_CLIENT_REDIRECT_HOSTS is empty (i.e. allowlist is APP_URL-only).

Tech Debt: log noise on auth fallthrough

packages/api/src/context.ts:25, :37, :53 calls console.warn for every failed Bearer / session / API-key validation. In an unauthenticated user flow that hits the same route via Bearer-not-present → session-cookie path, no warning fires, but in an actual token-mismatch path the warning fires on every request.

Impact: Log volume in production when API keys expire or rotate. Logs may leak token shape information indirectly via repeated warnings.

Fix approach: Drop to console.debug (which is no-op in default Node), or rate-limit the warning per token-hash.

Tech Debt: getStorageService() cached service singleton in module-load order

packages/api/src/services/storage.ts:343-350 caches the service at first call. Combined with the packages/api/src/routers/storage.ts:7 top-level call, if storage env vars are not present at module-load time (e.g. .env not loaded yet) the local backend is wired in permanently for the process.

Fix approach: Lazy-validate env on first use, not at module load.

Anti-pattern: empty catch swallows preview generation errors

apps/web/src/components/resume/preview.browser.tsx:120:

} catch {}

A failed PDF generation in the builder preview is silently swallowed. The crossfade machinery keeps showing the previous preview, masking template bugs or runtime errors from contributors during development.

Fix approach: At minimum, console.error the failure (matching the pattern used in pdf-canvas.tsx:68). Better: surface a toast or banner so failed-preview state is visible.

Anti-pattern: console.warn/error lacks structure

Every error log in packages/api/src uses console.warn/console.error with positional args (e.g. services/resume.ts:158, :293, :363, context.ts:25, :37, :53). There is no centralised logger, no structured fields, no request correlation ID.

Fix approach: Introduce a minimal logger (pino-light or a thin wrapper) with level, event, userId, requestId fields. Already partly done in the healthcheck (apps/web/src/routes/api/health.ts:65, "[Healthcheck]").

Fragile: in-process pub/sub for resume update events

packages/api/src/services/resume-events.ts (the subscribe path consumed by packages/api/src/routers/resume.ts:99-103's subscribeResumeUpdates) is in-process. Two web replicas will not see each other's resume update events.

Impact: Multi-tab / multi-device builder sync across replicas does not work behind a load balancer.

Fix approach: Redis pub/sub or Postgres LISTEN/NOTIFY once distributed deployment becomes a target.

Migration & Schema Concerns

Migration count is small but each is unversioned in app

• 13 migration files in migrations/ (migrations/20260114102228_* through migrations/20260507144406_*).
• The Nitro plugin runs every pending migration on every boot (apps/web/plugins/1.migrate.ts:23-40).
• There is no "down" / rollback story. Drizzle-kit generates forward-only migrations.

Action: Standard for drizzle workflows. Document that downgrade requires manual SQL.

Schema notes

• packages/db/src/schema/resume.ts:24 stores password as plaintext column type text, but content is always bcrypt-hashed at write (packages/api/src/services/resume.ts:453). Column name is misleading — should be password_hash. Renaming requires a non-trivial migration.

Dependency Risk

Pre-1.0 / preview dependencies in production critical path

Impact: Library upgrades in this stack are high-risk; the team must follow each upstream's release notes closely. The BETA / DEV versions also affect lockfile churn.

Fix approach:

• Set up Renovate / Dependabot for the beta packages specifically, so security patches are caught.
• Run pnpm knip and pnpm dlx npm-check-updates regularly (both are devDependencies, package.json:34, :40).

Several heavy native dependencies are externalised at build time

apps/web/vite.config.ts:55-57 externals bcrypt, sharp, @aws-sdk/client-s3. packages/runtime-externals is the workspace package that wraps these. Knip is configured to ignore them (knip.json:14).

Risk: Operators must ensure these are installed at runtime (covered in Dockerfile). Self-hosters using a Node base image without build-essentials may hit bcrypt build failures.

Fix approach: Mostly documented; consider switching bcrypt → bcryptjs (pure JS) to remove a build dependency.

Cross-Cutting Hard-to-Find Logic

These are non-obvious surfaces that consumers of this map should know about before changing related code:

1. PDF section filtering — packages/pdf/src/templates/shared/filtering.ts:25-60 decides which resume sections render in PDFs based on hidden flags and required title fields. Per-template visual exceptions live in each template directory; cross-template visual changes go here. Noted in AGENTS.md:44.

2. React-PDF font registration & CJK fallback stack — packages/pdf/src/hooks/use-register-fonts.ts:68-133. Owns standard-PDF-font handling (isStandardPdfFontFamily), CJK glyph-level fallback (#2986), and global hyphenation callback. Module-level registration cache (:18). Noted in AGENTS.md:45.

3. Resume access policy — packages/api/src/helpers/resume-access-policy.ts. Single source of truth for owner-vs-viewer redaction (name and metadata.notes stripped for non-owners), NOT_FOUND-vs-FORBIDDEN choice (not-found used to avoid existence disclosure), and self-view statistics exclusion. Owner-only mutations rely on SQL WHERE userId = clauses, not this policy — drift between SQL guards and policy is silent.

4. Resume password cookie — packages/api/src/helpers/resume-access.ts. Cookie name resume_access_{resumeId}, signed value is sha256(resumeId:passwordHash), TTL 10 minutes, httpOnly, sameSite: lax, secure only when APP_URL starts with https. Forgetting to set APP_URL=https://... in production drops the secure flag.

5. OAuth authorize request sanitization — apps/web/src/routes/api/auth.$.ts:6-51 strips control chars from OAuth parameters and decodes broken-but-decodable redirect URIs before passing to Better Auth. Easy to bypass if a new OAuth flow is added that does not route through this handler.

6. Dynamic client registration coercion to public client — apps/web/src/routes/api/auth.$.ts:53-80 forces token_endpoint_auth_method = "none" for unauthenticated registrations (specifically for Claude.ai's MCP onboarding quirk). This is non-standard behaviour buried in a request preprocessor.

7. Builder draft sync — apps/web/src/components/resume/builder-resume-draft.ts:46-100 manages per-resume zustand stores keyed by resume id, with debounced patch-and-resync. Failure to clean up runtimes Map on resume close = subscription/timer leaks.

Test Coverage Gaps (Priority Map)

Concerns audit: 2026-05-11