From 762784da6b558eedce5ec3e3993fdf42e79a97ec Mon Sep 17 00:00:00 2001 From: jenkins Date: Sun, 16 Aug 2026 19:54:41 -0300 Subject: [PATCH] hermes: bound Atlas pull request client --- services/hermes/agent-configmap.yaml | 30 +- services/hermes/agent-deployment.yaml | 9 + services/hermes/kustomization.yaml | 7 + services/hermes/scripts/gitea_api.py | 422 ++++++++++++++---- .../manage-atlas-pull-requests/SKILL.md | 58 +++ .../agents/openai.yaml | 4 + testing/tests/test_hermes_chat_quality.py | 8 +- testing/tests/test_hermes_gitea_pr_client.py | 322 +++++++++++++ testing/tests/test_hermes_runtime_access.py | 40 +- 9 files changed, 784 insertions(+), 116 deletions(-) mode change 100644 => 100755 services/hermes/scripts/gitea_api.py create mode 100644 services/hermes/skills/manage-atlas-pull-requests/SKILL.md create mode 100644 services/hermes/skills/manage-atlas-pull-requests/agents/openai.yaml create mode 100644 testing/tests/test_hermes_gitea_pr_client.py diff --git a/services/hermes/agent-configmap.yaml b/services/hermes/agent-configmap.yaml index 0b81379d..b35da59a 100644 --- a/services/hermes/agent-configmap.yaml +++ b/services/hermes/agent-configmap.yaml @@ -315,15 +315,21 @@ data: real branch only when the requested implementation is review-ready. Never force-push. - `scm.bstein.dev` is Gitea, not GitHub. Never load or follow a GitHub/`gh` - skill for an Atlas remote, and do not interpret an unauthenticated Gitea - HTTP 404 as a missing private repository. Use authenticated Git for clone, - fetch, and push. For pull-request metadata, comments, and merges, call - `/opt/coordinator/gitea_api.py METHOD /api/v1/...`; it injects the runtime - Vault token without exposing it to the command line, environment, output, - or transcript. Before a consequential merge, independently verify the - exact base/head ancestry and diff, run the relevant tests, and confirm the - candidate Jenkins result. Prefer the internal endpoint in + `scm.bstein.dev` is Forgejo/Gitea, not GitHub. Never load or follow a + GitHub/`gh` skill for an Atlas remote, and do not interpret an + unauthenticated Gitea HTTP 404 as a missing private repository. Use + authenticated Git for clone, fetch, and non-force push. For repository and + pull-request reads, or to create/update a review-ready draft PR, load + `$manage-atlas-pull-requests` and use `/opt/coordinator/gitea_api.py`. The + client injects the runtime Vault token without exposing it to the command + line, environment, output, or transcript. It permits only Atlas-scoped + reads, same-repository draft creation, and title/body updates while the PR + remains draft. Merge, approve, close, delete, comments, repository + administration, and force-push are unavailable; never bypass the client + with raw HTTP. Leave every PR unmerged for Brad's review. Independently + verify the exact base/head ancestry and diff, run the relevant tests, and + confirm the candidate Jenkins result before presenting a handoff. Prefer + the internal endpoint in `JENKINS_BASE_URL`; when Jenkins API authorization prevents a read, use the existing cluster access to inspect the controller's job/build files and logs rather than guessing a public hostname. The preferred read-only path @@ -331,10 +337,8 @@ data: [--commit SHA] --wait`; it distinguishes a genuinely terminal build from nested Jenkins execution metadata and returns bounded JSON/log evidence. With a branch, the helper also tries the conventional `JOB-branches` - multibranch name. For JSON Gitea mutations, prefer repeatable shell-safe - `--field KEY=VALUE` arguments over hand-quoted JSON. - After merging, observe the default-branch build to a terminal result and - report exact commit, build, and test evidence. + multibranch name. After Brad merges, observe the default-branch build to a + terminal result and report exact commit, build, and test evidence. The workspace already has a non-secret Hermes Git author identity. Do not use `git reset --hard`, even in a fresh clone; use a detached worktree or a diff --git a/services/hermes/agent-deployment.yaml b/services/hermes/agent-deployment.yaml index 5ed758cf..4b93f92d 100644 --- a/services/hermes/agent-deployment.yaml +++ b/services/hermes/agent-deployment.yaml @@ -663,6 +663,7 @@ spec: - {name: subprocess-secret-patch, mountPath: /opt/hermes/tools/process_registry.py, subPath: process_registry.py} - {name: kubeconfig, mountPath: /opt/data/home/.kube/config, subPath: config, readOnly: true} - {name: auto-router-plugin, mountPath: /opt/data/plugins/auto-router, readOnly: true} + - {name: atlas-pr-skill, mountPath: /opt/data/workspace/skills/manage-atlas-pull-requests, readOnly: true} - {name: routing-catalog, mountPath: /routing-catalog, readOnly: true} - {name: tmp, mountPath: /tmp} startupProbe: @@ -826,6 +827,7 @@ spec: - {name: subprocess-secret-patch, mountPath: /opt/hermes/tools/process_registry.py, subPath: process_registry.py} - {name: kubeconfig, mountPath: /opt/data/home/.kube/config, subPath: config, readOnly: true} - {name: auto-router-plugin, mountPath: /opt/data/plugins/auto-router, readOnly: true} + - {name: atlas-pr-skill, mountPath: /opt/data/workspace/skills/manage-atlas-pull-requests, readOnly: true} - {name: routing-catalog, mountPath: /routing-catalog, readOnly: true} - {name: tmp, mountPath: /tmp} - {name: ttyd-index, mountPath: /ttyd-index, readOnly: true} @@ -894,6 +896,7 @@ spec: - {name: auth-patch, mountPath: /opt/hermes/hermes_cli/auth.py, subPath: auth.py} - {name: kubeconfig, mountPath: /opt/data/home/.kube/config, subPath: config, readOnly: true} - {name: auto-router-plugin, mountPath: /opt/data/plugins/auto-router, readOnly: true} + - {name: atlas-pr-skill, mountPath: /opt/data/workspace/skills/manage-atlas-pull-requests, readOnly: true} - {name: tmp, mountPath: /tmp} resources: requests: {cpu: 100m, memory: 256Mi} @@ -1188,6 +1191,12 @@ spec: - {key: dashboard-api.py, path: dashboard/plugin_api.py} - {key: dashboard-index.js, path: dashboard/dist/index.js} - {key: dashboard-style.css, path: dashboard/dist/style.css} + - name: atlas-pr-skill + configMap: + name: hermes-atlas-pr-skill + items: + - {key: SKILL.md, path: SKILL.md} + - {key: openai.yaml, path: agents/openai.yaml} - name: image-policy configMap: name: hermes-image-policy diff --git a/services/hermes/kustomization.yaml b/services/hermes/kustomization.yaml index 3b9c2345..87984530 100644 --- a/services/hermes/kustomization.yaml +++ b/services/hermes/kustomization.yaml @@ -158,3 +158,10 @@ configMapGenerator: - alert-review.md=skills/tune-atlas-alerts/references/alert-review.md options: disableNameSuffixHash: true + - name: hermes-atlas-pr-skill + namespace: hermes + files: + - SKILL.md=skills/manage-atlas-pull-requests/SKILL.md + - openai.yaml=skills/manage-atlas-pull-requests/agents/openai.yaml + options: + disableNameSuffixHash: true diff --git a/services/hermes/scripts/gitea_api.py b/services/hermes/scripts/gitea_api.py old mode 100644 new mode 100755 index 5544c398..73a2bffd --- a/services/hermes/scripts/gitea_api.py +++ b/services/hermes/scripts/gitea_api.py @@ -1,43 +1,158 @@ #!/usr/bin/env python3 -"""Call the private Atlas Gitea API through a runtime-only token boundary.""" +"""Use the private Atlas Forgejo API through a least-authority PR boundary.""" from __future__ import annotations import argparse import json import os +import re import sys import urllib.error import urllib.parse import urllib.request +from collections.abc import Callable from pathlib import Path - -DEFAULT_BASE_URL = "https://scm.bstein.dev" +CANONICAL_BASE_URL = "https://scm.bstein.dev" +ALLOWED_OWNER = "atlas" DEFAULT_TOKEN_FILE = Path("/runtime-access/gitea-token") -ALLOWED_METHODS = ("GET", "POST", "PUT", "PATCH", "DELETE") +REPO_RE = re.compile(r"[A-Za-z0-9][A-Za-z0-9._-]{0,99}\Z") +PR_PATH_RE = re.compile(r"/api/v1/repos/atlas/([^/]+)/pulls/([1-9][0-9]*)\Z") +CREATE_PATH_RE = re.compile(r"/api/v1/repos/atlas/([^/]+)/pulls\Z") +READ_PATH_RE = re.compile(r"/api/v1/repos/atlas/([^/]+)(?:/.*)?\Z") +SAFE_REF_RE = re.compile(r"[A-Za-z0-9][A-Za-z0-9._/-]{0,199}\Z") +MAX_ERROR_BYTES = 8192 +DRAFT_TITLE_PREFIX = "WIP: " + + +class PolicyError(ValueError): + """Raised when a requested Forgejo operation exceeds the safe boundary.""" def read_token(path: Path = DEFAULT_TOKEN_FILE) -> str: - """Read and validate the token from its in-memory Vault projection.""" + """Read the token from the pod-lifetime Vault projection.""" token = path.read_text(encoding="utf-8").strip() if not token: raise ValueError(f"runtime credential is empty: {path}") return token -def api_url(base_url: str, path: str) -> str: - """Return a same-origin Gitea API URL for a validated API path.""" - base = urllib.parse.urlsplit(base_url.rstrip("/")) +def configured_base_url() -> str: + """Reject attempts to redirect credentials away from the canonical origin.""" + configured = os.environ.get("GITEA_BASE_URL", CANONICAL_BASE_URL).rstrip("/") + if configured != CANONICAL_BASE_URL: + raise PolicyError("Forgejo origin is fixed to the private Atlas SCM service") + return configured + + +def _validate_repo(repo: str) -> str: + if not REPO_RE.fullmatch(repo) or repo in {".", ".."}: + raise PolicyError("repository name is outside the Atlas allowlist") + return repo + + +def _validate_ref(value: object, name: str) -> str: + if not isinstance(value, str) or not SAFE_REF_RE.fullmatch(value): + raise PolicyError(f"{name} must be a same-repository branch name") + if ( + value.startswith(".") + or value.endswith(("/", ".", ".lock")) + or "//" in value + or ".." in value + or "@{" in value + ): + raise PolicyError(f"{name} must be a safe same-repository branch name") + return value + + +def _validate_text(value: object, name: str, maximum: int, *, required: bool) -> str: + if not isinstance(value, str): + raise PolicyError(f"{name} must be text") + if required and not value.strip(): + raise PolicyError(f"{name} must not be empty") + if len(value) > maximum or "\x00" in value: + raise PolicyError(f"{name} exceeds the safe request limit") + return value + + +def _draft_title(value: object) -> str: + """Return a bounded title using Gitea's configured default draft prefix.""" + title = _validate_text(value, "title", 251, required=True).strip() + for prefix in ("WIP:", "[WIP]"): + if title.upper().startswith(prefix): + title = title[len(prefix) :].lstrip() + break + if not title: + raise PolicyError("title must contain text after the draft prefix") + return DRAFT_TITLE_PREFIX + title + + +def _split_api_path(path: str) -> urllib.parse.SplitResult: target = urllib.parse.urlsplit(path) - if base.scheme not in {"http", "https"} or not base.netloc: - raise ValueError("GITEA_BASE_URL must be an absolute HTTP(S) URL") if target.scheme or target.netloc or target.fragment: - raise ValueError("API path must be relative to the configured Gitea origin") + raise PolicyError("API path must be relative to the Atlas SCM origin") if not target.path.startswith("/api/v1/"): - raise ValueError("API path must start with /api/v1/") + raise PolicyError("API path must start with /api/v1/") + if "%" in target.path or "//" in target.path or "/../" in f"{target.path}/": + raise PolicyError("encoded or non-canonical API paths are not allowed") + return target + + +def api_url(base_url: str, path: str) -> str: + """Return a canonical same-origin URL without forwarding credentials.""" + if base_url.rstrip("/") != CANONICAL_BASE_URL: + raise PolicyError("Forgejo origin is fixed to the private Atlas SCM service") + target = _split_api_path(path) return urllib.parse.urlunsplit( - (base.scheme, base.netloc, target.path, target.query, "") + ("https", "scm.bstein.dev", target.path, target.query, "") + ) + + +def authorize_request(method: str, path: str, data: object | None) -> str: + """Validate one request and return its bounded operation name.""" + normalized_method = method.upper() + target = _split_api_path(path) + if normalized_method == "GET": + if data is not None: + raise PolicyError("read operations cannot include a request body") + match = READ_PATH_RE.fullmatch(target.path) + if not match: + raise PolicyError("reads are limited to repositories owned by atlas") + _validate_repo(match.group(1)) + return "read" + + if target.query: + raise PolicyError("mutating operations cannot include query parameters") + if normalized_method == "POST": + match = CREATE_PATH_RE.fullmatch(target.path) + if not match: + raise PolicyError("POST is limited to creating an Atlas draft pull request") + _validate_repo(match.group(1)) + if not isinstance(data, dict) or set(data) != {"base", "body", "head", "title"}: + raise PolicyError("draft creation accepts only base, body, head, and title") + _validate_ref(data["base"], "base") + _validate_ref(data["head"], "head") + if data["title"] != _draft_title(data["title"]): + raise PolicyError("new pull requests must use the Gitea draft-title prefix") + _validate_text(data["body"], "body", 65536, required=False) + return "create-draft" + + if normalized_method == "PATCH": + match = PR_PATH_RE.fullmatch(target.path) + if not match: + raise PolicyError("PATCH is limited to Atlas draft pull-request metadata") + _validate_repo(match.group(1)) + if not isinstance(data, dict) or not data or not set(data) <= {"body", "title"}: + raise PolicyError("draft updates accept only title and body") + if "title" in data and data["title"] != _draft_title(data["title"]): + raise PolicyError("updated pull-request titles must preserve draft state") + if "body" in data: + _validate_text(data["body"], "body", 65536, required=False) + return "update-draft" + + raise PolicyError( + "method is not available: merge, approve, close, delete, and force-push are outside this client" ) @@ -49,100 +164,255 @@ def build_request( token: str, data: object | None = None, ) -> urllib.request.Request: - """Build one authenticated request without placing the token in its URL.""" + """Build an authorized request without putting the token in its URL or body.""" + authorize_request(method, path, data) payload = None if data is not None: payload = json.dumps(data, separators=(",", ":")).encode("utf-8") return urllib.request.Request( api_url(base_url, path), data=payload, - method=method, + method=method.upper(), headers={ "Accept": "application/json", "Authorization": f"token {token}", "Content-Type": "application/json", - "User-Agent": "hermes-atlas-operator/1", + "User-Agent": "hermes-atlas-pr-client/1", }, ) -def parse_args(argv: list[str] | None = None) -> argparse.Namespace: - """Parse a bounded method, API path, and optional JSON request body.""" - parser = argparse.ArgumentParser( - description="Call the Atlas Gitea API using the runtime Vault token." +def redact_bytes(value: bytes, token: str) -> bytes: + """Remove exact runtime-token and authorization-header values from output.""" + redacted = value.replace(token.encode("utf-8"), b"[REDACTED]") + return re.sub( + rb"(?i)(authorization\s*[:=]\s*)(?:token|bearer)?\s*[^\s,;\"}]+", + rb"\1[REDACTED]", + redacted, ) - parser.add_argument("method", choices=ALLOWED_METHODS) - parser.add_argument("path", help="Gitea path beginning with /api/v1/") - data_group = parser.add_mutually_exclusive_group() - data_group.add_argument("--data-json", help="JSON object/array request body") - data_group.add_argument( - "--data-file", type=Path, help="path to a JSON request body" - ) - data_group.add_argument( - "--field", - action="append", - metavar="KEY=VALUE", - help=( - "repeatable JSON field; bare text remains a string while true, false, " - "null, numbers, objects, and arrays are decoded as JSON" - ), - ) - return parser.parse_args(argv) -def parse_fields(values: list[str]) -> dict[str, object]: - """Build a JSON object from shell-safe, repeatable key/value arguments.""" - result: dict[str, object] = {} - for value in values: - key, separator, raw = value.partition("=") - if not separator or not key or any(char.isspace() for char in key): - raise ValueError("each --field must be KEY=VALUE with a non-space key") - try: - result[key] = json.loads(raw) - except json.JSONDecodeError: - result[key] = raw +def _request( + method: str, + path: str, + data: object | None, + *, + token: str, + opener: Callable[..., object] = urllib.request.urlopen, +) -> bytes: + request = build_request( + method, + path, + base_url=configured_base_url(), + token=token, + data=data, + ) + with opener(request, timeout=30) as response: # type: ignore[attr-defined] + return redact_bytes(response.read(), token) # type: ignore[attr-defined] + + +def read( + path: str, *, token: str, opener: Callable[..., object] = urllib.request.urlopen +) -> bytes: + """Read one Atlas repository API resource.""" + return _request("GET", path, None, token=token, opener=opener) + + +def create_draft( + repo: str, + *, + base: str, + head: str, + title: str, + body: str, + token: str, + opener: Callable[..., object] = urllib.request.urlopen, +) -> bytes: + """Create a same-repository draft pull request for human review.""" + data = { + "base": base, + "body": body, + "head": head, + "title": _draft_title(title), + } + result = _request( + "POST", + f"/api/v1/repos/{ALLOWED_OWNER}/{_validate_repo(repo)}/pulls", + data, + token=token, + opener=opener, + ) + _require_draft_response(result) return result -def load_data(args: argparse.Namespace) -> object | None: - """Decode the optional JSON body without involving a shell expansion.""" - if args.data_json is not None: - return json.loads(args.data_json) - if args.data_file is not None: - return json.loads(args.data_file.read_text(encoding="utf-8")) - if args.field is not None: - return parse_fields(args.field) - return None +def _require_draft_response(value: bytes) -> dict[str, object]: + """Fail closed unless Forgejo confirms the resulting PR remains a draft.""" + try: + document = json.loads(value) + except json.JSONDecodeError as exc: + raise PolicyError("Forgejo returned invalid pull-request metadata") from exc + if not isinstance(document, dict) or document.get("draft") is not True: + raise PolicyError( + "Forgejo did not confirm draft state; human review is required" + ) + return document + + +def update_draft( + repo: str, + number: int, + updates: dict[str, str], + *, + token: str, + opener: Callable[..., object] = urllib.request.urlopen, +) -> bytes: + """Update title/body only after Forgejo confirms the PR is still a draft.""" + if not isinstance(number, int) or isinstance(number, bool) or number < 1: + raise PolicyError("pull-request number must be positive") + path = f"/api/v1/repos/{ALLOWED_OWNER}/{_validate_repo(repo)}/pulls/{number}" + current_raw = _request("GET", path, None, token=token, opener=opener) + try: + current = json.loads(current_raw) + except json.JSONDecodeError as exc: + raise PolicyError("Forgejo returned invalid pull-request metadata") from exc + if not isinstance(current, dict) or current.get("draft") is not True: + raise PolicyError("pull request is not a draft; human review now owns it") + safe_updates = dict(updates) + if "title" in safe_updates: + safe_updates["title"] = _draft_title(safe_updates["title"]) + result = _request("PATCH", path, safe_updates, token=token, opener=opener) + _require_draft_response(result) + return result + + +def _body(args: argparse.Namespace) -> str: + if args.body_file is None: + return "" + return args.body_file.read_text(encoding="utf-8") + + +def parse_args(argv: list[str] | None = None) -> argparse.Namespace: + """Parse the three intentionally narrow client operations.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--dry-run", + action="store_true", + help="validate and describe the request without reading a token or using the network", + ) + subparsers = parser.add_subparsers(dest="operation", required=True) + + read_parser = subparsers.add_parser( + "read", help="read an Atlas repository API path" + ) + read_parser.add_argument("path") + + create_parser = subparsers.add_parser( + "create-draft", help="create a same-repository draft pull request" + ) + create_parser.add_argument("repo") + create_parser.add_argument("--base", required=True) + create_parser.add_argument("--head", required=True) + create_parser.add_argument("--title", required=True) + create_parser.add_argument("--body-file", type=Path) + + update_parser = subparsers.add_parser( + "update-draft", help="update title/body of an existing draft pull request" + ) + update_parser.add_argument("repo") + update_parser.add_argument("number", type=int) + update_parser.add_argument("--title") + update_parser.add_argument("--body-file", type=Path) + return parser.parse_args(argv) + + +def _operation(args: argparse.Namespace) -> tuple[str, str, object | None]: + if args.operation == "read": + return "GET", args.path, None + repo = _validate_repo(args.repo) + if args.operation == "create-draft": + data = { + "base": args.base, + "body": _body(args), + "head": args.head, + "title": _draft_title(args.title), + } + return "POST", f"/api/v1/repos/{ALLOWED_OWNER}/{repo}/pulls", data + if args.number < 1: + raise PolicyError("pull-request number must be positive") + data = {} + if args.title is not None: + data["title"] = _draft_title(args.title) + if args.body_file is not None: + data["body"] = _body(args) + return "PATCH", f"/api/v1/repos/{ALLOWED_OWNER}/{repo}/pulls/{args.number}", data + + +def _write_body(body: bytes) -> None: + if body: + sys.stdout.buffer.write(body) + if not body.endswith(b"\n"): + sys.stdout.buffer.write(b"\n") def main(argv: list[str] | None = None) -> int: - """Execute the request, print only its response body, and return HTTP status.""" - args = parse_args(argv) + """Validate, optionally describe, and execute one safe Forgejo operation.""" + token = "" try: - request = build_request( - args.method, - args.path, - base_url=os.environ.get("GITEA_BASE_URL", DEFAULT_BASE_URL), - token=read_token(), - data=load_data(args), - ) - with urllib.request.urlopen(request, timeout=30) as response: - body = response.read() - if body: - sys.stdout.buffer.write(body) - if not body.endswith(b"\n"): - sys.stdout.buffer.write(b"\n") + args = parse_args(argv) + method, path, data = _operation(args) + operation = authorize_request(method, path, data) + if args.dry_run: + print( + json.dumps( + { + "dry_run": True, + "host": "scm.bstein.dev", + "operation": operation, + "owner": ALLOWED_OWNER, + "path": path, + "request_fields": sorted(data) + if isinstance(data, dict) + else [], + "requires_draft_preflight": operation == "update-draft", + }, + sort_keys=True, + ) + ) + return 0 + token = read_token() + if args.operation == "read": + body = read(path, token=token) + elif args.operation == "create-draft": + assert isinstance(data, dict) + body = create_draft( + args.repo, + base=str(data["base"]), + head=str(data["head"]), + title=str(data["title"]), + body=str(data["body"]), + token=token, + ) + else: + assert isinstance(data, dict) + body = update_draft(args.repo, args.number, data, token=token) + _write_body(body) return 0 except urllib.error.HTTPError as exc: - body = exc.read(65536) - print(f"Gitea API returned HTTP {exc.code}", file=sys.stderr) + body = redact_bytes(exc.read(MAX_ERROR_BYTES), token) if token else b"" + print(f"Forgejo request failed with HTTP {exc.code}", file=sys.stderr) if body: sys.stderr.buffer.write(body) if not body.endswith(b"\n"): sys.stderr.buffer.write(b"\n") return 1 - except (OSError, ValueError, json.JSONDecodeError) as exc: - print(f"Gitea API request failed: {exc}", file=sys.stderr) + except (OSError, PolicyError, ValueError, json.JSONDecodeError): + # Deliberately omit exception details. File contents, HTTP request + # objects, and provider errors can carry credential material. + print( + "Forgejo request rejected or unavailable; no credential was disclosed", + file=sys.stderr, + ) return 1 diff --git a/services/hermes/skills/manage-atlas-pull-requests/SKILL.md b/services/hermes/skills/manage-atlas-pull-requests/SKILL.md new file mode 100644 index 00000000..4e73a3cf --- /dev/null +++ b/services/hermes/skills/manage-atlas-pull-requests/SKILL.md @@ -0,0 +1,58 @@ +--- +name: manage-atlas-pull-requests +description: Read private Atlas repository and pull-request metadata, create a draft pull request after a tested branch is pushed, or update the title/body of an existing draft through Hermes' least-authority Forgejo client. Use for PR handoff in scm.bstein.dev/atlas repositories. Never use it to merge, approve, close, delete, or force-push. +--- + +# Manage Atlas pull requests + +Use `/opt/coordinator/gitea_api.py` for Forgejo API access. It reads its token +from the pod-lifetime Vault projection; never read that file, copy the token, +put it in an argument or environment variable, or replace this client with +`curl`. + +## Read repository or PR state + +Pass an Atlas repository API path to the read operation: + +```sh +/opt/coordinator/gitea_api.py read /api/v1/repos/atlas/REPO/pulls/NUMBER +``` + +Reads outside `scm.bstein.dev/atlas` are rejected. Treat returned state as +evidence, not authorization to change it. + +## Create a review handoff + +Before opening a PR, verify the remote, branch, clean worktree, diff, tests, and +exact pushed commit. Never force-push. Write the PR description to a workspace +file so multiline Markdown is not shell-quoted, then validate without reading a +credential or using the network: + +```sh +/opt/coordinator/gitea_api.py --dry-run create-draft REPO \ + --base main --head hermes/TASK --title "Focused change" \ + --body-file /tmp/pr-body.md +``` + +Run the same command without `--dry-run` only when the branch is review-ready. +The client always creates a draft in the same repository. Report its URL and +exact head commit to Brad. Leave it unmerged for human review. + +## Update an existing draft + +Use only a title, a body file, or both: + +```sh +/opt/coordinator/gitea_api.py update-draft REPO NUMBER \ + --body-file /tmp/pr-body.md +``` + +The client first reads the PR from Forgejo and refuses the update unless it is +still a draft. It never changes head/base, review state, or merge state. + +## Stop at the authority boundary + +Merge, approve, close, delete, branch mutation, comments, releases, repository +administration, and force-push are intentionally unavailable. Do not bypass the +client with a raw HTTP request. If one of those actions is needed, present the +tested commit and evidence to Brad and stop for human review. diff --git a/services/hermes/skills/manage-atlas-pull-requests/agents/openai.yaml b/services/hermes/skills/manage-atlas-pull-requests/agents/openai.yaml new file mode 100644 index 00000000..57cb8496 --- /dev/null +++ b/services/hermes/skills/manage-atlas-pull-requests/agents/openai.yaml @@ -0,0 +1,4 @@ +interface: + display_name: "Manage Atlas Pull Requests" + short_description: "Create safe draft PR handoffs for human review" + default_prompt: "Use $manage-atlas-pull-requests to inspect the requested Atlas repository or prepare a tested branch as an unmerged draft pull request. Preserve the human review boundary." diff --git a/testing/tests/test_hermes_chat_quality.py b/testing/tests/test_hermes_chat_quality.py index 818f7679..09f6b00c 100644 --- a/testing/tests/test_hermes_chat_quality.py +++ b/testing/tests/test_hermes_chat_quality.py @@ -99,9 +99,11 @@ def test_agent_config_keeps_delegated_reviewers_from_owning_task_lifecycle(): assert "Never call `kanban_show` without\na known, non-empty task ID" in soul assert "must load a skill only when its workflow\nmaterially applies" in soul assert "runtime-only `GIT_ASKPASS`" in soul - assert "`scm.bstein.dev` is Gitea, not GitHub" in instructions - assert "Never load or follow a GitHub/`gh`" in instructions - assert "/opt/coordinator/gitea_api.py METHOD /api/v1/..." in instructions + assert "`scm.bstein.dev` is Forgejo/Gitea, not GitHub" in instructions + assert "GitHub/`gh` skill for an Atlas remote" in instructions + assert "load\n`$manage-atlas-pull-requests`" in instructions + assert "Merge, approve, close, delete, comments" in instructions + assert "Leave every PR unmerged for Brad's review" in instructions assert "JENKINS_BASE_URL" in instructions assert "Do not\nuse `git reset --hard`" in instructions rendered = (HERMES / "agent-deployment.yaml").read_text() diff --git a/testing/tests/test_hermes_gitea_pr_client.py b/testing/tests/test_hermes_gitea_pr_client.py new file mode 100644 index 00000000..658af189 --- /dev/null +++ b/testing/tests/test_hermes_gitea_pr_client.py @@ -0,0 +1,322 @@ +"""Adversarial contracts for the Atlas-only draft pull-request client.""" + +from __future__ import annotations + +import importlib.util +import io +import json +import sys +import urllib.error +from pathlib import Path + +import pytest +import yaml + +ROOT = Path(__file__).parents[2] +CLIENT_PATH = ROOT / "services/hermes/scripts/gitea_api.py" + + +def _load(): + spec = importlib.util.spec_from_file_location("safe_gitea_api", CLIENT_PATH) + assert spec and spec.loader + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +def _draft_payload(**updates): + payload = { + "base": "main", + "body": "Review evidence", + "head": "hermes/review-fix", + "title": "WIP: Repair review findings", + } + payload.update(updates) + return payload + + +class Response: + def __init__(self, body: object): + self.body = body if isinstance(body, bytes) else json.dumps(body).encode() + + def __enter__(self): + return self + + def __exit__(self, *_args): + return False + + def read(self): + return self.body + + +@pytest.mark.parametrize( + ("base_url", "path"), + [ + ("https://evil.example", "/api/v1/repos/atlas/cassandra"), + ("http://scm.bstein.dev", "/api/v1/repos/atlas/cassandra"), + ("https://scm.bstein.dev:443", "/api/v1/repos/atlas/cassandra"), + ("https://scm.bstein.dev", "https://evil.example/api/v1/repos/atlas/cassandra"), + ("https://scm.bstein.dev", "/api/v1/repos/evil/cassandra"), + ("https://scm.bstein.dev", "/api/v1/repos/%61tlas/cassandra"), + ("https://scm.bstein.dev", "/api/v1/repos/atlas/../admin"), + ], +) +def test_host_owner_and_path_escape_attempts_are_rejected(base_url: str, path: str): + client = _load() + + with pytest.raises(client.PolicyError): + client.build_request("GET", path, base_url=base_url, token="secret") + + +@pytest.mark.parametrize( + ("method", "path", "data"), + [ + ("DELETE", "/api/v1/repos/atlas/cassandra/pulls/4", None), + ("POST", "/api/v1/repos/atlas/cassandra/pulls/4/merge", {}), + ( + "POST", + "/api/v1/repos/atlas/cassandra/pulls/4/reviews", + {"event": "APPROVED"}, + ), + ("PATCH", "/api/v1/repos/atlas/cassandra/pulls/4", {"state": "closed"}), + ("PATCH", "/api/v1/repos/atlas/cassandra/pulls/4", {"draft": False}), + ("PUT", "/api/v1/repos/atlas/cassandra/branches/main", {}), + ], +) +def test_merge_approve_close_delete_and_other_mutations_are_rejected( + method: str, path: str, data: object +): + client = _load() + + with pytest.raises(client.PolicyError): + client.build_request( + method, path, base_url=client.CANONICAL_BASE_URL, token="secret", data=data + ) + + +def test_create_forces_draft_title_and_same_repository_branch_names(): + client = _load() + assert ( + client.authorize_request( + "POST", "/api/v1/repos/atlas/cassandra/pulls", _draft_payload() + ) + == "create-draft" + ) + + with pytest.raises(client.PolicyError, match="draft-title prefix"): + client.authorize_request( + "POST", + "/api/v1/repos/atlas/cassandra/pulls", + _draft_payload(title="Not a draft"), + ) + with pytest.raises(client.PolicyError): + client.authorize_request( + "POST", + "/api/v1/repos/atlas/cassandra/pulls", + _draft_payload(head="someone:branch"), + ) + + +def test_update_checks_server_draft_state_before_patching(): + client = _load() + calls = [] + responses = iter( + [ + Response({"number": 7, "draft": True}), + Response({"number": 7, "draft": True}), + ] + ) + + def opener(request, timeout): + calls.append((request.method, request.full_url, request.data, timeout)) + return next(responses) + + result = client.update_draft( + "cassandra", 7, {"title": "Narrowed repair"}, token="runtime", opener=opener + ) + + assert json.loads(result) == {"number": 7, "draft": True} + assert [call[0] for call in calls] == ["GET", "PATCH"] + assert json.loads(calls[1][2]) == {"title": "WIP: Narrowed repair"} + + +def test_update_rejects_non_draft_without_sending_patch(): + client = _load() + calls = [] + + def opener(request, timeout): + calls.append((request.method, timeout)) + return Response({"number": 7, "draft": False}) + + with pytest.raises(client.PolicyError, match="human review now owns"): + client.update_draft( + "cassandra", 7, {"body": "new"}, token="runtime", opener=opener + ) + assert calls == [("GET", 30)] + + +def test_runtime_token_is_only_an_authorization_header(): + client = _load() + request = client.build_request( + "POST", + "/api/v1/repos/atlas/cassandra/pulls", + base_url=client.CANONICAL_BASE_URL, + token="do-not-leak", + data=_draft_payload(), + ) + + assert "do-not-leak" not in request.full_url + assert b"do-not-leak" not in request.data + assert request.get_header("Authorization") == "token do-not-leak" + + +def test_create_uses_live_gitea_schema_and_verifies_draft_response(): + client = _load() + calls = [] + + def opener(request, timeout): + calls.append((json.loads(request.data), timeout)) + return Response({"number": 3, "draft": True}) + + result = client.create_draft( + "cassandra", + base="main", + head="hermes/fix", + title="Focused fix", + body="Evidence", + token="runtime", + opener=opener, + ) + + assert json.loads(result)["draft"] is True + assert calls[0][0]["title"] == "WIP: Focused fix" + assert "draft" not in calls[0][0] + + +def test_create_fails_closed_when_server_does_not_confirm_draft(): + client = _load() + + with pytest.raises(client.PolicyError, match="did not confirm draft"): + client.create_draft( + "cassandra", + base="main", + head="hermes/fix", + title="Focused fix", + body="Evidence", + token="runtime", + opener=lambda *_a, **_k: Response({"number": 3, "draft": False}), + ) + + +def test_output_redaction_covers_exact_token_and_authorization_header(): + client = _load() + raw = b'{"message":"do-not-leak","debug":"Authorization: token do-not-leak"}' + redacted = client.redact_bytes(raw, "do-not-leak") + + assert b"do-not-leak" not in redacted + assert redacted.count(b"[REDACTED]") >= 1 + + +def test_dry_run_does_not_read_token_or_use_network( + tmp_path: Path, monkeypatch, capsys +): + client = _load() + body = tmp_path / "body.md" + body.write_text("Evidence only\n", encoding="utf-8") + monkeypatch.setattr(client, "read_token", lambda: pytest.fail("read token")) + monkeypatch.setattr( + client.urllib.request, "urlopen", lambda *_a, **_k: pytest.fail("network") + ) + + assert ( + client.main( + [ + "--dry-run", + "create-draft", + "cassandra", + "--base", + "main", + "--head", + "hermes/fix", + "--title", + "Repair", + "--body-file", + str(body), + ] + ) + == 0 + ) + output = json.loads(capsys.readouterr().out) + assert output["operation"] == "create-draft" + assert output["owner"] == "atlas" + assert "Evidence only" not in json.dumps(output) + + +def test_http_error_path_redacts_token(monkeypatch, capsys): + client = _load() + monkeypatch.setattr(client, "read_token", lambda: "do-not-leak") + + def fail(*_args, **_kwargs): + raise urllib.error.HTTPError( + "https://scm.bstein.dev/api/v1/repos/atlas/cassandra", + 403, + "forbidden", + {}, + io.BytesIO(b"Authorization: token do-not-leak"), + ) + + monkeypatch.setattr(client, "read", lambda *_a, **_k: fail()) + + assert client.main(["read", "/api/v1/repos/atlas/cassandra"]) == 1 + captured = capsys.readouterr() + assert "do-not-leak" not in captured.err + assert "HTTP 403" in captured.err + + +def test_flux_manifest_projects_runtime_vault_token_and_skill_only(): + client_source = CLIENT_PATH.read_text(encoding="utf-8") + assert "/runtime-access/gitea-token" in client_source + assert "GITEA_TOKEN" not in client_source + + deployment = yaml.safe_load( + (ROOT / "services/hermes/agent-deployment.yaml").read_text(encoding="utf-8") + ) + template = deployment["spec"]["template"] + annotations = template["metadata"]["annotations"] + assert annotations["vault.hashicorp.com/agent-inject-secret-gitea-token"] == ( + "kv/data/atlas/hermes/developer-gitea" + ) + runtime = next( + volume + for volume in template["spec"]["volumes"] + if volume["name"] == "runtime-access" + ) + assert runtime["emptyDir"]["medium"] == "Memory" + + expected = {"hermes", "terminal", "cli-lane-runner"} + mounted = { + container["name"] + for container in template["spec"]["containers"] + if any( + mount["name"] == "atlas-pr-skill" + and mount["mountPath"] + == "/opt/data/workspace/skills/manage-atlas-pull-requests" + and mount.get("readOnly") is True + for mount in container.get("volumeMounts", []) + ) + } + assert mounted == expected + + kustomization = yaml.safe_load( + (ROOT / "services/hermes/kustomization.yaml").read_text(encoding="utf-8") + ) + generator = next( + item + for item in kustomization["configMapGenerator"] + if item["name"] == "hermes-atlas-pr-skill" + ) + assert generator["files"] == [ + "SKILL.md=skills/manage-atlas-pull-requests/SKILL.md", + "openai.yaml=skills/manage-atlas-pull-requests/agents/openai.yaml", + ] diff --git a/testing/tests/test_hermes_runtime_access.py b/testing/tests/test_hermes_runtime_access.py index 7e455eb6..7ad5087a 100644 --- a/testing/tests/test_hermes_runtime_access.py +++ b/testing/tests/test_hermes_runtime_access.py @@ -11,7 +11,6 @@ from pathlib import Path import pytest import yaml - ROOT = Path(__file__).parents[2] HERMES = ROOT / "services" / "hermes" SCRIPTS = HERMES / "scripts" @@ -31,19 +30,23 @@ def test_gitea_api_builds_runtime_authenticated_same_origin_requests(): request = gitea_api.build_request( "POST", - "/api/v1/repos/atlas/cassandra/issues/1/comments", + "/api/v1/repos/atlas/cassandra/pulls", base_url="https://scm.bstein.dev", token="runtime-only-value", - data={"body": "reviewed"}, + data={ + "base": "main", + "body": "Ready for review.", + "head": "hermes/repair", + "title": "WIP: Repair semantic review findings", + }, ) assert isinstance(request, urllib.request.Request) - assert request.full_url == ( - "https://scm.bstein.dev/api/v1/repos/atlas/cassandra/issues/1/comments" - ) + assert request.full_url == "https://scm.bstein.dev/api/v1/repos/atlas/cassandra/pulls" assert request.method == "POST" assert request.get_header("Authorization") == "token runtime-only-value" - assert json.loads(request.data) == {"body": "reviewed"} + assert json.loads(request.data)["title"].startswith("WIP: ") + assert "draft" not in json.loads(request.data) @pytest.mark.parametrize( @@ -61,25 +64,14 @@ def test_gitea_api_rejects_foreign_or_non_api_targets(path: str): gitea_api.api_url("https://scm.bstein.dev", path) -def test_gitea_api_parses_shell_safe_fields_as_json_values(): +@pytest.mark.parametrize("method", ["DELETE", "PUT", "OPTIONS"]) +def test_gitea_api_rejects_unavailable_methods(method: str): gitea_api = _load("gitea_api") - assert gitea_api.parse_fields( - ["Do=manually-merged", "MergeCommitID=abc123", "enabled=true", "count=2"] - ) == { - "Do": "manually-merged", - "MergeCommitID": "abc123", - "enabled": True, - "count": 2, - } - - -@pytest.mark.parametrize("value", ["missing-separator", "=missing-key", "bad key=x"]) -def test_gitea_api_rejects_invalid_shell_safe_fields(value: str): - gitea_api = _load("gitea_api") - - with pytest.raises(ValueError): - gitea_api.parse_fields([value]) + with pytest.raises(gitea_api.PolicyError): + gitea_api.authorize_request( + method, "/api/v1/repos/atlas/cassandra/pulls/1", None + ) def test_jenkins_evidence_ignores_nested_execution_result():