Skip to content

Add TraceContextSerializationBinder to constrain deserialization#1346

Draft
AnatoliB wants to merge 1 commit intomainfrom
anatolib/codeql-fix-37181650
Draft

Add TraceContextSerializationBinder to constrain deserialization#1346
AnatoliB wants to merge 1 commit intomainfrom
anatolib/codeql-fix-37181650

Conversation

@AnatoliB
Copy link
Copy Markdown
Collaborator

@AnatoliB AnatoliB commented May 1, 2026

Co-authored-by: Copilot <copilot@github.com>
Copilot AI review requested due to automatic review settings May 1, 2026 01:50
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR tightens JSON deserialization for trace-context payloads carried with orchestration messages by introducing a dedicated serialization binder that constrains allowed $type values.

Changes:

  • Add TraceContextSerializationBinder to allowlist TraceContextBase (and subclasses) during (de)serialization.
  • Wire the new binder into TraceContextBase’s JsonSerializerSettings.
  • Add tests intended to validate rejection of non-allowlisted $type values and a valid W3C trace-context round-trip.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/DurableTask.Core/TraceContextBase.cs Configures the trace-context serializer to use the new constrained binder.
src/DurableTask.Core/Serializing/TraceContextSerializationBinder.cs Implements a strict binder allowlisting TraceContextBase/subclasses (and Stack<TraceContextBase>).
Test/DurableTask.Core.Tests/TraceContextSerializationBinderTests.cs Adds tests for allowlist enforcement and valid deserialization paths.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +14 to +22
namespace DurableTask.Core.Tests
{
using System;
using Microsoft.VisualStudio.TestTools.UnitTesting;

[TestClass]
public class TraceContextSerializationBinderTests
{
[TestMethod]
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

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

This test file is added under the legacy Test/ folder, but the solution and active test project are under test/DurableTask.Core.Tests (lowercase) and do not include sources from Test/. As a result, these tests won't be compiled/executed in CI. Move this file into test/DurableTask.Core.Tests/ (or update the referenced test csproj to include it) so the new binder behavior is actually covered by the test suite.

Copilot uses AI. Check for mistakes.
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.

2 participants