feat: wallet-ownership proof, rate limiting, per-intent payment addresses, and direct image upload - #203
Merged
Conversation
…sses, and direct image upload Tier 1 security hardening plus the Cloudinary upload feature, rebased onto current main (this work predated main's Render-deploy compile fixes and had been sitting uncommitted locally): - Wallet-ownership proof: POST /v1/wallets now requires a signed ownership challenge (GET /v1/wallets/challenge) alongside public_key, so registering a wallet requires proving control of the private key rather than just knowing a public address. octo_wallet_core::verify_account_signature added for the ed25519 verification; verify_hs256 widened to pub(crate) for the challenge HMAC check from routes/wallets.rs. - Rate limiting: crates/api/src/rate_limit.rs, applied to signup, login, and payment-link intent creation. - Exact payment-intent matching: migration 0015 gives each payment intent its own deposit address (previously shared per-link), so concurrent payers on the same link can no longer be cross-matched. Ingest's confirm_payment_link tries the exact per-intent address first, falling back to the old oldest-pending-by-link-address path for pre-migration intents, and now rejects underpayment. crates/store: pending_payment_by_address, get_payment_link. - Direct-to-Cloudinary image upload: crates/api/src/routes/uploads.rs signs the upload request; the API never touches file bytes. - migration 0016: ingest last-polled tracking. - Test coverage: crates/api/tests/common/ (shared signed_challenge / wallet_body helpers), plus new tests for the challenge flow, rate limiting, and concurrent per-intent address allocation. Rebase notes: resolved conflicts against main's own Render-deploy fixes (Claims.jti, verify_hs256 visibility, the custodial withdraw/ trustline 410 tombstones) by keeping both sides' intent. Several test helpers across authz_matrix_tests.rs and malformed_body_tests.rs still built wallet-creation requests without the new challenge/ signature fields (pre-dating this feature in those files) and needed updating to the challenge-signing helper to match. Verified: cargo test/clippy/fmt --workspace all clean; a release build of the server binary succeeds.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This is the Tier 1 security-hardening work plus the Cloudinary image-upload feature — it had been sitting uncommitted in the local working directory (pre-dating the Render-deploy compile fixes already merged into
main), which is whyGET /v1/uploads/signaturewas 404ing in production: the route existed only locally and had never reachedmain.POST /v1/walletsnow requires a signed ownership challenge (GET /v1/wallets/challenge) alongsidepublic_key— registering a wallet requires proving control of the private key, not just knowing a public address.octo_wallet_core::verify_account_signatureadded for ed25519 verification.crates/api/src/rate_limit.rs, applied to signup, login, and payment-link intent creation.0015gives each payment intent its own deposit address (previously shared per-link), so concurrent payers on the same link can no longer be cross-matched. Ingest tries the exact per-intent address first, falls back to the old oldest-pending-by-link-address path for pre-migration intents, and now rejects underpayment (stays pending instead of confirming for less than owed).crates/api/src/routes/uploads.rssigns the upload request server-side; the API never touches file bytes. This is the route the frontend's payment-link image upload depends on.0016: ingest last-polled tracking.crates/api/tests/common/) plus coverage for the challenge flow, rate limiting, and concurrent per-intent address allocation.Rebase notes
This was rebased onto current
main(which had drifted 42 commits since this work started). Resolved conflicts againstmain's own Render-deploy fixes by keeping both sides' intent — e.g.Claims.jti,verify_hs256visibility, and the custodial withdraw/trustline410tombstones all came frommain; the per-intent address matching and challenge-signing logic came from this branch. A few test helpers (authz_matrix_tests.rs,malformed_body_tests.rs) built wallet-creation requests without the new challenge/signature fields — pre-dating this feature reaching those files — and needed updating to use the new sharedcommon::wallet_bodyhelper.Test plan
cargo test --workspace --locked— all green (37 test binaries, 0 failures)cargo clippy --workspace --all-targets --locked -- -D warnings— cleancargo fmt --all -- --check— cleancargo build --release -p octo-server— succeeds