Repository navigation
Fix/docker stale postmaster pid - #26
Closed
akosidencio wants to merge 25 commits into
Closed
akosidencio wants to merge 25 commits into
akosidencio wants to merge 25 commits into
Conversation
…imits The rate limiter believed the first X-Forwarded-For entry from any peer, so any client could pick its own rate-limit identity by sending the header, and one 60/min bucket covered every route. - New client_ip module: the TCP peer is the client unless it is listed in TRUSTED_PROXIES (IPs/CIDRs). From a trusted peer the X-Forwarded-For chain is walked from the right, skipping trusted hops; malformed or non-ASCII input falls back to the nearest trusted address, never to a client-chosen one. The walk is bounded. Invalid TRUSTED_PROXIES refuses startup. - Separate budgets for auth/setup, API, SDK snapshot, SSE connect and ingest (RATE_LIMIT_*_PER_MINUTE, 0 disables a class). /health is exempt. Only opening an SSE stream is counted, never events on it. - IPv6 clients are keyed by /64; idle entries are evicted every minute so limiter state stays bounded by recently active clients. - 429s carry Retry-After and a JSON body the dashboard login already reads. - Login lockout records and SDK health use the resolved client IP. Tests: resolver and limiter unit tests (spoofed headers, trusted, untrusted and multi-hop chains, malformed input, independent budgets, eviction, 429 shape, health exemption) and tests/rate_limit.rs against the real binary. The integration suite now marks 127.0.0.1 as a trusted proxy, which its X-Forwarded-For scenarios rely on.
… token The setup-complete check ran before the transaction that created the admin, so concurrent requests could all pass it, and the admin upsert (ON CONFLICT DO UPDATE) let a later racer overwrite the winner's password and role. Anyone who reached a fresh installation first could also claim it. - setup_complete takes pg_advisory_xact_lock and re-reads setup_complete under the lock, then creates the admin, workspace name, project name and completion flag in one transaction. Exactly one request wins, across any number of instances; the rest get 404. Any failure rolls everything back. - The admin upsert only claims a pre-password-auth row (password_hash IS NULL); an account that has a password is never overwritten (409). - /api/setup/key and /api/setup/complete require X-Checkgate-Setup-Token (header only). The token is SETUP_TOKEN (min. 16 chars, else startup fails) or, when unset, a random per-instance token written to SETUP_TOKEN_FILE (0600) with only its SHA-256 stored in settings, so every instance accepts it. The token is never logged. Completing setup revokes all tokens in the same transaction; stale token files are removed on the next start. - Dashboard wizard asks for the token on its first step and validates it before continuing; the token stays in memory only. - The image writes its token to /var/lib/checkgate/setup-token. Tests: tests/setup_ownership.rs (2 and 12 concurrent requests, concurrent requests on two instances, missing/wrong/query-string tokens, repeated setup, login after setup, injected failure rolls back and retry succeeds, generated tokens shared across instances and absent from logs, password accounts not overwritten, short token refused). The existing suites, the Node SDK e2e and the screenshot fixture pass the token.
GET /api/health/connections returned every SSE connection on the instance, with full client IPs, to any authenticated user or token. - Workspace admins keep installation-wide visibility and full addresses. - Everyone else (sessions and PATs of either scope, acting as their user) sees only connections of environments in projects they belong to, with client_ip reduced to its /24 (IPv4) or /48 (IPv6) network. - Connections without an environment (dashboard sessions, legacy SDK_KEY) are admin-only. SDK keys stay refused. - Connection records keep the resolved client IpAddr; the module docs, the API reference and the dashboard page state that tracking is per instance. Tests: tests/sdk_health.rs opens simultaneous streams in three environments across two projects plus an admin session, then checks the admin, a viewer of one project, its read-only PAT, an editor of the other project, a user with no membership, SDK keys and anonymous callers; also stream close and membership removal.
The server loaded every environment's flags into one checkgate_core FlagStore keyed only by flag key, so two environments defining the same key overwrote each other in it. Nothing on the server ever read that store: SSE bootstrap, /flags/snapshot and the REST API all query PostgreSQL per environment. It was only startup cost and a latent cross-environment hazard for any future reader. - Drop AppState::store, the startup flag/segment preload, the Redis subscriber's store updates and the handler-side upserts. The subscriber now forwards payloads unchanged, exactly as it did before; SSE handlers still filter them by environment. - The SDK-side FlagStore API in checkgate-core is unchanged; each SDK instance remains scoped to one environment. - Architecture and deployment docs no longer describe a server cache. Tests: tests/environment_isolation.rs defines `checkout` and `gate` in three environments (two projects) with different values and checks REST reads, poll snapshots, SSE bootstrap, update/delete/recreate in one environment, a server restart, and Redis-delivered updates on a second instance — evaluating each environment's SDK store with the core to cover targeting rules, weighted variants and prerequisites.
…loyment modes The entrypoint always started the bundled PostgreSQL and Redis and overwrote DATABASE_URL and REDIS_URL with localhost strings, so the documented "external services" deployment silently ran against a database inside the container. The release binary also fell back to localhost when either URL was missing. - One image, two modes, chosen by CHECKGATE_MODE (all-in-one | external). Unset: external when DATABASE_URL or REDIS_URL is given, else all-in-one. External mode starts no bundled service and never touches the URLs; a missing URL, a URL given with CHECKGATE_MODE=all-in-one, or an unknown mode stops the container with a message naming the problem. The target is logged with credentials redacted. - Release builds refuse to start without DATABASE_URL/REDIS_URL (debug builds keep the localhost defaults with a warning). - Bundled Redis now writes into /var/lib/redis, the volume the compose file and docs already mounted. Dropped the misleading SDK_KEY warning. - scripts/docker-smoke.sh exercises both modes against a built image: first boot, generated setup token (0600, never logged), setup and login, data surviving restart and container re-create, data landing in the external database, no bundled processes in external mode, and eight misconfigurations being refused. Runs in CI and gates the release. - Self-hosting guide restructured: modes, Compose all-in-one, external services, local development, production checklist, env reference, health checks, and upgrade notes (including moving data out of the bundled database for anyone who previously passed external URLs).
…adding 3,757 generated files were tracked: sdks/nodejs/node_modules (3,746 files), the Flutter SDK's .dart_tool/ and its build/ test caches (one 45 MB .dill). Nothing reads them from Git — every CI and release job runs npm install, bun install or (flutter|dart) pub get first, and the Node SDK unit tests mock their two runtime dependencies. Because pub packs from the working tree minus .gitignore, the Flutter caches could also have reached a published package. - Untracked with `git rm --cached`; local copies are left in place. Lockfiles, generated bindings and packaging scripts are kept. - .gitignore: node_modules/ everywhere, coverage/, .dart_tool/, Flutter plugin state and the Flutter SDK build directory. - scripts/check-tracked-artifacts.sh fails when generated or machine-specific paths are tracked; a new CI job runs it.
…ed in 4c393cb Commit 4c393cb ("satisfy clippy needless_range_loop") also picked up edits to NOTICE and README.md that were staged in the working tree at the time and are unrelated to this branch. Restore both files to main's content so this branch carries no attribution changes. (4c393cb also contains the entrypoint mode switch and connection_url check that belong with 4020782; they are kept.)
…ass) - Setup pins its transaction to READ COMMITTED before taking the advisory lock. The re-check of setup_complete relies on a fresh per-statement snapshot; under a REPEATABLE READ database default the snapshot was taken before the lock wait, so a losing request missed the winner's commit and failed with a serialization error (500) instead of 404. New test: concurrent setups against a database whose default isolation is repeatable read (fails without the fix, passes with it). - The entrypoint's logged connection target also drops the query string, since libpq-style URLs can carry ?password=; the smoke test checks it.
…ness Phase 1: security & correctness hardening
cargo-deny and cargo-audit on dev (after PR #21) reported: - RUSTSEC-2026-0382 rustls-webpki: certificate verification may accept distrusted intermediates → 0.103.16 - RUSTSEC-2026-0381 webpki-roots: bundled roots trust distrusted emSign intermediates → 1.0.11 - RUSTSEC-2026-0386 tokio: AsyncFd constructors violate I/O safety (unsound) → 1.54.1 Lockfile-only; `cargo deny check` and `cargo audit` pass.
… at start The Docker smoke job on dev failed in all-in-one mode: pg_ctl could not start because /var/run/postgresql existed only as a by-product of the package install at build time. CI's BuildKit builder (and any runtime that mounts a fresh tmpfs on /run) does not keep it. Reproduced locally with `--tmpfs /run`. The entrypoint now creates the directory on every start. Non-root (Phase 1 follow-up), preserving both deployment modes: - The server runs as `checkgate` (UID/GID 10001) via setpriv; the bundled PostgreSQL and Redis run as their package users. The entrypoint starts as root only in all-in-one mode, to prepare volumes (existing root-owned Redis volumes are re-owned), and drops privileges for every process. - External mode also runs with `--user 10001:10001` (Kubernetes runAsNonRoot), in which case nothing runs as root. All-in-one mode started as non-root is refused with a message pointing at external mode. - su -c string interpolation replaced by setpriv with argv; a `psql | grep -q` check that could fail under pipefail replaced. - HEALTHCHECK probes /health/live; POSTGRES_* defaults live in the entrypoint instead of image ENV (no credential baked into the image); builder apt uses --no-install-recommends. docker-smoke.sh now runs all-in-one with a fresh /run tmpfs and checks the UID of every service process, external mode under --user 10001:10001, the refusal of non-root all-in-one, /health/live and /health/ready, and the dashboard's security headers. An in-place upgrade from the 28eb233 image on the same volumes keeps data and login.
TRUSTED_PROXY_HEADER=forwarded reads the `for=` parameters of the RFC 7239 Forwarded header from trusted proxies, with the same right-to-left walk as X-Forwarded-For. Only one header is ever honoured: a proxy that sets one usually passes the other through from the client unchanged, so reading both would let clients choose their address. The default stays x-forwarded-for. Quoted values, IPv6 in brackets, ports, and parameter order are handled. Obfuscated identifiers (`_hidden`), `unknown`, a missing `for=`, or a value broken by a comma inside quotes end the walk at the nearest trusted proxy, as a malformed X-Forwarded-For hop already does — a broken header can merge clients into a proxy's budget, never mint a fresh identity.
…and SSE admission control Recovery (server/src/sync.rs) - A server that died between committing a change and publishing it left every other server's SSE clients on the old state indefinitely: Redis Pub/Sub is at-most-once and the only holder of the event (and of the RESYNC retry) was the dead process. Each server now keeps an index of the newest revision it relayed per flag, seeded from PostgreSQL, and every SYNC_RECONCILE_INTERVAL_SECS (default 30, ±20 % jitter) compares it with the committed revisions of all flags and tombstones in one snapshot. A committed version still unrelayed SYNC_RECONCILE_GRACE_SECS (default 10) after it was first noticed makes only that environment's clients re-bootstrap. Suspects are judged by the version first noticed, so busy flags do not false-alarm; events from pre-revision servers adopt the committed revision (rolling upgrades). Also repairs changes made in SQL. - A Redis subscription that stays open but delivers nothing was never noticed. The subscriber now pings every REDIS_PING_INTERVAL_SECS (default 10, timeout <= 5 s) and reconnects + resyncs on failure. Subscribed RESP2 connections answer ["pong", ""], so any reply counts. - Events are parsed once per instance (Arc<SyncEvent> broadcast) instead of once per SSE connection. REDIS_CHANNEL selects the channel. - Migration 28: archive/unarchive keeps the flag's revision. It publishes no event and changes nothing SDKs evaluate; a new revision would read as a lost change. Lifecycle (server/src/lifecycle.rs, main.rs) - Graceful shutdown never finished while an SSE stream was open, so every termination with a connected SDK ended in SIGKILL. SIGTERM now fails readiness, waits SHUTDOWN_DRAIN_DELAY_SECS, closes every stream, cancels the subscriber, reconciler, scheduler, resync retry and limiter eviction, bounds in-flight requests by SHUTDOWN_GRACE_PERIOD_SECS (default 20) and closes the pool. - GET /health/live (never dependency-bound) and GET /health/ready (PostgreSQL required, Redis optional via READY_REQUIRES_REDIS, 503 while draining). /health unchanged. Startup subscribes, then seeds the index, then listens. Pool acquire timeout 10 s. Admission control and backpressure (server/src/stream.rs) - SSE_MAX_CONNECTIONS (default 10 000) and SSE_BOOTSTRAP_CONCURRENCY slots (default half the pool) shared by bootstraps and /flags/snapshot, with a bounded wait; refusals answer 503 with a jittered Retry-After. - Resynced streams close at a random point within SSE_RESYNC_SPREAD_MS (default 5 s) instead of all at once. - TCP_USER_TIMEOUT_SECS (default 60, Linux): a client that stops reading leaves its stream unpolled, so no application timer can fire for it; the kernel now aborts such connections. - The connection guard is created before the stream, so a response dropped unpolled no longer leaks its health-tracking entry. Metrics (server/src/metrics.rs): Prometheus/OpenMetrics on METRICS_ADDR (off by default, never on PORT); route templates, status classes and small enums only as labels; enumerated series pre-created at zero. Static files: the dashboard fallback was added after the layers, so it bypassed the security headers; it now gets them, with a CSP that allows the web fonts index.html loads. Dashboard files stay rate-limit-exempt.
…uites Real server processes against one real PostgreSQL and Redis, real HTTP writes and real SSE streams held by simulated SDK clients that follow the shipped SDKs' revision, staging and reconnect/poll-fallback rules (common/sdk.rs). Faults come from per-instance TCP proxies (common/chaos.rs: refuse, blackhole, strand existing connections, sever) and process kills; each cluster (common/cluster.rs) gets its own database and Redis channel, so the suites run in parallel. Every scenario asserts convergence to the state committed in PostgreSQL within a bounded deadline, records the measured recovery window, and prints/saves instance logs on failure (CHECKGATE_TEST_ARTIFACTS). reliability.rs covers propagation, concurrent writes, update/delete races, cluster-wide and single-instance Redis outages, a silently dead subscription, kill/restart, a publisher killed between commit and publish, a commit with no event, slow and dead readers, bootstraps racing writes, dropped client links and poll fallback, and environment isolation. Against 28eb233 (CHECKGATE_TEST_SERVER_BIN), the silently dead subscription, the publisher crash, the eventless commit and the dead reader never converged. lifecycle.rs covers readiness against database and Redis loss, SIGTERM drain and exit, metrics validity and label hygiene, SSE capacity refusals, dashboard security headers, and that archival and segment edits cause no false resyncs while a real loss resyncs only its environment. The shared harness retries a cluster instance on a fresh port when another process takes its port between free_port() and the bind.
…measured baseline - bench/ (new, unpublished crate): Criterion benchmarks of checkgate-core — boolean lookup at 10/100/1 000/10 000 flags, missing flag, 1 and 10 targeting rules, rollout, weighted multivariate, prerequisite chains, concurrent readers with and without a writer replacing flags, and one SDK event apply — plus an allocation profile from a counting allocator. Kept out of core/ because the React Native packager copies core's manifest without benches/, which broke the packed SDK's native build (found by verify-android-native.sh). Dockerfile copies the new member. - server/examples/loadgen.rs: starts N release servers, then measures SSE ramp-up, held-connection RSS/CPU, write-to-visible propagation on one monotonic clock (no clock skew), fan-out spread, snapshot throughput and REST latency, and reads server-side histogram means from /metrics. - docs/performance: methodology, baseline (i5-9400F; two repetitions; raw JSON and Criterion output in results/), including a synchronous_commit=off run showing commit fsync is most of a write's latency.
… extended workflow - server-integration: also runs tests/lifecycle.rs and tests/reliability.rs (bounded deadlines, ~2 min), uploads instance logs and measured recovery windows as an artifact, has a 30-minute timeout, and pins the PostgreSQL (16.15) and Redis (7.4.11) service images by digest. - reliability.yml (nightly and on demand): repeats the fault-injection suites N times with a recovery-window summary, runs the control-plane load profiles, and runs the Criterion benchmarks, uploading all results. Kept off pull requests so ordinary CI stays fast.
…ment - docs/operations/reliability.md: how changes propagate, every loss mode and its recovery path, the stated guarantee (bounded staleness once PostgreSQL is reachable; not exactly-once), and the test matrix. - docs/operations/observability.md: metrics reference, how to tell healthy, Redis-down, database-down, lagging, resyncing and capacity states apart, alert rules, health endpoints, stable log events. - docs/operations/failure-recovery.md: lifecycle, Kubernetes probes and grace periods, Docker, load-balancer settings (no stickiness needed), capacity limits and overload behaviour, troubleshooting. - Self-hosting and API reference: new settings, health endpoints, 503 refusals, Forwarded header, per-instance rate-limit note; sidebar entries. - CHANGELOG and the Phase 2 engineering report.
…bility Phase 2: production reliability, resilience and observability
The Phase 2 engineering report is for internal use. It moves to internal_docs/, which is now gitignored, and the tracked-artifacts CI guard fails if anything under internal_docs/ is ever committed (for example with `git add -f`). Nothing tracked referenced the report.
chore: keep internal reports out of the repository
…go-deny The bench crate added in #22 declared `checkgate-core = { path = "../core" }` with no version, which cargo-deny's `wildcards = "deny"` rejects, failing the Supply chain job on dev. Declare it as the server crate does (`version = "0.1.0", path = "../core"`). Lockfile unchanged. `cargo deny --all-features check`: advisories, bans, licenses, sources ok.
fix(bench): version the checkgate-core path dependency for cargo-deny
The Docker smoke job on dev fails at "re-created container on the same volume": pg_ctl reports "server started" and psql then finds no socket. `docker rm -f` (like any kill, OOM or node loss) leaves postmaster.pid in the data volume. Since the entrypoint switched from `su` to `setpriv`, every boot after the first runs the same process sequence, so the new postmaster gets the same PID as the old one; when it also starts within two seconds of the old lock's start time (CI re-creates in ~1.4 s), `pg_ctl -w` accepts the stale file's "ready" line and returns before the server has finished crash recovery or created its socket. - Before starting, remove postmaster.pid when its PID is not a live postgres process in this container (nothing else runs PostgreSQL here). - After pg_ctl, wait for pg_isready on the socket (up to 60 s, then fail with a pointer to the log) before running psql. docker-smoke.sh now reproduces it deterministically: it records the postmaster PID of a normal boot, kills the container, plants a lock with that PID, a future start time and a "ready" status, and requires the next container to start. The pre-fix image fails that step with the CI error; the fixed image passes the whole smoke test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Type of change
Testing
cargo test --workspacepassesChecklist