8bb8954ff6
Analyze the NS App and document what it does: NMI card payments on Sales Invoices, AutoPay vaulting, multi-invoice settlement, guided Customer Quick Entry, and branded print formats. - docs/ARCHITECTURE.md — functional overview, component map, payment flow, data model, dependencies, config, known tech debt - docs/DESIGN_SPEC.md — goals, requirements, API contracts, failure modes, acceptance criteria - docs/SECURITY_NOTES.md — posture + hardening list - docs/README.md — docs index; link from root README Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
34 lines
2.2 KiB
Markdown
34 lines
2.2 KiB
Markdown
# NS App — Security Notes & Hardening
|
|
|
|
Companion to [ARCHITECTURE.md](./ARCHITECTURE.md) §9 and
|
|
[DESIGN_SPEC.md](./DESIGN_SPEC.md) §7.
|
|
|
|
## Current posture (as implemented)
|
|
|
|
- **PCI scope minimized.** Card number, expiry, and CVV are entered into
|
|
NMI-hosted Collect.js iframes. ERPNext receives only a single-use
|
|
`payment_token`. Raw card data never reaches the browser JS context or the
|
|
server.
|
|
- **No card storage.** Only the NMI Customer Vault ID is persisted on the
|
|
Customer (`custom_auto_pay_id`).
|
|
- **Secret handling.** `nmi_security_key` is read from `site_config.json`
|
|
(server-side) and sent only in server→NMI requests.
|
|
- **Transport.** All gateway calls use HTTPS to `secure.nmi.com`.
|
|
|
|
## Gaps & recommended hardening
|
|
|
|
| # | Issue | Recommendation |
|
|
|---|-------|----------------|
|
|
| 1 | **Unauthenticated webhook.** `crystalclear_webhook` is `allow_guest=True` and creates Payment Entries from any POST whose `orderid` matches an invoice. | Require a shared secret / HMAC signature; verify before writing. Optionally allow-list source IPs. |
|
|
| 2 | **Debug logging of gateway payloads.** `payments.py` writes request/response snippets via `frappe.log_error`. | Confirm no PAN/PII is ever logged; use a dedicated logger at an appropriate level; consider truncation/redaction and log retention limits. |
|
|
| 3 | **Hard-coded tokenization key** in `sales_invoice.js`. | Move to a server-provided value / site config; makes key rotation and per-environment keys possible. |
|
|
| 4 | **Hard-coded ledger account** `"ENB Bank Account - NIL"`. | Make company-aware via config or a Company-level custom field. |
|
|
| 5 | **Permissions.** Payment endpoints are whitelisted to any logged-in user. | Add role checks (as `create_customer_full` does with `frappe.only_for`) and/or rate limiting. |
|
|
| 6 | **`ignore_permissions=True`** on Payment Entry and Customer writes. | Acceptable for a system flow, but document the trust boundary and ensure the whitelisted entry points are themselves access-controlled. |
|
|
|
|
## Operational reminders
|
|
|
|
- Keep `enable_autopay_signup = 0` in production until vaulting is fully tested.
|
|
- Never commit `site_config.json` or the NMI security key to version control.
|
|
- Rotate the NMI security key and Collect.js key on any suspected exposure.
|