fix(audit): storage cluster — M16 (presign doc/contract), M17 (per-port byte cap), M18 (replay-after-stat), L17 (mime allow-list, fingerprint hash), L22 (brochure portSlug)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -20,7 +20,7 @@ import { Readable } from 'node:stream';
|
|||||||
|
|
||||||
import { NextRequest, NextResponse } from 'next/server';
|
import { NextRequest, NextResponse } from 'next/server';
|
||||||
|
|
||||||
import { MAX_FILE_SIZE } from '@/lib/constants/file-validation';
|
import { ALLOWED_MIME_TYPES, MAX_FILE_SIZE } from '@/lib/constants/file-validation';
|
||||||
import {
|
import {
|
||||||
AppError,
|
AppError,
|
||||||
errorResponse,
|
errorResponse,
|
||||||
@@ -63,21 +63,6 @@ export async function GET(
|
|||||||
}
|
}
|
||||||
const { payload } = result;
|
const { payload } = result;
|
||||||
|
|
||||||
// Single-use enforcement. SET NX with a TTL pinned to the token's own
|
|
||||||
// expiry so the dedup window never closes before the token does. Using
|
|
||||||
// the body half of the token as the dedup key (signature included
|
|
||||||
// would also work but body is enough - a reused token has the same body).
|
|
||||||
const replayKey = `storage:proxy:seen:${token.split('.')[0]}`;
|
|
||||||
const remainingSeconds = Math.max(
|
|
||||||
REPLAY_TTL_FLOOR_SECONDS,
|
|
||||||
Math.min(REPLAY_TTL_CEILING_SECONDS, payload.e - Math.floor(Date.now() / 1000) + 60),
|
|
||||||
);
|
|
||||||
const setOk = await redis.set(replayKey, '1', 'EX', remainingSeconds, 'NX');
|
|
||||||
if (setOk !== 'OK') {
|
|
||||||
logger.warn({ key: payload.k }, 'Storage proxy token replay rejected');
|
|
||||||
return errorResponse(new ForbiddenError('Token already used'));
|
|
||||||
}
|
|
||||||
|
|
||||||
let absolutePath: string;
|
let absolutePath: string;
|
||||||
try {
|
try {
|
||||||
absolutePath = backend.resolveKeyForProxy(payload.k);
|
absolutePath = backend.resolveKeyForProxy(payload.k);
|
||||||
@@ -86,6 +71,11 @@ export async function GET(
|
|||||||
return errorResponse(new ValidationError('Invalid key'));
|
return errorResponse(new ValidationError('Invalid key'));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Confirm the file is servable BEFORE burning the single-use replay key
|
||||||
|
// (audit M18). The old order consumed the SET-NX key first, so a transient
|
||||||
|
// `fs.stat` failure / NFS hiccup / ENOENT permanently bricked the emailed
|
||||||
|
// URL ("Token already used" for its full life). Now a stat failure leaves
|
||||||
|
// the token unused and a genuine retry succeeds.
|
||||||
let size: number;
|
let size: number;
|
||||||
try {
|
try {
|
||||||
const stat = await fs.stat(absolutePath);
|
const stat = await fs.stat(absolutePath);
|
||||||
@@ -101,12 +91,50 @@ export async function GET(
|
|||||||
return errorResponse(err);
|
return errorResponse(err);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Single-use enforcement. SET NX with a TTL pinned to the token's own
|
||||||
|
// expiry so the dedup window never closes before the token does. Using
|
||||||
|
// the body half of the token as the dedup key (signature included
|
||||||
|
// would also work but body is enough - a reused token has the same body).
|
||||||
|
// Claimed only now - after the file is confirmed servable - so an earlier
|
||||||
|
// transient error doesn't permanently consume the token (audit M18).
|
||||||
|
const replayKey = `storage:proxy:seen:${token.split('.')[0]}`;
|
||||||
|
const remainingSeconds = Math.max(
|
||||||
|
REPLAY_TTL_FLOOR_SECONDS,
|
||||||
|
Math.min(REPLAY_TTL_CEILING_SECONDS, payload.e - Math.floor(Date.now() / 1000) + 60),
|
||||||
|
);
|
||||||
|
const setOk = await redis.set(replayKey, '1', 'EX', remainingSeconds, 'NX');
|
||||||
|
if (setOk !== 'OK') {
|
||||||
|
logger.warn({ key: payload.k }, 'Storage proxy token replay rejected');
|
||||||
|
return errorResponse(new ForbiddenError('Token already used'));
|
||||||
|
}
|
||||||
|
|
||||||
// Convert the Node Readable into a Web ReadableStream for NextResponse.
|
// Convert the Node Readable into a Web ReadableStream for NextResponse.
|
||||||
|
// If the stream fails after this point we DEL the replay key so the
|
||||||
|
// customer's retry isn't bricked (audit M18) - the dedup intent is "one
|
||||||
|
// successful download", not "one attempt".
|
||||||
const nodeStream = createReadStream(absolutePath);
|
const nodeStream = createReadStream(absolutePath);
|
||||||
|
nodeStream.on('error', (err) => {
|
||||||
|
logger.warn({ err, key: payload.k }, 'Storage proxy stream failed; releasing replay key');
|
||||||
|
void redis.del(replayKey).catch(() => undefined);
|
||||||
|
});
|
||||||
const webStream = Readable.toWeb(nodeStream) as unknown as ReadableStream<Uint8Array>;
|
const webStream = Readable.toWeb(nodeStream) as unknown as ReadableStream<Uint8Array>;
|
||||||
|
|
||||||
const headers = new Headers();
|
const headers = new Headers();
|
||||||
headers.set('Content-Type', payload.c ?? 'application/octet-stream');
|
// L17(a): constrain the served Content-Type to a known-safe allow-list.
|
||||||
|
// `payload.c` is issuer-signed (not attacker-forgeable) but a future buggy
|
||||||
|
// issuer could mint an active type (e.g. text/html) that a browser would
|
||||||
|
// render inline. Anything off the allow-list is served as a download with
|
||||||
|
// a generic octet-stream type; `nosniff` is set unconditionally below.
|
||||||
|
const tokenContentType = payload.c && ALLOWED_MIME_TYPES.has(payload.c) ? payload.c : null;
|
||||||
|
headers.set('Content-Type', tokenContentType ?? 'application/octet-stream');
|
||||||
|
if (!tokenContentType) {
|
||||||
|
// Force download for any non-allow-listed type so an unexpected
|
||||||
|
// content-type can never be rendered inline by the browser.
|
||||||
|
headers.set(
|
||||||
|
'Content-Disposition',
|
||||||
|
`attachment; filename="${(payload.f ?? 'download').replace(/"/g, '')}"`,
|
||||||
|
);
|
||||||
|
}
|
||||||
headers.set('Content-Length', String(size));
|
headers.set('Content-Length', String(size));
|
||||||
if (payload.f) {
|
if (payload.f) {
|
||||||
// RFC 5987 - quote the filename and provide a UTF-8 fallback.
|
// RFC 5987 - quote the filename and provide a UTF-8 fallback.
|
||||||
@@ -167,15 +195,25 @@ export async function PUT(
|
|||||||
return errorResponse(new ForbiddenError('Token already used'));
|
return errorResponse(new ForbiddenError('Token already used'));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Effective byte cap. The token may carry a per-port `b` cap (from
|
||||||
|
// `system_settings.berth_pdf_max_upload_mb`); enforce the tighter of that
|
||||||
|
// and the global `MAX_FILE_SIZE` ceiling. Without this (audit M17) the
|
||||||
|
// proxy enforced only the global 50 MB and a rep could write 50 MB to a
|
||||||
|
// berth advertised as 15 MB-capped.
|
||||||
|
const effectiveCap =
|
||||||
|
typeof payload.b === 'number' && Number.isFinite(payload.b) && payload.b > 0
|
||||||
|
? Math.min(MAX_FILE_SIZE, payload.b)
|
||||||
|
: MAX_FILE_SIZE;
|
||||||
|
|
||||||
// Pre-flight size check via Content-Length so a malicious caller can't
|
// Pre-flight size check via Content-Length so a malicious caller can't
|
||||||
// exhaust disk by streaming hundreds of MB before we look at the body.
|
// exhaust disk by streaming hundreds of MB before we look at the body.
|
||||||
const contentLengthHeader = req.headers.get('content-length');
|
const contentLengthHeader = req.headers.get('content-length');
|
||||||
const contentLength = contentLengthHeader ? Number(contentLengthHeader) : NaN;
|
const contentLength = contentLengthHeader ? Number(contentLengthHeader) : NaN;
|
||||||
if (Number.isFinite(contentLength) && contentLength > MAX_FILE_SIZE) {
|
if (Number.isFinite(contentLength) && contentLength > effectiveCap) {
|
||||||
return errorResponse(
|
return errorResponse(
|
||||||
new AppError(
|
new AppError(
|
||||||
413,
|
413,
|
||||||
`File exceeds ${MAX_FILE_SIZE} byte cap (Content-Length: ${contentLength})`,
|
`File exceeds ${effectiveCap} byte cap (Content-Length: ${contentLength})`,
|
||||||
'PAYLOAD_TOO_LARGE',
|
'PAYLOAD_TOO_LARGE',
|
||||||
),
|
),
|
||||||
);
|
);
|
||||||
@@ -187,7 +225,7 @@ export async function PUT(
|
|||||||
|
|
||||||
// Read the body into a buffer with a hard cap. Filesystem deployments are
|
// Read the body into a buffer with a hard cap. Filesystem deployments are
|
||||||
// small-tenant (single-node only - see FilesystemBackend boot guard) so
|
// small-tenant (single-node only - see FilesystemBackend boot guard) so
|
||||||
// 50 MB ceiling fits comfortably in heap; no streaming needed.
|
// the ceiling fits comfortably in heap; no streaming needed.
|
||||||
let buffer: Buffer;
|
let buffer: Buffer;
|
||||||
try {
|
try {
|
||||||
const chunks: Buffer[] = [];
|
const chunks: Buffer[] = [];
|
||||||
@@ -197,14 +235,14 @@ export async function PUT(
|
|||||||
const { done, value } = await reader.read();
|
const { done, value } = await reader.read();
|
||||||
if (done) break;
|
if (done) break;
|
||||||
total += value.byteLength;
|
total += value.byteLength;
|
||||||
if (total > MAX_FILE_SIZE) {
|
if (total > effectiveCap) {
|
||||||
try {
|
try {
|
||||||
await reader.cancel();
|
await reader.cancel();
|
||||||
} catch {
|
} catch {
|
||||||
/* ignore */
|
/* ignore */
|
||||||
}
|
}
|
||||||
return errorResponse(
|
return errorResponse(
|
||||||
new AppError(413, `File exceeds ${MAX_FILE_SIZE} byte cap`, 'PAYLOAD_TOO_LARGE'),
|
new AppError(413, `File exceeds ${effectiveCap} byte cap`, 'PAYLOAD_TOO_LARGE'),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
chunks.push(Buffer.from(value));
|
chunks.push(Buffer.from(value));
|
||||||
|
|||||||
@@ -27,17 +27,24 @@ export const GET = withAuth(
|
|||||||
const id = params.id!;
|
const id = params.id!;
|
||||||
const content = await getSalesContentConfig(ctx.portId);
|
const content = await getSalesContentConfig(ctx.portId);
|
||||||
const storageKey = await generateBrochureStorageKey(ctx.portId, id);
|
const storageKey = await generateBrochureStorageKey(ctx.portId, id);
|
||||||
|
const maxBytes = content.brochureMaxUploadMb * 1024 * 1024;
|
||||||
const storage = await getStorageBackend();
|
const storage = await getStorageBackend();
|
||||||
const { url } = await storage.presignUpload(storageKey, {
|
const { url } = await storage.presignUpload(storageKey, {
|
||||||
expirySeconds: 900,
|
expirySeconds: 900,
|
||||||
contentType: 'application/pdf',
|
contentType: 'application/pdf',
|
||||||
|
// Bind the token to the port (engages the filesystem proxy `p`
|
||||||
|
// port-namespace assertion) - audit L22.
|
||||||
|
portSlug: ctx.portSlug,
|
||||||
|
// Embed the per-port cap so the filesystem proxy PUT enforces the
|
||||||
|
// advertised brochure cap rather than the global 50 MB - audit M17.
|
||||||
|
maxBytes,
|
||||||
});
|
});
|
||||||
return NextResponse.json({
|
return NextResponse.json({
|
||||||
data: {
|
data: {
|
||||||
storageKey,
|
storageKey,
|
||||||
uploadUrl: url,
|
uploadUrl: url,
|
||||||
method: 'PUT',
|
method: 'PUT',
|
||||||
maxBytes: content.brochureMaxUploadMb * 1024 * 1024,
|
maxBytes,
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
|
|||||||
@@ -1,7 +1,14 @@
|
|||||||
/**
|
/**
|
||||||
* Returns a presigned URL the browser can use to PUT a PDF directly to the
|
* Returns a presigned URL the browser can use to PUT a PDF directly to the
|
||||||
* active storage backend. The URL is constrained by content-length-range up
|
* active storage backend. `maxBytes` (from `system_settings.berth_pdf_max_upload_mb`,
|
||||||
* to `system_settings.berth_pdf_max_upload_mb` (default 15 MB) per §11.1.
|
* default 15 MB per §11.1) is returned to the client as a hint and used to
|
||||||
|
* early-reject an oversized `sizeBytes` before a URL is minted.
|
||||||
|
*
|
||||||
|
* NOTE (audit M16/M17): the S3 presigned-PUT path does NOT sign a
|
||||||
|
* content-length-range or Content-Type condition, so the cap is enforced
|
||||||
|
* server-side at register time (`uploadBerthPdf` re-HEADs + magic-byte
|
||||||
|
* probes and rejects over-cap bytes). The filesystem proxy path embeds the
|
||||||
|
* cap in the HMAC token (`b` field) and enforces it in the proxy PUT.
|
||||||
*
|
*
|
||||||
* For S3 backends this is a true signed URL; for filesystem backends it's a
|
* For S3 backends this is a true signed URL; for filesystem backends it's a
|
||||||
* CRM-internal proxy URL with an HMAC token (see `FilesystemBackend`).
|
* CRM-internal proxy URL with an HMAC token (see `FilesystemBackend`).
|
||||||
@@ -67,6 +74,9 @@ export const postHandler: RouteHandler = async (req, ctx, params) => {
|
|||||||
contentType: 'application/pdf',
|
contentType: 'application/pdf',
|
||||||
expirySeconds: 900,
|
expirySeconds: 900,
|
||||||
portSlug: ctx.portSlug,
|
portSlug: ctx.portSlug,
|
||||||
|
// Embed the per-port cap in the filesystem proxy token so the proxy
|
||||||
|
// PUT enforces the advertised 15 MB (not the global 50 MB) - audit M17.
|
||||||
|
maxBytes,
|
||||||
});
|
});
|
||||||
|
|
||||||
return NextResponse.json({
|
return NextResponse.json({
|
||||||
|
|||||||
@@ -108,6 +108,15 @@ interface ProxyTokenPayload {
|
|||||||
* tokens always include it.
|
* tokens always include it.
|
||||||
*/
|
*/
|
||||||
p?: string;
|
p?: string;
|
||||||
|
/**
|
||||||
|
* Optional per-port upload byte cap (audit M17). Carried only on `put`
|
||||||
|
* tokens. The proxy PUT handler enforces this in addition to the global
|
||||||
|
* `MAX_FILE_SIZE` ceiling, so a token minted against a 15 MB-capped port
|
||||||
|
* can't be replayed to write a 50 MB object. Absent on `get` tokens and
|
||||||
|
* on tokens minted before this field shipped (those fall back to the
|
||||||
|
* global ceiling).
|
||||||
|
*/
|
||||||
|
b?: number;
|
||||||
}
|
}
|
||||||
|
|
||||||
function b64urlEncode(buf: Buffer): string {
|
function b64urlEncode(buf: Buffer): string {
|
||||||
@@ -329,6 +338,9 @@ export class FilesystemBackend implements StorageBackend {
|
|||||||
op: 'put',
|
op: 'put',
|
||||||
c: opts.contentType,
|
c: opts.contentType,
|
||||||
...(opts.portSlug ? { p: opts.portSlug } : {}),
|
...(opts.portSlug ? { p: opts.portSlug } : {}),
|
||||||
|
...(typeof opts.maxBytes === 'number' && Number.isFinite(opts.maxBytes)
|
||||||
|
? { b: opts.maxBytes }
|
||||||
|
: {}),
|
||||||
},
|
},
|
||||||
this.hmacSecret,
|
this.hmacSecret,
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -11,6 +11,8 @@
|
|||||||
* truth.
|
* truth.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
|
import { createHash } from 'node:crypto';
|
||||||
|
|
||||||
import { and, eq, isNull } from 'drizzle-orm';
|
import { and, eq, isNull } from 'drizzle-orm';
|
||||||
|
|
||||||
import { db } from '@/lib/db';
|
import { db } from '@/lib/db';
|
||||||
@@ -47,6 +49,16 @@ export interface PresignOpts {
|
|||||||
* slug, so this is the matching enforcement.
|
* slug, so this is the matching enforcement.
|
||||||
*/
|
*/
|
||||||
portSlug?: string;
|
portSlug?: string;
|
||||||
|
/**
|
||||||
|
* Optional per-port upload byte cap for presigned uploads. Embedded in
|
||||||
|
* the filesystem proxy token (`b` field) and enforced by the proxy PUT
|
||||||
|
* handler, so a token minted against a 15 MB-capped port can't be used
|
||||||
|
* to write a 50 MB object (audit M17). S3 presigned PUTs can't sign a
|
||||||
|
* content-length-range on this path, so the cap there is re-checked
|
||||||
|
* server-side at register time. When unset, the proxy falls back to the
|
||||||
|
* global `MAX_FILE_SIZE` ceiling.
|
||||||
|
*/
|
||||||
|
maxBytes?: number;
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface StorageBackend {
|
export interface StorageBackend {
|
||||||
@@ -209,7 +221,12 @@ async function loadStorageConfig(): Promise<StorageConfigSnapshot> {
|
|||||||
* client is held in memory until the next mismatch.
|
* client is held in memory until the next mismatch.
|
||||||
*/
|
*/
|
||||||
function fingerprint(cfg: StorageConfigSnapshot): string {
|
function fingerprint(cfg: StorageConfigSnapshot): string {
|
||||||
return JSON.stringify(cfg);
|
// L17(c): hash the serialized config rather than holding the decrypted S3
|
||||||
|
// access key verbatim in a process-lifetime string. A SHA-256 digest still
|
||||||
|
// changes whenever any field (including the secret material) rotates, so
|
||||||
|
// the cache-invalidation semantics are unchanged, but the cleartext secret
|
||||||
|
// no longer lingers in the cache key.
|
||||||
|
return createHash('sha256').update(JSON.stringify(cfg)).digest('hex');
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -282,6 +282,28 @@ export class S3Backend implements StorageBackend {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Mint a presigned PUT URL for a direct browser upload.
|
||||||
|
*
|
||||||
|
* IMPORTANT (audit M16): `presignedPutObject` signs ONLY the bucket+key+
|
||||||
|
* expiry. It does NOT constrain the request's `Content-Type` or
|
||||||
|
* `Content-Length`, so a holder of this URL can PUT any bytes, of any
|
||||||
|
* type, up to the storage provider's own object-size ceiling for the
|
||||||
|
* 15-minute window. The `opts.contentType` / `opts.maxBytes` hints are
|
||||||
|
* advisory only on this path.
|
||||||
|
*
|
||||||
|
* Every consumer of an S3 presigned upload MUST re-validate the object
|
||||||
|
* server-side after the browser PUT (HEAD for size + magic-byte probe)
|
||||||
|
* and delete it on mismatch - the berth-PDF (`uploadBerthPdf`) and
|
||||||
|
* brochure (`registerBrochureVersion`) register endpoints already do
|
||||||
|
* this. Do NOT add a new presigned-upload consumer that trusts the
|
||||||
|
* uploaded bytes without that re-check.
|
||||||
|
*
|
||||||
|
* A stricter alternative is `presignedPostPolicy`, which DOES sign a
|
||||||
|
* `content-length-range` + `Content-Type` condition; it's deferred here
|
||||||
|
* because it changes the upload from a PUT to a multipart POST and every
|
||||||
|
* current caller is wired for PUT.
|
||||||
|
*/
|
||||||
async presignUpload(
|
async presignUpload(
|
||||||
key: string,
|
key: string,
|
||||||
opts: PresignOpts,
|
opts: PresignOpts,
|
||||||
|
|||||||
Reference in New Issue
Block a user