Skip to content

fix(app-store): verify manifest Store.Signature cryptographically (PILOT-98)#5

Merged
TeoSlayer merged 1 commit into
mainfrom
openclaw/pilot-98-20260528-103214
May 28, 2026
Merged

fix(app-store): verify manifest Store.Signature cryptographically (PILOT-98)#5
TeoSlayer merged 1 commit into
mainfrom
openclaw/pilot-98-20260528-103214

Conversation

@matthew-pilot
Copy link
Copy Markdown
Collaborator

What

manifest.Validate() checked Store.Signature only for non-empty — the ed25519 signature over (publisher||id||manifest_version||binary.sha256||grants-hash) was never cryptographically verified, making it a decorative no-op. An attacker who can write to an app dir could hand-edit grants and the daemon would accept the manifest as valid.

Fix

Adds manifest.VerifySignature() which:

  1. Decodes the ed25519 publisher key from Store.Publisher (formatted as "ed25519:<base64>")
  2. Recomputes the canonical signing payload (publisher || ":" || id || ":" || manifest_version || ":" || binary.sha256 || ":" || grants-sha256-hex)
  3. Calls crypto/ed25519.Verify against Store.Signature

The publisher key is included in the signing payload so swapping keys invalidates the signature. scanInstalled() now calls VerifySignature() after Validate(), rejecting manifests with invalid signatures at discovery time.

Files

  • pkg/manifest/manifest.go: +VerifySignature(), +signingPayload(), +canonicalJSON()
  • plugin/appstore/supervisor.go: +signature check in scanInstalled()
  • pkg/manifest/manifest_test.go: 3 new tests (valid sig, tampered grants, mismatched key)
  • plugin/appstore/testhelpers_test.go: test helper updated to produce valid signed manifests

Verification

go build ./...     # clean
go vet ./...       # clean
go test ./...      # all pass (5 packages)

Note

This does NOT check that Store.Publisher matches a trusted publisher key — that trust-anchor check is the natural follow-up hardening step. For now, this makes the signature field mean something (it must be a valid ed25519 signature over canonical manifest content), preventing unauthenticated tampering.

Closes PILOT-98

…LOT-98)

manifest.Validate() checked Store.Signature only for non-empty — the ed25519
signature over (publisher||id||manifest_version||binary.sha256||grants-hash)
was never cryptographically verified, making it a decorative no-op.

This adds manifest.VerifySignature() which decodes the ed25519 publisher
key from Store.Publisher, recomputes the signing payload, and calls
crypto/ed25519.Verify. The publisher key is included in the payload so
swapping keys invalidates the signature. A future trust-anchor check
(verifying Store.Publisher against a daemon-embedded key) will complete
the chain.

scanInstalled() now calls VerifySignature() after Validate(), rejecting
manifests with invalid signatures at discovery time.

Closes PILOT-98
@codecov
Copy link
Copy Markdown

codecov Bot commented May 28, 2026

Codecov Report

❌ Patch coverage is 65.71429% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/manifest/manifest.go 62.50% 6 Missing and 6 partials ⚠️

📢 Thoughts on this report? Let us know!

@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

🤖 Matthew PR Check — #5 PILOT-98

Status

  • State: OPEN · MERGEABLE · no merge conflicts
  • CI: test ✅ · snyk ✅ · codecov/patch ⚠️ (coverage threshold only)
  • Canary: not wired for app-store (harness supports rendezvous/web4/updater/common only)
  • Jira: PILOT-98 — QA/IN-REVIEW (assigned to Teodor Calin)
  • Last operator activity: N/A (newly opened PR)

Files changed: 4 (+197/−9)

pkg/manifest/manifest.go · plugin/appstore/supervisor.go · pkg/manifest/manifest_test.go · plugin/appstore/testhelpers_test.go

@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

🤖 Matthew — Code Walkthrough (#5, PILOT-98)

What this does

The manifest.Store.Signature field was previously checked only for non-empty — the ed25519 signature over (publisher||id||version||binary.sha256||grants-hash) was never cryptographically verified. This PR makes it real.

File-by-file

pkg/manifest/manifest.go (+69)

  • L171–174: canonicalJSON() — deterministic JSON marshaling for the grants payload.
  • L184–196: signingPayload() — builds the canonical signing input: publisher:id:manifest_version:binary_sha256:grants_sha256_hex. The publisher key is embedded so swapping keys invalidates the signature.
  • L210–238: VerifySignature() — strips the "ed25519:" prefix from Store.Publisher, decodes the base64 pubkey, decodes the signature, recomputes signingPayload(), and calls crypto/ed25519.Verify. Returns descriptive errors for: missing prefix, invalid base64, wrong key length, wrong sig length, and verification failure.

pkg/manifest/manifest_test.go (+69)

  • L324–337: signTestManifest() helper for the test suite.
  • L339–358: TestVerifySignatureRejectsModifiedManifest — valid sig passes; tampering Grants[0].Cap = "fs.delete" causes rejection.
  • L360–379: TestVerifySignatureRejectsWrongKey — preserving sig but swapping publisher key causes rejection.
  • L381–388: TestVerifySignatureRejectsEmptySignature — empty signature rejected.

plugin/appstore/supervisor.go (+6)

  • L243–247: scanInstalled() now calls m.VerifySignature() immediately after m.Validate(). Manifests with invalid signatures are logged and skipped at discovery time — they never reach the spawn path.

plugin/appstore/testhelpers_test.go (+56/−5)

  • writeValidAppDir() now generates a fresh ed25519 keypair per test app, signs the manifest with the canonical payload, and writes it out — so downstream tests exercise the real signature path.

What this does NOT do (yet)

This does NOT verify that Store.Publisher matches a trusted publisher key. That trust-anchor check is the follow-up hardening step. For now, this makes the signature field meaningful: it MUST be a valid ed25519 signature over canonical manifest content, preventing unauthenticated tampering of manifests.

TeoSlayer pushed a commit that referenced this pull request May 28, 2026
…5) (#3)

The install flow had no monotonic version check — re-installing an
older app_version on top of a newer one succeeded silently. This is a
classic rollback-attack vector: a CVE patched in v1.2 can be
re-introduced by reinstalling v1.1.

This adds:
- compareVersions() — basic semver comparison (MAJOR.MINOR.PATCH[-PRE])
- registerInstalled() now refuses to replace an in-memory entry with
  a lower app_version for the same app ID (startup path).
- rescanForNew() now detects on-disk manifest version changes for
  already-known apps and refuses downgrades; upgrades are accepted
  and trigger a clean restart of the supervise goroutine.
- Audit events (downgrade-refused) in the supervisor log for forensics.

The check is best-effort defense-in-depth — full protection requires
signed manifests (PILOT-98, the #5 dependency) to prevent an attacker
from forging the app_version field alongside the binary.

Closes PILOT-105

Co-authored-by: matthew-pilot <matthew-pilot@vulturelabs.io>
@TeoSlayer TeoSlayer merged commit 12f2eed into main May 28, 2026
2 of 3 checks passed
@TeoSlayer TeoSlayer deleted the openclaw/pilot-98-20260528-103214 branch May 28, 2026 17:56
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