File backend crashes on deleteObjects

[grschafer]

Bug report

Describe the bug

The deleteObjects implementation for the file backend crashes on the fs.rm call:

https://github.com/supabase/storage-api/blob/30b3cb50295d72d2816e98f2c345d9ca5ab59539/src/backend/file.ts#L90

The reason, from what I can tell, is that the version of fs-extra in use (8.1.0) doesn't export a version of rm (https://github.com/jprichardson/node-fs-extra/blob/8.1.0/lib/fs/index.js#L33), so it falls back to using the node built-in version of fs.rm (docs and code) which requires a callback parameter. The storage-api calls fs.rm without a callback parameter, leading to an error like the below when calling deleteObjects or emptyBucket API endpoints:

TypeError: callback is not a function
    at CB (internal/fs/rimraf.js:59:5)
    at internal/fs/rimraf.js:90:14
    at FSReqCallback.oncomplete (fs.js:179:23)
[ERROR] 18:43:57 TypeError: callback is not a function

Here's the line that crashes: https://github.com/nodejs/node/blob/v14.19.0/lib/internal/fs/rimraf.js#L59

And more context about the callback parameter: https://github.com/isaacs/rimraf/issues/111

To Reproduce

Follow the Development instructions in the README in this repo, but use the file STORAGE_BACKEND. Example of the .env I used:

ANON_KEY=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyb2xlIjoiYW5vbiIsImlhdCI6MTYxMzUzMTk4NSwiZXhwIjoxOTI5MTA3OTg1fQ.mqfi__KnQB4v6PkIjkhzfwWrYyF94MEbSC6LnuvVniE
SERVICE_KEY=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyb2xlIjoic2VydmljZV9yb2xlIiwiaWF0IjoxNjEzNTMxOTg1LCJleHAiOjE5MjkxMDc5ODV9.th84OKK0Iz8QchDyXZRrojmKSEZ-OuitQm_5DvLiSIc
TENANT_ID=stub
REGION=stub
GLOBAL_S3_BUCKET=stub
POSTGREST_URL=http://localhost:3000
PGRST_JWT_SECRET=f023d3db-39dc-4ac9-87b2-b2be72e9162b
DATABASE_URL=postgresql://postgres:[email protected]/postgres
PGOPTIONS="-c search_path=storage,public"
FILE_SIZE_LIMIT=52428800
STORAGE_BACKEND=file
FILE_STORAGE_BACKEND_PATH=./data

# Multitenant
# IS_MULTITENANT=true
MULTITENANT_DATABASE_URL=postgresql://postgres:[email protected]:5433/postgres
X_FORWARDED_HOST_REGEXP=
POSTGREST_URL_SUFFIX=/rest/v1
ADMIN_API_KEYS=apikey
ENCRYPTION_KEY=encryptionkey

After you've started the server and made the curl request to create a bucket, create a file and make another curl request to upload the file:

echo "hello" > world.txt
curl --location --request POST 'http://localhost:5000/object/avatars/myavatar' \
--header 'Authorization: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyb2xlIjoic2VydmljZV9yb2xlIiwiaWF0IjoxNjEzNTMxOTg1LCJleHAiOjE5MjkxMDc5ODV9.th84OKK0Iz8QchDyXZRrojmKSEZ-OuitQm_5DvLiSIc' \
--data @world.txt

Then, call the deleteObjects API:

curl --location --request DELETE 'http://localhost:5000/object/avatars' \
-H 'Authorization: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyb2xlIjoic2VydmljZV9yb2xlIiwiaWF0IjoxNjEzNTMxOTg1LCJleHAiOjE5MjkxMDc5ODV9.th84OKK0Iz8QchDyXZRrojmKSEZ-OuitQm_5DvLiSIc' \
-H 'content-type: application/json' \
--data '{"prefixes": ["myavatar"]}'

In the npm run dev output, you'll see the error:

TypeError: callback is not a function
    at CB (internal/fs/rimraf.js:59:5)
    at internal/fs/rimraf.js:90:14
    at FSReqCallback.oncomplete (fs.js:179:23)
[ERROR] 18:43:57 TypeError: callback is not a function

The file is deleted before the crash happens.

The dev server doesn't crash back out to a shell prompt, but it stops listening on port 5000. If you use supabase-cli and re-create this error, you'll see the storage-api container restart.

Expected behavior

I would expect the deleteObjects and emptyBucket API endpoints to not crash the server process/container.

Possible fixes

I'm not too familiar with the JS ecosystem yet, so I'm not sure what solution makes the most sense, but here are a couple ideas:

• instead of fs.rm, use fsPromises.rm, which doesn't have a required callback and matches how the current fs.rm call is being executed
• provide the required callback and wait for it before returning a response from storage-api

System information

• Version of Node.js: v16.13.1 and v14.19.0 both repro
• Version of storage-api: v0.13.1

Additional info

It seems to me like the fs-extra types for rm are incorrect (missing the callback) which might be what led to this problem in the first place. I'm not sure I'm tracking the implementation across all the different packages and versions correctly for the 9.0+ version of fs-extra though.