security audit fix start
This commit is contained in:
@@ -1,12 +1,13 @@
|
||||
import { object, string, url, minLength, optional, picklist } from "valibot";
|
||||
import { object, string, minLength, optional, picklist } from "valibot";
|
||||
import { returnUrlSchema } from "~/lib/url-validation";
|
||||
|
||||
export const CreateCheckoutSessionSchema = object({
|
||||
priceId: string([minLength(1)]),
|
||||
returnUrl: string([url()]),
|
||||
returnUrl: returnUrlSchema,
|
||||
});
|
||||
|
||||
export const CreatePortalSessionSchema = object({
|
||||
returnUrl: string([url()]),
|
||||
returnUrl: returnUrlSchema,
|
||||
});
|
||||
|
||||
export const CancelSubscriptionSchema = object({
|
||||
@@ -28,5 +29,5 @@ export const RequestFeatureTrialSchema = object({
|
||||
|
||||
export const UpgradeFromTrialSchema = object({
|
||||
plan: picklist(["basic", "plus", "premium"]),
|
||||
returnUrl: string([url()]),
|
||||
returnUrl: returnUrlSchema,
|
||||
});
|
||||
|
||||
149
web/src/server/api/schemas/webhook.test.ts
Normal file
149
web/src/server/api/schemas/webhook.test.ts
Normal file
@@ -0,0 +1,149 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { safeParse } from "valibot";
|
||||
import {
|
||||
CheckoutSessionSchema,
|
||||
SubscriptionSchema,
|
||||
InvoiceSchema,
|
||||
} from "./webhook";
|
||||
|
||||
describe("CheckoutSessionSchema", () => {
|
||||
it("accepts valid checkout session data", () => {
|
||||
const data = {
|
||||
id: "cs_test123",
|
||||
subscription: "sub_123",
|
||||
metadata: { userId: "user_123" },
|
||||
};
|
||||
const result = safeParse(CheckoutSessionSchema, data);
|
||||
expect(result.success).toBe(true);
|
||||
if (result.success) {
|
||||
expect(result.output.id).toBe("cs_test123");
|
||||
expect(result.output.metadata?.userId).toBe("user_123");
|
||||
}
|
||||
});
|
||||
|
||||
it("accepts session without optional fields", () => {
|
||||
const data = { id: "cs_test123" };
|
||||
const result = safeParse(CheckoutSessionSchema, data);
|
||||
expect(result.success).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects missing required id", () => {
|
||||
const data = { subscription: "sub_123" };
|
||||
const result = safeParse(CheckoutSessionSchema, data);
|
||||
expect(result.success).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects non-string id", () => {
|
||||
const data = { id: 123 };
|
||||
const result = safeParse(CheckoutSessionSchema, data);
|
||||
expect(result.success).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("SubscriptionSchema", () => {
|
||||
it("accepts valid subscription data with integer timestamps", () => {
|
||||
const data = {
|
||||
id: "sub_123",
|
||||
status: "active",
|
||||
current_period_start: 1700000000,
|
||||
current_period_end: 1702678400,
|
||||
cancel_at_period_end: "false",
|
||||
metadata: { userId: "user_123" },
|
||||
items: {
|
||||
data: { price: { id: "price_basic" } },
|
||||
},
|
||||
};
|
||||
const result = safeParse(SubscriptionSchema, data);
|
||||
expect(result.success).toBe(true);
|
||||
if (result.success) {
|
||||
expect(result.output.current_period_start).toBe(1700000000);
|
||||
expect(result.output.items?.data?.price?.id).toBe("price_basic");
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects non-integer timestamps", () => {
|
||||
const data = {
|
||||
id: "sub_123",
|
||||
current_period_start: "not-a-number",
|
||||
};
|
||||
const result = safeParse(SubscriptionSchema, data);
|
||||
expect(result.success).toBe(false);
|
||||
});
|
||||
|
||||
it("defaults cancel_at_period_end when not provided", () => {
|
||||
const data = { id: "sub_123" };
|
||||
const result = safeParse(SubscriptionSchema, data);
|
||||
expect(result.success).toBe(true);
|
||||
if (result.success) {
|
||||
expect(result.output.cancel_at_period_end).toBe("false");
|
||||
}
|
||||
});
|
||||
|
||||
it("accepts string cancel_at_period_end", () => {
|
||||
const data = { id: "sub_123", cancel_at_period_end: "true" };
|
||||
const result = safeParse(SubscriptionSchema, data);
|
||||
expect(result.success).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects missing required id", () => {
|
||||
const data = { status: "active" };
|
||||
const result = safeParse(SubscriptionSchema, data);
|
||||
expect(result.success).toBe(false);
|
||||
});
|
||||
|
||||
it("handles extra unexpected fields gracefully", () => {
|
||||
const data = {
|
||||
id: "sub_123",
|
||||
status: "active",
|
||||
unknown_field: "should be ignored",
|
||||
};
|
||||
const result = safeParse(SubscriptionSchema, data);
|
||||
expect(result.success).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("InvoiceSchema", () => {
|
||||
it("accepts valid invoice data", () => {
|
||||
const data = { subscription: "sub_123" };
|
||||
const result = safeParse(InvoiceSchema, data);
|
||||
expect(result.success).toBe(true);
|
||||
if (result.success) {
|
||||
expect(result.output.subscription).toBe("sub_123");
|
||||
}
|
||||
});
|
||||
|
||||
it("accepts invoice without subscription (for partial invoices)", () => {
|
||||
const data = { id: "in_123" };
|
||||
const result = safeParse(InvoiceSchema, data);
|
||||
expect(result.success).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects non-string subscription", () => {
|
||||
const data = { subscription: 123 };
|
||||
const result = safeParse(InvoiceSchema, data);
|
||||
expect(result.success).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("Webhook data validation - malformed payloads", () => {
|
||||
it("handles empty object", () => {
|
||||
const result = safeParse(SubscriptionSchema, {});
|
||||
expect(result.success).toBe(false);
|
||||
});
|
||||
|
||||
it("handles completely wrong data shape", () => {
|
||||
const result = safeParse(SubscriptionSchema, "not an object");
|
||||
expect(result.success).toBe(false);
|
||||
});
|
||||
|
||||
it("handles unexpected fields without crashing", () => {
|
||||
const data = {
|
||||
id: "sub_123",
|
||||
status: "active",
|
||||
unknown_field: "should be ignored",
|
||||
another_unknown: 42,
|
||||
};
|
||||
const result = safeParse(SubscriptionSchema, data);
|
||||
expect(result.success).toBe(true);
|
||||
});
|
||||
});
|
||||
56
web/src/server/api/schemas/webhook.ts
Normal file
56
web/src/server/api/schemas/webhook.ts
Normal file
@@ -0,0 +1,56 @@
|
||||
import { object, string, optional, number, type Output } from "valibot";
|
||||
|
||||
/**
|
||||
* Validates a Stripe Checkout Session object from webhook data.
|
||||
*/
|
||||
export const CheckoutSessionSchema = object({
|
||||
id: string(),
|
||||
subscription: optional(string()),
|
||||
metadata: optional(
|
||||
object({
|
||||
userId: optional(string()),
|
||||
}),
|
||||
),
|
||||
});
|
||||
|
||||
/**
|
||||
* Price item inside a Stripe Subscription.
|
||||
*/
|
||||
const PriceItemSchema = object({
|
||||
price: object({
|
||||
id: string(),
|
||||
}),
|
||||
});
|
||||
|
||||
/**
|
||||
* Validates a Stripe Subscription object from webhook data.
|
||||
*/
|
||||
export const SubscriptionSchema = object({
|
||||
id: string(),
|
||||
status: optional(string()),
|
||||
current_period_start: optional(number()),
|
||||
current_period_end: optional(number()),
|
||||
cancel_at_period_end: optional(string(), "false"),
|
||||
metadata: optional(
|
||||
object({
|
||||
userId: optional(string()),
|
||||
}),
|
||||
),
|
||||
items: optional(
|
||||
object({
|
||||
data: optional(PriceItemSchema),
|
||||
}),
|
||||
),
|
||||
});
|
||||
|
||||
/**
|
||||
* Validates a Stripe Invoice object from webhook data.
|
||||
*/
|
||||
export const InvoiceSchema = object({
|
||||
subscription: optional(string()),
|
||||
});
|
||||
|
||||
// Type exports for use in billing.service.ts
|
||||
export type ValidatedCheckoutSession = Output<typeof CheckoutSessionSchema>;
|
||||
export type ValidatedSubscription = Output<typeof SubscriptionSchema>;
|
||||
export type ValidatedInvoice = Output<typeof InvoiceSchema>;
|
||||
85
web/src/server/api/utils.test.ts
Normal file
85
web/src/server/api/utils.test.ts
Normal file
@@ -0,0 +1,85 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
|
||||
/**
|
||||
* Mirrors the SENSITIVE_PROCEDURES Set from utils.ts
|
||||
*/
|
||||
const SENSITIVE_PROCEDURES = new Set([
|
||||
"user.login",
|
||||
"user.signup",
|
||||
"user.forgotPassword",
|
||||
"user.resetPassword",
|
||||
"darkwatch.runScan",
|
||||
"darkwatch.runFullScan",
|
||||
"voiceprint.analyzeAudio",
|
||||
"voiceprint.createEnrollment",
|
||||
]);
|
||||
|
||||
function getRateLimitTier(path: string, userRole: string | null, hasUser: boolean): "sensitive" | "authenticated" | "public" | "admin" {
|
||||
if (userRole === "admin") return "admin";
|
||||
if (SENSITIVE_PROCEDURES.has(path)) return "sensitive";
|
||||
return hasUser ? "authenticated" : "public";
|
||||
}
|
||||
|
||||
describe("Rate limiter exact matching", () => {
|
||||
describe("sensitive procedures", () => {
|
||||
it("matches auth procedures", () => {
|
||||
expect(getRateLimitTier("user.login", null, true)).toBe("sensitive");
|
||||
expect(getRateLimitTier("user.signup", null, true)).toBe("sensitive");
|
||||
expect(getRateLimitTier("user.forgotPassword", null, true)).toBe("sensitive");
|
||||
expect(getRateLimitTier("user.resetPassword", null, true)).toBe("sensitive");
|
||||
});
|
||||
|
||||
it("matches darkwatch procedures", () => {
|
||||
expect(getRateLimitTier("darkwatch.runScan", null, true)).toBe("sensitive");
|
||||
expect(getRateLimitTier("darkwatch.runFullScan", null, true)).toBe("sensitive");
|
||||
});
|
||||
|
||||
it("matches voiceprint procedures", () => {
|
||||
expect(getRateLimitTier("voiceprint.analyzeAudio", null, true)).toBe("sensitive");
|
||||
expect(getRateLimitTier("voiceprint.createEnrollment", null, true)).toBe("sensitive");
|
||||
});
|
||||
});
|
||||
|
||||
describe("non-sensitive procedures", () => {
|
||||
it("returns authenticated tier for normal procedures", () => {
|
||||
expect(getRateLimitTier("blog.bySlug", null, true)).toBe("authenticated");
|
||||
expect(getRateLimitTier("correlation.search", null, true)).toBe("authenticated");
|
||||
expect(getRateLimitTier("spamshield.analyze", null, true)).toBe("authenticated");
|
||||
});
|
||||
|
||||
it("returns public tier for unauthenticated users", () => {
|
||||
expect(getRateLimitTier("blog.bySlug", null, false)).toBe("public");
|
||||
});
|
||||
|
||||
it("returns admin tier for admin users regardless of procedure", () => {
|
||||
expect(getRateLimitTier("user.login", "admin", true)).toBe("admin");
|
||||
expect(getRateLimitTier("darkwatch.runScan", "admin", true)).toBe("admin");
|
||||
expect(getRateLimitTier("voiceprint.analyzeAudio", "admin", true)).toBe("admin");
|
||||
});
|
||||
});
|
||||
|
||||
describe("substring bypass prevention", () => {
|
||||
it("does not match substring attacks on auth procedures", () => {
|
||||
// These should NOT be sensitive (substring match would incorrectly flag them)
|
||||
expect(getRateLimitTier("user.loginLike", null, true)).toBe("authenticated");
|
||||
expect(getRateLimitTier("user.signupPage", null, true)).toBe("authenticated");
|
||||
expect(getRateLimitTier("user.loginResetPassword", null, true)).toBe("authenticated");
|
||||
});
|
||||
|
||||
it("does not match substring attacks on darkwatch", () => {
|
||||
expect(getRateLimitTier("darkwatch.runScanLike", null, true)).toBe("authenticated");
|
||||
expect(getRateLimitTier("darkwatch.runScanHistory", null, true)).toBe("authenticated");
|
||||
});
|
||||
|
||||
it("does not match substring attacks on voiceprint", () => {
|
||||
expect(getRateLimitTier("voiceprint.analyzeAudioPlayer", null, true)).toBe("authenticated");
|
||||
expect(getRateLimitTier("voiceprint.createEnrollmentPage", null, true)).toBe("authenticated");
|
||||
});
|
||||
|
||||
it("does not match partial path segments", () => {
|
||||
expect(getRateLimitTier("notdarkwatch.runScan", null, true)).toBe("authenticated");
|
||||
expect(getRateLimitTier("darkwatch.notrunScan", null, true)).toBe("authenticated");
|
||||
expect(getRateLimitTier("voiceprint.analyze", null, true)).toBe("authenticated");
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -36,9 +36,19 @@ const isRateLimited = t.middleware(async ({ ctx, next, path }) => {
|
||||
const identifier = ctx.user?.id ?? ctx.apiKey ?? "anonymous";
|
||||
const tier = ctx.user?.role === "admin" ? "admin" : ctx.user ? "authenticated" : "public";
|
||||
|
||||
// Sensitive operations get stricter limits
|
||||
const sensitivePaths = ["login", "signup", "forgotPassword", "resetPassword"];
|
||||
const effectiveTier = sensitivePaths.some((p) => path.includes(p)) ? "sensitive" : tier;
|
||||
// Sensitive operations get stricter limits (exact match to prevent bypass)
|
||||
const SENSITIVE_PROCEDURES = new Set([
|
||||
"user.login",
|
||||
"user.signup",
|
||||
"user.forgotPassword",
|
||||
"user.resetPassword",
|
||||
"darkwatch.runScan",
|
||||
"darkwatch.runFullScan",
|
||||
"voiceprint.analyzeAudio",
|
||||
"voiceprint.createEnrollment",
|
||||
]);
|
||||
|
||||
const effectiveTier = SENSITIVE_PROCEDURES.has(path) ? "sensitive" : tier;
|
||||
|
||||
await checkRateLimitOrThrow(identifier, effectiveTier);
|
||||
return next();
|
||||
|
||||
Reference in New Issue
Block a user