diff --git a/BACKLOG.md b/BACKLOG.md index 4c5cf8a1..6787aa30 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -136,7 +136,8 @@ CC-001/CC-002 were consumed by PR #24 fix bundle inline, with no standalone entr | CC-691 | 🔵 active | **[pr-gate 在 reviewers 結束後的收尾階段太久:原生 Windows 回報約 6.5 分鐘,沒有階段標記看不出慢在哪一步(issue #754)]** 第 1 步:收尾各步驟加階段事件(`result_verify` 每次呼叫帶路由標籤與結束碼、`preflight_verify`、`assurance`、`qa_finalize`),下一次真實 gate 就能分段;之後依分段結果去掉重複驗證與重複指紋;計畫見下方 CC-691 一節。 | ops/gate | 2026-10-30 | feedback:2026-10-30 pr:#763 pr:#765 pr:#766 pr:#770 | P1 | hygiene | | CC-692 | 🔵 active | **[測試套件太慢:`test-pr-gate.sh` 約 330 個案例每個都跑完整 gate,本機 WSL 每分片 8 到 14 分鐘;判斷類案例應降到函式層,端到端只留接線]** 第 1 步(試點):`gate_reviewer_protocol_verify` 的 9 個純診斷案例改在新 suite `test-gate-reviewer-protocol.sh` 直接呼叫函式(含正向對照與突變驗證),`test-pr-gate.sh` 保留 2 個接線案例;計畫見下方 CC-692 一節。 | ops/test | 2026-11-02 | feedback:2026-11-02 pr:#764 | P2 | hygiene | | CC-693 | 🔵 active | **[`gate run` 的 context 階段在新 worktree 每次耗滿 90 秒上限且不保留進度(issue #780)]** 根因:`pmctl_context_workflow_refresh_bounded` 只用 `timeout 90` 從外面殺掉,`workflow-refresh` 沒傳預算給索引,索引在最後才 COMMIT,被殺就全丟。修法:`workflow-refresh` 新增 `--budget-ms`,wrapper 傳上限的三分之二,索引自己停下並保留進度,下一次接續(重用 #760 的機制)。 | ops/gate | 2026-11-05 | feedback:2026-11-05 pr:#784 | P2 | hygiene | -| CC-694 | 🔵 active | **[`PM_DISPATCH_STATE_ROOT` 用盘符写法时 `gate run` 先报告 detached 启动成功,监督进程不到 1 秒后才以 exit 2 失败(issue #779)]** `gate run` 在父程序、context 刷新與 operation record 之前檢查狀態根的寫法,拒絕時 exit 2、訊息指名變數(`PM_DISPATCH_STATE_ROOT` 或 `XDG_DATA_HOME`),不印 gate id;`docs/platform-support.md` 補一條。 | ops/gate | 2026-11-05 | feedback:2026-11-05 | P2 | hygiene | +| CC-694 | 🔵 active | **[`PM_DISPATCH_STATE_ROOT` 用盘符写法时 `gate run` 先报告 detached 启动成功,监督进程不到 1 秒后才以 exit 2 失败(issue #779)]** `gate run` 在父程序、context 刷新與 operation record 之前檢查狀態根的寫法,拒絕時 exit 2、訊息指名變數(`PM_DISPATCH_STATE_ROOT` 或 `XDG_DATA_HOME`),不印 gate id;`docs/platform-support.md` 補一條。 | ops/gate | 2026-11-05 | feedback:2026-11-05 pr:#785 | P2 | hygiene | +| CC-695 | 🔵 active | **[Windows 的 state root ACL 守衛:拒絕訊息沒有說是哪個主體(issue #778),測試的 `%TEMP%` 繼承 `CodexSandboxUsers` 讓 7 個 telemetry 案例失敗(issue #782)]** 守衛本身是對的,不放寬:訊息指名第一個違規主體與是否繼承,附上 `PM_DISPATCH_STATE_ROOT` 補救與不要隨手改父目錄 ACL 的提醒,`docs/platform-support.md` 補一條;`test-pmctl-context.sh` 用 `th_secure_state_root` 給 tmp_root 一個 owner-only ACL。 | ops/portability | 2026-11-05 | feedback:2026-11-05 | P2 | hygiene | --- @@ -2591,3 +2592,17 @@ symlink 或改名的 adapter 仍被拒絕。 **未做**:沒有量 issue 提到的「失敗前已花約 100 秒」(那是 [[CC-693]]);預設狀態根(`$HOME/.local/share/...`)在 Git Bash 本來就是 POSIX 寫法,沒有動。 **See**: issue #779、[[CC-615]]、[[CC-693]]。 + +--- + +## CC-695 — state root ACL 守衛的訊息與測試隔離 🔵 active + +**Problem**:issue #778 與 #782。原生 Windows 上,`%TEMP%` 與使用者目錄可能繼承 `CodexSandboxUsers` 的 Modify。守衛(`runtime/lib/state-writer.sh`,issue #592)據此拒絕是對的,但 (a) 訊息只寫 `a non-owner principal`,使用者看不出是誰、是不是繼承來的,也沒有補救指引(#778);(b) `test-pmctl-context.sh` 的 state root 建在 `%TEMP%` 底下,繼承那個授權,7 個 telemetry 案例被守衛擋下,真正的 telemetry 迴歸會被蓋住(#782)。 + +**修法**:PowerShell 檢查在 UNSAFE 時多輸出第一個違規主體與 `inherited`/`explicit`(`UNSAFE||`,bash 端仍接受單純的 `UNSAFE`),守衛與 `pmctl state status` 的原因帶出該主體;守衛另印一行補救(`PM_DISPATCH_STATE_ROOT` 指到只有自己能寫的目錄,繼承的授權可能屬於別的工具,不要未經確認就改父目錄)。`test-pmctl-context.sh` 呼叫既有的 `th_secure_state_root "$tmp_root"`(`test-pmctl-gate.sh` 已這麼做),不放寬守衛。 + +**驗證(原生 Windows,`%TEMP%` 含 `CodexSandboxUsers:(M)`)**:真實繼承授權(父目錄 `BUILTIN\Users:(M)`)的拒絕訊息為 `... non-owner principal: BUILTIN\Users, inherited`。#782 的 7 個案例(`context.queried` 2、`reuse-scan` 2、`pack` 3)在 main 上失敗,改後通過;`test-state-store.sh` 的 ACL 案例與四個真實 ACL 案例通過,兩個突變(原因不帶主體、PowerShell 不輸出主體)各讓對應案例失敗。 + +**未做**:`test-pmctl-context.sh` 其餘既有失敗(兩個 `no-arg ... external git repo` 案例以 exit 99 被隔離守衛拒絕、`marker round-trip`)與 ACL 無關,未動;整個套件超過 900 秒上限沒有完整跑;issue #778 建議 3(把已知 Codex 沙箱組列為可配置的允許名單)需要安全上的決定,沒做;守衛仍只指名第一個違規主體。 + +**See**: issue #778、#782、[[CC-694]];docs/platform-support.md 的 Windows 一節。 diff --git a/docs/architecture/script-variable-consumers.tsv b/docs/architecture/script-variable-consumers.tsv index 9f1eb5a3..31d6e8cf 100644 --- a/docs/architecture/script-variable-consumers.tsv +++ b/docs/architecture/script-variable-consumers.tsv @@ -374,6 +374,7 @@ PM_DISPATCH_STATE_ROOT PM_DISPATCH_STATE_ROOT runtime/lib/pmctl-context.sh produ PM_DISPATCH_STATE_ROOT PM_DISPATCH_STATE_ROOT runtime/lib/pmctl-gate.sh production PM_DISPATCH_STATE_ROOT PM_DISPATCH_STATE_ROOT runtime/lib/pmctl-worktree.sh production PM_DISPATCH_STATE_ROOT PM_DISPATCH_STATE_ROOT runtime/lib/state-paths.sh production +PM_DISPATCH_STATE_ROOT PM_DISPATCH_STATE_ROOT runtime/lib/state-writer.sh production PM_DISPATCH_TEST_COMMAND_IDENTITY PM_DISPATCH_TEST_COMMAND_IDENTITY runtime/bin/pr-gate.sh production PM_DISPATCH_TEST_FORCE_JQ_LF PM_DISPATCH_TEST_FORCE_JQ_LF tests/lib/test-harness.sh test PM_DISPATCH_TEST_FORCE_JQ_LF PM_DISPATCH_TEST_FORCE_JQ_LF tests/shell/test-jq-lf.sh test diff --git a/docs/platform-support.md b/docs/platform-support.md index 6fab35e8..aa34c15b 100644 --- a/docs/platform-support.md +++ b/docs/platform-support.md @@ -359,6 +359,22 @@ things to know: safety check, not a change to it. Do not use `PM_DISPATCH_ALLOW_UNSAFE_STATE_ROOT=1` to get past it; it turns the check off instead of fixing the directory. `pmctl` itself is unaffected: its state root is under your profile, not `%TEMP%`. +- **`pmctl gate run` can fail with `state-writer: unsafe state root rejected: ... (ACL grants write to a + non-owner principal: , inherited)` on the default state directory.** The check is correct: the + directory under your profile inherited a write grant for another account (for example `CodexSandboxUsers`, + which `[windows] sandbox = "elevated"` in `~/.codex/config.toml` creates), so another principal could write + your run state. The message names the first such principal and whether the grant is inherited or explicit + (issue #778). Do not edit the ACL of the default directory or its parent unless you know nothing else needs + that grant: the Codex sandbox may. Give `pmctl` a directory only you can write instead, and point + `PM_DISPATCH_STATE_ROOT` at it (write it as a POSIX path, `/c/Users/...`): + + ```bash + mkdir -p "$HOME/.pm-gate-state" + icacls "$(cygpath -w "$HOME/.pm-gate-state")" /inheritance:r /grant:r "$USERNAME:(OI)(CI)F" + export PM_DISPATCH_STATE_ROOT="$HOME/.pm-gate-state" + ``` + + `PM_DISPATCH_ALLOW_UNSAFE_STATE_ROOT=1` turns the check off; do not use it to get past this. - **`PM_DISPATCH_STATE_ROOT` (and `XDG_DATA_HOME`) must be written as a POSIX path, `/c/Users/...`, not `C:/Users/...`.** The gate's run directory is derived from the state root and its supervisor refuses a drive-letter run directory. `pmctl gate run` checks the state root before it starts anything diff --git a/runtime/lib/pmctl-state.sh b/runtime/lib/pmctl-state.sh index 51cd03db..a33e4f73 100644 --- a/runtime/lib/pmctl-state.sh +++ b/runtime/lib/pmctl-state.sh @@ -63,7 +63,7 @@ _pmctl_state_root_safety() { _sw_windows_store_root_allows_write "$store_root" || _win_acl_rc=$? if [[ "$_win_acl_rc" -eq 0 ]]; then _PMCTL_STATE_SAFE=0 - _PMCTL_STATE_SAFE_REASONS+=("ACL grants write to a non-owner principal") + _PMCTL_STATE_SAFE_REASONS+=("$(_sw_acl_unsafe_reason)") elif [[ "$_win_acl_rc" -eq 2 ]]; then _PMCTL_STATE_SAFE=0 _PMCTL_STATE_SAFE_REASONS+=("cannot verify Windows ACL (PowerShell/cygpath unavailable or Get-Acl failed)") diff --git a/runtime/lib/state-writer.sh b/runtime/lib/state-writer.sh index 33508dda..08821d1a 100644 --- a/runtime/lib/state-writer.sh +++ b/runtime/lib/state-writer.sh @@ -165,7 +165,10 @@ _sw_windows_store_root_allows_write() { } if ($allowedSids -contains $sid) { continue } if (([int]$ace.FileSystemRights -band $writeMask) -ne 0) { - Write-Output "UNSAFE" + # Issue #778: name the first offending principal and say whether the grant is inherited, so the + # rejection can tell the user what to look at. A caller that only compares with "UNSAFE" still works: + # the bash side accepts both forms. + Write-Output ("UNSAFE|" + $ace.IdentityReference.Value + "|" + $(if ($ace.IsInherited) { "inherited" } else { "explicit" })) exit 0 } } @@ -175,13 +178,23 @@ _sw_windows_store_root_allows_write() { } ' 2>/dev/null)" || return 2 out="${out//[$'\r\n']/}" + # The first offending principal as "|", for the rejection message; empty for a + # check that did not name one. + _SW_ACL_OFFENDER="" case "$out" in UNSAFE) return 0 ;; + UNSAFE\|*) _SW_ACL_OFFENDER="${out#UNSAFE|}"; return 0 ;; SAFE) return 1 ;; *) return 2 ;; esac } +# The rejection reason for an ACL that grants write to another principal, naming the first one when the check did +# (issue #778). One text for the writer and for `pmctl state status`. +_sw_acl_unsafe_reason() { + printf 'ACL grants write to a non-owner principal%s\n' "${_SW_ACL_OFFENDER:+: ${_SW_ACL_OFFENDER//|/, }}" +} + _sw_unsafe_store_root() { local root="$1" local reason="$2" @@ -240,7 +253,10 @@ _sw_ensure_store_root_safe() { local _win_acl_rc=0 _sw_windows_store_root_allows_write "$store_root" || _win_acl_rc=$? if [[ "$_win_acl_rc" -eq 0 ]]; then - _sw_unsafe_store_root "$(_sw_canonical_store_root "$store_root")" "ACL grants write to a non-owner principal" || return 1 + if ! _sw_unsafe_store_root "$(_sw_canonical_store_root "$store_root")" "$(_sw_acl_unsafe_reason)"; then + printf '%s\n' "state-writer: to use another location set PM_DISPATCH_STATE_ROOT to a directory only you can write (docs/platform-support.md); an inherited grant may belong to another tool, such as the Codex sandbox, so do not remove it from the parent directory without checking" >&2 + return 1 + fi elif [[ "$_win_acl_rc" -eq 2 ]]; then _sw_unsafe_store_root "$(_sw_canonical_store_root "$store_root")" "cannot verify Windows ACL (PowerShell/cygpath unavailable or Get-Acl failed)" || return 1 fi diff --git a/tests/shell/test-pmctl-context.sh b/tests/shell/test-pmctl-context.sh index 9a5e3790..d852e961 100755 --- a/tests/shell/test-pmctl-context.sh +++ b/tests/shell/test-pmctl-context.sh @@ -30,6 +30,11 @@ fi # reuse-scan emit — into the throwaway tmp_root, so the suite never pollutes or # contends on the developer's real install partition. Cases that need a specific # root override this locally. +# +# On native Windows the writer's guard refuses a state root whose ACL lets another principal write, and %TEMP% +# often inherits such a grant (CodexSandboxUsers, issue #782); one owner-only ACL on tmp_root is inherited by +# every state root below it. The guard is not loosened. +th_secure_state_root "$tmp_root" || { echo "FAIL: cannot give $tmp_root an owner-only ACL" >&2; exit 1; } export PM_DISPATCH_STATE_ROOT="$tmp_root/suite-state" # The developer's live repo context DB. Every context case must operate on an diff --git a/tests/shell/test-state-store.sh b/tests/shell/test-state-store.sh index cde2fe80..02c4e98e 100755 --- a/tests/shell/test-state-store.sh +++ b/tests/shell/test-state-store.sh @@ -457,22 +457,31 @@ case_state_store_init_windows_acl_unsafe_rejected() { # 3. Assert state_store_init fails loudly before writing VERSION. local name="state_store_init: Windows ACL write-grant rejected (POSIX mode bits are inert there)" should_run "$name" || return 0 - local store stubs rc=0 stderr_out - store="$tmp_root/root-windows-acl-unsafe" - mkdir -p "$store" + local store stubs rc stderr_out acl_out stubs="$tmp_root/windows-acl-unsafe-stubs" mkdir -p "$stubs" # shellcheck disable=SC2016 # $1/$2 expand when the generated cygpath stub runs. - printf '#!/usr/bin/env bash\n[[ "$1" == "-w" ]] || exit 1\nprintf "%%s\\n" "$2"\n' > "$stubs/cygpath" - printf '#!/usr/bin/env bash\nprintf "UNSAFE\\n"\n' > "$stubs/powershell.exe" - chmod +x "$stubs/cygpath" "$stubs/powershell.exe" - stderr_out="$(PATH="$stubs:$PATH" PM_DISPATCH_PLATFORM=windows \ - PM_DISPATCH_STATE_ROOT="$store" _SW_ALLOW_GLOBAL_PARTITION=1 state_store_init 2>&1 >/dev/null)" || rc=$? - if [[ "$rc" -ne 0 && "$stderr_out" == *"ACL grants write to a non-owner principal"* && ! -e "$store/VERSION" ]]; then - pass "$name" - else - fail "$name" "rc=$rc stderr=${stderr_out:-empty}" - fi + printf '#!/usr/bin/env bash\n[[ "$1" == "-w" ]] || exit 1\nprintf "%%s\n" "$2"\n' > "$stubs/cygpath" + chmod +x "$stubs/cygpath" + # Two answers from the PowerShell check: a bare UNSAFE (the form before issue #778) and one that names the + # principal and says the grant is inherited. Both are rejected before VERSION is written; the second also shows + # who to look at and how to point the store elsewhere. + for acl_out in 'UNSAFE' 'UNSAFE|HOST\CodexSandboxUsers|inherited'; do + store="$tmp_root/root-windows-acl-unsafe-${#acl_out}" + mkdir -p "$store" + printf '#!/usr/bin/env bash\ncat <<'"'"'ANSWER'"'"'\n%s\nANSWER\n' "$acl_out" > "$stubs/powershell.exe" + chmod +x "$stubs/powershell.exe" + rc=0 + stderr_out="$(PATH="$stubs:$PATH" PM_DISPATCH_PLATFORM=windows \ + PM_DISPATCH_STATE_ROOT="$store" _SW_ALLOW_GLOBAL_PARTITION=1 state_store_init 2>&1 >/dev/null)" || rc=$? + if [[ "$rc" -eq 0 || "$stderr_out" != *"ACL grants write to a non-owner principal"* || -e "$store/VERSION" ]]; then + fail "$name" "[$acl_out] rc=$rc stderr=${stderr_out:-empty}"; return 0 + fi + if [[ "$acl_out" == *"|"* ]] && [[ "$stderr_out" != *'principal: HOST\CodexSandboxUsers, inherited'* || "$stderr_out" != *"PM_DISPATCH_STATE_ROOT"* ]]; then + fail "$name" "the rejection should name the principal and the remedy: ${stderr_out:-empty}"; return 0 + fi + done + pass "$name" } case_state_store_init_windows_acl_safe_accepted() { @@ -677,11 +686,18 @@ _acl_writemask_case() { fi rc=0 _sw_windows_store_root_allows_write "$dir" || rc=$? - if [[ "$rc" -eq "$expected_rc" ]]; then - pass "$name" - else + if [[ "$rc" -ne "$expected_rc" ]]; then fail "$name" "rc=$rc (expected $expected_rc)" + return 0 fi + # Issue #778: an unsafe answer names the principal and says whether the grant is inherited. The grant here is + # explicit (the directory was stripped of inherited ACEs above); the name is the localized one, so only its + # presence is checked. + if [[ "$expected_rc" -eq 0 && "${_SW_ACL_OFFENDER:-}" != ?*"|explicit" ]]; then + fail "$name" "the offending principal was not named: [${_SW_ACL_OFFENDER:-}]" + return 0 + fi + pass "$name" } case_sw_windows_store_root_allows_write_readonly_is_safe() {