Signed BAA's - #2137
Conversation
|
🚨 Contributor flagged. Click here for more info: Superagent Dashboard |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (!user.stripeCustomerId) { | ||
| throw new Error( | ||
| "Your organization needs an active Cap Pro subscription before adding the Signed BAA add-on.", | ||
| ); | ||
| } | ||
| const customerId = user.stripeCustomerId; |
There was a problem hiding this comment.
When a former Pro owner retains their Stripe identifiers after subscription deletion, this check treats them as eligible and creates a $99 Signed BAA subscription without an active Cap Pro plan. This violates the add-on's base-plan requirement and allows an agreement to become active independently of Pro.
How this was verified: The deletion webhook preserves both Stripe identifiers while this action never checks stripeSubscriptionStatus or userIsPro before creating the subscription.
Knowledge Base Used: Web App (apps/web)
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/actions/organization/signed-baa.ts
Line: 220-225
Comment:
**Active Pro entitlement bypassed**
When a former Pro owner retains their Stripe identifiers after subscription deletion, this check treats them as eligible and creates a $99 Signed BAA subscription without an active Cap Pro plan. This violates the add-on's base-plan requirement and allows an agreement to become active independently of Pro.
**How this was verified:** The deletion webhook preserves both Stripe identifiers while this action never checks `stripeSubscriptionStatus` or `userIsPro` before creating the subscription.
**Knowledge Base Used:** [Web App (apps/web)](https://app.greptile.com/cap/-/custom-context/knowledge-base/capsoftware/cap/-/docs/web-app.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| } catch (error) { | ||
| await revertToPending(); |
There was a problem hiding this comment.
If Stripe creates the subscription but its response is lost or times out, this catch resets the row to pending without recording the subscription ID and reports that no charge occurred. The webhook only reconciles rows already keyed by that ID, so the customer remains billed while the dashboard provides neither an active BAA nor the signed document.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/actions/organization/signed-baa.ts
Line: 341-342
Comment:
**Ambiguous Stripe success is orphaned**
If Stripe creates the subscription but its response is lost or times out, this catch resets the row to pending without recording the subscription ID and reports that no charge occurred. The webhook only reconciles rows already keyed by that ID, so the customer remains billed while the dashboard provides neither an active BAA nor the signed document.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if (subscription.cancel_at_period_end) { | ||
| await stripe().subscriptions.update(subscription.id, { | ||
| cancel_at_period_end: false, | ||
| }); | ||
| } |
There was a problem hiding this comment.
When a canceling customer changes their seat count and the subsequent Stripe update or local persistence fails, this first update has already cleared cancel_at_period_end with no compensation. The requested change fails, but the subscription silently resumes at its old quantity and the client initially continues showing stale cancellation state.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/actions/organization/update-seat-quantity.ts
Line: 199-203
Comment:
**Failed seat change resumes billing**
When a canceling customer changes their seat count and the subsequent Stripe update or local persistence fails, this first update has already cleared `cancel_at_period_end` with no compensation. The requested change fails, but the subscription silently resumes at its old quantity and the client initially continues showing stale cancellation state.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| const { signatureDataUrl, ...contractFields } = details; | ||
| const recordFields = { ...contractFields, signatureData: signatureDataUrl }; | ||
|
|
||
| // Claim the record so concurrent submissions can never both reach Stripe. |
There was a problem hiding this comment.
This comment restates the processing-row claim expressed by the adjacent update, and the same narrative pattern appears throughout the new billing and webhook code. Repository guidance reserves comments for non-obvious investigative context; duplicating the implementation adds maintenance burden and creates prose that can drift from the behavior.
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/actions/organization/signed-baa.ts
Line: 237-240
Comment:
**Narrative comments duplicate implementation**
This comment restates the processing-row claim expressed by the adjacent update, and the same narrative pattern appears throughout the new billing and webhook code. Repository guidance reserves comments for non-obvious investigative context; duplicating the implementation adds maintenance burden and creates prose that can drift from the behavior.
**Context Used:** AGENTS.md ([source](https://github.com/capsoftware/cap/blob/main/AGENTS.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Add the ability for a user to purchase a signed BAA from Organization Settings
Greptile Summary
Adds organization-level Signed BAA purchasing, signing, PDF generation, email delivery, persistence, download support, and Stripe webhook synchronization. It also adds end-of-period Pro cancellation through seat management and a new security/compliance settings page.
signed_baasdatabase record and Stripe add-on subscription flow.Confidence Score: 1/5
The PR is not safe to merge until active-Pro eligibility, ambiguous Stripe creation recovery, and scheduled-cancellation partial failures are corrected.
The current flow can sell a BAA without the required Pro entitlement, leave a paying customer without an active local agreement after an ambiguous Stripe response, and silently revoke a scheduled Pro cancellation when a later seat update fails.
Files Needing Attention: apps/web/actions/organization/signed-baa.ts, apps/web/actions/organization/update-seat-quantity.ts, apps/web/app/api/webhooks/stripe/route.ts
Security Review
The purchase action does not enforce active Cap Pro status server-side. Because canceled users retain their Stripe identifiers, a former Pro owner can purchase the add-on despite the required base entitlement.
Important Files Changed
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat(billing): support cancel-to-zero fl..." | Re-trigger Greptile
Context used (3)
packages/database)