chore: align tooling with standards (pnpm 10, ESLint 9 flat, Zod config, validateRequest)#134
Open
JDIZM wants to merge 8 commits into
Open
chore: align tooling with standards (pnpm 10, ESLint 9 flat, Zod config, validateRequest)#134JDIZM wants to merge 8 commits into
JDIZM wants to merge 8 commits into
Conversation
Add MIT LICENSE file and expand .gitignore to cover common local/editor/OS files and Claude Code settings (keeping shared agent/command scaffolding committable per project standards).
- Bump packageManager to pnpm@10.17.1 and Dockerfile PNPM_VERSION - Replace .eslintrc.cjs with eslint.config.js (flat config, typescript-eslint 8) - Swap prettier config for .prettierrc.json aligned with turbo-nuxt-starter (no semi, double quotes, 100-col, trailing comma es5) - Fix package.json name to supabase-express-api, license to MIT, add engines.node >=22, drop redundant npx prefixes on drizzle-kit scripts - Add pnpm supply-chain guardrails: minimumReleaseAge 1440 blocks sub-24h releases, onlyBuiltDependencies allowlists the native packages permitted to run install scripts - Drop .eslintignore (moved into flat config); update .prettierignore
Standalone reformat commit (no logic changes). Applies the new no-semi / double-quote / 100-col / trailing-comma-es5 config across the codebase so future diffs stay readable.
- Replace hand-rolled config checks with Zod schema validation so
startup fails fast with per-field error messages
- Add Pino redaction for password/token/authorization/cookie fields
(including set-cookie response headers) and make log level
configurable via LOG_LEVEL
- Introduce validateRequest middleware factory (body/params/query)
mirroring the turbo-nuxt-starter pattern — stores validated data
on req.validated{Body,Params,Query} and overwrites req.body with
the parsed value so handlers stay slim
- Ignore .env.docker to keep docker-compose credentials out of git
Introduce AGENTS.md as the canonical AI-assistant guide so Claude Code, Cursor, Copilot and friends all read the same file. CLAUDE.md becomes a symlink pointing at it. Content refreshed to match the new patterns: pnpm 10 + Corepack, ESLint 9 flat config, Prettier double-quote/100-col, Zod-validated config, validateRequest middleware, Pino redaction, and pnpm supply-chain allowlist guidance. Also pin Node via .nvmrc for users without Volta.
- validateRequest: restore outer try/catch so throwing Zod transforms
/ unexpected parse failures surface as 500s via errorHandler
instead of unhandled async rejections
- config: defaults no longer mask missing production secrets
* POSTGRES_PASSWORD is required in production, placeholder in dev/test
* SUPABASE_URL rejects the example.supabase.co placeholder in prod
* move the 'running in env' log inside parseConfig so it only fires
after successful validation
- server: drop redundant Number(config.port) — Zod already coerces
- eslint: wire up eslint-config-prettier as the last flat-config entry
so it actually disables the stylistic rules that conflict with Prettier
- validateRequest: collapse the three near-identical body/params/query blocks into a table-driven loop - config: drop the redundant local 'port' binding - lint: 'lint' now runs with --fix (dev), 'lint:check' is the no-fix variant used by CI — matches turbo-nuxt-starter pattern - Update GitHub Actions to call lint:check so CI fails on lint issues instead of silently auto-fixing them
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modernizes the project's infrastructure and coding standards by migrating to ESLint 9 flat configuration, Prettier 3, and pnpm 10 with enhanced supply-chain security. It refactors the configuration system to use Zod for strict environment variable validation and introduces a centralized request validation middleware. The changes also include standardizing API responses, enhancing audit logging, and applying project-wide formatting updates, such as the removal of semicolons. I have no feedback to provide.
- validateRequest: route errors through next()/errorHandler instead of writing response directly — validation failures now flow through Sentry capture and structured 4xx logging like every other error. Also overwrite req.params with parsed data for coerce/transform consistency (Express 4: req.params is mutable). req.query is left alone since Express 5 makes it a getter — consumers read from req.validatedQuery. Drop the unused async keyword. - config: tighten supabaseUrl prod validation beyond the single "example.supabase.co" placeholder — explicit placeholder set plus host-shape check (*.supabase.co/.in). Export parseConfig so tests can exercise bad-config paths without mocking module init. - config: import logger directly instead of via helpers barrel to avoid dragging permissions.ts side-effects into startup. - logger: expand Pino redaction for snake_case OAuth tokens (access_token/refresh_token — what Supabase returns), passwordHash, deeper nesting (*.*.password etc), and explicit req.body.* paths so signup/login bodies don't leak.
There was a problem hiding this comment.
Pull request overview
Aligns the API package with workspace-wide tooling standards (pnpm 10, ESLint 9 flat, Prettier 3) and introduces runtime hardening (Zod-validated config, improved logging redaction, request validation middleware), along with broad formatting normalization.
Changes:
- Tooling upgrades: pnpm 10 pinning + Corepack in Docker, ESLint 9 flat config, Prettier 3 config migration, CI lint split into
lintvslint:check. - Runtime hardening: Zod-based
src/config.ts, expanded Pino redaction, newvalidateRequestmiddleware, plus various handler/middleware adjustments. - Repo hygiene/docs: MIT
LICENSE,.gitignoreexpansion,.nvmrc, andAGENTS.md.
Reviewed changes
Copilot reviewed 63 out of 66 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Formatting-only updates aligned to Prettier rules. |
| src/types/express.d.ts | Formatting-only; maintains Express Request augmentation. |
| src/types/database.ts | Formatting-only; keeps shared DB transaction type. |
| src/services/supabase.ts | Formatting-only; Supabase client setup unchanged. |
| src/services/sentry.ts | Formatting-only; Sentry init unchanged. |
| src/services/db/seeds/accounts.ts | Mostly formatting; seed logic unchanged. |
| src/services/db/seed.ts | Formatting-only; seed script behavior unchanged. |
| src/services/db/migrations.ts | Formatting-only; migration runner behavior unchanged. |
| src/services/db/drizzle.ts | Formatting-only; DB client init unchanged. |
| src/services/auditLog.ts | Formatting-only; audit logging behavior unchanged. |
| src/server.ts | Removes old config validity loop; relies on new config parsing; formatting updates. |
| src/schema.ts | Formatting + UUID schema export; supports broader Zod/OpenAPI usage. |
| src/routes/index.ts | Mostly formatting; some route argument formatting expanded. |
| src/routes/admin.ts | Mostly formatting; some route argument formatting expanded. |
| src/middleware/validateRequest.ts | Adds Zod validation middleware factory (currently not wired into routes). |
| src/middleware/rateLimiter.ts | Formatting-only; rate limiter behavior unchanged. |
| src/middleware/isAuthorized.ts | Formatting + retains verbose request logging (security concern). |
| src/middleware/isAuthenticated.ts | Formatting; still logs auth header/token (security concern). |
| src/middleware/errorHandler.ts | Formatting-only; error handler behavior unchanged. |
| src/middleware/checkAccountStatus.ts | Formatting-only; logic unchanged. |
| src/helpers/strings/strings.ts | Formatting-only. |
| src/helpers/strings/strings.test.ts | Formatting-only. |
| src/helpers/response.ts | Formatting-only; response envelope logic unchanged. |
| src/helpers/request.ts | Formatting-only. |
| src/helpers/permissions.ts | Formatting-only; permissions map unchanged. |
| src/helpers/logger.ts | Adds LOG_LEVEL support + redaction paths (needs expansion for actual logged shapes). |
| src/helpers/index.ts | Formatting-only; export surface unchanged. |
| src/helpers/Http.ts | Formatting-only; HttpErrors helpers unchanged. |
| src/handlers/workspaces/workspaces.methods.ts | Formatting-only; workspace creation method unchanged. |
| src/handlers/workspaces/workspaces.handlers.ts | Formatting + some validation/DB flow refactor in workspace/profile endpoints. |
| src/handlers/profiles/profiles.methods.ts | Formatting-only; profile create/get unchanged. |
| src/handlers/memberships/memberships.methods.ts | Formatting-only; membership helpers unchanged. |
| src/handlers/memberships/memberships.handlers.ts | Formatting + expanded validation/transaction structuring. |
| src/handlers/me/me.handlers.ts | Formatting-only; response structure unchanged. |
| src/handlers/auth/auth.methods.ts | Formatting-only; token verification unchanged. |
| src/handlers/auth/auth.handlers.ts | Formatting + minor structural refactors; preserves auth behavior. |
| src/handlers/admin/auditLogs.handlers.ts | Formatting-only; query validation/filtering unchanged. |
| src/handlers/admin/admin.handlers.ts | Formatting + some structural refactors; preserves admin behavior. |
| src/handlers/accounts/accounts.methods.ts | Formatting-only; account DB methods unchanged. |
| src/handlers/accounts/accounts.handlers.ts | Formatting-only; handler behavior unchanged. |
| src/features.ts | Formatting-only; feature-flag logic unchanged. |
| src/docs/swagger.ts | Formatting-only; Swagger gating unchanged. |
| src/docs/openapi.ts | Formatting-only; OpenAPI registry generation unchanged. |
| src/docs/openapi-schemas.ts | Updates/expands OpenAPI Zod schemas; central schema definitions for docs. |
| src/cors.ts | Formatting-only; CORS logic unchanged. |
| src/config.ts | Major: replaces ad-hoc config with Zod-validated config + prod-safe constraints. |
| scripts/token-test.ts | Formatting-only; utility behavior unchanged. |
| scripts/dev-setup.ts | Formatting-only; dev helper behavior unchanged. |
| package.json | Tooling alignment: pnpm 10 pin, ESLint 9, lint scripts split, pnpm guardrails, MIT license. |
| eslint.config.js | Adds ESLint 9 flat config with typescript-eslint + prettier compatibility. |
| drizzle.config.ts | Formatting; still logs full config object (security concern). |
| LICENSE | Adds MIT license text. |
| Dockerfile | Uses Corepack to install pinned pnpm version. |
| AGENTS.md | Adds AI assistant guidance; documents validateRequest-based request flow. |
| .prettierrc.json | Adds Prettier 3 JSON config. |
| .prettierrc.cjs | Removes legacy Prettier config. |
| .prettierignore | Updates ignores (drizzle/supabase migrations/lockfile, etc.). |
| .nvmrc | Pins Node version for nvm users. |
| .gitignore | Expands ignored env files/local tooling artifacts. |
| .github/workflows/release.yml | Uses lint:check in CI. |
| .github/workflows/pull-request.yml | Uses lint:check in CI. |
| .github/workflows/main.yml | Uses lint:check in CI. |
| .eslintrc.cjs | Removes legacy ESLint config. |
| .eslintignore | Removes legacy ESLint ignore file (replaced by flat-config ignores). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { logger } from "./src/helpers/index.ts" | ||
|
|
||
| logger.info({ msg: "config", config }); | ||
| logger.info({ msg: "config", config }) |
There was a problem hiding this comment.
This logs the entire parsed runtime config object, which includes secrets like db_password and potentially Supabase keys. Even with Pino redaction, these field names aren’t currently redacted, so secrets can be printed during drizzle-kit runs. Remove this log line or log only non-sensitive fields.
Suggested change
| logger.info({ msg: "config", config }) | |
| logger.info({ msg: "Loaded drizzle config" }) |
Comment on lines
+39
to
+45
| export const validateRequest = (schemas: ValidationSchemas) => { | ||
| return (req: Request, _res: Response, next: NextFunction): void => { | ||
| const sources = [ | ||
| { | ||
| key: "body" as const, | ||
| input: () => req.body, | ||
| assign: (data: unknown) => { |
There was a problem hiding this comment.
validateRequest is added but (based on current code) not wired into any routes/handlers, while AGENTS/PR description describe it as part of the standard request flow. Either integrate it into the routing layer (or key handlers) or remove it until it’s actually used to avoid dead code and documentation drift.
Comment on lines
+62
to
64
| logger.debug(`Authorizing for workspace id: ${workspaceId}`) | ||
| logger.debug(req, "isAuthorized: req") | ||
|
|
There was a problem hiding this comment.
Logging the entire Express req object can easily include sensitive headers/cookies and large noisy payloads. Prefer logging a minimal subset (e.g. request id, path, method, accountId/workspaceId) rather than logger.debug(req, ...).
Comment on lines
+15
to
19
| const authHeader = req.headers["authorization"] | ||
| const token = authHeader && authHeader.split(" ")[1] | ||
|
|
||
| logger.debug({ msg: "isAuthenticated middleware called", authHeader, token, route: req.route }); | ||
| logger.debug({ msg: "isAuthenticated middleware called", authHeader, token, route: req.route }) | ||
|
|
There was a problem hiding this comment.
isAuthenticated logs the full authorization header (authHeader), which contains the bearer token and is not currently covered by logger redaction. This can leak credentials into logs. Remove authHeader from the log payload (or explicitly redact/mask it before logging).
Comment on lines
+6
to
+41
| redact: { | ||
| paths: [ | ||
| "password", | ||
| "passwordHash", | ||
| "token", | ||
| "accessToken", | ||
| "refreshToken", | ||
| "access_token", | ||
| "refresh_token", | ||
| "authorization", | ||
| "*.password", | ||
| "*.passwordHash", | ||
| "*.token", | ||
| "*.accessToken", | ||
| "*.refreshToken", | ||
| "*.access_token", | ||
| "*.refresh_token", | ||
| "*.authorization", | ||
| "*.*.password", | ||
| "*.*.passwordHash", | ||
| "*.*.token", | ||
| "*.*.accessToken", | ||
| "*.*.refreshToken", | ||
| "*.*.access_token", | ||
| "*.*.refresh_token", | ||
| "req.body.password", | ||
| "req.body.passwordHash", | ||
| "req.body.token", | ||
| "req.body.accessToken", | ||
| "req.body.refreshToken", | ||
| "req.body.access_token", | ||
| "req.body.refresh_token", | ||
| "req.headers.authorization", | ||
| "req.headers.cookie", | ||
| "res.headers['set-cookie']", | ||
| ], |
There was a problem hiding this comment.
The logger redaction paths don’t cover common sensitive shapes that are actually logged elsewhere (e.g. logger.debug(req, ...) logs headers.cookie, not req.headers.cookie; and authHeader is logged in isAuthenticated). Consider expanding redaction paths to include headers.cookie (and other header variants you log) and any config secret keys you log (e.g. db_password, supabaseSecretKey).
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
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
Aligns this API with the workspace-wide tooling standards and hardens a few runtime concerns along the way. Seven commits, each self-contained so they can be reviewed independently.
Tooling
packageManager; DockerfilePNPM_VERSIONbumped to matcheslint.config.js) with typescript-eslint 8;.eslintrc.cjs+.eslintignoreremoved.prettierrc.json(no-semi, double quotes, 100-col,trailingComma: "es5") +eslint-config-prettierwired in lastminimumReleaseAge: 1440(blocks sub-24h releases) andonlyBuiltDependenciesallowlistlintruns with--fixfor dev; CI calls newlint:checkso it fails instead of silently auto-fixingLICENSEadded;.gitignoreexpanded;.nvmrcadded for non-Volta usersRuntime
src/config.ts) — startup fails fast with per-field messages; prod secrets no longer masked by dev defaults (POSTGRES_PASSWORDrequired in prod,SUPABASE_URLrejects the example placeholder)password,token,accessToken,refreshToken,authorization, cookie/set-cookie headers;LOG_LEVELenv var honouredvalidateRequestmiddleware —body/params/queryZod validation factory (turbo-nuxt-starter pattern); stores parsed data onreq.validated*and overwritesreq.body. Outer try/catch surfaces throwing transforms viaerrorHandlerDocs
AGENTS.mdas the canonical AI-assistant guide;CLAUDE.mdis now a symlink to itFormatting
style:commit reformats the whole codebase with the new Prettier config — reviewing with?w=1or skipping that commit makes the logic changes much easier to readTest plan
pnpm installsucceeds under pnpm 10 + Corepackpnpm tsc:checkpassespnpm lint(with--fix) andpnpm lint:check(no fix) both passpnpm format:checkpassespnpm testpassespnpm devand confirm Zod config errors surface cleanly when required env vars are missingvalidateRequestwith bad input and confirm the 400 response + error shape