diff --git a/cli/src/api/app.ts b/cli/src/api/app.ts index 7d3b6d6174..5f49b618d4 100644 --- a/cli/src/api/app.ts +++ b/cli/src/api/app.ts @@ -2,6 +2,7 @@ import type { SupabaseClient } from '@supabase/supabase-js' import type { Database } from '../types/supabase.types' import { log } from '@clack/prompts' import { buildCliRequestHeaders } from '../analytics/cli-headers' +import { CliUserError } from '../shared/cli-user-error' import { appAddHintMessage, formatCapgoApiErrorBody, getCapgoCliHttpStatus, hasCliPermission, invokeCapgoCliApi, isCapgoManagedSupabaseHost, resolveCapgoPublicApiHost, show2FADeniedError } from '../utils' export async function checkAppExists( @@ -217,10 +218,12 @@ export async function checkAppExistsAndHasPermissionOrgErr( } if (!(await hasCliPermission(supabase, apikey, requiredPermissionKey, { appId: appid, channelId: channelId ?? null }))) { - const msg = `Insufficient permissions for app ${appid}. Required RBAC permission for this action: ${requiredPermissionKey}.` if (!silent) - log.error(msg) - throw new Error(msg) + log.error(`Insufficient permissions for app ${appid}. Required RBAC permission for this action: ${requiredPermissionKey}.`) + // Keep the app id OUT of the CliUserError message so error tracking does not + // fingerprint one issue per app; it goes in context. The permission key is a + // small bounded enum, so it stays in the message (as in currentBundle.ts). + throw new CliUserError(`Insufficient permissions for app. Required RBAC permission for this action: ${requiredPermissionKey}.`, { appId: appid }) } return true diff --git a/cli/src/utils.ts b/cli/src/utils.ts index 45eedfb8dd..5aca2c2fa9 100644 --- a/cli/src/utils.ts +++ b/cli/src/utils.ts @@ -2051,7 +2051,10 @@ export async function resolveUserIdFromApiKey(supabase: SupabaseClient if (!userId) { if (!silent) log.error(`Capgo authentication failed: invalid Capgo API key or insufficient Capgo permissions.`) - throw new Error('Capgo authentication failed: invalid Capgo API key or insufficient Capgo permissions.') + // Throw a CliUserError so error tracking skips this by type: a bad or + // missing API key is an expected user-configuration failure, not a crash. + // Type classification stays true even if this wording changes later. + throw new CliUserError('Capgo authentication failed: invalid Capgo API key or insufficient Capgo permissions.') } return userId } diff --git a/cli/test/test-posthog-exception.mjs b/cli/test/test-posthog-exception.mjs index ed890c0d93..4c5b7a83fe 100644 --- a/cli/test/test-posthog-exception.mjs +++ b/cli/test/test-posthog-exception.mjs @@ -217,6 +217,18 @@ try { // never opens an error tracking issue. assert.equal(shouldCapturePosthogException(new CliUserError('Login cancelled')), false) assert.equal(shouldCapturePosthogException(new CliUserError('Upload cancelled by user')), false) + // `resolveUserIdFromApiKey` throws a bad-key failure as CliUserError, so a + // later reword of the message can no longer break the filter by substring. + assert.equal(shouldCapturePosthogException(new CliUserError('Capgo authentication failed: invalid Capgo API key or insufficient Capgo permissions.')), false) + // `checkAppExistsAndHasPermissionOrgErr` throws the RBAC failure as + // CliUserError; the app id lives in context (the permission key is a bounded + // enum and stays in the message), so every app maps to one issue per + // permission instead of one issue per app. + assert.equal(shouldCapturePosthogException(new CliUserError('Insufficient permissions for app. Required RBAC permission for this action: app.write.', { appId: 'com.example.app' })), false) + assert.equal( + new CliUserError('Insufficient permissions for app. Required RBAC permission for this action: channel.delete.', { appId: 'com.a' }).message, + new CliUserError('Insufficient permissions for app. Required RBAC permission for this action: channel.delete.', { appId: 'com.b' }).message, + ) // Two failures on different channels must be treated identically (one issue, // not one per channel), since the channel name lives in context, not the message. assert.equal(