From d88aee5623d21cf020597a2bc9fbd9102d78cf27 Mon Sep 17 00:00:00 2001 From: jonathan343 Date: Mon, 10 Nov 2025 09:54:23 -0500 Subject: [PATCH 1/6] Add simple process credentials resolver --- .../src/smithy_aws_core/identity/__init__.py | 3 + .../src/smithy_aws_core/identity/process.py | 99 ++++++ .../tests/unit/identity/test_process.py | 307 ++++++++++++++++++ 3 files changed, 409 insertions(+) create mode 100644 packages/smithy-aws-core/src/smithy_aws_core/identity/process.py create mode 100644 packages/smithy-aws-core/tests/unit/identity/test_process.py diff --git a/packages/smithy-aws-core/src/smithy_aws_core/identity/__init__.py b/packages/smithy-aws-core/src/smithy_aws_core/identity/__init__.py index 1b310e5bd..09b9467eb 100644 --- a/packages/smithy-aws-core/src/smithy_aws_core/identity/__init__.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/identity/__init__.py @@ -11,6 +11,7 @@ from .container import ContainerCredentialsResolver from .environment import EnvironmentCredentialsResolver from .imds import IMDSCredentialsResolver +from .process import ProcessCredentialsConfig, ProcessCredentialsResolver from .static import StaticCredentialsResolver __all__ = ( @@ -20,6 +21,8 @@ "ContainerCredentialsResolver", "EnvironmentCredentialsResolver", "IMDSCredentialsResolver", + "ProcessCredentialsConfig", + "ProcessCredentialsResolver", "StaticCredentialsResolver", ) diff --git a/packages/smithy-aws-core/src/smithy_aws_core/identity/process.py b/packages/smithy-aws-core/src/smithy_aws_core/identity/process.py new file mode 100644 index 000000000..ae685a01f --- /dev/null +++ b/packages/smithy-aws-core/src/smithy_aws_core/identity/process.py @@ -0,0 +1,99 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +import asyncio +import json +from dataclasses import dataclass +from datetime import UTC, datetime + +from smithy_core.aio.interfaces.identity import IdentityResolver +from smithy_core.exceptions import SmithyIdentityError + +from smithy_aws_core.identity.components import ( + AWSCredentialsIdentity, + AWSIdentityProperties, +) + +_DEFAULT_TIMEOUT = 30 + + +@dataclass +class ProcessCredentialsConfig: + """Configuration for process credential retrieval operations.""" + + timeout: int = _DEFAULT_TIMEOUT + + +class ProcessCredentialsResolver( + IdentityResolver[AWSCredentialsIdentity, AWSIdentityProperties] +): + """Resolves AWS Credentials from a process.""" + + def __init__( + self, + command: list[str], + config: ProcessCredentialsConfig | None = None, + ): + if not command: + raise ValueError("command must be a non-empty list") + self._command = command + self._config = config or ProcessCredentialsConfig() + self._credentials = None + + async def get_identity( + self, *, properties: AWSIdentityProperties + ) -> AWSCredentialsIdentity: + if self._credentials is not None: + # Long-term credentials (no expiration) should always be reused + if self._credentials.expiration is None: + return self._credentials + # Temporary credentials should be reused if not expired + if datetime.now(UTC) < self._credentials.expiration: + return self._credentials + + try: + process = await asyncio.create_subprocess_exec( + *self._command, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + ) + stdout, stderr = await asyncio.wait_for( + process.communicate(), timeout=self._config.timeout + ) + except TimeoutError as e: + raise SmithyIdentityError( + f"Credential process timed out after {self._config.timeout} seconds" + ) from e + + if process.returncode != 0: + raise SmithyIdentityError( + f"Credential process failed with non-zero exit code: {stderr.decode('utf-8')}" + ) + creds = json.loads(stdout.decode("utf-8")) + + version = creds.get("Version") + if version is None or version != 1: + raise SmithyIdentityError( + f"Unsupported version '{version}' for credential process provider, supported versions: 1" + ) + access_key_id = creds.get("AccessKeyId") + secret_access_key = creds.get("SecretAccessKey") + session_token = creds.get("SessionToken") + expiration = creds.get("Expiration") + account_id = creds.get("AccountId") + + if isinstance(expiration, str): + expiration = datetime.fromisoformat(expiration).replace(tzinfo=UTC) + + if access_key_id is None or secret_access_key is None: + raise SmithyIdentityError( + "AccessKeyId and SecretAccessKey are required for process credentials" + ) + + self._credentials = AWSCredentialsIdentity( + access_key_id=access_key_id, + secret_access_key=secret_access_key, + session_token=session_token, + expiration=expiration, + account_id=account_id, + ) + return self._credentials diff --git a/packages/smithy-aws-core/tests/unit/identity/test_process.py b/packages/smithy-aws-core/tests/unit/identity/test_process.py new file mode 100644 index 000000000..f9fd9e597 --- /dev/null +++ b/packages/smithy-aws-core/tests/unit/identity/test_process.py @@ -0,0 +1,307 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +from __future__ import annotations + +import asyncio +import json +from datetime import UTC, datetime, timedelta +from unittest.mock import AsyncMock, patch + +import pytest +from smithy_aws_core.identity.process import ( + ProcessCredentialsConfig, + ProcessCredentialsResolver, +) +from smithy_core.exceptions import SmithyIdentityError + +ISO8601 = "%Y-%m-%dT%H:%M:%SZ" + +DEFAULT_RESPONSE_DATA = { + "Version": 1, + "AccessKeyId": "akid123", + "SecretAccessKey": "s3cr3t", + "SessionToken": "session_token", +} + + +def test_config_default_values(): + config = ProcessCredentialsConfig() + assert config.timeout == 30 + + +def test_config_custom_values(): + config = ProcessCredentialsConfig(timeout=60) + assert config.timeout == 60 + + +def test_resolver_empty_command(): + with pytest.raises(ValueError, match="command must be a non-empty list"): + ProcessCredentialsResolver([]) + + +def test_resolver_none_command(): + with pytest.raises(ValueError, match="command must be a non-empty list"): + ProcessCredentialsResolver(None) # type: ignore[arg-type] + + +def mock_subprocess(returncode: int, stdout: bytes, stderr: bytes = b""): + """Helper to mock asyncio.create_subprocess_exec""" + process = AsyncMock() + process.returncode = returncode + process.communicate.return_value = (stdout, stderr) + return process + + +@pytest.mark.asyncio +async def test_valid_credentials_with_session_token(): + resp_body = json.dumps(DEFAULT_RESPONSE_DATA) + process = mock_subprocess(0, resp_body.encode("utf-8")) + + with patch("asyncio.create_subprocess_exec", return_value=process): + resolver = ProcessCredentialsResolver(["mock-process"]) + identity = await resolver.get_identity(properties={}) + + assert identity.access_key_id == "akid123" + assert identity.secret_access_key == "s3cr3t" + assert identity.session_token == "session_token" + assert identity.expiration is None + assert identity.account_id is None + + +@pytest.mark.asyncio +async def test_valid_credentials_without_session_token(): + resp_data = { + "Version": 1, + "AccessKeyId": "akid456", + "SecretAccessKey": "s3cr3t456", + } + resp_body = json.dumps(resp_data) + process = mock_subprocess(0, resp_body.encode("utf-8")) + + with patch("asyncio.create_subprocess_exec", return_value=process): + resolver = ProcessCredentialsResolver(["mock-process"]) + identity = await resolver.get_identity(properties={}) + + assert identity.access_key_id == "akid456" + assert identity.secret_access_key == "s3cr3t456" + assert identity.session_token is None + + +@pytest.mark.asyncio +async def test_credentials_with_expiration(): + current_time = datetime.now(UTC) + timedelta(minutes=10) + resp_data = dict(DEFAULT_RESPONSE_DATA) + resp_data["Expiration"] = current_time.strftime(ISO8601) + + resp_body = json.dumps(resp_data) + process = mock_subprocess(0, resp_body.encode("utf-8")) + + with patch("asyncio.create_subprocess_exec", return_value=process): + resolver = ProcessCredentialsResolver(["mock-process"]) + identity = await resolver.get_identity(properties={}) + + assert identity.expiration is not None + assert identity.expiration.tzinfo == UTC + + +@pytest.mark.asyncio +async def test_credentials_with_account_id(): + resp_data = dict(DEFAULT_RESPONSE_DATA) + resp_data["AccountId"] = "123456789012" + + resp_body = json.dumps(resp_data) + process = mock_subprocess(0, resp_body.encode("utf-8")) + + with patch("asyncio.create_subprocess_exec", return_value=process): + resolver = ProcessCredentialsResolver(["mock-process"]) + identity = await resolver.get_identity(properties={}) + + assert identity.account_id == "123456789012" + + +@pytest.mark.asyncio +async def test_non_zero_exit_code(): + process = mock_subprocess(1, b"", b"Process error message") + + with patch("asyncio.create_subprocess_exec", return_value=process): + resolver = ProcessCredentialsResolver(["mock-process"]) + with pytest.raises(SmithyIdentityError, match="non-zero exit code"): + await resolver.get_identity(properties={}) + + +@pytest.mark.asyncio +async def test_missing_access_key_id(): + resp_data = { + "Version": 1, + "SecretAccessKey": "s3cr3t", + } + resp_body = json.dumps(resp_data) + process = mock_subprocess(0, resp_body.encode("utf-8")) + + with patch("asyncio.create_subprocess_exec", return_value=process): + resolver = ProcessCredentialsResolver(["mock-process"]) + with pytest.raises( + SmithyIdentityError, + match="AccessKeyId and SecretAccessKey are required", + ): + await resolver.get_identity(properties={}) + + +@pytest.mark.asyncio +async def test_missing_secret_access_key(): + resp_data = { + "Version": 1, + "AccessKeyId": "akid123", + } + resp_body = json.dumps(resp_data) + process = mock_subprocess(0, resp_body.encode("utf-8")) + + with patch("asyncio.create_subprocess_exec", return_value=process): + resolver = ProcessCredentialsResolver(["mock-process"]) + with pytest.raises( + SmithyIdentityError, + match="AccessKeyId and SecretAccessKey are required", + ): + await resolver.get_identity(properties={}) + + +@pytest.mark.asyncio +async def test_invalid_version(): + resp_data = dict(DEFAULT_RESPONSE_DATA) + resp_data["Version"] = 2 + + resp_body = json.dumps(resp_data) + process = mock_subprocess(0, resp_body.encode("utf-8")) + + with patch("asyncio.create_subprocess_exec", return_value=process): + resolver = ProcessCredentialsResolver(["mock-process"]) + with pytest.raises(SmithyIdentityError, match="Unsupported version '2'"): + await resolver.get_identity(properties={}) + + +@pytest.mark.asyncio +async def test_missing_version(): + resp_data = { + "AccessKeyId": "akid123", + "SecretAccessKey": "s3cr3t", + } + resp_body = json.dumps(resp_data) + process = mock_subprocess(0, resp_body.encode("utf-8")) + + with patch("asyncio.create_subprocess_exec", return_value=process): + resolver = ProcessCredentialsResolver(["mock-process"]) + with pytest.raises(SmithyIdentityError, match="Unsupported version 'None'"): + await resolver.get_identity(properties={}) + + +@pytest.mark.asyncio +async def test_invalid_json(): + process = mock_subprocess(0, b"not valid json") + + with patch("asyncio.create_subprocess_exec", return_value=process): + resolver = ProcessCredentialsResolver(["mock-process"]) + with pytest.raises(json.JSONDecodeError): + await resolver.get_identity(properties={}) + + +@pytest.mark.asyncio +async def test_process_timeout(): + async def timeout_communicate(): + await asyncio.sleep(100) + return (b"", b"") + + process = AsyncMock() + process.communicate = timeout_communicate + + config = ProcessCredentialsConfig(timeout=1) + + with patch("asyncio.create_subprocess_exec", return_value=process): + resolver = ProcessCredentialsResolver(["mock-process"], config=config) + with pytest.raises(SmithyIdentityError, match="timed out after 1 seconds"): + await resolver.get_identity(properties={}) + + +@pytest.mark.asyncio +async def test_long_term_credentials_cached(): + """Test that credentials without expiration are cached indefinitely.""" + resp_body = json.dumps(DEFAULT_RESPONSE_DATA) + process = mock_subprocess(0, resp_body.encode("utf-8")) + + with patch("asyncio.create_subprocess_exec", return_value=process) as mock_exec: + resolver = ProcessCredentialsResolver(["mock-process"]) + identity_one = await resolver.get_identity(properties={}) + identity_two = await resolver.get_identity(properties={}) + + # Process should only be called once + assert mock_exec.call_count == 1 + # Should return the same identity instance + assert identity_one is identity_two + + +@pytest.mark.asyncio +async def test_temporary_credentials_cached_when_valid(): + """Test that temporary credentials are cached when not expired.""" + current_time = datetime.now(UTC) + timedelta(minutes=10) + resp_data = dict(DEFAULT_RESPONSE_DATA) + resp_data["Expiration"] = current_time.strftime(ISO8601) + + resp_body = json.dumps(resp_data) + process = mock_subprocess(0, resp_body.encode("utf-8")) + + with patch("asyncio.create_subprocess_exec", return_value=process) as mock_exec: + resolver = ProcessCredentialsResolver(["mock-process"]) + identity_one = await resolver.get_identity(properties={}) + identity_two = await resolver.get_identity(properties={}) + + # Process should only be called once + assert mock_exec.call_count == 1 + # Should return the same identity instance + assert identity_one is identity_two + + +@pytest.mark.asyncio +async def test_expired_credentials_refreshed(): + """Test that expired credentials are refreshed.""" + expired_time = datetime.now(UTC) - timedelta(minutes=10) + resp_data = dict(DEFAULT_RESPONSE_DATA) + resp_data["Expiration"] = expired_time.strftime(ISO8601) + + resp_body = json.dumps(resp_data) + process = mock_subprocess(0, resp_body.encode("utf-8")) + + with patch("asyncio.create_subprocess_exec", return_value=process) as mock_exec: + resolver = ProcessCredentialsResolver(["mock-process"]) + identity_one = await resolver.get_identity(properties={}) + identity_two = await resolver.get_identity(properties={}) + + # Process should be called twice (once for initial, once for refresh) + assert mock_exec.call_count == 2 + # Should be different instances + assert identity_one is not identity_two + # But have the same values + assert identity_one.access_key_id == identity_two.access_key_id + assert identity_one.secret_access_key == identity_two.secret_access_key + + +@pytest.mark.asyncio +async def test_command_with_multiple_args(): + """Test that commands with multiple arguments are passed correctly.""" + resp_body = json.dumps(DEFAULT_RESPONSE_DATA) + process = mock_subprocess(0, resp_body.encode("utf-8")) + + with patch("asyncio.create_subprocess_exec", return_value=process) as mock_exec: + resolver = ProcessCredentialsResolver( + ["aws-credential-helper", "--profile", "test", "--format", "json"] + ) + await resolver.get_identity(properties={}) + + # Verify the command was called with all arguments + mock_exec.assert_called_once_with( + "aws-credential-helper", + "--profile", + "test", + "--format", + "json", + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + ) From de84579bd3aea42c9c418518dce9e26f945b1156 Mon Sep 17 00:00:00 2001 From: jonathan343 Date: Sat, 14 Mar 2026 23:39:48 -0400 Subject: [PATCH 2/6] Improve process credentials resolver command handling --- .../src/smithy_aws_core/identity/process.py | 22 ++++- .../tests/unit/identity/test_process.py | 83 +++++++++++++++++-- 2 files changed, 93 insertions(+), 12 deletions(-) diff --git a/packages/smithy-aws-core/src/smithy_aws_core/identity/process.py b/packages/smithy-aws-core/src/smithy_aws_core/identity/process.py index ae685a01f..2ace33e3a 100644 --- a/packages/smithy-aws-core/src/smithy_aws_core/identity/process.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/identity/process.py @@ -2,6 +2,7 @@ # SPDX-License-Identifier: Apache-2.0 import asyncio import json +import shlex from dataclasses import dataclass from datetime import UTC, datetime @@ -30,12 +31,15 @@ class ProcessCredentialsResolver( def __init__( self, - command: list[str], + command: str | list[str], config: ProcessCredentialsConfig | None = None, ): - if not command: - raise ValueError("command must be a non-empty list") - self._command = command + normalized_command = ( + shlex.split(command) if isinstance(command, str) else command + ) + if not normalized_command: + raise ValueError("command must be a non-empty string or list") + self._command = list(normalized_command) self._config = config or ProcessCredentialsConfig() self._credentials = None @@ -56,10 +60,20 @@ async def get_identity( stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, ) + except OSError as e: + raise SmithyIdentityError(f"Credential process failed to start: {e}") from e + + try: stdout, stderr = await asyncio.wait_for( process.communicate(), timeout=self._config.timeout ) except TimeoutError as e: + if process.returncode is None: + try: + process.kill() + except ProcessLookupError: + pass + await process.wait() raise SmithyIdentityError( f"Credential process timed out after {self._config.timeout} seconds" ) from e diff --git a/packages/smithy-aws-core/tests/unit/identity/test_process.py b/packages/smithy-aws-core/tests/unit/identity/test_process.py index f9fd9e597..91eb6ef5b 100644 --- a/packages/smithy-aws-core/tests/unit/identity/test_process.py +++ b/packages/smithy-aws-core/tests/unit/identity/test_process.py @@ -5,13 +5,15 @@ import asyncio import json from datetime import UTC, datetime, timedelta -from unittest.mock import AsyncMock, patch +from unittest.mock import AsyncMock, Mock, patch import pytest +from smithy_aws_core.identity.components import AWSCredentialsIdentity from smithy_aws_core.identity.process import ( ProcessCredentialsConfig, ProcessCredentialsResolver, ) +from smithy_core.aio.identity import ChainedIdentityResolver from smithy_core.exceptions import SmithyIdentityError ISO8601 = "%Y-%m-%dT%H:%M:%SZ" @@ -35,15 +37,20 @@ def test_config_custom_values(): def test_resolver_empty_command(): - with pytest.raises(ValueError, match="command must be a non-empty list"): + with pytest.raises(ValueError, match="command must be a non-empty string or list"): ProcessCredentialsResolver([]) def test_resolver_none_command(): - with pytest.raises(ValueError, match="command must be a non-empty list"): + with pytest.raises(ValueError, match="command must be a non-empty string or list"): ProcessCredentialsResolver(None) # type: ignore[arg-type] +def test_resolver_empty_command_string(): + with pytest.raises(ValueError, match="command must be a non-empty string or list"): + ProcessCredentialsResolver("") + + def mock_subprocess(returncode: int, stdout: bytes, stderr: bytes = b""): """Helper to mock asyncio.create_subprocess_exec""" process = AsyncMock() @@ -206,12 +213,11 @@ async def test_invalid_json(): @pytest.mark.asyncio async def test_process_timeout(): - async def timeout_communicate(): - await asyncio.sleep(100) - return (b"", b"") - process = AsyncMock() - process.communicate = timeout_communicate + process.returncode = None + process.communicate = AsyncMock(side_effect=TimeoutError) + process.kill = Mock() + process.wait = AsyncMock() config = ProcessCredentialsConfig(timeout=1) @@ -220,6 +226,45 @@ async def timeout_communicate(): with pytest.raises(SmithyIdentityError, match="timed out after 1 seconds"): await resolver.get_identity(properties={}) + process.kill.assert_called_once_with() + process.wait.assert_awaited_once_with() + + +@pytest.mark.asyncio +async def test_process_startup_failure_raises_smithy_identity_error(): + with patch( + "asyncio.create_subprocess_exec", + side_effect=FileNotFoundError("No such file or directory"), + ): + resolver = ProcessCredentialsResolver(["missing-process"]) + with pytest.raises(SmithyIdentityError, match="failed to start"): + await resolver.get_identity(properties={}) + + +@pytest.mark.asyncio +async def test_process_startup_failure_allows_chained_fallback(): + class SuccessfulResolver: + async def get_identity(self, *, properties: dict[str, str]): + return AWSCredentialsIdentity( + access_key_id="fallback-akid", + secret_access_key="fallback-secret", + ) + + with patch( + "asyncio.create_subprocess_exec", + side_effect=FileNotFoundError("No such file or directory"), + ): + resolver = ChainedIdentityResolver( + [ + ProcessCredentialsResolver(["missing-process"]), + SuccessfulResolver(), + ] + ) + identity = await resolver.get_identity(properties={}) + + assert identity.access_key_id == "fallback-akid" + assert identity.secret_access_key == "fallback-secret" + @pytest.mark.asyncio async def test_long_term_credentials_cached(): @@ -305,3 +350,25 @@ async def test_command_with_multiple_args(): stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, ) + + +@pytest.mark.asyncio +async def test_string_command_with_multiple_args(): + resp_body = json.dumps(DEFAULT_RESPONSE_DATA) + process = mock_subprocess(0, resp_body.encode("utf-8")) + + with patch("asyncio.create_subprocess_exec", return_value=process) as mock_exec: + resolver = ProcessCredentialsResolver( + 'aws-credential-helper --profile "test profile" --format json' + ) + await resolver.get_identity(properties={}) + + mock_exec.assert_called_once_with( + "aws-credential-helper", + "--profile", + "test profile", + "--format", + "json", + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + ) From 287f3ae501b9a24a759756687ed21a82503233da Mon Sep 17 00:00:00 2001 From: jonathan343 Date: Mon, 16 Mar 2026 10:28:26 -0400 Subject: [PATCH 3/6] fix type checking errors --- .../smithy-aws-core/tests/unit/identity/test_process.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/smithy-aws-core/tests/unit/identity/test_process.py b/packages/smithy-aws-core/tests/unit/identity/test_process.py index 91eb6ef5b..1d4c8a1ea 100644 --- a/packages/smithy-aws-core/tests/unit/identity/test_process.py +++ b/packages/smithy-aws-core/tests/unit/identity/test_process.py @@ -8,7 +8,10 @@ from unittest.mock import AsyncMock, Mock, patch import pytest -from smithy_aws_core.identity.components import AWSCredentialsIdentity +from smithy_aws_core.identity.components import ( + AWSCredentialsIdentity, + AWSIdentityProperties, +) from smithy_aws_core.identity.process import ( ProcessCredentialsConfig, ProcessCredentialsResolver, @@ -244,7 +247,9 @@ async def test_process_startup_failure_raises_smithy_identity_error(): @pytest.mark.asyncio async def test_process_startup_failure_allows_chained_fallback(): class SuccessfulResolver: - async def get_identity(self, *, properties: dict[str, str]): + async def get_identity( + self, *, properties: AWSIdentityProperties + ) -> AWSCredentialsIdentity: return AWSCredentialsIdentity( access_key_id="fallback-akid", secret_access_key="fallback-secret", From 5d7f60733e04ee8c3abbec3738a28afa081364d3 Mon Sep 17 00:00:00 2001 From: jonathan343 Date: Mon, 16 Mar 2026 10:34:59 -0400 Subject: [PATCH 4/6] Simplify example creds names --- .../tests/unit/identity/test_process.py | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/smithy-aws-core/tests/unit/identity/test_process.py b/packages/smithy-aws-core/tests/unit/identity/test_process.py index 1d4c8a1ea..0e78e723c 100644 --- a/packages/smithy-aws-core/tests/unit/identity/test_process.py +++ b/packages/smithy-aws-core/tests/unit/identity/test_process.py @@ -23,9 +23,9 @@ DEFAULT_RESPONSE_DATA = { "Version": 1, - "AccessKeyId": "akid123", - "SecretAccessKey": "s3cr3t", - "SessionToken": "session_token", + "AccessKeyId": "foo", + "SecretAccessKey": "bar", + "SessionToken": "baz", } @@ -71,9 +71,9 @@ async def test_valid_credentials_with_session_token(): resolver = ProcessCredentialsResolver(["mock-process"]) identity = await resolver.get_identity(properties={}) - assert identity.access_key_id == "akid123" - assert identity.secret_access_key == "s3cr3t" - assert identity.session_token == "session_token" + assert identity.access_key_id == "foo" + assert identity.secret_access_key == "bar" + assert identity.session_token == "baz" assert identity.expiration is None assert identity.account_id is None @@ -82,8 +82,8 @@ async def test_valid_credentials_with_session_token(): async def test_valid_credentials_without_session_token(): resp_data = { "Version": 1, - "AccessKeyId": "akid456", - "SecretAccessKey": "s3cr3t456", + "AccessKeyId": "foo", + "SecretAccessKey": "bar", } resp_body = json.dumps(resp_data) process = mock_subprocess(0, resp_body.encode("utf-8")) @@ -92,8 +92,8 @@ async def test_valid_credentials_without_session_token(): resolver = ProcessCredentialsResolver(["mock-process"]) identity = await resolver.get_identity(properties={}) - assert identity.access_key_id == "akid456" - assert identity.secret_access_key == "s3cr3t456" + assert identity.access_key_id == "foo" + assert identity.secret_access_key == "bar" assert identity.session_token is None @@ -143,7 +143,7 @@ async def test_non_zero_exit_code(): async def test_missing_access_key_id(): resp_data = { "Version": 1, - "SecretAccessKey": "s3cr3t", + "SecretAccessKey": "bar", } resp_body = json.dumps(resp_data) process = mock_subprocess(0, resp_body.encode("utf-8")) @@ -161,7 +161,7 @@ async def test_missing_access_key_id(): async def test_missing_secret_access_key(): resp_data = { "Version": 1, - "AccessKeyId": "akid123", + "AccessKeyId": "foo", } resp_body = json.dumps(resp_data) process = mock_subprocess(0, resp_body.encode("utf-8")) @@ -192,8 +192,8 @@ async def test_invalid_version(): @pytest.mark.asyncio async def test_missing_version(): resp_data = { - "AccessKeyId": "akid123", - "SecretAccessKey": "s3cr3t", + "AccessKeyId": "foo", + "SecretAccessKey": "bar", } resp_body = json.dumps(resp_data) process = mock_subprocess(0, resp_body.encode("utf-8")) From 246f9579d325ced02813e6860abd1cdc017deadf Mon Sep 17 00:00:00 2001 From: jonathan343 Date: Mon, 16 Mar 2026 11:15:07 -0400 Subject: [PATCH 5/6] Fix process credentials timezone handling, JSON error wrapping --- .../src/smithy_aws_core/identity/process.py | 10 +- .../tests/unit/identity/test_process.py | 134 +++++++++++------- 2 files changed, 89 insertions(+), 55 deletions(-) diff --git a/packages/smithy-aws-core/src/smithy_aws_core/identity/process.py b/packages/smithy-aws-core/src/smithy_aws_core/identity/process.py index 2ace33e3a..54af45009 100644 --- a/packages/smithy-aws-core/src/smithy_aws_core/identity/process.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/identity/process.py @@ -82,7 +82,12 @@ async def get_identity( raise SmithyIdentityError( f"Credential process failed with non-zero exit code: {stderr.decode('utf-8')}" ) - creds = json.loads(stdout.decode("utf-8")) + try: + creds = json.loads(stdout.decode("utf-8")) + except json.JSONDecodeError as e: + raise SmithyIdentityError( + f"Failed to parse credential process output: {e}" + ) from e version = creds.get("Version") if version is None or version != 1: @@ -96,7 +101,8 @@ async def get_identity( account_id = creds.get("AccountId") if isinstance(expiration, str): - expiration = datetime.fromisoformat(expiration).replace(tzinfo=UTC) + dt = datetime.fromisoformat(expiration) + expiration = dt.astimezone(UTC) if dt.tzinfo else dt.replace(tzinfo=UTC) if access_key_id is None or secret_access_key is None: raise SmithyIdentityError( diff --git a/packages/smithy-aws-core/tests/unit/identity/test_process.py b/packages/smithy-aws-core/tests/unit/identity/test_process.py index 0e78e723c..ea000f14b 100644 --- a/packages/smithy-aws-core/tests/unit/identity/test_process.py +++ b/packages/smithy-aws-core/tests/unit/identity/test_process.py @@ -8,15 +8,10 @@ from unittest.mock import AsyncMock, Mock, patch import pytest -from smithy_aws_core.identity.components import ( - AWSCredentialsIdentity, - AWSIdentityProperties, -) from smithy_aws_core.identity.process import ( ProcessCredentialsConfig, ProcessCredentialsResolver, ) -from smithy_core.aio.identity import ChainedIdentityResolver from smithy_core.exceptions import SmithyIdentityError ISO8601 = "%Y-%m-%dT%H:%M:%SZ" @@ -39,19 +34,10 @@ def test_config_custom_values(): assert config.timeout == 60 -def test_resolver_empty_command(): - with pytest.raises(ValueError, match="command must be a non-empty string or list"): - ProcessCredentialsResolver([]) - - -def test_resolver_none_command(): - with pytest.raises(ValueError, match="command must be a non-empty string or list"): - ProcessCredentialsResolver(None) # type: ignore[arg-type] - - -def test_resolver_empty_command_string(): +@pytest.mark.parametrize("command", [[], "", None]) +def test_resolver_invalid_command(command: object): with pytest.raises(ValueError, match="command must be a non-empty string or list"): - ProcessCredentialsResolver("") + ProcessCredentialsResolver(command) # type: ignore[arg-type] def mock_subprocess(returncode: int, stdout: bytes, stderr: bytes = b""): @@ -97,6 +83,41 @@ async def test_valid_credentials_without_session_token(): assert identity.session_token is None +@pytest.mark.asyncio +async def test_missing_expiration(): + resp_body = json.dumps(DEFAULT_RESPONSE_DATA) + process = mock_subprocess(0, resp_body.encode("utf-8")) + + with patch("asyncio.create_subprocess_exec", return_value=process): + resolver = ProcessCredentialsResolver(["mock-process"]) + identity = await resolver.get_identity(properties={}) + + assert identity.access_key_id == "foo" + assert identity.secret_access_key == "bar" + assert identity.session_token == "baz" + assert identity.expiration is None + + +@pytest.mark.asyncio +async def test_missing_expiration_and_session_token(): + resp_data = { + "Version": 1, + "AccessKeyId": "foo", + "SecretAccessKey": "bar", + } + resp_body = json.dumps(resp_data) + process = mock_subprocess(0, resp_body.encode("utf-8")) + + with patch("asyncio.create_subprocess_exec", return_value=process): + resolver = ProcessCredentialsResolver(["mock-process"]) + identity = await resolver.get_identity(properties={}) + + assert identity.access_key_id == "foo" + assert identity.secret_access_key == "bar" + assert identity.session_token is None + assert identity.expiration is None + + @pytest.mark.asyncio async def test_credentials_with_expiration(): current_time = datetime.now(UTC) + timedelta(minutes=10) @@ -114,6 +135,25 @@ async def test_credentials_with_expiration(): assert identity.expiration.tzinfo == UTC +@pytest.mark.asyncio +async def test_credentials_with_non_utc_expiration(): + """Test that non-UTC expiration timestamps are correctly converted to UTC.""" + # 2026-03-16T10:00:00+05:00 should become 2026-03-16T05:00:00 UTC + resp_data = dict(DEFAULT_RESPONSE_DATA) + resp_data["Expiration"] = "2026-03-16T10:00:00+05:00" + + resp_body = json.dumps(resp_data) + process = mock_subprocess(0, resp_body.encode("utf-8")) + + with patch("asyncio.create_subprocess_exec", return_value=process): + resolver = ProcessCredentialsResolver(["mock-process"]) + identity = await resolver.get_identity(properties={}) + + assert identity.expiration is not None + assert identity.expiration.tzinfo == UTC + assert identity.expiration == datetime(2026, 3, 16, 5, 0, 0, tzinfo=UTC) + + @pytest.mark.asyncio async def test_credentials_with_account_id(): resp_data = dict(DEFAULT_RESPONSE_DATA) @@ -210,7 +250,7 @@ async def test_invalid_json(): with patch("asyncio.create_subprocess_exec", return_value=process): resolver = ProcessCredentialsResolver(["mock-process"]) - with pytest.raises(json.JSONDecodeError): + with pytest.raises(SmithyIdentityError, match="Failed to parse"): await resolver.get_identity(properties={}) @@ -244,33 +284,6 @@ async def test_process_startup_failure_raises_smithy_identity_error(): await resolver.get_identity(properties={}) -@pytest.mark.asyncio -async def test_process_startup_failure_allows_chained_fallback(): - class SuccessfulResolver: - async def get_identity( - self, *, properties: AWSIdentityProperties - ) -> AWSCredentialsIdentity: - return AWSCredentialsIdentity( - access_key_id="fallback-akid", - secret_access_key="fallback-secret", - ) - - with patch( - "asyncio.create_subprocess_exec", - side_effect=FileNotFoundError("No such file or directory"), - ): - resolver = ChainedIdentityResolver( - [ - ProcessCredentialsResolver(["missing-process"]), - SuccessfulResolver(), - ] - ) - identity = await resolver.get_identity(properties={}) - - assert identity.access_key_id == "fallback-akid" - assert identity.secret_access_key == "fallback-secret" - - @pytest.mark.asyncio async def test_long_term_credentials_cached(): """Test that credentials without expiration are cached indefinitely.""" @@ -313,13 +326,25 @@ async def test_temporary_credentials_cached_when_valid(): async def test_expired_credentials_refreshed(): """Test that expired credentials are refreshed.""" expired_time = datetime.now(UTC) - timedelta(minutes=10) - resp_data = dict(DEFAULT_RESPONSE_DATA) - resp_data["Expiration"] = expired_time.strftime(ISO8601) + initial_data = dict(DEFAULT_RESPONSE_DATA) + initial_data["Expiration"] = expired_time.strftime(ISO8601) - resp_body = json.dumps(resp_data) - process = mock_subprocess(0, resp_body.encode("utf-8")) + refreshed_time = datetime.now(UTC) + timedelta(minutes=10) + refreshed_data = { + "Version": 1, + "AccessKeyId": "foo-refreshed", + "SecretAccessKey": "bar-refreshed", + "SessionToken": "baz-refreshed", + "Expiration": refreshed_time.strftime(ISO8601), + } - with patch("asyncio.create_subprocess_exec", return_value=process) as mock_exec: + first_process = mock_subprocess(0, json.dumps(initial_data).encode("utf-8")) + second_process = mock_subprocess(0, json.dumps(refreshed_data).encode("utf-8")) + + with patch( + "asyncio.create_subprocess_exec", + side_effect=[first_process, second_process], + ) as mock_exec: resolver = ProcessCredentialsResolver(["mock-process"]) identity_one = await resolver.get_identity(properties={}) identity_two = await resolver.get_identity(properties={}) @@ -328,9 +353,12 @@ async def test_expired_credentials_refreshed(): assert mock_exec.call_count == 2 # Should be different instances assert identity_one is not identity_two - # But have the same values - assert identity_one.access_key_id == identity_two.access_key_id - assert identity_one.secret_access_key == identity_two.secret_access_key + assert identity_one.access_key_id == "foo" + assert identity_one.secret_access_key == "bar" + assert identity_one.session_token == "baz" + assert identity_two.access_key_id == "foo-refreshed" + assert identity_two.secret_access_key == "bar-refreshed" + assert identity_two.session_token == "baz-refreshed" @pytest.mark.asyncio From e341b1c203d9e17f47864c66746ec14eba3eae9f Mon Sep 17 00:00:00 2001 From: jonathan343 Date: Mon, 16 Mar 2026 12:44:24 -0400 Subject: [PATCH 6/6] Only allow commands as a list of strings --- .../src/smithy_aws_core/identity/process.py | 12 +++------ .../tests/unit/identity/test_process.py | 26 ++----------------- 2 files changed, 6 insertions(+), 32 deletions(-) diff --git a/packages/smithy-aws-core/src/smithy_aws_core/identity/process.py b/packages/smithy-aws-core/src/smithy_aws_core/identity/process.py index 54af45009..de22afe55 100644 --- a/packages/smithy-aws-core/src/smithy_aws_core/identity/process.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/identity/process.py @@ -2,7 +2,6 @@ # SPDX-License-Identifier: Apache-2.0 import asyncio import json -import shlex from dataclasses import dataclass from datetime import UTC, datetime @@ -31,15 +30,12 @@ class ProcessCredentialsResolver( def __init__( self, - command: str | list[str], + command: list[str], config: ProcessCredentialsConfig | None = None, ): - normalized_command = ( - shlex.split(command) if isinstance(command, str) else command - ) - if not normalized_command: - raise ValueError("command must be a non-empty string or list") - self._command = list(normalized_command) + if not command: + raise ValueError("command must be a non-empty list") + self._command = list(command) self._config = config or ProcessCredentialsConfig() self._credentials = None diff --git a/packages/smithy-aws-core/tests/unit/identity/test_process.py b/packages/smithy-aws-core/tests/unit/identity/test_process.py index ea000f14b..f2bc9ec84 100644 --- a/packages/smithy-aws-core/tests/unit/identity/test_process.py +++ b/packages/smithy-aws-core/tests/unit/identity/test_process.py @@ -34,9 +34,9 @@ def test_config_custom_values(): assert config.timeout == 60 -@pytest.mark.parametrize("command", [[], "", None]) +@pytest.mark.parametrize("command", [[], None]) def test_resolver_invalid_command(command: object): - with pytest.raises(ValueError, match="command must be a non-empty string or list"): + with pytest.raises((ValueError, TypeError)): ProcessCredentialsResolver(command) # type: ignore[arg-type] @@ -383,25 +383,3 @@ async def test_command_with_multiple_args(): stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, ) - - -@pytest.mark.asyncio -async def test_string_command_with_multiple_args(): - resp_body = json.dumps(DEFAULT_RESPONSE_DATA) - process = mock_subprocess(0, resp_body.encode("utf-8")) - - with patch("asyncio.create_subprocess_exec", return_value=process) as mock_exec: - resolver = ProcessCredentialsResolver( - 'aws-credential-helper --profile "test profile" --format json' - ) - await resolver.get_identity(properties={}) - - mock_exec.assert_called_once_with( - "aws-credential-helper", - "--profile", - "test profile", - "--format", - "json", - stdout=asyncio.subprocess.PIPE, - stderr=asyncio.subprocess.PIPE, - )