Verify signed skill bundles before extraction (#734)

* Verify signed skill bundles before extraction

Sign release ZIPs locally with an Ed25519 key from 1Password and pin the public trust root in the Rust installer. Reject unauthenticated downloads before extraction and preserve existing installs on failure. Document the signature-first rollout and explicit local trust paths.

AI-assisted implementation prepared by Codex at Paul Bakaus’s request.

* Fix signed-bundle review guardrails

Make keyring loading failures fatal before any download, accept standard release redirect statuses while retaining URL pinning, and require the signature sidecar before tagging. Add regressions for all three review findings.

AI-assisted changes prepared and tested by Codex at Paul Bakaus’s request.

tests/bundle-signing.test.mjs

@@ -0,0 +1,85 @@
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import { generateKeyPairSync, createPrivateKey, createPublicKey, verify } from 'node:crypto';
+import { mkdtempSync, writeFileSync, readFileSync, rmSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import path from 'node:path';
+import { signBundle, signaturePayload, publicKeyHex, signReleaseBundle, readTrustedKeys } from '../scripts/sign-bundle.mjs';
+
+const { privateKey, publicKey } = generateKeyPairSync('ed25519');
+const trustedKeys = { 'test-only': publicKeyHex(publicKey) };
+
+test('production keyring is populated and excludes the public test key', () => {
+  const keys = readTrustedKeys();
+  const fixture = JSON.parse(readFileSync(new URL('./fixtures/bundle-signature.json', import.meta.url)));
+  assert.ok(Object.keys(keys).length > 0);
+  for (const [id, key] of Object.entries(keys)) {
+    assert.match(id, /^[a-z0-9-]{1,64}$/);
+    assert.match(key, /^[0-9a-f]{64}$/);
+    assert.notEqual(key, fixture.keys['test-only']);
+  }
+});
+
+test('matches the shared Node/Rust interoperability vector (public test seed)', () => {
+  const fixture = JSON.parse(readFileSync(new URL('./fixtures/bundle-signature.json', import.meta.url)));
+  const key = createPrivateKey({
+    key: Buffer.concat([Buffer.from('302e020100300506032b657004220420', 'hex'), Buffer.alloc(32, 7)]),
+    format: 'der', type: 'pkcs8',
+  });
+  assert.deepEqual(signBundle(Buffer.from(fixture.bundle), '4.2.0', key, fixture.keys), fixture.envelope);
+});
+
+test('signs the exact bundle with version, size, digest, artifact and domain bound', () => {
+  const bundle = Buffer.from('test bundle');
+  const envelope = signBundle(bundle, '4.2.0', privateKey, trustedKeys);
+  assert.equal(envelope.keyId, 'test-only');
+  assert.equal(envelope.version, '4.2.0');
+  assert.equal(envelope.size, bundle.length);
+  assert.equal(envelope.artifact, 'universal.zip');
+  assert.equal(envelope.schema, 1);
+  assert.match(signaturePayload(envelope).toString(), /^impeccable-skill-bundle-v1\n/);
+  assert.ok(verify(null, signaturePayload(envelope), publicKey, Buffer.from(envelope.signature, 'hex')));
+  for (const changed of [
+    { version: '4.2.1' }, { size: 1 }, { sha256: '0'.repeat(64) },
+    { artifact: 'other.zip' }, { keyId: 'other-key' },
+  ]) {
+    assert.equal(verify(null, signaturePayload({ ...envelope, ...changed }), publicKey,
+      Buffer.from(envelope.signature, 'hex')), false);
+  }
+});
+
+test('rejects unknown or non-Ed25519 keys and invalid versions', () => {
+  assert.throws(() => signBundle(Buffer.from('zip'), '4.2.0', privateKey, {}), /trusted/);
+  for (const version of ['4.2.0\nother', '../4.2.0', '', '04.2.0', '4.2']) {
+    assert.throws(() => signBundle(Buffer.from('zip'), version, privateKey, trustedKeys), /version/);
+  }
+  const rsa = generateKeyPairSync('rsa', { modulusLength: 2048 });
+  assert.throws(() => publicKeyHex(rsa.publicKey), /Ed25519/);
+});
+
+test('1Password read uses a pipe, checks the pinned key, writes only a public signature', () => {
+  const root = mkdtempSync(path.join(tmpdir(), 'impeccable-sign-test-'));
+  try {
+    const zipPath = path.join(root, 'universal.zip');
+    writeFileSync(zipPath, 'test bundle');
+    const secretReference = 'op://test-vault/test-item/private-key';
+    let called = false;
+    signReleaseBundle({ zipPath, version: '4.2.0', trustedKeys, secretReference,
+      readSecret(reference) {
+        called = true;
+        assert.equal(reference, secretReference);
+        return privateKey.export({ type: 'pkcs8', format: 'pem' });
+      },
+    });
+    assert.ok(called);
+    const envelope = JSON.parse(readFileSync(`${zipPath}.sig.json`, 'utf8'));
+    assert.ok(verify(null, signaturePayload(envelope), createPublicKey(privateKey),
+      Buffer.from(envelope.signature, 'hex')));
+    assert.doesNotMatch(readFileSync(`${zipPath}.sig.json`, 'utf8'), /PRIVATE KEY/);
+    assert.throws(() => signReleaseBundle({ zipPath, version: '4.2.0', trustedKeys,
+      secretReference, readSecret() { throw new Error('SECRET that must not leak'); },
+    }), /Could not read.*1Password/);
+  } finally {
+    rmSync(root, { recursive: true, force: true });
+  }
+});

tests/fixtures/bundle-signature.json

@@ -0,0 +1,15 @@
+{
+  "bundle": "test bundle",
+  "keys": {
+    "test-only": "ea4a6c63e29c520abef5507b132ec5f9954776aebebe7b92421eea691446d22c"
+  },
+  "envelope": {
+    "schema": 1,
+    "keyId": "test-only",
+    "version": "4.2.0",
+    "artifact": "universal.zip",
+    "size": 11,
+    "sha256": "9df2a47bee5f48b9752b2cbd2d6075076556ee293adc97f66f0e0a916e4f6471",
+    "signature": "35dfe147573341b1fdcb4bc4054b0e96c5d07b9069bd1257f745d6ad6c3eca72f52876e7d0403cffeac2e60ef2dea41879ca4cb9cecaebb463332aaf9d15620b"
+  }
+}

tests/release.test.mjs

@@ -88,7 +88,7 @@ describe('release.mjs guards', () => {
     // (and check-engine-release.mjs imports fetch-engine.mjs), so stage them
     // too or the dry runs fail to resolve the modules instead of exercising
     // the guard.
-    for (const dep of ['check-engine-release.mjs', 'fetch-engine.mjs']) {
+    for (const dep of ['check-engine-release.mjs', 'fetch-engine.mjs', 'sign-bundle.mjs', 'bundle-signing-keys.json']) {
       fs.copyFileSync(path.join(REPO_ROOT, 'scripts', dep), path.join(workDir, 'scripts', dep));
     }
     write('.claude-plugin/plugin.json', JSON.stringify({ name: 'impeccable', version: '1.2.3' }));
@@ -171,6 +171,57 @@ describe('release.mjs guards', () => {
     assert.match(stdout, /tag is free/);
     assert.match(stdout, /\[dry-run\] git tag -a skill-v1\.2\.3/);
     assert.match(stdout, /\[dry-run\] gh release create skill-v1\.2\.3/);
+    assert.match(stdout, /1Password is not accessed/);
+    assert.match(stdout, /gh release create[^\n]+universal\.zip\.sig\.json/);
+    assert.equal(fs.existsSync(path.join(workDir, 'dist/universal.zip.sig.json')), false);
+  });
+
+  it('refuses a real release before tagging when signing is not configured', () => {
+    const pkg = JSON.parse(fs.readFileSync(path.join(workDir, 'package.json'), 'utf8'));
+    pkg.scripts = { 'build:release': 'node -e "process.exit(0)"' };
+    write('package.json', JSON.stringify(pkg));
+    git(workDir, 'add', 'package.json');
+    git(workDir, 'commit', '-m', 'fixture build command');
+    git(workDir, 'push', 'origin', 'main');
+    assert.throws(() => execFileSync(process.execPath, ['scripts/release.mjs', 'skill'], {
+      cwd: workDir, encoding: 'utf8', stdio: 'pipe',
+      env: { ...process.env, IMPECCABLE_SKIP_ENGINE_CHECK: '1', IMPECCABLE_SIGNING_KEY_REF: '' },
+    }), error => {
+      assert.match(error.stderr, /Set IMPECCABLE_SIGNING_KEY_REF/);
+      assert.doesNotMatch(error.stdout, /Creating annotated tag|Creating GitHub release/);
+      return true;
+    });
+    assert.equal(git(workDir, 'tag'), '');
+    assert.equal(git(workDir, 'ls-remote', '--tags', 'origin'), '');
+  });
+
+  it('refuses before tagging when the signer returns without creating the sidecar', () => {
+    const pkg = JSON.parse(fs.readFileSync(path.join(workDir, 'package.json'), 'utf8'));
+    pkg.scripts = { 'build:release': 'node -e "process.exit(0)"' };
+    write('package.json', JSON.stringify(pkg));
+    // Stub only inside this disposable repository. No 1Password access, tags,
+    // or real GitHub publication can occur even if the assertion regresses.
+    write('scripts/sign-bundle.mjs', 'export function signReleaseBundle() {}\n');
+    const releaseSource = fs.readFileSync(RELEASE_SCRIPT, 'utf8');
+    const tagStep = 'step(`Creating annotated tag ${tag}`);';
+    assert.ok(releaseSource.includes(tagStep), 'fixture must intercept the tag step');
+    write('scripts/release.mjs', releaseSource.replace(
+      tagStep,
+      'throw new Error("UNEXPECTED_TAG_STEP");'
+    ));
+    git(workDir, 'add', 'package.json', 'scripts/sign-bundle.mjs', 'scripts/release.mjs');
+    git(workDir, 'commit', '-m', 'fixture signer with missing output');
+    git(workDir, 'push', 'origin', 'main');
+    assert.throws(() => execFileSync(process.execPath, ['scripts/release.mjs', 'skill'], {
+      cwd: workDir, encoding: 'utf8', stdio: 'pipe',
+      env: { ...process.env, IMPECCABLE_SKIP_ENGINE_CHECK: '1' },
+    }), error => {
+      assert.match(error.stderr, /Missing artifact: dist\/universal\.zip\.sig\.json/);
+      assert.doesNotMatch(error.stderr, /UNEXPECTED_TAG_STEP/);
+      return true;
+    });
+    assert.equal(git(workDir, 'tag'), '');
+    assert.equal(git(workDir, 'ls-remote', '--tags', 'origin'), '');
   });
 
   it('converts the changelog entry to markdown release notes', () => {