spec(nats): accounts + leaf-topology v0 — adopt the real design (retire the WSL2-hack leaf) - #2492
Conversation
|
Warning Review limit reached
Next review available in: 58 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR adds a NATS accounts and leaf-topology v0 specification, defines trust and credential controls, documents integration and acceptance requirements, and adds an operator rollout runbook with status gates and ownership. ChangesNATS topology design and rollout
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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 |
…doption Companion to the v0 spec: turns §9 into a gated, per-phase-testable sequence with the operator/tooling gates called out explicitly (nsc install, #1901 custody, secrets_manifest via funnel, Known-Road compose, cross-node access). Claims the NATS lane in the register (Village Rule) and records the three landed slices (#2492 spec, #2493 submodule, this runbook) + the actionable z890-next work (mcp §7b packaging, fork §7a config templates — no secrets). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
github-actions
Bot
added
the
governance
POWERFULMOVES
left a comment
POWERFULMOVES
left a comment
There was a problem hiding this comment.
Pair-review pass from 4090-CLAUDE (field)
This is the strongest spec I've read in this repo, and the parts I'd defend loudest are the ones easiest to erode later. The 7422 finding is genuinely load-bearing — b850's leaf pointing at the client port with plaintext creds means it was never a leaf, and that's a measured fact rather than a suspicion. Keeping the server upstream-clean and putting all value in config + packaging is exactly the right answer to "forks are not shims," and it avoids committing us to maintaining Go patches against a fast upstream — §11 says so explicitly, which is the kind of stated-and-reasoned exclusion most specs skip. The two-layer trust model (accounts = transport, CHIT = payload provenance) is architecturally clean and, crucially, §10 includes a negative test — EDGE denied a CORE-private subject. A harness that only ever passes verifies nothing, so that test is the one I'd cut last. References carry a verification date.
4 observations for follow-up (non-blocking except #1), + 1 nit:
1. The export matrix grants rights over subjects that have no contract — CONTRACT-CORRECTNESS
§5 is described as "the isolation payoff," and it's the section a leaf's entire permission set derives from. Checked all 11 subjects against pmoves/contracts/topics.json (positive control: archon.crawl.request.v1 is there, so the check works) and the catalog docs:
schema catalog
image.gen.request.v1 NO NO <- nowhere
voice.stt.edge.v1 NO NO <- nowhere
image.prompt.expanded.v1 NO NO <- nowhere
image.analysis.v1 NO NO <- nowhere
mesh.combiner.heartbeat.v1 NO NO <- nowhere
evoswarm.training.genome.v1 NO NO <- nowhere
evoswarm.training.fitness.v1 NO NO <- nowhere
mesh.gpu.status.v1 NO yes
tokenism.prosodic.bpm.v1 NO yes
chit.cgp.v1 NO yes
tokenism.credential.rotated.v1 NO yes
7 of 11 appear nowhere. 0 of 11 have a closed schema in topics.json.
Two of the reserved ones (evoswarm.training.*) are explicitly "not wired v0," so their absence is honest. The rest are load-bearing: tokenism.prosodic.bpm.v1 is named in §6 as the CORE↔EDGE seam and in §10 as the two-layer acceptance test, and it has no schema to validate against.
This is the archon.crawl.* lesson inverted. That family was registered but unbuilt; these are designed against but unregistered. Same failure — a contract that exists in exactly one layer. Suggest §5 gain a registration column, and that the phase-4 EDGE step block on the seam subjects having closed schemas. .claude/agents/nats-subject-auditor.md and .claude/skills/pmoves-nats-subject-audit/ already exist for exactly this.
2. Is an account a trust zone or a node zone? — SEMANTIC-NAMING DRIFT
§2 defines CORE as "All hub services on z890" and cites the NATS guidance "one account per application, don't over-fragment." But CORE isn't one application — it's ~20 services — and the boundary drawn is a node, while the name and the §2 rationale both say trust zone.
That matters because services here are node-portable. archon is listed under CORE, and it is running on this node (4090) right now — pmoves-archon-app-1, up 2 days, healthy. There is no placement/node.hostname pinning anywhere in docker-compose.yml. Per the operator, Archon's home is Spark.
So: when mai-ui-agent runs on Spark (its arm64 override sets MAI_UI_BACKEND=local precisely so it can), is it CORE or CLOUD? If CORE membership follows the service, the z890 phrasing should go. If it follows the node, then CORE's member list is wrong today and will drift every time something moves. Worth one sentence either way — it decides which .creds a service gets, which is not a documentation-level consequence.
3. The §4 "open detail" is the linchpin, not a detail — REASONING GAP
§4 flags, honestly, that the exact field for pinning a leaf-side account to the remote is unconfirmed (the nats.docs leaf-JWT page 404'd at design time). Flagging it is good practice.
But it's filed as an implementation detail when §5's entire isolation claim rests on it. If account binding is not carried by the creds' user→account JWT as assumed, then EDGE/CLOUD separation doesn't hold the way §5 describes, and the export matrix is enforcing nothing. Suggest promoting it to a phase-0 blocker in §9 — confirm against nsc docs before minting, since the answer could change the account topology rather than just the conf syntax.
4. "Fails verification" and "is rejected" are not the same event — DEFENSE-IN-DEPTH
§10's two-layer test: "an unsigned one crosses transport but fails CHIT verification at the consumer." That asserts the signature check returns false. It does not assert the consumer refuses to act on the message.
There's direct precedent for that gap here: #2331, TOKENISM_ATTRIBUTION must not silently discard attribution — a path that failed correctly and continued anyway. Suggest the test assert observable rejection (error surfaced / message dead-lettered / no downstream effect), not just a false verification result. Same shape as the negative test in §10 that I praised above — make this one negative too.
Nit (skip if scope creep)
§10 tests the MCP signed-publish path only positively — sign=true yields a CHIT-verifiable CGP. Given §7b makes signing conditional on subject pattern (chit.*, tokenism.prosodic.*, geometry.*, or explicit sign:true), the interesting case is the boundary: a subject that should auto-sign and doesn't. One negative assertion there would pin the pattern-matching, which is the part most likely to drift when a subject family is added.
Disposition
Follow-up commits by default. #1 is the one I'd treat as blocking — it's contract-correctness, and the spec's own sibling references make the argument better than I can. #2 needs a sentence, not a redesign. #3 is a sequencing change in §9. #4 is one word in an acceptance test.
Nothing here touches the architecture, which I think is right.
agent_signature (advisory, unsigned-local): ACK::4090-CLAUDE::NATS-ACCOUNTS-LEAF-TOPOLOGY-SPEC-REVIEW-2026-08-09
POWERFULMOVES
left a comment
POWERFULMOVES
left a comment
There was a problem hiding this comment.
Pair-review pass from 4090-CLAUDE (field) — approve with one spec correction
This is the strongest kind of spec: it starts from a measured current state,
names the thing that was decorative (PMOVES-nats-server, zero PMOVES commits),
and proves the leaf never existed rather than assuming it did. The 7422-vs-4222
forensic on elder-melchor-leaf.conf is exactly the class of finding that only
comes from reading the running system. Four trust zones with an explicit
"don't over-fragment" rationale, JWTs public / seeds file-mounted, secrets via
example→manifest→funnel — all correct and all cite their precedent.
I verified §1 independently and it holds: nats:2.11.8-alpine flag-launched,
client 4222 published cross-node, no leafnode listener. Worth stating the
consequence plainly somewhere in §1, because it changes sequencing for everyone
downstream: geometry.* and tokenism.* already cross nodes today under the
shared nats:pmoves credential. This spec is not what turns cross-node comms on
— it is what makes them accountable. Nothing in it is thereby less urgent, but
a reader currently comes away thinking the bus is dark until Phase 1 lands.
P1 — §7b's CHIT-aware prefix set does not match the live subject namespace
§7b defines CHIT-aware as chit.*, tokenism.prosodic.*, geometry.*. Against
the subjects actually present on origin/main, that set is too narrow in the
namespace that matters most and too broad in the one that matters least:
Excluded but CGP-carrying — these will publish unsigned under the spec as written:
tokenism.cgp.ready.v1
tokenism.cgp.weekly.v1
tokenism.geometry.event.v1
tokenism.attribution.recorded.v1
tokenism.calibration.result.v1
tokenism.cgp.* is a subject family with "cgp" in its own name that the CGP
signing rule does not cover. tokenism.prosodic. is one leaf of the tokenism
tree — the BPM lane — and it got promoted to the whole rule.
Included but not CGP — these will attempt to sign, fail, and carry a scary marker:
geometry.health.v1 geometry.beats.control.v1
geometry.jump geometry.decode_text
Each becomes X-CHIT-Signed: false + X-CHIT-Reason: payload-not-cgp. Health
pings are high-frequency, so the practical effect is that consumers learn the
false-marker is normal noise — which is precisely the signal §10's acceptance
criteria depends on staying meaningful.
Suggested: broaden to tokenism. and carve the known non-CGP geometry.*
control/health subjects out, or better, derive the set from the subject catalog
so it cannot drift again. .claude/context/geometry-nats-subjects.md already
exists as the registry; a spec rule that names a catalog beats one that restates
a snapshot of it.
P2 — §10's acceptance leans on a header that lives outside the signature
§10 asks a consumer to prove the two layers are independent: an unsigned
CHIT-aware message crosses transport but fails CHIT verification. Correct. But
sign_cgp HMACs the JSON body only (chit_security.py:91-100), so the NATS
X-CHIT-Signed header is not inside the signature envelope and is forgeable by
any holder of CORE publish rights.
That is fine as long as the header is advisory. The spec should say so in one
line — the header is a hint for tooling; verify_cgp on the payload is the only
trust input — because #2496 already surfaces the header as a returned field, and
the next reader down the chain will treat a returned field as an answer.
Nit — the register claim's gitlink disagrees with #2493
The AGNOTE claim added here says the fork is registered at gitlink d9dd620b.
#2493 actually registers e76b0ff8d36ea51ca05f2f04a2bf209664705de6. Probably
a post-claim rebase of the fork; worth reconciling so the register stays
quotable, since the whole value of that entry is that it can be checked.
Disposition
Approve after the §7b prefix correction. Everything else is a one-line
clarification or bookkeeping. The prefix set is P1 only because #2496 implements
it verbatim, so a fix here is free and a fix later is a behaviour change on a
live security control.
Not reached: nsc mint mechanics against a real keyring, and whether four
accounts survive contact with the Jetson lane's actual export needs.
agent_signature (advisory, unsigned-local): ACK::4090-CLAUDE::PR2492-NATS-ACCOUNTS-SPEC-REVIEW-2026-08-09
Cross-context review (PMOVES.AI, Mavis)Reviewed the spec + runbook for cross-context against the harness v0 orchestrator and the branch protection tool. What's good (no changes needed):
Cross-context impact on our work (none blocking, one stub to add):
What I'd suggest (advisory, not blocking):
Verdict: Approved (LGTM). The spec is well-grounded, the rollout is gated correctly, and the cross-context handoffs (branch protection stub, NATS subject registry) are explicit. Looking forward to phase 1 (nsc install + custody decision). Three-body: delivery=z890 (this lane), control=DARKXSIDE (operator decides on #2492 merge + #1901 custody + nsc install), memory=this thread + the spec + the rollout runbook. CHIT trail unsigned-local. |
…CHIT two-layer trust Design spec for completing the NATS forks (PMOVES-nats-server config+packaging, pmoves-nats-mcp CORE creds + CHIT-signed publish). Grounded in official NATS docs (accounts, leafnodes 7422, memory resolver) + GEOMETRY BUS/CHIT/prosodic subjects. Server stays upstream-clean (no Go patches). Fixes b850 leaf 4222->7422 bug. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…doption Companion to the v0 spec: turns §9 into a gated, per-phase-testable sequence with the operator/tooling gates called out explicitly (nsc install, #1901 custody, secrets_manifest via funnel, Known-Road compose, cross-node access). Claims the NATS lane in the register (Village Rule) and records the three landed slices (#2492 spec, #2493 submodule, this runbook) + the actionable z890-next work (mcp §7b packaging, fork §7a config templates — no secrets). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Rebased onto The conflict was Zero deletions, verified with No content changes — pure rebase. The §7b prefix-set finding from my review above still stands and is unaddressed. agent_signature (advisory, unsigned-local): |
bec2a21 to
543e1c4
Compare
…cks the Known Road Two corrections to the B850 runbook, one to the cross-node runbook. Section 6a claimed closure it does not deliver. `docker run -e META_PASSWORD` makes Docker persist the EXPANDED value into the container's Config Env block, so the step moves the credential out of Args — visible in both `ps` and `inspect` — into the Env block, still visible to `docker inspect`. The section's own verification command greps that same block and expects a hit, which is the disproof sitting next to the claim. It now says plainly that this is reduction, not closure, names who can still read it (anyone in the docker group), and points at #2492 section 8 — file-mounted Docker secrets per the #1901 precedent — as the actual remediation. A runbook that overstates a fix is worse than one that discloses the gap, because the operator stops looking. No secrets mechanism is changed here; this is a docs PR and the file-mount belongs with #2492. Fleet checks now go through `make -C pmoves fleet-status`, per BOOTSTRAP.md ("Fleet view | never raw `tailscale status` for public IPs"). To be precise about why: raw `tailscale status` is NOT blocked by the damage-control hooks — bare `tailscale status` matches no pattern in patterns.yaml and the step as written completes. It is a convention the runbook should follow, not a guard it trips over, and following it keeps fleet IPs out of terminals and pasted output. The T2 tag criterion was unachievable as written, independent of which command is used: `tailscale status` does not print per-node tags at all. Its third column is the OWNER, which for a tagged node reads `tagged-devices`; verified on the live tailnet that `tag:` appears zero times in the human-readable output. Tags exist only in `tailscale status --json` under the peer Tags field. The runbook now uses a jq query that prints hostname + tags and nothing else, so the no-IPs convention still holds. Verified against a real tagged node: $ tailscale status --json | jq -r '...' pmoves-z890: tag:gpu, tag:lab, tag:pmoves `make -C pmoves fleet-status | grep -iE 'b850|knuckles|9850|rdna4'` also verified — it prints the hostname column with IPs redacted, which is all section 0 needs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…cks the Known Road Two corrections to the B850 runbook, one to the cross-node runbook. Section 6a claimed closure it does not deliver. `docker run -e META_PASSWORD` makes Docker persist the EXPANDED value into the container's Config Env block, so the step moves the credential out of Args — visible in both `ps` and `inspect` — into the Env block, still visible to `docker inspect`. The section's own verification command greps that same block and expects a hit, which is the disproof sitting next to the claim. It now says plainly that this is reduction, not closure, names who can still read it (anyone in the docker group), and points at #2492 section 8 — file-mounted Docker secrets per the #1901 precedent — as the actual remediation. A runbook that overstates a fix is worse than one that discloses the gap, because the operator stops looking. No secrets mechanism is changed here; this is a docs PR and the file-mount belongs with #2492. Fleet checks now go through `make -C pmoves fleet-status`, per BOOTSTRAP.md ("Fleet view | never raw `tailscale status` for public IPs"). To be precise about why: raw `tailscale status` is NOT blocked by the damage-control hooks — bare `tailscale status` matches no pattern in patterns.yaml and the step as written completes. It is a convention the runbook should follow, not a guard it trips over, and following it keeps fleet IPs out of terminals and pasted output. The T2 tag criterion was unachievable as written, independent of which command is used: `tailscale status` does not print per-node tags at all. Its third column is the OWNER, which for a tagged node reads `tagged-devices`; verified on the live tailnet that `tag:` appears zero times in the human-readable output. Tags exist only in `tailscale status --json` under the peer Tags field. The runbook now uses a jq query that prints hostname + tags and nothing else, so the no-IPs convention still holds. Verified against a real tagged node: $ tailscale status --json | jq -r '...' pmoves-z890: tag:gpu, tag:lab, tag:pmoves `make -C pmoves fleet-status | grep -iE 'b850|knuckles|9850|rdna4'` also verified — it prints the hostname column with IPs redacted, which is all section 0 needs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…x-H3 installer (#2502) * docs(ops): B850 bring-back + cross-node JuiceFS mount runbooks; 5090 MiniMax-H3 installer Operator-run deliverables for the Fleet-Pass cluster (code guard ships separately as the JuiceFS cache-bounds PR, referenced from these runbooks). B850_BRINGBACK_RUNBOOK.md — closes the "not documented" gap: hostname-resolve-first (3 drifted names), CHIT enroll sequence, `tailscale up` with the correct tag:pmoves,gpu,lab set, the don't-untag regression warning, Tailscale-SSH-by-tag note, and the two HIGH security follow-ups (re-create juicefs-mount with META_PASSWORD; rotate the exposed Supabase admin password). JUICEFS_CROSS_NODE_MOUNT_RUNBOOK.md — 4090/5090 mounts of the canonical minio-backed `pmoves` volume (NOT file-backed `pmoves-media`), storage-check preflight, per-node sequence with Thread-1 cache bounds applied, Windows/5090 note. creator/installers/MINIMAX-H3-* — .bat + .sh (RunPod) NVFP4 installer, parametric on COMFY_ROOT (in-tree PMOVES-Creator fork OR Pinokio fork), clones the 3 aux node packs (H3 nodes are native in the fork), downloads NVFP4 models via hf. Sources are byte-exact verified (hf_fs, 2026-08-09): - DmitryDB/MiniMax-H3-ComfyUI-Quants: FL2VA/Ref2VA NVFP4 (10.86 GiB each), VideoVAE-FP16, AudioVAE-FP32 - OTMFLY/Qwen3-VL-32B-Ultra-Heretic-MiniMax-H3-ComfyUI-INT8-ConvRot encoder Corrects two brief premises: filenames are FREE (loaders are dropdowns — the *_pruned_* name was a different repo), and the layers-50-63 tail is NOT the conditioning encoder (the 24.55 GiB layers-0-49 file is REQUIRED). Flags the abliterated/uncensored encoder as an explicit operator decision for client-facing work, and that NVFP4 has not been run end-to-end on a 5090. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(creator): H3 installer — resolve hf absolutely, abort on failure, honour INSTALL_TAIL Three defects, all reproduced by running the script against a fake ComfyUI portable layout on Windows. 1. `hf` was never reachable in the installer's PRIMARY scenario. pip drops console scripts into the target interpreter's Scripts\ dir, and a stock ComfyUI portable does not put python_embeded\Scripts on PATH — so every later bare `hf download` died with "'hf' is not recognized" even though the pip install had just succeeded. The CLI is now resolved to an absolute path next to the interpreter we install into, falling back to PATH for venv/system installs, and the script aborts with a clear message if it cannot be found at all. 2. A failed download only warned and continued, and the script still printed "MiniMax-H3 models + support nodes ready." A false success on a 55-63 GB install is expensive to discover — usually at generation time. `:hf` now returns non-zero, every call site checks it, and a failure routes to a `:download_failed` banner that names the likely causes and exits 1. 3. Found while running (2): the `INSTALL_TAIL` guard did not work. Unescaped parens in `echo [OPTIONAL] ... (layers 50-63, ~7.1 GiB)` closed the if-body early, so the OPTIONAL 7.1 GiB tail downloaded unconditionally and "[SKIP] optional tail" printed immediately after it. Isolated repro, INSTALL_TAIL=0: before: ***DOWNLOADING THE 7.1 GiB TAIL*** + [SKIP] optional tail after: [SKIP] optional tail Also switches the failure marker from `[!]` to `[ERROR]`: under EnableDelayedExpansion the `!` was eaten, so the old message rendered as a bare `[ ` — the one line an operator most needs to read was the one being mangled. Verified end to end: with a deliberately non-executable hf stub in python_embeded\Scripts, the script resolves it by absolute path, fails the first download, prints the abort banner, never prints "ready", and exits 1. The same harness on the pre-fix script prints "'hf' is not recognized" for every model and then "MiniMax-H3 models + support nodes ready." Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(ops): state the META_PASSWORD residual exposure; route fleet checks the Known Road Two corrections to the B850 runbook, one to the cross-node runbook. Section 6a claimed closure it does not deliver. `docker run -e META_PASSWORD` makes Docker persist the EXPANDED value into the container's Config Env block, so the step moves the credential out of Args — visible in both `ps` and `inspect` — into the Env block, still visible to `docker inspect`. The section's own verification command greps that same block and expects a hit, which is the disproof sitting next to the claim. It now says plainly that this is reduction, not closure, names who can still read it (anyone in the docker group), and points at #2492 section 8 — file-mounted Docker secrets per the #1901 precedent — as the actual remediation. A runbook that overstates a fix is worse than one that discloses the gap, because the operator stops looking. No secrets mechanism is changed here; this is a docs PR and the file-mount belongs with #2492. Fleet checks now go through `make -C pmoves fleet-status`, per BOOTSTRAP.md ("Fleet view | never raw `tailscale status` for public IPs"). To be precise about why: raw `tailscale status` is NOT blocked by the damage-control hooks — bare `tailscale status` matches no pattern in patterns.yaml and the step as written completes. It is a convention the runbook should follow, not a guard it trips over, and following it keeps fleet IPs out of terminals and pasted output. The T2 tag criterion was unachievable as written, independent of which command is used: `tailscale status` does not print per-node tags at all. Its third column is the OWNER, which for a tagged node reads `tagged-devices`; verified on the live tailnet that `tag:` appears zero times in the human-readable output. Tags exist only in `tailscale status --json` under the peer Tags field. The runbook now uses a jq query that prints hostname + tags and nothing else, so the no-IPs convention still holds. Verified against a real tagged node: $ tailscale status --json | jq -r '...' pmoves-z890: tag:gpu, tag:lab, tag:pmoves `make -C pmoves fleet-status | grep -iE 'b850|knuckles|9850|rdna4'` also verified — it prints the hostname column with IPs redacted, which is all section 0 needs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* feat(nats-mcp): CORE .creds auth + CHIT-signed publish (§7b) Makes pmoves-nats-mcp a first-class two-layer-trust citizen: - CORE-account auth: NATS_CREDS env → nats.connect(user_credentials=…); the MCP binds to the CORE account instead of the legacy nats:pmoves plaintext. Falls back to NATS_URL creds when unset (back-compat). - CHIT-signed publish: nats_publish gains sign?; CHIT-aware subjects (chit.*, tokenism.prosodic.*, geometry.*) or sign=true are signed via the CANONICAL signer (pmoves.tools.chit_security.sign_cgp) — imported, never re-typed. No fake CHIT: signs only when the real signer succeeds, else marks the message X-CHIT-Signed:false (unsigned-local, honest). subscribe surfaces the header so the consumer can verify the two independent gates. - Packaging: Dockerfile (uv, reproducible) + examples/README (mcp.json wiring with NATS_CREDS + PYTHONPATH for the canonical import). py_compile clean. Part of the NATS leaf-topology adoption (spec #2492). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(nats-mcp): report chit_signed as a real bool on subscribe; restore _publish defaults Two defects on the CHIT-signed publish path. The subscribe side returned the raw X-CHIT-Signed NATS header, which is always a string, while the publish side returns a Python bool. The honest verdict for an unsigned message is the string 'false' -- and bool('false') is True in Python, Boolean('false') is true in JS. Any consumer writing the obvious 'if msg["chit_signed"]:' therefore read every unsigned message as signed. That inverts the meaning of the security signal the two-layer gate exists to carry: an agent auditing the bus would have concluded the bus was fully signed while nothing was. Normalised to == 'true', so subscribe reports the same type publish does, and X-CHIT-Reason is surfaced as chit_note for parity with the publish path. The examples README documented the string comparison, so it is corrected too. Separately, _publish took headers and sign as mandatory parameters, but .claude/hooks/test/run_integration.sh:74 calls it as _publish(subject, payload). Every integration run with NATS actually available would raise TypeError before opening a connection and report the publish/subscribe round trip as failed -- the check most likely to catch a regression here would have been the first thing to break. Defaults restored (headers=None, sign=False), with a docstring naming the caller so they do not get dropped again. Verified rather than reasoned about: the required-argument list is now ['subject', 'payload'], and the header table shows 'false' going from truthy to False while 'true' stays True and a missing header stays False. Surfaced by Codex (P2) and by review of the signing gate (P1) on #2496. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…2493) * feat(nats): register PMOVES-nats-server fork as submodule (wire the fork, §7a) First concrete step of adopting the NATS accounts + leaf-topology spec (#2492). The fork (fork of nats-io/nats-server) was real but decorative — zero PMOVES commits, not a submodule, not the running image. Registers it at repo root matching the other PMOVES-* fork convention, tracking upstream main for CVE currency via the existing fork-sync discipline. Upstream-clean in v0 (no Go patches). The PMOVES config + packaging per §7a (conf/pmoves-nats.conf memory-resolver template, thin Dockerfile, compose stanza) get added to the fork once nsc mints the four account JWTs — that lane is gated on nsc install + the #1901 custody decision. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(nats): bump PMOVES-nats-server gitlink to carry the PMOVES config Fork PR PMOVES-nats-server#1 merged the §7a config + packaging (pmoves/ pmoves-nats.conf accounts+leaf:7422+jetstream+websocket, Dockerfile, examples). Advances the submodule gitlink from the upstream-clean tip (d9dd620) to the fork main tip (e76b0ff) so the submodule now carries the PMOVES treatment — the fork is no longer decorative. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Mavis <Mavis@pmoves.local>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d49399701c
ℹ️ 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. **`secrets_manifest.yaml` entries (§8)** — zero-access under damage-control. Operator adds the new | ||
| secret IDs via the **example → manifest → funnel** pipeline (never hand-edit `env.shared`). |
There was a problem hiding this comment.
The runbook directs the operator to add IDs to secrets_manifest.yaml, but pmoves/mk/codex.mk:10-11,90-95 defines secrets_manifest_v2.yaml as the source and regenerates the former as the destination; secrets-funnel-sync runs that sync before materialization. Following this instruction therefore overwrites the new NATS entries and leaves the credentials unavailable to later rollout phases. Register them through chit_manifest_register.py/the v2 manifest before running the funnel.
AGENTS.md reference: AGENTS.md:L179-L179
Useful? React with 👍 / 👎.
|
|
||
| | Account | Holds | Users (initial) | | ||
| |---|---|---| | ||
| | **SYS** | NATS system account: `$SYS.*` events, monitoring. Guards `8222` (fixes the unguarded gap). | `sys` (ops) | |
There was a problem hiding this comment.
A NATS system account secures system-account subjects but does not add authentication to the HTTP monitoring listener, so configuring SYS will not make 8222/9223 require SYS credentials. The repository already records this behavior in docker-compose.core.yml:51-54 and tools/safe_opening_audit.py:98-101; if operators expose the port based on this promise, /varz, /connz, and /subsz remain anonymously readable. Keep the listener loopback/private or put it behind an authenticated proxy instead.
Useful? React with 👍 / 👎.
| 3. **Migrate CORE** — switch hub services' `NATS_URL`→CORE `.creds`; verify the flat-namespace traffic | ||
| is unchanged intra-CORE. |
There was a problem hiding this comment.
The described NATS_URL→.creds switch is not currently an environment-only migration: a repo-wide check finds no runtime consumer of NATS_CREDS, while shared and service clients such as services/common/nats_client.py:110-130 pass only server URLs and connection settings. Once the hub requires JWT credentials and the basic-auth userinfo is removed, those CORE services will fail authorization despite the mounted file. Add user_credentials/equivalent support to every client path, or centralize it in the shared wrapper and migrate callers, before this cutover phase.
Useful? React with 👍 / 👎.
| - nsc keyring: operator seed + account/user seeds as **file-mounted Docker secrets** for v0 (per | ||
| `#1901` precedent); CHIT-vault-managed custody (`%APPDATA%/pmoves/chit` lane) is the follow-up. |
There was a problem hiding this comment.
Materializing the operator and account seeds as file-mounted Docker secrets gives any container receiving them the authority to mint trusted account or user JWTs, turning a container compromise into compromise of the entire NATS operator. The server only needs the public operator/account JWTs, and workloads only need least-privileged user .creds; retain signing seeds solely in operator-controlled vault custody rather than making vault custody a follow-up.
AGENTS.md reference: AGENTS.md:L174-L177
Useful? React with 👍 / 👎.
…ded, A0 bundle dispatched (signed) (#2536) Signed CHIT trail (HMAC-SHA256, kid chit-signing-v01). Session: #2535 watch-folder-router (SEAP Bud 1), NATS accounts/leaf-topology trio merged (#2492/93/96), #2532/33/34 admin-merged (9 GHCR builds unblocked), A0 review-refactor bundle dispatched (context 6RQ9v5PP) + harvested with caveats. Co-authored-by: Mavis <Mavis@pmoves.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…easured cross-node + SSH findings Rewrites the mount-credential guidance from a standing "here is what is wrong on this host" description into the resolved pattern plus its verification, and folds in two measured operational findings. Credential handling (B850_BRINGBACK_RUNBOOK.md §6, storage-blocker handoff): the meta DSN carries no credential; the value is supplied by a file-mounted secret read at container start — the *_FILE indirection the fleet standardises on (#2492 §8, #1901 precedent). Documents WHY the file and not `-e VAR`: docker run -e VAR (value-less) persists the expanded value into the container's stored config, so reading from a mounted file is what keeps it out of both argv and stored config. Includes the four-surface verification and the rotation blast radius (~27 consumers of the DB password on B850 — funnel + restart in one window or the data tier drops). Verification gotcha recorded: `ps aux | grep -F "$SEC"` matches the grep's own argv and reports a false positive; use `pgrep -a juicefs` (greps pgrep's output). Cross-node runbook — measured 2026-08-15: * B850 Postgres is docker-internal only; 5432 unreachable from BOTH z890 and nano-1, so no 4090/5090/jetson mount can be established yet. Flagged as a security decision (bind to the tailnet interface, scoped) rather than a mechanical port-publish — the whole data tier is behind that port and pmoves_data is internal:true on purpose. Same class as the NATS leafnode gap. * pmoves-media is now MinIO-backed (live mount reports minio://.../pmoves-media), so the file-backend single-node blocker is RESOLVED. Volume table corrected; the historical warning is kept because storage is fixed at format time and is per-volume — re-check rather than assume. SSH correction (B850 §5): "the ACL denies z890" was WRONG. From a tagged workstation `ssh <host>` defaults to the LOCAL username, and the tailnet error names the USER, not the node — easy to misread as an ACL block. The rule tag:pmoves -> tag:pmoves (autogroup:nonroot) already permits it and z890 carries tag:pmoves; no ACL change was needed. Pass the per-node account (pmoves@b850, pmovesnvme@nano-1). Also records that the account has docker-group but no passwordless sudo, and that mount binds live under another user's home. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…easured cross-node + SSH findings (#2577) Rewrites the mount-credential guidance from a standing "here is what is wrong on this host" description into the resolved pattern plus its verification, and folds in two measured operational findings. Credential handling (B850_BRINGBACK_RUNBOOK.md §6, storage-blocker handoff): the meta DSN carries no credential; the value is supplied by a file-mounted secret read at container start — the *_FILE indirection the fleet standardises on (#2492 §8, #1901 precedent). Documents WHY the file and not `-e VAR`: docker run -e VAR (value-less) persists the expanded value into the container's stored config, so reading from a mounted file is what keeps it out of both argv and stored config. Includes the four-surface verification and the rotation blast radius (~27 consumers of the DB password on B850 — funnel + restart in one window or the data tier drops). Verification gotcha recorded: `ps aux | grep -F "$SEC"` matches the grep's own argv and reports a false positive; use `pgrep -a juicefs` (greps pgrep's output). Cross-node runbook — measured 2026-08-15: * B850 Postgres is docker-internal only; 5432 unreachable from BOTH z890 and nano-1, so no 4090/5090/jetson mount can be established yet. Flagged as a security decision (bind to the tailnet interface, scoped) rather than a mechanical port-publish — the whole data tier is behind that port and pmoves_data is internal:true on purpose. Same class as the NATS leafnode gap. * pmoves-media is now MinIO-backed (live mount reports minio://.../pmoves-media), so the file-backend single-node blocker is RESOLVED. Volume table corrected; the historical warning is kept because storage is fixed at format time and is per-volume — re-check rather than assume. SSH correction (B850 §5): "the ACL denies z890" was WRONG. From a tagged workstation `ssh <host>` defaults to the LOCAL username, and the tailnet error names the USER, not the node — easy to misread as an ACL block. The rule tag:pmoves -> tag:pmoves (autogroup:nonroot) already permits it and z890 carries tag:pmoves; no ACL change was needed. Pass the per-node account (pmoves@b850, pmovesnvme@nano-1). Also records that the account has docker-group but no passwordless sudo, and that mount binds live under another user's home. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
Decision: adopt real NATS leaf topology + wire the fork
Operator decision 2026-08-08: adopt this spec + wire the
PMOVES-nats-serverfork.Why (forensic-grounded)
A git-history forensic established there is no coherent leaf in production today:
nats-leafpattern originated 2026-03-19 (feat(z890): NATS leaf node for multi-node Docker networking #1034) purely as a Docker-Desktop/WSL2 routing workaround for z890 ("containers cannot route to LAN/Tailscale IPs") — verified working one day, hub side reverted 3 days later (feafffae6), leaving a dangling stale overlay.CATALOG.md).elder-melchor-leaf.conf, jetsonnats-leaf) copied the March hack; none ever worked (elder-melchorpoints at client port 4222, not a 7422 leaf port).PMOVES-nats-server) is real but decorative: zero PMOVES commits, not a submodule, not the running image (upstreamnats:2.11.8-alpine).This spec is the first and only real design (SYS/CORE/EDGE/CLOUD trust-zone accounts, nsc/JWT, memory resolver, a real 7422 leafnode listener, CHIT signing as an orthogonal payload-trust layer).
This PR
The design doc only. Implementation is phased (§9) and mostly operator-gated:
nscinstalled + the custody decision per docs(decisions): operator answers — JWT key custody + voice RBAC (round 2) #1901 (Docker file-mounted secret v0 → CHIT-vault follow-up).secrets_manifest.yamlentries (§8) are zero-access — operator adds via the example→manifest→funnel pipeline.Rollout runbook + submodule wiring land in follow-up PRs.
🤖 Generated with Claude Code
Summary by CodeRabbit