diff --git a/internal/core/db.go b/internal/core/db.go index 09d5199..2bc779d 100644 --- a/internal/core/db.go +++ b/internal/core/db.go @@ -12,7 +12,10 @@ import ( // DB returns a scoped database connection. // // Production: uses per-app credentials injected by the platform via Lambda payload. -// Dev: uses DATABASE_URL env var with localhost fallback. +// Dev: uses DATABASE_URL env var with localhost fallback. Dev connections are +// wrapped with the cross-module fail-fast guard (db_guard.go): a raw query +// against another module's tables errors immediately instead of silently +// succeeding in the shared dev database — use ms.DependencyDB for that. // // conn, release, err := mod.DB(r.Context()) // if err != nil { ... } @@ -28,7 +31,7 @@ func (m *Module) DB(ctx context.Context) (db.Querier, func(), error) { releasePool() return nil, nil, err } - return querier, func() { + return m.devGuardFor(ctx, querier, db.CredentialFrom), func() { releaseConn() releasePool() }, nil @@ -51,7 +54,9 @@ func (m *Module) Tx(ctx context.Context, fn func(q db.Querier) error) error { return err } defer releasePool() - return db.Tx(ctx, pool, fn) + return db.Tx(ctx, pool, func(q db.Querier) error { + return fn(m.devGuardFor(ctx, q, db.CredentialFrom)) + }) } // resolvePool returns the per-app credential pool (production) or the dev @@ -111,7 +116,7 @@ func (m *Module) ModuleDB(ctx context.Context) (db.Querier, func(), error) { releasePool() return nil, nil, err } - return querier, func() { + return m.devGuardFor(ctx, querier, db.ModuleCredentialFrom), func() { releaseConn() releasePool() }, nil @@ -134,7 +139,9 @@ func (m *Module) ModuleTx(ctx context.Context, fn func(q db.Querier) error) erro } defer releasePool() moduleCtx := db.WithSchema(ctx, m.moduleSchemaFor(ctx)) - return db.Tx(moduleCtx, pool, fn) + return db.Tx(moduleCtx, pool, func(q db.Querier) error { + return fn(m.devGuardFor(ctx, q, db.ModuleCredentialFrom)) + }) } // resolveModulePool reads the per-module credential instead of the per-app diff --git a/internal/core/db_guard.go b/internal/core/db_guard.go new file mode 100644 index 0000000..e6a6643 --- /dev/null +++ b/internal/core/db_guard.go @@ -0,0 +1,229 @@ +package core + +import ( + "context" + "errors" + "fmt" + "regexp" + "strings" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" + + "github.com/mirrorstack-ai/app-module-sdk/db" +) + +// This file closes the "works on my machine" trap (decision 17 §2): in dev +// mode every module shares one owner pool with search_path-only scoping, so a +// raw SQL read/JOIN against ANOTHER module's tables silently succeeds locally +// — but can never work deployed, where the per-(app,module) Postgres role +// holds no grant on foreign tables. The guard makes dev fail the same way +// prod does, at the SDK's query chokepoint. +// +// KNOWN LIMITS (this is an honest dev-mode fail-fast, not a security +// boundary — production enforcement is the DB role's grants): +// - indirection through views or functions that touch a foreign table is +// not caught (the foreign name never appears in the module's SQL); +// - a foreign table reached only via an unprefixed name (no m_ +// segment anywhere in the statement) is not caught; +// - direct use of the low-level db.Open()/db.DB client bypasses Module.DB +// and therefore the guard. + +// errCrossModuleRelation is the sentinel wrapped into every guard rejection, +// so tests (and callers inside this package) can errors.Is the failure mode. +var errCrossModuleRelation = errors.New("cross-module table access") + +// moduleTableRe finds physical module-table identifiers: the platform-minted +// module ID form m<32 hex> followed by an underscore and the table name +// (m_users, app_.m_users, "m_users"). The leading +// (^|[^a-z0-9]) guard stops mid-word matches (e.g. inside a longer hex blob) +// while still matching after '.', '"', '(' and '_'. Case-folded because +// Postgres lowercases unquoted identifiers. +var moduleTableRe = regexp.MustCompile(`(?i)(?:^|[^a-z0-9])((m[0-9a-f]{32})_[a-z0-9_]*)`) + +// moduleSchemaRe finds references to a module's cross-app schema +// (mod_m.some_table), which end at the hex id — no trailing underscore — +// so moduleTableRe cannot see them. +var moduleSchemaRe = regexp.MustCompile(`(?i)(?:^|[^a-z0-9_])(mod_(m[0-9a-f]{32}))(?:[^a-z0-9_]|$)`) + +// crossModuleRes is hoisted to package level because the guard runs on every +// dev-mode statement — no per-query slice allocation. +var crossModuleRes = [...]*regexp.Regexp{moduleTableRe, moduleSchemaRe} + +// checkCrossModuleSQL scans one SQL statement for physical table or schema +// names owned by a different module and rejects with a clear, actionable +// error. String literals and comments are stripped first so mentioning a +// foreign table in a logged message or comment does not false-positive. +// Only platform-minted m<32 hex> IDs are recognized — an unregistered dev +// module's ad-hoc ID has no deployed physical form to protect. +func checkCrossModuleSQL(ownID, sql string) error { + stripped := stripSQLNoise(sql) + for _, re := range crossModuleRes { + for _, match := range re.FindAllStringSubmatch(stripped, -1) { + if id := strings.ToLower(match[2]); id != ownID { + return fmt.Errorf( + "mirrorstack/db: query references %q, which belongs to another module (module %q): %w — "+ + "this only appears to work in local dev because dev shares one database; "+ + "a deployed module's DB role has no grant on foreign tables, so it will always fail in production. "+ + "Declare the table with ms.DependsOn + n.Table(...) and read it through ms.DependencyDB instead", + strings.ToLower(match[1]), id, errCrossModuleRelation) + } + } + } + return nil +} + +// stripSQLNoise removes the regions of a SQL statement where an identifier +// pattern is meaningless: single-quoted string literals (with doubled-quote +// escapes), line comments, nested block comments, and dollar-quoted strings. Each +// removed region is replaced by a single space so identifier boundaries on +// either side stay intact. Double-quoted regions are kept — those are +// identifiers, exactly what the guard inspects. +func stripSQLNoise(s string) string { + var b strings.Builder + b.Grow(len(s)) + for i := 0; i < len(s); { + switch c := s[i]; { + case c == '\'': + i = skipSingleQuoted(s, i) + b.WriteByte(' ') + case c == '-' && i+1 < len(s) && s[i+1] == '-': + for i < len(s) && s[i] != '\n' { + i++ + } + case c == '/' && i+1 < len(s) && s[i+1] == '*': + i = skipBlockComment(s, i) + b.WriteByte(' ') + case c == '$': + if end, ok := skipDollarQuoted(s, i); ok { + i = end + b.WriteByte(' ') + continue + } + b.WriteByte(c) + i++ + default: + b.WriteByte(c) + i++ + } + } + return b.String() +} + +// skipSingleQuoted returns the index just past a '...' literal starting at +// s[start] (a doubled single quote inside the literal escapes a quote). An +// unterminated literal consumes the rest of the string — invalid SQL Postgres +// would reject anyway. +func skipSingleQuoted(s string, start int) int { + i := start + 1 + for i < len(s) { + if s[i] == '\'' { + if i+1 < len(s) && s[i+1] == '\'' { + i += 2 + continue + } + return i + 1 + } + i++ + } + return i +} + +// skipBlockComment returns the index just past a /* ... */ comment starting +// at s[start]. Postgres block comments nest, so depth is tracked. +func skipBlockComment(s string, start int) int { + depth := 1 + i := start + 2 + for i < len(s) && depth > 0 { + switch { + case s[i] == '/' && i+1 < len(s) && s[i+1] == '*': + depth++ + i += 2 + case s[i] == '*' && i+1 < len(s) && s[i+1] == '/': + depth-- + i += 2 + default: + i++ + } + } + return i +} + +// skipDollarQuoted checks for a dollar-quoted string ($$...$$ or +// $tag$...$tag$) starting at s[start] and returns the index just past it. +// ok=false when s[start] starts a parameter placeholder ($1, $2, ...) or any +// other non-dollar-quote use of '$'. +func skipDollarQuoted(s string, start int) (end int, ok bool) { + i := start + 1 + if i < len(s) && isDollarTagStart(s[i]) { + i++ + for i < len(s) && isDollarTagChar(s[i]) { + i++ + } + } + if i >= len(s) || s[i] != '$' { + return 0, false + } + tag := s[start : i+1] // "$tag$" including both dollars + rest := strings.Index(s[i+1:], tag) + if rest < 0 { + return len(s), true // unterminated — consume the rest + } + return i + 1 + rest + len(tag), true +} + +func isDollarTagStart(c byte) bool { + return c == '_' || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') +} + +func isDollarTagChar(c byte) bool { + return isDollarTagStart(c) || (c >= '0' && c <= '9') +} + +// guardQuerier wraps a db.Querier and runs checkCrossModuleSQL before every +// statement. Applied to dev-pool connections only — production connections +// are enforced by Postgres itself (the per-(app,module) role's grants). +type guardQuerier struct { + q db.Querier + ownID string +} + +func (g guardQuerier) Exec(ctx context.Context, sql string, args ...any) (pgconn.CommandTag, error) { + if err := checkCrossModuleSQL(g.ownID, sql); err != nil { + return pgconn.CommandTag{}, err + } + return g.q.Exec(ctx, sql, args...) +} + +func (g guardQuerier) Query(ctx context.Context, sql string, args ...any) (pgx.Rows, error) { + if err := checkCrossModuleSQL(g.ownID, sql); err != nil { + return nil, err + } + return g.q.Query(ctx, sql, args...) +} + +func (g guardQuerier) QueryRow(ctx context.Context, sql string, args ...any) pgx.Row { + if err := checkCrossModuleSQL(g.ownID, sql); err != nil { + return errRow{err: err} + } + return g.q.QueryRow(ctx, sql, args...) +} + +// errRow satisfies pgx.Row for the QueryRow signature, which has no error +// return of its own — the guard rejection surfaces on Scan, same as pgx's +// own deferred-error rows. +type errRow struct{ err error } + +func (r errRow) Scan(...any) error { return r.err } + +// devGuardFor wraps q with the cross-module fail-fast guard when the +// invocation runs against the shared dev pool — detected exactly the way +// resolvePoolFor picks the pool: no platform credential in ctx. getCred is +// db.CredentialFrom for the app scope and db.ModuleCredentialFrom for the +// module scope, matching the credential the corresponding resolve used. +func (m *Module) devGuardFor(ctx context.Context, q db.Querier, getCred func(context.Context) *db.Credential) db.Querier { + if getCred(ctx) != nil { + return q + } + return guardQuerier{q: q, ownID: m.config.ID} +} diff --git a/internal/core/db_guard_test.go b/internal/core/db_guard_test.go new file mode 100644 index 0000000..51fcc50 --- /dev/null +++ b/internal/core/db_guard_test.go @@ -0,0 +1,195 @@ +package core + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" + + "github.com/mirrorstack-ai/app-module-sdk/db" +) + +// Realistic platform-minted module IDs (m<32 hex>), matching the physical +// table prefix convention used in app_ schemas. +const ( + guardOwnID = "m81b3ac7081c1409495700c761e23b59e" + guardForeignID = "m9238a60fa3c943039c28252b454b8071" +) + +func TestCheckCrossModuleSQL(t *testing.T) { + tests := []struct { + name string + sql string + wantErr bool + }{ + {"own table", `SELECT * FROM ` + guardOwnID + `_users WHERE id = $1`, false}, + {"own insert", `INSERT INTO ` + guardOwnID + `_sessions (id) VALUES ($1)`, false}, + {"no module tables", `SELECT 1`, false}, + {"plain table", `SELECT version FROM schema_migrations WHERE scope = $1`, false}, + {"own contributions table", `CREATE TABLE IF NOT EXISTS ` + guardOwnID + `_contributions (slot text)`, false}, + {"own module schema pin", `SET LOCAL search_path TO "mod_` + guardOwnID + `"`, false}, + + {"foreign table", `SELECT * FROM ` + guardForeignID + `_users`, true}, + {"foreign join", `SELECT a.id FROM ` + guardOwnID + `_orders a JOIN ` + guardForeignID + `_users u ON u.id = a.user_id`, true}, + {"foreign write", `UPDATE ` + guardForeignID + `_settings SET v = 1`, true}, + {"foreign schema-qualified", `SELECT * FROM app_283e0ef9.` + guardForeignID + `_users`, true}, + {"foreign quoted identifier", `SELECT * FROM "` + guardForeignID + `_users"`, true}, + {"foreign uppercase folds", `SELECT * FROM ` + strings.ToUpper(guardForeignID) + `_USERS`, true}, + {"foreign module schema", `SELECT * FROM mod_` + guardForeignID + `.outbox`, true}, + + // Regions where the pattern is meaningless must not trip the guard. + {"foreign name in string literal", `INSERT INTO ` + guardOwnID + `_logs (msg) VALUES ('saw ` + guardForeignID + `_users today')`, false}, + {"foreign name in escaped literal", `SELECT 'it''s ` + guardForeignID + `_users'`, false}, + {"foreign name in line comment", "SELECT 1 -- " + guardForeignID + "_users\n", false}, + {"foreign name in block comment", `SELECT 1 /* ` + guardForeignID + `_users */`, false}, + {"foreign name in nested block comment", `SELECT 1 /* outer /* ` + guardForeignID + `_users */ still comment */`, false}, + {"foreign name in dollar-quoted string", `SELECT $$` + guardForeignID + `_users$$`, false}, + {"foreign name in tagged dollar-quoted string", `SELECT $body$` + guardForeignID + `_users$body$`, false}, + + // The pattern must appear as an identifier start, not mid-word. + {"foreign id mid-identifier", `SELECT * FROM x` + guardForeignID + `_users`, false}, + {"own table with hex-looking suffix", `SELECT * FROM ` + guardOwnID + `_cache_9238a60fa3c943039c28252b454b8071`, false}, + + // A comment must not swallow real SQL after it. + {"foreign after block comment", `SELECT 1 /* c */ ; SELECT * FROM ` + guardForeignID + `_users`, true}, + {"foreign after literal", `SELECT 'x', u.* FROM ` + guardForeignID + `_users u`, true}, + {"params not dollar quotes", `SELECT * FROM ` + guardForeignID + `_users WHERE a = $1 AND b = $2`, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := checkCrossModuleSQL(guardOwnID, tt.sql) + if tt.wantErr { + if err == nil { + t.Fatalf("checkCrossModuleSQL(%q) = nil, want cross-module error", tt.sql) + } + if !errors.Is(err, errCrossModuleRelation) { + t.Fatalf("error %v is not errCrossModuleRelation", err) + } + return + } + if err != nil { + t.Fatalf("checkCrossModuleSQL(%q) = %v, want nil", tt.sql, err) + } + }) + } +} + +// TestCheckCrossModuleSQLErrorMessage pins the actionable parts of the +// rejection: the offending table, the owning module, and ms.DependencyDB as +// the sanctioned path. +func TestCheckCrossModuleSQLErrorMessage(t *testing.T) { + err := checkCrossModuleSQL(guardOwnID, `SELECT * FROM `+guardForeignID+`_users`) + if err == nil { + t.Fatal("want error, got nil") + } + for _, want := range []string{guardForeignID + "_users", guardForeignID, "ms.DependencyDB"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error message missing %q:\n%s", want, err.Error()) + } + } +} + +// stubQuerier records whether the underlying Querier was reached. +type stubQuerier struct{ called bool } + +func (s *stubQuerier) Exec(context.Context, string, ...any) (pgconn.CommandTag, error) { + s.called = true + return pgconn.CommandTag{}, nil +} + +func (s *stubQuerier) Query(context.Context, string, ...any) (pgx.Rows, error) { + s.called = true + return nil, nil +} + +func (s *stubQuerier) QueryRow(context.Context, string, ...any) pgx.Row { + s.called = true + return errRow{} +} + +func TestGuardQuerierBlocksForeign(t *testing.T) { + ctx := context.Background() + foreignSQL := `SELECT * FROM ` + guardForeignID + `_users` + + t.Run("Exec", func(t *testing.T) { + stub := &stubQuerier{} + g := guardQuerier{q: stub, ownID: guardOwnID} + if _, err := g.Exec(ctx, foreignSQL); !errors.Is(err, errCrossModuleRelation) { + t.Fatalf("Exec err = %v, want errCrossModuleRelation", err) + } + if stub.called { + t.Fatal("foreign Exec reached the underlying querier") + } + }) + + t.Run("Query", func(t *testing.T) { + stub := &stubQuerier{} + g := guardQuerier{q: stub, ownID: guardOwnID} + if _, err := g.Query(ctx, foreignSQL); !errors.Is(err, errCrossModuleRelation) { + t.Fatalf("Query err = %v, want errCrossModuleRelation", err) + } + if stub.called { + t.Fatal("foreign Query reached the underlying querier") + } + }) + + t.Run("QueryRow", func(t *testing.T) { + stub := &stubQuerier{} + g := guardQuerier{q: stub, ownID: guardOwnID} + row := g.QueryRow(ctx, foreignSQL) + if stub.called { + t.Fatal("foreign QueryRow reached the underlying querier") + } + var v int + if err := row.Scan(&v); !errors.Is(err, errCrossModuleRelation) { + t.Fatalf("QueryRow Scan err = %v, want errCrossModuleRelation", err) + } + }) +} + +func TestGuardQuerierPassesOwn(t *testing.T) { + ctx := context.Background() + stub := &stubQuerier{} + g := guardQuerier{q: stub, ownID: guardOwnID} + if _, err := g.Exec(ctx, `SELECT * FROM `+guardOwnID+`_users`); err != nil { + t.Fatalf("own-table Exec err = %v, want nil", err) + } + if !stub.called { + t.Fatal("own-table Exec never reached the underlying querier") + } +} + +// TestDevGuardFor pins the dev/prod split: no credential in ctx (the shared +// dev pool) wraps; a platform credential (production / deployed runtime) +// leaves the querier untouched — there the DB role's grants enforce. +func TestDevGuardFor(t *testing.T) { + m, err := New(Config{ID: guardOwnID}) + if err != nil { + t.Fatal(err) + } + stub := &stubQuerier{} + + devQ := m.devGuardFor(context.Background(), stub, db.CredentialFrom) + if _, ok := devQ.(guardQuerier); !ok { + t.Fatalf("dev ctx: got %T, want guardQuerier", devQ) + } + + prodCtx := db.WithCredential(context.Background(), db.Credential{ + Host: "h", Port: 5432, Database: "d", Username: "u", Token: "t", + }) + prodQ := m.devGuardFor(prodCtx, stub, db.CredentialFrom) + if prodQ != db.Querier(stub) { + t.Fatalf("prod ctx: querier was wrapped (%T), want untouched stub", prodQ) + } + + // The scope's own credential getter is what decides: a per-app credential + // must not disable the guard for the module-scope path. + moduleQ := m.devGuardFor(prodCtx, stub, db.ModuleCredentialFrom) + if _, ok := moduleQ.(guardQuerier); !ok { + t.Fatalf("module scope with only app credential: got %T, want guardQuerier", moduleQ) + } +} diff --git a/internal/core/expose.go b/internal/core/expose.go index 1280346..8e0bb96 100644 --- a/internal/core/expose.go +++ b/internal/core/expose.go @@ -1,10 +1,13 @@ package core -// ExposeTable marks a table in this module's `mod_` schema as eligible to -// be read (SELECT) by a depending module. It is a pure DECLARATION — no -// runtime, no return value — recorded in the manifest under -// `exposes.tables` so the platform catalog can issue GRANT SELECT after the -// app owner approves a dependency. +// ExposeTable marks one of this module's per-app tables as eligible to be +// read (SELECT) by a depending module installed in the same app. Declare the +// bare table name; the platform resolves it to the physical form — +// app_."" in each app's tenant schema — when granting. +// It is a pure DECLARATION — no runtime, no return value — recorded in the +// manifest under `exposes.tables` so the platform catalog can issue GRANT +// SELECT after the app owner approves a dependency. Cross-app module state +// (the mod_ schema) is NOT shareable this way. // // v1 is TABLES ONLY, read-only. The producer marks a table READABLE; it does // NOT name WHO reads it. The app owner — the trust root — decides which diff --git a/internal/core/need.go b/internal/core/need.go index 52b2bdd..06b266f 100644 --- a/internal/core/need.go +++ b/internal/core/need.go @@ -10,11 +10,12 @@ type Need struct { events []string } -// Table records a relation name from the dep's `mod_` schema as a -// SELECT request. The catalog validates the name against the dep's -// schema at install time (introspecting `pg_class`); after app-owner -// approval, the platform issues GRANT SELECT against this consumer's -// per-app DB role. +// Table records a bare relation name from the dep's per-app tables as a +// SELECT request — the physical target is app_."
" in +// the shared app tenant schema, not a cross-app mod_ schema. The +// catalog validates the name against the dep's exposed tables at install +// time; after app-owner approval, the platform issues GRANT SELECT +// against this consumer's per-app DB role. // // ms.DependsOn("@anna/oauth@^0.4.0", func(n *ms.Need) { // n.Table("oauth_users")