fix(events): avoid mutating event validation input#6314
Open
VectorPeak wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
2. Or, if no issue exists, describe the change:
Problem:
Event._accept_convenience_kwargs()routes convenience inputs such asmessage,state,route, andnode_pathinto the model's canonical nested fields. It currently does that by callingpop()directly on thedatamapping received by themode="before"model validator.The before validator receives a
dictin both common entry paths:Event(message="Hello!", state={"key": "value"})receives a temporary dict assembled for model construction, so consuming convenience keys there stays internal.Event.model_validate(payload)receives the caller-provided dict itself. In that path, the validator mutates the original payload by replacing the convenience keys withcontent,actions, andnode_info.That second path is the bug: the issue is not just that a dict can theoretically reach the validator, but that the public
model_validate(dict)API can hand the validator the same mapping object still owned by the caller.A minimal reproduction before this fix:
Before the fix,
payloadis rewritten in place. That can surprise callers that reuse the same mapping for logging, retries, comparisons, or subsequent validation.Solution:
Copy the incoming mapping before consuming convenience keys. This keeps the existing convenience-key routing behavior unchanged while ensuring
Event.model_validate()does not mutate caller-owned input.The change is intentionally narrow:
messagestill routes tocontentstatestill routes toactions.state_deltaroutestill routes toactions.routenode_pathstill routes tonode_info.pathTesting Plan
Unit Tests:
Passed targeted unit tests locally:
Additional local checks:
Manual End-to-End (E2E) Tests:
Not applicable for this validator-level change. The regression is covered by a focused unit test that exercises the public
Event.model_validate()path and verifies both behaviors: the caller-provided payload remains unchanged, and the resultingEventstill receivescontent,actions.state_delta,actions.route, andnode_info.path.Checklist
Additional context
This PR does not remove the existing
pop()-based convenience-key routing. It only makes the validator operate on a local copy, so the canonical event output remains unchanged while avoiding a top-level mutation of caller-provided dictionaries.