From 0934cac37887003f7d2a535d8b84a24dffeb3f63 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Thu, 20 Aug 2026 23:36:24 +0000 Subject: [PATCH 001/475] chore(deps): update github-actions (non-major) --- .github/workflows/ci.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2b09c41f7..3d6095d3f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,7 +29,7 @@ jobs: - uses: actions/setup-go@v7 with: - go-version: "1.26" + go-version: "1.27" cache-dependency-path: server/go.sum - name: Create embed placeholders for lint/test @@ -76,7 +76,7 @@ jobs: uses: golangci/golangci-lint-action@v9.3.0 with: # https://github.com/golangci/golangci-lint/releases - version: v2.12.2 + version: v2.13.1 install-mode: goinstall working-directory: server # only-new-issues: true @@ -237,7 +237,7 @@ jobs: - uses: actions/setup-go@v7 with: - go-version: "1.26" + go-version: "1.27" cache-dependency-path: server/go.sum - name: Download dash dist From 8f2dd261cb1d0e073e064eea9b34f16f06392dd2 Mon Sep 17 00:00:00 2001 From: Florent Clairambault Date: Fri, 21 Aug 2026 01:50:00 +0200 Subject: [PATCH 002/475] chore: start implementing generic attachments and incident screenshots From 74461af8c616ca645e1df64587acd111a273f1f4 Mon Sep 17 00:00:00 2001 From: Florent Clairambault Date: Fri, 21 Aug 2026 01:50:27 +0200 Subject: [PATCH 003/475] docs: add implementation plan for generic attachments and incident screenshots --- ...ic-attachments-and-incident-screenshots.md | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/specs/todos/2026-08-21-01-generic-attachments-and-incident-screenshots.md b/specs/todos/2026-08-21-01-generic-attachments-and-incident-screenshots.md index 88e9fc09f..620ead5af 100644 --- a/specs/todos/2026-08-21-01-generic-attachments-and-incident-screenshots.md +++ b/specs/todos/2026-08-21-01-generic-attachments-and-incident-screenshots.md @@ -208,3 +208,95 @@ Sibling of the WS route (`api.GET("/agent/ws", ...)`, - Never-public audit extended to attachments. - E2E (dash0): incident with a seeded screenshot attachment renders the image with timestamp + region caption. + +## Implementation Plan + +Ordered so every step lands on a green tree. Steps 1–2 are the generic rail; +3–5 are the screenshot consumer; 6–7 are the agent + GC halves; 8 is the UI. + +### Step 1 — Schema + model + db layer (`files.topic`, `files.details`) + +- New `SECTION: generic-attachments` appended to `014_v0_17_0.up.sql` in BOTH + dialects, with the mirrored teardown prepended (reverse order) to both + `.down.sql` files. `topic` TEXT NULL, `details` jsonb/TEXT NULL, partial + index `files_org_topic_idx (organization_uid, topic) + WHERE deleted_at IS NULL AND topic IS NOT NULL`. +- `models.File`: `Topic *string`, `Details JSONMap` (`type:jsonb,nullzero`), + `ListFilesFilter.Topic` / `.TopicPrefix`. +- `db.Service`: `ListFilesByTopic(ctx, orgUID, topic)`, + `DeleteFilesByTopicPrefix(ctx, orgUID, prefix) (int, error)`, and + `ListOrphanIncidentAttachments(ctx, olderThan, limit)` for the GC sweep — + implemented in both `postgres.go` and `sqlite.go`. + +### Step 2 — `files.Service` attachment API + +- `CreateFile` grows a variadic `...CreateFileOption` tail (`WithTopic`, + `WithDetails`) so the two existing callers (feedback, org logos) are + untouched. +- `ListAttachments(ctx, orgUID, topic)` (newest first), + `DeleteAttachments(ctx, orgUID, topicPrefix) (int, error)`. +- `FileResponse` gains `topic` / `details` (camelCase, `omitempty`). + +### Step 3 — `Diagnostics.Screenshot` + browser capture + +- `checkerdef.Screenshot{PNG []byte `json:"-"`, CapturedAt, Width, Height}` + hung on `Diagnostics.Screenshot`. The BYTES field is `json:"-"` so the agent + WS result frame can never carry them; `FailureResponse` stays serialized as + the positive control. +- `BrowserConfig.Screenshot bool` (`screenshot`, default false), round-tripped + through `FromMap`/`GetConfig`. +- `checkbrowser`: on a FAILING outcome with the flag set, `chromedp.FullScreenshot` + into a buffer before the browser context is disposed, time-boxed + (`screenshotTimeout`), capped at `MaxScreenshotBytes` (4 MiB) — over-cap or + errored capture is logged and dropped, never changes the status. + +### Step 4 — Incident persistence on transitions only + +- `incidents.AttachmentStore` (small interface, mirrors `PublicationHook`) with + `CreateAttachment` / `DeleteAttachments`; wired in `server.go` from the files + service via an adapter. Nil-safe. +- `createIncident`: after the row is created, persist the screenshot under + topic `incidents//screenshot`. +- `reopenIncident`: soft-delete `incidents//` first, then persist the new + onset's capture (mirrors the `failureResponse` overwrite rule). No capture on + the relapse ⇒ the stale one is still dropped. +- Every non-transition failing run drops its capture on the floor. + +### Step 5 — Incident API + never-public audit + +- `IncidentResponse.Attachments []IncidentAttachment` (uid, kind, name, + mimeType, size, capturedAt, region, downloadUrl — signed via `signedurl`). + Populated by `GetIncident` only (not the list endpoint). +- `openapi.yaml` + `make generate`. +- `details_never_public_test.go`: forbid `attachments`, `downloadurl`, + `screenshot` as public field names, and add a value-level control. + +### Step 6 — `POST /api/v1/agent/attachments` + +- Sibling of `api.GET("/agent/ws", …)`, same Ed25519 signed-header auth + (`agentcrypto.VerifySignature`, ±5 min skew, DB-backed nonce replay guard). +- Raw body, `image/png` only (magic-byte sniff), `topic` + `kind` from query, + per-file cap, per-agent rate limit. +- `attachtopics` — a topic-prefix → authorizer registry. The + `incidents//` authorizer resolves the incident, requires it to belong to + the agent's org, and requires the incident's check to be served by a region + this agent serves. Org NEVER comes from the request. +- Response `{"fileUid": "..."}`. +- **Escape hatch (spec §6):** the WS upload-request frame + agent-side LRU are + DEFERRED to a follow-up spec; the endpoint + authorizer land here with direct + tests, and a follow-up spec file is created in `specs/todos/`. + +### Step 7 — GC + +- `DeleteAttachments` is the reaper primitive. +- The periodic state-cleanup job gains an attachment sweep: attachment rows + whose `incidents//…` topic points at an incident that no longer exists + (and which are older than a grace window) are soft-deleted. + +### Step 8 — dash0 + E2E + +- `IncidentAttachment` type in `api/hooks.ts`; `IncidentScreenshotCard` + rendered next to `ProbeResponseCard`, caption = captured-at + region, + labelled "shortly after failure detection". Responsive, i18n in all four + locales. +- Playwright spec in `web/dash0/e2e/` asserting the image + caption render. From 9c2172a1e8468cc38d0acd6c647ae95b8c04054d Mon Sep 17 00:00:00 2001 From: Florent Clairambault Date: Fri, 21 Aug 2026 01:52:53 +0200 Subject: [PATCH 004/475] feat: add files.topic/files.details generic attachment rail --- server/internal/db/models/file.go | 32 ++++++- .../postgres/migrations/014_v0_17_0.down.sql | 26 ++++++ .../db/postgres/migrations/014_v0_17_0.up.sql | 67 ++++++++++++++ server/internal/db/postgres/postgres.go | 89 +++++++++++++++++++ server/internal/db/service.go | 12 +++ .../db/sqlite/migrations/014_v0_17_0.down.sql | 24 +++++ .../db/sqlite/migrations/014_v0_17_0.up.sql | 30 +++++++ server/internal/db/sqlite/sqlite.go | 89 +++++++++++++++++++ server/internal/handlers/files/service.go | 87 ++++++++++++++++++ 9 files changed, 453 insertions(+), 3 deletions(-) diff --git a/server/internal/db/models/file.go b/server/internal/db/models/file.go index b9907e1c2..f0421f0db 100644 --- a/server/internal/db/models/file.go +++ b/server/internal/db/models/file.go @@ -19,6 +19,25 @@ type File struct { CreatedBy *string `bun:"created_by"` CreatedAt time.Time `bun:"created_at,notnull,default:current_timestamp"` DeletedAt *time.Time `bun:"deleted_at"` + + // Topic is the ATTACHMENT KEY (spec 2026-08-21-01): a path-like + // `//`, e.g. + // `incidents/9a1eb273-0a95-4d6b-b967-9af076c1f8e8/screenshot`. + // + // NIL IS THE NORM. A file that is not an attachment — an org logo, a + // feedback screenshot — carries no topic and is invisible to every + // attachment query. The path shape is what makes both accesses cheap on + // one index: an exact match lists one entity's attachments of one kind, a + // prefix match reaps everything hanging off an entity when it is deleted. + Topic *string `bun:"topic"` + + // Details is a free metadata bag for the attachment kind — for a + // screenshot: capturedAt, region, checkUid, trigger. Unconstrained on + // purpose so the next attachment kind needs no migration. + // + // SECURITY: operator-facing evidence, exactly like incidents.details. It + // must never be serialized onto a public surface. + Details JSONMap `bun:"details,type:jsonb,nullzero"` } // NewFile creates a new file record with a generated UID. @@ -37,7 +56,14 @@ func NewFile(orgUID, name, mimeType, fileURI string, size int64, createdBy *stri // ListFilesFilter provides filtering options for listing files. type ListFilesFilter struct { - Q string - Offset int - Limit int + Q string + // Topic restricts the listing to attachments with this EXACT topic. + // Mutually exclusive with TopicPrefix in practice; both may be set and + // both are then applied. + Topic string + // TopicPrefix restricts the listing to attachments whose topic starts with + // this string — the entity-scoped form, e.g. `incidents//`. + TopicPrefix string + Offset int + Limit int } diff --git a/server/internal/db/postgres/migrations/014_v0_17_0.down.sql b/server/internal/db/postgres/migrations/014_v0_17_0.down.sql index 8e97294f3..47d7dccbe 100644 --- a/server/internal/db/postgres/migrations/014_v0_17_0.down.sql +++ b/server/internal/db/postgres/migrations/014_v0_17_0.down.sql @@ -6,6 +6,32 @@ -- -- Several sections are lossy on the way down; each says so in its own note. +-- ========================================================================== +-- SECTION: generic-attachments +-- Was scratch migration 020_generic_attachments (spec 2026-08-21-01). Teardown half. +-- ========================================================================== + +-- Teardown/parity only — never run in production. Reverses the +-- generic-attachments section of 014_v0_17_0.up.sql, and it is the FIRST +-- section here because the down file unwinds in reverse order. +-- +-- LOSSY: dropping `topic` and `details` destroys every attachment link. The +-- blobs themselves survive in the storage backend and the `files` rows survive +-- in the table, but nothing knows what they were attached to any more, so a +-- re-migration starts from "no attachments exist". + +drop index if exists files_org_topic_idx; + +--bun:split + +alter table files drop column if exists details; + +--bun:split + +alter table files drop column if exists topic; + +--bun:split + -- ========================================================================== -- SECTION: dangling-notification-routes -- Was scratch migration 019_dangling_notification_routes (spec 2026-08-20-02). Teardown half. diff --git a/server/internal/db/postgres/migrations/014_v0_17_0.up.sql b/server/internal/db/postgres/migrations/014_v0_17_0.up.sql index dfdc6c21f..9a57ae930 100644 --- a/server/internal/db/postgres/migrations/014_v0_17_0.up.sql +++ b/server/internal/db/postgres/migrations/014_v0_17_0.up.sql @@ -577,3 +577,70 @@ where not exists ( where c.uid = r.contact_uid and c.deleted_at is null ); + +--bun:split + +-- ========================================================================== +-- SECTION: generic-attachments +-- Was scratch migration 020_generic_attachments (spec 2026-08-21-01). +-- ========================================================================== + +-- Generic file attachments: a `files` row can now say WHAT IT IS ATTACHED TO +-- (spec 2026-08-21-01). +-- +-- Until now a `files` row was an island. Nothing linked it to an owning +-- entity, so "list the attachments of this incident" was unanswerable and +-- there was no GC story at all — a blob whose owner was deleted stayed on the +-- bill forever. That gap blocks every capture feature (incident screenshots +-- first, HAR files and packet captures later), so it is fixed once, generically, +-- rather than once per capture kind. +-- +-- `topic` is a PATH-LIKE ATTACHMENT KEY, `//`, e.g. +-- incidents/9a1eb273-0a95-4d6b-b967-9af076c1f8e8/screenshot +-- Path-like on purpose: it makes both queries the feature needs cheap on the +-- same index — an EXACT match lists one entity's attachments of one kind, and +-- a PREFIX match (`incidents//`) reaps everything hanging off an entity +-- when it is deleted. A pair of (entity_type, entity_uid) columns would have +-- needed a third column per kind and a wider index for the same two queries. +-- +-- NULL is the norm, not an exception: every file that is not an attachment +-- (org logos, feedback screenshots) keeps a NULL topic and is untouched by +-- everything here. That is why the column is nullable with no default and no +-- backfill. +-- +-- `details` is a free metadata bag, jsonb, deliberately unconstrained. For a +-- screenshot it carries {"capturedAt", "region", "checkUid", "trigger"}. It +-- exists so the NEXT attachment kind does not need a migration. +-- +-- SECURITY NOTE: attachments are org-operational evidence, exactly like +-- `incidents.details`. Neither the topic, the details bag, nor a signed +-- download URL may ever reach a public surface (status pages, subscriber +-- payloads) — that rule is pinned by the never-public audit in +-- internal/handlers/statuspages/details_never_public_test.go. +alter table files add column if not exists topic text; + +--bun:split + +alter table files add column if not exists details jsonb; + +--bun:split + +-- Partial index, not a plain one: the overwhelming majority of `files` rows are +-- NOT attachments (topic IS NULL) and none of them can ever match an +-- attachment lookup, so keeping them out of the index keeps it proportional to +-- the attachment count rather than to the whole table. `deleted_at is null` is +-- in the predicate for the same reason — every attachment query is +-- live-rows-only, and a soft-deleted screenshot must never come back in a list. +create index if not exists files_org_topic_idx + on files (organization_uid, topic) + where deleted_at is null and topic is not null; + +--bun:split + +comment on column files.topic is + 'Attachment key, // (spec 2026-08-21-01). NULL for non-attachment files. Prefix-matched for entity-deletion reaping.'; + +--bun:split + +comment on column files.details is + 'Free metadata bag for the attachment kind. Org-operational evidence — never serialized onto a public surface.'; diff --git a/server/internal/db/postgres/postgres.go b/server/internal/db/postgres/postgres.go index 18903c901..0242808e4 100644 --- a/server/internal/db/postgres/postgres.go +++ b/server/internal/db/postgres/postgres.go @@ -5582,6 +5582,8 @@ func (s *Service) ListFiles( query = query.Where("name ILIKE ?", "%"+filter.Q+"%") } + query = applyFileTopicFilter(query, filter) + total, err := query.Count(ctx) if err != nil { return nil, 0, err @@ -6261,3 +6263,90 @@ func (s *Service) RemoveRecipientFromReportSchedules(ctx context.Context, orgUID return changed, nil } + +// applyFileTopicFilter narrows a files query to attachments. An exact Topic and +// a TopicPrefix are both honoured when both are set; neither set leaves the +// query untouched, which is what keeps the historical "list every file" +// behaviour intact for the dashboard's file browser. +// +// LIKE with an escaped prefix, not a raw concatenation: a topic is +// server-generated today, but the agent upload endpoint accepts one from the +// wire, and a `%` smuggled into a prefix would silently widen a REAP from one +// entity to a whole table. +func applyFileTopicFilter(query *bun.SelectQuery, filter models.ListFilesFilter) *bun.SelectQuery { + if filter.Topic != "" { + query = query.Where("topic = ?", filter.Topic) + } + + if filter.TopicPrefix != "" { + query = query.Where("topic LIKE ? ESCAPE '\\'", escapeLikePrefix(filter.TopicPrefix)+"%") + } + + return query +} + +// escapeLikePrefix neutralizes the LIKE wildcards in a literal prefix. +func escapeLikePrefix(prefix string) string { + replacer := strings.NewReplacer(`\`, `\\`, "%", `\%`, "_", `\_`) + + return replacer.Replace(prefix) +} + +// DeleteFilesByTopicPrefix soft-deletes every live attachment of an org under a +// topic prefix and returns how many rows changed. A zero count is a normal +// answer (nothing was attached), never an error — the reaper runs on entities +// that may well have no attachments at all. +func (s *Service) DeleteFilesByTopicPrefix(ctx context.Context, orgUID, prefix string) (int, error) { + if prefix == "" { + // Defensive: an empty prefix would match every attachment in the org. + return 0, nil + } + + res, err := s.db.NewUpdate(). + Model((*models.File)(nil)). + Where("organization_uid = ?", orgUID). + Where("topic LIKE ? ESCAPE '\\'", escapeLikePrefix(prefix)+"%"). + Where("deleted_at IS NULL"). + Set("deleted_at = ?", time.Now()). + Exec(ctx) + if err != nil { + return 0, err + } + + rows, err := res.RowsAffected() + if err != nil { + return 0, err + } + + return int(rows), nil +} + +// ListAttachmentsByTopicPrefix returns live attachment rows across all orgs +// under a topic prefix, older than `before`, capped at limit. Cross-org because +// its only caller is the GC sweep. +func (s *Service) ListAttachmentsByTopicPrefix( + ctx context.Context, prefix string, before time.Time, limit int, +) ([]*models.File, error) { + if prefix == "" { + return nil, nil + } + + var files []*models.File + + query := s.db.NewSelect(). + Model(&files). + Where("topic LIKE ? ESCAPE '\\'", escapeLikePrefix(prefix)+"%"). + Where("deleted_at IS NULL"). + Where("created_at < ?", before). + Order("created_at ASC") + + if limit > 0 { + query = query.Limit(limit) + } + + if err := query.Scan(ctx); err != nil { + return nil, err + } + + return files, nil +} diff --git a/server/internal/db/service.go b/server/internal/db/service.go index f81bee879..b656fb28c 100644 --- a/server/internal/db/service.go +++ b/server/internal/db/service.go @@ -924,6 +924,18 @@ type Service interface { ctx context.Context, orgUID string, filter models.ListFilesFilter, ) ([]*models.File, int64, error) DeleteFile(ctx context.Context, orgUID, uid string) error + // DeleteFilesByTopicPrefix soft-deletes every live attachment of an org + // whose topic starts with prefix, and reports how many rows changed. This + // is the entity-deletion reaper and the replace-on-reopen primitive + // (spec 2026-08-21-01) — a no-match is NOT an error, unlike DeleteFile. + DeleteFilesByTopicPrefix(ctx context.Context, orgUID, prefix string) (int, error) + // ListAttachmentsByTopicPrefix returns live attachment rows across ALL + // organizations whose topic starts with prefix and which were created + // before `before`, capped at limit. Cross-org by design: its only caller + // is the GC sweep, which has no org to scope to. + ListAttachmentsByTopicPrefix( + ctx context.Context, prefix string, before time.Time, limit int, + ) ([]*models.File, error) // CheckDependency operations CreateCheckDependency(ctx context.Context, dep *models.CheckDependency) error diff --git a/server/internal/db/sqlite/migrations/014_v0_17_0.down.sql b/server/internal/db/sqlite/migrations/014_v0_17_0.down.sql index 654529a48..f2e1e0646 100644 --- a/server/internal/db/sqlite/migrations/014_v0_17_0.down.sql +++ b/server/internal/db/sqlite/migrations/014_v0_17_0.down.sql @@ -6,6 +6,30 @@ -- -- Several sections are lossy on the way down; each says so in its own note. +-- ========================================================================== +-- SECTION: generic-attachments +-- Was scratch migration 020_generic_attachments (spec 2026-08-21-01). Teardown half. +-- ========================================================================== + +-- Teardown/parity only — never run in production. SQLite mirror of the +-- generic-attachments section of postgres/migrations/014_v0_17_0.down.sql, and +-- the FIRST section here because the down file unwinds in reverse order. +-- +-- LOSSY the same way the Postgres half is: dropping the columns destroys every +-- attachment link while leaving the blobs and their `files` rows behind. + +drop index if exists files_org_topic_idx; + +--bun:split + +alter table files drop column details; + +--bun:split + +alter table files drop column topic; + +--bun:split + -- ========================================================================== -- SECTION: dangling-notification-routes -- Was scratch migration 019_dangling_notification_routes (spec 2026-08-20-02). Teardown half. diff --git a/server/internal/db/sqlite/migrations/014_v0_17_0.up.sql b/server/internal/db/sqlite/migrations/014_v0_17_0.up.sql index f23a0a98f..5f4cc2cd3 100644 --- a/server/internal/db/sqlite/migrations/014_v0_17_0.up.sql +++ b/server/internal/db/sqlite/migrations/014_v0_17_0.up.sql @@ -489,3 +489,33 @@ where not exists ( where c.uid = user_notification_routes.contact_uid and c.deleted_at is null ); + +--bun:split + +-- ========================================================================== +-- SECTION: generic-attachments +-- Was scratch migration 020_generic_attachments (spec 2026-08-21-01). +-- ========================================================================== + +-- SQLite mirror of the generic-attachments section of +-- postgres/migrations/014_v0_17_0.up.sql — a `files` row can now say what it +-- is attached to (spec 2026-08-21-01). See the Postgres file for the full +-- rationale: why the key is path-like, why NULL is the norm, why the index is +-- partial, and why an attachment must never reach a public surface. +-- +-- Dialect differences, both cosmetic: +-- * `details` is TEXT here, like every other jsonb column in the SQLite +-- schema — models.JSONMap marshals to a string either way. +-- * SQLite has no `ADD COLUMN IF NOT EXISTS`, so these two statements are +-- not re-runnable — same as every other section in this file. +alter table files add column topic text; + +--bun:split + +alter table files add column details text; + +--bun:split + +create index if not exists files_org_topic_idx + on files (organization_uid, topic) + where deleted_at is null and topic is not null; diff --git a/server/internal/db/sqlite/sqlite.go b/server/internal/db/sqlite/sqlite.go index dd9a4cba8..7d2ec273d 100644 --- a/server/internal/db/sqlite/sqlite.go +++ b/server/internal/db/sqlite/sqlite.go @@ -5537,6 +5537,8 @@ func (s *Service) ListFiles( query = query.Where("name LIKE ?", "%"+filter.Q+"%") } + query = applyFileTopicFilter(query, filter) + total, err := query.Count(ctx) if err != nil { return nil, 0, err @@ -6219,3 +6221,90 @@ func (s *Service) RemoveRecipientFromReportSchedules(ctx context.Context, orgUID return changed, nil } + +// applyFileTopicFilter narrows a files query to attachments. An exact Topic and +// a TopicPrefix are both honoured when both are set; neither set leaves the +// query untouched, which is what keeps the historical "list every file" +// behaviour intact for the dashboard's file browser. +// +// LIKE with an escaped prefix, not a raw concatenation: a topic is +// server-generated today, but the agent upload endpoint accepts one from the +// wire, and a `%` smuggled into a prefix would silently widen a REAP from one +// entity to a whole table. +func applyFileTopicFilter(query *bun.SelectQuery, filter models.ListFilesFilter) *bun.SelectQuery { + if filter.Topic != "" { + query = query.Where("topic = ?", filter.Topic) + } + + if filter.TopicPrefix != "" { + query = query.Where("topic LIKE ? ESCAPE '\\'", escapeLikePrefix(filter.TopicPrefix)+"%") + } + + return query +} + +// escapeLikePrefix neutralizes the LIKE wildcards in a literal prefix. +func escapeLikePrefix(prefix string) string { + replacer := strings.NewReplacer(`\`, `\\`, "%", `\%`, "_", `\_`) + + return replacer.Replace(prefix) +} + +// DeleteFilesByTopicPrefix soft-deletes every live attachment of an org under a +// topic prefix and returns how many rows changed. A zero count is a normal +// answer (nothing was attached), never an error — the reaper runs on entities +// that may well have no attachments at all. +func (s *Service) DeleteFilesByTopicPrefix(ctx context.Context, orgUID, prefix string) (int, error) { + if prefix == "" { + // Defensive: an empty prefix would match every attachment in the org. + return 0, nil + } + + res, err := s.db.NewUpdate(). + Model((*models.File)(nil)). + Where("organization_uid = ?", orgUID). + Where("topic LIKE ? ESCAPE '\\'", escapeLikePrefix(prefix)+"%"). + Where("deleted_at IS NULL"). + Set("deleted_at = ?", time.Now()). + Exec(ctx) + if err != nil { + return 0, err + } + + rows, err := res.RowsAffected() + if err != nil { + return 0, err + } + + return int(rows), nil +} + +// ListAttachmentsByTopicPrefix returns live attachment rows across all orgs +// under a topic prefix, older than `before`, capped at limit. Cross-org because +// its only caller is the GC sweep. +func (s *Service) ListAttachmentsByTopicPrefix( + ctx context.Context, prefix string, before time.Time, limit int, +) ([]*models.File, error) { + if prefix == "" { + return nil, nil + } + + var files []*models.File + + query := s.db.NewSelect(). + Model(&files). + Where("topic LIKE ? ESCAPE '\\'", escapeLikePrefix(prefix)+"%"). + Where("deleted_at IS NULL"). + Where("created_at < ?", before). + Order("created_at ASC") + + if limit > 0 { + query = query.Limit(limit) + } + + if err := query.Scan(ctx); err != nil { + return nil, err + } + + return files, nil +} diff --git a/server/internal/handlers/files/service.go b/server/internal/handlers/files/service.go index 15931df31..3c131114c 100644 --- a/server/internal/handlers/files/service.go +++ b/server/internal/handlers/files/service.go @@ -52,6 +52,11 @@ type FileResponse struct { Size int64 `json:"size"` CreatedBy *string `json:"createdBy,omitempty"` CreatedAt time.Time `json:"createdAt"` + // Topic is the attachment key (spec 2026-08-21-01). Omitted entirely for + // the ordinary, unattached file so existing clients see no change. + Topic *string `json:"topic,omitempty"` + // Details is the attachment kind's metadata bag. Operator-facing only. + Details models.JSONMap `json:"details,omitempty"` } // ListResponse is the paginated wrapper returned by the list endpoint. @@ -70,6 +75,8 @@ func toResponse(file *models.File) FileResponse { Size: file.Size, CreatedBy: file.CreatedBy, CreatedAt: file.CreatedAt, + Topic: file.Topic, + Details: file.Details, } } @@ -201,11 +208,17 @@ func (s *Service) DeleteFile(ctx context.Context, orgSlug, fileUID string) error func (s *Service) CreateFile( ctx context.Context, orgUID uuid.UUID, group filestorage.GroupType, name, mimeType string, createdBy *string, body io.Reader, size int64, + opts ...CreateFileOption, ) (*models.File, error) { if size > MaxFileSize { return nil, ErrFileTooLarge } + settings := createFileSettings{} + for _, opt := range opts { + opt(&settings) + } + storage, err := filestorage.NewFileStorage(s.storageConfig()) if err != nil { return nil, fmt.Errorf("storage backend: %w", err) @@ -223,6 +236,9 @@ func (s *Service) CreateFile( } file := models.NewFile(orgUID.String(), name, mimeType, uri, size, createdBy) + file.Topic = settings.topic + file.Details = settings.details + if err := s.db.CreateFile(ctx, file); err != nil { return nil, fmt.Errorf("create file row: %w", err) } @@ -230,6 +246,77 @@ func (s *Service) CreateFile( return file, nil } +// createFileSettings collects the optional attachment metadata a caller may +// hang on a new file. +type createFileSettings struct { + topic *string + details models.JSONMap +} + +// CreateFileOption is the functional-option shape CreateFile takes. Options +// rather than two more positional arguments: the two historical callers +// (feedback screenshots, org logos) attach nothing at all, and adding +// parameters they must pass nil for is how a signature rots. +type CreateFileOption func(*createFileSettings) + +// WithTopic attaches the file to an entity under a path-like attachment key, +// `//`. An empty topic is ignored — an attachment key that +// is the empty string would be matched by every prefix reap. +func WithTopic(topic string) CreateFileOption { + return func(settings *createFileSettings) { + if topic == "" { + return + } + + settings.topic = &topic + } +} + +// WithDetails hangs the attachment kind's metadata bag on the file. +func WithDetails(details models.JSONMap) CreateFileOption { + return func(settings *createFileSettings) { + if len(details) == 0 { + return + } + + settings.details = details + } +} + +// ListAttachments returns the live attachments of an org under an EXACT topic, +// newest first. Empty (never an error) when nothing is attached. +func (s *Service) ListAttachments(ctx context.Context, orgUID, topic string) ([]*models.File, error) { + if topic == "" { + return nil, nil + } + + files, _, err := s.db.ListFiles(ctx, orgUID, models.ListFilesFilter{Topic: topic}) + if err != nil { + return nil, fmt.Errorf("list attachments: %w", err) + } + + return files, nil +} + +// DeleteAttachments soft-deletes every live attachment of an org under a topic +// PREFIX and returns how many rows changed. Two callers: entity-deletion +// reaping (`incidents//`) and replace-on-reopen. +// +// The blobs are deliberately left in storage — same posture as DeleteFile — +// and swept later by the GC pass. +func (s *Service) DeleteAttachments(ctx context.Context, orgUID, topicPrefix string) (int, error) { + if topicPrefix == "" { + return 0, nil + } + + count, err := s.db.DeleteFilesByTopicPrefix(ctx, orgUID, topicPrefix) + if err != nil { + return 0, fmt.Errorf("delete attachments: %w", err) + } + + return count, nil +} + func (s *Service) lookup(ctx context.Context, orgSlug, fileUID string) (*models.File, error) { org, err := s.db.GetOrganizationBySlug(ctx, orgSlug) if err != nil { From 8c8da17d3b8d3043063a82ca576a70bdc44cfb8c Mon Sep 17 00:00:00 2001 From: Florent Clairambault Date: Fri, 21 Aug 2026 01:52:56 +0200 Subject: [PATCH 005/475] chore(fly): deploy jp-1 agent on 0.17.0 The machine was still serving 0.7.1. This file was edited 0.7.1 -> 0.16.2 on 2026-08-19 but 'fly deploy' was never run, so the pin, the k8xp docs and the running machine disagreed for two days -- and jp-1 kept reporting a NULL 'capabilities' the whole time, since the egress self-probe only landed in v0.16.0. Deployed for real this time; jp-1 now reports 0.17.0 with {ipv4,ipv6}. Comment rewritten to record that editing the pin is not the deploy, and that 'fly status' is the only source of truth for this app. Co-Authored-By: Claude Opus 5 --- deploy/fly/fly.nrt.toml | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/deploy/fly/fly.nrt.toml b/deploy/fly/fly.nrt.toml index 5dbdf1b50..b5f2e02dd 100644 --- a/deploy/fly/fly.nrt.toml +++ b/deploy/fly/fly.nrt.toml @@ -26,21 +26,28 @@ app = "solidping-agent-nrt" primary_region = "nrt" [build] - # Pinned to the release the API is running (k8xp dev is on 0.16.2). Agent mode + # Pinned to the release the API is running (k8xp dev is on 0.17.0). Agent mode # is a runtime role, not a separate artifact — same image the cluster runs. # - # Bumped 0.7.1 -> 0.16.2 on 2026-08-19, restoring the invariant this comment - # already stated: the pin had drifted nine minors behind the API. The visible - # symptom was that jp-1's worker row carried a NULL `capabilities` — the - # egress self-probe that fills it landed in v0.16.0 (commit a5ee7c439), so - # this agent had no code to report with. NULL means "nothing probed", not - # "no IPv6"; jp-1 has always passed the IPv6-only check. + # Bumped to 0.17.0 on 2026-08-21, deployed the same day. + # + # HISTORY / LESSON: this file was edited 0.7.1 -> 0.16.2 on 2026-08-19, but + # `fly deploy` was never run — so the machine kept serving 0.7.1 for another + # two days while both this file and the k8xp docs claimed 0.16.2. Editing the + # pin is NOT the deploy. This app is not covered by `kubectl apply -k`; it + # ships only via `fly deploy -c fly.nrt.toml`, and the only source of truth + # for what is actually running is `fly status -a solidping-agent-nrt`. + # + # The long-standing symptom was jp-1's worker row carrying a NULL + # `capabilities` — the egress self-probe that fills it landed in v0.16.0 + # (commit a5ee7c439), so 0.7.1 had no code to report with. NULL means + # "nothing probed", not "no IPv6"; jp-1 has always passed the IPv6-only check. # # Safe to redeploy: identity is pinned in the SP_AGENT_KEYS secret and is # already enrolled, so recreated machines take the RECONNECT path rather than # enrolling fresh — no new agent rows to garbage-collect (see the NO VOLUME # note below). - image = "ghcr.io/fclairamb/solidping:0.16.2" + image = "ghcr.io/fclairamb/solidping:0.17.0" [env] # The role switch. Everything else about the process follows from it. From ba21239271efad1fd20e7dbe8b99f358e2443f8c Mon Sep 17 00:00:00 2001 From: Florent Clairambault Date: Fri, 21 Aug 2026 01:55:29 +0200 Subject: [PATCH 006/475] feat: capture a screenshot on failing browser checks (opt-in) --- .../internal/checkers/checkbrowser/checker.go | 119 +++++++- .../internal/checkers/checkbrowser/config.go | 18 ++ .../checkers/checkbrowser/screenshot_test.go | 279 ++++++++++++++++++ .../checkers/checkerdef/diagnostics.go | 43 +++ 4 files changed, 457 insertions(+), 2 deletions(-) create mode 100644 server/internal/checkers/checkbrowser/screenshot_test.go diff --git a/server/internal/checkers/checkbrowser/checker.go b/server/internal/checkers/checkbrowser/checker.go index 121a9dffc..d28cd74f3 100644 --- a/server/internal/checkers/checkbrowser/checker.go +++ b/server/internal/checkers/checkbrowser/checker.go @@ -3,6 +3,7 @@ package checkbrowser import ( "context" + "log/slog" "strings" "time" @@ -13,6 +14,26 @@ import ( const microsecondsPerMilli = 1000.0 +// MaxScreenshotBytes caps a single captured PNG (spec 2026-08-21-01). +// +// Deliberately far below files.MaxFileSize (25 MB): a full-page PNG of a real +// site is tens to a few hundred KB, so anything past 4 MiB is a pathological +// page (an enormous infinite-scroll canvas, a print-stylesheet poster) whose +// capture is worth less than the storage and transfer it costs. An over-cap +// capture is DROPPED, never truncated — a half-written PNG is not an image, +// it is a corrupt file that renders as a broken icon three days later. +const MaxScreenshotBytes = 4 * 1024 * 1024 + +// screenshotTimeout time-boxes the capture. The capture runs AFTER the verdict +// is decided, so this budget can never delay or change a check's outcome; it +// exists so a wedged renderer cannot hold a browser slot open indefinitely. +const screenshotTimeout = 5 * time.Second + +// screenshotQuality is chromedp.FullScreenshot's quality argument. It is +// ignored for PNG (lossless) and only matters if this ever switches to JPEG; +// 90 is the conventional value to pass. +const screenshotQuality = 90 + // MaxConcurrentBrowsers caps simultaneous browser executions per worker // process, on BOTH the remote and the exec path. // @@ -40,6 +61,13 @@ type BrowserChecker struct { session func( ctx context.Context, cfg *BrowserConfig, start time.Time, metrics, output map[string]any, ) *checkerdef.Result + + // screenshot replaces the real chromedp capture, for tests only. Nil in + // production, where captureScreenshot falls back to fullScreenshot. It is + // a separate seam from `session` on purpose: a test needs to drive the + // capture decision (opted in? failing? over cap? errored?) without also + // having to fake a browser session that produces the right verdict. + screenshot func(ctx context.Context) ([]byte, error) } // acquireSlot waits for one of the MaxConcurrentBrowsers slots, giving up when @@ -171,7 +199,10 @@ func (c *BrowserChecker) runBrowser( } if c.session != nil { - return c.session(ctx, cfg, start, metrics, output) + result := c.session(ctx, cfg, start, metrics, output) + c.captureScreenshot(ctx, cfg, result) + + return result } allocCtx, allocCancel := allocator(ctx, current) @@ -180,7 +211,91 @@ func (c *BrowserChecker) runBrowser( browserCtx, browserCancel := browserContext(allocCtx, current) defer browserCancel() - return c.navigateAndCheck(browserCtx, cfg, start, metrics, output) + result := c.navigateAndCheck(browserCtx, cfg, start, metrics, output) + + // Capture here, not inside the verdict paths: this is the last point at + // which the browser context is still alive (the defers above run when this + // function returns), and it is the ONE place every failing path funnels + // through — a new verdict branch cannot forget to capture. + c.captureScreenshot(browserCtx, cfg, result) + + return result +} + +// captureScreenshot hangs a PNG of the failing page on the result, when the +// check opted in. Best-effort by construction: it runs AFTER the verdict is +// decided and mutates only Diagnostics, so no outcome of this function — a +// capture error, an over-cap image, a dead browser — can change whether the +// check is reported up or down. That is the whole safety argument, and it is +// why this is a void function with no error return to ignore. +func (c *BrowserChecker) captureScreenshot( + ctx context.Context, cfg *BrowserConfig, result *checkerdef.Result, +) { + if !cfg.Screenshot || result == nil || !capturableStatus(result.Status) { + return + } + + capture := c.screenshot + if capture == nil { + capture = fullScreenshot + } + + // WithoutCancel, then a fresh budget: the common capture-worthy failure IS + // the check timing out, and by then the check's own context is already + // Done — inheriting it would make every timeout capture fail instantly. + // The chromedp target lives in the context's VALUES, which WithoutCancel + // keeps, so the capture still talks to the same tab. + shotCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), screenshotTimeout) + defer cancel() + + png, err := capture(shotCtx) + if err != nil { + slog.WarnContext(ctx, "browser check: screenshot capture failed", + "url", cfg.URL, "error", err) + + return + } + + if len(png) == 0 { + return + } + + if len(png) > MaxScreenshotBytes { + slog.WarnContext(ctx, "browser check: screenshot dropped, over cap", + "url", cfg.URL, "bytes", len(png), "cap", MaxScreenshotBytes) + + return + } + + if result.Diagnostics == nil { + result.Diagnostics = &checkerdef.Diagnostics{} + } + + result.Diagnostics.Screenshot = &checkerdef.Screenshot{ + PNG: png, + CapturedAt: time.Now(), + } +} + +// capturableStatus reports whether a verdict is worth a screenshot. +// +// StatusError is excluded on purpose: from this checker it means "no browser +// to drive" (Chrome missing, CDP endpoint unreachable), so there is no page to +// photograph and the attempt would only log noise. Down and Timeout are the +// two verdicts where a real tab really did look at the target. +func capturableStatus(status checkerdef.Status) bool { + return status == checkerdef.StatusDown || status == checkerdef.StatusTimeout +} + +// fullScreenshot is the production capture: a full-page PNG of the current tab. +func fullScreenshot(ctx context.Context) ([]byte, error) { + var buf []byte + + if err := chromedp.Run(ctx, chromedp.FullScreenshot(&buf, screenshotQuality)); err != nil { + return nil, err + } + + return buf, nil } // allocator builds the chromedp allocator for the configured backend: a remote diff --git a/server/internal/checkers/checkbrowser/config.go b/server/internal/checkers/checkbrowser/config.go index 4260e704f..1ef541842 100644 --- a/server/internal/checkers/checkbrowser/config.go +++ b/server/internal/checkers/checkbrowser/config.go @@ -20,6 +20,14 @@ type BrowserConfig struct { Keyword string `json:"keyword,omitempty"` InvertKeyword bool `json:"invertKeyword,omitempty"` Timeout time.Duration `json:"timeout,omitempty"` + // Screenshot opts this check into capturing a PNG of the page when the + // execution FAILS (spec 2026-08-21-01). Default false, and deliberately so: + // a capture costs a CDP round-trip and up to a few MiB of memory on the + // most expensive check type there is, and most operators never need it. + // + // Opting in changes storage, never verdicts — the capture is time-boxed and + // every failure path drops it silently (see captureScreenshot). + Screenshot bool `json:"screenshot,omitempty"` } // FromMap populates the configuration from a map. @@ -46,6 +54,12 @@ func (c *BrowserConfig) FromMap(configMap map[string]any) error { c.InvertKeyword = ik } + if shot, ok := configMap["screenshot"].(bool); ok { + c.Screenshot = shot + } else if configMap["screenshot"] != nil { + return checkerdef.NewConfigError("screenshot", "must be a boolean") + } + if t, ok := configMap["timeout"].(string); ok { duration, err := time.ParseDuration(t) if err != nil { @@ -78,6 +92,10 @@ func (c *BrowserConfig) GetConfig() map[string]any { cfg["invertKeyword"] = c.InvertKeyword } + if c.Screenshot { + cfg["screenshot"] = c.Screenshot + } + if c.Timeout != 0 { cfg["timeout"] = c.Timeout.String() } diff --git a/server/internal/checkers/checkbrowser/screenshot_test.go b/server/internal/checkers/checkbrowser/screenshot_test.go new file mode 100644 index 000000000..7722bdba7 --- /dev/null +++ b/server/internal/checkers/checkbrowser/screenshot_test.go @@ -0,0 +1,279 @@ +package checkbrowser + +import ( + "context" + "encoding/json" + "errors" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/fclairamb/solidping/server/internal/agents" + "github.com/fclairamb/solidping/server/internal/checkers/checkerdef" +) + +// fakePNG is a recognizable byte blob standing in for a real capture. Nothing +// in the capture path parses the image, so its content only has to be unique +// enough to assert on. +var fakePNG = []byte("\x89PNG\r\n\x1a\nfake-screenshot-bytes") + +// screenshotChecker builds a checker whose session returns `status` and whose +// capture seam returns whatever `capture` says. Both seams are needed: one +// decides the verdict, the other decides what the capture does. +func screenshotChecker( + status checkerdef.Status, capture func(ctx context.Context) ([]byte, error), +) *BrowserChecker { + return &BrowserChecker{ + session: func( + _ context.Context, _ *BrowserConfig, start time.Time, metrics, output map[string]any, + ) *checkerdef.Result { + return &checkerdef.Result{ + Status: status, Duration: time.Since(start), Metrics: metrics, Output: output, + } + }, + screenshot: capture, + } +} + +func screenshotSpec(enabled bool) *BrowserConfig { + return &BrowserConfig{URL: "https://example.com", Timeout: 5 * time.Second, Screenshot: enabled} +} + +// TestScreenshotCapturedOnlyOnOptedInFailure is the core behaviour: the capture +// happens when the check opted in AND failed, and in no other combination. +// +//nolint:paralleltest // mutates the process-wide settings +func TestScreenshotCapturedOnlyOnOptedInFailure(t *testing.T) { + server := fakeCDPServer(t) + withSettings(t, Settings{CDPURL: server.URL}) + + cases := []struct { + name string + enabled bool + status checkerdef.Status + want bool + }{ + {"opted in and down", true, checkerdef.StatusDown, true}, + {"opted in and timed out", true, checkerdef.StatusTimeout, true}, + {"opted in but up", true, checkerdef.StatusUp, false}, + {"opted in but infrastructure error", true, checkerdef.StatusError, false}, + {"failing but not opted in", false, checkerdef.StatusDown, false}, + } + + for _, tc := range cases { + //nolint:paralleltest // shares the process-wide settings installed above + t.Run(tc.name, func(t *testing.T) { + r := require.New(t) + + calls := 0 + checker := screenshotChecker(tc.status, func(context.Context) ([]byte, error) { + calls++ + + return fakePNG, nil + }) + + result, err := checker.Execute(t.Context(), screenshotSpec(tc.enabled)) + r.NoError(err) + r.Equal(tc.status, result.Status, "the capture must never change the verdict") + + if !tc.want { + r.Zero(calls, "no capture should have been attempted") + if result.Diagnostics != nil { + r.Nil(result.Diagnostics.Screenshot) + } + + return + } + + r.Equal(1, calls) + r.NotNil(result.Diagnostics) + r.NotNil(result.Diagnostics.Screenshot) + r.Equal(fakePNG, result.Diagnostics.Screenshot.PNG) + r.False(result.Diagnostics.Screenshot.CapturedAt.IsZero()) + }) + } +} + +// TestScreenshotFailureNeverChangesOutcome pins the safety property: whatever +// the capture does — error, empty, oversize — the check reports exactly the +// verdict it would have reported with the feature switched off. +// +//nolint:paralleltest // mutates the process-wide settings +func TestScreenshotFailureNeverChangesOutcome(t *testing.T) { + server := fakeCDPServer(t) + withSettings(t, Settings{CDPURL: server.URL}) + + cases := []struct { + name string + capture func(ctx context.Context) ([]byte, error) + }{ + {"capture errors", func(context.Context) ([]byte, error) { + return nil, errors.New("renderer gone") + }}, + {"capture returns nothing", func(context.Context) ([]byte, error) { + return nil, nil + }}, + {"capture is over the cap", func(context.Context) ([]byte, error) { + return make([]byte, MaxScreenshotBytes+1), nil + }}, + } + + for _, tc := range cases { + //nolint:paralleltest // shares the process-wide settings installed above + t.Run(tc.name, func(t *testing.T) { + r := require.New(t) + + checker := screenshotChecker(checkerdef.StatusDown, tc.capture) + + result, err := checker.Execute(t.Context(), screenshotSpec(true)) + r.NoError(err) + r.Equal(checkerdef.StatusDown, result.Status) + + if result.Diagnostics != nil { + r.Nil(result.Diagnostics.Screenshot, + "a failed or unusable capture must be dropped, never attached") + } + }) + } + + // Positive control: the same harness DOES attach a usable capture, so the + // assertions above are about the failure handling and not about a capture + // path that never attaches anything. + //nolint:paralleltest // shares the process-wide settings installed above + t.Run("positive control", func(t *testing.T) { + r := require.New(t) + + checker := screenshotChecker(checkerdef.StatusDown, func(context.Context) ([]byte, error) { + return make([]byte, MaxScreenshotBytes), nil + }) + + result, err := checker.Execute(t.Context(), screenshotSpec(true)) + r.NoError(err) + r.NotNil(result.Diagnostics) + r.NotNil(result.Diagnostics.Screenshot) + r.Len(result.Diagnostics.Screenshot.PNG, MaxScreenshotBytes, + "exactly at the cap is accepted; only past it is dropped") + }) +} + +// TestScreenshotCaptureSurvivesAnExpiredCheckContext proves the WithoutCancel +// detachment is load-bearing: the capture-worthy failure IS the timeout, and by +// then the check's own context is already Done. A capture inheriting it would +// never run. +// +//nolint:paralleltest // mutates the process-wide settings +func TestScreenshotCaptureSurvivesAnExpiredCheckContext(t *testing.T) { + server := fakeCDPServer(t) + withSettings(t, Settings{CDPURL: server.URL}) + + r := require.New(t) + + var sawDeadline bool + + checker := &BrowserChecker{ + session: func( + ctx context.Context, _ *BrowserConfig, start time.Time, metrics, output map[string]any, + ) *checkerdef.Result { + // Burn the whole (deliberately tiny) check budget, exactly as a + // real timing-out navigation would. + <-ctx.Done() + + return &checkerdef.Result{ + Status: checkerdef.StatusTimeout, Duration: time.Since(start), + Metrics: metrics, Output: output, + } + }, + screenshot: func(ctx context.Context) ([]byte, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + + _, sawDeadline = ctx.Deadline() + + return fakePNG, nil + }, + } + + spec := &BrowserConfig{URL: "https://example.com", Timeout: 20 * time.Millisecond, Screenshot: true} + + result, err := checker.Execute(t.Context(), spec) + r.NoError(err) + r.Equal(checkerdef.StatusTimeout, result.Status) + r.NotNil(result.Diagnostics) + r.NotNil(result.Diagnostics.Screenshot, "a timed-out check must still get its capture") + r.True(sawDeadline, "the capture runs on its own fresh budget, not an already-expired one") +} + +// TestScreenshotBytesNeverCrossTheAgentControlChannel is the security pin from +// the spec: Diagnostics rides the agent WebSocket result frame as JSON, and a +// megabyte PNG must never be smuggled through it as base64. +// +// FailureResponse is the positive control — it is on the SAME struct and it +// DOES serialize, so an assertion that merely proved "the frame is small" would +// pass even if the whole Diagnostics field had been dropped by accident. +func TestScreenshotBytesNeverCrossTheAgentControlChannel(t *testing.T) { + t.Parallel() + + r := require.New(t) + + const bodyMarker = "acme-origin-error-page" + + pngMarker := []byte("SCREENSHOT-BYTES-MUST-NOT-APPEAR") + + frame := agents.ClientFrame{ + Type: agents.MsgTypeResult, + JobUID: "job-1", + Diagnostics: &checkerdef.Diagnostics{ + FailureResponse: &checkerdef.FailureResponse{ + StatusCode: 503, + Body: "" + bodyMarker + "", + }, + Screenshot: &checkerdef.Screenshot{ + PNG: pngMarker, + CapturedAt: time.Now(), + Available: true, + CaptureID: "cap-42", + }, + }, + } + + raw, err := json.Marshal(frame) + r.NoError(err) + + // Positive control: the frame really does carry diagnostics. + r.Contains(string(raw), "diagnostics") + r.Contains(string(raw), bodyMarker, + "FailureResponse still serializes — that is what makes the absence below meaningful") + + // The marker bytes, and any base64 encoding of them, must be absent. + r.NotContains(string(raw), string(pngMarker)) + r.NotContains(string(raw), `"png"`) + r.NotContains(string(raw), `"PNG"`) + + // The MARKER fields the agent path does need are still there. + r.Contains(string(raw), "cap-42") + r.Contains(string(raw), `"available":true`) +} + +// TestScreenshotConfigRoundTrip pins the opt-in flag through FromMap/GetConfig, +// including the rejection of a non-boolean. +func TestScreenshotConfigRoundTrip(t *testing.T) { + t.Parallel() + + r := require.New(t) + + cfg := &BrowserConfig{} + r.NoError(cfg.FromMap(map[string]any{"url": "https://example.com", "screenshot": true})) + r.True(cfg.Screenshot) + r.Equal(true, cfg.GetConfig()["screenshot"]) + + // Default is off, and an off flag is omitted from the config map entirely. + off := &BrowserConfig{} + r.NoError(off.FromMap(map[string]any{"url": "https://example.com"})) + r.False(off.Screenshot) + r.NotContains(off.GetConfig(), "screenshot") + + bad := &BrowserConfig{} + r.Error(bad.FromMap(map[string]any{"url": "https://example.com", "screenshot": "yes"})) +} diff --git a/server/internal/checkers/checkerdef/diagnostics.go b/server/internal/checkers/checkerdef/diagnostics.go index 015d2431f..434c593b2 100644 --- a/server/internal/checkers/checkerdef/diagnostics.go +++ b/server/internal/checkers/checkerdef/diagnostics.go @@ -25,6 +25,49 @@ type Diagnostics struct { // response, so the capture degrades to absent and the existing error // output remains the only evidence. FailureResponse *FailureResponse `json:"failureResponse,omitempty"` + + // Screenshot is the capture of what a FAILING browser check's page looked + // like. Nil unless the check opted in (browser: `screenshot`) and the + // capture succeeded. + // + // See Screenshot's own doc comment for why its bytes never cross the agent + // WS control channel. + Screenshot *Screenshot `json:"screenshot,omitempty"` +} + +// Screenshot is a PNG capture of the page a failing browser check was looking +// at, taken before the browser context is disposed. +// +// HONESTY ABOUT WHAT THIS IS: it is what the page looked like a moment AFTER +// the check decided the target was unhealthy, not the frame at the instant of +// failure. Every surface that renders it must say so — presenting it as "the +// failure" invites an operator to conclude the wrong thing from a page that +// finished loading half a second later. +// +// THE BYTES NEVER CROSS THE CONTROL CHANNEL. Diagnostics is serialized onto +// the agent WebSocket result frame (internal/agents/protocol.go), which is +// JSON: a megabyte PNG would become a multi-megabyte base64 blob on the socket +// every agent uses to claim work. PNG is therefore `json:"-"` — a deported +// agent uploads its bytes out-of-band to POST /api/v1/agent/attachments and +// advertises only the marker fields below. The IN-PROCESS worker path keeps +// the bytes in memory and never serializes them at all, which is why the field +// works there with no wire representation. +type Screenshot struct { + // PNG is the raw image. NEVER SERIALIZED — see the type doc. + PNG []byte `json:"-"` + // CapturedAt is when the screenshot was taken. + CapturedAt time.Time `json:"capturedAt,omitzero"` + // Available is the agent-side MARKER: "I hold a capture for this result". + // It is what crosses the wire in place of the bytes, so the server can ask + // for the upload if (and only if) this result opens an incident. + Available bool `json:"available,omitempty"` + // CaptureID names the capture in the agent's local LRU, so the server's + // upload request can identify which one it wants. + CaptureID string `json:"captureId,omitempty"` + // Region is the probing region. Filled SERVER-SIDE from the persisted + // result row, never by the checker — a deported agent must not be the + // authority on where it ran. + Region string `json:"region,omitempty"` } // FailureResponse is the textual capture of the response a failing probe From 0538be9fdb381a5c07d4e9dd0244842ba572b616 Mon Sep 17 00:00:00 2001 From: Florent Clairambault Date: Fri, 21 Aug 2026 02:03:04 +0200 Subject: [PATCH 007/475] feat: persist incident screenshots and add the agent attachment upload endpoint --- server/internal/app/server.go | 19 +- server/internal/checkworker/worker.go | 10 + server/internal/db/postgres/postgres.go | 18 + server/internal/db/service.go | 5 + server/internal/db/sqlite/sqlite.go | 18 + .../internal/handlers/attachments/handler.go | 291 +++++++++++++++ .../internal/handlers/attachments/service.go | 334 ++++++++++++++++++ .../internal/handlers/attachments/topics.go | 173 +++++++++ server/internal/handlers/incidents/service.go | 153 ++++++++ server/internal/notifications/slack_test.go | 14 + 10 files changed, 1034 insertions(+), 1 deletion(-) create mode 100644 server/internal/handlers/attachments/handler.go create mode 100644 server/internal/handlers/attachments/service.go create mode 100644 server/internal/handlers/attachments/topics.go diff --git a/server/internal/app/server.go b/server/internal/app/server.go index e7147befc..e8cd64062 100644 --- a/server/internal/app/server.go +++ b/server/internal/app/server.go @@ -46,6 +46,7 @@ import ( entitlementsapi "github.com/fclairamb/solidping/server/internal/entitlements" agentsadmin "github.com/fclairamb/solidping/server/internal/handlers/agents" "github.com/fclairamb/solidping/server/internal/handlers/agentws" + "github.com/fclairamb/solidping/server/internal/handlers/attachments" "github.com/fclairamb/solidping/server/internal/handlers/auth" "github.com/fclairamb/solidping/server/internal/handlers/availability" "github.com/fclairamb/solidping/server/internal/handlers/badges" @@ -1067,6 +1068,12 @@ func (s *Server) SetupRoutes(ctx context.Context) { orgAgentsAdmin.GET("/agents", agentsAdminHandler.ListAgents) orgAgentsAdmin.DELETE("/agents/:uid", agentsAdminHandler.RevokeAgent) + // Files + attachments. Created HERE rather than next to the /files routes + // below because three later blocks need them: the agent upload endpoint, + // the agent-path incident pipeline, and the dashboard-path one. + filesService := files.NewService(s.dbService, s.config) + attachmentsService := attachments.NewService(filesService, s.dbService, s.config) + // Deported-agent WebSocket transport (spec 2026-07-16-02). Registered // directly on `api` — it carries its own auth, performed BEFORE the // upgrade: a one-shot spe_ enrollment bearer on first connect, Ed25519 @@ -1098,6 +1105,16 @@ func (s *Server) SetupRoutes(ctx context.Context) { ) api.GET("/agent/ws", agentWSHandler.Serve) + // Agent attachment upload (spec 2026-08-21-01) — the WS route's sibling, + // and its counterpart: the socket is the JSON control channel, this is the + // only way binary bytes get in. It carries the SAME Ed25519 signed-header + // auth performed inline, so it is registered on `api` and never on an + // authenticated group. + agentAttachmentsHandler := attachments.NewHandler(attachmentsService, s.dbService, s.config) + api.POST("/agent/attachments", agentAttachmentsHandler.Upload) + + agentWorkerIncidents.SetAttachmentStore(attachmentsService) + // Results routes (authentication required) resultsService := results.NewService(s.dbService) resultsHandler := results.NewHandler(resultsService, s.config) @@ -1117,6 +1134,7 @@ func (s *Server) SetupRoutes(ctx context.Context) { // Incidents routes (authentication required) incidentsService := incidents.NewService(s.dbService, s.jobSvc, s.services.Clock, s.services.Realtime) incidentsService.SetPublicationHook(incidentPublicationsService) + incidentsService.SetAttachmentStore(attachmentsService) incidentsHandler := incidents.NewHandler(incidentsService, s.config) orgIncidents := orgGroup("/orgs/:org/incidents") orgIncidents.GET("", incidentsHandler.ListIncidents) @@ -1260,7 +1278,6 @@ func (s *Server) SetupRoutes(ctx context.Context) { api.GET("/orgs/:org/events/ws", wsHandler.Serve) // Files routes (authentication required for org-scoped, plus public signed-URL route) - filesService := files.NewService(s.dbService, s.config) filesHandler := files.NewHandler(filesService, s.config) orgFiles := orgGroup("/orgs/:org/files") orgFiles.GET("", filesHandler.List) diff --git a/server/internal/checkworker/worker.go b/server/internal/checkworker/worker.go index a403083cd..9a10aae75 100644 --- a/server/internal/checkworker/worker.go +++ b/server/internal/checkworker/worker.go @@ -35,6 +35,8 @@ import ( "github.com/fclairamb/solidping/server/internal/db/models" "github.com/fclairamb/solidping/server/internal/entitlements" "github.com/fclairamb/solidping/server/internal/errorreport" + "github.com/fclairamb/solidping/server/internal/handlers/attachments" + "github.com/fclairamb/solidping/server/internal/handlers/files" "github.com/fclairamb/solidping/server/internal/handlers/incidentpublications" "github.com/fclairamb/solidping/server/internal/handlers/incidents" "github.com/fclairamb/solidping/server/internal/handlers/statussubscribers" @@ -209,6 +211,14 @@ func NewCheckWorker( incidentSvc.SetPublicationHook(publicationSvc) + // Incident screenshots (spec 2026-08-21-01) are captured by the IN-PROCESS + // browser checker, so this is the path that actually persists them — the + // HTTP server's own incident service would only ever see results submitted + // by remote workers. + incidentSvc.SetAttachmentStore( + attachments.NewService(files.NewService(dbService, cfg), dbService, cfg), + ) + directBackend := backend.NewDirectBackend( dbService, checkJobSvc, incidentSvc, svc.EventNotifier, svc.Credentials, ) diff --git a/server/internal/db/postgres/postgres.go b/server/internal/db/postgres/postgres.go index 0242808e4..d7b06d293 100644 --- a/server/internal/db/postgres/postgres.go +++ b/server/internal/db/postgres/postgres.go @@ -6350,3 +6350,21 @@ func (s *Service) ListAttachmentsByTopicPrefix( return files, nil } + +// GetIncidentAny retrieves an incident by UID without org scoping. Used by the +// attachment topic authorizer, which derives the organization FROM the incident +// rather than trusting the caller for it. +func (s *Service) GetIncidentAny(ctx context.Context, uid string) (*models.Incident, error) { + incident := new(models.Incident) + + err := s.db.NewSelect(). + Model(incident). + Where("uid = ?", uid). + Where("deleted_at IS NULL"). + Scan(ctx) + if err != nil { + return nil, err + } + + return incident, nil +} diff --git a/server/internal/db/service.go b/server/internal/db/service.go index b656fb28c..21bc4a9db 100644 --- a/server/internal/db/service.go +++ b/server/internal/db/service.go @@ -422,6 +422,11 @@ type Service interface { // Incident operations CreateIncident(ctx context.Context, incident *models.Incident) error GetIncident(ctx context.Context, orgUID, uid string) (*models.Incident, error) + // GetIncidentAny looks an incident up by UID with NO org scoping. Its only + // caller is the attachment topic authorizer (spec 2026-08-21-01), which has + // no org to scope by BY DESIGN: the incident row is what names the + // organization, so that a caller cannot pick one by forging a topic. + GetIncidentAny(ctx context.Context, uid string) (*models.Incident, error) // GetIncidentByNumber resolves the short per-org `#42` reference — the form // humans type into Telegram and read in Slack. Returns sql.ErrNoRows if none. GetIncidentByNumber(ctx context.Context, orgUID string, number int64) (*models.Incident, error) diff --git a/server/internal/db/sqlite/sqlite.go b/server/internal/db/sqlite/sqlite.go index 7d2ec273d..227f4216a 100644 --- a/server/internal/db/sqlite/sqlite.go +++ b/server/internal/db/sqlite/sqlite.go @@ -6308,3 +6308,21 @@ func (s *Service) ListAttachmentsByTopicPrefix( return files, nil } + +// GetIncidentAny retrieves an incident by UID without org scoping. Used by the +// attachment topic authorizer, which derives the organization FROM the incident +// rather than trusting the caller for it. +func (s *Service) GetIncidentAny(ctx context.Context, uid string) (*models.Incident, error) { + incident := new(models.Incident) + + err := s.db.NewSelect(). + Model(incident). + Where("uid = ?", uid). + Where("deleted_at IS NULL"). + Scan(ctx) + if err != nil { + return nil, err + } + + return incident, nil +} diff --git a/server/internal/handlers/attachments/handler.go b/server/internal/handlers/attachments/handler.go new file mode 100644 index 000000000..f65e05ae0 --- /dev/null +++ b/server/internal/handlers/attachments/handler.go @@ -0,0 +1,291 @@ +package attachments + +import ( + "errors" + "log/slog" + "net/http" + "sync" + "time" + + "github.com/fclairamb/solidping/server/internal/agents" + "github.com/fclairamb/solidping/server/internal/config" + "github.com/fclairamb/solidping/server/internal/db" + "github.com/fclairamb/solidping/server/internal/db/models" + "github.com/fclairamb/solidping/server/internal/handlers/base" +) + +// Signed-request headers. Identical to the agent WebSocket reconnect headers — +// same scheme, same skew, same replay guard — because an agent that can prove +// it is itself for a socket upgrade can prove it the same way for an upload. +// Reusing the scheme is the point: no second credential, no bearer token, and +// no S3 credentials on an agent, ever. +const ( + headerAgentUID = "X-Sp-Agent-Uid" + headerTimestamp = "X-Sp-Timestamp" + headerNonce = "X-Sp-Nonce" + headerSignature = "X-Sp-Signature" +) + +// nonceRetention mirrors agentws: twice the accepted clock skew, so a captured +// signature cannot be replayed anywhere inside the window it is valid in. +const nonceRetention = 2 * agents.DefaultClockSkew + +// uploadRateWindow / uploadRateBurst bound how often one agent may upload. +// +// Generous relative to the real traffic (attachments are minted on incident +// transitions, not per probe) and small in absolute terms, because the cost +// being bounded is storage, not CPU: an agent that has been compromised should +// not be able to fill a bucket faster than an operator notices. +const ( + uploadRateWindow = time.Minute + uploadRateBurst = 20 +) + +// Handler serves POST /api/v1/agent/attachments — the ONE endpoint through +// which a deported agent gets binary bytes into storage. +// +// It exists because the WS control channel is JSON: a PNG there would be +// base64 on the socket every agent uses to claim work. Agents hold no storage +// credentials by design, so the server is the only thing that can write the +// blob, and this is the narrow door it does it through. +type Handler struct { + base.HandlerBase + svc *Service + dbService db.Service + nonces agents.NonceGuard + authorizer *AuthorizerRegistry + limiter *agentRateLimiter + logger *slog.Logger +} + +// NewHandler builds the agent attachment upload handler with the default +// authorizer set (today: incidents). +func NewHandler(svc *Service, dbService db.Service, cfg *config.Config) *Handler { + registry := NewAuthorizerRegistry() + registry.Register(EntityIncidents, NewIncidentAuthorizer(dbService)) + + return &Handler{ + HandlerBase: base.NewHandlerBase(cfg), + svc: svc, + dbService: dbService, + nonces: agents.NewStoredNonceGuard(dbService, db.ErrAgentNonceReplayed), + authorizer: registry, + limiter: newAgentRateLimiter(uploadRateWindow, uploadRateBurst), + logger: slog.Default().With("component", "agent_attachments"), + } +} + +// SetNonceGuard swaps the replay guard. For tests and for embeddings with no +// `agent_nonces` table — same escape hatch agentws exposes. +func (h *Handler) SetNonceGuard(guard agents.NonceGuard) { + h.nonces = guard +} + +// Registry exposes the authorizer registry so a future attachment kind can +// register its own topic prefix without touching this handler. +func (h *Handler) Registry() *AuthorizerRegistry { + return h.authorizer +} + +// UploadResponse is the body returned on a successful upload. The agent +// references the file by uid from its result/diagnostics instead of carrying +// bytes anywhere else. +type UploadResponse struct { + FileUID string `json:"fileUid"` +} + +// Upload handles POST /api/v1/agent/attachments?topic=. +// +// The order of the checks below is deliberate and each one fails CLOSED: +// authenticate the agent, rate-limit it, validate the topic's SHAPE, ask the +// topic's authorizer who owns it (which is where the org comes from), and only +// then read and sniff the body. +func (h *Handler) Upload(writer http.ResponseWriter, req *http.Request) error { + agent, authErr := h.authenticate(req) + if authErr != nil { + return h.WriteError(writer, http.StatusUnauthorized, base.ErrorCodeInvalidToken, authErr.Error()) + } + + if !h.limiter.allow(agent.UID, time.Now()) { + return h.WriteError(writer, http.StatusTooManyRequests, base.ErrorCodeValidationError, + "Too many attachment uploads from this agent") + } + + topic, err := ParseTopic(req.URL.Query().Get("topic")) + if err != nil { + return h.WriteError(writer, http.StatusBadRequest, base.ErrorCodeValidationError, err.Error()) + } + + orgUID, err := h.authorizer.Authorize(req.Context(), topic, UploaderIdentity{ + AgentUID: agent.UID, + AgentOrgUID: agent.OrgUID(), + AgentRegion: agent.Region, + }) + if err != nil { + // 403, not 404: the caller authenticated fine, it simply may not write + // here. The message never says WHICH of the refusals fired — that + // would turn the endpoint into an incident-existence oracle for any + // enrolled agent. + h.logger.WarnContext(req.Context(), "agent attachment upload refused", + "agent", agent.UID, "topic", topic.Entity+"/"+topic.EntityUID+"/"+topic.Kind, + "error", err) + + return h.WriteError(writer, http.StatusForbidden, base.ErrorCodeForbidden, + "Not allowed to attach to this topic") + } + + body, err := ReadCapped(req.Body) + if err != nil { + if errors.Is(err, ErrAttachmentTooLarge) { + return h.WriteError(writer, http.StatusRequestEntityTooLarge, + base.ErrorCodeValidationError, "Attachment exceeds the size limit") + } + + return h.WriteError(writer, http.StatusBadRequest, base.ErrorCodeValidationError, err.Error()) + } + + fileUID, err := h.svc.Put(req.Context(), orgUID, rawTopic(topic), attachmentName(topic), body, + models.JSONMap{ + DetailKeyTrigger: "agent-upload", + }) + if err != nil { + return h.writePutError(writer, req, err) + } + + return h.WriteJSON(writer, http.StatusCreated, UploadResponse{FileUID: fileUID}) +} + +// writePutError maps the service's refusals to statuses. Everything unmapped is +// a genuine 500 and is reported. +func (h *Handler) writePutError(writer http.ResponseWriter, req *http.Request, err error) error { + switch { + case errors.Is(err, ErrEmptyAttachment): + return h.WriteError(writer, http.StatusBadRequest, base.ErrorCodeValidationError, + "Attachment body is empty") + case errors.Is(err, ErrAttachmentTooLarge): + return h.WriteError(writer, http.StatusRequestEntityTooLarge, + base.ErrorCodeValidationError, "Attachment exceeds the size limit") + case errors.Is(err, ErrUnsupportedMediaType): + // Sniffed, not declared: a caller that labels a zip as image/png gets + // here, which is the whole point of sniffing. + return h.WriteError(writer, http.StatusUnsupportedMediaType, + base.ErrorCodeValidationError, err.Error()) + default: + return h.WriteInternalError(writer, req, err) + } +} + +func rawTopic(topic ParsedTopic) string { + return topic.Entity + "/" + topic.EntityUID + "/" + topic.Kind +} + +func attachmentName(topic ParsedTopic) string { + return topic.Entity + "-" + topic.EntityUID + "-" + topic.Kind + ".png" +} + +// errUnauthenticated is the single message every authentication failure +// produces. Distinguishing "unknown agent" from "bad signature" would leak +// which agent uids exist. +var errUnauthenticated = errors.New("Invalid agent signature") + +// authenticate verifies the signed headers exactly the way the WS reconnect +// does: known active agent, timestamp inside the skew, nonce not seen before +// (cluster-wide, DB-backed), Ed25519 signature over method|path|timestamp|nonce. +func (h *Handler) authenticate(req *http.Request) (*models.Agent, error) { + agentUID := req.Header.Get(headerAgentUID) + timestamp := req.Header.Get(headerTimestamp) + nonce := req.Header.Get(headerNonce) + signature := req.Header.Get(headerSignature) + + if agentUID == "" || timestamp == "" || nonce == "" || signature == "" { + return nil, errUnauthenticated + } + + agent, err := h.dbService.GetAgent(req.Context(), agentUID) + if err != nil || agent == nil { + return nil, errUnauthenticated + } + + if agent.Status != models.AgentStatusActive { + return nil, errUnauthenticated + } + + if tsErr := agents.ValidateTimestamp(timestamp, time.Now(), agents.DefaultClockSkew); tsErr != nil { + return nil, errUnauthenticated + } + + // Replay guard BEFORE the signature check, same order as agentws: a valid + // signature presented twice must fail the second time, and the guard is + // what makes that true. Fails closed — a store error rejects the upload. + if nonceErr := h.nonces.CheckAndStore( + req.Context(), agent.UID, nonce, time.Now(), nonceRetention, + ); nonceErr != nil { + return nil, errUnauthenticated + } + + if sigErr := agents.VerifySignature( + agent.Ed25519PublicKey, req.Method, req.URL.Path, timestamp, nonce, signature, + ); sigErr != nil { + return nil, errUnauthenticated + } + + return agent, nil +} + +// agentRateLimiter is a per-agent fixed-window counter. Fixed window rather +// than a token bucket because the quantity being bounded is "blobs written per +// minute", where the edge-of-window doubling a fixed window allows is +// irrelevant and the simplicity is worth more. +type agentRateLimiter struct { + mu sync.Mutex + window time.Duration + burst int + buckets map[string]*rateBucket +} + +type rateBucket struct { + windowStart time.Time + count int +} + +func newAgentRateLimiter(window time.Duration, burst int) *agentRateLimiter { + return &agentRateLimiter{ + window: window, + burst: burst, + buckets: make(map[string]*rateBucket), + } +} + +// allow reports whether this agent may upload now, and counts the attempt. +func (l *agentRateLimiter) allow(agentUID string, now time.Time) bool { + l.mu.Lock() + defer l.mu.Unlock() + + bucket, ok := l.buckets[agentUID] + if !ok || now.Sub(bucket.windowStart) >= l.window { + l.buckets[agentUID] = &rateBucket{windowStart: now, count: 1} + + // Opportunistic sweep: the map is keyed by agent uid and would + // otherwise grow with every agent that ever uploaded. + l.sweepLocked(now) + + return true + } + + if bucket.count >= l.burst { + return false + } + + bucket.count++ + + return true +} + +// sweepLocked drops buckets whose window has fully elapsed. Caller holds the lock. +func (l *agentRateLimiter) sweepLocked(now time.Time) { + for uid, bucket := range l.buckets { + if now.Sub(bucket.windowStart) >= l.window { + delete(l.buckets, uid) + } + } +} diff --git a/server/internal/handlers/attachments/service.go b/server/internal/handlers/attachments/service.go new file mode 100644 index 000000000..fdcaf0724 --- /dev/null +++ b/server/internal/handlers/attachments/service.go @@ -0,0 +1,334 @@ +package attachments + +import ( + "bytes" + "context" + "errors" + "fmt" + "io" + "slices" + "time" + + "github.com/google/uuid" + + "github.com/fclairamb/solidping/server/internal/config" + "github.com/fclairamb/solidping/server/internal/db" + "github.com/fclairamb/solidping/server/internal/db/models" + "github.com/fclairamb/solidping/server/internal/handlers/files" + "github.com/fclairamb/solidping/server/internal/handlers/files/signedurl" + "github.com/fclairamb/solidping/server/internal/handlers/filestorage" +) + +// MaxAttachmentBytes caps a single uploaded attachment. Mirrors the browser +// checker's own capture cap so an in-process capture and an agent upload of the +// same screenshot are subject to the same ceiling — a per-path cap would mean +// "what fits" depends on where the check happened to run. +const MaxAttachmentBytes int64 = 4 * 1024 * 1024 + +// DownloadURLTTL is how long a signed attachment URL stays valid. Short: the +// dashboard re-fetches the incident (and therefore re-signs) whenever it +// renders it, so a long-lived URL would only widen the window in which a +// copied link keeps working outside the session that produced it. +const DownloadURLTTL = time.Hour + +// mimePNG is the only attachment content type accepted today. +const mimePNG = "image/png" + +// pngMagic is the 8-byte PNG signature. Content types are sniffed, not +// believed: a declared type is caller-controlled and the storage backend serves +// what it was given. +// +//nolint:gochecknoglobals // immutable byte constant +var pngMagic = []byte{0x89, 'P', 'N', 'G', '\r', '\n', 0x1a, '\n'} + +// Details keys written on an attachment's metadata bag. camelCase, matching +// the JSON they are served as. +const ( + DetailKeyCapturedAt = "capturedAt" + DetailKeyRegion = "region" + DetailKeyCheckUID = "checkUid" + DetailKeyTrigger = "trigger" +) + +// Trigger values recorded in an attachment's details bag. +const ( + // TriggerIncidentOpen marks the capture that opened an incident. + TriggerIncidentOpen = "incident-open" + // TriggerIncidentReopen marks the capture of a relapse's onset. + TriggerIncidentReopen = "incident-reopen" +) + +// Errors returned by the service. +var ( + // ErrUnsupportedMediaType means the declared or sniffed content type is + // not on the allowlist. + ErrUnsupportedMediaType = errors.New("unsupported attachment media type") + // ErrAttachmentTooLarge means the body is past MaxAttachmentBytes. + ErrAttachmentTooLarge = errors.New("attachment too large") + // ErrEmptyAttachment means the body carried no bytes. + ErrEmptyAttachment = errors.New("attachment is empty") +) + +// Service owns the attachment half of the files layer: writing an attachment +// under a topic, reaping by prefix, and rendering one for the API with a signed +// download URL. +type Service struct { + files *files.Service + dbSvc db.Service + cfg *config.Config +} + +// NewService builds the attachment service on top of the files service. +func NewService(filesSvc *files.Service, dbSvc db.Service, cfg *config.Config) *Service { + return &Service{files: filesSvc, dbSvc: dbSvc, cfg: cfg} +} + +// Response is the JSON shape of one attachment. It carries a signed download +// URL rather than the bytes: the incident payload stays small, and the URL +// expires. +// +// NEVER PUBLIC. An attachment is org-operational evidence exactly like +// incidents.details — a screenshot of an internal admin page is not something +// a status page may serve. The never-public audit +// (handlers/statuspages/details_never_public_test.go) pins that structurally. +type Response struct { + UID string `json:"uid"` + Kind string `json:"kind"` + Name string `json:"name"` + // MimeType is the stored content type, sniffed at write time. + MimeType string `json:"mimeType"` + Size int64 `json:"size"` + // DownloadURL is RELATIVE (`/pub/files/?exp=…&sig=…`) so it resolves + // against whichever host the dashboard is being served from — SolidPing + // answers on several, and an absolute URL baked from server.base_url would + // send a tenant to the wrong one. + DownloadURL string `json:"downloadUrl"` + CreatedAt time.Time `json:"createdAt"` + // CapturedAt / Region / CheckUID / Trigger are lifted out of the details + // bag so a client does not have to know the bag's schema. + CapturedAt *time.Time `json:"capturedAt,omitempty"` + Region string `json:"region,omitempty"` + CheckUID string `json:"checkUid,omitempty"` + Trigger string `json:"trigger,omitempty"` +} + +// PutIncidentScreenshot writes a PNG as the incident's screenshot attachment. +// It REPLACES any previous one: the caller's contract is "this is the evidence +// for the current onset", so the prior capture is soft-deleted first. +func (s *Service) PutIncidentScreenshot( + ctx context.Context, orgUID, incidentUID string, png []byte, details models.JSONMap, +) (string, error) { + if _, err := s.DeleteIncidentAttachments(ctx, orgUID, incidentUID); err != nil { + return "", err + } + + return s.put( + ctx, orgUID, + IncidentScreenshotTopic(incidentUID), + "incident-"+incidentUID+"-screenshot.png", + png, details, + ) +} + +// DeleteIncidentAttachments soft-deletes every attachment hanging off an +// incident. The reaper for incident deletion and the replace half of an +// on-reopen overwrite. +func (s *Service) DeleteIncidentAttachments(ctx context.Context, orgUID, incidentUID string) (int, error) { + return s.files.DeleteAttachments(ctx, orgUID, IncidentTopicPrefix(incidentUID)) +} + +// ListIncidentAttachments returns the incident's live attachments, rendered +// with signed download URLs. Empty (never an error) when there are none. +func (s *Service) ListIncidentAttachments( + ctx context.Context, orgUID, incidentUID string, +) ([]Response, error) { + rows, err := s.files.ListAttachments(ctx, orgUID, IncidentScreenshotTopic(incidentUID)) + if err != nil { + return nil, err + } + + out := make([]Response, 0, len(rows)) + for _, row := range rows { + out = append(out, s.toResponse(row)) + } + + return out, nil +} + +// Put writes an arbitrary attachment under an already-AUTHORIZED topic. The +// org must be the authorizer's answer, never a caller-supplied value. +func (s *Service) Put( + ctx context.Context, orgUID, topic, name string, body []byte, details models.JSONMap, +) (string, error) { + return s.put(ctx, orgUID, topic, name, body, details) +} + +func (s *Service) put( + ctx context.Context, orgUID, topic, name string, body []byte, details models.JSONMap, +) (string, error) { + if len(body) == 0 { + return "", ErrEmptyAttachment + } + + if int64(len(body)) > MaxAttachmentBytes { + return "", ErrAttachmentTooLarge + } + + mimeType, err := sniffMime(body) + if err != nil { + return "", err + } + + parsedOrg, err := uuid.Parse(orgUID) + if err != nil { + return "", fmt.Errorf("parse organization uid: %w", err) + } + + file, err := s.files.CreateFile( + ctx, parsedOrg, filestorage.GroupTypeScreenshots, + name, mimeType, nil, bytes.NewReader(body), int64(len(body)), + files.WithTopic(topic), files.WithDetails(details), + ) + if err != nil { + return "", fmt.Errorf("write attachment: %w", err) + } + + return file.UID, nil +} + +// sniffMime validates the bytes against the allowlist. PNG only today; the +// allowlist is a function rather than a set so a future kind (HAR, pcap) adds +// its own magic-byte check instead of trusting a header. +func sniffMime(body []byte) (string, error) { + if len(body) >= len(pngMagic) && bytes.Equal(body[:len(pngMagic)], pngMagic) { + return mimePNG, nil + } + + return "", fmt.Errorf("%w: only %s is accepted", ErrUnsupportedMediaType, mimePNG) +} + +// ReadCapped reads at most MaxAttachmentBytes+1 bytes and reports +// ErrAttachmentTooLarge when the body exceeds the cap. Reading one byte past +// the cap is what distinguishes "exactly at the limit" from "truncated". +func ReadCapped(reader io.Reader) ([]byte, error) { + body, err := io.ReadAll(io.LimitReader(reader, MaxAttachmentBytes+1)) + if err != nil { + return nil, fmt.Errorf("read attachment body: %w", err) + } + + if int64(len(body)) > MaxAttachmentBytes { + return nil, ErrAttachmentTooLarge + } + + return body, nil +} + +// toResponse renders one attachment row, signing a fresh download URL. +func (s *Service) toResponse(row *models.File) Response { + resp := Response{ + UID: row.UID, + Name: row.Name, + MimeType: row.MimeType, + Size: row.Size, + CreatedAt: row.CreatedAt, + } + + if row.Topic != nil { + if parsed, err := ParseTopic(*row.Topic); err == nil { + resp.Kind = parsed.Kind + } + } + + if fileUID, err := uuid.Parse(row.UID); err == nil { + exp, sig := signedurl.Sign([]byte(s.cfg.Auth.JWTSecret), fileUID, DownloadURLTTL) + resp.DownloadURL = signedurl.BuildURL("", fileUID, exp, sig) + } + + applyDetails(&resp, row.Details) + + return resp +} + +// applyDetails lifts the known keys out of the free-form details bag. Unknown +// keys are ignored rather than echoed: the bag is written by the server today, +// but the agent endpoint accepts one from the wire, and a response that +// reflected arbitrary caller JSON back at the dashboard is an XSS surface. +func applyDetails(resp *Response, details models.JSONMap) { + if len(details) == 0 { + return + } + + if raw, ok := details[DetailKeyCapturedAt].(string); ok { + if parsed, err := time.Parse(time.RFC3339, raw); err == nil { + resp.CapturedAt = &parsed + } + } + + if raw, ok := details[DetailKeyRegion].(string); ok { + resp.Region = raw + } + + if raw, ok := details[DetailKeyCheckUID].(string); ok { + resp.CheckUID = raw + } + + if raw, ok := details[DetailKeyTrigger].(string); ok { + resp.Trigger = raw + } +} + +// NewIncidentAuthorizer builds the `incidents//…` topic authorizer. +// +// This is the hard part of the agent upload endpoint, and it is written to be +// read as a chain of refusals: +// +// 1. The incident must exist. Its row is what names the organization — the +// request never does, so a forged topic cannot pick an org. +// 2. An ORG agent may only write to its own org's incidents. +// 3. The incident's check must be served by the region this agent is bound +// to. Without this, any enrolled agent could attach evidence to any +// incident in reach, which is how a screenshot becomes a defacement +// vector on somebody else's incident page. +func NewIncidentAuthorizer(dbSvc db.Service) TopicAuthorizer { + return AuthorizerFunc(func( + ctx context.Context, topic ParsedTopic, who UploaderIdentity, + ) (string, error) { + incident, err := dbSvc.GetIncidentAny(ctx, topic.EntityUID) + if err != nil || incident == nil { + return "", fmt.Errorf("%w: unknown incident", ErrTopicForbidden) + } + + if who.AgentOrgUID != "" && who.AgentOrgUID != incident.OrganizationUID { + return "", fmt.Errorf("%w: incident belongs to another organization", ErrTopicForbidden) + } + + check, err := dbSvc.GetCheck(ctx, incident.OrganizationUID, incident.CheckUID) + if err != nil || check == nil { + return "", fmt.Errorf("%w: incident has no readable check", ErrTopicForbidden) + } + + if !regionServesCheck(who.AgentRegion, check.Regions) { + return "", fmt.Errorf("%w: this agent's region does not serve the incident's check", ErrTopicForbidden) + } + + return incident.OrganizationUID, nil + }) +} + +// regionServesCheck reports whether an agent bound to `region` runs this check. +// +// An empty check region list means "any region", which is the default for a +// check nobody pinned — an agent in the org may legitimately have produced the +// failure. An agent with no region at all never matches: it could not have run +// anything. +func regionServesCheck(agentRegion string, checkRegions []string) bool { + if agentRegion == "" { + return false + } + + if len(checkRegions) == 0 { + return true + } + + return slices.Contains(checkRegions, agentRegion) +} diff --git a/server/internal/handlers/attachments/topics.go b/server/internal/handlers/attachments/topics.go new file mode 100644 index 000000000..034ffc252 --- /dev/null +++ b/server/internal/handlers/attachments/topics.go @@ -0,0 +1,173 @@ +// Package attachments is the generic attachment rail (spec 2026-08-21-01): the +// `files.topic` naming rules, the authorizer registry that decides whether a +// caller may write under a given topic, and the agent-facing upload endpoint +// that is its first untrusted writer. +// +// The rail is deliberately generic. Incident screenshots are its first +// consumer; HTTP-check screenshots, HAR files and packet captures are meant to +// ride the same endpoint with a new topic prefix and a new authorizer, not a +// new upload path. +package attachments + +import ( + "context" + "errors" + "fmt" + "strings" +) + +// Topic grammar. A topic is `//` — see models.File.Topic. +const ( + // EntityIncidents is the entity segment for incident attachments. + EntityIncidents = "incidents" + // KindScreenshot is the kind segment for a page capture. + KindScreenshot = "screenshot" +) + +// MaxTopicLength bounds a topic. Topics are machine-generated +// (`incidents//screenshot` is 55 chars) but the agent endpoint accepts +// one off the wire, so the length is checked rather than assumed. +const MaxTopicLength = 200 + +// Errors returned by topic parsing and authorization. +var ( + // ErrInvalidTopic means the string is not a well-formed attachment topic. + ErrInvalidTopic = errors.New("invalid attachment topic") + // ErrUnknownTopicEntity means no authorizer is registered for the topic's + // entity segment. Fails CLOSED: an unrecognized entity is rejected, never + // waved through. + ErrUnknownTopicEntity = errors.New("no authorizer registered for this attachment topic") + // ErrTopicForbidden means the caller may not write under this topic. + ErrTopicForbidden = errors.New("caller may not write under this attachment topic") +) + +// IncidentTopicPrefix returns the reap prefix for one incident: +// `incidents//`. The trailing slash is load-bearing — without it the +// prefix of incident `abc` would also match incident `abcdef`. +func IncidentTopicPrefix(incidentUID string) string { + return EntityIncidents + "/" + incidentUID + "/" +} + +// IncidentScreenshotTopic returns the exact topic an incident's screenshot is +// stored under. +func IncidentScreenshotTopic(incidentUID string) string { + return IncidentTopicPrefix(incidentUID) + KindScreenshot +} + +// ParsedTopic is a topic split into its three segments. +type ParsedTopic struct { + Entity string + EntityUID string + Kind string +} + +// ParseTopic validates and splits a topic. It is strict on purpose: this is the +// only thing standing between a wire-supplied string and a storage key, so +// anything ambiguous (empty segment, path traversal, extra segments, wildcards) +// is rejected rather than normalized. +func ParseTopic(topic string) (ParsedTopic, error) { + if topic == "" || len(topic) > MaxTopicLength { + return ParsedTopic{}, fmt.Errorf("%w: length", ErrInvalidTopic) + } + + parts := strings.Split(topic, "/") + if len(parts) != 3 { + return ParsedTopic{}, fmt.Errorf("%w: want //", ErrInvalidTopic) + } + + for _, part := range parts { + if part == "" || part == "." || part == ".." { + return ParsedTopic{}, fmt.Errorf("%w: empty or relative segment", ErrInvalidTopic) + } + + if !isTopicSegment(part) { + return ParsedTopic{}, fmt.Errorf("%w: segment %q has disallowed characters", ErrInvalidTopic, part) + } + } + + return ParsedTopic{Entity: parts[0], EntityUID: parts[1], Kind: parts[2]}, nil +} + +// isTopicSegment allows only [A-Za-z0-9._-]. That covers UUIDs and kind names +// while excluding the two characters that would make a topic dangerous: `%` +// and `_` are LIKE wildcards in the prefix reap (escaped there, but not +// relying on it twice is cheap), and `/` would let a caller invent extra +// segments. +func isTopicSegment(segment string) bool { + for _, char := range segment { + switch { + case char >= 'a' && char <= 'z', + char >= 'A' && char <= 'Z', + char >= '0' && char <= '9', + char == '-', char == '.': + default: + return false + } + } + + return true +} + +// UploaderIdentity is what an authorizer knows about the caller. It carries no +// organization: the org is the authorizer's OUTPUT, derived from the entity the +// topic names and cross-checked against the agent, never taken from the +// request. +type UploaderIdentity struct { + // AgentUID identifies the calling agent. + AgentUID string + // AgentOrgUID is the org the agent is bound to, or empty for a system + // agent (which serves a cloud region across orgs). + AgentOrgUID string + // AgentRegion is the region a system agent serves. Empty for an org agent. + AgentRegion string +} + +// TopicAuthorizer decides whether an identity may write under a topic, and +// returns the organization the resulting file belongs to. +// +// The returned org is THE authorization result, not a convenience: it comes +// from the entity the topic names, so a caller cannot widen its reach by +// naming someone else's entity — the authorizer refuses instead. +type TopicAuthorizer interface { + Authorize(ctx context.Context, topic ParsedTopic, who UploaderIdentity) (orgUID string, err error) +} + +// AuthorizerFunc adapts a function to TopicAuthorizer. +type AuthorizerFunc func(ctx context.Context, topic ParsedTopic, who UploaderIdentity) (string, error) + +// Authorize implements TopicAuthorizer. +func (f AuthorizerFunc) Authorize( + ctx context.Context, topic ParsedTopic, who UploaderIdentity, +) (string, error) { + return f(ctx, topic, who) +} + +// AuthorizerRegistry maps a topic's ENTITY segment to its authorizer. Keeping +// it a registry rather than a switch is what makes the next attachment kind a +// registration instead of an edit to the upload endpoint. +type AuthorizerRegistry struct { + byEntity map[string]TopicAuthorizer +} + +// NewAuthorizerRegistry builds an empty registry. +func NewAuthorizerRegistry() *AuthorizerRegistry { + return &AuthorizerRegistry{byEntity: make(map[string]TopicAuthorizer)} +} + +// Register attaches an authorizer to an entity segment. +func (r *AuthorizerRegistry) Register(entity string, authorizer TopicAuthorizer) { + r.byEntity[entity] = authorizer +} + +// Authorize resolves the topic's entity to its authorizer and delegates. An +// unregistered entity is ErrUnknownTopicEntity — fail closed. +func (r *AuthorizerRegistry) Authorize( + ctx context.Context, topic ParsedTopic, who UploaderIdentity, +) (string, error) { + authorizer, ok := r.byEntity[topic.Entity] + if !ok { + return "", fmt.Errorf("%w: %q", ErrUnknownTopicEntity, topic.Entity) + } + + return authorizer.Authorize(ctx, topic, who) +} diff --git a/server/internal/handlers/incidents/service.go b/server/internal/handlers/incidents/service.go index 4ba2c8611..161e1ff14 100644 --- a/server/internal/handlers/incidents/service.go +++ b/server/internal/handlers/incidents/service.go @@ -13,6 +13,7 @@ import ( "github.com/fclairamb/solidping/server/internal/db" "github.com/fclairamb/solidping/server/internal/db/models" + "github.com/fclairamb/solidping/server/internal/handlers/attachments" "github.com/fclairamb/solidping/server/internal/jobs/jobdef" "github.com/fclairamb/solidping/server/internal/jobs/jobsvc" "github.com/fclairamb/solidping/server/internal/jobs/jobtypes" @@ -100,6 +101,28 @@ const ( keyEffectiveRecoveryThreshold = "effective_recovery_threshold" ) +// AttachmentStore is the attachment side of the incident lifecycle (spec +// 2026-08-21-01). Like PublicationHook it is a deliberately small interface so +// this package never imports the files/attachment services — the dependency +// runs the other way, and the incident state machine must not be able to fail +// because a screenshot could not be written. +// +// Every call is best-effort at the call site: a missing screenshot is a +// papercut, a missing incident is an outage nobody is paged for. +type AttachmentStore interface { + // PutIncidentScreenshot stores (replacing any previous one) the PNG + // capture that is this incident's current onset evidence. + PutIncidentScreenshot( + ctx context.Context, orgUID, incidentUID string, png []byte, details models.JSONMap, + ) (string, error) + // DeleteIncidentAttachments soft-deletes everything attached to the + // incident and reports how many rows changed. + DeleteIncidentAttachments(ctx context.Context, orgUID, incidentUID string) (int, error) + // ListIncidentAttachments renders the incident's live attachments for the + // API, signed download URLs included. + ListIncidentAttachments(ctx context.Context, orgUID, incidentUID string) ([]attachments.Response, error) +} + // PublicationHook is the status-page publication side of the incident // lifecycle (spec 2026-08-19-08). It is an interface, and deliberately a small // one, so this package never imports the publication service — the dependency @@ -138,6 +161,12 @@ type Service struct { // Nil-safe — a nil publisher no-ops (realtime disabled, tests). rt *realtime.Publisher + // attachmentStore persists incident attachments (today: the browser + // screenshot of the onset). Nil-safe: every call goes through the + // attach*/reap* helpers below, which no-op when unset (tests, or a + // deployment with file storage disabled). + attachmentStore AttachmentStore + // maintenance resolves "is this check in an active maintenance window?". // Shared with the result-ingest paths (spec 2026-08-20-01) so an incident // suppressed as maintenance and a result tagged as maintenance can never @@ -162,6 +191,89 @@ func (s *Service) SetPublicationHook(hook PublicationHook) { s.publications = hook } +// SetAttachmentStore wires the attachment side. Optional. +func (s *Service) SetAttachmentStore(store AttachmentStore) { + s.attachmentStore = store +} + +// persistScreenshot writes the triggering result's browser capture as the +// incident's screenshot attachment, when there is one. +// +// TRANSITIONS ONLY. This is called from createIncident and reopenIncident and +// nowhere else, which is the whole storage argument of the feature: a 30 s +// browser check that flaps all day produces 2,880 captures, and exactly the +// handful that opened or reopened an incident are kept. Every other capture +// only ever existed in the worker's memory and dies with the result. +// +// Best-effort: a storage failure is logged and the incident proceeds. +func (s *Service) persistScreenshot( + ctx context.Context, check *models.Check, incident *models.Incident, + result *models.Result, trigger string, +) { + if s.attachmentStore == nil || result == nil || + result.Diagnostics == nil || result.Diagnostics.Screenshot == nil { + return + } + + shot := result.Diagnostics.Screenshot + if len(shot.PNG) == 0 { + // The agent path advertises a capture it holds rather than sending the + // bytes (see checkerdef.Screenshot). Nothing to persist here yet — the + // upload arrives out-of-band through POST /api/v1/agent/attachments. + return + } + + details := models.JSONMap{ + attachments.DetailKeyTrigger: trigger, + attachments.DetailKeyCheckUID: check.UID, + } + + if !shot.CapturedAt.IsZero() { + details[attachments.DetailKeyCapturedAt] = shot.CapturedAt.UTC().Format(time.RFC3339) + } + + // Region comes from the PERSISTED result row, never from the checker: a + // deported agent must not be the authority on where it ran. + if result.Region != nil { + details[attachments.DetailKeyRegion] = *result.Region + } + + if _, err := s.attachmentStore.PutIncidentScreenshot( + ctx, check.OrganizationUID, incident.UID, shot.PNG, details, + ); err != nil { + slog.WarnContext(ctx, "Failed to persist incident screenshot", + "incidentUid", incident.UID, "error", err) + } +} + +// dropStaleScreenshot removes the previous onset's evidence when a relapse +// brought none of its own. +// +// Same rule as failureResponse on reopen: a screenshot of the ORIGINAL outage +// shown next to an incident whose current onset is a different failure is +// worse than no screenshot — it invites the reader to diagnose the wrong +// thing. persistScreenshot already replaces on write, so this only has to +// handle the "relapse produced no capture" branch. +func (s *Service) dropStaleScreenshot( + ctx context.Context, check *models.Check, incident *models.Incident, result *models.Result, +) { + if s.attachmentStore == nil { + return + } + + if result != nil && result.Diagnostics != nil && + result.Diagnostics.Screenshot != nil && len(result.Diagnostics.Screenshot.PNG) > 0 { + return + } + + if _, err := s.attachmentStore.DeleteIncidentAttachments( + ctx, check.OrganizationUID, incident.UID, + ); err != nil { + slog.WarnContext(ctx, "Failed to drop stale incident screenshot", + "incidentUid", incident.UID, "error", err) + } +} + // publishOpened / publishResolved / publishReopened / publishMemberJoined are // the nil-safe wrappers every incident-lifecycle call site uses. func (s *Service) publishOpened(ctx context.Context, incident *models.Incident) { @@ -747,6 +859,10 @@ func (s *Service) createIncident(ctx context.Context, check *models.Check, resul return fmt.Errorf("failed to create incident: %w", err) } + // After the row exists: the attachment topic is keyed on the incident uid, + // so there is nothing to attach to before this point. + s.persistScreenshot(ctx, check, incident, result, attachments.TriggerIncidentOpen) + // Emit incident created event if err := s.emitEvent(ctx, check.OrganizationUID, models.EventTypeIncidentCreated, incident, models.JSONMap{ keyCheckUID: check.UID, @@ -952,6 +1068,11 @@ func (s *Service) reopenIncident( return fmt.Errorf("failed to reopen incident: %w", err) } + // A reopen is a NEW onset: the relapse's capture replaces the original + // one, and a relapse with no capture leaves no stale one behind. + s.dropStaleScreenshot(ctx, check, incident, result) + s.persistScreenshot(ctx, check, incident, result, attachments.TriggerIncidentReopen) + // Pass check fields to the incident for notification payload incident.RelapseCount = newRelapseCount @@ -1774,6 +1895,15 @@ type IncidentResponse struct { // incident open/reopen (failure_reason, first_result, and — after a // relapse — last_failure). See failureDetails / lastFailureDetails. Details models.JSONMap `json:"details,omitempty"` + // Attachments are the incident's stored evidence blobs (today: the browser + // screenshot of the onset), each with a short-lived signed download URL. + // + // Populated by the DETAIL endpoint only. The list endpoint deliberately + // leaves it empty: signing a URL per attachment per incident on a 100-row + // page is work nobody asked for, and the list view has nowhere to show one. + // + // NEVER PUBLIC — see attachments.Response. + Attachments []attachments.Response `json:"attachments,omitempty"` } // IncidentMemberResponse represents a single member of a group incident. @@ -2237,9 +2367,32 @@ func (s *Service) GetIncident( s.loadIncidentMembers(ctx, org.UID, incident, &response) } + s.loadIncidentAttachments(ctx, org.UID, incident, &response) + return &response, nil } +// loadIncidentAttachments fills the response's attachments. Best-effort: a +// storage hiccup must degrade the incident page to "no screenshot", never to a +// 500 on the page an operator opens during an outage. +func (s *Service) loadIncidentAttachments( + ctx context.Context, orgUID string, incident *models.Incident, resp *IncidentResponse, +) { + if s.attachmentStore == nil { + return + } + + list, err := s.attachmentStore.ListIncidentAttachments(ctx, orgUID, incident.UID) + if err != nil { + slog.WarnContext(ctx, "Failed to list incident attachments", + "incidentUid", incident.UID, "error", err) + + return + } + + resp.Attachments = list +} + // GetIncidentByUID gets an incident by UID, requiring organization UID. func (s *Service) GetIncidentByUID(ctx context.Context, orgUID, incidentUID string) (*models.Incident, error) { incident, err := s.db.GetIncident(ctx, orgUID, incidentUID) diff --git a/server/internal/notifications/slack_test.go b/server/internal/notifications/slack_test.go index 6ea4d2856..e12e0c428 100644 --- a/server/internal/notifications/slack_test.go +++ b/server/internal/notifications/slack_test.go @@ -1415,6 +1415,20 @@ func (m *mockDBService) ListFiles( panic("not implemented") } +func (m *mockDBService) DeleteFilesByTopicPrefix(_ context.Context, _, _ string) (int, error) { + return 0, nil +} + +func (m *mockDBService) ListAttachmentsByTopicPrefix( + _ context.Context, _ string, _ time.Time, _ int, +) ([]*models.File, error) { + return nil, nil +} + +func (m *mockDBService) GetIncidentAny(_ context.Context, _ string) (*models.Incident, error) { + return nil, nil +} + func (m *mockDBService) DeleteFile(_ context.Context, _, _ string) error { panic("not implemented") } From 3f02ada25d4b25deb6f072123904e5e87387c77a Mon Sep 17 00:00:00 2001 From: Florent Clairambault Date: Fri, 21 Aug 2026 02:09:53 +0200 Subject: [PATCH 008/475] feat: reap orphan attachments and pin attachments out of public payloads --- .../handlers/attachments/handler_test.go | 339 +++++++++++++++++ .../handlers/attachments/service_test.go | 350 ++++++++++++++++++ .../incidents/screenshot_persistence_test.go | 304 +++++++++++++++ .../statuspages/details_never_public_test.go | 75 +++- .../jobs/jobtypes/job_attachment_gc_test.go | 126 +++++++ .../jobs/jobtypes/job_state_cleanup.go | 84 +++++ 6 files changed, 1277 insertions(+), 1 deletion(-) create mode 100644 server/internal/handlers/attachments/handler_test.go create mode 100644 server/internal/handlers/attachments/service_test.go create mode 100644 server/internal/handlers/incidents/screenshot_persistence_test.go create mode 100644 server/internal/jobs/jobtypes/job_attachment_gc_test.go diff --git a/server/internal/handlers/attachments/handler_test.go b/server/internal/handlers/attachments/handler_test.go new file mode 100644 index 000000000..217fb4129 --- /dev/null +++ b/server/internal/handlers/attachments/handler_test.go @@ -0,0 +1,339 @@ +package attachments + +import ( + "bytes" + "context" + "crypto/ed25519" + "crypto/rand" + "encoding/base64" + "encoding/json" + "net/http" + "net/http/httptest" + "strconv" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/fclairamb/solidping/server/internal/agents" + "github.com/fclairamb/solidping/server/internal/config" + "github.com/fclairamb/solidping/server/internal/db" + "github.com/fclairamb/solidping/server/internal/db/models" +) + +const uploadPath = "/api/v1/agent/attachments" + +// nonceCounter makes every generated nonce unique across the package's tests. +// +//nolint:gochecknoglobals // test-only counter +var nonceCounter atomic.Uint64 + +// signedAgent is an enrolled agent plus the private key needed to sign for it. +type signedAgent struct { + agent *models.Agent + priv ed25519.PrivateKey +} + +// enrollAgent creates an active agent row bound to `region`, owned by `orgUID` +// (empty for a system agent), and returns it with its signing key. +func enrollAgent(t *testing.T, ctx context.Context, dbSvc db.Service, orgUID, region string) *signedAgent { + t.Helper() + + pub, priv, err := ed25519.GenerateKey(rand.Reader) + require.NoError(t, err) + + pubB64 := base64.StdEncoding.EncodeToString(pub) + + // Enroll through the real path (token -> EnrollAgent) rather than + // inserting a row: the agent identity this endpoint authenticates against + // is the one enrollment produces, and a hand-built row could drift from it. + rawToken, tokenHash, err := agents.GenerateEnrollmentToken() + require.NoError(t, err) + require.NotEmpty(t, rawToken) + + var token *models.AgentEnrollmentToken + if orgUID == "" { + token = models.NewSystemAgentEnrollmentToken(region, tokenHash, time.Now().Add(time.Hour), nil) + } else { + token = models.NewAgentEnrollmentToken(orgUID, region, tokenHash, time.Now().Add(time.Hour), nil) + } + + require.NoError(t, dbSvc.CreateAgentEnrollmentToken(ctx, token)) + + agent, err := dbSvc.EnrollAgent(ctx, tokenHash, "agent", pubB64, "age1fake", agents.KeyFingerprint(pubB64)) + require.NoError(t, err) + + return &signedAgent{agent: agent, priv: priv} +} + +// signedRequest builds a POST with valid signed headers for this agent. +func (s *signedAgent) signedRequest(t *testing.T, ctx context.Context, topic string, body []byte) *http.Request { + t.Helper() + + req := httptest.NewRequestWithContext(ctx, http.MethodPost, uploadPath+"?topic="+topic, bytes.NewReader(body)) + + timestamp := time.Now().UTC().Format(time.RFC3339) + + // A fresh nonce per request: the endpoint's replay guard is real, and two + // requests built in the same second would otherwise collide and make an + // unrelated assertion fail as a 401. + s.sign(req, timestamp, "nonce-"+strconv.FormatUint(nonceCounter.Add(1), 10)) + + return req +} + +// sign stamps the four signed headers onto a request. +func (s *signedAgent) sign(req *http.Request, timestamp, nonce string) { + challenge := agents.SignatureChallenge(req.Method, req.URL.Path, timestamp, nonce) + signature := base64.StdEncoding.EncodeToString(ed25519.Sign(s.priv, challenge)) + + req.Header.Set(headerAgentUID, s.agent.UID) + req.Header.Set(headerTimestamp, timestamp) + req.Header.Set(headerNonce, nonce) + req.Header.Set(headerSignature, signature) +} + +// uploadFixture is one wired-up endpoint plus the data an upload needs. +type uploadFixture struct { + ctx context.Context + db db.Service + svc *Service + handler *Handler + org *models.Organization + incident *models.Incident + check *models.Check +} + +func setupUploadTest(t *testing.T) *uploadFixture { + t.Helper() + + ctx, dbService, svc, org := setupAttachmentsTest(t) + + check := models.NewCheck(org.UID, "api", "browser") + check.Regions = []string{"eu-west"} + require.NoError(t, dbService.CreateCheck(ctx, check)) + + incident := models.NewIncident(org.UID, check.UID, time.Now(), "api is down") + require.NoError(t, dbService.CreateIncident(ctx, incident)) + + cfg := &config.Config{} + cfg.Auth.JWTSecret = "test-secret" + + handler := NewHandler(svc, dbService, cfg) + // In-memory replay guard: the sqlite test DB has the agent_nonces table, + // but an in-process cache keeps the test's intent (replay is refused) + // independent of the store's schema. + handler.SetNonceGuard(agents.NewNonceCache()) + + return &uploadFixture{ + ctx: ctx, db: dbService, svc: svc, handler: handler, + org: org, incident: incident, check: check, + } +} + +// serve runs the handler and returns the recorder. +func (f *uploadFixture) serve(t *testing.T, req *http.Request) *httptest.ResponseRecorder { + t.Helper() + + rec := httptest.NewRecorder() + require.NoError(t, f.handler.Upload(rec, req)) + + return rec +} + +// TestAgentUploadHappyPath is the positive control every rejection test below +// depends on: a correctly signed agent, serving the incident's region, gets a +// 201 and a real stored attachment. +func TestAgentUploadHappyPath(t *testing.T) { + t.Parallel() + + r := require.New(t) + f := setupUploadTest(t) + + agent := enrollAgent(t, f.ctx, f.db, f.org.UID, "eu-west") + topic := IncidentScreenshotTopic(f.incident.UID) + + rec := f.serve(t, agent.signedRequest(t, f.ctx, topic, pngBytes("agent-capture"))) + r.Equal(http.StatusCreated, rec.Code, rec.Body.String()) + + var resp UploadResponse + r.NoError(json.Unmarshal(rec.Body.Bytes(), &resp)) + r.NotEmpty(resp.FileUID) + + stored, err := f.db.GetFile(f.ctx, f.org.UID, resp.FileUID) + r.NoError(err) + r.NotNil(stored.Topic) + r.Equal(topic, *stored.Topic) + r.Equal("image/png", stored.MimeType) +} + +// TestAgentUploadRejectsBadSignature covers the authentication surface: missing +// headers, a signature from the wrong key, a stale timestamp, a revoked agent, +// and an unknown agent uid. Each must be a 401 and must store nothing. +func TestAgentUploadRejectsBadSignature(t *testing.T) { + t.Parallel() + + r := require.New(t) + f := setupUploadTest(t) + + agent := enrollAgent(t, f.ctx, f.db, f.org.UID, "eu-west") + topic := IncidentScreenshotTopic(f.incident.UID) + + t.Run("no headers", func(t *testing.T) { + req := httptest.NewRequestWithContext(f.ctx, http.MethodPost, + uploadPath+"?topic="+topic, bytes.NewReader(pngBytes("x"))) + r.Equal(http.StatusUnauthorized, f.serve(t, req).Code) + }) + + t.Run("signature from another key", func(t *testing.T) { + imposter := enrollAgent(t, f.ctx, f.db, f.org.UID, "eu-west") + + req := agent.signedRequest(t, f.ctx, topic, pngBytes("x")) + // Re-sign with the imposter's key while keeping the victim's agent uid. + challenge := agents.SignatureChallenge(req.Method, req.URL.Path, + req.Header.Get(headerTimestamp), req.Header.Get(headerNonce)) + req.Header.Set(headerSignature, + base64.StdEncoding.EncodeToString(ed25519.Sign(imposter.priv, challenge))) + + r.Equal(http.StatusUnauthorized, f.serve(t, req).Code) + }) + + t.Run("stale timestamp", func(t *testing.T) { + req := httptest.NewRequestWithContext(f.ctx, http.MethodPost, + uploadPath+"?topic="+topic, bytes.NewReader(pngBytes("x"))) + stale := time.Now().Add(-2 * agents.DefaultClockSkew).UTC().Format(time.RFC3339) + agent.sign(req, stale, "stale-nonce") + + r.Equal(http.StatusUnauthorized, f.serve(t, req).Code) + }) + + t.Run("revoked agent", func(t *testing.T) { + revoked := enrollAgent(t, f.ctx, f.db, f.org.UID, "eu-west") + r.NoError(f.db.RevokeAgent(f.ctx, f.org.UID, revoked.agent.UID)) + + r.Equal(http.StatusUnauthorized, + f.serve(t, revoked.signedRequest(t, f.ctx, topic, pngBytes("x"))).Code) + }) + + t.Run("unknown agent uid", func(t *testing.T) { + req := agent.signedRequest(t, f.ctx, topic, pngBytes("x")) + req.Header.Set(headerAgentUID, "00000000-0000-0000-0000-000000000000") + + r.Equal(http.StatusUnauthorized, f.serve(t, req).Code) + }) +} + +// TestAgentUploadRejectsReplay proves the nonce guard: the SAME bytes, headers +// and signature presented twice succeed once and fail once. +func TestAgentUploadRejectsReplay(t *testing.T) { + t.Parallel() + + r := require.New(t) + f := setupUploadTest(t) + + agent := enrollAgent(t, f.ctx, f.db, f.org.UID, "eu-west") + topic := IncidentScreenshotTopic(f.incident.UID) + + timestamp := time.Now().UTC().Format(time.RFC3339) + const nonce = "replay-me" + + build := func() *http.Request { + req := httptest.NewRequestWithContext(f.ctx, http.MethodPost, + uploadPath+"?topic="+topic, bytes.NewReader(pngBytes("replay"))) + agent.sign(req, timestamp, nonce) + + return req + } + + r.Equal(http.StatusCreated, f.serve(t, build()).Code) + r.Equal(http.StatusUnauthorized, f.serve(t, build()).Code, + "a captured signature must not work a second time") +} + +// TestAgentUploadRejectsForeignAndMalformedTopics is the authorization surface. +// The org is NEVER taken from the request, so these are the cases where a +// caller tries to take it anyway. +func TestAgentUploadRejectsForeignAndMalformedTopics(t *testing.T) { + t.Parallel() + + r := require.New(t) + f := setupUploadTest(t) + + other := models.NewOrganization("acmetwo", "Acme Two") + r.NoError(f.db.CreateOrganization(f.ctx, other)) + + foreignAgent := enrollAgent(t, f.ctx, f.db, other.UID, "eu-west") + wrongRegionAgent := enrollAgent(t, f.ctx, f.db, f.org.UID, "us-east") + goodAgent := enrollAgent(t, f.ctx, f.db, f.org.UID, "eu-west") + + topic := IncidentScreenshotTopic(f.incident.UID) + + r.Equal(http.StatusForbidden, + f.serve(t, foreignAgent.signedRequest(t, f.ctx, topic, pngBytes("x"))).Code, + "another org's agent must not attach to this incident") + + r.Equal(http.StatusForbidden, + f.serve(t, wrongRegionAgent.signedRequest(t, f.ctx, topic, pngBytes("x"))).Code, + "an agent whose region does not serve the check must be refused") + + r.Equal(http.StatusForbidden, + f.serve(t, goodAgent.signedRequest(t, f.ctx, + IncidentScreenshotTopic("11111111-1111-1111-1111-111111111111"), pngBytes("x"))).Code, + "an incident that does not exist must be refused") + + r.Equal(http.StatusForbidden, + f.serve(t, goodAgent.signedRequest(t, f.ctx, + "checks/"+f.check.UID+"/screenshot", pngBytes("x"))).Code, + "an entity with no registered authorizer must fail closed") + + r.Equal(http.StatusBadRequest, + f.serve(t, goodAgent.signedRequest(t, f.ctx, "incidents..screenshot", pngBytes("x"))).Code, + "a malformed topic is a bad request") +} + +// TestAgentUploadRejectsBadBodies covers the payload gates: a mime mismatch and +// an oversize body, both after a successful authorization so the rejection is +// unambiguously about the body. +func TestAgentUploadRejectsBadBodies(t *testing.T) { + t.Parallel() + + r := require.New(t) + f := setupUploadTest(t) + + agent := enrollAgent(t, f.ctx, f.db, f.org.UID, "eu-west") + topic := IncidentScreenshotTopic(f.incident.UID) + + // A declared image/png that is not one: sniffing is what catches this. + req := agent.signedRequest(t, f.ctx, topic, []byte("PK\x03\x04 this is a zip")) + req.Header.Set("Content-Type", "image/png") + r.Equal(http.StatusUnsupportedMediaType, f.serve(t, req).Code) + + oversize := append(pngBytes(""), make([]byte, MaxAttachmentBytes)...) + r.Equal(http.StatusRequestEntityTooLarge, + f.serve(t, agent.signedRequest(t, f.ctx, topic, oversize)).Code) + + r.Equal(http.StatusBadRequest, + f.serve(t, agent.signedRequest(t, f.ctx, topic, nil)).Code, + "an empty body is a bad request") +} + +// TestAgentUploadRateLimited pins the per-agent cap, and that it is PER AGENT: +// one agent burning its window must not lock another one out. +func TestAgentUploadRateLimited(t *testing.T) { + t.Parallel() + + r := require.New(t) + + limiter := newAgentRateLimiter(time.Minute, 3) + now := time.Now() + + r.True(limiter.allow("a", now)) + r.True(limiter.allow("a", now)) + r.True(limiter.allow("a", now)) + r.False(limiter.allow("a", now), "the fourth in the window is refused") + + r.True(limiter.allow("b", now), "the cap is per agent, not global") + + r.True(limiter.allow("a", now.Add(time.Minute)), "the window rolls") +} diff --git a/server/internal/handlers/attachments/service_test.go b/server/internal/handlers/attachments/service_test.go new file mode 100644 index 000000000..4755f627d --- /dev/null +++ b/server/internal/handlers/attachments/service_test.go @@ -0,0 +1,350 @@ +package attachments + +import ( + "context" + "encoding/json" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + + "github.com/fclairamb/solidping/server/internal/config" + "github.com/fclairamb/solidping/server/internal/db" + "github.com/fclairamb/solidping/server/internal/db/models" + "github.com/fclairamb/solidping/server/internal/db/sqlite" + "github.com/fclairamb/solidping/server/internal/handlers/files" + "github.com/fclairamb/solidping/server/internal/handlers/filestorage/localfs" +) + +// pngBytes is a minimal blob whose leading bytes are a real PNG signature, so +// the mime sniff accepts it without dragging an image encoder into the tests. +func pngBytes(marker string) []byte { + return append([]byte{0x89, 'P', 'N', 'G', '\r', '\n', 0x1a, '\n'}, []byte(marker)...) +} + +func setupAttachmentsTest(t *testing.T) (context.Context, db.Service, *Service, *models.Organization) { + t.Helper() + + ctx := t.Context() + + dbService, err := sqlite.New(ctx, sqlite.Config{InMemory: true}) + require.NoError(t, err) + require.NoError(t, dbService.Initialize(ctx)) + t.Cleanup(func() { _ = dbService.Close() }) + + org := models.NewOrganization("acme", "Acme") + require.NoError(t, dbService.CreateOrganization(ctx, org)) + + // The local-FS backend self-registers on demand; the production wiring does + // this once in server.go. + localfs.Register() + + cfg := &config.Config{} + cfg.Auth.JWTSecret = "test-secret" + cfg.FileStorage.Type = "local" + cfg.FileStorage.LocalRoot = t.TempDir() + + return ctx, dbService, NewService(files.NewService(dbService, cfg), dbService, cfg), org +} + +// TestAttachmentRoundTrip pins the whole rail end to end on a real database: +// the topic and the details bag survive the write, the exact-topic list finds +// the row, and the response carries camelCase JSON with a signed URL. +func TestAttachmentRoundTrip(t *testing.T) { + t.Parallel() + + r := require.New(t) + ctx, dbService, svc, org := setupAttachmentsTest(t) + + incidentUID := uuid.New().String() + capturedAt := time.Now().UTC().Truncate(time.Second) + + fileUID, err := svc.PutIncidentScreenshot(ctx, org.UID, incidentUID, pngBytes("first"), + models.JSONMap{ + DetailKeyCapturedAt: capturedAt.Format(time.RFC3339), + DetailKeyRegion: "eu-west", + DetailKeyCheckUID: "check-1", + DetailKeyTrigger: TriggerIncidentOpen, + }) + r.NoError(err) + r.NotEmpty(fileUID) + + // The row really carries the topic and the bag. + stored, err := dbService.GetFile(ctx, org.UID, fileUID) + r.NoError(err) + r.NotNil(stored.Topic) + r.Equal(IncidentScreenshotTopic(incidentUID), *stored.Topic) + r.Equal("eu-west", stored.Details[DetailKeyRegion]) + r.Equal("image/png", stored.MimeType) + + list, err := svc.ListIncidentAttachments(ctx, org.UID, incidentUID) + r.NoError(err) + r.Len(list, 1) + r.Equal(KindScreenshot, list[0].Kind) + r.Equal("eu-west", list[0].Region) + r.Equal("check-1", list[0].CheckUID) + r.Equal(TriggerIncidentOpen, list[0].Trigger) + r.NotNil(list[0].CapturedAt) + r.Equal(capturedAt, list[0].CapturedAt.UTC()) + r.Contains(list[0].DownloadURL, "/pub/files/"+fileUID) + r.Contains(list[0].DownloadURL, "sig=") + + // camelCase on the wire. + raw, err := json.Marshal(list[0]) + r.NoError(err) + r.Contains(string(raw), `"downloadUrl"`) + r.Contains(string(raw), `"mimeType"`) + r.Contains(string(raw), `"capturedAt"`) + r.NotContains(string(raw), `"download_url"`) +} + +// TestPutIncidentScreenshotReplaces proves the overwrite rule: a second write +// under the same incident retires the first one instead of accumulating. +func TestPutIncidentScreenshotReplaces(t *testing.T) { + t.Parallel() + + r := require.New(t) + ctx, _, svc, org := setupAttachmentsTest(t) + + incidentUID := uuid.New().String() + + first, err := svc.PutIncidentScreenshot(ctx, org.UID, incidentUID, pngBytes("first"), nil) + r.NoError(err) + + second, err := svc.PutIncidentScreenshot(ctx, org.UID, incidentUID, pngBytes("second"), nil) + r.NoError(err) + r.NotEqual(first, second) + + list, err := svc.ListIncidentAttachments(ctx, org.UID, incidentUID) + r.NoError(err) + r.Len(list, 1, "the previous capture must be retired, not accumulated") + r.Equal(second, list[0].UID) +} + +// TestDeleteAttachmentsIsPrefixScoped is the negative the prefix semantics +// stand or fall on: reaping one incident must not touch a DIFFERENT incident +// whose uid merely starts with the same characters, nor any non-attachment file. +func TestDeleteAttachmentsIsPrefixScoped(t *testing.T) { + t.Parallel() + + r := require.New(t) + ctx, dbService, svc, org := setupAttachmentsTest(t) + + // Deliberately overlapping: "abc" is a strict prefix of "abcdef". + const ( + shortUID = "abc" + longUID = "abcdef" + ) + + shortFile, err := svc.PutIncidentScreenshot(ctx, org.UID, shortUID, pngBytes("short"), nil) + r.NoError(err) + + longFile, err := svc.PutIncidentScreenshot(ctx, org.UID, longUID, pngBytes("long"), nil) + r.NoError(err) + + // A plain, unattached file: the sort of row that must never be swept. + plain := models.NewFile(org.UID, "logo.png", "image/png", "file://x", 3, nil) + r.NoError(dbService.CreateFile(ctx, plain)) + + count, err := svc.DeleteIncidentAttachments(ctx, org.UID, shortUID) + r.NoError(err) + r.Equal(1, count) + + gone, err := svc.ListIncidentAttachments(ctx, org.UID, shortUID) + r.NoError(err) + r.Empty(gone) + + survivor, err := svc.ListIncidentAttachments(ctx, org.UID, longUID) + r.NoError(err) + r.Len(survivor, 1, "the trailing slash must stop abc/ from matching abcdef/") + r.Equal(longFile, survivor[0].UID) + + _, err = dbService.GetFile(ctx, org.UID, plain.UID) + r.NoError(err, "an unattached file is invisible to the reaper") + + _, err = dbService.GetFile(ctx, org.UID, shortFile) + r.Error(err, "the reaped attachment is soft-deleted") +} + +// TestDeleteAttachmentsIsOrgScoped proves one org's reap cannot reach another's +// attachments even when the topics are identical. +func TestDeleteAttachmentsIsOrgScoped(t *testing.T) { + t.Parallel() + + r := require.New(t) + ctx, dbService, svc, org := setupAttachmentsTest(t) + + other := models.NewOrganization("acmetwo", "Acme Two") + r.NoError(dbService.CreateOrganization(ctx, other)) + + incidentUID := uuid.New().String() + + _, err := svc.PutIncidentScreenshot(ctx, org.UID, incidentUID, pngBytes("mine"), nil) + r.NoError(err) + _, err = svc.PutIncidentScreenshot(ctx, other.UID, incidentUID, pngBytes("theirs"), nil) + r.NoError(err) + + count, err := svc.DeleteIncidentAttachments(ctx, org.UID, incidentUID) + r.NoError(err) + r.Equal(1, count) + + theirs, err := svc.ListIncidentAttachments(ctx, other.UID, incidentUID) + r.NoError(err) + r.Len(theirs, 1) +} + +// TestPutRejectsBadBodies pins the size and mime gates, with a positive control +// so "everything is rejected" cannot pass. +func TestPutRejectsBadBodies(t *testing.T) { + t.Parallel() + + r := require.New(t) + ctx, _, svc, org := setupAttachmentsTest(t) + + topic := IncidentScreenshotTopic(uuid.New().String()) + + _, err := svc.Put(ctx, org.UID, topic, "x.png", nil, nil) + r.ErrorIs(err, ErrEmptyAttachment) + + _, err = svc.Put(ctx, org.UID, topic, "x.png", []byte("GIF89a-not-a-png"), nil) + r.ErrorIs(err, ErrUnsupportedMediaType) + + oversize := append(pngBytes(""), make([]byte, MaxAttachmentBytes)...) + _, err = svc.Put(ctx, org.UID, topic, "x.png", oversize, nil) + r.ErrorIs(err, ErrAttachmentTooLarge) + + // Positive control. + uid, err := svc.Put(ctx, org.UID, topic, "x.png", pngBytes("ok"), nil) + r.NoError(err) + r.NotEmpty(uid) +} + +// TestParseTopicRejectsHostileInput covers the strings that would be dangerous +// if a topic reached storage or a prefix reap unvalidated. +func TestParseTopicRejectsHostileInput(t *testing.T) { + t.Parallel() + + r := require.New(t) + + bad := []string{ + "", + "incidents", + "incidents/uid", + "incidents/uid/kind/extra", + "incidents//screenshot", + "incidents/../screenshot", + "incidents/uid/scre%nshot", + "incidents/uid/scre_nshot", + "incidents/ui d/screenshot", + } + + for _, topic := range bad { + _, err := ParseTopic(topic) + r.ErrorIs(err, ErrInvalidTopic, "topic %q must be rejected", topic) + } + + // Positive control: the real shape parses. + parsed, err := ParseTopic("incidents/9a1eb273-0a95-4d6b-b967-9af076c1f8e8/screenshot") + r.NoError(err) + r.Equal(EntityIncidents, parsed.Entity) + r.Equal(KindScreenshot, parsed.Kind) +} + +// TestAuthorizerRegistryFailsClosed pins that an entity nobody registered is +// refused rather than waved through. +func TestAuthorizerRegistryFailsClosed(t *testing.T) { + t.Parallel() + + r := require.New(t) + + registry := NewAuthorizerRegistry() + registry.Register(EntityIncidents, AuthorizerFunc(func( + context.Context, ParsedTopic, UploaderIdentity, + ) (string, error) { + return "org-1", nil + })) + + _, err := registry.Authorize(t.Context(), + ParsedTopic{Entity: "checks", EntityUID: "c1", Kind: "screenshot"}, UploaderIdentity{}) + r.ErrorIs(err, ErrUnknownTopicEntity) + + // Positive control: the registered entity resolves. + orgUID, err := registry.Authorize(t.Context(), + ParsedTopic{Entity: EntityIncidents, EntityUID: "i1", Kind: KindScreenshot}, UploaderIdentity{}) + r.NoError(err) + r.Equal("org-1", orgUID) +} + +// TestIncidentAuthorizer walks the refusal chain: unknown incident, foreign +// org, and a region that does not serve the incident's check — each rejected, +// with the legitimate case accepted as the positive control. +func TestIncidentAuthorizer(t *testing.T) { + t.Parallel() + + r := require.New(t) + ctx, dbService, _, org := setupAttachmentsTest(t) + + other := models.NewOrganization("acmetwo", "Acme Two") + r.NoError(dbService.CreateOrganization(ctx, other)) + + check := models.NewCheck(org.UID, "api", "browser") + check.Regions = []string{"eu-west"} + r.NoError(dbService.CreateCheck(ctx, check)) + + incident := models.NewIncident(org.UID, check.UID, time.Now(), "api is down") + r.NoError(dbService.CreateIncident(ctx, incident)) + + authorizer := NewIncidentAuthorizer(dbService) + topic := ParsedTopic{Entity: EntityIncidents, EntityUID: incident.UID, Kind: KindScreenshot} + + // Positive control FIRST, so the refusals below are known to be about the + // specific condition and not about an authorizer that refuses everything. + orgUID, err := authorizer.Authorize(ctx, topic, UploaderIdentity{ + AgentUID: "a1", AgentOrgUID: org.UID, AgentRegion: "eu-west", + }) + r.NoError(err) + r.Equal(org.UID, orgUID, "the org comes from the incident, never from the request") + + _, err = authorizer.Authorize(ctx, + ParsedTopic{Entity: EntityIncidents, EntityUID: uuid.New().String(), Kind: KindScreenshot}, + UploaderIdentity{AgentUID: "a1", AgentOrgUID: org.UID, AgentRegion: "eu-west"}) + r.ErrorIs(err, ErrTopicForbidden, "an unknown incident is refused") + + _, err = authorizer.Authorize(ctx, topic, UploaderIdentity{ + AgentUID: "a2", AgentOrgUID: other.UID, AgentRegion: "eu-west", + }) + r.ErrorIs(err, ErrTopicForbidden, "another org's agent is refused") + + _, err = authorizer.Authorize(ctx, topic, UploaderIdentity{ + AgentUID: "a3", AgentOrgUID: org.UID, AgentRegion: "us-east", + }) + r.ErrorIs(err, ErrTopicForbidden, "an agent whose region does not serve the check is refused") + + _, err = authorizer.Authorize(ctx, topic, UploaderIdentity{ + AgentUID: "a4", AgentOrgUID: org.UID, AgentRegion: "", + }) + r.ErrorIs(err, ErrTopicForbidden, "an agent with no region could not have run anything") + + // A SYSTEM agent (no org binding) serving the right region is allowed — + // otherwise cloud-region captures could never be uploaded at all. + orgUID, err = authorizer.Authorize(ctx, topic, UploaderIdentity{ + AgentUID: "sys", AgentOrgUID: "", AgentRegion: "eu-west", + }) + r.NoError(err) + r.Equal(org.UID, orgUID) +} + +// TestRegionServesCheckUnpinnedCheck pins the "no regions means any region" +// rule that makes an unpinned check's capture uploadable. +func TestRegionServesCheckUnpinnedCheck(t *testing.T) { + t.Parallel() + + r := require.New(t) + + r.True(regionServesCheck("eu-west", nil)) + r.True(regionServesCheck("eu-west", []string{})) + r.True(regionServesCheck("eu-west", []string{"us-east", "eu-west"})) + r.False(regionServesCheck("eu-west", []string{"us-east"})) + r.False(regionServesCheck("", nil)) +} diff --git a/server/internal/handlers/incidents/screenshot_persistence_test.go b/server/internal/handlers/incidents/screenshot_persistence_test.go new file mode 100644 index 000000000..547ce0b75 --- /dev/null +++ b/server/internal/handlers/incidents/screenshot_persistence_test.go @@ -0,0 +1,304 @@ +package incidents_test + +import ( + "context" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/fclairamb/solidping/server/internal/checkers/checkerdef" + "github.com/fclairamb/solidping/server/internal/db/models" + "github.com/fclairamb/solidping/server/internal/handlers/attachments" + "github.com/fclairamb/solidping/server/internal/handlers/incidents" +) + +// fakeAttachmentStore records every call the incident pipeline makes, so a test +// can assert not only WHAT was stored but how many times the pipeline decided +// to store anything at all — which is the whole "transitions only" claim. +type fakeAttachmentStore struct { + mu sync.Mutex + + puts []fakePut + deletes []string + // failPut makes the store report an error, to prove the pipeline survives + // a storage outage. + failPut error +} + +type fakePut struct { + incidentUID string + png []byte + details models.JSONMap +} + +func (f *fakeAttachmentStore) PutIncidentScreenshot( + _ context.Context, _, incidentUID string, png []byte, details models.JSONMap, +) (string, error) { + f.mu.Lock() + defer f.mu.Unlock() + + if f.failPut != nil { + return "", f.failPut + } + + f.puts = append(f.puts, fakePut{incidentUID: incidentUID, png: png, details: details}) + + return "file-" + incidentUID, nil +} + +func (f *fakeAttachmentStore) DeleteIncidentAttachments( + _ context.Context, _, incidentUID string, +) (int, error) { + f.mu.Lock() + defer f.mu.Unlock() + + f.deletes = append(f.deletes, incidentUID) + + return 1, nil +} + +func (f *fakeAttachmentStore) ListIncidentAttachments( + _ context.Context, _, _ string, +) ([]attachments.Response, error) { + return nil, nil +} + +func (f *fakeAttachmentStore) snapshot() ([]fakePut, []string) { + f.mu.Lock() + defer f.mu.Unlock() + + return append([]fakePut(nil), f.puts...), append([]string(nil), f.deletes...) +} + +// screenshotPNG is a recognizable blob; nothing decodes it. +func screenshotPNG(marker string) []byte { + return append([]byte{0x89, 'P', 'N', 'G', '\r', '\n', 0x1a, '\n'}, []byte(marker)...) +} + +// shotDownResult is a failing result carrying an in-process browser capture. +func shotDownResult(orgUID, checkUID, marker string) *models.Result { + result := downResult(orgUID, checkUID, "keyword check failed") + result.Diagnostics = &checkerdef.Diagnostics{ + Screenshot: &checkerdef.Screenshot{ + PNG: screenshotPNG(marker), + CapturedAt: time.Now().UTC(), + }, + } + + return result +} + +// TestCreateIncidentPersistsScreenshot is the headline: the capture that opened +// the incident is written under that incident, with the region stamped +// SERVER-SIDE from the result row. +func TestCreateIncidentPersistsScreenshot(t *testing.T) { + t.Parallel() + + r := require.New(t) + ctx := context.Background() + s := newFailureSnapshotSetup(t) + + store := &fakeAttachmentStore{} + s.svc.SetAttachmentStore(store) + + result := shotDownResult(s.org.UID, s.check.UID, "onset") + r.NoError(s.svc.CreateIncidentForTest(ctx, s.check, result)) + + inc, err := s.dbSvc.FindActiveIncidentByCheckUID(ctx, s.check.UID) + r.NoError(err) + + puts, _ := store.snapshot() + r.Len(puts, 1) + r.Equal(inc.UID, puts[0].incidentUID) + r.Equal(screenshotPNG("onset"), puts[0].png) + r.Equal(attachments.TriggerIncidentOpen, puts[0].details[attachments.DetailKeyTrigger]) + r.Equal(s.check.UID, puts[0].details[attachments.DetailKeyCheckUID]) + r.Equal("eu", puts[0].details[attachments.DetailKeyRegion], + "the region comes from the persisted result, never from the checker") + r.NotEmpty(puts[0].details[attachments.DetailKeyCapturedAt]) +} + +// TestScreenshotOnlyPersistedOnTransitions is the negative the storage math +// depends on: a failing run that opens nothing must persist nothing. +// +// Driven through the real ProcessCheckResult with a confirmation period long +// enough that the first failures cannot open an incident, then relaxed so the +// last one does — the positive control inside the same test. +func TestScreenshotOnlyPersistedOnTransitions(t *testing.T) { + t.Parallel() + + r := require.New(t) + ctx := context.Background() + s := newFailureSnapshotSetup(t) + + store := &fakeAttachmentStore{} + s.svc.SetAttachmentStore(store) + + // A long confirmation period: failures accumulate, no incident opens. + s.check.ConfirmationPeriodSeconds = 3600 + + for range 5 { + r.NoError(s.svc.ProcessCheckResult(ctx, s.check, + shotDownResult(s.org.UID, s.check.UID, "not-a-transition"))) + } + + puts, _ := store.snapshot() + r.Empty(puts, "a failing run that opens no incident must drop its capture on the floor") + + // Positive control: with the threshold satisfied the very same result shape + // DOES get persisted, so the assertion above is about the transition rule + // and not about a store that is never called. + s.check.ConfirmationPeriodSeconds = 0 + r.NoError(s.svc.ProcessCheckResult(ctx, s.check, + shotDownResult(s.org.UID, s.check.UID, "transition"))) + + puts, _ = store.snapshot() + r.Len(puts, 1) + r.Equal(screenshotPNG("transition"), puts[0].png) +} + +// TestReopenReplacesScreenshot pins the reopen rule: the relapse's capture +// becomes the incident's evidence, and the previous one is retired first. +func TestReopenReplacesScreenshot(t *testing.T) { + t.Parallel() + + r := require.New(t) + ctx := context.Background() + s := newFailureSnapshotSetup(t) + + store := &fakeAttachmentStore{} + s.svc.SetAttachmentStore(store) + + r.NoError(s.svc.CreateIncidentForTest(ctx, s.check, + shotDownResult(s.org.UID, s.check.UID, "first-onset"))) + + inc, err := s.dbSvc.FindActiveIncidentByCheckUID(ctx, s.check.UID) + r.NoError(err) + + // Resolve it so the next failure reopens rather than opening a new one. + resolved := models.IncidentStateResolved + resolvedAt := time.Now() + r.NoError(s.dbSvc.UpdateIncident(ctx, inc.UID, &models.IncidentUpdate{ + State: &resolved, ResolvedAt: &resolvedAt, + })) + + r.NoError(s.svc.CreateOrReopenIncidentForTest(ctx, s.check, + shotDownResult(s.org.UID, s.check.UID, "relapse"))) + + reopened, err := s.dbSvc.FindActiveIncidentByCheckUID(ctx, s.check.UID) + r.NoError(err) + r.Equal(inc.UID, reopened.UID, "the same incident must have been reopened") + + puts, deletes := store.snapshot() + r.Len(puts, 2) + r.Equal(screenshotPNG("relapse"), puts[1].png) + r.Equal(attachments.TriggerIncidentReopen, puts[1].details[attachments.DetailKeyTrigger]) + + // The pipeline does NOT issue a separate delete when the relapse brought + // its own capture: the write is itself a replace (see + // TestPutIncidentScreenshotReplaces in the attachments package), and an + // extra delete would open a window in which the incident has no evidence + // at all. The delete path is for the no-capture relapse — next test. + r.Empty(deletes) +} + +// TestReopenWithoutCaptureDropsTheStaleOne is the sharper half of the reopen +// rule: a relapse that produced NO capture must not leave the original outage's +// screenshot sitting next to a different failure. +func TestReopenWithoutCaptureDropsTheStaleOne(t *testing.T) { + t.Parallel() + + r := require.New(t) + ctx := context.Background() + s := newFailureSnapshotSetup(t) + + store := &fakeAttachmentStore{} + s.svc.SetAttachmentStore(store) + + r.NoError(s.svc.CreateIncidentForTest(ctx, s.check, + shotDownResult(s.org.UID, s.check.UID, "first-onset"))) + + inc, err := s.dbSvc.FindActiveIncidentByCheckUID(ctx, s.check.UID) + r.NoError(err) + + resolved := models.IncidentStateResolved + resolvedAt := time.Now() + r.NoError(s.dbSvc.UpdateIncident(ctx, inc.UID, &models.IncidentUpdate{ + State: &resolved, ResolvedAt: &resolvedAt, + })) + + // The relapse is a DNS timeout: no page, no capture. + r.NoError(s.svc.CreateOrReopenIncidentForTest(ctx, s.check, + downResult(s.org.UID, s.check.UID, "dial tcp: i/o timeout"))) + + puts, deletes := store.snapshot() + r.Len(puts, 1, "no new capture was written") + r.Contains(deletes, inc.UID, "the stale capture must be deleted, not left behind") +} + +// TestScreenshotStorageFailureNeverBlocksTheIncident pins the best-effort +// contract: an attachment store that is down must not stop an incident from +// opening. A missing screenshot is a papercut; a missing incident is an outage +// nobody is paged for. +func TestScreenshotStorageFailureNeverBlocksTheIncident(t *testing.T) { + t.Parallel() + + r := require.New(t) + ctx := context.Background() + s := newFailureSnapshotSetup(t) + + s.svc.SetAttachmentStore(&fakeAttachmentStore{failPut: context.DeadlineExceeded}) + + r.NoError(s.svc.CreateIncidentForTest(ctx, s.check, + shotDownResult(s.org.UID, s.check.UID, "onset"))) + + inc, err := s.dbSvc.FindActiveIncidentByCheckUID(ctx, s.check.UID) + r.NoError(err) + r.NotNil(inc) +} + +// TestAgentMarkerCarriesNoBytesToPersist covers the agent path's shape: the +// result frame advertises a capture (available + captureId) without the bytes, +// so the pipeline must NOT invent an empty attachment for it. +func TestAgentMarkerCarriesNoBytesToPersist(t *testing.T) { + t.Parallel() + + r := require.New(t) + ctx := context.Background() + s := newFailureSnapshotSetup(t) + + store := &fakeAttachmentStore{} + s.svc.SetAttachmentStore(store) + + result := downResult(s.org.UID, s.check.UID, "keyword check failed") + result.Diagnostics = &checkerdef.Diagnostics{ + Screenshot: &checkerdef.Screenshot{Available: true, CaptureID: "cap-1"}, + } + + r.NoError(s.svc.CreateIncidentForTest(ctx, s.check, result)) + + puts, _ := store.snapshot() + r.Empty(puts, "a marker with no bytes must not become a zero-byte attachment") +} + +// TestNoAttachmentStoreIsSafe pins the nil-safety: a deployment with no +// attachment store wired still opens incidents normally. +func TestNoAttachmentStoreIsSafe(t *testing.T) { + t.Parallel() + + r := require.New(t) + ctx := context.Background() + s := newFailureSnapshotSetup(t) + + r.NoError(s.svc.CreateIncidentForTest(ctx, s.check, + shotDownResult(s.org.UID, s.check.UID, "onset"))) + + inc, err := s.dbSvc.FindActiveIncidentByCheckUID(ctx, s.check.UID) + r.NoError(err) + r.NotNil(inc) +} + +// compile-time check that the fake satisfies the real interface. +var _ incidents.AttachmentStore = (*fakeAttachmentStore)(nil) diff --git a/server/internal/handlers/statuspages/details_never_public_test.go b/server/internal/handlers/statuspages/details_never_public_test.go index f9078649a..f0937a0af 100644 --- a/server/internal/handlers/statuspages/details_never_public_test.go +++ b/server/internal/handlers/statuspages/details_never_public_test.go @@ -10,16 +10,24 @@ import ( "github.com/stretchr/testify/require" "github.com/fclairamb/solidping/server/internal/db/models" + "github.com/fclairamb/solidping/server/internal/handlers/attachments" ) // isForbiddenPublicFieldName reports JSON keys that must never appear anywhere in a // PUBLIC status-page payload. `details` is the incidents.details JSONB column, // which since spec 2026-08-20-01 can carry the probe's failure capture — // response bodies with internal hostnames, stack traces or PII. +// +// The attachment names (spec 2026-08-21-01) are here for the same reason and a +// sharper one: an incident screenshot is a PICTURE of whatever the failing page +// showed — an internal admin console, a stack trace, a customer's data — and +// `downloadurl` is a SIGNED url, so publishing one would hand every reader of a +// public status page a working link to that picture, with no login in the way. func isForbiddenPublicFieldName(name string) bool { switch name { case "details", "failureresponse", "firstresult", "first_result", - "lastfailure", "last_failure", "output", "diagnostics": + "lastfailure", "last_failure", "output", "diagnostics", + "attachments", "attachment", "downloadurl", "screenshot", "signedurl": return true default: return false @@ -34,6 +42,8 @@ func isForbiddenPublicType(typ reflect.Type) bool { switch typ { case reflect.TypeOf(models.JSONMap{}), reflect.TypeOf(models.Incident{}), + reflect.TypeOf(models.File{}), + reflect.TypeOf(attachments.Response{}), reflect.TypeOf(map[string]any{}): return true default: @@ -227,3 +237,66 @@ func TestViewStatusPageNeverLeaksIncidentDetails(t *testing.T) { r.NotContains(string(rawView), "failure_reason") r.NotContains(string(rawView), `"details"`) } + +// TestPublicStatusPagePayloadCarriesNoAttachments is the value-level companion +// for spec 2026-08-21-01: an attachment rendered the way the incident DETAIL +// endpoint renders it — signed download URL and all — has no path into a public +// payload. +// +// The structural walk above already forbids the field and the type; this pins +// the concrete strings, because the failure mode that would actually hurt is a +// signed URL reaching a page anybody can load. +func TestPublicStatusPagePayloadCarriesNoAttachments(t *testing.T) { + t.Parallel() + + r := require.New(t) + + const ( + fileUID = "6f1b0f57-9b6b-4a9c-9a67-1cb2ee2f7f21" + signature = "acme-signed-attachment-signature" + ) + + // What the OPERATOR-facing incident payload carries. + attachment := attachments.Response{ + UID: fileUID, + Kind: attachments.KindScreenshot, + Name: "incident-screenshot.png", + MimeType: "image/png", + Size: 4096, + DownloadURL: "/pub/files/" + fileUID + "?exp=1&sig=" + signature, + Region: "eu-west", + } + + // Positive control: the operator payload really does carry the signed URL, + // so its absence from the public one below means something. + rawOperator, err := json.Marshal(attachment) + r.NoError(err) + r.Contains(string(rawOperator), signature) + r.Contains(string(rawOperator), fileUID) + + response := StatusPageResponse{ + UID: "page-1", + Name: "Acme Status", + Slug: testPublicSlug, + ActiveIncidents: []PublicIncident{{ + UID: "inc-1", + Title: "Investigating elevated errors", + State: "active", + StartedAt: time.Now(), + AffectedResources: []string{"API"}, + }}, + } + + rawPublic, err := json.Marshal(response) + r.NoError(err) + + // Positive control: this is a populated public payload, not an empty one. + r.Contains(string(rawPublic), "activeIncidents") + r.Contains(string(rawPublic), "Investigating elevated errors") + + r.NotContains(string(rawPublic), signature) + r.NotContains(string(rawPublic), fileUID) + r.NotContains(string(rawPublic), "downloadUrl") + r.NotContains(string(rawPublic), "attachments") + r.NotContains(string(rawPublic), "/pub/files/") +} diff --git a/server/internal/jobs/jobtypes/job_attachment_gc_test.go b/server/internal/jobs/jobtypes/job_attachment_gc_test.go new file mode 100644 index 000000000..23bdde2b3 --- /dev/null +++ b/server/internal/jobs/jobtypes/job_attachment_gc_test.go @@ -0,0 +1,126 @@ +package jobtypes + +import ( + "context" + "log/slog" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + + "github.com/fclairamb/solidping/server/internal/db" + "github.com/fclairamb/solidping/server/internal/db/models" + "github.com/fclairamb/solidping/server/internal/db/sqlite" + "github.com/fclairamb/solidping/server/internal/handlers/attachments" + "github.com/fclairamb/solidping/server/internal/jobs/jobdef" +) + +// attachmentGCSetup is a database with one org, ready for attachment rows. +func attachmentGCSetup(t *testing.T) (context.Context, db.Service, *models.Organization) { + t.Helper() + + ctx := t.Context() + + dbSvc, err := sqlite.New(ctx, sqlite.Config{InMemory: true}) + require.NoError(t, err) + require.NoError(t, dbSvc.Initialize(ctx)) + t.Cleanup(func() { _ = dbSvc.Close() }) + + org := models.NewOrganization("acme", "Acme") + require.NoError(t, dbSvc.CreateOrganization(ctx, org)) + + return ctx, dbSvc, org +} + +// writeAttachment inserts an attachment row directly (no blob), aged by `age`. +func writeAttachment(t *testing.T, ctx context.Context, dbSvc db.Service, + orgUID, topic string, age time.Duration, +) *models.File { + t.Helper() + + file := models.NewFile(orgUID, "shot.png", "image/png", "file://blob", 42, nil) + file.Topic = &topic + file.CreatedAt = time.Now().Add(-age) + + require.NoError(t, dbSvc.CreateFile(ctx, file)) + + return file +} + +// TestSweepOrphanIncidentAttachments is the GC's contract in one test: an +// attachment whose incident is gone is reaped, and NOTHING else is. +// +// The four survivors are the whole point — a sweep that deleted everything +// would satisfy the first assertion just as well. +func TestSweepOrphanIncidentAttachments(t *testing.T) { + t.Parallel() + + r := require.New(t) + ctx, dbSvc, org := attachmentGCSetup(t) + + check := models.NewCheck(org.UID, "api", "browser") + r.NoError(dbSvc.CreateCheck(ctx, check)) + + live := models.NewIncident(org.UID, check.UID, time.Now(), "api is down") + r.NoError(dbSvc.CreateIncident(ctx, live)) + + oldEnough := attachmentOrphanGrace + time.Hour + + orphan := writeAttachment(t, ctx, dbSvc, org.UID, + attachments.IncidentScreenshotTopic(uuid.New().String()), oldEnough) + attached := writeAttachment(t, ctx, dbSvc, org.UID, + attachments.IncidentScreenshotTopic(live.UID), oldEnough) + fresh := writeAttachment(t, ctx, dbSvc, org.UID, + attachments.IncidentScreenshotTopic(uuid.New().String()), time.Minute) + malformed := writeAttachment(t, ctx, dbSvc, org.UID, "incidents/only-two-parts", oldEnough) + + // A plain file with no topic at all: the ordinary case the sweep must be + // blind to. + plain := models.NewFile(org.UID, "logo.png", "image/png", "file://logo", 3, nil) + plain.CreatedAt = time.Now().Add(-oldEnough) + r.NoError(dbSvc.CreateFile(ctx, plain)) + + sweepOrphanIncidentAttachments(ctx, &jobdef.JobContext{ + DBService: dbSvc, + Logger: slog.Default(), + }) + + _, err := dbSvc.GetFile(ctx, org.UID, orphan.UID) + r.Error(err, "an attachment whose incident is gone must be reaped") + + for name, file := range map[string]*models.File{ + "attached to a live incident": attached, + "inside the grace window": fresh, + "malformed topic": malformed, + "not an attachment": plain, + } { + _, err := dbSvc.GetFile(ctx, org.UID, file.UID) + r.NoError(err, "%s must survive the sweep", name) + } +} + +// TestListAttachmentsByTopicPrefixIgnoresNonAttachments pins the query the +// sweep is built on: it must see attachment rows only, and only those older +// than the cutoff. +func TestListAttachmentsByTopicPrefixIgnoresNonAttachments(t *testing.T) { + t.Parallel() + + r := require.New(t) + ctx, dbSvc, org := attachmentGCSetup(t) + + old := writeAttachment(t, ctx, dbSvc, org.UID, + attachments.IncidentScreenshotTopic(uuid.New().String()), 48*time.Hour) + writeAttachment(t, ctx, dbSvc, org.UID, + attachments.IncidentScreenshotTopic(uuid.New().String()), time.Minute) + + plain := models.NewFile(org.UID, "logo.png", "image/png", "file://logo", 3, nil) + plain.CreatedAt = time.Now().Add(-48 * time.Hour) + r.NoError(dbSvc.CreateFile(ctx, plain)) + + rows, err := dbSvc.ListAttachmentsByTopicPrefix(ctx, + attachments.EntityIncidents+"/", time.Now().Add(-time.Hour), 100) + r.NoError(err) + r.Len(rows, 1) + r.Equal(old.UID, rows[0].UID) +} diff --git a/server/internal/jobs/jobtypes/job_state_cleanup.go b/server/internal/jobs/jobtypes/job_state_cleanup.go index ab4b16aa8..bfb6714d1 100644 --- a/server/internal/jobs/jobtypes/job_state_cleanup.go +++ b/server/internal/jobs/jobtypes/job_state_cleanup.go @@ -2,9 +2,12 @@ package jobtypes import ( "context" + "database/sql" "encoding/json" + "errors" "time" + "github.com/fclairamb/solidping/server/internal/handlers/attachments" "github.com/fclairamb/solidping/server/internal/jobs/jobdef" "github.com/fclairamb/solidping/server/internal/jobs/jobsvc" ) @@ -57,6 +60,8 @@ func (r *StateCleanupJobRun) Run(ctx context.Context, jctx *jobdef.JobContext) e log.InfoContext(ctx, "No expired state entries to delete") } + sweepOrphanIncidentAttachments(ctx, jctx) + // Schedule next run in 2 hours // Skip if services are not available (e.g., in tests without full service setup) if jctx.Services != nil && jctx.Services.Jobs != nil { @@ -72,3 +77,82 @@ func (r *StateCleanupJobRun) Run(ctx context.Context, jctx *jobdef.JobContext) e return nil } + +// attachmentOrphanGrace is how long an attachment must have existed before the +// sweep will consider it. It is a RACE GUARD, not a retention policy: an +// attachment row is written moments after its incident, and on the agent path +// the upload can even arrive before the caller has finished with the incident, +// so sweeping a fresh row risks deleting evidence for an entity that exists +// perfectly well. +const attachmentOrphanGrace = 6 * time.Hour + +// attachmentSweepBatch bounds one pass. The sweep runs every two hours off the +// back of the state-cleanup job; there is no value in doing the whole backlog +// in one transaction-hostile burst. +const attachmentSweepBatch = 500 + +// sweepOrphanIncidentAttachments soft-deletes attachment rows whose +// `incidents//…` topic points at an incident that no longer exists. +// +// WHY THIS IS NOT OPTIONAL: an attachment is a blob on somebody's storage +// bill. Without a reaper, every deleted incident leaves its screenshot behind +// forever, and the cost of the feature grows monotonically with churn rather +// than with the number of incidents an operator can actually look at. The +// topic IS the link, so a topic pointing at nothing is the exact definition of +// an orphan — no extra bookkeeping table is needed to find one. +// +// Best-effort and never fatal: this is housekeeping riding on a cleanup job, +// and a failure here must not stop expired state entries from being reaped or +// the next run from being scheduled. +func sweepOrphanIncidentAttachments(ctx context.Context, jctx *jobdef.JobContext) { + log := jctx.Logger + + rows, err := jctx.DBService.ListAttachmentsByTopicPrefix( + ctx, attachments.EntityIncidents+"/", + time.Now().Add(-attachmentOrphanGrace), attachmentSweepBatch, + ) + if err != nil { + log.WarnContext(ctx, "Failed to list incident attachments for GC", "error", err) + + return + } + + swept := 0 + + for _, row := range rows { + if row.Topic == nil { + continue + } + + topic, parseErr := attachments.ParseTopic(*row.Topic) + if parseErr != nil { + // A malformed topic can only have come from an older or buggier + // writer. Leave it alone rather than guess what it pointed at. + continue + } + + incident, incErr := jctx.DBService.GetIncidentAny(ctx, topic.EntityUID) + if incErr == nil && incident != nil { + continue + } + + if !errors.Is(incErr, sql.ErrNoRows) { + // A transient lookup failure must never be read as "the incident + // is gone" — that would delete live evidence on a database blip. + continue + } + + if delErr := jctx.DBService.DeleteFile(ctx, row.OrganizationUID, row.UID); delErr != nil { + log.WarnContext(ctx, "Failed to reap orphan attachment", + "fileUid", row.UID, "error", delErr) + + continue + } + + swept++ + } + + if swept > 0 { + log.InfoContext(ctx, "Reaped orphan incident attachments", "count", swept) + } +} From 1454b96ae636f627a6aa3ef7cf07fab7675db115 Mon Sep 17 00:00:00 2001 From: Florent Clairambault Date: Fri, 21 Aug 2026 02:32:36 +0200 Subject: [PATCH 009/475] feat: render incident screenshots in dash0, document the attachments rail --- server/internal/app/openapi/openapi.yaml | 67 + server/internal/app/server.go | 2 +- .../checkers/checkbrowser/screenshot_test.go | 33 +- server/internal/db/postgres/postgres.go | 4 +- server/internal/db/sqlite/sqlite.go | 4 +- .../internal/handlers/attachments/handler.go | 5 +- .../handlers/attachments/handler_test.go | 73 +- .../jobs/jobtypes/job_attachment_gc_test.go | 14 +- server/internal/notifications/slack_test.go | 2 +- server/pkg/client/client_generated.go | 57 + server/test/testdata/testdata.go | 103 +- ...5-agent-screenshot-upload-request-frame.md | 85 + web/dash0/e2e/incident-screenshot.spec.ts | 112 ++ web/dash0/src/api/hooks.ts | 646 ++++--- .../src/components/checks/form/types/web.tsx | 26 +- web/dash0/src/locales/de/incidents.json | 8 + web/dash0/src/locales/en/incidents.json | 8 + web/dash0/src/locales/es/incidents.json | 8 + web/dash0/src/locales/fr/incidents.json | 8 + .../src/routes/orgs/$org/design-reference.tsx | 1486 ++++++++++++----- .../orgs/$org/incidents.$incidentUid.tsx | 63 + wiki/api-specification/agents.md | 31 + wiki/api-specification/files.md | 32 + wiki/api-specification/results-incidents.md | 16 + wiki/conventions/checker-config.md | 1 + wiki/features/browser-monitoring.md | 41 +- 26 files changed, 2235 insertions(+), 700 deletions(-) create mode 100644 specs/todos/2026-08-21-05-agent-screenshot-upload-request-frame.md create mode 100644 web/dash0/e2e/incident-screenshot.spec.ts diff --git a/server/internal/app/openapi/openapi.yaml b/server/internal/app/openapi/openapi.yaml index c4a263c45..c8c20e6bd 100644 --- a/server/internal/app/openapi/openapi.yaml +++ b/server/internal/app/openapi/openapi.yaml @@ -9084,6 +9084,73 @@ components: dropped when the new onset has no capture. **This block is operator-only — it is never serialized onto a status page or a subscriber payload.** + attachments: + type: array + description: >- + Evidence blobs attached to this incident (spec 2026-08-21-01). + Present on the DETAIL endpoint only — the list endpoint never + populates it. Today the only kind is `screenshot`: the PNG a + browser check captured when the incident opened or reopened, for + checks with the browser `screenshot` option enabled. + + + **Operator-only, exactly like `details`.** Each entry carries a + short-lived SIGNED download URL, so it is never serialized onto a + status page or a subscriber payload. + items: + $ref: "#/components/schemas/IncidentAttachment" + + IncidentAttachment: + type: object + description: >- + One stored evidence blob hanging off an incident. The bytes are fetched + through `downloadUrl`, never inlined. + required: [uid, kind, name, mimeType, size, downloadUrl, createdAt] + properties: + uid: + type: string + format: uuid + description: The `files` row uid. + kind: + type: string + description: Attachment kind, e.g. `screenshot`. + name: + type: string + mimeType: + type: string + description: >- + Content type as SNIFFED at write time from the bytes themselves, + not as declared by the uploader. + size: + type: integer + format: int64 + downloadUrl: + type: string + description: >- + RELATIVE, short-lived signed URL + (`/pub/files/?exp=…&sig=…`). Relative so it resolves against + whichever host served the dashboard — SolidPing answers on + several. Re-signed on every fetch of the incident; do not cache it. + createdAt: + type: string + format: date-time + capturedAt: + type: string + format: date-time + description: >- + When the probe took the capture. For a screenshot this is a moment + AFTER failure detection, not the failing frame itself. + region: + type: string + description: >- + Probing region, stamped server-side from the persisted result row. + checkUid: + type: string + format: uuid + trigger: + type: string + description: What caused the capture to be kept. + enum: [incident-open, incident-reopen, agent-upload] IncidentCheck: type: object diff --git a/server/internal/app/server.go b/server/internal/app/server.go index e8cd64062..da8d38797 100644 --- a/server/internal/app/server.go +++ b/server/internal/app/server.go @@ -3193,7 +3193,7 @@ func (s *Server) InitializeTestData(ctx context.Context) error { slog.InfoContext(ctx, "Test mode detected, creating test data") - return testdata.CreateTestData(ctx, s.dbService) + return testdata.CreateTestData(ctx, s.dbService, s.config) } //nolint:ireturn // Returning interface is intentional for testing diff --git a/server/internal/checkers/checkbrowser/screenshot_test.go b/server/internal/checkers/checkbrowser/screenshot_test.go index 7722bdba7..42904041a 100644 --- a/server/internal/checkers/checkbrowser/screenshot_test.go +++ b/server/internal/checkers/checkbrowser/screenshot_test.go @@ -13,10 +13,21 @@ import ( "github.com/fclairamb/solidping/server/internal/checkers/checkerdef" ) +// testPageURL / cfgKeyURL keep the shared literals in one place. +const ( + testPageURL = "https://example.com" + cfgKeyURL = "url" +) + +// errRendererGone stands in for a capture that failed inside chromedp. +var errRendererGone = errors.New("renderer gone") + // fakePNG is a recognizable byte blob standing in for a real capture. Nothing // in the capture path parses the image, so its content only has to be unique // enough to assert on. -var fakePNG = []byte("\x89PNG\r\n\x1a\nfake-screenshot-bytes") +func fakePNG() []byte { + return []byte("\x89PNG\r\n\x1a\nfake-screenshot-bytes") +} // screenshotChecker builds a checker whose session returns `status` and whose // capture seam returns whatever `capture` says. Both seams are needed: one @@ -37,10 +48,10 @@ func screenshotChecker( } func screenshotSpec(enabled bool) *BrowserConfig { - return &BrowserConfig{URL: "https://example.com", Timeout: 5 * time.Second, Screenshot: enabled} + return &BrowserConfig{URL: testPageURL, Timeout: 5 * time.Second, Screenshot: enabled} } -// TestScreenshotCapturedOnlyOnOptedInFailure is the core behaviour: the capture +// TestScreenshotCapturedOnlyOnOptedInFailure is the core behavior: the capture // happens when the check opted in AND failed, and in no other combination. // //nolint:paralleltest // mutates the process-wide settings @@ -70,7 +81,7 @@ func TestScreenshotCapturedOnlyOnOptedInFailure(t *testing.T) { checker := screenshotChecker(tc.status, func(context.Context) ([]byte, error) { calls++ - return fakePNG, nil + return fakePNG(), nil }) result, err := checker.Execute(t.Context(), screenshotSpec(tc.enabled)) @@ -89,7 +100,7 @@ func TestScreenshotCapturedOnlyOnOptedInFailure(t *testing.T) { r.Equal(1, calls) r.NotNil(result.Diagnostics) r.NotNil(result.Diagnostics.Screenshot) - r.Equal(fakePNG, result.Diagnostics.Screenshot.PNG) + r.Equal(fakePNG(), result.Diagnostics.Screenshot.PNG) r.False(result.Diagnostics.Screenshot.CapturedAt.IsZero()) }) } @@ -109,7 +120,7 @@ func TestScreenshotFailureNeverChangesOutcome(t *testing.T) { capture func(ctx context.Context) ([]byte, error) }{ {"capture errors", func(context.Context) ([]byte, error) { - return nil, errors.New("renderer gone") + return nil, errRendererGone }}, {"capture returns nothing", func(context.Context) ([]byte, error) { return nil, nil @@ -191,11 +202,11 @@ func TestScreenshotCaptureSurvivesAnExpiredCheckContext(t *testing.T) { _, sawDeadline = ctx.Deadline() - return fakePNG, nil + return fakePNG(), nil }, } - spec := &BrowserConfig{URL: "https://example.com", Timeout: 20 * time.Millisecond, Screenshot: true} + spec := &BrowserConfig{URL: testPageURL, Timeout: 20 * time.Millisecond, Screenshot: true} result, err := checker.Execute(t.Context(), spec) r.NoError(err) @@ -264,16 +275,16 @@ func TestScreenshotConfigRoundTrip(t *testing.T) { r := require.New(t) cfg := &BrowserConfig{} - r.NoError(cfg.FromMap(map[string]any{"url": "https://example.com", "screenshot": true})) + r.NoError(cfg.FromMap(map[string]any{cfgKeyURL: testPageURL, "screenshot": true})) r.True(cfg.Screenshot) r.Equal(true, cfg.GetConfig()["screenshot"]) // Default is off, and an off flag is omitted from the config map entirely. off := &BrowserConfig{} - r.NoError(off.FromMap(map[string]any{"url": "https://example.com"})) + r.NoError(off.FromMap(map[string]any{cfgKeyURL: testPageURL})) r.False(off.Screenshot) r.NotContains(off.GetConfig(), "screenshot") bad := &BrowserConfig{} - r.Error(bad.FromMap(map[string]any{"url": "https://example.com", "screenshot": "yes"})) + r.Error(bad.FromMap(map[string]any{cfgKeyURL: testPageURL, "screenshot": "yes"})) } diff --git a/server/internal/db/postgres/postgres.go b/server/internal/db/postgres/postgres.go index d7b06d293..153a356b8 100644 --- a/server/internal/db/postgres/postgres.go +++ b/server/internal/db/postgres/postgres.go @@ -6265,9 +6265,9 @@ func (s *Service) RemoveRecipientFromReportSchedules(ctx context.Context, orgUID } // applyFileTopicFilter narrows a files query to attachments. An exact Topic and -// a TopicPrefix are both honoured when both are set; neither set leaves the +// a TopicPrefix are both honored when both are set; neither set leaves the // query untouched, which is what keeps the historical "list every file" -// behaviour intact for the dashboard's file browser. +// behavior intact for the dashboard's file browser. // // LIKE with an escaped prefix, not a raw concatenation: a topic is // server-generated today, but the agent upload endpoint accepts one from the diff --git a/server/internal/db/sqlite/sqlite.go b/server/internal/db/sqlite/sqlite.go index 227f4216a..4d1289f07 100644 --- a/server/internal/db/sqlite/sqlite.go +++ b/server/internal/db/sqlite/sqlite.go @@ -6223,9 +6223,9 @@ func (s *Service) RemoveRecipientFromReportSchedules(ctx context.Context, orgUID } // applyFileTopicFilter narrows a files query to attachments. An exact Topic and -// a TopicPrefix are both honoured when both are set; neither set leaves the +// a TopicPrefix are both honored when both are set; neither set leaves the // query untouched, which is what keeps the historical "list every file" -// behaviour intact for the dashboard's file browser. +// behavior intact for the dashboard's file browser. // // LIKE with an escaped prefix, not a raw concatenation: a topic is // server-generated today, but the agent upload endpoint accepts one from the diff --git a/server/internal/handlers/attachments/handler.go b/server/internal/handlers/attachments/handler.go index f65e05ae0..815aac986 100644 --- a/server/internal/handlers/attachments/handler.go +++ b/server/internal/handlers/attachments/handler.go @@ -103,7 +103,8 @@ type UploadResponse struct { func (h *Handler) Upload(writer http.ResponseWriter, req *http.Request) error { agent, authErr := h.authenticate(req) if authErr != nil { - return h.WriteError(writer, http.StatusUnauthorized, base.ErrorCodeInvalidToken, authErr.Error()) + return h.WriteError(writer, http.StatusUnauthorized, base.ErrorCodeInvalidToken, + "Invalid agent signature") } if !h.limiter.allow(agent.UID, time.Now()) { @@ -186,7 +187,7 @@ func attachmentName(topic ParsedTopic) string { // errUnauthenticated is the single message every authentication failure // produces. Distinguishing "unknown agent" from "bad signature" would leak // which agent uids exist. -var errUnauthenticated = errors.New("Invalid agent signature") +var errUnauthenticated = errors.New("invalid agent signature") // authenticate verifies the signed headers exactly the way the WS reconnect // does: known active agent, timestamp inside the skew, nonce not seen before diff --git a/server/internal/handlers/attachments/handler_test.go b/server/internal/handlers/attachments/handler_test.go index 217fb4129..509c9137a 100644 --- a/server/internal/handlers/attachments/handler_test.go +++ b/server/internal/handlers/attachments/handler_test.go @@ -37,7 +37,7 @@ type signedAgent struct { // enrollAgent creates an active agent row bound to `region`, owned by `orgUID` // (empty for a system agent), and returns it with its signing key. -func enrollAgent(t *testing.T, ctx context.Context, dbSvc db.Service, orgUID, region string) *signedAgent { +func enrollAgent(ctx context.Context, t *testing.T, dbSvc db.Service, orgUID, region string) *signedAgent { t.Helper() pub, priv, err := ed25519.GenerateKey(rand.Reader) @@ -68,7 +68,9 @@ func enrollAgent(t *testing.T, ctx context.Context, dbSvc db.Service, orgUID, re } // signedRequest builds a POST with valid signed headers for this agent. -func (s *signedAgent) signedRequest(t *testing.T, ctx context.Context, topic string, body []byte) *http.Request { +func (s *signedAgent) signedRequest( + ctx context.Context, t *testing.T, topic string, body []byte, +) *http.Request { t.Helper() req := httptest.NewRequestWithContext(ctx, http.MethodPost, uploadPath+"?topic="+topic, bytes.NewReader(body)) @@ -95,8 +97,11 @@ func (s *signedAgent) sign(req *http.Request, timestamp, nonce string) { } // uploadFixture is one wired-up endpoint plus the data an upload needs. +// +// ctx is a FUNCTION rather than a field: t.Context() is cheap, and storing a +// context on a struct is the pattern the containedctx linter (rightly) flags. type uploadFixture struct { - ctx context.Context + ctx func() context.Context db db.Service svc *Service handler *Handler @@ -127,7 +132,8 @@ func setupUploadTest(t *testing.T) *uploadFixture { handler.SetNonceGuard(agents.NewNonceCache()) return &uploadFixture{ - ctx: ctx, db: dbService, svc: svc, handler: handler, + ctx: func() context.Context { return ctx }, + db: dbService, svc: svc, handler: handler, org: org, incident: incident, check: check, } } @@ -151,17 +157,17 @@ func TestAgentUploadHappyPath(t *testing.T) { r := require.New(t) f := setupUploadTest(t) - agent := enrollAgent(t, f.ctx, f.db, f.org.UID, "eu-west") + agent := enrollAgent(f.ctx(), t, f.db, f.org.UID, "eu-west") topic := IncidentScreenshotTopic(f.incident.UID) - rec := f.serve(t, agent.signedRequest(t, f.ctx, topic, pngBytes("agent-capture"))) + rec := f.serve(t, agent.signedRequest(f.ctx(), t, topic, pngBytes("agent-capture"))) r.Equal(http.StatusCreated, rec.Code, rec.Body.String()) var resp UploadResponse r.NoError(json.Unmarshal(rec.Body.Bytes(), &resp)) r.NotEmpty(resp.FileUID) - stored, err := f.db.GetFile(f.ctx, f.org.UID, resp.FileUID) + stored, err := f.db.GetFile(f.ctx(), f.org.UID, resp.FileUID) r.NoError(err) r.NotNil(stored.Topic) r.Equal(topic, *stored.Topic) @@ -171,25 +177,30 @@ func TestAgentUploadHappyPath(t *testing.T) { // TestAgentUploadRejectsBadSignature covers the authentication surface: missing // headers, a signature from the wrong key, a stale timestamp, a revoked agent, // and an unknown agent uid. Each must be a 401 and must store nothing. +// +// one per-agent rate limiter; running them in parallel would make an assertion +// about a 401 depend on which subtest happened to consume the nonce first. +// +//nolint:paralleltest,tparallel // the subtests share one in-memory database and func TestAgentUploadRejectsBadSignature(t *testing.T) { t.Parallel() r := require.New(t) f := setupUploadTest(t) - agent := enrollAgent(t, f.ctx, f.db, f.org.UID, "eu-west") + agent := enrollAgent(f.ctx(), t, f.db, f.org.UID, "eu-west") topic := IncidentScreenshotTopic(f.incident.UID) t.Run("no headers", func(t *testing.T) { - req := httptest.NewRequestWithContext(f.ctx, http.MethodPost, + req := httptest.NewRequestWithContext(f.ctx(), http.MethodPost, uploadPath+"?topic="+topic, bytes.NewReader(pngBytes("x"))) r.Equal(http.StatusUnauthorized, f.serve(t, req).Code) }) t.Run("signature from another key", func(t *testing.T) { - imposter := enrollAgent(t, f.ctx, f.db, f.org.UID, "eu-west") + imposter := enrollAgent(f.ctx(), t, f.db, f.org.UID, "eu-west") - req := agent.signedRequest(t, f.ctx, topic, pngBytes("x")) + req := agent.signedRequest(f.ctx(), t, topic, pngBytes("x")) // Re-sign with the imposter's key while keeping the victim's agent uid. challenge := agents.SignatureChallenge(req.Method, req.URL.Path, req.Header.Get(headerTimestamp), req.Header.Get(headerNonce)) @@ -200,7 +211,7 @@ func TestAgentUploadRejectsBadSignature(t *testing.T) { }) t.Run("stale timestamp", func(t *testing.T) { - req := httptest.NewRequestWithContext(f.ctx, http.MethodPost, + req := httptest.NewRequestWithContext(f.ctx(), http.MethodPost, uploadPath+"?topic="+topic, bytes.NewReader(pngBytes("x"))) stale := time.Now().Add(-2 * agents.DefaultClockSkew).UTC().Format(time.RFC3339) agent.sign(req, stale, "stale-nonce") @@ -209,15 +220,15 @@ func TestAgentUploadRejectsBadSignature(t *testing.T) { }) t.Run("revoked agent", func(t *testing.T) { - revoked := enrollAgent(t, f.ctx, f.db, f.org.UID, "eu-west") - r.NoError(f.db.RevokeAgent(f.ctx, f.org.UID, revoked.agent.UID)) + revoked := enrollAgent(f.ctx(), t, f.db, f.org.UID, "eu-west") + r.NoError(f.db.RevokeAgent(f.ctx(), f.org.UID, revoked.agent.UID)) r.Equal(http.StatusUnauthorized, - f.serve(t, revoked.signedRequest(t, f.ctx, topic, pngBytes("x"))).Code) + f.serve(t, revoked.signedRequest(f.ctx(), t, topic, pngBytes("x"))).Code) }) t.Run("unknown agent uid", func(t *testing.T) { - req := agent.signedRequest(t, f.ctx, topic, pngBytes("x")) + req := agent.signedRequest(f.ctx(), t, topic, pngBytes("x")) req.Header.Set(headerAgentUID, "00000000-0000-0000-0000-000000000000") r.Equal(http.StatusUnauthorized, f.serve(t, req).Code) @@ -232,14 +243,14 @@ func TestAgentUploadRejectsReplay(t *testing.T) { r := require.New(t) f := setupUploadTest(t) - agent := enrollAgent(t, f.ctx, f.db, f.org.UID, "eu-west") + agent := enrollAgent(f.ctx(), t, f.db, f.org.UID, "eu-west") topic := IncidentScreenshotTopic(f.incident.UID) timestamp := time.Now().UTC().Format(time.RFC3339) const nonce = "replay-me" build := func() *http.Request { - req := httptest.NewRequestWithContext(f.ctx, http.MethodPost, + req := httptest.NewRequestWithContext(f.ctx(), http.MethodPost, uploadPath+"?topic="+topic, bytes.NewReader(pngBytes("replay"))) agent.sign(req, timestamp, nonce) @@ -261,34 +272,34 @@ func TestAgentUploadRejectsForeignAndMalformedTopics(t *testing.T) { f := setupUploadTest(t) other := models.NewOrganization("acmetwo", "Acme Two") - r.NoError(f.db.CreateOrganization(f.ctx, other)) + r.NoError(f.db.CreateOrganization(f.ctx(), other)) - foreignAgent := enrollAgent(t, f.ctx, f.db, other.UID, "eu-west") - wrongRegionAgent := enrollAgent(t, f.ctx, f.db, f.org.UID, "us-east") - goodAgent := enrollAgent(t, f.ctx, f.db, f.org.UID, "eu-west") + foreignAgent := enrollAgent(f.ctx(), t, f.db, other.UID, "eu-west") + wrongRegionAgent := enrollAgent(f.ctx(), t, f.db, f.org.UID, "us-east") + goodAgent := enrollAgent(f.ctx(), t, f.db, f.org.UID, "eu-west") topic := IncidentScreenshotTopic(f.incident.UID) r.Equal(http.StatusForbidden, - f.serve(t, foreignAgent.signedRequest(t, f.ctx, topic, pngBytes("x"))).Code, + f.serve(t, foreignAgent.signedRequest(f.ctx(), t, topic, pngBytes("x"))).Code, "another org's agent must not attach to this incident") r.Equal(http.StatusForbidden, - f.serve(t, wrongRegionAgent.signedRequest(t, f.ctx, topic, pngBytes("x"))).Code, + f.serve(t, wrongRegionAgent.signedRequest(f.ctx(), t, topic, pngBytes("x"))).Code, "an agent whose region does not serve the check must be refused") r.Equal(http.StatusForbidden, - f.serve(t, goodAgent.signedRequest(t, f.ctx, + f.serve(t, goodAgent.signedRequest(f.ctx(), t, IncidentScreenshotTopic("11111111-1111-1111-1111-111111111111"), pngBytes("x"))).Code, "an incident that does not exist must be refused") r.Equal(http.StatusForbidden, - f.serve(t, goodAgent.signedRequest(t, f.ctx, + f.serve(t, goodAgent.signedRequest(f.ctx(), t, "checks/"+f.check.UID+"/screenshot", pngBytes("x"))).Code, "an entity with no registered authorizer must fail closed") r.Equal(http.StatusBadRequest, - f.serve(t, goodAgent.signedRequest(t, f.ctx, "incidents..screenshot", pngBytes("x"))).Code, + f.serve(t, goodAgent.signedRequest(f.ctx(), t, "incidents..screenshot", pngBytes("x"))).Code, "a malformed topic is a bad request") } @@ -301,20 +312,20 @@ func TestAgentUploadRejectsBadBodies(t *testing.T) { r := require.New(t) f := setupUploadTest(t) - agent := enrollAgent(t, f.ctx, f.db, f.org.UID, "eu-west") + agent := enrollAgent(f.ctx(), t, f.db, f.org.UID, "eu-west") topic := IncidentScreenshotTopic(f.incident.UID) // A declared image/png that is not one: sniffing is what catches this. - req := agent.signedRequest(t, f.ctx, topic, []byte("PK\x03\x04 this is a zip")) + req := agent.signedRequest(f.ctx(), t, topic, []byte("PK\x03\x04 this is a zip")) req.Header.Set("Content-Type", "image/png") r.Equal(http.StatusUnsupportedMediaType, f.serve(t, req).Code) oversize := append(pngBytes(""), make([]byte, MaxAttachmentBytes)...) r.Equal(http.StatusRequestEntityTooLarge, - f.serve(t, agent.signedRequest(t, f.ctx, topic, oversize)).Code) + f.serve(t, agent.signedRequest(f.ctx(), t, topic, oversize)).Code) r.Equal(http.StatusBadRequest, - f.serve(t, agent.signedRequest(t, f.ctx, topic, nil)).Code, + f.serve(t, agent.signedRequest(f.ctx(), t, topic, nil)).Code, "an empty body is a bad request") } diff --git a/server/internal/jobs/jobtypes/job_attachment_gc_test.go b/server/internal/jobs/jobtypes/job_attachment_gc_test.go index 23bdde2b3..efa49ba7b 100644 --- a/server/internal/jobs/jobtypes/job_attachment_gc_test.go +++ b/server/internal/jobs/jobtypes/job_attachment_gc_test.go @@ -34,7 +34,7 @@ func attachmentGCSetup(t *testing.T) (context.Context, db.Service, *models.Organ } // writeAttachment inserts an attachment row directly (no blob), aged by `age`. -func writeAttachment(t *testing.T, ctx context.Context, dbSvc db.Service, +func writeAttachment(ctx context.Context, t *testing.T, dbSvc db.Service, orgUID, topic string, age time.Duration, ) *models.File { t.Helper() @@ -67,13 +67,13 @@ func TestSweepOrphanIncidentAttachments(t *testing.T) { oldEnough := attachmentOrphanGrace + time.Hour - orphan := writeAttachment(t, ctx, dbSvc, org.UID, + orphan := writeAttachment(ctx, t, dbSvc, org.UID, attachments.IncidentScreenshotTopic(uuid.New().String()), oldEnough) - attached := writeAttachment(t, ctx, dbSvc, org.UID, + attached := writeAttachment(ctx, t, dbSvc, org.UID, attachments.IncidentScreenshotTopic(live.UID), oldEnough) - fresh := writeAttachment(t, ctx, dbSvc, org.UID, + fresh := writeAttachment(ctx, t, dbSvc, org.UID, attachments.IncidentScreenshotTopic(uuid.New().String()), time.Minute) - malformed := writeAttachment(t, ctx, dbSvc, org.UID, "incidents/only-two-parts", oldEnough) + malformed := writeAttachment(ctx, t, dbSvc, org.UID, "incidents/only-two-parts", oldEnough) // A plain file with no topic at all: the ordinary case the sweep must be // blind to. @@ -109,9 +109,9 @@ func TestListAttachmentsByTopicPrefixIgnoresNonAttachments(t *testing.T) { r := require.New(t) ctx, dbSvc, org := attachmentGCSetup(t) - old := writeAttachment(t, ctx, dbSvc, org.UID, + old := writeAttachment(ctx, t, dbSvc, org.UID, attachments.IncidentScreenshotTopic(uuid.New().String()), 48*time.Hour) - writeAttachment(t, ctx, dbSvc, org.UID, + writeAttachment(ctx, t, dbSvc, org.UID, attachments.IncidentScreenshotTopic(uuid.New().String()), time.Minute) plain := models.NewFile(org.UID, "logo.png", "image/png", "file://logo", 3, nil) diff --git a/server/internal/notifications/slack_test.go b/server/internal/notifications/slack_test.go index e12e0c428..2c7787d16 100644 --- a/server/internal/notifications/slack_test.go +++ b/server/internal/notifications/slack_test.go @@ -1426,7 +1426,7 @@ func (m *mockDBService) ListAttachmentsByTopicPrefix( } func (m *mockDBService) GetIncidentAny(_ context.Context, _ string) (*models.Incident, error) { - return nil, nil + return &models.Incident{}, nil } func (m *mockDBService) DeleteFile(_ context.Context, _, _ string) error { diff --git a/server/pkg/client/client_generated.go b/server/pkg/client/client_generated.go index 4fd4fd004..c8671fd08 100644 --- a/server/pkg/client/client_generated.go +++ b/server/pkg/client/client_generated.go @@ -843,6 +843,27 @@ func (e HealthResponseStatus) Valid() bool { } } +// Defines values for IncidentAttachmentTrigger. +const ( + IncidentAttachmentTriggerAgentUpload IncidentAttachmentTrigger = "agent-upload" + IncidentAttachmentTriggerIncidentOpen IncidentAttachmentTrigger = "incident-open" + IncidentAttachmentTriggerIncidentReopen IncidentAttachmentTrigger = "incident-reopen" +) + +// Valid indicates whether the value is a known member of the IncidentAttachmentTrigger enum. +func (e IncidentAttachmentTrigger) Valid() bool { + switch e { + case IncidentAttachmentTriggerAgentUpload: + return true + case IncidentAttachmentTriggerIncidentOpen: + return true + case IncidentAttachmentTriggerIncidentReopen: + return true + default: + return false + } +} + // Defines values for IncidentDetailState. const ( IncidentDetailStateActive IncidentDetailState = "active" @@ -3354,6 +3375,37 @@ type IncidentAckRequest struct { Note *string `json:"note,omitempty"` } +// IncidentAttachment One stored evidence blob hanging off an incident. The bytes are fetched through `downloadUrl`, never inlined. +type IncidentAttachment struct { + // CapturedAt When the probe took the capture. For a screenshot this is a moment AFTER failure detection, not the failing frame itself. + CapturedAt *time.Time `json:"capturedAt,omitempty"` + CheckUid *openapi_types.UUID `json:"checkUid,omitempty"` + CreatedAt time.Time `json:"createdAt"` + + // DownloadUrl RELATIVE, short-lived signed URL (`/pub/files/?exp=…&sig=…`). Relative so it resolves against whichever host served the dashboard — SolidPing answers on several. Re-signed on every fetch of the incident; do not cache it. + DownloadUrl string `json:"downloadUrl"` + + // Kind Attachment kind, e.g. `screenshot`. + Kind string `json:"kind"` + + // MimeType Content type as SNIFFED at write time from the bytes themselves, not as declared by the uploader. + MimeType string `json:"mimeType"` + Name string `json:"name"` + + // Region Probing region, stamped server-side from the persisted result row. + Region *string `json:"region,omitempty"` + Size int64 `json:"size"` + + // Trigger What caused the capture to be kept. + Trigger *IncidentAttachmentTrigger `json:"trigger,omitempty"` + + // Uid The `files` row uid. + Uid openapi_types.UUID `json:"uid"` +} + +// IncidentAttachmentTrigger What caused the capture to be kept. +type IncidentAttachmentTrigger string + // IncidentCheck Embedded check details for incidents type IncidentCheck struct { Config *map[string]interface{} `json:"config,omitempty"` @@ -3371,6 +3423,11 @@ type IncidentCommentRequest struct { type IncidentDetail struct { AcknowledgedAt *time.Time `json:"acknowledgedAt,omitempty"` + // Attachments Evidence blobs attached to this incident (spec 2026-08-21-01). Present on the DETAIL endpoint only — the list endpoint never populates it. Today the only kind is `screenshot`: the PNG a browser check captured when the incident opened or reopened, for checks with the browser `screenshot` option enabled. + // + // **Operator-only, exactly like `details`.** Each entry carries a short-lived SIGNED download URL, so it is never serialized onto a status page or a subscriber payload. + Attachments *[]IncidentAttachment `json:"attachments,omitempty"` + // Check Embedded check details for incidents Check *IncidentCheck `json:"check,omitempty"` CheckName *string `json:"checkName,omitempty"` diff --git a/server/test/testdata/testdata.go b/server/test/testdata/testdata.go index 891bca172..2fa82c65d 100644 --- a/server/test/testdata/testdata.go +++ b/server/test/testdata/testdata.go @@ -3,19 +3,25 @@ package testdata import ( "context" + "encoding/base64" "encoding/json" "fmt" "log/slog" "time" + "github.com/fclairamb/solidping/server/internal/config" "github.com/fclairamb/solidping/server/internal/db" "github.com/fclairamb/solidping/server/internal/db/models" + "github.com/fclairamb/solidping/server/internal/handlers/attachments" + "github.com/fclairamb/solidping/server/internal/handlers/files" "github.com/fclairamb/solidping/server/internal/jobs/jobdef" "github.com/fclairamb/solidping/server/internal/utils/passwords" ) -// CreateTestData creates deterministic test data for test mode. -func CreateTestData(ctx context.Context, dbService db.Service) error { +// CreateTestData creates deterministic test data for test mode. cfg is needed +// by the seeders that write real blobs through the files service (the incident +// screenshot fixture); it may be nil, in which case those seeders are skipped. +func CreateTestData(ctx context.Context, dbService db.Service, cfg *config.Config) error { // Check if test organization already exists count, err := dbService.DB().NewSelect(). Model((*models.Organization)(nil)). @@ -98,6 +104,13 @@ func CreateTestData(ctx context.Context, dbService db.Service) error { return err } + // Seed a browser check whose incident carries a real screenshot attachment + // (spec 2026-08-21-01), so the incident screenshot card has deterministic + // data — bytes included, so the actually loads. + if err := createTestIncidentScreenshot(ctx, dbService, cfg, testOrg.UID, now); err != nil { + return err + } + // Seed an objective over a check with a month rollup, so the SLO list and // detail pages have deterministic non-null attainment (spec 2026-08-20-01). if err := createTestSLOData(ctx, dbService, testOrg.UID, now); err != nil { @@ -642,3 +655,89 @@ func createTestSLOData(ctx context.Context, dbService db.Service, orgUID string, return nil } + +// Fixture UIDs for the incident screenshot attachment (spec 2026-08-21-01). +const ( + shotCheckUID = "00000000-0000-0000-0000-000000000025" + shotIncidentUID = "00000000-0000-0000-0000-000000000026" +) + +// shotFixturePNG is a tiny but REAL PNG (a 2x2 image), base64-encoded. Real +// bytes rather than a stub: the E2E asserts the actually renders, which a +// blob that is not a decodable image would fail — and that failure would be +// indistinguishable from the card being broken. +const shotFixturePNG = "iVBORw0KGgoAAAANSUhEUgAAAAIAAAACCAYAAABytg0kAAAAF0lEQVR4nGP8z8Dwn4" + + "GBgYmBgYGBgQEAGgcCAV3wQOsAAAAASUVORK5CYII=" + +// createTestIncidentScreenshot seeds a down browser check with an active +// incident and a real screenshot attachment written through the normal +// attachment path — topic, details bag, storage blob and all. +// +// Written through the REAL path (unlike the failure-response fixture, which is +// hand-rolled JSON) precisely because the bytes have to exist in the storage +// backend for the signed download URL to serve anything. +func createTestIncidentScreenshot( + ctx context.Context, dbService db.Service, cfg *config.Config, orgUID string, now time.Time, +) error { + if cfg == nil { + slog.InfoContext(ctx, "Skipping incident screenshot fixture: no configuration") + + return nil + } + + check := models.NewCheck(orgUID, "shot-check", "browser") + check.UID = shotCheckUID + name := "Screenshot Check" + check.Name = &name + check.Config = models.JSONMap{"url": "https://acme.com/app", "screenshot": true} + check.Status = models.CheckStatusDown + check.StatusChangedAt = &now + check.CreatedAt = now + check.UpdatedAt = now + + if err := dbService.CreateCheck(ctx, check); err != nil { + return fmt.Errorf("failed to create screenshot test check: %w", err) + } + + incident := models.NewIncident(orgUID, shotCheckUID, now, "Screenshot Check is down") + incident.UID = shotIncidentUID + incident.CreatedAt = now + incident.UpdatedAt = now + incident.Details = models.JSONMap{ + "failure_reason": "keyword check failed", + "first_result": models.JSONMap{ + "resultUid": shotIncidentUID, + "status": "DOWN", + "region": "eu-west", + "duration": float32(2.5), + "periodStart": now, + "output": models.JSONMap{"error": "keyword check failed"}, + }, + } + + if err := dbService.CreateIncident(ctx, incident); err != nil { + return fmt.Errorf("failed to create screenshot test incident: %w", err) + } + + png, err := base64.StdEncoding.DecodeString(shotFixturePNG) + if err != nil { + return fmt.Errorf("failed to decode screenshot fixture: %w", err) + } + + svc := attachments.NewService(files.NewService(dbService, cfg), dbService, cfg) + + fileUID, err := svc.PutIncidentScreenshot(ctx, orgUID, shotIncidentUID, png, models.JSONMap{ + attachments.DetailKeyCapturedAt: now.UTC().Format(time.RFC3339), + attachments.DetailKeyRegion: "eu-west", + attachments.DetailKeyCheckUID: shotCheckUID, + attachments.DetailKeyTrigger: attachments.TriggerIncidentOpen, + }) + if err != nil { + return fmt.Errorf("failed to write screenshot test attachment: %w", err) + } + + slog.InfoContext(ctx, "Created test incident screenshot", + "incidentUID", shotIncidentUID, "fileUID", fileUID) + + return nil +} diff --git a/specs/todos/2026-08-21-05-agent-screenshot-upload-request-frame.md b/specs/todos/2026-08-21-05-agent-screenshot-upload-request-frame.md new file mode 100644 index 000000000..7f2356566 --- /dev/null +++ b/specs/todos/2026-08-21-05-agent-screenshot-upload-request-frame.md @@ -0,0 +1,85 @@ +--- +model: opus +effort: high +--- + +# Agent-side screenshot capture: the WS upload-request frame and the agent's capture LRU + +## Problem + +Spec [2026-08-21-01](2026-08-21-01-generic-attachments-and-incident-screenshots.md) +built the generic attachments rail and landed everything the SERVER side needs: + +- `files.topic` / `files.details` plus the prefix-reap and GC primitives. +- `POST /api/v1/agent/attachments` — Ed25519 signed-header auth (same scheme as + the WS upgrade), replay-guarded, per-agent rate-limited, magic-byte sniffed, + with the `incidents//…` topic authorizer that derives the org FROM the + incident and refuses an agent whose region does not serve the check + (`server/internal/handlers/attachments/`). +- The in-process capture path end to end: `checkbrowser` captures on a failing + opted-in run, the incident pipeline persists it on open/reopen, the dashboard + renders it. + +It explicitly took its own §6 escape hatch for the last piece: **a deported +agent still has no way to get its capture to that endpoint.** + +The endpoint contract is settled; what is missing is the *conversation* that +decides when to use it. `checkerdef.Screenshot` already carries the marker +fields the design calls for (`available`, `captureId`, `json:"-"` on `PNG`, so +the bytes can never be smuggled through the JSON control channel), and the +incident pipeline already declines to persist a marker that carries no bytes +(`persistScreenshot` in `server/internal/handlers/incidents/service.go`). So a +browser check running on a private agent captures nothing today: the agent has +no LRU, never sets the marker, and the server never asks for an upload. + +## Proposal + +Implement the flow spec 2026-08-21-01 §6 describes, in the order it describes: + +1. **Agent-side capture + LRU.** The agent runs the same `checkbrowser` code, so + the capture already happens; what is missing is somewhere to put the bytes. + Add a small bounded LRU (a few entries, a few MiB total, TTL'd) keyed by a + generated capture id. The bytes NEVER enter the result frame. +2. **Marker on the result frame.** The agent sets + `Diagnostics.Screenshot = {available: true, captureId: "…"}` — the fields + already exist and already serialize; nothing on the wire changes shape. +3. **Server → agent upload-request frame.** A new server-to-client frame type in + `server/internal/agents/protocol.go`, emitted by the incident pipeline when + (and only when) a result carrying the marker opens or reopens an incident. + It names the capture id and the topic the agent must POST to. + - The pipeline currently has no way to reach the agent's socket. It will need + a hook interface (mirroring `AttachmentStore` / `PublicationHook`: + small, nil-safe, best-effort) that `agentws` implements. + - The agent may have reconnected, or be a different replica's connection, in + the interval. Decide explicitly: request-on-best-effort and drop, or a + short-lived pending-request record. Do NOT block the incident on it. +4. **Agent honours the request** by POSTing the bytes to + `POST /api/v1/agent/attachments?topic=…` with its normal signed headers. + +### Decisions to settle in the plan + +- **What if the agent is gone?** A capture that cannot be uploaded is simply + lost; the incident must open exactly as it does today. +- **Bounding the ask.** One upload request per incident transition, never per + failing result. The server must not be able to be talked into requesting an + unbounded number of uploads by a chatty agent's markers. +- **Trusting the marker.** `captureId` is agent-supplied. It names something in + the agent's OWN cache, so it is not a security input — but the topic the + server sends back must be server-generated, never echoed from the marker. + +### Non-goals + +- Changing the endpoint contract, the authorizer registry, or the topic grammar + — those are settled by 2026-08-21-01 and have direct tests. +- Screenshots for HTTP checks; "after" screenshots on resolve. + +## Tests + +- Agent LRU: bounded by count AND bytes, evicts oldest, TTL'd. +- Marker round-trip: an agent result frame carries `available`/`captureId` and + never the bytes (positive control: `failureResponse` still serializes). +- The upload-request frame is emitted on open/reopen only — proved with a + flapping sequence that opens exactly one incident. +- An agent that disconnected between the result and the request: the incident + still opens, nothing is retried forever. +- End-to-end: marker → request → POST → attachment visible on the incident. diff --git a/web/dash0/e2e/incident-screenshot.spec.ts b/web/dash0/e2e/incident-screenshot.spec.ts new file mode 100644 index 000000000..b2ec7170a --- /dev/null +++ b/web/dash0/e2e/incident-screenshot.spec.ts @@ -0,0 +1,112 @@ +import { test, expect } from "./fixtures"; + +// Coverage for spec 2026-08-21-01: the incident screenshot card, the first +// consumer of the generic attachments rail. +// +// Deterministically seeded in test mode (server/test/testdata/testdata.go, +// createTestIncidentScreenshot): a down browser check whose active incident +// carries a REAL screenshot attachment — bytes written through the normal +// attachment path, so the signed download URL serves a decodable PNG. +// +// Incident 17 (createTestFailureResponseCaptures) is the negative case: it has +// a failure-response capture but no attachment, so its page must render the +// probe-response card and NOT this one. +const SHOT_INCIDENT = "00000000-0000-0000-0000-000000000026"; +const NO_SHOT_INCIDENT = "00000000-0000-0000-0000-000000000017"; +const SHOT_CHECK = "00000000-0000-0000-0000-000000000025"; + +test.describe("Incident screenshot attachment", () => { + test("renders the image with a captured-at + region caption", async ({ + authenticatedPage, + }) => { + const page = authenticatedPage; + + await page.goto(`/dash0/orgs/test/incidents/${SHOT_INCIDENT}`); + await page.waitForLoadState("networkidle"); + + const card = page.getByTestId("incident-screenshot-card"); + await expect(card).toBeVisible(); + + // The card must never claim to be the failure frame itself. + await expect(card).toContainText("Screenshot at failure detection"); + await expect(card).toContainText("Captured a moment AFTER detection"); + + const caption = page.getByTestId("incident-screenshot-caption"); + await expect(caption).toBeVisible(); + await expect(caption).toContainText("eu-west"); + await expect(caption).toContainText("shortly after failure detection"); + await expect(caption).not.toContainText("an unknown region"); + await expect(caption).not.toContainText("at an unknown time"); + + // The signed URL is relative and points at the public files route. + const img = page.getByTestId("incident-screenshot-image"); + await expect(img).toBeVisible(); + + const src = await img.getAttribute("src"); + expect(src).toContain("/pub/files/"); + expect(src).toContain("sig="); + expect(src).toContain("exp="); + + // The bytes really are served and really are an image: a broken src would + // leave naturalWidth at 0, which is the failure mode a "visible" assertion + // alone would miss. + await expect + .poll(async () => + img.evaluate((el) => (el as HTMLImageElement).naturalWidth), + ) + .toBeGreaterThan(0); + }); + + test("the card is absent entirely when the incident has no attachment", async ({ + authenticatedPage, + }) => { + const page = authenticatedPage; + + // Positive control first: on the seeded incident the card IS rendered, so + // "absent" below is evidence of the guard rather than of a wrong test id. + await page.goto(`/dash0/orgs/test/incidents/${SHOT_INCIDENT}`); + await page.waitForLoadState("networkidle"); + await expect(page.getByTestId("incident-screenshot-card")).toBeVisible(); + + await page.goto(`/dash0/orgs/test/incidents/${NO_SHOT_INCIDENT}`); + await page.waitForLoadState("networkidle"); + + // The page really did render — its sibling cards are there; only the + // screenshot card is withheld. + await expect(page.getByTestId("probe-response-card")).toBeVisible(); + await expect(page.getByTestId("incident-screenshot-card")).toHaveCount(0); + }); + + test("the card is usable on mobile", async ({ authenticatedPage }) => { + const page = authenticatedPage; + + await page.setViewportSize({ width: 375, height: 812 }); + await page.goto(`/dash0/orgs/test/incidents/${SHOT_INCIDENT}`); + await page.waitForLoadState("networkidle"); + + await expect(page.getByTestId("incident-screenshot-card")).toBeVisible(); + await expect(page.getByTestId("incident-screenshot-image")).toBeVisible(); + + const hasOverflow = await page.evaluate( + () => + document.documentElement.scrollWidth > + document.documentElement.clientWidth + 1, + ); + expect(hasOverflow).toBe(false); + }); + + test("the browser check form round-trips the screenshot opt-in", async ({ + authenticatedPage, + }) => { + const page = authenticatedPage; + + await page.goto(`/dash0/orgs/test/checks/${SHOT_CHECK}/edit`); + await page.waitForLoadState("networkidle"); + + const checkbox = page.getByTestId("check-browser-screenshot-checkbox"); + await expect(checkbox).toBeVisible(); + // The fixture check has the option ON, so an unchecked box here would mean + // the form silently drops a stored setting on every edit. + await expect(checkbox).toBeChecked(); + }); +}); diff --git a/web/dash0/src/api/hooks.ts b/web/dash0/src/api/hooks.ts index c58a6c5bc..46fc78acb 100644 --- a/web/dash0/src/api/hooks.ts +++ b/web/dash0/src/api/hooks.ts @@ -62,7 +62,47 @@ export interface Check { * check inherits its group's policy, then the org default, then none. */ escalationPolicyUid?: string | null; - type?: "http" | "tcp" | "icmp" | "dns" | "ssl" | "heartbeat" | "email" | "domain" | "smtp" | "udp" | "ssh" | "pop3" | "imap" | "websocket" | "postgresql" | "mysql" | "redis" | "mongodb" | "ftp" | "sftp" | "js" | "mssql" | "oracle" | "clickhouse" | "grpc" | "kafka" | "mqtt" | "a2s" | "minecraft" | "rabbitmq" | "snmp" | "docker" | "browser" | "freebox_line" | "dnsbl" | "sip" | "ntp" | "rdp" | "prometheus" | "sleep"; + type?: + | "http" + | "tcp" + | "icmp" + | "dns" + | "ssl" + | "heartbeat" + | "email" + | "domain" + | "smtp" + | "udp" + | "ssh" + | "pop3" + | "imap" + | "websocket" + | "postgresql" + | "mysql" + | "redis" + | "mongodb" + | "ftp" + | "sftp" + | "js" + | "mssql" + | "oracle" + | "clickhouse" + | "grpc" + | "kafka" + | "mqtt" + | "a2s" + | "minecraft" + | "rabbitmq" + | "snmp" + | "docker" + | "browser" + | "freebox_line" + | "dnsbl" + | "sip" + | "ntp" + | "rdp" + | "prometheus" + | "sleep"; config?: Record; /** * Derived, read-time-only host this check probes: config's `host` when @@ -148,7 +188,47 @@ export interface CreateCheckRequest { checkGroupUid?: string; /** Escalation policy to assign; omit/empty inherits (group → org default → none). */ escalationPolicyUid?: string; - type?: "http" | "tcp" | "icmp" | "dns" | "ssl" | "heartbeat" | "email" | "domain" | "smtp" | "udp" | "ssh" | "pop3" | "imap" | "websocket" | "postgresql" | "mysql" | "redis" | "mongodb" | "ftp" | "sftp" | "js" | "mssql" | "oracle" | "clickhouse" | "grpc" | "kafka" | "mqtt" | "a2s" | "minecraft" | "rabbitmq" | "snmp" | "docker" | "browser" | "freebox_line" | "dnsbl" | "sip" | "ntp" | "rdp" | "prometheus" | "sleep"; + type?: + | "http" + | "tcp" + | "icmp" + | "dns" + | "ssl" + | "heartbeat" + | "email" + | "domain" + | "smtp" + | "udp" + | "ssh" + | "pop3" + | "imap" + | "websocket" + | "postgresql" + | "mysql" + | "redis" + | "mongodb" + | "ftp" + | "sftp" + | "js" + | "mssql" + | "oracle" + | "clickhouse" + | "grpc" + | "kafka" + | "mqtt" + | "a2s" + | "minecraft" + | "rabbitmq" + | "snmp" + | "docker" + | "browser" + | "freebox_line" + | "dnsbl" + | "sip" + | "ntp" + | "rdp" + | "prometheus" + | "sleep"; config: Record; regions?: string[]; /** Omit to use the automatic default (period / region count). */ @@ -257,6 +337,27 @@ export interface IncidentFailureResponse { region?: string; } +/** One evidence blob attached to an incident (spec 2026-08-21-01). + * + * Operator-only: `downloadUrl` is a short-lived SIGNED url and is re-signed on + * every incident fetch, so it must never be cached or shared onward. */ +export interface IncidentAttachment { + uid: string; + /** Attachment kind, e.g. `screenshot`. */ + kind?: string; + name?: string; + mimeType?: string; + size?: number; + /** Relative signed URL: `/pub/files/?exp=…&sig=…`. */ + downloadUrl: string; + createdAt?: string; + /** When the probe took the capture — a moment AFTER failure detection. */ + capturedAt?: string; + region?: string; + checkUid?: string; + trigger?: "incident-open" | "incident-reopen" | "agent-upload"; +} + export interface IncidentDetails { /** Human-readable cause, same key the Slack notifier reads. */ failure_reason?: string; @@ -269,6 +370,8 @@ export interface IncidentDetails { } export interface IncidentDetail { + /** Evidence blobs. Populated by the DETAIL endpoint only. */ + attachments?: IncidentAttachment[]; uid?: string; /** Short per-org reference, rendered as `#42`. Assigned at creation, never reused. */ number?: number; @@ -342,14 +445,15 @@ function buildChecksUrl( /** Opt-in ordering. "group" = group sortOrder asc, ungrouped last, then * created_at DESC within a bucket. Omitted = default created_at DESC. */ sort?: string; - } + }, ): string { const params = new URLSearchParams(); if (options?.labels) params.set("labels", options.labels); if (options?.with) params.set("with", options.with); if (options?.q) params.set("q", options.q); if (options?.type) params.set("type", options.type); - if (options?.checkGroupUid) params.set("checkGroupUid", options.checkGroupUid); + if (options?.checkGroupUid) + params.set("checkGroupUid", options.checkGroupUid); if (options?.internal) params.set("internal", options.internal); if (options?.status) params.set("status", options.status); if (options?.limit) params.set("limit", options.limit.toString()); @@ -370,7 +474,7 @@ export function useChecks( type?: string; checkGroupUid?: string; limit?: number; - } + }, ) { return useQuery({ queryKey: ["checks", org, options], @@ -414,7 +518,7 @@ export interface CheckStats { */ export function useCheckStats( org: string, - options?: { refetchInterval?: number } + options?: { refetchInterval?: number }, ) { return useQuery({ queryKey: ["check-stats", org], @@ -443,7 +547,7 @@ export function useInfiniteChecks( * describes how often to refresh a cache entry, not which entry it is). * Kept as a second parameter so it can never accidentally fork the key. */ - queryOptions?: { refetchInterval?: number } + queryOptions?: { refetchInterval?: number }, ) { return useInfiniteQuery({ queryKey: ["checks", "infinite", org, options], @@ -472,7 +576,7 @@ export function useCheck( * guarantees the seed comes from fresh data, not a stale cache entry. */ refetchOnMount?: boolean | "always"; - } + }, ) { return useQuery({ // One canonical cache entry per check, keyed by org+uid only — every @@ -484,7 +588,7 @@ export function useCheck( queryKey: ["check", org, uid], queryFn: async () => apiFetch( - `/api/v1/orgs/${org}/checks/${uid}?with=last_result,last_status_change` + `/api/v1/orgs/${org}/checks/${uid}?with=last_result,last_status_change`, ), enabled: !!org && !!uid, refetchInterval: options?.refetchInterval, @@ -532,7 +636,7 @@ export interface LabelSuggestion { export function useLabelSuggestions( org: string, - opts: { key?: string; q?: string; limit?: number; enabled?: boolean } + opts: { key?: string; q?: string; limit?: number; enabled?: boolean }, ) { const params = new URLSearchParams(); if (opts.key) params.set("key", opts.key); @@ -688,7 +792,11 @@ export function useConvertChecks(org: string) { const queryClient = useQueryClient(); return useMutation({ - mutationFn: (params: { source: ConvertSource; body: string; dryRun?: boolean }) => + mutationFn: (params: { + source: ConvertSource; + body: string; + dryRun?: boolean; + }) => apiFetch( `/api/v1/orgs/${org}/checks/import/convert?source=${encodeURIComponent(params.source)}${ params.dryRun ? "&dryRun=true" : "" @@ -740,7 +848,7 @@ export function useCheckGroups(org: string) { queryKey: ["checkGroups", org], queryFn: async () => { const response = await apiFetch<{ data?: CheckGroup[] }>( - `/api/v1/orgs/${org}/check-groups` + `/api/v1/orgs/${org}/check-groups`, ); return response.data || []; }, @@ -924,10 +1032,7 @@ export function useDeleteCheckDependency(org: string, checkUid: string) { }); } -export function useDependencyGraph( - org: string, - opts?: { enabled?: boolean }, -) { +export function useDependencyGraph(org: string, opts?: { enabled?: boolean }) { return useQuery({ queryKey: ["dependencyGraph", org], queryFn: async () => { @@ -954,7 +1059,7 @@ export function useResults( cursor?: string; size?: number; refetchInterval?: number; - } + }, ) { const { refetchInterval, ...queryOptions } = options || {}; return useQuery({ @@ -963,8 +1068,10 @@ export function useResults( const params = new URLSearchParams(); if (options?.checkUid) params.set("checkUid", options.checkUid); if (options?.periodType) params.set("periodType", options.periodType); - if (options?.periodStartAfter) params.set("periodStartAfter", options.periodStartAfter); - if (options?.periodEndBefore) params.set("periodEndBefore", options.periodEndBefore); + if (options?.periodStartAfter) + params.set("periodStartAfter", options.periodStartAfter); + if (options?.periodEndBefore) + params.set("periodEndBefore", options.periodEndBefore); if (options?.region) params.set("region", options.region); if (options?.with) params.set("with", options.with); if (options?.cursor) params.set("cursor", options.cursor); @@ -991,7 +1098,7 @@ export function useResult( org: string, checkUid: string, resultUid: string, - options?: { region?: string } + options?: { region?: string }, ) { const region = options?.region; return useQuery({ @@ -1078,7 +1185,7 @@ export function useAllResults( with?: string; size?: number; refetchInterval?: number; - } + }, ) { const { refetchInterval, ...queryOptions } = options || {}; return useQuery({ @@ -1138,7 +1245,7 @@ export function useIncidents( // wire request always carries a UID and never a slug (issue #127). // Defaults to true so existing callers are unaffected. enabled?: boolean; - } + }, ) { const { refetchInterval, enabled, ...queryOptions } = options || {}; return useQuery({ @@ -1154,7 +1261,8 @@ export function useIncidents( if (options?.size) params.set("limit", options.size.toString()); if (options?.with) params.set("with", options.with); if (options?.hideSuppressed) params.set("hideSuppressed", "true"); - if (options?.causedByIncidentUid) params.set("causedByIncidentUid", options.causedByIncidentUid); + if (options?.causedByIncidentUid) + params.set("causedByIncidentUid", options.causedByIncidentUid); const query = params.toString(); const path = `/api/v1/orgs/${org}/incidents${query ? `?${query}` : ""}`; const response = await apiFetch<{ @@ -1269,7 +1377,7 @@ export function useEvents( cursor?: string; size?: number; refetchInterval?: number; - } + }, ) { const { refetchInterval, ...queryOptions } = options || {}; return useQuery({ @@ -1361,7 +1469,7 @@ export function useIncidentNotifications( options?: { status?: string; limit?: number; - } + }, ) { return useQuery({ queryKey: ["incidentNotifications", org, incidentUid, options], @@ -1381,13 +1489,13 @@ export function useIncidentNotifications( export function useIncidentNotification( org: string, incidentUid: string, - notifUid: string + notifUid: string, ) { return useQuery({ queryKey: ["incidentNotification", org, incidentUid, notifUid], queryFn: () => apiFetch( - `/api/v1/orgs/${org}/incidents/${incidentUid}/notifications/${notifUid}` + `/api/v1/orgs/${org}/incidents/${incidentUid}/notifications/${notifUid}`, ), enabled: !!org && !!incidentUid && !!notifUid, staleTime: Infinity, @@ -1402,7 +1510,7 @@ export function useOrgNotification(org: string, notifUid: string) { queryKey: ["orgNotification", org, notifUid], queryFn: () => apiFetch( - `/api/v1/orgs/${org}/notifications/${notifUid}` + `/api/v1/orgs/${org}/notifications/${notifUid}`, ), enabled: !!org && !!notifUid, staleTime: Infinity, @@ -1414,7 +1522,7 @@ export function useOrgNotification(org: string, notifUid: string) { export function useIntegrationNotifications( org: string, integrationUid: string, - limit = 10 + limit = 10, ) { return useQuery({ queryKey: ["integrationNotifications", org, integrationUid, limit], @@ -1424,7 +1532,7 @@ export function useIntegrationNotifications( limit: String(limit), }); const response = await apiFetch<{ data?: IncidentNotification[] }>( - `/api/v1/orgs/${org}/notifications?${params.toString()}` + `/api/v1/orgs/${org}/notifications?${params.toString()}`, ); return response.data || []; }, @@ -1452,7 +1560,7 @@ export function useEmailSuppressions(org: string) { queryKey: ["emailSuppressions", org], queryFn: async () => { const response = await apiFetch<{ data?: EmailSuppression[] }>( - `/api/v1/orgs/${org}/email-suppressions` + `/api/v1/orgs/${org}/email-suppressions`, ); return response.data || []; }, @@ -1482,7 +1590,7 @@ export function useMyNotifications( status?: string; limit?: number; before?: string; - } + }, ) { return useQuery({ queryKey: ["myNotifications", org, options], @@ -1548,7 +1656,7 @@ export function useTokens(org: string) { queryKey: ["tokens", org], queryFn: async () => { const response = await apiFetch<{ data?: TokenInfo[] }>( - `/api/v1/orgs/${org}/tokens?type=pat` + `/api/v1/orgs/${org}/tokens?type=pat`, ); return response.data || []; }, @@ -1641,7 +1749,7 @@ export function useSessions(org: string) { queryKey: ["sessions", org], queryFn: async () => { const response = await apiFetch<{ data?: SessionInfo[] }>( - `/api/v1/orgs/${org}/tokens?type=refresh` + `/api/v1/orgs/${org}/tokens?type=refresh`, ); return response.data || []; }, @@ -1888,7 +1996,7 @@ export function useStatusPages(org: string, opts?: { enabled?: boolean }) { queryKey: ["statusPages", org], queryFn: async () => { const response = await apiFetch<{ data?: StatusPage[] }>( - `/api/v1/orgs/${org}/status-pages` + `/api/v1/orgs/${org}/status-pages`, ); return response.data || []; }, @@ -1896,7 +2004,11 @@ export function useStatusPages(org: string, opts?: { enabled?: boolean }) { }); } -export function useStatusPage(org: string, uid: string, options?: { with?: string }) { +export function useStatusPage( + org: string, + uid: string, + options?: { with?: string }, +) { return useQuery({ queryKey: ["statusPage", org, uid, { with: options?.with }], queryFn: async () => { @@ -1964,7 +2076,7 @@ export function useVerifyStatusPageDomain(org: string, uid: string) { mutationFn: () => apiFetch( `/api/v1/orgs/${org}/status-pages/${uid}/custom-domain/verify`, - { method: "POST" } + { method: "POST" }, ), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["statusPages", org] }); @@ -2002,7 +2114,7 @@ export function useStatusPageSubscribers(org: string, statusPageUid: string) { export function useDeleteStatusPageSubscriber( org: string, - statusPageUid: string + statusPageUid: string, ) { const queryClient = useQueryClient(); @@ -2010,7 +2122,7 @@ export function useDeleteStatusPageSubscriber( mutationFn: (uid: string) => apiFetch( `/api/v1/orgs/${org}/status-pages/${statusPageUid}/subscribers/${uid}`, - { method: "DELETE" } + { method: "DELETE" }, ), onSuccess: () => { queryClient.invalidateQueries({ @@ -2026,7 +2138,7 @@ export function useStatusPageSections(org: string, statusPageUid: string) { queryKey: ["statusPageSections", org, statusPageUid], queryFn: async () => { const response = await apiFetch<{ data?: StatusPageSection[] }>( - `/api/v1/orgs/${org}/status-pages/${statusPageUid}/sections` + `/api/v1/orgs/${org}/status-pages/${statusPageUid}/sections`, ); return response.data || []; }, @@ -2041,27 +2153,39 @@ export function useCreateSection(org: string, statusPageUid: string) { mutationFn: (request: CreateSectionRequest) => apiFetch( `/api/v1/orgs/${org}/status-pages/${statusPageUid}/sections`, - { method: "POST", body: JSON.stringify(request) } + { method: "POST", body: JSON.stringify(request) }, ), onSuccess: () => { - queryClient.invalidateQueries({ queryKey: ["statusPageSections", org, statusPageUid] }); - queryClient.invalidateQueries({ queryKey: ["statusPage", org, statusPageUid] }); + queryClient.invalidateQueries({ + queryKey: ["statusPageSections", org, statusPageUid], + }); + queryClient.invalidateQueries({ + queryKey: ["statusPage", org, statusPageUid], + }); }, }); } -export function useUpdateSection(org: string, statusPageUid: string, sectionUid: string) { +export function useUpdateSection( + org: string, + statusPageUid: string, + sectionUid: string, +) { const queryClient = useQueryClient(); return useMutation({ mutationFn: (request: UpdateSectionRequest) => apiFetch( `/api/v1/orgs/${org}/status-pages/${statusPageUid}/sections/${sectionUid}`, - { method: "PATCH", body: JSON.stringify(request) } + { method: "PATCH", body: JSON.stringify(request) }, ), onSuccess: () => { - queryClient.invalidateQueries({ queryKey: ["statusPageSections", org, statusPageUid] }); - queryClient.invalidateQueries({ queryKey: ["statusPage", org, statusPageUid] }); + queryClient.invalidateQueries({ + queryKey: ["statusPageSections", org, statusPageUid], + }); + queryClient.invalidateQueries({ + queryKey: ["statusPage", org, statusPageUid], + }); }, }); } @@ -2073,11 +2197,15 @@ export function useDeleteSection(org: string, statusPageUid: string) { mutationFn: (sectionUid: string) => apiFetch( `/api/v1/orgs/${org}/status-pages/${statusPageUid}/sections/${sectionUid}`, - { method: "DELETE" } + { method: "DELETE" }, ), onSuccess: () => { - queryClient.invalidateQueries({ queryKey: ["statusPageSections", org, statusPageUid] }); - queryClient.invalidateQueries({ queryKey: ["statusPage", org, statusPageUid] }); + queryClient.invalidateQueries({ + queryKey: ["statusPageSections", org, statusPageUid], + }); + queryClient.invalidateQueries({ + queryKey: ["statusPage", org, statusPageUid], + }); }, }); } @@ -2086,13 +2214,13 @@ export function useDeleteSection(org: string, statusPageUid: string) { export function useStatusPageResources( org: string, statusPageUid: string, - sectionUid: string + sectionUid: string, ) { return useQuery({ queryKey: ["statusPageResources", org, statusPageUid, sectionUid], queryFn: async () => { const response = await apiFetch<{ data?: StatusPageResource[] }>( - `/api/v1/orgs/${org}/status-pages/${statusPageUid}/sections/${sectionUid}/resources` + `/api/v1/orgs/${org}/status-pages/${statusPageUid}/sections/${sectionUid}/resources`, ); return response.data || []; }, @@ -2100,20 +2228,26 @@ export function useStatusPageResources( }); } -export function useCreateResource(org: string, statusPageUid: string, sectionUid: string) { +export function useCreateResource( + org: string, + statusPageUid: string, + sectionUid: string, +) { const queryClient = useQueryClient(); return useMutation({ mutationFn: (request: CreateResourceRequest) => apiFetch( `/api/v1/orgs/${org}/status-pages/${statusPageUid}/sections/${sectionUid}/resources`, - { method: "POST", body: JSON.stringify(request) } + { method: "POST", body: JSON.stringify(request) }, ), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["statusPageResources", org, statusPageUid, sectionUid], }); - queryClient.invalidateQueries({ queryKey: ["statusPage", org, statusPageUid] }); + queryClient.invalidateQueries({ + queryKey: ["statusPage", org, statusPageUid], + }); }, }); } @@ -2128,17 +2262,25 @@ export function useCreateResource(org: string, statusPageUid: string, sectionUid // original slot before the server roundtrip lands. export function useReorderSections(org: string, statusPageUid: string) { const queryClient = useQueryClient(); - const pageWithSectionsKey = ["statusPage", org, statusPageUid, { with: "sections" }]; + const pageWithSectionsKey = [ + "statusPage", + org, + statusPageUid, + { with: "sections" }, + ]; return useMutation({ mutationFn: (uids: string[]) => apiFetch( `/api/v1/orgs/${org}/status-pages/${statusPageUid}/sections/reorder`, - { method: "POST", body: JSON.stringify({ uids }) } + { method: "POST", body: JSON.stringify({ uids }) }, ), onMutate: async (uids) => { - await queryClient.cancelQueries({ queryKey: ["statusPage", org, statusPageUid] }); - const snapshot = queryClient.getQueryData(pageWithSectionsKey); + await queryClient.cancelQueries({ + queryKey: ["statusPage", org, statusPageUid], + }); + const snapshot = + queryClient.getQueryData(pageWithSectionsKey); if (snapshot?.sections) { const byUid = new Map(snapshot.sections.map((s) => [s.uid, s])); const reordered = uids @@ -2157,24 +2299,38 @@ export function useReorderSections(org: string, statusPageUid: string) { } }, onSettled: () => { - queryClient.invalidateQueries({ queryKey: ["statusPage", org, statusPageUid] }); + queryClient.invalidateQueries({ + queryKey: ["statusPage", org, statusPageUid], + }); }, }); } -export function useReorderResources(org: string, statusPageUid: string, sectionUid: string) { +export function useReorderResources( + org: string, + statusPageUid: string, + sectionUid: string, +) { const queryClient = useQueryClient(); - const pageWithSectionsKey = ["statusPage", org, statusPageUid, { with: "sections" }]; + const pageWithSectionsKey = [ + "statusPage", + org, + statusPageUid, + { with: "sections" }, + ]; return useMutation({ mutationFn: (uids: string[]) => apiFetch( `/api/v1/orgs/${org}/status-pages/${statusPageUid}/sections/${sectionUid}/resources/reorder`, - { method: "POST", body: JSON.stringify({ uids }) } + { method: "POST", body: JSON.stringify({ uids }) }, ), onMutate: async (uids) => { - await queryClient.cancelQueries({ queryKey: ["statusPage", org, statusPageUid] }); - const snapshot = queryClient.getQueryData(pageWithSectionsKey); + await queryClient.cancelQueries({ + queryKey: ["statusPage", org, statusPageUid], + }); + const snapshot = + queryClient.getQueryData(pageWithSectionsKey); if (snapshot) { queryClient.setQueryData(pageWithSectionsKey, { ...snapshot, @@ -2199,43 +2355,63 @@ export function useReorderResources(org: string, statusPageUid: string, sectionU queryClient.invalidateQueries({ queryKey: ["statusPageResources", org, statusPageUid, sectionUid], }); - queryClient.invalidateQueries({ queryKey: ["statusPage", org, statusPageUid] }); + queryClient.invalidateQueries({ + queryKey: ["statusPage", org, statusPageUid], + }); }, }); } -export function useUpdateResource(org: string, statusPageUid: string, sectionUid: string) { +export function useUpdateResource( + org: string, + statusPageUid: string, + sectionUid: string, +) { const queryClient = useQueryClient(); return useMutation({ - mutationFn: ({ resourceUid, request }: { resourceUid: string; request: UpdateResourceRequest }) => + mutationFn: ({ + resourceUid, + request, + }: { + resourceUid: string; + request: UpdateResourceRequest; + }) => apiFetch( `/api/v1/orgs/${org}/status-pages/${statusPageUid}/sections/${sectionUid}/resources/${resourceUid}`, - { method: "PATCH", body: JSON.stringify(request) } + { method: "PATCH", body: JSON.stringify(request) }, ), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["statusPageResources", org, statusPageUid, sectionUid], }); - queryClient.invalidateQueries({ queryKey: ["statusPage", org, statusPageUid] }); + queryClient.invalidateQueries({ + queryKey: ["statusPage", org, statusPageUid], + }); }, }); } -export function useDeleteResource(org: string, statusPageUid: string, sectionUid: string) { +export function useDeleteResource( + org: string, + statusPageUid: string, + sectionUid: string, +) { const queryClient = useQueryClient(); return useMutation({ mutationFn: (resourceUid: string) => apiFetch( `/api/v1/orgs/${org}/status-pages/${statusPageUid}/sections/${sectionUid}/resources/${resourceUid}`, - { method: "DELETE" } + { method: "DELETE" }, ), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["statusPageResources", org, statusPageUid, sectionUid], }); - queryClient.invalidateQueries({ queryKey: ["statusPage", org, statusPageUid] }); + queryClient.invalidateQueries({ + queryKey: ["statusPage", org, statusPageUid], + }); }, }); } @@ -2257,7 +2433,7 @@ export function useProviders() { queryFn: async () => { const response = await apiFetch( "/api/v1/auth/providers", - { skipAuth: true } + { skipAuth: true }, ); return { providers: response.data || [], @@ -2291,7 +2467,12 @@ export function useConfirmRegistration() { accessToken: string; refreshToken?: string; expiresIn?: number; - user: { email: string; name?: string; avatarUrl?: string; role: string }; + user: { + email: string; + name?: string; + avatarUrl?: string; + role: string; + }; organization?: { uid: string; slug: string; name?: string }; }>("/api/v1/auth/confirm-registration", { method: "POST", @@ -2328,7 +2509,13 @@ export function useUpdateProfile() { return useMutation({ mutationFn: (data: { name: string }) => apiFetch<{ - user: { uid: string; email: string; name?: string; avatarUrl?: string; role: string }; + user: { + uid: string; + email: string; + name?: string; + avatarUrl?: string; + role: string; + }; organization: { uid: string; slug: string; name?: string }; organizations: { slug: string; name?: string; role: string }[]; }>("/api/v1/auth/me", { @@ -2386,7 +2573,7 @@ export function useCreateInvitation(org: string) { { method: "POST", body: JSON.stringify(data), - } + }, ), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["invitations", org] }); @@ -2443,7 +2630,9 @@ export function useCreateMembershipRequest() { body: JSON.stringify(data), }), onSuccess: () => { - queryClient.invalidateQueries({ queryKey: ["membership-requests", "me"] }); + queryClient.invalidateQueries({ + queryKey: ["membership-requests", "me"], + }); queryClient.invalidateQueries({ queryKey: ["auth", "me"] }); }, }); @@ -2454,7 +2643,7 @@ export function useMyMembershipRequests() { queryKey: ["membership-requests", "me"], queryFn: () => apiFetch<{ data: MembershipRequestSummary[] }>( - "/api/v1/auth/membership-requests" + "/api/v1/auth/membership-requests", ), }); } @@ -2467,7 +2656,9 @@ export function useCancelMembershipRequest() { method: "DELETE", }), onSuccess: () => { - queryClient.invalidateQueries({ queryKey: ["membership-requests", "me"] }); + queryClient.invalidateQueries({ + queryKey: ["membership-requests", "me"], + }); queryClient.invalidateQueries({ queryKey: ["auth", "me"] }); }, }); @@ -2475,7 +2666,7 @@ export function useCancelMembershipRequest() { export function useOrgMembershipRequests( org: string, - opts?: { status?: MembershipRequestStatus; enabled?: boolean } + opts?: { status?: MembershipRequestStatus; enabled?: boolean }, ) { const status = opts?.status; const qs = status ? `?status=${status}` : ""; @@ -2483,7 +2674,7 @@ export function useOrgMembershipRequests( queryKey: ["membership-requests", "org", org, status ?? "all"], queryFn: () => apiFetch<{ data: MembershipRequestAdminView[] }>( - `/api/v1/orgs/${org}/membership-requests${qs}` + `/api/v1/orgs/${org}/membership-requests${qs}`, ), enabled: opts?.enabled !== false, }); @@ -2493,13 +2684,10 @@ export function useApproveMembershipRequest(org: string) { const queryClient = useQueryClient(); return useMutation({ mutationFn: ({ uid, role }: { uid: string; role?: string }) => - apiFetch( - `/api/v1/orgs/${org}/membership-requests/${uid}/approve`, - { - method: "POST", - body: JSON.stringify(role ? { role } : {}), - } - ), + apiFetch(`/api/v1/orgs/${org}/membership-requests/${uid}/approve`, { + method: "POST", + body: JSON.stringify(role ? { role } : {}), + }), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["membership-requests", "org", org], @@ -2513,13 +2701,10 @@ export function useRejectMembershipRequest(org: string) { const queryClient = useQueryClient(); return useMutation({ mutationFn: ({ uid, reason }: { uid: string; reason?: string }) => - apiFetch( - `/api/v1/orgs/${org}/membership-requests/${uid}/reject`, - { - method: "POST", - body: JSON.stringify(reason ? { reason } : {}), - } - ), + apiFetch(`/api/v1/orgs/${org}/membership-requests/${uid}/reject`, { + method: "POST", + body: JSON.stringify(reason ? { reason } : {}), + }), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["membership-requests", "org", org], @@ -2595,7 +2780,12 @@ export interface DeleteOrgResponse { role: string; }; organization?: { uid: string; slug: string; name?: string }; - organizations?: { slug: string; name?: string; logoUrl?: string; role: string }[]; + organizations?: { + slug: string; + name?: string; + logoUrl?: string; + role: string; + }[]; loginAction?: string; } @@ -2712,18 +2902,20 @@ export function useInviteInfo(token: string) { export interface AcceptInviteResponse { accessToken: string; - user: { uid: string; email: string; name?: string; avatarUrl?: string; role: string }; + user: { + uid: string; + email: string; + name?: string; + avatarUrl?: string; + role: string; + }; organization: { uid: string; slug: string; name?: string }; organizations?: Array<{ slug: string; name?: string; role: string }>; } export function useAcceptInvite() { return useMutation({ - mutationFn: (data: { - token: string; - name?: string; - password?: string; - }) => + mutationFn: (data: { token: string; name?: string; password?: string }) => apiFetch("/api/v1/auth/accept-invite", { method: "POST", body: JSON.stringify(data), @@ -2750,8 +2942,7 @@ export interface OrgSettings { export function useOrgSettings(org: string) { return useQuery({ queryKey: ["org-settings", org], - queryFn: () => - apiFetch(`/api/v1/orgs/${org}/settings`), + queryFn: () => apiFetch(`/api/v1/orgs/${org}/settings`), }); } @@ -2965,9 +3156,12 @@ interface SystemParametersResponse { async function fetchSystemParameters(): Promise { const response = await apiFetch( - "/api/v1/system/parameters" + "/api/v1/system/parameters", ); - return { data: response.data || [], envOverrides: response.envOverrides || [] }; + return { + data: response.data || [], + envOverrides: response.envOverrides || [], + }; } export function useSystemParameters() { @@ -3013,7 +3207,7 @@ export function useSchedulingLaneLoad() { queryKey: ["system-scheduling-lane-load"], queryFn: async () => { const response = await apiFetch<{ data: WorkerLaneLoad[] }>( - "/api/v1/system/scheduling/lane-load" + "/api/v1/system/scheduling/lane-load", ); return response.data || []; }, @@ -3033,13 +3227,10 @@ export function useSetSystemParameter() { value: unknown; secret?: boolean; }) => - apiFetch( - `/api/v1/system/parameters/${key}`, - { - method: "PUT", - body: JSON.stringify({ value, secret }), - } - ), + apiFetch(`/api/v1/system/parameters/${key}`, { + method: "PUT", + body: JSON.stringify({ value, secret }), + }), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["system-parameters"] }); }, @@ -3059,9 +3250,7 @@ export function useSlackSocketStatus() { return useQuery({ queryKey: ["slack-socket-status"], queryFn: async () => - apiFetch( - "/api/v1/integrations/slack/socket/status", - ), + apiFetch("/api/v1/integrations/slack/socket/status"), refetchInterval: 5000, refetchIntervalInBackground: false, }); @@ -3075,7 +3264,7 @@ export function useTestEmail() { { method: "POST", body: JSON.stringify({ recipient }), - } + }, ), }); } @@ -3135,7 +3324,7 @@ export function useCheckTypes(org: string) { queryKey: ["check-types", org], queryFn: async () => { const response = await apiFetch<{ data: CheckTypeInfo[] }>( - `/api/v1/orgs/${org}/check-types` + `/api/v1/orgs/${org}/check-types`, ); return response.data || []; }, @@ -3148,9 +3337,9 @@ export function useSampleConfigs(checkType: string) { return useQuery({ queryKey: ["check-types", "samples", checkType], queryFn: async () => { - const response = await apiFetch<{ data: Array<{ checkType: string; samples: SampleConfig[] }> }>( - `/api/v1/check-types/samples?type=${encodeURIComponent(checkType)}` - ); + const response = await apiFetch<{ + data: Array<{ checkType: string; samples: SampleConfig[] }>; + }>(`/api/v1/check-types/samples?type=${encodeURIComponent(checkType)}`); return response.data?.[0]?.samples || []; }, staleTime: 10 * 60 * 1000, @@ -3249,18 +3438,12 @@ export function useOnCallSchedule(org: string, uid: string) { return useQuery({ queryKey: ["onCallSchedules", org, uid], queryFn: () => - apiFetch( - `/api/v1/orgs/${org}/on-call-schedules/${uid}`, - ), + apiFetch(`/api/v1/orgs/${org}/on-call-schedules/${uid}`), enabled: !!org && !!uid, }); } -export function useOnCallSchedulePreview( - org: string, - uid: string, - days = 14, -) { +export function useOnCallSchedulePreview(org: string, uid: string, days = 14) { return useQuery({ queryKey: ["onCallSchedules", org, uid, "preview", days], queryFn: async () => { @@ -3304,10 +3487,10 @@ export function useUpdateOnCallSchedule(org: string, uid: string) { const queryClient = useQueryClient(); return useMutation({ mutationFn: (request: UpdateOnCallScheduleRequest) => - apiFetch( - `/api/v1/orgs/${org}/on-call-schedules/${uid}`, - { method: "PATCH", body: JSON.stringify(request) }, - ), + apiFetch(`/api/v1/orgs/${org}/on-call-schedules/${uid}`, { + method: "PATCH", + body: JSON.stringify(request), + }), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["onCallSchedules", org] }); }, @@ -3574,7 +3757,10 @@ export interface UpdateEscalationPolicyRequest { steps?: EscalationPolicyStep[]; } -export function useEscalationPolicies(org: string, opts?: { enabled?: boolean }) { +export function useEscalationPolicies( + org: string, + opts?: { enabled?: boolean }, +) { return useQuery({ queryKey: ["escalationPolicies", org], queryFn: async () => { @@ -4128,8 +4314,7 @@ export interface IntegrationIdentityListResponse { data: IntegrationIdentity[]; } -export interface IntegrationIdentitySyncResponse - extends IntegrationIdentityListResponse { +export interface IntegrationIdentitySyncResponse extends IntegrationIdentityListResponse { matchedCount: number; notFoundCount: number; ambiguousCount: number; @@ -4171,10 +4356,7 @@ export function useSyncIntegrationIdentities( }); } -export function useSetIntegrationIdentity( - org: string, - integrationUid: string, -) { +export function useSetIntegrationIdentity(org: string, integrationUid: string) { const queryClient = useQueryClient(); return useMutation({ @@ -4384,7 +4566,13 @@ export interface StatusUpdate { title: string; bodyMarkdown: string; linkUrl?: string; - kind: "investigating" | "identified" | "monitoring" | "resolved" | "maintenance" | "info"; + kind: + | "investigating" + | "identified" + | "monitoring" + | "resolved" + | "maintenance" + | "info"; publishedAt: string; authorUid: string; createdAt: string; @@ -4423,7 +4611,7 @@ export function useStatusUpdates( incident?: string; limit?: number; offset?: number; - } = {} + } = {}, ) { return useQuery({ queryKey: ["statusUpdates", org, params], @@ -4437,7 +4625,7 @@ export function useStatusUpdates( if (params.offset) query.set("offset", params.offset.toString()); const qs = query.toString(); const response = await apiFetch<{ data?: StatusUpdate[] }>( - `/api/v1/orgs/${org}/status-updates${qs ? `?${qs}` : ""}` + `/api/v1/orgs/${org}/status-updates${qs ? `?${qs}` : ""}`, ); return response.data || []; }, @@ -4572,7 +4760,9 @@ export function useDiscoveryTypes(org: string) { return useQuery({ queryKey: ["discoveryTypes", org], queryFn: () => - apiFetch<{ data: DiscoveryType[] }>(`/api/v1/orgs/${org}/discovery/types`), + apiFetch<{ data: DiscoveryType[] }>( + `/api/v1/orgs/${org}/discovery/types`, + ), select: (res) => res?.data ?? [], }); } @@ -4598,7 +4788,9 @@ export function useListDiscoveryScans(org: string) { return useQuery({ queryKey: ["discoveryScans", org], queryFn: () => - apiFetch<{ data: DiscoveryScan[] }>(`/api/v1/orgs/${org}/discovery/scans`), + apiFetch<{ data: DiscoveryScan[] }>( + `/api/v1/orgs/${org}/discovery/scans`, + ), select: (res) => res?.data ?? [], }); } @@ -4642,7 +4834,9 @@ export function useCancelScan(org: string) { method: "POST", }), onSuccess: (_data, jobUid) => { - queryClient.invalidateQueries({ queryKey: ["discoveryScan", org, jobUid] }); + queryClient.invalidateQueries({ + queryKey: ["discoveryScan", org, jobUid], + }); queryClient.invalidateQueries({ queryKey: ["discoveryScans", org] }); }, }); @@ -4653,13 +4847,19 @@ export function useCancelScan(org: string) { // pollWhileActive to stream rows in as chunks land. export function useListDiscoveredChecks( org: string, - opts?: { jobUid?: string; group?: string; promoted?: boolean; source?: string }, + opts?: { + jobUid?: string; + group?: string; + promoted?: boolean; + source?: string; + }, pollWhileActive = false, ) { const params = new URLSearchParams(); if (opts?.jobUid) params.set("jobUid", opts.jobUid); if (opts?.group) params.set("group", opts.group); - if (opts?.promoted !== undefined) params.set("promoted", String(opts.promoted)); + if (opts?.promoted !== undefined) + params.set("promoted", String(opts.promoted)); if (opts?.source) params.set("source", opts.source); const qs = params.toString(); @@ -4759,7 +4959,9 @@ export function useNotificationRoutes(org: string) { return useQuery({ queryKey: ["notificationRoutes", org], queryFn: () => - apiFetch(`/api/v1/orgs/${org}/users/me/notification-routes`), + apiFetch( + `/api/v1/orgs/${org}/users/me/notification-routes`, + ), }); } @@ -4767,10 +4969,13 @@ export function useCreateNotificationContact(org: string) { const queryClient = useQueryClient(); return useMutation({ mutationFn: (body: { type: string; value: string; label?: string }) => - apiFetch(`/api/v1/orgs/${org}/users/me/notification-contacts`, { - method: "POST", - body: JSON.stringify(body), - }), + apiFetch( + `/api/v1/orgs/${org}/users/me/notification-contacts`, + { + method: "POST", + body: JSON.stringify(body), + }, + ), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["notificationRoutes", org] }); }, @@ -4781,9 +4986,12 @@ export function useDeleteNotificationContact(org: string) { const queryClient = useQueryClient(); return useMutation({ mutationFn: (contactUid: string) => - apiFetch(`/api/v1/orgs/${org}/users/me/notification-contacts/${contactUid}`, { - method: "DELETE", - }), + apiFetch( + `/api/v1/orgs/${org}/users/me/notification-contacts/${contactUid}`, + { + method: "DELETE", + }, + ), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["notificationRoutes", org] }); }, @@ -4818,9 +5026,12 @@ export interface TelegramLinkResponse { export function useCreateTelegramLink(org: string) { return useMutation({ mutationFn: () => - apiFetch(`/api/v1/orgs/${org}/users/me/telegram/link`, { - method: "POST", - }), + apiFetch( + `/api/v1/orgs/${org}/users/me/telegram/link`, + { + method: "POST", + }, + ), }); } @@ -4842,11 +5053,20 @@ export function useConfirmVerifyContact(org: string) { export function usePatchNotificationRoute(org: string) { const queryClient = useQueryClient(); return useMutation({ - mutationFn: ({ routeUid, patch }: { routeUid: string; patch: { enabled?: boolean } }) => - apiFetch(`/api/v1/orgs/${org}/users/me/notification-routes/${routeUid}`, { - method: "PATCH", - body: JSON.stringify(patch), - }), + mutationFn: ({ + routeUid, + patch, + }: { + routeUid: string; + patch: { enabled?: boolean }; + }) => + apiFetch( + `/api/v1/orgs/${org}/users/me/notification-routes/${routeUid}`, + { + method: "PATCH", + body: JSON.stringify(patch), + }, + ), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["notificationRoutes", org] }); }, @@ -4856,9 +5076,12 @@ export function usePatchNotificationRoute(org: string) { export function useTestNotificationRoute(org: string) { return useMutation({ mutationFn: (routeUid: string) => - apiFetch(`/api/v1/orgs/${org}/users/me/notification-routes/${routeUid}/test`, { - method: "POST", - }), + apiFetch( + `/api/v1/orgs/${org}/users/me/notification-routes/${routeUid}/test`, + { + method: "POST", + }, + ), }); } @@ -5158,7 +5381,11 @@ export function useBackgroundJob(org: string, uid: string, opts?: JobsScope) { } // useBackgroundJobChain fetches the ordered retry chain a job belongs to. -export function useBackgroundJobChain(org: string, uid: string, opts?: JobsScope) { +export function useBackgroundJobChain( + org: string, + uid: string, + opts?: JobsScope, +) { const allOrgs = opts?.allOrgs ?? false; return useQuery({ queryKey: ["backgroundJobChain", org, uid, { allOrgs }], @@ -5234,7 +5461,8 @@ export interface CreateMaintenanceWindowRequest { recurrenceEnd?: string | null; } -export type UpdateMaintenanceWindowRequest = Partial; +export type UpdateMaintenanceWindowRequest = + Partial; export interface SetMaintenanceWindowChecksRequest { checkUids: string[]; @@ -5265,7 +5493,9 @@ export function useMaintenanceWindow(org: string, uid: string) { return useQuery({ queryKey: ["maintenanceWindow", org, uid], queryFn: () => - apiFetch(`/api/v1/orgs/${org}/maintenance-windows/${uid}`), + apiFetch( + `/api/v1/orgs/${org}/maintenance-windows/${uid}`, + ), enabled: !!org && !!uid, }); } @@ -5312,7 +5542,9 @@ export function useUpdateMaintenanceWindow(org: string, uid: string) { ), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["maintenanceWindows", org] }); - queryClient.invalidateQueries({ queryKey: ["maintenanceWindow", org, uid] }); + queryClient.invalidateQueries({ + queryKey: ["maintenanceWindow", org, uid], + }); }, }); } @@ -5440,9 +5672,12 @@ export function useDeletePrivateRegion(org: string) { return useMutation({ mutationFn: (slug: string) => - apiFetch<{ status: string }>(`/api/v1/orgs/${org}/private-regions/${slug}`, { - method: "DELETE", - }), + apiFetch<{ status: string }>( + `/api/v1/orgs/${org}/private-regions/${slug}`, + { + method: "DELETE", + }, + ), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["private-regions", org] }); queryClient.invalidateQueries({ queryKey: ["regions", org] }); @@ -5457,7 +5692,9 @@ export function useAgents( return useQuery({ queryKey: ["agents", org], queryFn: async () => { - const response = await apiFetch<{ data?: AgentInfo[] }>(`/api/v1/orgs/${org}/agents`); + const response = await apiFetch<{ data?: AgentInfo[] }>( + `/api/v1/orgs/${org}/agents`, + ); return response.data || []; }, enabled: (options?.enabled ?? true) && !!org, @@ -5467,11 +5704,16 @@ export function useAgents( /** Fleet-wide agent list (superadmin only): every org agent plus every * platform-operated system agent, across all organizations. */ -export function useAllAgents(options?: { refetchInterval?: number; enabled?: boolean }) { +export function useAllAgents(options?: { + refetchInterval?: number; + enabled?: boolean; +}) { return useQuery({ queryKey: ["system-agents"], queryFn: async () => { - const response = await apiFetch<{ data?: AgentInfo[] }>(`/api/v1/system/agents`); + const response = await apiFetch<{ data?: AgentInfo[] }>( + `/api/v1/system/agents`, + ); return response.data || []; }, enabled: options?.enabled ?? true, @@ -5516,12 +5758,17 @@ export function useMintEnrollmentToken(org: string) { return useMutation({ mutationFn: (request: { regionSlug: string; expiresIn?: string }) => - apiFetch(`/api/v1/orgs/${org}/agent-enrollment-tokens`, { - method: "POST", - body: JSON.stringify(request), - }), + apiFetch( + `/api/v1/orgs/${org}/agent-enrollment-tokens`, + { + method: "POST", + body: JSON.stringify(request), + }, + ), onSuccess: () => { - queryClient.invalidateQueries({ queryKey: ["agent-enrollment-tokens", org] }); + queryClient.invalidateQueries({ + queryKey: ["agent-enrollment-tokens", org], + }); }, }); } @@ -5531,11 +5778,16 @@ export function useDeleteEnrollmentToken(org: string) { return useMutation({ mutationFn: (uid: string) => - apiFetch<{ status: string }>(`/api/v1/orgs/${org}/agent-enrollment-tokens/${uid}`, { - method: "DELETE", - }), + apiFetch<{ status: string }>( + `/api/v1/orgs/${org}/agent-enrollment-tokens/${uid}`, + { + method: "DELETE", + }, + ), onSuccess: () => { - queryClient.invalidateQueries({ queryKey: ["agent-enrollment-tokens", org] }); + queryClient.invalidateQueries({ + queryKey: ["agent-enrollment-tokens", org], + }); }, }); } @@ -5848,7 +6100,10 @@ export interface CreateSloRequest { export type UpdateSloRequest = Partial; -export function useSlos(org: string, params?: { checkUid?: string; enabled?: boolean }) { +export function useSlos( + org: string, + params?: { checkUid?: string; enabled?: boolean }, +) { return useQuery({ queryKey: ["slos", org, { checkUid: params?.checkUid }], queryFn: async () => { @@ -5875,7 +6130,8 @@ export function useSlo(org: string, uid: string) { export function useSloStatus(org: string, uid: string) { return useQuery({ queryKey: ["sloStatus", org, uid], - queryFn: () => apiFetch(`/api/v1/orgs/${org}/slos/${uid}/status`), + queryFn: () => + apiFetch(`/api/v1/orgs/${org}/slos/${uid}/status`), enabled: !!org && !!uid, }); } @@ -5913,7 +6169,8 @@ export interface SloBurndown { export function useSloBurndown(org: string, uid: string) { return useQuery({ queryKey: ["sloBurndown", org, uid], - queryFn: () => apiFetch(`/api/v1/orgs/${org}/slos/${uid}/burndown`), + queryFn: () => + apiFetch(`/api/v1/orgs/${org}/slos/${uid}/burndown`), enabled: !!org && !!uid, }); } @@ -6015,7 +6272,8 @@ export function useReportSchedules(org: string) { export function useReportSchedule(org: string, uid: string) { return useQuery({ queryKey: ["reportSchedule", org, uid], - queryFn: () => apiFetch(`/api/v1/orgs/${org}/report-schedules/${uid}`), + queryFn: () => + apiFetch(`/api/v1/orgs/${org}/report-schedules/${uid}`), enabled: !!org && !!uid, }); } @@ -6056,7 +6314,9 @@ export function useDeleteReportSchedule(org: string) { return useMutation({ mutationFn: (uid: string) => - apiFetch(`/api/v1/orgs/${org}/report-schedules/${uid}`, { method: "DELETE" }), + apiFetch(`/api/v1/orgs/${org}/report-schedules/${uid}`, { + method: "DELETE", + }), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["reportSchedules", org] }); }, diff --git a/web/dash0/src/components/checks/form/types/web.tsx b/web/dash0/src/components/checks/form/types/web.tsx index 23fe0ceab..526bf9819 100644 --- a/web/dash0/src/components/checks/form/types/web.tsx +++ b/web/dash0/src/components/checks/form/types/web.tsx @@ -1,6 +1,7 @@ import { cn } from "@/lib/utils"; import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; +import { Checkbox } from "@/components/ui/checkbox"; import { getFieldError } from "@/hooks/use-check-validation"; import type { CheckTypeModule } from "./index"; import type { CheckConfig, CheckTypeFieldsProps, FieldErrors } from "./common"; @@ -88,6 +89,7 @@ export interface BrowserState { url: string; waitSelector: string; keyword: string; + screenshot: boolean; } export const browserModule: CheckTypeModule = { @@ -96,12 +98,16 @@ export const browserModule: CheckTypeModule = { url: getConfigField(config, "url"), waitSelector: getConfigField(config, "waitSelector"), keyword: getConfigField(config, "keyword"), + screenshot: getConfigField(config, "screenshot") === "true", }), toConfig: (state) => { const cfg: CheckConfig = {}; if (state.url) cfg.url = state.url; if (state.waitSelector) cfg.waitSelector = state.waitSelector; if (state.keyword) cfg.keyword = state.keyword; + // Only sent when on: the backend default is off, and an explicit `false` + // would add noise to every browser check's stored config. + if (state.screenshot) cfg.screenshot = true; const errors: FieldErrors = state.url ? [] : [{ name: "url", message: "URL is required" }]; @@ -145,7 +151,8 @@ function BrowserFields({ data-testid="check-wait-selector-input" />

