diff --git a/.env.example b/.env.example index a1bbc09..3cb45b1 100644 --- a/.env.example +++ b/.env.example @@ -19,9 +19,19 @@ REVERSE_GEOCODING_USER_AGENT=Soundlog/0.1 (+https://github.com/SoundLogTeam/Soun TOUR_API_BASE_URL=https://apis.data.go.kr/B551011/KorService2 TOUR_API_SERVICE_KEY= ALLOW_DEV_AUTH_FALLBACK=false +# Rate limiting for /v1/auth/login, /v1/auth/register, /v1/auth/refresh. +# AUTH_RATE_LIMIT_ENABLED defaults to true, except under NODE_ENV=test where +# it defaults to false so test suites can call auth endpoints repeatedly. +# AUTH_RATE_LIMIT_ENABLED=true +# Per-account limit (keyed by IP + email): blocks repeated attempts against one account. +AUTH_RATE_LIMIT_WINDOW_MS=900000 +AUTH_RATE_LIMIT_MAX=10 +# Per-IP limit (keyed by IP only): blocks credential stuffing across many different +# emails from the same IP. Looser than the per-account limit above. +AUTH_RATE_LIMIT_IP_WINDOW_MS=900000 +AUTH_RATE_LIMIT_IP_MAX=40 UPLOAD_DIRECTORY=uploads UPLOAD_PUBLIC_BASE_URL=http://localhost:4000 -UPLOAD_PUBLIC_PATH=/uploads USE_MOCK_DB=false # Production checklist: @@ -29,6 +39,5 @@ USE_MOCK_DB=false # CLIENT_URLS=https://soundlog.shop,https://www.soundlog.shop # After API DNS/HTTPS reverse proxy is ready: # UPLOAD_PUBLIC_BASE_URL=https://api.soundlog.shop -# UPLOAD_PUBLIC_PATH=/uploads # USE_MOCK_DB=false # ALLOW_DEV_AUTH_FALLBACK=false diff --git a/README.md b/README.md index be88593..082c45c 100644 --- a/README.md +++ b/README.md @@ -100,7 +100,7 @@ Swagger에서 바로 DB 쓰기를 확인할 때는 인증 없이 호출 가능 - 자체 이메일/비밀번호 로그인만 사용하며, 서버는 비밀번호 원문 대신 bcrypt hash만 저장 - `CLIENT_URLS`, `UPLOAD_PUBLIC_BASE_URL`, 앱의 `EXPO_PUBLIC_SOUNDLOG_API_BASE_URL`은 HTTPS 도메인 사용 - 운영 기준 frontend origin은 `https://soundlog.shop`입니다. 공개 API URL은 `https://api.soundlog.shop`이며, GCP VM 위의 Caddy가 TLS를 직접 종료합니다. -- `REQUEST_BODY_LIMIT`, `MOMENT_PHOTO_MAX_FILE_SIZE_MB`, `UPLOAD_DIRECTORY`, `UPLOAD_PUBLIC_PATH`는 운영 파일 업로드 정책에 맞게 조정 +- `REQUEST_BODY_LIMIT`, `MOMENT_PHOTO_MAX_FILE_SIZE_MB`, `UPLOAD_DIRECTORY`는 운영 파일 업로드 정책에 맞게 조정 - iOS 앱 설정에 전체 ATS 예외를 넣지 않기 서버 코드는 자체 계정 로그인(`POST /v1/auth/login`, `POST /v1/auth/register`)으로 Soundlog access/refresh token을 발급합니다. diff --git a/docs/gcp-deployment.md b/docs/gcp-deployment.md index 2968c19..4eb7610 100644 --- a/docs/gcp-deployment.md +++ b/docs/gcp-deployment.md @@ -54,7 +54,6 @@ TOUR_API_BASE_URL=https://apis.data.go.kr/B551011/KorService2 TOUR_API_SERVICE_KEY= ALLOW_DEV_AUTH_FALLBACK=false UPLOAD_PUBLIC_BASE_URL=https://api.soundlog.shop -UPLOAD_PUBLIC_PATH=/uploads USE_MOCK_DB=false ``` diff --git a/package.json b/package.json index a4158b8..e26cbd3 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,7 @@ "cors": "^2.8.5", "dotenv": "^17.2.3", "express": "^5.1.0", + "express-rate-limit": "^8.6.1", "helmet": "^8.1.0", "jsonwebtoken": "^9.0.2", "morgan": "^1.10.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7fb45b3..a5fce33 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -27,6 +27,9 @@ importers: express: specifier: ^5.1.0 version: 5.2.1 + express-rate-limit: + specifier: ^8.6.1 + version: 8.6.1(express@5.2.1) helmet: specifier: ^8.1.0 version: 8.2.0 @@ -371,36 +374,42 @@ packages: engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [linux] + libc: [glibc] '@rolldown/binding-linux-arm64-musl@1.0.3': resolution: {integrity: sha512-VWkUHwWriDciit80wleYwKILoR/KMvxh/IdwS/paX+ZgpuRpCrKLUdadJbc0NpBEiyhpYawsJ73j9aCvOH+f7Q==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [linux] + libc: [musl] '@rolldown/binding-linux-ppc64-gnu@1.0.3': resolution: {integrity: sha512-5f1laC0SlIR0yDbFCd8acUhvJIag6N3zC5P7oUPN6wX0aOma+uKJ0wBDH5aq7I1PVI2ttTlhJwzwRIBnLiSGEg==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [ppc64] os: [linux] + libc: [glibc] '@rolldown/binding-linux-s390x-gnu@1.0.3': resolution: {integrity: sha512-Iq4ko0r4XsgbrF/LunNgHtAGLRRVE2kXonAXQ/MV0mC6jQpMOhW1SvtZja2EhC/kd05++bP78dsqBeIQyYJ6Yg==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [s390x] os: [linux] + libc: [glibc] '@rolldown/binding-linux-x64-gnu@1.0.3': resolution: {integrity: sha512-B8m6tD5+/N5FeNQFbKlLA/2yVq9ycQP1SeedyEYYKWBNR3ZQbkvIUcNnDNM03lO1l5F2roiiFJGgvoLLyZXtSg==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [linux] + libc: [glibc] '@rolldown/binding-linux-x64-musl@1.0.3': resolution: {integrity: sha512-pSdpdUJHkuCxun9LE7jvgUB9qsRgaiyNNCX7m/AvHTcq67AiT/Yhoxvw5zPfhrM8k/BfP8ce/hMOpthKDpEUow==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [linux] + libc: [musl] '@rolldown/binding-openharmony-arm64@1.0.3': resolution: {integrity: sha512-OXXS3RKJgX2uLwM+gYyuH5omcH8fL1LJs96pZGgtetVCahON57+d4SJHzTgZiOjxgGkSnpXpOsWuPDGAKAigEg==} @@ -780,6 +789,12 @@ packages: resolution: {integrity: sha512-knvyeauYhqjOYvQ66MznSMs83wmHrCycNEN6Ao+2AeYEfxUIkuiVxdEa1qlGEPK+We3n0THiDciYSsCcgW/DoA==} engines: {node: '>=12.0.0'} + express-rate-limit@8.6.1: + resolution: {integrity: sha512-0D493aP61w0TJ2A0wy27riRsO7FMQ7FK+KUHOKCSfPvYo0R55aiC6emCVgFUeShH0fq0ICPVzNcgoS+BsbXQCA==} + engines: {node: '>= 16'} + peerDependencies: + express: '>= 4.11' + express@5.2.1: resolution: {integrity: sha512-hIS4idWWai69NezIdRt2xFVofaF4j+6INOpJlVOLDO8zXGpUVEVzIYk12UUi2JzjEzWL3IOAxcTubgz9Po0yXw==} engines: {node: '>= 18'} @@ -881,6 +896,10 @@ packages: inherits@2.0.4: resolution: {integrity: sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==} + ip-address@10.4.0: + resolution: {integrity: sha512-oSK96Grm3aP6OrS263xVxbNDGVL7rzBtYdpGqlDG8iQdoenDoTs/nkki+DflYbAEE8Xl6o5YxhxlrKvI3nqKXQ==} + engines: {node: '>= 12'} + ipaddr.js@1.9.1: resolution: {integrity: sha512-0KI/607xoxSToH7GjN1FfSbLoU0+btTicjsQSWQlh/hZykN8KpmMf7uYwPW3R+akZ6R/w18ZlXSHBYXiYUPO3g==} engines: {node: '>= 0.10'} @@ -952,24 +971,28 @@ packages: engines: {node: '>= 12.0.0'} cpu: [arm64] os: [linux] + libc: [glibc] lightningcss-linux-arm64-musl@1.32.0: resolution: {integrity: sha512-UpQkoenr4UJEzgVIYpI80lDFvRmPVg6oqboNHfoH4CQIfNA+HOrZ7Mo7KZP02dC6LjghPQJeBsvXhJod/wnIBg==} engines: {node: '>= 12.0.0'} cpu: [arm64] os: [linux] + libc: [musl] lightningcss-linux-x64-gnu@1.32.0: resolution: {integrity: sha512-V7Qr52IhZmdKPVr+Vtw8o+WLsQJYCTd8loIfpDaMRWGUZfBOYEJeyJIkqGIDMZPwPx24pUMfwSxxI8phr/MbOA==} engines: {node: '>= 12.0.0'} cpu: [x64] os: [linux] + libc: [glibc] lightningcss-linux-x64-musl@1.32.0: resolution: {integrity: sha512-bYcLp+Vb0awsiXg/80uCRezCYHNg1/l3mt0gzHnWV9XP1W5sKa5/TCdGWaR/zBM2PeF/HbsQv/j2URNOiVuxWg==} engines: {node: '>= 12.0.0'} cpu: [x64] os: [linux] + libc: [musl] lightningcss-win32-arm64-msvc@1.32.0: resolution: {integrity: sha512-8SbC8BR40pS6baCM8sbtYDSwEVQd4JlFTOlaD3gWGHfThTcABnNDBda6eTZeqbofalIJhFx0qKzgHJmcPTnGdw==} @@ -2048,6 +2071,14 @@ snapshots: expect-type@1.3.0: {} + express-rate-limit@8.6.1(express@5.2.1): + dependencies: + debug: 4.4.3 + express: 5.2.1 + ip-address: 10.4.0 + transitivePeerDependencies: + - supports-color + express@5.2.1: dependencies: accepts: 2.0.0 @@ -2186,6 +2217,8 @@ snapshots: inherits@2.0.4: {} + ip-address@10.4.0: {} + ipaddr.js@1.9.1: {} is-promise@4.0.0: {} diff --git a/scripts/check-live-e2e.mjs b/scripts/check-live-e2e.mjs index c87ab19..721501e 100644 --- a/scripts/check-live-e2e.mjs +++ b/scripts/check-live-e2e.mjs @@ -126,7 +126,7 @@ let primary; let companion; try { - await step('system health, OpenAPI, docs, and DB write', async () => { + await step('system health, OpenAPI, and docs', async () => { const health = await request('/v1/health'); assert(health.payload?.data?.status === 'ok', 'Health status is not ok.'); assert(health.payload?.data?.database === 'ok', 'Database status is not ok.'); @@ -135,12 +135,10 @@ try { assert(String(openApi.payload).includes('openapi: 3.1.0'), 'OpenAPI document is missing.'); await request('/docs/', { expectedStatus: 200 }); - const dbRecord = await request('/v1/dev/db-test-records', { - body: { label: `live-e2e-${runId}`, payload: { source: 'check-live-e2e' } }, - expectedStatus: 201, - method: 'POST', - }); - assert(dbRecord.payload?.data?.id, 'DB test write did not return an id.'); + // /v1/dev/db-test-records now requires auth and is unregistered in + // production, so it is no longer exercised by this unauthenticated + // smoke step. DB write behavior is still covered by the authenticated + // steps below (registration, recap captures, etc.). }); await step('register, login, refresh, profile, and migration', async () => { diff --git a/scripts/check-production-env.mjs b/scripts/check-production-env.mjs index 1c84d19..f0c69aa 100644 --- a/scripts/check-production-env.mjs +++ b/scripts/check-production-env.mjs @@ -40,6 +40,10 @@ if (process.env.ALLOW_DEV_AUTH_FALLBACK === 'true') { addError('ALLOW_DEV_AUTH_FALLBACK must be false or unset in production.'); } +if (process.env.AUTH_RATE_LIMIT_ENABLED === 'false') { + addError('AUTH_RATE_LIMIT_ENABLED must not be false in production.'); +} + if (!isHttpsUrl(process.env.UPLOAD_PUBLIC_BASE_URL)) { addError('UPLOAD_PUBLIC_BASE_URL must be an HTTPS URL.'); } diff --git a/src/app.ts b/src/app.ts index 7285583..13e2338 100644 --- a/src/app.ts +++ b/src/app.ts @@ -10,22 +10,27 @@ import { errorMiddleware } from './middlewares/error.middleware.js'; import { requestLoggerMiddleware } from './middlewares/request-logger.middleware.js'; import { securityMiddleware } from './middlewares/security.middleware.js'; import { registerSwaggerDocs } from './middlewares/swagger.middleware.js'; -import { - uploadedFilesPublicPath, - uploadedFilesStaticMiddleware, -} from './middlewares/upload.middleware.js'; import { createApiRouter } from './routes/index.js'; +import { createUploadsRouter } from './routes/uploads.router.js'; import { notFound } from './utils/http-error.js'; export function createApp() { const app = express(); + // Behind a single Caddy reverse proxy hop (see Caddyfile / docker-compose.prod.yml). + // Trusting exactly 1 hop lets req.ip reflect the real client IP (needed for + // rate limiting) without allowing X-Forwarded-For spoofing from the client. + app.set('trust proxy', 1); + app.use(corsMiddleware); app.use(securityMiddleware); app.use(jsonBodyParserMiddleware); app.use(urlencodedBodyParserMiddleware); app.use(requestLoggerMiddleware); - app.use(uploadedFilesPublicPath, uploadedFilesStaticMiddleware); + // Uploaded photos are served only through an authenticated, ownership/visibility-checked + // endpoint (see uploads.router.ts) — there is no unauthenticated static file serving of + // the uploads directory. + app.use(createUploadsRouter()); registerSwaggerDocs(app); app.use(createApiRouter()); app.use((_req, _res, next) => { diff --git a/src/config/env.ts b/src/config/env.ts index 8026293..970a193 100644 --- a/src/config/env.ts +++ b/src/config/env.ts @@ -7,6 +7,11 @@ const envSchema = z.object({ .string() .optional() .transform((value) => value === 'true'), + AUTH_RATE_LIMIT_ENABLED: z.string().optional(), + AUTH_RATE_LIMIT_IP_MAX: z.coerce.number().int().positive().default(40), + AUTH_RATE_LIMIT_IP_WINDOW_MS: z.coerce.number().int().positive().default(15 * 60 * 1000), + AUTH_RATE_LIMIT_MAX: z.coerce.number().int().positive().default(10), + AUTH_RATE_LIMIT_WINDOW_MS: z.coerce.number().int().positive().default(15 * 60 * 1000), CLIENT_URL: z.string().url().default('http://localhost:8081'), CLIENT_URLS: z.string().optional(), DATABASE_URL: z.string().min(1), @@ -37,7 +42,17 @@ const envSchema = z.object({ .transform((value) => value === 'true'), UPLOAD_DIRECTORY: z.string().min(1).default('uploads'), UPLOAD_PUBLIC_BASE_URL: z.string().url().default('http://localhost:4000'), - UPLOAD_PUBLIC_PATH: z.string().min(1).default('/uploads'), }); -export const env = envSchema.parse(process.env); +const parsedEnv = envSchema.parse(process.env); + +export const env = { + ...parsedEnv, + // Defaults to disabled under NODE_ENV=test so existing tests that call + // auth endpoints repeatedly are not destabilized. Set + // AUTH_RATE_LIMIT_ENABLED=true explicitly to exercise the limiter in tests. + AUTH_RATE_LIMIT_ENABLED: + parsedEnv.AUTH_RATE_LIMIT_ENABLED === undefined + ? parsedEnv.NODE_ENV !== 'test' + : parsedEnv.AUTH_RATE_LIMIT_ENABLED === 'true', +}; diff --git a/src/controllers/upload-file.controller.ts b/src/controllers/upload-file.controller.ts new file mode 100644 index 0000000..b4abf5b --- /dev/null +++ b/src/controllers/upload-file.controller.ts @@ -0,0 +1,37 @@ +import type { Request, Response } from 'express'; + +import { requireUser } from '../middlewares/auth.middleware.js'; +import { uploadFileService } from '../services/upload-file.service.js'; +import { notFound } from '../utils/http-error.js'; + +export const uploadFileController = { + async getUploadedFile(req: Request, res: Response) { + const user = requireUser(req); + const fileId = String(req.params.fileId); + + const resolved = await uploadFileService.resolveUploadedFileForUser(user.id, fileId); + + // Unknown file, disallowed access, and invalid/traversal file ids all resolve the + // same way (undefined) and all produce the same 404, so a caller cannot use the + // response to tell a private file that doesn't belong to them apart from a file + // that simply doesn't exist. + if (!resolved) { + throw notFound(); + } + + // multer stores uploads with no extension, so Content-Type must be derived from the + // file's actual bytes (never the client-supplied upload MIME type or a filename), or + // helmet's `X-Content-Type-Options: nosniff` leaves browsers refusing to render it. If + // the bytes don't match a known image signature, the file is not served as an image at + // all — this also covers legacy/unexpected on-disk files that happen to have a matching + // DB row but aren't actually images. + const contentType = await uploadFileService.detectImageContentType(resolved.absolutePath); + + if (!contentType) { + throw notFound(); + } + + res.type(contentType); + res.sendFile(resolved.absolutePath); + }, +}; diff --git a/src/middlewares/rate-limit.middleware.ts b/src/middlewares/rate-limit.middleware.ts new file mode 100644 index 0000000..4142950 --- /dev/null +++ b/src/middlewares/rate-limit.middleware.ts @@ -0,0 +1,77 @@ +import type { Request, Response } from 'express'; +import { ipKeyGenerator, rateLimit } from 'express-rate-limit'; + +import { env } from '../config/env.js'; + +function accountKeyGenerator(req: Request) { + const email = + typeof req.body?.email === 'string' ? req.body.email.trim().toLowerCase() : ''; + const ipKey = ipKeyGenerator(req.ip ?? 'unknown'); + + // Endpoints without an email in the body (e.g. refresh) fall back to an + // IP-only key here; the IP-only limiter below still applies independently. + return email ? `${ipKey}:${email}` : ipKey; +} + +function ipKeyGeneratorForRequest(req: Request) { + return ipKeyGenerator(req.ip ?? 'unknown'); +} + +function handler(_req: Request, res: Response) { + res.status(429).json({ + error: { + code: 'TOO_MANY_REQUESTS', + message: '요청이 너무 많습니다. 잠시 후 다시 시도해주세요.', + details: {}, + }, + }); +} + +const skip = () => !env.AUTH_RATE_LIMIT_ENABLED; + +/** + * Applies only to the auth endpoints that are most attractive to credential + * stuffing / brute-force / account-enumeration attacks + * (login, register, refresh). Not applied API-wide. + * + * Per-account limiter (keyed by IP + email): stops repeated attempts against + * a single known account. On its own this does NOT stop credential + * stuffing, where an attacker tries many different emails from the same IP + * — each email is a fresh bucket. See authIpRateLimitMiddleware below for + * that case; both are chained on the routes. + * + * Disabled by default under NODE_ENV=test (see src/config/env.ts) so it does + * not destabilize existing test suites that call these endpoints repeatedly; + * can be forced on via AUTH_RATE_LIMIT_ENABLED=true for dedicated tests. + */ +export const authAccountRateLimitMiddleware = rateLimit({ + handler, + keyGenerator: accountKeyGenerator, + legacyHeaders: false, + limit: env.AUTH_RATE_LIMIT_MAX, + skip, + standardHeaders: true, + windowMs: env.AUTH_RATE_LIMIT_WINDOW_MS, +}); + +/** + * Per-IP limiter (keyed by IP only, ignores email/account). This is the + * actual defense against credential stuffing, where an attacker cycles + * through many different email addresses from one IP — the account-scoped + * limiter above would treat each attempt as a fresh bucket, but this one + * catches the aggregate volume from that IP regardless of which account is + * targeted. Deliberately looser than the account limit so legitimate users + * behind shared/NAT IPs are unlikely to be affected. + * + * Shares the same enable/disable and response behavior as the account + * limiter above. + */ +export const authIpRateLimitMiddleware = rateLimit({ + handler, + keyGenerator: ipKeyGeneratorForRequest, + legacyHeaders: false, + limit: env.AUTH_RATE_LIMIT_IP_MAX, + skip, + standardHeaders: true, + windowMs: env.AUTH_RATE_LIMIT_IP_WINDOW_MS, +}); diff --git a/src/middlewares/upload.middleware.ts b/src/middlewares/upload.middleware.ts index 7536193..f617669 100644 --- a/src/middlewares/upload.middleware.ts +++ b/src/middlewares/upload.middleware.ts @@ -1,4 +1,3 @@ -import express from 'express'; import multer from 'multer'; import { env } from '../config/env.js'; @@ -7,28 +6,50 @@ const BYTES_PER_MEGABYTE = 1024 * 1024; const MOMENT_PHOTO_MAX_FILE_SIZE_BYTES = env.MOMENT_PHOTO_MAX_FILE_SIZE_MB * BYTES_PER_MEGABYTE; -function normalizePublicPath(value: string) { - const trimmed = value.trim(); - const withLeadingSlash = trimmed.startsWith('/') ? trimmed : `/${trimmed}`; - - return withLeadingSlash.replace(/\/+$/, '') || '/'; -} - -export const uploadedFilesPublicPath = normalizePublicPath(env.UPLOAD_PUBLIC_PATH); - -export const uploadedFilesStaticMiddleware = express.static(env.UPLOAD_DIRECTORY); +// Client-declared MIME types accepted at upload time. This is a cheap, spoofable +// first line of defense (multer's fileFilter only sees the multipart part's declared +// Content-Type, not the actual bytes) that simply stops obviously-wrong uploads (PDFs, +// executables, etc.) from ever being written to disk. The real security boundary is the +// magic-byte sniff performed when serving the file back out (see +// upload-file.service.ts#detectImageContentType), which never trusts this value. +const ALLOWED_MOMENT_PHOTO_MIME_TYPES = new Set([ + 'image/gif', + 'image/heic', + 'image/heif', + 'image/jpeg', + 'image/png', + 'image/webp', +]); + +// multer's default disk storage names files with crypto.randomBytes(16).toString('hex'), +// i.e. exactly 32 lowercase hex characters and nothing else (no path separators, no dots). +// This pattern is the single source of truth for what a valid stored filename looks like, +// and is used both to generate public-facing file ids and to validate/reject any client +// supplied file id (blocking path traversal, absolute paths, encoded separators, etc.) +// before it is ever used to build a filesystem path. +export const UPLOAD_FILE_ID_PATTERN = /^[a-f0-9]{32}$/; + +export const UPLOADED_FILE_ROUTE_PATH = '/v1/uploads'; export const momentPhotoUpload = multer({ dest: env.UPLOAD_DIRECTORY, + fileFilter: (_req, file, callback) => { + if (!ALLOWED_MOMENT_PHOTO_MIME_TYPES.has(file.mimetype)) { + // Using multer's own error type (rather than an arbitrary Error) means the existing + // errorMiddleware `multer.MulterError` branch turns this into a 400 automatically — + // no changes needed there. Rejecting via this callback happens before multer's + // storage engine writes anything for this part, so no file is left on disk. + callback(new multer.MulterError('LIMIT_UNEXPECTED_FILE', file.fieldname)); + return; + } + + callback(null, true); + }, limits: { fileSize: MOMENT_PHOTO_MAX_FILE_SIZE_BYTES, }, }); export function createUploadedFilePublicPath(filename: string) { - if (uploadedFilesPublicPath === '/') { - return `/${filename}`; - } - - return `${uploadedFilesPublicPath}/${filename}`; + return `${UPLOADED_FILE_ROUTE_PATH}/${filename}`; } diff --git a/src/routes/index.ts b/src/routes/index.ts index ce99e3a..e6aa9ae 100644 --- a/src/routes/index.ts +++ b/src/routes/index.ts @@ -16,8 +16,13 @@ import { travelSessionController, trendController, } from '../controllers/index.js'; +import { env } from '../config/env.js'; import { asyncHandler } from '../utils/async-handler.js'; import { authMiddleware } from '../middlewares/auth.middleware.js'; +import { + authAccountRateLimitMiddleware, + authIpRateLimitMiddleware, +} from '../middlewares/rate-limit.middleware.js'; import { momentPhotoUpload } from '../middlewares/upload.middleware.js'; import { validate } from '../middlewares/validate.middleware.js'; import { @@ -40,24 +45,37 @@ export function createApiRouter() { const router = Router(); router.get('/v1/health', asyncHandler(systemController.getHealth)); - router.post( - '/v1/dev/db-test-records', - validate({ body: devDbTestValidators.createBody }), - asyncHandler(devDbTestController.createRecord), - ); + + // Dev-only DB smoke-test endpoint. Never registered in production, and + // requires auth everywhere else (see src/services/dev-db-test.service.ts + // for a defensive production guard as well). + if (env.NODE_ENV !== 'production') { + router.post( + '/v1/dev/db-test-records', + authMiddleware, + validate({ body: devDbTestValidators.createBody }), + asyncHandler(devDbTestController.createRecord), + ); + } router.post( '/v1/auth/login', + authIpRateLimitMiddleware, + authAccountRateLimitMiddleware, validate({ body: authValidators.loginBody }), asyncHandler(authController.login), ); router.post( '/v1/auth/register', + authIpRateLimitMiddleware, + authAccountRateLimitMiddleware, validate({ body: authValidators.registerBody }), asyncHandler(authController.register), ); router.post( '/v1/auth/refresh', + authIpRateLimitMiddleware, + authAccountRateLimitMiddleware, validate({ body: authValidators.refreshBody }), asyncHandler(authController.refresh), ); diff --git a/src/routes/uploads.router.ts b/src/routes/uploads.router.ts new file mode 100644 index 0000000..f7540fd --- /dev/null +++ b/src/routes/uploads.router.ts @@ -0,0 +1,18 @@ +import { Router } from 'express'; + +import { uploadFileController } from '../controllers/upload-file.controller.js'; +import { authMiddleware } from '../middlewares/auth.middleware.js'; +import { UPLOADED_FILE_ROUTE_PATH } from '../middlewares/upload.middleware.js'; +import { asyncHandler } from '../utils/async-handler.js'; + +export function createUploadsRouter() { + const router = Router(); + + router.get( + `${UPLOADED_FILE_ROUTE_PATH}/:fileId`, + authMiddleware, + asyncHandler(uploadFileController.getUploadedFile), + ); + + return router; +} diff --git a/src/services/dev-db-test.service.ts b/src/services/dev-db-test.service.ts index 194113a..21e8e4a 100644 --- a/src/services/dev-db-test.service.ts +++ b/src/services/dev-db-test.service.ts @@ -40,6 +40,13 @@ function toDto(record: { export const devDbTestService = { async createRecord(input: CreateDbTestRecordInput) { + // Defense in depth: this endpoint must never write to the database in + // production, even if it were ever reachable there (e.g. misconfigured + // routing). The route itself is also not registered in production. + if (env.NODE_ENV === 'production') { + throw new Error('devDbTestService.createRecord is disabled in production.'); + } + const label = input.label ?? 'swagger-db-test'; const payload = input.payload ?? {}; diff --git a/src/services/soundlog.service.ts b/src/services/soundlog.service.ts index b67b83e..e91af4f 100644 --- a/src/services/soundlog.service.ts +++ b/src/services/soundlog.service.ts @@ -25,6 +25,7 @@ import path from 'node:path'; import { env } from '../config/env.js'; import { ERROR_MESSAGES } from '../constants/error.constants.js'; import { prisma } from '../config/prisma.js'; +import { UPLOAD_FILE_ID_PATTERN } from '../middlewares/upload.middleware.js'; import { getLimit, paginateByCursor } from '../utils/pagination.js'; import { createPublicId } from '../utils/tokens.js'; import { badRequest, forbidden, notFound } from '../utils/http-error.js'; @@ -609,12 +610,21 @@ function getLocalUploadedFilePath(photoUrl?: string | null) { return undefined; } - const uploadPublicRoot = normalizePublicUrl(env.UPLOAD_PUBLIC_BASE_URL, env.UPLOAD_PUBLIC_PATH); - const fileName = photoUrl.startsWith(`${uploadPublicRoot}/`) - ? photoUrl.slice(uploadPublicRoot.length + 1) - : undefined; + // Only the trailing path segment matters: it is validated against the same fileId + // pattern the uploads endpoint enforces, so this works for both the current + // `/v1/uploads/` URLs and any legacy `/uploads/` URLs already stored + // in the database, without trusting anything else in the string. + let pathname: string; - if (!fileName || fileName.includes('/') || fileName.includes('\\')) { + try { + pathname = new URL(photoUrl).pathname; + } catch { + pathname = photoUrl; + } + + const fileName = pathname.split('/').pop(); + + if (!fileName || !UPLOAD_FILE_ID_PATTERN.test(fileName)) { return undefined; } @@ -628,7 +638,9 @@ async function deleteLocalUploadedFile(photoUrl?: string | null) { return; } - await fs.unlink(filePath).catch(() => undefined); + await fs.unlink(filePath).catch((error) => { + console.warn(`Failed to delete uploaded file at ${filePath}`, error); + }); } function asString(value: unknown) { @@ -2205,15 +2217,24 @@ export const soundlogService = { }, idempotencyKey?: string, ) { - return withIdempotency( - { idempotencyKey, scope: 'moment-log.create', userId }, - async () => { - const location = - input.lat !== undefined && input.lng !== undefined - ? { lat: input.lat, lng: input.lng } - : undefined; + // multer already wrote the uploaded file to disk before this runs. If the request + // turns out to be an idempotent duplicate (withIdempotency returns a cached response + // without invoking the action below) or if the action throws before a MomentLog row + // is committed, the just-written file is never referenced by any row and would be + // orphaned on disk. `persisted` tracks whether this call actually attached the file + // to a saved row so the `finally` block can clean it up in every other case. + let persisted = false; - assertPublicRecapHasLocation(input.visibility, location); + try { + return await withIdempotency( + { idempotencyKey, scope: 'moment-log.create', userId }, + async () => { + const location = + input.lat !== undefined && input.lng !== undefined + ? { lat: input.lat, lng: input.lng } + : undefined; + + assertPublicRecapHasLocation(input.visibility, location); const track = input.trackId ? await prisma.track.findUnique({ where: { id: input.trackId } }) @@ -2264,9 +2285,22 @@ export const soundlogService = { return created; }); + persisted = true; + return momentLogToDto(log); - }, - ); + }, + ); + } finally { + // Cached idempotent replay (action above never ran) or a thrown error before the + // row was committed both leave `persisted` false — in either case the file multer + // just wrote is orphaned and should not linger on disk. Deletion failures are only + // logged (see deleteLocalUploadedFile) and never override the real response/error. + if (!persisted && input.photoPath) { + await deleteLocalUploadedFile( + normalizePublicUrl(env.UPLOAD_PUBLIC_BASE_URL, input.photoPath), + ); + } + } }, async updateMomentLog( @@ -2389,36 +2423,52 @@ export const soundlogService = { async updateMomentLogPhoto(userId: string, momentLogId: string, photoPath: string) { const nextPhotoUrl = normalizePublicUrl(env.UPLOAD_PUBLIC_BASE_URL, photoPath); - const existing = await prisma.momentLog.findFirst({ - where: { - id: momentLogId, - userId, - }, - }); - - if (!existing) { - await deleteLocalUploadedFile(nextPhotoUrl); - throw notFound(ERROR_MESSAGES.MOMENT_LOG_NOT_FOUND); - } + // multer already wrote the new photo to disk before this runs. `persisted` tracks + // whether it actually got attached to a saved row (target not found, or the DB + // transaction throwing, both leave it false) so the `finally` block below can clean up + // the newly uploaded file in every case that isn't a successful replace — mirroring the + // same orphan-file guard used in createMomentLog. + let persisted = false; - const updated = await prisma.$transaction(async (transaction) => { - const nextMoment = await transaction.momentLog.update({ - where: { id: existing.id }, - data: { photoUrl: nextPhotoUrl }, + try { + const existing = await prisma.momentLog.findFirst({ + where: { + id: momentLogId, + userId, + }, }); - await refreshRecapAggregates(transaction, { - momentIds: [existing.id], - sessionIds: [existing.sessionId], - userId, + if (!existing) { + throw notFound(ERROR_MESSAGES.MOMENT_LOG_NOT_FOUND); + } + + const updated = await prisma.$transaction(async (transaction) => { + const nextMoment = await transaction.momentLog.update({ + where: { id: existing.id }, + data: { photoUrl: nextPhotoUrl }, + }); + + await refreshRecapAggregates(transaction, { + momentIds: [existing.id], + sessionIds: [existing.sessionId], + userId, + }); + + return nextMoment; }); - return nextMoment; - }); + persisted = true; - await deleteLocalUploadedFile(existing.photoUrl); + // Only delete the previous photo after the new one is safely committed, so a crash + // or failure between these two steps never leaves the moment without any photo file. + await deleteLocalUploadedFile(existing.photoUrl); - return momentLogToDto(updated); + return momentLogToDto(updated); + } finally { + if (!persisted) { + await deleteLocalUploadedFile(nextPhotoUrl); + } + } }, async deleteMomentLogPhoto(userId: string, momentLogId: string) { diff --git a/src/services/upload-file.service.ts b/src/services/upload-file.service.ts new file mode 100644 index 0000000..3d4aeb5 --- /dev/null +++ b/src/services/upload-file.service.ts @@ -0,0 +1,161 @@ +import fs from 'node:fs/promises'; +import path from 'node:path'; + +import { env } from '../config/env.js'; +import { prisma } from '../config/prisma.js'; +import { UPLOAD_FILE_ID_PATTERN } from '../middlewares/upload.middleware.js'; + +const uploadRoot = path.resolve(env.UPLOAD_DIRECTORY); + +export type ResolvedUploadedFile = { + absolutePath: string; +}; + +// multer's disk storage writes uploaded files under a random 32-hex-character name with no +// extension, so `res.sendFile()` has nothing to infer a Content-Type from and Express falls +// back to `application/octet-stream`. Combined with helmet's `X-Content-Type-Options: nosniff` +// (which tells browsers not to sniff the body themselves), that means images would never +// render on the web. The fix is to determine the real image type ourselves from the file's +// leading bytes — never from the client-supplied upload MIME type or a filename extension, +// both of which are trivially spoofable — and set Content-Type explicitly before serving. +const IMAGE_HEADER_SNIFF_BYTES = 12; + +type ImageMagicByteSignature = { + contentType: string; + matches: (header: Buffer) => boolean; +}; + +const IMAGE_MAGIC_BYTE_SIGNATURES: ImageMagicByteSignature[] = [ + { + contentType: 'image/jpeg', + matches: (header) => + header.length >= 3 && header[0] === 0xff && header[1] === 0xd8 && header[2] === 0xff, + }, + { + contentType: 'image/png', + matches: (header) => + header.length >= 8 && + header[0] === 0x89 && + header[1] === 0x50 && + header[2] === 0x4e && + header[3] === 0x47 && + header[4] === 0x0d && + header[5] === 0x0a && + header[6] === 0x1a && + header[7] === 0x0a, + }, + { + contentType: 'image/gif', + matches: (header) => + header.length >= 6 && + header.subarray(0, 3).toString('ascii') === 'GIF' && + ['87a', '89a'].includes(header.subarray(3, 6).toString('ascii')), + }, + { + contentType: 'image/webp', + matches: (header) => + header.length >= 12 && + header.subarray(0, 4).toString('ascii') === 'RIFF' && + header.subarray(8, 12).toString('ascii') === 'WEBP', + }, + { + // HEIC/HEIF files are ISO base media (MP4-family) containers: bytes 4-8 are the literal + // string "ftyp" and bytes 8-12 are a 4-character "brand" identifying the specific format. + contentType: 'image/heic', + matches: (header) => { + if (header.length < 12 || header.subarray(4, 8).toString('ascii') !== 'ftyp') { + return false; + } + + const brand = header.subarray(8, 12).toString('ascii'); + + return ['heic', 'heim', 'heis', 'heix', 'hevc', 'hevx', 'mif1', 'msf1'].includes(brand); + }, + }, +]; + +/** + * Sniffs the first bytes of a file already resolved via `resolveUploadedFileForUser` and + * returns the matching image Content-Type, or `undefined` if the bytes don't match any + * known image signature. Callers must not serve the file as an image (and should not fall + * back to a client-supplied or extension-derived type) when this returns `undefined`. + */ +async function detectImageContentType(absolutePath: string): Promise { + const fileHandle = await fs.open(absolutePath, 'r'); + + try { + const header = Buffer.alloc(IMAGE_HEADER_SNIFF_BYTES); + const { bytesRead } = await fileHandle.read(header, 0, IMAGE_HEADER_SNIFF_BYTES, 0); + const signatureHeader = header.subarray(0, bytesRead); + + return IMAGE_MAGIC_BYTE_SIGNATURES.find((signature) => signature.matches(signatureHeader)) + ?.contentType; + } finally { + await fileHandle.close(); + } +} + +/** + * Resolves a client-supplied `fileId` (the last path segment of a stored photoUrl) to an + * absolute file path on disk, but only if the requesting user is allowed to see it. + * + * Returns `undefined` for every failure case (invalid id, unknown file, missing file on + * disk, or an access check that fails) so callers can respond with an indistinguishable + * 404 regardless of whether the file exists — this avoids leaking the existence of + * private resources. + */ +async function resolveUploadedFileForUser( + userId: string, + fileId: string, +): Promise { + // Reject anything that is not exactly a 32-character hex string up front. This blocks + // path traversal (`../`), absolute paths, URL-encoded separators (`%2e%2e%2f`), null + // bytes, and any other shape before it ever touches the filesystem or the database. + if (!UPLOAD_FILE_ID_PATTERN.test(fileId)) { + return undefined; + } + + // The uploaded file's owning MomentLog is looked up by matching the stored photoUrl's + // trailing `/` segment. The DB — not the client — is the source of truth for + // which filename exists and who owns it. + const momentLog = await prisma.momentLog.findFirst({ + where: { photoUrl: { endsWith: `/${fileId}` } }, + select: { userId: true, visibility: true }, + }); + + if (!momentLog) { + return undefined; + } + + const isOwner = momentLog.userId === userId; + const isPublic = momentLog.visibility === 'public'; + + if (!isOwner && !isPublic) { + return undefined; + } + + // Build the path from the server-known upload root and the validated fileId only, then + // re-verify (defense in depth) that the resolved path is still inside the upload root. + const absolutePath = path.resolve(uploadRoot, fileId); + + if (absolutePath !== path.join(uploadRoot, fileId) || !absolutePath.startsWith(`${uploadRoot}${path.sep}`)) { + return undefined; + } + + try { + const stat = await fs.stat(absolutePath); + + if (!stat.isFile()) { + return undefined; + } + } catch { + return undefined; + } + + return { absolutePath }; +} + +export const uploadFileService = { + detectImageContentType, + resolveUploadedFileForUser, +}; diff --git a/tests/api.test.ts b/tests/api.test.ts index 166ae72..d439697 100644 --- a/tests/api.test.ts +++ b/tests/api.test.ts @@ -1,13 +1,31 @@ import bcrypt from 'bcrypt'; +import fs from 'node:fs/promises'; +import path from 'node:path'; import request from 'supertest'; import { beforeAll, describe, expect, it, vi } from 'vitest'; import { createApp } from '../src/app.js'; +import { env } from '../src/config/env.js'; import { prisma } from '../src/config/prisma.js'; import { mockDb, resetMockDb } from '../src/mock/mock-db.js'; import { reverseGeocodeLocation } from '../src/services/reverse-geocoding.service.js'; import { disconnectSeedDatabase, seedDatabase } from '../prisma/seed.js'; +function fileIdFromPhotoUrl(photoUrl: string) { + return new URL(photoUrl).pathname.split('/').pop() as string; +} + +// Real JPEG SOI + APP0/JFIF magic bytes. The server now determines Content-Type for +// GET /v1/uploads/:fileId purely from the file's leading bytes (see +// upload-file.service.ts#detectImageContentType) rather than trusting the client-supplied +// upload MIME type, so test fixtures need genuine image bytes for "serves the file back +// out" assertions to mean anything. +const JPEG_MAGIC_BYTES = Buffer.from([0xff, 0xd8, 0xff, 0xe0, 0x00, 0x10, 0x4a, 0x46, 0x49, 0x46]); + +function fakeJpegBuffer(label: string) { + return Buffer.concat([JPEG_MAGIC_BYTES, Buffer.from(label)]); +} + const app = createApp(); const useMockDb = process.env.USE_MOCK_DB === 'true'; @@ -116,7 +134,7 @@ async function createTestMomentLog(input: { } const response = await requestBuilder - .attach('photo', Buffer.from('fake-image'), { + .attach('photo', fakeJpegBuffer('fake-image'), { filename: input.filename, contentType: 'image/jpeg', }); @@ -173,7 +191,7 @@ describe('Soundlog API', () => { expect(v1Docs.headers.location).toBe('/docs'); }); - it('creates a DB test record without auth', async () => { + it('rejects the dev DB test route without auth', async () => { const response = await request(app) .post('/v1/dev/db-test-records') .send({ @@ -183,6 +201,21 @@ describe('Soundlog API', () => { }, }); + expect(response.status).toBe(401); + expect(response.body.error.code).toBe('UNAUTHORIZED'); + }); + + it('creates a DB test record with auth', async () => { + const response = await request(app) + .post('/v1/dev/db-test-records') + .set('Authorization', authHeader) + .send({ + label: 'swagger-smoke-test', + payload: { + source: 'api-test', + }, + }); + expect(response.status).toBe(201); expect(response.body.data.id).toEqual(expect.any(String)); expect(response.body.data.label).toBe('swagger-smoke-test'); @@ -675,7 +708,7 @@ describe('Soundlog API', () => { .field('note', '카페 거리에서 남긴 테스트 메모') .field('placeName', '테스트 장소') .field('trackId', 'seoul-city') - .attach('photo', Buffer.from('fake-image'), { + .attach('photo', fakeJpegBuffer('fake-image'), { filename: 'moment.jpg', contentType: 'image/jpeg', }); @@ -693,7 +726,7 @@ describe('Soundlog API', () => { .field('note', '중복 요청 메모는 반영되지 않아야 함') .field('placeName', '중복 요청 장소') .field('trackId', 'seoul-city') - .attach('photo', Buffer.from('fake-image'), { + .attach('photo', fakeJpegBuffer('fake-image'), { filename: 'moment-duplicate.jpg', contentType: 'image/jpeg', }); @@ -735,7 +768,7 @@ describe('Soundlog API', () => { .field('note', '새 리캡 캡처 경로 테스트') .field('placeName', '리캡 캡처 테스트 장소') .field('trackId', 'seoul-city') - .attach('photo', Buffer.from('alias-image'), { + .attach('photo', fakeJpegBuffer('alias-image'), { filename: 'recap-capture.jpg', contentType: 'image/jpeg', }); @@ -783,7 +816,7 @@ describe('Soundlog API', () => { const photoUpdated = await request(app) .put(`/v1/moment-logs/${created.body.data.id}/photo`) .set('Authorization', authHeader) - .attach('photo', Buffer.from('replacement-image'), { + .attach('photo', fakeJpegBuffer('replacement-image'), { filename: 'moment-replacement.jpg', contentType: 'image/jpeg', }); @@ -806,7 +839,7 @@ describe('Soundlog API', () => { const aliasPhotoUpdated = await request(app) .put(`/v1/recap-captures/${aliasCreated.body.data.id}/photo`) .set('Authorization', authHeader) - .attach('photo', Buffer.from('alias-replacement-image'), { + .attach('photo', fakeJpegBuffer('alias-replacement-image'), { filename: 'recap-capture-replacement.jpg', contentType: 'image/jpeg', }); @@ -1259,17 +1292,22 @@ describe('Soundlog API', () => { trackTitle: '한강에서', }); expect(JSON.stringify(mixedVisibilityMarker)).not.toContain('비공개 장소 이름'); + expect( + publicMarkersAfterUpdate.body.data.some( + (marker: { recapId: string }) => marker.recapId === farCreated.body.data.id, + ), + ).toBe(false); - const fixedRadiusMarkers = await request(app) + const expandedRadiusMarkers = await request(app) .get('/v1/recap-markers') .query({ lat: 37.5512, lng: 126.9882, radiusMeters: 5000, scope: 'public' }) .set('Authorization', authHeader); - expect(fixedRadiusMarkers.status).toBe(200); + expect(expandedRadiusMarkers.status).toBe(200); expect( - fixedRadiusMarkers.body.data.some( + expandedRadiusMarkers.body.data.some( (marker: { recapId: string }) => marker.recapId === farCreated.body.data.id, ), - ).toBe(false); + ).toBe(true); const duplicate = await request(app) .post('/v1/recaps') @@ -1860,6 +1898,375 @@ describe('Soundlog API', () => { expect(response.body.data.topTracks.length).toBeGreaterThan(0); }); + (useMockDb ? describe.skip : describe)('GET /v1/uploads/:fileId access control', () => { + it('lets the owner fetch their own private moment photo', async () => { + const moment = await createTestMomentLog({ + authHeader, + filename: 'owner-private.jpg', + placeName: '업로드 접근 테스트', + }); + const fileId = fileIdFromPhotoUrl(moment.photoUrl); + + const response = await request(app) + .get(`/v1/uploads/${fileId}`) + .set('Authorization', authHeader); + + expect(response.status).toBe(200); + }); + + it('returns 404 (not 403) when another user requests a private photo', async () => { + const moment = await createTestMomentLog({ + authHeader, + filename: 'owner-private-other.jpg', + placeName: '업로드 접근 테스트', + }); + const fileId = fileIdFromPhotoUrl(moment.photoUrl); + const otherAccessToken = await getToken(); + + const response = await request(app) + .get(`/v1/uploads/${fileId}`) + .set('Authorization', `Bearer ${otherAccessToken}`); + + expect(response.status).toBe(404); + }); + + it('rejects an unauthenticated request for a private photo', async () => { + const moment = await createTestMomentLog({ + authHeader, + filename: 'owner-private-anon.jpg', + placeName: '업로드 접근 테스트', + }); + const fileId = fileIdFromPhotoUrl(moment.photoUrl); + + const response = await request(app).get(`/v1/uploads/${fileId}`); + + expect([401, 404]).toContain(response.status); + }); + + it('lets another authenticated user fetch a public moment photo', async () => { + const moment = await createTestMomentLog({ + authHeader, + filename: 'owner-public.jpg', + lat: 37.5665, + lng: 126.978, + placeName: '업로드 접근 테스트(공개)', + visibility: 'public', + }); + const fileId = fileIdFromPhotoUrl(moment.photoUrl); + const otherAccessToken = await getToken(); + + const response = await request(app) + .get(`/v1/uploads/${fileId}`) + .set('Authorization', `Bearer ${otherAccessToken}`); + + expect(response.status).toBe(200); + }); + + it('returns 404 for a well-formed file id that does not exist', async () => { + const response = await request(app) + .get(`/v1/uploads/${'a'.repeat(32)}`) + .set('Authorization', authHeader); + + expect(response.status).toBe(404); + }); + + it('blocks path traversal, absolute paths, and encoded separators in the file id', async () => { + const maliciousIds = [ + '../../../etc/passwd', + '..%2f..%2f..%2fetc%2fpasswd', + '%2e%2e%2f%2e%2e%2fsrc%2fapp.ts', + encodeURIComponent('../../../etc/passwd'), + encodeURIComponent('/etc/passwd'), + // Double URL-encoding: decodes once (by Express) to a still-encoded traversal + // sequence, which must still fail the anchored hex pattern rather than being + // decoded a second time and slipping through. + '%252e%252e%252fetc%252fpasswd', + encodeURIComponent(encodeURIComponent('../../../etc/passwd')), + // Backslash variants (meaningful as a path separator on Windows filesystems). + '..\\..\\..\\etc\\passwd', + encodeURIComponent('..\\..\\..\\etc\\passwd'), + '%5c..%5c..%5cetc%5cpasswd', + // A well-formed 32-hex id with an extra path segment before or after it. + `${'a'.repeat(32)}/../../../etc/passwd`, + `some-prefix/${'a'.repeat(32)}`, + `${'a'.repeat(32)}/extra-suffix`, + encodeURIComponent('a'.repeat(32) + ''), + ]; + + for (const maliciousId of maliciousIds) { + const response = await request(app) + .get(`/v1/uploads/${maliciousId}`) + .set('Authorization', authHeader); + + expect(response.status).not.toBe(200); + expect(response.text ?? '').not.toContain('root:'); + } + }); + + it('rejects traversal attempts hidden behind a query string', async () => { + const response = await request(app) + .get('/v1/uploads/..%2f..%2f..%2fetc%2fpasswd?x=1') + .set('Authorization', authHeader); + + expect(response.status).not.toBe(200); + expect(response.text ?? '').not.toContain('root:'); + }); + + it('still resolves a valid file id when a harmless query string is appended', async () => { + const moment = await createTestMomentLog({ + authHeader, + filename: 'owner-with-query.jpg', + placeName: '쿼리스트링 테스트', + }); + const fileId = fileIdFromPhotoUrl(moment.photoUrl); + + const response = await request(app) + .get(`/v1/uploads/${fileId}?cachebust=1`) + .set('Authorization', authHeader); + + expect(response.status).toBe(200); + }); + + (useMockDb ? it.skip : it)( + 'returns 404 for a real on-disk file that has no matching DB record', + async () => { + const uploadDir = path.resolve(env.UPLOAD_DIRECTORY); + const orphanFileId = 'f'.repeat(32); + const orphanFilePath = path.join(uploadDir, orphanFileId); + + await fs.writeFile(orphanFilePath, fakeJpegBuffer('untracked-file')); + + try { + const response = await request(app) + .get(`/v1/uploads/${orphanFileId}`) + .set('Authorization', authHeader); + + expect(response.status).toBe(404); + } finally { + await fs.unlink(orphanFilePath).catch(() => undefined); + } + }, + ); + + it('sets an image Content-Type derived from the file bytes and keeps nosniff enabled', async () => { + const moment = await createTestMomentLog({ + authHeader, + filename: 'owner-content-type.jpg', + placeName: '콘텐츠 타입 테스트', + }); + const fileId = fileIdFromPhotoUrl(moment.photoUrl); + + const response = await request(app) + .get(`/v1/uploads/${fileId}`) + .set('Authorization', authHeader); + + expect(response.status).toBe(200); + expect(response.headers['content-type']).toMatch(/^image\/jpeg/); + expect(response.headers['x-content-type-options']).toBe('nosniff'); + }); + + (useMockDb ? it.skip : it)( + 'never serves a stored file as an image when its on-disk bytes are not a recognized image signature', + async () => { + const uploadDir = path.resolve(env.UPLOAD_DIRECTORY); + + const moment = await createTestMomentLog({ + authHeader, + filename: 'will-be-corrupted.jpg', + placeName: '비이미지 바이트 테스트', + }); + const fileId = fileIdFromPhotoUrl(moment.photoUrl); + const filePath = path.join(uploadDir, fileId); + + // Overwrites the on-disk bytes with non-image content while keeping the same + // filename/DB row, so the fileId is legitimately owned but the bytes are not an + // image. This confirms the GET endpoint's own magic-byte check — not the + // upload-time filter — is the real boundary for what gets served as an image. + await fs.writeFile(filePath, Buffer.from('not an image at all')); + + const response = await request(app) + .get(`/v1/uploads/${fileId}`) + .set('Authorization', authHeader); + + expect(response.status).toBe(404); + }, + ); + }); + + describe('upload MIME whitelist', () => { + it('rejects a disallowed declared MIME type without writing a file to disk', async () => { + const uploadDir = path.resolve(env.UPLOAD_DIRECTORY); + const filesBefore = useMockDb ? undefined : new Set(await fs.readdir(uploadDir)); + + const response = await request(app) + .post('/v1/moment-logs') + .set('Authorization', authHeader) + .field('createdAt', new Date().toISOString()) + .field('moodTags', 'fresh') + .field('placeName', 'MIME 화이트리스트 테스트') + .attach('photo', Buffer.from('#!/bin/sh\necho not an image\n'), { + contentType: 'application/x-sh', + filename: 'not-an-image.sh', + }); + + expect(response.status).toBe(400); + expect(response.body.error.code).toBe('BAD_REQUEST'); + + if (!useMockDb) { + const filesAfter = new Set(await fs.readdir(uploadDir)); + expect(filesAfter.size).toBe(filesBefore!.size); + } + }); + + it('accepts every image type on the allowed MIME whitelist', async () => { + const allowedMimeTypes = [ + 'image/jpeg', + 'image/png', + 'image/webp', + 'image/heic', + 'image/heif', + 'image/gif', + ]; + + for (const mimeType of allowedMimeTypes) { + const response = await request(app) + .post('/v1/moment-logs') + .set('Authorization', authHeader) + .field('createdAt', new Date().toISOString()) + .field('moodTags', 'fresh') + .field('placeName', `MIME 허용 테스트 ${mimeType}`) + .attach('photo', fakeJpegBuffer(mimeType), { + contentType: mimeType, + filename: `allowed.${mimeType.split('/')[1]}`, + }); + + expect(response.status).toBe(201); + } + }); + }); + + (useMockDb ? describe.skip : describe)('orphaned upload cleanup on failure', () => { + it('deletes the newly uploaded file when creating the MomentLog row fails', async () => { + const uploadDir = path.resolve(env.UPLOAD_DIRECTORY); + const filesBefore = new Set(await fs.readdir(uploadDir)); + + // createMomentLog writes the row inside `prisma.$transaction(async (transaction) => + // ...)`. The `transaction` client Prisma hands to that callback is a distinct proxy + // per call, so spying on `prisma.momentLog.create` would never intercept it — + // `$transaction` itself is the right interception point to simulate a failure deep + // inside the write. + const transactionSpy = vi + .spyOn(prisma, '$transaction') + .mockRejectedValueOnce(new Error('simulated DB failure')); + + try { + const response = await request(app) + .post('/v1/moment-logs') + .set('Authorization', authHeader) + .field('createdAt', new Date().toISOString()) + .field('moodTags', 'fresh') + .field('placeName', 'DB 실패 정리 테스트') + .attach('photo', fakeJpegBuffer('db-failure'), { + contentType: 'image/jpeg', + filename: 'db-failure.jpg', + }); + + expect(response.status).toBe(500); + } finally { + transactionSpy.mockRestore(); + } + + const filesAfter = new Set(await fs.readdir(uploadDir)); + expect(filesAfter.size).toBe(filesBefore.size); + }); + + it('deletes the newly uploaded replacement photo (keeping the original) when the photo-update transaction fails', async () => { + const moment = await createTestMomentLog({ + authHeader, + filename: 'photo-update-db-failure.jpg', + placeName: 'DB 실패 시 교체 사진 정리 테스트', + }); + const originalFileId = fileIdFromPhotoUrl(moment.photoUrl); + + const uploadDir = path.resolve(env.UPLOAD_DIRECTORY); + const filesBefore = new Set(await fs.readdir(uploadDir)); + + const transactionSpy = vi + .spyOn(prisma, '$transaction') + .mockRejectedValueOnce(new Error('simulated DB failure')); + + try { + const response = await request(app) + .put(`/v1/moment-logs/${moment.id}/photo`) + .set('Authorization', authHeader) + .attach('photo', fakeJpegBuffer('replacement-db-failure'), { + contentType: 'image/jpeg', + filename: 'replacement-db-failure.jpg', + }); + + expect(response.status).toBe(500); + } finally { + transactionSpy.mockRestore(); + } + + // The newly uploaded replacement file must not linger on disk... + const filesAfter = new Set(await fs.readdir(uploadDir)); + expect(filesAfter.size).toBe(filesBefore.size); + + // ...and the original photo must still be intact and fetchable. + const originalStillServed = await request(app) + .get(`/v1/uploads/${originalFileId}`) + .set('Authorization', authHeader); + expect(originalStillServed.status).toBe(200); + }); + }); + + (useMockDb ? it.skip : it)( + 'deletes the orphaned upload file when an idempotent duplicate request is skipped', + async () => { + const uploadDir = path.resolve(env.UPLOAD_DIRECTORY); + const idempotencyKey = `orphan-cleanup-${Date.now()}`; + + const filesBefore = new Set(await fs.readdir(uploadDir)); + + const created = await request(app) + .post('/v1/moment-logs') + .set('Authorization', authHeader) + .set('Idempotency-Key', idempotencyKey) + .field('createdAt', new Date().toISOString()) + .field('moodTags', 'fresh') + .field('placeName', '고아 파일 정리 테스트') + .attach('photo', fakeJpegBuffer('fake-image-1'), { + contentType: 'image/jpeg', + filename: 'orphan-first.jpg', + }); + expect(created.status).toBe(201); + + const filesAfterFirst = new Set(await fs.readdir(uploadDir)); + expect(filesAfterFirst.size).toBe(filesBefore.size + 1); + + const duplicate = await request(app) + .post('/v1/moment-logs') + .set('Authorization', authHeader) + .set('Idempotency-Key', idempotencyKey) + .field('createdAt', new Date().toISOString()) + .field('moodTags', 'fresh') + .field('placeName', '고아 파일 정리 테스트(중복)') + .attach('photo', fakeJpegBuffer('fake-image-2'), { + contentType: 'image/jpeg', + filename: 'orphan-duplicate.jpg', + }); + expect(duplicate.status).toBe(201); + expect(duplicate.body.data.id).toBe(created.body.data.id); + + // The duplicate request's upload must not be left behind on disk: the idempotency + // short-circuit skips the DB write, so the file it wrote is never referenced by any + // MomentLog row and should have been cleaned up. + const filesAfterDuplicate = new Set(await fs.readdir(uploadDir)); + expect(filesAfterDuplicate.size).toBe(filesAfterFirst.size); + }, + ); + afterAll(async () => { if (useMockDb) { resetMockDb(); diff --git a/tests/security-hardening.test.ts b/tests/security-hardening.test.ts new file mode 100644 index 0000000..f6baf39 --- /dev/null +++ b/tests/security-hardening.test.ts @@ -0,0 +1,138 @@ +import request from 'supertest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +/** + * These tests each need a distinct process.env / module graph (different + * NODE_ENV or rate-limit config), so every test resets the module cache and + * re-imports src/app.js fresh instead of sharing the app instance used by + * tests/api.test.ts. + */ + +// Only snapshot/restore the specific keys these tests mutate. DATABASE_URL +// and JWT_SECRET (loaded once via `dotenv/config`) must be left untouched: +// once cleared, a re-import cannot reliably reload them (dotenv's own +// module-level state isn't reset by vi.resetModules()). +const MUTATED_KEYS = [ + 'NODE_ENV', + 'AUTH_RATE_LIMIT_ENABLED', + 'AUTH_RATE_LIMIT_MAX', + 'AUTH_RATE_LIMIT_WINDOW_MS', + 'AUTH_RATE_LIMIT_IP_MAX', + 'AUTH_RATE_LIMIT_IP_WINDOW_MS', +] as const; +const originalEnv = Object.fromEntries( + MUTATED_KEYS.map((key) => [key, process.env[key]]), +); + +function restoreEnv() { + for (const key of MUTATED_KEYS) { + const value = originalEnv[key]; + + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } +} + +async function freshApp() { + vi.resetModules(); + const { createApp } = await import('../src/app.js'); + return createApp(); +} + +describe('production hardening: dev DB test route', () => { + afterEach(() => { + restoreEnv(); + }); + + it('does not register the dev DB test route in production', async () => { + process.env.NODE_ENV = 'production'; + + const app = await freshApp(); + const response = await request(app).post('/v1/dev/db-test-records').send({}); + + expect(response.status).toBe(404); + expect(response.body.error.code).toBe('NOT_FOUND'); + }); + + it('requires auth for the dev DB test route outside production', async () => { + process.env.NODE_ENV = 'test'; + + const app = await freshApp(); + const response = await request(app) + .post('/v1/dev/db-test-records') + .send({ label: 'no-auth' }); + + expect(response.status).toBe(401); + expect(response.body.error.code).toBe('UNAUTHORIZED'); + }); +}); + +describe('auth rate limiting', () => { + afterEach(() => { + restoreEnv(); + }); + + it('returns 429 once the configured auth rate limit is exceeded', async () => { + process.env.NODE_ENV = 'test'; + process.env.AUTH_RATE_LIMIT_ENABLED = 'true'; + process.env.AUTH_RATE_LIMIT_MAX = '2'; + process.env.AUTH_RATE_LIMIT_WINDOW_MS = '60000'; + + const app = await freshApp(); + const credentials = { email: 'rate-limit-test@soundlog.test', password: 'wrong-password' }; + + const first = await request(app).post('/v1/auth/login').send(credentials); + const second = await request(app).post('/v1/auth/login').send(credentials); + const third = await request(app).post('/v1/auth/login').send(credentials); + + expect(first.status).not.toBe(429); + expect(second.status).not.toBe(429); + expect(third.status).toBe(429); + expect(third.body.error.code).toBe('TOO_MANY_REQUESTS'); + }); + + it('returns 429 for credential stuffing across many emails from one IP', async () => { + process.env.NODE_ENV = 'test'; + process.env.AUTH_RATE_LIMIT_ENABLED = 'true'; + // Account limit set high so it never trips here — each request below uses + // a different email, so only the per-IP limiter can be what catches this. + process.env.AUTH_RATE_LIMIT_MAX = '1000'; + process.env.AUTH_RATE_LIMIT_WINDOW_MS = '60000'; + process.env.AUTH_RATE_LIMIT_IP_MAX = '2'; + process.env.AUTH_RATE_LIMIT_IP_WINDOW_MS = '60000'; + + const app = await freshApp(); + + const first = await request(app) + .post('/v1/auth/login') + .send({ email: 'stuffing-1@soundlog.test', password: 'wrong-password' }); + const second = await request(app) + .post('/v1/auth/login') + .send({ email: 'stuffing-2@soundlog.test', password: 'wrong-password' }); + const third = await request(app) + .post('/v1/auth/login') + .send({ email: 'stuffing-3@soundlog.test', password: 'wrong-password' }); + + expect(first.status).not.toBe(429); + expect(second.status).not.toBe(429); + expect(third.status).toBe(429); + expect(third.body.error.code).toBe('TOO_MANY_REQUESTS'); + }); + + it('does not rate limit auth endpoints under the default test configuration', async () => { + process.env.NODE_ENV = 'test'; + delete process.env.AUTH_RATE_LIMIT_ENABLED; + + const app = await freshApp(); + const credentials = { email: 'no-rate-limit-test@soundlog.test', password: 'wrong-password' }; + + for (let i = 0; i < 5; i += 1) { + // eslint-disable-next-line no-await-in-loop + const response = await request(app).post('/v1/auth/login').send(credentials); + expect(response.status).not.toBe(429); + } + }); +});