From f9ea25e14f14c05f9544d3878d1626d0dcec4b7a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 5 Aug 2026 08:47:19 -0700 Subject: [PATCH] fix(api): qualify batch skill security verdicts by owner (#3409) --- convex/httpApiV1.handlers.test.ts | 93 +++++++++++++++++++- convex/httpApiV1/skillsV1.ts | 14 ++- convex/skills.slugResolution.runtime.test.ts | 77 ++++++++++++++++ convex/skills.ts | 4 +- docs/http-api.md | 16 +++- specs/security-moderation.md | 5 ++ 6 files changed, 201 insertions(+), 8 deletions(-) diff --git a/convex/httpApiV1.handlers.test.ts b/convex/httpApiV1.handlers.test.ts index 3ef0b51b..1c67a387 100644 --- a/convex/httpApiV1.handlers.test.ts +++ b/convex/httpApiV1.handlers.test.ts @@ -6331,6 +6331,67 @@ describe("httpApiV1 handlers", () => { expect(runQuery.mock.calls.some(([, args]) => "skillId" in args)).toBe(false); }); + it("keeps owner-qualified bulk verdicts distinct for shared slug versions", async () => { + const runQuery = vi.fn(async (_query: unknown, args: Record) => { + const ownerHandle = args.ownerHandle as string; + return { + skill: { + _id: `skills:${ownerHandle}`, + slug: "weather", + displayName: `${ownerHandle} Weather`, + }, + owner: { + _id: `publishers:${ownerHandle}`, + handle: ownerHandle, + displayName: ownerHandle, + }, + moderationInfo: null, + version: { + _id: `skillVersions:${ownerHandle}`, + version: "1.2.3", + createdAt: 1, + llmAnalysis: { status: "clean", verdict: "clean", checkedAt: 2 }, + }, + }; + }); + const runMutation = vi.fn().mockResolvedValue(okRate()); + + const response = await __handlers.skillSecurityVerdictsV1Handler( + makeCtx({ runQuery, runMutation }), + new Request("https://example.com/api/v1/skills/-/security-verdicts", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + items: [ + { slug: "weather", ownerHandle: "@Alice", version: "1.2.3" }, + { slug: "weather", ownerHandle: "bob", version: "1.2.3" }, + ], + }), + }), + ); + + expect(response.status).toBe(200); + const json = await response.json(); + expect(json.items).toEqual([ + expect.objectContaining({ + requestedOwnerHandle: "alice", + requestedSlug: "weather", + requestedVersion: "1.2.3", + publisherHandle: "alice", + }), + expect.objectContaining({ + requestedOwnerHandle: "bob", + requestedSlug: "weather", + requestedVersion: "1.2.3", + publisherHandle: "bob", + }), + ]); + expect(runQuery.mock.calls.map(([, args]) => args)).toEqual([ + { slug: "weather", ownerHandle: "alice", version: "1.2.3" }, + { slug: "weather", ownerHandle: "bob", version: "1.2.3" }, + ]); + }); + it("uses the public site origin for production bulk verdict links", async () => { vi.stubEnv("CONVEX_DEPLOYMENT", "prod:wry-manatee-359"); const runQuery = vi.fn(async () => ({ @@ -6472,7 +6533,7 @@ describe("httpApiV1 handlers", () => { headers: { "content-type": "application/json" }, body: JSON.stringify({ items: [ - { slug: "missing", version: "1.0.0" }, + { slug: "missing", ownerHandle: "@Missing-Owner", version: "1.0.0" }, { slug: "no-version", version: "1.0.0" }, { slug: "soft", version: "2.0.0" }, ], @@ -6486,6 +6547,7 @@ describe("httpApiV1 handlers", () => { expect(json.items.map((item: { ok: boolean }) => item.ok)).toEqual([false, false, false]); expect(json.items[0]).toMatchObject({ requestedSlug: "missing", + requestedOwnerHandle: "missing-owner", requestedVersion: "1.0.0", decision: "fail", reasons: ["skill.not_found"], @@ -6631,6 +6693,35 @@ describe("httpApiV1 handlers", () => { expect(duplicate.status).toBe(400); expect(await duplicate.text()).toBe("Duplicate item: demo@1.0.0"); + const qualifiedDuplicate = await __handlers.skillSecurityVerdictsV1Handler( + makeCtx({ runQuery, runMutation }), + new Request("https://example.com/api/v1/skills/-/security-verdicts", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + items: [ + { slug: "demo", ownerHandle: "@Alice", version: "1.0.0" }, + { slug: "demo", ownerHandle: "alice", version: "1.0.0" }, + ], + }), + }), + ); + expect(qualifiedDuplicate.status).toBe(400); + expect(await qualifiedDuplicate.text()).toBe("Duplicate item: @alice/demo@1.0.0"); + + const invalidOwner = await __handlers.skillSecurityVerdictsV1Handler( + makeCtx({ runQuery, runMutation }), + new Request("https://example.com/api/v1/skills/-/security-verdicts", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + items: [{ slug: "demo", ownerHandle: 42, version: "1.0.0" }], + }), + }), + ); + expect(invalidOwner.status).toBe(400); + expect(await invalidOwner.text()).toBe("Invalid ownerHandle at items[0]"); + const ambiguous = await __handlers.skillSecurityVerdictsV1Handler( makeCtx({ runQuery, runMutation }), new Request("https://example.com/api/v1/skills/-/security-verdicts", { diff --git a/convex/httpApiV1/skillsV1.ts b/convex/httpApiV1/skillsV1.ts index cca61baa..5d64a059 100644 --- a/convex/httpApiV1/skillsV1.ts +++ b/convex/httpApiV1/skillsV1.ts @@ -38,6 +38,7 @@ import { type InstallResolverSource, type SkillInstallResolution, } from "../lib/installResolver"; +import { normalizePublisherHandle } from "../lib/publishers"; import { MAX_PUBLISH_FILE_BYTES } from "../lib/publishLimits"; import { getRuntimeRolloutCapabilities } from "../lib/rolloutCapabilities"; import type { @@ -689,6 +690,7 @@ type SkillVersionFingerprintSummary = { type SecurityVerdictRequestItem = { slug: string; + ownerHandle?: string; version: string; }; @@ -1012,10 +1014,15 @@ function parseSecurityVerdictItems( if (typeof raw.slug !== "string" || typeof raw.version !== "string") { return { ok: false, message: `items[${index}] requires slug and version strings` }; } + const rawOwnerHandle = raw.ownerHandle; + if (rawOwnerHandle !== undefined && typeof rawOwnerHandle !== "string") { + return { ok: false, message: `Invalid ownerHandle at items[${index}]` }; + } if ("tag" in raw) { return { ok: false, message: `items[${index}] uses version only; tag is not supported` }; } const slug = raw.slug.trim().toLowerCase(); + const ownerHandle = normalizePublisherHandle(rawOwnerHandle); const version = raw.version.trim(); if (!validateSlug(slug)) { return { ok: false, message: `Invalid slug at items[${index}]` }; @@ -1023,10 +1030,10 @@ function parseSecurityVerdictItems( if (!isValidRequestedVersion(version)) { return { ok: false, message: `Invalid version at items[${index}]` }; } - const key = `${slug}@${version}`; + const key = `${ownerHandle ? `@${ownerHandle}/` : ""}${slug}@${version}`; if (seen.has(key)) return { ok: false, message: `Duplicate item: ${key}` }; seen.add(key); - parsed.push({ slug, version }); + parsed.push({ slug, ...(ownerHandle ? { ownerHandle } : {}), version }); } return { ok: true, items: parsed }; @@ -1140,6 +1147,7 @@ function buildSecurityVerdictError( decision: "fail", reasons: [reason], requestedSlug: item.slug, + ...(item.ownerHandle ? { requestedOwnerHandle: item.ownerHandle } : {}), slug: item.slug, requestedVersion: item.version, version: null, @@ -1165,6 +1173,7 @@ async function buildSecurityVerdictItem( internalRefs.skills.getSecurityVerdictTargetInternal, { slug: item.slug, + ...(item.ownerHandle ? { ownerHandle: item.ownerHandle } : {}), version: item.version, }, ); @@ -1204,6 +1213,7 @@ async function buildSecurityVerdictItem( decision: reasons.length === 0 ? "pass" : "fail", reasons, requestedSlug: item.slug, + ...(item.ownerHandle ? { requestedOwnerHandle: item.ownerHandle } : {}), slug: result.skill.slug, displayName: result.skill.displayName, publisherHandle: result.owner?.handle ?? null, diff --git a/convex/skills.slugResolution.runtime.test.ts b/convex/skills.slugResolution.runtime.test.ts index 44e97064..04b6584e 100644 --- a/convex/skills.slugResolution.runtime.test.ts +++ b/convex/skills.slugResolution.runtime.test.ts @@ -72,6 +72,50 @@ async function createPublisherSlugFixture(options: { return { t, ...ids }; } +async function createOwnerCollisionVerdictFixture() { + const t = convexTest(schema, modules); + const ids = await t.run(async (ctx) => { + const owners = []; + for (const handle of ["alice", "bob"] as const) { + const userId = await ctx.db.insert("users", { handle }); + const publisherId = await ctx.db.insert("publishers", { + kind: "user", + handle, + displayName: handle, + linkedUserId: userId, + createdAt: 1, + updatedAt: 1, + }); + await ctx.db.patch(userId, { personalPublisherId: publisherId }); + const skillId = await ctx.db.insert("skills", { + slug: "shared-skill", + displayName: `${handle} skill`, + ownerUserId: userId, + ownerPublisherId: publisherId, + tags: {}, + badges: {}, + moderationStatus: "active", + stats: { comments: 0, downloads: 0, stars: 0, versions: 1 }, + createdAt: 1, + updatedAt: 1, + }); + const versionId = await ctx.db.insert("skillVersions", { + skillId, + version: "1.2.3", + changelog: "Initial", + files: [], + parsed: { frontmatter: {} }, + createdBy: userId, + createdAt: 1, + }); + await ctx.db.patch(skillId, { latestVersionId: versionId }); + owners.push({ handle, skillId, versionId }); + } + return owners; + }); + return { t, owners: ids }; +} + it("resolves the active skill when retained same-publisher history shares its slug", async () => { const fixture = await createPublisherSlugFixture({ activeCount: 1, softDeletedCount: 2 }); @@ -112,3 +156,36 @@ it("fails closed when only multiple soft-deleted skills share a publisher slug", }), ).rejects.toThrow(/soft-deleted publisher slug history is ambiguous/i); }); + +it("resolves security verdict targets by owner when slug and version collide", async () => { + const fixture = await createOwnerCollisionVerdictFixture(); + + for (const owner of fixture.owners) { + const verdictTarget = await fixture.t.query(internal.skills.getSecurityVerdictTargetInternal, { + slug: "shared-skill", + ownerHandle: owner.handle, + version: "1.2.3", + }); + const verifyTarget = await fixture.t.query(internal.skills.getVerifyTargetBySlugInternal, { + slug: "shared-skill", + ownerHandle: owner.handle, + }); + + expect(verdictTarget).toMatchObject({ + skill: { _id: owner.skillId }, + owner: { handle: owner.handle }, + version: { _id: owner.versionId, version: "1.2.3" }, + }); + expect(verifyTarget).toMatchObject({ + skill: { _id: owner.skillId }, + owner: { handle: owner.handle }, + }); + } + + await expect( + fixture.t.query(internal.skills.getSecurityVerdictTargetInternal, { + slug: "shared-skill", + version: "1.2.3", + }), + ).resolves.toBeNull(); +}); diff --git a/convex/skills.ts b/convex/skills.ts index 6bf8dad2..02c0d609 100644 --- a/convex/skills.ts +++ b/convex/skills.ts @@ -3413,9 +3413,9 @@ function compactSecurityVerdictVersion(version: Doc<"skillVersions">) { } export const getSecurityVerdictTargetInternal = internalQuery({ - args: { slug: v.string(), version: v.string() }, + args: { slug: v.string(), ownerHandle: v.optional(v.string()), version: v.string() }, handler: async (ctx, args) => { - const resolved = await resolveSkillBySlugOrAlias(ctx, args.slug); + const resolved = await resolveSkillBySlugOrAliasForOwner(ctx, args.slug, args.ownerHandle); const skill = resolved.skill; if (!skill) return null; diff --git a/docs/http-api.md b/docs/http-api.md index aca5640f..12c46596 100644 --- a/docs/http-api.md +++ b/docs/http-api.md @@ -446,11 +446,13 @@ Returns the Skill Card verification envelope used by `clawhub skill verify`. Query params: +- `ownerHandle` (optional): publisher handle for owner-qualified resolution. Use this when multiple publishers share the slug. - `version` (optional): specific version string. - `tag` (optional): resolve a tagged version (for example `latest`). Notes: +- `ownerHandle` is normalized by trimming whitespace, removing leading `@` characters, and lowercasing. - `ok` is `true` only when the selected version has a generated Skill Card, is not malware-blocked by moderation, and ClawScan verification is clean. - Skill identity, publisher identity, and selected version metadata are top-level envelope fields (`slug`, `displayName`, `publisherHandle`, `version`, `resolvedFrom`, `tag`, `createdAt`) so shell automation can read them without unpacking nested wrappers. - `security` is the top-level ClawScan/security verdict. Automation should key off `ok`, `decision`, `reasons`, and `security.status`. @@ -468,14 +470,20 @@ Request: ```json { - "items": [{ "slug": "gifgrep", "version": "1.2.3" }] + "items": [ + { "slug": "gifgrep", "ownerHandle": "steipete", "version": "1.2.3" }, + { "slug": "gifgrep", "ownerHandle": "another-publisher", "version": "1.2.3" } + ] } ``` Notes: -- `items` must contain 1-100 unique `{ slug, version }` pairs. -- Results are per item; one missing skill or version does not fail the whole response. +- `ownerHandle` is optional. When present, it selects that publisher's skill before exact-version resolution; omitting it preserves legacy unqualified slug resolution. +- Owner handles are normalized by trimming whitespace, removing leading `@` characters, and lowercasing. +- `items` must contain 1-100 unique `{ ownerHandle?, slug, version }` combinations. The same slug and version may appear under different owners. +- Qualified success and failure items echo the normalized owner as `requestedOwnerHandle`; unqualified items omit that field. +- Results are per item; one missing skill, owner-qualified skill, or version does not fail the whole response. - The response is security-only. It does not include Skill Card data, generated card status, artifact file lists, or detailed scanner payloads. - `security.signals` contains status-level supporting evidence only; use `/scan` or the ClawHub security-audit page for full scanner details. - `security.signals.dependencyRegistry` is retained for v1 response compatibility, but the dependency registry existence scanner is retired and this key is always `null`. @@ -493,6 +501,7 @@ Response: "decision": "pass", "reasons": [], "requestedSlug": "gifgrep", + "requestedOwnerHandle": "steipete", "slug": "gifgrep", "displayName": "GifGrep", "publisherHandle": "steipete", @@ -519,6 +528,7 @@ Response: "decision": "fail", "reasons": ["version.not_found"], "requestedSlug": "missing-version", + "requestedOwnerHandle": "another-publisher", "requestedVersion": "1.0.0", "error": { "code": "version_not_found", "message": "Version not found" }, "security": null diff --git a/specs/security-moderation.md b/specs/security-moderation.md index eb8ef5a1..2cdf2ac0 100644 --- a/specs/security-moderation.md +++ b/specs/security-moderation.md @@ -390,6 +390,11 @@ See also: [acceptable-usage.md](./acceptable-usage.md) for the marketplace polic scanner evidence grouped under `security.signals`. Clients should key install decisions off `ok`, `decision`, `reasons`, and `security.status` instead of re-deriving trust from individual signal payloads. +- Exact-version security verdict reads preserve the complete skill identity. + Batch callers may qualify a request with the publisher handle; owner, slug, + and version form the dedupe identity, and qualified success or failure results + echo the normalized requested owner. Unqualified reads retain legacy slug + resolution, including fail-closed ambiguity, for older clients. - ClawScan verdicts treat purpose-aligned notes as user guidance, not a suspicious verdict. Medium-only material concerns are visible `flagged.review` guidance and must not set `isSuspicious`; high or critical