Repository navigation
fix: harden auth, secrets at rest, payments, and webhooks - #1
Merged
Merged
Conversation
The REST and gRPC authorization caches were keyed by ApiTokenId alone, so once a valid key warmed an entry, any secret carrying that same token id reused it and skipped Argon2 verification entirely. Both transports now go through one verification component keyed by a SHA-256 digest of the decoded token id and the presented secret. Unknown token and wrong secret collapse to one Unauthorized result so the response cannot confirm a guessed token id. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011WiwhEunTESEw8qgKPHnpk
Connector secrets and OAuth PKCE verifiers were encrypted with ChaCha20-Poly1305 using the first 12 bytes of the encryption key as the nonce, so every record under a key shared one nonce. That leaks plaintext XOR across records and reuses the Poly1305 key, which also permits forgery. New ciphertext is written as mtr1:<nonce hex>:<ciphertext hex> with a nonce from the OS CSPRNG. Decryption still reads the legacy bare-hex format so a startup migration can rewrite existing rows; it runs before any listener opens, in one locked transaction, and fails the boot rather than commit a partial rewrite. Re-encryption does not un-expose values readable in an older database snapshot, so the migration logs a warning to rotate the affected provider credentials. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011WiwhEunTESEw8qgKPHnpk
calculate_slot_upgrade_amount already returns the prorated total for the whole seat delta, but create_slot_upgrade_invoice_internal treated it as a per-unit rate and multiplied by delta again. preview_slot_update does not, so a 5-seat upgrade quoted $27.42 was invoiced $137.10. Bills the prorated total directly, derives the display unit price from it, and rejects a non-positive delta where the invariant is relied on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011WiwhEunTESEw8qgKPHnpk
The metering ingestion listener cached authorization by token id, the same defect just fixed on the meteroid API. A forged secret carrying a known token id rode a warm entry and could write usage events into another tenant's meters. Reuses the shared credential fingerprint, which now takes a plain Uuid so both binaries reach it without converting id types. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011WiwhEunTESEw8qgKPHnpk
Portal invoice payment and every checkout path took payment_method_id straight from the request and resolved it with a tenant-only lookup, so any customer of a tenant could charge another customer's stored card while settling their own invoice. Adds get_by_id_for_customer and puts the payer in the signatures of create_payment_intent and charge_payment_method_directly, so a caller cannot omit it rather than relying on a check per call site. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011WiwhEunTESEw8qgKPHnpk
process_payment_intent_events ignored its connector argument and resolved the transaction from provider metadata, so a tenant could settle another tenant's invoice with an event signed by their own webhook secret. The setup-intent handler had the same shape, trusting a metadata customer id. Payment intents now must name the signing connector's tenant and must match the transaction's amount and currency. Setup intents resolve the connection under that tenant and take the customer from the row instead of the metadata. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011WiwhEunTESEw8qgKPHnpk
CouponStatusRowPatch::patch matched on id alone, so any tenant could archive, disable, or re-enable another tenant's coupon and read it back from the RETURNING clause. tenant_id is declared part of the primary key, so AsChangeset drops it from the statement and the struct field looked like it was doing work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011WiwhEunTESEw8qgKPHnpk
The public inbound webhook route skipped signature verification entirely when a connector had no sensitive data, and connect_stripe stored whatever webhook_secret it was given. Only the browser form required a non-empty value, so a connector created over gRPC could leave the route forgeable by anyone. The route now rejects a connector without usable Stripe secret material, verify_webhook refuses an empty key rather than HMAC-ing with it, and Store::connect_stripe requires a whsec_ secret. The webhook test signs its payload and asserts an unsigned delivery is rejected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011WiwhEunTESEw8qgKPHnpk
h2 0.4.14 lets an unauthenticated peer hold a connection open with unbounded empty DATA frames (RUSTSEC-2026-0258); it serves the public gRPC listeners via tonic. Bumped to 0.4.18. Also silences RUSTSEC-2026-0235: rkyv reaches Cargo.lock only through an optional rust_decimal feature that nothing here enables, so it is never built. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011WiwhEunTESEw8qgKPHnpk
nwilkens
changed the title
Binding the charged payment method to the payer added an 8th argument, one past the clippy threshold. Matches how the rest of this crate handles the lint rather than wrapping a single-call-site private helper in a struct. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011WiwhEunTESEw8qgKPHnpk
The step hard-failed on every run in this fork because DOCKER_HUB_USERNAME and DOCKER_HUB_PASSWORD are not set here, and it skipped the test step with it, so no PR was ever actually tested. It only existed to pull testcontainers images authenticated; anonymous pulls work, subject to Docker Hub rate limits. docker.yaml keeps its logins for publishing, already guarded by a -n check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011WiwhEunTESEw8qgKPHnpk
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.
Hardening across authentication, secret storage at rest, payment method handling, and inbound webhook verification, plus one dependency advisory bump.
Nine fixes, one per commit. Each carries a regression test that was verified to fail against the previous behavior. Rationale is in the individual commit messages.
Verification
cargo test --workspace --liband the 288-test integration suite pass.cargo auditexits clean.Deployment
The secret-storage change needs a coordinated restart: values written by this build cannot be read by the previous binary, and a startup migration rewrites existing rows before listeners open. Rollback requires the pre-migration backup. Procedure and rationale are in
docs/superpowers/specs/2026-08-19-critical-billing-security-design.md.Operators should rotate stored provider credentials after upgrading.
Scope
Further work is identified but deliberately not in this PR, to keep each change reviewable on its own.
馃 Generated with Claude Code
https://claude.ai/code/session_011WiwhEunTESEw8qgKPHnpk