Skip to content

Verify signed marketplace feed refresh#98338

Merged
giodl73-repo merged 5 commits into
openclaw:mainfrom
giodl73-repo:feeds-trust-refresh-state
Jul 9, 2026
Merged

Verify signed marketplace feed refresh#98338
giodl73-repo merged 5 commits into
openclaw:mainfrom
giodl73-repo:feeds-trust-refresh-state

Conversation

@giodl73-repo

@giodl73-repo giodl73-repo commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Wire signed marketplace feed profiles into hosted catalog refresh using the signed envelope verifier and configured threshold.
  • Persist bounded signed-feed trust state with hosted snapshots, and re-verify signed snapshots with the current source profile before using them.
  • Count signed-feed thresholds over unique normalized Ed25519 public-key material so duplicate key IDs or duplicate key material cannot weaken a multi-signature threshold.
  • Keep signed profiles fail-closed on invalid live bodies or unsigned old snapshots, without changing install eligibility or the existing marketplace refresh/entries payload shape.

Validation

> node scripts/run-vitest.mjs src/config/zod-schema.marketplaces.test.ts src/plugins/official-external-plugin-catalog.test.ts src/plugins/official-external-plugin-catalog-envelope.test.ts src/state/openclaw-state-db.test.ts --reporter=dot
Test Files 4 passed (4)
Tests 106 passed (106)
[test] passed 4 Vitest shards

> npx oxfmt --check <touched source/test files>
All matched files use the correct format.

> npx oxlint <touched source/test files>
Found 0 warnings and 0 errors.

> git diff --check origin/main...HEAD && git diff --check
PASS

> pnpm tsgo:core
PASS

> node scripts/plugin-sdk-surface-report.mjs --check
PASS

> pnpm db:kysely:check
PASS

@openclaw-barnacle openclaw-barnacle Bot added size: XL maintainer Maintainer-authored PR labels Jul 1, 2026
@clawsweeper

clawsweeper Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 9, 2026, 1:41 PM ET / 17:41 UTC.

Summary
The branch wires signed marketplace feed verification into hosted refresh, persists signed snapshot trust metadata in SQLite, and rejects signed feed rollbacks before replacing snapshots.

PR surface: Source +315, Tests +571, Generated +10, Other 0. Total +896 across 11 files.

Reproducibility: not applicable. as a bug repro, but the changed behavior is source-reproducible: the exact-head harness and focused tests exercise signed refresh, trust persistence, offline signed snapshot reverify, and rollback rejection.

Review metrics: 2 noteworthy metrics.

  • Stored snapshot trust schema: 5 nullable columns added. The PR changes shared SQLite state, so maintainers should notice the additive upgrade path before merge.
  • Signed refresh trust surface: 2 load-result variants extended. Both hosted and hosted-snapshot results can now carry signed trust state, which affects downstream CLI and diagnostics consumers.

Stored data model
Persistent data-model change detected: database schema: src/state/openclaw-state-db.test.ts, serialized state: src/plugins/official-external-plugin-catalog-snapshot-store.ts, serialized state: src/state/openclaw-state-db.generated.d.ts, serialized state: src/state/openclaw-state-db.test.ts. Migration or upgrade compatibility proof is recorded; maintainers should verify it before merge.

Root-cause cluster
Relationship: canonical
Canonical: #98338
Summary: This PR is the canonical refresh/snapshot enforcement slice in a staged signed marketplace feed trust stack; adjacent PRs provide verifier, config, CLI visibility, and default-root follow-ups.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Risk before merge

  • [P1] This PR changes a security boundary: a verifier or rollback mistake could cause OpenClaw to trust the wrong hosted marketplace feed despite green CI.
  • [P1] The PR adds five nullable columns to shared SQLite state; the additive migration and tests reduce upgrade risk, but maintainers still own the stored-state compatibility decision.
  • [P1] The branch is marked behind the moving base, so final merge should use the actual three-way result and exact-head checks rather than older restack proof alone.

Maintainer options:

  1. Merge with accepted trust-state upgrade (recommended)
    Proceed after current-head checks if maintainers accept the additive SQLite columns and signed-profile fail-closed behavior already called out in the PR discussion.
  2. Require one more upgrade/security signoff
    Pause until a maintainer explicitly confirms the current head after the final restack still matches the accepted stored-state and trust-boundary behavior.
  3. Defer to the broader trust stack
    Keep this PR open or close/rework it only if maintainers want the refresh, visibility, and default-root trust slices redesigned together.

