fix(windows): validate PLUGIN_DATA using Windows ACLs

[Zythenth]

Summary

• validate PLUGIN_DATA, existing state files, and backups on Windows through native owner and ACL SIDs instead of Bun's POSIX mode projection
• reject unexpected owners, null DACLs, and allow ACEs outside the approved Windows principals
• validate config.json, managed-files.json, and transaction.json before reading them
• validate existing .bak files before reusing the backup store and verify generated backups after exclusive creation
• preserve the existing POSIX permission behavior on Linux and macOS
• retain directory fsync durability on Windows by opening directories with r+
• locate the OS-provided Windows PowerShell from SystemRoot / WINDIR first, with drive-based fallbacks
• derive the filesystem root from the PLUGIN_DATA path itself, so cross-drive roots and junction ancestors are validated correctly
• run the MCP test suite on Ubuntu, macOS, and Windows

Root cause

Bun projects Windows file permissions as POSIX mode bits. A private Windows directory can therefore appear as mode 0666, causing the MCP server to reject a valid PLUGIN_DATA directory before setup can start.

The fix uses Windows PowerShell 5.1 to inspect native security descriptors while leaving the Linux and macOS code path unchanged.

Security follow-up

Windows authorizes each file from its own security descriptor; the parent directory ACL is not sufficient when a caller can bypass traverse checking and knows the exact child path. The same owner/DACL policy is now enforced on every pre-existing state file before it is read. Backups are checked before reuse, and a newly copied backup is removed if its post-copy ACL verification fails.

References:

• https://learn.microsoft.com/en-us/windows/win32/fileio/file-security-and-access-rights
• https://learn.microsoft.com/en-us/previous-versions/windows/it-pro/windows-10/security/threat-protection/security-policy-settings/bypass-traverse-checking

User impact

Sol Advisor setup can now run with a correctly private Windows PLUGIN_DATA directory. Unsafe owners, null DACLs, broad grants such as Everyone, Users, or Authenticated Users, and permissive pre-existing state files fail closed.

Validation

• Local Windows, Bun 1.3.14: bun test plugins/sol-advisor/mcp/server.test.ts — 33 passed, 0 failed, 166 assertions
• Added direct coverage for an unexpected TrustedInstaller owner
• Added regression coverage for permissive config.json, managed-files.json, transaction.json, and .bak files
• Existing private Windows state, transaction rollback/recovery, junction defense, null DACL rejection, and minimal-environment stdio startup all pass
• git diff --check — passed
• bun run validate remains blocked on Windows by pre-existing CRLF-sensitive schema/frontmatter checks

Hosted CI

The fork workflow passed on commit 4ac08e6: validation, Ubuntu MCP, macOS MCP, and Windows MCP are all green. The upstream pull-request workflow still reports action_required until a maintainer authorizes contributor Actions.

• https://github.com/Zythenth/sol-advisor/actions/runs/31286305124

Resumo (pt-BR)

Esta PR corrige a validação de privacidade do PLUGIN_DATA no Windows usando proprietário e ACLs nativas, estende a mesma política aos arquivos de estado e backups, preserva o comportamento POSIX no Linux/macOS e adiciona regressões específicas para os caminhos apontados na revisão de segurança.

Fixes #20

[AlexMnrs]

Follow-up test on Codex Desktop / Windows

I tested the server.ts change from this PR locally with Sol Advisor 0.5.0, Bun 1.3.14, and a fresh Codex restart.

Result

The patch moves past the original Bun/POSIX 0666 failure, but setup still fails with:

PLUGIN_DATA must be private (ACL contains an unapproved allow ACE)

The actual inherited allow entries on the Codex-managed plugin-data directory are:

• Current user — Full Control
• SYSTEM — Full Control
• Administrators — Full Control
• CodexSandboxUsers — Read and Execute

There are no Everyone, Users, or Authenticated Users allow entries. CodexSandboxUsers is a Codex-managed local group containing the offline and online sandbox accounts.

Impact

After the restart, Codex discovers the MCP tools, but get_setup_status, get_preferences, and validate_configuration all fail at the ACL check. The logical profile exists, but adapter preview/install cannot proceed.

Question

Should CodexSandboxUsers be treated as an approved, narrowly scoped Codex principal, or should the supported installation change the plugin-data ACL? Removing this inherited entry blindly may interfere with Codex's sandbox, so I did not change it.

This looks like a compatibility gap in the Windows allowlist rather than a broad-permission problem. A Codex-specific regression test or documented ACL guidance would help.

[AlexMnrs]

Update: local workaround validated

Following up on my earlier test, I added a temporary local-only allowlist entry for the machine-local CodexSandboxUsers group on top of this PR's server.ts change. The group is resolved by its local name/SID; I did not change the Windows ACL.

With that narrow workaround, on Codex Desktop / Windows with Bun 1.3.14:

• get_setup_status: ready
• validate_configuration: valid, with no warnings
• User-scope adapter preview: three expected .toml files
• Installation: succeeded, and all three managed files matched the preview hashes

This confirms that the original Bun/POSIX 0666 issue is addressed by the PR. The remaining Codex-specific compatibility gap is that the stock inherited CodexSandboxUsers allow ACE is not included in the PR's Windows allowlist.

A supported accommodation, host-specific ACL guidance, or Codex regression test would make this work without a local patch. The workaround is temporary and may be overwritten by a plugin update.

[Zythenth]

Thanks for the detailed follow up. You were right. I checked the current Codex Windows sandbox implementation, and CodexSandboxUsers is a managed local group that intentionally receives read and execute access. Removing or changing that inherited ACL would be the wrong fix.

I updated the PR to resolve the exact SID of the local group and allow it only when every allow ACE stays within read and execute. Modify, write, delete, permission changes, ownership changes, and full control still fail closed. If the group does not exist or cannot be resolved, no exception is added. Everyone, Users, Authenticated Users, and unrelated principals remain rejected.

I also added Windows regression coverage for inherited RX on the config, manifest, transaction journal, and backups, including the status, validation, preview, and installation flow. M and F are explicitly covered as failures. README and CHANGELOG now document this boundary.

Locally, all 34 MCP tests pass and the repository validator passes. Thanks again. Your workaround identified the compatibility gap exactly.