From dc3580dac49219ca0c843262b2f847fb46d7ba00 Mon Sep 17 00:00:00 2001 From: haruotsu <65439874+haruotsu@users.noreply.github.com> Date: Wed, 30 Sep 2026 18:51:45 +0900 Subject: [PATCH 1/2] Add a github-app connection whose tokens gete issues itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some agents need to read the same repositories whoever calls them, and a per-user token forwarded by Gemini Enterprise cannot give them that. A GitHub App installation token can, but until now the only way to use one was a python tool holding its own credential, outside the connection's host check and the openapi block's operations, params, and does_not. The new connection kind keeps every one of those guards: the token is issued from the App's private key inside gete's client, narrowed on every issue to the repositories and permissions gete.yaml declares, and sent only to the connection's hosts. Nobody approves anything, so register creates no authorization and the agent offers no reauthorization tool; a missing key or a refused issue reaches the user as text. The key is delivered like secret_env, so an agent cannot swap it for its own. MCP blocks are refused for now: ADK reads MCP headers synchronously, while issuing a token is a request of its own. 🤖 Generated with Claude Code --- README.md | 60 ++++- pyproject.toml | 3 + src/gete/catalog/connections/github-app.yaml | 42 +++ src/gete/cli/__init__.py | 7 +- src/gete/connection/checks.py | 70 ++++- src/gete/connection/client.py | 23 +- src/gete/connection/github_app.py | 266 +++++++++++++++++++ src/gete/connection/registry.py | 68 ++++- src/gete/connections_listing.py | 48 +++- src/gete/declaration.py | 18 ++ src/gete/register.py | 49 +++- src/gete/run.py | 16 +- src/gete/runtime/openapi.py | 4 + src/gete/runtime/tools.py | 5 +- src/gete/schema/connection.json | 59 +++- src/gete/secrets.py | 9 +- src/gete/terraform.py | 8 +- src/gete/validate.py | 35 ++- tests/test_check_secrets.py | 37 +++ tests/test_connection_client.py | 83 ++++++ tests/test_connection_registry.py | 77 ++++++ tests/test_connections_command.py | 31 +++ tests/test_github_app.py | 261 ++++++++++++++++++ tests/test_register.py | 58 ++++ tests/test_run_command.py | 29 ++ tests/test_runtime_openapi.py | 61 +++++ tests/test_terraform.py | 35 +++ tests/test_validate.py | 132 +++++++++ uv.lock | 2 + 29 files changed, 1538 insertions(+), 58 deletions(-) create mode 100644 src/gete/catalog/connections/github-app.yaml create mode 100644 src/gete/connection/github_app.py create mode 100644 tests/test_github_app.py diff --git a/README.md b/README.md index 1c4d892..13953ff 100644 --- a/README.md +++ b/README.md @@ -239,7 +239,7 @@ tools: ### Connections `gete connections` lists what ships: `freee`, `freee-mcp`, `google`, `github`, -`notion-mcp`, `slack-mcp`, and `zendesk`. Add your own or override a catalog +`github-app`, `notion-mcp`, `slack-mcp`, and `zendesk`. Add your own or override a catalog entry in `gete.yaml`: ```yaml @@ -422,6 +422,64 @@ connections: Adding a connection to the catalog is one YAML file under `src/gete/catalog/connections/`; the conformance tests check it. +### Connections gete issues tokens for + +Some reads have no user's token behind them either: the agent is meant to +read the same repositories whoever calls it. `github-app` is a connection whose tokens gete issues itself, from +a GitHub App's private key, instead of receiving them from Gemini Enterprise. +Agents use it from `openapi` blocks (and python tools through gete's client) +exactly like any other connection — `operations`, `params`, `does_not`, and +the host check all apply as before: + +```yaml +# gete.yaml +connections: + github-app: + base_url: https://ghe.example.com/api/v3 # leave out for github.com + app: + app_id: "123" + private_key_secret: ge-github-app-private-key + # The ceiling of every token issued through this connection + repositories: [example-org/requests] + permissions: {issues: read} + +# agent.yaml +connections: [github-app] +tools: + - openapi: + spec: ./specs/github.yaml + connection: github-app + effect: read + operations: [SearchIssues, GetIssue, ListIssueComments] + params: + SearchIssues: + q: {prefix: "repo:example-org/requests is:issue "} +``` + +For such a connection gete: + +- signs an RS256 App JWT from `app_id` and the key, backdated a minute and + valid for less than ten, and finds the installation through the first of + `repositories` the App is installed on + (`GET /repos/{owner}/{repo}/installation`); +- asks for an installation token narrowed to `repositories` and + `permissions`, and reuses it in the process until a few minutes before + its `expires_at`; +- creates no Gemini Enterprise authorization and offers no reauthorization + tool. A missing key, or GitHub refusing to issue a token, is reported to + the user as text; +- delivers the key like `secret_env`: `private_key_secret` reaches the + deployment as `GETE_APP_KEY_GITHUB_APP`, which the agent cannot set + itself. The App ID and the ceiling travel in the resolved declaration. + `gete run` reads the PEM from the same variable. + +`repositories` must share one owner, since a token comes from one +installation. `permissions` is required: left out, a token would carry +everything the installation was granted. Whoever can call the agent acts as +the App within that ceiling, whatever they could reach on GitHub +themselves, so keep it to what the agent's tools read. `mcp` blocks cannot +use an app connection yet. + ### Shared credentials A connection reads with the caller's token. Some writes have no such token diff --git a/pyproject.toml b/pyproject.toml index 4c2c8b3..45882ed 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -15,6 +15,9 @@ dependencies = [ # The mcp extra is what makes mcp: tools work; without it the toolset # module fails to import where the agent is deployed. "google-adk[mcp]>=2.6,<2.9", + # Signs a GitHub App's JWT for the app connections. ADK brings it in + # already; it is named because gete imports it itself. + "cryptography>=43", "httpx>=0.28", "jsonschema>=4.23", "pyyaml>=6.0", diff --git a/src/gete/catalog/connections/github-app.yaml b/src/gete/catalog/connections/github-app.yaml new file mode 100644 index 0000000..33c4932 --- /dev/null +++ b/src/gete/catalog/connections/github-app.yaml @@ -0,0 +1,42 @@ +id: github-app +display_name: GitHub App +docs: https://docs.github.com/en/apps/creating-github-apps/authenticating-with-a-github-app/authenticating-as-a-github-app-installation + +# Tokens are issued by gete from the App's private key, not handed over by +# Gemini Enterprise, so nobody authorizes anything. The root is both where +# tokens are issued and where they are sent. GitHub Enterprise Server serves +# the API from a root of its own; setting base_url in gete.yaml replaces this +# one, and api.github.com is then no longer a host the token may go to. +base_url: https://api.github.com +# ghs_ is an installation access token, the only kind this connection issues. +token_prefixes: + - ghs_ + +# Which App, where its key lives, and what every token is narrowed to differ +# per installation; gete.yaml fills them in. +app: {} + +setup: | + Create a GitHub App (or pick an existing one) and install it on the + account that owns the repositories. + + - Grant the App no more than the permissions the agents need; a token + can be narrowed below the installation's grant, never above it. + - Generate a private key and store the PEM in Secret Manager under the + name given as app.private_key_secret. The key never appears in a + declaration. + - In gete.yaml, set app.app_id, app.private_key_secret, + app.repositories (owner/name, all under one owner), and + app.permissions. Every token gete issues is limited to them. + + Anyone who can call an agent holding this connection acts as the App + within that limit, whatever they could reach on GitHub themselves. + +examples: + accepts: + - "ghs_16C7e42F292c6912E7710c838347Ae178B4a" + rejects: + - "ya29.a0AfH6SMB" # Google access token + - "eyJhbGciOiJSUzI1NiJ9.e30.sig" # JWT + - "gho_16C7e42F292c6912E7710c838347Ae178B4a" # a user's OAuth token + - "ghu_16C7e42F292c6912E7710c838347Ae178B4a" # a user-to-server token diff --git a/src/gete/cli/__init__.py b/src/gete/cli/__init__.py index 1e86c29..4250d50 100644 --- a/src/gete/cli/__init__.py +++ b/src/gete/cli/__init__.py @@ -261,17 +261,18 @@ def run(name: str) -> None: find_agent, initial_state, missing_tokens, + user_authorized, ) try: project = load_project(find_project_file(Path.cwd())) agent = build_local_agent(project, name) - declared = find_agent(project, name) + authorized = user_authorized(project, find_agent(project, name)) except GeteError as error: click.echo(str(error), err=True) sys.exit(1) - state = initial_state(name, declared.connections, os.environ) - missing = missing_tokens(name, declared.connections, state) + state = initial_state(name, authorized, os.environ) + missing = missing_tokens(name, authorized, state) if missing: click.echo( f"no token for {', '.join(missing)}; set GETE_TOKEN_", err=True diff --git a/src/gete/connection/checks.py b/src/gete/connection/checks.py index 2947afa..93b0f99 100644 --- a/src/gete/connection/checks.py +++ b/src/gete/connection/checks.py @@ -3,7 +3,12 @@ from collections.abc import Iterable from urllib.parse import urlsplit -from gete.connection.registry import GOOGLE_ACCESS_TOKEN_PREFIX, Connection, Registry +from gete.connection.registry import ( + GOOGLE_ACCESS_TOKEN_PREFIX, + Connection, + OAuth, + Registry, +) # Platform domains under which unrelated parties host services. Hosts are # matched exactly, so listing one of these is almost certainly a mistake @@ -182,18 +187,8 @@ def connection_problems(connection: Connection, registry: Registry) -> list[str] f"tokens: format {connection.token_format} decides on its own; the " "token_prefixes declared beside it are never read" ) - for scope in sorted(connection.oauth.optional_scopes): - if scope in connection.oauth.scopes: - problems.append( - f"oauth.optional_scopes: {scope} is already a default scope" - ) - if connection.oauth.optional_scopes and connection.oauth.authorization_query: - # The verbatim query is the whole authorization URL; a selection - # would be accepted and then never reach the consent screen. - problems.append( - "oauth.optional_scopes: the menu cannot be offered next to a " - "verbatim authorization_query, which fixes the scopes" - ) + if connection.oauth is not None: + problems.extend(_oauth_problems(connection.oauth)) for token in connection.examples.accepts: if not connection.accepts_token(token): problems.append(f"examples.accepts: {token!r} is not accepted") @@ -218,3 +213,52 @@ def connection_problems(connection: Connection, registry: Registry) -> list[str] ): problems.append(f"mcp.url: {connection.mcp_url} is not covered by hosts") return problems + + +def _oauth_problems(oauth: OAuth) -> list[str]: + problems: list[str] = [] + for scope in sorted(oauth.optional_scopes): + if scope in oauth.scopes: + problems.append( + f"oauth.optional_scopes: {scope} is already a default scope" + ) + if oauth.optional_scopes and oauth.authorization_query: + # The verbatim query is the whole authorization URL; a selection + # would be accepted and then never reach the consent screen. + problems.append( + "oauth.optional_scopes: the menu cannot be offered next to a " + "verbatim authorization_query, which fixes the scopes" + ) + return problems + + +def app_problems(connection: Connection) -> list[str]: + """What an app connection still lacks before a token can be issued. + + A catalog entry leaves the App open, the way it leaves a moving root + open, so the gap is refused where an agent picks the connection up. + """ + app = connection.app + if app is None: + return [] + where = f"connections.{connection.id}.app" + problems = [ + f"{connection.id} has no app.{name}; set {where}.{name} in gete.yaml" + for name, value in ( + ("app_id", app.app_id), + ("private_key_secret", app.private_key_secret), + ("repositories", app.repositories), + # Without it a token carries everything the installation was + # granted, which is exactly what the ceiling is there to stop. + ("permissions", app.permissions), + ) + if not value + ] + owners = sorted({repository.partition("/")[0] for repository in app.repositories}) + if len(owners) > 1: + problems.append( + f"{connection.id}: app.repositories belong to {', '.join(owners)}; a " + "token is issued by one installation, and an installation belongs " + "to one account" + ) + return problems diff --git a/src/gete/connection/client.py b/src/gete/connection/client.py index ad9d23f..d4bc250 100644 --- a/src/gete/connection/client.py +++ b/src/gete/connection/client.py @@ -34,6 +34,7 @@ import httpx +from gete.connection.github_app import AppTokenUnavailable, installation_tokens from gete.connection.registry import Connection from gete.connection.runtime import caller_token, resolve_connection from gete.errors import GeteError, UserFacingError @@ -371,7 +372,10 @@ async def get_bytes( await response.aclose() _check_redirect(connection, target) headers = ( - {**self._headers, **self._authorization(connection, target, state)} + { + **self._headers, + **await self._authorization(connection, target, state), + } if connection.allows(target) # Off the connection's own hosts nothing of ours travels: not # the token, and not the constants that name this service. @@ -392,9 +396,17 @@ async def get_bytes( def _connection(self) -> Connection: return resolve_connection(self._target) - def _authorization( + async def _authorization( self, connection: Connection, url: str, state: Any ) -> dict[str, str]: + if connection.app is not None: + try: + issued = await installation_tokens(connection).token() + except AppTokenUnavailable as error: + # Not a reauthorization: there is nothing for the user to + # approve, and the reason was written to be shown. + raise AuthorizationRefused(str(error)) from None + return {"Authorization": f"Bearer {issued}"} token = caller_token(connection, state) if token is None: # The user sees a re-authorization prompt; operators would not. @@ -429,7 +441,7 @@ async def _request( # is put in last, so nothing can displace it. sent = httpx.Headers(self._headers) sent.update(_refuse_masking_headers(headers or {})) - sent.update(self._authorization(connection, url, state)) + sent.update(await self._authorization(connection, url, state)) # A GET can be sent again because sending it again changes nothing. # Anything else may already have been applied by the time the answer # went missing, so only a refusal the service made before acting - a @@ -457,6 +469,11 @@ async def _request( if response.status_code == 401: await response.aclose() + if connection.app is not None: + # An issued token refused before its time would refuse + # every request until it expired; the next one is issued + # afresh. + installation_tokens(connection).forget() # There is no way to refresh; authorization is Gemini Enterprise's job. logger.warning( "token for %s was rejected url=%s", connection.id, _loggable(url) diff --git a/src/gete/connection/github_app.py b/src/gete/connection/github_app.py new file mode 100644 index 0000000..0034733 --- /dev/null +++ b/src/gete/connection/github_app.py @@ -0,0 +1,266 @@ +"""Installation tokens gete issues itself, from a GitHub App's private key. + +A connection usually reads with the token Gemini Enterprise forwards for the +calling user. An app connection has no such token: the deployment holds the +App's key, signs in as the App, and asks GitHub for an installation token +narrowed to the repositories and permissions gete.yaml declares. Whoever can +call the agent acts as the App within that ceiling, so the ceiling is sent +with every issue rather than left to the installation's grant. + +The key and the App's JWT only travel to the connection's own hosts, and +nothing about either is logged. A token is reused until shortly before it +expires; GitHub issues them for an hour. +""" + +import asyncio +import base64 +import json +import logging +import os +import time +import urllib.parse +from collections.abc import Callable, Mapping +from datetime import datetime +from typing import Any + +import httpx +from cryptography.exceptions import UnsupportedAlgorithm +from cryptography.hazmat.primitives import hashes, serialization +from cryptography.hazmat.primitives.asymmetric import padding, rsa + +from gete.connection.registry import Connection +from gete.errors import UserFacingError + +logger = logging.getLogger(__name__) + +TIMEOUT_SECONDS = 30.0 +# GitHub refuses an App JWT valid for more than ten minutes, and one issued +# in the future by its own clock; backdating iat covers a deployment's clock +# running ahead, and the lifetime is counted from the backdated iat. +JWT_BACKDATE_SECONDS = 60 +JWT_LIFETIME_SECONDS = 9 * 60 +# A token handed out this close to its expiry could lapse in the middle of +# the request it was issued for. +REISSUE_BEFORE_SECONDS = 5 * 60 +ACCEPT = "application/vnd.github+json" + + +class AppTokenUnavailable(UserFacingError): + """No installation token could be issued. + + UserFacingError because every message is written here from the + connection's declaration and a status code: never a response body, never + anything read from the key. + """ + + +class InstallationTokens: + """Issues and reuses the installation tokens of one app connection.""" + + def __init__( + self, + connection: Connection, + *, + client: httpx.AsyncClient | None = None, + environ: Mapping[str, str] | None = None, + clock: Callable[[], float] = time.time, + ) -> None: + if connection.app is None: + raise ValueError(f"connection {connection.id} is not an app connection") + self._connection = connection + self._app = connection.app + self._client = client or httpx.AsyncClient(timeout=TIMEOUT_SECONDS) + self._environ = os.environ if environ is None else environ + self._clock = clock + self._lock = asyncio.Lock() + self._installation: int | None = None + self._token: str | None = None + self._expires_at = 0.0 + + async def token(self) -> str: + """A token the connection accepts, issued when the last one is near expiry.""" + async with self._lock: + if self._token is None or self._clock() >= ( + self._expires_at - REISSUE_BEFORE_SECONDS + ): + self._token, self._expires_at = await self._issue() + return self._token + + def forget(self) -> None: + """Drop the token held, so the next request is issued a new one. + + For a token the service refused before its time: holding on to it + would refuse every request until it expired. + """ + self._token = None + + async def _issue(self) -> tuple[str, float]: + headers = {"Authorization": f"Bearer {self._jwt()}", "Accept": ACCEPT} + if self._installation is None: + self._installation = await self._find_installation(headers) + response = await self._send( + "POST", + f"/app/installations/{self._installation}/access_tokens", + headers, + { + # The installation's own grant is the most a token could + # carry; asking for the declared ceiling every time is what + # keeps a token from carrying it. + "repositories": [ + repository.partition("/")[2] + for repository in self._app.repositories + ], + "permissions": dict(self._app.permissions), + }, + ) + if response.status_code != 201: + self._installation = None + raise self._unavailable( + f"GitHub refused to issue an installation token " + f"({response.status_code})" + ) + payload = _json_object(response) + token = payload.get("token") + if not isinstance(token, str) or not self._connection.accepts_token(token): + raise self._unavailable( + "GitHub answered with something that is not an installation token" + ) + try: + expires_at = datetime.fromisoformat(str(payload["expires_at"])) + except (KeyError, ValueError): + raise self._unavailable( + "GitHub issued a token without saying when it expires" + ) from None + logger.info("issued an installation token for %s", self._connection.id) + return token, expires_at.timestamp() + + async def _find_installation(self, headers: Mapping[str, str]) -> int: + """The installation on the first declared repository the App is on. + + validate holds the repositories to one owner, so whichever answers + is the one installation every one of them is reached through. + """ + for repository in self._app.repositories: + owner, _, name = repository.partition("/") + response = await self._send( + "GET", + f"/repos/{urllib.parse.quote(owner)}/{urllib.parse.quote(name)}" + "/installation", + headers, + ) + if response.status_code == 404: + continue + if response.status_code != 200: + raise self._unavailable( + f"GitHub refused to name the App's installation " + f"({response.status_code})" + ) + installation = _json_object(response).get("id") + if isinstance(installation, int): + return installation + raise self._unavailable( + f"the App is installed on none of {', '.join(self._app.repositories)}" + ) + + async def _send( + self, + method: str, + path: str, + headers: Mapping[str, str], + body: Any = None, + ) -> httpx.Response: + root = self._connection.base_url + url = f"{(root or '').rstrip('/')}{path}" + if root is None or not self._connection.allows(url): + # The JWT signs in as the App; it goes where the tokens go or + # nowhere. + raise self._unavailable("the connection names no root on its own hosts") + try: + return await self._client.request(method, url, headers=headers, json=body) + except httpx.HTTPError as error: + raise self._unavailable( + f"could not reach GitHub ({type(error).__name__})" + ) from None + + def _jwt(self) -> str: + """The App's own credential, signed with the key the deployment holds.""" + if not self._app.app_id: + raise self._unavailable("no App is declared for it") + pem = self._environ.get(self._connection.app_key_env) + if not pem: + raise self._unavailable( + f"the App's private key is not in {self._connection.app_key_env}" + ) + try: + key = serialization.load_pem_private_key(pem.encode(), password=None) + except (ValueError, TypeError, UnsupportedAlgorithm): + # Whatever the parser says about the key, the key is the secret; + # the message stays ours. + key = None + if not isinstance(key, rsa.RSAPrivateKey): + raise self._unavailable( + f"{self._connection.app_key_env} does not hold an RSA private key" + ) + now = int(self._clock()) + issued_at = now - JWT_BACKDATE_SECONDS + signing_input = ".".join( + _base64url(json.dumps(part, separators=(",", ":")).encode()) + for part in ( + {"alg": "RS256", "typ": "JWT"}, + { + "iat": issued_at, + "exp": issued_at + JWT_LIFETIME_SECONDS, + "iss": self._app.app_id, + }, + ) + ) + signature = key.sign( + signing_input.encode(), padding.PKCS1v15(), hashes.SHA256() + ) + return f"{signing_input}.{_base64url(signature)}" + + def _unavailable(self, reason: str) -> AppTokenUnavailable: + logger.warning("no installation token for %s: %s", self._connection.id, reason) + return AppTokenUnavailable( + f"{self._connection.display_name} is unavailable: {reason}. " + "Ask the operator to check the App." + ) + + +def _json_object(response: httpx.Response) -> Mapping[str, Any]: + """The answer's JSON object, or an empty one when it is anything else; + the callers then find none of the fields they need and say so.""" + try: + payload = response.json() + except ValueError: + return {} + return payload if isinstance(payload, Mapping) else {} + + +def _base64url(data: bytes) -> str: + return base64.urlsafe_b64encode(data).rstrip(b"=").decode() + + +_issuers: dict[tuple[Any, ...], InstallationTokens] = {} + + +def installation_tokens(connection: Connection) -> InstallationTokens: + """The process's issuer for this connection, shared by every tool call. + + Keyed by everything a token depends on, so a registry that declares the + connection differently never reuses a token issued for another ceiling. + """ + app = connection.app + if app is None: + raise ValueError(f"connection {connection.id} is not an app connection") + key = ( + connection.id, + connection.base_url, + app.app_id, + app.repositories, + tuple(sorted(app.permissions.items())), + ) + issuer = _issuers.get(key) + if issuer is None: + issuer = _issuers[key] = InstallationTokens(connection) + return issuer diff --git a/src/gete/connection/registry.py b/src/gete/connection/registry.py index 10c8a84..5489e0d 100644 --- a/src/gete/connection/registry.py +++ b/src/gete/connection/registry.py @@ -139,6 +139,39 @@ def from_mapping( ) +# Where the deployment finds an app connection's private key: the secret named +# in gete.yaml is delivered under this name plus the connection id, the way +# secret_env delivers any other. +APP_KEY_ENV_PREFIX = "GETE_APP_KEY_" + + +@dataclass(frozen=True) +class App: + """A GitHub App gete issues installation tokens for. + + Every field may be missing: a catalog entry cannot know which App an + installation registered, and validate refuses the gap where an agent + picks the connection up, the way it refuses an open base_url. + repositories and permissions are the ceiling of every token issued. + """ + + app_id: str | None = None + private_key_secret: str | None = None + repositories: tuple[str, ...] = () + permissions: Mapping[str, str] = field(default_factory=dict) + + @classmethod + def from_mapping(cls, data: Mapping[str, Any]) -> "App": + app_id = data.get("app_id") + secret = data.get("private_key_secret") + return cls( + app_id=None if app_id is None else str(app_id), + private_key_secret=None if secret is None else str(secret), + repositories=tuple(data.get("repositories", ())), + permissions=dict(data.get("permissions", {})), + ) + + def _stays_below(path: str, prefix: str) -> bool: """Whether the request path stays below the prefix however a server reads it. @@ -173,7 +206,10 @@ class Connection: id: str display_name: str - oauth: OAuth + # Exactly one of the two: a user's authorization forwarded by Gemini + # Enterprise, or an App gete issues tokens for itself. + oauth: OAuth | None = None + app: App | None = None hosts: frozenset[str] = frozenset() # Hosts a download may be redirected to, declared one by one; the token # never travels to them. Empty means downloads stay on hosts. @@ -223,7 +259,10 @@ def from_mapping(cls, data: Mapping[str, Any]) -> "Connection": return cls( id=data["id"], display_name=data["display_name"], - oauth=OAuth.from_mapping(data["oauth"], base_url), + oauth=( + OAuth.from_mapping(data["oauth"], base_url) if "oauth" in data else None + ), + app=App.from_mapping(data["app"]) if "app" in data else None, hosts=frozenset(hosts), redirect_hosts=frozenset(data.get("redirect_hosts", ())), token_prefixes=tuple(data.get("token_prefixes", ())), @@ -254,13 +293,19 @@ def needs_base_url(self) -> bool: """ return any( url is not None and BASE_URL in url - for url in ( - self.oauth.authorization_url, - self.oauth.token_url, - self.mcp_url, - ) + for url in (*self._oauth_urls(), self.mcp_url) ) + def _oauth_urls(self) -> tuple[str, ...]: + if self.oauth is None: + return () + return (self.oauth.authorization_url, self.oauth.token_url) + + @property + def app_key_env(self) -> str: + """The environment variable the App's private key is delivered in.""" + return APP_KEY_ENV_PREFIX + self.id.upper().replace("-", "_") + @property def secret_prefix(self) -> str: """Prefix of the Secret Manager secrets that hold the OAuth client.""" @@ -339,7 +384,7 @@ def issuer_hosts(self) -> frozenset[str]: own = {entry.partition("/")[0] for entry in self.hosts} own.update( host - for url in (self.oauth.authorization_url, self.oauth.token_url) + for url in self._oauth_urls() if (host := urlsplit(url).hostname) is not None ) return frozenset(own) @@ -415,6 +460,13 @@ def rejected_message(self) -> str: """ if self.rejected is not None: return self.rejected + if self.app is not None: + # Nobody approved anything, so there is no authorization to reset; + # what can be wrong is the App's installation or its grant. + return ( + f"{self.display_name} refused the token issued for it. Ask the " + "operator to check the App's installation and permissions." + ) return ( f"{self.display_name} refused the authorization. Approving again in " "Gemini Enterprise will not help; ask the operator to reset the " diff --git a/src/gete/connections_listing.py b/src/gete/connections_listing.py index fcc9f9b..67aaa24 100644 --- a/src/gete/connections_listing.py +++ b/src/gete/connections_listing.py @@ -54,9 +54,6 @@ def format_connection(connection: Connection) -> str: once. Some providers hand out no way to delete a client again, so a person guessing at any one of the three has to get it right the first time. """ - oauth = connection.oauth - scopes = [f"{scope}: {text}" for scope, text in oauth.scopes.items()] - optional = [f"{scope}: {text}" for scope, text in oauth.optional_scopes.items()] fields: list[tuple[str, list[str]]] = [ # The same word the listing uses; the reason gets a line of its own. ("status", ["retired" if connection.retired else "available"]), @@ -74,15 +71,7 @@ def format_connection(connection: Connection) -> str: ), ("token prefixes", [token_shapes(connection)]), ("mcp url", [connection.mcp_url or NONE_DECLARED]), - ("authorization", [oauth.authorization_url]), - ("token url", [oauth.token_url]), - ("scopes", scopes or [NONE_DECLARED]), - # The menu, because the OAuth client has to be prepared for every - # scope an agent may select, not only the defaults. - ("optional scopes", optional or [NONE_DECLARED]), - ("client id", [connection.client_id_secret]), - ("client secret", [connection.client_secret_secret]), - ("redirect uri", [REDIRECT_URI]), + *(_app_fields(connection) if connection.app else _oauth_fields(connection)), ] width = max(len(name) for name, _ in fields) lines = [f"{connection.id} {connection.display_name}"] @@ -102,6 +91,41 @@ def format_connection(connection: Connection) -> str: return "\n".join(lines) + "\n" +def _oauth_fields(connection: Connection) -> list[tuple[str, list[str]]]: + oauth = connection.oauth + if oauth is None: + return [] + scopes = [f"{scope}: {text}" for scope, text in oauth.scopes.items()] + optional = [f"{scope}: {text}" for scope, text in oauth.optional_scopes.items()] + return [ + ("authorization", [oauth.authorization_url]), + ("token url", [oauth.token_url]), + ("scopes", scopes or [NONE_DECLARED]), + # The menu, because the OAuth client has to be prepared for every + # scope an agent may select, not only the defaults. + ("optional scopes", optional or [NONE_DECLARED]), + ("client id", [connection.client_id_secret]), + ("client secret", [connection.client_secret_secret]), + ("redirect uri", [REDIRECT_URI]), + ] + + +def _app_fields(connection: Connection) -> list[tuple[str, list[str]]]: + """The App in place of an OAuth client: nobody authorizes the connection, + so what a person prepares is the App, its key, and the ceiling.""" + app = connection.app + if app is None: + return [] + permissions = [f"{name}: {level}" for name, level in app.permissions.items()] + return [ + ("app id", [app.app_id or NONE_DECLARED]), + ("private key", [app.private_key_secret or NONE_DECLARED]), + ("delivered as", [connection.app_key_env]), + ("repositories", list(app.repositories) or [NONE_DECLARED]), + ("permissions", permissions or [NONE_DECLARED]), + ] + + def format_table(rows: list[dict[str, Any]]) -> str: columns = ("id", "status", "source", "verified", "hosts") widths = {c: max(len(c), *(len(str(row[c])) for row in rows)) for c in columns} diff --git a/src/gete/declaration.py b/src/gete/declaration.py index f3e9d64..80b27fe 100644 --- a/src/gete/declaration.py +++ b/src/gete/declaration.py @@ -21,6 +21,7 @@ "Problem", "Project", "Resolved", + "app_key_secrets", "find_project_file", "load_project", "load_resolved", @@ -240,6 +241,23 @@ def resolve(project: Project, agent: Agent) -> dict[str, Any]: } +def app_key_secrets(project: Project, agent: Agent) -> dict[str, str]: + """The private key secrets of the agent's app connections, by their env names. + + Named once in gete.yaml and delivered the way secret_env delivers any + other: the agent holds the connection and gets the wiring, without + writing - or being able to change - where the key comes from. A key not + named yet is left out rather than invented; validate reports the gap. + """ + registry = Registry.from_catalog(project.connection_overrides) + wired: dict[str, str] = {} + for connection_id in agent.connections: + connection = registry.get(connection_id, include_retired=True) + if connection.app is not None and connection.app.private_key_secret: + wired[connection.app_key_env] = connection.app.private_key_secret + return wired + + @dataclass(frozen=True) class Resolved: """A resolved declaration as read back where it was deployed.""" diff --git a/src/gete/register.py b/src/gete/register.py index 1885a97..ae5d715 100644 --- a/src/gete/register.py +++ b/src/gete/register.py @@ -19,7 +19,7 @@ from urllib.parse import urlencode from gete.connection import Connection, Registry, authorization_id -from gete.connection.registry import missing_base_url +from gete.connection.registry import OAuth, missing_base_url from gete.declaration import Agent, Project from gete.errors import DeclarationError, GeteError from gete.gcp import GcpApi, GcpError @@ -47,6 +47,16 @@ def in_use_by_another(message: str) -> bool: return AUTHORIZATION_IN_USE in message +def _oauth(connection: Connection) -> OAuth: + """The connection's OAuth, which only a connection users authorize has.""" + if connection.oauth is None: + raise DeclarationError( + f"connection {connection.id} issues its own tokens; nobody " + "authorizes it in Gemini Enterprise" + ) + return connection.oauth + + def requested_scopes(connection: Connection, selected: Sequence[str]) -> list[str]: """The connection's default scopes plus the agent's selection, in that order. @@ -55,14 +65,15 @@ def requested_scopes(connection: Connection, selected: Sequence[str]) -> list[st a scope from outside it is refused here too rather than sent to the consent screen. """ - menu = connection.oauth.optional_scopes + oauth = _oauth(connection) + menu = oauth.optional_scopes unknown = [scope for scope in selected if scope not in menu] if unknown: raise DeclarationError( f"connection {connection.id}: {', '.join(unknown)} not in " "oauth.optional_scopes; an agent selects from the menu only" ) - defaults = list(connection.oauth.scopes) + defaults = list(oauth.scopes) return defaults + [scope for scope in selected if scope not in defaults] @@ -70,7 +81,7 @@ def authorization_uri( connection: Connection, client_id: str, selected_scopes: Sequence[str] = () ) -> str: """Where the user is sent to consent: the default scopes plus the selection.""" - oauth = connection.oauth + oauth = _oauth(connection) if oauth.authorization_query is not None: if selected_scopes: # The query is used as written; building the selection into it @@ -117,14 +128,15 @@ def authorization_body( # validate refuses this, but register can be run without it, and what # would be stored here is the link every user of the agent is sent to. raise DeclarationError(f"connection {missing_base_url(connection.id)}") + oauth = _oauth(connection) name = f"{parent}/authorizations/{authorization_id(agent_name, connection.id)}" oauth2: dict[str, Any] = { "clientId": client_id, "clientSecret": client_secret, "authorizationUri": authorization_uri(connection, client_id, selected_scopes), - "tokenUri": connection.oauth.token_url, + "tokenUri": oauth.token_url, } - if connection.oauth.pkce: + if oauth.pkce: # Left out when off rather than sent as false: an update replaces # serverSideOauth2 whole, so absent already means off, and every # connection that has never heard of PKCE stays as it reads. @@ -271,7 +283,7 @@ def _reset_owners( return {} declared: dict[str, Agent] = {} for agent in self._project.agents: - for connection_id in agent.connections: + for connection_id in self._authorized(agent): try: declared[authorization_id(agent.name, connection_id)] = agent except ValueError: @@ -311,6 +323,27 @@ def _reset_owners( ) return {identifier: declared[identifier].name for identifier in reset} + def _authorized(self, agent: Agent) -> list[str]: + """The agent's connections a user authorizes, in declaration order. + + An app connection is left out: gete issues its tokens from the App's + key, so there is no consent screen to send anyone to, no OAuth client + to read, and nothing a reset could bring back. + """ + authorized: list[str] = [] + for connection_id in agent.connections: + try: + connection = self._registry.get(connection_id, include_retired=True) + except GeteError: + # Kept, so the agent's own turn fails on the unknown id the + # way it always has, rather than a reset of another agent + # failing on it here. + authorized.append(connection_id) + continue + if connection.app is None: + authorized.append(connection_id) + return authorized + @property def _parent(self) -> str: if self._number is None: @@ -364,7 +397,7 @@ def _register( agent.scope_selections.get(connection_id, ()), summary, ) - for connection_id in agent.connections + for connection_id in self._authorized(agent) ] registered = find_by_reasoning_engine( self._gcp.list_all(self._agents_url(engine), "agents"), reasoning_engine diff --git a/src/gete/run.py b/src/gete/run.py index 5c11c6b..e73e66c 100644 --- a/src/gete/run.py +++ b/src/gete/run.py @@ -3,7 +3,7 @@ from collections.abc import Callable, Iterable, Mapping from typing import Any -from gete.connection import authorization_id +from gete.connection import Registry, authorization_id from gete.declaration import Agent, Project, resolve from gete.errors import DeclarationError from gete.runtime import build_document @@ -43,6 +43,20 @@ def missing_tokens( ] +def user_authorized(project: Project, agent: Agent) -> list[str]: + """The agent's connections read with a user's token, in declaration order. + + An app connection is issued its tokens from the key in its + GETE_APP_KEY_ variable; a GETE_TOKEN_ variable for it would never be read. + """ + registry = Registry.from_catalog(project.connection_overrides) + return [ + connection_id + for connection_id in agent.connections + if registry.get(connection_id, include_retired=True).app is None + ] + + def find_agent(project: Project, name: str) -> Agent: for agent in project.agents: if agent.name == name: diff --git a/src/gete/runtime/openapi.py b/src/gete/runtime/openapi.py index 6747fcf..5b11253 100644 --- a/src/gete/runtime/openapi.py +++ b/src/gete/runtime/openapi.py @@ -250,6 +250,10 @@ async def get_tools(self, readonly_context: Any = None) -> list[Any]: # No context at all counts as no token: the Agent Card is built that # way, and it must not promise what an unauthorized user cannot call. state = getattr(readonly_context, "state", None) + if self._connection.app is not None: + # No user's token is involved. The token is issued when a request + # is made, and a failure to issue one is told then, as text. + return list(self._tools) if usable_token(self._connection, self._key, state) is None: return [] return list(self._tools) diff --git a/src/gete/runtime/tools.py b/src/gete/runtime/tools.py index 0e7e088..f5d0c93 100644 --- a/src/gete/runtime/tools.py +++ b/src/gete/runtime/tools.py @@ -100,7 +100,10 @@ def build_tools( denied=denied, ) tools.append(openapi) - authorized.append(openapi.connection) + if openapi.connection.app is None: + # An app connection's token is issued, not approved; there is + # nothing to send the user back to. + authorized.append(openapi.connection) # Shared credential tools carry their effects with them; the write among # them is confirmed and denied like any declared write tool. for name in agent.shared_credentials: diff --git a/src/gete/schema/connection.json b/src/gete/schema/connection.json index 1a7234d..a193e90 100644 --- a/src/gete/schema/connection.json +++ b/src/gete/schema/connection.json @@ -2,13 +2,24 @@ "$schema": "https://json-schema.org/draft/2020-12/schema", "$id": "urn:gete:schema:connection", "title": "connection", - "description": "An external service: where user tokens may be sent, what they look like, and how users authorize.", + "description": "An external service: where tokens may be sent, what they look like, and where they come from - a user's authorization (oauth) or a GitHub App gete issues for (app).", "type": "object", "additionalProperties": false, "required": [ "id", - "display_name", - "oauth" + "display_name" + ], + "oneOf": [ + { + "required": [ + "oauth" + ] + }, + { + "required": [ + "app" + ] + } ], "properties": { "id": { @@ -122,6 +133,48 @@ } } }, + "app": { + "description": "A GitHub App gete issues installation tokens for, in place of a user's authorization. No Gemini Enterprise authorization is created; the private key is delivered to the deployment like secret_env, and every token is narrowed to repositories and permissions. A catalog entry leaves these open and the installation fills them in.", + "type": "object", + "additionalProperties": false, + "properties": { + "app_id": { + "description": "The App's numeric ID, from its settings page.", + "type": "string", + "pattern": "^[0-9]+$" + }, + "private_key_secret": { + "description": "Secret Manager secret holding the App's private key (PEM). The latest version is used.", + "type": "string", + "pattern": "^[A-Za-z0-9_-]+$" + }, + "repositories": { + "description": "owner/name of every repository a token may reach, all under one owner: a token is issued by one installation, and an installation belongs to one account.", + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "pattern": "^[A-Za-z0-9-]+/[A-Za-z0-9._-]+$" + } + }, + "permissions": { + "description": "Permission to access level, the ceiling of every token issued. Required rather than defaulted: left out, a token would carry everything the installation was granted.", + "type": "object", + "minProperties": 1, + "propertyNames": { + "pattern": "^[a-z_]+$" + }, + "additionalProperties": { + "enum": [ + "read", + "write", + "admin" + ] + } + } + } + }, "oauth_client": { "description": "Prefix of the Secret Manager secrets holding the OAuth client: -client-id and -client-secret.", "type": "string", diff --git a/src/gete/secrets.py b/src/gete/secrets.py index f70be36..e4672b4 100644 --- a/src/gete/secrets.py +++ b/src/gete/secrets.py @@ -6,7 +6,7 @@ """ from gete.connection import Registry -from gete.declaration import Problem, Project +from gete.declaration import Problem, Project, app_key_secrets from gete.gcp import GcpApi, GcpError SECRET_MANAGER = "https://secretmanager.googleapis.com/v1" @@ -25,9 +25,16 @@ def secrets_needed(project: Project) -> dict[str, list[str]]: secret = shared.get(name, {}).get("token_secret") if secret: names.append(str(secret)) + # Delivered like secret_env too, registered or not: the deployment + # issues its tokens with the key. + names.extend(app_key_secrets(project, agent).values()) if agent.data.get("registration"): for connection_id in agent.connections: connection = registry.get(connection_id) + if connection.oauth is None: + # Nobody authorizes an app connection; there is no + # OAuth client behind it. + continue names.extend( (connection.client_id_secret, connection.client_secret_secret) ) diff --git a/src/gete/terraform.py b/src/gete/terraform.py index 21fc77f..88e5f1e 100644 --- a/src/gete/terraform.py +++ b/src/gete/terraform.py @@ -12,7 +12,7 @@ from pathlib import Path, PurePosixPath from typing import Any -from gete.declaration import Agent, Project +from gete.declaration import Agent, Project, app_key_secrets from gete.shared_credentials import SHARED_CREDENTIALS # No version in the header: a development build's version changes with every @@ -150,7 +150,11 @@ def _runtime_settings( scalars: dict[str, str] = {} if runtime.get("env"): maps["env"] = _hcl_map(runtime["env"]) - secret_env = {**runtime.get("secret_env", {}), **_shared_secret_env(project, agent)} + secret_env = { + **runtime.get("secret_env", {}), + **_shared_secret_env(project, agent), + **app_key_secrets(project, agent), + } if secret_env: maps["secret_env"] = _hcl_map(secret_env) for key in ("min_instances", "max_instances"): diff --git a/src/gete/validate.py b/src/gete/validate.py index b8c89a0..e8f2ef2 100644 --- a/src/gete/validate.py +++ b/src/gete/validate.py @@ -7,7 +7,11 @@ from gete._yaml import read_yaml from gete.connection import Registry -from gete.connection.checks import connection_problems, elimination_problems +from gete.connection.checks import ( + app_problems, + connection_problems, + elimination_problems, +) from gete.connection.registry import missing_base_url from gete.declaration import Agent, Problem, Project from gete.errors import DeclarationError, GeteError @@ -117,7 +121,27 @@ def _agent_problems( found.append(f"connections: {error}") continue known.add(connection_id) - menu = connection.oauth.optional_scopes + if connection.app is not None: + found.extend( + f"connections: {message}" for message in app_problems(connection) + ) + if connection_id in agent.scope_selections: + found.append( + f"connections: {connection_id} issues its own tokens and " + "offers no scopes to select; what they may do is " + f"connections.{connection_id}.app.permissions in gete.yaml" + ) + for block, values in (("env", agent.env), ("secret_env", agent.secret_env)): + if connection.app_key_env in values: + # Delivered from gete.yaml; an agent pointing the variable + # elsewhere would issue tokens as an App of its choosing. + found.append( + f"runtime.agent_engine.{block}: {connection.app_key_env} " + f"is delivered from connections.{connection_id}.app." + "private_key_secret in gete.yaml; the agent does not set it" + ) + continue + menu = connection.oauth.optional_scopes if connection.oauth else {} outside = [ scope for scope in agent.scope_selections.get(connection_id, ()) @@ -320,6 +344,13 @@ def _mcp_problems( ] url: str = mcp["url"] connection = registry.get(connection_id) + if connection.app is not None: + # The MCP toolset attaches a token it reads synchronously, and an + # issued token is fetched by gete's own client when a request is made. + return [ + f"mcp: connection {connection_id!r} issues its own tokens, which " + "only openapi blocks and gete's client send" + ] if not connection.allows(url): hosts = ", ".join(sorted(connection.hosts)) return [ diff --git a/tests/test_check_secrets.py b/tests/test_check_secrets.py index f6aaf25..72cbe6e 100644 --- a/tests/test_check_secrets.py +++ b/tests/test_check_secrets.py @@ -142,3 +142,40 @@ def test_a_shared_credentials_token_secret_is_checked(project: ProjectBuilder) - loaded = load_project(project.root / "gete.yaml") reported = [str(problem) for problem in check_secrets(loaded, gcp)] assert any("slack-bot-token" in message for message in reported) + + +def test_an_app_connections_private_key_is_checked_instead_of_an_oauth_client( + project: ProjectBuilder, +) -> None: + """The key is delivered to the deployment whether or not the agent is + registered, and there is no OAuth client behind an App connection.""" + gcp = FakeGcp() + gcp.route("GET", versions("ge-github-app-private-key"), {}) + project.write_project( + { + "version": 1, + "project": "example-project", + "location": "us-central1", + "connections": { + "github-app": { + "app": { + "app_id": "123", + "private_key_secret": "ge-github-app-private-key", + "repositories": ["example-org/requests"], + "permissions": {"issues": "read"}, + } + } + }, + } + ) + project.write_agent( + "triage", + { + "connections": ["github-app"], + "registration": {"gemini_enterprise": {"engine": "app_1"}}, + }, + ) + loaded = load_project(project.root / "gete.yaml") + reported = [str(problem) for problem in check_secrets(loaded, gcp)] + assert len(reported) == 1, reported + assert "ge-github-app-private-key" in reported[0] diff --git a/tests/test_connection_client.py b/tests/test_connection_client.py index 68b0ea7..6d2283a 100644 --- a/tests/test_connection_client.py +++ b/tests/test_connection_client.py @@ -752,3 +752,86 @@ def handler(request: httpx.Request) -> httpx.Response: ) await reader.get_json(URL, headers={"accept": "text/csv"}) assert seen[0].headers.get_list("accept") == ["text/csv"] + + +APP_REGISTRY = Registry.from_catalog( + { + "github-app": { + "app": { + "app_id": "123", + "private_key_secret": "ge-github-app-private-key", + "repositories": ["example-org/requests"], + "permissions": {"issues": "read"}, + } + } + } +) +GITHUB_APP = APP_REGISTRY.get("github-app") +APP_TOKEN = "ghs_16C7e42F292c6912E7710c838347Ae178B4a" + + +class FakeTokens: + def __init__(self, error: Exception | None = None) -> None: + self.error = error + self.forgotten = 0 + + async def token(self) -> str: + if self.error is not None: + raise self.error + return APP_TOKEN + + def forget(self) -> None: + self.forgotten += 1 + + +@pytest.fixture +def tokens(monkeypatch: pytest.MonkeyPatch) -> FakeTokens: + fake = FakeTokens() + monkeypatch.setattr( + "gete.connection.client.installation_tokens", lambda connection: fake + ) + return fake + + +async def test_an_app_connection_sends_the_token_it_issued(tokens: FakeTokens) -> None: + """No user's token is involved: the state holds none, and none is asked for.""" + set_tool_call( + ToolCall(SimpleNamespace(state={}, user_id="u"), {}, registry=APP_REGISTRY) + ) + seen: list[httpx.Request] = [] + + def handler(request: httpx.Request) -> httpx.Response: + seen.append(request) + return httpx.Response(200, json={}) + + await client(handler, GITHUB_APP).get_json(URL) + assert seen[0].headers["Authorization"] == f"Bearer {APP_TOKEN}" + + +async def test_an_issue_that_failed_is_told_to_the_user_and_nothing_is_sent( + tokens: FakeTokens, +) -> None: + from gete.connection.github_app import AppTokenUnavailable + + tokens.error = AppTokenUnavailable("GitHub App is unavailable: no key.") + seen: list[httpx.Request] = [] + + def handler(request: httpx.Request) -> httpx.Response: + seen.append(request) + return httpx.Response(200, json={}) + + with pytest.raises(AuthorizationRefused, match="no key"): + await client(handler, GITHUB_APP).get_json(URL) + assert seen == [] + + +async def test_a_refused_app_token_is_dropped_and_the_operator_named( + tokens: FakeTokens, +) -> None: + """Nobody approved anything, so there is no authorization to reset; the + next request is issued a fresh token instead of the refused one.""" + with pytest.raises(AuthorizationRefused) as raised: + await client(lambda request: httpx.Response(401), GITHUB_APP).get_json(URL) + assert tokens.forgotten == 1 + assert "reset" not in str(raised.value) + assert "operator" in str(raised.value) diff --git a/tests/test_connection_registry.py b/tests/test_connection_registry.py index b64e2bf..9751b6a 100644 --- a/tests/test_connection_registry.py +++ b/tests/test_connection_registry.py @@ -751,3 +751,80 @@ def test_the_root_itself_may_not_contain_the_placeholder() -> None: Connection.from_mapping( {**ROOTED, "base_url": "https://acme.example.com/{base_url}"} ) + + +APP: dict[str, Any] = { + "id": "example-app", + "display_name": "Example App", + "hosts": ["api.example.com"], + "token_prefixes": ["ghs_"], + "app": { + "app_id": "123", + "private_key_secret": "example-app-private-key", + "repositories": ["example-org/requests"], + "permissions": {"issues": "read"}, + }, +} + + +def test_an_app_connection_carries_its_app_instead_of_oauth() -> None: + entry = Connection.from_mapping(APP) + assert entry.oauth is None + assert entry.app is not None + assert entry.app.app_id == "123" + assert entry.app.private_key_secret == "example-app-private-key" + assert entry.app.repositories == ("example-org/requests",) + assert entry.app.permissions == {"issues": "read"} + + +def test_an_oauth_connection_has_no_app() -> None: + assert connection().app is None + + +def test_the_private_key_arrives_in_a_variable_named_after_the_connection() -> None: + assert Connection.from_mapping(APP).app_key_env == "GETE_APP_KEY_EXAMPLE_APP" + + +def test_the_catalog_offers_a_github_app_connection(catalog: Registry) -> None: + entry = catalog.get("github-app") + assert entry.app is not None + assert entry.oauth is None + assert entry.allows("https://api.github.com/repos/o/r/issues") + assert entry.accepts_token("ghs_16C7e42F292c6912E7710c838347Ae178B4a") + assert not entry.accepts_token("gho_16C7e42F292c6912E7710c838347Ae178B4a") + + +def test_gete_yaml_fills_in_the_github_app_it_issues_for() -> None: + registry = Registry.from_catalog( + { + "github-app": { + "base_url": "https://ghe.example.com/api/v3", + "app": { + "app_id": "123", + "private_key_secret": "ge-github-app-private-key", + "repositories": ["example-org/requests"], + "permissions": {"issues": "read"}, + }, + } + } + ) + entry = registry.get("github-app") + assert entry.app is not None + assert entry.app.app_id == "123" + assert entry.allows("https://ghe.example.com/api/v3/repos/o/r") + # The installation's own server replaces the public one; nothing it issues + # is meant for api.github.com. + assert not entry.allows("https://api.github.com/repos/o/r") + + +def test_a_connection_needs_oauth_or_an_app() -> None: + bare = {key: value for key, value in EXAMPLE.items() if key != "oauth"} + with pytest.raises(DeclarationError): + Registry.from_catalog({"bare": bare}) + + +def test_a_connection_cannot_be_both_oauth_and_an_app() -> None: + both = {**APP, "oauth": EXAMPLE["oauth"]} + both.pop("id") + with pytest.raises(DeclarationError): + Registry.from_catalog({"example-app": both}) diff --git a/tests/test_connections_command.py b/tests/test_connections_command.py index b16dc33..182f21c 100644 --- a/tests/test_connections_command.py +++ b/tests/test_connections_command.py @@ -229,3 +229,34 @@ def test_a_declared_token_format_reads_as_the_format_it_declares() -> None: assert "jwt" in rows[0]["token_prefixes"] assert "elimination" not in rows[0]["token_prefixes"] assert "jwt" in format_connection(entry) + + +def test_an_app_connection_is_described_by_its_app_not_an_oauth_client() -> None: + """Nobody registers an OAuth client for it; what a person prepares is the + App, its key, and the limit every token is narrowed to.""" + registry = Registry.from_catalog( + { + "github-app": { + "app": { + "app_id": "123", + "private_key_secret": "ge-github-app-private-key", + "repositories": ["example-org/requests"], + "permissions": {"issues": "read"}, + } + } + } + ) + output = format_connection(registry.get("github-app")) + assert "123" in output + assert "ge-github-app-private-key" in output + assert "GETE_APP_KEY_GITHUB_APP" in output + assert "example-org/requests" in output + assert "issues: read" in output + assert "redirect uri" not in output + assert "client id" not in output + + +def test_the_catalogs_app_connection_says_what_is_still_open() -> None: + output = format_connection(Registry.from_catalog().get("github-app")) + assert "app id" in output + assert "(none)" in output diff --git a/tests/test_github_app.py b/tests/test_github_app.py new file mode 100644 index 0000000..716f954 --- /dev/null +++ b/tests/test_github_app.py @@ -0,0 +1,261 @@ +"""Installation tokens gete issues from a GitHub App's private key.""" + +import base64 +import json +from collections.abc import Callable +from datetime import UTC, datetime +from typing import Any + +import httpx +import pytest +from cryptography.hazmat.primitives import hashes, serialization +from cryptography.hazmat.primitives.asymmetric import padding, rsa + +from gete.connection import Connection, Registry +from gete.connection.github_app import AppTokenUnavailable, InstallationTokens +from gete.errors import UserFacingError + +PRIVATE_KEY = rsa.generate_private_key(public_exponent=65537, key_size=2048) +PEM = PRIVATE_KEY.private_bytes( + serialization.Encoding.PEM, + serialization.PrivateFormat.PKCS8, + serialization.NoEncryption(), +).decode() + +NOW = 1_800_000_000.0 +TOKEN = "ghs_16C7e42F292c6912E7710c838347Ae178B4a" +API = "https://api.github.com" + + +def app_connection(**override: Any) -> Connection: + registry = Registry.from_catalog( + { + "github-app": { + "app": { + "app_id": "123", + "private_key_secret": "ge-github-app-private-key", + "repositories": ["example-org/requests", "example-org/other"], + "permissions": {"issues": "read"}, + }, + **override, + } + } + ) + return registry.get("github-app") + + +def iso(seconds: float) -> str: + return datetime.fromtimestamp(seconds, UTC).strftime("%Y-%m-%dT%H:%M:%SZ") + + +class GitHub: + """The two endpoints an installation token takes, answering as told.""" + + def __init__(self, root: str = API) -> None: + self.root = root + self.requests: list[httpx.Request] = [] + self.installations: dict[str, httpx.Response] = { + "example-org/requests": httpx.Response(200, json={"id": 42}) + } + self.issued: list[httpx.Response] = [] + self.expires_at = iso(NOW + 3600) + + def __call__(self, request: httpx.Request) -> httpx.Response: + self.requests.append(request) + path = request.url.path.removeprefix(httpx.URL(self.root).path.rstrip("/")) + if path.startswith("/repos/") and path.endswith("/installation"): + repository = path.removeprefix("/repos/").removesuffix("/installation") + return self.installations.get(repository, httpx.Response(404)) + if path == "/app/installations/42/access_tokens": + if self.issued: + return self.issued.pop(0) + return httpx.Response( + 201, json={"token": TOKEN, "expires_at": self.expires_at} + ) + return httpx.Response(404) + + def token_requests(self) -> list[httpx.Request]: + return [r for r in self.requests if r.url.path.endswith("/access_tokens")] + + +def issuer( + github: GitHub, + *, + connection: Connection | None = None, + environ: dict[str, str] | None = None, + clock: Callable[[], float] = lambda: NOW, +) -> InstallationTokens: + return InstallationTokens( + connection or app_connection(), + client=httpx.AsyncClient(transport=httpx.MockTransport(github)), + environ={"GETE_APP_KEY_GITHUB_APP": PEM} if environ is None else environ, + clock=clock, + ) + + +def claims_of(jwt: str) -> tuple[dict[str, Any], dict[str, Any]]: + """Header and claims, after checking the signature against the App's key.""" + header, payload, signature = jwt.split(".") + + def decode(part: str) -> bytes: + return base64.urlsafe_b64decode(part + "=" * (-len(part) % 4)) + + PRIVATE_KEY.public_key().verify( + decode(signature), + f"{header}.{payload}".encode(), + padding.PKCS1v15(), + hashes.SHA256(), + ) + return json.loads(decode(header)), json.loads(decode(payload)) + + +async def test_the_app_signs_in_with_a_short_lived_rs256_jwt() -> None: + github = GitHub() + await issuer(github).token() + authorization = github.requests[0].headers["Authorization"] + assert authorization.startswith("Bearer ") + header, claims = claims_of(authorization.removeprefix("Bearer ")) + assert header["alg"] == "RS256" + assert claims["iss"] == "123" + # Backdated for a clock that runs ahead of GitHub's, and short of the ten + # minutes GitHub accepts at most. + assert claims["iat"] < NOW + assert claims["exp"] - claims["iat"] < 600 + + +async def test_the_installation_is_found_from_a_permitted_repository() -> None: + github = GitHub() + await issuer(github).token() + assert github.requests[0].method == "GET" + assert str(github.requests[0].url) == ( + f"{API}/repos/example-org/requests/installation" + ) + + +async def test_a_repository_the_app_is_not_installed_on_is_passed_over() -> None: + github = GitHub() + github.installations = { + "example-org/other": httpx.Response(200, json={"id": 42}), + } + assert await issuer(github).token() == TOKEN + + +async def test_the_token_is_narrowed_to_the_declared_ceiling() -> None: + github = GitHub() + assert await issuer(github).token() == TOKEN + [request] = github.token_requests() + assert request.method == "POST" + assert json.loads(request.content) == { + "repositories": ["requests", "other"], + "permissions": {"issues": "read"}, + } + # Signed in as the App, never with a token it issued. + assert request.headers["Authorization"] != f"Bearer {TOKEN}" + + +async def test_the_token_is_reused_until_shortly_before_it_expires() -> None: + github = GitHub() + now = [NOW] + tokens = issuer(github, clock=lambda: now[0]) + await tokens.token() + now[0] = NOW + 3600 - 600 + await tokens.token() + assert len(github.token_requests()) == 1 + now[0] = NOW + 3600 - 60 + await tokens.token() + assert len(github.token_requests()) == 2 + # The installation does not move; it is looked up once. + assert sum(r.url.path.endswith("/installation") for r in github.requests) == 1 + + +async def test_a_forgotten_token_is_issued_again() -> None: + github = GitHub() + tokens = issuer(github) + await tokens.token() + tokens.forget() + await tokens.token() + assert len(github.token_requests()) == 2 + + +async def test_github_enterprise_is_reached_below_its_own_root() -> None: + root = "https://ghe.example.com/api/v3" + github = GitHub(root) + connection = app_connection(base_url=root) + await issuer(github, connection=connection).token() + assert str(github.requests[0].url).startswith(f"{root}/repos/") + assert str(github.token_requests()[0].url) == ( + f"{root}/app/installations/42/access_tokens" + ) + + +async def test_without_the_key_nothing_is_sent_and_the_reason_is_text() -> None: + github = GitHub() + with pytest.raises(AppTokenUnavailable) as raised: + await issuer(github, environ={}).token() + assert isinstance(raised.value, UserFacingError) + assert "GETE_APP_KEY_GITHUB_APP" in str(raised.value) + assert github.requests == [] + + +async def test_a_key_that_is_not_a_private_key_is_reported_without_its_content() -> ( + None +): + github = GitHub() + with pytest.raises(AppTokenUnavailable) as raised: + await issuer(github, environ={"GETE_APP_KEY_GITHUB_APP": "not-a-key"}).token() + assert "not-a-key" not in str(raised.value) + assert github.requests == [] + + +async def test_an_app_installed_on_none_of_the_repositories_is_reported() -> None: + github = GitHub() + github.installations = {} + with pytest.raises(AppTokenUnavailable, match="example-org/requests"): + await issuer(github).token() + + +@pytest.mark.parametrize("status", [401, 403, 422]) +async def test_a_refused_issue_is_reported_with_the_status_only(status: int) -> None: + """The body may name what the App was denied; the status is diagnosis enough.""" + github = GitHub() + github.issued = [httpx.Response(status, json={"message": "secret detail"})] + with pytest.raises(AppTokenUnavailable) as raised: + await issuer(github).token() + assert str(status) in str(raised.value) + assert "secret detail" not in str(raised.value) + + +async def test_a_token_of_another_shape_is_refused() -> None: + """Whatever came back is sent on as this connection's token; it has to + look like one.""" + github = GitHub() + github.issued = [ + httpx.Response(201, json={"token": "gho_x", "expires_at": iso(NOW + 3600)}) + ] + with pytest.raises(AppTokenUnavailable): + await issuer(github).token() + + +async def test_an_unreachable_github_is_reported_as_text() -> None: + def refuse(request: httpx.Request) -> httpx.Response: + raise httpx.ConnectError("down") + + tokens = InstallationTokens( + app_connection(), + client=httpx.AsyncClient(transport=httpx.MockTransport(refuse)), + environ={"GETE_APP_KEY_GITHUB_APP": PEM}, + clock=lambda: NOW, + ) + with pytest.raises(AppTokenUnavailable): + await tokens.token() + + +async def test_an_answer_that_is_not_json_is_reported_as_text() -> None: + github = GitHub() + github.issued = [httpx.Response(201, content=b"")] + with pytest.raises(AppTokenUnavailable): + await issuer(github).token() + github = GitHub() + github.installations = {"example-org/requests": httpx.Response(200, content=b"{")} + with pytest.raises(AppTokenUnavailable): + await issuer(github).token() diff --git a/tests/test_register.py b/tests/test_register.py index e7ff664..87145a5 100644 --- a/tests/test_register.py +++ b/tests/test_register.py @@ -1122,3 +1122,61 @@ def _with_pkce(enabled: bool) -> Connection: return Connection.from_mapping( {"id": "example", "display_name": "Example", "oauth": oauth} ) + + +def project_with_github_app(project: ProjectBuilder, agent: dict[str, Any]) -> Any: + project.write_project( + { + "version": 1, + "project": "example-project", + "location": "us-central1", + "gemini_enterprise": {"project_number": NUMBER}, + "connections": { + "github-app": { + "app": { + "app_id": "123", + "private_key_secret": "ge-github-app-private-key", + "repositories": ["example-org/requests"], + "permissions": {"issues": "read"}, + } + } + }, + } + ) + project.write_agent(agent["name"], agent) + return load_project(project.root / "gete.yaml") + + +def test_an_app_connection_gets_no_authorization( + project: ProjectBuilder, gcp: FakeGcp, tmp_path: Path +) -> None: + """Nobody approves an App connection: gete issues its tokens, so there is + no consent screen to send anyone to and no OAuth client to read.""" + loaded = project_with_github_app( + project, {**FINANCE, "connections": ["freee", "github-app"]} + ) + notice = tmp_path / "n.md" + summary = register_project(loaded, gcp, notice) + assert summary.failed == [] + posts = gcp.writes("POST") + assert [post[1] for post in posts] == [{"authorizationId": "finance-freee"}] + assert not any("github-app" in url for _, url, _, _ in gcp.calls) + assert "finance-github-app" not in notice.read_text() + + +def test_an_app_connection_has_no_authorization_to_reset( + project: ProjectBuilder, gcp: FakeGcp, tmp_path: Path +) -> None: + loaded = project_with_github_app( + project, {**FINANCE, "connections": ["freee", "github-app"]} + ) + with pytest.raises(DeclarationError, match="finance-github-app"): + register_project(loaded, gcp, tmp_path / "n.md", reset=["finance-github-app"]) + assert gcp.writes("DELETE") == [] + + +def test_an_app_connection_has_no_authorization_body() -> None: + """register may run without validate; nothing may reach Gemini Enterprise + claiming users approve a connection nobody authorizes.""" + with pytest.raises(DeclarationError, match="issues its own tokens"): + authorization_body(GE, "finance", CATALOG.get("github-app"), "c", "s") diff --git a/tests/test_run_command.py b/tests/test_run_command.py index 7113c42..000adf2 100644 --- a/tests/test_run_command.py +++ b/tests/test_run_command.py @@ -82,3 +82,32 @@ def test_the_connections_without_a_token_are_named(project: ProjectBuilder) -> N "finance", ["freee", "google"], {"GETE_TOKEN_FREEE": "a1b2c3"} ) assert missing_tokens("finance", ["freee", "google"], state) == ["google"] + + +def test_an_app_connection_is_not_asked_for_a_users_token( + project: ProjectBuilder, +) -> None: + """Its token is issued from the key in GETE_APP_KEY_; a + GETE_TOKEN_ variable for it would never be read.""" + from gete.run import user_authorized + + project.write_project( + { + "version": 1, + "project": "example-project", + "location": "us-central1", + "connections": { + "github-app": { + "app": { + "app_id": "123", + "private_key_secret": "ge-github-app-private-key", + "repositories": ["example-org/requests"], + "permissions": {"issues": "read"}, + } + } + }, + } + ) + project.write_agent("triage", {"connections": ["freee", "github-app"]}) + loaded = load_project(project.root / "gete.yaml") + assert user_authorized(loaded, loaded.agents[0]) == ["freee"] diff --git a/tests/test_runtime_openapi.py b/tests/test_runtime_openapi.py index 4200e4e..610301c 100644 --- a/tests/test_runtime_openapi.py +++ b/tests/test_runtime_openapi.py @@ -778,3 +778,64 @@ async def test_the_pruned_description_builds_the_same_tools(tmp_path: Path) -> N ours._get_declaration().model_dump_json() == theirs._get_declaration().model_dump_json() ) + + +GITHUB_APP_OVERRIDE: dict[str, Any] = { + "app": { + "app_id": "123", + "private_key_secret": "ge-github-app-private-key", + "repositories": ["example-org/requests"], + "permissions": {"issues": "read"}, + } +} + + +def build_app_agent(project: ProjectBuilder) -> Any: + project.write_project( + { + "version": 1, + "project": "example-project", + "location": "us-central1", + "connections": {"github-app": GITHUB_APP_OVERRIDE}, + } + ) + directory = project.write_agent( + "mail-triage", + { + "connections": ["github-app"], + "tools": [ + { + "openapi": { + "spec": "./spec.yaml", + "connection": "github-app", + "operations": ["ListSearchResults"], + "effect": "read", + } + } + ], + }, + ) + (directory / "spec.yaml").write_text(yaml.safe_dump(SPEC, sort_keys=False)) + loaded = load_project(project.root / "gete.yaml") + path = directory / RESOLVED_FILE + path.write_text(yaml.safe_dump(resolve(loaded, loaded.agents[0]), sort_keys=False)) + return build(path) + + +async def test_an_app_connections_tools_are_offered_without_a_users_token( + project: ProjectBuilder, +) -> None: + """The token is issued when a request is made; whether it can be is told + then, as text, not by hiding the tools.""" + [built] = build_app_agent(project).tools + assert isinstance(built, OpenApiToolset) + tools = await built.get_tools(Context({})) + assert [tool.name for tool in tools] == ["ListSearchResults"] + + +def test_an_app_connection_is_offered_no_reauthorization_tool( + project: ProjectBuilder, +) -> None: + """There is nothing for a user to approve.""" + tools = build_app_agent(project).tools + assert not any(isinstance(tool, ReauthorizationToolset) for tool in tools) diff --git a/tests/test_terraform.py b/tests/test_terraform.py index 9467f4e..2fffd45 100644 --- a/tests/test_terraform.py +++ b/tests/test_terraform.py @@ -213,3 +213,38 @@ def test_without_the_project_entry_nothing_is_injected( """validate reports it; the generated call must not invent a secret name.""" project.write_agent("poster", {"shared_credentials": ["slack_post"]}) assert "SLACK_BOT_TOKEN" not in files(project)["poster.tf"] + + +APP_PROJECT: dict[str, Any] = { + "version": 1, + "project": "example-project", + "location": "us-central1", + "connections": { + "github-app": { + "app": { + "app_id": "123", + "private_key_secret": "ge-github-app-private-key", + "repositories": ["example-org/requests"], + "permissions": {"issues": "read"}, + } + } + }, +} + + +def test_an_app_connections_private_key_is_wired_into_secret_env( + project: ProjectBuilder, +) -> None: + """Named once in gete.yaml, delivered to every agent holding the connection.""" + project.write_project(APP_PROJECT) + project.write_agent("triage", {"connections": ["github-app"]}) + text = files(project)["triage.tf"] + assert 'GETE_APP_KEY_GITHUB_APP = "ge-github-app-private-key"' in text + + +def test_an_agent_without_the_app_connection_gets_no_key( + project: ProjectBuilder, +) -> None: + project.write_project(APP_PROJECT) + project.write_agent("triage") + assert "GETE_APP_KEY" not in files(project)["triage.tf"] diff --git a/tests/test_validate.py b/tests/test_validate.py index 903e9a0..07dfc73 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -896,3 +896,135 @@ def test_openapi_operations_are_held_against_the_description( ) found = problems(project) assert any("Nope" in p for p in found), found + + +GITHUB_APP = { + "app_id": "123", + "private_key_secret": "ge-github-app-private-key", + "repositories": ["example-org/requests"], + "permissions": {"issues": "read"}, +} + + +def write_github_app(project: ProjectBuilder, **app: Any) -> None: + project.write_project( + { + "version": 1, + "project": "example-project", + "location": "us-central1", + "connections": {"github-app": {"app": {**GITHUB_APP, **app}}}, + } + ) + + +def test_a_configured_github_app_passes(project: ProjectBuilder) -> None: + write_github_app(project) + project.write_agent("triage", {"connections": ["github-app"]}) + assert problems(project) == [] + + +@pytest.mark.parametrize( + "missing", ["app_id", "private_key_secret", "repositories", "permissions"] +) +def test_an_app_the_installation_did_not_fill_in_is_refused( + project: ProjectBuilder, missing: str +) -> None: + """The catalog cannot know which App; without one there is nothing to + issue with, and without the limits a token would carry everything.""" + project.write_project( + { + "version": 1, + "project": "example-project", + "location": "us-central1", + "connections": { + "github-app": { + "app": {k: v for k, v in GITHUB_APP.items() if k != missing} + } + }, + } + ) + project.write_agent("triage", {"connections": ["github-app"]}) + found = problems(project) + assert any(f"app.{missing}" in p and "gete.yaml" in p for p in found), found + + +def test_app_repositories_under_more_than_one_owner_are_refused( + project: ProjectBuilder, +) -> None: + """A token comes from one installation, and an installation belongs to one + account; the repositories of a second owner could never be reached.""" + write_github_app(project, repositories=["example-org/a", "other-org/b"]) + project.write_agent("triage", {"connections": ["github-app"]}) + found = problems(project) + assert any("example-org" in p and "other-org" in p for p in found), found + + +def test_an_app_connection_selects_no_scopes(project: ProjectBuilder) -> None: + write_github_app(project) + project.write_agent( + "triage", {"connections": [{"id": "github-app", "scopes": ["repo"]}]} + ) + found = problems(project) + assert any("github-app" in p and "scopes" in p for p in found), found + + +def test_an_app_connection_does_not_back_an_mcp_block( + project: ProjectBuilder, +) -> None: + """Its tokens are issued per request by gete's own client, which the MCP + toolset does not go through.""" + write_github_app(project) + project.write_agent( + "triage", + { + "connections": ["github-app"], + "tools": [ + { + "mcp": { + "url": "https://api.github.com/mcp", + "connection": "github-app", + "effect": "read", + } + } + ], + }, + ) + found = problems(project) + assert any("mcp" in p and "github-app" in p for p in found), found + + +@pytest.mark.parametrize("block", ["env", "secret_env"]) +def test_the_agent_cannot_claim_the_app_key_variable_itself( + project: ProjectBuilder, block: str +) -> None: + """The key is delivered from gete.yaml; an agent setting the variable + would issue tokens as an App of its own choosing.""" + write_github_app(project) + project.write_agent( + "triage", + { + "connections": ["github-app"], + "runtime": { + "agent_engine": {block: {"GETE_APP_KEY_GITHUB_APP": "elsewhere"}} + }, + }, + ) + found = problems(project) + assert any("GETE_APP_KEY_GITHUB_APP" in p for p in found), found + + +def test_an_openapi_block_reads_through_an_app_connection( + project: ProjectBuilder, +) -> None: + write_github_app(project) + write_openapi_agent( + project, + { + "spec": "./spec.yaml", + "connection": "github-app", + "operations": ["ListThings"], + "effect": "read", + }, + connections=["github-app"], + ) + assert problems(project) == [] diff --git a/uv.lock b/uv.lock index ee4dd99..ec874b5 100644 --- a/uv.lock +++ b/uv.lock @@ -666,6 +666,7 @@ wheels = [ name = "gete" source = { editable = "." } dependencies = [ + { name = "cryptography" }, { name = "google-adk", extra = ["mcp"] }, { name = "httpx" }, { name = "jsonschema" }, @@ -691,6 +692,7 @@ dev = [ [package.metadata] requires-dist = [ { name = "click", marker = "extra == 'cli'", specifier = ">=8.1" }, + { name = "cryptography", specifier = ">=43" }, { name = "google-adk", extras = ["mcp"], specifier = ">=2.6,<2.9" }, { name = "google-auth", marker = "extra == 'cli'", specifier = ">=2.38" }, { name = "httpx", specifier = ">=0.28" }, From ddbd9f5ec7a62c4e06ceb29d48e75ec000b988cb Mon Sep 17 00:00:00 2001 From: haruotsu <65439874+haruotsu@users.noreply.github.com> Date: Wed, 30 Sep 2026 19:00:00 +0900 Subject: [PATCH 2/2] Keep an App's key out of the environment and mark app connections as bots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An app connection acts as the App for whoever calls the agent, the same trust model as a shared credential, but gete graph drew it like a connection carrying the caller's authorization. It is now marked (bot). The private key could issue a token with the installation's whole grant, above the ceiling gete.yaml declares. Left in GETE_APP_KEY_*, any tool reading its settings, and any process it starts, would find it. The agent build now takes it out of the environment before the agent's own modules are imported. This is not a sandbox against code in the same process, which the README now says. A bare app_id in YAML is read as a number, and the schema refused it although the loader already turned it into a string; both forms are accepted now. 🤖 Generated with Claude Code --- README.md | 16 +++++++- src/gete/connection/github_app.py | 39 ++++++++++++++++-- src/gete/graph.py | 35 +++++++++++++--- src/gete/runtime/__init__.py | 7 ++++ src/gete/schema/connection.json | 10 +++-- tests/test_connection_registry.py | 7 ++++ tests/test_github_app.py | 54 ++++++++++++++++++++++++- tests/test_graph.py | 66 +++++++++++++++++++++++++++++++ tests/test_runtime_openapi.py | 14 ++++++- tests/test_validate.py | 7 ++++ 10 files changed, 239 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 13953ff..c31bd62 100644 --- a/README.md +++ b/README.md @@ -437,7 +437,7 @@ connections: github-app: base_url: https://ghe.example.com/api/v3 # leave out for github.com app: - app_id: "123" + app_id: 123 private_key_secret: ge-github-app-private-key # The ceiling of every token issued through this connection repositories: [example-org/requests] @@ -471,7 +471,11 @@ For such a connection gete: - delivers the key like `secret_env`: `private_key_secret` reaches the deployment as `GETE_APP_KEY_GITHUB_APP`, which the agent cannot set itself. The App ID and the ceiling travel in the resolved declaration. - `gete run` reads the PEM from the same variable. + `gete run` reads the PEM from the same variable. When the agent is built, + before its own modules are imported, gete takes the variable out of the + environment and keeps the key to itself; +- draws the connection in `gete graph` marked `(bot)`, like a shared + credential. `repositories` must share one owner, since a token comes from one installation. `permissions` is required: left out, a token would carry @@ -480,6 +484,14 @@ the App within that ceiling, whatever they could reach on GitHub themselves, so keep it to what the agent's tools read. `mcp` blocks cannot use an app connection yet. +The ceiling binds the tokens gete issues, not the key. Python tools run in +the same process as gete, and code that goes looking for the key there can +find it and issue a token with the installation's whole grant. Taking it out +of the environment keeps it away from tools reading their settings and from +processes they start; it is not a sandbox. Grant the App itself no more than +the agents holding the connection may do, and review the python tools of +those agents as code that holds the key. + ### Shared credentials A connection reads with the caller's token. Some writes have no such token diff --git a/src/gete/connection/github_app.py b/src/gete/connection/github_app.py index 0034733..b115a8e 100644 --- a/src/gete/connection/github_app.py +++ b/src/gete/connection/github_app.py @@ -19,7 +19,7 @@ import os import time import urllib.parse -from collections.abc import Callable, Mapping +from collections.abc import Callable, Iterable, Mapping, MutableMapping from datetime import datetime from typing import Any @@ -70,7 +70,7 @@ def __init__( self._connection = connection self._app = connection.app self._client = client or httpx.AsyncClient(timeout=TIMEOUT_SECONDS) - self._environ = os.environ if environ is None else environ + self._environ = environ self._clock = clock self._lock = asyncio.Lock() self._installation: int | None = None @@ -186,7 +186,7 @@ def _jwt(self) -> str: """The App's own credential, signed with the key the deployment holds.""" if not self._app.app_id: raise self._unavailable("no App is declared for it") - pem = self._environ.get(self._connection.app_key_env) + pem = _app_key(self._connection.app_key_env, self._environ) if not pem: raise self._unavailable( f"the App's private key is not in {self._connection.app_key_env}" @@ -227,6 +227,39 @@ def _unavailable(self, reason: str) -> AppTokenUnavailable: ) +# Keys taken out of the environment by hold_app_keys, by variable name. +_held_keys: dict[str, str] = {} + + +def hold_app_keys( + connections: Iterable[Connection], + environ: MutableMapping[str, str] = os.environ, +) -> None: + """Take the app connections' private keys out of the environment. + + Called before an agent's own modules are imported. The key could issue + a token with the installation's whole grant; left in the environment, + any tool reading its settings, and any process it starts, would find it + there. This does not isolate the key from code running in the same + process, which can still reach this module; it keeps the key off the + path every other setting is read from. + """ + for connection in connections: + if connection.app is None: + continue + pem = environ.pop(connection.app_key_env, None) + if pem: + _held_keys[connection.app_key_env] = pem + + +def _app_key(name: str, environ: Mapping[str, str] | None) -> str | None: + if environ is not None: + return environ.get(name) + # Not held when nothing built an agent first, as when gete's client is + # used on its own; the environment is then the only place it can be. + return _held_keys.get(name) or os.environ.get(name) + + def _json_object(response: httpx.Response) -> Mapping[str, Any]: """The answer's JSON object, or an empty one when it is anything else; the callers then find none of the fields they need and say so.""" diff --git a/src/gete/graph.py b/src/gete/graph.py index c9fc315..62fe366 100644 --- a/src/gete/graph.py +++ b/src/gete/graph.py @@ -7,6 +7,10 @@ from gete.connection import Registry from gete.declaration import Agent, Project +from gete.errors import GeteError + +# Marks what acts with a credential the agent holds rather than the caller's. +BOT = " (bot)" def label(text: str) -> str: @@ -58,7 +62,9 @@ def mermaid(project: Project, names: list[str] | None = None) -> str: connection = tool["mcp"].get("connection") if connection: connected.add(connection) - lines.append(f" {node} -. {connection} .-> {tool_node}") + lines.append( + f" {node} -. {_via(registry, connection)} .-> {tool_node}" + ) elif "openapi" in tool: count = len(tool["openapi"]["operations"]) noun = "operation" if count == 1 else "operations" @@ -67,24 +73,43 @@ def mermaid(project: Project, names: list[str] | None = None) -> str: ) connection = tool["openapi"]["connection"] connected.add(connection) - lines.append(f" {node} -. {connection} .-> {tool_node}") + lines.append( + f" {node} -. {_via(registry, connection)} .-> {tool_node}" + ) for name in agent.shared_credentials: # Marked as the bot it is: the diagram must not read as if these # tools acted with the caller's authorization. lines.append( - f' {node} --> {node}_shared_{_ident(name)}["{label(name)} (bot)"]' + f' {node} --> {node}_shared_{_ident(name)}["{label(name)}{BOT}"]' ) for connection_id in agent.connections: if connection_id in connected: continue - display = registry.get(connection_id, include_retired=True).display_name + connection = registry.get(connection_id, include_retired=True) + display = connection.display_name + if connection.app is not None: + display += BOT lines.append( - f" {node} -. {connection_id} .-> " + f" {node} -. {_via(registry, connection_id)} .-> " f'conn_{_ident(connection_id)}[("{label(display)}")]' ) return "\n".join(lines) + "\n" +def _via(registry: Registry, connection_id: str) -> str: + """The edge label: the connection, marked when it acts as an App. + + An app connection reaches GitHub with the App's token whoever calls, so + the diagram must not read as if it carried the caller's authorization. + """ + try: + connection = registry.get(connection_id, include_retired=True) + except GeteError: + # validate reports the unknown id; the diagram still draws the rest. + return connection_id + return connection_id + (BOT if connection.app is not None else "") + + def _engine(agent: Agent) -> str | None: registration: Mapping[str, Any] = agent.data.get("registration", {}) engine = registration.get("gemini_enterprise", {}).get("engine") diff --git a/src/gete/runtime/__init__.py b/src/gete/runtime/__init__.py index 7a98d1d..b340019 100644 --- a/src/gete/runtime/__init__.py +++ b/src/gete/runtime/__init__.py @@ -5,6 +5,7 @@ from pathlib import Path from typing import Any +from gete.connection.github_app import hold_app_keys from gete.connection.runtime import authorization_id from gete.declaration import Agent, Resolved, load_resolved, resolved_from_document from gete.policies import applicable, compose_instruction @@ -33,6 +34,12 @@ def _build(resolved: Resolved) -> Any: from google.adk.agents import LlmAgent agent = resolved.agent + # Before build_tools imports the agent's own modules, so none of them + # finds an App's key in the environment. + hold_app_keys( + resolved.registry.get(connection_id, include_retired=True) + for connection_id in agent.connections + ) policies = applicable(resolved.policies, resolved.data) rules = RedactRules.from_policies(policies) authorizations = { diff --git a/src/gete/schema/connection.json b/src/gete/schema/connection.json index a193e90..463846b 100644 --- a/src/gete/schema/connection.json +++ b/src/gete/schema/connection.json @@ -139,9 +139,13 @@ "additionalProperties": false, "properties": { "app_id": { - "description": "The App's numeric ID, from its settings page.", - "type": "string", - "pattern": "^[0-9]+$" + "description": "The App's numeric ID, from its settings page. Quoted or not; YAML reads a bare one as a number.", + "type": [ + "string", + "integer" + ], + "pattern": "^[0-9]+$", + "minimum": 1 }, "private_key_secret": { "description": "Secret Manager secret holding the App's private key (PEM). The latest version is used.", diff --git a/tests/test_connection_registry.py b/tests/test_connection_registry.py index 9751b6a..8c7f1bc 100644 --- a/tests/test_connection_registry.py +++ b/tests/test_connection_registry.py @@ -828,3 +828,10 @@ def test_a_connection_cannot_be_both_oauth_and_an_app() -> None: both.pop("id") with pytest.raises(DeclarationError): Registry.from_catalog({"example-app": both}) + + +def test_an_app_id_given_as_a_number_is_read_as_its_digits() -> None: + registry = Registry.from_catalog({"github-app": {"app": {"app_id": 123}}}) + app = registry.get("github-app").app + assert app is not None + assert app.app_id == "123" diff --git a/tests/test_github_app.py b/tests/test_github_app.py index 716f954..208a725 100644 --- a/tests/test_github_app.py +++ b/tests/test_github_app.py @@ -11,8 +11,12 @@ from cryptography.hazmat.primitives import hashes, serialization from cryptography.hazmat.primitives.asymmetric import padding, rsa -from gete.connection import Connection, Registry -from gete.connection.github_app import AppTokenUnavailable, InstallationTokens +from gete.connection import Connection, Registry, github_app +from gete.connection.github_app import ( + AppTokenUnavailable, + InstallationTokens, + hold_app_keys, +) from gete.errors import UserFacingError PRIVATE_KEY = rsa.generate_private_key(public_exponent=65537, key_size=2048) @@ -259,3 +263,49 @@ async def test_an_answer_that_is_not_json_is_reported_as_text() -> None: github.installations = {"example-org/requests": httpx.Response(200, content=b"{")} with pytest.raises(AppTokenUnavailable): await issuer(github).token() + + +@pytest.fixture +def held(monkeypatch: pytest.MonkeyPatch) -> dict[str, str]: + keys: dict[str, str] = {} + monkeypatch.setattr(github_app, "_held_keys", keys) + return keys + + +def test_holding_the_keys_takes_them_out_of_the_environment( + held: dict[str, str], +) -> None: + """What the agent's own code or a process it starts reads from the + environment no longer includes the App's key.""" + environ = {"GETE_APP_KEY_GITHUB_APP": PEM, "OTHER": "kept"} + hold_app_keys([app_connection()], environ) + assert environ == {"OTHER": "kept"} + + +def test_a_connection_that_is_not_an_app_keeps_its_variable( + held: dict[str, str], +) -> None: + environ = {"GETE_APP_KEY_GITHUB": "not ours to take"} + github = Registry.from_catalog().get("github") + hold_app_keys([github], environ) + assert environ == {"GETE_APP_KEY_GITHUB": "not ours to take"} + + +async def test_a_held_key_still_signs(held: dict[str, str]) -> None: + hold_app_keys([app_connection()], {"GETE_APP_KEY_GITHUB_APP": PEM}) + github = GitHub() + tokens = InstallationTokens( + app_connection(), + client=httpx.AsyncClient(transport=httpx.MockTransport(github)), + environ=None, + clock=lambda: NOW, + ) + assert await tokens.token() == TOKEN + + +def test_holding_twice_keeps_the_key_already_held(held: dict[str, str]) -> None: + """A second build in the same process finds the variable gone; the key it + took the first time is still the one used.""" + hold_app_keys([app_connection()], {"GETE_APP_KEY_GITHUB_APP": PEM}) + hold_app_keys([app_connection()], {}) + assert held == {"GETE_APP_KEY_GITHUB_APP": PEM} diff --git a/tests/test_graph.py b/tests/test_graph.py index acefd69..e23f86c 100644 --- a/tests/test_graph.py +++ b/tests/test_graph.py @@ -133,3 +133,69 @@ def test_openapi_tools_appear_with_their_connection(project: ProjectBuilder) -> text = graph(project) assert 'desk --> desk_tool_0[("openapi
2 operations")]' in text assert "desk -. freee .-> desk_tool_0" in text + + +GITHUB_APP_CONNECTION: dict[str, Any] = { + "github-app": { + "app": { + "app_id": "123", + "private_key_secret": "ge-github-app-private-key", + "repositories": ["example-org/requests"], + "permissions": {"issues": "read"}, + } + } +} + + +def test_an_app_connection_is_drawn_as_the_bot_it_acts_as( + project: ProjectBuilder, +) -> None: + project.write_project( + { + "version": 1, + "project": "example-project", + "location": "us-central1", + "connections": GITHUB_APP_CONNECTION, + } + ) + project.write_agent( + "triage", + { + "connections": ["github-app"], + "tools": [ + { + "openapi": { + "spec": "./specs/github.yaml", + "connection": "github-app", + "operations": ["GetIssue"], + "effect": "read", + } + } + ], + }, + ) + text = graph(project) + assert "triage -. github-app (bot) .-> triage_tool_0" in text + + +def test_an_app_connection_no_tool_uses_is_still_drawn_as_a_bot( + project: ProjectBuilder, +) -> None: + project.write_project( + { + "version": 1, + "project": "example-project", + "location": "us-central1", + "connections": GITHUB_APP_CONNECTION, + } + ) + project.write_agent("triage", {"connections": ["github-app"]}) + text = graph(project) + assert '[("GitHub App (bot)")]' in text + + +def test_a_user_authorized_connection_is_not_drawn_as_a_bot( + project: ProjectBuilder, +) -> None: + project.write_agent("finance", {"connections": ["freee"]}) + assert "bot" not in graph(project) diff --git a/tests/test_runtime_openapi.py b/tests/test_runtime_openapi.py index 610301c..643806b 100644 --- a/tests/test_runtime_openapi.py +++ b/tests/test_runtime_openapi.py @@ -1,6 +1,7 @@ """OpenAPI tools: operations become tools, and requests go through gete's client.""" import copy +import os from pathlib import Path from typing import Any @@ -8,7 +9,7 @@ import yaml from conftest import ProjectBuilder -from gete.connection import Registry +from gete.connection import Registry, github_app from gete.declaration import RESOLVED_FILE, Agent, load_project, resolve from gete.errors import DeclarationError, GeteError from gete.openapi import pruned_description @@ -839,3 +840,14 @@ def test_an_app_connection_is_offered_no_reauthorization_tool( """There is nothing for a user to approve.""" tools = build_app_agent(project).tools assert not any(isinstance(tool, ReauthorizationToolset) for tool in tools) + + +def test_building_an_app_agent_takes_the_key_out_of_the_environment( + project: ProjectBuilder, monkeypatch: pytest.MonkeyPatch +) -> None: + """Taken before the agent's own modules are imported, so none of them + finds the key where every other setting is.""" + monkeypatch.setattr(github_app, "_held_keys", {}) + monkeypatch.setenv("GETE_APP_KEY_GITHUB_APP", "pem") + build_app_agent(project) + assert "GETE_APP_KEY_GITHUB_APP" not in os.environ diff --git a/tests/test_validate.py b/tests/test_validate.py index 07dfc73..7c636c1 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -948,6 +948,13 @@ def test_an_app_the_installation_did_not_fill_in_is_refused( assert any(f"app.{missing}" in p and "gete.yaml" in p for p in found), found +def test_an_app_id_written_as_a_bare_number_passes(project: ProjectBuilder) -> None: + """YAML reads an unquoted App ID as a number; it names the same App.""" + write_github_app(project, app_id=123) + project.write_agent("triage", {"connections": ["github-app"]}) + assert problems(project) == [] + + def test_app_repositories_under_more_than_one_owner_are_refused( project: ProjectBuilder, ) -> None: