influitive / apartment

Database multi-tenancy for Rack (and Rails) applications
2.66k stars 460 forks source link

Direct uploads writing to random tenants #669

Open ian-kinner opened 2 years ago

ian-kinner commented 2 years ago

On a busy server with multi-tenancy, direct uploads end up in the active_storage_blob/attachment tables of random tenants. It appears that it might be writing to the tenant most recently used by active storage in other requests on that particular server - and I think this because attachment entries seem to end up in busier tenants more often and dormant tenants never. We narrowly avoided customer-data cross contamination due to this bug because we caught it early. This applies to both excluded and non-excluded models.

Expected behavior is to write to the active_storage_blob/attachment tables of that tenant, not withstanding the problems presented in issue #611 with excluded models' attachments ended up in non-excluded active storage tables.

Actual behavior is that the active_storage_blob/attachment tables written to are not the ones they should be. It's almost like a variable used to store the tenant isn't re-initialized from another request on the same server. This causes cross-customer data leakage.

Rails 5.2.4.6 / Ruby 2.6.5 / Apartment 2.2.1 / AWS Elastic Beanstalk / MySQL (aurora 5.6)

apartment.rb is stock except for the excluded model list and prepend environment is false

archonic commented 2 years ago

Are you switching tenants using a block or Apartment::Tenant.switch!? If it's the 2nd then I think you're hitting a threadsafety issue. I'm not an expert there but that's my suspicion.