Skip to content

D062 Revision 2: enforce explicit operator SID pipe DACL - #105

Merged
LogicDuke merged 2 commits into
mainfrom
repair/d062-explicit-pipe-dacl
Sep 13, 2026
Merged

LogicDuke merged 2 commits into
mainfrom
repair/d062-explicit-pipe-dacl

Conversation

@LogicDuke

@LogicDuke LogicDuke commented Sep 13, 2026

Copy link
Copy Markdown
Owner

D062 Revision 2 — explicit operator-SID control pipe security

Implements the adopted D062 Revision-2 transport access boundary and DDR-D062-C.

Scope

  • Adds one in-process Windows Node-API named-pipe accept provider.
  • Node remains the actual named-pipe server process.
  • Every pipe instance is created with an explicit protected security descriptor.
  • Owner is the runtime process TokenUser SID.
  • DACL contains exactly one ALLOW ACE for that same SID.
  • Access mask is exactly 0x12019F.
  • No Everyone, Anonymous, Users, Authenticated Users, Administrators, SYSTEM, or foreign principal ACE is added.
  • No helper-owned server, broker, service, shell, PATH lookup, registry access, network authority, protocol parsing, dispatch authority, or native client opener is introduced.
  • Existing descriptor-v4, same-handle attestation, creation-time pinning, HMAC client→server, and Ed25519 server→client semantics are preserved.
  • No fallback to the default named-pipe security descriptor exists.

Native build policy

  • Initial baseline: Node 24.12.0 win-x64.
  • Minimum Node-API: 10.
  • Exact build-only dependency: node-api-headers@1.9.0.
  • Windows import library is generated locally from the package's official .def material using the existing MSVC toolchain.
  • No node-gyp, binding.gyp, cmake-js, node-addon-api, or foreign prebuilt addon.
  • The addon participates in existing source/artifact provenance verification and fails the control channel closed on absence, mismatch, or load failure.

Validation evidence

Exact committed candidate:

  • commit: de59a5a171ed783dd0eba192d71f37290f51edc0
  • tree: 47c9ba47fcaef149bda05f060a694798d9fa071c
  • parent/base: 726b2c8b4f1e6a42ff2e5b5192af35299e49cd11

Independent validation and NF-1 revalidation passed.

Latest deterministic results:

  • control-runtime: 56/56 PASS
  • control-integration: 17/17 PASS
  • pipe-acceptor: 26/26 PASS
  • full suite: 2326 passed / 0 failed / 1 skipped
  • typecheck: PASS
  • lint: PASS
  • build: PASS
  • git diff --check: PASS

Independent sacrificial Windows validation proved:

  • owner = exact runtime TokenUser SID
  • DACL PRESENT + PROTECTED
  • exactly one ALLOW ACE
  • mask exactly 0x12019F
  • broad principals absent
  • GetNamedPipeServerProcessId reports the Node runtime PID
  • same-SID ordinary client connectivity works
  • recreated pipe instances retain the explicit security descriptor
  • addon/provenance failure fails control closed with no default-descriptor fallback

Finding state

Primary D062 transport-access finding:

  • CURRENT / P2
  • occurrence 1
  • family breaker NOT TRIGGERED

NF-1 verification-wiring finding:

  • FIXED
  • occurrence 1
  • family breaker NOT TRIGGERED

Authority boundary

This PR is intentionally created as Draft.

No Ready transition, reviewer request, Codex review, CodeRabbit review, merge, deployment, runtime cutover, Scheduled Task mutation, production pipe/descriptor mutation, ACL/provisioning mutation, or branch/worktree retirement is authorized by creation of this PR.

Do not add labels.
Do not add assignees.
Do not request reviewers.
Do not add comments.
Do not invoke Codex.
Do not invoke CodeRabbit.

Summary by CodeRabbit

  • Security Enhancements

    • Control-channel connections now use an authenticated local pipe restricted to the current Windows user.
    • Insecure or unverifiable pipe providers are rejected, with control functionality failing closed when validation cannot be completed.
  • Reliability

    • Improved connection acceptance, recovery, socket handling, and shutdown coordination for control-channel communication.
    • Listen and startup failures now provide clearer error reporting.
  • Build & Validation

    • Added packaging and validation support for the Windows control-channel provider, including provenance checks and expanded automated coverage.

@coderabbitai

coderabbitai Bot commented Sep 13, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 8ea321e6-b424-4f58-a7c4-7e27f1f2cc6d

📥 Commits

Reviewing files that changed from the base of the PR and between 726b2c8 and 409666a.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (18)
  • package.json
  • src/control/control-channel.ts
  • src/control/control-runtime.ts
  • src/control/control-store.ts
  • tests/control/control-integration.test.ts
  • tests/control/control-runtime.test.ts
  • tests/control/launch-lifecycle.test.ts
  • tests/control/owner-helper.win.test.ts
  • tests/control/pipe-acceptor.test.ts
  • tests/control/support.ts
  • tools/control-owner/agentbridge-win-pipe-accept.c
  • tools/control-owner/build.mjs
  • tools/control-owner/ensure-helper.mjs
  • tools/control-owner/helper-pair.d.mts
  • tools/control-owner/helper-pair.mjs
  • tools/control-owner/msvc-toolchain.d.mts
  • tools/control-owner/msvc-toolchain.mjs
  • tools/control-owner/provenance-format.mjs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Changes

The control channel now uses a verified in-process Windows pipe acceptor with an explicit operator-SID DACL. The build provisions and validates a fourth native artifact. Accepted handles become Node sockets, and provider failures stop startup without fallback.

Control pipe acceptor

Layer / File(s) Summary
Acceptor artifact and build pipeline
package.json, tools/control-owner/*, tests/control/launch-lifecycle.test.ts
The project builds and provisions a Node-API .node acceptor with pinned headers, generated import libraries, and canonical provenance validation.
Native explicit-DACL acceptor
tools/control-owner/agentbridge-win-pipe-accept.c
The addon creates local overlapped pipes with a protected single-ACE DACL for the process token SID. It exposes createServer, accept, and close, and returns adopted file descriptors.
Verified production transport
src/control/control-store.ts, src/control/control-channel.ts, src/control/control-runtime.ts
The loader verifies provenance and addon bytes before loading. The control server adopts accepted descriptors, serves sockets, re-arms accepts, tracks shutdown, and fails closed on errors.
Transport wiring and validation
tests/control/*
Tests use a plain server for protocol-focused scenarios and cover provenance failures, startup behavior, artifact integrity, DACL properties, provider fallback prevention, and live Windows socket exchange.

Priority: ➖ Normal

Estimated code review effort: 5 (Critical) | ~100 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant ControlRuntime
  participant ControlChannelServer
  participant PipeAcceptorLoader
  participant NativePipeAcceptor
  participant ClientSocket
  ControlRuntime->>ControlChannelServer: listen()
  ControlChannelServer->>PipeAcceptorLoader: loadPipeAcceptor()
  PipeAcceptorLoader->>NativePipeAcceptor: verify bytes and load addon
  ControlChannelServer->>NativePipeAcceptor: createServer(pipePath, onAccept)
  ClientSocket->>NativePipeAcceptor: connect to named pipe
  NativePipeAcceptor-->>ControlChannelServer: accepted file descriptor
  ControlChannelServer->>ClientSocket: serve adopted net.Socket
Loading

Merge Risk: ⚪ Minimal · up to 40966

No actionable issue remains, and the explicit-DACL transport is covered by build, fail-closed, provenance, lifecycle, and live Windows validation.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 76.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 60 functions across 17 files. (1 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: enforcing an explicit operator-SID DACL for the control pipe.
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 76.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 60 functions across 17 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch repair/d062-explicit-pipe-dacl

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

A rabbit reads each line,
The patch grows clear beneath the moon,
Small changes hop in place,
Tests guard the garden path,
Reviews bloom before the dawn.

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

@LogicDuke
LogicDuke marked this pull request as ready for review September 13, 2026 22:28
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 13, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-13T22:31:59.223341Z 409666a Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@LogicDuke

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 13, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@LogicDuke
LogicDuke merged commit 1b831c4 into main Sep 13, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant