diff --git a/README.md b/README.md index 1c4d892..c31bd62 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,76 @@ 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. 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 +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. + +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/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..b115a8e --- /dev/null +++ b/src/gete/connection/github_app.py @@ -0,0 +1,299 @@ +"""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, Iterable, Mapping, MutableMapping +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 = 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 = _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}" + ) + 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." + ) + + +# 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.""" + 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/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/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/__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/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..463846b 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,52 @@ } } }, + "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. 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.", + "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..8c7f1bc 100644 --- a/tests/test_connection_registry.py +++ b/tests/test_connection_registry.py @@ -751,3 +751,87 @@ 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}) + + +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_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..208a725 --- /dev/null +++ b/tests/test_github_app.py @@ -0,0 +1,311 @@ +"""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, 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) +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() + + +@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_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..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 @@ -778,3 +779,75 @@ 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) + + +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_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..7c636c1 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -896,3 +896,142 @@ 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_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: + """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" },