From cdffa33c60300bcd9683dc3d897a5b9007a4e290 Mon Sep 17 00:00:00 2001 From: Alex Wang Date: Tue, 11 Aug 2026 19:43:09 +0000 Subject: [PATCH 1/2] fix(plugin): keep payloads out of hook info repr Addresses the high-severity Codex review comment on #616. Dataclass fields land in the generated repr, and instrumentation logs hook infos wholesale: the bundled OTel plugins at debug level, and the plugin example at info. Customer input and results -- potentially secrets, potentially megabytes -- would therefore be written to logs implicitly, just by adding these fields. Reproduced before fixing. Set repr=False on execution_input and execution_result. Identity fields still render, so the repr stays useful, and the values remain readable through the attributes for plugins that deliberately record them. Adds a regression test asserting secret-looking values never appear in either hook info's repr, plus one pinning the field declarations. Refs #616 --- .../plugin.py | 12 +++++ .../tests/plugin_test.py | 46 +++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/packages/aws-durable-execution-sdk-python/src/aws_durable_execution_sdk_python/plugin.py b/packages/aws-durable-execution-sdk-python/src/aws_durable_execution_sdk_python/plugin.py index 96bfb373..379862eb 100644 --- a/packages/aws-durable-execution-sdk-python/src/aws_durable_execution_sdk_python/plugin.py +++ b/packages/aws-durable-execution-sdk-python/src/aws_durable_execution_sdk_python/plugin.py @@ -179,6 +179,7 @@ class InvocationInfo: execution_input: Any = field( default=None, kw_only=True, + repr=False, metadata={"experimental": True}, ) """EXPERIMENTAL: The deserialized execution input, when available. @@ -186,6 +187,12 @@ class InvocationInfo: Surfaced to instrumentation plugins that need to record it (e.g. Workflow Insight). Mirrors the JS SDK's ``InvocationInfo.executionInput``. + Excluded from ``repr`` on purpose: instrumentation logs hook infos wholesale + (the bundled OTel plugins at debug level, the plugin example at info), so + including the payload here would implicitly write customer input -- possibly + secrets, possibly megabytes -- into logs. Read the attribute explicitly to + record it. + Defaults to ``None`` only when the field is not populated (a hook info built without it); ``durable_execution()`` always populates it with the deserialized input payload, which is ``{}`` when the payload is empty. @@ -208,6 +215,7 @@ class InvocationEndInfo(InvocationInfo): execution_result: str | None = field( default=None, kw_only=True, + repr=False, metadata={"experimental": True}, ) """EXPERIMENTAL: The serialized execution result, when available. @@ -215,6 +223,10 @@ class InvocationEndInfo(InvocationInfo): A JSON string, or ``""`` when the result was checkpointed out-of-band for a large payload. Mirrors the JS SDK's ``InvocationEndInfo.executionResult``. ``None`` on failure or suspend. + + Excluded from ``repr`` for the same reason as + :attr:`InvocationInfo.execution_input`: hook infos are logged wholesale by + instrumentation, and the result is customer data. """ @classmethod diff --git a/packages/aws-durable-execution-sdk-python/tests/plugin_test.py b/packages/aws-durable-execution-sdk-python/tests/plugin_test.py index 556dac0a..2cd65791 100644 --- a/packages/aws-durable-execution-sdk-python/tests/plugin_test.py +++ b/packages/aws-durable-execution-sdk-python/tests/plugin_test.py @@ -115,6 +115,52 @@ class TestDataClasses(unittest.TestCase): + def test_payload_fields_are_excluded_from_repr(self): + """Payload values must not leak into repr. + + Instrumentation logs hook infos wholesale -- the bundled OTel plugins at + debug level, the plugin example at info -- so a payload in repr would + implicitly write customer input and results into logs. + """ + secret_input = {"password": "hunter2", "ssn": "123-45-6789"} + secret_result = '{"token": "sk-live-do-not-log"}' + + start_info = InvocationStartInfo( + request_id="req-1", + execution_arn="arn:test", + is_first_invocation=True, + execution_input=secret_input, + ) + end_info = InvocationEndInfo( + request_id="req-1", + execution_arn="arn:test", + is_first_invocation=False, + status=InvocationStatus.SUCCEEDED, + execution_input=secret_input, + execution_result=secret_result, + ) + + for info in (start_info, end_info): + rendered = repr(info) + for leaked in ("hunter2", "123-45-6789", "sk-live-do-not-log"): + self.assertNotIn(leaked, rendered, f"{type(info).__name__}: {leaked}") + # Identity fields are still rendered, so the repr stays useful. + self.assertIn("req-1", rendered) + self.assertIn("arn:test", rendered) + + # The values remain readable via the attributes themselves. + self.assertEqual(secret_input, start_info.execution_input) + self.assertEqual(secret_result, end_info.execution_result) + + def test_payload_fields_are_declared_non_repr(self): + """Pin the field declaration, not just the rendered string.""" + start_fields = {f.name: f for f in fields(InvocationStartInfo)} + end_fields = {f.name: f for f in fields(InvocationEndInfo)} + + self.assertFalse(start_fields["execution_input"].repr) + self.assertFalse(end_fields["execution_input"].repr) + self.assertFalse(end_fields["execution_result"].repr) + def test_payload_fields_are_marked_experimental(self): plugin_info_types = ( OperationInfo, From 2edac5ab694c65e2c494bb0ca2c2019842287125 Mon Sep 17 00:00:00 2001 From: Alex Wang Date: Tue, 11 Aug 2026 20:50:35 +0000 Subject: [PATCH 2/2] fix(plugin): exclude payloads from info equality Addresses the medium-severity Codex review comment on #616. The payload fields joined the generated __eq__ and __hash__, so the widening was not additive. execution_input holds arbitrary deserialized JSON, and a dict or list value made a previously hashable InvocationStartInfo raise TypeError on hash(); both fields also made infos built from the earlier field set compare unequal to infos carrying a payload. Both effects were reproduced first. Set compare=False, hash=False on execution_input and execution_result. Identity fields still drive equality, so payload-only differences now compare equal -- payloads are incidental data on what is otherwise an event record. Adds tests for hashability across dict, list, nested and scalar payloads, for equality against the prior field set, and for the field declarations themselves. Note OperationInfo.result / OperationInfo.error from #625 remain in compare. They are hashable types so they do not break hash(), but the equality asymmetry with these fields is worth a maintainer decision. Refs #616 --- .../plugin.py | 14 +++- .../tests/plugin_test.py | 81 +++++++++++++++++++ 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/packages/aws-durable-execution-sdk-python/src/aws_durable_execution_sdk_python/plugin.py b/packages/aws-durable-execution-sdk-python/src/aws_durable_execution_sdk_python/plugin.py index 379862eb..7ccb8d50 100644 --- a/packages/aws-durable-execution-sdk-python/src/aws_durable_execution_sdk_python/plugin.py +++ b/packages/aws-durable-execution-sdk-python/src/aws_durable_execution_sdk_python/plugin.py @@ -180,6 +180,8 @@ class InvocationInfo: default=None, kw_only=True, repr=False, + compare=False, + hash=False, metadata={"experimental": True}, ) """EXPERIMENTAL: The deserialized execution input, when available. @@ -193,6 +195,11 @@ class InvocationInfo: secrets, possibly megabytes -- into logs. Read the attribute explicitly to record it. + Excluded from ``__eq__`` and ``__hash__`` so adding it stays additive. The + value is arbitrary deserialized JSON, so a dict or list payload would make a + previously hashable info unhashable, and comparisons against infos built + from the earlier field set would start returning False. + Defaults to ``None`` only when the field is not populated (a hook info built without it); ``durable_execution()`` always populates it with the deserialized input payload, which is ``{}`` when the payload is empty. @@ -216,6 +223,8 @@ class InvocationEndInfo(InvocationInfo): default=None, kw_only=True, repr=False, + compare=False, + hash=False, metadata={"experimental": True}, ) """EXPERIMENTAL: The serialized execution result, when available. @@ -224,9 +233,10 @@ class InvocationEndInfo(InvocationInfo): large payload. Mirrors the JS SDK's ``InvocationEndInfo.executionResult``. ``None`` on failure or suspend. - Excluded from ``repr`` for the same reason as + Excluded from ``repr``, ``__eq__`` and ``__hash__`` for the same reasons as :attr:`InvocationInfo.execution_input`: hook infos are logged wholesale by - instrumentation, and the result is customer data. + instrumentation, and adding the field should not change how existing infos + compare. """ @classmethod diff --git a/packages/aws-durable-execution-sdk-python/tests/plugin_test.py b/packages/aws-durable-execution-sdk-python/tests/plugin_test.py index 2cd65791..89de9c68 100644 --- a/packages/aws-durable-execution-sdk-python/tests/plugin_test.py +++ b/packages/aws-durable-execution-sdk-python/tests/plugin_test.py @@ -161,6 +161,87 @@ def test_payload_fields_are_declared_non_repr(self): self.assertFalse(end_fields["execution_input"].repr) self.assertFalse(end_fields["execution_result"].repr) + def test_payload_fields_do_not_break_hashability(self): + """A payload must not make a previously hashable info unhashable. + + ``execution_input`` holds arbitrary deserialized JSON, so a dict or list + value would otherwise propagate into the generated ``__hash__`` and + raise ``TypeError``. + """ + base = { + "request_id": "req-1", + "execution_arn": "arn:test", + "is_first_invocation": True, + } + + for payload in ({"k": "v"}, ["a", "b"], {"nested": {"deep": [1, 2]}}, "plain"): + info = InvocationStartInfo(**base, execution_input=payload) + # Must not raise, and must match the payload-free hash. + self.assertEqual(hash(InvocationStartInfo(**base)), hash(info)) + + end_info = InvocationEndInfo( + **base, + status=InvocationStatus.SUCCEEDED, + execution_input={"k": "v"}, + execution_result='{"big": "payload"}', + ) + self.assertEqual( + hash(InvocationEndInfo(**base, status=InvocationStatus.SUCCEEDED)), + hash(end_info), + ) + + def test_payload_fields_are_excluded_from_equality(self): + """Adding the payload fields must not change how infos compare. + + Infos built from the earlier field set still compare equal to infos + carrying a payload, so this widening stays additive for callers. + """ + base = { + "request_id": "req-1", + "execution_arn": "arn:test", + "is_first_invocation": True, + } + + self.assertEqual( + InvocationStartInfo(**base), + InvocationStartInfo(**base, execution_input={"k": "v"}), + ) + # Two different payloads also compare equal -- payloads are incidental + # data, not part of the info's identity. + self.assertEqual( + InvocationStartInfo(**base, execution_input={"a": 1}), + InvocationStartInfo(**base, execution_input={"b": 2}), + ) + self.assertEqual( + InvocationEndInfo(**base, status=InvocationStatus.SUCCEEDED), + InvocationEndInfo( + **base, + status=InvocationStatus.SUCCEEDED, + execution_input={"k": "v"}, + execution_result='"result"', + ), + ) + # Identity fields still drive inequality. + self.assertNotEqual( + InvocationStartInfo(**base, execution_input={"k": "v"}), + InvocationStartInfo( + **{**base, "request_id": "req-2"}, execution_input={"k": "v"} + ), + ) + + def test_payload_fields_are_declared_non_compare(self): + """Pin the declarations, not just the observed behaviour.""" + start_fields = {f.name: f for f in fields(InvocationStartInfo)} + end_fields = {f.name: f for f in fields(InvocationEndInfo)} + + for holder, name in ( + (start_fields, "execution_input"), + (end_fields, "execution_input"), + (end_fields, "execution_result"), + ): + self.assertFalse(holder[name].compare, name) + self.assertIs(holder[name].hash, False, name) + def test_payload_fields_are_marked_experimental(self): plugin_info_types = ( OperationInfo,