Repository navigation
Tenancy walk perf + WHERE-only leak fix, PermissionDenied, TenantDeleted, validator related fields, wait_for, tooling checks, test gaps (#491 #512 #514 #518 #520 #558 #559) - #560
Merged
Conversation
#520) Add framework/cli and framework/inertia to ty extra-paths; check_metadata.py now fails when extra-paths or testpaths drift from framework/* and modules/*. Runs in make lint and the python-lint CI job. new_module.py already appends to both lists. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
…ants shared props, upload guards (#514) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
) RequiresPermission raises PermissionDenied(HTTPException) with .permission; the 403 page reads it via isinstance instead of slicing the message. The JSON body and page props are unchanged. PERMISSION_DENIED_PREFIX is kept (now in simple_module_core.permissions, still re-exported from simple_module_hosting.permissions) for external importers; the parser is gone. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
Adds TenantService.delete_tenant (there was no delete path), which removes the tenant with its memberships and invitations and queues TenantDeleted (tenant_id, slug, name) on commit like TenantStatusChanged. Documented with the other tenant events. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
# Conflicts: # .github/workflows/pr.yml
The walk that decides whether a statement holds a Core reference runs on every ORM query while tenancy names are set. It now expands fewer nodes, more cheaply, without changing which elements are reachable — so the set of statements that get scoped is unchanged: * each element is expanded once (a FROM named by columns and joins); * bound parameters, which have no clause children, are not expanded; * a SELECT's declared children skip its empty collections, and no longer repeat its FROM list or (under the ORM plugin, with an all-entity columns clause) its columns-clause FROMs, which the walk prunes anyway; * `table` is looked up without the AttributeError a column expression's __getattr__ raises and formats for it. The walk moves to `_core_walk.py`. Checked against the previous walk on every statement the db and tenants suites execute (2347 calls, no disagreement). `scripts/bench_core_scope.py` times it: an uncloned ORM select drops from 25-86 µs to 14-44 µs (get 26 -> 15, pagination page 34 -> 17, pagination count 83 -> 44). New tests pin that Core columns and subqueries inside an otherwise-ORM select (columns, WHERE, HAVING, ORDER BY, joins) stay filtered, and refuse under strict mode with no tenant. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
Pre-existing tenant leak found by the #559 coverage tests. An ORM select that reads a raw `Model.__table__` only through its WHERE — `select(Tag.name).where(link.c.tag_id == Tag.id, link.c.article_id == 7)`, also with an `aliased()` entity — pulls that table into FROM, but the scope pass took WHERE FROMs only for ORM-less selects. The table was detected, the statement cloned, and no predicate added: tenant A saw a row through tenant B's link, and strict mode with no tenant did not refuse. An ORM select now also gets WHERE predicates for the FROMs its WHERE pulls in through plain columns. ORM-annotated elements and nested selects are pruned (an entity column's `_from_objects` is its bare table, and an aliased entity's would gain a stray cartesian FROM). The #417 outer-join exclusion and the joins' covered set still apply. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
…ssion-denied tests, one db stub - background_tasks/tests-js: one 204-aware stubFetch in stub-fetch.ts - api_tokens/tests-js: token()/stubFetch in fixtures.ts - drop per-file afterEach cleanup() (RTL auto-cleanup, globals: true) and the jest-dom import where the module tsconfig does not typecheck tests-js - move the PermissionDenied tests into TestRequiredPermissionProp - drop duplicated tenants cache fixture and scripts sys.path insert - file_storage upload guards: one _db_on() session stub Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
…pler sleep lint entry - permissions.py: one-line simple_module_core.permissions import - waiting.py: single-expression detail; wait_for/wait_for_sync share _poll_once - check_test_sleeps.py: drop the unused argv root option, use Path.cwd() - test_bg_invalidation: asyncio.sleep(timeout or 0) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
- tenants: delete_tenant now publishes MembershipRemoved for every member (collected before the bulk delete) alongside TenantDeleted, after commit only; the existing global membership-cache invalidation still runs post- commit, and a rollback publishes and invalidates nothing. Tests cover both. - core: test_publish_nowait yields to the loop after the wait so a duplicate delivery would be caught by the exactly-one assertion. - testing: wait_for bounds an async predicate's await by the remaining timeout; wait_for_sync rejects async predicates. - scripts: check_test_sleeps parses with ast, so a sleep split over lines or after a '#' inside a string is still flagged (regex fallback on bad syntax). Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Deploying simple-module-python with
|
| Latest commit: |
1d3b6b7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d731c5af.simple-module-python.pages.dev |
| Branch Preview URL: | https://chore-batch-512-559.simple-module-python.pages.dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #491
Closes #512
Closes #514
Closes #518
Closes #520
Closes #558
Closes #559
Changes
scripts/bench_core_scope.py.main: an ORM select that reads a rawModel.__table__, or analiased()entity's table, only in its WHERE clause got no tenant filter. Example:select(Tag.name).where(link.c.tag_id == Tag.id, …). Strict mode also didn't refuse it without a tenant. It is now filtered and refused, with 21 tests covering Core inside ORM selects. Checked on SQLite and on Postgres (a disposable DB, dropped afterwards). This is the same class of leak as Tenancy: Core outer join leaves the tenant-scoped right side unfiltered (cross-tenant counts) #506.PermissionDenied.simple_module_core.permission_denied.PermissionDeniedis anHTTPExceptionthat carries.permission. The 403 handler reads that attribute instead of parsing the message string. The JSON body and the page props are unchanged.TenantDeleted. NewTenantDeletedevent andTenantService.delete_tenant. After commit it publishes oneMembershipRemovedper member, thenTenantDeleted, and clears every worker's membership cache. A rollback publishes nothing. No endpoint or UI yet, because what deleting a tenant should mean for other modules' rows is still undecided.field_error. Lenient hydration then drops only the stored value that broke the check, and the settings editor shows the error on a field. The users and keycloak production-secret checks still refuse, on purpose: dropping the value would fall back to the same unsafe default.wait_forhelper #512,wait_for. Newsimple_module_test.wait_for/wait_for_sync. 10 fixed sleeps are now waits on a condition, and 12 are marked# intentional-sleep:.make ci-check-test-sleepsflags new unmarked sleeps, inmake lintand CI.scripts/check_metadata.pychecks that the tyextra-pathsand pytesttestpathslists match the packages on disk, inmake lintand CI. It also adds the missingframework/cliandframework/inertiaentries.tenants/shared_props.pyandfile_storage/upload_guards.py.Verification
make lint,make test-jsand the full Python suite (4883 passed) all pass.wait_for).wait_forand sleep-lint details, the per-member side effects ofdelete_tenant, and an events test that couldn't catch a duplicate delivery.multi_tenantwas off.file_storage.public_uploadsrow showed SYSTEM "off" while EFFECTIVE showed "Enabled".https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4