Enforce safe JSON deserialization in ServiceFabric - #1386
Open
Naiyuan Tian (nytian) wants to merge 1 commit into
Open
Enforce safe JSON deserialization in ServiceFabric#1386Naiyuan Tian (nytian) wants to merge 1 commit into
Naiyuan Tian (nytian) wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the Azure Service Fabric proxy’s JSON deserialization by ensuring a serialization binder is always applied when TypeNameHandling.All is enabled, preventing untrusted requests from deserializing arbitrary .NET types via $type metadata.
Changes:
- Default
Startupto anAllowedTypesSerializationBinderwhen no binder is provided, and always assign a binder to the JSON formatter settings. - Change
FabricOrchestrationProviderSettings.JsonSerializationBindersonullrestores the default allowed-types binder instead of disabling restrictions. - Update the corresponding binder-setting unit test expectation (but currently in a non-build/test project path).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Test/DurableTask.AzureServiceFabric.Tests/AllowedTypesSerializationBinderTests.cs | Updates test to expect null binder assignment to restore the allowed-types binder (currently under Test/, not the active test/ project). |
| src/DurableTask.AzureServiceFabric/Service/Startup.cs | Ensures a non-null binder is always applied to JsonFormatter.SerializerSettings.SerializationBinder. |
| src/DurableTask.AzureServiceFabric/FabricOrchestrationProviderSettings.cs | Makes JsonSerializationBinder non-null by default and treats null as “reset to default binder”. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
225
to
+230
| [TestMethod] | ||
| public void Settings_BinderCanBeSetToNull() | ||
| public void Settings_NullBinderRestoresAllowedTypesBinder() | ||
| { | ||
| var providerSettings = new FabricOrchestrationProviderSettings(); | ||
| providerSettings.JsonSerializationBinder = null; | ||
| Assert.IsNull(providerSettings.JsonSerializationBinder); | ||
| Assert.IsInstanceOfType(providerSettings.JsonSerializationBinder, typeof(AllowedTypesSerializationBinder)); |
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.
Ensure the Service Fabric JSON formatter always uses the allowed-types binder. This prevents untrusted requests from deserializing arbitrary .NET types through type metadata. Tests are added.