Skip to content

fix(settings): normalize file-only ACL flags - #31

Merged
Kibnet merged 3 commits into
masterfrom
fix/windows-acl-normalization
Sep 21, 2026
Merged

Kibnet merged 3 commits into
masterfrom
fix/windows-acl-normalization

Conversation

@Kibnet

@Kibnet Kibnet commented Sep 21, 2026

Copy link
Copy Markdown
Owner

Summary

  • Fixes false ACL incompatibility after Windows normalizes inheritance-propagation flags during File.Replace.
  • Keeps effective file-access checks strict: SID, mask, allow/deny ordering and InheritOnly remain significant.
  • Adds a Windows CI job because atomic writes are intentionally Windows-only.

Changes

  • Ignore ObjectInherit, ContainerInherit and NoPropagateInherit when comparing ACEs for a file; these flags only control propagation to child objects, and a file cannot have children.
  • Add regression coverage while preserving the negative InheritOnly case.
  • Bump package version to 8.1.2.

Validation

  • Local Windows Release suite: 43 passed, 1 non-Windows-only test skipped.
  • Package build: WritableJsonConfiguration.8.1.2.nupkg created successfully.
  • Unlimotion Settings-focused integration against the local 8.1.2 package: 123/123 passed.
  • Triggering incident: Unlimotion PR #305 on GitHub-hosted Windows produced six false ACL mismatches after restore/first save with 8.1.1.

Risks / Rollback

  • Normalization is limited to propagation-only flags on file ACEs; InheritOnly is deliberately not ignored.
  • Rollback by reverting this commit and returning the package version to 8.1.1.

Links

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-21T13:14:04.473297Z b70e9ae PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b70e9ae130

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

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

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 👍 / 👎.

<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 👍 / 👎.

@Kibnet
Kibnet merged commit 5ed17da into master Sep 21, 2026
2 checks passed
@Kibnet
Kibnet deleted the fix/windows-acl-normalization branch September 21, 2026 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant