diff --git a/ainative/lifecycle/release_providers.py b/ainative/lifecycle/release_providers.py index e72d890..3c0e08b 100644 --- a/ainative/lifecycle/release_providers.py +++ b/ainative/lifecycle/release_providers.py @@ -360,7 +360,7 @@ def _project_url(self) -> str: def _get_json(self, url: str) -> object: payload = transportlib.get(url, limit=MAX_MANIFEST_BYTES, endpoint=self.endpoint, kind=transportlib.METADATA, - accept=transportlib.ACCEPT_GITHUB_JSON, + accept=transportlib.ACCEPT_JSON, token=self._token()) try: return json.loads(payload.decode("utf-8")) diff --git a/ainative/lifecycle/release_source.py b/ainative/lifecycle/release_source.py index 7b3a4ed..5441787 100644 --- a/ainative/lifecycle/release_source.py +++ b/ainative/lifecycle/release_source.py @@ -39,7 +39,11 @@ RELEASE_URL_ENV = "AINATIVE_UPDATE_URL" CONFIG_RELATIVE = Path(".ai-native") / "release-providers.json" -RESERVED_PROVIDERS = ("github", "gitlab", "local") +# `release-api` is a historical alias of the built-in GitHub provider and +# stays accepted in the environment selector; it is reserved so a machine +# configuration can never declare a provider that the alias would silently +# shadow (#178). +RESERVED_PROVIDERS = ("github", "gitlab", "local", "release-api") CONFIG_SCHEMA_VERSION = 1 GITLAB_API_BASE_URL = "https://gitlab.com/api/v4" @@ -173,7 +177,8 @@ def _gitlab_source(entry: object) -> ReleaseSource: api_version=str(entry.get("api_version") or "v4"), credential_source=str(entry.get("credential_source") or f"environment:{GITLAB_TOKEN_ENV}"), - auth_header="PRIVATE-TOKEN", auth_prefix="") + auth_header="PRIVATE-TOKEN", auth_prefix="", + api_version_header="") return ReleaseSource(kind=KIND_GITLAB, provider_name="gitlab", endpoint=endpoint, project_ref=project_ref.strip(), reason="gitlab provider (machine configuration)") diff --git a/ainative/lifecycle/transport.py b/ainative/lifecycle/transport.py index 30272b7..aacb1ca 100644 --- a/ainative/lifecycle/transport.py +++ b/ainative/lifecycle/transport.py @@ -42,6 +42,7 @@ USER_AGENT = "ainative-lifecycle" ACCEPT_GITHUB_JSON = "application/vnd.github+json" +ACCEPT_JSON = "application/json" ACCEPT_OCTET_STREAM = "application/octet-stream" GITHUB_PROVIDER = "github" @@ -70,6 +71,9 @@ class ReleaseProviderEndpointConfig: `auth_header` and `auth_prefix` exist because providers disagree on the shape of authentication: GitHub wants `Authorization: Bearer `, GitLab wants `PRIVATE-TOKEN: `. The default is GitHub's. + `api_version_header` is the header name that carries `api_version` — a + GitHub-only header (`X-GitHub-Api-Version`); a provider that has no such + header leaves it empty, so the transport never invents one. """ provider: str @@ -79,6 +83,7 @@ class ReleaseProviderEndpointConfig: credential_source: str = "" auth_header: str = "Authorization" auth_prefix: str = "Bearer" + api_version_header: str = "" GITHUB_ENDPOINT = ReleaseProviderEndpointConfig( @@ -86,7 +91,8 @@ class ReleaseProviderEndpointConfig: api_base_url=GITHUB_API_BASE_URL, auth_origin=GITHUB_API_BASE_URL, api_version=GITHUB_API_VERSION, - credential_source=ENVIRONMENT_CREDENTIAL_SOURCE, + credential_source="environment:GITHUB_TOKEN,GH_TOKEN", + api_version_header="X-GitHub-Api-Version", ) @@ -139,8 +145,8 @@ def _may_receive(endpoint: ReleaseProviderEndpointConfig, origin: str) -> bool: def _request_headers(endpoint: ReleaseProviderEndpointConfig, origin: str, *, accept: str, token: str) -> dict: headers = {"User-Agent": USER_AGENT, "Accept": accept} - if endpoint.api_version: - headers["X-GitHub-Api-Version"] = endpoint.api_version + if endpoint.api_version_header and endpoint.api_version: + headers[endpoint.api_version_header] = endpoint.api_version if token and _may_receive(endpoint, origin): value = f"{endpoint.auth_prefix} {token}".strip() if endpoint.auth_prefix else token headers[endpoint.auth_header] = value @@ -173,12 +179,33 @@ def _redirect_target(current: str, error: urllib.error.HTTPError) -> str: return target -def _http_refusal(error: urllib.error.HTTPError) -> LifecycleError: - # The status is the diagnosis; the message never echoes headers. - detail = ("the release source rate-limited this check; set GITHUB_TOKEN " - "or GH_TOKEN to raise the limit" - if error.code in (403, 429) - else f"the release source answered HTTP {error.code}") +def _rate_limit_hint(endpoint: ReleaseProviderEndpointConfig) -> str: + """The honest remedy for a rate limit, from the endpoint's own config. + + An environment credential source names the variables to set; an anonymous + endpoint gets no advice it cannot act on. Never a secret value. + """ + + source = endpoint.credential_source or "" + if not source.startswith("environment:"): + return "" + names = [part.strip() for part in source.split(":", 1)[1].split(",") if part.strip()] + if not names: + return "" + return "set " + " or ".join(names) + " to raise the limit" + + +def _http_refusal(error: urllib.error.HTTPError, + endpoint: ReleaseProviderEndpointConfig) -> LifecycleError: + # The status is the diagnosis; the message never echoes headers or itself + # suggests a credential the endpoint cannot use. + if error.code in (403, 429): + hint = _rate_limit_hint(endpoint) + detail = "the release source rate-limited this check" + if hint: + detail = f"{detail}; {hint}" + else: + detail = f"the release source answered HTTP {error.code}" return LifecycleError("UPDATE_CHECK_FAILED", detail) @@ -205,7 +232,7 @@ def get(url: str, *, limit: int, endpoint: ReleaseProviderEndpointConfig, payload = response.read(limit + 1) except urllib.error.HTTPError as error: if error.code not in _REDIRECT_CODES: - raise _http_refusal(error) from error + raise _http_refusal(error, endpoint) from error target = _redirect_target(current, error) if kind == METADATA and _origin_of(target) != metadata_origin: raise LifecycleError( @@ -232,7 +259,7 @@ def get(url: str, *, limit: int, endpoint: ReleaseProviderEndpointConfig, __all__ = [ "ReleaseProviderEndpointConfig", "GITHUB_ENDPOINT", "anonymous_endpoint", "get", "METADATA", "ARTIFACT", "MAX_REDIRECTS", "NETWORK_TIMEOUT_SECONDS", - "USER_AGENT", "ACCEPT_GITHUB_JSON", "ACCEPT_OCTET_STREAM", + "USER_AGENT", "ACCEPT_GITHUB_JSON", "ACCEPT_JSON", "ACCEPT_OCTET_STREAM", "GITHUB_PROVIDER", "GITHUB_API_BASE_URL", "GITHUB_API_VERSION", "ANONYMOUS_PROVIDER", "ENVIRONMENT_CREDENTIAL_SOURCE", ] diff --git a/tests/test_lifecycle_update_transport.py b/tests/test_lifecycle_update_transport.py index a597b06..fb385dd 100644 --- a/tests/test_lifecycle_update_transport.py +++ b/tests/test_lifecycle_update_transport.py @@ -230,6 +230,43 @@ def send(request): self.assertIn("rate-limited", raised.exception.message) self.assertNotIn("github_pat_secret_value", raised.exception.message) + def test_a_rate_limit_hint_names_the_endpoint_credentials_only(self): + gitlab = transportlib.ReleaseProviderEndpointConfig( + provider="gitlab", api_base_url="https://gitlab.com/api/v4", + auth_origin="https://gitlab.com", api_version="v4", + credential_source="environment:GITLAB_TOKEN", + auth_header="PRIVATE-TOKEN", auth_prefix="") + cases = ( + (transportlib.GITHUB_ENDPOINT, + "https://api.github.com/repos/x/y/releases/latest", + "GITHUB_TOKEN or GH_TOKEN", True), + (gitlab, "https://gitlab.com/api/v4/projects/x/releases", + "GITLAB_TOKEN", True), + (transportlib.anonymous_endpoint("https://mirror.example/api"), + "https://mirror.example/api/releases/latest", None, False), + ) + for endpoint, url, hint, expects_advice in cases: + with self.subTest(provider=endpoint.provider): + def send(request): + raise urllib.error.HTTPError(request.full_url, 403, "Forbidden", {}, None) + + with mock.patch.object(transportlib, "_send", send), \ + mock.patch.dict("os.environ", + {"GITLAB_TOKEN": "glpat-secret-value"}): + with self.assertRaises(LifecycleError) as raised: + transportlib.get(url, limit=100, endpoint=endpoint, + kind=transportlib.METADATA, + accept=transportlib.ACCEPT_JSON) + message = raised.exception.message + self.assertIn("rate-limited", message) + self.assertNotIn("glpat-secret-value", message) + self.assertNotIn("github_pat_secret_value", message) + if expects_advice: + self.assertIn(hint, message) + else: + self.assertNotIn("GITHUB_TOKEN", message) + self.assertNotIn("GITLAB_TOKEN", message) + # --- artifacts: the CDN hop is anonymous ------------------------------ def test_the_asset_api_redirect_to_the_cdn_strips_the_bearer(self): diff --git a/tests/test_release_providers.py b/tests/test_release_providers.py index 27fae13..eb4ade6 100644 --- a/tests/test_release_providers.py +++ b/tests/test_release_providers.py @@ -201,6 +201,21 @@ def test_fetching_before_enumerating_is_a_programming_refusal(self): version="2.5.0", identity="v2.5.0")) self.assertEqual(raised.exception.code, "UPDATE_CHECK_FAILED") + def test_github_requests_carry_the_api_version_header(self): + server = ScriptedServer({LIST_URL: json.dumps([release_document()]).encode("utf-8")}) + captured = [] + + def spy(request): + captured.append({key.lower(): value for key, value in request.headers.items()}) + return server.send(request) + + provider = providerslib.GitHubReleaseProvider(transportlib.GITHUB_ENDPOINT, + repository="o/r") + with mock.patch.object(transportlib, "_send", spy): + provider.enumerate(release_v3lib.ReleaseQuery("stable")) + self.assertEqual(captured[0]["x-github-api-version"], "2022-11-28") + self.assertEqual(captured[0]["accept"], "application/vnd.github+json") + def test_resolve_manifest_runs_the_whole_chain(self): manifest = anchored_manifest() server = ScriptedServer({ @@ -472,6 +487,21 @@ def test_the_whole_chain_runs_against_a_scripted_gitlab(self): self.assertEqual(candidate.version, "2.5.0") self.assertEqual(payload, b"bundle-bytes") + def test_gitlab_requests_carry_no_github_header(self): + server = self.server() + provider = self.provider(server) + captured = [] + + def spy(request): + captured.append({key.lower(): value for key, value in request.headers.items()}) + return server.send(request) + + with mock.patch.object(transportlib, "_send", spy): + provider.enumerate(release_v3lib.ReleaseQuery("stable")) + first = captured[0] + self.assertEqual(first["accept"], "application/json") + self.assertNotIn("x-github-api-version", first) + def test_an_object_storage_redirect_strips_the_gitlab_token(self): """Scenario J: the blob hop is anonymous, the API hop is not.""" diff --git a/tests/test_release_source.py b/tests/test_release_source.py index e91ff8c..0adc183 100644 --- a/tests/test_release_source.py +++ b/tests/test_release_source.py @@ -127,6 +127,15 @@ def test_a_reserved_name_cannot_be_redefined(self): sourcelib.resolve_release_source(environ={}, home=self.home) self.assertEqual(raised.exception.code, "RELEASE_CONFIG_INVALID") + def test_the_release_api_alias_cannot_be_redefined(self): + """A config must never declare a provider a built-in alias shadows (#178).""" + + self.write_config({"schema_version": 1, "providers": { + "release-api": {"api_base_url": "https://evil.example/api"}}}) + with self.assertRaises(LifecycleError) as raised: + sourcelib.resolve_release_source(environ={}, home=self.home) + self.assertEqual(raised.exception.code, "RELEASE_CONFIG_INVALID") + def test_the_gitlab_provider_requires_a_project_reference(self): with self.assertRaises(LifecycleError) as raised: sourcelib.resolve_release_source(