Skip to content

Recipicity API Backend -- Architecture Review

Date: 2026-02-25 Reviewer: Claude Opus 4.6 Scope: /opt/development/recipicity/staging/recipicity-api/src/ Codebase: Node.js/TypeScript Express API with Prisma ORM, PostgreSQL, Redis, MinIO


Executive Summary

The Recipicity API is a substantial, feature-rich backend serving a social recipe platform. It demonstrates solid fundamentals in many areas -- proper JWT authentication, input validation via Joi schemas, rate limiting, security headers, CORS hardening, and audit logging. However, the codebase suffers from significant architectural debt: fat route handlers that embed business logic, pervasive N+1 query patterns, inconsistent error handling conventions, duplicated utility code across files, and a lack of authorization enforcement on certain endpoints. This review identifies 37 discrete findings across 8 categories.


Table of Contents

  1. Route Organization and RESTful Design
  2. Middleware Patterns and Error Handling
  3. Prisma Query Efficiency
  4. Authentication and Authorization
  5. Input Validation and Sanitization
  6. Code Duplication and DRY Violations
  7. Error Response Consistency
  8. Service Layer Separation

1. Route Organization and RESTful Design

Finding 1.1: Duplicate Comment Endpoints (Medium)

Files: - /opt/development/recipicity/staging/recipicity-api/src/routes/social.ts (lines 256-415) - /opt/development/recipicity/staging/recipicity-api/src/routes/comments.ts (full file)

Comments for recipes are implemented in TWO separate route files. social.ts has GET/POST/DELETE /api/social/recipes/:recipeId/comments while comments.ts has the same operations at GET/POST /api/comments/recipe/:recipeId plus edit and reply functionality. Both are mounted in server.ts. This means:

  • Two different URL paths serve the same purpose
  • The social.ts version lacks threading (replies), editing, and the isAuthor flag
  • The social.ts version does NOT check if the commenter is the recipe author before sending notifications (it sends a "new comment" notification even to yourself)

Recommendation: Remove comment endpoints from social.ts entirely. The comments.ts version is more complete and correct.

Finding 1.2: Inconsistent Resource Naming (Low)

File: /opt/development/recipicity/staging/recipicity-api/src/server.ts (lines 333-366)

Route paths use inconsistent naming conventions: - /api/mealplans (no hyphen) - /api/planned-meals (hyphenated) - /api/grocerylists (no hyphen) - /api/grocery-integrations (hyphenated) - /api/shared-meal-plans (hyphenated) - /api/push-notifications (hyphenated) - /api/recipe-import (hyphenated) - /api/ai-recipes (hyphenated)

Recommendation: Standardize on hyphenated kebab-case for all multi-word resource paths (/api/meal-plans, /api/grocery-lists).

Finding 1.3: Inline Route Handlers in server.ts (Medium)

File: /opt/development/recipicity/staging/recipicity-api/src/server.ts (lines 380-531)

The main server file contains inline route handlers: - GET /api/subscription-plans (lines 380-503) -- a 120-line inline handler with hardcoded fallback plan data - GET /api/search/recipes (lines 510-521) - GET /api/search (lines 524-531)

These should be extracted to dedicated route files. The hardcoded subscription plan fallback is a maintenance hazard -- if plan details change, someone must update both the database AND this code.

Recommendation: Extract to /routes/subscription-plans.ts and /routes/search.ts. Move hardcoded plan data to a configuration file or seed script.

Finding 1.4: Non-RESTful Search Redirect Pattern (Low)

File: /opt/development/recipicity/staging/recipicity-api/src/server.ts (lines 510-531)

GET /api/search/recipes uses res.redirect() to send a 302 to /api/recipes?search=.... This forces an extra HTTP round-trip on every search request. The redirect is transparent to browsers but not to API clients that may not follow redirects (native mobile apps, curl with default settings).

Recommendation: Proxy the request internally or simply have the frontend call /api/recipes?search=... directly.

Finding 1.5: Mixed Use of PUT and PATCH (Low)

File: /opt/development/recipicity/staging/recipicity-api/src/routes/notifications.ts (lines 94-181)

Both PUT and PATCH are registered for the same mark-as-read endpoints, duplicating the handler code. While ostensibly for "backward compatibility," this doubles the surface area and the handler code is literally copy-pasted.

Recommendation: Pick one HTTP method (PATCH is semantically correct for partial updates) and alias the other using a simple redirect or single shared handler function.


2. Middleware Patterns and Error Handling

Finding 2.1: Two Separate asyncHandler Implementations (High)

Files: - /opt/development/recipicity/staging/recipicity-api/src/utils/asyncHandler.ts (standalone utility) - /opt/development/recipicity/staging/recipicity-api/src/middleware/errorHandler.ts (lines 85-87, exported as asyncHandler)

There are TWO identical asyncHandler implementations. Different route files import from different locations:

// Some routes use:
import { asyncHandler } from '../utils/asyncHandler';
// Others use:
import { asyncHandler } from '../middleware/errorHandler';

Files importing from errorHandler.ts: - social.ts, bookmarks.ts, tags.ts, upload.ts, recipeImport.routes.ts

Files importing from utils/asyncHandler.ts: - recipes.ts, users.ts, mealplans.ts, grocerylists.ts, ratings.ts, comments.ts, families.ts, aiRecipes.routes.ts

Both implementations are functionally identical, but the Function type used in both is overly broad and loses TypeScript safety.

Recommendation: Delete the duplicate in errorHandler.ts, update all imports to use utils/asyncHandler.ts, and type the function parameter properly:

type AsyncRouteHandler = (req: Request, res: Response, next: NextFunction) => Promise<unknown>;
export const asyncHandler = (fn: AsyncRouteHandler) => ...

Finding 2.2: Local asyncHandler Re-declaration in notifications.ts (Medium)

File: /opt/development/recipicity/staging/recipicity-api/src/routes/notifications.ts (lines 14-17)

This file declares its OWN local asyncHandler instead of importing the shared one:

const asyncHandler =
  (fn: (req: Request, res: Response, next: NextFunction) => Promise<unknown>) =>
  (req: Request, res: Response, next: NextFunction) =>
    Promise.resolve(fn(req, res, next)).catch(next);

Three implementations of the same 3-line function in a single codebase.

Recommendation: Delete the local declaration and import from utils/asyncHandler.ts.

Finding 2.3: Readiness Check Creates New Redis Connections (High)

File: /opt/development/recipicity/staging/recipicity-api/src/server.ts (lines 248-262)

The /readyz endpoint creates a NEW Redis instance on every request:

const Redis = require('ioredis');
const redis = new Redis(/* ... */);
await redis.ping();
await redis.disconnect();

This bypasses the RedisManager singleton, creates connection churn, and uses a require() call inside a request handler. If /readyz is polled every 10 seconds by a health checker, that is 8,640 unnecessary Redis connections per day.

Recommendation: Use RedisManager.healthCheck() which already exists in /opt/development/recipicity/staging/recipicity-api/src/lib/redis.ts (lines 62-70).

Finding 2.4: Missing Rate Limiting on Several Routes (Medium)

Files: - /opt/development/recipicity/staging/recipicity-api/src/routes/upload.ts -- No rate limiter applied - /opt/development/recipicity/staging/recipicity-api/src/routes/tags.ts -- No rate limiter applied - /opt/development/recipicity/staging/recipicity-api/src/routes/collections.ts -- Uses its own inline rate limiter instead of the shared standardLimiter

Upload is a particularly expensive operation (image processing, storage writes) and should have strict rate limiting.

Recommendation: Apply standardLimiter to upload.ts and tags.ts. For upload, consider using strictLimiter or a custom limiter with lower thresholds.

Finding 2.5: Incomplete Private IP Range Detection in rateLimiter.ts (Low)

File: /opt/development/recipicity/staging/recipicity-api/src/middleware/rateLimiter.ts (lines 8-25)

The isInternalIp function checks for 172.16.* through 172.3* but the private range is 172.16.0.0 - 172.31.255.255. The current pattern ip.startsWith('172.2') incorrectly matches 172.20-172.29, and ip.startsWith('172.3') incorrectly matches 172.30-172.39 (172.32+ are NOT private). Meanwhile 172.31.* IS private and IS matched (via 172.3), but 172.32-172.39 are also incorrectly matched.

Recommendation: Use a proper CIDR check or fix the prefix matching:

const octet2 = parseInt(ip.split('.')[1]);
ip.startsWith('172.') && octet2 >= 16 && octet2 <= 31


3. Prisma Query Efficiency

Finding 3.1: Severe N+1 Problem in User Search and Recent Users (Critical)

File: /opt/development/recipicity/staging/recipicity-api/src/routes/users.ts

Lines 377-388 (GET /recent):

const usersWithCounts = await Promise.all(
  users.map(async (user) => {
    const [recipeCount, followerCount] = await Promise.all([
      prisma.recipe.count({ where: { authorId: user.id, published: true } }),
      prisma.follow.count({ where: { followingId: user.id } }),
    ]);
    return { ...user, _count: { recipes: recipeCount, followers: followerCount } };
  })
);

For 6 users, this fires 12 additional queries (2 per user). The same pattern appears in:

  • Lines 470-485 (GET /search): Up to 50 users x 2 queries = 100 additional queries
  • Lines 96-101 (GET /profile): 3 additional count queries per profile view
  • Lines 998-1001 (GET /:username): 3 additional count queries

Prisma's _count in include can handle this in a single query:

const users = await prisma.user.findMany({
  where: { ... },
  include: {
    _count: {
      select: { recipes: true, followers: true }
    }
  }
});

The code comments say "FIXED: Manually count for each user to ensure accuracy" -- suggesting the Prisma _count was producing incorrect results. This is likely a Prisma version bug that should be retested, because the current approach is O(N) queries.

Recommendation: Re-test Prisma _count with the current Prisma version. If it still fails, use a single raw SQL query with COUNT and GROUP BY instead of N+1 individual counts.

Finding 3.2: N+1 in Tag Recipe Listing (Critical)

File: /opt/development/recipicity/staging/recipicity-api/src/routes/tags.ts (lines 223-281)

const transformedRecipes = await Promise.all(
  recipes.map(async (recipe) => {
    let isLiked = false;
    let isBookmarked = false;
    let userRating = null;

    if (userId) {
      const [like, bookmark, rating] = await Promise.all([
        prisma.like.findUnique({ where: { userId_recipeId: { userId, recipeId: recipe.id } } }),
        prisma.bookmark.findUnique({ where: { userId_recipeId: { userId, recipeId: recipe.id } } }),
        prisma.rating.findUnique({ where: { userId_recipeId: { userId, recipeId: recipe.id } } }),
      ]);
      // ...
    }
    const avgRating = await prisma.rating.aggregate({ where: { recipeId: recipe.id }, _avg: { rating: true } });
    // ...
  })
);

For an authenticated user viewing 12 recipes, this fires 48 additional queries (3 user-specific lookups + 1 aggregate per recipe). This is the worst N+1 pattern in the codebase.

Recommendation: Batch the user-specific lookups:

const [userLikes, userBookmarks, userRatings] = await Promise.all([
  prisma.like.findMany({ where: { userId, recipeId: { in: recipeIds } } }),
  prisma.bookmark.findMany({ where: { userId, recipeId: { in: recipeIds } } }),
  prisma.rating.findMany({ where: { userId, recipeId: { in: recipeIds } } }),
]);
const likeSet = new Set(userLikes.map(l => l.recipeId));
// ... then use Sets for O(1) lookups

Finding 3.3: Over-fetching in Social Like/Unlike (Medium)

File: /opt/development/recipicity/staging/recipicity-api/src/routes/social.ts (lines 38-40)

const recipe = await prisma.recipe.findUnique({
  where: { id: recipeId },
});

This fetches the entire recipe row (including large JSON ingredients and instructions columns) just to check existence and get authorId. The same pattern repeats for unlike, follow, and comment creation.

Recommendation: Use select to fetch only needed fields:

const recipe = await prisma.recipe.findUnique({
  where: { id: recipeId },
  select: { id: true, authorId: true, title: true },
});

Finding 3.4: Missing Database Indexes on Key Query Patterns (Medium)

File: /opt/development/recipicity/staging/recipicity-api/prisma/schema.prisma

The Follow model (visible in the schema) is queried heavily by both followerId and followingId for counting followers/following, checking follow status, and building feeds. While Prisma auto-creates indexes for @unique constraints, the individual column indexes should be verified.

The Notification model is queried by (userId, read) for unread counts (called on nearly every page load) but there is likely no compound index for this.

The Bookmark model is queried by userId for listing but may lack a userId index beyond the unique constraint.

Recommendation: Add compound indexes:

@@index([userId, read])  // on Notification
@@index([userId, createdAt])  // on Bookmark

Finding 3.5: Collections GET Fetches All Items Eagerly (Medium)

File: /opt/development/recipicity/staging/recipicity-api/src/routes/collections.ts (lines 23-55)

GET /api/collections fetches ALL collections with ALL their items (recipes with nested author data). For a user with 10 collections averaging 20 recipes each, this loads 200 recipe objects in a single query. The list endpoint should only return collection metadata and counts.

Recommendation: Remove the items include from the list endpoint; fetch items only on GET /api/collections/:id.


4. Authentication and Authorization

Finding 4.1: optionalAuthMiddleware Does Not Verify User Active/Password Status (High)

File: /opt/development/recipicity/staging/recipicity-api/src/middleware/auth.ts (lines 79-114)

The optionalAuthMiddleware decodes the JWT but does NOT: - Check if the user is still active (active: true) - Check if the password was changed after token issuance - Query the database at all

This means a deactivated or banned user can still make authenticated requests to any endpoint using optionalAuthMiddleware. They can see private recipes, check follow status, and access personalized data.

In contrast, authMiddleware (lines 12-77) correctly validates active status and password change timestamps via a database query.

Recommendation: Either make optionalAuthMiddleware also validate active status (with caching to avoid per-request DB hits), or accept the trade-off and document it. At minimum, add a comment explaining the security implication.

Finding 4.2: Upload Delete Endpoint Uses Path-Based Authorization (High)

File: /opt/development/recipicity/staging/recipicity-api/src/routes/upload.ts (lines 322-349)

if (!fileName.includes(`/${userId}/`)) {
  return res.status(403).json({ error: 'Not authorized to delete this image' });
}

Authorization is based on whether the file path contains the user's ID. This is fragile -- a malicious user could potentially craft a path like recipe/victimId/../../attacker/victimId/file.webp to bypass the check (depending on how MinIO normalizes paths). While the slash structure makes this unlikely in practice, path-based authorization is an anti-pattern.

Recommendation: Store image ownership in the database (a simple uploads table mapping fileName to userId) and validate against that.

Finding 4.3: Legacy Import Endpoint Trusts Client-Provided userId (Critical)

File: /opt/development/recipicity/staging/recipicity-api/src/routes/recipeImport.routes.ts (lines 939-1000)

The legacy /import endpoint reads userId from the request body:

const { url, userId, saveAsDraft = true } = req.body;

Despite having authMiddleware, the code uses the client-supplied userId instead of req.user.userId. An authenticated user could import recipes into ANY other user's account by providing a different userId in the body.

Recommendation: Replace req.body.userId with req.user?.userId. This is a privilege escalation vulnerability.

Finding 4.4: Comment Delete in social.ts Allows Recipe Author to Delete Any Comment (Low)

File: /opt/development/recipicity/staging/recipicity-api/src/routes/social.ts (lines 388-415)

if (comment.userId !== userId && comment.recipe.authorId !== userId) {
  return res.status(403).json({ error: 'Not authorized to delete this comment' });
}

This allows recipe authors to delete any comment on their recipe. This is a design decision, not a bug, but it is worth noting that the standalone comments.ts file (line 525) does NOT grant recipe authors this power -- only the comment author can delete. The two files have contradictory authorization logic.

Finding 4.5: Family Invite Code Lookup Does Not Require Authentication (Low)

File: /opt/development/recipicity/staging/recipicity-api/src/routes/families.ts (lines 510-550)

GET /api/families/invite/:inviteCode is publicly accessible. While it only returns the family name, description, and member count, it allows anyone to probe invite codes and enumerate family names. The invite codes are 8 hex characters (4 bytes), giving only 4.3 billion possibilities -- feasible to brute-force.

Recommendation: Require authentication for this endpoint, or use longer invite codes (e.g., 16 bytes / 32 hex chars).


5. Input Validation and Sanitization

Finding 5.1: No Validation on Profile Update (Medium)

File: /opt/development/recipicity/staging/recipicity-api/src/routes/users.ts (lines 125-159)

PUT /api/users/profile accepts firstName, lastName, bio, and location from req.body with NO validation schema. There is no length limit on bio or location. A user could submit a 10MB string as their bio.

Note that updateProfileSchema exists in /opt/development/recipicity/staging/recipicity-api/src/validation/auth.ts (lines 60-85) but is never imported or used in the users route.

Recommendation: Import and apply updateProfileSchema to the profile update endpoint.

Finding 5.2: No Validation on Notification Settings Update (Medium)

File: /opt/development/recipicity/staging/recipicity-api/src/routes/users.ts (lines 721-817)

PUT /api/users/notification-settings has some manual validation for digestDay, digestTime, and digestFrequency, but passes the raw req.body as updateData directly to Prisma's upsert. An attacker could inject arbitrary fields:

{
  "digestFrequency": "weekly",
  "id": "attacker-controlled-id",
  "userId": "different-user-id"
}

Prisma will silently ignore unknown fields, but userId IS a valid field on NotificationSettings, potentially allowing a user to overwrite another user's settings.

Recommendation: Whitelist allowed fields before passing to Prisma:

const { emailLikes, emailComments, digestFrequency, digestDay, digestTime, digestTimezone } = req.body;
const updateData = { emailLikes, emailComments, digestFrequency, digestDay, digestTime, digestTimezone };

Finding 5.3: Email Preferences Accepts Arbitrary JSON (Medium)

File: /opt/development/recipicity/staging/recipicity-api/src/routes/users.ts (lines 564-604)

POST /api/users/email-preferences does const updates = req.body then spreads it over existing preferences with no validation. A user could store arbitrary JSON in their emailPreferences field:

{ "marketing": true, "xss": "<script>alert(1)</script>", "arbitraryField": "anything" }

While the sanitizeHtml middleware strips script tags from strings, the schema itself is not validated.

Recommendation: Define a Joi schema for email preferences that only accepts known boolean fields.

Finding 5.4: No CUID Validation on Path Parameters (Medium)

Most route handlers accept :id, :recipeId, :userId, etc. as path parameters and pass them directly to Prisma queries without validating the format. While Prisma will simply return null for an invalid CUID, this wastes a database round-trip.

The validateObjectId middleware exists in /opt/development/recipicity/staging/recipicity-api/src/middleware/security.ts (lines 48-59) but is NEVER used anywhere in the codebase.

Recommendation: Apply validateObjectId middleware to all routes that accept ID parameters, or validate IDs in a shared middleware applied to the router.

Finding 5.5: Password Reset Minimum Length Inconsistency (Low)

Files: - /opt/development/recipicity/staging/recipicity-api/src/routes/auth.ts (line 704): Reset password accepts password.length < 6 - /opt/development/recipicity/staging/recipicity-api/src/validation/auth.ts (line 23): Registration requires min(8) plus uppercase, lowercase, and digit

A user who registers with a strong password (8+ chars, mixed case, digits) can reset it to just 6 lowercase characters. The reset endpoint has weaker validation than registration.

Recommendation: Apply the same password validation rules to reset-password as registration, or at minimum match the 8-character minimum.


6. Code Duplication and DRY Violations

Finding 6.1: getUserId Helper Duplicated Across 6+ Files (High)

The pattern for extracting userId from an authenticated request is reimplemented in nearly every route file:

// Version 1 (recipes.ts, users.ts):
const getUserId = (req: Request): string =>
  ((req as unknown as Record<string, unknown>).user as { userId: string }).userId;

// Version 2 (bookmarks.ts, grocerylists.ts):
const userId = ((req as unknown as Record<string, unknown>).user as { userId: string }).userId;

// Version 3 (social.ts, comments.ts):
const userId = (req as AuthenticatedRequest).user.userId;

// Version 4 (recipeImport.routes.ts, aiRecipes.routes.ts):
const userId = req.user?.userId;

Some versions use triple-casting through unknown and Record, some use the typed AuthenticatedRequest, and some use the augmented Express types. Version 4 is the cleanest (leverages the express.d.ts type augmentation).

Recommendation: Create a single utility in utils/auth.ts:

export function getUserId(req: Request): string {
  if (!req.user?.userId) throw new UnauthorizedError();
  return req.user.userId;
}
export function getOptionalUserId(req: Request): string | undefined {
  return req.user?.userId;
}

Finding 6.2: MinIO Client Initialized Twice (Medium)

Files: - /opt/development/recipicity/staging/recipicity-api/src/routes/upload.ts (lines 20-37) - /opt/development/recipicity/staging/recipicity-api/src/routes/users.ts (lines 24-41)

Both files create their own MinioClient instance with identical configuration. Both validate credentials at module load time and throw fatal errors.

Recommendation: Extract MinIO client initialization to a shared lib/minio.ts singleton, similar to lib/database.ts and lib/redis.ts.

Finding 6.3: Pagination Calculation Duplicated Everywhere (Low)

Every paginated route repeats:

const page = Math.max(1, parseInt(req.query.page as string) || 1);
const limit = Math.min(50, Math.max(1, parseInt(req.query.limit as string) || 12));
const skip = (page - 1) * limit;

And the response pagination object:

pagination: { page, limit, total, pages: Math.ceil(total / limit) }

This appears in at least 15 route handlers.

Recommendation: Create a parsePagination(req, defaults?) utility and a buildPaginationResponse(page, limit, total) helper.

Finding 6.4: Rate Limiter Defined Three Times for Auth (Low)

Files: - /opt/development/recipicity/staging/recipicity-api/src/routes/auth.ts (lines 38-47): Local authLimiter - /opt/development/recipicity/staging/recipicity-api/src/middleware/rateLimiter.ts (lines 70-79): Exported authLimiter - /opt/development/recipicity/staging/recipicity-api/src/server.ts (lines 162-180): Global limiter

The auth route defines its own rate limiter instead of importing the shared one from rateLimiter.ts. Both have identical settings (15 requests / 15 minutes).

Recommendation: Use the shared authLimiter from rateLimiter.ts.

Finding 6.5: Email Preferences Parsing Duplicated 4 Times (Medium)

File: /opt/development/recipicity/staging/recipicity-api/src/routes/users.ts

The pattern for parsing emailPreferences JSON appears in: - Lines 536-549 (GET /email-preferences) - Lines 570-594 (POST /email-preferences) - Lines 662-676 (GET /notification-settings) - Lines 786-798 (PUT /notification-settings)

Each instance repeats the same typeof === 'string' ? JSON.parse() : value pattern with try/catch fallback to defaults.

Recommendation: Extract to a parseEmailPreferences(raw: unknown) utility function.


7. Error Response Consistency

Finding 7.1: Inconsistent Error Response Shapes (Medium)

Different endpoints return errors in different formats:

// Format 1 (most routes):
{ error: 'Something went wrong' }

// Format 2 (auth.ts login):
{ error: 'Validation error', details: ['...'] }

// Format 3 (recipeImport, aiRecipes):
{ success: false, error: 'Something went wrong', details: [...] }

// Format 4 (notifications.ts):
{ success: true, message: 'Notification deleted' }
// vs
{ message: 'Comment deleted successfully' }

Frontend code must handle multiple error shapes. The success boolean is used by AI/import routes but not by any other route family.

Recommendation: Standardize on a single error envelope:

{ error: { message: string, code?: string, details?: unknown[] } }

Finding 7.2: Success Response Wrapper Inconsistency (Low)

Some endpoints return raw data:

return res.json(tags);  // tags.ts
return res.json(mapped);  // collections.ts GET /

Others wrap in an object:

return res.json({ users: usersWithFollowStatus, total, query });  // users.ts
return res.json({ mealPlans, pagination });  // mealplans.ts

And some include a message field:

return res.json({ message: 'Recipe liked successfully' });
return res.json({ success: true });

Recommendation: Standardize on { data: T, pagination?: P } for collections and { data: T } for single resources.

Finding 7.3: DELETE Responses Inconsistent (Low)

  • DELETE /api/collections/:id returns 204 No Content (correct REST practice)
  • DELETE /api/bookmarks/:recipeId returns 200 { message: '...' }
  • DELETE /api/notifications/:id returns 200 { success: true, message: '...' }
  • DELETE /api/comments/:id returns 200 { message: '...' }
  • DELETE /api/mealplans/:id returns 200 { message: '...' }

Recommendation: Pick one convention (either 204 with no body, or 200 with { message }) and use it consistently.


8. Service Layer Separation

Finding 8.1: Fat Route Handlers -- Business Logic in Routes (Critical)

The most significant architectural issue. Nearly all route files contain business logic directly in the handler functions instead of delegating to a service layer. Examples:

  • recipes.ts: 800+ line file with complex search query building, privacy filtering, tag management, nutrition calculation triggers, and usage tracking all inline
  • users.ts: 1,279 lines with avatar upload/delete, email preferences management, digest preview, notification settings -- all inline
  • grocerylists.ts: Ingredient aggregation, fraction math, categorization logic all in route handlers
  • auth.ts: Apple JWKS caching, OAuth user creation, password hashing, email sending all inline

The only properly extracted services are: - notification.service.ts (called from routes) - aiRecipeGeneration.service.ts / recipeImport.service.ai.ts (AI operations) - featureFlag.service.ts (feature gating)

Recommendation: Extract service classes for core domains: - RecipeService -- search, CRUD, privacy filtering, tag management - UserService -- profile, preferences, avatar, digest - MealPlanService -- plan CRUD, recipe management - GroceryListService -- list CRUD, ingredient aggregation

Finding 8.2: recipeImport.routes.ts Feature Check Duplication (High)

File: /opt/development/recipicity/staging/recipicity-api/src/routes/recipeImport.routes.ts

Every endpoint in this file repeats an identical 30-line block: 1. Fetch user with subscription: prisma.user.findUnique({ include: { subscription: { include: { plan: true } } } }) 2. Build userContext object 3. Check FeatureFlagService.isFeatureEnabled('recipe-import', userContext) 4. Check FeatureFlagService.checkUsageLimit(userId, 'recipes')

This appears in from-url (lines 150-194), preview (lines 325-359), from-preview (lines 469-515), from-image (lines 629-663), and from-text (lines 770-814) -- five times.

The same pattern appears in aiRecipes.routes.ts for AI recipe generation.

Recommendation: Create a middleware:

const requireFeature = (featureName: string) => async (req, res, next) => {
  const user = await prisma.user.findUnique({ ... });
  const access = await FeatureFlagService.isFeatureEnabled(featureName, userContext);
  if (!access.enabled) return res.status(403).json({ ... });
  req.featureContext = { user, userContext };
  next();
};

Finding 8.3: No Transaction Usage for Multi-Step Operations (Medium)

Several operations perform multiple database writes that should be atomic:

  • Bookmark creation (bookmarks.ts, lines 176-206): Creates a Bookmark record AND a RecipeCollectionItem record in separate queries. If the collection item creation fails, the bookmark exists without the corresponding collection entry.

  • Recipe creation (recipes.ts): Creates the recipe, then creates tag associations, then triggers nutrition calculation -- all as separate operations.

  • OAuth user creation (auth.ts, lines 1159-1169): Creates a user and then sends a welcome email. If the email function throws before the non-blocking catch, the user exists without the welcome email (minor, but illustrative).

Recommendation: Use prisma.$transaction() for operations that must succeed or fail together.

Finding 8.4: Admin Audit Middleware Logs Request Bodies (Medium)

File: /opt/development/recipicity/staging/recipicity-api/src/middleware/adminAudit.ts (lines 47-53)

details: {
  method: req.method,
  path: req.path,
  query: req.query,
  body: req.method !== 'GET' ? req.body : undefined,
  // ...
}

The entire request body (including potentially sensitive data like passwords if an admin endpoint accepted them) is stored in the audit log as JSONB. While admin endpoints likely do not accept passwords, this is a data minimization concern under GDPR/CCPA.

Recommendation: Redact sensitive fields from the body before logging, or only log a curated set of fields.


Summary of Findings by Severity

Severity Count Key Issues
Critical 3 N+1 queries in user search/tags (3.1, 3.2), legacy import userId trust (4.3), fat route handlers (8.1)
High 5 Duplicate asyncHandler (2.1), readiness check Redis leak (2.3), optionalAuth no active check (4.1), path-based upload auth (4.2), getUserId duplication (6.1), feature check duplication (8.2)
Medium 16 Duplicate comment routes (1.1), inline server routes (1.3), missing rate limiting (2.4), over-fetching (3.3), missing indexes (3.4), eager loading (3.5), no profile validation (5.1), notification settings injection (5.2), email prefs arbitrary JSON (5.3), no CUID validation (5.4), MinIO duplication (6.2), email prefs parsing duplication (6.5), error response inconsistency (7.1), no transactions (8.3), audit body logging (8.4)
Low 13 Route naming (1.2), search redirect (1.4), PUT/PATCH duplication (1.5), IP range detection (2.5), comment delete auth mismatch (4.4), invite code brute-force (4.5), password reset weakness (5.5), pagination duplication (6.3), rate limiter duplication (6.4), success wrapper inconsistency (7.2), delete response inconsistency (7.3)

  1. Fix the Critical Security Issue: Replace req.body.userId with req.user?.userId in the legacy /import endpoint (recipeImport.routes.ts line 946). This is a privilege escalation vulnerability that can be fixed in one line.

  2. Extract Service Layer: Start with RecipeService and UserService to pull business logic out of route handlers. This will make the N+1 fixes possible without route-level refactoring.

  3. Fix N+1 Query Patterns: Batch user-specific lookups in tags.ts and convert manual count loops in users.ts to use Prisma _count or raw SQL. These are the biggest performance bottlenecks.

  4. Consolidate Shared Utilities: Create utils/auth.ts (getUserId), lib/minio.ts (MinIO singleton), and utils/pagination.ts. Delete duplicate asyncHandler from errorHandler.ts. This reduces the maintenance surface area significantly.

  5. Standardize Error/Response Format: Define a consistent envelope for all API responses and migrate endpoints incrementally. This will simplify frontend error handling and reduce cross-team friction.