Skip to content

Commit 557e407

Browse files
committed
fix(supabase): address review feedback on update-bucket, signed upload url, and introspect
- storage_update_bucket now fetches the bucket's current config first and only overrides isPublic/fileSizeLimit/allowedMimeTypes when the caller explicitly provides them, instead of silently forcing public: false on every update (the Storage API's PUT is a full replace, not a patch) - storage_create_signed_upload_url and storage_empty_bucket now check response.ok before surfacing an error, so a non-2xx response reports Supabase's actual error instead of a misleading parse-side message - introspect.ts drops the spurious ?select=* query param from the OpenAPI spec request (meaningless on the root spec endpoint)
1 parent 52b655a commit 557e407

4 files changed

Lines changed: 97 additions & 33 deletions

File tree

apps/sim/tools/supabase/introspect.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export const introspectTool: ToolConfig<SupabaseIntrospectParams, SupabaseIntros
5151
},
5252

5353
request: {
54-
url: (params) => `${supabaseBaseUrl(params.projectId)}/rest/v1/?select=*`,
54+
url: (params) => `${supabaseBaseUrl(params.projectId)}/rest/v1/`,
5555
method: 'GET',
5656
headers: (params) => ({
5757
apikey: params.apiKey,

apps/sim/tools/supabase/storage_create_signed_upload_url.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,12 @@ export const storageCreateSignedUploadUrlTool: ToolConfig<
7777
)
7878
}
7979

80+
if (!response.ok) {
81+
throw new Error(
82+
`Failed to create signed upload URL: ${data.message || data.error || response.statusText}`
83+
)
84+
}
85+
8086
const relativeUrl = data.url
8187
if (!relativeUrl) {
8288
throw new Error('Supabase did not return a signed upload URL path in its response')

apps/sim/tools/supabase/storage_empty_bucket.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,12 @@ export const storageEmptyBucketTool: ToolConfig<
5959
throw new Error(`Failed to parse Supabase storage empty bucket response: ${parseError}`)
6060
}
6161

62+
if (!response.ok) {
63+
throw new Error(
64+
`Failed to empty storage bucket: ${data.message || data.error || response.statusText}`
65+
)
66+
}
67+
6268
return {
6369
success: true,
6470
output: {

apps/sim/tools/supabase/storage_update_bucket.ts

Lines changed: 84 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { getErrorMessage } from '@sim/utils/errors'
12
import {
23
STORAGE_MESSAGE_OUTPUT_PROPERTIES,
34
type SupabaseStorageUpdateBucketParams,
@@ -32,19 +33,21 @@ export const storageUpdateBucketTool: ToolConfig<
3233
type: 'boolean',
3334
required: false,
3435
visibility: 'user-or-llm',
35-
description: 'Whether the bucket should be publicly accessible (default: false)',
36+
description:
37+
'Whether the bucket should be publicly accessible (leave unset to keep the current value)',
3638
},
3739
fileSizeLimit: {
3840
type: 'number',
3941
required: false,
4042
visibility: 'user-or-llm',
41-
description: 'Maximum file size in bytes (optional)',
43+
description: 'Maximum file size in bytes (leave unset to keep the current value)',
4244
},
4345
allowedMimeTypes: {
4446
type: 'array',
4547
required: false,
4648
visibility: 'user-or-llm',
47-
description: 'Array of allowed MIME types (e.g., ["image/png", "image/jpeg"])',
49+
description:
50+
'Array of allowed MIME types (e.g., ["image/png", "image/jpeg"]) — leave unset to keep the current value',
4851
},
4952
apiKey: {
5053
type: 'string',
@@ -54,51 +57,100 @@ export const storageUpdateBucketTool: ToolConfig<
5457
},
5558
},
5659

60+
/**
61+
* Unreachable: `directExecution` below always handles this tool because
62+
* the update must first read the bucket's current configuration (the
63+
* Storage API's update-bucket endpoint is a full-replace PUT, not a
64+
* partial patch). Declared only to satisfy `ToolConfig`'s required
65+
* `request` field.
66+
*/
5767
request: {
58-
url: (params) => {
59-
const bucket = encodeStorageSegment(params.bucket)
60-
return `${supabaseBaseUrl(params.projectId)}/storage/v1/bucket/${bucket}`
61-
},
68+
url: (params) =>
69+
`${supabaseBaseUrl(params.projectId)}/storage/v1/bucket/${encodeStorageSegment(params.bucket)}`,
6270
method: 'PUT',
6371
headers: (params) => ({
6472
apikey: params.apiKey,
6573
Authorization: `Bearer ${params.apiKey}`,
6674
'Content-Type': 'application/json',
6775
}),
68-
body: (params) => {
76+
},
77+
78+
/**
79+
* The Storage API's update-bucket endpoint is a full-replace PUT
80+
* (`{id, name, public, file_size_limit?, allowed_mime_types?}`), not a
81+
* partial patch. Fetching the bucket's current configuration first lets
82+
* unset params fall back to their existing value instead of silently
83+
* resetting to a default (e.g. flipping a public bucket private just
84+
* because `isPublic` wasn't provided).
85+
*/
86+
directExecution: async (
87+
params: SupabaseStorageUpdateBucketParams
88+
): Promise<SupabaseStorageUpdateBucketResponse> => {
89+
const baseUrl = supabaseBaseUrl(params.projectId)
90+
const bucket = encodeStorageSegment(params.bucket)
91+
const headers = {
92+
apikey: params.apiKey,
93+
Authorization: `Bearer ${params.apiKey}`,
94+
'Content-Type': 'application/json',
95+
}
96+
97+
try {
98+
const currentResponse = await fetch(`${baseUrl}/storage/v1/bucket/${bucket}`, {
99+
method: 'GET',
100+
headers,
101+
})
102+
103+
if (!currentResponse.ok) {
104+
const errorText = await currentResponse.text()
105+
throw new Error(`Failed to read current bucket configuration: ${errorText}`)
106+
}
107+
108+
const current = await currentResponse.json()
109+
69110
const payload: any = {
70111
id: params.bucket,
71112
name: params.bucket,
72-
public: params.isPublic || false,
113+
public: params.isPublic !== undefined ? params.isPublic : Boolean(current.public),
114+
file_size_limit:
115+
params.fileSizeLimit !== undefined
116+
? Number(params.fileSizeLimit)
117+
: (current.file_size_limit ?? null),
118+
allowed_mime_types:
119+
params.allowedMimeTypes !== undefined
120+
? params.allowedMimeTypes
121+
: (current.allowed_mime_types ?? null),
73122
}
74123

75-
if (params.fileSizeLimit) {
76-
payload.file_size_limit = Number(params.fileSizeLimit)
77-
}
124+
const updateResponse = await fetch(`${baseUrl}/storage/v1/bucket/${bucket}`, {
125+
method: 'PUT',
126+
headers,
127+
body: JSON.stringify(payload),
128+
})
78129

79-
if (params.allowedMimeTypes && params.allowedMimeTypes.length > 0) {
80-
payload.allowed_mime_types = params.allowedMimeTypes
130+
if (!updateResponse.ok) {
131+
const errorText = await updateResponse.text()
132+
throw new Error(`Failed to update bucket: ${errorText}`)
81133
}
82134

83-
return payload
84-
},
85-
},
86-
87-
transformResponse: async (response: Response) => {
88-
let data
89-
try {
90-
data = await response.json()
91-
} catch (parseError) {
92-
throw new Error(`Failed to parse Supabase storage update bucket response: ${parseError}`)
93-
}
135+
const data = await updateResponse.json()
94136

95-
return {
96-
success: true,
97-
output: {
98-
message: 'Successfully updated storage bucket',
99-
results: data,
100-
},
101-
error: undefined,
137+
return {
138+
success: true,
139+
output: {
140+
message: 'Successfully updated storage bucket',
141+
results: data,
142+
},
143+
error: undefined,
144+
}
145+
} catch (error) {
146+
return {
147+
success: false,
148+
output: {
149+
message: 'Failed to update storage bucket',
150+
results: {},
151+
},
152+
error: getErrorMessage(error, 'Unknown error occurred'),
153+
}
102154
}
103155
},
104156

0 commit comments

Comments
 (0)