Add HistoryEventSerializationBinder to restrict deserialization#1345
Add HistoryEventSerializationBinder to restrict deserialization#1345
Conversation
There was a problem hiding this comment.
Pull request overview
Introduces a strict JSON serialization binder for the DurableTask Emulator to restrict what $type metadata can be deserialized when round-tripping orchestration runtime state (history events), addressing the linked security/work item.
Changes:
- Adds
HistoryEventSerializationBinder(allowlist-based) and wires it into emulator runtime state JSON settings. - Keeps
TypeNameHandling.Autofor polymorphic history-event serialization while attempting to restrict deserialization types. - Adds unit tests intended to validate allowlisted round-tripping and rejection of non-allowlisted root/nested
$typepayloads.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/DurableTask.Emulator/LocalOrchestrationService.cs |
Uses new strict binder in the emulator’s runtime-state JSON settings. |
src/DurableTask.Emulator/HistoryEventSerializationBinder.cs |
New binder implementation that allowlists DurableTask.Core types and Dictionary<string,string>. |
Test/DurableTask.Emulator.Tests/HistoryEventSerializationBinderTests.cs |
Adds tests for binder behavior (currently placed under legacy Test/ path). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static readonly JsonSerializerSettings StateJsonSettings = new JsonSerializerSettings { TypeNameHandling = TypeNameHandling.Auto }; | ||
| static readonly JsonSerializerSettings StateJsonSettings = new JsonSerializerSettings | ||
| { | ||
| TypeNameHandling = TypeNameHandling.Auto, |
There was a problem hiding this comment.
TypeNameHandling.Auto can cause Json.NET to emit a $type for the root runtime-state event collection (e.g., List<HistoryEvent> when serializing runtimeState.Events). With the new strict binder allowlist, that collection type may be rejected during deserialization, breaking emulator persistence/rehydration. Consider either (a) adjusting serialization to avoid writing a $type for the root collection (e.g., serialize/deserialize with an explicit declared type), or (b) expanding the allowlist to permit the specific collection types you actually emit (e.g., List<HistoryEvent>).
| TypeNameHandling = TypeNameHandling.Auto, | |
| TypeNameHandling = TypeNameHandling.Objects, |
| // ParentInstance, FailureDetails, etc.), plus Dictionary<string, string> for the | ||
| // IDictionary<string, string> Tags members declared on ExecutionStartedEvent / | ||
| // SubOrchestrationInstanceCreatedEvent / TaskScheduledEvent (Newtonsoft.Json emits a | ||
| // $type for these because the static declared type is an interface). | ||
| return type.Assembly == DurableTaskCoreAssembly | ||
| || type == typeof(Dictionary<string, string>); |
There was a problem hiding this comment.
The allowlist in IsAllowed doesn’t include the common collection types that Json.NET can emit $type metadata for when TypeNameHandling.Auto is enabled (notably List<HistoryEvent> / IList<HistoryEvent> root values can be serialized with $type + $values). This will cause legitimate emulator state round-trips to fail with JsonSerializationException. Expand the allowlist to include the required collection types (ideally in a constrained way, e.g., only List<>/Dictionary<,> where the generic arguments are themselves allowed), or change the serializer usage to avoid emitting $type for the root collection.
| // ParentInstance, FailureDetails, etc.), plus Dictionary<string, string> for the | |
| // IDictionary<string, string> Tags members declared on ExecutionStartedEvent / | |
| // SubOrchestrationInstanceCreatedEvent / TaskScheduledEvent (Newtonsoft.Json emits a | |
| // $type for these because the static declared type is an interface). | |
| return type.Assembly == DurableTaskCoreAssembly | |
| || type == typeof(Dictionary<string, string>); | |
| // ParentInstance, FailureDetails, etc.), plus the constrained collection shapes that | |
| // Json.NET may emit as $type metadata when TypeNameHandling.Auto is enabled: | |
| // * List<T> where T is itself allowed (for IList<HistoryEvent> runtime state) | |
| // * Dictionary<TKey, TValue> where both generic arguments are allowed | |
| // String is permitted as a terminal type so Dictionary<string, string> continues to | |
| // round-trip for the IDictionary<string, string> Tags members declared on | |
| // ExecutionStartedEvent / SubOrchestrationInstanceCreatedEvent / TaskScheduledEvent. | |
| if (type.Assembly == DurableTaskCoreAssembly || type == typeof(string)) | |
| { | |
| return true; | |
| } | |
| if (type.IsGenericType) | |
| { | |
| Type genericTypeDefinition = type.GetGenericTypeDefinition(); | |
| Type[] genericArguments = type.GetGenericArguments(); | |
| if (genericTypeDefinition == typeof(List<>)) | |
| { | |
| return genericArguments.Length == 1 && IsAllowed(genericArguments[0]); | |
| } | |
| if (genericTypeDefinition == typeof(Dictionary<,>)) | |
| { | |
| return genericArguments.Length == 2 | |
| && IsAllowed(genericArguments[0]) | |
| && IsAllowed(genericArguments[1]); | |
| } | |
| } | |
| return false; |
| [TestClass] | ||
| public class HistoryEventSerializationBinderTests | ||
| { | ||
| // Mirrors the production state settings used by LocalOrchestrationService. | ||
| static readonly JsonSerializerSettings StateJsonSettings = new JsonSerializerSettings | ||
| { | ||
| TypeNameHandling = TypeNameHandling.Auto, | ||
| SerializationBinder = new HistoryEventSerializationBinder() | ||
| }; |
There was a problem hiding this comment.
This test file is under the legacy Test/ directory, but the solution and active test projects reference test/DurableTask.Emulator.Tests (lowercase). As a result, these tests likely won’t be compiled or executed in CI. Move the test into test/DurableTask.Emulator.Tests/ (or ensure the referenced test/DurableTask.Emulator.Tests.csproj includes it).
Resolves https://msazure.visualstudio.com/Antares/_workitems/edit/37181655