Skip to content

Fix post-v1 logic and security review findings - #23

Merged
Phloraxx merged 1 commit into
mainfrom
post-v1-logic-review
Aug 2, 2026
Merged

Fix post-v1 logic and security review findings#23
Phloraxx merged 1 commit into
mainfrom
post-v1-logic-review

Conversation

@Phloraxx

@Phloraxx Phloraxx commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Why

A second independent post-v1 review found several issues that passed the original happy-path and production acceptance tests but matter under retries, delayed evidence, large retention backlogs, optional integrations and hostile redirects.

Correctness fixes

  • prevent a failed refund from being reactivated after a replacement refund has consumed the remaining refundable amount
  • include refund metadata in idempotency equality and reject same-status retries with contradictory bank references
  • enforce the payment fingerprint reuse boundary during audited manual matching
  • permit delayed reconciliation only when the bank transaction itself occurred within the original safe fingerprint window
  • allow a narrowly bounded two-second payment-creation tolerance for bank timestamps rounded to whole seconds
  • interpret timezone-less Indian bank statements in configurable STATEMENT_TIMEZONE (Asia/Kolkata by default)
  • process all expired retention rows in transaction-sized batches instead of at most 250 per daily run

Reliability/security fixes

  • actually start the durable operator-alert delivery worker
  • send a new external notification when an open alert escalates from warning to critical
  • refuse redirects for payment and operator-alert webhooks
  • keep structural config validation enabled in test mode
  • enforce the normal minimum length for the optional legacy webhook secret
  • report the operator-alert configuration accurately in Settings
  • add strict operator-SPA CSP/referrer/permissions/HSTS headers and return real 404s for unknown browser paths

Code cleanup

  • remove unused review candidate and webhook retry-count exports
  • replace the dashboard's inline capacity style with semantic <progress> markup so strict CSP remains possible
  • document statement timezone and timestamp precision behavior

Verification

  • frontend typecheck and production build
  • all Go tests and go vet
  • staticcheck
  • govulncheck: no reachable vulnerabilities
  • full go test -race -count=1 ./...
  • fresh production image build
  • fresh-volume acceptance: payment, second-precision SMS match, review creation, refund flow, backup verification, restore drill, task recreation and persistence

Phloraxx commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

The non-blocking maintainability findings from this review have been separated into issue #25. That issue covers API/router decomposition, state-machine complexity, shared durable-delivery mechanics, lifecycle integration tests and operator UI structure. None of those refactors are required for this correctness/security PR and they should be delivered incrementally after this patch is stable in production.

@Phloraxx
Phloraxx merged commit d93225a into main Aug 2, 2026
1 check passed
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