Fix process credentials timezone handling, JSON error wrapping

Author: jonathan343

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(

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