Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 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
10 changes: 9 additions & 1 deletion supabase/functions/_backend/public/channel/post.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@
}
const { data: existingChannel } = await supabaseAdmin(c)
.from('channels')
.select('id, version, rollout_version')
.select('id, version, rollout_version, public')
.eq('app_id', body.app_id)
.eq('name', body.channel)
.maybeSingle()
Expand All @@ -345,6 +345,14 @@
else if (!(await checkPermission(c, 'app.create_channel', { appId: body.app_id }))) {
throw simpleError('cannot_access_app', 'You can\'t access this app', { app_id: body.app_id })
}
// A public/default channel changes the app's delivery configuration. Preview
// keys may bootstrap private channels only, so they cannot create or flip one
// to public without app.update_settings. Retaining an already-public channel
// stays channel-scoped, matching the UPDATE trigger boundary.
const isPublicizing = body.public === true && (existingChannel == null || existingChannel.public !== true)

Check warning on line 352 in supabase/functions/_backend/public/channel/post.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Prefer using an optional chain expression instead, as it's more concise and easier to read.

See more on https://sonarcloud.io/project/issues?id=Cap-go_capgo&issues=AZ_inE9iIdg5QZyCp8Bt&open=AZ_inE9iIdg5QZyCp8Bt&pullRequest=2713
if (isPublicizing && !(await checkPermission(c, 'app.update_settings', { appId: body.app_id }))) {
throw simpleError('cannot_access_app', 'You can\'t access this app', { app_id: body.app_id, channel: body.channel })
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const { data: org, error } = await supabaseAdmin(c).from('apps').select('owner_org').eq('app_id', body.app_id).single()
if (error || !org) {
throw simpleError('invalid_app_id', 'You can\'t access this app', { app_id: body.app_id })
Expand Down
1 change: 1 addition & 0 deletions supabase/functions/_backend/triggers/logsnag_insights.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ type AppBuildOnboardingMetrics = Record<string, unknown> & {
apps_with_manual_builds_24h: number
}
interface AppBuildOnboardingMetricRow {
[key: string]: unknown
created_at: string | Date | null
created_from_onboarding: boolean | null
onboarding_completed_at: string | Date | null
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
-- A public/default channel changes app delivery settings. App-preview keys may
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
-- bootstrap private channels, but must not create or flip a channel to public.
-- INSERT RLS blocks public creates. UPDATE uses a BEFORE trigger so the check
-- only applies on private -> public transitions; channel-scoped admins can still
-- edit already-public channels. The channel endpoint mirrors the same guard.
DROP POLICY IF EXISTS "Allow RBAC channels insert" ON public.channels;
CREATE POLICY "Allow RBAC channels insert"
Comment thread
riderx marked this conversation as resolved.
ON public.channels
FOR INSERT
TO anon, authenticated
WITH CHECK (
public.rbac_check_permission_request(
public.rbac_perm_app_create_channel(),
owner_org,
app_id,
NULL::bigint
)
AND (
"public" IS FALSE
OR public.rbac_check_permission_request(
public.rbac_perm_app_update_settings(),
owner_org,
app_id,
NULL::bigint
)
)
AND (
(version IS NULL AND rollout_version IS NULL)
OR public.rbac_check_permission_request(
public.rbac_perm_channel_promote_bundle(),
owner_org,
app_id,
NULL::bigint
)
)
);

-- Keep UPDATE RLS channel-scoped. Requiring app.update_settings whenever the
-- NEW row is public would block legitimate edits to already-public channels.
DROP POLICY IF EXISTS "Allow RBAC channels update" ON public.channels;
CREATE POLICY "Allow RBAC channels update"
ON public.channels
FOR UPDATE
TO anon, authenticated
USING (
public.rbac_check_permission_request(
public.rbac_perm_channel_update_settings(),
owner_org,
app_id,
id
)
)
WITH CHECK (
public.rbac_check_permission_request(
public.rbac_perm_channel_update_settings(),
owner_org,
app_id,
id
)
);

CREATE OR REPLACE FUNCTION public.enforce_public_channel_app_settings_permission()
RETURNS trigger
LANGUAGE plpgsql
SECURITY DEFINER
SET search_path = ''
AS $$
DECLARE
v_request_role text := COALESCE(auth.role(), session_user);
BEGIN
-- Only gate private -> public transitions. Service-role/admin paths enforce
-- the matching app.update_settings check in application code.
IF NEW.public IS TRUE
AND OLD.public IS NOT TRUE
AND v_request_role NOT IN ('service_role', 'postgres')
THEN
IF v_request_role IS DISTINCT FROM 'anon' AND v_request_role IS DISTINCT FROM 'authenticated' THEN
RAISE EXCEPTION 'PERMISSION_DENIED_APP_UPDATE_SETTINGS'
USING ERRCODE = '42501';
END IF;

IF NOT public.rbac_check_permission_request(
public.rbac_perm_app_update_settings(),
NEW.owner_org,
NEW.app_id,
NULL::bigint
) THEN
RAISE EXCEPTION 'PERMISSION_DENIED_APP_UPDATE_SETTINGS'
USING ERRCODE = '42501';
END IF;
END IF;

RETURN NEW;
END;
$$;

ALTER FUNCTION public.enforce_public_channel_app_settings_permission() OWNER TO postgres;
REVOKE ALL ON FUNCTION public.enforce_public_channel_app_settings_permission() FROM PUBLIC;
GRANT EXECUTE ON FUNCTION public.enforce_public_channel_app_settings_permission() TO service_role;

DROP TRIGGER IF EXISTS enforce_public_channel_app_settings_permission ON public.channels;
CREATE TRIGGER enforce_public_channel_app_settings_permission
BEFORE UPDATE OF "public" ON public.channels
FOR EACH ROW
EXECUTE FUNCTION public.enforce_public_channel_app_settings_permission();

COMMENT ON FUNCTION public.enforce_public_channel_app_settings_permission() IS
'Requires app.update_settings when a user-context write flips a channel from private to public.';
26 changes: 25 additions & 1 deletion supabase/tests/26_test_rls_policies.sql
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
-- Test RLS Policies
-- This file tests all Row Level Security policies in the database
BEGIN;
SELECT plan(70);
SELECT plan(72);
SELECT
policies_are(
'public',
Expand Down Expand Up @@ -410,6 +410,30 @@ SELECT
'channels update policy should honor channel-scoped update permission'
);

SELECT
ok(
(
SELECT COALESCE(with_check, '') ~ 'rbac_perm_app_update_settings'
Comment thread
cursor[bot] marked this conversation as resolved.
FROM pg_policies
WHERE schemaname = 'public'
AND tablename = 'channels'
AND policyname = 'Allow RBAC channels insert'
),
'channels insert policy should require app.update_settings for public channels'
);

SELECT
ok(
EXISTS (
SELECT 1
FROM pg_trigger
WHERE tgname = 'enforce_public_channel_app_settings_permission'
AND tgrelid = 'public.channels'::regclass
AND NOT tgisinternal
),
'channels should enforce app.update_settings on private-to-public updates'
);

SELECT
ok(
NOT EXISTS (
Expand Down
73 changes: 73 additions & 0 deletions tests/channel-post.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ function buildAdminChain(body: {
existingChannelId?: number | null
existingChannelVersion?: number | null
existingRolloutVersion?: number | null
existingChannelPublic?: boolean
ownerOrg?: string
versionId?: number
versionError?: { message: string } | null
Expand Down Expand Up @@ -98,6 +99,7 @@ function buildAdminChain(body: {
id: body.existingChannelId,
version: body.existingChannelVersion ?? null,
rollout_version: body.existingRolloutVersion ?? null,
public: body.existingChannelPublic ?? false,
},
error: null,
}),
Expand Down Expand Up @@ -256,6 +258,77 @@ describe('public channel post', () => {
expect(updateOrCreateChannel).toHaveBeenCalledWith(c, expect.not.objectContaining({ electron: false }), null, true)
})

it('requires app settings permission to create a public channel', async () => {
checkPermission
.mockResolvedValueOnce(true)
.mockResolvedValueOnce(false)
const { post } = await import('../supabase/functions/_backend/public/channel/post.ts')
const c = context()

await expect(post(c, {
app_id: 'com.test.preview',
channel: 'preview-default',
public: true,
}, apiKey())).rejects.toMatchObject({
cause: expect.objectContaining({ error: 'cannot_access_app' }),
})

expect(checkPermission).toHaveBeenNthCalledWith(1, c, 'app.create_channel', { appId: 'com.test.preview' })
expect(checkPermission).toHaveBeenNthCalledWith(2, c, 'app.update_settings', { appId: 'com.test.preview' })
expect(updateOrCreateChannel).not.toHaveBeenCalled()
})

it('requires app settings permission to make an existing private channel public', async () => {
supabaseAdmin.mockImplementation(() => buildAdminChain({
existingChannelId: 42,
existingChannelVersion: 123,
existingChannelPublic: false,
}))
checkPermission
.mockResolvedValueOnce(true)
.mockResolvedValueOnce(false)
const { post } = await import('../supabase/functions/_backend/public/channel/post.ts')
const c = context()

await expect(post(c, {
app_id: 'com.test.preview',
channel: 'preview-existing',
public: true,
}, apiKey())).rejects.toMatchObject({
cause: expect.objectContaining({ error: 'cannot_access_app' }),
})

expect(checkPermission).toHaveBeenNthCalledWith(1, c, 'channel.update_settings', { appId: 'com.test.preview', channelId: 42 })
expect(checkPermission).toHaveBeenNthCalledWith(2, c, 'app.update_settings', { appId: 'com.test.preview' })
expect(updateOrCreateChannel).not.toHaveBeenCalled()
})

it('allows channel settings updates that retain an already-public channel', async () => {
supabaseAdmin.mockImplementation(() => buildAdminChain({
existingChannelId: 42,
existingChannelVersion: 123,
existingChannelPublic: true,
}))
const { post } = await import('../supabase/functions/_backend/public/channel/post.ts')
const c = context()

await post(c, {
app_id: 'com.test.already-public',
channel: 'production',
public: true,
allow_emulator: true,
}, apiKey())

expect(checkPermission).toHaveBeenCalledTimes(1)
expect(checkPermission).toHaveBeenCalledWith(c, 'channel.update_settings', { appId: 'com.test.already-public', channelId: 42 })
expect(updateOrCreateChannel).toHaveBeenCalledWith(
c,
expect.objectContaining({ version: 123, public: true, allow_emulator: true }),
42,
true,
)
})

it('preserves the stable version for a settings-only update without channel.read or bundle lookup', async () => {
const fromCalls: string[] = []
supabaseAdmin.mockImplementation(() => buildAdminChain({
Expand Down
24 changes: 20 additions & 4 deletions tests/cli-channel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -955,25 +955,41 @@ describe('tests CLI channel commands', () => {
expect(scopedChannelsError).toBeNull()
expect(scopedChannels).toEqual([{ id: target!.id, name: targetChannelName }])

const { error: targetUpdateError } = await scopedSupabase
// Channel-scoped admins may edit settings, but promoting to public
// requires app.update_settings and must be denied here.
const { error: targetPublicUpdateError } = await scopedSupabase
.from('channels')
.update({ public: true })
.eq('id', target!.id)
expect(targetPublicUpdateError?.code).toBe('42501')

const { error: targetUpdateError } = await scopedSupabase
.from('channels')
.update({ allow_emulator: true })
.eq('id', target!.id)
expect(targetUpdateError).toBeNull()

const { error: siblingUpdateError } = await scopedSupabase
.from('channels')
.update({ public: true })
.update({ allow_emulator: true })
.eq('id', sibling!.id)
expect(siblingUpdateError).toBeNull()

const { data: siblingAfterDirectUpdate, error: siblingAfterDirectUpdateError } = await supabase
.from('channels')
.select('public')
.select('allow_emulator')
.eq('id', sibling!.id)
.single()
expect(siblingAfterDirectUpdateError).toBeNull()
expect(siblingAfterDirectUpdate?.public).toBe(false)
expect(siblingAfterDirectUpdate?.allow_emulator).toBe(false)

const { data: targetAfterDirectUpdate, error: targetAfterDirectUpdateError } = await supabase
.from('channels')
.select('public, allow_emulator')
.eq('id', target!.id)
.single()
expect(targetAfterDirectUpdateError).toBeNull()
expect(targetAfterDirectUpdate).toEqual({ public: false, allow_emulator: true })

const postResponse = await fetch(`${BASE_URL}/channel`, {
method: 'POST',
Expand Down
30 changes: 30 additions & 0 deletions tests/cli-preview-lifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ const APPNAME = `com.cli.preview.lifecycle.${id}`
const CHANNEL_NAME = `preview-${id.slice(0, 8)}`
const SECOND_CHANNEL_NAME = `preview-other-${id.slice(0, 8)}`
const MAIN_CHANNEL_NAME = `main-${id.slice(0, 8)}`
const DEFAULT_CHANNEL_NAME = `preview-default-${id.slice(0, 8)}`
const PUBLIC_POST_CHANNEL_NAME = `preview-public-post-${id.slice(0, 8)}`
const BUNDLE_NAME = `1.0.0-preview-${id.slice(0, 8)}`
const LEGACY_CHANNEL_NAME = `preview-legacy-${id.slice(0, 8)}`
const LEGACY_BUNDLE_NAME = `1.0.0-legacy-${id.slice(0, 8)}`
Expand Down Expand Up @@ -248,6 +250,34 @@ describe('cli app preview lifecycle', () => {
supaAnon: SUPABASE_ANON_KEY,
}

await expect(addChannelInternal(DEFAULT_CHANNEL_NAME, APPNAME, {
...cliOptions,
default: true,
}, true)).rejects.toThrow('Cannot create channel')

const publicChannelResponse = await fetch(`${BASE_URL}/channel`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'capgkey': apiKey.key,
},
body: JSON.stringify({
app_id: APPNAME,
channel: PUBLIC_POST_CHANNEL_NAME,
public: true,
}),
})
expect(publicChannelResponse.status).toBe(400)
await expect(publicChannelResponse.json()).resolves.toMatchObject({ error: 'cannot_access_app' })

const blockedPublicChannels = await executeSQL(
`SELECT COUNT(*)::integer AS count
FROM public.channels
WHERE app_id = $1 AND name = ANY($2::varchar[])`,
[APPNAME, [DEFAULT_CHANNEL_NAME, PUBLIC_POST_CHANNEL_NAME]],
)
expect(Number(blockedPublicChannels[0]?.count ?? 0)).toBe(0)

const { upload, requests } = await (async () => {
const fetchSpy = vi.spyOn(globalThis, 'fetch')
try {
Expand Down
Loading