Skip to content

feat: support additional database backends beyond SQLite #56

Description

@rado0x54

Summary

ShellWatch currently only supports SQLite via better-sqlite3 and drizzle-orm/better-sqlite3. Add support for additional database backends (e.g. PostgreSQL) for deployments where SQLite is not ideal — multi-instance setups, larger scale, or environments where a centralized database is preferred.

Current state

  • src/db/connection.ts hardcodes the better-sqlite3 driver and rejects non-SQLite connection strings
  • drizzle.config.ts uses dialect: "sqlite"
  • Migrations in drizzle/ are SQLite-specific SQL
  • runMigrations() uses drizzle-orm/better-sqlite3/migrator

Considerations

  • Drizzle ORM supports multiple dialects (PostgreSQL via drizzle-orm/node-postgres, MySQL via drizzle-orm/mysql2), but migrations are dialect-specific
  • Would need per-dialect migration directories or a generation step
  • Connection string parsing in connection.ts should route to the appropriate driver based on scheme (postgres://, mysql://, sqlite:)
  • SQLite should remain the default for simplicity — no external dependencies for single-instance deployments
  • Docker image would need additional native dependencies (e.g. pg or mysql2) or a separate image variant

Concurrency audit (read-then-write TOCTOU)

The repository layer was written against better-sqlite3's single-process synchronous model, where SELECT ... ; if (...) INSERT/UPDATE is effectively atomic because the event loop cannot yield between statements and only one writer process exists. Once we add a second backend (Postgres) or run multiple writer processes against the same SQLite file, that assumption breaks: two requests can interleave their SELECT and INSERT, producing stale-read bugs and silent overwrites.

Before/while landing this ticket, audit every read-then-decide-then-write pattern in src/db/repositories/ and convert each to one of:

  • a single statement with a conditional ON CONFLICT DO UPDATE ... WHERE ... clause (preferred — driver-agnostic, atomic at the statement level), or
  • an explicit db.transaction(...) wrapping the read and write.

Known instance (do not regress when porting):

  • DrizzlePushSubscriptionRepository.upsert (src/db/repositories/push-subscription-repo.ts) — does a SELECT to enforce that an endpoint cannot be claimed by a second account, then an INSERT with onConflictDoUpdate. Safe under better-sqlite3, racy under multi-writer Postgres / multi-process SQLite. Flagged in PR security(MEDIUM): SSRF via user-controlled push subscription endpoint #131 review. Fix at port time: collapse to a single INSERT ... ON CONFLICT (endpoint) DO UPDATE SET ... WHERE accountId = :accountId RETURNING * and treat zero-row return as the cross-account-rejection case.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions