harden auth-free redirect, ssrf pinning, first-admin and quota races, service.name

CHANGELOG.md

@@ -13,6 +13,11 @@ Two correctness follow-ups from the multi-engine bug-hunt sweep (issue #255): Si
 ### Security
 - **Cross-tenant read on the dashboard API endpoints (fixed)**: the five dashboard endpoints (`/api/v1/dashboard/stats`, `/timeseries`, `/top-services`, `/timeline-events`, `/recent-errors`) plus the newer `/activity-overview` took `organizationId` from the query string and only ran the organization-membership check behind `if (request.user?.id)`, which is set for session auth only. For API-key auth `requireFullAccess` lets any non-write key through without setting `request.user`, so the membership check was skipped and the org was read from the attacker-supplied query rather than the key's bound `request.organizationId`; when `projectId` was omitted the project-in-org check was skipped too. A holder of any full-access API key (bound to org A) could read another organization's dashboard data by passing that org's id. All six handlers now route through a shared `resolveDashboardScope` that, for API-key auth, requires the requested org to match the key's bound org and the requested project to match (defaulting to the key's bound project when omitted), mirroring `resolveQueryProjectId` which already protects the query and traces routes. Session auth keeps the org-membership and project-in-org checks. Reported privately via KIberblick.de
 - **Stored XSS via OTLP `service.name` in the service map (fixed)**: `service.name` from ingested traces passed through `sanitizeForPostgres`, which only strips null bytes, so `< > " '` survived into `span.service_name` and were served verbatim by the service-map API. In `ServiceMap.svelte` the ECharts `tooltip.formatter` returned a raw HTML string built from `params.name` / `params.data.source` / `params.data.target`, and ECharts renders tooltip output as HTML, so a `service.name` like `<img src=x onerror=...>` executed in the browser of any operator who opened the project's service map and hovered the node or edge. User-derived tooltip values are now HTML-escaped via a shared `escapeHtml` util (also adopted by the SIEM HTML report builder, replacing its private copy). Stored data is left raw on purpose (escaping at the sink, not the store, avoids double-encoding and keeps the JSON API correct). Reported privately via KIberblick.de
+- **Open redirect on auth-free login/register (fixed)**: in `authMode === 'none'` deployments the login and register pages forwarded the user-supplied `redirect` query parameter via `goto(redirectUrl)` with no validation, while the normal submit path already checked it. The check now lives in a shared `isSafeInternalPath`/`safeRedirect` helper used by both the auth-free and normal paths on both pages; it requires a single-leading-slash path and rejects protocol-relative forms including the backslash variant (`/\evil.com`) that some browsers normalize to `//`. Reported privately via KIberblick.de
+- **SSRF guard now pins the validated IP (DNS rebinding hardening)**: `safeFetch` resolved and validated the target host, then let `fetch()` re-resolve at connect time, leaving a resolve-then-connect window where a hostname could rebind to an internal address between check and connect (the TCP monitor path already pinned, the HTTP path did not). The HTTP(S) path now connects through a per-request undici dispatcher whose lookup is pinned to the already-validated address, so the socket reaches the exact IP that passed validation; TLS SNI and certificate validation still use the original hostname. Reported privately via KIberblick.de
+- **First-admin bootstrap race (fixed)**: `createUser` decided the automatic first-admin promotion with a non-atomic `hasAnyAdmin()` check followed by a separate insert, so concurrent registrations in the zero-admin window could each observe "no admin yet" and all be promoted. The check-then-insert now runs inside a transaction holding a Postgres advisory lock, so at most one registration wins the promotion. Only reachable before the first admin exists (and closed entirely when `INITIAL_ADMIN_*` is set). Reported privately via KIberblick.de
+- **Capability limit check-then-act race (fixed)**: resource-creating routes (api keys, custom dashboards, alert rules, sigma rule import/enable, notification channels) ran `COUNT -> assertWithinLimit -> insert` without serialization, so parallel requests could each read a count under the limit and then all insert, exceeding a configured finite cap (a quota bypass, not a tenant boundary; the OSS default has no finite limits). The count+create now runs through a shared `withLimitLock` helper that takes a per-organization, per-capability transaction-scoped advisory lock, so concurrent creators of the same resource type serialize and the cap holds. Reported privately via KIberblick.de
+- **Defense-in-depth on OTLP `service.name` at ingestion**: complementing the service-map output-encoding fix above, ingested `service.name` (for logs, spans and metrics) is now run through a shared `sanitizeServiceName` that strips control characters (C0/DEL/C1, including null bytes) and caps the length, while deliberately preserving otherwise legitimate characters (escaping still happens at each sink). This keeps a raw payload from resurfacing through a sink that is added later or forgets to encode. Suggested by KIberblick.de
 - **PII masking now also covers trace span attributes**: masking was wired only into log ingestion, so trace spans were stored with their attributes verbatim. Spans routinely carry `http.request_body` / `http.response_body` (plaintext credentials, JWTs), `net.peer.ip` and user agents, all persisted unmasked. `tracesService.ingestSpans` now runs the same org/project masking rules over each span's `attributes`, `resourceAttributes` and event/link attributes before storage, and drops (fail-closed) any span whose masking throws. Because request/response bodies are opaque stringified JSON that field-name rules can't see into, those body attributes are deep-masked (parse the JSON, mask `password`/`token`/email inside, re-serialize) with full redaction as a fallback when the value is not parseable JSON. Metric attributes are not yet masked (tracked separately)
 
 ### Added

SECURITY.md

@@ -37,6 +37,7 @@ We thank the following researchers for responsibly disclosing security issues:
 
 - **Bertie** — cross-tenant authorization gaps in project-scoped routes (alert preview/creation, monitor creation, source map list/delete) and SSRF / internal port-scanning via HTTP/TCP monitors and webhook delivery. Fixed in 0.9.6.
 - **tonghuaroot** — SSRF in the alert/Sigma webhook delivery path, which still used the bypassable inline filter instead of the centralized `safeFetch` guard (incomplete-fix sibling-gap of the 0.9.6 hardening). Fixed in 0.9.7. (GHSA-7v53-pw6r-99vj)
+- **KIberblick.de** ([kiberblick.de](https://kiberblick.de)) — cross-tenant read on the dashboard API endpoints (organization taken from the attacker-supplied query string under API-key auth) and stored XSS via OTLP `service.name` in the service map; plus an open redirect on the auth-free login/register path, a DNS-rebinding gap in the SSRF guard's HTTP path, a first-admin bootstrap promotion race, and a capability-limit check-then-act race. Fixed in 1.0.3.
 
 ## Supported Versions
 

packages/backend/package.json

@@ -70,6 +70,7 @@
     "safe-regex2": "^5.0.0",
     "source-map": "^0.7.6",
     "tsx": "^4.21.0",
+    "undici": "^7.28.0",
     "zod": "^3.25.76"
   },
   "devDependencies": {

packages/backend/src/capabilities/index.ts

@@ -32,3 +32,4 @@ export {
 
 export { quotaFlagCache } from './quota-cache.js';
 export { QuotaEvaluator } from './quota-evaluator.js';
+export { withLimitLock } from './limit-lock.js';

packages/backend/src/capabilities/limit-lock.ts

@@ -0,0 +1,73 @@
+import { sql } from 'kysely';
+import { db } from '../database/connection.js';
+
+// Namespace for the (int4, int4) Postgres advisory lock keyspace used by
+// capability limit enforcement, kept distinct from other advisory locks.
+const CAP_LOCK_NAMESPACE = 0x4c54; // 'LT'
+
+/** Deterministic 32-bit signed hash (FNV-1a) for advisory lock keys. */
+function hash32(input: string): number {
+  let h = 2166136261;
+  for (let i = 0; i < input.length; i++) {
+    h ^= input.charCodeAt(i);
+    h = Math.imul(h, 16777619);
+  }
+  return h | 0; // coerce to signed int32 for pg_advisory_xact_lock(int4, int4)
+}
+
+// In-process serialization tails, keyed by org+capability. This bounds the
+// number of callers that block on the DB advisory lock to ONE per key per
+// process, so concurrent requests don't each hold a transaction connection
+// while waiting (which would exhaust the pool).
+const localTails = new Map<string, Promise<void>>();
+
+async function runExclusiveInProcess<T>(key: string, fn: () => Promise<T>): Promise<T> {
+  const prevTail = localTails.get(key) ?? Promise.resolve();
+  let release!: () => void;
+  const tail = new Promise<void>((resolve) => {
+    release = resolve;
+  });
+  localTails.set(key, tail);
+  await prevTail; // wait for the previous holder of this key
+  try {
+    return await fn();
+  } finally {
+    release();
+    // Drop the entry once we are the last in line, to keep the map bounded.
+    if (localTails.get(key) === tail) localTails.delete(key);
+  }
+}
+
+/**
+ * Serialize a "count current usage -> assert under limit -> create" sequence
+ * against concurrent callers for the same (organization, capability).
+ *
+ * The enforcement pattern (COUNT -> assertWithinLimit -> insert) is a
+ * check-then-act: without serialization, parallel requests can each read a count
+ * below the limit and then all insert, pushing usage past a finite cap.
+ *
+ * Serialization happens at two levels: an in-process mutex per org+capability
+ * (so within one backend instance only one such create runs at a time, and
+ * waiters do not hold a database connection while queued), wrapping a
+ * transaction-scoped Postgres advisory lock on the same key (so the guarantee
+ * also holds across multiple backend instances sharing the database). The
+ * advisory lock is released automatically on commit/rollback.
+ *
+ * Different organizations and different capabilities use distinct keys and do
+ * not contend. When no finite limit is configured (the OSS default) the only
+ * added cost is one advisory lock/unlock round-trip.
+ */
+export async function withLimitLock<T>(
+  organizationId: string,
+  capabilityKey: string,
+  fn: () => Promise<T>,
+): Promise<T> {
+  const key = `${organizationId}:${capabilityKey}`;
+  const key2 = hash32(key);
+  return runExclusiveInProcess(key, () =>
+    db.transaction().execute(async (trx) => {
+      await sql`SELECT pg_advisory_xact_lock(${CAP_LOCK_NAMESPACE}, ${key2})`.execute(trx);
+      return fn();
+    }),
+  );
+}

packages/backend/src/modules/alerts/routes.ts

@@ -8,7 +8,7 @@ import { OrganizationsService } from '../organizations/service.js';
 import { projectsService } from '../projects/service.js';
 import { notificationChannelsService } from '../notification-channels/index.js';
 import { auditLogService } from '../audit-log/index.js';
-import { assertWithinLimit } from '../../capabilities/index.js';
+import { assertWithinLimit, withLimitLock } from '../../capabilities/index.js';
 
 const organizationsService = new OrganizationsService();
 
@@ -184,26 +184,27 @@ export async function alertsRoutes(fastify: FastifyInstance) {
       // Session-auth requests don't populate request.organizationId in the ALS
       // context, so we establish a scoped system context with the org from the
       // validated body. Same pattern as otlp/trace-routes.ts.
-      // Note: count -> insert is not atomic; a concurrent create can briefly
-      // exceed the cap by one. Acceptable for user-initiated rule creation.
-      await context.runAsSystem('alerts:create-limit-check', async () => {
-        await context.with({ organizationId: body.organizationId }, async () => {
-          const currentRuleCount = await alertsService.countAlertRules(body.organizationId);
-          await assertWithinLimit('alerts.max_rules', currentRuleCount);
-        });
-      });
-
+      // The count -> insert is serialized per org via withLimitLock so concurrent
+      // creates can't race past the cap.
       const { channelIds, alertType, baselineType, deviationMultiplier, minBaselineValue, cooldownMinutes, sustainedMinutes, metadataFilters, ...alertData } = body;
-      const alertRule = await alertsService.createAlertRule({
-        ...alertData,
-        alertType: alertType || 'threshold',
-        baselineType: baselineType || null,
-        deviationMultiplier: deviationMultiplier ?? null,
-        minBaselineValue: minBaselineValue ?? null,
-        cooldownMinutes: cooldownMinutes ?? null,
-        sustainedMinutes: sustainedMinutes ?? null,
-        emailRecipients: alertData.emailRecipients || [],
-        metadataFilters: metadataFilters ?? [],
+      const alertRule = await withLimitLock(body.organizationId, 'alerts.max_rules', async () => {
+        await context.runAsSystem('alerts:create-limit-check', async () => {
+          await context.with({ organizationId: body.organizationId }, async () => {
+            const currentRuleCount = await alertsService.countAlertRules(body.organizationId);
+            await assertWithinLimit('alerts.max_rules', currentRuleCount);
+          });
+        });
+        return alertsService.createAlertRule({
+          ...alertData,
+          alertType: alertType || 'threshold',
+          baselineType: baselineType || null,
+          deviationMultiplier: deviationMultiplier ?? null,
+          minBaselineValue: minBaselineValue ?? null,
+          cooldownMinutes: cooldownMinutes ?? null,
+          sustainedMinutes: sustainedMinutes ?? null,
+          emailRecipients: alertData.emailRecipients || [],
+          metadataFilters: metadataFilters ?? [],
+        });
       });
 
       // Associate channels with the alert rule

packages/backend/src/modules/api-keys/routes.ts

@@ -6,7 +6,7 @@ import { apiKeysService } from './service.js';
 import { authenticate } from '../auth/middleware.js';
 import { projectsService } from '../projects/service.js';
 import { auditLogService } from '../audit-log/index.js';
-import { assertWithinLimit } from '../../capabilities/index.js';
+import { assertWithinLimit, withLimitLock } from '../../capabilities/index.js';
 import { CapabilityError } from '../../capabilities/errors.js';
 
 const createApiKeySchema = z.object({
@@ -72,18 +72,20 @@ export async function apiKeysRoutes(fastify: FastifyInstance) {
         });
       }
 
-      await context.runAsSystem('apikeys:create-limit-check', async () => {
-        await context.with({ organizationId: project.organizationId }, async () => {
-          const count = await apiKeysService.countKeysForOrg(project.organizationId);
-          await assertWithinLimit('apikeys.max', count);
+      const result = await withLimitLock(project.organizationId, 'apikeys.max', async () => {
+        await context.runAsSystem('apikeys:create-limit-check', async () => {
+          await context.with({ organizationId: project.organizationId }, async () => {
+            const count = await apiKeysService.countKeysForOrg(project.organizationId);
+            await assertWithinLimit('apikeys.max', count);
+          });
         });
-      });
 
-      const result = await apiKeysService.createApiKey({
-        projectId,
-        name: body.name,
-        type: body.type,
-        allowedOrigins: body.allowedOrigins ?? null,
+        return apiKeysService.createApiKey({
+          projectId,
+          name: body.name,
+          type: body.type,
+          allowedOrigins: body.allowedOrigins ?? null,
+        });
       });
 
       await auditLogService.record({

packages/backend/src/modules/custom-dashboards/routes.ts

@@ -6,7 +6,7 @@ import { customDashboardsService } from './service.js';
 import { panelInstanceSchema } from './panel-registry.js';
 import { fetchPanelData } from './panel-data-service.js';
 import { context } from '@logtide/shared/context';
-import { assertWithinLimit } from '../../capabilities/index.js';
+import { assertWithinLimit, withLimitLock } from '../../capabilities/index.js';
 import { CapabilityError } from '../../capabilities/errors.js';
 import { auditLogService } from '../audit-log/service.js';
 
@@ -92,24 +92,26 @@ export async function customDashboardsRoutes(fastify: FastifyInstance) {
         return reply.status(403).send({ error: 'Forbidden' });
       }
 
-      await context.runAsSystem('dashboards:create-limit-check', async () => {
-        await context.with({ organizationId: body.organizationId }, async () => {
-          const count = await customDashboardsService.countForOrg(body.organizationId);
-          await assertWithinLimit('dashboards.max_custom', count);
+      const dashboard = await withLimitLock(body.organizationId, 'dashboards.max_custom', async () => {
+        await context.runAsSystem('dashboards:create-limit-check', async () => {
+          await context.with({ organizationId: body.organizationId }, async () => {
+            const count = await customDashboardsService.countForOrg(body.organizationId);
+            await assertWithinLimit('dashboards.max_custom', count);
+          });
         });
-      });
 
-      const dashboard = await customDashboardsService.create(
-        {
-          organizationId: body.organizationId,
-          projectId: body.projectId ?? null,
-          name: body.name,
-          description: body.description ?? null,
-          isPersonal: body.isPersonal,
-          panels: body.panels,
-        },
-        request.user.id
-      );
+        return customDashboardsService.create(
+          {
+            organizationId: body.organizationId,
+            projectId: body.projectId ?? null,
+            name: body.name,
+            description: body.description ?? null,
+            isPersonal: body.isPersonal,
+            panels: body.panels,
+          },
+          request.user.id
+        );
+      });
 
       await auditLogService.record({
         action: 'dashboard.created',
@@ -140,18 +142,20 @@ export async function customDashboardsRoutes(fastify: FastifyInstance) {
         return reply.status(403).send({ error: 'Forbidden' });
       }
 
-      await context.runAsSystem('dashboards:create-limit-check', async () => {
-        await context.with({ organizationId: body.organizationId }, async () => {
-          const count = await customDashboardsService.countForOrg(body.organizationId);
-          await assertWithinLimit('dashboards.max_custom', count);
+      const dashboard = await withLimitLock(body.organizationId, 'dashboards.max_custom', async () => {
+        await context.runAsSystem('dashboards:create-limit-check', async () => {
+          await context.with({ organizationId: body.organizationId }, async () => {
+            const count = await customDashboardsService.countForOrg(body.organizationId);
+            await assertWithinLimit('dashboards.max_custom', count);
+          });
         });
-      });
 
-      const dashboard = await customDashboardsService.importYaml(
-        body.yaml,
-        body.organizationId,
-        request.user.id
-      );
+        return customDashboardsService.importYaml(
+          body.yaml,
+          body.organizationId,
+          request.user.id
+        );
+      });
 
       await auditLogService.record({
         action: 'dashboard.imported',