feat: bind signed ClawHub default feed trust#101981
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 18, 2026, 1:36 PM ET / 17:36 UTC. Summary PR surface: Source +236, Tests +350, Docs +32. Total +618 across 8 files. Reproducibility: yes. from source: a currently valid signed profile containing Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Agree the signed-profile upgrade contract first, then ship a narrow doctor-guided migration or deliberately staged rollout that preserves valid existing configurations until an operator can supply the required payload identity, followed by redacted deployment-equivalent refresh proof once ClawHub provisions the signing key. Do we have a high-confidence way to reproduce the issue? Yes, from source: a currently valid signed profile containing Is this the best way to solve the issue? No; payload identity binding is a reasonable security control, but unconditional config rejection is not the best rollout until maintainers choose and prove an upgrade path for existing signed profiles. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 8d89ddd8c22b. Label changesLabel justifications:
Evidence reviewedPR surface: Source +236, Tests +350, Docs +32. Total +618 across 8 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (12 earlier review cycles; latest 8 shown)
|
5bd6f0a to
708d017
Compare
708d017 to
b59b36b
Compare
b59b36b to
a518ad4
Compare
Patrick-Erichsen
left a comment
There was a problem hiding this comment.
Reviewed exact head a518ad4ed3a2ea4d18ca923d61902d3f60035fb6.
Findings
-
[P1] Preserve built-in trust when
clawhub-publicis configured —src/plugins/official-external-plugin-catalog.ts:405resolveOfficialExternalPluginCatalogProfileConfig()builds the trusted default and then replaces the entire profile via...config?.feeds; the caller also omitsenvwhenever that profile exists in config (src/plugins/official-external-plugin-catalog.ts:463). A valid config entry that only restates the ClawHub URL/feed ID therefore drops the built-in verification and ignores both documented trust env vars, accepting unsigned JSON. Merge user fields into the built-in profile and require an explicitverification: { mode: "unsigned" }downgrade, or reject overrides of the built-in trust fields. Add coverage for a configuredclawhub-publicprofile with both trust env vars present. -
[P1] Enforce signed feed expiry before accepting fresh or cached authority —
src/plugins/official-external-plugin-catalog.ts:291The ClawHub producer's stored payload requires
expiresAt, and RFC 0009 requires expiry validation and reporting stale content without silently granting new authority. The consumer feed type/guard does not parseexpiresAt, andparseHostedCatalogFeedBody()plusloadHostedCatalogSnapshotResult()accept a validly signed payload or snapshot indefinitely. During endpoint failure/offline mode, an expired snapshot can therefore keep authorizing installs forever. ValidateexpiresAt > generatedAt, reject expired fresh payloads, and make snapshot fallback preserve visibility without granting install authority after expiry; cover fresh, 304, offline, and error fallback paths.
Verdict
The direction is plausible, but this is not yet the best or merge-ready fix: the default trust profile remains configuration-downgradable and the signed producer's expiry contract is not enforced. The key-rotation correction in #108342 should also land before this default feed trust is enabled.
Exact-head CI has no substantive source proof: CI preflight/tests/build/docs/security lanes were skipped, and the green “Real behavior proof” job only checked maintainer membership against the base checkout. The missing production public key and live signed endpoint proof are expected deployment blockers, separate from the code/spec findings above. ClawHub #3005 and RFC #39 are still open cross-repo dependencies.
|
@Patrick-Erichsen Both requested code changes are addressed at exact head fe939bc: configured clawhub-public profiles inherit built-in/env trust unless verification is explicitly overridden, and signed feeds now enforce expiresAt > generatedAt across fresh, 304, offline, and error paths. Expired or legacy no-expiry snapshots remain visible but have install authority removed; legacy snapshots still participate in rollback checks so upgrades do not deadlock. Focused catalog suite: 41 passed, 2 SQLite tests skipped locally because Node 24.14.0 is below the repo's WAL-safe 24.15.0 floor. Final codex review found no actionable issues. #108342 remains the separate rotation prerequisite. |
4edff30 to
e0c2e9d
Compare
|
Rebased onto current Scoped current-head proof:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
e0c2e9d to
8ef434f
Compare
|
Rebased again onto current Scoped current-head proof:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
8ef434f to
7378c93
Compare
|
Rebased onto current Scoped current-head proof:
Note: @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
7378c93 to
c689aa8
Compare
|
Rebased onto current Scoped current-head proof:
No code changes beyond the clean rebase. @clawsweeper re-review |
c689aa8 to
966e375
Compare
|
Rebased onto current Scoped current-head proof:
No code changes beyond the clean rebase. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
* feat: bind signed ClawHub feed trust * fix(feeds): preserve trust and enforce expiry * fix(feeds): enforce signed catalog HTTP contract * fix(feeds): align signed feed validation * fix: preserve hosted catalog trust boundaries * fix(feeds): stage custom feed identity binding * fix(feeds): require complete entry trust authority * fix(feeds): retain feed schema trust context * fix(feeds): enforce schema trust before overlays * fix(feeds): satisfy catalog trust CI checks --------- Co-authored-by: Gio Della-Libera <[email protected]>
Feed stack — OpenClaw trust foundation
Can review and merge now, in parallel with ClawHub #3005.
Required before: #110250 (sharded consumer) and production signing activation.
This PR defines and hardens the trust/identity/expiry contract but intentionally does not bundle a production key or activate the default signed feed. Key handoff and activation remain a separate proof-backed operation after the code stack lands.
Summary - bind the built-in
clawhub-publicprofile to payload identityclawhub-official, preserving built-in/environment trust when the profile is customized - keep existing custom signed profiles withoutfeedIdoperational during the staged rollout; new profiles can opt into identity binding now - support paired key-id/public-key environment overrides and fail closed before fetch when the pair is incomplete - enforce the signed DSSE HTTP contract, payload expiry, monotonic rollback/equivocation protection, and snapshot re-verification - keep expired or legacy no-expiry snapshots visible while stripping install authority - require schema-v2 entries to declare the completeavailable+officialinstall-authority pair before overlays or clones - document trusted-key distribution and the non-inventing Doctor migration requirement for the separately landing feed-profile config surface ## Dependencies - merged envelope compatibility policy: #110037 - merged signed snapshot key rotation: #108342 - ClawHub stored-catalog signer and key-management tooling: openclaw/clawhub#3005 - corrected hosted-feed v1 specs: openclaw/rfcs#39 ## Rollout policy The built-inclawhub-publicprofile is identity-bound immediately. Existing custom signed profiles that omitfeedIdretain signature verification without payload-identity binding during the transition. The separately landing user-facing profile configuration must diagnose the missing value and ask the operator to supply it; Doctor must not infer a feed identity from its URL. This PR intentionally does not invent or bundle a production key. ClawHub must generate and retain the Ed25519 private key in deployment secret storage and hand OpenClaw the matching public key and key ID. Bundling that trust root and activating the production default remain a separate proof-backed change. ## Validation -node scripts/run-vitest.mjs src/plugins/official-external-plugin-catalog-envelope.test.ts src/plugins/official-external-plugin-catalog.test.ts --run: 74 passed, including SQLite snapshot/rotation cases on Node 24.15 - changed-fileoxfmt, type-awareoxlint, andgit diff --check: passed -codex review --base upstream/mainat51552f0ee4f: no actionable correctness findings ## Real behavior proof Behavior addressed: A customized built-in profile could lose default trust; signed authority lacked a complete expiry and HTTP contract; legacy snapshots could interfere with rollback state; and schema-v2 manifest fallback could bypass entry-level install authority. Environment tested: WSL Ubuntu with Node 24.15, generated Ed25519 keys, HTTP response fixtures, in-memory snapshots, and the real SQLite snapshot store. Proof covered: 1. Built-in trust inheritance, explicit unsigned downgrade, incomplete-env failure, and cross-feed replay rejection. 2. Standard DSSE live responses, snapshot-only beta compatibility, ETag-based 304 re-verification, expiry, rollback, equivocation, and key rotation. 3. Expired and no-expiry snapshots remain visible but non-installable. 4. Legacy custom signed profiles withoutfeedIdwork through configured and explicit-URL paths. 5. Schema-v2 entries missing either or both authority fields lose install authority before catalog overlays. Not yet tested: The production ClawHub signer and bundled public trust root. Those do not exist in this branch and are intentionally reserved for the separate activation proof. ## Maintainer decision Approved to merge this as staged trust-binding enforcement before production ClawHub signing-key activation proof. This branch identity-binds the built-inclawhub-publicprofile, preserves the documented legacy custom signed-profile transition, and keeps the production public trust root plus signer activation as a separate proof-backed change. Before that later activation, ClawHub must provide the signing implementation/key handoff and OpenClaw must receive redacted production-equivalent refresh proof for the default path.