From 2352b6016622c2fd3329ffed59ff8dfe22278f71 Mon Sep 17 00:00:00 2001 From: Pengfei Hu Date: Thu, 8 Oct 2026 04:23:59 -0700 Subject: [PATCH] feat(diff): name the changed declarations in an OpenShell policy row (#968) A changed OpenShell policy row printed endpoint counts and a facts digest. It now names the filesystem paths added or removed per list, binary selector changes, and destination or request declarations added, removed or edited, bounded and redacted by the inventory's own rules. A policy selected or removed whole lists what it declares and which defaults stood in. Display only: rows, direction, expands, severity, signals and check decisions are unchanged, and no schema moves. --- CHANGELOG.md | 1 + docs/openshell-support.md | 24 + .../core/capability_diff_rows.py | 26 + .../core/openshell_row_detail.py | 338 ++++++++++++ tests/shard_seconds.json | 1 + tests/test_openshell_row_detail.py | 488 ++++++++++++++++++ 6 files changed, 878 insertions(+) create mode 100644 src/agents_shipgate/core/openshell_row_detail.py create mode 100644 tests/test_openshell_row_detail.py diff --git a/CHANGELOG.md b/CHANGELOG.md index cb5693fe..52bfb82f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Changes +- **An OpenShell policy row names the declarations that changed.** Replaying openshift-kni/ai-sandbox#29, a selected policy that removed `/sandbox/.uv/python` from `filesystem_policy.read_only`, changed binary selectors and added GitHub release destinations printed `authored, OpenShell 0.1.2, 6 endpoint(s), facts → …7 endpoint(s), facts `, which names none of them. `diff` (text and `--json`), `verify`, `check` and the PR comment now print the difference: `filesystem read_only -/sandbox/.uv/python`, `destination aiplatform.googleapis.com:443 (l4) binaries +/tmp/**/node_modules/…/claude -/usr/local/bin/claude`, `removed destination oauth2.googleapis.com:443 (l4) via /usr/local/bin/claude` and `added destinations objects.githubusercontent.com:443, release-assets.githubusercontent.com:443 (l4) via …`, with changed request declarations, `include_workdir`, Landlock compatibility and process identity named the same way. Each list names its first entries and counts the rest. Only values the OpenShell inventory already publishes are named, and a value that would change under redaction prints as ``. Query and parameter constraints and endpoint options appear by field name, not value. A policy selected or removed whole lists what it declares and which sections and inspected endpoints took an upstream default, as a description and not a proof that authority grew. Display only: the published `rows[]`, direction, `expands`, severity, expansion signals and `check` decisions are unchanged, the existing unknown-direction and runtime-limit wording stays in each row's reason, and no schema changes. (#968) - **`diff` classifies a large base tree about eight times faster.** Translating each boundary glob into a regular expression was repeated for every path in the tree and was 74–86% of first-run `diff` time on two large repositories (#698's measurement). The compiled pattern is now cached per pattern; which paths match is unchanged, and a test checks the cached matcher against the previous implementation on over 10,000 random pattern and path pairs. (#698) - **A hook row names a change to its `args` or to a background setting.** Changing only a command hook's exec-form arguments (`command: "python3", args: ["guard-readonly.py"]` → `["guard-write.py"]`), or only `"async": true` to `"asyncRewake": true`, was a `changed` row reading `no difference in the matcher, command or timeout; the change is in a detail this output does not show`. Each handler now publishes its `args` as a script path and a digest (host-grants `0.9`, extended in place): the first argument shaped as a relative script path that no redaction rule rewrites, alone or after the arguments before it, and a digest of the rest, as an MCP server's arguments are published, so no flag, value, absolute path or URL is shown. It also publishes the documented `type`, `async`, `asyncRewake`, `shell` and `once` settings it declares. The rows read `PreToolUse: args script guard-readonly.py → guard-write.py`, `args changed (sha256:… → sha256:…)` and `PostToolUse: async true → (none); asyncRewake (none) → true` on `diff` (text and `--json`), `verify`, the PR comment and `check`, and an added hook's cell lists them. The direction stays unknown, no script effect is inferred, and the fields are left out of grant equality, the digests and saved baselines, so no row, severity, expansion signal, loading basis or `check` decision moves, and rotating a value the digest already redacts is still no row. A change confined to `if`, `statusMessage` or another unpublished field still says it is not shown, and now names them. See [STABILITY.md](STABILITY.md#hook-args-and-settings-972). (#972, #971) - **A hook row describes an inline command's structure, or says the digest moved and the config has to be opened.** Replaying CirrusRedOrg/EntityFrameworkCore.Jet#303, an edited `PreToolUse` script read `command changed ( sha256:f23acba4b10f → sha256:3f1dc36980b7)`: two digests, and no name because the script opens with an assignment. Each handler's `command` now also publishes a `shape` (host-grants `0.9`, extended in place), read by a bounded, static reader that never runs the command: the plain-token programs at its command positions, the counts of commands, pipes, `$( … )` substitutions, `if`/`while`/`for` keywords and quoted strings, the redirects to a file (the target only for `/dev/null` or a repository-relative path), and a script path published as `args` publish one. A command the reader refuses whole (a here-document, a backquote, `case`, a function, an unterminated quote, more than 8,192 characters, or a `shell` other than `bash`) is `shape_limit` instead. The Jet row now reads `…; same programs (cat, printf, sed, grep, echo, true); simple commands 15 → 25; pipes 5 → 9; conditionals and loops 4 → 7; quoted strings 18 → 32)`; an added pipeline stage reads `programs +sh; pipes 1 → 2`, an added redirect `redirects +>> logs/x.log`, a moved script `script a.py → b.py`; an edit that keeps the structure says `same programs and structure; the change is in an argument or in quoted text this output does not show, open the config to read the change`, and an undescribed command `not described: head command uses shell syntax this output does not describe; the digest moved, open the config to read the change`. `diff` (text and `--json`), `verify`, the PR comment and `check` print the same entry. No argument, quoted string, URL, credential or absolute path is published (every shape #987 withholds stays withheld, and its rotations stay quiet), a program name is never inferred to widen or narrow anything, and the shape is left out of grant equality, the digests and saved baselines, so no row, severity, direction, expansion signal or `check` decision moves and the row still says `hook edit; authority direction is unknown`. See [STABILITY.md](STABILITY.md#hook-command-shape-934). (#934) diff --git a/docs/openshell-support.md b/docs/openshell-support.md index 57bc2ea0..7ebbd216 100644 --- a/docs/openshell-support.md +++ b/docs/openshell-support.md @@ -93,6 +93,30 @@ or request-partition cells. Unsupported semantics never become inferred safe narrowing. An HTTP method or MCP tool name still supplies no business effect, approval, argument restriction or deployed agent binding. +## Naming the changed declarations + +A changed policy row names what differs, beside the unchanged limits in its +reason, instead of two endpoint counts and a facts digest. The text is +`review.changes[].change` in `diff --json` and the `verify` host comparison, and +the entry in `diff`, `check` and the PR comment: filesystem paths added or +removed per list (`read_only`, `read_write`) and a changed `include_workdir`; +the Landlock compatibility and process identity; and, per destination, each +binary selector added or removed and each destination or request declaration +(protocol, enforcement, access preset, allowed and denied method/path pairs) +added, removed or edited. Destinations that changed alike share one entry. Each +list names its first five or eight entries, then counts the rest. + +Only values the inventory already publishes are named. Query and parameter +constraints, endpoint options and credential options appear by field name, never +by value, and a value that would change under redaction is printed as +``. A change confined to an option value says `other options changed +(not shown)`. A policy whose changed fields none of these cover says so. A +newly selected or removed document lists what it declares, with the sections and +inspected endpoints left to an upstream default named. That list is a +description: it never marks the change as an expansion, and a row's direction, +expansion signals, severity and `check` outcome are those of the comparison +above. The published `rows[]` keep their counts and digest. + ## Read limits and coverage The shared identity-bound host reader limits individual files to 1 MiB, along diff --git a/src/agents_shipgate/core/capability_diff_rows.py b/src/agents_shipgate/core/capability_diff_rows.py index ebd7460f..58461ad2 100644 --- a/src/agents_shipgate/core/capability_diff_rows.py +++ b/src/agents_shipgate/core/capability_diff_rows.py @@ -64,6 +64,10 @@ from agents_shipgate.core.host_settings import rate_claude_setting, setting_value_text from agents_shipgate.core.mcp_host_selection import UNATTRIBUTED_MCP_HOST from agents_shipgate.core.openshell_compare import compare_openshell_grants +from agents_shipgate.core.openshell_row_detail import ( + openshell_policy_change, + openshell_policy_declarations, +) from agents_shipgate.core.permission_lattice import ( exec_equivalent_argument, parse_rule, @@ -707,6 +711,17 @@ def _call_change_reasons(changes: list[CallChange]) -> list[str]: return reasons +def _openshell_cell(cell: str, grant: dict[str, Any] | None) -> str: + """A wholly added or removed OpenShell policy, with what it declares beside its digest (#968). + + The counts and the facts digest stay, so the cell still identifies the + document; the declarations are display only and imply no direction. + """ + + declared = openshell_policy_declarations(grant) + return f"{cell}; declared: {declared}" if declared else cell + + def _grant_value( grant: dict[str, Any] | None, *, @@ -2383,6 +2398,17 @@ def capability_diff_rows( before=_hook_cell(row.before, before_grant), after=_hook_cell(row.after, after_grant), ) + elif kind == "openshell_policy" and before_grant and after_grant: + view = _RowView( + before=row.before, + after=row.after, + change=openshell_policy_change(before_grant, after_grant), + ) + elif kind == "openshell_policy": + view = _RowView( + before=_openshell_cell(row.before, before_grant), + after=_openshell_cell(row.after, after_grant), + ) else: view = _RowView(before=row.before, after=row.after) # Kept with the row through sorting, never emitted as row fields. diff --git a/src/agents_shipgate/core/openshell_row_detail.py b/src/agents_shipgate/core/openshell_row_detail.py new file mode 100644 index 00000000..9f3a11de --- /dev/null +++ b/src/agents_shipgate/core/openshell_row_detail.py @@ -0,0 +1,338 @@ +"""The declared facts a changed OpenShell policy row names for a human (#968). + +The row's value cell is a count and a digest. A reviewer needs to know *which* +filesystem path left a list, which binary selector changed and which +destination or request declaration came or went, so this module reads that off +the same normalized ``facts`` the comparator reads and says it in words. + +Presentation only. It decides no direction, expansion, severity or check +outcome: ``compare_openshell_grants`` still does, and its limits stay in the +row's ``why``. Every value printed is one the inventory already publishes — a +document whose policy text would change under redaction is never read into a +grant (``_openshell_public_value`` in ``host_grants``) — and each is passed +through that same sanitizer again, so a grant that reached here some other way +(an older saved baseline) still withholds a secret-shaped value. Nothing is +printed that the typed policy does not hold: no raw policy text, comment, rule +name, query value or credential option value. +""" + +from __future__ import annotations + +import re +from typing import Any + +from agents_shipgate.core.host_grants import _openshell_public_value +from agents_shipgate.core.openshell_compare import _canonical + +#: Filesystem paths, or changed binary selectors, one list names before counting the rest. +_ENTRY_LIMIT = 5 +#: Destination entries one row names before counting the rest. +_NETWORK_LIMIT = 8 +#: Binary selectors one destination entry names before counting the rest. +_BINARY_LIMIT = 3 +#: Request rules one declaration names before counting the rest. +_RULE_LIMIT = 3 +#: The longest value printed whole. The typed policy admits a 4096-byte path. +_VALUE_CHARS = 120 + +#: Said when a changed policy differs only in fields this output does not name. +_NOT_NAMED = ( + "no filesystem, Landlock, process, destination or request declaration differs " + "in the fields this output names" +) + +_REDACTED = "" +_CONTROL = re.compile(r"[\x00-\x1f\x7f]") + +#: Endpoint options a reader is told are set, by name and never by value. +_OPTION_FIELDS = ( + "tls", + "allow_encoded_slash", + "websocket_credential_rewrite", + "request_body_credential_rewrite", + "allow_uninspected_credentials", + "credential_signing", + "signing_service", + "signing_region", + "credential_binding", + "allowed_ips", + "json_rpc", + "mcp", + "graphql_persisted_queries", +) +#: Request constraints a matcher carries beyond a method and a path, by name. +_CONSTRAINT_FIELDS = ("command", "query", "operation_type", "operation_name", "fields", "params") + + +def _value(raw: Any) -> str: + """One published value as it may be printed: sanitized, one line, bounded.""" + + text = str(raw) + if _openshell_public_value(text) != text: + return _REDACTED + text = _CONTROL.sub(" ", text) + return text if len(text) <= _VALUE_CHARS else text[: _VALUE_CHARS - 1] + "…" + + +def _list(tokens: list[str], limit: int) -> str: + shown, rest = tokens[:limit], len(tokens) - limit + return ", ".join(shown) + (f", and {rest} more" if rest > 0 else "") + + +def _entries(entries: list[str], limit: int) -> list[str]: + """At most ``limit`` entries, then how many more there are.""" + + shown, rest = entries[:limit], len(entries) - limit + return shown + ([f"and {rest} more"] if rest > 0 else []) + + +def _signed(old: Any, new: Any, limit: int) -> list[str]: + """``+added`` then ``-removed`` values, each side bounded on its own.""" + + old_set, new_set = set(old or []), set(new or []) + tokens: list[str] = [] + for sign, values in (("+", sorted(new_set - old_set)), ("-", sorted(old_set - new_set))): + tokens.extend(f"{sign}{_value(item)}" for item in values[:limit]) + if len(values) > limit: + tokens.append(f"and {len(values) - limit} more") + return tokens + + +def _policy(grant: dict[str, Any] | None) -> dict[str, Any] | None: + facts = (grant or {}).get("facts") + policy = facts.get("policy") if isinstance(facts, dict) else None + return policy if isinstance(policy, dict) else None + + +def _matcher(matcher: dict[str, Any]) -> str: + parts = [str(matcher[key]) for key in ("method", "path") if matcher.get(key)] + tool = matcher.get("tool") + if isinstance(tool, dict): + tool = "|".join(map(str, tool.get("any", [])[:_RULE_LIMIT])) + if tool: + parts.append(f"tool {tool}") + named = [key for key in _CONSTRAINT_FIELDS if matcher.get(key)] + text = _value(" ".join(parts) or "(no method or path)") + return text + (f"[{', '.join(named)}]" if named else "") + + +def _declaration(endpoint: dict[str, Any]) -> str: + """An endpoint's request declaration, without any value an option holds.""" + + protocol = endpoint.get("protocol") or "l4" + protocol = "l4" if protocol == "tcp" else protocol + parts = [protocol] + if protocol != "l4": + parts.append(endpoint.get("enforcement") or "audit") + if endpoint.get("access"): + parts.append(str(endpoint["access"])) + if endpoint.get("path"): + parts.append(f"path {_value(endpoint['path'])}") + allows = sorted(_matcher(rule["allow"]) for rule in endpoint.get("rules") or []) + if allows: + parts.append(f"allow {_list(allows, _RULE_LIMIT)}") + denies = sorted(_matcher(rule) for rule in endpoint.get("deny_rules") or []) + if denies: + parts.append(f"deny {_list(denies, _RULE_LIMIT)}") + options = [name for name in _OPTION_FIELDS if endpoint.get(name)] + if endpoint.get("persisted_queries") not in {"", "deny", None}: + options.append("persisted_queries") + if endpoint.get("graphql_max_body_bytes", 65536) != 65536: + options.append("graphql_max_body_bytes") + if options: + parts.append("options " + ", ".join(options)) + return "(" + ", ".join(parts) + ")" + + +def _destinations(endpoint: dict[str, Any]) -> list[str]: + host = endpoint.get("host") or ",".join(sorted(endpoint.get("allowed_ips") or [])) + ports = endpoint.get("ports") or [endpoint.get("port")] + return [f"{_value(host)}:{port}" for port in sorted(set(ports))] + + +#: destination -> declaration identity -> (declaration text, binary selectors) +_Groups = dict[str, dict[str, tuple[str, set[str]]]] + + +def _groups(policy: dict[str, Any]) -> _Groups: + """Each endpoint declaration with the binary selectors of every rule that holds it.""" + + groups: _Groups = {} + for rule in (policy.get("network_policies") or {}).values(): + binaries = {str(item["path"]) for item in rule.get("binaries") or []} + for endpoint in rule.get("endpoints") or []: + identity = _canonical(endpoint) + text = _declaration(endpoint) + for destination in _destinations(endpoint): + entry = groups.setdefault(destination, {}).setdefault(identity, (text, set())) + entry[1].update(binaries) + return groups + + +def _via(binaries: set[str]) -> str: + if not binaries: + return "no binary selector" + return _list([_value(item) for item in sorted(binaries)], _BINARY_LIMIT) + + +def _merged(items: list[tuple[str, str, str]]) -> list[str]: + """Destinations that changed alike are one entry: ``(label, destination, tail)``.""" + + merged: dict[tuple[str, str], list[str]] = {} + for label, destination, tail in items: + merged.setdefault((label, tail), []).append(destination) + entries = [] + for (label, tail), destinations in merged.items(): + noun = "destination" if len(destinations) == 1 else "destinations" + entries.append(f"{label + ' ' if label else ''}{noun} {_list(destinations, _ENTRY_LIMIT)} {tail}") + return entries + + +def _network_changes(old: dict[str, Any], new: dict[str, Any]) -> list[str]: + before, after = _groups(old), _groups(new) + items: list[tuple[str, str, str]] = [] + for destination in sorted(before.keys() | after.keys()): + left, right = before.get(destination, {}), after.get(destination, {}) + gone = {key: value for key, value in left.items() if key not in right} + came = {key: value for key, value in right.items() if key not in left} + for key in sorted(left.keys() & right.keys()): + moved = _signed(left[key][1], right[key][1], _ENTRY_LIMIT) + if moved: + items.append(("", destination, f"{left[key][0]} binaries {' '.join(moved)}")) + if len(gone) == 1 and len(came) == 1: + (old_text, old_bins), (new_text, new_bins) = next(iter(gone.values())), next(iter(came.values())) + moved = _signed(old_bins, new_bins, _ENTRY_LIMIT) + transition = ( + f"{old_text} → {new_text}" if old_text != new_text + else f"{new_text} other options changed (not shown)" + ) + items.append(("", destination, transition + (f", binaries {' '.join(moved)}" if moved else ""))) + continue + items.extend(("removed", destination, f"{text} via {_via(binaries)}") for text, binaries in gone.values()) + items.extend(("added", destination, f"{text} via {_via(binaries)}") for text, binaries in came.values()) + return _entries(_merged(items), _NETWORK_LIMIT) + + +def _filesystem_text(section: dict[str, Any] | None) -> str: + if section is None: + return "omitted (runtime defaults)" + lists = [ + f"{name} {_list([_value(path) for path in section.get(name) or []], _ENTRY_LIMIT)}" + for name in ("read_only", "read_write") if section.get(name) + ] + return ", ".join([*lists, f"include_workdir {str(bool(section.get('include_workdir'))).lower()}"]) + + +def _filesystem_changes(old: dict[str, Any], new: dict[str, Any]) -> list[str]: + left, right = old.get("filesystem_policy"), new.get("filesystem_policy") + if left == right: + return [] + if left is None or right is None: + return [f"filesystem {_filesystem_text(left)} → {_filesystem_text(right)}"] + entries = [ + f"filesystem {name} {' '.join(moved)}" + for name in ("read_only", "read_write") + if (moved := _signed(left.get(name), right.get(name), _ENTRY_LIMIT)) + ] + if bool(left.get("include_workdir")) != bool(right.get("include_workdir")): + entries.append( + f"filesystem include_workdir {str(bool(left.get('include_workdir'))).lower()}" + f" → {str(bool(right.get('include_workdir'))).lower()}" + ) + return entries + + +def _landlock(policy: dict[str, Any]) -> str: + return str((policy.get("landlock") or {}).get("compatibility", "best_effort")) + + +def _process_changes(old: dict[str, Any], new: dict[str, Any]) -> list[str]: + left, right = old.get("process") or {}, new.get("process") or {} + return [ + f"process {name} {_value(left.get(name) or '(unset)')} → {_value(right.get(name) or '(unset)')}" + for name in ("run_as_user", "run_as_group") + if (left.get(name) or "") != (right.get(name) or "") + ] + + +def openshell_policy_change( + before: dict[str, Any] | None, after: dict[str, Any] | None +) -> str | None: + """What differs between two readings of one selected policy, in its published facts. + + ``None`` when either side publishes no policy, so a grant that carries no + facts is described by the counts and digest it always was. + """ + + if before is None or after is None: + return None + old, new = _policy(before), _policy(after) + if old is None or new is None: + return None + left, right = before["facts"], after["facts"] + parts = [ + f"{name} {_value(left.get(name))} → {_value(right.get(name))}" + for name in ("role", "runtime_version", "policy_schema_version") + if left.get(name) != right.get(name) + ] + parts.extend(_filesystem_changes(old, new)) + if _landlock(old) != _landlock(new): + parts.append(f"landlock {_value(_landlock(old))} → {_value(_landlock(new))}") + parts.extend(_process_changes(old, new)) + parts.extend(_network_changes(old, new)) + head = f"{_value(right.get('role'))}, OpenShell {_value(right.get('runtime_version'))}" + return f"{head}: " + ("; ".join(parts) if parts else _NOT_NAMED) + + +def _defaulted_audit_endpoints(facts: dict[str, Any], policy: dict[str, Any]) -> int: + """Inspected endpoints whose ``enforcement`` the document left to the upstream default.""" + + defaulted = set(facts.get("defaulted_fields") or []) + count = 0 + for key, rule in (policy.get("network_policies") or {}).items(): + escaped = key.replace("~", "~0").replace("/", "~1") + for index, endpoint in enumerate(rule.get("endpoints") or []): + if endpoint.get("protocol") in {"", "tcp", None}: + continue + if f"/network_policies/{escaped}/endpoints/{index}/enforcement" in defaulted: + count += 1 + return count + + +def openshell_policy_declarations(grant: dict[str, Any] | None) -> str | None: + """What a wholly added or removed selected policy declares, with its defaults named. + + A document that appears or disappears establishes no replacement runtime + policy, so this lists declarations and where a default stood in for one, + and says nothing about whether authority grew. + """ + + policy = _policy(grant) + if grant is None or policy is None: + return None + facts = grant["facts"] + parts = [f"filesystem {_filesystem_text(policy.get('filesystem_policy'))}"] + landlock = policy.get("landlock") + parts.append( + f"landlock {_landlock(policy)}" if landlock is not None else "landlock omitted (best_effort)" + ) + process = policy.get("process") + parts.append( + "process omitted (driver default)" if process is None + else "process " + (", ".join( + f"{name} {_value(process[name])}" for name in ("run_as_user", "run_as_group") if process.get(name) + ) or "identity unset") + ) + declared = [ + ("", destination, f"{text} via {_via(binaries)}") + for destination, groups in sorted(_groups(policy).items()) + for text, binaries in groups.values() + ] + parts.extend(_entries(_merged(declared), _NETWORK_LIMIT)) + audit = _defaulted_audit_endpoints(facts, policy) + if audit: + parts.append(f"{audit} inspected endpoint(s) default to audit enforcement") + return "; ".join(parts) + + +__all__ = ["openshell_policy_change", "openshell_policy_declarations"] diff --git a/tests/shard_seconds.json b/tests/shard_seconds.json index f5eb2895..0822cded 100644 --- a/tests/shard_seconds.json +++ b/tests/shard_seconds.json @@ -274,6 +274,7 @@ "tests/test_openshell_inventory.py": 0.8, "tests/test_openshell_native.py": 50.4, "tests/test_openshell_routes.py": 213.2, + "tests/test_openshell_row_detail.py": 24.0, "tests/test_operation_attribution_verification.py": 23.3, "tests/test_org_governance.py": 0.1, "tests/test_out_path_resolution.py": 58.2, diff --git a/tests/test_openshell_row_detail.py b/tests/test_openshell_row_detail.py new file mode 100644 index 00000000..2c9a6fc9 --- /dev/null +++ b/tests/test_openshell_row_detail.py @@ -0,0 +1,488 @@ +"""#968: a changed OpenShell policy row names the declarations that changed. + +The fixture reproduces the shape of openshift-kni/ai-sandbox#29 as static +policy text: a read-only filesystem path leaves the list, binary selectors +change (two of them wildcards), a rule is removed and release destinations are +added. Nothing is installed or run. +""" + +from __future__ import annotations + +import hashlib +import json + +import pytest +from test_openshell_inputs import REGISTRATION, comparison, selection +from test_partial_host_comparison import _repository +from typer.testing import CliRunner + +from agents_shipgate.cli.main import app +from agents_shipgate.core.capability_diff_rows import capability_diff_rows, review_changes +from agents_shipgate.core.host_grants import diff_host_grants, host_grant_expansion_signals +from agents_shipgate.core.openshell import parse_policy +from agents_shipgate.core.openshell_compare import compare_openshell_grants + +POLICY_PATH = "rds-policy/evals/openshell-policy.yaml" + +BASE = """\ +# comments are never published +version: 1 +filesystem_policy: + include_workdir: false + read_only: + - /usr + - /lib + - /sbin + - /etc + - /proc + - /sandbox/.uv/python + read_write: + - /tmp + - /dev +landlock: + compatibility: best_effort +network_policies: + vertex: + name: vertex-ai + endpoints: + - host: aiplatform.googleapis.com + port: 443 + enforcement: enforce + binaries: + - path: /usr/local/bin/claude + - path: /usr/bin/node + google_auth: + name: google-adc-token + endpoints: + - host: oauth2.googleapis.com + port: 443 + enforcement: enforce + binaries: + - path: /usr/local/bin/claude + npm: + name: npm-registry + endpoints: + - host: registry.npmjs.org + port: 443 + enforcement: enforce + binaries: + - path: /usr/bin/node + pypi: + name: python-packages + endpoints: + - host: pypi.org + port: 443 + enforcement: enforce + - host: files.pythonhosted.org + port: 443 + enforcement: enforce + binaries: + - path: /usr/local/bin/uv + github: + name: eval-repo + endpoints: + - host: github.com + port: 443 + enforcement: enforce + binaries: + - path: /usr/bin/git + - path: /usr/lib/git-core/** +""" + +HEAD = """\ +version: 1 +filesystem_policy: + include_workdir: false + read_only: + - /usr + - /lib + - /sbin + - /etc + - /proc + read_write: + - /tmp + - /dev +landlock: + compatibility: best_effort +network_policies: + vertex: + name: vertex-ai + endpoints: + - host: aiplatform.googleapis.com + port: 443 + enforcement: enforce + binaries: + - path: /tmp/**/node_modules/@anthropic-ai/claude-agent-sdk-linux-x64/claude + - path: /usr/bin/node + npm: + name: npm-registry + endpoints: + - host: registry.npmjs.org + port: 443 + enforcement: enforce + binaries: + - path: /usr/bin/node + pypi: + name: python-packages + endpoints: + - host: pypi.org + port: 443 + enforcement: enforce + - host: files.pythonhosted.org + port: 443 + enforcement: enforce + binaries: + - path: /usr/bin/python3* + - path: /tmp/.local/bin/uv + github: + name: github + endpoints: + - host: github.com + port: 443 + enforcement: enforce + - host: release-assets.githubusercontent.com + port: 443 + enforcement: enforce + - host: objects.githubusercontent.com + port: 443 + enforcement: enforce + binaries: + - path: /usr/bin/git + - path: /usr/lib/git-core/** + - path: /tmp/.local/bin/uv +""" + +#: A two-endpoint REST policy whose declarations the controls edit. +REST = """\ +version: 1 +filesystem_policy: {include_workdir: false, read_only: [/data]} +network_policies: + api: + binaries: [{path: /usr/bin/client}] + endpoints: + - host: api.example.com + port: 443 + protocol: rest + enforcement: enforce + rules: + - allow: {method: GET, path: /records} +""" + + +def _policy_rows(root, *, head="HEAD"): + result = comparison(root, head=head) + assert result.comparison_status == "comparable" + return [row for row in result.rows if row.subject.startswith("openshell")], result + + +def _repo(tmp_path, base, head, path=POLICY_PATH): + return _repository( + tmp_path, {REGISTRATION: selection(path), path: base}, {path: head} + ) + + +def _diff(root, *args): + result = CliRunner().invoke(app, ["diff", "--workspace", str(root), "--base", "main", *args]) + assert result.exit_code == 0, result.output + return result.output + + +def _grant(text, *, role="authored", defaulted=()): + policy = parse_policy(text) + facts = { + "registration": REGISTRATION, "role": role, "runtime_version": "0.1.2", + "policy_schema_version": 1, "policy": policy.model_dump(mode="json"), + "defaulted_fields": list(defaulted), "field_paths": [], + } + return { + "grant_id": "one", "kind": "openshell_policy", "host": "openshell", "source": POLICY_PATH, + "scope": "repository", "risk": "unknown", "access": "unknown", + "config_sha256": hashlib.sha256(json.dumps(facts, sort_keys=True).encode()).hexdigest(), + "facts": facts, + } + + +def _rows(before, after): + change = {"grant_id": "one", "baseline": before, "current": after} + signals = host_grant_expansion_signals([change]) + rows = capability_diff_rows({"changes": [change], "expansion_signals": signals}) + return rows, review_changes(rows) + + +def _change(old, new): + rows, changes = _rows(_grant(old), _grant(new)) + (row,), (item,) = rows, changes + return row, item + + +def test_issue_shape_names_the_changed_paths_binaries_and_destinations(tmp_path): + root = _repo(tmp_path, BASE, HEAD) + (row,), result = _policy_rows(root) + review, = result.review.changes + detail = review.change + assert detail is not None + # The filesystem path that narrowed, by list. + assert "filesystem read_only -/sandbox/.uv/python" in detail + # The rule that left, with the binary that reached it. + assert "removed destination oauth2.googleapis.com:443 (l4) via /usr/local/bin/claude" in detail + # Binary selectors that changed on a destination that stayed. + assert "destination aiplatform.googleapis.com:443 (l4) binaries " in detail + assert "+/tmp/**/node_modules/@anthropic-ai/claude-agent-sdk-linux-x64/claude" in detail + assert "-/usr/local/bin/claude" in detail + assert "binaries +/tmp/.local/bin/uv +/usr/bin/python3* -/usr/local/bin/uv" in detail + # Added destinations, each with its binary selectors. + assert ( + "added destinations objects.githubusercontent.com:443, release-assets.githubusercontent.com:443 " + "(l4) via /tmp/.local/bin/uv, /usr/bin/git, /usr/lib/git-core/**" in detail + ) + # Counts and digests are what the cells still hold; the detail replaces them in the text. + assert "facts" not in detail + assert row.before.endswith(row.before.rpartition("facts ")[2]) and "6 endpoint(s)" in row.before + assert "7 endpoint(s)" in row.after + assert review.before == row.before and review.after == row.after + + +def test_issue_shape_keeps_direction_expansion_and_limits(tmp_path): + (row,), result = _policy_rows(_repo(tmp_path, BASE, HEAD)) + assert row.direction == "changed" and not row.expands and row.severity == "unknown" + assert "wildcard or unresolved executable selector is outside exact comparison" in row.why + assert "a single authority direction is unknown" in row.why + assert "runtime enforcement and freshness are unverified" in row.why + assert result.review.summary.widenings == 0 + + +def test_diff_text_and_json_say_the_same_thing(tmp_path): + root = _repo(tmp_path, BASE, HEAD) + text = _diff(root) + payload = json.loads(_diff(root, "--json")) + detail = payload["review"]["changes"][0]["change"] + assert detail.startswith("authored, OpenShell 0.1.2: filesystem read_only -/sandbox/.uv/python") + assert detail in text + assert "a single authority direction is unknown" in text + # The published rows carry the values they always did. + row, = [row for row in payload["rows"] if row["subject"].startswith("openshell")] + assert row["before"].startswith("authored, OpenShell 0.1.2, 6 endpoint(s), facts ") + assert row["after"].startswith("authored, OpenShell 0.1.2, 7 endpoint(s), facts ") + + +def test_unchanged_policy_has_no_row(tmp_path): + root = _repo(tmp_path, BASE, BASE + "\n# a comment only\n") + rows, result = _policy_rows(root) + assert rows == [] and result.review.changes == [] + + +def _document(*, read_only=("/data",), rules=None): + """A policy as JSON (a YAML subset) from ``{rule: (binaries, endpoints)}``.""" + + return json.dumps({ + "version": 1, + "filesystem_policy": {"include_workdir": False, "read_only": list(read_only)}, + "network_policies": { + name: {"binaries": [{"path": path} for path in binaries], "endpoints": endpoints} + for name, (binaries, endpoints) in (rules or {}).items() + }, + }) + + +def _rest(**changes): + endpoint = { + "host": "api.example.com", "port": 443, "protocol": "rest", "enforcement": "enforce", + "rules": [{"allow": {"method": "GET", "path": "/records"}}], + } + return {**endpoint, **changes} + + +def _only(endpoint, binaries=("/usr/bin/client",)): + return _document(rules={"api": (binaries, [endpoint])}) + + +def test_equivalent_respelling_has_no_row(): + old = _only(_rest()) + new = _document(rules={"renamed": (("/usr/bin/client",), [_rest()])}) + assert compare_openshell_grants(_grant(old), _grant(new)).direction == "equivalent" + assert diff_host_grants({"grants": [_grant(old)]}, {"grants": [_grant(new)]}) == [] + + +def test_only_an_unnamed_option_value_changed_keeps_the_limit_wording(): + old = _only(_rest(protocol="", rules=[], enforcement="audit", credential_binding={"provider": "one"})) + new = _only(_rest(protocol="", rules=[], enforcement="audit", credential_binding={"provider": "two"})) + row, item = _change(old, new) + assert item.change == ( + "authored, OpenShell 0.1.2: destination api.example.com:443 " + "(l4, options credential_binding) other options changed (not shown)" + ) + assert "transport or credential semantics are outside exact comparison" in row.why + assert "one" not in item.change and "two" not in item.change + + +def test_a_change_confined_to_composition_names_no_field_and_keeps_the_limit(): + old, new = _grant(_only(_rest())), _grant(_only(_rest())) + old["facts"]["composition"], new["facts"]["composition"] = {"workspace": "one"}, {"workspace": "two"} + rows, changes = _rows(old, new) + assert changes[0].change == ( + "authored, OpenShell 0.1.2: no filesystem, Landlock, process, destination or request " + "declaration differs in the fields this output names" + ) + assert "composition workspace resolution changed" in rows[0].why + assert "a single authority direction is unknown" in rows[0].why + + +def test_role_change_is_named_beside_the_existing_limit(): + rows, changes = _rows(_grant(_only(_rest())), _grant(_only(_rest()), role="effective_snapshot")) + assert changes[0].change.startswith("effective_snapshot, OpenShell 0.1.2: role authored → effective_snapshot") + assert "policy version or input role changed" in rows[0].why + + +def test_many_filesystem_paths_are_bounded_with_a_count_of_the_rest(): + old = _document(read_only=["/old", *(f"/kept/{n}" for n in range(3))]) + new = _document(read_only=[*(f"/kept/{n}" for n in range(3)), *(f"/new/{n:02}" for n in range(9))]) + _, item = _change(old, new) + assert item.change == ( + "authored, OpenShell 0.1.2: filesystem read_only " + "+/new/00 +/new/01 +/new/02 +/new/03 +/new/04 and 4 more -/old" + ) + + +def test_many_destinations_are_bounded_with_a_count_of_the_rest(): + def rules(count): + return {f"r{n}": ((f"/bin/{n}",), [{"host": f"h{n:02}.example.com", "port": 443}]) for n in range(count)} + + _, item = _change(_document(rules=rules(1)), _document(rules=rules(13))) + entries = item.change.partition(": ")[2].split("; ") + assert len(entries) == 9 and entries[-1] == "and 4 more" + assert entries[0] == "added destination h01.example.com:443 (l4) via /bin/1" + # Destinations that changed alike are one entry, itself bounded. + same = {f"r{n}": (("/bin/a",), [{"host": f"h{n:02}.example.com", "port": 443}]) for n in range(12)} + _, item = _change(_document(), _document(rules=same)) + assert item.change.endswith( + "added destinations h00.example.com:443, h01.example.com:443, h02.example.com:443, " + "h03.example.com:443, h04.example.com:443, and 7 more (l4) via /bin/a" + ) + + +def test_many_binary_selectors_are_bounded_with_a_count_of_the_rest(): + endpoints = [{"host": "x.example.com", "port": 443}] + old = _document(rules={"one": (("/a",), endpoints)}) + new = _document(rules={"one": (tuple(f"/bin/{n}" for n in range(8)), endpoints)}) + _, item = _change(old, new) + assert item.change.endswith( + "destination x.example.com:443 (l4) binaries +/bin/0 +/bin/1 +/bin/2 +/bin/3 +/bin/4 and 3 more -/a" + ) + # Added destinations name at most three selectors. + _, item = _change(_document(), _document(rules={"one": (tuple(f"/bin/{n}" for n in range(8)), endpoints)})) + assert item.change.endswith("via /bin/0, /bin/1, /bin/2, and 5 more") + + +def test_request_declarations_are_named_without_query_or_credential_values(): + rules = [ + {"allow": {"method": "GET", "path": "/records"}}, + {"allow": {"method": "POST", "path": "/records", "query": {"token": "abc", "page": "1"}}}, + ] + new = _only(_rest(rules=rules, deny_rules=[{"method": "DELETE", "path": "/records"}])) + _, item = _change(_only(_rest()), new) + assert item.change == ( + "authored, OpenShell 0.1.2: destination api.example.com:443 " + "(rest, enforce, allow GET /records) → " + "(rest, enforce, allow GET /records, POST /records[query], deny DELETE /records)" + ) + assert "abc" not in item.change and "token" not in item.change + + +def test_a_preset_and_enforcement_change_reads_as_a_declaration_transition(): + old = _only(_rest(rules=[], access="read-only")) + new = _only(_rest(rules=[], access="read-write", enforcement="audit")) + row, item = _change(old, new) + assert item.change.endswith( + "destination api.example.com:443 (rest, enforce, read-only) → (rest, audit, read-write)" + ) + assert row.direction == "widened" and row.expands + + +def test_secret_shaped_values_are_withheld(): + secret = "sk-proj-abcdefghijklmnopqrstuvwxyz0123456789" + clean = _grant(_only(_rest())) + # The reader refuses a document whose values would change under redaction + # (see below), so a grant like these reaches the rows only from an older + # saved baseline. The row still withholds the value. + for edit in ( + {"host": f"{secret}.example.com"}, + {"rules": [{"allow": {"method": "GET", "path": f"/records/{secret}"}}]}, + ): + _, item = _change_grants(clean, _grant(_only(_rest(**edit)))) + assert secret not in item.change + assert "" in item.change + + +def test_a_secret_shaped_destination_is_refused_by_the_reader_not_summarized(tmp_path): + secret = "ghp_" + "a1B2c3D4e5F6g7H8i9J0k1L2m3N4o5P6q7R8" + leaked = _only(_rest(host=f"{secret}.example.com")) + root = _repo(tmp_path, _only(_rest()), leaked) + result = comparison(root, head="HEAD") + assert secret not in result.model_dump_json() + assert not [row for row in result.rows if row.subject.startswith("openshell")] + assert "cannot be compared after redaction" in result.model_dump_json() + + +def _change_grants(old, new): + rows, changes = _rows(old, new) + (row,), (item,) = rows, changes + return row, item + + +def test_a_newly_selected_document_shows_declarations_and_default_provenance(): + added = _only(_rest(enforcement="audit")) + grant = _grant( + added, + defaulted=["/landlock", "/process", "/network_policies/api/endpoints/0/enforcement"], + ) + rows, changes = _rows(None, grant) + (row,), (item,) = rows, changes + assert row.before == "—" and item.change is None and item.before == "—" + assert item.after.startswith(row.after + "; declared: ") + declared = item.after.partition("declared: ")[2] + assert declared == ( + "filesystem read_only /data, include_workdir false; " + "landlock omitted (best_effort); process omitted (driver default); " + "destination api.example.com:443 (rest, audit, allow GET /records) via /usr/bin/client; " + "1 inspected endpoint(s) default to audit enforcement" + ) + # Declarations are not a proof of expansion. + assert not row.expands and row.direction == "changed" + # The same for a removed document. + rows, changes = _rows(grant, None) + assert changes[0].before == f"{rows[0].before}; declared: {declared}" + assert changes[0].after == "—" + + +def test_a_newly_selected_document_in_a_repository(tmp_path): + root = _repository(tmp_path, {}, {REGISTRATION: selection(POLICY_PATH), POLICY_PATH: HEAD}) + row, = [row for row in comparison(root, head="HEAD").review.changes if "openshell" in row.subject] + declared = row.after.partition("; declared: ")[2] + assert declared.startswith("filesystem read_only /usr, /lib, /sbin, /etc, /proc, read_write /tmp, /dev, include_workdir false") + assert "destination " in declared and "default to audit" not in declared + assert row.expands is False + + +@pytest.mark.parametrize("surface", ["verify", "preview"]) +def test_verify_host_comparison_carries_the_same_detail(tmp_path, surface): + root = _repo(tmp_path, BASE, HEAD) + args = ["verify", "--workspace", str(root), "--base", "main", "--head", "HEAD", "--json"] + if surface == "preview": + args.insert(1, "--preview") + result = CliRunner().invoke(app, args) + assert result.exit_code in (0, 10, 20), result.output + review = json.loads(result.output)["host_comparison"]["review"] + assert "filesystem read_only -/sandbox/.uv/python" in review["changes"][0]["change"] + + +def test_the_pr_comment_and_a_tight_comment_name_the_changed_declarations(tmp_path): + from agents_shipgate.report.host_comparison import host_comparison_lines + + result = comparison(_repo(tmp_path, BASE, HEAD), head="HEAD") + whole = "\n".join(host_comparison_lines(result, markdown=True)) + assert "filesystem read_only -/sandbox/.uv/python" in whole + assert "removed destination oauth2.googleapis.com:443" in whole + # A bounded surface cuts the entry rather than falling back to counts and digests. + tight = "\n".join(host_comparison_lines(result, markdown=True, entry_max_chars=0)) + assert "filesystem read_only -/sandbox/.uv/python" in tight + assert "facts " not in tight and "…" in tight