feat: build ts/packages/core - #11
Merged
Merged
Conversation
Mearman
marked this pull request as ready for review
September 10, 2026 09:39
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. |
Member
Author
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The TypeScript implementation of wire-mesh's protocol: ports/adapters architecture per the org's portable-runtime-and-storage-boundaries rules. Transport (length-prefixed CBOR frames over a connection), Storage (async key/value), Identity (device-id derivation, COSE signing/verification), and Clock are first-class port contracts; domain logic depends only on these and the generated schemas, never on a specific adapter's own imports. src/generated/protocol.ts is committed, machine-generated output (generate.ts, run via `pnpm generate`) from cddl.js against ../../../spec/protocol.cddl -- consumed as a git dependency (github:ExaDev/cddl.js#main), since it isn't published to npm. Domain logic is real for the two families the build-out plan names explicitly: handshake negotiation (protocol-version and capability-domain negotiation between two peers -- the mechanism agent-comms issue #31's fix depends on) and capability-token verification (COSE_Sign1 signature check, the self-certifying issuer-key rule, expiry/not-before/revocation, and recursive delegation-chain narrowing). Every other frame family is covered by schema validation only, not yet dispatch/session logic -- see the package's own README for what's deferred and why. Adapters: a Node net.Socket-based TCP transport matching Cascade's own transport shape, an in-memory KeyValueStorage for tests and single-process nodes, a Web Crypto identity (ECDSA P-256 and Ed25519, the two algorithms the spec's conformance vectors use), and the system wall clock.
ts/packages/core now exists, so the justfile's own "ts/ does not exist yet" no-op branches for these recipes are stale -- dispatch for real, matching how the conformance recipe already invokes rust/'s branch conditionally (still genuinely absent) and ts/'s unconditionally now.
… test) Mirrors conformance-verify's own pattern: install, lint, typecheck (which also builds every package and verifies its dual ESM/CJS surface with attw), confirm generate.ts's output matches the committed src/generated/protocol.ts (regenerate and diff, the same drift check conformance/'s vector files already get), then run the test suite -- including the conformance-check that round-trips conformance/'s golden vectors through the generated schemas. Added to required-checks alongside the two existing jobs.
It's no longer "None yet" -- record what actually exists (the ports/adapters architecture, which domain logic is real vs. deferred, the conformance-check) rather than leaving the README describing code that's now there.
CI failed installing cddl.js as a git dependency: its prepare script couldn't load tsdown.config.ts, since Node's native TS-stripping refuses to process a .ts file under node_modules -- exactly where a git-dependency install resolves into while prepare runs. Fixed upstream (ExaDev/cddl.js#7, tsdown.config.ts -> tsdown.config.js); this repins the lockfile to pick it up.
The previous fix (a prepare script that builds cddl.js during install) still failed in CI: eslint's type-aware linting of generate.ts resolved parse/emitModule to an error type, not real type info, something never reproduced locally across four different attempts (hoisted and isolated pnpm linkers, a from-scratch store, a genuine Linux container matching CI's exact Node/pnpm versions). Fixed upstream properly instead of chasing the install-time build further: ExaDev/cddl.js#8 commits pre-built, attw-verified dist/ output and removes the prepare script entirely, so a git-dependency install ships already-correct code and type declarations with no build step, and therefore no install-time environment sensitivity, at all.
coordinator-frame is new; the federation link frames, mesh-id, and share-descriptor are gone; revocation-entry is now a cose-sign1 over revocation-claims; handle-claims' mailboxes field is pluralised.
…n entries
RevocationCheck.isRevoked now receives the token's own issuer alongside its token-id, so an entry only counts when BOTH match -- only a token's own issuer may revoke it, and a third party's entry for someone else's token-id is ignored (the unsigned {token-id, revoked-at} shape management.cddl replaced let any peer falsely revoke any token). New verifyRevocationEntry ingests a gossiped revocation-announce entry: signature against its own embedded issuer-key, self-certifying issuer check, schema-parsed claims -- entries failing any check are dropped rather than stored. The ancestor-chain sweep (every parent's token-id checked, not just the leaf's) already falls out of verifyCapabilityToken's existing recursion.
Mearman
force-pushed
the
feat/ts-packages-core
branch
from
September 10, 2026 12:32
d3ebfd2 to
17cb5a4
Compare
Three verifier defects from review, all in the delegation chain: Scope narrowing was not implemented at all -- only the bearer-chain and expiry checks existed, so a child token with kind:"org" over /everything, a sibling path (/home/private under /work), or no path at all (the kind's whole-scope root) all verified under a path-narrowed parent. Narrowing now covers all three axes of authority: identical kind, an equal-or-descendant path compared on "/"-segment boundaries (/work/sub narrows /work; /workbook does not), and an identical capability verb (the verb grammar has no sub-verb relation, so a different verb is a different authority, not a narrower one). expectedBearer was propagated into the parent recursion, failing every ancestor in any chain of two or more hops -- the parent's bearer is structurally the child's issuer, never the leaf's presenter. It now applies to the leaf verdict only. decode() on a hostile payload or parent field threw instead of returning a verdict; both are now caught and mapped to the malformed/parent_invalid reasons that already existed for exactly this.
core/federation is retired per handshake.cddl ("a peer must never advertise or negotiate it"), but negotiate() intersected advertised domains unfiltered, so two peers that buggily advertise it would end up speaking a dead domain. Retired domains are now excluded from the intersection even when both sides advertise them.
… process frameReader called cbor2 decode() unguarded from the socket data handler, so one malformed length-prefixed body threw inside the EventEmitter listener and killed the process -- any peer could DoS any node. A body that fails to decode now rejects the receive() iteration and destroys that one connection: hostile wire input is a connection-level failure surfaced through the Transport port, not a process crash and not a silently swallowed frame. Schema-invalid but decodable frames are still dropped in-stream, as before -- an unrecognised frame from a newer peer is what version negotiation exists to tolerate.
The task's real input (../../../spec/protocol.cddl) lives above this pnpm workspace's root, and turbo refuses workspace-external paths in both task inputs and globalDependencies -- so every cache hit was keyed on generate.ts alone and replayed stale output after a spec edit (reproduced: touch the spec, turbo replays the cached run). Generation is a sub-second script, so the _generate task is now simply uncached.
Only exercised indirectly until now. The derivation is the exact identity.cddl rule both Cascade and agent-comms previously got wrong (hashing the whole certificate DER), so an explicit test recomputing sha256 over the same raw bytes catches any drift back toward hashing anything else.
… and the deferral rationale The token-verification bullet defined delegation narrowing as bearer-chain and expiry only, silently dropping the scope and capability axes the verifier enforces. The deferral paragraph cited a build-out plan that lives outside this repository; replaced with the actual rationale (first pass scopes domain logic to the families with real business rules; conformance already covers every family's wire shape). generate.ts's comment named a nonexistent core-verify CI job (it is ts-core-verify).
…ive path segments verifyRevocationEntry called decode() unguarded on attacker-controlled gossiped bytes, so an entry with a garbage CBOR payload or non-canonical map keys threw instead of returning the malformed verdict that already existed for exactly this -- the earlier hostile-input guard covered the token path only. Guarded, with both shapes tested. pathNarrows compared paths purely lexically, so a delegate could sign "/work/../org" (or "/work/a/../b") and verify under a "/work" parent -- a path that normalises outside what the parent authorised. The spec is silent on path syntax, so the comparison now fails closed: any path containing a "." or ".." segment never narrows anything, documented in the function comment. Also pins the neighbouring edges the rule composes with: trailing-slash child under a non-slash parent accepted, a "/" parent admits every well-formed child path, empty and case-different child paths rejected.
listen() resolved to a bare close function, so a caller passing port 0 could never learn the OS-assigned port -- the contract had no path from "I asked for any free port" to "here is the address to dial". It now resolves a Listener carrying both the close handle and the address actually bound. The TCP adapter reads it back from the server's own listening callback; the transport test binds port 0 instead of a fixed port, so concurrent CI runs can never collide.
Member
Author
|
Merging on the strength of an independent adversarial review in place of the Codex security review, which is currently blocked by an external usage quota. The local review ran three rounds (full-diff, fix delta, final confirmation), verified every claim by executing the toolchain non-cached, and surfaced two blocking correctness defects (delegation scope-narrowing unimplemented; expectedBearer leaking into the parent recursion) plus robustness gaps (unguarded hostile-input decodes, retired-domain negotiation, a _generate cache key that ignored the spec) — all resolved in d0497d1..11a3973, confirmed by execution. Final state: 70/70 tests, 26/26 conformance vectors byte-exact, all CI checks green. |
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.
The TypeScript implementation of wire-mesh's protocol: ports/adapters architecture per the org's portable-runtime-and-storage-boundaries rules. Transport (length-prefixed CBOR frames over a connection), Storage (async key/value), Identity (device-id derivation, COSE signing/verification), and Clock are first-class port contracts; domain logic depends only on these and the generated schemas, never on a specific adapter's own imports.
src/generated/protocol.tsis committed, machine-generated output from cddl.js against../../../spec/protocol.cddl, consumed as a git dependency (github:ExaDev/cddl.js#main) since it isn't published to npm. CI regenerates and diffs it against the committed file, the same drift checkconformance/'s own vector files already get.Domain logic is real for the families with genuine business rules: handshake negotiation (the mechanism agent-comms issue #31's fix depends on, excluding retired domains from the intersection) and capability-token verification -- COSE_Sign1 signature check, the self-certifying issuer-key rule, expiry/not-before, issuer-matched revocation (only a token's own issuer's signed revocation-entry counts, swept across every ancestor in the delegation chain), hostile inputs returning verdicts rather than throwing, and delegation-chain narrowing across all three axes of authority: bearer-chain continuity, expiry, and scope/capability (identical kind and verb; equal-or-descendant path on segment boundaries). Every other frame family is covered by schema validation only for now, with the deferral documented in the package's own README.
Built against the redesigned spec (coordinator election, federation-link removal, signed revocation entries, pluralised mailboxes) -- the branch was rebased onto that merge and the domain logic updated for the new revocation obligations.
conformance-checkround-trips every vector inconformance/'s golden suite (26 vectors post-redesign) through the generated schemas, all passing byte-exactly. 62/62 tests total, plus a pinned test that device-id is exactly sha256 of the raw public-key bytes (the derivation both Cascade and agent-comms previously got wrong).just build/just test/just lint/just conformanceall dispatch intots/for real now.Follow-up PRs against cddl.js landed along the way, each a real gap found while actually consuming the generator as a dependency: exporting
parseas public API, and committingdist/so a git-dependency install needs no build step at all (an earlierprepare-script approach was replaced by this, fixing the root cause rather than working around Node's refusal to run TS configs undernode_modules).