feat: keep a per-repository security finding ledger (#379)

changelog.d/379-security-findings.security.md

@@ -0,0 +1 @@
+Jobs can report the security findings they saw, and each repository now keeps a ledger of them: a finding that was dismissed or fixed and comes back is shown as regressed on the job and repository pages, a dismissal or fix without a written reason is refused, and a finding missing from a later report is never treated as fixed.

docs/api.md

@@ -559,7 +559,10 @@ Headers: `Authorization: Bearer <callbackSecret>`, `Content-Type: application/js
     "checks": "CI green",
     "files": 3,
     "risk": "low",
-    "rollback": "revert the commit"
+    "rollback": "revert the commit",
+    "findings": [
+      { "rule": "js/path-injection", "path": "src/app.ts", "line": 120 }
+    ]
   },
   "checkpoint": "implemented the parser; next: add the rejection test"
 }
@@ -579,7 +582,13 @@ progress heartbeat: it updates `summary`, `checkpoint` and optionally `branch`,
 into the goal when a failed job is retried. A `plan` job writes the plan to
 `summary` and must not create a branch, issue or PR; the super admin can execute it
 later as an iterate job. `evidence` is stored structurally and shown on the job
-detail page; the Fledge automation comment stays compact. With
+detail page; the Fledge automation comment stays compact. `evidence.findings` is
+the security-finding report: up to 100 `{ rule, path, line, state? }` entries,
+with `rule` at most 120 characters, `path` at most 200, a positive integer
+`line` and `state` one of `open`/`dismissed`. An entry that breaks any of that,
+or repeats an id, is dropped and counted in `evidence.findingsDropped`; the
+ledger keeps one row per id and is described in
+[security.md](security.md#security-finding-ledger-379). With
 `repos.stage_mode = 'enforced'`, an iterate callback must also include `scope`,
 `acceptance`, `testRed`, `testGreen` and Dev/Test attribution, and a review
 callback must include a Reviewer and `reviewScore`; otherwise the job fails with

docs/architecture.md

@@ -1902,6 +1902,7 @@ listing, and every row links to the page that already owns it.
 | `repo_members` | per-repository `maintainer`/`developer`/`viewer` rows with a revocation time (#541) |
 | `scoped_grants` | explicit capability grants with source, evidence, expiry, revocation and revision (#541) |
 | `identity_migrations` | one report row per identity backfill version (#541) |
+| `security_findings` | one row per `rule + path + line` a job reported, with `open`/`fixed`/`dismissed`/`regressed`, the job that dismissed it and the written reason or evidence; absence from a later report never moves a row (#379) |
 | `settings` | small key/value flags (e.g. quota notice dedupe); age-based pruning reclaims the per-job plan markers (`plan_expired:`, `plan_reminded:`) on the same window as the `jobs` rows |
 
 ## Deployment records

docs/security.md

@@ -792,3 +792,39 @@ at an entry outside the actor's audience is refused as if the entry were absent,
 before the revision fence, so a hidden key is not a write target or a revision
 oracle. The store refuses a write whose `expected_revision` is stale instead of
 overwriting the current body.
+
+## Security-finding ledger (`#379`)
+
+A job may report the security findings it saw in `evidence.findings`, and the
+repository keeps one row per finding id instead of treating every run as a
+fresh snapshot. The id is `rule + " " + path + ":" + line` (`js/path-injection
+src/app.ts:120`), the list is bounded to 100 entries per report, and an entry
+that breaks the contract (rule over 120 characters, path over 200, a line that
+is not a positive integer, an unknown state, a duplicate id) is dropped and
+counted rather than silently discarded. The block is explicit that these are
+**reported** ids: the ledger does not read GitHub's code-scanning alert store in
+this slice, so it is a memory of what jobs said, not a second alert source.
+
+A finding is `open` the first time a job reports it. Two states are decisions,
+and each one needs a written reason:
+
+- `dismissed` is set by a maintainer on the repository page, with a reason; a
+  dismissal with no reason is refused by the route and by the store, so it can
+  never be stored as a silent accept (or as `open`).
+- `fixed` is set by a maintainer with evidence (a commit, a link or a
+  measurement). A fix with no evidence is refused, and a `fixed` row whose
+  evidence is missing is shown as **unverified** instead of "verified fixed".
+
+The ledger fails closed in both directions. A finding that no later job reports
+is not evidence of a fix: absence never moves a row, so only an explicit,
+evidenced decision can close one. And a finding that comes back after a decision
+is `regressed` — the row keeps the dismissal reason or the fix evidence, the job
+page says `regressed (dismissed in job <id>)` when the decision rests on a
+dismissal, and the repository page lists it under `regressed` until someone
+decides again. The job page lists the findings whose last report was that job,
+which is why the line names the job rather than the full history.
+
+Scope of this slice: recording and displaying reported ids. Reading the ids from
+GitHub's alerts (or the SARIF the CI gate already parses) and reconciling the two
+sources is a later slice, as is blocking an automatic merge on a `regressed`
+finding.

src/app.ts

@@ -2117,6 +2117,9 @@ export async function buildApp(options: BuildOptions): Promise<BuiltApp> {
         queued: store.listQueuedJobs(),
         prJobs: store.listPrJobsForRepo(repo.id, 10),
         workItems,
+        // #379: the repository page lists every state the ledger can hold. These
+        // are the ids jobs reported, not GitHub's alert store.
+        securityFindings: store.listSecurityFindings(repo.id),
         commands,
         // #359: the page names the deadline and the way out, so an ignored plan
         // is a visible decision with a date rather than a silent wait.
@@ -3027,6 +3030,9 @@ export async function buildApp(options: BuildOptions): Promise<BuiltApp> {
       repoLabels,
       events: store.listJobEvents(job.id, 200),
       relations: store.listTaskRelations(job.id),
+      // #379: the findings whose last report was this job, so the page can name
+      // where a dismissed finding was last seen when it came back.
+      findings: store.listSecurityFindingsForJob(job.id),
       postmortem: store.getPostmortemForJob(job.id) ?? null,
       // #540: the cancel form carries the revision it rendered and a fresh key,
       // so a double submit replays the same receipt and a stale page conflicts.
@@ -3469,6 +3475,42 @@ export async function buildApp(options: BuildOptions): Promise<BuiltApp> {
     return reply.redirect(`/repos/${repo.id}?${result.merged.length > 0 ? 'notice' : 'error'}=${encodeURIComponent(message)}#delivery`)
   })
 
+  // #379: the two operator decisions a reported finding can receive. Both need a
+  // written reason, refused here and again by the store: a dismissal with no
+  // reason would read as a silent accept, and a fix with no evidence is exactly
+  // the claim the ledger exists to keep honest.
+  // #379: the two written decisions on a finding. Both are maintainer-only and
+  // both refuse a blank reason/evidence in the route *and* in the store, because
+  // a row that reads as dismissed or fixed with nothing behind it is exactly the
+  // silent-accept this ledger exists to prevent.
+  app.post('/repos/:id/security-findings/:findingId/dismiss', async (request, reply) => {
+    const { viewer, repo } = ownedRepo(request, String((request.params as Record<string, string>).id), true)
+    const t = translator(request.locale ?? 'zh')
+    if (repo.repo_role !== 'maintainer') return reply.redirect(`/repos/${repo.id}?error=${encodeURIComponent(t('只有 maintainer 可以处理安全发现。', 'Only a maintainer can act on a security finding.'))}#security`)
+    const finding = String((request.params as Record<string, string>).findingId)
+    const reason = (form(request).reason ?? '').trim().slice(0, 500)
+    if (reason === '') return reply.redirect(`/repos/${repo.id}?error=${encodeURIComponent(t('忽略一条安全发现需要写明理由。', 'Dismissing a security finding needs a written reason.'))}#security`)
+    if (!store.setSecurityFindingState(repo.id, finding, 'dismissed', reason)) {
+      return reply.redirect(`/repos/${repo.id}?error=${encodeURIComponent(t('这条安全发现不存在。', 'That security finding does not exist.'))}#security`)
+    }
+    store.audit(viewer.user.id, 'security_finding.dismissed', `${repo.id}: ${finding}`.slice(0, 600))
+    return reply.redirect(`/repos/${repo.id}?notice=${encodeURIComponent(t('已忽略这条安全发现；如果它再次出现，会记为回归。', 'Dismissed. If the same id appears again it is recorded as a regression.'))}#security`)
+  })
+
+  app.post('/repos/:id/security-findings/:findingId/fix', async (request, reply) => {
+    const { viewer, repo } = ownedRepo(request, String((request.params as Record<string, string>).id), true)
+    const t = translator(request.locale ?? 'zh')
+    if (repo.repo_role !== 'maintainer') return reply.redirect(`/repos/${repo.id}?error=${encodeURIComponent(t('只有 maintainer 可以处理安全发现。', 'Only a maintainer can act on a security finding.'))}#security`)
+    const finding = String((request.params as Record<string, string>).findingId)
+    const evidence = (form(request).evidence ?? '').trim().slice(0, 500)
+    if (evidence === '') return reply.redirect(`/repos/${repo.id}?error=${encodeURIComponent(t('标记已修复需要写明证据（提交、链接或测量值）。', 'Marking a finding fixed needs evidence: a commit, a link or a measurement.'))}#security`)
+    if (!store.setSecurityFindingState(repo.id, finding, 'fixed', evidence)) {
+      return reply.redirect(`/repos/${repo.id}?error=${encodeURIComponent(t('这条安全发现不存在。', 'That security finding does not exist.'))}#security`)
+    }
+    store.audit(viewer.user.id, 'security_finding.fixed', `${repo.id}: ${finding}`.slice(0, 600))
+    return reply.redirect(`/repos/${repo.id}?notice=${encodeURIComponent(t('已记录修复证据；如果同一个 id 再次出现，会记为回归。', 'Fix evidence recorded. If the same id appears again it is recorded as a regression.'))}#security`)
+  })
+
   app.post('/repos/:id/work/:workItemId/scope', async (request, reply) => {
     const { viewer, repo } = ownedRepo(request, String((request.params as Record<string, string>).id), true)
     const item = store.getWorkItem(String((request.params as Record<string, string>).workItemId))

src/db.ts

@@ -1438,6 +1438,26 @@ CREATE TABLE IF NOT EXISTS repo_rule_overrides (
   PRIMARY KEY (repo_id, path)
 );
 
+-- #379: the per-repository security-finding ledger. The id is rule + path +
+-- line, so a finding that comes back is the same row. Only an operator decision
+-- may write 'dismissed' or 'fixed', and the CHECK below refuses either without
+-- the written reason (a dismissal) or evidence (a fix) it rests on; the report
+-- path can only move a row to open/regressed.
+CREATE TABLE IF NOT EXISTS security_findings (
+  repo_id TEXT NOT NULL REFERENCES repos(id) ON DELETE CASCADE,
+  id TEXT NOT NULL,
+  state TEXT NOT NULL CHECK (state IN ('open', 'dismissed', 'fixed', 'regressed')),
+  first_seen_job TEXT NOT NULL,
+  last_seen_job TEXT NOT NULL,
+  reason TEXT,
+  dismissed_in_job TEXT,
+  updated_at TEXT NOT NULL,
+  PRIMARY KEY (repo_id, id),
+  CHECK (state NOT IN ('dismissed', 'fixed') OR (reason IS NOT NULL AND trim(reason) != ''))
+);
+CREATE INDEX IF NOT EXISTS security_findings_repo_idx ON security_findings(repo_id, state, updated_at DESC);
+CREATE INDEX IF NOT EXISTS security_findings_job_idx ON security_findings(last_seen_job);
+
 -- #384: one postmortem per failed/stalled/cancelled job, plus a ledger row per
 -- recurrence key. The job_id unique constraint is the idempotency guard: a
 -- replayed terminal transition cannot create a second write-up for the same job.
@@ -1940,6 +1960,31 @@ export interface ReleaseTrackingPatch {
   lastCheckedAt: string
 }
 
+// #379: one row per reported finding id, per repository. `first_seen_job` and
+// `last_seen_job` are job ids kept as text, not foreign keys: the ledger is the
+// record of what a job reported, and it must outlive the job row if one is ever
+// pruned. `state` is checked by the table, and a decision state requires the
+// written reason it rests on.
+export interface SecurityFindingRow {
+  repo_id: string
+  id: string
+  state: string
+  first_seen_job: string
+  last_seen_job: string
+  reason: string | null
+  dismissed_in_job: string | null
+  updated_at: string
+}
+
+export interface SecurityFindingWrite {
+  id: string
+  state: string
+  firstSeenJob: string
+  lastSeenJob: string
+  reason: string | null
+  dismissedInJob: string | null
+}
+
 export class Store {
   readonly db: Database
 
@@ -4837,6 +4882,77 @@ export class Store {
     return this.get<JobContextFileRow>('SELECT * FROM job_context_files WHERE job_id = ? AND path = ?', jobId, path)
   }
 
+  // #379: the ledger write is one transaction per report, so a report that names
+  // several findings is recorded whole or not at all. The state machine that
+  // decides each row lives in src/lib/securityFindings.ts; this method stores
+  // the decision it returns.
+  writeSecurityFindings(repoId: string, rows: SecurityFindingWrite[]): void {
+    if (rows.length === 0) return
+    const updatedAt = nowIso()
+    this.transaction(() => {
+      for (const row of rows) {
+        this.run(
+          `INSERT INTO security_findings (repo_id, id, state, first_seen_job, last_seen_job, reason, dismissed_in_job, updated_at)
+           VALUES (?, ?, ?, ?, ?, ?, ?, ?)
+           ON CONFLICT(repo_id, id) DO UPDATE SET
+             state = excluded.state,
+             last_seen_job = excluded.last_seen_job,
+             reason = excluded.reason,
+             dismissed_in_job = excluded.dismissed_in_job,
+             updated_at = excluded.updated_at`,
+          repoId,
+          row.id,
+          row.state,
+          row.firstSeenJob,
+          row.lastSeenJob,
+          row.reason,
+          row.dismissedInJob,
+          updatedAt,
+        )
+      }
+    })
+  }
+
+  getSecurityFinding(repoId: string, id: string): SecurityFindingRow | undefined {
+    return this.get<SecurityFindingRow>('SELECT * FROM security_findings WHERE repo_id = ? AND id = ?', repoId, id)
+  }
+
+  //: Newest first, so the page lists the findings a maintainer is most likely
+  //: to act on before the older ones.
+  listSecurityFindings(repoId: string): SecurityFindingRow[] {
+    return this.all<SecurityFindingRow>('SELECT * FROM security_findings WHERE repo_id = ? ORDER BY updated_at DESC, id ASC', repoId)
+  }
+
+  //: The job page shows the findings whose last report was this job. One table
+  //: keeps the ledger from recording a second, per-job link table; the page
+  //: says "last reported by this job" rather than claiming the full history.
+  listSecurityFindingsForJob(jobId: string): SecurityFindingRow[] {
+    return this.all<SecurityFindingRow>('SELECT * FROM security_findings WHERE last_seen_job = ? ORDER BY id ASC', jobId)
+  }
+
+  //: The only write that may set `dismissed` or `fixed`. A blank reason is
+  //: refused here as well as at the route, so a caller that forgets the check
+  //: cannot put the ledger into a state that reads as verified without evidence.
+  setSecurityFindingState(repoId: string, id: string, state: 'dismissed' | 'fixed', reason: string): boolean {
+    const clean = reason.trim().slice(0, 500)
+    if (clean === '') return false
+    const row = this.getSecurityFinding(repoId, id)
+    if (row === undefined) return false
+    // `dismissed_in_job` names the job the finding was last seen in when the
+    // operator decided, which is what the regression line reads back.
+    const dismissedInJob = state === 'dismissed' ? row.last_seen_job : row.dismissed_in_job
+    this.run(
+      'UPDATE security_findings SET state = ?, reason = ?, dismissed_in_job = ?, updated_at = ? WHERE repo_id = ? AND id = ?',
+      state,
+      clean,
+      dismissedInJob,
+      nowIso(),
+      repoId,
+      id,
+    )
+    return true
+  }
+
   listRepoRuleOverrides(repoId: string): RepoRuleOverrideRow[] {
     return this.all<RepoRuleOverrideRow>('SELECT * FROM repo_rule_overrides WHERE repo_id = ? ORDER BY path', repoId)
   }