diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 3eba879..593ed5a 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-marketplace.json", "name": "clean-code-toolkit", - "version": "3.3.0", + "version": "3.4.0", "description": "Clean-code and product-handoff tools for AI-assisted builders.", "owner": { "name": "Tarik Moody" @@ -10,7 +10,7 @@ { "name": "clean-code-toolkit", "description": "Review code, assess product readiness, refactor safely, and prepare a developer handoff.", - "version": "3.3.0", + "version": "3.4.0", "author": { "name": "Tarik Moody" }, diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 1ae2d6c..02062a8 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "clean-code-toolkit", - "version": "3.3.0", + "version": "3.4.0", "description": "Practical clean-code, product-readiness, and developer-handoff workflows for AI-assisted projects.", "author": { "name": "Tarik Moody" diff --git a/CHANGELOG.md b/CHANGELOG.md index f7c4018..9de6863 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,33 @@ # Changelog +## 3.4.0 + +An audit of the auditor. A deliberately hollow repo, every control a costume, scored 78 out of 100 with nine passing checks, five of them false. Fixing that exposed more. + +**Confidence now follows evidence, structurally.** +- A pass with an empty evidence list can no longer claim to be `verified`. The downgrade is automatic, so the hand-maintained `WEAK_PASS_IDS` set no longer has to be remembered when a check is added. +- Checks that always had real evidence now carry it: `log-1` and `log-2` cite the matched dependency, `dep-1` the lockfile, `ci-6` the script, `res-2` the matching line. `sec-3`, `sec-4` and `ms-1` cite the scope they searched, which is the honest evidence for a proof of absence. + +**Existence is not function.** Six checks passed on controls that existed in name only. +- `ci-2` (critical) was satisfied by a CI step of `echo "test skipped"`. It now requires a real test-runner invocation, with string literals stripped first. +- `ci-6` was satisfied by a `test` script of `echo "no tests yet" && exit 0`. +- `res-1` was satisfied by an empty `runbook.md`. It now requires 50 words of prose. +- `res-2` was satisfied by an unchecked `- [ ] figure out rollback` TODO box. +- `ms-2` was a bag-of-words test marked verified; it is now labelled a text match. +- `log-1`/`log-2` matched substrings of a serialized dependency blob, so `@types/pino` scored a verified pass. They now match package names, excluding type stubs. + +**Security: the waiver file is untrusted input.** `.prod-audit-waivers.json` lives in the audited repo, and its text was written into the Markdown report unescaped. A crafted `reason` field forged a second `## Repository Controls Score: 100/100` heading in the report. Waiver text is now flattened to a single length-capped line with pipes escaped and leading markup stripped, at the point it is loaded rather than at each render site. + +**The generated documents are now linted.** `scripts/check_report.py` checks a finished audit against the JSON it was written from: a quoted file path must appear in the scan, a `weak` pass must not be written up as a win, every `critical` must be addressed in the fix brief, a waived finding must stay visible, no placeholders, no em dashes. The skill runs it before sharing. Run against real generated documents that had already been reviewed by hand, it found six problems. + +**Refactor.** `prod_audit.py` was 1836 lines, more than twice this project's own 800-line ceiling, with three hand-maintained tables far from the checks they described. It is now an `audit/` package of eight modules, largest 503 lines, plus a thin entry point that re-exports the public names so every documented command and import keeps working. 77 unused imports removed. + +**House style.** Em dashes are gone from every generated string, so the report linter no longer rejects the documents this toolkit produces. + +Tests: 59, up from 37. + +Known and not yet fixed: `claude plugin eval` is in early access and could not be run, so the five prompt-only skills still have no automated coverage. Each was tested once by hand against a real repository; the findings are tracked for 3.5.0. + ## 3.3.0 Two verified defects in how the coach treats evidence, and the trap they created in the fix brief. diff --git a/skills/prod-readiness-coach/README.md b/skills/prod-readiness-coach/README.md index f481b7e..e30c324 100644 --- a/skills/prod-readiness-coach/README.md +++ b/skills/prod-readiness-coach/README.md @@ -9,6 +9,8 @@ python3 scripts/prod_audit.py --repo /path/to/repo --output report.md --json rep python3 scripts/prod_audit.py --help # --profile, --context, --fail-on ``` +`scripts/check_report.py` lints a finished audit against the JSON it was written from: invented file paths, a text match sold as a win, a critical finding missing from the fix brief, a waived finding quietly dropped. The skill runs it before sharing. + Tests: `python3 -m unittest discover skills/prod-readiness-coach/tests` from the toolkit root. Everything else (what it checks, profiles, confidence labels, limits, when to run it) lives in the [toolkit README](../../README.md) and [how it works](../../docs/how-it-works.md), so there is one place to keep current. Decisions that shaped this skill are in [`docs/decisions/`](docs/decisions/). diff --git a/skills/prod-readiness-coach/SKILL.md b/skills/prod-readiness-coach/SKILL.md index 51de9ec..d54e7c0 100644 --- a/skills/prod-readiness-coach/SKILL.md +++ b/skills/prod-readiness-coach/SKILL.md @@ -259,6 +259,25 @@ severity before starting the next phase"). Substance rules: ### 8. Review both documents before sharing +**Run the linter first.** It checks the rules below mechanically, so you +only have to think about the ones it cannot: + +```bash +python3 /scripts/check_report.py \ + AUDIT_PLAIN_ENGLISH.md FIX_BRIEF_FOR_CLAUDE_CODE.md --json +``` + +It fails if a quoted file path never appeared in the scan, if a `weak` +pass is written up as a win, if a `critical` finding is missing from the +brief, if a waived finding was quietly dropped, or if a placeholder or an +em dash survived. Fix everything it reports before sharing. If you +verified a path by hand rather than from the scan, say so in the sentence +that uses it, which is the rule in step 7 and what the linter is asking +you to make explicit. + +Then check by eye what it cannot: + + - Scan for any leftover jargon that isn't immediately explained — if you used a technical term, either the glossary already covers it or you need to add a one-clause explanation inline. diff --git a/skills/prod-readiness-coach/references/report-templates.md b/skills/prod-readiness-coach/references/report-templates.md index 0c052b7..82840b2 100644 --- a/skills/prod-readiness-coach/references/report-templates.md +++ b/skills/prod-readiness-coach/references/report-templates.md @@ -2,7 +2,7 @@ Two documents get generated per audit run. Follow these structures. Fill every `{{...}}` placeholder using the JSON output from `prod_audit.py` and -the plain-English glossary — never paste raw technical `detail` strings from +the plain-English glossary, never paste raw technical `detail` strings from the JSON without translating them first. --- @@ -12,7 +12,7 @@ the JSON without translating them first. ```markdown # How Production-Ready Is {{repo_name}}? -*A plain-English walkthrough — no jargon left untranslated.* +*A plain-English walkthrough, no jargon left untranslated.* ## The short version @@ -33,7 +33,7 @@ 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 +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 @@ -60,7 +60,7 @@ medium → low), write ONE section using this exact structure:}} **What we checked:** {{one sentence, plain English, what this check looks for}} -**What we found:** {{plain English restatement of the technical finding — +**What we found:** {{plain English restatement of the technical finding, name specific files/evidence when available, so it feels concrete not abstract}} @@ -70,15 +70,15 @@ hint of this, not proof." Then say what you did (or did not do) to confirm it. The warm voice stays; the uncertainty is stated first, not tucked at the end.}} -**Why it matters:** {{a real, concrete consequence — what actually happens +**Why it matters:** {{a real, concrete consequence, what actually happens to you or your users if this stays unfixed. Avoid abstractions like "poses a risk"; say what the risk *looks like* in practice.}} **The concept, in one paragraph:** {{a short teaching moment explaining the -underlying idea using the glossary — written so someone who's never heard +underlying idea using the glossary, written so someone who's never heard the term walks away actually understanding it, not just able to repeat it}} -**How urgent is this really?** {{one honest sentence calibrating — is this +**How urgent is this really?** {{one honest sentence calibrating, is this "fix before you have real users" or "nice to have before you scale past a few hundred users"? Don't inflate urgency for effect.}} @@ -86,7 +86,7 @@ few hundred users"? Don't inflate urgency for effect.}} ## Your next-lesson roadmap -{{A short ordered list — 3-6 items — framing the fixes as a learning +{{A short ordered list, 3-6 items, framing the fixes as a learning sequence, e.g.: 1. Learn what a CI pipeline does by adding your first one (30-60 min) 2. Learn why "silent failures" are dangerous by adding error tracking @@ -95,7 +95,7 @@ Each item should feel achievable, not overwhelming.}} --- -*This audit was generated by a static-analysis tool — it reads your code +*This audit was generated by a static-analysis tool, it reads your code and config files, it doesn't run your app. Some findings may need a quick human double-check. Full technical findings with file-level evidence and best-practice citations are in the companion report.* @@ -115,9 +115,9 @@ code appear. # Fix Brief: {{repo_name}} Paste this into Claude Code inside this repo. **This brief is split into -phases by severity — do not start Phase 2 until every Phase 1 acceptance +phases by severity, do not start Phase 2 until every Phase 1 acceptance criterion is checked off and you've re-run the audit to confirm it. Same -gate between every later phase.** This isn't just pacing — a phase can +gate between every later phase.** This isn't just pacing, a phase can change what a later fix should even look like (e.g. adding CI in Phase 1 changes how you verify a Phase 2 fix), so skipping ahead risks doing Phase 2 work against a moving target. @@ -125,10 +125,10 @@ Phase 2 work against a moving target. ## Before you start {{2-3 sentences setting expectations: what stack this repo uses (from -stack_fingerprint in the JSON — name the actual frameworks/deploy +stack_fingerprint in the JSON, name the actual frameworks/deploy surfaces detected, e.g. "Next.js on Vercel with a Convex backend"), and a reminder that it's fine to ask Claude Code to explain any step before -running it — that's how you learn what it's doing instead of just +running it, that's how you learn what it's doing instead of just trusting it blindly.}} {{If stack_fingerprint.adapters_matched is non-empty, add one sentence @@ -140,22 +140,22 @@ for the fact that rolling back one doesn't roll back the other."}} {{Group every failing/warning check into phases by severity: Phase 1 = all `critical` findings, Phase 2 = all `high` findings, Phase 3 = all -`medium`/`low` findings. Skip a phase entirely if it has zero findings — +`medium`/`low` findings. Skip a phase entirely if it has zero findings, don't manufacture filler. Open each phase with this block, then the numbered sections for just that phase's findings:}} ## Phase {{n}}: {{phase title, e.g. "Release Blockers"}} {{severity badge}} -**Acceptance criteria for this phase — all must be true before moving on:** +**Acceptance criteria for this phase, all must be true before moving on:** {{One checkbox line per finding in this phase, phrased as an observable outcome, not a task. E.g. "- [ ] `python3 prod_audit.py --repo . --json /tmp/audit.json` shows zero remaining `critical` findings" rather than "- [ ] Added CI pipeline."}} -**Gate — do not proceed to Phase {{n+1}} until:** +**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 — **or** every + `--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 @@ -163,19 +163,19 @@ outcome, not a task. E.g. "- [ ] `python3 prod_audit.py --repo . --json 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 + human's sign-off, an AI agent should not silently skip these and report the phase complete. {{One numbered section per finding in this phase:}} ## {{n}}. {{plain-English title}} {{severity badge, e.g. "🔴 Fix this first"}} -**In plain English:** {{1-2 sentences — what's missing and why, using the +**In plain English:** {{1-2 sentences, what's missing and why, using the glossary voice, no unexplained jargon}} **What to ask Claude Code to do:** -{{A precise, technical, copy-pasteable instruction block — specific file +{{A precise, technical, copy-pasteable instruction block, specific file paths from the audit evidence, specific commands, specific acceptance criteria. This part CAN and SHOULD use correct technical terminology, because this is the part an AI agent executes. Model it on this shape:}} @@ -183,12 +183,12 @@ because this is the part an AI agent executes. Model it on this shape:}} > Add {{specific thing}} to {{specific file/location}}. It should > {{specific behavior}}. Verify by {{specific verification step}}. -**What you'll learn from this fix:** {{1-2 sentences — the transferable +**What you'll learn from this fix:** {{1-2 sentences, the transferable skill/concept this teaches, so the user internalizes *why*, not just -*what*. E.g. "This is your first taste of CI/CD — once you've set this up +*what*. E.g. "This is your first taste of CI/CD, once you've set this up once, you'll reuse this exact pattern on every future project."}} -**How to know it worked:** {{a concrete, observable check — a green +**How to know it worked:** {{a concrete, observable check, a green checkmark, a log line, a webpage that now loads, etc.}} --- @@ -197,7 +197,7 @@ checkmark, a log line, a webpage that now loads, etc.}} {{Closing note: remind them to re-run the full audit (not just --fail-on critical) to see the score improve, and to feel good about the -delta — cite the before/after score if known.}} +delta, cite the before/after score if known.}} ## Manual steps for a human (not for the AI agent to execute) @@ -205,23 +205,23 @@ delta — cite the before/after score if known.}} agent might run through unattended. Populate this from anything flagged during drafting as: interactive-only (a setup wizard that requires a human at a keyboard), plan/tier-dependent (e.g. GitHub branch protection -on a private repo needs a paid plan — confirm the plan before promising +on a private repo needs a paid plan, confirm the plan before promising this fix), or a judgment call with real consequences (e.g. "should we actually run this destructive migration's rollback, or restore from -backup instead?"). If this list is empty, state that plainly — don't +backup instead?"). If this list is empty, state that plainly, don't invent an entry to fill the section.}} ``` ## Formatting rules for both documents -- Use `##`/`###` headers exactly as templated — don't invent deeper nesting. +- Use `##`/`###` headers exactly as templated, don't invent deeper nesting. - Never leave a `{{placeholder}}` unfilled in the final output. -- Keep every "Why it matters" and "In plain English" block under ~60 words — +- Keep every "Why it matters" and "In plain English" block under ~60 words, concise beats thorough here. - If a category has zero failing/warning checks, skip the "needs attention" - section entirely for it — don't manufacture filler content. + section entirely for it, don't manufacture filler content. - Preserve file-path evidence from the JSON output verbatim (e.g. - `app/api/ats-check/route.ts`) inside the technical instruction blocks — + `app/api/ats-check/route.ts`) inside the technical instruction blocks, precision matters there even though the surrounding prose is plain English. - Any fact that came from the user rather than the tool gets attributed inline: "you told me you're on Vercel Pro, so...". Any hand-verification diff --git a/skills/prod-readiness-coach/scripts/audit/__init__.py b/skills/prod-readiness-coach/scripts/audit/__init__.py new file mode 100644 index 0000000..814eced --- /dev/null +++ b/skills/prod-readiness-coach/scripts/audit/__init__.py @@ -0,0 +1 @@ +"""Production-readiness audit engine.""" diff --git a/skills/prod-readiness-coach/scripts/audit/checks_build.py b/skills/prod-readiness-coach/scripts/audit/checks_build.py new file mode 100644 index 0000000..3719605 --- /dev/null +++ b/skills/prod-readiness-coach/scripts/audit/checks_build.py @@ -0,0 +1,228 @@ +"""Checks about how code gets built and shipped: agent context, CI, test wiring.""" +import re +from dataclasses import asdict +from datetime import datetime +from pathlib import Path +from typing import Optional + +from .model import CheckResult, runs_a_test_suite +from .repo import Repo + +# -------------------------------------------------------------------------- +# Individual checks, each returns a CheckResult +# -------------------------------------------------------------------------- + +def check_claude_md(repo: Repo) -> CheckResult: + path = repo.exists("CLAUDE.md", "AGENTS.md") + ref = "https://docs.claude.com/en/docs/claude-code/memory" + if not path: + return CheckResult( + "agent-1", "AI Agent Context", "CLAUDE.md / AGENTS.md present", + "fail", "high", + "No CLAUDE.md or AGENTS.md found at the repository root.", + "Add a CLAUDE.md (or AGENTS.md) documenting build/test/lint commands, architecture " + "conventions, and guardrails so AI coding agents (and new engineers) " + "operate consistently instead of re-deriving project context every session.", + best_practice_ref=ref, + ) + content = repo.read(path).strip() + # A CLAUDE.md that only delegates (e.g. "@AGENTS.md") is valid but should + # be flagged as info so the auditor knows to check the delegate target too. + delegate_match = re.match(r"^@([\w./-]+\.md)\s*$", content) + if delegate_match: + target = delegate_match.group(1) + target_exists = repo.exists(target) + if target_exists: + return CheckResult( + "agent-1", "AI Agent Context", "CLAUDE.md / AGENTS.md present", + "pass", "info", + f"{path} delegates to {target}, which exists.", + evidence=[path, target], + best_practice_ref=ref, + ) + return CheckResult( + "agent-1", "AI Agent Context", "CLAUDE.md / AGENTS.md present", + "fail", "medium", + f"{path} delegates to {target}, but that file was not found.", + f"Create {target} or point CLAUDE.md at an existing file.", + evidence=[path], + best_practice_ref=ref, + ) + word_count = len(content.split()) + if word_count < 30: + return CheckResult( + "agent-1", "AI Agent Context", "CLAUDE.md / AGENTS.md present", + "warn", "low", + f"{path} exists but is very short ({word_count} words), may be a stub.", + "Expand CLAUDE.md with commands (build/test/lint/deploy), directory " + "map, and any non-obvious conventions or footguns.", + evidence=[path], + best_practice_ref=ref, + ) + return CheckResult( + "agent-1", "AI Agent Context", "CLAUDE.md / AGENTS.md present", + "pass", "info", + f"CLAUDE.md exists with {word_count} words of guidance.", + evidence=[path], + best_practice_ref=ref, + ) + + +def check_ci_pipeline(repo: Repo) -> list[CheckResult]: + ref = "https://docs.github.com/en/actions/learn-github-actions" + results = [] + + workflow_files = repo.find_any([ + ".github/workflows/*.yml", ".github/workflows/*.yaml", + ".gitlab-ci.yml", ".circleci/config.yml", "azure-pipelines.yml", + "Jenkinsfile", ".buildkite/pipeline.yml", + ]) + if not workflow_files: + results.append(CheckResult( + "ci-1", "CI/CD Pipeline", "CI configuration exists", + "fail", "critical", + "No CI/CD pipeline configuration found (checked GitHub Actions, " + "GitLab CI, CircleCI, Azure Pipelines, Jenkins, Buildkite).", + "Add a CI workflow (e.g. .github/workflows/ci.yml) that runs on " + "every pull request: install deps, lint, typecheck, run tests, " + "and build. Without this, broken code can merge to main undetected.", + best_practice_ref=ref, + )) + # Downstream checks are moot without any pipeline. + results.append(CheckResult( + "ci-2", "CI/CD Pipeline", "Pipeline runs automated tests", + "fail", "critical", + "Cannot verify test execution, no CI pipeline exists.", + "See ci-1.", + best_practice_ref=ref, + )) + return results + + combined = "\n".join(repo.read(f) for f in workflow_files) + results.append(CheckResult( + "ci-1", "CI/CD Pipeline", "CI configuration exists", + "pass", "info", + f"Found {len(workflow_files)} CI config file(s).", + evidence=workflow_files, + best_practice_ref=ref, + )) + + has_test_step = runs_a_test_suite(combined) + if has_test_step: + results.append(CheckResult( + "ci-2", "CI/CD Pipeline", "Pipeline runs automated tests", + "pass", "info", + "CI runs a real test runner, not just a step with the word test in it.", + evidence=workflow_files, + best_practice_ref=ref, + )) + else: + results.append(CheckResult( + "ci-2", "CI/CD Pipeline", "Pipeline runs automated tests", + "fail", "critical", + "CI configuration exists but no step invokes a test runner. A step that only " + "prints the word test does not count.", + "Add an explicit test step (e.g. `run: npm test` / `pytest`) to the " + "workflow so regressions are caught before merge, not in production.", + evidence=workflow_files, + best_practice_ref=ref, + )) + + lint_patterns = [r"\blint\b", r"eslint", r"ruff", r"flake8", r"pylint", r"tsc\b", r"typecheck"] + has_lint_step = any(re.search(p, combined, re.IGNORECASE) for p in lint_patterns) + results.append(CheckResult( + "ci-3", "CI/CD Pipeline", "Pipeline runs lint / typecheck", + "pass" if has_lint_step else "fail", + "info" if has_lint_step else "medium", + "CI runs a lint/typecheck step." if has_lint_step else + "No lint or typecheck step detected in CI configuration.", + "" if has_lint_step else "Add a lint/typecheck step (eslint/tsc, ruff/mypy, " + "etc.) so style and type errors are caught pre-merge.", + evidence=workflow_files, + best_practice_ref=ref, + )) + + triggers_on_pr = bool(re.search(r"pull_request", combined, re.IGNORECASE)) or "merge_request" in combined.lower() + results.append(CheckResult( + "ci-4", "CI/CD Pipeline", "Pipeline gates pull requests", + "pass" if triggers_on_pr else "fail", + "info" if triggers_on_pr else "high", + "Pipeline triggers on pull/merge requests." if triggers_on_pr else + "No pull_request/merge_request trigger found, CI may only run after merge, " + "which doesn't block bad code from landing on main.", + "" if triggers_on_pr else "Add `on: pull_request` (or equivalent) so CI runs " + "before code merges, not just after.", + evidence=workflow_files, + best_practice_ref=ref, + )) + + has_deploy_step = bool(re.search(r"deploy|vercel|docker\s+push|kubectl|helm|ecs|render\.com", combined, re.IGNORECASE)) + results.append(CheckResult( + "ci-5", "CI/CD Pipeline", "Automated deployment step defined", + "pass" if has_deploy_step else "warn", + "info" if has_deploy_step else "low", + "A deployment step/integration was detected in CI config." if has_deploy_step else + "No deployment step found in CI config (deployment may be handled by an " + "external platform e.g. Vercel/Netlify git integration, verify manually).", + evidence=workflow_files, + best_practice_ref=ref, + )) + + return results + + +def check_test_scripts_defined(repo: Repo) -> CheckResult: + ref = "https://12factor.net/" + pkg = repo.package_json() + scripts = pkg.get("scripts", {}) if pkg else {} + if "test" in scripts: + cmd = str(scripts.get("test", "")) + if runs_a_test_suite(cmd): + return CheckResult( + "ci-6", "CI/CD Pipeline", "Test command defined in project manifest", + "pass", "info", + f"package.json defines a `test` script that runs a test runner: `{cmd}`.", + evidence=[f"package.json scripts.test: {cmd}"], + best_practice_ref=ref, + ) + return CheckResult( + "ci-6", "CI/CD Pipeline", "Test command defined in project manifest", + "fail", "high", + f"package.json has a `test` script, but it does not run a test runner: `{cmd}`. " + "A script that only prints a message and exits 0 makes CI green while testing nothing.", + "Point the `test` script at a real runner (vitest, jest, pytest, `node --test`), " + "so a green pipeline means the suite actually ran.", + evidence=[f"package.json scripts.test: {cmd}"], + best_practice_ref=ref, + ) + req = repo.requirements_text() + has_pytest = bool(repo.find_any(["pytest.ini", "setup.cfg", "pyproject.toml"])) and "pytest" in req.lower() + if has_pytest: + return CheckResult( + "ci-6", "CI/CD Pipeline", "Test command defined in project manifest", + "pass", "info", + "pytest configuration detected in project manifest.", + evidence=[repo.find_any(["pytest.ini", "setup.cfg", "pyproject.toml"])[0]], + best_practice_ref=ref, + ) + if pkg: + return CheckResult( + "ci-6", "CI/CD Pipeline", "Test command defined in project manifest", + "fail", "high", + "package.json exists but does not define a `test` script.", + "Add a `test` script to package.json so `npm test`/CI has a stable " + "entry point regardless of the underlying test runner.", + best_practice_ref=ref, + ) + return CheckResult( + "ci-6", "CI/CD Pipeline", "Test command defined in project manifest", + "warn", "medium", + "Could not determine project manifest / test entry point.", + "Ensure the project has a documented, single-command way to run tests.", + best_practice_ref=ref, + ) + + + + + diff --git a/skills/prod-readiness-coach/scripts/audit/checks_quality.py b/skills/prod-readiness-coach/scripts/audit/checks_quality.py new file mode 100644 index 0000000..920b45c --- /dev/null +++ b/skills/prod-readiness-coach/scripts/audit/checks_quality.py @@ -0,0 +1,129 @@ +"""Checks about tests and dependencies.""" +import json +import re +from dataclasses import asdict +from datetime import datetime +from pathlib import Path +from typing import Optional + +from .model import CheckResult +from .repo import Repo + +def check_testing_quality_gates(repo: Repo) -> list[CheckResult]: + ref = "https://martinfowler.com/testing/" + results = [] + + test_files = repo.find_any([ + "**/*.test.*", "**/*.spec.*", "**/__tests__/*", "**/test_*.py", "**/*_test.py", "tests/**", + ]) + results.append(CheckResult( + "test-1", "Testing & Quality Gates", "Automated test files exist", + "pass" if test_files else "fail", + "info" if test_files else "critical", + f"Found {len(test_files)} test file(s)." if test_files else + "No test files found anywhere in the repository.", + "" if test_files else "Add automated tests before production release, " + "at minimum, cover critical business logic, payment/billing paths, and " + "auth flows. Zero test coverage is a direct release blocker.", + evidence=test_files[:10], + best_practice_ref=ref, + )) + + test_config = repo.exists( + "jest.config.js", "jest.config.ts", "vitest.config.ts", "vitest.config.js", + "pytest.ini", "playwright.config.ts", "cypress.config.ts", "phpunit.xml", + ) + results.append(CheckResult( + "test-2", "Testing & Quality Gates", "Test runner configured", + "pass" if test_config else "warn", + "info" if test_config else "low", + f"Found test runner config: {test_config}." if test_config else + "No explicit test runner configuration file found.", + best_practice_ref=ref, + )) + + coverage_cfg = bool(re.search(r"coverage", repo.read(test_config or "") + json.dumps(repo.package_json()))) + results.append(CheckResult( + "test-3", "Testing & Quality Gates", "Coverage tracking configured", + "pass" if coverage_cfg else "warn", + "info" if coverage_cfg else "low", + "Coverage configuration detected." if coverage_cfg else + "No test coverage configuration/threshold detected.", + "" if coverage_cfg else "Track coverage (even informally) so you know " + "which critical paths are untested before shipping.", + best_practice_ref=ref, + )) + + tsconfig = repo.read("tsconfig.json") + strict_ts = bool(re.search(r'"strict"\s*:\s*true', tsconfig)) + mypy_cfg = repo.exists("mypy.ini", "setup.cfg") and "mypy" in repo.requirements_text().lower() + if tsconfig: + results.append(CheckResult( + "test-4", "Testing & Quality Gates", "Static type checking enforced", + "pass" if strict_ts else "warn", + "info" if strict_ts else "medium", + "tsconfig.json has strict mode enabled." if strict_ts else + "tsconfig.json exists but `strict` mode is not enabled.", + "" if strict_ts else "Enable `\"strict\": true` in tsconfig.json to " + "catch null/undefined and type errors before runtime.", + best_practice_ref=ref, + )) + elif mypy_cfg: + results.append(CheckResult( + "test-4", "Testing & Quality Gates", "Static type checking enforced", + "pass", "info", "mypy configuration detected for Python type checking.", + best_practice_ref=ref, + )) + + return results + + +def check_dependency_security(repo: Repo) -> list[CheckResult]: + ref = "https://docs.github.com/en/code-security/dependabot" + results = [] + + lockfile = repo.exists( + "package-lock.json", "pnpm-lock.yaml", "yarn.lock", "bun.lockb", + "poetry.lock", "Pipfile.lock", "go.sum", "Cargo.lock", + ) + results.append(CheckResult( + "dep-1", "Dependency & Supply-chain Security", "Dependency lockfile committed", + "pass" if lockfile else "fail", + "info" if lockfile else "high", + f"Found lockfile: {lockfile}." if lockfile else "No dependency lockfile found.", + "" if lockfile else "Commit a lockfile so builds are reproducible across " + "environments, without one, production can silently pull different " + "dependency versions than what was tested.", + evidence=[lockfile] if lockfile else [], + best_practice_ref=ref, + )) + + bot_cfg = repo.exists(".github/dependabot.yml", ".github/dependabot.yaml", "renovate.json", ".renovaterc") + results.append(CheckResult( + "dep-2", "Dependency & Supply-chain Security", "Automated dependency updates configured", + "pass" if bot_cfg else "warn", + "info" if bot_cfg else "low", + f"Found {bot_cfg}." if bot_cfg else "No Dependabot/Renovate configuration found.", + "" if bot_cfg else "Enable Dependabot or Renovate so security patches for " + "dependencies land automatically instead of accumulating silently.", + best_practice_ref=ref, + )) + + workflow_files = repo.find_any([".github/workflows/*.yml", ".github/workflows/*.yaml"]) + combined = "\n".join(repo.read(f) for f in workflow_files) + has_audit_step = bool(re.search(r"npm audit|pip-audit|trivy|snyk|osv-scanner|cargo audit", combined, re.IGNORECASE)) + results.append(CheckResult( + "dep-3", "Dependency & Supply-chain Security", "CI runs a dependency vulnerability scan", + "pass" if has_audit_step else "warn", + "info" if has_audit_step else "medium", + "CI includes a dependency vulnerability scan step." if has_audit_step else + "No dependency vulnerability scan (npm audit / pip-audit / Trivy / Snyk) " + "found in CI.", + "" if has_audit_step else "Add an automated vulnerability scan step to CI " + "so known-CVE dependencies are flagged before release.", + best_practice_ref=ref, + )) + + return results + + diff --git a/skills/prod-readiness-coach/scripts/audit/checks_runtime.py b/skills/prod-readiness-coach/scripts/audit/checks_runtime.py new file mode 100644 index 0000000..3e6eff7 --- /dev/null +++ b/skills/prod-readiness-coach/scripts/audit/checks_runtime.py @@ -0,0 +1,515 @@ +"""Checks about the app while it runs: logging, secrets, resilience, rollback.""" +import re +from dataclasses import asdict +from datetime import datetime +from pathlib import Path +from typing import Optional + +from .model import (MIN_DOC_WORDS, TODO_LINE_RX, CheckResult, has_substance) +from .repo import SECRET_SCAN_EXTS, Repo +from .fingerprint import StackFingerprint + +LOGGING_LIBS = [ + "pino", "winston", "bunyan", "log4js", + "structlog", "loguru", "python-json-logger", + "zerolog", "logrus", "zap", + "@opentelemetry/api", "serilog", +] + +APM_LIBS = [ + "sentry", "@sentry", "sentry-sdk", "datadog", "dd-trace", + "newrelic", "new-relic", "opentelemetry", "@vercel/otel", + "rollbar", "bugsnag", "honeycomb", "@axiomhq", "logtail", +] + + +def check_structured_logging(repo: Repo) -> list[CheckResult]: + ref = "https://sre.google/sre-book/monitoring-distributed-systems/" + results = [] + pkg = repo.package_json() + dep_names = {n.lower() for n in ((pkg.get("dependencies", {}) | pkg.get("devDependencies", {})).keys() + if pkg else [])} + # Python requirements: one package per line, strip version specifiers. + dep_names |= {re.split(r"[=<>!~\[; ]", line.strip(), 1)[0].lower() + for line in repo.requirements_text().splitlines() if line.strip() + and not line.lstrip().startswith("#")} + # A dependency named @types/pino is a type stub, not a logger. Match the package + # name (allowing a scope prefix), never a substring of a serialized blob. + def _installed(lib: str) -> bool: + lib = lib.lower() + return any(n == lib or (n.endswith(f"/{lib}") and not n.startswith("@types/")) + for n in dep_names) + + matched_logging_libs = [lib for lib in LOGGING_LIBS if _installed(lib)] + if matched_logging_libs: + results.append(CheckResult( + "log-1", "Structured Logging & Observability", "Structured logging library configured", + "pass", "info", + f"Structured logging dependency detected: {', '.join(matched_logging_libs)}.", + evidence=[f"dependency: {lib}" for lib in matched_logging_libs], + best_practice_ref=ref, + )) + else: + # Count raw console/print calls as a proxy for unstructured logging reliance. + console_hits = repo.grep(r"console\.(log|error|warn|info|debug)\s*\(") + print_hits = repo.grep(r"(? str: + """Keep a 4-char head and tail as a fingerprint; hide the rest.""" + token = token.strip().strip("\"'") + if len(token) <= 8: + return "****" + return f"{token[:4]}…{token[-4:]}" + + +def check_secrets_management(repo: Repo) -> list[CheckResult]: + ref = "https://cheatsheetseries.owasp.org/cheatsheets/Secrets_Management_Cheat_Sheet.html" + results = [] + + env_example = repo.exists(".env.example", ".env.sample", ".env.template", "env.example") + reads_env = bool(repo.grep(r"process\.env\.|os\.environ|os\.getenv|import\.meta\.env|Deno\.env|System\.getenv|ENV\[|os\.Getenv")) \ + or bool(repo.untracked_files() and any(f.startswith(".env") for f in repo.untracked_files())) + if not reads_env and not env_example: + results.append(CheckResult( + "sec-1", "Secrets & Environment Management", "Environment variable template committed", + "n/a", "info", + "No environment-variable reads detected, so a template is not required.", + best_practice_ref=ref, + )) + else: + results.append(CheckResult( + "sec-1", "Secrets & Environment Management", "Environment variable template committed", + "pass" if env_example else "fail", + "info" if env_example else "medium", + f"Found {env_example}." if env_example else + "The code reads environment variables but no .env.example / .env.sample template was found.", + "" if env_example else "Commit a .env.example listing required variable " + "names (no real values) so new environments/contributors can be " + "provisioned without guessing configuration.", + best_practice_ref=ref, + )) + + gitignore = repo.read(".gitignore") + ignores_env = bool(re.search(r"^\.env", gitignore, re.MULTILINE)) + results.append(CheckResult( + "sec-2", "Secrets & Environment Management", ".gitignore excludes env files", + "pass" if ignores_env else "fail", + "info" if ignores_env else "critical", + "`.gitignore` excludes .env* files." if ignores_env else + "`.gitignore` does not exclude .env files, real secrets could be " + "committed accidentally.", + "" if ignores_env else "Add `.env*` (with an explicit `!.env.example` " + "allow-rule) to .gitignore immediately.", + best_practice_ref=ref, + )) + + tracked_env_files = [ + f for f in repo.git_files() + if re.match(r"^(.*/)?\.env(\.[\w-]+)?$", f) + and not re.search(r"\.(example|sample|template)$", f) + ] + if tracked_env_files: + results.append(CheckResult( + "sec-3", "Secrets & Environment Management", "No real .env files committed to git", + "fail", "critical", + f"Found tracked env file(s) that are not templates: {', '.join(tracked_env_files)}.", + "Remove these from version control (`git rm --cached`), rotate any " + "secrets they contained, and confirm .gitignore now excludes them. " + "Treat any previously-committed secret as compromised.", + evidence=tracked_env_files, + best_practice_ref=ref, + )) + else: + results.append(CheckResult( + "sec-3", "Secrets & Environment Management", "No real .env files committed to git", + "pass", "info", + f"No non-template .env files are tracked by git ({len(repo.git_files())} tracked files checked).", + evidence=[f"scope: {len(repo.git_files())} git-tracked files, none matching .env"], + best_practice_ref=ref, + )) + + secret_hits = [] + scan_files = sorted({ + f for f in repo.git_files() + repo.untracked_files() + if Path(f).suffix in SECRET_SCAN_EXTS or Path(f).name.startswith(".env") + }) + for pattern, label in SECRET_PATTERNS: + rx = re.compile(pattern) + for f, lineno, line in repo.grep(pattern, paths=scan_files, flags=0, skip_comments=False, with_lineno=True): + m = rx.search(line) + token = m.group(0) if m else "" + secret_hits.append((f, lineno, label, redact(token))) + if secret_hits: + results.append(CheckResult( + "sec-4", "Secrets & Environment Management", "No hardcoded secrets in source", + "fail", "critical", + f"Found {len(secret_hits)} potential hardcoded secret(s) in source code.", + "Move all secrets to environment variables or a secrets manager " + "(Vercel/Doppler/AWS Secrets Manager/Vault). Rotate any credential " + "that was ever committed, even if later removed, it remains in git history.", + # Never put the matched line in the report: it would carry the secret + # into JSON, Markdown, CI artifacts, and the agent's context. + evidence=[f"{f}:{lineno} [{label}] {fp}" for f, lineno, label, fp in secret_hits[:10]], + best_practice_ref=ref, + )) + else: + results.append(CheckResult( + "sec-4", "Secrets & Environment Management", "No hardcoded secrets in source", + "pass", "info", + evidence=[f"scope: {len(scan_files)} code and config files scanned in the working tree"], + detail=f"No obvious hardcoded secret patterns in {len(scan_files)} tracked + untracked " + "code/config files (pattern-based scan of the working tree only, not git " + "history; not a substitute for gitleaks/truffleHog).", + best_practice_ref=ref, + )) + + env_specific_hits = repo.find_any([ + "*.env.production", "*.env.staging", "*.env.development", + "config/production*", "config/staging*", + ]) + vercel_env_ref = bool(re.search(r"VERCEL_ENV", repo.read("vercel.json") + repo.read("next.config.ts") + repo.read("next.config.js"))) + platform_env_mgmt = env_specific_hits or vercel_env_ref + results.append(CheckResult( + "sec-5", "Secrets & Environment Management", "Environment-specific configuration separation", + "pass" if platform_env_mgmt else "warn", + "info" if platform_env_mgmt else "medium", + "Environment-specific config or platform env branching (e.g. VERCEL_ENV) detected." + if platform_env_mgmt else + "No clear separation between dev/staging/production configuration was found.", + "" if platform_env_mgmt else "Confirm staging and production use distinct " + "secrets/config (not just different .env values on the same box), managed " + "via your deploy platform's environment dashboard or a secrets manager, " + "never share production credentials with lower environments.", + best_practice_ref=ref, + )) + + return results + + +def check_resilience_and_runbooks(repo: Repo) -> list[CheckResult]: + ref = "https://sre.google/sre-book/postmortem-culture/" + results = [] + + runbook_files = repo.find_any([ + "*RUNBOOK*", "*runbook*", "docs/runbook*", "docs/**/runbook*", + "*DISASTER_RECOVERY*", "*disaster-recovery*", "*INCIDENT*", + "docs/**/incident*", "docs/**/on-call*", "*ONCALL*", + ]) + with_content = [f for f in runbook_files if has_substance(repo.read(f))] + if with_content: + results.append(CheckResult( + "res-1", "Resilience & Failover", "Runbook / incident-response docs present", + "pass", "info", + f"Found operational doc(s) with real content: {', '.join(with_content[:5])}.", + evidence=with_content, + best_practice_ref=ref, + )) + elif runbook_files: + results.append(CheckResult( + "res-1", "Resilience & Failover", "Runbook / incident-response docs present", + "fail", "high", + f"Found {', '.join(runbook_files[:5])}, but it is empty or under " + f"{MIN_DOC_WORDS} words. A placeholder is not a runbook.", + "Fill it in: how you notice the failure, how you re-run or roll back, " + "and how you turn it off if it keeps failing.", + evidence=runbook_files, + best_practice_ref=ref, + )) + else: + results.append(CheckResult( + "res-1", "Resilience & Failover", "Runbook / incident-response docs present", + "fail", "high", + "No runbook, incident-response, or disaster-recovery documentation found.", + "Document what to do when the service degrades or fails: who's paged, " + "how to roll back a deploy, how to fail over the database, and how to " + "communicate status. Without this, an outage becomes a fire drill " + "instead of a checklist.", + best_practice_ref=ref, + )) + + readme = repo.read("README.md") + rollback_lines = [l.strip() for l in readme.splitlines() + if re.search(r"rollback|roll back|revert deploy|backup and restore", l, re.IGNORECASE) + and not TODO_LINE_RX.search(l)] + mentions_rollback = bool(rollback_lines) + results.append(CheckResult( + "res-2", "Resilience & Failover", "Rollback/backup procedure documented", + "pass" if mentions_rollback else "warn", + "info" if mentions_rollback else "medium", + "README references rollback/backup procedures." if mentions_rollback else + "No rollback or backup procedure documented in README.", + "" if mentions_rollback else "Document how to roll back a bad deploy and " + "how database backups/restores work, including RTO/RPO expectations.", + evidence=rollback_lines[:3], + best_practice_ref=ref, + )) + + rate_limit_hits = repo.grep(r"rate[-_]?limit|@upstash/ratelimit|express-rate-limit|throttle") + results.append(CheckResult( + "res-3", "Resilience & Failover", "Rate limiting / throttling implemented", + "pass" if rate_limit_hits else "fail", + "info" if rate_limit_hits else "medium", + f"Found {len(rate_limit_hits)} rate-limiting reference(s)." if rate_limit_hits else + "No rate limiting or throttling mechanism detected.", + "" if rate_limit_hits else "Add rate limiting on public/authenticated API " + "routes to prevent abuse, runaway costs (especially for LLM/API-billed " + "endpoints), and cascading overload during traffic spikes.", + best_practice_ref=ref, + )) + + migration_dirs = repo.find_any([ + "prisma/migrations/*", "migrations/*", "drizzle/*", "convex/schema.*", + "alembic/versions/*", "db/migrate/*", + ]) + results.append(CheckResult( + "res-4", "Resilience & Failover", "Schema migration tooling in place", + "pass" if migration_dirs else "warn", + "info" if migration_dirs else "low", + f"Found {len(migration_dirs)} migration/schema file(s)." if migration_dirs else + "No formal migration tooling detected, schema changes may be applied ad hoc.", + "" if migration_dirs else "Adopt versioned, reversible schema migrations " + "so production database changes are repeatable and auditable, and can be " + "rolled back if a deploy fails.", + best_practice_ref=ref, + )) + + cron_files = repo.find_any(["**/crons.*", "**/cron/*", "**/*scheduled*"]) + if cron_files: + # Registration-only files (e.g. Convex's crons.ts) wire a schedule to a + # target function but don't contain the job logic itself, checking + # THEM for try/catch is a false signal. Split into "registrars" vs + # files that likely hold real job logic, and only apply the + # error-handling heuristic to the latter. + registrar_pattern = re.compile(r"cronJobs\s*\(|export\s+default\s+crons\b") + registrar_files, logic_files = [], [] + for f in cron_files: + content = repo.read(f) + if registrar_pattern.search(content): + registrar_files.append(f) + else: + logic_files.append(f) + + if logic_files: + logic_content = "\n".join(repo.read(f) for f in logic_files) + has_error_handling = bool(re.search(r"try\s*{|except\s|catch\s*\(|\.catch\(", logic_content)) + results.append(CheckResult( + "res-5", "Resilience & Failover", "Scheduled jobs have error handling", + "pass" if has_error_handling else "fail", + "info" if has_error_handling else "medium", + "Scheduled job code includes try/catch or exception handling." if has_error_handling + else f"Scheduled job file(s) found ({', '.join(logic_files)}) with no visible error handling.", + "" if has_error_handling else "Wrap scheduled/cron job logic in error " + "handling with alerting on failure, a silently-failing cron job is a " + "common source of undetected production data drift.", + evidence=logic_files, + best_practice_ref=ref, + )) + elif registrar_files: + # Only a registrar was found, the actual job implementations live in + # the functions it references. Flag as informational, not a failure, + # but point the auditor at the files to check by hand. + results.append(CheckResult( + "res-5", "Resilience & Failover", "Scheduled jobs have error handling", + "info", "info", + f"Found cron schedule registration file(s) ({', '.join(registrar_files)}) " + "that reference target job functions elsewhere in the codebase. This tool " + "cannot statically resolve those target functions, so error handling in " + "the actual job logic was not verified automatically.", + "Manually confirm each scheduled job function wraps its logic in " + "try/catch (or equivalent) with failure alerting, a silently-failing " + "cron job is a common source of undetected production data drift.", + evidence=registrar_files, + best_practice_ref=ref, + )) + + return results + + +DESTRUCTIVE_MIGRATION_PATTERN = re.compile( + r"DROP\s+TABLE|DROP\s+COLUMN|TRUNCATE\s+TABLE|ALTER\s+TABLE\s+\w+\s+DROP|DELETE\s+FROM\s+\w+\s*;", + re.IGNORECASE, +) + + +def check_multi_surface_deployment(repo: Repo, fp: StackFingerprint) -> list[CheckResult]: + """First-class check for the 'paired rollback trap': when a deploy ships + more than one independently-rollback-able surface (e.g. a frontend host + + a backend/BaaS, or a docker-compose stack with its own migration step), + rolling back only one surface can silently desync the system. This used + to be a single generic bullet inside Resilience & Failover, split out + because it's a distinct failure mode with its own severity, not a + sub-case of "no rollback docs". + """ + ref = "https://vercel.com/docs/deployments/rollback-production-deployment" + results = [] + + if not fp.multi_surface: + results.append(CheckResult( + "ms-1", "Multi-Surface Deployment & Coordinated Rollback", + "Multi-surface deployment risk", + "pass", "info", + "Single deploy surface detected " + + (f"({', '.join(fp.deploy_surfaces)}). " if fp.deploy_surfaces else "(no deploy platform config found). ") + + "Standard rollback documentation (see Resilience & Failover) is sufficient, " + "there is no second surface that can drift out of sync.", + evidence=([f"deploy surface: {x}" for x in fp.deploy_surfaces] + or ["scope: no deploy platform config found in the repo"]), + best_practice_ref=ref, + )) + else: + results.append(CheckResult( + "ms-1", "Multi-Surface Deployment & Coordinated Rollback", + "Multi-surface deployment risk", + "fail", "high", + "Multiple independently-deployable surfaces detected: " + " | ".join(fp.multi_surface_evidence), + "Rolling back one surface without an explicit, matching plan for the others " + "(schema/data compatibility, which one rolls back first) can leave the system " + "in a half-rolled-back state that's worse than the original incident.", + evidence=fp.deploy_surfaces, + best_practice_ref=ref, + )) + + runbook_files = repo.find_any([ + "*RUNBOOK*", "*runbook*", "docs/runbook*", "docs/**/runbook*", + "*DISASTER_RECOVERY*", "*disaster-recovery*", "README.md", + ]) + combined_text = "\n".join(repo.read(f) for f in runbook_files).lower() + surface_mentions = sum(1 for s in fp.deploy_surfaces if s.replace("-", " ") in combined_text or s in combined_text) + mentions_coordination = bool(re.search(r"rollback|roll back|revert", combined_text)) and surface_mentions >= 2 + results.append(CheckResult( + "ms-2", "Multi-Surface Deployment & Coordinated Rollback", + "Coordinated rollback procedure documented", + "pass" if mentions_coordination else "fail", + "info" if mentions_coordination else "high", + "Runbook/README references rollback across multiple named surfaces." if mentions_coordination else + f"No documentation found that addresses rollback across all detected surfaces " + f"({', '.join(fp.deploy_surfaces)}) together. A generic 'how to roll back' note for " + "just one platform is not sufficient here.", + "" if mentions_coordination else "Document, for this specific stack, which surface " + "rolls back first, how to verify the other surface(s) are still compatible with the " + "rolled-back version, and what breaks if only one side is reverted.", + evidence=runbook_files[:5], + best_practice_ref=ref, + )) + + # Irreversible-migration safety is checked regardless of multi-surface status, + # a single-surface app can still lose data permanently from a destructive migration + # that a code rollback cannot undo. + migration_paths = repo.find_any([ + "prisma/migrations/**/*.sql", + "drizzle/*.sql", "**/drizzle/*.sql", "drizzle/**/*.sql", "**/drizzle/**/*.sql", + "alembic/versions/*.py", "**/alembic/versions/*.py", + "db/migrate/*.rb", "**/db/migrate/*.rb", + ]) + destructive_hits = [] + for f in migration_paths: + content = repo.read(f) + if DESTRUCTIVE_MIGRATION_PATTERN.search(content): + destructive_hits.append(f) + if migration_paths: + results.append(CheckResult( + "ms-3", "Multi-Surface Deployment & Coordinated Rollback", + "Irreversible migrations flagged", + "fail" if destructive_hits else "pass", + "critical" if destructive_hits else "info", + f"{len(destructive_hits)} migration file(s) contain destructive operations " + f"(DROP/TRUNCATE/DELETE) that cannot be undone by redeploying an older app version: " + f"{', '.join(destructive_hits[:5])}." if destructive_hits else + f"Scanned {len(migration_paths)} migration file(s), no destructive SQL patterns detected.", + "" if not destructive_hits else "For each destructive migration, confirm a tested backup " + "exists from immediately before it ran, and document the actual recovery step (restore from " + "backup, not 'redeploy old code', which does not undo a schema/data change already applied).", + evidence=destructive_hits, + best_practice_ref=ref, + )) + + return results + + diff --git a/skills/prod-readiness-coach/scripts/audit/fingerprint.py b/skills/prod-readiness-coach/scripts/audit/fingerprint.py new file mode 100644 index 0000000..e9640d9 --- /dev/null +++ b/skills/prod-readiness-coach/scripts/audit/fingerprint.py @@ -0,0 +1,214 @@ +"""What kind of project is this, and where does it deploy.""" +import re +from dataclasses import dataclass, field +from datetime import datetime +from pathlib import Path +from typing import Optional + +from .model import CHECK_SKIPS_BY_PROFILE, PROFILES +from .repo import Repo + +# -------------------------------------------------------------------------- +# Stack fingerprinting, deterministic detection of runtime/framework/deploy +# signals from manifests and deploy configs. This is what lets the skill +# layer load only the reference adapter file(s) relevant to THIS repo +# (progressive disclosure) instead of hardcoding stack-specific advice into +# one generic prompt. Two runs on the same commit must produce the same +# fingerprint, no LLM involvement here. +# -------------------------------------------------------------------------- + +@dataclass +class StackFingerprint: + languages: list[str] = field(default_factory=list) + package_managers: list[str] = field(default_factory=list) + frameworks: list[str] = field(default_factory=list) + deploy_surfaces: list[str] = field(default_factory=list) + 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 + multi_surface_evidence: list[str] = field(default_factory=list) + adapters_matched: list[str] = field(default_factory=list) + + def add_surface(self, name: str, evidence: str) -> None: + if name not in self.deploy_surfaces: + self.deploy_surfaces.append(name) + self.deploy_evidence.setdefault(name, []) + if evidence not in self.deploy_evidence[name]: + self.deploy_evidence[name].append(evidence) + + +def parse_compose_services(content: str) -> dict[str, str]: + """Lightweight, stdlib-only extraction of top-level service blocks from a + docker-compose file. Not a full YAML parser, deliberately heuristic and + indentation-based, which is sufficient for the standard 2-space-indented + `services:` block every compose file uses, and keeps this tool dependency-free. + """ + services: dict[str, str] = {} + lines = content.splitlines() + in_services = False + current: Optional[str] = None + buf: list[str] = [] + for line in lines: + if re.match(r"^services:\s*$", line): + in_services = True + continue + if not in_services: + continue + if re.match(r"^\S", line): # dedented back to top-level key, services block ended + if current: + services[current] = "\n".join(buf) + break + m = re.match(r"^ ([\w.-]+):\s*$", line) + if m: + if current: + services[current] = "\n".join(buf) + current = m.group(1) + buf = [] + elif current is not None: + buf.append(line) + if current: + services[current] = "\n".join(buf) + return services + + +def detect_stack_fingerprint(repo: Repo) -> StackFingerprint: + fp = StackFingerprint() + pkg = repo.package_json() + deps = {**pkg.get("dependencies", {}), **pkg.get("devDependencies", {})} + + # --- Languages & package managers ------------------------------------- + if pkg: + fp.languages.append("typescript" if repo.exists("tsconfig.json") else "javascript") + if repo.exists("bun.lock", "bun.lockb") or pkg.get("packageManager", "").startswith("bun"): + fp.package_managers.append("bun") + elif repo.exists("pnpm-lock.yaml"): + fp.package_managers.append("pnpm") + elif repo.exists("yarn.lock"): + fp.package_managers.append("yarn") + elif repo.exists("package-lock.json"): + fp.package_managers.append("npm") + if repo.requirements_text() or repo.exists("requirements.txt", "pyproject.toml", "Pipfile"): + fp.languages.append("python") + fp.package_managers.append("pip" if repo.exists("requirements.txt") else "poetry/pipenv") + if repo.exists("go.mod"): + fp.languages.append("go") + fp.package_managers.append("go modules") + + # --- Frameworks --------------------------------------------------------- + framework_deps = { + "next": "nextjs", "convex": "convex", "@remix-run/dev": "remix", + "express": "express", "fastify": "fastify", "@sveltejs/kit": "sveltekit", + "nuxt": "nuxt", "@nestjs/core": "nestjs", "astro": "astro", + } + for dep_name, fw in framework_deps.items(): + if dep_name in deps: + fp.frameworks.append(fw) + req_text = repo.requirements_text().lower() + for needle, fw in (("django", "django"), ("flask", "flask"), ("fastapi", "fastapi")): + if needle in req_text: + fp.frameworks.append(fw) + + # --- Deploy surfaces ------------------------------------------------ + member_vercel = repo.find_any(["apps/*/vercel.json", "packages/*/vercel.json"]) + if repo.exists("vercel.json") or repo.exists(".vercel") or member_vercel or "vercel-build" in pkg.get("scripts", {}): + fp.add_surface("vercel", repo.exists("vercel.json", ".vercel") or (member_vercel[0] if member_vercel else "package.json scripts.vercel-build")) + def anywhere(*names: str) -> list[str]: + """Root or a workspace member (apps/*, packages/*, services/*).""" + pats = list(names) + [f"{d}/*/{n}" for d in ("apps", "packages", "services") for n in names] + return repo.find_any(pats) + for surface, names in (("netlify", ("netlify.toml",)), ("fly", ("fly.toml",)), + ("cloudflare-workers", ("wrangler.toml", "wrangler.jsonc", "wrangler.json")), + ("render", ("render.yaml",)), ("heroku", ("Procfile",))): + hits = anywhere(*names) + if hits: + fp.add_surface(surface, hits[0]) + dockerfiles = repo.find_any(["**/Dockerfile", "Dockerfile"]) + compose_files = repo.find_any(["docker-compose.yml", "docker-compose.yaml", "compose.yml", "compose.yaml"]) + if dockerfiles or compose_files: + fp.add_surface("docker", ", ".join(dockerfiles[:3] + compose_files[:2])) + convex_schema = repo.exists("convex/schema.ts", "convex/schema.js") or repo.find_any(["**/convex/schema.ts", "**/convex/schema.js"]) + if "convex" in fp.frameworks or convex_schema: + fp.add_surface("convex", "convex/ directory or convex dependency") + + # --- Runtimes ------------------------------------------------------- + if "convex" in fp.frameworks: + fp.runtimes.append("convex-v8-isolate") + if repo.grep(r'^[\'"]use node[\'"]', paths=repo.find_any(["**/convex/**/*.ts", "**/convex/**/*.js"])): + fp.runtimes.append("convex-node") + if "cloudflare-workers" in fp.deploy_surfaces: + fp.runtimes.append("cloudflare-v8-isolate") + if "nextjs" in fp.frameworks: + fp.runtimes.append("node") + if repo.grep(r"runtime\s*[:=]\s*['\"]edge['\"]") or repo.exists("middleware.ts", "middleware.js"): + fp.runtimes.append("edge") + if "python" in fp.languages: + fp.runtimes.append("python") + + # --- Migration tooling ------------------------------------------------ + if repo.find_any(["prisma/migrations/*"]): + fp.migration_tooling.append("prisma") + if repo.exists("drizzle.config.ts", "drizzle.config.js") or repo.find_any(["drizzle/*", "**/drizzle/*"]): + fp.migration_tooling.append("drizzle") + if repo.find_any(["alembic/versions/*"]): + fp.migration_tooling.append("alembic") + if repo.find_any(["db/migrate/*"]): + fp.migration_tooling.append("rails-migrations") + if convex_schema: + fp.migration_tooling.append("convex-schema") + + # --- Multi-surface detection ------------------------------------------ + # Most Next.js apps on Vercel have no vercel.json. If Next.js is present and + # no other frontend host was detected, assume Vercel (flagged as inferred). + root_docker = bool(repo.exists("Dockerfile", "docker-compose.yml", "docker-compose.yaml", "compose.yml", "compose.yaml")) + frontend_hosts = ("vercel", "netlify", "cloudflare-workers", "render", "heroku") + if "nextjs" in fp.frameworks and not any(sfc in fp.deploy_surfaces for sfc in frontend_hosts) \ + and not root_docker and not (repo.exists("fly.toml")): + fp.add_surface("vercel", "inferred: Next.js with no other frontend host config (confirm)") + + independently_deployable = {"vercel", "netlify", "fly", "cloudflare-workers", "render", "heroku", "convex"} + matched_platforms = [s for s in fp.deploy_surfaces if s in independently_deployable] + if len(matched_platforms) >= 2: + fp.multi_surface = True + fp.multi_surface_evidence.append( + f"Multiple independently-deployable platforms detected: {', '.join(sorted(matched_platforms))}. " + "These deploy and roll back on separate timelines." + ) + for cf in compose_files: + content = repo.read(cf) + services = parse_compose_services(content) + migrate_services = [s for s, block in services.items() + if re.search(r"migrat", s, re.IGNORECASE) or re.search(r"migrat", block, re.IGNORECASE)] + app_services = [s for s in services if s not in migrate_services] + if migrate_services and app_services: + fp.multi_surface = True + fp.multi_surface_evidence.append( + f"{cf}: dedicated migration service(s) {migrate_services} run alongside " + f"{len(app_services)} app/data service(s) ({', '.join(app_services[:6])}). " + "A bad deploy needs the app rolled back AND the migration's effects accounted for." + ) + + # --- Adapter matching (progressive disclosure) ------------------------- + if "vercel" in fp.deploy_surfaces and "nextjs" in fp.frameworks: + fp.adapters_matched.append("nextjs-vercel") + if "convex" in fp.deploy_surfaces: + fp.adapters_matched.append("convex") + if "fly" in fp.deploy_surfaces: + fp.adapters_matched.append("fly") + if "cloudflare-workers" in fp.deploy_surfaces: + fp.adapters_matched.append("cloudflare-workers") + if "netlify" in fp.deploy_surfaces: + fp.adapters_matched.append("netlify") + + fp.languages = sorted(set(fp.languages)) + fp.package_managers = sorted(set(fp.package_managers)) + fp.frameworks = sorted(set(fp.frameworks)) + fp.runtimes = sorted(set(fp.runtimes)) + fp.migration_tooling = sorted(set(fp.migration_tooling)) + return fp + + diff --git a/skills/prod-readiness-coach/scripts/audit/model.py b/skills/prod-readiness-coach/scripts/audit/model.py new file mode 100644 index 0000000..2ca637d --- /dev/null +++ b/skills/prod-readiness-coach/scripts/audit/model.py @@ -0,0 +1,210 @@ +"""Findings, categories, waivers, contradictions, and the substance helpers.""" +import json +import re +from dataclasses import dataclass, field +from datetime import datetime, timezone +from pathlib import Path +from typing import Optional + +# -------------------------------------------------------------------------- +# Data model +# -------------------------------------------------------------------------- + +SEVERITY_ORDER = {"critical": 0, "high": 1, "medium": 2, "low": 3, "info": 4, "pass": 5} +SEVERITY_LABEL = { + "critical": "🔴 CRITICAL", + "high": "🟠 HIGH", + "medium": "🟡 MEDIUM", + "low": "🔵 LOW", + "info": "ℹ️ INFO", + "pass": "✅ PASS", +} +# Points removed from the 100-point category score when a check with this +# severity fails. Weighted so a single CRITICAL miss visibly tanks the score. +SEVERITY_PENALTY = {"critical": 30, "high": 18, "medium": 10, "low": 5, "info": 0} + + +@dataclass +class CheckResult: + id: str + category: str + title: str + status: str # "pass" | "fail" | "warn" | "info" + severity: str # "critical" | "high" | "medium" | "low" | "info" + detail: str + recommendation: str = "" + evidence: list[str] = field(default_factory=list) + best_practice_ref: str = "" + # "verified": structural evidence (file/config/dependency exists). + # "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", "ms-2"} + +# 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."), +] + + +# A control that exists in name only is a costume. `docs/runbook.md` can be an +# empty file; a CI step can be `echo "test skipped"`; a README "rollback" can be +# an unchecked TODO. Existence is not function, so these checks look at content. +TEST_RUNNER_RX = re.compile( + r"\b(?:npm|pnpm|yarn|bun)\s+(?:run\s+)?test\b|\bnpx?\s+(?:vitest|jest|mocha|ava|playwright|cypress)\b" + r"|\b(?:vitest|jest|mocha|ava|karma)\b|\bpytest\b|\bpython\s+-m\s+(?:pytest|unittest)\b" + r"|\bgo\s+test\b|\bcargo\s+test\b|\brspec\b|\bphpunit\b|\bdotnet\s+test\b|\bmvn\s+test\b" + r"|\bgradle(?:w)?\s+test\b|\bnode\s+--test\b|\brake\s+test\b", + re.IGNORECASE, +) +# An unchecked task box, or a promise to do it later, is not a procedure. +TODO_LINE_RX = re.compile(r"^\s*[-*]?\s*\[\s\]|\b(?:todo|tbd|coming soon|someday|we should|should probably)\b", + re.IGNORECASE) +MIN_DOC_WORDS = 50 + + +def runs_a_test_suite(text: str) -> bool: + """True when text invokes a real test runner, not merely the word 'test'. + Shell strings are stripped first so `echo "test skipped"` does not count.""" + without_strings = re.sub(r"""(['"]).*?\1""", " ", text or "", flags=re.S) + return bool(TEST_RUNNER_RX.search(without_strings)) + + +def has_substance(text: str, min_words: int = MIN_DOC_WORDS) -> bool: + """A document with almost no prose is a placeholder, not documentation.""" + body = "\n".join(l for l in (text or "").splitlines() if not l.lstrip().startswith("#")) + return len(body.split()) >= min_words + + +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 + # The waiver file lives in the audited repo, so its text is untrusted input. + # Flatten it to one safe line: no newlines to break out of the table, no + # pipes to forge columns, no leading # to forge a heading, and a length cap. + good[str(w["id"]).strip()] = {k: _one_safe_line(w[k]) for k in WAIVER_FIELDS} + return good, problems + + +def _one_safe_line(value, limit: int = 300) -> str: + """Collapse untrusted repo text to a single table-safe cell.""" + text = " ".join(str(value).split()) + text = text.replace("|", "\\|").lstrip("#>*-= ") + return text[:limit] + ("..." if len(text) > limit else "") + + +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 +# profile is "unknown" and runtime checks report insufficient evidence. +PROFILES = ("web-app", "api", "worker", "cli", "library", "unknown") +_SERVICE_ONLY = {"log-3", "res-3"} # needs an HTTP surface +_DEPLOYED_ONLY = {"log-1", "log-2", "log-4", "res-1", "res-2", "res-4", "res-5", + "sec-5", "ms-1", "ci-5"} | _SERVICE_ONLY # needs to run somewhere +CHECK_SKIPS_BY_PROFILE = { + "web-app": set(), + "api": set(), + "worker": _SERVICE_ONLY, + "cli": _DEPLOYED_ONLY, + "library": _DEPLOYED_ONLY, + # Nothing recognizable was detected. Runtime-specific checks are reported + # as n/a ("insufficient evidence") rather than failed web-app checks. + "unknown": _DEPLOYED_ONLY, +} + + +@dataclass +class Category: + key: str + title: str + description: str + best_practice_ref: str + checks: list[CheckResult] = field(default_factory=list) + + @property + def score(self) -> int: + score = 100 + for c in self.checks: + if c.status == "fail": + 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) + + @property + def blocking_count(self) -> int: + return sum(1 for c in self.checks if c.status == "fail" and c.severity == "critical") + + diff --git a/skills/prod-readiness-coach/scripts/audit/repo.py b/skills/prod-readiness-coach/scripts/audit/repo.py new file mode 100644 index 0000000..3e1aed8 --- /dev/null +++ b/skills/prod-readiness-coach/scripts/audit/repo.py @@ -0,0 +1,172 @@ +"""Reading a repository safely: tracked files, greps, symlink containment.""" +import fnmatch +import json +import re +import subprocess +from dataclasses import asdict +from datetime import datetime +from pathlib import Path +from typing import Optional + +# -------------------------------------------------------------------------- +# Repo scanning helpers +# -------------------------------------------------------------------------- + +DEFAULT_EXCLUDE_DIRS = { + ".git", "node_modules", ".next", "dist", "build", "out", "coverage", + ".venv", "venv", "__pycache__", ".pnpm", ".turbo", ".vercel", "vendor", + ".cache", "target", ".idea", ".vscode", +} + +CODE_EXTS = { + ".ts", ".tsx", ".js", ".jsx", ".mjs", ".cjs", ".py", ".go", ".rb", + ".java", ".kt", ".rs", ".php", ".cs", +} +# Secrets live in config as often as in code, so the secret scan covers more. +SECRET_SCAN_EXTS = CODE_EXTS | { + ".yaml", ".yml", ".json", ".toml", ".env", ".sh", ".tf", ".ini", ".cfg", ".properties", +} +COMMENT_LINE_RX = re.compile(r"^\s*(#|//|/\*|\*|