Skip to content

Commit 5ffc166

Browse files
fix(api): guard stale latest version outputs
1 parent 4bb9bef commit 5ffc166

3 files changed

Lines changed: 126 additions & 7 deletions

File tree

convex/lib/skillFileAccess.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ export function getPublicSkillFileAccessBlock(
2525
if (moderationInfo?.isPendingScan) {
2626
return {
2727
status: 423,
28-
message: "This skill is pending a ClawScan security review. Please try again in a few minutes.",
28+
message:
29+
"This skill is pending a ClawScan security review. Please try again in a few minutes.",
2930
};
3031
}
3132
if (moderationInfo?.isRemoved) {

convex/skills.public.test.ts

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ const { getSkillBadgeMap } = await import("./lib/badges");
1717
const {
1818
getBySlug,
1919
listSkillReportsInternal,
20+
resolveVersionByHash,
2021
resolveSkillAppealForUserInternal,
2122
submitSkillAppealForUserInternal,
2223
triageSkillReportForUserInternal,
@@ -75,6 +76,19 @@ const getBySlugHandler = (
7576
>
7677
)._handler;
7778

79+
const resolveVersionByHashHandler = (
80+
resolveVersionByHash as unknown as WrappedHandler<
81+
{
82+
slug: string;
83+
hash: string;
84+
},
85+
{
86+
match: { version: string } | null;
87+
latestVersion: { version: string } | null;
88+
} | null
89+
>
90+
)._handler;
91+
7892
const submitSkillAppealForUserInternalHandler = (
7993
submitSkillAppealForUserInternal as unknown as WrappedHandler<
8094
{
@@ -215,6 +229,39 @@ function makeSkill(overrides: Record<string, unknown> = {}) {
215229
};
216230
}
217231

232+
function makeResolveCtx(args: {
233+
skill: Record<string, unknown>;
234+
latestVersion?: Record<string, unknown> | null;
235+
matchVersion?: Record<string, unknown> | null;
236+
fingerprintMatches?: Array<Record<string, unknown>>;
237+
}) {
238+
const fingerprintMatches = args.fingerprintMatches ?? [
239+
{ versionId: "skillVersions:match", createdAt: 10 },
240+
];
241+
const query = vi.fn((table: string) => {
242+
if (table === "skills") {
243+
return { withIndex: vi.fn(() => ({ unique: vi.fn().mockResolvedValue(args.skill) })) };
244+
}
245+
if (table === "skillVersionFingerprints") {
246+
return { withIndex: vi.fn(() => ({ take: vi.fn().mockResolvedValue(fingerprintMatches) })) };
247+
}
248+
if (table === "skillVersions") {
249+
return {
250+
withIndex: vi.fn(() => ({
251+
order: vi.fn(() => ({ take: vi.fn().mockResolvedValue([]) })),
252+
})),
253+
};
254+
}
255+
throw new Error(`Unexpected query table: ${table}`);
256+
});
257+
const get = vi.fn(async (id: string) => {
258+
if (id === args.skill.latestVersionId) return args.latestVersion ?? null;
259+
if (id === "skillVersions:match") return args.matchVersion ?? null;
260+
return null;
261+
});
262+
return { db: { query, get } } as never;
263+
}
264+
218265
describe("skills.getBySlug", () => {
219266
beforeEach(() => {
220267
vi.mocked(getAuthUserId).mockReset();
@@ -527,6 +574,77 @@ describe("skills.getBySlug", () => {
527574
expect(result?.skill).toMatchObject({ latestVersionId: "skillVersions:other" });
528575
expect(result?.latestVersion).toBeNull();
529576
});
577+
578+
it("does not expose a soft-deleted latest version", async () => {
579+
const ctx = makeCtx({
580+
skill: makeSkill({ latestVersionId: "skillVersions:deleted" }),
581+
owner: makeOwner("users:1", "demo-owner"),
582+
latestVersion: {
583+
_id: "skillVersions:deleted",
584+
_creationTime: 2,
585+
skillId: "skills:1",
586+
version: "2.0.0",
587+
fingerprint: "abc",
588+
changelog: "",
589+
changelogSource: "user",
590+
files: [],
591+
createdBy: "users:1",
592+
createdAt: 2,
593+
softDeletedAt: 3,
594+
},
595+
});
596+
597+
const result = await getBySlugHandler(ctx, { slug: "demo" } as never);
598+
599+
expect(result?.latestVersion).toBeNull();
600+
});
601+
});
602+
603+
describe("skills.resolveVersionByHash", () => {
604+
const hash = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
605+
606+
it("does not expose a soft-deleted latest version", async () => {
607+
const ctx = makeResolveCtx({
608+
skill: makeSkill({ latestVersionId: "skillVersions:deleted" }),
609+
latestVersion: {
610+
_id: "skillVersions:deleted",
611+
skillId: "skills:1",
612+
version: "2.0.0",
613+
softDeletedAt: 3,
614+
},
615+
matchVersion: {
616+
_id: "skillVersions:match",
617+
skillId: "skills:1",
618+
version: "1.0.0",
619+
files: [],
620+
},
621+
});
622+
623+
const result = await resolveVersionByHashHandler(ctx, { slug: "demo", hash });
624+
625+
expect(result).toMatchObject({ match: { version: "1.0.0" }, latestVersion: null });
626+
});
627+
628+
it("does not expose a latest version that belongs to another skill", async () => {
629+
const ctx = makeResolveCtx({
630+
skill: makeSkill({ latestVersionId: "skillVersions:other" }),
631+
latestVersion: {
632+
_id: "skillVersions:other",
633+
skillId: "skills:other",
634+
version: "9.9.9",
635+
},
636+
matchVersion: {
637+
_id: "skillVersions:match",
638+
skillId: "skills:1",
639+
version: "1.0.0",
640+
files: [],
641+
},
642+
});
643+
644+
const result = await resolveVersionByHashHandler(ctx, { slug: "demo", hash });
645+
646+
expect(result).toMatchObject({ match: { version: "1.0.0" }, latestVersion: null });
647+
});
530648
});
531649

532650
describe("skill artifact moderation", () => {

convex/skills.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,7 @@ import {
9999
} from "./lib/reservedSlugs";
100100
import { matchesAllTokens, matchesExploratoryTokenPrefixes, tokenize } from "./lib/searchText";
101101
import { SKILL_CAPABILITY_TAGS } from "./lib/skillCapabilityTags";
102-
import {
103-
isPublicSkillVersionAvailableForSkill,
104-
isSkillVersionForSkill,
105-
} from "./lib/skillFileAccess";
102+
import { isPublicSkillVersionAvailableForSkill } from "./lib/skillFileAccess";
106103
import { normalizeSkillIconValue } from "./lib/skillIcon";
107104
import {
108105
fetchText,
@@ -1997,7 +1994,7 @@ export const getBySlug = query({
19971994

19981995
const latestVersionDoc = skill.latestVersionId ? await ctx.db.get(skill.latestVersionId) : null;
19991996
const latestVersion = toPublicSkillVersion(
2000-
isSkillVersionForSkill(latestVersionDoc, skill._id) ? latestVersionDoc : null,
1997+
isPublicSkillVersionAvailableForSkill(latestVersionDoc, skill._id) ? latestVersionDoc : null,
20011998
);
20021999
const owner = toPublicPublisher(ownerPublisher);
20032000
if (!owner) return null;
@@ -7395,7 +7392,10 @@ export const resolveVersionByHash = query({
73957392
const skill = resolved.skill;
73967393
if (!skill) return null;
73977394

7398-
const latestVersion = skill.latestVersionId ? await ctx.db.get(skill.latestVersionId) : null;
7395+
const latestVersionDoc = skill.latestVersionId ? await ctx.db.get(skill.latestVersionId) : null;
7396+
const latestVersion = isPublicSkillVersionAvailableForSkill(latestVersionDoc, skill._id)
7397+
? latestVersionDoc
7398+
: null;
73997399

74007400
const fingerprintMatches = await ctx.db
74017401
.query("skillVersionFingerprints")

0 commit comments

Comments
 (0)