Baseline scaffold: NestJS API + Postgres + strict CORS + Docker#4
Conversation
Reviewer's GuideIntroduce a minimal NestJS API with strict CORS, containerized Postgres, and runtime-configured API base URL for the SPA, plus docs and tooling to run the stack locally or via Docker. Sequence diagram for SPA calling NestJS API with strict CORSsequenceDiagram
actor User
participant Browser_SPA
participant NestJS_API
participant strictCors
participant AppController
User->>Browser_SPA: Open http://localhost:3000
Browser_SPA->>Browser_SPA: Load config_js and index_html
Browser_SPA->>Browser_SPA: Set window_API_BASE_URL from __APP_CONFIG__ or fallback
User->>Browser_SPA: Trigger API call
Browser_SPA->>NestJS_API: OPTIONS /api/hello with Origin http://localhost:3000
NestJS_API->>strictCors: Invoke strictCors middleware
strictCors-->>NestJS_API: Validate origin and method
NestJS_API-->>Browser_SPA: 204 No Content with Access_Control_Allow_Origin
Browser_SPA->>NestJS_API: GET /api/hello with Origin http://localhost:3000
NestJS_API->>strictCors: Invoke strictCors middleware
strictCors-->>NestJS_API: Set CORS headers
NestJS_API->>AppController: Route to hello handler
AppController-->>NestJS_API: { message: Hello from NestJS (dev starter). }
NestJS_API-->>Browser_SPA: 200 OK JSON response
Class diagram for NestJS API modules and CORS middlewareclassDiagram
class AppModule {
}
class AppController {
+hello() any
+health() any
}
class StrictCorsMiddleware {
+strictCors(req Request, res Response, next NextFunction) void
}
class MainBootstrap {
+bootstrap() Promise~void~
}
AppModule --> AppController : registers
MainBootstrap --> AppModule : bootstraps
MainBootstrap --> StrictCorsMiddleware : uses
class Request
class Response
class NextFunction
StrictCorsMiddleware ..> Request : depends_on
StrictCorsMiddleware ..> Response : depends_on
StrictCorsMiddleware ..> NextFunction : depends_on
Flow diagram for strict CORS middleware decision logicflowchart LR
A["Incoming_request"] --> B{Method_is_OPTIONS?}
B -->|Yes| C{Origin_present_and_allowlisted?}
B -->|No| D{Origin_present_and_allowlisted?}
C -->|Yes| E["Set CORS headers"] --> F["Respond 204 No Content"]
C -->|No| G["Do not set CORS headers"] --> H["Call next()"]
D -->|Yes| E
D -->|No| G
F --> I["End_request"]
H --> I
Flow diagram for SPA runtime API base URL resolutionflowchart LR
A["Load app/index.html"] --> B["Load app/config.js"]
B --> C["Initialize window.__APP_CONFIG__"]
C --> D{NEXT_PUBLIC_API_BASE_URL set?}
D -->|Yes| E["Use NEXT_PUBLIC_API_BASE_URL"]
D -->|No| F{APP_API_BASE_URL set?}
F -->|Yes| G["Use APP_API_BASE_URL"]
F -->|No| H["Use fallback http://localhost:7072"]
E --> I["Normalize trailing slash and set window.API_BASE_URL"]
G --> I
H --> I
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
strictCors,getAllowedOrigins()is recomputed on every request; consider resolving and storing the allowed-origins set once at startup (or memoizing it) to avoid unnecessary per-request parsing of the environment variable. - The CORS middleware only sets
Vary: Originwhen the origin is allowed; to avoid incorrect caching of responses across different origins, it’s safer to setVary: Originwhenever anOriginheader is present, even if the origin is not allowlisted.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `strictCors`, `getAllowedOrigins()` is recomputed on every request; consider resolving and storing the allowed-origins set once at startup (or memoizing it) to avoid unnecessary per-request parsing of the environment variable.
- The CORS middleware only sets `Vary: Origin` when the origin is allowed; to avoid incorrect caching of responses across different origins, it’s safer to set `Vary: Origin` whenever an `Origin` header is present, even if the origin is not allowlisted.
## Individual Comments
### Comment 1
<location path="apps/api/src/cors.middleware.ts" line_range="3-13" />
<code_context>
+const CORS_ALLOWED_HEADERS =
+ 'Origin,X-Requested-With,Content-Type,Accept,Authorization';
+
+const getAllowedOrigins = (): Set<string> =>
+ new Set(
+ (process.env.CORS_ALLOWED_ORIGINS ?? '')
+ .split(',')
</code_context>
<issue_to_address>
**suggestion (performance):** Avoid recomputing allowed origins on every request by caching the parsed environment variable.
`getAllowedOrigins()` currently splits and trims the env string and creates a new `Set` on every call, which adds avoidable overhead on high-traffic paths. Consider computing this set once at module load (or lazily on first call) and reusing it, unless you specifically need to support dynamic changes to `CORS_ALLOWED_ORIGINS`.
```suggestion
const CORS_ALLOWED_METHODS = 'GET,POST,PUT,PATCH,DELETE,OPTIONS';
const CORS_ALLOWED_HEADERS =
'Origin,X-Requested-With,Content-Type,Accept,Authorization';
const ALLOWED_ORIGINS: Set<string> = new Set(
(process.env.CORS_ALLOWED_ORIGINS ?? '')
.split(',')
.map((origin) => origin.trim())
.filter(Boolean),
);
const getAllowedOrigins = (): Set<string> => ALLOWED_ORIGINS;
```
</issue_to_address>
### Comment 2
<location path="apps/api/src/cors.middleware.ts" line_range="28-29" />
<code_context>
+ res.setHeader('Access-Control-Allow-Headers', CORS_ALLOWED_HEADERS);
+ }
+
+ if (req.method === 'OPTIONS' && isAllowed) {
+ res.status(204).send();
+ return;
+ }
</code_context>
<issue_to_address>
**issue:** Clarify behavior for disallowed preflight (OPTIONS) requests to avoid ambiguous responses.
Right now, preflight requests from disallowed origins just fall through to `next()`, likely ending as a 404 or other generic response that the browser interprets as a CORS failure without a clear API signal. Consider explicitly handling `OPTIONS` when `!isAllowed` (e.g., 403 or 204 without CORS headers) so behavior is consistent and independent of downstream middleware or routes.
</issue_to_address>
### Comment 3
<location path="apps/api/Dockerfile" line_range="5-14" />
<code_context>
+WORKDIR /usr/src/app
+
+COPY package*.json ./
+RUN npm install
+
+COPY tsconfig*.json ./
+COPY src ./src
+RUN npm run build
+
+FROM node:20-alpine AS runner
+WORKDIR /usr/src/app
+ENV NODE_ENV=production
+
+COPY package*.json ./
+RUN npm install --omit=dev
+COPY --from=builder /usr/src/app/dist ./dist
+
</code_context>
<issue_to_address>
**suggestion (performance):** Consider using a lockfile-aware install (`npm ci`) and sharing the dependency layer between build and runtime images.
Dependencies are installed twice with `npm install` (builder and runner), which slows builds and ignores any lockfile. If you commit/copy a lockfile (e.g. `package-lock.json`), you can switch to `npm ci` in both stages and either copy `node_modules` from the builder to the runner or share the install layer, improving build speed and reproducibility.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const CORS_ALLOWED_METHODS = 'GET,POST,PUT,PATCH,DELETE,OPTIONS'; | ||
| const CORS_ALLOWED_HEADERS = | ||
| 'Origin,X-Requested-With,Content-Type,Accept,Authorization'; | ||
|
|
||
| const getAllowedOrigins = (): Set<string> => | ||
| new Set( | ||
| (process.env.CORS_ALLOWED_ORIGINS ?? '') | ||
| .split(',') | ||
| .map((origin) => origin.trim()) | ||
| .filter(Boolean), | ||
| ); |
There was a problem hiding this comment.
suggestion (performance): Avoid recomputing allowed origins on every request by caching the parsed environment variable.
getAllowedOrigins() currently splits and trims the env string and creates a new Set on every call, which adds avoidable overhead on high-traffic paths. Consider computing this set once at module load (or lazily on first call) and reusing it, unless you specifically need to support dynamic changes to CORS_ALLOWED_ORIGINS.
| const CORS_ALLOWED_METHODS = 'GET,POST,PUT,PATCH,DELETE,OPTIONS'; | |
| const CORS_ALLOWED_HEADERS = | |
| 'Origin,X-Requested-With,Content-Type,Accept,Authorization'; | |
| const getAllowedOrigins = (): Set<string> => | |
| new Set( | |
| (process.env.CORS_ALLOWED_ORIGINS ?? '') | |
| .split(',') | |
| .map((origin) => origin.trim()) | |
| .filter(Boolean), | |
| ); | |
| const CORS_ALLOWED_METHODS = 'GET,POST,PUT,PATCH,DELETE,OPTIONS'; | |
| const CORS_ALLOWED_HEADERS = | |
| 'Origin,X-Requested-With,Content-Type,Accept,Authorization'; | |
| const ALLOWED_ORIGINS: Set<string> = new Set( | |
| (process.env.CORS_ALLOWED_ORIGINS ?? '') | |
| .split(',') | |
| .map((origin) => origin.trim()) | |
| .filter(Boolean), | |
| ); | |
| const getAllowedOrigins = (): Set<string> => ALLOWED_ORIGINS; |
| if (req.method === 'OPTIONS' && isAllowed) { | ||
| res.status(204).send(); |
There was a problem hiding this comment.
issue: Clarify behavior for disallowed preflight (OPTIONS) requests to avoid ambiguous responses.
Right now, preflight requests from disallowed origins just fall through to next(), likely ending as a 404 or other generic response that the browser interprets as a CORS failure without a clear API signal. Consider explicitly handling OPTIONS when !isAllowed (e.g., 403 or 204 without CORS headers) so behavior is consistent and independent of downstream middleware or routes.
| RUN npm install | ||
|
|
||
| COPY tsconfig*.json ./ | ||
| COPY src ./src | ||
| RUN npm run build | ||
|
|
||
| FROM node:20-alpine AS runner | ||
| WORKDIR /usr/src/app | ||
| ENV NODE_ENV=production | ||
|
|
There was a problem hiding this comment.
suggestion (performance): Consider using a lockfile-aware install (npm ci) and sharing the dependency layer between build and runtime images.
Dependencies are installed twice with npm install (builder and runner), which slows builds and ignores any lockfile. If you commit/copy a lockfile (e.g. package-lock.json), you can switch to npm ci in both stages and either copy node_modules from the builder to the runner or share the install layer, improving build speed and reproducibility.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| depends_on: | ||
| - postgres | ||
| ports: | ||
| - "7072:7072" |
There was a problem hiding this comment.
🔴 Docker port mapping hardcoded while container PORT is configurable, causing unreachable API
The docker-compose.yml hardcodes the port mapping as "7072:7072" (host:container), but the container's internal PORT environment variable is configurable via ${PORT:-7072}. If a user sets PORT=8080 in their .env file, the NestJS app inside the container will listen on port 8080, but the port mapping still forwards host port 7072 to container port 7072 — where nothing is listening.
Root Cause and Impact
The mismatch is between line 18 (PORT: ${PORT:-7072}) and line 24 ("7072:7072"). The main.ts at apps/api/src/main.ts:11 uses Number(process.env.PORT ?? 7072) to determine the listen port, so the app will bind to whatever PORT is set to. But the Docker port mapping doesn't reference the same variable for the container-side port.
The fix should use the PORT variable in the port mapping as well, e.g. "${PORT:-7072}:${PORT:-7072}", so the host-to-container mapping stays in sync with the actual listen port.
Impact: Any user who customizes the PORT variable (a reasonable thing to do) will find the API completely unreachable from the host, with no obvious error message — the container starts fine but traffic never reaches the app.
| - "7072:7072" | |
| - "${PORT:-7072}:${PORT:-7072}" |
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (req.method === 'OPTIONS' && isAllowed) { | ||
| res.status(204).send(); | ||
| return; | ||
| } | ||
|
|
||
| next(); |
There was a problem hiding this comment.
🟡 Disallowed-origin OPTIONS preflight requests are forwarded to route handlers instead of being rejected
When an OPTIONS preflight request arrives from a disallowed (or missing) origin, the CORS middleware calls next(), forwarding the request to the NestJS route handler. The handler processes it as a normal GET-like request and returns a 200 with a response body, instead of the middleware terminating it early.
Detailed Explanation
At apps/api/src/cors.middleware.ts:28-31, the early 204 return only fires when req.method === 'OPTIONS' && isAllowed. When isAllowed is false, execution falls through to next() on line 33. This means:
- A preflight from
http://evil.comhits the actual controller (e.g.,AppController.hello()), doing unnecessary work. - The response is
200with a JSON body rather than a clear rejection (e.g.,403or204with no CORS headers), which is inconsistent with the "strict CORS" intent described in the PR. - While browsers will still block the cross-origin request (no
Access-Control-Allow-Originheader is set), non-browser clients see a successful response, which is misleading.
The middleware should terminate disallowed OPTIONS requests early, e.g., with res.status(403).end().
| if (req.method === 'OPTIONS' && isAllowed) { | |
| res.status(204).send(); | |
| return; | |
| } | |
| next(); | |
| if (req.method === 'OPTIONS') { | |
| if (isAllowed) { | |
| res.status(204).send(); | |
| } else { | |
| res.status(403).end(); | |
| } | |
| return; | |
| } | |
| next(); |
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Motivation
NEXT_PUBLIC_API_BASE_URL(orAPP_API_BASE_URLfor static builds) with a localhost fallback to avoid hardcoded endpoints.Description
apps/apiexposingGET /api/helloreturning{ message: "Hello from NestJS (dev starter)." }andGET /api/healthreturning{ ok: true }implemented inapps/api/src/app.controller.ts.apps/api/src/cors.middleware.tsthat parsesCORS_ALLOWED_ORIGINSas a comma-separated allowlist, setsAccess-Control-Allow-Originonly when the request origin is present and allowlisted, responds204for allowedOPTIONSpreflight, and never uses wildcard CORS.apps/api/src/main.ts) to add sane security headers on API responses.apps/api/Dockerfile, rootdocker-compose.yml(services:postgresandapi), root.env.example, andcodebase.yamldescribing the services, ports, env vars, and run commands.app/config.jsand updatingapp/index.htmlto setwindow.API_BASE_URLfromNEXT_PUBLIC_API_BASE_URLorAPP_API_BASE_URLwith ahttp://localhost:7072fallback, and updatedREADME.mdwith local dev, curl examples, and SPA run instructions.Testing
node --check app/config.jsandnode --check apps/api/src/main.ts, both completed successfully.npm installinsideapps/apibut it failed in this environment due to registry access (403 Forbidden), so package installation/npm run start:devwas not executed here.docker compose configto validate compose output but Docker CLI is not available in this environment, so container brings-up were not run here.app/index.htmlandapp/config.jschanges are syntactically loadable and that the SPA now derivesAPI_BASE_URLat runtime per the specified precedence (tested with staticnode --check).Codex Task
Summary by Sourcery
Introduce a NestJS-based API scaffold with strict CORS and Dockerized Postgres to provide a real backend for the SPA and align the frontend with runtime-configured API base URLs.
New Features:
Enhancements:
Build:
Deployment:
Documentation:
Chores: