Skip to content

Add tests for admin DLQ routes, admin suspension, storage, profile pictures, and route mounts - #311

Merged
Tybravo merged 1 commit into
SwiftChainn:mainfrom
BigNathan1:feat/admin-dlq-storage-route-tests
Oct 22, 2026
Merged

Tybravo merged 1 commit into
SwiftChainn:mainfrom
BigNathan1:feat/admin-dlq-storage-route-tests

Conversation

@BigNathan1

Copy link
Copy Markdown
Contributor

Summary

This PR adds test coverage for the admin DLQ routes, admin user suspension, the storage drivers and profile picture pipeline, and a route-mount smoke test. It also fixes a path traversal gap in the local storage driver's upload path that the storage tests exposed.

closes #232
closes #234
closes #235
closes #236

Implementation details

#232 Admin DLQ routes (tests/adminDlqRoutes.test.ts)

  • List: empty page, populated page, pagination (page/limit), 401 without a token, 403 for non-admins.
  • Retry: success resolves the entry and calls the Stellar retry once; failure keeps the entry pending, records the error, and increments retryCount; repeated failures accumulate; already-resolved entries return 400; unknown IDs return 404; non-admins get 403.
  • Real MongoDB (MongoMemoryServer) and real JWT auth. Only stellarService.submitEscrowLock is mocked.

#235 Admin user suspension

  • tests/unit/controllers/adminController.test.ts: reason required and trimmed (400), ban must be boolean (400), ban defaults to false, ban: true passed through, 409/404 forwarded from the service, 401 without an actor, and requireRole(ADMIN) returns 403 for non-admins.
  • tests/unit/services/adminService.test.ts (real MongoDB): suspend and ban apply status and reason; 409 for already-suspended or already-banned; 404 unknown user; 400 malformed ID; 422 self-action; 403 targeting another admin; audit log entry includes the acting admin ID and reason.

#234 Storage and profile pictures

  • tests/unit/services/storage.service.test.ts: local driver round-trip on a temp directory, nested keys, no-op delete of a missing file, traversal rejected on upload and delete; S3 driver with a fake client (signed URL returned, upload/delete commands, error propagation); factory selects the driver from config and caches it until reset.
  • tests/unit/services/profilePicture.service.test.ts: real Sharp. Oversized images fit within 500px and are encoded as JPEG; small images are not enlarged; non-image input returns 500; unsupported MIME and oversize return 400; replacement deletes the previous key; old-file delete failure does not fail the request; delete clears fields; isValidImage.
  • Storage faked with a jest mock, so no bucket or disk writes outside the temp directory.

Source fix (src/services/storage.service.ts)

  • LocalStorageDriver.upload() previously built the path with path.join and no containment check, so a key such as ../escape.txt was written outside the upload root. Upload and delete now share a resolveWithinRoot helper that rejects any key resolving outside the root.

#236 Route mount smoke test (tests/unit/routes/mountSmoke.test.ts)

  • Walks the real routes/index.ts router stack to collect every mounted route (more than 50).
  • Sends GET (and POST/PUT/PATCH/DELETE with an empty JSON body) with no auth. Asserts each response is a JSON envelope with a boolean success and does not leak a ReferenceError/TypeError text.
  • AST check: every controller or middleware passed directly to router.<verb>() or router.use() is bound by an import or declaration in its route file.
  • README parity: a route file that is both not imported by routes/index.ts and named in the README fails the test. Currently none do.

Tests and results

Command run:

MONGOMS_VERSION=7.0.14 npx jest tests/adminDlqRoutes.test.ts tests/unit/controllers/adminController.test.ts tests/unit/services/adminService.test.ts tests/unit/services/storage.service.test.ts tests/unit/services/profilePicture.service.test.ts tests/unit/routes/mountSmoke.test.ts --coverage

Result: 6 suites passed, 68 tests passed, 0 failed.

Coverage of touched files (from that run): storage.service.ts 97.95% lines, profilePicture.service.ts 86.81%, dlqController.ts 94.11%, dlqService.ts 92.85%, adminController.ts 53.84%, adminService.ts 63.82%. The lower adminService/adminController figures come from getAdminDisputes and the disputes controller, which are outside this issue's scope.

ESLint: clean on all touched files. Prettier applied.

Full pnpm test was not run as part of this PR (see known limitations).

Compatibility and security considerations

  • The only production code change is the upload containment check in LocalStorageDriver. Keys that already resolved inside the upload root behave as before. Keys that escaped now throw instead of writing outside the directory.
  • No API, route, or response shape changes.
  • No secrets or real data are used; tests use temporary directories and in-memory MongoDB.

Known limitations

  • [Testing] Write integration tests for the admin DLQ routes (list, retry, exhaustion) #232 exhaustion is not tested. The issue asks for a maxAttempts limit and an exhausted state. Neither exists in DlqService: retries are unbounded and a failed retry returns the entry to pending. I did not invent that behavior in this PR; it needs a maintainer decision.
  • MongoDB version: jest.setup.js pins MONGOMS_VERSION=7.0.14, but Jest loads tests/jest.setup.ts, which does not. Without the env var, tests download MongoDB 6.0.14, which was slow and interrupted on my machine. The Mongo-backed suites pass with MONGOMS_VERSION=7.0.14. Consider pinning in tests/jest.setup.ts as a follow-up.
  • Pre-existing failures on main, not caused by this PR: npx tsc --noEmit reports errors in src/services/escrow.service.ts (duplicate AuditLog identifier, a sort typing error, and a missing ScanLapsedFundingWindowsResult) and in tests/unit/services/idempotency.service.test.ts (missing uuid types). pnpm run build also fails on ignored build scripts. Neither is touched here.
  • Issue-to-repo paths: the issues reference backend/tests/...; this repository keeps src/ and tests/ at the root, so the files are placed there.
  • Issue [Testing] Add a route-mount smoke test that fails on orphaned or unimported handlers #236 counts: the issue mentions seven orphaned route files; the current tree has one unmounted route module (indexerRoutes.ts), which the README does not document.
  • The issues ask for a screenshot or Postman proof of work. Test output above is the proof provided.

…l upload

Resolve issues SwiftChainn#232, SwiftChainn#234, SwiftChainn#235, SwiftChainn#236.

- Add integration tests for the admin DLQ routes covering list pagination,
  retry success, retry failure with retryCount increments, resolved-entry
  rejection, 404, and admin-only gating (SwiftChainn#232).
- Add unit tests for admin user suspension: controller reason and ban
  validation, service status rules (409, 404, 400, 422, 403), and audit log
  entries (SwiftChainn#235).
- Add storage driver tests (local disk round-trip, S3 with a fake client,
  driver factory) and profile picture pipeline tests (Sharp resize, JPEG
  encoding, replace and delete flows) (SwiftChainn#234).
- Add a route mount smoke test that walks the Express router stack, checks
  each route returns a JSON envelope without crashing, verifies handlers
  passed to routers are imported, and checks route files are not both
  unmounted and documented in the README (SwiftChainn#236).

Fix: LocalStorageDriver.upload() now rejects keys that resolve outside the
upload root, matching the existing check in delete(). Previously a key such
as "../escape.txt" was written outside the upload directory.
@Tybravo

Tybravo commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator

@BigNathan1

Well done and Thank you for your contribution

@Tybravo
Tybravo merged commit 16e19a6 into SwiftChainn:main Oct 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment