fix(api): qualify batch skill security verdicts by owner (#3409)

This commit is contained in:
Peter Steinberger
2026-08-05 08:47:19 -07:00
committed by GitHub
parent 459caf6250
commit f9ea25e14f
6 changed files with 201 additions and 8 deletions
+92 -1
View File
@@ -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<string, unknown>) => {
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", {
+12 -2
View File
@@ -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,
@@ -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();
});
+2 -2
View File
@@ -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;
+13 -3
View File
@@ -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
+5
View File
@@ -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