Skip to content

Commit 8267552

Browse files
Alex Wangwangyb-A
authored andcommitted
fix(plugin): isolate execution input from handler
Addresses both Codex review comments on #616. durable_execution() handed the same mutable object to the user handler and to on_invocation_start, so the aliasing ran both ways: a plugin mutating info.execution_input changed the handler's event and could alter execution behaviour, and a handler mutating its event changed what the frozen start info -- and the end info derived from it -- reported afterwards. Both directions were confirmed reproducible. PluginExecutor now deep-copies the input for the plugin view. The copy is eager because the handler starts immediately after the hook, and is skipped when no plugins are registered so non-plugin executions pay nothing. On copy failure it falls back to the shared reference rather than dropping the input. Adds unit tests for both mutation directions (verified to fail without the copy) and an e2e suite under tests/e2e/ per AGENTS.md, covering the input and serialized result through complete invocations, a suspend/replay pair where only the replay carries a result, and deep isolation from the handler. Refs #616
1 parent bc1480f commit 8267552

3 files changed

Lines changed: 361 additions & 1 deletion

File tree

packages/aws-durable-execution-sdk-python/src/aws_durable_execution_sdk_python/plugin.py

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from __future__ import annotations
22

33
import contextlib
4+
import copy
45
import datetime
56
import functools
67
import logging
@@ -378,10 +379,40 @@ def on_invocation_start(
378379
request_id=aws_request_id,
379380
is_first_invocation=is_first_invocation,
380381
execution_start_time=execution_start_time,
381-
execution_input=execution_input,
382+
execution_input=self._snapshot_execution_input(execution_input),
382383
)
383384
self.execute_plugins(self._invocation_status, sync=True)
384385

386+
def _snapshot_execution_input(self, execution_input: Any) -> Any:
387+
"""Deep-copy the execution input so the plugin view is isolated.
388+
389+
``durable_execution()`` hands the same mutable object to the user handler
390+
and to this hook. Without a copy the aliasing runs both ways: a plugin
391+
mutating ``info.execution_input`` would change the handler's event and so
392+
alter execution behaviour, and a handler mutating its event would change
393+
what this frozen info -- and the invocation-end info derived from it --
394+
reports afterwards.
395+
396+
The copy is eager rather than deferred: the handler starts running
397+
immediately after this hook, so a lazily-taken snapshot could already
398+
have observed the handler's mutations. It is skipped when no plugins are
399+
registered, so non-plugin executions pay nothing.
400+
401+
The snapshot is shared by all plugins for this invocation; plugins should
402+
still treat it as read-only with respect to each other.
403+
"""
404+
if not self._plugins or execution_input is None:
405+
return execution_input
406+
try:
407+
return copy.deepcopy(execution_input)
408+
except Exception:
409+
# A plugin-facing view must never break the execution. Fall back to
410+
# the shared reference rather than dropping the input entirely.
411+
logger.exception(
412+
"Failed to copy execution input for plugins; passing shared reference"
413+
)
414+
return execution_input
415+
385416
def on_invocation_end(
386417
self,
387418
output: "DurableExecutionInvocationOutput",
Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
1+
"""Integration tests for the plugin invocation payload surfaces.
2+
3+
Exercises `InvocationInfo.execution_input` / `InvocationEndInfo.execution_result`
4+
through complete `durable_execution()` invocations -- across the decorator, the
5+
plugin executor, and the invocation hooks -- including a suspend/replay pair
6+
where the suspending invocation has no result and the replay carries the
7+
terminal one, and the isolation guarantee between the plugin view and the user
8+
handler's event.
9+
"""
10+
11+
from __future__ import annotations
12+
13+
from typing import Any
14+
from unittest.mock import Mock, patch
15+
16+
from aws_durable_execution_sdk_python.config import Duration
17+
from aws_durable_execution_sdk_python.context import DurableContext
18+
from aws_durable_execution_sdk_python.execution import (
19+
InvocationStatus,
20+
durable_execution,
21+
)
22+
from aws_durable_execution_sdk_python.lambda_service import (
23+
CheckpointOutput,
24+
CheckpointUpdatedExecutionState,
25+
Operation,
26+
OperationStatus,
27+
OperationType,
28+
)
29+
from aws_durable_execution_sdk_python.plugin import DurableInstrumentationPlugin
30+
from tests.test_helpers import operation_id_sequence
31+
32+
33+
class _PayloadRecordingPlugin(DurableInstrumentationPlugin):
34+
"""Records the payload surfaces seen on each invocation hook."""
35+
36+
def __init__(self) -> None:
37+
self.starts: list[Any] = []
38+
self.ends: list[tuple[str, Any, str | None]] = []
39+
40+
def on_invocation_start(self, info) -> None:
41+
self.starts.append(info.execution_input)
42+
43+
def on_invocation_end(self, info) -> None:
44+
self.ends.append(
45+
(info.status.value, info.execution_input, info.execution_result)
46+
)
47+
48+
49+
def _lambda_context() -> Mock:
50+
ctx = Mock()
51+
ctx.aws_request_id = "test-request-id"
52+
ctx.client_context = None
53+
ctx.identity = None
54+
ctx._epoch_deadline_time_in_ms = 0 # noqa: SLF001
55+
ctx.invoked_function_arn = "test-arn"
56+
ctx.tenant_id = None
57+
return ctx
58+
59+
60+
def _event(input_payload: str, extra_operations: list[dict] | None = None) -> dict:
61+
"""Build an invocation event carrying the given execution input payload."""
62+
execution_operation = {
63+
"Id": "execution-1",
64+
"Type": "EXECUTION",
65+
"Status": "STARTED",
66+
"ExecutionDetails": {"InputPayload": input_payload},
67+
}
68+
return {
69+
"DurableExecutionArn": "test-arn/execution-1",
70+
"CheckpointToken": "test-token",
71+
"InitialExecutionState": {
72+
"Operations": [execution_operation, *(extra_operations or [])],
73+
"NextMarker": "",
74+
},
75+
"LocalRunner": True,
76+
}
77+
78+
79+
def _tracking_checkpoint(initial_operations: list[Operation] | None = None):
80+
"""Checkpoint mock that accumulates operations, as the service would.
81+
82+
A stub returning an empty execution state is not enough for suspending
83+
paths: after the WAIT START is checkpointed the SDK re-reads the operation
84+
from the returned state, so it must be present.
85+
"""
86+
operations: list[Operation] = list(initial_operations or [])
87+
88+
def mock_checkpoint(
89+
durable_execution_arn, # noqa: ARG001
90+
checkpoint_token, # noqa: ARG001
91+
updates,
92+
client_token="token", # noqa: S107, ARG001
93+
) -> CheckpointOutput:
94+
for update in updates:
95+
operations.append(
96+
Operation(
97+
operation_id=update.operation_id,
98+
operation_type=update.operation_type,
99+
status=OperationStatus.STARTED,
100+
parent_id=update.parent_id,
101+
)
102+
)
103+
return CheckpointOutput(
104+
checkpoint_token="new_token", # noqa: S106
105+
new_execution_state=CheckpointUpdatedExecutionState(
106+
operations=operations.copy()
107+
),
108+
)
109+
110+
return mock_checkpoint
111+
112+
113+
def test_plugin_sees_execution_input_and_result_end_to_end():
114+
"""A completing invocation surfaces the input on both hooks and the result."""
115+
plugin = _PayloadRecordingPlugin()
116+
117+
@durable_execution(plugins=[plugin])
118+
def my_handler(event: Any, context: DurableContext) -> dict: # noqa: ARG001
119+
return {"greeting": f"Hello, {event['name']}!"}
120+
121+
with patch(
122+
"aws_durable_execution_sdk_python.execution.LambdaClient"
123+
) as mock_client_class:
124+
mock_client = Mock()
125+
mock_client.checkpoint = _tracking_checkpoint()
126+
mock_client_class.initialize_client.return_value = mock_client
127+
128+
result = my_handler(_event('{"name": "World"}'), _lambda_context())
129+
130+
assert result["Status"] == InvocationStatus.SUCCEEDED.value
131+
132+
# Start hook: the deserialized input, not the raw payload string.
133+
assert plugin.starts == [{"name": "World"}]
134+
135+
# End hook: the same input, plus the serialized result.
136+
assert len(plugin.ends) == 1
137+
status, end_input, end_result = plugin.ends[0]
138+
assert status == InvocationStatus.SUCCEEDED.value
139+
assert end_input == {"name": "World"}
140+
assert end_result == '{"greeting": "Hello, World!"}'
141+
142+
143+
def test_plugin_payload_surfaces_on_suspending_invocation():
144+
"""A suspending invocation carries the input but no execution result."""
145+
plugin = _PayloadRecordingPlugin()
146+
147+
@durable_execution(plugins=[plugin])
148+
def my_handler(event: Any, context: DurableContext) -> str:
149+
context.wait(Duration.from_seconds(60))
150+
return f"done-{event['name']}"
151+
152+
with patch(
153+
"aws_durable_execution_sdk_python.execution.LambdaClient"
154+
) as mock_client_class:
155+
mock_client = Mock()
156+
mock_client.checkpoint = _tracking_checkpoint()
157+
mock_client_class.initialize_client.return_value = mock_client
158+
159+
result = my_handler(_event('{"name": "World"}'), _lambda_context())
160+
161+
assert result["Status"] == InvocationStatus.PENDING.value
162+
assert plugin.starts == [{"name": "World"}]
163+
164+
status, end_input, end_result = plugin.ends[0]
165+
assert status == InvocationStatus.PENDING.value
166+
# The input is still reported on a non-terminal invocation-end.
167+
assert end_input == {"name": "World"}
168+
# But a suspending invocation produced no execution result.
169+
assert end_result is None
170+
171+
172+
def test_plugin_payload_surfaces_on_replay_invocation():
173+
"""A replay past a completed wait carries the input and the terminal result."""
174+
plugin = _PayloadRecordingPlugin()
175+
176+
@durable_execution(plugins=[plugin])
177+
def my_handler(event: Any, context: DurableContext) -> str:
178+
context.wait(Duration.from_seconds(60))
179+
return f"done-{event['name']}"
180+
181+
# The wait completed externally while the execution was suspended.
182+
completed_wait = {
183+
"Id": next(operation_id_sequence()),
184+
"Type": OperationType.WAIT.value,
185+
"Status": OperationStatus.SUCCEEDED.value,
186+
}
187+
188+
with patch(
189+
"aws_durable_execution_sdk_python.execution.LambdaClient"
190+
) as mock_client_class:
191+
mock_client = Mock()
192+
mock_client.checkpoint = _tracking_checkpoint()
193+
mock_client_class.initialize_client.return_value = mock_client
194+
195+
result = my_handler(
196+
_event('{"name": "World"}', extra_operations=[completed_wait]),
197+
_lambda_context(),
198+
)
199+
200+
assert result["Status"] == InvocationStatus.SUCCEEDED.value
201+
# The input is carried identically across invocations of one execution.
202+
assert plugin.starts == [{"name": "World"}]
203+
204+
status, end_input, end_result = plugin.ends[0]
205+
assert status == InvocationStatus.SUCCEEDED.value
206+
assert end_input == {"name": "World"}
207+
assert end_result == '"done-World"'
208+
209+
210+
def test_plugin_execution_input_is_isolated_from_handler_end_to_end():
211+
"""The plugin's input view and the handler's event must not alias.
212+
213+
durable_execution() hands one mutable object to both, so the plugin view is
214+
deep-copied. Without that a plugin could alter execution behaviour, and a
215+
handler could retroactively change what the frozen hook info reports.
216+
"""
217+
218+
class _MutatingPlugin(DurableInstrumentationPlugin):
219+
def __init__(self) -> None:
220+
self.end_inputs: list[Any] = []
221+
222+
def on_invocation_start(self, info) -> None:
223+
info.execution_input["injected_by_plugin"] = True
224+
info.execution_input["nested"]["items"].append("from_plugin")
225+
226+
def on_invocation_end(self, info) -> None:
227+
self.end_inputs.append(info.execution_input)
228+
229+
plugin = _MutatingPlugin()
230+
handler_saw: dict[str, Any] = {}
231+
232+
@durable_execution(plugins=[plugin])
233+
def my_handler(event: Any, context: DurableContext) -> str: # noqa: ARG001
234+
handler_saw.update(
235+
{"top": dict(event), "nested_items": list(event["nested"]["items"])}
236+
)
237+
event["injected_by_handler"] = True
238+
return "ok"
239+
240+
with patch(
241+
"aws_durable_execution_sdk_python.execution.LambdaClient"
242+
) as mock_client_class:
243+
mock_client = Mock()
244+
mock_client.checkpoint = _tracking_checkpoint()
245+
mock_client_class.initialize_client.return_value = mock_client
246+
247+
my_handler(
248+
_event('{"name": "World", "nested": {"items": ["original"]}}'),
249+
_lambda_context(),
250+
)
251+
252+
# The plugin's mutations never reached the handler, at any depth.
253+
assert "injected_by_plugin" not in handler_saw["top"]
254+
assert handler_saw["nested_items"] == ["original"]
255+
# The handler's mutation never reached the end hook.
256+
assert "injected_by_handler" not in plugin.end_inputs[0]

packages/aws-durable-execution-sdk-python/tests/execution_test.py

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import json
55
import time
66
import warnings
7+
from copy import deepcopy
78
from typing import Any
89
from unittest.mock import Mock, patch
910

@@ -3022,6 +3023,78 @@ def test_handler(event: Any, context: DurableContext) -> str:
30223023
assert plugin.start_execution_inputs == [{}]
30233024

30243025

3026+
def test_durable_execution_isolates_execution_input_from_handler():
3027+
"""Plugin and handler must not observe each other's input mutations.
3028+
3029+
durable_execution() hands one mutable object to both, so the plugin view is
3030+
deep-copied. Without that, a plugin could alter execution behaviour and a
3031+
handler could retroactively change what the frozen hook info reports.
3032+
"""
3033+
mock_client = Mock(spec=DurableServiceClient)
3034+
mock_client.checkpoint.return_value = CheckpointOutput(
3035+
checkpoint_token="new_token", # noqa: S106
3036+
new_execution_state=CheckpointUpdatedExecutionState(),
3037+
)
3038+
3039+
observed: dict[str, Any] = {}
3040+
3041+
class _MutatingPlugin(DurableInstrumentationPlugin):
3042+
def on_invocation_start(self, info):
3043+
# Direction A: plugin mutates its view before the handler runs.
3044+
info.execution_input["injected_by_plugin"] = True
3045+
3046+
def on_invocation_end(self, info):
3047+
observed["end_input"] = dict(info.execution_input)
3048+
3049+
@durable_execution(plugins=[_MutatingPlugin()])
3050+
def test_handler(event: Any, context: DurableContext) -> dict:
3051+
observed["handler_saw"] = dict(event)
3052+
# Direction B: handler mutates its event after the start hook fired.
3053+
event["injected_by_handler"] = True
3054+
return {"ok": True}
3055+
3056+
test_handler(
3057+
_make_invocation_input(mock_client, input_payload='{"name": "World"}'),
3058+
_make_lambda_context(),
3059+
)
3060+
3061+
# Direction A: the plugin's mutation must not reach the handler.
3062+
assert observed["handler_saw"] == {"name": "World"}
3063+
# Direction B: the handler's mutation must not reach the end hook, which
3064+
# still reports the plugin-side snapshot taken at invocation-start.
3065+
assert "injected_by_handler" not in observed["end_input"]
3066+
assert observed["end_input"] == {"name": "World", "injected_by_plugin": True}
3067+
3068+
3069+
def test_durable_execution_isolates_nested_execution_input():
3070+
"""Isolation must be deep, not just a top-level copy."""
3071+
mock_client = Mock(spec=DurableServiceClient)
3072+
mock_client.checkpoint.return_value = CheckpointOutput(
3073+
checkpoint_token="new_token", # noqa: S106
3074+
new_execution_state=CheckpointUpdatedExecutionState(),
3075+
)
3076+
3077+
observed: dict[str, Any] = {}
3078+
3079+
class _NestedMutatingPlugin(DurableInstrumentationPlugin):
3080+
def on_invocation_start(self, info):
3081+
info.execution_input["outer"]["inner"].append("from_plugin")
3082+
3083+
@durable_execution(plugins=[_NestedMutatingPlugin()])
3084+
def test_handler(event: Any, context: DurableContext) -> dict:
3085+
observed["handler_saw"] = deepcopy(event)
3086+
return {"ok": True}
3087+
3088+
test_handler(
3089+
_make_invocation_input(
3090+
mock_client, input_payload='{"outer": {"inner": ["original"]}}'
3091+
),
3092+
_make_lambda_context(),
3093+
)
3094+
3095+
assert observed["handler_saw"] == {"outer": {"inner": ["original"]}}
3096+
3097+
30253098
def test_durable_execution_with_plugins_failure():
30263099
"""Test that plugins receive invocation end and execution end on user error."""
30273100
mock_client = Mock(spec=DurableServiceClient)

0 commit comments

Comments
 (0)