diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d472147..59391d63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 Two correctness follow-ups from the multi-engine bug-hunt sweep (issue #255): Sigma detection now honors full SigmaHQ field-modifier chains, and the service-map p95 is a true window percentile on every storage engine. No database migrations; drop-in upgrade. The storage-layer change was validated against real ClickHouse, MongoDB and TimescaleDB. This line also fixes two operational bugs: a Redis memory leak where completed/failed BullMQ jobs were never evicted, and a nightly SigmaHQ sync that re-imported the whole catalog as enabled and auto-created alert rules. Plus a few frontend touch-ups: theme-aware trace/session IDs in the log detail, per-occurrence trace links on the error page, metadata copy buttons, a breadcrumbs timeline and nested metadata columns in log search. ### 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 `` 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 - **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 diff --git a/packages/backend/src/modules/dashboard/routes.ts b/packages/backend/src/modules/dashboard/routes.ts index 3c069d79..1b66f534 100644 --- a/packages/backend/src/modules/dashboard/routes.ts +++ b/packages/backend/src/modules/dashboard/routes.ts @@ -37,6 +37,76 @@ async function verifyProjectBelongsToOrg(projectId: string, organizationId: stri return !!result; } +/** Returned when access was denied and a response has already been sent. */ +const SCOPE_DENIED = Symbol('dashboard-scope-denied'); + +/** + * Resolve and authorize the tenant scope for a dashboard request. + * + * Session auth (request.user set): the user must be a member of the requested + * organization; a provided projectId must belong to that organization. + * + * API-key auth (no user; request.organizationId/projectId bound by the auth + * plugin): the requested organizationId MUST match the key's bound organization + * and a provided projectId MUST match the key's bound project. When projectId is + * omitted it defaults to the key's bound project, so a project-scoped key can + * never read org-wide or cross-org data. This mirrors resolveQueryProjectId, + * which already protects the query and traces routes. + * + * Returns the effective projectId to scope on (string | undefined), or + * SCOPE_DENIED if a 403/404 was already sent. + */ +async function resolveDashboardScope( + request: any, + reply: any, + organizationId: string, + projectId?: string, +): Promise { + // Session-based auth: org-wide membership check. + if (request.user?.id) { + const hasAccess = await verifyOrganizationAccess(organizationId, request.user.id); + if (!hasAccess) { + reply.code(403).send({ + error: 'Access denied - you are not a member of this organization', + }); + return SCOPE_DENIED; + } + + if (projectId) { + const belongsToOrg = await verifyProjectBelongsToOrg(projectId, organizationId); + if (!belongsToOrg) { + reply.code(404).send({ error: 'Project not found in this organization' }); + return SCOPE_DENIED; + } + } + + return projectId; + } + + // API-key auth: the key is bound to a single org/project by the auth plugin. + // Enforce that the requested org/project match the key's bound values. + const boundOrg = request.organizationId; + const boundProject = request.projectId; + + if (boundOrg && organizationId !== boundOrg) { + reply.code(403).send({ + error: 'Access denied - API key is not bound to this organization', + }); + return SCOPE_DENIED; + } + + if (boundProject && projectId && projectId !== boundProject) { + reply.code(403).send({ + error: 'Access denied - API key is not bound to this project', + }); + return SCOPE_DENIED; + } + + // Default to the key's bound project so a project-scoped key cannot read + // org-wide data (mirrors resolveQueryProjectId). + return projectId ?? boundProject; +} + const dashboardRoutes: FastifyPluginAsync = async (fastify) => { // GET /api/v1/dashboard/stats - Get dashboard statistics fastify.get('/api/v1/dashboard/stats', { @@ -63,26 +133,10 @@ const dashboardRoutes: FastifyPluginAsync = async (fastify) => { }); } - // SECURITY: Verify user is member of this organization (if using session auth) - if (request.user?.id) { - const hasAccess = await verifyOrganizationAccess(organizationId, request.user.id); - - if (!hasAccess) { - return reply.code(403).send({ - error: 'Access denied - you are not a member of this organization', - }); - } - } - - // Verify project belongs to org if specified - if (projectId) { - const belongsToOrg = await verifyProjectBelongsToOrg(projectId, organizationId); - if (!belongsToOrg) { - return reply.code(404).send({ error: 'Project not found in this organization' }); - } - } + const scope = await resolveDashboardScope(request, reply, organizationId, projectId); + if (scope === SCOPE_DENIED) return; - const stats = await dashboardService.getStats(organizationId, projectId); + const stats = await dashboardService.getStats(organizationId, scope); return stats; }, }); @@ -112,25 +166,10 @@ const dashboardRoutes: FastifyPluginAsync = async (fastify) => { }); } - // SECURITY: Verify user is member of this organization (if using session auth) - if (request.user?.id) { - const hasAccess = await verifyOrganizationAccess(organizationId, request.user.id); - - if (!hasAccess) { - return reply.code(403).send({ - error: 'Access denied - you are not a member of this organization', - }); - } - } + const scope = await resolveDashboardScope(request, reply, organizationId, projectId); + if (scope === SCOPE_DENIED) return; - if (projectId) { - const belongsToOrg = await verifyProjectBelongsToOrg(projectId, organizationId); - if (!belongsToOrg) { - return reply.code(404).send({ error: 'Project not found in this organization' }); - } - } - - const timeseries = await dashboardService.getTimeseries(organizationId, projectId); + const timeseries = await dashboardService.getTimeseries(organizationId, scope); return { timeseries }; }, }); @@ -164,27 +203,14 @@ const dashboardRoutes: FastifyPluginAsync = async (fastify) => { return reply.code(400).send({ error: 'organizationId is required' }); } - if (request.user?.id) { - const hasAccess = await verifyOrganizationAccess(organizationId, request.user.id); - if (!hasAccess) { - return reply.code(403).send({ - error: 'Access denied - you are not a member of this organization', - }); - } - } - - if (projectId) { - const belongsToOrg = await verifyProjectBelongsToOrg(projectId, organizationId); - if (!belongsToOrg) { - return reply.code(404).send({ error: 'Project not found in this organization' }); - } - } + const scope = await resolveDashboardScope(request, reply, organizationId, projectId); + if (scope === SCOPE_DENIED) return; const config: ActivityOverviewConfig = { type: 'activity_overview', title: 'Activity Overview', source: 'mixed', - projectId: projectId ?? null, + projectId: scope ?? null, timeRange: timeRange ?? '24h', series: ACTIVITY_OVERVIEW_SERIES, }; @@ -223,25 +249,10 @@ const dashboardRoutes: FastifyPluginAsync = async (fastify) => { }); } - // SECURITY: Verify user is member of this organization (if using session auth) - if (request.user?.id) { - const hasAccess = await verifyOrganizationAccess(organizationId, request.user.id); - - if (!hasAccess) { - return reply.code(403).send({ - error: 'Access denied - you are not a member of this organization', - }); - } - } + const scope = await resolveDashboardScope(request, reply, organizationId, projectId); + if (scope === SCOPE_DENIED) return; - if (projectId) { - const belongsToOrg = await verifyProjectBelongsToOrg(projectId, organizationId); - if (!belongsToOrg) { - return reply.code(404).send({ error: 'Project not found in this organization' }); - } - } - - const services = await dashboardService.getTopServices(organizationId, limit || 5, projectId); + const services = await dashboardService.getTopServices(organizationId, limit || 5, scope); return { services }; }, }); @@ -271,24 +282,10 @@ const dashboardRoutes: FastifyPluginAsync = async (fastify) => { }); } - if (request.user?.id) { - const hasAccess = await verifyOrganizationAccess(organizationId, request.user.id); - - if (!hasAccess) { - return reply.code(403).send({ - error: 'Access denied - you are not a member of this organization', - }); - } - } + const scope = await resolveDashboardScope(request, reply, organizationId, projectId); + if (scope === SCOPE_DENIED) return; - if (projectId) { - const belongsToOrg = await verifyProjectBelongsToOrg(projectId, organizationId); - if (!belongsToOrg) { - return reply.code(404).send({ error: 'Project not found in this organization' }); - } - } - - const events = await dashboardService.getTimelineEvents(organizationId, projectId); + const events = await dashboardService.getTimelineEvents(organizationId, scope); return { events }; }, }); @@ -318,25 +315,10 @@ const dashboardRoutes: FastifyPluginAsync = async (fastify) => { }); } - // SECURITY: Verify user is member of this organization (if using session auth) - if (request.user?.id) { - const hasAccess = await verifyOrganizationAccess(organizationId, request.user.id); - - if (!hasAccess) { - return reply.code(403).send({ - error: 'Access denied - you are not a member of this organization', - }); - } - } - - if (projectId) { - const belongsToOrg = await verifyProjectBelongsToOrg(projectId, organizationId); - if (!belongsToOrg) { - return reply.code(404).send({ error: 'Project not found in this organization' }); - } - } + const scope = await resolveDashboardScope(request, reply, organizationId, projectId); + if (scope === SCOPE_DENIED) return; - const errors = await dashboardService.getRecentErrors(organizationId, projectId); + const errors = await dashboardService.getRecentErrors(organizationId, scope); return { errors }; }, }); diff --git a/packages/backend/src/tests/modules/dashboard/routes.test.ts b/packages/backend/src/tests/modules/dashboard/routes.test.ts index 5c6b3ba5..c045a10f 100644 --- a/packages/backend/src/tests/modules/dashboard/routes.test.ts +++ b/packages/backend/src/tests/modules/dashboard/routes.test.ts @@ -2,7 +2,14 @@ import { describe, it, expect, beforeEach, afterAll, beforeAll } from 'vitest'; import Fastify, { FastifyInstance } from 'fastify'; import { db } from '../../../database/index.js'; import dashboardRoutes from '../../../modules/dashboard/routes.js'; -import { createTestContext, createTestLog } from '../../helpers/factories.js'; +import { + createTestContext, + createTestLog, + createTestUser, + createTestOrganization, + createTestProject, + createTestApiKey, +} from '../../helpers/factories.js'; import crypto from 'crypto'; async function createTestSession(userId: string) { @@ -34,6 +41,25 @@ describe('Dashboard Routes', () => { // but rely on request.user being set. We register the routes and // add a mock auth hook that sets request.user from the session. app.addHook('onRequest', async (request: any) => { + // API-key auth: mirror the real auth plugin - bind project/org from the + // key, do NOT set request.user. + const apiKey = request.headers['x-api-key']; + if (apiKey) { + const keyHash = crypto.createHash('sha256').update(apiKey).digest('hex'); + const row = await db + .selectFrom('api_keys') + .innerJoin('projects', 'projects.id', 'api_keys.project_id') + .select(['api_keys.project_id', 'api_keys.type', 'projects.organization_id']) + .where('api_keys.key_hash', '=', keyHash) + .executeTakeFirst(); + if (row) { + request.projectId = row.project_id; + request.organizationId = row.organization_id; + request.apiKeyType = row.type; + } + return; + } + const authHeader = request.headers.authorization; if (!authHeader) return; @@ -491,4 +517,103 @@ describe('Dashboard Routes', () => { expect(res.statusCode).toBe(404); }); }); + + // ========================================================================= + // API-key tenant isolation (regression for cross-tenant dashboard access) + // + // A full-access API key is PROJECT-scoped. It must only read its own org's + // (and project's) dashboard data, never another organization's, regardless + // of the organizationId/projectId passed in the query string. + // ========================================================================= + describe('API-key tenant isolation', () => { + const ENDPOINTS = [ + '/api/v1/dashboard/stats', + '/api/v1/dashboard/timeseries', + '/api/v1/dashboard/top-services', + '/api/v1/dashboard/timeline-events', + '/api/v1/dashboard/recent-errors', + '/api/v1/dashboard/activity-overview', + ]; + + async function buildOtherOrg() { + const owner = await createTestUser(); + const org = await createTestOrganization({ ownerId: owner.id }); + const project = await createTestProject({ organizationId: org.id, userId: owner.id }); + const apiKey = await createTestApiKey({ projectId: project.id }); + return { org, project, apiKey }; + } + + it('rejects a key bound to org A reading org B with 403 on every endpoint', async () => { + // testOrganization is org A; its key: + const orgAKey = await createTestApiKey({ projectId: testProject.id }); + const other = await buildOtherOrg(); // org B + + for (const url of ENDPOINTS) { + const res = await app.inject({ + method: 'GET', + url: `${url}?organizationId=${other.org.id}`, + headers: { 'x-api-key': orgAKey.plainKey }, + }); + expect(res.statusCode, `${url} should be 403`).toBe(403); + } + }); + + it('rejects a key reading another org even when its own projectId is passed', async () => { + const orgAKey = await createTestApiKey({ projectId: testProject.id }); + const other = await buildOtherOrg(); + + const res = await app.inject({ + method: 'GET', + url: `/api/v1/dashboard/stats?organizationId=${other.org.id}&projectId=${testProject.id}`, + headers: { 'x-api-key': orgAKey.plainKey }, + }); + expect(res.statusCode).toBe(403); + }); + + it('rejects a key passing a foreign projectId within its own org with 403', async () => { + const orgAKey = await createTestApiKey({ projectId: testProject.id }); + const other = await buildOtherOrg(); + + const res = await app.inject({ + method: 'GET', + url: `/api/v1/dashboard/stats?organizationId=${testOrganization.id}&projectId=${other.project.id}`, + headers: { 'x-api-key': orgAKey.plainKey }, + }); + expect(res.statusCode).toBe(403); + }); + + it('allows a key to read its own organization', async () => { + const orgAKey = await createTestApiKey({ projectId: testProject.id }); + + const res = await app.inject({ + method: 'GET', + url: `/api/v1/dashboard/stats?organizationId=${testOrganization.id}`, + headers: { 'x-api-key': orgAKey.plainKey }, + }); + expect(res.statusCode).toBe(200); + }); + + it('scopes a key to its own project: only its project data is counted', async () => { + // Second project in the same org with its own logs. + const otherProject = await createTestProject({ + organizationId: testOrganization.id, + userId: testUser.id, + }); + await createTestLog({ projectId: otherProject.id, level: 'info' }); + await createTestLog({ projectId: testProject.id, level: 'info' }); + + const orgAKey = await createTestApiKey({ projectId: testProject.id }); + + // No projectId in query: must default to the key's bound project, + // so only the single log in testProject is counted (not both). + const res = await app.inject({ + method: 'GET', + url: `/api/v1/dashboard/stats?organizationId=${testOrganization.id}`, + headers: { 'x-api-key': orgAKey.plainKey }, + }); + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.payload); + expect(body.totalLogsToday.value).toBe(1); + }); + }); }); diff --git a/packages/frontend/src/lib/components/ServiceMap.svelte b/packages/frontend/src/lib/components/ServiceMap.svelte index 79c894b4..371b33a6 100644 --- a/packages/frontend/src/lib/components/ServiceMap.svelte +++ b/packages/frontend/src/lib/components/ServiceMap.svelte @@ -4,6 +4,7 @@ import type { ServiceDependencies, EnrichedServiceDependencies } from "$lib/api/traces"; import { themeStore } from "$lib/stores/theme"; import { getEChartsTheme, getTooltipStyle } from "$lib/utils/echarts-theme"; + import { escapeHtml } from "$lib/utils/html"; interface Props { dependencies: ServiceDependencies | EnrichedServiceDependencies; @@ -138,7 +139,7 @@ formatter: (params: any) => { if (params.dataType === "node") { const node = dependencies.nodes.find((n) => n.name === params.name); - let html = `${params.name}
Calls: ${params.value}`; + let html = `${escapeHtml(params.name)}
Calls: ${params.value}`; if (node && isEnrichedNode(node)) { html += `
Error rate: ${(node.errorRate * 100).toFixed(1)}%`; html += `
Avg latency: ${formatLatency(node.avgLatencyMs)}`; @@ -148,7 +149,7 @@ const edge = dependencies.edges.find( (e) => e.source === params.data.source && e.target === params.data.target ); - let html = `${params.data.source} → ${params.data.target}
Calls: ${params.data.value}`; + let html = `${escapeHtml(params.data.source)} → ${escapeHtml(params.data.target)}
Calls: ${params.data.value}`; if (edge && isEnrichedEdge(edge) && edge.type === 'log_correlation') { html += `
(log correlation)`; } diff --git a/packages/frontend/src/lib/utils/html.test.ts b/packages/frontend/src/lib/utils/html.test.ts new file mode 100644 index 00000000..aee82f76 --- /dev/null +++ b/packages/frontend/src/lib/utils/html.test.ts @@ -0,0 +1,36 @@ +import { describe, it, expect } from 'vitest'; +import { escapeHtml } from './html'; + +describe('escapeHtml', () => { + it('escapes the five HTML-significant characters', () => { + expect(escapeHtml('&')).toBe('&'); + expect(escapeHtml('<')).toBe('<'); + expect(escapeHtml('>')).toBe('>'); + expect(escapeHtml('"')).toBe('"'); + expect(escapeHtml("'")).toBe('''); + }); + + it('neutralizes an XSS payload smuggled through a service name', () => { + const payload = ''; + const escaped = escapeHtml(payload); + expect(escaped).not.toContain(' { }; } -function escapeHtml(text: string): string { - return text - .replace(/&/g, '&') - .replace(//g, '>') - .replace(/"/g, '"') - .replace(/'/g, '''); -} - function formatHistoryAction(entry: IncidentHistoryEntry): string { switch (entry.action) { case 'created':