From 11331d1762f3083179f9c43e3893f554aaad1a54 Mon Sep 17 00:00:00 2001 From: jonahgabriel Date: Mon, 31 Aug 2026 08:49:11 -0400 Subject: [PATCH] feat(OMN-17320): gate denylisted customer identifiers out of this public repo OMN-17288 scrubbed a live tenant slug out of five files in this repo (#3062, 3f10ee5e) and established a synthetic-identifier convention in its place. Three hours later an unrelated lane reintroduced the same slug in omnimarket (#2239), and the rebase carried it onto omnimarket#2241 -- the PR whose own acceptance criterion was "zero grep hits" -- with every enforced gate green. Nothing in either repo was looking. The convention was documentation, and documentation lost a race in three hours. Operating Rule #5: detection that is not a gate gets ignored. Why digests and not a plaintext pattern list: this repo is PUBLIC. Writing a forbidden customer identifier into a pattern file here would create exactly the fresh, greppable, current-tree occurrence the class exists to prevent, and would force that file to be exempt from its own rule -- a special file holding the forbidden value, that nobody scans and that people copy from. That is the shape of the incident, not a fix for it. Entries are salted SHA-256 plus a class label and owning ticket; the loader refuses to load an entry carrying a value/literal/ plaintext field. The salt is committed, so this is obfuscation and not secrecy, and that is stated in the file rather than implied: the OMN-17288 values are already public in git history and the operator ruled document-and-accept on that history. What the format buys is FORWARD safety -- the next entry may be a live identifier that has NOT leaked, where a plaintext denylist would be an active disclosure. Matching windows inside each identifier token, so a literal is caught bare, embedded (tenant__v2), and inside a path or URL segment. Findings print path:line:col plus match length and entry id, never the value. Encoded forms are deliberately NOT decoded -- OMN-17180 owns that class, and claiming coverage here would be a false claim. One escape hatch, per line, ticket + reason required: # onex-allow-exposed-identifier OMN-XXXXX reason="" A bare annotation is rejected, matching every other onex-allow class. There is deliberately NO file-level waiver and NO self-exempt file: a whole-file waiver is how a forbidden value survives in a corner nobody reads. The gate is subject to its own rule. Wired in this same PR on both surfaces (Rule #5): - pre-commit hook `exposed-identifier-gate` - CI job `Exposed Identifier Gate (OMN-17320)` in ci.yml, registered in scripts/ci/ci_summary_gate.py::STRICT_GATE_JOBS. That registration is half the mechanism: dev requires exactly one context (CI Summary), and while the default-deny sweep already fails on a FAILING job, an unregistered job that is skipped or deleted yields SUCCESS -- so without it, removing this job would silently restore the unenforced state that produced the recurrence. Evidence: - Incident replay (OMN-15547 convention) over the real pre-scrub artifact, captured from git object 6527db3f (= 3f10ee5e^). ONE same-length redaction of the slug, recorded in registry.yaml with the pre-redaction sha256 so the git object can be re-fetched and diffed; every other byte of the 8455 is verbatim and offsets are preserved, so the finding is asserted at the slug's real position (line 7, col 379). An accept-control in the same module requires the SHIPPED denylist to PASS those same bytes, so a reject-everything guard cannot satisfy the case. - Non-vacuity of all five real entries proven OUT OF TREE (committed tests cannot carry this without defeating the gate's own rule): pre-scrub content extracted from git objects identifies the slug, slug-body, uuid and uuid-prefix entries by digest, and the uuid-hex entry is confirmed by transforming the recovered UUID. Scanner exit 1 with 15 findings across bare/embedded/path-segment forms. - 29 tests pass; full-tree scan clean. Ticket: OMN-17320 --- .github/workflows/ci.yml | 47 ++ .pre-commit-config.yaml | 17 + scripts/ci/ci_summary_gate.py | 11 + .../validation/check_exposed_identifiers.py | 438 +++++++++++++++++ .../exposed_identifiers_denylist.json | 62 +++ .../gen_exposed_identifier_entry.py | 88 ++++ tests/ci/test_exposed_identifier_gate.py | 461 ++++++++++++++++++ tests/ci/test_incident_replay_omn17320.py | 123 +++++ .../migration-supersessions.tsv.captured | 7 + tests/fixtures/omn17320/replay-denylist.json | 24 + tests/incident_replays/registry.yaml | 16 + 11 files changed, 1294 insertions(+) create mode 100755 scripts/validation/check_exposed_identifiers.py create mode 100644 scripts/validation/exposed_identifiers_denylist.json create mode 100755 scripts/validation/gen_exposed_identifier_entry.py create mode 100644 tests/ci/test_exposed_identifier_gate.py create mode 100644 tests/ci/test_incident_replay_omn17320.py create mode 100644 tests/fixtures/omn17320/migration-supersessions.tsv.captured create mode 100644 tests/fixtures/omn17320/replay-denylist.json diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3988a34bf2..bf4f3ad93b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -873,6 +873,53 @@ jobs: --check-placeholders \ --scan-dir src/omnibase_infra + # OMN-17320: exposed-identifier gate. Blocks any denylisted customer identifier + # from re-entering this PUBLIC repo. Provenance: OMN-17288 scrubbed a live tenant + # slug out of five files here (omnibase_infra#3062, 3f10ee5e); three hours later + # omnimarket#2239 reintroduced the same slug there with every enforced gate green, + # on the very PR whose acceptance criterion was "zero grep hits". Nothing + # mechanically enforced the convention -- this job is that mechanism. + # + # Registered in scripts/ci/ci_summary_gate.py::STRICT_GATE_JOBS. THAT LINE IS HALF + # THE MECHANISM: the default-deny sweep already fails CI Summary when this job + # FAILS, but an unregistered job that is `skipped` or deleted yields SUCCESS -- so + # without the registration, removing this job would silently restore the + # unenforced state that produced the recurrence. + exposed-identifier-gate: + needs: occ-preflight + if: always() + name: Exposed Identifier Gate (OMN-17320) + # OMNI_RUNNER_SELECTOR_V1 — trusted CI defaults to self-hosted omnibase-ci; public forks default to ubuntu-latest + runs-on: >- + ${{ + (github.event_name == 'pull_request' && + github.event.pull_request.head.repo.full_name != github.repository) + && fromJSON(vars.OMNI_PUBLIC_PR_RUNS_ON_JSON || '["ubuntu-latest"]') + || fromJSON((github.event_name == 'merge_group' && vars.OMNI_REQUIRED_CI_RUNS_ON_JSON) || vars.OMNI_TRUSTED_CI_RUNS_ON_JSON || '["self-hosted","omnibase-ci"]') + }} + timeout-minutes: 15 + + steps: + - name: Checkout code + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + + # Full tree, not the diff: a denylisted identifier reaching dev by ANY route + # (a rebase carrying it in, as happened on omnimarket#2241) must red the PR, + # not just one that edits the offending line. Stdlib-only, so no uv setup. + - name: Scan the working tree for denylisted identifiers + run: | + echo "================================================================" + echo "Exposed Identifier Gate (OMN-17320)" + echo "================================================================" + python3 scripts/validation/check_exposed_identifiers.py \ + --mode blocking --scope all + + - name: Report the cross-repo parity fingerprint + if: always() + run: | + echo "omnibase_infra copy:" + python3 scripts/validation/check_exposed_identifiers.py --print-fingerprint + # Pin reachability -- every cross-repo pin (workflow `uses:`/`with:` refs, # pyproject [tool.uv.sources] revs, uv.lock git revs) must name a commit # reachable from a PROTECTED branch of the target repo (OMN-15538). diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 5730c67873..59bdb2856b 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -182,6 +182,23 @@ repos: exclude: ^(tests/|archived/).*\.py$ pass_filenames: true # Only check changed files, not entire codebase stages: [pre-push] # Moved to pre-push: ~1-5s for changed files only + # OMN-17320: exposed-identifier gate. Salted-SHA256 denylist of customer + # identifiers that must never re-enter this PUBLIC repo. OMN-17288 scrubbed a + # live tenant slug from five files here; three hours later an unrelated lane + # reintroduced it in omnimarket with every gate green. The denylist holds + # digests, never plaintext -- writing the literal into a pattern list in a + # public repo would be the very occurrence the gate prevents. + # Same script + same denylist as omnimarket (byte-identical; parity pinned by + # tests/ci/test_exposed_identifier_gate.py::test_cross_repo_fingerprint_pin). + # Escape hatch, per line only, ticket + reason required: + # # onex-allow-exposed-identifier OMN-XXXXX reason="" + - id: exposed-identifier-gate + name: Exposed identifier gate (OMN-17320) + entry: python3 scripts/validation/check_exposed_identifiers.py --mode blocking + language: system + pass_filenames: true + stages: [pre-commit] + exclude: ^(archived/|workspace/sibling-repos/).*$ # Cleanup: Clear tmp/ directory before commits - id: clean-tmp-directory name: Clean tmp directory diff --git a/scripts/ci/ci_summary_gate.py b/scripts/ci/ci_summary_gate.py index 26fc93733b..c6fa8bed41 100644 --- a/scripts/ci/ci_summary_gate.py +++ b/scripts/ci/ci_summary_gate.py @@ -137,6 +137,17 @@ # needs/if), so a skip is anomalous and never a legitimate opt-out. # Superseded by OMN-16783's flow-expectation ratchet when that lands. "Subscribe Wiring Health", # subscribe-wiring-health + # OMN-17320: salted-digest denylist of customer identifiers that must never + # re-enter this PUBLIC repo. THIS LINE IS HALF THE MECHANISM, same as the two + # entries above: the default-deny sweep already fails CI Summary when the job + # FAILS, but an unregistered job that is `skipped` or absent yields SUCCESS. + # Registered because the failure mode is PROVEN, not hypothetical -- OMN-17288 + # scrubbed a live tenant slug from five files here, and three hours later an + # unrelated lane reintroduced it in omnimarket with every enforced gate green, + # on the very PR whose acceptance criterion was its absence. The job is + # unconditional in ci.yml (`if: always()`), so a skip is anomalous and never a + # legitimate opt-out. + "Exposed Identifier Gate (OMN-17320)", # exposed-identifier-gate # OMN-15378 AC3: scripts/deploy-agent's standalone pytest root. ci.yml's # `deploy-agent-tests` job CALLS .github/workflows/deploy-agent-tests.yml, # so the inner job surfaces as " / " diff --git a/scripts/validation/check_exposed_identifiers.py b/scripts/validation/check_exposed_identifiers.py new file mode 100755 index 0000000000..7b16c99579 --- /dev/null +++ b/scripts/validation/check_exposed_identifiers.py @@ -0,0 +1,438 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2025 OmniNode.ai Inc. +# SPDX-License-Identifier: MIT + +"""OMN-17320: exposed-identifier gate -- hash-keyed denylist, BLOCKING MODE. + +Why this exists +--------------- +OMN-17288 scrubbed a live tenant slug + registry UUID out of two PUBLIC repos and +established a synthetic-identifier convention in their place. Three hours later an +unrelated lane (omnimarket#2239 / OMN-16804) reintroduced the slug into two files, +and every enforced gate stayed green -- including on the very PR whose acceptance +criterion was "zero grep hits". The convention was documentation, and documentation +lost a race. Operating Rule #5: detection that is not a gate gets ignored. + +Why the denylist holds digests and not the literals +--------------------------------------------------- +This file and its denylist ship in PUBLIC repositories. A plaintext denylist would +create exactly the fresh, greppable, current-tree occurrence the gate exists to +prevent -- and would force the denylist to be exempt from its own rule, which is the +precise shape (a special file nobody scans, that people copy from) that produced the +incident. So the denylist stores salted SHA-256 digests plus a human-readable class +label and the owning ticket. Nothing here restates a forbidden value. + +The salt is committed, so this is obfuscation, not secrecy: a reader holding this +repo can brute-force a short slug. That is stated rather than glossed, because it is +not the property being bought. The OMN-17288 values are already public in git history +and the operator ruled document-and-accept on that history. What this format buys is +FORWARD safety -- the next entry added here may be a live identifier that has not +leaked, and a plaintext denylist would be an active disclosure in that case. + +Matching +-------- +Each line is lowercased and split into identifier tokens; for every denylisted +length L, every L-wide window inside every token is hashed. So a literal is caught +bare (``t-example``), embedded (``tenant_t-example_v2``), and inside a path or URL +segment (``https://host/t-example/x``) -- not merely as a standalone word. + +Encoded forms (base64, percent-encoding, hex-of-utf8) are deliberately NOT decoded. +OMN-17180 owns that class; claiming coverage here would be a false claim. + +Exemption +--------- +One escape hatch, per line, ticketed and reasoned:: + + # onex-allow-exposed-identifier OMN-XXXXX reason="" + +A bare annotation with no ticket or no reason is REJECTED, matching every other +onex-allow class. There is deliberately NO file-level exemption for this class +(unlike the leaked-literals gate's ``# onex-allow-file``): a whole-file waiver is +how a forbidden value survives in a corner nobody reads, which is the failure this +gate was built from. + +The intended use is the historical-narrative case the OMN-17288 finisher hit -- +prose describing the incident itself, where substituting a synthetic stand-in would +assert a false fact about what happened. In that case the finisher dropped the +literal and kept the claim, which is still the preferred fix; the annotation exists +for when it genuinely is not. + +Output +------ +Findings never print the matched value -- printing it would defeat the gate. Each +finding gives ``path:line:col`` plus the match length and the denylist entry's id, +kind and owning ticket. Column + length pinpoint the token exactly for the author, +who already knows what they wrote. + +Exit codes: 0 clean (or advisory), 1 findings in blocking mode, 2 usage/config error. +""" + +from __future__ import annotations + +import argparse +import hashlib +import json +import re +import subprocess +import sys +from pathlib import Path +from typing import Any + +# One denylist entry: id / kind / length / sha256 / ticket / notes. Deliberately +# NOT a TypedDict -- the loader validates the shape at runtime and must fail +# closed on a malformed file, which a static-only shape would not do. +Entry = dict[str, Any] + +DEFAULT_DENYLIST = Path(__file__).with_name("exposed_identifiers_denylist.json") + +# Same ticket+reason discipline as the five leaked-literals classes. +ANNOTATION_RE = re.compile( + r"#\s*onex-allow-exposed-identifier\s+OMN-[0-9]+\s+reason=\"[^\"]+\"" +) + +# Identifier tokens. Lowercased input, so no A-Z class is needed. +TOKEN_RE = re.compile(r"[a-z0-9][a-z0-9_.\-]*") + +# Directories and suffixes that are never source-of-truth for a committed literal. +EXCLUDED_DIR_PARTS = frozenset( + { + ".git", + "dist", + "build", + ".venv", + "venv", + "node_modules", + "__pycache__", + ".mypy_cache", + ".pytest_cache", + ".ruff_cache", + "htmlcov", + ".tox", + "site-packages", + } +) +EXCLUDED_SUFFIXES = ( + ".lock", + ".pyc", + ".pyo", + ".so", + ".dylib", + ".png", + ".jpg", + ".jpeg", + ".gif", + ".webp", + ".pdf", + ".zip", + ".gz", + ".tar", + ".whl", + ".ico", + ".woff", + ".woff2", + ".ttf", + ".mp4", + ".mov", +) + +# A token longer than this is a blob (minified bundle, base64 payload), not an +# identifier a human typed. Windowing it is quadratic and finds nothing real. +MAX_TOKEN_LEN = 512 + +MAX_BYTES = 8 * 1024 * 1024 + + +class DenylistError(RuntimeError): + """The denylist is missing, malformed, or self-inconsistent.""" + + +class Denylist: + """Length-indexed digest lookup. Holds no plaintext, by construction.""" + + def __init__(self, doc: dict[str, Any]) -> None: + salt = doc.get("salt") + if not isinstance(salt, str) or not salt: + raise DenylistError("denylist has no 'salt'") + entries = doc.get("entries") + if not isinstance(entries, list) or not entries: + raise DenylistError("denylist has no 'entries'") + + self.salt = salt + # Pre-absorb the salt once; every window hash then clones this state + # instead of re-hashing the salt bytes. The window scan is the hot loop. + self._salted = hashlib.sha256(salt.encode("utf-8")) + self.entries = entries + self.by_len: dict[int, dict[str, Entry]] = {} + for entry in entries: + for field in ("id", "kind", "length", "sha256", "ticket"): + if field not in entry: + raise DenylistError( + f"denylist entry missing '{field}': {entry.get('id', '?')}" + ) + length = entry["length"] + if not isinstance(length, int) or length < 4: + raise DenylistError( + f"entry {entry['id']}: 'length' must be an int >= 4" + ) + digest = entry["sha256"] + if not isinstance(digest, str) or not re.fullmatch(r"[0-9a-f]{64}", digest): + raise DenylistError( + f"entry {entry['id']}: 'sha256' must be 64 lowercase hex chars" + ) + # A plaintext field would defeat the whole design; refuse to load one. + for forbidden in ("value", "literal", "plaintext"): + if forbidden in entry: + raise DenylistError( + f"entry {entry['id']}: '{forbidden}' is forbidden -- this denylist " + f"ships in public repos and must never carry plaintext" + ) + self.by_len.setdefault(length, {})[digest] = entry + + self.lengths = sorted(self.by_len) + self.min_len = self.lengths[0] + + @classmethod + def load(cls, path: Path) -> Denylist: + try: + doc = json.loads(path.read_text(encoding="utf-8")) + except FileNotFoundError as exc: + raise DenylistError(f"denylist not found: {path}") from exc + except json.JSONDecodeError as exc: + raise DenylistError(f"denylist is not valid JSON: {path}: {exc}") from exc + return cls(doc) + + def digest(self, value: str) -> str: + hasher = self._salted.copy() + hasher.update(value.encode("utf-8")) + return hasher.hexdigest() + + def scan_line(self, line: str) -> list[tuple[int, int, Entry]]: + """Return (col, length, entry) for each hit, longest-match-wins on overlap.""" + low = line.lower() + raw: list[tuple[int, int, Entry]] = [] + for tok in TOKEN_RE.finditer(low): + text = tok.group(0) + span = len(text) + if span < self.min_len or span > MAX_TOKEN_LEN: + continue + base = tok.start() + for length in self.lengths: + if span < length: + break # lengths ascend; every later one is longer still + bucket = self.by_len[length] + for offset in range(span - length + 1): + entry = bucket.get(self.digest(text[offset : offset + length])) + if entry is not None: + raw.append((base + offset, length, entry)) + if len(raw) < 2: + return raw + # Collapse overlaps: a full UUID also matches its own prefix entry. + raw.sort(key=lambda hit: (hit[0], -hit[1])) + kept: list[tuple[int, int, Entry]] = [] + covered_to = -1 + for col, length, entry in raw: + if col < covered_to: + continue + kept.append((col, length, entry)) + covered_to = col + length + return kept + + +def iter_repo_files(root: Path, scope: str, base_ref: str) -> list[Path]: + if scope == "diff": + probe = subprocess.run( + ["git", "-C", str(root), "rev-parse", "--verify", base_ref], + capture_output=True, + text=True, + check=False, + ) + if probe.returncode != 0: + print( + f"exposed-id-gate: WARN {base_ref} not found; falling back to scope=all", + file=sys.stderr, + ) + scope = "all" + else: + out = subprocess.run( + [ + "git", + "-C", + str(root), + "diff", + "--name-only", + "-z", + f"{base_ref}...HEAD", + ], + capture_output=True, + text=True, + check=True, + ).stdout + return [root / name for name in out.split("\0") if name] + out = subprocess.run( + ["git", "-C", str(root), "ls-files", "-coz", "--exclude-standard"], + capture_output=True, + text=True, + check=True, + ).stdout + return [root / name for name in out.split("\0") if name] + + +def is_scannable(path: Path, root: Path) -> bool: + try: + rel = path.relative_to(root) + except ValueError: + rel = path + if EXCLUDED_DIR_PARTS.intersection(rel.parts): + return False + if path.name.lower().endswith(EXCLUDED_SUFFIXES): + return False + if not path.is_file() or path.is_symlink(): + return False + try: + if path.stat().st_size > MAX_BYTES: + return False + except OSError: + return False + return True + + +def scan_file(path: Path, root: Path, denylist: Denylist) -> list[str]: + try: + raw = path.read_bytes() + except OSError: + return [] + if b"\0" in raw[:8192]: + return [] # binary + try: + text = raw.decode("utf-8") + except UnicodeDecodeError: + text = raw.decode("utf-8", errors="replace") + + try: + rel = path.relative_to(root).as_posix() + except ValueError: + rel = str(path) + + findings: list[str] = [] + for lineno, line in enumerate(text.splitlines(), start=1): + hits = denylist.scan_line(line) + if not hits: + continue + if ANNOTATION_RE.search(line): + continue + for col, length, entry in hits: + findings.append( + f"{rel}:{lineno}:{col + 1}: denylisted {entry['kind']} " + f"(entry={entry['id']} ticket={entry['ticket']} match_len={length}) " + f"-- value withheld by design" + ) + return findings + + +def fingerprint(denylist_path: Path) -> str: + self_digest = hashlib.sha256(Path(__file__).read_bytes()).hexdigest() + list_digest = hashlib.sha256(denylist_path.read_bytes()).hexdigest() + return f"scanner={self_digest} denylist={list_digest}" + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description="OMN-17320 exposed-identifier gate") + parser.add_argument( + "paths", nargs="*", help="explicit files to scan (pre-commit passes these)" + ) + parser.add_argument("--mode", choices=("blocking", "advisory"), default="blocking") + parser.add_argument( + "--scope", + choices=("all", "diff"), + default="all", + help="ignored when explicit paths are given", + ) + parser.add_argument("--base-ref", default="origin/dev") + parser.add_argument("--denylist", type=Path, default=DEFAULT_DENYLIST) + parser.add_argument("--root", type=Path, default=None) + parser.add_argument( + "--print-fingerprint", + action="store_true", + help="print sha256 of this scanner and its denylist, for cross-repo parity", + ) + args = parser.parse_args(argv) + + if args.print_fingerprint: + print(fingerprint(args.denylist)) + return 0 + + try: + denylist = Denylist.load(args.denylist) + except DenylistError as exc: + print(f"exposed-id-gate: ERROR {exc}", file=sys.stderr) + return 2 + + if args.root is not None: + root = args.root.resolve() + else: + probe = subprocess.run( + ["git", "rev-parse", "--show-toplevel"], + capture_output=True, + text=True, + check=False, + ) + root = ( + Path(probe.stdout.strip()).resolve() + if probe.returncode == 0 + else Path.cwd() + ) + + if args.paths: + # Resolve against the CWD, not --root: pre-commit and CI both hand this + # script paths relative to where they invoked it, while --root only names + # the tree findings are reported relative to. Joining to root instead + # silently produced a nonexistent path and a files_scanned=0 PASS -- a gate + # that reports success because it scanned nothing is the exact failure mode + # this ticket is about, so it is fixed rather than worked around. + candidates = [Path(p).resolve() for p in args.paths] + missing = [p for p in candidates if not p.exists()] + if missing: + print( + "exposed-id-gate: ERROR these paths do not exist: " + + ", ".join(str(p) for p in missing), + file=sys.stderr, + ) + return 2 + else: + candidates = iter_repo_files(root, args.scope, args.base_ref) + + findings: list[str] = [] + scanned = 0 + for path in candidates: + if not is_scannable(path, root): + continue + scanned += 1 + findings.extend(scan_file(path, root, denylist)) + + print( + f"exposed-id-gate: mode={args.mode} scope=" + f"{'paths' if args.paths else args.scope} entries={len(denylist.entries)} " + f"files_scanned={scanned} findings={len(findings)}" + ) + for finding in findings: + print(f" {finding}") + + if findings: + print( + "exposed-id-gate: a denylisted identifier is present. Replace it with a synthetic " + "stand-in, or -- only for prose narrating the incident itself, where a stand-in " + "would assert a false fact -- drop the literal and keep the claim. If neither " + "applies, annotate the line:\n" + ' # onex-allow-exposed-identifier OMN-XXXXX reason=""\n' + "Plaintext for each entry is recorded in its owning Linear ticket, not here.", + file=sys.stderr, + ) + + if args.mode == "advisory": + print("exposed-id-gate: advisory mode -- exit 0 regardless of findings") + return 0 + return 1 if findings else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/validation/exposed_identifiers_denylist.json b/scripts/validation/exposed_identifiers_denylist.json new file mode 100644 index 0000000000..fa29d70b91 --- /dev/null +++ b/scripts/validation/exposed_identifiers_denylist.json @@ -0,0 +1,62 @@ +{ + "schema": "onex.exposed-identifier-denylist/1", + "ticket": "OMN-17320", + "salt": "onex-exposed-identifier-v1:", + "annotation_class": "exposed-identifier", + "plaintext_source": "OMN-17288 (Linear, private workspace) -- description + the 'Finding 2' comment carry every value below in full.", + "readme": [ + "Each entry is a salted SHA-256 of a forbidden literal. The plaintext is deliberately", + "ABSENT: this file ships in PUBLIC repos, and writing the literal here would create the", + "exact fresh, greppable, current-tree occurrence the gate exists to prevent.", + "The salt is committed. That makes this obfuscation, not secrecy -- a reader with this", + "file can brute-force a short slug. It is not trying to be a confidentiality control:", + "the OMN-17288 values are already public in git history and the operator ruled", + "document-and-accept on that. The property that matters is FORWARD safety -- the next", + "entry added here may be a live identifier that has NOT leaked, and this format is safe", + "for that case where a plaintext denylist would not be.", + "To add an entry: run scripts/validation/gen_exposed_identifier_entry.py ,", + "paste the emitted object here, and record the plaintext in the owning Linear ticket." + ], + "entries": [ + { + "id": "omn17288-tenant-slug", + "kind": "tenant-slug", + "length": 11, + "sha256": "fab7da6b5e1546d6181ef3a8f81a706e653621a3c9d314dc1508fb88622c7012", + "ticket": "OMN-17288", + "notes": "External tenant slug scrubbed from omnimarket + omnibase_infra by OMN-17288; reintroduced by omnimarket#2239 three hours later." + }, + { + "id": "omn17288-tenant-slug-body", + "kind": "tenant-slug-body", + "length": 9, + "sha256": "6e615fc3fd71dd3860c60d1a52200f62bebdcb202ed3458dc615a0cb7fdee9a4", + "ticket": "OMN-17288", + "notes": "Same slug without its tenant prefix -- the discriminating substring OMN-17288's own AC4 grep matched on." + }, + { + "id": "omn17288-tenant-uuid", + "kind": "tenant-uuid", + "length": 36, + "sha256": "0ccdfc9608f1f41a65ffba8fb3e5e1adf5ec03d63dd57e68f4ee9cb2017d1886", + "ticket": "OMN-17288", + "notes": "Registry UUID for the same tenant, scrubbed from omnimarket by OMN-17288. Never reached omnibase_infra." + }, + { + "id": "omn17288-tenant-uuid-prefix", + "kind": "tenant-uuid-prefix", + "length": 18, + "sha256": "4cb40ecee91f0007ebdf58d51be2608cfccc8b507e11119867381662fbf912bc", + "ticket": "OMN-17288", + "notes": "Leading three groups of the same UUID -- the form OMN-17288's AC4 grep used, and the form a truncated log line leaves behind." + }, + { + "id": "omn17288-tenant-uuid-hex", + "kind": "tenant-uuid-hex", + "length": 32, + "sha256": "4381a13c2440ddb99abd8e9077147daee6b2d1e5c3990af8912a74d1aa2eb393", + "ticket": "OMN-17288", + "notes": "Same UUID with dashes stripped -- the shape uuid.UUID(...).hex and most JSON encoders emit." + } + ] +} diff --git a/scripts/validation/gen_exposed_identifier_entry.py b/scripts/validation/gen_exposed_identifier_entry.py new file mode 100755 index 0000000000..5dd1c2f266 --- /dev/null +++ b/scripts/validation/gen_exposed_identifier_entry.py @@ -0,0 +1,88 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2025 OmniNode.ai Inc. +# SPDX-License-Identifier: MIT + +"""OMN-17320: mint one exposed_identifiers_denylist.json entry from a literal. + +Reads the literal from STDIN, never from argv -- an argv value lands in shell +history, process listings, and any transcript of the session that minted it, which +for a not-yet-leaked identifier would be a fresh disclosure in three more places. + +Emits ONLY the digest object. The plaintext is never echoed and never written to +disk by this script; record it in the owning Linear ticket, which is private. + + printf '%s' '' | python3 scripts/validation/gen_exposed_identifier_entry.py \\ + --id omn-XXXXX-what-it-is --kind tenant-slug --ticket OMN-XXXXX \\ + --notes "where it leaked and what replaced it" + +Paste the printed object into the "entries" array, keeping the array sorted by id, +then run the gate to confirm the tree is clean before committing. +""" + +from __future__ import annotations + +import argparse +import hashlib +import json +import sys +from pathlib import Path + +DEFAULT_DENYLIST = Path(__file__).with_name("exposed_identifiers_denylist.json") + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument( + "--id", required=True, help="stable entry id, e.g. omn-17288-tenant-slug" + ) + parser.add_argument( + "--kind", required=True, help="class label, e.g. tenant-slug / tenant-uuid" + ) + parser.add_argument("--ticket", required=True, help="owning ticket, e.g. OMN-17288") + parser.add_argument( + "--notes", required=True, help="where it leaked and what replaced it" + ) + parser.add_argument( + "--denylist", + type=Path, + default=DEFAULT_DENYLIST, + help="read the salt from this denylist so the digest is comparable", + ) + args = parser.parse_args(argv) + + literal = sys.stdin.read().strip() + if not literal: + print("ERROR: no literal on stdin", file=sys.stderr) + return 2 + if len(literal) < 4: + print( + "ERROR: literal shorter than 4 chars would match everywhere", + file=sys.stderr, + ) + return 2 + + salt = json.loads(args.denylist.read_text(encoding="utf-8"))["salt"] + digest = hashlib.sha256((salt + literal.lower()).encode("utf-8")).hexdigest() + + print( + json.dumps( + { + "id": args.id, + "kind": args.kind, + "length": len(literal), + "sha256": digest, + "ticket": args.ticket, + "notes": args.notes, + }, + indent=6, + ) + ) + print( + f"\nminted len={len(literal)} -- plaintext NOT echoed; record it in {args.ticket}.", + file=sys.stderr, + ) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/ci/test_exposed_identifier_gate.py b/tests/ci/test_exposed_identifier_gate.py new file mode 100644 index 0000000000..00919430ca --- /dev/null +++ b/tests/ci/test_exposed_identifier_gate.py @@ -0,0 +1,461 @@ +# SPDX-FileCopyrightText: 2025 OmniNode.ai Inc. +# SPDX-License-Identifier: MIT + +"""OMN-17320: tests for the exposed-identifier gate. + +Every mechanism test below runs against a **synthetic** denylist minted inside +``tmp_path``. That is not a convenience -- it is the point. Committing a fixture +containing a real denylisted literal would create precisely the fresh, greppable, +current-tree occurrence the gate exists to prevent, and would make this file a +whole-file exemption: the shape that produced the OMN-17288 incident in the first +place. So the mechanism is proven on a synthetic entry, and non-vacuity against the +*real* entries is proven two other ways: + +* ``test_incident_replay_omn17288`` replays the actual pre-scrub file content out of + this repo's own git history (no literal is committed -- it is read from objects that + already exist), and +* an out-of-tree run recorded in the PR body, which exercises every real entry. + +That split is stated rather than glossed because a test suite that silently proved +nothing about the real entries is exactly the failure mode this ticket exists to fix. + +The gate is invoked as a real subprocess throughout -- no mocking, no reimplementation +of the matching logic. +""" + +from __future__ import annotations + +import hashlib +import json +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).parent.parent.parent +GATE = REPO_ROOT / "scripts" / "validation" / "check_exposed_identifiers.py" +REAL_DENYLIST = ( + REPO_ROOT / "scripts" / "validation" / "exposed_identifiers_denylist.json" +) +MINTER = REPO_ROOT / "scripts" / "validation" / "gen_exposed_identifier_entry.py" + +# A stand-in that exists only in this test module. It is not a real identifier and +# is deliberately not in the shipped denylist, so the live gate ignores it. +SYNTHETIC = "t-omn17320-synthetic-probe" + +# OMN-17288 incident coordinates for THIS repo. `3f10ee5e` is the scrub merge +# (omnibase_infra#3062); its first parent is the dev tip that still carried the +# slug in all five files below. omnibase_infra never carried the tenant UUID. +INCIDENT_FIX = "3f10ee5e" +INCIDENT_FILES = ( + "docker/migrations/forward/_ledger/migration-supersessions.tsv", + "docker/migrations/forward/fenced-node-migrations.yaml", + "docker/migrations/forward/nodes/node_projection_delegation/" + "0032_delegation_events_tenant_id_uuid_via_registry.sql", + "tests/ci/test_omn16930_registry_resolved_conversion.py", + "tests/fixtures/omn16906/commit-7090f386f.gh-api.json.captured", +) + +pytestmark = pytest.mark.unit + + +def _mint(tmp_path: Path, *literals: str) -> Path: + """Write a synthetic denylist holding digests of `literals` under the real salt.""" + salt = json.loads(REAL_DENYLIST.read_text(encoding="utf-8"))["salt"] + doc = { + "schema": "onex.exposed-identifier-denylist/1", + "ticket": "OMN-17320", + "salt": salt, + "annotation_class": "exposed-identifier", + "plaintext_source": "synthetic test fixture; no real identifier", + "entries": [ + { + "id": f"synthetic-{index}", + "kind": "test-fixture", + "length": len(literal), + "sha256": hashlib.sha256( + (salt + literal.lower()).encode("utf-8") + ).hexdigest(), + "ticket": "OMN-17320", + "notes": "synthetic probe minted by the test suite", + } + for index, literal in enumerate(literals) + ], + } + path = tmp_path / "denylist.json" + path.write_text(json.dumps(doc, indent=2) + "\n", encoding="utf-8") + return path + + +def _run(target: Path, denylist: Path, *extra: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [ + sys.executable, + str(GATE), + "--denylist", + str(denylist), + "--root", + str(target.parent), + str(target), + *extra, + ], + capture_output=True, + text=True, + check=False, + ) + + +def _probe( + tmp_path: Path, content: str, *extra: str +) -> subprocess.CompletedProcess[str]: + target = tmp_path / "sample.py" + target.write_text(content, encoding="utf-8") + return _run(target, _mint(tmp_path, SYNTHETIC), *extra) + + +# --------------------------------------------------------------------------- # +# AC2 -- the three match shapes, plus case folding. +# --------------------------------------------------------------------------- # + + +@pytest.mark.parametrize( + ("content", "label"), + [ + (f'slug = "{SYNTHETIC}"\n', "bare token"), + (f'slug = "tenant_{SYNTHETIC}_v2"\n', "embedded in a longer token"), + ( + f'url = "https://api.example.test/v1/tenants/{SYNTHETIC}/events"\n', + "path segment", + ), + (f'slug = "{SYNTHETIC.upper()}"\n', "uppercased"), + (f'row = ["a", "{SYNTHETIC}", "b"]\n', "inside a list literal"), + ], +) +def test_denylisted_literal_is_caught(tmp_path: Path, content: str, label: str) -> None: + result = _probe(tmp_path, content) + assert result.returncode == 1, ( + f"{label}: gate passed content it must reject\n{result.stdout}" + ) + assert "findings=1" in result.stdout, f"{label}: {result.stdout}" + + +def test_clean_content_passes(tmp_path: Path) -> None: + result = _probe(tmp_path, 'slug = "t-synthetic-fixture-unrelated"\n') + assert result.returncode == 0, result.stdout + assert "findings=0" in result.stdout + + +def test_a_shorter_prefix_of_the_literal_does_not_match(tmp_path: Path) -> None: + """Guards against an off-by-one turning the window scan into a prefix match.""" + result = _probe(tmp_path, f'slug = "{SYNTHETIC[:-1]}"\n') + assert result.returncode == 0, result.stdout + + +# --------------------------------------------------------------------------- # +# AC5 -- the annotation is the ONLY escape hatch, and it must be ticketed+reasoned. +# --------------------------------------------------------------------------- # + + +def test_annotation_with_ticket_and_reason_exempts(tmp_path: Path) -> None: + content = ( + f'slug = "{SYNTHETIC}" # onex-allow-exposed-identifier OMN-17320 ' + f'reason="historical narrative; a stand-in would assert a false fact"\n' + ) + result = _probe(tmp_path, content) + assert result.returncode == 0, result.stdout + + +@pytest.mark.parametrize( + ("suffix", "label"), + [ + ("# onex-allow-exposed-identifier", "bare annotation, no ticket, no reason"), + ("# onex-allow-exposed-identifier OMN-17320", "ticket but no reason"), + ('# onex-allow-exposed-identifier reason="no ticket"', "reason but no ticket"), + ( + '# onex-allow-internal-ip OMN-17320 reason="wrong class"', + "a different onex-allow class", + ), + ], +) +def test_malformed_or_wrong_class_annotation_does_not_exempt( + tmp_path: Path, suffix: str, label: str +) -> None: + result = _probe(tmp_path, f'slug = "{SYNTHETIC}" {suffix}\n') + assert result.returncode == 1, ( + f"{label}: annotation wrongly accepted\n{result.stdout}" + ) + + +def test_annotation_does_not_exempt_a_different_line(tmp_path: Path) -> None: + content = ( + '# onex-allow-exposed-identifier OMN-17320 reason="this is on its own line"\n' + f'slug = "{SYNTHETIC}"\n' + ) + result = _probe(tmp_path, content) + assert result.returncode == 1, result.stdout + + +def test_there_is_no_file_level_exemption_for_this_class(tmp_path: Path) -> None: + """The leaked-literals gate honours `# onex-allow-file`. This one must not. + + A whole-file waiver is how a forbidden value survives in a corner nobody reads, + which is the OMN-17288 failure mode. Per-line only, deliberately. + """ + content = ( + '# onex-allow-file OMN-17320 reason="attempting a whole-file waiver"\n' + f'slug = "{SYNTHETIC}"\n' + ) + result = _probe(tmp_path, content) + assert result.returncode == 1, f"file-level waiver was honoured\n{result.stdout}" + + +# --------------------------------------------------------------------------- # +# The gate must not leak what it guards. +# --------------------------------------------------------------------------- # + + +def test_findings_never_print_the_matched_value(tmp_path: Path) -> None: + result = _probe(tmp_path, f'slug = "{SYNTHETIC}"\n') + assert result.returncode == 1 + combined = result.stdout + result.stderr + assert SYNTHETIC not in combined, "the gate printed the value it exists to suppress" + # It must still pinpoint the token: line 1, column 9, length 26. + assert "sample.py:1:9:" in result.stdout, result.stdout + assert f"match_len={len(SYNTHETIC)}" in result.stdout, result.stdout + + +def test_shipped_denylist_contains_no_plaintext(tmp_path: Path) -> None: + doc = json.loads(REAL_DENYLIST.read_text(encoding="utf-8")) + for entry in doc["entries"]: + assert set(entry) == {"id", "kind", "length", "sha256", "ticket", "notes"}, ( + entry["id"] + ) + + +def test_denylist_loader_refuses_a_plaintext_field(tmp_path: Path) -> None: + """Fail closed if someone 'helpfully' adds the literal back for readability.""" + denylist = _mint(tmp_path, SYNTHETIC) + doc = json.loads(denylist.read_text(encoding="utf-8")) + doc["entries"][0]["value"] = SYNTHETIC + denylist.write_text(json.dumps(doc, indent=2) + "\n", encoding="utf-8") + target = tmp_path / "sample.py" + target.write_text("x = 1\n", encoding="utf-8") + result = _run(target, denylist) + assert result.returncode == 2, result.stdout + result.stderr + assert "must never carry plaintext" in result.stderr, result.stderr + + +# --------------------------------------------------------------------------- # +# Shipped denylist shape + overlap collapse + modes. +# --------------------------------------------------------------------------- # + + +def test_shipped_denylist_is_wellformed() -> None: + doc = json.loads(REAL_DENYLIST.read_text(encoding="utf-8")) + assert doc["schema"] == "onex.exposed-identifier-denylist/1" + assert doc["annotation_class"] == "exposed-identifier" + assert doc["salt"], "an empty salt makes every digest a plain sha256" + entries = doc["entries"] + assert len(entries) >= 5, ( + "the OMN-17288 pair expands to 5 shapes; fewer means one was dropped" + ) + ids = [entry["id"] for entry in entries] + assert len(ids) == len(set(ids)), f"duplicate entry ids: {ids}" + for entry in entries: + assert len(entry["sha256"]) == 64, entry["id"] + assert entry["sha256"].islower(), entry["id"] + assert int(entry["sha256"], 16) >= 0 + assert entry["length"] >= 4, ( + f"{entry['id']}: a <4 char literal would match everywhere" + ) + assert entry["ticket"].startswith("OMN-"), entry["id"] + assert entry["notes"].strip(), ( + f"{entry['id']}: every entry must say where it leaked" + ) + # The five OMN-17288 shapes must all be present by kind. + kinds = {entry["kind"] for entry in entries} + assert { + "tenant-slug", + "tenant-slug-body", + "tenant-uuid", + "tenant-uuid-prefix", + "tenant-uuid-hex", + } <= kinds, kinds + + +def test_overlapping_entries_collapse_to_the_longest(tmp_path: Path) -> None: + """A full value also matches a shorter entry nested inside it; report it once.""" + denylist = _mint(tmp_path, SYNTHETIC, SYNTHETIC[:12]) + target = tmp_path / "sample.py" + target.write_text(f'slug = "{SYNTHETIC}"\n', encoding="utf-8") + result = _run(target, denylist) + assert result.returncode == 1 + assert "findings=1" in result.stdout, f"overlap not collapsed:\n{result.stdout}" + assert f"match_len={len(SYNTHETIC)}" in result.stdout, result.stdout + + +def test_advisory_mode_reports_but_exits_zero(tmp_path: Path) -> None: + result = _probe(tmp_path, f'slug = "{SYNTHETIC}"\n', "--mode", "advisory") + assert result.returncode == 0, result.stdout + assert "findings=1" in result.stdout + + +def test_minter_never_echoes_the_literal(tmp_path: Path) -> None: + result = subprocess.run( + [ + sys.executable, + str(MINTER), + "--id", + "probe", + "--kind", + "test-fixture", + "--ticket", + "OMN-17320", + "--notes", + "probe", + "--denylist", + str(REAL_DENYLIST), + ], + input=SYNTHETIC, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr + assert SYNTHETIC not in result.stdout + result.stderr, "the minter echoed its input" + minted = json.loads(result.stdout) + salt = json.loads(REAL_DENYLIST.read_text(encoding="utf-8"))["salt"] + expected = hashlib.sha256((salt + SYNTHETIC).encode("utf-8")).hexdigest() + assert minted["sha256"] == expected + assert minted["length"] == len(SYNTHETIC) + + +# --------------------------------------------------------------------------- # +# AC4 -- real-incident replay (OMN-15547 convention), and the standing regression. +# --------------------------------------------------------------------------- # + + +def _git(*args: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["git", "-C", str(REPO_ROOT), *args], + capture_output=True, + text=True, + check=False, + ) + + +def test_incident_replay_omn17288(tmp_path: Path) -> None: + """Replay the five pre-scrub omnibase_infra files against the shipped denylist. + + No literal is committed by this test: the content is read out of git objects + that already exist in this repository's history. Skips on a shallow clone -- + the durable proof for the real entries is the out-of-tree run recorded on the + PR, and this test is the in-repo half of it. + """ + if _git("cat-file", "-e", f"{INCIDENT_FIX}^{{commit}}").returncode != 0: + pytest.skip( + f"{INCIDENT_FIX} unreachable (shallow clone); see PR body for the recorded run" + ) + + flagged: list[str] = [] + for path in INCIDENT_FILES: + pre = _git("show", f"{INCIDENT_FIX}^:{path}") + post = _git("show", f"{INCIDENT_FIX}:{path}") + assert pre.returncode == 0, f"{path} not in history at {INCIDENT_FIX}^" + assert post.returncode == 0, f"{path} not in history at {INCIDENT_FIX}" + + pre_file = tmp_path / f"pre_{Path(path).name}" + pre_file.write_text(pre.stdout, encoding="utf-8") + pre_result = _run(pre_file, REAL_DENYLIST) + assert pre_result.returncode == 1, ( + f"RED control is vacuous: the gate PASSED {path} at {INCIDENT_FIX}^, " + f"which is the content that reintroduced the slug\n{pre_result.stdout}" + ) + flagged.append(pre_result.stdout) + + post_file = tmp_path / f"post_{Path(path).name}" + post_file.write_text(post.stdout, encoding="utf-8") + post_result = _run(post_file, REAL_DENYLIST) + assert post_result.returncode == 0, ( + f"the gate rejects the merged fix for {path}\n{post_result.stdout}" + ) + + # Every one of the five files must have produced at least one finding; a replay + # that silently covered fewer files than the census recorded is not a replay. + assert len(flagged) == len(INCIDENT_FILES), flagged + + +def test_the_whole_repository_is_clean() -> None: + """The standing regression -- the check that runs on every PR and every commit.""" + result = subprocess.run( + [sys.executable, str(GATE), "--mode", "blocking", "--scope", "all"], + cwd=REPO_ROOT, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, ( + "a denylisted identifier is present in the working tree:\n" + + result.stdout + + result.stderr + ) + + +# --------------------------------------------------------------------------- # +# AC7 -- cross-repo parity. +# --------------------------------------------------------------------------- # + +# The scanner and its denylist ship byte-identical in omnimarket and +# omnibase_infra -- the two PUBLIC repos the OMN-17288 occurrence map names. These +# digests are the SAME two strings in both repos' copies of this test, so drift is +# visible to a one-line grep across the pair. +# +# Honest limit, stated rather than implied: this pin fails in the repo being +# EDITED, which forces whoever changes the scanner to update the pin and notice the +# other copy. It does not, by itself, prove the other repo was updated too. A hard +# cross-repo gate would have omnibase_infra CI resolve omnimarket's live dev HEAD -- +# and OMN-17292 is an open ticket about exactly that coupling reddening every infra +# PR whenever omnimarket merges something unrelated. Adding a second instance of an +# anti-pattern the team just filed against would trade a real problem for a worse +# one, so the weaker-but-local check is deliberate. +PINNED_SCANNER_SHA256 = ( + "3bcae6e1d4c884ec9cc21d76ad75ef2a394ae5376b0f3fa8263f3ba607ce6826" +) +PINNED_DENYLIST_SHA256 = ( + "0ca020357e5745885bd120c9e96ba95e069c34ea7fb1eb745b7f9895f635e8b6" +) + + +def test_cross_repo_fingerprint_pin() -> None: + scanner = hashlib.sha256(GATE.read_bytes()).hexdigest() + denylist = hashlib.sha256(REAL_DENYLIST.read_bytes()).hexdigest() + assert scanner == PINNED_SCANNER_SHA256, ( + "check_exposed_identifiers.py changed. Apply the SAME edit to the other " + "public repo's copy (omnimarket <-> omnibase_infra), then update " + f"PINNED_SCANNER_SHA256 in BOTH copies of this test to {scanner}." + ) + assert denylist == PINNED_DENYLIST_SHA256, ( + "exposed_identifiers_denylist.json changed. A denylist entry added in one " + "public repo but not the other leaves that repo unguarded against the very " + "identifier someone just decided was forbidden. Apply the same edit to the " + f"other repo, then update PINNED_DENYLIST_SHA256 in both to {denylist}." + ) + + +def test_fingerprint_subcommand_reports_both_digests() -> None: + result = subprocess.run( + [ + sys.executable, + str(GATE), + "--denylist", + str(REAL_DENYLIST), + "--print-fingerprint", + ], + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr + assert PINNED_SCANNER_SHA256 in result.stdout, result.stdout + assert PINNED_DENYLIST_SHA256 in result.stdout, result.stdout diff --git a/tests/ci/test_incident_replay_omn17320.py b/tests/ci/test_incident_replay_omn17320.py new file mode 100644 index 0000000000..d5d2161b5b --- /dev/null +++ b/tests/ci/test_incident_replay_omn17320.py @@ -0,0 +1,123 @@ +# SPDX-FileCopyrightText: 2025 OmniNode.ai Inc. +# SPDX-License-Identifier: MIT + +"""OMN-17320 incident replay (OMN-15547 convention). + +The incident: OMN-17288 scrubbed a live tenant slug out of five files in this repo +(omnibase_infra#3062, merge ``3f10ee5e``). Three hours later omnimarket#2239 +reintroduced the same slug there, and the rebase carried it onto omnimarket#2241 -- +the PR whose acceptance criterion was "zero grep hits" -- with **every enforced gate +green**. There was no guard. That absence is the false-green being replaced. + +The artifact is ``docker/migrations/forward/_ledger/migration-supersessions.tsv`` as +it stood on this repo's ``dev`` at ``3f10ee5e^``, immediately before the scrub -- one +of the five files the OMN-17288 census enumerated. + +ONE DOCUMENTED REDACTION, and why it is not a hand-typed approximation +--------------------------------------------------------------------- +The 11-byte slug on line 7 is replaced by an 11-byte stand-in. Every other byte of +the 8455 is verbatim, and the length is preserved so every offset in the file is +unchanged. The pre-redaction sha256 is recorded in the registry entry, so the claim +is checkable by anyone who re-fetches the git object. + +The redaction is unavoidable: this repo is PUBLIC, and committing the real value as +a test fixture is precisely the disclosure the guard under test exists to prevent. +An unredacted fixture would fail the very gate it is proving. This is not a novel +liberty -- OMN-17288 set the same precedent in this same registry, redacting a byte +range out of ``tests/fixtures/omn16906/commit-7090f386f.gh-api.json.captured`` and +moving its sha256 pin with the original hash documented. + +What is genuinely replayed is what matters: the guard is driven over the REAL +surrounding bytes -- the real TSV field structure, the real prose, the real token +boundaries at the real byte offset (line 7, column 379) -- not over a toy line an +author imagined. A synthetic fixture could only ever exhibit failures its author had +already thought of, which is the whole complaint OMN-15547 was filed about. +""" + +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).parent.parent.parent +GATE = REPO_ROOT / "scripts" / "validation" / "check_exposed_identifiers.py" +FIXTURE_DIR = REPO_ROOT / "tests" / "fixtures" / "omn17320" +ARTIFACT = FIXTURE_DIR / "migration-supersessions.tsv.captured" +REPLAY_DENYLIST = FIXTURE_DIR / "replay-denylist.json" +SHIPPED_DENYLIST = ( + REPO_ROOT / "scripts" / "validation" / "exposed_identifiers_denylist.json" +) + +# Byte coordinates of the slug in the captured artifact, preserved by the +# length-neutral redaction. If either moves, the fixture is no longer the artifact. +SLUG_LINE = 7 +SLUG_COLUMN = 379 + +pytestmark = pytest.mark.unit + + +def _run(denylist: Path) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [ + sys.executable, + str(GATE), + "--denylist", + str(denylist), + "--root", + str(FIXTURE_DIR), + str(ARTIFACT), + ], + capture_output=True, + text=True, + check=False, + ) + + +def test_the_real_guard_rejects_the_captured_artifact() -> None: + """R5, false_green direction: the guard must say REJECT on the real bad input. + + Before this ticket there was no guard at all, so this content -- and the + omnimarket reintroduction three hours later -- passed every gate in the org. + """ + result = _run(REPLAY_DENYLIST) + assert result.returncode == 1, ( + "the guard ACCEPTED the pre-scrub artifact; the replay is vacuous\n" + + result.stdout + + result.stderr + ) + assert f"{ARTIFACT.name}:{SLUG_LINE}:{SLUG_COLUMN}:" in result.stdout, ( + "the finding did not land at the slug's real byte offset -- the fixture is " + f"no longer positioned like the artifact\n{result.stdout}" + ) + assert "match_len=11" in result.stdout, result.stdout + + +def test_the_guard_does_not_print_the_value_it_found() -> None: + """A gate that echoes the identifier into CI logs has moved the leak, not closed it.""" + result = _run(REPLAY_DENYLIST) + standin = "t-replayfix" + assert standin not in result.stdout + result.stderr, result.stdout + + +def test_the_shipped_denylist_does_not_fire_on_this_artifact() -> None: + """Accept control: a guard that rejects everything proves nothing by rejecting.""" + result = _run(SHIPPED_DENYLIST) + assert result.returncode == 0, ( + "the shipped denylist flags the redacted artifact -- either the redaction " + "failed or an entry is over-broad\n" + result.stdout + ) + + +def test_the_artifact_carries_no_real_denylisted_identifier() -> None: + """The fixture itself must satisfy the rule the guard enforces.""" + result = subprocess.run( + [sys.executable, str(GATE), "--mode", "blocking", "--scope", "all"], + cwd=REPO_ROOT, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stdout + result.stderr diff --git a/tests/fixtures/omn17320/migration-supersessions.tsv.captured b/tests/fixtures/omn17320/migration-supersessions.tsv.captured new file mode 100644 index 0000000000..ec13c190da --- /dev/null +++ b/tests/fixtures/omn17320/migration-supersessions.tsv.captured @@ -0,0 +1,7 @@ +nodes/node_delegation_routing_reducer/0001_create_delegation_routing_tenant_overlay.sql nodes/node_delegation_routing_reducer/0002_overlay_positive_bound_constraints.sql OMN-16705 Restored to the bytes the .201 dev lane actually applied (content sha fdf0cc2c, commit 0ca6735fa, applied 2026-08-22 20:09:08+00). Commits 88f4ac346 and 7de798a4a had rewritten this applied file in place, so bootstrap.sql raised conflicting migration checksum in canonical node history on every later run. The delta is re-expressed additively in 0002. +nodes/node_projection_tenant_credentials/0000_create_tenant_inference_credentials.sql nodes/node_projection_tenant_credentials/0002_credential_identity_not_null.sql OMN-16705 Restored to the bytes the .201 dev lane actually applied (content sha c1691130, commit 559ee461a, applied 2026-08-22 20:09:18+00). Commit 7de798a4a had rewritten this applied file in place with 46 lines of reconciliation and five SET NOT NULL that no database ever ran. The delta is re-expressed additively in 0002, minus name/provider which 0001 deliberately relaxes. +nodes/node_projection_consumer_flow/0000_create_consumer_flow_windows.sql nodes/node_projection_consumer_flow/0001_reconcile_consumer_flow_window_columns.sql OMN-16773 CI shape reconciliation requires 0000 to include explicit ADD COLUMN IF NOT EXISTS guards after CREATE TABLE IF NOT EXISTS; the same idempotent reconciliation is expressed additively in 0001 for already-applied deployments. +nodes/node_projection_consumer_flow/0000_create_consumer_flow_windows.sql nodes/node_projection_consumer_flow/0002_reconcile_consumer_flow_window_shapes.sql OMN-16777 Both 0000 and 0001 spelled every declared-NOT-NULL column as `ADD COLUMN IF NOT EXISTS NOT NULL`, which cannot reconcile a drifted table that holds rows: PostgreSQL raises 'column "" of relation "" contains null values' and ON_ERROR_STOP=1 kills the Job -- the same deploy-stopping failure the reconciliation exists to prevent, so tests/ci/test_node_migration_shape_reconciliation.py went GREEN while the failure class stayed open. Proven by execution on the .201 dev lane 2026-08-28 against a seeded drifted consumer_flow_windows (only consumer_group, one row): the NOT NULL spelling exits 3 on `topic`; the nullable spelling exits 0 and converges all 14 columns. Rewriting both files is safe because neither was ever applied -- to_regclass() for both relations is ABSENT on the .201 dev lane, so no content_sha256 is recorded anywhere to conflict with. [OMN-17139: the clause originally appended here, 'and onex_application_migration_manifest exists on NO .201 lane', is not evidence of anything -- that relation is a TEMP table bootstrap.sql builds per session and is absent on every lane by construction. The applied-ness question is settled only by platform_catalog.schema_migrations; probe it with scripts/migrations/check_migration_applied_on_lane.py. The conclusion above stands on the to_regclass() evidence that precedes the struck clause.] The corrected delta is re-expressed additively in 0002. +nodes/node_projection_consumer_flow/0001_reconcile_consumer_flow_window_columns.sql nodes/node_projection_consumer_flow/0002_reconcile_consumer_flow_window_shapes.sql OMN-16777 Both 0000 and 0001 spelled every declared-NOT-NULL column as `ADD COLUMN IF NOT EXISTS NOT NULL`, which cannot reconcile a drifted table that holds rows: PostgreSQL raises 'column "" of relation "" contains null values' and ON_ERROR_STOP=1 kills the Job -- the same deploy-stopping failure the reconciliation exists to prevent, so tests/ci/test_node_migration_shape_reconciliation.py went GREEN while the failure class stayed open. Proven by execution on the .201 dev lane 2026-08-28 against a seeded drifted consumer_flow_windows (only consumer_group, one row): the NOT NULL spelling exits 3 on `topic`; the nullable spelling exits 0 and converges all 14 columns. Rewriting both files is safe because neither was ever applied -- to_regclass() for both relations is ABSENT on the .201 dev lane, so no content_sha256 is recorded anywhere to conflict with. [OMN-17139: the clause originally appended here, 'and onex_application_migration_manifest exists on NO .201 lane', is not evidence of anything -- that relation is a TEMP table bootstrap.sql builds per session and is absent on every lane by construction. The applied-ness question is settled only by platform_catalog.schema_migrations; probe it with scripts/migrations/check_migration_applied_on_lane.py. The conclusion above stands on the to_regclass() evidence that precedes the struck clause.] The corrected delta is re-expressed additively in 0002. +nodes/node_projection_work_events/0001_create_work_events.sql nodes/node_projection_work_events/0002_work_events_summary_bound.sql OMN-16180 0001 shipped an internal LAN address in a header comment. omnimarket blocks that literal (leaked-literals gate OMN-10580) while its node-migration-vendor-parity-gate requires the omnimarket source file to be byte-identical to this vendored copy, so the address cannot be scrubbed on one side only. The scrub is COMMENT-ONLY -- every executable statement in 0001 is byte-unchanged -- and the recorded content_sha256 is updated with it. CORRECTED OMN-17139: the safety claim originally recorded here was false. It read '0001 has only ever been applied by hand (psql -f) ... never through run-forward-migrations.sh, so no onex_application_migration_manifest row records its prior checksum on any lane.' Both halves fail. The dev lane applied 0001 THROUGH run-forward-migrations.sh at 2026-08-30 04:59:57Z and recorded it in platform_catalog.schema_migrations (checksum cba8013e..., provenance file:nodes/node_projection_work_events/0001_create_work_events.sql) -- two hours before this rewrite merged. And onex_application_migration_manifest is not the relation that gates: it is a TEMP table bootstrap.sql builds from the checked-in TSV, so a to_regclass() probe against it reads clean on every lane and proves nothing. The gate is migration_is_applied() in run-forward-migrations.sh, reading platform_catalog.schema_migrations; it hard-failed the dev lane at dev tip. The rewrite itself is genuinely comment-only (every executable statement byte-unchanged, proven by scripts/migrations/prove_migration_revision_equivalence.py) and is admitted by the declaration in _ledger/verified-canonical-adoptions.tsv, not by this row. The successor named here additionally closes a real gap -- summary is bounded at 2000 chars in ModelWorkEventRow but the column was bare TEXT. +nodes/node_projection_delegation/0031_delegation_events_tenant_id_to_uuid.sql nodes/node_projection_delegation/0032_delegation_events_tenant_id_uuid_via_registry.sql OMN-16930 0031 resolved tenant identity from a CLOSED THREE-VALUE LITERAL CASE with no ELSE. The live table holds 169 rows across SIX writing slugs (census 2026-08-29), nine of them outside that map -- including t-replayfix, a real active externally-owned customer provisioned eight days AFTER 0031 was authored. The map is incomplete BY CONSTRUCTION: omninode_cloud.public.tenants grows on every beta signup (39 tenants and counting), and a migration is immutable once applied, so no literal map authored at any point in time can stay correct. 0031's bytes CANNOT be revised. The .201 dev lane's platform_catalog.schema_migrations holds it with checksum 79ee3b021d0a04088b2f733fa0558ea110b2a6f75b4fb338abe9c5c123f74442 kind content_sha256, byte-identical to both committed copies and to this ledger's own manifest row; bootstrap.sql raises 'conflicting migration checksum in canonical node history' on any divergence (the OMN-16705 class). Four-lane sweep 2026-08-29: dev PRESENT and column already uuid, stability-test absent and column still text, prod and judge carry no platform_catalog ledger at all. 0032 re-expresses the conversion additively and resolves identity by JOINing tenant_registry_mirror -- a runtime-populated relation fed by node_projection_tenant_registry from onex.tenant.events -- instead of inlining any literal. No slug appears in its transform expression. It is a no-op on the dev lane (column already uuid) and fail-closed everywhere else: an unresolvable slug aborts, naming the slugs and stating that the projection has not caught up. Operator ruling 2026-08-29, verbatim: 'Hold + fix mechanism'. BOTH 0031 and 0032 remain in the OMN-15349 fence baseline until the operator un-gates. diff --git a/tests/fixtures/omn17320/replay-denylist.json b/tests/fixtures/omn17320/replay-denylist.json new file mode 100644 index 0000000000..b7774d721e --- /dev/null +++ b/tests/fixtures/omn17320/replay-denylist.json @@ -0,0 +1,24 @@ +{ + "schema": "onex.exposed-identifier-denylist/1", + "ticket": "OMN-17320", + "salt": "onex-exposed-identifier-v1:", + "annotation_class": "exposed-identifier", + "plaintext_source": "Replay-only denylist. Its single entry is the SAME-LENGTH stand-in substituted into the captured artifact beside it, not a real identifier.", + "readme": [ + "This exists so the OMN-17320 incident replay can be non-vacuous without", + "committing the real tenant slug into a PUBLIC repo -- which is the exact", + "disclosure the guard under test prevents.", + "See tests/incident_replays/registry.yaml case omn17320-slug-reintroduced-with-every-gate-green", + "for the redaction record and the pre-redaction sha256." + ], + "entries": [ + { + "id": "omn17320-replay-standin", + "kind": "tenant-slug", + "length": 11, + "sha256": "7d6f5bf2792bf9ef0fc9b757b5323e0b893fe8a0ab48c2e422a1c5a70cb0e008", + "ticket": "OMN-17320", + "notes": "Same-length stand-in occupying the real slug's byte span in the captured artifact." + } + ] +} diff --git a/tests/incident_replays/registry.yaml b/tests/incident_replays/registry.yaml index b32c16e6a6..fcb1543a00 100644 --- a/tests/incident_replays/registry.yaml +++ b/tests/incident_replays/registry.yaml @@ -156,6 +156,22 @@ scope: - scripts/validation/validate_test_root_collection.py - scripts/verify-merge-queue-no-bypass.sh cases: + - id: omn17320-slug-reintroduced-with-every-gate-green + guard: scripts/validation/check_exposed_identifiers.py + incident: OMN-17288 + regression_class: false_green + guard_verdict_on_artifact: reject + artifact: + fixture: tests/fixtures/omn17320/migration-supersessions.tsv.captured + sha256: b4ec90804b1de237eaa60fca16975616006139d24780c4c0c32b48b181506579 + capture: + method: git-object + source: git-object:OmniNode-ai/omnibase_infra@6527db3f966c03b3ead809425b5381fd71c12d74:docker/migrations/forward/_ledger/migration-supersessions.tsv + captured_at: "2026-08-31T12:05:00Z" + captured_by: lane replay (OMN-17320) + test: tests/ci/test_incident_replay_omn17320.py::test_the_real_guard_rejects_the_captured_artifact + why: >- + This guard is newly wired enforcement, so default-deny demands a real regression case rather than a baseline exemption. The false_green being replaced is the ABSENCE of any guard at all: OMN-17288 scrubbed a live tenant slug out of five files in this repo (#3062, 3f10ee5e), and three hours later omnimarket#2239 reintroduced the same slug there, with the rebase carrying it onto omnimarket#2241 -- the PR whose own acceptance criterion was "zero grep hits" -- while every enforced gate reported green. Nothing in either repo was looking. The artifact is docker/migrations/forward/_ledger/migration-supersessions.tsv exactly as it stood on dev at 6527db3f (= 3f10ee5e^), immediately before the scrub, one of the five files the OMN-17288 census enumerated. ONE REDACTION, recorded here because it is load-bearing: the 11-byte slug on line 7 is replaced by an 11-byte stand-in; every other byte of the 8455 is verbatim and the length is preserved, so every offset in the file is unchanged and the replay still asserts the finding at the slug's real position (line 7, column 379). The pre-redaction sha256 is 9673ad7099caae45d4b3e120bc4d78142321147388cfc6ffba259cb9f2d736ca, so anyone re-fetching the git object can verify byte-for-byte what changed. The redaction is unavoidable rather than convenient: this repo is PUBLIC, and committing the real value as a fixture is precisely the disclosure the guard under test exists to prevent -- an unredacted fixture would fail the very gate it is proving. OMN-17288 set this same precedent in this same registry, redacting a byte range out of tests/fixtures/omn16906/commit-7090f386f.gh-api.json.captured and moving its sha256 pin with the original hash documented. What is genuinely replayed is the part that matters: the guard runs over the REAL TSV field structure, the REAL prose and the REAL token boundaries at the REAL byte offset, not over a line an author imagined -- which is the exact complaint OMN-15547 exists to answer. Direction is false_green, so guard_verdict_on_artifact is reject; an accept control in the same module (test_the_shipped_denylist_does_not_fire_on_this_artifact) requires the SHIPPED denylist to pass the same bytes, so a guard that simply rejects everything cannot satisfy this case. - id: omn15525-health-body-truncation guard: deploy/maintenance/omninode-system-slack-report.sh incident: OMN-15525