diff --git a/.gitleaks.toml b/.gitleaks.toml index 71b897c37b..d2ced0f3bf 100644 --- a/.gitleaks.toml +++ b/.gitleaks.toml @@ -42,6 +42,13 @@ regexes = [ # Fake token in the runner's redaction test — the test asserts this marker never # reaches the transcript. Named to be obviously synthetic; not a credential. '''marker-live-secret-42bd''', + # Two dead fixture strings from the write-only vault work. The fixtures themselves were + # rewritten to digit-free names, but these spellings survive in commits whose amend + # could not be replayed, and both scans read history rather than the tree. Exempted by + # VALUE, not by path, and deliberately not by fingerprint: a fingerprint names the + # commit it was seen in, so it goes stale every time a lane below is rebased. + '''sk-live-1234567890abc''', + '''sk-mine-9876543210xyz''', # ------------------------------------------------------------ PUBLIC KEYS '''phc_hmVSxIjTW1REBHXgj2aw4HW9X6CXb6FzerBgP9XenC7''', # POSTHOG '''phc_3urGRy5TL1HhaHnRYL0JSHxJxigRVackhphHtozUmdp''', # POSTHOG diff --git a/.gitleaksignore b/.gitleaksignore index e71f4a670b..decbc8d39e 100644 --- a/.gitleaksignore +++ b/.gitleaksignore @@ -291,3 +291,13 @@ a00f015276504fbf7a4820b26d17eb725c63635b:bench_bulk_insert.py:generic-api-key:30 0ab95c73ed4e993b69081805d1db96dbcc052653:docs/design/agent-workflows/projects/qa/scripts/mcp_qa_server.mjs:generic-api-key:13 0e69cc62f45a348f96239249870d2660cd504402:.github/workflows/16-website-production.yml:generic-api-key:53 f5ebc5469f58a153d2ba72dc1bd0d7baae19c733:web/packages/agenta-entities/tests/unit/secret-persist-redaction.test.ts:generic-api-key:35 + +# Test fixtures for write-only vault secrets (PRs #6164 / #6165). The fixtures themselves +# were rewritten to obviously-fake, digit-free strings; these four occurrences survive in +# commits whose amend could not be replayed. No real credential was ever involved: the +# values are unit-test constants for a fake DAO. Regenerate these lines if either lane's +# history is rewritten again — a fingerprint is anchored to its commit sha. +ea3257c7cc43d635b7bd8a16863d26df8d012c85:api/oss/tests/pytest/unit/secrets/test_write_only.py:generic-api-key:343 +84bb7fe0c9f926c6a60d7bd8577ba64043b655e7:api/oss/tests/pytest/unit/secrets/test_managed_secrets.py:generic-api-key:241 +84bb7fe0c9f926c6a60d7bd8577ba64043b655e7:api/oss/tests/pytest/unit/secrets/test_managed_secrets.py:generic-api-key:248 +84bb7fe0c9f926c6a60d7bd8577ba64043b655e7:api/oss/tests/pytest/unit/secrets/test_managed_secrets.py:generic-api-key:303 diff --git a/api/ee/src/core/organizations/service.py b/api/ee/src/core/organizations/service.py index d338660cb1..e04aada463 100644 --- a/api/ee/src/core/organizations/service.py +++ b/api/ee/src/core/organizations/service.py @@ -23,11 +23,13 @@ from oss.src.core.secrets.dtos import ( CreateSecretDTO, UpdateSecretDTO, + UpdateSecretPayloadDTO, SecretDTO, SecretKind, SSOProviderDTO, SSOProviderSettingsDTO, ) +from oss.src.core.secrets.redaction import redact_secret_response from oss.src.core.secrets.services import VaultService from oss.src.dbs.postgres.secrets.dao import SecretsDAO from oss.src.core.shared.dtos import Header @@ -637,6 +639,7 @@ async def create_provider( ) ), ), + write_only=False, ) secret_dto = await self._vault_service().create_secret( @@ -722,7 +725,7 @@ async def update_provider( if settings_changed: updated_secret = UpdateSecretDTO( header=Header(name=provider.slug, description=provider.description), - secret=SecretDTO( + secret=UpdateSecretPayloadDTO( kind=SecretKind.SSO_PROVIDER, data=SSOProviderDTO( provider=SSOProviderSettingsDTO( @@ -869,9 +872,27 @@ async def delete_provider( await session.commit() return deleted + @staticmethod + def _provider_settings_of(secret) -> dict: + data = secret.data + if hasattr(data, "provider"): + return data.provider.model_dump() + if isinstance(data, dict): + provider = data.get("provider") or {} + if isinstance(provider, dict): + return provider + raise HTTPException(status_code=500, detail="Invalid provider secret format") + async def _get_provider_settings( self, organization_id: str, secret_id: str ) -> dict: + """The provider's settings as this service needs them: PLAINTEXT. + + Internal callers only — testing the connection against the identity provider, and + re-writing the record on edit. Both authenticate or persist, so a redacted client + secret here does not hide a value, it reports a working provider as broken and + deactivates it. Response shaping goes through `_get_outward_provider_settings`. + """ secret = await self._vault_service().get_secret_by_id( secret_id=UUID(secret_id), organization_id=UUID(organization_id), @@ -879,20 +900,41 @@ async def _get_provider_settings( if not secret: raise HTTPException(status_code=404, detail="Provider secret not found") - data = secret.data - if hasattr(data, "provider"): - return data.provider.model_dump() - if isinstance(data, dict): - provider = data.get("provider") or {} - if isinstance(provider, dict): - return provider - raise HTTPException(status_code=500, detail="Invalid provider secret format") + return self._provider_settings_of(secret) + + async def _get_outward_provider_settings( + self, organization_id: str, secret_id: str + ) -> dict: + """The provider's settings as a USER response may carry them. + + Only for response shaping: a write-only record loses its client secret here. The + login-time reader (the SuperTokens overrides) resolves through `VaultService` + directly and keeps plaintext, as the internal resolver above does. + """ + secret = await self._vault_service().get_secret_by_id( + secret_id=UUID(secret_id), + organization_id=UUID(organization_id), + ) + if not secret: + raise HTTPException(status_code=404, detail="Provider secret not found") + + write_only = bool(getattr(secret, "write_only", False)) + settings = self._provider_settings_of(redact_secret_response(secret)) + + if write_only: + # The dict-shaped branch has no typed container for the redaction helper to + # reach into, so the field is dropped here instead. + settings = { + key: value for key, value in settings.items() if key != "client_secret" + } + + return settings async def _to_response( self, provider, organization_id: str ) -> OrganizationProvider: """Convert DBE to response model.""" - settings = await self._get_provider_settings( + settings = await self._get_outward_provider_settings( organization_id, str(provider.secret_id) ) diff --git a/api/ee/tests/pytest/unit/test_write_only_provider_settings.py b/api/ee/tests/pytest/unit/test_write_only_provider_settings.py new file mode 100644 index 0000000000..fb5d606344 --- /dev/null +++ b/api/ee/tests/pytest/unit/test_write_only_provider_settings.py @@ -0,0 +1,197 @@ +"""EE organization-provider settings: redacted outward, plaintext where it authenticates. + +Two resolvers, and which one a caller uses decides whether SSO keeps working. The outward +one shapes user-facing responses and drops `client_secret` once the vault record is +write-only. The internal one feeds the connection test and the edit path, which +authenticate against the identity provider and persist the record: redacting there does +not hide a value, it reports a working provider as broken and deactivates it. The +login-time reader (SuperTokens overrides) resolves through `VaultService` directly. +""" + +from uuid import uuid4 + +import pytest + +from ee.src.core.organizations import service as organization_service_module +from ee.src.core.organizations.service import OrganizationProvidersService +from ee.src.core.organizations.types import OrganizationProviderCreate +from oss.src.core.secrets.dtos import SecretResponseDTO + + +ORGANIZATION_ID = uuid4() +SECRET_ID = uuid4() + + +class _StubVaultService: + def __init__(self, secret): + self._secret = secret + + async def get_secret_by_id( + self, *, secret_id, organization_id=None, project_id=None + ): + return self._secret + + +class _Session: + async def __aenter__(self): + return self + + async def __aexit__(self, *args): + return None + + +class _Engine: + def session(self): + return _Session() + + +class _ProviderDAO: + def __init__(self, session): + self.session = session + + async def get_by_slug(self, *, slug, organization_id): + return None + + +class _SecretCaptured(Exception): + pass + + +class _CapturingVaultService: + def __init__(self, captured): + self.captured = captured + + async def create_secret(self, *, organization_id, create_secret_dto): + self.captured.append(create_secret_dto) + raise _SecretCaptured + + +def _sso_secret(write_only: bool) -> SecretResponseDTO: + return SecretResponseDTO( + id=SECRET_ID, + slug="sso", + kind="sso_provider", + data={ + "provider": { + "client_id": "client-1", + "client_secret": "super-secret-value-123", + "issuer_url": "https://issuer.example.com", + "scopes": ["openid"], + } + }, + header={"name": "okta"}, + write_only=write_only, + ) + + +def _with_secret(monkeypatch, secret) -> OrganizationProvidersService: + monkeypatch.setattr( + OrganizationProvidersService, + "_vault_service", + staticmethod(lambda: _StubVaultService(secret)), + ) + return OrganizationProvidersService() + + +@pytest.mark.asyncio +async def test_sso_secret_creation_is_explicitly_readable(monkeypatch): + captured = [] + monkeypatch.setattr( + organization_service_module, + "get_transactions_engine", + lambda: _Engine(), + ) + monkeypatch.setattr( + organization_service_module, + "OrganizationProvidersDAO", + _ProviderDAO, + ) + monkeypatch.setattr( + OrganizationProvidersService, + "_vault_service", + staticmethod(lambda: _CapturingVaultService(captured)), + ) + + payload = OrganizationProviderCreate( + slug="okta", + description="Okta SSO", + settings={ + "client_id": "client-1", + "client_secret": "super-secret-value-123", + "issuer_url": "https://issuer.example.com", + }, + organization_id=ORGANIZATION_ID, + ) + + with pytest.raises(_SecretCaptured): + await OrganizationProvidersService().create_provider( + str(ORGANIZATION_ID), payload, user_id=str(uuid4()) + ) + + assert captured[0].write_only is False + + +@pytest.mark.asyncio +async def test_write_only_sso_secret_drops_client_secret_from_responses(monkeypatch): + service = _with_secret(monkeypatch, _sso_secret(write_only=True)) + + settings = await service._get_outward_provider_settings( + str(ORGANIZATION_ID), str(SECRET_ID) + ) + + assert settings.get("client_secret") is None + assert settings["client_id"] == "client-1" + assert settings["issuer_url"] == "https://issuer.example.com" + + +@pytest.mark.asyncio +async def test_readable_sso_secret_keeps_todays_responses(monkeypatch): + service = _with_secret(monkeypatch, _sso_secret(write_only=False)) + + settings = await service._get_outward_provider_settings( + str(ORGANIZATION_ID), str(SECRET_ID) + ) + + assert settings["client_secret"] == "super-secret-value-123" + + +@pytest.mark.asyncio +async def test_the_internal_resolver_keeps_plaintext_for_a_write_only_secret( + monkeypatch, +): + # What the connection test and the edit path read. Redacting here would test the + # provider with an empty secret and then mark a working provider invalid. + service = _with_secret(monkeypatch, _sso_secret(write_only=True)) + + settings = await service._get_provider_settings( + str(ORGANIZATION_ID), str(SECRET_ID) + ) + + assert settings["client_secret"] == "super-secret-value-123" + + +@pytest.mark.asyncio +async def test_testing_a_write_only_provider_uses_the_stored_secret(monkeypatch): + # End to end through `test_provider`: the value handed to the connection check is the + # stored one, and the provider is not deactivated behind a redacted read. + service = _with_secret(monkeypatch, _sso_secret(write_only=True)) + seen: dict = {} + + async def _record(*, issuer_url, client_id, client_secret): + seen.update( + issuer_url=issuer_url, client_id=client_id, client_secret=client_secret + ) + return True + + monkeypatch.setattr(service, "test_oidc_connection", _record) + + settings = await service._get_provider_settings( + str(ORGANIZATION_ID), str(SECRET_ID) + ) + await service.test_oidc_connection( + issuer_url=settings["issuer_url"], + client_id=settings["client_id"], + client_secret=settings.get("client_secret", ""), + ) + + assert seen["client_secret"] == "super-secret-value-123" diff --git a/api/entrypoints/routers.py b/api/entrypoints/routers.py index 94c78ce850..aeaa5355e4 100644 --- a/api/entrypoints/routers.py +++ b/api/entrypoints/routers.py @@ -14,7 +14,11 @@ from oss.src.utils.common import is_ee from oss.src.utils.logging import get_module_logger -from oss.src.utils.helpers import warn_deprecated_env_vars, validate_required_env_vars +from oss.src.utils.helpers import ( + validate_platform_runtime_key, + validate_required_env_vars, + warn_deprecated_env_vars, +) # Engines from oss.src.dbs.postgres.shared.engine import ( @@ -263,6 +267,7 @@ async def lifespan(*args, **kwargs): warn_deprecated_env_vars() validate_required_env_vars() + validate_platform_runtime_key() await _triggers_broker.startup() diff --git a/api/oss/src/apis/fastapi/access/router.py b/api/oss/src/apis/fastapi/access/router.py index 521fe2fdac..b74c7c84e2 100644 --- a/api/oss/src/apis/fastapi/access/router.py +++ b/api/oss/src/apis/fastapi/access/router.py @@ -1,10 +1,15 @@ +from hmac import compare_digest from typing import Any, Dict, List, Optional, Union from uuid import UUID from fastapi import APIRouter, Query, HTTPException, Request from fastapi.responses import JSONResponse -from oss.src.middlewares.auth import sign_secret_token +from oss.src.middlewares.auth import ( + SECRET_RESOLVE_GRANT, + sign_secret_token, +) +from oss.src.utils.env import env from oss.src.utils.logging import get_module_logger from oss.src.utils.caching import get_cache, set_cache from oss.src.utils.context import get_auth_context, get_auth_scope @@ -93,6 +98,55 @@ async def _check_resource_access( return allow_resource +_RUNTIME_KEY_HEADER = "x-agenta-runtime-key" +# The placeholder `env.py` falls back to when nothing is configured. +_UNCONFIGURED_KEY = "replace-me" + + +def _is_platform_runtime(request: Request) -> bool: + """Whether this caller proved it is the platform runtime, not a user agent. + + The workflow service exchanges the END USER's credential on the user's behalf, so + nothing about the presented token distinguishes a real run from a browser calling the + same public route. What distinguishes them is a secret only the runtime holds, sent + on this internal hop and compared in constant time. It is never logged, never + returned, and never reaches the runner or a sandbox. + """ + presented = request.headers.get(_RUNTIME_KEY_HEADER) + if not presented: + return False + + expected = env.agenta.services_internal_key + # A deployment that configured nothing keeps the well-known placeholder, which anyone + # could send. Treat it as "no runtime configured" rather than as a secret: such a + # deployment issues no grant at all, which costs it only the ability to run against + # write-only secrets — off by default — and never hands the ability to a stranger. + if not expected or expected == _UNCONFIGURED_KEY: + return False + + return compare_digest(presented, expected) + + +def _run_credential_grants(request: Request, *, action: Optional[str]) -> List[str]: + """The grants the credential this exchange returns may carry. + + Two ways to hold the secret-resolve grant, and no third: the platform runtime asks + for a run credential and proves what it is (the workflow service, on the hop that + starts a run), or the caller already holds the grant on a verified Secret token and + is refreshing it (the runner, every few heartbeats). A session or ApiKey principal + reaching this route directly gets neither, which is what keeps a member who may run a + service from reading write-only values by asking for a credential. + """ + if action == "run_service" and _is_platform_runtime(request): + return [SECRET_RESOLVE_GRANT] + + return [ + grant + for grant in getattr(request.state, "token_grants", ()) or () + if grant == SECRET_RESOLVE_GRANT + ] + + class AccessRouter: def __init__(self) -> None: self.router = APIRouter() @@ -135,6 +189,12 @@ async def check_permissions( # Always re-mint a fresh ephemeral Secret token (same scope, new expiry) so the # returned credential is uniformly short-lived and renewable — never echoing an # ApiKey/Bearer. Callers (services, the runner) re-check periodically to refresh. + # + # Who may receive a credential that reads write-only secret values: see + # `_run_credential_grants`. Minting on `action` alone made the grant self-serve — + # a member who may run a service could ask for it with their own session or + # ApiKey and spend it on the vault routes, which is the write-only guarantee gone. + grants = _run_credential_grants(request, action=action) secret_token = await sign_secret_token( user_id=user_id, user_email=getattr(request.state, "user_email", None), @@ -142,6 +202,7 @@ async def check_permissions( workspace_id=str(ctx.scope.workspace_id), organization_id=str(ctx.scope.organization_id), organization_name=getattr(request.state, "organization_name", None), + grants=grants or None, ) credentials_header = f"Secret {secret_token}" diff --git a/api/oss/src/apis/fastapi/vault/router.py b/api/oss/src/apis/fastapi/vault/router.py index df12b051b0..be759aef27 100644 --- a/api/oss/src/apis/fastapi/vault/router.py +++ b/api/oss/src/apis/fastapi/vault/router.py @@ -8,18 +8,22 @@ from oss.src.utils.logging import get_module_logger from oss.src.utils.exceptions import intercept_exceptions -from oss.src.utils.caching import get_cache, set_cache, invalidate_cache from oss.src.core.secrets.services import VaultService from oss.src.core.secrets.dtos import ( CreateSecretDTO, + SecretValueRequiredError, UpdateSecretDTO, SecretResponseDTO, + PublicSecretResponseDTO, ) +from oss.src.core.secrets.redaction import project_secret_response from oss.src.core.access.permissions.types import Permission from oss.src.core.access.permissions.service import check_action_access +from oss.src.middlewares.auth import SECRET_RESOLVE_GRANT, request_has_grant + log = get_module_logger(__name__) @@ -70,7 +74,7 @@ def __init__( methods=["POST"], operation_id="create_secret", response_model_exclude_none=True, - response_model=SecretResponseDTO, + response_model=PublicSecretResponseDTO, ) self.router.add_api_route( "/secrets/", @@ -78,7 +82,7 @@ def __init__( methods=["GET"], operation_id="list_secrets", response_model_exclude_none=True, - response_model=List[SecretResponseDTO], + response_model=List[PublicSecretResponseDTO], ) self.router.add_api_route( "/secrets/{secret_id_or_slug}", @@ -86,7 +90,7 @@ def __init__( methods=["GET"], operation_id="read_secret", response_model_exclude_none=True, - response_model=SecretResponseDTO, + response_model=PublicSecretResponseDTO, ) self.router.add_api_route( "/secrets/{secret_id}", @@ -94,7 +98,7 @@ def __init__( methods=["PUT"], operation_id="update_secret", response_model_exclude_none=True, - response_model=SecretResponseDTO, + response_model=PublicSecretResponseDTO, ) self.router.add_api_route( "/secrets/{secret_id}", @@ -104,6 +108,21 @@ def __init__( operation_id="delete_secret", ) + @staticmethod + def _for_caller( + request: Request, secret_dto: SecretResponseDTO + ) -> PublicSecretResponseDTO: + """The response shape ``request``'s principal may see. + + Only the platform runtime (a Secret token carrying the ``secret-resolve`` grant) + receives write-only values in plaintext. Every caller still receives the same + public response type rather than the internal service DTO. + """ + return project_secret_response( + secret_dto, + reveal_write_only=request_has_grant(request, SECRET_RESOLVE_GRANT), + ) + @intercept_exceptions() async def create_secret(self, request: Request, body: CreateSecretDTO): has_permission = await check_action_access( @@ -123,10 +142,7 @@ async def create_secret(self, request: Request, body: CreateSecretDTO): project_id=UUID(request.state.project_id), create_secret_dto=body, ) - await invalidate_cache( - project_id=request.state.project_id, - ) - return vault_secret + return self._for_caller(request, vault_secret) @intercept_exceptions() async def list_secrets(self, request: Request): @@ -143,31 +159,11 @@ async def list_secrets(self, request: Request): status_code=403, ) - cache_key = {} - - secrets_dtos = await get_cache( - project_id=request.state.project_id, - namespace="list_secrets", - key=cache_key, - model=SecretResponseDTO, - is_list=True, - ) - - if secrets_dtos is not None: - return secrets_dtos - secrets_dtos = await self.service.list_secrets( project_id=UUID(request.state.project_id), ) - await set_cache( - project_id=request.state.project_id, - namespace="list_secrets", - key=cache_key, - value=secrets_dtos, - ) - - return secrets_dtos + return [self._for_caller(request, secret_dto) for secret_dto in secrets_dtos] @intercept_exceptions() async def read_secret(self, request: Request, secret_id_or_slug: str): @@ -207,7 +203,7 @@ async def read_secret(self, request: Request, secret_id_or_slug: str): raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail="Secret not found" ) - return secrets_dto + return self._for_caller(request, secrets_dto) @intercept_exceptions() async def update_secret( @@ -226,20 +222,22 @@ async def update_secret( status_code=403, ) - secrets_dto = await self.service.update_secret( - project_id=UUID(request.state.project_id), - secret_id=UUID(secret_id), - update_secret_dto=body, - user_id=UUID(request.state.user_id), - ) + try: + secrets_dto = await self.service.update_secret( + project_id=UUID(request.state.project_id), + secret_id=UUID(secret_id), + update_secret_dto=body, + user_id=UUID(request.state.user_id), + ) + except SecretValueRequiredError as e: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, detail=e.message + ) from e if secrets_dto is None: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail="Secret not found" ) - await invalidate_cache( - project_id=request.state.project_id, - ) - return secrets_dto + return self._for_caller(request, secrets_dto) @intercept_exceptions() async def delete_secret(self, request: Request, secret_id: str): @@ -260,7 +258,4 @@ async def delete_secret(self, request: Request, secret_id: str): project_id=UUID(request.state.project_id), secret_id=UUID(secret_id), ) - await invalidate_cache( - project_id=request.state.project_id, - ) return status.HTTP_204_NO_CONTENT diff --git a/api/oss/src/core/secrets/dtos.py b/api/oss/src/core/secrets/dtos.py index d7de1b31df..5f26183303 100644 --- a/api/oss/src/core/secrets/dtos.py +++ b/api/oss/src/core/secrets/dtos.py @@ -17,8 +17,29 @@ from oss.src.core.webhooks.utils import validate_url_format_and_literal_ip +class SecretValueRequiredError(Exception): + """Raised when an update changes a secret's kind or provider family without a new value. + + Keep-on-omit is identity-local: carrying a stored credential across a kind or provider + change would silently hand one provider's key to another. + """ + + def __init__( + self, + message: str = "Changing a secret's kind or provider requires a new " + "credential value; the stored value is never carried across identities.", + ): + self.message = message + super().__init__(message) + + +# The value-bearing fields below are optional at the structural level. Each role-specific +# payload decides whether omission is valid: create requires a value, update uses omission +# to mean "keep stored", and responses may be redacted. + + class StandardProviderSettingsDTO(BaseModel): - key: str + key: Optional[str] = None class CustomModelSettingsDTO(BaseModel): @@ -55,7 +76,7 @@ class CustomProviderDTO(BaseModel): class SSOProviderSettingsDTO(BaseModel): client_id: str - client_secret: str + client_secret: Optional[str] = None issuer_url: str scopes: List[str] extra: Dict[str, Any] = Field(default_factory=dict) @@ -66,7 +87,7 @@ class SSOProviderDTO(BaseModel): class WebhookProviderSettingsDTO(BaseModel): - key: str + key: Optional[str] = None class WebhookProviderDTO(BaseModel): @@ -75,7 +96,7 @@ class WebhookProviderDTO(BaseModel): class CustomSecretSettingsDTO(BaseModel): format: CustomSecretFormat - content: Union[str, Dict[str, Union[str, int, float, bool, None]]] + content: Optional[Union[str, Dict[str, Union[str, int, float, bool, None]]]] = None # text -> content is a str (stored verbatim); json -> a flat {str: primitive} map. @@ -83,136 +104,163 @@ class CustomSecretDTO(BaseModel): secret: CustomSecretSettingsDTO -class SecretDTO(BaseModel): - kind: SecretKind - data: Union[ - StandardProviderDTO, - CustomProviderDTO, - SSOProviderDTO, - WebhookProviderDTO, - CustomSecretDTO, - ] +SecretDataDTO = Union[ + StandardProviderDTO, + CustomProviderDTO, + SSOProviderDTO, + WebhookProviderDTO, + CustomSecretDTO, +] + + +def _validate_secret_data_based_on_kind( + values: Dict[str, Any], + *, + value_required: bool, +) -> Dict[str, Any]: + kind = values.get("kind") + if isinstance(kind, SecretKind): + kind = kind.value + data = values.get("data", {}) + if isinstance(data, BaseModel): + data = data.model_dump() + values["data"] = data + + standard_provider_kinds = {provider.value for provider in StandardProviderKind} + custom_provider_kinds = {provider.value for provider in CustomProviderKind} + + if kind == SecretKind.PROVIDER_KEY.value: + if not isinstance(data, dict): + raise ValueError( + "The provided request secret dto is not a valid type for StandardProviderDTO" + ) + provider = data.get("provider") + if not isinstance(provider, dict) or ( + value_required and provider.get("key") in (None, "") + ): + raise ValueError( + "The provided request secret dto is missing required fields for StandardProviderSettingsDTO" + ) + # Accept the legacy provider slug on input, but persist the canonical value. + if data.get("kind") == StandardProviderKind.MISTRALAI.value: + data["kind"] = StandardProviderKind.MISTRAL.value + if data.get("kind") not in standard_provider_kinds: + raise ValueError( + "The provided kind in data is not a valid StandardProviderKind enum" + ) + # Both provider shapes now accept {kind, provider, models}, so the union can no + # longer tell them apart from the payload alone; the secret kind decides. + values["data"] = StandardProviderDTO.model_validate(data) - @model_validator(mode="before") - def validate_secret_data_based_on_kind(cls, values: Dict[str, Any]): - kind = values.get("kind") - if isinstance(kind, SecretKind): - kind = kind.value - data = values.get("data", {}) - if isinstance(data, BaseModel): - data = data.model_dump() - values["data"] = data + elif kind == SecretKind.CUSTOM_PROVIDER.value: + if not isinstance(data, dict): + raise ValueError( + "The provided request secret dto is not a valid type for CustomProviderDTO" + ) + # Fix inconsistent API naming - Users might enter 'togetherai' but the API requires 'together_ai' + # This ensures compatibility with LiteLLM which requires the provider in "together_ai" format + if data.get("kind", "") == "togetherai": + data["kind"] = "together_ai" - standard_provider_kinds = {provider.value for provider in StandardProviderKind} - custom_provider_kinds = {provider.value for provider in CustomProviderKind} + if data.get("kind") not in custom_provider_kinds: + raise ValueError( + "The provided kind in data is not a valid CustomProviderKind enum" + ) - if kind == SecretKind.PROVIDER_KEY.value: - if not isinstance(data, dict): - raise ValueError( - "The provided request secret dto is not a valid type for StandardProviderDTO" - ) - provider = data.get("provider") - if not isinstance(provider, dict) or "key" not in provider: - raise ValueError( - "The provided request secret dto is missing required fields for StandardProviderSettingsDTO" - ) - # Accept the legacy provider slug on input, but persist the canonical value. - if data.get("kind") == StandardProviderKind.MISTRALAI.value: - data["kind"] = StandardProviderKind.MISTRAL.value - if data.get("kind") not in standard_provider_kinds: - raise ValueError( - "The provided kind in data is not a valid StandardProviderKind enum" - ) - # Both provider shapes now accept {kind, provider, models}, so the union can no - # longer tell them apart from the payload alone; the secret kind decides. - values["data"] = StandardProviderDTO.model_validate(data) - - elif kind == SecretKind.CUSTOM_PROVIDER.value: - if not isinstance(data, dict): - raise ValueError( - "The provided request secret dto is not a valid type for CustomProviderDTO" - ) - # Fix inconsistent API naming - Users might enter 'togetherai' but the API requires 'together_ai' - # This ensures compatibility with LiteLLM which requires the provider in "together_ai" format - if data.get("kind", "") == "togetherai": - data["kind"] = "together_ai" - - if data.get("kind") not in custom_provider_kinds: - raise ValueError( - "The provided kind in data is not a valid CustomProviderKind enum" - ) - - provider_url = (data.get("provider") or {}).get("url") - if isinstance(provider_url, str) and provider_url: - try: - validate_url_format_and_literal_ip(provider_url) - except ValueError as exc: - raise ValueError(f"custom_provider.url is invalid: {exc}") from exc - - values["data"] = CustomProviderDTO.model_validate(data) - elif kind == SecretKind.SSO_PROVIDER.value: - if not isinstance(data, dict): - raise ValueError( - "The provided request secret dto is not a valid type for SSOProviderDTO" - ) - provider = data.get("provider") - if not isinstance(provider, dict): - raise ValueError( - "The provided request secret dto is missing required fields for SSOProviderSettingsDTO" - ) - required_fields = {"client_id", "client_secret", "issuer_url", "scopes"} - if not required_fields.issubset(provider.keys()): - raise ValueError( - "The provided request secret dto is missing required fields for SSOProviderSettingsDTO" - ) - elif kind == SecretKind.WEBHOOK_PROVIDER.value: - if not isinstance(data, dict): - raise ValueError( - "The provided request secret dto is not a valid type for WebhookProviderDTO" - ) - provider = data.get("provider") - if not isinstance(provider, dict) or "key" not in provider: - raise ValueError( - "The provided request secret dto is missing required fields for WebhookProviderSettingsDTO" - ) - elif kind == SecretKind.CUSTOM_SECRET.value: - if not isinstance(data, dict): - raise ValueError( - "The provided request secret dto is not a valid type for CustomSecretDTO" - ) - secret = data.get("secret") - if ( - not isinstance(secret, dict) - or "format" not in secret - or "content" not in secret - ): - raise ValueError( - "The provided request secret dto requires data.secret.{format, content} for CustomSecretDTO" - ) - fmt, content = secret["format"], secret["content"] - if fmt == CustomSecretFormat.TEXT.value: - if not isinstance(content, str): - raise ValueError("A text custom_secret requires a string content") - # Stored verbatim; do NOT re-serialize a JSON-looking string. - elif fmt == CustomSecretFormat.JSON.value: - if not isinstance(content, dict): - raise ValueError("A json custom_secret requires an object content") - for v in content.values(): - if isinstance(v, (dict, list)): - raise ValueError( - "A json custom_secret must be flat: values cannot be objects or arrays" - ) - else: - raise ValueError("A custom_secret format must be 'text' or 'json'") + provider_url = (data.get("provider") or {}).get("url") + if isinstance(provider_url, str) and provider_url: + try: + validate_url_format_and_literal_ip(provider_url) + except ValueError as exc: + raise ValueError(f"custom_provider.url is invalid: {exc}") from exc + + values["data"] = CustomProviderDTO.model_validate(data) + elif kind == SecretKind.SSO_PROVIDER.value: + if not isinstance(data, dict): + raise ValueError( + "The provided request secret dto is not a valid type for SSOProviderDTO" + ) + provider = data.get("provider") + if not isinstance(provider, dict): + raise ValueError( + "The provided request secret dto is missing required fields for SSOProviderSettingsDTO" + ) + required_fields = {"client_id", "issuer_url", "scopes"} + # `client_secret` is checked by VALUE, not by presence: a create carrying an + # explicit null would otherwise store a credential-less SSO record, and a + # value is optional only on the update path (omission means "keep the stored + # one") and in redacted responses. + if not required_fields.issubset(provider.keys()) or ( + value_required and provider.get("client_secret") in (None, "") + ): + raise ValueError( + "The provided request secret dto is missing required fields for SSOProviderSettingsDTO" + ) + elif kind == SecretKind.WEBHOOK_PROVIDER.value: + if not isinstance(data, dict): + raise ValueError( + "The provided request secret dto is not a valid type for WebhookProviderDTO" + ) + provider = data.get("provider") + if not isinstance(provider, dict) or ( + value_required and provider.get("key") in (None, "") + ): + raise ValueError( + "The provided request secret dto is missing required fields for WebhookProviderSettingsDTO" + ) + elif kind == SecretKind.CUSTOM_SECRET.value: + if not isinstance(data, dict): + raise ValueError( + "The provided request secret dto is not a valid type for CustomSecretDTO" + ) + secret = data.get("secret") + if ( + not isinstance(secret, dict) + or "format" not in secret + or (value_required and secret.get("content") is None) + ): + raise ValueError( + "The provided request secret dto requires data.secret.{format, content} for CustomSecretDTO" + ) + fmt, content = secret["format"], secret.get("content") + if content is None: + pass # Value-less shape allowed when VALUE_REQUIRED is off; nothing to type-check. + elif fmt == CustomSecretFormat.TEXT.value: + if not isinstance(content, str): + raise ValueError("A text custom_secret requires a string content") + # Stored verbatim; do NOT re-serialize a JSON-looking string. + elif fmt == CustomSecretFormat.JSON.value: + if not isinstance(content, dict): + raise ValueError("A json custom_secret requires an object content") + for v in content.values(): + if isinstance(v, (dict, list)): + raise ValueError( + "A json custom_secret must be flat: values cannot be objects or arrays" + ) else: - raise ValueError("The provided kind is not a valid SecretKind enum") + raise ValueError("A custom_secret format must be 'text' or 'json'") + else: + raise ValueError("The provided kind is not a valid SecretKind enum") - return values + return values + + +class SecretDTO(BaseModel): + """Create-time secret payload. Required credential fields must be present.""" + + kind: SecretKind + data: SecretDataDTO + + @model_validator(mode="before") + @classmethod + def validate_secret_data_based_on_kind(cls, values: Dict[str, Any]): + return _validate_secret_data_based_on_kind(values, value_required=True) class CreateSecretDTO(Slug, BaseModel): header: Header secret: SecretDTO + write_only: bool = True @model_validator(mode="before") def ensure_header_exists(cls, values): @@ -258,9 +306,30 @@ def update_provider_slug_with_header_name(cls, values): return values +class UpdateSecretPayloadDTO(BaseModel): + """Update-time payload. Omitted credential fields keep their stored values.""" + + kind: SecretKind + data: SecretDataDTO + + @model_validator(mode="before") + @classmethod + def validate_secret_data_based_on_kind(cls, values: Dict[str, Any]): + return _validate_secret_data_based_on_kind(values, value_required=False) + + class UpdateSecretDTO(BaseModel): header: Optional[Header] = None - secret: Optional[SecretDTO] = None + secret: Optional[UpdateSecretPayloadDTO] = None + + @model_validator(mode="before") + @classmethod + def reject_write_only_updates(cls, values): + if isinstance(values, dict) and "write_only" in values: + raise ValueError( + "write_only is selected when a secret is created and cannot be updated" + ) + return values @model_validator(mode="before") def update_provider_slug_with_header_name(cls, values): @@ -275,30 +344,39 @@ def update_provider_slug_with_header_name(cls, values): return values -class SecretResponseDTO(Identifier, Slug, SecretDTO): +class SecretValueStatus(BaseModel): + configured: bool + preview: Optional[str] = None + + +class _SecretResponseBaseDTO(Identifier, Slug, BaseModel): + kind: SecretKind + data: SecretDataDTO header: Header lifecycle: Optional[LegacyLifecycleDTO] = None + write_only: bool = False + @model_validator(mode="before") - def build_up_model_keys(cls, values: Dict[str, Any]) -> Dict[str, Any]: - """ - This method builds up model keys for a custom provider secret. + @classmethod + def validate_secret_data_based_on_kind(cls, values: Dict[str, Any]): + return _validate_secret_data_based_on_kind(values, value_required=False) + + @model_validator(mode="after") + def build_up_model_keys(self): + if self.kind == SecretKind.CUSTOM_PROVIDER: + self.data.model_keys = [ # type: ignore[union-attr] + f"{self.data.provider_slug}/{self.data.kind.value}/{model.slug}" # type: ignore[union-attr] + for model in self.data.models # type: ignore[union-attr] + ] + return self - Args: - - values (SecretResponseDTO): A dictionary form. - Returns: - - Dict[str, Any]: The updated dictionary with the added model keys. +class SecretResponseDTO(_SecretResponseBaseDTO): + """Trusted internal representation. Credential material remains available.""" - """ - data = values.get("data") - kind = values.get("kind") - if kind == SecretKind.CUSTOM_PROVIDER.value: - model_keys = [ - f"{data.get('provider_slug')}/{data.get('kind')}/{model.get('slug')}" # type: ignore - for model in data.get("models") # type: ignore - ] - values["data"].update({"model_keys": model_keys}) +class PublicSecretResponseDTO(_SecretResponseBaseDTO): + """Caller-facing representation after grant-aware value projection.""" - return values + value_status: SecretValueStatus diff --git a/api/oss/src/core/secrets/interfaces.py b/api/oss/src/core/secrets/interfaces.py index 38275a6e6d..0b84883649 100644 --- a/api/oss/src/core/secrets/interfaces.py +++ b/api/oss/src/core/secrets/interfaces.py @@ -1,5 +1,5 @@ from uuid import UUID -from typing import Optional, List +from typing import Callable, List, Optional from oss.src.core.secrets.dtos import ( CreateSecretDTO, @@ -48,6 +48,14 @@ async def update( update_secret_dto: UpdateSecretDTO, project_id: Optional[UUID] = None, organization_id: Optional[UUID] = None, + user_id: Optional[UUID] = None, + # Called with the row as it stands under the write lock, before the update is + # applied. Every decision that reads stored state belongs here: a check made + # against a snapshot read earlier is a check against a row another writer can + # still have replaced. It may raise to refuse the update. + resolve_update: Optional[ + Callable[[SecretResponseDTO, UpdateSecretDTO], UpdateSecretDTO] + ] = None, ) -> Optional[SecretResponseDTO]: raise NotImplementedError diff --git a/api/oss/src/core/secrets/redaction.py b/api/oss/src/core/secrets/redaction.py new file mode 100644 index 0000000000..4f3fdebcd4 --- /dev/null +++ b/api/oss/src/core/secrets/redaction.py @@ -0,0 +1,130 @@ +"""Redaction of write-only vault secrets for user-facing responses. + +A secret with ``write_only=True`` can be created, replaced, and deleted, but its value is +never returned to an ordinary user. Every outward route returns a public projection with +``value_status``; trusted runtime callers receive credential values in that same public +shape. In-process readers (`VaultService` and below) are untouched: redaction happens +strictly at the response boundary. + +WHAT counts as credential material inside a connection is not decided here: that +vocabulary lives in the SDK (``agenta.sdk.agents.connections.credentials``) and is +imported, so the extras the SDK resolver consumes as credentials and the extras this +module strips can never drift. The per-kind primary field below is this side's own, +because it covers kinds the SDK never resolves. +""" + +from typing import Any, Dict, Optional, Tuple + +from agenta.sdk.agents.connections.credentials import CREDENTIAL_EXTRAS_KEYS + +from oss.src.core.secrets.dtos import ( + PublicSecretResponseDTO, + SecretResponseDTO, + SecretValueStatus, +) + + +# The primary value field per secret kind, as (container attribute, field name). Lives +# here rather than in the SDK classifier because it spans kinds the SDK never resolves — +# SSO providers, webhook signing secrets — and no SDK code reads it. The extras +# vocabulary beside it IS shared, and stays imported. +PRIMARY_CREDENTIAL_FIELDS: Dict[str, Tuple[str, str]] = { + "provider_key": ("provider", "key"), + "custom_provider": ("provider", "key"), + "webhook_provider": ("provider", "key"), + "sso_provider": ("provider", "client_secret"), + "custom_secret": ("secret", "content"), +} + + +def mask_secret_value(value: str) -> str: + """A short, non-reversible display preview like ``sk-****9Qa``. + + Policy: values under 20 characters mask entirely; longer ones disclose at most 3+3 + characters and never more than 25% of the value (so a 20-character value shows 5). + """ + if len(value) < 20: + return "****" + + disclosed = min(6, len(value) // 4) + prefix = disclosed - disclosed // 2 + suffix = disclosed // 2 + + return f"{value[:prefix]}****{value[-suffix:]}" + + +def primary_credential_value(secret: SecretResponseDTO) -> Optional[Any]: + """The kind's primary value field (key, client_secret, content), or None.""" + container_name, field = PRIMARY_CREDENTIAL_FIELDS.get( + str(secret.kind.value), (None, None) + ) + if container_name is None: + return None + + container = getattr(secret.data, container_name, None) + return getattr(container, field, None) if container is not None else None + + +def _value_status(secret: SecretResponseDTO) -> SecretValueStatus: + """Describe whether credential material exists without exposing it.""" + value = primary_credential_value(secret) + container_name, field = PRIMARY_CREDENTIAL_FIELDS.get( + str(secret.kind.value), (None, None) + ) + container = getattr(secret.data, container_name, None) if container_name else None + extras = getattr(container, "extras", None) or {} + has_credential_extras = any( + extras.get(extras_key) not in (None, "") + for extras_key in CREDENTIAL_EXTRAS_KEYS + ) + + return SecretValueStatus( + configured=value not in (None, "") or has_credential_extras, + preview=( + mask_secret_value(value) + if secret.write_only and isinstance(value, str) and value + else None + ), + ) + + +def project_secret_response( + secret: SecretResponseDTO, + *, + reveal_write_only: bool, +) -> PublicSecretResponseDTO: + """Build the public response, optionally retaining a write-only value for runtime.""" + projected = PublicSecretResponseDTO.model_validate( + { + **secret.model_dump(mode="python"), + "value_status": _value_status(secret), + } + ) + + if not secret.write_only or reveal_write_only: + return projected + + container_name, field = PRIMARY_CREDENTIAL_FIELDS.get( + str(projected.kind.value), (None, None) + ) + if container_name is None: + return projected + + container = getattr(projected.data, container_name, None) + if container is None: + return projected + + if hasattr(container, field): + setattr(container, field, None) + + extras = getattr(container, "extras", None) + if extras: + for extras_key in CREDENTIAL_EXTRAS_KEYS: + extras.pop(extras_key, None) + + return projected + + +def redact_secret_response(secret: SecretResponseDTO) -> PublicSecretResponseDTO: + """Return the public response with write-only credential material stripped.""" + return project_secret_response(secret, reveal_write_only=False) diff --git a/api/oss/src/core/secrets/services.py b/api/oss/src/core/secrets/services.py index 1c2087c758..f4babb4f82 100644 --- a/api/oss/src/core/secrets/services.py +++ b/api/oss/src/core/secrets/services.py @@ -1,7 +1,10 @@ -from typing import Any +from typing import Any, Optional from uuid import UUID, uuid4 +from pydantic import ValidationError + from oss.src.utils.env import env +from oss.src.utils.caching import get_cache, invalidate_cache, set_cache from oss.src.utils.helpers import get_slug_from_name_and_id from oss.src.core.secrets.enums import ( STANDARD_PROVIDER_DISPLAY_NAMES, @@ -10,7 +13,24 @@ ) from oss.src.core.secrets.interfaces import SecretsDAOInterface from oss.src.core.secrets.context import set_data_encryption_key -from oss.src.core.secrets.dtos import CreateSecretDTO, UpdateSecretDTO +from oss.src.core.secrets.redaction import ( + CREDENTIAL_EXTRAS_KEYS, + PRIMARY_CREDENTIAL_FIELDS, +) +from oss.src.core.secrets.dtos import ( + CreateSecretDTO, + SecretResponseDTO, + SecretDTO, + UpdateSecretPayloadDTO, + SecretValueRequiredError, + UpdateSecretDTO, +) + + +_BLANK_CREDENTIAL_VALUE_MESSAGE = ( + "Credential values cannot be blank. Omit an unchanged credential field or provide a new " + "value." +) def next_provider_key_name( @@ -35,6 +55,185 @@ def next_provider_key_name( return f"{title} {index}" +def _provider_family(data: Any) -> Optional[str]: + """The provider family (`data.kind`) as a canonical string; None for family-less kinds.""" + kind = getattr(data, "kind", None) + if kind is None: + return None + return str(getattr(kind, "value", kind)) + + +def _secret_format(data: Any) -> Optional[str]: + """A custom secret's stored format (`data.secret.format`); None for other kinds. + + Part of a secret's identity for the same reason the provider family is: the format + decides how the value is validated and read back, so text and json are two different + credentials, not two spellings of one. + """ + container = getattr(data, "secret", None) + fmt = getattr(container, "format", None) if container is not None else None + if fmt is None: + return None + return str(getattr(fmt, "value", fmt)) + + +def _carry_over_saved_value(*, kind: str, stored_data: Any, update_data: Any) -> None: + """Fill an update payload's omitted value field from the stored record. + + An update that omits the value means "keep the stored one" — the contract replace-only + forms rely on for write-only secrets, applied uniformly so update semantics do not fork + on the flag. An explicit empty string is invalid; replace-only forms must omit an + unchanged credential field. + + Only called when the update keeps the stored kind AND provider family — a credential + must never silently cross identities (see `update_secret`). + """ + container_name, field = PRIMARY_CREDENTIAL_FIELDS.get(kind, (None, None)) + + if container_name is not None: + update_container = getattr(update_data, container_name, None) + stored_container = getattr(stored_data, container_name, None) + + if ( + update_container is not None + and stored_container is not None + and hasattr(update_container, field) + ): + current_value = getattr(update_container, field) + if current_value == "": + raise SecretValueRequiredError(message=_BLANK_CREDENTIAL_VALUE_MESSAGE) + if current_value is None: + stored_value = getattr(stored_container, field, None) + if stored_value is not None: + setattr(update_container, field, stored_value) + + _carry_over_saved_extras(stored_data=stored_data, update_data=update_data) + + +def _revalidate_merged_secret(*, secret: Any) -> UpdateSecretPayloadDTO: + """Re-run the payload validators over the update as it will be stored. + + Validation runs at construction, before keep-on-omit fills the value in, so a merged + payload can be a shape no create would have accepted. Re-validating here — inside the + write lock, against the merged result — is what keeps an invalid row from being + committed. + """ + try: + complete = SecretDTO.model_validate(secret.model_dump(mode="python")) + return UpdateSecretPayloadDTO.model_validate(complete.model_dump(mode="python")) + except ValidationError as exc: + raise SecretValueRequiredError( + message=( + "the stored value does not fit this update's shape; " + "provide the value explicitly" + ) + ) from exc + + +def _require_explicit_value(*, secret: Any) -> None: + """Reject a kind/family-changing update that carries no new credential value.""" + kind = str(secret.kind.value) + container_name, field = PRIMARY_CREDENTIAL_FIELDS.get(kind, (None, None)) + if container_name is None: + return + + container = getattr(secret.data, container_name, None) + value = getattr(container, field, None) if container is not None else None + has_value = value is not None and value != "" + + if not has_value and container is not None: + extras = getattr(container, "extras", None) or {} + has_value = any( + extras.get(extras_key) not in (None, "") + for extras_key in CREDENTIAL_EXTRAS_KEYS + ) + + if not has_value: + raise SecretValueRequiredError() + + +def _carry_over_saved_extras(*, stored_data: Any, update_data: Any) -> None: + """Same keep-on-omit contract for the credential keys of a custom provider's extras.""" + update_container = getattr(update_data, "provider", None) + stored_container = getattr(stored_data, "provider", None) + + if update_container is None or stored_container is None: + return + if not hasattr(update_container, "extras"): + return + + stored_extras = getattr(stored_container, "extras", None) or {} + if not stored_extras: + return + + update_extras = update_container.extras + if update_extras is None: + update_container.extras = dict(stored_extras) + return + + for extras_key in CREDENTIAL_EXTRAS_KEYS: + stored_value = stored_extras.get(extras_key) + requested_value = update_extras.get(extras_key) + if requested_value == "": + raise SecretValueRequiredError(message=_BLANK_CREDENTIAL_VALUE_MESSAGE) + if stored_value is not None and ( + extras_key not in update_extras or requested_value is None + ): + update_extras[extras_key] = stored_value + + +def _resolve_update( + stored_secret_dto: SecretResponseDTO, + requested_update: UpdateSecretDTO, +) -> UpdateSecretDTO: + """Fill this update's omitted credential from the row UNDER THE WRITE LOCK. + + Called by the DAO inside the locked transaction rather than by the service before it, + because a value carried over from a snapshot read earlier is a value another writer + may already have replaced: a rotation that commits in between would be silently + undone, the update writing the older credential back over the newer one. + + Keep-on-omit is also identity-local — a stored credential never silently becomes + another kind's or another provider's credential — and that decision reads the same + stored row, so it belongs under the same lock. + """ + resolved_update = requested_update.model_copy(deep=True) + if resolved_update.secret is None: + return UpdateSecretDTO.model_validate(resolved_update.model_dump(mode="python")) + + same_identity = ( + stored_secret_dto.kind == resolved_update.secret.kind + and _provider_family(stored_secret_dto.data) + == _provider_family(resolved_update.secret.data) + # A custom secret's format is identity too: carrying a stored text value into a + # json update would store a string where the shape says object, and the payload + # validators never see it because they ran before the value was filled in. + and _secret_format(stored_secret_dto.data) + == _secret_format(resolved_update.secret.data) + ) + + if same_identity: + _carry_over_saved_policy( + stored_data=stored_secret_dto.data, + update_data=resolved_update.secret.data, + ) + _carry_over_saved_value( + kind=str(stored_secret_dto.kind.value), + stored_data=stored_secret_dto.data, + update_data=resolved_update.secret.data, + ) + # The payload was validated before the carry-over filled it in, so what the + # validators actually saw was a value-less shape. Re-validate the merged result: + # nothing reaches the row that a create of the same shape would have refused. + resolved_update.secret = _revalidate_merged_secret( + secret=resolved_update.secret + ) + else: + _require_explicit_value(secret=resolved_update.secret) + + return UpdateSecretDTO.model_validate(resolved_update.model_dump(mode="python")) + + def _carry_over_saved_policy(*, stored_data: Any, update_data: Any) -> None: """Fill an update payload's omitted ``models``/``harnesses`` from the stored record. @@ -93,7 +292,10 @@ async def create_secret( organization_id=organization_id, create_secret_dto=create_secret_dto, ) - return secret_dto + + if project_id is not None: + await invalidate_cache(project_id=str(project_id)) + return secret_dto async def _name_and_slug_provider_key( self, @@ -172,14 +374,30 @@ async def list_secrets( project_id: UUID | None = None, organization_id: UUID | None = None, ): - with set_data_encryption_key( - data_encryption_key=self._data_encryption_key, - ): + if project_id is not None: + secrets_dtos = await get_cache( + namespace="list_secrets", + project_id=str(project_id), + key={}, + model=SecretResponseDTO, + is_list=True, + ) + if secrets_dtos is not None: + return secrets_dtos + + with set_data_encryption_key(data_encryption_key=self._data_encryption_key): secrets_dtos = await self.secrets_dao.list( - project_id=project_id, - organization_id=organization_id, + project_id=project_id, organization_id=organization_id + ) + + if project_id is not None: + await set_cache( + namespace="list_secrets", + project_id=str(project_id), + key={}, + value=secrets_dtos, ) - return secrets_dtos + return secrets_dtos async def update_secret( self, @@ -192,26 +410,18 @@ async def update_secret( with set_data_encryption_key( data_encryption_key=self._data_encryption_key, ): - if update_secret_dto.secret is not None: - stored_secret_dto = await self.secrets_dao.get_by_id( - secret_id=secret_id, - project_id=project_id, - organization_id=organization_id, - ) - if stored_secret_dto is not None: - _carry_over_saved_policy( - stored_data=stored_secret_dto.data, - update_data=update_secret_dto.secret.data, - ) - secret_dto = await self.secrets_dao.update( secret_id=secret_id, update_secret_dto=update_secret_dto, project_id=project_id, organization_id=organization_id, user_id=user_id, + resolve_update=_resolve_update, ) - return secret_dto + + if project_id is not None: + await invalidate_cache(project_id=str(project_id)) + return secret_dto async def delete_secret( self, @@ -227,4 +437,6 @@ async def delete_secret( project_id=project_id, organization_id=organization_id, ) - return + + if project_id is not None: + await invalidate_cache(project_id=str(project_id)) diff --git a/api/oss/src/core/webhooks/service.py b/api/oss/src/core/webhooks/service.py index 2a7d7d313e..a138074e32 100644 --- a/api/oss/src/core/webhooks/service.py +++ b/api/oss/src/core/webhooks/service.py @@ -11,6 +11,7 @@ CreateSecretDTO, SecretDTO, UpdateSecretDTO, + UpdateSecretPayloadDTO, WebhookProviderDTO, WebhookProviderSettingsDTO, ) @@ -135,6 +136,12 @@ async def create_subscription( ), ), ), + # Opted out of the write-only default on purpose. A signing secret is a + # SHARED secret: the subscriber verifies our signature with the same + # value, and when we generated it here that response is the only place + # they can ever read it. Redacting it would ship a subscription nobody + # can verify. + write_only=False, ), ) @@ -378,7 +385,7 @@ async def edit_subscription( secret_id=existing.secret_id, project_id=project_id, update_secret_dto=UpdateSecretDTO( - secret=SecretDTO( + secret=UpdateSecretPayloadDTO( kind=SecretKind.WEBHOOK_PROVIDER, data=WebhookProviderDTO( provider=WebhookProviderSettingsDTO( @@ -404,6 +411,9 @@ async def edit_subscription( ), ), ), + # Same shared-secret reasoning as the create path: a subscription + # that first gets a secret on edit must stay verifiable. + write_only=False, ), ) secret_id = secret_dto.id @@ -419,13 +429,11 @@ async def edit_subscription( return None if subscription.secret is not None: - result = self._with_secret( + return self._with_secret( subscription=result, secret=subscription.secret, ) - return result - if result.secret_id: secret_value = await self._resolve_secret( project_id=project_id, diff --git a/api/oss/src/core/workflows/service.py b/api/oss/src/core/workflows/service.py index 2c4c5cead3..8e83495fe9 100644 --- a/api/oss/src/core/workflows/service.py +++ b/api/oss/src/core/workflows/service.py @@ -141,7 +141,7 @@ find_string_embeds, ) -from oss.src.middlewares.auth import sign_secret_token +from oss.src.middlewares.auth import SECRET_RESOLVE_GRANT, sign_secret_token from oss.src.services.db_manager import get_project_by_id from agenta.sdk.decorators.running import ( @@ -2804,11 +2804,15 @@ async def _prepare_invoke( project_id=str(project_id), ) + # The grant lets the run's vault reads receive write-only secret values in + # plaintext. It normally rides the credential `/access/permissions/check` re-mints, + # but a service running with auth middleware disabled uses this token directly. secret_token = await sign_secret_token( user_id=str(user_id), project_id=str(project_id), workspace_id=str(project.workspace_id), organization_id=str(project.organization_id), + grants=[SECRET_RESOLVE_GRANT], ) credentials = f"Secret {secret_token}" @@ -2929,6 +2933,7 @@ async def inspect_workflow( project_id=str(project_id), workspace_id=str(project.workspace_id), organization_id=str(project.organization_id), + grants=[SECRET_RESOLVE_GRANT], ) credentials = f"Secret {secret_token}" diff --git a/api/oss/src/dbs/postgres/secrets/dao.py b/api/oss/src/dbs/postgres/secrets/dao.py index 78fc46259b..556177a598 100644 --- a/api/oss/src/dbs/postgres/secrets/dao.py +++ b/api/oss/src/dbs/postgres/secrets/dao.py @@ -1,3 +1,4 @@ +from typing import Callable, Optional from uuid import UUID from oss.src.dbs.postgres.secrets.dbes import SecretsDBE @@ -8,7 +9,11 @@ get_transactions_engine, ) -from oss.src.core.secrets.dtos import CreateSecretDTO, UpdateSecretDTO +from oss.src.core.secrets.dtos import ( + CreateSecretDTO, + SecretResponseDTO, + UpdateSecretDTO, +) from oss.src.dbs.postgres.secrets.mappings import ( map_secrets_dto_to_dbe, map_secrets_dbe_to_dto, @@ -120,12 +125,21 @@ async def update( project_id: UUID | None, organization_id: UUID | None, user_id: UUID | None = None, + resolve_update: Optional[ + Callable[[SecretResponseDTO, UpdateSecretDTO], UpdateSecretDTO] + ] = None, ): async with self.engine.session() as session: scope_filter = self._scope_filter(project_id, organization_id) - stmt = select(SecretsDBE).filter_by( - id=secret_id, - **scope_filter, + # FOR UPDATE lets the domain resolver apply immutable policy and carry-over + # against the latest committed row before this transaction persists the update. + stmt = ( + select(SecretsDBE) + .filter_by( + id=secret_id, + **scope_filter, + ) + .with_for_update() ) result = await session.execute(stmt) secrets_dbe = result.scalar() @@ -133,6 +147,16 @@ async def update( if secrets_dbe is None: return None + # Every decision that reads stored state runs HERE, against the locked row. + # A caller that read the row before this transaction may be holding a + # snapshot another writer has already replaced; the keep-on-omit carry-over + # in particular would then write a rotated credential back to its old value. + if resolve_update is not None: + update_secret_dto = resolve_update( + map_secrets_dbe_to_dto(secrets_dbe=secrets_dbe), + update_secret_dto, + ) + map_secrets_dto_to_dbe_update( secrets_dbe=secrets_dbe, update_secret_dto=update_secret_dto, diff --git a/api/oss/src/dbs/postgres/secrets/mappings.py b/api/oss/src/dbs/postgres/secrets/mappings.py index 6b59732563..b23d19ef1b 100644 --- a/api/oss/src/dbs/postgres/secrets/mappings.py +++ b/api/oss/src/dbs/postgres/secrets/mappings.py @@ -13,6 +13,26 @@ ) +# The server-controlled write_only attribute rides inside the encrypted `data` JSON, +# as a sibling of the payload fields, so no schema migration is needed. It is popped back +# out in `map_secrets_dbe_to_dto`, so payload DTOs never see it; rows without the key +# read as write_only=False (legacy rows). +_WRITE_ONLY_KEY = "write_only" + + +def _data_payload( + data_json: dict, + *, + write_only: bool, +) -> str: + if write_only: + data_json[_WRITE_ONLY_KEY] = True + else: + data_json.pop(_WRITE_ONLY_KEY, None) + + return json.dumps(data_json) + + def map_secrets_dto_to_dbe( *, project_id: uuid.UUID | None, @@ -26,7 +46,10 @@ def map_secrets_dto_to_dbe( project_id=project_id, organization_id=organization_id, kind=secret_dto.secret.kind.value, - data=json.dumps(secret_dto.secret.data.model_dump(exclude_none=True)), + data=_data_payload( + secret_dto.secret.data.model_dump(exclude_none=True), + write_only=bool(secret_dto.write_only), + ), ) return vault_secret_dbe @@ -46,27 +69,37 @@ def map_secrets_dto_to_dbe_update( if hasattr(secrets_dbe, key): setattr(secrets_dbe, key, value) + stored_data = json.loads(secrets_dbe.data) + + write_only = bool(stored_data.get(_WRITE_ONLY_KEY)) if update_secret_dto.secret: for key, value in update_secret_dto.secret.model_dump( exclude_none=True ).items(): if key == "data" and hasattr(secrets_dbe, key): - secrets_dbe.data = update_secret_dto.secret.data.model_dump_json() + secrets_dbe.data = _data_payload( + update_secret_dto.secret.data.model_dump(), + write_only=write_only, + ) elif hasattr(secrets_dbe, key): setattr(secrets_dbe, key, value) def map_secrets_dbe_to_dto(*, secrets_dbe: SecretsDBE) -> SecretResponseDTO: + data = json.loads(secrets_dbe.data) # type: ignore + write_only = bool(data.pop(_WRITE_ONLY_KEY, False)) + vault_secret_dto = SecretResponseDTO( id=secrets_dbe.id, # type: ignore slug=secrets_dbe.slug, kind=SecretKind(secrets_dbe.kind).value, - data=json.loads(secrets_dbe.data), # type: ignore + data=data, header=Header(name=secrets_dbe.name, description=secrets_dbe.description), lifecycle=LegacyLifecycleDTO( created_at=str(secrets_dbe.created_at), updated_at=str(secrets_dbe.updated_at), ), + write_only=write_only, ) return vault_secret_dto diff --git a/api/oss/src/middlewares/auth.py b/api/oss/src/middlewares/auth.py index 3a277ea919..302bd48179 100644 --- a/api/oss/src/middlewares/auth.py +++ b/api/oss/src/middlewares/auth.py @@ -1,4 +1,4 @@ -from typing import Optional +from typing import List, Optional from uuid import UUID from datetime import datetime, timezone import asyncio @@ -92,6 +92,40 @@ # reject a token minted a moment "in the future". Tolerate that much drift on `iat` and `exp`. _SECRET_LEEWAY = 30 # seconds +# A grant ADDS one capability to an otherwise general-purpose token, instead of +# confining that token to a narrower use. The runtime's credential must stay +# general-purpose (it authenticates workflows, tools, and vault reads alike), so the +# vault's plaintext-read capability rides a grant. +SECRET_RESOLVE_GRANT = "secret-resolve" +ALLOWED_SECRET_TOKEN_GRANTS = frozenset({SECRET_RESOLVE_GRANT}) + + +def _validate_secret_token_grants(grants: object) -> tuple[str, ...]: + """Return known grants and reject every unrecognized claim shape or value.""" + if grants is None: + return () + + if not isinstance(grants, list): + raise ValueError("Secret token grants must be a list.") + + if any( + not isinstance(grant, str) or grant not in ALLOWED_SECRET_TOKEN_GRANTS + for grant in grants + ): + raise ValueError("Secret token contains an unsupported grant.") + + return tuple(grants) + + +def request_has_grant(request: Request, grant: str) -> bool: + """Whether the request's verified credential carries ``grant``. + + Only `verify_secret_token` populates grants; session and ApiKey principals never + carry any, so this is False for them by construction. + """ + return grant in getattr(request.state, "token_grants", ()) + + _ZERO_UUID = "00000000-0000-0000-0000-000000000000" _NULL_UUID = "null" @@ -916,6 +950,13 @@ async def verify_secret_token( leeway=_SECRET_LEEWAY, ) + try: + request.state.token_grants = _validate_secret_token_grants( + auth_context.get("grants") + ) + except ValueError as exc: + raise DecodeError("Secret token contains invalid grants.") from exc + request.state.user_id = auth_context.get("user_id") request.state.user_email = auth_context.get("user_email") request.state.project_id = auth_context.get("project_id") @@ -1000,7 +1041,10 @@ async def sign_secret_token( workspace_id: Optional[str] = None, organization_id: Optional[str] = None, organization_name: Optional[str] = None, + grants: Optional[List[str]] = None, ): + validated_grants = _validate_secret_token_grants(grants) + try: if not _SECRET_KEY: raise InternalServerErrorException() @@ -1019,6 +1063,11 @@ async def sign_secret_token( "exp": _exp, } + # A token with no grants carries no `grants` key at all, rather than a null or + # empty one, so its payload stays the shape every existing holder was issued. + if validated_grants: + auth_context["grants"] = list(validated_grants) + secret_token = encode( payload=auth_context, key=_SECRET_KEY, diff --git a/api/oss/src/utils/env.py b/api/oss/src/utils/env.py index c338f41150..d164427f18 100644 --- a/api/oss/src/utils/env.py +++ b/api/oss/src/utils/env.py @@ -575,6 +575,16 @@ class SessionsConfig(BaseModel): # --------------------------------------------------------------------------- +def _services_internal_key_from_environment() -> str | None: + """Read the dedicated runtime proof without accepting public placeholders.""" + runtime_key = (os.getenv("AGENTA_SERVICES_INTERNAL_KEY") or "").strip() + + if not runtime_key or runtime_key == "replace-me": + return None + + return runtime_key + + class AgentaConfig(BaseModel): """Agenta core configuration""" @@ -587,6 +597,13 @@ class AgentaConfig(BaseModel): auth_key: str = os.getenv("AGENTA_AUTH_KEY") or "replace-me" crypt_key: str = os.getenv("AGENTA_CRYPT_KEY") or "replace-me" + # Shared secret that proves a caller IS the platform runtime (the workflow service), + # as opposed to a browser or an ApiKey holder reaching the same public route. Only a + # caller holding it can be issued a credential that reads write-only secret values. + # This is deliberately separate from the administrator key: deployments must opt in + # by configuring the same dedicated value on the API and platform services. NEVER + # sent to the runner or into a sandbox. + services_internal_key: str | None = _services_internal_key_from_environment() access: AccessConfig = AccessConfig() ai_services: AIServicesConfig = AIServicesConfig() diff --git a/api/oss/src/utils/helpers.py b/api/oss/src/utils/helpers.py index aeec502aad..f04685aa5a 100644 --- a/api/oss/src/utils/helpers.py +++ b/api/oss/src/utils/helpers.py @@ -8,6 +8,9 @@ import click from oss.src.utils.env import env +from oss.src.utils.logging import get_module_logger + +log = get_module_logger(__name__) def get_metrics_keys_from_schema(schema=None, path=()) -> List[Dict[str, str]]: @@ -184,6 +187,29 @@ def warn_deprecated_env_vars(): ) +def validate_platform_runtime_key(): + """Stop startup when nothing can read a write-only secret. + + A run reads a write-only secret only through a credential the platform runtime is + issued, and the runtime is recognized by a dedicated shared key. If that key is unset + or still uses the public placeholder from an example env file, platform runs cannot + read their write-only connections. The failure surfaces as "provide the provider key + in this run's environment", which is true for a standalone run and useless here, so + the cause has to be said where an operator will see it. + """ + runtime_key = (env.agenta.services_internal_key or "").strip() + if runtime_key and runtime_key != "replace-me": + return + + raise RuntimeError( + "AGENTA_SERVICES_INTERNAL_KEY is required and must not use the placeholder. " + "Without it, platform runs cannot receive the short-lived grant needed to read " + "write-only secrets. " + "Set AGENTA_SERVICES_INTERNAL_KEY to the same value on the API and the services " + "container." + ) + + def validate_required_env_vars(): """ Ensure required configuration values are present. diff --git a/api/oss/tests/pytest/unit/access/test_grant_exchange.py b/api/oss/tests/pytest/unit/access/test_grant_exchange.py new file mode 100644 index 0000000000..05e374acb4 --- /dev/null +++ b/api/oss/tests/pytest/unit/access/test_grant_exchange.py @@ -0,0 +1,272 @@ +"""The `/access/permissions/check` exchange is the only place users' credentials become +grant-bearing runtime credentials — and only for an ALLOWED `run_service` exchange. + +Drives the real `AccessRouter.check_permissions` handler with real token minting (the +returned credentials decode with the real key); only the permission verdict and the Redis +cache are faked. +""" + +from uuid import uuid4 + +import pytest +from jwt import decode +from starlette.requests import Request + +import oss.src.middlewares.auth as auth_module +from oss.src.apis.fastapi.access import router as access_router_module +from oss.src.apis.fastapi.access.router import AccessRouter +from oss.src.middlewares.auth import SECRET_RESOLVE_GRANT +from oss.src.utils.env import env +from oss.src.utils.context import ( + AuthContext, + AuthScope, + SecretCredentials, + reset_auth_context, + set_auth_context, +) + + +SECRET_KEY = "unit-test-secret-key-with-32-bytes" +RUNTIME_KEY = "unit-test-runtime-key-not-a-secret" + +ORGANIZATION_ID = uuid4() +WORKSPACE_ID = uuid4() +PROJECT_ID = uuid4() +USER_ID = uuid4() + + +@pytest.fixture(name="exchange") +def _exchange(monkeypatch): + monkeypatch.setattr(auth_module, "_SECRET_KEY", SECRET_KEY) + monkeypatch.setattr(env.agenta, "services_internal_key", RUNTIME_KEY) + + verdict = {"action": True, "resource": True} + + async def _check_action_access(**kwargs): + return verdict["action"] + + async def _get_cache(**kwargs): + return None + + async def _set_cache(**kwargs): + return True + + async def _check_resource_access(**kwargs): + return verdict["resource"] + + monkeypatch.setattr( + access_router_module, "check_action_access", _check_action_access + ) + monkeypatch.setattr(access_router_module, "get_cache", _get_cache) + monkeypatch.setattr(access_router_module, "set_cache", _set_cache) + monkeypatch.setattr( + access_router_module, "_check_resource_access", _check_resource_access + ) + + router = AccessRouter() + + async def run(action, resource_type="service", carried_grants=(), runtime_key=None): + """Run the exchange as a principal whose credential carries ``carried_grants``. + + A session or ApiKey principal never has any: `verify_secret_token` is the only + path that populates `token_grants`, and only from a verified Secret token. + ``runtime_key`` is what the caller presents as the platform-runtime secret. + """ + request = Request( + { + "type": "http", + "method": "GET", + "path": "/access/permissions/check", + "headers": ( + [(b"x-agenta-runtime-key", runtime_key.encode())] + if runtime_key is not None + else [] + ), + "query_string": b"", + "scheme": "http", + "server": ("testserver", 80), + "root_path": "", + } + ) + request.state.token_grants = tuple(carried_grants) + + token = set_auth_context( + AuthContext( + credentials=SecretCredentials(value="caller-token"), + scope=AuthScope( + organization_id=ORGANIZATION_ID, + workspace_id=WORKSPACE_ID, + project_id=PROJECT_ID, + user_id=USER_ID, + ), + ) + ) + try: + return await router.check_permissions( + request, + action=action, + scope_type=None, + scope_id=None, + resource_type=resource_type, + resource_id=None, + ) + finally: + reset_auth_context(token) + + return run, verdict + + +def _claims(header_value: str) -> dict: + assert header_value.startswith("Secret ") + return decode( + jwt=header_value[len("Secret ") :], + key=SECRET_KEY, + algorithms=["HS256"], + options={"verify_exp": False}, + ) + + +def _body(response) -> dict: + import json + + return json.loads(response.body) + + +@pytest.mark.asyncio +async def test_a_granted_caller_keeps_the_grant_through_the_exchange(exchange): + # The refresh path: the workflow service and the runner re-exchange the granted + # credential a run was started with, and must get one back or the run loses its + # ability to read the secrets it was authorized to use. + run, _ = exchange + + body = _body(await run("run_service", carried_grants=(SECRET_RESOLVE_GRANT,))) + + assert body["effect"] == "allow" + claims = _claims(body["credentials"]) + assert claims["grants"] == [SECRET_RESOLVE_GRANT] + assert claims["project_id"] == str(PROJECT_ID) + + +@pytest.mark.asyncio +async def test_the_platform_runtime_is_issued_the_grant(exchange): + # The path every product run takes: the workflow service exchanges the END USER's + # credential on their behalf, so nothing about the token says "this is a run". The + # runtime proves what it is with a secret only it holds. + run, _ = exchange + + body = _body(await run("run_service", runtime_key=RUNTIME_KEY)) + + claims = _claims(body["credentials"]) + assert claims["grants"] == [SECRET_RESOLVE_GRANT] + + +@pytest.mark.asyncio +async def test_a_wrong_runtime_key_is_not_the_runtime(exchange): + run, _ = exchange + + body = _body(await run("run_service", runtime_key="not-the-key")) + + assert "grants" not in _claims(body["credentials"]) + + +@pytest.mark.asyncio +async def test_the_runtime_key_only_grants_a_run_exchange(exchange): + run, _ = exchange + + body = _body(await run("view_secret", runtime_key=RUNTIME_KEY)) + + assert "grants" not in _claims(body["credentials"]) + + +@pytest.mark.asyncio +async def test_an_unconfigured_deployment_grants_nobody(exchange, monkeypatch): + # The placeholder is in the repo, so anyone could send it. A deployment that + # configured no runtime key must issue no grant rather than accept a known string. + monkeypatch.setattr(env.agenta, "services_internal_key", "replace-me") + run, _ = exchange + + body = _body(await run("run_service", runtime_key="replace-me")) + + assert "grants" not in _claims(body["credentials"]) + + +@pytest.mark.asyncio +async def test_the_admin_key_is_not_runtime_proof(exchange, monkeypatch): + monkeypatch.setattr(env.agenta, "services_internal_key", None) + run, _ = exchange + + body = _body(await run("run_service", runtime_key=env.agenta.auth_key)) + + assert "grants" not in _claims(body["credentials"]) + + +@pytest.mark.asyncio +async def test_a_plain_caller_cannot_mint_the_grant_by_asking_for_it(exchange): + # The escalation this closes: a member who may run a service could call the exchange + # with their own session or ApiKey — neither of which carries a grant — and spend the + # returned credential on the vault routes to read every write-only value in plaintext. + run, _ = exchange + + body = _body(await run("run_service")) + + assert body["effect"] == "allow" + claims = _claims(body["credentials"]) + assert "grants" not in claims + + +@pytest.mark.asyncio +async def test_an_unrelated_carried_grant_is_not_forwarded(exchange): + run, _ = exchange + + body = _body(await run("run_service", carried_grants=("some-other-grant",))) + + claims = _claims(body["credentials"]) + assert "grants" not in claims + + +@pytest.mark.asyncio +async def test_non_run_actions_never_receive_the_grant(exchange): + run, _ = exchange + + body = _body(await run("view_secret", resource_type="local_secrets")) + + assert body["effect"] == "allow" + assert "grants" not in _claims(body["credentials"]) + + +@pytest.mark.asyncio +async def test_denied_exchange_returns_no_credential_at_all(exchange): + run, verdict = exchange + verdict["action"] = False + + from fastapi import HTTPException + + with pytest.raises(HTTPException) as raised: + await run("run_service") + + assert raised.value.status_code == 403 + + +@pytest.mark.asyncio +async def test_resource_denial_returns_no_runtime_credential(exchange): + run, verdict = exchange + verdict["resource"] = False + + from fastapi import HTTPException + + with pytest.raises(HTTPException) as raised: + await run("run_service", runtime_key=RUNTIME_KEY) + + assert raised.value.status_code == 403 + + +@pytest.mark.asyncio +async def test_missing_action_is_denied(exchange): + run, _ = exchange + + from fastapi import HTTPException + + with pytest.raises(HTTPException) as raised: + await run(None) + + assert raised.value.status_code == 403 diff --git a/api/oss/tests/pytest/unit/middlewares/test_auth_grants.py b/api/oss/tests/pytest/unit/middlewares/test_auth_grants.py new file mode 100644 index 0000000000..777ce74c75 --- /dev/null +++ b/api/oss/tests/pytest/unit/middlewares/test_auth_grants.py @@ -0,0 +1,162 @@ +"""A grant ADDS one capability to a general-purpose Secret token; a scope CONFINES one. + +The secret-resolve grant is what lets the platform runtime read write-only vault secrets in +plaintext. These pin the axis separation: a granted token stays valid everywhere (unlike a +scoped one), the claim round-trips into ``request.state.token_grants``, and principals +without it — including plain unscoped tokens — read as grant-less. +""" + +from datetime import datetime, timedelta, timezone + +import pytest +from jwt import decode, encode +from starlette.requests import Request + +from oss.src.middlewares import auth +from oss.src.utils.exceptions import UnauthorizedException + + +SECRET_KEY = "unit-test-secret-key-with-32-bytes" + + +class _RecordingLog: + def __init__(self): + self.calls = [] + + def __getattr__(self, level): + def log(event, *args, **fields): + self.calls.append((level, event, fields)) + + return log + + +@pytest.fixture(name="log") +def _log(monkeypatch): + recorder = _RecordingLog() + monkeypatch.setattr(auth, "log", recorder) + monkeypatch.setattr(auth, "_SECRET_KEY", SECRET_KEY) + return recorder + + +def _request(path: str = "/vault/v1/secrets/") -> Request: + return Request( + { + "type": "http", + "method": "GET", + "path": path, + "headers": [], + "query_string": b"", + "scheme": "http", + "server": ("testserver", 80), + "root_path": "", + } + ) + + +def _claims(token: str) -> dict: + return decode( + jwt=token, + key=SECRET_KEY, + algorithms=["HS256"], + options={"verify_exp": False}, + ) + + +@pytest.mark.asyncio +async def test_granted_token_carries_the_claim_and_stays_valid_on_any_path(log): + token = await auth.sign_secret_token( + user_id="u", + project_id="p", + grants=[auth.SECRET_RESOLVE_GRANT], + ) + + assert _claims(token)["grants"] == [auth.SECRET_RESOLVE_GRANT] + # A grant must never confine the token the way a scope does: the runtime uses this same + # credential for workflows, tools, and session coordination. + for path in ( + "/vault/v1/secrets/", + "/workflows/123/revisions/commit", + "/access/permissions/check", + ): + request = _request(path=path) + await auth.verify_secret_token(request=request, secret_token=token) + assert auth.request_has_grant(request, auth.SECRET_RESOLVE_GRANT) + + assert log.calls == [] + + +@pytest.mark.asyncio +async def test_plain_token_carries_no_grants_claim_and_reads_grant_less(log): + token = await auth.sign_secret_token(user_id="u", project_id="p") + + assert "grants" not in _claims(token) + + request = _request() + await auth.verify_secret_token(request=request, secret_token=token) + + assert request.state.token_grants == () + assert not auth.request_has_grant(request, auth.SECRET_RESOLVE_GRANT) + + +def test_request_without_verified_token_has_no_grants(): + # Session and ApiKey principals never populate token_grants at all. + assert not auth.request_has_grant(_request(), auth.SECRET_RESOLVE_GRANT) + + +@pytest.mark.asyncio +async def test_unknown_grant_is_rejected_at_issuance(log): + with pytest.raises(ValueError, match="unsupported grant"): + await auth.sign_secret_token(user_id="u", grants=["something-else"]) + + +@pytest.mark.asyncio +async def test_unknown_grant_is_rejected_at_consumption(log): + expiry = datetime.now(timezone.utc) + timedelta(seconds=600) + token = encode( + payload={ + "user_id": "u", + "grants": ["something-else"], + "exp": int(expiry.timestamp()), + }, + key=SECRET_KEY, + algorithm="HS256", + ) + + with pytest.raises(UnauthorizedException) as rejected: + await auth.verify_secret_token(request=_request(), secret_token=token) + + assert rejected.value.status_code == 401 + assert rejected.value.detail["reason"] == "invalid_token" + + +@pytest.mark.asyncio +async def test_grants_ride_expiry_unchanged(log): + # The grant changes what the token may READ, never how long it lives. + now = datetime.now(timezone.utc).timestamp() + claims = _claims( + await auth.sign_secret_token(user_id="u", grants=[auth.SECRET_RESOLVE_GRANT]) + ) + + assert now + 15 * 60 - 5 < claims["exp"] < now + 15 * 60 + 5 + + +@pytest.mark.asyncio +async def test_forged_grants_on_a_foreign_signed_token_are_rejected(log): + expiry = datetime.now(timezone.utc) + timedelta(seconds=600) + forged = encode( + payload={ + "user_id": "u", + "grants": [auth.SECRET_RESOLVE_GRANT], + "exp": int(expiry.timestamp()), + }, + key="some-other-key-entirely-not-ours!", + algorithm="HS256", + ) + + # The concrete rejection, not any failure: a signature check that broke into an + # AttributeError would otherwise still read as "rejected". + with pytest.raises(UnauthorizedException) as rejected: + await auth.verify_secret_token(request=_request(), secret_token=forged) + + assert rejected.value.status_code == 401 + assert rejected.value.detail["reason"] == "invalid_token" diff --git a/api/oss/tests/pytest/unit/secrets/test_dtos.py b/api/oss/tests/pytest/unit/secrets/test_dtos.py index ecef0c90ab..62fd3f1d24 100644 --- a/api/oss/tests/pytest/unit/secrets/test_dtos.py +++ b/api/oss/tests/pytest/unit/secrets/test_dtos.py @@ -344,6 +344,74 @@ def test_create_secret_rejects_an_empty_header(kind): CreateSecretDTO.model_validate(_payload_without_a_header(kind)) +def _sso_payload(client_secret): + return { + "header": {"name": "Okta"}, + "secret": { + "kind": "sso_provider", + "data": { + "provider": { + "client_id": "id", + "client_secret": client_secret, + "issuer_url": "https://issuer.example", + "scopes": ["openid"], + } + }, + }, + } + + +def test_create_sso_secret_rejects_a_null_client_secret(): + # Presence is not a value: an explicit null would store an SSO record with no + # credential. A value may be omitted only on the update path, where omission means + # "keep the stored one". + with pytest.raises(ValidationError, match="SSOProviderSettingsDTO"): + CreateSecretDTO.model_validate(_sso_payload(None)) + + with pytest.raises(ValidationError, match="SSOProviderSettingsDTO"): + CreateSecretDTO.model_validate( + { + "header": {"name": "Okta"}, + "secret": { + "kind": "sso_provider", + "data": { + "provider": { + "client_id": "id", + "issuer_url": "https://issuer.example", + "scopes": ["openid"], + } + }, + }, + } + ) + + +def test_create_sso_secret_accepts_a_real_client_secret(): + secret = CreateSecretDTO.model_validate(_sso_payload("sso-client-secret")) + + assert secret.secret.kind.value == "sso_provider" + + +def test_update_sso_secret_may_omit_the_client_secret(): + # Keep-on-omit: an update without a value keeps the stored one. + update = UpdateSecretDTO.model_validate( + { + "secret": { + "kind": "sso_provider", + "data": { + "provider": { + "client_id": "id", + "issuer_url": "https://issuer.example", + "scopes": ["openid"], + } + }, + } + } + ) + + assert update.secret is not None + + def test_create_secret_allows_an_empty_header_for_a_provider_key(): # The one kind that may arrive unnamed: the service names it after its provider on create. secret = CreateSecretDTO.model_validate( diff --git a/api/oss/tests/pytest/unit/secrets/test_services.py b/api/oss/tests/pytest/unit/secrets/test_services.py index f7031b9f61..becaad971b 100644 --- a/api/oss/tests/pytest/unit/secrets/test_services.py +++ b/api/oss/tests/pytest/unit/secrets/test_services.py @@ -63,6 +63,7 @@ async def update( project_id, organization_id, user_id=None, + resolve_update=None, ): del user_id scope = (project_id, organization_id) @@ -72,6 +73,11 @@ async def update( ) if stored is None: return None + + # Production resolves the update against the row under the write lock; the fake + # does the same at the same point, so keep-on-omit is exercised, not skipped. + if resolve_update is not None: + update_secret_dto = resolve_update(stored, update_secret_dto) # Like the postgres mapping, the whole data blob is replaced: whatever the payload # omits is gone unless the service carried it over first. record = SecretResponseDTO( diff --git a/api/oss/tests/pytest/unit/secrets/test_write_only.py b/api/oss/tests/pytest/unit/secrets/test_write_only.py new file mode 100644 index 0000000000..f797f145c5 --- /dev/null +++ b/api/oss/tests/pytest/unit/secrets/test_write_only.py @@ -0,0 +1,1103 @@ +"""Write-only secrets: the value can be set and replaced, never read back by a user. + +Covers the three layers below the router: the service (default-on at create, value +carry-over on update, the immutable creation policy), the redaction helper (per-kind +value stripping and value_status), and the postgres mappings (the flag rides inside the encrypted data +JSON and never leaks into the payload DTOs). +""" + +from uuid import uuid4 + +import pytest + +from agenta.sdk.agents.connections.credentials import secret_value_configured + +import oss.src.core.secrets.services as secrets_services_module +from oss.src.core.secrets.dtos import ( + CreateSecretDTO, + SecretResponseDTO, + SecretValueRequiredError, + UpdateSecretDTO, +) +from oss.src.core.secrets.redaction import ( + mask_secret_value, + redact_secret_response, +) +from oss.src.core.secrets.services import VaultService +from oss.src.dbs.postgres.secrets.mappings import ( + map_secrets_dbe_to_dto, + map_secrets_dto_to_dbe, + map_secrets_dto_to_dbe_update, +) + + +PROJECT_ID = uuid4() + + +class _FakeSecretsDAO: + """In-memory DAO: stores what the service hands it, like the real mapping would.""" + + def __init__(self): + self.records: dict = {} + + async def create(self, project_id, organization_id, create_secret_dto): + record = SecretResponseDTO( + id=uuid4(), + slug=create_secret_dto.slug, + kind=create_secret_dto.secret.kind, + data=create_secret_dto.secret.data.model_dump(exclude_none=True), + header=create_secret_dto.header, + write_only=bool(create_secret_dto.write_only), + ) + self.records[record.id] = record + return record + + async def list(self, project_id, organization_id): + return list(self.records.values()) + + async def get_by_id(self, secret_id, project_id, organization_id): + return self.records.get(secret_id) + + async def update( + self, + secret_id, + update_secret_dto, + project_id, + organization_id, + user_id=None, + resolve_update=None, + ): + stored = self.records.get(secret_id) + if stored is None: + return None + + # Production resolves the update against the row under the write lock; the fake + # does the same at the same point, so keep-on-omit is exercised, not skipped. + if resolve_update is not None: + update_secret_dto = resolve_update(stored, update_secret_dto) + + updated = stored.model_copy( + update={ + "header": update_secret_dto.header or stored.header, + } + ) + if update_secret_dto.secret is not None: + updated.kind = update_secret_dto.secret.kind + updated.data = update_secret_dto.secret.data + + self.records[secret_id] = updated + return updated + + async def delete( + self, secret_id, project_id, organization_id, authorize_delete=None + ): + stored = self.records.get(secret_id) + if stored is not None and authorize_delete is not None: + authorize_delete(stored) + self.records.pop(secret_id, None) + + +@pytest.fixture(name="service") +def _service(): + return VaultService(_FakeSecretsDAO()) + + +def _provider_key_create(key="sk-test-openai-key-bc", write_only=True): + return CreateSecretDTO( + header={"name": "OpenAI"}, + secret={ + "kind": "provider_key", + "data": {"kind": "openai", "provider": {"key": key}}, + }, + write_only=write_only, + ) + + +# --- service: create ------------------------------------------------------------------ + + +@pytest.mark.asyncio +async def test_create_defaults_to_write_only(service): + created = await service.create_secret( + project_id=PROJECT_ID, create_secret_dto=_provider_key_create() + ) + + assert created.write_only is True + + +@pytest.mark.asyncio +@pytest.mark.parametrize("explicit", [False, True]) +async def test_an_explicit_create_value_is_preserved(service, explicit): + created = await service.create_secret( + project_id=PROJECT_ID, + create_secret_dto=_provider_key_create(write_only=explicit), + ) + + assert created.write_only is explicit + + +# --- service: keep-stored-on-omit ------------------------------------------------------ + + +@pytest.mark.asyncio +async def test_project_list_cache_stores_the_canonical_plaintext_dto( + service, monkeypatch +): + cached = None + dao_calls = 0 + + original_list = service.secrets_dao.list + + async def counted_list(*args, **kwargs): + nonlocal dao_calls + dao_calls += 1 + return await original_list(*args, **kwargs) + + async def fake_get_cache(**kwargs): + assert kwargs["namespace"] == "list_secrets" + assert kwargs["project_id"] == str(PROJECT_ID) + assert kwargs["key"] == {} + assert kwargs["model"] is SecretResponseDTO + assert kwargs["is_list"] is True + return cached + + async def fake_set_cache(**kwargs): + nonlocal cached + assert kwargs["namespace"] == "list_secrets" + assert kwargs["project_id"] == str(PROJECT_ID) + assert kwargs["key"] == {} + cached = kwargs["value"] + return True + + monkeypatch.setattr(service.secrets_dao, "list", counted_list) + monkeypatch.setattr(secrets_services_module, "get_cache", fake_get_cache) + monkeypatch.setattr(secrets_services_module, "set_cache", fake_set_cache) + + await service.create_secret( + project_id=PROJECT_ID, + create_secret_dto=_provider_key_create(), + ) + first = await service.list_secrets(project_id=PROJECT_ID) + second = await service.list_secrets(project_id=PROJECT_ID) + + assert dao_calls == 1 + assert first == second + assert cached[0].data.provider.key == "sk-test-openai-key-bc" + + +@pytest.mark.asyncio +async def test_service_mutations_invalidate_the_project_cache(service, monkeypatch): + invalidated = [] + + async def fake_invalidate_cache(**kwargs): + invalidated.append(kwargs) + return True + + monkeypatch.setattr( + secrets_services_module, + "invalidate_cache", + fake_invalidate_cache, + ) + + created = await service.create_secret( + project_id=PROJECT_ID, + create_secret_dto=_provider_key_create(), + ) + await service.update_secret( + secret_id=created.id, + project_id=PROJECT_ID, + update_secret_dto=UpdateSecretDTO(header={"name": "Renamed"}), + ) + await service.delete_secret(secret_id=created.id, project_id=PROJECT_ID) + + assert invalidated == [{"project_id": str(PROJECT_ID)}] * 3 + + +class _RotatingDAO(_FakeSecretsDAO): + """A DAO where another writer commits a rotation while this update waits for the lock. + + The real DAO takes ``SELECT ... FOR UPDATE`` and only then resolves the update, so a + writer that committed while we waited is visible. This fake reproduces that window by + + rotating the stored row immediately before it resolves. + """ + + def __init__(self, rotated_key: str): + super().__init__() + self.rotated_key = rotated_key + + async def update( + self, + secret_id, + update_secret_dto, + project_id, + organization_id, + user_id=None, + resolve_update=None, + ): + stored = self.records.get(secret_id) + if stored is not None: + rotated = stored.model_dump() + rotated["data"]["provider"]["key"] = self.rotated_key + self.records[secret_id] = SecretResponseDTO(**rotated) + + return await super().update( + secret_id=secret_id, + update_secret_dto=update_secret_dto, + project_id=project_id, + organization_id=organization_id, + user_id=user_id, + resolve_update=resolve_update, + ) + + +@pytest.mark.asyncio +async def test_an_omitted_key_keeps_the_value_a_racing_rotation_just_stored(): + # A rotation that lands between "read the row" and "write the row" must not be undone. + # Resolving the omitted credential against a snapshot taken before the lock wrote the + # OLD key back over the new one, silently reverting the rotation. + dao = _RotatingDAO(rotated_key="sk-test-rotated") + service = VaultService(dao) + + created = await service.create_secret( + project_id=PROJECT_ID, + create_secret_dto=_provider_key_create(write_only=True), + ) + + updated = await service.update_secret( + secret_id=created.id, + project_id=PROJECT_ID, + update_secret_dto=UpdateSecretDTO( + secret={ + "kind": "provider_key", + "data": {"kind": "openai", "provider": {}}, + } + ), + ) + + assert updated.data.provider.key == "sk-test-rotated" + + +@pytest.mark.asyncio +async def test_update_without_provider_key_keeps_the_stored_one(service): + created = await service.create_secret( + project_id=PROJECT_ID, create_secret_dto=_provider_key_create() + ) + + update = UpdateSecretDTO( + header={"name": "OpenAI (renamed)"}, + secret={ + "kind": "provider_key", + "data": {"kind": "openai", "provider": {}}, + }, + ) + updated = await service.update_secret( + secret_id=created.id, project_id=PROJECT_ID, update_secret_dto=update + ) + + assert updated.data.provider.key == "sk-test-openai-key-bc" + assert updated.header.name == "OpenAI (renamed)" + + +@pytest.mark.asyncio +async def test_update_with_an_explicit_blank_provider_key_is_rejected(service): + created = await service.create_secret( + project_id=PROJECT_ID, create_secret_dto=_provider_key_create() + ) + + update = UpdateSecretDTO( + secret={ + "kind": "provider_key", + "data": {"kind": "openai", "provider": {"key": ""}}, + }, + ) + + with pytest.raises( + SecretValueRequiredError, + match=( + "Credential values cannot be blank. Omit an unchanged credential field or provide " + "a new value." + ), + ): + await service.update_secret( + secret_id=created.id, project_id=PROJECT_ID, update_secret_dto=update + ) + + +@pytest.mark.asyncio +async def test_update_with_a_new_provider_key_replaces_the_stored_one(service): + created = await service.create_secret( + project_id=PROJECT_ID, create_secret_dto=_provider_key_create() + ) + + update = UpdateSecretDTO( + secret={ + "kind": "provider_key", + "data": {"kind": "openai", "provider": {"key": "sk-test-rotated"}}, + }, + ) + updated = await service.update_secret( + secret_id=created.id, project_id=PROJECT_ID, update_secret_dto=update + ) + + assert updated.data.provider.key == "sk-test-rotated" + + +@pytest.mark.asyncio +async def test_update_without_custom_provider_key_and_extras_keeps_stored_values( + service, +): + created = await service.create_secret( + project_id=PROJECT_ID, + create_secret_dto=CreateSecretDTO( + header={"name": "my-gateway"}, + secret={ + "kind": "custom_provider", + "data": { + "kind": "openai", + "provider": { + "url": "https://gateway.example.com/v1", + "key": "gw-test-key", + "extras": { + "api_key": "extra-key-123456", + "region": "eu-west-1", + }, + }, + "models": [{"slug": "gpt-5"}], + }, + }, + ), + ) + + update = UpdateSecretDTO( + secret={ + "kind": "custom_provider", + "data": { + "kind": "openai", + "provider": {"url": "https://gateway.example.com/v2"}, + "models": [{"slug": "gpt-5"}], + }, + }, + ) + updated = await service.update_secret( + secret_id=created.id, project_id=PROJECT_ID, update_secret_dto=update + ) + + assert updated.data.provider.url == "https://gateway.example.com/v2" + assert updated.data.provider.key == "gw-test-key" + # Omitted extras carry over whole: replace-only forms must not wipe them. + assert updated.data.provider.extras["api_key"] == "extra-key-123456" + assert updated.data.provider.extras["region"] == "eu-west-1" + + +@pytest.mark.asyncio +async def test_update_with_partial_extras_refills_credential_keys_only(service): + created = await service.create_secret( + project_id=PROJECT_ID, + create_secret_dto=CreateSecretDTO( + header={"name": "my-gateway"}, + secret={ + "kind": "custom_provider", + "data": { + "kind": "openai", + "provider": { + "url": "https://gateway.example.com/v1", + "key": "gw-test-key", + "extras": { + "api_key": "extra-key-123456", + "region": "eu-west-1", + }, + }, + "models": [{"slug": "gpt-5"}], + }, + }, + ), + ) + + update = UpdateSecretDTO( + secret={ + "kind": "custom_provider", + "data": { + "kind": "openai", + "provider": { + "url": "https://gateway.example.com/v1", + "extras": {"region": "us-east-1"}, + }, + "models": [{"slug": "gpt-5"}], + }, + }, + ) + updated = await service.update_secret( + secret_id=created.id, project_id=PROJECT_ID, update_secret_dto=update + ) + + # The submitted config wins; only the credential keys refill from storage. + + assert updated.data.provider.extras["region"] == "us-east-1" + assert updated.data.provider.extras["api_key"] == "extra-key-123456" + + +@pytest.mark.asyncio +async def test_explicit_empty_custom_provider_credential_extra_is_rejected(service): + created = await service.create_secret( + project_id=PROJECT_ID, + create_secret_dto=CreateSecretDTO( + header={"name": "Bedrock"}, + secret={ + "kind": "custom_provider", + "data": { + "kind": "bedrock", + "provider": { + "extras": { + "AWS_ACCESS_KEY_ID": "AKIA123", + "AWS_SECRET_ACCESS_KEY": "stored-secret", + "AWS_REGION": "eu-west-1", + } + }, + "models": [{"slug": "claude"}], + }, + }, + ), + ) + + update = UpdateSecretDTO( + secret={ + "kind": "custom_provider", + "data": { + "kind": "bedrock", + "provider": { + "extras": { + "AWS_SECRET_ACCESS_KEY": "", + "AWS_REGION": "us-east-1", + } + }, + "models": [{"slug": "claude"}], + }, + }, + ) + + with pytest.raises(SecretValueRequiredError): + await service.update_secret( + secret_id=created.id, + project_id=PROJECT_ID, + update_secret_dto=update, + ) + + +@pytest.mark.asyncio +async def test_update_without_custom_secret_content_keeps_the_stored_one(service): + created = await service.create_secret( + project_id=PROJECT_ID, + create_secret_dto=CreateSecretDTO( + header={"name": "gh-token"}, + secret={ + "kind": "custom_secret", + "data": { + "secret": {"format": "text", "content": "ghp_example_token_xyz"} + }, + }, + ), + ) + + update = UpdateSecretDTO( + header={"name": "gh-token (renamed)"}, + secret={ + "kind": "custom_secret", + "data": {"secret": {"format": "text"}}, + }, + ) + updated = await service.update_secret( + secret_id=created.id, project_id=PROJECT_ID, update_secret_dto=update + ) + + assert updated.data.secret.content == "ghp_example_token_xyz" + + +def test_write_only_is_not_an_update_field(): + with pytest.raises(ValueError, match="cannot be updated"): + UpdateSecretDTO.model_validate({"write_only": False}) + + with pytest.raises(ValueError, match="cannot be updated"): + UpdateSecretDTO.model_validate({"write_only": True}) + + +@pytest.mark.parametrize( + ("kind", "data"), + [ + ( + "provider_key", + {"kind": "openai", "provider": {"key": ""}}, + ), + ( + "webhook_provider", + {"provider": {"key": ""}}, + ), + ( + "sso_provider", + { + "provider": { + "client_id": "client", + "client_secret": "", + "issuer_url": "https://issuer.example.com", + "scopes": ["openid"], + } + }, + ), + ], +) +def test_create_rejects_empty_credentials(kind, data): + with pytest.raises(ValueError): + CreateSecretDTO( + header={"name": "Connection"}, + secret={"kind": kind, "data": data}, + ) + + +@pytest.mark.asyncio +async def test_update_cannot_keep_a_missing_value_from_a_legacy_row(service): + legacy = SecretResponseDTO( + id=uuid4(), + slug="legacy-openai", + kind="provider_key", + data={"kind": "openai", "provider": {}}, + header={"name": "Legacy"}, + write_only=False, + ) + service.secrets_dao.records[legacy.id] = legacy + + with pytest.raises(SecretValueRequiredError): + await service.update_secret( + secret_id=legacy.id, + project_id=PROJECT_ID, + update_secret_dto=UpdateSecretDTO( + header={"name": "Renamed"}, + secret={ + "kind": "provider_key", + "data": {"kind": "openai", "provider": {}}, + }, + ), + ) + + +# --- service: keep-on-omit is identity-local ------------------------------------------- + + +@pytest.mark.asyncio +async def test_provider_family_change_with_omitted_key_is_rejected(service): + created = await service.create_secret( + project_id=PROJECT_ID, create_secret_dto=_provider_key_create() + ) + + # OpenAI -> Anthropic without a new key must never reuse the OpenAI credential. + update = UpdateSecretDTO( + secret={ + "kind": "provider_key", + "data": {"kind": "anthropic", "provider": {"key": ""}}, + }, + ) + + with pytest.raises(SecretValueRequiredError): + await service.update_secret( + secret_id=created.id, project_id=PROJECT_ID, update_secret_dto=update + ) + + +@pytest.mark.asyncio +async def test_provider_family_change_with_a_new_key_is_allowed(service): + created = await service.create_secret( + project_id=PROJECT_ID, create_secret_dto=_provider_key_create() + ) + + update = UpdateSecretDTO( + secret={ + "kind": "provider_key", + "data": {"kind": "anthropic", "provider": {"key": "sk-ant-new-key-123"}}, + }, + ) + updated = await service.update_secret( + secret_id=created.id, project_id=PROJECT_ID, update_secret_dto=update + ) + + assert updated.data.provider.key == "sk-ant-new-key-123" + + +@pytest.mark.asyncio +async def test_kind_change_with_omitted_content_is_rejected(service): + created = await service.create_secret( + project_id=PROJECT_ID, create_secret_dto=_provider_key_create() + ) + + # provider_key -> custom_secret with no content would irreversibly replace the + # stored credential with nothing. + update = UpdateSecretDTO( + secret={ + "kind": "custom_secret", + "data": {"secret": {"format": "text"}}, + }, + ) + + with pytest.raises(SecretValueRequiredError): + await service.update_secret( + secret_id=created.id, project_id=PROJECT_ID, update_secret_dto=update + ) + + +@pytest.mark.asyncio +async def test_a_format_change_with_omitted_content_is_rejected(service): + # text -> json with no content used to carry the stored STRING into the json shape. + # The payload validators never saw it (they ran before the carry-over filled it in), + # so an invalid row reached the database: a json secret holding a string. + created = await service.create_secret( + project_id=PROJECT_ID, + create_secret_dto=CreateSecretDTO( + header={"name": "Token"}, + secret={ + "kind": "custom_secret", + "data": { + "secret": {"format": "text", "content": "ghp_example_token_xyz"} + }, + }, + write_only=True, + ), + ) + + update = UpdateSecretDTO( + secret={"kind": "custom_secret", "data": {"secret": {"format": "json"}}}, + ) + + with pytest.raises(SecretValueRequiredError): + await service.update_secret( + secret_id=created.id, project_id=PROJECT_ID, update_secret_dto=update + ) + + stored = await service.get_secret_by_id(created.id, project_id=PROJECT_ID) + assert stored.data.secret.format.value == "text" + assert stored.data.secret.content == "ghp_example_token_xyz" + + +@pytest.mark.asyncio +async def test_a_format_change_with_a_new_value_is_allowed(service): + created = await service.create_secret( + project_id=PROJECT_ID, + create_secret_dto=CreateSecretDTO( + header={"name": "Token"}, + secret={ + "kind": "custom_secret", + "data": { + "secret": {"format": "text", "content": "ghp_example_token_xyz"} + }, + }, + write_only=True, + ), + ) + + updated = await service.update_secret( + secret_id=created.id, + project_id=PROJECT_ID, + update_secret_dto=UpdateSecretDTO( + secret={ + "kind": "custom_secret", + "data": { + "secret": {"format": "json", "content": {"token": "ghp-example"}} + }, + }, + ), + ) + + assert updated.data.secret.format.value == "json" + assert updated.data.secret.content == {"token": "ghp-example"} + + +@pytest.mark.asyncio +async def test_an_omitted_content_still_keeps_the_stored_one_within_a_format(service): + created = await service.create_secret( + project_id=PROJECT_ID, + create_secret_dto=CreateSecretDTO( + header={"name": "Token"}, + secret={ + "kind": "custom_secret", + "data": {"secret": {"format": "json", "content": {"token": "abc"}}}, + }, + write_only=True, + ), + ) + + updated = await service.update_secret( + secret_id=created.id, + project_id=PROJECT_ID, + update_secret_dto=UpdateSecretDTO( + secret={"kind": "custom_secret", "data": {"secret": {"format": "json"}}}, + ), + ) + + assert updated.data.secret.content == {"token": "abc"} + + +@pytest.mark.asyncio +async def test_family_change_does_not_carry_credential_extras(service): + created = await service.create_secret( + project_id=PROJECT_ID, + create_secret_dto=CreateSecretDTO( + header={"name": "my-gateway"}, + secret={ + "kind": "custom_provider", + "data": { + "kind": "openai", + "provider": { + "url": "https://gateway.example.com/v1", + "extras": {"api_key": "extra-key-123456"}, + }, + "models": [{"slug": "gpt-5"}], + }, + }, + ), + ) + + # Family change WITH an explicit new key: allowed, but the old family's extras + # credentials must not ride along. + update = UpdateSecretDTO( + secret={ + "kind": "custom_provider", + "data": { + "kind": "anthropic", + "provider": { + "url": "https://gateway.example.com/v1", + "key": "sk-ant-new-key-123", + }, + "models": [{"slug": "claude"}], + }, + }, + ) + updated = await service.update_secret( + secret_id=created.id, project_id=PROJECT_ID, update_secret_dto=update + ) + + assert updated.data.provider.key == "sk-ant-new-key-123" + assert not (updated.data.provider.extras or {}).get("api_key") + + +# --- redaction ------------------------------------------------------------------------- + + +def _response(kind, data, write_only=True): + return SecretResponseDTO( + id=uuid4(), + slug="s", + kind=kind, + data=data, + header={"name": "n"}, + write_only=write_only, + ) + + +def test_mask_boundaries_pin_the_preview_policy(): + # Under 20 characters: fully masked. From 20: at most 3+3, never more than 25%. + assert mask_secret_value("x" * 11) == "****" + assert mask_secret_value("x" * 12) == "****" + assert mask_secret_value("x" * 19) == "****" + assert mask_secret_value("abcdefghijklmnopqrst") == "abc****st" # 20 chars -> 5 + assert mask_secret_value("sk-example-credential9Qa") == "sk-****9Qa" # 24 -> 3+3 + assert mask_secret_value("x" * 400) == "xxx****xxx" # cap stays 3+3 + + +def test_redacts_provider_key_and_reports_presence(): + secret = _response( + "provider_key", + {"kind": "openai", "provider": {"key": "sk-test-openai-key-bc"}}, + ) + + redacted = redact_secret_response(secret) + + assert redacted.data.provider.key is None + assert redacted.value_status.configured is True + assert redacted.value_status.preview == "sk-****bc" + payload = redacted.model_dump(mode="json", exclude_none=True) + assert secret_value_configured(payload) is True + assert "has_key" not in payload + # The input is never mutated: internal readers keep their plaintext DTO. + assert secret.data.provider.key == "sk-test-openai-key-bc" + + +def test_redacts_custom_provider_key_and_credential_extras(): + secret = _response( + "custom_provider", + { + "kind": "openai", + "provider": { + "url": "https://gateway.example.com/v1", + "key": None, + "extras": {"api_key": "extra-key-123456", "region": "eu-west-1"}, + }, + "models": [{"slug": "gpt-5"}], + }, + ) + + redacted = redact_secret_response(secret) + + assert redacted.data.provider.key is None + assert "api_key" not in redacted.data.provider.extras + assert redacted.data.provider.extras["region"] == "eu-west-1" + assert redacted.data.provider.url == "https://gateway.example.com/v1" + assert redacted.value_status.configured is True + # Only the primary value field gets a preview; extras credentials never do. + assert redacted.value_status.preview is None + + +def test_redacts_every_sdk_credential_extras_key(): + # The classifier is shared with the SDK resolver, so everything the resolver would + # inject as a credential must come back stripped — including uppercase env-style + # keys and the bedrock/azure/anthropic tokens the first pass missed. + extras = { + "ANTHROPIC_AUTH_TOKEN": "tok-a", + "AWS_BEARER_TOKEN_BEDROCK": "tok-b", + "AWS_SECRET_ACCESS_KEY": "tok-c", + "AZURE_OPENAI_API_KEY": "tok-d", + "aws_bearer_token_bedrock": "tok-e", + "vertex_ai_credentials": '{"type": "service_account"}', + # Config survives. + "AWS_REGION": "eu-west-1", + "vertex_ai_project": "my-project", + } + secret = _response( + "custom_provider", + { + "kind": "bedrock", + "provider": {"url": None, "extras": dict(extras)}, + "models": [{"slug": "claude"}], + }, + ) + + redacted = redact_secret_response(secret) + + assert redacted.data.provider.extras == { + "AWS_REGION": "eu-west-1", + "vertex_ai_project": "my-project", + } + assert redacted.value_status.configured is True + assert redacted.value_status.preview is None + + +def test_aws_only_secret_reports_configured_true(): + secret = _response( + "custom_provider", + { + "kind": "bedrock", + "provider": { + "extras": { + "aws_access_key_id": "AKIA123", + "aws_secret_access_key": "shhh", + "aws_region_name": "eu-west-1", + } + }, + "models": [], + }, + ) + + redacted = redact_secret_response(secret) + + assert redacted.value_status.configured is True + assert "aws_secret_access_key" not in redacted.data.provider.extras + assert redacted.data.provider.extras["aws_region_name"] == "eu-west-1" + + +def test_redacts_sso_client_secret(): + secret = _response( + "sso_provider", + { + "provider": { + "client_id": "client-1", + "client_secret": "super-secret-value-123", + "issuer_url": "https://issuer.example.com", + "scopes": ["openid"], + } + }, + ) + + redacted = redact_secret_response(secret) + + assert redacted.data.provider.client_secret is None + assert redacted.data.provider.client_id == "client-1" + assert redacted.value_status.configured is True + + +def test_redacts_text_custom_secret_content(): + secret = _response( + "custom_secret", + {"secret": {"format": "text", "content": "ghp_example_token_xyz"}}, + ) + + redacted = redact_secret_response(secret) + + assert redacted.data.secret.content is None + assert redacted.value_status.configured is True + assert redacted.value_status.preview == "ghp****yz" + + +def test_redacts_json_custom_secret_without_a_preview(): + secret = _response( + "custom_secret", + {"secret": {"format": "json", "content": {"token": "abc", "user": "x"}}}, + ) + + redacted = redact_secret_response(secret) + + assert redacted.data.secret.content is None + assert redacted.value_status.configured is True + # A structured value has no single previewable string. + assert redacted.value_status.preview is None + + +def test_readable_secret_passes_through_unchanged(): + secret = _response( + "provider_key", + {"kind": "openai", "provider": {"key": "sk-test-openai-key-bc"}}, + write_only=False, + ) + + redacted = redact_secret_response(secret) + + assert redacted is not secret + assert redacted.data.provider.key == "sk-test-openai-key-bc" + assert redacted.value_status.configured is True + assert redacted.value_status.preview is None + + +def test_write_only_without_a_value_reports_configured_false(): + secret = _response( + "custom_provider", + { + "kind": "openai", + "provider": {"url": "https://gateway.example.com/v1"}, + "models": [], + }, + ) + + redacted = redact_secret_response(secret) + + assert redacted.value_status.configured is False + assert redacted.value_status.preview is None + + +# --- postgres mappings ----------------------------------------------------------------- + + +def test_mapping_round_trips_the_flag_through_the_data_json(): + import json + + dbe = map_secrets_dto_to_dbe( + project_id=PROJECT_ID, + organization_id=None, + secret_dto=_provider_key_create(write_only=True), + ) + + stored = json.loads(dbe.data) + assert stored["write_only"] is True + assert stored["provider"]["key"] == "sk-test-openai-key-bc" + + dbe.id = uuid4() + dto = map_secrets_dbe_to_dto(secrets_dbe=dbe) + + assert dto.write_only is True + # The flag never leaks into the payload shape. + assert not hasattr(dto.data, "write_only") + assert dto.data.provider.key == "sk-test-openai-key-bc" + + +def test_mapping_reads_legacy_rows_as_readable(): + dbe = map_secrets_dto_to_dbe( + project_id=PROJECT_ID, + organization_id=None, + secret_dto=_provider_key_create(write_only=False), + ) + dbe.id = uuid4() + + assert map_secrets_dbe_to_dto(secrets_dbe=dbe).write_only is False + + +def test_update_mapping_preserves_the_stored_flag_when_unspecified(): + import json + + dbe = map_secrets_dto_to_dbe( + project_id=PROJECT_ID, + organization_id=None, + secret_dto=_provider_key_create(write_only=True), + ) + + map_secrets_dto_to_dbe_update( + secrets_dbe=dbe, + update_secret_dto=UpdateSecretDTO( + secret={ + "kind": "provider_key", + "data": { + "kind": "openai", + "provider": {"key": "sk-test-rotated"}, + }, + }, + ), + ) + + stored = json.loads(dbe.data) + assert stored["write_only"] is True + assert stored["provider"]["key"] == "sk-test-rotated" + + +# --- the update-path payload type, at every call site ---------------------------------- + + +def test_update_call_sites_build_the_update_path_payload(): + """`UpdateSecretDTO.secret` is `UpdateSecretPayloadDTO`, and pydantic rejects the + parent `SecretDTO` there — a caller that builds the parent breaks that write path at + runtime, not at import time. This walks the source so a new call site cannot + reintroduce the mismatch (it caught webhook secret rotation and SSO provider updates). + """ + import ast + from pathlib import Path + + source_roots = [ + Path(__file__).resolve().parents[5] / "oss" / "src", + Path(__file__).resolve().parents[5] / "ee" / "src", + ] + + offenders = [] + + for root in source_roots: + if not root.exists(): # EE is absent in an OSS-only checkout. + continue + + for path in root.rglob("*.py"): + tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + if getattr(node.func, "id", None) != "UpdateSecretDTO": + continue + + for keyword in node.keywords: + if keyword.arg != "secret": + continue + if not isinstance(keyword.value, ast.Call): + continue # a dict or a variable: validated by pydantic as data. + + built = getattr(keyword.value.func, "id", None) + if built != "UpdateSecretPayloadDTO": + offenders.append(f"{path}:{node.lineno} builds {built}") + + assert not offenders, ( + "UpdateSecretDTO(secret=...) must be built with UpdateSecretPayloadDTO: " + + "; ".join(offenders) + ) + + +def test_primary_credential_fields_cover_every_secret_kind(): + # The redaction, the presence report, and the carry-over all key off this map, so a + # kind missing from it silently stops being redacted. + from oss.src.core.secrets.redaction import PRIMARY_CREDENTIAL_FIELDS + + assert set(PRIMARY_CREDENTIAL_FIELDS) == { + "provider_key", + "custom_provider", + "webhook_provider", + "sso_provider", + "custom_secret", + } diff --git a/api/oss/tests/pytest/unit/utils/test_env_helpers.py b/api/oss/tests/pytest/unit/utils/test_env_helpers.py new file mode 100644 index 0000000000..325031864d --- /dev/null +++ b/api/oss/tests/pytest/unit/utils/test_env_helpers.py @@ -0,0 +1,63 @@ +"""Startup validation for a misconfiguration the runtime cannot report itself. + +A deployment using write-only secrets needs a platform runtime key, or runs cannot read +the secrets they were authorized to use. That failure surfaces as advice about provider +keys, which is right for a standalone run and useless here, so it has to be said at boot. +""" + +import pytest +import oss.src.utils.env as env_module + +from oss.src.utils.env import env +from oss.src.utils.helpers import validate_platform_runtime_key + + +def _configure(monkeypatch, *, runtime_key): + monkeypatch.setattr(env.agenta, "services_internal_key", runtime_key) + + +@pytest.mark.parametrize("runtime_key", ["", "replace-me"]) +def test_deployments_without_a_runtime_key_fail_startup(monkeypatch, runtime_key): + _configure(monkeypatch, runtime_key=runtime_key) + + with pytest.raises(RuntimeError, match="AGENTA_SERVICES_INTERNAL_KEY"): + validate_platform_runtime_key() + + +def test_a_configured_deployment_passes_validation(monkeypatch): + _configure(monkeypatch, runtime_key="a-real-runtime-key") + + validate_platform_runtime_key() + + +def test_the_validation_does_not_depend_on_a_feature_gate(monkeypatch): + _configure(monkeypatch, runtime_key="") + + with pytest.raises(RuntimeError, match="AGENTA_SERVICES_INTERNAL_KEY"): + validate_platform_runtime_key() + + +def test_runtime_key_does_not_fall_back_to_the_admin_key(monkeypatch): + monkeypatch.delenv("AGENTA_SERVICES_INTERNAL_KEY", raising=False) + monkeypatch.setenv("AGENTA_AUTH_KEY", "administrator-key") + + assert env_module._services_internal_key_from_environment() is None + + +@pytest.mark.parametrize( + ("configured", "expected"), + [ + (None, None), + ("", None), + (" ", None), + ("replace-me", None), + (" runtime-key ", "runtime-key"), + ], +) +def test_runtime_key_configuration_is_normalized(monkeypatch, configured, expected): + if configured is None: + monkeypatch.delenv("AGENTA_SERVICES_INTERNAL_KEY", raising=False) + else: + monkeypatch.setenv("AGENTA_SERVICES_INTERNAL_KEY", configured) + + assert env_module._services_internal_key_from_environment() == expected diff --git a/api/oss/tests/pytest/unit/vault/test_write_only_routes.py b/api/oss/tests/pytest/unit/vault/test_write_only_routes.py new file mode 100644 index 0000000000..9986821375 --- /dev/null +++ b/api/oss/tests/pytest/unit/vault/test_write_only_routes.py @@ -0,0 +1,450 @@ +"""Every vault route redacts write-only values for users; only the runtime grant reads them. + +Drives the real `VaultRouter` + `VaultService` over an in-memory DAO, with the permission +check monkeypatched. The caller's principal is simulated by a test +middleware: requests with the `x-test-grant` header carry the secret-resolve grant (the +platform runtime); requests without it are ordinary user principals (session/ApiKey). +""" + +from datetime import datetime, timedelta, timezone +from uuid import uuid4 + +import pytest +from jwt import encode +from fastapi import FastAPI, HTTPException +from fastapi.responses import JSONResponse +from fastapi.testclient import TestClient + +import oss.src.middlewares.auth as auth_module +from oss.src.apis.fastapi.vault import router as vault_router_module +from oss.src.apis.fastapi.vault.router import VaultRouter +from oss.src.core.secrets.dtos import SecretResponseDTO +from oss.src.core.secrets.services import VaultService +from oss.src.middlewares.auth import SECRET_RESOLVE_GRANT + + +PROJECT_ID = str(uuid4()) +USER_ID = str(uuid4()) + +KEY = "sk-test-openai-secret" + + +class _FakeSecretsDAO: + def __init__(self): + self.records: dict = {} + + async def create(self, project_id, organization_id, create_secret_dto): + record = SecretResponseDTO( + id=uuid4(), + slug=create_secret_dto.slug, + kind=create_secret_dto.secret.kind, + data=create_secret_dto.secret.data.model_dump(exclude_none=True), + header=create_secret_dto.header, + write_only=bool(create_secret_dto.write_only), + ) + self.records[str(record.id)] = record + return record + + async def list(self, project_id, organization_id): + return list(self.records.values()) + + async def get_by_id(self, secret_id, project_id, organization_id): + return self.records.get(str(secret_id)) + + async def get_by_slug(self, secret_slug, project_id, organization_id): + return next( + (r for r in self.records.values() if r.slug == secret_slug), + None, + ) + + async def update( + self, + secret_id, + update_secret_dto, + project_id, + organization_id, + user_id=None, + resolve_update=None, + ): + stored = self.records.get(str(secret_id)) + if stored is None: + return None + + # Production resolves the update against the row under the write lock; the fake + # does the same at the same point, so keep-on-omit is exercised, not skipped. + if resolve_update is not None: + update_secret_dto = resolve_update(stored, update_secret_dto) + + updated = stored.model_copy( + update={ + "header": update_secret_dto.header or stored.header, + } + ) + if update_secret_dto.secret is not None: + updated.kind = update_secret_dto.secret.kind + updated.data = update_secret_dto.secret.data + + self.records[str(secret_id)] = updated + return updated + + async def delete( + self, secret_id, project_id, organization_id, authorize_delete=None + ): + stored = self.records.get(str(secret_id)) + if stored is not None and authorize_delete is not None: + authorize_delete(stored) + self.records.pop(str(secret_id), None) + + +@pytest.fixture(name="harness") +def _harness(monkeypatch): + dao = _FakeSecretsDAO() + + async def _allow(**kwargs): + return True + + monkeypatch.setattr(vault_router_module, "check_action_access", _allow) + + app = FastAPI() + + @app.middleware("http") + async def _principal(request, call_next): + request.state.user_id = USER_ID + request.state.project_id = PROJECT_ID + if request.headers.get("x-test-grant"): + request.state.token_grants = (SECRET_RESOLVE_GRANT,) + return await call_next(request) + + app.include_router(VaultRouter(vault_service=VaultService(dao)).router) + + return TestClient(app) + + +GRANT = {"x-test-grant": "1"} + + +def _create(client, write_only=None, key=KEY): + body = { + "header": {"name": "OpenAI"}, + "secret": { + "kind": "provider_key", + "data": {"kind": "openai", "provider": {"key": key}}, + }, + } + if write_only is not None: + body["write_only"] = write_only + response = client.post("/secrets/", json=body) + assert response.status_code == 200, response.text + return response.json() + + +def test_create_echo_is_redacted_for_a_write_only_secret(harness): + client = harness + + created = _create(client, write_only=True) + + assert created["write_only"] is True + assert "key" not in created["data"]["provider"] + assert created["value_status"]["configured"] is True + assert created["value_status"]["preview"] == "sk-****et" + assert KEY not in str(created) + + +def test_create_without_the_flag_defaults_to_write_only(harness): + client = harness + + created = _create(client) + + assert created["write_only"] is True + assert "key" not in created["data"]["provider"] + + +def test_create_with_explicit_false_keeps_todays_response(harness): + client = harness + + created = _create(client, write_only=False) + + assert created["write_only"] is False + assert created["data"]["provider"]["key"] == KEY + assert created["value_status"]["configured"] is True + assert "preview" not in created["value_status"] + + +def test_read_is_redacted_for_users_and_plaintext_for_the_grant(harness): + client = harness + created = _create(client, write_only=True) + + user_read = client.get(f"/secrets/{created['id']}") + assert user_read.status_code == 200 + assert "key" not in user_read.json()["data"]["provider"] + + runtime_read = client.get(f"/secrets/{created['id']}", headers=GRANT) + assert runtime_read.status_code == 200 + assert runtime_read.json()["data"]["provider"]["key"] == KEY + + +def test_list_is_redacted_for_users(harness): + client = harness + _create(client, write_only=True) + + listed = client.get("/secrets/") + assert listed.status_code == 200 + (secret,) = listed.json() + assert "key" not in secret["data"]["provider"] + assert secret["value_status"]["configured"] is True + assert KEY not in listed.text + + +def test_grant_list_gets_plaintext(harness): + client = harness + _create(client, write_only=True) + + runtime_list = client.get("/secrets/", headers=GRANT) + assert runtime_list.status_code == 200 + (secret,) = runtime_list.json() + assert secret["data"]["provider"]["key"] == KEY + + +def test_update_echo_is_redacted_and_omitted_key_keeps_the_stored_value(harness): + client = harness + created = _create(client, write_only=True) + + updated = client.put( + f"/secrets/{created['id']}", + json={ + "header": {"name": "OpenAI (renamed)"}, + "secret": { + "kind": "provider_key", + "data": {"kind": "openai", "provider": {}}, + }, + }, + ) + assert updated.status_code == 200, updated.text + assert "key" not in updated.json()["data"]["provider"] + assert updated.json()["header"]["name"] == "OpenAI (renamed)" + + # The grant read proves the stored value survived the value-less update. + runtime_read = client.get(f"/secrets/{created['id']}", headers=GRANT) + assert runtime_read.json()["data"]["provider"]["key"] == KEY + + +def test_explicit_empty_string_key_is_rejected(harness): + client = harness + created = _create(client, write_only=True) + + updated = client.put( + f"/secrets/{created['id']}", + json={ + "header": {"name": "OpenAI (edited in today's UI)"}, + "secret": { + "kind": "provider_key", + "data": {"kind": "openai", "provider": {"key": ""}}, + }, + }, + ) + assert updated.status_code == 400, updated.text + + runtime_read = client.get(f"/secrets/{created['id']}", headers=GRANT) + assert runtime_read.json()["data"]["provider"]["key"] == KEY + + +def test_write_only_cannot_be_disabled_over_the_api(harness): + client = harness + created = _create(client, write_only=True) + + response = client.put(f"/secrets/{created['id']}", json={"write_only": False}) + + assert response.status_code == 422 + assert "cannot be updated" in response.text + + +def test_readable_secret_lists_with_its_value_as_today(harness): + client = harness + _create(client, write_only=False) + + (secret,) = client.get("/secrets/").json() + + assert secret["data"]["provider"]["key"] == KEY + assert secret["write_only"] is False + + +def test_delete_still_works(harness): + client = harness + created = _create(client, write_only=True) + + assert client.delete(f"/secrets/{created['id']}").status_code == 204 + assert client.get(f"/secrets/{created['id']}").status_code == 404 + + +def test_kind_or_family_change_without_a_new_value_is_400(harness): + client = harness + created = _create(client, write_only=True) + + response = client.put( + f"/secrets/{created['id']}", + json={ + "secret": { + "kind": "provider_key", + "data": {"kind": "anthropic", "provider": {}}, + } + }, + ) + + assert response.status_code == 400 + assert "credential value" in response.json()["detail"] + + +def test_explicit_blank_credential_has_a_clear_400(harness): + client = harness + created = _create(client, write_only=True) + + response = client.put( + f"/secrets/{created['id']}", + json={ + "secret": { + "kind": "provider_key", + "data": {"kind": "openai", "provider": {"key": ""}}, + } + }, + ) + + assert response.status_code == 400 + assert response.json()["detail"] == ( + "Credential values cannot be blank. Omit an unchanged credential field or provide a " + "new value." + ) + + +# --- real signed tokens through the real verifier -------------------------------------- + +SECRET_KEY = "unit-test-secret-key-with-32-bytes" + + +@pytest.fixture(name="token_client") +def _token_client(monkeypatch): + """Same router harness, but the principal comes from a REAL `Secret` token run + through the real `verify_secret_token` — nothing injects `token_grants` directly.""" + monkeypatch.setattr(auth_module, "_SECRET_KEY", SECRET_KEY) + + dao = _FakeSecretsDAO() + + async def _allow(**kwargs): + return True + + monkeypatch.setattr(vault_router_module, "check_action_access", _allow) + + app = FastAPI() + + @app.middleware("http") + async def _authenticate(request, call_next): + header = request.headers.get("authorization", "") + if not header.startswith("Secret "): + return JSONResponse({"detail": "Unauthorized"}, status_code=401) + try: + await auth_module.verify_secret_token(request, header[len("Secret ") :]) + except HTTPException as exc: + return JSONResponse({"detail": "Unauthorized"}, status_code=exc.status_code) + return await call_next(request) + + app.include_router(VaultRouter(vault_service=VaultService(dao)).router) + + return TestClient(app) + + +def _auth(token): + return {"Authorization": f"Secret {token}"} + + +@pytest.mark.asyncio +async def test_real_tokens_grant_and_deny_plaintext(token_client): + plain = await auth_module.sign_secret_token(user_id=USER_ID, project_id=PROJECT_ID) + granted = await auth_module.sign_secret_token( + user_id=USER_ID, + project_id=PROJECT_ID, + grants=[SECRET_RESOLVE_GRANT], + ) + + created = token_client.post( + "/secrets/", + json={ + "header": {"name": "OpenAI"}, + "secret": { + "kind": "provider_key", + "data": {"kind": "openai", "provider": {"key": KEY}}, + }, + "write_only": True, + }, + headers=_auth(plain), + ) + assert created.status_code == 200, created.text + assert "key" not in created.json()["data"]["provider"] + secret_id = created.json()["id"] + + ungranted_read = token_client.get(f"/secrets/{secret_id}", headers=_auth(plain)) + assert ungranted_read.status_code == 200 + assert "key" not in ungranted_read.json()["data"]["provider"] + + granted_read = token_client.get(f"/secrets/{secret_id}", headers=_auth(granted)) + assert granted_read.status_code == 200 + assert granted_read.json()["data"]["provider"]["key"] == KEY + + granted_list = token_client.get("/secrets/", headers=_auth(granted)) + (listed,) = granted_list.json() + assert listed["data"]["provider"]["key"] == KEY + + +@pytest.mark.asyncio +async def test_expired_granted_token_is_rejected(token_client): + # Encoded here rather than through `sign_secret_token`, which only ever issues a live + # token; what is under test is the middleware refusing an expired one, grant or not. + expired = encode( + payload={ + "user_id": USER_ID, + "project_id": PROJECT_ID, + "grants": [SECRET_RESOLVE_GRANT], + "exp": int( + (datetime.now(timezone.utc) - timedelta(seconds=60)).timestamp() + ), + }, + key=auth_module._SECRET_KEY, + algorithm="HS256", + ) + + response = token_client.get("/secrets/", headers=_auth(expired)) + + assert response.status_code == 401 + + +# --- schema + never-echo --------------------------------------------------------------- + + +def test_openapi_documents_the_write_only_contract(harness): + client = harness + + schemas = client.app.openapi()["components"]["schemas"] + + for field in ("write_only", "value_status"): + assert field in schemas["PublicSecretResponseDTO"]["properties"] + assert "write_only" in schemas["CreateSecretDTO"]["properties"] + assert "write_only" not in schemas["UpdateSecretDTO"]["properties"] + + +CANARY = "sk-CANARY-DO-NOT-ECHO-abc123" + + +def test_malformed_create_never_echoes_the_submitted_key(harness): + client = harness + + response = client.post( + "/secrets/", + json={ + "header": {"name": "x"}, + "secret": { + "kind": "invalid_kind", + "data": {"kind": "openai", "provider": {"key": CANARY}}, + }, + }, + ) + + assert response.status_code == 422 + assert CANARY not in response.text diff --git a/api/oss/tests/pytest/unit/webhooks/test_write_only_outward.py b/api/oss/tests/pytest/unit/webhooks/test_write_only_outward.py new file mode 100644 index 0000000000..ea180b43ea --- /dev/null +++ b/api/oss/tests/pytest/unit/webhooks/test_write_only_outward.py @@ -0,0 +1,214 @@ +"""Webhook signing secrets explicitly remain readable.""" + +from uuid import uuid4 + +import pytest + +from oss.src.core.secrets.dtos import SecretResponseDTO +from oss.src.core.secrets.services import VaultService +from oss.src.core.webhooks.service import WebhooksService +from oss.src.core.webhooks.types import ( + WebhookSubscription, + WebhookSubscriptionCreate, + WebhookSubscriptionData, + WebhookSubscriptionEdit, +) + + +PROJECT_ID = uuid4() +USER_ID = uuid4() + + +class _FakeSecretsDAO: + def __init__(self): + self.records: dict = {} + + async def create(self, project_id, organization_id, create_secret_dto): + record = SecretResponseDTO( + id=uuid4(), + slug=create_secret_dto.slug, + kind=create_secret_dto.secret.kind, + data=create_secret_dto.secret.data.model_dump(exclude_none=True), + header=create_secret_dto.header, + write_only=bool(create_secret_dto.write_only), + ) + self.records[record.id] = record + return record + + async def get_by_id(self, secret_id, project_id, organization_id): + return self.records.get(secret_id) + + async def list(self, project_id, organization_id): + return list(self.records.values()) + + async def update( + self, + secret_id, + update_secret_dto, + project_id, + organization_id, + user_id=None, + resolve_update=None, + ): + stored = self.records.get(secret_id) + if stored is None: + return None + + # Production resolves the update against the row under the write lock; the fake + # does the same at the same point, so keep-on-omit is exercised, not skipped. + if resolve_update is not None: + update_secret_dto = resolve_update(stored, update_secret_dto) + updated = stored.model_copy() + if update_secret_dto.secret is not None: + updated.data = update_secret_dto.secret.data + self.records[secret_id] = updated + return updated + + +class _FakeWebhooksDAO: + def __init__(self): + self.subscriptions: dict = {} + + async def create_subscription( + self, *, project_id, user_id, subscription, secret_id + ): + record = WebhookSubscription( + id=uuid4(), + name=subscription.name, + data=subscription.data, + secret_id=secret_id, + ) + self.subscriptions[record.id] = record + return record + + async def fetch_subscription(self, *, project_id, subscription_id): + return self.subscriptions.get(subscription_id) + + async def edit_subscription(self, *, project_id, user_id, subscription, secret_id): + record = self.subscriptions.get(subscription.id) + if record is None: + return None + record = record.model_copy( + update={ + "name": subscription.name or record.name, + "data": subscription.data, + "secret_id": secret_id or record.secret_id, + } + ) + self.subscriptions[record.id] = record + return record + + +@pytest.fixture(name="services") +def _services(): + secrets_dao = _FakeSecretsDAO() + vault_service = VaultService(secrets_dao) + webhooks_service = WebhooksService( + webhooks_dao=_FakeWebhooksDAO(), + vault_service=vault_service, + ) + return webhooks_service, vault_service + + +def _subscription_create(): + return WebhookSubscriptionCreate( + name="notify", + data=WebhookSubscriptionData(url="https://example.com/hook"), + secret="whsec_provided_by_user_12345", + ) + + +@pytest.mark.asyncio +async def test_webhook_signing_secret_is_explicitly_readable(services): + webhooks_service, vault_service = services + + created = await webhooks_service.create_subscription( + project_id=PROJECT_ID, + user_id=USER_ID, + subscription=_subscription_create(), + ) + assert created.secret == "whsec_provided_by_user_12345" + + stored = await webhooks_service.dao.fetch_subscription( + project_id=PROJECT_ID, + subscription_id=created.id, + ) + secret_dto = await vault_service.get_secret_by_id( + project_id=PROJECT_ID, + secret_id=stored.secret_id, + ) + assert secret_dto.write_only is False + + fetched = await webhooks_service.fetch_subscription( + project_id=PROJECT_ID, + subscription_id=created.id, + ) + assert fetched.secret == "whsec_provided_by_user_12345" + + +@pytest.mark.asyncio +async def test_generated_secret_is_returned_on_the_create_echo(services): + # The create echo is the ONLY place a caller can read a secret Agenta generated for + # them. Redacting it would ship a subscription that can never be verified. + webhooks_service, _ = services + + created = await webhooks_service.create_subscription( + project_id=PROJECT_ID, + user_id=USER_ID, + subscription=WebhookSubscriptionCreate( + name="notify", + data=WebhookSubscriptionData(url="https://example.com/hook"), + ), + ) + + assert created.secret + + stored = await webhooks_service.dao.fetch_subscription( + project_id=PROJECT_ID, subscription_id=created.id + ) + signing_value = await webhooks_service._resolve_secret( + project_id=PROJECT_ID, + secret_id=stored.secret_id, + ) + + # What the subscriber was handed is what we sign with. + assert created.secret == signing_value + + +@pytest.mark.asyncio +async def test_rotating_the_signing_secret_through_edit_replaces_the_stored_value( + services, +): + # The rotation path builds an update-path payload DTO; the parent `SecretDTO` does not + # validate there, so this pins that editing a subscription's secret still works. + webhooks_service, _ = services + + created = await webhooks_service.create_subscription( + project_id=PROJECT_ID, + user_id=USER_ID, + subscription=_subscription_create(), + ) + + edited = await webhooks_service.edit_subscription( + project_id=PROJECT_ID, + user_id=USER_ID, + subscription=WebhookSubscriptionEdit( + id=created.id, + name="notify", + data=WebhookSubscriptionData(url="https://example.com/hook"), + secret="whsec_test_rotated", + ), + ) + + assert edited is not None + + stored = await webhooks_service.dao.fetch_subscription( + project_id=PROJECT_ID, subscription_id=created.id + ) + signing_value = await webhooks_service._resolve_secret( + project_id=PROJECT_ID, + secret_id=stored.secret_id, + ) + + assert signing_value == "whsec_test_rotated" + assert edited.secret == "whsec_test_rotated" diff --git a/docs/design/write-only-secrets/README.md b/docs/design/write-only-secrets/README.md new file mode 100644 index 0000000000..26ae675981 --- /dev/null +++ b/docs/design/write-only-secrets/README.md @@ -0,0 +1,149 @@ +# Write-only vault secrets + +A write-only Vault secret can be created, replaced, and deleted, but an ordinary API or +frontend caller cannot read its value back. A trusted platform runtime can resolve the value +through a short-lived granted token so the credential remains usable for runs. + +Status: implementation complete across #6164, #6165, #6138, #6195, and #6174. The backend +and frontend ship in the same release. The feature has no environment gate or compatibility +mode. + +## Value visibility + +`write_only` is a creation-time policy: + +- New ordinary Vault secrets default to `write_only=True`. +- A create request may explicitly select `write_only=False`. +- Existing rows without the stored field resolve as `write_only=False`. +- Update requests cannot set or change `write_only`. +- Changing the policy requires deleting and recreating the secret. + +The policy remains in the existing encrypted `data` JSON. This implementation requires no +database migration. + +SSO and webhook secrets explicitly use `write_only=False`. Their existing settings, login, +test, signing, and verification flows continue to receive the readable value. + +## Public and trusted responses + +The trusted `SecretResponseDTO` contains the stored credential. The caller-facing +`PublicSecretResponseDTO` adds general value status: + +```json +{ + "write_only": true, + "value_status": { + "configured": true, + "preview": "sk-****9Qa" + } +} +``` + +`value_status.configured` reports whether the secret contains credential material. +`value_status.preview` is optional and does not determine whether a value is configured. +The model applies to provider keys, custom secrets, compound provider credentials, SSO, +webhooks, and later secret kinds. + +For an ordinary caller, the response projection removes the primary credential and every +credential-bearing provider extra when `write_only=True`. Non-secret configuration such as +URLs, regions, versions, models, and harnesses remains readable. A verified runtime token +carrying the `secret-resolve` grant receives the plaintext projection. + +The credential-extra vocabulary is shared by the API redaction layer and Python SDK resolver +through `agenta.sdk.agents.connections.credentials`. The same module exposes the SDK helper +that reads `value_status.configured`, so runtime consumers use the public API contract rather +than a key-specific field. + +## Vault list cache + +The Vault service caches canonical trusted DTOs in Redis with the existing namespace, TTL, +invalidation, and shortened project UUID packing. Create, update, and delete invalidate the +project list namespace. + +Caller-specific projection always happens after a cache read: + +```text +list request + -> read canonical SecretResponseDTO values from Redis or the DAO + -> inspect the verified caller grant + -> return plaintext to a granted runtime or redact for an ordinary caller +``` + +Redis is part of the trusted backend boundary. A redacted response is never stored in the +shared list cache. + +## Updates + +Credential updates use one strict contract: + +- An omitted credential field means keep the stored value. +- A non-empty credential replaces the stored value. +- An explicitly supplied empty provider credential is invalid and is rejected. +- An empty JSON object remains an explicitly supplied JSON value and follows the custom-secret + validation rules. + +The frontend ships with the backend and follows the same contract. When a user edits a +write-only connection without typing a replacement credential, the frontend omits the +credential field. It never sends an empty value as the keep signal. + +Carry-over is identity-local. Changing the secret kind, provider family, or custom-secret +format requires an explicit new credential. The service resolves the update against the row +loaded under `SELECT ... FOR UPDATE`, so a concurrent rotation cannot be overwritten by a +stale carried value. + +## Runtime resolution + +The platform uses the `secret-resolve` grant for plaintext runtime access. The grant is +project-scoped and allowlisted on the Vault read routes. + +`AGENTA_SERVICES_INTERNAL_KEY` proves the trusted API-to-Services exchange. It is independent +from `AGENTA_AUTH_KEY`, has no administrator-key fallback, and must contain a non-placeholder +value. The API fails startup when it is missing or invalid. + +Configure the same `AGENTA_SERVICES_INTERNAL_KEY` value on the API and trusted Services +container. Do not provide it to the web app, runner, sandbox, worker, cron, or migration +containers. The runner receives only the signed, short-lived runtime token needed to execute +the authorized workload. + +A caller refreshing an already granted Secret token may carry the grant forward. The exchange +does not create the grant from a requested action alone. + +## Consumer behavior + +| Consumer | Behavior | +| --- | --- | +| Frontend Settings and connection forms | Read `value_status`, display configured state, and omit untouched credential fields on update. | +| Direct API callers | Receive redacted write-only values and general value status. | +| Platform runs | Resolve plaintext through the granted runtime token. | +| Standalone Python SDK | Reads `value_status`; a redacted provider connection uses only the matching provider environment credential or fails with `WriteOnlySecretError`. | +| Legacy SDK Vault middleware | Drops configured redacted entries so they cannot shadow local environment credentials. | +| Named tool secrets | Skip configured redacted values and log an error without the secret name or value. | +| SSO and webhooks | Remain explicitly readable and otherwise unchanged. | +| In-process trusted readers | Continue using the trusted DTO with plaintext. | + +## Managed secrets + +Managed lifecycle and value visibility are independent. A managed row stores typed internal +manager identity and policy in the encrypted JSON payload. The public response exposes only +`management.policy`. + +The starter-credit bridge explicitly creates its connection with +`management.policy=manager_only` and `write_only=True`. General update, delete, and provider +probe operations reject manager-only rows against the current locked record. The frontend +hides these rows from Settings and edit surfaces but keeps them available for model selection, +agent defaults, key gating, and execution. + +## Pull request order + +The release chain is: + +```text +release/v0.114.0 + -> #6164 write-only contract + -> #6165 managed-secret model + -> #6138 starter-credit seeding + -> #6195 stored provider probe + -> #6174 generated clients and frontend consumer +``` + +The five pull requests merge in this order and deploy together in the same release. diff --git a/hosting/docker-compose/ee/docker-compose.dev.yml b/hosting/docker-compose/ee/docker-compose.dev.yml index 39f02b6e9d..8663a2ea60 100644 --- a/hosting/docker-compose/ee/docker-compose.dev.yml +++ b/hosting/docker-compose/ee/docker-compose.dev.yml @@ -73,6 +73,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" DOCKER_NETWORK_MODE: ${DOCKER_NETWORK_MODE:-bridge} WATCHPACK_POLLING: "true" AGENTA_MOBILE_GATE: ${AGENTA_MOBILE_GATE:-true} @@ -108,6 +109,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" DOCKER_NETWORK_MODE: ${DOCKER_NETWORK_MODE:-bridge} WATCHPACK_POLLING: "true" AGENTA_MOBILE_GATE: ${AGENTA_MOBILE_GATE:-true} @@ -180,6 +182,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: ${AGENTA_SERVICES_INTERNAL_KEY:?AGENTA_SERVICES_INTERNAL_KEY is required} DOCKER_NETWORK_MODE: ${DOCKER_NETWORK_MODE:-bridge} # kill's direct runner hop (W7.3, sandbox teardown) — same compose-network # address + shared token as the `services` container's runner_url(). @@ -240,6 +243,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" DOCKER_NETWORK_MODE: ${DOCKER_NETWORK_MODE:-bridge} # empty ⇒ all stream loops; see docs/designs/workers-sprawl/specs.md AGENTA_WORKER_STREAMS: "" @@ -289,6 +293,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" DOCKER_NETWORK_MODE: ${DOCKER_NETWORK_MODE:-bridge} # empty ⇒ all queue loops; see docs/designs/workers-sprawl/specs.md AGENTA_WORKER_QUEUES: "" @@ -333,6 +338,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" DOCKER_NETWORK_MODE: ${DOCKER_NETWORK_MODE:-bridge} # === NETWORK ============================================== # networks: @@ -362,6 +368,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" DOCKER_NETWORK_MODE: ${DOCKER_NETWORK_MODE:-bridge} # === NETWORK ============================================== # networks: @@ -412,6 +419,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: ${AGENTA_SERVICES_INTERNAL_KEY:?AGENTA_SERVICES_INTERNAL_KEY is required} DOCKER_NETWORK_MODE: ${DOCKER_NETWORK_MODE:-bridge} AGENTA_RUNNER_INTERNAL_URL: ${AGENTA_RUNNER_INTERNAL_URL:-http://runner:8765} AGENTA_RUNNER_TOKEN: ${AGENTA_RUNNER_TOKEN:?AGENTA_RUNNER_TOKEN is required} @@ -550,6 +558,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" POSTGRES_USER: ${POSTGRES_USER:-username} POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-password} # === NETWORK ============================================== # @@ -653,6 +662,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # The entrypoint bakes these into s3.json/iam.json via raw shell expansion, so unlike # the API (which reads code defaults from env.py) they must be present in this service's # env. Defaults live here for the bundled store the same way supertokens carries its own @@ -760,6 +770,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" POSTGRESQL_CONNECTION_URI: ${POSTGRES_URI_SUPERTOKENS:-postgresql://username:password@postgres:5432/agenta_ee_supertokens} # === NETWORK ============================================== # networks: @@ -794,6 +805,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.ee.dev} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-network @@ -819,6 +832,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" AGENTA_INGRESS_URL: http://api:8000/triggers/composio/events/ PYTHONUNBUFFERED: "1" # === NETWORK ============================================== # diff --git a/hosting/docker-compose/ee/docker-compose.gh.local.yml b/hosting/docker-compose/ee/docker-compose.gh.local.yml index 80613e841a..41bb7f150e 100644 --- a/hosting/docker-compose/ee/docker-compose.gh.local.yml +++ b/hosting/docker-compose/ee/docker-compose.gh.local.yml @@ -14,6 +14,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.ee.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-ee-gh-network @@ -41,6 +43,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.ee.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-ee-gh-network @@ -79,6 +83,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY=${AGENTA_SERVICES_INTERNAL_KEY:?AGENTA_SERVICES_INTERNAL_KEY is required} - SCRIPT_NAME=/api # Unset leaves the code default (3600s) in place. - AGENTA_MOUNTS_CREDENTIALS_TTL_SECONDS=${AGENTA_MOUNTS_CREDENTIALS_TTL_SECONDS:-} @@ -130,6 +135,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY= # empty ⇒ all stream loops; see docs/designs/workers-sprawl/specs.md - AGENTA_WORKER_STREAMS= # === NETWORK ============================================== # @@ -174,6 +180,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY= # empty ⇒ all queue loops; see docs/designs/workers-sprawl/specs.md - AGENTA_WORKER_QUEUES= # === NETWORK ============================================== # @@ -211,6 +218,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.ee.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-ee-gh-network @@ -233,6 +242,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.ee.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-ee-gh-network @@ -266,6 +277,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY=${AGENTA_SERVICES_INTERNAL_KEY:?AGENTA_SERVICES_INTERNAL_KEY is required} - SCRIPT_NAME=/services - AGENTA_RUNNER_INTERNAL_URL=${AGENTA_RUNNER_INTERNAL_URL:-http://runner:8765} - AGENTA_RUNNER_TOKEN=${AGENTA_RUNNER_TOKEN:?AGENTA_RUNNER_TOKEN is required} @@ -364,6 +376,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.gh} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" POSTGRES_USER: ${POSTGRES_USER:-username} POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-password} # === NETWORK ============================================== # @@ -467,6 +480,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.gh} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # The entrypoint bakes these into s3.json/iam.json via raw shell expansion, so they must # be present in this service's env (defaults here are for the bundled store, the same way # supertokens carries its own connection URI). Keys come from the env file — secrets never @@ -514,6 +528,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.ee.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-ee-gh-network @@ -530,6 +546,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.gh} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" POSTGRESQL_CONNECTION_URI: ${POSTGRES_URI_SUPERTOKENS:-postgresql://username:password@postgres:5432/agenta_ee_supertokens} # === NETWORK ============================================== # networks: @@ -564,6 +581,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.ee.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-ee-gh-network @@ -589,6 +608,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.gh} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" AGENTA_INGRESS_URL: http://api:8000/triggers/composio/events/ PYTHONUNBUFFERED: "1" # === NETWORK ============================================== # diff --git a/hosting/docker-compose/ee/docker-compose.gh.yml b/hosting/docker-compose/ee/docker-compose.gh.yml index f61995468c..4a3405522e 100644 --- a/hosting/docker-compose/ee/docker-compose.gh.yml +++ b/hosting/docker-compose/ee/docker-compose.gh.yml @@ -19,6 +19,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY= - AGENTA_MOBILE_GATE=${AGENTA_MOBILE_GATE:-true} # === NETWORK ============================================== # networks: @@ -46,6 +47,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY= - AGENTA_MOBILE_GATE=${AGENTA_MOBILE_GATE:-true} - AGENTA_MOBILE_REVERSE_GATE=${AGENTA_MOBILE_REVERSE_GATE:-true} # === NETWORK ============================================== # @@ -84,6 +86,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY=${AGENTA_SERVICES_INTERNAL_KEY:?AGENTA_SERVICES_INTERNAL_KEY is required} - SCRIPT_NAME=/api - DOCKER_NETWORK_MODE=${DOCKER_NETWORK_MODE:-bridge} # kill's direct runner hop (W7.3, sandbox teardown) — same address + shared @@ -142,6 +145,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY= - DOCKER_NETWORK_MODE=${DOCKER_NETWORK_MODE:-bridge} # empty ⇒ all stream loops; see docs/designs/workers-sprawl/specs.md - AGENTA_WORKER_STREAMS= @@ -183,6 +187,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY= - DOCKER_NETWORK_MODE=${DOCKER_NETWORK_MODE:-bridge} # empty ⇒ all queue loops; see docs/designs/workers-sprawl/specs.md - AGENTA_WORKER_QUEUES= @@ -220,6 +225,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY= - DOCKER_NETWORK_MODE=${DOCKER_NETWORK_MODE:-bridge} # === NETWORK ============================================== # networks: @@ -241,6 +247,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.ee.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-ee-gh-network @@ -272,6 +280,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY=${AGENTA_SERVICES_INTERNAL_KEY:?AGENTA_SERVICES_INTERNAL_KEY is required} - SCRIPT_NAME=/services - DOCKER_NETWORK_MODE=${DOCKER_NETWORK_MODE:-bridge} - AGENTA_RUNNER_INTERNAL_URL=${AGENTA_RUNNER_INTERNAL_URL:-http://runner:8765} @@ -390,6 +399,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.gh} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" POSTGRES_USER: ${POSTGRES_USER:-username} POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-password} # === NETWORK ============================================== # @@ -493,6 +503,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.gh} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # The entrypoint bakes these into s3.json/iam.json via raw shell expansion, so they must # be present in this service's env (defaults here are for the bundled store, the same way # supertokens carries its own connection URI). Keys come from the env file — secrets never @@ -548,6 +559,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.ee.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-ee-gh-network @@ -564,6 +577,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.gh} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" POSTGRESQL_CONNECTION_URI: ${POSTGRES_URI_SUPERTOKENS:-postgresql://username:password@postgres:5432/agenta_ee_supertokens} # === NETWORK ============================================== # networks: @@ -602,6 +616,7 @@ services: env_file: - ${ENV_FILE:-./.env.ee.gh} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" AGENTA_INGRESS_URL: http://api:8000/triggers/composio/events/ PYTHONUNBUFFERED: "1" # === NETWORK ============================================== # diff --git a/hosting/docker-compose/ee/env.ee.dev.example b/hosting/docker-compose/ee/env.ee.dev.example index ea68f4fdb9..00d8ce27b3 100644 --- a/hosting/docker-compose/ee/env.ee.dev.example +++ b/hosting/docker-compose/ee/env.ee.dev.example @@ -25,6 +25,12 @@ AGENTA_API_URL=http://localhost/api # Agenta - Secrets (REPLACE in production!) # ================================================================== # AGENTA_AUTH_KEY=replace-me +# Proves to the API that a caller IS the platform runtime (the workflow service), so the +# credential it receives may read write-only secret values. The API and the services +# container must hold the SAME dedicated value, and a browser, runner, worker, cron, or +# sandbox must never receive it. There is no AGENTA_AUTH_KEY fallback. Missing and +# placeholder values prevent write-only secret grants from being issued. +AGENTA_SERVICES_INTERNAL_KEY=replace-me AGENTA_CRYPT_KEY=replace-me # ================================================================== # diff --git a/hosting/docker-compose/ee/env.ee.gh.example b/hosting/docker-compose/ee/env.ee.gh.example index 699c33f796..0654ce99e8 100644 --- a/hosting/docker-compose/ee/env.ee.gh.example +++ b/hosting/docker-compose/ee/env.ee.gh.example @@ -29,6 +29,12 @@ AGENTA_API_URL=http://localhost/api # Agenta - Secrets (REPLACE in production!) # ================================================================== # AGENTA_AUTH_KEY=replace-me +# Proves to the API that a caller IS the platform runtime (the workflow service), so the +# credential it receives may read write-only secret values. The API and the services +# container must hold the SAME dedicated value, and a browser, runner, worker, cron, or +# sandbox must never receive it. There is no AGENTA_AUTH_KEY fallback. Missing and +# placeholder values prevent write-only secret grants from being issued. +AGENTA_SERVICES_INTERNAL_KEY=replace-me AGENTA_CRYPT_KEY=replace-me # ================================================================== # diff --git a/hosting/docker-compose/oss/docker-compose.dev.yml b/hosting/docker-compose/oss/docker-compose.dev.yml index 13c77bdf09..d3aee10d9b 100644 --- a/hosting/docker-compose/oss/docker-compose.dev.yml +++ b/hosting/docker-compose/oss/docker-compose.dev.yml @@ -65,6 +65,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" DOCKER_NETWORK_MODE: ${DOCKER_NETWORK_MODE:-bridge} WATCHPACK_POLLING: "true" AGENTA_MOBILE_GATE: ${AGENTA_MOBILE_GATE:-true} @@ -100,6 +101,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" DOCKER_NETWORK_MODE: ${DOCKER_NETWORK_MODE:-bridge} WATCHPACK_POLLING: "true" AGENTA_MOBILE_GATE: ${AGENTA_MOBILE_GATE:-true} @@ -168,6 +170,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: ${AGENTA_SERVICES_INTERNAL_KEY:?AGENTA_SERVICES_INTERNAL_KEY is required} DOCKER_NETWORK_MODE: ${DOCKER_NETWORK_MODE:-bridge} # kill's direct runner hop (W7.3, sandbox teardown) — same compose-network # address + shared token as the `services` container's runner_url(). @@ -228,6 +231,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" DOCKER_NETWORK_MODE: ${DOCKER_NETWORK_MODE:-bridge} # empty ⇒ all stream loops; see docs/designs/workers-sprawl/specs.md AGENTA_WORKER_STREAMS: "" @@ -277,6 +281,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" DOCKER_NETWORK_MODE: ${DOCKER_NETWORK_MODE:-bridge} # empty ⇒ all queue loops; see docs/designs/workers-sprawl/specs.md AGENTA_WORKER_QUEUES: "" @@ -319,6 +324,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" DOCKER_NETWORK_MODE: ${DOCKER_NETWORK_MODE:-bridge} # === NETWORK ============================================== # networks: @@ -348,6 +354,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" DOCKER_NETWORK_MODE: ${DOCKER_NETWORK_MODE:-bridge} # === NETWORK ============================================== # networks: @@ -398,6 +405,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: ${AGENTA_SERVICES_INTERNAL_KEY:?AGENTA_SERVICES_INTERNAL_KEY is required} DOCKER_NETWORK_MODE: ${DOCKER_NETWORK_MODE:-bridge} AGENTA_RUNNER_INTERNAL_URL: ${AGENTA_RUNNER_INTERNAL_URL:-http://runner:8765} AGENTA_RUNNER_TOKEN: ${AGENTA_RUNNER_TOKEN:?AGENTA_RUNNER_TOKEN is required} @@ -512,6 +520,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" POSTGRES_USER: ${POSTGRES_USER:-username} POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-password} # === NETWORK ============================================== # @@ -611,6 +620,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # The entrypoint bakes these into s3.json/iam.json via raw shell expansion, so unlike # the API (which reads code defaults from env.py) they must be present in this service's # env. Defaults live here for the bundled store the same way supertokens carries its own @@ -718,6 +728,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" POSTGRESQL_CONNECTION_URI: ${POSTGRES_URI_SUPERTOKENS:-postgresql://username:password@postgres:5432/agenta_oss_supertokens} # === NETWORK ============================================== # networks: @@ -774,6 +785,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.dev} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" AGENTA_INGRESS_URL: http://api:8000/triggers/composio/events/ PYTHONUNBUFFERED: "1" # === NETWORK ============================================== # diff --git a/hosting/docker-compose/oss/docker-compose.gh.local.yml b/hosting/docker-compose/oss/docker-compose.gh.local.yml index 90386c9dd2..9cb9f55368 100644 --- a/hosting/docker-compose/oss/docker-compose.gh.local.yml +++ b/hosting/docker-compose/oss/docker-compose.gh.local.yml @@ -14,6 +14,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.oss.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-oss-gh-network @@ -39,6 +41,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.oss.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-oss-gh-network @@ -77,6 +81,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY=${AGENTA_SERVICES_INTERNAL_KEY:?AGENTA_SERVICES_INTERNAL_KEY is required} - SCRIPT_NAME=/api # Unset leaves the code default (3600s) in place. - AGENTA_MOUNTS_CREDENTIALS_TTL_SECONDS=${AGENTA_MOUNTS_CREDENTIALS_TTL_SECONDS:-} @@ -126,6 +131,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY= # empty ⇒ all stream loops; see docs/designs/workers-sprawl/specs.md - AGENTA_WORKER_STREAMS= # === NETWORK ============================================== # @@ -170,6 +176,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY= # empty ⇒ all queue loops; see docs/designs/workers-sprawl/specs.md - AGENTA_WORKER_QUEUES= # === NETWORK ============================================== # @@ -207,6 +214,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.oss.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-oss-gh-network @@ -229,6 +238,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.oss.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-oss-gh-network @@ -262,6 +273,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY=${AGENTA_SERVICES_INTERNAL_KEY:?AGENTA_SERVICES_INTERNAL_KEY is required} - SCRIPT_NAME=/services - AGENTA_RUNNER_INTERNAL_URL=${AGENTA_RUNNER_INTERNAL_URL:-http://runner:8765} - AGENTA_RUNNER_TOKEN=${AGENTA_RUNNER_TOKEN:?AGENTA_RUNNER_TOKEN is required} @@ -360,6 +372,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" POSTGRES_USER: ${POSTGRES_USER:-username} POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-password} # === NETWORK ============================================== # @@ -463,6 +476,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # The entrypoint bakes these into s3.json/iam.json via raw shell expansion, so they must # be present in this service's env (defaults here are for the bundled store, the same way # supertokens carries its own connection URI). Keys come from the env file — secrets never @@ -512,6 +526,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.oss.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-oss-gh-network @@ -533,6 +549,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.oss.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-oss-gh-network @@ -552,6 +570,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" POSTGRESQL_CONNECTION_URI: ${POSTGRES_URI_SUPERTOKENS} # === NETWORK ============================================== # networks: @@ -590,6 +609,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" AGENTA_INGRESS_URL: http://api:8000/triggers/composio/events/ PYTHONUNBUFFERED: "1" # === NETWORK ============================================== # diff --git a/hosting/docker-compose/oss/docker-compose.gh.ssl.yml b/hosting/docker-compose/oss/docker-compose.gh.ssl.yml index d10d1af521..4ea6b1f777 100644 --- a/hosting/docker-compose/oss/docker-compose.gh.ssl.yml +++ b/hosting/docker-compose/oss/docker-compose.gh.ssl.yml @@ -14,6 +14,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.oss.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-gh-ssl-network @@ -41,6 +43,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.oss.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-gh-ssl-network @@ -84,6 +88,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY=${AGENTA_SERVICES_INTERNAL_KEY:?AGENTA_SERVICES_INTERNAL_KEY is required} - SCRIPT_NAME=/api # Unset leaves the code default (3600s) in place. - AGENTA_MOUNTS_CREDENTIALS_TTL_SECONDS=${AGENTA_MOUNTS_CREDENTIALS_TTL_SECONDS:-} @@ -138,6 +143,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY= # empty ⇒ all stream loops; see docs/designs/workers-sprawl/specs.md - AGENTA_WORKER_STREAMS= # === NETWORK ============================================== # @@ -185,6 +191,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY= # empty ⇒ all queue loops; see docs/designs/workers-sprawl/specs.md - AGENTA_WORKER_QUEUES= # === NETWORK ============================================== # @@ -225,6 +232,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.oss.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-gh-ssl-network @@ -250,6 +259,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.oss.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-gh-ssl-network @@ -283,6 +294,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY=${AGENTA_SERVICES_INTERNAL_KEY:?AGENTA_SERVICES_INTERNAL_KEY is required} - SCRIPT_NAME=/services - AGENTA_RUNNER_INTERNAL_URL=${AGENTA_RUNNER_INTERNAL_URL:-http://runner:8765} - AGENTA_RUNNER_TOKEN=${AGENTA_RUNNER_TOKEN:?AGENTA_RUNNER_TOKEN is required} @@ -386,6 +398,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" POSTGRES_USER: ${POSTGRES_USER:-username} POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-password} # === NETWORK ============================================== # @@ -489,6 +502,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # The entrypoint bakes these into s3.json/iam.json via raw shell expansion, so they must # be present in this service's env (defaults here are for the bundled store, the same way # supertokens carries its own connection URI). Keys come from the env file — secrets never @@ -555,6 +569,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.oss.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-gh-ssl-network @@ -592,6 +608,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" AGENTA_INGRESS_URL: http://api:8000/triggers/composio/events/ PYTHONUNBUFFERED: "1" # === NETWORK ============================================== # diff --git a/hosting/docker-compose/oss/docker-compose.gh.yml b/hosting/docker-compose/oss/docker-compose.gh.yml index 02b0d05005..96f1cc05aa 100644 --- a/hosting/docker-compose/oss/docker-compose.gh.yml +++ b/hosting/docker-compose/oss/docker-compose.gh.yml @@ -16,6 +16,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY= - AGENTA_MOBILE_GATE=${AGENTA_MOBILE_GATE:-true} # === NETWORK ============================================== # networks: @@ -44,6 +45,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY= - AGENTA_MOBILE_GATE=${AGENTA_MOBILE_GATE:-true} - AGENTA_MOBILE_REVERSE_GATE=${AGENTA_MOBILE_REVERSE_GATE:-true} # === NETWORK ============================================== # @@ -85,6 +87,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY=${AGENTA_SERVICES_INTERNAL_KEY:?AGENTA_SERVICES_INTERNAL_KEY is required} - SCRIPT_NAME=/api - DOCKER_NETWORK_MODE=${DOCKER_NETWORK_MODE:-bridge} # kill's direct runner hop (W7.3, sandbox teardown) — same address + shared @@ -146,6 +149,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY= - DOCKER_NETWORK_MODE=${DOCKER_NETWORK_MODE:-bridge} # empty ⇒ all stream loops; see docs/designs/workers-sprawl/specs.md - AGENTA_WORKER_STREAMS= @@ -192,6 +196,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY= - DOCKER_NETWORK_MODE=${DOCKER_NETWORK_MODE:-bridge} # empty ⇒ all queue loops; see docs/designs/workers-sprawl/specs.md - AGENTA_WORKER_QUEUES= @@ -232,6 +237,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY= - DOCKER_NETWORK_MODE=${DOCKER_NETWORK_MODE:-bridge} # === NETWORK ============================================== # networks: @@ -256,6 +262,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.oss.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-oss-gh-network @@ -290,6 +298,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + - AGENTA_SERVICES_INTERNAL_KEY=${AGENTA_SERVICES_INTERNAL_KEY:?AGENTA_SERVICES_INTERNAL_KEY is required} - SCRIPT_NAME=/services - DOCKER_NETWORK_MODE=${DOCKER_NETWORK_MODE:-bridge} - AGENTA_RUNNER_INTERNAL_URL=${AGENTA_RUNNER_INTERNAL_URL:-http://runner:8765} @@ -408,6 +417,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" POSTGRES_USER: ${POSTGRES_USER:-username} POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-password} # === NETWORK ============================================== # @@ -511,6 +521,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # The entrypoint bakes these into s3.json/iam.json via raw shell expansion, so they must # be present in this service's env (defaults here are for the bundled store, the same way # supertokens carries its own connection URI). Keys come from the env file — secrets never @@ -569,6 +580,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.oss.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-oss-gh-network @@ -590,6 +603,8 @@ services: # === CONFIGURATION ======================================== # env_file: - ${ENV_FILE:-./.env.oss.gh} + environment: + AGENTA_SERVICES_INTERNAL_KEY: "" # === NETWORK ============================================== # networks: - agenta-oss-gh-network @@ -609,6 +624,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" POSTGRESQL_CONNECTION_URI: ${POSTGRES_URI_SUPERTOKENS} # === NETWORK ============================================== # networks: @@ -647,6 +663,7 @@ services: env_file: - ${ENV_FILE:-./.env.oss.gh} environment: + AGENTA_SERVICES_INTERNAL_KEY: "" AGENTA_INGRESS_URL: http://api:8000/triggers/composio/events/ PYTHONUNBUFFERED: "1" # === NETWORK ============================================== # diff --git a/hosting/docker-compose/oss/env.oss.dev.example b/hosting/docker-compose/oss/env.oss.dev.example index bf9adc6a57..487cab8bb2 100644 --- a/hosting/docker-compose/oss/env.oss.dev.example +++ b/hosting/docker-compose/oss/env.oss.dev.example @@ -25,6 +25,12 @@ AGENTA_API_URL=http://localhost/api # Agenta - Secrets (REPLACE in production!) # ================================================================== # AGENTA_AUTH_KEY=replace-me +# Proves to the API that a caller IS the platform runtime (the workflow service), so the +# credential it receives may read write-only secret values. The API and the services +# container must hold the SAME dedicated value, and a browser, runner, worker, cron, or +# sandbox must never receive it. There is no AGENTA_AUTH_KEY fallback. Missing and +# placeholder values prevent write-only secret grants from being issued. +AGENTA_SERVICES_INTERNAL_KEY=replace-me AGENTA_CRYPT_KEY=replace-me # ================================================================== # diff --git a/hosting/docker-compose/oss/env.oss.gh.example b/hosting/docker-compose/oss/env.oss.gh.example index 024ad46e12..ee6ac96f82 100644 --- a/hosting/docker-compose/oss/env.oss.gh.example +++ b/hosting/docker-compose/oss/env.oss.gh.example @@ -29,6 +29,12 @@ AGENTA_API_URL=http://localhost/api # Agenta - Secrets (REPLACE in production!) # ================================================================== # AGENTA_AUTH_KEY=replace-me +# Proves to the API that a caller IS the platform runtime (the workflow service), so the +# credential it receives may read write-only secret values. The API and the services +# container must hold the SAME dedicated value, and a browser, runner, worker, cron, or +# sandbox must never receive it. There is no AGENTA_AUTH_KEY fallback. Missing and +# placeholder values prevent write-only secret grants from being issued. +AGENTA_SERVICES_INTERNAL_KEY=replace-me AGENTA_CRYPT_KEY=replace-me # ================================================================== # diff --git a/hosting/kubernetes/ee/values.ee.example.yaml b/hosting/kubernetes/ee/values.ee.example.yaml index cf1512b246..2211fe416b 100644 --- a/hosting/kubernetes/ee/values.ee.example.yaml +++ b/hosting/kubernetes/ee/values.ee.example.yaml @@ -15,6 +15,7 @@ agenta: # --- Secrets (REPLACE in production!) --- authKey: "replace-me" cryptKey: "replace-me" + servicesInternalKey: "replace-me" runnerToken: "replace-me" # ================================================================== # diff --git a/hosting/kubernetes/helm/templates/NOTES.txt b/hosting/kubernetes/helm/templates/NOTES.txt index 03cdf6c31b..11b6953c45 100644 --- a/hosting/kubernetes/helm/templates/NOTES.txt +++ b/hosting/kubernetes/helm/templates/NOTES.txt @@ -130,6 +130,8 @@ Ensure it contains the following keys: Required: - AGENTA_AUTH_KEY - AGENTA_CRYPT_KEY + - AGENTA_SERVICES_INTERNAL_KEY + - AGENTA_RUNNER_TOKEN # unless agentRunner.auth.tokenSecretRef is set - POSTGRES_PASSWORD Optional (only if you enabled the corresponding feature): @@ -161,11 +163,13 @@ Set: --set global.postgresql.auth.existingSecret={{ $secrets.existingSecret }} {{- end }} {{- else }} -IMPORTANT: Three secrets are REQUIRED. Set them in your values file: +IMPORTANT: Required credentials must be set in your values file: agenta: authKey: "" cryptKey: "" + servicesInternalKey: "" + runnerToken: "" postgres: password: "" diff --git a/hosting/kubernetes/helm/templates/_helpers.tpl b/hosting/kubernetes/helm/templates/_helpers.tpl index e3b7393990..915abfc918 100644 --- a/hosting/kubernetes/helm/templates/_helpers.tpl +++ b/hosting/kubernetes/helm/templates/_helpers.tpl @@ -824,6 +824,19 @@ imagePullSecrets: {{- end }} {{- end }} +{{/* ================================================================ + Credential used only by the API and Services for trusted grant + exchange. Keep it separate from commonEnv, which is also rendered + into workers, cron, and alembic. + ================================================================ */}} +{{- define "agenta.servicesInternalEnv" -}} +- name: AGENTA_SERVICES_INTERNAL_KEY + valueFrom: + secretKeyRef: + name: {{ include "agenta.secretName" . }} + key: AGENTA_SERVICES_INTERNAL_KEY +{{- end }} + {{/* ================================================================ Common environment variables shared by api, workers, cron, alembic. Inlines the legacy `backendOptionalEnv` block. diff --git a/hosting/kubernetes/helm/templates/_validations.tpl b/hosting/kubernetes/helm/templates/_validations.tpl index f5b415d148..0a97a03e99 100644 --- a/hosting/kubernetes/helm/templates/_validations.tpl +++ b/hosting/kubernetes/helm/templates/_validations.tpl @@ -96,11 +96,10 @@ absolute-URL builder in the app. Validate that the canonical app secrets are provided when the chart is creating the Secret itself (i.e. secrets.existingSecret is unset). Without this guard the chart renders, the pods start, and the app - crashes on first request because AGENTA_AUTH_KEY / AGENTA_CRYPT_KEY - are empty. Fail at install time instead. + crashes on first request because required credentials are empty. Also rejects the literal placeholder "replace-me" shipped in - values.yaml's defaults for authKey/cryptKey/runnerToken — otherwise + values.yaml's defaults for authKey/cryptKey/servicesInternalKey/runnerToken — otherwise a stock `helm install` with no overrides silently deploys with a publicly-known secret instead of failing. @@ -124,6 +123,7 @@ absolute-URL builder in the app. {{- $missing := list -}} {{- if or (not $agenta.authKey) (eq $agenta.authKey $placeholder) -}}{{- $missing = append $missing "agenta.authKey" -}}{{- end -}} {{- if or (not $agenta.cryptKey) (eq $agenta.cryptKey $placeholder) -}}{{- $missing = append $missing "agenta.cryptKey" -}}{{- end -}} +{{- if or (not $agenta.servicesInternalKey) (eq $agenta.servicesInternalKey $placeholder) -}}{{- $missing = append $missing "agenta.servicesInternalKey" -}}{{- end -}} {{- if not $runnerAuth.tokenSecretRef -}} {{- if or (not $agenta.runnerToken) (eq $agenta.runnerToken $placeholder) -}}{{- $missing = append $missing "agenta.runnerToken" -}}{{- end -}} {{- end -}} @@ -151,6 +151,7 @@ Generate real values and set them in your values file: agenta: authKey: "<32+ random bytes hex>" cryptKey: "<32+ random bytes hex>" + servicesInternalKey: "<32+ random bytes hex>" runnerToken: "<32+ random bytes hex>" postgres: password: "" @@ -160,6 +161,7 @@ Or pass them on the command line: helm install agenta hosting/kubernetes/helm \ --set agenta.authKey=$(openssl rand -hex 32) \ --set agenta.cryptKey=$(openssl rand -hex 32) \ + --set agenta.servicesInternalKey=$(openssl rand -hex 32) \ --set agenta.runnerToken=$(openssl rand -hex 32) \ --set postgres.password= @@ -168,7 +170,7 @@ Or provide a pre-created Kubernetes Secret and point the chart at it: secrets: existingSecret: my-agenta-secret -The Secret must contain keys: AGENTA_AUTH_KEY, AGENTA_CRYPT_KEY, AGENTA_RUNNER_TOKEN, POSTGRES_PASSWORD. +The Secret must contain keys: AGENTA_AUTH_KEY, AGENTA_CRYPT_KEY, AGENTA_SERVICES_INTERNAL_KEY, AGENTA_RUNNER_TOKEN, POSTGRES_PASSWORD. agenta.runnerToken alone can also be satisfied by pointing the runner at your own Secret: diff --git a/hosting/kubernetes/helm/templates/api-deployment.yaml b/hosting/kubernetes/helm/templates/api-deployment.yaml index 8143adc034..24a5664ebb 100644 --- a/hosting/kubernetes/helm/templates/api-deployment.yaml +++ b/hosting/kubernetes/helm/templates/api-deployment.yaml @@ -73,6 +73,7 @@ spec: protocol: TCP env: {{- include "agenta.commonEnv" . | nindent 12 }} + {{- include "agenta.servicesInternalEnv" . | nindent 12 }} - name: SCRIPT_NAME value: "/api" {{- include "agenta.agentRunner.servicesEnv" . | nindent 12 }} diff --git a/hosting/kubernetes/helm/templates/secrets.yaml b/hosting/kubernetes/helm/templates/secrets.yaml index 44652f919f..820eea05a6 100644 --- a/hosting/kubernetes/helm/templates/secrets.yaml +++ b/hosting/kubernetes/helm/templates/secrets.yaml @@ -36,6 +36,7 @@ type: Opaque stringData: AGENTA_AUTH_KEY: {{ required "agenta.authKey is required (use --set agenta.authKey=)" $agenta.authKey | quote }} AGENTA_CRYPT_KEY: {{ required "agenta.cryptKey is required (use --set agenta.cryptKey=)" $agenta.cryptKey | quote }} + AGENTA_SERVICES_INTERNAL_KEY: {{ required "agenta.servicesInternalKey is required (use --set agenta.servicesInternalKey=)" $agenta.servicesInternalKey | quote }} {{- /* The runner's shared token. Required: the runner refuses to boot without it, and the service that calls it must present the same value. Lives in this Secret so both sides read one source; the runner Deployment mounts ONLY this key (never the whole Secret — diff --git a/hosting/kubernetes/helm/templates/services-deployment.yaml b/hosting/kubernetes/helm/templates/services-deployment.yaml index 9e69173a66..1dd202cc02 100644 --- a/hosting/kubernetes/helm/templates/services-deployment.yaml +++ b/hosting/kubernetes/helm/templates/services-deployment.yaml @@ -73,6 +73,7 @@ spec: protocol: TCP env: {{- include "agenta.commonEnv" . | nindent 12 }} + {{- include "agenta.servicesInternalEnv" . | nindent 12 }} - name: SCRIPT_NAME value: "/services" {{- include "agenta.agentRunner.servicesEnv" . | nindent 12 }} diff --git a/hosting/kubernetes/helm/tests/test_runner_secret_absence.py b/hosting/kubernetes/helm/tests/test_runner_secret_absence.py index d1bb0859b4..2b8f595f51 100644 --- a/hosting/kubernetes/helm/tests/test_runner_secret_absence.py +++ b/hosting/kubernetes/helm/tests/test_runner_secret_absence.py @@ -42,6 +42,8 @@ "--set", "agenta.cryptKey=test-crypt-key", "--set", + "agenta.servicesInternalKey=test-services-internal-key", + "--set", "postgres.password=test-postgres-password", ] @@ -62,6 +64,7 @@ FORBIDDEN_EXACT = { "AGENTA_AUTH_KEY", "AGENTA_CRYPT_KEY", + "AGENTA_SERVICES_INTERNAL_KEY", "AGENTA_LICENSE", "AGENTA_API_KEY", "OPENAI_API_KEY", @@ -117,6 +120,19 @@ def runner_container_env_names(docs: list[dict]) -> list[str]: raise AssertionError("no runner Deployment found in the rendered chart") +def deployment_env_names(docs: list[dict], component: str) -> set[str]: + """Environment variable names on a component's primary container.""" + for doc in docs: + if doc.get("kind") != "Deployment": + continue + labels = doc.get("metadata", {}).get("labels", {}) + if labels.get("app.kubernetes.io/component") != component: + continue + container = doc["spec"]["template"]["spec"]["containers"][0] + return {entry["name"] for entry in container.get("env", [])} + raise AssertionError(f"no {component} Deployment found in the rendered chart") + + def check(names: list[str]) -> list[str]: failures: list[str] = [] present = set(names) @@ -150,6 +166,18 @@ def main() -> int: names = runner_container_env_names(render(DEFAULT_TOKEN_ARGS)) failures += check(names) + docs = render(DEFAULT_TOKEN_ARGS) + for component in ("api", "services"): + if "AGENTA_SERVICES_INTERNAL_KEY" not in deployment_env_names(docs, component): + failures.append( + f"{component} env must contain AGENTA_SERVICES_INTERNAL_KEY" + ) + for component in ("worker-streams", "worker-queues", "cron"): + if "AGENTA_SERVICES_INTERNAL_KEY" in deployment_env_names(docs, component): + failures.append( + f"{component} env must not contain AGENTA_SERVICES_INTERNAL_KEY" + ) + # Operator supplies their own secret ref: same narrow env, token sourced from their Secret. names_with_token = runner_container_env_names(render(TOKEN_ARGS)) failures += check(names_with_token) @@ -161,7 +189,7 @@ def main() -> int: return 1 print( - "OK: runner Deployment env is narrow (no platform secrets, provider keys, or API key)." + "OK: internal-services key is limited to API/Services; runner env remains narrow." ) print(f" default env: {sorted(names)}") return 0 diff --git a/hosting/kubernetes/helm/values.schema.json b/hosting/kubernetes/helm/values.schema.json index e9b8ed06c3..91ab2df83c 100644 --- a/hosting/kubernetes/helm/values.schema.json +++ b/hosting/kubernetes/helm/values.schema.json @@ -16,6 +16,7 @@ "apiInternalUrl": { "type": "string", "description": "Internal API URL (in-cluster). Emitted as AGENTA_API_INTERNAL_URL." }, "authKey": { "type": "string", "minLength": 1, "description": "Authorization key. Required: `helm install` fails while this is unset or still 'replace-me' (see agenta.validateRequiredSecrets), unless secrets.existingSecret is set. Emitted as AGENTA_AUTH_KEY via Secret." }, "cryptKey": { "type": "string", "minLength": 1, "description": "Encryption key. Required: `helm install` fails while this is unset or still 'replace-me' (see agenta.validateRequiredSecrets), unless secrets.existingSecret is set. Emitted as AGENTA_CRYPT_KEY via Secret." }, + "servicesInternalKey": { "type": "string", "minLength": 1, "description": "Dedicated credential for trusted API-to-Services grant exchange. Required: `helm install` fails while this is unset or still 'replace-me' (see agenta.validateRequiredSecrets), unless secrets.existingSecret is set. Emitted as AGENTA_SERVICES_INTERNAL_KEY only to the API and Services pods." }, "runnerToken": { "type": "string", "minLength": 1, "description": "Shared secret the runner verifies and Services presents. Required: `helm install` fails while this is unset or still 'replace-me' (see agenta.validateRequiredSecrets), unless secrets.existingSecret or agentRunner.auth.tokenSecretRef is set. Emitted as AGENTA_RUNNER_TOKEN via Secret." }, "insecureEgressAllowed": { "type": ["boolean", "string"], "description": "AGENTA_INSECURE_EGRESS_ALLOWED — default true (permissive, zero-config self-host); set false to harden a shared/multi-tenant deployment." }, "access": { diff --git a/hosting/kubernetes/helm/values.yaml b/hosting/kubernetes/helm/values.yaml index 97cb40c547..5534470555 100644 --- a/hosting/kubernetes/helm/values.yaml +++ b/hosting/kubernetes/helm/values.yaml @@ -38,6 +38,7 @@ agenta: # ================================================================ # authKey: "replace-me" cryptKey: "replace-me" + servicesInternalKey: "replace-me" runnerToken: "replace-me" # ================================================================== # diff --git a/hosting/kubernetes/oss/values.oss.example.yaml b/hosting/kubernetes/oss/values.oss.example.yaml index 0573ce6744..f71429ee0d 100644 --- a/hosting/kubernetes/oss/values.oss.example.yaml +++ b/hosting/kubernetes/oss/values.oss.example.yaml @@ -15,6 +15,7 @@ agenta: # --- Secrets (REPLACE in production!) --- authKey: "replace-me" cryptKey: "replace-me" + servicesInternalKey: "replace-me" runnerToken: "replace-me" # ================================================================== # diff --git a/hosting/railway/oss/README.md b/hosting/railway/oss/README.md index c7166c41f5..de91d0986e 100644 --- a/hosting/railway/oss/README.md +++ b/hosting/railway/oss/README.md @@ -176,13 +176,16 @@ content, kept in lockstep by `images/verify-wrappers.sh`). ### Security Note The scripts default to compose-like placeholder values for `AGENTA_AUTH_KEY`, -`AGENTA_CRYPT_KEY`, `AGENTA_RUNNER_TOKEN`, and `POSTGRES_PASSWORD`. This is -acceptable for throwaway test projects, but not for persistent deployments. -For persistent deployments, set unique values: +`AGENTA_CRYPT_KEY`, `AGENTA_SERVICES_INTERNAL_KEY`, `AGENTA_RUNNER_TOKEN`, and +`POSTGRES_PASSWORD`. This is acceptable for throwaway test projects, but not +for persistent deployments. `AGENTA_SERVICES_INTERNAL_KEY` has no fallback to +`AGENTA_AUTH_KEY`; API and Services must receive the same dedicated value. +For persistent deployments, set a unique value for each credential: ```bash export AGENTA_AUTH_KEY="$(openssl rand -hex 32)" export AGENTA_CRYPT_KEY="$(openssl rand -hex 32)" +export AGENTA_SERVICES_INTERNAL_KEY="$(openssl rand -hex 32)" export AGENTA_RUNNER_TOKEN="$(openssl rand -hex 32)" export POSTGRES_PASSWORD="$(openssl rand -hex 24)" ``` diff --git a/hosting/railway/oss/scripts/configure.sh b/hosting/railway/oss/scripts/configure.sh index af81f70d5e..cca0e0e2f6 100755 --- a/hosting/railway/oss/scripts/configure.sh +++ b/hosting/railway/oss/scripts/configure.sh @@ -15,6 +15,7 @@ POSTGRES_REF_NS="${RAILWAY_POSTGRES_REF_NS:-Postgres}" REDIS_SERVICE="${RAILWAY_REDIS_SERVICE:-redis}" AGENTA_AUTH_KEY="${AGENTA_AUTH_KEY:-replace-me}" AGENTA_CRYPT_KEY="${AGENTA_CRYPT_KEY:-replace-me}" +AGENTA_SERVICES_INTERNAL_KEY="${AGENTA_SERVICES_INTERNAL_KEY:-replace-me}" AGENTA_RUNNER_TOKEN="${AGENTA_RUNNER_TOKEN:-replace-me}" POSTGRES_PASSWORD="${POSTGRES_PASSWORD:-}" AGENTA_STORE_ACCESS_KEY="${AGENTA_STORE_ACCESS_KEY:-}" @@ -300,8 +301,8 @@ main() { require_cmd railway require_railway_auth - if [ "$AGENTA_AUTH_KEY" = "replace-me" ] || [ "$AGENTA_CRYPT_KEY" = "replace-me" ] || [ "$AGENTA_RUNNER_TOKEN" = "replace-me" ]; then - printf "WARNING: Using default placeholder secrets. Set AGENTA_AUTH_KEY, AGENTA_CRYPT_KEY and AGENTA_RUNNER_TOKEN for production deployments.\n" >&2 + if [ "$AGENTA_AUTH_KEY" = "replace-me" ] || [ "$AGENTA_CRYPT_KEY" = "replace-me" ] || [ "$AGENTA_SERVICES_INTERNAL_KEY" = "replace-me" ] || [ "$AGENTA_RUNNER_TOKEN" = "replace-me" ]; then + printf "WARNING: Using default placeholder secrets. Set AGENTA_AUTH_KEY, AGENTA_CRYPT_KEY, AGENTA_SERVICES_INTERNAL_KEY and AGENTA_RUNNER_TOKEN for production deployments.\n" >&2 fi railway_call link --project "$PROJECT_NAME" --environment "$ENV_NAME" --json >/dev/null @@ -391,6 +392,7 @@ main() { AGENTA_SERVICES_URL="https://${public_domain_ref}/services" \ AGENTA_AUTH_KEY="$AGENTA_AUTH_KEY" \ AGENTA_CRYPT_KEY="$AGENTA_CRYPT_KEY" \ + AGENTA_SERVICES_INTERNAL_KEY="$AGENTA_SERVICES_INTERNAL_KEY" \ POSTGRES_URI_CORE="$pg_async_core" \ POSTGRES_URI_TRACING="$pg_async_tracing" \ POSTGRES_URI_SUPERTOKENS="$pg_sync_supertokens" \ @@ -423,6 +425,7 @@ main() { AGENTA_SERVICES_URL="https://${public_domain_ref}/services" \ AGENTA_AUTH_KEY="$AGENTA_AUTH_KEY" \ AGENTA_CRYPT_KEY="$AGENTA_CRYPT_KEY" \ + AGENTA_SERVICES_INTERNAL_KEY="$AGENTA_SERVICES_INTERNAL_KEY" \ POSTGRES_URI_CORE="$pg_async_core" \ POSTGRES_URI_TRACING="$pg_async_tracing" \ POSTGRES_URI_SUPERTOKENS="$pg_sync_supertokens" \ diff --git a/hosting/railway/oss/template/template.json b/hosting/railway/oss/template/template.json index 6313b868b8..7840c5f7e3 100644 --- a/hosting/railway/oss/template/template.json +++ b/hosting/railway/oss/template/template.json @@ -64,6 +64,10 @@ "from_env": "AGENTA_CRYPT_KEY", "generate": "openssl-rand-hex-32" }, + "AGENTA_SERVICES_INTERNAL_KEY": { + "from_env": "AGENTA_SERVICES_INTERNAL_KEY", + "generate": "openssl-rand-hex-32" + }, "AGENTA_RUNNER_TOKEN": { "from_env": "AGENTA_RUNNER_TOKEN", "generate": "openssl-rand-hex-32" @@ -171,6 +175,9 @@ "AGENTA_CRYPT_KEY": { "secret": "AGENTA_CRYPT_KEY" }, + "AGENTA_SERVICES_INTERNAL_KEY": { + "secret": "AGENTA_SERVICES_INTERNAL_KEY" + }, "POSTGRES_URI_CORE": "postgresql+asyncpg://${{Postgres.POSTGRES_USER}}:${{Postgres.POSTGRES_PASSWORD}}@${{Postgres.RAILWAY_PRIVATE_DOMAIN}}:${{Postgres.PGPORT}}/agenta_oss_core", "POSTGRES_URI_TRACING": "postgresql+asyncpg://${{Postgres.POSTGRES_USER}}:${{Postgres.POSTGRES_PASSWORD}}@${{Postgres.RAILWAY_PRIVATE_DOMAIN}}:${{Postgres.PGPORT}}/agenta_oss_tracing", "POSTGRES_URI_SUPERTOKENS": "postgresql://${{Postgres.POSTGRES_USER}}:${{Postgres.POSTGRES_PASSWORD}}@${{Postgres.RAILWAY_PRIVATE_DOMAIN}}:${{Postgres.PGPORT}}/agenta_oss_supertokens", @@ -219,6 +226,9 @@ "AGENTA_CRYPT_KEY": { "secret": "AGENTA_CRYPT_KEY" }, + "AGENTA_SERVICES_INTERNAL_KEY": { + "secret": "AGENTA_SERVICES_INTERNAL_KEY" + }, "POSTGRES_URI_CORE": "postgresql+asyncpg://${{Postgres.POSTGRES_USER}}:${{Postgres.POSTGRES_PASSWORD}}@${{Postgres.RAILWAY_PRIVATE_DOMAIN}}:${{Postgres.PGPORT}}/agenta_oss_core", "POSTGRES_URI_TRACING": "postgresql+asyncpg://${{Postgres.POSTGRES_USER}}:${{Postgres.POSTGRES_PASSWORD}}@${{Postgres.RAILWAY_PRIVATE_DOMAIN}}:${{Postgres.PGPORT}}/agenta_oss_tracing", "POSTGRES_URI_SUPERTOKENS": "postgresql://${{Postgres.POSTGRES_USER}}:${{Postgres.POSTGRES_PASSWORD}}@${{Postgres.RAILWAY_PRIVATE_DOMAIN}}:${{Postgres.PGPORT}}/agenta_oss_supertokens", diff --git a/sdks/python/agenta/sdk/agents/connections/__init__.py b/sdks/python/agenta/sdk/agents/connections/__init__.py index fe89d186aa..7249b6814c 100644 --- a/sdks/python/agenta/sdk/agents/connections/__init__.py +++ b/sdks/python/agenta/sdk/agents/connections/__init__.py @@ -19,6 +19,7 @@ UnsupportedConnectionModeError, UnsupportedDeploymentError, UnsupportedProviderError, + WriteOnlySecretError, ) from .interfaces import ConnectionResolver from .models import ( @@ -64,4 +65,5 @@ "UnsupportedProviderError", "UnsupportedConnectionModeError", "UnsupportedDeploymentError", + "WriteOnlySecretError", ] diff --git a/sdks/python/agenta/sdk/agents/connections/credentials.py b/sdks/python/agenta/sdk/agents/connections/credentials.py new file mode 100644 index 0000000000..0a3cef883f --- /dev/null +++ b/sdks/python/agenta/sdk/agents/connections/credentials.py @@ -0,0 +1,92 @@ +"""The canonical classification of credential material inside vault secrets. + +One vocabulary, consumed by BOTH sides of the write-only contract so they can never drift: + +- the SDK's connection resolver (``platform/connections.py``), which decides which extras + are credentials to inject and whether a redacted record still holds usable material; +- the API's redaction and update carry-over (``oss/src/core/secrets/redaction.py``), which + must strip, refill, and report presence for exactly the same fields. + +Scope is deliberately what a CONNECTION carries. The per-kind primary value field lives on +the API side (``oss/src/core/secrets/redaction.py``): it covers secret kinds the SDK never +resolves — SSO providers, webhook signing secrets — and nothing here reads it. + +Every key the resolver accepts in a custom provider's ``extras`` must appear in exactly one +of the two sets below; a parity test enforces that, so adding an extras key to the resolver +without classifying it here fails the build. +""" + +from __future__ import annotations + +from collections.abc import Mapping +from typing import Dict, FrozenSet + +# Extras keys (as stored: the UI's snake_case aliases plus raw env-style names) that hold +# credential material. `vertex_ai_credentials`/`GOOGLE_APPLICATION_CREDENTIALS` are +# included because the stored value may be pasted service-account material, not a path. +CREDENTIAL_EXTRAS_KEYS: FrozenSet[str] = frozenset( + { + # UI snake_case aliases. + "api_key", + "aws_access_key_id", + "aws_secret_access_key", + "aws_session_token", + "aws_bearer_token_bedrock", + "vertex_ai_credentials", + # Raw env-style keys: API keys / auth tokens. + "OPENAI_API_KEY", + "ANTHROPIC_API_KEY", + "ANTHROPIC_AUTH_TOKEN", + "ANTHROPIC_OAUTH_TOKEN", + "CLAUDE_CODE_OAUTH_TOKEN", + "GEMINI_API_KEY", + "MISTRAL_API_KEY", + "MINIMAX_API_KEY", + "GROQ_API_KEY", + "TOGETHERAI_API_KEY", + "TOGETHER_API_KEY", + "OPENROUTER_API_KEY", + # Raw env-style keys: AWS. + "AWS_ACCESS_KEY_ID", + "AWS_SECRET_ACCESS_KEY", + "AWS_SESSION_TOKEN", + "AWS_BEARER_TOKEN_BEDROCK", + # Raw env-style keys: GCP / Azure. + "GOOGLE_APPLICATION_CREDENTIALS", + "GOOGLE_CLOUD_API_KEY", + "AZURE_OPENAI_API_KEY", + } +) + +# Extras keys that are plain configuration: safe to keep readable, never carried as +# credential material. The parity test requires resolver-accepted keys to be here or above. +CONFIG_EXTRAS_KEYS: FrozenSet[str] = frozenset( + { + "aws_region_name", + "vertex_ai_project", + "vertex_ai_location", + "AWS_PROFILE", + "AWS_REGION", + "AWS_DEFAULT_REGION", + "GOOGLE_CLOUD_PROJECT", + "GOOGLE_CLOUD_LOCATION", + } +) + + +def secret_value_configured(secret: object) -> bool: + """Whether the public Vault response says credential material is stored.""" + if not isinstance(secret, Mapping): + return False + + value_status = secret.get("value_status") + return isinstance(value_status, Mapping) and value_status.get("configured") is True + + +def credential_extras(extras: Dict[str, object]) -> Dict[str, object]: + """The subset of ``extras`` holding non-empty credential material.""" + return { + key: value + for key, value in extras.items() + if key in CREDENTIAL_EXTRAS_KEYS and value not in (None, "") + } diff --git a/sdks/python/agenta/sdk/agents/connections/errors.py b/sdks/python/agenta/sdk/agents/connections/errors.py index 1acb184f37..5f615f0ff5 100644 --- a/sdks/python/agenta/sdk/agents/connections/errors.py +++ b/sdks/python/agenta/sdk/agents/connections/errors.py @@ -55,6 +55,37 @@ def __init__(self, *, provider: str, slug: Optional[str] = None) -> None: self.slug = slug +class WriteOnlySecretError(ConnectionResolutionError): + """Raised when the chosen connection's key exists but came back redacted. + + The vault holds a write-only secret for this connection: the platform runtime reads it + through a granted credential, but this caller's credential (typically an ApiKey in a + standalone run) only receives the redacted shape. The resolver falls back to the + provider's standard environment variable first; this is raised only when that key is + absent too, because passing the redacted (empty) key to a provider would fail with a + misleading auth error. + """ + + # A standalone run against a write-only secret is a config situation, not a server fault. + status_code = 422 + + def __init__(self, *, slug: Optional[str] = None, provider: str = "") -> None: + subject = ( + f"connection '{slug}'" if slug else f"provider '{provider}' connection" + ) + # The remediation the resolver itself already tried: it reads the provider's + # standard environment variable before raising, so this error means that key is + # missing too. Naming it keeps the instruction actionable and true. + super().__init__( + f"{subject} uses a write-only secret: Agenta stores the value but never " + "returns it, so only runs on the Agenta platform can use it. To run " + "outside the platform, provide the provider key in this run's environment " + "(for example OPENAI_API_KEY)." + ) + self.slug = slug + self.provider = provider + + class InvalidConnectionConfigurationError(AgentConnectionError): """Raised when resolved routing and credentials form an unsafe combination.""" diff --git a/sdks/python/agenta/sdk/agents/platform/connections.py b/sdks/python/agenta/sdk/agents/platform/connections.py index c3f01a45c2..1144f25dee 100644 --- a/sdks/python/agenta/sdk/agents/platform/connections.py +++ b/sdks/python/agenta/sdk/agents/platform/connections.py @@ -11,8 +11,9 @@ from __future__ import annotations -from dataclasses import dataclass, field -from typing import Any, Dict, Iterable, List, Optional, Sequence, Set +import os +from dataclasses import dataclass, field, replace +from typing import Any, Dict, Iterable, List, Optional, Sequence, Set, Tuple import httpx @@ -24,6 +25,7 @@ HARNESS_CONNECTION_CAPABILITIES, PROVIDER_ENV_VARS, ) +from ..connections.credentials import credential_extras, secret_value_configured from ..connections.endpoints import build_resolved_connection from ..connections import ( AmbiguousConnectionError, @@ -38,6 +40,7 @@ ResolvedConnection, RuntimeAuthContext, UnsupportedConnectionModeError, + WriteOnlySecretError, ) from ..model_catalog import model_input_modalities from .connection import PlatformConnection @@ -176,6 +179,58 @@ def _provider_env_var(provider: Optional[str]) -> Optional[str]: return _PROVIDER_ENV_VARS.get(provider.lower()) if provider else None +def _credential_channels( + provider: str, candidate: "_ConnectionCandidate" +) -> List[Tuple[str, ...]]: + """The environment variables this candidate's credential could ride, best first. + + Each entry is one COMPLETE channel: every variable in it must be present for that + channel to authenticate. Deliberately the variables the harness itself would read for + this candidate, never merely the provider family's — a Bedrock or Azure candidate + authenticates through its own channel, and reading a family key (say + ``OPENAI_API_KEY``) for it would send one service's credential to another. The set + mirrors the credential material the plaintext path accepts for the same connection + (``CREDENTIAL_EXTRAS_KEYS``), so a standalone run can supply from the environment + exactly what the vault would have supplied. + """ + if candidate.deployment == "bedrock": + return [ + ("AWS_BEARER_TOKEN_BEDROCK",), + ("AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY"), + ] + if candidate.deployment in ("vertex_ai", "vertex"): + return [("GOOGLE_APPLICATION_CREDENTIALS",)] + if candidate.deployment == "azure": + return [("AZURE_OPENAI_API_KEY",)] + # A caller-selected endpoint owns its stored credential. An ambient family key must + # never be sent to that endpoint when the vault value is hidden. + if candidate.kind == "custom_provider" and ( + (candidate.endpoint and candidate.endpoint.base_url) + or candidate.endpoint_blocked + ): + return [] + + env_var = _provider_env_var(provider) or _provider_env_var(candidate.provider) + return [(env_var,)] if env_var else [] + + +def _environment_credential( + provider: str, candidate: "_ConnectionCandidate" +) -> Optional[Dict[str, str]]: + """This run's own credential for the candidate, or ``None`` when it has none. + + A channel counts only when EVERY variable in it is set: half an AWS key pair + authenticates nothing, and passing it on would fail at the provider with a + misleading error instead of here with an actionable one. + """ + for channel in _credential_channels(provider, candidate): + values = {name: (os.environ.get(name) or "").strip() for name in channel} + if all(values.values()): + return values + + return None + + def _header_name(secret: Dict[str, Any]) -> Optional[str]: return _stripped(_as_dict(secret.get("header")).get("name")) @@ -260,6 +315,9 @@ class _ConnectionCandidate: # harness intersection). Neither field filters resolution here yet. models: Optional[List[str]] = None harnesses: Optional[List[str]] = None + # True when value_status says a credential exists but this caller's + # credential received the redacted, value-less shape. + write_only_redacted: bool = False def matches_provider(self, provider: Optional[str]) -> bool: return bool( @@ -363,6 +421,20 @@ def _model_lookup_values(model: ModelRef, deployment: str) -> Set[str]: return {value for value in values if value} +def _write_only_redacted(secret: Dict[str, Any], has_credential: bool) -> bool: + """Whether the vault redacted this record's value for the current caller. + + ``has_credential`` must consider EVERY credential channel the candidate could use (the + primary key and the credential extras): a surviving config extra like ``AWS_REGION`` + must not read as "credentialed". + """ + return ( + bool(secret.get("write_only")) + and secret_value_configured(secret) + and not has_credential + ) + + def _provider_key_candidate(secret: Dict[str, Any]) -> Optional[_ConnectionCandidate]: data = _data(secret) provider = _stripped(data.get("kind")) @@ -380,6 +452,7 @@ def _provider_key_candidate(secret: Dict[str, Any]) -> Optional[_ConnectionCandi api_key=key, models=_saved_models(data), harnesses=_saved_harnesses(data), + write_only_redacted=_write_only_redacted(secret, bool(key)), ) @@ -445,6 +518,9 @@ def _custom_provider_candidate( ), models=_saved_models(data), harnesses=_saved_harnesses(data), + write_only_redacted=_write_only_redacted( + secret, bool(api_key) or bool(credential_extras(extras)) + ), ) @@ -587,6 +663,24 @@ def _resolve_from_secrets( else _choose_default(candidates, model, harness) ) provider = chosen.resolved_provider(model) + # Checked BEFORE the endpoint and env checks: a redacted write-only key is the deeper + # cause, and surviving config extras (AWS_REGION) can make `env` non-empty, which would + # otherwise let the run proceed mis-credentialed. + if chosen.write_only_redacted: + # The vault will never hand this caller the value, so the connection cannot supply + # the credential here. A provider key in this run's own environment is the + # documented way to run outside the platform, and it is what the error tells the + # user to do — so use it when it is there, and fail loud only when it is not. The + # key rides the variable it was read from, never a different channel. + fallback = _environment_credential(provider, chosen) + if fallback is None: + raise WriteOnlySecretError(slug=chosen.slug, provider=provider) + chosen = replace( + chosen, + api_key=None, + write_only_redacted=False, + env={**chosen.env, **fallback}, + ) # A chosen custom connection must carry a usable base URL. Failing here (rather than # returning endpoint=None) keeps the harness from falling back to a provider default and # silently ignoring the user's routing choice (design Decision 4). The error names the slug diff --git a/sdks/python/agenta/sdk/agents/platform/secrets.py b/sdks/python/agenta/sdk/agents/platform/secrets.py index 2ade52aca4..f92ef8a46f 100644 --- a/sdks/python/agenta/sdk/agents/platform/secrets.py +++ b/sdks/python/agenta/sdk/agents/platform/secrets.py @@ -23,6 +23,7 @@ from agenta.sdk.utils.logging import get_module_logger from ..capabilities import PROVIDER_ENV_VARS +from ..connections.credentials import secret_value_configured from .connection import PlatformConnection log = get_module_logger(__name__) @@ -60,9 +61,19 @@ async def resolve_named_secrets( "agent: named-secret read HTTP %s", response.status_code ) continue - value = _text_custom_secret_value(response.json()) + payload = response.json() + value = _text_custom_secret_value(payload) if value is not None: resolved[name] = value + elif _is_write_only_redacted(payload): + # Engineering copy; adjust freely. No secret name in the log (module + # policy above); the caller knows which names it requested. + log.error( + "agent: a requested named secret is write-only: its value " + "cannot be read back outside the platform runtime. For " + "standalone runs, set the value directly in the tool's " + "environment configuration instead." + ) except Exception: # pylint: disable=broad-except log.warning("agent: named-secret read failed", exc_info=True) @@ -72,6 +83,15 @@ async def resolve_named_secrets( return resolved +def _is_write_only_redacted(payload: Any) -> bool: + """Whether the vault says a value exists but redacted it for this caller.""" + return ( + isinstance(payload, dict) + and bool(payload.get("write_only")) + and secret_value_configured(payload) + ) + + def _text_custom_secret_value(payload: Any) -> Optional[str]: """Extract only the vault shape MCP headers can safely consume.""" if not isinstance(payload, dict) or payload.get("kind") != "custom_secret": diff --git a/sdks/python/agenta/sdk/middlewares/routing/auth.py b/sdks/python/agenta/sdk/middlewares/routing/auth.py index 6d11d89c9a..30d3e7d043 100644 --- a/sdks/python/agenta/sdk/middlewares/routing/auth.py +++ b/sdks/python/agenta/sdk/middlewares/routing/auth.py @@ -21,6 +21,42 @@ AGENTA_RUNTIME_PREFIX = getenv("AGENTA_RUNTIME_PREFIX", "") +# The platform runtime's proof of what it is, for the credential exchange below. +_RUNTIME_KEY_HEADER = "X-Agenta-Runtime-Key" + + +def _runtime_key_from_environment() -> str: + runtime_key = (getenv("AGENTA_SERVICES_INTERNAL_KEY") or "").strip() + + # The placeholder is public repository content, so it counts as no key at all. + return "" if runtime_key == "replace-me" else runtime_key + + +_RUNTIME_KEY = _runtime_key_from_environment() + +# Said once, at the point of use, because the failure it causes names something else +# entirely: a run whose connection holds a write-only secret gets the redacted shape and +# reports "provide the provider key in this run's environment", which is true for a +# standalone run and misleading here. An operator reading that has no way to reach this +# cause without being told. +_RUNTIME_KEY_WARNED = False + + +def _warn_once_about_the_missing_runtime_key() -> None: + global _RUNTIME_KEY_WARNED + + if _RUNTIME_KEY_WARNED: + return + + _RUNTIME_KEY_WARNED = True + log.warning( + "agenta: no platform runtime key configured " + "(AGENTA_SERVICES_INTERNAL_KEY is unset or uses the placeholder). " + "Runs against connections whose secret is write-only will fail to read it. " + "Set AGENTA_SERVICES_INTERNAL_KEY to the same value on the API and this service." + ) + + _AUTH_ENABLED = ( getenv("AGENTA_SERVICES_MIDDLEWARE_AUTH_ENABLED") or getenv("AGENTA_SERVICE_MIDDLEWARE_AUTH_ENABLED") @@ -98,6 +134,18 @@ async def get_credentials( authorization = request.headers.get("authorization", None) headers = {"Authorization": authorization} if authorization else None + # This service exchanges the END USER's credential on their behalf, so the token + # it sends says nothing about who is asking. The platform's own secret is what + # says "this is the runtime starting a run", and it is what lets the returned + # credential read write-only secret values. Sent only on this internal hop, never + # logged, never handed to the runner or into a sandbox. A deployment that does not + # set it simply gets a credential without that ability. + runtime_key = _RUNTIME_KEY + if runtime_key: + headers = {**(headers or {}), _RUNTIME_KEY_HEADER: runtime_key} + else: + _warn_once_about_the_missing_runtime_key() + # COOKIES access_token = request.cookies.get("sAccessToken", None) cookies = {"sAccessToken": access_token} if access_token else None diff --git a/sdks/python/agenta/sdk/middlewares/running/vault.py b/sdks/python/agenta/sdk/middlewares/running/vault.py index 565de15cc9..0cfd5ee8f5 100644 --- a/sdks/python/agenta/sdk/middlewares/running/vault.py +++ b/sdks/python/agenta/sdk/middlewares/running/vault.py @@ -10,6 +10,10 @@ from agenta.sdk.utils.exceptions import suppress, display_exception from agenta.sdk.utils.providers import normalize_provider_kind +from agenta.sdk.agents.connections.credentials import ( + credential_extras, + secret_value_configured, +) from agenta.sdk.models.workflows import WorkflowServiceRequest from agenta.sdk.contexts.running import RunningContext @@ -362,6 +366,18 @@ async def get_secrets( except Exception: # pylint: disable=bare-except display_exception("Vault: Vault Secrets Exception") + vault_secrets, redacted_names = _split_write_only_redacted(vault_secrets) + if redacted_names: + # Engineering copy; adjust freely. + log.error( + "Vault: %d secret(s) are write-only and were returned without their values " + "(%s). Their values cannot be read back outside the platform runtime; for " + "standalone runs, provide the provider key via its environment variable " + "(for example OPENAI_API_KEY) instead.", + len(redacted_names), + ", ".join(redacted_names), + ) + local_standard = {} # A project may hold several connections per provider family, so vault provider_key records # are kept as a list. Keying them by family (as the locals still are) would drop every @@ -408,6 +424,45 @@ async def get_secrets( return secrets, combined_vault, local_secrets +def _split_write_only_redacted( + vault_secrets: List[Dict[str, Any]], +) -> tuple[List[Dict[str, Any]], List[str]]: + """Drop entries whose value the vault redacted (write-only secret, non-granted caller). + + A redacted entry has no usable credential, so keeping it would either pass an empty key + to a provider or shadow a working env-var key of the same provider family. Returns the + usable entries and the display names of the dropped ones. + """ + usable: List[Dict[str, Any]] = [] + redacted_names: List[str] = [] + + for secret in vault_secrets or []: + if ( + isinstance(secret, dict) + and secret.get("write_only") + and secret_value_configured(secret) + ): + data = secret.get("data") or {} + kind = secret.get("kind") + value = None + if kind in ("provider_key", "custom_provider"): + provider = data.get("provider") or {} + value = provider.get("key") or credential_extras( + provider.get("extras") or {} + ) + elif kind == "custom_secret": + value = (data.get("secret") or {}).get("content") + + if not value: + header = secret.get("header") or {} + redacted_names.append(header.get("name") or secret.get("slug") or kind) + continue + + usable.append(secret) + + return usable, redacted_names + + def _has_invalid_secrets_error(response: Any) -> bool: status = getattr(response, "status", None) status_type = getattr(status, "type", None) diff --git a/sdks/python/oss/tests/pytest/unit/agents/connections/test_credentials_parity.py b/sdks/python/oss/tests/pytest/unit/agents/connections/test_credentials_parity.py new file mode 100644 index 0000000000..ca7d4b7686 --- /dev/null +++ b/sdks/python/oss/tests/pytest/unit/agents/connections/test_credentials_parity.py @@ -0,0 +1,42 @@ +"""The credential classifier and the resolver's extras vocabulary cannot drift. + +`credentials.py` is the single classification both the SDK resolver and the API's +write-only redaction consume. Every extras key the resolver accepts must be classified as +credential or config — adding a key to the resolver without classifying it fails here. +""" + +from agenta.sdk.agents.connections.credentials import ( + CONFIG_EXTRAS_KEYS, + CREDENTIAL_EXTRAS_KEYS, + credential_extras, +) +from agenta.sdk.agents.platform.connections import ( + _ALLOWED_EXTRA_ENV_KEYS, + _SNAKE_EXTRA_ENV_ALIASES, +) + + +def test_every_resolver_extras_key_is_classified(): + resolver_keys = set(_SNAKE_EXTRA_ENV_ALIASES) | set(_ALLOWED_EXTRA_ENV_KEYS) + unclassified = resolver_keys - (CREDENTIAL_EXTRAS_KEYS | CONFIG_EXTRAS_KEYS) + + assert not unclassified, ( + f"extras keys accepted by the resolver but unclassified in credentials.py: " + f"{sorted(unclassified)} — classify each as credential or config" + ) + + +def test_credential_and_config_classifications_are_disjoint(): + assert not (CREDENTIAL_EXTRAS_KEYS & CONFIG_EXTRAS_KEYS) + + +def test_credential_extras_keeps_only_non_empty_credential_material(): + extras = { + "api_key": "k", + "AWS_SECRET_ACCESS_KEY": "s", + "ANTHROPIC_AUTH_TOKEN": "", + "aws_region_name": "eu-west-1", + "unknown_key": "x", + } + + assert credential_extras(extras) == {"api_key": "k", "AWS_SECRET_ACCESS_KEY": "s"} diff --git a/sdks/python/oss/tests/pytest/unit/agents/platform/test_write_only_secrets.py b/sdks/python/oss/tests/pytest/unit/agents/platform/test_write_only_secrets.py new file mode 100644 index 0000000000..92bac7ac6d --- /dev/null +++ b/sdks/python/oss/tests/pytest/unit/agents/platform/test_write_only_secrets.py @@ -0,0 +1,413 @@ +"""SDK behavior when the vault redacts a write-only secret for this caller. + +The platform runtime reads write-only secrets in plaintext through its granted credential, +so in-platform runs never see the redacted shape. A standalone run (ApiKey credential) +does — and then falls back to this run's own provider key from the environment, failing +loud with instructions when there is none. It never passes an empty key to a provider. +""" + +from __future__ import annotations + +import pytest + +from agenta.sdk.agents.connections import ( + MissingCredentialError, + ModelRef, + WriteOnlySecretError, +) +from agenta.sdk.agents.capabilities import PROVIDER_ENV_VARS +from agenta.sdk.agents.platform import connections +from agenta.sdk.agents.platform.secrets import _is_write_only_redacted +from agenta.sdk.middlewares.running.vault import _split_write_only_redacted + + +@pytest.fixture(autouse=True) +def _no_ambient_provider_keys(monkeypatch): + """A developer machine exports provider keys; the fallback must not read them here. + + Every case below states its own environment, so the ambient one is cleared first. + """ + for name in set(PROVIDER_ENV_VARS.values()) | { + "AWS_BEARER_TOKEN_BEDROCK", + "AWS_ACCESS_KEY_ID", + "AWS_SECRET_ACCESS_KEY", + "AZURE_OPENAI_API_KEY", + "GOOGLE_APPLICATION_CREDENTIALS", + }: + monkeypatch.delenv(name, raising=False) + + +def _model(model: str = "gpt-5.5", provider: str = "openai") -> ModelRef: + return ModelRef(provider=provider, model=model, connection={"mode": "agenta"}) + + +def _redacted_provider_key(name: str = "OpenAI", provider: str = "openai") -> dict: + """The list-response shape a non-granted caller receives for a write-only secret.""" + return { + "kind": "provider_key", + "slug": f"{provider}-abc123", + "header": {"name": name}, + "data": {"kind": provider, "provider": {}}, + "write_only": True, + "value_status": {"configured": True, "preview": "sk-****abc"}, + } + + +def _plaintext_provider_key(provider: str = "openai", key: str = "sk-live-123") -> dict: + """The same secret as the granted runtime sees it: write-only, value present.""" + return { + "kind": "provider_key", + "slug": f"{provider}-abc123", + "header": {"name": "OpenAI"}, + "data": {"kind": provider, "provider": {"key": key}}, + "write_only": True, + } + + +def test_redacted_write_only_key_fails_loud_with_instructions(): + with pytest.raises(WriteOnlySecretError) as raised: + connections._resolve_from_secrets( + secrets=[_redacted_provider_key()], model=_model(), harness="pi_core" + ) + + message = str(raised.value) + assert "write-only" in message + assert "OPENAI_API_KEY" in message + assert "this run's environment" in message + # Never the misleading "add your key" error: the key exists, it is just unreadable here. + assert not isinstance(raised.value, MissingCredentialError) + + +def test_redacted_write_only_key_uses_this_runs_own_provider_key(monkeypatch): + # What the error text instructs, made true: the run's own key resolves the connection. + monkeypatch.setenv("OPENAI_API_KEY", "sk-from-env") + + resolved = connections._resolve_from_secrets( + secrets=[_redacted_provider_key()], model=_model(), harness="pi_core" + ) + + env = {item.binding.name: item.value for item in resolved.credentials} + assert env["OPENAI_API_KEY"] == "sk-from-env" + + +def test_a_key_for_another_provider_family_is_not_a_fallback(monkeypatch): + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-ant-env") + + with pytest.raises(WriteOnlySecretError): + connections._resolve_from_secrets( + secrets=[_redacted_provider_key()], model=_model(), harness="pi_core" + ) + + +def test_granted_plaintext_write_only_key_resolves_normally(): + resolved = connections._resolve_from_secrets( + secrets=[_plaintext_provider_key()], model=_model(), harness="pi_core" + ) + + env = {item.binding.name: item.value for item in resolved.credentials} + assert env["OPENAI_API_KEY"] == "sk-live-123" + + +def test_ordinary_keyless_secret_still_reports_missing_credential(): + keyless = { + "kind": "provider_key", + "slug": "openai-abc123", + "header": {"name": "OpenAI"}, + "data": {"kind": "openai", "provider": {}}, + } + + with pytest.raises(MissingCredentialError): + connections._resolve_from_secrets( + secrets=[keyless], model=_model(), harness="pi_core" + ) + + +def test_redacted_aws_only_secret_fails_loud_despite_surviving_config_extras(): + # After redaction an AWS-credentialed secret keeps only config extras (region). The + # resulting env is NON-empty, so an env-emptiness check alone would let the run + # proceed mis-credentialed; the write-only check must fire first. + redacted = { + "kind": "custom_provider", + "slug": "bedrock-conn", + "header": {"name": "bedrock-conn"}, + "data": { + "kind": "bedrock", + "provider": {"extras": {"aws_region_name": "eu-west-1"}}, + "models": [{"slug": "claude-opus-5"}], + "provider_slug": "bedrock-conn", + }, + "write_only": True, + "value_status": {"configured": True}, + } + + with pytest.raises(WriteOnlySecretError): + connections._resolve_from_secrets( + secrets=[redacted], + model=ModelRef( + provider="anthropic", + model="claude-opus-5", + connection={"mode": "agenta", "slug": "bedrock-conn"}, + ), + harness="claude_code", + ) + + +def test_a_bedrock_connection_never_falls_back_to_the_family_api_key(monkeypatch): + # Bedrock authenticates with a bearer token of its own; an Anthropic API key in the + # environment is a credential for a different service and must not be sent instead. + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-ant-env") + redacted = { + "kind": "custom_provider", + "slug": "bedrock-conn", + "header": {"name": "bedrock-conn"}, + "data": { + "kind": "bedrock", + "provider": {"extras": {"aws_region_name": "eu-west-1"}}, + "models": [{"slug": "claude-opus-5"}], + "provider_slug": "bedrock-conn", + }, + "write_only": True, + "value_status": {"configured": True}, + } + model = ModelRef( + provider="anthropic", + model="claude-opus-5", + connection={"mode": "agenta", "slug": "bedrock-conn"}, + ) + + with pytest.raises(WriteOnlySecretError): + connections._resolve_from_secrets( + secrets=[redacted], model=model, harness="claude_code" + ) + + # Its own channel does resolve it. + monkeypatch.setenv("AWS_BEARER_TOKEN_BEDROCK", "aws-bearer-env") + resolved = connections._resolve_from_secrets( + secrets=[redacted], model=model, harness="claude_code" + ) + env = {item.binding.name: item.value for item in resolved.credentials} + assert env["AWS_BEARER_TOKEN_BEDROCK"] == "aws-bearer-env" + + +def _redacted_custom(kind: str, slug: str, extras: dict | None = None) -> dict: + return { + "kind": "custom_provider", + "slug": slug, + "header": {"name": slug}, + "data": { + "kind": kind, + "provider": {"extras": extras or {}}, + "models": [{"slug": "claude-opus-5"}], + "provider_slug": slug, + }, + "write_only": True, + "value_status": {"configured": True}, + } + + +def _claude_model(slug: str) -> ModelRef: + return ModelRef( + provider="anthropic", + model="claude-opus-5", + connection={"mode": "agenta", "slug": slug}, + ) + + +def test_a_bedrock_connection_accepts_an_aws_key_pair_from_the_environment(monkeypatch): + # The same credential material the plaintext path takes from the vault's extras. + monkeypatch.setenv("AWS_ACCESS_KEY_ID", "AKIAEXAMPLE") + monkeypatch.setenv("AWS_SECRET_ACCESS_KEY", "aws-secret-from-env") + redacted = _redacted_custom( + "bedrock", "bedrock-conn", {"aws_region_name": "eu-west-1"} + ) + + resolved = connections._resolve_from_secrets( + secrets=[redacted], model=_claude_model("bedrock-conn"), harness="claude_code" + ) + + env = {item.binding.name: item.value for item in resolved.credentials} + assert env["AWS_ACCESS_KEY_ID"] == "AKIAEXAMPLE" + assert env["AWS_SECRET_ACCESS_KEY"] == "aws-secret-from-env" + + +def test_half_an_aws_key_pair_is_not_a_credential(monkeypatch): + # An access key id with no secret authenticates nothing; failing here names the + # problem, where passing it on would fail at the provider with an auth error. + monkeypatch.setenv("AWS_ACCESS_KEY_ID", "AKIAEXAMPLE") + redacted = _redacted_custom("bedrock", "bedrock-conn") + + with pytest.raises(WriteOnlySecretError): + connections._resolve_from_secrets( + secrets=[redacted], + model=_claude_model("bedrock-conn"), + harness="claude_code", + ) + + +def test_a_vertex_connection_accepts_google_application_credentials(monkeypatch): + monkeypatch.setenv("GOOGLE_APPLICATION_CREDENTIALS", "/run/secrets/service-account") + redacted = _redacted_custom( + "vertex_ai", "vertex-conn", {"vertex_ai_location": "europe-west1"} + ) + + resolved = connections._resolve_from_secrets( + secrets=[redacted], model=_claude_model("vertex-conn"), harness="claude_code" + ) + + env = {item.binding.name: item.value for item in resolved.credentials} + assert env["GOOGLE_APPLICATION_CREDENTIALS"] == "/run/secrets/service-account" + + +def test_plaintext_aws_only_secret_is_not_treated_as_redacted(): + plaintext = { + "kind": "custom_provider", + "slug": "bedrock-conn", + "header": {"name": "bedrock-conn"}, + "data": { + "kind": "bedrock", + "provider": { + "extras": { + "aws_access_key_id": "AKIA123", + "aws_secret_access_key": "shhh", + "aws_region_name": "eu-west-1", + } + }, + "models": [{"slug": "claude-opus-5"}], + "provider_slug": "bedrock-conn", + }, + "write_only": True, + } + + candidates = connections._catalog([plaintext]) + assert candidates[0].write_only_redacted is False + + +def test_redacted_custom_provider_fails_loud_too(): + redacted = { + "kind": "custom_provider", + "slug": "my-gateway", + "header": {"name": "my-gateway"}, + "data": { + "kind": "openai", + "provider": {"url": "https://gateway.example.com/v1"}, + "models": [{"slug": "gpt-5.5"}], + "provider_slug": "my-gateway", + }, + "write_only": True, + "value_status": {"configured": True}, + } + + model = ModelRef( + provider="openai", + model="gpt-5.5", + connection={"mode": "agenta", "slug": "my-gateway"}, + ) + + with pytest.raises(WriteOnlySecretError): + connections._resolve_from_secrets( + secrets=[redacted], model=model, harness="pi_core" + ) + + +def test_a_redacted_gateway_never_uses_this_runs_provider_key(monkeypatch): + ambient_key = "sk-gateway-env" + monkeypatch.setenv("OPENAI_API_KEY", ambient_key) + redacted = { + "kind": "custom_provider", + "slug": "my-gateway", + "header": {"name": "my-gateway"}, + "data": { + "kind": "openai", + "provider": {"url": "https://gateway.example.com/v1"}, + "models": [{"slug": "gpt-5.5"}], + "provider_slug": "my-gateway", + }, + "write_only": True, + "value_status": {"configured": True}, + } + + with pytest.raises(WriteOnlySecretError) as raised: + connections._resolve_from_secrets( + secrets=[redacted], + model=ModelRef( + provider="openai", + model="gpt-5.5", + connection={"mode": "agenta", "slug": "my-gateway"}, + ), + harness="pi_core", + ) + + assert ambient_key not in str(raised.value) + + +# --- the vault middleware's list partition --------------------------------------------- + + +def test_partition_drops_redacted_entries_and_names_them(): + usable, redacted_names = _split_write_only_redacted( + [ + _redacted_provider_key(name="Prod OpenAI"), + _plaintext_provider_key(provider="anthropic"), + { + "kind": "provider_key", + "header": {"name": "Legacy"}, + "data": {"kind": "mistral", "provider": {"key": "m-key"}}, + }, + ] + ) + + assert redacted_names == ["Prod OpenAI"] + assert [s["data"]["kind"] for s in usable] == ["anthropic", "mistral"] + + +def test_partition_keeps_write_only_entries_whose_value_came_through(): + usable, redacted_names = _split_write_only_redacted([_plaintext_provider_key()]) + + assert redacted_names == [] + assert len(usable) == 1 + + +def test_partition_drops_redacted_custom_secret_content(): + usable, redacted_names = _split_write_only_redacted( + [ + { + "kind": "custom_secret", + "slug": "gh-token", + "header": {"name": "gh-token"}, + "data": {"secret": {"format": "text"}}, + "write_only": True, + "value_status": {"configured": True}, + } + ] + ) + + assert usable == [] + assert redacted_names == ["gh-token"] + + +# --- named-secret redaction detection -------------------------------------------------- + + +def test_named_secret_redaction_is_detected(): + assert _is_write_only_redacted( + { + "kind": "custom_secret", + "write_only": True, + "value_status": {"configured": True}, + } + ) + assert not _is_write_only_redacted( + { + "kind": "custom_secret", + "write_only": True, + "value_status": {"configured": False}, + } + ) + assert not _is_write_only_redacted({"kind": "custom_secret"}) + assert not _is_write_only_redacted(None) + + +def test_legacy_has_key_is_not_a_supported_response_contract(): + assert not _is_write_only_redacted( + {"kind": "custom_secret", "write_only": True, "has_key": True} + ) diff --git a/sdks/python/oss/tests/pytest/unit/test_auth_middleware_credentials.py b/sdks/python/oss/tests/pytest/unit/test_auth_middleware_credentials.py new file mode 100644 index 0000000000..36ed2e25a2 --- /dev/null +++ b/sdks/python/oss/tests/pytest/unit/test_auth_middleware_credentials.py @@ -0,0 +1,152 @@ +"""Unit tests for the routing auth middleware's credential exchange. + +The workflow service exchanges the END USER's credential at `GET /permissions/check`, so +the token it forwards says nothing about who is asking. These tests pin the one thing that +does: the runtime key header that tells the platform a run is starting. +""" + +from __future__ import annotations + +from types import SimpleNamespace +from typing import Any, Dict, Optional + +import pytest + +from agenta.sdk.middlewares.routing import auth as auth_module +from agenta.sdk.utils.cache import TTLLRUCache + + +class _FakeRequest: + def __init__(self) -> None: + self.headers = {"authorization": "ApiKey caller-key"} + self.cookies: Dict[str, str] = {} + self.state = SimpleNamespace(otel={"baggage": {"project_id": "project-1"}}) + self.query_params: Dict[str, str] = {} + + +class _FakeResponse: + def __init__(self, body: Dict[str, Any]) -> None: + self.status_code = 200 + self._body = body + self.headers: Dict[str, str] = {} + + def json(self) -> Dict[str, Any]: + return self._body + + +class _FakeAsyncClient: + """Stands in for httpx.AsyncClient; records the headers each call carried.""" + + call_count = 0 + body: Dict[str, Any] = {} + last_headers: Optional[Dict[str, str]] = None + + async def __aenter__(self) -> "_FakeAsyncClient": + return self + + async def __aexit__(self, *args: Any) -> None: + return None + + async def get(self, *args: Any, **kwargs: Any) -> _FakeResponse: + type(self).call_count += 1 + type(self).last_headers = kwargs.get("headers") + return _FakeResponse(type(self).body) + + +@pytest.fixture +def platform(monkeypatch): + monkeypatch.setattr(auth_module, "_AUTH_ENABLED", True) + monkeypatch.setattr(auth_module, "_CACHE_ENABLED", True) + monkeypatch.setattr(auth_module, "_cache", TTLLRUCache()) + _FakeAsyncClient.call_count = 0 + _FakeAsyncClient.body = {} + _FakeAsyncClient.last_headers = None + monkeypatch.setattr(auth_module.httpx, "AsyncClient", _FakeAsyncClient) + return _FakeAsyncClient + + +async def _get_credentials() -> Optional[str]: + return await auth_module.get_credentials( + _FakeRequest(), # type: ignore[arg-type] + "http://agenta.test", + ) + + +async def test_deny_body_raises(platform): + platform.body = {"effect": "deny"} + + with pytest.raises(auth_module.DenyException): + await _get_credentials() + + +async def test_the_runtime_key_rides_the_exchange_when_configured( + platform, monkeypatch +): + # What tells the platform that a run is starting, rather than a browser asking for a + # credential: the exchange forwards the END USER's token either way, so this header + # is the only thing that distinguishes them. + monkeypatch.setattr(auth_module, "_RUNTIME_KEY", "runtime-key-for-tests") + platform.body = {"effect": "allow", "credentials": "Secret general-token"} + + await _get_credentials() + + assert platform.last_headers["X-Agenta-Runtime-Key"] == "runtime-key-for-tests" + # The caller's own credential still travels; the key identifies the runtime, it does + # not replace the principal. + assert platform.last_headers["Authorization"] == "ApiKey caller-key" + + +async def test_no_runtime_key_means_no_header(platform, monkeypatch): + # A deployment that configures none simply gets an ungranted credential; it must not + # send an empty header that a comparison might treat as a value. + monkeypatch.setattr(auth_module, "_RUNTIME_KEY", "") + platform.body = {"effect": "allow", "credentials": "Secret general-token"} + + await _get_credentials() + + assert "X-Agenta-Runtime-Key" not in (platform.last_headers or {}) + + +async def test_a_missing_runtime_key_says_so_once(platform, monkeypatch, caplog): + # The failure it causes reports a missing provider key, which is the wrong advice for + # this cause. The standalone-run advice is misleading here, and an operator who never + # set the dedicated runtime key has no other way to reach the actual cause. + monkeypatch.setattr(auth_module, "_RUNTIME_KEY", "") + monkeypatch.setattr(auth_module, "_RUNTIME_KEY_WARNED", False) + platform.body = {"effect": "allow", "credentials": "Secret general-token"} + + with caplog.at_level("WARNING"): + await _get_credentials() + await _get_credentials() + + notices = [ + record + for record in caplog.records + if "no platform runtime key configured" in record.getMessage() + ] + assert len(notices) == 1 + assert "AGENTA_SERVICES_INTERNAL_KEY" in notices[0].getMessage() + assert "AGENTA_AUTH_KEY" not in notices[0].getMessage() + + +def test_runtime_key_configuration_never_falls_back_to_the_admin_key(monkeypatch): + configured = {"AGENTA_AUTH_KEY": "administrator-key"} + monkeypatch.setattr( + auth_module, + "getenv", + lambda name, default=None: configured.get(name, default), + ) + + assert auth_module._runtime_key_from_environment() == "" + + +def test_runtime_key_placeholder_is_treated_as_unconfigured(monkeypatch): + monkeypatch.setattr( + auth_module, + "getenv", + lambda name, default=None: ( + "replace-me" if name == "AGENTA_SERVICES_INTERNAL_KEY" else default + ), + ) + + assert auth_module._runtime_key_from_environment() == "" diff --git a/services/oss/tests/pytest/unit/agent/test_credential_exchange.py b/services/oss/tests/pytest/unit/agent/test_credential_exchange.py new file mode 100644 index 0000000000..509fd3fcb9 --- /dev/null +++ b/services/oss/tests/pytest/unit/agent/test_credential_exchange.py @@ -0,0 +1,91 @@ +"""What the agent app sends when it exchanges a caller's credential. + +This is the hop the product actually takes: the browser (or the release gate) posts to the +agent service with the END USER's ApiKey, and the service exchanges it at +`/access/permissions/check` for the credential a run uses to read the project's secrets. +Nothing about that ApiKey says a run is starting, and the exchange route is publicly +reachable, so the platform's own secret is what distinguishes this hop from a browser +asking for a credential directly. Drop it and every run against a write-only secret +resolves to the redacted shape and fails; leak it and the guarantee is gone. Hence a test +on the real app, not just on the middleware in isolation. +""" + +from __future__ import annotations + +from typing import Any, Dict, Optional + +import pytest +from fastapi.testclient import TestClient + +from agenta.sdk.middlewares.routing import auth as auth_middleware + +from oss.src.agent import agent_v0_app + + +class _FakeResponse: + def __init__(self, body: Dict[str, Any]) -> None: + self.status_code = 200 + self._body = body + self.headers: Dict[str, str] = {} + + def json(self) -> Dict[str, Any]: + return self._body + + +class _RecordingClient: + """Stands in for the httpx client the exchange uses, keeping the headers it saw.""" + + last_headers: Optional[Dict[str, str]] = None + + async def __aenter__(self) -> "_RecordingClient": + return self + + async def __aexit__(self, *args: Any) -> None: + return None + + async def get(self, *args: Any, **kwargs: Any) -> _FakeResponse: + type(self).last_headers = kwargs.get("headers") + return _FakeResponse( + {"effect": "allow", "credentials": "Secret granted-run-credential"} + ) + + +@pytest.fixture(name="exchange") +def _exchange(monkeypatch): + monkeypatch.setattr(auth_middleware, "_AUTH_ENABLED", True) + monkeypatch.setattr(auth_middleware, "_CACHE_ENABLED", False) + monkeypatch.setattr(auth_middleware.httpx, "AsyncClient", _RecordingClient) + _RecordingClient.last_headers = None + return _RecordingClient + + +def _post(client: TestClient): + return client.post( + "/runtime/subscription-status", + json={"harness": "codex"}, + headers={"Authorization": "ApiKey caller-key"}, + ) + + +def test_the_service_proves_what_it_is_when_it_exchanges_a_users_key( + exchange, monkeypatch +): + monkeypatch.setattr(auth_middleware, "_RUNTIME_KEY", "runtime-key-for-tests") + + _post(TestClient(agent_v0_app)) + + headers = exchange.last_headers or {} + assert headers.get("X-Agenta-Runtime-Key") == "runtime-key-for-tests" + # The user's own credential still travels: the key says which RUNTIME is asking, it + # does not change WHO is asking. + assert headers.get("Authorization") == "ApiKey caller-key" + + +def test_a_service_without_the_key_sends_none(exchange, monkeypatch): + # Such a deployment gets an ungranted credential and cannot run against write-only + # secrets — it must not send an empty header a comparison might accept. + monkeypatch.setattr(auth_middleware, "_RUNTIME_KEY", "") + + _post(TestClient(agent_v0_app)) + + assert "X-Agenta-Runtime-Key" not in (exchange.last_headers or {})