fix(web-app-n8n): provision the MCP surfaces against the n8n 2 API

n8n 2 pages GET /rest/api-keys as {data: {items, counts, totals, owners}},
ten keys at a time, and matches `label` as a substring, so walking the body
raised "'str' object has no attribute 'get'". It also enforces API key
scopes, so activating the MCP server workflow needs `workflow:activate`.
Both scripts now read every list and create response through one
normalisation that unwraps the page, look the managed key up by label, and
name an unexpected shape by its keys only. The normalisation is taken over
from a392e2dd49 on feature/store-i18n.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

roles/web-app-n8n/files/python/provision/client.py

@@ -25,6 +25,7 @@ import os
 import re
 import sys
 import urllib.error
+import urllib.parse
 import urllib.request
 
 BASE = os.environ.get("N8N_BASE", "").rstrip("/")
@@ -90,6 +91,52 @@ def call(path, method="GET", payload=None, api_key=None):
         return error.code, error.read().decode(errors="replace")
 
 
+def shape(value):
+    """Describe a value by its type and, for a mapping, its keys, never its content.
+
+    Args:
+        value: decoded response fragment.
+    """
+    if isinstance(value, dict):
+        return f"a mapping with keys {sorted(map(str, value))}"
+    if isinstance(value, list):
+        return f"a list of {sorted({type(item).__name__ for item in value})}"
+    return f"a {type(value).__name__}"
+
+
+def records(body, what):
+    """Return the records of a list response.
+
+    Args:
+        body: decoded body: ``{"data": [...]}``, a bare list, or the paged
+            ``{"data": {"items": [...], ...}}`` of n8n 2.
+        what: noun naming the collection, for the failure message.
+    """
+    listed = body.get("data", body) if isinstance(body, dict) else body
+    if isinstance(listed, dict) and "items" in listed:
+        listed = listed["items"]
+    if listed is None:
+        return []
+    if not isinstance(listed, list) or not all(
+        isinstance(item, dict) for item in listed
+    ):
+        sys.exit(f"FAILED listing {what}: expected records, got {shape(listed)}")
+    return listed
+
+
+def record(body, what):
+    """Return the single record of a create response.
+
+    Args:
+        body: decoded body, either ``{"data": {...}}`` or a bare mapping.
+        what: noun naming the record, for the failure message.
+    """
+    created = body.get("data", body) if isinstance(body, dict) else body
+    if not isinstance(created, dict):
+        sys.exit(f"FAILED creating {what}: expected a record, got {shape(created)}")
+    return created
+
+
 def login():
     status, body = call(
         f"{REST}/login",
@@ -106,14 +153,18 @@ def api_key():
     n8n returns the usable secret only once, as ``rawApiKey``, and a listed key
     is redacted, so the key cannot be carried between runs. It is minted here
     and revoked before the run ends; an entry left behind under the managed name
-    belongs to a run that died and is deleted rather than guessed at.
+    belongs to a run that died and is deleted rather than guessed at. n8n pages
+    the listing at ten keys and matches ``label`` as a substring, so the query
+    narrows to the managed name and the exact match happens here.
     """
-    status, body = call(f"{REST}/api-keys")
+    query = urllib.parse.urlencode({"label": KEY_NAME})
+    status, body = call(f"{REST}/api-keys?{query}")
     if status != 200:
         sys.exit(f"FAILED listing api keys: {status} {body}")
 
-    existing = (body or {}).get("data") if isinstance(body, dict) else body
-    matches = [key for key in existing or [] if key.get("label") == KEY_NAME]
+    matches = [
+        key for key in records(body, "api keys") if key.get("label") == KEY_NAME
+    ]
     if len(matches) > 1:
         sys.exit(f"FAILED: {len(matches)} api keys named {KEY_NAME}")
     for key in matches:
@@ -136,7 +187,7 @@ def api_key():
     )
     if status not in (200, 201):
         sys.exit(f"FAILED creating api key {KEY_NAME}: {status} {body}")
-    created = (body or {}).get("data") if isinstance(body, dict) else body
+    created = record(body, f"api key {KEY_NAME}")
     return str(created["rawApiKey"]), str(created["id"])
 
 
@@ -174,8 +225,7 @@ def listed_credentials():
     status, body = call(f"{REST}/credentials")
     if status != 200:
         sys.exit(f"FAILED listing credentials: {status} {body}")
-    existing = (body or {}).get("data") if isinstance(body, dict) else body
-    return list(existing or [])
+    return records(body, "credentials")
 
 
 def credential(provider):
@@ -198,7 +248,7 @@ def credential(provider):
         status, body = call(f"{REST}/credentials", method="POST", payload=payload)
         if status not in (200, 201):
             sys.exit(f"FAILED creating {name}: {status} {body}")
-        created = (body or {}).get("data") if isinstance(body, dict) else body
+        created = record(body, name)
         return {"id": str(created["id"]), "name": name}, True
 
     status, body = call(
@@ -317,8 +367,9 @@ def managed_workflow(key):
     status, body = call(f"{PUBLIC}/workflows", api_key=key)
     if status != 200:
         sys.exit(f"FAILED listing workflows: {status} {body}")
-    existing = (body or {}).get("data") if isinstance(body, dict) else body
-    matches = [flow for flow in existing or [] if flow.get("name") == WORKFLOW_NAME]
+    matches = [
+        flow for flow in records(body, "workflows") if flow.get("name") == WORKFLOW_NAME
+    ]
     if len(matches) > 1:
         sys.exit(f"FAILED: {len(matches)} workflows named {WORKFLOW_NAME}")
     return matches[0] if matches else None

roles/web-app-n8n/files/python/provision/mcp.py

@@ -19,6 +19,7 @@ import json
 import os
 import sys
 import urllib.error
+import urllib.parse
 import urllib.request
 
 BASE = os.environ.get("N8N_BASE", "").rstrip("/")
@@ -95,6 +96,52 @@ def call(path, method="GET", payload=None, api_key=None):
         return error.code, error.read().decode(errors="replace")
 
 
+def shape(value):
+    """Describe a value by its type and, for a mapping, its keys, never its content.
+
+    Args:
+        value: decoded response fragment.
+    """
+    if isinstance(value, dict):
+        return f"a mapping with keys {sorted(map(str, value))}"
+    if isinstance(value, list):
+        return f"a list of {sorted({type(item).__name__ for item in value})}"
+    return f"a {type(value).__name__}"
+
+
+def records(body, what):
+    """Return the records of a list response.
+
+    Args:
+        body: decoded body: ``{"data": [...]}``, a bare list, or the paged
+            ``{"data": {"items": [...], ...}}`` of n8n 2.
+        what: noun naming the collection, for the failure message.
+    """
+    listed = body.get("data", body) if isinstance(body, dict) else body
+    if isinstance(listed, dict) and "items" in listed:
+        listed = listed["items"]
+    if listed is None:
+        return []
+    if not isinstance(listed, list) or not all(
+        isinstance(item, dict) for item in listed
+    ):
+        sys.exit(f"FAILED listing {what}: expected records, got {shape(listed)}")
+    return listed
+
+
+def record(body, what):
+    """Return the single record of a create response.
+
+    Args:
+        body: decoded body, either ``{"data": {...}}`` or a bare mapping.
+        what: noun naming the record, for the failure message.
+    """
+    created = body.get("data", body) if isinstance(body, dict) else body
+    if not isinstance(created, dict):
+        sys.exit(f"FAILED creating {what}: expected a record, got {shape(created)}")
+    return created
+
+
 def login():
     status, body = call(
         f"{REST}/login",
@@ -111,14 +158,18 @@ def api_key():
     n8n returns the usable secret only once, as ``rawApiKey``, and a listed key
     is redacted, so the key cannot be carried between runs. It is minted here
     and revoked before the run ends; an entry left behind under the managed name
-    belongs to a run that died and is deleted rather than guessed at.
+    belongs to a run that died and is deleted rather than guessed at. n8n pages
+    the listing at ten keys and matches ``label`` as a substring, so the query
+    narrows to the managed name and the exact match happens here.
     """
-    status, body = call(f"{REST}/api-keys")
+    query = urllib.parse.urlencode({"label": KEY_NAME})
+    status, body = call(f"{REST}/api-keys?{query}")
     if status != 200:
         sys.exit(f"FAILED listing api keys: {status} {body}")
 
-    existing = (body or {}).get("data") if isinstance(body, dict) else body
-    matches = [key for key in existing or [] if key.get("label") == KEY_NAME]
+    matches = [
+        key for key in records(body, "api keys") if key.get("label") == KEY_NAME
+    ]
     if len(matches) > 1:
         sys.exit(f"FAILED: {len(matches)} api keys named {KEY_NAME}")
     for key in matches:
@@ -134,13 +185,14 @@ def api_key():
                 "workflow:read",
                 "workflow:update",
                 "workflow:list",
+                "workflow:activate",
             ],
             "expiresAt": None,
         },
     )
     if status not in (200, 201):
         sys.exit(f"FAILED creating api key {KEY_NAME}: {status} {body}")
-    created = (body or {}).get("data") if isinstance(body, dict) else body
+    created = record(body, f"api key {KEY_NAME}")
     return str(created["rawApiKey"]), str(created["id"])
 
 
@@ -166,8 +218,11 @@ def bearer_credential():
     if status != 200:
         sys.exit(f"FAILED listing credentials: {status} {body}")
 
-    existing = (body or {}).get("data") if isinstance(body, dict) else body
-    matches = [item for item in existing or [] if item.get("name") == CREDENTIAL_NAME]
+    matches = [
+        item
+        for item in records(body, "credentials")
+        if item.get("name") == CREDENTIAL_NAME
+    ]
     if len(matches) > 1:
         sys.exit(f"FAILED: {len(matches)} credentials named {CREDENTIAL_NAME}")
 
@@ -180,7 +235,7 @@ def bearer_credential():
         status, body = call(f"{REST}/credentials", method="POST", payload=payload)
         if status not in (200, 201):
             sys.exit(f"FAILED creating {CREDENTIAL_NAME}: {status} {body}")
-        created = (body or {}).get("data") if isinstance(body, dict) else body
+        created = record(body, CREDENTIAL_NAME)
         return {"id": str(created["id"]), "name": CREDENTIAL_NAME}, True
 
     status, body = call(
@@ -268,8 +323,9 @@ def upsert_workflow(key, credential):
     if status != 200:
         sys.exit(f"FAILED listing workflows: {status} {body}")
 
-    existing = (body or {}).get("data") if isinstance(body, dict) else body
-    matches = [flow for flow in existing or [] if flow.get("name") == WORKFLOW_NAME]
+    matches = [
+        flow for flow in records(body, "workflows") if flow.get("name") == WORKFLOW_NAME
+    ]
     if len(matches) > 1:
         sys.exit(f"FAILED: {len(matches)} workflows named {WORKFLOW_NAME}")
 

tests/unit/python/roles/web-app-n8n/files/provision/test_client.py

@@ -8,10 +8,12 @@ import io
 import json
 import unittest
 from unittest.mock import patch
+from urllib.parse import parse_qs, urlsplit
 
 from . import PROJECT_ROOT
 
 SCRIPT_PATH = PROJECT_ROOT / "roles/web-app-n8n/files/python/provision/client.py"
+PAGE_SIZE = 10
 
 PROVIDER = {
     "id": "web-app-gitea",
@@ -78,8 +80,10 @@ class FakeApi:
             self.deleted_credentials.append(dead)
             self.credentials = [c for c in self.credentials if c["id"] != dead]
             return 200, {"success": True}
-        if path == "/rest/api-keys" and method == "GET":
-            return 200, {"data": [dict(key) for key in self.keys]}
+        if path.startswith("/rest/api-keys?") and method == "GET":
+            label = parse_qs(urlsplit(path).query)["label"][0].lower()
+            items = [dict(key) for key in self.keys if label in key["label"].lower()]
+            return 200, {"data": {"items": items[:PAGE_SIZE]}}
         if path == "/rest/api-keys" and method == "POST":
             self.keys = [{"id": "k1", "label": payload["label"]}]
             return 200, {"data": {"id": "k1", "rawApiKey": "n8n-raw-key"}}
@@ -179,6 +183,28 @@ class TestProvisionClient(unittest.TestCase):
             module.main()
         self.assertEqual(["k1"], api.deleted_keys)
 
+    def test_a_stale_managed_key_behind_a_full_page_is_still_replaced(self) -> None:
+        module = load_script()
+        humans = [{"id": f"h{n}", "label": f"human {n}"} for n in range(PAGE_SIZE)]
+        api = FakeApi(keys=[*humans, {"id": "old", "label": "infinito:mcp"}])
+        with patch.object(module, "call", api):
+            module.main()
+        self.assertEqual(["old", "k1"], api.deleted_keys)
+
+    def test_an_unknown_listing_shape_names_its_keys_but_never_a_value(self) -> None:
+        module = load_script()
+        api = FakeApi()
+
+        def odd(path, method="GET", payload=None, api_key=None):
+            if path == "/rest/credentials" and method == "GET":
+                return 200, {"data": {"token": "t0ps3cret"}}
+            return api(path, method, payload, api_key)
+
+        with patch.object(module, "call", odd), self.assertRaises(SystemExit) as exit_:
+            module.main()
+        self.assertIn("['token']", str(exit_.exception))
+        self.assertNotIn("t0ps3cret", str(exit_.exception))
+
     def test_a_rotated_provider_token_replaces_the_stored_one(self) -> None:
         module = load_script()
         api = FakeApi(credentials=[dict(REFERENCE)])

tests/unit/python/roles/web-app-n8n/files/provision/test_mcp.py

@@ -8,10 +8,12 @@ import io
 import json
 import unittest
 from unittest.mock import patch
+from urllib.parse import parse_qs, urlsplit
 
 from . import PROJECT_ROOT
 
 SCRIPT_PATH = PROJECT_ROOT / "roles/web-app-n8n/files/python/provision/mcp.py"
+PAGE_SIZE = 10
 
 ENV = {
     "N8N_BASE": "http://n8n:5678",
@@ -51,6 +53,7 @@ class FakeApi:
         self.deleted_keys = []
         self.logins = 0
         self.minted = 0
+        self.scopes = set()
 
     def __call__(self, path, method="GET", payload=None, api_key=None):
         if path == "/rest/login":
@@ -72,10 +75,13 @@ class FakeApi:
             self.deleted_credentials.append(dead)
             self.credentials = [c for c in self.credentials if c["id"] != dead]
             return 200, {"success": True}
-        if path == "/rest/api-keys" and method == "GET":
-            return 200, {"data": [dict(key) for key in self.keys]}
+        if path.startswith("/rest/api-keys?") and method == "GET":
+            label = parse_qs(urlsplit(path).query)["label"][0].lower()
+            items = [dict(key) for key in self.keys if label in key["label"].lower()]
+            return 200, {"data": {"items": items[:PAGE_SIZE]}}
         if path == "/rest/api-keys" and method == "POST":
             self.keys = [{"id": "k1", "label": payload["label"]}]
+            self.scopes = set(payload["scopes"])
             return 200, {"data": {"id": "k1", "rawApiKey": "n8n-raw-key"}}
         if path.startswith("/rest/api-keys/") and method == "DELETE":
             self.deleted_keys.append(path.rsplit("/", 1)[-1])
@@ -89,6 +95,8 @@ class FakeApi:
             self.updated_workflows.append(payload)
             return 200, payload
         if path.endswith("/activate") and method == "POST":
+            if "workflow:activate" not in self.scopes:
+                return 403, '{"message":"Forbidden"}'
             self.activated_workflows.append(path.split("/")[-2])
             return 200, {"active": True}
         if path.startswith("/api/v1/workflows/") and method == "DELETE":
@@ -184,6 +192,48 @@ class TestProvisionMcp(unittest.TestCase):
             module.main()
         self.assertEqual(["old", "k1"], api.deleted_keys)
 
+    def test_a_stale_managed_key_behind_a_full_page_is_still_replaced(self) -> None:
+        module = load_script()
+        humans = [{"id": f"h{n}", "label": f"human {n}"} for n in range(PAGE_SIZE)]
+        api = FakeApi(keys=[*humans, {"id": "old", "label": "infinito:mcp"}])
+        with patch.object(module, "call", api):
+            module.main()
+        self.assertEqual(["old", "k1"], api.deleted_keys)
+
+    def test_an_unknown_listing_shape_names_its_keys_but_never_a_value(self) -> None:
+        module = load_script()
+        api = FakeApi()
+
+        def odd(path, method="GET", payload=None, api_key=None):
+            if path.startswith("/rest/api-keys?"):
+                return 200, {"data": {"secret": "t0ps3cret"}}
+            return api(path, method, payload, api_key)
+
+        with patch.object(module, "call", odd), self.assertRaises(SystemExit) as exit_:
+            module.main()
+        self.assertIn("['secret']", str(exit_.exception))
+        self.assertNotIn("t0ps3cret", str(exit_.exception))
+
+    def test_an_unpaged_listing_of_n8n_1_still_finds_the_stale_key(self) -> None:
+        module = load_script()
+        api = FakeApi()
+
+        def unpaged(path, method="GET", payload=None, api_key=None):
+            if path.startswith("/rest/api-keys?"):
+                return 200, {"data": [{"id": "old", "label": "infinito:mcp"}]}
+            return api(path, method, payload, api_key)
+
+        with patch.object(module, "call", unpaged):
+            module.main()
+        self.assertEqual(["old", "k1"], api.deleted_keys)
+
+    def test_a_key_that_only_contains_the_managed_name_is_left_alone(self) -> None:
+        module = load_script()
+        api = FakeApi(keys=[{"id": "near", "label": "infinito:mcp-legacy"}])
+        with patch.object(module, "call", api):
+            module.main()
+        self.assertEqual(["k1"], api.deleted_keys)
+
     def test_the_run_leaves_no_api_key_behind(self) -> None:
         module = load_script()
         api = FakeApi()