D62 — Authenticated post-start Autoflow control channel - #84
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DBynXdqPS9ZzfXshWHTyaD
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 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. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b00c91c52
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| // 3. Anchor ACL (read-only). | ||
| const icacls = await runProcess(icaclsPath(systemRoot), [anchorPath]); |
There was a problem hiding this comment.
Verify the control-directory owner
When the anchor is owned by another local account but its current DACL lists only the runtime operator and SYSTEM, this verification succeeds because the plain icacls <anchor> output is used only to inspect ACEs and the owner is never queried. A Windows object owner can change its DACL, so that account can subsequently grant itself access, read the descriptor token, and authenticate to the control channel; verify that the owner is also the runtime operator or SYSTEM before accepting the anchor.
Useful? React with 👍 / 👎.
| pipePath, | ||
| close: (): Promise<void> => | ||
| new Promise<void>((resolvePromise) => { | ||
| removeDescriptorFile(anchorPath, deps.descriptorDeps); |
There was a problem hiding this comment.
Delete only this server's descriptor
When two runtime instances overlap, the newer instance replaces the fixed descriptor file, but shutting down the older instance unconditionally removes that newer descriptor here. The newer pipe remains live yet the official CLI can no longer discover it; this also occurs during a normal start-before-stop restart. Associate cleanup with the descriptor this handle published and avoid removing the file if it now identifies another process/channel.
Useful? React with 👍 / 👎.
| if (/processed\s+\d+\s+files/i.test(line)) { | ||
| sawSummary = true; | ||
| continue; |
There was a problem hiding this comment.
Accept localized successful icacls output
On a non-English Windows installation, icacls localizes its success-summary text, so this English-only regex does not recognize the footer. The footer is then passed to parseAce, which returns null, causing every otherwise valid anchor to be rejected as ACL_UNREADABLE and disabling the control channel. Determine command success from the already-checked exit status or parse the output without requiring an English sentence.
Useful? React with 👍 / 👎.
Decision 062 implementation.
Authority:
Security boundary:
Validation before commit:
Exact validated candidate:
Deployment remains separately gated.
No ACL provisioning or runtime cutover is part of this PR.