mirror of
https://github.com/yusufipk/OpenFrame.git
synced 2026-09-11 09:36:08 +00:00
fix(uploads): stop a storage hold from being dropped by whoever can name it
A reservation id was never a secret and could not have been one. An upload token is base64url(payload) followed by its signature, so a client can read every claim out of its own token, and the two R2 init routes hand their reservation ids to the client outright. The asset route takes a reservation id from the request body and deleted it on the strength of that id and the billed user alone, and every hold an account owns is billed to the same user. So a caller could start a Bunny upload, read the id out of the token they were just given, quote it while attaching a one byte image or even a bare YouTube link, and have the quota handed back while the upload carried on. Repeat and a trial worth three gigabytes uploads as much as it likes for as long as Bunny takes to report a figure of its own. Signing the id rather than handing it over bought nothing, because signing is not hiding. A hold now records what it was opened for and is only ever consumed by that flow, so naming one is no longer enough to drop it. Guests hold against the workspace owner's quota rather than their own and had no way to give it back: the release was gated on being signed in. Declaring a size and walking away cost the guest nothing and cost the owner their whole remaining allowance for two hours. The guest grant now carries the reservation and the declared size, bound to the Bunny video as well as to ours, so cancelling gives the quota back and costs them the upload it stood for. What a guest can hold without cancelling lapses in half an hour rather than two hours. The in-transaction fallback check counted the account's Bunny storage as zero on a Bunny upload, because the figure was only prefetched for R2 providers and that branch was unreachable for Bunny until this PR made it reachable. On an account whose storage is all Bunny that was a check that could not fail. It is prefetched for every provider that can reach the fallback now.
This commit is contained in:
+110
-16
@@ -20,6 +20,25 @@ interface GuestUploadTokenPayload {
|
||||
exp: number;
|
||||
intent: GuestUploadIntent;
|
||||
ctx: string;
|
||||
/**
|
||||
* The provider's own id for the video this grant was issued against.
|
||||
*
|
||||
* `vid` is our video, the one the asset will hang off. That is not enough to
|
||||
* hand a guest the right to release a storage hold: the hold stands for one
|
||||
* particular upload, and a grant that names only our video would let a guest
|
||||
* drop it while the upload it stands for carried on. Binding the provider's id
|
||||
* makes releasing cost the guest the upload itself, which is the same bargain
|
||||
* the signed-in path already makes.
|
||||
*/
|
||||
bvid?: string;
|
||||
/** The storage reservation this upload holds, when it holds one. */
|
||||
rid?: string;
|
||||
/**
|
||||
* The size the client declared when it asked for this grant, as a decimal
|
||||
* string because JSON has no integer wide enough. Signed so a guest cannot
|
||||
* declare one size to pass the quota check and another to be billed for.
|
||||
*/
|
||||
sz?: string;
|
||||
}
|
||||
|
||||
interface GuestUploadTokenSubject {
|
||||
@@ -29,6 +48,20 @@ interface GuestUploadTokenSubject {
|
||||
context: string;
|
||||
}
|
||||
|
||||
interface GuestUploadTokenClaims {
|
||||
/** The provider video id to bind this grant to. */
|
||||
providerVideoId?: string | null;
|
||||
reservationId?: string | null;
|
||||
declaredSizeBytes?: bigint | null;
|
||||
}
|
||||
|
||||
export interface GuestUploadGrant {
|
||||
/** The storage reservation this upload holds, or null if it holds none. */
|
||||
reservationId: string | null;
|
||||
/** What the client said it was uploading, or null on a grant that predates the claim. */
|
||||
declaredSizeBytes: bigint | null;
|
||||
}
|
||||
|
||||
const TRUSTED_IP_PATTERN = /^[\da-fA-F.:]+$/;
|
||||
|
||||
function getGuestUploadTokenSecret(): string {
|
||||
@@ -77,7 +110,10 @@ function isValidPayload(value: unknown): value is GuestUploadTokenPayload {
|
||||
typeof payload.exp === 'number' &&
|
||||
Number.isFinite(payload.exp) &&
|
||||
(payload.intent === 'audio' || payload.intent === 'image' || payload.intent === 'bunny') &&
|
||||
typeof payload.ctx === 'string'
|
||||
typeof payload.ctx === 'string' &&
|
||||
(payload.bvid === undefined || typeof payload.bvid === 'string') &&
|
||||
(payload.rid === undefined || typeof payload.rid === 'string') &&
|
||||
(payload.sz === undefined || typeof payload.sz === 'string')
|
||||
);
|
||||
}
|
||||
|
||||
@@ -95,7 +131,7 @@ export function deriveGuestUploadContext(
|
||||
}
|
||||
|
||||
export function createGuestUploadToken(
|
||||
subject: GuestUploadTokenSubject,
|
||||
subject: GuestUploadTokenSubject & GuestUploadTokenClaims,
|
||||
ttlSeconds = DEFAULT_GUEST_UPLOAD_TOKEN_TTL_SECONDS
|
||||
): string {
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
@@ -107,6 +143,9 @@ export function createGuestUploadToken(
|
||||
exp: now + ttlSeconds,
|
||||
intent: subject.intent,
|
||||
ctx: subject.context,
|
||||
...(subject.providerVideoId ? { bvid: subject.providerVideoId } : {}),
|
||||
...(subject.reservationId ? { rid: subject.reservationId } : {}),
|
||||
...(subject.declaredSizeBytes ? { sz: subject.declaredSizeBytes.toString() } : {}),
|
||||
};
|
||||
|
||||
const encodedPayload = Buffer.from(JSON.stringify(payload), 'utf8').toString('base64url');
|
||||
@@ -114,39 +153,94 @@ export function createGuestUploadToken(
|
||||
return `${encodedPayload}.${signature}`;
|
||||
}
|
||||
|
||||
export function verifyGuestUploadToken(token: string, subject: GuestUploadTokenSubject): boolean {
|
||||
/**
|
||||
* The verified payload, or null when the token is not a genuine grant for this
|
||||
* subject.
|
||||
*
|
||||
* `providerVideoId` is checked only when the grant carries one, so a token
|
||||
* issued before that claim existed keeps working rather than failing an upload
|
||||
* in flight. A grant that does carry one and does not match is a forgery as far
|
||||
* as this is concerned.
|
||||
*/
|
||||
function readGuestUploadToken(
|
||||
token: string,
|
||||
subject: GuestUploadTokenSubject,
|
||||
providerVideoId?: string | null
|
||||
): GuestUploadTokenPayload | null {
|
||||
try {
|
||||
const parts = token.split('.');
|
||||
if (parts.length !== 2) return false;
|
||||
if (parts.length !== 2) return null;
|
||||
|
||||
const [encodedPayload, providedSignature] = parts;
|
||||
if (!encodedPayload || !providedSignature) return false;
|
||||
if (!encodedPayload || !providedSignature) return null;
|
||||
|
||||
const expectedSignature = signPayload(encodedPayload);
|
||||
const providedBuffer = Buffer.from(providedSignature, 'utf8');
|
||||
const expectedBuffer = Buffer.from(expectedSignature, 'utf8');
|
||||
if (providedBuffer.length !== expectedBuffer.length) return false;
|
||||
if (!timingSafeEqual(providedBuffer, expectedBuffer)) return false;
|
||||
if (providedBuffer.length !== expectedBuffer.length) return null;
|
||||
if (!timingSafeEqual(providedBuffer, expectedBuffer)) return null;
|
||||
|
||||
const payloadRaw = Buffer.from(encodedPayload, 'base64url').toString('utf8');
|
||||
const payloadUnknown: unknown = JSON.parse(payloadRaw);
|
||||
if (!isValidPayload(payloadUnknown)) return false;
|
||||
if (!isValidPayload(payloadUnknown)) return null;
|
||||
|
||||
const payload = payloadUnknown;
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
if (payload.exp < now) return false;
|
||||
if (payload.exp < now) return null;
|
||||
|
||||
return (
|
||||
payload.pid === subject.projectId &&
|
||||
payload.vid === subject.videoId &&
|
||||
payload.intent === subject.intent &&
|
||||
payload.ctx === subject.context
|
||||
);
|
||||
if (
|
||||
payload.pid !== subject.projectId ||
|
||||
payload.vid !== subject.videoId ||
|
||||
payload.intent !== subject.intent ||
|
||||
payload.ctx !== subject.context
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (payload.bvid !== undefined && payload.bvid !== providerVideoId) return null;
|
||||
|
||||
return payload;
|
||||
} catch {
|
||||
return false;
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
export function verifyGuestUploadToken(
|
||||
token: string,
|
||||
subject: GuestUploadTokenSubject,
|
||||
providerVideoId?: string | null
|
||||
): boolean {
|
||||
return readGuestUploadToken(token, subject, providerVideoId) !== null;
|
||||
}
|
||||
|
||||
/**
|
||||
* What a genuine guest grant for this subject carries, or null when the token is
|
||||
* not one.
|
||||
*
|
||||
* Null and empty fields mean the same thing to every caller: there is nothing
|
||||
* here to release and nothing to bill.
|
||||
*/
|
||||
export function readGuestUploadGrant(
|
||||
token: string,
|
||||
subject: GuestUploadTokenSubject,
|
||||
providerVideoId?: string | null
|
||||
): GuestUploadGrant | null {
|
||||
const payload = readGuestUploadToken(token, subject, providerVideoId);
|
||||
if (!payload) return null;
|
||||
|
||||
let declaredSizeBytes: bigint | null = null;
|
||||
if (payload.sz) {
|
||||
try {
|
||||
const parsed = BigInt(payload.sz);
|
||||
declaredSizeBytes = parsed > BigInt(0) ? parsed : null;
|
||||
} catch {
|
||||
declaredSizeBytes = null;
|
||||
}
|
||||
}
|
||||
|
||||
return { reservationId: payload.rid ?? null, declaredSizeBytes };
|
||||
}
|
||||
|
||||
export async function enforceGuestUploadQuota(
|
||||
request: Request,
|
||||
videoId: string,
|
||||
|
||||
+36
-2
@@ -29,6 +29,33 @@ export async function getStorageLimitForUser(userId: string): Promise<bigint> {
|
||||
// TTL for upload reservations: 30 minutes is enough for R2 image/audio uploads
|
||||
const RESERVATION_TTL_MS = 30 * 60 * 1000;
|
||||
|
||||
/**
|
||||
* What a hold was opened for.
|
||||
*
|
||||
* A reservation is only ever consumed by the flow that opened it, and the
|
||||
* finalize routes match on this as well as on the id. Without it, naming a
|
||||
* reservation would be enough to drop it: the asset route takes a reservation id
|
||||
* from the request body, and every hold an account owns is billed to the same
|
||||
* user, so an image being attached could quietly release a video upload that was
|
||||
* still in flight. The ids are not secret. Two of them are handed to the client
|
||||
* outright, and the rest ride inside signed-but-readable token payloads.
|
||||
*/
|
||||
export const UPLOAD_RESERVATION_PURPOSES = {
|
||||
/** A comment attachment or standalone image going to R2. */
|
||||
IMAGE: 'IMAGE',
|
||||
/** A voice note going to R2. */
|
||||
AUDIO: 'AUDIO',
|
||||
/** Image and voice attachments weighed together when a comment is posted. */
|
||||
ATTACHMENT: 'ATTACHMENT',
|
||||
/** A presigned direct upload to our own S3-compatible storage. */
|
||||
R2_VIDEO: 'R2_VIDEO',
|
||||
/** A direct upload to Bunny, where the bytes never pass through us. */
|
||||
BUNNY: 'BUNNY',
|
||||
} as const;
|
||||
|
||||
export type UploadReservationPurpose =
|
||||
(typeof UPLOAD_RESERVATION_PURPOSES)[keyof typeof UPLOAD_RESERVATION_PURPOSES];
|
||||
|
||||
// Sentinel error thrown inside a Prisma transaction to signal quota exceeded
|
||||
class QuotaExceededError extends Error {}
|
||||
|
||||
@@ -137,6 +164,7 @@ export async function enforceStorageQuota(
|
||||
export async function reserveStorageQuota(
|
||||
userId: string,
|
||||
incomingSizeBytes: bigint,
|
||||
purpose: UploadReservationPurpose,
|
||||
reservationTtlMs: number = RESERVATION_TTL_MS
|
||||
): Promise<{ reservationId: string | null } | { error: NextResponse }> {
|
||||
if (!isStripeFeatureEnabled()) {
|
||||
@@ -199,7 +227,7 @@ export async function reserveStorageQuota(
|
||||
}
|
||||
|
||||
const reservation = await tx.uploadReservation.create({
|
||||
data: { billedUserId: userId, sizeBytes: incomingSizeBytes, expiresAt },
|
||||
data: { billedUserId: userId, sizeBytes: incomingSizeBytes, expiresAt, purpose },
|
||||
select: { id: true },
|
||||
});
|
||||
|
||||
@@ -218,16 +246,22 @@ export async function reserveStorageQuota(
|
||||
/**
|
||||
* Deletes an upload reservation created by `reserveStorageQuota`.
|
||||
* Safe to call with `null` (no-op) for flows where billing is disabled.
|
||||
*
|
||||
* Pass the purpose wherever the caller knows it. A release that names only an id
|
||||
* will delete a hold opened for something else, which is the same hole the
|
||||
* purpose column exists to close.
|
||||
*/
|
||||
export async function releaseStorageReservation(
|
||||
reservationId: string | null,
|
||||
billedUserId?: string | null
|
||||
billedUserId?: string | null,
|
||||
purpose?: UploadReservationPurpose
|
||||
): Promise<void> {
|
||||
if (!reservationId) return;
|
||||
await db.uploadReservation.deleteMany({
|
||||
where: {
|
||||
id: reservationId,
|
||||
...(billedUserId ? { billedUserId } : {}),
|
||||
...(purpose ? { purpose } : {}),
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user