Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions supabase/functions/_backend/plugin_runtime/utils/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -665,14 +665,15 @@ export async function updateWithPG(
}
}
if (version.name === 'builtin' && greaterOrEqual(parse(plugin_version), parse('6.2.0'))) {
if (body.version_name === 'builtin' && version.name === 'builtin') {
// 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) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

return updateError200(c, 'already_on_builtin', 'Already on builtin')
}
else {
return updateError200(c, 'already_on_builtin', 'Already on builtin', {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a8e30a7. Configure here.

version: 'builtin',
})
}
// Kill-switch: tell plugin to reset to the store binary (no error/kind).
await sendStatsAndDevice(c, device, [{ action: 'get', versionName: 'builtin' }])

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

return c.json({ version: 'builtin' }, 200)
}
else if (version.name === 'builtin' && !greaterOrEqual(parse(plugin_version), parse('6.2.0'))) {
return updateError200(c, 'revert_to_builtin_plugin_version_too_old', 'revert_to_builtin used, but plugin version is too old')
Expand Down
26 changes: 13 additions & 13 deletions supabase/functions/_backend/public/notifications/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { NativeNotificationEvent, NativeNotificationPlatform, NativeNotific
import type { Permission } from '../../utils/rbac.ts'
import { sql } from 'drizzle-orm'
import { BRES, createHono, parseBody, quickError, simpleError, simpleRateLimit, useCors } from '../../utils/hono.ts'
import { middlewareKey } from '../../utils/hono_middleware.ts'
import { middlewareAuth } from '../../utils/hono_middleware.ts'
import {
createNotificationEventProof,
createNotificationIdentityProof,
Expand Down Expand Up @@ -682,15 +682,15 @@ app.post('/sync', async (c) => {
})
})

app.post('/recipients/proof', middlewareKey(), async (c) => {
app.post('/recipients/proof', middlewareAuth(), async (c) => {
const body = await parseBody<RecipientProofBody>(c)
const appId = assertString(body.appId, 'appId', 128)
const externalId = assertString(body.externalId, 'externalId', 512)
await assertAppPermission(c, NOTIFICATION_MANAGE_PERMISSION, appId)
return c.json({ identityProof: await createNotificationIdentityProof(c, appId, externalId) })
})

app.post('/recipients/lookup', middlewareKey(), async (c) => {
app.post('/recipients/lookup', middlewareAuth(), async (c) => {
const body = await parseBody<{ appId: string, externalId?: string, recipientKey?: string, limit?: number }>(c)
const appId = assertString(body.appId, 'appId', 128)
await assertAppPermission(c, NOTIFICATION_MANAGE_PERMISSION, appId)
Expand All @@ -701,18 +701,18 @@ app.post('/recipients/lookup', middlewareKey(), async (c) => {
return c.json({ recipientKey, devices: devices.map(publicDevice), count: devices.length })
})

app.get('/settings', middlewareKey(), async (c) => {
app.get('/settings', middlewareAuth(), async (c) => {
const appId = assertString(c.req.query('app_id'), 'app_id', 128)
await assertAppPermission(c, NOTIFICATION_MANAGE_PERMISSION, appId)
return c.json(await getNotificationSettings(c, appId))
})

app.put('/settings', middlewareKey(), async (c) => {
app.put('/settings', middlewareAuth(), async (c) => {
const body = await parseBody<SettingsBody>(c)
return c.json(await upsertNotificationSettings(c, body))
})

app.post('/badge', middlewareKey(), async (c) => {
app.post('/badge', middlewareAuth(), async (c) => {
const body = await parseBody<BadgeBody>(c)
const appId = assertString(body.appId, 'appId', 128)
await assertAppPermission(c, NOTIFICATION_MANAGE_PERMISSION, appId)
Expand Down Expand Up @@ -740,7 +740,7 @@ app.post('/badge', middlewareKey(), async (c) => {
return c.json({ ...BRES, campaignId, queued, queuedBuckets: plan.buckets.length, targeted: null, badgeRevision })
})

app.post('/update-check', middlewareKey(), async (c) => {
app.post('/update-check', middlewareAuth(), async (c) => {
const body = await parseBody<UpdateCheckBody>(c)
const appId = assertString(body.appId, 'appId', 128)
await assertAppPermission(c, NOTIFICATION_MANAGE_PERMISSION, appId)
Expand Down Expand Up @@ -792,7 +792,7 @@ app.post('/update-check', middlewareKey(), async (c) => {
return c.json({ ...BRES, campaignId, queued, queuedBuckets: plan.buckets.length, targeted: null })
})

app.post('/send', middlewareKey(), async (c) => {
app.post('/send', middlewareAuth(), async (c) => {
const body = await parseBody<SendBody>(c)
const appId = assertString(body.appId, 'appId', 128)
await assertAppPermission(c, NOTIFICATION_MANAGE_PERMISSION, appId)
Expand Down Expand Up @@ -828,7 +828,7 @@ app.post('/send', middlewareKey(), async (c) => {
return c.json({ ...BRES, campaignId, queued, queuedBuckets: plan.buckets.length, targeted: null })
})

app.get('/campaigns', middlewareKey(), async (c) => {
app.get('/campaigns', middlewareAuth(), async (c) => {
const appId = assertString(c.req.query('app_id'), 'app_id', 128)
await assertAppPermission(c, NOTIFICATION_MANAGE_PERMISSION, appId)
let pgClient: ReturnType<typeof getPgClient> | undefined
Expand All @@ -850,12 +850,12 @@ app.get('/campaigns', middlewareKey(), async (c) => {
}
})

app.post('/campaigns', middlewareKey(), async (c) => {
app.post('/campaigns', middlewareAuth(), async (c) => {
const body = await parseBody<CampaignBody>(c)
return c.json(await createCampaignRecord(c, body))
})

app.get('/stats', middlewareKey(), async (c) => {
app.get('/stats', middlewareAuth(), async (c) => {
const appId = assertString(c.req.query('app_id'), 'app_id', 128)
await assertAppPermission(c, NOTIFICATION_MANAGE_PERMISSION, appId)
const days = Number(c.req.query('days') ?? 30)
Expand All @@ -864,7 +864,7 @@ app.get('/stats', middlewareKey(), async (c) => {
return c.json({ data })
})

app.get('/providers', middlewareKey(), async (c) => {
app.get('/providers', middlewareAuth(), async (c) => {
const appId = assertString(c.req.query('app_id'), 'app_id', 128)
await assertAppPermission(c, NOTIFICATION_MANAGE_PERMISSION, appId)
let pgClient: ReturnType<typeof getPgClient> | undefined
Expand All @@ -885,7 +885,7 @@ app.get('/providers', middlewareKey(), async (c) => {
}
})

app.put('/providers', middlewareKey(), async (c) => {
app.put('/providers', middlewareAuth(), async (c) => {
const body = await parseBody<ProviderBody>(c)
const appId = assertString(body.appId, 'appId', 128)
await assertAppPermission(c, NOTIFICATION_MANAGE_PERMISSION, appId)
Expand Down
1 change: 1 addition & 0 deletions tests/native-notifications-api.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const {
}))

vi.mock('../supabase/functions/_backend/utils/hono_middleware.ts', () => ({
middlewareAuth: () => async (_c: unknown, next: () => Promise<void>) => next(),
middlewareKey: () => async (_c: unknown, next: () => Promise<void>) => next(),
middlewareV2: () => async (_c: unknown, next: () => Promise<void>) => next(),
}))
Expand Down
5 changes: 4 additions & 1 deletion tests/private-analytics-validation.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,10 @@ describe('private analytics route validation', () => {
[],
'2.0.0',
undefined,
'android',
{
platform: 'android',
updatedAt: { gt: undefined, lte: undefined },
},
)
})
})
23 changes: 22 additions & 1 deletion tests/updates.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -456,8 +456,29 @@ describe('[POST] /updates', () => {
expect(response.status).toBe(200)

const json = await response.json<UpdateRes>()
expect(json.error).toBe('already_on_builtin')
// Kill-switch: device on OTA must get success { version: 'builtin' } with no error/kind
// so the plugin can reset to the store binary.
expect(json.version).toBe('builtin')
expect(json.error).toBeUndefined()
expect(json.kind).toBeUndefined()

// Device on store binary: plugin sends version_name "builtin" (rewritten to
// version_build) or already reports version_name === version_build.
const alreadyOnBuiltin = getBaseData(APP_NAME_UPDATE)
alreadyOnBuiltin.version_name = 'builtin'
const upToDateResponse = await postUpdate(alreadyOnBuiltin)
expect(upToDateResponse.status).toBe(200)
const upToDateJson = await upToDateResponse.json<UpdateRes>()
expect(upToDateJson.error).toBe('already_on_builtin')
expect(upToDateJson.kind).toBe('up_to_date')

const alreadyOnNativeBuild = getBaseData(APP_NAME_UPDATE)
alreadyOnNativeBuild.version_name = alreadyOnNativeBuild.version_build
const nativeResponse = await postUpdate(alreadyOnNativeBuild)
expect(nativeResponse.status).toBe(200)
const nativeJson = await nativeResponse.json<UpdateRes>()
expect(nativeJson.error).toBe('already_on_builtin')
expect(nativeJson.kind).toBe('up_to_date')
}
finally {
await supabase
Expand Down
Loading