supabase-community/postgrest-csharp

Delete returns a success status code even if it actually failed due to RLS policies

oli-g-sk opened this issue · 1 comments

Bug report

When I tried to delete a row from a table where I forgot to set up a permissive DELETE policy, the client kept returning a successful response code, even though the row wasn't deleted.

To Reproduce

  1. Set up a Supabase table with RLS enabled but without any DELETE policies
  2. Populate it with some rows
  3. Try deleting a row
try
{
  await supaBaseClient.From<Item>()
    .Where(i => i.Id == item.Id)
    .Delete();
}
catch (Exception ex)
{
  // log
  throw;
}

Expected behavior

  • The operation fails
  • In the provided application code, an exception is logged and re-thrown
  • In the client's MakeRequest method in Helper.cs, response.IsSuccessStatusCode is false in the following code block, and the method proceeds to create and throw a PostgrestException
var response = await Client.SendAsync(requestMessage, cancellationToken);
var content = await response.Content.ReadAsStringAsync();

if (response.IsSuccessStatusCode)
  return new BaseResponse(clientOptions, response, content);

System information

  • OS: Windows 11
  • Version of postgrest-csharp: 3.5.1
  • Version of supabase-csharp: 0.16.1

Hey @oli-g-sk - thanks for the issue!

I am able to reduplicate it myself, but I'm not sure what approach to take to rectify it. Would love some input.

The hosted supabase instance will return the rows it has deleted as its content when processing a DELETE request. When a DELETE request fails because of RLS or because there are no rows affected, it will return an empty array: [ ]. Because of this, the actual HTTP Status code is a success. Alternatively, it will return an array of model content when it succeeds.

I don't believe the solution is to mark any empty return from a DELETE request to be an exception (as a filter with no matching content isn't an exception imo).

So, one solution would be to change the void return type on Table.cs to ModeledResponse and the developer could verify the rows have been deleted in the response. This seems less than ideal to me though.

Another would be to change the void return type on Table.cs to bool and execute a COUNT using the provided filter first, then do the DELETE, then verify the count matches the returned DELETE rows.

Thoughts?