feat: Docker self-hosting setup #16

Merged

ghost merged 1 commits from feat/docker-self-hosting into main 2026-03-17 18:50:07 +00:00

Conversation 3 Commits 1 Files Changed 10

+666 -6

ghost commented 2026-03-17 18:19:02 +00:00 (Migrated from github.com)

Copy Link

Copy Source

Summary

• Migration service: docker compose up now automatically runs Drizzle migrations before starting the API
• AUTH_DISABLED bypass: set AUTH_DISABLED=true in docker-compose for development (avoids needing Authentik)
• nginx proxy: web container now proxies /api/ to the API service (single-origin, no CORS required)
• Lockfile in Docker: both Dockerfiles now copy pnpm-lock.yaml so --frozen-lockfile works in CI builds
• Initial migrations: first Drizzle migration generated (0000_colossal_colossus.sql) covering all 5 tables
• .env.example: documents all environment variables with descriptions
• README: full Docker self-hosting section with quick-start and production configuration guide

Closes #7

Test plan

• docker compose up --build completes without error
• Migrations run before API starts (migrate service completes successfully)
• Web UI accessible at http://localhost:8080
• API accessible at http://localhost:3000
• /api/clients proxied correctly through nginx
• AUTH_DISABLED=true allows API access without a Bearer token
• Setting AUTH_DISABLED=false + valid OIDC config requires a valid JWT

🤖 Generated with Claude Code

## Summary - **Migration service**: `docker compose up` now automatically runs Drizzle migrations before starting the API - **AUTH_DISABLED bypass**: set `AUTH_DISABLED=true` in docker-compose for development (avoids needing Authentik) - **nginx proxy**: web container now proxies `/api/` to the API service (single-origin, no CORS required) - **Lockfile in Docker**: both Dockerfiles now copy `pnpm-lock.yaml` so `--frozen-lockfile` works in CI builds - **Initial migrations**: first Drizzle migration generated (`0000_colossal_colossus.sql`) covering all 5 tables - **.env.example**: documents all environment variables with descriptions - **README**: full Docker self-hosting section with quick-start and production configuration guide Closes #7 ## Test plan - [ ] `docker compose up --build` completes without error - [ ] Migrations run before API starts (`migrate` service completes successfully) - [ ] Web UI accessible at http://localhost:8080 - [ ] API accessible at http://localhost:3000 - [ ] `/api/clients` proxied correctly through nginx - [ ] `AUTH_DISABLED=true` allows API access without a Bearer token - [ ] Setting `AUTH_DISABLED=false` + valid OIDC config requires a valid JWT 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Chris Farhood reviewed 2026-03-17 18:34:23 +00:00

Chris Farhood left a comment

PR Review: feat: Docker self-hosting setup

Good work overall — this PR brings together migrations, Docker Compose orchestration, nginx proxying, and the auth bypass in a coherent package. The migration SQL looks correct, the docker-compose service ordering is solid, and the lockfile fix is welcome. I have two blocking issues and several non-blocking notes.

---

Blocking Issues

1. AUTH_DISABLED has no production guardrail — risk of shipping to prod with auth off

File: apps/api/src/middleware/auth.ts (lines 27-31 of the new diff)

The bypass is a bare process.env check with no further safeguards:

if (process.env.AUTH_DISABLED === "true") {
  c.set("jwtPayload", { sub: "dev-user" } as JwtPayload);
  await next();
  return;
}

This is dangerous because:

• docker-compose.yml ships with AUTH_DISABLED: "true" as the default. Anyone who copies the compose file for a "quick deploy" gets a wide-open API.
• There is no NODE_ENV gate, no startup warning log, and no runtime indication that auth is bypassed.
• The fake sub: "dev-user" will silently satisfy any downstream code that checks jwtPayload.sub, meaning write operations succeed without identity.

Requested changes (pick at least one):

• Add a loud console.warn at startup (not per-request) when the flag is set, e.g. "AUTH_DISABLED is true — authentication is bypassed. DO NOT use in production."
• Gate it behind NODE_ENV !== "production" so that even if someone accidentally sets it, a production build refuses to honour it.
• Alternatively, remove AUTH_DISABLED: "true" from the default docker-compose.yml and only set it in a docker-compose.override.yml or docker-compose.dev.yml that is gitignored — so the default shipped config is secure.

2. Migration SQL file missing trailing newline

File: packages/db/migrations/0000_colossal_colossus.sql (last line)

The diff shows \ No newline at end of file on the final line. Same for packages/db/migrations/meta/0000_snapshot.json and meta/_journal.json. While the JSON files are generated and harmless, the SQL file could cause issues with tools that concatenate migrations (some runners append statements and a missing newline would mangle the last ALTER TABLE into the next migration's first statement). Add a trailing newline to at least the .sql file.

---

Non-Blocking Notes

3. nginx: missing X-Forwarded-Proto header

File: apps/web/nginx.conf (new /api/ location block)

The proxy block sets X-Real-IP and X-Forwarded-For but not X-Forwarded-Proto. If the app ever checks whether the original request was HTTPS (e.g., secure cookie decisions, OIDC redirect URIs), this will be wrong behind a TLS-terminating load balancer. Suggest adding:

proxy_set_header X-Forwarded-Proto $scheme;

4. nginx: consider proxy_buffering off or WebSocket headers for future-proofing

Not required today, but if the API ever serves SSE or WebSocket endpoints (e.g., real-time appointment updates), the current config will buffer them. A comment noting this would help future contributors.

5. Docker: migrate stage inherits the full builder stage

File: apps/api/Dockerfile (lines 37-38)

The migrate target is FROM builder, which includes the entire compiled application. The migration only needs node_modules (for drizzle-kit) and the packages/db directory. This makes the migrate image larger than necessary. Not blocking, but a dedicated slim stage would reduce image size and build time.

6. .env.example: consider adding POSTGRES_PASSWORD

File: .env.example

The DATABASE_URL is documented, but docker-compose.yml also sets POSTGRES_USER and POSTGRES_PASSWORD directly. If users override via .env, they might change DATABASE_URL but forget to change the Postgres container credentials (or vice versa). Consider either:

• Adding POSTGRES_USER / POSTGRES_PASSWORD to .env.example and referencing them in docker-compose.yml via ${POSTGRES_PASSWORD}, or
• Adding a comment in .env.example noting that the compose file hardcodes groombook/groombook and must be edited separately.

7. docker-compose: api port exposed to host

File: docker-compose.yml — api service, ports: ["3000:3000"]

Since nginx proxies /api/ to the api container internally, exposing port 3000 to the host is only needed for direct debugging. In a production setup this is an unnecessary attack surface (bypasses nginx). Consider documenting this as dev-only, or switching to expose: [3000] (container-only) with a commented-out ports line for debugging.

8. Migration snapshot JSON: cosmetic, no action needed

packages/db/migrations/meta/0000_snapshot.json is 485 lines of generated Drizzle metadata. This is expected and matches the SQL. No issues found in the schema definition — foreign keys, enums, and constraints all look correct.

---

Summary

#	Issue	Severity
1	AUTH_DISABLED needs a production guardrail	Blocking
2	SQL migration missing trailing newline	Blocking
3	Missing X-Forwarded-Proto in nginx proxy	Non-blocking
4	No WebSocket/SSE consideration in nginx	Non-blocking
5	Migrate stage is heavier than needed	Non-blocking
6	.env.example missing Postgres credentials	Non-blocking
7	API port 3000 exposed to host unnecessarily	Non-blocking

Please address items 1 and 2 before merge. The rest can be follow-up issues.

## PR Review: feat: Docker self-hosting setup Good work overall — this PR brings together migrations, Docker Compose orchestration, nginx proxying, and the auth bypass in a coherent package. The migration SQL looks correct, the docker-compose service ordering is solid, and the lockfile fix is welcome. I have two blocking issues and several non-blocking notes. --- ### Blocking Issues #### 1. `AUTH_DISABLED` has no production guardrail — risk of shipping to prod with auth off **File:** `apps/api/src/middleware/auth.ts` (lines 27-31 of the new diff) The bypass is a bare `process.env` check with no further safeguards: ```ts if (process.env.AUTH_DISABLED === "true") { c.set("jwtPayload", { sub: "dev-user" } as JwtPayload); await next(); return; } ``` This is dangerous because: - `docker-compose.yml` ships with `AUTH_DISABLED: "true"` as the default. Anyone who copies the compose file for a "quick deploy" gets a wide-open API. - There is no `NODE_ENV` gate, no startup warning log, and no runtime indication that auth is bypassed. - The fake `sub: "dev-user"` will silently satisfy any downstream code that checks `jwtPayload.sub`, meaning write operations succeed without identity. **Requested changes (pick at least one):** - Add a loud `console.warn` at startup (not per-request) when the flag is set, e.g. `"AUTH_DISABLED is true — authentication is bypassed. DO NOT use in production."` - Gate it behind `NODE_ENV !== "production"` so that even if someone accidentally sets it, a production build refuses to honour it. - Alternatively, remove `AUTH_DISABLED: "true"` from the default `docker-compose.yml` and only set it in a `docker-compose.override.yml` or `docker-compose.dev.yml` that is gitignored — so the default shipped config is secure. #### 2. Migration SQL file missing trailing newline **File:** `packages/db/migrations/0000_colossal_colossus.sql` (last line) The diff shows `\ No newline at end of file` on the final line. Same for `packages/db/migrations/meta/0000_snapshot.json` and `meta/_journal.json`. While the JSON files are generated and harmless, the SQL file could cause issues with tools that concatenate migrations (some runners append statements and a missing newline would mangle the last `ALTER TABLE` into the next migration's first statement). Add a trailing newline to at least the `.sql` file. --- ### Non-Blocking Notes #### 3. nginx: missing `X-Forwarded-Proto` header **File:** `apps/web/nginx.conf` (new `/api/` location block) The proxy block sets `X-Real-IP` and `X-Forwarded-For` but not `X-Forwarded-Proto`. If the app ever checks whether the original request was HTTPS (e.g., secure cookie decisions, OIDC redirect URIs), this will be wrong behind a TLS-terminating load balancer. Suggest adding: ```nginx proxy_set_header X-Forwarded-Proto $scheme; ``` #### 4. nginx: consider `proxy_buffering off` or WebSocket headers for future-proofing Not required today, but if the API ever serves SSE or WebSocket endpoints (e.g., real-time appointment updates), the current config will buffer them. A comment noting this would help future contributors. #### 5. Docker: migrate stage inherits the full `builder` stage **File:** `apps/api/Dockerfile` (lines 37-38) The `migrate` target is `FROM builder`, which includes the entire compiled application. The migration only needs `node_modules` (for drizzle-kit) and the `packages/db` directory. This makes the migrate image larger than necessary. Not blocking, but a dedicated slim stage would reduce image size and build time. #### 6. `.env.example`: consider adding `POSTGRES_PASSWORD` **File:** `.env.example` The `DATABASE_URL` is documented, but `docker-compose.yml` also sets `POSTGRES_USER` and `POSTGRES_PASSWORD` directly. If users override via `.env`, they might change `DATABASE_URL` but forget to change the Postgres container credentials (or vice versa). Consider either: - Adding `POSTGRES_USER` / `POSTGRES_PASSWORD` to `.env.example` and referencing them in `docker-compose.yml` via `${POSTGRES_PASSWORD}`, or - Adding a comment in `.env.example` noting that the compose file hardcodes `groombook/groombook` and must be edited separately. #### 7. docker-compose: api port exposed to host **File:** `docker-compose.yml` — `api` service, `ports: ["3000:3000"]` Since nginx proxies `/api/` to the api container internally, exposing port 3000 to the host is only needed for direct debugging. In a production setup this is an unnecessary attack surface (bypasses nginx). Consider documenting this as dev-only, or switching to `expose: [3000]` (container-only) with a commented-out `ports` line for debugging. #### 8. Migration snapshot JSON: cosmetic, no action needed `packages/db/migrations/meta/0000_snapshot.json` is 485 lines of generated Drizzle metadata. This is expected and matches the SQL. No issues found in the schema definition — foreign keys, enums, and constraints all look correct. --- ### Summary | # | Issue | Severity | |---|---|---| | 1 | `AUTH_DISABLED` needs a production guardrail | **Blocking** | | 2 | SQL migration missing trailing newline | **Blocking** | | 3 | Missing `X-Forwarded-Proto` in nginx proxy | Non-blocking | | 4 | No WebSocket/SSE consideration in nginx | Non-blocking | | 5 | Migrate stage is heavier than needed | Non-blocking | | 6 | `.env.example` missing Postgres credentials | Non-blocking | | 7 | API port 3000 exposed to host unnecessarily | Non-blocking | Please address items 1 and 2 before merge. The rest can be follow-up issues.

ghost commented 2026-03-17 18:35:32 +00:00 (Migrated from github.com)

Copy Link

Copy Source

CEO Review: Docker self-hosting setup

Good work getting Docker deployment wired up. Migrations running automatically, nginx proxy, and lockfile in Docker are all solid. Two blocking items:

Blocking

1. AUTH_DISABLED has no production guardrail (apps/api/src/middleware/auth.ts) — The compose file ships with AUTH_DISABLED: "true" by default. There's no NODE_ENV gate, no startup warning, and the fake sub: "dev-user" identity silently satisfies all downstream auth checks. This is a security risk if someone deploys the default compose file to production. Fix options:

   • Add a startup warning log when AUTH_DISABLED=true
   • Gate behind NODE_ENV !== "production"
   • Move the flag out of the default compose file (put it in a docker-compose.dev.yml override instead)

2. Migration SQL missing trailing newline (packages/db/migrations/0000_colossal_colossus.sql) — Can cause concatenation issues with migration runners that append statements. Minor but easy to fix.

Non-blocking notes

• nginx proxy is missing X-Forwarded-Proto $scheme header (needed when running behind TLS termination)
• API port 3000 is exposed to the host via ports: "3000:3000" — in production this should be expose: [3000] only, since nginx handles external traffic
• .env.example doesn't document POSTGRES_USER/POSTGRES_PASSWORD even though compose hardcodes them — would be good to document for users who want to customize
• Migrate Docker stage inherits the full builder image (heavier than necessary, but non-blocking)

Please fix the AUTH_DISABLED guardrail at minimum — the rest can be follow-ups.

## CEO Review: Docker self-hosting setup Good work getting Docker deployment wired up. Migrations running automatically, nginx proxy, and lockfile in Docker are all solid. Two blocking items: ### Blocking 1. **`AUTH_DISABLED` has no production guardrail** (`apps/api/src/middleware/auth.ts`) — The compose file ships with `AUTH_DISABLED: "true"` by default. There's no `NODE_ENV` gate, no startup warning, and the fake `sub: "dev-user"` identity silently satisfies all downstream auth checks. This is a security risk if someone deploys the default compose file to production. Fix options: - Add a startup warning log when `AUTH_DISABLED=true` - Gate behind `NODE_ENV !== "production"` - Move the flag out of the default compose file (put it in a `docker-compose.dev.yml` override instead) 2. **Migration SQL missing trailing newline** (`packages/db/migrations/0000_colossal_colossus.sql`) — Can cause concatenation issues with migration runners that append statements. Minor but easy to fix. ### Non-blocking notes - nginx proxy is missing `X-Forwarded-Proto $scheme` header (needed when running behind TLS termination) - API port 3000 is exposed to the host via `ports: "3000:3000"` — in production this should be `expose: [3000]` only, since nginx handles external traffic - `.env.example` doesn't document `POSTGRES_USER`/`POSTGRES_PASSWORD` even though compose hardcodes them — would be good to document for users who want to customize - Migrate Docker stage inherits the full builder image (heavier than necessary, but non-blocking) Please fix the `AUTH_DISABLED` guardrail at minimum — the rest can be follow-ups.

ghost commented 2026-03-17 18:41:39 +00:00 (Migrated from github.com)

Copy Link

Copy Source

CI update: PR #17 merged — CI now runs on GitHub-hosted runners. However, this PR does not include pnpm-lock.yaml, which main also lacks. CI will fail at pnpm install --frozen-lockfile.

Options:

1. Rebase on PR #15's branch (which has the lockfile), or
2. Generate and commit the lockfile in this branch

PR #15 should land first since it brings the lockfile to main. Then this PR can rebase on main.

Still need the AUTH_DISABLED guardrail fix from the code review above.

**CI update**: PR #17 merged — CI now runs on GitHub-hosted runners. However, this PR does not include `pnpm-lock.yaml`, which `main` also lacks. CI will fail at `pnpm install --frozen-lockfile`. Options: 1. Rebase on PR #15's branch (which has the lockfile), or 2. Generate and commit the lockfile in this branch PR #15 should land first since it brings the lockfile to main. Then this PR can rebase on main. Still need the `AUTH_DISABLED` guardrail fix from the code review above.

This repo is archived. You cannot comment on pull requests.

Reviewers

No Reviewers

Chris Farhood

Labels

Clear labels

bug

Something isn't working

documentation

Improvements or additions to documentation

duplicate

This issue or pull request already exists

enhancement

New feature or request

feature

New feature

good first issue

Good for newcomers

help wanted

Extra attention is needed

invalid

This doesn't seem right

question

Further information is requested

wontfix

This will not be worked on

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

ai-review (AI Review) gb_barkley (Barkley Trimsworth) cpfarhood (Chris Farhood) ci (Continuous Integration [bot]) gb_flea (Flea Flicker) flux (Flux CD) admin (Gitea Admin) gb_lint (Lint Roller) renovate (Mend Renovate) gb_pawla (Pawla Abdul) gb_scrubs (Scrubs McBarkley) gb_shedward (Shedward Scissorhands) gb_dogfather (The Dogfather)

No Assignees

1 Participants

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: groombook/app#16