From 0ca6aa0fbd7a0e10fda62d08e45008c8baf649c0 Mon Sep 17 00:00:00 2001 From: Aditya Rana Date: Sat, 29 Aug 2026 23:09:10 +0530 Subject: [PATCH 1/6] add a .dockerignore Both Dockerfiles build with `COPY . .` and there was no ignore file, so every image carried the whole working tree -- including `.env`, which holds a real GitHub client secret and the session secret. A secret copied into a layer stays in that layer regardless of what the running container does with it. This lands first and alone because it is the one fault that is actively dangerous rather than merely broken: the other seven stop the stack from working, this one would have shipped credentials. Also keeps host `node_modules` out (wrong platform anyway -- the parser's tree-sitter bindings are compiled), stale `dist` out, and test files out, which incidentally closes the compiled-tests-in-dist gap CLAUDE.md records as "harmless locally, wrong in an image". --- .dockerignore | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 .dockerignore diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 0000000..17c8b2b --- /dev/null +++ b/.dockerignore @@ -0,0 +1,46 @@ +# Both Dockerfiles build from the repo root and do `COPY . .`, so without this +# every image carried the whole working tree -- including `.env`, which holds a +# real GitHub client secret and session secret. A secret in a layer stays in +# that layer whatever the running container does with it. + +# Secrets first, because this is the line that matters. +.env +.env.* +!.env.example + +# Reinstalled inside the image, and a host node_modules is the wrong platform +# anyway -- the parser's tree-sitter bindings are compiled. +node_modules +**/node_modules + +# Build output. The image builds its own; copying a stale one in is how a +# container serves last week's bundle. +dist +**/dist +apps/web/dist +services/parser/bin + +# History and tooling that never affect the build, but do bust the cache on +# every commit. +.git +.github +.claude +.agents +**/*.log + +# Tests do not run in the image. This also keeps compiled test files out of +# `apps/api/dist`, which CLAUDE.md lists as harmless locally and wrong here. +**/*.test.ts +**/*.test.tsx +**/*_test.go +**/__tests__ + +# Documentation and plans. +docs +*.md +!README.md + +# Editor and OS noise. +.vscode +.idea +.DS_Store From 88321b66ad3685fb1ac9063994c700dcb0fdb541 Mon Sep 17 00:00:00 2001 From: Aditya Rana Date: Sat, 29 Aug 2026 23:17:26 +0530 Subject: [PATCH 2/6] give the API and the worker an image that starts Four faults in one commit, because they only fail one at a time. The build context was `./apps/api` while the Dockerfile COPYs workspace manifests and packages/shared from above it, so the build died on its first COPY. It is the repo root now, with `dockerfile:` pointing here. The runner never copied packages/shared and ran `node dist/index.js`, but that package exports `./src/*.ts` and node cannot follow a `.ts` export. The image ships shared as source and runs under tsx, which leaves `pnpm dev`, Vite and Vitest resolving exactly as they did. `pnpm start` was `node dist/index.js` and could never have worked either; it is `tsx src/index.ts` now, closing a gap CLAUDE.md had recorded. There was no worker service at all. A webhook verified its HMAC, enqueued a job, answered 202, and nothing ever consumed it -- incremental refresh silently did nothing, which reads as a slow parse rather than as a missing process. It is the same image with a different command, since the two differ only in entrypoint. Three things found by building it rather than reading it: pnpm 11.15.1 cannot run on Node 20. It uses a builtin the runtime does not have and dies with ERR_UNKNOWN_BUILTIN_MODULE before installing anything, so both stages are node:24-alpine. The worker needs `git` in the image. `internal/clone/clone.go` shells out to `git clone`, and a bare Alpine has none -- the existing parser image has the same hole, which went unnoticed because the isolation harness only ever parses a local fixture. `.env` is written for host-native `make start`, where Postgres and Redis are on localhost. Inside a container localhost is the container, so DATABASE_URL and REDIS_URL are overridden with service names; `environment` beats `env_file`. WEB_APP_URL, CORS_ORIGIN and GITHUB_REDIRECT_URI stay host-facing because the browser is on the host. The API port is published on 127.0.0.1 rather than 0.0.0.0. Verified running: healthz answers 200, /api/repos answers 401, the worker attaches to the queue, the parser binary and git are both present in the image, and no .env was baked into a layer. --- apps/api/Dockerfile | 75 +++++++++++++++++++++++++++++++++++-------- apps/api/package.json | 3 +- docker-compose.yml | 39 ++++++++++++++++++++-- 3 files changed, 99 insertions(+), 18 deletions(-) diff --git a/apps/api/Dockerfile b/apps/api/Dockerfile index a43de5b..e61b835 100644 --- a/apps/api/Dockerfile +++ b/apps/api/Dockerfile @@ -1,23 +1,70 @@ -FROM node:20-alpine AS base +# The API and the parse worker, in one image. +# +# Build context is the **repo root**, not apps/api -- every COPY below reaches +# outside this directory. `docker-compose.yml` sets `context: .` and points +# `dockerfile:` here. It used to say `build: ./apps/api`, which put the context +# inside this folder and failed on the first COPY. +# +# One image for two services because they differ only in entrypoint, and +# keeping two Dockerfiles in step is a job nobody remembers to do. The worker is +# the one that actually needs the parser binary and git -- `runParser` is called +# from `queue/worker.ts` and nowhere else -- but splitting the image to save the +# API a few megabytes buys less than it costs. + +# --- The Go parser ----------------------------------------------------------- +# CGO is required: every tree-sitter grammar is a C library. +FROM golang:1.26-alpine AS parser +RUN apk add --no-cache build-base +WORKDIR /src +COPY services/parser/go.mod services/parser/go.sum ./ +RUN go mod download +COPY services/parser/ ./ +RUN CGO_ENABLED=1 go build -ldflags="-w -s" -o /parser ./cmd/parser + +# --- Node dependencies ------------------------------------------------------- +# Manifests only, so this layer survives every source edit. +# +# Node 24, not 20: `packageManager` pins pnpm 11.15.1, which uses a builtin +# module Node 20 does not have and dies with ERR_UNKNOWN_BUILTIN_MODULE before +# it installs anything. +FROM node:24-alpine AS deps RUN corepack enable WORKDIR /app - -FROM base AS deps -COPY package.json pnpm-workspace.yaml turbo.json .npmrc ./ +COPY package.json pnpm-workspace.yaml pnpm-lock.yaml turbo.json .npmrc ./ COPY apps/api/package.json apps/api/ COPY packages/shared/package.json packages/shared/ -RUN pnpm install --frozen-lockfile=false +# Dev dependencies included on purpose: `tsx` is one, and the runtime needs it. +# `packages/shared` exports `./src/*.ts`, so plain node cannot follow them -- +# see the CMD note. --frozen-lockfile so the image cannot silently resolve a +# different tree than the one committed. +RUN pnpm install --frozen-lockfile + +# --- Runtime ----------------------------------------------------------------- +FROM node:24-alpine AS runner +# git, because the parser shells out to `git clone` (internal/clone/clone.go). +# Without it a clone fails at runtime with a message about a missing binary, +# long after the image looked fine. +RUN apk add --no-cache git +RUN addgroup -S funcatlas && adduser -S funcatlas -G funcatlas +WORKDIR /app -FROM base AS build COPY --from=deps /app/node_modules ./node_modules -COPY . . -RUN pnpm --filter api build +COPY --from=deps /app/apps/api/node_modules ./apps/api/node_modules +COPY --from=deps /app/packages/shared/node_modules ./packages/shared/node_modules + +COPY package.json pnpm-workspace.yaml turbo.json ./ +COPY apps/api ./apps/api +# Shipped as source, not built. The API imports it through exports that point +# at `./src/*.ts`, which is why `node dist/index.js` exited immediately before. +COPY packages/shared ./packages/shared + +COPY --from=parser /parser /usr/local/bin/funcatlas-parser -FROM base AS runner -RUN addgroup -S funcatlas && adduser -S funcatlas -G funcatlas -COPY --from=build /app/node_modules ./node_modules -COPY --from=build /app/apps/api/dist ./dist -COPY --from=build /app/apps/api/package.json ./package.json USER funcatlas +WORKDIR /app/apps/api EXPOSE 3000 -CMD ["node", "dist/index.js"] + +# tsx directly out of node_modules/.bin rather than through `pnpm` or corepack: +# corepack tries to fetch its pinned pnpm on first use, which a non-root user in +# a container with no network to the registry cannot do. +CMD ["./node_modules/.bin/tsx", "src/index.ts"] diff --git a/apps/api/package.json b/apps/api/package.json index b36be8d..e985752 100644 --- a/apps/api/package.json +++ b/apps/api/package.json @@ -7,7 +7,8 @@ "dev": "tsx watch src/index.ts", "worker": "tsx watch src/worker.ts", "build": "tsc -p tsconfig.json", - "start": "node dist/index.js", + "start": "tsx src/index.ts", + "start:worker": "tsx src/worker.ts", "lint": "eslint . --max-warnings 0", "typecheck": "tsc --noEmit", "test": "vitest run" diff --git a/docker-compose.yml b/docker-compose.yml index 4aeccba..4e05130 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -29,16 +29,49 @@ services: timeout: 5s retries: 5 - api: - build: ./apps/api + # The API and the worker are the same image with different entrypoints. Only + # the worker spawns the parser, but one image is one thing to keep in step. + api: &node + build: + # The repo root, not apps/api: the Dockerfile COPYs the workspace + # manifests and packages/shared, which live above that directory. + context: . + dockerfile: apps/api/Dockerfile + image: funcatlas-node env_file: .env + environment: + # `environment` wins over `env_file`, which is the point: .env is written + # for host-native `make start`, where Postgres and Redis are on + # localhost. Inside a container localhost is the container, so these two + # have to be service names or every connection is refused. + DATABASE_URL: postgres://funcatlas:funcatlas@postgres:5432/funcatlas?sslmode=disable + REDIS_URL: redis://redis:6379 + # The default is an absolute host path from `make go-build-bin`. In the + # image the binary is somewhere else entirely. + PARSER_BIN: /usr/local/bin/funcatlas-parser + # Everything else stays host-facing on purpose: the browser runs on the + # host, so WEB_APP_URL, CORS_ORIGIN and GITHUB_REDIRECT_URI are correct + # as written in .env. depends_on: postgres: condition: service_healthy redis: condition: service_healthy + # Bound to loopback, not 0.0.0.0. With FUNCATLAS_SINGLE_USER set there is + # no authentication in front of this, and the host's port mapping is the + # only thing keeping it off the network -- the process cannot bind loopback + # itself or Docker could not reach it. See R39. ports: - - "3000:3000" + - "127.0.0.1:3000:3000" + + # Nothing consumed the queue before this existed. A webhook verified its + # HMAC, enqueued a job, answered 202, and the graph never changed -- which + # reads as a slow parse rather than as a missing process. + worker: + <<: *node + image: funcatlas-node + command: ["./node_modules/.bin/tsx", "src/worker.ts"] + ports: [] web: build: ./apps/web From afb754be4bddfad03382efe946cd701d673c103a Mon Sep 17 00:00:00 2001 From: Aditya Rana Date: Sat, 29 Aug 2026 23:29:04 +0530 Subject: [PATCH 3/6] add migrations, a web image, and stop the parser pretending to be a service Three of the remaining faults. Nothing ran migrations in compose. `make start` uses the golang-migrate CLI, which someone running `docker compose up` has no reason to have installed, so the API and worker would have reached a database with no tables and failed in a way that reads as a code bug. A `migrate` service on the image CI already uses (R17), and both consumers gate on `service_completed_successfully` -- finished, not merely started. `apps/web` had no Dockerfile at all, so `docker compose build` failed here first. It is a Vite build served by nginx, with `try_files` doing the SPA fallback `vercel.json` does on Vercel -- without it a refresh on /app is a 404. Asset requests deliberately do not fall back: a missing bundle should 404 rather than quietly return the HTML shell, which is how a broken build disguises itself as a routing bug. VITE_API_URL is a build ARG because Vite inlines it; setting it on the container would do nothing. The parser service declared `network_mode: none` and a dependency on Postgres health, which cannot both hold -- with no interfaces it can never reach Postgres over TCP. The dependency was vestigial and nobody hit it because `make parser-isolated` runs it with `--no-deps`. It is gone; the three isolation settings stay, since those are the actual contract. A `tools` profile keeps `docker compose up` from starting a container the product never uses (R38). Two more found by building rather than reading. `tsconfig.json` extends `@funcatlas/typescript-config/base.json`, so omitting that workspace package made tsc fall back to its own defaults and report forty errors that all traced to one missing file. And `pnpm run` verifies the workspace before executing a script, so the build stage needs the lockfile and every manifest, not just the ones the bundle imports -- otherwise it aborts with ERR_PNPM_WORKSPACE_PKG_NOT_FOUND attributed to the bundler. Verified with the stack actually up: migrate applies and exits, / and /app both answer 200, a missing asset answers 404, healthz answers 200, and the parser container is not started. --- apps/api/Dockerfile | 7 +++++++ apps/web/Dockerfile | 51 +++++++++++++++++++++++++++++++++++++++++++++ apps/web/nginx.conf | 29 ++++++++++++++++++++++++++ docker-compose.yml | 47 ++++++++++++++++++++++++++++++++++------- 4 files changed, 127 insertions(+), 7 deletions(-) create mode 100644 apps/web/Dockerfile create mode 100644 apps/web/nginx.conf diff --git a/apps/api/Dockerfile b/apps/api/Dockerfile index e61b835..542cec5 100644 --- a/apps/api/Dockerfile +++ b/apps/api/Dockerfile @@ -32,7 +32,13 @@ RUN corepack enable WORKDIR /app COPY package.json pnpm-workspace.yaml pnpm-lock.yaml turbo.json .npmrc ./ COPY apps/api/package.json apps/api/ +# Both config packages are copied, not just shared: `tsconfig.json` extends +# `@funcatlas/typescript-config/base.json`, and without it tsc silently falls +# back to its own defaults -- ES3 target, no JSX namespace -- and reports forty +# downstream errors that all trace to one missing file. COPY packages/shared/package.json packages/shared/ +COPY packages/typescript-config/ packages/typescript-config/ +COPY packages/eslint-config/package.json packages/eslint-config/ # Dev dependencies included on purpose: `tsx` is one, and the runtime needs it. # `packages/shared` exports `./src/*.ts`, so plain node cannot follow them -- # see the CMD note. --frozen-lockfile so the image cannot silently resolve a @@ -57,6 +63,7 @@ COPY apps/api ./apps/api # Shipped as source, not built. The API imports it through exports that point # at `./src/*.ts`, which is why `node dist/index.js` exited immediately before. COPY packages/shared ./packages/shared +COPY packages/typescript-config ./packages/typescript-config COPY --from=parser /parser /usr/local/bin/funcatlas-parser diff --git a/apps/web/Dockerfile b/apps/web/Dockerfile new file mode 100644 index 0000000..0df9f59 --- /dev/null +++ b/apps/web/Dockerfile @@ -0,0 +1,51 @@ +# The web app: a Vite build served as static files. +# +# Build context is the repo root, like the API's -- the bundle needs the +# workspace manifests and `packages/shared`, which live above this directory. +# `docker-compose.yml` sets `context: .` and points `dockerfile:` here. + +FROM node:24-alpine AS deps +RUN corepack enable +WORKDIR /app +COPY package.json pnpm-workspace.yaml pnpm-lock.yaml turbo.json .npmrc ./ +COPY apps/web/package.json apps/web/ +# Both config packages are copied, not just shared: `tsconfig.json` extends +# `@funcatlas/typescript-config/base.json`, and without it tsc silently falls +# back to its own defaults -- ES3 target, no JSX namespace -- and reports forty +# downstream errors that all trace to one missing file. +COPY packages/shared/package.json packages/shared/ +COPY packages/typescript-config/ packages/typescript-config/ +COPY packages/eslint-config/package.json packages/eslint-config/ +RUN pnpm install --frozen-lockfile + +FROM node:24-alpine AS build +RUN corepack enable +WORKDIR /app +COPY --from=deps /app/node_modules ./node_modules +COPY --from=deps /app/apps/web/node_modules ./apps/web/node_modules +COPY --from=deps /app/packages/shared/node_modules ./packages/shared/node_modules +# The lockfile and every workspace manifest, not just the ones the bundle +# imports: `pnpm run` verifies the dependency tree before executing a script, +# and a workspace member it cannot find aborts the build with +# ERR_PNPM_WORKSPACE_PKG_NOT_FOUND -- reported against `pnpm --filter web +# build`, which makes it look like a bundler failure. +COPY package.json pnpm-workspace.yaml pnpm-lock.yaml turbo.json .npmrc ./ +COPY apps/web ./apps/web +COPY packages/shared ./packages/shared +COPY packages/typescript-config ./packages/typescript-config +COPY packages/eslint-config/package.json ./packages/eslint-config/ + +# Vite inlines env at build time, so this is an ARG rather than a runtime +# variable -- setting it on the container would do nothing at all. +ARG VITE_API_URL=http://localhost:3000 +ENV VITE_API_URL=${VITE_API_URL} +RUN pnpm --filter web build + +# Deliberately not a showcase build: this stack has an API behind it, so the +# call to action goes to the canvas rather than to the repository. VITE_SHOWCASE +# is for the web-only Vercel deploy. + +FROM nginx:alpine AS runner +COPY apps/web/nginx.conf /etc/nginx/conf.d/default.conf +COPY --from=build /app/apps/web/dist /usr/share/nginx/html +EXPOSE 80 diff --git a/apps/web/nginx.conf b/apps/web/nginx.conf new file mode 100644 index 0000000..2a85221 --- /dev/null +++ b/apps/web/nginx.conf @@ -0,0 +1,29 @@ +# The web app is a single-page bundle with two client routes, `/` and `/app`. +# Without the fallback below, a refresh on /app or a link straight to it asks +# nginx for a file that does not exist and gets a 404 -- the same job +# `vercel.json`'s rewrite does on Vercel. +server { + listen 80; + server_name _; + root /usr/share/nginx/html; + index index.html; + + # Hashed filenames, so these can be cached indefinitely. Kept above the + # catch-all: a missing asset should 404 rather than quietly return the + # HTML shell, which is how a broken build looks like a routing bug. + location /assets/ { + try_files $uri =404; + expires 1y; + add_header Cache-Control "public, immutable"; + } + + location / { + try_files $uri $uri/ /index.html; + } + + # The shell itself must never be cached, or a deploy leaves browsers asking + # for the previous build's asset names. + location = /index.html { + add_header Cache-Control "no-cache"; + } +} diff --git a/docker-compose.yml b/docker-compose.yml index 4e05130..30208d4 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -29,6 +29,22 @@ services: timeout: 5s retries: 5 + # Nothing ran migrations in compose. `make start` does it with the + # golang-migrate CLI, which a contributor running `docker compose up` has no + # reason to have installed. Same image CI already uses (R17). + migrate: + image: migrate/migrate:v4.18.1 + volumes: + - ./services/parser/migrations:/migrations:ro + command: + - -path=/migrations + - -database=postgres://funcatlas:funcatlas@postgres:5432/funcatlas?sslmode=disable + - up + depends_on: + postgres: + condition: service_healthy + restart: "no" + # The API and the worker are the same image with different entrypoints. Only # the worker spawns the parser, but one image is one thing to keep in step. api: &node @@ -57,6 +73,10 @@ services: condition: service_healthy redis: condition: service_healthy + # Not just started -- finished. Either service reaching a database with + # no tables fails in a way that reads as a code bug. + migrate: + condition: service_completed_successfully # Bound to loopback, not 0.0.0.0. With FUNCATLAS_SINGLE_USER set there is # no authentication in front of this, and the host's port mapping is the # only thing keeping it off the network -- the process cannot bind loopback @@ -74,24 +94,37 @@ services: ports: [] web: - build: ./apps/web + build: + context: . + dockerfile: apps/web/Dockerfile + args: + # Vite inlines this at build time, so it cannot be an environment + # variable on the running container. The browser is on the host, so it + # is localhost rather than the `api` service name. + VITE_API_URL: http://localhost:3000 depends_on: - api ports: - - "5173:5173" + - "127.0.0.1:5173:80" + # The isolation harness, not a service the stack runs. `make parser-isolated` + # invokes it with `docker compose run --rm --no-deps`, which is why nobody + # noticed it declared both `network_mode: none` and a dependency on Postgres + # health -- with no interfaces it can never reach Postgres over TCP, so it + # could never have started. The depends_on was vestigial and is gone; the + # three isolation settings are the actual contract (docs/SECURITY.md). + # + # Behind a profile so `docker compose up` does not try to start it. The + # product spawns the parser as a subprocess of the worker and never uses this + # container -- see R38. parser: build: ./services/parser + profiles: ["tools"] env_file: .env network_mode: "none" read_only: true cap_drop: - ALL - depends_on: - postgres: - condition: service_healthy - redis: - condition: service_healthy volumes: pgdata: From f23569186e4682e5dd8522e9776dd1ecf7bebd12 Mon Sep 17 00:00:00 2001 From: Aditya Rana Date: Sat, 29 Aug 2026 23:42:33 +0530 Subject: [PATCH 4/6] add single-user mode and make setup Registering a GitHub OAuth app was the largest thing between a clone and a running stack. FUNCATLAS_SINGLE_USER removes it: the API registers no /auth/login, /auth/callback or /auth/logout and resolves every request to that user. The branch lives in `requireSession`, which is the only function that ever sets `req.session` -- a second hook would be a parallel auth path, and those drift until one forgets a check. The OAuth routes are not registered rather than registered-and-refusing: 404 says nothing, 401 tells a prober there is something here worth finding credentials for, which is the reasoning that shaped R30. This is not R30 in a new hat. That was a session-minting endpoint shipped in every deployment behind a NODE_ENV gate, so the gate had to be right exactly once. Here there is no endpoint and no gate: the instance is unauthenticated because a human wrote their own username into their own .env. What it is not is safe to expose, and I said otherwise when proposing it. The process binds 0.0.0.0 -- it must, inside a container, or Docker cannot reach it -- so an in-process loopback check is impossible. Compose publishes on 127.0.0.1, the server warns on every start, and R39 records the residual risk instead of implying it is closed. Three things this dragged in. The GitHub OAuth credentials had to become conditionally required, or the API still refused to start without an app it would never call; the check runs at startup, naming which are missing and both ways to fix it, rather than surfacing as a 500 on first sign-in. The arctic client is built lazily for the same reason. And a blank FUNCATLAS_SINGLE_USER= now means off, because answering "I tried to turn this off" with a Zod stack trace about string length is a bad first impression. `make setup` writes .env, generates both secrets, creates the test database and prints what it did -- including why the instance now has no authentication. Idempotent, and it never overwrites an existing .env. Two test faults fixed on the way. env.test classified any key without `.default(` as required, so it demanded the newly optional ones in CI. And a running compose worker consumes the jobs queue/parse.test.ts asserts on, which reads as a broken queue rather than as two things sharing one Redis; `make test` now says so and stops instead of failing obscurely. --- .env.example | 13 +++- Makefile | 33 +++++++- apps/api/src/auth/github.ts | 23 ++++-- apps/api/src/auth/routes.test.ts | 8 +- apps/api/src/auth/routes.ts | 37 +++++++-- apps/api/src/auth/session.ts | 27 +++++++ apps/api/src/auth/single-user.test.ts | 104 ++++++++++++++++++++++++++ apps/api/src/env.test.ts | 17 ++++- apps/api/src/env.ts | 57 +++++++++++++- apps/web/src/App.test.tsx | 10 +-- apps/web/src/components/AppHeader.tsx | 21 ++++-- packages/shared/src/types.ts | 7 ++ 12 files changed, 322 insertions(+), 35 deletions(-) create mode 100644 apps/api/src/auth/single-user.test.ts diff --git a/.env.example b/.env.example index 98e5de6..e8ec96d 100644 --- a/.env.example +++ b/.env.example @@ -35,8 +35,19 @@ REDIS_URL=redis://localhost:6379 # Queue name must match between API (producer) and parser (consumer). QUEUE_NAME=funcatlas-parse +# --- Running without a GitHub OAuth app --- +# Set this to any name and the API registers no /auth/login, /auth/callback or +# /auth/logout, and treats every request as that user. `make setup` writes it +# for you, which is what makes `docker compose up` work from a fresh clone. +# +# It means the instance has NO AUTHENTICATION. Compose publishes on 127.0.0.1 +# so nothing off-box can reach it; do not expose the port. Blank means off. +# See docs/RISKS.md R39. +FUNCATLAS_SINGLE_USER= + # --- GitHub OAuth --- -# Register an OAuth app; set its redirect URI to the value below. +# Only needed when FUNCATLAS_SINGLE_USER is blank. Register an OAuth app and +# set its callback to GITHUB_REDIRECT_URI below. GITHUB_CLIENT_ID= GITHUB_CLIENT_SECRET= GITHUB_REDIRECT_URI=http://localhost:3000/auth/callback diff --git a/Makefile b/Makefile index 9cd7db3..3efa544 100644 --- a/Makefile +++ b/Makefile @@ -2,7 +2,7 @@ # Run `make `. Most targets shell out to pnpm/turbo or docker. # `make start` is the one that brings the whole thing up; `make help` lists all. -.PHONY: start stop wait-infra migrate-test install dev build lint typecheck test \ +.PHONY: setup start stop wait-infra migrate-test install dev build lint typecheck test \ migrate up down health parser-isolated \ go-build go-build-bin go-test go-vet go-run go-tidy go-lint clean @@ -11,6 +11,29 @@ # need a variable from it write `$$DATABASE_URL`, not `$(DATABASE_URL)`. ENV := set -a && . ./.env && set +a +setup: ## One-time: write .env, generate secrets, create the test database + @test -f .env && echo ".env already exists, leaving it alone." || { \ + cp .env.example .env; \ + sed -i "s|^SESSION_SECRET=.*|SESSION_SECRET=$$(openssl rand -hex 32)|" .env; \ + sed -i "s|^GITHUB_WEBHOOK_SECRET=.*|GITHUB_WEBHOOK_SECRET=$$(openssl rand -hex 32)|" .env; \ + sed -i "s|^FUNCATLAS_SINGLE_USER=.*|FUNCATLAS_SINGLE_USER=$$(id -un)|" .env; \ + echo "Wrote .env with generated secrets."; \ + echo ""; \ + echo " FUNCATLAS_SINGLE_USER=$$(id -un) -- the API will run with NO"; \ + echo " AUTHENTICATION so you do not need a GitHub OAuth app. Compose"; \ + echo " publishes on 127.0.0.1 only. Do not expose the port."; \ + echo " Blank the value in .env to use real GitHub sign-in instead."; \ + echo ""; \ + } + $(MAKE) up + $(MAKE) wait-infra + @# Idempotent: the second run finds the database and says so. + $(ENV) && docker compose exec -T postgres psql -U funcatlas -d postgres \ + -c "CREATE DATABASE funcatlas_test OWNER funcatlas" 2>/dev/null \ + && echo "Created funcatlas_test." || echo "funcatlas_test already exists." + @echo "" + @echo "Ready. Run 'docker compose up' and open http://localhost:5173" + start: ## Bring up EVERYTHING: Postgres, Redis, migrations, parser binary, API, web @test -f .env || { echo "No .env — copy .env.example to .env first."; exit 1; } $(MAKE) up @@ -56,6 +79,14 @@ typecheck: ## Type-check all packages pnpm typecheck test: ## Run all tests (TS + Go) + @# A running compose worker consumes the very jobs queue/parse.test.ts + @# enqueues and asserts on, so the failures read as a broken queue rather + @# than as two things sharing one Redis. + @docker compose ps --services --filter status=running 2>/dev/null | grep -qx worker && { \ + echo "The compose worker is running; it will consume the jobs these tests assert on."; \ + echo "Run 'docker compose stop worker' first."; \ + exit 1; \ + } || true pnpm test # Sourced, or dbtest finds no DATABASE_URL and every integration test skips # -- a green run that never touched Postgres. diff --git a/apps/api/src/auth/github.ts b/apps/api/src/auth/github.ts index 6aed52f..109f1cb 100644 --- a/apps/api/src/auth/github.ts +++ b/apps/api/src/auth/github.ts @@ -2,11 +2,24 @@ import { GitHub } from "arctic"; import { env } from "../env.js"; import { GITHUB_USER_ENDPOINT } from "./constants.js"; -export const github = new GitHub( - env.GITHUB_CLIENT_ID, - env.GITHUB_CLIENT_SECRET, - env.GITHUB_REDIRECT_URI, -); +/** + * Built on demand rather than at import. + * + * Under FUNCATLAS_SINGLE_USER the credentials are absent and no OAuth route is + * registered, so constructing this at module load would fail on a client + * nothing was ever going to call. `env.ts` guarantees all three are present + * whenever single-user mode is off, which is the only time this runs. + */ +let client: GitHub | null = null; + +export function githubClient(): GitHub { + client ??= new GitHub( + env.GITHUB_CLIENT_ID as string, + env.GITHUB_CLIENT_SECRET as string, + env.GITHUB_REDIRECT_URI as string, + ); + return client; +} /** Only what a session needs. The rest of the profile is not ours to keep. */ diff --git a/apps/api/src/auth/routes.test.ts b/apps/api/src/auth/routes.test.ts index f3c5855..83ad2ef 100644 --- a/apps/api/src/auth/routes.test.ts +++ b/apps/api/src/auth/routes.test.ts @@ -115,7 +115,13 @@ describe("GET /auth/callback", () => { url: "/auth/me", headers: { cookie: String(sessionCookie) }, }); - expect(me.json()).toEqual({ userId: githubUser.id, login: githubUser.login }); + expect(me.json()).toEqual({ + userId: githubUser.id, + login: githubUser.login, + // False on the OAuth path. The web app reads this to decide + // whether a Sign out button has anything to POST to. + singleUser: false, + }); }); it("keeps the access token out of every response", async () => { diff --git a/apps/api/src/auth/routes.ts b/apps/api/src/auth/routes.ts index 3ad1b1f..d3fd4c3 100644 --- a/apps/api/src/auth/routes.ts +++ b/apps/api/src/auth/routes.ts @@ -9,11 +9,12 @@ import { OAUTH_STATE_TTL, } from "./constants.js"; import { clearCookie, readSignedCookie, setSignedCookie } from "./cookies.js"; -import { fetchGitHubUser, github } from "./github.js"; +import { fetchGitHubUser, githubClient } from "./github.js"; import { clearSessionCookie, createSession, destroySession, + isSingleUser, requireSession, sessionIdFrom, setSessionCookie, @@ -36,12 +37,27 @@ function sameState(a: string, b: string): boolean { } export function registerAuth(app: FastifyInstance) { + // Under FUNCATLAS_SINGLE_USER the three OAuth routes are never registered, + // so they answer 404 rather than existing and refusing -- a handler that + // returns 401 tells a prober there is something here worth credentials for. + // /auth/me stays: the web app calls it to learn who it is. + if (isSingleUser()) { + app.log.warn( + { login: env.FUNCATLAS_SINGLE_USER }, + "FUNCATLAS_SINGLE_USER is set: this API has no authentication. Every " + + "request is treated as this user and there is no sign-in. Do not " + + "expose this process beyond localhost. See docs/RISKS.md R39.", + ); + registerMe(app); + return; + } + app.get("/auth/login", async (_req, reply) => { const state = randomBytes(OAUTH_STATE_BYTES).toString("hex"); setSignedCookie(reply, OAUTH_STATE_COOKIE, state, OAUTH_STATE_TTL); // arctic 3's createAuthorizationURL is synchronous. - return reply.redirect(github.createAuthorizationURL(state, OAUTH_SCOPES).toString()); + return reply.redirect(githubClient().createAuthorizationURL(state, OAUTH_SCOPES).toString()); }); app.get("/auth/callback", async (req, reply) => { @@ -62,7 +78,7 @@ export function registerAuth(app: FastifyInstance) { let sessionId: string; try { - const tokens = await github.validateAuthorizationCode(params.data.code); + const tokens = await githubClient().validateAuthorizationCode(params.data.code); const accessToken = tokens.accessToken(); const user = await fetchGitHubUser(accessToken); sessionId = await createSession({ userId: user.id, login: user.login, accessToken }); @@ -92,11 +108,20 @@ export function registerAuth(app: FastifyInstance) { return reply.code(204).send(); }); - // Field by field on purpose: spreading the session would put the access - // token in the response body. + registerMe(app); +} + +/** + * Who the caller is. Registered on both paths, because the web app asks this + * before it renders anything and has no other way to learn the answer. + * + * Field by field on purpose: spreading the session would put the access token + * in the response body. + */ +function registerMe(app: FastifyInstance) { app.get("/auth/me", { preHandler: requireSession }, async (req) => ({ userId: req.session?.userId, login: req.session?.login, + singleUser: isSingleUser(), })); - } diff --git a/apps/api/src/auth/session.ts b/apps/api/src/auth/session.ts index 5d745d4..e2ffa93 100644 --- a/apps/api/src/auth/session.ts +++ b/apps/api/src/auth/session.ts @@ -67,11 +67,38 @@ export function sessionIdFrom(req: FastifyRequest): string | null { return readSignedCookie(req, env.SESSION_COOKIE_NAME); } +/** + * The identity every request gets under FUNCATLAS_SINGLE_USER. + * + * `userId` is 0 because there is no GitHub account behind it and a real id + * would imply one. The access token is empty, which is honest: nothing reads + * it -- the parser clones over public HTTPS and the token is fetched once for + * a display name and never used again. + */ +function singleUserSession(login: string): Session { + return { userId: 0, login, accessToken: "" }; +} + +/** Whether this process is running unauthenticated. One place to ask. */ +export function isSingleUser(): boolean { + return env.FUNCATLAS_SINGLE_USER !== undefined; +} + /** * preHandler that rejects anonymous requests. Applied once to the whole /api * subtree in A4, not repeated per route. + * + * The single-user branch is here rather than in a second hook because this is + * the only function that sets `req.session`. A parallel auth path is how two + * of them drift until one forgets a check. */ export async function requireSession(req: FastifyRequest, reply: FastifyReply) { + const login = env.FUNCATLAS_SINGLE_USER; + if (login !== undefined) { + req.session = singleUserSession(login); + return; + } + const id = sessionIdFrom(req); const session = id === null ? null : await readSession(id); if (session === null) { diff --git a/apps/api/src/auth/single-user.test.ts b/apps/api/src/auth/single-user.test.ts new file mode 100644 index 0000000..0a33039 --- /dev/null +++ b/apps/api/src/auth/single-user.test.ts @@ -0,0 +1,104 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { FastifyInstance } from "fastify"; + +/** + * `FUNCATLAS_SINGLE_USER` removes authentication entirely, so what it does has + * to be pinned rather than assumed. + * + * `env` is parsed once at import, so each case rebuilds the module graph under + * a stubbed environment -- stubbing after the import would leave the + * already-parsed value in place and every assertion would pass against the + * same build. The same technique the deleted dev-login's production test used. + */ +async function buildWith(login: string | undefined): Promise { + vi.resetModules(); + if (login === undefined) { + vi.stubEnv("FUNCATLAS_SINGLE_USER", ""); + } else { + vi.stubEnv("FUNCATLAS_SINGLE_USER", login); + } + + const { buildApp } = await import("../app.js"); + return buildApp(); +} + +/** Each rebuild opens its own Redis connection; without closing it the run + * hangs on an open handle after the last test. */ +async function close(app: FastifyInstance) { + const { redis } = await import("../redis.js"); + await app.close(); + redis.disconnect(); +} + +afterEach(() => { + vi.unstubAllEnvs(); + vi.resetModules(); +}); + +describe("with FUNCATLAS_SINGLE_USER set", () => { + it("answers a gated route without any cookie", async () => { + const app = await buildWith("octocat"); + try { + const res = await app.inject({ method: "GET", url: "/api/repos" }); + + // The gate is the whole point: this exact request is 401 without it, + // which the sibling case below asserts rather than assumes. + expect(res.statusCode).toBe(200); + } finally { + await close(app); + } + }); + + it("does not register the OAuth routes at all", async () => { + const app = await buildWith("octocat"); + try { + // 404, not 401. A handler that exists and refuses tells a prober there + // is something here worth finding credentials for -- the same reasoning + // that shaped the dev-login gate in R30. + for (const url of ["/auth/login", "/auth/callback", "/auth/logout"]) { + const res = await app.inject({ method: "GET", url }); + expect(res.statusCode, url).toBe(404); + } + } finally { + await close(app); + } + }); + + it("tells the web app there is nobody to sign out", async () => { + const app = await buildWith("octocat"); + try { + const res = await app.inject({ method: "GET", url: "/auth/me" }); + + expect(res.statusCode).toBe(200); + expect(res.json()).toMatchObject({ login: "octocat", singleUser: true }); + } finally { + await close(app); + } + }); +}); + +describe("without it", () => { + it("still refuses an anonymous request", async () => { + const app = await buildWith(undefined); + try { + const res = await app.inject({ method: "GET", url: "/api/repos" }); + + expect(res.statusCode).toBe(401); + } finally { + await close(app); + } + }); + + it("still registers the OAuth entry point", async () => { + const app = await buildWith(undefined); + try { + const res = await app.inject({ method: "GET", url: "/auth/login" }); + + // A broken rebuild that registered nothing would pass the 404 assertion + // above for the wrong reason. This is what stops that. + expect(res.statusCode).toBe(302); + } finally { + await close(app); + } + }); +}); diff --git a/apps/api/src/env.test.ts b/apps/api/src/env.test.ts index 084c2b0..c274020 100644 --- a/apps/api/src/env.test.ts +++ b/apps/api/src/env.test.ts @@ -23,16 +23,25 @@ function read(relative: string): string { } /** - * Keys the schema declares with no `.default(...)`, which are therefore - * required at startup. Parsed from the source so adding one to `env.ts` is - * enough to make this test start demanding it. + * Keys the schema declares with neither `.default(...)` nor `.optional()`, + * which are therefore required at startup. Parsed from the source so adding + * one to `env.ts` is enough to make this test start demanding it. + * + * `.optional()` joined `.default(` here when FUNCATLAS_SINGLE_USER and the + * three GitHub OAuth credentials became conditional: they are legitimately + * absent, and the old heuristic demanded them in CI. */ function requiredKeys(): string[] { const source = read("apps/api/src/env.ts"); const body = source.slice(source.indexOf("z.object({"), source.indexOf("export const env")); return [...body.matchAll(/^\s{2}([A-Z][A-Z0-9_]*):\s*([\s\S]*?)(?=^\s{2}[A-Z][A-Z0-9_]*:|^\}\))/gm)] - .filter(([, , definition]) => definition !== undefined && !definition.includes(".default(")) + .filter( + ([, , definition]) => + definition !== undefined && + !definition.includes(".default(") && + !definition.includes(".optional("), + ) .map(([, key]) => key as string); } diff --git a/apps/api/src/env.ts b/apps/api/src/env.ts index 95687dd..f601980 100644 --- a/apps/api/src/env.ts +++ b/apps/api/src/env.ts @@ -25,11 +25,43 @@ const schema = z.object({ REDIS_URL: z.string().url(), QUEUE_NAME: z.string().min(1), - GITHUB_CLIENT_ID: z.string().min(1), - GITHUB_CLIENT_SECRET: z.string().min(1), - GITHUB_REDIRECT_URI: z.string().url(), + // Optional, and enforced below only when there is actually an OAuth flow to + // run. Requiring them unconditionally would mean registering a GitHub OAuth + // app before `docker compose up` did anything, which is exactly the friction + // FUNCATLAS_SINGLE_USER exists to remove. + // + // GITHUB_WEBHOOK_SECRET stays required on both paths: it is a random string + // anyone can generate, not a credential that has to be registered somewhere. + GITHUB_CLIENT_ID: z.string().min(1).optional(), + GITHUB_CLIENT_SECRET: z.string().min(1).optional(), + GITHUB_REDIRECT_URI: z.string().url().optional(), GITHUB_WEBHOOK_SECRET: z.string().min(1), + /** + * Run with no authentication at all, as this GitHub login. + * + * Set, the API registers no /auth/login, /auth/callback or /auth/logout and + * every request resolves to this user. It exists so `docker compose up` + * works without registering a GitHub OAuth app, which was the single + * largest thing standing between a clone and a running stack. + * + * This is not R30 wearing a different hat. R30 was a session-minting + * endpoint that shipped in every deployment behind a NODE_ENV gate, so the + * gate had to be right exactly once. Here there is no endpoint to reach and + * no gate to be wrong: the instance is unauthenticated because a human wrote + * their username into their own .env. What it is not is safe to expose -- + * the process binds 0.0.0.0 (it must, inside a container) and cannot know + * what sits in front of it, so compose publishes on 127.0.0.1 and the + * server warns on every start. R39. + */ + // Blank counts as unset. `FUNCATLAS_SINGLE_USER=` in a .env is how someone + // turns this off, and a bare `.min(1)` answers that with a Zod stack trace + // about a string length instead of just starting normally. + FUNCATLAS_SINGLE_USER: z.preprocess( + (v) => (typeof v === "string" && v.trim() === "" ? undefined : v), + z.string().min(1).optional(), + ), + SESSION_SECRET: z.string().min(16), SESSION_COOKIE_NAME: z.string().min(1).default("funcatlas_session"), SESSION_TTL: z.coerce.number().int().positive().default(604800), @@ -43,4 +75,21 @@ const schema = z.object({ PARSE_CONCURRENCY: z.coerce.number().int().positive().default(2), }); -export const env = schema.parse(process.env); +const parsed = schema.parse(process.env); + +// Checked here rather than at the first request: a missing client id should +// stop the process on the line that starts it, not surface as a 500 the first +// time somebody clicks sign in. +if (parsed.FUNCATLAS_SINGLE_USER === undefined) { + const missing = (["GITHUB_CLIENT_ID", "GITHUB_CLIENT_SECRET", "GITHUB_REDIRECT_URI"] as const) + .filter((key) => parsed[key] === undefined); + + if (missing.length > 0) { + throw new Error( + `Missing ${missing.join(", ")}. Either register a GitHub OAuth app and fill them in, ` + + "or set FUNCATLAS_SINGLE_USER to run with no authentication (see .env.example).", + ); + } +} + +export const env = parsed; diff --git a/apps/web/src/App.test.tsx b/apps/web/src/App.test.tsx index 2010b45..345a489 100644 --- a/apps/web/src/App.test.tsx +++ b/apps/web/src/App.test.tsx @@ -66,7 +66,7 @@ describe("routing", () => { it("shows the canvas route for anything else", async () => { window.history.replaceState(null, "", APP_ROUTE); - mocked.me.mockResolvedValue({ userId: 7, login: "octocat" }); + mocked.me.mockResolvedValue({ userId: 7, login: "octocat", singleUser: false }); renderApp(); @@ -95,7 +95,7 @@ describe("session states", () => { }); it("shows the explorer when signed in", async () => { - mocked.me.mockResolvedValue({ userId: 7, login: "octocat" }); + mocked.me.mockResolvedValue({ userId: 7, login: "octocat", singleUser: false }); renderApp(); @@ -146,7 +146,7 @@ describe("signing in and out", () => { }); it("returns to the sign-in card after signing out", async () => { - mocked.me.mockResolvedValue({ userId: 7, login: "octocat" }); + mocked.me.mockResolvedValue({ userId: 7, login: "octocat", singleUser: false }); mocked.logout.mockResolvedValue(undefined); renderApp(); @@ -159,7 +159,7 @@ describe("signing in and out", () => { describe("the file tree panel", () => { it("labels the toggle by what it does, and survives being used", async () => { - mocked.me.mockResolvedValue({ userId: 7, login: "octocat" }); + mocked.me.mockResolvedValue({ userId: 7, login: "octocat", singleUser: false }); renderApp(); @@ -175,7 +175,7 @@ describe("the file tree panel", () => { }); it("gives the panel a keyboard-reachable resize handle", async () => { - mocked.me.mockResolvedValue({ userId: 7, login: "octocat" }); + mocked.me.mockResolvedValue({ userId: 7, login: "octocat", singleUser: false }); renderApp(); await screen.findByText("octocat"); diff --git a/apps/web/src/components/AppHeader.tsx b/apps/web/src/components/AppHeader.tsx index f9469c5..d867574 100644 --- a/apps/web/src/components/AppHeader.tsx +++ b/apps/web/src/components/AppHeader.tsx @@ -48,14 +48,19 @@ export function AppHeader({
{user.login} - + {/* No sign-out under FUNCATLAS_SINGLE_USER: the server registers no + /auth/logout, so the button would POST at a 404, and there is no + session to end in the first place. */} + {user.singleUser ? null : ( + + )}
); diff --git a/packages/shared/src/types.ts b/packages/shared/src/types.ts index bde8707..3655ecb 100644 --- a/packages/shared/src/types.ts +++ b/packages/shared/src/types.ts @@ -141,4 +141,11 @@ export interface TraversalResponse { export interface SessionUser { userId: number; login: string; + /** + * True when the API is running under FUNCATLAS_SINGLE_USER: there is no + * OAuth, and no sign-out either, because there is no session to end. The web + * app reads this to hide a Sign out button that would POST to a route the + * server did not register. + */ + singleUser: boolean; } From ed013fa21b0735e0174d971d0206ed88c8f867df Mon Sep 17 00:00:00 2001 From: Aditya Rana Date: Sat, 29 Aug 2026 23:50:59 +0530 Subject: [PATCH 5/6] make a freshly copied .env actually start Three faults, all found by cloning into /tmp and running the thing rather than by reading it. `.env.example` shipped `GITHUB_CLIENT_ID=`, and the Makefile sources that file with `set -a && . ./.env`. A shell reads `<` as a redirection, so every env-using target died with "newline unexpected" the moment `make setup` had copied it. The placeholders are empty values now, which is also what the schema wants, and the guidance moved into comments. `make setup` sourced .env to create the test database, which it does not need: the credentials there are compose's own. The one that actually mattered: `.optional()` only accepts a *missing* variable, and an empty value is a present empty string, so `.min(1)` rejected it. `GITHUB_CLIENT_ID=` -- the state a fresh .env is in, and the state anyone leaves a key they are not using -- crashed the API on startup complaining about the length of a credential single-user mode was never going to use. I had already fixed exactly this for FUNCATLAS_SINGLE_USER and left the three siblings broken, which is the whole argument for fixing at the shared point: it is now one `blankAsUnset` helper wrapping all four. Postgres and Redis published on 0.0.0.0 while everything else in this file was bound to 127.0.0.1. On a stack whose API runs unauthenticated, an internet-facing Postgres with the password `funcatlas` is the worse hole. They are published at all only because `make start` runs the API natively. Exit test passes from a clean clone with `make start` never run: `make setup` then `docker compose up`, register sindresorhus/p-limit through the API with no cookie, and the worker parses it to ready -- 6 files, 36 functions, commit SHA recorded. The startup warning fires, /auth/me answers without a cookie, and /auth/login is a 404. --- .env.example | 15 ++++++++++----- Makefile | 4 +++- apps/api/src/env.ts | 28 ++++++++++++++++++---------- docker-compose.yml | 7 +++++-- 4 files changed, 36 insertions(+), 18 deletions(-) diff --git a/.env.example b/.env.example index e8ec96d..b6c4014 100644 --- a/.env.example +++ b/.env.example @@ -1,5 +1,10 @@ # Local development environment for funcatlas. -# Copy to .env and fill in values. .env is gitignored; this file is committed. +# Copy to .env and fill in values -- or just run `make setup`, which does it +# and generates the secrets for you. .env is gitignored; this file is committed. +# +# Values are left empty rather than : the Makefile sources this +# file with `set -a && . ./.env`, and a shell reads `<` as a redirection, so a +# placeholder made every env-using target fail with "newline unexpected". # --- App (Fastify API) --- NODE_ENV=development @@ -48,16 +53,16 @@ FUNCATLAS_SINGLE_USER= # --- GitHub OAuth --- # Only needed when FUNCATLAS_SINGLE_USER is blank. Register an OAuth app and # set its callback to GITHUB_REDIRECT_URI below. -GITHUB_CLIENT_ID= -GITHUB_CLIENT_SECRET= +GITHUB_CLIENT_ID= +GITHUB_CLIENT_SECRET= GITHUB_REDIRECT_URI=http://localhost:3000/auth/callback # --- Webhooks --- # HMAC secret for verifying GitHub webhook payloads. -GITHUB_WEBHOOK_SECRET= +GITHUB_WEBHOOK_SECRET= # --- Sessions --- -SESSION_SECRET= +SESSION_SECRET= SESSION_COOKIE_NAME=funcatlas_session SESSION_TTL=604800 diff --git a/Makefile b/Makefile index 3efa544..9c78ab2 100644 --- a/Makefile +++ b/Makefile @@ -28,7 +28,9 @@ setup: ## One-time: write .env, generate secrets, create the test database $(MAKE) up $(MAKE) wait-infra @# Idempotent: the second run finds the database and says so. - $(ENV) && docker compose exec -T postgres psql -U funcatlas -d postgres \ + @# No $(ENV) here: the credentials are compose's own, and sourcing .env + @# before it has been filled in is how this used to fail. + docker compose exec -T postgres psql -U funcatlas -d postgres \ -c "CREATE DATABASE funcatlas_test OWNER funcatlas" 2>/dev/null \ && echo "Created funcatlas_test." || echo "funcatlas_test already exists." @echo "" diff --git a/apps/api/src/env.ts b/apps/api/src/env.ts index f601980..a0f8e15 100644 --- a/apps/api/src/env.ts +++ b/apps/api/src/env.ts @@ -11,6 +11,20 @@ const repoRoot = path.resolve(import.meta.dirname, "../../.."); // A missing .env is not an error -- CI supplies the environment directly. config({ path: path.join(repoRoot, ".env") }); +/** + * An optional key where blank means absent. + * + * `.optional()` alone only accepts a *missing* variable. A key present and + * empty -- which is how `.env.example` ships every value you have not filled + * in, and how anyone turns one off -- is a present empty string, and + * `.min(1)` rejects it. Without this the API refuses to start on a freshly + * copied .env, complaining about the length of a credential it was never + * going to use. + */ +function blankAsUnset(inner: T) { + return z.preprocess((v) => (typeof v === "string" && v.trim() === "" ? undefined : v), inner); +} + // Fail-fast env validation. All keys mirror .env.example. const schema = z.object({ NODE_ENV: z.enum(["development", "production", "test"]).default("development"), @@ -32,9 +46,9 @@ const schema = z.object({ // // GITHUB_WEBHOOK_SECRET stays required on both paths: it is a random string // anyone can generate, not a credential that has to be registered somewhere. - GITHUB_CLIENT_ID: z.string().min(1).optional(), - GITHUB_CLIENT_SECRET: z.string().min(1).optional(), - GITHUB_REDIRECT_URI: z.string().url().optional(), + GITHUB_CLIENT_ID: blankAsUnset(z.string().min(1).optional()), + GITHUB_CLIENT_SECRET: blankAsUnset(z.string().min(1).optional()), + GITHUB_REDIRECT_URI: blankAsUnset(z.string().url().optional()), GITHUB_WEBHOOK_SECRET: z.string().min(1), /** @@ -54,13 +68,7 @@ const schema = z.object({ * what sits in front of it, so compose publishes on 127.0.0.1 and the * server warns on every start. R39. */ - // Blank counts as unset. `FUNCATLAS_SINGLE_USER=` in a .env is how someone - // turns this off, and a bare `.min(1)` answers that with a Zod stack trace - // about a string length instead of just starting normally. - FUNCATLAS_SINGLE_USER: z.preprocess( - (v) => (typeof v === "string" && v.trim() === "" ? undefined : v), - z.string().min(1).optional(), - ), + FUNCATLAS_SINGLE_USER: blankAsUnset(z.string().min(1).optional()), SESSION_SECRET: z.string().min(16), SESSION_COOKIE_NAME: z.string().min(1).default("funcatlas_session"), diff --git a/docker-compose.yml b/docker-compose.yml index 30208d4..525fbde 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -5,8 +5,11 @@ services: POSTGRES_USER: funcatlas POSTGRES_PASSWORD: funcatlas POSTGRES_DB: funcatlas + # Loopback, like everything else here. Published at all only because + # `make start` runs the API and worker natively on the host; a + # compose-only stack reaches Postgres over the internal network. ports: - - "5432:5432" + - "127.0.0.1:5432:5432" volumes: - pgdata:/var/lib/postgresql/data healthcheck: @@ -18,7 +21,7 @@ services: redis: image: redis:7 ports: - - "6379:6379" + - "127.0.0.1:6379:6379" # Sessions live here. Without a volume every restart signs everyone out, # which in development means losing the login on each `make up`. volumes: From ef31ced9dfcea4b085a850af853b44148bebe814 Mon Sep 17 00:00:00 2001 From: Aditya Rana Date: Sat, 29 Aug 2026 23:53:49 +0530 Subject: [PATCH 6/6] document the one-command path The README told contributors `docker compose up` did not work and listed four prerequisites. It is two commands now -- `make setup && docker compose up` -- with Docker, make and openssl the only things needed, and the native path kept for anyone who wants hot reload. The single-user warning is stated where it will be read: in the quickstart, in "what it does not do", and in CLAUDE.md's known gaps. A README that buries "this has no authentication" under a heading nobody scrolls to is not honest just because the sentence exists somewhere. R39 records the residual risk and says plainly that the mitigation is weaker than the plan claimed -- the process binds 0.0.0.0 and cannot check what is in front of it, so the protection is a loopback port map and a human writing the value themselves, not a guard in code. R11 said prod runs everything in compose. That was the intent from Phase 0 and had never been true, which is how eight faults accumulated in a file nobody ran; it is true now and says when it became so. PRD NFR-4 likewise. Two CLAUDE.md gaps close with it: `pnpm start` could never run the API, and compiled test files landed in `apps/api/dist`. --- CLAUDE.md | 11 +++++-- PRD.md | 2 +- README.md | 81 +++++++++++++++++++++++---------------------------- TASKLIST.md | 36 +++++++++++++++++++++++ docs/RISKS.md | 5 ++-- 5 files changed, 84 insertions(+), 51 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 26a8249..ade4122 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -200,9 +200,14 @@ Phase 3b is closed. `TASKLIST.md` is the chunk-level truth; this is what outlive the insert. Write edges for a file whose functions you did not delete and they silently double. - **Public repositories only.** The OAuth scope is `read:user`, and the parser clones over public HTTPS. See R26 for why `repo` was not the answer. -- **`pnpm start` cannot run the API.** `packages/shared` exports point at `./src/*.ts`, so plain - `node dist/index.js` cannot follow them. `pnpm dev` (tsx) works. Fix before containerising. -- **Compiled test files land in `apps/api/dist`.** Harmless locally, wrong in an image. +- **`docker compose up` is the front door, and everything runs under `tsx`.** `packages/shared` + exports point at `./src/*.ts`, so `node dist/index.js` cannot follow them -- the image ships + source and runs tsx, and `pnpm start` does the same. Building shared to `dist` and repointing + `exports` is the cleaner endpoint; it was not done because it puts a build step in front of the + dev loop. `.dockerignore` keeps compiled tests, `node_modules` and `.env` out of every image. +- **`make setup` writes `FUNCATLAS_SINGLE_USER`, so the default stack has no authentication.** + One branch in `requireSession`, no OAuth routes registered, every compose port on `127.0.0.1`, + and a warning on every start. R39 -- read it before assuming this is safe to deploy. - **Resolution limits that are honest, not broken:** barrel re-export chains, default imports, and `tsconfig` path aliases all resolve to `unresolved`. So does every per-language construct in "Per-language extraction limits" — Go's single-type-argument generic call (ambiguous with a diff --git a/PRD.md b/PRD.md index a1ea4c7..0ad1898 100644 --- a/PRD.md +++ b/PRD.md @@ -79,7 +79,7 @@ database, saved canvas layouts, real-time multi-user editing. Rationale for each | NFR-1 | **Performance** — 300-file TypeScript repo parsed and resolved in under 90 s; `functions-for-file` p95 under 150 ms; 5-hop traversal over 10k edges under 500 ms; 60 fps at 2,000 visible nodes. | | NFR-2 | **Security** — no network egress during parse; an untrusted repo cannot read host files; webhooks are replay- and flood-safe; every graph endpoint is session-gated; secrets come from the environment. | | NFR-3 | **Correctness** — re-parsing a renamed or deleted function leaves no orphan edges, and resolution never claims certainty it does not have. | -| NFR-4 | **Operability** — one `docker compose up` brings the whole stack up; `/healthz` on api and parser; structured logs (zap in the parser, pino in the api). | +| NFR-4 | **Operability** — one `docker compose up` brings the whole stack up; `/healthz` on api and parser; structured logs (zap in the parser, pino in the api). **Met on the clone-and-run branch**, verified from a clean clone with `make start` never run. | | NFR-5 | **Maintainability** — shared TypeScript types only in `packages/shared`; one SQL migration source at `services/parser/migrations/`; explicit SQL via sqlx in Go, Drizzle in the API, no full ORM anywhere. | | NFR-6 | **UX** — dark-mode-first with accent tokens; motion that explains rather than decorates; skeleton loading; actionable errors; `prefers-reduced-motion` respected. | diff --git a/README.md b/README.md index 593272b..0cb36ac 100644 --- a/README.md +++ b/README.md @@ -45,21 +45,28 @@ produces *less* reads as one that worked. See [`docs/PARSING_STRATEGY.md`](docs/ ## Running it -### Prerequisites +Two commands. You need **Docker**, plus `make` and `openssl` — both already present on macOS and +any Linux. -- **Node 20+** and **pnpm** (`npm i -g pnpm`) -- **Go 1.24+** with a C toolchain (`gcc`) — tree-sitter uses cgo -- **Docker** and **Docker Compose** — for Postgres and Redis -- **golang-migrate** CLI — or the `migrate/migrate` Docker image, as CI does -- A **GitHub OAuth App** — the app has no other way to know who you are +```bash +git clone https://github.com/ARCoder181105/funcatlas.git +cd funcatlas +make setup # writes .env, generates secrets, creates the test database +docker compose up # then open http://localhost:5173 +``` + +Paste a public repository URL and explore it. `⌘K` finds any function by name. -> `docker compose up` does **not** work yet and is the next piece of work: `apps/web` has no -> Dockerfile, there is no worker service, and the API image cannot start. Until then the four -> prerequisites above are all required. See NFR-4 in [`PRD.md`](PRD.md). +> **`make setup` sets `FUNCATLAS_SINGLE_USER`, which means the API runs with no authentication.** +> That is what lets you skip registering a GitHub OAuth app. Compose publishes every port on +> `127.0.0.1`, so nothing off-box can reach it — but do not expose these ports, and do not run it +> this way on a server. Blank the value in `.env` to use real GitHub sign-in instead +> ([`docs/RISKS.md`](docs/RISKS.md) R39). -### Register a GitHub OAuth App +### With real GitHub sign-in - → **New OAuth App**. The exact values: +Blank `FUNCATLAS_SINGLE_USER` in `.env` and register an OAuth app at + → **New OAuth App**: | Field | Value | |---|---| @@ -67,47 +74,28 @@ produces *less* reads as one that worked. See [`docs/PARSING_STRATEGY.md`](docs/ | Homepage URL | `http://localhost:5173` | | Authorization callback URL | `http://localhost:3000/auth/callback` | -Generate a client secret and keep both values for the next step. The scope requested is -`read:user` and nothing more — funcatlas never writes to your account, and the token is read once -for your username and then never used again. It clones over public HTTPS, which is why only public -repositories work: the only scope that reads private ones is `repo`, and that also grants **write** -access to every private repository you can reach. +Put the client id and secret into `.env` as `GITHUB_CLIENT_ID` and `GITHUB_CLIENT_SECRET`. The +scope requested is `read:user` and nothing more — funcatlas never writes to your account, and the +token is read once for your username and then never used again. It clones over public HTTPS, which +is why only public repositories work: the only scope that reads private ones is `repo`, and that +also grants **write** access to every private repository you can reach. -### Set up and start +### Working on the code -```bash -git clone https://github.com/ARCoder181105/funcatlas.git -cd funcatlas -pnpm install -cp .env.example .env -``` +`docker compose up` has no hot reload. For that, run the services natively — which needs three more +things installed: -Then edit `.env` and fill in four values: - -```bash -GITHUB_CLIENT_ID= -GITHUB_CLIENT_SECRET= -GITHUB_WEBHOOK_SECRET=$(openssl rand -hex 32) # unused locally, but required -SESSION_SECRET=$(openssl rand -hex 32) -``` - -Create the test database once, or `make test` truncates the one you develop against: - -```bash -docker compose up -d postgres redis -docker compose exec postgres psql -U funcatlas -d postgres \ - -c "CREATE DATABASE funcatlas_test OWNER funcatlas;" -``` - -Then bring the whole thing up — infra, migrations, the parser binary, the API, the web app and the -parse worker: +- **Node 24+** and **pnpm** (`npm i -g pnpm`) — pnpm 11 does not run on Node 20 +- **Go 1.25+** with a C toolchain (`gcc`) — tree-sitter uses cgo +- **golang-migrate** CLI — or the `migrate/migrate` Docker image, as CI does ```bash -make start # then open http://localhost:5173 +pnpm install +make start # infra in compose, api + web + worker natively, with watch ``` -Sign in with GitHub, paste a public repository URL, and explore it. `⌘K` finds any function by -name. +Stop the compose worker first if it is running: it consumes the queue the tests assert on, and +`make test` will tell you so rather than failing obscurely. ### Without the app @@ -140,6 +128,9 @@ Stated here rather than discovered later: - **Public repositories only.** See the OAuth section above for why. - **No hosted instance.** You run it. +- **No authentication by default.** `make setup` turns on single-user mode so you can skip + registering an OAuth app. Fine on a laptop behind a loopback-only port map, wrong anywhere else + ([`docs/RISKS.md`](docs/RISKS.md) R39). - **An incremental re-parse still re-parses everything.** The *write* is scoped to changed files; the clone, extract and resolve are not. Resolution is whole-repo, and a partial symbol table would emit a confident edge where the whole repository would correctly say ambiguous diff --git a/TASKLIST.md b/TASKLIST.md index d802094..1463b02 100644 --- a/TASKLIST.md +++ b/TASKLIST.md @@ -184,3 +184,39 @@ available browser window and `resize_window` was ignored, so neither could be ob dist/index.js` against `packages/shared` exports that point at `.ts`; and the `parser` service sets both `network_mode: none` and `depends_on` Postgres health. Do **not** resolve the last by giving the parser a network — that undoes a Phase 1 guarantee. + +--- + +## NFR-4 — `docker compose up` (`clone-and-run`, not a phase) + +The promise in `PRD.md` since Phase 0, never once tested. Eight faults, not the six on record: +the `api` build context and the missing `.dockerignore` turned up on re-reading, and three more +only appeared when the images were actually built and run. + +- [x] **`.dockerignore`.** First and alone: `COPY . .` with no ignore file put `.env` — a real + client secret — into every image layer. +- [x] **API and worker image.** Root build context, `packages/shared` copied, runs under `tsx`. + Go stage builds the parser with cgo; `git` installed, because `clone.go` shells out to it. +- [x] **Worker service.** Nothing consumed the queue: a webhook answered 202 and the graph never + moved. +- [x] **`migrate` service**, gating api and worker on `service_completed_successfully`. +- [x] **Web image** — Vite build behind nginx with a `try_files` SPA fallback. +- [x] **Parser service** — vestigial `depends_on` dropped, isolation kept, `tools` profile so + `docker compose up` does not start it. +- [x] **`make setup`** and **`FUNCATLAS_SINGLE_USER`**, so a clone needs no GitHub OAuth app. +- [x] **Every port on `127.0.0.1`**, Postgres and Redis included. + +**Exit test passed** from a clean clone in /tmp with `make start` never run: `make setup`, +`docker compose up`, register `sindresorhus/p-limit` through the API with no cookie, worker parses +it to `ready` — 6 files, 36 functions, commit SHA recorded. + +Found by running it rather than reading it: pnpm 11 will not start on Node 20; `pnpm run` verifies +the workspace before executing, so a build stage needs every manifest; `.env`'s `localhost` means +the container itself; `` values broke every `set -a && . ./.env`; and `.optional()` +rejects a present-but-empty variable, which is the state every fresh `.env` is in. + +## Next + +- [ ] **NFR-1 — the performance targets have never been measured.** `honojs/hono` is the fixed + benchmark (R3); the timings are not. +- [ ] **FR-7 — several file cards at once.** Cut deliberately in 3b (`UI_GUIDE.md` §6). diff --git a/docs/RISKS.md b/docs/RISKS.md index c54ff2e..77f5b4f 100644 --- a/docs/RISKS.md +++ b/docs/RISKS.md @@ -26,7 +26,7 @@ it was. | **R29** | Every repository defaulting to `master` was recorded as being on `main`. | The parser's `--branch` defaulted to `"main"` while a shallow clone checks out whatever the remote's default actually is. Branch and commit are now read off the checkout with `git rev-parse`; the caller hands over a URL and never clones, so it could not have known either. | | **R9** | The Go parser cannot import `packages/shared`, so it has no shared types. | The parser keeps **Go-native IR types** in `internal/ir/ir.go`, mirroring the schema by hand. The duplication is accepted; generating Go structs from the migration was judged more machinery than the drift is worth at four tables. | | **R10** | Single migration source, so the parser and API can't drift. | **`services/parser/migrations/`**, plain SQL via golang-migrate. Both the Go writer and the TypeScript reader use it. CI runs the migrations against a live Postgres on every PR. | -| **R11** | Dev and prod modes differ — prod is `docker compose up`, dev needs HMR and watch. | Both documented in `DEVELOPMENT.md`. Dev runs infra in compose and the three services natively; prod runs everything in compose. | +| **R11** | Dev and prod modes differ — prod is `docker compose up`, dev needs HMR and watch. | Both documented in `DEVELOPMENT.md`. Dev runs infra in compose and the three services natively; prod runs everything in compose. **True as of the clone-and-run branch** — it had been the intent since Phase 0 and was never tested, which is how eight separate faults accumulated in a file nobody ran (NFR-4). | | **R16** | Grammar versions drift and break parsing silently. | Pinned in `go.mod`: `tree-sitter/go-tree-sitter` **v0.25.0**, `tree-sitter/tree-sitter-typescript` **v0.23.2**. The golden extraction tests fail loudly if a bump changes node kinds. | | **R17** | CI needs Docker for the integration tests and the migration check. | `ubuntu-latest` runners include Docker. `.github/workflows/go-ci.yml` runs a Postgres service container and a `migrate/migrate` container; the Go tests connect to it via `DATABASE_URL`. See R22. | | **R27** | Registration runs the parser inline, so a large repository holds an HTTP request open for the length of the parse. | **Closed in Phase 4.** `POST /api/repos` writes the row, enqueues, and answers 202. The row has to exist before the job does, or a worker picks it up with nothing to mark as parsing -- which reversed who owns the insert, since the parser used to. `repos.parse_status` is how the client follows a parse it is no longer waiting on. | @@ -62,11 +62,12 @@ Nothing outstanding -- R19 through R22 and R26 through R29 all closed; see Decid | **R36** | **A test can measure the boundary with the function that defines it.** The Phase 5 exit test first compared `ResolutionGroup(caller)` with `ResolutionGroup(callee)` -- and passed with `ResolutionGroup` returning a constant, because both sides moved together. | Found by breaking the function on purpose and watching nothing fail. The assertions now compare language names from a literal set written in the test file. The same shape is worth suspecting anywhere a test derives its expectation from the code under test -- and the fixture had a second version of the problem: every file defined `helper`, so ambiguity answered `unresolved` whether the partition worked or not. It now also carries names defined in exactly one language. | | **R37** | **Six languages share one resolver, and only TypeScript's rules are modelled.** Same-file resolution is language-agnostic and stays `exact`; everything else outside the ECMAScript family is `name_match` or `unresolved`. | Deliberate, and the reason Phase 5 was scoped to extraction (`PLAN.md`). The risk is drift: a later change that widens a rule for one language widens it for all six, because there is one `resolve`. The guard is `TestPolyglot_CrossFileIsNeverExactOutsideECMAScript`, which fails the moment any non-ECMAScript language starts answering confidently across files. | -### Found while planning NFR-4 +### Found while closing NFR-4 | | Risk | Notes | |---|---|---| | **R38** | **The parser sandbox is real, tested, and not used by the product.** `docs/SECURITY.md` ticked "clone/parse runs in an isolated container" and "parser has no outbound network access" from Phase 1 onward, `CLAUDE.md` said isolation "was built in Phase 1, not deferred", and the landing page told readers a repository is cloned into a sandbox with no network and no capabilities. All true of `make parser-isolated` and of nothing else: `repos/register.ts` runs the parser with `execFile(env.PARSER_BIN, ...)` from the queue worker, so on `make start` and in any composed stack it is a plain child process carrying the worker's network, filesystem and user. | Found by reading the spawn rather than the checklist. **The claim is corrected rather than the code**, because both fixes cost more than the gap: shelling out to `docker run` needs the Docker socket mounted into the worker, which grants root-equivalent host control — a worse property than the one it buys — and namespaces/seccomp/bubblewrap avoids that but is Linux-only and a project of its own. What survives is enforced everywhere, because it lives *in* the parser rather than around it: symlinks hard-fail, size/count/depth caps, a `--depth 1` clone with credential prompts disabled, no repo scripts executed, and the clone removed on the failure path too. Two lessons worth more than the fix. A checkbox ticked against a harness says nothing about the product — the harness passes `--no-deps`, which is also why nobody hit the `parser` service's `network_mode: none` / `depends_on` contradiction. And the deferral in the TOCTOU checklist item had quietly leaned on this claim inside its own justification, so one false statement had already propagated into a second decision. | +| **R39** | **`FUNCATLAS_SINGLE_USER` makes the instance unauthenticated, and nothing in the process can stop it being exposed.** Set, the API registers no OAuth routes and resolves every request to that user. `make setup` writes it by default, so the common case is a stack with no authentication at all. | **Accepted, and the mitigation is weaker than it first looked.** The plan said the server would refuse to start unless bound to loopback; it binds `0.0.0.0` and must, or Docker's port mapping cannot reach it, and a process cannot know what sits in front of it. So the protection is arrangement, not code: `make setup` writes the value into the user's own `.env` and prints what it did, compose publishes every port on `127.0.0.1` including Postgres and Redis, and the API logs a warning naming this risk on every start. Deliberately not R30's shape -- there is no endpoint to reach and no gate to be wrong, only an instance a human configured as open. Blank the value in `.env` to get real GitHub sign-in back. Revisit if this is ever deployed anywhere but a laptop. | ---