ADR-029: Security standards enforcement — CI checks vs review discipline

Status: Accepted Date: 2026-04-21

Context

security-standards.md lists ~15 rules every service must follow. Some can be enforced mechanically (dependency pins, SHA-pinned base images, presence of struct-tag validators in a route handler). Others require judgment (whether a {@html} sanitiser is sufficient for the data it receives, whether a new anon endpoint was the right tier). A third category is subtle (whether an audit event is emitted at the right time).

The commitment "we will always review everything carefully" is not a sustainable enforcement strategy. Review load is high, reviewer attention is uneven, and one missed check can produce an exploit. The commitment "all rules are mechanical" is also wrong — some rules are inherently judgment calls. The right posture is a conscious split: mechanical where we can, review where we must, with a clear list of which is which.

This ADR captures that split, aligned to the rules in security-standards.md.

Decision

Each standard in security-standards.md is mapped to one of four enforcement modes:

1. CI-gated — fails the build. Example: gitleaks on a committed secret.
2. Pre-commit hook — warns at commit time; can be bypassed with --no-verify but the bypass is visible in the commit message (reviewer catches).
3. Review-gated — a reviewer blocks the PR. No automated check.
4. Runtime-detective — the violation is detectable at runtime via a tripwire (security-tripwires.md) rather than at merge time.

The mapping

The shared libraries target

A significant portion of the review-gated items become CI-gated once shared libraries ship:

• gyrum-go/pkg/auth — single library, three backends (session-cookie, OAuth, API-key). Wraps RequireOwnership, audit event emission, session HMAC verification. Once a service adopts pkg/auth, the review check "does this handler check ownership?" becomes "does this handler use pkg/auth.ResourceHandler[T]?" — which is mechanical.
• gyrum-go/pkg/middleware.CSRF — moves CSRF from "roll it per service" to "one middleware, review-checked once". Makes "does this cookie-authed state-changing endpoint have CSRF?" mechanical.
• gyrum-go/pkg/saferequest — standard outbound HTTP client with SSRF protections. Review becomes "is this a saferequest.Client?" instead of "does this code handle all the SSRF cases?".

Landing these libraries is the highest-leverage security work after this tranche. Tracked as Tier-2 items in ADR-026.

What isn't enforced anywhere

A minority of standards cannot be enforced at all by either tooling or review:

• "No PII on log lines without explicit approval." A reviewer cannot see PII they don't know is PII. Mitigation: the redactor catches the well-known patterns; the never-log list in standards is the review-level doc; retroactive detection is via tripwire 13.
• "No algorithmic-complexity regex on user input." Catching every (a+)+b pattern is a research problem. RE2 rejects them at compile time, and the stdlib regexp is RE2-backed — this is mechanically caught for stdlib regexes, but not for third-party regex libraries.
• "Audit event emitted at the right moment." A reviewer can check presence ("is there an audit call in this handler?") but not semantic correctness ("is this the right event name and fields?"). Mitigation: the audit sink's review (security-standards §auditability) reads the emitted events; drift is caught at audit-review time.

These are accepted gaps. The tripwires and audits partially mitigate; full elimination is not achievable with current tooling.

Consequences

• Mechanical checks are load-bearing. Developers learn what the CI flags. A flagged PR is a learning moment; an unflagged PR with a standards violation is a missed learning moment.
• Review capacity is focused on the hard stuff. Reviewers spend time on authorization logic, output encoding, audit-event semantics — the things only humans can meaningfully judge. They don't waste cycles checking for unpinned base images (CI catches) or committed secrets (pre-commit catches).
• The shared libraries become the biggest security investment. Every review-gated check that becomes CI-gated after pkg/auth / middleware.CSRF / pkg/saferequest ships is a permanent reduction in review load. The argument for landing these libraries is: the ongoing review-cost saving pays back the one-time build cost quickly.
• The gap list is explicit. The standards we cannot enforce are named. Operators and reviewers know what they are responsible for catching.
• CI-gated checks can be bypassed, but visibly. gitleaks in pre-commit can be --no-verify'd. govulncheck in CI can be .ignore'd. Every bypass is recorded (commit message, ignore file); the monthly review inspects bypasses. The anti-pattern "silent downgrade" — turning an error into a warning to unblock a deploy — is the specific thing the visibility prevents.
• The ADR is the shared understanding. When a reviewer asks "why isn't this a CI check?", the answer is "because this rule is judgment-gated, here is the ADR". When a developer asks "why did my PR fail on this?", the answer is "because this rule is mechanical, here is the ADR".

Alternatives considered

• Everything CI-gated. All rules are mechanical checks. Rejected: some rules cannot be expressed mechanically (sanitiser adequacy, audit semantics). Pretending they can creates false confidence in the checks, and the corresponding review attention evaporates.
• Everything review-gated. Trust the reviewer for everything. Rejected at current team size: one reviewer is a bottleneck; two reviewers with different attention still miss the third thing. Offloading the mechanical part to CI is non-negotiable.
• Tool adoption (Snyk, Semgrep, Codacy) for bulk coverage. Rejected for now: external static-analysis tools cover broad cases with high false-positive rates; our standards are more specific. A Semgrep rule for RequireOwnership is bespoke anyway, and we'd be maintaining the rule definitions there instead of here. Revisit if the tool's catch rate exceeds what we can build.
• Auto-generated CI from standards doc. Parse security-standards.md and emit check scripts. Rejected: over-engineering for the current scale. The standards doc updates slowly; the CI scripts drift even more slowly; the connection between the two is maintained by hand in this ADR. A generator is appropriate at 50+ standards, not 15.

Supersedes: none Superseded by: