Repository navigation
fix: tenancy, boot-config, audit and session-cookie bug batch (#494 #495 #497 #498 #499 #503 #506 #554) - #556
Merged
Merged
Conversation
…pin it with a test (#503) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
…gs are unreadable (#494) A failed pre-app read used to log at DEBUG and build with defaults, so a worker booting during a DB blip ran single-tenant (multi_tenant off) next to strict siblings. A missing settings table (fresh install) stays quiet; a connect/query failure now warns, and outside development raises HostSettingsUnavailableError when the settings module is installed. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
…ashing (#497) merge_host_settings now builds Settings leniently: a stored value that parses but fails validation (log_format='xml') falls back to its default with a WARNING, keeping the other overrides. The parser and the lenient loop move to simple_module_core.stored_settings, shared with settings.hydrate (which re-exports RELATED_FIELDS_CTX). The i18n default-locale model error now names both fields so the stored half can be dropped. The stored secret key rides along in the overrides read, so an established install opens one pre-app connection instead of two. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
multi_tenant, tenant_header, default_tenant, i18n_*, auth_provider, auth_public_paths, trusted_proxy, log_level and log_format are consumed while create_app (or host/main.py) builds the app, so an admin-UI edit only lands after a restart. They now carry the restart marker, which the settings editor turns into a 'Requires restart' hint. A test builds the app on a recording Settings and asserts the marked set equals the set create_app reads, in both directions. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
New HostSettings.session_cookie_secure (SM_SESSION_COOKIE_SECURE, restart to apply). Unset means Secure everywhere but development/testing, which keep working over http://localhost; SessionMiddleware now gets https_only. Deployment docs cover TLS terminators (leave it on, set SM_TRUSTED_PROXY) and plain-HTTP installs (set it false). Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
…498) The audit callback lived in a module-level DatabaseState global that only create_app set, so worker writes were never audited and the last app built in a process won. - listeners read audit_callback from the policy bound to the flushing session's engine (DatabaseState / EngineTenancy), like tenancy does. - EngineTenancy gains audit_callback; simple_module_db.audit_sinks discovers sinks from the new "simple_module.audit_sinks" entry point, which audit_log publishes. background_tasks binds it to both worker engines (sync_db and the outbox worker) without importing audit_log. - correlation_id ContextVar moves to simple_module_core.correlation (re-exported by simple_module_hosting.logging): db no longer imports hosting. - Docs: worker capture and known gaps (bulk/Core DML, text() DML, outbox bulk UPDATEs). Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
Follow-up to #494/#497 from review: - Stored booleans parse strictly; an unknown token ('tru') is dropped with a warning instead of reading as False. - Only a missing settings package means 'not installed'; any other import error is a pre-app read failure (strict outside development). - SM_SECRET_KEY resolution falls back to an ephemeral key only on a confirmed fresh install (no settings table); outside development a failed lookup/save or no settings module refuses boot. A blank stored key is repaired atomically instead of forcing an ephemeral key forever. - A failed module hydrate rolls the shared session back so later modules still read (Postgres aborted-transaction poisoning) and keeps that module's current settings. - A raising required setup step counts as first run only when the DB has no migration history. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
…enant (#506) A pure Core outer join over Model.__table__ got no tenant or soft-delete criteria in ON or WHERE, so aggregates counted other tenants' rows. A Codex review found the same gap for other Core shapes: table aliases, subqueries, CTEs, scalar subqueries, union/intersect/except members, the extra FROM of an UPDATE/DELETE, INSERT ... FROM SELECT sources, and exists() over a join or inside an ON clause. - core_scope: one walk detects a plain (non-ORM) reference to a scoped table; only then is the statement cloned once and every SELECT in it, innermost first, gets predicates for the tables it reads. UPDATE/DELETE get predicates for the tables their WHERE joins in. Replaces subquery_guard. - _core_joins: a LEFT join's nullable side gets its predicates in ON; a FULL join gets both sides' predicates in ON plus WHERE (pred OR pk IS NULL) per side, which gives the same result as filtering both inputs first. Nested joins, aliases and select().outerjoin(table, on) are covered. - Strict mode with no tenant still raises TenantIsolationError for every Core reference; include_deleted / all_tenants are honoured. - Remove the strict xfail; add tests for every shape (SQLite + Postgres). - Docs: list what is covered, and that text() and soft-delete on DML are out of scope by design. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
…g settings-db seeder, db test session builder - _base_name/_is_orm move to _core_joins; is_plain_leaf and predicates_for use them (is_plain_leaf keeps its own Alias loop: it needs the base element, not its name) - query_filter calls engine_policy directly (drop _policy alias) - framework/hosting/tests/_hosting_settings_db.py: one settings_setting seeder, unmigrated-DB helper and connection-refused coroutine - db tests: _tenant_session takes metadata + optional seed, lives in _models.py; seeded builds on it; _rows shared from _core_scope_models - host_settings: _RESTART_SECURITY constant Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
…and restated correlation comment SidebarLayout plain-props rewrite skipped: Biome's a11y rule rejects a static aria-modal on <aside> without a static dialog role. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
- openapi export builds settings with merge_host_settings(strict=False), so an unreachable database no longer fails a one-shot export. - construct_leniently retries until no stored culprit remains (multi-pass), so a cross-field rule that only trips once the first bad value is dropped is handled too. - #495: warn once per process (WARNING, naming SM_TRUSTED_PROXY and SM_SESSION_COOKIE_SECURE) when a request reaches the app over plain http for a non-loopback host while the session cookie is Secure; symptom documented in docs/reference/deployment.md. - #506: a FULL .outerjoin(..., full=True) step now keeps its left side: the plain tables of the FROM built so far get (pred OR pk IS NULL) in that step's ON and in WHERE instead of a plain WHERE that made it a RIGHT join. A step without an explicit ON uses the onclause SQLAlchemy infers (also for LEFT steps, which no longer collapse to inner). Tested on SQLite and Postgres; docs/framework/multi-tenancy.md lists the shapes that still over-filter. - #498: OutboxMessage sets __audit_exclude__; TaskExecution excludes heartbeat_at, queued_at, started_at, finished_at and retries, so updates touching only those write no audit row while creation and status changes are still audited. Docs and CHANGELOG updated. - useDrawerA11y closes the drawer (releasing the focus trap and aria-modal) when the viewport crosses the lg breakpoint, and the trap skips focusables that are not rendered, [hidden] or [inert]. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
Deploying simple-module-python with
|
| Latest commit: |
00d2782
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a0f0ff07.simple-module-python.pages.dev |
| Branch Preview URL: | https://fix-bug-batch-494-554.simple-module-python.pages.dev |
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. |
This was referenced Oct 19, 2026
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
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 #494
Closes #495
Closes #497
Closes #498
Closes #499
Closes #503
Closes #506
Closes #554
Security
Tenancy: Core outer join leaves the tenant-scoped right side unfiltered (cross-tenant counts) #506 — tenancy filter. The tenant and soft-delete filters now cover Core statements that used to return other tenants' rows or deleted rows. An independent Codex review confirmed each of these leaked on
mainfirst:Model.__table__, including setup joins and steps without an explicit ON;exists()over joins or inside ON clauses;union/intersect/except;insert().from_select()sources.The work lives in
core_scope.pyand_core_joins.py, replacingsubquery_guard.py. ORM-only statements are returned untouched, and strict mode still fails closed. Rawtext()stays unfiltered by design. Two shapes over-filter but never leak, and both are documented: an ORM entity on the left of a FULL step, and a table without a primary key.Session cookie is never marked Secure #495 — Secure session cookie. The session cookie is
Secureoutside development and testing, through the newsession_cookie_securesetting (SM_SESSION_COOKIE_SECURE). If a production install is reached over plain HTTP, it now logs one WARNING per process namingSM_TRUSTED_PROXYandSM_SESSION_COOKIE_SECURE. Before, sign-in would silently fail.Tenancy fails open when the pre-app host-settings read hits a transient DB error #494 — boot fails closed. A failed pre-app settings read is no longer mistaken for a fresh install.
HostSettingsUnavailableErrorrather than quietly building single-tenant.SecretKeyUnavailableError). An ephemeral key is used only before the settings table exists, and a blank stored key is repaired atomically.Correctness
"tru"was read as false. There is now one sharedsimple_module_core.stored_settingsmodule. One failing module settings read no longer resets that module, and on Postgres it no longer poisons the shared session.HostSettingsfield thatcreate_appreads is marked restart-required. A test checks both directions: every field read is marked, and every marked field is read.simple_module.audit_sinksentry-point group.background_tasksnever importsaudit_log.OutboxMessageis excluded from auditing.TaskExecutionaudits its creation and status changes but not its heartbeat or timestamps.docs/database/migrations.mdnow describe the real graph: two roots, four heads, and inherited labels. A test pins that shape. No revision file was changed.Verification
make lint,make test-js,make doctorand the full Python suite (4800 passed) all pass. The tenancy tests were also run on Postgres with a disposable database.openapi_cmdstrict=False.log_format='xml'andmaintenance_mode='tru'stored, and a smoke test of every admin page withmulti_tenanton. Report: https://claude.ai/artifact/H2MQPJzCxjRg3tpjRK2SnP. It is a private artifact; share it if reviewers need access.SM_ENVIRONMENT=productionoverhttp://localhost. It relies on browsers keeping Secure cookies on localhost; CI will confirm.https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4