From 6d2ff66001736e4dd58db281a3c996793befa542 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 16 May 2026 14:10:04 +0000 Subject: [PATCH 1/2] Security: close marketplace-poisoning / verified-author escalation Audit of the upload + publish + MCP-serving paths found a critical chain: - author.server.insertDraftPackage hardcoded author_handle="@admin" and author_verified=true for EVERY uploaded package, regardless of uploader. - The UI bulkUploadPackages trusted a client-supplied `publish:true`, and the "packages author write" RLS policy is FOR ALL USING(author_id=uid) with no WITH CHECK and no column-level guard. Since the browser uses the anon key, an authenticated user could bypass all server-fn gates with a direct PostgREST update setting is_published+review_status='approved'+ author_verified=true, getting an arbitrary/malicious skill served by the MCP discovery tools to every connected agent as admin-verified and review-approved, with no adversarial testing. Fixes: - insertDraftPackage now always creates a private, unverified, draft package (author_verified=false, is_published=false, review_status=draft). - New migration adds BEFORE INSERT/UPDATE triggers that revert any change to trust/visibility columns (author_verified, author_handle, review_status, reviewed_*, is_published, install_count, author_id) unless the caller is an admin; the author write policy gets a WITH CHECK; any published-but-unapproved drift is normalized. - UI upload no longer offers an instant-publish toggle; publishing routes through submit-for-review + admin approval. Dead `publish` plumbing removed from the upload pipeline; admin import/meta-ads flows now publish via an explicit, authorized post-insert update. - MCP instructions/tool copy corrected (they advertised publish:true which the code never honored). https://claude.ai/code/session_01CV6zb1KBVoe3eBttyK9U4Z --- src/lib/admin/author.server.ts | 15 ++- src/lib/admin/imports.functions.ts | 8 +- src/lib/admin/meta-ads-pack.functions.ts | 11 ++- src/lib/mcp/tools/skills.ts | 4 +- src/lib/uploads/uploads.functions.ts | 5 +- src/lib/uploads/uploads.server.ts | 4 +- src/routes/api/mcp.ts | 2 +- src/routes/upload.tsx | 12 +-- ...000000_marketplace_integrity_hardening.sql | 99 +++++++++++++++++++ 9 files changed, 136 insertions(+), 24 deletions(-) create mode 100644 supabase/migrations/20260516000000_marketplace_integrity_hardening.sql diff --git a/src/lib/admin/author.server.ts b/src/lib/admin/author.server.ts index e303c9b..293a12a 100644 --- a/src/lib/admin/author.server.ts +++ b/src/lib/admin/author.server.ts @@ -37,7 +37,7 @@ export async function insertDraftPackage( supabase: any, userId: string, draft: any, - meta: { source_kind: "github" | "markdown" | "request" | "wizard"; source_ref: string; publish?: boolean } + meta: { source_kind: "github" | "markdown" | "request" | "wizard"; source_ref: string } ) { const baseSlug = draft.slug; let slug = baseSlug; @@ -63,9 +63,14 @@ export async function insertDraftPackage( description: draft.description, long_description: draft.long_description, author_id: userId, - author_handle: "@admin", - author_verified: true, - is_published: !!meta.publish, + // Trust fields are NOT self-asserted. A new draft is unverified and + // unreviewed; `author_verified` and `review_status='approved'` are only + // ever granted by an admin via the review workflow. The DB also enforces + // this with a BEFORE UPDATE trigger so a compromised/abused client + // cannot escalate via direct RLS writes. + author_verified: false, + is_published: false, + review_status: "draft", latest_version: "0.1.0", scopes: draft.scopes, source_kind: meta.source_kind, @@ -78,7 +83,7 @@ export async function insertDraftPackage( const { error: verErr } = await supabase.from("package_versions").insert({ package_id: pkg.id, version: "0.1.0", - status: meta.publish ? "stable" : "beta", + status: "beta", notes: `Source: ${meta.source_kind} (${meta.source_ref})`, system_prompt: draft.system_prompt, rules: draft.rules, diff --git a/src/lib/admin/imports.functions.ts b/src/lib/admin/imports.functions.ts index 13d0859..fdcca79 100644 --- a/src/lib/admin/imports.functions.ts +++ b/src/lib/admin/imports.functions.ts @@ -25,8 +25,14 @@ export const wizardCreatePackage = createServerFn({ method: "POST" }) const pkg = await insertDraftPackage(supabase, userId, draft, { source_kind: "wizard", source_ref: vertical || "wizard", - publish: data.publish, }); + // Admin-only (requireAdmin) flow; publishing is an explicit authorized step. + if (data.publish) { + await supabase + .from("packages") + .update({ is_published: true, review_status: "approved" }) + .eq("id", pkg.id); + } return { package: pkg, draft }; }); diff --git a/src/lib/admin/meta-ads-pack.functions.ts b/src/lib/admin/meta-ads-pack.functions.ts index 3014178..ae56a58 100644 --- a/src/lib/admin/meta-ads-pack.functions.ts +++ b/src/lib/admin/meta-ads-pack.functions.ts @@ -105,8 +105,17 @@ export const generateMetaAdsBlueprint = createServerFn({ method: "POST" }) const pkg = await insertDraftPackage(supabase, userId, draft, { source_kind: "wizard", source_ref: `meta-ads-mcp:${bp.id}`, - publish: data.publish, }); + // insertDraftPackage always creates a private, unverified draft. This is an + // admin-only (requireAdmin) flow, so publishing is an explicit, authorized + // step — the DB trust-column trigger permits it because auth.uid() is an + // admin here. + if (data.publish) { + await supabase + .from("packages") + .update({ is_published: true, review_status: "approved" }) + .eq("id", pkg.id); + } // 4) Patch the version row with mcp_servers / permissions / live_resources columns // (insertDraftPackage's INSERT only writes the core version fields). diff --git a/src/lib/mcp/tools/skills.ts b/src/lib/mcp/tools/skills.ts index d3e9361..766c015 100644 --- a/src/lib/mcp/tools/skills.ts +++ b/src/lib/mcp/tools/skills.ts @@ -60,7 +60,7 @@ export const overviewTool = defineTool({ description: "Push a local primitive upward to the registry so others (and future-you) benefit. Requires OAuth.", workflow: [ - "1. upload_packages — bulk upload markdown/prompt/JSON files. Normalised by SkillForge into draft packages. Pass publish:true to publish immediately.", + "1. upload_packages — bulk upload markdown/prompt/JSON files. Normalised by SkillForge into PRIVATE draft packages. Never auto-published; public listing requires author submit + admin review/approval.", "2. request_primitive — ask SuperAgentSkill to AUTHOR a brand-new primitive from scratch via the forge pipeline.", ], tools: ["upload_packages", "request_primitive"], @@ -289,7 +289,7 @@ export const uploadPackagesTool = defineTool({ }); try { // Always private. Marketplace listing requires an explicit user action in the UI. - const results = await processBulkUpload(supabaseAdmin as any, userId, files, { publish: false }); + const results = await processBulkUpload(supabaseAdmin as any, userId, files); const ok = results.filter((r) => r.ok).length; return json({ uploaded: ok, diff --git a/src/lib/uploads/uploads.functions.ts b/src/lib/uploads/uploads.functions.ts index 47a9904..ba75d99 100644 --- a/src/lib/uploads/uploads.functions.ts +++ b/src/lib/uploads/uploads.functions.ts @@ -11,7 +11,6 @@ const FileSchema = z.object({ const Input = z.object({ files: z.array(FileSchema).min(1).max(10), - publish: z.boolean().default(false), }); /** @@ -24,8 +23,6 @@ export const bulkUploadPackages = createServerFn({ method: "POST" }) .handler(async ({ data, context }): Promise<{ results: UploadResult[] }> => { const { supabase: _sbCtx, userId } = context as any; const supabase = _sbCtx as any; - const results = await processBulkUpload(supabase as any, userId, data.files, { - publish: data.publish, - }); + const results = await processBulkUpload(supabase as any, userId, data.files); return { results }; }); diff --git a/src/lib/uploads/uploads.server.ts b/src/lib/uploads/uploads.server.ts index 81aa10f..df55ad7 100644 --- a/src/lib/uploads/uploads.server.ts +++ b/src/lib/uploads/uploads.server.ts @@ -30,8 +30,7 @@ export type UploadResult = { export async function processBulkUpload( supabase: any, userId: string, - files: UploadFileInput[], - opts?: { publish?: boolean } + files: UploadFileInput[] ): Promise { const results: UploadResult[] = []; for (const f of files) { @@ -80,7 +79,6 @@ export async function processBulkUpload( const pkg = await insertDraftPackage(supabase, userId, draft, { source_kind: "markdown", source_ref: `upload:${f.name}`, - publish: !!opts?.publish, }); out.ok = true; out.package_id = pkg.id; diff --git a/src/routes/api/mcp.ts b/src/routes/api/mcp.ts index 5b700f4..137d3db 100644 --- a/src/routes/api/mcp.ts +++ b/src/routes/api/mcp.ts @@ -41,7 +41,7 @@ const mcp = createMcpServer({ "", "## 3. PUBLISH new primitives back to the registry", "When the user wants to contribute a local skill upward so others (and future versions of themselves) benefit:", - " - `upload_packages` with the raw file content(s) → normalised by the SkillForge author pipeline, inserted as drafts owned by the token holder. Pass `publish: true` to publish immediately (subject to author permissions).", + " - `upload_packages` with the raw file content(s) → normalised by the SkillForge author pipeline, inserted as PRIVATE drafts owned by the token holder. Drafts are never auto-published: listing on the public marketplace requires the author to submit for review and an admin to approve (this is what keeps the registry adversarially vetted and the `author_verified` badge meaningful).", " - `request_primitive` if the user wants SuperAgentSkill to AUTHOR a brand-new primitive from scratch via the forge pipeline.", "", "## Auth", diff --git a/src/routes/upload.tsx b/src/routes/upload.tsx index 7cff692..021eda4 100644 --- a/src/routes/upload.tsx +++ b/src/routes/upload.tsx @@ -42,14 +42,13 @@ const MAX_BYTES = 120_000; function UploadPage() { const [files, setFiles] = useState([]); - const [publish, setPublish] = useState(false); const [results, setResults] = useState(null); const [error, setError] = useState(null); const inputRef = useRef(null); const fn = useServerFn(bulkUploadPackages); const mut = useMutation({ - mutationFn: (payload: { files: { name: string; content: string; type?: Staged["type"] }[]; publish: boolean }) => + mutationFn: (payload: { files: { name: string; content: string; type?: Staged["type"] }[] }) => fn({ data: payload }), onSuccess: (res) => setResults(res.results), onError: (e: Error) => @@ -87,7 +86,6 @@ function UploadPage() { content: f.content, type: f.type === "auto" ? undefined : f.type, })), - publish, }); } @@ -175,10 +173,10 @@ function UploadPage() { {/* Submit bar */}
- +

+ Uploads are saved as private drafts. Listing on the public marketplace + requires submitting for review and admin approval. +