From 65446fa47b25948366b92ed4afa5f96786646ba9 Mon Sep 17 00:00:00 2001 From: Makisuo Date: Thu, 12 Nov 2026 00:32:03 +0100 Subject: [PATCH 1/3] fix(alerts): previewing or testing a rule needs read scopes for its telemetry A rule preview or test runs the rule's query and returns what it measured, so a scoped API key must also be able to read the telemetry that query reads. --- apps/api/src/routes/v2/alert-rules.http.ts | 46 ++++++++++++ apps/api/src/routes/v2/alerts.http.test.ts | 74 ++++++++++++++++++- .../MapleAPI/Sources/MapleAPI/openapi.json | 2 +- packages/domain/src/http/v2/alert-rules.ts | 4 +- 4 files changed, 120 insertions(+), 6 deletions(-) diff --git a/apps/api/src/routes/v2/alert-rules.http.ts b/apps/api/src/routes/v2/alert-rules.http.ts index 29de78d385..284660f488 100644 --- a/apps/api/src/routes/v2/alert-rules.http.ts +++ b/apps/api/src/routes/v2/alert-rules.http.ts @@ -26,8 +26,10 @@ import type { import { fromV2Threshold, MapleApiV2, + missingReadScopes, paginateArray, scopeAllows, + TELEMETRY_READ_FAMILIES, timestamp, toV2Threshold, V2ParameterInvalid, @@ -427,6 +429,48 @@ const mergeUpsertRequest = ( }) }) +type TelemetryFamily = (typeof TELEMETRY_READ_FAMILIES)[number] + +const isTelemetryFamily = (source: string): source is TelemetryFamily => + Arr.some(TELEMETRY_READ_FAMILIES, (family) => family === source) + +/** The telemetry a rule's query reads. Raw SQL, and a source that is not a family, read all of it. */ +const ruleReadFamilies = (rule: AlertRuleUpsertRequest): ReadonlyArray => { + switch (rule.signalType) { + case "error_rate": + case "p95_latency": + case "p99_latency": + case "apdex": + case "throughput": + return ["traces"] + case "anomaly": + return [] + case "builder_query": { + const source = rule.queryBuilderDraft?.dataSource + return source !== undefined && isTelemetryFamily(source) ? [source] : TELEMETRY_READ_FAMILIES + } + case "raw_query": + case "uptime": + case "slo_burn_rate": + return TELEMETRY_READ_FAMILIES + } +} + +/** + * Preview and test run the rule's query and hand back what it measured, grouped by whatever + * the caller chose: a telemetry read. A scoped key must be able to read that telemetry itself. + */ +const requireRuleReadScopes = (scopes: ReadonlyArray | undefined, rule: AlertRuleUpsertRequest) => { + const missing = missingReadScopes(scopes, ruleReadFamilies(rule)) + return missing.length === 0 + ? Effect.void + : Effect.fail( + new AlertForbiddenError({ + message: `Running this rule's query requires ${Arr.join(Arr.map(missing, (family) => `"${family}:read"`), ", ")}, because it returns the telemetry it reads.`, + }), + ) +} + const toV2PreviewResult = (preview: AlertRulePreviewResponse): V2AlertRulePreviewResult => ({ object: "alert_rule.preview", bucket_seconds: preview.bucketSeconds, @@ -562,6 +606,7 @@ export const HttpV2AlertRulesLive = HttpApiBuilder.group(MapleApiV2, "alertRules Effect.gen(function* () { const tenant = yield* CurrentTenant.Context const rule = yield* toUpsertRequest(payload.rule) + yield* requireRuleReadScopes(tenant.scopes, rule) const result = yield* alerts.testRule( tenant.orgId, tenant.userId, @@ -600,6 +645,7 @@ export const HttpV2AlertRulesLive = HttpApiBuilder.group(MapleApiV2, "alertRules 'Previewing a raw SQL alert requires the "alerts:write" scope, because it executes your query against the warehouse.', }) } + yield* requireRuleReadScopes(tenant.scopes, rule) const preview = yield* alerts.previewRule( tenant.orgId, tenant.roles, diff --git a/apps/api/src/routes/v2/alerts.http.test.ts b/apps/api/src/routes/v2/alerts.http.test.ts index 639bf40c72..0cd890ecf7 100644 --- a/apps/api/src/routes/v2/alerts.http.test.ts +++ b/apps/api/src/routes/v2/alerts.http.test.ts @@ -1012,12 +1012,80 @@ describe("v2 alerts over HTTP", () => { await harness.dispose() }) - it("previews a raw SQL alert for alerts:write keys", async () => { + const errorRatePreview = { + rule: { + name: "Error rate preview", + severity: "warning", + signal_type: "error_rate", + comparator: "gt", + threshold: 5, + window_minutes: 5, + destination_ids: [], + }, + start_time: "2026-01-01T00:00:00.000Z", + end_time: "2026-01-01T00:30:00.000Z", + } + + it("refuses a preview to a key that cannot read the telemetry the rule queries", async () => { + let warehouseCalls = 0 const harness = makeHarness({ ...warehouseStub, - rawSqlQuery: () => Effect.succeed([{ bucket: "2026-01-01 00:10:00", value: 42, samples: 20 }]), + compiledQuery: (tenant, compiled) => { + warehouseCalls += 1 + return warehouseStub.compiledQuery(tenant, compiled) + }, }) + // The preview returns what the query measured, grouped however the caller chose, + // so an alerts-only key would otherwise read traces it was never granted. const key = await harness.bootstrapKey(["alerts:write"]) + const response = await harness.request( + "POST", + "/v2/alerts/rules/preview", + key.secret, + errorRatePreview, + ) + expect(response.status).toBe(403) + expect(warehouseCalls).toBe(0) + await harness.dispose() + }) + + it("refuses a raw SQL preview to a key missing one telemetry family", async () => { + let warehouseCalls = 0 + const harness = makeHarness({ + ...warehouseStub, + rawSqlQuery: () => { + warehouseCalls += 1 + return Effect.succeed([]) + }, + }) + const key = await harness.bootstrapKey(["alerts:write", "traces:read", "logs:read"]) + const response = await harness.request("POST", "/v2/alerts/rules/preview", key.secret, { + rule: { + name: "Raw preview", + severity: "warning", + signal_type: "raw_query", + raw_query_sql: + "SELECT count() AS value FROM traces WHERE $__orgFilter AND $__timeFilter(Timestamp)", + raw_query_reducer: "max", + comparator: "gt", + threshold: 10, + window_minutes: 5, + destination_ids: [], + }, + start_time: "2026-01-01T00:00:00.000Z", + end_time: "2026-01-01T00:30:00.000Z", + }) + expect(response.status).toBe(403) + expect(warehouseCalls).toBe(0) + await harness.dispose() + }) + + it("previews a raw SQL alert for keys that can write alerts and read all telemetry", async () => { + const harness = makeHarness({ + ...warehouseStub, + rawSqlQuery: () => Effect.succeed([{ bucket: "2026-01-01 00:10:00", value: 42, samples: 20 }]), + }) + const key = await harness.bootstrapKey(["alerts:write", "traces:read", "logs:read", "metrics:read"]) const response = await harness.request("POST", "/v2/alerts/rules/preview", key.secret, { rule: { name: "Raw preview", @@ -1063,7 +1131,7 @@ describe("v2 alerts over HTTP", () => { ) : Effect.succeed([{ value: 1, samples: 1 }]), }) - const key = await harness.bootstrapKey(["alerts:write"]) + const key = await harness.bootstrapKey(["alerts:write", "traces:read", "logs:read", "metrics:read"]) const response = await harness.request("POST", "/v2/alerts/rules/preview", key.secret, { rule: { name: "Heavy raw preview", diff --git a/apps/ios/Packages/MapleAPI/Sources/MapleAPI/openapi.json b/apps/ios/Packages/MapleAPI/Sources/MapleAPI/openapi.json index be8e2efde7..179457999a 100644 --- a/apps/ios/Packages/MapleAPI/Sources/MapleAPI/openapi.json +++ b/apps/ios/Packages/MapleAPI/Sources/MapleAPI/openapi.json @@ -8151,7 +8151,7 @@ }, "/v2/alerts/rules/preview": { "post": { - "description": "Replays a rule definition over a historical range and returns the per-window observations and would-have-fired spans. Read-only (sends nothing). Requires the `alerts:read` scope.", + "description": "Replays a rule definition over a historical range and returns the per-window observations and would-have-fired spans. Read-only (sends nothing). Requires the `alerts:read` scope (`alerts:write` for `raw_query`) plus read scopes for the telemetry the rule queries (`traces:read` for the built-in signals, the draft's source for `builder_query`, and all of `traces:read`, `logs:read`, `metrics:read` for `raw_query`).", "operationId": "previewAlertRule", "parameters": [], "requestBody": { diff --git a/packages/domain/src/http/v2/alert-rules.ts b/packages/domain/src/http/v2/alert-rules.ts index 763de6ecd8..0769e65788 100644 --- a/packages/domain/src/http/v2/alert-rules.ts +++ b/packages/domain/src/http/v2/alert-rules.ts @@ -1042,7 +1042,7 @@ export class V2AlertRulesApiGroup extends HttpApiGroup.make("alertRules") identifier: "testAlertRule", summary: "Test an alert rule", description: - "Evaluates a rule definition once against live data without saving it, optionally delivering a test notification. Requires an org-admin role and the `alerts:write` scope.", + "Evaluates a rule definition once against live data without saving it, optionally delivering a test notification. Requires an org-admin role, the `alerts:write` scope, and read scopes for the telemetry the rule queries (`traces:read` for the built-in signals, the draft's source for `builder_query`, and all of `traces:read`, `logs:read`, `metrics:read` for `raw_query`).", }), ), ) @@ -1062,7 +1062,7 @@ export class V2AlertRulesApiGroup extends HttpApiGroup.make("alertRules") identifier: "previewAlertRule", summary: "Preview an alert rule", description: - "Replays a rule definition over a historical range and returns the per-window observations and would-have-fired spans. Read-only (sends nothing). Requires the `alerts:read` scope.", + "Replays a rule definition over a historical range and returns the per-window observations and would-have-fired spans. Read-only (sends nothing). Requires the `alerts:read` scope (`alerts:write` for `raw_query`) plus read scopes for the telemetry the rule queries (`traces:read` for the built-in signals, the draft's source for `builder_query`, and all of `traces:read`, `logs:read`, `metrics:read` for `raw_query`).", }), ), ) From 0f631e08703c64f2631edf9c39f4bdf6f0421a71 Mon Sep 17 00:00:00 2001 From: Makisuo Date: Thu, 12 Nov 2026 00:37:53 +0100 Subject: [PATCH 2/3] test(alerts): pin which telemetry each rule signal reads Every branch of the signal-to-family mapping is unit tested, and a logs builder preview is refused over HTTP to a key that reads only traces. --- .../v2/alert-rule-read-families.test.ts | 46 +++++++++++++++++++ .../src/routes/v2/alert-rule-read-families.ts | 32 +++++++++++++ apps/api/src/routes/v2/alert-rules.http.ts | 34 ++------------ apps/api/src/routes/v2/alerts.http.test.ts | 36 +++++++++++++++ 4 files changed, 119 insertions(+), 29 deletions(-) create mode 100644 apps/api/src/routes/v2/alert-rule-read-families.test.ts create mode 100644 apps/api/src/routes/v2/alert-rule-read-families.ts diff --git a/apps/api/src/routes/v2/alert-rule-read-families.test.ts b/apps/api/src/routes/v2/alert-rule-read-families.test.ts new file mode 100644 index 0000000000..4dcc11e40b --- /dev/null +++ b/apps/api/src/routes/v2/alert-rule-read-families.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from "vitest" +import { ruleReadFamilies } from "./alert-rule-read-families" + +const draft = (dataSource: "traces" | "logs" | "metrics" | "product_events") => ({ + id: "q", + name: "A", + dataSource, + aggregation: "count", +}) + +describe("ruleReadFamilies", () => { + it.each(["error_rate", "p95_latency", "p99_latency", "apdex", "throughput"] as const)( + "%s reads traces", + (signalType) => { + expect(ruleReadFamilies({ signalType })).toEqual(["traces"]) + }, + ) + + it.each(["traces", "logs", "metrics"] as const)("a %s builder draft reads only its source", (source) => { + expect(ruleReadFamilies({ signalType: "builder_query", queryBuilderDraft: draft(source) })).toEqual([ + source, + ]) + }) + + it("a builder draft on a source that is not a family needs all three", () => { + expect( + ruleReadFamilies({ signalType: "builder_query", queryBuilderDraft: draft("product_events") }), + ).toEqual(["traces", "logs", "metrics"]) + }) + + it("a builder rule with no draft needs all three", () => { + expect(ruleReadFamilies({ signalType: "builder_query", queryBuilderDraft: null })).toEqual([ + "traces", + "logs", + "metrics", + ]) + }) + + it("raw SQL needs all three", () => { + expect(ruleReadFamilies({ signalType: "raw_query" })).toEqual(["traces", "logs", "metrics"]) + }) + + it("an anomaly rule runs no query and needs none", () => { + expect(ruleReadFamilies({ signalType: "anomaly" })).toEqual([]) + }) +}) diff --git a/apps/api/src/routes/v2/alert-rule-read-families.ts b/apps/api/src/routes/v2/alert-rule-read-families.ts new file mode 100644 index 0000000000..9ea8de3778 --- /dev/null +++ b/apps/api/src/routes/v2/alert-rule-read-families.ts @@ -0,0 +1,32 @@ +import type { AlertRuleUpsertRequest } from "@maple/domain/http" +import { TELEMETRY_READ_FAMILIES } from "@maple/domain/http/v2" +import { Array as Arr } from "effect" + +type TelemetryFamily = (typeof TELEMETRY_READ_FAMILIES)[number] + +const isTelemetryFamily = (source: string): source is TelemetryFamily => + Arr.some(TELEMETRY_READ_FAMILIES, (family) => family === source) + +/** The telemetry a rule's query reads. Raw SQL, and a source that is not a family, read all of it. */ +export const ruleReadFamilies = ( + rule: Pick, +): ReadonlyArray => { + switch (rule.signalType) { + case "error_rate": + case "p95_latency": + case "p99_latency": + case "apdex": + case "throughput": + return ["traces"] + case "anomaly": + return [] + case "builder_query": { + const source = rule.queryBuilderDraft?.dataSource + return source !== undefined && isTelemetryFamily(source) ? [source] : TELEMETRY_READ_FAMILIES + } + case "raw_query": + case "uptime": + case "slo_burn_rate": + return TELEMETRY_READ_FAMILIES + } +} diff --git a/apps/api/src/routes/v2/alert-rules.http.ts b/apps/api/src/routes/v2/alert-rules.http.ts index 284660f488..8f32418c11 100644 --- a/apps/api/src/routes/v2/alert-rules.http.ts +++ b/apps/api/src/routes/v2/alert-rules.http.ts @@ -29,7 +29,6 @@ import { missingReadScopes, paginateArray, scopeAllows, - TELEMETRY_READ_FAMILIES, timestamp, toV2Threshold, V2ParameterInvalid, @@ -38,6 +37,7 @@ import { AlertForbiddenError } from "@maple/domain/http" import { Array as Arr, DateTime, Effect, Option, Result, Schema, String as Str } from "effect" import { Base64Url } from "effect/encoding" import { auditDiff } from "@/routes/v2/audit-changes" +import { ruleReadFamilies } from "@/routes/v2/alert-rule-read-families" import { recordHttpAudit } from "@maple/backend/services/audit/AuditLogService" import { AlertsService } from "@maple/backend/services/alerts/AlertsService" import { AlertReadModelsService } from "@maple/backend/services/alerts/AlertReadModelsService" @@ -429,33 +429,6 @@ const mergeUpsertRequest = ( }) }) -type TelemetryFamily = (typeof TELEMETRY_READ_FAMILIES)[number] - -const isTelemetryFamily = (source: string): source is TelemetryFamily => - Arr.some(TELEMETRY_READ_FAMILIES, (family) => family === source) - -/** The telemetry a rule's query reads. Raw SQL, and a source that is not a family, read all of it. */ -const ruleReadFamilies = (rule: AlertRuleUpsertRequest): ReadonlyArray => { - switch (rule.signalType) { - case "error_rate": - case "p95_latency": - case "p99_latency": - case "apdex": - case "throughput": - return ["traces"] - case "anomaly": - return [] - case "builder_query": { - const source = rule.queryBuilderDraft?.dataSource - return source !== undefined && isTelemetryFamily(source) ? [source] : TELEMETRY_READ_FAMILIES - } - case "raw_query": - case "uptime": - case "slo_burn_rate": - return TELEMETRY_READ_FAMILIES - } -} - /** * Preview and test run the rule's query and hand back what it measured, grouped by whatever * the caller chose: a telemetry read. A scoped key must be able to read that telemetry itself. @@ -466,7 +439,10 @@ const requireRuleReadScopes = (scopes: ReadonlyArray | undefined, rule: ? Effect.void : Effect.fail( new AlertForbiddenError({ - message: `Running this rule's query requires ${Arr.join(Arr.map(missing, (family) => `"${family}:read"`), ", ")}, because it returns the telemetry it reads.`, + message: `Running this rule's query requires ${Arr.join( + Arr.map(missing, (family) => `"${family}:read"`), + ", ", + )}, because it returns the telemetry it reads.`, }), ) } diff --git a/apps/api/src/routes/v2/alerts.http.test.ts b/apps/api/src/routes/v2/alerts.http.test.ts index 0cd890ecf7..c20af2bc5a 100644 --- a/apps/api/src/routes/v2/alerts.http.test.ts +++ b/apps/api/src/routes/v2/alerts.http.test.ts @@ -1049,6 +1049,42 @@ describe("v2 alerts over HTTP", () => { await harness.dispose() }) + const logsBuilderPreview = { + rule: { + name: "Logs builder preview", + severity: "warning", + signal_type: "builder_query", + query_builder_draft: { id: "q", name: "A", dataSource: "logs", aggregation: "count" }, + comparator: "gt", + threshold: 10, + window_minutes: 5, + destination_ids: [], + }, + start_time: "2026-01-01T00:00:00.000Z", + end_time: "2026-01-01T00:30:00.000Z", + } + + it("refuses a logs builder preview to a key that reads only traces", async () => { + let warehouseCalls = 0 + const harness = makeHarness({ + ...warehouseStub, + compiledQuery: (tenant, compiled) => { + warehouseCalls += 1 + return warehouseStub.compiledQuery(tenant, compiled) + }, + }) + const key = await harness.bootstrapKey(["alerts:write", "traces:read"]) + const response = await harness.request( + "POST", + "/v2/alerts/rules/preview", + key.secret, + logsBuilderPreview, + ) + expect(response.status).toBe(403) + expect(warehouseCalls).toBe(0) + await harness.dispose() + }) + it("refuses a raw SQL preview to a key missing one telemetry family", async () => { let warehouseCalls = 0 const harness = makeHarness({ From 4ca4b35daa58deb69552d2bf52e4fbe5982dbb5a Mon Sep 17 00:00:00 2001 From: Makisuo Date: Thu, 12 Nov 2026 00:56:13 +0100 Subject: [PATCH 3/3] fix(alerts): managed signals need no telemetry scopes, and pin the rule test gate Uptime and SLO rules are refused before any query runs, so asking for read scopes first only hid that answer. The rule test endpoint's scope refusal now has an HTTP test of its own. --- .../v2/alert-rule-read-families.test.ts | 9 ++++++--- .../src/routes/v2/alert-rule-read-families.ts | 6 ++++-- apps/api/src/routes/v2/alerts.http.test.ts | 19 +++++++++++++++++++ 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/apps/api/src/routes/v2/alert-rule-read-families.test.ts b/apps/api/src/routes/v2/alert-rule-read-families.test.ts index 4dcc11e40b..5e7cbe5385 100644 --- a/apps/api/src/routes/v2/alert-rule-read-families.test.ts +++ b/apps/api/src/routes/v2/alert-rule-read-families.test.ts @@ -40,7 +40,10 @@ describe("ruleReadFamilies", () => { expect(ruleReadFamilies({ signalType: "raw_query" })).toEqual(["traces", "logs", "metrics"]) }) - it("an anomaly rule runs no query and needs none", () => { - expect(ruleReadFamilies({ signalType: "anomaly" })).toEqual([]) - }) + it.each(["anomaly", "uptime", "slo_burn_rate"] as const)( + "%s runs no query of its own and needs none", + (signalType) => { + expect(ruleReadFamilies({ signalType })).toEqual([]) + }, + ) }) diff --git a/apps/api/src/routes/v2/alert-rule-read-families.ts b/apps/api/src/routes/v2/alert-rule-read-families.ts index 9ea8de3778..8b92667da9 100644 --- a/apps/api/src/routes/v2/alert-rule-read-families.ts +++ b/apps/api/src/routes/v2/alert-rule-read-families.ts @@ -18,15 +18,17 @@ export const ruleReadFamilies = ( case "apdex": case "throughput": return ["traces"] + // No query of their own: an anomaly rule reads open incidents, and uptime and SLO rules + // are refused by the generic rule APIs before anything runs. case "anomaly": + case "uptime": + case "slo_burn_rate": return [] case "builder_query": { const source = rule.queryBuilderDraft?.dataSource return source !== undefined && isTelemetryFamily(source) ? [source] : TELEMETRY_READ_FAMILIES } case "raw_query": - case "uptime": - case "slo_burn_rate": return TELEMETRY_READ_FAMILIES } } diff --git a/apps/api/src/routes/v2/alerts.http.test.ts b/apps/api/src/routes/v2/alerts.http.test.ts index c20af2bc5a..3870879674 100644 --- a/apps/api/src/routes/v2/alerts.http.test.ts +++ b/apps/api/src/routes/v2/alerts.http.test.ts @@ -1085,6 +1085,25 @@ describe("v2 alerts over HTTP", () => { await harness.dispose() }) + it("refuses a rule test to a key that cannot read the telemetry the rule queries", async () => { + let warehouseCalls = 0 + const harness = makeHarness({ + ...warehouseStub, + compiledQuery: (tenant, compiled) => { + warehouseCalls += 1 + return warehouseStub.compiledQuery(tenant, compiled) + }, + }) + const key = await harness.bootstrapKey(["alerts:write", "traces:read"]) + const response = await harness.request("POST", "/v2/alerts/rules/test", key.secret, { + rule: logsBuilderPreview.rule, + send_notification: false, + }) + expect(response.status).toBe(403) + expect(warehouseCalls).toBe(0) + await harness.dispose() + }) + it("refuses a raw SQL preview to a key missing one telemetry family", async () => { let warehouseCalls = 0 const harness = makeHarness({