refactor(plan): unify bucket validation + lock root-verb routing

- web-api create-bucket and s3 virtual-host now use BucketNameSchema
  (single canonical validator; no inline regex left in src/)
- routes '/': HEAD/DELETE/POST now check shouldHandleS3 with headers
  like every other branch (non-S3 -> 404, never S3-direct blind)
- s3-routing.test.ts: 3 new tests locking the fixed root verbs
This commit is contained in:
asepharyana
2026-09-14 18:17:18 +07:00
parent da7fca9fdc
commit e610f1ca21
4 changed files with 38 additions and 9 deletions
@@ -11,6 +11,7 @@ import { sanitizeFilenameHeader } from '../../../shared/http/filename';
import logger from '../../../shared/logger/index'; import logger from '../../../shared/logger/index';
import { getErrorMessage } from '../../../shared/utils/file'; import { getErrorMessage } from '../../../shared/utils/file';
import { streamToTemp } from '../../../shared/utils/temp-stream'; import { streamToTemp } from '../../../shared/utils/temp-stream';
import { BucketNameSchema } from '../../../shared/validation/schemas';
/** Lazily built upload use case wired to the DI singletons. */ /** Lazily built upload use case wired to the DI singletons. */
const getUploadUseCase = () => const getUploadUseCase = () =>
@@ -80,7 +81,7 @@ export const handleListBucketsV1 = async (): Promise<Response> => {
*/ */
export const handleCreateBucketV1 = async (req: Request): Promise<Response> => { export const handleCreateBucketV1 = async (req: Request): Promise<Response> => {
const body = (await req.json()) as { name?: string }; const body = (await req.json()) as { name?: string };
if (!body.name || !/^[a-z0-9][a-z0-9.-]{1,61}[a-z0-9]$/.test(body.name)) { if (!body.name || !BucketNameSchema.safeParse(body.name).success) {
return jsonError('Invalid bucket name. Use lowercase, 3-63 chars, no underscore', 400); return jsonError('Invalid bucket name. Use lowercase, 3-63 chars, no underscore', 400);
} }
const existing = await bucketRepository.findByName(body.name); const existing = await bucketRepository.findByName(body.name);
+12 -3
View File
@@ -100,9 +100,18 @@ export const routes = {
if (shouldHandleS3(req, headers)) return handleS3Direct(req); if (shouldHandleS3(req, headers)) return handleS3Direct(req);
return Promise.resolve(new Response('Not Allowed', { status: 405 })); return Promise.resolve(new Response('Not Allowed', { status: 405 }));
}, },
HEAD: handleS3Direct, HEAD: (req: Request): Promise<Response> => {
DELETE: handleS3Direct, if (shouldHandleS3(req, Object.fromEntries(req.headers))) return handleS3Direct(req);
POST: handleS3Direct, return Promise.resolve(new Response('Not Found', { status: 404 }));
},
DELETE: (req: Request): Promise<Response> => {
if (shouldHandleS3(req, Object.fromEntries(req.headers))) return handleS3Direct(req);
return Promise.resolve(new Response('Not Found', { status: 404 }));
},
POST: (req: Request): Promise<Response> => {
if (shouldHandleS3(req, Object.fromEntries(req.headers))) return handleS3Direct(req);
return Promise.resolve(new Response('Not Found', { status: 404 }));
},
OPTIONS: handleCatchAllOptions, OPTIONS: handleCatchAllOptions,
}, },
// Catch-all for S3 path-style requests (/{bucket}/{key} ...) // Catch-all for S3 path-style requests (/{bucket}/{key} ...)
+7 -5
View File
@@ -7,11 +7,13 @@ const stripPort = (host: string): string => {
return host.split(':')[0].toLowerCase().replace(/\.$/, ''); return host.split(':')[0].toLowerCase().replace(/\.$/, '');
}; };
const isValidBucketLabel = (bucket: string): boolean => import { BucketNameSchema } from '../../shared/validation/schemas';
/^[a-z0-9][a-z0-9.-]{1,61}[a-z0-9]$/.test(bucket) &&
!bucket.includes('..') && /**
!bucket.includes('.-') && * Validates a virtual-hosted bucket label against the single canonical
!bucket.includes('-.'); * bucket-name schema (same rules as bucket creation).
*/
const isValidBucketLabel = (bucket: string): boolean => BucketNameSchema.safeParse(bucket).success;
export const extractS3BucketFromHost = (host: string, domains: string[]): string | null => { export const extractS3BucketFromHost = (host: string, domains: string[]): string | null => {
const normalizedHost = stripPort(host); const normalizedHost = stripPort(host);
+17
View File
@@ -129,4 +129,21 @@ describe('S3 routing (routes table)', () => {
); );
expect(res.status).toBe(204); expect(res.status).toBe(204);
}); });
it('answers HEAD / without S3 headers as 404 (never S3-direct)', async () => {
const res = await routes['/'].HEAD(new Request('http://localhost:4000/', { method: 'HEAD' }));
expect(res.status).toBe(404);
});
it('answers DELETE / without S3 headers as 404 (never S3-direct)', async () => {
const res = await routes['/'].DELETE(
new Request('http://localhost:4000/', { method: 'DELETE' }),
);
expect(res.status).toBe(404);
});
it('answers POST / without S3 headers as 404 (never S3-direct)', async () => {
const res = await routes['/'].POST(new Request('http://localhost:4000/', { method: 'POST' }));
expect(res.status).toBe(404);
});
}); });