Skip to content

fix security issues and bugs found during code review#89

Open
YashejShah wants to merge 4 commits into
razorpay:mainfrom
YashejShah:fix/security-and-bug-fixes
Open

fix security issues and bugs found during code review#89
YashejShah wants to merge 4 commits into
razorpay:mainfrom
YashejShah:fix/security-and-bug-fixes

Conversation

@YashejShah

Copy link
Copy Markdown

what i found

went through the codebase and found some security + reliability issues worth fixing.

security

  • ssrf bypass in url validationstrings.Contains(host, "razorpay.com") can be bypassed with domains like razorpay.com.evil.com. switched to proper suffix matching with isRazorpayURL() that checks exact match or .razorpay.com suffix
  • sensitive data in logs — full payment request/response bodies were being logged at INFO level via the MCP hooks. added redactSensitiveFields() that strips out card numbers, tokens, bank account details, cvv etc before anything hits the logs
  • credentials in process args — api key/secret were passed as CLI flags in the Dockerfile entrypoint, making them visible via ps aux. removed the --key/--secret flags from ENTRYPOINT since viper already picks up RAZORPAY_KEY_ID/RAZORPAY_KEY_SECRET from env vars
  • no startup validation — server would happily start with empty credentials and fail silently on first API call. added early validation in the stdio command that exits with a clear error message
  • log file permissions — log files were created with 0666 (world-readable). changed to 0600

bugs & improvements

  • expand[] parameter dropping valuesValidateAndAddExpand was overwriting the same map key in a loop, so only the last expand value survived. fixed to accumulate into a slice
  • fabricated email addresses — when no email was provided, the code was generating fake emails like 9876543210@mcp.razorpay.com. removed this and made email properly optional
  • type assertion safety — added comma-ok pattern to type assertions across payments, orders, and payment_links to prevent panics on unexpected data
  • razorpay ID format validation — added validateRazorpayID() helper and applied it to payment_id and order_id extraction to catch malformed IDs early
  • unused json import — removed the forced var _ = json.Marshal and its import from integrations/helpers.go
  • added .env and coverage.out to .gitignore
  • removed committed coverage.out from tracking

tested locally, all existing tests pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant