From c81c3e966cb6aad505f095ed840e00320df6d282 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Mon, 21 Sep 2026 13:33:54 +0900 Subject: [PATCH 1/2] Bound policy transport and trust stages --- .../AuthenticatedBrokerTransport.cs | 188 +++---------- ...UniGetUI.AgentPolicy.ElevatedHelper.csproj | 2 + .../BrokerPolicyManagementModels.cs | 9 + .../BrokerPolicyManagementService.cs | 5 +- .../PolicyElevationPreflightRunner.cs | 96 +++++-- .../Protocol/PolicyElevationProtocol.cs | 3 + .../Shared/BoundedNamedPipeBrokerTransport.cs | 253 ++++++++++++++++++ .../WindowsPolicyElevationPreflight.cs | 1 + .../WindowsPolicyWriteElevator.cs | 16 +- .../BrokerPolicyTransportTests.cs | 59 +++- .../AuthenticatedBrokerTransportTests.cs | 54 ++++ .../PolicyElevationHelperLocatorTests.cs | 39 +++ .../WindowsPolicyWriteElevatorTests.cs | 51 ++++ 13 files changed, 590 insertions(+), 186 deletions(-) create mode 100644 src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/Shared/BoundedNamedPipeBrokerTransport.cs diff --git a/src/UniGetUI.AgentPolicy.ElevatedHelper/AuthenticatedBrokerTransport.cs b/src/UniGetUI.AgentPolicy.ElevatedHelper/AuthenticatedBrokerTransport.cs index 31b21c0b0b..6f45e50335 100644 --- a/src/UniGetUI.AgentPolicy.ElevatedHelper/AuthenticatedBrokerTransport.cs +++ b/src/UniGetUI.AgentPolicy.ElevatedHelper/AuthenticatedBrokerTransport.cs @@ -1,6 +1,5 @@ using System.IO.Pipes; using System.Security.Principal; -using System.Text; using Devolutions.Now.Policy.Api; using Devolutions.Now.Policy.Client; using Microsoft.Win32.SafeHandles; @@ -11,12 +10,10 @@ namespace UniGetUI.AgentPolicy.ElevatedHelper; internal sealed class AuthenticatedBrokerTransport : IBrokerTransport { - private const string DefaultPipeName = "Devolutions.Now.PackageBroker.v1"; private const int ConnectTimeoutMilliseconds = 5000; private const int ReadTimeoutMilliseconds = 30000; - private const int MaxHeaderBytes = 65536; internal const int MaxPolicyManagementResponseBytes = - BrokerApi.MaxPolicyManagementBodyBytes * 3 + MaxHeaderBytes; + BoundedNamedPipeBrokerTransport.MaxPolicyManagementResponseBodyBytes; private readonly string _pipeName; private readonly Func _authenticate; @@ -29,7 +26,7 @@ internal AuthenticatedBrokerTransport( string? pipeName, Func authenticate) { - _pipeName = string.IsNullOrWhiteSpace(pipeName) ? DefaultPipeName : pipeName; + _pipeName = string.IsNullOrWhiteSpace(pipeName) ? BrokerApi.DefaultPipeName : pipeName; _authenticate = authenticate; } @@ -40,15 +37,15 @@ public async Task Send( CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(request); - + var pipe = new NamedPipeClientStream( + ".", + _pipeName, + PipeDirection.InOut, + PipeOptions.Asynchronous, + TokenImpersonationLevel.Identification); + bool cleanupTransferred = false; try { - using var pipe = new NamedPipeClientStream( - ".", - _pipeName, - PipeDirection.InOut, - PipeOptions.Asynchronous, - TokenImpersonationLevel.Identification); using (CancellationTokenSource connectCancellation = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken)) { @@ -56,15 +53,36 @@ public async Task Send( await pipe.ConnectAsync(connectCancellation.Token).ConfigureAwait(false); } - using IDisposable server = _authenticate(pipe, request.Path); - await WriteRequestAsync(pipe, request, cancellationToken).ConfigureAwait(false); + PolicyElevationHelperSynchronousStageResult authentication = + await PolicyElevationHelperSynchronousStageRunner.RunAsync( + () => _authenticate(pipe, request.Path), + cancellationToken, + static abandoned => abandoned.Dispose(), + pipe.Dispose).ConfigureAwait(false); + if (!authentication.Completed) + { + // Authentication may still hold the pipe handle. Its continuation now owns both the + // eventual authentication result and the pipe, so the helper can stop waiting safely. + cleanupTransferred = true; + cancellationToken.ThrowIfCancellationRequested(); + throw BrokerFailure( + BrokerClientErrorKind.Timeout, + $"Timed out authenticating the package broker at {request.Path}.", + request.Path); + } + using IDisposable server = authentication.Value; + await BoundedNamedPipeBrokerTransport + .WriteRequestAsync(pipe, request, cancellationToken) + .ConfigureAwait(false); using CancellationTokenSource readCancellation = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); readCancellation.CancelAfter(ReadTimeoutMilliseconds); - return await ReadResponseAsync( + return await BoundedNamedPipeBrokerTransport.ReadResponseAsync( pipe, request.Path, + MaxPolicyManagementResponseBytes, + BrokerClientErrorKind.BrokerUnavailable, readCancellation.Token).ConfigureAwait(false); } catch (OperationCanceledException ex) when (!cancellationToken.IsCancellationRequested) @@ -91,147 +109,15 @@ public async Task Send( request.Path, ex); } - } - - public void Dispose() - { - } - - private static async Task WriteRequestAsync( - Stream pipe, - BrokerTransportRequest request, - CancellationToken cancellationToken) - { - var headers = new StringBuilder() - .Append(request.Method) - .Append(' ') - .Append(request.Path) - .Append(" HTTP/1.1\r\n") - .Append("Host: now-package-broker\r\n") - .Append("Connection: close\r\n"); - foreach ((string name, string value) in request.Headers) + finally { - if (!name.Equals("Host", StringComparison.OrdinalIgnoreCase)) - { - headers.Append(name).Append(": ").Append(value).Append("\r\n"); - } + if (!cleanupTransferred) + pipe.Dispose(); } - - byte[]? body = request.Body is null ? null : Encoding.UTF8.GetBytes(request.Body); - headers.Append("Content-Length: ").Append(body?.Length ?? 0).Append("\r\n\r\n"); - await pipe.WriteAsync( - Encoding.ASCII.GetBytes(headers.ToString()), - cancellationToken).ConfigureAwait(false); - if (body is not null) - { - await pipe.WriteAsync(body, cancellationToken).ConfigureAwait(false); - } - - await pipe.FlushAsync(cancellationToken).ConfigureAwait(false); } - private static async Task ReadResponseAsync( - Stream pipe, - string path, - CancellationToken cancellationToken) + public void Dispose() { - byte[] buffer = new byte[MaxHeaderBytes]; - int totalRead = 0; - while (totalRead < MaxHeaderBytes) - { - int read = await pipe.ReadAsync( - buffer.AsMemory(totalRead, MaxHeaderBytes - totalRead), - cancellationToken).ConfigureAwait(false); - if (read == 0) - { - throw BrokerFailure( - BrokerClientErrorKind.BrokerUnavailable, - $"The package broker disconnected before sending a complete response for {path}.", - path); - } - - totalRead += read; - string received = Encoding.ASCII.GetString(buffer, 0, totalRead); - int headerEnd = received.IndexOf("\r\n\r\n", StringComparison.Ordinal); - if (headerEnd < 0) - { - continue; - } - - string[] lines = received[..headerEnd].Split("\r\n"); - string[] status = lines[0].Split(' ', 3, StringSplitOptions.RemoveEmptyEntries); - if (status.Length < 2 || !int.TryParse(status[1], out int statusCode)) - { - throw BrokerFailure( - BrokerClientErrorKind.InvalidResponse, - $"The package broker returned an invalid HTTP status line for {path}.", - path); - } - - int? contentLength = null; - for (int index = 1; index < lines.Length; index++) - { - int separator = lines[index].IndexOf(':'); - if (separator <= 0) - { - continue; - } - - string name = lines[index][..separator].Trim(); - if (!name.Equals("Content-Length", StringComparison.OrdinalIgnoreCase)) - { - continue; - } - - if (contentLength is not null - || !int.TryParse(lines[index][(separator + 1)..].Trim(), out int parsed) - || parsed < 0 - || parsed > MaxPolicyManagementResponseBytes) - { - throw BrokerFailure( - BrokerClientErrorKind.InvalidResponse, - $"The package broker returned an invalid Content-Length for {path}.", - path); - } - - contentLength = parsed; - } - - int bodyLength = contentLength ?? 0; - int bodyStart = headerEnd + 4; - if (bodyStart + bodyLength > buffer.Length) - { - Array.Resize(ref buffer, bodyStart + bodyLength); - } - - int bodyRead = totalRead - bodyStart; - while (bodyRead < bodyLength) - { - read = await pipe.ReadAsync( - buffer.AsMemory(bodyStart + bodyRead, bodyLength - bodyRead), - cancellationToken).ConfigureAwait(false); - if (read == 0) - { - throw BrokerFailure( - BrokerClientErrorKind.BrokerUnavailable, - $"The package broker disconnected before sending the complete response body for {path}.", - path); - } - - bodyRead += read; - } - - return new BrokerTransportResponse - { - StatusCode = statusCode, - Body = Encoding.UTF8.GetString(buffer, bodyStart, bodyLength), - }; - } - - throw BrokerFailure( - BrokerClientErrorKind.InvalidResponse, - $"The package broker returned response headers that are too large for {path}.", - path); } private static BrokerClientException BrokerFailure( diff --git a/src/UniGetUI.AgentPolicy.ElevatedHelper/UniGetUI.AgentPolicy.ElevatedHelper.csproj b/src/UniGetUI.AgentPolicy.ElevatedHelper/UniGetUI.AgentPolicy.ElevatedHelper.csproj index a900ce6581..130a1c8477 100644 --- a/src/UniGetUI.AgentPolicy.ElevatedHelper/UniGetUI.AgentPolicy.ElevatedHelper.csproj +++ b/src/UniGetUI.AgentPolicy.ElevatedHelper/UniGetUI.AgentPolicy.ElevatedHelper.csproj @@ -46,6 +46,8 @@ Link="Shared\Protocol\%(Filename)%(Extension)" /> + diff --git a/src/UniGetUI.PackageEngine.AgentBroker/PolicyManagement/BrokerPolicyManagementModels.cs b/src/UniGetUI.PackageEngine.AgentBroker/PolicyManagement/BrokerPolicyManagementModels.cs index 4e8ac6dbb9..f7a4bae535 100644 --- a/src/UniGetUI.PackageEngine.AgentBroker/PolicyManagement/BrokerPolicyManagementModels.cs +++ b/src/UniGetUI.PackageEngine.AgentBroker/PolicyManagement/BrokerPolicyManagementModels.cs @@ -1,4 +1,5 @@ using Devolutions.Now.Policy.Api; +using UniGetUI.PackageEngine.AgentBroker.PolicyWriteElevation; namespace UniGetUI.PackageEngine.AgentBroker.PolicyManagement; @@ -58,6 +59,14 @@ public static class BrokerPolicyManagementLimits /// public const int MaxRequestBodyBytes = BrokerApi.MaxPolicyManagementBodyBytes; + /// + /// Maximum UTF-8 response body accepted for any policy-management call. A successful replacement + /// can contain three copies of policy content (parsed policy, canonical draft, and management + /// snapshot), plus a fourth full contract budget for the response envelope, findings, and metadata. + /// + public const int MaxResponseBodyBytes = + BoundedNamedPipeBrokerTransport.MaxPolicyManagementResponseBodyBytes; + /// Maximum length (in Unicode scalar values) kept for the sanitized configured-path diagnostic field. public const int MaxSanitizedPathLength = 4096; diff --git a/src/UniGetUI.PackageEngine.AgentBroker/PolicyManagement/BrokerPolicyManagementService.cs b/src/UniGetUI.PackageEngine.AgentBroker/PolicyManagement/BrokerPolicyManagementService.cs index 037df8c13f..97cfa8b74b 100644 --- a/src/UniGetUI.PackageEngine.AgentBroker/PolicyManagement/BrokerPolicyManagementService.cs +++ b/src/UniGetUI.PackageEngine.AgentBroker/PolicyManagement/BrokerPolicyManagementService.cs @@ -6,6 +6,7 @@ using Devolutions.Now.Policy.Api; using Devolutions.Now.Policy.Client; using UniGetUI.Core.Logging; +using UniGetUI.PackageEngine.AgentBroker.PolicyWriteElevation; using ApiElevation = Devolutions.Now.Policy.Api.Elevation; namespace UniGetUI.PackageEngine.AgentBroker.PolicyManagement; @@ -36,7 +37,9 @@ public BrokerPolicyManagementService() } private static BrokerClient CreateStandardClient() => - BrokerClientFactory.Create(ApiElevation.Standard); + BrokerClientFactory.Create( + ApiElevation.Standard, + new BoundedNamedPipeBrokerTransport(BrokerPolicyManagementLimits.MaxResponseBodyBytes)); public BrokerPolicyManagementService(Func clientFactory, Func? isWindows = null) { diff --git a/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/PolicyElevationPreflightRunner.cs b/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/PolicyElevationPreflightRunner.cs index d1606d1cd8..4295782a98 100644 --- a/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/PolicyElevationPreflightRunner.cs +++ b/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/PolicyElevationPreflightRunner.cs @@ -5,19 +5,41 @@ internal static class PolicyElevationPreflightRunner { private static readonly SemaphoreSlim WorkerGate = new(1, 1); + public static Task VerifyAsync( + IPolicyElevationPreflight preflight, + CancellationToken cancellationToken) => + VerifyAsync(preflight, PolicyElevationProtocol.PreflightTimeout, cancellationToken); + public static async Task VerifyAsync( IPolicyElevationPreflight preflight, + TimeSpan timeout, CancellationToken cancellationToken) { - await WorkerGate.WaitAsync(cancellationToken).ConfigureAwait(false); + ArgumentNullException.ThrowIfNull(preflight); + if (timeout <= TimeSpan.Zero || timeout == Timeout.InfiniteTimeSpan) + throw new ArgumentOutOfRangeException(nameof(timeout)); + + using var deadline = new CancellationTokenSource(timeout); + using CancellationTokenSource boundedCancellation = + CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, deadline.Token); + try + { + await WorkerGate.WaitAsync(boundedCancellation.Token).ConfigureAwait(false); + } + catch (OperationCanceledException) when ( + !cancellationToken.IsCancellationRequested && deadline.IsCancellationRequested) + { + return TimedOut(); + } + Task worker; try { worker = Task.Run( () => { - cancellationToken.ThrowIfCancellationRequested(); - return preflight.Verify(cancellationToken); + boundedCancellation.Token.ThrowIfCancellationRequested(); + return preflight.Verify(boundedCancellation.Token); }, CancellationToken.None); } @@ -30,37 +52,57 @@ public static async Task VerifyAsync( try { PolicyElevationPreflightResult result = - await worker.WaitAsync(cancellationToken).ConfigureAwait(false); + await worker.WaitAsync(boundedCancellation.Token).ConfigureAwait(false); WorkerGate.Release(); return result; } + catch (OperationCanceledException) when ( + !cancellationToken.IsCancellationRequested && deadline.IsCancellationRequested) + { + ReleaseGateAfterWorker(worker); + return TimedOut(); + } catch { - Task release = worker.ContinueWith( - static completed => - { - try - { - if (completed.Status == TaskStatus.RanToCompletion) - completed.Result.Dispose(); - else - _ = completed.Exception; - } - finally - { - WorkerGate.Release(); - } - }, - CancellationToken.None, - TaskContinuationOptions.ExecuteSynchronously, - TaskScheduler.Default); - _ = release.ContinueWith( - static faulted => _ = faulted.Exception, - CancellationToken.None, - TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously, - TaskScheduler.Default); + ReleaseGateAfterWorker(worker); throw; } } + + private static PolicyElevationPreflightResult TimedOut() + { + const string reason = "Security verification of the packaged policy write helper timed out."; + return PolicyElevationPreflightResult.Rejected( + PolicyElevationHelperLocation.NotFound(reason), + PolicyElevationPreflightFailureKind.TimedOut, + reason); + } + + private static void ReleaseGateAfterWorker(Task worker) + { + Task release = worker.ContinueWith( + static completed => + { + try + { + if (completed.Status == TaskStatus.RanToCompletion) + completed.Result.Dispose(); + else + _ = completed.Exception; + } + finally + { + WorkerGate.Release(); + } + }, + CancellationToken.None, + TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); + _ = release.ContinueWith( + static faulted => _ = faulted.Exception, + CancellationToken.None, + TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); + } } #endif diff --git a/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/Protocol/PolicyElevationProtocol.cs b/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/Protocol/PolicyElevationProtocol.cs index 930139f2fa..852c408ce7 100644 --- a/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/Protocol/PolicyElevationProtocol.cs +++ b/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/Protocol/PolicyElevationProtocol.cs @@ -177,6 +177,9 @@ public static class PolicyElevationProtocol // ---- Timeouts -------------------------------------------------------------------------- + /// Maximum time allowed for non-elevated helper location and trust preflight. + public static readonly TimeSpan PreflightTimeout = TimeSpan.FromSeconds(45); + /// How long the host waits for the elevated helper to connect after consent was granted. public static readonly TimeSpan ConnectTimeout = TimeSpan.FromSeconds(45); diff --git a/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/Shared/BoundedNamedPipeBrokerTransport.cs b/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/Shared/BoundedNamedPipeBrokerTransport.cs new file mode 100644 index 0000000000..35da95f85f --- /dev/null +++ b/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/Shared/BoundedNamedPipeBrokerTransport.cs @@ -0,0 +1,253 @@ +using System.Globalization; +using System.IO.Pipes; +using System.Security.Principal; +using System.Text; +using Devolutions.Now.Policy.Api; +using Devolutions.Now.Policy.Client; + +namespace UniGetUI.PackageEngine.AgentBroker.PolicyWriteElevation; + +/// +/// HTTP/1.1-over-named-pipe transport with a caller-supplied response-body budget. +/// The fixed-length broker protocol lets the transport reject an oversized response before +/// allocating its body buffer or handing JSON to . +/// +internal sealed class BoundedNamedPipeBrokerTransport : IBrokerTransport +{ + private const int ConnectTimeoutMilliseconds = 5000; + private const int ReadTimeoutMilliseconds = 30000; + private const int MaxHeaderBytes = 65536; + // A replacement response can contain the accepted policy three times: the parsed policy, + // Validation.CanonicalDraft, and Management.Policy. A fourth full contract budget covers the + // response envelope, findings, and metadata without constraining any one policy below 16 MiB. + internal const int MaxPolicyManagementResponseBodyBytes = + BrokerApi.MaxPolicyManagementBodyBytes * 4; + + private readonly string _pipeName; + private readonly int _maxResponseBodyBytes; + private readonly TokenImpersonationLevel _impersonationLevel; + + public BoundedNamedPipeBrokerTransport( + int maxResponseBodyBytes, + string? pipeName = null, + TokenImpersonationLevel impersonationLevel = TokenImpersonationLevel.None) + { + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(maxResponseBodyBytes); + _maxResponseBodyBytes = maxResponseBodyBytes; + _pipeName = string.IsNullOrWhiteSpace(pipeName) ? BrokerApi.DefaultPipeName : pipeName; + _impersonationLevel = impersonationLevel; + } + + public Transport Kind => Transport.HttpNamedPipe; + + public async Task Send( + BrokerTransportRequest request, + CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(request); + try + { + using var pipe = new NamedPipeClientStream( + ".", _pipeName, PipeDirection.InOut, PipeOptions.Asynchronous, _impersonationLevel); + using (CancellationTokenSource connectCancellation = + CancellationTokenSource.CreateLinkedTokenSource(cancellationToken)) + { + connectCancellation.CancelAfter(ConnectTimeoutMilliseconds); + await pipe.ConnectAsync(connectCancellation.Token).ConfigureAwait(false); + } + + await WriteRequestAsync(pipe, request, cancellationToken).ConfigureAwait(false); + using CancellationTokenSource readCancellation = + CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + readCancellation.CancelAfter(ReadTimeoutMilliseconds); + return await ReadResponseAsync( + pipe, + request.Path, + _maxResponseBodyBytes, + BrokerClientErrorKind.InvalidResponse, + readCancellation.Token).ConfigureAwait(false); + } + catch (OperationCanceledException ex) when (!cancellationToken.IsCancellationRequested) + { + throw BrokerFailure( + BrokerClientErrorKind.Timeout, + $"Timed out communicating with the package broker at {request.Path}.", + request.Path, + ex); + } + catch (IOException ex) + { + throw BrokerFailure( + BrokerClientErrorKind.BrokerUnavailable, + $"Unable to communicate with the package broker at {request.Path}.", + request.Path, + ex); + } + catch (UnauthorizedAccessException ex) + { + throw BrokerFailure( + BrokerClientErrorKind.BrokerUnavailable, + $"Access to the package broker was denied while calling {request.Path}.", + request.Path, + ex); + } + } + + public void Dispose() + { + } + + internal static async Task WriteRequestAsync( + Stream pipe, + BrokerTransportRequest request, + CancellationToken cancellationToken) + { + var headers = new StringBuilder() + .Append(request.Method).Append(' ').Append(request.Path).Append(" HTTP/1.1\r\n") + .Append("Host: now-package-broker\r\n") + .Append("Connection: close\r\n"); + foreach ((string name, string value) in request.Headers) + { + if (!name.Equals("Host", StringComparison.OrdinalIgnoreCase)) + headers.Append(name).Append(": ").Append(value).Append("\r\n"); + } + + byte[]? body = request.Body is null ? null : Encoding.UTF8.GetBytes(request.Body); + headers.Append("Content-Length: ").Append(body?.Length ?? 0).Append("\r\n\r\n"); + await pipe.WriteAsync(Encoding.ASCII.GetBytes(headers.ToString()), cancellationToken) + .ConfigureAwait(false); + if (body is not null) + await pipe.WriteAsync(body, cancellationToken).ConfigureAwait(false); + await pipe.FlushAsync(cancellationToken).ConfigureAwait(false); + } + + internal static async Task ReadResponseAsync( + Stream pipe, + string path, + int maxResponseBodyBytes, + BrokerClientErrorKind incompleteResponseKind, + CancellationToken cancellationToken) + { + byte[] buffer = new byte[MaxHeaderBytes]; + int totalRead = 0; + while (totalRead < MaxHeaderBytes) + { + int read = await pipe.ReadAsync( + buffer.AsMemory(totalRead, MaxHeaderBytes - totalRead), cancellationToken) + .ConfigureAwait(false); + if (read == 0) + { + throw BrokerFailure( + incompleteResponseKind, + $"The package broker disconnected before sending a complete response for {path}.", + path); + } + + totalRead += read; + string received = Encoding.ASCII.GetString(buffer, 0, totalRead); + int headerEnd = received.IndexOf("\r\n\r\n", StringComparison.Ordinal); + if (headerEnd < 0) + continue; + + string[] lines = received[..headerEnd].Split("\r\n"); + string[] status = lines[0].Split(' ', 3, StringSplitOptions.RemoveEmptyEntries); + if (status.Length < 2 + || !int.TryParse(status[1], NumberStyles.None, CultureInfo.InvariantCulture, out int statusCode)) + { + throw BrokerFailure( + BrokerClientErrorKind.InvalidResponse, + $"The package broker returned an invalid HTTP status line for {path}.", + path); + } + + int? contentLength = null; + for (int index = 1; index < lines.Length; index++) + { + int separator = lines[index].IndexOf(':'); + if (separator <= 0) + continue; + + string name = lines[index][..separator].Trim(); + if (!name.Equals("Content-Length", StringComparison.OrdinalIgnoreCase)) + continue; + + if (contentLength is not null + || !int.TryParse( + lines[index][(separator + 1)..].Trim(), + NumberStyles.None, + CultureInfo.InvariantCulture, + out int parsed) + || parsed < 0 + || parsed > maxResponseBodyBytes) + { + throw BrokerFailure( + BrokerClientErrorKind.InvalidResponse, + $"The package broker returned an invalid or oversized Content-Length for {path}.", + path, + statusCode: statusCode); + } + + contentLength = parsed; + } + + if (contentLength is null) + { + throw BrokerFailure( + BrokerClientErrorKind.InvalidResponse, + $"The package broker response omitted Content-Length for {path}.", + path, + statusCode: statusCode); + } + + int bodyLength = contentLength.Value; + int bodyStart = headerEnd + 4; + int bodyRead = totalRead - bodyStart; + if (bodyRead > bodyLength) + { + throw BrokerFailure( + BrokerClientErrorKind.InvalidResponse, + $"The package broker returned more response data than declared for {path}.", + path, + statusCode: statusCode); + } + + if (bodyStart + bodyLength > buffer.Length) + Array.Resize(ref buffer, bodyStart + bodyLength); + + while (bodyRead < bodyLength) + { + read = await pipe.ReadAsync( + buffer.AsMemory(bodyStart + bodyRead, bodyLength - bodyRead), cancellationToken) + .ConfigureAwait(false); + if (read == 0) + { + throw BrokerFailure( + incompleteResponseKind, + $"The package broker disconnected before sending the complete response body for {path}.", + path); + } + + bodyRead += read; + } + + return new BrokerTransportResponse + { + StatusCode = statusCode, + Body = Encoding.UTF8.GetString(buffer, bodyStart, bodyLength), + }; + } + + throw BrokerFailure( + BrokerClientErrorKind.InvalidResponse, + $"The package broker returned response headers that are too large for {path}.", + path); + } + + private static BrokerClientException BrokerFailure( + BrokerClientErrorKind kind, + string message, + string path, + Exception? innerException = null, + int? statusCode = null) => + new(kind, message, path, statusCode, null, innerException); +} diff --git a/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/WindowsPolicyElevationPreflight.cs b/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/WindowsPolicyElevationPreflight.cs index e7b7002c9d..28b55c3378 100644 --- a/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/WindowsPolicyElevationPreflight.cs +++ b/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/WindowsPolicyElevationPreflight.cs @@ -9,6 +9,7 @@ public enum PolicyElevationPreflightFailureKind HelperUnavailable, RunningHostMismatch, SignerBindingFailed, + TimedOut, } public sealed class PolicyElevationPreflightResult : IDisposable diff --git a/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/WindowsPolicyWriteElevator.cs b/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/WindowsPolicyWriteElevator.cs index 891ff054c6..8dbc414a24 100644 --- a/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/WindowsPolicyWriteElevator.cs +++ b/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/WindowsPolicyWriteElevator.cs @@ -103,6 +103,8 @@ public PolicyElevationPeerAuthenticationResult Authenticate( /// public sealed record PolicyElevationTimeouts(TimeSpan Connect, TimeSpan Exchange, TimeSpan Exit) { + public TimeSpan Preflight { get; init; } = PolicyElevationProtocol.PreflightTimeout; + public static PolicyElevationTimeouts Default { get; } = new( PolicyElevationProtocol.ConnectTimeout, PolicyElevationProtocol.ExchangeTimeout, @@ -201,7 +203,7 @@ public async Task ReplacePolicyAsync( using PolicyElevationPreflightResult preflight = await PolicyElevationPreflightRunner - .VerifyAsync(_preflight, cancellationToken) + .VerifyAsync(_preflight, _timeouts.Preflight, cancellationToken) .ConfigureAwait(false); if (!preflight.Succeeded) { @@ -210,10 +212,14 @@ await PolicyElevationPreflightRunner Logger.Warn($"[PolicyElevation] Preflight failed: {preflight.Detail}"); } - PolicyElevationOutcome outcome = - preflight.Failure == PolicyElevationPreflightFailureKind.HelperUnavailable - ? PolicyElevationOutcome.HelperUnavailable - : PolicyElevationOutcome.HelperUntrusted; + PolicyElevationOutcome outcome = preflight.Failure switch + { + PolicyElevationPreflightFailureKind.HelperUnavailable => + PolicyElevationOutcome.HelperUnavailable, + PolicyElevationPreflightFailureKind.TimedOut => + PolicyElevationOutcome.TimedOut, + _ => PolicyElevationOutcome.HelperUntrusted, + }; return Fail( request, outcome, diff --git a/src/UniGetUI.PackageEngine.Tests/BrokerPolicyTransportTests.cs b/src/UniGetUI.PackageEngine.Tests/BrokerPolicyTransportTests.cs index e139581d85..8c078390af 100644 --- a/src/UniGetUI.PackageEngine.Tests/BrokerPolicyTransportTests.cs +++ b/src/UniGetUI.PackageEngine.Tests/BrokerPolicyTransportTests.cs @@ -5,6 +5,7 @@ using Devolutions.Now.Policy.Client; using UniGetUI.PackageEngine.AgentBroker; using UniGetUI.PackageEngine.AgentBroker.PolicyManagement; +using UniGetUI.PackageEngine.AgentBroker.PolicyWriteElevation; namespace UniGetUI.PackageEngine.Tests; @@ -83,20 +84,74 @@ public async Task PinnedClient_ReportsEofWithoutHttpStatus(string wireResponse) Assert.Null(exception.BrokerError); } + [Theory] + [InlineData("management")] + [InlineData("validation")] + public async Task PolicyManagementTransport_MapsAnnouncedOversizeToInvalidResponse( + string endpoint) + { + string response = + $"HTTP/1.1 200 OK\r\nContent-Length: {BrokerPolicyManagementLimits.MaxResponseBodyBytes + 1}\r\n\r\n"; + + string status = await WithPipeAsync(response, async (client, token) => + { + var service = new BrokerPolicyManagementService(() => client, () => true); + if (endpoint == "management") + return (await service.GetManagementAsync(token)).Status.ToString(); + + using JsonDocument draft = JsonDocument.Parse("{}"); + return (await service.ValidateAsync(draft.RootElement, token)).Status.ToString(); + }, boundedTransport: true); + + Assert.Equal("InvalidResponse", status); + } + + [Fact] + public async Task PolicyManagementFraming_RejectsActualBodyBeyondDeclaredLength() + { + byte[] response = Encoding.UTF8.GetBytes( + "HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}x"); + using var stream = new MemoryStream(response); + + BrokerClientException exception = await Assert.ThrowsAsync(() => + BoundedNamedPipeBrokerTransport.ReadResponseAsync( + stream, + "/v1/policy/management", + BrokerPolicyManagementLimits.MaxResponseBodyBytes, + BrokerClientErrorKind.InvalidResponse, + CancellationToken.None)); + + Assert.Equal(BrokerClientErrorKind.InvalidResponse, exception.Kind); + } + + [Fact] + public void PolicyManagementResponseBudget_AllowsDuplicatedReplacementContent() + { + Assert.True( + BrokerPolicyManagementLimits.MaxResponseBodyBytes + > BrokerApi.MaxPolicyManagementBodyBytes * 3); + } + private static string HttpResponse(int status, string body) => $"HTTP/1.1 {status} Test\r\nContent-Length: {Encoding.UTF8.GetByteCount(body)}\r\n\r\n{body}"; private static async Task WithPipeAsync( string? wireResponse, - Func> action) + Func> action, + bool boundedTransport = false) { string pipeName = $"unigetui-policy-tests-{Guid.NewGuid():N}"; using var timeout = new CancellationTokenSource(TimeSpan.FromSeconds(15)); using var server = new NamedPipeServerStream( pipeName, PipeDirection.InOut, 1, PipeTransmissionMode.Byte, PipeOptions.Asynchronous); + using IBrokerTransport transport = boundedTransport + ? new BoundedNamedPipeBrokerTransport( + BrokerPolicyManagementLimits.MaxResponseBodyBytes, + pipeName) + : new NamedPipeBrokerTransport(pipeName); using var client = new BrokerClient(new BrokerClientOptions { - Transport = new NamedPipeBrokerTransport(pipeName), + Transport = transport, RequestedElevation = Elevation.Standard, EffectiveUser = "CONTOSO\\tester", ClientExecutablePath = @"C:\Tests\UniGetUI.exe", diff --git a/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/AuthenticatedBrokerTransportTests.cs b/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/AuthenticatedBrokerTransportTests.cs index 1ab03531cf..6184998601 100644 --- a/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/AuthenticatedBrokerTransportTests.cs +++ b/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/AuthenticatedBrokerTransportTests.cs @@ -112,6 +112,55 @@ public async Task Transport_EofBeforeHeadersIsUnavailable() Assert.Equal(BrokerClientErrorKind.BrokerUnavailable, exception.Kind); } + [Fact] + public async Task BlockingAuthentication_CancellationReturnsAndDisposesLateResult() + { + string pipeName = $"unigetui-policy-broker-{Guid.NewGuid():N}"; + using var release = new ManualResetEventSlim(); + using var timeout = new CancellationTokenSource(TimeSpan.FromSeconds(10)); + using var cancellation = new CancellationTokenSource(); + var started = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var disposed = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + Task server = RunServerAsync( + pipeName, + async _ => await disposed.Task.WaitAsync(timeout.Token), + timeout.Token); + using var transport = new AuthenticatedBrokerTransport( + pipeName, + (_, _) => + { + started.TrySetResult(); + release.Wait(CancellationToken.None); + return new CallbackDisposable(() => disposed.TrySetResult()); + }); + + Task pending = transport.Send( + new BrokerTransportRequest + { + Method = "PUT", + Path = "/v1/policy", + Headers = new Dictionary(), + Body = "{}", + }, + cancellation.Token); + await started.Task.WaitAsync(timeout.Token); + cancellation.Cancel(); + + try + { + await Assert.ThrowsAnyAsync(async () => + await pending.WaitAsync(TimeSpan.FromSeconds(2))); + Assert.False(disposed.Task.IsCompleted); + } + finally + { + release.Set(); + } + + await disposed.Task.WaitAsync(timeout.Token); + await server; + } + private static AuthenticatedBrokerTransport TestTransport(string pipeName) => new(pipeName, (_, _) => new NoopDisposable()); @@ -248,6 +297,11 @@ private static async Task WriteResponseAsync( await stream.FlushAsync(cancellationToken); } + private sealed class CallbackDisposable(Action dispose) : IDisposable + { + public void Dispose() => dispose(); + } + private sealed class NoopDisposable : IDisposable { public void Dispose() diff --git a/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/PolicyElevationHelperLocatorTests.cs b/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/PolicyElevationHelperLocatorTests.cs index e0e68e7e32..ef94c57ba0 100644 --- a/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/PolicyElevationHelperLocatorTests.cs +++ b/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/PolicyElevationHelperLocatorTests.cs @@ -363,6 +363,45 @@ public async Task CancelledQueuedPreflights_DoNotAccumulateWorkers() cancellation.Dispose(); } + [Fact] + public async Task PreflightDeadline_BoundsQueuedCallersAndPreservesSingleFlight() + { + using var preflight = new NonCooperativeBlockingPreflight(); + try + { + Task first = PolicyElevationPreflightRunner.VerifyAsync( + preflight, + TimeSpan.FromMilliseconds(100), + CancellationToken.None); + await preflight.Started.Task.WaitAsync(TimeSpan.FromSeconds(2)); + + using PolicyElevationPreflightResult firstResult = + await first.WaitAsync(TimeSpan.FromSeconds(2)); + Assert.Equal(PolicyElevationPreflightFailureKind.TimedOut, firstResult.Failure); + + using PolicyElevationPreflightResult queued = + await PolicyElevationPreflightRunner.VerifyAsync( + preflight, + TimeSpan.FromMilliseconds(100), + CancellationToken.None).WaitAsync(TimeSpan.FromSeconds(2)); + Assert.Equal(PolicyElevationPreflightFailureKind.TimedOut, queued.Failure); + Assert.Equal(1, preflight.InvocationCount); + } + finally + { + preflight.Release(); + } + + await preflight.Completed.Task.WaitAsync(TimeSpan.FromSeconds(2)); + using PolicyElevationPreflightResult recovered = + await PolicyElevationPreflightRunner.VerifyAsync( + preflight, + TimeSpan.FromSeconds(2), + CancellationToken.None); + Assert.True(recovered.Succeeded); + Assert.Equal(2, preflight.InvocationCount); + } + [Fact] public void EmptyInstallRoot_FailsClosed() { diff --git a/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/WindowsPolicyWriteElevatorTests.cs b/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/WindowsPolicyWriteElevatorTests.cs index ff0a16b6ae..51b81b96ff 100644 --- a/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/WindowsPolicyWriteElevatorTests.cs +++ b/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/WindowsPolicyWriteElevatorTests.cs @@ -296,6 +296,57 @@ public async Task CancelledPreflightWait_DisposesLeaseWhenWorkerFinishes() File.Delete(leasePath); } + [Fact] + public async Task PreflightDeadline_MapsTimeoutAndDisposesLeaseAfterLateCompletion() + { + string leasePath = Path.GetTempFileName(); + SafeFileHandle leaseHandle = File.OpenHandle( + leasePath, + FileMode.Open, + FileAccess.Read, + FileShare.ReadWrite); + PolicyElevationLocationVerification verification = + PolicyElevationLocationVerification.Protected( + [leaseHandle], + FakeHelperLocator.PackagedRoot, + FakeHelperLocator.PackagedHelperPath, + FakeHelperLocator.PackagedHostPath); + var location = new PolicyElevationHelperLocation( + true, + FakeHelperLocator.PackagedHelperPath, + FakeHelperLocator.PackagedHostPath, + FakeHelperLocator.PackagedRoot, + Verification: verification); + using var preflight = new BlockingPreflight( + () => PolicyElevationPreflightResult.Success(location), + honorCancellation: false); + FakeHelperLauncher launcher = + FakeHelperLauncher.Running((_, _) => Task.CompletedTask); + WindowsPolicyWriteElevator elevator = Build( + launcher, + preflight: preflight, + timeouts: FastTimeouts with { Preflight = TimeSpan.FromMilliseconds(100) }); + + try + { + PolicyElevationResult result = await elevator + .ReplacePolicyAsync(BuildRequest(), CancellationToken.None) + .WaitAsync(TimeSpan.FromSeconds(2)); + + Assert.Equal(PolicyElevationOutcome.TimedOut, result.Outcome); + Assert.False(leaseHandle.IsClosed); + Assert.Null(launcher.LaunchedPath); + } + finally + { + preflight.Release(); + } + + await preflight.Completed.Task.WaitAsync(TimeSpan.FromSeconds(2)); + Assert.True(SpinWait.SpinUntil(() => leaseHandle.IsClosed, TimeSpan.FromSeconds(2))); + File.Delete(leasePath); + } + [Fact] public async Task PreflightException_IsPropagatedWithoutLaunchingHelper() { From 02d5080de0cd1606b8d137f968b41f61cf488138 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Mon, 21 Sep 2026 14:27:04 +0900 Subject: [PATCH 2/2] Harden policy editor state and release checks --- .github/workflows/build-release.yml | 37 +++++++++ .../PolicyEditor/PolicyEditorSession.cs | 26 ++++++- .../PolicyEditorSessionViewModel.cs | 34 +++++++-- .../PolicyEditor/PolicyEditorDialog.axaml.cs | 2 +- .../BrokerPolicyManagementModels.cs | 4 +- .../BrokerPolicyManagementService.cs | 12 +-- .../Protocol/PolicyElevationFrame.cs | 46 ++++++++---- .../Protocol/PolicyElevationProtocol.cs | 7 +- .../Shared/BoundedNamedPipeBrokerTransport.cs | 14 ++++ .../WindowsElevatedHelperLauncher.cs | 3 + .../WindowsPolicyWriteElevator.cs | 17 ++++- .../BrokerPolicyTransportTests.cs | 55 ++++++++++++++ .../PolicyElevationContractTests.cs | 63 +++++++++++++++- .../PolicyElevationProtocolTests.cs | 5 +- .../PolicyElevationTrustPolicyTests.cs | 20 +++++ .../PolicyEditorSessionViewModelTests.cs | 75 +++++++++++++++++++ .../PolicyEditorViewModelFakes.cs | 20 +++++ .../SettingsSearchIndexTests.cs | 6 +- 18 files changed, 404 insertions(+), 42 deletions(-) diff --git a/.github/workflows/build-release.yml b/.github/workflows/build-release.yml index a239e788d5..fa6ea8633f 100644 --- a/.github/workflows/build-release.yml +++ b/.github/workflows/build-release.yml @@ -265,6 +265,43 @@ jobs: throw "Elevated policy helper is not validly signed (status: $($HelperSignature.Status))." } + - name: Validate staged policy signer binding (${{ matrix.platform }}) + if: ${{ fromJSON(needs.preflight.outputs.dry-run) == false }} + shell: pwsh + run: | + $Platform = '${{ matrix.platform }}' + $HostPath = Join-Path $PWD "unigetui_bin/UniGetUI.exe" + $HelperPath = Join-Path $PWD "unigetui_bin/Assets/Utilities/UniGetUI.PolicyElevator.exe" + if (-not (Test-Path -LiteralPath $HostPath -PathType Leaf)) { throw "Windows app host was not staged at $HostPath" } + if (-not (Test-Path -LiteralPath $HelperPath -PathType Leaf)) { throw "Elevated policy helper was not staged at $HelperPath" } + # WinVerifyTrust validates PE files without executing them, so the x64 test host can inspect arm64. + $env:UNIGETUI_RELEASE_POLICY_ELEVATION_HOST_PATH = $HostPath + $env:UNIGETUI_RELEASE_POLICY_ELEVATION_HELPER_PATH = $HelperPath + Write-Host "Authenticode/SPKI binding validation for win-$Platform staged artifacts." + $ResultsDirectory = Join-Path $env:RUNNER_TEMP "policy-signer-binding-$Platform" + if (Test-Path -LiteralPath $ResultsDirectory) { + Remove-Item -LiteralPath $ResultsDirectory -Recurse -Force + } + + dotnet test src/UniGetUI.PackageEngine.Tests/UniGetUI.PackageEngine.Tests.csproj ` + --no-build --no-restore --verbosity q --nologo /p:Platform=x64 ` + --framework net10.0-windows10.0.26100.0 ` + --filter "FullyQualifiedName=UniGetUI.PackageEngine.Tests.PolicyWriteElevation.PolicyElevationTrustPolicyTests.ReleaseArtifacts_AreAuthenticodeBound_WhenProvidedByReleaseWorkflow" ` + --results-directory $ResultsDirectory ` + --logger "trx;LogFileName=policy-signer-binding.trx" + if ($LASTEXITCODE -ne 0) { throw "Staged policy elevation signer binding validation failed for win-$Platform." } + + $TrxPath = Join-Path $ResultsDirectory "policy-signer-binding.trx" + if (-not (Test-Path -LiteralPath $TrxPath -PathType Leaf)) { + throw "Policy signer-binding test did not produce $TrxPath." + } + + [xml]$Trx = Get-Content -LiteralPath $TrxPath -Raw + $ExecutedTests = @($Trx.SelectNodes("//*[local-name()='UnitTestResult']")) + if ($ExecutedTests.Count -ne 1 -or $ExecutedTests[0].outcome -ne "Passed") { + throw "Expected exactly one passing policy signer-binding test; observed $($ExecutedTests.Count)." + } + - name: Build installer shell: pwsh run: | diff --git a/src/UniGetUI.Avalonia/ViewModels/Pages/SettingsPages/PolicyEditor/PolicyEditorSession.cs b/src/UniGetUI.Avalonia/ViewModels/Pages/SettingsPages/PolicyEditor/PolicyEditorSession.cs index 86240fc539..f41b2ea1cf 100644 --- a/src/UniGetUI.Avalonia/ViewModels/Pages/SettingsPages/PolicyEditor/PolicyEditorSession.cs +++ b/src/UniGetUI.Avalonia/ViewModels/Pages/SettingsPages/PolicyEditor/PolicyEditorSession.cs @@ -8,7 +8,8 @@ public sealed record PolicyEditorValidationState( string SubmittedRawJson, PolicyDraftDocument CanonicalDraft, string Receipt, - PolicyEditorFindingIndex Findings); + PolicyEditorFindingIndex Findings, + long Epoch); public sealed record PolicyEditorConflictSnapshot( string SubmittedCanonicalRawJson, @@ -35,6 +36,7 @@ public sealed class PolicyEditorSession private long _mutationGeneration; private long _cleanMutationGeneration; private long _baselineVersion; + private long _validationEpoch; private bool _isDirty; public PolicyEditorOperationKind Operation { get; private set; } @@ -56,6 +58,8 @@ public sealed class PolicyEditorSession public long MutationGeneration => _mutationGeneration; + internal long ValidationEpoch => _validationEpoch; + public bool IsRawAnalysisPending { get; private set; } internal bool LastRawAnalysisWasFormattingOnly { get; private set; } @@ -65,6 +69,7 @@ public sealed class PolicyEditorSession public bool IsValidationCurrent => Validation is not null + && Validation.Epoch == _validationEpoch && !IsRawAnalysisPending && string.Equals( Validation.SubmittedRawJson, @@ -367,10 +372,21 @@ public void ApplyValidationResult( string submittedRawJson, PolicyValidationResult validation, IReadOnlyList? boundedFindings = null, + int omittedFindingCount = 0) => + TryApplyValidationResult(submittedRawJson, validation, _validationEpoch, + boundedFindings, omittedFindingCount); + + internal bool TryApplyValidationResult( + string submittedRawJson, + PolicyValidationResult validation, + long validationEpoch, + IReadOnlyList? boundedFindings = null, int omittedFindingCount = 0) { ArgumentNullException.ThrowIfNull(submittedRawJson); ArgumentNullException.ThrowIfNull(validation); + if (validationEpoch != _validationEpoch) + return false; IReadOnlyList findings; if (boundedFindings is not null) @@ -406,15 +422,17 @@ public void ApplyValidationResult( || string.IsNullOrWhiteSpace(validation.ValidationReceipt)) { Validation = null; - return; + return true; } Validation = new PolicyEditorValidationState( submittedRawJson, PolicyEditorMapper.CloneDraftDocument(validation.CanonicalDraft), validation.ValidationReceipt, - Findings); + Findings, + _validationEpoch); Operation = ResolveOperationForDraftId(validation.CanonicalDraft.Metadata.Id); + return true; } public void CaptureConflict( @@ -588,6 +606,7 @@ private void SetBaseline(string baselineRawJson, long mutationGeneration) _baselineRawJson = baselineRawJson; _cleanMutationGeneration = mutationGeneration; _baselineVersion++; + _validationEpoch++; } private void ClearContentState() @@ -623,6 +642,7 @@ private bool TryGetCanonicalEffectiveRaw( string effectiveRawJson = GetEffectiveRawJson(); if (Validation is not null + && Validation.Epoch == _validationEpoch && string.Equals( Validation.SubmittedRawJson, effectiveRawJson, diff --git a/src/UniGetUI.Avalonia/ViewModels/Pages/SettingsPages/PolicyEditor/PolicyEditorSessionViewModel.cs b/src/UniGetUI.Avalonia/ViewModels/Pages/SettingsPages/PolicyEditor/PolicyEditorSessionViewModel.cs index 2802b62900..e7c4996a9c 100644 --- a/src/UniGetUI.Avalonia/ViewModels/Pages/SettingsPages/PolicyEditor/PolicyEditorSessionViewModel.cs +++ b/src/UniGetUI.Avalonia/ViewModels/Pages/SettingsPages/PolicyEditor/PolicyEditorSessionViewModel.cs @@ -525,6 +525,7 @@ private async Task SaveCoreAsync( { string submitted = Session.GetEffectiveRawJson(); long attemptGeneration = Session.MutationGeneration; + long validationEpoch = Session.ValidationEpoch; ReconcileDirtyAtBoundary(submitted); // Correction #14: reuse the exact current validation (same receipt/CanonicalDraft) when @@ -552,7 +553,8 @@ private async Task SaveCoreAsync( await _validationClient.ValidateAsync(submittedElement, cancellationToken); if (!CanApply(cancellationToken) || saveGeneration != Volatile.Read(ref _saveGeneration) - || Session.MutationGeneration != attemptGeneration) + || Session.MutationGeneration != attemptGeneration + || Session.ValidationEpoch != validationEpoch) return; if (validationOutcome.Validation is null) { @@ -560,11 +562,13 @@ private async Task SaveCoreAsync( return; } - Session.ApplyValidationResult( + if (!Session.TryApplyValidationResult( submitted, validationOutcome.Validation, + validationEpoch, validationOutcome.BoundedFindings, - validationOutcome.OmittedFindingCount); + validationOutcome.OmittedFindingCount)) + return; OnEditorStateChanged(); validation = Session.Validation; if (validation is null) @@ -721,10 +725,13 @@ private async Task SaveCoreAsync( { if (write.Response is not null) { + CancelAuthoritativeValidation(); Session.MarkSavedPreservingCurrentDraft(write.Response, attemptGeneration); SavedWithNewerChanges = true; LastSaveSucceeded = true; ScheduleCurrentModeDirtyAnalysis(); + if (Session.IsDirty) + ScheduleAuthoritativeValidation(); OnEditorStateChanged(); } @@ -740,6 +747,7 @@ private async Task SaveCoreAsync( if (write.Response is not null) { + CancelAuthoritativeValidation(); Session.MarkSaved(write.Response); SavedWithNewerChanges = false; SavedThenSuperseded = write.SavedThenSuperseded; @@ -1309,7 +1317,14 @@ private void ScheduleAuthoritativeValidation() private void ScheduleAuthoritativeValidation( string raw, JsonElement draft, - long mutationGeneration) + long mutationGeneration) => + ScheduleAuthoritativeValidation(raw, draft, mutationGeneration, Session.ValidationEpoch); + + private void ScheduleAuthoritativeValidation( + string raw, + JsonElement draft, + long mutationGeneration, + long validationEpoch) { var cancellation = CancellationTokenSource.CreateLinkedTokenSource( _lifetimeCancellation.Token); @@ -1321,6 +1336,7 @@ private void ScheduleAuthoritativeValidation( raw, draft.Clone(), mutationGeneration, + validationEpoch, cancellation); } @@ -1328,6 +1344,7 @@ private async Task ValidateAuthoritativeAsync( string raw, JsonElement draft, long mutationGeneration, + long validationEpoch, CancellationTokenSource cancellation) { try @@ -1338,17 +1355,22 @@ private async Task ValidateAuthoritativeAsync( if (cancellation.IsCancellationRequested || Volatile.Read(ref _isDisposed) != 0 || mutationGeneration != Session.MutationGeneration + || validationEpoch != Session.ValidationEpoch || !string.Equals(raw, Session.GetEffectiveRawJson(), StringComparison.Ordinal) || outcome.Validation is null) { return; } - Session.ApplyValidationResult( + if (!Session.TryApplyValidationResult( raw, outcome.Validation, + validationEpoch, outcome.BoundedFindings, - outcome.OmittedFindingCount); + outcome.OmittedFindingCount)) + { + return; + } _hasLocalSemanticErrors = Session.Findings.All.Any( finding => finding.Severity == PolicyValidationSeverity.Error); SyntaxError = null; diff --git a/src/UniGetUI.Avalonia/Views/Pages/SettingsPages/PolicyEditor/PolicyEditorDialog.axaml.cs b/src/UniGetUI.Avalonia/Views/Pages/SettingsPages/PolicyEditor/PolicyEditorDialog.axaml.cs index b19372c579..45cb402c0d 100644 --- a/src/UniGetUI.Avalonia/Views/Pages/SettingsPages/PolicyEditor/PolicyEditorDialog.axaml.cs +++ b/src/UniGetUI.Avalonia/Views/Pages/SettingsPages/PolicyEditor/PolicyEditorDialog.axaml.cs @@ -12,7 +12,7 @@ namespace UniGetUI.Avalonia.Views.Pages.SettingsPages.PolicyEditor; /// /// Modal structured/raw editor for a package broker policy draft. Hosted as an -/// (not a settings page) so this Phase 2 surface never touches +/// (not a settings page) so the policy editor never touches /// SettingsBasePage's page-navigation switch. must be a /// . /// diff --git a/src/UniGetUI.PackageEngine.AgentBroker/PolicyManagement/BrokerPolicyManagementModels.cs b/src/UniGetUI.PackageEngine.AgentBroker/PolicyManagement/BrokerPolicyManagementModels.cs index f7a4bae535..472a545bef 100644 --- a/src/UniGetUI.PackageEngine.AgentBroker/PolicyManagement/BrokerPolicyManagementModels.cs +++ b/src/UniGetUI.PackageEngine.AgentBroker/PolicyManagement/BrokerPolicyManagementModels.cs @@ -46,7 +46,7 @@ public enum BrokerPolicyValidationStatus } /// -/// Shared constants for the Phase 2 policy management/validation surface. +/// Shared constants for policy management and validation. /// public static class BrokerPolicyManagementLimits { @@ -112,7 +112,7 @@ public sealed record BrokerPolicyDiagnosticsView( /// /// Result of . exposes -/// the package's own contract type directly (mirroring the Phase 1 BrokerPolicyInspectionResult +/// the package's own contract type directly (matching the existing BrokerPolicyInspectionResult /// pattern) so callers retain full fidelity (state, write capability/reason, configured path, store token, /// and - when Active - the policy document). additionally provides a sanitized, /// bounded view of Invalid-state findings suitable for direct UI rendering. diff --git a/src/UniGetUI.PackageEngine.AgentBroker/PolicyManagement/BrokerPolicyManagementService.cs b/src/UniGetUI.PackageEngine.AgentBroker/PolicyManagement/BrokerPolicyManagementService.cs index 97cfa8b74b..cea0e7f581 100644 --- a/src/UniGetUI.PackageEngine.AgentBroker/PolicyManagement/BrokerPolicyManagementService.cs +++ b/src/UniGetUI.PackageEngine.AgentBroker/PolicyManagement/BrokerPolicyManagementService.cs @@ -12,9 +12,9 @@ namespace UniGetUI.PackageEngine.AgentBroker.PolicyManagement; /// -/// Read-only adapter over the Phase 2 Agent policy management/validation endpoints +/// Read-only adapter over the Agent policy management and validation endpoints /// (GET /v1/policy/management, POST /v1/policy/validate). This is independent of the -/// Phase 1 IBrokerPolicyInspector (GET /v1/policy) and does not read the +/// IBrokerPolicyInspector snapshot endpoint (GET /v1/policy) and does not read the /// UseAgentBroker setting: callers decide when to invoke it. /// public interface IBrokerPolicyManagementService @@ -117,7 +117,7 @@ public async Task ValidateAsync(JsonElement draft // invariants (violations throw JsonException, surfaced by BrokerClient as // BrokerClientException(Kind = InvalidResponse) before ever reaching this adapter). What is *not* // enforced by the package - and is therefore checked here - is the envelope's ResponseVersion format - // and ServerVersion bound (mirroring the Phase 1 BrokerPolicyInspector checks for the sibling GET + // and ServerVersion bound (matching the BrokerPolicyInspector checks for the sibling GET // /v1/policy endpoint), plus defensive Enum.IsDefined checks for forward-compatibility. private static bool HasRequiredManagementData(PolicyManagementResponse response) { @@ -173,9 +173,9 @@ private static bool IsResponseVersion(string? value) // Keeps 404/NotFound/UnsupportedEndpoint mapped as "older unsupported Agent", and the three // policy-path/format/filesystem error codes distinct from each other and from every other outcome, - // per the Phase 2 contract. Mirrors the Phase 1 BrokerPolicyInspector.MapFailure - // precedent: BrokerClientErrorKind.BrokerError collapses every structured broker error into one kind, - // so disambiguation must happen via StatusCode/BrokerError.Code first. + // per the policy management contract. This follows the BrokerPolicyInspector.MapFailure precedent: + // BrokerClientErrorKind.BrokerError collapses every structured broker error into one kind, so + // disambiguation must happen via StatusCode/BrokerError.Code first. private static BrokerPolicyManagementStatus MapManagementFailure(BrokerClientException ex) { if (ex.StatusCode == 404 diff --git a/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/Protocol/PolicyElevationFrame.cs b/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/Protocol/PolicyElevationFrame.cs index 21d61981f0..ed0dc95b73 100644 --- a/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/Protocol/PolicyElevationFrame.cs +++ b/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/Protocol/PolicyElevationFrame.cs @@ -168,12 +168,12 @@ public static void ValidateRequest(PolicyElevationRequestMessage request) throw Malformed("Policy elevation request carried an undefined enumeration value."); } - RequireRequiredSafeAscii( + RequireRequiredCredential( request.ExpectedStoreToken, PolicyElevationProtocol.MaxStoreTokenCharacters, "expectedStoreToken"); - RequireRequiredSafeAscii( + RequireRequiredCredential( request.ValidationReceipt, PolicyElevationProtocol.MaxValidationReceiptCharacters, "validationReceipt"); @@ -205,7 +205,7 @@ public static void ValidateResponse(PolicyElevationResponseMessage response) switch (response.Disposition) { case PolicyElevationDisposition.Committed: - RequireRequiredSafeAscii( + RequireRequiredCredential( response.CommittedStoreToken, PolicyElevationProtocol.MaxStoreTokenCharacters, "committedStoreToken"); @@ -317,7 +317,7 @@ private static void ValidateConflictContext(PolicyElevationResponseMessage respo return; } - RequireRequiredSafeAscii( + RequireRequiredCredential( response.ConflictStoreToken, PolicyElevationProtocol.MaxStoreTokenCharacters, "conflictStoreToken"); @@ -337,16 +337,9 @@ private static void ValidateConflictContext(PolicyElevationResponseMessage respo } /// - /// Mirrors the shared policy store-token / validation-receipt grammar exactly: one or more - /// characters, every one of them printable ASCII, and the first one an ASCII alphanumeric. + /// Requires a generic bounded printable-ASCII protocol field with an ASCII alphanumeric first + /// character. /// - /// - /// The shared converters that enforce this on the broker side - /// (PolicyStoreTokenJsonConverter and PolicyValidationReceiptJsonConverter) are - /// internal to the policy API package and cannot be referenced from here, so the rule is - /// mirrored rather than reused. It is verified against the real converters by the round-trip - /// tests, which reject anything this method accepts but the broker would not. - /// private static void RequireRequiredSafeAscii( string? value, int maxCharacters, @@ -365,6 +358,33 @@ private static void RequireRequiredSafeAscii( } } + /// + /// Mirrors the shared policy store-token / validation-receipt grammar exactly: one or more + /// ASCII alphanumeric characters or ., _, ~, : and -, with + /// an ASCII alphanumeric first character. + /// + /// + /// The package's PolicyStoreTokenJsonConverter and + /// PolicyValidationReceiptJsonConverter are internal, so this protocol boundary mirrors + /// their published grammar. The contract tests exercise both implementations against the same + /// boundary values. + /// + private static void RequireRequiredCredential(string? value, int maxCharacters, string field) + { + if (string.IsNullOrEmpty(value) || value.Length > maxCharacters) + throw Malformed($"Policy elevation frame carried an invalid {field}."); + if (!char.IsAsciiLetterOrDigit(value[0])) + throw Malformed($"Policy elevation frame carried an invalid {field}."); + foreach (char character in value) + { + if (!char.IsAsciiLetterOrDigit(character) + && character is not ('.' or '_' or '~' or ':' or '-')) + { + throw Malformed($"Policy elevation frame carried an invalid {field}."); + } + } + } + private static PolicyElevationFrameException Malformed(string message) => new(PolicyElevationFrameError.Malformed, message); diff --git a/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/Protocol/PolicyElevationProtocol.cs b/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/Protocol/PolicyElevationProtocol.cs index 852c408ce7..156ffa41d8 100644 --- a/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/Protocol/PolicyElevationProtocol.cs +++ b/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/Protocol/PolicyElevationProtocol.cs @@ -99,9 +99,8 @@ public static class PolicyElevationProtocol // System.Text.Json's default encoder emits HTML-sensitive safe-ASCII characters such as // quotation marks as six-byte \uXXXX escapes. private const int MaxSafeAsciiJsonBytesPerCharacter = 6; - private const int MaxSafeAsciiStoreTokenValueBytes = - QuoteBytes + 1 - + ((MaxStoreTokenCharacters - 1) * MaxSafeAsciiJsonBytesPerCharacter); + private const int MaxCredentialStoreTokenValueBytes = + QuoteBytes + MaxStoreTokenCharacters; private const int MaxSafeAsciiConflictPolicyIdValueBytes = QuoteBytes + 1 + ((MaxConflictPolicyIdCharacters - 1) * MaxSafeAsciiJsonBytesPerCharacter); @@ -157,7 +156,7 @@ public static class PolicyElevationProtocol + MaxInt32Bytes // brokerStatusCode + StaleErrorCodeValueBytes + 4 // committedStoreToken is null in a stale response - + MaxSafeAsciiStoreTokenValueBytes + + MaxCredentialStoreTokenValueBytes + ActiveManagementStateBytes + MaxSafeAsciiConflictPolicyIdValueBytes; diff --git a/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/Shared/BoundedNamedPipeBrokerTransport.cs b/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/Shared/BoundedNamedPipeBrokerTransport.cs index 35da95f85f..8a202ef689 100644 --- a/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/Shared/BoundedNamedPipeBrokerTransport.cs +++ b/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/Shared/BoundedNamedPipeBrokerTransport.cs @@ -230,6 +230,20 @@ internal static async Task ReadResponseAsync( bodyRead += read; } + // Requests explicitly require Connection: close, so EOF is part of the single-response + // frame. Probe one byte past Content-Length to make excess-data rejection independent + // of how the pipe happened to chunk its reads. + byte[] trailing = new byte[1]; + int trailingRead = await pipe.ReadAsync(trailing, cancellationToken).ConfigureAwait(false); + if (trailingRead != 0) + { + throw BrokerFailure( + BrokerClientErrorKind.InvalidResponse, + $"The package broker returned more response data than declared for {path}.", + path, + statusCode: statusCode); + } + return new BrokerTransportResponse { StatusCode = statusCode, diff --git a/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/WindowsElevatedHelperLauncher.cs b/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/WindowsElevatedHelperLauncher.cs index 60bb91286c..bcc8bc7dd1 100644 --- a/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/WindowsElevatedHelperLauncher.cs +++ b/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/WindowsElevatedHelperLauncher.cs @@ -52,6 +52,9 @@ Task LaunchAsync( /// /// Starts the helper through ShellExecuteEx with the runas verb, which is what /// raises the consent prompt. The command line carries routing arguments only. +/// UniGetUI Elevator is deliberately not used because the authenticated exchange requires a handle +/// to the exact helper process. Keeping elevation in this separate, minimal one-shot executable +/// also prevents the full UniGetUI application from receiving an elevated token. /// public sealed class WindowsElevatedHelperLauncher : IElevatedHelperLauncher { diff --git a/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/WindowsPolicyWriteElevator.cs b/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/WindowsPolicyWriteElevator.cs index 8dbc414a24..0adf50c053 100644 --- a/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/WindowsPolicyWriteElevator.cs +++ b/src/UniGetUI.PackageEngine.AgentBroker/PolicyWriteElevation/WindowsPolicyWriteElevator.cs @@ -186,7 +186,22 @@ public async Task ReplacePolicyAsync( { ArgumentNullException.ThrowIfNull(request); - PolicyElevationRequestMessage preflightRequest = CreateRequestMessage(request, string.Empty); + PolicyElevationRequestMessage preflightRequest = CreateRequestMessage( + request, + new string('0', PolicyElevationProtocol.RequestIdCharacters)); + try + { + // Init-only request members can be changed after the constructor has validated its + // default operation. Validate the complete wire request before creating a pipe or + // launching the privileged helper, so an internal malformed request is deterministic. + PolicyElevationFrame.ValidateRequest(preflightRequest); + } + catch (PolicyElevationFrameException) + { + return Fail(request, PolicyElevationOutcome.MalformedResponse, + "The policy elevation request is malformed."); + } + if (!PolicyElevationReplacementDispatcher.IsBrokerRequestWithinLimit(preflightRequest)) { return Fail( diff --git a/src/UniGetUI.PackageEngine.Tests/BrokerPolicyTransportTests.cs b/src/UniGetUI.PackageEngine.Tests/BrokerPolicyTransportTests.cs index 8c078390af..1afa20fdcf 100644 --- a/src/UniGetUI.PackageEngine.Tests/BrokerPolicyTransportTests.cs +++ b/src/UniGetUI.PackageEngine.Tests/BrokerPolicyTransportTests.cs @@ -124,6 +124,24 @@ public async Task PolicyManagementFraming_RejectsActualBodyBeyondDeclaredLength( Assert.Equal(BrokerClientErrorKind.InvalidResponse, exception.Kind); } + [Fact] + public async Task PolicyManagementFraming_RejectsChunkedBodyBeyondDeclaredLength() + { + using var stream = new ChunkedReadStream( + Encoding.UTF8.GetBytes("HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}"), + Encoding.UTF8.GetBytes("x")); + + BrokerClientException exception = await Assert.ThrowsAsync(() => + BoundedNamedPipeBrokerTransport.ReadResponseAsync( + stream, + "/v1/policy/management", + BrokerPolicyManagementLimits.MaxResponseBodyBytes, + BrokerClientErrorKind.InvalidResponse, + CancellationToken.None)); + + Assert.Equal(BrokerClientErrorKind.InvalidResponse, exception.Kind); + } + [Fact] public void PolicyManagementResponseBudget_AllowsDuplicatedReplacementContent() { @@ -199,4 +217,41 @@ async Task ServeAsync() server.Disconnect(); } } + + private sealed class ChunkedReadStream(params byte[][] chunks) : Stream + { + private readonly Queue _chunks = new(chunks); + + public override bool CanRead => true; + public override bool CanSeek => false; + public override bool CanWrite => false; + public override long Length => throw new NotSupportedException(); + public override long Position + { + get => throw new NotSupportedException(); + set => throw new NotSupportedException(); + } + + public override ValueTask ReadAsync( + Memory buffer, + CancellationToken cancellationToken = default) + { + cancellationToken.ThrowIfCancellationRequested(); + if (_chunks.Count == 0) + return ValueTask.FromResult(0); + + byte[] chunk = _chunks.Dequeue(); + Assert.True(chunk.Length <= buffer.Length); + chunk.CopyTo(buffer); + return ValueTask.FromResult(chunk.Length); + } + + public override int Read(byte[] buffer, int offset, int count) => + throw new NotSupportedException(); + public override void Flush() => throw new NotSupportedException(); + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + public override void SetLength(long value) => throw new NotSupportedException(); + public override void Write(byte[] buffer, int offset, int count) => + throw new NotSupportedException(); + } } diff --git a/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/PolicyElevationContractTests.cs b/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/PolicyElevationContractTests.cs index a05efa8e50..71bbb1fa6b 100644 --- a/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/PolicyElevationContractTests.cs +++ b/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/PolicyElevationContractTests.cs @@ -287,21 +287,36 @@ public void CredentialLengths_AreAcceptedExactlyUpToTheSharedMaximum( } [Theory] - // The shared grammar: printable ASCII, first character an ASCII alphanumeric. + // The shared grammar: ASCII alphanumerics plus . _ ~ : -, first character ASCII alphanumeric. [InlineData("a", true)] [InlineData("0", true)] [InlineData("Z", true)] [InlineData("tok-1.2:3_4~5", true)] + [InlineData("a._~:-Z9", true)] [InlineData("-leading", false)] [InlineData("_leading", false)] [InlineData(".leading", false)] [InlineData("~leading", false)] + [InlineData(":leading", false)] [InlineData(" leading", false)] [InlineData("tok en", false)] [InlineData("tok\ten", false)] [InlineData("tok\nen", false)] [InlineData("tokén", false)] [InlineData("token ", false)] + [InlineData("token!bang", false)] + [InlineData("token\"quote", false)] + [InlineData(@"tokenackslash", false)] + [InlineData("token/slash", false)] + [InlineData("token+plus", false)] + [InlineData("token=equals", false)] + [InlineData("token[bracket", false)] + [InlineData("token{brace", false)] + [InlineData("token,comma", false)] + [InlineData("token#hash", false)] + [InlineData("token@at", false)] + [InlineData("token$dollar", false)] + [InlineData("token%percent", false)] public void CredentialGrammar_MirrorsTheSharedConverters(string credential, bool accepted) { static void Validate(string token, string receipt) => PolicyElevationFrame.ValidateRequest( @@ -314,6 +329,28 @@ static void Validate(string token, string receipt) => PolicyElevationFrame.Valid Draft = JsonDocument.Parse(DraftJson).RootElement.Clone(), }); + static bool SharedConvertersAccept(string value) + { + try + { + _ = BrokerSerializer.Serialize(new PolicyReplacementRequest + { + ExpectedStoreToken = value, + ValidationReceipt = value, + Operation = PolicyReplacementOperation.Update, + ConflictHandling = PolicyConflictHandling.Reject, + Draft = JsonDocument.Parse("{}").RootElement.Clone(), + }); + return true; + } + catch (JsonException) + { + return false; + } + } + + Assert.Equal(accepted, SharedConvertersAccept(credential)); + if (accepted) { Validate(credential, credential); @@ -324,11 +361,33 @@ static void Validate(string token, string receipt) => PolicyElevationFrame.Valid Assert.Throws(() => Validate("token", credential)); } + [Fact] + public async Task MalformedInternalRequest_IsRejectedBeforeHelperLaunch() + { + FakeHelperLauncher launcher = FakeHelperLauncher.Failing( + ElevatedHelperLaunchResult.Failed( + PolicyElevationOutcome.LaunchFailed, + "The test launcher must not run.")); + PolicyElevationWriteRequest request = Request() with + { + Operation = (PolicyElevationOperation)99, + }; + PolicyElevationResult result = await Build(launcher) + .ReplacePolicyAsync(request, CancellationToken.None); + Assert.Equal(PolicyElevationOutcome.MalformedResponse, result.Outcome); + Assert.NotEqual(PolicyElevationOutcome.WriteResultUnknown, result.Outcome); + Assert.Null(launcher.LaunchedPath); + Assert.Null(launcher.LaunchedArguments); + } + // ---- Protocol v2: bounded post-commit acknowledgement ------------------------------------ [Fact] public async Task AMaximumStaleAcknowledgementExactlyFitsTheResponseBudget() { + static string WorstCaseCredential(int length) => + "a" + new string('Z', length - 1); + static string WorstCaseSafeAscii(int length) => "a" + new string('"', length - 1); @@ -338,7 +397,7 @@ static string WorstCaseSafeAscii(int length) => Disposition = PolicyElevationDisposition.Rejected, BrokerStatusCode = int.MinValue, BrokerErrorCode = ErrorCode.StalePolicyStoreToken.ToString(), - ConflictStoreToken = WorstCaseSafeAscii( + ConflictStoreToken = WorstCaseCredential( PolicyElevationProtocol.MaxStoreTokenCharacters), ConflictState = PolicyElevationManagementState.Active, ConflictPolicyId = WorstCaseSafeAscii( diff --git a/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/PolicyElevationProtocolTests.cs b/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/PolicyElevationProtocolTests.cs index caa23f37ad..3b46bf4a03 100644 --- a/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/PolicyElevationProtocolTests.cs +++ b/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/PolicyElevationProtocolTests.cs @@ -92,7 +92,7 @@ public void ResponseEnvelopeOverhead_CoversAWorstCaseEnvelope() BrokerStatusCode = int.MinValue, BrokerErrorCode = ErrorCode.StalePolicyStoreToken.ToString(), CommittedStoreToken = null, - ConflictStoreToken = WorstCaseSafeAscii( + ConflictStoreToken = WorstCaseCredential( PolicyElevationProtocol.MaxStoreTokenCharacters), ConflictState = PolicyElevationManagementState.Active, ConflictPolicyId = WorstCaseSafeAscii( @@ -186,6 +186,9 @@ private static int SumPropertyNameLengths() return total; } + private static string WorstCaseCredential(int length) => + "a" + new string('Z', length - 1); + private static string WorstCaseSafeAscii(int length) => "a" + new string('"', length - 1); } diff --git a/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/PolicyElevationTrustPolicyTests.cs b/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/PolicyElevationTrustPolicyTests.cs index 75f59184d9..90f37b4661 100644 --- a/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/PolicyElevationTrustPolicyTests.cs +++ b/src/UniGetUI.PackageEngine.Tests/PolicyWriteElevation/PolicyElevationTrustPolicyTests.cs @@ -170,6 +170,26 @@ public void BindingFailures_AreSafeToShowToAUser() } #if WINDOWS + [Fact] + [Trait("Category", "ReleaseArtifact")] + public void ReleaseArtifacts_AreAuthenticodeBound_WhenProvidedByReleaseWorkflow() + { + string? host = Environment.GetEnvironmentVariable("UNIGETUI_RELEASE_POLICY_ELEVATION_HOST_PATH"); + string? helper = Environment.GetEnvironmentVariable("UNIGETUI_RELEASE_POLICY_ELEVATION_HELPER_PATH"); + if (string.IsNullOrWhiteSpace(host) && string.IsNullOrWhiteSpace(helper)) + return; + Assert.False(string.IsNullOrWhiteSpace(host), "The release host path must be supplied with the release helper path."); + Assert.False(string.IsNullOrWhiteSpace(helper), "The release helper path must be supplied with the release host path."); + Assert.Equal(PolicyElevationProtocol.HostFileName, Path.GetFileName(host)); + Assert.Equal(PolicyElevationProtocol.HelperFileName, Path.GetFileName(helper)); + Assert.True(File.Exists(host), "The staged release host executable is missing."); + Assert.True(File.Exists(helper), "The staged release helper executable is missing."); + PolicyElevationSignerBindingResult binding = PolicyElevationSignerBinding.Bind( + new WindowsAuthenticodeTrustVerifier(), host, helper); + Assert.True(binding.IsBound, binding.FailureReason + ?? "The staged release host and helper did not pass Authenticode signer binding."); + } + [Fact] public void ProductionVerifier_ReportsTheSignerOfAValidlySignedBinary() { diff --git a/src/UniGetUI.Tests/PolicyEditor/PolicyEditorSessionViewModelTests.cs b/src/UniGetUI.Tests/PolicyEditor/PolicyEditorSessionViewModelTests.cs index 82f068a7d1..38a428f252 100644 --- a/src/UniGetUI.Tests/PolicyEditor/PolicyEditorSessionViewModelTests.cs +++ b/src/UniGetUI.Tests/PolicyEditor/PolicyEditorSessionViewModelTests.cs @@ -195,6 +195,81 @@ public async Task ConfirmOverwriteCommand_AlwaysReValidatesEvenWhenCurrentValida Assert.True(vm.LastSaveSucceeded); } + [Fact] + public async Task SuccessfulSave_RejectsPreSaveAutomaticValidation() + { + PolicyEditorSession session = PolicyEditorSession.StartUpdate(PolicyEditorTestFixtures.BuildActiveManagement(PolicyEditorTestFixtures.BuildDocument(id: "id-1"), "token-1")); + var validation = new GatedValidationClient(); + var writer = new FakeWriteClient(); + using var vm = new PolicyEditorSessionViewModel(session, validation, new FakeConfirmationPrompt(), writer, structuredDirtyDebounce: TimeSpan.Zero); + GatedValidationCall stale = validation.QueueCall(); + vm.Draft.Metadata.Description = "draft"; + vm.NotifyDraftChangedCommand.Execute(null); + await stale.Started.Task; + GatedValidationCall save = validation.QueueCall(); + PolicyDocument authoritative = PolicyEditorTestFixtures.BuildDocument(id: "id-1"); + authoritative.Metadata.Description = "draft"; + writer.NextOutcome = PolicyWriteOutcome.Success(PolicyEditorTestFixtures.BuildReplacementResponse(authoritative, "token-2")); + Task saveTask = vm.SaveCommand.ExecuteAsync(null); + await save.Started.Task; + save.Completion.SetResult(new PolicyEditorValidationOutcome(ValidResultFor(vm, "save"))); + await saveTask; + stale.Completion.SetResult(new PolicyEditorValidationOutcome(ValidResultFor(vm, "stale", [new PolicyFinding { Path = "/Rules", Severity = PolicyFindingSeverity.Error, Message = "stale" }]))); + Assert.Null(vm.Session.Validation); + Assert.Empty(vm.Findings); + Assert.False(vm.Session.IsValidationCurrent); + } + + [Fact] + public async Task SuccessfulSaveWithNewerDraft_ReschedulesValidationForPreservedDraft() + { + PolicyEditorSession session = PolicyEditorSession.StartUpdate(PolicyEditorTestFixtures.BuildActiveManagement(PolicyEditorTestFixtures.BuildDocument(id: "id-1"), "token-1")); + var validation = new GatedValidationClient(); + var writer = new FakeWriteClient { Started = new TaskCompletionSource(), Gate = new TaskCompletionSource() }; + using var vm = new PolicyEditorSessionViewModel(session, validation, new FakeConfirmationPrompt(), writer, structuredDirtyDebounce: TimeSpan.Zero); + GatedValidationCall save = validation.QueueCall(); + PolicyDocument authoritative = PolicyEditorTestFixtures.BuildDocument(id: "id-1"); + writer.NextOutcome = PolicyWriteOutcome.Success(PolicyEditorTestFixtures.BuildReplacementResponse(authoritative, "token-2")); + Task saveTask = vm.SaveCommand.ExecuteAsync(null); + await save.Started.Task; + save.Completion.SetResult(new PolicyEditorValidationOutcome(ValidResultFor(vm, "save"))); + await writer.Started.Task; + GatedValidationCall stale = validation.QueueCall(); + vm.Draft.Metadata.Description = "newer"; + vm.NotifyDraftChangedCommand.Execute(null); + await stale.Started.Task; + GatedValidationCall fresh = validation.QueueCall(); + writer.Gate.SetResult(); + await saveTask; + await fresh.Started.Task; + stale.Completion.SetResult(new PolicyEditorValidationOutcome(ValidResultFor(vm, "stale"))); + fresh.Completion.SetResult(new PolicyEditorValidationOutcome(ValidResultFor(vm, "fresh"))); + await vm.WaitForAuthoritativeValidationAsync(); + Assert.True(vm.SavedWithNewerChanges); + Assert.True(vm.Session.IsValidationCurrent); + Assert.Equal("fresh", vm.Session.Validation!.Receipt); + Assert.Equal("newer", vm.Draft.Metadata.Description); + } + + [Fact] + public async Task OriginTokenChange_RejectsStaleAutomaticValidationReceipt() + { + PolicyEditorSession session = PolicyEditorSession.StartUpdate(PolicyEditorTestFixtures.BuildActiveManagement(PolicyEditorTestFixtures.BuildDocument(id: "id-1"), "token-1")); + var validation = new GatedValidationClient(); + using var vm = new PolicyEditorSessionViewModel(session, validation, new FakeConfirmationPrompt(), new FakeWriteClient(), structuredDirtyDebounce: TimeSpan.Zero); + GatedValidationCall stale = validation.QueueCall(); + vm.Draft.Metadata.Description = "draft"; + vm.NotifyDraftChangedCommand.Execute(null); + await stale.Started.Task; + PolicyDocument authoritative = PolicyEditorTestFixtures.BuildDocument(id: "id-1"); + authoritative.Metadata.Description = "draft"; + session.MarkSaved(PolicyEditorTestFixtures.BuildReplacementResponse(authoritative, "token-2")); + stale.Completion.SetResult(new PolicyEditorValidationOutcome(ValidResultFor(vm, "stale"))); + Assert.Equal("token-2", session.OriginManagement.StoreToken); + Assert.Null(session.Validation); + Assert.False(session.IsValidationCurrent); + } + // ---- Raw/structured mode switching (correction #3) --------------------------------------- [Fact] diff --git a/src/UniGetUI.Tests/PolicyEditor/PolicyEditorViewModelFakes.cs b/src/UniGetUI.Tests/PolicyEditor/PolicyEditorViewModelFakes.cs index f5aa8eb16b..7d9cd69984 100644 --- a/src/UniGetUI.Tests/PolicyEditor/PolicyEditorViewModelFakes.cs +++ b/src/UniGetUI.Tests/PolicyEditor/PolicyEditorViewModelFakes.cs @@ -71,6 +71,7 @@ internal sealed class FakeWriteClient : IPolicyWriteClient public int CallCount { get; private set; } public PolicyEditorWriteRequest? LastRequest { get; private set; } + public TaskCompletionSource? Started { get; set; } public TaskCompletionSource? Gate { get; set; } public async Task WriteAsync( @@ -79,8 +80,27 @@ public async Task WriteAsync( { CallCount++; LastRequest = request; + Started?.TrySetResult(); if (Gate is not null) await Gate.Task; return NextOutcome; } } + +internal sealed class GatedValidationClient : IPolicyValidationClient +{ + private readonly Queue _calls = []; + public GatedValidationCall QueueCall() { var call = new GatedValidationCall(); _calls.Enqueue(call); return call; } + public async Task ValidateAsync(JsonElement draft, CancellationToken cancellationToken) + { + GatedValidationCall call = _calls.Dequeue(); + call.Started.SetResult(); + return await call.Completion.Task; + } +} + +internal sealed class GatedValidationCall +{ + public TaskCompletionSource Started { get; } = new(); + public TaskCompletionSource Completion { get; } = new(); +} diff --git a/src/UniGetUI.Tests/SettingsSearchIndexTests.cs b/src/UniGetUI.Tests/SettingsSearchIndexTests.cs index e4e9de29b1..ba4acb4abd 100644 --- a/src/UniGetUI.Tests/SettingsSearchIndexTests.cs +++ b/src/UniGetUI.Tests/SettingsSearchIndexTests.cs @@ -14,9 +14,9 @@ public class SettingsSearchIndexTests public void Search_ReturnsPolicyInspectorOnWindows(string query) { // The base "Inspect active package broker policy" entry (Anchor == null) must remain the - // unique unanchored match. Phase 2 policy-management action entries (Edit/Create/ - // Replace identity, all anchored) intentionally also match the "policy" query, so we only - // assert uniqueness of the unanchored base entry rather than of the whole result set. + // unique unanchored match. The anchored policy-management action entries (Edit/Create/ + // Replace identity) intentionally also match the "policy" query, so we only assert + // uniqueness of the unanchored base entry rather than of the whole result set. SettingsSearchResult result = Assert.Single( SettingsSearchIndex.Search(query, limit: 100, isWindows: true), result => result.PageType == typeof(AgentPolicyInspector) && result.Anchor is null);