mirror of
https://github.com/supabase/supabase.git
synced 2026-10-05 09:25:06 +03:00
feat(studio): add admonition for public bucket rls (#44447)
## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? Similar to the Advisor, this adds an admonition enabling the user to disable a public RLS select via dashboard. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Detect public buckets that have removable SELECT policies and show a contextual warning on the bucket page. * Let users remove the policy via a confirmation dialog that previews the removal action. * Show success/error feedback and automatically refresh storage views after removal. * Adjust page layout to surface the warning above the storage explorer. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Charis Lam <26616127+charislam@users.noreply.github.com>
This commit is contained in:
1 parent
9e97aeab69
commit
00ecdca2b4
4 files changed
+249
-13
No files matched your search
@@ -0,0 +1,145 @@
|
||||
import { ident } from '@supabase/pg-meta/src/pg-format'
|
||||
import { useMutation, useQueryClient } from '@tanstack/react-query'
|
||||
import { databasePoliciesKeys } from 'data/database-policies/keys'
|
||||
import { storageKeys } from 'data/storage/keys'
|
||||
import { useState, type ReactNode } from 'react'
|
||||
import { toast } from 'sonner'
|
||||
import { Button } from 'ui'
|
||||
import { Admonition } from 'ui-patterns/admonition'
|
||||
import { CodeBlock } from 'ui-patterns/CodeBlock'
|
||||
import { ConfirmationModal } from 'ui-patterns/Dialogs/ConfirmationModal'
|
||||
|
||||
import { executeSql } from '@/data/sql/execute-sql-query'
|
||||
import { usePublicBucketsWithSelectPoliciesQuery } from '@/data/storage/public-buckets-with-select-policies-query'
|
||||
import { useSelectedProjectQuery } from '@/hooks/misc/useSelectedProject'
|
||||
|
||||
function generatePolicyRemovalSql(policyName: string) {
|
||||
return `DROP POLICY IF EXISTS ${ident(policyName)} ON storage.objects;`
|
||||
}
|
||||
|
||||
export interface PublicBucketWarningProps {
|
||||
projectRef: string
|
||||
bucketId: string
|
||||
}
|
||||
|
||||
export function PublicBucketWarning({ projectRef, bucketId }: PublicBucketWarningProps): ReactNode {
|
||||
const queryClient = useQueryClient()
|
||||
const { data: project } = useSelectedProjectQuery()
|
||||
|
||||
const { data } = usePublicBucketsWithSelectPoliciesQuery({
|
||||
projectRef,
|
||||
connectionString: project?.connectionString,
|
||||
bucketId,
|
||||
})
|
||||
const policyToRemove = data?.[0]
|
||||
|
||||
const { mutate: removePolicy, isPending: isRemovingPolicy } = useMutation({
|
||||
mutationFn: async (policyName: string) => {
|
||||
await executeSql({
|
||||
projectRef,
|
||||
connectionString: project?.connectionString,
|
||||
sql: generatePolicyRemovalSql(policyName),
|
||||
})
|
||||
},
|
||||
onSuccess: async () => {
|
||||
await Promise.all([
|
||||
queryClient.invalidateQueries({
|
||||
queryKey: storageKeys.publicBucketsWithSelectPolicies(projectRef, bucketId),
|
||||
}),
|
||||
queryClient.invalidateQueries({
|
||||
queryKey: databasePoliciesKeys.list(projectRef, 'storage'),
|
||||
}),
|
||||
])
|
||||
setShowModal(false)
|
||||
toast.success('Policy removed successfully')
|
||||
},
|
||||
onError: (error) => {
|
||||
console.error('Failed to remove policy', error)
|
||||
toast.error(`Failed to remove policy: ${error.message}`)
|
||||
},
|
||||
})
|
||||
|
||||
const [showModal, setShowModal] = useState(false)
|
||||
|
||||
return policyToRemove ? (
|
||||
<PublicBucketWarningView
|
||||
_tag="policy-to-remove"
|
||||
policyName={policyToRemove.policyname}
|
||||
isRemovingPolicy={isRemovingPolicy}
|
||||
onRemovePolicy={() => removePolicy(policyToRemove.policyname)}
|
||||
isModalVisible={showModal}
|
||||
onShowModal={() => setShowModal(true)}
|
||||
onHideModal={() => setShowModal(false)}
|
||||
/>
|
||||
) : (
|
||||
<PublicBucketWarningView _tag="no-policy-to-remove" />
|
||||
)
|
||||
}
|
||||
|
||||
type PublicBucketWarningViewProps_NoPolicyToRemove = {
|
||||
_tag: 'no-policy-to-remove'
|
||||
}
|
||||
|
||||
type PublicBucketWarningViewProps_PolicyToRemove = {
|
||||
_tag: 'policy-to-remove'
|
||||
policyName: string
|
||||
isRemovingPolicy: boolean
|
||||
onRemovePolicy: () => void
|
||||
isModalVisible: boolean
|
||||
onShowModal: () => void
|
||||
onHideModal: () => void
|
||||
}
|
||||
|
||||
type PublicBucketWarningViewProps =
|
||||
| PublicBucketWarningViewProps_NoPolicyToRemove
|
||||
| PublicBucketWarningViewProps_PolicyToRemove
|
||||
|
||||
function PublicBucketWarningView(props: PublicBucketWarningViewProps): ReactNode {
|
||||
if (props._tag === 'no-policy-to-remove') {
|
||||
return null
|
||||
}
|
||||
|
||||
const { policyName, isRemovingPolicy, onRemovePolicy, isModalVisible, onShowModal, onHideModal } =
|
||||
props
|
||||
|
||||
return (
|
||||
<>
|
||||
<Admonition
|
||||
type="warning"
|
||||
layout="horizontal"
|
||||
title="Anyone can list all files in this bucket"
|
||||
description="A SELECT policy on storage.objects allows clients to retrieve a full list of files. Public buckets don’t need this and it may expose more data than intended."
|
||||
actions={
|
||||
<Button type="warning" size="tiny" onClick={onShowModal}>
|
||||
Remove policy
|
||||
</Button>
|
||||
}
|
||||
/>
|
||||
<ConfirmationModal
|
||||
visible={isModalVisible}
|
||||
variant="destructive"
|
||||
title="Remove SELECT policy"
|
||||
confirmLabel="Remove policy"
|
||||
loading={isRemovingPolicy}
|
||||
onCancel={onHideModal}
|
||||
onConfirm={onRemovePolicy}
|
||||
>
|
||||
<div className="flex flex-col gap-3">
|
||||
<p className="text-sm text-foreground-light">
|
||||
This will drop the <code>SELECT</code> policy that makes the bucket's contents
|
||||
listable. Object URLs will continue to work.
|
||||
</p>
|
||||
<div className="-mx-4 md:-mx-5 -mb-4 border-t">
|
||||
<CodeBlock
|
||||
hideLineNumbers
|
||||
language="sql"
|
||||
value={generatePolicyRemovalSql(policyName)}
|
||||
wrapperClassName="[&_pre]:px-4 [&_pre]:py-3 [&>pre]:rounded-none [&>pre]:border-0 [&_pre>*]:!whitespace-pre-wrap"
|
||||
className="[&_code]:text-foreground"
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
</ConfirmationModal>
|
||||
</>
|
||||
)
|
||||
}
|
||||
@@ -32,6 +32,8 @@ export const storageKeys = {
|
||||
vectorBucketsIndexes: (projectRef: string | undefined, vectorBucketName: string | undefined) =>
|
||||
['projects', projectRef, 'vector-buckets', vectorBucketName, 'indexes'] as const,
|
||||
archive: (projectRef: string | undefined) => ['projects', projectRef, 'archive'] as const,
|
||||
publicBucketsWithSelectPolicies: (projectRef: string | undefined, bucketId: string | undefined) =>
|
||||
['projects', projectRef, 'public-buckets-with-select-policies', bucketId] as const,
|
||||
icebergNamespaces: ({ projectRef, warehouse }: { projectRef?: string; warehouse?: string }) =>
|
||||
[projectRef, 'warehouse', warehouse, 'namespaces'] as const,
|
||||
icebergNamespace: ({
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
import { literal } from '@supabase/pg-meta/src/pg-format'
|
||||
import { useQuery } from '@tanstack/react-query'
|
||||
import { executeSql } from 'data/sql/execute-sql-query'
|
||||
import { useSelectedProjectQuery } from 'hooks/misc/useSelectedProject'
|
||||
import { PROJECT_STATUS } from 'lib/constants'
|
||||
import type { ResponseError, UseCustomQueryOptions } from 'types'
|
||||
|
||||
import { storageKeys } from './keys'
|
||||
|
||||
export type PublicBucketsWithSelectPoliciesVariables = {
|
||||
projectRef?: string
|
||||
connectionString?: string | null
|
||||
bucketId: string
|
||||
}
|
||||
|
||||
export type PublicBucketSelectPolicy = {
|
||||
bucket_id: string
|
||||
bucket_name: string
|
||||
policyname: string
|
||||
}
|
||||
|
||||
/**
|
||||
* For the given public bucket, checks whether any SELECT policy on storage.objects
|
||||
* references this bucket's ID in its qual expression. This combination means anyone
|
||||
* can enumerate all objects in the bucket, which is usually unintentional — public
|
||||
* buckets don't require SELECT policies for object access by URL.
|
||||
*
|
||||
* Scoped to a single bucket so the query is a point-lookup rather than a full scan.
|
||||
*/
|
||||
async function getPublicBucketsWithSelectPolicies({
|
||||
projectRef,
|
||||
connectionString,
|
||||
bucketId,
|
||||
}: PublicBucketsWithSelectPoliciesVariables) {
|
||||
const { result } = await executeSql<PublicBucketSelectPolicy[]>({
|
||||
projectRef,
|
||||
connectionString,
|
||||
sql: `
|
||||
SELECT b.id AS bucket_id, b.name AS bucket_name, p.policyname
|
||||
FROM storage.buckets b
|
||||
JOIN pg_policies p
|
||||
ON p.schemaname = 'storage'
|
||||
AND p.tablename = 'objects'
|
||||
AND p.cmd = 'SELECT'
|
||||
WHERE b.public = true
|
||||
AND b.id = ${literal(bucketId)}
|
||||
AND p.qual ~* ('bucket_id\\s*=\\s*' || quote_literal(b.id))
|
||||
`,
|
||||
})
|
||||
|
||||
return result
|
||||
}
|
||||
|
||||
export type PublicBucketsWithSelectPoliciesData = Awaited<
|
||||
ReturnType<typeof getPublicBucketsWithSelectPolicies>
|
||||
>
|
||||
export type PublicBucketsWithSelectPoliciesError = ResponseError
|
||||
|
||||
export const usePublicBucketsWithSelectPoliciesQuery = <
|
||||
TData = PublicBucketsWithSelectPoliciesData,
|
||||
>(
|
||||
{ projectRef, connectionString, bucketId }: PublicBucketsWithSelectPoliciesVariables,
|
||||
{
|
||||
enabled = true,
|
||||
...options
|
||||
}: UseCustomQueryOptions<
|
||||
PublicBucketsWithSelectPoliciesData,
|
||||
PublicBucketsWithSelectPoliciesError,
|
||||
TData
|
||||
> = {}
|
||||
) => {
|
||||
const { data: project } = useSelectedProjectQuery()
|
||||
const isActive = project?.status === PROJECT_STATUS.ACTIVE_HEALTHY
|
||||
|
||||
return useQuery<PublicBucketsWithSelectPoliciesData, PublicBucketsWithSelectPoliciesError, TData>(
|
||||
{
|
||||
queryKey: storageKeys.publicBucketsWithSelectPolicies(projectRef, bucketId),
|
||||
queryFn: () => getPublicBucketsWithSelectPolicies({ projectRef, connectionString, bucketId }),
|
||||
enabled: enabled && typeof projectRef !== 'undefined' && isActive,
|
||||
staleTime: 5 * 60 * 1000,
|
||||
refetchOnWindowFocus: false,
|
||||
...options,
|
||||
}
|
||||
)
|
||||
}
|
||||
@@ -1,15 +1,4 @@
|
||||
import { useParams } from 'common'
|
||||
import { DeleteBucketModal } from 'components/interfaces/Storage/DeleteBucketModal'
|
||||
import { EditBucketModal } from 'components/interfaces/Storage/EditBucketModal'
|
||||
import { EmptyBucketModal } from 'components/interfaces/Storage/EmptyBucketModal'
|
||||
import { useSelectedBucket } from 'components/interfaces/Storage/FilesBuckets/useSelectedBucket'
|
||||
import { PUBLIC_BUCKET_TOOLTIP } from 'components/interfaces/Storage/Storage.constants'
|
||||
import StorageBucketsError from 'components/interfaces/Storage/StorageBucketsError'
|
||||
import { StorageExplorer } from 'components/interfaces/Storage/StorageExplorer/StorageExplorer'
|
||||
import { useBucketPolicyCount } from 'components/interfaces/Storage/useBucketPolicyCount'
|
||||
import DefaultLayout from 'components/layouts/DefaultLayout'
|
||||
import { PageLayout } from 'components/layouts/PageLayout/PageLayout'
|
||||
import StorageLayout from 'components/layouts/StorageLayout/StorageLayout'
|
||||
import { ChevronDown, FolderOpen, Settings, Shield, Trash2 } from 'lucide-react'
|
||||
import Link from 'next/link'
|
||||
import { useRouter } from 'next/router'
|
||||
@@ -30,6 +19,18 @@ import {
|
||||
TooltipTrigger,
|
||||
} from 'ui'
|
||||
|
||||
import { DeleteBucketModal } from '@/components/interfaces/Storage/DeleteBucketModal'
|
||||
import { EditBucketModal } from '@/components/interfaces/Storage/EditBucketModal'
|
||||
import { EmptyBucketModal } from '@/components/interfaces/Storage/EmptyBucketModal'
|
||||
import { useSelectedBucket } from '@/components/interfaces/Storage/FilesBuckets/useSelectedBucket'
|
||||
import { PublicBucketWarning } from '@/components/interfaces/Storage/PublicBucketWarning'
|
||||
import { PUBLIC_BUCKET_TOOLTIP } from '@/components/interfaces/Storage/Storage.constants'
|
||||
import StorageBucketsError from '@/components/interfaces/Storage/StorageBucketsError'
|
||||
import { StorageExplorer } from '@/components/interfaces/Storage/StorageExplorer/StorageExplorer'
|
||||
import { useBucketPolicyCount } from '@/components/interfaces/Storage/useBucketPolicyCount'
|
||||
import DefaultLayout from '@/components/layouts/DefaultLayout'
|
||||
import { PageLayout } from '@/components/layouts/PageLayout/PageLayout'
|
||||
import StorageLayout from '@/components/layouts/StorageLayout/StorageLayout'
|
||||
import { StorageExplorerStateContextProvider } from '@/state/storage-explorer'
|
||||
|
||||
const BucketPage: NextPageWithLayout = () => {
|
||||
@@ -150,8 +151,11 @@ const BucketPage: NextPageWithLayout = () => {
|
||||
</>
|
||||
}
|
||||
>
|
||||
<div className="flex-1 min-h-0 px-6 pb-6">
|
||||
<StorageExplorer />
|
||||
<div className="flex-1 min-h-0 px-6 pb-6 flex flex-col gap-4">
|
||||
{ref && bucketId && <PublicBucketWarning projectRef={ref} bucketId={bucketId} />}
|
||||
<div className="flex-1 min-h-0">
|
||||
<StorageExplorer />
|
||||
</div>
|
||||
</div>
|
||||
</PageLayout>
|
||||
|
||||
|
||||
Reference in new issue
Block a user