chore(deps): bump node from 20-alpine to 25-alpine in /apps/web - #2
chore(deps): bump node from 20-alpine to 25-alpine in /apps/web#2dependabot[bot] wants to merge 1 commit into
Conversation
Bumps node from 20-alpine to 25-alpine. --- updated-dependencies: - dependency-name: node dependency-version: 25-alpine dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
LabelsThe following labels could not be found: Please fix the above issues or remove invalid values from |
|
Closing: Node 25 is non-LTS. Should target Node 22-alpine (current LTS) instead. |
|
OK, I won't notify you again about this release, but will get in touch when a new version is available. If you'd rather skip all updates until the next major or minor version, let me know by commenting If you change your mind, just re-open this PR and I'll resolve any conflicts on it. |
TestHandleCommandUsesWorkerPoolLimits had two bugs that combined to hang the heartbeat package for 10 minutes on Linux CI: 1. Race between Submit and worker pickup. The test creates workerpool.New(1, 1) (1 worker, queue size 1) and immediately submits two blocking tasks. It assumed the worker had already read task #1 out of the 1-slot channel before Submit #2 ran. On Linux the worker consistently lost the race, so Submit #2 saw a full channel, returned false, and the test t.Fatal'd. 2. Deferred Shutdown deadlock. t.Fatal triggers runtime.Goexit, which runs the deferred pool.Shutdown(context.Background()). Drain → wg.Wait() — but the in-flight task #1 was still blocked on <-blocker (the test only closes blocker on the happy path past the t.Fatal). With no context deadline, Drain blocked forever, leaving heartbeat.test as an orphan process at GHA cleanup. Fix: - Add a `started` channel that task #1 closes as its first action. After Submit #1, wait on <-started (2s deadline) so the worker has provably drained the queue before Submit #2 runs. - Replace the deferred Shutdown with a closure that idempotently unblocks any in-flight tasks and drains with a 5s context deadline so a future regression can't hang CI for 10 minutes. - Make unblock() idempotent so the happy-path close and the deferred cleanup don't double-close. Verified with -count=50 -race on darwin and in a Linux Docker container (golang:1.25.9). Both 50/50 passes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Database-layer hardening following the RLS security review. The app previously connected as POSTGRES_USER (SUPERUSER with BYPASSRLS), making every RLS policy a no-op. Closes that gap and fixes every hole that was hiding behind it. - Add unprivileged breeze_app role (NOSUPERUSER, NOBYPASSRLS) via new ensureAppRole bootstrap in autoMigrate. API connects via DATABASE_URL_APP with a startup warning if the connected user still has BYPASSRLS. (Finding #1) - Rewrite broken backup/DR/C2C/vault policies that referenced an unset session variable app.current_org_id. 15 tables now use breeze_has_org_access(org_id) + FORCE ROW LEVEL SECURITY. (Finding #2) - Add org_id column + RLS policies to device_metrics, backfilled from parent devices table. (Finding #3) - Restrict cis_check_catalog write policies to system scope. (Finding #4) - Replace withSystemDbAccessContext with org-scoped contexts in AI agent services (aiAgentSdk, aiAgentSdkTools, scriptBuilder, streamingSessionManager). Capture canonical orgId on ActiveSession. (Finding #5) - Fix 13 runOutsideDbContext sites in commandQueue and backup restore/vmrestore that would have silently failed once BYPASSRLS is removed. (Finding #7) audit_logs NULL org_id (Finding #6) is intentional and unchanged — system-level events are only visible to system scope by design. Verified: 3114/3120 API tests pass; same 6 pre-existing failures in enrollmentKeys_installer.test.ts on main. Manual DB check: breeze_app sees 0 devices without scope, 6 with system scope — RLS now enforced. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…e fixes Addresses review findings on PR #389 (heartbeat starvation instrumentation): CRITICAL #1 -- timedRWMutex now instruments write-lock HOLD time in addition to wait time. Long holds under contention (e.g. handleConnection holding b.mu.Lock across a 15s SendCommandAndWait) were invisible to the existing wait-only instrumentation, defeating the diagnostic goal for the exact bug class it was designed to surface. Implementation tracks acquiredAt inside Lock()/Unlock() -- safe because exactly one writer holds the lock at a time. Read-lock hold time is NOT instrumented: a single field is racy with multiple concurrent RLock holders, and alternatives (gid maps, tokens, atomic pointers) either race or uglify the API. RLock holders in this broker never perform long-blocking work, so instrumenting Lock holds alone captures the dangerous class of bug. slowLockThreshold extracted to a package var (1s default) so tests can shorten it. CRITICAL #2 -- snapshotSessions now copies the live map inside the locked fallback path instead of returning it and iterating after the deferred RUnlock fires. A one-time WARN log is emitted via atomic.Bool if the fallback ever fires in production (tests construct Broker{} directly). IMPORTANT #3 -- sendHeartbeatWithWatchdog uses `defer close(done)` so a panic in sendHeartbeat cancels the watchdog instead of letting it dump a misleading "exceeded 15s" warning. sync.Once removed (the select fires at most once). heartbeatWatchdogTimeout extracted to a package var and a sendHeartbeatFn field added to *Heartbeat so tests can inject fast/slow implementations and a short timeout without waiting 15 real seconds. IMPORTANT #4 -- Session.GetCapabilities() added. It takes s.mu.Lock() and returns a copy. All snapshot-path readers (FindCapableSession, preferredDesktopSessionFromSnap, preferredDesktopSessionLocked, reapIdleSessions) now go through it instead of reading s.Capabilities directly. Before PR #389, b.mu.RLock() accidentally serialised those reads with SetCapabilities writers; with the snapshot refactor the accidental protection is gone and direct reads race under -race. The log site inside TypeCapabilities handling now reads from the locally-held sanitized copy instead of a post-Set read of s.Capabilities. IMPORTANT #5 -- TCCStatus now loads b.snap directly and only falls back to snapshotSessions when snap is nil. Eliminates the ad-hoc partial sessionSnapshot construction (byIdentity was nil) that would silently break any future reader touching snap.byIdentity. publishSnapshotLocked is now the sole construction site for sessionSnapshot outside the fallback. IMPORTANT #6 -- BenchmarkFindCapableSessionUnderConcurrentConnections now asserts FindCapableSession returns non-nil. The benchmark pre-registers capture-capable sessions in WinSessionID "1" and write-storm goroutines only churn storm-* sessions, so nil indicates a regression. Suggestions applied: noCopy sentinel on timedRWMutex for go vet, sync/atomic merged into the sync import group, callerName closure-suffix stripping fixed to handle both "-" and "." separators, CloseSessionsByDesktopContext stale-snapshot latency window documented. All tests pass with -race. No public API changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Database-layer hardening following the RLS security review. The app previously connected as POSTGRES_USER (SUPERUSER with BYPASSRLS), making every RLS policy a no-op. Closes that gap and fixes every hole that was hiding behind it. - Add unprivileged breeze_app role (NOSUPERUSER, NOBYPASSRLS) via new ensureAppRole bootstrap in autoMigrate. API connects via DATABASE_URL_APP with a startup warning if the connected user still has BYPASSRLS. (Finding #1) - Rewrite broken backup/DR/C2C/vault policies that referenced an unset session variable app.current_org_id. 15 tables now use breeze_has_org_access(org_id) + FORCE ROW LEVEL SECURITY. (Finding #2) - Add org_id column + RLS policies to device_metrics, backfilled from parent devices table. (Finding #3) - Restrict cis_check_catalog write policies to system scope. (Finding #4) - Replace withSystemDbAccessContext with org-scoped contexts in AI agent services (aiAgentSdk, aiAgentSdkTools, scriptBuilder, streamingSessionManager). Capture canonical orgId on ActiveSession. (Finding #5) - Fix 13 runOutsideDbContext sites in commandQueue and backup restore/vmrestore that would have silently failed once BYPASSRLS is removed. (Finding #7) audit_logs NULL org_id (Finding #6) is intentional and unchanged — system-level events are only visible to system scope by design. Verified: 3114/3120 API tests pass; same 6 pre-existing failures in enrollmentKeys_installer.test.ts on main. Manual DB check: breeze_app sees 0 devices without scope, 6 with system scope — RLS now enforced. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…500) ## Summary Closes 13 of 14 findings from the 2026-04-20 security review of `feature/device-org-move-wip`. 12 discrete tasks + 2 correction commits, each independently revertible. Full plan: `docs/superpowers/plans/2026-04-20-security-review-remediations.md`. ## Findings addressed | # | Sev | Area | Fix | |---|---|---|---| | #1 | HIGH | uninstall scripts | Publish SHA256SUMS + surface hash/verify cmd in `AddDeviceModal` | | #2 | HIGH | public-download token in URL | One-time Redis handle via new `POST /enrollment-keys/:id/download-handle`; back-compat `?token=` path retained with deprecation warning | | #3 | HIGH | enrollment-key write routes unrated | New `userRateLimit` middleware (10/60s per user) on all 5 write routes (create / rotate / delete / installer-link / bootstrap-token) | | #4 | MEDIUM | 8h viewer JWT | Dropped to 2h; added `jti` claim; Redis-backed revocation + per-tunnel-session revoke-on-close | | #5 / #6 | MEDIUM | tunnels.ts `:id` / `siteId` unvalidated | `zValidator('param'/'query', …)` on 7 routes | | #7 | MEDIUM | install.bat template | `assertValidEnrollmentKey` (64-hex regex) + quoted `ENROLLMENT_KEY` | | #8 | MEDIUM | viewer HTTP allowlist | `isLocalhost()` replaces `isPrivateHost()` — 127.0.0.1 / ::1 / localhost only | | #9 | MEDIUM | VNC server reason → UI | `KNOWN_VNC_REASONS` allowlist + `friendlyReason()` fallback (incl. ARD form) | | #10 | MEDIUM | siteId ownership on enroll create | Query `sites` WHERE `sites.orgId = orgId` before insert | | #12 | MEDIUM | short-link claim race | Atomic UPDATE folds expiry + max-usage into WHERE; claim-before-insert | | #13 | LOW | helper install templates | `Exec=%q` on Linux; `xml.EscapeText` on Darwin plist | | #14 | LOW | session config perms | 0644 → 0600 on 3 write sites | Finding #11 (encrypted `enrollment.json` inside installer zip) and other low-severity items were intentionally deferred — documented in the plan's "Out of scope" section. ## Test plan - [x] API: `pnpm --filter @breeze/api test:run` — 3247 passed, 25 skipped - [x] Viewer: `npx vitest run` in `apps/viewer` — 114 passed - [x] Web: `npx vitest run` in `apps/web` — 279 passed - [x] Agent helper: `cd agent && go test -race ./internal/helper/...` — ok - [ ] Manual smoke: trigger a macOS installer download from staging and confirm the agent enrolls (exercises finding #2 + #7 + #10 end-to-end). - [ ] Manual smoke: open a WebRTC session, hit DELETE `/tunnels/:id`, confirm the viewer sees a 401 on its next poll (exercises finding #4). - [ ] Visual review: uninstall-script SHA256 block renders correctly below each download link. ## Notes for the reviewer - Two commits (`7db3b1fb`, `845e2483`, `6be03849`) have muddled boundaries because several subagents staged changes concurrently in the same worktree. The tree state is correct; only the commit-per-task narrative got blurred. Squash-merge will render this moot. - `enrollmentKeys.ts` grew to 1333 lines. It's cohesive but approaching hard-to-navigate; candidate for a follow-up split of `publicEnrollmentRoutes` + `publicShortLinkRoutes` + `serveInstaller` into a `enrollmentKeys.public.ts`. - `/:id/download-handle` uses `===` on SHA-256 digests rather than `timingSafeEqual`. Low risk given both sides are digests + the route is auth-gated + rate-limited, but the convention elsewhere (`apiKeyAuth.ts`) uses `timingSafeEqual`. Trailing TODO worth picking up. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Moves remote-access launch URL issuance off the GET /devices/:id response
so the substituted password-bearing URL is never broadcast on every
device-detail fetch.
GET /devices/:id now returns hasRemoteAccessLauncher (bool) and
remoteAccessLaunchSkipReason (config_error | no_provider_configured |
provider_disabled | missing_device_identifier | empty_url_template |
scheme_not_allowed). The substituted URL is gone from this response.
POST /devices/:id/remote-access-launch:
- requires the same organization/partner/system scope GET has
- resolves the partner-configured provider, builds the substituted URL,
and returns {launchUrl, scheme, providerId}
- emits device.remote_access_launch_url.issued audit on success with
only deviceId, providerId, and scheme in details (NEVER the URL or
password)
- returns 422 with code=scheme_not_allowed AND emits
device.remote_access_launch_url.scheme_rejected (result=denied) AND
routes to Sentry when a tampered template resolves to a disallowed
scheme only after substitution
- registered before PATCH/DELETE /:id so Hono's
match-in-registration-order resolves the path correctly
Includes tests for success/issuance audit, no-provider 404, scheme-
rejected 422/audit/Sentry, and verification that the substituted URL
and password never appear in the audit details JSON.
Addresses Todd's review items #1, #2, and LanternOps#3.
Moves remote-access launch URL issuance off the GET /devices/:id response
so the substituted password-bearing URL is never broadcast on every
device-detail fetch.
GET /devices/:id now returns hasRemoteAccessLauncher (bool) and
remoteAccessLaunchSkipReason (config_error | no_provider_configured |
provider_disabled | missing_device_identifier | empty_url_template |
scheme_not_allowed). The substituted URL is gone from this response.
POST /devices/:id/remote-access-launch:
- requires the same organization/partner/system scope GET has
- resolves the partner-configured provider, builds the substituted URL,
and returns {launchUrl, scheme, providerId}
- emits device.remote_access_launch_url.issued audit on success with
only deviceId, providerId, and scheme in details (NEVER the URL or
password)
- returns 422 with code=scheme_not_allowed AND emits
device.remote_access_launch_url.scheme_rejected (result=denied) AND
routes to Sentry when a tampered template resolves to a disallowed
scheme only after substitution
- registered before PATCH/DELETE /:id so Hono's
match-in-registration-order resolves the path correctly
Includes tests for success/issuance audit, no-provider 404, scheme-
rejected 422/audit/Sentry, and verification that the substituted URL
and password never appear in the audit details JSON.
Addresses Todd's review items #1, #2, and LanternOps#3.
Moves remote-access launch URL issuance off the GET /devices/:id response
so the substituted password-bearing URL is never broadcast on every
device-detail fetch.
GET /devices/:id now returns hasRemoteAccessLauncher (bool) and
remoteAccessLaunchSkipReason (config_error | no_provider_configured |
provider_disabled | missing_device_identifier | empty_url_template |
scheme_not_allowed). The substituted URL is gone from this response.
POST /devices/:id/remote-access-launch:
- requires the same organization/partner/system scope GET has
- resolves the partner-configured provider, builds the substituted URL,
and returns {launchUrl, scheme, providerId}
- emits device.remote_access_launch_url.issued audit on success with
only deviceId, providerId, and scheme in details (NEVER the URL or
password)
- returns 422 with code=scheme_not_allowed AND emits
device.remote_access_launch_url.scheme_rejected (result=denied) AND
routes to Sentry when a tampered template resolves to a disallowed
scheme only after substitution
- registered before PATCH/DELETE /:id so Hono's
match-in-registration-order resolves the path correctly
Includes tests for success/issuance audit, no-provider 404, scheme-
rejected 422/audit/Sentry, and verification that the substituted URL
and password never appear in the audit details JSON.
Addresses Todd's review items #1, #2, and LanternOps#3.
Moves remote-access launch URL issuance off the GET /devices/:id response
so the substituted password-bearing URL is never broadcast on every
device-detail fetch.
GET /devices/:id now returns hasRemoteAccessLauncher (bool) and
remoteAccessLaunchSkipReason (config_error | no_provider_configured |
provider_disabled | missing_device_identifier | empty_url_template |
scheme_not_allowed). The substituted URL is gone from this response.
POST /devices/:id/remote-access-launch:
- requires the same organization/partner/system scope GET has
- resolves the partner-configured provider, builds the substituted URL,
and returns {launchUrl, scheme, providerId}
- emits device.remote_access_launch_url.issued audit on success with
only deviceId, providerId, and scheme in details (NEVER the URL or
password)
- returns 422 with code=scheme_not_allowed AND emits
device.remote_access_launch_url.scheme_rejected (result=denied) AND
routes to Sentry when a tampered template resolves to a disallowed
scheme only after substitution
- registered before PATCH/DELETE /:id so Hono's
match-in-registration-order resolves the path correctly
Includes tests for success/issuance audit, no-provider 404, scheme-
rejected 422/audit/Sentry, and verification that the substituted URL
and password never appear in the audit details JSON.
Addresses Todd's review items #1, #2, and LanternOps#3.
…apper, consent race, schema drift (#741) Stacked on `feat/mobile-approval-mode` (base = PR #696's branch, same pattern as #698). Branched off `ecc5350d` so #698's fixes are preserved. Fixes the four Critical/scoped items from the #696 review that made the feature **non-functional in production** despite green (mocked) unit tests. ## Fixes - **#1 — account-deletion admin queue returned empty.** Two layered defects: (a) the `account_deletion_requests` Shape-6 RLS policy had no system-scope OR branch, and (b) `accountDeletion.ts`'s `runWithSystemDbAccess` lacked `runOutsideDbContext`, so inside an admin request it inherited the admin's own scope. Both fixed (migration + wrapper now mirrors `lifecycle.ts`'s `asSystem`). - **#2 — expiry reaper never expired anything.** `approval_requests` Shape-6 policy had no system-scope OR branch → under SYSTEM scope `breeze_current_user_id()` is NULL → FORCE RLS hid every row from `breeze_app`. Same migration adds the branch (the reaper's own wrapper was already correct — BullMQ worker, no ambient context). - **#3 — mobile biometric consent race.** A focus swap during the OS biometric prompt could rebind consent to a different approval. Request id is now captured at press time, threaded `ApprovalButtons → ApprovalScreen`, and gated by a pure `decisionTarget()` — aborts with a re-review prompt instead of deciding the wrong action. - **#7 — schema drift.** `oauth.ts` now declares the `oauth_client_blocks_org_client_uniq UNIQUE(org_id, client_id)` index the migration creates (the lifecycle upsert relies on it). *Note: not a CI blocker — `ci.yml`'s drift step is `continue-on-error`.* The migration mirrors the `oauth_authorization_codes`/`sessions` `OR breeze_current_scope() = 'system'` pattern. Request-scoped callers never have `scope='system'`, so the `user_id` predicate still governs — no cross-user exposure (the unchanged cross-user forge tests prove this). ## Verification (test-driven, RED→GREEN) - New `approval-system-scope.integration.test.ts` — #1 + #2 against a real DB (watched fail with the policy reverted). - New `decisionTarget.test.ts` — pure consent-binding guard, 4/4. - RLS contract + cross-user forge **13/13**; accountDeletion/admin/autoMigrate units **38/38**; full mobile suite 137 pass; no new type errors. - Stale "no system-scope OR branch" comments in `rls-coverage` corrected (comment rot this migration introduced). ## Scope Criticals + #7 only. The review's Important items (report-suspicious non-atomic flip, Expo-push silent auto-deny, receipts, `oauthRevocation` coverage, cross-user denial tests, `azp` block bypass, biometric-on-deny) are intentionally **out of scope** — for the #696 split. Refs #696, #698. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… fix (#753) ## Summary - Phase 1: cursor fan-out for `alertWorker` and `offlineDetector`, plus an env-tunable per-run cap on `staleCommandReaper`. Removes implicit fleet-size caps in the alert/offline/reaper jobs. - Phase 2: seven `CREATE INDEX IF NOT EXISTS` migrations targeting `/devices`, `/alerts`, `audit_logs`, `device_software`, `script_executions`, `device_patches`, and `agent_logs`. Plain (non-concurrent) `CREATE INDEX` because `autoMigrate` wraps each migration in a transaction; `CONCURRENTLY` requires running outside a tx. - Phase 2.2 (bug fix, load-bearing): rewrite of `breeze_accessible_org_ids` / `breeze_accessible_partner_ids` to drop the plpgsql `EXCEPTION WHEN others` blocks. Without this, the Phase 2 indexes will crash `/devices` on any partner-scope tenant. - Phase 3 (server side only): additive cursor pagination for `/mobile` list endpoints. Existing page-based callers unchanged. ## Why this batch The RLS helper rewrite is the load-bearing reason these ship together. `2026-05-17-e` (already in this PR set) marks `breeze_accessible_org_ids` and `breeze_accessible_partner_ids` as `PARALLEL SAFE LEAKPROOF`, but the bodies still wrap the parse in `EXCEPTION WHEN others`. Postgres `EXCEPTION` blocks create implicit subtransactions, which cannot run inside parallel workers. The label is honored, the runtime is not. In steady state on small fleets the planner doesn't choose parallel scans, so this is latent. Once the Phase 2 indexes land and `/devices` starts going parallel on partner-scope reads, every request 500s with: ``` cannot start subtransactions during a parallel operation ``` Reproduced this in production tonight on a 78-device fleet. Phase 2 indexes alone made `/devices` unreachable for partner-scope users; the `2026-05-18-a` rewrite (regex pre-validation, same fail-closed semantics, no implicit subtransaction) restored it. Anyone deploying the Phase 2 indexes without the rewrite will hit this. ## Verified numbers (production, 78-device fleet) - `GET /devices?limit=500` partner scope: 10s timeout → 154ms. Crash fixed by the RLS rewrite; latency gain stacks on top of the LATERAL+LIMIT 1 work in #747. - `audit_logs` org-scoped list query (`WHERE org_id = ? ORDER BY timestamp DESC LIMIT 50`): pre-index pattern was Parallel Seq Scan over the full table (~140ms+ on 448k rows). Post-index is Index Scan on `audit_logs_org_timestamp_idx`, **0.43ms execution / 52 buffer hits** at 448k rows. Scales as `O(log n)` instead of `O(n)`. - `alertWorker` on a 78-device fleet: with cursor fan-out enabled, `[AlertWorker] Evaluate-all completed: 71/72 devices queued` consistently across last 10 runs (live log, this PR's branch). The prior `Math.min(100)` cap was the difference between alerting ~1.7% of fleet per cycle and 100% at 10k. ## Commits ``` 1461910 perf(db): add scale-readiness indexes (Phase 2: S4-S8) 9dbf925 perf(jobs): cursor fan-out for alertWorker + offlineDetector (Phase 1: S1+S2) b943fc7 perf(jobs): raise StaleCommandReaper per-run cap to env-tunable 5000 (Phase 1: S3) e635413 perf(db): add device_patches scale indexes + agent_logs composite (Phase 2.1) e17a829 perf(db): rewrite RLS helpers to drop EXCEPTION blocks (Phase 2.2) 40dd3be feat(api): /mobile cursor pagination — additive (Phase 3: PR-S9 server-side) ``` Branch: `bdunncompany:scale-pack-pr-2026-05-18` (cherry-picked clean from `bdunncompany:scale-readiness-2026-05-17`; Huntress integration fixes on the source branch are intentionally excluded and would land separately). Compare: `main...bdunncompany:scale-pack-pr-2026-05-18` ## Migrations (all idempotent) ``` 2026-05-17-a-devices-scale-indexes.sql 2026-05-17-b-alerts-scale-indexes.sql 2026-05-17-c-audit-logs-scale-indexes.sql 2026-05-17-d-device-software-script-executions-indexes.sql 2026-05-17-e-rls-helpers-leakproof-parallel-safe.sql 2026-05-17-f-device-patches-indexes.sql 2026-05-17-g-agent-logs-composite-index.sql 2026-05-18-a-rls-helpers-parallel-safe-rewrite.sql ``` Safety notes: - Index migrations use `CREATE INDEX IF NOT EXISTS`. Plain (non-concurrent) `CREATE INDEX` takes a `SHARE` lock on the target table for the build — blocks writes, allows reads. Re-running on a populated DB is a no-op (IF NOT EXISTS). - `2026-05-17-e` (already in tree) and `2026-05-18-a` are `CREATE OR REPLACE FUNCTION`, preserving signatures and return types; `2026-05-18-a` additionally drops the `EXCEPTION` block from two helpers so the existing `PARALLEL SAFE` marker is no longer a lie. No call-site changes. - `autoMigrate.test.ts` (filename-sort regression) clean against the set. ## Test plan - `apps/api/src/jobs/alertWorker.test.ts` — 6 tests covering single-chunk fan-out, multi-chunk pagination, `ALERT_WORKER_MAX_DEVICES_PER_RUN` cap enforcement + warning, `cap=0` unlimited semantics, caller-supplied `batchSize` back-compat, and empty-fleet handling. - `apps/api/src/jobs/offlineDetector_fanout.test.ts` — 5 tests covering the same shape on `OFFLINE_DETECTOR_MAX_DEVICES_PER_RUN`. - `apps/api/src/routes/mobile.cursor.test.ts` — 6 tests on the new `encodeCursor` / `decodeCursor` helpers (round-trip, ISO string input, null/empty guards, garbage-input fail-closed, URL-safe base64url output). - `apps/api/src/db/autoMigrate.test.ts` — 27 tests, all green; includes the filename-sort regression that catches `-a-`/`-b-` ordering bugs. - Production smoke (single 78-device partner-scope tenant, 2026-05-17 → 2026-05-18 UTC): ran the full migration set + Phase 1 job changes; hit `/devices`, `/alerts`, `/audit_logs`, `/mobile` endpoints; observed the numbers above. Pre-rewrite, `/devices` 500'd with "cannot start subtransactions during a parallel operation"; post-rewrite (`2026-05-18-a`) and API restart to flush stale prepared-statement plans, 0 occurrences in subsequent 30+ min. ## Coverage against the local audit Tier 0: #1 alertWorker cap, #2 offlineDetector cap, #3 staleCommandReaper cap, #4 mobile pagination (server side only here; mobile client follow-up). Tier 1: #5 alerts indexes, #6 devices indexes. Tier 2: #17 audit_logs indexes, #20 device_software, #21 script_executions, #23 RLS helpers parallel-safe. ## Related - Builds on #747 (LATERAL+LIMIT 1 for `/devices` latest-metrics) and #748 (limit cap raise to 500). - Cursor pagination Path B: Discussion #742 (LATERAL latest-metrics + cap lift + server-side pagination plan). - Happy to split this into smaller PRs (Phase 1 jobs / Phase 2+2.2 DB / Phase 3 /mobile) if that's easier to review — say the word.
…p + doc fixes (#845 follow-up) (#848) ## Summary Follow-up to PR #845 (which fixed issue #816 — the agent's in-place upgrade now deploys `breeze-user-helper.exe`). The multi-agent review surfaced several correctness gaps; this PR addresses the **critical** ones plus three small low-risk suggestions. Strictly in scope (per the plan). The type-design refactor (PR B) and remaining test coverage (PR C) are separate, planned follow-ups. ## Fixes mapped to review findings ### #1 — PowerShell hardening (C1, restart_windows.go) PowerShell's `Copy-Item` is non-terminating by default; a failed Copy (locked file, ACL denied, disk full) would silently regress to the partial-success state #816 was filed against. The generated script now: - sets `$ErrorActionPreference = 'Stop'` globally, - wraps the swap block (`Stop-Service` / `Stop-Process` / `Copy-Item agent` / `Copy-Item user-helper` / `Start-Service`) in a single `try { … } catch { … }`, - on failure, logs `Exception.Message`, `Exception.StackTrace`, `ScriptStackTrace`, and the failing operation to `${env:TEMP}\breeze-update-failure-<unix>.log` via `Out-File -Append -Encoding utf8` (uses `$env:TEMP` not `C:\ProgramData\Breeze` — the latter may not exist on a fresh install, cf. #609), - always attempts `Start-Service` afterwards (catch path uses `-ErrorAction SilentlyContinue` so a failed start doesn't re-throw and skip cleanup), - keeps `Remove-Item` cleanups outside the try/catch so they always run. Decision recorded: **single try/catch wrapping the entire swap block**, with `Start-Service` invoked once inside the try (success path) and once inside the catch (degraded path). I considered a nested try/catch around each Copy-Item but it'd be more code for the same outcome — any Copy failure already needs to abort the swap, and a single catch keeps the failure log structure clean. `Stop-Service` keeps its `-ErrorAction SilentlyContinue` since the service legitimately may not exist on some test paths. ### #2 — User-helper temp leak (C2, updater.go) `UpdateTo`'s error branches called `removeCleanup(tempPath)` only for the agent temp. The pre-downloaded user-helper temp would orphan in `%TEMP%` on every failed upgrade. Fixed at two sites: - `UpdateToWithUserHelper` now cleans up the helper temp when `UpdateTo` returns an error. - `UpdateTo`'s `RestartWithHelper` failure branch also cleans up `u.extras.userHelperTempPath`. On success the spawned restart script owns the file and removes it. ### #3 — Stale doc references (C3, updater.go) Comments at the `extras` field and `updateExtras` struct cited `UpdateToWithCompanions` / `UpdateFromURLWithCompanions`, neither of which exists. The actual setter is `UpdateToWithUserHelper`. Also added an explicit "asymmetry footgun" note: `UpdateFromURL` reads `u.extras` but has no companion-setter wrapper, so a dev-push caller wanting a helper swap on that path must mutate `u.extras` directly. Will be cleaned up by PR B's refactor, but the misleading references were actively wrong now. ### I5 — User-helper download WARN context (heartbeat.go) Added `currentVersion` to the `"user-helper download failed; proceeding with agent-only upgrade"` WARN — the single most useful field for distinguishing "release pre-dates artifact" from "release should have shipped artifact but didn't." Rewrote the preceding comment block: the original "404 / network / checksum" wording undersold the scope (the catch swallows any error from `DownloadBinary`, including auth-token expiry, manifest signature failure, JSON decode, FS write). New comment makes the scope explicit and documents the two intentional reasons we degrade. ### S1 — Ordering rationale in production code (restart_windows.go) Mirrored the test docstring rationale ("agent Copy must come before helper Copy — partial failure leaves a working pre-#816 install, not a helper-installed-but-agent-stale state") into a comment in `buildRestartScript` so the constraint is documented at the line that enforces it. ### S2 — binarySync.ts WARN on metadata resolution failure The agent/helper/user-helper sync loops all had a silent `if (!metadata) continue;` after the silent `if (!asset) continue;`. The `!asset` case is legitimate (release predates the artifact) and stays silent; the `!metadata` case indicates an unexpected manifest/checksums inconsistency and now emits a `console.warn` with release tag, asset name, and component. No behavior change otherwise. ### S3 — `DownloadBinary` doc explaining the second checksum verify Added a one-paragraph note: internal `downloadBinary` verifies the signed manifest (JSON payload's Ed25519 signature) but does not verify the downloaded file bytes against `manifest.Checksum`. `DownloadBinary` does that file-checksum verification here so exported callers get a verified file without knowing the manifest-vs-file distinction. A future "simplifier" must not delete it as redundant. ## Test Plan All existing tests pass; new tests added: **Go (`agent/internal/updater/`):** - `TestBuildRestartScript_ErrorActionPreference` — generated script sets `$ErrorActionPreference = 'Stop'`. - `TestBuildRestartScript_TryCatchWrapsSwap` — both Copy-Item calls are inside `try { … } catch { … }`. - `TestBuildRestartScript_StartServiceInBothPaths` — `Start-Service` appears twice (try + catch); catch path uses `-ErrorAction SilentlyContinue`. - `TestBuildRestartScript_FailureLogUsesTemp` — failure log path uses `$env:TEMP`, not a hardcoded drive letter; uses `Out-File -Append -Encoding utf8`. - `TestBuildRestartScript_CleanupOutsideTryCatch` — `Remove-Item` cleanup lines appear after the catch-block close. - `TestUpdateToWithUserHelper_CleansHelperTempOnFailure` — synthesizes a helper temp file, forces `UpdateTo` to fail, asserts the helper temp is gone. - `TestUpdateToWithUserHelper_NoHelperTempPathIsNoOp` — regression guard that the new cleanup is a no-op on agent-only upgrades. **Verification commands:** - `cd agent && go build ./...` — clean - `cd agent && go vet ./...` — clean (m1cpu warnings are upstream) - `cd agent && GOOS=windows GOARCH=amd64 go build ./...` — clean - `cd agent && go test -race ./internal/updater/... ./internal/heartbeat/...` — all pass - `cd apps/api && npx tsc --noEmit` — clean - `cd apps/api && npx vitest run src/services/binarySync.test.ts src/routes/agentVersions.test.ts` — 27/27 pass - `cd apps/api && npx eslint src/services/binarySync.ts` — clean ## Follow-up series This is part 1 of 3: - **PR B (planned)** — `updateExtras` field type-design refactor (e.g. an options struct passed into `UpdateTo` rather than a transient struct field). Will clean up the `UpdateFromURL`/`u.extras` footgun documented in this PR. - **PR C (planned)** — Test coverage for the `doUpgrade` user-helper fallback block and the `binarySync.ts` `USER_HELPER_TARGETS` sync loop. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
) ## Why `TestRunBackup_WithRetention` is flaky on CI runners. Most recent hit: [Test Agent run 26416137750 on PR #890](https://github.com/LanternOps/breeze/actions/runs/26416137750/job/77760906193) — ``` backup_test.go:494: RunBackup #2 status = "skipped", want "completed" ``` The 2nd backup decides there are no eligible files because the file's mtime is not strictly after the prior `snapshot.Timestamp`. The cutoff filter is `modTime.After(cutoff)` in `shouldIncludeFile` (`backup.go:461`). `snapshot.Timestamp` is set as `time.Now().UTC()` inside `CreateSnapshotContext` (`snapshot.go:65`) BEFORE the mock provider upload. On a fast runner, iter 1's snapshot timestamp + mock upload + return + iter 2's `WriteFile` can all land within the same filesystem-mtime tick, so the file's mtime equals (or rounds equal to) the cutoff and gets filtered out. The pre-existing `time.Sleep(10ms)` is BETWEEN `WriteFile` and `RunBackup` — that doesn't change the mtime of the file already written, so it didn't address the actual race. ## What - Sleep 100ms BEFORE iter 2's `WriteFile` so the new mtime is strictly after the prior `snapshot.Timestamp`. - Keep the original 10ms post-write sleep as belt-and-suspenders. - Comment explains the race and links to the failing CI run. No production code changes. Adds ~100ms wall-clock per test run. ## Local CI - `go test ./internal/backup/ -count=100 -race` → 100/100 pass, 46s - `go vet ./internal/backup/` → clean - `govulncheck ./internal/backup/...` → 0 vulnerabilities affecting code ## Test plan - [ ] CI green on this PR - [ ] Re-run #890's Test Agent (or wait for next Dependabot PR) — should no longer flake on this test
…1691) **Plan A of the SSO domain-ownership hardening** (spec PR #1689) — the last security-review-#2 item (H-2 root cause). Makes "configure an org's SSO provider" a distinct `sso:admin` capability instead of general `organizations:write`, without locking out any existing SSO admin. ## Changes 1. **`sso:admin` permission** added to the shared catalog (`packages/shared/src/constants/permissions.ts`) — auto-flows into `KNOWN`/`ASSIGNABLE`. 2. **Gate swap**: the 5 SSO provider-mutation routes (`POST /providers`, `PATCH`/`DELETE /providers/:id`, `POST /providers/:id/status`, `/test`) now require `sso:admin`; read routes unchanged; zero `organizations:write` left on SSO routes. 3. **Backfill migration** (`2026-06-25-sso-admin-permission-backfill.sql`): grants `sso:admin` to every role with an explicit `organizations:write` row — **non-breaking** day one (wildcard `*:*` roles pass at check time, no row needed). Idempotent. 4. Audit of provider mutations was already present (no change). ## Why non-breaking The old-gate population = explicit `organizations:write` roles ∪ `*:*` wildcard roles. The backfill covers the former; wildcards satisfy `sso:admin` at check time. No class is missed, nothing is over-granted. ## Verification - `tsc` clean; **104 affected unit tests pass** (permissions, sso routes, migration-ordering). - New tests: route-guard regression test (recorded guards include `sso:admin`, exclude `organizations:write` — fails if any route reverts); backfill integration test (grant + non-grant + idempotency) — runs in CI's Integration Tests job. - Built via subagent-driven development: per-task implementer + review, final whole-branch review (verdict: **READY TO MERGE**, 0 Critical/Important). Plan: `docs/superpowers/plans/2026-06-20-sso-admin-gating.md` (in #1689). **Plan B** (DNS domain verification) is a separate follow-up. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Todd Hebebrand <todd@lanternops.io> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…2 root cause) (#1689) Design spec (no code) for the **last deferred security-review-#2 item** — the SSO H-2 root cause. ## Context Review #2 (Core authentication) found that a malicious/compromised org-admin could point their org's OIDC provider at an attacker IdP and impersonate co-members by asserting their email. The exploitable + steady-state parts shipped in **#1655 / #1671 / #1677 / #1680**. #1671 (identity-first + safe JIT linking) neutralized the *steady-state* takeover; this spec covers the **root-cause hardening**: prove you own a domain before SSO provisions/links an email in it, and make IdP config a distinct `sso:admin` capability. ## What's specced - **DNS-TXT domain verification** (`sso_verified_domains` table) gating SSO **provisioning / JIT-linking** only — already-linked `(provider, sub)` users are never domain-checked. - **`sso:admin` permission** for provider create/update/activate + domain management; **backfilled to all `orgs:write` holders** (non-breaking). - **Per-org auto-enforce rollout**: warn mode until an org verifies its first domain, then refuse-on-unverified — no flag day. Global `SSO_DOMAIN_VERIFICATION_STRICT` override. - **Sticky** verification + a daily BullMQ re-check that audits drift without auto-unverifying. - Two implementation plans (A: gating, B: verification), built A then B. Spec: `docs/superpowers/specs/2026-06-20-sso-domain-ownership-design.md`. Please review before I turn it into an implementation plan. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Todd Hebebrand <todd@lanternops.io> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…pth) — backend (#1695) **Provenance:** Plan B of the SSO H-2 design (spec #1689). Backend only — admin UI is a tracked follow-up. **What & why:** A malicious or compromised **org-admin** could point their org's SSO at an attacker IdP and provision/JIT-link emails in a domain the org doesn't own (security review #2, H-2). The *exploitable* and steady-state takeover were already closed by #1655 (mandatory id_token signature), #1671 (identity-first lookup + safe JIT-linking), and #1691 (`sso:admin` gate). This adds the industry-standard remaining layer: **DNS-TXT proof of domain ownership** before SSO will JIT-link-by-email or provision a *new* account. Defense-in-depth, not an open exploit fix. **Behavior:** - Enforcement is **global** via `SSO_DOMAIN_VERIFICATION_STRICT=true`, or **per-org**: an org becomes gated once it has ≥1 verified domain (gradual rollout). Ships dark (flag off, no org gated until it verifies a domain). - **Already-linked identities** (matched by `(provider, sub)`) are exempt — turning enforcement on never locks out existing SSO users. - Verification is **sticky** — a transient DNS failure never un-verifies a working org. **Changes (6 tasks, TDD):** - **Schema/RLS:** `sso_verified_domains` table (shape 1, direct `org_id`; ENABLE+FORCE RLS) + idempotent migration `2026-06-26-sso-verified-domains.sql` seeding pending rows from `sso_providers.allowed_domains`; cross-org forge test; added to `ORG_CASCADE_DELETE_ORDER` (`tenantCascade.ts`). - **Service** `services/ssoDomainVerification.ts`: `normalizeDomain`, `createPendingDomain`, `verifyDomain` (DNS TXT at `_breeze-verify.<domain>` == `breeze-domain-verify=<token>`), `isDomainVerifiedForOrg`, `orgHasAnyVerifiedDomain`, `isSsoProvisioningBlocked`, `recheckAllDomains`. - **Routes** (`routes/sso.ts`): `GET/POST /sso/domains`, `POST /sso/domains/:id/verify`, `DELETE /sso/domains/:id` — every mutation gated `requirePermission(sso:admin)` + `requireMfa()`; `:id` routes do `canAccessOrg` before acting. - **Env flag** `SSO_DOMAIN_VERIFICATION_STRICT` (`config/validate.ts`, optional, never blocks boot). - **Callback gate** (`routes/sso.ts`): refuses link/provision in an unverified domain when enforcing, after the identity-first lookup and before both email-based paths, under system DB context. - **Daily re-check worker** `services/ssoDomainRecheckWorker.ts` (sticky), wired into `index.ts`. **Tests:** 102 unit tests green locally (45 service + 42 routes + 15 cascade) + tsc clean. Cross-org RLS forge test and tenantCascade list-contract run in the **Integration Tests** CI job (real DB). **Review:** Each task passed an individual spec+quality review (incl. an adversarial control-flow review of the callback gate). Whole-branch final review: **READY**, no must-fix defects. **Follow-ups (not blockers):** admin UI; a per-org cap on pending domains (DNS-sweep abuse hardening). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Todd Hebebrand <todd@lanternops.io> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…1709) **Root cause:** `run_as_user` script/exec routing is bound to the active console session (the #1009 fix #3, shipped in PR #1242) — but the binding drops delivery *silently*. `Broker.PreferredRunAsUserSession()` returns `nil` both when there is genuinely no user-role helper connected AND when a user helper exists only in a *non-console* session. Both live callers — `handlers_script.go:resolveRunAsSession` and `heartbeat.go:makeUserExecFunc` (winget user-context exec) — treat that `nil` as "no helper" and downgrade to local SYSTEM execution / error with no signal. On an unusual multi-session / RDS / terminal-server host where the operator's helper genuinely lives off the active console session (e.g. the physical console sits at the lock screen while the operator works over RDP), this manifests as an **unobservable dropped delivery** — exactly the breaking-change risk the security tracker flagged for the still-open `run_as_user` slice of #1009. This is the still-open slice of #1009 (the closed issue's fix #1/#2/#4 — the `roleIdentityRejection` console binding + helper-token gate + tests — already shipped). There is **no server/payload target to bind to**: `runAs` is an enum (`system`/`user`/`elevated`) carrying no target session id, SID, or username, so `runAs=user` means "the interactive console user" and the active console session **is** the binding target. A target-scoped selector would have no input to bind on, so the residual gap is observability, not a new binding axis. **Fix:** In `preferredRunAsUserSessionForOS` (Windows path), count run_as_user helpers excluded *solely* for being off-console. When that exclusion is the only reason nothing was selected (`best == nil && excludedNonConsole > 0`), emit a clear `WARN` — `"run_as_user delivery suppressed: no helper in the active console session"` with `consoleWinSession` and `excludedNonConsole` fields — so the dropped delivery is diagnosable. When no run_as_user helper is connected at all, the selector stays quiet (no per-poll noise). The fail-safe behaviour (never deliver a run_as_user script to the wrong principal) is **unchanged** — only its observability is added. `log.Warn` is emitted outside the broker read-lock. **Tests:** 3 table-driven cases in `agent/internal/sessionbroker/console_session_gate_test.go`: - off-console-only helper ⇒ `nil` returned **and** a suppression `WARN` with diagnostic fields fires (the RDS-host drop is now observable, not a silent wrong-session/SYSTEM downgrade), - no user helper connected ⇒ `nil`, **no** WARN (not noise), - console helper present ⇒ selected, **no** WARN. RED proven against the pre-fix silent selector (the suppression-WARN assertion failed; the no-WARN assertions passed because the old code was silent everywhere). Full `go test -race ./internal/sessionbroker/...` and `./internal/heartbeat/...` green; `go vet` and cross-compile (`GOOS=windows`) clean. **Risk / release-notes:** behaviour-possibly-affecting only in the sense that it adds a diagnostic `WARN` on the rare RDS/multi-session host where a run_as_user script can't reach a console helper — no routing/selection change, no new drops, Unix path untouched. Worth a release-notes line so operators know to look for the new "run_as_user delivery suppressed" warning when a `runAs=user` script silently runs as SYSTEM on a terminal-server host. Refs #1009. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Todd Hebebrand <todd@lanternops.io> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#1802) (#1817) Implements two of the deferred follow-ups from #1802 (the #1800 watchdog auto-update tracking issue). **Scope:** addresses **item #2** (telemetry lag) and **item #4** (local watchdog registration). Item #3 (download.ts S3 error masking) already shipped in #1807. Item #1 is a **manual Windows canary / release gate** that this PR cannot satisfy in code — so this PR **does not close the issue**; it stays open and assigned for the canary. ### Item #2 — main agent reports installed watchdog version in its normal heartbeat `devices.watchdog_version` was written **only** from watchdog FAILOVER heartbeats (`heartbeat.ts`). A recovered, healthy watchdog returns to monitoring and stops failover-heartbeating, so the dashboard kept showing the OLD version and the server re-sent `watchdogUpgradeTo` indefinitely (the agent deduped it in-memory). - **Agent:** `HeartbeatPayload.watchdogVersion` is now populated by `installedWatchdogVersion()`, which prefers the version swapped in this run, else a **cached** read of the on-disk binary via `breeze-watchdog status` (exec'd at most once per process run, 5s timeout, best-effort — never fails or stalls the heartbeat). Added per-OS `watchdogBinaryPath()` and a `watchdogVersionReader` test seam (mirrors the existing `watchdogInstaller` seam). - **Server:** heartbeat schema gains `watchdogVersion`; the main-agent branch **persists** it to the device row, and the `watchdogUpgradeTo` computation now **prefers the reported version** over the stored column (mirroring the helper path) so a successful swap stops the re-send on the very next heartbeat. Old agents that omit the field keep the existing stored-column behavior. ### Item #4 — `BINARY_SOURCE=local` watchdog registration `syncFromGitHub` registered the `watchdog` component but the local-binaries scan path did not, so self-hosters on `BINARY_SOURCE=local` never got watchdog auto-update. The local path now scans + registers `breeze-watchdog-*` siblings (served by the existing `/download/watchdog` route). Extracted `registerLocalBinaries()` and **scoped its `isLatest` demote per-component** — the inline agent path demoted ALL components for a platform/arch, a latent clobber once a 2nd component is registered locally (the GitHub `upsertVersion` path already scoped this way). ### Tests - Agent: `parseWatchdogStatusVersion` table tests + reader-priority/caching unit tests (`watchdog_version_test.go`). - API: heartbeat `watchdogVersion` persistence, old-agent omission, reported-version re-send suppression, and genuine-behind-still-upgrades tests. - binarySync: local watchdog-registration present/absent tests. Verification: `go test -race ./internal/heartbeat/... ./cmd/breeze-watchdog/...` green; affected API vitest files green (71 passed); `tsc --noEmit` clean; Windows + host Go builds clean. Addresses #1802 (items 2 and 4). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Todd Hebebrand <todd@lanternops.io> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ng (#2194) (#2202) Follow-ups from the five-agent post-merge review of #2194 (all findings + all suggestions). ## Behavioral fixes - **Password reset now respects partner-axis enforceSSO** (`passwordResetEligibility.ts`): partner-axis-only users (org_id null) were skipping the SSO gate entirely — a partner with an active `enforceSSO` provider still had a live password-reset path for its staff. The helper's `scope:'partner'` branch already existed; this was a missed call site. Covered by unit tests including a no-double-fire org regression case. - **LoginBrandingCard data-loss trap**: a silently failed initial GET rendered a blank form identical to "never configured", and the full-replace PUT would then wipe real saved branding on Save. Failed loads now show a warning banner and disable Save until a successful load. - **Misleading user-facing copy**: the card claimed branding shows at `/login?partner=…` — no such route/param exists. Copy + JSDoc now describe the real single-partner auto-detect mechanism. - **ConnectSsoCard**: backend failures were indistinguishable from "no SSO to link" (card silently vanished). Failures now log and render an inline error line. - **Login page enforceSSO treatment**: `login-context` now includes `enforceSSO`; when set, the password form collapses behind a "Sign in with password instead" toggle. The form stays reachable — org-axis users on the same instance are never SSO-gated (enforcement is per-user, server-side). ## Type/contract hardening - New shared wire contract `LoginContext*` in `@breeze/shared` — previously the same shape was independently declared 4× (Drizzle/Zod/two web literals) and the server route returned `unknown`. Drift is now a compile error. Dropped the always-`true` `available` field (presence = availability). - New migration `2026-07-04-partner-login-branding-accent-check.sql`: `#rrggbb` CHECK on `accent_color` (idempotent, guarded, row-count-reported cleanup) so the invariant no longer lives only in Zod. ## Test coverage (closes the review's gaps) - Real-DB `status='testing'` exclusion: login initiation 404 + enforceSSO non-suppression — previously verified only by mocks whose `where()` ignored its arguments. - New `loginContext.integration.test.ts`: first integration coverage of the public endpoint (single-partner payload, testing-provider exclusion, multi-partner leak-nothing, no-branding null). - `partnerLoginBrandingRls`: org-scope forge case (org tokens never pass `breeze_has_partner_access`) + route-level full-replace null-clearing proven against real Postgres. - Web: direct `loginContext` client tests (ok/!ok/throw/memoization/coalescing), `?error=sso_link_required` banner, enforceSSO collapse/reveal, `identity_in_use`/`user_gone` copy, client `ownerScope` contract (create sends, edit omits), `ConnectSsoCard` registered in the `no-silent-mutations` guard (56→57). ## Comment hygiene - Rewrote dangling "Task 5/9/11" and "security review #2 (C-1/H-1/H-2)" references (the plan doc was never merged to main) to state each invariant standalone. - Added a `KNOWN BUG (#2195)` marker above the bare `existingIdentity` read in the SSO callback — the one pre-auth read in that handler without an RLS-hazard comment, so its silence read as "this one's fine". ## Verification - API: 117 unit tests passing, `tsc --noEmit` clean; migration exercised against real Postgres in a rolled-back transaction (idempotent re-run, cleanup WARNING, 23514 on bad insert). - Web: 120 tests passing across all touched files, `astro check` clean. - Integration: 19/19 passing against the real :5433 test DB (non-vacuous — `.env.test` RLS contexts verified). Refs #2194 #2183. The callback bare-RLS-read fixes remain tracked in #2195 (marker comment added here; fix deliberately not duplicated). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Todd Hebebrand <todd@lanternops.io> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…emand recompute throttle, #1105 ingest context (#2266) ## Problem Production US (v0.91.0) showed intermittent high API latency — `GET /devices/:id/events` averaging **22.4s** (max 29s) while the same query `EXPLAIN ANALYZE`s in 0.3ms. Root cause was **event-loop contention**, not slow SQL: the in-process device-reliability scoring worker read each device's **entire 90-day history via `SELECT *`** (including the ~2KB/row `rawMetrics` JSONB) and bucketed it in JS, on every ingest POST, at worker concurrency 5. One runaway device posting every ~18s had accumulated **13,774 rows** and pegged a core on each post. Reliability ingests also sat **idle-in-transaction 5–20s** (the #1105 held-transaction pattern), flooding 1,215 warnings in 2 hours. Full design: `docs/superpowers/specs/2026-07-06-reliability-scoring-event-loop-hardening-design.md`. ## Changes 1. **Projected history read** — `getHistoryForDevice` selects only the 7 columns the scorer consumes and drops the `rawMetrics` JSONB from every 90-day read. Enforced by a narrowed `ScoringHistoryRow` type + a `satisfies Record<keyof ScoringHistoryRow, AnyPgColumn>` guard on the projection so the SQL can't silently drift back to fetching the blob. 2. **On-demand recompute throttle** — `ON_DEMAND_RELIABILITY_DEDUPE_WINDOW_MS` 30s → 10 min. A device recomputes at most once per fixed 10-min bucket regardless of post rate (the jobId slot keys on this window). This is what bounds the runaway device. 3. **Worker concurrency** 5 → 2 — caps simultaneous heavy computes. 4. **#1105 ingest context** — `POST /agents/:id/reliability` joins `SELF_MANAGED_DB_CONTEXT_ACTIONS`; a short org-scoped `withDbAccessContext` wraps only the device lookup + history insert, while the BullMQ enqueue and audit write run **outside** any open transaction (no pooled connection pinned idle-in-transaction across Redis/non-DB work). > **Note on the O(days) reduction:** the design floated pushing per-day aggregation into SQL. It was **intentionally deferred** — the #1904 global event-dedup needs per-event keys across the whole window, which a plain `GROUP BY` can't express without changing persisted counts. JS bucketing stays row-based; the runaway load is instead bounded by changes #2 + #3. Documented inline and in the design doc. ## Hardening from PR review (multi-agent pass) **Critical** - The Redis-outage **inline fallback** originally ran under an **org** context — but `computeAndPersistDeviceReliability`'s ml-feature-flag gate reads `organizations INNER JOIN partners`, invisible under org-scope RLS, so the compute **silently no-op'd** (persisted nothing). Now runs `runOutsideDbContext(() => withSystemDbAccessContext(...))`, mirroring the worker. A new integration test proves org scope no-ops while system scope persists, against **real RLS-enforced Postgres**. - The fallback was un-`try/catch`'d: after the history row was committed, a throw flipped the request to **500 → agent retries → duplicate inserts** during an outage. Now best-effort (log + `captureException`, response stays 200), matching the enqueue path. **Important** - `captureException` added to the history-insert catch (was stdout-only, so a persistent RLS misconfig would be Sentry-blind). - Fail fast with **401** when an agent token carries no `orgId` (previously a silent 404 masquerade from a vacuous org RLS context). - `LookupResult` collapsed to two arms + exhaustive `never`-check consumer so a new failure reason can't silently become a 500. **Comment accuracy** — removed non-unique `(Task 1)` and dead `task-2-report.md` references; corrected "each compute is cheap" (projection cut I/O, not the O(rows) CPU) and the fixed-bucket dedupe wording. ## Testing - **Unit** (route/worker/agentAuth/scoring): fallback runs under **system** scope; fallback-throw stays 200; `insert_failed → 500` skips enqueue/audit; missing-`orgId → 401`; `#1105` depth-0 for enqueue + audit; dedupe-window jobId reuse; concurrency pinned to 2. ✅ 190 pass - **Integration** (real Postgres): golden-value score from a projected read of a high-frequency device (50 posts/day, populated 2KB blobs); org-scoped compute no-ops (regression lock). ✅ 2 pass - `tsc --noEmit` clean. ## Rollout Normal release — no migration, no new env vars, no infra/compose changes. Relieves prod even while the runaway agent is still misbehaving. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Todd Hebebrand <todd@lanternops.io> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Hardens the agent→API trust boundary against a **compromised enrolled agent**, from a focused security review. 10 findings (5 HIGH, 5 MEDIUM) were independently verified, then fixed and re-verified. ## Findings & fixes **HIGH** - **#1 renew-cert quarantine bypass** — the route read org mTLS policy and wrote device state *after* its DB context closed, so under forced RLS as the unprivileged role those reads/writes silently returned 0 rows and a `quarantine` policy fell back to `auto_reissue`. Now reads policy + all device writes inside `withSystemDbAccessContext`, fails closed on a missing org row or any 0-row write, and only returns cert material after metadata persists (else revokes the issued cert). - **#2 previous-token self-renewal** — a superseded token (5-min grace) could mint durable new agent/watchdog/helper credentials. Now rejected (401) on both rotate-token and renew-cert; rotate-token UPDATE is a compare-and-swap bound to the authenticating token hash (409, mints nothing, on a losing race). - **#3 established WS bypass containment** — quarantine and tenant suspension/deletion didn't sever a live agent socket. Now they do; plus a lightweight per-message lifecycle recheck on the command-claim and command-result paths. - **#4 duplicate WS orphan** — a second socket replaced the tracked one without closing the first, leaving an orphan revocation couldn't reach. Now single-socket-per-agent (superseded socket closed on connect) with connection-identity-guarded ping state. - **#5 private-key redaction** — the agent regex missed PKCS#8 and left the key body intact. Now removes whole PEM blocks (PKCS#8 + RSA/EC/DSA/OPENSSH/ENCRYPTED), plus a server-side redaction layer at result ingest for pre-update agents. **MEDIUM** - **#6** CSV formula-injection — 5 web exporters now neutralize agent-controlled cells via the shared helper. - **#7** watchdog diagnostics never reached the API (wrong wire shape / status / false "completed"); now speaks the log contract and reports failure honestly. - **#8** WS command-result completions weren't audited (REST path was); now emit `agent.command.result.submit` once per real transition. - **#9** `logs.ts` ingest had no audit event; added a content-free one. - **#10** heartbeat overwrote security-relevant state with no history; now emits `agent.heartbeat.state_change` on genuine transitions only. ## PR-review follow-up (same branch) A multi-agent review verified the auth cluster clean (no bypass/RLS/silent-mint) and caught issues in the redaction layer, all fixed here: a **ReDoS** in the new server-side redactor (bounded → linear), an unredacted `error` field, truncated-key passthrough, and redaction parity (ported the agent's AWS-key/bearer/JWT/connection-string/secret-pair patterns server-side). ## Verification - `apps/api` typecheck clean; **171** unit tests pass - `apps/web` **15** tests pass - Go build + `-race` (executor / watchdog / breeze-watchdog) pass ## Pre-merge follow-ups (not done here) 1. **Real-Postgres integration test for renew-cert (#1).** The fix's premise — a contextless read returns 0 rows under forced RLS, a system-context read returns the real row — is proven only via mocks. Should be closed with one integration test against real Postgres per the repo's RLS-contract discipline. 2. **Cross-replica socket severance.** `disconnectAgent` is in-process only, so a live socket on another API replica isn't severed immediately on quarantine/tenant-suspension (mitigated by the per-message recheck; matches the existing decommission convention). Fine if prod is single-API-replica per region — worth confirming. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Todd Hebebrand <todd@lanternops.io> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ate limits stop trusting spoofable headers (SR2-16) (#2516) PR 6 of the six-PR core-authentication-hardening epic (#2489), and the last one. **SR2-16** — every security decision that depends on "who is this client" now derives the client IP from a peer we actually trust, or refuses to decide. PRs 1 (#2378), 2 (#2385), 3 (#2492), 4 (#2507) and the MCP hotfix (#2510) are merged. PR 5 (#2514) is open and independent of this one — this branch is based directly on `main` and shares no commits with it. ## What this fixes | Item | Before | After | |---|---|---| | **Allowlist fail-open (the headline)** | `evaluateIpAllowlist` returned `{decision:'skip', reason:'untrusted_ip'}` when no client IP could be derived, and `isBlocked(skip) === false`. An allowlist-protected partner was **silently let through** whenever proxy trust was unconfigured, stale, or a request arrived from an untrusted peer carrying a spoofed forwarded header. | `untrusted_ip` moved from `skip` to `deny`. No trustworthy IP → **no access**, never "waved through". | | **Spoofable rate-limit key** | With no trusted IP, per-client auth rate limits fell back to a `UA + lang + XFF` fingerprint — every input **attacker-controlled**. Rotate any of them and mint an unlimited number of fresh buckets: an uncapped credential-stuffing surface. | Key derives from `ip:<trusted>` → `socket:<TCP peer>` → `fp:<UA/lang/origin>`. The socket peer cannot be forged by a header. **No IP header ever feeds the fingerprint.** | | **Stragglers** | Enrollment and session-creation paths read raw IP headers directly, bypassing the resolver. | All routed through the trusted-proxy resolver. | | **Bundled stack couldn't see client IPs** | Base compose defaulted `TRUST_PROXY_HEADERS=false` while every override operators actually run (`dev`/`ghcr`/`local-build`) already defaulted it to **true**, with no `TRUSTED_PROXY_CIDRS` to pair with it. | Base defaults to `true` with `TRUSTED_PROXY_CIDRS` pinned to the Caddy address we pin ourselves — matching `deploy/docker-compose.prod.yml`. | ## The trust default — why it flipped, and why it does not widen trust This was reviewed as an owner decision and deliberately reversed after review. The reasoning: **A default only binds operators who do not override it.** The bundled stack pins Caddy's address itself, so the API can *name* its own trusted hop rather than guess at one. Declaring trust for an address we assign in our own compose file is not trusting the network — it is stating a fact about our own topology. For an operator fronting the API with **their own** nginx/Traefik, the shipped default is **inert**: their proxy's peer address does not match the pinned CIDR, so no headers are trusted and they get exactly the fail-closed behaviour they had before. They override `TRUSTED_PROXY_CIDRS` with their own proxy's address regardless — that is setup #2 in `.env.example`. **A CIDR that matches nothing trusts nothing.** What `false` cost the bundled majority was not theoretical, and both consequences are created by this PR's own earlier commits: - **Partner IP allowlists become unusable.** Task 1 makes them fail closed without a trusted IP, and the `orgs.ts` enable-gate then (correctly) refuses to arm one. We would ship a security feature switched off by default on the stack we build ourselves. - **All auth rate limits collapse onto one bucket.** With trust off behind the bundled Caddy, the socket peer is *Caddy* for every request, so registration (5/hr), partner-registration (3/hr), invite (10/hr), verification (10/5min) and password-reset (3/window) share a single instance-wide bucket and legitimate users throttle each other. **Neither degradation is spoofable** — that is the point. Trust is what buys per-client granularity, and it is only ever granted to an address we pin. **Upgrades are unaffected.** A `.env` value overrides a compose default, and the old `.env.example` shipped an explicit `TRUST_PROXY_HEADERS=false`. Existing deployments keep whatever their `.env` says and cannot be surprised. The new default reaches only operators who adopt the new `.env.example` or who set neither variable. ## Security properties, proven The fail-closed rule is absolute: **there is no path where an underivable, untrusted, or spoofed client IP yields "allowed"**. A **real-DB integration suite** (`ipAllowlistTrustBoundary.integration.test.ts`) drives the real `enforceIpAllowlist` / `evaluateIpAllowlist` / `getClientRateLimitKey` against real Postgres as the unprivileged `breeze_app` with **forced** RLS, across 8 scenarios spanning direct / Cloudflare / generic-proxy modes. The deny assertions were verified to go **RED** by reverting the `untrusted_ip` branch to `skip`, then restored GREEN — they bite. The compose static guard now bites **for real**. It previously short-circuited on `trustDefault !== 'true'` and *could never fail*; with trust on, it executes. Verified by mutation against the real file — mismatched CIDR, absent CIDR, and removed pin each go RED, and restore GREEN. ## Deploy notes - **No new required env vars.** `BREEZE_DOCKER_SUBNET` / `BREEZE_CADDY_IP` are optional overrides, commented out in `.env.example`. - **`TRUSTED_PROXY_CIDRS` tracks `BREEZE_CADDY_IP`** via nested interpolation, so moving the network cannot strand the pin. Verified against `docker compose config`, including the override-both case. - **Self-host subnet hazard:** the `172.31.0.0/24` default (and prod's `172.30.0.0/24`) sit inside Docker's default bridge pool (`172.17.0.0/12`). A host that has already auto-allocated that range needs `BREEZE_DOCKER_SUBNET` set outside it — previously a hardcoded literal, now movable. - **Break-glass:** `IP_ALLOWLIST_ENFORCEMENT_MODE=off` globally disables allowlist enforcement. - A runtime detector for this class already exists: watch `[proxy-trust] MISCONFIGURATION` in API logs and the `breeze_proxy_trust_untrusted_peer_total` counter — both should stay at zero. ##⚠️ Operator-visible behaviour changes (release notes) 1. **Partner IP allowlists now FAIL CLOSED.** Previously an allowlist silently permitted everything when the API could not see real client IPs. It now denies. Any partner running an allowlist on a deployment without working proxy trust **loses dashboard access** until trust is configured (setup #1/#2 in `.env.example`) or enforcement is broken open with `IP_ALLOWLIST_ENFORCEMENT_MODE=off`. This is the fix, not a regression — but it is the one change that can lock someone out. 2. **The enable-gate cannot protect against trust breaking *later*.** `orgs.ts` refuses to let a partner enable an allowlist while no trusted IP is derivable, which prevents arming a lockout. It only guards the empty→non-empty transition; if trust breaks *after* an allowlist is live (proxy moved, CIDR stale, container recreated without a static IP), the allowlist starts denying. That is what the break-glass switch and the `[proxy-trust]` warnings are for. 3. **`TRUST_PROXY_HEADERS` now defaults to `true`** for the bundled stack, pinned to the bundled Caddy. Existing `.env` files win over compose defaults, so upgrades do not change behaviour. Operators running their own proxy should set `TRUSTED_PROXY_CIDRS=<your-proxy-peer-ip>/32`; those exposing the API directly should set `TRUST_PROXY_HEADERS=false`. ## Known limitation (documented, not fixed here) `enforceIpAllowlist` returns `{skip, no_partner}` when `partnerId` is null, so **org-scoped tokens bypass the partner allowlist entirely**. This is pre-existing and out of scope for SR2-16 — noted for follow-up rather than widened into this PR. ## Testing - `tsc` clean; **98 tests** green across the allowlist service, client-IP resolver, auth helpers, and the compose static guard; **251** green including the config validator suite. - **8/8 real-DB scenarios** green on a correctly-provisioned Postgres (`rolsuper = f` verified — a superuser would make RLS vacuous and pass these for the wrong reason). - Guard-bite verified by actual mutation, not assertion, on both the allowlist deny branch and the compose contract. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Todd Hebebrand <todd@lanternops.io> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-snapshot errors, surface dashboard degraded state (#2556 review) IMPORTANT #1 — agentWs: a malformed or oversize verify/test-restore result set device_commands to failed but early-returned before the per-type handler ran, stranding the backup_verifications / restore_jobs row in running/pending until the 30-min stale-timeout sweep. On a validation rejection we now still dispatch the handler for the verification and restore families so the linked record transitions to a terminal failed state via its normal failure path. IMPORTANT #2 — legacy snapshots with configId=null (predating destination tracking) previously 422'd/threw a misleading "destination configuration not found". Added resolveBackupDestinationError() to backupProviderConfig.ts and threaded it through restore.ts, aiToolsBackup.ts and verificationService.ts to return a distinct "predates backup destination tracking" message (with a reason discriminator), without any current-config fallback that could read the wrong destination. SUGGESTION #3 — dashboard resolveAttentionItems swallowed all errors and returned [], rendering a transient DB failure as "healthy". It now returns a degraded flag; the /dashboard response carries an additive, backward-compatible attentionError signal so the UI can distinguish "no failures" from "couldn't compute". Tests added/updated for all three across agentWs, restore, verificationService and dashboard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#2756) ## Summary Closes #2531. The desktop-helper manager (`internal/helper` — the Tauri tray-app helper) terminated helpers via a **check-then-kill across two separate bare-PID `OpenProcess` calls**: the caller ran `isOurProcess(pid)` (OpenProcess #1, image-path check) and then `stopByPID(pid)` (OpenProcess #2, terminate). If the helper exited and Windows recycled that exact PID onto an unrelated process between the two opens, the check passed on the old process but the terminate landed on the new one. ## Fix Both check-then-kill sites in `manager.go` (`ensureRunningSession` lingering-helper cleanup, and `ensureStoppedSession`) now go through a single **`stopByPIDIfOurs(pid, binaryPath)`**: - **Windows** (`install_windows.go`): open the PID **once** with `PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_TERMINATE`, verify the image path via `QueryFullProcessImageName` on that handle, and `TerminateProcess` on the **same** handle. A Windows handle pins the exact kernel process object, so PID reuse can never redirect the kill; if the PID was already recycled before we opened it, the image check fails and nothing is terminated. This matches the handle-retention standard already used by the session-broker spawner (`internal/sessionbroker/spawner_windows.go`). - **POSIX** (`install_linux.go` / `install_darwin.go`): keeps verify-then-signal via the same helper. POSIX PID allocation is roughly monotonic and does not hand the same number to an unrelated process between two adjacent syscalls, so the reported vulnerability is Windows-specific — documented inline. `isOurProcessFunc` is retained for the pure **liveness** checks (deciding whether to respawn), which are not kills and have no TOCTOU. Scope note: kept intentionally narrow to the `stopByPID` TOCTOU; does not touch the separate scheduled-helper duplication concern (#2530). ## Testing - New `TestStopByPIDIfOurs` (`stop_if_ours_posix_test.go`, `linux || darwin`) starts a real self-owned child and asserts a non-matching image path is **never** killed, a matching path is, and `pid <= 0` is a harmless no-op. - `go build` for `GOOS=windows`, `darwin`, and `linux`; `go vet`; `gofmt`; full `internal/helper` package suite — all green. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TTL cap (LanternOps#2 must-fix): - Add PARTNER_API_ENROLLMENT_KEY_MAX_TTL_MINUTES env var (default 10_080 = 7 days) to cap how long a partner-API-minted enrollment key may be valid. Before this fix the Zod schema validated only against the hardcoded MAX_TTL_MINUTES = 525_600 (1 year), so any partner-API caller could mint year-long keys. - The schema now validates ttlMinutes against PARTNER_API_MAX_TTL_MINUTES; requests above the cap return 400 via zValidator before the handler runs. - Default for ttlMinutes is MIN(ENROLLMENT_KEY_DEFAULT_TTL_MINUTES, cap) to avoid a default that silently exceeds the cap when the cap env var is set low. - Two new tests: cap exceeded returns 400, cap boundary (10_080) returns 201. CRLF (LanternOps#5 must-fix): - Normalize partnerApi/index.ts from CRLF to LF; the original commit stored the file with CRLF line endings, causing the GitHub PR diff to render as a full-file rewrite instead of the two-line change it actually is.
…#3214) (#3224) Closes #3214 ## Root cause — found, reproduced, and pinned The issue listed the `Immediate.nextWrite` TypeError as a *likely* poisoner. It is the whole story, and the mechanism is now reproduced deterministically with no database, no network and no timing luck (`apps/api/src/db/postgresJsPoolPoisoning.test.ts`). In **postgres.js 3.4.9** (`src/connection.js`), small writes are batched behind a `setImmediate`, guarded by an "already scheduled?" check: ```js function write(x, fn) { // :246 chunk = chunk ? Buffer.concat([chunk, x]) : Buffer.from(x) if (fn || chunk.length >= 1024) return nextWrite(fn) nextWriteTimer === null && (nextWriteTimer = setImmediate(nextWrite)) return true } function nextWrite(fn) { // :254 const x = socket.write(chunk, fn) nextWriteTimer !== null && clearImmediate(nextWriteTimer) chunk = nextWriteTimer = null // <- the ONLY reset return x } ``` `nextWrite` actually running is the only thing that ever resets `nextWriteTimer`. But both teardown paths cancel the immediate **without clearing the variable**, and without dropping the buffered `chunk`: ```js function terminate() { ... clearImmediate(nextWriteTimer) ... } // :427 async function closed(hadError) { ... clearImmediate(nextWriteTimer) ... // :440 socket = null ... } ``` So a connection whose socket dies with a deferred write pending is left with `nextWriteTimer` permanently non-null. The guard at `:250` is false forever after, no flush is ever scheduled again, and the `StartupMessage` written by `connected()` on **every** subsequent reconnect is appended to `chunk` and never reaches the socket. The handshake cannot complete, `connect_timeout` expires, `connectTimedOut()` reports `CONNECT_TIMEOUT` and destroys the socket, `closed()` reconnects — forever. That accounts for every symptom in the report: | Observation | Explanation | |---|---| | `TLSSocket.closed` errors starting days early | each one poisons one pool slot | | pool decay 35 → 9 over hours | slots poisoned cumulatively, never recovered | | ~144 `CONNECT_TIMEOUT`/min against a healthy DB | poisoned slots reconnect-looping | | `Immediate.nextWrite` TypeError | the same broken lifecycle (`socket` nulled / stale `chunk`) | | restart fixes it in seconds | fresh closures | **Verification of the repro** — the control (undisturbed connection) writes 77 bytes of `StartupMessage`; kill the socket while the flush is queued and the *reconnect* writes **0 bytes**. ## Suggested fix #1 — answered, and it is a dead end > *check whether postgres.js > 3.4.9 fixes it, or file upstream* **3.4.9 is the latest published release.** `npm view postgres version` → `3.4.9`. There is nothing to bump to, and the broken state lives inside a closure that nothing outside the driver can reach. The reproduction in this PR is dependency-free and ready to attach to an upstream report. ## What this PR ships (fixes #2 and #3) **`apps/api/src/services/dbConnectTimeoutStats.ts`** — a rolling `CONNECT_TIMEOUT` count, fed from `safeDiagnoseConnectTimeout` (the wrapper both production call sites already use). Deduped per error object with a `Symbol`, because `app.onError` and `captureException` classify the *same* error — without that the rate would be silently 2x on the request path and 1x on the worker path, making any threshold meaningless. Zero-import leaf, so it does not recreate the `services → db` back-edge the classifier's docblock warns about. **`apps/api/src/db/dbPoolHealthMonitor.ts`** — the watchdog. A sustained timeout rate on its own is ambiguous: unreachable DB and poisoned pool look identical. So when the rate breaches the threshold it opens **one brand-new connection** to the same database, outside the pool. A fresh client has fresh closures and is immune to the poisoning: - fresh connection **succeeds** → `pool-degraded`: the DB is fine, the pool is not, **restart the API** - fresh connection **fails too** → `database-unreachable`: real fault, **restarting will not help** - probe could not be *attempted* → `unknown`, explicitly refusing to blame either That is exactly the manual step that ended the incident ("`psql` from the same host connected in under a second"), performed automatically. Steady state costs one unref'd timer tick — the probe only opens a socket once the threshold is already breached. **`apps/api/src/routes/metrics.ts`** — `breeze_db_connect_timeouts_total{cause}`, `breeze_db_connect_timeout_rate_per_min`, `breeze_db_pool_health{verdict}`. Seeded at 0 so alert rules never query a nonexistent series. All verdict series stay 0 before the first evaluation — absence of a verdict must not read as `healthy`. Plus `.env.example` + `deploy/environment.mdx` for the six new knobs, and a bootstrap-wiring guard (`dbPoolHealthWiring.test.ts`) — a watchdog that is defined but never started fails completely silently, which is how this class of blindness ships. ## What I did NOT do, and why - **Recycling the `sql` instance.** The issue offers this as optional; I left it out. `db/index.ts` hands the client to Drizzle at module load, and the resulting `baseDb` is captured by the exported proxy, by every open `withDbAccessContext` transaction, and by the AsyncLocalStorage stores. A mid-flight swap would abort in-flight tenant transactions, and a bug in it is a whole-API outage — a worse failure than the one being fixed. Wants its own PR. - **Patching the driver in `node_modules`.** The actual repair is ~2 lines (reset `chunk`/`nextWriteTimer` in `closed()` and `terminate()`), and the pin test would prove it. I left it out: this repo has **no** `patchedDependencies` today, so it would be the first — introduced on the DB driver, touching the Docker and CI install surface, and I cannot validate a driver-lifecycle patch against real production traffic from here. It deserves a dedicated PR with deploy validation. - **Anything that claims to fix the pool.** This PR makes the failure *visible and correctly attributed*. It does not repair it. `pool-degraded` still means someone restarts the API. The pin test is deliberately asserting the **bug**. The day the driver is fixed or patched, it goes red with a message naming exactly what to delete (the pin and the watchdog). ## Review round Three passes (`code-reviewer`, `silent-failure-hunter`, `pr-test-analyzer`) found real defects, all sharing one shape — the change reintroduced, in its own code, the blindness it exists to remove. Fixed in `914926027`: - **`captureException` classified connect timeouts *below* its `initialized` guard.** On any instance without a Sentry DSN — the self-hosted default — `app.onError` was the counter's only feed, so every worker, scheduler and `unhandledRejection` path contributed nothing. That is exactly the incident's profile ("the loudest signature in Sentry was the patch scheduler, not any route"), meaning the watchdog would have reported the pool fine right through it. Classification now happens above the guard. - **The quiet verdict `healthy` is now `below-threshold`.** The count is a documented floor and slot occupancy is never observed, so an affirmative all-clear was a claim the evidence cannot support. There is deliberately no `healthy` series to alert on the negation of. - **A failed evaluation left the previous verdict standing**, so `/metrics` republished a stale reading for hours about a dead watchdog. It now clears the verdict, counts the failure, and reports itself to Sentry (it was console-only). - **A probe timeout under event-loop starvation reported `database-unreachable`** with "restarting will not help" attached — the #3022 misdirection one layer up, since the probe's own budget needs the same blocked loop. Now a distinct error type and an `unknown` verdict. - **In-flight guard + probe budget clamped to half the interval.** Legal env combinations (`INTERVAL=5000, PROBE=30000`) could stack probes, each opening another connection to the database being diagnosed. Probe close failures are now counted, not silently swallowed. - Stable Sentry title (interpolated rates minted a new issue per capture, so no alert bound to one could fire twice), a suppressed-since-last-capture count, and sample retention that trims to the widest window any reader requested rather than the caller's. - **The six `DB_POOL_HEALTH_*` vars are threaded through both compose files.** The docs promise they work, and an unmapped var set in `.env` is silently inert (the IS_HOSTED / #570 failure mode). ### Tests The test pass found that **five mutations survived the entire suite**: deleting the counter feed, the recorder binding, the scrape refresh, the interval body, or the throttle guard all stayed green — i.e. every line of production wiring could be removed without CI noticing. Each is now covered, and I re-applied the mutations to confirm they fail. Also added hermetic tests for the real probe (no database needed) and a **negative control** on the driver pin, so "the reconnect wrote nothing" now demonstrably means "*because* the socket died with a buffered write" rather than for any unrelated reason. ### Second review round (delta re-review) Because the first round's fixes touched `captureException` — every error path in the API — the delta got its own pass. Three more real findings, fixed in `57898f941`: - **The Sentry captures were arriving contentless.** `scrubEvent` deletes `message`, `logentry` and `extra` from *every* outgoing event, and my tag key was not in `ALLOWED_TAG_NAMES` — so it was dropped twice over. A `pool-degraded` alert reached Sentry as an unlabelled blank that grouped with every other blank. The verdict is now allowlisted as `db_pool_health_verdict` (the same precedent `connect_timeout_cause` set for #3022), with a scrubber test proving it survives. The docs no longer claim more than Sentry actually receives. - **The probe-timeout downgrade tested the wrong direction.** It asked "does starvation dominate?", which leaves open exactly the hole it exists to close: with the event-loop monitor disabled or warming up, *every* timeout classifies as `unknown`, so the starvation count is 0, no dominance is found, and the confident `database-unreachable` fires in the configuration with the least evidence. Now inverted to require positive evidence *for* connectivity. - **A throwing `captureMessage` erased a valid verdict.** It fell through to the outer catch, which (correctly) clears `lastAssessment` — so a reporter fault would blank a real degraded verdict from `/metrics` at the moment it fired. Plus: the two failure counters were `Gauge`s carrying the `_total` suffix OpenMetrics reserves for counters (renamed), and the check-failed key never drained its suppressed count. ## Verification - `vitest run src/db src/services src/routes/metrics.test.ts src/config/envComposeParity.test.ts --no-file-parallelism` → **622 files, 10084 passed, 35 skipped** - `tsc --noEmit -p apps/api/tsconfig.json` (with the CI `--max-old-space-size=8192`) → clean - `eslint` on all touched files → clean - Driver pin run 8x consecutively → 8/8 stable (the `setImmediate` ordering is guaranteed by the driver's microtask-only path from socket factory to `connected()`, not merely usual) - Both compose files parse and carry all six new vars 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Todd Hebebrand <todd@lanternops.io> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bumps node from 20-alpine to 25-alpine.
Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot mergewill merge this PR after your CI passes on it@dependabot squash and mergewill squash and merge this PR after your CI passes on it@dependabot cancel mergewill cancel a previously requested merge and block automerging@dependabot reopenwill reopen this PR if it is closed@dependabot closewill close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)