Skip to content

Security and code quality fixes for PR #217 review comments#250

Merged
rafiqul4 merged 3 commits intosubscriptionmodelfrom
copilot/sub-pr-217
Feb 16, 2026
Merged

Security and code quality fixes for PR #217 review comments#250
rafiqul4 merged 3 commits intosubscriptionmodelfrom
copilot/sub-pr-217

Conversation

Copy link
Contributor

Copilot AI commented Feb 16, 2026

Addresses actionable security vulnerabilities and code quality issues identified in PR #217 review. Implements production-hardening for authentication, webhooks, and test endpoints.

Security Fixes

Cron Authentication

  • Single Bearer token method with constant-time comparison prevents timing attacks
  • Removed dual-header bypass vulnerability (x-cron-secret OR authorization)

Webhook Signature Verification

  • SSLCommerz MD5 hash validation (MD5(transaction_id + store_password))
  • Returns 401 for invalid signatures before processing

PII Logging

  • All sensitive logging gated behind NODE_ENV === 'development'
  • Removed email/user data from production logs in signup and stores routes

Endpoint Protection

  • Demo/test endpoints return 403 in production (/api/demo/create-store, /api/subscriptions/init-trial)
  • Prevents database spam and subscription abuse

Credentials

  • .env.example uses placeholders (no real credentials)
  • Test scripts load from environment variables with validation
  • Added security warnings to script headers

Code Quality

Unused Imports

  • Removed: DialogTrigger, notifyGracePeriod, notifySubscriptionExpired, isValidTransition, Suspense
  • Commented out unused SmsChannel class

Error Handling

// Before: All errors returned 500
catch (error) {
  return NextResponse.json({ errors: { _form: [error.message] } }, { status: 500 });
}

// After: Proper status codes
catch (error) {
  if (error instanceof z.ZodError) {
    return NextResponse.json({ errors: { _form: error.errors.map(e => e.message) } }, { status: 400 });
  }
  return NextResponse.json({ errors: { _form: [error.message] } }, { status: 500 });
}

TypeScript

  • Fixed React.CSSProperties import (import type { CSSProperties } from 'react')
  • Removed node-fetch dependency (use built-in Node.js 18+ fetch)

Configuration

  • Webhook gateway default: 'sslcommerz' (only registered gateway)

Documentation

Added PR_217_REVIEW_FIXES_SUMMARY.md

  • Complete fix descriptions with code examples
  • Remaining architectural issues with recommendations:
    • Middleware subscription enforcement (route protection)
    • Store creation race condition (transaction refactoring needed)
    • x-store-id header trust (session-based validation needed)
  • Deployment checklists and security validation steps

Notes

x-store-id Security Warning
Added documentation to src/lib/subscription/middleware.ts clarifying that x-store-id header should only be used for internal/admin routes. User-facing routes must derive storeId from authenticated session.

Validation

  • Zero lint errors
  • All logging conditional on development environment
  • Production endpoints properly gated

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@vercel
Copy link

vercel bot commented Feb 16, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
stormcomui Ready Ready Preview, Comment Feb 16, 2026 1:07pm

@rafiqul4 rafiqul4 marked this pull request as ready for review February 16, 2026 13:01
Copilot AI review requested due to automatic review settings February 16, 2026 13:01
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.

Copilot AI and others added 2 commits February 16, 2026 13:02
- Remove unused imports (DialogTrigger, notifyGracePeriod, notifySubscriptionExpired, isValidTransition, Suspense)
- Comment out unused SmsChannel class in notification service
- Fix TypeScript React.CSSProperties import issue in admin subscriptions page
- Fix cron authentication to use single Bearer token method with constant-time comparison
- Add webhook signature verification to prevent spoofed callbacks
- Add security warning to subscription middleware about x-store-id header trust
- Gate PII logging behind NODE_ENV development checks in signup and stores routes
- Fix error handling to return 400 for validation errors (not 500)
- Protect demo and test endpoints with production environment checks
- Fix webhook gateway default from 'stripe' to 'sslcommerz'
- Update .env.example to use placeholder values instead of real credentials
- Secure test scripts by loading credentials from environment variables
- Remove node-fetch dependency from test scripts (use built-in Node.js fetch)

Co-authored-by: rafiqul4 <124497017+rafiqul4@users.noreply.github.com>
Add detailed documentation covering:
- All completed security and code quality fixes
- Remaining architectural issues with design recommendations
- Testing validation results
- Deployment recommendations and checklists
- Next steps for implementation

Co-authored-by: rafiqul4 <124497017+rafiqul4@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix critical bugs in subscription management system Security and code quality fixes for PR #217 review comments Feb 16, 2026
Copilot AI requested a review from rafiqul4 February 16, 2026 13:05
@rafiqul4 rafiqul4 merged commit 56918dd into subscriptionmodel Feb 16, 2026
2 checks passed
@rafiqul4 rafiqul4 deleted the copilot/sub-pr-217 branch February 16, 2026 13:07
@github-project-automation github-project-automation bot moved this from Backlog to Done in StormCom Feb 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants

Comments