Skip to content

Commit 2715e3f

Browse files
CBenoitCopilot
andcommitted
Harden package policy editor
Address policy editing, validation, elevation protocol, cancellation, and NativeAOT safety findings while retaining authoritative Agent behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 3c4e399 commit 2715e3f

32 files changed

Lines changed: 2102 additions & 762 deletions

src/Languages/lang_en.json

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1216,7 +1216,6 @@
12161216
"Correct the highlighted fields": "Correct the highlighted fields",
12171217
"Devolutions Agent reported an unrecognized policy finding.": "Devolutions Agent reported an unrecognized policy finding.",
12181218
"Enter a valid ISO 8601 date and time.": "Enter a valid ISO 8601 date and time.",
1219-
"Enter a whole number from 0 through 4294967295.": "Enter a whole number from 0 through 4294967295.",
12201219
"Policy management state": "Policy management state",
12211220
"Policy operation in progress": "Policy operation in progress",
12221221
"Policy path source": "Policy path source",
@@ -1246,5 +1245,45 @@
12461245
"The policy was saved, but newer draft changes remain unsaved.": "The policy was saved, but newer draft changes remain unsaved.",
12471246
"Policy saved; newer changes remain": "Policy saved; newer changes remain",
12481247
"{0} additional validation finding(s) were omitted.": "{0} additional validation finding(s) were omitted.",
1249-
"{0}. Location: {1}": "{0}. Location: {1}"
1248+
"{0}. Location: {1}": "{0}. Location: {1}",
1249+
"Policy saved, then replaced again": "Policy saved, then replaced again",
1250+
"The policy was saved, but another writer replaced it before management state was refreshed.": "The policy was saved, but another writer replaced it before management state was refreshed.",
1251+
"The policy write result is unknown. Refresh policy management state before retrying.": "The policy write result is unknown. Refresh policy management state before retrying.",
1252+
"The document is not a valid policy draft": "The document is not a valid policy draft",
1253+
"The document is empty.": "The document is empty.",
1254+
"The JSON syntax is invalid.": "The JSON syntax is invalid.",
1255+
"The policy draft uses an unsupported schema.": "The policy draft uses an unsupported schema.",
1256+
"The policy draft uses an unsupported policy type.": "The policy draft uses an unsupported policy type.",
1257+
"The policy draft is missing the Enforcement object.": "The policy draft is missing the Enforcement object.",
1258+
"The policy draft uses an unsupported rule precedence.": "The policy draft uses an unsupported rule precedence.",
1259+
"The policy draft is missing the Metadata object.": "The policy draft is missing the Metadata object.",
1260+
"The document does not match the policy draft format.": "The document does not match the policy draft format.",
1261+
"Enter a whole number from 0 through 2147483647.": "Enter a whole number from 0 through 2147483647.",
1262+
"Allowed custom locations": "Allowed custom locations",
1263+
"Allowed hash-check skipping": "Allowed hash-check skipping",
1264+
"Allowed pre/post commands": "Allowed pre/post commands",
1265+
"AdministratorRequired": "Administrator required",
1266+
"BadRequest": "Bad request",
1267+
"BrokerPaused": "Broker paused",
1268+
"Conflict": "Conflict",
1269+
"Forbidden": "Forbidden",
1270+
"Info": "Information",
1271+
"InternalError": "Internal error",
1272+
"InvalidPolicy": "Invalid policy",
1273+
"MalformedDraft": "Malformed draft",
1274+
"NotFound": "Not found",
1275+
"PayloadTooLarge": "Payload too large",
1276+
"PolicyActivationFailed": "Policy activation failed",
1277+
"PolicyPersistenceFailed": "Policy persistence failed",
1278+
"StalePolicyStoreToken": "The policy changed since editing began",
1279+
"Timeout": "Timed out",
1280+
"Unauthenticated": "Authentication required",
1281+
"Unauthorized": "Unauthorized",
1282+
"UnsafePolicyPath": "Unsafe policy path",
1283+
"UnsupportedEndpoint": "Unsupported endpoint",
1284+
"UnsupportedMediaType": "Unsupported media type",
1285+
"UnsupportedPolicyFilesystem": "Unsupported policy filesystem",
1286+
"UnsupportedPolicyFormat": "Unsupported policy format",
1287+
"ValidationFailed": "Validation failed",
1288+
"WarningConfirmationRequired": "Warning confirmation required"
12501289
}

src/UniGetUI.AgentPolicy.ElevatedHelper/PolicyReplacementExecutor.cs

Lines changed: 31 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -36,54 +36,57 @@ public static async Task<PolicyElevationResponseMessage> ExecuteAsync(
3636
},
3737
cancellationToken).ConfigureAwait(false);
3838

39-
response.Outcome = PolicyElevationResponseStatus.Replaced;
40-
response.Payload = SerializePayload(replacement);
39+
response.Disposition = PolicyElevationDisposition.Committed;
40+
response.CommittedStoreToken = replacement.Management.StoreToken;
41+
PolicyElevationFrame.ValidateResponse(response);
4142
return response;
4243
}
4344
catch (BrokerClientException ex)
4445
{
45-
response.Outcome = ex.Kind switch
46-
{
47-
BrokerClientErrorKind.BrokerUnavailable => PolicyElevationResponseStatus.BrokerUnavailable,
48-
BrokerClientErrorKind.Timeout => PolicyElevationResponseStatus.BrokerUnavailable,
49-
BrokerClientErrorKind.EmptyResponse => PolicyElevationResponseStatus.BrokerInvalidResponse,
50-
BrokerClientErrorKind.InvalidResponse => PolicyElevationResponseStatus.BrokerInvalidResponse,
51-
_ => PolicyElevationResponseStatus.BrokerRejected,
52-
};
53-
46+
response.Disposition = ex.Kind is
47+
BrokerClientErrorKind.BrokerUnavailable
48+
or BrokerClientErrorKind.Timeout
49+
or BrokerClientErrorKind.EmptyResponse
50+
or BrokerClientErrorKind.InvalidResponse
51+
? PolicyElevationDisposition.Unknown
52+
: PolicyElevationDisposition.Rejected;
5453
response.BrokerStatusCode = ex.StatusCode;
5554
response.BrokerErrorCode = Truncate(
5655
ex.BrokerError?.Code.ToString() ?? ex.Kind.ToString(),
5756
PolicyElevationProtocol.MaxBrokerErrorCodeCharacters);
58-
response.Message = "The Agent rejected the policy write.";
59-
response.Payload = ex.BrokerError is null
60-
? null
61-
: SerializePayload(ex.BrokerError);
57+
if (response.Disposition == PolicyElevationDisposition.Rejected
58+
&& ex.BrokerError is
59+
{
60+
Code: ErrorCode.StalePolicyStoreToken,
61+
Management: not null,
62+
} stale)
63+
{
64+
response.ConflictStoreToken = stale.Management.StoreToken;
65+
response.ConflictState = stale.Management.State switch
66+
{
67+
PolicyManagementState.Active => PolicyElevationManagementState.Active,
68+
PolicyManagementState.Missing => PolicyElevationManagementState.Missing,
69+
PolicyManagementState.Invalid => PolicyElevationManagementState.Invalid,
70+
_ => throw new InvalidDataException("The stale response carried an invalid management state."),
71+
};
72+
response.ConflictPolicyId = stale.Management.Policy?.Metadata.Id;
73+
}
6274
return response;
6375
}
6476
catch (OperationCanceledException)
6577
{
66-
response.Outcome = PolicyElevationResponseStatus.BrokerUnavailable;
67-
response.Message = "The broker did not answer before the elevated helper timed out.";
78+
response.Disposition = PolicyElevationDisposition.Unknown;
79+
response.BrokerErrorCode = BrokerClientErrorKind.Timeout.ToString();
6880
return response;
6981
}
7082
catch (Exception ex) when (ex is IOException or InvalidOperationException or JsonException)
7183
{
72-
response.Outcome = PolicyElevationResponseStatus.BrokerInvalidResponse;
73-
response.Message = "The Agent returned an invalid policy response.";
84+
response.Disposition = PolicyElevationDisposition.Unknown;
85+
response.BrokerErrorCode = BrokerClientErrorKind.InvalidResponse.ToString();
7486
return response;
7587
}
7688
}
7789

78-
public static PolicyElevationResponseMessage Rejected(string requestId, string reason)
79-
=> new()
80-
{
81-
ProtocolVersion = PolicyElevationProtocol.Version,
82-
RequestId = requestId,
83-
Outcome = PolicyElevationResponseStatus.HelperRejected,
84-
Message = Truncate(reason, PolicyElevationProtocol.MaxMessageCharacters),
85-
};
86-
8790
private static BrokerClient CreateClient()
8891
=> new(new BrokerClientOptions
8992
{
@@ -98,12 +101,6 @@ private static string GetEffectiveUser()
98101
? Environment.UserName
99102
: $"{Environment.UserDomainName}\\{Environment.UserName}";
100103

101-
private static JsonElement SerializePayload<T>(T payload)
102-
{
103-
using JsonDocument document = JsonDocument.Parse(BrokerSerializer.Serialize(payload));
104-
return document.RootElement.Clone();
105-
}
106-
107104
private static string? Truncate(string? value, int maxCharacters)
108105
{
109106
if (value is null)

src/UniGetUI.AgentPolicy.ElevatedHelper/Program.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,9 @@ await pipe.ConnectAsync((int)PolicyElevationProtocol.ConnectTimeout.TotalMillise
184184
// flushed, under the same bounded, cancellable token as every other stage. Closing the
185185
// handle afterwards is enough: a synchronous drain would block on the reader with no
186186
// timeout and no cancellation, which is exactly the unbounded hang this design forbids.
187-
await PolicyElevationFrame.WriteResponseAsync(pipe, response, cancellationToken).ConfigureAwait(false);
187+
using var responseWrite =
188+
new CancellationTokenSource(PolicyElevationProtocol.ResponseWriteTimeout);
189+
await PolicyElevationFrame.WriteResponseAsync(pipe, response, responseWrite.Token).ConfigureAwait(false);
188190

189191
return PolicyElevationProtocol.ExitSuccess;
190192
}

src/UniGetUI.AgentPolicy.ElevatedHelper/UniGetUI.AgentPolicy.ElevatedHelper.csproj

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@
3636
</PropertyGroup>
3737

3838
<ItemGroup>
39-
<PackageReference Include="Devolutions.Now.Policy.Api" Version="2026.8.29-phase2.bdf80471" />
40-
<PackageReference Include="Devolutions.Now.Policy.Client" Version="2026.8.29-phase2.bdf80471" />
39+
<PackageReference Include="Devolutions.Now.Policy.Api" Version="2026.9.3" />
40+
<PackageReference Include="Devolutions.Now.Policy.Client" Version="2026.9.3" />
4141
</ItemGroup>
4242

4343
<ItemGroup>

src/UniGetUI.Avalonia/ViewModels/Pages/SettingsPages/PolicyEditor/PolicyEditorDependencies.cs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,22 +47,28 @@ public enum PolicyWriteFailureKind
4747
ProtocolFailed,
4848
HelperFailed,
4949
BrokerRejected,
50+
WriteResultUnknown,
5051
}
5152

5253
public sealed record PolicyWriteOutcome(
5354
PolicyReplacementResponse? Response,
5455
ErrorResponse? Error,
55-
PolicyWriteFailureKind FailureKind = PolicyWriteFailureKind.None)
56+
PolicyWriteFailureKind FailureKind = PolicyWriteFailureKind.None,
57+
PolicyEditorRetryDecision? ConflictDecision = null,
58+
bool SavedThenSuperseded = false)
5659
{
5760
public bool Succeeded => Response is not null;
5861

59-
public static PolicyWriteOutcome Success(PolicyReplacementResponse response) =>
60-
new(response, null);
62+
public static PolicyWriteOutcome Success(
63+
PolicyReplacementResponse response,
64+
bool savedThenSuperseded = false) =>
65+
new(response, null, SavedThenSuperseded: savedThenSuperseded);
6166

6267
public static PolicyWriteOutcome Failure(
6368
PolicyWriteFailureKind kind,
64-
ErrorResponse? error = null) =>
65-
new(null, error, kind);
69+
ErrorResponse? error = null,
70+
PolicyEditorRetryDecision? conflictDecision = null) =>
71+
new(null, error, kind, conflictDecision);
6672
}
6773

6874
public interface IPolicyWriteClient
@@ -79,7 +85,8 @@ public sealed record PolicyEditorConfirmationRequest(
7985
string ExpectedStoreToken,
8086
PolicyManagementState State,
8187
string? ActivePolicyId,
82-
IReadOnlyList<PolicyValidationFinding> Findings);
88+
IReadOnlyList<PolicyValidationFinding> Findings,
89+
int WarningCount = 0);
8390

8491
public interface IPolicyEditorConfirmationPrompt
8592
{

src/UniGetUI.Avalonia/ViewModels/Pages/SettingsPages/PolicyEditor/PolicyEditorDialogViewModel.cs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,15 @@ private void RefreshStatus()
166166
return;
167167
}
168168

169+
if (Session.SavedThenSuperseded)
170+
{
171+
SetStatus(
172+
CoreTools.Translate("Policy saved, then replaced again"),
173+
CoreTools.Translate("The policy was saved, but another writer replaced it before management state was refreshed."),
174+
InfoBarSeverity.Warning);
175+
return;
176+
}
177+
169178
if (Session.LastSaveSucceeded)
170179
{
171180
SetStatus(
@@ -178,8 +187,8 @@ private void RefreshStatus()
178187
if (Session.SyntaxError is { } syntaxError)
179188
{
180189
SetStatus(
181-
CoreTools.Translate("The document is not valid JSON"),
182-
syntaxError.Message,
190+
Session.SyntaxErrorTitle,
191+
Session.SyntaxErrorMessage,
183192
InfoBarSeverity.Error);
184193
return;
185194
}
@@ -241,6 +250,8 @@ private static string DescribeWriteFailure(PolicyWriteFailureKind kind, ErrorCod
241250
CoreTools.Translate("The elevated helper stopped unexpectedly."),
242251
PolicyWriteFailureKind.BrokerRejected =>
243252
CoreTools.Translate("Devolutions Agent rejected the policy replacement."),
253+
PolicyWriteFailureKind.WriteResultUnknown =>
254+
CoreTools.Translate("The policy write result is unknown. Refresh policy management state before retrying."),
244255
_ => null,
245256
};
246257

src/UniGetUI.Avalonia/ViewModels/Pages/SettingsPages/PolicyEditor/PolicyEditorProductionAdapters.cs

Lines changed: 90 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,15 +83,19 @@ .. outcome.Diagnostics.Findings.Select(PolicyValidationFinding.FromSanitized),
8383
public sealed class WindowsPolicyEditorWriteClient : IPolicyWriteClient
8484
{
8585
private readonly IPolicyWriteElevator _elevator;
86+
private readonly IBrokerPolicyManagementService _managementService;
8687

8788
public WindowsPolicyEditorWriteClient()
88-
: this(CreateDefaultElevator())
89+
: this(CreateDefaultElevator(), new BrokerPolicyManagementService())
8990
{
9091
}
9192

92-
public WindowsPolicyEditorWriteClient(IPolicyWriteElevator elevator)
93+
public WindowsPolicyEditorWriteClient(
94+
IPolicyWriteElevator elevator,
95+
IBrokerPolicyManagementService? managementService = null)
9396
{
9497
_elevator = elevator;
98+
_managementService = managementService ?? new BrokerPolicyManagementService();
9599
}
96100

97101
private static IPolicyWriteElevator CreateDefaultElevator()
@@ -124,17 +128,62 @@ public async Task<PolicyWriteOutcome> WriteAsync(PolicyEditorWriteRequest reques
124128
throw;
125129
}
126130

127-
if (result.Succeeded && result.Response is not null)
131+
if (result.Outcome is PolicyElevationOutcome.Cancelled)
128132
{
129-
return PolicyWriteOutcome.Success(result.Response);
133+
if (cancellationToken.IsCancellationRequested)
134+
throw new OperationCanceledException(cancellationToken);
135+
136+
Logger.Warn(
137+
"[PolicyEditor] The elevation path reported cancellation without caller cancellation.");
138+
return PolicyWriteOutcome.Failure(PolicyWriteFailureKind.ProtocolFailed);
139+
}
140+
141+
if (result.Succeeded && result.CommittedStoreToken is not null)
142+
{
143+
BrokerPolicyManagementResult refreshed =
144+
await _managementService.GetManagementAsync(CancellationToken.None).ConfigureAwait(false);
145+
if (refreshed is
146+
{
147+
Status: BrokerPolicyManagementStatus.Retrieved,
148+
Snapshot:
149+
{
150+
State: PolicyManagementState.Active,
151+
Policy: not null,
152+
} snapshot,
153+
})
154+
{
155+
var response = new PolicyReplacementResponse
156+
{
157+
Policy = snapshot.Policy,
158+
Management = snapshot,
159+
};
160+
return PolicyWriteOutcome.Success(
161+
response,
162+
savedThenSuperseded: !string.Equals(
163+
result.CommittedStoreToken,
164+
snapshot.StoreToken,
165+
StringComparison.Ordinal));
166+
}
167+
168+
Logger.Warn(
169+
"[PolicyEditor] The Agent committed the policy, but management state could not be refreshed.");
170+
return PolicyWriteOutcome.Failure(PolicyWriteFailureKind.WriteResultUnknown);
130171
}
131172

132173
if (result.ErrorMessage is not null)
133174
{
134175
Logger.Warn($"[PolicyEditor] Elevated policy write did not succeed ({result.Outcome}): {result.ErrorMessage}");
135176
}
136177

137-
return PolicyWriteOutcome.Failure(MapFailureKind(result.Outcome), result.Error);
178+
ErrorCode? errorCode = TryParseErrorCode(result.BrokerErrorCode);
179+
ErrorResponse? error = errorCode is null
180+
? null
181+
: new ErrorResponse { Code = errorCode.Value };
182+
PolicyEditorRetryDecision? conflict = BuildConflictDecision(request, result, errorCode);
183+
return PolicyWriteOutcome.Failure(
184+
MapFailureKind(result.Outcome),
185+
error,
186+
conflict);
138187
}
139188

140189
private static PolicyWriteFailureKind MapFailureKind(PolicyElevationOutcome outcome) => outcome switch
@@ -154,10 +203,45 @@ or PolicyElevationOutcome.TimedOut
154203
PolicyElevationOutcome.BrokerRejected
155204
or PolicyElevationOutcome.BrokerUnavailable
156205
or PolicyElevationOutcome.BrokerInvalidResponse => PolicyWriteFailureKind.BrokerRejected,
157-
PolicyElevationOutcome.Cancelled => PolicyWriteFailureKind.LaunchFailed,
206+
PolicyElevationOutcome.WriteResultUnknown => PolicyWriteFailureKind.WriteResultUnknown,
207+
PolicyElevationOutcome.Cancelled => PolicyWriteFailureKind.ProtocolFailed,
158208
_ => PolicyWriteFailureKind.HelperFailed,
159209
};
160210

211+
private static ErrorCode? TryParseErrorCode(string? value) =>
212+
Enum.TryParse(value, ignoreCase: false, out ErrorCode parsed)
213+
&& Enum.IsDefined(parsed)
214+
? parsed
215+
: null;
216+
217+
private static PolicyEditorRetryDecision? BuildConflictDecision(
218+
PolicyEditorWriteRequest request,
219+
PolicyElevationResult result,
220+
ErrorCode? errorCode)
221+
{
222+
if (errorCode != ErrorCode.StalePolicyStoreToken
223+
|| result.ConflictStoreToken is null
224+
|| result.ConflictState is null)
225+
{
226+
return null;
227+
}
228+
229+
PolicyManagementState state = result.ConflictState.Value switch
230+
{
231+
PolicyElevationManagementState.Active => PolicyManagementState.Active,
232+
PolicyElevationManagementState.Missing => PolicyManagementState.Missing,
233+
PolicyElevationManagementState.Invalid => PolicyManagementState.Invalid,
234+
_ => throw new InvalidDataException("The helper returned an invalid conflict state."),
235+
};
236+
string draftId = request.Draft.GetProperty("Metadata").GetProperty("Id").GetString()
237+
?? throw new InvalidDataException("The validated draft did not carry an identity.");
238+
return PolicyEditorRetryResolver.Resolve(
239+
draftId,
240+
state,
241+
result.ConflictStoreToken,
242+
result.ConflictPolicyId);
243+
}
244+
161245
private static PolicyElevationOperation MapOperation(PolicyReplacementOperation operation) => operation switch
162246
{
163247
PolicyReplacementOperation.Update => PolicyElevationOperation.Update,

0 commit comments

Comments
 (0)