mirror of
https://github.com/yusufipk/OpenFrame.git
synced 2026-09-11 09:36:08 +00:00
The suite that landed in #43/#44 was written against existing behaviour, so a number of tests pinned bugs rather than asserting correct behaviour. This fixes the production code and moves each of those tests onto the fixed behaviour in the same change. Security: - project-download: derive the archive entry extension from the last path segment and restrict it to a short alphanumeric run, so an extensionless allowlisted url can no longer contribute a path separator; validate the r2 branch against the strict proxy-path pattern instead of a `startsWith`, which let `/api/upload/video/clip.mp4/../../etc/passwd` through verbatim. - rate-limit: hash a key or action wider than its column instead of skipping the query. Both the guard and the failing INSERT used to answer "allowed", so the limit stopped applying entirely. Warn at startup when TRUSTED_PROXY_MODE is unset in production. - video uploads: the file name decides the content type; a client-declared video mime no longer makes `payload.exe` acceptable. - email templates: escape in the helpers rather than relying on every caller, with an explicit `rawEmailHtml()` opt-out for the one call site that builds markup. `escapeHtml` now covers the single quote. - CSP: allow loopback object storage outside production only. - route-access: reach the billing redirect only for the workspace owner. Keying it off the owner's billing status alone made the redirect target an oracle for whose subscription had lapsed, and sent members to a page they cannot act on. - search: carry the same billing condition every other read path carries. - logger: check `err.name` as well as `err.constructor.name`, so a re-thrown, deserialised or minified Prisma error is still redacted. - upload tokens: resolve the signing secret outside the try, so a server booted without one fails loudly instead of reporting every grant as a forgery. - invitations: never downgrade an existing membership, and report a scoped invitation that points at nothing as not_found rather than accepted. - auth: resolve the workspace role for every signed-in caller, so checkProjectAccess and computeProjectAccess stop disagreeing about the owner who also owns the workspace. The `intent` option is gone with it. - r2-media-proxy: validate the object key inside the proxy so the guard travels with the function; delete the unused, unanchored `mediaUrlToR2Key`. - r2: sign the content type into presigned PUT grants. Correctness: - frame rate snapping picks the nearest standard, not the first within tolerance, so 24, 30 and 60 fps are reachable at all. - a version upload registers its Bunny cleanup as soon as bunny-init answers, so a failed tus upload no longer leaves a billed video behind. - deleting videos clears storage before the rows, so a refused DELETE leaves a retryable row rather than an orphaned object. - an expired upload session can be cancelled, which is what releases its quota. - `voice/` joins the delete allowlist, so a voice note can be removed by the module that wrote it. - a failed CORS write propagates instead of being mistaken for an empty config and replacing the bucket's rules. - filtering projects by workspace no longer hides projects the unfiltered call returns. - upload retries skip aborts and permanent 4xx; progress no longer divides by zero. - reply edits no longer clear the comment's tag; optimistic resolve rolls back to the state it replaced; the delete snapshot is captured once. - assorted UI fixes: duplicate React keys, double-click guards reading stale closures, the tag list fetched twice per load, a failed member list rendering as an empty one, a stale "Initializing upload..." beside a failure, and a registration banner pointing at an email that never arrives. Consistency and access: - the two download routes answer 404 for an id belonging to another tenant, as the comment export route already did. A caller who does belong still gets 403. - accessible names for the share-link password field, the guest name gates, the version dialog inputs and the comment-tag controls. Repository health: - the runner image installs production dependencies only. - a setup file for the unit project restores stubbed env centrally. - native tsconfig path resolution replaces vite-tsconfig-paths. - `uploadBytesWithProgress` exists once. - admin stats bill Bunny storage to the workspace owner like every other quota, gate on the configured flag, wire up the single-flight guard and count the statuses that belonged to no bucket. - `r2Client.destroy()` releases the presign client too. - `prepare` tolerates a production install, where husky is absent.
371 lines
14 KiB
TypeScript
371 lines
14 KiB
TypeScript
// Result scoping for GET /api/search.
|
|
//
|
|
// The auth matrix already proves an anonymous caller gets 401, and beyond that
|
|
// there is nothing to test on the authorization axis: the route reads the
|
|
// caller's id straight off the session, so no "forbidden caller" exists. The
|
|
// question this file asks instead is a data-leak one, and nothing asserted it
|
|
// before: does the search actually restrict its rows to the caller's own
|
|
// tenants?
|
|
//
|
|
// It matters because search is the one endpoint that queries `project`,
|
|
// `workspace` and `video` globally rather than through a project id in the URL.
|
|
// Every filter is inline in the handler, none of it goes through
|
|
// `checkProjectAccess()`, and there is no shared helper that a regression would
|
|
// have to break twice. Dropping the `projectAccessFilter` clause from the video
|
|
// query would turn the search box into a list of every video title in the
|
|
// database, and until this file nothing would have noticed.
|
|
//
|
|
// Every test uses a term that appears in exactly one tenant's rows, so a hit is
|
|
// unambiguous. Each refusal is paired with the same query run by a caller who
|
|
// should see it, which is what rules out an empty result that came from the
|
|
// query never matching anything in the first place.
|
|
|
|
import { describe, expect, it } from 'vitest';
|
|
import { GET as search } from '@/app/api/search/route';
|
|
import { GET as listProjects } from '@/app/api/projects/route';
|
|
import { apiRequest, callRoute, readData } from '../helpers/request';
|
|
import { signedInAs, signedOut } from '../helpers/session';
|
|
import {
|
|
addProjectMember,
|
|
addWorkspaceMember,
|
|
createExpiredUser,
|
|
createProject,
|
|
createUser,
|
|
createVideo,
|
|
createWorkspace,
|
|
nextSeq,
|
|
seedProject,
|
|
} from '../factories';
|
|
|
|
interface SearchResults {
|
|
projects: Array<{ id: string; name: string }>;
|
|
workspaces: Array<{ id: string; name: string }>;
|
|
videos: Array<{ id: string; title: string }>;
|
|
}
|
|
|
|
async function searchFor(term: string): Promise<SearchResults> {
|
|
const response = await callRoute(
|
|
search,
|
|
apiRequest('/api/search', { searchParams: { q: term } })
|
|
);
|
|
expect(response.status).toBe(200);
|
|
return readData<SearchResults>(response);
|
|
}
|
|
|
|
/** A term that cannot collide with a factory default name or another test's rows. */
|
|
function uniqueTerm(): string {
|
|
return `Zephyrine${nextSeq()}`;
|
|
}
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// Cross-tenant leakage
|
|
// ---------------------------------------------------------------------------
|
|
describe('GET /api/search does not reach into another tenant', () => {
|
|
it('returns nothing to an anonymous caller', async () => {
|
|
const term = uniqueTerm();
|
|
const { project } = await seedProject({ projectName: `${term} project` });
|
|
await createVideo({ projectId: project.id, title: `${term} cut` });
|
|
signedOut();
|
|
|
|
const response = await callRoute(
|
|
search,
|
|
apiRequest('/api/search', { searchParams: { q: term } })
|
|
);
|
|
|
|
expect(response.status).toBe(401);
|
|
});
|
|
|
|
it('hides a stranger project matching the term by name', async () => {
|
|
const term = uniqueTerm();
|
|
await seedProject({ projectName: `${term} deliverables` });
|
|
const outsider = await createUser();
|
|
signedInAs(outsider);
|
|
|
|
const results = await searchFor(term);
|
|
|
|
expect(results.projects).toEqual([]);
|
|
});
|
|
|
|
// The positive control for the case above. Same term, same row, and the only
|
|
// difference is who is asking, so an empty result cannot be blamed on the
|
|
// query failing to match.
|
|
it('shows that same project to its owner', async () => {
|
|
const term = uniqueTerm();
|
|
const { owner } = await seedProject({ projectName: `${term} deliverables` });
|
|
signedInAs(owner);
|
|
|
|
const results = await searchFor(term);
|
|
|
|
expect(results.projects.map((project) => project.name)).toEqual([`${term} deliverables`]);
|
|
});
|
|
|
|
it('hides a stranger project matching the term only in its description', async () => {
|
|
const term = uniqueTerm();
|
|
const scenario = await seedProject();
|
|
await createProject({
|
|
ownerId: scenario.owner.id,
|
|
workspaceId: scenario.workspace.id,
|
|
name: 'Unremarkable name',
|
|
description: `Rough cut for the ${term} campaign`,
|
|
});
|
|
const outsider = await createUser();
|
|
signedInAs(outsider);
|
|
|
|
const results = await searchFor(term);
|
|
|
|
expect(results.projects).toEqual([]);
|
|
});
|
|
|
|
it('shows the description match to the project owner', async () => {
|
|
const term = uniqueTerm();
|
|
const scenario = await seedProject();
|
|
await createProject({
|
|
ownerId: scenario.owner.id,
|
|
workspaceId: scenario.workspace.id,
|
|
name: 'Unremarkable name',
|
|
description: `Rough cut for the ${term} campaign`,
|
|
});
|
|
signedInAs(scenario.owner);
|
|
|
|
const results = await searchFor(term);
|
|
|
|
expect(results.projects).toHaveLength(1);
|
|
});
|
|
|
|
// The headline case from the gap inventory: a video title is the most
|
|
// sensitive string in this product's search index, because it is usually a
|
|
// client name or an unannounced campaign.
|
|
it('hides a video in a stranger project whose title matches the term', async () => {
|
|
const term = uniqueTerm();
|
|
const { project } = await seedProject();
|
|
await createVideo({ projectId: project.id, title: `${term} launch cut` });
|
|
// The outsider owns a real tenant of their own, so nothing about the request
|
|
// is unusual: they simply have no relationship to the project holding the hit.
|
|
await seedProject();
|
|
const outsider = await createUser();
|
|
signedInAs(outsider);
|
|
|
|
const results = await searchFor(term);
|
|
|
|
expect(results.videos).toEqual([]);
|
|
});
|
|
|
|
it('shows that same video to the owner of the project holding it', async () => {
|
|
const term = uniqueTerm();
|
|
const { owner, project } = await seedProject();
|
|
await createVideo({ projectId: project.id, title: `${term} launch cut` });
|
|
signedInAs(owner);
|
|
|
|
const results = await searchFor(term);
|
|
|
|
expect(results.videos.map((video) => video.title)).toEqual([`${term} launch cut`]);
|
|
});
|
|
|
|
it('hides a stranger workspace matching the term by name', async () => {
|
|
const term = uniqueTerm();
|
|
const stranger = await createUser();
|
|
await createWorkspace({ ownerId: stranger.id, name: `${term} Studio` });
|
|
const outsider = await createUser();
|
|
signedInAs(outsider);
|
|
|
|
const results = await searchFor(term);
|
|
|
|
expect(results.workspaces).toEqual([]);
|
|
});
|
|
|
|
it('shows that same workspace to its owner', async () => {
|
|
const term = uniqueTerm();
|
|
const stranger = await createUser();
|
|
await createWorkspace({ ownerId: stranger.id, name: `${term} Studio` });
|
|
signedInAs(stranger);
|
|
|
|
const results = await searchFor(term);
|
|
|
|
expect(results.workspaces.map((workspace) => workspace.name)).toEqual([`${term} Studio`]);
|
|
});
|
|
|
|
// Search is scoped by membership, not by visibility: `checkProjectAccess()`
|
|
// would let this caller open the project, but it does not surface in their
|
|
// search. Pinned because it is the one place the two rules deliberately differ,
|
|
// and because widening search to match the access check would be a real
|
|
// exposure of every public project's video titles.
|
|
it('hides a PUBLIC stranger project the caller has never joined', async () => {
|
|
const term = uniqueTerm();
|
|
const { project } = await seedProject({
|
|
visibility: 'PUBLIC',
|
|
projectName: `${term} open project`,
|
|
});
|
|
await createVideo({ projectId: project.id, title: `${term} open cut` });
|
|
const outsider = await createUser();
|
|
signedInAs(outsider);
|
|
|
|
const results = await searchFor(term);
|
|
|
|
expect(results.projects).toEqual([]);
|
|
expect(results.videos).toEqual([]);
|
|
});
|
|
});
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// The three ways in
|
|
// ---------------------------------------------------------------------------
|
|
// The access filter has three branches. Each one is exercised here, because a
|
|
// scoping test that only proves "strangers see nothing" would still pass if the
|
|
// filter had collapsed to `ownerId` and quietly stopped showing collaborators
|
|
// their own work.
|
|
describe('GET /api/search reaches everything the caller is entitled to', () => {
|
|
it('shows a project the caller was added to directly', async () => {
|
|
const term = uniqueTerm();
|
|
const { project } = await seedProject({ projectName: `${term} shared cut` });
|
|
const collaborator = await createUser();
|
|
await addProjectMember({
|
|
projectId: project.id,
|
|
userId: collaborator.id,
|
|
role: 'COMMENTATOR',
|
|
});
|
|
signedInAs(collaborator);
|
|
|
|
const results = await searchFor(term);
|
|
|
|
expect(results.projects.map((entry) => entry.id)).toEqual([project.id]);
|
|
});
|
|
|
|
it('shows videos in a project the caller was added to directly', async () => {
|
|
const term = uniqueTerm();
|
|
const { project } = await seedProject();
|
|
const video = await createVideo({ projectId: project.id, title: `${term} rough cut` });
|
|
const collaborator = await createUser();
|
|
await addProjectMember({ projectId: project.id, userId: collaborator.id });
|
|
signedInAs(collaborator);
|
|
|
|
const results = await searchFor(term);
|
|
|
|
expect(results.videos.map((entry) => entry.id)).toEqual([video.id]);
|
|
});
|
|
|
|
// A workspace member with no project membership row at all. This is the branch
|
|
// most likely to be dropped by accident, because the other two are obvious.
|
|
it('shows a project the caller reaches only through workspace membership', async () => {
|
|
const term = uniqueTerm();
|
|
const { workspace, project } = await seedProject({ projectName: `${term} workspace cut` });
|
|
const video = await createVideo({ projectId: project.id, title: `${term} workspace video` });
|
|
const workspaceMember = await createUser();
|
|
await addWorkspaceMember({ workspaceId: workspace.id, userId: workspaceMember.id });
|
|
signedInAs(workspaceMember);
|
|
|
|
const results = await searchFor(term);
|
|
|
|
expect(results.projects.map((entry) => entry.id)).toEqual([project.id]);
|
|
expect(results.videos.map((entry) => entry.id)).toEqual([video.id]);
|
|
});
|
|
|
|
it('shows a workspace the caller is a member of', async () => {
|
|
const term = uniqueTerm();
|
|
const stranger = await createUser();
|
|
const workspace = await createWorkspace({ ownerId: stranger.id, name: `${term} Studio` });
|
|
const member = await createUser();
|
|
await addWorkspaceMember({ workspaceId: workspace.id, userId: member.id });
|
|
signedInAs(member);
|
|
|
|
const results = await searchFor(term);
|
|
|
|
expect(results.workspaces.map((entry) => entry.id)).toEqual([workspace.id]);
|
|
});
|
|
|
|
// A project membership must not leak the enclosing workspace, which usually
|
|
// carries the agency's own name and its other clients.
|
|
it('does not show the enclosing workspace to a project-only member', async () => {
|
|
const term = uniqueTerm();
|
|
const owner = await createUser();
|
|
const workspace = await createWorkspace({ ownerId: owner.id, name: `${term} Studio` });
|
|
const project = await createProject({
|
|
ownerId: owner.id,
|
|
workspaceId: workspace.id,
|
|
name: `${term} client project`,
|
|
});
|
|
const collaborator = await createUser();
|
|
await addProjectMember({ projectId: project.id, userId: collaborator.id });
|
|
signedInAs(collaborator);
|
|
|
|
const results = await searchFor(term);
|
|
|
|
expect(results.projects.map((entry) => entry.id)).toEqual([project.id]);
|
|
expect(results.workspaces).toEqual([]);
|
|
});
|
|
});
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// Billing
|
|
// ---------------------------------------------------------------------------
|
|
// Search carries the same billing condition every other read path does. It used to carry
|
|
// none: GET /api/projects filters every row through
|
|
// `workspace.owner: buildBillingAccessWhereInput()`, and `checkProjectAccess()` makes
|
|
// `hasAccess` false the moment the workspace owner's billing lapses, so the project
|
|
// itself answers 403, while search went on returning names, descriptions and video
|
|
// titles for the same tenant.
|
|
describe('GET /api/search and lapsed billing', () => {
|
|
it('hides a project whose workspace owner has lost billing access', async () => {
|
|
const term = uniqueTerm();
|
|
const expiredOwner = await createExpiredUser();
|
|
await seedProject({ ownerUser: expiredOwner, projectName: `${term} lapsed project` });
|
|
signedInAs(expiredOwner);
|
|
|
|
const results = await searchFor(term);
|
|
|
|
expect(results.projects).toEqual([]);
|
|
});
|
|
|
|
// The positive control: the same shape with billing intact still comes back, so the
|
|
// assertion above is about billing and not about the fixture failing to seed.
|
|
it('still returns a project whose workspace owner is paying', async () => {
|
|
const term = uniqueTerm();
|
|
const scenario = await seedProject({ projectName: `${term} live project` });
|
|
signedInAs(scenario.owner);
|
|
|
|
const results = await searchFor(term);
|
|
|
|
expect(results.projects.map((entry) => entry.name)).toEqual([`${term} live project`]);
|
|
});
|
|
|
|
// The same caller, the same row, through the list endpoint instead: the two agree now.
|
|
it('is hidden from GET /api/projects for the same caller and the same row', async () => {
|
|
const expiredOwner = await createExpiredUser();
|
|
await seedProject({ ownerUser: expiredOwner, projectName: 'Lapsed project' });
|
|
signedInAs(expiredOwner);
|
|
|
|
const response = await callRoute(listProjects, apiRequest('/api/projects'));
|
|
|
|
expect(response.status).toBe(200);
|
|
const { projects } = await readData<{ projects: Array<{ id: string }> }>(response);
|
|
expect(projects).toEqual([]);
|
|
});
|
|
|
|
it('hides a video title from a lapsed workspace, even from a collaborator', async () => {
|
|
const term = uniqueTerm();
|
|
const expiredOwner = await createExpiredUser();
|
|
const { project } = await seedProject({ ownerUser: expiredOwner });
|
|
await createVideo({ projectId: project.id, title: `${term} lapsed cut` });
|
|
const collaborator = await createUser();
|
|
await addProjectMember({ projectId: project.id, userId: collaborator.id });
|
|
signedInAs(collaborator);
|
|
|
|
const results = await searchFor(term);
|
|
|
|
expect(results.videos).toEqual([]);
|
|
});
|
|
|
|
it('still returns a video title to a collaborator while the owner is paying', async () => {
|
|
const term = uniqueTerm();
|
|
const { project } = await seedProject();
|
|
|
|
await createVideo({ projectId: project.id, title: `${term} live cut` });
|
|
const collaborator = await createUser();
|
|
await addProjectMember({ projectId: project.id, userId: collaborator.id });
|
|
signedInAs(collaborator);
|
|
|
|
const results = await searchFor(term);
|
|
|
|
expect(results.videos.map((entry) => entry.title)).toEqual([`${term} live cut`]);
|
|
});
|
|
});
|