Skip to content

feat(local-harness): trust the runtime pack release key - #4635

Merged
chelojimenez merged 2 commits into
mainfrom
feat/pack-signing-key
Sep 3, 2026
Merged

feat(local-harness): trust the runtime pack release key#4635
chelojimenez merged 2 commits into
mainfrom
feat/pack-signing-key

Conversation

@chelojimenez

@chelojimenez chelojimenez commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Draft until LOCAL_HARNESS_PACK_SIGNING_KEY is set — see Before merging. The key material is generated but not yet installed anywhere.

Why

PACK_SIGNING_KEYS shipped empty. That is a refusal, not a bypass — verifyPackManifestSignature turns away every network-sourced pack while the list is empty — and it was the right default before a key existed.

It is also why four of five legs of build-local-harness-pack failed on the 2026-09-02 release: the job refuses to publish packs no Inspector would install, and the secret did not exist. docs/local-harness.md already calls this out as "a hard blocker for release"; this is that checklist item.

What

Ed25519 key generated. Public half committed here, private half only in the CI secret — no copy anywhere else, which makes recovery rotation rather than restore (the module's rotation model is additive, so that is the intended path).

The tests move with it:

  • "refuses everything while no key is configured" was asserting PACK_SIGNING_KEYS is []. That pinned the repository's state rather than the function's property, so it would have to be deleted the moment a key landed. It now passes an explicit empty list and keeps testing the refusal.
  • Two new tests cover what committing a key actually risks. A PEM is a string until something parses it, and verifyPackManifestSignature swallows a malformed key on purpose so one bad entry cannot shadow a good one — so a typo would survive the build, the typecheck and every existing test, and surface only when a user's install reported "not signed by any key this Inspector trusts". One parses every committed key and asserts Ed25519; the other refuses duplicate key ids, since the id is how a refusal names the key that turned a pack away.

Verification

pack-signing-key.test.ts — 9 passed. Whole local/__tests__ directory — 518 passed, 29 skipped; runtime-install.test.ts fails on a tar helper, which reproduces identically on clean main (macOS bsdtar) and is unrelated.

Out of band, against the real production path: a manifest signed with the private half exactly as build-local-harness-pack.mjs signs it verifies against the committed public half with keyId: "pack-2026-09", and a manifest altered after signing is refused as bad-signature.

Before merging

The private half has to reach CI, or the release build still fails its "a release must have the pack signing key" gate:

gh secret set LOCAL_HARNESS_PACK_SIGNING_KEY -R MCPJam/inspector < <path to the generated private PEM>

If you would rather generate the key yourself, say so and this PR gets the public half of yours instead — nothing here depends on which key it is.

🤖 Generated with Claude Code

https://claude.ai/code/session_011qNR584viu78RCTK7obRVU


Summary by cubic

Trusts the local-harness runtime pack release key so network-sourced packs can be verified and published. PACK_SIGNING_KEYS shipped empty, which made verifyPackManifestSignature refuse every pack and blocked the release job; the public half of the Ed25519 release key is now committed, and the private half exists only in the CI secret LOCAL_HARNESS_PACK_SIGNING_KEY, so recovery is rotation rather than restore.

Before merging

  • Set LOCAL_HARNESS_PACK_SIGNING_KEY in CI; without it, the release build still fails its signing gate.

Tests

  • The “no keys configured” refusal test now passes an explicit empty list instead of depending on the repository being empty.
  • New tests assert each committed key parses as Ed25519 and that key IDs are unique.

Written for commit c85b238. Summary will update on new commits.

Review in cubic


Note

Medium Risk
Changes which keys are trusted for runtime pack manifests (security-sensitive), but verification logic is unchanged and the private key stays in CI only.

Overview
Enables network installs of signed local-harness runtime packs by committing the public half of the Ed25519 release key (pack-2026-09) into PACK_SIGNING_KEYS, which previously shipped empty and caused verifyPackManifestSignature to refuse every downloaded pack.

Comments in pack-signing-key.ts are updated to describe empty-list refusal and additive key rotation. Tests no longer assert the repo’s key list is empty; the “no keys” case passes [] explicitly. New coverage ensures each committed PEM parses as Ed25519 and that keyId values are unique.

A patch changeset is added for @mcpjam/inspector. CI must still have LOCAL_HARNESS_PACK_SIGNING_KEY set so release builds can sign packs this Inspector will accept.

Reviewed by Cursor Bugbot for commit c85b238. Bugbot is set up for automated code reviews on this repo. Configure here.

`PACK_SIGNING_KEYS` shipped empty. That is a refusal rather than a bypass —
`verifyPackManifestSignature` turns away every network-sourced pack while the
list is empty — and it was the right default before a key existed. It is also
why four of five legs of `build-local-harness-pack` failed on the 2026-09-02
release: the job refuses to publish packs no Inspector would install, and
`LOCAL_HARNESS_PACK_SIGNING_KEY` did not exist.

So generate the key. Ed25519, public half here, private half only in the CI
secret — no copy anywhere else, which makes recovery rotation rather than
restore. `docs/local-harness.md` already called this out as a hard blocker for
release; this is that checklist item.

The tests move with it. "Refuses everything while no key is configured" was
asserting `PACK_SIGNING_KEYS` is `[]`, which pinned the repository's state
rather than the function's property; it now passes an explicit empty list, so
it keeps testing the refusal after the repository stops being empty.

Two new ones cover what committing a key actually risks. A PEM is a string
until something parses it, and `verifyPackManifestSignature` swallows a
malformed key on purpose so one bad entry cannot shadow a good one — so a typo
would survive the build, the typecheck and every existing test, and surface
only when a user's install reported "not signed by any key this Inspector
trusts". One test parses every committed key and asserts it is Ed25519; the
other refuses duplicate key ids, since the id is how a refusal names which key
turned a pack away.

Verified out of band that the generated pair round-trips through the real
verify path: a manifest signed with the private half as
`build-local-harness-pack.mjs` signs it verifies against the committed public
half with `keyId: "pack-2026-09"`, and a manifest altered after signing is
refused as `bad-signature`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011qNR584viu78RCTK7obRVU
@chelojimenez

chelojimenez commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Internal preview

Preview URL will appear in Railway after the deploy finishes.
Backend target: staging fallback.
Access is employee-only in non-production environments.

@chelojimenez
chelojimenez marked this pull request as ready for review September 3, 2026 00:39
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@cursor

cursor Bot commented Sep 3, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_7116d29a-8fd3-4b24-95a6-db626d6a7870)

@chelojimenez
chelojimenez merged commit dbf145f into main Sep 3, 2026
16 of 17 checks passed
@chelojimenez
chelojimenez deleted the feat/pack-signing-key branch September 3, 2026 00:39
@cursor

cursor Bot commented Sep 3, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_dbaeada6-e395-4875-8b29-ed729aa7169e)

@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 1e9efe1e-a848-4c52-91b1-42e31e5da742

📥 Commits

Reviewing files that changed from the base of the PR and between 38f9afa and c85b238.

📒 Files selected for processing (3)
  • .changeset/local-harness-pack-signing-key.md
  • mcpjam-inspector/server/utils/harness/local/__tests__/pack-signing-key.test.ts
  • mcpjam-inspector/server/utils/harness/local/pack-signing-key.ts

Walkthrough

The local harness now includes the pack-2026-09 Ed25519 public key in PACK_SIGNING_KEYS. Tests explicitly cover empty trust lists, valid Ed25519 key parsing, key identifier formatting, and duplicate identifiers. A patch changeset documents the signing-key release.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

chelojimenez added a commit that referenced this pull request Sep 3, 2026
…he repo

`cannot resolve a platform the shipped manifest has no pack digest for` passed
`LOCAL_HARNESS_MANIFEST` and relied on the SHIPPED table being empty —
`compatibility.ts` builds `bundleDigest` straight from `PACK_TREE_DIGESTS`, so
committing the digests gave linux-x64 a digest and the fixture bundle (one file
reading "x") stopped being absent and started being a mismatch.

The property under test is the manifest's, not the repository's: a target with
no digest must refuse rather than accept whatever bundle directory is on disk,
because there would be nothing to check it against. So build a manifest with
`bundleDigest: {}` and assert against that. It now holds whatever the shipped
table contains, which is what it was always trying to say.

The inverse — that no platform is called native without a digest to admit it —
is already `pack-digests.test.ts`'s job, and it passes.

Same shape as the `PACK_SIGNING_KEYS` test in #4635: an assertion that pinned
the pre-launch repo state rather than the behaviour, and had to move the moment
the state changed. This was the last one; nothing else in the suite reads the
generated table for its emptiness.

Verified: whole `local/__tests__` directory 518 passed, 29 skipped, and
`server/tsconfig.json` typechecks clean. `runtime-install.test.ts` still fails
locally on its `tar` helper, identically on clean main — macOS bsdtar, and
green in CI.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011qNR584viu78RCTK7obRVU
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.

1 participant