Fix DURABLE0010 false positives when replay-safe logger is passed to helpers (#717) by Meir017 · Pull Request #718 · microsoft/durabletask-dotnet

Meir017 · 2026-05-04T14:56:21Z

Summary

Fixes #717. The LoggerOrchestrationAnalyzer (DURABLE0010) previously produced false positives when an orchestrator passed a replay-safe logger (created via context.CreateReplaySafeLogger(...)) to a helper method. This made the rule unusable for the common pattern of factoring logging into private helpers.

Repro (no longer flagged)

public class DemoOrchestrator : TaskOrchestrator<DemoInput, DemoResult>
{
    public override async Task<DemoResult> RunAsync(TaskOrchestrationContext context, DemoInput input)
    {
        var logger = context.CreateReplaySafeLogger(nameof(DemoOrchestrator));
        LogData(logger);
        return new DemoResult($""Processed: {input.Value}"");
    }

    private static void LogData(ILogger logger)
    {
        logger.LogInformation(""Logging some data for demonstration purposes."");
    }
}

Approach

Replaced the previous per-method visit with a per-orchestration data-flow analysis (ReplaySafeLoggerFlowAnalysis):

Walk reachable methods from the orchestration root through IInvocationOperation targets, recording every ILogger reference and every call-site argument of ILogger type. Local function helpers are included.
Resolve safety on demand with memoization-free recursion guarded by a visiting set:
- Locals resolve through their initializer expression (ILocalSymbol.DeclaringSyntaxReferences).
- Helper parameters are safe iff every observed call site within the reachable graph passes a safe value.
- Entry-method parameters are always unsafe (externally supplied).
- Fields/properties are always unsafe (intentional — values can be mutated between replays).
Recognize safe sources:
- TaskOrchestrationContext.CreateReplaySafeLogger invocations, walking the OverriddenMethod chain to support subclass overrides.
- Conditional (? :) and coalesce (??) expressions where all operands are safe.
- IConversionOperation and IParenthesizedOperation are unwrapped.
Write-only references (LHS of simple assignments) are excluded from the candidate list, so this.field = ... no longer flags the LHS — only subsequent reads.

Why a hand-rolled flow analysis

The polished Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.* framework (PointsTo / TaintedData) used by the CA security rules would have collapsed this to ~30 lines, but it is not a publicly stable NuGet — it is source-vendored from dotnet/roslyn-analyzers, which is not appropriate for a Microsoft-shipped analyzer assembly. SemanticModel.AnalyzeDataFlow only tracks symbol-level liveness, not value origin, so it is not applicable here.

Tests

24/24 logger analyzer tests pass (14 pre-existing + 10 new):

Primary repro from DURABLE0010 false positive when ILogger is passed to a helper method called from an orchestrator #717 (helper receiving safe logger → no diag)
Local alias chain (var b = a; where a came from CreateReplaySafeLogger)
DurableFunction equivalent
Local function helper
Mixed call sites (helper called with one safe and one unsafe arg → flags both)
Transitive helper chain (Run → Outer → Inner)
Recursive helper (verifies cycle guard terminates)
Conditional + coalesce expressions in initializer
Field assigned from CreateReplaySafeLogger is still flagged on read (documents the intentional field-is-unsafe limitation)

Documented limitations (accepted for v1)

Local re-assignments after the initializer are not tracked (flow-insensitive on locals).
ref/out parameter aliasing is not tracked.
Wrapper factory methods returning ILogger are not traced.
Method-group/delegate indirection is not tracked.
Field/property values are always treated as unsafe even when assigned from CreateReplaySafeLogger (a future enhancement could relax this for fields assigned exactly once in an orchestrator-local context).

Copilot

Pull request overview

Fixes DURABLE0010 false positives by replacing the logger analyzer’s simple reachable-method scan with a per-orchestration flow analysis that tries to distinguish replay-safe loggers from unsafe ILogger usages.

Changes:

Reworked LoggerOrchestrationAnalyzer to trace ILogger values across locals, helper parameters, conditionals, and coalescing expressions.
Added analyzer tests covering helper methods, local functions, recursion, mixed safe/unsafe call sites, and field reads.
Documented the analyzer fix in CHANGELOG.md.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
`src/Analyzers/Orchestration/LoggerOrchestrationAnalyzer.cs`	Replaces the previous method-level scan with logger-origin flow analysis and updated reporting logic.
`test/Analyzers.Tests/Orchestration/LoggerOrchestrationAnalyzerTests.cs`	Adds regression tests for replay-safe logger propagation through helpers and related cases.
`CHANGELOG.md`	Notes the DURABLE0010 false-positive fix in the unreleased changelog.

Meir017 · 2026-05-04T15:55:15Z

+            // Resolve the local's origin from its declarator's initializer. This is flow-insensitive:
+            // re-assignments after the initial declaration are not tracked. The common pattern
+            // `var logger = context.CreateReplaySafeLogger(...);` is fully supported.
+            if (local.DeclaringSyntaxReferences.Length == 0)
+            {
+                return false;
+            }
+
+            SyntaxNode declSyntax = local.DeclaringSyntaxReferences[0].GetSyntax();
+            if (declSyntax is not VariableDeclaratorSyntax declarator || declarator.Initializer is null)
            {
                return false;
            }

-            // First check for exact match with ILogger
+            SemanticModel semanticModel = this.compilation.GetSemanticModel(declarator.SyntaxTree);
+            IOperation? initOperation = semanticModel.GetOperation(declarator.Initializer.Value);
+            return initOperation != null && this.IsExpressionSafe(initOperation, visiting);


Fixed in d47e764. The analyzer now records every assignment to ILogger locals (initializer + reassignments) in a unified map; IsLocalSafe requires every recorded value to be safe. Added regression test TaskOrchestratorLocalReassignedToUnsafeValueHasDiag.

Meir017 · 2026-05-04T16:00:41Z

+            // For helper parameters we require every observed call site to pass a safe value.
+            // No observed call sites means the helper isn't called from this orchestration's
+            // reachable graph — be conservative and treat as unsafe.
+            if (!this.callSites.TryGetValue(parameter, out List<IOperation>? args) || args.Count == 0)
+            {
+                return false;
+            }
+
+            foreach (IOperation arg in args)
+            {
+                if (!this.IsExpressionSafe(arg, visiting))
+                {
+                    return false;
+                }
+            }
+
+            return true;


Fixed in d47e764. Reassignments to ILogger parameters inside helpers are now recorded alongside call-site args; IsParameterSafe requires both to be safe. Added regression test TaskOrchestratorHelperParameterReassignedToUnsafeValueHasDiag.

Meir017 · 2026-05-04T15:24:52Z

Thanks for the review! Both findings were valid false-negatives — addressed in d47e764:

The analyzer now records all assignments to ILogger locals/parameters (initializer + reassignments) in a single map, and IsLocalSafe/IsParameterSafe require every recorded value to be safe (flow-insensitive AND semantics — sound, since any unsafe reassignment reaches every subsequent read).
Added two regression tests: TaskOrchestratorLocalReassignedToUnsafeValueHasDiag and TaskOrchestratorHelperParameterReassignedToUnsafeValueHasDiag.

All 26 tests pass.

Meir017 · 2026-05-04T18:31:57Z

@halspang / @YunchuWang please review

…helpers The LoggerOrchestrationAnalyzer previously flagged ILogger references inside helper methods called from an orchestrator even when the logger originated from context.CreateReplaySafeLogger(...). This is a common pattern that should not produce a warning. The analyzer now performs a per-orchestration data-flow analysis: - Walks all reachable methods from the orchestration root, recording every ILogger reference and every call-site argument of ILogger type. - Resolves each ILogger reference's safety on demand: locals are resolved through their initializers; helper-method parameters are resolved by requiring every observed call site to pass a safe value. - Recognizes safe sources: TaskOrchestrationContext.CreateReplaySafeLogger invocations (walking OverriddenMethod chains), conditional and coalesce expressions where all operands are safe, and unwraps conversions and parenthesized expressions. - Local function helpers are included in the reachable set. - Write-only references (LHS of simple assignments) are excluded from the candidate list. Fixes microsoft#717 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Address PR review feedback: a local initialized from CreateReplaySafeLogger that is later reassigned to a non-replay-safe value, or a helper parameter reassigned inside the helper, must flip the symbol to unsafe. Records all assignments (initializers + reassignments) and requires every assigned value to be safe (flow-insensitive AND semantics). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings May 4, 2026 14:56

Copilot started reviewing on behalf of Meir017 May 4, 2026 14:57 View session

Copilot AI reviewed May 4, 2026

View reviewed changes

github-actions Bot mentioned this pull request May 5, 2026

DURABLE0010 false positive when ILogger is passed to a helper method called from an orchestrator #717

Open

Meir017 and others added 2 commits May 8, 2026 07:57

torosent force-pushed the fix/durable0010-replay-safe-logger-flow branch from d47e764 to 5682402 Compare May 8, 2026 14:57

Copilot AI review requested due to automatic review settings May 8, 2026 14:57

Copilot started reviewing on behalf of torosent May 8, 2026 14:57 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix DURABLE0010 false positives when replay-safe logger is passed to helpers (#717)#718

Fix DURABLE0010 false positives when replay-safe logger is passed to helpers (#717)#718
Meir017 wants to merge 2 commits intomicrosoft:mainfrom
Meir017:fix/durable0010-replay-safe-logger-flow

Meir017 commented May 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Meir017 May 4, 2026

Uh oh!

Meir017 May 4, 2026

Uh oh!

Meir017 commented May 4, 2026

Uh oh!

Meir017 commented May 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Meir017 commented May 4, 2026

Summary

Repro (no longer flagged)

Approach

Why a hand-rolled flow analysis

Tests

Documented limitations (accepted for v1)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Meir017 May 4, 2026

Choose a reason for hiding this comment

Uh oh!

Meir017 May 4, 2026

Choose a reason for hiding this comment

Uh oh!

Meir017 commented May 4, 2026

Uh oh!

Meir017 commented May 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants