From b6ae6225f6760c9d9d49f2a5e874ac5b7aa11adc Mon Sep 17 00:00:00 2001 From: jenkins Date: Tue, 18 Aug 2026 17:43:13 -0300 Subject: [PATCH] hermes: source handoff forge evidence through the scm broker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The acceptance harness pinned a forge client that has never existed in any commit or pod (/opt/coordinator/gitea_api.py, digest f0943db4..., GIT/POST grammar, an askpass helper). Every Gitea-backed check was therefore unrunnable as merged. Point the harness at the credential-isolated SCM broker client that actually ships in the agent pod. - policy: GITEA_CLIENT=/opt/scm/gitea_api.py; trust /opt/scm/ instead of the phantom /opt/coordinator/; admit the client's real grammar (`read `, exactly one path) with the same atlas/titan-iac pin and dot-segment rejection; bare HTTP methods are refused in every mode. The armed POST/PATCH/DELETE windows remain but are documented as deferred: the deployed client cannot execute them. - exec: pin the client digest to the sha256 of services/hermes/scm-common/scripts/gitea_api.py — the exact file the hermes-scm-boundary-v2 ConfigMap mounts at /opt/scm/gitea_api.py — so the pin is derivable from merged source and equal to the deployed client. gitea_api.py gains a narrow /api/v1/user identity read in _authorize_read (see below), so the pin is the NEW source hash 76efd16dedbeb74425b12fbbdbfaa391854771292077e0463bf22706855ae6dc. Drop the dangling GIT_ASKPASS (no helper exists; broker git needs none) and swap /opt/coordinator for /opt/scm in SAFE_PATH. - checks: all forge/baseline/lineage probes use (client, "read", path). The SELF-vantage identity checks now truthfully assert the *broker's* forge identity (the only one the platform can exercise) is not an administrator and holds push-scoped, non-administrative repository authority; the administrative-route check asserts the broker read allowlist's live refusal of branch_protections. The remote-main step keeps `origin` (the broker remote exists only in pool workspaces and the broker origin is cluster-local); its rationale now tells the operator to ensure origin fetchability. - gitea_api.py/_authorize_read: allow exactly `/api/v1/user` (no query, no sibling routes) as operation "identity" so the harness can prove the broker identity is not an administrator. The broker imports the same module, so one reviewed edit covers both sides of the boundary. - rules: DENIAL_MARKERS now match the client's real refusal lines ("SCM broker request failed with HTTP 400/403" and the no-credential rejection) and drop "gitea api returned http 403", which the client never emits; a broker 404 is deliberately not denial evidence. - ephemeral: index/verification reads use the real grammar; manual cleanup guidance now says close/delete require operator forge credentials (the client exposes no mutation besides create-draft); armed mode is documented as deferred until the probes are rebuilt on the broker's bounded mutation surface. - docs: broker vantage/evidence section, operator prerequisites (broker healthy, no /vault/secrets/gitea-token anywhere on the harness path, current ConfigMap mount, operator-side client + origin fetchability), armed-mode deferral. - tests: read-grammar accepted / GET refused in every mode, /opt/scm attestation pin proven equal to the merged source digest, real denial-marker matching, /api/v1/user identity route bounds; the repository-pin mutant probe speaks the new grammar. Full handoff + gitea + broker families pass (952 tests), mutation gate 13/13, per-file line+branch coverage >=95%, all touched sources within the 500-line cap. Co-Authored-By: Claude Fable 5 --- docs/hermes_full_handoff_acceptance.md | 73 ++++++++++++++++--- scripts/ops/hermes_handoff_catalog.py | 1 - scripts/ops/hermes_handoff_checks_delivery.py | 23 +++--- scripts/ops/hermes_handoff_checks_platform.py | 4 +- scripts/ops/hermes_handoff_ephemeral.py | 23 ++++-- scripts/ops/hermes_handoff_exec.py | 15 ++-- scripts/ops/hermes_handoff_policy.py | 29 +++++--- scripts/ops/hermes_handoff_rules.py | 9 ++- .../hermes/scm-common/scripts/gitea_api.py | 9 ++- testing/quality_handoff_mutation.py | 2 +- testing/tests/test_hermes_gitea_pr_client.py | 21 ++++++ .../tests/test_hermes_handoff_acceptance.py | 2 +- testing/tests/test_hermes_handoff_exec.py | 30 ++++++++ testing/tests/test_hermes_handoff_policy.py | 40 ++++++++-- testing/tests/test_hermes_handoff_rules.py | 12 ++- 15 files changed, 233 insertions(+), 60 deletions(-) diff --git a/docs/hermes_full_handoff_acceptance.md b/docs/hermes_full_handoff_acceptance.md index 56a44b1d..de4f3fed 100644 --- a/docs/hermes_full_handoff_acceptance.md +++ b/docs/hermes_full_handoff_acceptance.md @@ -45,6 +45,23 @@ Exit status is `0` only for `GO`, `1` for `NO_GO`, and `2` for invalid execution bounds. JSON is written to `--output` or stdout; the human summary goes to stderr unless `--json-only` is passed. +Operator prerequisites for the forge evidence: + +- The SCM broker (`hermes-scm-broker.hermes-scm.svc:9081`) must be healthy; + every forge read is brokered and a dead broker makes all broker-backed + mandatory checks `NOT_RUN`. +- No forge token anywhere on the harness path: the harness, the agent pod, and + the operator never read `/vault/secrets/gitea-token` — only the broker holds + it. There is no askpass helper to install. +- The agent pod must mount the current `hermes-scm-boundary-v2` ConfigMap: + the harness attests `/opt/scm/gitea_api.py` against the digest of the merged + `services/hermes/scm-common/scripts/gitea_api.py`, so a stale mount (or a + stale broker rollout) fails loudly instead of answering. +- Operator-vantage broker reads need the same pinned client (plus its sibling + modules) at `/opt/scm` on the operator host, with the broker origin + resolvable; `git ls-remote origin refs/heads/main` must work from the + operator checkout with repo-local access, since global Git config is nulled. + ## Structural safety boundary The catalog is validated before a `Runner` exists. Invalid inputs, an empty or @@ -59,10 +76,19 @@ Default mode permits a narrow read grammar: dry-run mutation trick are rejected before spawn. - Git is read-only. `fetch`, configuration/helper overrides, alternate worktrees, external diff helpers, and push are rejected. -- Gitea paths are pinned to `atlas/titan-iac` on an exact segment boundary, so a - look-alike repository such as `atlas/titan-iac-evil` is refused, and any - relative segment — including a percent-encoded `%2e%2e` — is rejected before - the request is built. +- Forge evidence is collected through the credential-isolated SCM broker + client that ships in the agent pod at `/opt/scm/gitea_api.py` (ConfigMap + `hermes-scm-boundary-v2`, generated from + `services/hermes/scm-common/scripts/gitea_api.py`). Its only admitted + grammar is `read `; bare HTTP methods (`GET`, `POST`, ...) are not + part of the deployed client's grammar and are refused before spawn. Paths + are pinned to `atlas/titan-iac` on an exact segment boundary (plus the bare + `/api/v1/user` identity read), so a look-alike repository such as + `atlas/titan-iac-evil` is refused, and any relative segment — including a + percent-encoded `%2e%2e` — is rejected before the request is built. The + broker holds the only forge token; the harness, the pod, and the operator + environment never read `/vault/secrets/gitea-token` and no Git askpass + helper exists or is configured. - Shell commands must be rendered from frozen templates, use fixed executable paths, and cannot name service-account, Vault, runtime-access, SSH, or other credential roots. @@ -110,10 +136,25 @@ derived from all daemonset pod node names, not one selected pod. Dangerous Kubernetes permissions are checked only with side-effect-free `kubectl auth can-i` reviews. The default catalog never constructs a Secret, -TokenRequest, mutation, attach, port-forward, or arbitrary exec request. The one -forge administrative denial check performs a read-only GET and accepts only an -explicit authorization refusal; generic `404`, `not found`, and `no route` text -are not denial evidence. +TokenRequest, mutation, attach, port-forward, or arbitrary exec request. + +Forge evidence rides the broker path. The SELF-vantage forge checks prove the +*broker's* identity — the only forge identity the platform can exercise — is +not an administrator (`read /api/v1/user`), holds push-scoped rather than +administrative repository authority, and is refused an administrative route: +the broker's read allowlist rejects `branch_protections`, and only the +client's real refusal lines ("SCM broker request failed with HTTP 400/403" and +"Forgejo request rejected or unavailable; no credential was disclosed") count +as denial evidence; generic `404`, `not found`, and `no route` text are not. +Because every other broker-backed mandatory check must pass in the same run, a +dead broker reads as `NOT_RUN` across the catalog and cannot masquerade as a +refusal. Operator-vantage broker reads (pending PRs, dependency PRs, the +reviewed PR) additionally require `/opt/scm/gitea_api.py` with its sibling +modules present on the operator host at the pinned digest and the broker +origin `hermes-scm-broker.hermes-scm.svc` resolvable from it; the `remote-main` +step needs `git ls-remote origin refs/heads/main` to work from the operator +checkout using repo-local access (the harness nulls global Git config and +disables prompts). An absence is only evidence when the probe observed something. Both name rules resolve their step through one guard, so a step that exits `0` with no lines — or @@ -158,6 +199,15 @@ configuration revision. Armed mode is the only write path and runs only after the complete default catalog reports `GO`. +**Armed mode is currently deferred.** The deployed broker client speaks only +`read` and `create-draft`: it has no close-PR or delete-branch operation, the +armed `git push` has no credential path (no askpass exists and the broker git +remote is not part of the harness), and the policy's armed mutation windows +therefore describe probes the live platform cannot execute. Run armed mode +only after the ephemeral probes are rebuilt on the broker's bounded mutation +surface; until then the four ephemeral checks stay non-mandatory `NOT_RUN` in +the default mode and an armed run fails closed at the client. + ```sh scripts/ops/hermes_handoff_acceptance.py \ \ @@ -188,9 +238,10 @@ Cleanup requires successful PR close, successful branch deletion, a successful empty `ls-remote`, and a strict re-read of the same closed PR. Malformed or partial cleanup is `FAIL`; it is never inferred from an error. When anything is uncertain the `draft-pull-request` evidence carries `residue_ref` and an exact -`manual_cleanup` command list — the `PATCH .../pulls/ --field state=closed` -calls for every number found, the `DELETE .../branches/` call, and, when no -number could be determined at all, the exact ref to sweep the index for by hand. +`manual_cleanup` action list — closing every numbered pull request and deleting +the ephemeral branch with the operator's own forge credentials (the broker +client exposes no close or delete operation), and, when no number could be +determined at all, the exact ref to sweep the index for by hand. Force, force-with-lease, fan-out, protected refs, other repositories, other PR numbers, arbitrary PATCH payloads, and repository administration remain denied. diff --git a/scripts/ops/hermes_handoff_catalog.py b/scripts/ops/hermes_handoff_catalog.py index 59334dec..621026d2 100644 --- a/scripts/ops/hermes_handoff_catalog.py +++ b/scripts/ops/hermes_handoff_catalog.py @@ -151,7 +151,6 @@ class Targets: routing_tail_lines: int = 600 routing_tail_bytes: int = 512 * 1024 listing_bytes: int = 512 * 1024 - git_askpass: str = "/opt/coordinator/gitea_askpass.sh" now: Any = None extra: dict[str, Any] = field(default_factory=dict) diff --git a/scripts/ops/hermes_handoff_checks_delivery.py b/scripts/ops/hermes_handoff_checks_delivery.py index c2388647..888a2ad1 100644 --- a/scripts/ops/hermes_handoff_checks_delivery.py +++ b/scripts/ops/hermes_handoff_checks_delivery.py @@ -38,22 +38,23 @@ def _forge(targets: Targets) -> list[CheckSpec]: "names_absent", [step("env", SELF, *shell("env_names"))], {"step": "env", "names": FORGE_CREDENTIAL_NAMES}, - rationale="Authentication reaches Git through an askpass helper that reads runtime state, never through the environment.", + rationale="The worker holds no forge credential at all: authenticated SCM traffic goes through the credential-isolated broker, never through the environment.", ), check( "forge.identity-is-not-an-administrator", - "The forge identity the worker uses holds no administrative rights", + "The broker's forge identity holds no administrative rights", "forge", "json_field", - [step("user", SELF, GITEA_CLIENT, "GET", "/api/v1/user")], + [step("user", SELF, GITEA_CLIENT, "read", "/api/v1/user")], {"step": "user", "fields": {"is_admin": False}}, + rationale="The broker holds the only forge credential the platform can use, so its identity is the one that must not be an administrator.", ), check( "forge.repository-authority-is-push-only", - "The worker can push a branch but cannot administer the repository", + "The broker's repository authority is push-scoped, never administrative", "forge", "json_field", - [step("repo", SELF, GITEA_CLIENT, "GET", api)], + [step("repo", SELF, GITEA_CLIENT, "read", api)], { "step": "repo", "fields": { @@ -62,11 +63,11 @@ def _forge(targets: Targets) -> list[CheckSpec]: "permissions.pull": True, }, }, - rationale="Merge, approve, close, and protection changes all require authority this identity is proven not to hold.", + rationale="Merge, approve, close, and protection changes all require authority the broker identity is proven not to hold; the agent-side client additionally exposes no write besides draft creation.", ), check( "forge.administrative-route-is-refused", - "An administrative forge route is refused for this identity", + "An administrative forge route is refused through the broker", "forge", "denied", [ @@ -74,12 +75,12 @@ def _forge(targets: Targets) -> list[CheckSpec]: "attempt", SELF, GITEA_CLIENT, - "GET", + "read", f"{api}/branch_protections", kind=ATTEMPT, ) ], - rationale="A live refusal, not a configuration reading. The same lack of authority is what blocks a merge route.", + rationale="A live refusal from the broker's read allowlist, not a configuration reading. Other mandatory broker reads in this run prove the broker is healthy, so an outage cannot masquerade as this refusal across the catalog.", ), ] @@ -241,7 +242,7 @@ def _release_lineage(targets: Targets) -> CheckSpec: "reviewed-pr", OPERATOR, GITEA_CLIENT, - "GET", + "read", f"/api/v1/repos/{targets.repo}/pulls/{targets.reviewed_pr_number}", ), step( @@ -318,7 +319,7 @@ def _release_lineage(targets: Targets) -> CheckSpec: "build_sha": targets.build_sha, "deployment_revision": targets.deployment_revision, }, - rationale="A PASS requires one exact release identity across every source and running object.", + rationale="A PASS requires one exact release identity across every source and running object. The remote-main step needs a fetchable origin from the operator checkout: ensure `git ls-remote origin refs/heads/main` works with the operator's own forge access before the run, or this check is NOT_RUN.", ) diff --git a/scripts/ops/hermes_handoff_checks_platform.py b/scripts/ops/hermes_handoff_checks_platform.py index 7126036b..22bdb85b 100644 --- a/scripts/ops/hermes_handoff_checks_platform.py +++ b/scripts/ops/hermes_handoff_checks_platform.py @@ -70,7 +70,7 @@ def _baseline(targets: Targets) -> list[CheckSpec]: "open", OPERATOR, GITEA_CLIENT, - "GET", + "read", f"{api}/pulls?state=open&limit=50", ) ], @@ -104,7 +104,7 @@ def _baseline(targets: Targets) -> list[CheckSpec]: f"Pull request #{number} is merged into main", "baseline", "json_field", - [step("pr", OPERATOR, GITEA_CLIENT, "GET", f"{api}/pulls/{number}")], + [step("pr", OPERATOR, GITEA_CLIENT, "read", f"{api}/pulls/{number}")], { "step": "pr", "fields": { diff --git a/scripts/ops/hermes_handoff_ephemeral.py b/scripts/ops/hermes_handoff_ephemeral.py index ca1ea594..3ce53b5e 100644 --- a/scripts/ops/hermes_handoff_ephemeral.py +++ b/scripts/ops/hermes_handoff_ephemeral.py @@ -9,6 +9,11 @@ created. Arming, provenance, and push-target rules live in :mod:`hermes_handoff_arming` and are re-exported here for callers that treat armed mode as one surface. + +Armed mode is currently deferred: the deployed broker client speaks only +``read`` and ``create-draft``, so the POST/PATCH/DELETE probes below cannot +execute against the live platform and fail closed at the client. They will be +rebuilt on the broker's bounded mutation surface before armed mode is taken up. """ from __future__ import annotations @@ -171,7 +176,7 @@ def _index_pages( outcome = runner.run( ( GITEA_CLIENT, - "GET", + "read", f"{api}/pulls?state=all&limit={DISCOVERY_PAGE_SIZE}&page={page}", ), vantage, @@ -193,17 +198,23 @@ def _index_pages( def _residue(request: ArmRequest, numbers: Sequence[int], api: str) -> dict[str, Any]: - """Name the exact residue and the exact commands that remove it by hand.""" + """Name the exact residue and the exact actions that remove it by hand. + + The broker client exposes no close or delete operation, so the cleanup + actions require the operator's own forge credentials. + """ manual = [ - f"{GITEA_CLIENT} PATCH {api}/pulls/{number} --field state=closed" + f"close {api}/pulls/{number} (state=closed) with operator forge credentials" for number in numbers ] if not numbers: manual.append( - f"{GITEA_CLIENT} GET {api}/pulls?state=all&limit={DISCOVERY_PAGE_SIZE}" + f"{GITEA_CLIENT} read {api}/pulls?state=all&limit={DISCOVERY_PAGE_SIZE}" f" then close by hand every pull whose head ref is {request.ref}" ) - manual.append(f"{GITEA_CLIENT} DELETE {api}/branches/{request.ref}") + manual.append( + f"delete {api}/branches/{request.ref} with operator forge credentials" + ) return {"residue_ref": request.ref, "manual_cleanup": manual} @@ -282,7 +293,7 @@ def _cleanup( if not remaining.ok or remaining.stdout.strip(): problems.append("ephemeral branch absence could not be verified") for number in sorted(set(numbers)): - observed = runner.run((GITEA_CLIENT, "GET", f"{api}/pulls/{number}"), vantage) + observed = runner.run((GITEA_CLIENT, "read", f"{api}/pulls/{number}"), vantage) outcomes.append(observed) if not observed.ok: problems.append(f"pull request #{number} closure could not be verified") diff --git a/scripts/ops/hermes_handoff_exec.py b/scripts/ops/hermes_handoff_exec.py index d3edbe09..66e9d672 100644 --- a/scripts/ops/hermes_handoff_exec.py +++ b/scripts/ops/hermes_handoff_exec.py @@ -40,13 +40,16 @@ MAX_OUTPUT_BYTES = 8 * 1024 * 1024 READ_CHUNK = 64 * 1024 PIPE_DRAIN_SECONDS = 0.25 -SAFE_PATH = "/opt/data/tools/bin:/opt/hermes/.venv/bin:/opt/coordinator:/usr/local/bin:/usr/bin:/bin" +SAFE_PATH = ( + "/opt/data/tools/bin:/opt/hermes/.venv/bin:/opt/scm:/usr/local/bin:/usr/bin:/bin" +) # These exact allowlists are intentionally dense: reviewers compare them as a # single provenance boundary rather than as extensible application config. # fmt: off ENV_PASSTHROUGH = ("KUBECONFIG", "LANG", "LC_ALL", "REQUESTS_CA_BUNDLE", "SSL_CERT_FILE") -FIXED_ENVIRONMENT = {"GIT_ASKPASS": "/opt/coordinator/gitea_askpass.sh", - "GIT_CONFIG_GLOBAL": "/dev/null", "GIT_CONFIG_NOSYSTEM": "1", +# No GIT_ASKPASS: no askpass helper ships in the pod, and none is needed — +# authenticated SCM traffic goes through the credential-isolated broker. +FIXED_ENVIRONMENT = {"GIT_CONFIG_GLOBAL": "/dev/null", "GIT_CONFIG_NOSYSTEM": "1", "GIT_TERMINAL_PROMPT": "0", "GITEA_BASE_URL": "https://scm.bstein.dev", "LC_ALL": "C", "PATH": SAFE_PATH} EXPECTED_PATHS = { @@ -61,7 +64,10 @@ EXPECTED_SHA256 = { "git": {"a0e562e4bd3c4c79379e91d8c07a10104b2cefe8fac966dc6bd4874a57a807f3"}, "sh": {"367967c823a0c391e5049b15a67c6a0a629c88b9b6dcdca75ef13ac9d65334b1"}, "hermes": {"c8419290ef7f1a95f59eadcea9f58f446b1e6535a7f4ffee9f1c0cb1172335e4"}, - GITEA_CLIENT: {"f0943db4e81ce96908e94b0f0e78aa4b5ef95d6c4f7803f9f74a43e761576d85"}, + # sha256 of services/hermes/scm-common/scripts/gitea_api.py: the ConfigMap + # hermes-scm-boundary-v2 mounts that exact file at /opt/scm/gitea_api.py, + # so this pin is derivable from merged source and equal to the deployed one. + GITEA_CLIENT: {"76efd16dedbeb74425b12fbbdbfaa391854771292077e0463bf22706855ae6dc"}, } DEADLINE_ERROR = "deadline-exceeded" @@ -194,7 +200,6 @@ def build_environment( environment.update(vantage.env) environment["PATH"] = SAFE_PATH environment["GITEA_BASE_URL"] = FIXED_ENVIRONMENT["GITEA_BASE_URL"] - environment["GIT_ASKPASS"] = FIXED_ENVIRONMENT["GIT_ASKPASS"] return environment diff --git a/scripts/ops/hermes_handoff_policy.py b/scripts/ops/hermes_handoff_policy.py index 11c8bc1c..3e569f02 100644 --- a/scripts/ops/hermes_handoff_policy.py +++ b/scripts/ops/hermes_handoff_policy.py @@ -21,7 +21,7 @@ READ_ONLY = "read-only" ARMED = "ephemeral-armed" MODES = (READ_ONLY, ARMED) -GITEA_CLIENT = "/opt/coordinator/gitea_api.py" +GITEA_CLIENT = "/opt/scm/gitea_api.py" EXPECTED_REPO = "atlas/titan-iac" EXPECTED_REMOTE = "origin" EXPECTED_BASE = "main" @@ -41,7 +41,7 @@ TRUSTED_EXECUTABLE_ROOTS = ( "/usr/local/bin/", "/opt/data/tools/bin/", "/opt/hermes/.venv/bin/", - "/opt/coordinator/", + "/opt/scm/", ) TRUSTED_INNER_PATHS = { "/bin/sh": "sh", @@ -129,7 +129,6 @@ GIT_UNSAFE_FLAGS = ( "--upload-pack", "--receive-pack", ) -GITEA_READ_METHODS = frozenset({"GET"}) HERMES_READ_COMMANDS = frozenset( {("kanban", "list"), ("kanban", "show"), ("sessions", "list"), ("status",)} ) @@ -255,7 +254,7 @@ def positionals(argv: tuple[str, ...], start: int = 1) -> Iterator[str]: def _binary_name(value: str) -> str: if value == GITEA_CLIENT or ( - value.startswith("/opt/coordinator/") and value.endswith("/gitea_api.py") + value.startswith("/opt/scm/") and value.endswith("/gitea_api.py") ): return GITEA_CLIENT if "/" not in value: @@ -417,9 +416,15 @@ def _reject_dot_segments(path: str) -> None: def _check_gitea(argv: tuple[str, ...], mode: str) -> None: + """Admit the deployed broker client's only read grammar: ``read ``. + + Bare HTTP methods are refused before spawn. The armed mutation windows + below belong to the deferred armed mode, whose forge mutations the + deployed broker-routed client cannot execute yet. + """ if len(argv) < 3: - raise PolicyError("gitea_api.py requires a method and a path") - method, path = argv[1].upper(), argv[2] + raise PolicyError("gitea_api.py requires an operation and a path") + operation, path = argv[1], argv[2] if not path.startswith("/api/v1/") or not _SAFE_API_PATH_RE.fullmatch(path): raise PolicyError("gitea_api.py requires a bounded API path") _reject_dot_segments(path) @@ -430,13 +435,15 @@ def _check_gitea(argv: tuple[str, ...], mode: str) -> None: and not path.startswith((f"{repo_root}/", f"{repo_root}?")) ): raise PolicyError("gitea_api.py is pinned to the Atlas titan-iac repository") - if method in GITEA_READ_METHODS: + if operation == "read": + if len(argv) != 3: + raise PolicyError("gitea_api.py read takes exactly one API path") return if mode == ARMED: fields = [ argv[index + 1] for index, item in enumerate(argv[:-1]) if item == "--field" ] - if method == "POST" and path == f"{repo_root}/pulls" and _ARMED_REF: + if operation == "POST" and path == f"{repo_root}/pulls" and _ARMED_REF: required = { "title=WIP: Hermes handoff acceptance ephemeral probe", f"head={_ARMED_REF}", @@ -447,20 +454,20 @@ def _check_gitea(argv: tuple[str, ...], mode: str) -> None: return match = re.fullmatch(rf"{re.escape(repo_root)}/pulls/([1-9][0-9]*)", path) if ( - method == "PATCH" + operation == "PATCH" and match and int(match.group(1)) in _ARMED_PULLS and fields == ["state=closed"] ): return if ( - method == "DELETE" + operation == "DELETE" and _ARMED_REF and path == f"{repo_root}/branches/{_ARMED_REF}" and len(argv) == 3 ): return - raise PolicyError(f"gitea_api.py {method} is not permitted in {mode} mode") + raise PolicyError(f"gitea_api.py {operation} is not permitted in {mode} mode") def _check_shell(argv: tuple[str, ...]) -> None: diff --git a/scripts/ops/hermes_handoff_rules.py b/scripts/ops/hermes_handoff_rules.py index 5cca2c5a..7a32d474 100644 --- a/scripts/ops/hermes_handoff_rules.py +++ b/scripts/ops/hermes_handoff_rules.py @@ -28,7 +28,14 @@ DENIAL_MARKERS = ( "cannot delete", "cannot patch", "not permitted", - "gitea api returned http 403", + # The deployed broker client's real refusal lines. The broker rejects a + # disallowed read with HTTP 400 and an upstream authorization failure + # surfaces as HTTP 403; a generic 404 is deliberately not a denial. The + # last line is the client's own fail-closed rejection, which never names + # a credential. + "scm broker request failed with http 400", + "scm broker request failed with http 403", + "forgejo request rejected or unavailable; no credential was disclosed", ) diff --git a/services/hermes/scm-common/scripts/gitea_api.py b/services/hermes/scm-common/scripts/gitea_api.py index 3a7de241..310fff4c 100644 --- a/services/hermes/scm-common/scripts/gitea_api.py +++ b/services/hermes/scm-common/scripts/gitea_api.py @@ -98,7 +98,14 @@ def _split_api_path(path: str) -> urllib.parse.SplitResult: def _authorize_read( target: urllib.parse.SplitResult, *, forbidden: tuple[str, ...] = () ) -> str: - """Allow only repository, PR, branch, commit, and status metadata reads.""" + """Allow only identity, repository, PR, branch, commit, and status reads.""" + if target.path == "/api/v1/user": + # The broker holds the only forge credential, so this answers with the + # broker identity's own account metadata (name, is_admin) and nothing + # else; acceptance tooling uses it to prove that identity holds no + # administrative rights. No query, no other /user route. + _validate_query(target, set()) + return "identity" prefix = "/api/v1/repos/atlas/" remainder = target.path.removeprefix(prefix) if remainder == target.path: diff --git a/testing/quality_handoff_mutation.py b/testing/quality_handoff_mutation.py index 95d2df42..862f1b0c 100644 --- a/testing/quality_handoff_mutation.py +++ b/testing/quality_handoff_mutation.py @@ -156,7 +156,7 @@ MUTATIONS = ( ' and not path.startswith((f"{repo_root}/", f"{repo_root}?"))\n', " and not path.startswith(repo_root)\n", "from hermes_handoff_policy import *\n" - "try: check_argv((GITEA_CLIENT,'GET','/api/v1/repos/atlas/titan-iac-evil/pulls'))\n" + "try: check_argv((GITEA_CLIENT,'read','/api/v1/repos/atlas/titan-iac-evil/pulls'))\n" "except PolicyError: raise SystemExit(0)\n" "raise SystemExit(1)\n", ), diff --git a/testing/tests/test_hermes_gitea_pr_client.py b/testing/tests/test_hermes_gitea_pr_client.py index 4ddb9830..53b8c87d 100644 --- a/testing/tests/test_hermes_gitea_pr_client.py +++ b/testing/tests/test_hermes_gitea_pr_client.py @@ -131,6 +131,7 @@ def test_noncanonical_round_trip_or_segments_never_reach_opener(path: str): @pytest.mark.parametrize( "path", [ + "/api/v1/user", "/api/v1/repos/atlas/cassandra", "/api/v1/repos/atlas/cassandra/pulls?state=open&limit=20&page=1", "/api/v1/repos/atlas/cassandra/pulls/7", @@ -154,6 +155,26 @@ def test_explicit_read_allowlist_accepts_only_engineering_metadata(path: str): assert request.method == "GET" +def test_the_identity_route_reads_only_the_bare_user_document(): + """`/api/v1/user` answers with the broker identity's own metadata. + + Acceptance tooling uses it to prove the platform's forge identity is not + an administrator; every sibling or sub-route stays refused. + """ + client = _load() + + assert client.authorize_request("GET", "/api/v1/user", None) == "identity" + for path in ( + "/api/v1/user?full=true", + "/api/v1/user/tokens", + "/api/v1/user/emails", + "/api/v1/users/atlas", + "/api/v1/userx", + ): + with pytest.raises(client.PolicyError): + client.authorize_request("GET", path, None) + + @pytest.mark.parametrize( "path", [ diff --git a/testing/tests/test_hermes_handoff_acceptance.py b/testing/tests/test_hermes_handoff_acceptance.py index de4a411c..3ef98323 100644 --- a/testing/tests/test_hermes_handoff_acceptance.py +++ b/testing/tests/test_hermes_handoff_acceptance.py @@ -341,7 +341,7 @@ def test_an_armed_run_does_not_mutate_until_read_only_acceptance_is_go( ("PATCH", completed()), ("DELETE", completed()), ( - "GET /api/v1/repos/atlas/titan-iac/pulls/9", + "read /api/v1/repos/atlas/titan-iac/pulls/9", completed(stdout='{"state": "closed"}'), ), *HEALTHY_CLUSTER, diff --git a/testing/tests/test_hermes_handoff_exec.py b/testing/tests/test_hermes_handoff_exec.py index fb54d3a5..b9bd24e3 100644 --- a/testing/tests/test_hermes_handoff_exec.py +++ b/testing/tests/test_hermes_handoff_exec.py @@ -315,6 +315,36 @@ def test_descendant_holding_pipes_is_bounded_after_the_leader_exits() -> None: assert time.monotonic() - started < 1.5 +def test_the_scm_client_pin_is_derivable_from_the_merged_source() -> None: + """The attested client digest must equal the ConfigMap-generated source. + + ``/opt/scm/gitea_api.py`` is mounted from ConfigMap ``hermes-scm-boundary-v2``, + which is generated verbatim from ``services/hermes/scm-common/scripts/ + gitea_api.py``. Pinning anything else would make the harness unable to + attest the client that actually ships in the agent pod. + """ + from pathlib import Path + + client = policy.GITEA_CLIENT + assert client == "/opt/scm/gitea_api.py" + assert runner_module.EXPECTED_PATHS[client] == {client} + assert runner_module.POD_COMMAND_PATHS[client] == client + source = ( + Path(__file__).resolve().parents[2] + / "services/hermes/scm-common/scripts/gitea_api.py" + ) + digest = hashlib.sha256(source.read_bytes()).hexdigest() + assert runner_module.EXPECTED_SHA256[client] == {digest} + + +def test_no_phantom_askpass_or_coordinator_path_is_configured() -> None: + assert "GIT_ASKPASS" not in runner_module.FIXED_ENVIRONMENT + assert "/opt/coordinator" not in runner_module.SAFE_PATH + assert "/opt/scm" in runner_module.SAFE_PATH.split(":") + environment = runner_module.build_environment(OPERATOR, {}) + assert "GIT_ASKPASS" not in environment + + def test_attestation_rejects_a_reviewer_authored_executable(tmp_path) -> None: fake = tmp_path / "kubectl" fake.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") diff --git a/testing/tests/test_hermes_handoff_policy.py b/testing/tests/test_hermes_handoff_policy.py index 41f1fc7e..c9dbe5ba 100644 --- a/testing/tests/test_hermes_handoff_policy.py +++ b/testing/tests/test_hermes_handoff_policy.py @@ -31,7 +31,9 @@ def check(*argv: str, mode: str | None = None) -> None: ("hermes", "kanban", "list", "--json"), ("hermes", "sessions", "list", "--source", "telegram"), ("hermes", "status"), - (policy.GITEA_CLIENT, "GET", "/api/v1/user"), + (policy.GITEA_CLIENT, "read", "/api/v1/user"), + (policy.GITEA_CLIENT, "read", "/api/v1/repos/atlas/titan-iac/pulls/19"), + ("/opt/scm/gitea_api.py", "read", "/api/v1/repos/atlas/titan-iac"), ], ) def test_read_only_commands_are_permitted(argv: tuple[str, ...]) -> None: @@ -77,9 +79,15 @@ def test_read_only_commands_are_permitted(argv: tuple[str, ...]) -> None: ("hermes",), (policy.GITEA_CLIENT, "POST", "/api/v1/x"), (policy.GITEA_CLIENT, "GET"), + (policy.GITEA_CLIENT, "GET", "/api/v1/user"), + (policy.GITEA_CLIENT, "GET", "/api/v1/repos/atlas/titan-iac"), + (policy.GITEA_CLIENT, "read", "/api/v1/user", "extra"), + (policy.GITEA_CLIENT, "create-draft", "/api/v1/repos/atlas/titan-iac/pulls"), + ("/opt/coordinator/gitea_api.py", "read", "/api/v1/user"), ("sh", "-c", "rm -rf /"), ("sh", "echo"), ("git", "log", "/etc/shadow"), + ("git",), ], ) def test_unsafe_commands_are_refused(argv: tuple[str, ...]) -> None: @@ -166,7 +174,7 @@ def test_the_repository_pin_has_an_exact_boundary_and_rejects_dot_segments() -> "/api/v1/repos/atlas/titan-iac../pulls", ): with pytest.raises(policy.PolicyError): - check(policy.GITEA_CLIENT, "GET", path) + check(policy.GITEA_CLIENT, "read", path) for path in ( "/api/v1/repos/atlas/titan-iac/../../user/tokens", "/api/v1/repos/atlas/titan-iac/%2e%2e/%2e%2e/user/tokens", @@ -175,9 +183,26 @@ def test_the_repository_pin_has_an_exact_boundary_and_rejects_dot_segments() -> "/api/v1/repos/atlas/titan-iac//pulls", ): with pytest.raises(policy.PolicyError, match="relative segments"): - check(policy.GITEA_CLIENT, "GET", path) - check(policy.GITEA_CLIENT, "GET", "/api/v1/repos/atlas/titan-iac") - check(policy.GITEA_CLIENT, "GET", "/api/v1/repos/atlas/titan-iac/pulls?state=all") + check(policy.GITEA_CLIENT, "read", path) + check(policy.GITEA_CLIENT, "read", "/api/v1/repos/atlas/titan-iac") + check(policy.GITEA_CLIENT, "read", "/api/v1/repos/atlas/titan-iac/pulls?state=all") + + +def test_the_client_grammar_is_read_not_http_methods_in_every_mode() -> None: + """The deployed broker client speaks ``read ``, never a bare method.""" + for mode in (policy.READ_ONLY, policy.ARMED): + check(policy.GITEA_CLIENT, "read", "/api/v1/user", mode=mode) + with pytest.raises(policy.PolicyError, match="not permitted"): + check(policy.GITEA_CLIENT, "GET", "/api/v1/user", mode=mode) + with pytest.raises(policy.PolicyError, match="exactly one API path"): + check(policy.GITEA_CLIENT, "read", "/api/v1/user", "--field", "x=y") + + +def test_the_client_is_pinned_to_the_scm_boundary_mount() -> None: + assert policy.GITEA_CLIENT == "/opt/scm/gitea_api.py" + assert "/opt/scm/" in policy.TRUSTED_EXECUTABLE_ROOTS + assert "/opt/coordinator/" not in policy.TRUSTED_EXECUTABLE_ROOTS + assert policy.TRUSTED_INNER_PATHS[policy.GITEA_CLIENT] == policy.GITEA_CLIENT def test_every_allowlisted_binary_is_release_attestable() -> None: @@ -267,7 +292,6 @@ def test_arming_and_projection_registration_fail_closed() -> None: def test_every_frozen_template_renders_and_is_accepted() -> None: parameters = { "account": "hermes-agent", - "askpass": "/opt/coordinator/gitea_askpass.sh", "fields": "state,model", "limit": "200", "path": "/host-etc/passwd", @@ -313,8 +337,8 @@ def test_a_subcommand_must_precede_its_resource_arguments() -> None: ("kubectl", "get", "pods", "-o", "jsonpath={.spec.containers[*].env[*].value}"), ("git", "-C", "/tmp", "status"), ("git", "--work-tree=/tmp", "status"), - (policy.GITEA_CLIENT, "GET", "not-an-api-path"), - (policy.GITEA_CLIENT, "GET", "/api/v1/repos/atlas/other"), + (policy.GITEA_CLIENT, "read", "not-an-api-path"), + (policy.GITEA_CLIENT, "read", "/api/v1/repos/atlas/other"), ], ) def test_provenance_projection_and_repository_bypasses_are_rejected(argv) -> None: diff --git a/testing/tests/test_hermes_handoff_rules.py b/testing/tests/test_hermes_handoff_rules.py index 03d5785f..76a9df17 100644 --- a/testing/tests/test_hermes_handoff_rules.py +++ b/testing/tests/test_hermes_handoff_rules.py @@ -91,7 +91,17 @@ def test_an_optional_step_may_be_absent() -> None: [ ("Error from server (Forbidden): pods is forbidden", 1, True), ("User cannot list resource", 1, True), - ("Gitea API returned HTTP 403", 1, True), + # The deployed broker client's two real refusal lines. + ("SCM broker request failed with HTTP 400", 1, True), + ("SCM broker request failed with HTTP 403", 1, True), + ( + "Forgejo request rejected or unavailable; no credential was disclosed", + 1, + True, + ), + # Lines the deployed client never emits, or that are not refusals. + ("Gitea API returned HTTP 403", 1, False), + ("SCM broker request failed with HTTP 404", 1, False), ("connection refused", 1, False), ("", 0, False), ],