feat(daemon): dual capability sockets + container topology (phase 6)#28
Merged
Conversation
TDD red→green. Resolves the SO_PEERCRED deferral via the contract's own amendment process (§2.2 amended, with forcing rationale). Forcing constraint: Node exposes no public SO_PEERCRED API. Resolution keeps the security property (capability kernel-attested, never client-asserted) via TWO capability sockets — the parent DESIGN already sanctioned 'a different socket / capability', so this is principled, not ad-hoc: - operator socket 0600 (brain uid only) → operator capability - agent socket 0666 (bind-mounted into the container) → agent, capped (access != privilege: that socket only ever grants agent capability) - server.ts: optional socketMode + shared dispatcher (DRY reuse). - dual-socket.ts: DualSocketDaemon — one Dispatcher, two MykbDaemon listeners; 4 tests (capability-by-socket, 0600/0666 modes, shared brain, agent verify_entry → TRUST_DENIED over the wire). - main.ts: dual-socket production mode when MYKB_OPERATOR_SOCKET + MYKB_AGENT_SOCKET set; single-socket operator dev default retained. - deploy/mykbd.service: reference systemd unit (hardened, templated). - docs/v2-container-topology.md: RO-brain-mount + agent-socket-bind-mount topology + the viloforge-platform argo integration seam (the cross-repo manifest change is a deployment action, out of mykb-repo scope per the container-only scope + vafi-config-in-viloforge-platform fact). - contract §7.1 SO_PEERCRED row flipped ⏭→✅. Full suite 650/650 across 2 runs; tsc + lint clean.
📝 WalkthroughWalkthroughThis PR implements dual-socket daemon architecture to enforce capability-based access control. A privileged daemon listens on two Unix sockets—operator (0600, full access) and agent (0666, capped operations)—sharing a single brain data store. The system is deployed via systemd, wired at runtime via environment variables, and validated by integration tests exercising socket permissions and capability separation. ChangesDual-socket daemon capability separation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/mykbd.service`:
- Around line 8-10: Update the top install comments to reflect that this is a
templated unit (it uses instance specifiers %i and %h) by instructing users to
install the file as mykbd@.service and enable/start it with the instance name
(e.g., systemctl daemon-reload && systemctl enable --now mykbd@<brainuser>), and
make sure the comment references the templated unit name mykbd@.service to match
the procedure in docs/v2-container-topology.md.
In `@docs/v2-container-topology.md`:
- Around line 17-29: The fenced code block in docs/v2-container-topology.md (the
block starting with ``` and containing the "HOST (trusted)
│ Pi CONTAINER (untrusted LLM)" diagram) needs a language hint to satisfy
markdownlint MD040; change the opening fence from ``` to ```text so the block
becomes a labeled code fence (e.g., replace the opening ``` with ```text) and
leave the closing ``` unchanged.
In `@docs/v2-protocol-contract-DESIGN.md`:
- Around line 38-45: Update the DESIGN doc so the dual-socket approach is the
canonical auth model: mark all prior SO_PEERCRED text as
historical/non-normative (or remove it), replace remaining normative references
to SO_PEERCRED with the dual-socket language (operator socket / agent socket)
and ensure references to the capability resolver point to
DaemonOptions.resolveCapability as the injection seam; keep the single-socket
dev-mode note as explicit dev/test-only behavior and retain the original
SO_PEERCRED description only in a clearly labeled "History" or "Non-normative"
subsection for design provenance.
In `@src/daemon/dual-socket.ts`:
- Around line 55-57: The current listen() calls this.operator.listen() and
this.agent.listen() concurrently and can leave one daemon running if the other
fails; change listen() to perform both starts atomically by using
Promise.allSettled (or sequential try/catch) to detect partial success, and if
one listen resolved while the other rejected call the corresponding teardown on
the started peer (e.g., this.operator.close() or this.operator.stop()/dispose()
and/or this.agent.close()/stop()/dispose()—choose the actual cleanup method
implemented) to roll back, then rethrow an aggregated error so startup either
fully succeeds or is fully unwound.
In `@src/daemon/main.ts`:
- Around line 37-53: The current startup logic silently falls back to
single-socket mode when only one of the dual-socket env vars
(MYKB_OPERATOR_SOCKET or MYKB_AGENT_SOCKET) is set; update the init in main.ts
to detect the XOR case (op defined XOR ag defined) and fail fast by
logging/throwing a clear error and exiting instead of creating a MykbDaemon;
keep the existing DualSocketDaemon creation when both op and ag are present and
the MykbDaemon/defaultSocketPath path when neither is present, but explicitly
abort when exactly one of op or ag is provided to surface misconfiguration.
In `@src/daemon/server.ts`:
- Around line 61-65: The chmodSync call inside the server.listen callback can
throw and escape the Promise rejection path; wrap
fs.chmodSync(this.opts.socketPath, this.opts.socketMode ?? 0o600) in a try/catch
inside the listen callback (or where the code currently calls fs.chmodSync), and
on error call reject(err) (and still call server.off('error', reject) before
rejecting) so the Promise is always settled via reject instead of throwing;
ensure the success path still calls server.off('error', reject) and resolves as
before.
In `@tests/daemon/dual-socket.test.ts`:
- Around line 96-104: The test currently asserts only that agentSocketPath has
some group/other RW bits; change the assertion to require the exact mode 0o666
per the contract. In the test named "the operator socket is created mode 0600;
the agent socket is group/other-connectable" replace the loose check on
fs.statSync(agentSocketPath).mode with an exact mask/assertion that the full
permission bits (e.g., mode & 0o777) equal 0o666 so the agent socket's
permissions are enforced precisely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 96c0d643-4d8e-4849-a5f5-c065c688a75f
📒 Files selected for processing (7)
deploy/mykbd.servicedocs/v2-container-topology.mddocs/v2-protocol-contract-DESIGN.mdsrc/daemon/dual-socket.tssrc/daemon/main.tssrc/daemon/server.tstests/daemon/dual-socket.test.ts
Comment on lines
+8
to
+10
| # Install: copy to /etc/systemd/system/mykbd.service, adjust the | ||
| # placeholders, then: systemctl daemon-reload && systemctl enable --now mykbd | ||
| # |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Service files present =="
fd -a --glob 'mykbd*.service' deploy
echo
echo "== Instance specifiers in unit =="
rg -n '%i|%I|%h' deploy/mykbd.service
echo
echo "== Install instructions referencing unit naming =="
rg -n 'Install:|systemctl enable --now|mykbd@|mykbd\.service' deploy/mykbd.service docs/v2-container-topology.mdRepository: vilosource/mykb
Length of output: 833
Update install instructions to use templated unit name, matching documented procedure.
The service file uses instance specifiers (%i on line 23, %h on lines 25, 29, 44) that require a templated unit, but the install comments on lines 8–9 reference the non-template name mykbd.service. This conflicts with the documented correct procedure in docs/v2-container-topology.md (lines 59–61), which already instructs installing as mykbd@.service and enabling with mykbd@<brainuser>. Inconsistent install instructions risk incorrect deployment.
Suggested fix
-# Install: copy to /etc/systemd/system/mykbd.service, adjust the
-# placeholders, then: systemctl daemon-reload && systemctl enable --now mykbd
+# Install: copy to /etc/systemd/system/mykbd@.service, adjust the
+# placeholders, then:
+# systemctl daemon-reload
+# systemctl enable --now mykbd@<brainuser>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Install: copy to /etc/systemd/system/mykbd.service, adjust the | |
| # placeholders, then: systemctl daemon-reload && systemctl enable --now mykbd | |
| # | |
| # Install: copy to /etc/systemd/system/mykbd@.service, adjust the | |
| # placeholders, then: | |
| # systemctl daemon-reload | |
| # systemctl enable --now mykbd@<brainuser> | |
| # |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deploy/mykbd.service` around lines 8 - 10, Update the top install comments to
reflect that this is a templated unit (it uses instance specifiers %i and %h) by
instructing users to install the file as mykbd@.service and enable/start it with
the instance name (e.g., systemctl daemon-reload && systemctl enable --now
mykbd@<brainuser>), and make sure the comment references the templated unit name
mykbd@.service to match the procedure in docs/v2-container-topology.md.
Comment on lines
+17
to
+29
| ``` | ||
| HOST (trusted) │ Pi CONTAINER (untrusted LLM) | ||
| │ | ||
| ~/.mykb/ ── owned by brain uid ────────┼── mounted READ-ONLY | ||
| │ │ bash > facts.jsonl → EROFS ✗ | ||
| │ (only writer) │ write tool → EROFS ✗ | ||
| ▼ │ python -c 'open(w)' → EROFS ✗ | ||
| mykbd (systemd, brain uid) │ MykbStore.append… ──┐ | ||
| ├─ operator socket 0600 ───────────┼── (NOT mounted in) │ | ||
| │ host operator / kb CLI only │ │ | ||
| └─ agent socket 0666 ───────────┼── bind-mounted in ◄───────┘ | ||
| capability capped at 'agent' │ the ONLY writable path | ||
| ``` |
There was a problem hiding this comment.
Add a language hint to the fenced block to satisfy markdownlint MD040.
Suggested fix
-```
+```text
HOST (trusted) │ Pi CONTAINER (untrusted LLM)
...
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 17-17: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/v2-container-topology.md` around lines 17 - 29, The fenced code block in
docs/v2-container-topology.md (the block starting with ``` and containing the
"HOST (trusted) │ Pi CONTAINER (untrusted LLM)"
diagram) needs a language hint to satisfy markdownlint MD040; change the opening
fence from ``` to ```text so the block becomes a labeled code fence (e.g.,
replace the opening ``` with ```text) and leave the closing ``` unchanged.
Comment on lines
+38
to
+45
| ### 2.2 Auth model → **OS-enforced capability; dual capability sockets (amended Phase 6)** | ||
|
|
||
| > **AMENDED 2026-05-16 (Phase 6), per this doc's own change process** ("deviations require a revision to this doc, not an ad-hoc code decision", §1). **Forcing constraint:** Node.js exposes **no public `SO_PEERCRED` API** — the connecting peer's uid is not readable from a `net.Socket` without a native addon, which is against the project's deps-minimal ethos. **Resolution:** keep the *security property* (capability is kernel-attested, never client-asserted) but realize it through **two capability sockets** instead of one socket + `SO_PEERCRED`: | ||
| > | ||
| > - **operator socket** — created mode `0600`, owned by the brain uid. The kernel permits only the brain-owning uid to `connect()`. A connection here ⇒ `operator` capability. | ||
| > - **agent socket** — the one bind-mounted into the Pi container (Phase 6 topology). A connection here ⇒ `agent` capability; asserted `trust` capped at `agent` (§3.1a). | ||
| > | ||
| > This is **not an ad-hoc deviation**: the parent DESIGN §Operator-vs-extension explicitly sanctioned "operator commands gated by a different token / **socket** / capability", and §2.2's own text below already said splitting is protocol-compatible. The kernel still attests identity (filesystem-perms `connect()` enforcement instead of `SO_PEERCRED` readout); the daemon still never trusts a client-asserted capability. The capability resolver remains the injected Strategy seam (`DaemonOptions.resolveCapability`) decided in Phase 2 — the dual-socket resolver is one implementation of it; a future native-`SO_PEERCRED` single-socket resolver could replace it with no contract change. Single-socket dev-mode (default → `operator`) is retained for the host operator / tests (parent DESIGN §Dev-mode). The original single-socket+`SO_PEERCRED` text is kept below for design history. |
There was a problem hiding this comment.
Unify the contract to one auth model to avoid contradictory normative guidance.
This amendment is written as the active contract, but other normative sections still prescribe SO_PEERCRED behavior. Please mark the old SO_PEERCRED passages as historical/non-normative (or remove them) and update remaining references to dual-socket capability language to keep the contract self-consistent.
Also applies to: 301-301
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/v2-protocol-contract-DESIGN.md` around lines 38 - 45, Update the DESIGN
doc so the dual-socket approach is the canonical auth model: mark all prior
SO_PEERCRED text as historical/non-normative (or remove it), replace remaining
normative references to SO_PEERCRED with the dual-socket language (operator
socket / agent socket) and ensure references to the capability resolver point to
DaemonOptions.resolveCapability as the injection seam; keep the single-socket
dev-mode note as explicit dev/test-only behavior and retain the original
SO_PEERCRED description only in a clearly labeled "History" or "Non-normative"
subsection for design provenance.
Comment on lines
+55
to
+57
| async listen(): Promise<void> { | ||
| await Promise.all([this.operator.listen(), this.agent.listen()]); | ||
| } |
There was a problem hiding this comment.
Make dual-socket startup atomic (rollback on partial listen failure).
If one listen() succeeds and the other fails, one daemon can remain active after startup failure. Add rollback so startup either fully succeeds or fully unwinds.
Suggested fix
async listen(): Promise<void> {
- await Promise.all([this.operator.listen(), this.agent.listen()]);
+ await this.operator.listen();
+ try {
+ await this.agent.listen();
+ } catch (e) {
+ await this.operator.close().catch(() => {});
+ throw e;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async listen(): Promise<void> { | |
| await Promise.all([this.operator.listen(), this.agent.listen()]); | |
| } | |
| async listen(): Promise<void> { | |
| await this.operator.listen(); | |
| try { | |
| await this.agent.listen(); | |
| } catch (e) { | |
| await this.operator.close().catch(() => {}); | |
| throw e; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/daemon/dual-socket.ts` around lines 55 - 57, The current listen() calls
this.operator.listen() and this.agent.listen() concurrently and can leave one
daemon running if the other fails; change listen() to perform both starts
atomically by using Promise.allSettled (or sequential try/catch) to detect
partial success, and if one listen resolved while the other rejected call the
corresponding teardown on the started peer (e.g., this.operator.close() or
this.operator.stop()/dispose() and/or this.agent.close()/stop()/dispose()—choose
the actual cleanup method implemented) to roll back, then rethrow an aggregated
error so startup either fully succeeds or is fully unwound.
Comment on lines
+37
to
+53
| const op = process.env.MYKB_OPERATOR_SOCKET; | ||
| const ag = process.env.MYKB_AGENT_SOCKET; | ||
|
|
||
| let daemon: Runnable; | ||
| let banner: string; | ||
| if (op && ag) { | ||
| daemon = new DualSocketDaemon({ | ||
| brainPath, | ||
| operatorSocketPath: op, | ||
| agentSocketPath: ag, | ||
| }); | ||
| banner = `mykbd (dual-socket) operator=${op} agent=${ag} (brain: ${brainPath})`; | ||
| } else { | ||
| const socketPath = defaultSocketPath(brainPath); | ||
| daemon = new MykbDaemon({ brainPath, socketPath }); | ||
| banner = `mykbd (single-socket, operator) ${socketPath} (brain: ${brainPath})`; | ||
| } |
There was a problem hiding this comment.
Fail fast when only one dual-socket env var is set.
Current logic silently drops to single-socket mode on partial config (MYKB_OPERATOR_SOCKET xor MYKB_AGENT_SOCKET), which can mask deployment errors.
Suggested fix
const op = process.env.MYKB_OPERATOR_SOCKET;
const ag = process.env.MYKB_AGENT_SOCKET;
+ if ((op && !ag) || (!op && ag)) {
+ throw new Error(
+ 'Set both MYKB_OPERATOR_SOCKET and MYKB_AGENT_SOCKET for dual-socket mode',
+ );
+ }
let daemon: Runnable;
let banner: string;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const op = process.env.MYKB_OPERATOR_SOCKET; | |
| const ag = process.env.MYKB_AGENT_SOCKET; | |
| let daemon: Runnable; | |
| let banner: string; | |
| if (op && ag) { | |
| daemon = new DualSocketDaemon({ | |
| brainPath, | |
| operatorSocketPath: op, | |
| agentSocketPath: ag, | |
| }); | |
| banner = `mykbd (dual-socket) operator=${op} agent=${ag} (brain: ${brainPath})`; | |
| } else { | |
| const socketPath = defaultSocketPath(brainPath); | |
| daemon = new MykbDaemon({ brainPath, socketPath }); | |
| banner = `mykbd (single-socket, operator) ${socketPath} (brain: ${brainPath})`; | |
| } | |
| const op = process.env.MYKB_OPERATOR_SOCKET; | |
| const ag = process.env.MYKB_AGENT_SOCKET; | |
| if ((op && !ag) || (!op && ag)) { | |
| throw new Error( | |
| 'Set both MYKB_OPERATOR_SOCKET and MYKB_AGENT_SOCKET for dual-socket mode', | |
| ); | |
| } | |
| let daemon: Runnable; | |
| let banner: string; | |
| if (op && ag) { | |
| daemon = new DualSocketDaemon({ | |
| brainPath, | |
| operatorSocketPath: op, | |
| agentSocketPath: ag, | |
| }); | |
| banner = `mykbd (dual-socket) operator=${op} agent=${ag} (brain: ${brainPath})`; | |
| } else { | |
| const socketPath = defaultSocketPath(brainPath); | |
| daemon = new MykbDaemon({ brainPath, socketPath }); | |
| banner = `mykbd (single-socket, operator) ${socketPath} (brain: ${brainPath})`; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/daemon/main.ts` around lines 37 - 53, The current startup logic silently
falls back to single-socket mode when only one of the dual-socket env vars
(MYKB_OPERATOR_SOCKET or MYKB_AGENT_SOCKET) is set; update the init in main.ts
to detect the XOR case (op defined XOR ag defined) and fail fast by
logging/throwing a clear error and exiting instead of creating a MykbDaemon;
keep the existing DualSocketDaemon creation when both op and ag are present and
the MykbDaemon/defaultSocketPath path when neither is present, but explicitly
abort when exactly one of op or ag is provided to surface misconfiguration.
Comment on lines
+61
to
65
| // Default 0600: only the brain-owning uid may connect (the | ||
| // operator-capability kernel gate, §2.2 amended). The agent | ||
| // socket overrides this to be container-connectable. | ||
| fs.chmodSync(this.opts.socketPath, this.opts.socketMode ?? 0o600); | ||
| server.off('error', reject); |
There was a problem hiding this comment.
Handle chmodSync failures in listen() instead of throwing from callback.
A permission/path error here can throw out of the server.listen callback and bypass your promise rejection flow.
Suggested fix
server.listen(this.opts.socketPath, () => {
- // Default 0600: only the brain-owning uid may connect (the
- // operator-capability kernel gate, §2.2 amended). The agent
- // socket overrides this to be container-connectable.
- fs.chmodSync(this.opts.socketPath, this.opts.socketMode ?? 0o600);
+ // Default 0600: only the brain-owning uid may connect (the
+ // operator-capability kernel gate, §2.2 amended). The agent
+ // socket overrides this to be container-connectable.
+ try {
+ fs.chmodSync(this.opts.socketPath, this.opts.socketMode ?? 0o600);
+ } catch (e) {
+ server.off('error', reject);
+ server.close(() => reject(e));
+ return;
+ }
server.off('error', reject);
resolve();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Default 0600: only the brain-owning uid may connect (the | |
| // operator-capability kernel gate, §2.2 amended). The agent | |
| // socket overrides this to be container-connectable. | |
| fs.chmodSync(this.opts.socketPath, this.opts.socketMode ?? 0o600); | |
| server.off('error', reject); | |
| // Default 0600: only the brain-owning uid may connect (the | |
| // operator-capability kernel gate, §2.2 amended). The agent | |
| // socket overrides this to be container-connectable. | |
| try { | |
| fs.chmodSync(this.opts.socketPath, this.opts.socketMode ?? 0o600); | |
| } catch (e) { | |
| server.off('error', reject); | |
| server.close(() => reject(e)); | |
| return; | |
| } | |
| server.off('error', reject); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/daemon/server.ts` around lines 61 - 65, The chmodSync call inside the
server.listen callback can throw and escape the Promise rejection path; wrap
fs.chmodSync(this.opts.socketPath, this.opts.socketMode ?? 0o600) in a try/catch
inside the listen callback (or where the code currently calls fs.chmodSync), and
on error call reject(err) (and still call server.off('error', reject) before
rejecting) so the Promise is always settled via reject instead of throwing;
ensure the success path still calls server.off('error', reject) and resolves as
before.
Comment on lines
+96
to
+104
| it('the operator socket is created mode 0600; the agent socket is group/other-connectable', async () => { | ||
| await withTempBrain(async (bp) => { | ||
| initBrain(bp); | ||
| const { operatorSocketPath, agentSocketPath } = await start(bp); | ||
| expect(fs.statSync(operatorSocketPath).mode & 0o777).toBe(0o600); | ||
| // agent socket must be connectable by the container (different uid): | ||
| // at least group/other read+write. | ||
| expect(fs.statSync(agentSocketPath).mode & 0o066).not.toBe(0); | ||
| }); |
There was a problem hiding this comment.
Assert exact agent socket mode (0666) to match the contract.
The current check only requires some group/other RW bits. It won’t catch drift from the explicit 0666 requirement.
Suggested fix
expect(fs.statSync(operatorSocketPath).mode & 0o777).toBe(0o600);
- // agent socket must be connectable by the container (different uid):
- // at least group/other read+write.
- expect(fs.statSync(agentSocketPath).mode & 0o066).not.toBe(0);
+ expect(fs.statSync(agentSocketPath).mode & 0o777).toBe(0o666);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('the operator socket is created mode 0600; the agent socket is group/other-connectable', async () => { | |
| await withTempBrain(async (bp) => { | |
| initBrain(bp); | |
| const { operatorSocketPath, agentSocketPath } = await start(bp); | |
| expect(fs.statSync(operatorSocketPath).mode & 0o777).toBe(0o600); | |
| // agent socket must be connectable by the container (different uid): | |
| // at least group/other read+write. | |
| expect(fs.statSync(agentSocketPath).mode & 0o066).not.toBe(0); | |
| }); | |
| it('the operator socket is created mode 0600; the agent socket is group/other-connectable', async () => { | |
| await withTempBrain(async (bp) => { | |
| initBrain(bp); | |
| const { operatorSocketPath, agentSocketPath } = await start(bp); | |
| expect(fs.statSync(operatorSocketPath).mode & 0o777).toBe(0o600); | |
| expect(fs.statSync(agentSocketPath).mode & 0o777).toBe(0o666); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/daemon/dual-socket.test.ts` around lines 96 - 104, The test currently
asserts only that agentSocketPath has some group/other RW bits; change the
assertion to require the exact mode 0o666 per the contract. In the test named
"the operator socket is created mode 0600; the agent socket is
group/other-connectable" replace the loose check on
fs.statSync(agentSocketPath).mode with an exact mask/assertion that the full
permission bits (e.g., mode & 0o777) equal 0o666 so the agent socket's
permissions are enforced precisely.
This was referenced May 16, 2026
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
1 participant
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.
Phase 6 of the v2 privileged-write-channel (issue #1). Resolves the
SO_PEERCRED deferral and ships the container-topology artifacts. Builds
on merged Phases 1–5.
Contract amendment (principled, not ad-hoc)
Node exposes no public SO_PEERCRED API. Per this doc's own change
process (§1: "deviations require a revision to this doc"), §2.2 is
amended with the forcing rationale: capability is established by
which socket a connection arrives on, kernel-enforced via
connect()permissions — the parent DESIGN already sanctioned "a different socket /
capability", so this is faithful, not a deviation.
operatoragent(capped)0666is safe: access ≠ privilege — that socket only ever grants theagentcapability (dispatcher gate, §5/§3.1a).What landed (TDD red→green)
server.ts: optionalsocketMode+ sharedDispatcher(DRY reuse).dual-socket.ts:DualSocketDaemon— one Dispatcher, two listeners.4 tests: capability-by-socket, 0600/0666 modes, shared brain,
agent
verify_entry→TRUST_DENIEDover the wire.main.ts: dual-socket production mode (MYKB_OPERATOR_SOCKET+MYKB_AGENT_SOCKET); single-socket operator dev default retained.deploy/mykbd.service: hardened reference systemd unit.docs/v2-container-topology.md: RO-brain + agent-socket-bind-mounttopology + the viloforge-platform argo integration seam (the
cross-repo manifest change is a deployment action, out of mykb-repo
scope per container-only scope + vafi-config-in-viloforge-platform).
Evidence
Full suite 650/650 across 2 runs; 69/69 daemon post-rebase; tsc +
lint clean.
Next: P7 — flip
bash-bypass-known-gap🐛→✅ (the acceptance criterionfor #1).
Summary by CodeRabbit
Documentation
New Features