Skip to content

fix(marketplaces): allow trusted feed key rotation#108342

Merged
giodl73-repo merged 3 commits into
openclaw:mainfrom
giodl73-repo:fix/hosted-feed-trust-rotation-snapshot
Jul 23, 2026
Merged

fix(marketplaces): allow trusted feed key rotation#108342
giodl73-repo merged 3 commits into
openclaw:mainfrom
giodl73-repo:fix/hosted-feed-trust-rotation-snapshot

Conversation

@giodl73-repo

@giodl73-repo giodl73-repo commented Jul 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary - compare newly verified signed feeds against persisted accepted monotonic metadata - avoid requiring an old snapshot to verify under a newly rotated key merely to perform rollback comparison - retain fail-closed fallback behavior when the old snapshot does not satisfy the current trust policy - restore persisted monotonic metadata when signed snapshots are loaded from SQLite - cover successful key rotation, lower-sequence rejection, and retained snapshot fallback through the durable store - preserve merged #110037 policy: live feeds require standard DSSE while persisted beta snapshots remain readable ## Real behavior proof Behavior or issue addressed: A newly valid higher-sequence hosted feed could be rejected after trusted signing keys rotated because OpenClaw reverified the old snapshot with the new key before comparing sequences. The durable proof also exposed that SQLite stored accepted monotonic metadata but dropped it when reconstructing a snapshot. Real environment tested: Windows checkout using Node 24.15.0 and SQLite 3.51.3. The production hosted-catalog loader and real SQLite snapshot store were exercised with a temporary state database and generated Ed25519 Q2/Q3/Q4 key pairs. Network responses used the existing guarded-fetch harness so the signed bytes and trust transitions were deterministic. Exact steps or command run after this patch: powershell & "$env:LOCALAPPDATA\pnpm\store\v11\links\@\node\24.15.0\4b86559dec5f3e7def9c585f1a41d64f741f7930cf3b31f21bf2b4e73beef981\node_modules\node\bin\node.exe" scripts/run-vitest.mjs src/plugins/official-external-plugin-catalog.test.ts --run The durable regression: 1. Opens the real SQLite marketplace state store. 2. Accepts sequence 8 signed by acme-root-2026-q2. 3. Reopens the persisted snapshot, replaces configured trust with acme-root-2026-q3, and accepts sequence 9. 4. Replaces configured trust with acme-root-2026-q4 and rejects sequence 7. 5. Reloads the retained sequence 9 snapshot under the Q3 policy. Evidence after fix: The full catalog suite reported 46 tests passed. The key-rotation case persisted each accepted revision through SQLite and verified the retained snapshot after the rollback attempt. Observed result after fix: Sequence 9 was accepted under the rotated Q3 key. Sequence 7 under Q4 failed closed with the rollback diagnostic. Sequence 9 remained persisted and readable under its accepted Q3 policy. Persisted signed snapshots now reconstruct their accepted monotonic record from the authenticated body; unsigned rows cannot acquire that authority. What was not tested: A production publisher endpoint or production key-management system was not used. The proof intentionally uses generated keys and deterministic guarded network responses while exercising the real production loader and durable SQLite implementation. ## Validation - full hosted catalog suite on Node 24.15.0 / SQLite 3.51.3: 50 passed - targeted oxfmt --check - targeted type-aware oxlint - git diff --check - final codex review --base upstream/main: no actionable correctness issues

Maintainer decision

Approved for ordinary hosted-feed key rotation. Previously accepted signed-feed monotonic metadata may remain rollback-authoritative across routine configured-key rotation, while every newly fetched feed must verify under the current trust set. Emergency key compromise or revocation is a separate operator recovery path that clears or rebuilds local hosted-feed snapshot state.

@openclaw-barnacle openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels Jul 15, 2026
@giodl73-repo giodl73-repo mentioned this pull request Jul 15, 2026
Merged
@giodl73-repo
giodl73-repo force-pushed the fix/hosted-feed-trust-rotation-snapshot branch from 49292a4 to 4aa135f Compare July 15, 2026 15:02
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels Jul 16, 2026
@clawsweeper

clawsweeper Bot commented Jul 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 20, 2026, 5:15 PM ET / 21:15 UTC.

Summary
The branch compares newly verified signed marketplace feeds with persisted monotonic metadata across key rotation, restores that metadata from SQLite snapshots, and rejects same-sequence payload changes.

PR surface: Source +39, Tests +120. Total +159 across 3 files.

Reproducibility: yes. for the reported regression: the submitted real SQLite scenario creates Q2, Q3, and Q4 key transitions and demonstrates acceptance, rollback rejection, and retained-snapshot reload. I could not independently execute that path in the read-only command sandbox.

Review metrics: 1 noteworthy metric.

  • Persisted monotonic identity: 1 persisted signed-feed record extended with a SHA-256 payload identity. This is the security-relevant state change that distinguishes safe same-sequence re-signing from conflicting same-sequence content.

Stored data model
Persistent data-model change detected: serialized state: src/plugins/official-external-plugin-catalog-snapshot-store.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: canonical
Canonical: #108342
Summary: This PR is the narrow signed-snapshot key-rotation dependency for the broader, still-open feed-trust proposal.

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:

  • Obtain maintainer confirmation of the retained-metadata versus emergency-revocation policy.
  • Refresh the review against the current merge result before landing.

Risk before merge

  • [P1] A snapshot authenticated under a formerly trusted key can continue to reject a lower-sequence feed after that key is removed from current trust; maintainers need to explicitly decide whether that retained anti-rollback authority is acceptable after revocation.
  • [P1] The branch is mergeable but behind main; refresh the security review against the merge result before landing because this trust path is security-sensitive.

Maintainer options:

  1. Confirm and document the revocation boundary (recommended)
    Before merge, have the responsible security owner confirm that retained historic metadata is allowed for anti-rollback after routine rotation and define the explicit emergency invalidation path.
  2. Adopt a continuity-based design
    Require the rotated trust set to authorize prior monotonic state before it can influence rollback decisions, trading simpler recovery for stricter revocation semantics.
  3. Pause until the feed specification resolves revocation
    Do not merge this behavioral policy independently if hosted-feed revocation rules remain undecided.

Next step before merge

  • [P2] A maintainer must select the hosted-feed key-revocation policy; automation cannot safely choose whether retained prior-key metadata should remain rollback-authoritative.

Maintainer decision needed

  • Question: Should authenticated monotonic metadata from a snapshot signed by a now-removed key remain authoritative solely to reject lower-sequence feeds after ordinary key rotation, and what explicit revocation path must clear that authority?
  • Rationale: The code can correctly implement either policy, but only maintainers can define the intended security boundary between anti-rollback continuity and emergency key revocation.
  • Likely owner: giodl73-repo — They are the identifiable contributor carrying both this narrowly scoped rotation fix and the related hosted-feed trust work; a repository security owner should make the final policy call.
  • Options:
    • Approve retained rollback metadata (recommended): Document that ordinary key rotation preserves previously authenticated sequence metadata for rollback checks, with a named operator recovery path for emergency key revocation.
    • Require key continuity: Reject the design unless the new trusted key set cryptographically authorizes the retained metadata from the prior key.
    • Pause pending trust specification: Hold this PR until the hosted-feed specification defines revocation, recovery, and rollback semantics.

Security
Needs attention: No line-level vulnerability is established, but the PR changes the security authority of persisted signed-feed metadata after key removal and requires an explicit maintainer policy decision.

Review details

Best possible solution:

Document and approve a narrow policy that retained metadata from a previously accepted signed feed may enforce rollback protection after ordinary key rotation, while defining the explicit revocation or emergency-recovery path that can invalidate that metadata.

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

Yes for the reported regression: the submitted real SQLite scenario creates Q2, Q3, and Q4 key transitions and demonstrates acceptance, rollback rejection, and retained-snapshot reload. I could not independently execute that path in the read-only command sandbox.

Is this the best way to solve the issue?

Unclear until a maintainer confirms the revocation policy. Using authenticated persisted monotonic state is a focused solution to ordinary key rotation, but it must not silently define emergency key-revocation behavior.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 21bfc75648d1.

Label changes

Label justifications:

  • P2: This is a bounded marketplace trust hardening change with meaningful but non-emergency user and operator impact.
  • merge-risk: 🚨 compatibility: Persisted hosted-catalog snapshot semantics change across upgrades and can alter whether existing cached feeds are accepted or rejected.
  • merge-risk: 🚨 security-boundary: The patch changes which previously authenticated trust state remains authoritative after configured signing keys rotate or are revoked.
  • 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 (terminal): The PR provides a concrete after-fix Windows run against the production loader and real SQLite store, with observable Q2→Q3 rotation success, Q4 rollback rejection, retained-snapshot reload, and a reported 46 passing catalog tests.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR provides a concrete after-fix Windows run against the production loader and real SQLite store, with observable Q2→Q3 rotation success, Q4 rollback rejection, retained-snapshot reload, and a reported 46 passing catalog tests.
Evidence reviewed

PR surface:

Source +39, Tests +120. Total +159 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 2 54 15 +39
Tests 1 123 3 +120
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 177 18 +159

Security concerns:

  • [medium] Define revocation authority for retained metadata — src/plugins/official-external-plugin-catalog.ts:1051
    The new rollback check can use metadata authenticated under a formerly trusted key without revalidating the old snapshot under the current trust set. That is appropriate for ordinary rotation only if maintainers explicitly define how emergency revocation invalidates that retained authority.
    Confidence: 0.93

