fix(billing): bind Checkout redirects to trusted configuration - #505
fix(billing): bind Checkout redirects to trusted configuration#505seonghobae wants to merge 34 commits into
Conversation
|
Warning Review limit reachedNext included review available in 3 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughStripe Checkout은 요청 authority 대신 운영자가 설정한 canonical public origin을 사용합니다. 시작 시 billing 설정과 Checkout URL을 검증합니다. 비활성화 상태는 HTTP 503으로 처리하며, mock Checkout은 개발 모드에서만 허용합니다. Provider 오류는 HTTP 502로 처리합니다. ChangesBilling Checkout
Planning Analysis Attribution
Estimated code review effort: 3 (Moderate) | ~30 minutes Merge Risk: 🟡 Moderate · up to The PR binds Checkout redirects to trusted configuration and fails closed on invalid provider responses, but the exact head is not merge-ready until the required CI/security gates and independent review are complete. Possibly related issues
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Client
participant Checkout API
participant Billing Configuration
participant Stripe
Client->>Checkout API: orgId와 Checkout 요청 전송
Checkout API->>Billing Configuration: billing mode와 publicOrigin 확인
alt billing disabled
Checkout API-->>Client: HTTP 503 billing_not_configured
else development mock
Checkout API-->>Client: canonical origin 기반 mock URL
else live Stripe
Checkout API->>Stripe: canonical success_url, cancel_url로 세션 생성
Stripe-->>Checkout API: 세션 URL 반환
Checkout API-->>Client: live Checkout 응답
end
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
@opencode-agent review Please submit a formal review for exact current head |
|
@coderabbitai review Please review exact current head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== checked-out commit =='
git rev-parse HEAD
echo '== requested head available =='
git cat-file -t 5477a34278444c23d40c38ba9278cb56de63b882
echo '== diff summary against protected base =='
git diff --stat 44e7903cf8891c65410f7fc6ca5144de3fdb5185 5477a34278444c23d40c38ba9278cb56de63b882
echo '== changed paths =='
git diff --name-status 44e7903cf8891c65410f7fc6ca5144de3fdb5185 5477a34278444c23d40c38ba9278cb56de63b882
echo '== implementation outlines =='
ast-grep outline server/billing.mjs --items all
ast-grep outline server/billing_configuration.mjs --items all
echo '== test outlines =='
ast-grep outline tests/unit/billing-checkout.test.mjs --items all
ast-grep outline tests/unit/billing-configuration.test.mjs --items all
echo '== direct request authority and checkout call sites =='
rg -n -C 3 --glob '*.mjs' '\b(createCheckout|SCOPEWEAVE_PUBLIC_ORIGIN|req(?:uest)?\.(?:headers|host)|Host|host|fetch\s*\()' server testsLength of output: 26348 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== billing implementation =='
nl -ba server/billing.mjs | sed -n '1,240p'
echo '== billing configuration =='
nl -ba server/billing_configuration.mjs | sed -n '1,180p'
echo '== checkout route =='
nl -ba server/app.mjs | sed -n '570,615p'
echo '== billing checkout unit tests =='
nl -ba tests/unit/billing-checkout.test.mjs | sed -n '1,310p'
echo '== billing configuration unit tests =='
nl -ba tests/unit/billing-configuration.test.mjs | sed -n '1,160p'
echo '== checkout API test =='
nl -ba tests/api/billing-checkout.test.mjs | sed -n '1,140p'
echo '== billing patch =='
git diff --find-renames --find-copies --unified=80 \
44e7903cf8891c65410f7fc6ca5144de3fdb5185 \
5477a34278444c23d40c38ba9278cb56de63b882 \
-- server/billing.mjs server/billing_configuration.mjs server/app.mjs package.json
echo '== checkout response and exception handling in app =='
rg -n -C 4 --glob '*.mjs' '(HTTPException|onError|errorHandler|createCheckout\s*\()' server/app.mjs serverLength of output: 1379 🏁 Script executed: #!/bin/bash
set -euo pipefail
show_range() {
local file="$1"
local start="$2"
local end="$3"
echo "== ${file}:${start}-${end} =="
sed -n "${start},${end}p" "$file" | awk -v n="$start" '{printf "%6d\t%s\n", n++, $0}'
}
show_range server/billing.mjs 1 240
show_range server/billing_configuration.mjs 1 180
show_range server/app.mjs 570 615
show_range tests/unit/billing-checkout.test.mjs 1 310
show_range tests/unit/billing-configuration.test.mjs 1 160
show_range tests/api/billing-checkout.test.mjs 1 140
echo '== billing patch =='
git diff --find-renames --find-copies --unified=80 \
44e7903cf8891c65410f7fc6ca5144de3fdb5185 \
5477a34278444c23d40c38ba9278cb56de63b882 \
-- server/billing.mjs server/billing_configuration.mjs server/app.mjs package.json
echo '== checkout response and exception handling in app =='
rg -n -C 4 --glob '*.mjs' '(HTTPException|onError|errorHandler|createCheckout\s*\()' server/app.mjs serverLength of output: 74509 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== billing documentation: transport, response validation, and checkout URL rules =='
rg -n -i -C 5 '(response|payload|body|size|limit|timeout|checkout.*url|host|hostname|redirect|webhook)' \
docs/billing-production.md docs/doctoring/stripe-checkout-trusted-origin.md CHANGELOG.md
echo '== Stripe and webhook server paths =='
rg -n -i -C 5 '(stripe|checkout|webhook|billing)' server
echo '== bounded provider-body implementations =='
rg -n -C 8 '(content-length|MAX_.*RESPONSE|response.*size|getReader\s*\(|arrayBuffer\s*\(|response\.json\s*\()' \
server tests/unit
echo '== route-level error middleware and application construction =='
sed -n '1,140p' server/app.mjs | awk '{printf "%6d\t%s\n", NR, $0}'
echo '== server bootstrap imports =='
fd -t f -e mjs . server | sort | xargs -r rg -n -C 3 'from .*app\.mjs|import\(.*app\.mjs|serve\s*\('
echo '== exact changed lines in app and relevant docs =='
git diff --unified=20 \
44e7903cf8891c65410f7fc6ca5144de3fdb5185 \
5477a34278444c23d40c38ba9278cb56de63b882 \
-- server/app.mjs docs/billing-production.md docs/doctoring/stripe-checkout-trusted-origin.mdLength of output: 95736
FindingP2 — Bound the successful Stripe response body before JSON parsing
Reject an oversized valid I did not infer approval.
|
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head mergeability evidence and changed-file flow before approval, then found merge conflicts on the affected path.
Findings
1. HIGH Merge Conflict Guidance - Resolve the PR branch against the latest base branch
- Problem: GitHub reports mergeStateStatus
DIRTYfor this pull request. - Root cause: Branch
feat/stripe-trusted-checkout-config-488cannot be merged cleanly intodevelop; the changed-file flow below shows which review/runtime path is blocked by the conflict. - Fix: Merge or rebase the latest
developintofeat/stripe-trusted-checkout-config-488, resolve conflict markers in the PR branch, rerun the focused checks, and push the same branch. - Repair commands:
gh pr checkout 505 --repo ContextualWisdomLab/scopeweave
git fetch origin develop
git merge --no-ff origin/develop # or: git rebase origin/develop
git status --short
# resolve files, then git add <resolved-files>
# merge path: git commit
# rebase path: git rebase --continue
git push origin HEAD:feat/stripe-trusted-checkout-config-488
# rebase path only: git push --force-with-lease origin HEAD:feat/stripe-trusted-checkout-config-488- Regression test: Keep OpenCode approval gated on mergeability so model-output failures cannot approve a conflicted PR.
Merge Conflict Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (4 files)"]
S1 --> I1["repository behavior"]
I1 --> Conflict["Merge conflict blocks this path"]
Conflict --> V1["required checks"]
Evidence --> S2["Docs (2 files)"]
S2 --> I2["operator or user guidance"]
I2 --> Conflict["Merge conflict blocks this path"]
Conflict --> V2["docs review"]
Evidence --> S3["Test (4 files)"]
S3 --> I3["regression suite"]
I3 --> Conflict["Merge conflict blocks this path"]
Conflict --> V3["targeted test run"]
- Result: REQUEST_CHANGES
- Reason: mergeStateStatus is
DIRTY; mergeable isCONFLICTING. - Head SHA:
5477a34278444c23d40c38ba9278cb56de63b882 - Workflow run: 32188770641
- Workflow attempt: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (4 files)"]
S1 --> I1["repository behavior"]
I1 --> Conflict["Merge conflict blocks this path"]
Conflict --> V1["required checks"]
Evidence --> S2["Docs (2 files)"]
S2 --> I2["operator or user guidance"]
I2 --> Conflict["Merge conflict blocks this path"]
Conflict --> V2["docs review"]
Evidence --> S3["Test (4 files)"]
S3 --> I3["regression suite"]
I3 --> Conflict["Merge conflict blocks this path"]
Conflict --> V3["targeted test run"]
OpenCode Review Overview
Pull request overviewOpenCode reviewed the current-head mergeability evidence and changed-file flow before approval, then found merge conflicts on the affected path. Findings1. HIGH Merge Conflict Guidance - Resolve the PR branch against the latest base branch
gh pr checkout 505 --repo ContextualWisdomLab/scopeweave
git fetch origin develop
git merge --no-ff origin/develop # or: git rebase origin/develop
git status --short
# resolve files, then git add <resolved-files>
# merge path: git commit
# rebase path: git rebase --continue
git push origin HEAD:feat/stripe-trusted-checkout-config-488
# rebase path only: git push --force-with-lease origin HEAD:feat/stripe-trusted-checkout-config-488
Merge Conflict Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (4 files)"]
S1 --> I1["repository behavior"]
I1 --> Conflict["Merge conflict blocks this path"]
Conflict --> V1["required checks"]
Evidence --> S2["Docs (2 files)"]
S2 --> I2["operator or user guidance"]
I2 --> Conflict["Merge conflict blocks this path"]
Conflict --> V2["docs review"]
Evidence --> S3["Test (4 files)"]
S3 --> I3["regression suite"]
I3 --> Conflict["Merge conflict blocks this path"]
Conflict --> V3["targeted test run"]
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (4 files)"]
S1 --> I1["repository behavior"]
I1 --> Conflict["Merge conflict blocks this path"]
Conflict --> V1["required checks"]
Evidence --> S2["Docs (2 files)"]
S2 --> I2["operator or user guidance"]
I2 --> Conflict["Merge conflict blocks this path"]
Conflict --> V2["docs review"]
Evidence --> S3["Test (4 files)"]
S3 --> I3["regression suite"]
I3 --> Conflict["Merge conflict blocks this path"]
Conflict --> V3["targeted test run"]
Merge Conflict Guidance
gh pr checkout 505 --repo ContextualWisdomLab/scopeweave
git fetch origin develop
git merge --no-ff origin/develop # or: git rebase origin/develop
git status --short
# resolve files, then git add <resolved-files>
# merge path: git commit
# rebase path: git rebase --continue
git push origin HEAD:feat/stripe-trusted-checkout-config-488
# rebase path only: git push --force-with-lease origin HEAD:feat/stripe-trusted-checkout-config-488 |
This review is bound to predecessor head 5477a34 and requested only merge-conflict repair. Current head b80d502 is mechanically mergeable, is zero-behind current protected develop 2c32887 with that commit as merge base, and the current exact-head repository-native Server Tests, Fuzz, Security Scan, Dependency Review, OSV Scanner, and Semgrep runs are terminal success. Dismissing the stale conflict verdict does not constitute approval; fresh independent current-head review and all live required checks remain required.
opencode-agent
Bot
added
priority: medium
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
1 participant
Buyer and security impact
ScopeWeave no longer lets inbound request authority become Stripe Checkout redirect authority, production no longer silently substitutes a successful billing mock when Stripe is absent, and the live provider path no longer treats Stripe error documents or malformed session payloads as successful Checkout. This is the root trusted-configuration/provider-response slice of #488; it intentionally does not close the subscription lifecycle issue.
Exact current scope
develop@2c328875e00e86537df3e965170be80532571cad;b80d502e659fa9c65a665f7b74fd52a460b91e54;feat/stripe-trusted-checkout-config-488;server/billing.mjs,server/billing_configuration.mjs, focused unit/API tests and smoke environment, package test registration, billing/doctoring docs, and additive CHANGELOG evidence;Any contributor-head or protected-base movement invalidates revision-sensitive evidence and requires fresh reconciliation.
Protected-base reconciliation
OpenCode correctly found predecessor
5477a34278444c23d40c38ba9278cb56de63b882conflicted with a newer protecteddevelop. An intermediate merge-parent repair made ancestry current while retaining stale semantic content, and fresh comparison exposed that it would delete or weaken protected contextual-orchestrator attribution/routing behavior. That intermediate state was rejected.Current head
b80d502...is the later non-destructive reconciliation that incorporates protecteddevelop@2c328875...while preserving the bounded billing slice and the protected Playwright update. The historical conflict review is already dismissed and does not govern this current tree.Trusted configuration and Checkout contract
STRIPE_SECRET_KEY,STRIPE_PRICE_ID, andSTRIPE_WEBHOOK_SECRETare an all-or-none live tuple;SCOPEWEAVE_PUBLIC_ORIGIN;URL, must be a root HTTPS origin, and rejects credentials, path, query, fragment, and unsupported schemes;SCOPEWEAVE_DEV=1plus a valid configured origin;striperuntime package;TDD and causal repairs
The initial regression imported an absent configuration module before implementation. Hosted tests later exposed an asynchronous assertion defect and an explicit-development smoke process missing the required loopback public origin; those harness/configuration defects were repaired without weakening production policy.
A subsequent source audit and CodeRabbit finding identified the undeclared runtime
stripeimport in the live path. The current production path instead uses the approved direct provider transport boundary, and the corresponding inline review thread is resolved. A later regression reproduced unsafe provider-response handling before status/transport/JSON/redirect validation and stable failure envelopes were strengthened.Current exact-head evidence
For exact head
b80d502e659fa9c65a665f7b74fd52a460b91e54, the currently enumerated repository-native runs are terminal GitHub-success:32383575121— success;32383575026— success;32383575090— success;32383575096— success;32383575209— success; and32383575464— success.CodeQL Requiredis absent from the current-head workflow enumeration and therefore remains non-passing, not implicitly successful. The organization-owned exact-head SAST/Security evidence defect remains tracked in the existing.github#1222owner lane; central green labels cannot become immutable merge authority until that protected repair integrates and downstream exact-head evidence is regenerated.The current inline review thread set is resolved. The visible Devin review found no issues on its reviewed state. There is no qualifying independent current-head/last-push approval, and model-only/comment-only review does not substitute for it.
Standards and remaining #488 lifecycle
docs/doctoring/stripe-checkout-trusted-origin.mdrecords current Stripe Checkout, HTTP error handling, idempotency, webhook, and WHATWG URL primary references while keeping protected-shipped truth separate from active-PR and follow-up behavior. This PR is not a production-complete subscription implementation. Remaining work includes durable Checkout-attempt/idempotency state, bounded provider-response lifecycle controls, audited reconciliation, raw-body webhook verification, durable event deduplication, monotonic subscription/entitlement state, recovery/privacy/incident acceptance, and protected release verification.Merge gate
Do not merge or enable auto-merge until the unchanged exact head satisfies every applicable live functional/browser/owned coverage/docstring, CodeQL/SAST, security/dependency/supply-chain, package/provenance, resolved-valid-thread, corrected organization-evidence, and qualifying independent-review gate on one unchanged head. Pending, queued, skipped-required, cancelled, absent, neutral-required, failed, stale, predecessor, synthetic-only, status-only, author-only, or model-only evidence is non-passing. Any head or protected-base movement requires fresh reconciliation.
Refs #488