From 6202d92c27845540d961dca9ce0bd5c15cfd9fb3 Mon Sep 17 00:00:00 2001 From: Keith Jackson Date: Sat, 12 Sep 2026 21:15:25 -0500 Subject: [PATCH] test(conformance): gate the URL desired state, and prove the selftest can fail L0 (212 assertions, was 175): * runs ci/render-keycloak-client-urls.py --check, so client-urls.yaml must be exactly what the realm import renders. A hand edit that makes the two disagree is unmergeable -- this is what keeps the realm import the single source of truth for hostnames rather than just the place they were first typed. * asserts client-urls.yaml and the realm import are rendered by the SAME Flux Kustomization. postBuild.substituteFrom is per-Kustomization, so a split would let one ${BASE_DOMAIN} resolve to two different values and the realm would converge on a hostname the HTTPRoutes do not serve. * asserts the desired state uses no substitution token the realm definition lacks, names only clients the import declares, and declares no field outside the five-field write surface; that the CronJob mounts it readOnly at the path the script defaults to; and that the script, renderer and check agree on which fields are writable. * NEW mutations.py: injects 12 plausible regressions into the EXTRACTED script and requires --selftest to catch every one. A passing suite proves nothing if it cannot fail, and "reported success while the realm was broken" is precisely what this component exists to end -- so the same standard applies to its own test. ~20s. L2: the audit Job now mounts keycloak-client-urls and asserts KEYCLOAK_CLIENT_URLS_AUDIT_OK. Without that it would pass on a realm still holding pre-migration redirect URIs -- the exact state prod was measured in. Alerts: the existing Failing/Stale pair is EXTENDED to name redirect-URI drift, not duplicated. One CronJob gets one pair; a second pair on the same job_name regex would fire twice for one cause. No new critical, no new permanently-firing rule. Co-Authored-By: Claude Opus 5 --- .../check.sh | 225 +++++++++++++++++- .../mutations.py | 190 +++++++++++++++ .../chainsaw-test.yaml | 40 +++- platform/monitoring/alerts.yaml | 30 ++- 4 files changed, 465 insertions(+), 20 deletions(-) create mode 100755 conformance/tests/l0-static/keycloak-client-secret-reconciler/mutations.py diff --git a/conformance/tests/l0-static/keycloak-client-secret-reconciler/check.sh b/conformance/tests/l0-static/keycloak-client-secret-reconciler/check.sh index 185e9d5..6f672ee 100755 --- a/conformance/tests/l0-static/keycloak-client-secret-reconciler/check.sh +++ b/conformance/tests/l0-static/keycloak-client-secret-reconciler/check.sh @@ -56,6 +56,23 @@ # no-secret-logging, the endpoint allow-list (users/groups/roles are # unreachable) and fail-closed behaviour. What CI proves is byte-for-byte # what ships. +# (6b) AND THAT PROOF IS FALSIFIABLE: mutations.py injects plausible regressions +# into the extracted script one at a time and requires --selftest to CATCH +# every one. A passing suite proves nothing if it cannot fail, and "reported +# success while the realm was broken" is the exact failure this component +# exists to end — so the same standard is applied to its own test. +# (8) THE URL DESIRED STATE IS DERIVED, NOT DECLARED TWICE: the reconciler's +# second pass converges the realm's environment-derived client URLs +# (redirectUris, webOrigins, rootUrl, baseUrl, adminUrl) on +# platform/keycloak/client-urls.yaml. That file is GENERATED from the realm +# import by ci/render-keycloak-client-urls.py, and this check RUNS the +# renderer with --check, so a hand edit that makes the two disagree is +# unmergeable. It further asserts the two files are rendered by the SAME Flux +# Kustomization (postBuild.substituteFrom is per-Kustomization, so a split +# would let one ${BASE_DOMAIN} resolve to two different values), that the +# desired state uses no substitution token the realm definition does not, +# that every client it names exists in the import, and that it declares no +# field outside the reconciler's five-field write surface. # AND (7) THIS CHECK IS LOAD-BEARING: ci/l0.sh runs it unconditionally, ci/gate.sh # blocks the merge when the l0 stage fails, ci/touched-requirements.sh maps # platform/keycloak/ to these requirement IDs, and an L2 test exists that @@ -63,8 +80,13 @@ # and isn't" failure this repo keeps hitting. # # NOT proven here (named honestly, in the style of the sibling L0 checks): -# - that the LIVE realm's stored secrets match git. That is cluster state; it is -# conformance/tests/l2-behavioral/keycloak-client-secrets-match-git. +# - that the LIVE realm's stored secrets OR redirect URIs match git. That is cluster +# state; it is conformance/tests/l2-behavioral/keycloak-client-secrets-match-git, +# which runs the reconciler's own --audit against the live realm. +# - that the hostnames the realm import declares are the ones the Gateway/HTTPRoutes +# actually serve. This check proves the realm converges on what GIT says; it cannot +# prove git is right about the domain. A BASE_DOMAIN flip that misses an HTTPRoute is +# a different invariant. # - that the SOPS ciphertext decrypts to a usable secret. Key PRESENCE is checkable # offline; the plaintext is not, and must not be. # - that the CronJob's image can be pulled or its schedule fires. @@ -77,9 +99,10 @@ PYTHON="${PYTHON:-python3}" IMPORT_FILE="platform/keycloak/keycloak-realm-import.yaml" SOPS_FILE="platform/keycloak/keycloak-realm-clients.sops.yaml" RECON_FILE="platform/keycloak/client-secret-reconciler.yaml" +URLS_FILE="platform/keycloak/client-urls.yaml" # Honest SKIP (never green) if the thing under test is absent entirely. -for f in "$IMPORT_FILE" "$SOPS_FILE" "$RECON_FILE"; do +for f in "$IMPORT_FILE" "$SOPS_FILE" "$RECON_FILE" "$URLS_FILE"; do if [ ! -f "${ROOT}/${f}" ]; then echo "check SKIPPED: ${f} absent — nothing to audit" >&2 exit 3 @@ -95,6 +118,7 @@ the Flux graph walk rather than forked, so there is one definition of "what a Fl renders" in the harness.""" from __future__ import annotations +import json import re import subprocess import sys @@ -116,10 +140,21 @@ import status as st # noqa: E402 — reuse the walk, don't f IMPORT_FILE = "platform/keycloak/keycloak-realm-import.yaml" SOPS_FILE = "platform/keycloak/keycloak-realm-clients.sops.yaml" RECON_FILE = "platform/keycloak/client-secret-reconciler.yaml" +URLS_FILE = "platform/keycloak/client-urls.yaml" SECRET_NAME = "keycloak-realm-clients" ADMIN_SECRET = "keycloak-admin-credentials" +URLS_CM_NAME = "keycloak-client-urls" KEY_SUFFIX = "-client-secret" SECRETS_MOUNT = "/etc/keycloak-realm-clients" +URLS_MOUNT = "/etc/keycloak-client-urls" +URLS_DATA_KEY = "client-urls.json" +RENDERER = "ci/render-keycloak-client-urls.py" +DIR_PATH = ROOT / "conformance" / "tests" / "l0-static" / "keycloak-client-secret-reconciler" +# The ONLY client fields the reconciler may write besides the secret. Kept in lockstep +# with the renderer and the script; asserted below to be exactly what they agree on. +URL_LIST_FIELDS = ("redirectUris", "webOrigins") +URL_STR_FIELDS = ("rootUrl", "baseUrl", "adminUrl") +URL_FIELDS = URL_LIST_FIELDS + URL_STR_FIELDS DOLLAR = chr(36) FAILURES: list[str] = [] @@ -446,6 +481,24 @@ if cj is not None: check(len(cm_vols) == 1, f"{RECON_FILE}: the CronJob does not mount the reconciler ConfigMap, so it " f"would run nothing") + # The URL desired state must be MOUNTED at the path the script defaults to, for the + # same reason as the Secret: the pod holds no ServiceAccount token and cannot read + # the KeycloakRealmImport CR, so an unmounted ConfigMap means pass 2 fails closed on + # every run forever -- correct behaviour, but a permanently red reconciler. + url_vols = [v for v in vols.values() + if (v.get("configMap") or {}).get("name") == URLS_CM_NAME] + check(len(url_vols) == 1, + f"{RECON_FILE}: the CronJob must mount the {URLS_CM_NAME} ConfigMap — that " + f"mount IS the desired state the URL pass reconciles towards, and the pod has " + f"no Kubernetes API access to read the realm import instead") + if url_vols: + mp = mounts.get(url_vols[0]["name"], {}).get("mountPath") + check(mp == URLS_MOUNT, + f"{RECON_FILE}: {URLS_CM_NAME} is mounted at {mp}, but the script defaults " + f"to {URLS_MOUNT}/{URLS_DATA_KEY} and no env var overrides it") + check(mounts.get(url_vols[0]["name"], {}).get("readOnly") is True, + f"{RECON_FILE}: the {URLS_CM_NAME} mount must be readOnly — the reconciler " + f"reads its desired state, it never writes it") env_srcs = {e["name"]: e for e in ctr.get("env", [])} for var in ("KEYCLOAK_ADMIN_USER", "KEYCLOAK_ADMIN_PASSWORD"): ref = (env_srcs.get(var, {}).get("valueFrom") or {}).get("secretKeyRef") or {} @@ -467,6 +520,110 @@ if cj is not None: check(re.search(r":[0-9]", img), f"{RECON_FILE}: container image {img!r} is not pinned to an explicit version") +# ================================================ (8) the URL desired state is DERIVED +# The reconciler's second pass makes the realm's environment-derived client URLs match +# platform/keycloak/client-urls.yaml. That file must not be an independent declaration of +# every public hostname: two declarations that can disagree is the same shape as the bug +# this whole change closes. It is GENERATED from the realm import, and the generation is +# gated HERE -- a hand edit to either file that makes them disagree is unmergeable. +urls_docs = [d for d in load(URLS_FILE) if d] +urls_cm = next((d for d in urls_docs if d.get("kind") == "ConfigMap"), None) +check(urls_cm is not None, f"{URLS_FILE}: no ConfigMap document found") +check((urls_cm or {}).get("metadata", {}).get("name") == URLS_CM_NAME, + f"{URLS_FILE}: the ConfigMap must be named {URLS_CM_NAME} (the CronJob mounts it " + f"by that name)") + +urls_want: dict[str, dict] = {} +if urls_cm is not None: + data = urls_cm.get("data") or {} + check(set(data) == {URLS_DATA_KEY}, + f"{URLS_FILE}: the ConfigMap must carry exactly the key {URLS_DATA_KEY} " + f"(found {sorted(data)}) — the script reads that filename from its mount") + raw = data.get(URLS_DATA_KEY, "") + try: + urls_want = json.loads(raw) + except ValueError as e: + check(False, f"{URLS_FILE}: {URLS_DATA_KEY} is not valid JSON ({e}) — the " + f"reconciler would fail closed on every run") + check(isinstance(urls_want, dict) and bool(urls_want), + f"{URLS_FILE}: {URLS_DATA_KEY} must be a non-empty JSON object of clients") + +# THE GATE: the committed file must be exactly what the renderer produces from the realm +# import. This is what makes the realm import the single source of truth for hostnames +# rather than merely the place they were first typed. +if (ROOT / RENDERER).is_file(): + proc = subprocess.run([sys.executable, str(ROOT / RENDERER), "--check"], + capture_output=True, text=True, timeout=120, cwd=str(ROOT)) + check(proc.returncode == 0, + f"{URLS_FILE} is NOT what {RENDERER} renders from {IMPORT_FILE}. The URL " + f"reconciler would converge the live realm on a desired state the realm import " + f"does not declare — two sources of truth for every public hostname, which is " + f"the failure class this change exists to close. Regenerate with: " + f"python3 {RENDERER} --write\n " + + "\n ".join((proc.stdout + proc.stderr).strip().splitlines()[:12])) +else: + check(False, f"{RENDERER} is missing — {URLS_FILE} would become a hand-maintained " + f"second declaration of every hostname with nothing keeping it honest") + +# Every client named in the desired state must exist in the realm import (otherwise every +# reconciler run reports NOT FOUND forever), and every field must be one the reconciler is +# actually allowed to write. +import_client_ids = {c.get("clientId") for c in clients} +for cid in sorted(urls_want): + check(cid in import_client_ids, + f"{URLS_FILE}: declares URLs for client '{cid}', which {IMPORT_FILE} does not " + f"declare. The reconciler would look it up, not find it, and fail every run") + entry = urls_want[cid] if isinstance(urls_want.get(cid), dict) else {} + check(bool(entry), f"{URLS_FILE}: client '{cid}' declares no URL fields") + for field in sorted(entry): + check(field in URL_FIELDS, + f"{URLS_FILE}: client '{cid}' declares field '{field}', which is not one of " + f"the writable URL fields ({', '.join(URL_FIELDS)}). Anything else would be " + f"a client-representation field written into the realm by a file edit") + +# The three places that name the writable field set must agree, or the gate is checking a +# different contract than the one that ships. +for field in URL_FIELDS: + check(field in script, + f"{RECON_FILE}: the shipped script never mentions the writable URL field " + f"'{field}', but this check and {RENDERER} both treat it as writable — the " + f"three definitions of the write surface have diverged") + +# A redirect URI is only as good as the hostname it points at. If the realm import uses a +# Flux substitution token, the desired-state ConfigMap must carry the SAME token (not a +# resolved value) and must be rendered by the SAME Kustomization, or the two are +# substituted differently and disagree about the domain at reconcile time. +# Scan the DATA PAYLOAD, not the whole file: both files carry header prose that discusses +# substitution tokens by name, and matching those would be a false positive. +TOKEN_RE = re.compile(re.escape(DOLLAR) + r"\{([A-Z_][A-Z0-9_]*)\}") +import_tokens = set(TOKEN_RE.findall( + yaml.safe_dump((realm_import or {}).get("spec", {}).get("realm", {})))) +urls_tokens = set(TOKEN_RE.findall( + ((urls_cm or {}).get("data") or {}).get(URLS_DATA_KEY, ""))) +check(urls_tokens <= import_tokens, + f"{URLS_FILE} uses substitution token(s) {sorted(urls_tokens - import_tokens)} that " + f"the realm definition in {IMPORT_FILE} does not — the desired state would resolve " + f"to a hostname the realm import never declared") + +urls_owners = owners(URLS_FILE) +check(bool(urls_owners), + f"{URLS_FILE} is rendered by NO Flux Kustomization — the reconciler would mount a " + f"ConfigMap that does not exist and fail closed on every run") +check(set(urls_owners) == set(import_owners), + f"{URLS_FILE} is rendered by {urls_owners} but {IMPORT_FILE} by {import_owners}. " + f"They MUST be rendered by the same Flux Kustomization: postBuild.substituteFrom is " + f"per-Kustomization, so a split would let the same " + f"{DOLLAR}{{BASE_DOMAIN}} resolve to two different values — the realm converging on " + f"a hostname the HTTPRoutes do not serve") +for env in ("dev", "prod"): + entry = f"clusters/{env}" + if not (ROOT / entry).is_dir(): + continue + applied, _ = st.reconciled_files(ROOT, entry) + check(URLS_FILE in applied, + f"{URLS_FILE} is not reachable from the {entry} entrypoint — the URL pass would " + f"fail closed on that cluster forever") + # ============================== (6) the reconciler's BEHAVIOUR, proven by its own selftest if script: with tempfile.TemporaryDirectory() as td: @@ -482,6 +639,35 @@ if script: if proc.returncode == 0: print(" reconciler --selftest: " + (tail[-1] if tail else "(no output)")) + # ---- (6b) THE SELFTEST IS FALSIFIABLE, not decorative ----------------------- + # A passing suite proves nothing unless it can be shown to FAIL. mutations.py + # injects plausible regressions one at a time -- order-sensitive comparison, a + # fat round-tripped PUT, additive-only URI sets, a widened allow-list, coupled + # passes, fail-open on an unreadable input -- and requires --selftest to catch + # every one. A mutation that SURVIVES means the suite does not check that + # property, and this check fails naming it. + # + # This matters more than usual here: the whole incident was a component that + # REPORTED SUCCESS while the realm was broken. A green test that cannot fail is + # the same lie one level up. ~20s, no network, no cluster. + mut = DIR_PATH / "mutations.py" + if mut.is_file(): + mproc = subprocess.run([sys.executable, str(mut), str(p)], + capture_output=True, text=True, timeout=600) + mtail = (mproc.stdout + mproc.stderr).strip().splitlines() + check(mproc.returncode == 0, + "the reconciler's --selftest is NOT FALSIFIABLE — at least one injected " + "regression survived it, so the suite does not prove what it claims:\n" + " " + "\n ".join(mtail[-14:])) + if mproc.returncode == 0: + print(" reconciler mutations: " + + (mtail[-1] if mtail else "(no output)")) + else: + check(False, + "conformance/tests/l0-static/keycloak-client-secret-reconciler/" + "mutations.py is missing — nothing proves the --selftest can fail, and " + "an unfalsifiable proof is the failure mode this component exists to end") + # ======================================================= (7) this check is load-bearing L0 = (ROOT / "ci" / "l0.sh").read_text() if (ROOT / "ci" / "l0.sh").is_file() else "" GATE = (ROOT / "ci" / "gate.sh").read_text() if (ROOT / "ci" / "gate.sh").is_file() else "" @@ -504,9 +690,36 @@ check(l2.is_file(), "keycloak-client-secrets-match-git) is missing — this static check cannot see a " "realm that drifted after apply, which is the whole failure mode") if l2.is_file(): - check("KEYCLOAK_CLIENT_SECRETS_AUDIT_OK" in l2.read_text(), - "the L2 test does not assert the reconciler's audit sentinel, so it could pass " - "on a Job that never ran the audit") + l2_text = l2.read_text() + check("KEYCLOAK_CLIENT_SECRETS_AUDIT_OK" in l2_text, + "the L2 test does not assert the reconciler's secret audit sentinel, so it could " + "pass on a Job that never ran the audit") + check("KEYCLOAK_CLIENT_URLS_AUDIT_OK" in l2_text, + "the L2 test does not assert the reconciler's URL audit sentinel. The live realm " + "could hold pre-migration redirect URIs and the test would still pass — which is " + "exactly the state prod was measured in") + check(URLS_CM_NAME in l2_text, + f"the L2 audit Job does not mount the {URLS_CM_NAME} ConfigMap, so its URL pass " + f"would fail closed and the test would report a false failure") + +# The alert pair must cover the reconciler's silence. Extended, not duplicated: one CronJob +# gets one Failing/Stale pair, because two pairs keyed to the same job_name regex would +# both fire for a single cause. +ALERTS = "platform/monitoring/alerts.yaml" +alerts_text = (ROOT / ALERTS).read_text() if (ROOT / ALERTS).is_file() else "" +for rule in ("KeycloakClientSecretReconcilerFailing", "KeycloakClientSecretReconcilerStale"): + check(rule in alerts_text, + f"{ALERTS} has no {rule} rule — the reconciler's silence would be unobservable, " + f"and a healer nobody notices is absent is the 'looks applied and isn't' failure") +check("redirect" in alerts_text.lower(), + f"{ALERTS}: the reconciler alert descriptions never mention redirect URIs, so an " + f"operator paged at 3am would look only for a client-secret problem while the actual " + f"drift was a hostname") + +# The renderer must be a real, runnable, tracked file — not a documented intention. +check((ROOT / RENDERER).is_file(), + f"{RENDERER} is missing; without it {URLS_FILE} cannot be regenerated and the " + f"single-source-of-truth gate above has nothing to compare against") # ------------------------------------------------------------------------------ report print(f"realm import: {len(clients)} client(s), {len(declared_pairs)} confidential with a " diff --git a/conformance/tests/l0-static/keycloak-client-secret-reconciler/mutations.py b/conformance/tests/l0-static/keycloak-client-secret-reconciler/mutations.py new file mode 100755 index 0000000..cbc99f3 --- /dev/null +++ b/conformance/tests/l0-static/keycloak-client-secret-reconciler/mutations.py @@ -0,0 +1,190 @@ +#!/usr/bin/env python3 +# ============================================================================= +# mutations.py -- prove the reconciler's --selftest IS FALSIFIABLE. +# +# WHY THIS EXISTS +# --------------- +# A selftest that passes proves nothing on its own: a selftest asserting `True == +# True` also passes. The reconciler exists because a one-shot import REPORTED +# SUCCESS while the realm was broken, and the same trap applies one level up -- a +# green test suite that cannot fail is the same lie in a different place. +# +# So this injects a series of PLAUSIBLE regressions into the shipped script, one at +# a time, and requires that --selftest FAILS for every one. A mutation that +# SURVIVES means the suite does not actually check that property, and the check +# fails, naming it. +# +# Each mutation is a mistake a reasonable person could make while editing this +# code -- not a nonsense edit. Several were live design questions while it was +# written (order-sensitivity, additive-vs-exact URI sets, whether the two passes +# should share a failure path); the mutation is the record that the answer is +# enforced and not merely commented. +# +# Invoked by check.sh with the script EXTRACTED from the shipped ConfigMap, so what +# is proven falsifiable is byte-for-byte what runs on the cluster. +# +# python3 mutations.py +# +# Runtime is ~20s (one subprocess per mutation, each running the full selftest +# against an in-process stub Keycloak; no network, no cluster). +# ============================================================================= +from __future__ import annotations + +import re +import subprocess +import sys +import tempfile +from pathlib import Path + + +def dedent4(text: str) -> str: + """Anchors are written with the YAML file's indentation. The script extracted from + the ConfigMap block scalar has 4 fewer leading spaces on every line, so anchors are + authored to match the YAML (easy to diff against the manifest) and dedented here.""" + return "\n".join(ln[4:] if ln.startswith(" ") else ln for ln in text.split("\n")) + + +# (name, the mistake being modelled, anchor, replacement) +MUTATIONS: list[tuple[str, str, str, str]] = [ + ("order-sensitive URL comparison", + "compare redirectUris in the order Keycloak returns them -- Keycloak does not " + "preserve the authored order, so a converged realm is rewritten on every run " + "forever while every run still reports success", + "if sorted(stored) != sorted(declared_value):", + "if stored != declared_value:"), + + ("fat round-tripped URL PUT", + "PUT the whole client representation back instead of only the drifted fields -- " + "the live server rejects it with 400 'Cannot parse the JSON', and even if it did " + "not, the blast radius would cover every field of the client", + " rep = {\"clientId\": client.get(\"clientId\")}\n" + " for field in sorted(drift):\n" + " rep[field] = want[cid][field]", + " rep = dict(client)\n" + " for field in sorted(drift):\n" + " rep[field] = want[cid][field]"), + + ("write unconditionally (compare-then-write removed)", + "drop the converged short-circuit -- every client is rewritten on every run, so " + "the CronJob becomes a permanent write load against the IdP database", + " drift = url_drift(client, want[cid])\n if not drift:", + " drift = url_drift(client, want[cid]) or dict(\n" + " (f, (\"\", want[cid][f])) for f in want[cid])\n if False:"), + + ("additive-only URI reconciliation", + "union the declared URIs with the stored ones -- a redirect URI REMOVED from git " + "is never removed from the realm, so a stale wildcard keeps working and quietly " + "widens what the realm will redirect to", + " rep[field] = want[cid][field]", + " rep[field] = (sorted(set(want[cid][field]) | set(client.get(field) or []))\n" + " if field in URL_LIST_FIELDS else want[cid][field])"), + + ("URL pass fails OPEN on an unreadable input", + "treat a missing ConfigMap as 'nothing declared' -- the run reports success while " + "reconciling nothing, which is precisely the fail-open of the realm import", + " except OSError as e:\n" + " url_input_fail = (\"cannot read %s (%s) -- the keycloak-client-urls ConfigMap \"\n" + " \"is not mounted\" % (URLS_FILE, e.__class__.__name__))", + " except OSError:\n want_urls = {}"), + + ("URL pass skips public clients", + "copy the secret pass's publicClient skip into the URL pass -- the `kubernetes` " + "client is PKCE-public, so kubectl's redirect URIs would never converge", + " drift = url_drift(client, want[cid])", + " if client.get(\"publicClient\"):\n continue\n" + " drift = url_drift(client, want[cid])"), + + ("the two passes are coupled", + "abort the whole run when either input is bad -- a typo in a hostname ConfigMap " + "would stop client-SECRET repair, making credential healing depend on a file that " + "has nothing to do with credentials", + " if want_secrets is None and want_urls is None:", + " if want_secrets is None or want_urls is None:"), + + ("unmanaged fields forced to empty", + "iterate every writable field instead of only the declared ones -- a rootUrl or " + "adminUrl the realm holds and git does not declare would be WIPED, turning " + "'not declared' into 'must be empty'", + " for field, declared_value in sorted(wanted.items()):", + " for field in URL_FIELDS:\n" + " declared_value = wanted.get(\n" + " field, [] if field in URL_LIST_FIELDS else \"\")"), + + ("no readback verification", + "trust the 204 -- a server that accepts the PUT and does not apply it is reported " + "as REPAIRED, which is 'reports success while the realm never converged' again", + " still = url_drift(after[0], want[cid])", + " still = {}"), + + ("allow-list widened to the whole realm", + "add a realm-level endpoint 'just in case' -- /users, /groups, /roles and " + "/authentication stop being structurally unreachable", + " r\"/admin/realms/[^/]+/clients\",", + " r\"/admin/realms/[^/]+\",\n r\"/admin/realms/[^/]+/clients\","), + + ("desired-state field validation removed", + "accept any field name from the ConfigMap -- an arbitrary client field becomes " + "writable through a file edit, bypassing the five-field write surface entirely", + " if field not in URL_FIELDS:", + " if False:"), + + ("URL drift detected but never repaired", + "leave the audit-mode early-continue in place on the write path -- drift is " + "reported in the log and silently not fixed", + " if audit:\n continue\n # MINIMAL representation", + " if True:\n continue\n # MINIMAL representation"), +] + + +def main() -> int: + if len(sys.argv) != 2: + print("usage: mutations.py ", file=sys.stderr) + return 2 + src = Path(sys.argv[1]).read_text() + + problems: list[str] = [] + killed = 0 + for name, why, old, new in MUTATIONS: + old, new = dedent4(old), dedent4(new) + # An anchor that no longer matches is a FAILURE, not a skip: it means the code + # moved and this mutation silently stopped proving anything. + n = src.count(old) + if n != 1: + problems.append("%s: anchor matches %d times (expected exactly 1) -- the " + "code moved and this mutation no longer proves anything. " + "Re-anchor it against the current source." % (name, n)) + continue + with tempfile.NamedTemporaryFile("w", suffix=".py", delete=False) as fh: + fh.write(src.replace(old, new, 1)) + path = fh.name + try: + proc = subprocess.run([sys.executable, path, "--selftest"], + capture_output=True, text=True, timeout=240) + finally: + Path(path).unlink(missing_ok=True) + if proc.returncode == 0: + problems.append("%s SURVIVED -- --selftest still passed with this regression " + "applied, so nothing in the suite actually checks it. The " + "mistake it models: %s" % (name, why)) + continue + killed += 1 + out = proc.stdout + proc.stderr + m = re.search(r"SELFTEST FAIL: (\d+) of \d+", out) + how = ("%s assertion(s) caught it" % m.group(1) if m + else "the selftest aborted (caught, less cleanly)") + print(" killed %-42s %s" % (name, how)) + + print() + if problems: + print("MUTATION CHECK FAIL: %d of %d mutation(s) not proven" + % (len(problems), len(MUTATIONS)), file=sys.stderr) + for p in problems: + print(" FAIL %s" % p, file=sys.stderr) + return 1 + print("MUTATION CHECK OK: all %d injected regressions were caught by --selftest " + "(the suite is falsifiable, not decorative)" % killed) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/conformance/tests/l2-behavioral/keycloak-client-secrets-match-git/chainsaw-test.yaml b/conformance/tests/l2-behavioral/keycloak-client-secrets-match-git/chainsaw-test.yaml index c384ca2..81656ca 100644 --- a/conformance/tests/l2-behavioral/keycloak-client-secrets-match-git/chainsaw-test.yaml +++ b/conformance/tests/l2-behavioral/keycloak-client-secrets-match-git/chainsaw-test.yaml @@ -1,6 +1,6 @@ # ============================================================================= -# L2 BEHAVIORAL — every OIDC client's STORED secret equals the one git declares -# (R-IDP-02, R-IDP-04) [RUNNABLE] +# L2 BEHAVIORAL — every OIDC client's STORED secret AND redirect URIs equal what git +# declares (R-IDP-02, R-IDP-04) [RUNNABLE] # Spec: docs/15 identity-and-access; ADR-0014. # Impl: platform/keycloak/client-secret-reconciler.yaml (the CronJob + its script). # @@ -27,14 +27,26 @@ # its endpoint allow-list makes users/groups/roles unreachable), # -> THEN every declared client's stored secret matches git, and none holds an # unresolved import placeholder. Emits KEYCLOAK_CLIENT_SECRETS_AUDIT_OK. +# -> AND every declared client URL — redirectUris, webOrigins, rootUrl, baseUrl, +# adminUrl — matches git. Emits KEYCLOAK_CLIENT_URLS_AUDIT_OK. +# +# THE URL HALF EXISTS FOR THE SAME REASON, ONE FIELD OVER. The realm import is templated +# per environment, so moving prod's BASE_DOMAIN re-renders every declared redirect URI in +# git — and reaches the realm never, because a completed import does not re-run. Measured +# on prod: stored redirect URIs still on the PRE-migration hostname while git declared the +# new one. Every OIDC login would fail with a redirect_uri mismatch, and once again every +# signal would be green: the CR reads Done=True, the pods are healthy, and the +# secret-only version of this very test would have PASSED. Asserting only the secret +# sentinel is what would have let that ship. # # WHY IT RUNS THE SHIPPED SCRIPT rather than re-implementing the comparison: a detector # that duplicates the healer's logic drifts from it. This audits with the same code, in # the same mode the L0 gate selftests, so a passing audit is evidence about the healer # too. It runs as a Job IN the keycloak namespace (not in the ephemeral chainsaw -# namespace) because the desired state is the mounted keycloak-realm-clients Secret and -# the admin credential is keycloak-admin-credentials — both namespace-local, which is -# also why neither this Job nor the CronJob needs any Kubernetes API access. +# namespace) because the desired state is the mounted keycloak-realm-clients Secret plus +# the keycloak-client-urls ConfigMap, and the admin credential is +# keycloak-admin-credentials — all namespace-local, which is also why neither this Job nor +# the CronJob needs any Kubernetes API access. # # chainsaw v0.2.12: Job + status assert; the leftover-Job delete keeps a re-run from # colliding with an immutable Job left by a previous failure. @@ -149,6 +161,12 @@ spec: - name: client-secrets mountPath: /etc/keycloak-realm-clients readOnly: true + # The URL pass's desired state. Without this mount the audit + # fails closed on pass 2 and this test would report a false + # failure rather than auditing the realm's redirect URIs. + - name: client-urls + mountPath: /etc/keycloak-client-urls + readOnly: true securityContext: allowPrivilegeEscalation: false readOnlyRootFilesystem: true @@ -170,6 +188,10 @@ spec: secret: secretName: keycloak-realm-clients defaultMode: 0400 + - name: client-urls + configMap: + name: keycloak-client-urls + defaultMode: 0444 - assert: resource: apiVersion: batch/v1 @@ -192,6 +214,14 @@ spec: || { echo "FAIL: the audit did not report every client secret matching git"; exit 1; } echo "$out" | grep -q 'UNRESOLVED IMPORT PLACEHOLDER' \ && { echo "FAIL: a client still holds the literal import placeholder"; exit 1; } + # The URL half. A realm still holding PRE-migration redirect URIs is the + # state prod was measured in: every OIDC login fails with a redirect_uri + # mismatch while the KeycloakRealmImport CR still reads Done=True. Asserting + # only the secret sentinel would pass on exactly that realm. + echo "$out" | grep -q 'KEYCLOAK_CLIENT_URLS_AUDIT_OK' \ + || { echo "FAIL: the audit did not report every client URL matching git"; exit 1; } + echo "$out" | grep -q 'DRIFT -- realm has' \ + && { echo "FAIL: a client URL still differs from the declared one"; exit 1; } echo KEYCLOAK_CLIENT_SECRET_DRIFT_NONE_OK check: (contains($stdout, 'KEYCLOAK_CLIENT_SECRET_DRIFT_NONE_OK')): true diff --git a/platform/monitoring/alerts.yaml b/platform/monitoring/alerts.yaml index 5260b8e..daa1eb5 100644 --- a/platform/monitoring/alerts.yaml +++ b/platform/monitoring/alerts.yaml @@ -431,7 +431,7 @@ spec: summary: "Keycloak database unavailable" description: "Keycloak's PostgreSQL is not ready for 5m — the IdP will fail (docs/15)." runbook_url: "https://${BASE_DOMAIN}/runbooks/keycloak-db" - # --- The client-secret reconciler (platform/keycloak/client-secret-reconciler.yaml) --- + # --- The client reconciler (platform/keycloak/client-secret-reconciler.yaml) --- # WHY THESE EXIST: on 2026-09-06 every OIDC login on prod failed because the realm import # had stored the LITERAL `(env:...)` placeholder as all 12 client secrets — and every # signal the cluster offered was GREEN (the import Job succeeded, the CR read Done=True, @@ -440,6 +440,14 @@ spec: # the healer; these two rules make its SILENCE observable, so the class cannot sit # undetected again. They are the runtime counterpart of the L2 audit test. # + # THE CronJob NOW RECONCILES TWO THINGS — client secrets, and the environment-derived + # client URLs (redirectUris, webOrigins, rootUrl, baseUrl, adminUrl) that a BASE_DOMAIN + # migration re-renders in git and the one-shot import never carries to the realm. The + # pair below is EXTENDED to say so rather than DUPLICATED: one CronJob gets one + # Failing/Stale pair, because a second pair keyed to the same job_name regex would fire + # twice for a single cause and train the operator to ignore both. The Job log names which + # pass failed, which client, and for a URL the exact before/after. + # # UNVERIFIED-ON-CLUSTER at authoring time: no CronJob existed on either cluster yet, so # these series had no samples to check against. `kube_job_status_failed` / # `kube_job_status_succeeded` and the `job_name` label WERE confirmed present (the realm @@ -457,12 +465,15 @@ spec: labels: severity: warning annotations: - summary: "Keycloak client-secret reconciler failing" + summary: "Keycloak client reconciler failing" description: >- - A keycloak-client-secret-reconciler Job has been failing for 15m — an OIDC client - secret may differ from git (unresolved import placeholder, or an unrepairable - drift) and SSO logins for that client will fail. Read the Job log: it names the - client and never prints a secret. + A keycloak-client-secret-reconciler Job has been failing for 15m — an OIDC client's + stored SECRET or its REDIRECT URIs may differ from git (an unresolved import + placeholder, a pre-migration hostname the one-shot realm import never updated, or + an unrepairable drift), and SSO logins for that client will fail — a bad + redirectUri fails with a redirect_uri mismatch at the IdP. Read the Job log: it + names the pass, the client and the exact URL before/after, and never prints a + secret. runbook_url: "https://${BASE_DOMAIN}/runbooks/keycloak-client-secrets" - alert: KeycloakClientSecretReconcilerStale # No SUCCESSFUL run in scope. Catches the failure modes the rule above cannot see: the @@ -475,11 +486,12 @@ spec: labels: severity: warning annotations: - summary: "Keycloak client-secret reconciler has not succeeded" + summary: "Keycloak client reconciler has not succeeded" description: >- No successful keycloak-client-secret-reconciler run in 2h (schedule is every 15m). - The CronJob may be suspended, unapplied, or unable to schedule — client-secret - drift would go uncorrected and unnoticed (docs/15). + The CronJob may be suspended, unapplied, or unable to schedule — client-secret AND + redirect-URI drift would go uncorrected and unnoticed (docs/15). This is the rule + that would catch the healer being quietly absent during a domain migration. runbook_url: "https://${BASE_DOMAIN}/runbooks/keycloak-client-secrets" # =====================================================================================