Public: anyone can read this; the crew can change it.
Each sentinel detector declares a kind, so a security finding lands as a high bug on its clock #8986
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/8969-each-sentinel-detector-declares-a-kind-s → main 1 commit fork
5 files
changed +93
−29
docs/SENTINEL.md +8 −4
| @@ -31,15 +31,19 @@ and nobody has to sort it: | ||
| 31 | 31 | |
| 32 | 32 | | Kind | Labels | Clock | |
| 33 | 33 | | --- | --- | --- | |
| 34 | | `security` (a hand-installed trigger) | `bug`, `high` | 4 h | | |
| 34 | | `security` (a hand-installed trigger; a personal page not sent to sign-in) | `bug`, `high` | 4 h | | |
| 35 | | `broken-route` (a public page, link or documented route that answers 404 or 5xx) | `bug`, `high` | 4 h | | |
| 35 | 36 | | `defect` (every other finding, and the performance watcher's slow routes) | `bug` | 48 h | |
| 37 | | `cosmetic` (a link that lands on `/` and is not Home) | `polish` | none | | |
| 36 | 38 | |
| 37 | 39 | A label the repository lacks is made (`src/hunt/label.ts`, shared by the |
| 38 | 40 | sentinel and `src/perf/duty.ts`); labels already on the issue stay, and an |
| 39 | 41 | issue adopted after its record was lost is labelled again. A budget breach |
| 40 | also gets `P0 Trust`. A kind joins the table with the first detector that | |
| 41 | files it: the route hunters (`src/hunt/door.ts`, `links.ts`, `contract.ts`) | |
| 42 | file nothing yet, so there is no `broken` or `polish` kind. | |
| 42 | also gets `P0 Trust`. Every detector declares its kind, and the type check | |
| 43 | refuses one that does not (#8969): a sentinel `Finding` requires `kind`, and | |
| 44 | the route hunters (`src/hunt/door.ts`, `links.ts`, `contract.ts`) and the | |
| 45 | performance watcher name their detector in `DETECTOR_KINDS` | |
| 46 | (`src/hunt/finding.ts`), which `found` reads the kind from. | |
| 43 | 47 | |
| 44 | 48 | **What it shows.** To a site admin only: the board's forge strip and the |
| 45 | 49 | terminal palette's `Sentinel` command (as a toast) show the count of open findings and the |
src/hunt/finding.ts +46 −20
| @@ -1,7 +1,50 @@ | ||
| 1 | 1 | /** What a detector says: plain data, one issue per `key` (the sentinel's rule). */ |
| 2 | 2 | |
| 3 | /** | |
| 4 | * What a finding is, so it is filed already sorted (#8856): a security hole or | |
| 5 | * a route that answers broken is a `high` bug (the 4 h clock in | |
| 6 | * `flow/bugs.ts`), any other defect a bug (48 h), and a cosmetic one `polish`. | |
| 7 | */ | |
| 8 | export type FindingKind = "security" | "broken-route" | "defect" | "cosmetic"; | |
| 9 | ||
| 10 | export const KIND_LABELS: Readonly<Record<FindingKind, readonly string[]>> = { | |
| 11 | security: ["bug", "high"], | |
| 12 | "broken-route": ["bug", "high"], | |
| 13 | defect: ["bug"], | |
| 14 | cosmetic: ["polish"], | |
| 15 | }; | |
| 16 | ||
| 17 | /** Each label a kind is filed with, and the colour it is made in when the repository has none by that name. */ | |
| 18 | export const LABEL_COLORS: Readonly<Record<string, string>> = { | |
| 19 | bug: "d73a4a", | |
| 20 | high: "b60205", | |
| 21 | polish: "c5def5", | |
| 22 | }; | |
| 23 | ||
| 24 | /** | |
| 25 | * Every detector that files through `found`, and its kind (#8969). A detector | |
| 26 | * not in this table is not a `Detector`, so `found` refuses it at the type check. | |
| 27 | */ | |
| 28 | export const DETECTOR_KINDS = { | |
| 29 | // An anonymous visitor who is not sent to sign-in may be reading a person's page. | |
| 30 | "door.signin-redirect": "security", | |
| 31 | "door.signin-next": "defect", | |
| 32 | "door.public-page": "broken-route", | |
| 33 | "door.unknown-404": "defect", | |
| 34 | "links.status": "broken-route", | |
| 35 | "links.lands-home": "cosmetic", | |
| 36 | "contract.openapi": "defect", | |
| 37 | "contract.no-500": "broken-route", | |
| 38 | "contract.documented": "defect", | |
| 39 | "performance-watcher": "defect", | |
| 40 | } as const satisfies Record<string, FindingKind>; | |
| 41 | ||
| 42 | export type Detector = keyof typeof DETECTOR_KINDS; | |
| 43 | ||
| 3 | 44 | export interface Finding { |
| 4 | detector: string; | |
| 45 | detector: Detector; | |
| 46 | /** How it is filed: its labels, and so its clock. */ | |
| 47 | kind: FindingKind; | |
| 5 | 48 | /** Stable across runs: the same defect has the same key, so it files once. */ |
| 6 | 49 | key: string; |
| 7 | 50 | title: string; |
| @@ -9,8 +52,9 @@ export interface Finding { | ||
| 9 | 52 | evidence: string; |
| 10 | 53 | } |
| 11 | 54 | |
| 12 | export const found = (detector: string, subject: string, title: string, evidence: string): Finding => ({ | |
| 55 | export const found = (detector: Detector, subject: string, title: string, evidence: string): Finding => ({ | |
| 13 | 56 | detector, |
| 57 | kind: DETECTOR_KINDS[detector], | |
| 14 | 58 | key: `${detector}:${subject}`, |
| 15 | 59 | title, |
| 16 | 60 | evidence, |
| @@ -18,21 +62,3 @@ export const found = (detector: string, subject: string, title: string, evidence | ||
| 18 | 62 | |
| 19 | 63 | export const seenLine = (path: string, status: number, location?: string | null): string => |
| 20 | 64 | `GET ${path} → ${status}${location ? ` → ${location}` : ""}`; |
| 21 | ||
| 22 | /** | |
| 23 | * What a finding is, so it is filed already sorted (#8856): a security hole is | |
| 24 | * a `high` bug (the 4 h clock in `flow/bugs.ts`), any other defect a bug (48 h). | |
| 25 | * A kind joins this list with the first detector that files it. | |
| 26 | */ | |
| 27 | export type FindingKind = "security" | "defect"; | |
| 28 | ||
| 29 | export const KIND_LABELS: Readonly<Record<FindingKind, readonly string[]>> = { | |
| 30 | security: ["bug", "high"], | |
| 31 | defect: ["bug"], | |
| 32 | }; | |
| 33 | ||
| 34 | /** Each label a kind is filed with, and the colour it is made in when the repository has none by that name. */ | |
| 35 | export const LABEL_COLORS: Readonly<Record<string, string>> = { | |
| 36 | bug: "d73a4a", | |
| 37 | high: "b60205", | |
| 38 | }; |
src/perf/duty.ts +3 −4
| @@ -3,7 +3,8 @@ | ||
| 3 | 3 | * file each new finding, of a route or of a git subcommand (#8158), as one |
| 4 | 4 | * issue in the forge's own repository, opened by its owner. A title already |
| 5 | 5 | * open is not filed twice, and neither is a key this process has filed. A |
| 6 | * slow route or subcommand is a defect: filed as a `bug`, on its clock (#8856). | |
| 6 | * slow route or subcommand is a defect: filed with its kind's labels, a `bug` | |
| 7 | * on its clock (#8856). | |
| 7 | 8 | */ |
| 8 | 9 | |
| 9 | 10 | import type { App } from "../app.ts"; |
| @@ -11,14 +12,12 @@ import { openIssue } from "../actions/issues.ts"; | ||
| 11 | 12 | import { getUserById } from "../core/users.ts"; |
| 12 | 13 | import { getRepo } from "../core/repos.ts"; |
| 13 | 14 | import { listIssues } from "../core/issues.ts"; |
| 14 | import type { FindingKind } from "../hunt/finding.ts"; | |
| 15 | 15 | import { addLabels, kindLabels } from "../hunt/label.ts"; |
| 16 | 16 | import { createGitRecord } from "./git.ts"; |
| 17 | 17 | import type { PerfRecord } from "./record.ts"; |
| 18 | 18 | import { evaluate, evaluateGit, fresh } from "./watch.ts"; |
| 19 | 19 | |
| 20 | 20 | export const TICK_MS = 60_000; |
| 21 | const KIND: FindingKind = "defect"; | |
| 22 | 21 | |
| 23 | 22 | export function startPerfWatcher( |
| 24 | 23 | app: App, |
| @@ -41,7 +40,7 @@ export function startPerfWatcher( | ||
| 41 | 40 | for (const f of fresh(findings, filed)) { |
| 42 | 41 | if (open.has(f.title)) continue; |
| 43 | 42 | const made = openIssue(app, actor, repo, { title: f.title, body: f.evidence }); |
| 44 | if (made.ok) addLabels(app, repo.id, actor.id, made.value, kindLabels(app, repo.id, KIND)); | |
| 43 | if (made.ok) addLabels(app, repo.id, actor.id, made.value, kindLabels(app, repo.id, f.kind)); | |
| 45 | 44 | } |
| 46 | 45 | } catch (error) { |
| 47 | 46 | app.log.warn("performance watcher failed", { error: String(error) }); |
test/hunt.test.ts +15
| @@ -68,6 +68,21 @@ describe("the hunter", () => { | ||
| 68 | 68 | expect(keys.filter((k) => k === "links.status:/gone")).toHaveLength(1); |
| 69 | 69 | }); |
| 70 | 70 | |
| 71 | test("each finding carries its detector's kind (#8969)", async () => { | |
| 72 | const { fetcher } = site({ | |
| 73 | ...healthy, | |
| 74 | "/": { body: '<a href="/gone">Gone</a><a href="/mine">Your repositories</a>' }, | |
| 75 | "/mine": { status: 302, location: "/" }, | |
| 76 | "/settings": { status: 200 }, | |
| 77 | "/api/v1/apps": { status: 500, type: "application/json", body: "{}" }, | |
| 78 | }); | |
| 79 | const kinds = Object.fromEntries((await huntTarget(BASE, fetcher)).findings.map((f) => [f.key, f.kind])); | |
| 80 | expect(kinds["door.signin-redirect:/settings"]).toBe("security"); | |
| 81 | expect(kinds["links.status:/gone"]).toBe("broken-route"); | |
| 82 | expect(kinds["contract.no-500:/api/v1/apps"]).toBe("broken-route"); | |
| 83 | expect(kinds["links.lands-home:/mine"]).toBe("cosmetic"); | |
| 84 | }); | |
| 85 | ||
| 71 | 86 | test("it refuses a path off the target", async () => { |
| 72 | 87 | const { fetcher } = site({}); |
| 73 | 88 | await expect(probe(BASE, fetcher).get("https://evil.test/x")).rejects.toThrow(); |
test/sentinel.test.ts +21 −1
| @@ -21,7 +21,7 @@ import { createRun } from "../src/core/runs.ts"; | ||
| 21 | 21 | import { createUser, type User } from "../src/core/users.ts"; |
| 22 | 22 | import { recordCap } from "../src/crew/guard.ts"; |
| 23 | 23 | import { sharedCause } from "../src/checks/shared_red.ts"; |
| 24 | import { bugClockAlarm, HIGH_BUG_MS } from "../src/flow/bugs.ts"; | |
| 24 | import { bugClockAlarm, HIGH_BUG_MS, openBugs } from "../src/flow/bugs.ts"; | |
| 25 | 25 | import { |
| 26 | 26 | budgetBreaches, |
| 27 | 27 | degradedCauses, |
| @@ -1059,4 +1059,24 @@ describe("a finding is filed sorted (#8856)", () => { | ||
| 1059 | 1059 | expect(hunt(kapp, "alice/forge", NOW, defect).filed).toBe(1); |
| 1060 | 1060 | expect(names(openTitled("log says x")!.id)).toEqual(["bug"]); |
| 1061 | 1061 | }); |
| 1062 | ||
| 1063 | test("a cosmetic finding lands as polish, on no bug clock (#8969)", () => { | |
| 1064 | const cosmetic = () => [ | |
| 1065 | { key: "look:y", title: "The link y lands on / and is not Home", evidence: ["y"], pillar: "Craft", lookFor: "No y.", kind: "cosmetic" as const }, | |
| 1066 | ]; | |
| 1067 | expect(hunt(kapp, "alice/forge", NOW, cosmetic).filed).toBe(1); | |
| 1068 | const issue = openTitled("link y lands")!; | |
| 1069 | expect(names(issue.id)).toEqual(["polish"]); | |
| 1070 | expect(openBugs(kapp.db).some((b) => b.number === issue.number)).toBe(false); | |
| 1071 | }); | |
| 1072 | ||
| 1073 | test("a broken route lands as a high bug (#8969)", () => { | |
| 1074 | const broken = () => [ | |
| 1075 | { key: "route:z", title: "The route /z answers 500", evidence: ["GET /z → 500"], pillar: "Reliability", lookFor: "/z answers.", kind: "broken-route" as const }, | |
| 1076 | ]; | |
| 1077 | expect(hunt(kapp, "alice/forge", NOW, broken).filed).toBe(1); | |
| 1078 | const issue = openTitled("route /z")!; | |
| 1079 | expect(names(issue.id)).toEqual(["bug", "high"]); | |
| 1080 | expect(openBugs(kapp.db).find((b) => b.number === issue.number)?.high).toBe(true); | |
| 1081 | }); | |
| 1062 | 1082 | }); |
new above
offline