test(cli): skip unreadable-ledger run-ledger test under root - #9913
Merged
Conversation
The test simulates an unreadable ledger with chmod 000, but root bypasses permission bits, so the read succeeds and the append goes through, failing the test on every root run. Skip it there since the property is untestable without DAC enforcement. Fixes #9909 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
github-actions
Bot
added
the
review/self-reported
Collaborator
Collaborator
|
Re-run after the autofix round (win32 extension) and the merge from Template looks good ✓ Problem: observed, with a real reproduction. #9909 shows the exact assertion failure from a suite run in a root container against a clean tree ( Direction: aligned. Test-suite hygiene only — no product code, no behavioral surface. Keeps the suite green for root dev environments without weakening non-root coverage. Size: not applicable — no core paths. One test file under Approach: minimal. One test wrapped in the repo's established Risk: no high-risk path matches; no elevated risk signals. Moving on to code review. 🔍 中文说明autofix 轮次（win32 扩展）与 模板完整 ✓ 问题：已观测到，有真实复现。#9909 给出了在 root 容器中对干净树（ 方向：对齐。纯测试卫生——不改产品代码，不涉及行为面。让 root 开发环境的套件变绿，同时不削弱非 root 覆盖。 规模：不适用——未触及核心路径。 方案：最小化。单个测试包进仓库既有的 风险：未命中高风险路径；无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewMy independent take before reading the diff: a chmod-based unreadability test cannot pass as root, and the honest options are skip-under-root or mock the read failure — and mocking would gut a test that exists precisely to exercise real filesystem behavior ("the properties the threat model rests on"). The PR takes the skip path, which is what I'd have done, and it goes one step right: it uses the repo's conventional condition rather than a root-only one, because Verified against the diff at the reviewed commit:
Hygiene note (non-blocking): the title says "under root" while the diff now also skips on win32. The win32 half came from the autofix round addressing review feedback and follows the repo convention, so it belongs here — no action needed. Testing evidenceUnattended CI run — no PR code executed in this environment; the evidence below is this PR's own CI on the reviewed commit, fetched via the API. Test-only change with nothing user-visible: real-scenario (tmux) coverage is N/A.
The skipped checks are the repo's standing CI shape, not a gap introduced here: Sandboxed verification would settle this: 中文说明代码审查读 diff 前的独立想法：基于 chmod 的不可读测试在 root 下不可能通过，诚实的选项只有 root 下跳过，或 mock 读取失败——而 mock 会毁掉一个本为验证真实文件系统行为（"threat model 赖以成立的性质"）而存在的测试。PR 选了跳过，与我的方案一致，而且更进一步：用的是仓库约定的条件而不是仅限 root 的条件，因为 在被审提交的 diff 上核验：
卫生提示（不阻断）：标题写的是 "under root"，而 diff 现在还跳过 win32。win32 那一半来自 autofix 轮次处理评审反馈，遵循仓库约定，理应在此——无需处理。 测试证据无人值守 CI 运行——本环境未执行任何 PR 代码；以下证据为该 PR 自身在被审提交上的 CI，经 API 获取。纯测试改动且无用户可见面：真实场景（tmux）覆盖为 N/A。 （CI 结果表格见上方英文部分） 被跳过的检查是仓库既有的 CI 形态，并非本 PR 引入的缺口： 沙箱验证可以落定这一点： — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 5/5 — clean across every stage; exactly the minimal fix this problem needed. Stepping back: my independent proposal for #9909 was precisely what the PR does — skip the test where its fault premise cannot hold, because mocking the read failure would gut a threat-model test, and running it where it cannot be true is noise. The final shape matches the repo's established condition verbatim, the test body is untouched, the green ubuntu run shows non-root coverage is preserved (the test still executes there), and the earlier Windows feedback was absorbed correctly by the autofix round. Every line in the diff earns its place; the merge from If I had to maintain this in six months, the comment tells me exactly why the skip exists and where the convention comes from — that's the right outcome for a threat-model test whose premise is platform-dependent. CI is green where it runs for PRs; macOS/Windows/integration are merge-queue-only by design and will pick this up there. Approval pinned to the reviewed commit. Thanks for the clean reproduction in #9909 — that's what made this a fast yes. 中文说明置信度：5/5 —— 每个阶段都干净；正是这个问题所需的最小修复。 退一步看：我对 #9909 的独立方案正是本 PR 所做的——在故障前提不成立的地方跳过该测试，因为 mock 读取失败会毁掉一个 threat-model 测试，而在不可能为真的环境里跑它只是噪音。最终形态逐字采用仓库既有条件，测试体未动，绿色的 ubuntu 运行证明非 root 覆盖得以保留（该测试在那里仍会执行），早先的 Windows 反馈已由 autofix 轮次正确吸收。diff 中每一行都有存在理由；与 如果六个月后由我来维护，注释恰好告诉我跳过存在的原因与约定出处——对于一个前提依赖平台的 threat-model 测试，这就是正确的形态。 CI 在 PR 阶段运行的部分全绿；macOS/Windows/集成按设计只在 merge queue 运行，会在那里接住本 PR。已针对被审提交钉住批准。感谢 #9909 中干净的复现——这是它快速通过的原因。 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Author
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
Collaborator
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
previously approved these changes
Aug 24, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Test Plan (not a blocker): src/commands/review/lib/run-ledger.test.ts — no such file or directory.
中文说明
已审查——无阻断问题。 建议见行内评论。
Test Plan（非阻断）：src/commands/review/lib/run-ledger.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.0)
Comment on lines
+689
to
+690
| it.skipIf(process.getuid?.() === 0)( | ||
| 'refuses to append over a ledger it could not read', |
Collaborator
There was a problem hiding this comment.
[Suggestion] The skip condition here drops the process.platform === 'win32' || clause that the repo's documented convention uses for permission-bit fault-injection tests — cost-ledger.test.ts records the convention ("chmod 0o000 is a POSIX-only fault: on Windows it toggles the read-only attribute … and root bypasses the mode entirely — the repo convention for this shape"), and 11+ sibling sites (the recover-findings, worktree, transcripts, parse-args, repo-context, scratch-tree and manifest-repository-context tests) all guard with the win32-or-root form. On Windows chmodSync(path, 0o000) only toggles the read-only attribute, so the ledger stays readable and this test's premise — a present-but-unreadable ledger — cannot hold there either.
The effect is that on the merge queue's Windows lane the test executes (process.getuid is undefined there) but never witnesses the guard it is named for: chmod leaves the file readable, ledgerOccupant returns ok instead of refused, and the append proceeds; it then passes only because atomicWriteFileSync's rename onto the read-only target throws and appendRunSession's outer catch swallows the error — so on Windows the test pins "write failures are swallowed" rather than "refuse to append over an unreadable ledger". A future change to atomicWriteFileSync's read-only-target handling would flip this test's Windows result for reasons unrelated to the threat model, and the comment added here points only at root. The property itself stays covered everywhere by the seam-based probes in run-ledger.race.test.ts (they inject the fault through ledgerIoForTests with no platform precondition), so adding the win32 clause loses nothing.
Suggested change
| it.skipIf(process.getuid?.() === 0)( | |
| 'refuses to append over a ledger it could not read', | |
| it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)( | |
| 'refuses to append over a ledger it could not read', |
中文说明
这里的跳过条件缺少了仓库约定中用于权限位故障注入测试的 process.platform === 'win32' || 子句 —— cost-ledger.test.ts 记录了该约定（"chmod 0o000 是 POSIX 专有故障：在 Windows 上它只切换只读属性 …… 而 root 会完全绕过权限位 —— 这是仓库对此类场景的约定"），且 11+ 处同类测试（recover-findings、worktree、transcripts、parse-args、repo-context、scratch-tree、manifest-repository-context 等）都使用 win32-or-root 形式。在 Windows 上 chmodSync(path, 0o000) 只设置只读属性，文件依然可读，因此本测试的前提 —— "存在但不可读的 ledger" —— 在 Windows 上同样无法成立。
这导致合并队列的 Windows 任务中该测试会执行（那里 process.getuid 为 undefined），但实际上没有验证它命名要测的守卫：chmod 后文件仍可读，ledgerOccupant 返回 ok 而不是 refused，append 照常进行；测试之所以通过，只是因为 atomicWriteFileSync 对只读目标的 rename 抛错、而 appendRunSession 的外层 catch 吞掉了该错误 —— 也就是说在 Windows 上该测试实际钉住的是"写失败会被吞掉"，而不是"拒绝覆盖不可读的 ledger"。未来 atomicWriteFileSync 对只读目标处理的任何变化，都可能让该测试在 Windows 上的结果因与威胁模型无关的原因翻转，而这里新增的注释只提到了 root。该属性本身已由 run-ledger.race.test.ts 中基于 seam 的探针在所有环境覆盖（它们通过 ledgerIoForTests 注入故障，无平台前提），因此加上 win32 子句不会损失任何覆盖。
— qwen3.8-max via Qwen Code /review (v0.22.0)
Collaborator
Collaborator
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Round summary — PR #9913 (address-review)One actionable finding this round; implemented. No conflict ( Feedback points and dispositions[rc:3842991529] Inline Suggestion — skip condition drops the
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent 4": none — no checks were cut short; I simply had nothing further to verify (all 51 diff lines were read and cross-checked against HEAD)..
Test Plan (not a blocker): src/commands/review/lib/run-ledger.test.ts — no such file or directory; Tests 72 passed — this review observed 23733 passed; 72 passed — this review observed 23733 passed.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent 4"：none — no checks were cut short; I simply had nothing further to verify (all 51 diff lines were read and cross-checked against HEAD).。
Test Plan（非阻断）：src/commands/review/lib/run-ledger.test.ts — no such file or directory; Tests 72 passed — this review observed 23733 passed; 72 passed — this review observed 23733 passed。
— qwen3.8-max via Qwen Code /review (v0.22.0)
Collaborator
|
🤖 AutoFix could not reach the model — [API Error: 500 <500> InternalError.Algo: An error occurred in model serving, error message is: [Receive batching backend response failed!] (attempt 2/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 无法连接模型 —— [API Error: 500 <500> InternalError.Algo: An error occurred in model serving, error message is: [Receive batching backend response failed!]（第 2/100 次尝试）—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/32740992248 🧠 Handled by Qwen Code · model/模型 |
Collaborator
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Round summary — PR #9913 (address-review): no action neededThis round's feedback window (newer than the last evaluation, 2026-08-24T11:23:52Z) contains no actionable findings:
Dispositions for the review's disclosed notes (all informational, no defect claimed)
No code change was made this round and nothing was committed. Verification
中文说明轮次总结 — PR #9913（address-review）：无需任何操作本轮反馈窗口（晚于上次评估时间 2026-08-24T11:23:52Z）中没有任何可操作的发现：
该评审披露事项的逐条处置（均为信息性说明，未指认任何缺陷）
本轮未做任何代码改动，也未提交任何内容。 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
wenshao
enabled auto-merge
August 24, 2026 15:58
doudouOUC
approved these changes
Aug 24, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed at 234078d359.
- The change is test-only and follows the repository convention for permission-based fixtures by skipping on Windows and uid 0.
- The earlier Windows suggestion is fixed at the current head.
- The chmod integration test still executes and passes for a non-root POSIX user, while a uid 0 simulation skips exactly that test.
- The platform-independent fault-injection coverage in
run-ledger.race.test.tsstill exercises the production refusal path, so this does not remove the only coverage of the behavior.
Verification: the two focused files passed 76/76 tests; the uid 0 simulation passed 71 tests with the intended single skip; ESLint, Prettier, and git diff --check passed. Exact-head Linux CI and security checks are green.
No blocking or non-blocking findings.
Collaborator
Author
|
@qwen-code /triage |
Collaborator
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 14 passed · 0 failed · 14 total Flakiness gate: ✅ 1 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：14 通过 · 0 失败 · 14 总计 抖动门：✅ 1 changed test file(s) x 5 identical rounds, no divergence Verification reportPR 9913 Deep Verification —
|
| Cell | Test file | Environment | Scripted oracle | Result |
|---|---|---|---|---|
| C1 | head | plain, uid 1000 | exit 0; 72 total / 72 passed / 0 skipped; target executed | ✅ exactly that (also matches PR's claimed "72 passed") |
| C2 | base (extracted from HEAD^1) |
plain, uid 1000 | identical counts to C1 (A/A parity: non-root behavior unchanged) | ✅ 72/72, 0 skipped — parity |
| C3 | head | simulated root | exit 0; 71 passed; exactly 1 skipped and it is the target test | ✅ skipped = refuses to append over a ledger it could not read |
| C4 | base | simulated root + DAC override, -t target |
exit ≠ 0; target fails on its intended assertion | ✅ AssertionError: expected '[{"sessionId":"S1",…' to be '…' at base-test line 702 (the toBe(before) identity check); no EACCES/EPERM (not a harness artifact) |
| C5 | base | getuid stub only (no DAC sim), -t target |
exit 0; target passes | ✅ passed — isolates the factor |
| C6 | head | simulated win32, whole file | exit 0; 0 failed; target skipped via platform clause; no collateral | ✅ 70 passed, 2 skipped: target + the pre-existing win32-only symlink test |
Witnesses: 01-c1-head-plain-72-passed.png, 02-c2-base-plain-72-passed.png, 03-c3-head-simroot-target-skipped.png, 04-c4-base-simroot-dac-red.png, 05-scripted-assertions-14-of-14.png. Raw vitest JSON per cell: logs/c1.json … logs/c6.json; assertion run log: logs/assertions-run.txt.
Why the base control is valid without a base build
The diff is test-only (assertion S1: git diff --name-only HEAD^1..HEAD is exactly the one test file), so production run-ledger.ts is byte-identical between arms and the base cell reuses the head tree's production code by construction — the base test file was extracted via git show HEAD^1:… into the head tree and removed after the run. No workspace-link confound is possible because no workspace boundary is crossed.
Vacuity / pin checks
- S2: the target test body is byte-invariant base vs head (modulo comments/whitespace: 403 normalized chars on both sides) — the skip changes when the test runs, not what it pins.
- C4 is the non-vacuity proof: under the emulated root condition the base test goes red on the exact behavioral assertion it exists to guard (ledger content unchanged after a refused append), not on an import/setup error. The test is pinned by the property, and the skip is load-bearing for root runs.
- S5: exactly one new
skipIfpredicate and exactly onechmodSync(…, 0o000)fault in the file — the skip covers the file's only permission-bit fault. - S3: the comment's "repo convention for this shape" is accurate — 13 other test files already use the identical predicate (e.g.
packages/cli/src/dualOutput/DualOutputBridge.test.ts:374,repo-context.test.ts,manifest-repository-context.test.ts,cost-ledger.test.ts,scratch-tree.test.ts,transcripts.test.ts,worktree.test.ts,recover-findings.test.tsin the samereviewsubsystem, plus inline early-return variants inworkspace-file-system.test.ts/conversation-workspace.test.ts).
Targeted gates (affected workspace: packages/cli)
| Gate | Result | Liveness proof |
|---|---|---|
run-ledger.test.ts suite (6 cells above) |
green per table | C4 proves the suite can go red |
tsc --noEmit in packages/cli |
exit 0 | — |
eslint on the changed file |
exit 0 | planted const unusedLintProbe = 1 in a scratch file in the same dir → reported as @typescript-eslint/no-unused-vars error, then removed |
Corrections
None — the PR description's claims ("72 passed" non-root; "getuid stub → target skipped, exit 0") reproduced exactly (C1, C3).
Findings
F1 — informational, pre-existing (not introduced by this PR): two permission-fault tests elsewhere still lack the root guard. The sibling sweep enumerated every chmod(…, 0) / 0o000 permission-fault test in the repo. 13 files already carry this PR's exact predicate (or an equivalent early return), but two tests have no root guard:
packages/core/src/services/session-writer-lease.test.ts—'classifies an unreadable owned lock as unavailable':it.runIf(process.platform !== 'win32')with no root clause; chmods the lock0o000and expectsSessionWriterUnavailableError. Under root the read succeeds and the classification path changes.packages/cli/src/serve/multi-workspace-sessions.test.ts—'falls back to live-only listing when persisted probing fails': nogetuid/skipIf/win32guard anywhere in the file; chmods the chats dir0o000to force probe failure, a premise root voids.
Both files are byte-identical at base and head, and neither could be exercised here (no root in this container), so this is a static, unconfirmed observation — severity informational. If #9909 was a full-suite-red report from a root environment, these are the next candidates to surface after this PR lands. Not a merge condition for this PR.
Not covered
- Real root run — container is uid 1000 and
unshare -ris blocked by seccomp; root behavior was verified through the two-layer emulation whose fidelity the C4/C5 pair establishes (reproduces the wire shape of the root failure — read succeeds, append proceeds, assertion reds — via the mechanism's observable consequence, not via CAP_DAC_OVERRIDE itself). - Real Windows run — the platform clause is proven by
process.platformstubbing (C6) only; the underlying claim thatchmod 0o000keeps the file readable on Windows (Node maps the write bit to the read-only attribute) is documented Node behavior, not exercised here. - Per-commit attribution — depth-2 checkout:
git rev-list HEAD^1..HEAD^2yields only the merge commit while the metadata snapshot lists 3 commits, so the two functional commits (root skip; CI-bot win32 extension) were verified as the aggregateHEAD^1..HEADdiff. C3 covers the first commit's behavior, C6 the second's. - Full-suite run under root (would dynamically confirm/refute F1) — needs real root.
- Repo-wide gates beyond the affected workspace (repo-wide
npm run lint/npm run typecheck/ full suite) — the PR is one test file; workspace-level gates were deemed sufficient.
Methodology
Ran in the CI verify container (uid 1000, node v22, npm ci + npm run build pre-done at HEAD). All six vitest cells ran packages/cli's own vitest config (merged by vitest.cells.config.mts, which layers per-cell setupFiles stubs on top of the package's test-setup.ts, disables coverage, and keeps all aliases), cwd packages/cli, with the JSON reporter as the machine oracle (logs/c*.json) and the verbose reporter for terminal witnesses rasterised by scripts/verify-capture.mjs (evidence/*.png). The base test file was extracted with git show HEAD^1:… into the head tree for cells C2/C4/C5 and deleted afterwards; validity rests on the diff being test-only (S1). Root/win32 conditions were injected via vitest setup files (stub-root.mjs, stub-root-dac.mjs, stub-win32.mjs), never by touching the code under test. The 14 scripted assertions (S1–S5 static, C1–C6 behavioral) live in assert.mjs, which also wrote assertions.json; base-arm cells encode their expected red as a PASS, so fail counts only unexpected outcomes.
Flakiness gate log
rounds=5 files=1 skipped=0
file packages/cli/src/commands/review/lib/run-ledger.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/lib/run-ledger.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/commands/review/lib/run-ledger.test.ts: PPPPP
verdict: pass
summary: 1 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/commands/review/lib/run-ledger.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/lib/run-ledger.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/lib/run-ledger.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/lib/run-ledger.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/lib/run-ledger.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
approved these changes
Aug 24, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Collaborator
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round: no action takenI triaged all feedback newer than the last evaluation on PR #9913 and found nothing actionable:
For context, the PR's full diff remains the two test commits: the unreadable-ledger run-ledger test is skipped on Windows (where No code changes were made this round; the branch is unchanged at its current head ( 中文说明Autofix 审查轮次：未采取任何操作我对 PR #9913 上自上次评估以来的所有反馈进行了分类，结论是没有可处理的事项：
作为背景说明，本 PR 的完整差异仍然是那两个测试提交：将"账本不可读时拒绝追加"的 run-ledger 测试在 Windows 上（ 本轮没有修改任何代码；分支仍停留在当前头部提交（ Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Collaborator
|
Released in v0.22.2. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Skips the run-ledger test that asserts an unreadable ledger blocks appends when the test suite runs as root, and documents why in a comment.
Why it is needed
The test simulates an unreadable ledger file with
chmod 000, but root bypasses permission-bit checks (CAP_DAC_OVERRIDE), so the read inside the append path succeeds and the append goes through — failing the assertion on every root run. CI is unprivileged and never sees this, but any root dev environment (root container, sudo shell) gets a guaranteed red on the full suite. The property itself is untestable under root, so skipping is the correct behavior; non-root runs (including CI) keep full coverage.Reviewer Test Plan
How to verify
Run the run-ledger unit tests as a normal user and confirm the suite passes with the test still executing:
Expected: 72 passed, including "refuses to append over a ledger it could not read". In a root container, the same suite should pass with exactly that one test skipped instead of failing.
Evidence (Before & After)
N/A (test-only change). Local run as non-root:
With
process.getuidstubbed to return 0 (simulated root): the suite reports the target test skipped, exit code 0.Tested on
Environment (optional)
N/A — unit tests only.
Risk & Scope
getuidstub).Linked Issues
Fixes #9909
中文说明
本 PR 做了什么
当测试套件以 root 运行时，跳过那条断言“不可读 ledger 会阻止 append”的 run-ledger 测试，并在注释中说明原因。
为什么需要
该测试用
chmod 000模拟不可读的 ledger 文件，但 root 会绕过权限位检查（CAP_DAC_OVERRIDE），导致 append 路径里的读取成功、append 照常执行——在每次 root 运行中断言失败。CI 是非特权用户所以从未遇到，但任何 root 开发环境（root 容器、sudo shell）跑全量套件都会稳定变红。该属性在 root 下本身不可测，因此跳过是正确做法；非 root 运行（包括 CI）仍保留完整覆盖。审查者测试计划
如何验证
以普通用户运行 run-ledger 单元测试，确认套件通过且该测试仍在执行：
预期：72 个测试全部通过，包括 “refuses to append over a ledger it could not read”。在 root 容器中，同一套件应通过，且恰好该测试被跳过而不是失败。
证据（前后对比）
N/A（仅测试改动）。本地非 root 运行：
将
process.getuidstub 为返回 0（模拟 root）时：套件报告目标测试被跳过，退出码为 0。测试平台
环境（可选）
N/A —— 仅单元测试。
风险与范围
getuidstub）。关联 Issue
Fixes #9909