diff --git a/README.md b/README.md index 6270d16..6c805d7 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,22 @@ You built it by vibe. You shipped it like someone who knows what they are doing. 3. **Do not let AI clean up code you have not read.** A rename or a "simpler" condition can quietly change behavior. 4. **"I don't know" is a fine answer.** A handoff that lists unknowns is safer than confident documentation the AI made up. +## When a control lives outside the repo + +Some real controls are invisible to a file scan: CI that runs in an org-level pipeline, error tracking switched on in a platform dashboard, secrets kept in a vault. Without a way to say so, the fix list becomes a trap. The audit can never reach zero, and the tempting move is to add a file whose only job is to satisfy the scan. + +So the audit takes waivers. Add `.prod-audit-waivers.json` to your repo: + +```json +[{"id": "ci-1", + "reason": "CI runs in the org-level pipeline, not per repo.", + "evidence": "gitlab.example.com/org/platform/pipelines", + "approved_by": "Your Name", + "date": "2026-08-22"}] +``` + +Every field is required. A waived check stops counting toward the score and the exit code, but it appears in every report with its reason, its evidence, and the name of the person who accepted it. Waivers expire after 180 days, and then the finding comes back. Claude will never write this file for you. It shows you the entry and you decide. + ## The two readiness tools, and when to use which Both ask "is it ready?" They answer different halves. diff --git a/skills/prod-readiness-coach/SKILL.md b/skills/prod-readiness-coach/SKILL.md index 315efcf..51de9ec 100644 --- a/skills/prod-readiness-coach/SKILL.md +++ b/skills/prod-readiness-coach/SKILL.md @@ -123,12 +123,31 @@ or library. Then re-run with `--profile`. Top-level fields that matter: `evidence` (file paths/line snippets), `best_practice_ref` (a citation URL), and `confidence` (`verified` or `weak`). A `weak` pass means the tool only found matching text — write it up as "looks like - this may exist; confirm by hand," never as a confirmed win. + this may exist; confirm by hand," never as a confirmed win. Each + category also has `has_weak_evidence`: when true, that category is not + a clean win no matter how high it scored. +- `contradictions` — pairs where one check passed while another failed in + a way that undercuts it (a clean secret scan with no `.gitignore` guard; + tests that no pipeline runs). **Write these up before the phases**, in + their own short section, because a phase plan built on a pass that is + not real wastes the whole phase. +- `waivers` → `applied` and `problems`. A waiver is a human's written, + dated acceptance of a real finding whose control lives outside the repo + (an org-level pipeline, a platform dashboard, a vault). Waived checks + have `status: "waived"` and `waived_from`, and they stop counting toward + the score and the exit code. `problems` lists waivers that were rejected + (missing a field, bad date, expired) — those findings still count. Treat this JSON as the authoritative record of **what the scanner observed**, not as final truth. The scanner is pattern-based static analysis. Rules: - Never invent a finding that isn't in the JSON. +- **Never write or edit `.prod-audit-waivers.json` yourself.** A waiver + carries a person's name and their acceptance of a real risk. If a + critical finding cannot be fixed inside the repo because the control + lives elsewhere, put it on the manual-steps list, show the user the + exact waiver entry to add, and let them add it. An agent that waives its + own blockers has learned to game the scan, which is worse than the gap. - Verify every `critical` and `high` finding that surprises you by reading the actual files before writing it up. Monitoring configured outside the repo, an org-level CI pipeline, or an unusual layout can all produce a diff --git a/skills/prod-readiness-coach/references/report-templates.md b/skills/prod-readiness-coach/references/report-templates.md index ed3d97f..0c052b7 100644 --- a/skills/prod-readiness-coach/references/report-templates.md +++ b/skills/prod-readiness-coach/references/report-templates.md @@ -26,13 +26,31 @@ turn a small bug into a multi-hour mystery."}} ## What's already solid ✅ -{{For every category with `applicable: true` that scored 90+ with no -failing checks, one bullet each, named warmly. A category whose checks +{{For every category with `applicable: true` and `has_weak_evidence: false` +that scored 90+ with no failing checks, one bullet each, named warmly. +A category with `has_weak_evidence: true` never belongs here no matter +what it scored; it goes in the "Looks fine, but only from a text match" +list below instead. A category whose checks are all `n/a` has `score: null`; it is neither a win nor a gap, so leave it out. E.g. "**Your secrets are safe.** You're not accidentally leaking passwords or API keys into your code — a mistake even experienced teams make. Nice work."}} +## Looks fine, but only from a text match + +{{One bullet for each category with `has_weak_evidence: true`, naming the +check and what would prove it. E.g. "**Rate limiting.** The tool found the +words `rate_limit` in `server.js`, which is a hint, not proof. Open that +file: if it is a config value nothing reads, this control does not exist." +Skip this section entirely if no category has weak evidence.}} + +## Conflicting signals + +{{One bullet per entry in the JSON `contradictions` array, in plain +English, saying which pass is undercut and by what. Put this BEFORE the +ranked gaps: a reader needs to know which "wins" are not real before they +read the list of losses. Skip the section if the array is empty.}} + ## What needs attention, ranked by urgency {{For each failing/warning check, ordered by severity (critical → high → @@ -137,7 +155,12 @@ outcome, not a task. E.g. "- [ ] `python3 prod_audit.py --repo . --json **Gate — do not proceed to Phase {{n+1}} until:** 1. Every checkbox above is true. 2. You've re-run: `python3 prod_audit.py --repo . --fail-on critical` (or - `--fail-on high` for a Phase 2 gate) and it exits 0. + `--fail-on high` for a Phase 2 gate) and it exits 0 — **or** every + finding still failing at that severity is one whose control genuinely + lives outside this repo, and the human has added a waiver for it in + `.prod-audit-waivers.json`. If you cannot reach 0 and cannot honestly + waive, stop and say so. Do not add a file whose only purpose is to + satisfy the scan. 3. Anything on the separate "Manual steps for a human" list tied to this phase (see below) has been done or explicitly deferred with the human's sign-off — an AI agent should not silently skip these and diff --git a/skills/prod-readiness-coach/scripts/prod_audit.py b/skills/prod-readiness-coach/scripts/prod_audit.py index 2e21e32..0a5feb1 100644 --- a/skills/prod-readiness-coach/scripts/prod_audit.py +++ b/skills/prod-readiness-coach/scripts/prod_audit.py @@ -44,7 +44,7 @@ import subprocess import sys from dataclasses import dataclass, field, asdict -from datetime import datetime, timezone +from datetime import datetime, timedelta, timezone from pathlib import Path from typing import Callable, Optional @@ -81,11 +81,86 @@ class CheckResult: # "weak": a text match only. A weak pass means "possible, verify by hand"; # it never counts as proof that the control exists. confidence: str = "verified" + # Set when a repo waiver moved this check out of "fail". Keeps the original status. + waived_from: str = "" # Checks whose pass is based on a text search, not a structural check. WEAK_PASS_IDS = {"log-4", "res-3", "sec-4"} +# A control can live outside the repository (an org-level pipeline, a platform +# dashboard, a secrets vault). Without a way to say so, --fail-on can never +# reach 0 and a phase gate becomes a trap that rewards faking the fix. A waiver +# is that escape hatch, and it is deliberately expensive: every field is +# required, it is dated, it expires, and it is printed in every report. +WAIVER_FILE = ".prod-audit-waivers.json" +WAIVER_FIELDS = ("id", "reason", "evidence", "approved_by", "date") +WAIVER_MAX_AGE_DAYS = 180 + +# Pairs where one check's pass is undermined by another check's failure. +# (passing ids, failing ids, what the conflict means) +CONTRADICTIONS = [ + ({"sec-4"}, {"sec-2"}, + "The secret scan found nothing, but `.gitignore` does not exclude env files. A real `.env` " + "could be committed and never match a pattern. Treat the clean scan as unproven, not as a win."), + ({"log-4"}, {"log-1"}, + "Request or correlation IDs are mentioned, but no logging library is configured. IDs with " + "nothing to write them into are a mention, not a control."), + ({"test-1"}, {"ci-2"}, + "Test files exist but no pipeline step runs them. Tests nobody runs are documentation."), + ({"res-2"}, {"ms-1"}, + "A rollback procedure is documented, but this repo deploys to more than one surface and no " + "coordinated procedure covering all of them was found. The documented rollback may undo only part."), +] + + +def load_waivers(repo: "Repo") -> tuple[dict, list[str]]: + """Return ({check_id: waiver}, [problems]). A bad waiver is reported, never silently ignored.""" + raw = repo.read(WAIVER_FILE) + if not raw.strip(): + return {}, [] + try: + entries = json.loads(raw) + except Exception as e: + return {}, [f"{WAIVER_FILE} is not valid JSON ({e}). No waivers applied."] + if not isinstance(entries, list): + return {}, [f"{WAIVER_FILE} must be a list of waiver objects. No waivers applied."] + good, problems, now = {}, [], datetime.now(timezone.utc) + for i, w in enumerate(entries): + if not isinstance(w, dict): + problems.append(f"{WAIVER_FILE}[{i}] is not an object; skipped.") + continue + missing = [f for f in WAIVER_FIELDS if not str(w.get(f, "")).strip()] + if missing: + problems.append(f"{WAIVER_FILE}[{i}] ({w.get('id', 'no id')}) is missing " + f"{', '.join(missing)}; skipped. Every field is required.") + continue + try: + when = datetime.strptime(str(w["date"]).strip(), "%Y-%m-%d").replace(tzinfo=timezone.utc) + except ValueError: + problems.append(f"{WAIVER_FILE}[{i}] ({w['id']}) has date '{w['date']}'; " + "use YYYY-MM-DD. Skipped.") + continue + age = (now - when).days + if age > WAIVER_MAX_AGE_DAYS: + problems.append(f"Waiver for {w['id']} expired ({age} days old, limit " + f"{WAIVER_MAX_AGE_DAYS}). The finding is back. Re-confirm it or fix it.") + continue + good[str(w["id"]).strip()] = w + return good, problems + + +def find_contradictions(categories: list["Category"]) -> list[dict]: + """Where a pass and a fail disagree about the same control, say so.""" + by_id = {c.id: c for cat in categories for c in cat.checks} + out = [] + for passing, failing, note in CONTRADICTIONS: + if all(by_id.get(i) and by_id[i].status == "pass" for i in passing) \ + and all(by_id.get(i) and by_id[i].status == "fail" for i in failing): + out.append({"ids": sorted(passing | failing), "passing": sorted(passing), + "failing": sorted(failing), "note": note}) + return out + # What kind of thing the repo is. A check that does not apply to the profile # is reported as "n/a" and left out of the score. A framework or deploy # surface implies web-app (strict). A language alone implies nothing, so the @@ -122,6 +197,11 @@ def score(self) -> int: score -= SEVERITY_PENALTY.get(c.severity, 5) return max(0, score) + @property + def has_weak_evidence(self) -> bool: + """True when any pass here rests only on a text match, so this is not a clean win.""" + return any(c.status == "pass" and c.confidence == "weak" for c in self.checks) + @property def applicable(self) -> bool: return any(c.status != "n/a" for c in self.checks) @@ -311,6 +391,8 @@ class StackFingerprint: deploy_evidence: dict[str, list[str]] = field(default_factory=dict) runtimes: list[str] = field(default_factory=list) migration_tooling: list[str] = field(default_factory=list) + waivers_applied: list = field(default_factory=list) + waiver_problems: list = field(default_factory=list) profile: str = "web-app" profile_source: str = "default" # "given" | "guessed" | "default" multi_surface: bool = False @@ -1356,6 +1438,9 @@ def run_audit(repo_path: Path, profile: Optional[str] = None) -> tuple[list[Cate else: fingerprint.profile, fingerprint.profile_source = guess_profile(repo, fingerprint), "guessed" skips = CHECK_SKIPS_BY_PROFILE[fingerprint.profile] + waivers, waiver_problems = load_waivers(repo) + fingerprint.waivers_applied = [{k: w[k] for k in WAIVER_FIELDS} for w in waivers.values()] + fingerprint.waiver_problems = waiver_problems categories = {key: Category(key, title, desc, ref) for key, title, desc, ref in CATEGORY_META} categories["AI Agent Context"].checks.append(check_claude_md(repo)) @@ -1373,6 +1458,13 @@ def run_audit(repo_path: Path, profile: Optional[str] = None) -> tuple[list[Cate for c in cat.checks: if c.id in WEAK_PASS_IDS and c.status == "pass": c.confidence = "weak" + w = waivers.get(c.id) + if w and c.status == "fail": + c.waived_from = c.status + c.status, c.confidence = "waived", "verified" + c.detail = (f"Waived {w['date']} by {w['approved_by']}: {w['reason']} " + f"Evidence: {w['evidence']} Original finding: {c.detail}") + c.recommendation = "" if c.id in skips: c.status, c.severity = "n/a", "info" why = ("Project type not determined; insufficient evidence to apply this check. Re-run with --profile." @@ -1485,9 +1577,45 @@ def render_markdown(categories: list[Category], repo_name: str, fp: Optional[Sta crit = sum(1 for c in fails if c.severity == "critical") hi = sum(1 for c in fails if c.severity == "high") score_cell = f"{cat.score}/100" if cat.applicable else "N/A" + if cat.has_weak_evidence: + score_cell += " (text match only)" lines.append(f"| {cat.title} | {score_cell} | {crit} | {hi} | {len(fails)} |") lines.append("") + conflicts = find_contradictions(categories) + if conflicts: + lines.append("## Conflicting signals") + lines.append("") + lines.append("One check passed while another failed in a way that undercuts it. " + "Resolve these before trusting either result.") + lines.append("") + for c in conflicts: + lines.append(f"- **{' + '.join(c['ids'])}** — {c['note']}") + lines.append("") + + applied = getattr(fp, "waivers_applied", []) if fp is not None else [] + problems = getattr(fp, "waiver_problems", []) if fp is not None else [] + if applied: + lines.append("## Waived, with evidence") + lines.append("") + lines.append(f"These findings are real but were accepted by a human in `{WAIVER_FILE}`. " + f"They do not count toward the score or the exit code. Waivers expire after " + f"{WAIVER_MAX_AGE_DAYS} days.") + lines.append("") + lines.append("| Check | Reason | Evidence | Approved by | Date |") + lines.append("|---|---|---|---|---|") + for w in applied: + lines.append(f"| `{w['id']}` | {w['reason']} | {w['evidence']} | {w['approved_by']} | {w['date']} |") + lines.append("") + if problems: + lines.append("## Waivers that were rejected") + lines.append("") + lines.append("These entries did not apply, so their findings still count:") + lines.append("") + for _p in problems: + lines.append(f"- {_p}") + lines.append("") + if blocking: lines.append("## 🔴 Release Blockers (Critical)") lines.append("") @@ -1527,7 +1655,7 @@ def render_markdown(categories: list[Category], repo_name: str, fp: Optional[Sta lines.append("| Check | Status | Severity | Detail |") lines.append("|---|---|---|---|") for c in sorted(cat.checks, key=lambda x: SEVERITY_ORDER.get(x.severity, 9)): - status_icon = {"pass": "✅", "fail": "❌", "warn": "⚠️", "info": "ℹ️", "n/a": "➖"}.get(c.status, "") + status_icon = {"pass": "✅", "fail": "❌", "warn": "⚠️", "info": "ℹ️", "n/a": "➖", "waived": "🟦"}.get(c.status, "") detail = c.detail.replace("|", "\\|") status_text = f"{status_icon} {c.status}" + (" (text match — verify by hand)" if c.confidence == "weak" else "") lines.append(f"| {c.title} | {status_text} | {SEVERITY_LABEL.get(c.severity, c.severity)} | {detail} |") @@ -1572,12 +1700,16 @@ def render_json(categories: list[Category], repo_name: str, fp: Optional[StackFi "grade": grade_for(overall_score(categories), sum(c.blocking_count for c in categories)), "stack_fingerprint": asdict(fp) if fp is not None else None, "product_context": product_context or None, + "waivers": {"applied": getattr(fp, "waivers_applied", []) if fp else [], + "problems": getattr(fp, "waiver_problems", []) if fp else []}, + "contradictions": find_contradictions(categories), "categories": [ { "key": cat.key, "title": cat.title, "score": cat.score if cat.applicable else None, "applicable": cat.applicable, + "has_weak_evidence": cat.has_weak_evidence, "checks": [asdict(c) for c in cat.checks], } for cat in categories diff --git a/skills/prod-readiness-coach/tests/test_prod_audit.py b/skills/prod-readiness-coach/tests/test_prod_audit.py index 4201db7..ac684ce 100644 --- a/skills/prod-readiness-coach/tests/test_prod_audit.py +++ b/skills/prod-readiness-coach/tests/test_prod_audit.py @@ -25,6 +25,12 @@ def audit(files: dict[str, str], profile=None) -> dict: return prod_audit.render_json(cats, root.name, fp) +def audit_markdown(files: dict, profile=None) -> str: + root = make_repo(files) + cats, fp = prod_audit.run_audit(root, profile) + return prod_audit.render_markdown(cats, root.name, fp) + + class EmptyRepo(unittest.TestCase): def test_empty_repo_does_not_crash_and_has_empty_fingerprint(self): r = audit({}) @@ -235,6 +241,89 @@ def test_worker_dockerfile_in_member_does_not_block_vercel_inference(self): self.assertTrue(fp["multi_surface"]) +NEXT_APP = { + "package.json": json.dumps({"dependencies": {"next": "15.0.0"}, "scripts": {"test": "vitest"}}), + "package-lock.json": "{}", "tsconfig.json": "{}", "tests/a.test.ts": "test('x',()=>{})", +} + + +class Waivers(unittest.TestCase): + def waiver(self, **over): + w = {"id": "ci-1", "reason": "CI runs in the org-level pipeline, not per repo.", + "evidence": "gitlab.example.com/org/pipelines", "approved_by": "A Person", + "date": prod_audit.datetime.now(prod_audit.timezone.utc).strftime("%Y-%m-%d")} + w.update(over) + return json.dumps([w]) + + def test_valid_waiver_marks_check_waived_and_unblocks_the_gate(self): + r = audit({**NEXT_APP, ".prod-audit-waivers.json": self.waiver()}) + c = check(r, "ci-1") + self.assertEqual(c["status"], "waived") + self.assertEqual(c["waived_from"], "fail") + self.assertIn("A Person", c["detail"]) + self.assertNotIn("ci-1", [k["id"] for cat in r["categories"] for k in cat["checks"] + if k["status"] == "fail" and k["severity"] == "critical"]) + self.assertEqual(r["waivers"]["problems"], []) + self.assertEqual([w["id"] for w in r["waivers"]["applied"]], ["ci-1"]) + + def test_waiver_missing_a_field_is_rejected_and_reported(self): + r = audit({**NEXT_APP, ".prod-audit-waivers.json": self.waiver(evidence="")}) + self.assertEqual(check(r, "ci-1")["status"], "fail") + self.assertTrue(any("evidence" in p for p in r["waivers"]["problems"])) + + def test_expired_waiver_stops_applying(self): + old = (prod_audit.datetime.now(prod_audit.timezone.utc) - prod_audit.timedelta(days=200)).strftime("%Y-%m-%d") + r = audit({**NEXT_APP, ".prod-audit-waivers.json": self.waiver(date=old)}) + self.assertEqual(check(r, "ci-1")["status"], "fail") + self.assertTrue(any("expired" in p.lower() for p in r["waivers"]["problems"])) + + def test_waivers_never_hide_the_finding_from_the_report(self): + md = audit_markdown({**NEXT_APP, ".prod-audit-waivers.json": self.waiver()}) + self.assertIn("Waived, with evidence", md) + self.assertIn("A Person", md) + self.assertIn("org-level pipeline", md) + + def test_a_rejected_waiver_is_shouted_in_the_report(self): + md = audit_markdown({**NEXT_APP, ".prod-audit-waivers.json": self.waiver(approved_by="")}) + self.assertIn("Waivers that were rejected", md) + + +class WeakEvidenceIsNotAWin(unittest.TestCase): + def test_category_carrying_only_a_text_match_is_flagged(self): + r = audit({"package.json": json.dumps({"dependencies": {"next": "15.0.0", "express": "4.0.0"}}), + "package-lock.json": "{}", + "server.js": "const limiter = { rate_limit: 100 };\n", + "docs/runbook.md": "# runbook\nrollback: redeploy the previous commit.\n"}) + res = next(c for c in r["categories"] if c["key"] == "Resilience & Failover") + self.assertEqual(check(r, "res-3")["confidence"], "weak") + self.assertTrue(res["has_weak_evidence"], + "a category whose pass rests on a text match must not read as a clean win") + + +class Contradictions(unittest.TestCase): + def test_clean_secret_scan_with_no_gitignore_guard_is_reported_as_conflicting(self): + r = audit({"package.json": json.dumps({"dependencies": {"next": "15.0.0"}}), "package-lock.json": "{}"}) + pairs = [set(c["ids"]) for c in r["contradictions"]] + self.assertIn({"sec-4", "sec-2"}, pairs) + + def test_tests_that_nothing_runs_is_reported_as_conflicting(self): + r = audit({"package.json": json.dumps({"dependencies": {"next": "15.0.0"}, "scripts": {"test": "vitest"}}), + "package-lock.json": "{}", "tests/a.test.ts": "test('x',()=>{})", + ".github/workflows/ci.yml": "on: push\njobs:\n b:\n steps:\n - run: npm run build\n"}) + pairs = [set(c["ids"]) for c in r["contradictions"]] + self.assertIn({"test-1", "ci-2"}, pairs) + + def test_contradictions_are_printed_in_the_report(self): + md = audit_markdown({"package.json": json.dumps({"dependencies": {"next": "15.0.0"}}), + "package-lock.json": "{}"}) + self.assertIn("Conflicting signals", md) + + def test_no_contradictions_on_a_healthy_repo(self): + r = audit({"package.json": json.dumps({"dependencies": {"next": "15.0.0"}}), "package-lock.json": "{}", + ".gitignore": ".env*\n"}, profile="library") + self.assertNotIn({"sec-4", "sec-2"}, [set(c["ids"]) for c in r["contradictions"]]) + + class CiDetection(unittest.TestCase): def test_unittest_step_counts_as_running_tests(self): wf = "on: push\njobs:\n t:\n runs-on: ubuntu-latest\n steps:\n - run: python -m unittest discover tests\n"