fix: harden payment replay and purchase guardrails#47
Conversation
Prevent payment tool parts from being replayed as executable calls, add intent/state-based policy overrides to block unsafe purchases, and persist payment tool state for safer status-first continuity across provider switches. Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 23 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/api/chat/intent/payment-intent.ts">
<violation number="1" location="app/api/chat/intent/payment-intent.ts:55">
P1: Substring matching can misclassify intent (e.g., "goodbye" includes "buy") and accidentally treat non‑purchase messages as `new_purchase`. Use word/phrase boundaries to avoid false positives for safety-critical intent classification.</violation>
</file>
<file name="lib/config.ts">
<violation number="1" location="lib/config.ts:315">
P2: Validate PAYMENT_GUARDRAIL_MODE so only "observe" or "enforce" are accepted; otherwise fall back to "observe". The current type assertion doesn’t prevent invalid env values from propagating.</violation>
</file>
<file name="lib/tools/capability-policy.ts">
<violation number="1" location="lib/tools/capability-policy.ts:351">
P1: Bug: observe mode returns `null` instead of a loggable override, so this case is silently dropped. The route handler (`app/api/chat/route.ts:703`) gates logging on `if (policyOverrides)`, meaning the "unknown intent + active job" scenario in observe mode produces zero telemetry — contradicting the comment's stated intent. Return an override object without `denyTools` so the route can log it.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryThis PR implements a comprehensive payment guardrail system to prevent accidental purchase re-execution during message replay and edit/resend operations. Key Changes:
Issues Found:
Architecture Notes: Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User Message] --> B[Route: Classify Intent]
B --> C{Intent Class?}
C -->|status_check| D[Policy: Deny pay_purchase]
C -->|new_purchase| E{Has Active Job?}
C -->|unknown| F{Has Active Job?}
E -->|Yes| D
E -->|No| G[Policy: Allow Both Tools]
F -->|Yes| H{Guardrail Mode?}
F -->|No| G
H -->|enforce| D
H -->|observe| G
D --> I[Remove pay_purchase from ToolSet]
G --> J[Keep All Tools]
I --> K[Model Generation]
J --> K
K --> L{Tool Called?}
L -->|pay_purchase| M{isPurchaseBlocked?}
L -->|pay_status| N[Execute Status Check]
M -->|true| O[Throw Error]
M -->|false| P[Execute Purchase]
P --> Q[Upsert chatToolState]
N --> R[Update Status in chatToolState]
Q --> S[Set activePurchaseJobId]
R --> T{isTerminal?}
T -->|Yes| U[Clear activePurchaseJobId]
T -->|No| V[Keep activePurchaseJobId]
style D fill:#ffcccc
style O fill:#ffcccc
style Q fill:#ccffcc
style R fill:#ccffcc
Last reviewed commit: 35fd30b |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Improve payment intent keyword matching to avoid substring collisions, preserve payment context in replay fallback summaries, and harden canonical state/backfill handling to reduce cross-job and stale-state errors. Made-with: Cursor
Avoid forcing an in-progress status label when replay context lacks a terminal signal, and include chat version/state mutation metadata for payment tool observability. Also clear canonical chat tool state when clearing chat history and cover fallback behavior with a regression test. Made-with: Cursor
Keep payment chat state synchronized with pay_purchase/pay_status tool outputs and clear stale state when edits truncate history, preventing outdated job context from surviving resend flows. Made-with: Cursor
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="convex/messages.ts">
<violation number="1" location="convex/messages.ts:290">
P2: Backfilled payment state (chatVersion 0 with no sourceMessageTimestamp) will never be cleared by this truncation fallback, leaving stale payment state after edits. Consider treating chatVersion 0 as unknown and deleting it when any truncation occurs.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const versionWasTruncated = | ||
| toDelete.length > 0 && toolState.chatVersion >= truncationMinVersion |
There was a problem hiding this comment.
P2: Backfilled payment state (chatVersion 0 with no sourceMessageTimestamp) will never be cleared by this truncation fallback, leaving stale payment state after edits. Consider treating chatVersion 0 as unknown and deleting it when any truncation occurs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At convex/messages.ts, line 290:
<comment>Backfilled payment state (chatVersion 0 with no sourceMessageTimestamp) will never be cleared by this truncation fallback, leaving stale payment state after edits. Consider treating chatVersion 0 as unknown and deleting it when any truncation occurs.</comment>
<file context>
@@ -277,11 +281,16 @@ export const deleteFromTimestamp = mutation({
+ typeof toolState.sourceMessageTimestamp === "number" &&
toolState.sourceMessageTimestamp >= timestamp
- ) {
+ const versionWasTruncated =
+ toDelete.length > 0 && toolState.chatVersion >= truncationMinVersion
+
</file context>
| const versionWasTruncated = | |
| toDelete.length > 0 && toolState.chatVersion >= truncationMinVersion | |
| const versionWasTruncated = | |
| toDelete.length > 0 && | |
| (toolState.chatVersion === 0 || | |
| toolState.chatVersion >= truncationMinVersion) |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| const rowId = await ctx.db.insert("chatToolState", { | ||
| chatId, | ||
| userId: user._id, | ||
| chatVersion: 0, // Backfilled, not from a real version |
There was a problem hiding this comment.
Backfilled chatToolState rows immune to edit truncation
Medium Severity
Backfilled chatToolState rows use chatVersion: 0 and sourceMessageTimestamp: undefined, making them immune to both truncation mechanisms during message edits. The server-side check toolState.chatVersion >= truncationMinVersion evaluates to 0 >= ≥1 (false), and sourceTimestampWasTruncated is false since the timestamp is undefined. The client-side truncateFromVersion has the same blind spot. This directly contradicts the code comment stating backfilled rows "don't survive an edit branch," and violates the plan invariant that edit truncation must not leave stale payment state active. A stale backfilled row with activePurchaseJobId set would incorrectly block new purchases after an edit.
Additional Locations (2)
| confidence: "high", | ||
| reason: "explicit_purchase_intent", | ||
| } | ||
| } |
There was a problem hiding this comment.
Classifier case ordering shadows status-keyword-only fallback
Low Severity
The check at line 112 (hasPurchaseKeyword && !hasActiveJob) is evaluated before the check at line 121 (hasStatusKeyword && !hasActiveJob && !hasAnyJob), making the latter unreachable when both keyword types are present. A message like "What is the status of my order?" with no existing jobs matches both "status" and "order" keywords, so it's classified as new_purchase (high confidence) instead of unknown. The test suite avoids this case by using messages without overlapping keywords.
Additional Locations (1)
1 participant
Summary
Test plan
Made with Cursor
Note
High Risk
Touches payment side-effect execution by adding intent-based tool suppression, execution-time blocking, and a new Convex
chatToolStateledger with backfill; misclassification or state/version bugs could block valid purchases or fail to prevent side effects. Feature flags reduce blast radius but rollout/backfill paths still need careful review.Overview
Prevents payment side effects from being triggered by replay/edit/model-switch flows by adding a canonical per-chat payment state ledger (
chatToolState) with idempotent upserts, version-based staleness rejection, and best-effort cleanup on message truncation/clear.Introduces server-side payment intent classification and request-scoped policy overrides to remove
pay_purchaseonstatus_check/active-job turns (plus a defense-in-depthisPurchaseBlockedexecution guard), and adds optional lazy backfill fromtoolCallLogfor legacy chats.Hardens replay by marking payment tools as explicitly non-replayable, extracting
jobId/url/statusintoplatformToolContext, and compiling them into deterministic continuity text (not tool calls); expands regression tests and adds observability fields/flags (PAYMENT_*) includingtoolCallLogmetadata (chatVersion,toolKey,stateMutationKey).Written by Cursor Bugbot for commit f680add. This will update automatically on new commits. Configure here.
Summary by cubic
Prevents accidental purchases during status checks and stops payment replay from causing side effects. Adds a canonical per-chat payment state synced to tool outputs, propagates chatVersion for safe edits/resends, and truncates stale state on edit and clear.
New Features
Migration
Written for commit f680add. Summary will update on new commits.