diff --git a/CHANGELOG.md b/CHANGELOG.md index 13fc64cc..04117727 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -191,6 +191,19 @@ breaking changes may land in a minor release. ### Changed +- **BREAKING: `probe-adapter` now runs `diagnose`'s egress leak self-check, and captured hook + payloads ship as a schema instead of scrubbed values (#199).** The rendered report re-scans + itself before emitting (the guard moved to `sanitize.guard`, one audited implementation for + both commands): an email / secret / home path / username in the final bytes makes the command + refuse to emit — message on stderr, empty stdout, exit ≠ 0, no `--out` file — and a stray + occurrence of the pseudonymized project directory name is repaired to its alias and disclosed. + Each captured event now reports dotted key paths with leaf types (`tool_input.command:str`), + never payload values, so the `--json` document's `schema_version` bumps to 2 + (`captured_events[].payload` removed, `payload_schema` added; `payload_keys` stays, now + identifier-gated). Collection hardening rides along: transcript-location components that embed + the username are redacted, the project dir name is aliased in locations, a home-rooted + `--binary` hint renders `~`-relative, and a credential-shaped dict key can no longer surface + in the token key paths. - **BREAKING: `diagnose --json` and `probe-adapter --json` now emit a pure JSON document (#195).** Both used to print their human-readable report with a fenced ` ```json ` block appended, so a consumer had to scrape the fence out of prose. `--json` now emits the diff --git a/docs/adapter-authoring-guide.md b/docs/adapter-authoring-guide.md index a96bae8f..a1157deb 100644 --- a/docs/adapter-authoring-guide.md +++ b/docs/adapter-authoring-guide.md @@ -194,15 +194,23 @@ If `tmux` or the binary is missing, probe degrades gracefully to a scan. The report is built to be **safe to paste into an issue or PR**. A single audited sanitizer (`src/bmad_loop/sanitize.py`) is the only chokepoint: -- **numbers, booleans, and `null` pass through** — token _counts_ are not PII; -- **dict keys are kept verbatim** — field names and casing are the whole point of - a payload probe; -- every **leaf string** is `$HOME`→`~` redacted and then kept **only if** it looks - like a short machine identifier (e.g. `claude-opus-4-8`, `session-abc_123`); - anything else — prose, code, paths, emails — becomes ``; -- **list lengths are preserved**, contents are scrubbed element by element; +- **captured hook payloads ship as a schema, never as values** — per event you + get the field names and the dotted key paths with leaf _types_ + (`tool_input.command:str`); no payload value of any kind reaches the report, + and a dynamic or credential-shaped key collapses to ``; +- **numbers, booleans, and `null` pass through** elsewhere — token _counts_ are + not PII; +- **transcript locations are redacted per component** — home → `~`, anything + that isn't a plain machine identifier (or that embeds your username) → + ``, and your project directory name → a salted alias + (`project-3f2a9c…`), the same pseudonymization `diagnose` uses; - `--help` / `--version` text and log tails have the home dir and any emails - redacted, with a line cap. + redacted, with a line cap; +- **the rendered report re-scans itself before emitting** — the same + `sanitize.guard` egress backstop as `diagnose`. A stray occurrence of the + aliased project name is repaired and disclosed; an email / secret / home path / + username in the final bytes makes the command **refuse to emit** (message on + stderr, empty stdout, exit ≠ 0, no `--out` file) rather than ship it. In PROBE mode the raw capture exists **only transiently** inside the temp dir, which is `rmtree`'d in a `finally` (even on exception or Ctrl-C). The CLI's own @@ -247,9 +255,9 @@ bmad-loop probe-adapter --probe --project /tmp/scratch ``` The **Hook payload shape** section now shows, per captured event, the native→ -canonical pairing, the payload keys, and the scrubbed payload — so you can confirm -`session_id` / `transcript_path` casing and that the CLI accepted the hook config -for your dialect. If the CLI rejects the config or never fires a hook, the report +canonical pairing, the payload keys, and the payload **schema** (key paths + leaf +types, never values) — so you can confirm `session_id` / `transcript_path` casing +and that the CLI accepted the hook config for your dialect. If the CLI rejects the config or never fires a hook, the report says so (with a scrubbed log tail) instead of silently producing nothing. ### 4. Write the `usage_parser` diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 701f4c33..0211e2dd 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -1929,6 +1929,7 @@ def cmd_tui(args: argparse.Namespace) -> int: def cmd_probe(args: argparse.Namespace) -> int: from . import probe as probe_mod + from . import sanitize from .adapters.profile import ProfileError, get_profile project = _project(args) @@ -1965,6 +1966,20 @@ def cmd_probe(args: argparse.Namespace) -> int: ) return 1 + # Pseudonymize the one identifier-shaped value the probe KNOWS is the + # user's (diagnose aliases the same value, ns="project"): the project + # directory name, which would otherwise pass the location redaction's + # identifier gate verbatim. Registration is skipped when the name collides + # with a token that legitimately appears in the report (the CLI or binary + # name) — otherwise the guard's repair pass would rewrite every occurrence + # of that legitimate token into the alias and mangle the report. + pseudo = sanitize.Pseudonymizer() + legit_tokens = {args.cli, Path(args.binary).name if args.binary else ""} + if profile is not None: + legit_tokens.add(Path(profile.binary).name) + if project.name not in legit_tokens: + pseudo.alias(project.name, ns="project") + if args.probe: if profile is None: print("FAIL: --probe needs a known profile (its hook dialect/events)", file=sys.stderr) @@ -1976,16 +1991,49 @@ def cmd_probe(args: argparse.Namespace) -> int: hints=hints, timeout_s=args.timeout, keep_temp=args.keep_temp, + pseudo=pseudo, ) else: - finding = probe_mod.scan(cli=args.cli, profile=profile, project=project, hints=hints) + finding = probe_mod.scan( + cli=args.cli, profile=profile, project=project, hints=hints, pseudo=pseudo + ) # One or the other, never both: --json selects the pure JSON document # (machine.py contract), otherwise the human-readable markdown report. - if args.json: - report = probe_mod.render_json(finding) - else: - report = probe_mod.render_markdown(finding) + # Either way the renderer runs the egress self-check over its own rendered + # bytes and raises instead of returning tainted output. + repairs: list[tuple[str, int]] = [] + try: + if args.json: + report = probe_mod.render_json(finding, pseudo=pseudo, repairs=repairs) + else: + report = probe_mod.render_markdown(finding, pseudo=pseudo, repairs=repairs) + except probe_mod.LeakDetected as e: + # Fail closed BEFORE any egress: message → stderr, stdout stays empty, + # no partial --out file, exit != 0 (the machine.py error shape). + print( + f"FAIL: refusing to emit — leak self-check fired: {', '.join(e.rules)}", + file=sys.stderr, + ) + print( + "hint: sensitive[project:] is your project directory name; any " + "other rule means PII/secret-shaped content reached the report — please " + "report the rule names above as a bmad-loop bug", + file=sys.stderr, + ) + return 1 + + if repairs: + merged: dict[str, int] = {} + for label, count in repairs: + merged[label] = merged.get(label, 0) + count + summary = ", ".join(f"{label} x{count}" for label, count in sorted(merged.items())) + print( + f"warning: leak backstop pseudonymized {sum(merged.values())} stray " + f"occurrence(s) ({summary}) — a per-field routing gap in bmad-loop; " + "please include this line in your bug report", + file=sys.stderr, + ) # Every `ok:` trailer is human-facing chatter, so in JSON mode it goes to # stderr — stdout is the document alone, or empty when --out took it. diff --git a/src/bmad_loop/diagnostics.py b/src/bmad_loop/diagnostics.py index fec42913..6fc51647 100644 --- a/src/bmad_loop/diagnostics.py +++ b/src/bmad_loop/diagnostics.py @@ -17,7 +17,8 @@ **pseudonymized** (story keys/branches/SHAs are identifier-shaped and would otherwise survive verbatim), or scrubbed. Unknown/future fields default to a ``scrub_json`` pass, never raw. As a final backstop the rendered bytes are run -through :func:`sanitize.assert_no_leak`. A stray pseudonymized original (a +through :func:`sanitize.guard` — the fail-closed egress self-check shared with +``probe-adapter`` since #199. A stray pseudonymized original (a per-field routing gap — the value is in the legend, so its safe alias is known) is **repaired** by substituting the alias, re-verified, and disclosed in the dump itself — a "Backstop repairs" section in the markdown report, an optional @@ -43,6 +44,14 @@ from .journal import Journal, load_state from .model import RunState, StoryTask +# The guard machinery (fail-closed egress self-check + alias-substitution +# repair) moved to sanitize.py so probe-adapter shares the single audited +# implementation (#199). LeakDetected stays importable from here — cli.py and +# external callers resolve `diagnostics.LeakDetected` — so it carries a noqa: +# without it ruff F401 autofixes the re-export away and the except clause in +# cmd_diagnose breaks. +from .sanitize import LeakDetected # noqa: F401 — re-export + # Deliberately NOT bumped when `--json` stopped being a fenced ```json block # inside the markdown report and became a pure document (machine.py contract): # this number versions the *document*, and the payload did not change. Bumping it @@ -525,7 +534,7 @@ def render_json( # real non-ASCII is safe — and it must be identical in both dumps below, or # the bytes we verified would not be the bytes we emit. rendered = json.dumps(_to_jsonable(d), indent=2, sort_keys=True, ensure_ascii=False) - rendered, reps = _guard(rendered, pseudo) + rendered, reps = sanitize.guard(rendered, pseudo) if reps: # Disclose the repair in the dump itself so the routing gap surfaces as # a reportable bug. Substitution preserved JSON validity — a leaked @@ -535,7 +544,7 @@ def render_json( data = json.loads(rendered) data["backstop_repairs"] = dict(reps) rendered = json.dumps(data, indent=2, sort_keys=True, ensure_ascii=False) - _assert_clean(rendered, pseudo) + sanitize.assert_clean(rendered, pseudo) if repairs is not None: repairs.extend(reps) return rendered @@ -637,7 +646,7 @@ def render_markdown( out.append("") rendered = "\n".join(out) - rendered, reps = _guard(rendered, pseudo) + rendered, reps = sanitize.guard(rendered, pseudo) if reps: note = [ "", @@ -654,7 +663,7 @@ def render_markdown( rendered += "\n".join(note) # The note is appended after the repair loop verified the body, so # re-check the whole thing: the note must sit inside the verified bytes. - _assert_clean(rendered, pseudo) + sanitize.assert_clean(rendered, pseudo) if repairs is not None: repairs.extend(reps) return rendered @@ -664,91 +673,3 @@ def _dict_inline(d: dict) -> str: if not d: return "—" return ", ".join(f"{k}={v}" for k, v in sorted(d.items())) - - -_MAX_REPAIR_PASSES = 3 - - -def _repair_candidates(pseudo: sanitize.Pseudonymizer | None) -> list[tuple[str, str, str]]: - """``(original, alias, label)`` triples for the leak check and repair. - - Filtered to assert_no_leak's ≥4-char detection threshold (repair must never - rewrite an occurrence detection would not fire on) and deduped by original - (a value aliased under two namespaces gets one deterministic label — the - first insertion's). Labels are ``ns:alias`` — safe to print by construction, - never the original.""" - if pseudo is None: - return [] - candidates: list[tuple[str, str, str]] = [] - seen: set[str] = set() - for ns, original, alias in pseudo.entries(): - if len(original) >= 4 and original not in seen: - seen.add(original) - candidates.append((original, alias, f"{ns}:{alias}")) - return candidates - - -def _assert_clean(rendered: str, pseudo: sanitize.Pseudonymizer | None) -> None: - """One plain, no-repair re-check — run after a repair note is appended so - the note itself sits inside the verified bytes.""" - extras = [(orig, label) for orig, _alias, label in _repair_candidates(pseudo)] - fired = sanitize.assert_no_leak(rendered, extra=extras) - if fired: - raise LeakDetected(fired) - - -def _guard( - rendered: str, pseudo: sanitize.Pseudonymizer | None -) -> tuple[str, list[tuple[str, int]]]: - """Verify the rendered bytes; repair stray pseudonymized originals; fail closed. - - When the pseudonymizer is supplied, its legend's original values (the real - story keys/branches/SHAs) are fed into the self-check too, so any that - slipped through the per-field routing are caught here in the final bytes. - Unlike a hard-rule hit, such a miss is repairable: the backstop knows the - original's safe alias, so it substitutes it and re-verifies instead of - refusing outright. Returns ``(text, [(label, count), ...])`` of applied - repairs. Raises :class:`LeakDetected` on any hard rule (email / secret / - home-path / url-creds / username — genuine PII never auto-repairs) or if - repair does not converge within the pass bound.""" - candidates = _repair_candidates(pseudo) - extras = [(orig, label) for orig, _alias, label in candidates] - # Longest-first: a branch embedding a story slug at a "-" boundary must be - # replaced whole, not spliced into a half-alias mongrel by the inner slug. - by_length = sorted(candidates, key=lambda c: len(c[0]), reverse=True) - - tally: dict[str, int] = {} - for _ in range(_MAX_REPAIR_PASSES): - fired = sanitize.assert_no_leak(rendered, extra=extras) - if not fired: - return rendered, sorted(tally.items()) - if any(not rule.startswith("sensitive[") for rule in fired): - raise LeakDetected(fired) - # A repair pass replaces every standalone occurrence of every candidate, - # so pass 2 is reachable only if a substitution manufactured a NEW - # standalone occurrence of a different original — a hash-output - # coincidence (the alias alphabet is [A-Za-z0-9-] and "-" is itself a - # boundary char). The bound turns a pathological substitution cycle - # into a fail-closed refusal instead of a loop. - for original, alias, label in by_length: - rendered, n = sanitize.replace_standalone(rendered, original, alias) - if n: - tally[label] = tally.get(label, 0) + n - fired = sanitize.assert_no_leak(rendered, extra=extras) - if fired: - raise LeakDetected(fired) - return rendered, sorted(tally.items()) - - -class LeakDetected(Exception): - """The rendered dump tripped sanitize.assert_no_leak — emission is refused. - - Raised only for hard rules (email/secret/home-path/url-creds/username) or a - ``sensitive[*]`` repair that did not converge; a plain stray-original hit is - repaired by alias substitution instead. ``rules`` carries the fired rule - names — ``sensitive[:]`` for pseudonymizer originals, printable - because the label never contains the original value.""" - - def __init__(self, rules: list[str]): - self.rules = rules - super().__init__("diagnostic dump tripped leak self-check: " + ", ".join(rules)) diff --git a/src/bmad_loop/machine.py b/src/bmad_loop/machine.py index 0a99442d..c6a8e239 100644 --- a/src/bmad_loop/machine.py +++ b/src/bmad_loop/machine.py @@ -44,11 +44,10 @@ - :func:`emit` takes a ``dict`` and serializes it — ``status`` and ``list``. - :func:`emit_document` takes an already-serialized ``str`` — ``diagnose`` and - ``probe-adapter``, whose renderers return text. Their reasons differ, and only - one is a constraint: ``probe.render_json`` simply *returns a string*, so there - is nothing to re-encode, whereas ``diagnostics.render_json`` serializes - *before* running its leak self-check, making those exact bytes the ones the - check verified — re-encoding them here would emit bytes nothing verified. + ``probe-adapter``, whose renderers return text. For both this is a hard + constraint: each serializes *before* running the shared leak self-check + (``sanitize.guard``), making those exact bytes the ones the check verified — + re-encoding them here would emit bytes nothing verified. :func:`write_document` is the ``--out`` sibling of :func:`emit_document`, taking the same already-serialized string to a file instead of stdout. @@ -56,8 +55,8 @@ The serializer flags differ by family on purpose and are not to be unified: the renderers behind :func:`emit_document` sort their keys (a diff-stable dump is worth more than field order there) while :func:`emit`'s dicts are built in the -order they are meant to be read, and ``diagnostics.render_json`` alone passes -``ensure_ascii=False`` because its leak guard has to scan the values unescaped. +order they are meant to be read, and the two guarded renderers pass +``ensure_ascii=False`` because the leak guard has to scan the values unescaped. """ from __future__ import annotations @@ -77,8 +76,9 @@ def _validated(rendered: str) -> str: """Return ``rendered`` unchanged, or raise if it is not a whole JSON document. Parse to assert, never to re-serialize: the caller's exact bytes are what gets - written. ``diagnostics.render_json`` runs its leak self-check *before* handing - the string over, so re-encoding here would ship bytes nothing checked. + written. The ``diagnose``/``probe-adapter`` renderers run their leak self-check + *before* handing the string over, so re-encoding here would ship bytes nothing + checked. """ try: json.loads(rendered) @@ -93,16 +93,16 @@ def emit_document(rendered: str) -> None: Verifies well-formedness before writing, then writes the ORIGINAL string — never a re-serialization of the parsed result. Half the commands reach stdout through here rather than through :func:`emit`, so without this the contract - would hold for them by convention only; but ``diagnostics.render_json`` - validated these exact bytes with its leak self-check, so emitting anything - re-derived would ship bytes nothing checked. Parse to assert, print verbatim. + would hold for them by convention only; but the guarded renderers validated + these exact bytes with their leak self-check, so emitting anything re-derived + would ship bytes nothing checked. Parse to assert, print verbatim. Raises :class:`ValueError` on a malformed document — a bug in the caller's renderer, and far better surfaced as a crash with empty stdout (which the contract permits) than as a half-parsable stream a consumer has to diagnose. stdout is switched to UTF-8 first, because a document is not necessarily - ASCII — ``diagnostics.render_json`` dumps with ``ensure_ascii=False`` so its + ASCII — the guarded renderers dump with ``ensure_ascii=False`` so the leak guard can scan the values unescaped, which lets a non-sensitive non-ASCII field (a localized ``platform.release()``, say) through to here verbatim. Encoding it for a legacy non-UTF-8 console then raised diff --git a/src/bmad_loop/probe.py b/src/bmad_loop/probe.py index 773351f5..c246df1e 100644 --- a/src/bmad_loop/probe.py +++ b/src/bmad_loop/probe.py @@ -26,14 +26,18 @@ ``schema_version``; do not confuse it with the document's ``version`` key, which holds the *probed CLI's* own ``--version`` output. -Safety model — note how it differs from :mod:`bmad_loop.diagnostics`. Both route -captured data through :mod:`bmad_loop.sanitize` at COLLECTION time, but only -diagnostics adds an egress self-check (``assert_no_leak`` over its own rendered -bytes, refusing to write on a hit). This module has **no such final backstop**: -its safety rests entirely on the collection-time scrubbing being complete, so a -new field that forgets to scrub would ship. That gap is real and tracked in -https://github.com/bmad-code-org/bmad-loop/issues/199 — deliberately not fixed -here, since a guard needs the pseudonymizer/repair machinery to be useful. +Safety model — the same two layers as :mod:`bmad_loop.diagnostics` (closed by +#199). Captured data is scrubbed/reduced at COLLECTION time (captured payloads +ship as key-path:type *schema*, never values; paths are per-component redacted +with the project basename routed through a :class:`sanitize.Pseudonymizer` +alias), and both renderers run :func:`sanitize.guard` over their own rendered +bytes before returning — a hard-rule hit (email/secret/home-path/url-creds/ +username) refuses to emit, while a stray occurrence of a registered alias +original is repaired, re-verified, and disclosed. The residual is stated +honestly: identifier-shaped proprietary values the probe cannot know about +(an arbitrary slug in a dynamic key, say) match no hard rule and no registered +extra, so the guard cannot see them — which is why collection reduces payloads +to shape instead of trying to enumerate what might be sensitive. """ from __future__ import annotations @@ -56,12 +60,19 @@ from .adapters.profile import CLIProfile from .install import merge_hooks, relay_registered from .process_host import get_process_host + +# cmd_probe catches `probe.LeakDetected` around the renderers, mirroring +# diagnostics — the noqa keeps ruff's F401 autofix from deleting the re-export. +from .sanitize import LeakDetected # noqa: F401 — re-export from .signals import SignalWatcher from .tokens import _jsonl_entries, read_usage # Version of the `--json` document (machine.py contract). Distinct from the # document's `version` key, which holds the *probed CLI's* `--version` output. -SCHEMA_VERSION = 1 +# v2: `captured_events[].payload` (scrubbed values) was removed in favor of +# `payload_schema` (key paths + leaf types, never values) — a field removal, +# which the additive-only contract says must bump the version (#199). +SCHEMA_VERSION = 2 # Per-parser transcript-location conventions (from tokens.py docstrings). TRANSCRIPT_GLOBS = { @@ -136,8 +147,8 @@ class TokenSchema: class EventCapture: native_event: str canonical_event: str | None - payload_keys: list[str] - payload: dict # scrubbed + payload_keys: list[str] # top-level field names, identifier-gated + payload_schema: list[str] # dotted key paths + leaf TYPES only, never values @dataclass @@ -195,24 +206,48 @@ def run_version_help(binary: str, timeout_s: float = 10) -> FlagFinding: # ------------------------------------------------------ transcript discovery -def _redact_location(path: Path) -> str: - """Redact a path to a paste-safe form: home -> ``~``, and any path component - that isn't a plain machine identifier (e.g. a munged-cwd dir that embeds a - username) -> ````. The session-id filename usually survives.""" +def _redact_location(path: Path, aliases: dict[str, str] | None = None) -> str: + """Redact a path to a paste-safe form: home -> ``~``, any known-sensitive + component (e.g. the project directory name, which is identifier-shaped and + would pass the gate below verbatim) -> its pseudonymizer alias, and any + other component that isn't a plain machine identifier (e.g. a munged-cwd + dir that embeds a username) -> ````. The session-id filename + usually survives.""" def comp(c: str) -> str: - return c if sanitize.looks_like_identifier(c) else "" + if aliases and c in aliases: + return aliases[c] + # The username check closes a hole the identifier gate leaves open: + # a component like `pytest-of-alice` (or a $TMPDIR under /var/folders + # named after the user) is identifier-shaped yet names the user — and + # the egress guard's username hard rule would refuse the whole report. + if not sanitize.looks_like_identifier(c) or sanitize.embeds_current_username(c): + return "" + return c home = Path(os.path.expanduser("~")) try: rel = path.relative_to(home) return "/".join(["~", *(comp(c) for c in rel.parts)]) except ValueError: - parts = [comp(c) for c in path.parts if c not in ("/", "")] - return "/" + "/".join(parts) - - -def _describe_transcript(path: Path, *, glob_pat: str | None, multiple: bool) -> TranscriptFinding: + # ``parts[0]`` is the anchor on an absolute path. A bare root separator + # ("/" on POSIX, "\\" for a rooted path on Windows) is structure, not a + # component: drop it, or the identifier gate turns the separator itself + # into a phantom leading ````. A drive or UNC anchor ("C:\\", + # "\\\\server\\share\\") does carry content, so it stays and is judged. + parts = list(path.parts) + if parts and parts[0] in ("/", "\\"): + parts.pop(0) + return "/" + "/".join(comp(c) for c in parts if c) + + +def _describe_transcript( + path: Path, + *, + glob_pat: str | None, + multiple: bool, + aliases: dict[str, str] | None = None, +) -> TranscriptFinding: try: stat = path.stat() size = stat.st_size @@ -227,7 +262,7 @@ def _describe_transcript(path: Path, *, glob_pat: str | None, multiple: bool) -> pass return TranscriptFinding( glob=glob_pat, - location=_redact_location(path), + location=_redact_location(path, aliases), fmt="jsonl" if path.suffix == ".jsonl" else (path.suffix.lstrip(".") or "unknown"), size_bytes=size, line_count=line_count, @@ -246,20 +281,23 @@ def discover_transcript( *, cli: str, hints: Hints, + aliases: dict[str, str] | None = None, ) -> TranscriptFinding | None: """Locate the newest existing transcript via override or convention glob.""" if hints.transcript: path = Path(hints.transcript).expanduser() if not path.is_file(): return TranscriptFinding(note=f"--transcript path does not exist: {path.name}") - return _describe_transcript(path, glob_pat=None, multiple=False) + return _describe_transcript(path, glob_pat=None, multiple=False, aliases=aliases) if hints.session_dir: base = Path(hints.session_dir).expanduser() matches = sorted(base.glob("**/*.jsonl")) or sorted(base.glob("**/*.json")) if not matches: return TranscriptFinding(note=f"no *.jsonl/*.json under --session-dir {base.name}") - return _describe_transcript(_newest(matches), glob_pat=None, multiple=len(matches) > 1) + return _describe_transcript( + _newest(matches), glob_pat=None, multiple=len(matches) > 1, aliases=aliases + ) pattern = TRANSCRIPT_GLOBS.get(parser) or FAMILY_GLOBS.get(cli) if not pattern: @@ -275,7 +313,9 @@ def discover_transcript( note="no existing transcript matched the convention glob; " "use --transcript / --session-dir, or run --probe", ) - return _describe_transcript(_newest(matches), glob_pat=pattern, multiple=len(matches) > 1) + return _describe_transcript( + _newest(matches), glob_pat=pattern, multiple=len(matches) > 1, aliases=aliases + ) # ---------------------------------------------------------- schema inference @@ -302,10 +342,15 @@ def _walk_paths(obj, prefix: str, out: set[str]) -> None: A dict key that isn't a plain identifier (e.g. a transcript that keys by relative file path or a per-file backup id) is collapsed to ```` — static field names (the ones a parser keys on, like ``input_tokens``) survive - untouched, but dynamic keys can't leak paths/content into the summary.""" + untouched, but dynamic keys can't leak paths/content into the summary. A + credential-shaped key (``ghp_…`` as a map key) is identifier-shaped yet must + not ship, so it collapses too — the same hole :func:`sanitize.scrub_json` + closes for values.""" if isinstance(obj, dict): for key, value in obj.items(): - key = str(key) if sanitize.looks_like_identifier(str(key)) else "" + key = str(key) + if not sanitize.looks_like_identifier(key) or sanitize.looks_like_secret(key): + key = "" child = f"{prefix}.{key}" if prefix else key _walk_paths(value, child, out) elif isinstance(obj, list): @@ -382,14 +427,21 @@ def scan( profile: CLIProfile | None, project: Path, hints: Hints, + pseudo: sanitize.Pseudonymizer | None = None, ) -> ProfileFinding: + # Aliases registered up front (the project basename) are routed at + # collection time so a normal run needs zero egress repairs. + aliases = {orig: alias for _ns, orig, alias in pseudo.entries()} if pseudo else None binary = hints.binary or (profile.binary if profile else cli) parser = profile.usage_parser if profile else "none" finding = ProfileFinding( cli=cli, mode="scan", known_profile=profile is not None, - binary=binary, + # The finding is render-only; a --binary hint may be an absolute path + # under $HOME, which must not reach the report (the raw local keeps + # driving which/--version below). + binary=sanitize.redact_home(binary), parser=parser, dialect=profile.hooks.dialect if profile else None, declared_events=dict(profile.hooks.events) if profile else {}, @@ -397,8 +449,10 @@ def scan( finding.flags = run_version_help(binary) if not finding.flags.found: + # finding.binary, not the raw local: a home-rooted --binary hint in a + # rendered warning would trip the egress guard's home-path rule. finding.warnings.append( - f"binary {binary!r} not found on PATH — version/help unavailable " + f"binary {finding.binary!r} not found on PATH — version/help unavailable " "(scan continues from on-disk conventions)" ) @@ -411,7 +465,7 @@ def scan( "or re-run with --probe" ) - finding.transcript = discover_transcript(parser, cli=cli, hints=hints) + finding.transcript = discover_transcript(parser, cli=cli, hints=hints, aliases=aliases) if finding.transcript and finding.transcript.note: finding.warnings.append(finding.transcript.note) if finding.transcript and finding.transcript.real_path is not None: @@ -491,6 +545,12 @@ def _captured_transcript_path(capture_dir: Path) -> Path | None: def _collect_captures(capture_dir: Path, events_map: dict[str, str]) -> list[EventCapture]: + """Reduce each raw captured payload to its SHAPE: top-level keys and dotted + key-path:type paths. The payload's diagnostic value for profile authoring is + where the fields live and what type they are — so no payload *value* of any + kind ships, which removes the widest identifier-shaped egress surface + outright (#199). The walk sees the raw dict so leaf types are faithful; + dynamic (non-identifier) keys collapse to ```` in both projections.""" captures: list[EventCapture] = [] for payload_file in sorted(capture_dir.glob("*.payload.json")): import json @@ -502,12 +562,16 @@ def _collect_captures(capture_dir: Path, events_map: dict[str, str]) -> list[Eve if not isinstance(raw, dict): continue native = str(raw.pop("argv_event", "Unknown")) + paths: set[str] = set() + _walk_paths(raw, "", paths) captures.append( EventCapture( native_event=native, canonical_event=events_map.get(native), - payload_keys=sorted(raw.keys()), - payload=sanitize.scrub_event_payload(raw), + payload_keys=sorted( + str(k) if sanitize.looks_like_identifier(str(k)) else "" for k in raw + ), + payload_schema=sorted(paths), ) ) return captures @@ -521,15 +585,18 @@ def probe( hints: Hints, timeout_s: float = 90, keep_temp: bool = False, + pseudo: sanitize.Pseudonymizer | None = None, ) -> ProfileFinding: import json + aliases = {orig: alias for _ns, orig, alias in pseudo.entries()} if pseudo else None binary = hints.binary or profile.binary finding = ProfileFinding( cli=cli, mode="probe", known_profile=True, - binary=binary, + # render-only; see the identical redaction in scan() + binary=sanitize.redact_home(binary), parser=profile.usage_parser, dialect=profile.hooks.dialect, declared_events=dict(profile.hooks.events), @@ -547,9 +614,10 @@ def probe( except Exception: # noqa: BLE001 — a raising host probe means "cannot probe", not a crash mux_ready = False if not mux_ready or not shutil.which(binary): - missing = f"multiplexer backend {type(mux).__name__}" if not mux_ready else binary + # finding.binary, not the raw local — see the identical note in scan() + missing = f"multiplexer backend {type(mux).__name__}" if not mux_ready else finding.binary finding.warnings.append(f"{missing} not on PATH — cannot probe; falling back to scan") - scanned = scan(cli=cli, profile=profile, project=project, hints=hints) + scanned = scan(cli=cli, profile=profile, project=project, hints=hints, pseudo=pseudo) scanned.mode = "probe" return scanned @@ -644,9 +712,13 @@ def probe( # session. Falls back to the glob when the CLI reports no path. live = _captured_transcript_path(capture_dir) if live is not None and live.is_file(): - finding.transcript = _describe_transcript(live, glob_pat=None, multiple=False) + finding.transcript = _describe_transcript( + live, glob_pat=None, multiple=False, aliases=aliases + ) else: - finding.transcript = discover_transcript(profile.usage_parser, cli=cli, hints=hints) + finding.transcript = discover_transcript( + profile.usage_parser, cli=cli, hints=hints, aliases=aliases + ) if finding.transcript and finding.transcript.note: finding.warnings.append(finding.transcript.note) if finding.transcript and finding.transcript.real_path is not None: @@ -655,9 +727,11 @@ def probe( finally: launcher.kill() if keep_temp: + # ~-relative so a $TMPDIR under $HOME can't trip the egress guard; + # the shell expands ~ so the printed path stays inspectable. finding.warnings.append( - f"--keep-temp: RAW probe data retained at {tmpdir} — DO NOT SHARE; " - "delete it after inspection" + f"--keep-temp: RAW probe data retained at {sanitize.redact_home(str(tmpdir))} " + "— DO NOT SHARE; delete it after inspection" ) else: shutil.rmtree(tmpdir, ignore_errors=True) @@ -681,7 +755,12 @@ def _fmt_kv(label: str, value) -> str: return f"- **{label}:** {value}" -def render_markdown(f: ProfileFinding) -> str: +def render_markdown( + f: ProfileFinding, + *, + pseudo: sanitize.Pseudonymizer | None = None, + repairs: list[tuple[str, int]] | None = None, +) -> str: out: list[str] = [] out.append(f"# Profile finalize report — {f.cli} ({f.mode})") out.append("") @@ -732,7 +811,11 @@ def render_markdown(f: ProfileFinding) -> str: out.append( _fmt_kv("payload keys", ", ".join(f"`{k}`" for k in ev.payload_keys) or "—") ) - out.append("\n```json\n" + _json_dump(ev.payload) + "\n```") + out.append("\n**Payload schema** (key paths + leaf types, never values):") + if ev.payload_schema: + out.append("\n```\n" + "\n".join(ev.payload_schema) + "\n```") + else: + out.append("\n- _empty payload._") else: out.append("_no hook payloads captured (see warnings)._") out.append("") @@ -786,16 +869,37 @@ def render_markdown(f: ProfileFinding) -> str: for s in f.next_steps: out.append(f"- → {s}") out.append("") - return "\n".join(out) - - -def _json_dump(obj) -> str: - import json - - return json.dumps(obj, indent=2, sort_keys=True) - -def render_json(f: ProfileFinding) -> str: + rendered = "\n".join(out) + rendered, reps = sanitize.guard(rendered, pseudo) + if reps: + note = [ + "", + "### Backstop repairs", + "", + "_The leak self-check caught stray occurrences of pseudonymized " + "identifiers that the per-field routing missed, and substituted " + "their aliases — a bmad-loop routing gap; please report it._", + "", + ] + for label, count in reps: + note.append(f"- `{label}`: {count} stray occurrence(s) pseudonymized") + note.append("") + rendered += "\n".join(note) + # The note is appended after the repair loop verified the body, so + # re-check the whole thing: the note must sit inside the verified bytes. + sanitize.assert_clean(rendered, pseudo) + if repairs is not None: + repairs.extend(reps) + return rendered + + +def render_json( + f: ProfileFinding, + *, + pseudo: sanitize.Pseudonymizer | None = None, + repairs: list[tuple[str, int]] | None = None, +) -> str: import json def transcript_dict(t: TranscriptFinding | None): @@ -830,7 +934,7 @@ def transcript_dict(t: TranscriptFinding | None): "native_event": ev.native_event, "canonical_event": ev.canonical_event, "payload_keys": ev.payload_keys, - "payload": ev.payload, + "payload_schema": ev.payload_schema, } for ev in f.captured_events ], @@ -851,4 +955,23 @@ def transcript_dict(t: TranscriptFinding | None): } # sort_keys so two probes of the same CLI diff cleanly — the document has # consumers now, and dict-literal order is an implementation detail. - return json.dumps(data, indent=2, sort_keys=True) + # ensure_ascii=False is a SAFETY requirement, not cosmetics (see + # diagnostics.render_json): with the default, a non-ASCII sensitive value + # reaches the guard as \uXXXX escapes and matches nothing, yet json.loads + # hands the consumer back the original. machine.emit/write_document emit + # this string verbatim, so the guarded bytes ARE the emitted bytes. + rendered = json.dumps(data, indent=2, sort_keys=True, ensure_ascii=False) + rendered, reps = sanitize.guard(rendered, pseudo) + if reps: + # Disclose the repair in the document itself so the routing gap surfaces + # as a reportable bug. Substitution preserved JSON validity — a leaked + # original is identifier-shaped and its alias is [A-Za-z0-9-], neither + # side carries quotes or backslashes — so reload-and-extend is safe. + # backstop_repairs is an optional additive key: absent on a clean report. + loaded = json.loads(rendered) + loaded["backstop_repairs"] = dict(reps) + rendered = json.dumps(loaded, indent=2, sort_keys=True, ensure_ascii=False) + sanitize.assert_clean(rendered, pseudo) + if repairs is not None: + repairs.extend(reps) + return rendered diff --git a/src/bmad_loop/sanitize.py b/src/bmad_loop/sanitize.py index 14895725..aceab0c6 100644 --- a/src/bmad_loop/sanitize.py +++ b/src/bmad_loop/sanitize.py @@ -8,11 +8,12 @@ Guarantees: - token *counts* are non-PII, so numbers/bools/null pass through verbatim; -- dict **keys** are kept verbatim — field names/casing are the whole point of a - payload probe — but every leaf **string** is `$HOME`-redacted and then kept - ONLY if it matches a conservative identifier shape (a short slug with no - spaces / `@` / `/`, e.g. ``claude-opus-4-8`` or ``session-abc_123``); - anything else (prose, code, paths, emails) becomes ````; +- dict **keys** get the same scrub as leaf strings (a home-path or + credential-shaped key can't leak where the equivalent value would be caught); + every leaf **string** is `$HOME`-redacted and then kept ONLY if it matches a + conservative identifier shape (a short slug with no spaces / `@` / `/`, e.g. + ``claude-opus-4-8`` or ``session-abc_123``); anything else (prose, code, + paths, emails) becomes ````; - an identifier-shaped string that looks like a **secret** (a known credential prefix such as ``ghp_``/``sk-``/``AKIA``, a JWT, or a long high-entropy blob) becomes ```` even though it would otherwise pass — the one @@ -20,17 +21,18 @@ - list lengths are preserved (the count is structural, the contents aren't); - recursion is depth-guarded so a pathological payload can't blow the stack. -It also offers two helpers the dump leans on but the probe does not need: -:class:`Pseudonymizer` (stable, irreversible per-dump aliases for proprietary -identifiers — story keys, branches, SHAs — that *are* identifier-shaped and so -would otherwise survive verbatim) and :func:`assert_no_leak` (a final-output -self-check the dump runs over its own rendered bytes before writing, so a -routing bug or a future field can never silently ship a secret/PII/path). -`assert_no_leak` accepts labeled extras (``(value, label)``) so a hit can be -reported by a printable label instead of an opaque index, and its companion -:func:`replace_standalone` substitutes a leaked value under identical -word-boundary semantics — the check stays the authority; whether a caller -repairs-and-rechecks or refuses outright is the caller's policy. +It also owns the shared egress backstop both commands run over their own +rendered bytes before emitting: :class:`Pseudonymizer` (stable, irreversible +per-report aliases for proprietary identifiers — story keys, branches, SHAs, +project names — that *are* identifier-shaped and so would otherwise survive +verbatim), :func:`assert_no_leak` (the raw re-scan; accepts labeled extras +``(value, label)`` so a hit is reported by a printable label instead of an +opaque index), and :func:`guard` / :func:`assert_clean` (the fail-closed +policy around it: hard-rule hits — email/secret/home-path/url-creds/username — +refuse outright by raising :class:`LeakDetected`, while a stray pseudonymizer +original is repaired via :func:`replace_standalone` under identical +word-boundary semantics, re-verified, and disclosed to the caller). A routing +bug or a future field can therefore never silently ship a secret/PII/path. """ from __future__ import annotations @@ -312,6 +314,21 @@ def entries(self) -> list[tuple[str, str, str]]: return [(ns, value, alias) for (ns, value), alias in self._map.items()] +def embeds_current_username(s: str) -> bool: + """True when the current username (≥5 chars — the :func:`assert_no_leak` + hard rule's threshold) appears anywhere in ``s``. Collection-time callers + redact such values pre-emptively — e.g. a kept-verbatim path component like + ``pytest-of-alice`` — so the guard's username rule (standalone semantics, + strictly narrower than this substring check) can never fire on them.""" + try: + user = getpass.getuser() + except Exception: + # No passwd entry / no USER env (minimal containers): same degradation + # as the assert_no_leak username rule. + return False + return len(user) >= 5 and user in s + + def assert_no_leak(text: str, *, extra: Iterable[str | tuple[str, str]] = ()) -> list[str]: """Re-scan already-rendered output for anything that must not ship. @@ -361,3 +378,94 @@ def assert_no_leak(text: str, *, extra: Iterable[str | tuple[str, str]] = ()) -> if len(value) >= 4 and _contains_standalone(text, value): fired.append(f"sensitive[{label}]") return fired + + +class LeakDetected(Exception): + """The rendered report tripped :func:`assert_no_leak` — emission is refused. + + Raised only for hard rules (email/secret/home-path/url-creds/username) or a + ``sensitive[*]`` repair that did not converge; a plain stray-original hit is + repaired by alias substitution instead. ``rules`` carries the fired rule + names — ``sensitive[:]`` for pseudonymizer originals, printable + because the label never contains the original value.""" + + def __init__(self, rules: list[str]): + self.rules = rules + super().__init__("rendered report tripped leak self-check: " + ", ".join(rules)) + + +_MAX_REPAIR_PASSES = 3 + + +def _repair_candidates(pseudo: Pseudonymizer | None) -> list[tuple[str, str, str]]: + """``(original, alias, label)`` triples for the leak check and repair. + + Filtered to assert_no_leak's ≥4-char detection threshold (repair must never + rewrite an occurrence detection would not fire on) and deduped by original + (a value aliased under two namespaces gets one deterministic label — the + first insertion's). Labels are ``ns:alias`` — safe to print by construction, + never the original.""" + if pseudo is None: + return [] + candidates: list[tuple[str, str, str]] = [] + seen: set[str] = set() + for ns, original, alias in pseudo.entries(): + if len(original) >= 4 and original not in seen: + seen.add(original) + candidates.append((original, alias, f"{ns}:{alias}")) + return candidates + + +def assert_clean(rendered: str, pseudo: Pseudonymizer | None = None) -> None: + """One plain, no-repair re-check — run after a repair note is appended so + the note itself sits inside the verified bytes.""" + extras = [(orig, label) for orig, _alias, label in _repair_candidates(pseudo)] + fired = assert_no_leak(rendered, extra=extras) + if fired: + raise LeakDetected(fired) + + +def guard( + rendered: str, + pseudo: Pseudonymizer | None = None, + *, + max_passes: int = _MAX_REPAIR_PASSES, +) -> tuple[str, list[tuple[str, int]]]: + """Verify the rendered bytes; repair stray pseudonymized originals; fail closed. + + When the pseudonymizer is supplied, its legend's original values (the real + story keys/branches/project names) are fed into the self-check too, so any + that slipped through the per-field routing are caught here in the final + bytes. Unlike a hard-rule hit, such a miss is repairable: the backstop knows + the original's safe alias, so it substitutes it and re-verifies instead of + refusing outright. Returns ``(text, [(label, count), ...])`` of applied + repairs. Raises :class:`LeakDetected` on any hard rule (email / secret / + home-path / url-creds / username — genuine PII never auto-repairs) or if + repair does not converge within the pass bound.""" + candidates = _repair_candidates(pseudo) + extras = [(orig, label) for orig, _alias, label in candidates] + # Longest-first: a branch embedding a story slug at a "-" boundary must be + # replaced whole, not spliced into a half-alias mongrel by the inner slug. + by_length = sorted(candidates, key=lambda c: len(c[0]), reverse=True) + + tally: dict[str, int] = {} + for _ in range(max_passes): + fired = assert_no_leak(rendered, extra=extras) + if not fired: + return rendered, sorted(tally.items()) + if any(not rule.startswith("sensitive[") for rule in fired): + raise LeakDetected(fired) + # A repair pass replaces every standalone occurrence of every candidate, + # so pass 2 is reachable only if a substitution manufactured a NEW + # standalone occurrence of a different original — a hash-output + # coincidence (the alias alphabet is [A-Za-z0-9-] and "-" is itself a + # boundary char). The bound turns a pathological substitution cycle + # into a fail-closed refusal instead of a loop. + for original, alias, label in by_length: + rendered, n = replace_standalone(rendered, original, alias) + if n: + tally[label] = tally.get(label, 0) + n + fired = assert_no_leak(rendered, extra=extras) + if fired: + raise LeakDetected(fired) + return rendered, sorted(tally.items()) diff --git a/tests/test_diagnostics.py b/tests/test_diagnostics.py index c84edc0a..92640120 100644 --- a/tests/test_diagnostics.py +++ b/tests/test_diagnostics.py @@ -316,19 +316,11 @@ def test_no_repairs_on_fully_routed_run(project): assert "backstop_repairs" not in js -def test_guard_hard_rules_still_fail_closed(): - pseudo = sanitize.Pseudonymizer() - with pytest.raises(diagnostics.LeakDetected) as exc: - diagnostics._guard("contact victim.canary@example.com", pseudo) - assert "email" in exc.value.rules - # a hard rule alongside a repairable one: refuse immediately, and the - # sensitive rule rides along under its printable ns:alias label - key_alias = pseudo.alias(STORY_KEY, ns="story", epic=1) - with pytest.raises(diagnostics.LeakDetected) as exc: - diagnostics._guard(f"{STORY_KEY} contact victim.canary@example.com", pseudo) - assert "email" in exc.value.rules - assert f"sensitive[story:{key_alias}]" in exc.value.rules - assert STORY_KEY not in str(exc.value) +def test_leakdetected_is_the_shared_sanitize_exception(): + """The re-export must stay importable as diagnostics.LeakDetected — cli.py's + except clause resolves it here, and ruff's F401 autofix deletes a bare + re-export (the noqa carries it; this pin catches the regression).""" + assert diagnostics.LeakDetected is sanitize.LeakDetected def test_repair_note_is_inside_verified_bytes(project): @@ -397,18 +389,6 @@ def test_non_ascii_survives_the_utf8_round_trip(tmp_path, monkeypatch): assert json.loads(path.read_text(encoding="utf-8"))["note"] == "café — naïve ✓" -class _CyclicPseudo(sanitize.Pseudonymizer): - """Adversarial stand-in: each alias embeds the OTHER original at a "-" - boundary, so every substitution reintroduces the other value — a cycle a - real Pseudonymizer could only produce by hash-output coincidence.""" - - def entries(self): - return [ - ("story", "alpha-key", "s1-beta-key"), - ("branch", "beta-key", "branch-alpha-key"), - ] - - -def test_repair_loop_bound_terminates_and_fails_closed(): - with pytest.raises(diagnostics.LeakDetected): - diagnostics._guard("ref alpha-key end", _CyclicPseudo()) +# The pure guard-mechanics tests (hard-rule refusal, repair tally, cyclic +# termination) live in tests/test_sanitize.py since #199 made guard shared API; +# this file keeps the integration surface: real collectors, real renders. diff --git a/tests/test_probe.py b/tests/test_probe.py index 61af0b58..8b6f1255 100644 --- a/tests/test_probe.py +++ b/tests/test_probe.py @@ -2,11 +2,13 @@ CLI plumbing, and end-to-end scrub-through. No live CLI required.""" import json +import re import pytest from conftest import machine_json +from test_probe_hook import run_hook -from bmad_loop import cli, probe +from bmad_loop import cli, probe, sanitize from bmad_loop.adapters.profile import get_profile # ----------------------------------------------------------- fixtures / helpers @@ -359,7 +361,7 @@ def test_cli_json_emits_pure_document(tmp_path, capsys): # `!=` between them would be vacuously true (str vs int never compare equal), # so pin the TYPE contract instead — that is what breaks if the two are ever # conflated: schema_version is our int, version is the CLI's own string. - assert data["schema_version"] == probe.SCHEMA_VERSION == 1 + assert data["schema_version"] == probe.SCHEMA_VERSION == 2 assert isinstance(data["schema_version"], int) assert "version" in data and (data["version"] is None or isinstance(data["version"], str)) @@ -410,7 +412,9 @@ def test_cli_json_unknown_profile_notice_goes_to_stderr(tmp_path, capsys): def _seed_pii_scan(tmp_path, monkeypatch): - """A transcript whose rows carry an email and a home-rooted path.""" + """A transcript whose rows carry an email, a home-rooted path, and a + credential-shaped dynamic dict key (identifier-shaped, so only the + explicit secret gate in _walk_paths keeps it out of the key paths).""" monkeypatch.setenv("HOME", str(tmp_path)) rows = [ { @@ -418,6 +422,7 @@ def _seed_pii_scan(tmp_path, monkeypatch): "author": "secret@example.com", "cwd": f"{tmp_path}/private/project", "message": {"usage": {"input_tokens": 7, "output_tokens": 3}}, + "cache": {"ghp_0123456789abcdefghijklmnopqrstuvwxyz": 1}, } ] return _write_jsonl(tmp_path / "t.jsonl", rows) @@ -445,6 +450,10 @@ def test_scan_report_contains_no_pii(tmp_path, capsys, monkeypatch): out = capsys.readouterr().out assert "secret@example.com" not in out assert "private/project" not in out + # the credential-shaped dynamic key collapsed instead of shipping (and + # instead of tripping the guard's secret rule into a refusal — rc was 0) + assert "ghp_" not in out + assert "cache.:int" in out # but the token schema is still there assert "message.usage.input_tokens:int" in out @@ -457,5 +466,227 @@ def test_scan_json_document_contains_no_pii(tmp_path, capsys, monkeypatch): out = capsys.readouterr().out assert "secret@example.com" not in out assert "private/project" not in out + assert "ghp_" not in out doc = json.loads(out) assert "message.usage.input_tokens:int" in doc["tokens"]["key_paths"] + assert "cache.:int" in doc["tokens"]["key_paths"] + + +# ----------------------------------------------------------- egress guard (#199) +# Probe's canary kit mirrors tests/test_diagnostics.py: PROJ is identifier-shaped +# (it passes every scrub gate on shape), so only the pseudonymizer/guard pair +# keeps it out of a shipped report. + +PROJ = "AcmeQuantumBilling" +EMAIL = "victim.canary@example.com" +SECRET_GH = "ghp_0123456789abcdefghijklmnopqrstuvwxyz" +HOME_PATH = "/home/canaryuser/secret/proj" +ALIAS_RE = re.compile(r"project-[0-9a-f]{12}") # blake2s digest_size=6 → 12 hex chars + + +def _dirty_finding(**kw): + defaults = dict(cli="claude", mode="scan", known_profile=True, binary="claude", parser="none") + defaults.update(kw) + return probe.ProfileFinding(**defaults) + + +def test_probe_leakdetected_is_the_shared_sanitize_exception(): + """Same pin as diagnostics': ruff's F401 autofix deletes a bare re-export.""" + assert probe.LeakDetected is sanitize.LeakDetected + + +def test_renderers_fail_closed_on_dirty_finding(): + """A field that skipped collection scrubbing cannot ship: both renderers + re-scan their own rendered bytes and raise instead of returning.""" + f = _dirty_finding(warnings=[f"contact {EMAIL}"]) + with pytest.raises(probe.LeakDetected) as exc: + probe.render_markdown(f) + assert "email" in exc.value.rules + with pytest.raises(probe.LeakDetected) as exc: + probe.render_json(f) + assert "email" in exc.value.rules + + +def test_cli_refusal_text_mode_emits_nothing(tmp_path, capsys, monkeypatch): + """Fail-closed shape: message → stderr, stdout EMPTY, exit != 0.""" + monkeypatch.setattr(probe, "scan", lambda **kw: _dirty_finding(warnings=[f"see {EMAIL}"])) + rc = cli.main(["probe-adapter", "claude", "--project", str(tmp_path)]) + out, err = capsys.readouterr() + assert rc == 1 + assert out == "" + assert "FAIL: refusing to emit" in err and "email" in err + + +def test_cli_refusal_json_out_writes_no_partial_file(tmp_path, capsys, monkeypatch): + """#195 pure-document contract on refusal: empty stdout AND no --out file.""" + monkeypatch.setattr(probe, "scan", lambda **kw: _dirty_finding(warnings=[f"see {EMAIL}"])) + out_file = tmp_path / "report.json" + rc = cli.main( + ["probe-adapter", "claude", "--project", str(tmp_path), "--json", "--out", str(out_file)] + ) + out, err = capsys.readouterr() + assert rc == 1 + assert out == "" + assert "FAIL: refusing to emit" in err + assert not out_file.exists() + + +def test_collect_captures_emits_schema_not_values(tmp_path): + """The capture reduction ships SHAPE only — no payload value of any kind.""" + capture = tmp_path / "capture" + capture.mkdir() + payload = { + "argv_event": "Stop", + "session_id": "abc-123", + "transcript_path": f"{HOME_PATH}/t.jsonl", + "prompt": "the launch codes are 0000", + "api_key": SECRET_GH, + "tool_input": {"command": "cat /home/canaryuser/.ssh/id_rsa"}, + "weird key with spaces": 1, + "counts": [1, 2, 3], + } + (capture / "001-Stop.payload.json").write_text(json.dumps(payload), encoding="utf-8") + caps = probe._collect_captures(capture, {"Stop": "stop"}) + assert len(caps) == 1 + ev = caps[0] + assert not hasattr(ev, "payload") # the value-bearing field is GONE, not empty + assert ev.native_event == "Stop" and ev.canonical_event == "stop" + assert "session_id" in ev.payload_keys and "" in ev.payload_keys + assert "session_id:str" in ev.payload_schema + assert "tool_input.command:str" in ev.payload_schema + assert "api_key:str" in ev.payload_schema + assert "counts[]:int" in ev.payload_schema + joined = "\n".join(ev.payload_schema + ev.payload_keys) + for canary in ("canaryuser", "launch codes", "ghp_", "abc-123", "id_rsa"): + assert canary not in joined + + +def test_probe_capture_egress_via_hook_seam(tmp_path): + """End-to-end over the real capture seam: the hook subprocess retains the raw + payload ("the command scrubs later" — test_probe_hook), so THIS is the test + that the command actually does, all the way to the rendered egress bytes.""" + capture = tmp_path / "capture" + env = {"BMAD_LOOP_PROBE_CAPTURE_DIR": str(capture), "BMAD_LOOP_TASK_ID": "probe"} + payload = { + "session_id": "sess-canary-1", + "transcript_path": f"{HOME_PATH}/t.jsonl", + "prompt": f"proprietary {PROJ} launch plan, contact {EMAIL}", + "token": SECRET_GH, + } + proc = run_hook("Stop", env, payload) + assert proc.returncode == 0 + + caps = probe._collect_captures(capture, {"Stop": "stop"}) + f = _dirty_finding(mode="probe", parser="claude-jsonl", captured_events=caps) + md = probe.render_markdown(f) + js = probe.render_json(f) + combined = md + "\n" + js + for canary in (PROJ, EMAIL, SECRET_GH, "canaryuser", "sess-canary-1", "launch plan"): + assert canary not in combined + # the shape that makes the report useful survived + assert "prompt:str" in combined and "token:str" in combined + assert json.loads(js)["captured_events"][0]["payload_schema"] + + +def test_location_aliases_project_dir_zero_repairs(tmp_path, capsys): + """The one identifier-shaped value probe KNOWS is the user's — the project + dir name — is aliased at collection time, so the canonical run needs ZERO + egress repairs (probe's analogue of diagnostics' zero-repair invariant).""" + project = tmp_path / PROJ + transcript = _write_jsonl(project / "transcripts" / "t.jsonl", CLAUDE_ROWS) + rc = cli.main( + ["probe-adapter", "claude", "--project", str(project), "--transcript", str(transcript)] + ) + out, err = capsys.readouterr() + assert rc == 0 + assert PROJ not in out and PROJ not in err + assert ALIAS_RE.search(out) # the location line carries the alias instead + assert "leak backstop" not in err # collection routed it — no repair needed + assert "Backstop repairs" not in out + + +def test_backstop_repairs_stray_project_name_in_key_paths(tmp_path, capsys): + """A surface collection can't route — the project name as an identifier-shaped + dynamic dict key survives _walk_paths into key_paths — is caught at egress, + repaired to the alias, and disclosed in the document + on stderr.""" + project = tmp_path / PROJ + project.mkdir() + rows = [{"message": {"usage": {"input_tokens": 7, PROJ: 7}}}] + transcript = _write_jsonl(tmp_path / "t.jsonl", rows) + data = machine_json( + [ + "probe-adapter", + "claude", + "--project", + str(project), + "--transcript", + str(transcript), + "--json", + ], + capsys, + err_contains="leak backstop pseudonymized", + ) + assert PROJ not in json.dumps(data) + (label,) = data["backstop_repairs"] + assert re.fullmatch(r"project:project-[0-9a-f]{12}", label) + alias = label.split(":", 1)[1] + assert f"message.usage.{alias}:int" in data["tokens"]["key_paths"] + + +def test_render_json_non_ascii_roundtrip_and_guard(): + """ensure_ascii=False is a safety requirement: the guard must see the string + as itself. Clean non-ASCII round-trips unescaped; a registered non-ASCII + sensitive value is caught and repaired in the exact emitted bytes.""" + f = _dirty_finding(warnings=["café — naïve ✓"]) + js = probe.render_json(f) + assert json.loads(js)["warnings"] == ["café — naïve ✓"] + assert "caf\\u00e9" not in js # emitted unescaped, not as a \uXXXX escape + + pseudo = sanitize.Pseudonymizer() + alias = pseudo.alias("café-user", ns="project") + f2 = _dirty_finding(warnings=["ref café-user end"]) + js2 = probe.render_json(f2, pseudo=pseudo) + assert "café-user" not in js2 + assert json.loads(js2)["backstop_repairs"] == {f"project:{alias}": 1} + + +def test_scan_redacts_home_rooted_binary_hint(tmp_path, monkeypatch): + """--binary under $HOME must not ship an absolute home path (it previously + rendered verbatim in the summary AND the not-found warning).""" + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.setenv("USERPROFILE", str(tmp_path)) + hints = probe.Hints(binary=f"{tmp_path}/.local/bin/claude-x") + finding = probe.scan(cli="no-such-cli", profile=None, project=tmp_path, hints=hints) + assert finding.binary == "~/.local/bin/claude-x" + md = probe.render_markdown(finding) # and the guard agrees: no refusal + assert str(tmp_path) not in md + + +def test_redact_location_strikes_username_components(monkeypatch): + """An identifier-shaped component that embeds the current username (pytest's + own tmp roots are the live example: pytest-of-) is redacted, so the + guard's username hard rule can't refuse the whole report over a path.""" + monkeypatch.setattr(sanitize.getpass, "getuser", lambda: "canaryuser") + from pathlib import Path + + loc = probe._redact_location(Path("/var/tmp/build-of-canaryuser/session-1/t.jsonl")) + assert "canaryuser" not in loc + assert loc == "/var/tmp//session-1/t.jsonl" + + +def test_redact_location_drops_the_root_anchor_on_every_flavour(monkeypatch): + """The root separator is structure, not a component. Windows spells it + ``\\`` rather than ``/``, and letting it reach the identifier gate prepended + a phantom ```` to every non-home absolute path (a Windows-only CI + failure). Driven through both flavours so either platform catches it.""" + monkeypatch.setattr(sanitize.getpass, "getuser", lambda: "canaryuser") + from pathlib import PureWindowsPath + + # A rooted path carries no drive: the anchor vanishes on both flavours. + assert probe._redact_location(PureWindowsPath("/var/tmp/s-1/t.jsonl")) == ( + "/var/tmp/s-1/t.jsonl" + ) + # A drive anchor is content, not structure, so it is judged and struck. + assert probe._redact_location(PureWindowsPath("C:/build/s-1/t.jsonl")) == ( + "//build/s-1/t.jsonl" + ) diff --git a/tests/test_sanitize.py b/tests/test_sanitize.py index ac8e7b77..d865832d 100644 --- a/tests/test_sanitize.py +++ b/tests/test_sanitize.py @@ -318,3 +318,84 @@ def test_pseudonymizer_entries_expose_ns(): ] # legend keeps its shape: alias -> original, ns discarded assert p.legend() == {a_story: "1.2-secret", a_branch: "feat/secret"} + + +# ------------------------------------------------- guard / assert_clean +# The fail-closed egress policy shared by diagnose and probe-adapter (#199). +# STORY_KEY embeds a proprietary product name as a substring, so "the original +# never appears in the exception" is asserted against the nastiest shape. + +STORY_KEY = "1.2-AcmeQuantumBillingEngine" + + +def test_guard_clean_text_is_returned_verbatim(): + pseudo = sanitize.Pseudonymizer() + pseudo.alias(STORY_KEY, ns="story", epic=1) + assert sanitize.guard("nothing sensitive here", pseudo) == ("nothing sensitive here", []) + # and a missing pseudonymizer still runs the hard rules + with pytest.raises(sanitize.LeakDetected) as exc: + sanitize.guard("contact victim.canary@example.com") + assert exc.value.rules == ["email"] + + +def test_guard_repairs_stray_original_and_tallies(): + pseudo = sanitize.Pseudonymizer() + alias = pseudo.alias(STORY_KEY, ns="story", epic=1) + text = f"path a/{STORY_KEY}/b and again {STORY_KEY}" + out, reps = sanitize.guard(text, pseudo) + assert STORY_KEY not in out + assert out.count(alias) == 2 + assert reps == [(f"story:{alias}", 2)] + # the repaired text passes the same check that fired on the input + assert sanitize.assert_no_leak(out, extra=[STORY_KEY]) == [] + + +def test_guard_hard_rules_never_auto_repair(): + pseudo = sanitize.Pseudonymizer() + with pytest.raises(sanitize.LeakDetected) as exc: + sanitize.guard("contact victim.canary@example.com", pseudo) + assert "email" in exc.value.rules + # a hard rule alongside a repairable one: refuse immediately, and the + # sensitive rule rides along under its printable ns:alias label + key_alias = pseudo.alias(STORY_KEY, ns="story", epic=1) + with pytest.raises(sanitize.LeakDetected) as exc: + sanitize.guard(f"{STORY_KEY} contact victim.canary@example.com", pseudo) + assert "email" in exc.value.rules + assert f"sensitive[story:{key_alias}]" in exc.value.rules + assert STORY_KEY not in str(exc.value) + + +class _CyclicPseudo(sanitize.Pseudonymizer): + """Adversarial stand-in: each alias embeds the OTHER original at a "-" + boundary, so every substitution reintroduces the other value — a cycle a + real Pseudonymizer could only produce by hash-output coincidence.""" + + def entries(self): + return [ + ("story", "alpha-key", "s1-beta-key"), + ("branch", "beta-key", "branch-alpha-key"), + ] + + +def test_guard_repair_bound_terminates_and_fails_closed(): + with pytest.raises(sanitize.LeakDetected): + sanitize.guard("ref alpha-key end", _CyclicPseudo()) + + +def test_assert_clean_raises_and_never_repairs(): + pseudo = sanitize.Pseudonymizer() + alias = pseudo.alias(STORY_KEY, ns="story", epic=1) + with pytest.raises(sanitize.LeakDetected) as exc: + sanitize.assert_clean(f"stray {STORY_KEY} here", pseudo) + assert exc.value.rules == [f"sensitive[story:{alias}]"] + sanitize.assert_clean("all clean", pseudo) # no raise on clean input + + +def test_embeds_current_username(monkeypatch): + monkeypatch.setattr(sanitize.getpass, "getuser", lambda: "alice") + assert sanitize.embeds_current_username("pytest-of-alice") + assert sanitize.embeds_current_username("alice") + assert not sanitize.embeds_current_username("someone-else") + # below the ≥5 threshold shared with the assert_no_leak username rule + monkeypatch.setattr(sanitize.getpass, "getuser", lambda: "bob") + assert not sanitize.embeds_current_username("bob-dir")