From da99c85a5ee5366683dc5e98ae8189041836bdeb Mon Sep 17 00:00:00 2001 From: Stran Dutton Date: Fri, 11 Sep 2026 12:59:44 -0500 Subject: [PATCH 1/5] BED-9509: add aws secrets manager service --- pyproject.toml | 2 + .../core/clients/aws_secrets_manager.py | 396 ++++++++++++++++++ tests/test_aws_secrets_manager.py | 329 +++++++++++++++ uv.lock | 84 ++++ 4 files changed, 811 insertions(+) create mode 100644 src/openhound/core/clients/aws_secrets_manager.py create mode 100644 tests/test_aws_secrets_manager.py diff --git a/pyproject.toml b/pyproject.toml index ab5f718..5206f59 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -19,6 +19,7 @@ dependencies = [ "jinja2>=3.1.6", "types-requests==2.33.0.20260503", "pydantic-extra-types>=2.11.1", + "boto3>=1.43.90", ] [project.optional-dependencies] @@ -70,6 +71,7 @@ dev = [ "ipython>=9.13.0", "pre-commit>=4.5.1", "pytest>=9.0.1", + "boto3-stubs~=1.43.90", "marimo>=0.23.5", "altair>=6.1.0", "fastapi>=0.136.1", diff --git a/src/openhound/core/clients/aws_secrets_manager.py b/src/openhound/core/clients/aws_secrets_manager.py new file mode 100644 index 0000000..ae877e4 --- /dev/null +++ b/src/openhound/core/clients/aws_secrets_manager.py @@ -0,0 +1,396 @@ +import json +import logging +from collections.abc import Iterable, Mapping +from typing import Any, Protocol, cast + +import boto3 +from botocore.exceptions import ( + BotoCoreError, + ClientError, + NoCredentialsError, + NoRegionError, +) + +logger = logging.getLogger(__name__) + +MAX_BATCH_SIZE = 20 +_PROVIDER = "aws.secretsmanager" + + +class SecretsManagerClient(Protocol): + def get_secret_value(self, *, SecretId: str) -> Mapping[str, Any]: ... + + def batch_get_secret_value( + self, *, SecretIdList: list[str] + ) -> Mapping[str, Any]: ... + + +class SecretRetrievalError(Exception): + """Base class for errors retrieving a secret.""" + + +class SecretNotFoundError(SecretRetrievalError): + """The requested secret does not exist.""" + + +class SecretPermissionError(SecretRetrievalError): + """The configured AWS identity cannot read the requested secret.""" + + +class AWSConfigurationError(SecretRetrievalError): + """The AWS SDK or Secrets Manager client is not configured correctly.""" + + +class SecretRequestError(SecretRetrievalError): + """AWS rejected a request for an otherwise unclassified reason.""" + + +class SecretBatchError(SecretRetrievalError): + """One or more requested secrets could not be retrieved.""" + + def __init__(self, failures: Mapping[str, SecretRetrievalError]) -> None: + self.failures = dict(failures) + super().__init__("One or more AWS secret retrievals failed") + + +class InvalidSecretValueError(SecretRetrievalError): + """The retrieved secret uses an unsupported or invalid format.""" + + +SecretValue = str | Mapping[str, Any] + + +class AWSSecretsManager: + def __init__(self, client: SecretsManagerClient | None = None) -> None: + if client is not None and not ( + callable(getattr(client, "get_secret_value", None)) + or callable(getattr(client, "batch_get_secret_value", None)) + ): + raise AWSConfigurationError( + "AWS Secrets Manager client does not support secret retrieval" + ) + + self._client = client + + def get_secret(self, secret_id: str) -> SecretValue: + """Retrieve one text or JSON-object secret.""" + _log_attempt("get_secret_value") + try: + client = self._get_client() + except SecretRetrievalError: + _log_failure("get_secret_value") + _log_result("get_secret_value", "failure") + raise + return self._retrieve_secret(client, secret_id) + + def get_secrets(self, secret_ids: Iterable[str]) -> dict[str, SecretValue]: + """Retrieve any number of secrets in AWS-supported batch sizes.""" + requested_ids = list(secret_ids) + if not requested_ids: + return {} + + try: + client = self._get_client() + except SecretRetrievalError as error: + _log_attempt("batch_get_secret_value", len(requested_ids)) + _log_failure("batch_get_secret_value", len(requested_ids)) + _log_result("batch_get_secret_value", "failure", len(requested_ids)) + failures = {secret_id: error for secret_id in requested_ids} + raise SecretBatchError(failures) from None + if not callable(getattr(client, "batch_get_secret_value", None)): + _log_attempt("batch_get_secret_value", len(requested_ids)) + _log_result("batch_get_secret_value", "fallback", len(requested_ids)) + return self._get_secrets_individually(client, requested_ids) + + values: dict[str, SecretValue] = {} + for chunk_index, chunk in enumerate(_chunks(requested_ids, MAX_BATCH_SIZE)): + secret_count = len(chunk) + _log_attempt("batch_get_secret_value", secret_count) + + try: + response = client.batch_get_secret_value(SecretIdList=chunk) + except ClientError as error: + if _is_batch_fallback_error(error): + _log_result("batch_get_secret_value", "fallback", secret_count) + values.update( + self._get_secrets_individually( + client, + requested_ids[chunk_index * MAX_BATCH_SIZE :], + ) + ) + return values + classified = _classify_client_error(error) + failures = {secret_id: classified for secret_id in chunk} + _log_failure("batch_get_secret_value", secret_count) + _log_result("batch_get_secret_value", "failure", secret_count) + raise SecretBatchError(failures) from None + except BotoCoreError as error: + classified = _classify_boto_core_error(error) + failures = {secret_id: classified for secret_id in chunk} + _log_failure("batch_get_secret_value", secret_count) + _log_result("batch_get_secret_value", "failure", secret_count) + raise SecretBatchError(failures) from None + + failures = _batch_response_failures(response, chunk) + chunk_values, value_failures = _parse_batch_values(response, chunk) + for secret_id, value_error in value_failures.items(): + failures.setdefault(secret_id, value_error) + if failures: + _log_failure("batch_get_secret_value", secret_count) + _log_result("batch_get_secret_value", "failure", secret_count) + raise SecretBatchError(failures) from None + + values.update({secret_id: chunk_values[secret_id] for secret_id in chunk}) + _log_result("batch_get_secret_value", "success", secret_count) + + return values + + def _get_client(self) -> SecretsManagerClient: + if self._client is not None: + return self._client + try: + return cast(SecretsManagerClient, boto3.client("secretsmanager")) + except BotoCoreError as error: + raise _classify_boto_core_error(error) from None + + def _retrieve_secret( + self, client: SecretsManagerClient, secret_id: str + ) -> SecretValue: + _log_attempt("get_secret_value") + try: + if not callable(getattr(client, "get_secret_value", None)): + raise AWSConfigurationError( + "AWS Secrets Manager client does not support secret retrieval" + ) + response = client.get_secret_value(SecretId=secret_id) + value = _parse_secret_value(response) + except SecretRetrievalError: + _log_failure("get_secret_value") + _log_result("get_secret_value", "failure") + raise + except ClientError as error: + classified = _classify_client_error(error) + _log_failure("get_secret_value") + _log_result("get_secret_value", "failure") + raise classified from None + except BotoCoreError as error: + classified = _classify_boto_core_error(error) + _log_failure("get_secret_value") + _log_result("get_secret_value", "failure") + raise classified from None + + _log_result("get_secret_value", "success") + return value + + def _get_secrets_individually( + self, client: SecretsManagerClient, secret_ids: list[str] + ) -> dict[str, SecretValue]: + values: dict[str, SecretValue] = {} + failures: dict[str, SecretRetrievalError] = {} + for secret_id in secret_ids: + try: + values[secret_id] = self._retrieve_secret(client, secret_id) + except SecretRetrievalError as error: + failures[secret_id] = error + if failures: + raise SecretBatchError(failures) from None + return values + + +def _chunks(values: list[str], size: int) -> Iterable[list[str]]: + for start in range(0, len(values), size): + yield values[start : start + size] + + +def _parse_secret_value(response: Mapping[str, Any]) -> SecretValue: + if not isinstance(response, Mapping): + raise InvalidSecretValueError("AWS secret response has an invalid format") + if response.get("SecretBinary") is not None: + raise InvalidSecretValueError( + "SecretBinary payloads are not supported; use SecretString" + ) + + secret_string = response.get("SecretString") + if not isinstance(secret_string, str): + raise InvalidSecretValueError( + "AWS secret response did not contain a valid SecretString payload" + ) + + candidate = secret_string.strip() + if candidate.startswith(("{", "[")): + try: + parsed = json.loads(secret_string) + except json.JSONDecodeError: + raise InvalidSecretValueError( + "AWS secret contains malformed JSON" + ) from None + if not isinstance(parsed, dict): + raise InvalidSecretValueError( + "Only JSON-object secret payloads are supported" + ) + return parsed + + return secret_string + + +def _parse_batch_values( + response: Mapping[str, Any], + requested_ids: list[str], +) -> tuple[dict[str, SecretValue], dict[str, SecretRetrievalError]]: + if not isinstance(response, Mapping): + error = InvalidSecretValueError("AWS batch response has an invalid format") + return {}, {secret_id: error for secret_id in requested_ids} + + entries = response.get("SecretValues", []) + if not isinstance(entries, list): + response_error = InvalidSecretValueError( + "AWS batch response has an invalid format" + ) + return {}, {secret_id: response_error for secret_id in requested_ids} + + values: dict[str, SecretValue] = {} + failures: dict[str, SecretRetrievalError] = {} + requested_set = set(requested_ids) + for entry in entries: + if not isinstance(entry, Mapping): + response_error = InvalidSecretValueError( + "AWS batch response has an invalid format" + ) + for secret_id in requested_ids: + failures.setdefault(secret_id, response_error) + break + + matched_secret_id = _matching_requested_id(entry, requested_set) + if matched_secret_id is None: + continue + try: + values[matched_secret_id] = _parse_secret_value(entry) + except SecretRetrievalError as value_error: + failures[matched_secret_id] = value_error + + for secret_id in requested_ids: + if secret_id not in values and secret_id not in failures: + failures[secret_id] = SecretRequestError( + "AWS batch response omitted a requested secret" + ) + + return values, failures + + +def _batch_response_failures( + response: Mapping[str, Any], + requested_ids: list[str], +) -> dict[str, SecretRetrievalError]: + if not isinstance(response, Mapping): + return {} + entries = response.get("Errors", []) + if not isinstance(entries, list): + return { + secret_id: InvalidSecretValueError( + "AWS batch response has an invalid format" + ) + for secret_id in requested_ids + } + + requested_set = set(requested_ids) + failures: dict[str, SecretRetrievalError] = {} + for index, entry in enumerate(entries): + if not isinstance(entry, Mapping): + failures[f""] = InvalidSecretValueError( + "AWS batch response has an invalid format" + ) + continue + secret_id = entry.get("SecretId") + if not isinstance(secret_id, str) or secret_id not in requested_set: + secret_id = f"" + failures[secret_id] = _classify_error_code(entry.get("ErrorCode")) + return failures + + +def _matching_requested_id( + entry: Mapping[str, Any], requested_ids: set[str] +) -> str | None: + for field in ("Name", "ARN"): + value = entry.get(field) + if isinstance(value, str) and value in requested_ids: + return value + return None + + +def _classify_client_error(error: ClientError) -> SecretRetrievalError: + return _classify_error_code(_client_error_code(error)) + + +def _classify_boto_core_error(error: BotoCoreError) -> SecretRetrievalError: + if isinstance(error, (NoCredentialsError, NoRegionError)): + return AWSConfigurationError( + "AWS Secrets Manager client configuration is invalid" + ) + return SecretRequestError("AWS Secrets Manager request could not be completed") + + +def _is_batch_fallback_error(error: ClientError) -> bool: + return _client_error_code(error) in { + "AccessDenied", + "AccessDeniedException", + "UnauthorizedOperation", + "UnknownOperationException", + "InvalidAction", + } + + +def _classify_error_code(error_code: Any) -> SecretRetrievalError: + if error_code == "ResourceNotFoundException": + return SecretNotFoundError("AWS secret was not found") + if error_code in { + "AccessDenied", + "AccessDeniedException", + "AuthorizationError", + "UnauthorizedOperation", + "UnrecognizedClientException", + }: + return SecretPermissionError("AWS Secrets Manager denied access to the secret") + return SecretRequestError("AWS Secrets Manager request could not be completed") + + +def _client_error_code(error: ClientError) -> Any: + return error.response.get("Error", {}).get("Code") + + +def _log_attempt(operation: str, secret_count: int | None = None) -> None: + extra: dict[str, str | int] = { + "provider": _PROVIDER, + "operation": operation, + "outcome": "attempt", + } + if secret_count is not None: + extra["secret_count"] = secret_count + logger.debug( + "Secret retrieval attempt", + extra=extra, + ) + + +def _log_result(operation: str, outcome: str, secret_count: int | None = None) -> None: + extra: dict[str, str | int] = { + "provider": _PROVIDER, + "operation": operation, + "outcome": outcome, + } + if secret_count is not None: + extra["secret_count"] = secret_count + logger.debug( + "Secret retrieval result", + extra=extra, + ) + + +def _log_failure(operation: str, secret_count: int | None = None) -> None: + extra: dict[str, str | int] = { + "provider": _PROVIDER, + "operation": operation, + "outcome": "failure", + } + if secret_count is not None: + extra["secret_count"] = secret_count + logger.error("Failure to retrieve secret", extra=extra) diff --git a/tests/test_aws_secrets_manager.py b/tests/test_aws_secrets_manager.py new file mode 100644 index 0000000..9ae2694 --- /dev/null +++ b/tests/test_aws_secrets_manager.py @@ -0,0 +1,329 @@ +import logging +from typing import Any + +import pytest +from botocore.exceptions import ( + ClientError, + EndpointConnectionError, + NoCredentialsError, + NoRegionError, +) + +from openhound.core.clients.aws_secrets_manager import ( + AWSConfigurationError, + AWSSecretsManager, + InvalidSecretValueError, + SecretBatchError, + SecretNotFoundError, + SecretPermissionError, + SecretRequestError, +) + + +class FakeSecretsManagerClient: + def __init__( + self, + response: dict[str, Any] | None = None, + error=None, + batch_response: dict[str, Any] | None = None, + batch_error=None, + ) -> None: + self.response = response + self.error = error + self.batch_response = batch_response + self.batch_error = batch_error + self.requested_secret_id = None + self.batch_requests: list[list[str]] = [] + + def get_secret_value(self, *, SecretId: str) -> dict[str, Any]: + self.requested_secret_id = SecretId + if self.error is not None: + raise self.error + assert self.response is not None + return self.response + + def batch_get_secret_value(self, *, SecretIdList: list[str]) -> dict[str, Any]: + self.batch_requests.append(SecretIdList) + if self.batch_error is not None: + raise self.batch_error + assert self.batch_response is not None + return self.batch_response + + +def aws_error(code: str, message: str) -> ClientError: + return ClientError( + { + "Error": {"Code": code, "Message": message}, + "ResponseMetadata": {"HTTPStatusCode": 400}, + }, + "GetSecretValue", + ) + + +def test_get_secret_returns_plain_text_and_passes_secret_id() -> None: + client = FakeSecretsManagerClient({"SecretString": "secret-value"}) + + result = AWSSecretsManager(client).get_secret("secret-id") + + assert result == "secret-value" + assert client.requested_secret_id == "secret-id" + + +def test_get_secret_returns_json_object() -> None: + client = FakeSecretsManagerClient({"SecretString": '{"username":"user"}'}) + + result = AWSSecretsManager(client).get_secret("secret-id") + + assert result == {"username": "user"} + + +@pytest.mark.parametrize( + ("error_code", "exception_type"), + [ + ("ResourceNotFoundException", SecretNotFoundError), + ("AccessDeniedException", SecretPermissionError), + ], +) +def test_get_secret_maps_aws_errors_without_leaking_values( + error_code: str, exception_type: type[Exception], caplog: pytest.LogCaptureFixture +) -> None: + secret_value = "do-not-leak-this-value" + client = FakeSecretsManagerClient(error=aws_error(error_code, secret_value)) + + with caplog.at_level(logging.DEBUG), pytest.raises(exception_type) as raised: + AWSSecretsManager(client).get_secret("secret-id") + + assert secret_value not in str(raised.value) + assert secret_value not in caplog.text + assert "secret-id" not in caplog.text + assert "2d4e223a1833" not in caplog.text + assert ( + sum( + record.levelno == logging.ERROR + and record.getMessage() == "Failure to retrieve secret" + for record in caplog.records + ) + == 1 + ) + messages = [record.getMessage() for record in caplog.records] + assert "Secret retrieval attempt" in messages + assert "Secret retrieval result" in messages + + +def test_get_secret_rejects_malformed_json_without_leaking_value() -> None: + secret_value = '{"password": "do-not-leak-this-value"' + client = FakeSecretsManagerClient({"SecretString": secret_value}) + + with pytest.raises(InvalidSecretValueError) as raised: + AWSSecretsManager(client).get_secret("secret-id") + + assert secret_value not in str(raised.value) + + +def test_get_secret_rejects_binary_payload() -> None: + client = FakeSecretsManagerClient({"SecretBinary": b"secret-value"}) + + with pytest.raises(InvalidSecretValueError, match="SecretBinary"): + AWSSecretsManager(client).get_secret("secret-id") + + +def test_get_secret_classifies_network_failures_as_request_errors() -> None: + client = FakeSecretsManagerClient( + error=EndpointConnectionError(endpoint_url="https://secretsmanager.example") + ) + + with pytest.raises(SecretRequestError): + AWSSecretsManager(client).get_secret("secret-id") + + +def test_get_secrets_returns_values_mapped_to_requested_ids() -> None: + client = FakeSecretsManagerClient( + batch_response={ + "SecretValues": [ + {"Name": "second", "SecretString": '{"value":2}'}, + {"Name": "first", "SecretString": "value-1"}, + ] + } + ) + + result = AWSSecretsManager(client).get_secrets(["first", "second"]) + + assert result == {"first": "value-1", "second": {"value": 2}} + assert client.batch_requests == [["first", "second"]] + + +def test_get_secrets_falls_back_when_batch_api_is_unavailable() -> None: + class LegacyClient: + def __init__(self) -> None: + self.requested_ids: list[str] = [] + + def get_secret_value(self, *, SecretId: str) -> dict[str, str]: + self.requested_ids.append(SecretId) + return {"SecretString": f"value-{SecretId}"} + + client = LegacyClient() + + result = AWSSecretsManager(client).get_secrets(["first", "second"]) + + assert result == {"first": "value-first", "second": "value-second"} + assert client.requested_ids == ["first", "second"] + + +def test_get_secrets_falls_back_when_batch_permission_is_missing() -> None: + class BatchPermissionClient(FakeSecretsManagerClient): + def get_secret_value(self, *, SecretId: str) -> dict[str, str]: + self.requested_secret_id = SecretId + return {"SecretString": f"value-{SecretId}"} + + client = BatchPermissionClient( + batch_error=aws_error("AccessDeniedException", "batch permission required") + ) + + result = AWSSecretsManager(client).get_secrets(["first", "second"]) + + assert result == {"first": "value-first", "second": "value-second"} + assert client.batch_requests == [["first", "second"]] + assert client.requested_secret_id == "second" + + +def test_get_secrets_splits_requests_into_chunks_of_20() -> None: + secret_ids = [f"secret-{index}" for index in range(43)] + + def response_for(requested_ids: list[str]) -> dict[str, Any]: + return { + "SecretValues": [ + {"Name": secret_id, "SecretString": secret_id} + for secret_id in reversed(requested_ids) + ] + } + + class ChunkingClient(FakeSecretsManagerClient): + def batch_get_secret_value(self, *, SecretIdList: list[str]) -> dict[str, Any]: + self.batch_requests.append(SecretIdList) + return response_for(SecretIdList) + + client = ChunkingClient() + + result = AWSSecretsManager(client).get_secrets(secret_ids) + + assert list(result) == secret_ids + assert [len(request) for request in client.batch_requests] == [20, 20, 3] + + +def test_get_secrets_raises_typed_error_for_partial_batch_failure( + caplog: pytest.LogCaptureFixture, +) -> None: + secret_value = "do-not-log-this-value" + client = FakeSecretsManagerClient( + batch_response={ + "SecretValues": [{"Name": "available", "SecretString": secret_value}], + "Errors": [ + { + "SecretId": "missing", + "ErrorCode": "ResourceNotFoundException", + "ErrorMessage": secret_value, + } + ], + } + ) + + with caplog.at_level(logging.DEBUG), pytest.raises(SecretBatchError) as raised: + AWSSecretsManager(client).get_secrets(["available", "missing"]) + + assert isinstance(raised.value.failures["missing"], SecretNotFoundError) + assert secret_value not in str(raised.value) + assert secret_value not in caplog.text + assert "missing" not in caplog.text + assert "ffa63583dfa6" not in caplog.text + assert ( + sum( + record.levelno == logging.ERROR + and record.getMessage() == "Failure to retrieve secret" + for record in caplog.records + ) + == 1 + ) + for record in caplog.records: + record_text = repr(record.__dict__) + assert "available" not in record_text + assert "missing" not in record_text + assert "ffa63583dfa6" not in record_text + assert record.provider == "aws.secretsmanager" + assert record.operation == "batch_get_secret_value" + assert record.secret_count == 2 + + +@pytest.mark.parametrize( + ("error", "expected"), + [ + (NoCredentialsError(), AWSConfigurationError), + (NoRegionError(), AWSConfigurationError), + ( + EndpointConnectionError(endpoint_url="https://secretsmanager.example"), + SecretRequestError, + ), + (aws_error("SomeOtherError", "do-not-leak-this-value"), SecretRequestError), + ], +) +def test_get_secrets_classifies_sdk_failures(error, expected) -> None: + client = FakeSecretsManagerClient(batch_error=error) + + with pytest.raises(SecretBatchError) as raised: + AWSSecretsManager(client).get_secrets(["secret-id"]) + + assert isinstance(raised.value.failures["secret-id"], expected) + + +def test_retrieval_logs_attempt_and_result_without_secret_values( + caplog: pytest.LogCaptureFixture, +) -> None: + text_value = "plain-secret-value" + json_value = '{"password":"json-secret-value"}' + client = FakeSecretsManagerClient( + batch_response={ + "SecretValues": [ + {"Name": "text-secret", "SecretString": text_value}, + {"Name": "json-secret", "SecretString": json_value}, + ] + } + ) + + with caplog.at_level(logging.DEBUG): + AWSSecretsManager(client).get_secrets(["text-secret", "json-secret"]) + + messages = [record.getMessage() for record in caplog.records] + assert "Secret retrieval attempt" in messages + assert "Secret retrieval result" in messages + forbidden_values = { + "text-secret", + "json-secret", + text_value, + json_value, + "a5353f2b7585", + "9bda9177a37d", + } + for record in caplog.records: + record_text = repr(record.__dict__) + assert all(value not in record_text for value in forbidden_values) + assert record.provider == "aws.secretsmanager" + assert record.operation == "batch_get_secret_value" + assert record.secret_count == 2 + + assert text_value not in caplog.text + assert json_value not in caplog.text + assert "Secret retrieval attempt" in messages + assert "Secret retrieval result" in messages + + +def test_constructor_rejects_invalid_client() -> None: + with pytest.raises(AWSConfigurationError, match="does not support"): + AWSSecretsManager(object()) + + +def test_constructor_does_not_create_an_aws_client(monkeypatch) -> None: + monkeypatch.setattr( + "openhound.core.clients.aws_secrets_manager.boto3.client", + lambda *_: pytest.fail(), + ) + + AWSSecretsManager() diff --git a/uv.lock b/uv.lock index d00e9cb..8cce2cd 100644 --- a/uv.lock +++ b/uv.lock @@ -159,6 +159,56 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/cd/0c/31cfaa6b56fe23488ecb993bc9fc526c0d84d89607decdf2a10776426c2e/binaryornot-0.6.0-py3-none-any.whl", hash = "sha256:900adfd5e1b821255ba7e63139b0396b14c88b9286e74e03b6f51e0200331337", size = 14185, upload-time = "2026-03-08T16:26:27.466Z" }, ] +[[package]] +name = "boto3" +version = "1.43.90" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "botocore" }, + { name = "jmespath" }, + { name = "s3transfer" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/67/0b/53f833efea7331aab106a65f1156e0035beed9bacbfcb309b128204468d4/boto3-1.43.90.tar.gz", hash = "sha256:4b669742d5b45b8fd20ca50ac414a4e4cf995ebb8f280d21be28676e71c97594", size = 112689, upload-time = "2026-09-08T19:22:30.875Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/4b/c3/1d5c5ea7f599e2b80d122beeb2a9c1a8daff78325ea2391521e5fd5af1fd/boto3-1.43.90-py3-none-any.whl", hash = "sha256:aaaa1216d65ddb3dcf86bf9d93cfa436af05cbcd1e1f8b2847f2b83116012186", size = 140026, upload-time = "2026-09-08T19:22:28.555Z" }, +] + +[[package]] +name = "boto3-stubs" +version = "1.43.90" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "botocore-stubs" }, + { name = "types-s3transfer" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/db/cb/d5967b06c53f59afe9dea69e3018aad40fa07af7c74ac68e4200787bca8f/boto3_stubs-1.43.90.tar.gz", hash = "sha256:ffca416bc23a315d2a0e770401b13d349d9b79cf30d8f75d01768e9f2fda4812", size = 104565, upload-time = "2026-09-08T20:29:14.726Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/97/26/6b55acc18177a0dd7c9f4745ed32a5c8415aa96f2f19e829e36a80739232/boto3_stubs-1.43.90-py3-none-any.whl", hash = "sha256:6350af6f4199d759d04654ce831bf5d4e90bfc7d606ae18c0085b1566b9af418", size = 71510, upload-time = "2026-09-08T20:29:10.782Z" }, +] + +[[package]] +name = "botocore" +version = "1.43.90" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "jmespath" }, + { name = "python-dateutil" }, + { name = "urllib3" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/6b/3e/29872261075d878581a31e9e98c512c0b85322a36e6d6784b9f0694a918d/botocore-1.43.90.tar.gz", hash = "sha256:a139ed601e8b8fb1d730022355fe2b284b8c15cfe9ac0f100254a35d2273e1d3", size = 16081908, upload-time = "2026-09-08T19:22:25.645Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/f6/b8/65cbd0d65553c3e51d27159de4c74308ea008c652b05ed3f830f32667efb/botocore-1.43.90-py3-none-any.whl", hash = "sha256:65f3394ef07314e45c92a90120988531d651136390d464a94938a92f665893f7", size = 15775313, upload-time = "2026-09-08T19:22:22.713Z" }, +] + +[[package]] +name = "botocore-stubs" +version = "1.43.67" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/3f/45/53d662227dc4787b2c854445ee7eb4751cb5d74cfb5c686a6ecbe1f94c17/botocore_stubs-1.43.67.tar.gz", hash = "sha256:853e74014a1f557055c4ffae5fb38d7c65c7c0520e1aab366cac41d5428f419d", size = 42846, upload-time = "2026-08-08T14:57:53.412Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/4e/5e/bdbf19967898a032292da65a47d6e25b2eee55865db4e687f861d80b5602/botocore_stubs-1.43.67-py3-none-any.whl", hash = "sha256:c51262bac3341c1cda71f05fa01141fffd3990d7a92c7960e3b755c1bc830373", size = 67244, upload-time = "2026-08-08T14:57:52.01Z" }, +] + [[package]] name = "certifi" version = "2026.7.22" @@ -800,6 +850,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/62/a1/3d680cbfd5f4b8f15abc1d571870c5fc3e594bb582bc3b64ea099db13e56/jinja2-3.1.6-py3-none-any.whl", hash = "sha256:85ece4451f492d0c13c5dd7c13a64681a86afae63a5f347908daf103ce6d2f67", size = 134899, upload-time = "2025-03-05T20:05:00.369Z" }, ] +[[package]] +name = "jmespath" +version = "1.1.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/d3/59/322338183ecda247fb5d1763a6cbe46eff7222eaeebafd9fa65d4bf5cb11/jmespath-1.1.0.tar.gz", hash = "sha256:472c87d80f36026ae83c6ddd0f1d05d4e510134ed462851fd5f754c8c3cbb88d", size = 27377, upload-time = "2026-01-22T16:35:26.279Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/14/2f/967ba146e6d58cf6a652da73885f52fc68001525b4197effc174321d70b4/jmespath-1.1.0-py3-none-any.whl", hash = "sha256:a5663118de4908c91729bea0acadca56526eb2698e83de10cd116ae0f4e97c64", size = 20419, upload-time = "2026-01-22T16:35:24.919Z" }, +] + [[package]] name = "joserfc" version = "1.7.4" @@ -1269,6 +1328,7 @@ name = "openhound" source = { editable = "." } dependencies = [ { name = "alive-progress" }, + { name = "boto3" }, { name = "cookiecutter" }, { name = "dlt" }, { name = "duckdb" }, @@ -1303,6 +1363,7 @@ okta = [ [package.dev-dependencies] dev = [ { name = "altair" }, + { name = "boto3-stubs" }, { name = "fastapi" }, { name = "httpx" }, { name = "ipython" }, @@ -1320,6 +1381,7 @@ dev = [ [package.metadata] requires-dist = [ { name = "alive-progress", specifier = ">=3.3.0" }, + { name = "boto3", specifier = ">=1.43.90" }, { name = "cookiecutter", specifier = ">=2.6.0" }, { name = "dlt", specifier = "==1.26.0" }, { name = "duckdb", specifier = "==1.5.2" }, @@ -1345,6 +1407,7 @@ provides-extras = ["all", "github", "jamf", "okta"] [package.metadata.requires-dev] dev = [ { name = "altair", specifier = ">=6.1.0" }, + { name = "boto3-stubs", specifier = "~=1.43.90" }, { name = "fastapi", specifier = ">=0.136.1" }, { name = "httpx", specifier = ">=0.28.1" }, { name = "ipython", specifier = ">=9.13.0" }, @@ -2091,6 +2154,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/cb/9a/8415f2657cbe200f41a4531ccededf135505a92d4a012229121f885b26f9/ruff-0.16.0-py3-none-win_arm64.whl", hash = "sha256:14296fedcd2705c77ab8235439278bbb38f285cf7da5528b00b3e330c3d4872d", size = 11273407, upload-time = "2026-07-23T19:11:28.705Z" }, ] +[[package]] +name = "s3transfer" +version = "0.19.2" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "botocore" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/76/43/35e4d8aa320bffe8287fe8f65f578fa2d2db0a64212f0e710dce58267854/s3transfer-0.19.2.tar.gz", hash = "sha256:ba0309fd86be3c27dbf78cdd813c13c5e1df16e5874b99d2535ebbdfb9892993", size = 165592, upload-time = "2026-07-22T19:30:44.432Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/bc/e7/5c595c75e9f41a44f30e526eda465ea0b4eec93470e074e4a111b253f13a/s3transfer-0.19.2-py3-none-any.whl", hash = "sha256:d8168eccca828cbb2cd573675333f3bddd254313a9c42494b84c76b539e8ba25", size = 90216, upload-time = "2026-07-22T19:30:43.251Z" }, +] + [[package]] name = "semver" version = "3.0.4" @@ -2333,6 +2408,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/c3/82/959113a6351f3ca046cd0a8cd2cee071d7ea47473560557a01eeae9a6fe2/types_requests-2.33.0.20260503-py3-none-any.whl", hash = "sha256:02aaa7e3577a13471715bb1bddb693cc985ea514f754b503bf033e6a09a3e528", size = 20736, upload-time = "2026-05-03T05:20:07.858Z" }, ] +[[package]] +name = "types-s3transfer" +version = "0.16.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/fe/64/42689150509eb3e6e82b33ee3d89045de1592488842ddf23c56957786d05/types_s3transfer-0.16.0.tar.gz", hash = "sha256:b4636472024c5e2b62278c5b759661efeb52a81851cde5f092f24100b1ecb443", size = 13557, upload-time = "2025-12-08T08:13:09.928Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/98/27/e88220fe6274eccd3bdf95d9382918716d312f6f6cef6a46332d1ee2feff/types_s3transfer-0.16.0-py3-none-any.whl", hash = "sha256:1c0cd111ecf6e21437cb410f5cddb631bfb2263b77ad973e79b9c6d0cb24e0ef", size = 19247, upload-time = "2025-12-08T08:13:08.426Z" }, +] + [[package]] name = "typing-extensions" version = "4.16.0" From 08c2e2a1bc20890405a7767c8e1eeadae21a82b0 Mon Sep 17 00:00:00 2001 From: Stran Dutton Date: Mon, 14 Sep 2026 11:04:59 -0500 Subject: [PATCH 2/5] logging cleanup --- .../core/clients/aws_secrets_manager.py | 143 +++++++----------- tests/test_aws_secrets_manager.py | 65 ++++++-- 2 files changed, 110 insertions(+), 98 deletions(-) diff --git a/src/openhound/core/clients/aws_secrets_manager.py b/src/openhound/core/clients/aws_secrets_manager.py index ae877e4..2eae3e6 100644 --- a/src/openhound/core/clients/aws_secrets_manager.py +++ b/src/openhound/core/clients/aws_secrets_manager.py @@ -17,14 +17,19 @@ _PROVIDER = "aws.secretsmanager" -class SecretsManagerClient(Protocol): +class GetSecretValueClient(Protocol): def get_secret_value(self, *, SecretId: str) -> Mapping[str, Any]: ... + +class BatchGetSecretValueClient(Protocol): def batch_get_secret_value( self, *, SecretIdList: list[str] ) -> Mapping[str, Any]: ... +SecretsManagerClient = GetSecretValueClient | BatchGetSecretValueClient + + class SecretRetrievalError(Exception): """Base class for errors retrieving a secret.""" @@ -74,17 +79,21 @@ def __init__(self, client: SecretsManagerClient | None = None) -> None: def get_secret(self, secret_id: str) -> SecretValue: """Retrieve one text or JSON-object secret.""" - _log_attempt("get_secret_value") try: client = self._get_client() + value = self._retrieve_secret( + cast(GetSecretValueClient, client), secret_id + ) except SecretRetrievalError: - _log_failure("get_secret_value") _log_result("get_secret_value", "failure") raise - return self._retrieve_secret(client, secret_id) + _log_result("get_secret_value", "success") + return value def get_secrets(self, secret_ids: Iterable[str]) -> dict[str, SecretValue]: """Retrieve any number of secrets in AWS-supported batch sizes.""" + if isinstance(secret_ids, str): + raise TypeError("secret_ids must be an iterable of secret identifiers") requested_ids = list(secret_ids) if not requested_ids: return {} @@ -92,56 +101,52 @@ def get_secrets(self, secret_ids: Iterable[str]) -> dict[str, SecretValue]: try: client = self._get_client() except SecretRetrievalError as error: - _log_attempt("batch_get_secret_value", len(requested_ids)) - _log_failure("batch_get_secret_value", len(requested_ids)) - _log_result("batch_get_secret_value", "failure", len(requested_ids)) failures = {secret_id: error for secret_id in requested_ids} + _log_result("batch_get_secret_value", "failure", len(requested_ids)) raise SecretBatchError(failures) from None - if not callable(getattr(client, "batch_get_secret_value", None)): - _log_attempt("batch_get_secret_value", len(requested_ids)) - _log_result("batch_get_secret_value", "fallback", len(requested_ids)) - return self._get_secrets_individually(client, requested_ids) + try: + if not callable(getattr(client, "batch_get_secret_value", None)): + values = self._get_secrets_individually( + cast(GetSecretValueClient, client), requested_ids + ) + else: + values = self._get_secrets_in_batches(client, requested_ids) + except SecretBatchError: + _log_result("batch_get_secret_value", "failure", len(requested_ids)) + raise + _log_result("batch_get_secret_value", "success", len(requested_ids)) + return values + def _get_secrets_in_batches( + self, client: SecretsManagerClient, secret_ids: list[str] + ) -> dict[str, SecretValue]: values: dict[str, SecretValue] = {} - for chunk_index, chunk in enumerate(_chunks(requested_ids, MAX_BATCH_SIZE)): - secret_count = len(chunk) - _log_attempt("batch_get_secret_value", secret_count) - + for chunk_index, chunk in enumerate(_chunks(secret_ids, MAX_BATCH_SIZE)): try: - response = client.batch_get_secret_value(SecretIdList=chunk) - except ClientError as error: - if _is_batch_fallback_error(error): - _log_result("batch_get_secret_value", "fallback", secret_count) + response = cast( + BatchGetSecretValueClient, client + ).batch_get_secret_value(SecretIdList=chunk) + except (ClientError, BotoCoreError) as error: + if isinstance(error, ClientError) and _is_batch_fallback_error(error): values.update( self._get_secrets_individually( - client, - requested_ids[chunk_index * MAX_BATCH_SIZE :], + cast(GetSecretValueClient, client), + secret_ids[chunk_index * MAX_BATCH_SIZE :], ) ) - return values - classified = _classify_client_error(error) - failures = {secret_id: classified for secret_id in chunk} - _log_failure("batch_get_secret_value", secret_count) - _log_result("batch_get_secret_value", "failure", secret_count) - raise SecretBatchError(failures) from None - except BotoCoreError as error: - classified = _classify_boto_core_error(error) - failures = {secret_id: classified for secret_id in chunk} - _log_failure("batch_get_secret_value", secret_count) - _log_result("batch_get_secret_value", "failure", secret_count) - raise SecretBatchError(failures) from None + break + classified = _classify_sdk_error(error) + raise SecretBatchError( + {secret_id: classified for secret_id in chunk} + ) from None failures = _batch_response_failures(response, chunk) chunk_values, value_failures = _parse_batch_values(response, chunk) for secret_id, value_error in value_failures.items(): failures.setdefault(secret_id, value_error) if failures: - _log_failure("batch_get_secret_value", secret_count) - _log_result("batch_get_secret_value", "failure", secret_count) raise SecretBatchError(failures) from None - values.update({secret_id: chunk_values[secret_id] for secret_id in chunk}) - _log_result("batch_get_secret_value", "success", secret_count) return values @@ -154,9 +159,8 @@ def _get_client(self) -> SecretsManagerClient: raise _classify_boto_core_error(error) from None def _retrieve_secret( - self, client: SecretsManagerClient, secret_id: str + self, client: GetSecretValueClient, secret_id: str ) -> SecretValue: - _log_attempt("get_secret_value") try: if not callable(getattr(client, "get_secret_value", None)): raise AWSConfigurationError( @@ -164,26 +168,12 @@ def _retrieve_secret( ) response = client.get_secret_value(SecretId=secret_id) value = _parse_secret_value(response) - except SecretRetrievalError: - _log_failure("get_secret_value") - _log_result("get_secret_value", "failure") - raise - except ClientError as error: - classified = _classify_client_error(error) - _log_failure("get_secret_value") - _log_result("get_secret_value", "failure") - raise classified from None - except BotoCoreError as error: - classified = _classify_boto_core_error(error) - _log_failure("get_secret_value") - _log_result("get_secret_value", "failure") - raise classified from None - - _log_result("get_secret_value", "success") + except (ClientError, BotoCoreError) as error: + raise _classify_sdk_error(error) from None return value def _get_secrets_individually( - self, client: SecretsManagerClient, secret_ids: list[str] + self, client: GetSecretValueClient, secret_ids: list[str] ) -> dict[str, SecretValue]: values: dict[str, SecretValue] = {} failures: dict[str, SecretRetrievalError] = {} @@ -329,6 +319,14 @@ def _classify_boto_core_error(error: BotoCoreError) -> SecretRetrievalError: return SecretRequestError("AWS Secrets Manager request could not be completed") +def _classify_sdk_error( + error: ClientError | BotoCoreError, +) -> SecretRetrievalError: + if isinstance(error, ClientError): + return _classify_client_error(error) + return _classify_boto_core_error(error) + + def _is_batch_fallback_error(error: ClientError) -> bool: return _client_error_code(error) in { "AccessDenied", @@ -357,20 +355,6 @@ def _client_error_code(error: ClientError) -> Any: return error.response.get("Error", {}).get("Code") -def _log_attempt(operation: str, secret_count: int | None = None) -> None: - extra: dict[str, str | int] = { - "provider": _PROVIDER, - "operation": operation, - "outcome": "attempt", - } - if secret_count is not None: - extra["secret_count"] = secret_count - logger.debug( - "Secret retrieval attempt", - extra=extra, - ) - - def _log_result(operation: str, outcome: str, secret_count: int | None = None) -> None: extra: dict[str, str | int] = { "provider": _PROVIDER, @@ -379,18 +363,7 @@ def _log_result(operation: str, outcome: str, secret_count: int | None = None) - } if secret_count is not None: extra["secret_count"] = secret_count - logger.debug( - "Secret retrieval result", - extra=extra, - ) - - -def _log_failure(operation: str, secret_count: int | None = None) -> None: - extra: dict[str, str | int] = { - "provider": _PROVIDER, - "operation": operation, - "outcome": "failure", - } - if secret_count is not None: - extra["secret_count"] = secret_count - logger.error("Failure to retrieve secret", extra=extra) + if outcome == "failure": + logger.error("Secret retrieval failed", extra=extra) + else: + logger.debug("Secret retrieval completed", extra=extra) diff --git a/tests/test_aws_secrets_manager.py b/tests/test_aws_secrets_manager.py index 9ae2694..b490bb1 100644 --- a/tests/test_aws_secrets_manager.py +++ b/tests/test_aws_secrets_manager.py @@ -69,6 +69,18 @@ def test_get_secret_returns_plain_text_and_passes_secret_id() -> None: assert client.requested_secret_id == "secret-id" +def test_get_secret_logs_one_final_outcome(caplog: pytest.LogCaptureFixture) -> None: + client = FakeSecretsManagerClient({"SecretString": "secret-value"}) + + with caplog.at_level(logging.DEBUG): + AWSSecretsManager(client).get_secret("secret-id") + + assert len(caplog.records) == 1 + assert caplog.records[0].operation == "get_secret_value" + assert caplog.records[0].outcome == "success" + assert caplog.records[0].getMessage() == "Secret retrieval completed" + + def test_get_secret_returns_json_object() -> None: client = FakeSecretsManagerClient({"SecretString": '{"username":"user"}'}) @@ -100,14 +112,11 @@ def test_get_secret_maps_aws_errors_without_leaking_values( assert ( sum( record.levelno == logging.ERROR - and record.getMessage() == "Failure to retrieve secret" + and record.getMessage() == "Secret retrieval failed" for record in caplog.records ) == 1 ) - messages = [record.getMessage() for record in caplog.records] - assert "Secret retrieval attempt" in messages - assert "Secret retrieval result" in messages def test_get_secret_rejects_malformed_json_without_leaking_value() -> None: @@ -152,7 +161,9 @@ def test_get_secrets_returns_values_mapped_to_requested_ids() -> None: assert client.batch_requests == [["first", "second"]] -def test_get_secrets_falls_back_when_batch_api_is_unavailable() -> None: +def test_get_secrets_falls_back_when_batch_api_is_unavailable( + caplog: pytest.LogCaptureFixture, +) -> None: class LegacyClient: def __init__(self) -> None: self.requested_ids: list[str] = [] @@ -163,10 +174,22 @@ def get_secret_value(self, *, SecretId: str) -> dict[str, str]: client = LegacyClient() - result = AWSSecretsManager(client).get_secrets(["first", "second"]) + with caplog.at_level(logging.DEBUG): + result = AWSSecretsManager(client).get_secrets(["first", "second"]) assert result == {"first": "value-first", "second": "value-second"} assert client.requested_ids == ["first", "second"] + assert len(caplog.records) == 1 + assert caplog.records[0].operation == "batch_get_secret_value" + assert caplog.records[0].outcome == "success" + assert caplog.records[0].secret_count == 2 + + +def test_get_secrets_rejects_string_secret_ids() -> None: + client = FakeSecretsManagerClient(batch_response={"SecretValues": []}) + + with pytest.raises(TypeError, match="iterable of secret identifiers"): + AWSSecretsManager(client).get_secrets("secret-id") def test_get_secrets_falls_back_when_batch_permission_is_missing() -> None: @@ -238,7 +261,7 @@ def test_get_secrets_raises_typed_error_for_partial_batch_failure( assert ( sum( record.levelno == logging.ERROR - and record.getMessage() == "Failure to retrieve secret" + and record.getMessage() == "Secret retrieval failed" for record in caplog.records ) == 1 @@ -274,7 +297,7 @@ def test_get_secrets_classifies_sdk_failures(error, expected) -> None: assert isinstance(raised.value.failures["secret-id"], expected) -def test_retrieval_logs_attempt_and_result_without_secret_values( +def test_retrieval_logs_one_outcome_without_secret_values( caplog: pytest.LogCaptureFixture, ) -> None: text_value = "plain-secret-value" @@ -291,9 +314,8 @@ def test_retrieval_logs_attempt_and_result_without_secret_values( with caplog.at_level(logging.DEBUG): AWSSecretsManager(client).get_secrets(["text-secret", "json-secret"]) - messages = [record.getMessage() for record in caplog.records] - assert "Secret retrieval attempt" in messages - assert "Secret retrieval result" in messages + assert len(caplog.records) == 1 + assert caplog.records[0].getMessage() == "Secret retrieval completed" forbidden_values = { "text-secret", "json-secret", @@ -311,8 +333,6 @@ def test_retrieval_logs_attempt_and_result_without_secret_values( assert text_value not in caplog.text assert json_value not in caplog.text - assert "Secret retrieval attempt" in messages - assert "Secret retrieval result" in messages def test_constructor_rejects_invalid_client() -> None: @@ -327,3 +347,22 @@ def test_constructor_does_not_create_an_aws_client(monkeypatch) -> None: ) AWSSecretsManager() + + +def test_get_secret_logs_failure_when_client_creation_fails( + monkeypatch, caplog: pytest.LogCaptureFixture +) -> None: + def fail_client_creation(*_args): + raise NoRegionError() + + monkeypatch.setattr( + "openhound.core.clients.aws_secrets_manager.boto3.client", + fail_client_creation, + ) + + with caplog.at_level(logging.DEBUG), pytest.raises(AWSConfigurationError): + AWSSecretsManager().get_secret("secret-id") + + assert len(caplog.records) == 1 + assert caplog.records[0].operation == "get_secret_value" + assert caplog.records[0].outcome == "failure" From 1094907d325fe98c6c64e29e3daad886dd4226e4 Mon Sep 17 00:00:00 2001 From: Stran Dutton Date: Mon, 14 Sep 2026 15:19:54 -0500 Subject: [PATCH 3/5] consolidate error handling helpers, simplify unit tests --- .../core/clients/aws_secrets_manager.py | 31 +-- tests/test_aws_secrets_manager.py | 187 ++++++++++-------- 2 files changed, 118 insertions(+), 100 deletions(-) diff --git a/src/openhound/core/clients/aws_secrets_manager.py b/src/openhound/core/clients/aws_secrets_manager.py index 2eae3e6..7c0dde9 100644 --- a/src/openhound/core/clients/aws_secrets_manager.py +++ b/src/openhound/core/clients/aws_secrets_manager.py @@ -81,9 +81,7 @@ def get_secret(self, secret_id: str) -> SecretValue: """Retrieve one text or JSON-object secret.""" try: client = self._get_client() - value = self._retrieve_secret( - cast(GetSecretValueClient, client), secret_id - ) + value = self._retrieve_secret(cast(GetSecretValueClient, client), secret_id) except SecretRetrievalError: _log_result("get_secret_value", "failure") raise @@ -156,7 +154,7 @@ def _get_client(self) -> SecretsManagerClient: try: return cast(SecretsManagerClient, boto3.client("secretsmanager")) except BotoCoreError as error: - raise _classify_boto_core_error(error) from None + raise _classify_sdk_error(error) from None def _retrieve_secret( self, client: GetSecretValueClient, secret_id: str @@ -307,11 +305,10 @@ def _matching_requested_id( return None -def _classify_client_error(error: ClientError) -> SecretRetrievalError: - return _classify_error_code(_client_error_code(error)) - - -def _classify_boto_core_error(error: BotoCoreError) -> SecretRetrievalError: +def _classify_sdk_error(error: ClientError | BotoCoreError) -> SecretRetrievalError: + """Map a raised boto or SDK error to the public retrieval exception.""" + if isinstance(error, ClientError): + return _classify_error_code(error.response.get("Error", {}).get("Code")) if isinstance(error, (NoCredentialsError, NoRegionError)): return AWSConfigurationError( "AWS Secrets Manager client configuration is invalid" @@ -319,16 +316,9 @@ def _classify_boto_core_error(error: BotoCoreError) -> SecretRetrievalError: return SecretRequestError("AWS Secrets Manager request could not be completed") -def _classify_sdk_error( - error: ClientError | BotoCoreError, -) -> SecretRetrievalError: - if isinstance(error, ClientError): - return _classify_client_error(error) - return _classify_boto_core_error(error) - - def _is_batch_fallback_error(error: ClientError) -> bool: - return _client_error_code(error) in { + """Return whether a batch-only failure should fall back to individual reads.""" + return error.response.get("Error", {}).get("Code") in { "AccessDenied", "AccessDeniedException", "UnauthorizedOperation", @@ -338,6 +328,7 @@ def _is_batch_fallback_error(error: ClientError) -> bool: def _classify_error_code(error_code: Any) -> SecretRetrievalError: + """Map an AWS error code to its public retrieval exception.""" if error_code == "ResourceNotFoundException": return SecretNotFoundError("AWS secret was not found") if error_code in { @@ -351,10 +342,6 @@ def _classify_error_code(error_code: Any) -> SecretRetrievalError: return SecretRequestError("AWS Secrets Manager request could not be completed") -def _client_error_code(error: ClientError) -> Any: - return error.response.get("Error", {}).get("Code") - - def _log_result(operation: str, outcome: str, secret_count: int | None = None) -> None: extra: dict[str, str | int] = { "provider": _PROVIDER, diff --git a/tests/test_aws_secrets_manager.py b/tests/test_aws_secrets_manager.py index b490bb1..149e11f 100644 --- a/tests/test_aws_secrets_manager.py +++ b/tests/test_aws_secrets_manager.py @@ -1,6 +1,7 @@ import logging -from typing import Any +from typing import Any, cast +import boto3 import pytest from botocore.exceptions import ( ClientError, @@ -8,6 +9,7 @@ NoCredentialsError, NoRegionError, ) +from botocore.stub import Stubber from openhound.core.clients.aws_secrets_manager import ( AWSConfigurationError, @@ -32,7 +34,7 @@ def __init__( self.error = error self.batch_response = batch_response self.batch_error = batch_error - self.requested_secret_id = None + self.requested_secret_id: str | None = None self.batch_requests: list[list[str]] = [] def get_secret_value(self, *, SecretId: str) -> dict[str, Any]: @@ -54,7 +56,13 @@ def aws_error(code: str, message: str) -> ClientError: return ClientError( { "Error": {"Code": code, "Message": message}, - "ResponseMetadata": {"HTTPStatusCode": 400}, + "ResponseMetadata": { + "HTTPHeaders": {}, + "HTTPStatusCode": 400, + "HostId": "", + "RequestId": "", + "RetryAttempts": 0, + }, }, "GetSecretValue", ) @@ -69,18 +77,6 @@ def test_get_secret_returns_plain_text_and_passes_secret_id() -> None: assert client.requested_secret_id == "secret-id" -def test_get_secret_logs_one_final_outcome(caplog: pytest.LogCaptureFixture) -> None: - client = FakeSecretsManagerClient({"SecretString": "secret-value"}) - - with caplog.at_level(logging.DEBUG): - AWSSecretsManager(client).get_secret("secret-id") - - assert len(caplog.records) == 1 - assert caplog.records[0].operation == "get_secret_value" - assert caplog.records[0].outcome == "success" - assert caplog.records[0].getMessage() == "Secret retrieval completed" - - def test_get_secret_returns_json_object() -> None: client = FakeSecretsManagerClient({"SecretString": '{"username":"user"}'}) @@ -89,6 +85,74 @@ def test_get_secret_returns_json_object() -> None: assert result == {"username": "user"} +def test_get_secret_works_with_a_real_boto3_client_offline() -> None: + client = boto3.client( + "secretsmanager", + region_name="us-east-1", + aws_access_key_id="testing", + aws_secret_access_key="testing", + ) + response = { + "ARN": "arn:aws:secretsmanager:us-east-1:123456789012:secret:local-secret", + "Name": "local-secret", + "SecretString": '{"username":"local-user"}', + "VersionId": "00000000000000000000000000000001", + "CreatedDate": 0, + } + + with Stubber(client) as stubber: + stubber.add_response( + "get_secret_value", + response, + {"SecretId": "local-secret"}, + ) + + result = AWSSecretsManager(client).get_secret("local-secret") + + assert result == {"username": "local-user"} + stubber.assert_no_pending_responses() + + +def test_get_secrets_works_with_a_real_boto3_client_offline() -> None: + client = boto3.client( + "secretsmanager", + region_name="us-east-1", + aws_access_key_id="testing", + aws_secret_access_key="testing", + ) + response = { + "SecretValues": [ + { + "ARN": "arn:aws:secretsmanager:us-east-1:123456789012:secret:first", + "Name": "first", + "SecretString": "value-1", + "VersionId": "00000000000000000000000000000001", + "CreatedDate": 0, + }, + { + "ARN": "arn:aws:secretsmanager:us-east-1:123456789012:secret:second", + "Name": "second", + "SecretString": "value-2", + "VersionId": "00000000000000000000000000000002", + "CreatedDate": 0, + }, + ], + "Errors": [], + } + + with Stubber(client) as stubber: + stubber.add_response( + "batch_get_secret_value", + response, + {"SecretIdList": ["first", "second"]}, + ) + + result = AWSSecretsManager(client).get_secrets(["first", "second"]) + + assert result == {"first": "value-1", "second": "value-2"} + stubber.assert_no_pending_responses() + + @pytest.mark.parametrize( ("error_code", "exception_type"), [ @@ -97,26 +161,16 @@ def test_get_secret_returns_json_object() -> None: ], ) def test_get_secret_maps_aws_errors_without_leaking_values( - error_code: str, exception_type: type[Exception], caplog: pytest.LogCaptureFixture + error_code: str, exception_type: type[Exception] ) -> None: secret_value = "do-not-leak-this-value" client = FakeSecretsManagerClient(error=aws_error(error_code, secret_value)) - with caplog.at_level(logging.DEBUG), pytest.raises(exception_type) as raised: + with pytest.raises(exception_type) as raised: AWSSecretsManager(client).get_secret("secret-id") assert secret_value not in str(raised.value) - assert secret_value not in caplog.text - assert "secret-id" not in caplog.text - assert "2d4e223a1833" not in caplog.text - assert ( - sum( - record.levelno == logging.ERROR - and record.getMessage() == "Secret retrieval failed" - for record in caplog.records - ) - == 1 - ) + assert "secret-id" not in str(raised.value) def test_get_secret_rejects_malformed_json_without_leaking_value() -> None: @@ -161,9 +215,7 @@ def test_get_secrets_returns_values_mapped_to_requested_ids() -> None: assert client.batch_requests == [["first", "second"]] -def test_get_secrets_falls_back_when_batch_api_is_unavailable( - caplog: pytest.LogCaptureFixture, -) -> None: +def test_get_secrets_falls_back_when_batch_api_is_unavailable() -> None: class LegacyClient: def __init__(self) -> None: self.requested_ids: list[str] = [] @@ -174,15 +226,10 @@ def get_secret_value(self, *, SecretId: str) -> dict[str, str]: client = LegacyClient() - with caplog.at_level(logging.DEBUG): - result = AWSSecretsManager(client).get_secrets(["first", "second"]) + result = AWSSecretsManager(client).get_secrets(["first", "second"]) assert result == {"first": "value-first", "second": "value-second"} assert client.requested_ids == ["first", "second"] - assert len(caplog.records) == 1 - assert caplog.records[0].operation == "batch_get_secret_value" - assert caplog.records[0].outcome == "success" - assert caplog.records[0].secret_count == 2 def test_get_secrets_rejects_string_secret_ids() -> None: @@ -254,26 +301,20 @@ def test_get_secrets_raises_typed_error_for_partial_batch_failure( AWSSecretsManager(client).get_secrets(["available", "missing"]) assert isinstance(raised.value.failures["missing"], SecretNotFoundError) - assert secret_value not in str(raised.value) - assert secret_value not in caplog.text - assert "missing" not in caplog.text - assert "ffa63583dfa6" not in caplog.text - assert ( - sum( - record.levelno == logging.ERROR - and record.getMessage() == "Secret retrieval failed" - for record in caplog.records - ) - == 1 + forbidden_values = {"available", "missing", secret_value} + assert all(value not in str(raised.value) for value in forbidden_values) + assert all(value not in caplog.text for value in forbidden_values) + assert all( + all(value not in repr(record.__dict__) for value in forbidden_values) + for record in caplog.records ) - for record in caplog.records: - record_text = repr(record.__dict__) - assert "available" not in record_text - assert "missing" not in record_text - assert "ffa63583dfa6" not in record_text - assert record.provider == "aws.secretsmanager" - assert record.operation == "batch_get_secret_value" - assert record.secret_count == 2 + assert len(caplog.records) == 1 + record = cast(Any, caplog.records[0]) + assert record.levelno == logging.ERROR + assert record.provider == "aws.secretsmanager" + assert record.operation == "batch_get_secret_value" + assert record.outcome == "failure" + assert record.secret_count == 2 @pytest.mark.parametrize( @@ -297,7 +338,7 @@ def test_get_secrets_classifies_sdk_failures(error, expected) -> None: assert isinstance(raised.value.failures["secret-id"], expected) -def test_retrieval_logs_one_outcome_without_secret_values( +def test_get_secrets_logs_structured_success_without_secret_values( caplog: pytest.LogCaptureFixture, ) -> None: text_value = "plain-secret-value" @@ -315,29 +356,25 @@ def test_retrieval_logs_one_outcome_without_secret_values( AWSSecretsManager(client).get_secrets(["text-secret", "json-secret"]) assert len(caplog.records) == 1 - assert caplog.records[0].getMessage() == "Secret retrieval completed" + record = cast(Any, caplog.records[0]) + assert record.levelno == logging.DEBUG + assert record.provider == "aws.secretsmanager" + assert record.operation == "batch_get_secret_value" + assert record.outcome == "success" + assert record.secret_count == 2 forbidden_values = { "text-secret", "json-secret", text_value, json_value, - "a5353f2b7585", - "9bda9177a37d", } - for record in caplog.records: - record_text = repr(record.__dict__) - assert all(value not in record_text for value in forbidden_values) - assert record.provider == "aws.secretsmanager" - assert record.operation == "batch_get_secret_value" - assert record.secret_count == 2 - - assert text_value not in caplog.text - assert json_value not in caplog.text + assert all(value not in repr(record.__dict__) for value in forbidden_values) + assert all(value not in caplog.text for value in forbidden_values) def test_constructor_rejects_invalid_client() -> None: with pytest.raises(AWSConfigurationError, match="does not support"): - AWSSecretsManager(object()) + AWSSecretsManager(cast(Any, object())) def test_constructor_does_not_create_an_aws_client(monkeypatch) -> None: @@ -349,9 +386,7 @@ def test_constructor_does_not_create_an_aws_client(monkeypatch) -> None: AWSSecretsManager() -def test_get_secret_logs_failure_when_client_creation_fails( - monkeypatch, caplog: pytest.LogCaptureFixture -) -> None: +def test_get_secret_classifies_lazy_client_creation_failure(monkeypatch) -> None: def fail_client_creation(*_args): raise NoRegionError() @@ -360,9 +395,5 @@ def fail_client_creation(*_args): fail_client_creation, ) - with caplog.at_level(logging.DEBUG), pytest.raises(AWSConfigurationError): + with pytest.raises(AWSConfigurationError): AWSSecretsManager().get_secret("secret-id") - - assert len(caplog.records) == 1 - assert caplog.records[0].operation == "get_secret_value" - assert caplog.records[0].outcome == "failure" From 0c1ada842e4d00700796629b421d5f813c7d638a Mon Sep 17 00:00:00 2001 From: Stran Dutton Date: Mon, 14 Sep 2026 18:27:21 -0500 Subject: [PATCH 4/5] refactor --- .github/workflows/test.yml | 4 + pyproject.toml | 1 - .../core/clients/aws_secrets_manager.py | 329 +++++------ tests/test_aws_secrets_manager.py | 540 ++++++++---------- uv.lock | 33 -- 5 files changed, 372 insertions(+), 535 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 43575ac..bdc53f0 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -42,6 +42,10 @@ jobs: run: | uv pip install --python .venv -e ".[all]" --group dev + - name: Run AWS Secrets Manager tests + run: | + .venv/bin/pytest tests/test_aws_secrets_manager.py -v + - name: Run log handling tests run: | .venv/bin/pytest tests/test_log_handlers.py -v diff --git a/pyproject.toml b/pyproject.toml index 5206f59..7d168e4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -71,7 +71,6 @@ dev = [ "ipython>=9.13.0", "pre-commit>=4.5.1", "pytest>=9.0.1", - "boto3-stubs~=1.43.90", "marimo>=0.23.5", "altair>=6.1.0", "fastapi>=0.136.1", diff --git a/src/openhound/core/clients/aws_secrets_manager.py b/src/openhound/core/clients/aws_secrets_manager.py index 7c0dde9..63adc72 100644 --- a/src/openhound/core/clients/aws_secrets_manager.py +++ b/src/openhound/core/clients/aws_secrets_manager.py @@ -1,14 +1,19 @@ import json import logging from collections.abc import Iterable, Mapping -from typing import Any, Protocol, cast +from typing import Any, Protocol import boto3 from botocore.exceptions import ( BotoCoreError, ClientError, + ConfigNotFound, + CredentialRetrievalError, + InvalidConfigError, NoCredentialsError, NoRegionError, + PartialCredentialsError, + ProfileNotFound, ) logger = logging.getLogger(__name__) @@ -17,19 +22,14 @@ _PROVIDER = "aws.secretsmanager" -class GetSecretValueClient(Protocol): +class SecretsManagerClient(Protocol): def get_secret_value(self, *, SecretId: str) -> Mapping[str, Any]: ... - -class BatchGetSecretValueClient(Protocol): def batch_get_secret_value( self, *, SecretIdList: list[str] ) -> Mapping[str, Any]: ... -SecretsManagerClient = GetSecretValueClient | BatchGetSecretValueClient - - class SecretRetrievalError(Exception): """Base class for errors retrieving a secret.""" @@ -58,35 +58,21 @@ def __init__(self, failures: Mapping[str, SecretRetrievalError]) -> None: super().__init__("One or more AWS secret retrievals failed") -class InvalidSecretValueError(SecretRetrievalError): - """The retrieved secret uses an unsupported or invalid format.""" - - -SecretValue = str | Mapping[str, Any] +SecretValue = str | dict[str, Any] class AWSSecretsManager: def __init__(self, client: SecretsManagerClient | None = None) -> None: - if client is not None and not ( - callable(getattr(client, "get_secret_value", None)) - or callable(getattr(client, "batch_get_secret_value", None)) - ): - raise AWSConfigurationError( - "AWS Secrets Manager client does not support secret retrieval" - ) - self._client = client def get_secret(self, secret_id: str) -> SecretValue: """Retrieve one text or JSON-object secret.""" try: client = self._get_client() - value = self._retrieve_secret(cast(GetSecretValueClient, client), secret_id) - except SecretRetrievalError: - _log_result("get_secret_value", "failure") + except SecretRetrievalError as error: + _log_failure("get_secret_value", error, 1, 1) raise - _log_result("get_secret_value", "success") - return value + return self._retrieve_secret(client, secret_id) def get_secrets(self, secret_ids: Iterable[str]) -> dict[str, SecretValue]: """Retrieve any number of secrets in AWS-supported batch sizes.""" @@ -100,51 +86,50 @@ def get_secrets(self, secret_ids: Iterable[str]) -> dict[str, SecretValue]: client = self._get_client() except SecretRetrievalError as error: failures = {secret_id: error for secret_id in requested_ids} - _log_result("batch_get_secret_value", "failure", len(requested_ids)) + _log_failure( + "batch_get_secret_value", + error, + len(requested_ids), + len(failures), + ) raise SecretBatchError(failures) from None - try: - if not callable(getattr(client, "batch_get_secret_value", None)): - values = self._get_secrets_individually( - cast(GetSecretValueClient, client), requested_ids - ) - else: - values = self._get_secrets_in_batches(client, requested_ids) - except SecretBatchError: - _log_result("batch_get_secret_value", "failure", len(requested_ids)) - raise - _log_result("batch_get_secret_value", "success", len(requested_ids)) - return values + return self._get_secrets_in_batches(client, requested_ids) def _get_secrets_in_batches( self, client: SecretsManagerClient, secret_ids: list[str] ) -> dict[str, SecretValue]: values: dict[str, SecretValue] = {} + failures: dict[str, SecretRetrievalError] = {} for chunk_index, chunk in enumerate(_chunks(secret_ids, MAX_BATCH_SIZE)): + _log_debug("batch_get_secret_value", "attempt", len(chunk), chunk_index + 1) try: - response = cast( - BatchGetSecretValueClient, client - ).batch_get_secret_value(SecretIdList=chunk) + response = client.batch_get_secret_value(SecretIdList=chunk) except (ClientError, BotoCoreError) as error: - if isinstance(error, ClientError) and _is_batch_fallback_error(error): - values.update( - self._get_secrets_individually( - cast(GetSecretValueClient, client), - secret_ids[chunk_index * MAX_BATCH_SIZE :], - ) - ) - break - classified = _classify_sdk_error(error) - raise SecretBatchError( - {secret_id: classified for secret_id in chunk} - ) from None - - failures = _batch_response_failures(response, chunk) - chunk_values, value_failures = _parse_batch_values(response, chunk) - for secret_id, value_error in value_failures.items(): - failures.setdefault(secret_id, value_error) - if failures: - raise SecretBatchError(failures) from None - values.update({secret_id: chunk_values[secret_id] for secret_id in chunk}) + classified = _classify_aws_error(error) + chunk_values: dict[str, SecretValue] = {} + chunk_failures = { + secret_id: classified for secret_id in chunk + } + else: + chunk_values, chunk_failures = _parse_batch_response(response, chunk) + + values.update(chunk_values) + failures.update(chunk_failures) + if chunk_failures: + _log_failure( + "batch_get_secret_value", + SecretBatchError(chunk_failures), + len(chunk), + len(chunk_failures), + chunk_index + 1, + ) + else: + _log_debug( + "batch_get_secret_value", "success", len(chunk), chunk_index + 1 + ) + + if failures: + raise SecretBatchError(failures) from None return values @@ -152,37 +137,27 @@ def _get_client(self) -> SecretsManagerClient: if self._client is not None: return self._client try: - return cast(SecretsManagerClient, boto3.client("secretsmanager")) + return boto3.client("secretsmanager") except BotoCoreError as error: - raise _classify_sdk_error(error) from None + raise _classify_aws_error(error) from None def _retrieve_secret( - self, client: GetSecretValueClient, secret_id: str + self, client: SecretsManagerClient, secret_id: str ) -> SecretValue: + _log_debug("get_secret_value", "attempt", 1) try: - if not callable(getattr(client, "get_secret_value", None)): - raise AWSConfigurationError( - "AWS Secrets Manager client does not support secret retrieval" - ) response = client.get_secret_value(SecretId=secret_id) value = _parse_secret_value(response) except (ClientError, BotoCoreError) as error: - raise _classify_sdk_error(error) from None - return value + classified = _classify_aws_error(error) + _log_failure("get_secret_value", classified, 1, 1) + raise classified from None + except SecretRetrievalError as error: + _log_failure("get_secret_value", error, 1, 1) + raise - def _get_secrets_individually( - self, client: GetSecretValueClient, secret_ids: list[str] - ) -> dict[str, SecretValue]: - values: dict[str, SecretValue] = {} - failures: dict[str, SecretRetrievalError] = {} - for secret_id in secret_ids: - try: - values[secret_id] = self._retrieve_secret(client, secret_id) - except SecretRetrievalError as error: - failures[secret_id] = error - if failures: - raise SecretBatchError(failures) from None - return values + _log_debug("get_secret_value", "success", 1) + return value def _chunks(values: list[str], size: int) -> Iterable[list[str]]: @@ -191,70 +166,49 @@ def _chunks(values: list[str], size: int) -> Iterable[list[str]]: def _parse_secret_value(response: Mapping[str, Any]) -> SecretValue: - if not isinstance(response, Mapping): - raise InvalidSecretValueError("AWS secret response has an invalid format") - if response.get("SecretBinary") is not None: - raise InvalidSecretValueError( - "SecretBinary payloads are not supported; use SecretString" - ) - secret_string = response.get("SecretString") if not isinstance(secret_string, str): - raise InvalidSecretValueError( - "AWS secret response did not contain a valid SecretString payload" + raise SecretRequestError( + "AWS Secrets Manager response did not contain a SecretString payload" ) - candidate = secret_string.strip() - if candidate.startswith(("{", "[")): + if secret_string.lstrip().startswith("{"): try: parsed = json.loads(secret_string) except json.JSONDecodeError: - raise InvalidSecretValueError( - "AWS secret contains malformed JSON" - ) from None - if not isinstance(parsed, dict): - raise InvalidSecretValueError( - "Only JSON-object secret payloads are supported" - ) - return parsed + return secret_string + if isinstance(parsed, dict): + return parsed return secret_string -def _parse_batch_values( +def _parse_batch_response( response: Mapping[str, Any], requested_ids: list[str], ) -> tuple[dict[str, SecretValue], dict[str, SecretRetrievalError]]: - if not isinstance(response, Mapping): - error = InvalidSecretValueError("AWS batch response has an invalid format") - return {}, {secret_id: error for secret_id in requested_ids} - - entries = response.get("SecretValues", []) - if not isinstance(entries, list): - response_error = InvalidSecretValueError( - "AWS batch response has an invalid format" - ) - return {}, {secret_id: response_error for secret_id in requested_ids} - values: dict[str, SecretValue] = {} failures: dict[str, SecretRetrievalError] = {} - requested_set = set(requested_ids) - for entry in entries: - if not isinstance(entry, Mapping): - response_error = InvalidSecretValueError( - "AWS batch response has an invalid format" - ) - for secret_id in requested_ids: - failures.setdefault(secret_id, response_error) - break - - matched_secret_id = _matching_requested_id(entry, requested_set) - if matched_secret_id is None: + for entry in response.get("SecretValues", []): + secret_id = next( + ( + entry.get(field) + for field in ("Name", "ARN") + if entry.get(field) in requested_ids + ), + None, + ) + if not isinstance(secret_id, str): continue try: - values[matched_secret_id] = _parse_secret_value(entry) + values[secret_id] = _parse_secret_value(entry) except SecretRetrievalError as value_error: - failures[matched_secret_id] = value_error + failures[secret_id] = value_error + + for entry in response.get("Errors", []): + secret_id = entry.get("SecretId") + if isinstance(secret_id, str) and secret_id in requested_ids: + failures[secret_id] = _classify_aws_error(entry.get("ErrorCode")) for secret_id in requested_ids: if secret_id not in values and secret_id not in failures: @@ -265,73 +219,30 @@ def _parse_batch_values( return values, failures -def _batch_response_failures( - response: Mapping[str, Any], - requested_ids: list[str], -) -> dict[str, SecretRetrievalError]: - if not isinstance(response, Mapping): - return {} - entries = response.get("Errors", []) - if not isinstance(entries, list): - return { - secret_id: InvalidSecretValueError( - "AWS batch response has an invalid format" - ) - for secret_id in requested_ids - } - - requested_set = set(requested_ids) - failures: dict[str, SecretRetrievalError] = {} - for index, entry in enumerate(entries): - if not isinstance(entry, Mapping): - failures[f""] = InvalidSecretValueError( - "AWS batch response has an invalid format" - ) - continue - secret_id = entry.get("SecretId") - if not isinstance(secret_id, str) or secret_id not in requested_set: - secret_id = f"" - failures[secret_id] = _classify_error_code(entry.get("ErrorCode")) - return failures - - -def _matching_requested_id( - entry: Mapping[str, Any], requested_ids: set[str] -) -> str | None: - for field in ("Name", "ARN"): - value = entry.get(field) - if isinstance(value, str) and value in requested_ids: - return value - return None - - -def _classify_sdk_error(error: ClientError | BotoCoreError) -> SecretRetrievalError: - """Map a raised boto or SDK error to the public retrieval exception.""" +def _classify_aws_error( + error: ClientError | BotoCoreError | str | None, +) -> SecretRetrievalError: + """Map a boto error or AWS error code to a public retrieval exception.""" if isinstance(error, ClientError): - return _classify_error_code(error.response.get("Error", {}).get("Code")) - if isinstance(error, (NoCredentialsError, NoRegionError)): + error = error.response.get("Error", {}).get("Code") + elif isinstance( + error, + ( + ConfigNotFound, + CredentialRetrievalError, + InvalidConfigError, + NoCredentialsError, + NoRegionError, + PartialCredentialsError, + ProfileNotFound, + ), + ): return AWSConfigurationError( "AWS Secrets Manager client configuration is invalid" ) - return SecretRequestError("AWS Secrets Manager request could not be completed") - - -def _is_batch_fallback_error(error: ClientError) -> bool: - """Return whether a batch-only failure should fall back to individual reads.""" - return error.response.get("Error", {}).get("Code") in { - "AccessDenied", - "AccessDeniedException", - "UnauthorizedOperation", - "UnknownOperationException", - "InvalidAction", - } - - -def _classify_error_code(error_code: Any) -> SecretRetrievalError: - """Map an AWS error code to its public retrieval exception.""" - if error_code == "ResourceNotFoundException": + if error == "ResourceNotFoundException": return SecretNotFoundError("AWS secret was not found") - if error_code in { + if error in { "AccessDenied", "AccessDeniedException", "AuthorizationError", @@ -342,15 +253,39 @@ def _classify_error_code(error_code: Any) -> SecretRetrievalError: return SecretRequestError("AWS Secrets Manager request could not be completed") -def _log_result(operation: str, outcome: str, secret_count: int | None = None) -> None: +def _log_debug( + operation: str, + outcome: str, + secret_count: int, + batch_number: int | None = None, +) -> None: extra: dict[str, str | int] = { "provider": _PROVIDER, "operation": operation, "outcome": outcome, + "secret_count": secret_count, + } + if batch_number is not None: + extra["batch_number"] = batch_number + message = "Secret retrieval attempt" if outcome == "attempt" else "Secret retrieval result" + logger.debug(message, extra=extra) + + +def _log_failure( + operation: str, + error: SecretRetrievalError, + secret_count: int, + failure_count: int, + batch_number: int | None = None, +) -> None: + extra: dict[str, str | int] = { + "provider": _PROVIDER, + "operation": operation, + "outcome": "failure", + "failure_category": type(error).__name__, + "secret_count": secret_count, + "failure_count": failure_count, } - if secret_count is not None: - extra["secret_count"] = secret_count - if outcome == "failure": - logger.error("Secret retrieval failed", extra=extra) - else: - logger.debug("Secret retrieval completed", extra=extra) + if batch_number is not None: + extra["batch_number"] = batch_number + logger.error("Secret retrieval failed", extra=extra) diff --git a/tests/test_aws_secrets_manager.py b/tests/test_aws_secrets_manager.py index 149e11f..fbb1e6b 100644 --- a/tests/test_aws_secrets_manager.py +++ b/tests/test_aws_secrets_manager.py @@ -1,58 +1,73 @@ import logging -from typing import Any, cast +from collections.abc import Mapping, Sequence +from typing import Any, NoReturn, cast -import boto3 import pytest from botocore.exceptions import ( + BotoCoreError, ClientError, + ConfigNotFound, + CredentialRetrievalError, EndpointConnectionError, + InvalidConfigError, NoCredentialsError, NoRegionError, + PartialCredentialsError, + ProfileNotFound, ) -from botocore.stub import Stubber from openhound.core.clients.aws_secrets_manager import ( AWSConfigurationError, AWSSecretsManager, - InvalidSecretValueError, SecretBatchError, SecretNotFoundError, SecretPermissionError, SecretRequestError, ) +LOGGER_NAME = "openhound.core.clients.aws_secrets_manager" +SECRET_ID = "secret-id-that-must-not-be-logged" +SECRET_VALUE = "secret-value-that-must-not-be-logged" +PROVIDER_MESSAGE = "provider message that must not be exposed" + class FakeSecretsManagerClient: def __init__( self, - response: dict[str, Any] | None = None, - error=None, - batch_response: dict[str, Any] | None = None, - batch_error=None, + *, + get_response: Mapping[str, Any] | None = None, + get_error: BaseException | None = None, + batch_responses: Sequence[Mapping[str, Any] | BaseException] | None = None, ) -> None: - self.response = response - self.error = error - self.batch_response = batch_response - self.batch_error = batch_error - self.requested_secret_id: str | None = None + self.get_response = get_response + self.get_error = get_error + self.batch_responses = list(batch_responses or []) + self.get_requests: list[str] = [] self.batch_requests: list[list[str]] = [] - def get_secret_value(self, *, SecretId: str) -> dict[str, Any]: - self.requested_secret_id = SecretId - if self.error is not None: - raise self.error - assert self.response is not None - return self.response - - def batch_get_secret_value(self, *, SecretIdList: list[str]) -> dict[str, Any]: - self.batch_requests.append(SecretIdList) - if self.batch_error is not None: - raise self.batch_error - assert self.batch_response is not None - return self.batch_response - - -def aws_error(code: str, message: str) -> ClientError: + def get_secret_value(self, *, SecretId: str) -> Mapping[str, Any]: + self.get_requests.append(SecretId) + if self.get_error is not None: + raise self.get_error + return self.get_response or {} + + def batch_get_secret_value( + self, *, SecretIdList: list[str] + ) -> Mapping[str, Any]: + self.batch_requests.append(list(SecretIdList)) + if not self.batch_responses: + return {"SecretValues": [], "Errors": []} + response = self.batch_responses.pop(0) + if isinstance(response, BaseException): + raise response + return response + + +def aws_error( + code: str, + message: str, + operation: str = "GetSecretValue", +) -> ClientError: return ClientError( { "Error": {"Code": code, "Message": message}, @@ -64,336 +79,253 @@ def aws_error(code: str, message: str) -> ClientError: "RetryAttempts": 0, }, }, - "GetSecretValue", + operation, ) -def test_get_secret_returns_plain_text_and_passes_secret_id() -> None: - client = FakeSecretsManagerClient({"SecretString": "secret-value"}) - - result = AWSSecretsManager(client).get_secret("secret-id") - - assert result == "secret-value" - assert client.requested_secret_id == "secret-id" - +def assert_safe_failure(error: BaseException, forbidden: set[str]) -> None: + assert all(value not in str(error) for value in forbidden) -def test_get_secret_returns_json_object() -> None: - client = FakeSecretsManagerClient({"SecretString": '{"username":"user"}'}) - result = AWSSecretsManager(client).get_secret("secret-id") - - assert result == {"username": "user"} - - -def test_get_secret_works_with_a_real_boto3_client_offline() -> None: - client = boto3.client( - "secretsmanager", - region_name="us-east-1", - aws_access_key_id="testing", - aws_secret_access_key="testing", - ) - response = { - "ARN": "arn:aws:secretsmanager:us-east-1:123456789012:secret:local-secret", - "Name": "local-secret", - "SecretString": '{"username":"local-user"}', - "VersionId": "00000000000000000000000000000001", - "CreatedDate": 0, - } - - with Stubber(client) as stubber: - stubber.add_response( - "get_secret_value", - response, - {"SecretId": "local-secret"}, - ) - - result = AWSSecretsManager(client).get_secret("local-secret") - - assert result == {"username": "local-user"} - stubber.assert_no_pending_responses() - - -def test_get_secrets_works_with_a_real_boto3_client_offline() -> None: - client = boto3.client( - "secretsmanager", - region_name="us-east-1", - aws_access_key_id="testing", - aws_secret_access_key="testing", +def assert_safe_logs( + caplog: pytest.LogCaptureFixture, forbidden: set[str] +) -> None: + assert all(value not in caplog.text for value in forbidden) + assert all( + all(value not in record.getMessage() for value in forbidden) + and all(value not in repr(record.__dict__) for value in forbidden) + for record in caplog.records ) - response = { - "SecretValues": [ - { - "ARN": "arn:aws:secretsmanager:us-east-1:123456789012:secret:first", - "Name": "first", - "SecretString": "value-1", - "VersionId": "00000000000000000000000000000001", - "CreatedDate": 0, - }, - { - "ARN": "arn:aws:secretsmanager:us-east-1:123456789012:secret:second", - "Name": "second", - "SecretString": "value-2", - "VersionId": "00000000000000000000000000000002", - "CreatedDate": 0, - }, - ], - "Errors": [], - } - - with Stubber(client) as stubber: - stubber.add_response( - "batch_get_secret_value", - response, - {"SecretIdList": ["first", "second"]}, - ) - - result = AWSSecretsManager(client).get_secrets(["first", "second"]) - - assert result == {"first": "value-1", "second": "value-2"} - stubber.assert_no_pending_responses() @pytest.mark.parametrize( - ("error_code", "exception_type"), + ("secret_text", "expected"), [ - ("ResourceNotFoundException", SecretNotFoundError), - ("AccessDeniedException", SecretPermissionError), + (SECRET_VALUE, SECRET_VALUE), + ('{"username":"user","enabled":true}', {"username": "user", "enabled": True}), + ('{"malformed":', '{"malformed":'), + ('[{"not": "an object result"}]', '[{"not": "an object result"}]'), ], ) -def test_get_secret_maps_aws_errors_without_leaking_values( - error_code: str, exception_type: type[Exception] +def test_get_secret_uses_get_secret_value_for_text_and_json_objects( + secret_text: str, expected: str | dict[str, Any] ) -> None: - secret_value = "do-not-leak-this-value" - client = FakeSecretsManagerClient(error=aws_error(error_code, secret_value)) - - with pytest.raises(exception_type) as raised: - AWSSecretsManager(client).get_secret("secret-id") - - assert secret_value not in str(raised.value) - assert "secret-id" not in str(raised.value) - - -def test_get_secret_rejects_malformed_json_without_leaking_value() -> None: - secret_value = '{"password": "do-not-leak-this-value"' - client = FakeSecretsManagerClient({"SecretString": secret_value}) + client = FakeSecretsManagerClient(get_response={"SecretString": secret_text}) - with pytest.raises(InvalidSecretValueError) as raised: - AWSSecretsManager(client).get_secret("secret-id") + assert AWSSecretsManager(client).get_secret(SECRET_ID) == expected + assert client.get_requests == [SECRET_ID] + assert client.batch_requests == [] - assert secret_value not in str(raised.value) - -def test_get_secret_rejects_binary_payload() -> None: - client = FakeSecretsManagerClient({"SecretBinary": b"secret-value"}) - - with pytest.raises(InvalidSecretValueError, match="SecretBinary"): - AWSSecretsManager(client).get_secret("secret-id") - - -def test_get_secret_classifies_network_failures_as_request_errors() -> None: - client = FakeSecretsManagerClient( - error=EndpointConnectionError(endpoint_url="https://secretsmanager.example") - ) - - with pytest.raises(SecretRequestError): - AWSSecretsManager(client).get_secret("secret-id") - - -def test_get_secrets_returns_values_mapped_to_requested_ids() -> None: +def test_get_secrets_returns_text_and_json_objects_from_batch_values() -> None: + json_secret = "json-secret" client = FakeSecretsManagerClient( - batch_response={ - "SecretValues": [ - {"Name": "second", "SecretString": '{"value":2}'}, - {"Name": "first", "SecretString": "value-1"}, - ] - } - ) - - result = AWSSecretsManager(client).get_secrets(["first", "second"]) - - assert result == {"first": "value-1", "second": {"value": 2}} - assert client.batch_requests == [["first", "second"]] - - -def test_get_secrets_falls_back_when_batch_api_is_unavailable() -> None: - class LegacyClient: - def __init__(self) -> None: - self.requested_ids: list[str] = [] - - def get_secret_value(self, *, SecretId: str) -> dict[str, str]: - self.requested_ids.append(SecretId) - return {"SecretString": f"value-{SecretId}"} - - client = LegacyClient() - - result = AWSSecretsManager(client).get_secrets(["first", "second"]) - - assert result == {"first": "value-first", "second": "value-second"} - assert client.requested_ids == ["first", "second"] - - -def test_get_secrets_rejects_string_secret_ids() -> None: - client = FakeSecretsManagerClient(batch_response={"SecretValues": []}) - - with pytest.raises(TypeError, match="iterable of secret identifiers"): - AWSSecretsManager(client).get_secrets("secret-id") - - -def test_get_secrets_falls_back_when_batch_permission_is_missing() -> None: - class BatchPermissionClient(FakeSecretsManagerClient): - def get_secret_value(self, *, SecretId: str) -> dict[str, str]: - self.requested_secret_id = SecretId - return {"SecretString": f"value-{SecretId}"} - - client = BatchPermissionClient( - batch_error=aws_error("AccessDeniedException", "batch permission required") + batch_responses=[ + { + "SecretValues": [ + {"Name": "text-secret", "SecretString": "value-1"}, + {"ARN": json_secret, "SecretString": '{"value": 2}'}, + ], + "Errors": [], + } + ] ) - result = AWSSecretsManager(client).get_secrets(["first", "second"]) - - assert result == {"first": "value-first", "second": "value-second"} - assert client.batch_requests == [["first", "second"]] - assert client.requested_secret_id == "second" + result = AWSSecretsManager(client).get_secrets(["text-secret", json_secret]) + assert result == {"text-secret": "value-1", json_secret: {"value": 2}} + assert client.batch_requests == [["text-secret", json_secret]] + assert client.get_requests == [] -def test_get_secrets_splits_requests_into_chunks_of_20() -> None: - secret_ids = [f"secret-{index}" for index in range(43)] - def response_for(requested_ids: list[str]) -> dict[str, Any]: - return { +def test_get_secrets_splits_requests_into_chunks_of_20_and_merges_values() -> None: + secret_ids = [f"secret-{index}" for index in range(41)] + responses = [ + { "SecretValues": [ - {"Name": secret_id, "SecretString": secret_id} - for secret_id in reversed(requested_ids) + {"Name": secret_id, "SecretString": f"value-{secret_id}"} + for secret_id in secret_ids[start : start + 20] ] } - - class ChunkingClient(FakeSecretsManagerClient): - def batch_get_secret_value(self, *, SecretIdList: list[str]) -> dict[str, Any]: - self.batch_requests.append(SecretIdList) - return response_for(SecretIdList) - - client = ChunkingClient() + for start in range(0, len(secret_ids), 20) + ] + client = FakeSecretsManagerClient(batch_responses=responses) result = AWSSecretsManager(client).get_secrets(secret_ids) - assert list(result) == secret_ids - assert [len(request) for request in client.batch_requests] == [20, 20, 3] + assert result == {secret_id: f"value-{secret_id}" for secret_id in secret_ids} + assert [len(request) for request in client.batch_requests] == [20, 20, 1] + assert client.batch_requests == [ + secret_ids[:20], + secret_ids[20:40], + secret_ids[40:], + ] -def test_get_secrets_raises_typed_error_for_partial_batch_failure( +def test_get_secrets_continues_and_aggregates_typed_failures_safely( caplog: pytest.LogCaptureFixture, ) -> None: - secret_value = "do-not-log-this-value" + secret_ids = [f"secret-{index}" for index in range(21)] + first_failure = secret_ids[2] + second_failure = secret_ids[-1] client = FakeSecretsManagerClient( - batch_response={ - "SecretValues": [{"Name": "available", "SecretString": secret_value}], - "Errors": [ - { - "SecretId": "missing", - "ErrorCode": "ResourceNotFoundException", - "ErrorMessage": secret_value, - } - ], - } + batch_responses=[ + { + "SecretValues": [ + {"Name": secret_id, "SecretString": SECRET_VALUE} + for secret_id in secret_ids[:20] + if secret_id != first_failure + ], + "Errors": [ + { + "SecretId": first_failure, + "ErrorCode": "ResourceNotFoundException", + "Message": PROVIDER_MESSAGE, + } + ], + }, + aws_error( + "AccessDeniedException", + PROVIDER_MESSAGE, + operation="BatchGetSecretValue", + ), + ] ) - - with caplog.at_level(logging.DEBUG), pytest.raises(SecretBatchError) as raised: - AWSSecretsManager(client).get_secrets(["available", "missing"]) - - assert isinstance(raised.value.failures["missing"], SecretNotFoundError) - forbidden_values = {"available", "missing", secret_value} - assert all(value not in str(raised.value) for value in forbidden_values) - assert all(value not in caplog.text for value in forbidden_values) - assert all( - all(value not in repr(record.__dict__) for value in forbidden_values) + forbidden = set(secret_ids) | {SECRET_VALUE, PROVIDER_MESSAGE} + + with caplog.at_level(logging.DEBUG, logger=LOGGER_NAME), pytest.raises( + SecretBatchError + ) as raised: + AWSSecretsManager(client).get_secrets(secret_ids) + + assert client.batch_requests == [secret_ids[:20], secret_ids[20:]] + assert set(raised.value.failures) == {first_failure, second_failure} + assert isinstance(raised.value.failures[first_failure], SecretNotFoundError) + assert isinstance(raised.value.failures[second_failure], SecretPermissionError) + assert_safe_failure(raised.value, forbidden) + for failure in raised.value.failures.values(): + assert_safe_failure(failure, forbidden) + + attempts = [ + cast(Any, record) for record in caplog.records - ) - assert len(caplog.records) == 1 - record = cast(Any, caplog.records[0]) - assert record.levelno == logging.ERROR - assert record.provider == "aws.secretsmanager" - assert record.operation == "batch_get_secret_value" - assert record.outcome == "failure" - assert record.secret_count == 2 + if getattr(record, "outcome", None) == "attempt" + ] + assert [record.batch_number for record in attempts] == [1, 2] + error_records = [ + cast(Any, record) + for record in caplog.records + if record.levelno == logging.ERROR + ] + assert len(error_records) == 2 + assert all(record.failure_category == "SecretBatchError" for record in error_records) + assert_safe_logs(caplog, forbidden) @pytest.mark.parametrize( - ("error", "expected"), + ("error", "expected_type"), [ - (NoCredentialsError(), AWSConfigurationError), - (NoRegionError(), AWSConfigurationError), + ( + aws_error("ResourceNotFoundException", PROVIDER_MESSAGE), + SecretNotFoundError, + ), + ( + aws_error("AccessDeniedException", PROVIDER_MESSAGE), + SecretPermissionError, + ), ( EndpointConnectionError(endpoint_url="https://secretsmanager.example"), SecretRequestError, ), - (aws_error("SomeOtherError", "do-not-leak-this-value"), SecretRequestError), ], ) -def test_get_secrets_classifies_sdk_failures(error, expected) -> None: - client = FakeSecretsManagerClient(batch_error=error) - - with pytest.raises(SecretBatchError) as raised: - AWSSecretsManager(client).get_secrets(["secret-id"]) - - assert isinstance(raised.value.failures["secret-id"], expected) - - -def test_get_secrets_logs_structured_success_without_secret_values( - caplog: pytest.LogCaptureFixture, +def test_get_secret_classifies_aws_errors( + error: BotoCoreError | ClientError, + expected_type: type[Exception], ) -> None: - text_value = "plain-secret-value" - json_value = '{"password":"json-secret-value"}' - client = FakeSecretsManagerClient( - batch_response={ - "SecretValues": [ - {"Name": "text-secret", "SecretString": text_value}, - {"Name": "json-secret", "SecretString": json_value}, - ] - } - ) - - with caplog.at_level(logging.DEBUG): - AWSSecretsManager(client).get_secrets(["text-secret", "json-secret"]) + client = FakeSecretsManagerClient(get_error=error) - assert len(caplog.records) == 1 - record = cast(Any, caplog.records[0]) - assert record.levelno == logging.DEBUG - assert record.provider == "aws.secretsmanager" - assert record.operation == "batch_get_secret_value" - assert record.outcome == "success" - assert record.secret_count == 2 - forbidden_values = { - "text-secret", - "json-secret", - text_value, - json_value, - } - assert all(value not in repr(record.__dict__) for value in forbidden_values) - assert all(value not in caplog.text for value in forbidden_values) + with pytest.raises(expected_type) as raised: + AWSSecretsManager(client).get_secret(SECRET_ID) + assert_safe_failure(raised.value, {SECRET_ID, PROVIDER_MESSAGE}) -def test_constructor_rejects_invalid_client() -> None: - with pytest.raises(AWSConfigurationError, match="does not support"): - AWSSecretsManager(cast(Any, object())) +@pytest.mark.parametrize( + "sdk_error", + [ + NoCredentialsError(), + PartialCredentialsError(provider="aws", cred_var="secret-key"), + NoRegionError(), + InvalidConfigError(error_msg=PROVIDER_MESSAGE), + ConfigNotFound(path=PROVIDER_MESSAGE), + ProfileNotFound(profile=PROVIDER_MESSAGE), + CredentialRetrievalError( + provider="credential-provider", error_msg=PROVIDER_MESSAGE + ), + ], +) +def test_client_creation_configuration_failures_are_classified( + monkeypatch: pytest.MonkeyPatch, + sdk_error: BotoCoreError, +) -> None: + def fail_client(*_args: object, **_kwargs: object) -> NoReturn: + raise sdk_error -def test_constructor_does_not_create_an_aws_client(monkeypatch) -> None: monkeypatch.setattr( - "openhound.core.clients.aws_secrets_manager.boto3.client", - lambda *_: pytest.fail(), + "openhound.core.clients.aws_secrets_manager.boto3.client", fail_client ) - AWSSecretsManager() - + with pytest.raises(AWSConfigurationError): + AWSSecretsManager().get_secret(SECRET_ID) -def test_get_secret_classifies_lazy_client_creation_failure(monkeypatch) -> None: - def fail_client_creation(*_args): - raise NoRegionError() - monkeypatch.setattr( - "openhound.core.clients.aws_secrets_manager.boto3.client", - fail_client_creation, +def test_successful_retrieval_logs_safe_attempt_and_result_records( + caplog: pytest.LogCaptureFixture, +) -> None: + client = FakeSecretsManagerClient(get_response={"SecretString": SECRET_VALUE}) + + with caplog.at_level(logging.DEBUG, logger=LOGGER_NAME): + AWSSecretsManager(client).get_secret(SECRET_ID) + + records = [cast(Any, record) for record in caplog.records] + assert [(record.levelno, record.outcome) for record in records] == [ + (logging.DEBUG, "attempt"), + (logging.DEBUG, "success"), + ] + assert all(record.provider == "aws.secretsmanager" for record in records) + assert all(record.operation == "get_secret_value" for record in records) + assert all(record.secret_count == 1 for record in records) + assert_safe_logs(caplog, {SECRET_ID, SECRET_VALUE}) + + caplog.clear() + batch_client = FakeSecretsManagerClient( + batch_responses=[ + { + "SecretValues": [ + {"Name": "first", "SecretString": SECRET_VALUE}, + {"Name": "second", "SecretString": "second-value"}, + ] + } + ] + ) + with caplog.at_level(logging.DEBUG, logger=LOGGER_NAME): + AWSSecretsManager(batch_client).get_secrets(["first", "second"]) + + batch_records = [cast(Any, record) for record in caplog.records] + assert [(record.levelno, record.outcome) for record in batch_records] == [ + (logging.DEBUG, "attempt"), + (logging.DEBUG, "success"), + ] + assert all(record.operation == "batch_get_secret_value" for record in batch_records) + assert all(record.secret_count == 2 for record in batch_records) + assert all(record.batch_number == 1 for record in batch_records) + assert_safe_logs( + caplog, + {"first", "second", SECRET_VALUE, "second-value"}, ) - with pytest.raises(AWSConfigurationError): - AWSSecretsManager().get_secret("secret-id") + +def test_get_secrets_rejects_a_string_instead_of_ids() -> None: + with pytest.raises(TypeError, match="iterable of secret identifiers"): + AWSSecretsManager(FakeSecretsManagerClient()).get_secrets(SECRET_ID) diff --git a/uv.lock b/uv.lock index 8cce2cd..20a4cf4 100644 --- a/uv.lock +++ b/uv.lock @@ -173,19 +173,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/4b/c3/1d5c5ea7f599e2b80d122beeb2a9c1a8daff78325ea2391521e5fd5af1fd/boto3-1.43.90-py3-none-any.whl", hash = "sha256:aaaa1216d65ddb3dcf86bf9d93cfa436af05cbcd1e1f8b2847f2b83116012186", size = 140026, upload-time = "2026-09-08T19:22:28.555Z" }, ] -[[package]] -name = "boto3-stubs" -version = "1.43.90" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "botocore-stubs" }, - { name = "types-s3transfer" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/db/cb/d5967b06c53f59afe9dea69e3018aad40fa07af7c74ac68e4200787bca8f/boto3_stubs-1.43.90.tar.gz", hash = "sha256:ffca416bc23a315d2a0e770401b13d349d9b79cf30d8f75d01768e9f2fda4812", size = 104565, upload-time = "2026-09-08T20:29:14.726Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/97/26/6b55acc18177a0dd7c9f4745ed32a5c8415aa96f2f19e829e36a80739232/boto3_stubs-1.43.90-py3-none-any.whl", hash = "sha256:6350af6f4199d759d04654ce831bf5d4e90bfc7d606ae18c0085b1566b9af418", size = 71510, upload-time = "2026-09-08T20:29:10.782Z" }, -] - [[package]] name = "botocore" version = "1.43.90" @@ -200,15 +187,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/f6/b8/65cbd0d65553c3e51d27159de4c74308ea008c652b05ed3f830f32667efb/botocore-1.43.90-py3-none-any.whl", hash = "sha256:65f3394ef07314e45c92a90120988531d651136390d464a94938a92f665893f7", size = 15775313, upload-time = "2026-09-08T19:22:22.713Z" }, ] -[[package]] -name = "botocore-stubs" -version = "1.43.67" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/3f/45/53d662227dc4787b2c854445ee7eb4751cb5d74cfb5c686a6ecbe1f94c17/botocore_stubs-1.43.67.tar.gz", hash = "sha256:853e74014a1f557055c4ffae5fb38d7c65c7c0520e1aab366cac41d5428f419d", size = 42846, upload-time = "2026-08-08T14:57:53.412Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/4e/5e/bdbf19967898a032292da65a47d6e25b2eee55865db4e687f861d80b5602/botocore_stubs-1.43.67-py3-none-any.whl", hash = "sha256:c51262bac3341c1cda71f05fa01141fffd3990d7a92c7960e3b755c1bc830373", size = 67244, upload-time = "2026-08-08T14:57:52.01Z" }, -] - [[package]] name = "certifi" version = "2026.7.22" @@ -1363,7 +1341,6 @@ okta = [ [package.dev-dependencies] dev = [ { name = "altair" }, - { name = "boto3-stubs" }, { name = "fastapi" }, { name = "httpx" }, { name = "ipython" }, @@ -1407,7 +1384,6 @@ provides-extras = ["all", "github", "jamf", "okta"] [package.metadata.requires-dev] dev = [ { name = "altair", specifier = ">=6.1.0" }, - { name = "boto3-stubs", specifier = "~=1.43.90" }, { name = "fastapi", specifier = ">=0.136.1" }, { name = "httpx", specifier = ">=0.28.1" }, { name = "ipython", specifier = ">=9.13.0" }, @@ -2408,15 +2384,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/c3/82/959113a6351f3ca046cd0a8cd2cee071d7ea47473560557a01eeae9a6fe2/types_requests-2.33.0.20260503-py3-none-any.whl", hash = "sha256:02aaa7e3577a13471715bb1bddb693cc985ea514f754b503bf033e6a09a3e528", size = 20736, upload-time = "2026-05-03T05:20:07.858Z" }, ] -[[package]] -name = "types-s3transfer" -version = "0.16.0" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/fe/64/42689150509eb3e6e82b33ee3d89045de1592488842ddf23c56957786d05/types_s3transfer-0.16.0.tar.gz", hash = "sha256:b4636472024c5e2b62278c5b759661efeb52a81851cde5f092f24100b1ecb443", size = 13557, upload-time = "2025-12-08T08:13:09.928Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/98/27/e88220fe6274eccd3bdf95d9382918716d312f6f6cef6a46332d1ee2feff/types_s3transfer-0.16.0-py3-none-any.whl", hash = "sha256:1c0cd111ecf6e21437cb410f5cddb631bfb2263b77ad973e79b9c6d0cb24e0ef", size = 19247, upload-time = "2025-12-08T08:13:08.426Z" }, -] - [[package]] name = "typing-extensions" version = "4.16.0" From 4c7befe41c3b36a356f7e4a8e27818dea0da746d Mon Sep 17 00:00:00 2001 From: Stran Dutton Date: Tue, 15 Sep 2026 13:04:00 -0500 Subject: [PATCH 5/5] preserve key order --- src/openhound/core/clients/aws_secrets_manager.py | 6 +++++- tests/test_aws_secrets_manager.py | 6 ++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/openhound/core/clients/aws_secrets_manager.py b/src/openhound/core/clients/aws_secrets_manager.py index 63adc72..567369c 100644 --- a/src/openhound/core/clients/aws_secrets_manager.py +++ b/src/openhound/core/clients/aws_secrets_manager.py @@ -131,7 +131,11 @@ def _get_secrets_in_batches( if failures: raise SecretBatchError(failures) from None - return values + return { + secret_id: values[secret_id] + for secret_id in secret_ids + if secret_id in values + } def _get_client(self) -> SecretsManagerClient: if self._client is not None: diff --git a/tests/test_aws_secrets_manager.py b/tests/test_aws_secrets_manager.py index fbb1e6b..98b01e1 100644 --- a/tests/test_aws_secrets_manager.py +++ b/tests/test_aws_secrets_manager.py @@ -123,8 +123,8 @@ def test_get_secrets_returns_text_and_json_objects_from_batch_values() -> None: batch_responses=[ { "SecretValues": [ - {"Name": "text-secret", "SecretString": "value-1"}, {"ARN": json_secret, "SecretString": '{"value": 2}'}, + {"Name": "text-secret", "SecretString": "value-1"}, ], "Errors": [], } @@ -134,6 +134,7 @@ def test_get_secrets_returns_text_and_json_objects_from_batch_values() -> None: result = AWSSecretsManager(client).get_secrets(["text-secret", json_secret]) assert result == {"text-secret": "value-1", json_secret: {"value": 2}} + assert list(result) == ["text-secret", json_secret] assert client.batch_requests == [["text-secret", json_secret]] assert client.get_requests == [] @@ -144,7 +145,7 @@ def test_get_secrets_splits_requests_into_chunks_of_20_and_merges_values() -> No { "SecretValues": [ {"Name": secret_id, "SecretString": f"value-{secret_id}"} - for secret_id in secret_ids[start : start + 20] + for secret_id in reversed(secret_ids[start : start + 20]) ] } for start in range(0, len(secret_ids), 20) @@ -154,6 +155,7 @@ def test_get_secrets_splits_requests_into_chunks_of_20_and_merges_values() -> No result = AWSSecretsManager(client).get_secrets(secret_ids) assert result == {secret_id: f"value-{secret_id}" for secret_id in secret_ids} + assert list(result) == secret_ids assert [len(request) for request in client.batch_requests] == [20, 20, 1] assert client.batch_requests == [ secret_ids[:20],