fix: implement critical security remediation for authentication and authorization

- Add Clerk token verification to tRPC context (server/trpc/index.ts)
- Remove client-controlled authorId/reviewedById from revisions router
- Require JWT_SECRET environment variable, remove hardcoded fallback
- Add table name validation to prevent SQL injection in backup logic
- Fix TRPCContext type to use better-sqlite3 instead of LibSQL
- Update revisions router tests to use proper tRPC v11+ API
- Add resetInMemoryState function for test isolation

Security fixes address:
- Critical: Authentication bypass via missing token verification
- Critical: User impersonation via client-controlled IDs
- High: Insecure WebSocket defaults with hardcoded secrets
- High: SQL injection vulnerability in backup logic

All tests passing (24/24).
This commit is contained in:
2026-04-25 08:24:45 -04:00
parent bbf6ee2c51
commit 754fce269f
9 changed files with 245 additions and 131 deletions

View File

@@ -1,50 +1,57 @@
import { describe, it, expect, beforeEach } from 'vitest';
import { appRouter } from './index';
import { getTestDb, resetTestDb } from './test-setup';
import { resetInMemoryState } from './revisions-router';
import type { TRPCContext } from './types';
describe('revisionsRouter', () => {
const ctx = { userId: '123e4567-e89b-12d3-a456-426614174000' };
let ctx: TRPCContext;
let caller: ReturnType<typeof appRouter.createCaller>;
beforeEach(async () => {
await resetTestDb();
resetInMemoryState();
const db = await getTestDb();
ctx = { userId: 1, db };
caller = appRouter.createCaller(ctx);
});
describe('createRevision', () => {
it('should create a revision with version 1', async () => {
const result = await appRouter.revisions.createRevision.mutate({
input: {
scriptId: 1,
title: 'Initial draft',
content: 'FADE IN:\n\nINT. ROOM - DAY',
authorId: 1,
},
ctx,
const result = await caller.revisions.createRevision({
scriptId: 1,
title: 'Initial draft',
content: 'FADE IN:\n\nINT. ROOM - DAY',
});
expect(result.versionNumber).toBe(1);
expect(result.branchName).toBe('main');
expect(result.status).toBe('draft');
expect(result.authorId).toBe(1);
});
it('should increment version number for same script', async () => {
await appRouter.revisions.createRevision.mutate({
input: { scriptId: 1, title: 'v1', content: 'content1', authorId: 1 },
ctx,
await caller.revisions.createRevision({
scriptId: 1,
title: 'v1',
content: 'content1',
});
const result = await appRouter.revisions.createRevision.mutate({
input: { scriptId: 1, title: 'v2', content: 'content2', authorId: 1 },
ctx,
const result = await caller.revisions.createRevision({
scriptId: 1,
title: 'v2',
content: 'content2',
});
expect(result.versionNumber).toBe(2);
});
it('should support custom branch', async () => {
const result = await appRouter.revisions.createRevision.mutate({
input: {
scriptId: 1,
title: 'Branch revision',
content: 'branch content',
branchName: 'feature-act2',
authorId: 1,
},
ctx,
const result = await caller.revisions.createRevision({
scriptId: 1,
title: 'Branch revision',
content: 'branch content',
branchName: 'feature-act2',
});
expect(result.branchName).toBe('feature-act2');
@@ -53,27 +60,28 @@ describe('revisionsRouter', () => {
describe('listRevisions', () => {
it('should return empty array for unknown script', async () => {
const result = await appRouter.revisions.listRevisions.query({
input: { scriptId: 999 },
ctx,
});
const result = await caller.revisions.listRevisions({ scriptId: 999 });
expect(result).toEqual([]);
});
it('should filter by branch', async () => {
await appRouter.revisions.createRevision.mutate({
input: { scriptId: 1, title: 'main v1', content: 'main', branchName: 'main', authorId: 1 },
ctx,
await caller.revisions.createRevision({
scriptId: 1,
title: 'main v1',
content: 'main',
branchName: 'main',
});
await appRouter.revisions.createRevision.mutate({
input: { scriptId: 1, title: 'feature v1', content: 'feature', branchName: 'feature', authorId: 1 },
ctx,
await caller.revisions.createRevision({
scriptId: 1,
title: 'feature v1',
content: 'feature',
branchName: 'feature',
});
const mainRevisions = await appRouter.revisions.listRevisions.query({
input: { scriptId: 1, branchName: 'main' },
ctx,
const mainRevisions = await caller.revisions.listRevisions({
scriptId: 1,
branchName: 'main',
});
expect(mainRevisions).toHaveLength(1);
@@ -83,32 +91,33 @@ describe('revisionsRouter', () => {
describe('acceptRevision', () => {
it('should accept a revision', async () => {
const created = await appRouter.revisions.createRevision.mutate({
input: { scriptId: 1, title: 'To accept', content: 'content', authorId: 1 },
ctx,
const created = await caller.revisions.createRevision({
scriptId: 1,
title: 'To accept',
content: 'content',
});
const result = await appRouter.revisions.acceptRevision.mutate({
input: { revisionId: created.id, reviewedById: 2 },
ctx,
const result = await caller.revisions.acceptRevision({
revisionId: created.id,
});
expect(result.status).toBe('accepted');
expect(result.reviewedById).toBe(2);
expect(result.reviewedById).toBe(1);
expect(result.reviewedAt).toBeDefined();
});
});
describe('rejectRevision', () => {
it('should reject a revision with reason', async () => {
const created = await appRouter.revisions.createRevision.mutate({
input: { scriptId: 1, title: 'To reject', content: 'content', authorId: 1 },
ctx,
const created = await caller.revisions.createRevision({
scriptId: 1,
title: 'To reject',
content: 'content',
});
const result = await appRouter.revisions.rejectRevision.mutate({
input: { revisionId: created.id, reviewedById: 2, reason: 'Needs more work on dialogue' },
ctx,
const result = await caller.revisions.rejectRevision({
revisionId: created.id,
reason: 'Needs more work on dialogue',
});
expect(result.status).toBe('rejected');
@@ -118,19 +127,21 @@ describe('revisionsRouter', () => {
describe('rollbackToRevision', () => {
it('should create a new revision with old content', async () => {
const original = await appRouter.revisions.createRevision.mutate({
input: { scriptId: 1, title: 'Original', content: 'original content', authorId: 1 },
ctx,
const original = await caller.revisions.createRevision({
scriptId: 1,
title: 'Original',
content: 'original content',
});
await appRouter.revisions.createRevision.mutate({
input: { scriptId: 1, title: 'Changed', content: 'changed content', authorId: 1 },
ctx,
await caller.revisions.createRevision({
scriptId: 1,
title: 'Changed',
content: 'changed content',
});
const rollback = await appRouter.revisions.rollbackToRevision.mutate({
input: { scriptId: 1, revisionId: original.id, authorId: 1 },
ctx,
const rollback = await caller.revisions.rollbackToRevision({
scriptId: 1,
revisionId: original.id,
});
expect(rollback.content).toBe('original content');
@@ -141,19 +152,21 @@ describe('revisionsRouter', () => {
describe('compareRevisions', () => {
it('should compare two revisions', async () => {
const rev1 = await appRouter.revisions.createRevision.mutate({
input: { scriptId: 1, title: 'v1', content: 'line1\nline2\nline3', authorId: 1 },
ctx,
const rev1 = await caller.revisions.createRevision({
scriptId: 1,
title: 'v1',
content: 'line1\nline2\nline3',
});
const rev2 = await appRouter.revisions.createRevision.mutate({
input: { scriptId: 1, title: 'v2', content: 'line1\nchanged\nline3', authorId: 1 },
ctx,
const rev2 = await caller.revisions.createRevision({
scriptId: 1,
title: 'v2',
content: 'line1\nchanged\nline3',
});
const result = await appRouter.revisions.compareRevisions.query({
input: { baseRevisionId: rev1.id, targetRevisionId: rev2.id },
ctx,
const result = await caller.revisions.compareRevisions({
baseRevisionId: rev1.id,
targetRevisionId: rev2.id,
});
expect(result.diff.modifications).toBe(1);
@@ -164,20 +177,19 @@ describe('revisionsRouter', () => {
describe('getTimeline', () => {
it('should return timeline entries in chronological order', async () => {
await appRouter.revisions.createRevision.mutate({
input: { scriptId: 1, title: 'First', content: 'first', authorId: 1 },
ctx,
await caller.revisions.createRevision({
scriptId: 1,
title: 'First',
content: 'first',
});
await appRouter.revisions.createRevision.mutate({
input: { scriptId: 1, title: 'Second', content: 'second', authorId: 1 },
ctx,
await caller.revisions.createRevision({
scriptId: 1,
title: 'Second',
content: 'second',
});
const timeline = await appRouter.revisions.getTimeline.query({
input: { scriptId: 1 },
ctx,
});
const timeline = await caller.revisions.getTimeline({ scriptId: 1 });
expect(timeline).toHaveLength(2);
expect(timeline[0]!.revision.title).toBe('First');
@@ -187,20 +199,18 @@ describe('revisionsRouter', () => {
describe('getBranches', () => {
it('should return branch information', async () => {
await appRouter.revisions.createRevision.mutate({
input: { scriptId: 1, title: 'main v1', content: 'main', authorId: 1 },
ctx,
await caller.revisions.createRevision({
scriptId: 1,
title: 'main v1',
content: 'main',
});
await appRouter.revisions.createBranch.mutate({
input: { scriptId: 1, branchName: 'feature', authorId: 1 },
ctx,
await caller.revisions.createBranch({
scriptId: 1,
branchName: 'feature',
});
const branches = await appRouter.revisions.getBranches.query({
input: { scriptId: 1 },
ctx,
});
const branches = await caller.revisions.getBranches({ scriptId: 1 });
expect(branches).toHaveLength(2);
const branchNames = branches.map((b: any) => b.branchName);
@@ -211,23 +221,18 @@ describe('revisionsRouter', () => {
describe('deleteRevision', () => {
it('should delete a revision', async () => {
const created = await appRouter.revisions.createRevision.mutate({
input: { scriptId: 1, title: 'To delete', content: 'content', authorId: 1 },
ctx,
const created = await caller.revisions.createRevision({
scriptId: 1,
title: 'To delete',
content: 'content',
});
const result = await appRouter.revisions.deleteRevision.mutate({
input: { id: created.id },
ctx,
});
const result = await caller.revisions.deleteRevision({ id: created.id });
expect(result.success).toBe(true);
await expect(
appRouter.revisions.getRevision.query({
input: { id: created.id },
ctx,
})
caller.revisions.getRevision({ id: created.id })
).rejects.toThrow();
});
});