From 85529bc6dd1947533682acf27a898af6ca1cb6ff Mon Sep 17 00:00:00 2001 From: haruotsu <65439874+haruotsu@users.noreply.github.com> Date: Thu, 27 Aug 2026 21:51:03 +0900 Subject: [PATCH 1/7] Let an agent select scopes from a connection's menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Authorizations are created per agent, but their scopes were composed from the connection definition alone: every agent holding a connection asked for everything the definition listed. Widening a catalog entry would have raised every holder's token to the new maximum at their next consent, so the catalog could only ever carry the least common denominator. A connection now offers a menu next to its defaults. oauth.scopes stays what every agent gets; oauth.optional_scopes is what an agent may add by selecting it in its own declaration, and the selection is built into that agent's authorization URI alone, so one agent's opt-in grants nothing to any other. validate refuses a selection from outside the menu, and register refuses it again because it can be run without validate. A menu next to a verbatim authorization_query is refused at the definition: the query is used as written, so no selection could ever reach the consent screen, and dropping it silently would grant less than the agent declared. 🤖 Generated with Claude Code --- src/gete/connection/checks.py | 12 ++++ src/gete/connection/registry.py | 7 ++- src/gete/connections_listing.py | 4 ++ src/gete/declaration.py | 20 ++++++- src/gete/register.py | 61 ++++++++++++++++++--- src/gete/schema/agent.json | 32 ++++++++++- src/gete/schema/connection.json | 10 +++- src/gete/validate.py | 17 ++++++ tests/test_connection_registry.py | 36 ++++++++++++ tests/test_connections_command.py | 25 ++++++++- tests/test_declaration.py | 30 ++++++++++ tests/test_register.py | 91 +++++++++++++++++++++++++++++++ tests/test_schema.py | 53 ++++++++++++++++++ tests/test_validate.py | 70 ++++++++++++++++++++++++ 14 files changed, 453 insertions(+), 15 deletions(-) create mode 100644 tests/test_declaration.py diff --git a/src/gete/connection/checks.py b/src/gete/connection/checks.py index 4c9dea7..38a026d 100644 --- a/src/gete/connection/checks.py +++ b/src/gete/connection/checks.py @@ -85,6 +85,18 @@ def connection_problems(connection: Connection, registry: Registry) -> list[str] problems.append( f"token_prefixes: {prefix!r} overlaps {theirs!r} ({other.id})" ) + 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" + ) for token in connection.examples.accepts: if not connection.accepts_token(token): problems.append(f"examples.accepts: {token!r} is not accepted") diff --git a/src/gete/connection/registry.py b/src/gete/connection/registry.py index 0a8dc54..6e57a15 100644 --- a/src/gete/connection/registry.py +++ b/src/gete/connection/registry.py @@ -66,7 +66,10 @@ def looks_like_jwt(token: str) -> bool: class OAuth: """How users authorize the connection. - scopes maps a scope to the explanation shown on the consent screen. + scopes maps a scope to the explanation shown on the consent screen; every + agent that declares the connection gets them. optional_scopes is the menu + an agent may select from on top of that, in the same shape; none of them + reach a token unless the agent declares them. scope_parameter is the query parameter that carries the user scopes; Slack uses user_scope because scope means the app's own permissions there. authorization_query, when set, is used verbatim as the authorization URL's @@ -80,6 +83,7 @@ class OAuth: authorization_url: str token_url: str scopes: Mapping[str, str] + optional_scopes: Mapping[str, str] = field(default_factory=dict) scope_parameter: str = "scope" authorization_query: Mapping[str, str] | None = None pkce: bool = False @@ -92,6 +96,7 @@ def from_mapping( authorization_url=str(_rooted(data["authorization_url"], base_url)), token_url=str(_rooted(data["token_url"], base_url)), scopes=dict(data["scopes"]), + optional_scopes=dict(data.get("optional_scopes", {})), scope_parameter=data.get("scope_parameter", "scope"), pkce=bool(data.get("pkce", False)), authorization_query=( diff --git a/src/gete/connections_listing.py b/src/gete/connections_listing.py index 7821e80..2142e47 100644 --- a/src/gete/connections_listing.py +++ b/src/gete/connections_listing.py @@ -43,6 +43,7 @@ def format_connection(connection: Connection) -> str: """ 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"]), @@ -63,6 +64,9 @@ def format_connection(connection: Connection) -> str: ("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]), diff --git a/src/gete/declaration.py b/src/gete/declaration.py index a54fe37..f3e9d64 100644 --- a/src/gete/declaration.py +++ b/src/gete/declaration.py @@ -69,7 +69,25 @@ def name(self) -> str: @property def connections(self) -> tuple[str, ...]: - return tuple(self.data.get("connections", ())) + """Connection ids, whichever way each entry is written. + + An entry is the id itself, or a mapping that also selects scopes from + the connection's menu; everything that only needs to know which + connections the agent holds reads the ids from here. + """ + return tuple( + entry if isinstance(entry, str) else str(entry["id"]) + for entry in self.data.get("connections", ()) + ) + + @property + def scope_selections(self) -> dict[str, tuple[str, ...]]: + """Selected optional scopes by id; ids that select nothing are absent.""" + return { + str(entry["id"]): tuple(entry["scopes"]) + for entry in self.data.get("connections", ()) + if not isinstance(entry, str) + } @property def tools(self) -> tuple[Mapping[str, Any], ...]: diff --git a/src/gete/register.py b/src/gete/register.py index 764c331..7d3eff0 100644 --- a/src/gete/register.py +++ b/src/gete/register.py @@ -12,7 +12,7 @@ import base64 import json -from collections.abc import Mapping +from collections.abc import Mapping, Sequence from dataclasses import dataclass, field from pathlib import Path from typing import Any @@ -47,10 +47,39 @@ def in_use_by_another(message: str) -> bool: return AUTHORIZATION_IN_USE in message -def authorization_uri(connection: Connection, client_id: str) -> str: - """Where the user is sent to consent, with the scopes the connection declares.""" +def requested_scopes(connection: Connection, selected: Sequence[str]) -> list[str]: + """The connection's default scopes plus the agent's selection, in that order. + + The selection must come from the connection's optional_scopes menu. The + menu is the reviewed ceiling, and register can be run without validate, so + a scope from outside it is refused here too rather than sent to the + consent screen. + """ + menu = connection.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) + return defaults + [scope for scope in selected if scope not in defaults] + + +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 if oauth.authorization_query is not None: + if selected_scopes: + # The query is used as written; building the selection into it + # would second-guess the form known to work, and leaving it out + # would grant less than the agent declared without a word. + raise DeclarationError( + f"connection {connection.id} fixes its authorization_query; " + "a scope selection cannot reach the consent screen" + ) # The form known to work for this service; Gemini Enterprise adds # client_id and redirect_uri itself. params: dict[str, str] = dict(oauth.authorization_query) @@ -59,7 +88,9 @@ def authorization_uri(connection: Connection, client_id: str) -> str: "client_id": client_id, "redirect_uri": REDIRECT_URI, "response_type": "code", - oauth.scope_parameter: " ".join(oauth.scopes), + oauth.scope_parameter: " ".join( + requested_scopes(connection, selected_scopes) + ), # Without offline access there is no refresh token and reading # stops an hour after consent. "access_type": "offline", @@ -74,8 +105,14 @@ def authorization_body( connection: Connection, client_id: str, client_secret: str, + selected_scopes: Sequence[str] = (), ) -> dict[str, Any]: - """The Authorization resource, named per agent so two agents never share one.""" + """The Authorization resource, named per agent so two agents never share one. + + The scope selection rides in the authorization URI, so the selection is + per agent too: another agent on the same connection consents to its own + scopes and no more. + """ if connection.needs_base_url: # 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. @@ -84,7 +121,7 @@ def authorization_body( oauth2: dict[str, Any] = { "clientId": client_id, "clientSecret": client_secret, - "authorizationUri": authorization_uri(connection, client_id), + "authorizationUri": authorization_uri(connection, client_id, selected_scopes), "tokenUri": connection.oauth.token_url, } if connection.oauth.pkce: @@ -236,7 +273,10 @@ def _register(self, agent: Agent, engine: str, summary: Summary) -> None: ) authorizations = [ self._upsert_authorization( - agent, self._registry.get(connection_id), summary + agent, + self._registry.get(connection_id), + agent.scope_selections.get(connection_id, ()), + summary, ) for connection_id in agent.connections ] @@ -299,7 +339,11 @@ def _register(self, agent: Agent, engine: str, summary: Summary) -> None: summary.registered.append(agent.name) def _upsert_authorization( - self, agent: Agent, connection: Connection, summary: Summary + self, + agent: Agent, + connection: Connection, + selected_scopes: Sequence[str], + summary: Summary, ) -> str: body = authorization_body( self._parent, @@ -307,6 +351,7 @@ def _upsert_authorization( connection, self._secret(connection.client_id_secret), self._secret(connection.client_secret_secret), + selected_scopes, ) resource = str(body["name"]) identifier = resource.rsplit("/", 1)[-1] diff --git a/src/gete/schema/agent.json b/src/gete/schema/agent.json index c83fc88..36b987f 100644 --- a/src/gete/schema/agent.json +++ b/src/gete/schema/agent.json @@ -47,8 +47,36 @@ "type": "array", "uniqueItems": true, "items": { - "type": "string", - "pattern": "^[a-z][a-z0-9-]*$" + "oneOf": [ + { + "type": "string", + "pattern": "^[a-z][a-z0-9-]*$" + }, + { + "description": "A connection with scopes selected from its optional_scopes menu, on top of the defaults every agent gets. An entry that selects nothing is written as the plain string instead.", + "type": "object", + "additionalProperties": false, + "required": [ + "id", + "scopes" + ], + "properties": { + "id": { + "type": "string", + "pattern": "^[a-z][a-z0-9-]*$" + }, + "scopes": { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "minLength": 1 + } + } + } + } + ] } }, "shared_credentials": { diff --git a/src/gete/schema/connection.json b/src/gete/schema/connection.json index a8f64e6..738f840 100644 --- a/src/gete/schema/connection.json +++ b/src/gete/schema/connection.json @@ -67,7 +67,15 @@ "$ref": "#/$defs/rooted_url" }, "scopes": { - "description": "Scope to the explanation shown on the consent screen.", + "description": "Scope to the explanation shown on the consent screen. Every agent that declares the connection gets these.", + "type": "object", + "additionalProperties": { + "type": "string", + "minLength": 1 + } + }, + "optional_scopes": { + "description": "Scope to the explanation shown on the consent screen. The menu an agent may select from; none of these reach a token unless the agent declares them.", "type": "object", "additionalProperties": { "type": "string", diff --git a/src/gete/validate.py b/src/gete/validate.py index 85b4ba5..11413c0 100644 --- a/src/gete/validate.py +++ b/src/gete/validate.py @@ -100,12 +100,29 @@ def _agent_problems( ) known: set[str] = set() for connection_id in agent.connections: + if connection_id in known: + # uniqueItems cannot see that a string entry and a mapping entry + # name the same connection. + found.append(f"connections: {connection_id} is declared twice") + continue try: connection = registry.get(connection_id) except GeteError as error: found.append(f"connections: {error}") continue known.add(connection_id) + menu = connection.oauth.optional_scopes + outside = [ + scope + for scope in agent.scope_selections.get(connection_id, ()) + if scope not in menu + ] + if outside: + offered = ", ".join(sorted(menu)) or "nothing" + found.append( + f"connections: {connection_id} does not offer " + f"{', '.join(outside)}; oauth.optional_scopes offers {offered}" + ) if connection.needs_base_url: # The definition left the root open because it moves with the # installation. Nothing it declares is an address until then: not diff --git a/tests/test_connection_registry.py b/tests/test_connection_registry.py index 6a629e8..8892347 100644 --- a/tests/test_connection_registry.py +++ b/tests/test_connection_registry.py @@ -96,6 +96,42 @@ def test_oauth_client_defaults_to_ge_oauth_id() -> None: ) +def test_optional_scopes_are_a_menu_next_to_the_defaults() -> None: + entry = connection( + oauth={**EXAMPLE["oauth"], "optional_scopes": {"write": "Change data"}} + ) + assert entry.oauth.scopes == {"read": "Read data"} + assert entry.oauth.optional_scopes == {"write": "Change data"} + + +def test_a_connection_without_a_menu_offers_nothing() -> None: + assert connection().oauth.optional_scopes == {} + + +def test_an_optional_scope_repeated_in_the_defaults_is_reported() -> None: + entry = connection( + oauth={**EXAMPLE["oauth"], "optional_scopes": {"read": "Read data"}} + ) + assert any( + "read" in problem for problem in connection_problems(entry, Registry([entry])) + ) + + +def test_a_menu_next_to_a_verbatim_authorization_query_is_reported() -> None: + """The query is used verbatim; no selection could ever reach the consent screen.""" + entry = connection( + oauth={ + **EXAMPLE["oauth"], + "authorization_query": {"response_type": "code"}, + "optional_scopes": {"write": "Change data"}, + } + ) + assert any( + "authorization_query" in problem + for problem in connection_problems(entry, Registry([entry])) + ) + + def test_catalog_entry_can_be_overridden_partially() -> None: registry = Registry.from_catalog( {"github": {"base_url": "https://api.github.example.com"}} diff --git a/tests/test_connections_command.py b/tests/test_connections_command.py index 86e455d..c0e66e7 100644 --- a/tests/test_connections_command.py +++ b/tests/test_connections_command.py @@ -6,8 +6,8 @@ from conftest import ProjectBuilder from gete.cli import main -from gete.connection import Registry -from gete.connections_listing import connections_table +from gete.connection import Connection, Registry +from gete.connections_listing import connections_table, format_connection def test_table_lists_every_connection_with_hosts_and_verification() -> None: @@ -40,6 +40,27 @@ def test_table_includes_private_connections_and_marks_their_source() -> None: assert rows["freee"]["source"] == "catalog" +def test_the_description_shows_the_menu_next_to_the_default_scopes() -> None: + """Preparing the OAuth client takes everything an agent may select, too.""" + entry = Connection.from_mapping( + { + "id": "example", + "display_name": "Example", + "hosts": ["api.example.com"], + "oauth": { + "authorization_url": "https://auth.example.com/authorize", + "token_url": "https://auth.example.com/token", + "scopes": {"read": "Read data"}, + "optional_scopes": {"write": "Change data"}, + }, + } + ) + output = format_connection(entry) + assert "read: Read data" in output + assert "write: Change data" in output + assert "optional scopes" in output + + def test_cli_prints_one_line_per_connection(project: ProjectBuilder) -> None: runner = CliRunner() with runner.isolated_filesystem(temp_dir=project.root): diff --git a/tests/test_declaration.py b/tests/test_declaration.py new file mode 100644 index 0000000..da457e1 --- /dev/null +++ b/tests/test_declaration.py @@ -0,0 +1,30 @@ +"""Typed access to agent.yaml: both spellings of a connection entry.""" + +from pathlib import Path + +from gete.declaration import Agent + +WRITE_SHEETS = "https://www.googleapis.com/auth/spreadsheets" + + +def agent_with(connections: list[object]) -> Agent: + return Agent( + directory=Path("agents/mail-triage"), + data={"name": "mail-triage", "connections": connections}, + ) + + +def test_connections_return_the_id_whichever_form_the_entry_uses() -> None: + agent = agent_with(["freee", {"id": "google", "scopes": [WRITE_SHEETS]}]) + assert agent.connections == ("freee", "google") + + +def test_scope_selections_come_only_from_entries_that_select() -> None: + agent = agent_with(["freee", {"id": "google", "scopes": [WRITE_SHEETS]}]) + assert agent.scope_selections == {"google": (WRITE_SHEETS,)} + + +def test_an_agent_without_connections_selects_nothing() -> None: + agent = Agent(directory=Path("agents/mail-triage"), data={"name": "mail-triage"}) + assert agent.connections == () + assert agent.scope_selections == {} diff --git a/tests/test_register.py b/tests/test_register.py index 92eed53..b3bfdc2 100644 --- a/tests/test_register.py +++ b/tests/test_register.py @@ -88,6 +88,62 @@ def test_authorization_uri_copies_a_verbatim_query_when_declared() -> None: ) +MENU: dict[str, Any] = { + "id": "example", + "display_name": "Example", + "hosts": ["api.example.com"], + "token_prefixes": ["ex_"], + "oauth": { + "authorization_url": "https://auth.example.com/authorize", + "token_url": "https://auth.example.com/token", + "scopes": {"read": "Read data"}, + "optional_scopes": {"write": "Change data", "admin": "Administer data"}, + }, +} + + +def test_authorization_uri_appends_the_selection_after_the_defaults() -> None: + entry = Connection.from_mapping(MENU) + params = query(authorization_uri(entry, "c", ["write"])) + assert params["scope"] == ["read write"] + + +def test_without_a_selection_the_defaults_stand_alone() -> None: + entry = Connection.from_mapping(MENU) + params = query(authorization_uri(entry, "c")) + assert params["scope"] == ["read"] + + +def test_a_selection_outside_the_menu_is_refused() -> None: + """register may run without validate; the menu is the reviewed ceiling.""" + entry = Connection.from_mapping(MENU) + with pytest.raises(DeclarationError, match="optional_scopes"): + authorization_uri(entry, "c", ["repo"]) + + +def test_a_selection_next_to_a_verbatim_query_is_refused() -> None: + """The query is used as written, so the selection would silently go missing.""" + entry = Connection.from_mapping( + { + **MENU, + "oauth": { + **MENU["oauth"], + "authorization_query": {"response_type": "code"}, + }, + } + ) + with pytest.raises(DeclarationError, match="authorization_query"): + authorization_uri(entry, "c", ["write"]) + + +def test_authorization_body_carries_the_selection_into_the_uri() -> None: + body = authorization_body( + GE, "finance-agent", Connection.from_mapping(MENU), "id-1", "s", ["write"] + ) + params = query(body["serverSideOauth2"]["authorizationUri"]) + assert params["scope"] == ["read write"] + + def test_authorization_body_is_named_per_agent_and_carries_the_client() -> None: body = authorization_body( GE, "finance-agent", CATALOG.get("freee"), "id-1", "secret-1" @@ -274,6 +330,41 @@ def test_authorization_is_created_when_absent_and_a_notice_is_written( assert "
" in text +def test_the_registrar_builds_each_authorization_from_that_agents_selection( + project: ProjectBuilder, gcp: FakeGcp, tmp_path: Path +) -> None: + project.write_project( + { + "version": 1, + "project": "example-project", + "location": "us-central1", + "gemini_enterprise": {"project_number": NUMBER}, + "connections": {"example": {k: v for k, v in MENU.items() if k != "id"}}, + } + ) + project.write_agent( + "finance", + { + **FINANCE, + "connections": [{"id": "example", "scopes": ["write"]}], + }, + ) + for suffix, value in (("client-id", "id-1"), ("client-secret", "secret-1")): + gcp.route( + "GET", + f"{SECRETS}/ge-oauth-example-{suffix}/versions/latest:access", + secret(value), + ) + summary = register_project( + load_project(project.root / "gete.yaml"), gcp, tmp_path / "n.md" + ) + assert summary.failed == [] + posts = gcp.writes("POST") + assert posts[0][1] == {"authorizationId": "finance-example"} + params = query(posts[0][2]["serverSideOauth2"]["authorizationUri"]) + assert params["scope"] == ["read write"] + + def test_existing_authorization_is_updated_not_recreated( project: ProjectBuilder, gcp: FakeGcp, tmp_path: Path ) -> None: diff --git a/tests/test_schema.py b/tests/test_schema.py index e4b2b20..6128b02 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -157,6 +157,59 @@ def test_connections_must_be_unique() -> None: ) +def test_a_connection_entry_may_select_scopes() -> None: + validate_document( + "agent", + { + **AGENT, + "connections": [ + "freee", + { + "id": "google", + "scopes": ["https://www.googleapis.com/auth/spreadsheets"], + }, + ], + }, + source="agent.yaml", + ) + + +@pytest.mark.parametrize( + "entry", + [ + {"scopes": ["https://www.googleapis.com/auth/spreadsheets"]}, + {"id": "google", "scope": ["https://www.googleapis.com/auth/spreadsheets"]}, + {"id": "google", "scopes": []}, + {"id": "google", "scopes": ["a", "a"]}, + {"id": "Google", "scopes": ["a"]}, + ], +) +def test_a_connection_entry_needs_an_id_and_a_real_selection( + entry: dict[str, Any], +) -> None: + """An entry that selects nothing is written as the plain string instead.""" + with pytest.raises(DeclarationError, match="connections"): + validate_document( + "agent", {**AGENT, "connections": [entry]}, source="agent.yaml" + ) + + +def test_optional_scopes_map_a_scope_to_its_explanation() -> None: + oauth = {**CONNECTION["oauth"], "optional_scopes": {"write": "Change data"}} + validate_document( + "connection", {**CONNECTION, "oauth": oauth}, source="connection.yaml" + ) + + +def test_an_optional_scope_needs_a_non_empty_explanation() -> None: + """The explanation is what the consent screen shows; a blank one hides the grant.""" + oauth = {**CONNECTION["oauth"], "optional_scopes": {"write": ""}} + with pytest.raises(DeclarationError, match="optional_scopes"): + validate_document( + "connection", {**CONNECTION, "oauth": oauth}, source="connection.yaml" + ) + + @pytest.mark.parametrize( "tool", [ diff --git a/tests/test_validate.py b/tests/test_validate.py index 472f93e..fe726b5 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -76,6 +76,76 @@ def test_duplicate_connection_is_reported(project: ProjectBuilder) -> None: assert any("connections" in p for p in problems(project)) +# INTERNAL_API with a menu an agent may select write access from. +MENU_API: dict[str, Any] = { + **INTERNAL_API, + "oauth": { + **INTERNAL_API["oauth"], + "optional_scopes": {"write": "Change internal data"}, + }, +} + + +def write_menu_api(project: ProjectBuilder) -> None: + project.write_project( + { + "version": 1, + "project": "example-project", + "location": "us-central1", + "connections": {"internal-api": MENU_API}, + } + ) + + +def test_a_selection_from_the_menu_passes(project: ProjectBuilder) -> None: + write_menu_api(project) + project.write_agent( + "mail-triage", + {"connections": [{"id": "internal-api", "scopes": ["write"]}]}, + ) + assert problems(project) == [] + + +def test_a_scope_outside_the_menu_is_reported_with_the_menu( + project: ProjectBuilder, +) -> None: + write_menu_api(project) + project.write_agent( + "mail-triage", + {"connections": [{"id": "internal-api", "scopes": ["admin"]}]}, + ) + found = problems(project) + assert any("admin" in p and "write" in p for p in found) + + +def test_a_selection_on_a_connection_without_a_menu_is_reported( + project: ProjectBuilder, +) -> None: + write_internal_api(project) + project.write_agent( + "mail-triage", + {"connections": [{"id": "internal-api", "scopes": ["write"]}]}, + ) + assert any("optional_scopes" in p for p in problems(project)) + + +def test_the_same_connection_in_both_forms_is_reported( + project: ProjectBuilder, +) -> None: + """uniqueItems cannot see that the string and the mapping name one connection.""" + write_menu_api(project) + project.write_agent( + "mail-triage", + { + "connections": [ + "internal-api", + {"id": "internal-api", "scopes": ["write"]}, + ] + }, + ) + assert any("twice" in p for p in problems(project)) + + def test_authorization_id_longer_than_63_is_reported(project: ProjectBuilder) -> None: """- becomes an authorization id, which is a DNS-style label.""" name = "a" * 58 From 82630d1d28c7e7abb089c8a50839165eb9f13028 Mon Sep 17 00:00:00 2001 From: haruotsu <65439874+haruotsu@users.noreply.github.com> Date: Thu, 27 Aug 2026 21:51:59 +0900 Subject: [PATCH 2/7] Offer the Workspace read and write scopes on google's menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The google connection listed sheets, drive, and people among its hosts while its scopes could read none of them: a token could travel to the Sheets API and only ever be refused there. With scopes now selected per agent, the catalog can carry the rest of Workspace without touching what a bare `connections: [google]` means - the defaults stay the read-only gmail and calendar pair, and everything wider is on the menu, reaching only the agents that declare it. The menu offers reads for Sheets, Drive, Docs, and Slides, and writes for Sheets, Docs, Slides, and calendar events. Drive write is drive.file only - the files the agent creates or the user opens with it - because Drive-wide write would put every file the user can touch behind one consent. gmail.send is the heaviest entry: mail sent as the user is outward and cannot be recalled, which is what the old read-only comment feared. It is offered because it stays opt-in and its consent text says plainly that the agent sends as the user; gmail.modify and mail.google.com stay off the menu because rewriting or deleting the inbox is not needed to send. docs.googleapis.com and slides.googleapis.com join the hosts as the endpoints those scopes are used against, keeping one host per API so a Workspace authorization still cannot reach GCP. 🤖 Generated with Claude Code --- README.md | 14 ++++++++ src/gete/catalog/connections/google.yaml | 29 +++++++++++++-- tests/conformance/test_catalog.py | 45 ++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 01b5168..fb0b8ad 100644 --- a/README.md +++ b/README.md @@ -212,6 +212,20 @@ connections: scopes: {read: Read internal data} ``` +A connection's `oauth.scopes` go to every agent that declares it, so they stay +a read-only minimum. Scopes under `oauth.optional_scopes` are a menu: an agent +gets one only by selecting it in its own declaration, and the selection lands +in that agent's own authorization, so consenting to one agent's writes grants +nothing to any other. A scope outside the menu is refused by `gete validate`. + +```yaml +# agent.yaml +connections: + - freee # the defaults only + - id: google # the defaults plus a selection from the menu + scopes: [https://www.googleapis.com/auth/spreadsheets] +``` + `oauth.pkce: true` asks Gemini Enterprise to carry a code challenge through the flow. An authorization server that requires PKCE refuses the code exchange without one, and there is no other way to ask for it from a declaration. diff --git a/src/gete/catalog/connections/google.yaml b/src/gete/catalog/connections/google.yaml index b52ab82..423c981 100644 --- a/src/gete/catalog/connections/google.yaml +++ b/src/gete/catalog/connections/google.yaml @@ -2,6 +2,7 @@ id: google display_name: Google Workspace docs: https://developers.google.com/workspace +# The ceiling: every host a token may be sent to, whatever scopes it carries. # One host per API. Listing googleapis.com as a whole would let a Workspace # authorization reach GCP APIs, and so would www.googleapis.com, which serves # storage, compute, and oauth2 next to the Workspace APIs. @@ -11,17 +12,41 @@ hosts: - sheets.googleapis.com - drive.googleapis.com - people.googleapis.com + - docs.googleapis.com + - slides.googleapis.com token_prefixes: - ya29. oauth: authorization_url: https://accounts.google.com/o/oauth2/v2/auth token_url: https://oauth2.googleapis.com/token - # Read-only. Asking for write access would let an agent send or approve - # on the user's behalf, and gete agents are not meant to. + # The floor: what every agent that declares the connection gets. Read-only + # and minimal, so a bare `connections: [google]` keeps meaning what it + # always meant. scopes: https://www.googleapis.com/auth/gmail.readonly: Read the subject, sender, and body of your mail https://www.googleapis.com/auth/calendar.readonly: Read your calendar events + # The menu: an agent gets one of these only by selecting it in its own + # declaration, and the selection lands in that agent's authorization alone. + # Write access lives here and never in the defaults, so no agent can write + # without saying so on its consent screen. + # + # drive.file reaches only the files the agent creates or the user opens + # with it; Drive-wide write is deliberately not offered. gmail.send is the + # heaviest entry - mail sent as the user is outward and cannot be recalled - + # and it grants sending alone: gmail.modify and mail.google.com are not + # offered, because rewriting or deleting the inbox is not needed to send. + optional_scopes: + https://www.googleapis.com/auth/spreadsheets.readonly: Read your spreadsheets + https://www.googleapis.com/auth/drive.readonly: Read your Drive files + https://www.googleapis.com/auth/documents.readonly: Read your documents + https://www.googleapis.com/auth/presentations.readonly: Read your presentations + https://www.googleapis.com/auth/spreadsheets: Read and edit your spreadsheets + https://www.googleapis.com/auth/documents: Read and edit your documents + https://www.googleapis.com/auth/presentations: Read and edit your presentations + https://www.googleapis.com/auth/calendar.events: Read and edit events on your calendars + https://www.googleapis.com/auth/drive.file: Read and edit the Drive files this agent creates or opens + https://www.googleapis.com/auth/gmail.send: Send mail as you examples: accepts: diff --git a/tests/conformance/test_catalog.py b/tests/conformance/test_catalog.py index 532d8a1..4163742 100644 --- a/tests/conformance/test_catalog.py +++ b/tests/conformance/test_catalog.py @@ -60,6 +60,51 @@ def test_google_hosts_are_specific_apis_not_the_whole_domain() -> None: assert all(host.endswith(".googleapis.com") for host in hosts) +def test_google_defaults_stay_the_read_only_minimum() -> None: + """A bare `connections: [google]` grants what it always did, nothing more.""" + assert set(CATALOG["google"]["oauth"]["scopes"]) == { + "https://www.googleapis.com/auth/gmail.readonly", + "https://www.googleapis.com/auth/calendar.readonly", + } + + +def test_google_offers_workspace_reads_and_writes_on_the_menu() -> None: + menu = set(CATALOG["google"]["oauth"]["optional_scopes"]) + assert { + "https://www.googleapis.com/auth/spreadsheets.readonly", + "https://www.googleapis.com/auth/drive.readonly", + "https://www.googleapis.com/auth/documents.readonly", + "https://www.googleapis.com/auth/presentations.readonly", + "https://www.googleapis.com/auth/spreadsheets", + "https://www.googleapis.com/auth/documents", + "https://www.googleapis.com/auth/presentations", + "https://www.googleapis.com/auth/calendar.events", + "https://www.googleapis.com/auth/drive.file", + "https://www.googleapis.com/auth/gmail.send", + } <= menu + + +def test_google_menu_has_no_blanket_drive_write_and_no_inbox_mutation() -> None: + """drive.file writes only what the agent created or opened, and sending + mail does not need the power to rewrite or delete the inbox.""" + menu = set(CATALOG["google"]["oauth"]["optional_scopes"]) + assert "https://www.googleapis.com/auth/drive" not in menu + assert "https://www.googleapis.com/auth/gmail.modify" not in menu + assert "https://mail.google.com/" not in menu + + +def test_google_sending_mail_says_so_on_the_consent_screen() -> None: + """Sending as the user is outward and irreversible; the text must not soften it.""" + menu = CATALOG["google"]["oauth"]["optional_scopes"] + assert "as you" in menu["https://www.googleapis.com/auth/gmail.send"] + + +def test_google_hosts_cover_the_docs_and_slides_apis() -> None: + hosts = CATALOG["google"]["hosts"] + assert "docs.googleapis.com" in hosts + assert "slides.googleapis.com" in hosts + + def test_catalog_files_are_read_with_dates_as_strings() -> None: """The loader used for the catalog is the same one that keeps dates as strings.""" text = ( From bfadb4deaf4f38b5f395d2f2c6d40ca7265c1830 Mon Sep 17 00:00:00 2001 From: haruotsu <65439874+haruotsu@users.noreply.github.com> Date: Thu, 27 Aug 2026 22:27:05 +0900 Subject: [PATCH 3/7] Let a hosts entry scope itself to a path prefix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some platforms serve unrelated APIs from a single host: www.googleapis.com carries Drive and Calendar next to GCP's storage, compute, and oauth2, so a connection could not name it without opening the whole platform to its tokens. An entry written as host/path/ now admits only requests below that path. A dot segment or an encoded separator - percent-encoded any number of times - is refused rather than resolved, because resolving would have to guess how many times the server decodes. The schema requires the trailing slash so /api cannot quietly admit /api-and-more. 🤖 Generated with Claude Code --- README.md | 5 ++++ src/gete/connection/registry.py | 46 +++++++++++++++++++++++++------ src/gete/schema/connection.json | 15 ++++++++-- tests/test_connection_registry.py | 43 +++++++++++++++++++++++++++++ tests/test_schema.py | 27 ++++++++++++++++++ 5 files changed, 126 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index fb0b8ad..af62e14 100644 --- a/README.md +++ b/README.md @@ -212,6 +212,11 @@ connections: scopes: {read: Read internal data} ``` +A `hosts` entry is an exact host name; nothing is matched by suffix. When one +host serves unrelated APIs side by side — `www.googleapis.com` carries Drive +and Calendar next to GCP's storage and compute — the entry can be scoped to a +path prefix, written `host/path/`, and requests must stay below that path. + A connection's `oauth.scopes` go to every agent that declares it, so they stay a read-only minimum. Scopes under `oauth.optional_scopes` are a menu: an agent gets one only by selecting it in its own declaration, and the selection lands diff --git a/src/gete/connection/registry.py b/src/gete/connection/registry.py index 6e57a15..f4a6be2 100644 --- a/src/gete/connection/registry.py +++ b/src/gete/connection/registry.py @@ -10,7 +10,7 @@ from collections.abc import Iterable, Iterator, Mapping from dataclasses import dataclass, field, replace from typing import Any -from urllib.parse import urlsplit +from urllib.parse import unquote, urlsplit from gete.catalog import catalog_connections from gete.errors import DeclarationError, RetiredConnection, UnknownConnection @@ -107,6 +107,26 @@ def from_mapping( ) +def _stays_below(path: str, prefix: str) -> bool: + """Whether the request path stays below the prefix however a server reads it. + + A dot segment or an encoded separator - percent-encoded any number of + times - is refused rather than resolved: resolving would have to guess + how many times the server decodes. + """ + for segment in path.split("/"): + decoded = unquote(segment) + while decoded != segment: + segment, decoded = decoded, unquote(decoded) + if segment in (".", "..") or "/" in segment or "\\" in segment: + return False + if not prefix.endswith("/"): + # The schema requires the trailing slash; a definition that dodged it + # must not widen the ceiling to /prefix-and-more. + prefix += "/" + return path.startswith("/" + prefix) + + @dataclass(frozen=True) class Examples: """Token shapes, not real tokens. The catalog checks accepts_token against them.""" @@ -230,21 +250,31 @@ def allows(self, url: str) -> bool: """Whether a token may be attached to a request for this URL. Only https, and only an exact host match. Prefix or suffix matching - would accept names such as slack.com.example.com. + would accept names such as slack.com.example.com. An entry written as + host/path/ admits only requests below that path: some platforms serve + unrelated APIs from one host, and the path is where they part. """ parsed = urlsplit(url) - return parsed.scheme == "https" and parsed.hostname in self.hosts + if parsed.scheme != "https" or parsed.hostname is None: + return False + for entry in self.hosts: + host, slash, prefix = entry.partition("/") + if parsed.hostname != host: + continue + if not slash or _stays_below(parsed.path, prefix): + return True + return False def allows_redirect(self, url: str) -> bool: """Whether a download may follow a redirect here; the token may not. - Exact hostname matching against the declared lists only: a named - host is no safer than an address literal, so nothing is accepted - for merely looking like a public name. + Exact matching against the declared lists only: a named host is no + safer than an address literal, so nothing is accepted for merely + looking like a public name. """ parsed = urlsplit(url) - return parsed.scheme == "https" and ( - parsed.hostname in self.hosts or parsed.hostname in self.redirect_hosts + return self.allows(url) or ( + parsed.scheme == "https" and parsed.hostname in self.redirect_hosts ) def reauthorization_message(self) -> str: diff --git a/src/gete/schema/connection.json b/src/gete/schema/connection.json index 738f840..1e00441 100644 --- a/src/gete/schema/connection.json +++ b/src/gete/schema/connection.json @@ -23,11 +23,18 @@ "$ref": "#/$defs/https_url" }, "hosts": { - "description": "Host names the user token may be sent to, matched exactly.", + "description": "Host names the user token may be sent to, matched exactly. An entry written as host/path/ admits only requests below that path, for hosts that serve unrelated APIs side by side.", "type": "array", "uniqueItems": true, "items": { - "$ref": "#/$defs/hostname" + "oneOf": [ + { + "$ref": "#/$defs/hostname" + }, + { + "$ref": "#/$defs/host_path_prefix" + } + ] } }, "base_url": { @@ -183,6 +190,10 @@ "hostname": { "type": "string", "pattern": "^(?=.{1,253}$)([a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?\\.)+[a-z]{2,63}$" + }, + "host_path_prefix": { + "type": "string", + "pattern": "^(?=[^/]{1,253}/)([a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?\\.)+[a-z]{2,63}(/[a-z0-9._~-]+)+/$" } } } diff --git a/tests/test_connection_registry.py b/tests/test_connection_registry.py index 8892347..b48b0b7 100644 --- a/tests/test_connection_registry.py +++ b/tests/test_connection_registry.py @@ -85,6 +85,49 @@ def test_base_url_host_is_allowed_too() -> None: assert entry.allows("https://api.example.com/v1") +@pytest.mark.parametrize( + ("url", "allowed"), + [ + ("https://shared.example.com/api/v1/things", True), + ("https://shared.example.com/api/", True), + ("https://shared.example.com/api", False), + ("https://shared.example.com/apix/v1", False), + ("https://shared.example.com/other/v1", False), + ("https://shared.example.com/", False), + ("https://shared.example.com/api/../other/v1", False), + ("https://shared.example.com/api/%2e%2e/other/v1", False), + ("https://shared.example.com/api/%252e%252e/other/v1", False), + ("https://shared.example.com/api/..%2fother/v1", False), + ("https://shared.example.com/api/x%5c../other", False), + ("http://shared.example.com/api/v1", False), + ], +) +def test_a_path_scoped_host_allows_only_requests_below_the_path( + url: str, allowed: bool +) -> None: + """Some platforms serve unrelated APIs from one host; a host/path/ entry + admits one API without admitting the platform. A dot segment or an encoded + separator is refused rather than resolved, because resolving would have to + guess how many times the server decodes.""" + entry = connection(hosts=["shared.example.com/api/"]) + assert entry.allows(url) is allowed + + +def test_a_plain_hostname_entry_keeps_admitting_every_path() -> None: + entry = connection(hosts=["api.example.com", "shared.example.com/api/"]) + assert entry.allows("https://api.example.com/anything/at/all") + assert not entry.allows("https://shared.example.com/anything/at/all") + + +def test_a_redirect_may_go_below_a_path_scoped_host_but_not_beside_it() -> None: + entry = connection( + hosts=["shared.example.com/api/"], redirect_hosts=["cdn.example.com"] + ) + assert entry.allows_redirect("https://shared.example.com/api/download") + assert not entry.allows_redirect("https://shared.example.com/other/download") + assert entry.allows_redirect("https://cdn.example.com/download") + + def test_oauth_client_defaults_to_ge_oauth_id() -> None: entry = connection() assert entry.secret_prefix == "ge-oauth-example" diff --git a/tests/test_schema.py b/tests/test_schema.py index 6128b02..d128c77 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -194,6 +194,33 @@ def test_a_connection_entry_needs_an_id_and_a_real_selection( ) +def test_a_host_may_be_scoped_to_a_path_prefix() -> None: + """One host can serve unrelated APIs; the entry admits one API's paths.""" + validate_document( + "connection", + {**CONNECTION, "hosts": ["www.example.com/api/", "www.example.com/up/api/"]}, + source="connection.yaml", + ) + + +@pytest.mark.parametrize( + "host", + [ + "www.example.com/api", + "www.example.com/API/", + "www.example.com//", + "www.example.com/", + "Www.example.com/api/", + ], +) +def test_a_path_scoped_host_ends_in_a_slash_and_stays_lowercase(host: str) -> None: + """Without the trailing slash /api would also admit /api-and-more.""" + with pytest.raises(DeclarationError, match="hosts"): + validate_document( + "connection", {**CONNECTION, "hosts": [host]}, source="connection.yaml" + ) + + def test_optional_scopes_map_a_scope_to_its_explanation() -> None: oauth = {**CONNECTION["oauth"], "optional_scopes": {"write": "Change data"}} validate_document( From b5ce50b3ce8cdd9db9a2177a2564d8c188f222ed Mon Sep 17 00:00:00 2001 From: haruotsu <65439874+haruotsu@users.noreply.github.com> Date: Thu, 27 Aug 2026 22:27:11 +0900 Subject: [PATCH 4/7] Reach Drive and Calendar at their real serving paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drive v3 and Calendar v3 are served from www.googleapis.com and nowhere else: drive.googleapis.com and calendar-json.googleapis.com are service names that route no requests, yet they sat on the host list looking like endpoints while every Drive and Calendar call was refused by the ceiling. Replace them with www.googleapis.com/calendar/, /drive/, and /upload/drive/ so the calendar.readonly default and the Drive and Calendar menu entries actually work, and give people.googleapis.com - until now a host no scope could use - contacts.readonly on the menu. Conformance pins the exact menu, that www.googleapis.com appears only path-scoped, and that storage, compute, and oauth2 stay refused. 🤖 Generated with Claude Code --- src/gete/catalog/connections/google.yaml | 17 ++++++---- tests/conformance/test_catalog.py | 40 +++++++++++++++++++++--- 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/src/gete/catalog/connections/google.yaml b/src/gete/catalog/connections/google.yaml index 423c981..87f3cd7 100644 --- a/src/gete/catalog/connections/google.yaml +++ b/src/gete/catalog/connections/google.yaml @@ -3,17 +3,21 @@ display_name: Google Workspace docs: https://developers.google.com/workspace # The ceiling: every host a token may be sent to, whatever scopes it carries. -# One host per API. Listing googleapis.com as a whole would let a Workspace -# authorization reach GCP APIs, and so would www.googleapis.com, which serves -# storage, compute, and oauth2 next to the Workspace APIs. +# One entry per API. Listing googleapis.com as a whole would let a Workspace +# authorization reach GCP APIs, and so would a bare www.googleapis.com, which +# serves storage, compute, and oauth2 next to Drive and Calendar. Those two +# are served nowhere else - their service names drive.googleapis.com and +# calendar-json.googleapis.com route no requests - so their entries scope +# www.googleapis.com to the APIs' own paths, Drive uploads included. hosts: - gmail.googleapis.com - - calendar-json.googleapis.com - sheets.googleapis.com - - drive.googleapis.com - - people.googleapis.com - docs.googleapis.com - slides.googleapis.com + - people.googleapis.com + - www.googleapis.com/calendar/ + - www.googleapis.com/drive/ + - www.googleapis.com/upload/drive/ token_prefixes: - ya29. @@ -41,6 +45,7 @@ oauth: https://www.googleapis.com/auth/drive.readonly: Read your Drive files https://www.googleapis.com/auth/documents.readonly: Read your documents https://www.googleapis.com/auth/presentations.readonly: Read your presentations + https://www.googleapis.com/auth/contacts.readonly: Read your contacts https://www.googleapis.com/auth/spreadsheets: Read and edit your spreadsheets https://www.googleapis.com/auth/documents: Read and edit your documents https://www.googleapis.com/auth/presentations: Read and edit your presentations diff --git a/tests/conformance/test_catalog.py b/tests/conformance/test_catalog.py index 4163742..4ee4e38 100644 --- a/tests/conformance/test_catalog.py +++ b/tests/conformance/test_catalog.py @@ -55,9 +55,37 @@ def test_google_hosts_are_specific_apis_not_the_whole_domain() -> None: """A Workspace authorization must not be usable against GCP APIs.""" hosts = CATALOG["google"]["hosts"] assert "googleapis.com" not in hosts - # www.googleapis.com serves storage, compute, and oauth2 as well. + # www.googleapis.com serves storage, compute, and oauth2 as well; it may + # appear only scoped to the path of an API that has no other home. assert "www.googleapis.com" not in hosts - assert all(host.endswith(".googleapis.com") for host in hosts) + for host in hosts: + name, _, path = host.partition("/") + assert name.endswith(".googleapis.com") + if name == "www.googleapis.com": + assert path, host + + +def test_google_reaches_drive_and_calendar_below_their_paths_only() -> None: + """Drive v3 and Calendar v3 are served from www.googleapis.com and nowhere + else. Their service names (drive.googleapis.com, calendar-json.googleapis.com) + route no requests and must not sit on the list looking like they do.""" + hosts = CATALOG["google"]["hosts"] + assert "www.googleapis.com/calendar/" in hosts + assert "www.googleapis.com/drive/" in hosts + assert "www.googleapis.com/upload/drive/" in hosts + assert "drive.googleapis.com" not in hosts + assert "calendar-json.googleapis.com" not in hosts + + +def test_google_scoped_entries_admit_the_apis_and_refuse_the_platform() -> None: + google = Registry.from_catalog().get("google") + assert google.allows("https://www.googleapis.com/calendar/v3/calendars/primary") + assert google.allows("https://www.googleapis.com/drive/v3/files") + assert google.allows("https://www.googleapis.com/upload/drive/v3/files") + assert not google.allows("https://www.googleapis.com/storage/v1/b/bucket") + assert not google.allows("https://www.googleapis.com/compute/v1/projects/p") + assert not google.allows("https://www.googleapis.com/oauth2/v4/token") + assert not google.allows("https://www.googleapis.com/drive/../storage/v1/b/b") def test_google_defaults_stay_the_read_only_minimum() -> None: @@ -69,19 +97,21 @@ def test_google_defaults_stay_the_read_only_minimum() -> None: def test_google_offers_workspace_reads_and_writes_on_the_menu() -> None: - menu = set(CATALOG["google"]["oauth"]["optional_scopes"]) - assert { + """The whole menu, pinned: every entry is reviewed, and every entry is + served by a host in the ceiling.""" + assert set(CATALOG["google"]["oauth"]["optional_scopes"]) == { "https://www.googleapis.com/auth/spreadsheets.readonly", "https://www.googleapis.com/auth/drive.readonly", "https://www.googleapis.com/auth/documents.readonly", "https://www.googleapis.com/auth/presentations.readonly", + "https://www.googleapis.com/auth/contacts.readonly", "https://www.googleapis.com/auth/spreadsheets", "https://www.googleapis.com/auth/documents", "https://www.googleapis.com/auth/presentations", "https://www.googleapis.com/auth/calendar.events", "https://www.googleapis.com/auth/drive.file", "https://www.googleapis.com/auth/gmail.send", - } <= menu + } def test_google_menu_has_no_blanket_drive_write_and_no_inbox_mutation() -> None: From 402fa402bdc607f5634f275a75e2adbc65dfd928 Mon Sep 17 00:00:00 2001 From: haruotsu <65439874+haruotsu@users.noreply.github.com> Date: Thu, 27 Aug 2026 22:27:16 +0900 Subject: [PATCH 5/7] Refuse a connection declared in both forms at register time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit uniqueItems cannot see that a string entry and a mapping entry name the same connection, and scope selections are keyed by id, so one entry's selection would silently stand for every duplicate on the consent screen. validate already reports the duplicate, but register can run without validate, so it now refuses the duplicate too rather than registering an authorization the declaration does not state unambiguously. validate also reports the duplicate itself when the connection is unknown, instead of two unknown-connection errors. 🤖 Generated with Claude Code --- src/gete/register.py | 13 +++++++++++++ src/gete/validate.py | 7 ++++++- tests/test_register.py | 30 ++++++++++++++++++++++++++++++ tests/test_validate.py | 11 +++++++++++ 4 files changed, 60 insertions(+), 1 deletion(-) diff --git a/src/gete/register.py b/src/gete/register.py index 7d3eff0..1e5d071 100644 --- a/src/gete/register.py +++ b/src/gete/register.py @@ -259,6 +259,19 @@ def _parent(self) -> str: return f"projects/{self._number}/locations/{self._ge_location}" def _register(self, agent: Agent, engine: str, summary: Summary) -> None: + seen: set[str] = set() + for connection_id in agent.connections: + if connection_id in seen: + # The schema cannot see that a string entry and a mapping + # entry name the same connection, and scope selections are + # keyed by id, so one entry's selection would stand for every + # duplicate silently. + raise DeclarationError( + f"connections: {connection_id} is declared twice; " + "only one entry's scope selection could reach the " + "consent screen" + ) + seen.add(connection_id) engines_url = ( f"https://{self._location}-aiplatform.googleapis.com/v1/projects/" f"{self._gcp_project}/locations/{self._location}/reasoningEngines" diff --git a/src/gete/validate.py b/src/gete/validate.py index 11413c0..6ea9550 100644 --- a/src/gete/validate.py +++ b/src/gete/validate.py @@ -98,13 +98,18 @@ def _agent_problems( f"{MIN_SERVICE_ACCOUNT_ID_LENGTH} and {MAX_SERVICE_ACCOUNT_ID_LENGTH} " "characters; Terraform would be refused at apply time" ) + # known holds only the ids the registry resolved; the rules below look + # them up again. Duplicates are tracked apart so an unknown connection + # declared twice is still one duplicate, not two unknowns. known: set[str] = set() + declared: set[str] = set() for connection_id in agent.connections: - if connection_id in known: + if connection_id in declared: # uniqueItems cannot see that a string entry and a mapping entry # name the same connection. found.append(f"connections: {connection_id} is declared twice") continue + declared.add(connection_id) try: connection = registry.get(connection_id) except GeteError as error: diff --git a/tests/test_register.py b/tests/test_register.py index b3bfdc2..47886b4 100644 --- a/tests/test_register.py +++ b/tests/test_register.py @@ -365,6 +365,36 @@ def test_the_registrar_builds_each_authorization_from_that_agents_selection( assert params["scope"] == ["read write"] +def test_the_registrar_refuses_a_connection_declared_in_both_forms( + project: ProjectBuilder, gcp: FakeGcp, tmp_path: Path +) -> None: + """The schema cannot see the two forms are one connection, register can run + without validate, and one entry's selection would stand for both silently.""" + project.write_project( + { + "version": 1, + "project": "example-project", + "location": "us-central1", + "gemini_enterprise": {"project_number": NUMBER}, + "connections": {"example": {k: v for k, v in MENU.items() if k != "id"}}, + } + ) + project.write_agent( + "finance", + { + **FINANCE, + "connections": ["example", {"id": "example", "scopes": ["write"]}], + }, + ) + summary = register_project( + load_project(project.root / "gete.yaml"), gcp, tmp_path / "n.md" + ) + assert summary.failed == ["finance"] + assert any("twice" in message for message in summary.messages) + assert gcp.writes("POST") == [] + assert gcp.writes("PATCH") == [] + + def test_existing_authorization_is_updated_not_recreated( project: ProjectBuilder, gcp: FakeGcp, tmp_path: Path ) -> None: diff --git a/tests/test_validate.py b/tests/test_validate.py index fe726b5..d78c779 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -146,6 +146,17 @@ def test_the_same_connection_in_both_forms_is_reported( assert any("twice" in p for p in problems(project)) +def test_a_duplicate_is_reported_even_when_the_connection_is_unknown( + project: ProjectBuilder, +) -> None: + """A typo repeated in both forms is one mistake, not two unknown connections.""" + project.write_agent( + "mail-triage", + {"connections": ["salesforce", {"id": "salesforce", "scopes": ["write"]}]}, + ) + assert any("twice" in p for p in problems(project)) + + def test_authorization_id_longer_than_63_is_reported(project: ProjectBuilder) -> None: """- becomes an authorization id, which is a DNS-style label.""" name = "a" * 58 From f04bbbb03692ec7991da7c9bec528f6006d6720f Mon Sep 17 00:00:00 2001 From: haruotsu <65439874+haruotsu@users.noreply.github.com> Date: Thu, 27 Aug 2026 22:42:24 +0900 Subject: [PATCH 6/7] Keep the declaration checks in step with path-scoped hosts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three ways a declaration could promise path scoping that allows() would not deliver, each now caught where the declaration is checked: A bare hosts entry admits every path on its host, so a scoped entry for the same host never applies - it reads as a restriction it does not make. base_url puts its host on the list bare, so setting one on a scoped host widened the ceiling the same way, silently. Both are reported as connection problems. mcp.url was matched by hostname membership, which cannot see a path-scoped entry: a URL below the path was wrongly refused, and one beside the path on the same host would need a bare entry to pass. The check now asks allows(), the same question the token answers to. The hosts schema admitted "." and ".." segments in a host/path/ entry. allows() refuses every request that carries a dot segment, so such an entry could never match anything; refusing it at declaration time beats shipping a silently dead entry. 🤖 Generated with Claude Code --- README.md | 3 ++ src/gete/connection/checks.py | 30 +++++++++++++++++--- src/gete/schema/connection.json | 2 +- tests/test_connection_registry.py | 46 +++++++++++++++++++++++++++++++ tests/test_schema.py | 27 +++++++++++++++++- 5 files changed, 102 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index af62e14..3aa54e6 100644 --- a/README.md +++ b/README.md @@ -216,6 +216,9 @@ A `hosts` entry is an exact host name; nothing is matched by suffix. When one host serves unrelated APIs side by side — `www.googleapis.com` carries Drive and Calendar next to GCP's storage and compute — the entry can be scoped to a path prefix, written `host/path/`, and requests must stay below that path. +A bare entry admits every path on its host, so declaring the same host bare +next to a scoped entry — or setting `base_url` on that host, which lists it +bare — is reported: the scoping would silently not happen. A connection's `oauth.scopes` go to every agent that declares it, so they stay a read-only minimum. Scopes under `oauth.optional_scopes` are a menu: an agent diff --git a/src/gete/connection/checks.py b/src/gete/connection/checks.py index 38a026d..8f78e75 100644 --- a/src/gete/connection/checks.py +++ b/src/gete/connection/checks.py @@ -76,6 +76,24 @@ def connection_problems(connection: Connection, registry: Registry) -> list[str] problems.append( f"hosts: {host} is a whole platform domain, list the API host" ) + for entry in sorted(connection.hosts): + # A bare entry admits every path on its host, so a scoped entry for + # the same host never applies - it reads as a restriction it does not + # make. base_url puts its host on the list bare, so setting one on a + # scoped host silently widens the ceiling the same way. + host, slash, _ = entry.partition("/") + if not slash or host not in connection.hosts: + continue + if connection.base_url and urlsplit(connection.base_url).hostname == host: + problems.append( + f"hosts: {entry} never applies; base_url puts {host} on the " + "list bare, and a bare entry admits every path" + ) + else: + problems.append( + f"hosts: {entry} never applies; the bare {host} entry admits " + "every path" + ) for other in registry.all(include_retired=True): if other.id == connection.id: continue @@ -111,8 +129,12 @@ def connection_problems(connection: Connection, registry: Registry) -> list[str] ) if not claims_google and connection.accepts_token(_GOOGLE_ACCESS_TOKEN_EXAMPLE): problems.append("a Google access token is accepted") - if connection.mcp_url is not None and not connection.needs_base_url: - mcp_host = urlsplit(connection.mcp_url).hostname - if mcp_host not in connection.hosts: - problems.append(f"mcp.url: host {mcp_host} is not in hosts") + # The MCP server is spoken to with the user's token, so its URL must sit + # where hosts lets the token go - path scoping included. + if ( + connection.mcp_url is not None + and not connection.needs_base_url + and not connection.allows(connection.mcp_url) + ): + problems.append(f"mcp.url: {connection.mcp_url} is not covered by hosts") return problems diff --git a/src/gete/schema/connection.json b/src/gete/schema/connection.json index 1e00441..5647653 100644 --- a/src/gete/schema/connection.json +++ b/src/gete/schema/connection.json @@ -193,7 +193,7 @@ }, "host_path_prefix": { "type": "string", - "pattern": "^(?=[^/]{1,253}/)([a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?\\.)+[a-z]{2,63}(/[a-z0-9._~-]+)+/$" + "pattern": "^(?=[^/]{1,253}/)([a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?\\.)+[a-z]{2,63}(/(?!\\.\\.?/)[a-z0-9._~-]+)+/$" } } } diff --git a/tests/test_connection_registry.py b/tests/test_connection_registry.py index b48b0b7..fbad96f 100644 --- a/tests/test_connection_registry.py +++ b/tests/test_connection_registry.py @@ -256,6 +256,32 @@ def test_connection_without_hosts_is_reported() -> None: ) +def test_a_path_scoped_entry_next_to_the_bare_host_is_reported() -> None: + """The bare entry admits every path, so the scoped one reads as a + restriction it does not make.""" + entry = connection(hosts=["shared.example.com", "shared.example.com/api/"]) + assert any( + "shared.example.com/api/" in problem + for problem in connection_problems(entry, Registry([entry])) + ) + + +def test_a_base_url_on_the_scoped_host_is_reported_as_the_source() -> None: + """base_url puts its host on the list bare; a scoped entry cannot narrow it.""" + entry = connection( + hosts=["shared.example.com/api/"], base_url="https://shared.example.com" + ) + assert any( + "base_url" in problem + for problem in connection_problems(entry, Registry([entry])) + ) + + +def test_a_path_scoped_entry_without_the_bare_host_is_not_reported() -> None: + entry = connection(hosts=["api.example.com", "shared.example.com/api/"]) + assert connection_problems(entry, Registry([entry])) == [] + + def test_mcp_host_must_be_a_declared_host() -> None: entry = connection(mcp={"url": "https://mcp.example.com/mcp"}) assert any( @@ -269,6 +295,26 @@ def test_mcp_host_must_be_a_declared_host() -> None: assert connection_problems(good, Registry([good])) == [] +def test_mcp_url_below_a_path_scoped_host_is_a_declared_host() -> None: + entry = connection( + hosts=["shared.example.com/api/"], + mcp={"url": "https://shared.example.com/api/mcp"}, + ) + assert connection_problems(entry, Registry([entry])) == [] + + +def test_mcp_url_beside_the_scoped_path_is_reported() -> None: + """The MCP server is spoken to with the token; beside the path is off-limits.""" + entry = connection( + hosts=["shared.example.com/api/"], + mcp={"url": "https://shared.example.com/mcp"}, + ) + assert any( + "mcp.url" in problem + for problem in connection_problems(entry, Registry([entry])) + ) + + def test_examples_are_checked_against_accepts_token() -> None: entry = connection( token_prefixes=["ex_"], examples={"accepts": ["other_1"], "rejects": ["ex_1"]} diff --git a/tests/test_schema.py b/tests/test_schema.py index d128c77..412577e 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -198,11 +198,36 @@ def test_a_host_may_be_scoped_to_a_path_prefix() -> None: """One host can serve unrelated APIs; the entry admits one API's paths.""" validate_document( "connection", - {**CONNECTION, "hosts": ["www.example.com/api/", "www.example.com/up/api/"]}, + { + **CONNECTION, + "hosts": [ + "www.example.com/api/", + "www.example.com/up/api/", + "www.example.com/api.v3/", + ], + }, source="connection.yaml", ) +@pytest.mark.parametrize( + "host", + [ + "www.example.com/../", + "www.example.com/./", + "www.example.com/api/../", + "www.example.com/./api/", + ], +) +def test_a_path_scoped_host_may_not_contain_dot_segments(host: str) -> None: + """allows() refuses every request that carries a dot segment, so such an + entry could never match; refusing it here says so at declaration time.""" + with pytest.raises(DeclarationError, match="hosts"): + validate_document( + "connection", {**CONNECTION, "hosts": [host]}, source="connection.yaml" + ) + + @pytest.mark.parametrize( "host", [ From e68e3e1b4f9c21ac4dfb138eba7d17763c9596b2 Mon Sep 17 00:00:00 2001 From: haruotsu <65439874+haruotsu@users.noreply.github.com> Date: Thu, 27 Aug 2026 22:44:02 +0900 Subject: [PATCH 7/7] Fold the bare-host message the way ruff format writes it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI checks formatting with ruff format --check, which local verification had skipped; the message fits on one line, so format folds it there. 🤖 Generated with Claude Code --- src/gete/connection/checks.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/gete/connection/checks.py b/src/gete/connection/checks.py index 8f78e75..7886823 100644 --- a/src/gete/connection/checks.py +++ b/src/gete/connection/checks.py @@ -91,8 +91,7 @@ def connection_problems(connection: Connection, registry: Registry) -> list[str] ) else: problems.append( - f"hosts: {entry} never applies; the bare {host} entry admits " - "every path" + f"hosts: {entry} never applies; the bare {host} entry admits every path" ) for other in registry.all(include_retired=True): if other.id == connection.id: