sapiens-web2/backend-issues.md

Backend Issues - Code Review Report

🔴 Critical Issues (Immediate Fix Required)

1. Race Condition in Bid Placement - Data Corruption Risk

Severity: CRITICAL
Files Affected:

• server/storage.ts (placeBid method)
• server/routes.ts (POST /api/auctions/:id/bid)

Problem: Bid validation and database update happen in separate statements without transaction:

// storage.ts - placeBid method
async placeBid(auctionId: string, userId: string, amount: number, qualityScore: number) {
  const auction = await this.getAuctionById(auctionId);
  
  // ❌ RACE CONDITION: Another bid can come in here!
  if (parseFloat(auction.currentBid || "0") >= amount) {
    throw new Error("Bid must be higher than current bid");
  }
  
  // ❌ Two concurrent bids can both pass validation and overwrite each other
  await db.insert(bids).values({ /* ... */ });
  await db.update(auctions).set({ currentBid: amount.toString() });
}

Impact:

• Data corruption in live auctions
• Lower bid can win if timing is right
• Auction integrity compromised
• Financial implications for users
• Business logic violations

Reproduction Scenario:

1. Current bid is $100
2. User A submits $150 bid
3. User B submits $120 bid simultaneously
4. Both read currentBid as $100
5. Both pass validation
6. Whichever writes last wins (could be the $120 bid!)

Fix Solution:

// ✅ Fixed with transaction and SELECT FOR UPDATE
async placeBid(auctionId: string, userId: string, amount: number, qualityScore: number) {
  return await db.transaction(async (tx) => {
    // Lock the auction row to prevent concurrent updates
    const [auction] = await tx
      .select()
      .from(auctions)
      .where(eq(auctions.id, auctionId))
      .for('update'); // PostgreSQL row-level lock
    
    if (!auction) {
      throw new Error("Auction not found");
    }
    
    const currentBid = parseFloat(auction.currentBid || "0");
    if (currentBid >= amount) {
      throw new Error(`Bid must be higher than current bid of $${currentBid}`);
    }
    
    // Insert bid and update auction atomically
    const [newBid] = await tx.insert(bids).values({
      id: crypto.randomUUID(),
      auctionId,
      userId,
      amount: amount.toString(),
      qualityScore,
      createdAt: new Date()
    }).returning();
    
    await tx.update(auctions)
      .set({ 
        currentBid: amount.toString(),
        qualityScore,
        updatedAt: new Date()
      })
      .where(eq(auctions.id, auctionId));
    
    return newBid;
  });
}

Priority: P0 - Fix immediately before production

---

2. Incorrect HTTP Status Codes - Validation Errors Return 500

Severity: CRITICAL
Files Affected:

• server/routes.ts (multiple endpoints)

Problem: Zod validation errors and client errors return 500 (Internal Server Error):

app.post('/api/articles', async (req, res) => {
  try {
    const article = insertArticleSchema.parse(req.body); // Can throw ZodError
    // ...
  } catch (error) {
    // ❌ All errors become 500, even validation errors!
    res.status(500).json({ message: "Failed to create article" });
  }
});

Impact:

• Misleading error responses
• Client errors (400) appear as server errors (500)
• Difficult to debug for frontend
• Poor API design
• Monitoring alerts on client mistakes
• Can't distinguish real server errors

Affected Endpoints:

• POST /api/articles
• POST /api/auctions/:id/bid
• POST /api/media-outlets/:slug/auction/bids
• POST /api/media-outlet-requests
• PATCH /api/media-outlet-requests/:id
• And more...

Fix Solution:

// ✅ Fixed with proper error handling
import { ZodError } from "zod";

app.post('/api/articles', async (req, res) => {
  try {
    const article = insertArticleSchema.parse(req.body);
    // ... rest of logic
  } catch (error) {
    // Handle Zod validation errors
    if (error instanceof ZodError) {
      return res.status(400).json({ 
        message: "Invalid request data",
        errors: error.errors.map(e => ({
          field: e.path.join('.'),
          message: e.message
        }))
      });
    }
    
    // Handle known business logic errors
    if (error instanceof Error) {
      if (error.message.includes("not found")) {
        return res.status(404).json({ message: error.message });
      }
      if (error.message.includes("permission")) {
        return res.status(403).json({ message: error.message });
      }
    }
    
    // Genuine server errors
    console.error("Server error:", error);
    res.status(500).json({ message: "Internal server error" });
  }
});

Priority: P0 - Fix immediately

---

3. Request Status Updates Without Validation

Severity: CRITICAL
File: server/routes.ts

Problem: Status updates accept any string and don't verify affected rows:

app.patch('/api/media-outlet-requests/:id', async (req, res) => {
  const { status } = req.body; // ❌ No validation!
  const updated = await storage.updateMediaOutletRequest(req.params.id, { status });
  // ❌ Returns null without checking, client gets 200 with null!
  res.json(updated);
});

Impact:

• Invalid status values accepted
• Non-existent IDs return 200 OK with null
• Domain rules violated
• State machine bypassed

Fix Solution:

// Define allowed statuses
const allowedStatuses = ['pending', 'approved', 'rejected'] as const;

app.patch('/api/media-outlet-requests/:id', isAuthenticated, async (req: any, res) => {
  try {
    const { status } = req.body;
    
    // Validate status
    if (!allowedStatuses.includes(status)) {
      return res.status(400).json({ 
        message: `Invalid status. Must be one of: ${allowedStatuses.join(', ')}` 
      });
    }
    
    // Check permissions
    const user = req.user;
    if (user.role !== 'admin' && user.role !== 'superadmin') {
      return res.status(403).json({ message: "Insufficient permissions" });
    }
    
    const updated = await storage.updateMediaOutletRequest(req.params.id, { status });
    
    // Check if request exists
    if (!updated) {
      return res.status(404).json({ message: "Request not found" });
    }
    
    res.json(updated);
  } catch (error) {
    console.error("Error updating request:", error);
    res.status(500).json({ message: "Failed to update request" });
  }
});

Priority: P0 - Fix immediately

---

🟠 High Priority Issues

4. Missing Authentication on Sensitive Endpoints

Severity: HIGH
Files Affected: server/routes.ts

Problem: Some endpoints should require authentication but don't:

// Anyone can create articles!
app.post('/api/articles', async (req, res) => {
  // ❌ No isAuthenticated middleware!
});

// Anyone can see all prediction bets
app.get('/api/prediction-bets', async (req, res) => {
  // ❌ No authentication check!
});

Impact:

• Unauthorized content creation
• Data exposure
• Spam and abuse potential

Fix Solution:

// Add authentication middleware
app.post('/api/articles', isAuthenticated, async (req: any, res) => {
  // Now req.user is available
});

// Check role for admin actions
app.post('/api/media-outlets', isAuthenticated, async (req: any, res) => {
  if (req.user.role !== 'admin' && req.user.role !== 'superadmin') {
    return res.status(403).json({ message: "Insufficient permissions" });
  }
  // ...
});

Priority: P1 - Add before production

---

5. No Input Sanitization - Potential XSS

Severity: HIGH
Files Affected: Multiple routes

Problem: User input stored directly without sanitization:

const article = insertArticleSchema.parse(req.body);
await storage.createArticle({
  ...article,
  content: req.body.content // ❌ No sanitization!
});

Impact:

• XSS attacks possible
• Script injection
• Data integrity issues

Fix Solution:

import DOMPurify from 'isomorphic-dompurify';

const article = insertArticleSchema.parse(req.body);
await storage.createArticle({
  ...article,
  content: DOMPurify.sanitize(req.body.content),
  title: DOMPurify.sanitize(req.body.title)
});

Priority: P1 - Fix before production

---

🟡 Medium Priority Issues

6. Incomplete Error Logging

Severity: MEDIUM
Files Affected: Multiple routes

Problem: Errors logged inconsistently:

catch (error) {
  console.error("Error:", error); // Minimal context
  res.status(500).json({ message: "Failed" });
}

Impact:

• Difficult debugging
• Lost context
• Can't trace issues

Fix Solution:

catch (error) {
  console.error("Failed to create article:", {
    error: error instanceof Error ? error.message : error,
    stack: error instanceof Error ? error.stack : undefined,
    userId: req.user?.id,
    timestamp: new Date().toISOString(),
    body: req.body
  });
  res.status(500).json({ message: "Failed to create article" });
}

Priority: P2 - Improve observability

---

7. No Rate Limiting

Severity: MEDIUM
Files Affected: All routes

Problem: No rate limiting on any endpoint:

// Anyone can spam requests!
app.post('/api/bids', async (req, res) => {
  // No rate limiting
});

Impact:

• DoS vulnerability
• Resource exhaustion
• Abuse potential

Fix Solution:

import rateLimit from 'express-rate-limit';

const apiLimiter = rateLimit({
  windowMs: 15 * 60 * 1000, // 15 minutes
  max: 100, // Limit each IP to 100 requests per window
  message: 'Too many requests, please try again later'
});

app.use('/api/', apiLimiter);

// Stricter limits for sensitive operations
const bidLimiter = rateLimit({
  windowMs: 60 * 1000, // 1 minute
  max: 5, // 5 bids per minute
});

app.post('/api/auctions/:id/bid', bidLimiter, isAuthenticated, async (req, res) => {
  // ...
});

Priority: P2 - Add before production

---

🟢 Low Priority Issues

8. Inconsistent Response Formats

Severity: LOW
Files Affected: Multiple routes

Problem: Success and error responses have different structures:

// Success
res.json(data);

// Error
res.json({ message: "Error" });

// Sometimes
res.json({ error: "Error" });

Impact:

• Confusing for frontend
• Inconsistent parsing

Fix Solution: Standardize response format:

// Standard success
res.json({ 
  success: true, 
  data: result 
});

// Standard error
res.json({ 
  success: false, 
  error: { 
    message: "...", 
    code: "ERROR_CODE" 
  } 
});

Priority: P3 - Polish

---

9. Missing Request Validation Middleware

Severity: LOW
Files Affected: All routes

Problem: Schema validation repeated in every route:

app.post('/api/articles', async (req, res) => {
  const article = insertArticleSchema.parse(req.body); // Repeated pattern
});

Impact:

• Code duplication
• Maintenance burden

Fix Solution:

// Create reusable middleware
const validateBody = (schema: z.ZodSchema) => {
  return (req: Request, res: Response, next: NextFunction) => {
    try {
      req.body = schema.parse(req.body);
      next();
    } catch (error) {
      if (error instanceof ZodError) {
        return res.status(400).json({
          message: "Validation failed",
          errors: error.errors
        });
      }
      next(error);
    }
  };
};

// Use it
app.post('/api/articles', 
  validateBody(insertArticleSchema), 
  async (req, res) => {
    // req.body is already validated
  }
);

Priority: P3 - Refactor

---

Summary

Severity	Count	Must Fix Before Production
🔴 Critical	3	✅ Yes - Blocking
🟠 High	2	✅ Yes - Important
🟡 Medium	2	⚠️ Recommended
🟢 Low	2	❌ Nice to have

Total Issues: 9

Critical Path to Production:

1. ✅ Fix race condition in bidding (P0)
2. ✅ Fix HTTP status codes (P0)
3. ✅ Add request validation (P0)
4. ✅ Add authentication checks (P1)
5. ✅ Add input sanitization (P1)
6. ⚠️ Add rate limiting (P2)
7. ⚠️ Improve error logging (P2)

Estimated Fix Time:

• Critical issues: 4-6 hours
• High priority: 3-4 hours
• Medium priority: 4-6 hours
• Low priority: 4-6 hours

Recommended Testing:

• Load test bid placement with concurrent requests
• Test all error scenarios for proper status codes
• Verify authentication on all protected routes
• Test input sanitization with XSS payloads