Skip to content

VPC_CODE_REVIEW_REPORT.md

Latest commit

 

History

History
196 lines (144 loc) · 5.84 KB

VPC_CODE_REVIEW_REPORT.md

File metadata and controls

196 lines (144 loc) · 5.84 KB
Raw

VPC Project Code Review Report

Generated by Nancy | March 11, 2026

Project Overview

VPC (Virtual PC Control) is a web-based OS-style VPS management dashboard with:

  • PostgreSQL management (DB)
  • Server monitoring
  • File gallery
  • API key management
  • Web hosting for projects
  • Terminal command execution

Tech Stack:

  • Backend: Node.js, Express, PostgreSQL (pg)
  • Frontend: React 18, Vite, TailwindCSS, Radix UI, Zustand
  • Process Manager: PM2
  • Reverse Proxy: Nginx

Issues Found

CRITICAL SECURITY ISSUES

1. Rate Limiting is Disabled (server.js:9)

// Rate limiters removed — private system, no throttling needed

Problem: Even for private systems, rate limiting prevents brute force attacks. The login endpoint has rate limiting defined in rateLimiter.js but the global limiter is completely disabled.

Risk: Brute force attacks on login, API abuse, DoS potential

2. SQL Injection Risk in Terminal Service (terminalService.js:210-227)

case 'sql': {
  const sql = parts.slice(2).join(' ').trim();
  // ... directly executes user-provided SQL
  const result = await projectPool.query(sql);
}

Problem: The vpc db <slug> sql <query> command allows executing arbitrary SQL on project databases. While this is behind auth, there's no query sanitization or restrictions.

Risk: Data deletion, data exfiltration, privilege escalation

3. Dynamic DB Query Command (terminalService.js:249-267)

async function handleDbQuery(command, pool, start) {
  const sql = command.replace('vpc db query ', '').trim();
  const result = await pool.query(sql);
}

Problem: Same issue - arbitrary SQL execution on the main VPC database.

4. Helmet Security Headers Disabled (server.js:14-19)

app.use(helmet({
  contentSecurityPolicy: false,
  crossOriginEmbedderPolicy: false,
  crossOriginOpenerPolicy: false,
  originAgentCluster: false,
}));

Problem: All major Helmet protections are disabled. CSP being off means XSS attacks are easier.

5. Potential Command Injection in Web Hosting (webHosting.js:125)

execSync(`git clone --depth 1 -b "${branch}" "${cloneUrl}" "${deployPath}"`, {...});

Problem: If branch or cloneUrl contain shell metacharacters, they could be exploited. The branch comes from project.git_branch which is user input.

Risk: Remote code execution

MEDIUM SECURITY ISSUES

6. JWT Secret Not Validated (jwt.js:3)

const SECRET = process.env.JWT_SECRET;

Problem: No check if JWT_SECRET is actually set. If undefined, JWT signing would fail or use an empty secret.

7. CORS Fully Open for DB API (server.js:31)

app.use('/api/db', cors());

Problem: The DB external API accepts requests from any origin. While it uses API key auth, this is still a wider attack surface.

8. Missing Input Validation (various routes)

  • Project names, slugs, and other inputs have minimal validation
  • File paths in web hosting could potentially be manipulated

CODE QUALITY ISSUES

9. Inconsistent Error Handling

Some routes return generic errors while others expose stack traces or internal messages.

10. app.js Listens Twice (app.js + server.js)

Both app.js:41 and server.js:99 call app.listen(). In production, only app.js should listen, but server.js also has a listen call that will conflict.

Fix: Remove app.listen() from server.js since app.js is the entry point.

11. Hardcoded Paths

  • /var/www/vpc hardcoded in deploy scripts
  • Various log paths hardcoded with fallbacks

12. Missing Connection Pool Cleanup

DB creates connection pools per project but there's no visible cleanup when projects are deleted.

FRONTEND ISSUES

13. Token Stored in localStorage (useAuthStore.js)

localStorage.setItem('vpc-token', data.token);

Problem: localStorage is vulnerable to XSS. For a VPS management tool with high privileges, httpOnly cookies would be more secure.

14. Client-Side Token Expiry Check (useAuthStore.js:52-53)

const payload = JSON.parse(atob(token.split('.')[1]));
if (payload.exp * 1000 < Date.now()) {

Problem: This can be bypassed by modifying the token. Server-side validation already exists, so this is minor, but the fallback behavior (line 66-72) grants { all: true } permissions if /me fails.

DEPLOYMENT ISSUES

15. No Automated Security Updates

Deploy script doesn't include system package updates.

16. No Health Check in PM2 Config

instances: 1,
autorestart: true,

Should add health_check_interval or listen_timeout for production.

Recommendations

High Priority

  1. Re-enable rate limiting, at least for public endpoints
  2. Restrict SQL execution capabilities or add query whitelisting
  3. Enable Helmet CSP and other security headers
  4. Sanitize inputs for shell commands (git clone, etc.)
  5. Move JWT token to httpOnly cookie

Medium Priority

  1. Add JWT_SECRET validation on startup
  2. Implement proper CORS for DB API
  3. Add input validation across all routes
  4. Fix double listen() issue in server.js
  5. Implement connection pool cleanup

Low Priority

  1. Standardize error handling
  2. Remove hardcoded paths, use env vars
  3. Add health checks to PM2 config
  4. Add automated security update checks

Summary

The VPC project is a solid foundation for a VPS management GUI. The architecture is clean with good separation between frontend/backend. However, there are several security issues that should be addressed before production deployment, especially around:

  1. SQL injection via terminal commands
  2. Command injection in git operations
  3. Disabled security headers
  4. Missing rate limiting

The code quality is generally good with proper async/await usage, modular structure, and TypeScript-ready patterns in the frontend.

Report generated by Nancy - Yash's AI