morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #5941] [MERGED] fix(ci): keep Windows ACL observation in SID form #7110

New issue

Closed

opened 2026-11-01 21:45:28 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:45:28 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5941
Author: @gensokyo-dev
Created: 10/21/2026
Status: ✅ Merged
Merged: 10/21/2026
Merged by: @gensokyo-dev

Base: main ← Head: fix/windows-acl-observer-20261021

📝 Commits (3)

bbe3ce5 fix(ci): keep Windows ACL observation in SID form
56e5bb4 fix(ci): override the ACL convenience member through type data
d97a1b6 test(ci): require the ACL observer guard to be active

📊 Changes

2 files changed (+40 additions, -1 deletions)

View changed files

📝 docs/testing.md (+8 -0)
📝 scripts/verify-windows-native.mjs (+32 -1)

📄 Description

Summary

The Windows native verifier twice timed out inside its 15s PowerShell private-runtime ACL probe with empty stdout/stderr: PR qualification and merged-main push, job 119206876119. Official full-source typecheck and curated tests passed. The earlier identical-SHA retry passed, but the repeated failure shows that did not establish a fix. Exact timeout phase/cause remains unknown.

The observer already reads SID rules for trustee checks, then separately uses .Access only to count them. PowerShell's implementation requests NTAccount rules for that convenience property. Reuse the single SID collection for both checks and poison the convenience property in this real observer so accidental reintroduction is observable. All facts still come from the actual private-runtime ACL; no simulated native control.

Add bounded stderr markers for script start, ACL read, owner SID, rule SIDs, readonly mutation and completion. These distinguish work that began from a failure before the script marker and identify the last completed stage. Preserve the original 15s deadline and all protected-DACL, current-owner, permitted-trustee, exact-rule-count and readonly assertions. No automatic retry, production API or provider authority change. Owning testing guidance documents the boundary. Maintainer scope decision: not applicable to this existing CI observer.

Validation

Base ce1f4b1bb (latest fetched main); head d97a1b675dfb98278ee11fb899938d69dc39ae66. Node 24.18.0 / npm 11.16.0; four-core affinity, two checker threads, serial Turbo.

JavaScript syntax, extracted full-observer PowerShell AST parsing, formatting and git diff --check passed on the final guard.
npm run check passed before the final three-line guard installation assertion: complete typecheck/build/lint/format/knip/metadata, resolved module boundaries and architecture guards. Elapsed 33.4s, CPU 61.9s, sampled process-family RSS 1630 MiB; this is one execution, not a performance comparison or hard memory cap.
CI routing and curated Windows lane tests: 278 passed, zero skipped. Existing ownership selects Windows native/package verification for this path.
Downloaded portable PowerShell 7.5.2 from the official release and matched its published SHA-256. A real inherited type-data experiment reproduced Add-Member's refusal, confirmed Update-TypeData's rejecting getter was invoked, preserved an independent .NET property and observed zero account-display getter calls. PowerShell can suppress getter errors: the old count becomes 1 and fails the real oracle's required count of 2. The real verifier also checks that its rejecting ScriptProperty was installed. This API/parser experiment ran on Linux, not a Windows ACL reproduction.
Real Windows lane passed on this head: official complete-source typecheck, curated tests, native private-runtime ACL/readonly controls, packaging, and Node24/22 native boundaries. All five ACL facts and the active ScriptProperty guard were verified. The script phase clock reached completion at1348ms; PowerShell process startup is outside that clock, but remains inside the original15s limit. No retry of this head.
Full verification checks merge preview 12a9df8c14082c3897bd4d8606fb4ea7cca79e98, including main 25e1a24a3ed3671a0148757555f18d16c29d936a; tree 8599f632dfeb83ff1735f8a07c9affbc4fdc8cf7. All verification jobs passed; the linked run records report aggregation and the final gate. The Apple lane passed in7m7s under its existing15m budget. Codex and Greptile reviewed exact head d97a1b675, with no actionable findings.

We have not established that account lookup caused either historical timeout; the source-confirmed redundant lookup is removed and phase evidence is retained for future failures. A separate older main qualification hit the Apple job's15m budget; this PR changes only the Windows observer and its documentation, and does not claim to fix Apple timing.

Author readiness

Human owner: @gensokyo-dev

