← Back to PRs

#20823: fix(security): Windows ACL audit false-positive with localized SYSTEM (ru-RU)

by xinhuagu open 2026-02-19 10:10 View on GitHub →
size: M trusted-contributor
## Problem On non-English Windows (e.g. ru-RU), `openclaw security audit` reports false-positive CRITICAL findings for files that are correctly restricted to SYSTEM + current user only. The root cause: `icacls` outputs localized account names (`NT AUTHORITY\СИСТЕМА` in Russian) but the ACL classifier only matches English names (`NT AUTHORITY\SYSTEM`). The localized name fails string comparison and gets classified as untrusted. Fixes #20808 ## Solution Add a **SDDL fast path** in `inspectWindowsAcl()`: 1. Before parsing `icacls` text output, try `powershell -NoProfile -Command "(Get-Acl 'path').Sddl"` 2. SDDL uses **locale-independent abbreviations** (`SY`=SYSTEM, `BA`=Administrators, `CO`=Creator Owner) that are consistent across all Windows languages 3. Classify principals from SDDL abbreviations — immune to localization issues 4. If PowerShell is unavailable, fall back to the existing `icacls` text parsing Also adds **SID-based fallback** in `classifyPrincipal()` for cases where `icacls` outputs raw SIDs (e.g. `*S-1-5-18`) instead of display names. ## Changes - `src/security/windows-acl.ts`: Add `classifyFromSddl()`, well-known SID/SDDL constants, SID regex fallback in `classifyPrincipal()`, SDDL fast path in `inspectWindowsAcl()` - `src/security/windows-acl.test.ts`: 10 new tests (SDDL parsing, SID classification, ru-RU simulation, PowerShell fallback) ## Testing All 36 tests pass (26 existing + 10 new): ``` ✓ src/security/windows-acl.test.ts (36 tests) 7ms ``` <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR fixes a security audit false-positive on non-English Windows systems by implementing SDDL-based ACL classification. The implementation adds a PowerShell fast path that uses locale-independent SID abbreviations (`SY`, `BA`, etc.) instead of relying on localized display names from `icacls` output. Key improvements: - Resolves current user SID via `whoami /user` for precise trust classification - Falls back to `icacls` parsing when PowerShell unavailable - Properly escapes PowerShell path parameters (single quotes and backticks) - Tokenizes SDDL rights codes to avoid substring matching false positives Previous security concerns have been addressed: - User SIDs are only trusted when they match the current user's SID (obtained via `whoami`) - PowerShell command includes backtick escaping alongside single-quote escaping - SDDL rights parsing now tokenizes into 2-character codes Test coverage is comprehensive with 10 new tests covering SDDL parsing, SID classification, localization scenarios, and fallback paths. <h3>Confidence Score: 4/5</h3> - This PR is safe to merge with low risk - it addresses a legitimate security audit bug with comprehensive testing - The code properly addresses previous security review comments (SID matching, PowerShell escaping, rights parsing). Test coverage is comprehensive with 10 new tests. One minor concern exists with the DACL regex pattern being slightly complex, but it appears to work correctly for the SDDL format. The changes are well-isolated to Windows ACL security auditing and include proper fallback mechanisms. - No files require special attention <sub>Last reviewed commit: 810583f</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs