# Alpha release checklist Pre-release audit, 2026-09-20. Work through the sections in order: blockers first, then should-fix, then minor. Tick items as they land. ## Baseline at time of audit - `vitest run` (root): 34 files, 417 tests, all pass - `@flagraft/admin-ui`: 70 files, 664 tests, all pass - `pnpm typecheck:all`: clean across root, `admin-ui`, `sdk-js` - `eslint .`: no issues - No `TODO` / `FIXME` / `HACK` markers anywhere in `src`, `packages`, `tests`, `e2e`, `docs` Not verified in this audit: the Playwright e2e suite was not run, the Docker image was not built or booted, and the admin UI and the Unleash import adapter were not deep-reviewed (their tests pass). --- ## Blockers ### 1. Rate limiter counts the reverse proxy as one client `src/server.ts:60`, `src/modules/client/client.routes.ts:31` `Fastify({...})` never sets `trustProxy`, and `@fastify/rate-limit` keys on `request.ip` by default. Behind nginx, traefik or any load balancer every SDK instance in the fleet shares a single 100/min bucket and starts getting 429s. Fixed with a `TRUST_PROXY` env var, off by default, passed to Fastify's `trustProxy`. It accepts `true`, `false`, a hop count, or a comma-separated list of proxy addresses and CIDR ranges. Off is the right default in both directions: with no proxy in front, trusting `X-Forwarded-For` lets any caller invent an IP per request and bypass the limit entirely. Keying the limiter on the API key instead of the IP was considered and rejected: one key belongs to a whole application, so per-key limiting collapses an entire server fleet into one bucket. Per-IP is the right unit; it just needed the real IP. (Corrected under item 4: the original reasoning here said a client key "is embedded in frontend bundles". It is not -- the SDK is server-side. The conclusion is unchanged.) - [x] Fixed -- `TRUST_PROXY` in `src/config.ts`, applied in `src/server.ts` - [x] Documented in the README config table, with a "Running behind a reverse proxy" section - [x] Parsing covered by tests in `tests/config.test.ts` ### 2. Login has no rate limit and no lockout `src/modules/auth/auth.routes.ts:13` The rate-limit plugin is registered inside `clientRoutes`, so Fastify encapsulation confines it to `/api/v1/client/*`. `/admin/auth/login` is `skipAuth` and completely unthrottled. argon2 is deliberately expensive, so this is both a credential-stuffing surface and a CPU-exhaustion DoS reachable without any credential. Fixed with a separate, much tighter limit (`AUTH_RATE_LIMIT_MAX`, default 20 per 15 minutes per IP) registered with `global: false` and opted into per route, so logout and `/public/workspace` stay open -- the login screen fetches the latter on every page load. Scope was wider than the finding said. `/public/invite/:token/accept` also runs argon2 on a guessable token, and `/public/invite/:token` allows invite enumeration, so both are throttled too. Fixing login alone would have left the identical hole next door. Keying on email as well as IP was rejected: with a rate limit rather than a lockout, an email key lets anyone lock a colleague out by guessing at their address. **This uncovered a pre-existing bug.** `@fastify/rate-limit` signals a breach by throwing the plain object its `errorResponseBuilder` returns. That matches no error class, so it fell through `toResponse` to the catch-all: every rate limit breach, including the client evaluation one the README documents as a `429`, was answered with `500 Internal error` and logged as a server fault. The error handler now keeps the status of any thrown 4xx, while a 5xx still refuses to describe itself. Regression tests at both the unit and integration level. Account lockout stays out of alpha -- see the roadmap entry for why, along with the residual distributed-attack gap. - [x] Fixed -- login and both invite routes - [x] Pre-existing 500-instead-of-429 bug fixed in `src/plugins/errorHandler.ts` - [x] Documented in the README config table and the rate limiting section - [x] Lockout and the per-IP gap recorded in `docs/ROADMAP.md` ### 3. The Docker image cannot migrate itself, and there are no deployment docs `Dockerfile`, `README.md` The second stage installs prod dependencies only and copies `dist`, so the image has no `drizzle-kit`, no `src/db/migrations` and no `drizzle.config.ts`. Anyone running the image has no way to run `pnpm db:migrate`. The image also never builds or serves the admin UI. The README has sixteen headings and none of them is Deployment, Docker or Production. For a self-hosted product this is the most important missing document. Fix: decide how migrations run in a container (bundle them and migrate on boot, or ship a separate migrate entrypoint), decide how the admin UI is served, and write the deployment section. Both decisions taken, both matching what Unleash does (verified against the running `unleashorg/unleash-server:6.5.2`: one container serves the UI at `/` and the API on the same origin, and `db-migrate` ships as a runtime dependency). Migrations run at startup unless `RUN_MIGRATIONS=false`, with `node dist/db/migrate.cjs` as the standalone path. The migrations folder is copied to the same path it has in the repo, so the migrator needs no config. The admin UI is served by the server from `/public`, with a SPA fallback so a refresh on `/flags/...` does not 404. Same origin by construction, which is why **item 4 is now mostly resolved** -- no CORS, and `SameSite=Strict` works untouched. Auth now guards `/api/` rather than exempting `/docs`, and a test pins the route surface outside `/api/` so making something public stays deliberate. Verified by building the image and running it against a genuinely empty database: migrations applied, first-boot seed ran, container healthy, UI served, deep links fell back correctly, API 404s stayed JSON, and a real login round trip set and accepted the session cookie. **Two bugs found by actually building it**, neither visible from the source: - `@fastify/static@8` requires Fastify 5; this project is on 4.29.1. It never failed locally because the plugin short-circuits when no `public/` exists. Pinned to `^7`. - `packages/admin-ui` imports `@heroui/styles` in `src/styles/index.css` without declaring it. It resolved only through pnpm's hidden hoisted store, so the UI build worked on the host and in CI but failed in a clean install. Now a declared dependency. - [x] Migrations reachable from the image, applied on boot, overridable - [x] Admin UI served by the server, one container - [x] README deployment section -- docker run, Compose, migrations, proxy, prod checklist - [x] Covered by `tests/plugins/staticUi.test.ts` and a route-surface guard ### 4. Cross-origin admin UI is silently broken in production `src/server.ts:66`, `src/modules/auth/session.ts:8` `origin: NODE_ENV === 'production' ? false : true` combined with `sameSite: 'strict'` on the session cookie means the UI must be same-origin with the API in production. But `VITE_API_URL` is documented as "set it to your own domain when self-hosting", which implies the opposite. In development the Vite proxy masks this entirely, so it will not surface until someone deploys. Decided: **same origin only, no `CORS_ORIGIN`.** Item 3 already made the server serve the UI, so the supported path is same-origin by construction and the bug as written can no longer reach anyone using the image. The reason there is no cross-origin mode is stronger than "we did not get to it". Nothing legitimate calls this API cross-origin: the admin UI is same-origin, and the SDK is server-side -- its README says never to construct a client in browser code -- so browser apps proxy evaluation through their own backend and never talk to Flagraft directly. `origin: false` in production was therefore already correct rather than accidental; it is now commented as a decision so nobody "fixes" it. Failure is made loud instead of silent: the UI logs a console error when `VITE_API_URL` resolves to an origin other than the page's, naming the dropped cookie. That was the whole trap -- login succeeds, every later call 401s, and it reads like a login bug. **This item corrected a factual error made earlier in the audit.** Three places claimed a client key "ships inside frontend bundles" and is effectively public. The SDK documents the opposite. `SECURITY.md` said finding a client key in a published bundle was expected; it now says that is a real report. The conclusions in items 1 and 7 survive, but their reasoning was wrong and is corrected below. - [x] Decided and implemented -- same origin only, cross-origin warned about at runtime - [x] Documented -- "Same origin is the only supported topology" in the README - [x] `SECURITY.md` corrected on client key exposure - [x] Covered by `packages/admin-ui/src/lib/__tests__/crossOrigin.test.ts` ### 5. README config table is missing required variables `README.md:184` Present in `.env.example`, absent from the table the README presents as the config reference: `JWT_SECRET` (the server refuses to boot without it), `DEFAULT_ADMIN_EMAIL`, `DEFAULT_ADMIN_PASSWORD`, `DEFAULT_ADMIN_NAME`, `DEFAULT_PROJECT_NAME`, `DEFAULT_PROJECT_SLUG`, every `SMTP_*` variable, and `APP_BASE_URL`. The single flat table is now grouped into Core, Caching and rate limiting, First-boot seed, and Email, because 22 rows in one block is poor reference material. Checked mechanically against the zod schema: all 22 variables are documented, none invented, and `.env.example` carries the same set. Two descriptions were wrong on a first pass and were corrected against the code rather than `.env.example`: `SMTP_FROM` falls back to `SMTP_USER` and then to `no-reply@flagraft.local`, and `APP_BASE_URL` falls back to the origin of the request that created the invite. - [x] Table completed ### 6. README contradicts the code on environment count `README.md:38` vs `src/limits.ts:15` The README says "Each project can have any number of environments". The code caps it at `MAX_ENVIRONMENTS_PER_PROJECT = 3`, enforced in `src/modules/environments/environment.service.ts:17` and surfaced in the UI. Fix: correct the README, or raise the cap. This is front-page copy, so it should not go out wrong. - [x] Resolved -- README now says "up to three environments" --- ## Should fix ### 7. ReDoS on the evaluation hot path `src/modules/client/evaluate.ts:22` `safeRegexTest` catches syntax errors but not catastrophic backtracking. The pattern is admin-authored, but the value it is tested against is caller-supplied. One crafted context value against a pattern like `(a+)+$` hangs the event loop for every evaluation request. (Corrected under item 4: this originally called a client key "effectively public", which overstated the deliberate-attack risk -- a caller needs a client key, and those are server-side. The accidental case stands on its own and was always the stronger argument: `^(\w+\s?)*$` is a pattern a careful admin writes by hand, and it takes the flag server down for everyone.) Fixed by validating the pattern at write time in `constraintError`, which is the single choke point both the strategy endpoint and the flag importer already go through. A bad pattern now comes back as a `400` the admin can act on, instead of surfacing as an outage later. The evaluation path is untouched. Two corrections to the audit's own advice, found while implementing: - **Length caps do not fix this.** Backtracking is exponential in the input length, so `(a+)+$` (7 characters) against a 40-character value is already hours of CPU. A cap short enough to be safe is too short to be useful. - **`redos-detector` was the wrong library**, despite being the obvious pick. It models `.test()`'s unanchored scan, so it flags ordinary patterns like `[a-z]+@[a-z]+` and `.*@acme\.com$` as unsafe, and `maxScore` cannot tune that away because the score comes back infinite. `recheck` does report the exponential/polynomial split but ships 5.8MB plus per-platform native binaries, which contradicts the air-gapped claim in the README. `safe-regex` was measured against a corpus of 12 ordinary and 8 catastrophic patterns: 0 false positives, 7 of 8 caught. The miss is `^(a|a)*$`, ambiguous alternation at star height 1, which has to be written deliberately. Recorded in the roadmap's known limitations rather than chased. - [x] Fixed -- `safe-regex` check in `src/modules/strategies/constraint-rules.ts` - [x] Covered by unit tests over both corpora, plus an integration test for the 400 - [x] Documented in `docs/API.md`, with the residual gap in `docs/ROADMAP.md` ### 8. 1MB default body limit on import No `bodyLimit` is set anywhere, so Fastify's 1MB default applies. A large Unleash export fails with a raw `FST_ERR_CTP_BODY_TOO_LARGE` rather than a useful message. Worse, that error fell through the error handler's catch-all and came back as `500 Internal error`, which reads as a server bug when the real answer is "your file is too big". - [x] `MAX_IMPORT_BODY_BYTES` (10MB) in `src/limits.ts`, applied to both import routes - [x] `FST_ERR_CTP_BODY_TOO_LARGE` now maps to a 413 `PayloadTooLarge`, with a test ### 9. Docker image runs as root and has no HEALTHCHECK `Dockerfile` No `USER` directive, and `/ready` exists but nothing is wired to it. Verified by building the image and booting it: the process runs as uid 1000, and the probe honours `PORT` rather than assuming 3000. busybox `wget -q --spider` is already in `node:20-alpine` and exits non-zero on both a 503 and a refused connection, so no extra tooling was needed. Note the boot-time case is not covered by the healthcheck: an unreachable database makes the server exit 1 during the onReady seed rather than come up unhealthy. The healthcheck is for a database that dies after boot, where the server stays up and `/ready` starts answering 503. A restart policy covers the other half. - [x] Non-root user (`USER node`) - [x] `HEALTHCHECK` against `/ready` ### 10. Playwright e2e is orphaned `playwright.config.ts`, `e2e/auth.spec.ts`, `package.json` `@playwright/test` is a devDependency and the config and spec both exist, but there is no `test:e2e` script and no CI job. The suite has never run in CI. Wired into CI as an `e2e` job. Running the suite for the first time turned it from a wiring task into a repair job. **The suite was 70% broken** -- 17 of 24 failing, all from the same cause: the login field's label was renamed `Work email` to `Email` during the HeroUI migration and the spec was never updated. Two more tests covered features that no longer exist at all (Google/SAML SSO buttons, the brand-panel stats row); both were deleted rather than rewritten. The seven copies of the sign-in preamble are now one `signIn` helper, so the next rename is a one-line fix. 22 tests, all passing. **CI runs it against the built server, not the Vite dev proxy.** Since item 3 the server bundles the UI, and item 4 made same-origin the only supported topology, so the dev proxy no longer resembles what ships. `E2E_BASE_URL` picks the target and defaults to the dev server for local work. Two things worth recording: - The audit's port-3000 assumption was wrong on this machine: an unrelated Lago API answers there, and it serves `/health` too, so a naive check looks healthy while every Flagraft route 404s. `E2E_BASE_URL` is the escape hatch. - **The suite trips the login rate limit from item 2.** It signs in on nearly every test, so one clean run fits under 20 per 15 minutes but a rerun or a retry gets `429`. CI and the local instructions set `AUTH_RATE_LIMIT_MAX=1000`. Verified by running twice back to back. - [x] `pnpm test:e2e` script added - [x] Spec repaired -- 22 passing against the built server - [x] CI job with Postgres, a build, a boot wait, and report upload on failure - [x] Both local paths documented in `CONTRIBUTING.md` ### 11. Missing open-source furniture `.github/` contains only `workflows/`. No `CONTRIBUTING.md`, `SECURITY.md`, `CHANGELOG.md` or issue templates. There is also no release or publish workflow, so `@flagraft/sdk` at `0.0.1` has no path to npm. Decision taken: the SDK **is** in scope, published under the `alpha` dist-tag so `npm i @flagraft/sdk@alpha` works while nothing resolves as `latest`. The README already advertises the package, so a 404 during the alpha was the worse option. `SECURITY.md` points at GitHub private vulnerability reporting rather than an email address, because no security contact exists yet and inventing one would be worse than none. **Enable it in repo settings** (Settings, Security, Private vulnerability reporting) or the link 404s. The Discussions link in the issue template config needs Discussions enabled too, or it should be removed. Releases are cut by hand -- no publish workflow, by decision. The steps and the two rules the tooling cannot enforce (never tag a prerelease `latest`, always `--access public`) are written up under "Releasing" in `CONTRIBUTING.md`. The package stays ready to publish: version `0.1.0-alpha.0` and `publishConfig.access: public`. `--provenance` is not part of the manual path -- it needs a CI environment with OIDC and fails from a laptop. `SECURITY.md` points at GitHub private vulnerability reporting rather than an email address, because no security contact exists yet and inventing one would be worse than none. **Enable it in repo settings** (Settings, Security, Private vulnerability reporting) or the link 404s. The Discussions link in the issue template config needs Discussions enabled too, or it should be removed. - [x] CONTRIBUTING.md -- real setup, the two-database test gotcha, house style - [x] SECURITY.md - [x] CHANGELOG.md -- Keep a Changelog, seeded with the 0.1.0-alpha.0 feature set - [x] Issue templates -- bug, feature, plus config routing security elsewhere - [x] SDK publish decision -- alpha dist-tag, published by hand - [x] Release workflow dropped; manual steps documented in `CONTRIBUTING.md` - [ ] Enable private vulnerability reporting and Discussions in repo settings - [ ] Create the `@flagraft` npm org (still needed to publish, by hand or not) --- ## Minor ### 12. `packages/mcp/` is a local orphan Contains only `dist/` and `node_modules/`, no `package.json`, untracked by git, but it matches the `packages/*` workspace glob. Delete it locally. - [x] Removed ### 13. Session cookie reads `process.env` directly `src/modules/auth/session.ts:8` `SESSION_COOKIE_OPTS.secure` reads `process.env.NODE_ENV` at module load instead of the validated config. It works, but it is the one config read that bypasses `loadConfig`, which makes it untestable and inconsistent with everything else. Now a `sessionCookieOpts(config)` function reading `config.NODE_ENV`. The exported constant was dropped rather than kept: nothing outside `session.ts` imported it. Being testable was the point, so the regression guard asserts the `Secure` flag follows the config even when `process.env.NODE_ENV` disagrees -- that test fails against the old module-load read. - [x] Moved onto the config object - [x] Covered by `tests/unit/session.test.ts` ### 14. Two migrations share the `0002` prefix `src/db/migrations/0002_same_misty_knight.sql`, `src/db/migrations/0002_white_paibok.sql` Drizzle orders by the journal, so this is harmless today. It is only confusing to read. Renaming a migration that has already been applied breaks Drizzle's hash check, so the correct answer is to leave it. - [x] Deliberately left alone