Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions apps/web/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
FROM node:22-slim AS build
ENV PNPM_HOME="/pnpm"
ENV PATH="$PNPM_HOME:$PATH"
RUN corepack enable && corepack prepare pnpm@9.0.0 --activate

WORKDIR /app
COPY pnpm-lock.yaml pnpm-workspace.yaml package.json turbo.json ./
COPY apps/web/ ./apps/web/
COPY apps/api/ ./apps/api/
COPY packages/ ./packages/
RUN pnpm install --frozen-lockfile
RUN pnpm --filter @repo/api build
RUN pnpm --filter @repo/api-client build
ARG NEXT_PUBLIC_API_URL
ARG NEXT_PUBLIC_REALTIME_URL
ARG NEXT_PUBLIC_MAX_FILE_UPLOAD_SIZE
ENV NEXT_PUBLIC_API_URL=$NEXT_PUBLIC_API_URL
ENV NEXT_PUBLIC_REALTIME_URL=$NEXT_PUBLIC_REALTIME_URL
ENV NEXT_PUBLIC_MAX_FILE_UPLOAD_SIZE=$NEXT_PUBLIC_MAX_FILE_UPLOAD_SIZE
RUN pnpm --filter web build
Comment thread
coderabbitai[bot] marked this conversation as resolved.

FROM nginx:alpine
COPY --from=build /app/apps/web/dist /usr/share/nginx/html
COPY apps/web/nginx.conf /etc/nginx/conf.d/default.conf
EXPOSE 3000
CMD ["nginx", "-g", "daemon off;"]
Comment on lines +22 to +26
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -e
rg -n '^(FROM|USER|EXPOSE|CMD)\b' apps/web/Dockerfile

Repository: BuckyMcYolo/townhall

Length of output: 166


Add a USER instruction to run the Nginx container as an unprivileged user.

The final stage starts with FROM nginx:alpine (line 16) but never includes a USER instruction, so the container runs as root. Since the Nginx service listens on port 3000 (not a privileged port < 1024), there is no need to retain elevated privileges. Add an explicit USER directive to drop to a non-root user (e.g., USER nginx), or switch to an unprivileged Nginx base image variant.

🧰 Tools
🪛 Checkov (3.2.510)

[low] 1-20: Ensure that HEALTHCHECK instructions have been added to container images

(CKV_DOCKER_2)


[low] 1-20: Ensure that a user for the container has been created

(CKV_DOCKER_3)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/Dockerfile` around lines 16 - 20, The Dockerfile currently uses FROM
nginx:alpine and runs nginx as root because there is no USER instruction; update
the final stage to drop privileges by adding a USER directive (e.g., set the
container to USER nginx or another unprivileged user) so the nginx process
started by CMD ["nginx", "-g", "daemon off;"] runs non-root; ensure this change
is compatible with the copied site files (COPY --from=build /app/apps/web/dist
/usr/share/nginx/html) and nginx.conf and that file permissions allow the chosen
user to read the files and bind to EXPOSE 3000.

14 changes: 14 additions & 0 deletions apps/web/nginx.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
server {
listen 3000;
root /usr/share/nginx/html;
index index.html;

location / {
try_files $uri $uri/ /index.html;
}

location ~* \.(js|css|png|jpg|jpeg|gif|ico|svg|woff|woff2|ttf|eot)$ {
expires 1y;
add_header Cache-Control "public, immutable";
}
}
2 changes: 1 addition & 1 deletion apps/web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"scripts": {
"dev": "vite --port 3000",
"build": "vite build",
"start": "vite preview --port 3000",
"start": "vite preview --port 3000 --host",
"check-types": "tsc --noEmit"
},
"dependencies": {
Expand Down
14 changes: 14 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,20 @@ services:
redis:
condition: service_healthy

web:
build:
context: .
dockerfile: apps/web/Dockerfile
args:
NEXT_PUBLIC_API_URL: ${NEXT_PUBLIC_API_URL:-http://localhost:8080}
NEXT_PUBLIC_REALTIME_URL: ${NEXT_PUBLIC_REALTIME_URL:-http://localhost:8000}
Comment on lines +118 to +120
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't ship localhost as the public API/realtime origin.

These values are compiled into the SPA during the image build. Once the app is opened from another machine, http://localhost:8080 and http://localhost:8000 point to the visitor's machine, not the Docker host, so API and realtime calls fail. Use explicit public origins for deployment, or same-origin/relative URLs.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docker-compose.yml` around lines 118 - 120, The build args
NEXT_PUBLIC_API_URL and NEXT_PUBLIC_REALTIME_URL in docker-compose.yml currently
default to http://localhost:8080 and http://localhost:8000 which will be baked
into the SPA and break for remote clients; change the defaults so the image is
built with either explicit public origins or same-origin/relative URLs (for
example use a relative root '/' or leave no default so the CI/CD/deployment
injects the correct public origin) and ensure NEXT_PUBLIC_API_URL and
NEXT_PUBLIC_REALTIME_URL are set to the real public hostnames in your deployment
pipeline or compose override rather than localhost.

