feat: added dockerfile to web app and fixed vite config to use --host#34
feat: added dockerfile to web app and fixed vite config to use --host#34BuckyMcYolo merged 2 commits intomainfrom
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds containerization for the web app: new multi-stage Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/Dockerfile`:
- Around line 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.
- Around line 6-14: The Dockerfile's build stage doesn't declare or expose
NEXT_PUBLIC_API_URL and NEXT_PUBLIC_REALTIME_URL, so the web build won't see
compose values; add ARG NEXT_PUBLIC_API_URL and ARG NEXT_PUBLIC_REALTIME_URL in
the build stage and then set ENV NEXT_PUBLIC_API_URL=$NEXT_PUBLIC_API_URL and
ENV NEXT_PUBLIC_REALTIME_URL=$NEXT_PUBLIC_REALTIME_URL before the RUN pnpm
--filter web build command so the pnpm --filter web build (and any build-time
tools) pick up those values from the build args.
In `@docker-compose.yml`:
- Around line 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.
In `@packages/env/src/server.ts`:
- 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.
- 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c6e00306-5a5b-4e86-ab06-fb5403d286eb
📒 Files selected for processing (5)
apps/web/Dockerfileapps/web/nginx.confapps/web/package.jsondocker-compose.ymlpackages/env/src/server.ts
| 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;"] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
rg -n '^(FROM|USER|EXPOSE|CMD)\b' apps/web/DockerfileRepository: 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.
| args: | ||
| NEXT_PUBLIC_API_URL: ${NEXT_PUBLIC_API_URL:-http://localhost:8080} | ||
| NEXT_PUBLIC_REALTIME_URL: ${NEXT_PUBLIC_REALTIME_URL:-http://localhost:8000} |
There was a problem hiding this comment.
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.
| 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), |
There was a problem hiding this comment.
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.
🧩 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)}`)
}
NODERepository: 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.
| 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.
1 participant
Overview
This PR adds Docker containerization for the web application with Nginx runtime configuration, updates the Vite preview server to bind to all interfaces, integrates the web service into docker-compose, and adjusts an environment variable default.
Changes
Docker Support for Web App
apps/web/Dockerfile: Added multi-stage Docker build using
node:22-slim(build) andnginx:alpine(runtime). Build stage enables Corepack, activatespnpm@9.0.0, installs dependencies withpnpm install --frozen-lockfile, builds@repo/api,@repo/api-client, and thewebpackage. Declares build argsNEXT_PUBLIC_API_URL,NEXT_PUBLIC_REALTIME_URL, andNEXT_PUBLIC_MAX_FILE_UPLOAD_SIZEand exports them as environment variables for the build. Runtime stage copiesapps/web/distinto Nginx web root, replaces Nginx config withapps/web/nginx.conf, exposes port3000, and runs Nginx in the foreground.apps/web/nginx.conf: Added Nginx configuration serving the SPA on port
3000, enabling SPA fallback (try_files $uri $uri/ /index.html) and long-lived caching for static assets (expires 1y; Cache-Control: public, immutable).Docker Compose Integration
webservice that builds fromapps/web/Dockerfilewith build argsNEXT_PUBLIC_API_URLandNEXT_PUBLIC_REALTIME_URL(defaults to localhost values), exposes port3000:3000, restartsunless-stopped, and depends on theapiservice (condition: service_started).Configuration Updates
apps/web/package.json: Modified the
startscript to include--hostfor Vite preview (vite preview --port 3000 --host) so the preview server binds to all interfaces (allowing containerized access).packages/env/src/server.ts: Changed the
SELF_HOSTEDenvironment variable default fromtruetofalsein the Zod schema (SELF_HOSTED: z.coerce.boolean().default(false)).Issues & Concerns
@repo/apiand@repo/api-clientduring the web build. If those builds are unnecessary for producing the web static assets, this increases build time and image complexity and could be optimized.SELF_HOSTEDdefault fromtruetofalsealters runtime defaults and may affect existing deployments that rely on the previous default. Consider documenting this change and its implications or require explicit env override.Confidence Score: 4/5
The changes are coherent and correctly implement Docker build args and runtime configuration. Main concerns are build efficiency (unnecessary package builds) and the behavioral change to the
SELF_HOSTEDdefault which should be communicated to operators.