What I checked:

  • Durable real-behavior proof: The PR body documents a Windows Node 24.15.0 / SQLite 3.51.3 run that exercised the production hosted-catalog loader and SQLite snapshot store through Q2→Q3 key rotation, a Q4 rollback rejection, and retained-snapshot reload; the timeline records the proof: sufficient label. (src/plugins/official-external-plugin-catalog.test.ts:534, f9220abc1de9)
  • Security decision remains unresolved: The prior completed ClawSweeper review at commit 6f45b915dc3d2d3ae990cb52f0ce7c7a5207408d explicitly required a maintainer choice on key revocation and retained rollback metadata; the supplied later commits add proof and same-sequence equivocation coverage but do not establish that policy choice. (src/plugins/official-external-plugin-catalog.ts:1051, f9220abc1de9)
  • Persisted anti-equivocation state: The branch adds a SHA-256 identity to persisted signed-feed monotonic state, so a different payload cannot reuse an already accepted sequence while a re-signature of the same payload can proceed. (src/plugins/official-external-plugin-catalog-snapshot-store.ts:52, f9220abc1de9)
  • Protected maintainer routing: The PR carries the maintainer label, so this cleanup workflow must not auto-close it; its merge-risk: 🚨 security-boundary label matches the unresolved revocation semantics.

Likely related people:

  • giodl73-repo: Authored the current two-commit signed-feed rotation/equivocation implementation and the linked broader trust-profile PR that declares this change as a dependency. (role: related trust-work contributor; confidence: medium; commits: 651b415cad55, f9220abc1de9; files: src/plugins/official-external-plugin-catalog.ts, src/plugins/official-external-plugin-catalog-snapshot-store.ts, src/plugins/official-external-plugin-catalog.test.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 (3 earlier review cycles)
  • reviewed 2026-07-16T03:35:23.475Z sha 4aa135f :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-18T22:07:25.314Z sha 6286335 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-18T22:20:33.956Z sha 6f45b91 :: needs maintainer review before merge. :: none

@giodl73-repo giodl73-repo mentioned this pull request Jul 18, 2026
Open
@giodl73-repo
giodl73-repo force-pushed the fix/hosted-feed-trust-rotation-snapshot branch from 4aa135f to 607890c Compare July 18, 2026 21:56
@giodl73-repo

giodl73-repo commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

Durable proof is now on the current head. The regression uses the real SQLite snapshot store under Node 24.15.0 / SQLite 3.51.3: Q2 sequence 8 persists, Q3 sequence 9 survives key rotation, Q4 sequence 7 is rejected, and the Q3 snapshot remains readable. This run exposed and fixed a real read-path defect where accepted monotonic metadata was written but not reconstructed from SQLite. Full catalog suite: 37/37. @clawsweeper re-review

@giodl73-repo
giodl73-repo force-pushed the fix/hosted-feed-trust-rotation-snapshot branch from 607890c to 6286335 Compare July 18, 2026 22:03
@giodl73-repo

giodl73-repo commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto upstream 4d683904dfe and revalidated on exact head 62863355ad3. The merge preserves upstream malformed-timestamp repair and fail-closed signature/trust handling while using accepted monotonic metadata for key rotation. The durable SQLite decoder now restores the required signed-feed discriminator. Full catalog suite: 46/46; format, type-aware lint, and diff checks pass. @clawsweeper re-review

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 18, 2026
@giodl73-repo
giodl73-repo force-pushed the fix/hosted-feed-trust-rotation-snapshot branch from 6286335 to 6f45b91 Compare July 18, 2026 22:16
@giodl73-repo

giodl73-repo commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

CI diagnostics on 62863355ad3 all reduced to one exact-optional-property type error. Exact head 6f45b915dc3 now grants the signed-feed discriminator only when the authenticated persisted record has both a valid sequence and generatedAt; malformed timestamp rows retain the fail-closed repair path. Durable catalog suite remains 46/46; format, type-aware lint, and diff checks pass. @clawsweeper re-review

@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 18, 2026
@giodl73-repo
giodl73-repo force-pushed the fix/hosted-feed-trust-rotation-snapshot branch from 6f45b91 to 651b415 Compare July 20, 2026 17:29
@giodl73-repo

giodl73-repo commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Restacked onto upstream c715d89de81; exact head is 651b415cad5. The validated staged tree was hash-matched to this Windows commit, then Node 24.15.0 passed all 46 catalog tests including the real SQLite Q2/Q3/Q4 rotation regression. Scoped oxfmt --check, type-aware oxlint, and git diff --check also pass. No CI jobs were manually queued.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 20, 2026 •
edited
Loading

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.

Re-review progress:

@clawsweeper clawsweeper Bot added status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jul 21, 2026
@giodl73-repo
giodl73-repo force-pushed the fix/hosted-feed-trust-rotation-snapshot branch from 5620aed to 30f0642 Compare July 23, 2026 21:08
@giodl73-repo
giodl73-repo merged commit bd48b7b into openclaw:main Jul 23, 2026
130 of 131 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Patrick-Erichsen Patrick-Erichsen Awaiting requested review from Patrick-Erichsen

Assignees

No one assigned

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. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@giodl73-repo @Patrick-Erichsen