- CSS selector to wait for before checking. Leave empty to wait for body. + CSS selector to wait for before checking. Leave empty to wait for + body.

@@ -162,6 +169,23 @@ function BrowserFields({ Text to search for in the rendered page content.

+
+ +

+ Keeps a PNG of the page on the incident this check opens or reopens. + Taken just after the failure is detected, from the probing region — + evidence, not the failing frame itself. Off by default. +

+
); } diff --git a/web/dash0/src/locales/de/incidents.json b/web/dash0/src/locales/de/incidents.json index 9c65a276e..2da489d63 100644 --- a/web/dash0/src/locales/de/incidents.json +++ b/web/dash0/src/locales/de/incidents.json @@ -113,6 +113,14 @@ "truncated": "Body auf {{shown}} von {{total}} gekürzt.", "binary": "Body nicht gespeichert: binär oder kein gültiges UTF-8. Nur Metadaten — {{size}}, SHA-256 {{hash}}.", "unknownSize": "unbekannte Größe" + }, + "screenshot": { + "title": "Screenshot bei Fehlererkennung", + "description": "So sah die Seite aus, als dieser Check den Fehler meldete. Kurz NACH der Erkennung aufgenommen, aus einer Region — ein Hinweis, nicht das fehlerhafte Bild selbst.", + "alt": "Screenshot der Seite, aufgenommen beim Öffnen des Vorfalls", + "caption": "Aufgenommen {{capturedAt}} aus {{region}}, kurz nach der Fehlererkennung.", + "unknownTime": "zu unbekannter Zeit", + "unknownRegion": "einer unbekannten Region" } }, "timeline": { diff --git a/web/dash0/src/locales/en/incidents.json b/web/dash0/src/locales/en/incidents.json index 03cc9c264..973a01184 100644 --- a/web/dash0/src/locales/en/incidents.json +++ b/web/dash0/src/locales/en/incidents.json @@ -113,6 +113,14 @@ "truncated": "Body truncated to {{shown}} of {{total}}.", "binary": "Body not stored: it is binary or not valid UTF-8. Metadata only — {{size}}, SHA-256 {{hash}}.", "unknownSize": "an unknown size" + }, + "screenshot": { + "title": "Screenshot at failure detection", + "description": "What the page looked like when this check reported the failure. Captured a moment AFTER detection, from one region — treat it as evidence, not as the failing frame itself.", + "alt": "Screenshot of the page captured when the incident opened", + "caption": "Captured {{capturedAt}} from {{region}}, shortly after failure detection.", + "unknownTime": "at an unknown time", + "unknownRegion": "an unknown region" } }, "timeline": { diff --git a/web/dash0/src/locales/es/incidents.json b/web/dash0/src/locales/es/incidents.json index 4e7032d8f..79505f083 100644 --- a/web/dash0/src/locales/es/incidents.json +++ b/web/dash0/src/locales/es/incidents.json @@ -113,6 +113,14 @@ "truncated": "Cuerpo truncado a {{shown}} de {{total}}.", "binary": "Cuerpo no almacenado: es binario o UTF-8 no válido. Solo metadatos — {{size}}, SHA-256 {{hash}}.", "unknownSize": "un tamaño desconocido" + }, + "screenshot": { + "title": "Captura al detectar el fallo", + "description": "Cómo se veía la página cuando este check informó del fallo. Tomada justo DESPUÉS de la detección, desde una sola región: es una pista, no el fotograma exacto del fallo.", + "alt": "Captura de la página tomada al abrirse la incidencia", + "caption": "Capturada {{capturedAt}} desde {{region}}, poco después de detectar el fallo.", + "unknownTime": "a una hora desconocida", + "unknownRegion": "una región desconocida" } }, "timeline": { diff --git a/web/dash0/src/locales/fr/incidents.json b/web/dash0/src/locales/fr/incidents.json index a76181b26..ad50b773a 100644 --- a/web/dash0/src/locales/fr/incidents.json +++ b/web/dash0/src/locales/fr/incidents.json @@ -113,6 +113,14 @@ "truncated": "Corps tronqué à {{shown}} sur {{total}}.", "binary": "Corps non conservé : binaire ou UTF-8 invalide. Métadonnées seulement — {{size}}, SHA-256 {{hash}}.", "unknownSize": "une taille inconnue" + }, + "screenshot": { + "title": "Capture d'écran à la détection", + "description": "À quoi ressemblait la page quand ce check a signalé l'échec. Prise juste APRÈS la détection, depuis une seule région — c'est un indice, pas l'image exacte de l'échec.", + "alt": "Capture d'écran de la page prise à l'ouverture de l'incident", + "caption": "Capturée {{capturedAt}} depuis {{region}}, peu après la détection de l'échec.", + "unknownTime": "à une heure inconnue", + "unknownRegion": "une région inconnue" } }, "timeline": { diff --git a/web/dash0/src/routes/orgs/$org/design-reference.tsx b/web/dash0/src/routes/orgs/$org/design-reference.tsx index 8aa8603db..4d5424782 100644 --- a/web/dash0/src/routes/orgs/$org/design-reference.tsx +++ b/web/dash0/src/routes/orgs/$org/design-reference.tsx @@ -40,13 +40,22 @@ import { } from "lucide-react"; import { toast } from "sonner"; -import { EventTypeBadge, getEventTone } from "@/components/dashboard/event-display"; -import { CheckTypeBadge, CheckTypeIcon } from "@/components/shared/check-type-identity"; +import { + EventTypeBadge, + getEventTone, +} from "@/components/dashboard/event-display"; +import { + CheckTypeBadge, + CheckTypeIcon, +} from "@/components/shared/check-type-identity"; import { CheckMultiPicker } from "@/components/shared/check-multi-picker"; import { CheckGroupPicker } from "@/components/shared/check-group-picker"; import { RecipientsInput } from "@/components/shared/recipients-input"; import { TokenChipsInput } from "@/components/shared/token-chips-input"; -import { isValidStatusPattern, normalizeStatusPattern } from "@/lib/http-status"; +import { + isValidStatusPattern, + normalizeStatusPattern, +} from "@/lib/http-status"; import { CollapsibleCode, CopyableCode, @@ -94,12 +103,7 @@ import { import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; import { SegmentedControl } from "@/components/ui/segmented-control"; -import { - Card, - CardContent, - CardHeader, - CardTitle, -} from "@/components/ui/card"; +import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; import { Logo } from "@/components/ui/logo"; import { AuroraPanel } from "@/components/ui/aurora-panel"; import { @@ -119,7 +123,11 @@ import { DropdownMenuSeparator, DropdownMenuTrigger, } from "@/components/ui/dropdown-menu"; -import { Popover, PopoverContent, PopoverTrigger } from "@/components/ui/popover"; +import { + Popover, + PopoverContent, + PopoverTrigger, +} from "@/components/ui/popover"; import { Tooltip, TooltipContent, @@ -167,7 +175,9 @@ export const Route = createFileRoute("/orgs/$org/design-reference")({ // Computed once at module scope (not inside a component render) so the // "N ago" showcase below stays a pure render — see the LiveDurationAgo // example. -const RELATIVE_TIME_DEMO_SINCE = new Date(Date.now() - 5 * 60_000).toISOString(); +const RELATIVE_TIME_DEMO_SINCE = new Date( + Date.now() - 5 * 60_000, +).toISOString(); const TIME_AGO_DEMO_DATE = new Date(Date.now() - 46 * 60_000).toISOString(); const SECTIONS: { id: string; label: string }[] = [ @@ -237,6 +247,7 @@ function DesignReferencePage() { + @@ -265,13 +276,19 @@ function ConventionsSection() { >
-

Editing always changes the route

+

+ Editing always changes the route +

Editing an entity must navigate to a dedicated route, never open a modal dialog. Mirror the create flow:{" "} - /<resource>/new{" "} + + /<resource>/new + {" "} for creation,{" "} - /<resource>/$id/edit{" "} + + /<resource>/$id/edit + {" "} for editing. The edit route renders a full page and reuses the same form component as the create route.

@@ -287,40 +304,62 @@ function ConventionsSection() {

-

Row actions: icons, not menus

+

+ Row actions: icons, not menus +

In list/table rows, prefer two ghost icon buttons ( Pencil{" "} for edit,{" "} Trash2{" "} for delete with{" "} - text-destructive + + text-destructive + ) over a{" "} - DropdownMenu{" "} + + DropdownMenu + {" "} with a{" "} - MoreVertical{" "} + + MoreVertical + {" "} trigger. The Edit icon links to the edit route; the Delete icon opens an{" "} - AlertDialog + + AlertDialog + . Other per-row actions live on the edit page, not in the row.

-

Delete is always red, always a trash bin

+

+ Delete is always red, always a trash bin +

Every delete (or otherwise irreversible) action is rendered in red and paired with the{" "} Trash2{" "} (trash bin) icon — no exceptions. Use a{" "} - Button variant="destructive"{" "} + + Button variant="destructive" + {" "} for standalone/prominent buttons, an icon button with{" "} - text-destructive{" "} + + text-destructive + {" "} in row actions, and{" "} - text-destructive focus:text-destructive{" "} + + text-destructive focus:text-destructive + {" "} on the delete item inside a{" "} - DropdownMenu + + DropdownMenu + . Both colors resolve to the{" "} - --destructive{" "} + + --destructive + {" "} token so dark mode stays correct. The destructive red is reserved for destructive actions — never use it for a neutral or primary action, and never delete with a different icon or a muted color. @@ -466,7 +505,16 @@ function OverviewSection() {

- Tip: open web/dash0/src/routes/orgs/$org/design-reference.tsx when copying patterns — many examples here lift directly from real routes (e.g. checks.index.tsx) and the source file is the canonical reference. + Tip: open{" "} + + web/dash0/src/routes/orgs/$org/design-reference.tsx + {" "} + when copying patterns — many examples here lift directly from real + routes (e.g.{" "} + + checks.index.tsx + + ) and the source file is the canonical reference.

@@ -539,7 +587,9 @@ import { PageHeader } from "@/components/shared/page-header"; title="Page header" description="Every page opens with a page header — the page title plus its right-aligned actions. 'Page title' and 'page header' are the same surface, not two primitives. List and section pages render it with the boxed PageHeader component (@/components/shared/page-header); detail and edit pages compose the same header inline so it can carry a back arrow and per-record actions. Both patterns are documented here." > -

List & section pages: the PageHeader component

+

+ List & section pages: the PageHeader component +

Pass icon, title, an optional description, and right-aligned actions; it renders a rounded muted icon tile, a text-2xl font-semibold title, the @@ -565,15 +615,19 @@ import { PageHeader } from "@/components/shared/page-header";

- Notes: pass the same per-page Lucide icon you would have rendered - inline — PageHeader{" "} + Notes: pass the same per-page Lucide icon you would have rendered inline + —{" "} + PageHeader{" "} wraps it in the muted tile for you. Put the primary action(s) that used to sit in the header row (e.g.{" "} + New X, export/import, a refresh button) into the{" "} actions{" "} prop; leave filter/search toolbars on their own row below the header. - Add className="flex-wrap"{" "} + Add{" "} + + className="flex-wrap" + {" "} so actions wrap instead of overflowing on mobile. Pass{" "} docsHref{" "} when a genuinely relevant docs page exists — it renders a small{" "} @@ -583,34 +637,49 @@ import { PageHeader } from "@/components/shared/page-header"; Docs link {" "} - section below for the standalone primitive. The detail/edit-page - header — back arrow inside the right-aligned action cluster — is the - same surface for detail pages; it is documented just below. + section below for the standalone primitive. The detail/edit-page header + — back arrow inside the right-aligned action cluster — is the same + surface for detail pages; it is documented just below.

-

Detail & edit pages: title block + right-aligned action cluster (back arrow first)

+

+ Detail & edit pages: title block + right-aligned action cluster + (back arrow first) +

On detail/edit pages, compose a{" "} - flex items-start justify-between gap-3{" "} + + flex items-start justify-between gap-3 + {" "} row. The left is the title block — the page{" "} h1{" "} plus any subtitle/status — wrapped in{" "} - min-w-0 flex-1{" "} + + min-w-0 flex-1 + {" "} so it truncates instead of shoving the actions off-screen. The{" "} right is a{" "} - flex gap-2 shrink-0{" "} - cluster whose first child is the icon-only ghost back button, + + flex gap-2 shrink-0 + {" "} + cluster whose{" "} + first child is the icon-only ghost back button, followed by the page actions (View / Edit / Delete, Refresh, …). The back arrow is not on the far left — it leads the - right-aligned cluster. It is{" "} - always icon-only — never paired with a "Back" - label. Use{" "} - ArrowLeft{" "} + right-aligned cluster. It is always icon-only — never + paired with a "Back" label. Use{" "} + + ArrowLeft + {" "} with{" "} - variant="ghost" size="icon"{" "} + + variant="ghost" size="icon" + {" "} and an{" "} - aria-label. - A trailing Refresh button labels itself on desktop and collapses to + + aria-label + + . A trailing Refresh button labels itself on desktop and collapses to the icon below{" "} sm.

@@ -647,24 +716,39 @@ import { PageHeader } from "@/components/shared/page-header"; importLine={`
\n
\n

{title}

\n {subtitle &&

{subtitle}

}\n
\n
\n \n {/* One cluster = one button height. Don't mix size="sm" with the default. */}\n \n \n \n
\n
`} /> -

Detail & edit pages: collapse the action cluster into an overflow menu on mobile

+

+ Detail & edit pages: collapse the action cluster into an overflow + menu on mobile +

When a detail header carries more than two or three actions, the inline toolbar overflows on a phone. Keep only the icon-only ghost{" "} back button always visible; render the labeled action buttons in a{" "} - hidden md:flex{" "} + + hidden md:flex + {" "} cluster, and mirror every one of them as items inside a{" "} - md:hidden{" "} - DropdownMenu{" "} + + md:hidden + {" "} + + DropdownMenu + {" "} triggered by a{" "} - MoreVertical{" "} + + MoreVertical + {" "} (⋯) button. The delete item is{" "} - text-destructive focus:text-destructive{" "} + + text-destructive focus:text-destructive + {" "} with a{" "} - Trash2{" "} - icon, just like the inline destructive button. Drive any confirm - dialog from controlled state so it opens from either surface. + + Trash2 + {" "} + icon, just like the inline destructive button. Drive any confirm dialog + from controlled state so it opens from either surface.

- @@ -722,28 +810,37 @@ import { PageHeader } from "@/components/shared/page-header"; importLine={`
\n \n
\n {/* labeled action buttons */}\n
\n
\n \n \n \n \n \n {/* mirror each action; delete = text-destructive */}\n \n \n
\n
`} /> -

Detail & edit pages: stack the action toolbar on its own row (action-dense headers)

+

+ Detail & edit pages: stack the action toolbar on its own row + (action-dense headers) +

When a detail header carries more than ~three actions{" "} (e.g. the check detail page: back + Edit, Enable/Disable, Clone, Badges, Refresh, Delete) the labeled toolbar and a long title fight for the same row — even on a wide desktop. Instead of shrinking the buttons or hiding - them behind an overflow menu, drop the toolbar onto its own row. Make the - outer wrapper a{" "} - flex flex-col gap-3{" "} + them behind an overflow menu, drop the toolbar onto its own row. Make + the outer wrapper a{" "} + + flex flex-col gap-3 + {" "} column: the title block (still wrapped in{" "} - min-w-0 flex-1{" "} + + min-w-0 flex-1 + {" "} so the{" "} h1{" "} truncates) takes the first row, then the action cluster — back arrow leading, as ever — sits on a second row wrapped in{" "} - flex flex-wrap items-center justify-end gap-2. - It is right-aligned and wraps across lines on a narrow phone rather than - overflowing. The per-button responsive behaviour (icon-only below{" "} + + flex flex-wrap items-center justify-end gap-2 + + . It is right-aligned and wraps across lines on a narrow phone rather + than overflowing. The per-button responsive behaviour (icon-only below{" "} lg, icon + label at{" "} - lg+) is - unchanged — only the wrappers move. + lg+) + is unchanged — only the wrappers move.

h-7 w-7{" "} icon is no longer canonical. Do not reach for it on new pages — and migrate any remaining inline headers to{" "} - PageHeader{" "} + + PageHeader + {" "} so the app stays consistent.

@@ -854,31 +953,31 @@ function ButtonPlacementSection() { PageHeader actions: primary, page-level actions only

- The actions{" "} - slot on PageHeader{" "} + The{" "} + actions{" "} + slot on{" "} + PageHeader{" "} is reserved for the page's primary action — typically a single "New <resource>" create button, plus at most one secondary page-level action (export/import, a scope toggle). It is{" "} not a catch-all toolbar: a page that has a - search/filter toolbar row below the header does not put Refresh in - the header — Refresh moves into that row (see below). The one - exception is a page with{" "} - no search toolbar at all — on-call has no search - field, so it keeps Refresh in the header, to the left of the primary - button. + search/filter toolbar row below the header does not put Refresh in the + header — Refresh moves into that row (see below). The one exception is a + page with no search toolbar at all — on-call has no + search field, so it keeps Refresh in the header, to the left of the + primary button.

A third case reads like a toolbar but isn't one:{" "} filters scoped to a single table or card, rendered - inside that card's own header next to its title (e.g. a - source-type select next to a "Scans" card title, or a - per-tab status filter above one tab's table). That's card-level - chrome, not a page-level toolbar row — the row this section means - sits directly under PageHeader, - outside and above any card. A page whose only filtering controls are - card-nested like this has, from the page's point of view, no - toolbar at all — Refresh stays in the header, per the exception - above. + inside that card's own header next to its title (e.g. a source-type + select next to a "Scans" card title, or a per-tab status + filter above one tab's table). That's card-level chrome, not a + page-level toolbar row — the row this section means sits directly under{" "} + PageHeader + , outside and above any card. A page whose only filtering controls are + card-nested like this has, from the page's point of view, no toolbar at + all — Refresh stays in the header, per the exception above.

@@ -891,8 +990,8 @@ function ButtonPlacementSection() { flex flex-wrap items-center gap-4 {" "} row below the header. Refresh sits to the{" "} - right of the search input (after any filter - selects, if the row has them) — mirror{" "} + right of the search input (after any filter selects, if + the row has them) — mirror{" "} integrations.index.tsx {" "} @@ -962,12 +1061,15 @@ function DocsLinkSection() { importLine={importLine} />

- Renders a BookOpen{" "} + Renders a{" "} + BookOpen{" "} icon in a ~h-8 w-8 ghost button, with a "Documentation" tooltip and{" "} - aria-label. The{" "} - href is a - same-origin relative path (e.g.{" "} - /docs/features/... + aria-label + . The href{" "} + is a same-origin relative path (e.g.{" "} + + /docs/features/... + ), opened with{" "} target="_blank" rel="noopener" @@ -1025,24 +1127,43 @@ if (isFooBar) { >

- Look at the header bar above the sidebar trigger — the breadcrumb shows{" "} - Design Reference{" "} + Look at the header bar above the sidebar trigger — the breadcrumb + shows{" "} + + Design Reference + {" "} with the Palette icon. That branch was added alongside the others in{" "} - $org.tsx. + $org.tsx + .

-

Context-driven breadcrumbs with ?from=

+

+ Context-driven breadcrumbs with{" "} + ?from= +

- When a detail page can be reached from multiple parent surfaces, encode the - navigation context in a ?from=type:uid search - param (e.g. ?from=incident:abc123 or{" "} - ?from=integration:xyz). - The breadcrumb reads the param and renders the matching parent chain. Label resolution - uses the query cache — no extra fetch. The notification detail route - (/orgs/$org/notifications/$notificationUid) is - the canonical example of this pattern. + When a detail page can be reached from multiple parent surfaces, + encode the navigation context in a{" "} + + ?from=type:uid + {" "} + search param (e.g.{" "} + + ?from=incident:abc123 + {" "} + or{" "} + + ?from=integration:xyz + + ). The breadcrumb reads the param and renders the matching parent + chain. Label resolution uses the query cache — no extra fetch. The + notification detail route ( + + /orgs/$org/notifications/$notificationUid + + ) is the canonical example of this pattern.

@@ -1050,36 +1171,99 @@ if (isFooBar) { ); } -const COLOR_TOKENS: { name: string; varName: string; description?: string }[] = [ - { name: "primary", varName: "--primary", description: "Action color (buttons, links, focus rings)" }, - { name: "brand", varName: "--brand", description: "Logo/marketing chrome — never an interactive affordance" }, - { name: "brand-muted", varName: "--brand-muted", description: "Soft brand wash for headers / hero strips" }, - { name: "destructive", varName: "--destructive", description: "Delete / irreversible action confirms" }, - { name: "accent", varName: "--accent", description: "Hover/highlight surface" }, - { - name: "control", - varName: "--control", - description: - "Fill of interactive form surfaces (input, textarea, select trigger, outline button). Light raises it to white; dark recesses it below --card. Use bg-control — bg-background is reserved for the page shell, the sidebar rail and the switch thumb.", - }, - { - name: "input", - varName: "--input", - description: - "BORDER color of form controls, consumed as border-input — not a fill. The fill is --control; there is deliberately no --input-background.", - }, - { name: "muted-foreground", varName: "--muted-foreground", description: "Secondary text" }, - { name: "status-ok", varName: "--status-ok", description: "Healthy / passing — swatch color (dots, bars, soft tints)" }, - { name: "status-ok-foreground", varName: "--status-ok-foreground", description: "Text on a soft status-ok tint (badges, alerts)" }, - { name: "status-warning", varName: "--status-warning", description: "Degraded — swatch color" }, - { name: "status-warning-foreground", varName: "--status-warning-foreground", description: "Text on a soft status-warning tint" }, - { name: "status-error", varName: "--status-error", description: "Failing — swatch color" }, - { name: "status-error-foreground", varName: "--status-error-foreground", description: "Text on a soft status-error tint" }, -]; +const COLOR_TOKENS: { name: string; varName: string; description?: string }[] = + [ + { + name: "primary", + varName: "--primary", + description: "Action color (buttons, links, focus rings)", + }, + { + name: "brand", + varName: "--brand", + description: "Logo/marketing chrome — never an interactive affordance", + }, + { + name: "brand-muted", + varName: "--brand-muted", + description: "Soft brand wash for headers / hero strips", + }, + { + name: "destructive", + varName: "--destructive", + description: "Delete / irreversible action confirms", + }, + { + name: "accent", + varName: "--accent", + description: "Hover/highlight surface", + }, + { + name: "control", + varName: "--control", + description: + "Fill of interactive form surfaces (input, textarea, select trigger, outline button). Light raises it to white; dark recesses it below --card. Use bg-control — bg-background is reserved for the page shell, the sidebar rail and the switch thumb.", + }, + { + name: "input", + varName: "--input", + description: + "BORDER color of form controls, consumed as border-input — not a fill. The fill is --control; there is deliberately no --input-background.", + }, + { + name: "muted-foreground", + varName: "--muted-foreground", + description: "Secondary text", + }, + { + name: "status-ok", + varName: "--status-ok", + description: "Healthy / passing — swatch color (dots, bars, soft tints)", + }, + { + name: "status-ok-foreground", + varName: "--status-ok-foreground", + description: "Text on a soft status-ok tint (badges, alerts)", + }, + { + name: "status-warning", + varName: "--status-warning", + description: "Degraded — swatch color", + }, + { + name: "status-warning-foreground", + varName: "--status-warning-foreground", + description: "Text on a soft status-warning tint", + }, + { + name: "status-error", + varName: "--status-error", + description: "Failing — swatch color", + }, + { + name: "status-error-foreground", + varName: "--status-error-foreground", + description: "Text on a soft status-error tint", + }, + ]; -const CHART_TOKENS = ["--chart-1", "--chart-2", "--chart-3", "--chart-4", "--chart-5"]; +const CHART_TOKENS = [ + "--chart-1", + "--chart-2", + "--chart-3", + "--chart-4", + "--chart-5", +]; -function Swatch({ varName, label, description }: { varName: string; label: string; description?: string }) { +function Swatch({ + varName, + label, + description, +}: { + varName: string; + label: string; + description?: string; +}) { return (
-

Action buttons (icon + label, mobile collapses to icon)

+

+ Action buttons (icon + label, mobile collapses to icon) +

Pair every action with a recognisable icon and a one-word verb. Use{" "} - Save (floppy disk) for - save, Trash2 for - delete, Pencil for - edit, and RotateCw{" "} + + Save + {" "} + (floppy disk) for save,{" "} + + Trash2 + {" "} + for delete,{" "} + + Pencil + {" "} + for edit, and{" "} + + RotateCw + {" "} for reload. Below the{" "} - sm breakpoint, the - label collapses and only the icon remains: wrap the label in{" "} + sm{" "} + breakpoint, the label collapses and only the icon remains: wrap the + label in{" "} <span className="hidden sm:inline"> {" "} and pair every button with an{" "} - aria-label so screen - readers still announce the action when the text is gone. Resize your viewport to verify. + + aria-label + {" "} + so screen readers still announce the action when the text is gone. + Resize your viewport to verify.

\n \n Save\n`} /> -

Header refresh button (icon-only on mobile)

+

+ Header refresh button (icon-only on mobile) +

The canonical list/detail header refresh control. An{" "} - outline button - wrapping{" "} - RefreshCw that shows - the word Refresh from the{" "} - sm breakpoint up and - collapses to icon-only below it. Drop{" "} - size="icon"{" "} + + outline + {" "} + button wrapping{" "} + + RefreshCw + {" "} + that shows the word Refresh from the{" "} + sm{" "} + breakpoint up and collapses to icon-only below it. Drop{" "} + + size="icon" + {" "} so the button sizes to its content, put{" "} - sm:mr-2 on the icon so - the gap only appears with the label, and keep an{" "} - aria-label for the - icon-only state. Add{" "} - animate-spin while the - query is refetching. (Use the localized{" "} - common:refresh string - on real pages.) + + sm:mr-2 + {" "} + on the icon so the gap only appears with the label, and keep an{" "} + + aria-label + {" "} + for the icon-only state. Add{" "} + + animate-spin + {" "} + while the query is refetching. (Use the localized{" "} + + common:refresh + {" "} + string on real pages.)

-

Button with "Last used" badge

+

+ Button with "Last used" badge +

The promoted-slot pattern used on the login page: a full-width action button carries an inline{" "} - secondary{" "} + + secondary + {" "} Badge marking the option a returning user picked last. Keep the badge last in the button and spaced with{" "} - ml-2. + + ml-2 + + .

The small dot rendered beside a check name (listing) and the detail header. Colours come from the single source of truth{" "} - statusStyle(){" "} + + statusStyle() + {" "} so the dot always matches the{" "} - StatusBadge{" "} + + StatusBadge + {" "} beside it. A disabled check ( - enabled === false) - renders a neutral grey dot that overrides the last/live status colour, so a - paused check no longer reads as "healthy & live". Pass a localized{" "} - title (the - translated "Disabled") for the tooltip and accessible label. + + enabled === false + + ) renders a neutral grey dot that overrides the last/live status + colour, so a paused check no longer reads as "healthy & live". Pass a + localized{" "} + + title + {" "} + (the translated "Disabled") for the tooltip and accessible label.

Unknown - Disabled + {" "} + Disabled } @@ -1282,17 +1516,21 @@ function ButtonsBadgesSection() {

IPv6 capability badge

- What a region's live workers report about their IPv6 egress - (spec 2026-08-15-11). Three states, three renderings —{" "} - unknown means - "not reported yet" (no live worker, or an older agent) and must{" "} + What a region's live workers report about their IPv6 + egress (spec 2026-08-15-11). Three states, three renderings —{" "} + + unknown + {" "} + means "not reported yet" (no live worker, or an older agent) and must{" "} never be rendered as{" "} - no. The value is a - hint only: it never hides, disables or filters a region, because the run-time egress - probe is the authority. Use{" "} - hideUnknown on dense - inline surfaces (the region picker) — "no" always renders, so a missing badge can - never be misread as a negative. + no. + The value is a hint only: it never hides, disables or filters a + region, because the run-time egress probe is the authority. Use{" "} + + hideUnknown + {" "} + on dense inline surfaces (the region picker) — "no" always renders, so + a missing badge can never be misread as a negative.

`} /> -

SLO state chip & error-budget meter

+

+ SLO state chip & error-budget meter +

- The four objective states (spec 2026-08-20-01) and the remaining-budget meter that - accompanies them.{" "} - unknown is the - no-data state and is deliberately neutral grey, never green: an - objective over a window with no probes has null attainment, and rendering that as a - healthy 100% would turn "we were not watching" into "everything was fine". Attainment - itself follows the same rule — render{" "} - null as a dash via{" "} - formatAttainment. - The meter is clamped to [0, 1] by{" "} - budgetRemainingFraction, - while the label keeps the sign so an overspent budget reads as{" "} - -12m 30s. + The four objective states (spec 2026-08-20-01) and the + remaining-budget meter that accompanies them.{" "} + + unknown + {" "} + is the no-data state and is deliberately{" "} + neutral grey, never green: an objective over a window + with no probes has null attainment, and rendering that as a healthy + 100% would turn "we were not watching" into "everything was fine". + Attainment itself follows the same rule — render{" "} + + null + {" "} + as a dash via{" "} + + formatAttainment + + . The meter is clamped to [0, 1] by{" "} + + budgetRemainingFraction + + , while the label keeps the sign so an overspent budget reads as{" "} + + -12m 30s + + .

Healthy At risk - Breached + + Breached + No data
-
+

- {formatBudgetSeconds(990)} of {formatBudgetSeconds(2592)} remaining + {formatBudgetSeconds(990)} of {formatBudgetSeconds(2592)}{" "} + remaining

@@ -1345,15 +1604,19 @@ function ButtonsBadgesSection() {

Error-budget burn-down chart

- The objective detail page's burn-down (spec 2026-08-20-01): remaining budget over the - current window against the straight "ideal" line that spends it exactly. Two - conventions are load-bearing. The actual series is{" "} - never clamped at zero — an overspent budget dips below the dashed - destructive reference line, and flattening it there would hide the magnitude of a - breach. And every DATA dot renders a{" "} - <title>, which - recharts' hover activeDot does not, so an E2E can count{" "} - circle:has(title){" "} + The objective detail page's burn-down (spec 2026-08-20-01): remaining + budget over the current window against the straight "ideal" line that + spends it exactly. Two conventions are load-bearing. The actual series + is never clamped at zero — an overspent budget dips + below the dashed destructive reference line, and flattening it there + would hide the magnitude of a breach. And every DATA dot renders a{" "} + + <title> + + , which recharts' hover activeDot does not, so an E2E can count{" "} + + circle:has(title) + {" "} deterministically.

Agent version cell

- Compares an agent's self-reported build version against this server's own (spec - 2026-08-19-07). Unlike the IPv6 badge above, this is not a stored three-state value — - it is a comparison computed at read time from two inputs, each already two-state: the - agent's version (null{" "} + Compares an agent's self-reported build version against this server's + own (spec 2026-08-19-07). Unlike the IPv6 badge above, this is not a + stored three-state value — it is a comparison computed at read time + from two inputs, each already two-state: the agent's version ( + + null + {" "} = never reported) and the server's own (from{" "} - useVersion()). - Matching and unknown render as plain text; only a genuine mismatch gets the amber - "Drifted" badge — null{" "} - must never render as drifted, or an agent that simply predates this - feature would look broken. + + useVersion() + + ). Matching and unknown render as plain text; only a genuine mismatch + gets the amber "Drifted" badge —{" "} + + null + {" "} + must never render as drifted, or an agent that simply + predates this feature would look broken.

Check group status header

The checks index ( - checks.index.tsx - ) buckets checks by checkGroupUid into - collapsible sections. The header always reuses the same{" "} - StatusBadge as check rows — no - new colors — next to a compact member summary derived from the group's{" "} - memberStatusCounts: the "N/N up" - form when every counted member is up (the collapse-eligible case), otherwise - severity-ordered parts like "1 down · 3 up". A group defaults to collapsed only - when its rollup status is up; any - manual toggle overrides the default and persists per org in localStorage. + + checks.index.tsx + + ) buckets checks by{" "} + + checkGroupUid + {" "} + into collapsible sections. The header always reuses the same{" "} + + StatusBadge + {" "} + as check rows — no new colors — next to a compact member summary + derived from the group's{" "} + + memberStatusCounts + + : the "N/N up" form when every counted member is up (the + collapse-eligible case), otherwise severity-ordered parts like "1 down + · 3 up". A group defaults to collapsed only when its rollup status is{" "} + up; + any manual toggle overrides the default and persists per org in + localStorage.

prod-eu-west 4/4 up - 4 + + 4 +
prod-us-east - 1 down · 3 up - 4 + + 1 down · 3 up + + + 4 +
} @@ -1427,15 +1716,28 @@ function ButtonsBadgesSection() {

Live connection status dot

- Passive indicator for the org live-updates WebSocket, mounted in the sidebar - footer utility row alongside LanguageSwitcher{" "} - and ThemeToggle. Reads{" "} - useLiveConnectionStatus(){" "} - from LiveEventsContext — green - while streaming, red while dropped/retrying, grey while connecting or when - realtime is disabled/forbidden server-side (never red for a by-design - non-live state). Purely informational: no click action, no popover. The dot - below reflects this page's actual live connection. + Passive indicator for the org live-updates WebSocket, mounted in the + sidebar footer utility row alongside{" "} + + LanguageSwitcher + {" "} + and{" "} + + ThemeToggle + + . Reads{" "} + + useLiveConnectionStatus() + {" "} + from{" "} + + LiveEventsContext + {" "} + — green while streaming, red while dropped/retrying, grey while + connecting or when realtime is disabled/forbidden server-side (never + red for a by-design non-live state). Purely informational: no click + action, no popover. The dot below reflects this page's actual live + connection.

Live-ticking "N ago" text for a timestamp — used by the check summary cards' "last checked" line and the private locations agents table's - "Last seen" column. Ticks on its own 1s interval (cleared on - unmount) via{" "} - LiveDurationAgo, - formats with the shared{" "} - formatDuration(){" "} - (caps at Xd Yh Zm), - and wraps the result in the translated{" "} - checks:detail.summary.ago{" "} - template so FR/DE/ES render correctly instead of a hard-coded "ago" suffix. Pair it with a{" "} - title carrying the full - local timestamp so the exact moment stays reachable on hover, and keep a separate - "never" fallback for a null/undefined timestamp — the component itself has no empty state. + "Last seen" column. Ticks on its own 1s interval (cleared on unmount) + via{" "} + + LiveDurationAgo + + , formats with the shared{" "} + + formatDuration() + {" "} + (caps at{" "} + + Xd Yh Zm + + ), and wraps the result in the translated{" "} + + checks:detail.summary.ago + {" "} + template so FR/DE/ES render correctly instead of a hard-coded "ago" + suffix. Pair it with a{" "} + + title + {" "} + carrying the full local timestamp so the exact moment stays reachable + on hover, and keep a separate "never" fallback for a null/undefined + timestamp — the component itself has no empty state.

\n \n \n) : (\n t("privateLocations.agents.never", "never")\n)}`} /> -

TimeAgo (hover/tap + click-to-copy)

+

+ TimeAgo (hover/tap + click-to-copy) +

- The incidents list, incident detail (timeline, comments, header) and jobs - pages' timestamp. Unlike LiveDurationAgo above, - the absolute time isn't just a passive title{" "} + The incidents list, incident detail (timeline, comments, header) and + jobs pages' timestamp. Unlike{" "} + + LiveDurationAgo + {" "} + above, the absolute time isn't just a passive{" "} + + title + {" "} — hovering (or tapping, on touch) opens a{" "} - Tooltip with both local and - UTC time, and clicking copies the UTC time as ISO 8601 (2026-08-14T09:31:07Z) - to the clipboard — the format that pastes cleanly into a log query. All instances - share a single 30s re-render timer (not one setInterval per - row) so long-lived tabs don't drift. + + Tooltip + {" "} + with both local and UTC time, and clicking copies the UTC time as + ISO 8601 ( + + 2026-08-14T09:31:07Z + + ) to the clipboard — the format that pastes cleanly into a log query. + All instances share a single 30s re-render timer (not one{" "} + + setInterval + {" "} + per row) so long-lived tabs don't drift.

- tooltip (default, - dense lists): + + tooltip + {" "} + (default, dense lists):{" "} + } importLine={`import { TimeAgo } from "@/components/ui/time-ago";\n\n// Dense lists (incidents index, jobs): compact relative text, absolute\n// time behind hover/tap.\n{incident.startedAt ? : "-"}`} @@ -1499,9 +1837,15 @@ function ButtonsBadgesSection() { - inline (incident - detail — several timestamps compared at once):{" "} - + + inline + {" "} + (incident detail — several timestamps compared at once):{" "} + } importLine={`import { TimeAgo } from "@/components/ui/time-ago";\n\n// Incident detail (timeline, comments, header): absolute time shown\n// inline instead of hidden behind hover.\n`} @@ -1509,16 +1853,39 @@ function ButtonsBadgesSection() {

Session card

- The account Sessions page (/orgs/$org/account/sessions) lists - login/refresh-token sessions distinctly from API tokens. Each row: a device - icon from parseUserAgent().device{" "} - (Smartphone /{" "} - Tablet /{" "} - Monitor), a{" "} - border-primary accent - + "Current session" badge on the caller's own row, a login-method badge, the - raw user agent as muted mono text, and a destructive ghost icon revoke button. Static mock - below — the real page is data-driven via useSessions(org). + The account Sessions page ( + + /orgs/$org/account/sessions + + ) lists login/refresh-token sessions distinctly from API tokens. Each + row: a device icon from{" "} + + parseUserAgent().device + {" "} + ( + + Smartphone + {" "} + /{" "} + + Tablet + {" "} + /{" "} + + Monitor + + ), a{" "} + + border-primary + {" "} + accent + "Current session" badge on the caller's own + row, a login-method badge, the raw user agent as muted mono text, and + a destructive ghost icon revoke button. Static mock below — the real + page is data-driven via{" "} + + useSessions(org) + + .

Chrome 128 on macOS - Current session + + Current session + password
@@ -1541,7 +1910,12 @@ function ButtonsBadgesSection() {
- @@ -1559,13 +1933,21 @@ function ButtonsBadgesSection() {

Organization row

- The account Organizations page (/orgs/$org/account/organizations) reuses - the same current-marker card as the session list above: a{" "} - border-primary accent + - "Current" badge on the active org's row. The logo box falls back to the{" "} - Building icon exactly - like the sidebar org switcher, and every other row gets an outline Switch button instead - of a revoke action. + The account Organizations page ( + + /orgs/$org/account/organizations + + ) reuses the same current-marker card as the session list above: a{" "} + + border-primary + {" "} + accent + "Current" badge on the active org's row. The + logo box falls back to the{" "} + + Building + {" "} + icon exactly like the sidebar org switcher, and every other row gets + an outline Switch button instead of a revoke action.

Acme Corp Current -
acme · Role: owner
+
+ acme · Role: owner +
@@ -1594,10 +1978,14 @@ function ButtonsBadgesSection() {
Other Org -
other-org · Role: admin
+
+ other-org · Role: admin +
- + @@ -1605,31 +1993,44 @@ function ButtonsBadgesSection() { importLine={`import { Card, CardContent } from "@/components/ui/card";\nimport { Badge } from "@/components/ui/badge";\nimport { Building } from "lucide-react";\n\n\n \n {/* logo/Building fallback + name + slug + role, "Current" badge or a Switch button */}\n \n`} /> -

Secondary-path divider + sub-card

+

+ Secondary-path divider + sub-card +

When a page has one primary action and a clearly subordinate alternative path (e.g. the empty-state onboarding hero's "let AI set everything up" MCP link under the quick-create form), separate them with a hairline "or" divider followed by a bordered sub-card: icon + title/description on the left, an - outline Button asChild{" "} - CTA on the right. Stacks vertically below sm. + outline{" "} + + Button asChild + {" "} + CTA on the right. Stacks vertically below{" "} + sm. The divider is decorative — mark it{" "} - aria-hidden. + + aria-hidden + + .