From 8d4d78f13f87d9c812a18439c854fd32f9d541ad Mon Sep 17 00:00:00 2001 From: Richard Hope Date: Sun, 16 Aug 2026 16:02:47 +1000 Subject: [PATCH] Make the documentation checkable, and close its gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #150 `docs/` was already substantial. What it lacked was anything holding it to the code, so this adds the checks first and then fixes what they found — which is the only order that keeps the answer true next month. **`make docs-check` grew two checks.** A documented `make` target that no longer exists is a stale doc in the worst possible place: the first command a new contributor runs. A documented `ICEBERG_*` variable that no setting backs leaves an operator configuring something nothing reads. Both are matched against the Makefile and the settings classes respectively, so neither can drift. The `make X` pattern deliberately only matches code — inline backticks or a line in a fenced block — because matching prose catches "make it clear" and "make the engine", and a check that cries wolf is one people turn off. It also now reads only the files git tracks. `rglob("*.md")` walks a contributor's local tool directories too, which is how it exits non-zero on a permission error in a file that has nothing to do with the project. **`tests/test_docs_invariants.py` checks the claims a regex cannot.** All seven failed when written, which is what made them worth writing: - CONTRIBUTING named `make check` and `make docs-check` but not `lint`, `type`, `test`, `version-check` or `rehearse`. A contributor who runs everything the guide names and still fails CI has been told the wrong thing, and the second time it happens they stop running any of it. It now carries the whole gate as a table, in CI's order, with what each one proves. - Four documents were orphaned — nothing in the repository linked to `retention.md`, `runbooks/controlled-pilot.md`, `spikes/python-3.14-compat.md` or `web/README.md`. A guide nobody links to is the one that rots, because nothing points at it when the thing it describes changes. - ADR 0013 and two runbooks were missing from the index. A runbook is read under pressure by somebody who did not write it; if it is not in the index it will not be found in time to matter. - SECURITY.md answered how to report a vulnerability but not "is my version still getting fixes?" — it now points at the support window, and says fixes are announced as GitHub Security Advisories with the identifier in the CHANGELOG. - The backup/restore runbook claimed a rehearsal without naming one. It now names `make rehearse`, and the test asserts the two reference each other — a runbook that claims to be exercised by a script nobody can find is a claim, not evidence. Against #150's criteria: the production-install and clean-room paths already existed; backup and restore are now literally rehearsed in CI rather than described; security reporting and the support window are public; the contributor gate is CI's gate and a test says so; and the docs are versioned with the release that ships them and checked automatically on every pull request. make check green: 1826 passed. --- CHANGELOG.md | 6 ++ CONTRIBUTING.md | 29 ++++++- Makefile | 2 +- SECURITY.md | 11 +++ docs/README.md | 10 +++ docs/runbooks/backup-restore.md | 6 ++ scripts/check_docs.py | 133 +++++++++++++++++++++++++++-- tests/test_docs_invariants.py | 145 ++++++++++++++++++++++++++++++++ 8 files changed, 329 insertions(+), 13 deletions(-) create mode 100644 tests/test_docs_invariants.py diff --git a/CHANGELOG.md b/CHANGELOG.md index a5f70ec..0103b39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,12 @@ Reversible, and the downgrade restores the previous shape exactly. request against real Postgres: every revision applied and reversed one at a time, the previous release's schema upgraded onto the current tree, and a backup destroyed and restored. +### Changed + +- `make docs-check` now also verifies that every documented `make` target and every `ICEBERG_*` + setting the docs name still exists, and reads only the files git tracks — so it no longer fails + on a contributor's unrelated local directory (#150). + ## [0.1.0] — unreleased The first tagged release. Everything below is the state of the project at the point a version diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fe68fb3..fd8c4bf 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -14,8 +14,23 @@ make check make docs-check ``` -`make check` is the same lint, type, and test gate used by CI. `make images-verify` and -`make helm-verify` are required when changing deployment or container files. +`make check` is `make lint`, `make type` and `make test` — the same gate CI runs, in the same +order. The rest of CI is reachable the same way, so nothing in the pipeline is a command you can +only run by pushing: + +| Command | What it proves | When you need it | +|---|---|---| +| `make check` | lint, types, and the whole test suite | always | +| `make docs-check` | links, `make` targets, and settings the docs name still exist | always | +| `make version-check` | every shipped component declares the same version | before tagging | +| `make images-verify` | both images build, serve, and hold the ADR 0002 boundary | container or dependency changes | +| `make helm-verify` | the rendered chart carries no engine database credentials | chart changes | +| `make rehearse` | migrations apply and reverse one at a time, and a backup restores | schema changes | + +`make rehearse` needs a scratch Postgres and the libpq client tools; CI runs the identical script +against its own service container, so what you can rehearse locally is what every pull request +already rehearses. `make sync` also installs the pre-commit hooks, one of which is the gitleaks +scan — CI runs it over the full history either way, but by then the secret is committed. ## Design boundaries @@ -29,11 +44,17 @@ make docs-check - Browser routes call the API contract and remain CSRF-protected; do not add inline scripts or styles to templates. - Schema changes require an Alembic revision, SQLite migration coverage, and PostgreSQL upgrade, - downgrade, and re-apply coverage. + downgrade, and re-apply coverage — `make rehearse` is that coverage, and it also compares the + restored schema against the models, which is how three drifts SQLite cannot express were found. +- Every migration must be additive with respect to the **previous release**: a rolling upgrade + briefly runs the old API against the new schema ([`docs/releases.md`](docs/releases.md)). ## Pull requests -Describe the user outcome, threat-model impact, migration/rollback behavior, and tests. Keep +Describe the user outcome, threat-model impact, migration/rollback behavior, and tests. A change +that alters operator-visible behaviour needs a `CHANGELOG.md` entry under **Unreleased**; a change +that adds a migration needs a **Migrations** line in it, which +[`tests/test_release_invariants.py`](tests/test_release_invariants.py) enforces at release time. Keep changes focused. Do not include credentials, live source content, or unmasked canaries in commits, fixtures, screenshots, logs, or issue comments. Reviewers may request a clean-room walkthrough for deployment or recovery claims; document what was actually exercised rather than implying that a diff --git a/Makefile b/Makefile index a46b348..82953aa 100644 --- a/Makefile +++ b/Makefile @@ -40,7 +40,7 @@ test: ## pytest across all workspace members check: lint type test ## Everything CI runs -docs-check: ## Verify repository-local Markdown links +docs-check: ## Verify links, documented make targets, and named settings uv run python scripts/check_docs.py # A release is one number applied to both images, the chart's appVersion, and diff --git a/SECURITY.md b/SECURITY.md index 27b2034..d1a7199 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -22,6 +22,17 @@ Provide the affected version or commit, deployment mode, a minimal reproduction credentials, impact, and any proposed mitigation. Use synthetic canaries and redact all source identifiers that are not needed to reproduce the issue. +## Supported versions + +Security fixes land on the current minor and are backported to the supported previous one. Which +that is, and for how long, is in [`docs/releases.md`](docs/releases.md) — along with how to verify +that the release you are running is the one this project published. Only tagged releases are +supported; a commit on `main` may be perfectly good, but nothing rehearses an upgrade from it and +no artifact is signed for it. + +Fixes are announced as a GitHub Security Advisory on this repository, which is also what populates +the ecosystem vulnerability databases. The `CHANGELOG.md` entry carries the advisory identifier. + ## Handling expectations The maintainers will acknowledge a private report when they can, reproduce it in an isolated diff --git a/docs/README.md b/docs/README.md index f495f51..9369f91 100644 --- a/docs/README.md +++ b/docs/README.md @@ -11,13 +11,22 @@ Design specification and reference docs. Start with [`../ARCHITECTURE.md`](../AR - [`rules.md`](./rules.md) — detection engine, rule packs, suppressions - [`secret-validation.md`](./secret-validation.md) — opt-in credential liveness contracts and controls - [`security.md`](./security.md) — threat model and mitigations +- [`notifications.md`](./notifications.md) — channels, the delivery outbox, escalation, payloads +- [`retention.md`](./retention.md) — what is pruned, when, and what is kept forever - [`deployment.md`](./deployment.md) — docker-compose (dev) + Helm (prod) - [`releases.md`](./releases.md) — versioning, support window, compatibility, upgrade and rollback - [`runbooks/production-install.md`](./runbooks/production-install.md) — production-oriented installation and go-live checks - [`runbooks/backup-restore.md`](./runbooks/backup-restore.md) — isolated recovery rehearsal +- [`runbooks/key-rotation.md`](./runbooks/key-rotation.md) — rotating the master key and the + fingerprint pepper without losing triage history +- [`runbooks/controlled-pilot.md`](./runbooks/controlled-pilot.md) — running a first scan against + a real source, with the blast radius bounded - [`runbooks/release.md`](./runbooks/release.md) — cutting a release, and verifying a published one - [`backlog.md`](./backlog.md) — milestones, epics, and issues (mirrors GitHub) +- [`spikes/python-3.14-compat.md`](./spikes/python-3.14-compat.md) — why the workspace pins 3.14, + and what had to be true first +- [`../web/README.md`](../web/README.md) — vendoring the console's frontend assets ## Decision records (ADRs) - [0001 — Job queue: Redis + Dramatiq](./adr/0001-job-queue.md) @@ -32,3 +41,4 @@ Design specification and reference docs. Start with [`../ARCHITECTURE.md`](../AR - [0010 — Credential liveness validation](./adr/0010-secret-liveness-validation.md) - [0011 — Credential correlation & exposure clusters](./adr/0011-credential-correlation.md) - [0012 — Rotation guidance & remediation evidence](./adr/0012-remediation-evidence.md) +- [0013 — Incremental & resumable scanning](./adr/0013-incremental-scanning.md) diff --git a/docs/runbooks/backup-restore.md b/docs/runbooks/backup-restore.md index 0190d56..78eab65 100644 --- a/docs/runbooks/backup-restore.md +++ b/docs/runbooks/backup-restore.md @@ -4,6 +4,12 @@ IcebergSST's database contains finding locations, triage, audit history, task st credential references. A database backup without the matching master key and fingerprint pepper is not a recoverable backup. Never put either value in a command argument, log, ticket, or repository. +> **This procedure is rehearsed, not merely written.** `make rehearse` — which CI runs on every +> pull request against a real Postgres — takes a backup, destroys the database, restores it, and +> asserts both that a row written before the dump came back and that the restored schema still +> matches the models. A backup nobody has restored is a plan; see +> [`release.md`](./release.md) for where it sits in an upgrade. + ## Before the drill - Record the exact application/chart/image versions and database migration revision. diff --git a/scripts/check_docs.py b/scripts/check_docs.py index 0623474..3300f94 100644 --- a/scripts/check_docs.py +++ b/scripts/check_docs.py @@ -1,22 +1,81 @@ #!/usr/bin/env python3 -"""Fail when a repository-local Markdown link points at a missing file.""" +"""Fail when the documentation refers to something that is not there. + +Three kinds of rot, each cheap to detect and expensive to find by hand: + +* **A link to a file that has moved.** The original check, and still the common + one — a runbook renamed with its references left behind reads as a complete + document right up to the moment somebody clicks. +* **A `make` target that no longer exists.** The docs are full of `make check`, + `make rehearse`, `make helm-verify`. A renamed target leaves an instruction + that fails on the first command a new contributor runs, which is the worst + possible place to have a stale doc. +* **A configuration variable that no longer exists.** Every setting is + `ICEBERG_`-prefixed, which makes them findable in prose without a vocabulary + list. Checked against the settings classes rather than against `.env.example`: + the example file is the *compose* surface (`tests/test_deploy_invariants.py` + holds it to that), while a renamed field is the drift that leaves an operator + setting a variable nothing reads. + +Scoped to files git tracks. `rglob("*.md")` also walks a contributor's local tool +directories, which is how this exits non-zero on a permission error in a file +that has nothing to do with the project — and a check that fails for reasons +unrelated to the change is one people learn to ignore. +""" from __future__ import annotations import re +import subprocess import sys from pathlib import Path from urllib.parse import unquote, urlsplit LINK = re.compile(r"!?\[[^\]]*\]\(([^)]+)\)") +#: `make check` **as code** — inline backticks, or a line in a fenced block. +#: Matching it in prose would catch "make it clear" and "make the engine", which +#: is how a check like this ends up being one people disable. +MAKE_TARGET = re.compile( + r"`make ([a-z][a-z0-9-]*)[^`]*`" # inline code + r"|^\s{0,3}make ([a-z][a-z0-9-]*)", # a command line in a fenced block + re.MULTILINE, +) + +#: A Makefile target definition, ignoring the pattern rules and `.PHONY`. +MAKE_DEFINITION = re.compile(r"^([a-z][a-z0-9-]*):", re.MULTILINE) + +#: A configuration variable, wherever it is written. All of them share a prefix, +#: which is what makes this findable without a vocabulary list. +ENV_VARIABLE = re.compile(r"\bICEBERG_[A-Z0-9_]+\b") + +#: Named in documentation but not a settings field: variables read by something +#: other than pydantic-settings, or supplied per-run rather than configured. +ENV_EXEMPT = frozenset( + { + # Read by the compose stack rather than by pydantic-settings: it selects + # the published port, which no application code has an opinion about. + "ICEBERG_API_PORT", + } +) + + +def tracked_documents(root: Path) -> list[Path]: + """Every Markdown file git tracks, as absolute paths.""" + listed = subprocess.run( # git, with a fixed argument list + ["git", "ls-files", "-z", "*.md"], # noqa: S607 # resolved from PATH, as everywhere else + cwd=root, + capture_output=True, + text=True, + check=True, + ) + return [root / name for name in listed.stdout.split("\0") if name] + def local_targets(root: Path) -> list[tuple[Path, str, Path]]: + """Repository-local links that point at nothing.""" failures: list[tuple[Path, str, Path]] = [] - for document in sorted(root.rglob("*.md")): - ignored = {".git", ".venv", ".mypy_cache", ".pytest_cache", ".ruff_cache"} - if any(part in ignored for part in document.parts): - continue + for document in tracked_documents(root): text = document.read_text(encoding="utf-8") for raw in LINK.findall(text): target = raw.strip().split(maxsplit=1)[0].strip("<>") @@ -29,12 +88,70 @@ def local_targets(root: Path) -> list[tuple[Path, str, Path]]: return failures +def make_targets(root: Path) -> list[tuple[Path, str]]: + """Documented `make` commands with no matching target.""" + defined = set(MAKE_DEFINITION.findall((root / "Makefile").read_text(encoding="utf-8"))) + if not defined: # pragma: no cover — the pattern would be silently checking nothing + raise ValueError("no targets found in the Makefile; the pattern is wrong") + + failures: list[tuple[Path, str]] = [] + for document in tracked_documents(root): + for inline, fenced in MAKE_TARGET.findall(document.read_text(encoding="utf-8")): + name = inline or fenced + if name and name not in defined: + failures.append((document, name)) + return failures + + +def settings_variables() -> set[str]: + """Every `ICEBERG_*` variable the settings classes actually read.""" + from iceberg_core.config import ( + ApiSettings, + CoreSettings, + EngineSettings, + SecretStoreSettings, + ) + + # Both roles, because the deployment docs configure both and a variable only + # the engine reads is no less real than one only the API does. + names: set[str] = set() + for model in (CoreSettings, SecretStoreSettings, ApiSettings, EngineSettings): + prefix = model.model_config.get("env_prefix", "") + names.update(f"{prefix}{field}".upper() for field in model.model_fields) + return names + + +def env_variables(root: Path) -> list[tuple[Path, str]]: + """Documented configuration variables that no settings field backs.""" + documented = settings_variables() | ENV_EXEMPT + if not documented: # pragma: no cover — same reasoning as above + raise ValueError("no settings fields found; the reflection is wrong") + + failures: list[tuple[Path, str]] = [] + for document in tracked_documents(root): + for name in ENV_VARIABLE.findall(document.read_text(encoding="utf-8")): + if name not in documented: + failures.append((document, name)) + return failures + + def main() -> int: root = Path(__file__).resolve().parents[1] - failures = local_targets(root) - for document, target, resolved in failures: + failed = False + + for document, target, resolved in local_targets(root): + failed = True print(f"{document.relative_to(root)}: missing link {target!r} -> {resolved}") - return int(bool(failures)) + for document, name in make_targets(root): + failed = True + print(f"{document.relative_to(root)}: documents `make {name}`, which the Makefile has not") + for document, name in env_variables(root): + failed = True + print(f"{document.relative_to(root)}: names {name}, which no setting backs") + + if not failed: + print(f"{len(tracked_documents(root))} documents check out") + return int(failed) if __name__ == "__main__": diff --git a/tests/test_docs_invariants.py b/tests/test_docs_invariants.py new file mode 100644 index 0000000..5ac20a1 --- /dev/null +++ b/tests/test_docs_invariants.py @@ -0,0 +1,145 @@ +"""The documentation says what the project actually does (#150). + +`scripts/check_docs.py` catches the mechanical rot — a moved file, a renamed +target, a setting that no longer exists. These are the claims it cannot check +from a regular expression, and each one is a promise the docs make on behalf of +somebody else: + +* **CONTRIBUTING lists the gate CI runs.** A contributor who runs everything the + guide names and still fails CI has been told the wrong thing, and the second + time it happens they stop running any of it. +* **Every document is reachable.** A guide nobody links to is a guide nobody + reads — and it is also the one that rots, because nothing points at it when the + thing it describes changes. +* **Every ADR is indexed.** Same reasoning, and the index is how the decision + record works as a record at all. +""" + +import importlib.util +import re +from pathlib import Path +from types import ModuleType + +ROOT = Path(__file__).resolve().parents[1] +CI = ROOT / ".github/workflows/ci.yml" +CONTRIBUTING = ROOT / "CONTRIBUTING.md" +DOCS = ROOT / "docs" + +#: `run: make check`, in the workflow. +_CI_MAKE = re.compile(r"run:\s*make ([a-z][a-z0-9-]*)") + + +def _load(name: str) -> ModuleType: + """Import a `scripts/` module by path — see `test_release_invariants.py`.""" + path = ROOT / "scripts" / f"{name}.py" + spec = importlib.util.spec_from_file_location(name, path) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +check_docs = _load("check_docs") + + +def _linked_from(document: Path) -> set[str]: + """Repository-local link targets in one document, resolved to repo paths.""" + targets: set[str] = set() + for raw in check_docs.LINK.findall(document.read_text(encoding="utf-8")): + target = raw.strip().split(maxsplit=1)[0].strip("<>").split("#")[0] + if not target or target.startswith(("http", "mailto:")): + continue + resolved = (document.parent / target).resolve() + if resolved.is_relative_to(ROOT): + targets.add(str(resolved.relative_to(ROOT))) + return targets + + +# ─── The contributor gate is the CI gate ────────────────────────────────────── + + +def test_contributing_names_every_make_target_ci_runs() -> None: + """A contributor who runs everything the guide names and still fails CI has + been told the wrong thing — and the second time, they stop running any of it.""" + ci_targets = set(_CI_MAKE.findall(CI.read_text(encoding="utf-8"))) + assert ci_targets, "no `run: make …` steps found; the pattern is checking nothing" + + guide = CONTRIBUTING.read_text(encoding="utf-8") + missing = sorted(name for name in ci_targets if f"make {name}" not in guide) + + assert missing == [], f"CI runs these and CONTRIBUTING does not mention them: {missing}" + + +def test_contributing_names_the_scripts_ci_runs_directly() -> None: + """The rehearsal is a script rather than a bare `make` step in CI, so the + pattern above cannot see it — and it is the longest-running local check a + contributor might reasonably want to know about before pushing.""" + workflow = CI.read_text(encoding="utf-8") + guide = CONTRIBUTING.read_text(encoding="utf-8") + + assert "./scripts/rehearse_release.sh" in workflow + assert "make rehearse" in guide + + +# ─── Everything is reachable ────────────────────────────────────────────────── + + +def test_every_document_is_linked_from_somewhere() -> None: + """A guide nobody links to is a guide nobody reads, and it is the one that + rots — nothing points at it when the thing it describes changes.""" + documents = { + str(path.relative_to(ROOT)) for path in check_docs.tracked_documents(ROOT) if path != ROOT + } + linked: set[str] = set() + for path in check_docs.tracked_documents(ROOT): + linked |= _linked_from(path) + + # The entry points nothing needs to link to: a reader arrives at them by + # convention, and GitHub surfaces all four in its own UI. + entry_points = {"README.md", "CONTRIBUTING.md", "SECURITY.md", "SUPPORT.md", "CLAUDE.md"} + orphans = sorted(documents - linked - entry_points) + + assert orphans == [], f"nothing links to: {orphans}" + + +def test_every_adr_is_in_the_index() -> None: + """The index is how a decision record works as a record.""" + index = (DOCS / "README.md").read_text(encoding="utf-8") + records = sorted(path.name for path in (DOCS / "adr").glob("0*.md")) + + assert records, "no ADRs found; the glob is looking in the wrong place" + missing = [name for name in records if name not in index] + assert missing == [], f"not indexed in docs/README.md: {missing}" + + +def test_every_runbook_is_in_the_index() -> None: + """A runbook is read under pressure, by somebody who did not write it. If it + is not in the index it will not be found in time to matter.""" + index = (DOCS / "README.md").read_text(encoding="utf-8") + runbooks = sorted(path.name for path in (DOCS / "runbooks").glob("*.md")) + + assert runbooks, "no runbooks found; the glob is looking in the wrong place" + missing = [name for name in runbooks if name not in index] + assert missing == [], f"not indexed in docs/README.md: {missing}" + + +# ─── The operator-facing promises ───────────────────────────────────────────── + + +def test_the_security_policy_points_at_the_supported_versions() -> None: + """ "Is my version still getting security fixes?" is the question a disclosure + policy has to answer, and the answer lives in the release policy.""" + policy = (ROOT / "SECURITY.md").read_text(encoding="utf-8") + + assert "releases.md" in policy + + +def test_the_recovery_runbook_is_the_one_ci_rehearses() -> None: + """The backup/restore runbook claims a rehearsal. `scripts/rehearse_release.sh` + is that rehearsal, so the two must name each other — a runbook that claims to + be exercised by a script nobody can find is a claim, not evidence.""" + runbook = (DOCS / "runbooks/backup-restore.md").read_text(encoding="utf-8") + script = (ROOT / "scripts/rehearse_release.sh").read_text(encoding="utf-8") + + assert "rehearse_release.sh" in runbook or "make rehearse" in runbook + assert "backup-restore.md" in script