Skip to content

Allow configuring JsonSerializerSettings for Entity Framework persistence providers - #1450

Merged
danielgerlag merged 3 commits into
danielgerlag:masterfrom
thrisikos-exida:ef-serializer-settings
Oct 22, 2026
Merged

danielgerlag merged 3 commits into
danielgerlag:masterfrom
thrisikos-exida:ef-serializer-settings

Conversation

@thrisikos-exida

Copy link
Copy Markdown
Contributor

Describe the change
The Entity Framework persistence providers serialize workflow data, persistence data, outcomes, event data, subscription data and extension attributes with a private, static JsonSerializerSettings using TypeNameHandling.All.
Because there is no way to customize those settings, consumers cannot install a SerializationBinder to restrict which types may be instantiated when a row is read back.

This adds an opt-in constructor overload to EntityFrameworkPersistenceProvider and LargeDataOptimizedEntityFrameworkPersistenceProvider that accepts an Action<JsonSerializerSettings>, applied on top of the existing defaults.
This is defense in depth: exploiting it requires write access to the persistence store, but with a binder configured, a tampered $type is rejected with a JsonSerializationException instead of instantiating an arbitrary type.

Closes #1178
Related: #1280

Describe your implementation or design

  • ExtensionMethods stays internal; the static field is replaced by an internal CreateSerializerSettings(Action<JsonSerializerSettings>) factory, and the mapping methods take the settings instance as a parameter.
  • Each provider builds its settings once in the constructor; the existing constructors chain to the new ones with null, so default behavior and the stored JSON format are unchanged.
  • New overloads rather than optional parameters, so existing compiled callers are not broken.
  • No new public types; the only new public surface is one constructor overload on each provider.
  • Consumers wire it up with UsePersistence after the usual UseSqlite/UseSqlServer/etc. call, which keeps the purger registration.
    Convenience overloads on the Use* extension methods could follow in a separate PR if you prefer that shape.
  • Added a "Restricting deserialized types" section to docs/persistence.md with an allow-list binder example.

Tests
Added SerializerSettingsFixture in WorkflowCore.Tests.Sqlite:

  • default settings produce JSON identical to the previous settings;
  • with a binder, an allowed type round-trips through the provider;
  • with a binder, a row whose $type was overwritten with a disallowed type throws JsonSerializationException;
  • without configuration, the same overwritten row still loads (unchanged behavior).

Updated ExtensionMethodsFixture for the new parameter.
WorkflowCore.Tests.Sqlite (14/14) and WorkflowCore.IntegrationTests (45/45) pass on all target frameworks.
WorkflowCore.UnitTests has one failure, Should run pre-middlewares for sync workflows, which also fails on master without this change.
Provider test suites that need Docker were not run.

Breaking change
No. Existing constructors and the default serializer settings are unchanged; existing persisted data reads exactly as before.

Additional context

  • Other providers (MongoDB, Redis, Cosmos DB / Azure Table Storage, DynamoDB) have similar hard-coded TypeNameHandling settings; they could adopt the same pattern in follow-up PRs.
  • Separate observation, not addressed here: when a workflow instance fails to deserialize (e.g. rejected by a binder), the engine keeps re-polling it and logs an error every poll interval; a poisoned instance is never quarantined.
    Happy to open an issue for that if useful.

AI-assisted: Claude Opus 5.5 (claude-opus-5-5)

…ence providers

- Add constructor overloads to `EntityFrameworkPersistenceProvider` and `LargeDataOptimizedEntityFrameworkPersistenceProvider` accepting an `Action<JsonSerializerSettings>`, e.g. to set a `SerializationBinder`.
- Replace the static settings in `ExtensionMethods` with a per-provider instance; defaults are unchanged.
- Add `SerializerSettingsFixture` covering default output, allowed round-trip, rejected type and unchanged default behavior.
- Document restricting deserialized types in `docs/persistence.md`.

Closes danielgerlag#1178

Co-Authored-By: Claude Opus 5.5 <claude-opus-5-5@anthropic.com>
@thrisikos-exida

Copy link
Copy Markdown
Contributor Author

Opened #1451 for the observation in the last section: an instance that fails to load (including one rejected by a binder configured via this change) is re-queued every poll interval indefinitely. It includes a minimal repro against 3.21.0.

@danielgerlag danielgerlag left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for this, @thrisikos-exida, and welcome! It's a really clean first contribution: opt-in, no change to the stored JSON or default behaviour, overloads instead of optional params so binaries don't break, plus tests and docs. I pulled it locally. It builds clean and the Sqlite (14/14) and integration (45/45) suites pass. I also confirmed that the "pre-middlewares for sync workflows" failure happens on master too. I also ran an end-to-end workflow (ForEach/If/Delay/WaitFor) with the allow-list binder from the docs and it completed fine.

A few small, optional suggestions:

  • Persistence providers are registered as transient, so the configure action runs on every resolution. In the docs example, could you create the binder once outside the lambda (e.g. var binder = new AllowListBinder();) and mention that it must be thread-safe? Otherwise the binder's type cache is rebuilt each time.
  • Maybe add a short note or example that generic collections like List<string> on the data class must be allowed too (the example binder currently rejects them), and add the Newtonsoft.Json / Newtonsoft.Json.Serialization usings to the snippet.
  • Optionally, a test covering LargeDataOptimizedEntityFrameworkPersistenceProvider with a binder would be nice.

Thanks also for opening #1451 about poisoned instances being re-polled; we'll track that separately. I've approved the CI run.

- Create the binder once outside the provider factory in the `docs/persistence.md` example and note that it must be thread-safe.
- Allow `List<string>` in the example binder and add the required usings.
- Run the binder tests in `SerializerSettingsFixture` against `LargeDataOptimizedEntityFrameworkPersistenceProvider` as well.

Co-Authored-By: Claude Opus 5.5 <claude-opus-5-5@anthropic.com>
@thrisikos-exida

Copy link
Copy Markdown
Contributor Author

Thanks @danielgerlag, I was trying to keep it as clean as possible, and I appreciate all the feedback!

I've pushed a follow-up commit addressing all three:

  • The docs example now creates the binder once outside the factory lambda, with a note that providers are transient and the shared binder must be thread-safe.
  • The example data class now has a List<string> property, the binder allows it, and the snippet includes the full set of usings. I compiled and ran the snippet against Sqlite to confirm it round-trips.
  • The binder round-trip and rejection tests now also run against LargeDataOptimizedEntityFrameworkPersistenceProvider. They go through IPersistenceProvider, since that provider hides the base methods with new.

For the CI run, besides the known pre-middlewares failure, Integration-Tests failed in RetrySagaWithUserTaskScenario with "Collection was modified" in GetOpenUserActions. That scenario uses the in-memory provider, so it looks like a race between the test reading the instance and the engine updating it, rather than anything in this change.

…ly check

Co-Authored-By: Claude Opus 5.5 <claude-opus-5-5@anthropic.com>
@thrisikos-exida

Copy link
Copy Markdown
Contributor Author

After reviewing the doc changes, we realized we could clarify how the example binder treats generic and array types. It now only trusts non-generic, non-array types from the Workflow Core assembly, and the docs note that allowing types by assembly also trusts the concrete types their members declare.

@danielgerlag danielgerlag left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick follow-ups, @thrisikos-exida! The docs and tests look great. Approving.

@danielgerlag
danielgerlag merged commit d4fe0ea into danielgerlag:master Oct 22, 2026
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Provide JsonSerializerSettings customization for EntityFramework provider

2 participants