-
Notifications
You must be signed in to change notification settings - Fork 668
Security/plugin hardening #393
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
b4bf36f
1f3cb2e
9f5adb7
844ee61
5f6f4e7
4a9374a
bcc0691
e308215
128bdb0
1ccb658
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,100 @@ const componentSchema = z.object({ | |
| metadata: z.record(z.string(), z.unknown()).optional(), | ||
| }); | ||
|
|
||
| type ComponentInput = z.infer<typeof componentSchema>; | ||
|
|
||
| type ExistingComponent = { | ||
| type: string; | ||
| name: string; | ||
| slug: string; | ||
| description: string | null; | ||
| content: string | null; | ||
| metadata: Record<string, unknown> | null; | ||
| sort_order: number; | ||
| }; | ||
|
|
||
| function slugify(value: string): string { | ||
| return value | ||
| .toLowerCase() | ||
| .replace(/[^a-z0-9]+/g, "-") | ||
| .replace(/^-+|-+$/g, ""); | ||
| } | ||
|
|
||
| function componentKey(type: string, slug: string): string { | ||
| return `${type}:${slug}`; | ||
| } | ||
|
|
||
| // Only fields that affect the install payload — cosmetic edits to name, | ||
| // description, or sort_order must not trigger a rescan. MCP install links and | ||
| // configs can live in metadata, so compare the metadata that will be saved. | ||
| function fingerprintComponent(c: { | ||
| type: string; | ||
| slug?: string | null; | ||
| name: string; | ||
| content?: string | null; | ||
| metadata?: Record<string, unknown> | null; | ||
| }): string { | ||
| const slug = c.slug || slugify(c.name); | ||
| return JSON.stringify({ | ||
| type: c.type, | ||
| slug, | ||
| content: c.content ?? "", | ||
| metadata: c.metadata ?? {}, | ||
| }); | ||
| } | ||
|
|
||
| function resolveComponentMetadata( | ||
| comp: ComponentInput, | ||
| prevByKey: Map<string, ExistingComponent>, | ||
| ): Record<string, unknown> { | ||
| const slug = comp.slug || slugify(comp.name); | ||
| const submitted = comp.metadata; | ||
| if (submitted && Object.keys(submitted).length > 0) { | ||
| return submitted; | ||
| } | ||
| const prev = prevByKey.get(componentKey(comp.type, slug)); | ||
| return prev?.metadata ?? {}; | ||
| } | ||
|
|
||
| function installRelevantChanged( | ||
| prevComponents: ExistingComponent[], | ||
| prevRepository: string | null, | ||
| nextComponents: ComponentInput[], | ||
| nextRepository: string | null, | ||
| ): boolean { | ||
| if ((prevRepository ?? null) !== (nextRepository ?? null)) { | ||
| return true; | ||
| } | ||
|
|
||
| if (prevComponents.length !== nextComponents.length) { | ||
| return true; | ||
| } | ||
|
|
||
| const prevSorted = [...prevComponents] | ||
| .sort((a, b) => a.sort_order - b.sort_order) | ||
| .map(fingerprintComponent) | ||
| .sort(); | ||
| const prevByKey = new Map( | ||
| prevComponents.map((c) => [ | ||
| componentKey(c.type, c.slug || slugify(c.name)), | ||
| c, | ||
| ]), | ||
| ); | ||
| const nextSorted = nextComponents | ||
| .map((component) => | ||
| fingerprintComponent({ | ||
| ...component, | ||
| metadata: resolveComponentMetadata(component, prevByKey), | ||
| }), | ||
| ) | ||
| .sort(); | ||
|
|
||
| for (let i = 0; i < prevSorted.length; i++) { | ||
| if (prevSorted[i] !== nextSorted[i]) return true; | ||
| } | ||
| return false; | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
|
||
|
|
||
| export const updatePluginAction = authActionClient | ||
| .metadata({ | ||
| actionName: "update-plugin", | ||
|
|
@@ -62,7 +156,9 @@ export const updatePluginAction = authActionClient | |
|
|
||
| const { data: existing, error: fetchError } = await supabase | ||
| .from("plugins") | ||
| .select("id, owner_id, slug") | ||
| .select( | ||
| "id, owner_id, slug, repository, github_repo_id, active, plugin_components(type, name, slug, description, content, metadata, sort_order)", | ||
| ) | ||
| .eq("id", id) | ||
| .single(); | ||
|
|
||
|
|
@@ -83,16 +179,45 @@ export const updatePluginAction = authActionClient | |
| ); | ||
| } | ||
|
|
||
| // Source URL is pinned to the parsed GitHub repo so an owner can't keep | ||
| // the verified-looking badge while swapping the install payload. | ||
| const repositoryLocked = existing.github_repo_id != null; | ||
| const effectiveRepository = repositoryLocked | ||
| ? (existing.repository ?? null) | ||
| : (repository ?? null); | ||
|
|
||
| const prevComponents = (existing.plugin_components ?? | ||
| []) as ExistingComponent[]; | ||
|
|
||
| const installChanged = installRelevantChanged( | ||
| prevComponents, | ||
| existing.repository ?? null, | ||
| components, | ||
| effectiveRepository, | ||
| ); | ||
|
|
||
| const shouldRescan = installChanged; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Scan quota on cosmetic editsMedium Severity · Logic Bug
Reviewed by Cursor Bugbot for commit 1ccb658. Configure here. |
||
| const updatePayload: Record<string, unknown> = { | ||
| name, | ||
| description, | ||
| logo: logo || null, | ||
| repository: effectiveRepository, | ||
| homepage: homepage || null, | ||
| keywords: keywords || [], | ||
| }; | ||
|
|
||
| if (shouldRescan && existing.active) { | ||
| updatePayload.active = false; | ||
| updatePayload.scan_status = "pending"; | ||
| updatePayload.flag_summary = null; | ||
| updatePayload.flag_reasons = []; | ||
| updatePayload.flag_severity = null; | ||
| updatePayload.flagged_at = null; | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
|
||
|
|
||
| const { error: updateError } = await supabase | ||
| .from("plugins") | ||
| .update({ | ||
| name, | ||
| description, | ||
| logo: logo || null, | ||
| repository: repository || null, | ||
| homepage: homepage || null, | ||
| keywords: keywords || [], | ||
| }) | ||
| .update(updatePayload) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Delist before components savedHigh Severity · Logic Bug When an install-relevant edit runs on an active plugin, the row is set Reviewed by Cursor Bugbot for commit 1ccb658. Configure here. |
||
| .eq("id", id); | ||
|
|
||
| if (updateError) { | ||
|
|
@@ -117,25 +242,24 @@ export const updatePluginAction = authActionClient | |
| ); | ||
| } | ||
|
|
||
| type ComponentInput = z.infer<typeof componentSchema>; | ||
| const componentRows = components.map( | ||
| (comp: ComponentInput, i: number) => ({ | ||
| plugin_id: id, | ||
| type: comp.type, | ||
| name: comp.name, | ||
| slug: | ||
| comp.slug || | ||
| comp.name | ||
| .toLowerCase() | ||
| .replace(/[^a-z0-9]+/g, "-") | ||
| .replace(/^-+|-+$/g, ""), | ||
| description: comp.description || null, | ||
| content: comp.content || null, | ||
| metadata: comp.metadata || {}, | ||
| sort_order: i, | ||
| }), | ||
| const prevByKey = new Map( | ||
| prevComponents.map((c) => [ | ||
| componentKey(c.type, c.slug || slugify(c.name)), | ||
| c, | ||
| ]), | ||
| ); | ||
|
|
||
| const componentRows = components.map((comp, i) => ({ | ||
| plugin_id: id, | ||
| type: comp.type, | ||
| name: comp.name, | ||
| slug: comp.slug || slugify(comp.name), | ||
| description: comp.description || null, | ||
| content: comp.content || null, | ||
| metadata: resolveComponentMetadata(comp, prevByKey), | ||
| sort_order: i, | ||
| })); | ||
|
|
||
| const { error: compError } = await supabase | ||
| .from("plugin_components") | ||
| .insert(componentRows); | ||
|
|
@@ -145,17 +269,22 @@ export const updatePluginAction = authActionClient | |
| `Failed to save plugin components: ${compError.message}`, | ||
| ); | ||
| } | ||
|
|
||
| try { | ||
| await enqueuePluginScan(id); | ||
| kickDrainAfterResponse(); | ||
| } catch (queueError) { | ||
| console.error("Failed to enqueue plugin scan", queueError); | ||
| if (shouldRescan) { | ||
| try { | ||
| await enqueuePluginScan(id); | ||
| kickDrainAfterResponse(); | ||
| } catch (queueError) { | ||
| console.error("Failed to enqueue plugin scan", queueError); | ||
| } | ||
| } | ||
|
|
||
| revalidatePath("/"); | ||
| revalidatePath(`/plugins/${existing.slug}`); | ||
|
|
||
| return { slug: existing.slug }; | ||
| return { | ||
| slug: existing.slug, | ||
| rescanQueued: shouldRescan, | ||
| repositoryLocked, | ||
| }; | ||
| }, | ||
| ); | ||


Uh oh!
There was an error while loading. Please reload this page.