diff --git a/README.md b/README.md index c90adf5..b8255c3 100644 --- a/README.md +++ b/README.md @@ -239,8 +239,8 @@ tools: ### Connections `gete connections` lists what ships: `freee`, `freee-mcp`, `google`, `github`, -`notion-mcp`, `zendesk`, and `slack` (retired, with the reason). Add your own or -override a catalog entry in `gete.yaml`: +`notion-mcp`, `slack-mcp`, and `zendesk`. Add your own or override a catalog +entry in `gete.yaml`: ```yaml connections: @@ -401,9 +401,9 @@ Adding a connection to the catalog is one YAML file under ### Shared credentials A connection reads with the caller's token. Some writes have no such token -to ride on — Slack posting is one, which is why the `slack` connection is -retired — so gete also ships tools that act with a credential the agent -holds. Whoever can call the agent acts through that credential; the tools +to ride on — posting to Slack from an agent nobody has authorized is one — +so gete also ships tools that act with a credential the agent holds. +Whoever can call the agent acts through that credential; the tools and their guardrails ship with gete, and a declaration can only name them: ```yaml diff --git a/src/gete/catalog/connections/slack-mcp.yaml b/src/gete/catalog/connections/slack-mcp.yaml new file mode 100644 index 0000000..35cda3c --- /dev/null +++ b/src/gete/catalog/connections/slack-mcp.yaml @@ -0,0 +1,126 @@ +id: slack-mcp +display_name: Slack MCP +docs: https://docs.slack.dev/ai/slack-mcp-server/ + +# The token this hands out is an ordinary Slack user token, and slack.com's +# Web API would very likely take it. slack.com still does not belong on this +# list: the face of this connection is the MCP server's tools, which an mcp: +# block can hold to an allow list. Naming slack.com beside it would make the +# whole Web API the face - every method the token's scopes reach, with +# nothing to hold it to. +hosts: + - mcp.slack.com +# Slack user tokens announce themselves, so this connection is not accepted +# by elimination and may sit beside one that is. With token rotation enabled +# the prefix becomes xoxe.xoxp-. +token_prefixes: + - xoxp- + - xoxe.xoxp- + +oauth: + # The authorization server mcp.slack.com advertises (RFC 8414): its own + # issuer, with the endpoints on slack.com. This is the user pair, not the + # app pair at oauth/v2/authorize and oauth.v2.access. That one answers with + # the app's token at the top level and the user's underneath, in + # authed_user, where Gemini Enterprise never looks; oauth.v2.user.access + # answers with the user's token at the top level and nothing else to take. + authorization_url: https://slack.com/oauth/v2_user/authorize + # The metadata names client_secret_post as the only client authentication + # method. Whether Gemini Enterprise sends the secret that way is + # unconfirmed, and a connection cannot say: the Authorization resource has + # no field for it. + token_url: https://slack.com/api/oauth.v2.user.access + # scope_parameter stays at its default. The app pair needs user_scope + # because oauth/v2/authorize reads scope as the app's own permissions. This + # endpoint is reached by standard MCP clients as an RFC 8414 authorization + # server, and those send a plain scope parameter, so scope it is - + # unconfirmed; user_scope is the switch if the consent screen answers + # invalid_scope. + # pkce stays unset: S256 is supported here, not required. + # No authorization_query either. One would keep access_type and prompt off + # the URL, as notion-mcp does, but it would also fix the scopes, and the + # menu below could not be offered beside it. Slack is expected to ignore + # parameters it does not know; unconfirmed. + # + # Slack fixes a user token's scopes at consent. A scope added here later + # reaches nobody already authorized until their authorization is reset, so + # the floor is all that reading Slack takes, and the menu holds the rest. + scopes: + search:read.public: Search public channels + search:read.private: Search private channels you are a member of + search:read.im: Search your direct messages + search:read.mpim: Search your group direct messages + search:read.users: Search people in the workspace + channels:history: Read messages in public channels + groups:history: Read messages in private channels you are a member of + im:history: Read your direct messages + mpim:history: Read your group direct messages + channels:read: List public channels and their members + groups:read: List private channels you are a member of and their members + im:read: List your direct messages + mpim:read: List your group direct messages + users:read: Read people's profiles + optional_scopes: + search:read.files: Search files + files:read: Read files shared with you + chat:write: Send messages as you + reactions:write: Add reactions as you + canvases:read: Read canvases + canvases:write: Create and edit canvases as you + lists:read: Read lists + lists:write: Create and edit lists as you + # Not offered at all: users:read.email, because nothing here needs a + # person's email address; channels:write, groups:write, im:write and + # mpim:write, because no agent has a reason to open a conversation; + # files:write and emoji:read, for want of a use. + +mcp: + url: https://mcp.slack.com/mcp + +setup: | + The MCP server registers no clients of its own: an MCP client is backed by + a Slack app, and only an app from the Slack Marketplace or one internal to + the workspace will do; an unlisted app is refused. Create an internal app, + or take an existing one, and under OAuth & Permissions add every scope + named above to its User Token Scopes - the defaults and the optional ones + both, since an agent may select any of them. Leave Bot Token Scopes alone: + this connection carries the user's token and never the app's. + + Register exactly one redirect URI on the app (Slack calls them Redirect + URLs): the one named above. Put the app's client id and client secret in + the secrets named above. + + Where the workspace requires apps to be approved, an admin has to approve + this one before anyone can authorize. Semantic search needs a plan that + includes Slack AI search; keyword search works on any plan. + + No authorization has been taken through Gemini Enterprise yet, so the + following is read off the server's metadata rather than seen to work: + - The token endpoint names client_secret_post as its only client + authentication method. Whether Gemini Enterprise sends the client secret + that way is unconfirmed. + - The scopes are sent under scope, the parameter a standard MCP client + uses. If the consent screen answers invalid_scope, set + oauth.scope_parameter to user_scope in gete.yaml. + - The authorization URL carries access_type and prompt, which are + Google's vocabulary. Whether Slack ignores them is unconfirmed. + - A standard MCP client sends resource (RFC 8707) with the code exchange; + Gemini Enterprise does not. Whether Slack requires it is unconfirmed. + - The metadata offers the refresh_token grant, so tokens may expire. + Whether Gemini Enterprise refreshes them is its own doing. + +# verified is absent because no authorization has been taken through Gemini +# Enterprise yet. + +examples: + accepts: + - "xoxp-1234-5678-abcdef" + - "xoxe.xoxp-1-abcdef" + rejects: + - "ya29.a0AfH6SMB" # Google access token + # Claims: {"iss": "https://accounts.google.com"}, as in an ID token. + - "eyJhbGciOiJSUzI1NiJ9.eyJpc3MiOiJodHRwczovL2FjY291bnRzLmdvb2dsZS5jb20ifQ.sig" + - "gho_16C7e42F292c6912E7710c838347Ae178B4a" # GitHub + # A bot token belongs to the app, not to a person; the connection is per + # user and refusing the shape keeps it so. + - "xoxb-1234-5678-abcdef" diff --git a/src/gete/catalog/connections/slack.yaml b/src/gete/catalog/connections/slack.yaml deleted file mode 100644 index 6d32070..0000000 --- a/src/gete/catalog/connections/slack.yaml +++ /dev/null @@ -1,32 +0,0 @@ -id: slack -display_name: Slack -docs: https://api.slack.com/ - -retired: >- - Slack is read through the Gemini Enterprise native connector, which is - authorized per user and searchable. Custom agents cannot reach that data - store, so a token declared here would never arrive. - -hosts: - - slack.com -# With token rotation enabled the prefix becomes xoxe.xoxp-. -token_prefixes: - - xoxp- - - xoxe.xoxp- - -oauth: - authorization_url: https://slack.com/oauth/v2/authorize - token_url: https://slack.com/api/oauth.v2.access - # "scope" names the app's permissions; user permissions go in user_scope. - scope_parameter: user_scope - scopes: - search:read: Search messages - channels:history: Read messages in channels you are a member of - -examples: - accepts: - - "xoxp-1234-5678-abcdef" - - "xoxe.xoxp-1-abcdef" - rejects: - - "ya29.a0AfH6SMB" - - "eyJhbGciOiJSUzI1NiJ9.e30.sig" diff --git a/tests/conformance/test_catalog.py b/tests/conformance/test_catalog.py index f57c57b..07bb147 100644 --- a/tests/conformance/test_catalog.py +++ b/tests/conformance/test_catalog.py @@ -14,7 +14,7 @@ def test_catalog_has_the_initial_connections() -> None: - assert {"freee", "google", "github", "notion-mcp", "slack"} <= set(CATALOG) + assert {"freee", "google", "github", "notion-mcp", "slack-mcp"} <= set(CATALOG) @pytest.mark.parametrize("connection_id", sorted(CATALOG)) @@ -47,10 +47,6 @@ def test_google_access_tokens_are_rejected_everywhere_but_google() -> None: assert entry.accepts_token("ya29.a0AfH6SMB") is expected, entry.id -def test_slack_is_retired_with_a_reason() -> None: - assert CATALOG["slack"]["retired"] - - 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"] @@ -209,6 +205,106 @@ def test_freee_and_freee_mcp_cannot_be_held_by_one_agent() -> None: assert elimination_problems(["freee", "freee-mcp"], registry) +def test_slack_mcp_does_not_reach_the_slack_web_api() -> None: + """The token is an ordinary Slack user token and the Web API would very + likely take it, but the face of this connection is the MCP server's tools. + slack.com beside it would make every Web API method the token's scopes + reach part of that face.""" + hosts = CATALOG["slack-mcp"]["hosts"] + assert hosts == ["mcp.slack.com"] + assert "slack.com" not in hosts + + +def test_slack_mcp_defaults_stay_read_only_with_writes_on_the_menu() -> None: + """A bare `connections: [slack-mcp]` searches and reads what the user can; + files, writing, canvases and lists have to be selected.""" + oauth = CATALOG["slack-mcp"]["oauth"] + assert set(oauth["scopes"]) == { + "search:read.public", + "search:read.private", + "search:read.im", + "search:read.mpim", + "search:read.users", + "channels:history", + "groups:history", + "im:history", + "mpim:history", + "channels:read", + "groups:read", + "im:read", + "mpim:read", + "users:read", + } + assert set(oauth["optional_scopes"]) == { + "search:read.files", + "files:read", + "chat:write", + "reactions:write", + "canvases:read", + "canvases:write", + "lists:read", + "lists:write", + } + + +def test_slack_mcp_offers_no_email_addresses_and_creates_no_conversations() -> None: + """Nothing here needs a person's email address, and no agent has a reason + to open channels or direct messages; neither is on the menu at all.""" + oauth = CATALOG["slack-mcp"]["oauth"] + offered = set(oauth["scopes"]) | set(oauth["optional_scopes"]) + for scope in ( + "users:read.email", + "channels:write", + "groups:write", + "im:write", + "mpim:write", + "files:write", + "emoji:read", + ): + assert scope not in offered, scope + + +def test_slack_mcp_sends_its_scopes_under_scope_with_no_verbatim_query() -> None: + """Slack's app pair needs user_scope because oauth/v2/authorize reads scope + as the app's own permissions; the user pair behind the MCP server is + reached by standard MCP clients with a plain scope parameter. A verbatim + authorization_query would fix the scopes and leave no menu.""" + oauth = CATALOG["slack-mcp"]["oauth"] + assert "scope_parameter" not in oauth + assert "authorization_query" not in oauth + assert "pkce" not in oauth + + +def test_slack_mcp_takes_user_tokens_and_refuses_bot_tokens() -> None: + """The connection is per user; an app's xoxb- token must never pass as one.""" + slack_mcp = Registry.from_catalog().get("slack-mcp") + assert slack_mcp.accepts_token("xoxp-1234-5678-abcdef") + assert slack_mcp.accepts_token("xoxe.xoxp-1-abcdef") + assert not slack_mcp.accepts_token("xoxb-1234-5678-abcdef") + assert "xoxb-1234-5678-abcdef" in CATALOG["slack-mcp"]["examples"]["rejects"] + + +def test_slack_mcp_says_what_a_person_has_to_do_before_authorizing() -> None: + """The app's user scopes, the one redirect URI, and what has not been seen + to work yet.""" + setup = CATALOG["slack-mcp"]["setup"] + assert "User Token Scopes" in setup + assert "Bot Token Scopes" in setup + assert "redirect URI" in setup + assert "unconfirmed" in setup + + +def test_slack_mcp_is_not_verified_until_an_authorization_has_been_taken() -> None: + assert "verified" not in CATALOG["slack-mcp"] + + +def test_slack_mcp_can_sit_beside_a_connection_accepted_by_elimination() -> None: + """Its tokens announce themselves, so it takes nothing by elimination.""" + registry = Registry.from_catalog() + assert elimination_problems(["slack-mcp", "freee"], registry) == [] + assert elimination_problems(["slack-mcp", "notion-mcp"], registry) == [] + + def test_zendesk_leaves_its_root_open_until_an_installation_names_it() -> None: """The tenant is a subdomain; a stand-in host would be a name a stranger could register, and then a user's token would be sent there.""" diff --git a/tests/test_connection_registry.py b/tests/test_connection_registry.py index 39f93b6..f296238 100644 --- a/tests/test_connection_registry.py +++ b/tests/test_connection_registry.py @@ -381,10 +381,11 @@ def test_unknown_connection_names_the_known_ones(catalog: Registry) -> None: catalog.get("nope") -def test_retired_connection_explains_why(catalog: Registry) -> None: - with pytest.raises(RetiredConnection, match="connector"): - catalog.get("slack") - assert catalog.get("slack", include_retired=True).retired +def test_retired_connection_explains_why() -> None: + registry = Registry([connection(id="old", retired="Declare new instead.")]) + with pytest.raises(RetiredConnection, match="Declare new instead"): + registry.get("old") + assert registry.get("old", include_retired=True).retired def test_overlapping_prefixes_between_connections_are_reported() -> None: diff --git a/tests/test_connections_command.py b/tests/test_connections_command.py index 636bfca..2a6cd05 100644 --- a/tests/test_connections_command.py +++ b/tests/test_connections_command.py @@ -13,11 +13,10 @@ def test_table_lists_every_connection_with_hosts_and_verification() -> None: rows = connections_table(Registry.from_catalog()) by_id = {row["id"]: row for row in rows} - assert set(by_id) >= {"freee", "google", "github", "slack"} + assert set(by_id) >= {"freee", "google", "github", "slack-mcp"} assert "api.freee.co.jp" in by_id["freee"]["hosts"] assert by_id["freee"]["verified"] == "2026-08-20" assert by_id["github"]["verified"] == "not verified in Gemini Enterprise" - assert by_id["slack"]["status"] == "retired" assert by_id["freee"]["status"] == "available" @@ -62,13 +61,14 @@ def test_the_description_shows_the_menu_next_to_the_default_scopes() -> None: def test_cli_prints_one_line_per_connection(project: ProjectBuilder) -> None: + write_connections(project) runner = CliRunner() with runner.isolated_filesystem(temp_dir=project.root): result = runner.invoke(main, ["connections"]) assert result.exit_code == 0, result.output lines = [line for line in result.output.splitlines() if line.strip()] assert any(line.startswith("freee") for line in lines) - assert any("retired" in line and line.startswith("slack") for line in lines) + assert any("retired" in line and line.startswith("old-api") for line in lines) def test_cli_connections_works_without_a_project() -> None: @@ -100,15 +100,33 @@ def test_cli_connections_works_without_a_project() -> None: } -def describe(project: ProjectBuilder, connection_id: str) -> Any: +# A connection nobody may declare any more; the reason travels with it. +RETIRED: dict[str, Any] = { + "display_name": "Old API", + "hosts": ["api.old.example.com"], + "token_prefixes": ["old_"], + "retired": "Its data is read through the native connector; declare internal-api.", + "oauth": { + "authorization_url": "https://auth.old.example.com/authorize", + "token_url": "https://auth.old.example.com/token", + "scopes": {"read": "Read old data"}, + }, +} + + +def write_connections(project: ProjectBuilder) -> None: project.write_project( { "version": 1, "project": "example-project", "location": "us-central1", - "connections": {"internal-api": WITH_SETUP}, + "connections": {"internal-api": WITH_SETUP, "old-api": RETIRED}, } ) + + +def describe(project: ProjectBuilder, connection_id: str) -> Any: + write_connections(project) runner = CliRunner() with runner.isolated_filesystem(temp_dir=project.root): return runner.invoke(main, ["connections", connection_id]) @@ -158,16 +176,27 @@ def test_a_catalog_connection_can_be_described_without_a_project() -> None: assert "api.github.com" in result.output -def test_a_retired_connection_reads_retired_with_the_reason_alongside() -> None: +def test_a_retired_connection_reads_retired_with_the_reason_alongside( + project: ProjectBuilder, +) -> None: """The listing says "retired"; describing one must not say something else.""" - runner = CliRunner() - with runner.isolated_filesystem(): - result = runner.invoke(main, ["connections", "slack"]) + result = describe(project, "old-api") assert result.exit_code == 0, result.output assert "status" in result.output and "retired" in result.output assert "native connector" in result.output +def test_describing_slack_mcp_prints_the_app_setup_without_a_project() -> None: + """The Slack app is prepared by a person; the description has to carry it.""" + runner = CliRunner() + with runner.isolated_filesystem(): + result = runner.invoke(main, ["connections", "slack-mcp"]) + assert result.exit_code == 0, result.output + assert "mcp.slack.com" in result.output + assert "Before anyone can authorize:" in result.output + assert "User Token Scopes" in result.output + + def prefixless(**patch: Any) -> Connection: return Connection.from_mapping( { diff --git a/tests/test_register.py b/tests/test_register.py index 966997a..e7ff664 100644 --- a/tests/test_register.py +++ b/tests/test_register.py @@ -74,9 +74,23 @@ def test_authorization_uri_joins_scopes_with_spaces() -> None: def test_authorization_uri_uses_the_connections_scope_parameter() -> None: - slack = CATALOG.get("slack", include_retired=True) - params = query(authorization_uri(slack, "c")) - assert "user_scope" in params + """Slack's app pair reads scope as the app's own permissions and takes the + user's under user_scope; a connection declares which the service reads.""" + 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"}, + "scope_parameter": "user_scope", + }, + } + ) + params = query(authorization_uri(entry, "c")) + assert params["user_scope"] == ["read"] assert "scope" not in params diff --git a/tests/test_resolve.py b/tests/test_resolve.py index 521afd5..154182c 100644 --- a/tests/test_resolve.py +++ b/tests/test_resolve.py @@ -77,7 +77,6 @@ def test_resolved_connections_carry_the_overrides_and_every_known_prefix( assert "google" in connections, ( "other services' prefixes are needed for elimination" ) - assert connections["slack"]["retired"] def test_a_declared_token_format_travels_to_the_runtime( diff --git a/tests/test_validate.py b/tests/test_validate.py index 80e3f3b..e474772 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -67,8 +67,18 @@ def test_unknown_connection_is_reported(project: ProjectBuilder) -> None: def test_retired_connection_is_reported_with_the_reason( project: ProjectBuilder, ) -> None: - project.write_agent("mail-triage", {"connections": ["slack"]}) - assert any("connector" in p for p in problems(project)) + project.write_project( + { + "version": 1, + "project": "example-project", + "location": "us-central1", + "connections": { + "old-api": {**INTERNAL_API, "retired": "Declare internal-api instead."} + }, + } + ) + project.write_agent("mail-triage", {"connections": ["old-api"]}) + assert any("Declare internal-api instead" in p for p in problems(project)) def test_duplicate_connection_is_reported(project: ProjectBuilder) -> None: