Skip to content

Commit a117a06

Browse files
siracusa5claude
andauthored
fix: close stored XSS, anon RPC, and rate limiting gaps in marketplace (#32)
* security: close stored XSS, anon RPC, and rate limiting gaps in marketplace - Replace raw innerHTML rendering with sanitize-html (allowlist-based) in plugin detail page — blocks stored XSS via community plugin markdown - Add marketplace/lib/rate-limit.ts: in-memory sliding-window limiter - Rate limit /api/install (5/slug/IP/hr), /api/search (60/IP/min), /api/register (10/IP/hr) with 429 + Retry-After responses - Add migration 00003: REVOKE EXECUTE on increment_install_count from anon + authenticated roles (migration applied to production) - Add *.tsbuildinfo to .gitignore Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(marketplace): move rate limit before auth check in /api/register Rate limiting must run before authentication so that failed auth attempts increment the counter — otherwise an attacker can brute-force the API key without ever hitting the rate limit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c2bfa6c commit a117a06

9 files changed

Lines changed: 251 additions & 11 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ website/.source/
2323
# Marketplace
2424
marketplace/.next/
2525
marketplace/out/
26+
*.tsbuildinfo
2627

2728
# Supabase local
2829
marketplace/supabase/.temp/

marketplace/app/api/install/route.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { NextRequest, NextResponse } from "next/server";
22
import { createClient } from "@supabase/supabase-js";
3+
import { checkRateLimit, getClientIp } from "@/lib/rate-limit";
34

45
function getServiceSupabase() {
56
const url = process.env.NEXT_PUBLIC_SUPABASE_URL;
@@ -33,6 +34,19 @@ export async function POST(request: NextRequest) {
3334
);
3435
}
3536

37+
// 5 installs per slug per IP per hour
38+
const ip = getClientIp(request);
39+
const { allowed, retryAfter } = checkRateLimit(`install:${ip}:${slug}`, {
40+
maxRequests: 5,
41+
windowMs: 60 * 60 * 1000,
42+
});
43+
if (!allowed) {
44+
return NextResponse.json(
45+
{ error: "Too many requests" },
46+
{ status: 429, headers: { "Retry-After": String(retryAfter) } },
47+
);
48+
}
49+
3650
// Atomically increment install_count to avoid race conditions
3751
const { data: updated, error: updateError } = await supabase
3852
.rpc("increment_install_count", { component_slug: slug });

marketplace/app/api/register/route.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { NextRequest, NextResponse } from "next/server";
22
import { createClient } from "@supabase/supabase-js";
3+
import { checkRateLimit, getClientIp } from "@/lib/rate-limit";
34

45
function getServiceSupabase() {
56
const url = process.env.NEXT_PUBLIC_SUPABASE_URL;
@@ -49,6 +50,20 @@ async function fetchGitHubFile(
4950
}
5051

5152
export async function POST(request: NextRequest) {
53+
// Rate limit before auth so brute-force attempts are counted regardless of
54+
// whether the key is correct. 10 requests per hour per IP.
55+
const ip = getClientIp(request);
56+
const { allowed, retryAfter } = checkRateLimit(`register:${ip}`, {
57+
maxRequests: 10,
58+
windowMs: 60 * 60 * 1000,
59+
});
60+
if (!allowed) {
61+
return NextResponse.json(
62+
{ error: "Too many requests" },
63+
{ status: 429, headers: { "Retry-After": String(retryAfter) } },
64+
);
65+
}
66+
5267
// Require API key for registration to prevent spam
5368
const apiKey = process.env.REGISTER_API_KEY;
5469
if (!apiKey) {

marketplace/app/api/search/route.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { NextRequest, NextResponse } from "next/server";
22
import { supabase } from "@/lib/supabase";
3+
import { checkRateLimit, getClientIp } from "@/lib/rate-limit";
34

45
type SearchType = "component" | "profile";
56

@@ -15,6 +16,19 @@ export async function GET(request: NextRequest) {
1516
);
1617
}
1718

19+
// 60 searches per minute per IP
20+
const ip = getClientIp(request);
21+
const { allowed, retryAfter } = checkRateLimit(`search:${ip}`, {
22+
maxRequests: 60,
23+
windowMs: 60 * 1000,
24+
});
25+
if (!allowed) {
26+
return NextResponse.json(
27+
{ error: "Too many requests" },
28+
{ status: 429, headers: { "Retry-After": String(retryAfter) } },
29+
);
30+
}
31+
1832
// Sanitize and convert query to tsquery format for full-text search
1933
const sanitizedTokens = query
2034
.trim()

marketplace/app/plugins/[slug]/page.tsx

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import Link from "next/link";
22
import { notFound } from "next/navigation";
3+
import sanitizeHtml from "sanitize-html";
34
import { supabase } from "@/lib/supabase";
45
import type { Component, Profile, TrustTier } from "@/lib/types";
56

@@ -18,9 +19,24 @@ function TrustBadge({ tier }: { tier: TrustTier }) {
1819
);
1920
}
2021

22+
/**
23+
* Allowed tags and attributes for sanitizeHtml.
24+
* Permits the Tailwind classes emitted by renderMarkdown while blocking all
25+
* event handlers and javascript: URLs.
26+
*/
27+
const SANITIZE_OPTIONS: sanitizeHtml.IOptions = {
28+
allowedTags: ["h1", "h2", "h3", "h4", "p", "pre", "code", "strong", "a", "li", "ul"],
29+
allowedAttributes: {
30+
"*": ["class"],
31+
a: ["href", "target", "rel"],
32+
},
33+
allowedSchemes: ["http", "https", "mailto"],
34+
};
35+
2136
/**
2237
* Minimal server-side markdown-to-HTML renderer.
23-
* Content is trusted (from our own Supabase database, not user input).
38+
* Output is sanitized via sanitizeHtml before rendering — community plugins
39+
* store arbitrary markdown from GitHub repos which is untrusted content.
2440
*/
2541
function renderMarkdown(md: string): string {
2642
const html = md
@@ -138,12 +154,11 @@ export default async function PluginDetailPage({
138154
notFound();
139155
}
140156

141-
// Trusted content from our own database -- see renderMarkdown comment above
142157
const skillHtml = component.skill_md
143-
? renderMarkdown(component.skill_md)
158+
? sanitizeHtml(renderMarkdown(component.skill_md), SANITIZE_OPTIONS)
144159
: null;
145160
const readmeHtml = component.readme_md
146-
? renderMarkdown(component.readme_md)
161+
? sanitizeHtml(renderMarkdown(component.readme_md), SANITIZE_OPTIONS)
147162
: null;
148163

149164
const updatedDate = component.updated_at
@@ -220,7 +235,7 @@ export default async function PluginDetailPage({
220235
</div>
221236
)}
222237

223-
{/* SKILL.md content -- trusted content from our own Supabase database */}
238+
{/* SKILL.md content */}
224239
{skillHtml && (
225240
<section className="mb-10">
226241
<h2 className="mb-4 text-xl font-bold">Skill Definition</h2>
@@ -231,7 +246,7 @@ export default async function PluginDetailPage({
231246
</section>
232247
)}
233248

234-
{/* README.md content -- trusted content from our own Supabase database */}
249+
{/* README.md content */}
235250
{readmeHtml && (
236251
<section className="mb-10">
237252
<h2 className="mb-4 text-xl font-bold">Documentation</h2>

marketplace/lib/rate-limit.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
interface RateLimitEntry {
2+
count: number;
3+
resetAt: number;
4+
}
5+
6+
const store = new Map<string, RateLimitEntry>();
7+
8+
export interface RateLimitConfig {
9+
/** Maximum number of requests allowed within the window. */
10+
maxRequests: number;
11+
/** Window duration in milliseconds. */
12+
windowMs: number;
13+
}
14+
15+
/**
16+
* In-memory sliding-window rate limiter.
17+
*
18+
* Keyed by an arbitrary string (e.g. "install:<ip>:<slug>"). Expired entries
19+
* are pruned on each call so the Map doesn't grow unbounded.
20+
*
21+
* NOTE: This is per-process. In a multi-instance deployment use an external
22+
* store (Redis / Cloudflare KV). Sufficient for a single-instance launch.
23+
*/
24+
export function checkRateLimit(
25+
key: string,
26+
config: RateLimitConfig,
27+
): { allowed: boolean; retryAfter?: number } {
28+
const now = Date.now();
29+
30+
// Prune expired entries to prevent memory growth.
31+
for (const [k, entry] of store.entries()) {
32+
if (entry.resetAt <= now) store.delete(k);
33+
}
34+
35+
const entry = store.get(key);
36+
37+
if (!entry || entry.resetAt <= now) {
38+
store.set(key, { count: 1, resetAt: now + config.windowMs });
39+
return { allowed: true };
40+
}
41+
42+
if (entry.count >= config.maxRequests) {
43+
return {
44+
allowed: false,
45+
retryAfter: Math.ceil((entry.resetAt - now) / 1000),
46+
};
47+
}
48+
49+
entry.count++;
50+
return { allowed: true };
51+
}
52+
53+
/**
54+
* Extract the client IP from a request, preferring Cloudflare headers.
55+
*/
56+
export function getClientIp(request: Request): string {
57+
return (
58+
request.headers.get("cf-connecting-ip") ??
59+
request.headers.get("x-forwarded-for")?.split(",")[0]?.trim() ??
60+
"unknown"
61+
);
62+
}

marketplace/package.json

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,21 @@
88
"start": "next start"
99
},
1010
"dependencies": {
11+
"@harness-kit/shared": "workspace:*",
12+
"@supabase/supabase-js": "^2.0.0",
1113
"next": "^15.0.0",
1214
"react": "^19.0.0",
1315
"react-dom": "^19.0.0",
14-
"@supabase/supabase-js": "^2.0.0",
15-
"@harness-kit/shared": "workspace:*"
16+
"sanitize-html": "^2.17.1"
1617
},
1718
"devDependencies": {
19+
"@tailwindcss/postcss": "^4.0.0",
1820
"@types/node": "^22.0.0",
1921
"@types/react": "^19.0.0",
2022
"@types/react-dom": "^19.0.0",
21-
"typescript": "^5.0.0",
23+
"@types/sanitize-html": "^2.16.1",
24+
"postcss": "^8.0.0",
2225
"tailwindcss": "^4.0.0",
23-
"@tailwindcss/postcss": "^4.0.0",
24-
"postcss": "^8.0.0"
26+
"typescript": "^5.0.0"
2527
}
2628
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
-- Revoke direct RPC access to increment_install_count from unprivileged roles.
2+
-- The /api/install route uses service_role which bypasses grants and continues
3+
-- to work. Direct calls via the anon or authenticated key (e.g., from the
4+
-- Supabase client SDK) are now blocked, preventing install-count manipulation.
5+
REVOKE EXECUTE ON FUNCTION increment_install_count(text) FROM anon;
6+
REVOKE EXECUTE ON FUNCTION increment_install_count(text) FROM authenticated;

0 commit comments

Comments
 (0)