Skip to content

fix(db): scope secondary-table loaders, aliased selectables and attached objects; subqueryload works on scoped models - #822

Merged
antosubash merged 6 commits into
mainfrom
fix/db-review-read-scope
Nov 4, 2026
Merged

antosubash merged 6 commits into
mainfrom
fix/db-review-read-scope

Conversation

@antosubash

Copy link
Copy Markdown
Owner

Summary

  • Tenant and soft-delete scoping in simple_module_db now also covers these shapes:
    • Relationship secondary tables mapped by MultiTenantMixin / SoftDeleteMixin models, in joinedload(), selectinload(), subqueryload() and joins through the relationship. Lazy loads were already covered. Writes through the relationship skip the soft-delete condition, like every write. A bare association Table is not filtered.

    • aliased(Model, <Core selectable>): the selectable is read and checked. Outside all_tenants(), three kinds of selectable raise TenantIsolationError:

      • one that reads other scoped tables, including through a Join;
      • one built from SQL text;
      • one that combines rows (GROUP BY, aggregates, window functions, functions not on the row-wise allowlist).

      The entity's own tables and plain untenanted tables stay allowed.

    • Objects attached detached or merged (add() of a detached object, merge(load=False), make_transient_to_detached) are checked against the stored owner before flush. This costs one SELECT per identity, and loaded or inserted objects cost nothing.

    • Sessions on derived engines (engine.execution_options(...)), per-model binds={...} and overridden get_bind() keep the engine's strict mode, fallback tenant and audit sink.

    • Core statements on Model.__table__ are scoped even before any ORM use has configured the mappers.

  • db: subqueryload() on filtered models fails with a duplicate anon subquery alias #816: subqueryload() on scoped models no longer fails with a duplicate subquery alias.
  • Behaviour change:
    • the refused aliased() shapes above now raise;
    • flushing an attached object whose stored owner is another tenant raises;
    • simple_module_db now requires SQLAlchemy ≥ 2.0.2.
  • Docs (docs/framework/multi-tenancy.md, docs/database/mixins.md), the database skill and the CHANGELOG describe what is covered and what is refused.

Verification

  • Local CI: make lint passed, along with these suites:
    • make test-py: 7806 passed
    • make test-js: passed
    • make doctor: clean
    • e2e: 98 passed against a live API + Vite
  • framework/db/tests on a disposable Postgres database: 768 passed.
  • Not browser-tested: no UI changes.
  • Verification report: https://claude.ai/artifact/HYn14x2XEzui7siLNCSpSU

Report links are Claude Artifacts and start private; the author can share them from claude.ai/code/artifacts if reviewers need access.

Closes #816

Test plan

  • CI is green, including the Postgres workflow (this PR touches framework/db)

https://claude.ai/code/session_01FXHA3ZicrtZ4C5QPEgxfgq

…mory persistent objects (#816)

Tenant and soft-delete scoping now also applies to:

- relationships whose secondary is a MultiTenantMixin / SoftDeleteMixin
  table, in joinedload, selectinload, subqueryload and joins through the
  relationship: the secondaryjoin carries criteria bound per execution
  (_secondary_scope);
- Core statements on Model.__table__ before the mappers configure: tables
  register when the mapper is constructed;
- sessions on engine.execution_options(...) or bound per model (binds=...):
  the engine policy resolves through the base engine (_engine_policies).

Behaviour changes (TenantIsolationError):

- flushing an UPDATE / DELETE of an object made persistent in memory
  (make_transient_to_detached, merge(load=False)) reads its row's tenant_id
  first and refuses a row of another tenant (_persisted_owner);
- aliased(Model, <subquery or CTE>) whose selectable reads a scoped table
  through Core, other than the entity's own table, is refused outside
  all_tenants(); built from ORM entities it is scoped as before.

subqueryload() no longer fails with a duplicate anon_1 alias: the
selectable of an aliased entity is never cloned (#816).

Claude-Session: https://claude.ai/code/session_01FXHA3ZicrtZ4C5QPEgxfgq
- aliased own-rows check shares the entity's own tables with the join check,
  so a joined-inheritance entity's subquery over its mapper's tables is allowed
- the own-rows shortcut trusts only a small set of row-wise functions; any
  other function declines it
- a missing row is not memoized by the flush owner check; a found owner still is
- session_policy skips unmapped classes among the tenant models
- policy bindings invalidate memoized session policies

Claude-Session: https://claude.ai/code/session_01FXHA3ZicrtZ4C5QPEgxfgq
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Deploying simple-module-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: 03b6235
Status: ✅  Deploy successful!
Preview URL: https://a5f425df.simple-module-python.pages.dev
Branch Preview URL: https://fix-db-review-read-scope.simple-module-python.pages.dev

View logs

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
🔒 Security Review ✅ Completed 2026-11-04T16:49:46.559193Z 286d226 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@antosubash
antosubash merged commit c4cee9a into main Nov 4, 2026
17 checks passed
@antosubash
antosubash deleted the fix/db-review-read-scope branch November 4, 2026 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

db: subqueryload() on filtered models fails with a duplicate anon subquery alias

1 participant