[security] fix(payment): enforce ownership on Stripe account refresh#6840
[security] fix(payment): enforce ownership on Stripe account refresh#6840Hinotoi-agent wants to merge 2 commits intoBasedHardware:mainfrom
Conversation
Greptile SummaryThis PR fixes a broken object-level authorization (IDOR) on Confidence Score: 5/5Safe to merge — the security fix is correct and the only remaining findings are P2 style suggestions in the test file. The core change is a narrow, correct IDOR fix with no schema, migration, or webhook changes. All open findings are P2 (test cleanup pattern and hardcoded test secret), neither of which affects runtime correctness or the security boundary being enforced. backend/tests/unit/test_stripe_connect_account_refresh_authz.py — minor test hygiene issues (no-cleanup patching, hardcoded secret value) Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant Endpoint as POST /v1/stripe/refresh/{account_id}
participant DB as get_stripe_connect_account_id(uid)
participant Stripe as refresh_connect_account_link
Note over Client,Stripe: After fix (this PR)
Client->>Endpoint: uid (from auth token) + account_id (path param)
Endpoint->>DB: lookup stored account for uid
DB-->>Endpoint: expected_account_id
alt No stored account
Endpoint-->>Client: 404 Connect account not found
else account_id != expected_account_id
Endpoint-->>Client: 403 Not authorized
else account_id == expected_account_id
Endpoint->>Stripe: refresh_connect_account_link(expected_account_id)
Stripe-->>Endpoint: account link
Endpoint-->>Client: 200 account link
end
Reviews (1): Last reviewed commit: "fix(payment): enforce ownership on Strip..." | Re-trigger Greptile |
|
Checked the Greptile findings against current head No further code change appears needed:
The Greptile summary looks stale relative to the current branch head, and the individual review threads appear resolved. Please re-review/re-run if anything still looks open. |
|
Hey @Hinotoi-agent 👋 Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request. After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:
Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out. Thank you for being part of the Omi community! 💜 |
|
thanks for your compute and tokens btw unfortunately bots are not welcome here yet things might change but not now :) |
|
Thanks for the update and information. |
Summary
This PR fixes an authenticated authorization flaw in the Stripe Connect account-link refresh flow.
/v1/stripe/refresh/{account_id}previously accepted any path-supplied Stripe Connect account id after user authenticationaccount_idbelonged to the authenticated userSecurity issue covered
Before this PR
POST /v1/stripe/refresh/{account_id}trusted the caller-suppliedaccount_idrefresh_connect_account_link(account_id)uidAfter this PR
get_stripe_connect_account_id(uid)404403Why this matters
The vulnerable route was a broken object-level authorization path in a payment control plane.
A normal authenticated user should only ever be able to refresh the onboarding link for the Stripe Connect account attached to their own user record. Before this patch, the route accepted an arbitrary path account id and created a fresh Stripe account link for it.
Attack flow
Affected code
backend/routers/payment.pybackend/database/users.pybackend/tests/unit/test_stripe_connect_account_refresh_authz.pyRoot cause
CVSS assessment
CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:L/I:L/A:NRationale:
Safe reproduction steps
POST /v1/stripe/refresh/{victim_account_id}{victim_account_id}toget_stripe_connect_account_id(attacker_uid)403Expected vulnerable behavior
Before this PR, the route would accept a caller-supplied Stripe Connect account id and return:
account_id: <victim account id>url: <fresh Stripe account link>without verifying that the account belonged to the authenticated caller.
Changes in this PR
refresh_account_link_endpoint403refresh_connect_account_linkFiles changed
backend/routers/payment.pybackend/tests/unit/test_stripe_connect_account_refresh_authz.pyMaintainer impact
Fix rationale
The backend already has a canonical ownership source for Stripe Connect accounts:
get_stripe_connect_account_id(uid).The safest pattern is to treat that server-side value as authoritative and fail closed on any mismatch, rather than trusting a client-controlled path parameter for a payment control-plane action.
Type of change
Test plan
git diff --checkExecuted:
cd backend /Users/lennon/.hermes/hermes-agent/venv/bin/python3.11 -m pytest tests/unit/test_stripe_connect_account_refresh_authz.py -qObserved result:
Disclosure notes