Declarations entered under the user's explicit delegation; independent human review was not observed in this session.

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/5941 **Author:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Created:** 10/21/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `fix/windows-acl-observer-20261021` --- ### 📝 Commits (3) - [`bbe3ce5`](https://github.com/morluto/rea/commit/bbe3ce5da460c9b4e61b53afd918474ba3e56d4f) fix(ci): keep Windows ACL observation in SID form - [`56e5bb4`](https://github.com/morluto/rea/commit/56e5bb4483dfcaee218f68db7f06dbca0dd05488) fix(ci): override the ACL convenience member through type data - [`d97a1b6`](https://github.com/morluto/rea/commit/d97a1b675dfb98278ee11fb899938d69dc39ae66) test(ci): require the ACL observer guard to be active ### 📊 Changes **2 files changed** (+40 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `docs/testing.md` (+8 -0) 📝 `scripts/verify-windows-native.mjs` (+32 -1) </details> ### 📄 Description ## Summary The Windows native verifier twice timed out inside its 15s PowerShell private-runtime ACL probe with empty stdout/stderr: [PR qualification](https://github.com/morluto/rea/actions/runs/39584801109) and [merged-main push](https://github.com/morluto/rea/actions/runs/39594629876), job 119206876119. Official full-source typecheck and curated tests passed. The earlier identical-SHA retry passed, but the repeated failure shows that did not establish a fix. Exact timeout phase/cause remains unknown. The observer already reads SID rules for trustee checks, then separately uses `.Access` only to count them. [PowerShell's implementation](https://github.com/PowerShell/PowerShell/blob/v7.5.2/src/Microsoft.PowerShell.Security/security/AclCommands.cs#L262-L283) requests NTAccount rules for that convenience property. Reuse the single SID collection for both checks and poison the convenience property in this real observer so accidental reintroduction is observable. All facts still come from the actual private-runtime ACL; no simulated native control. Add bounded stderr markers for script start, ACL read, owner SID, rule SIDs, readonly mutation and completion. These distinguish work that began from a failure before the script marker and identify the last completed stage. Preserve the original 15s deadline and all protected-DACL, current-owner, permitted-trustee, exact-rule-count and readonly assertions. No automatic retry, production API or provider authority change. Owning testing guidance documents the boundary. Maintainer scope decision: not applicable to this existing CI observer. ## Validation Base `ce1f4b1bb` (latest fetched main); head `d97a1b675dfb98278ee11fb899938d69dc39ae66`. Node 24.18.0 / npm 11.16.0; four-core affinity, two checker threads, serial Turbo. - JavaScript syntax, extracted full-observer PowerShell AST parsing, formatting and `git diff --check` passed on the final guard. - `npm run check` passed before the final three-line guard installation assertion: complete typecheck/build/lint/format/knip/metadata, resolved module boundaries and architecture guards. Elapsed 33.4s, CPU 61.9s, sampled process-family RSS 1630 MiB; this is one execution, not a performance comparison or hard memory cap. - CI routing and curated Windows lane tests: 278 passed, zero skipped. Existing ownership selects Windows native/package verification for this path. - Downloaded portable PowerShell 7.5.2 from the official release and matched its published SHA-256. A real inherited type-data experiment reproduced Add-Member's refusal, confirmed Update-TypeData's rejecting getter was invoked, preserved an independent .NET property and observed zero account-display getter calls. PowerShell can suppress getter errors: the old count becomes 1 and fails the real oracle's required count of 2. The real verifier also checks that its rejecting ScriptProperty was installed. This API/parser experiment ran on Linux, **not a Windows ACL reproduction**. - Real Windows lane passed on this head: official complete-source typecheck, curated tests, native private-runtime ACL/readonly controls, packaging, and Node24/22 native boundaries. All five ACL facts and the active ScriptProperty guard were verified. The script phase clock reached completion at1348ms; PowerShell process startup is outside that clock, but remains inside the original15s limit. No retry of this head. - [Full verification](https://github.com/morluto/rea/actions/runs/39599008194) checks merge preview `12a9df8c14082c3897bd4d8606fb4ea7cca79e98`, including main `25e1a24a3ed3671a0148757555f18d16c29d936a`; tree `8599f632dfeb83ff1735f8a07c9affbc4fdc8cf7`. All verification jobs passed; the linked run records report aggregation and the final gate. The Apple lane passed in7m7s under its existing15m budget. Codex and Greptile reviewed exact head `d97a1b675`, with no actionable findings. We have not established that account lookup caused either historical timeout; the source-confirmed redundant lookup is removed and phase evidence is retained for future failures. A separate older main qualification hit the Apple job's15m budget; this PR changes only the Windows observer and its documentation, and does not claim to fix Apple timing. ## Author readiness Human owner: @gensokyo-dev Declarations entered under the user's explicit delegation; independent human review was not observed in this session. - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:45:28 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-03 13:39:25 +00:00

[PR #7124] [MERGED] fix(cli): report JSON Lines selection refusals as caller errors #7657

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#7110

No description provided.

Rows
Columns