diff --git a/src/Aspire.Hosting/ApplicationModel/CommandLineArgsCallbackAnnotation.cs b/src/Aspire.Hosting/ApplicationModel/CommandLineArgsCallbackAnnotation.cs index cd927e8f944..6641aa4362d 100644 --- a/src/Aspire.Hosting/ApplicationModel/CommandLineArgsCallbackAnnotation.cs +++ b/src/Aspire.Hosting/ApplicationModel/CommandLineArgsCallbackAnnotation.cs @@ -70,6 +70,15 @@ void IArgCallbackAnnotation.ForgetCachedResult() } } + bool IArgCallbackAnnotation.TryGetCachedResult(out Task>? result) + { + lock(_lock) + { + result = _callbackTask; + return result is not null; + } + } + private async Task> ExecuteCallbackAsync(CommandLineArgsCallbackContext context) { await Callback(context).ConfigureAwait(false); diff --git a/src/Aspire.Hosting/ApplicationModel/EnvironmentCallbackAnnotation.cs b/src/Aspire.Hosting/ApplicationModel/EnvironmentCallbackAnnotation.cs index 410f0282ef8..336d5ee214b 100644 --- a/src/Aspire.Hosting/ApplicationModel/EnvironmentCallbackAnnotation.cs +++ b/src/Aspire.Hosting/ApplicationModel/EnvironmentCallbackAnnotation.cs @@ -103,6 +103,15 @@ void IEnvCallbackAnnotation.ForgetCachedResult() } } + bool IEnvCallbackAnnotation.TryGetCachedResult(out Task>? result) + { + lock(_lock) + { + result = _callbackTask; + return result is not null; + } + } + private async Task> ExecuteCallbackAsync(EnvironmentCallbackContext context) { await Callback(context).ConfigureAwait(false); diff --git a/src/Aspire.Hosting/ApplicationModel/ICallbackResourceAnnotation.cs b/src/Aspire.Hosting/ApplicationModel/ICallbackResourceAnnotation.cs index 767cf003cc4..6113df76b0a 100644 --- a/src/Aspire.Hosting/ApplicationModel/ICallbackResourceAnnotation.cs +++ b/src/Aspire.Hosting/ApplicationModel/ICallbackResourceAnnotation.cs @@ -19,6 +19,25 @@ internal interface ICallbackResourceAnnotation /// The cached result of the callback evaluation. Task EvaluateOnceAsync(TContext context); + /// + /// Peeks at the already-cached callback result without ever executing the callback. + /// + /// + /// When this method returns , the task previously produced by + /// ; otherwise . + /// + /// + /// when a cached result exists; otherwise . + /// + /// + /// This is a read-only peek: unlike it never invokes the callback and never + /// populates the cache. It exists so that read-only consumers (such as aspire describe observing live + /// resource snapshots) can inspect values that DCP has already resolved without racing DCP's own + /// cache lifecycle. Invoking the callback from such a consumer would run it with the consumer's cancellation + /// token and could cache a canceled or faulted task that DCP would later reuse on the resource's execution path. + /// + bool TryGetCachedResult(out Task? result); + /// /// Clears the cached result so that the next call to will re-execute the callback. /// diff --git a/src/Aspire.Hosting/ApplicationModel/LaunchToolArgsCallbackAnnotation.cs b/src/Aspire.Hosting/ApplicationModel/LaunchToolArgsCallbackAnnotation.cs index 0947196c2a1..f9c99dddc54 100644 --- a/src/Aspire.Hosting/ApplicationModel/LaunchToolArgsCallbackAnnotation.cs +++ b/src/Aspire.Hosting/ApplicationModel/LaunchToolArgsCallbackAnnotation.cs @@ -107,6 +107,15 @@ void IArgCallbackAnnotation.ForgetCachedResult() } } + bool IArgCallbackAnnotation.TryGetCachedResult(out Task>? result) + { + lock (_lock) + { + result = _callbackTask; + return result is not null; + } + } + private async Task> ExecuteCallbackAsync(CommandLineArgsCallbackContext context) { await Callback(context).ConfigureAwait(false); diff --git a/src/Aspire.Hosting/ApplicationModel/ResourceDependencyDiscoveryOptions.cs b/src/Aspire.Hosting/ApplicationModel/ResourceDependencyDiscoveryOptions.cs index 3f7e752a67c..8881b126937 100644 --- a/src/Aspire.Hosting/ApplicationModel/ResourceDependencyDiscoveryOptions.cs +++ b/src/Aspire.Hosting/ApplicationModel/ResourceDependencyDiscoveryOptions.cs @@ -19,4 +19,18 @@ public sealed class ResourceDependencyDiscoveryOptions /// on subsequent evaluations of the same annotation, rather than re-evaluating the callback each time. /// public bool CacheAnnotationCallbackResults { get; init; } + + /// + /// When true, discovery reads only callback results that have already been cached by a prior + /// call and never invokes a + /// callback itself. Annotations without a completed cached result are skipped. + /// + /// + /// This is for read-only consumers (such as aspire describe inspecting live resource snapshots) that + /// must not run resource callbacks. Invoking a callback from such a consumer would execute it with the + /// consumer's cancellation token and could poison DCP's cache with a canceled or faulted task that DCP later + /// reuses on the resource's execution path. When this is set, has + /// no effect because nothing is executed or newly cached. + /// + internal bool PeekCachedCallbackResultsOnly { get; init; } } diff --git a/src/Aspire.Hosting/ApplicationModel/ResourceExtensions.cs b/src/Aspire.Hosting/ApplicationModel/ResourceExtensions.cs index 949ac242dd8..1d801ba5891 100644 --- a/src/Aspire.Hosting/ApplicationModel/ResourceExtensions.cs +++ b/src/Aspire.Hosting/ApplicationModel/ResourceExtensions.cs @@ -288,6 +288,7 @@ internal static async ValueTask> GatherArgumentValuesWithoutCaching executionContext, logger, cacheAnnotationCallbackResult: false, + peekCachedResultOnly: false, cancellationToken).ConfigureAwait(false); args.InsertRange(0, launchToolArgs); @@ -299,6 +300,7 @@ private static async ValueTask> GatherLaunchToolArgumentValuesAsyn DistributedApplicationExecutionContext executionContext, ILogger logger, bool cacheAnnotationCallbackResult, + bool peekCachedResultOnly, CancellationToken cancellationToken) { // Launch tool arguments run against an isolated list and do not apply to containers, matching @@ -309,6 +311,15 @@ private static async ValueTask> GatherLaunchToolArgumentValuesAsyn return []; } + if (peekCachedResultOnly) + { + // Read-only discovery: never invoke the callback. Only surface a result DCP has already resolved + // and cached; skip anything still in flight, faulted, or canceled. + return annotation.AsCallbackAnnotation().TryGetCachedResult(out var cachedTask) && cachedTask!.IsCompletedSuccessfully + ? cachedTask.Result + : []; + } + var context = new CommandLineArgsCallbackContext([], resource, cancellationToken) { Logger = logger, @@ -1630,52 +1641,83 @@ private static async Task> GatherRawEnvironmentAndArgumentValuesAsy // Gather environment variable values if (resource.TryGetEnvironmentVariables(out var envAnnotations)) { - var envVars = new Dictionary(); - var context = new EnvironmentCallbackContext(executionContext, resource, envVars, cancellationToken: cancellationToken); - - if (options.CacheAnnotationCallbackResults) + if (options.PeekCachedCallbackResultsOnly) { + // Read-only discovery: never invoke a callback. Only harvest values DCP has already resolved + // and cached via EvaluateOnceAsync. Skip in-flight/faulted/canceled tasks so we never block + // describe on an unresolved value nor observe a poisoned task. foreach (var ann in envAnnotations) { - var resultingVars = await ann.AsCallbackAnnotation().EvaluateOnceAsync(context).ConfigureAwait(false); - rawValues.AddRange(resultingVars.Values); + if (ann.AsCallbackAnnotation().TryGetCachedResult(out var cachedTask) && + cachedTask!.IsCompletedSuccessfully) + { + rawValues.AddRange(cachedTask.Result.Values); + } } - } else { - foreach (var ann in envAnnotations) + var envVars = new Dictionary(); + var context = new EnvironmentCallbackContext(executionContext, resource, envVars, cancellationToken: cancellationToken); + + if (options.CacheAnnotationCallbackResults) { - await ann.Callback(context).ConfigureAwait(false); + foreach (var ann in envAnnotations) + { + var resultingVars = await ann.AsCallbackAnnotation().EvaluateOnceAsync(context).ConfigureAwait(false); + rawValues.AddRange(resultingVars.Values); + } + + } + else + { + foreach (var ann in envAnnotations) + { + await ann.Callback(context).ConfigureAwait(false); + } + rawValues.AddRange(envVars.Values); } - rawValues.AddRange(envVars.Values); } } // Gather command-line argument values if (resource.TryGetAnnotationsOfType(out var argAnnotations)) { - var args = new List(); - var context = new CommandLineArgsCallbackContext(args, resource, cancellationToken) - { - ExecutionContext = executionContext - }; - - if (options.CacheAnnotationCallbackResults) + if (options.PeekCachedCallbackResultsOnly) { foreach (var ann in argAnnotations) { - var resultingArgs = await ann.AsCallbackAnnotation().EvaluateOnceAsync(context).ConfigureAwait(false); - rawValues.AddRange(resultingArgs); + if (ann.AsCallbackAnnotation().TryGetCachedResult(out var cachedTask) && + cachedTask!.IsCompletedSuccessfully) + { + rawValues.AddRange(cachedTask.Result); + } } } else { - foreach (var ann in argAnnotations) + var args = new List(); + var context = new CommandLineArgsCallbackContext(args, resource, cancellationToken) + { + ExecutionContext = executionContext + }; + + if (options.CacheAnnotationCallbackResults) + { + foreach (var ann in argAnnotations) + { + var resultingArgs = await ann.AsCallbackAnnotation().EvaluateOnceAsync(context).ConfigureAwait(false); + rawValues.AddRange(resultingArgs); + } + } + else { - await ann.Callback(context).ConfigureAwait(false); + foreach (var ann in argAnnotations) + { + await ann.Callback(context).ConfigureAwait(false); + } + rawValues.AddRange(args); } - rawValues.AddRange(args); } } @@ -1684,6 +1726,7 @@ private static async Task> GatherRawEnvironmentAndArgumentValuesAsy executionContext, NullLogger.Instance, options.CacheAnnotationCallbackResults, + options.PeekCachedCallbackResultsOnly, cancellationToken).ConfigureAwait(false); rawValues.AddRange(launchToolArgs); diff --git a/src/Aspire.Hosting/Backchannel/AuxiliaryBackchannelRpcTarget.cs b/src/Aspire.Hosting/Backchannel/AuxiliaryBackchannelRpcTarget.cs index d162a794ef9..8ec3a73ac31 100644 --- a/src/Aspire.Hosting/Backchannel/AuxiliaryBackchannelRpcTarget.cs +++ b/src/Aspire.Hosting/Backchannel/AuxiliaryBackchannelRpcTarget.cs @@ -973,10 +973,6 @@ private async Task> GetResourceSnapshotsAsync(bool resour var notificationService = serviceProvider.GetRequiredService(); var results = new List(); - // This is a point-in-time batch, so the set of resolved secret values is identical for - // every resource. Compute it once here rather than once per resource. - var secretParameterValues = GetResolvedSecretParameterValues(); - // Get current state for each resource directly using TryGetCurrentState foreach (var resource in appModel.Resources) { @@ -992,7 +988,12 @@ async Task AddResult(string resourceName) { if (notificationService.TryGetCurrentState(resourceName, out var resourceEvent)) { - var snapshot = await CreateResourceSnapshotFromEventAsync(resourceEvent, resourcePropertiesAsJson, secretParameterValues, cancellationToken).ConfigureAwait(false); + // The secret redaction set is resolved per snapshot inside CreateResourceSnapshotFromEventAsync, + // not once for the whole batch. Building each snapshot can await MCP tool discovery for up to + // s_mcpDiscoveryTimeout, and a parameter can resolve during that window, so a set computed once up + // front could miss a secret that a later resource's snapshot already carries and leak it in + // plaintext. Resolving per snapshot keeps redaction bound to each snapshot. + var snapshot = await CreateResourceSnapshotFromEventAsync(resourceEvent, resourcePropertiesAsJson, cancellationToken).ConfigureAwait(false); if (snapshot is not null) { results.Add(snapshot); @@ -1024,11 +1025,10 @@ private async IAsyncEnumerable WatchResourceSnapshotsAsync( await foreach (var resourceEvent in resourceEvents.WithCancellation(cancellationToken).ConfigureAwait(false)) { - // Recompute the resolved secret values for every event. Secrets can be resolved between - // events (e.g. interactive parameter entry after the watch starts), so caching the set - // once outside the loop would let a value that becomes secret later bypass redaction. - var secretParameterValues = GetResolvedSecretParameterValues(); - var snapshot = await CreateResourceSnapshotFromEventAsync(resourceEvent, resourcePropertiesAsJson, secretParameterValues, cancellationToken).ConfigureAwait(false); + // The secret redaction set is resolved per event inside CreateResourceSnapshotFromEventAsync, so a + // secret resolved (or replaced, or newly referenced after a resource restart) while the watch is open + // is reflected on later events rather than fixed to when the watch started. + var snapshot = await CreateResourceSnapshotFromEventAsync(resourceEvent, resourcePropertiesAsJson, cancellationToken).ConfigureAwait(false); if (snapshot is not null) { yield return snapshot; @@ -1039,7 +1039,6 @@ private async IAsyncEnumerable WatchResourceSnapshotsAsync( private async Task CreateResourceSnapshotFromEventAsync( ResourceEvent resourceEvent, bool resourcePropertiesAsJson, - HashSet secretParameterValues, CancellationToken cancellationToken) { var resource = resourceEvent.Resource; @@ -1112,11 +1111,17 @@ private async IAsyncEnumerable WatchResourceSnapshotsAsync( }) .ToArray(); + // Resolve the secret redaction set for THIS snapshot, after the MCP discovery await above. The snapshot's + // environment values were captured before that await, so any secret they carry was already resolved by the + // time we get here. Resolving now (rather than once for a whole describe batch) keeps redaction bound to + // the snapshot even though building each snapshot can block on MCP discovery for up to s_mcpDiscoveryTimeout, + // during which another parameter can resolve. The redaction history is add-only and AppHost-scoped (see + // GetResolvedSecretParameterValuesAsync), so resolving per snapshot only ever grows the set and also keeps + // redacting a secret whose value has since been replaced or whose owning resource has restarted. + var secretParameterValues = await GetResolvedSecretParameterValuesAsync(cancellationToken).ConfigureAwait(false); + // Build environment variables. Values that match a secret parameter's value are // redacted so secrets don't leak through clients (e.g. aspire describe --format json). - // The secret values are computed by the caller: once per batch for the one-shot - // GetResourceSnapshotsAsync, but per event for the streaming WatchResourceSnapshotsAsync - // so that secrets resolved mid-stream are still redacted. var environmentVariables = snapshot.EnvironmentVariables .Select(e => new ResourceSnapshotEnvironmentVariable { @@ -1211,33 +1216,28 @@ private async IAsyncEnumerable WatchResourceSnapshotsAsync( => value is not null && secretParameterValues.Contains(value) ? null : value; /// - /// Collects the resolved values of secret parameters in the application model so they can - /// be redacted from data sent to clients. Only values that have already been resolved are - /// returned; this never blocks waiting for interactive parameter resolution. + /// Collects the resolved values of secret parameters reachable from the application model so they can be + /// redacted from data sent to clients. Only values that have already been resolved are included; this never + /// blocks waiting for interactive parameter resolution. Resolved values are accumulated add-only in the + /// AppHost-scoped (which ParameterProcessor also populates at + /// assignment time), so a value a parameter has since been reassigned away from stays redacted while an older + /// snapshot can still carry it. /// - private HashSet GetResolvedSecretParameterValues() + private async Task> GetResolvedSecretParameterValuesAsync(CancellationToken cancellationToken) { - var secretValues = new HashSet(StringComparer.Ordinal); - - if (serviceProvider.GetService() is not { } appModel) - { - return secretValues; - } + // Resolve the current value of each accumulated secret parameter (peek-only; never blocks on interactive + // resolution), then merge into the AppHost-scoped add-only value history below. + var resolvedThisPass = new List(); - foreach (var parameter in appModel.Resources.OfType()) + foreach (var parameter in await GetSecretParametersAsync(cancellationToken).ConfigureAwait(false)) { - if (!parameter.Secret) - { - continue; - } - if (parameter.WaitForValueTcs is { } waitForValueTcs) { // Run mode: peek at the resolved value without waiting for resolution. if (waitForValueTcs.Task is { IsCompletedSuccessfully: true } valueTask && valueTask.Result is { Length: > 0 } value) { - secretValues.Add(value); + resolvedThisPass.Add(value); } } else @@ -1246,7 +1246,7 @@ private HashSet GetResolvedSecretParameterValues() { if (parameter.ValueInternal is { Length: > 0 } value) { - secretValues.Add(value); + resolvedThisPass.Add(value); } } catch @@ -1256,7 +1256,103 @@ private HashSet GetResolvedSecretParameterValues() } } - return secretValues; + // Accumulate this pass's resolved secret strings add-only and return everything seen so far. A parameter's + // value can be replaced in place (the runtime "Set parameter" path swaps its completed WaitForValueTcs), + // so re-resolving a retained parameter later yields only the new value; keeping every value we have ever + // resolved ensures a still-current snapshot carrying the previous value is still redacted. + return serviceProvider.GetRequiredService().AddValuesAndSnapshot(resolvedThisPass); + } + + /// + /// Gets the set of secret instances reachable from the application + /// model, including parameters that are only referenced by another resource rather than registered + /// as a top-level resource. + /// + /// + /// The set includes generated parameters (such as the password created by AddPostgres) that are + /// referenced by a resource but never registered in the model, which enumerating + /// appModel.Resources.OfType<ParameterResource>() alone would miss and leak in plaintext + /// (https://github.com/microsoft/aspire/issues/19241). + /// + /// Discovery is peek-only: it reads callback results that DCP already resolved and cached while + /// starting the resource () and + /// never invokes a callback itself. This matters because aspire describe observes live resource + /// snapshots concurrently with DCP's own cache lifecycle: DCP forgets and re-evaluates a resource's callbacks + /// on restart (see DcpExecutor.ForgetCachedCallbackResults). Invoking a callback from here would run it + /// with the client's cancellation token and could cache a canceled or faulted task that DCP would then reuse on + /// the resource's execution path. A running resource can only appear in a snapshot after DCP has resolved and + /// cached its values, so peeking still observes every secret that a snapshot could expose. + /// + /// + /// The discovered set is merged into the AppHost-scoped, add-only : it only + /// ever grows for the life of the AppHost and is shared across connections. A restart can change which secret a + /// resource references, and a still-in-flight snapshot from the prior incarnation can carry the previous value, + /// so the redaction set must never shrink or that value would be emitted in plaintext. + /// + /// + private async Task> GetSecretParametersAsync(CancellationToken cancellationToken) + { + var history = serviceProvider.GetRequiredService(); + + if (serviceProvider.GetService() is not { } appModel) + { + // No model resolved yet; return the secrets accumulated so far without adding any. + return history.AddParametersAndSnapshot([]); + } + + var executionContext = serviceProvider.GetRequiredService(); + + // Peek at the callback results DCP cached while starting each resource; never invoke a callback. This + // observes the same referenced resources that produced the running snapshot without racing DCP's cache + // lifecycle or running stateful callbacks with the client's cancellation token. + var discoveryOptions = new ResourceDependencyDiscoveryOptions + { + DiscoveryMode = ResourceDependencyDiscoveryMode.Recursive, + PeekCachedCallbackResultsOnly = true + }; + + // Parameter resources referenced by annotations are not registered in the model, so they are not + // subject to its unique-name constraint. Collect by reference to preserve distinct same-named secrets. + var secretParameters = new HashSet(ReferenceEqualityComparer.Instance); + + foreach (var parameter in appModel.Resources.OfType()) + { + if (parameter.Secret) + { + secretParameters.Add(parameter); + } + } + + // Compute the transitive dependency closure of every resource in a single multi-root walk. It shares + // one visited set across all roots, so each resource's (execution-cached) callbacks are read at most + // once. Discovering per resource instead would repeat the traversal for every resource and make the + // initial WatchAsync stream — which emits one event per resource — do quadratic work. + IReadOnlySet dependencies; + try + { + dependencies = await ResourceExtensions.GetDependenciesAsync(appModel.Resources, executionContext, discoveryOptions, cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + // Fail closed at this confidentiality boundary. Peek-only discovery does not invoke callbacks, so a + // failure here is unexpected; if it does happen the redaction set is incomplete and a snapshot built + // from it could expose a secret in plaintext, so propagate instead of continuing with a partial set. + // Cancellation is intentionally not caught here so it surfaces as cancellation rather than a leak. + logger.LogDebug(ex, "Failed to compute resource dependencies while collecting secret parameters for redaction."); + throw; + } + + foreach (var parameter in dependencies.OfType()) + { + if (parameter.Secret) + { + secretParameters.Add(parameter); + } + } + + // Merge this pass's discoveries into the add-only history and return everything seen so far, so a secret + // referenced by an earlier incarnation stays redacted even after a restart re-points the resource. + return history.AddParametersAndSnapshot(secretParameters); } private static ResourceSnapshotCommandArgument CreateCommandArgument(InteractionInput input) diff --git a/src/Aspire.Hosting/Backchannel/SecretRedactionHistory.cs b/src/Aspire.Hosting/Backchannel/SecretRedactionHistory.cs new file mode 100644 index 00000000000..cf27883948a --- /dev/null +++ b/src/Aspire.Hosting/Backchannel/SecretRedactionHistory.cs @@ -0,0 +1,92 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Aspire.Hosting.ApplicationModel; + +namespace Aspire.Hosting.Backchannel; + +/// +/// AppHost-scoped, add-only history of the secret parameters and resolved secret values that any +/// aspire describe/watch backchannel connection has ever observed, so they stay redacted from data +/// sent to clients (https://github.com/microsoft/aspire/issues/19241). +/// +/// +/// Registered as a singleton for the lifetime of the AppHost and shared by every +/// (one is created per connection). The redaction set must outlive an +/// individual connection: a resource's secret can change while the app runs, and a snapshot carrying an older value +/// can be emitted to a client that connected after the change. If each connection tracked history on its +/// own, a freshly connected client's target would start empty and leak that older value, so history is accumulated +/// once per AppHost instead. +/// +/// Both sets only ever grow: +/// +/// +/// +/// The parameter set grows because when DCP restarts a resource it forgets and re-evaluates the resource's callbacks +/// (DcpExecutor.ForgetCachedCallbackResults), which can swap which secret a resource references. A +/// still-in-flight snapshot from the prior incarnation can carry the old secret, so we keep trying to resolve every +/// secret parameter ever observed rather than only the current pass's set. +/// +/// +/// The value set grows because a parameter's resolved value can be replaced in place: the runtime "Set parameter" +/// path swaps a completed for a new one +/// (ParameterProcessor.SetParameterValue), so re-resolving a retained parameter later yields only the new +/// value. An already-published or still-current snapshot can still carry the previous value, so we must keep +/// redacting every secret string ever resolved or that old value would be emitted in plaintext. +/// +/// +/// +/// A backchannel connection only observes a secret value once it is open, so a value assigned and then reassigned +/// before the first connection would otherwise be absent from the history and leak from a lagging snapshot. To close +/// that cold-start residual, ParameterProcessor also records secret values into this history at the moment it +/// assigns or replaces them (see its SecretRedactionHistory wiring), independent of any connection. Values +/// that never flow through the parameter processor still rely on connection-time observation. +/// +/// +internal sealed class SecretRedactionHistory +{ + // Collected by reference: parameter resources referenced by annotations are not registered in the model and so + // are not subject to its unique-name constraint, and distinct same-named secrets must be preserved. + private readonly HashSet _parameters = new(ReferenceEqualityComparer.Instance); + private readonly HashSet _values = new(StringComparer.Ordinal); + private readonly object _lock = new(); + + /// + /// Merges into the history and returns a snapshot of every secret parameter seen so + /// far. Pass an empty sequence to read the current snapshot without adding. + /// + public IReadOnlyList AddParametersAndSnapshot(IEnumerable parameters) + { + lock (_lock) + { + _parameters.UnionWith(parameters); + return [.. _parameters]; + } + } + + /// + /// Merges into the history and returns a fresh snapshot of every resolved secret value + /// seen so far, for membership testing while redacting. + /// + public HashSet AddValuesAndSnapshot(IEnumerable values) + { + lock (_lock) + { + _values.UnionWith(values); + return new HashSet(_values, StringComparer.Ordinal); + } + } + + /// + /// Merges into the history without allocating a snapshot. Used to record secret values + /// as they are assigned (see ParameterProcessor), where the caller only writes and never needs to read the + /// set back. + /// + public void AddValues(IEnumerable values) + { + lock (_lock) + { + _values.UnionWith(values); + } + } +} diff --git a/src/Aspire.Hosting/DistributedApplicationBuilder.cs b/src/Aspire.Hosting/DistributedApplicationBuilder.cs index 20f7fad0aa8..9d4af0aad9f 100644 --- a/src/Aspire.Hosting/DistributedApplicationBuilder.cs +++ b/src/Aspire.Hosting/DistributedApplicationBuilder.cs @@ -382,7 +382,16 @@ public DistributedApplicationBuilder(DistributedApplicationOptions options) _innerBuilder.Services.TryAddSingleton(); _innerBuilder.Services.AddSingleton(); _innerBuilder.Services.AddSingleton(sp => sp.GetRequiredService()); - _innerBuilder.Services.AddSingleton(); + _innerBuilder.Services.AddSingleton(static sp => + { + var parameterProcessor = ActivatorUtilities.CreateInstance(sp); + // Wire the AppHost-scoped redaction history after construction (not through the public constructor) so + // the processor records resolved secret values as they are assigned/replaced. This populates the + // describe/watch redaction set from startup, independent of any backchannel connection, while keeping + // ParameterProcessor's public constructor unchanged (https://github.com/microsoft/aspire/issues/19241). + parameterProcessor.SecretRedactionHistory = sp.GetRequiredService(); + return parameterProcessor; + }); _innerBuilder.Services.AddSingleton(Eventing); _innerBuilder.Services.AddSingleton(); _innerBuilder.Services.AddHealthChecks(); @@ -425,6 +434,9 @@ public DistributedApplicationBuilder(DistributedApplicationOptions options) _innerBuilder.Services.AddSingleton(); _innerBuilder.Services.AddSingleton(); _innerBuilder.Services.AddHostedService(sp => sp.GetRequiredService()); + // Shared by every per-connection AuxiliaryBackchannelRpcTarget so the describe/watch secret redaction set + // outlives an individual connection (https://github.com/microsoft/aspire/issues/19241). + _innerBuilder.Services.AddSingleton(); _innerBuilder.Services.AddSingleton(); _innerBuilder.Services.AddSingleton(); diff --git a/src/Aspire.Hosting/Orchestrator/ParameterProcessor.cs b/src/Aspire.Hosting/Orchestrator/ParameterProcessor.cs index 6671aa9ce39..34907d1411d 100644 --- a/src/Aspire.Hosting/Orchestrator/ParameterProcessor.cs +++ b/src/Aspire.Hosting/Orchestrator/ParameterProcessor.cs @@ -6,6 +6,7 @@ using System.Globalization; using Aspire.Hosting.ApplicationModel; +using Aspire.Hosting.Backchannel; using Aspire.Hosting.Pipelines; using Aspire.Hosting.Resources; using Microsoft.Extensions.Logging; @@ -33,6 +34,15 @@ public sealed class ParameterProcessor( private CancellationTokenSource? _allParametersResolvedCts; private Task? _parameterResolutionTask; + /// + /// AppHost-scoped history that records resolved secret parameter values so aspire describe/watch + /// can redact them. Assigned by DI after construction (see DistributedApplicationBuilder) rather than + /// injected through the public constructor, to keep the public API surface unchanged for backporting. Null when + /// the processor is created outside the AppHost container (e.g. in unit tests), in which case recording is a + /// no-op. + /// + internal SecretRedactionHistory? SecretRedactionHistory { get; set; } + /// /// Initializes parameter resources and handles unresolved parameters if interaction service is available. /// @@ -154,6 +164,7 @@ private async Task ProcessParameterAsync(ParameterResource parameterResource) await UpdateParameterStateAsync(parameterResource, value, KnownResourceStates.Running).ConfigureAwait(false); parameterResource.WaitForValueTcs?.TrySetResult(value); + RecordSecretValueForRedaction(parameterResource, value); } catch (Exception ex) { @@ -494,6 +505,7 @@ private async Task ApplyParameterValueAsync(ParameterResource parameterResource, } parameterResource.WaitForValueTcs?.TrySetResult(inputValue); + RecordSecretValueForRedaction(parameterResource, inputValue); await UpdateParameterStateAsync(parameterResource, inputValue, KnownResourceStates.Running).ConfigureAwait(false); @@ -518,6 +530,20 @@ private async Task ApplyParameterValueAsync(ParameterResource parameterResource, } } + // Record a resolved secret value into the AppHost-scoped redaction history at the moment it is assigned or + // replaced, so `aspire describe`/`watch` redacts it even before any backchannel connection has peeked it. The + // describe path only observes a secret once a connection is open; a value assigned (and possibly replaced) before + // the first connection would otherwise be absent from the history and leak from a lagging snapshot + // (https://github.com/microsoft/aspire/issues/19241). No-op for non-secret parameters, empty values, or when no + // history is wired (e.g. unit tests that construct the processor directly). + private void RecordSecretValueForRedaction(ParameterResource parameterResource, string? value) + { + if (parameterResource.Secret && value is { Length: > 0 } && SecretRedactionHistory is { } history) + { + history.AddValues([value]); + } + } + // Internal for testing purposes - allows passing specific parameters to test. internal async Task HandleUnresolvedParametersAsync(IList unresolvedParameters, CancellationToken cancellationToken) { diff --git a/tests/Aspire.Hosting.Tests/Backchannel/AuxiliaryBackchannelRpcTargetTests.cs b/tests/Aspire.Hosting.Tests/Backchannel/AuxiliaryBackchannelRpcTargetTests.cs index 3c85f4a7948..124ab1c2ea1 100644 --- a/tests/Aspire.Hosting.Tests/Backchannel/AuxiliaryBackchannelRpcTargetTests.cs +++ b/tests/Aspire.Hosting.Tests/Backchannel/AuxiliaryBackchannelRpcTargetTests.cs @@ -375,24 +375,662 @@ await notificationService.PublishUpdateAsync(custom.Resource, s => s with await app.StopAsync().DefaultTimeout(); } + [Fact] + public async Task GetResourceSnapshotsAsync_RedactsSecretParameterReferencedByOwningResourceEnvironment() + { + using var builder = TestDistributedApplicationBuilder.Create(outputHelper); + + // Mimic AddPostgres: a generated secret password parameter that is referenced by the owning + // resource's own environment but is never registered as a top-level resource in the model. Prior + // to the fix for https://github.com/microsoft/aspire/issues/19241 the redaction set only contained + // top-level ParameterResources, so the owning resource leaked this value in plaintext even though + // the same value was redacted when it flowed into a dependent resource. + var passwordParameter = new ParameterResource("pg-password", _ => "generated-s3cr3t", secret: true); + var owner = builder.AddResource(new CustomResourceWithEnvironment("pg")) + .WithEnvironment(context => context.EnvironmentVariables["POSTGRES_PASSWORD"] = passwordParameter); + + using var app = builder.Build(); + await app.StartAsync().DefaultTimeout(); + + // Simulate the generated parameter having been resolved to its runtime value. + passwordParameter.WaitForValueTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + passwordParameter.WaitForValueTcs.SetResult("generated-s3cr3t"); + + // Cache the env callback the way DCP does on start, so peek-only discovery can observe the reference. + await PrimeEnvironmentCallbackCacheAsync(owner.Resource, app.Services).DefaultTimeout(); + + var notificationService = app.Services.GetRequiredService(); + await notificationService.PublishUpdateAsync(owner.Resource, s => s with + { + State = new ResourceStateSnapshot("Running", KnownResourceStateStyles.Success), + EnvironmentVariables = + [ + new EnvironmentVariableSnapshot("POSTGRES_PASSWORD", "generated-s3cr3t", true) + ] + }).DefaultTimeout(); + + var target = new AuxiliaryBackchannelRpcTarget( + NullLogger.Instance, + app.Services.GetRequiredService(), + app.Services.GetRequiredService(), + app.Services); + + var result = await target.GetResourceSnapshotsAsync().DefaultTimeout(); + + var snapshot = Assert.Single(result, r => r.Name == "pg"); + var password = Assert.Single(snapshot.EnvironmentVariables, e => e.Name == "POSTGRES_PASSWORD"); + + // The owning resource's own environment variable must be redacted even though the secret parameter + // is only referenced by the resource and is not a top-level resource in the model. + Assert.Null(password.Value); + + await app.StopAsync().DefaultTimeout(); + } + + [Fact] + public async Task GetResourceSnapshotsAsync_RedactsDistinctSecretParametersWithSameName() + { + using var builder = TestDistributedApplicationBuilder.Create(outputHelper); + + var topLevelParameter = builder.AddParameter("shared-name", "top-level-secret", secret: true); + var referencedParameter = new ParameterResource("shared-name", _ => "referenced-secret", secret: true); + var owner = builder.AddResource(new CustomResourceWithEnvironment("owner")) + .WithEnvironment(context => context.EnvironmentVariables["REFERENCED_SECRET"] = referencedParameter); + + using var app = builder.Build(); + await app.StartAsync().DefaultTimeout(); + + topLevelParameter.Resource.WaitForValueTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + topLevelParameter.Resource.WaitForValueTcs.SetResult("top-level-secret"); + referencedParameter.WaitForValueTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + referencedParameter.WaitForValueTcs.SetResult("referenced-secret"); + + // Cache the env callback the way DCP does on start, so peek-only discovery can observe the reference. + await PrimeEnvironmentCallbackCacheAsync(owner.Resource, app.Services).DefaultTimeout(); + + var notificationService = app.Services.GetRequiredService(); + await notificationService.PublishUpdateAsync(owner.Resource, s => s with + { + State = new ResourceStateSnapshot("Running", KnownResourceStateStyles.Success), + EnvironmentVariables = + [ + new EnvironmentVariableSnapshot("TOP_LEVEL_SECRET", "top-level-secret", true), + new EnvironmentVariableSnapshot("REFERENCED_SECRET", "referenced-secret", true) + ] + }).DefaultTimeout(); + + var target = new AuxiliaryBackchannelRpcTarget( + NullLogger.Instance, + app.Services.GetRequiredService(), + app.Services.GetRequiredService(), + app.Services); + + var result = await target.GetResourceSnapshotsAsync().DefaultTimeout(); + + var snapshot = Assert.Single(result, r => r.Name == "owner"); + Assert.Null(Assert.Single(snapshot.EnvironmentVariables, e => e.Name == "TOP_LEVEL_SECRET").Value); + Assert.Null(Assert.Single(snapshot.EnvironmentVariables, e => e.Name == "REFERENCED_SECRET").Value); + + await app.StopAsync().DefaultTimeout(); + } + + [Fact] + public async Task GetResourceSnapshotsAsync_RedactsNewlyReferencedSecret_AfterRestartRepointsResource() + { + using var builder = TestDistributedApplicationBuilder.Create(outputHelper); + + // The set of secret parameters a resource references is not fixed for the connection lifetime. DCP + // clears and re-evaluates a resource's environment/argument callbacks when it restarts (see + // DcpExecutor.ForgetCachedCallbackResults), so a restart can make a resource reference a secret it + // did not reference before. Discovery is peek-only (it reads the callback results DCP cached on start + // and never invokes a callback), so this test primes the cache like DCP does, then simulates a restart + // between two snapshot calls on the same target — flipping the referenced secret, clearing the cached + // callback result, and re-priming exactly as a restart does — and asserts the second call redacts the + // newly referenced secret. + var secretA = new ParameterResource("secret-a", _ => "value-a", secret: true); + secretA.WaitForValueTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + secretA.WaitForValueTcs.SetResult("value-a"); + + var secretB = new ParameterResource("secret-b", _ => "value-b", secret: true); + secretB.WaitForValueTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + secretB.WaitForValueTcs.SetResult("value-b"); + + // The environment callback references whichever secret this local points at when it is evaluated. + // The callback is registered once, before the host is built, so the resource's annotation collection + // is never mutated on a live host — mutating annotations after StartAsync races with background + // annotation evaluation and makes the test flaky. The restart is simulated below by flipping this + // local, clearing the callback's cached result, and re-priming, not by adding another annotation. + var referencedSecret = secretA; + var owner = builder.AddResource(new CustomResourceWithEnvironment("owner")) + .WithEnvironment(context => context.EnvironmentVariables["SECRET"] = referencedSecret); + + using var app = builder.Build(); + await app.StartAsync().DefaultTimeout(); + + // Cache the callback result (referencing secretA) the way DCP does when it first starts the resource. + await PrimeEnvironmentCallbackCacheAsync(owner.Resource, app.Services).DefaultTimeout(); + + var notificationService = app.Services.GetRequiredService(); + await notificationService.PublishUpdateAsync(owner.Resource, s => s with + { + State = new ResourceStateSnapshot("Running", KnownResourceStateStyles.Success), + EnvironmentVariables = + [ + new EnvironmentVariableSnapshot("SECRET", "value-a", true) + ] + }).DefaultTimeout(); + + // The same target instance is reused across both calls so the add-only accumulator persists between + // them, exactly as it would over the lifetime of a single describe/watch connection. + var target = new AuxiliaryBackchannelRpcTarget( + NullLogger.Instance, + app.Services.GetRequiredService(), + app.Services.GetRequiredService(), + app.Services); + + var firstResult = await target.GetResourceSnapshotsAsync().DefaultTimeout(); + var firstSnapshot = Assert.Single(firstResult, r => r.Name == "owner"); + Assert.Null(Assert.Single(firstSnapshot.EnvironmentVariables, e => e.Name == "SECRET").Value); + + // Simulate a restart: the resource now references a different secret. Clear the previously cached + // callback result exactly as DcpExecutor.ForgetCachedCallbackResults does, then re-prime so the cache + // holds the new reference (secretB) as it would after DCP re-evaluates on restart. + referencedSecret = secretB; + var environmentCallback = owner.Resource.Annotations.OfType().Single(); + environmentCallback.AsCallbackAnnotation().ForgetCachedResult(); + await PrimeEnvironmentCallbackCacheAsync(owner.Resource, app.Services).DefaultTimeout(); + + await notificationService.PublishUpdateAsync(owner.Resource, s => s with + { + State = new ResourceStateSnapshot("Running", KnownResourceStateStyles.Success), + EnvironmentVariables = + [ + new EnvironmentVariableSnapshot("SECRET", "value-b", true) + ] + }).DefaultTimeout(); + + var secondResult = await target.GetResourceSnapshotsAsync().DefaultTimeout(); + var secondSnapshot = Assert.Single(secondResult, r => r.Name == "owner"); + + // After the restart the redaction set must reflect the newly referenced secret. Peek-only discovery on + // the second call observes secretB (freshly cached), so value-b is redacted. + Assert.Null(Assert.Single(secondSnapshot.EnvironmentVariables, e => e.Name == "SECRET").Value); + + await app.StopAsync().DefaultTimeout(); + } + + [Fact] + public async Task GetResourceSnapshotsAsync_RetainsPreviouslyReferencedSecret_AfterRestartRepointsResource() + { + using var builder = TestDistributedApplicationBuilder.Create(outputHelper); + + // Regression test for the leak adamint identified in review of the #19241 fix: during a restart, a + // still-in-flight snapshot from the prior incarnation can carry the OLD secret value while the resource's + // callback has already been re-pointed at a new secret. If discovery only redacted the current pass's + // set, the stale old value would be emitted in plaintext. The per-connection redaction set is add-only, + // so a secret observed on an earlier pass stays redacted even after the resource stops referencing it. + var secretA = new ParameterResource("secret-a", _ => "value-a", secret: true); + secretA.WaitForValueTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + secretA.WaitForValueTcs.SetResult("value-a"); + + var secretB = new ParameterResource("secret-b", _ => "value-b", secret: true); + secretB.WaitForValueTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + secretB.WaitForValueTcs.SetResult("value-b"); + + var referencedSecret = secretA; + var owner = builder.AddResource(new CustomResourceWithEnvironment("owner")) + .WithEnvironment(context => context.EnvironmentVariables["SECRET"] = referencedSecret); + + using var app = builder.Build(); + await app.StartAsync().DefaultTimeout(); + + await PrimeEnvironmentCallbackCacheAsync(owner.Resource, app.Services).DefaultTimeout(); + + var notificationService = app.Services.GetRequiredService(); + await notificationService.PublishUpdateAsync(owner.Resource, s => s with + { + State = new ResourceStateSnapshot("Running", KnownResourceStateStyles.Success), + EnvironmentVariables = + [ + new EnvironmentVariableSnapshot("SECRET", "value-a", true) + ] + }).DefaultTimeout(); + + var target = new AuxiliaryBackchannelRpcTarget( + NullLogger.Instance, + app.Services.GetRequiredService(), + app.Services.GetRequiredService(), + app.Services); + + // First pass observes secretA and adds it to the connection's redaction set. + var firstResult = await target.GetResourceSnapshotsAsync().DefaultTimeout(); + var firstSnapshot = Assert.Single(firstResult, r => r.Name == "owner"); + Assert.Null(Assert.Single(firstSnapshot.EnvironmentVariables, e => e.Name == "SECRET").Value); + + // Restart re-points the resource at secretB, but a lagging snapshot from the old incarnation still + // carries value-a. Re-point and re-prime the cache so discovery would, on its own, only find secretB. + referencedSecret = secretB; + var environmentCallback = owner.Resource.Annotations.OfType().Single(); + environmentCallback.AsCallbackAnnotation().ForgetCachedResult(); + await PrimeEnvironmentCallbackCacheAsync(owner.Resource, app.Services).DefaultTimeout(); + + await notificationService.PublishUpdateAsync(owner.Resource, s => s with + { + State = new ResourceStateSnapshot("Stopping", KnownResourceStateStyles.Info), + EnvironmentVariables = + [ + new EnvironmentVariableSnapshot("SECRET", "value-a", true) + ] + }).DefaultTimeout(); + + var secondResult = await target.GetResourceSnapshotsAsync().DefaultTimeout(); + var secondSnapshot = Assert.Single(secondResult, r => r.Name == "owner"); + + // The stale value-a must still be redacted. Without add-only accumulation, discovery on this pass finds + // only secretB and would publish value-a in plaintext — the exact leak the reviewer called out. + Assert.Null(Assert.Single(secondSnapshot.EnvironmentVariables, e => e.Name == "SECRET").Value); + + await app.StopAsync().DefaultTimeout(); + } + + [Fact] + public async Task GetResourceSnapshotsAsync_RetainsPreviousSecretValue_AfterParameterValueIsReplaced() + { + using var builder = TestDistributedApplicationBuilder.Create(outputHelper); + + // Regression test for the leak the reviewer identified: the runtime "Set parameter" path replaces a + // parameter's already-completed WaitForValueTcs with a new value (ParameterProcessor.SetParameterValue), + // so re-resolving the SAME retained parameter object later yields only the new value. An already-published + // or still-current snapshot can still carry the previous secret value, so redaction accumulates resolved + // secret STRINGS add-only — retaining the parameter object alone is not enough because its value has been + // overwritten in place. The owner keeps referencing the same parameter throughout; only its value changes. + var secret = new ParameterResource("secret", _ => "value-a", secret: true); + secret.WaitForValueTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + secret.WaitForValueTcs.SetResult("value-a"); + + var owner = builder.AddResource(new CustomResourceWithEnvironment("owner")) + .WithEnvironment(context => context.EnvironmentVariables["SECRET"] = secret); + + using var app = builder.Build(); + await app.StartAsync().DefaultTimeout(); + + await PrimeEnvironmentCallbackCacheAsync(owner.Resource, app.Services).DefaultTimeout(); + + var notificationService = app.Services.GetRequiredService(); + await notificationService.PublishUpdateAsync(owner.Resource, s => s with + { + State = new ResourceStateSnapshot("Running", KnownResourceStateStyles.Success), + EnvironmentVariables = + [ + new EnvironmentVariableSnapshot("SECRET", "value-a", true) + ] + }).DefaultTimeout(); + + // The same target instance is reused across both calls so the add-only accumulator persists between them, + // exactly as it would over the lifetime of a single describe/watch connection. + var target = new AuxiliaryBackchannelRpcTarget( + NullLogger.Instance, + app.Services.GetRequiredService(), + app.Services.GetRequiredService(), + app.Services); + + // First pass observes value-a and adds it to the connection's redaction set. + var firstResult = await target.GetResourceSnapshotsAsync().DefaultTimeout(); + var firstSnapshot = Assert.Single(firstResult, r => r.Name == "owner"); + Assert.Null(Assert.Single(firstSnapshot.EnvironmentVariables, e => e.Name == "SECRET").Value); + + // The runtime replaces the parameter's resolved value with value-b (as SetParameterValue does by swapping + // the completed WaitForValueTcs), but a lagging snapshot still carries value-a. Re-resolving the same + // parameter object now yields only value-b. + secret.WaitForValueTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + secret.WaitForValueTcs.SetResult("value-b"); + + await notificationService.PublishUpdateAsync(owner.Resource, s => s with + { + State = new ResourceStateSnapshot("Running", KnownResourceStateStyles.Success), + EnvironmentVariables = + [ + new EnvironmentVariableSnapshot("SECRET", "value-a", true) + ] + }).DefaultTimeout(); + + var secondResult = await target.GetResourceSnapshotsAsync().DefaultTimeout(); + var secondSnapshot = Assert.Single(secondResult, r => r.Name == "owner"); + + // value-a must still be redacted. Without add-only value accumulation, re-resolving the parameter yields + // only value-b, so the stale value-a would be emitted in plaintext — the exact leak the reviewer called out. + Assert.Null(Assert.Single(secondSnapshot.EnvironmentVariables, e => e.Name == "SECRET").Value); + + await app.StopAsync().DefaultTimeout(); + } + + [Fact] + public async Task GetResourceSnapshotsAsync_RetainsPreviousSecretValue_AcrossSeparateConnections() + { + using var builder = TestDistributedApplicationBuilder.Create(outputHelper); + + // Regression test for the AppHost-scope gap the reviewer identified: every backchannel connection gets its + // own AuxiliaryBackchannelRpcTarget (AuxiliaryBackchannelService.HandleClientConnectionAsync). If the + // redaction history lived on the target, a client that connected AFTER a secret's value was replaced would + // start with an empty set and leak the previous value carried by a lagging snapshot. The history is + // AppHost-scoped (the SecretRedactionHistory singleton) and shared by every target, so a value one + // connection observed stays redacted for a later, independently constructed connection. + var secret = new ParameterResource("secret", _ => "value-a", secret: true); + secret.WaitForValueTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + secret.WaitForValueTcs.SetResult("value-a"); + + var owner = builder.AddResource(new CustomResourceWithEnvironment("owner")) + .WithEnvironment(context => context.EnvironmentVariables["SECRET"] = secret); + + using var app = builder.Build(); + await app.StartAsync().DefaultTimeout(); + + await PrimeEnvironmentCallbackCacheAsync(owner.Resource, app.Services).DefaultTimeout(); + + var notificationService = app.Services.GetRequiredService(); + await notificationService.PublishUpdateAsync(owner.Resource, s => s with + { + State = new ResourceStateSnapshot("Running", KnownResourceStateStyles.Success), + EnvironmentVariables = + [ + new EnvironmentVariableSnapshot("SECRET", "value-a", true) + ] + }).DefaultTimeout(); + + // First connection observes value-a and records it in the shared, AppHost-scoped history. + var firstConnection = new AuxiliaryBackchannelRpcTarget( + NullLogger.Instance, + app.Services.GetRequiredService(), + app.Services.GetRequiredService(), + app.Services); + + var firstResult = await firstConnection.GetResourceSnapshotsAsync().DefaultTimeout(); + var firstSnapshot = Assert.Single(firstResult, r => r.Name == "owner"); + Assert.Null(Assert.Single(firstSnapshot.EnvironmentVariables, e => e.Name == "SECRET").Value); + + // The runtime replaces the parameter's resolved value with value-b (as SetParameterValue does by swapping + // the completed WaitForValueTcs), but a lagging snapshot still carries value-a. + secret.WaitForValueTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + secret.WaitForValueTcs.SetResult("value-b"); + + await notificationService.PublishUpdateAsync(owner.Resource, s => s with + { + State = new ResourceStateSnapshot("Running", KnownResourceStateStyles.Success), + EnvironmentVariables = + [ + new EnvironmentVariableSnapshot("SECRET", "value-a", true) + ] + }).DefaultTimeout(); + + // A second, independently constructed connection that never observed value-a itself must still redact it, + // because the redaction history is shared across connections for the life of the AppHost. + var secondConnection = new AuxiliaryBackchannelRpcTarget( + NullLogger.Instance, + app.Services.GetRequiredService(), + app.Services.GetRequiredService(), + app.Services); + + var secondResult = await secondConnection.GetResourceSnapshotsAsync().DefaultTimeout(); + var secondSnapshot = Assert.Single(secondResult, r => r.Name == "owner"); + + // value-a must still be redacted. With per-connection history the fresh connection would start empty and + // resolve only value-b, so the stale value-a would be emitted in plaintext. + Assert.Null(Assert.Single(secondSnapshot.EnvironmentVariables, e => e.Name == "SECRET").Value); + + await app.StopAsync().DefaultTimeout(); + } + + [Fact] + public async Task GetResourceSnapshotsAsync_RedactsSecretResolvedDuringAnotherResourceMcpDiscoveryWindow() + { + using var builder = TestDistributedApplicationBuilder.Create(outputHelper); + + // Regression test for the per-snapshot concern the reviewer identified: building a resource's snapshot can + // await MCP tool discovery for up to s_mcpDiscoveryTimeout, and a parameter can resolve during that window. + // If the redaction set is computed once for the whole describe batch (before the resource loop), a value + // resolved mid-loop is missing from it, so a LATER resource whose snapshot already carries that value leaks + // it in plaintext. Resolving the redaction set per snapshot (after the MCP await) closes the window. + var secret = new ParameterResource("mcp-window-secret", _ => "unused", secret: true); + + // The first resource exposes an MCP endpoint. Its resolver is awaited while its snapshot is built, and here + // it deterministically resolves the secret — standing in for a parameter that happens to resolve during the + // real (up to 5s) MCP discovery window. Returning null skips the network TryListToolsAsync call. + var mcpResource = builder.AddResource(new CustomResourceWithEndpoints("mcp")) + .WithAnnotation(new McpServerEndpointAnnotation((resource, cancellationToken) => + { + secret.WaitForValueTcs!.TrySetResult("leaked-during-mcp"); + return Task.FromResult(null); + })); + + // The second resource owns an environment variable carrying the secret's resolved value. It is registered + // AFTER the MCP resource so its snapshot is built after the MCP resolver has run. + var owner = builder.AddResource(new CustomResourceWithEnvironment("owner")) + .WithEnvironment(context => context.EnvironmentVariables["SECRET"] = secret); + + using var app = builder.Build(); + await app.StartAsync().DefaultTimeout(); + + // Start the secret UNRESOLVED; the MCP resolver completes it mid-loop while the "owner" snapshot is still to + // be built. (A fresh uncompleted TCS also discards any startup resolution of the referenced parameter.) + secret.WaitForValueTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + await PrimeEnvironmentCallbackCacheAsync(owner.Resource, app.Services).DefaultTimeout(); + + var notificationService = app.Services.GetRequiredService(); + await notificationService.PublishUpdateAsync(mcpResource.Resource, s => s with + { + State = new ResourceStateSnapshot("Running", KnownResourceStateStyles.Success) + }).DefaultTimeout(); + await notificationService.PublishUpdateAsync(owner.Resource, s => s with + { + State = new ResourceStateSnapshot("Running", KnownResourceStateStyles.Success), + EnvironmentVariables = + [ + new EnvironmentVariableSnapshot("SECRET", "leaked-during-mcp", true) + ] + }).DefaultTimeout(); + + var target = new AuxiliaryBackchannelRpcTarget( + NullLogger.Instance, + app.Services.GetRequiredService(), + app.Services.GetRequiredService(), + app.Services); + + var result = await target.GetResourceSnapshotsAsync().DefaultTimeout(); + + var ownerSnapshot = Assert.Single(result, r => r.Name == "owner"); + + // The owner's snapshot is built after the MCP resource resolved the secret. A per-snapshot redaction set + // observes the now-resolved value and redacts it; a batch-wide set computed before the loop would not. + Assert.Null(Assert.Single(ownerSnapshot.EnvironmentVariables, e => e.Name == "SECRET").Value); + + await app.StopAsync().DefaultTimeout(); + } + + [Fact] + public async Task GetResourceSnapshotsAsync_RedactsSecretReplacedBeforeFirstConnection() + { + using var builder = TestDistributedApplicationBuilder.Create(outputHelper); + + // Regression test for the cold-start residual the reviewer identified: a backchannel connection only observes + // a secret once it is open. A value assigned at startup and then replaced before the FIRST connection would, + // with connection-time observation alone, be absent from the redaction history — a fresh connection peeking + // the current value would find only the replacement and leak the original from a lagging snapshot. The + // parameter processor records secret values at assignment time (see its SecretRedactionHistory wiring), so + // the original value is in the history from startup, independent of any connection. + var coldSecret = builder.AddParameter("cold-secret", "value-a", secret: true); + var owner = builder.AddResource(new CustomResourceWithEnvironment("owner")) + .WithEnvironment(context => context.EnvironmentVariables["SECRET"] = coldSecret.Resource); + + using var app = builder.Build(); + + // Startup resolves the parameter to value-a; assignment-time recording adds value-a to the AppHost-scoped + // redaction history before any backchannel connection exists. + await app.StartAsync().DefaultTimeout(); + + await PrimeEnvironmentCallbackCacheAsync(owner.Resource, app.Services).DefaultTimeout(); + + // The value is replaced with value-b in place (as the runtime "Set parameter" path does). Peek-only discovery + // on a first-ever connection would now resolve only value-b, so value-a can only stay redacted via the + // assignment-time record captured at startup. + coldSecret.Resource.WaitForValueTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + coldSecret.Resource.WaitForValueTcs.SetResult("value-b"); + + var notificationService = app.Services.GetRequiredService(); + await notificationService.PublishUpdateAsync(owner.Resource, s => s with + { + State = new ResourceStateSnapshot("Running", KnownResourceStateStyles.Success), + EnvironmentVariables = + [ + new EnvironmentVariableSnapshot("SECRET", "value-a", true) + ] + }).DefaultTimeout(); + + // A brand-new connection that never observed value-a must still redact it. + var firstConnection = new AuxiliaryBackchannelRpcTarget( + NullLogger.Instance, + app.Services.GetRequiredService(), + app.Services.GetRequiredService(), + app.Services); + + var result = await firstConnection.GetResourceSnapshotsAsync().DefaultTimeout(); + var ownerSnapshot = Assert.Single(result, r => r.Name == "owner"); + + Assert.Null(Assert.Single(ownerSnapshot.EnvironmentVariables, e => e.Name == "SECRET").Value); + + await app.StopAsync().DefaultTimeout(); + } + + [Fact] + public async Task GetResourceSnapshotsAsync_DoesNotInvokeUncachedResourceCallback_AndLeavesItEvaluable() + { + using var builder = TestDistributedApplicationBuilder.Create(outputHelper); + + // Peek-only discovery must never invoke a resource callback: it reads only results DCP already cached. + // A callback that has not been cached yet is simply skipped, so describe succeeds without running it, and + // the callback is left untouched — no cached result, still evaluable exactly as authored. This replaces + // the previous "fail closed when the callback throws during discovery" test: discovery no longer invokes + // callbacks, so a throwing callback can never be reached (and thus never poisoned) by describe. + var invocations = 0; + Action throwingCallback = _ => + { + invocations++; + throw new InvalidOperationException("resource callbacks must not be invoked by describe discovery"); + }; + var owner = builder.AddResource(new CustomResourceWithEnvironment("owner")) + .WithEnvironment(throwingCallback); + + using var app = builder.Build(); + + var target = new AuxiliaryBackchannelRpcTarget( + NullLogger.Instance, + app.Services.GetRequiredService(), + app.Services.GetRequiredService(), + app.Services); + + // Describe succeeds because the uncached, throwing callback is peeked (found absent) rather than invoked. + _ = await target.GetResourceSnapshotsAsync().DefaultTimeout(); + Assert.Equal(0, invocations); + + // Discovery left the cache pristine: nothing was cached, so DCP can still evaluate the callback and it + // behaves exactly as authored (invoked once, throwing its own exception). + var annotation = owner.Resource.Annotations.OfType().Single(); + Assert.False(annotation.AsCallbackAnnotation().TryGetCachedResult(out var cached)); + Assert.Null(cached); + + var executionContext = app.Services.GetRequiredService(); + await Assert.ThrowsAsync( + async () => await annotation.AsCallbackAnnotation() + .EvaluateOnceAsync(new EnvironmentCallbackContext(executionContext, owner.Resource, new Dictionary())) + .DefaultTimeout()); + Assert.Equal(1, invocations); + } + + [Fact] + public async Task GetResourceSnapshotsAsync_DoesNotInvokeOrPoisonCallbackCache_WhenDescribeIsCanceled() + { + using var builder = TestDistributedApplicationBuilder.Create(outputHelper); + + // The precise leak adamint identified in review: previously discovery invoked resource callbacks, so a + // describe/watch cancelled while an unresolved parameter callback awaited the client's cancellation token + // cached a *cancelled* task that DCP then reused, failing the resource until the next restart. Peek-only + // discovery never invokes a callback, so even a cancelled describe cannot run it nor cache a cancelled + // task, and the annotation stays fully evaluable for DCP afterward. + var invocations = 0; + Action cancelAwareCallback = context => + { + invocations++; + // If discovery had invoked this under the cancelled describe token (old behaviour), the throw would + // have been cached as a cancelled/faulted task and reused by DCP. + context.CancellationToken.ThrowIfCancellationRequested(); + context.EnvironmentVariables["SECRET"] = "resolved"; + }; + var owner = builder.AddResource(new CustomResourceWithEnvironment("owner")) + .WithEnvironment(cancelAwareCallback); + + using var app = builder.Build(); + + var target = new AuxiliaryBackchannelRpcTarget( + NullLogger.Instance, + app.Services.GetRequiredService(), + app.Services.GetRequiredService(), + app.Services); + + using var cts = new CancellationTokenSource(); + cts.Cancel(); + + try + { + await target.GetResourceSnapshotsAsync(cts.Token).DefaultTimeout(); + } + catch (OperationCanceledException) + { + // Cancellation surfacing is acceptable; the guarantee under test is that the callback was neither + // invoked nor cached, regardless of whether the cancelled describe returns or throws. + } + + var annotation = owner.Resource.Annotations.OfType().Single(); + Assert.Equal(0, invocations); + Assert.False(annotation.AsCallbackAnnotation().TryGetCachedResult(out var cached)); + Assert.Null(cached); + + // The annotation is still evaluable: DCP evaluates it to a successful, cached result. The earlier + // cancelled describe left nothing poisoned behind, so the resource is not stuck failing until a restart. + var executionContext = app.Services.GetRequiredService(); + var evaluated = await annotation.AsCallbackAnnotation() + .EvaluateOnceAsync(new EnvironmentCallbackContext(executionContext, owner.Resource, new Dictionary())) + .DefaultTimeout(); + Assert.Equal(1, invocations); + Assert.True(annotation.AsCallbackAnnotation().TryGetCachedResult(out var cachedAfter)); + Assert.True(cachedAfter!.IsCompletedSuccessfully); + Assert.Equal("resolved", evaluated["SECRET"]); + } + [Fact] public async Task GetResourceSnapshotsAsync_DoesNotBlockOnUnresolvedSecretParameter() { using var builder = TestDistributedApplicationBuilder.Create(outputHelper); - var secret = builder.AddParameter("dbpassword", "s3cr3t-value", secret: true); - var custom = builder.AddResource(new CustomResource("myresource")); + // A secret parameter that is only referenced by a resource (never registered as a top-level parameter) so it + // is not resolved — and therefore not recorded for redaction — during run-mode startup. This lets the test + // exercise a genuinely unresolved secret at describe time. + var secret = new ParameterResource("dbpassword", _ => "s3cr3t-value", secret: true); + var owner = builder.AddResource(new CustomResourceWithEnvironment("myresource")) + .WithEnvironment(context => context.EnvironmentVariables["DB_PASSWORD"] = secret); using var app = builder.Build(); await app.StartAsync().DefaultTimeout(); - // Simulate a secret parameter whose value has not been resolved yet by replacing its completion - // source with one that never completes. GetResolvedSecretParameterValues must peek (not await) the - // task, so an unresolved secret cannot block the snapshot call. - secret.Resource.WaitForValueTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + // Leave the secret unresolved: its completion source never completes. GetResolvedSecretParameterValues must + // peek (not await) the task, so an unresolved secret cannot block the snapshot call. + secret.WaitForValueTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + // Cache the env callback the way DCP does on start, so peek-only discovery can observe (and then skip) the + // unresolved secret reference. + await PrimeEnvironmentCallbackCacheAsync(owner.Resource, app.Services).DefaultTimeout(); var notificationService = app.Services.GetRequiredService(); - await notificationService.PublishUpdateAsync(custom.Resource, s => s with + await notificationService.PublishUpdateAsync(owner.Resource, s => s with { State = new ResourceStateSnapshot("Running", KnownResourceStateStyles.Success), EnvironmentVariables = @@ -414,7 +1052,7 @@ await notificationService.PublishUpdateAsync(custom.Resource, s => s with var snapshot = Assert.Single(result, r => r.Name == "myresource"); var dbPassword = Assert.Single(snapshot.EnvironmentVariables, e => e.Name == "DB_PASSWORD"); - // Because the secret value was not resolved, it is not part of the redaction set: the unresolved + // Because the secret value was never resolved, it is not part of the redaction set: the unresolved // secret is skipped rather than awaited. Once resolved it is redacted (see the streaming test). Assert.Equal("s3cr3t-value", dbPassword.Value); @@ -426,15 +1064,21 @@ public async Task WatchResourceSnapshotsAsync_RedactsSecretResolvedAfterWatchSta { using var builder = TestDistributedApplicationBuilder.Create(outputHelper); - var secret = builder.AddParameter("dbpassword", "s3cr3t-value", secret: true); - var custom = builder.AddResource(new CustomResource("myresource")); + // A secret parameter referenced by the resource (not a top-level parameter), so it is not resolved or + // recorded for redaction during run-mode startup and the watch can begin with it genuinely unresolved. + var secret = new ParameterResource("dbpassword", _ => "s3cr3t-value", secret: true); + var owner = builder.AddResource(new CustomResourceWithEnvironment("myresource")) + .WithEnvironment(context => context.EnvironmentVariables["DB_PASSWORD"] = secret); using var app = builder.Build(); await app.StartAsync().DefaultTimeout(); // Begin with the secret unresolved so the watch starts before the value is known. var waitForValueTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - secret.Resource.WaitForValueTcs = waitForValueTcs; + secret.WaitForValueTcs = waitForValueTcs; + + // Cache the env callback the way DCP does on start, so peek-only discovery can observe the secret reference. + await PrimeEnvironmentCallbackCacheAsync(owner.Resource, app.Services).DefaultTimeout(); var notificationService = app.Services.GetRequiredService(); @@ -449,7 +1093,7 @@ public async Task WatchResourceSnapshotsAsync_RedactsSecretResolvedAfterWatchSta try { // Phase 1: secret unresolved. The env var value is not redacted because the secret value is unknown. - await notificationService.PublishUpdateAsync(custom.Resource, s => s with + await notificationService.PublishUpdateAsync(owner.Resource, s => s with { State = new ResourceStateSnapshot("Running", KnownResourceStateStyles.Success), EnvironmentVariables = @@ -465,7 +1109,7 @@ await notificationService.PublishUpdateAsync(custom.Resource, s => s with // Resolve the secret mid-stream, then push a new event whose env var now matches the secret value. waitForValueTcs.SetResult("s3cr3t-value"); - await notificationService.PublishUpdateAsync(custom.Resource, s => s with + await notificationService.PublishUpdateAsync(owner.Resource, s => s with { State = new ResourceStateSnapshot("Running", KnownResourceStateStyles.Success), EnvironmentVariables = @@ -548,6 +1192,21 @@ private static bool HasEnvironmentVariable(ResourceSnapshot snapshot, string nam private static string? GetEnvironmentVariableValue(ResourceSnapshot snapshot, string name) => snapshot.EnvironmentVariables.Single(e => e.Name == name).Value; + // Mirror what DCP does when it starts a resource: evaluate the resource's environment callbacks once so the + // result is cached. Peek-only secret discovery (used by describe/watch) never invokes callbacks itself — it + // only reads results DCP already cached — so a test must prime that cache the same way a real run would before + // a secret referenced through a callback can be discovered and redacted. + private static async Task PrimeEnvironmentCallbackCacheAsync(IResource resource, IServiceProvider services) + { + var executionContext = services.GetRequiredService(); + foreach (var annotation in resource.Annotations.OfType()) + { + await annotation.AsCallbackAnnotation() + .EvaluateOnceAsync(new EnvironmentCallbackContext(executionContext, resource, new Dictionary())) + .ConfigureAwait(false); + } + } + [Fact] public async Task WaitForResourceAsync_ReturnsFailureWhenResourceHasErrorStateStyle() { @@ -933,6 +1592,14 @@ private sealed class CustomResource(string name) : Resource(name) { } + private sealed class CustomResourceWithEnvironment(string name) : Resource(name), IResourceWithEnvironment + { + } + + private sealed class CustomResourceWithEndpoints(string name) : Resource(name), IResourceWithEndpoints + { + } + // Synthesise per-replica terminal layouts directly rather than going through the public // WithTerminal() path so the test stays focused on backchannel snapshot stamping and doesn't // depend on real DCP terminal-host provisioning. Mirrors DashboardServiceDataTerminalTests. diff --git a/tests/Aspire.Hosting.Tests/Orchestrator/ParameterProcessorTests.cs b/tests/Aspire.Hosting.Tests/Orchestrator/ParameterProcessorTests.cs index 3049f21331e..762e4cc647b 100644 --- a/tests/Aspire.Hosting.Tests/Orchestrator/ParameterProcessorTests.cs +++ b/tests/Aspire.Hosting.Tests/Orchestrator/ParameterProcessorTests.cs @@ -4,6 +4,7 @@ using System.Globalization; using System.Text.Json.Nodes; using Aspire.Dashboard.Model; +using Aspire.Hosting.Backchannel; using Aspire.Hosting.Pipelines; using Aspire.Hosting.Pipelines.Internal; using Aspire.Hosting.Resources; @@ -1174,6 +1175,50 @@ public async Task SetParameterAsync_WithUserInput_UpdatesParameterValueAndSavedS Assert.Equal("newValue", savedValueNode?.GetValue()); } + [Fact] + public async Task InitializeParametersAsync_RecordsResolvedSecretValues_ForRedaction() + { + // Assignment-time recording (https://github.com/microsoft/aspire/issues/19241): the processor records a + // secret's resolved value into the AppHost-scoped redaction history the moment it assigns it, so + // aspire describe/watch can redact it even before any backchannel connection has observed the value. + var redactionHistory = new SecretRedactionHistory(); + var parameterProcessor = CreateParameterProcessor(secretRedactionHistory: redactionHistory); + + var secretParam = CreateParameterResource("db-password", "s3cr3t-value", secret: true); + var nonSecretParam = CreateParameterResource("region", "public-value", secret: false); + + await parameterProcessor.InitializeParametersAsync([secretParam, nonSecretParam], waitForResolution: true).DefaultTimeout(); + + // Only the secret value is recorded; the non-secret value must never enter the redaction set (asserting a + // single element proves "public-value" was not recorded). + var recorded = redactionHistory.AddValuesAndSnapshot([]); + Assert.Equal("s3cr3t-value", Assert.Single(recorded)); + } + + [Fact] + public async Task SetParameterCoreAsync_RecordsReplacedSecretValue_ForRedaction() + { + // The runtime "Set parameter" path replaces a secret's resolved value in place. Both the original and the + // replacement value must stay redactable, because a still-current or lagging snapshot can carry the previous + // value. Recording at assignment time accumulates both add-only, independent of any backchannel connection. + var redactionHistory = new SecretRedactionHistory(); + var parameterProcessor = CreateParameterProcessor(secretRedactionHistory: redactionHistory); + + var secretParam = CreateParameterResource("db-password", "initial-secret", secret: true); + + await parameterProcessor.InitializeParametersAsync([secretParam], waitForResolution: true).DefaultTimeout(); + + // Recreate the completed TCS the way the runtime does before replacing a resolved value. + secretParam.WaitForValueTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + var result = await parameterProcessor.SetParameterCoreAsync(secretParam, CreateSetParameterArguments("replacement-secret"), CancellationToken.None).DefaultTimeout(); + Assert.True(result.Success); + + var recorded = redactionHistory.AddValuesAndSnapshot([]); + Assert.Contains("initial-secret", recorded); + Assert.Contains("replacement-secret", recorded); + } + private static InteractionInputCollection CreateSetParameterArguments(string? value, string? saveToUserSecrets = null) { return new InteractionInputCollection([ @@ -1212,7 +1257,8 @@ private static ParameterProcessor CreateParameterProcessor( bool disableDashboard = true, DistributedApplicationExecutionContext? executionContext = null, IDeploymentStateManager? deploymentStateManager = null, - IUserSecretsManager? userSecretsManager = null) + IUserSecretsManager? userSecretsManager = null, + SecretRedactionHistory? secretRedactionHistory = null) { return new ParameterProcessor( notificationService ?? ResourceNotificationServiceTestHelpers.Create(), @@ -1221,8 +1267,13 @@ private static ParameterProcessor CreateParameterProcessor( logger ?? new NullLogger(), executionContext ?? new DistributedApplicationExecutionContext(DistributedApplicationOperation.Run), deploymentStateManager ?? new MockDeploymentStateManager(), - userSecretsManager ?? UserSecrets.NoopUserSecretsManager.Instance - ); + userSecretsManager ?? UserSecrets.NoopUserSecretsManager.Instance) + { + // Mirror the DI wiring in DistributedApplicationBuilder so tests can observe assignment-time secret + // recording. Left null by default (property is a no-op then), matching a processor created outside the + // AppHost container. + SecretRedactionHistory = secretRedactionHistory + }; } private static InteractionService CreateInteractionService(bool disableDashboard = false) diff --git a/tests/Aspire.Hosting.Tests/ResourceDependencyTests.cs b/tests/Aspire.Hosting.Tests/ResourceDependencyTests.cs index a1eb2b32c2d..db787e8b029 100644 --- a/tests/Aspire.Hosting.Tests/ResourceDependencyTests.cs +++ b/tests/Aspire.Hosting.Tests/ResourceDependencyTests.cs @@ -234,6 +234,45 @@ public async Task ParameterInLaunchToolArgsIsIncluded(bool cacheAnnotationCallba Assert.Collection(dependencies, dependency => Assert.Same(param.Resource, dependency)); } + [Fact] + public async Task PeekCachedCallbackResultsOnly_OnlySeesCachedResultsAndNeverInvokesCallback() + { + using var builder = TestDistributedApplicationBuilder.Create(); + + var param = builder.AddParameter("config"); + var invocations = 0; + var executable = builder.AddExecutable("app", "myapp", ".") + .WithEnvironment(context => + { + invocations++; + context.EnvironmentVariables["CONFIG"] = param.Resource; + }); + + var executionContext = new DistributedApplicationExecutionContext(DistributedApplicationOperation.Run); + var peekOptions = new ResourceDependencyDiscoveryOptions + { + DiscoveryMode = ResourceDependencyDiscoveryMode.DirectOnly, + PeekCachedCallbackResultsOnly = true + }; + + // Nothing has evaluated/cached the callback yet, so peek-only discovery must not invoke it and must not + // discover the referenced parameter — it only reads results that were already cached. + var beforePriming = await executable.Resource.GetResourceDependenciesAsync(executionContext, peekOptions); + Assert.Equal(0, invocations); + Assert.Empty(beforePriming); + + // Mirror what DCP does when it starts the resource: evaluate the callback once so its result is cached. + var annotation = executable.Resource.Annotations.OfType().Single(); + await annotation.AsCallbackAnnotation().EvaluateOnceAsync( + new EnvironmentCallbackContext(executionContext, executable.Resource, new Dictionary())); + Assert.Equal(1, invocations); + + // Peek-only discovery now surfaces the cached reference without invoking the callback a second time. + var afterPriming = await executable.Resource.GetResourceDependenciesAsync(executionContext, peekOptions); + Assert.Equal(1, invocations); + Assert.Contains(param.Resource, afterPriming); + } + [Fact] public async Task OnlyLastLaunchToolArgsAnnotationContributesDependencies() {