Repository navigation
Security hardening, pipeline scale, alert channels, new check types and opt-in status pages - #109
Merged
Merged
Conversation
Phase 1 of the hardening plan — confirmed defects in shipped code. - monitor.analytics.ts loaded a monitor by bare id with no workspace scoping, so any member of one workspace could read another tenant's uptime/latency analytics. Re-checks against the monitor's own workspace and 404s (not 403s) so it doesn't leak existence. - Razorpay `subscription.charged` is the *success* event but was handled by setting planTier=FREE — every successful renewal downgraded the paying customer. Split into upgrade/downgrade event sets, added a ProcessedPayment replay guard, bound verifyPayment to the workspace that owns the subscription, and made signature compares constant-time. - Mutating monitor/webhook services re-resolved the workspace from the resource and then checked only membership, so an ADMIN of workspace A who was a VIEWER of B could delete B's monitors. Added assertWorkspaceRole and applied it to every mutating service. - No SSRF protection on user-supplied monitor/webhook URLs. Added lib/ssrf.ts (private/loopback/link-local/CGNAT/IPv6-ULA blocking, including ::ffff: mapped addresses), applied at save time and before every outbound request, with manual redirect re-validation. ALLOW_PRIVATE_TARGETS=1 opts out for internal monitoring. - API keys were stored in plaintext. Now sha256-hashed with a non-secret display prefix; migration backfills existing keys so they keep working. Wired the previously-unused requireScope to mutating routes. - Go engine ran with InsecureSkipVerify, so an expired or mismatched certificate reported UP in a product that sells SSL monitoring. Now verifies, and reports *why* it failed. Probe errors were discarded entirely, leaving MonitorCheck.errorMessage permanently null; they are now classified and persisted. Also shared one transport across the pool, drained bodies for keep-alive, and made shutdown actually drain. - docker-compose shipped a working default JWT_SECRET, making the insecure path the happy path. Removed; the API now refuses to boot with that value and mints an ephemeral per-process key when unset so zero-config compose still works.
…ation MonitorCheck was the scaling bottleneck: unbounded, keyed by an Int4 sequence that overflows around 2.1B rows (~1k monitors/year at the 30s minimum interval), and indexed on monitorId and checkedAt separately while every hot query filters by both. - MonitorCheck/WebhookDeliveryLog ids -> BigInt, with a JSON serializer so the wire format stays numeric for values under 2^53 (and degrades to a string rather than silently losing precision beyond it). - Composite (monitorId, checkedAt) index replacing the bare monitorId one; WebhookDeliveryLog had no index at all; Incident.startedAt added. - MonitorCheckDaily rollup table + nextCheckAt/dispatchedAt columns for the scheduler work that follows. - Unique partial index allowing only one active incident per monitor, with existing duplicates resolved (not deleted) in the migration. - getMonitorStatsService did an uncapped findMany and computed percentiles in Node, pulling a monitor's whole history into heap on every request. Now one SQL pass with FILTER + percentile_cont for all three windows. Verified equal to the previous output on real data. - The public status page fetched 90 days x every monitor and grouped in JS, unauthenticated. Now grouped in Postgres (<=90 rows/monitor), cached in Redis for 30s, and rate-limited separately from the global budget.
…shutdown - Scheduler loaded every active monitor each 15s tick and filtered for due-ness in Node. It now claims due monitors in a single UPDATE ... FOR UPDATE SKIP LOCKED against the (isActive, nextCheckAt) index, which also makes the claim atomic: a monitor slower than the tick interval is no longer dispatched twice. Verified: freeing monitors yields exactly one dispatch across three ticks. - Dispatch and heartbeat sweeps now run under a Redis leader lock, so two API replicas no longer double-publish to Kafka or race on applyHeartbeatMiss. Verified: one winner under concurrent contention. - Kafka consumer ran with autoCommit on and swallowed errors, so any transient DB failure advanced the offset and lost the check result silently. Now manual commits (offset left unresolved on failure so the message is redelivered), a DLQ for poison messages, and Redis-backed dedup so redelivery can't double-increment consecutiveFailures. - Only one active incident per monitor is now enforced by a unique partial index; applyCheckResult treats the resulting P2002 as 'someone else opened it', not an error to retry forever. - Redis used maxRetriesPerRequest: null, so an outage made commands hang forever instead of failing — the check-now cooldown hung the request and /health hung on ping(). Now bounded. - /health was the only probe, had no timeouts, and ignored Kafka, so the app reported healthy while silently running no checks. Split into /live (process only, so a slow DB can't get the container killed) and /ready (DB + Redis + dispatch staleness), both timeout-bounded. - Shutdown left the BullMQ worker, both extra Redis connections and Prisma open, with no timeout. Now closes everything in dependency order, isolates failures, and force-exits after 20s. BullMQ jobs also get removeOnComplete/removeOnFail so Redis stops growing forever. Verified: clean exit in 5s with no failed steps.
MonitorCheckDaily existed but nothing populated it. Adds the job that makes it useful, and the retention that makes MonitorCheck finite. - rollupDailyChecks aggregates completed days into MonitorCheckDaily, idempotently (ON CONFLICT recomputes), skipping today since it is still accumulating. - pruneOldChecks deletes raw checks past CHECK_RETENTION_DAYS (default 30) in bounded batches, and only for days that already have a rollup row, so a summary always exists before raw data is dropped. - Runs every 6h under the same leader lock as the other periodic jobs. Set CHECK_RETENTION_DAYS=0 to keep raw checks forever. Verified against real data: rollups match the status-page aggregation exactly, re-running is stable at 40 rows, and a 10-day window pruned 1648 rows leaving the oldest raw check exactly at the boundary with all rollups intact.
The only alert transport was a generic HTTP webhook carrying a PulseOps-shaped payload — so there was no email-on-incident (despite lib/email.ts already existing for invites and resets), and pointing it at a Slack or Discord incoming webhook produced nothing usable. - NotificationChannel + NotificationDeliveryLog models, with one adapter per type rendering the provider's native format: Slack Block Kit, Discord embeds, PagerDuty Events API v2 (dedup_key derived from the incident so recovery auto-resolves the alert), themed HTML email, and the original signed webhook. - Full CRUD + test-send + delivery log API, with secrets write-only: webhook URLs, routing keys and signing secrets are redacted on read. - Circuit breaker: a channel failing 10 consecutive deliveries is auto-paused for an hour instead of being retried on every incident forever. Retries reuse the existing BullMQ queue rather than adding a second mechanism, and jobs now expire instead of filling Redis. - Adapter validation errors surface as 400 with the provider-specific reason, not a bare 500. - Settings UI for managing channels, matching the current design system. The legacy WebhookEndpoint path is untouched and still delivers, so existing endpoints keep working; channels are purely additive and are deliberately not auto-imported, which would double-send every alert. Verified end to end: provider payload rendering against a local receiver, wrong-provider URLs rejected, breaker tripping at 10 and suppressing further attempts, and the full CRUD/test/log/delete flow.
…tion graceThreshold (consecutive failures) was the only debounce, so a flapping monitor could page repeatedly, a long outage went silent after the first alert, and ACKNOWLEDGED recorded neither who nor when. - alertCooldownSeconds (default 5m) debounces repeat alerts per monitor. Recovery notifications are deliberately never suppressed — withholding 'it's back' would leave someone believing the outage is ongoing. - mutedUntil snoozes alerting without pausing the checks themselves. - Suppression applies to the alert, not the incident: incidents are still recorded in full so the timeline stays accurate. - reminderIntervalSeconds (opt-in, 0 by default) re-notifies while an incident stays open, via a leader-locked sweep. - Incident.acknowledgedAt/acknowledgedBy, plus an incident.acknowledged event so channels can report that someone picked it up. - Manual resolve now also dispatches to channels (it only fired legacy webhooks), and both acknowledge and resolve now check role against the incident's own workspace rather than bare membership — the same escalation pattern fixed earlier in the monitor and webhook services. Verified: first outage pages, recovery always pages, a re-fail inside the cooldown is suppressed while still recording the incident, and a muted monitor stays silent.
The Go engine only did HTTP with an exact status-code comparison. - probes.go adds TCP port connect, DNS record resolution (A/AAAA/CNAME/ MX/TXT/NS, with an optional expected-value assertion), and body keyword matching with a bounded read. - statusMatches accepts a code, a class (2xx), a range (200-299) or a list (200,204). A single Int made 'any 2xx' inexpressible. - Target carries the per-type config; probe() dispatches on type. Verified live: TCP 443 up / closed port down, DNS resolve / NXDOMAIN / value-mismatch, keyword present / absent, and all matcher forms.
Backend half of the new check types. - MonitorType gains TCP/DNS/KEYWORD, with per-type columns and a per-monitor sslWarningDays (the threshold was hardcoded at 7 days). - expectedStatusMatch widens the status expectation; NULL falls back to the legacy expectedStatus so existing monitors are unchanged. - Zod asserts the fields each type actually needs, so 'A port is required for TCP monitors' reaches the user instead of a generic 400. - probes.ts implements TCP/DNS in Node for the on-demand check-now path, mirroring the Go engine so both paths agree; keyword assertion and the status matcher fold into the existing HTTP probe. - The dispatcher now claims every non-HEARTBEAT type and ships the per-type config to Kafka. - createMonitorService persisted only a fixed field list, so the new config validated but was silently dropped — caught by the end-to-end check, where a TCP monitor then reported 'requires a port'. Verified: all four types create, persist and check-now correctly, and each per-type validation error surfaces with its own message.
Type picker extends to five options, with per-type fields: port for TCP, record type + optional expected value for DNS, keyword + present/ absent condition for KEYWORD, and a free-text status matcher (2xx, 200-299, 200,204) replacing the number input for HTTP-like checks. The create action forwards only the fields belonging to the selected type, and now surfaces the API's validation message rather than a generic 'Failed to create monitor'.
/status/:slug resolved a workspace by slug and returned every active monitor in it, with no opt-in — a monitor named after an internal hostname became public the moment it was created. - StatusPage + StatusPageMonitor: monitors are published explicitly, with an optional public display name so an internal monitor name never has to be exposed. A workspace with no StatusPage has no public page. - isPublic gates visibility; an unpublished page 404s exactly like a missing one, so a slug's existence isn't confirmed to someone who can't view it. - Published monitors are verified to belong to the workspace, so a page can't be used to expose another tenant's status. - Maintenance windows are surfaced: a monitor inside a declared window reports as planned work and no longer counts as an outage. - Response carries a server-stamped generatedAt; the page previously rendered new Date() client-side and claimed to be fresher than the 30s revalidation allowed. - Edits invalidate the Redis cache for both the old and new slug, so a rename can't leave a stale page served under the previous URL. Migration backfills a published page for every workspace that already had one, with exactly the monitors that were already visible, so no existing public URL goes dark on deploy. Verified: unpublished 404s, aliases win over real names, unpublished monitors don't leak, cross-tenant monitors are rejected with 400, and deletion takes the page offline.
Consumes the new status-page shape: an UNDER_MAINTENANCE system state, a per-monitor maintenance badge instead of a misleading DOWN, the page description, and the server-stamped collection time in place of a render-time timestamp that overstated freshness.
The opt-in API shipped without a way to reach it: a workspace with no StatusPage row had no public page and no means to create one. Adds the config form on /workspaces/:id/status — title, public slug, description, per-monitor publish checkboxes with optional public display names, a publish toggle, and a take-offline action. Gated to owners and admins; the preview now renders only when the page is actually published, so it reflects what visitors really see rather than implying a live page that doesn't exist.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 34564155 | Triggered | Generic High Entropy Secret | 38bc606 | backend/src/lib/jwt.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
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
1 participant
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.
Twelve focused commits covering three themes: closing security and correctness defects, making the check pipeline survive real load, and shipping the alerting / check-type / status-page features those fixes unblocked.
Each commit is self-contained and reviewable on its own, in the order below.
Security & correctness
38bc606fix(security): close tenant isolation, billing, SSRF and TLS defectsWorkspace-access middleware, API-key hashing at rest (plus a payment replay guard), SSRF protection on user-supplied URLs, TLS inspector fixes, and billing controller hardening. Touches the middleware,
lib/ssrf.ts,lib/hash.ts, JWT/session handling and the modules that consumed the leaky paths.Scale & pipeline
b9a082cperf(db): index checks, BigInt PKs, rollup table, and SQL-side aggregationChecks table gets proper indexes and BigInt primary keys, aggregation moves out of Node and into SQL, and a rollup table lands.
lib/bigint-json.tskeeps BigInt serializable over the wire.28c6a2aperf(pipeline): scale scheduling, stop losing results, real health + shutdownRedis leader lock so only one instance schedules, result-loss fixes in the metrics consumer and webhook queue, an honest health check, and graceful shutdown.
c76ad79feat(telemetry): daily rollups and check retentionRetention job that rolls raw checks into dailies and prunes them.
Alerting
34f0e7cfeat(notifications): email, Slack, Discord and PagerDuty alert channelsNew
notifications/module — one file per channel behind a shared dispatch, plus CRUD routes and the workspace-settings channel manager UI.73c2aabfeat(alerts): cooldown, reminders, snooze and acknowledgement attributionalertCooldownSecondsdebounces repeat alerts,mutedUntilsnoozes without pausing checks, a reminder scheduler re-pages on long outages, and ACKNOWLEDGED now records who and when. Recovery notifications are deliberately never suppressed.New check types
08a9c25feat(engine): TCP, DNS and keyword probes, plus a status matcher — Go engine side.62ca369feat(monitors): TCP, DNS and keyword check types — schema, Zod validation, Node probes for check-now, dispatcher.e655e6bfeat(monitors): create-form support for the new check types — frontend create form.expectedStatusMatchwidens status expectations to classes (2xx), ranges (200-299) and lists (200,204); NULL falls back to the legacyexpectedStatus, so existing monitors are unchanged.Status pages
574378efeat(status): opt-in public status pages/status/:slugpreviously exposed every active monitor in a workspace with no opt-in. Monitors are now published explicitly, with optional public display names,isPublicgating, cross-tenant verification, and maintenance-window awareness. The migration backfills a published page for every workspace that already had one, with exactly the monitors that were already visible — no existing public URL goes dark on deploy.5985c40feat(status): render maintenance, description and real freshness — public page consumes the new shape.a90e2e1feat(status): status page configuration UI — owner/admin config form, since the opt-in API shipped with no way to reach it.Migrations
Six, applied in timestamp order:
20260725120000_hash_api_keys_and_payment_replay_guard20260726090000_scale_checks_rollups_and_scheduling20260726140000_notification_channels20260726160000_alert_quality20260726180000_check_typesexpectedStatusMatch20260726200000_status_pagesStatusPage+StatusPageMonitor(with backfill)Testing
Verified per-commit as described in each commit message: all four check types create, persist and check-now correctly with their own validation messages; Go probes verified live (TCP open/closed, DNS resolve/NXDOMAIN/value-mismatch, keyword present/absent, all matcher forms); status pages verified for unpublished 404s, alias precedence, unpublished-monitor leakage, cross-tenant rejection and takedown on delete.