From 50a6eede7eb3ce4232e818a9d2ffd063d086b667 Mon Sep 17 00:00:00 2001 From: Garrett Allen <98648590+Gerrrt@users.noreply.github.com> Date: Sat, 5 Sep 2026 03:57:21 +0000 Subject: [PATCH] fix(secrets): the lab's sops rule matched nothing, so it used the estate's key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0020 gave stacks/lab its own creation_rule above the catch-all because that catch-all matches ALL of secrets/: a recipient added there can decrypt the estate's SNMP communities and Grafana admin password, and a lab host holding those inverts the trust direction ADR-0007 exists to protect. The rule shipped as `secrets/lab\..*\.sops\.ya?ml$`, which cannot match `secrets/lab.sops.yaml`. After `secrets/lab\.` consumes the only dot before `sops`, `\.sops\.` has no second dot left; it would have matched `secrets/lab.something.sops.yaml` and nothing else. So the rule matched nothing, sops fell through to the catch-all, and `make secrets-init STACK=lab` on the guest encrypted the lab's secrets to the ESTATE's key — doing precisely what the rule was added to prevent, and reporting success while it did. Nothing caught it. sops does not warn about a creation_rule that matches no file; it silently uses the next one. The separation existed in the file, was reviewed, was merged, and was not real. It surfaced days later on `alexander` as sops' "no identity matched any of the recipients", which names the symptom and not the cause — the guest holds only its own key, and the file was not encrypted to it. No bad ciphertext reached the repository: secrets/lab.sops.yaml was never committed, so the damage is one untracked file on one host, holding the template's placeholder values. The runbook gains the recovery, which is to remove it and re-run secrets-init against the fixed rule. scripts/check_sops_rules.py is the guard, and its central assertion is the one that would have caught this: every creation_rule must match at least one file this repository actually encrypts. A rule matching nothing is a typo or dead, and both are indistinguishable from working until the day the fall-through matters. It also prints which rule each path resolves to, so the separation is visible in CI output rather than inferred from two regexes. The paths are derived — stacks from scripts/stacks.sh, the firewall path from the shape backup-firewall.sh writes — rather than a fourth hand-kept list. Verified by putting the broken pattern back: the check fails and names the dead rule. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 7 ++ .sops.yaml | 12 ++- docs/runbooks/build-the-lab-guest.md | 34 +++++++ scripts/check_sops_rules.py | 139 +++++++++++++++++++++++++++ scripts/validate.sh | 15 +++ 5 files changed, 206 insertions(+), 1 deletion(-) create mode 100755 scripts/check_sops_rules.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c35ad8e..45907c6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -283,6 +283,13 @@ jobs: # This ran in scripts/validate.sh and nowhere else — it was the only check # `make validate` had that CI did not, so it had never run on a pull # request. That asymmetry is #68. + # sops silently uses the next rule when one matches nothing, so a + # recipient separation can exist in the file, be reviewed, be merged, and + # not be real. That is exactly what happened to the stacks/lab rule + # ADR-0020 added. + - name: Verify every .sops.yaml rule matches the file it was written for + run: python3 scripts/check_sops_rules.py + - name: Verify the SNMP inventory agrees with itself run: ./scripts/snmp-targets.sh --check diff --git a/.sops.yaml b/.sops.yaml index 030b54a..7e6e52b 100644 --- a/.sops.yaml +++ b/.sops.yaml @@ -58,7 +58,17 @@ creation_rules: # ON that guest, fills it in — and refuses if the key it would write is # already a recipient below, because that would collapse the two rules back # into one. - - path_regex: secrets/lab\..*\.sops\.ya?ml$ + # + # The optional group in the pattern is load-bearing. This shipped as + # `secrets/lab\..*\.sops\.ya?ml$`, which cannot match `secrets/lab.sops.yaml` + # — after `secrets/lab\.` consumes the only dot before `sops`, `\.sops\.` + # has no second dot left to match. So the rule matched NOTHING, sops fell + # through to the catch-all below, and the lab's secrets were encrypted to the + # estate's key: silently, and as the precise inversion this rule exists to + # prevent. It surfaced only on the guest, as sops' "no identity matched any + # of the recipients". scripts/check_sops_rules.py now fails on any rule that + # matches no file, which is what makes a dead rule loud instead of harmless. + - path_regex: secrets/lab(\..+)?\.sops\.ya?ml$ age: >- REPLACE_WITH_LAB_AGE_PUBLIC_KEY diff --git a/docs/runbooks/build-the-lab-guest.md b/docs/runbooks/build-the-lab-guest.md index cba69de..eae9b76 100644 --- a/docs/runbooks/build-the-lab-guest.md +++ b/docs/runbooks/build-the-lab-guest.md @@ -170,6 +170,40 @@ assumed to be compromised. Commit and push `.sops.yaml` and `secrets/lab.sops.yaml` from here; the ciphertext belongs in the repository, the private key never does. +> [!WARNING] +> **If `secrets-edit` says `no identity matched any of the recipients`,** the +> file was encrypted to the estate's key rather than this host's. That was a +> real bug in the lab's `path_regex` — it matched nothing, so sops fell through +> to the catch-all — fixed on 2026-09-05, and +> `scripts/check_sops_rules.py` now fails CI on any rule that matches no file. +> +> Recover on this host. Nothing is lost: the file holds only the template's +> placeholder values, and it was never committed. +> +> ```bash +> cd ~/HomeLab +> rm secrets/lab.sops.yaml +> git checkout .sops.yaml +> git pull +> make secrets-init STACK=lab +> make secrets-edit STACK=lab +> ``` +> +> `git checkout .sops.yaml` discards the public key `secrets-init` wrote there +> so the pull applies cleanly; the re-run puts it back, into a rule that now +> matches. Your age private key in `~/.config/sops/age/keys.txt` is untouched +> throughout — `secrets-init` reuses an existing keypair rather than minting a +> second. +> +> Confirm the separation is real before moving on: +> +> ```bash +> python3 scripts/check_sops_rules.py +> ``` +> +> `secrets/lab.sops.yaml` must resolve to the `secrets/lab...` rule, not to the +> catch-all. + ## 5. The certificate **On the monitoring host**, where the CA lives: diff --git a/scripts/check_sops_rules.py b/scripts/check_sops_rules.py new file mode 100755 index 0000000..f01884b --- /dev/null +++ b/scripts/check_sops_rules.py @@ -0,0 +1,139 @@ +#!/usr/bin/env python3 +"""Assert every creation_rule in .sops.yaml matches the file it was written for. + +Why this exists +--------------- +ADR-0020 gave `stacks/lab` its own creation_rule and its own age recipient, +above the catch-all, because that catch-all matches ALL of secrets/ — so a +recipient added to it can decrypt the estate's SNMP communities and Grafana +admin password. A lab host holding the estate's credentials inverts the trust +direction ADR-0007 exists to protect. + +The rule shipped as: + + path_regex: secrets/lab\\..*\\.sops\\.ya?ml$ + +which cannot match `secrets/lab.sops.yaml`. After `secrets/lab\\.` consumes the +only dot before `sops`, `\\.sops\\.` has no second dot left to match; it would +have matched `secrets/lab.something.sops.yaml` and nothing else. So the rule +matched NOTHING, sops fell through to the catch-all, and `make secrets-init +STACK=lab` on the guest encrypted the lab's secrets to the ESTATE's key — +doing precisely what the rule was added to prevent, and reporting success. + +Nothing caught it. sops does not warn about a rule that matches no file: it +just uses the next one. The separation existed in the file, was reviewed, was +merged, and was not real. It surfaced days later on the lab guest as sops' +"no identity matched any of the recipients", which names the symptom and not +the cause. + +What this asserts +----------------- +1. Every stack's secrets file resolves to some rule — otherwise sops refuses to + encrypt it at all, which is loud but worth naming here rather than at deploy + time on a machine you had to walk to. + +2. **Every rule matches at least one real path.** This is the one that would + have caught the bug. A creation_rule matching nothing is either a typo or + dead, and both are indistinguishable from working until the day the + fall-through matters. + +3. Which rule each path resolves to is printed, so the separation ADR-0020 + decided is visible in CI output rather than inferred from two regexes. + +The paths are derived, not listed: the stacks come from scripts/stacks.sh, the +one definition of what a stack is, and the firewall backup path is the shape +scripts/backup-firewall.sh actually writes. A hand-kept list here would be the +fourth copy of something and would rot the same way. + +Usage: scripts/check_sops_rules.py +""" +from __future__ import annotations + +import pathlib +import re +import subprocess +import sys + +try: + import yaml +except ModuleNotFoundError: # pragma: no cover - CI installs it + print("PyYAML is required: python3 -m pip install pyyaml", file=sys.stderr) + raise SystemExit(1) + +REPO = pathlib.Path(__file__).resolve().parent.parent +POLICY = REPO / ".sops.yaml" + + +def stacks() -> list[str]: + listed = subprocess.run( + [str(REPO / "scripts/stacks.sh")], + capture_output=True, text=True, check=True, + ) + return listed.stdout.split() + + +def main() -> int: + if not POLICY.exists(): + print(f"no {POLICY.name}", file=sys.stderr) + return 1 + + policy = yaml.safe_load(POLICY.read_text(encoding="utf-8")) or {} + rules = policy.get("creation_rules") or [] + if not rules: + print(f"{POLICY.name} declares no creation_rules", file=sys.stderr) + return 1 + + patterns: list[tuple[str, re.Pattern[str]]] = [] + problems: list[str] = [] + for rule in rules: + raw = rule.get("path_regex") + if not raw: + problems.append("a creation_rule has no path_regex") + continue + try: + patterns.append((raw, re.compile(raw))) + except re.error as exc: + problems.append(f"path_regex {raw!r} does not compile — {exc}") + + # The paths sops is actually asked to encrypt. Stack secrets files, plus one + # firewall export: the stamp is arbitrary, so any well-formed name stands in + # for the whole class. + paths = [f"secrets/{stack}.sops.yaml" for stack in stacks()] + paths.append("backups/firewall/config-20260101T000000Z.sops.yaml") + + matched_by: dict[str, str] = {} + used: set[str] = set() + for path in paths: + hit = next((raw for raw, pat in patterns if pat.search(path)), None) + if hit is None: + problems.append( + f"{path} matches no creation_rule in {POLICY.name} — sops will " + f"refuse to encrypt it" + ) + continue + matched_by[path] = hit + used.add(hit) + + # The assertion that would have caught the lab rule. + for raw, _pat in patterns: + if raw not in used: + problems.append( + f"creation_rule {raw!r} matches none of the files this " + f"repository encrypts — sops does not warn about a dead rule, " + f"it silently uses the next one, so the recipient separation " + f"this rule was added for is not in effect (ADR-0020)" + ) + + if problems: + for problem in problems: + print(f" {problem}", file=sys.stderr) + return 1 + + print(f"{POLICY.name} OK — {len(patterns)} creation_rule(s), each matching:") + for path, raw in matched_by.items(): + print(f" {path} -> {raw}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/validate.sh b/scripts/validate.sh index 9ceeb44..86462d5 100755 --- a/scripts/validate.sh +++ b/scripts/validate.sh @@ -554,6 +554,21 @@ fi # --------------------------------------------------------------------------- head_ "Secrets" # --------------------------------------------------------------------------- +# Which age recipient each encrypted file actually resolves to. ADR-0020 gives +# stacks/lab its own rule above the catch-all so the lab guest's key cannot also +# decrypt the estate's — and that rule shipped with a path_regex matching +# nothing, so sops fell through and encrypted the lab's secrets to the estate's +# key while reporting success. sops does not warn about a dead rule. This does. +if have python3; then + if python3 scripts/check_sops_rules.py; then + pass "every .sops.yaml rule matches the file it was written for" + else + fail "a .sops.yaml rule matches nothing — the recipient separation is not in effect" + fi +else + skip "python3 not installed" +fi + # Both scans, matching CI exactly. Running only the working-tree scan locally # would let `make validate` pass while CI fails on history, or vice versa. # The docker fallback is the same shape promtool, amtool and alloy have above,