fix(api): JWT notifications auth + builtin kill-switch revert - #2805
fix(api): JWT notifications auth + builtin kill-switch revert#2805riderx wants to merge 6 commits into
Conversation
Management notification routes used API-key-only middleware, so the dashboard Bearer session failed with Invalid apikey when queuing silent update checks after promoting builtin. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
Comment |
Merging this PR will not alter performance
Comparing 
Footnotes
|
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Superseded by #2807, which includes this JWT/ |
Devices on an OTA bundle with a channel pointed at builtin need a success
{ version: 'builtin' } response (no error/kind) so the plugin can reset.
Already-on-builtin stays on the dedicated already_on_builtin path.
Co-authored-by: Cursor <cursoragent@cursor.com>
riderx
changed the title
plugin_parser maps version_name "builtin" to version_build, so compare version_name === version_build for the store-binary up-to-date case. Co-authored-by: Cursor <cursoragent@cursor.com>
Main changed countDevices to take a platform/updatedAt options object; update the private analytics unit expectation so merge CI stays green. Co-authored-by: Cursor <cursoragent@cursor.com>
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a8e30a7. Configure here.
| return updateError200(c, 'already_on_builtin', 'Already on builtin') | ||
| } | ||
| else { | ||
| return updateError200(c, 'already_on_builtin', 'Already on builtin', { |
There was a problem hiding this comment.
Matching OTA semver skips kill-switch
Medium Severity
For a built-in channel, already_on_builtin is returned when version_name === version_build. A device can still be running an OTA bundle while both fields report the same semver (common when bundle and native versions align). The kill-switch then returns already_on_builtin instead of { version: "builtin" }, so the plugin may not reset to the store binary.
Reviewed by Cursor Bugbot for commit a8e30a7. Configure here.
There was a problem hiding this comment.
Risk: high. Cursor Bugbot did not complete successfully (skipped) and left one unresolved medium finding on the builtin kill-switch path; this PR also changes notification auth and /updates semantics above the low-risk approval threshold. Human review is needed before merge.
Sent by Cursor Approval Agent: Pull Request Approver External
There was a problem hiding this comment.
Risk: high. Cursor Bugbot skipped as a check and left one unresolved medium finding on the builtin kill-switch path; this PR also changes notification auth and /updates semantics above the medium approval threshold. Human review is needed; reviewers were assigned.
Sent by Cursor Approval Agent: Pull Request Approver
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 5 files
Confidence score: 2/5
- In
supabase/functions/_backend/plugin_runtime/utils/update.ts, awaitingsendStatsAndDevicebefore returning the kill-switch/reset payload can cause the response to never be sent if that side effect fails, leaving devices stuck on OTA instead of rolling back to native — make stats/device reporting non-blocking or isolate failures so the reset response is always returned. - In
supabase/functions/_backend/plugin_runtime/utils/update.ts, the kill-switch conditionversion_name === version_buildcan misclassify devices that are still on an OTA bundle when semver matches the native build, which can trigger incorrect reset behavior and state tracking — tighten the check with an explicit runtime/source-of-version signal before deciding rollback state.
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="supabase/functions/_backend/plugin_runtime/utils/update.ts">
<violation number="1" location="supabase/functions/_backend/plugin_runtime/utils/update.ts:671">
P2: The kill-switch check `version_name === version_build` can also match a device that is still running an OTA bundle whose semver happens to equal the native build version (a common case when bundle and native versions are kept in sync). In that scenario the endpoint returns `already_on_builtin` instead of `{ version: 'builtin' }`, so the plugin never enters its reset-to-store-binary path even though the channel has reverted to built-in. Consider distinguishing “on OTA bundle” vs “on native binary” using a signal other than semver equality (e.g. whether a bundle id/version is actually installed) before short-circuiting to `already_on_builtin`.</violation>
<violation number="2" location="supabase/functions/_backend/plugin_runtime/utils/update.ts:675">
P1: Devices can remain on the OTA bundle when the stats/device side effect fails, because the awaited `sendStatsAndDevice` rejects before the reset response is sent. The kill-switch response is the mechanism that tells the plugin to restore the store binary, so this telemetry write should be best-effort or handled so the `{ version: 'builtin' }` response is still returned.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| }) | ||
| } | ||
| // Kill-switch: tell plugin to reset to the store binary (no error/kind). | ||
| await sendStatsAndDevice(c, device, [{ action: 'get', versionName: 'builtin' }]) |
There was a problem hiding this comment.
P1: Devices can remain on the OTA bundle when the stats/device side effect fails, because the awaited sendStatsAndDevice rejects before the reset response is sent. The kill-switch response is the mechanism that tells the plugin to restore the store binary, so this telemetry write should be best-effort or handled so the { version: 'builtin' } response is still returned.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/functions/_backend/plugin_runtime/utils/update.ts, line 675:
<comment>Devices can remain on the OTA bundle when the stats/device side effect fails, because the awaited `sendStatsAndDevice` rejects before the reset response is sent. The kill-switch response is the mechanism that tells the plugin to restore the store binary, so this telemetry write should be best-effort or handled so the `{ version: 'builtin' }` response is still returned.</comment>
<file context>
@@ -665,14 +665,15 @@ export async function updateWithPG(
- })
- }
+ // Kill-switch: tell plugin to reset to the store binary (no error/kind).
+ await sendStatsAndDevice(c, device, [{ action: 'get', versionName: 'builtin' }])
+ return c.json({ version: 'builtin' }, 200)
}
</file context>
| // plugin_parser rewrites version_name "builtin" -> version_build, so we cannot | ||
| // compare version_name === "builtin" here. Store binary => names match build; | ||
| // OTA bundle => version_name is the live bundle and differs from version_build. | ||
| if (version_name === version_build) { |
There was a problem hiding this comment.
P2: The kill-switch check version_name === version_build can also match a device that is still running an OTA bundle whose semver happens to equal the native build version (a common case when bundle and native versions are kept in sync). In that scenario the endpoint returns already_on_builtin instead of { version: 'builtin' }, so the plugin never enters its reset-to-store-binary path even though the channel has reverted to built-in. Consider distinguishing “on OTA bundle” vs “on native binary” using a signal other than semver equality (e.g. whether a bundle id/version is actually installed) before short-circuiting to already_on_builtin.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/functions/_backend/plugin_runtime/utils/update.ts, line 671:
<comment>The kill-switch check `version_name === version_build` can also match a device that is still running an OTA bundle whose semver happens to equal the native build version (a common case when bundle and native versions are kept in sync). In that scenario the endpoint returns `already_on_builtin` instead of `{ version: 'builtin' }`, so the plugin never enters its reset-to-store-binary path even though the channel has reverted to built-in. Consider distinguishing “on OTA bundle” vs “on native binary” using a signal other than semver equality (e.g. whether a bundle id/version is actually installed) before short-circuiting to `already_on_builtin`.</comment>
<file context>
@@ -665,14 +665,15 @@ export async function updateWithPG(
+ // plugin_parser rewrites version_name "builtin" -> version_build, so we cannot
+ // compare version_name === "builtin" here. Store binary => names match build;
+ // OTA bundle => version_name is the live bundle and differs from version_build.
+ if (version_name === version_build) {
return updateError200(c, 'already_on_builtin', 'Already on builtin')
}
</file context>
1 participant
Summary (AI generated)
middlewareKeytomiddlewareAuthso the dashboard JWT can call/notifications/update-check(and related routes) withoutInvalid apikey./updatesnow returns success{ version: "builtin" }with noerror/kind, so the plugin can reset to the store binary.already_on_builtin+kind: up_to_date.Motivation (AI generated)
A customer enabling silent push after reverting a channel to built-in hit two bugs:
/updatesalways returnedalready_on_builtin+kind: up_to_datefor built-in channels, so the plugin never entered itsversion == "builtin"reset path.Business Impact (AI generated)
Restores silent background update push from the console and the intended emergency rollback / kill-switch (channel → built-in).
Test Plan (AI generated)
/notifications/update-checkfrom the dashboard while logged in (JWT) and confirm it queues withoutInvalid apikey./updatesreturns{ "version": "builtin" }withouterror/kind.already_on_builtin/up_to_date.bunx vitest run tests/native-notifications-api.unit.test.tsGenerated with AI
Note
Medium Risk
Touches authenticated notification APIs and changes
/updatessuccess vs error semantics for built-in channels, which affects emergency rollbacks and console push-update flows.Overview
Notification management routes now use
middlewareAuthinstead ofmiddlewareKey, so the dashboard can call endpoints like/notifications/update-checkwith a session JWT while Capgo API keys still work.Built-in channel kill-switch on
/updatesis fixed for plugin ≥ 6.2.0: afterplugin_parserrewritesversion_name"builtin"toversion_build, the server treatsversion_name === version_buildas already on the store binary (already_on_builtin/up_to_date). Devices still running an OTA bundle get a success{ version: "builtin" }with noerrororkind, plus stats, so the plugin can reset to the store binary.Tests cover the new builtin responses, mock
middlewareAuthfor notifications, and aligncountDevicesexpectations with a platform/options filter object.Reviewed by Cursor Bugbot for commit a8e30a7. Bugbot is set up for automated code reviews on this repo. Configure here.