Next step before merge

  • [P2] No repair lane is needed; the current head has no supported findings, but the protected maintainer label and signed-feed trust boundary leave merge sequencing to maintainer review.

Maintainer decision needed

  • Question: Should this signed marketplace refresh slice merge now with the accepted fail-closed signed-profile behavior and additive SQLite trust-state columns, or wait for broader trust-stack sequencing?
  • Rationale: The code review can verify the implementation path, but the permanent trust boundary and stored-state upgrade policy are maintainer/security intent choices for the marketplace trust stack.
  • Likely owner: Patrick-Erichsen — Patrick is the current assignee and supplied the substantive review, CI repair, and acceptance context for this exact trust-stack slice.
  • Options:
    • Merge this signed-refresh slice (recommended): Land after required exact-head checks because the verifier/config prerequisites are already merged, prior findings are addressed, and maintainer discussion accepts the fail-closed and additive SQLite behavior.
    • Hold for stack sequencing: Pause if maintainers want refresh enforcement, CLI trust output, and default ClawHub root support reviewed or landed as a coordinated trust-boundary batch.
    • Narrow before merge: Request a smaller follow-up only if maintainers decide the SQLite trust state or fail-closed policy should change before this slice becomes part of main.

Security
Cleared: No concrete security or supply-chain defect was found; the diff uses the existing Ed25519 helper, preserves the SSRF-guarded fetch path, fails closed for signed profiles, and persists bounded trust metadata without secrets.

Review details

Best possible solution:

Land this slice only after maintainer security-boundary acceptance on the current head, keeping CLI trust-state display and default ClawHub trust-root work in their separate stacked PRs.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a bug repro, but the changed behavior is source-reproducible: the exact-head harness and focused tests exercise signed refresh, trust persistence, offline signed snapshot reverify, and rollback rejection.

Is this the best way to solve the issue?

Yes, this is the best bounded slice: it consumes the already-merged verifier/config primitives in the catalog refresh owner, persists only bounded trust metadata, and leaves CLI visibility/default-root policy to stacked follow-ups.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 745b8c315eaa.

Label changes

Label changes:

  • add merge-risk: 🚨 compatibility: The PR adds nullable columns to shared state and extends hosted catalog result shapes, so upgrade compatibility depends on the additive migration and consumer handling.
  • add merge-risk: 🚨 security-boundary: The PR decides when signed hosted marketplace feeds and snapshots are trusted or rejected, which is a security boundary green CI cannot fully settle.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR discussion includes exact-head terminal/source-harness output for 868375b showing signed refresh, SQLite trust persistence, offline signed snapshot reverify, and rollback fallback, plus current-head CI success.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (logs): The PR discussion includes exact-head terminal/source-harness output for 868375b showing signed refresh, SQLite trust persistence, offline signed snapshot reverify, and rollback fallback, plus current-head CI success.

Label justifications:

  • P2: This is a normal-priority marketplace trust hardening feature with limited blast radius and no current blocking correctness finding.
  • merge-risk: 🚨 compatibility: The PR adds nullable columns to shared state and extends hosted catalog result shapes, so upgrade compatibility depends on the additive migration and consumer handling.
  • merge-risk: 🚨 security-boundary: The PR decides when signed hosted marketplace feeds and snapshots are trusted or rejected, which is a security boundary green CI cannot fully settle.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (logs): The PR discussion includes exact-head terminal/source-harness output for 868375b showing signed refresh, SQLite trust persistence, offline signed snapshot reverify, and rollback fallback, plus current-head CI success.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR discussion includes exact-head terminal/source-harness output for 868375b showing signed refresh, SQLite trust persistence, offline signed snapshot reverify, and rollback fallback, plus current-head CI success.
Evidence reviewed

PR surface:

Source +315, Tests +571, Generated +10, Other 0. Total +896 across 11 files.

View PR surface stats
Area Files Added Removed Net
Source 5 345 30 +315
Tests 3 593 22 +571
Docs 0 0 0 0
Config 0 0 0 0
Generated 2 10 0 +10
Other 1 2 2 0
Total 11 950 54 +896

What I checked:

Likely related people:

  • Patrick-Erichsen: Assigned on the live PR, authored a mechanical CI repair commit on this branch, and provided the substantive review/acceptance context for the signed trust-state behavior. (role: reviewer and current assignee; confidence: high; commits: 9375841fe983; files: src/plugins/official-external-plugin-catalog.ts, src/plugins/official-external-plugin-catalog.test.ts, scripts/plugin-sdk-surface-report.mjs)
  • giodl73-repo: Authored this branch and the merged hosted-feed verifier/config prerequisites that current main now carries for the signed marketplace feed trust stack. (role: feature owner and recent area contributor; confidence: high; commits: 4b91ce76acd0, e822d4dfd408, 868375bdd227; files: src/plugins/official-external-plugin-catalog.ts, src/plugins/official-external-plugin-catalog-envelope.ts, src/plugins/official-external-plugin-catalog-snapshot-store.ts)
  • Peter Steinberger: Available git history/blame in this checkout shows the current catalog files carried through a recent broad main commit authored by Peter, making him useful adjacent context though not the signed-feed feature owner. (role: recent area contributor; confidence: medium; commits: f8dbd34ab8e8; files: src/plugins/official-external-plugin-catalog.ts, src/plugins/official-external-plugin-catalog-envelope.ts, src/plugins/official-external-plugin-catalog-snapshot-store.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (24 earlier review cycles; latest 8 shown)
  • reviewed 2026-07-09T15:59:27.858Z sha 609e766 :: needs real behavior proof before merge. :: [P1] Make signed snapshot replacement atomic
  • reviewed 2026-07-09T16:01:49.439Z sha 609e766 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-09T16:20:56.552Z sha 609e766 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-09T16:41:05.426Z sha d2d8ab1 :: needs changes before merge. :: [P1] Make snapshot replacement atomic
  • reviewed 2026-07-09T16:49:29.613Z sha d2d8ab1 :: needs changes before merge. :: [P1] Make signed snapshot replacement atomic
  • reviewed 2026-07-09T17:07:28.324Z sha 6423fda :: needs maintainer review before merge. :: none
  • reviewed 2026-07-09T17:10:42.488Z sha 6423fda :: needs maintainer review before merge. :: none
  • reviewed 2026-07-09T17:24:16.159Z sha 3b6d07c :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels Jul 1, 2026
@giodl73-repo
giodl73-repo force-pushed the feeds-trust-refresh-state branch from 27be652 to bf3a3a7 Compare July 1, 2026 03:55
@giodl73-repo

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Applied the duplicate trust-anchor fix: config rejects duplicate key IDs/material and the verifier now counts thresholds over unique normalized public-key material. Added envelope regression coverage for same key material under two key IDs with threshold 2, reran focused tests, tsgo, Kysely, lint/format, and updated the PR body.

@clawsweeper

clawsweeper Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jul 1, 2026
@giodl73-repo
giodl73-repo force-pushed the feeds-trust-refresh-state branch from bf3a3a7 to 3ce0cfa Compare July 1, 2026 04:10
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 1, 2026
@giodl73-repo

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Final head is 3ce0cfa, rebased on source-profile 16cbe67. The stack now rejects malformed raw base64url keys, rejects duplicate trust anchors, and counts thresholds over unique normalized public-key material. Focused tests, tsgo, Kysely, lint/format, and Codex review are clean; PR body updated.

@clawsweeper

clawsweeper Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@giodl73-repo

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

The current review already marks proof sufficient, but the label still says waiting on author. Please refresh against head 3ce0cfafaf; the remaining decision should be maintainer/security acceptance and predecessor ordering, not contributor proof work.

@giodl73-repo
giodl73-repo marked this pull request as ready for review July 7, 2026 21:37
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. label Jul 7, 2026
@giodl73-repo
giodl73-repo force-pushed the feeds-trust-refresh-state branch 2 times, most recently from 308bf3d to 01e1f22 Compare July 7, 2026 21:51
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 7, 2026
@Patrick-Erichsen Patrick-Erichsen self-assigned this Jul 7, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 9, 2026
@giodl73-repo

Copy link
Copy Markdown
Contributor Author

Exact-head update for 6423fda41b51892fc8e1031bda2ae2586e8508c1 after the atomic signed snapshot replacement fix.

Validation run in /root/src/openclaw-workflow/.worktrees/pr-98338:

  • node scripts/run-vitest.mjs src/plugins/official-external-plugin-catalog.test.ts --reporter=dot passed: 60 tests.
  • node scripts/run-vitest.mjs src/config/zod-schema.marketplaces.test.ts src/plugins/official-external-plugin-catalog-envelope.test.ts src/plugins/official-external-plugin-catalog.test.ts src/state/openclaw-state-db.test.ts --reporter=dot passed: 107 tests.
  • npx oxfmt --check on changed files passed.
  • npx oxlint on changed files passed.
  • git diff --check origin/main...HEAD && git diff --check passed.
  • pnpm tsgo:core passed.
  • pnpm db:kysely:check passed.
  • node scripts/plugin-sdk-surface-report.mjs --check passed.
  • pnpm build passed.

Additional regression for the ClawSweeper P1: keeps signed SQLite snapshot replacement monotonic under concurrent writes now covers concurrent signed snapshot writes so the lower signed sequence cannot win the final SQLite row.

Source-harness proof on exact head:

SIGNED_FEED_PROOF_OK exactHead=6423fda41b51892fc8e1031bda2ae2586e8508c1
hosted=hosted trust=signed sequence=10
offline=hosted-snapshot trust=signed sequence=10
rollback=hosted-snapshot trust=signed sequence=10

This preserves the intended behavior: a verified signed hosted refresh persists signed trust state, offline mode re-verifies the signed snapshot, and a signed rollback attempt falls back to the newer signed snapshot without replacing it.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jul 9, 2026
@giodl73-repo
giodl73-repo force-pushed the feeds-trust-refresh-state branch from 6423fda to 3b6d07c Compare July 9, 2026 17:16
@giodl73-repo

Copy link
Copy Markdown
Contributor Author

Restacked exact-head update for 3b6d07c853f7afba88dec957e866a5249a2fefd6 after origin/main advanced with merge-critical workflow drift.

The rebase onto current origin/main was clean and did not change the PR-owned diff. Local restack validation in /root/src/openclaw-workflow/.worktrees/pr-98338:

  • node scripts/run-vitest.mjs src/plugins/official-external-plugin-catalog.test.ts --reporter=dot passed: 60 tests.
  • git diff --check origin/main...HEAD && git diff --check passed.
  • npx oxfmt --check src/plugins/official-external-plugin-catalog.ts src/plugins/official-external-plugin-catalog-snapshot-store.ts src/plugins/official-external-plugin-catalog.test.ts passed.
  • npx oxlint src/plugins/official-external-plugin-catalog.ts src/plugins/official-external-plugin-catalog-snapshot-store.ts src/plugins/official-external-plugin-catalog.test.ts passed.

Source-harness proof on the new exact head:

SIGNED_FEED_PROOF_OK exactHead=3b6d07c853f7afba88dec957e866a5249a2fefd6
hosted=hosted trust=signed sequence=10
offline=hosted-snapshot trust=signed sequence=10
rollback=hosted-snapshot trust=signed sequence=10

The previous broader validation remains applicable to the same PR-owned diff, and GitHub exact-head CI is running again for this restacked head.

@giodl73-repo
giodl73-repo force-pushed the feeds-trust-refresh-state branch from 3b6d07c to 868375b Compare July 9, 2026 17:29
@giodl73-repo

Copy link
Copy Markdown
Contributor Author

Second restacked exact-head update for 868375bdd2276dac379e7d715ecb4e8a72f06d23 after origin/main advanced again with merge-critical pnpm-lock.yaml drift.

The rebase onto current origin/main was clean and did not change the PR-owned diff. Local restack validation:

  • node scripts/run-vitest.mjs src/plugins/official-external-plugin-catalog.test.ts --reporter=dot passed: 60 tests.
  • git diff --check origin/main...HEAD && git diff --check passed.
  • Targeted npx oxfmt --check passed on the three touched catalog files.
  • Targeted npx oxlint passed on the three touched catalog files.

Source-harness proof on the new exact head:

SIGNED_FEED_PROOF_OK exactHead=868375bdd2276dac379e7d715ecb4e8a72f06d23
hosted=hosted trust=signed sequence=10
offline=hosted-snapshot trust=signed sequence=10
rollback=hosted-snapshot trust=signed sequence=10

GitHub exact-head CI is running again for this restacked head.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 9, 2026
@giodl73-repo

Copy link
Copy Markdown
Contributor Author

Merged via squash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. scripts Repository scripts size: XL status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants