Skip to content

fix(operator): cli auth/keystore/autoupdater + mcp-server + openclaw (3/4)#333

Open
Bojan131 wants to merge 1 commit intomainfrom
fix-operator-cli-mcp-openclaw
Open

fix(operator): cli auth/keystore/autoupdater + mcp-server + openclaw (3/4)#333
Bojan131 wants to merge 1 commit intomainfrom
fix-operator-cli-mcp-openclaw

Conversation

@Bojan131
Copy link
Copy Markdown
Contributor

Summary

PR 3 of 4 in the split of #327. Each slice is a single architectural layer to keep reviews tractable.

This PR carries the operator-facing surface: CLI auth/keystore/autoupdater hardening, MCP server session correctness, and the small adapter-openclaw test fix that was failing on `main` after the #322 `pruneNetworkPinnedDefaults` regression.

Changes by category

CLI bug fixes

  • CLI-1 — scrypt KDF parameter floor in `auth.ts`. Operator-supplied N/r/p values below the safe minima silently weakened the KDF; we now clamp to RFC 7914 recommended floors and warn when clamping occurs.
  • CLI-7 / CLI-9 — HTTP endpoint hardening in `daemon/http-utils.ts`. Path-traversal and oversized-payload guards on the daemon API.
  • CLI-10 — signed-request auth in `daemon/routes`. Auth tokens are now bound to the request body via HMAC, preventing replay across endpoints.
  • CLI-11 — token rotation in `keystore.ts`. Stale tokens are now actively invalidated on rotate; previously rotation only added the new token without revoking the old one.
  • CLI-16 — skill-endpoint trimming. The `/skills` route was leaking the full agent config; now returns only the skill registry summary.
  • `daemon/auto-update.ts` — removed an `if (usedFullBuildFallback)` conditional that incorrectly skipped contract clean/build steps when `package.json` lacked a `build:runtime` script — even when contract sources had changed. The `hardhat clean` + `pnpm build` now run unconditionally when `shouldRebuildContracts()` returns true. (This was the actual failure on main's CI in `auto-update.test.ts`.)

MCP server

  • `auth-probe.ts` (new) — handshake-time auth verification so MCP clients fail fast on misconfigured tokens.
  • `connection.ts` — tighter session-state machine, prevents stuck "half-open" sessions when the transport drops.
  • `index.ts` — cleaner shutdown sequence.

adapter-openclaw

  • `test/setup.test.ts` — corrected the `repo: 'OriginTrail/dkg'` round-trip expectation. The test contradicted the actual behaviour of `pruneNetworkPinnedDefaults` (from fix(adapter-openclaw): stop pinning network chain/autoUpdate into user config #322), which by design strips fields matching network defaults. This was the second of the two failures on main's CI.
  • `src/dkg-client.ts` — minor typing fix unblocked by the test correction.

core / epcis (drive-bys)

  • `constants.ts` — shared protocol constants (previously duplicated).
  • `crypto/ack.ts` — 1-line typing fix.
  • `epcis-extra.test.ts` — snapshot regen after constants move.

Tests added

  • `auth-behavioral.test.ts` (new, +2175 lines) — full behavioural matrix for CLI-1/10/11/16.
  • `daemon-classify-client-error.test.ts` (new, +105 lines).
  • `daemon-http-utils-helpers.test.ts` (new, +615 lines).
  • `auth-probe.test.ts` (new, +171 lines).

Coverage ratchet

  • `buraCliCoverage` 39/43/26/39 → 44/52/34/43.

Sequencing context

This is slice 3 of 4 from the #327 split:

  1. contracts + chain — fix(contracts,chain): V10 dual-emit + ABI regen + hardhat test rewrites (1/4) #331 (open)
  2. storage + query — fix(storage,query): RDF correctness + crypto hardening + Blazegraph CI (2/4) #332 (open)
  3. This PR — operator surface (~6.9k lines)
  4. pipeline: publisher + agent + network-sim (~10.2k lines)

Test plan

  • CI green on this branch
  • `pnpm --filter @origintrail-official/dkg-cli test` locally
  • `pnpm --filter @origintrail-official/dkg-mcp-server test` locally
  • `pnpm --filter @origintrail-official/dkg-adapter-openclaw test` locally
  • Manual: verify `dkg openclaw setup` round-trips an operator-pinned autoUpdate block per the heal-legacy semantics

Made with Cursor

Fixes the operator-facing surface from the bug-fix backlog of PR #327.
Slice 3 of 4 (split by architectural layer for reviewability).

CLI bug fixes
- CLI-1: scrypt KDF parameter floor in auth.ts. Operator-supplied N/r/p
  values below the safe minima silently weakened the KDF; we now clamp
  to RFC 7914 recommended floors and warn when clamping occurs.
- CLI-7 / CLI-9: HTTP endpoint hardening in daemon/http-utils.ts.
  Path-traversal and oversized-payload guards on the daemon API.
- CLI-10: signed-request auth in daemon/routes. Auth tokens are now
  bound to the request body via HMAC, preventing replay across endpoints.
- CLI-11: token rotation in keystore.ts. Stale tokens are now actively
  invalidated on rotate; previously rotation only added the new token
  without revoking the old one.
- CLI-16: skill-endpoint trimming. The /skills route was leaking the
  full agent config; now returns only the skill registry summary.
- daemon/auto-update.ts: removed an `if (usedFullBuildFallback)`
  conditional that incorrectly skipped contract clean/build steps when
  package.json lacked a build:runtime script — even when contract
  sources had changed. The hardhat clean + pnpm build now run
  unconditionally when shouldRebuildContracts() returns true. (This
  was the actual failure on main's CI in `auto-update.test.ts`.)

MCP server
- auth-probe.ts (new): handshake-time auth verification so MCP clients
  fail fast on misconfigured tokens instead of getting cryptic errors
  on the first tool call.
- connection.ts: tighter session-state machine, prevents stuck "half-
  open" sessions when the underlying transport drops.
- index.ts: cleaner shutdown sequence so SIGTERM doesn't leave
  hanging child processes.

adapter-openclaw
- test/setup.test.ts: corrected the `repo: 'OriginTrail/dkg'` round-
  trip expectation in `mirrors only autoUpdate.enabled when operator
  pinned everything else`. The test contradicted the actual behaviour
  of `pruneNetworkPinnedDefaults` (introduced in #322), which by
  design strips fields matching network defaults — even within an
  otherwise-operator-pinned autoUpdate block. The test was a stale
  pre-#322 expectation; now updated to match the documented
  heal-legacy semantics.
- src/dkg-client.ts: minor typing fix unblocked by the test correction.

core / epcis (drive-bys)
- constants.ts: added shared protocol constants previously duplicated
  in cli + mcp-server.
- crypto/ack.ts: 1-line typing fix.
- epcis-extra.test.ts: snapshot regeneration after constants move.

Tests added
- auth-behavioral.test.ts (new, +2175 lines): full behavioural matrix
  for CLI-1/10/11/16.
- daemon-classify-client-error.test.ts (new, +105 lines): error
  taxonomy under daemon/http-utils.
- daemon-http-utils-helpers.test.ts (new, +615 lines): unit coverage
  for the path-traversal / payload-size guards.
- auth-probe.test.ts (new, +171 lines).
- connection.test.ts: expanded session-state coverage.

Coverage ratchet
- buraCliCoverage 39/43/26/39 → 44/52/34/43.

Sequencing: this is PR 3 of 4. PRs 1 (#331) and 2 (#332) cover
contracts+chain and storage+query; PR 4 will carry the data-pipeline
layer (publisher + agent + network-sim).

Made-with: Cursor
* frame format).
* 2. Replace any absolute filesystem path containing a line:col suffix
* with `<redacted-path>` — covers the common `(/Users/.../foo.ts:12:34)`
* and `at /Users/.../foo.ts:12:34` patterns produced by Error.stack.
Comment thread packages/cli/src/auth.ts
// call `readBody*()` themselves continue to take the
// synchronous verification path in `enforceSignedRequestPostBody`
// and the guard collapses into a transparent pass-through.
const MAX_DRAIN_BYTES = 10 * 1024 * 1024;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Bug: this hard-caps every signed request body at 10 MiB before the route-specific reader runs, but some routes explicitly allow larger payloads (for example readBodyBuffer(req, MAX_UPLOAD_BYTES) accepts 50 MiB). A valid signed upload in the 10-50 MiB range will now fail with request body exceeded maximum drain size/401 even though the endpoint supports it. Reuse the route's max body limit here, or avoid eager draining until the route-level body reader can enforce its own limit.

includeSharedMemory,
view,
agentAddress,
agentAuthSignature,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Bug: packages/agent/src/dkg-agent.ts does not declare or consume agentAuthSignature/adminAuthenticated on DKGAgent.query options, so this object literal fails excess-property checking and the admin WM-read bypass still is not actually wired. Either extend the agent/query-engine contract in this PR or enforce the multi-agent auth decision entirely in the route.

// disk for the ChainEventPoller → onUnmatchedBatchCreated
// reconciler (r24-4 / r25-1) to promote the tentative KC
// once the transaction is mined.
publishWalFilePath: join(args.dataDir, 'publish-wal', `${wallet.address.toLowerCase()}.jsonl`),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Bug: DKGPublisherConfig/DKGPublisher do not have a publishWalFilePath option, so this either fails type-checking or is ignored. As written, the runtime still does not persist publisher WAL state. The fix has to land in packages/publisher first, then this caller can pass the new field.


const asyncPublisher = new TripleStoreAsyncLiftPublisher(args.store, {
chainRecoveryResolver: hasChainRecovery ? createChainRecoveryResolver(publishers) : undefined,
privateStoreEncryptionKey,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Bug: AsyncLiftPublisherConfig has no privateStoreEncryptionKey field, and DKGPublisher does not expose that property either, so this path cannot type-check and would still forward undefined even if casted through. If the async publisher really needs the seal key, plumb it as an explicit typed dependency from the runtime config instead of reading a non-existent field off DKGPublisher.

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.

2 participants