Fix security audit findings: auth scoping, OIDC hardening, CSP, file download
C3: Budget allocation now verifies project belongs to company M4: Expense approve/reject scoped by company via project join H2: OIDC cookies get secure flag on HTTPS H3: OIDC auto-link only when email_verified by provider H4: Content-Security-Policy + X-Content-Type-Options in hooks M7: SSRF favicon redirect depth capped at 3 M2: File downloads use attachment disposition (not inline) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
+9
-1
@@ -30,5 +30,13 @@ export const handle: Handle = async ({ event, resolve }) => {
|
||||
event.locals.session = null;
|
||||
}
|
||||
|
||||
return resolve(event);
|
||||
const response = await resolve(event);
|
||||
|
||||
response.headers.set(
|
||||
'Content-Security-Policy',
|
||||
"default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self'; connect-src 'self'; frame-ancestors 'none'"
|
||||
);
|
||||
response.headers.set('X-Content-Type-Options', 'nosniff');
|
||||
|
||||
return response;
|
||||
};
|
||||
|
||||
@@ -117,7 +117,7 @@ export async function exchangeCode(
|
||||
|
||||
export async function getUserInfo(
|
||||
accessToken: string
|
||||
): Promise<{ sub: string; email: string; name?: string }> {
|
||||
): Promise<{ sub: string; email: string; name?: string; email_verified?: boolean }> {
|
||||
const config = await getOIDCConfig();
|
||||
|
||||
const res = await fetch(config.userinfoEndpoint, {
|
||||
|
||||
@@ -74,7 +74,8 @@ async function resolvePublicIp(hostname: string): Promise<string> {
|
||||
return ips[0];
|
||||
}
|
||||
|
||||
async function safeFetch(targetUrl: URL): Promise<Response | null> {
|
||||
async function safeFetch(targetUrl: URL, depth = 0): Promise<Response | null> {
|
||||
if (depth > 3) return null;
|
||||
if (targetUrl.protocol !== 'http:' && targetUrl.protocol !== 'https:') return null;
|
||||
try {
|
||||
await resolvePublicIp(targetUrl.hostname);
|
||||
@@ -109,7 +110,7 @@ async function safeFetch(targetUrl: URL): Promise<Response | null> {
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
return safeFetch(next);
|
||||
return safeFetch(next, depth + 1);
|
||||
}
|
||||
return res;
|
||||
} catch {
|
||||
|
||||
@@ -9,7 +9,7 @@ import {
|
||||
expenses,
|
||||
companyLog
|
||||
} from '$lib/server/db/schema.js';
|
||||
import { eq, sql } from 'drizzle-orm';
|
||||
import { and, eq, sql } from 'drizzle-orm';
|
||||
import { requireCompanyRole } from '$lib/server/authorization.js';
|
||||
import { logCompanyEvent } from '$lib/server/audit.js';
|
||||
import { formatCurrency } from '$lib/utils/currency.js';
|
||||
@@ -118,12 +118,13 @@ export const actions: Actions = {
|
||||
return fail(400, { error: 'Project and non-zero amount are required' });
|
||||
}
|
||||
|
||||
// Get project name and company currency for the log
|
||||
// Verify project belongs to this company
|
||||
const [project] = await db
|
||||
.select({ name: projects.name })
|
||||
.from(projects)
|
||||
.where(eq(projects.id, projectId))
|
||||
.where(and(eq(projects.id, projectId), eq(projects.companyId, params.companyId)))
|
||||
.limit(1);
|
||||
if (!project) return fail(400, { error: 'Project not found in this company' });
|
||||
|
||||
const [company] = await db
|
||||
.select({ currency: companies.currency })
|
||||
@@ -137,7 +138,7 @@ export const actions: Actions = {
|
||||
allocatedBudget: sql`${projects.allocatedBudget}::numeric + ${amount.toFixed(2)}::numeric`,
|
||||
updatedAt: new Date()
|
||||
})
|
||||
.where(eq(projects.id, projectId));
|
||||
.where(and(eq(projects.id, projectId), eq(projects.companyId, params.companyId)));
|
||||
|
||||
await db.insert(budgetAllocations).values({
|
||||
companyId: params.companyId,
|
||||
|
||||
+3
-2
@@ -41,8 +41,9 @@ export const GET: RequestHandler = async ({ locals, params }) => {
|
||||
|
||||
return new Response(new Blob([buf as BlobPart], { type: row.mimeType }), {
|
||||
headers: {
|
||||
'Content-Disposition': `inline; filename="${safeName}"`,
|
||||
'Cache-Control': 'private, no-store'
|
||||
'Content-Disposition': `attachment; filename="${safeName}"`,
|
||||
'Cache-Control': 'private, no-store',
|
||||
'X-Content-Type-Options': 'nosniff'
|
||||
}
|
||||
});
|
||||
};
|
||||
|
||||
@@ -94,8 +94,10 @@ export const actions: Actions = {
|
||||
accountId: expenses.accountId
|
||||
})
|
||||
.from(expenses)
|
||||
.where(eq(expenses.id, expenseId))
|
||||
.innerJoin(projects, eq(expenses.projectId, projects.id))
|
||||
.where(and(eq(expenses.id, expenseId), eq(projects.companyId, params.companyId)))
|
||||
.limit(1);
|
||||
if (!expense) return fail(404, { error: 'Expense not found' });
|
||||
|
||||
await db.transaction(async (tx) => {
|
||||
await tx
|
||||
@@ -137,8 +139,10 @@ export const actions: Actions = {
|
||||
const [expense] = await db
|
||||
.select({ title: expenses.title, amount: expenses.amount, currency: expenses.currency })
|
||||
.from(expenses)
|
||||
.where(eq(expenses.id, expenseId))
|
||||
.innerJoin(projects, eq(expenses.projectId, projects.id))
|
||||
.where(and(eq(expenses.id, expenseId), eq(projects.companyId, params.companyId)))
|
||||
.limit(1);
|
||||
if (!expense) return fail(404, { error: 'Expense not found' });
|
||||
|
||||
await db.transaction(async (tx) => {
|
||||
await tx
|
||||
|
||||
@@ -7,7 +7,8 @@ import {
|
||||
isOIDCEnabled
|
||||
} from '$lib/server/auth/oidc.js';
|
||||
|
||||
export const GET: RequestHandler = async ({ cookies }) => {
|
||||
export const GET: RequestHandler = async ({ cookies, url: reqUrl }) => {
|
||||
const isSecure = reqUrl.protocol === 'https:';
|
||||
if (!isOIDCEnabled()) {
|
||||
redirect(302, '/login');
|
||||
}
|
||||
@@ -17,6 +18,7 @@ export const GET: RequestHandler = async ({ cookies }) => {
|
||||
|
||||
cookies.set('oidc_state', state, {
|
||||
httpOnly: true,
|
||||
secure: isSecure,
|
||||
sameSite: 'lax',
|
||||
path: '/',
|
||||
maxAge: 600 // 10 minutes
|
||||
@@ -24,6 +26,7 @@ export const GET: RequestHandler = async ({ cookies }) => {
|
||||
|
||||
cookies.set('oidc_code_verifier', codeVerifier, {
|
||||
httpOnly: true,
|
||||
secure: isSecure,
|
||||
sameSite: 'lax',
|
||||
path: '/',
|
||||
maxAge: 600
|
||||
|
||||
@@ -45,8 +45,8 @@ export const GET: RequestHandler = async (event) => {
|
||||
.then((r) => r[0] ?? null);
|
||||
|
||||
if (!user) {
|
||||
// Check if a user with this email exists (link accounts)
|
||||
if (userInfo.email) {
|
||||
// Check if a user with this email exists — only auto-link if provider verified the email
|
||||
if (userInfo.email && userInfo.email_verified) {
|
||||
user = await db
|
||||
.select()
|
||||
.from(users)
|
||||
@@ -55,7 +55,7 @@ export const GET: RequestHandler = async (event) => {
|
||||
.then((r) => r[0] ?? null);
|
||||
|
||||
if (user) {
|
||||
// Link OIDC identity to existing user
|
||||
// Link OIDC identity to existing user (email verified by provider)
|
||||
await db
|
||||
.update(users)
|
||||
.set({
|
||||
|
||||
Reference in New Issue
Block a user