Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,23 @@

All notable changes to this project will be documented in this file.

## [1.0.0-alpha.18] - 2026-05-06

### Security
- **#33** Fix missing rate-limit import in `/api/check-tokens` (would have bypassed DoS protection at runtime)
- **#35** Replace `createRequire` with native ESM import for `pdf-parse` (restores tree-shaking)
- **#36** Unify admin authorization in `getRequestUserContext` to delegate to `isAdminUser` (NFKC normalization + `admin_emails` table lookup); secondary admins were silently losing privileges in `/api/analyses` endpoints

### Performance
- **#34** Cancel stale `/api/check-tokens` requests on rapid file selection in dashboard via `AbortController`

### Tests / Tooling
- **#37** Split vitest into `unit` and `integration` projects so the `#supabase/server` mock alias only applies to unit tests; adds `npm run test:unit` and `npm run test:integration`
- New unit tests for `getRequestUserContext` admin paths
- Extract `isAdminEmail(event, email)` helper to avoid redundant `auth.getUser()` round-trip

---

## [1.0.0-alpha.17] - 2026-05-06

### Changed
Expand Down
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "clarify",
"version": "1.0.0-alpha.17",
"version": "1.0.0-alpha.18",
"private": true,
"type": "module",
"scripts": {
Expand All @@ -14,6 +14,8 @@
"test": "vitest",
"test:ui": "vitest --ui",
"test:run": "vitest run",
"test:unit": "vitest run --project unit",
"test:integration": "vitest run --project integration",
"test:e2e": "playwright test",
"test:e2e:ui": "playwright test --ui",
"security:audit": "npm audit --audit-level=high",
Expand Down
26 changes: 24 additions & 2 deletions pages/dashboard.vue
Original file line number Diff line number Diff line change
Expand Up @@ -1226,10 +1226,21 @@ const handleDropzoneError = (message: string) => {
analyzeError.value = message;
};

// Cancellation guard for in-flight token checks. If the user uploads a
// new file before the previous /api/check-tokens response arrives, we
// abort the stale request so its result can't overwrite the fresh one.
let tokenCheckAbortController: AbortController | null = null;

// Handle uploaded event from Dropzone - file was uploaded successfully
const handleDropzoneUploaded = async (data: { file_url: string }) => {
if (!data.file_url) return;

if (tokenCheckAbortController) {
tokenCheckAbortController.abort();
}
const controller = new AbortController();
tokenCheckAbortController = controller;

checkingTokens.value = true;
analyzeError.value = "";

Expand All @@ -1245,15 +1256,21 @@ const handleDropzoneUploaded = async (data: { file_url: string }) => {
// Check Tokens
const tokenResponse = await $fetch<any>("/api/check-tokens", {
method: "POST",
body: { file_url: uploadedFileUrl.value },
body: { file_url: data.file_url },
signal: controller.signal,
});

if (controller.signal.aborted) return;

if (!tokenResponse.success) {
throw new Error(tokenResponse.error || "Error calculating tokens");
}

tokenCheckResult.value = tokenResponse;
} catch (error: any) {
if (controller.signal.aborted || error?.name === "AbortError") {
return;
}
// Extract error message from h3/Nuxt error structure
const errorMessage =
error.data?.message ||
Expand All @@ -1265,7 +1282,12 @@ const handleDropzoneUploaded = async (data: { file_url: string }) => {
uploadedFileUrl.value = "";
tokenCheckResult.value = null;
} finally {
checkingTokens.value = false;
if (tokenCheckAbortController === controller) {
tokenCheckAbortController = null;
}
if (!controller.signal.aborted) {
checkingTokens.value = false;
}
}
};

Expand Down
1 change: 1 addition & 0 deletions server/api/check-tokens.post.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { preprocessDocument } from "~/server/utils/preprocessing";
import { getPromptConfig } from "~/server/utils/config";
import { validateSupabaseStorageUrl } from "~/server/utils/ssrf-protection";
import { handleApiError } from "~/server/utils/error-handler";
import { applyRateLimit, RateLimitPresets } from "~/server/utils/rate-limit";

export default defineEventHandler(async (event) => {
let user;
Expand Down
8 changes: 5 additions & 3 deletions server/utils/analysis-security.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
*/

import type { Analysis } from "~/types";
import { isAdminEmail } from "./auth";

/**
* Strip sensitive debug info from analysis summary
Expand Down Expand Up @@ -97,9 +98,10 @@ export async function getRequestUserContext(event: any) {
};
}

// Check admin status by comparing with admin email from runtime config
const config = useRuntimeConfig(event);
const isAdmin = user.email === config.adminEmail;
// [SECURITY FIX H1] Delegate admin check to single source of truth
// (case/Unicode-normalized comparison + admin_emails table lookup).
// Pass the already-fetched email to avoid a redundant auth.getUser() call.
const isAdmin = user.email ? await isAdminEmail(event, user.email) : false;

return {
userId: user.id,
Expand Down
19 changes: 17 additions & 2 deletions server/utils/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,26 @@ export async function isAdminUser(event: H3Event): Promise<boolean> {
return false;
}

return isAdminEmail(event, user.email);
}

/**
* Same admin check as isAdminUser, but accepts an already-resolved email
* to avoid a redundant auth.getUser() round-trip when the caller has just
* fetched the user themselves.
*/
export async function isAdminEmail(
event: H3Event,
email: string,
): Promise<boolean> {
if (!email) {
return false;
}

const config = useRuntimeConfig();
const adminEmailConfig = config.adminEmail;

// Normalize user's email for comparison
const userEmail = normalizeEmail(user.email);
const userEmail = normalizeEmail(email);

// Check 1: Compare against config.adminEmail (normalized)
if (adminEmailConfig) {
Expand Down
4 changes: 1 addition & 3 deletions server/utils/pdf-parser.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import { createRequire } from "module";
const require = createRequire(import.meta.url);
const { PDFParse } = require("pdf-parse");
import { PDFParse } from "pdf-parse";

export const extractTextFromPDF = async (
fileBuffer: Buffer,
Expand Down
64 changes: 64 additions & 0 deletions tests/unit/server/utils/analysis-security.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { describe, it, expect, vi, beforeEach } from 'vitest'
import { getRequestUserContext } from '@/server/utils/analysis-security'
import type { H3Event } from 'h3'

const mockClient = vi.fn()
const mockRuntimeConfig = vi.fn()

vi.mock('#supabase/server', () => ({
serverSupabaseClient: (...args: any[]) => mockClient(...args),
}))

vi.stubGlobal('useRuntimeConfig', mockRuntimeConfig)

function clientWith(user: { email: string | null } | null, adminTableMatch = false) {
const maybeSingle = vi.fn().mockResolvedValue({
data: adminTableMatch ? { email: user?.email } : null,
error: null,
})
return {
auth: { getUser: vi.fn().mockResolvedValue({ data: { user } }) },
from: vi.fn().mockReturnValue({
select: vi.fn().mockReturnValue({
eq: vi.fn().mockReturnValue({
eq: vi.fn().mockReturnValue({ maybeSingle }),
}),
}),
}),
}
}

describe('getRequestUserContext (security #36)', () => {
let event: H3Event
beforeEach(() => {
vi.clearAllMocks()
event = {} as H3Event
mockRuntimeConfig.mockReturnValue({ adminEmail: 'admin@clarify.com' })
})

it('returns unauthenticated context when no user', async () => {
mockClient.mockResolvedValue(clientWith(null))
const ctx = await getRequestUserContext(event)
expect(ctx).toEqual({ userId: null, email: null, isAdmin: false, authenticated: false })
})

it('flags admin via config.adminEmail (case-insensitive)', async () => {
mockClient.mockResolvedValue(clientWith({ email: 'ADMIN@clarify.com' } as any))
const ctx = await getRequestUserContext(event)
expect(ctx.authenticated).toBe(true)
expect(ctx.isAdmin).toBe(true)
})

it('flags admin via admin_emails table when config does not match', async () => {
mockClient.mockResolvedValue(clientWith({ email: 'secondary@clarify.com' } as any, true))
const ctx = await getRequestUserContext(event)
expect(ctx.isAdmin).toBe(true)
})

it('returns isAdmin=false for non-admin users', async () => {
mockClient.mockResolvedValue(clientWith({ email: 'random@user.com' } as any))
const ctx = await getRequestUserContext(event)
expect(ctx.authenticated).toBe(true)
expect(ctx.isAdmin).toBe(false)
})
})
65 changes: 50 additions & 15 deletions vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,59 @@ import { defineConfig } from "vitest/config";
import { resolve } from "node:path";
import vue from "@vitejs/plugin-vue";

const root = resolve(__dirname, ".");

const sharedAlias = {
"~": root,
"@": root,
};

const sharedTest = {
environment: "happy-dom" as const,
globals: true,
setupFiles: [resolve(root, "tests/setup.ts")],
server: {
deps: {
inline: ["@nuxt/test-utils"],
},
},
};

export default defineConfig({
plugins: [vue()] as any,
test: {
environment: "happy-dom",
include: ["tests/**/*.test.ts", "tests/**/*.spec.ts"],
globals: true,
setupFiles: ["./tests/setup.ts"],
server: {
deps: {
inline: ["@nuxt/test-utils"],
projects: [
{
plugins: [vue()] as any,
test: {
...sharedTest,
name: "unit",
include: ["tests/unit/**/*.{test,spec}.ts"],
},
resolve: {
alias: {
...sharedAlias,
// Mock the Nuxt-injected Supabase server helper. Unit tests stub
// auth/DB responses β€” they do not touch a real Supabase instance.
"#supabase/server": resolve(root, "./tests/mocks/supabase.ts"),
},
},
},
},
},
resolve: {
alias: {
"~": resolve(__dirname, "."),
"@": resolve(__dirname, "."),
"#supabase/server": resolve(__dirname, "./tests/mocks/supabase.ts"),
},
{
plugins: [vue()] as any,
test: {
...sharedTest,
name: "integration",
include: ["tests/integration/**/*.{test,spec}.ts"],
},
resolve: {
alias: sharedAlias,
// No #supabase/server alias here. Integration tests must either
// mock @supabase/supabase-js explicitly (current pattern) or run
// against a real Supabase instance via env vars. Falsely aliasing
// the helper would silently turn integration tests into unit tests.
},
},
],
},
});
Loading