From ea3af6efd86d70c21fffd894316122d99666c149 Mon Sep 17 00:00:00 2001 From: Paulo Date: Wed, 30 Sep 2026 16:30:15 +0200 Subject: [PATCH] Give an entry for github.com the GitHub service under any name --- src/host_secrets/catalog.py | 7 ++++++- src/host_secrets/schemas.py | 20 ++++++++++++++++--- src/host_secrets/tests/test_api.py | 4 ++++ src/host_secrets/tests/test_catalog.py | 12 +++++++++++ src/host_secrets/tests/test_schemas.py | 10 ++++++++++ src/hosts/schemas.py | 9 +++++++++ src/providers/docker_sbx/secrets.py | 12 +++++------ .../docker_sbx/tests/test_secrets.py | 16 +++++++++++++++ src/secrets_exchange/tests/test_app.py | 18 +++++++++++++++++ 9 files changed, 98 insertions(+), 10 deletions(-) diff --git a/src/host_secrets/catalog.py b/src/host_secrets/catalog.py index a4435d0..4bbb4ad 100644 --- a/src/host_secrets/catalog.py +++ b/src/host_secrets/catalog.py @@ -39,9 +39,14 @@ class Service(NamedTuple): } +# git and gh need the whole service: its variable, and Basic at github.com. +# An entry for one of these hosts is that service under any name. +BUILT_IN_HOSTS: dict[str, Service] = {"github.com": CATALOG["github"]} + + def service(name: str, entry: dict[str, Any]) -> Service: if "host" in entry: - return Service( + return BUILT_IN_HOSTS.get(entry["host"]) or Service( entry["auth_variable"], (Upstream(entry["host"], entry["auth_header"], entry["auth_prefix"]),), ) diff --git a/src/host_secrets/schemas.py b/src/host_secrets/schemas.py index c4e8b9f..3e4bf0b 100644 --- a/src/host_secrets/schemas.py +++ b/src/host_secrets/schemas.py @@ -11,7 +11,7 @@ model_validator, ) -from host_secrets.catalog import BEARER_HEADER, BEARER_PREFIX, SERVICE_FIELDS +from host_secrets.catalog import BEARER_HEADER, BEARER_PREFIX, BUILT_IN_HOSTS, SERVICE_FIELDS HOST_PATTERN = ( r"^(?:[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?\.)*" @@ -68,17 +68,31 @@ class SecretEntry(BaseModel): value: SecretValue | None = None issuer: SecretIssuer | None = None + @field_validator("host") + @classmethod + def lower_case(cls, host: str | None) -> str | None: + # The proxy and the built-in hosts match a host exactly. + if host: + return host.lower() + return host + @model_validator(mode="after") def validate_shape(self) -> Self: if bool(self.value) == bool(self.issuer): raise ValueError("provide exactly one of value or issuer") - if SERVICE_FIELDS & self.model_fields_set and not (self.host and self.auth_variable): + custom = SERVICE_FIELDS & self.model_fields_set and self.host not in BUILT_IN_HOSTS + if custom and not (self.host and self.auth_variable): raise ValueError("a custom service needs host and auth_variable") return self def to_storage(self) -> dict[str, Any]: - entry = self.model_dump(include=set(SERVICE_FIELDS)) if self.host else {} + # A built-in host carries the whole service, so the entry keeps only the host. + entry: dict[str, Any] = {} + if self.host in BUILT_IN_HOSTS: + entry = {"host": self.host} + elif self.host: + entry = self.model_dump(include=set(SERVICE_FIELDS)) if self.value: entry["value"] = self.value.get_secret_value() elif self.issuer: diff --git a/src/host_secrets/tests/test_api.py b/src/host_secrets/tests/test_api.py index afd5408..34620d3 100644 --- a/src/host_secrets/tests/test_api.py +++ b/src/host_secrets/tests/test_api.py @@ -73,6 +73,10 @@ async def test_secrets_are_ciphertext_at_rest(client, monkeypatch) -> None: [ ({"acme": {"value": "one"}}, "unknown secret service"), ({"GitHub": {"value": "one"}}, "invalid secret service name"), + ( + {"github": {"value": "one"}, "reviewer": {"host": "github.com", "value": "one"}}, + "share the variable GH_TOKEN", + ), ( { "anthropic": { diff --git a/src/host_secrets/tests/test_catalog.py b/src/host_secrets/tests/test_catalog.py index 43a2c10..032ee5f 100644 --- a/src/host_secrets/tests/test_catalog.py +++ b/src/host_secrets/tests/test_catalog.py @@ -38,5 +38,17 @@ def test_a_custom_entry_names_one_host_with_its_own_header_shape() -> None: assert service("acme", entry).upstreams[0].credential("ak_live") == "ak_live" +def test_an_entry_for_github_com_is_the_github_service_under_any_name() -> None: + entry = { + "host": "github.com", + "auth_header": "Authorization", + "auth_prefix": "Bearer ", + "auth_variable": "GITHUB_REVIEWER", + "value": "ghs_x", + } + + assert service("github_reviewer", entry) is CATALOG["github"] + + def test_a_built_in_entry_resolves_through_the_catalog() -> None: assert service("github", {"value": "ghs_x"}) is CATALOG["github"] diff --git a/src/host_secrets/tests/test_schemas.py b/src/host_secrets/tests/test_schemas.py index 5919e91..7c44fa7 100644 --- a/src/host_secrets/tests/test_schemas.py +++ b/src/host_secrets/tests/test_schemas.py @@ -55,6 +55,16 @@ def test_custom_entry_stores_the_whole_service_with_bearer_defaults() -> None: } +def test_an_entry_for_a_built_in_host_keeps_only_the_host() -> None: + registration = SecretEntry.model_validate( + {"host": "GitHub.com", "auth_variable": "GITHUB_REVIEWER", "value": "static-secret"} + ) + bare = SecretEntry.model_validate({"host": "github.com", "value": "static-secret"}) + + assert registration.to_storage() == {"host": "github.com", "value": "static-secret"} + assert bare.to_storage() == registration.to_storage() + + def test_custom_entry_can_override_the_auth_shape() -> None: registration = SecretEntry.model_validate( { diff --git a/src/hosts/schemas.py b/src/hosts/schemas.py index 2bf7911..3f80ca8 100644 --- a/src/hosts/schemas.py +++ b/src/hosts/schemas.py @@ -78,11 +78,20 @@ def reject_blank(cls, value: str | None, info: ValidationInfo) -> str | None: def reject_unknown_secret_services( cls, secrets: dict[str, SecretEntry] ) -> dict[str, SecretEntry]: + variables: dict[str, str] = {} for name, registration in secrets.items(): if not re.fullmatch(SECRET_NAME_PATTERN, name): raise ValueError(f"invalid secret service name {name!r}") if not registration.host and name not in catalog.CATALOG: raise ValueError(f"unknown secret service {name!r}") + # Two entries that resolve to one service would overwrite each other's variable. + variable = catalog.service(name, registration.to_storage()).auth_variable + if variable in variables: + raise ValueError( + f"secret services {variables[variable]!r} and {name!r} share the " + f"variable {variable}" + ) + variables[variable] = name return secrets @field_validator("env") diff --git a/src/providers/docker_sbx/secrets.py b/src/providers/docker_sbx/secrets.py index 304226c..2cb53dc 100644 --- a/src/providers/docker_sbx/secrets.py +++ b/src/providers/docker_sbx/secrets.py @@ -12,9 +12,9 @@ from .api import SbxCLI from .exceptions import DockerSbxProviderError -# sbx's own secret for these covers git and gh, when the entry reaches the -# service itself. Every other entry is a custom secret on its hosts. -_NATIVE_SERVICES = {"github": CATALOG["github"]} +# sbx's own secret for these covers git and gh, under the name sbx gives it. +# Every other entry is a custom secret on its hosts. +_NATIVE_SERVICES = {CATALOG["github"]: "github"} class SbxInjection(SecretInjectionCapability): @@ -43,8 +43,8 @@ async def put_secret( path = self.write_value(vm, placeholder.service, value) command = f"cat {shlex.quote(str(path))}" try: - if _NATIVE_SERVICES.get(placeholder.service) == service: - await self.api.set_secret(placeholder.service, sandbox=vm, command=command) + if service in _NATIVE_SERVICES: + await self.api.set_secret(_NATIVE_SERVICES[service], sandbox=vm, command=command) else: await self.api.set_custom_secret( sandbox=vm, @@ -65,7 +65,7 @@ async def delete_secrets(self, *, vm: str) -> None: """sbx keeps a sandbox's secrets after the sandbox is removed, and answers a missing one with success, so this can run again.""" try: - for name in _NATIVE_SERVICES: + for name in _NATIVE_SERVICES.values(): await self.api.remove_secret(name, sandbox=vm) for placeholder in await self.api.custom_placeholders(sandbox=vm): await self.api.remove_custom_secret(sandbox=vm, placeholder=placeholder) diff --git a/src/providers/docker_sbx/tests/test_secrets.py b/src/providers/docker_sbx/tests/test_secrets.py index 8e36706..27f77f0 100644 --- a/src/providers/docker_sbx/tests/test_secrets.py +++ b/src/providers/docker_sbx/tests/test_secrets.py @@ -109,6 +109,22 @@ async def test_a_new_value_replaces_the_file_and_the_secret(tmp_path: Path) -> N assert api.set_custom_secret.await_count == 2 +async def test_the_github_service_under_another_name_is_sbx_own_github_secret( + tmp_path: Path, +) -> None: + api = _api_mock() + placeholder = Placeholder.mint(uuid.uuid4(), "github_reviewer") + + environment = await SbxInjection(api, tmp_path).put_secret( + vm="sb-one", service=CATALOG["github"], placeholder=placeholder, value="ghs_real" + ) + + path = tmp_path / "sb-one" / "github_reviewer" + api.set_secret.assert_awaited_once_with("github", sandbox="sb-one", command=f"cat {path}") + api.set_custom_secret.assert_not_awaited() + assert environment == {"GH_TOKEN": str(placeholder)} + + async def test_a_custom_entry_named_github_is_a_custom_secret_on_its_host(tmp_path: Path) -> None: api = _api_mock() placeholder = Placeholder.mint(uuid.uuid4(), "github") diff --git a/src/secrets_exchange/tests/test_app.py b/src/secrets_exchange/tests/test_app.py index 24d1cfe..8456cae 100644 --- a/src/secrets_exchange/tests/test_app.py +++ b/src/secrets_exchange/tests/test_app.py @@ -74,6 +74,24 @@ async def test_the_git_host_of_github_gets_basic_with_the_token_as_the_password( ) +async def test_an_entry_for_github_com_under_another_name_is_the_github_service(edge) -> None: + host_id = uuid.uuid4() + minted = Placeholder.mint(host_id, "github_reviewer") + entry = { + "host": "github.com", + "value": "ghs_real", + "placeholder_fingerprint": minted.fingerprint, + } + await _create_host(host_id, {"github_reviewer": entry}) + + at_git = await edge.get("/authorize", headers=_headers(str(minted), "github.com")) + at_api = await edge.get("/authorize", headers=_headers(str(minted), "api.github.com")) + + assert (at_git.status_code, at_api.status_code) == (200, 200) + assert at_git.headers["X-Upstream-Credential"].startswith("Basic ") + assert at_api.headers["X-Upstream-Credential"] == "Bearer ghs_real" + + async def test_a_custom_service_gets_its_own_header_shape(edge) -> None: host_id = uuid.uuid4() minted = Placeholder.mint(host_id, "acme")