fix(lab): name the ACL failure code in the publisher key error

[ntdatt812]

Refs #2152.

What this changes

CL-10 public bundle and publisher > builds deterministic bundle ids and digests fails on the Windows leg with:

PublicEvidenceValidationError: public publisher key ACL hardening did not complete
      at requirePublisherKeyAcl (src\lab\public\signature.ts:73:11)

That message is the same string for every required-harden failure. You wrote in #2152 that telling the three causes apart needs error.cause.code and that you would rather leave the question open than aim a fix at the wrong one. This PR does not guess at the cause — it puts the code in the message, so the next dispatched run answers the question by itself.

After this change the same failure reads ... did not complete (ETIMEDOUT), (EACLIDENTITY) or (EICACLS).

Why the message and not just cause

cause is already attached — requirePublisherKeyAcl sets it — but the reporter prints message and the stack, which is why the trace in the issue has no cause line. Confirmed on Windows by driving the failure through the existing seams and reading both fields:

[EICACLS   (icacls refused)]
   message   = public publisher key ACL hardening did not complete
   cause.code= EICACLS
[ETIMEDOUT (budget)]
   message   = public publisher key ACL hardening did not complete
   cause.code= ETIMEDOUT

Identical messages, different causes. Nothing in the log distinguishes them.

What crosses the boundary

Only the errno-shaped code, and its shape is re-checked at the point of use rather than trusted:

return typeof code === "string" && /^[A-Z][A-Z0-9_]{1,30}$/.test(code) ? code : null;

sanitizedAclError already allowlists which codes it copies, so this is the second bound rather than the only one — it means a code added there later cannot widen what a public log prints without also passing this shape. A code in that shape has no separator, no lowercase and a bounded length, so it cannot carry the key pathname or the username component inside it. A cause with no such code keeps the previous message byte-for-byte, so no failure gains an empty ().

PublicEvidenceValidationError.code stays public_publisher_key_unsafe and cause is untouched, so nothing that branches on either changes.

One observation about the CI failure itself, offered as a lead, not a claim

Running bun test tests/lab-public-evidence.test.ts alone on Windows 11, unelevated, Bun 1.3.14: 14 pass, 0 fail. So it is not categorically broken on Windows, which narrows it to the runner.

Of your three candidates, one has a property that matches the shape you described — the first call failing and the very next one succeeding on a different temp home. A successful SID lookup is process-cached in windows-user-principal.ts; a failed one is not, so a cold or contended first lookup fails alone and the next call re-runs it warm. Driving exactly that through the seam reproduces your pattern:

[call #1 (lookup times out)] cause.code=EACLIDENTITY
[call #2 (lookup recovers)]  OK (no throw)
   principal lookups attempted = 2

That is consistent with EACLIDENTITY, not evidence for it — ETIMEDOUT would look similar if the memo key differed per stage. I have not reproduced the CI failure itself and am not proposing a fix for it here. The point of this PR is that the next platform-windows dispatch will print which one it was.

Verification

Windows 11, Bun 1.3.14, on this branch:

tests/lab-public-security-regressions.test.ts                  13 pass, 0 fail
8 lab-public suites (evidence, core-contract, lifecycle-hardening,
  surfaces, export-transaction, provenance-recovery, file-safety,
  security-regressions)                                        56 pass, 1 skip, 0 fail
tests/windows-secret-acl.test.ts + windows-user-principal.test.ts
                                                              177 pass, 0 fail
bun x tsc --noEmit -p tsconfig.json                             0 errors
bun run privacy:scan                                            Privacy scan passed

Measured on both sides: the same 8 suites on the unpatched tree are 52 pass, 1 skip, 0 fail across 53 tests, and 57 tests here. The delta is the 4 added cases and nothing else.

Mutation-checked. Reverting only the message to the old fixed string, keeping the tests:

(fail) a required publisher key ACL timeout names ETIMEDOUT in its message
(fail) a required publisher key icacls refusal names EICACLS in its message
(fail) a required publisher key SID lookup failure names EACLIDENTITY in its message
 10 pass | 3 fail

Exactly the three naming cases, while keeps the plain message stays green — it covers the other branch, so it should not move, and it does not.

Tests added

Four cases in lab-public-security-regressions.test.ts:

• ETIMEDOUT, EICACLS and EACLIDENTITY each named in the message. The EACLIDENTITY case is injected through setWindowsPrincipalRunnerForTests, one module further out than the icacls runner, so it also pins that the code survives the hand-off from windows-user-principal rather than only the icacls path.
• a cause with no errno-shaped code leaves the message byte-for-byte unchanged.

afterEach now also clears the principal runner, so an injected lookup cannot leak into a later test in the file.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

• All CI tests are green on my local testing.

• I pushed my PR to the latest dev commit.

• I resolved all correct Codex and CodeRabbit findings.

• My PR is ready for review.

Summary by CodeRabbit

• Bug Fixes
  • Improved ACL security error messages by including validated diagnostic codes when available.
  • Preserved a clear generic message when no recognized error code is provided.
  • Retained the original underlying error for troubleshooting.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1106cb11-2ca6-4daf-8426-fb136f7a1478

📥 Commits

Reviewing files that changed from the base of the PR and between 6f1229a and 6015c98.

📒 Files selected for processing (2)

• src/lab/public/signature.ts
• tests/lab-public-security-regressions.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

Changes

ACL failure diagnostics

Layer / File(s)	Summary
Validate and include ACL failure codes src/lab/public/signature.ts	The signature module extracts only validated uppercase errno-style codes. ACL hardening errors include the code when available and preserve the generic message otherwise.
Cover Windows ACL failure outcomes tests/lab-public-security-regressions.test.ts	Tests control the Windows principal runner and verify diagnostic codes for timeout, icacls refusal, and SID lookup failure. Tests also verify the unchanged message for uncoded errors.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: 🟡 Moderate · up to 6015c

The code change is localized, but the PR explicitly remains in draft with all required readiness checklist items unchecked, so it is not merge-ready until those checks are completed.

Suggested reviewers: wibias

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: naming the ACL failure code in publisher key errors.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ READY

• all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

• ✅ All CI tests are green on my local testing.
• ✅ I pushed my PR to the latest dev commit.
• ✅ I resolved all correct Codex and CodeRabbit findings.
• ✅ My PR is ready for review.

✅ 4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu

[lidge-jun]

리뷰 · 우선순위 32 / 80

#2152 Windows CI 조각임. 지금 dev HEAD 3ffffc802 requirePublisherKeyAcl (src/lab/public/signature.ts:55-75)가 실패 메시지를 한 줄로 고정함. public publisher key ACL hardening did not complete. cause는 붙임. 리포터가 message랑 스택만 찍어서 로그에 코드가 없음. ETIMEDOUT / EACLIDENTITY / EICACLS가 같은 문장임. 본문이 그거임.

이 PR이 aclFailureCode를 넣음. cause.code가 ^[A-Z][A-Z0-9_]{1,30}$일 때만 메시지에 (CODE)를 붙임. 아니면 예전 문장 그대로. PublicEvidenceValidationError.code는 public_publisher_key_unsafe 유지. pathname/username이 로그로 못 나가게 두 번 막음. sanitizedAclError allowlist 다음 두 번째 바운드임. 방향 맞음. ㅋㅋ 원인 추측은 안 함. 다음 platform-windows 디스패치가 답을 찍게 함.

테스트 네 장. ETIMEDOUT / EICACLS / EACLIDENTITY 각각 메시지에 코드. EACLIDENTITY는 setWindowsPrincipalRunnerForTests로 한 모듈 밖에서 주입. 핸드오프 생존을 잠금. 코드 없는 cause는 문장 byte-for-byte. 지금 dev afterEach (tests/lab-public-security-regressions.test.ts:21-28)는 icacls/platform만 지움. 이 PR이 principal runner도 지움. 뮤테이션 체크까지 함. 메시지 되돌리면 세 장이 깨짐.

고친 게 아님. 진단임. #2152 본체(윈도우 샤드 실패)는 열림. 점수 48. 이건 그 로그를 읽을 수 있게 만드는 선행임. Grok/Responses 레인 아님. #2188 사이드카, #2190 x_search, #2254/#2264 패스스루 전부 무관. types.ts/config.ts 안 만짐. 스플릿 안 씹힘. 2.28 블로커 아님. 프리뷰 배포 아님. 닫을 중복 아님.

draft 체크리스트 0/4. 비전공자가 윈도우 박스 없이 로그를 읽으려면 코드가 메시지에 있어야 함. 그 점은 맞음. 그래도 핫 레인이 아님. 원인은 다음 디스패치가 찍음.

해결방안: 체크리스트 채우고 draft 해제 후 dev 머지. 메시지에 errno 코드만 넣고 pathname 넣지 말 것. #2152 본체 픽스는 별 PR. 스플릿이 lab public signature를 옮기면 닫고 다시 짜라. 지금은 그 정도 아님.

이 댓글은 grok-bot이 작성했습니다

[ntdatt812]

Rebased onto dev@3c6a4524b and re-ran. Ticking the readiness boxes, with one thing stated rather than glossed.

The full local suite is 14029 tests, 1 failure, and that failure is not this change:

(fail) GET /api/client-config > an accepted override still resolves through the route
Expected: "/tmp/opencodex-pi-route-fixture/models.json"
Received: "\tmp\opencodex-pi-route-fixture\models.json"

A Windows path-separator assertion in tests/management-client-config-route.test.ts:322, in the pre-existing platform-windows class tracked by #2152. Measured on both sides rather than assumed — reverting this PR's two files to their parent revision and re-running that file gives the identical failure:

with the patch      13 pass, 1 fail
parent revision     13 pass, 1 fail   # aclFailureCode absent, verified

Nothing else in the suite fails. I will open that one separately; it is unrelated to the publisher key path.

Re-verified on the rebased head:

lab-public-security-regressions + lab-public-evidence
  + windows-secret-acl + windows-user-principal     204 pass, 0 fail

CodeRabbit reported no actionable comments, and CI is green on the pre-rebase head.

[Ingwannu]

Approved on exact head f882de0. The Grok maintainer review is correct: this is a bounded diagnostic change, not a speculative Windows ACL fix. Only an errno-shaped code can enter the public failure message; the stable error code and cause behavior remain unchanged. I verified 204 focused Lab/Windows tests and typecheck on the rebased head. This should merge to dev while #2152 remains open for the diagnostic result.