Files
shopify-ai-backup/security-review/SECURITY_AND_FUNCTIONALITY_REVIEW.md
Developer 2a7971eda1 Security fixes: Remove PAT, add idempotency, fix admin auth
- Remove exposed GitHub PAT from git remote URL
- Remove admin password plaintext fallback (bcrypt only)
- Add webhook idempotency protection to prevent duplicate payments
- Fix webhook error handling to return 500 on errors (enables retry)
- Upgrade archiver to v7 to fix npm vulnerabilities
- Add production environment validation for critical secrets
- Add comprehensive security review documentation
2026-02-20 21:51:45 +00:00

14 KiB

Security and Functionality Review

Plugin Compass AI App Builder - Pre-Launch Assessment

Review Date: February 20, 2026 Review Scope: Main chat application, builder, server, and payment systems Status: CRITICAL ISSUES FOUND - DO NOT LAUNCH WITHOUT REMEDIATION


Critical Security Issues (MUST FIX BEFORE LAUNCH)

1. EXPOSED GITHUB PERSONAL ACCESS TOKEN (CRITICAL)

Location: .git/config line 7 Severity: CRITICAL CVSS: 9.1 (Critical)

Issue:

url = https://southseact-3d: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@github.com/...

A GitHub Personal Access Token (PAT) is embedded directly in the git remote URL configuration. This token has been committed to the repository and provides full access to the GitHub account.

Impact:

  • Complete compromise of the GitHub account
  • Access to all repositories
  • Potential for supply chain attacks
  • Token may have already been scraped by automated tools

Remediation:

  1. IMMEDIATELY revoke the exposed token at https://github.com/settings/tokens
  2. Remove the token from git config:
    git remote set-url origin https://github.com/southseact-3d/shopify-ai-backup.git
    
  3. Force push to remove from history if necessary
  4. Use SSH keys or a credential manager instead
  5. Never commit credentials to version control

2. Admin Password Plaintext Fallback (HIGH)

Location: chat/server.js lines 15796-15802 Severity: HIGH

Issue:

// Validate password using bcrypt
let passwordValid = false;
if (adminPasswordHash) {
  passwordValid = await bcrypt.compare(pass, adminPasswordHash);
} else {
  // Fallback to plaintext comparison if hashing failed at startup
  passwordValid = pass === ADMIN_PASSWORD.trim();
}

The admin login has a fallback to plaintext password comparison when bcrypt hashing fails. This creates a timing attack vulnerability and undermines the security model.

Impact:

  • Timing attacks possible on plaintext comparison
  • Weaker authentication if bcrypt initialization fails silently
  • Admin account compromise potential

Remediation:

  1. Remove the plaintext fallback entirely
  2. Fail hard if bcrypt initialization fails
  3. Log and alert on any password comparison failures
  4. Add logging for bcrypt initialization status

3. Session Secret Auto-Generation Warning (MEDIUM)

Location: chat/server.js lines 377-385 Severity: MEDIUM

Issue:

const USER_SESSION_SECRET = process.env.USER_SESSION_SECRET || process.env.SESSION_SECRET || (() => {
  const generatedSecret = randomBytes(32).toString('hex');
  console.warn('⚠️  WARNING: No USER_SESSION_SECRET or SESSION_SECRET found. Generated a random secret...');
  return generatedSecret;
})();

The session secret is auto-generated if not provided. While there's a warning, if this happens in production:

  • All user sessions will be invalidated on server restart
  • An attacker who reads logs could potentially forge session tokens

Remediation:

  1. Make USER_SESSION_SECRET a required environment variable in production
  2. Exit the application if not set in production mode
  3. Never log generated secrets

4. Webhook Error Handling Returns 200 (MEDIUM)

Location: chat/server.js lines 14458-14461 Severity: MEDIUM

Issue:

} catch (error) {
  log('Dodo webhook error', { error: String(error), stack: error.stack });
  sendJson(res, 200, { received: true });
}

The webhook handler returns HTTP 200 even on errors, which prevents the payment provider from retrying. This could lead to:

  • Lost payment confirmations
  • Inconsistent subscription states
  • Customers charged but not credited

Remediation:

  1. Return appropriate HTTP error codes (400/500) for actual errors
  2. Only return 200 when processing is successful
  3. Implement proper retry handling for transient errors

High Priority Issues

5. No Idempotency Protection for Payments (HIGH)

Location: Payment handling functions throughout server.js Severity: HIGH

Issue: Payment processing lacks idempotency keys. If the same webhook is delivered multiple times (which payment providers do by design), the same payment could be processed multiple times.

Impact:

  • Customers could receive duplicate token credits
  • Revenue loss from over-crediting
  • Inconsistent account states

Remediation:

  1. Store processed webhook event IDs
  2. Check for duplicates before processing
  3. Implement idempotency keys for all payment operations

6. Payment Method Save Endpoint Logs Without Verification (MEDIUM)

Location: chat/server.js lines 14580-14582 Severity: MEDIUM

Issue:

if (inferredType === 'payment_method_save') {
  log('payment_method_save: payment method added via webhook', { userId, eventId: event.id, checkoutId });
  return;
}

Payment method save events are logged but not properly processed/verified.

Remediation:

  1. Verify the payment method save with the payment provider
  2. Update user record with payment method details
  3. Implement proper state tracking

7. Email Marketing Webhook Hardcoded (LOW)

Location: chat/server.js line 11441 Severity: LOW

Issue:

await fetch('https://emailmarketing.modelrailway3d.co.uk/api/webhooks/incoming/wh_0Z49zi_DGj4-lKJMOPO8-g', ...

A webhook URL with an apparent API key is hardcoded. This should be configurable.

Remediation:

  1. Move to environment variable
  2. Make configurable per environment

Functionality Issues

8. Builder State Not Properly Cleared on Logout

Location: chat/public/builder.js lines 52-66 Severity: MEDIUM

Issue: The builder state is persisted to localStorage but the clearBuilderState function preserves pluginPrompt and subsequentPrompt. When a user logs out, sensitive state could persist.

Remediation:

  1. Clear all builder state on logout
  2. Clear state when switching accounts
  3. Consider encrypting sensitive builder state

9. External Testing MCP Server Path

Location: chat/server.js lines 9640-9651 Severity: MEDIUM

Issue:

const wpMcpServerPath = path.resolve(__dirname, '../opencode/mcp-servers/wp-cli-testing/index.js');

This path is relative to the chat folder and may not exist in all deployments. No validation that the file exists before use.

Remediation:

  1. Validate file exists before enabling feature
  2. Add graceful degradation if file not found
  3. Log warning when feature is disabled due to missing file

10. Memory Management Could Cause Performance Issues

Location: chat/server.js lines 681-999 Severity: LOW

Issue: Memory cleanup runs every 5 minutes with debouncing to 60 seconds. For high-traffic deployments, this may not be sufficient.

Observations:

  • Session max age: 7 days
  • Message history limit: 50 messages
  • Stale stream timeout: 10 minutes

Recommendation:

  1. Consider making cleanup intervals configurable
  2. Add memory pressure-based cleanup triggers
  3. Implement connection pooling limits

11. Token Validation May Reject Valid Usage

Location: chat/server.js lines 7459-7499 Severity: LOW

Issue: Token validation has strict character-per-token ratio checking that might reject legitimate edge cases (very dense code, minified JS, etc.).

Recommendation:

  1. Widen acceptable character-per-token ratio bounds
  2. Add context-aware validation for code vs natural language

Payment System Issues

12. Dodo Product IDs Using Placeholder Values

Location: chat/server.js lines 254-276 Severity: HIGH

Issue:

starter_monthly_usd: process.env.DODO_STARTER_MONTHLY_USD || 'prod_starter_monthly_usd',

Default product IDs are placeholder strings. If environment variables aren't set, payment checkouts will fail silently or create invalid products.

Remediation:

  1. Validate all required product IDs on startup
  2. Exit application if payment product IDs not configured in production
  3. Add startup health check for payment configuration

13. Subscription Renewal Date Calculation

Location: Multiple locations Severity: MEDIUM

The renewal date calculation should be reviewed to ensure proper handling of:

  • Month-end dates (Feb 28/29)
  • Timezone consistency
  • Leap years

Authentication Issues

14. CSRF Token Implementation

Location: chat/server.js lines 1574, 1935-1946 Severity: MEDIUM

Issue: CSRF tokens are implemented but it's unclear if they're consistently validated on all state-changing requests.

Recommendation:

  1. Audit all POST/PUT/DELETE endpoints for CSRF validation
  2. Add CSRF token requirement to all forms
  3. Consider using SameSite=Strict cookies for session cookies

15. Password Requirements

Location: chat/server.js - password validation function Severity: LOW

The password validation appears reasonable but should be reviewed to ensure it meets current security standards:

  • Minimum 8 characters recommended
  • No common password blacklist
  • No breach database check

Recommendation:

  1. Implement password strength meter on frontend
  2. Add common password blacklist check
  3. Consider HaveIBeenPwned API integration

Builder Functionality Issues

16. Model Selection Lock for Free Plans

Location: chat/public/builder.js lines 1036-1090 Severity: LOW

Issue: Free plan users have model selection locked to 'auto'. The logic adds an 'auto' option and prevents selection. This UX could be confusing.

Recommendation:

  1. Show clear messaging about auto model selection
  2. Consider showing which model was actually used after request
  3. Add upgrade prompt when user attempts to select specific model

17. Message Input Caching

Location: chat/public/builder.js lines 447-508 Severity: LOW

Issue: Message input is cached in localStorage for 24 hours. If a user types sensitive information and doesn't send, it persists.

Recommendation:

  1. Clear cache on logout
  2. Add user notification about caching
  3. Consider shorter cache duration

Configuration Issues

18. Missing Environment Variable Documentation

Severity: MEDIUM

Issue: Many environment variables are required for proper operation but not documented in a single location.

Required Variables:

  • DODO_PAYMENTS_API_KEY - Payment processing
  • DODO_PAYMENTS_WEBHOOK_KEY - Webhook verification
  • DODO_TOPUP_*_USD/GBP/EUR - Product IDs (12 variables)
  • USER_SESSION_SECRET - Session security
  • ADMIN_USER / ADMIN_PASSWORD - Admin access
  • OPENROUTER_API_KEY - AI model access
  • Various model-specific API keys

Remediation:

  1. Create .env.example with all required variables
  2. Add startup validation for required variables
  3. Document in deployment guide

19. Docker Configuration Security

Location: docker-compose.yml (root) Severity: MEDIUM

Review Docker configuration for:

  • Container user (should not run as root)
  • Resource limits
  • Secret management
  • Network isolation

Recommendations Summary

Before Launch (MUST DO):

  1. Revoke exposed GitHub PAT immediately
  2. Remove plaintext password fallback
  3. Add idempotency to payment processing
  4. Validate all payment product IDs on startup
  5. Fix webhook error response codes
  6. Make session secret required in production

Soon After Launch:

  1. Audit all CSRF validation
  2. Add payment method save handling
  3. External MCP server path validation
  4. Environment variable documentation

Future Improvements:

  1. Password breach database check
  2. Memory management optimization
  3. Token validation edge cases
  4. Better free plan UX for model selection

Files Reviewed

File Lines Focus
chat/server.js ~19,500 Main server, auth, payments, API
chat/public/builder.js ~2,500 Builder frontend
chat/public/app.js ~300 Main app frontend
chat/security/index.js 20 Security module
chat/security/prompt-sanitizer.js 497 Prompt injection protection
chat/package.json 27 Dependencies
.git/config - Git configuration

Testing Recommendations

Before launch, perform:

  1. Penetration Testing

    • Authentication bypass attempts
    • Payment manipulation
    • Session hijacking
    • CSRF attacks
  2. Load Testing

    • Concurrent user limits
    • Payment processing under load
    • Memory leak detection
  3. Payment Flow Testing

    • Complete payment flows in test mode
    • Webhook delivery failures
    • Duplicate payment handling
    • Refund processing
  4. Security Scanning

    • Dependency vulnerability scan (npm audit)
    • Static code analysis
    • Secret detection in codebase

Dependency Vulnerabilities

20. npm Audit Findings (HIGH)

Severity: HIGH Status: Multiple high-severity vulnerabilities found

archiver - HIGH severity (via archiver-utils, readdir-glob, zip-stream)
archiver-utils - HIGH severity (via glob)
glob - HIGH severity (via minimatch)
minimatch - HIGH severity (ReDoS vulnerability GHSA-3ppc-4f35-3m26)

Remediation:

npm update archiver
# Or pin to secure version:
npm install archiver@4.0.2

21. Test Framework Dependency (LOW)

Severity: LOW Location: chat/package.json line 7

Issue:

"test": "bun test"

Tests are written for Bun test framework but Bun is not installed in the environment.

Impact:

  • Cannot run tests in production environment
  • CI/CD may fail

Remediation:

  1. Either install Bun in deployment environment
  2. Or migrate tests to use Node.js built-in test runner
  3. Add test runner to documentation

Conclusion

DO NOT LAUNCH until critical issues #1-4 are resolved. The exposed GitHub token (#1) is a severe security breach that could lead to complete account compromise. The payment-related issues (#4, #5) could result in revenue loss or customer disputes.

After remediation of critical and high-priority issues, the application should be suitable for launch with the noted improvements implemented over time.


This review was generated by automated analysis. Manual verification of all findings is recommended.