Fix payment-callback security, clean up Android, harden formula parser#19
Conversation
Expose parser rejection reasons through the quant view model/UI and cover invalid formula feedback with focused policy tests.
Payment callbacks now fail closed: verify_callback_signature requires a verified HMAC signature by default, so an unsigned callback can no longer forge a PAID event and grant VIP. The unsigned path needs an explicit QUANTTRADING_REQUIRE_CALLBACK_SIGNATURE=0 opt-out. sandbox_payment_callback runs under BEGIN IMMEDIATE so two concurrent callbacks for one order cannot both pass the status check and double-grant entitlements. Sessions: refresh tokens are single-use (rotated on refresh) and expire after 30 days; add POST /v1/auth/logout; prune dead sessions on login; index orders(user_id), orders(provider_transaction_id) and payment_callbacks(order_id). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Delete the core/ offline-first layer (OfflineFirstRepository, RepositoryResponse, CacheEntry, DataProvenance, ProviderHealthSnapshot, DataOrigin) and its test -- ~630 lines referenced only by each other, never wired into production code. Replace the local-account password hash: bare unsalted SHA-256 becomes salted PBKDF2-HMAC-SHA256 (120k iterations) in a new PasswordHasher, covered by PasswordHasherTest. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
README: drop the "v1.0.0 / waiting-for-store" and "complete
PIPL-compliant" claims that contradicted COMMERCIALIZATION_GAP.md, add
an explicit No-go list, and stop describing SQLite as "Postgres-ready".
SERVER_CONTRACT.md: correct paymentPayload to the real
{sandbox,orderId,channel,note} shape, add the callback timestamp field,
document POST /v1/auth/logout and refresh-token rotation, and fix the
access-token / refresh-response examples.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36e423ddc8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "SELECT * FROM sessions WHERE refresh_token = ? AND device_id = ?", | ||
| (refresh_token, device_id), | ||
| ).fetchone() |
There was a problem hiding this comment.
refresh_session reads the session row first and revokes it later, so two concurrent refresh calls with the same refresh_token/device_id can both pass the SELECT before either DELETE commits; the second DELETE becomes a no-op but still returns a new session, which breaks the intended single-use guarantee. This is exploitable specifically when a token is replayed in parallel (e.g., leaked token used by attacker and client at the same time).
Useful? React with 👍 / 👎.
| columns = {row["name"] for row in conn.execute("PRAGMA table_info(sessions)")} | ||
| if "refresh_expires_at" not in columns: | ||
| conn.execute( | ||
| "ALTER TABLE sessions ADD COLUMN refresh_expires_at INTEGER NOT NULL DEFAULT 0" |
There was a problem hiding this comment.
Adding refresh_expires_at with DEFAULT 0 causes all pre-migration session rows to look already expired (0 < now_millis()), so users with existing sessions will get 401 refresh token expired as soon as they need refresh after deploy. This creates an avoidable mass re-login event on upgrade; migration should initialize legacy rows to a non-expired value (or derived expiry) instead of zero.
Useful? React with 👍 / 👎.
| val matched = state.phone == phone && | ||
| state.passwordHash?.let { PasswordHasher.verify(password, it) } == true |
There was a problem hiding this comment.
Offline login now only validates passwordHash through PasswordHasher.verify, which expects the new salt:digest format; hashes created by previous app versions (plain SHA-256 string) will always fail verification, so existing local users cannot log in when backend auth is unavailable. This regression affects upgraded installs with preexisting local credentials unless legacy format checking/migration is retained.
Useful? React with 👍 / 👎.
#19) * fix(quant): surface formula validation errors Expose parser rejection reasons through the quant view model/UI and cover invalid formula feedback with focused policy tests. * fix: explain unsupported formula punctuation * fix: explain full-width formula operators * fix: explain reversed formula comparators * fix: explain percent formula literals * fix: explain full-width percent literals * fix: explain full-width digit literals * fix: explain full-width decimal literals * fix: explain full-width formula parentheses * fix: allow full-width formula spaces * fix(backend): harden payment callbacks and session lifecycle Payment callbacks now fail closed: verify_callback_signature requires a verified HMAC signature by default, so an unsigned callback can no longer forge a PAID event and grant VIP. The unsigned path needs an explicit QUANTTRADING_REQUIRE_CALLBACK_SIGNATURE=0 opt-out. sandbox_payment_callback runs under BEGIN IMMEDIATE so two concurrent callbacks for one order cannot both pass the status check and double-grant entitlements. Sessions: refresh tokens are single-use (rotated on refresh) and expire after 30 days; add POST /v1/auth/logout; prune dead sessions on login; index orders(user_id), orders(provider_transaction_id) and payment_callbacks(order_id). * refactor(android): remove dead offline layer, salt password hashes Delete the core/ offline-first layer (OfflineFirstRepository, RepositoryResponse, CacheEntry, DataProvenance, ProviderHealthSnapshot, DataOrigin) and its test -- ~630 lines referenced only by each other, never wired into production code. Replace the local-account password hash: bare unsalted SHA-256 becomes salted PBKDF2-HMAC-SHA256 (120k iterations) in a new PasswordHasher, covered by PasswordHasherTest. * docs: align README with gap docs, correct server contract README: drop the "v1.0.0 / waiting-for-store" and "complete PIPL-compliant" claims that contradicted COMMERCIALIZATION_GAP.md, add an explicit No-go list, and stop describing SQLite as "Postgres-ready". SERVER_CONTRACT.md: correct paymentPayload to the real {sandbox,orderId,channel,note} shape, add the callback timestamp field, document POST /v1/auth/logout and refresh-token rotation, and fix the access-token / refresh-response examples. ---------
2 participants
Summary
Two streams of work on this branch (13 commits off
main).Post-evaluation P0/P1/P2 fixes (
b6e567f,25674b6,36e423d)verify_callback_signaturerequires a verified HMAC signature by default, so an unsigned callback can no longer forge aPAIDevent and grant VIP (explicitQUANTTRADING_REQUIRE_CALLBACK_SIGNATURE=0opt-out for local/dev).sandbox_payment_callbackruns underBEGIN IMMEDIATE, so two concurrent callbacks for one order can no longer both pass the status check and double-grant entitlements.POST /v1/auth/logout; dead sessions pruned on login; indexes added onorders(user_id),orders(provider_transaction_id),payment_callbacks(order_id).core/offline-first layer (~630 lines, referenced only by itself + its test, never wired into production); replaced bare unsalted SHA-256 local password hashing with salted PBKDF2-HMAC-SHA256 (newPasswordHasher+PasswordHasherTest).README.mdno longer over-claims "v1.0.0 / ready to ship" or "complete PIPL-compliant" (now consistent withCOMMERCIALIZATION_GAP.md, with an explicit No-go list);SERVER_CONTRACT.mdcorrected to the real API (paymentPayloadschema, callbacktimestamp,/v1/auth/logout).Quant formula parser robustness (
87d7f7c…3ce9b36, 10 earlier commits)Test plan
cd backend && python3 -m pytest-> 46 passing. 8 new tests cover: fail-closed callback default, the explicit opt-out, concurrent-callback double-grant, refresh-token rotation, refresh-token expiry, logout, and the sessions schema migration.cd QuantTradingApp && ./gradlew :app:testDebugUnitTest— not run; the dev environment has no JVM. Needs verification (coversPasswordHasherTestand the formula parser tests)../gradlew :app:assembleDebug.🤖 Generated with Claude Code