---
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: closed
---

# Security, DB Performance, and Logic Audit - 2026-06-07

Fresh post-remediation audit track after the DB Query & Schema Audit was closed. This pass targets security, authorization, sensitive data handling, DB/performance regressions outside the closed report, and business-logic/state-machine correctness.

> [!info] Scope
> Initial source pass covered mounted backend routes, auth/ownership middleware, uploads/static file serving, dispute controllers/services, delivery-code flows, frontend token handling, and template checkout batch paths. This document is the starting register; expand it as each wave verifies more surfaces.

## Method

- Review only code that is mounted or imported by mounted routes, unless the finding explicitly calls out legacy/dead code.
- Separate confirmed findings from follow-up scan queue items.
- Use the same severity posture as the DB audit:
  - Critical: cross-tenant privilege bypass, funds/control-plane compromise, or exploitable unauthenticated secret exposure.
  - High: authenticated cross-tenant mutation/read, sensitive token/code leakage, unbounded user-triggered load, or logic that can corrupt user-visible state.
  - 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.

## Remediation Summary

| 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

### H1 - Dashboard dispute creation does not verify purchase-request ownership

**Files:** `backend/src/routes/disputeRoutes.ts:10-11`, `backend/src/controllers/disputeController.ts:9-31`, `backend/src/services/dispute/DisputeService.ts:96-170`

**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.

An authenticated user can therefore create a dashboard dispute for another user's purchase request if they know or can infer the request id. The new dispute stores the attacker as `buyerId` and can pull the real selected/preferred seller into a chat.

**Impact:** Cross-tenant state mutation, support queue pollution, seller notification/chat creation for unrelated requests, and possible confusion in admin dispute handling.

**Fix:** Before creating the chat or dispute, load the purchase request context and require one of:

- requester is the request buyer;
- requester is the selected/preferred seller, if seller-initiated disputes are intentionally allowed;
- requester is `admin` or `resolver`.

Add a regression test where user B attempts to dispute user A's purchase request and receives 403.

### H2 - Delivery codes are printed in backend logs

**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:** Fixed in backend `c0e80a7`

Delivery codes are sensitive one-time confirmation values. The mounted controller logs the full generated code:

```ts
console.log(`Delivery code generated for request ${id}: ${deliveryCode}`);
```

`DeliveryService.verifyDeliveryCode` also logs failed attempts with both the submitted value and the expected code.

**Impact:** Anyone with backend log access can recover or validate a delivery code and influence delivery confirmation. The failed-attempt log is especially risky because it turns an invalid guess into the correct code in logs.

**Fix:** Never log the code or expected value. Log only request id, actor id, success/failure reason, and an event id. If auditability is needed, store a salted hash or redacted code suffix only.

### H3 - Google OAuth and bearer-token debug logs expose access-token material in development logs

**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:** 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.

**Impact:** Dev/local screenshots, browser consoles, test logs, or shared debugging sessions can leak usable OAuth or API bearer material. Production `info` logs are disabled, so this is not currently a production console leak, but it violates the "never log tokens" rule and is easy to re-enable accidentally.

**Fix:** Remove token value logging entirely. Keep boolean/length-only diagnostics if required, and add a small frontend logger redaction helper for keys containing `token`, `authorization`, `secret`, `password`, or `code`.

### H4 - Generic file delete/info routes are broken and the service path handling is unsafe for a future repair

**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:** 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.

The latent security issue is in the service: `deleteFile(filePath)` and `getFileInfo(filePath)` accept arbitrary raw filesystem paths when the input does not start with `/uploads`. A future route-param fix could accidentally expose arbitrary file stat/delete behavior to any authenticated caller.

**Impact:** Broken file-management functionality now; high-risk latent arbitrary file access/delete if the route is repaired by simply passing request params through.

**Fix:** Replace path-based delete/info with file identifiers or strict allow-listed upload-relative paths. Resolve with `path.resolve`, require the final path to stay under the configured upload directory, reject `..`, reject absolute paths, and add route tests for traversal inputs.

### H5 - Template batch endpoints accept unbounded arrays and perform per-id/per-item work

**Files:** `backend/src/services/marketplace/requestTemplateRoutes.ts:267-325`, `backend/src/services/marketplace/RequestTemplateService.ts:472-663`

**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.

`complete-payment` validates `requestIds` with `min: 1`, but no maximum, then runs:

```ts
await Promise.all(requestIds.map((id) => this.marketplaceRepo.findPurchaseRequestById(id)))
```

before the bulk status update.

**Impact:** An authenticated caller can submit large arrays within the request body limit and fan out many DB reads/writes or a large concurrent read burst. This is a DB/performance and abuse-control issue.

**Fix:** Add explicit max lengths for `items` and `requestIds` (for example 50 or the intended cart maximum), reject duplicate request ids, and move the ownership check to a single repository method that fetches all requested rows by id.

### H6 - User-facing legacy user list exposes broad user data to any authenticated account

**Files:** `backend/src/app.ts:591-592`, `backend/src/services/user/userRoutes.ts:830-879`, `backend/src/services/user/userRoutes.ts:988-1006`

**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.

**Impact:** Broad authenticated user enumeration and privacy exposure. This also creates avoidable DB load because the route still uses Mongoose `User.find`/`countDocuments` rather than the newer repository path.

**Fix:** Decide whether non-admin user discovery is still a product requirement. If not, make `/api/users` admin-only. If yes, narrow returned fields, cap limits much lower, remove email unless explicitly needed, and move it behind a purpose-specific contacts/search endpoint.

## Medium Findings

### M1 - Purchase-request dispute-hold route uses raw ID string comparisons

**Files:** `backend/src/services/dispute/disputeRoutes.ts:44-50`, `backend/src/services/dispute/disputeRoutes.ts:146-153`

**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.

**Impact:** Compatibility/logic failure, especially during mixed legacy/UUID flows. This looks more like false-deny than privilege bypass.

**Fix:** Reuse the existing `sameUser`/canonical-id helper or expose canonical participant checks from the repository.

### M2 - Dashboard dispute participant checks assume populated `_id` shapes

**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:** 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.

**Impact:** Future repository shape changes can break dispute reads/evidence with 500s or false denials.

**Fix:** Add a small `isDisputeParticipant(dispute, user)` helper using `toIdString` plus canonical user comparison, and use it for details/evidence access.

### M3 - Public `/uploads` serving exposes all non-chat uploaded files cross-origin

**Files:** `backend/src/app.ts:527-551`, `backend/src/services/file/fileRoutes.ts:21-69`, `backend/src/services/file/fileService.ts:75-109`

**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.

**Impact:** This may be intended for avatars, blog images, and product/request-template images, but generic temp/documents uploads become public once uploaded. There is no ownership check, no signed URL, no magic-byte validation for generic documents, and no malware/content scanning.

**Fix:** Split public media uploads from private documents. Keep only intentional public media under static `/uploads`; serve private documents through authenticated file routes with ownership checks. Validate file signatures, not only MIME.

### M4 - Several list endpoints parse user-supplied limits without consistent normalization

**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:** 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.

**Impact:** Performance variability and route-specific edge cases. This is lower priority than H5 because it requires endpoint-specific downstream confirmation.

**Fix:** Add a shared `parsePagination` helper with min/max/defaults and replace local ad hoc parsing.

## Follow-up Scan Queue

1. Security wave: fix/verify H1-H4 first, then re-scan authz on mounted marketplace/payment routes.
2. DB/performance wave: quantify H5 with expected max cart size; inspect remaining route-level `limit` parsing and user-route Mongoose paths.
3. Logic wave: review delivery-code state transitions, dispute creation/resolution side effects, and payment status mutation routes.
4. Privacy wave: decide intended visibility for `/api/users`, generic uploaded documents, and dispute evidence URLs.
5. Regression tests: add at least one failing test per high finding before code fixes where practical.

## Already Checked in This Pass

| Area | Result |
|---|---|
| Payment callback auth | `paymentCallbackAuth` fails closed when secret is unset and uses timing-safe comparison. |
| Chat attachment serving | Static `/uploads/chat` is blocked; authenticated chat attachment route checks membership and path traversal. |
| Dashboard dispute body `buyerId` | Controller ignores body `buyerId` and uses `req.user.id`; the missing check is purchase-request ownership, not body trust. |
| Legacy marketplace route module | `backend/src/services/marketplace/routes.ts` still contains old route code, but `app.ts` mounts `marketplaceControllerRouter`, not the legacy router. |
| Taskmaster | `task-master next` reported no available tasks. |