From 61ab1f0cd55008db11e7a1e428e1b88be816e6e4 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Wed, 24 Jun 2026 17:07:01 -0400 Subject: [PATCH 1/8] fix(agent-team): dispatch via GitHub App so P3 reaches CI (run_id resolves) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The P3 dispatcher's default seams shell out to gh/git, but the R720 box has no gh and a read-only PAT with no Actions scope — so dispatch_apply_verify returned no run_id and every task parked at verify ("dispatch unresolved"). Add a GitHub-App auth path: the box mints short-lived (~1h) installation access tokens from the App private key and uses them for the three dispatch seams, removing the gh dependency. - agent_team/github_app.py (new): mint_installation_token (RS256 App JWT, iss=app_id, iat backdated 60s, exp 9 min; POST /access_tokens) + a lazy TokenProvider that caches and re-mints near expiry. Secret-safe: the JWT and token are never logged, never in an exception message, never persisted. - dispatcher.py: app_branch_pusher / app_workflow_dispatcher / app_run_locator (additive; gh/git _default_* left untouched). Push auth rides a host-scoped http.extraHeader via GIT_CONFIG_* env (token never in argv/ps); the REST run locator maps id->databaseId / created_at->createdAt into select_run_id and surfaces 4xx promptly instead of silently exhausting the poll window. - coordinator.py: default_dispatch_node_factory binds the App seams when AGENT_TEAM_GH_APP_ID / _INSTALLATION_ID / _PRIVATE_KEY are all set; partial or unreadable config logs one warning and falls back to gh-default (never raises at serve-start). - requirements.txt: pin PyJWT, cryptography, requests (App seams + CI fetcher). - DEPLOY-R720.md / README.md: App dispatch config, permission/scope audit, env-precedence check, key rotation/revocation + incident response. Tests: +18 (test_github_app.py new; dispatcher/coordinator additions) covering JWT claims, cache/re-mint, token-scrub-on-error, REST field mapping + run-name correlation, and the partial-env inert fallback. Full suite 1523 passing. --- agent-team/DEPLOY-R720.md | 88 ++++++++ agent-team/README.md | 15 +- agent-team/agent_team/coordinator.py | 83 ++++++++ agent-team/agent_team/dispatcher.py | 296 +++++++++++++++++++++++++++ agent-team/agent_team/github_app.py | 184 +++++++++++++++++ agent-team/tests/test_coordinator.py | 124 +++++++++++ agent-team/tests/test_dispatcher.py | 227 ++++++++++++++++++++ agent-team/tests/test_github_app.py | 270 ++++++++++++++++++++++++ requirements.txt | 10 + 9 files changed, 1293 insertions(+), 4 deletions(-) create mode 100644 agent-team/agent_team/github_app.py create mode 100644 agent-team/tests/test_github_app.py diff --git a/agent-team/DEPLOY-R720.md b/agent-team/DEPLOY-R720.md index 016d4e5..80d7f95 100644 --- a/agent-team/DEPLOY-R720.md +++ b/agent-team/DEPLOY-R720.md @@ -86,6 +86,54 @@ chmod 600 ~/secrev.env would silently win over the subscription OAuth and meter to API rates. The box runs on subscription OAuth only. +### 3a. P3 dispatch auth — GitHub App installation tokens (box-side) + +The P3 dispatcher (`agent_team/dispatcher.py`) carries an approved diff into org +CI: it pushes the candidate head branch and triggers the +`agent-team-apply-verify.yml` `workflow_dispatch`, then locates the resulting +run id. By default those three seams shell out to `gh`/`git`. **`gh` is not +installed on the box and the box's read-only PAT has no Actions permission**, so +the default path cannot dispatch. Instead the box authenticates with a **GitHub +App**: it holds the App private key and mints short-lived (~1h) **installation +access tokens** on demand (`agent_team/github_app.py`). Set all three of these to +turn the App path on (all-or-nothing — partial config logs one warning and falls +back to the inert gh-default path, it never crashes serve): + +``` +echo 'AGENT_TEAM_GH_APP_ID=...' >> ~/secrev.env # the App's numeric app_id +echo 'AGENT_TEAM_GH_APP_INSTALLATION_ID=...' >> ~/secrev.env # installation id on Sea-Haven-Industries/orchestrator +echo 'AGENT_TEAM_GH_APP_PRIVATE_KEY=/home/adam/.sea-haven/agent-team-apply.pem' >> ~/secrev.env # PATH to the .pem +chmod 600 ~/secrev.env +``` + +Place the App private key on the box and lock it down — it is a write-capable +credential and must be owner-only: + +``` +install -m 600 /dev/stdin ~/.sea-haven/agent-team-apply.pem < the-downloaded-key.pem +chmod 600 ~/.sea-haven/agent-team-apply.pem && ls -l ~/.sea-haven/agent-team-apply.pem # expect -rw------- +``` + +- `AGENT_TEAM_GH_APP_PRIVATE_KEY` is a **filesystem path** to the `.pem`, not the + key material. The coordinator reads the file at graph-build time; an unreadable + path logs one warning and falls back to gh-default (never crashes). +- **App permission/scope audit (do before deploy).** The App (currently the + reused **`agent-team-apply`** App) must be installed on **only** + `Sea-Haven-Industries/orchestrator` with **Contents: Read/Write** + **Actions: + Read/Write** and nothing broader. Minting an installation token grants exactly + the App's scopes; over-broad scopes widen blast radius. (Follow-up: move to a + dedicated least-privilege App to replace `agent-team-apply`.) +- The minted token is short-lived (~1h), is **never** logged, never put in an + exception message, and never written to the ledger/graph state; on the + authenticated push it rides in a host-scoped `http.extraHeader` passed via + `GIT_CONFIG_*` env (never in argv/`ps`). +- **Env-file precedence (verify after editing).** The unit loads **both** + `~/secrev.env` and `~/orchestrator/.env` (last-wins). Confirm only the intended + values are present in both so a stale entry can't shadow the App config: + ``` + grep -nE 'AGENT_TEAM_GH_APP|AGENT_TEAM_REPO_(OWNER|NAME)' ~/secrev.env ~/orchestrator/.env 2>/dev/null + ``` + ## 4. Deploy steps ``` @@ -324,6 +372,32 @@ daemon against a half-migrated or suspect ledger. Note the secrets in `~/secrev.env` are NOT removed by rollback - leave them, or strip the four agent-team keys if you are decommissioning entirely. +### GitHub App key — compromise / rotation / revocation + +The box holds a write-capable App private key, so it needs its own incident path +(separate from the VM snapshot rollback above): + +``` +# 1. Stop the daemon so no further token mints happen: +sudo systemctl stop agent-team-coordinator.service +# 2. Remove the key + the App env vars from the box (kills the App path -> inert): +rm -f ~/.sea-haven/agent-team-apply.pem +sed -i '/AGENT_TEAM_GH_APP_/d' ~/secrev.env +# 3. In GitHub: rotate (generate a new private key, delete the old one) under the +# App's settings, or uninstall the App from the repo to revoke all access. +# Existing installation tokens are short-lived (~1h) and expire on their own. +# 4. Restart inert (gh-default dispatch) and confirm no App env is loaded: +sudo systemctl start agent-team-coordinator.service +sudo systemctl show agent-team-coordinator.service -p Environment | grep -c AGENT_TEAM_GH_APP # expect 0 +``` + +- **Rotation cadence is Adam's call** — there is no automated rotation. Rotate on + any suspected box compromise, on operator turnover, and on a periodic cadence + Adam sets. Document the rotation event in Confluence. +- Removing the App env vars (or the key file) is a safe partial rollback on its + own: the dispatcher falls back to the inert gh-default path (which is itself + inert without `gh`), so no diff is ever pushed — never a fabricated pass. + ## 7. Security - **Slack inbound listener (Socket Mode) is the auth + untrusted-input surface.** @@ -335,5 +409,19 @@ strip the four agent-team keys if you are decommissioning entirely. - **No IAM / OIDC is involved in P1.** The box runs on subscription OAuth (`CLAUDE_CODE_OAUTH_TOKEN`) and Slack tokens only; there is no AWS role, no OIDC trust relationship, no cloud permission surface in this deploy. +- **GitHub App key on the box (P3 dispatch) — conscious, mitigated trade-off.** + The dispatcher's original design comment said "never a box-held token / operator + host only." Moving dispatch onto the box (so P3 reaches CI without `gh`) + deliberately deviates from that. Mitigations: the App is scoped to **one repo** + with **Contents + Actions only**; minted installation tokens are **short-lived + (~1h)**; the key file is **mode 600**; tokens are **never logged / never in + exception text / never in argv** (push auth rides a host-scoped + `http.extraHeader` via `GIT_CONFIG_*`); CI **re-verifies** the pushed content by + hash and the draft PR is still gated by the `agent-apply` environment's required + reviewer. Any mint/HTTP/git failure **parks** the task (no `run_id` → the verify + gate fails closed) — never a fabricated pass. See §3a for the + permission/scope audit, env-precedence check, and §6 for key rotation/ + revocation. **Credential-handling change → `/sh-security-review` is mandatory + before merge** (auth + secret handling surface). - `~/secrev.env` stays mode 600 and out of git; `state/` (ledger + audit log) is gitignored and written 0600. diff --git a/agent-team/README.md b/agent-team/README.md index d7b126e..a058e66 100644 --- a/agent-team/README.md +++ b/agent-team/README.md @@ -89,9 +89,16 @@ agent-team/ handbook.py # WS5 load_handbook_conventions (handbook seam, # fail-safe → "" if dir missing); planner context dispatch_invoker.py # P3 DISPATCH node — pushes the per-dispatch head branch, - # triggers CI (gh workflow run), and captures run_id + - # dispatched_at into state via a correlation-tagged poll - # of gh run list (fails closed on an unfound run) + # triggers CI, and captures run_id + dispatched_at into + # state via a correlation-tagged poll (fails closed on an + # unfound run). Auth via injected seams: gh/git CLI by + # default, or GitHub-App installation tokens box-side + # (github_app.py) when AGENT_TEAM_GH_APP_* is set + github_app.py # mints short-lived (~1h) GitHub-App installation tokens on + # the box (RS256 JWT → /access_tokens); TokenProvider + # caches + re-mints near expiry. Powers the App dispatch + # seams (dispatcher.app_branch_pusher/_workflow_dispatcher/ + # _run_locator) so P3 reaches CI with no gh + no box PAT transport/ # one adapter contract + a live impl per channel base.py # Transport ABC + QuestionSet / NormalizedAnswer slack_adapter.py + slack_live.py + slack_listener.py # Block Kit + Socket Mode + /new-task @@ -196,7 +203,7 @@ P1–P4 pipeline. What is **live** vs **inert** after the rollout: | WS1 | `invoker_multi.py` (in-process GPT-4.1 / DeepSeek / Gemini via the orchestrator's `models.py`) + `api.py` (FastAPI HTTP API, bearer auth via `AGENT_TEAM_API_TOKEN`, binds `127.0.0.1:8765`, `/docs`+`/openapi` disabled, concurrency-capped) | `bind_multi_invoker()` wired in `run-team.py` `_cmd_serve` (LIVE); the **HTTP API is a separate opt-in process** (`api.serve()`), NOT started by the coordinator | | WS5 | `nodes/handbook.py` `load_handbook_conventions` (reads `SEA_HAVEN_HANDBOOK_DIR` or `~/.sea-haven/engineering-handbook`, fail-safe → `""`); `retriever.py` `save_memory` writes to a `_box-drafts/` review queue | LIVE — the planner prompt receives the handbook via the `context_provider` seam in `run-team.py` `_build_coordinator` | | WS2/WS0/WS4 | Slack `/new-task` slash command (AUTHZ-01 owner-allowlist gated) → `Coordinator.set_new_task_callback`; the `sea-haven-claude-plugin/` (CLAUDE.md, settings, `/delegate` `UserPromptSubmit` hook) | LIVE (`/new-task` wired in `serve`); the plugin/HTTP-API path is opt-in | -| WS3 / P3 | `nodes/dispatch_invoker.py` (DISPATCH LangGraph node) + the BUILD → DISPATCH → VERIFY reorder, the `tick()`-driven CI-watcher, per-task `run_id` plumbing, and the bound `serve` default | **Built on `feat/agent-team-p3-box-integration`, gated behind the C1 re-review** before it ships to the box. The `agent-apply` GitHub Environment human-approval gate is KEPT; dispatch is **operator-initiated** (the box holds no standing write token — the branch push + `gh workflow run` use operator-host credentials). The bound P3 wiring is the new fail-safe `serve` default: a missing `AGENT_TEAM_REPO_OWNER`/`_NAME` or CI-read token degrades to the INERT P3 path (task parks + a `#agent-team` notice), never a serve-start crash | +| WS3 / P3 | `nodes/dispatch_invoker.py` (DISPATCH LangGraph node) + the BUILD → DISPATCH → VERIFY reorder, the `tick()`-driven CI-watcher, per-task `run_id` plumbing, and the bound `serve` default | **Built on `feat/agent-team-p3-box-integration`, gated behind the C1 re-review** before it ships to the box. The `agent-apply` GitHub Environment human-approval gate is KEPT. Dispatch auth is **seam-injected**: gh/git CLI by default, or — when `AGENT_TEAM_GH_APP_*` is set — **box-side GitHub-App installation tokens** (`github_app.py`), so the box mints a short-lived (~1h) write token on demand instead of needing `gh` or a standing PAT (a conscious, mitigated deviation from "operator host only" — App scoped to one repo, Contents+Actions only, tokens short-lived & never logged; CI still re-verifies by hash and the `agent-apply` reviewer gate still applies). The bound P3 wiring is the fail-safe `serve` default: a missing `AGENT_TEAM_REPO_OWNER`/`_NAME` or CI-read token degrades to the INERT P3 path (task parks + a `#agent-team` notice), never a serve-start crash; a partial/unreadable `AGENT_TEAM_GH_APP_*` config logs one warning and falls back to the gh-default seams | The HTTP API endpoints: `POST /tasks` (start a task), `GET /tasks/{thread_id}` (status), `POST /orchestrator/invoke` (one-shot model invoke). See diff --git a/agent-team/agent_team/coordinator.py b/agent-team/agent_team/coordinator.py index 12b7b94..c85959c 100644 --- a/agent-team/agent_team/coordinator.py +++ b/agent-team/agent_team/coordinator.py @@ -437,6 +437,67 @@ def gated_build_verify_wiring( return build_node, verify_node, route_after_verify +def _app_dispatch_seams() -> "tuple[Any, Any, Any] | None": + """Resolve the GitHub-App dispatch seams from env, or None for gh-default. + + The auto-dispatch node carries an approved diff into org CI. Two auth paths + exist behind the same :data:`DispatchNodeFactory` seam: the default ``gh``/ + ``git`` CLI path (:func:`agent_team.dispatcher` ``_default_*`` seams, used by + :func:`agent_team.nodes.dispatch_invoker.make_dispatch_node` when no seams are + passed) and a GitHub-App path that mints short-lived installation tokens. This + helper decides — from the environment ONLY — whether to wire the App path: + + * ``AGENT_TEAM_GH_APP_ID``, ``AGENT_TEAM_GH_APP_INSTALLATION_ID``, and + ``AGENT_TEAM_GH_APP_PRIVATE_KEY`` (a filesystem path to the App's ``.pem``) + must ALL be set → returns the ``(pusher, dispatcher, locator)`` App-seam + triple built over a lazy :class:`~agent_team.github_app.TokenProvider`. + * NONE set → returns ``None`` (the gh-default behavior is untouched). + * PARTIALLY set, or the key path cannot be read → logs ONE warning and returns + ``None`` so the daemon stays inert on the App path and falls back to + gh-default. It NEVER raises (a half-configured box must still serve) and + NEVER logs the key path's contents. + + The :class:`TokenProvider` is lazy: no JWT is minted and no key is validated + here, so a wiring-time call costs nothing and a bad key only fails closed + later, when a dispatch actually tries to mint a token (the node parks). + """ + app_id = os.environ.get("AGENT_TEAM_GH_APP_ID", "").strip() + inst = os.environ.get("AGENT_TEAM_GH_APP_INSTALLATION_ID", "").strip() + key_path = os.environ.get("AGENT_TEAM_GH_APP_PRIVATE_KEY", "").strip() + + present = [v for v in (app_id, inst, key_path) if v] + if not present: + return None # none set -> gh-default behavior + if len(present) != 3: + _LOG.warning( + "GitHub App dispatch wiring is INERT: AGENT_TEAM_GH_APP_ID/" + "INSTALLATION_ID/PRIVATE_KEY are not all set; falling back to " + "gh-default dispatch seams." + ) + return None # partial -> inert, NEVER raise, NEVER log key + + try: + from pathlib import Path + + pem = Path(key_path).read_text(encoding="utf-8") + except Exception: # noqa: BLE001 - unreadable key -> inert, never crash serve + _LOG.warning( + "GitHub App dispatch wiring is INERT: AGENT_TEAM_GH_APP_PRIVATE_KEY " + "path could not be read; falling back to gh-default dispatch seams." + ) + return None + + from agent_team.dispatcher import ( + app_branch_pusher, + app_run_locator, + app_workflow_dispatcher, + ) + from agent_team.github_app import TokenProvider + + tp = TokenProvider(app_id=app_id, private_key_pem=pem, installation_id=inst) + return (app_branch_pusher(tp), app_workflow_dispatcher(tp), app_run_locator(tp)) + + def default_dispatch_node_factory() -> "Callable[[Any], Any]": """Build the live auto-dispatch node from env vars (WS3, OPT-IN, INERT by default). @@ -449,6 +510,16 @@ def default_dispatch_node_factory() -> "Callable[[Any], Any]": Raises ``RuntimeError`` if the required env vars are absent (fail closed: the node must never dispatch to an unknown owner/repo). + **Auth path (GitHub App vs gh-default).** When the three + ``AGENT_TEAM_GH_APP_*`` env vars (App id, installation id, and a path to the + App's ``.pem``) are all set, :func:`_app_dispatch_seams` resolves the + GitHub-App ``(pusher, dispatcher, locator)`` triple and they are forwarded to + ``make_dispatch_node`` so dispatch authenticates with short-lived installation + tokens. Absent (or only partially set / unreadable key) the seams are left + unpassed and ``make_dispatch_node`` uses its built-in ``gh``/``git`` CLI + defaults — the unchanged behavior. The App branch never weakens the owner/repo + fail-closed check above. + This satisfies :data:`DispatchNodeFactory` (zero-arg → node callable). Pass it as ``dispatch_node_wiring=default_dispatch_node_factory`` AFTER the §3.3.2 CI trust-boundary gate clears. The production ``run-team.py`` path @@ -466,6 +537,18 @@ def default_dispatch_node_factory() -> "Callable[[Any], Any]": "AGENT_TEAM_REPO_OWNER and AGENT_TEAM_REPO_NAME must be set " "for auto-dispatch (default_dispatch_node_factory)" ) + + seams = _app_dispatch_seams() + if seams is not None: + pusher, dispatcher, locator = seams + return make_dispatch_node( + owner=owner, + repo=repo, + base=base, + pusher=pusher, + dispatcher=dispatcher, + locator=locator, + ) return make_dispatch_node(owner=owner, repo=repo, base=base) diff --git a/agent-team/agent_team/dispatcher.py b/agent-team/agent_team/dispatcher.py index e9bf59d..d7adb3d 100644 --- a/agent-team/agent_team/dispatcher.py +++ b/agent-team/agent_team/dispatcher.py @@ -38,12 +38,22 @@ __all__ = [ "DispatchResult", "DispatcherError", "RunLocator", + "app_branch_pusher", + "app_run_locator", + "app_workflow_dispatcher", "build_dispatch_inputs", "dispatch_apply_verify", "head_branch_for", "select_run_id", ] +# GitHub REST API root. The App-token seams (:func:`app_branch_pusher` / +# :func:`app_workflow_dispatcher` / :func:`app_run_locator`) talk to the REST API +# directly with an installation token instead of shelling out to ``gh`` — so the +# box can dispatch with a freshly-minted, short-lived App token and no operator +# ``gh`` auth. Mirrors :data:`agent_team.ci_fetcher.GITHUB_API_ROOT`. +GITHUB_API_ROOT = "https://api.github.com" + def _utc_now_iso() -> str: """UTC now as an ISO-8601 ``...Z`` string (matches GitHub Actions ``createdAt``).""" @@ -528,3 +538,289 @@ def _default_run_locator() -> RunLocator: return None return _locate + + +# ─────────────────────────────────────────────────────────────────────────── +# GitHub App-token seams (box-side). Same three behaviours as the ``gh``/``git`` +# defaults above, but authenticated with a short-lived installation token from an +# injected ``token_provider`` (``.token() -> str``) instead of the operator's +# ``gh`` auth. This lets the read-only box mint a write token on demand and +# dispatch directly via the REST API + a token-embedding clone URL. +# +# SECRET HYGIENE (BLOCKING): the installation token — and any clone URL that +# embeds it — MUST NEVER be logged, placed in an exception message / ``str()``, +# or written to ledger / graph / task state. The git seam never lets a raw +# ``CalledProcessError`` propagate (its ``.cmd`` / ``.output`` carry the token): +# it scrubs the token + remote URL to ``***`` and re-raises a +# :class:`DispatcherError` with ``from None``. The HTTP seams never include the +# token in any message (status only). +# ─────────────────────────────────────────────────────────────────────────── + + +def app_branch_pusher(token_provider: Any, *, _run: Any = None) -> BranchPusher: + """App-token :class:`BranchPusher`: clone + apply + push with an installation token. + + Mirrors the apply/commit/push semantics of :func:`_default_branch_pusher` + exactly — only the auth + clone differ: this clones the plain HTTPS remote + (``https://github.com/owner/repo.git``) and injects the installation token + minted on demand from ``token_provider.token()`` via an in-memory + ``-c http.extraHeader='Authorization: Bearer '`` on the network-facing + git steps (clone + push), rather than embedding the token in the remote URL or + relying on the operator's ``gh``/``git`` credentials. Carrying auth in a + config header keeps the token OUT of the remote URL and therefore out of the + visible process argv / stored ``origin`` — it is never in a position to leak + via ``ps``/``/proc//cmdline``. The diff is applied with ``--index`` + (stages exactly the diff, nothing stray) so the committed head tree is + precisely base+diff — the same bytes CI hash-verifies. + + ``_run`` injects the subprocess runner for tests; the default shells out to + ``git`` with ``check=True`` + ``capture_output=True``. + + SECRET HYGIENE: the token still appears in a git argv element (the + ``http.extraHeader`` config value), so every git step is wrapped so a raw + ``CalledProcessError`` (whose ``.cmd`` / ``.output`` may carry that header) + NEVER propagates. Any failure is re-raised as a :class:`DispatcherError` whose + message has the token scrubbed to ``***`` (``from None`` so the original — + token-bearing — exception is not chained). + """ + + def _default_run( + cmd: list[str], *, cwd: str | None = None, env: dict[str, str] | None = None + ) -> None: + import os + import subprocess + + full_env = {**os.environ, **env} if env else None + subprocess.run(cmd, cwd=cwd, check=True, capture_output=True, env=full_env) + + run = _run if _run is not None else _default_run + + def _push( + *, owner: str, repo: str, base: str, head_branch: str, diff_text: str + ) -> None: + import tempfile + from pathlib import Path + + token = token_provider.token() + # Plain remote — the token is NEVER in the URL/argv/stored origin. + remote = f"https://github.com/{owner}/{repo}.git" + # Auth is injected via http.extraHeader through git's GIT_CONFIG_* env + # vars (NOT argv) on the network steps (clone + push). Env is visible + # only to the same uid via /proc//environ, never via ps argv. The + # header key is SCOPED to the github.com host + # (``http.https://github.com/.extraHeader``, the GitHub-Actions checkout + # pattern) so git never sends the Authorization header to any other host + # it might be redirected to. + auth_env = { + "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_KEY_0": "http.https://github.com/.extraHeader", + "GIT_CONFIG_VALUE_0": f"Authorization: Bearer {token}", + } + + def _scrub(s: str) -> str: + return s.replace(token, "***") + + with tempfile.TemporaryDirectory() as tmp: + tmpdir = Path(tmp) + diff_path = tmpdir / CANDIDATE_DIFF_FILENAME + diff_path.write_text(diff_text, encoding="utf-8") + clonedir = tmpdir / "repo" + + try: + run( + [ + "git", + "clone", + "--depth", + "1", + "--branch", + base, + remote, + str(clonedir), + ], + env=auth_env, + ) + except Exception as exc: # noqa: BLE001 - scrub token before surfacing + raise DispatcherError(f"git clone failed: {_scrub(str(exc))}") from None + try: + run(["git", "-C", str(clonedir), "checkout", "-B", head_branch]) + except Exception as exc: # noqa: BLE001 + raise DispatcherError( + f"git checkout failed: {_scrub(str(exc))}" + ) from None + try: + # --index applies AND stages exactly the diff (incl. new files) and + # NOTHING else, so the head tree is precisely base+diff (LOGIC-1). + run(["git", "-C", str(clonedir), "apply", "--index", str(diff_path)]) + except Exception as exc: # noqa: BLE001 + raise DispatcherError(f"git apply failed: {_scrub(str(exc))}") from None + try: + run( + [ + "git", + "-C", + str(clonedir), + "-c", + "user.name=agent-team", + "-c", + "user.email=agent-team@seahavenind.com", + "commit", + "-m", + f"agent-team apply: {head_branch}", + ] + ) + except Exception as exc: # noqa: BLE001 + raise DispatcherError( + f"git commit failed: {_scrub(str(exc))}" + ) from None + try: + run( + [ + "git", + "-C", + str(clonedir), + "push", + "--no-verify", + "--force-with-lease", + "origin", + head_branch, + ], + env=auth_env, + ) + except Exception as exc: # noqa: BLE001 + raise DispatcherError(f"git push failed: {_scrub(str(exc))}") from None + + return _push + + +def app_workflow_dispatcher( + token_provider: Any, *, _http: Any = None +) -> WorkflowDispatcher: + """App-token :class:`WorkflowDispatcher`: trigger the workflow via the REST API. + + Same trigger as :func:`_default_workflow_dispatcher` (the apply/verify + ``workflow_dispatch``), but issues + ``POST /repos/{owner}/{repo}/actions/workflows/{WORKFLOW_FILE}/dispatches`` + directly with an installation token from ``token_provider.token()`` instead of + shelling out to ``gh``. ``_http`` injects a ``requests``-like client for tests + (``.post(url, *, json=..., headers=..., timeout=...)`` -> response exposing + ``.status_code``); the default lazily imports ``requests``. + + GitHub returns ``204`` on success; any other status fails closed with a + :class:`DispatcherError` carrying the STATUS only (NEVER the token). + """ + + def _fire(*, owner: str, repo: str, inputs: dict[str, str], ref: str) -> None: + url = ( + f"{GITHUB_API_ROOT}/repos/{owner}/{repo}" + f"/actions/workflows/{WORKFLOW_FILE}/dispatches" + ) + headers = { + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + "Authorization": f"Bearer {token_provider.token()}", + } + body = {"ref": ref, "inputs": inputs} + if _http is not None: + resp = _http.post(url, json=body, headers=headers, timeout=15.0) + else: + import requests # deferred: optional dependency + + resp = requests.post(url, json=body, headers=headers, timeout=15.0) + status = getattr(resp, "status_code", None) + if status != 204: + raise DispatcherError(f"workflow dispatch failed: status={status}") + + return _fire + + +def app_run_locator( + token_provider: Any, + *, + _http: Any = None, + _sleep: Any = None, + _now: Any = None, +) -> RunLocator: + """App-token :class:`RunLocator`: match the triggered run via the REST API. + + Same anti-stale SELECTION as :func:`_default_run_locator` — it floors on the + dispatched-at watermark (minus :data:`_LOCATE_SKEW_S` for clock skew), + delegates to the pure :func:`select_run_id`, and polls a bounded number of + times — but lists runs via + ``GET /repos/{owner}/{repo}/actions/runs`` with an installation token instead + of ``gh run list``. The REST rows are mapped to the field names + :func:`select_run_id` reads (``id`` -> ``databaseId``, ``created_at`` -> + ``createdAt``). Returns ``None`` if no matching run registers within the poll + window (fails closed). + + A non-200 list response (e.g. a 401/403/404 auth/scope edge) raises a + :class:`DispatcherError` carrying the STATUS only (NEVER the token) so the + dispatch node parks immediately with an actionable signal, rather than + treating the error body as "no runs" and silently exhausting the poll window. + + ``_http`` injects a ``requests``-like client (``.get(url, *, params=..., + headers=..., timeout=...)`` -> response exposing ``.json()``); ``_sleep`` / + ``_now`` are injected for tests. No token ever appears in a log or message. + """ + sleep = _sleep if _sleep is not None else None + + def _locate(*, owner: str, repo: str, task_id: str, since_iso: str) -> str | None: + import time + from datetime import timedelta + + do_sleep = sleep if sleep is not None else time.sleep + + try: + floor_dt = datetime.strptime(since_iso, "%Y-%m-%dT%H:%M:%SZ").replace( + tzinfo=timezone.utc + ) - timedelta(seconds=_LOCATE_SKEW_S) + floor_iso = floor_dt.strftime("%Y-%m-%dT%H:%M:%SZ") + except ValueError: + floor_iso = since_iso + + headers = { + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + "Authorization": f"Bearer {token_provider.token()}", + } + url = f"{GITHUB_API_ROOT}/repos/{owner}/{repo}/actions/runs" + params = { + "event": "workflow_dispatch", + "created": f">={floor_iso}", + "per_page": 50, + } + + for attempt in range(_LOCATE_ATTEMPTS): + if _http is not None: + resp = _http.get(url, params=params, headers=headers, timeout=15.0) + else: + import requests # deferred: optional dependency + + resp = requests.get(url, params=params, headers=headers, timeout=15.0) + # Surface auth/4xx promptly with the STATUS only (NEVER the token) + # instead of treating a 401/403/404 error body as "no runs" and + # silently exhausting the ~60s poll window. A genuine 200 with no + # matching run still falls through to the None-on-no-match path below. + status = getattr(resp, "status_code", 200) + if status != 200: + raise DispatcherError(f"run list failed: status={status}") + body = resp.json() + runs_raw = body.get("workflow_runs") or [] + mapped = [ + { + "databaseId": r.get("id"), + "name": r.get("name"), + "createdAt": r.get("created_at"), + "status": r.get("status"), + "conclusion": r.get("conclusion"), + } + for r in runs_raw + ] + run_id = select_run_id(mapped, task_id=task_id, floor_iso=floor_iso) + if run_id is not None: + return run_id + if attempt < _LOCATE_ATTEMPTS - 1: + do_sleep(_LOCATE_DELAY_S) + return None + + return _locate diff --git a/agent-team/agent_team/github_app.py b/agent-team/agent_team/github_app.py new file mode 100644 index 0000000..4db4045 --- /dev/null +++ b/agent-team/agent_team/github_app.py @@ -0,0 +1,184 @@ +"""GitHub App auth: mint short-lived installation tokens on the trusted host. + +The trusted apply path (:mod:`agent_team.dispatcher`) needs a write-capable +GitHub token to push the candidate head branch and trigger the apply/verify +``workflow_dispatch``. Rather than park a long-lived PAT on the operator host, +this module mints an **installation access token** from a GitHub App private +key: build a short-lived App JWT (signed RS256 with the App key), POST it to +``/app/installations/{installation_id}/access_tokens``, and receive a token that +expires within the hour. :class:`TokenProvider` caches the minted token and +re-mints just before expiry so callers can ask for a fresh token cheaply. + +SECRET HYGIENE (BLOCKING): the App JWT, the installation token, and anything +derived from them are NEVER logged, NEVER placed in an exception message or +``str()``, and NEVER written to ledger / graph / task state. Mint/HTTP failures +fail closed (raise :class:`GitHubAppError` with a scrubbed reason) so the +dispatch node parks rather than fabricating success. + +Prevailing HTTP approach mirrors :mod:`agent_team.ci_fetcher`: ``requests`` is a +deferred optional import, and an injectable ``requests``-like client (any object +exposing ``post(url, *, json, timeout)`` / a callable for tests, with +``.status_code`` and ``.json()``) makes this unit-testable with no network. +``jwt`` (PyJWT) is likewise a deferred import. +""" + +from __future__ import annotations + +from datetime import datetime, timedelta, timezone +from typing import Any, Callable + +__all__ = ["mint_installation_token", "TokenProvider", "GitHubAppError"] + +GITHUB_API_ROOT = "https://api.github.com" + +# App JWT lifetime knobs. GitHub rejects an App JWT whose ``exp`` is more than 10 +# minutes out and is sensitive to clock skew, so we backdate ``iat`` by 60s and +# cap the lifetime well under the 10-minute ceiling. +_JWT_BACKDATE_S = 60 +_JWT_LIFETIME_S = 540 # 9 minutes (<= 10 min GitHub ceiling) + +# Conservative default timeout for the single mint POST. A hang must fail (the +# dispatch node parks), never wedge the operator host. +_DEFAULT_TIMEOUT_S = 15.0 + + +class GitHubAppError(Exception): + """Raised on a missing/invalid key, a missing lib, or a mint failure. + + The message is ALWAYS scrubbed: it never contains the App private key, the + App JWT, or the minted installation token. + """ + + +def _utc_now() -> datetime: + """UTC now as an aware datetime (the default ``_now`` clock).""" + return datetime.now(timezone.utc) + + +def mint_installation_token( + *, + app_id: str, + private_key_pem: str, + installation_id: str, + _http: Callable[..., Any] | None = None, + _now: Callable[[], datetime] | None = None, +) -> dict: + """Mint a GitHub App installation access token (fail-closed, secret-safe). + + Builds a short-lived RS256 App JWT from ``private_key_pem`` (``iss=app_id``, + ``iat`` backdated 60s, ``exp`` 9 min out), then POSTs it to + ``/app/installations/{installation_id}/access_tokens`` and returns + ``{"token", "expires_at"}`` from the ``201`` response. + + ``_http`` injects a callable ``(url, *, headers, timeout) -> response`` (with + ``.status_code`` / ``.json()``) for tests; when omitted, ``requests.post`` is + used via a deferred import. ``_now`` injects the clock (a zero-arg callable + returning an aware UTC datetime) for deterministic JWT claims. + + Raises :class:`GitHubAppError` (with a scrubbed message — never the key, the + JWT, or the token) if PyJWT is unavailable, the key is empty/invalid, or the + mint request does not return a ``201`` with a token + expiry. + """ + now = (_now or _utc_now)() + iat = int(now.timestamp()) - _JWT_BACKDATE_S + exp = int(now.timestamp()) + _JWT_LIFETIME_S + + try: + import jwt # deferred: optional dependency (PyJWT) + + payload = {"iss": str(app_id), "iat": iat, "exp": exp} + token_jwt = jwt.encode(payload, private_key_pem, algorithm="RS256") + except Exception as exc: # noqa: BLE001 - missing lib / empty / invalid key + # SECRET HYGIENE: surface only the exception TYPE, never the key or any + # partially-built JWT material that an exception payload might carry. + raise GitHubAppError(f"could not build app JWT: {type(exc).__name__}") from None + + url = f"{GITHUB_API_ROOT}/app/installations/{installation_id}/access_tokens" + headers = { + "Authorization": f"Bearer {token_jwt}", + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + } + + if _http is not None: + resp = _http(url, headers=headers, timeout=_DEFAULT_TIMEOUT_S) + else: + import requests # deferred: optional dependency (see module docstring) + + resp = requests.post(url, headers=headers, timeout=_DEFAULT_TIMEOUT_S) + + status = getattr(resp, "status_code", None) + body = resp.json() if status == 201 else None + if status != 201 or not isinstance(body, dict): + # NEVER include the JWT or any token in the failure message. Avoid even + # the literal substring "tok" so a naive secret scan can't false-positive. + raise GitHubAppError(f"installation-access mint failed: status={status}") + + token = body.get("token") + expires_at = body.get("expires_at") + if not token or not expires_at: + raise GitHubAppError(f"installation-access mint failed: status={status}") + + return {"token": token, "expires_at": expires_at} + + +class TokenProvider: + """Caches a minted installation token, re-minting just before expiry. + + Construction does NO network and NO key validation — minting is lazy, on the + first :meth:`token` call. The cached token is re-minted once ``now`` reaches + ``expiry - refresh_margin_s`` so a caller always gets a token with usable + headroom. The token is NEVER logged or otherwise exposed. + """ + + def __init__( + self, + *, + app_id: str, + private_key_pem: str, + installation_id: str, + _http: Callable[..., Any] | None = None, + _now: Callable[[], datetime] | None = None, + refresh_margin_s: int = 300, + ) -> None: + self._app_id = app_id + self._private_key_pem = private_key_pem + self._installation_id = installation_id + self._http = _http + self._now = _now + self._refresh_margin_s = refresh_margin_s + self._cached_token: str | None = None + self._cached_expiry: datetime | None = None + + def token(self) -> str: + """Return a valid installation token, minting/re-minting as needed. + + Mints on first use and re-mints once within ``refresh_margin_s`` of the + cached expiry. Propagates :class:`GitHubAppError` on a mint failure (the + caller fails closed). The returned token is NEVER logged. + """ + now = (self._now or _utc_now)() + if ( + self._cached_token is None + or self._cached_expiry is None + or now + timedelta(seconds=self._refresh_margin_s) >= self._cached_expiry + ): + result = mint_installation_token( + app_id=self._app_id, + private_key_pem=self._private_key_pem, + installation_id=self._installation_id, + _http=self._http, + _now=self._now, + ) + self._cached_token = result["token"] + self._cached_expiry = _parse_expires_at(result["expires_at"]) + return self._cached_token + + +def _parse_expires_at(expires_at: str) -> datetime: + """Parse a GitHub ``expires_at`` ISO-8601 ``...Z`` string to aware UTC. + + GitHub returns e.g. ``2026-06-24T12:00:00Z``; normalise the trailing ``Z`` to + a ``+00:00`` offset for :meth:`datetime.fromisoformat`. + """ + return datetime.fromisoformat(expires_at.replace("Z", "+00:00")) diff --git a/agent-team/tests/test_coordinator.py b/agent-team/tests/test_coordinator.py index 6b2c1d1..ad46419 100644 --- a/agent-team/tests/test_coordinator.py +++ b/agent-team/tests/test_coordinator.py @@ -17,6 +17,7 @@ Every test injects: from __future__ import annotations +import logging import queue from datetime import datetime, timedelta, timezone from pathlib import Path @@ -2067,3 +2068,126 @@ def test_plan_decision_expiry_posts_recovery_notice( message, thread_ts = notice[0] assert thread_ts == "ROOT.TS" assert "re-assign" in message.lower() or "force-resume" in message.lower() + + +# --------------------------------------------------------------------------- # +# default_dispatch_node_factory — GitHub App vs gh-default auth seams (FILE 3) +# --------------------------------------------------------------------------- # + + +def _capture_make_dispatch_node( + monkeypatch: pytest.MonkeyPatch, +) -> list[dict[str, Any]]: + """Stub ``make_dispatch_node`` at its lazy import site; record each kwargs dict. + + ``default_dispatch_node_factory`` imports ``make_dispatch_node`` lazily from + :mod:`agent_team.nodes.dispatch_invoker`, so the patch must land on that + module (the name the ``from ... import`` resolves at call time), not on the + coordinator module. Returns the list the stub appends its kwargs to. + """ + calls: list[dict[str, Any]] = [] + + def _fake_make_dispatch_node(**kwargs: Any) -> Any: + calls.append(kwargs) + return lambda state: state + + monkeypatch.setattr( + "agent_team.nodes.dispatch_invoker.make_dispatch_node", + _fake_make_dispatch_node, + ) + return calls + + +def _clear_app_env(monkeypatch: pytest.MonkeyPatch) -> None: + """Ensure the owner/repo are set and no AGENT_TEAM_GH_APP_* leaks in.""" + monkeypatch.setenv("AGENT_TEAM_REPO_OWNER", "Sea-Haven-Industries") + monkeypatch.setenv("AGENT_TEAM_REPO_NAME", "orchestrator") + for var in ( + "AGENT_TEAM_GH_APP_ID", + "AGENT_TEAM_GH_APP_INSTALLATION_ID", + "AGENT_TEAM_GH_APP_PRIVATE_KEY", + ): + monkeypatch.delenv(var, raising=False) + + +def test_dispatch_factory_wires_app_seams_when_all_three_set( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """All three AGENT_TEAM_GH_APP_* set → make_dispatch_node gets the App seams. + + With the App id, installation id, and a readable ``.pem`` path present, the + factory resolves the GitHub-App ``(pusher, dispatcher, locator)`` triple (over + a LAZY TokenProvider — the dummy pem text is never parsed here) and forwards + all three to ``make_dispatch_node`` non-None. + """ + from agent_team.coordinator import default_dispatch_node_factory + + _clear_app_env(monkeypatch) + pem = tmp_path / "app.pem" + pem.write_text( + "-----BEGIN RSA PRIVATE KEY-----\nnot-a-real-key\n", encoding="utf-8" + ) + monkeypatch.setenv("AGENT_TEAM_GH_APP_ID", "123456") + monkeypatch.setenv("AGENT_TEAM_GH_APP_INSTALLATION_ID", "987654") + monkeypatch.setenv("AGENT_TEAM_GH_APP_PRIVATE_KEY", str(pem)) + + calls = _capture_make_dispatch_node(monkeypatch) + default_dispatch_node_factory() + + assert len(calls) == 1 + kwargs = calls[0] + assert kwargs["owner"] == "Sea-Haven-Industries" + assert kwargs["repo"] == "orchestrator" + assert kwargs["pusher"] is not None + assert kwargs["dispatcher"] is not None + assert kwargs["locator"] is not None + + +def test_dispatch_factory_inert_on_partial_app_env( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """Only APP_ID set → no App seams forwarded, ONE warning logged, NO exception. + + A half-configured App env must NEVER raise (the daemon must still serve) and + must fall back to gh-default — ``make_dispatch_node`` is called without the + pusher/dispatcher/locator seams (absent or None). + """ + from agent_team.coordinator import default_dispatch_node_factory + + _clear_app_env(monkeypatch) + monkeypatch.setenv("AGENT_TEAM_GH_APP_ID", "123456") # the only one set + + calls = _capture_make_dispatch_node(monkeypatch) + with caplog.at_level(logging.WARNING, logger="agent_team.coordinator"): + default_dispatch_node_factory() # must not raise + + assert len(calls) == 1 + kwargs = calls[0] + assert kwargs["owner"] == "Sea-Haven-Industries" + assert kwargs["repo"] == "orchestrator" + assert kwargs.get("pusher") is None + assert kwargs.get("dispatcher") is None + assert kwargs.get("locator") is None + assert any("INERT" in rec.message for rec in caplog.records) + + +def test_dispatch_factory_gh_default_when_no_app_env( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """None of the AGENT_TEAM_GH_APP_* set → unchanged gh-default path (no seams).""" + from agent_team.coordinator import default_dispatch_node_factory + + _clear_app_env(monkeypatch) + + calls = _capture_make_dispatch_node(monkeypatch) + default_dispatch_node_factory() + + assert len(calls) == 1 + kwargs = calls[0] + assert kwargs["owner"] == "Sea-Haven-Industries" + assert kwargs["repo"] == "orchestrator" + assert "pusher" not in kwargs + assert "dispatcher" not in kwargs + assert "locator" not in kwargs diff --git a/agent-team/tests/test_dispatcher.py b/agent-team/tests/test_dispatcher.py index 6008abe..b2f99fb 100644 --- a/agent-team/tests/test_dispatcher.py +++ b/agent-team/tests/test_dispatcher.py @@ -16,6 +16,9 @@ from agent_team.dispatcher import ( DispatcherError, DispatchInputs, DispatchResult, + app_branch_pusher, + app_run_locator, + app_workflow_dispatcher, build_dispatch_inputs, dispatch_apply_verify, head_branch_for, @@ -328,3 +331,227 @@ def test_select_run_id_returns_none_when_only_cancelled_matches() -> None: select_run_id([only_cancelled], task_id=TASK, floor_iso="2026-06-23T09:58:00Z") is None ) + + +# --------------------------------------------------------------------------- # +# app_branch_pusher (App-token git seam; secret hygiene) +# --------------------------------------------------------------------------- # + + +class _StubTokenProvider: + """Minimal token provider: returns a fixed installation token.""" + + def __init__(self, token: str = "ghs_TESTTOKEN") -> None: + self._token = token + + def token(self) -> str: + return self._token + + +def test_app_branch_pusher_keeps_token_out_of_argv_and_uses_plain_remote() -> None: + # The clone must use the PLAIN HTTPS remote (no token in the URL/argv), and + # the installation token must be injected via http.extraHeader through git's + # GIT_CONFIG_* env vars on the network steps (clone + push) — never in argv. + recorded: list[dict] = [] + + def fake_run(cmd, *, cwd=None, env=None): + recorded.append({"cmd": list(cmd), "env": env}) + + push = app_branch_pusher(_StubTokenProvider(), _run=fake_run) + push( + owner="owner", + repo="repo", + base="main", + head_branch="agent-team/apply/abc", + diff_text=DIFF, + ) + + clone_cmd = recorded[0]["cmd"] + assert "clone" in clone_cmd + assert "https://github.com/owner/repo.git" in clone_cmd + # The token NEVER appears in any git argv element. + for call in recorded: + for arg in call["cmd"]: + assert "ghs_TESTTOKEN" not in arg + + # Auth rides in GIT_CONFIG_* env on the network steps only (clone + push). + clone_env = recorded[0]["env"] + assert clone_env["GIT_CONFIG_KEY_0"] == "http.https://github.com/.extraHeader" + assert clone_env["GIT_CONFIG_VALUE_0"] == "Authorization: Bearer ghs_TESTTOKEN" + push_env = recorded[-1]["env"] + assert push_env["GIT_CONFIG_VALUE_0"] == "Authorization: Bearer ghs_TESTTOKEN" + # The non-network steps (checkout/apply/commit) carry no auth env. + for call in recorded[1:4]: + assert call["env"] is None + + # The five expected git steps fired in order. Skip the global ``git`` + # options (``-C ``, ``-c ``) to find each subcommand. + def _git_subcommand(cmd: list[str]) -> str: + i = 1 # cmd[0] == "git" + while i < len(cmd) and cmd[i] in ("-C", "-c"): + i += 2 # each takes one argument + return cmd[i] + + steps = [_git_subcommand(c["cmd"]) for c in recorded] + assert steps == ["clone", "checkout", "apply", "commit", "push"] + + +def test_app_branch_pusher_scrubs_token_from_errors() -> None: + # The token rides in the http.extraHeader config value (passed via env, not + # argv). If a failing git step's exception string ever surfaces that header, + # it must NEVER leak the token — it is re-raised as a DispatcherError with the + # token scrubbed to ***. + def failing_run(cmd, *, cwd=None, env=None): + # Simulate an error whose string carries the Authorization header value. + raise RuntimeError( + "fatal: remote rejected (Authorization: Bearer ghs_TESTTOKEN)" + ) + + push = app_branch_pusher(_StubTokenProvider(), _run=failing_run) + with pytest.raises(DispatcherError) as excinfo: + push( + owner="owner", + repo="repo", + base="main", + head_branch="agent-team/apply/abc", + diff_text=DIFF, + ) + assert "ghs_TESTTOKEN" not in str(excinfo.value) + assert "***" in str(excinfo.value) + # ``from None``: the token-bearing original is not chained onto the raised error. + assert excinfo.value.__cause__ is None + + +# --------------------------------------------------------------------------- # +# app_workflow_dispatcher (App-token REST seam) +# --------------------------------------------------------------------------- # + + +class _FakeHttp: + """Records POST/GET calls; returns a canned response object.""" + + def __init__(self, *, post_status=204, get_body=None, get_status=200) -> None: + self.post_calls: list[dict] = [] + self.get_calls: list[dict] = [] + self._post_status = post_status + self._get_body = get_body or {} + self._get_status = get_status + + def post(self, url, *, json=None, headers=None, timeout=None): + self.post_calls.append({"url": url, "json": json, "headers": headers}) + return _FakeResp(status_code=self._post_status) + + def get(self, url, *, params=None, headers=None, timeout=None): + self.get_calls.append({"url": url, "params": params, "headers": headers}) + return _FakeResp(status_code=self._get_status, body=self._get_body) + + +class _FakeResp: + def __init__(self, *, status_code=200, body=None) -> None: + self.status_code = status_code + self._body = body or {} + + def json(self): + return self._body + + +def test_app_workflow_dispatcher_posts_correct_url_and_body() -> None: + http = _FakeHttp(post_status=204) + fire = app_workflow_dispatcher(_StubTokenProvider(), _http=http) + inputs = {"task_id": TASK, "diff_b64": "x"} + fire(owner="owner", repo="repo", inputs=inputs, ref="main") + + call = http.post_calls[0] + assert call["url"].endswith( + "/actions/workflows/agent-team-apply-verify.yml/dispatches" + ) + assert call["json"] == {"ref": "main", "inputs": inputs} + # The installation token is carried as a Bearer header (and nowhere else). + assert call["headers"]["Authorization"] == "Bearer ghs_TESTTOKEN" + + +def test_app_workflow_dispatcher_fails_closed_without_token_in_message() -> None: + http = _FakeHttp(post_status=422) + fire = app_workflow_dispatcher(_StubTokenProvider(), _http=http) + with pytest.raises(DispatcherError) as excinfo: + fire(owner="owner", repo="repo", inputs={"task_id": TASK}, ref="main") + assert "422" in str(excinfo.value) + assert "ghs_TESTTOKEN" not in str(excinfo.value) + + +# --------------------------------------------------------------------------- # +# app_run_locator (App-token REST seam; field mapping + run-name correlation) +# --------------------------------------------------------------------------- # + + +def test_app_run_locator_maps_rest_fields_and_correlates_run_name() -> None: + # The REST list endpoint uses id/created_at; the locator must map them to the + # databaseId/createdAt names select_run_id reads, and correlate on run-name. + body = { + "workflow_runs": [ + { + "id": 12345, + "name": run_name_for(TASK), + "created_at": "2026-06-23T10:00:00Z", + "status": "in_progress", + "conclusion": None, + } + ] + } + http = _FakeHttp(get_body=body) + locate = app_run_locator(_StubTokenProvider(), _http=http, _sleep=lambda *_a: None) + run_id = locate( + owner="owner", + repo="repo", + task_id=TASK, + since_iso="2026-06-23T09:58:00Z", + ) + assert run_id == "12345" + # The list GET is authenticated + filtered to workflow_dispatch events. + call = http.get_calls[0] + assert call["url"].endswith("/actions/runs") + assert call["params"]["event"] == "workflow_dispatch" + assert call["headers"]["Authorization"] == "Bearer ghs_TESTTOKEN" + + +def test_app_run_locator_returns_none_for_non_matching_task() -> None: + body = { + "workflow_runs": [ + { + "id": 999, + "name": "agent-team-apply someone-else", + "created_at": "2026-06-23T10:00:00Z", + "status": "in_progress", + "conclusion": None, + } + ] + } + http = _FakeHttp(get_body=body) + locate = app_run_locator(_StubTokenProvider(), _http=http, _sleep=lambda *_a: None) + assert ( + locate( + owner="owner", + repo="repo", + task_id=TASK, + since_iso="2026-06-23T09:58:00Z", + ) + is None + ) + + +def test_app_run_locator_raises_on_auth_error_without_token_in_message() -> None: + # A 401/403/404 list response must park immediately with the STATUS only + # (never the token), not silently poll ~60s and return None as "no runs". + http = _FakeHttp(get_status=403, get_body={"message": "Bad credentials"}) + locate = app_run_locator(_StubTokenProvider(), _http=http, _sleep=lambda *_a: None) + with pytest.raises(DispatcherError) as excinfo: + locate( + owner="owner", + repo="repo", + task_id=TASK, + since_iso="2026-06-23T09:58:00Z", + ) + assert "403" in str(excinfo.value) + assert "ghs_TESTTOKEN" not in str(excinfo.value) + # Fails fast on the first attempt — no poll-window exhaustion. + assert len(http.get_calls) == 1 diff --git a/agent-team/tests/test_github_app.py b/agent-team/tests/test_github_app.py new file mode 100644 index 0000000..07ac31f --- /dev/null +++ b/agent-team/tests/test_github_app.py @@ -0,0 +1,270 @@ +"""Unit tests for agent_team.github_app — App-JWT mint + TokenProvider cache. + +These tests generate a throwaway RSA-2048 key with ``cryptography``, sign a real +App JWT, and intercept the mint HTTP call with a fake client so no network and no +real GitHub App is touched. They assert: the JWT claims/alg are correct, a clean +``201`` returns the ``{token, expires_at}`` mapping, the provider caches and +re-mints around the refresh margin, and — critically — that no token or JWT +material ever appears in a raised error's message (secret hygiene). +""" + +from __future__ import annotations + +from datetime import datetime, timedelta, timezone + +import jwt +import pytest +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.asymmetric import rsa + +from agent_team.github_app import ( + GitHubAppError, + TokenProvider, + mint_installation_token, +) + +_APP_ID = "123456" +_INSTALLATION_ID = "987654" + +# A fixed clock so JWT iat/exp are deterministic. +_FIXED_NOW = datetime(2026, 6, 24, 12, 0, 0, tzinfo=timezone.utc) + + +@pytest.fixture +def rsa_keypair(): + """Generate a throwaway RSA-2048 keypair; return (private_pem, public_obj).""" + private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + private_pem = private_key.private_bytes( + encoding=serialization.Encoding.PEM, + format=serialization.PrivateFormat.PKCS8, + encryption_algorithm=serialization.NoEncryption(), + ).decode("ascii") + return private_pem, private_key.public_key() + + +class _FakeResponse: + def __init__(self, status: int, body): + self.status_code = status + self._body = body + + def json(self): + return self._body + + +class _FakeHttp: + """A callable (url, *, headers, timeout) mint client recording calls.""" + + def __init__(self, response=None, raise_exc=None): + self._response = response + self._raise = raise_exc + self.calls: list[dict] = [] + + def __call__(self, url, *, headers=None, timeout=None): + self.calls.append({"url": url, "headers": headers, "timeout": timeout}) + if self._raise is not None: + raise self._raise + return self._response + + @property + def call_count(self) -> int: + return len(self.calls) + + +def _fixed_now(): + return _FIXED_NOW + + +def _future_iso(seconds: int = 3600) -> str: + return (_FIXED_NOW + timedelta(seconds=seconds)).strftime("%Y-%m-%dT%H:%M:%SZ") + + +# --------------------------------------------------------------------------- # +# JWT claims / algorithm # +# --------------------------------------------------------------------------- # +def test_mint_signs_jwt_with_expected_claims(rsa_keypair): + private_pem, public_key = rsa_keypair + http = _FakeHttp(_FakeResponse(201, {"token": "tok", "expires_at": _future_iso()})) + + mint_installation_token( + app_id=_APP_ID, + private_key_pem=private_pem, + installation_id=_INSTALLATION_ID, + _http=http, + _now=_fixed_now, + ) + + auth = http.calls[0]["headers"]["Authorization"] + assert auth.startswith("Bearer ") + token = auth.split(" ", 1)[1] + + assert jwt.get_unverified_header(token)["alg"] == "RS256" + # Verify the signature and claims, but not exp/iat against the real wall + # clock: the JWT is minted from the fixed test clock (_fixed_now), so its + # short-lived exp is in the past relative to the real time the suite runs. + claims = jwt.decode( + token, + public_key, + algorithms=["RS256"], + options={"verify_aud": False, "verify_exp": False, "verify_iat": False}, + ) + assert claims["iss"] == _APP_ID + now_ts = int(_FIXED_NOW.timestamp()) + assert claims["iat"] <= now_ts + assert claims["exp"] - claims["iat"] <= 600 + + +def test_mint_targets_installation_token_url(rsa_keypair): + private_pem, _ = rsa_keypair + http = _FakeHttp(_FakeResponse(201, {"token": "tok", "expires_at": _future_iso()})) + + mint_installation_token( + app_id=_APP_ID, + private_key_pem=private_pem, + installation_id=_INSTALLATION_ID, + _http=http, + _now=_fixed_now, + ) + + assert http.calls[0]["url"].endswith( + f"/app/installations/{_INSTALLATION_ID}/access_tokens" + ) + + +# --------------------------------------------------------------------------- # +# Mint success # +# --------------------------------------------------------------------------- # +def test_mint_success_returns_token_and_expiry(rsa_keypair): + private_pem, _ = rsa_keypair + expires_at = _future_iso() + http = _FakeHttp(_FakeResponse(201, {"token": "tok", "expires_at": expires_at})) + + result = mint_installation_token( + app_id=_APP_ID, + private_key_pem=private_pem, + installation_id=_INSTALLATION_ID, + _http=http, + _now=_fixed_now, + ) + + assert result == {"token": "tok", "expires_at": expires_at} + + +# --------------------------------------------------------------------------- # +# TokenProvider caching / re-mint # +# --------------------------------------------------------------------------- # +def test_provider_caches_token_across_calls(rsa_keypair): + private_pem, _ = rsa_keypair + http = _FakeHttp( + _FakeResponse(201, {"token": "tok", "expires_at": _future_iso(86400)}) + ) + provider = TokenProvider( + app_id=_APP_ID, + private_key_pem=private_pem, + installation_id=_INSTALLATION_ID, + _http=http, + _now=_fixed_now, + ) + + assert provider.token() == "tok" + assert provider.token() == "tok" + assert http.call_count == 1 + + +def test_provider_remints_near_expiry(rsa_keypair): + private_pem, _ = rsa_keypair + # First mint expires 10 minutes out; with a 5-minute margin the second call + # (clock advanced past expiry - margin) must re-mint. + responses = [ + _FakeResponse(201, {"token": "tok1", "expires_at": _future_iso(600)}), + _FakeResponse(201, {"token": "tok2", "expires_at": _future_iso(1200)}), + ] + + class _SeqHttp(_FakeHttp): + def __call__(self, url, *, headers=None, timeout=None): + self.calls.append({"url": url}) + return responses[len(self.calls) - 1] + + http = _SeqHttp() + + clock = {"now": _FIXED_NOW} + + def _moving_now(): + return clock["now"] + + provider = TokenProvider( + app_id=_APP_ID, + private_key_pem=private_pem, + installation_id=_INSTALLATION_ID, + _http=http, + _now=_moving_now, + refresh_margin_s=300, + ) + + assert provider.token() == "tok1" + assert http.call_count == 1 + + # Advance past (expiry - margin) = +300s -> re-mint. + clock["now"] = _FIXED_NOW + timedelta(seconds=400) + assert provider.token() == "tok2" + assert http.call_count == 2 + + +# --------------------------------------------------------------------------- # +# Secret hygiene # +# --------------------------------------------------------------------------- # +def test_mint_failure_status_is_scrubbed(rsa_keypair): + private_pem, _ = rsa_keypair + # Even on a failure GitHub may echo the (bad) token; ensure nothing leaks. + http = _FakeHttp(_FakeResponse(401, {"token": "tok", "message": "Bad creds"})) + + with pytest.raises(GitHubAppError) as exc: + mint_installation_token( + app_id=_APP_ID, + private_key_pem=private_pem, + installation_id=_INSTALLATION_ID, + _http=http, + _now=_fixed_now, + ) + + message = str(exc.value) + assert "tok" not in message + # No JWT material (RS256 JWTs start with the base64 header "eyJ"). + assert "eyJ" not in message + + +def test_mint_http_error_is_scrubbed(rsa_keypair): + private_pem, _ = rsa_keypair + http = _FakeHttp(raise_exc=RuntimeError("boom")) + + # A raised transport error propagates (fail closed); it must not carry the + # JWT. We do not catch a specific type here — only assert no JWT leaks if it + # were ever wrapped. + with pytest.raises(Exception) as exc: # noqa: PT011 + mint_installation_token( + app_id=_APP_ID, + private_key_pem=private_pem, + installation_id=_INSTALLATION_ID, + _http=http, + _now=_fixed_now, + ) + + assert "eyJ" not in str(exc.value) + + +def test_mint_invalid_key_is_scrubbed(): + # An empty/invalid PEM must fail closed with a scrubbed message (no key). + bad_key = "-----BEGIN PRIVATE KEY-----\nnotreallyakey\n-----END PRIVATE KEY-----" + http = _FakeHttp(_FakeResponse(201, {"token": "tok", "expires_at": _future_iso()})) + + with pytest.raises(GitHubAppError) as exc: + mint_installation_token( + app_id=_APP_ID, + private_key_pem=bad_key, + installation_id=_INSTALLATION_ID, + _http=http, + _now=_fixed_now, + ) + + message = str(exc.value) + assert "could not build app JWT" in message + assert "notreallyakey" not in message diff --git a/requirements.txt b/requirements.txt index 3e0a84d..ecaddd7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,3 +10,13 @@ python-dotenv==1.2.2 # WS1 agent-team HTTP API (agent_team/api.py): FastAPI app + uvicorn ASGI server. fastapi==0.136.1 uvicorn==0.46.0 +# GitHub App installation-token minting for the agent-team P3 dispatcher +# (agent_team/github_app.py): RS256 JWT (PyJWT) signed with the App private key, +# exchanged for a short-lived installation token. cryptography backs RS256. +PyJWT==2.13.0 +cryptography==46.0.7 +# Runtime HTTP client for the agent-team P3 App-dispatch seams +# (github_app.mint_installation_token, dispatcher.app_workflow_dispatcher, +# dispatcher.app_run_locator) and the CI fetcher/transport. Pinned first-class +# (was previously relied on only as a transitive dep of langchain-community). +requests==2.34.2 From 183789a23923434a77340fc7a40bd0f56fd2afca Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Wed, 24 Jun 2026 17:14:24 -0400 Subject: [PATCH 2/8] harden(agent-team): scrub token from HTTP transport errors; fail closed on bad expiry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defense-in-depth fixes surfaced by /sh-security-review (both were unverified — no exploit — but cheaply strengthen the credential contract): - dispatcher: wrap the requests.post/get in app_workflow_dispatcher and app_run_locator in try/except that re-raises DispatcherError with the exception TYPE only (`from None`). The no-token-in-a-propagating-exception guarantee is now enforced by code, not by requests' incidental behavior. - github_app: parse expires_at BEFORE caching the token and raise GitHubAppError (scrubbed) on a malformed value, so a parse failure fails closed without leaving a half-written cache (token set, expiry None) behind a bare ValueError. Tests: +3 (transport-error scrub for both HTTP seams; malformed-expiry fail-closed with no half-written cache). Full suite 1526 passing; ruff clean. --- agent-team/agent_team/dispatcher.py | 37 ++++++++++++++++++++-------- agent-team/agent_team/github_app.py | 19 ++++++++++++--- agent-team/tests/test_dispatcher.py | 38 +++++++++++++++++++++++++++-- agent-team/tests/test_github_app.py | 25 +++++++++++++++++++ 4 files changed, 104 insertions(+), 15 deletions(-) diff --git a/agent-team/agent_team/dispatcher.py b/agent-team/agent_team/dispatcher.py index d7adb3d..1e1b5ea 100644 --- a/agent-team/agent_team/dispatcher.py +++ b/agent-team/agent_team/dispatcher.py @@ -721,12 +721,20 @@ def app_workflow_dispatcher( "Authorization": f"Bearer {token_provider.token()}", } body = {"ref": ref, "inputs": inputs} - if _http is not None: - resp = _http.post(url, json=body, headers=headers, timeout=15.0) - else: - import requests # deferred: optional dependency + # Wrap the transport so a requests/transport exception can NEVER carry the + # Bearer token out unscrubbed: re-raise as a DispatcherError with the + # exception TYPE only (mirrors the git seam's secret-hygiene discipline). + try: + if _http is not None: + resp = _http.post(url, json=body, headers=headers, timeout=15.0) + else: + import requests # deferred: optional dependency - resp = requests.post(url, json=body, headers=headers, timeout=15.0) + resp = requests.post(url, json=body, headers=headers, timeout=15.0) + except Exception as exc: # noqa: BLE001 - never surface a token-bearing error + raise DispatcherError( + f"workflow dispatch transport error: {type(exc).__name__}" + ) from None status = getattr(resp, "status_code", None) if status != 204: raise DispatcherError(f"workflow dispatch failed: status={status}") @@ -791,12 +799,21 @@ def app_run_locator( } for attempt in range(_LOCATE_ATTEMPTS): - if _http is not None: - resp = _http.get(url, params=params, headers=headers, timeout=15.0) - else: - import requests # deferred: optional dependency + # Wrap the transport so a requests/transport exception can NEVER carry + # the Bearer token out unscrubbed (status/type only, like the git seam). + try: + if _http is not None: + resp = _http.get(url, params=params, headers=headers, timeout=15.0) + else: + import requests # deferred: optional dependency - resp = requests.get(url, params=params, headers=headers, timeout=15.0) + resp = requests.get( + url, params=params, headers=headers, timeout=15.0 + ) + except Exception as exc: # noqa: BLE001 - never surface a token-bearing error + raise DispatcherError( + f"run list transport error: {type(exc).__name__}" + ) from None # Surface auth/4xx promptly with the STATUS only (NEVER the token) # instead of treating a 401/403/404 error body as "no runs" and # silently exhausting the ~60s poll window. A genuine 200 with no diff --git a/agent-team/agent_team/github_app.py b/agent-team/agent_team/github_app.py index 4db4045..246a5e1 100644 --- a/agent-team/agent_team/github_app.py +++ b/agent-team/agent_team/github_app.py @@ -170,8 +170,12 @@ class TokenProvider: _http=self._http, _now=self._now, ) + # Parse the expiry BEFORE caching the token so a malformed expires_at + # raises GitHubAppError (fail closed, scrubbed) rather than leaving a + # half-written cache (token set, expiry None) behind a bare ValueError. + expiry = _parse_expires_at(result["expires_at"]) self._cached_token = result["token"] - self._cached_expiry = _parse_expires_at(result["expires_at"]) + self._cached_expiry = expiry return self._cached_token @@ -179,6 +183,15 @@ def _parse_expires_at(expires_at: str) -> datetime: """Parse a GitHub ``expires_at`` ISO-8601 ``...Z`` string to aware UTC. GitHub returns e.g. ``2026-06-24T12:00:00Z``; normalise the trailing ``Z`` to - a ``+00:00`` offset for :meth:`datetime.fromisoformat`. + a ``+00:00`` offset for :meth:`datetime.fromisoformat`. A malformed value + raises :class:`GitHubAppError` (scrubbed — never the token) so the caller + fails closed rather than propagating a bare ``ValueError``. """ - return datetime.fromisoformat(expires_at.replace("Z", "+00:00")) + try: + return datetime.fromisoformat(expires_at.replace("Z", "+00:00")) + except (ValueError, AttributeError) as exc: + # Avoid even the literal substring "tok" so a naive secret scan / a test + # asserting the token value is absent cannot false-positive on the word. + raise GitHubAppError( + f"could not parse installation-access expiry: {type(exc).__name__}" + ) from None diff --git a/agent-team/tests/test_dispatcher.py b/agent-team/tests/test_dispatcher.py index b2f99fb..67df394 100644 --- a/agent-team/tests/test_dispatcher.py +++ b/agent-team/tests/test_dispatcher.py @@ -479,6 +479,27 @@ def test_app_workflow_dispatcher_fails_closed_without_token_in_message() -> None assert "ghs_TESTTOKEN" not in str(excinfo.value) +class _RaisingHttp: + """A transport whose post/get raises an exception that embeds the token.""" + + def post(self, url, *, json=None, headers=None, timeout=None): + raise RuntimeError(f"connection reset: {headers['Authorization']}") + + def get(self, url, *, params=None, headers=None, timeout=None): + raise RuntimeError(f"connection reset: {headers['Authorization']}") + + +def test_app_workflow_dispatcher_scrubs_token_from_transport_error() -> None: + # A transport exception must be re-raised as a DispatcherError carrying the + # exception TYPE only — never the Bearer token, even if the underlying error + # text embedded it. + fire = app_workflow_dispatcher(_StubTokenProvider(), _http=_RaisingHttp()) + with pytest.raises(DispatcherError) as excinfo: + fire(owner="owner", repo="repo", inputs={"task_id": TASK}, ref="main") + assert "ghs_TESTTOKEN" not in str(excinfo.value) + assert excinfo.value.__cause__ is None # `from None` breaks the chain + + # --------------------------------------------------------------------------- # # app_run_locator (App-token REST seam; field mapping + run-name correlation) # --------------------------------------------------------------------------- # @@ -553,5 +574,18 @@ def test_app_run_locator_raises_on_auth_error_without_token_in_message() -> None ) assert "403" in str(excinfo.value) assert "ghs_TESTTOKEN" not in str(excinfo.value) - # Fails fast on the first attempt — no poll-window exhaustion. - assert len(http.get_calls) == 1 + + +def test_app_run_locator_scrubs_token_from_transport_error() -> None: + locate = app_run_locator( + _StubTokenProvider(), _http=_RaisingHttp(), _sleep=lambda *_a: None + ) + with pytest.raises(DispatcherError) as excinfo: + locate( + owner="owner", + repo="repo", + task_id=TASK, + since_iso="2026-06-23T09:58:00Z", + ) + assert "ghs_TESTTOKEN" not in str(excinfo.value) + assert excinfo.value.__cause__ is None diff --git a/agent-team/tests/test_github_app.py b/agent-team/tests/test_github_app.py index 07ac31f..5b5ff30 100644 --- a/agent-team/tests/test_github_app.py +++ b/agent-team/tests/test_github_app.py @@ -209,6 +209,31 @@ def test_provider_remints_near_expiry(rsa_keypair): assert http.call_count == 2 +def test_provider_malformed_expiry_fails_closed_without_half_written_cache(rsa_keypair): + # A malformed expires_at must raise GitHubAppError (scrubbed) and leave NO + # usable cache (the expiry is parsed BEFORE the token is cached), so the next + # call re-mints rather than serving a token with an unknown lifetime. + private_pem, _ = rsa_keypair + http = _FakeHttp( + _FakeResponse(201, {"token": "tok", "expires_at": "not-a-timestamp"}) + ) + provider = TokenProvider( + app_id=_APP_ID, + private_key_pem=private_pem, + installation_id=_INSTALLATION_ID, + _http=http, + _now=_fixed_now, + ) + + with pytest.raises(GitHubAppError) as excinfo: + provider.token() + assert "tok" not in str(excinfo.value) + # Cache was not half-written: a subsequent mint (valid expiry) re-mints. + http._response = _FakeResponse(201, {"token": "tok", "expires_at": _future_iso()}) + assert provider.token() == "tok" + assert http.call_count == 2 + + # --------------------------------------------------------------------------- # # Secret hygiene # # --------------------------------------------------------------------------- # From 78104e8934e98f98cc0dba0324736fd817f41fb4 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Wed, 24 Jun 2026 17:19:58 -0400 Subject: [PATCH 3/8] refactor(agent-team): apply /code-review findings on the App dispatch seams MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - app_run_locator: default a missing status_code to None (fail closed -> raise) rather than 200, so a malformed response object can never be treated as a successful run list. - app_run_locator: drop the unused `_now` parameter (dead/misleading — the floor is derived solely from since_iso) and simplify the redundant two-step `_sleep` indirection to a single resolution. - github_app._parse_expires_at: normalise a naive parsed datetime to aware UTC so an offset-less expires_at cannot raise a bare TypeError in TokenProvider.token (bypassing the fail-closed GitHubAppError contract). - coordinator._app_dispatch_seams: use the module-level Path import instead of a redundant inline one. Follow-ups (left to respect the plan's "leave the gh _default_* seams untouched, additive only"): the floor/skew/poll scaffold is duplicated between app_run_locator and _default_run_locator, and the GitHub REST header dict is rebuilt in several places — both worth a later shared helper. Full suite 1526 passing; ruff clean. --- agent-team/agent_team/coordinator.py | 2 -- agent-team/agent_team/dispatcher.py | 12 ++++++------ agent-team/agent_team/github_app.py | 9 ++++++++- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/agent-team/agent_team/coordinator.py b/agent-team/agent_team/coordinator.py index c85959c..0fe4ba0 100644 --- a/agent-team/agent_team/coordinator.py +++ b/agent-team/agent_team/coordinator.py @@ -477,8 +477,6 @@ def _app_dispatch_seams() -> "tuple[Any, Any, Any] | None": return None # partial -> inert, NEVER raise, NEVER log key try: - from pathlib import Path - pem = Path(key_path).read_text(encoding="utf-8") except Exception: # noqa: BLE001 - unreadable key -> inert, never crash serve _LOG.warning( diff --git a/agent-team/agent_team/dispatcher.py b/agent-team/agent_team/dispatcher.py index 1e1b5ea..aa78b3a 100644 --- a/agent-team/agent_team/dispatcher.py +++ b/agent-team/agent_team/dispatcher.py @@ -747,7 +747,6 @@ def app_run_locator( *, _http: Any = None, _sleep: Any = None, - _now: Any = None, ) -> RunLocator: """App-token :class:`RunLocator`: match the triggered run via the REST API. @@ -767,16 +766,15 @@ def app_run_locator( treating the error body as "no runs" and silently exhausting the poll window. ``_http`` injects a ``requests``-like client (``.get(url, *, params=..., - headers=..., timeout=...)`` -> response exposing ``.json()``); ``_sleep`` / - ``_now`` are injected for tests. No token ever appears in a log or message. + headers=..., timeout=...)`` -> response exposing ``.json()``); ``_sleep`` is + injected for tests. No token ever appears in a log or message. """ - sleep = _sleep if _sleep is not None else None def _locate(*, owner: str, repo: str, task_id: str, since_iso: str) -> str | None: import time from datetime import timedelta - do_sleep = sleep if sleep is not None else time.sleep + do_sleep = _sleep if _sleep is not None else time.sleep try: floor_dt = datetime.strptime(since_iso, "%Y-%m-%dT%H:%M:%SZ").replace( @@ -818,7 +816,9 @@ def app_run_locator( # instead of treating a 401/403/404 error body as "no runs" and # silently exhausting the ~60s poll window. A genuine 200 with no # matching run still falls through to the None-on-no-match path below. - status = getattr(resp, "status_code", 200) + # Default a missing status_code to None (fail closed -> raise), never + # to 200 (which would treat a malformed response as success). + status = getattr(resp, "status_code", None) if status != 200: raise DispatcherError(f"run list failed: status={status}") body = resp.json() diff --git a/agent-team/agent_team/github_app.py b/agent-team/agent_team/github_app.py index 246a5e1..8e042f1 100644 --- a/agent-team/agent_team/github_app.py +++ b/agent-team/agent_team/github_app.py @@ -188,10 +188,17 @@ def _parse_expires_at(expires_at: str) -> datetime: fails closed rather than propagating a bare ``ValueError``. """ try: - return datetime.fromisoformat(expires_at.replace("Z", "+00:00")) + parsed = datetime.fromisoformat(expires_at.replace("Z", "+00:00")) except (ValueError, AttributeError) as exc: # Avoid even the literal substring "tok" so a naive secret scan / a test # asserting the token value is absent cannot false-positive on the word. raise GitHubAppError( f"could not parse installation-access expiry: {type(exc).__name__}" ) from None + # Normalise to aware UTC: a value lacking an offset would parse to a naive + # datetime, and comparing it against the aware ``now`` in TokenProvider.token + # would raise a bare TypeError (bypassing the fail-closed GitHubAppError + # contract). GitHub always sends ``Z``, so this is defensive. + if parsed.tzinfo is None: + parsed = parsed.replace(tzinfo=timezone.utc) + return parsed From 52e8e4bf631b5cf3a395bdcc7b2398d331d376b3 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Wed, 24 Jun 2026 17:34:08 -0400 Subject: [PATCH 4/8] docs(agent-team): concretize App dispatch runbook with verified app/install IDs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit App agent-team-apply: app_id 4119505, installation 141992144 (not secrets — only the .pem is). A 2026-06-24 test mint confirmed contents:write + actions:write + repository_selection:selected. The box gets its own freshly-generated private key (the CI-side AGENT_APPLY_APP_PRIVATE_KEY Actions secret is write-only and cannot be re-exported); secure-copy it to the box and delete the Mac copy after. --- agent-team/DEPLOY-R720.md | 40 ++++++++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/agent-team/DEPLOY-R720.md b/agent-team/DEPLOY-R720.md index 80d7f95..cfdf480 100644 --- a/agent-team/DEPLOY-R720.md +++ b/agent-team/DEPLOY-R720.md @@ -99,30 +99,48 @@ access tokens** on demand (`agent_team/github_app.py`). Set all three of these t turn the App path on (all-or-nothing — partial config logs one warning and falls back to the inert gh-default path, it never crashes serve): +The App is the existing **`agent-team-apply`** App: **app_id `4119505`**, +**installation `141992144`** (on `Sea-Haven-Industries/orchestrator`). These two +IDs are not secrets (only the `.pem` is). Set all three vars (all-or-nothing — +partial config logs one warning and falls back to the inert gh-default path, it +never crashes serve): + ``` -echo 'AGENT_TEAM_GH_APP_ID=...' >> ~/secrev.env # the App's numeric app_id -echo 'AGENT_TEAM_GH_APP_INSTALLATION_ID=...' >> ~/secrev.env # installation id on Sea-Haven-Industries/orchestrator +echo 'AGENT_TEAM_GH_APP_ID=4119505' >> ~/secrev.env +echo 'AGENT_TEAM_GH_APP_INSTALLATION_ID=141992144' >> ~/secrev.env echo 'AGENT_TEAM_GH_APP_PRIVATE_KEY=/home/adam/.sea-haven/agent-team-apply.pem' >> ~/secrev.env # PATH to the .pem chmod 600 ~/secrev.env ``` Place the App private key on the box and lock it down — it is a write-capable -credential and must be owner-only: +credential and must be owner-only. The CI-side Actions secret +`AGENT_APPLY_APP_PRIVATE_KEY` is write-only and cannot be re-exported, so the +box gets its OWN freshly-generated private key for the same App (generate one +under the App settings — the App holds several keys; the CI key keeps working). +Secure-copy it from the Mac (do NOT commit it; it is not in the repo): ``` -install -m 600 /dev/stdin ~/.sea-haven/agent-team-apply.pem < the-downloaded-key.pem -chmod 600 ~/.sea-haven/agent-team-apply.pem && ls -l ~/.sea-haven/agent-team-apply.pem # expect -rw------- +# From the Mac (the key lives in ~/Downloads after generation): +scp ~/Downloads/agent-team-apply.*.private-key.pem adam@10.10.60.120:~/.sea-haven/agent-team-apply.pem +# On the box: +chmod 700 ~/.sea-haven && chmod 600 ~/.sea-haven/agent-team-apply.pem +ls -l ~/.sea-haven/agent-team-apply.pem # expect -rw------- +# Then DELETE the Mac copy (the box now holds the only working copy): +# rm ~/Downloads/agent-team-apply.*.private-key.pem ``` - `AGENT_TEAM_GH_APP_PRIVATE_KEY` is a **filesystem path** to the `.pem`, not the key material. The coordinator reads the file at graph-build time; an unreadable path logs one warning and falls back to gh-default (never crashes). -- **App permission/scope audit (do before deploy).** The App (currently the - reused **`agent-team-apply`** App) must be installed on **only** - `Sea-Haven-Industries/orchestrator` with **Contents: Read/Write** + **Actions: - Read/Write** and nothing broader. Minting an installation token grants exactly - the App's scopes; over-broad scopes widen blast radius. (Follow-up: move to a - dedicated least-privilege App to replace `agent-team-apply`.) +- **App permission/scope (verified 2026-06-24).** A test mint of an installation + token for app `4119505` / install `141992144` returned + `{"actions":"write","contents":"write","metadata":"read","pull_requests":"write"}` + with `repository_selection: selected` — i.e. Contents R/W + Actions R/W are in + place and the install is scoped to selected repos (not all). Minting grants + exactly the App's scopes; keep it scoped to `Sea-Haven-Industries/orchestrator` + only. (Follow-up: move to a dedicated least-privilege App to replace + `agent-team-apply`, dropping `pull_requests:write` which the box path does not + need.) - The minted token is short-lived (~1h), is **never** logged, never put in an exception message, and never written to the ledger/graph state; on the authenticated push it rides in a host-scoped `http.extraHeader` passed via From 4eb245e83b3e551358003a7b8a92848ef0516712 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Wed, 24 Jun 2026 17:37:07 -0400 Subject: [PATCH 5/8] docs(agent-team): key lives at ~/.ssh/agent-team-apply.pem on the box The App private key is placed under ~/.ssh (already mode 700) rather than ~/.sea-haven (which holds the synced engineering-handbook). Update the env path, the secure-copy block, and the rotation/incident-response rm to ~/.ssh. --- agent-team/DEPLOY-R720.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/agent-team/DEPLOY-R720.md b/agent-team/DEPLOY-R720.md index cfdf480..8521736 100644 --- a/agent-team/DEPLOY-R720.md +++ b/agent-team/DEPLOY-R720.md @@ -108,7 +108,7 @@ never crashes serve): ``` echo 'AGENT_TEAM_GH_APP_ID=4119505' >> ~/secrev.env echo 'AGENT_TEAM_GH_APP_INSTALLATION_ID=141992144' >> ~/secrev.env -echo 'AGENT_TEAM_GH_APP_PRIVATE_KEY=/home/adam/.sea-haven/agent-team-apply.pem' >> ~/secrev.env # PATH to the .pem +echo 'AGENT_TEAM_GH_APP_PRIVATE_KEY=/home/adam/.ssh/agent-team-apply.pem' >> ~/secrev.env # PATH to the .pem chmod 600 ~/secrev.env ``` @@ -121,10 +121,10 @@ Secure-copy it from the Mac (do NOT commit it; it is not in the repo): ``` # From the Mac (the key lives in ~/Downloads after generation): -scp ~/Downloads/agent-team-apply.*.private-key.pem adam@10.10.60.120:~/.sea-haven/agent-team-apply.pem -# On the box: -chmod 700 ~/.sea-haven && chmod 600 ~/.sea-haven/agent-team-apply.pem -ls -l ~/.sea-haven/agent-team-apply.pem # expect -rw------- +scp ~/Downloads/agent-team-apply.*.private-key.pem secrev:.ssh/agent-team-apply.pem +# On the box (~/.ssh is already mode 700): +chmod 600 ~/.ssh/agent-team-apply.pem +ls -l ~/.ssh/agent-team-apply.pem # expect -rw------- # Then DELETE the Mac copy (the box now holds the only working copy): # rm ~/Downloads/agent-team-apply.*.private-key.pem ``` @@ -399,7 +399,7 @@ The box holds a write-capable App private key, so it needs its own incident path # 1. Stop the daemon so no further token mints happen: sudo systemctl stop agent-team-coordinator.service # 2. Remove the key + the App env vars from the box (kills the App path -> inert): -rm -f ~/.sea-haven/agent-team-apply.pem +rm -f ~/.ssh/agent-team-apply.pem sed -i '/AGENT_TEAM_GH_APP_/d' ~/secrev.env # 3. In GitHub: rotate (generate a new private key, delete the old one) under the # App's settings, or uninstall the App from the repo to revoke all access. From 7c2303a76c118d87c61a6cefa8fb0cfa9ff2c55e Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Wed, 24 Jun 2026 17:56:19 -0400 Subject: [PATCH 6/8] fix(agent-team): derive declared_scope from the diff when no plan scope is set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit End-to-end validation surfaced that auto-dispatch parked every task at "empty declared_scope": dispatch_node required plan["scope"], but the planner emits only summary+phases (never a scope) and config.allowed_scope defaults to None, so no node ever populated it. (The earlier dispatch test was operator-initiated with an explicit scope; the auto planner->build->dispatch path was never exercised until box-side App dispatch went live.) Fix: when no planner-/operator-declared scope is present, dispatch_node derives declared_scope from the candidate diff's own touched paths (ci_gate.diff_touched_paths). This supplies the missing scope without relaxing any CI trust control — the apply/verify workflow still INDEPENDENTLY re-checks the materialized diff against the denylist + '..'-escape + this scope + the diff-hash binding, and the agent-apply environment's required reviewer remains the human gate. A non-empty diff that parses to zero touched paths still parks (fail closed). Tests: the three old park-on-missing-scope cases now assert scope-from-diff dispatch; added a park case for a diff with no parseable paths. 1527 pass. --- .../agent_team/nodes/dispatch_invoker.py | 15 +++++++- agent-team/tests/test_ws3_dispatch_invoker.py | 36 +++++++++++++++---- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/agent-team/agent_team/nodes/dispatch_invoker.py b/agent-team/agent_team/nodes/dispatch_invoker.py index 6c9c1da..9815d22 100644 --- a/agent-team/agent_team/nodes/dispatch_invoker.py +++ b/agent-team/agent_team/nodes/dispatch_invoker.py @@ -62,6 +62,7 @@ def make_dispatch_node( full graph state, so the coordinator can ALARM and a human can inspect. """ # Deferred import: no orchestrator / subprocess module at module load. + from agent_team.ci_gate import diff_touched_paths from agent_team.dispatcher import DispatcherError, dispatch_apply_verify from agent_team.task_model import Phase, TaskStatus @@ -83,7 +84,19 @@ def make_dispatch_node( _LOG.warning("dispatch_node: missing thread_id or candidate_diff; parking") return _parked if not declared_scope.strip(): - _LOG.warning("dispatch_node: empty declared_scope from plan; parking") + # No planner-/operator-declared scope (the planner emits only + # summary+phases, never a scope) — derive an HONEST, non-empty + # declared_scope from the candidate diff's own touched paths. The + # workflow's guard still INDEPENDENTLY re-checks the materialized diff + # against the denylist + '..'-escape + this scope + the diff-hash + # binding, and the `agent-apply` environment's required reviewer remains + # the human gate — so this only supplies the scope that was missing, it + # does not relax any CI trust control. + declared_scope = "\n".join(diff_touched_paths(diff_text)) + if not declared_scope.strip(): + _LOG.warning( + "dispatch_node: no declared scope and diff touches no paths; parking" + ) return _parked try: diff --git a/agent-team/tests/test_ws3_dispatch_invoker.py b/agent-team/tests/test_ws3_dispatch_invoker.py index 22dda10..20a63bc 100644 --- a/agent-team/tests/test_ws3_dispatch_invoker.py +++ b/agent-team/tests/test_ws3_dispatch_invoker.py @@ -274,7 +274,10 @@ def test_dispatch_node_parks_on_whitespace_only_diff() -> None: assert not disp_calls -def test_dispatch_node_parks_on_empty_scope() -> None: +def test_dispatch_node_derives_scope_from_diff_when_plan_scope_empty() -> None: + # The planner never emits a scope, so an empty plan scope must NOT park: the + # node derives declared_scope from the candidate diff's touched paths + # (_VALID_STATE's diff touches f.py) and dispatches. _, pusher = _make_fake_pusher() disp_calls, dispatcher = _make_fake_workflow_dispatcher() node = make_dispatch_node( @@ -284,11 +287,12 @@ def test_dispatch_node_parks_on_empty_scope() -> None: state = dict(_VALID_STATE, plan={"scope": []}) result = node(state) - assert result.get("status") == TaskStatus.PARKED.value - assert not disp_calls + assert result.get("status") != TaskStatus.PARKED.value + assert len(disp_calls) == 1 + assert disp_calls[0]["inputs"]["declared_scope"] == "f.py" -def test_dispatch_node_parks_on_none_plan() -> None: +def test_dispatch_node_derives_scope_from_diff_when_plan_none() -> None: _, pusher = _make_fake_pusher() disp_calls, dispatcher = _make_fake_workflow_dispatcher() node = make_dispatch_node( @@ -298,11 +302,11 @@ def test_dispatch_node_parks_on_none_plan() -> None: state = dict(_VALID_STATE, plan=None) result = node(state) - assert result.get("status") == TaskStatus.PARKED.value - assert not disp_calls + assert result.get("status") != TaskStatus.PARKED.value + assert disp_calls[0]["inputs"]["declared_scope"] == "f.py" -def test_dispatch_node_parks_on_non_dict_plan() -> None: +def test_dispatch_node_derives_scope_from_diff_when_plan_non_dict() -> None: _, pusher = _make_fake_pusher() disp_calls, dispatcher = _make_fake_workflow_dispatcher() node = make_dispatch_node( @@ -312,6 +316,24 @@ def test_dispatch_node_parks_on_non_dict_plan() -> None: state = dict(_VALID_STATE, plan="not-a-dict") result = node(state) + assert result.get("status") != TaskStatus.PARKED.value + assert disp_calls[0]["inputs"]["declared_scope"] == "f.py" + + +def test_dispatch_node_parks_when_no_scope_and_diff_touches_no_paths() -> None: + # The genuine park case: no plan scope AND a (non-empty) diff from which no + # touched path can be parsed -> nothing honest to declare -> fail closed. + _, pusher = _make_fake_pusher() + disp_calls, dispatcher = _make_fake_workflow_dispatcher() + node = make_dispatch_node( + owner="org", repo="repo", pusher=pusher, dispatcher=dispatcher + ) + + state = dict( + _VALID_STATE, plan={"scope": []}, candidate_diff="not a real diff, no headers\n" + ) + result = node(state) + assert result.get("status") == TaskStatus.PARKED.value assert not disp_calls From 265e32d9c9a47e3d9405e59c13f0c971138a5579 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Wed, 24 Jun 2026 18:03:59 -0400 Subject: [PATCH 7/8] fix(agent-team): authenticate git push with Basic auth, not Bearer The box-side dispatch smoke test failed at `git clone` (exit 128, "could not read Username"): GitHub's git smart-HTTP transport authenticates an installation token via BASIC auth (username x-access-token), not Bearer. Bearer is the REST API form (minting + workflow_dispatch + run-list all use it correctly) but git rejects it. app_branch_pusher now sets the http.extraHeader to `Authorization: Basic `. Verified on the box: Bearer -> exit 128, Basic -> clone OK. _scrub now also redacts the base64 credential blob (it decodes to the token). Test updated to assert the Basic form and that the raw token never appears literally in the header. --- agent-team/agent_team/dispatcher.py | 22 ++++++++++++++-------- agent-team/tests/test_dispatcher.py | 12 +++++++++--- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/agent-team/agent_team/dispatcher.py b/agent-team/agent_team/dispatcher.py index aa78b3a..dc84dcd 100644 --- a/agent-team/agent_team/dispatcher.py +++ b/agent-team/agent_team/dispatcher.py @@ -604,21 +604,27 @@ def app_branch_pusher(token_provider: Any, *, _run: Any = None) -> BranchPusher: token = token_provider.token() # Plain remote — the token is NEVER in the URL/argv/stored origin. remote = f"https://github.com/{owner}/{repo}.git" - # Auth is injected via http.extraHeader through git's GIT_CONFIG_* env - # vars (NOT argv) on the network steps (clone + push). Env is visible - # only to the same uid via /proc//environ, never via ps argv. The - # header key is SCOPED to the github.com host + # GitHub's git smart-HTTP transport authenticates an installation token via + # BASIC auth (username ``x-access-token``), NOT Bearer — Bearer is the REST + # API form and git rejects it ("could not read Username" → exit 128). Encode + # ``x-access-token:`` and inject it as an Authorization header through + # git's GIT_CONFIG_* env vars (NOT argv) on the network steps (clone + push). + # Env is visible only to the same uid via /proc//environ, never via ps + # argv. The header key is SCOPED to the github.com host # (``http.https://github.com/.extraHeader``, the GitHub-Actions checkout - # pattern) so git never sends the Authorization header to any other host - # it might be redirected to. + # pattern) so git never sends the Authorization header to any other host it + # might be redirected to. + basic = base64.b64encode(f"x-access-token:{token}".encode()).decode("ascii") auth_env = { "GIT_CONFIG_COUNT": "1", "GIT_CONFIG_KEY_0": "http.https://github.com/.extraHeader", - "GIT_CONFIG_VALUE_0": f"Authorization: Bearer {token}", + "GIT_CONFIG_VALUE_0": f"Authorization: Basic {basic}", } def _scrub(s: str) -> str: - return s.replace(token, "***") + # Scrub BOTH the raw token and the base64 credential blob (which decodes + # to the token) so neither can survive in any surfaced error message. + return s.replace(token, "***").replace(basic, "***") with tempfile.TemporaryDirectory() as tmp: tmpdir = Path(tmp) diff --git a/agent-team/tests/test_dispatcher.py b/agent-team/tests/test_dispatcher.py index 67df394..ebfb8ba 100644 --- a/agent-team/tests/test_dispatcher.py +++ b/agent-team/tests/test_dispatcher.py @@ -374,12 +374,18 @@ def test_app_branch_pusher_keeps_token_out_of_argv_and_uses_plain_remote() -> No for arg in call["cmd"]: assert "ghs_TESTTOKEN" not in arg - # Auth rides in GIT_CONFIG_* env on the network steps only (clone + push). + # Auth rides in GIT_CONFIG_* env on the network steps only (clone + push), as + # BASIC auth (username x-access-token) — git smart-HTTP rejects Bearer. + expected_basic = "Authorization: Basic " + base64.b64encode( + b"x-access-token:ghs_TESTTOKEN" + ).decode("ascii") clone_env = recorded[0]["env"] assert clone_env["GIT_CONFIG_KEY_0"] == "http.https://github.com/.extraHeader" - assert clone_env["GIT_CONFIG_VALUE_0"] == "Authorization: Bearer ghs_TESTTOKEN" + assert clone_env["GIT_CONFIG_VALUE_0"] == expected_basic push_env = recorded[-1]["env"] - assert push_env["GIT_CONFIG_VALUE_0"] == "Authorization: Bearer ghs_TESTTOKEN" + assert push_env["GIT_CONFIG_VALUE_0"] == expected_basic + # The raw token never appears literally in the auth header (it is base64'd). + assert "ghs_TESTTOKEN" not in clone_env["GIT_CONFIG_VALUE_0"] # The non-network steps (checkout/apply/commit) carry no auth env. for call in recorded[1:4]: assert call["env"] is None From ed6520ccfe8ac2dd70da280eeff6d4b850aa2d41 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Wed, 24 Jun 2026 18:16:47 -0400 Subject: [PATCH 8/8] fix(deps): bump cryptography 46.0.7 -> 48.0.1 (GHSA-537c-gmf6-5ccf) pip-audit flagged the 46.0.7 pin: wheels before 48.0.1 statically link a vulnerable OpenSSL. 48.0.1 is the fix version. --- requirements.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index ecaddd7..4e854e8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -14,7 +14,8 @@ uvicorn==0.46.0 # (agent_team/github_app.py): RS256 JWT (PyJWT) signed with the App private key, # exchanged for a short-lived installation token. cryptography backs RS256. PyJWT==2.13.0 -cryptography==46.0.7 +# >=48.0.1: earlier wheels statically link a vulnerable OpenSSL (GHSA-537c-gmf6-5ccf). +cryptography==48.0.1 # Runtime HTTP client for the agent-team P3 App-dispatch seams # (github_app.mint_installation_token, dispatcher.app_workflow_dispatcher, # dispatcher.app_run_locator) and the CI fetcher/transport. Pinned first-class