basecamp / basecamp/activerecord-tenanted

Stale schema cache dump is trusted unconditionally (check_schema_cache_dump_version is disabled) — validate against migration files instead

Open
#319 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Ruby
Stars
617
Forks
29
PR merge metrics
No merged PRs in 30d

Description

### Summary

The railtie disables Rails' schema-cache staleness protection:

```ruby
# lib/active_record/tenanted/railtie.rb (0.6.0 and 0.7.0)
# The schema cache version check needs to query the database, which isn't always possible
config.active_record.check_schema_cache_dump_version = false
```

The reason is sound — tenant databases are lazy and may not exist at boot, so there is nothing to query a schema version from. But the consequence is that a stale `db/tenant_schema_cache.yml` is loaded and **trusted unconditionally**, with no fallback and no warning. In a plain Rails app a stale dump is harmless (the version check discards it and pools fall back to live schema queries); with this gem it produces very confusing failures on columns that plainly exist in every database.

### Symptoms we hit

- `Undeclared attribute type for enum 'kind' in Payout` raised from test setup — the column existed in `schema.rb` and in every database file, but not in the cached columns.
- After merging two branches that each added a column: hundreds of errors across the test suite (view rendering, fixture setup), none of which mention the schema cache.

Both cost real debugging time because the failing tests point everywhere except the cache.

### How the cache goes stale

`database_tasks.rb` only re-dumps the cache when a tenant migration actually runs in that checkout **and** `Rails.env.development? || ENV["ARTENANT_SCHEMA_DUMP"]`, or when the file is missing. So the dump drifts whenever migrations arrive without a dev-environment migrate in that specific checkout, e.g.:

- pulling or merging a branch containing migrations, then running the test suite directly (test env never dumps),
- switching between branches with different migration sets (the cache can also be *newer* than the checkout's migrations, which is equally wrong),
- CI checkouts that restore a cached `db/` artifact.

### Proposal

The staleness check doesn't actually need a database. The dump already embeds the schema version it was created at (`version:` in the YAML), and the expected version is derivable from the migration filenames on disk (the tenant config's `migrations_paths`). Comparing those two — file against files — restores the protection Rails normally provides, without querying any tenant database:

```ruby
# sketch — where the tenanted config resolves its schema_cache_path
dumped = File.foreach(cache_path).find { |l| l.start_with?("version:") }.to_s[/\d+/].to_i
expected = Dir.glob("#{migrations_paths}/[0-9]*.rb").map { |f| File.basename(f).to_i }.max.to_i
discard_cache if dumped != expected
```

On mismatch, ignoring the dump (falling back to live schema queries, as stock Rails does) turns a baffling suite-wide failure into a slightly slower boot, and the next `db:prepare` re-dumps it. A `!=` comparison rather than `<` also covers the switched-to-an-older-branch case.

### Workaround we're using

An initializer that performs the same check at boot in dev/test and deletes a mismatched dump:

```ruby
if Rails.env.local?
cache_path = Rails.root.join("db/tenant_schema_cache.yml")
if cache_path.exist?
cache_version = File.foreach(cache_path).find { |line| line.start_with?("version:") }.to_s[/\d+/].to_i
latest_migration = Rails.root.glob("db/migrate/[0-9]*.rb").map { |file| file.basename.to_s.to_i }.max.to_i
if cache_version != latest_migration
warn "Discarding stale db/tenant_schema_cache.yml (dumped at #{cache_version}, migrations at #{latest_migration})"
cache_path.delete
end
end
end
```

This works, but it seems like something the gem could own for every app — happy to send a PR if the approach sounds right (open to where it should live: the railtie initializer, or `TenantConfig` where `schema_cache_path` is resolved so it applies per tenanted config).

### Environment

- activerecord-tenanted 0.6.0 (also inspected 0.7.0 — same behavior)
- Rails `main` (8.2.0.alpha), SQLite, one database file per tenant

Contributor guide

Open the contributing guide

Research direction

Start in lib/active_record/tenanted/railtie.rb and database_tasks.rb, then trace TenantConfig where schema_cache_path and migrations_paths are resolved. Compare the cache's version with the migration filenames for each tenanted configuration; done means a mismatch is discarded so Rails falls back to live schema queries, including when the checkout switches to older migrations.

Written by the indexing model from the issue text.

Assessment

Tech stack
rails, ruby
Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
50/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.