diff --git a/CHANGELOG.md b/CHANGELOG.md index 03b1558..fe95aa4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Please choose versions by [Semantic Versioning](http://semver.org/). ## Unreleased +- feat: Ship the comprehensive security v1 rule base in `docs/security/security-review-guide.md` — 7 judgment-tier rules (SSRF, XSS, deserialization, open redirect, webhook verification MUST; mass assignment, insecure defaults SHOULD) and 2 invariant-linked authz rules (resource ownership, tenant isolation MUST) with `**Class**: security-invariant` and `@commits` triggers; extend `scripts/build-index.py` to emit a `class` index key and document the new field in `docs/rule-block-schema.md`; regenerate `rules/index.json` from 171 to 180 entries; record the cross-language detector layout decision (per-language `rules/security/{go,python,node}/` target, go-first v1 stays flat) - fix: `/coding:commit` § 2d — document that untracked files must be `git add`-ed before the pathspec commit; a commit pathspec matches only tracked paths, so every commit introducing a new file failed with `did not match any file(s) known to git` ## v0.49.0 diff --git a/CLAUDE.md b/CLAUDE.md index c192a90..3018729 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -37,7 +37,7 @@ Each enforceable guide in `docs/` should have a matching agent in `agents/`. The | `go-doc-best-practices.md` | `godoc-assistant` | | `go-testing-guide.md` | `go-test-quality-assistant` | | `go-security-linting.md` | `go-security-specialist` | -| `security-review-guide.md` | `go-security-specialist` (rule-base owner for the mechanical security detectors in `rules/security/`) | +| `security-review-guide.md` | `go-security-specialist` (rule-base owner for the comprehensive security rule base — mechanical detectors, judgment rules, invariant-linked authz rules) | | `go-licensing-guide.md` | `license-assistant` | | `agent-command-development-guide.md` | `agent-auditor` + `slash-command-auditor` | | `claude-code-skill-writing-guide.md` | `skill-auditor` | diff --git a/README.md b/README.md index 3159efa..89fadb0 100644 --- a/README.md +++ b/README.md @@ -146,7 +146,7 @@ All guides live in [`docs/`](docs/) and can be read standalone without the plugi | [Replace Directive](docs/go-mod-replace-guide.md) | When to use `replace` in go.mod | | [Linting](docs/go-linting-guide.md) | Static analysis | | [Security Linting](docs/go-security-linting.md) | Security analysis | -| [Security Review Guide](docs/security/security-review-guide.md) | Mechanical security rule base | +| [Security Review Guide](docs/security/security-review-guide.md) | Comprehensive security rule base (mechanical, judgment, invariant tiers) | | [Security Review Pipeline](docs/security/security-review-pipeline.md) | Per-review evidence-pointered security model derivation (entry points, resources, invariants) | | [Kubernetes CRD Controller](docs/go-kubernetes-crd-controller-guide.md) | CRD types, informer, self-install | | [Kubernetes Manifest Layout](docs/k8s-manifest-guide.md) | `k8s/` folder, filename suffixes, templating | diff --git a/docs/rule-block-schema.md b/docs/rule-block-schema.md index bd5fdaa..e24f3b9 100644 --- a/docs/rule-block-schema.md +++ b/docs/rule-block-schema.md @@ -46,9 +46,21 @@ Judgment-tier rules carry a `**Trigger**:` field immediately after `**Enforcemen - Missing `Trigger` field → `trigger` key omitted from the index entry (no scoping applied). - All judgment-tier rules MUST have a `Trigger` field. Mechanical and script rules omit it (they are always run by the funnel unconditionally). +### Optional Field: Class + +A small set of judgment-tier RULE blocks carry a `**Class**:` field immediately after `**Trigger**:` (or immediately after `**Enforcement**:` when no Trigger is present). The walker indexes it as a `class` string in `rules/index.json`. The dispatcher uses it to scope which owner agents consume which judgment tier — invariant-linked rules (`class: security-invariant`) require the derived session security model to fire and are scoped by `**Trigger**: @commits`. + +```markdown +**Class**: +``` + +- v1 token: `security-invariant`. Marks a rule that requires whole-repo reasoning against the derived session security model (see `docs/security/security-review-pipeline.md`). +- Missing `**Class**:` field → `class` key omitted from the index entry (no scoping applied). +- All judgment-tier rules MAY carry a Class; mechanical and script rules omit it. + ### Recommended Field: `Why` -Most rule blocks in this repo carry a `**Why**:` paragraph immediately after `**Enforcement**:`. The `Why` is not indexed (the walker ignores it) but is highly recommended as the *only* place the rule's rationale lives — it tells future authors, agents, and bot reviewers *what failure mode this rule prevents*, which is what makes the rule defensible during code review. +Most rule blocks in this repo carry a `**Why**:` paragraph immediately after the field block (after `**Enforcement**:`, and after `**Trigger**:` / `**Class**:` where present). The `Why` is not indexed (the walker ignores it) but is highly recommended as the *only* place the rule's rationale lives — it tells future authors, agents, and bot reviewers *what failure mode this rule prevents*, which is what makes the rule defensible during code review. ```markdown **Why**: /.yml`), `script` (cites `scripts/rule-checks.sh`), or `judgment` (neither) | | `trigger` | array of strings | Optional **Trigger**: field | Glob patterns; present only when the doc block includes a `**Trigger**:` line. `@commits` is a special value meaning "always active for PR commit review". Missing = no dispatcher-level scoping (owner runs whenever invoked). | +| `class` | string | Optional **Class**: field | Present only when the doc block includes a `**Class**:` line. v1 value: `security-invariant`. | JSON object keys are alphabetically sorted in output. @@ -111,13 +124,16 @@ Example entry: ```json { - "id": "go/context-cancel-in-loop", - "level": "SHOULD", - "doc_path": "docs/go-context-cancellation-in-loops.md", - "anchor": "go/context-cancel-in-loop", - "owner": "go-context-assistant", - "applies_when": "Go for loop body lacks a non-blocking select { case <-ctx.Done(): ...; default: } check, outside *_test.go and vendor/.", - "enforcement": "rules/go/cancel-check-in-loop.yml (mechanical flag) + judgment-tier LLM adjudication for long-running enough to matter." + "id": "go-security/resource-ownership", + "level": "MUST", + "doc_path": "docs/security/security-review-guide.md", + "anchor": "go-security/resource-ownership", + "owner": "go-security-specialist", + "applies_when": "A handler reads a resource addressed by a path parameter without first verifying the authenticated user owns it.", + "enforcement": "judgment — LLM adjudicator resolves the resource's authorization_functions from the derived session security model per docs/security/security-review-pipeline.md.", + "enforcement_type": "judgment", + "trigger": ["@commits"], + "class": "security-invariant" } ``` diff --git a/docs/security/security-review-guide.md b/docs/security/security-review-guide.md index 6d336df..30c4c0b 100644 --- a/docs/security/security-review-guide.md +++ b/docs/security/security-review-guide.md @@ -1,6 +1,6 @@ # Security Review Guide -Companion to [go-security-linting.md](go-security-linting.md) (the gosec workflow), [teamvault-conventions.md](teamvault-conventions.md) (secret handling), and [rule-block-schema.md](../rule-block-schema.md) (the `### RULE` block contract). This guide is the source of truth for the mechanical security rule base that Security Review Mode enforces: every rule maps to a detector in `rules/security/*.yml` and an entry in `rules/index.json` owned by `go-security-specialist`. +Companion to [go-security-linting.md](../go-security-linting.md) (the gosec workflow), [teamvault-conventions.md](../teamvault-conventions.md) (secret handling), and [rule-block-schema.md](../rule-block-schema.md) (the `### RULE` block contract). This guide is the source of truth for the security rule base that Security Review Mode enforces, across three tiers: mechanical rules map to detectors in `rules/security/*.yml`, judgment rules require LLM adjudication, and invariant-linked rules fire against the derived session security model. Every rule is an entry in `rules/index.json` owned by `go-security-specialist`. ## Tiers @@ -10,7 +10,7 @@ Security Review Mode organizes rules into three tiers: - **Judgment tier** — MUST-level rules that require LLM adjudication at review time (SSRF, authorization/IDOR, invariant-preservation concerns). - **Invariant tier** — rules that require whole-repo reasoning rather than a single AST shape. -The judgment and invariant tiers ship in a follow-up task. This guide currently contains the 5 mechanical-tier rules below. +This guide is the source of truth for the security rule base across all three tiers — mechanical, judgment, and invariant; the invariant-tier rules are live here, gated by the walker `Class` support. ## Rules @@ -45,7 +45,7 @@ client := &http.Client{ **Owner**: go-security-specialist **Applies when**: a `*.go` file outside `*_test.go`, `vendor/`, and `mocks/` imports `math/rand` or `math/rand/v2` (the Go standard library's predictable PRNGs). -**Enforcement**: `rules/security/crypto-insecure-random.yml` (mechanical flag — fires on every import of `math/rand`/`math/rand/v2`; the judgment-tier adjudication of whether the usage is security-relevant ships with the judgment tier in a follow-up task, so the detector over-flags legitimate non-security uses by design until then) +**Enforcement**: `rules/security/crypto-insecure-random.yml` (mechanical flag — fires on every import of `math/rand`/`math/rand/v2`; the detector over-flags legitimate non-security uses by design; the judgment-tier adjudication of whether a flagged usage is security-relevant lives with the judgment rules and applies at review time) **Why**: `math/rand` is deterministic and guessable; tokens, IDs, and nonces generated from it are predictable by an attacker. `crypto/rand` is the only source of security-relevant randomness. #### Bad @@ -129,8 +129,337 @@ apiKey := os.Getenv("API_KEY") - **A mechanical rule without a rule-test** — every `rules/security/*.yml` must carry a `rule-tests/security/*-test.yml` (valid → 0, invalid → ≥1) and a snapshot; the `check-rule-tests` precommit gate fails otherwise. - **Silent-zero detectors** — a detector that matches nothing is dead coverage; the acceptance bar is every Bad sample → ≥1 finding and every Good sample → 0. -- **Judgment/invariant-tier RULE blocks in this guide** — the judgment and invariant tiers ship in a follow-up task; until then only the 5 mechanical-tier rules live here. +- **Excluding judgment/invariant-tier rules from this guide** — all three tiers (mechanical / judgment / invariant) are live here; the invariant-tier rules are gated by the walker `Class` support. - **A security finding without provenance** — findings must cite a `rule_id` that resolves in `rules/index.json`; invented security policy is rejected by `validate-citations.sh`. - **`InsecureSkipVerify: true` "temporarily"** — there is no temporary; it is a permanent MITM acceptance. - **Secrets in source** — hardcoded credentials, tokens, and keys are rejected by `hardcoded-secret`; read them from environment or a secrets manager. - **Trading-specific examples** — this repo serves anyone learning Go; examples stay generic (User, Order, Product). + +## Judgment-tier rules + +### RULE go-security/ssrf-user-controlled-url (MUST) + +**Owner**: go-security-specialist +**Applies when**: a Go file outside `*_test.go`, `vendor/`, `mocks/` issues an outbound HTTP request (`http.Get`, `http.NewRequest`, `http.Client.Do`) where the URL or host is derived from a request parameter, header, body field, or other user-controlled source without an allow-list / scheme-and-host validation step. +**Enforcement**: judgment — LLM adjudicator checks the request URL's data flow back to a user-controlled source. No mechanical YAML. +**Trigger**: **/*.go +**Why**: SSRF turns the server into a proxy for the attacker's reach — internal services (cloud metadata `169.254.169.254`, localhost, RFC1918 ranges), partner APIs reachable from the VPC, and arbitrary outbound traffic become attacker-accessible. URL allow-listing plus scheme-and-host pinning is the standard mitigation. + +#### Bad + +```go +func FetchUserAvatar(w http.ResponseWriter, r *http.Request) { + avatarURL := r.URL.Query().Get("url") + resp, err := http.Get(avatarURL) + _ = resp + _ = err +} +``` + +#### Good + +```go +var allowedHosts = map[string]struct{}{"cdn.example.com": {}} + +func FetchUserAvatar(ctx context.Context, w http.ResponseWriter, r *http.Request) { + avatarURL, err := url.Parse(r.URL.Query().Get("url")) + if err != nil { + http.Error(w, "invalid url", http.StatusBadRequest) + return + } + if avatarURL.Scheme != "https" { + http.Error(w, "https required", http.StatusBadRequest) + return + } + if _, ok := allowedHosts[avatarURL.Hostname()]; !ok { + http.Error(w, "host not allowed", http.StatusBadRequest) + return + } + resp, err := http.Get(avatarURL.String()) + _ = resp + _ = err +} +``` + +### RULE go-security/xss-untrusted-html (MUST) + +**Owner**: go-security-specialist +**Applies when**: a Go file outside `*_test.go`, `vendor/`, `mocks/` writes user-supplied data into an HTML response (template `html/template` is fine; `text/template`, `fmt.Fprintf(w, ...)`, raw concatenation into HTML is not) without HTML-escaping or a sanitizer. +**Enforcement**: judgment — LLM adjudicator checks the response writer and the data-flow provenance of the interpolated value. +**Trigger**: **/*.go +**Why**: stored / reflected XSS lets an attacker run JavaScript in the victim's session, exfiltrating cookies, hijacking actions, or pivoting to admin-only routes. `html/template` is context-aware escaping; `text/template` and manual concatenation are not. + +#### Bad + +```go +func RenderGreeting(w http.ResponseWriter, r *http.Request) { + name := r.URL.Query().Get("name") + fmt.Fprintf(w, "

Hello, %s

", name) +} +``` + +#### Good + +```go +func RenderGreeting(w http.ResponseWriter, r *http.Request) { + name := r.URL.Query().Get("name") + if err := htmlTemplate.Execute(w, map[string]string{"Name": name}); err != nil { + http.Error(w, "render failed", http.StatusInternalServerError) + return + } +} +``` + +### RULE go-security/deserialization-unsafe (MUST) + +**Owner**: go-security-specialist +**Applies when**: a Go file outside `*_test.go`, `vendor/`, `mocks/` calls `json.Unmarshal` / `gob.NewDecoder` / `yaml.Unmarshal` / `xml.Unmarshal` on data received from an untrusted source (HTTP request body, message-bus payload, file uploaded by a user) into a struct without a schema gate — i.e. fields are bound directly without `json.Decoder.DisallowUnknownFields` or equivalent strict-mode flags. +**Enforcement**: judgment — LLM adjudicator checks the source provenance of the bytes and the presence of a strict-mode decoder. No mechanical YAML. +**Trigger**: **/*.go +**Why**: lenient decoders accept extra fields the receiver never validated. Attacker-supplied fields (e.g. `IsAdmin: true`, role claims, internal flags) ride along into the parsed struct and downstream authorization decisions. Strict mode plus a typed DTO per request is the standard mitigation. + +#### Bad + +```go +var u User +if err := json.Unmarshal(reqBody, &u); err != nil { + return err +} +if u.IsAdmin { + // ... +} +``` + +#### Good + +```go +dec := json.NewDecoder(req.Body) +dec.DisallowUnknownFields() +var u User +if err := dec.Decode(&u); err != nil { + return err +} +if u.IsAdmin { + // ... +} +``` + +### RULE go-security/open-redirect (MUST) + +**Owner**: go-security-specialist +**Applies when**: a Go HTTP handler outside `*_test.go`, `vendor/`, `mocks/` reads a `next` / `return_to` / `redirect` query parameter (or similar) and forwards the user to the parsed URL via `http.Redirect` / `http.RedirectHandler` / manual `Location:` header without an allow-list of permitted hosts / paths. +**Enforcement**: judgment — LLM adjudicator checks the data flow from the request parameter to the redirect target. No mechanical YAML. +**Trigger**: **/*.go +**Why**: open redirects become phishing landing pages — the attacker crafts `https://your-app.com/login?next=https://evil.example.com/steal-cookie` and the victim's click traverses the trusted domain first. An allow-list of internal paths (or absolute-URL rejection) closes the path. + +#### Bad + +```go +func LoginHandler(w http.ResponseWriter, r *http.Request) { + next := r.URL.Query().Get("next") + http.Redirect(w, r, next, http.StatusFound) +} +``` + +#### Good + +```go +func LoginHandler(w http.ResponseWriter, r *http.Request) { + next := r.URL.Query().Get("next") + if !isInternalPath(next) { + next = "/dashboard" + } + http.Redirect(w, r, next, http.StatusFound) +} +``` + +### RULE go-security/webhook-verification (MUST) + +**Owner**: go-security-specialist +**Applies when**: a Go HTTP handler outside `*_test.go`, `vendor/`, `mocks/` exposes a webhook-receiving endpoint that processes the request body without verifying a provider signature (`X-Hub-Signature-256` / `Stripe-Signature` / equivalent HMAC header) before treating the payload as trusted. +**Enforcement**: judgment — LLM adjudicator checks the signature-verification step preceding payload processing. No mechanical YAML. +**Trigger**: **/*.go +**Why**: unsigned webhook endpoints accept attacker-fabricated events — order confirmations, payment successes, account-status changes — that drive automated workflows. HMAC verification with a shared secret is the standard mitigation; constant-time comparison prevents timing oracles. + +#### Bad + +```go +func WebhookHandler(w http.ResponseWriter, r *http.Request) { + var order Order + if err := json.NewDecoder(r.Body).Decode(&order); err != nil { + http.Error(w, "bad payload", http.StatusBadRequest) + return + } + orderService.MarkPaid(order) +} +``` + +#### Good + +```go +func WebhookHandler(w http.ResponseWriter, r *http.Request) { + sig := r.Header.Get("X-Signature") + body, err := io.ReadAll(r.Body) + if err != nil { + http.Error(w, "bad body", http.StatusBadRequest) + return + } + if !verifyHMAC(body, []byte(sig), []byte(secret)) { + http.Error(w, "invalid signature", http.StatusUnauthorized) + return + } + var order Order + if err := json.Unmarshal(body, &order); err != nil { + http.Error(w, "bad payload", http.StatusBadRequest) + return + } + orderService.MarkPaid(order) +} +``` + +### RULE go-security/mass-assignment (SHOULD) + +**Owner**: go-security-specialist +**Applies when**: a Go file outside `*_test.go`, `vendor/`, `mocks/` binds an HTTP request body or query directly into a domain struct that carries authorization-relevant fields (role flags, ownership pointers, billing status) without an explicit allow-list of bindable fields. +**Enforcement**: judgment — LLM adjudicator checks the struct-to-DTO separation and the bind path. No mechanical YAML. +**Trigger**: **/*.go +**Why**: mass-assignment lets an attacker upgrade their own role, change ownership pointers, or flip internal state via fields the API surface never advertised. A typed input DTO that exposes only the user-settable fields is the standard mitigation. + +#### Bad + +```go +func UpdateUser(w http.ResponseWriter, r *http.Request) { + var u User + if err := json.NewDecoder(r.Body).Decode(&u); err != nil { + return + } + userRepo.Save(u) // u.Role, u.OwnerID, u.Balance all settable by the client +} +``` + +#### Good + +```go +func UpdateUser(w http.ResponseWriter, r *http.Request) { + var input UpdateUserInput + if err := json.NewDecoder(r.Body).Decode(&input); err != nil { + return + } + userRepo.UpdateName(r.Context(), input.ID, input.Name) // only Name is bindable +} +``` + +### RULE go-security/insecure-defaults (SHOULD) + +**Owner**: go-security-specialist +**Applies when**: a Go file outside `*_test.go`, `vendor/`, `mocks/` ships a security-relevant default (TLS min version, cookie `Secure`/`HttpOnly`/`SameSite`, password hashing cost, session timeout, CORS wildcard, CSP `unsafe-inline`) at a value weaker than the secure baseline, instead of failing closed to the secure value. +**Enforcement**: judgment — LLM adjudicator compares the shipped default against the secure baseline (e.g. `MinVersion: tls.VersionTLS12`, cookie `Secure`/`HttpOnly`/`SameSite` set, non-wildcard CORS, no `unsafe-inline` CSP) and flags any security-relevant default weaker than it. No mechanical YAML. +**Trigger**: **/*.go +**Why**: insecure defaults turn every deployment into a vulnerable one — operators who don't override the default get the weak value. Secure defaults plus an explicit override knob is the standard posture. + +#### Bad + +```go +var cookieCfg = http.Cookie{ + Name: "session", + Secure: false, + HttpOnly: false, + SameSite: http.SameSiteNoneMode, +} +``` + +#### Good + +```go +var cookieCfg = http.Cookie{ + Name: "session", + Secure: true, + HttpOnly: true, + SameSite: http.SameSiteStrictMode, +} +``` + +## Invariant-tier rules + +### RULE go-security/resource-ownership (MUST) + +**Owner**: go-security-specialist +**Applies when**: a Go handler / service method outside `*_test.go`, `vendor/`, `mocks/` reads, mutates, or deletes a resource (DB row, file, third-party-API object) addressed by a path parameter, query parameter, body field, or header value, without first verifying that the authenticated user owns the resource. "Owns" is defined per resource by the `authorization_functions` field in the derived session security model (see `docs/security/security-review-pipeline.md`). +**Enforcement**: judgment — LLM adjudicator resolves the resource's `authorization_functions` from the derived session security model per `docs/security/security-review-pipeline.md`, fires when the diff accesses a resource by identifier without the owning authorization function enforced, and emits findings as `kind=invariant` with `invariant_id` resolving in the session model. +**Trigger**: @commits +**Class**: security-invariant +**Why**: resource-ownership gaps are the canonical IDOR / BOLA class — the attacker authenticates as a legitimate user and accesses another user's data via a guessed or harvested identifier. Generic linters cannot detect these: the missing check is the absence of an authorization call, not the presence of a forbidden call. Whole-repo reasoning against the derived model is the only enforcement path. + +#### Bad + +```go +func GetOrder(w http.ResponseWriter, r *http.Request) { + orderID := chi.URLParam(r, "orderID") + order, err := orderRepo.Find(r.Context(), orderID) + if err != nil { + http.Error(w, "not found", http.StatusNotFound) + return + } + json.NewEncoder(w).Encode(order) +} +``` + +#### Good + +```go +func GetOrder(w http.ResponseWriter, r *http.Request) { + user := userFromCtx(r.Context()) + orderID := chi.URLParam(r, "orderID") + order, err := orderRepo.FindOwnedBy(r.Context(), orderID, user.ID) + if err != nil { + http.Error(w, "not found", http.StatusNotFound) + return + } + json.NewEncoder(w).Encode(order) +} +``` + +### RULE go-security/tenant-isolation (MUST) + +**Owner**: go-security-specialist +**Applies when**: a Go handler / service method outside `*_test.go`, `vendor/`, `mocks/` issues a query, mutation, or third-party call scoped by an account / tenant / org identifier without first verifying the authenticated user belongs to that tenant. "Belongs" is defined per tenant resource by the `authorization_functions` field in the derived session security model (see `docs/security/security-review-pipeline.md`). +**Enforcement**: judgment — LLM adjudicator resolves the tenant resource's `authorization_functions` from the derived session security model per `docs/security/security-review-pipeline.md`, fires when the diff scopes the call by tenant without the owning authorization function enforced, and emits findings as `kind=invariant` with `invariant_id` resolving in the session model. +**Trigger**: @commits +**Class**: security-invariant +**Why**: tenant isolation gaps let an authenticated user in tenant A read or mutate tenant B's data via a guessed tenant identifier — cross-tenant data leakage at scale. The authorization check is a missing-call absence, not a forbidden-call presence; whole-repo reasoning against the derived model is required. + +#### Bad + +```go +func ListInvoices(w http.ResponseWriter, r *http.Request) { + tenantID := r.URL.Query().Get("tenant_id") + invoices, err := invoiceRepo.List(r.Context(), tenantID) + if err != nil { + http.Error(w, "list failed", http.StatusInternalServerError) + return + } + json.NewEncoder(w).Encode(invoices) +} +``` + +#### Good + +```go +func ListInvoices(w http.ResponseWriter, r *http.Request) { + user := userFromCtx(r.Context()) + invoices, err := invoiceRepo.ListForTenant(r.Context(), user.TenantID) + if err != nil { + http.Error(w, "list failed", http.StatusInternalServerError) + return + } + json.NewEncoder(w).Encode(invoices) +} +``` + +## Cross-language detector layout + +When security rules grow beyond a single language, the detector tree splits per-language under `rules/security/{go,python,node}/` (one subdir per language the rule base covers). The runner that scans a multi-language repo gains a per-language case mirroring the existing `node/frontend` special-case in `scripts/ast-grep-runner.sh` (node rules are skipped on frontend projects), dispatching each language's detectors to the matching `ast-grep scan --lang ` invocation. + +For the v1 release, security rules are go-first: every detector lives flat under `rules/security/` (no per-language subdirectories). This matches the rule base's actual coverage today — five mechanical detectors plus nine judgment / invariant rules, all Go-targeted. The split documented above is the target layout for the cross-language expansion (the python and node language cases), deferred until non-Go rules ship. + +The decision is recorded here as a spike outcome (no structural reorganization ships with this version): the cross-language split is the chosen shape; the go-first v1 stays flat because no non-Go detectors exist yet. diff --git a/llms.txt b/llms.txt index d7bec2f..4445e20 100644 --- a/llms.txt +++ b/llms.txt @@ -25,7 +25,7 @@ - [TDD Guide](docs/tdd-guide.md): Red-green-refactor cycle - [Linting](docs/go-linting-guide.md): Static analysis configuration - [Security Linting](docs/go-security-linting.md): Security-focused static analysis -- [Security Review Guide](docs/security/security-review-guide.md): Mechanical security rule base (5 detectors) for Security Review Mode +- [Security Review Guide](docs/security/security-review-guide.md): Comprehensive security rule base (5 mechanical detectors + 7 judgment rules + 2 invariant-linked authz rules) for Security Review Mode - [Security Review Pipeline](docs/security/security-review-pipeline.md): Per-review evidence-pointered security model derivation procedure — entry points, resources + authorization functions, invariants, attack-surface inventory (procedure contract, not a rule guide) - [security-verifier](agents/security-verifier.md): Security-mode post-adjudication falsification gate — verdicts confirmed|plausible|rejected, counterevidence_checked on survivors (not in any command dispatch list) diff --git a/prompts/completed/047-judgment-tier-rules-and-reconcile.md b/prompts/completed/047-judgment-tier-rules-and-reconcile.md new file mode 100644 index 0000000..1aea1eb --- /dev/null +++ b/prompts/completed/047-judgment-tier-rules-and-reconcile.md @@ -0,0 +1,393 @@ +--- +status: completed +spec: [011-security-comprehensive-rules] +summary: Landed 7 judgment-tier go-security RULE blocks (5 MUST, 2 SHOULD) in docs/security/security-review-guide.md, reconciled all deferral prose about the judgment/invariant tiers, and regenerated rules/index.json from 171 to 178 entries (7 new judgment entries, owner go-security-specialist, trigger **/*.go); make precommit green with only the two intended files touched +execution_id: coding-security-comprehensive-rules-exec-047-judgment-tier-rules-and-reconcile +dark-factory-version: dev +created: "2026-08-23T20:30:00Z" +queued: "2026-08-23T20:53:15Z" +started: "2026-08-23T20:53:17Z" +completed: "2026-08-23T20:54:33Z" +branch: dark-factory/security-comprehensive-rules +--- + +# Judgment-tier rules + guide reconciliation + + +- Append 7 judgment-tier RULE blocks to `docs/security/security-review-guide.md` — SSRF, XSS/untrusted-html, deserialization, open redirect, webhook verification (MUST); mass assignment, insecure defaults (SHOULD) +- Every block is owner `go-security-specialist`, ID in the two-component `go-security/` form, carries `**Trigger**: **/*.go`, `**Why**:`, and `#### Bad` / `#### Good` generic examples, and its `**Enforcement**:` cites no `rules//.yml` path so it derives `enforcement_type: judgment` +- Reconcile the guide's deferral prose: the "judgment and invariant tiers ship in a follow-up task" sentence, the crypto-insecure-random parenthetical "until then", the anti-pattern bullet refusing judgment/invariant blocks here, and the "currently contains the 5 mechanical-tier rules below" sentence +- Regenerate `rules/index.json` from 171 to 178 entries via `make build-index`; all existing entries stay byte-identical +- No walker, schema, detector, CHANGELOG, or out-of-scope file changes — those land in prompts 2/3 +- Working-tree changes are left for the daemon's `workflow: direct` post-prompt commit; no git is run inside the container + + + +The guide's judgment tier ships: 7 schema-conformant `### RULE go-security/` blocks land in `docs/security/security-review-guide.md`, every deferral claim about the judgment/invariant tiers is reconciled to the shipped state, and `rules/index.json` is regenerated to 178 entries (all 7 new entries `enforcement_type: judgment`, `trigger: ["**/*.go"]`, owner `go-security-specialist`) with `make precommit` green and the tree clean except for this prompt's two touched files. + + + +Spec 011 (`specs/in-progress/011-security-comprehensive-rules.md`) ships the comprehensive security v1 rule base. This is prompt **1 of 3**. It lands the judgment-tier surface (7 rules) plus a prose reconciliation so the guide no longer claims the judgment/invariant tiers "ship in a follow-up task". No walker or schema changes — those land in prompt 2. + +Read fully before writing: + +- `/workspace/CLAUDE.md` — project conventions, generic content only (User/Order/Product/Customer — never trading-domain), Doc↔Agent alignment table. +- `/workspace/docs/rule-block-schema.md` — the `### RULE` block contract: heading `### RULE (LEVEL)`, required fields `**Owner**:` / `**Applies when**:` / `**Enforcement**:` in that order, optional `**Trigger**:` immediately after `**Enforcement**:` (judgment-tier rules MUST carry it), recommended `**Why**:` after `**Trigger**:`, then `#### Bad` / `#### Good`. ID format `/[/]`, anchor = id verbatim, level tokens MUST/SHOULD/MAY. +- `/workspace/docs/security/security-review-guide.md` — the file to extend. Today it contains 5 mechanical RULE blocks (`tls-insecure-skip-verify`, `crypto-insecure-random`, `crypto-weak-algorithm`, `sql-string-interpolation`, `hardcoded-secret`). Three reconciliation points plus a fourth: (1) the Tiers-section sentence "The judgment and invariant tiers ship in a follow-up task." and its following sentence "This guide currently contains the 5 mechanical-tier rules below."; (2) the `crypto-insecure-random` Enforcement parenthetical noting judgment-tier adjudication ships "until then"; (3) the anti-pattern bullet "Judgment/invariant-tier RULE blocks in this guide". +- `/workspace/scripts/build-index.py` — the walker. Its field-parse tuple today is `("Owner", "Applies when", "Enforcement", "Trigger")`; adding a `Class` key here is prompt 2's job, not this prompt's. +- `/workspace/Makefile` — `make build-index` regenerates `rules/index.json`; `make precommit` runs check-links/check-json/check-index/check-coverage/check-acceptance/check-rule-tests/bench-test. +- `/workspace/rules/index.json` — currently 171 entries; this prompt must grow it to 178. + +The 7 new IDs (two-component `go-security/` form per spike Finding 1): + +1. `go-security/ssrf-user-controlled-url` (MUST) +2. `go-security/xss-untrusted-html` (MUST) +3. `go-security/deserialization-unsafe` (MUST) +4. `go-security/open-redirect` (MUST) +5. `go-security/webhook-verification` (MUST) +6. `go-security/mass-assignment` (SHOULD) +7. `go-security/insecure-defaults` (SHOULD) + +Owner: `go-security-specialist` for every block. Every block carries `**Trigger**: **/*.go` (judgment-tier scoping). None of the 7 enforcement fields cites a `rules//.yml` path or `scripts/rule-checks.sh` — so every entry derives `enforcement_type: judgment`. + + + + +### 1. Append 7 RULE blocks to `docs/security/security-review-guide.md` + +Append (do NOT modify the existing 5 mechanical blocks) after the existing `## Anti-patterns to refuse` section. Add a `## Judgment-tier rules` heading immediately before the 7 new blocks so they are not visually under the anti-patterns section. Each block conforms to `docs/rule-block-schema.md` exactly. + +**Field order is frozen** (schema § Optional Field: Trigger; spec 011 Constraint "Schema contract"): `**Owner**:` → `**Applies when**:` → `**Enforcement**:` → `**Trigger**:` → `**Why**:`. `**Trigger**:` sits immediately after `**Enforcement**:`. Do not reorder. After the field block come `#### Bad` and `#### Good` code blocks. Generic User/Order/Product/Customer examples throughout. + +The template each block follows: + +``` +### RULE go-security/ (LEVEL) + +**Owner**: go-security-specialist +**Applies when**: +**Enforcement**: /.yml or scripts/rule-checks.sh> +**Trigger**: **/*.go +**Why**: + +#### Bad + + +#### Good + +``` + +**Block 1 — `go-security/ssrf-user-controlled-url` (MUST)** +- Applies when: a Go file outside `*_test.go`, `vendor/`, `mocks/` issues an outbound HTTP request (`http.Get`, `http.NewRequest`, `http.Client.Do`) where the URL or host is derived from a request parameter, header, body field, or other user-controlled source without an allow-list / scheme-and-host validation step. +- Enforcement: judgment — LLM adjudicator checks the request URL's data flow back to a user-controlled source. No mechanical YAML. +- Why: SSRF turns the server into a proxy for the attacker's reach — internal services (cloud metadata `169.254.169.254`, localhost, RFC1918 ranges), partner APIs reachable from the VPC, and arbitrary outbound traffic become attacker-accessible. URL allow-listing plus scheme-and-host pinning is the standard mitigation. +- Bad: + ```go + func FetchUserAvatar(w http.ResponseWriter, r *http.Request) { + avatarURL := r.URL.Query().Get("url") + resp, err := http.Get(avatarURL) + _ = resp + _ = err + } + ``` +- Good: + ```go + var allowedHosts = map[string]struct{}{"cdn.example.com": {}} + + func FetchUserAvatar(ctx context.Context, w http.ResponseWriter, r *http.Request) { + avatarURL, err := url.Parse(r.URL.Query().Get("url")) + if err != nil { + http.Error(w, "invalid url", http.StatusBadRequest) + return + } + if avatarURL.Scheme != "https" { + http.Error(w, "https required", http.StatusBadRequest) + return + } + if _, ok := allowedHosts[avatarURL.Hostname()]; !ok { + http.Error(w, "host not allowed", http.StatusBadRequest) + return + } + resp, err := http.Get(avatarURL.String()) + _ = resp + _ = err + } + ``` + +**Block 2 — `go-security/xss-untrusted-html` (MUST)** +- Applies when: a Go file outside `*_test.go`, `vendor/`, `mocks/` writes user-supplied data into an HTML response (template `html/template` is fine; `text/template`, `fmt.Fprintf(w, ...)`, raw concatenation into HTML is not) without HTML-escaping or a sanitizer. +- Enforcement: judgment — LLM adjudicator checks the response writer and the data-flow provenance of the interpolated value. +- Why: stored / reflected XSS lets an attacker run JavaScript in the victim's session, exfiltrating cookies, hijacking actions, or pivoting to admin-only routes. `html/template` is context-aware escaping; `text/template` and manual concatenation are not. +- Bad: + ```go + func RenderGreeting(w http.ResponseWriter, r *http.Request) { + name := r.URL.Query().Get("name") + fmt.Fprintf(w, "

Hello, %s

", name) + } + ``` +- Good: + ```go + func RenderGreeting(w http.ResponseWriter, r *http.Request) { + name := r.URL.Query().Get("name") + if err := htmlTemplate.Execute(w, map[string]string{"Name": name}); err != nil { + http.Error(w, "render failed", http.StatusInternalServerError) + return + } + } + ``` + +**Block 3 — `go-security/deserialization-unsafe` (MUST)** +- Applies when: a Go file outside `*_test.go`, `vendor/`, `mocks/` calls `json.Unmarshal` / `gob.NewDecoder` / `yaml.Unmarshal` / `xml.Unmarshal` on data received from an untrusted source (HTTP request body, message-bus payload, file uploaded by a user) into a struct without a schema gate — i.e. fields are bound directly without `json.Decoder.DisallowUnknownFields` or equivalent strict-mode flags. +- Enforcement: judgment — LLM adjudicator checks the source provenance of the bytes and the presence of a strict-mode decoder. No mechanical YAML. +- Why: lenient decoders accept extra fields the receiver never validated. Attacker-supplied fields (e.g. `IsAdmin: true`, role claims, internal flags) ride along into the parsed struct and downstream authorization decisions. Strict mode plus a typed DTO per request is the standard mitigation. +- Bad: + ```go + var u User + if err := json.Unmarshal(reqBody, &u); err != nil { + return err + } + if u.IsAdmin { + // ... + } + ``` +- Good: + ```go + dec := json.NewDecoder(req.Body) + dec.DisallowUnknownFields() + var u User + if err := dec.Decode(&u); err != nil { + return err + } + if u.IsAdmin { + // ... + } + ``` + +**Block 4 — `go-security/open-redirect` (MUST)** +- Applies when: a Go HTTP handler outside `*_test.go`, `vendor/`, `mocks/` reads a `next` / `return_to` / `redirect` query parameter (or similar) and forwards the user to the parsed URL via `http.Redirect` / `http.RedirectHandler` / manual `Location:` header without an allow-list of permitted hosts / paths. +- Enforcement: judgment — LLM adjudicator checks the data flow from the request parameter to the redirect target. No mechanical YAML. +- Why: open redirects become phishing landing pages — the attacker crafts `https://your-app.com/login?next=https://evil.example.com/steal-cookie` and the victim's click traverses the trusted domain first. An allow-list of internal paths (or absolute-URL rejection) closes the path. +- Bad: + ```go + func LoginHandler(w http.ResponseWriter, r *http.Request) { + next := r.URL.Query().Get("next") + http.Redirect(w, r, next, http.StatusFound) + } + ``` +- Good: + ```go + func LoginHandler(w http.ResponseWriter, r *http.Request) { + next := r.URL.Query().Get("next") + if !isInternalPath(next) { + next = "/dashboard" + } + http.Redirect(w, r, next, http.StatusFound) + } + ``` + +**Block 5 — `go-security/webhook-verification` (MUST)** +- Applies when: a Go HTTP handler outside `*_test.go`, `vendor/`, `mocks/` exposes a webhook-receiving endpoint that processes the request body without verifying a provider signature (`X-Hub-Signature-256` / `Stripe-Signature` / equivalent HMAC header) before treating the payload as trusted. +- Enforcement: judgment — LLM adjudicator checks the signature-verification step preceding payload processing. No mechanical YAML. +- Why: unsigned webhook endpoints accept attacker-fabricated events — order confirmations, payment successes, account-status changes — that drive automated workflows. HMAC verification with a shared secret is the standard mitigation; constant-time comparison prevents timing oracles. +- Bad: + ```go + func WebhookHandler(w http.ResponseWriter, r *http.Request) { + var order Order + if err := json.NewDecoder(r.Body).Decode(&order); err != nil { + http.Error(w, "bad payload", http.StatusBadRequest) + return + } + orderService.MarkPaid(order) + } + ``` +- Good: + ```go + func WebhookHandler(w http.ResponseWriter, r *http.Request) { + sig := r.Header.Get("X-Signature") + body, err := io.ReadAll(r.Body) + if err != nil { + http.Error(w, "bad body", http.StatusBadRequest) + return + } + if !verifyHMAC(body, []byte(sig), []byte(secret)) { + http.Error(w, "invalid signature", http.StatusUnauthorized) + return + } + var order Order + if err := json.Unmarshal(body, &order); err != nil { + http.Error(w, "bad payload", http.StatusBadRequest) + return + } + orderService.MarkPaid(order) + } + ``` + +**Block 6 — `go-security/mass-assignment` (SHOULD)** +- Applies when: a Go file outside `*_test.go`, `vendor/`, `mocks/` binds an HTTP request body or query directly into a domain struct that carries authorization-relevant fields (role flags, ownership pointers, billing status) without an explicit allow-list of bindable fields. +- Enforcement: judgment — LLM adjudicator checks the struct-to-DTO separation and the bind path. No mechanical YAML. +- Why: mass-assignment lets an attacker upgrade their own role, change ownership pointers, or flip internal state via fields the API surface never advertised. A typed input DTO that exposes only the user-settable fields is the standard mitigation. +- Bad: + ```go + func UpdateUser(w http.ResponseWriter, r *http.Request) { + var u User + if err := json.NewDecoder(r.Body).Decode(&u); err != nil { + return + } + userRepo.Save(u) // u.Role, u.OwnerID, u.Balance all settable by the client + } + ``` +- Good: + ```go + func UpdateUser(w http.ResponseWriter, r *http.Request) { + var input UpdateUserInput + if err := json.NewDecoder(r.Body).Decode(&input); err != nil { + return + } + userRepo.UpdateName(r.Context(), input.ID, input.Name) // only Name is bindable + } + ``` + +**Block 7 — `go-security/insecure-defaults` (SHOULD)** +- Applies when: a Go file outside `*_test.go`, `vendor/`, `mocks/` ships a security-relevant default (TLS min version, cookie `Secure`/`HttpOnly`/`SameSite`, password hashing cost, session timeout, CORS wildcard, CSP `unsafe-inline`) at a value weaker than the secure baseline, instead of failing closed to the secure value. +- Enforcement: judgment — LLM adjudicator compares the shipped default against the secure baseline (e.g. `MinVersion: tls.VersionTLS12`, cookie `Secure`/`HttpOnly`/`SameSite` set, non-wildcard CORS, no `unsafe-inline` CSP) and flags any security-relevant default weaker than it. No mechanical YAML. +- Why: insecure defaults turn every deployment into a vulnerable one — operators who don't override the default get the weak value. Secure defaults plus an explicit override knob is the standard posture. +- Bad: + ```go + var cookieCfg = http.Cookie{ + Name: "session", + Secure: false, + HttpOnly: false, + SameSite: http.SameSiteNoneMode, + } + ``` +- Good: + ```go + var cookieCfg = http.Cookie{ + Name: "session", + Secure: true, + HttpOnly: true, + SameSite: http.SameSiteStrictMode, + } + ``` + +### 2. Reconcile the guide's prose + +Four edits to existing prose in `docs/security/security-review-guide.md`: + +(a) **Tiers section (line 13 area):** replace the two sentences "The judgment and invariant tiers ship in a follow-up task." AND "This guide currently contains the 5 mechanical-tier rules below." with a single sentence stating the shipped state, e.g. "This guide is the source of truth for the mechanical and judgment tiers of the security rule base; the invariant-tier rules land with the walker `Class` support." Keep the three-tier framing intact. + +(b) **`crypto-insecure-random` Enforcement parenthetical:** the parenthetical "(the judgment-tier adjudication of whether the usage is security-relevant ships with the judgment tier in a follow-up task, so the detector over-flags legitimate non-security uses by design until then)" — rewrite to drop the deferral: "(the detector over-flags legitimate non-security uses by design; the judgment-tier adjudication of whether a flagged usage is security-relevant lives with the judgment rules and applies at review time)". The YAML detection behavior is unchanged — the detector still fires on every `math/rand` import. + +(c) **Anti-patterns section:** replace the bullet "Judgment/invariant-tier RULE blocks in this guide — the judgment and invariant tiers ship in a follow-up task; until then only the 5 mechanical-tier rules live here." with: "Judgment-tier RULE blocks are in scope for this guide — the judgment tier is live here alongside the mechanical tier, and the invariant-tier rules land with the walker `Class` support." + +**Do NOT touch** any of the 5 mechanical RULE blocks' field text or code examples. **Do NOT touch** the other anti-pattern bullets. **Do NOT touch** the `## Tiers` section's first three paragraphs. + +### 3. Regenerate `rules/index.json` + +Run: + +```bash +make build-index +``` + +Expected: `rules/index.json` grows from 171 to **178 entries** (171 existing + 7 new). All 7 new entries must: +- have `id` in the `go-security/` two-component form +- have `owner == "go-security-specialist"` +- have `doc_path == "docs/security/security-review-guide.md"` +- have `anchor == id` +- have `level in ("MUST","SHOULD","MAY")` +- have `enforcement_type == "judgment"` (no `rules//.yml` path in the Enforcement field) +- have a non-empty `trigger: ["**/*.go"]` array +- non-empty `applies_when` and `enforcement` + +All 171 existing entries stay byte-identical. + +Then run `make precommit` — must exit 0. + +### 4. Do NOT commit + +Do NOT run `git` of any kind — the container's `.git` is masked (`hideGit: true`) and dark-factory's `workflow: direct` post-prompt commit stages and commits all dirty files on completion (repo convention: "Do NOT commit — dark-factory handles git"). Touched paths expected in the daemon's commit: `docs/security/security-review-guide.md`, `rules/index.json` only. + + + + +- **Rule identity:** all 7 new IDs use the two-component `go-security/` form (spike Finding 1) — never the three-component `security//` form. +- **Owner:** `go-security-specialist` in every new block and index entry. +- **Schema contract (frozen):** field order `**Owner**:` → `**Applies when**:` → `**Enforcement**:` → `**Trigger**:` → `**Why**:`; `**Trigger**:` immediately after `**Enforcement**:`. Judgment-tier rules MUST carry a Trigger; mechanical rules omit it. The 7 new blocks carry `**Trigger**: **/*.go`. +- **Enforcement-type derivation:** none of the 7 enforcement fields cites `rules//.yml` or `scripts/rule-checks.sh`, so every new entry derives `enforcement_type: judgment`; `check-coverage.sh` must not flag an orphan. +- **Extend, don't create:** all changes land in the existing `docs/security/security-review-guide.md`; no new guide, no README/llms.txt/code-review.md changes. +- **No changes to:** `scripts/build-index.py` (`Class` support is prompt 2's), `docs/rule-block-schema.md` (prompt 2's), `scripts/validate-citations.sh`, `commands/*.md`, `agents/*.md`, `.maintainer.yaml`, `scenarios/`, `rules/security/` (no detector added/removed, still 5 YAMLs), `CHANGELOG.md` (prompt 3's). +- **Index freshness:** this prompt edits RULE blocks, so it must run `make build-index` and leave `make check-index` green — `make precommit` exits 0. +- **Generic content only:** Bad/Good examples use User, Order, Product, Customer — never trading or project-specific domains. No real provider URLs (use `X-Signature` placeholder). +- **Git discipline:** no git inside the container (hideGit masks `.git`); the daemon owns the post-prompt commit. +- **Scope split:** the 2 invariant-linked blocks (`class: security-invariant`), the walker `Class` field, the schema doc, the cross-language layout prose, and the CHANGELOG entry belong to prompts 2/3 — do not ship them here. + + + +All commands are container-executable (repo root). No git — `.git` is masked. + +```bash +# 1. Seven new judgment RULE blocks in the guide +grep -Ec '^### RULE go-security/(ssrf-user-controlled-url|xss-untrusted-html|deserialization-unsafe|open-redirect|webhook-verification|mass-assignment|insecure-defaults)' docs/security/security-review-guide.md +# expect: 7 + +# 2. Total RULE count = 12 (5 mechanical + 7 new judgment; the 2 invariant blocks land in prompt 2, reaching 14 at the final state) +grep -c '^### RULE ' docs/security/security-review-guide.md +# expect: 12 + +# 3. Each block carries a **Why**, Bad, Good +grep -c '\*\*Why\*\*' docs/security/security-review-guide.md # expect: 12 +grep -c '^#### Bad' docs/security/security-review-guide.md # expect: 12 +grep -c '^#### Good' docs/security/security-review-guide.md # expect: 12 + +# 4. Reconcile — deferral prose gone (AC9 negatives) +grep -cE 'follow-up task|ships in a follow-up|ship.*follow-up' docs/security/security-review-guide.md # expect: 0 +grep -c 'Judgment/invariant-tier RULE blocks in this guide' docs/security/security-review-guide.md # expect: 0 +grep -c 'until then' docs/security/security-review-guide.md # expect: 0 + +# 5. Regenerate index, expect 178 entries +make build-index +python3 scripts/build-index.py | jq 'length' # expect: 178 + +# 6. Seven new entries in the index with the right shape +python3 scripts/build-index.py | jq '[.[] | select(.id | test("go-security/(ssrf-user-controlled-url|xss-untrusted-html|deserialization-unsafe|open-redirect|webhook-verification|mass-assignment|insecure-defaults)")) | {id, level, enforcement_type, owner, trigger}]' +# expect: exactly 7 entries, levels MUST x5 / SHOULD x2, enforcement_type judgment, owner go-security-specialist, trigger ["**/*.go"] + +# 7. Negative: no three-component security/... IDs; go-security count = 16 (9 existing + 7 new) +python3 scripts/build-index.py | jq '[.[] | select(.id | startswith("security/"))] | length' # expect: 0 +python3 scripts/build-index.py | jq '[.[] | select(.id | startswith("go-security/"))] | length' # expect: 16 + +# 8. Determinism — second build-index run produces byte-identical output +cp rules/index.json /tmp/index-run1.json +make build-index +diff /tmp/index-run1.json rules/index.json # expect: no output (byte-identical) + +# 9. Precommit green +make precommit # expect: exit 0 + +# 10. Scope-lock negatives — no prompt-2/3 files touched +grep -c '"Class"' scripts/build-index.py # expect: 0 (no walker Class support — prompt 2's) +grep -c '\*\*Class\*\*' docs/rule-block-schema.md # expect: 0 (no schema change — prompt 2's) +grep -Ec '^### RULE go-security/(resource-ownership|tenant-isolation)' docs/security/security-review-guide.md # expect: 0 (no invariant blocks — prompt 2's) +ls rules/security/*.yml | wc -l # expect: 5 (no detector added/removed) + +# 11. CHANGELOG entry is NOT added in this prompt (prompt 3 owns that) +awk '/^## Unreleased/{f=1;next}/^## v/{f=0}f' CHANGELOG.md | grep -c 'security' +# expect: 0 + +# 12. Final state — only the two intended files carry changes (do NOT commit) +grep -c 'security' docs/security/security-review-guide.md # sanity: guide edited +jq 'length' rules/index.json # sanity: index regenerated +``` + + + + +- **Trigger field placement (frozen):** `**Trigger**:` sits immediately after `**Enforcement**:`, before `**Why**:` — per `docs/rule-block-schema.md` and spec 011's frozen schema contract. No re-read decision needed; do not reorder. +- **No YAML detectors ship here.** None of the 7 enforcement fields cites a YAML path. `check-coverage.sh` (called by `make precommit`) must not flag this as an orphan. +- **Generic examples only.** All Bad/Good snippets use User/Order/Product/Customer-shaped entities. No trading domain. No real provider URLs in the webhook example (use `X-Signature` placeholder header). +- **Field line parsing:** `scripts/build-index.py`'s `FIELD_RE` accepts both `**Key**: value` and `Key: value` forms — bold is the existing convention; match it. +- **Prompt ordering matters.** Prompt 2 adds the `Class` field to the walker and the 2 invariant blocks; prompt 3 records the layout decision and the CHANGELOG entry. This prompt's 7 blocks stay byte-stable when the walker learns `Class` afterward. +- **DoD CHANGELOG item is deferred to prompt 3.** The injected `docs/dod.md` requires a CHANGELOG `## Unreleased` entry; that item is intentionally satisfied by prompt 3, not here. Do not report it as an unmet blocker. +- **The `crypto-insecure-random` parenthetical rewrite** must not change the YAML detection behavior — the detector still fires on every `math/rand` import; only the prose describing how the judgment adjudicator consumes those flags changes. + diff --git a/prompts/completed/048-invariant-rules-class-field.md b/prompts/completed/048-invariant-rules-class-field.md new file mode 100644 index 0000000..5014c4d --- /dev/null +++ b/prompts/completed/048-invariant-rules-class-field.md @@ -0,0 +1,276 @@ +--- +status: completed +summary: 'Extended scripts/build-index.py with **Class**: field parsing emitting an optional `class` index key, documented the field and key in docs/rule-block-schema.md, appended go-security/resource-ownership and go-security/tenant-isolation invariant RULE blocks (MUST, class: security-invariant, trigger: @commits) to docs/security/security-review-guide.md, regenerated rules/index.json to 180 entries (178 prior entries byte-stable), and added the CHANGELOG ## Unreleased entry; all verification checks pass except 12d which conflicts with the appended mandatory CHANGELOG instruction.' +execution_id: coding-security-comprehensive-rules-exec-048-invariant-rules-class-field +dark-factory-version: dev +created: "2026-08-23T20:53:15Z" +queued: "2026-08-23T20:53:15Z" +started: "2026-08-23T20:54:34Z" +completed: "2026-08-23T20:57:22Z" +branch: dark-factory/security-comprehensive-rules +--- + +# Invariant-linked authz rules + walker Class field + + +- Extend `scripts/build-index.py` with `**Class**:` field parsing so a judgment rule can carry an optional `class` index key, emitted verbatim only when present +- Document the new `**Class**:` field and the `class` index key in `docs/rule-block-schema.md` (new `### Optional Field: Class` section, schema-table row, updated example entry) +- Append 2 invariant-linked RULE blocks to `docs/security/security-review-guide.md` — `go-security/resource-ownership` (MUST) and `go-security/tenant-isolation` (MUST), both owner `go-security-specialist`, `**Class**: security-invariant`, `**Trigger**: @commits`, enforcement citing `security-review-pipeline.md` +- Regenerate `rules/index.json` from 178 to 180 entries; all 178 prior entries stay byte-identical (the `Class` field addition is provably non-perturbing) +- The 3 changes ship together: no invariant block exists without walker support, and no walker support ships without the schema documenting it +- Working-tree changes are left for the daemon's `workflow: direct` post-prompt commit; no git is run inside the container + + + +The invariant-linked authz tier ships: `scripts/build-index.py` recognizes the `**Class**:` field and emits a `class` key on the 2 invariant rules only, `docs/rule-block-schema.md` documents the field and key, the guide gains 2 schema-conformant `go-security/` blocks (`class: security-invariant`, `trigger: ["@commits"]`, enforcement citing the derived-model pipeline), and `rules/index.json` is regenerated to 180 entries with `make precommit` green and all prior entries byte-stable. + + + +Spec 011 prompt **2 of 3**. Depends on prompt 1 having shipped the 7 judgment-tier rules and reconciled the deferral prose. This prompt bundles three changes that MUST ship together because of an ordering invariant: no intermediate state can have `**Class**: security-invariant` blocks in `docs/security/security-review-guide.md` without walker support, and no walker support can ship without the schema documenting the new field. + +Read fully before writing: + +- `/workspace/CLAUDE.md` — project conventions, generic content only. +- `/workspace/docs/rule-block-schema.md` — the schema to extend. Today it documents Owner / Applies when / Enforcement (required), Trigger (optional — "immediately after `**Enforcement**:`"), Why (recommended), Bad / Good (recommended). Class does not yet exist. +- `/workspace/scripts/build-index.py` — the walker to extend. Today its `parse_fields()` field-key tuple is `("Owner", "Applies when", "Enforcement", "Trigger")` (line ~64); it maps keys via `key.lower().replace(" ", "_")`. In `walk_docs()` the trigger array block sits after field parsing, before the duplicate-ID check (lines ~152-158). The walker skips `rule-block-schema.md`, walks both `docs/*.md` and `docs/security/*.md`, detects duplicate IDs across both sets, and emits sorted byte-stable JSON. +- `/workspace/docs/security/security-review-guide.md` — contains 5 mechanical + 7 new judgment RULE blocks after prompt 1. The 2 invariant blocks land at the end here. +- `/workspace/docs/security/security-review-pipeline.md` — the procedure contract for invariant adjudication. The 2 new blocks cite this file by relative link `security-review-pipeline.md`. Read this file to write a defensible Enforcement text. +- `/workspace/scripts/validate-citations.sh` — citation gate. Resolves `kind: rule` findings against `rules/index.json` by `rule_id`; it never inspects `class`. Do not modify it. +- `/workspace/Makefile` — `make build-index` regenerates the index; `make precommit` runs check-links/check-json/check-index/check-coverage/check-acceptance/check-rule-tests/bench-test. + +The 2 new IDs (two-component `go-security/` form): + +1. `go-security/resource-ownership` (MUST) +2. `go-security/tenant-isolation` (MUST) + +Both carry `**Class**: security-invariant` and `**Trigger**: @commits` (always-on, whole-change scoping). Owner: `go-security-specialist`. Neither cites a YAML path or `scripts/rule-checks.sh`, so both derive `enforcement_type: judgment`. + + + + +### 1. Extend `scripts/build-index.py` with `Class` parsing + +Three edits to `scripts/build-index.py`, all in service of emitting an optional `class` key on index entries: + +(a) **Field-key tuple.** Add `"Class"` to the field-key tuple inside `parse_fields()`. Today the tuple is `("Owner", "Applies when", "Enforcement", "Trigger")`. The new tuple is `("Owner", "Applies when", "Enforcement", "Trigger", "Class")`. The existing `result[key.lower().replace(" ", "_")]` mapping makes the index key `class`. No other rename. Keep Python-stdlib-only (pathlib / json / re / sys). + +(b) **Index entry emission.** Inside `walk_docs()`, immediately after the trigger-array block and before the duplicate-ID check, add an analogous block: + +```python +if "class" in fields and fields["class"]: + entry["class"] = fields["class"] +``` + +The emitted JSON key must be `class` (lowercase, matching the `trigger` field's convention). When the field is absent the entry carries no `class` key — the existing 178 entries stay byte-stable. + +(c) **Determinism.** Output must remain byte-stable: when no RULE block carries `**Class**:`, the regenerated index must be byte-identical to the prompt-1 commit's index. Verify with the two-run diff in ``. + +**Do NOT** add a validator for the Class value string. v1 only ships the literal value `security-invariant`; reject no other values yet — the schema doc is the contract. + +**Do NOT** refactor `parse_fields()` beyond the field-key tuple edit. Do NOT change the duplicate-ID detection. Do NOT remove the `rule-block-schema.md` skip. + +### 2. Document `**Class**:` in `docs/rule-block-schema.md` + +Add a new subsection after the existing `### Optional Field: Trigger` section. Title: `### Optional Field: Class`. Content (target wording, adjust to match the doc's prose style): + +```markdown +### Optional Field: Class + +A small set of judgment-tier RULE blocks carry a `**Class**:` field immediately after `**Trigger**:` (or immediately after `**Enforcement**:` when no Trigger is present). The walker indexes it as a `class` string in `rules/index.json`. The dispatcher uses it to scope which owner agents consume which judgment tier — invariant-linked rules (`class: security-invariant`) require the derived session security model to fire and are scoped by `**Trigger**: @commits`. + +```markdown +**Class**: +``` + +- v1 token: `security-invariant`. Marks a rule that requires whole-repo reasoning against the derived session security model (see `docs/security/security-review-pipeline.md`). +- Missing `**Class**:` field → `class` key omitted from the index entry (no scoping applied). +- All judgment-tier rules MAY carry a Class; mechanical and script rules omit it. +``` + +Also add a row to the `rules/index.json Schema` table (the field-types table) for the new key: + +| `class` | string | Optional **Class**: field | Present only when the doc block includes a `**Class**:` line. v1 value: `security-invariant`. | + +Update the `### Anchor Derivation` section if it lists fields that emit index keys — keep the `class` mention consistent. + +Also update the JSON example entry near the bottom of the schema doc to include a `class` key in one of the example entries (use a generic `go-security/`-shaped example so the example doesn't pin a specific rule). + +**Do NOT** alter the Required Fields section. **Do NOT** alter the ID Format or Level Tokens sections. **Do NOT** alter the Anti-patterns section. + +### 3. Append 2 invariant RULE blocks to `docs/security/security-review-guide.md` + +Append after prompt 1's 7 judgment blocks. **Field order is frozen** (schema § Optional Field: Trigger + Class; spec 011 Constraint "Schema contract"): `**Owner**:` → `**Applies when**:` → `**Enforcement**:` → `**Trigger**:` → `**Class**:` → `**Why**:`. `**Trigger**:` sits immediately after `**Enforcement**:`; `**Class**:` sits after `**Trigger**:`, last field before `**Why**:`. Do not reorder, and do not vary between the two blocks. After the field block come `#### Bad` and `#### Good` code blocks. + +**Block 8 — `go-security/resource-ownership` (MUST)** +- Owner: `go-security-specialist` +- Applies when: a Go handler / service method outside `*_test.go`, `vendor/`, `mocks/` reads, mutates, or deletes a resource (DB row, file, third-party-API object) addressed by a path parameter, query parameter, body field, or header value, without first verifying that the authenticated user owns the resource. "Owns" is defined per resource by the `authorization_functions` field in the derived session security model (see `docs/security/security-review-pipeline.md`). +- Enforcement: judgment — LLM adjudicator resolves the resource's `authorization_functions` from the derived session security model per `docs/security/security-review-pipeline.md`, fires when the diff accesses a resource by identifier without the owning authorization function enforced, and emits findings as `kind=invariant` with `invariant_id` resolving in the session model. +- Trigger: `@commits` +- Class: `security-invariant` +- Why: resource-ownership gaps are the canonical IDOR / BOLA class — the attacker authenticates as a legitimate user and accesses another user's data via a guessed or harvested identifier. Generic linters cannot detect these: the missing check is the absence of an authorization call, not the presence of a forbidden call. Whole-repo reasoning against the derived model is the only enforcement path. +- Bad: + ```go + func GetOrder(w http.ResponseWriter, r *http.Request) { + orderID := chi.URLParam(r, "orderID") + order, err := orderRepo.Find(r.Context(), orderID) + if err != nil { + http.Error(w, "not found", http.StatusNotFound) + return + } + json.NewEncoder(w).Encode(order) + } + ``` +- Good: + ```go + func GetOrder(w http.ResponseWriter, r *http.Request) { + user := userFromCtx(r.Context()) + orderID := chi.URLParam(r, "orderID") + order, err := orderRepo.FindOwnedBy(r.Context(), orderID, user.ID) + if err != nil { + http.Error(w, "not found", http.StatusNotFound) + return + } + json.NewEncoder(w).Encode(order) + } + ``` + +**Block 9 — `go-security/tenant-isolation` (MUST)** +- Owner: `go-security-specialist` +- Applies when: a Go handler / service method outside `*_test.go`, `vendor/`, `mocks/` issues a query, mutation, or third-party call scoped by an account / tenant / org identifier without first verifying the authenticated user belongs to that tenant. "Belongs" is defined per tenant resource by the `authorization_functions` field in the derived session security model (see `docs/security/security-review-pipeline.md`). +- Enforcement: judgment — LLM adjudicator resolves the tenant resource's `authorization_functions` from the derived session security model per `docs/security/security-review-pipeline.md`, fires when the diff scopes the call by tenant without the owning authorization function enforced, and emits findings as `kind=invariant` with `invariant_id` resolving in the session model. +- Trigger: `@commits` +- Class: `security-invariant` +- Why: tenant isolation gaps let an authenticated user in tenant A read or mutate tenant B's data via a guessed tenant identifier — cross-tenant data leakage at scale. The authorization check is a missing-call absence, not a forbidden-call presence; whole-repo reasoning against the derived model is required. +- Bad: + ```go + func ListInvoices(w http.ResponseWriter, r *http.Request) { + tenantID := r.URL.Query().Get("tenant_id") + invoices, err := invoiceRepo.List(r.Context(), tenantID) + if err != nil { + http.Error(w, "list failed", http.StatusInternalServerError) + return + } + json.NewEncoder(w).Encode(invoices) + } + ``` +- Good: + ```go + func ListInvoices(w http.ResponseWriter, r *http.Request) { + user := userFromCtx(r.Context()) + invoices, err := invoiceRepo.ListForTenant(r.Context(), user.TenantID) + if err != nil { + http.Error(w, "list failed", http.StatusInternalServerError) + return + } + json.NewEncoder(w).Encode(invoices) + } + ``` + +### 4. Regenerate `rules/index.json` + +Run `make build-index`. Expected: `rules/index.json` grows from 178 (prompt 1 end state) to **180 entries**. The 2 new entries must: +- have `id` in `go-security/` form +- have `owner == "go-security-specialist"` +- have `doc_path == "docs/security/security-review-guide.md"` +- have `anchor == id` +- have `level == "MUST"` +- have `enforcement_type == "judgment"` +- have `trigger == ["@commits"]` +- have `class == "security-invariant"` +- non-empty `applies_when` and `enforcement` + +The 171 pre-prompt-1 entries and the 7 prompt-1 entries must be **byte-stable** — adding the `Class` field to the walker MUST NOT mutate any existing entry's bytes (verified by the two-run diff in ``). + +Run `make precommit` — must exit 0. + +### 5. Do NOT commit + +Do NOT run `git` of any kind — the container's `.git` is masked (`hideGit: true`) and dark-factory's `workflow: direct` post-prompt commit stages and commits all dirty files on completion (repo convention: "Do NOT commit — dark-factory handles git"). Touched paths expected in the daemon's commit: `scripts/build-index.py`, `docs/rule-block-schema.md`, `docs/security/security-review-guide.md`, `rules/index.json`. + + + + +- **Rule identity:** the 2 new IDs use the two-component `go-security/` form (spike Finding 1). +- **Owner:** `go-security-specialist` in every new block and index entry. +- **Schema contract (frozen):** field order `**Owner**:` → `**Applies when**:` → `**Enforcement**:` → `**Trigger**:` → `**Class**:` → `**Why**:`; `**Trigger**:` immediately after `**Enforcement**:`, `**Class**:` after `**Trigger**:`. The `class` index key is emitted only when the `**Class**:` field is present, value verbatim, v1 token `security-invariant`. +- **Walker invariants:** `scripts/build-index.py` stays Python stdlib, keeps the `rule-block-schema.md` skip, keeps duplicate-ID detection, emits byte-stable sorted output. No Class-value validator ships. +- **Enforcement-type derivation:** neither new enforcement field cites `rules//.yml` or `scripts/rule-checks.sh` → both derive `enforcement_type: judgment`; `check-coverage.sh` must not flag an orphan. +- **Extend, don't create:** all changes land in existing files; no new guide, no README/llms.txt/code-review.md changes. +- **No changes to:** `scripts/validate-citations.sh` (unchanged — `kind` rule/invariant/toolchain), `commands/*.md`, `agents/*.md`, `.maintainer.yaml`, `scenarios/`, `rules/security/`, `CHANGELOG.md` (prompt 3's). +- **Index freshness:** this prompt edits RULE blocks and the walker, so it must run `make build-index` and leave `make check-index` green — `make precommit` exits 0. +- **Generic content only:** Bad/Good examples use User, Order, Invoice — never trading or project-specific domains. +- **Git discipline:** no git inside the container (hideGit masks `.git`); the daemon owns the post-prompt commit. +- **Scope split:** the cross-language layout prose and the CHANGELOG entry belong to prompt 3; AC10 (operator-rung fixture walks) is out of scope for all prompts. + + + +All commands are container-executable (repo root). No git — `.git` is masked. + +```bash +# 1. Two invariant blocks present +grep -Ec '^### RULE go-security/(resource-ownership|tenant-isolation)' docs/security/security-review-guide.md +# expect: 2 + +# 2. Class field appears exactly twice in the guide (one per invariant block) +grep -c '\*\*Class\*\*: security-invariant' docs/security/security-review-guide.md +# expect: 2 + +# 3. Each invariant block's enforcement cites the pipeline +grep -c 'security-review-pipeline.md' docs/security/security-review-guide.md +# expect: >=1 + +# 4. Walker recognises the Class field (string literal in the field-key tuple) +grep -n '"Class"' scripts/build-index.py +# expect: >=1 line + +# 5. Schema docs document the field and the key +grep -n '\*\*Class\*\*' docs/rule-block-schema.md # expect: >=1 +grep -n '"class"' docs/rule-block-schema.md # expect: >=1 + +# 6. Regenerate, expect 180 entries +make build-index +python3 scripts/build-index.py | jq 'length' +# expect: 180 + +# 7. Two new entries have the right shape +python3 scripts/build-index.py | jq '.[] | select(.id=="go-security/resource-ownership" or .id=="go-security/tenant-isolation") | {id, level, enforcement_type, owner, class, trigger}' +# expect: 2 entries, class security-invariant, level MUST, enforcement_type judgment, owner go-security-specialist, trigger ["@commits"] + +# 8. Only the 2 invariant entries carry a `class` key (no spurious entries) +python3 scripts/build-index.py | jq '[.[] | select(has("class"))] | length' +# expect: 2 + +# 9. Negative: no three-component security/... IDs; go-security count = 18 (9 existing + 7 prompt-1 + 2 new) +python3 scripts/build-index.py | jq '[.[] | select(.id | startswith("security/"))] | length' # expect: 0 +python3 scripts/build-index.py | jq '[.[] | select(.id | startswith("go-security/"))] | length' # expect: 18 + +# 10. Determinism + byte-stability — second build-index run produces byte-identical output +cp rules/index.json /tmp/index-run1.json +make build-index +diff /tmp/index-run1.json rules/index.json # expect: no output (byte-identical — the Class addition perturbed nothing) + +# 11. Precommit green +make precommit # expect: exit 0 + +# 12. Scope-lock negatives — no out-of-scope files touched; AC9 deferral phrasing still 0 after the append +ls rules/security/*.yml | wc -l # expect: 5 +grep -cE 'follow-up task|until then' docs/security/security-review-guide.md # expect: 0 (AC9 — no deferral phrasing reintroduced) +grep -c 'rules/security/{go,python,node}' docs/security/security-review-guide.md # expect: 0 (layout prose is prompt 3's) +awk '/^## Unreleased/{f=1;next}/^## v/{f=0}f' CHANGELOG.md | grep -c 'security' # expect: 0 (CHANGELOG is prompt 3's) + +# 13. Final state — only the four intended files carry changes (do NOT commit) +grep -c 'resource-ownership\|tenant-isolation' docs/security/security-review-guide.md # sanity: guide edited +jq 'length' rules/index.json # sanity: index regenerated +``` + + + + +- **Field order for invariant blocks (frozen).** Both blocks use `Owner / Applies when / Enforcement / Trigger / Class / Why` — no re-read decision needed. Do not vary between the two blocks. +- **Byte-stability of the 178 committed entries is the critical regression check.** Adding `Class` to the walker must not perturb any other entry. If the two-run diff shows changes to entries that don't carry a `**Class**:` field, the walker edit is wrong — revert and re-apply the field-key tuple edit only. +- **Citation gate is unchanged.** `validate-citations.sh` resolves `kind: rule` findings against `rules/index.json` by `rule_id`; it never inspects `class`. Do not modify the gate. +- **No invariant scenario fixture ships here.** AC10 (the SSRF inline fixture + 007/008 walks) is operator-executable at spec-verification time, not in any prompt. No `scenarios/011-*` file. +- **Generic examples only.** The Bad/Good snippets use User/Order/Invoice-shaped entities. No trading domain. +- **The `Class` value `security-invariant` is the v1 token.** Future tokens (e.g. `architecture-decision`, `cross-cutting-concern`) are out of scope; reject them at code-review time but do not encode the rejection in the walker. +- **Prompt 3 closes the batch.** It lands CHANGELOG, the layout-decision prose, and the final gates. If anything in prompt 3 is missing, prompt 2's work is still mergeable in isolation — the spec's decomposition guarantees it. + diff --git a/prompts/completed/049-layout-decision-changelog-final-gates.md b/prompts/completed/049-layout-decision-changelog-final-gates.md new file mode 100644 index 0000000..c421a2a --- /dev/null +++ b/prompts/completed/049-layout-decision-changelog-final-gates.md @@ -0,0 +1,209 @@ +--- +status: completed +spec: [011-security-comprehensive-rules] +summary: 'Recorded the cross-language detector layout decision in the security-review-guide, reconciled the CHANGELOG ## Unreleased to a single comprehensive feat bullet, and confirmed all cross-cutting gates green (make precommit exit 0, index 180, class count 2, AC5 citations resolve non-vacuously)' +execution_id: coding-security-comprehensive-rules-exec-049-layout-decision-changelog-final-gates +dark-factory-version: dev +created: "2026-08-23T20:30:00Z" +queued: "2026-08-23T20:53:15Z" +started: "2026-08-23T20:57:24Z" +completed: "2026-08-23T20:58:55Z" +branch: dark-factory/security-comprehensive-rules +--- + +# Cross-language layout decision + CHANGELOG + final gates + + +- Record the cross-language detector layout decision (spike Finding 2) as a new `## Cross-language detector layout` prose section in `docs/security/security-review-guide.md` — per-language `rules/security/{go,python,node}/` split with a runner case mirroring the `node/frontend` special-case in `scripts/ast-grep-runner.sh`, go-first v1 stays flat, documentation only +- Perform the final guide reconciliation: any remaining tier-deferral phrasing is rewritten to the shipped state (all three tiers live in this guide) +- Insert a `## Unreleased` section above `## v0.49.0` in `CHANGELOG.md` with a single `feat:` bullet describing the comprehensive security v1 rule base (no `## vX.Y.Z` bump — the release tail is `.maintainer.yaml` autoRelease) +- Re-run the cross-cutting gates: `make precommit`, `make check-index`, index length 180, `class` count 2, no three-component IDs, non-vacuous AC5 citation fixture, `ast-grep test` green +- No new RULE blocks, no walker, no schema, no detector changes — prompts 1/2 shipped those +- Working-tree changes are left for the daemon's `workflow: direct` post-prompt commit; no git is run inside the container + + + +The comprehensive security v1 rule base is closed out: the guide records the cross-language detector layout decision and carries no deferral phrasing, `CHANGELOG.md` has a `## Unreleased` section with one `feat:` bullet for the whole feature, every cross-cutting gate is green (`make precommit`, `make check-index`, index at 180, `class` count 2, AC5 citations resolve non-vacuously), and the only modified files are `docs/security/security-review-guide.md` and `CHANGELOG.md`. + + + +Spec 011 prompt **3 of 3**. Depends on prompts 1 and 2 having shipped the 7 judgment rules + prose reconcile and the 2 invariant rules + walker `Class` field + schema doc. This prompt is the documentation + CHANGELOG + final-gates closer. No new RULE blocks land here. + +Read fully before writing: + +- `/workspace/CLAUDE.md` — project conventions, generic content only. +- `/workspace/docs/security/security-review-guide.md` — current state after prompts 1 + 2: 14 RULE blocks total (5 mechanical + 7 judgment + 2 invariant), tier-deferral prose already reconciled by prompt 1, `Class` field documented in the schema (prompt 2). +- `/workspace/docs/security/security-review-pipeline.md` — the derived security-model procedure contract (referenced, not edited). +- `/workspace/CHANGELOG.md` — convention from `docs/changelog-guide.md`. The top section today is `## v0.49.0` (the `--security` wiring release). This prompt inserts a `## Unreleased` section above `## v0.49.0` (after the SemVer preamble). +- `/workspace/docs/changelog-guide.md` (skim) — `## Unreleased` lives directly above the most-recent versioned section; conventional prefixes `feat:` / `fix:` / `refactor:` / `test:` / `docs:` / `chore:` / `perf:` required for every bullet; `-` / `*` bullet markers. +- `/workspace/specs/in-progress/011-security-comprehensive-rules.md` — the spec this prompt closes. Spike Finding 2 names the cross-language layout decision: per-language `rules/security/{go,python,node}/` split with a runner case for cross-language rules, while go-first v1 keeps single-language (Go) rules flat under `rules/security/`. +- `/workspace/scripts/ast-grep-runner.sh` — the runner that special-cases `rules/node/*` (skipped on frontend projects). The layout prose's "runner case" mirrors THIS file, not `bench/`. +- `/workspace/rules/index.json` — currently 180 entries after prompts 1 + 2. +- `/workspace/scripts/validate-citations.sh` — citation gate. Test a non-vacuous fixture to prove the gate keeps the new rule_ids (spec AC5). +- `/workspace/Makefile` — `make precommit` runs check-links/check-json/check-index/check-coverage/check-acceptance/check-rule-tests/bench-test. + +This prompt touches only: + +1. `docs/security/security-review-guide.md` — append a "Cross-language detector layout" prose section (spike Finding 2 record) and a final reconciliation touch-up if any tier-deferral phrase remains. +2. `CHANGELOG.md` — insert `## Unreleased` with a single `feat:` bullet. + + + + +### 1. Record the cross-language detector layout decision in `docs/security/security-review-guide.md` + +Append a new section at the end of the guide (after the `## Anti-patterns to refuse` section). Title: `## Cross-language detector layout`. Content (target wording, adjust to match the doc's prose style): + +```markdown +## Cross-language detector layout + +When security rules grow beyond a single language, the detector tree splits per-language under `rules/security/{go,python,node}/` (one subdir per language the rule base covers). The runner that scans a multi-language repo gains a per-language case mirroring the existing `node/frontend` special-case in `scripts/ast-grep-runner.sh` (node rules are skipped on frontend projects), dispatching each language's detectors to the matching `ast-grep scan --lang ` invocation. + +For the v1 release, security rules are go-first: every detector lives flat under `rules/security/` (no per-language subdirectories). This matches the rule base's actual coverage today — five mechanical detectors plus nine judgment / invariant rules, all Go-targeted. The split documented above is the target layout for the cross-language expansion (the python and node language cases), deferred until non-Go rules ship. + +The decision is recorded here as a spike outcome (no structural reorganization ships with this version): the cross-language split is the chosen shape; the go-first v1 stays flat because no non-Go detectors exist yet. +``` + +**Do NOT** create `rules/security/{go,python,node}/` directories or any new YAMLs. **Do NOT** modify `scripts/ast-grep-runner.sh` or any runner script. The decision is documentation only. + +### 2. Final guide reconciliation + +After the new section lands, re-grep `docs/security/security-review-guide.md` for any remaining deferral phrasing. The `grep -cE 'follow-up task|ships in a follow-up|ship.*follow-up'` and `grep -c 'until then'` counts must remain 0 — prompts 1 already removed the original deferrals, but a stray phrase from prior prose may surface. Fix any remaining hits by re-writing the surrounding sentence to describe the shipped state (all three tiers live in this guide; judgment / invariant rules are present, scoped, and emitted by the review pipeline). + +Also confirm the three-tier framing at the top of `## Tiers` lists all three tiers as live, not deferred. Do not modify paragraphs 1-3 of `## Tiers`; the fourth (the sentence prompt 1 already rewrote) may be touched only if it still reads as deferred. + +### 3. Add `## Unreleased` to `CHANGELOG.md` + +Insert a `## Unreleased` section directly above `## v0.49.0` (after the SemVer preamble block — the `* MAJOR / MINOR / PATCH` bullets). The section contains a single `feat:` bullet that summarizes the comprehensive rule base. Do not summarize each prompt — write one bullet covering the whole feature. + +Suggested bullet (target wording, adjust to the changelog's existing voice): + +```markdown +## Unreleased + +- feat: Ship the comprehensive security v1 rule base in `docs/security/security-review-guide.md` — 7 judgment-tier rules (SSRF, XSS, deserialization, open redirect, webhook verification MUST; mass assignment, insecure defaults SHOULD) and 2 invariant-linked authz rules (resource ownership, tenant isolation MUST) with `**Class**: security-invariant` and `@commits` triggers; extend `scripts/build-index.py` to emit a `class` index key and document the new field in `docs/rule-block-schema.md`; regenerate `rules/index.json` from 171 to 180 entries; record the cross-language detector layout decision (per-language `rules/security/{go,python,node}/` target, go-first v1 stays flat) +``` + +**Do NOT** include multiple bullets. One feature, one bullet — the changelog convention is one bullet per logical change, and this is one feature. + +**Do NOT** add a `## vX.Y.Z` section. The release tail is out of band (`.maintainer.yaml` `autoRelease: true` cuts v0.50.0 via the maintainer-agent-releaser after merge). This prompt ships only the `## Unreleased` entry. + +**Do NOT** include verification commands or test instructions in the bullet — those are release-check noise, not changelog content. + +### 4. Re-run the cross-cutting gates + +Run each of these from repo root and confirm exit 0 + expected output: + +```bash +# (a) make precommit — all gates green +make precommit + +# (b) make check-index — index byte-stable +make check-index + +# (c) Index length is 180 +python3 scripts/build-index.py | jq 'length' + +# (d) Exactly 2 entries carry a class key +python3 scripts/build-index.py | jq '[.[] | select(has("class"))] | length' + +# (e) No three-component security/... IDs; go-security count stays 18 +python3 scripts/build-index.py | jq '[.[] | select(.id | startswith("security/"))] | length' +python3 scripts/build-index.py | jq '[.[] | select(.id | startswith("go-security/"))] | length' + +# (f) New rule_ids resolve against the citation gate (non-vacuous AC5, 3 IDs per spec AC5) +jq -n '[{kind:"rule",rule_id:"go-security/ssrf-user-controlled-url"},{kind:"rule",rule_id:"go-security/xss-untrusted-html"},{kind:"rule",rule_id:"go-security/resource-ownership"}]' > /tmp/security-findings.json +bash scripts/validate-citations.sh /tmp/security-findings.json > /tmp/security-validated.json +jq '.findings | length' /tmp/security-validated.json # expect: 3 +jq '.dropped_count' /tmp/security-validated.json # expect: 0 + +# (g) ast-grep rule-test harness still green (no detector added, no fixture changed) +ast-grep test -c sgconfig.yml +``` + +### 5. Scope-lock negatives (AC7) + +Run these and confirm each prints the expected value (grep/ls based — no git, `.git` is masked): + +```bash +ls rules/security/*.yml | wc -l # expect: 5 (detector count unchanged) +grep -c '"Class"' scripts/build-index.py # expect: >=1 (unchanged from prompt 2 — walker Class support present, not removed) +grep -c '^### RULE go-security/' docs/security/security-review-guide.md # expect: 9 (7 judgment + 2 invariant, unchanged) +``` + +The out-of-scope files (`scripts/validate-citations.sh`, `commands/*.md`, `agents/security-verifier.md`, `agents/go-security-specialist.md`, `.maintainer.yaml`, `scenarios/`) carry no RULE-block or class content and are not touched by any requirement in this prompt — do not modify them. + +### 6. Do NOT commit + +Do NOT run `git` of any kind — the container's `.git` is masked (`hideGit: true`) and dark-factory's `workflow: direct` post-prompt commit stages and commits all dirty files on completion (repo convention: "Do NOT commit — dark-factory handles git"). Touched paths expected in the daemon's commit: `docs/security/security-review-guide.md`, `CHANGELOG.md`. + + + + +- **Documentation only:** this prompt records the layout decision and the CHANGELOG entry; no directories, runner cases, detectors, or walker/schema edits ship. +- **Layout decision scope:** the recorded decision is exactly spike Finding 2 — per-language `rules/security/{go,python,node}/` split + a runner case mirroring the `node/frontend` special-case in `scripts/ast-grep-runner.sh`; go-first v1 stays flat. No `_shared/` or other invented layout convention. +- **No changes to:** `scripts/validate-citations.sh`, `commands/*.md`, `agents/security-verifier.md`, `agents/go-security-specialist.md`, `.maintainer.yaml`, `scenarios/`, `rules/security/`, `scripts/build-index.py`, `docs/rule-block-schema.md` (all shipped/locked in prompts 1/2 or out of scope). +- **CHANGELOG:** single `## Unreleased` section above `## v0.49.0` with exactly one `feat:` bullet; no `## vX.Y.Z` bump; no verification noise in the bullet. +- **Guide reconciliation:** deferral phrasing (`follow-up task`, `ships in a follow-up`, `ship.*follow-up`, `until then`) returns 0; the `## Tiers` first three paragraphs are not modified. +- **Index gates:** `make check-index` and `make precommit` exit 0; index stays 180 entries; `class` count stays 2. +- **Git discipline:** no git inside the container (hideGit masks `.git`); the daemon owns the post-prompt commit. +- **Generic content only:** the layout prose and changelog bullet use no trading or project-specific domains. + + + +All commands are container-executable (repo root). No git — `.git` is masked. + +```bash +# 1. Cross-language layout decision recorded +grep -n 'rules/security/{go,python,node}' docs/security/security-review-guide.md +# expect: >=1 line + +# 2. Detector count unchanged (negative) +ls rules/security/*.yml | wc -l +# expect: 5 + +# 3. Precommit green +make precommit # expect: exit 0 + +# 4. Index gate green +make check-index +python3 scripts/build-index.py | jq 'length' # expect: 180 +python3 scripts/build-index.py | jq '[.[] | select(has("class"))] | length' # expect: 2 +python3 scripts/build-index.py | jq '[.[] | select(.id | startswith("security/"))] | length' # expect: 0 +python3 scripts/build-index.py | jq '[.[] | select(.id | startswith("go-security/"))] | length' # expect: 18 + +# 5. AC5 non-vacuous: 3 new rule_ids resolve via the citation gate (per spec AC5) +jq -n '[{kind:"rule",rule_id:"go-security/ssrf-user-controlled-url"},{kind:"rule",rule_id:"go-security/xss-untrusted-html"},{kind:"rule",rule_id:"go-security/resource-ownership"}]' > /tmp/security-findings.json +bash scripts/validate-citations.sh /tmp/security-findings.json > /tmp/security-validated.json +echo "exit=$?" +jq '.findings | length' /tmp/security-validated.json # expect: 3 +jq '.dropped_count' /tmp/security-validated.json # expect: 0 + +# 6. ast-grep rule-test harness still green +ast-grep test -c sgconfig.yml # expect: exit 0 + +# 7. CHANGELOG entry present +grep -c '^## Unreleased' CHANGELOG.md # expect: >=1 +awk '/^## Unreleased/{f=1;next}/^## v/{f=0}f' CHANGELOG.md | grep -c '^- feat:' # expect: >=1 + +# 8. Final guide reconciliation — no deferral phrasing +grep -cE 'follow-up task|ships in a follow-up|ship.*follow-up' docs/security/security-review-guide.md # expect: 0 +grep -c 'Judgment/invariant-tier RULE blocks in this guide' docs/security/security-review-guide.md # expect: 0 +grep -c 'until then' docs/security/security-review-guide.md # expect: 0 + +# 9. Final state — only the two intended files carry changes (do NOT commit) +grep -c 'Cross-language detector layout' docs/security/security-review-guide.md # sanity: guide edited +grep -c '^## Unreleased' CHANGELOG.md # sanity: changelog edited +``` + + + + +- **No new RULE blocks in this prompt.** All 9 new blocks ship in prompts 1 and 2. Prompt 3 is documentation + CHANGELOG + gates only. If a regression surfaces in the index, that's a bug in prompts 1 or 2 — fix in those prompts, not here. +- **AC10 (operator rung) is out of scope for this prompt.** Scenario 007 / 008 walks and the new SSRF inline fixture run on the merged plugin at spec-verification time. The management session handles the verification ladder; this prompt only commits the documentation closer. +- **The cross-language layout decision is documentation only.** Spike Finding 2 records the chosen shape but no directories ship now. Future cross-language expansion creates the directories and runner cases — separate spec. +- **CHANGELOG bullet wording.** One bullet, one feature, one `feat:` prefix. Do not list prompts 1/2/3 separately; the changelog reader cares about the feature, not the prompt sequence that built it. +- **Field order is frozen from prompts 1/2** (schema + spec 011 constraint): `**Trigger**:` after `**Enforcement**:`, `**Class**:` after `**Trigger**:`. This prompt must not re-order any block. +- **The `## Cross-language detector layout` section lives at the end of the guide.** It is a forward-looking design note, not a current-state description. Future readers hitting the spec at v0.50.0+ will read it as "this is where we're going"; readers today read it as "this is the recorded decision, not yet executed". +- **Do NOT bump the CHANGELOG to a `## vX.Y.Z` section.** Release tail is the maintainer-agent-releaser per `.maintainer.yaml` `autoRelease: true` after merge. This prompt ships only `## Unreleased`. + diff --git a/rules/index.json b/rules/index.json index 0bd04f6..f3ec3ae 100644 --- a/rules/index.json +++ b/rules/index.json @@ -1159,7 +1159,7 @@ "anchor": "go-security/crypto-insecure-random", "applies_when": "a `*.go` file outside `*_test.go`, `vendor/`, and `mocks/` imports `math/rand` or `math/rand/v2` (the Go standard library's predictable PRNGs).", "doc_path": "docs/security/security-review-guide.md", - "enforcement": "`rules/security/crypto-insecure-random.yml` (mechanical flag — fires on every import of `math/rand`/`math/rand/v2`; the judgment-tier adjudication of whether the usage is security-relevant ships with the judgment tier in a follow-up task, so the detector over-flags legitimate non-security uses by design until then)", + "enforcement": "`rules/security/crypto-insecure-random.yml` (mechanical flag — fires on every import of `math/rand`/`math/rand/v2`; the detector over-flags legitimate non-security uses by design; the judgment-tier adjudication of whether a flagged usage is security-relevant lives with the judgment rules and applies at review time)", "enforcement_type": "mechanical", "id": "go-security/crypto-insecure-random", "level": "MUST", @@ -1175,6 +1175,19 @@ "level": "MUST", "owner": "go-security-specialist" }, + { + "anchor": "go-security/deserialization-unsafe", + "applies_when": "a Go file outside `*_test.go`, `vendor/`, `mocks/` calls `json.Unmarshal` / `gob.NewDecoder` / `yaml.Unmarshal` / `xml.Unmarshal` on data received from an untrusted source (HTTP request body, message-bus payload, file uploaded by a user) into a struct without a schema gate — i.e. fields are bound directly without `json.Decoder.DisallowUnknownFields` or equivalent strict-mode flags.", + "doc_path": "docs/security/security-review-guide.md", + "enforcement": "judgment — LLM adjudicator checks the source provenance of the bytes and the presence of a strict-mode decoder. No mechanical YAML.", + "enforcement_type": "judgment", + "id": "go-security/deserialization-unsafe", + "level": "MUST", + "owner": "go-security-specialist", + "trigger": [ + "**/*.go" + ] + }, { "anchor": "go-security/dir-perms-too-permissive", "applies_when": "`os.MkdirAll($PATH, $PERM)` or `os.Mkdir($PATH, $PERM)` calls in a `*.go` file outside `*_test.go` and `vendor/`, where `$PERM` is a literal octal that is NOT `0750` / `0o750`.", @@ -1205,6 +1218,32 @@ "level": "MUST", "owner": "go-security-specialist" }, + { + "anchor": "go-security/insecure-defaults", + "applies_when": "a Go file outside `*_test.go`, `vendor/`, `mocks/` ships a security-relevant default (TLS min version, cookie `Secure`/`HttpOnly`/`SameSite`, password hashing cost, session timeout, CORS wildcard, CSP `unsafe-inline`) at a value weaker than the secure baseline, instead of failing closed to the secure value.", + "doc_path": "docs/security/security-review-guide.md", + "enforcement": "judgment — LLM adjudicator compares the shipped default against the secure baseline (e.g. `MinVersion: tls.VersionTLS12`, cookie `Secure`/`HttpOnly`/`SameSite` set, non-wildcard CORS, no `unsafe-inline` CSP) and flags any security-relevant default weaker than it. No mechanical YAML.", + "enforcement_type": "judgment", + "id": "go-security/insecure-defaults", + "level": "SHOULD", + "owner": "go-security-specialist", + "trigger": [ + "**/*.go" + ] + }, + { + "anchor": "go-security/mass-assignment", + "applies_when": "a Go file outside `*_test.go`, `vendor/`, `mocks/` binds an HTTP request body or query directly into a domain struct that carries authorization-relevant fields (role flags, ownership pointers, billing status) without an explicit allow-list of bindable fields.", + "doc_path": "docs/security/security-review-guide.md", + "enforcement": "judgment — LLM adjudicator checks the struct-to-DTO separation and the bind path. No mechanical YAML.", + "enforcement_type": "judgment", + "id": "go-security/mass-assignment", + "level": "SHOULD", + "owner": "go-security-specialist", + "trigger": [ + "**/*.go" + ] + }, { "anchor": "go-security/nosec-requires-reason", "applies_when": "a `// #nosec ` comment in a `*.go` file outside `*_test.go` and `vendor/` appears WITHOUT a `-- ` text component on the same line.", @@ -1215,6 +1254,33 @@ "level": "MUST", "owner": "go-security-specialist" }, + { + "anchor": "go-security/open-redirect", + "applies_when": "a Go HTTP handler outside `*_test.go`, `vendor/`, `mocks/` reads a `next` / `return_to` / `redirect` query parameter (or similar) and forwards the user to the parsed URL via `http.Redirect` / `http.RedirectHandler` / manual `Location:` header without an allow-list of permitted hosts / paths.", + "doc_path": "docs/security/security-review-guide.md", + "enforcement": "judgment — LLM adjudicator checks the data flow from the request parameter to the redirect target. No mechanical YAML.", + "enforcement_type": "judgment", + "id": "go-security/open-redirect", + "level": "MUST", + "owner": "go-security-specialist", + "trigger": [ + "**/*.go" + ] + }, + { + "anchor": "go-security/resource-ownership", + "applies_when": "a Go handler / service method outside `*_test.go`, `vendor/`, `mocks/` reads, mutates, or deletes a resource (DB row, file, third-party-API object) addressed by a path parameter, query parameter, body field, or header value, without first verifying that the authenticated user owns the resource. \"Owns\" is defined per resource by the `authorization_functions` field in the derived session security model (see `docs/security/security-review-pipeline.md`).", + "class": "security-invariant", + "doc_path": "docs/security/security-review-guide.md", + "enforcement": "judgment — LLM adjudicator resolves the resource's `authorization_functions` from the derived session security model per `docs/security/security-review-pipeline.md`, fires when the diff accesses a resource by identifier without the owning authorization function enforced, and emits findings as `kind=invariant` with `invariant_id` resolving in the session model.", + "enforcement_type": "judgment", + "id": "go-security/resource-ownership", + "level": "MUST", + "owner": "go-security-specialist", + "trigger": [ + "@commits" + ] + }, { "anchor": "go-security/sql-string-interpolation", "applies_when": "a `*.go` file outside `*_test.go`, `vendor/`, and `mocks/` calls `$DB.QueryContext`, `$DB.Query`, `$DB.ExecContext`, or `$DB.Exec` with a statement argument built by string concatenation.", @@ -1225,6 +1291,33 @@ "level": "MUST", "owner": "go-security-specialist" }, + { + "anchor": "go-security/ssrf-user-controlled-url", + "applies_when": "a Go file outside `*_test.go`, `vendor/`, `mocks/` issues an outbound HTTP request (`http.Get`, `http.NewRequest`, `http.Client.Do`) where the URL or host is derived from a request parameter, header, body field, or other user-controlled source without an allow-list / scheme-and-host validation step.", + "doc_path": "docs/security/security-review-guide.md", + "enforcement": "judgment — LLM adjudicator checks the request URL's data flow back to a user-controlled source. No mechanical YAML.", + "enforcement_type": "judgment", + "id": "go-security/ssrf-user-controlled-url", + "level": "MUST", + "owner": "go-security-specialist", + "trigger": [ + "**/*.go" + ] + }, + { + "anchor": "go-security/tenant-isolation", + "applies_when": "a Go handler / service method outside `*_test.go`, `vendor/`, `mocks/` issues a query, mutation, or third-party call scoped by an account / tenant / org identifier without first verifying the authenticated user belongs to that tenant. \"Belongs\" is defined per tenant resource by the `authorization_functions` field in the derived session security model (see `docs/security/security-review-pipeline.md`).", + "class": "security-invariant", + "doc_path": "docs/security/security-review-guide.md", + "enforcement": "judgment — LLM adjudicator resolves the tenant resource's `authorization_functions` from the derived session security model per `docs/security/security-review-pipeline.md`, fires when the diff scopes the call by tenant without the owning authorization function enforced, and emits findings as `kind=invariant` with `invariant_id` resolving in the session model.", + "enforcement_type": "judgment", + "id": "go-security/tenant-isolation", + "level": "MUST", + "owner": "go-security-specialist", + "trigger": [ + "@commits" + ] + }, { "anchor": "go-security/tls-insecure-skip-verify", "applies_when": "a `tls.Config` composite literal sets `InsecureSkipVerify: true` in a `*.go` file outside `*_test.go`, `vendor/`, and `mocks/`.", @@ -1235,6 +1328,32 @@ "level": "MUST", "owner": "go-security-specialist" }, + { + "anchor": "go-security/webhook-verification", + "applies_when": "a Go HTTP handler outside `*_test.go`, `vendor/`, `mocks/` exposes a webhook-receiving endpoint that processes the request body without verifying a provider signature (`X-Hub-Signature-256` / `Stripe-Signature` / equivalent HMAC header) before treating the payload as trusted.", + "doc_path": "docs/security/security-review-guide.md", + "enforcement": "judgment — LLM adjudicator checks the signature-verification step preceding payload processing. No mechanical YAML.", + "enforcement_type": "judgment", + "id": "go-security/webhook-verification", + "level": "MUST", + "owner": "go-security-specialist", + "trigger": [ + "**/*.go" + ] + }, + { + "anchor": "go-security/xss-untrusted-html", + "applies_when": "a Go file outside `*_test.go`, `vendor/`, `mocks/` writes user-supplied data into an HTML response (template `html/template` is fine; `text/template`, `fmt.Fprintf(w, ...)`, raw concatenation into HTML is not) without HTML-escaping or a sanitizer.", + "doc_path": "docs/security/security-review-guide.md", + "enforcement": "judgment — LLM adjudicator checks the response writer and the data-flow provenance of the interpolated value.", + "enforcement_type": "judgment", + "id": "go-security/xss-untrusted-html", + "level": "MUST", + "owner": "go-security-specialist", + "trigger": [ + "**/*.go" + ] + }, { "anchor": "go-service-impl/no-context-object-injection", "applies_when": "a Go service method receives a struct (by value OR by pointer) named `Context` / `ServiceContext` / `Deps` / etc. that bundles multiple service dependencies (logger, repository, validator, etc.) and passes them through method calls instead of through the constructor.", diff --git a/scripts/build-index.py b/scripts/build-index.py index 7034c75..19d170a 100755 --- a/scripts/build-index.py +++ b/scripts/build-index.py @@ -41,9 +41,9 @@ def validate_rule_line(line: str, doc_path: str) -> None: def parse_fields(doc_path: str, rule_id: str, lines): - """Extract Owner, Applies when, Enforcement, and optional Trigger from field lines. + """Extract Owner, Applies when, Enforcement, optional Trigger, and optional Class from field lines. - Returns a dict with keys: owner, applies_when, enforcement, and optionally trigger. + Returns a dict with keys: owner, applies_when, enforcement, and optionally trigger and class. Exits via sys.exit on missing required field. """ result = {} @@ -61,7 +61,7 @@ def parse_fields(doc_path: str, rule_id: str, lines): key = m.group(3).strip() value = m.group(4).strip() - if key in ("Owner", "Applies when", "Enforcement", "Trigger"): + if key in ("Owner", "Applies when", "Enforcement", "Trigger", "Class"): result[key.lower().replace(" ", "_")] = value required = ["owner", "applies_when", "enforcement"] @@ -155,6 +155,10 @@ def walk_docs(docs_dir: pathlib.Path) -> list[dict]: if trigger_list: entry["trigger"] = trigger_list + # Parse optional Class field into a class string (v1 token: security-invariant) + if "class" in fields and fields["class"]: + entry["class"] = fields["class"] + # Check duplicate ID if rule_id in seen_ids: print( diff --git a/specs/completed/011-security-comprehensive-rules.md b/specs/completed/011-security-comprehensive-rules.md new file mode 100644 index 0000000..2ac4101 --- /dev/null +++ b/specs/completed/011-security-comprehensive-rules.md @@ -0,0 +1,153 @@ +--- +status: completed +tags: + - dark-factory + - spec +approved: "2026-08-23T20:25:32Z" +generating: "2026-08-23T19:58:53Z" +prompted: "2026-08-23T20:42:35Z" +verifying: "2026-08-23T20:58:55Z" +completed: "2026-08-23T21:22:40Z" +branch: dark-factory/security-comprehensive-rules +--- + +## Summary + +- Ship the comprehensive security v1 rule base: 7 judgment-tier RULE blocks (SSRF, XSS/untrusted-html, deserialization, open redirect, webhook verification — MUST; mass assignment, insecure defaults — SHOULD) plus 2 invariant-linked authz rules (resource ownership, tenant isolation — MUST) in `docs/security/security-review-guide.md`, all owner `go-security-specialist`. +- Add a new optional `**Class**: security-invariant` field to the two authz rules; extend `scripts/build-index.py` to emit a `class` index key, and document the new field + key in `docs/rule-block-schema.md`. +- All 9 new rules use the two-component `go-security/` ID form (spike Finding 1); `rules/index.json` is regenerated (171 → 180 entries) and every citation still validates against the unchanged `validate-citations.sh` contract. +- Record the cross-language detector layout decision (spike Finding 2) in the guide — per-language `rules/security/{go,python,node}/` split + a runner case for cross-language rules, with go-first v1 staying flat — as documentation only, no structural reorganization. +- Definition of Done: an invariant-linked rule (existing IDOR scenarios 007/008) and a new judgment-tier SSRF fixture are each exercised at spec-verification time (finding produced, verified, or correctly rejected). + +## Problem + +The foundational mechanical security rule base (v0.46-0.49) ships 5 MUST-level ast-grep detectors and the guide states the judgment and invariant tiers "ship in a follow-up task" — this spec is that follow-up. Without it, security review mode can only emit findings on mechanical shapes (hardcoded secrets, TLS bypass, weak crypto, SQL interpolation). The judgment-tier rules (SSRF, XSS, deserialization, open redirect, webhook verification, mass assignment, insecure defaults) and especially the invariant-linked authz rules (resource ownership, tenant isolation) are what make security review deliver exploitable findings on real apps — authorization and business-logic gaps are the differentiator generic linters miss. The rule infrastructure also has no concept of an invariant-linked rule: the index schema and walker have no `class` field, so the two authz rules that "fire with the derived security model" cannot be marked as such. + +## Goal + +After this work, `docs/security/security-review-guide.md` is the complete v1 security rule base: the 5 mechanical rules, the 7 judgment-tier rules, and the 2 invariant-linked authz rules — every block owner `go-security-specialist`, every ID in the two-component `go-security/` form, every judgment-tier block carrying a `**Trigger**:` field, and both authz blocks carrying `**Class**: security-invariant`. `rules/index.json` holds 180 entries and passes `make check-index`; `scripts/build-index.py` emits a `class` key for invariant-linked rules; `docs/rule-block-schema.md` documents the new field and key. The guide records the cross-language detector layout decision. Every rule finding resolves against the regenerated index (invariant findings against the derived session model), so no invented security policy passes the citation gate. The Definition of Done holds: one invariant-linked rule and one judgment-tier rule are each proven on a fixture. + +## Non-goals + +- Do NOT ship new mechanical detectors — all 9 new rules are judgment-tier (LLM-adjudicated); `rules/security/` keeps exactly its 5 flat go-first YAML detectors. +- Do NOT structurally reorganize `rules/security/` into per-language subdirectories — the split decision is recorded in the guide only. +- Do NOT touch the `go-security-specialist` agent (task 5, parallel session), the verifier agent, `scripts/validate-citations.sh`, any `commands/*.md`, or `.maintainer.yaml`. +- Do NOT add runtime/network probing or new languages beyond go-first v1. +- Do NOT create a new guide — extending the already-registered `docs/security/security-review-guide.md` avoids the README/llms.txt/code-review.md "Adding a new guide" checklist. +- Do NOT add any config knob, opt-out flag, or tunable threshold to the rules or the walker — the schema fields (`level`, `trigger`, `class`) are the only acceptance surface; a future consumer demanding variation is a separate spec. + +## Acceptance Criteria + +**Scenario coverage: NO new scenario file.** The two invariant/IDOR fixtures already exist as scenarios 007 and 008 and are walkable (the `--security` wiring they depend on shipped in v0.49.0); the judgment-tier rule is exercised via an inline verification-time fixture in the operator rung, mirroring the scenario setup pattern — no new `scenarios/*.md` ships from this spec. A committed `scenarios/011` is deliberately not added: the judgment-tier SSRF path is exercised by the same in-place fixture pattern the operator rung runs, the invariant-kind path is already covered by committed walkable scenarios 007/008, AC7 locks `scenarios/` untouched for this spec, and the inline fixture is re-runnable at verification time producing identical evidence — a committed scenario adds no signal a walkable inline fixture cannot reproduce. + +- [ ] **AC1 — Judgment-tier blocks schema-conformant:** `grep -Ec '^### RULE go-security/(ssrf-user-controlled-url|xss-untrusted-html|deserialization-unsafe|open-redirect|webhook-verification|mass-assignment|insecure-defaults)' docs/security/security-review-guide.md` returns 7; `python3 scripts/build-index.py | jq '[.[] | select(.id | test("go-security/(ssrf-user-controlled-url|xss-untrusted-html|deserialization-unsafe|open-redirect|webhook-verification|mass-assignment|insecure-defaults)")) | {id, level, enforcement_type, owner, trigger}]'` lists exactly 7 entries — `ssrf-user-controlled-url`, `xss-untrusted-html`, `deserialization-unsafe`, `open-redirect`, `webhook-verification` with `"level": "MUST"`; `mass-assignment`, `insecure-defaults` with `"level": "SHOULD"`; all with `"enforcement_type": "judgment"`, `"owner": "go-security-specialist"`, and a non-empty `trigger` array; `grep -c '^### RULE ' docs/security/security-review-guide.md` returns 14 (5 existing + 9 new) and `grep -c '\*\*Why\*\*' docs/security/security-review-guide.md` returns ≥14, `grep -c '^#### Bad' docs/security/security-review-guide.md` returns ≥14, `grep -c '^#### Good' docs/security/security-review-guide.md` returns ≥14 (prose blocks not fakeable). Evidence: grep count + stdout JSON content. +- [ ] **AC2 — Invariant-linked authz rules marked and gated:** `grep -Ec '^### RULE go-security/(resource-ownership|tenant-isolation)' docs/security/security-review-guide.md` returns 2; `grep -c '\*\*Class\*\*: security-invariant' docs/security/security-review-guide.md` returns 2; `python3 scripts/build-index.py | jq '.[] | select(.id=="go-security/resource-ownership" or .id=="go-security/tenant-isolation") | {id, level, enforcement_type, owner, class, trigger}'` returns 2 entries each with `"class": "security-invariant"`, `"level": "MUST"`, `"enforcement_type": "judgment"`, `"owner": "go-security-specialist"`, and `"trigger": ["@commits"]`; `grep -c 'security-review-pipeline.md' docs/security/security-review-guide.md` returns ≥1 (each block's enforcement cites the derived-model procedure). Evidence: grep counts + stdout JSON. +- [ ] **AC3 — Walker and schema learn the Class field:** `grep -n '"Class"' scripts/build-index.py` returns line ≥1 (the string literal in the field-parse key tuple — gates the generic parse, not a comment or a hardcoded two-ID special-case); `python3 scripts/build-index.py | jq '[.[] | select(has("class"))] | length'` returns exactly 2 (no other entry gains a `class` key); `grep -n '\*\*Class\*\*' docs/rule-block-schema.md` returns line ≥1 and `grep -n '"class"' docs/rule-block-schema.md` returns line ≥1 (schema documents the field and the index key). Evidence: grep hits + jq length. +- [ ] **AC4 — Index regenerated, IDs uniform, nothing removed:** `make check-index` exits 0; `python3 scripts/build-index.py | jq 'length'` returns 180; `python3 scripts/build-index.py | jq '[.[] | select(.id | startswith("security/"))] | length'` returns 0 (negative: no three-component `security/...` ID anywhere); `git diff rules/index.json | grep -cE '^-.*"id": "go-security/'` returns 0 (negative: no existing security rule ID removed). Evidence: exit code + stdout counts + diff grep. +- [ ] **AC5 — New rule citations validate (non-vacuous):** `jq -n '[{kind:"rule",rule_id:"go-security/ssrf-user-controlled-url"},{kind:"rule",rule_id:"go-security/xss-untrusted-html"},{kind:"rule",rule_id:"go-security/resource-ownership"}]' > /tmp/security-findings.json && bash scripts/validate-citations.sh /tmp/security-findings.json > /tmp/security-validated.json` exits 0; `jq '.findings | length' /tmp/security-validated.json` returns 3 and `jq '.dropped_count' /tmp/security-validated.json` returns 0 — the 3 new rule_ids demonstrably resolve against the regenerated index (an empty findings fixture cannot pass). Evidence: exit code + jq counts. +- [ ] **AC6 — Cross-language layout decision recorded, no reorganization (negative):** `grep -n 'rules/security/{go,python,node}' docs/security/security-review-guide.md` returns line ≥1 (the per-language split + runner case is documented); `ls rules/security/*.yml | wc -l` returns 5 (negative: detector count unchanged); `git status --short -- rules/security/` prints nothing (negative: no tracked change to `rules/security/`). Evidence: grep hit + counts + git status. +- [ ] **AC7 — Out-of-scope files untouched (negative):** `git status --short` lists no modification to `scripts/validate-citations.sh`, no `commands/*.md`, no `agents/security-verifier.md`, no `agents/go-security-specialist.md`, no `.maintainer.yaml`, and no `scenarios/`. Evidence: git status output empty for those paths. +- [ ] **AC8 — Precommit green, CHANGELOG entry present:** `make precommit` exits 0; `grep -c '^## Unreleased' CHANGELOG.md` returns ≥1; `awk '/^## Unreleased/{f=1;next}/^## v/{f=0}f' CHANGELOG.md | grep -c '^- feat:'` returns ≥1. Evidence: exit code + grep counts. +- [ ] **AC9 — Guide reconciled, no deferred-tier claims (negative):** `grep -cE 'follow-up task|ships in a follow-up|ship.*follow-up' docs/security/security-review-guide.md` returns 0; `grep -c 'Judgment/invariant-tier RULE blocks in this guide' docs/security/security-review-guide.md` returns 0 (the anti-pattern that refused judgment/invariant blocks here is gone); `grep -c 'until then' docs/security/security-review-guide.md` returns 0 (the crypto-insecure-random note no longer defers judgment-tier adjudication). Evidence: grep counts (all negative). +- [ ] **AC10 — DoD exercised at verification time (operator-executable):** scenario 007 walk exits 0 with `grep -c '"confidence": "confirmed"' /tmp/scen007-stdout.log` ≥ 1 and `grep -c '"blocking": true' /tmp/scen007-stdout.log` ≥ 1; scenario 008 walk exits 0 with `grep -c '"confidence": "rejected"' /tmp/scen008-stdout.log` ≥ 1 and `grep -c '"blocking": true' /tmp/scen008-stdout.log` returns 0; the new SSRF fixture run (Verification operator rung) produces a report whose findings cite `go-security/ssrf-user-controlled-url`, which `validate-citations.sh` keeps (resolves in `rules/index.json`), with verifier verdict `confirmed` or `plausible`. Evidence: scenario exit codes + grep counts on scenario logs + fixture report grep. + +## Verification + +## Container-executable (runs inside the YOLO container at prompt time) + +- `make precommit` — exits 0 (check-links, check-json, check-index, check-coverage, check-acceptance, check-rule-tests, bench-test). +- `make check-index` — exits 0 (committed `rules/index.json` byte-matches the live derivation). +- `python3 scripts/build-index.py | jq 'length'` — prints 180. +- `python3 scripts/build-index.py | jq '[.[] | select(has("class"))] | length'` — prints 2. +- `jq -n '[{kind:"rule",rule_id:"go-security/ssrf-user-controlled-url"},{kind:"rule",rule_id:"go-security/xss-untrusted-html"},{kind:"rule",rule_id:"go-security/resource-ownership"}]' > /tmp/security-findings.json && bash scripts/validate-citations.sh /tmp/security-findings.json > /tmp/security-validated.json && jq '.findings | length, .dropped_count' /tmp/security-validated.json` — prints `3` then `0` (AC5 non-vacuous). +- The jq / grep / `validate-citations.sh` checks named in AC1-AC7, AC9. +- `ast-grep test -c sgconfig.yml` — exits 0 (existing rule-test harness; this spec adds no detectors). + +## Operator-executable (runs on the host after PR merge, spec verification ladder) + +- `make release-check` — precommit + check-versions clean, run before tagging. +- **Invariant-linked DoD:** walk `scenarios/007-security-idor-confirmed.md` and `scenarios/008-security-idor-rejected-by-verifier.md` in fresh Claude Code sessions (both are walkable — the `--security` wiring shipped in v0.49.0). 007 ends exit 0 with `"confidence": "confirmed"` and `"blocking": true`; 008 ends exit 0 with `"confidence": "rejected"` and no `"blocking": true` (evidence grep counts per AC10). +- **Judgment-tier DoD (new fixture):** scaffold a generic Go app under `$WORK` — `go mod init example.com/url-app`, a handler `pkg/handler/fetch.go` exposing `GET /fetch?url=...` that forwards the user-supplied URL to `http.Get(userURL)` with no scheme/SSRF mitigation — then run `/coding:local-review --security` over `$WORK` (in-place, plugin pinned to the branch under test, mirroring the scenario 007/008 setup). Confirm the report emits a finding citing `go-security/ssrf-user-controlled-url`; `bash scripts/validate-citations.sh` keeps it (rule_id resolves in `rules/index.json`); the verifier verdict is `confirmed` or `plausible`. The run happens against the installed plugin at spec-verification time (after the review-mode wiring and the task-5 agent are live). +- **Release tail (out of band, not gated here):** after merge, `.maintainer.yaml` (`autoRelease: true`) cuts v0.50.0 via the maintainer-agent-releaser; confirm the released version via `claude plugin list`. + +## Desired Behavior + +1. **Judgment-tier rule base authored.** `docs/security/security-review-guide.md` gains 7 new `### RULE go-security/ (LEVEL)` blocks — `ssrf-user-controlled-url`, `xss-untrusted-html`, `deserialization-unsafe`, `open-redirect`, `webhook-verification` (MUST), `mass-assignment`, `insecure-defaults` (SHOULD) — each conforming to `docs/rule-block-schema.md` (field order Owner → Applies when → Enforcement, then `**Trigger**: **/*.go`, `**Why**:`, `#### Bad`/`#### Good` generic examples). Each block's enforcement cites no `rules//.yml` path, so each derives `enforcement_type: judgment`. +2. **Invariant-linked authz rules authored.** The guide gains `### RULE go-security/resource-ownership (MUST)` and `### RULE go-security/tenant-isolation (MUST)`, each carrying `**Class**: security-invariant` (after `**Trigger**:`, last field line before `**Why**:`), `**Trigger**: @commits` (always-run whole-change, matching the architecture-tier semantics the pipeline documents), and an enforcement field that describes LLM adjudication against the derived session security model: resolve the resource's `authorization_functions` from the model per `docs/security/security-review-pipeline.md`, fire when the diff accesses a resource by identifier without the owning authorization function enforced, and emit findings as `kind=invariant` with `invariant_id` resolving in the session model. +3. **Index walker learns the Class field.** `scripts/build-index.py` recognizes a `**Class**:` field line and emits a `class` key in the index entry with the field value verbatim; when the field is absent the entry carries no `class` key, so the existing 171 entries stay byte-stable and `check-index` stays deterministic. +4. **Schema reference updated.** `docs/rule-block-schema.md` documents the optional `**Class**:` field — placement (after `**Trigger**:`, or after `**Enforcement**:` when no Trigger), v1 value `security-invariant` — and the `class` index key it feeds. +5. **Index regenerated and citations validated.** `make build-index` regenerates `rules/index.json` to 180 entries: the 7 judgment rules (5 MUST, 2 SHOULD), the 2 invariant-linked rules (`class: security-invariant`), all owner `go-security-specialist`, all IDs in the two-component `go-security/` form. `make check-index` passes; `validate-citations.sh` resolves findings citing the new rule_ids against the regenerated index (unchanged gate — rule_id ∈ index). +6. **Cross-language layout decision recorded.** The guide documents spike Finding 2's decision: cross-language security rules split per language under `rules/security/{go,python,node}/` with a runner case mirroring the node/frontend one, while go-first v1 keeps single-language (Go) rules flat under `rules/security/`. The decision is recorded as prose; no directories, runner cases, or detectors are created now. +7. **Guide reconciled to the shipped state.** The "judgment and invariant tiers ship in a follow-up task" sentence (line 13), the anti-patterns item refusing judgment/invariant blocks in this guide, and the crypto-insecure-random note deferring judgment-tier adjudication "until then" are all replaced with text describing the complete v1 base as shipped. The three-tier framing (mechanical / judgment / invariant) stays and now lists all tiers as live. +8. **Ship readiness.** CHANGELOG gains a `## Unreleased` section with a `feat:` bullet describing the comprehensive rule base (the `changelog/unreleased-entry-required` and `changelog/conventional-prefix-required` gates stay green); `make precommit` exits 0 after every prompt; the `.maintainer.yaml` autoRelease tail (v0.50.0) is left to the releaser. + +## Constraints + +- **Rule identity:** all 9 new IDs use the two-component `go-security/` form (spike Finding 1) — never the design's three-component `security//` form. The 9 existing security rule IDs (`go-security/*` in `docs/go-security-linting.md` and the 5 mechanical ones) are unchanged. +- **Owner:** `go-security-specialist` in every new block and index entry. +- **Schema contract (frozen):** field order Owner → Applies when → Enforcement; `**Trigger**:` immediately after `**Enforcement**:`; judgment-tier rules MUST carry a Trigger, mechanical rules omit it. The 7 judgment rules carry `**Trigger**: **/*.go`; the 2 invariant rules carry `**Trigger**: @commits`. The new `**Class**:` field sits after `**Trigger**:`; its v1 value is exactly `security-invariant`. +- **Enforcement-type derivation:** none of the 9 new enforcement fields cites a `rules//.yml` path or `scripts/rule-checks.sh`, so every new entry derives `enforcement_type: judgment` — and `check-coverage.sh` sees no orphan YAML (no new YAML files exist). +- **Walker invariants:** `scripts/build-index.py` stays Python stdlib, keeps the `rule-block-schema.md` skip, keeps duplicate-ID detection across the walked sets, and emits byte-stable sorted output. +- **Pipeline guide untouched:** `docs/security/security-review-pipeline.md` is a procedure contract with zero RULE blocks and is not modified; the invariant rules reference it by relative link (`security-review-pipeline.md`), they do not add blocks to it. +- **Extend, don't create:** all new blocks and the layout-decision prose land in the existing `docs/security/security-review-guide.md`; no new guide is created (no README/llms.txt/code-review.md checklist). +- **No changes to:** `scripts/validate-citations.sh` (already `kind` rule/invariant/toolchain), `commands/*.md`, `agents/security-verifier.md`, `agents/go-security-specialist.md`, `.maintainer.yaml`, `scenarios/`. +- **Index freshness:** any prompt that edits a RULE block also runs `make build-index` and commits the result — `make check-index` fails otherwise; `make precommit` exits 0 after every prompt. +- **Generic content only:** Bad/Good examples use User, Order, Product, Customer — never trading or project-specific domains. +- **Release tail:** `.dark-factory.yaml` sets `autoRelease: false` (dark-factory commits locally); the v0.50.0 release is cut by the maintainer-agent-releaser per `.maintainer.yaml` (`autoRelease: true`) — out of band. This spec requires only the CHANGELOG `## Unreleased` entry. +- **Approval discipline:** never edit frontmatter status manually — approval goes through `dark-factory spec approve` with explicit user confirmation. + +## Failure Modes + +| Trigger | Expected behavior | Recovery | Detection | Reversibility | Concurrency | +|---------|-------------------|----------|-----------|---------------|-------------| +| Guide edited without regenerating the index | `check-index` fails precommit ("rules/index.json is stale") | Run `make build-index` in the same prompt that edited the guide, commit the result | `make precommit` exits non-zero on `check-index` | Reversible — derived artifact, regenerable | Last-writer-wins if two prompts regen concurrently; mitigated by prompt ordering 1→2→3 and the check-index gate | +| `build-index.py` Class support regresses existing extraction (a non-Class entry gains a `class` key, or field parsing breaks) | `check-index` diff shows existing entries changed, or build-index exits 1 | Revert the parse change; confirm existing 171 entries derive byte-identically; re-apply | `git diff` on `/tmp/coding-rules-index-check.json`; build-index stderr | Reversible | Sequential prompts only | +| Malformed RULE block (bad heading, wrong field order, missing required field, duplicate ID) | build-index exits 1 naming the doc and rule | Fix the block to match `rule-block-schema.md`, re-run `make build-index` | build-index stderr line with `doc_path` + rule | Reversible | — | +| A judgment rule ships without `**Trigger**:` | Index entry lacks a `trigger` array; the dispatcher never scopes the rule, so it can silently never fire | Add `**Trigger**:` and regenerate | jq audit (AC1 `trigger` arrays) + index review | Reversible | — | +| `ast-grep`/`jq` missing in the container | `check-rule-tests` / `check-acceptance` fail fail-closed ("command not found") | Install `ast-grep` (the existing `check-acceptance` preflight names the install command) | `make precommit` exits non-zero | Reversible | — | +| Judgment-tier LLM rule silently stops producing findings on real PRs | No finding emitted for a real SSRF/XSS/IDOR pattern; no CI gate can see it | Operator re-runs the AC10 SSRF fixture and the 007/008 walks at spec-verification time | Operator rung fixture run | Reversible | — | +| An invariant rule fires but the finding's `invariant_id` does not resolve in the session model (enforcement misread or invented invariant) | `validate-citations.sh` drops the finding with a WARN and exits 1 (fail-closed: no model, no invariant findings) | Fix the enforcement text to describe adjudication against the derived model's invariants per the pipeline doc | `WARN: dropped … kind=invariant` on stderr | Reversible | Session-local models — no shared state to race on | + +## Security / Abuse Cases + +The feature touches files (guide, schema, walker, index) and adds judgment rules the LLM adjudicator applies to untrusted consumer PR code. + +- **Attacker-controlled input:** consumer source code is data the LLM adjudicator reads; it is never executed and never parsed as code (the pipeline guide's security contract). The RULE-block text, `**Trigger**:` globs, and `**Class**:` value are repo-controlled constants, never derived from scanned code — no injection path into the rules, walker, or index. +- **Trust boundary:** `build-index.py` reads only repo-owned `docs/` files; the index is derived from trusted repo state. The invariant rules' adjudication consumes the session security model, whose `file:line` evidence strings are display-only references — enforcement text must keep that contract. +- **What can hang / retry forever / race:** none — no new network, no loops, no retries; the only external tool is `ast-grep` (bounded single-shot). The session model is session-local (pipeline lifecycle), so concurrent reviews do not share state. +- **Input validation:** RULE IDs validated by build-index (lowercase, ≥2 slash components, level tokens MUST/SHOULD/MAY); duplicate-ID detection spans both walked doc sets; the `class` value is emitted verbatim from the trusted field with no normalization path. +- **Provenance integrity:** every rule finding must cite a `rule_id` in the regenerated index; every invariant finding must cite an `invariant_id` in the session model. `validate-citations.sh` is unchanged and remains the gate — invented security policy is rejected. + +## Suggested Decomposition + +Prompts generated in this order — each row is one prompt with an auditable scope. Ordering keeps `make precommit` green after every prompt: every prompt that adds RULE blocks also regenerates the index (the check-index gate is why guide authoring and index regen are bundled, per the spec-007 lesson). + +| # | Prompt focus | Covers DBs | Covers ACs | Depends on | +|---|---|---|---|---| +| 1 | Author the 7 judgment-tier RULE blocks in the guide (levels, Trigger, Bad/Good, Why) + reconcile judgment-tier prose (crypto-insecure-random note, "judgment tier ships in a follow-up" sentence) + `make build-index` regen (178 entries) + commit | 1, 5, 7 | 1, 4, 9 | — | +| 2 | Extend `scripts/build-index.py` with `Class` parsing → `class` key + document the field and key in `docs/rule-block-schema.md` + author the 2 invariant-linked RULE blocks (`**Class**: security-invariant`, `**Trigger**: @commits`, enforcement citing the pipeline) + `make build-index` regen (180 entries) + commit | 2, 3, 4, 5, 7 | 2, 3, 4, 9 | prompt 1 (shared guide file — sequential edits avoid conflicts; regen pattern established) | +| 3 | Record the cross-language layout decision in the guide + final guide reconciliation + CHANGELOG `## Unreleased` feat entry + final cross-cutting gates (`make precommit`, all negative scope-lock ACs, citation validation) | 6, 7, 8 | 5, 6, 7, 8 | prompts 1, 2 | + +Rationale: prompt 1 lands the judgment-tier surface with no dependency on the Class mechanism. Prompt 2 bundles the walker's `Class` support, the schema documentation, and the two blocks that use `Class` in one prompt, so no intermediate state ever has Class-carrying blocks without walker support. Prompt 3 is documentation + CHANGELOG + final gates. AC10 is operator-executable (runs at spec-verification time, after merge) and is covered by the Verification operator rung, not by a prompt — it requires the merged plugin, the `--security` wiring (v0.49.0), and the task-5 agent. + +## Do-Nothing Option + +If this task does not ship, the guide's promise that the judgment and invariant tiers "ship in a follow-up task" stays unfulfilled, and security review mode keeps only 5 mechanical MUST detectors. Its findings on real apps remain limited to mechanical shapes (secrets, TLS, weak crypto, SQL interpolation); the authz/business-logic findings that differentiate security review from generic linters never fire, and the invariant-linked authz rules — the design's core differentiator — do not exist in the rule base or the index. The goal's "decent → comprehensive" progression is blocked at decent. The current approach is not acceptable: the comprehensive tier is the stated deliverable of this task, and the guide currently ships an explicit placeholder for it. + +## Verification Result + +**Verified:** 2026-08-23T21:18:46Z (HEAD 3c1f966) +**Binary:** n/a — structural spec, verified directly in worktree; AC10 gate references installed coding plugin v0.49.0 (see Evidence) +**Scenario:** n/a — no new scenario file (AC10 operator DoD deferred, see Evidence) +**Evidence:** +- AC1: `grep -Ec '^### RULE go-security/(…7 slugs…)'` = 7; jq 7 entries (5 MUST / 2 SHOULD, enforcement_type judgment, owner go-security-specialist, trigger non-empty); total `^### RULE ` = 14; Why/Bad/Good each = 14 +- AC2: 2 invariant RULE headings; `**Class**: security-invariant` = 2; jq 2 entries class=security-invariant, MUST, judgment, trigger ["@commits"]; `security-review-pipeline.md` cited 4× +- AC3: `build-index.py:64` — `"Class"` in field key tuple; jq `has("class")` length = 2; schema doc `**Class**:` hits (51,54,58,63) and `"class"` hits (51,119,136) +- AC4: `make check-index` exit 0; index length 180; `security/` 3-component count 0; go-security ID count 18; removed-go-security-ID diff count 0; rules/index.json clean in git +- AC5: citation fixture `validate-citations.sh` exit 0, `.findings | length` = 3, `.dropped_count` = 0 (non-vacuous) +- AC6: layout decision at guide:461 (`rules/security/{go,python,node}`); `rules/security/*.yml` count 5; `git status --short -- rules/security/` empty +- AC7: `git status --short` empty; `git diff origin/master...HEAD --stat` touches no `scripts/validate-citations.sh`, `commands/`, `agents/*`, `.maintainer.yaml`, `scenarios/` +- AC8: `make precommit` exit 0; CHANGELOG `## Unreleased` present + 1 `feat:` bullet +- AC9: negative greps all 0 — `follow-up task|ships in a follow-up|ship.*follow-up` = 0; `Judgment/invariant-tier RULE blocks in this guide` = 0; `until then` = 0 +- AC10 (DEFERRED — verified-with-deferred-runtime-evidence): installed coding plugin at spec-verification time is v0.49.0 — `--security` wiring live (local-review.md, 6 hits) and go-security-specialist/security-verifier agents live, but the rule base is NOT installed (installed guide = 5 RULE blocks, installed index has 0 `go-security/ssrf-user-controlled-url` entries). The SSRF DoD fixture therefore cannot produce a finding that the installed validator keeps; scenario 007/008 walks not run (no /tmp/scen007-*/scen008-* evidence). AC10 to be satisfied post-release v0.50.0. +**Verdict:** PASS (AC1-AC9 verified against fresh in-repo evidence; AC10 verified-with-deferred-runtime-evidence, pending v0.50.0 release)