Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .github/workflows/dotnetcore.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,17 @@ jobs:
- name: Push nuget packages to GitHub registry
if: github.event_name == 'push'
run: dotnet nuget push ./out/*.nupkg --skip-duplicate --no-symbols -s "github"

windows-tests:
runs-on: windows-latest
Comment on lines +42 to +43

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Gate package publishing on the Windows test job

On pushes to master, this newly added job runs independently of the existing build job, whose final steps publish the package to NuGet and GitHub. Because jobs without a needs relationship run in parallel, version 8.1.2 can be published before these Windows-only tests finish—and remains published even if they fail—so the new coverage does not protect releases of the Windows-only atomic-write path. Move publishing into a job that depends on both test jobs, or otherwise make publishing wait for windows-tests.

Useful? React with 👍 / 👎.


steps:
- uses: actions/checkout@v4
- name: Setup .NET Core
uses: actions/setup-dotnet@v4
with:
dotnet-version: 8.0.x
- name: Restore
run: dotnet restore
- name: Test Windows atomic writes
run: dotnet test --configuration Release --no-restore
9 changes: 9 additions & 0 deletions WritableJsonConfiguration.Tests/AtomicWriteTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,11 @@ public void PersistedProtectedBootstrapBackupAllowsSuccessiveSavesOfInheritedMai
using var root = (IDisposable)Create();
var configuration = (IConfigurationRoot)root;
configuration["Theme"] = "first";
var mainPermissions = new FileInfo(SettingsPath).GetAccessControl(AccessControlSections.Access);
var backupPermissions = new FileInfo(SettingsPath + ".bak").GetAccessControl(AccessControlSections.Access);
Assert.True(AtomicSettingsFile.HaveEquivalentAccess(mainPermissions, backupPermissions),
$"Main ACL: {mainPermissions.GetSecurityDescriptorSddlForm(AccessControlSections.Access)}; " +
$"backup ACL: {backupPermissions.GetSecurityDescriptorSddlForm(AccessControlSections.Access)}");
configuration["Theme"] = "second";
configuration["Theme"] = "third";
Assert.Equal("third", configuration["Theme"]);
Expand All @@ -369,6 +374,10 @@ public void AccessComparisonIgnoresOnlyProvenanceAndOrderWithinSameKind()
var inherited = Security("D:AI(A;ID;FR;;;SY)(A;ID;FA;;;BA)");
var explicitReversed = Security("D:P(A;;FA;;;BA)(A;;FR;;;SY)");
Assert.True(AtomicSettingsFile.HaveEquivalentAccess(inherited, explicitReversed));
Assert.True(AtomicSettingsFile.HaveEquivalentAccess(
Security("D:P(A;OICINP;FR;;;SY)"), Security("D:P(A;;FR;;;SY)")));
Assert.True(AtomicSettingsFile.HaveEquivalentAccess(
Security("D:P(A;;FR;;;SY)(A;ID;FR;;;SY)"), Security("D:P(A;ID;FR;;;SY)")));
Assert.False(AtomicSettingsFile.HaveEquivalentAccess(inherited, Security("D:P(A;;FA;;;BA)(A;;FA;;;SY)")));
Assert.False(AtomicSettingsFile.HaveEquivalentAccess(inherited, Security("D:P(A;;FA;;;BA)(A;;FR;;;WD)")));
Assert.False(AtomicSettingsFile.HaveEquivalentAccess(inherited, Security("D:P(A;;FA;;;BA)(A;IO;FR;;;SY)")));
Expand Down
27 changes: 22 additions & 5 deletions src/WritableJsonConfiguration/AtomicSettingsFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,11 @@ private static string AccessFingerprint(FileSecurity security)
for (int index = 0; index < dacl.Count; index++)
{
var ace = dacl[index];
// Windows removes inherited provenance and reorders allow ACEs when persisting
// a protected copy. Neither changes access. Never reorder deny across allow:
// their relative position can change effective permissions.
// Windows removes inherited provenance and can normalize propagation flags when
// persisting an ACL on a file. Those flags only control inheritance by children;
// a file cannot have children. InheritOnly still changes access to this file and
// must remain significant. Never reorder deny across allow: their relative
// position can change effective permissions.
var common = ace as CommonAce;
bool canReorder = common != null && !common.IsCallback &&
(common.AceQualifier == AceQualifier.AccessAllowed || common.AceQualifier == AceQualifier.AccessDenied);
Expand All @@ -92,7 +94,9 @@ private static string AccessFingerprint(FileSecurity security)
var bytes = new byte[ace.BinaryLength];
ace.GetBinaryForm(bytes, 0);
var normalized = GenericAce.CreateFromBinaryForm(bytes, 0);
if (canReorder) normalized.AceFlags &= ~AceFlags.Inherited;
if (canReorder)
normalized.AceFlags &= ~(AceFlags.Inherited | AceFlags.ObjectInherit |
AceFlags.ContainerInherit | AceFlags.NoPropagateInherit);
normalized.GetBinaryForm(bytes, 0);
group.Add(Convert.ToBase64String(bytes));
}
Expand All @@ -104,7 +108,20 @@ private static void AppendGroup(StringBuilder result, List<string> group, int ty
{
if (group.Count == 0) return;
group.Sort(StringComparer.Ordinal);
result.Append(type).Append(':').Append(string.Join(",", group)).Append(';');
result.Append(type).Append(':');
string previous = null;
bool appended = false;
foreach (var entry in group)
{
// File.Replace can retain the same effective ACE once as explicit and once as
// inherited. After provenance normalization, exact duplicates do not change access.
if (entry == previous) continue;
if (appended) result.Append(',');
result.Append(entry);
previous = entry;
appended = true;
}
result.Append(';');
group.Clear();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
<Authors>Kibnet</Authors>
<RepositoryUrl>https://github.com/Kibnet/WritableJsonConfiguration</RepositoryUrl>
<PackageProjectUrl>https://github.com/Kibnet/WritableJsonConfiguration</PackageProjectUrl>
<PackageVersion>8.1.1</PackageVersion>
<PackageVersion>8.1.2</PackageVersion>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Update the release metadata for version 8.1.2

When this version is packed and published, PackageReleaseNotes still describes the 8.1.1 numeric-path fix, and CHANGELOG.md still ends at 8.1.1. Consequently package consumers see no mention of the ACL-normalization change that actually distinguishes 8.1.2. Update the package release notes and changelog alongside this version bump.

Useful? React with 👍 / 👎.

<Description>Source of configurations in JSON format with the ability to edit values directly from the running application. Based on Microsoft.Extensions.Configuration.</Description>
<PackageIcon>JSON_logo.png</PackageIcon>
<PackageReadmeFile>README.md</PackageReadmeFile>
<PackageLicenseExpression>MIT</PackageLicenseExpression>
<PackageReleaseNotes>Fix atomic-path handling for numeric object keys and reject sparse array indexes instead of writing a different index.</PackageReleaseNotes>
<PackageReleaseNotes>Fix false atomic-write ACL mismatches after Windows normalizes equivalent file access entries during File.Replace.</PackageReleaseNotes>
</PropertyGroup>

<ItemGroup>
Expand Down
Loading