From 727207d9497cb2740d2773def9f45c282fc42ca2 Mon Sep 17 00:00:00 2001 From: Siavash Sameni Date: Sun, 7 Jun 2026 21:46:14 +0400 Subject: [PATCH] =?UTF-8?q?docs:=20sync=20from=20backend=20c0e80a7=20?= =?UTF-8?q?=E2=80=94=20security=20audit=20closeout?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- 09 - Audits/Activity Log.md | 10 ++++ ...DB Performance Logic Audit - 2026-06-07.md | 53 ++++++++++++------- 2 files changed, 44 insertions(+), 19 deletions(-) diff --git a/09 - Audits/Activity Log.md b/09 - Audits/Activity Log.md index 18e9475..ee88491 100644 --- a/09 - Audits/Activity Log.md +++ b/09 - Audits/Activity Log.md @@ -12,6 +12,16 @@ entries on top. Maintained by agents per the rule in `../AGENTS.md`. --- +### 2026-06-07 — backend@c0e80a7, frontend@38ff0db — security DB/performance audit closeout + +**Commits:** `c0e80a7` `38ff0db` +**Touched:** backend `src/shared/utils/identity.ts`, `src/shared/utils/pagination.ts`, dispute controllers/services/routes, delivery/file/template/payment/chat/points/user routes, `src/app.ts`, `src/services/auth/googleOAuthService.ts`, `__tests__/security-db-performance-logic-audit.test.ts`, `package.json`, `package-lock.json`; frontend `src/auth/services/google-oauth.ts`, `src/lib/axios.ts`, `src/utils/logger.ts`, `package.json`; docs `09 - Audits/Security DB Performance Logic Audit - 2026-06-07.md`, `09 - Audits/Activity Log.md` +**Why:** Close all 10 findings from the fresh security, DB performance, and logic audit. The changes add canonical identity checks for dispute paths, remove delivery-code/token log leakage, confine generic file paths, cap template batches, make broad user listing admin-only, block private upload directories from static serving, and normalize audited pagination inputs. +**Verification:** `task-master next` (no tasks available); backend `npx jest __tests__/security-db-performance-logic-audit.test.ts --runInBand` (7 tests); backend `npm run typecheck`; backend focused `npx eslint ...` (0 errors, existing warnings only); frontend focused `npx eslint src/auth/services/google-oauth.ts src/lib/axios.ts src/utils/logger.ts`; source grep checks for delivery-code/token log patterns and audited ad-hoc pagination patterns returned no matches; backend/frontend `git diff --cached --check`. Frontend full `npx tsc --noEmit --ignoreDeprecations 6.0` remains blocked by pre-existing dirty E2E test files outside this audit change. +**Linked docs updated:** [[09 - Audits/Security DB Performance Logic Audit - 2026-06-07]] + +--- + ### 2026-06-07 — backend@dedc5fe, frontend@9a5fa13 — DB audit remaining M/L closeout **Commits:** `dedc5fe` `9a5fa13` diff --git a/09 - Audits/Security DB Performance Logic Audit - 2026-06-07.md b/09 - Audits/Security DB Performance Logic Audit - 2026-06-07.md index e75cf05..979fa93 100644 --- a/09 - Audits/Security DB Performance Logic Audit - 2026-06-07.md +++ b/09 - Audits/Security DB Performance Logic Audit - 2026-06-07.md @@ -3,7 +3,7 @@ title: Security, DB Performance, and Logic Audit - 2026-06-07 tags: [audit, security, database, performance, logic, findings] created: 2026-06-07 updated: 2026-06-07 -status: open +status: closed --- # Security, DB Performance, and Logic Audit - 2026-06-07 @@ -23,15 +23,30 @@ Fresh post-remediation audit track after the DB Query & Schema Audit was closed. - Medium: broken route behavior, false-deny authorization, latent risk, degraded performance, or cleanup needed before the next refactor. - Low: hardening and maintainability issues with limited user impact. -## Initial Summary +## Remediation Summary -| Severity | Count | -|---|---:| -| Critical | 0 | -| High | 6 | -| Medium | 4 | -| Low | 0 | -| Total | 10 | +| Severity | Open | Fixed | +|---|---:|---:| +| Critical | 0 | 0 | +| High | 0 | 6 | +| Medium | 0 | 4 | +| Low | 0 | 0 | +| Total | 0 | 10 | + +All initial findings were remediated in backend `c0e80a7` and frontend `38ff0db`. + +| Finding | Status | Fixed in | Notes | +|---|---|---|---| +| H1 | Fixed | backend `c0e80a7` | Dashboard dispute creation now checks canonical purchase-request ownership before creating dispute/chat state. | +| H2 | Fixed | backend `c0e80a7` | Delivery-code generation and verification logs no longer print code values or expected values. | +| H3 | Fixed | backend `c0e80a7`, frontend `38ff0db` | Google OAuth/bearer logs no longer print token material; frontend logger redacts sensitive object keys. | +| H4 | Fixed | backend `c0e80a7` | File delete/info routes use upload-relative paths resolved under the upload root. | +| H5 | Fixed | backend `c0e80a7` | Template batch endpoints cap array sizes and reject duplicate payment-completion ids. | +| H6 | Fixed | backend `c0e80a7` | Broad legacy user list is admin-only; contacts/search no longer expose email/phone. | +| M1 | Fixed | backend `c0e80a7` | Purchase-request dispute-hold routes use canonical `sameUser` checks. | +| M2 | Fixed | backend `c0e80a7` | Dashboard dispute participant checks use canonical identity comparison. | +| M3 | Fixed | backend `c0e80a7` | Temp/document uploads are blocked from public static serving. | +| M4 | Fixed | backend `c0e80a7` | Audited list endpoints use the shared `parsePagination` helper. | ## High Findings @@ -39,7 +54,7 @@ Fresh post-remediation audit track after the DB Query & Schema Audit was closed. **Files:** `backend/src/routes/disputeRoutes.ts:10-11`, `backend/src/controllers/disputeController.ts:9-31`, `backend/src/services/dispute/DisputeService.ts:96-170` -**Status:** Open +**Status:** Fixed in backend `c0e80a7` `POST /api/disputes` is mounted under `/api/disputes` and only requires authentication. The controller correctly ignores the client-submitted `buyerId` and passes `req.user.id`, but `DisputeService.createDispute` only verifies that `purchaseRequestId` exists. It does not verify that the authenticated user is the purchase request buyer, seller, admin, or resolver before creating a dispute and dispute chat tied to that purchase request. @@ -59,7 +74,7 @@ Add a regression test where user B attempts to dispute user A's purchase request **Files:** `backend/src/services/marketplace/marketplaceController.ts:2022-2025`, `backend/src/services/delivery/DeliveryService.ts:133-147`, legacy `backend/src/services/marketplace/routes.ts:2941-2947` -**Status:** Open +**Status:** Fixed in backend `c0e80a7` Delivery codes are sensitive one-time confirmation values. The mounted controller logs the full generated code: @@ -77,7 +92,7 @@ console.log(`Delivery code generated for request ${id}: ${deliveryCode}`); **Files:** `frontend/src/auth/services/google-oauth.ts:117-121`, `frontend/src/auth/services/google-oauth.ts:185-193`, `frontend/src/lib/axios.ts:69-76`, `frontend/src/utils/logger.ts:29-34` -**Status:** Open +**Status:** Fixed in backend `c0e80a7`, frontend `38ff0db` The frontend logger is development-enabled by default. Google signup logs the full OAuth `tokenResponse` and then the full `access_token`. Google signin logs token metadata and a token prefix. The axios request interceptor logs a bearer-token preview and localStorage key names. @@ -89,7 +104,7 @@ The frontend logger is development-enabled by default. Google signup logs the fu **Files:** `backend/src/services/file/fileRoutes.ts:71-80`, `backend/src/services/file/fileController.ts:247-280`, `backend/src/services/file/fileService.ts:225-268` -**Status:** Open +**Status:** Fixed in backend `c0e80a7` `DELETE /api/files/delete` never supplies `req.params.filename`, while the controller requires it. `GET /api/files/info/:filePath` provides `filePath`, but the controller also reads `filename`. These endpoints currently fail validation rather than doing useful work. @@ -103,7 +118,7 @@ The latent security issue is in the service: `deleteFile(filePath)` and `getFile **Files:** `backend/src/services/marketplace/requestTemplateRoutes.ts:267-325`, `backend/src/services/marketplace/RequestTemplateService.ts:472-663` -**Status:** Open +**Status:** Fixed in backend `c0e80a7` `batch-convert` validates that `items` is an array with `min: 1`, but no maximum. Each item can request `quantity` up to 100 and the service loops items sequentially, creates requests/offers, increments usage, re-fetches populated purchase requests, and schedules notifications. @@ -123,7 +138,7 @@ before the bulk status update. **Files:** `backend/src/app.ts:591-592`, `backend/src/services/user/userRoutes.ts:830-879`, `backend/src/services/user/userRoutes.ts:988-1006` -**Status:** Open +**Status:** Fixed in backend `c0e80a7` The legacy `/api/users` route is still mounted. `GET /api/users` requires authentication but no role restriction, allows `limit` up to 1000, and returns email, role, status, profile fields, and last login metadata for matching active users. The search endpoint also returns email and profile contact metadata to any authenticated caller with a two-character search term. @@ -137,7 +152,7 @@ The legacy `/api/users` route is still mounted. `GET /api/users` requires authen **Files:** `backend/src/services/dispute/disputeRoutes.ts:44-50`, `backend/src/services/dispute/disputeRoutes.ts:146-153` -**Status:** Open +**Status:** Fixed in backend `c0e80a7` The `/api/disputes/pr/:purchaseRequestId/...` route compares `request.buyerId.toString()` and preferred seller ids directly against `req.user.id`. In this codebase, many repositories now resolve UUID and legacy ObjectId forms. Raw comparison can false-deny legitimate users when one side is UUID and the other is legacy id. @@ -149,7 +164,7 @@ The `/api/disputes/pr/:purchaseRequestId/...` route compares `request.buyerId.to **Files:** `backend/src/controllers/disputeController.ts:125-130`, `backend/src/controllers/disputeController.ts:253-258`, `backend/src/db/repositories/drizzle/DrizzleDisputeRepo.ts:271-278` -**Status:** Open +**Status:** Fixed in backend `c0e80a7` The controller checks `dispute.buyerId._id.toString()` and `dispute.sellerId?._id.toString()`. Drizzle dispute records currently map these references into `{ _id: displayId }`, so this works today, but the controller is tightly coupled to a Mongoose-like shape rather than using canonical participant helpers. @@ -161,7 +176,7 @@ The controller checks `dispute.buyerId._id.toString()` and `dispute.sellerId?._i **Files:** `backend/src/app.ts:527-551`, `backend/src/services/file/fileRoutes.ts:21-69`, `backend/src/services/file/fileService.ts:75-109` -**Status:** Open +**Status:** Fixed in backend `c0e80a7` Chat attachments are explicitly blocked from static serving, but all other uploads are served through `/uploads` with `Cross-Origin-Resource-Policy: cross-origin`. Generic upload accepts images, PDFs, and Word documents by MIME type and returns a public URL. @@ -173,7 +188,7 @@ Chat attachments are explicitly blocked from static serving, but all other uploa **Files:** `backend/src/controllers/disputeController.ts:70-71`, `backend/src/controllers/pointsController.ts:46-78`, `backend/src/services/payment/paymentRoutes.ts:171-174`, `backend/src/services/chat/chatController.ts:301-355` -**Status:** Open +**Status:** Fixed in backend `c0e80a7` The DB audit fixed many concrete query caps, but the current codebase still has route handlers that call `parseInt(limit)` and pass the result onward without a shared normalizer. Some downstream repos cap values, some do not, and malformed/negative values can produce inconsistent behavior.