restart: unless-stopped
ports:
- "3000:3000"
depends_on:
api:
condition: service_started

volumes:
postgres_data:
redis_data:
2 changes: 1 addition & 1 deletion packages/env/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const serverSchema = z.object({
REALTIME_CORS_ORIGIN: z.string().default(DEFAULT_REALTIME_CORS_ORIGIN),
REDIS_URL: z.string().url(),
BETTER_AUTH_SECRET: z.string().min(1),
SELF_HOSTED: z.coerce.boolean().default(true),
SELF_HOSTED: z.coerce.boolean().default(false),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Keep the new SELF_HOSTED default in sync with the shipped config.

.env.example still documents SELF_HOSTED=true, and docker-compose.yml in this PR still leaves the variable unset. That means compose deployments now silently get false while the documented default remains true.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/env/src/server.ts` at line 36, The SELF_HOSTED config default was
changed to false but the shipped config/docs expect true; update the SELF_HOSTED
Zod declaration (SELF_HOSTED: z.coerce.boolean().default(...)) to .default(true)
so the runtime default matches .env.example and docker-compose behavior, and
verify any related docs or docker-compose environment omissions remain
consistent with SELF_HOSTED=true.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -e

echo "Current parser:"
sed -n '34,37p' packages/env/src/server.ts

echo
echo "JavaScript truthiness used by z.coerce.boolean():"
node - <<'NODE'
for (const value of ["true", "false", "1", "0", "", undefined]) {
  console.log(`${String(value)} -> ${Boolean(value)}`)
}
NODE

Repository: BuckyMcYolo/townhall

Length of output: 409


Replace z.coerce.boolean() with explicit string parsing to avoid truthiness bugs.

process.env is string-based. z.coerce.boolean() applies JavaScript truthiness, so SELF_HOSTED="false" and SELF_HOSTED="0" both parse as true (non-empty strings are truthy). Users setting the variable explicitly to false will get the opposite behavior.

🔧 Proposed fix
-  SELF_HOSTED: z.coerce.boolean().default(false),
+  SELF_HOSTED: z
+    .preprocess((value) => {
+      if (typeof value === "boolean") return value
+      if (typeof value === "string") {
+        const normalized = value.trim().toLowerCase()
+        if (normalized === "true") return true
+        if (normalized === "false") return false
+      }
+      return value
+    }, z.boolean())
+    .default(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
SELF_HOSTED: z.coerce.boolean().default(false),
SELF_HOSTED: z
.preprocess((value) => {
if (typeof value === "boolean") return value
if (typeof value === "string") {
const normalized = value.trim().toLowerCase()
if (normalized === "true") return true
if (normalized === "false") return false
}
return value
}, z.boolean())
.default(false),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/env/src/server.ts` at line 36, The SELF_HOSTED schema entry
currently uses z.coerce.boolean() which interprets any non-empty string as true;
update the SELF_HOSTED validator in packages/env/src/server.ts to parse the raw
string explicitly (e.g., accept "true" or "1" → true, "false" or "0" → false,
case-insensitive) and then coerce that into a boolean with a default of false so
that values like "false" and "0" resolve correctly; reference the SELF_HOSTED
schema key and ensure the validator returns a boolean type and still applies the
default(false) behavior.

MAX_FILE_UPLOAD_SIZE: z.coerce.number().default(DEFAULT_MAX_FILE_UPLOAD_SIZE),
NEXT_PUBLIC_API_URL: z.string().min(1).transform(addProtocol),
S3_ENDPOINT: z.string().url(),
Expand Down
Loading