From 2876ee7724875ad957d3cf711834ce95f8050a5f Mon Sep 17 00:00:00 2001 From: Duy Nguyen Date: Sat, 17 Oct 2026 16:18:10 +0700 Subject: [PATCH] fix(skill-tree): never derive publisher-signed trust for skillx releases A SkillX registry signature is not a publisher signature, so no skillx input (verified flag, empty, whitespace or null origin) can raise trust above local-unverified. Vendor the AgentKit dev identity vectors that pin this and cover the origin and verified combinations. Co-Authored-By: Claude Sonnet 5.5 --- packages/skill-tree/fixtures/PROVENANCE.md | 2 +- .../fixtures/vectors/identity-v1-vectors.json | 4 +-- packages/skill-tree/src/identity.ts | 5 ++-- .../skill-tree/test/identity-vectors.test.ts | 26 +++++++++++++++++++ packages/skill-tree/test/provenance.test.ts | 2 +- 5 files changed, 33 insertions(+), 6 deletions(-) diff --git a/packages/skill-tree/fixtures/PROVENANCE.md b/packages/skill-tree/fixtures/PROVENANCE.md index 887cce0..2e1c0ea 100644 --- a/packages/skill-tree/fixtures/PROVENANCE.md +++ b/packages/skill-tree/fixtures/PROVENANCE.md @@ -7,7 +7,7 @@ update by copying the new upstream file and its new hash, never by editing a vec | File | Source | Commit | sha256 | |---|---|---|---| | `vectors/skill-tree-v1-vectors.json` | `bestagentkits/agentkit` `docs/conformance/skill-lifecycle/skill-tree-v1-vectors.json` | `dev` @ `479483bfe8ff022063f7bff21e75df17fabf0997` (git blob `ecb6722af896f41b176e5f5a5e1ba367c58449aa`) | `8eb46f1b8bec49cace740b75614f28f8fb4a6fc266fecfcd7e3d6892f28f3172` | -| `vectors/identity-v1-vectors.json` | `bestagentkits/agentkit` `docs/conformance/skill-lifecycle/identity-v1-vectors.json` | `dev` @ `479483bfe8ff022063f7bff21e75df17fabf0997` (git blob `d532820a500e792eef6445b0e54ba49554ab3fa6`) | `1ab9e013c229d78871abd60e3cd3bf26fcfd0e6067d98dcdd79b4f6bcc1daefb` | +| `vectors/identity-v1-vectors.json` | `bestagentkits/agentkit` `docs/conformance/skill-lifecycle/identity-v1-vectors.json` | `dev` @ `5a7578bad0136fa12b598175319bd749e3727a87` (git blob `ab366dc4c3e894436ef2a6ca9e28d582e69dbab3`) | `7981d0f324c4771cf48e4a2aaec1f5e01030cbc063c537f2e1493470cc8e9024` | | `ucd/CaseFolding-15.0.0.txt` | Unicode 15.0.0 `CaseFolding.txt`, as vendored by AgentKit at `apps/cli/internal/core/skilltree/testdata/CaseFolding-15.0.0.txt` | `840d924a7` (branch `mrgoonie/feat/skilltree-canonical-digest`) | `cdd49e55eae3bbf1f0a3f6580c974a0263cb86a6a08daa10fbf705b4808a56f7` | The contract: AgentKit `docs/specs/skill-lifecycle-contract.md` sections 2, 4 and 10.2, and diff --git a/packages/skill-tree/fixtures/vectors/identity-v1-vectors.json b/packages/skill-tree/fixtures/vectors/identity-v1-vectors.json index d532820..ab366dc 100644 --- a/packages/skill-tree/fixtures/vectors/identity-v1-vectors.json +++ b/packages/skill-tree/fixtures/vectors/identity-v1-vectors.json @@ -5,7 +5,7 @@ "notes": [ "Each identity segment must also pass the canonical-tree segment rules and be at most 64 bytes.", "kitMapping rows without an error give the identity and portable alias of a kit skill directory.", - "provenance rows give the trust recorded for an accepted identity, or the rejection code. An unknown provider, a kit row without a known origin, an invalid identity, or a kit row whose identity is not @agentkit fails with identity.invalid; these checks run before the reserved-publisher check. verified on a skillx row says whether the release provenance verified.", + "provenance rows give the trust recorded for an accepted identity, or the rejection code. An unknown provider, a kit row without a known origin, an invalid identity, or a kit row whose identity is not @agentkit fails with identity.invalid; these checks run before the reserved-publisher check. verified on a skillx row says whether the release manifest's registry signature verified; a SkillX registry signature is not a publisher signature, so a skillx row is local-unverified either way.", "aliases rows are invocation aliases. An alias is 1 to 128 characters from [A-Za-z0-9._:/$-]; '$' may appear anywhere. A leading '/' is allowed only in the native slash form '/:', a single segment after the '/'. Split on '/', the alias has no empty, '.' or '..' segment, so no trailing '/'; every ':' in a segment has non-empty text on both sides; and every segment, and every ':'-separated part of one, holds at least one ASCII letter or digit, so '$', '$$' and '/$:x' are invalid. Any other alias, including a non-ASCII one, fails with identity.invalid.", "Reserved prefixes: after removing every leading '/' of the alias and every leading '$' of each '/'-separated segment, if any segment starts with ak- or ak:, compared after mapping only ASCII A-Z to a-z, the alias is reserved for @agentkit identities; any other identity fails with identity.reserved_alias. Shape errors are checked first.", "directoryAliases rows are projection directory names (the portable runtime alias): an alias that is one segment, passes the canonical-tree segment rules (so it never starts with .ak-tmp.) and contains no ':' or '$'; otherwise identity.invalid, then the reserved-prefix check. groupKey is the alias with only ASCII A-Z mapped to a-z; collision and co-ownership grouping use it on every host. directoryAliasCollisions pairs collide exactly when their groupKeys are equal.", @@ -198,7 +198,7 @@ "provider": "skillx", "verified": true, "expect": { - "trust": "publisher-signed" + "trust": "local-unverified" } }, { diff --git a/packages/skill-tree/src/identity.ts b/packages/skill-tree/src/identity.ts index 7f28485..62decda 100644 --- a/packages/skill-tree/src/identity.ts +++ b/packages/skill-tree/src/identity.ts @@ -75,7 +75,7 @@ export interface Provenance { provider: string; /** Kit sources only. */ origin?: string; - /** SkillX releases only: whether publisher provenance verified. */ + /** SkillX releases only: whether the registry signature verified. It never raises trust. */ verified?: boolean; /** Whether the composition has a non-empty overlay. */ overlay?: boolean; @@ -90,7 +90,8 @@ export function trustOf(identity: string, p: Provenance): Trust { checkPublisherFor(identity, p.provider); if (p.overlay === true) return "local-unverified"; if (p.provider === "kit" && p.origin === "remote-signed") return "signed-kit"; - if (p.provider === "skillx" && p.verified === true) return "publisher-signed"; + // A SkillX registry signature is not a publisher signature, so no skillx input + // (verified, any origin) can reach publisher-signed; only a remote-signed kit is signed. return "local-unverified"; } diff --git a/packages/skill-tree/test/identity-vectors.test.ts b/packages/skill-tree/test/identity-vectors.test.ts index 640f817..1151f17 100644 --- a/packages/skill-tree/test/identity-vectors.test.ts +++ b/packages/skill-tree/test/identity-vectors.test.ts @@ -124,6 +124,32 @@ describe("identity rules beyond the vectors", () => { } }); + it("never derives publisher-signed from a skillx row, whatever its origin or verified flag", () => { + const origins: Array = [undefined, "", " ", " ", "remote-signed", "local-dev", "null"]; + for (const origin of origins) { + for (const verified of [true, false, undefined]) { + for (const overlay of [true, false, undefined]) { + const p = { provider: "skillx", origin, verified, overlay } as Parameters[1]; + expect(trustOf("@acme/plan", p), JSON.stringify(p)).toBe("local-unverified"); + } + } + } + // A JSON null origin from an untyped caller is also fail-closed. + const nullOrigin = { provider: "skillx", origin: null, verified: true } as unknown as Parameters[1]; + expect(trustOf("@acme/plan", nullOrigin)).toBe("local-unverified"); + }); + + it("only a remote-signed kit without overlay reaches a signed trust tier", () => { + for (const provider of ["local", "git", "skillx"]) { + for (const verified of [true, false]) { + expect(trustOf("@acme/plan", { provider, origin: "remote-signed", verified }), `${provider} ${verified}`).toBe("local-unverified"); + } + } + for (const origin of [undefined, "", " ", "mirror"]) { + expect(codeOf(() => trustOf("@agentkit/plan", { provider: "kit", origin })), String(origin)).toBe("identity.invalid"); + } + }); + it("checks the provider before the reserved publisher", () => { expect(codeOf(() => trustOf("@agentkit/plan", { provider: "npm" }))).toBe("identity.invalid"); expect(codeOf(() => trustOf("@agentkit/plan", { provider: "skillx", verified: true }))).toBe("identity.reserved_publisher"); diff --git a/packages/skill-tree/test/provenance.test.ts b/packages/skill-tree/test/provenance.test.ts index 5ea2b7f..1ef2128 100644 --- a/packages/skill-tree/test/provenance.test.ts +++ b/packages/skill-tree/test/provenance.test.ts @@ -12,7 +12,7 @@ import { fixtureBytes, sha256 } from "./vector-helpers"; const PINS: Record = { "vectors/skill-tree-v1-vectors.json": "8eb46f1b8bec49cace740b75614f28f8fb4a6fc266fecfcd7e3d6892f28f3172", - "vectors/identity-v1-vectors.json": "1ab9e013c229d78871abd60e3cd3bf26fcfd0e6067d98dcdd79b4f6bcc1daefb", + "vectors/identity-v1-vectors.json": "7981d0f324c4771cf48e4a2aaec1f5e01030cbc063c537f2e1493470cc8e9024", "ucd/CaseFolding-15.0.0.txt": "cdd49e55eae3bbf1f0a3f6580c974a0263cb86a6a08daa10fbf705b4808a56f7", }; const GO_TABLES_SHA256 = "0d0155948ceea434fa241b59937db1a41689fa2ccb8231d35d96a2c56f57b52b";