fix: security and settings bug batch (#517, #532, #533, #543, #544, #545, #547, #548) by antosubash · Pull Request #551 · antosubash/simple_module_python

antosubash · 2026-10-19T05:47:41Z

Fixes a batch of open bugs, most of them security issues found while reviewing and testing #525.

Closes #517
Closes #532
Closes #533
Closes #543
Closes #544
Closes #545
Closes #547
Closes #548

Behaviour changes

file_storage: list/get/download/visibility endpoints have no owner check (any user role sees every file) #544 file_storage owner scope. A caller without file_storage.manage can list, get, download, thumbnail, change visibility and delete only files they uploaded. Anyone else's file is a 404. manage is held by admins (*) and by tenant:admin and tenant:owner. Files with no recorded uploader are now visible to managers only. Internal callers such as branding are unscoped.
file_storage: ACTIVE_CONTENT_TYPES misses scriptable XML types and malformed content types #545 inline serving uses an allow-list. Only raster images, PDF, JSON/GeoJSON, plain text, CSV, opaque binaries and audio/video/font are served inline. Everything else is served as a sandboxed attachment, including any *+xml, text/xsl, and any malformed value (such as a comma or quote anywhere).
users: OAuth sign-in provisions new accounts even when allow_signup is off #543 OAuth signup gate. While allow_signup is off, OAuth refuses to create accounts with a 403 OAUTH_SIGNUP_DISABLED. Existing linked accounts and accounts matched by email still sign in.
users: require_verification setting is never read — unverified signups can never log in #547 require_verification. While the setting is off, no verification email is sent and password login doesn't require verification; the register page copy follows the setting. To prevent pre-hijacking, OAuth now refuses to link by email to an unverified account that has a password (403 OAUTH_ACCOUNT_NOT_VERIFIED). Pending invites still link.
auth: Bearer scheme matching is case-sensitive, but /auth/token returns token_type "bearer" #533 Bearer scheme. The scheme is parsed case-insensitively through one helper, auth.contracts.tokens.parse_bearer, used by the users, keycloak and api_tokens resolvers. /auth/token returns token_type="Bearer".

Fixes

PermissionRegistry caches a failed runtime source as empty until invalidated #517 A failing permission source is retried after 30s instead of being cached as empty until invalidated, and its traceback is logged once per window.
background_tasks: cached sync engine is not reset after Celery prefork, so children can share inherited DB connections #532 The sync engine records the PID that built it. A prefork child discards the inherited engine with dispose(close=False), both on a PID mismatch and on worker_process_init.
Settings validation gaps: negative file_storage quota_bytes, soft task time limit above the hard limit #548
- quota_bytes must be between 1 and 2^53-1.
- task_time_limit must be greater than task_soft_time_limit.
- During lenient hydration, a validator can name related fields. If the hard limit comes from an env var and the soft limit from the DB, only the stored soft limit is dropped, instead of falling back to defaults for the whole module or stopping the worker from booting.

Review

Each cluster was implemented in its own worktree, then the batch got an adversarial security review.
The review found and this PR fixes:
- An account pre-hijacking path through users: require_verification setting is never read — unverified signups can never log in #547 plus OAuth email association.
- A boot failure in Settings validation gaps: negative file_storage quota_bytes, soft task time limit above the hard limit #548 when the hard limit comes from env and the soft limit from the DB.
- Log flooding from PermissionRegistry caches a failed runtime source as empty until invalidated #517.
- A comma-in-parameter parser differential in file_storage: ACTIVE_CONTENT_TYPES misses scriptable XML types and malformed content types #545 (image/png;x=1,text/html).
Not in this PR: users: login lockout is per-process and keyed by username+IP, so it does not hold across workers or source IPs #530, the login lockout that holds across workers. It needs Redis with a DB fallback, an admin UI and audit logging.

Verification

make lint: pass.
make test-js: pass.
Full Python suite: 4633 passed, 21 skipped.
The pg_backend_pid() fork test for background_tasks: cached sync engine is not reset after Celery prefork, so children can share inherited DB connections #532 is marked @pytest.mark.postgres. It passed once against the local Postgres; CI's SQLite run skips it.

/ship

Optimize: removed 30 net lines in 2 commits. Shared test helpers; one OwnerScope dependency; the engine PID resets inside _drop_engine; Register picks its copy once; _culprits is typed.
Code review: clean in 2 passes. Fixes:
- /auth/token now refuses unverified accounts while require_verification is on. It used to mint bearer tokens for them, so this is now in the CHANGELOG.
- all_permissions is cached until a failing source's retry instead of being rebuilt on every check.
Browser QA (fresh install): 38 checks passed, 0 P0–P2. QA report: https://claude.ai/artifact/LhRnmh1P6P34LM1ygRBVZU (a private artifact; share it from claude.ai if reviewers need it). P3 notes:
- The browse page's usage figure is tenant-wide.
- The quota inline error wraps one word per line.
- Clearing the quota keeps a null override row.
- The authenticated download uses the global CSP (still an attachment with nosniff).
Local CI: lint, JS and Python all pass (4636 passed).

https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

Record the creating PID; rebuild with dispose(close=False) on mismatch and drop inherited engines on worker_process_init. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

…#548) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

Add auth.contracts.tokens.parse_bearer(), the one reader of an Authorization: Bearer header: the scheme is matched case-insensitively (RFC 9110 §11.1) and surrounding whitespace is tolerated. The users and keycloak providers and the api_tokens resolver all use it, so a client that builds its header from /auth/token's token_type authenticates. /auth/token now returns token_type="Bearer". Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

…on (#543) The OAuth callback now checks, before find-or-create, whether the identity already belongs to an account (linked OAuth account, then email association). With allow_signup off and no such account it answers 403 OAUTH_SIGNUP_DISABLED, the same error-code shape as the callback's other refusals; existing accounts still link and sign in. The fake OAuth provider used by test_external_users moves to a shared _users_oauth_support helper. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

With users.require_verification off, on_after_register marks a self-registered account verified and sends no verification email, so the new user can sign in at once. Request-less creates (admin, invite) and OAuth provisioning are unchanged; with the setting on, behaviour is as before. The register view passes require_verification to the page, whose aside bullet and success screen now follow it (new users.register keys, i18n keys regenerated). attach_default_role moves to users/registration.py to keep manager.py under the 300-line cap. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

…7-548

Every authenticated file route (list, get, download, thumbnail, publish, delete, bulk delete) and the browse screen now act only on rows the caller uploaded (created_by == current user id) unless they hold file_storage.manage. Another user's id answers 404 like an unknown one. The scope is resolved by deps.get_owner_scope and passed to the service as an explicit owner= argument, so internal callers (branding, reapers) stay unscoped and anonymous public serving is untouched. file_storage.manage is now mapped to tenant admins and owners; platform admins hold it via *. Browse facets for scoped callers come from their own rows only. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

Replace the ACTIVE_CONTENT_TYPES deny-list, which missed scriptable XML (atom/rss/mathml +xml, text/xsl, vnd.wap.xhtml+xml) and let odd casing, parameters or malformed values through, with an allow-list of types safe to serve inline: common raster images, PDF, JSON, text/plain, text/csv and audio/* or video/* (minus any xml/html subtype). The stored type is lower-cased and stripped of parameters; anything that is not exactly one type/subtype token pair (text/html,) is treated as active rather than cleaned up, so it is served as a sandboxed attachment and streamed. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

quota_bytes accepted -1, 0 and 2**63. It is now Field(gt=0, le=2**53 - 1): None still means unmeasured, and the upper bound keeps the value exact in the browse screen and settings editor, which read it as a JS number. Stored out-of-range rows are dropped by the existing lenient hydration. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

…545) application/octet-stream (PMTiles), zip, gzip, protobuf / vector tiles, GeoJSON and font/* are never rendered as documents, and treating them as active would stream S3 range reads through the app instead of redirecting to the presigned URL. Caught by test_public_route_serves_ranges_with_its_headers. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

…7-548

… it active (#545) The check read the part before ';', but a browser splits the header on commas first: image/png;x=1,text/html was classified inline-safe and rendered as HTML. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

…ng; OAuth won't link unverified accounts (#547) Self-registration with users.require_verification off no longer marks the account verified. Password login skips the verification requirement while the setting is off, so earlier signups get in too. The OAuth callback refuses to associate by email with an unverified, self-registered password account (403 OAUTH_ACCOUNT_NOT_VERIFIED), closing an account pre-hijack: an attacker registering the victim's address would otherwise share the victim's account once they signed in with SSO. Already-linked identities, verified accounts, external accounts and pending invites still sign in. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

…ld rule (#548) The background_tasks soft/hard time-limit rule is reported on task_time_limit so the editor shows it on that field. With the hard limit pinned by env and the soft limit stored, lenient hydration had no DB key to drop and re-raised: the web lost every background_tasks override and the worker refused to start. A validator can now name the other fields it depends on under ctx[RELATED_FIELDS_CTX]; when the reported field did not come from the DB, lenient hydration drops those instead and logs it. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

…every read (#517) A permanently failing runtime permission source was called, and logged a full traceback, on every read of the registry — about once per request. The failure is now remembered for SOURCE_RETRY_SECONDS (monotonic clock, injectable for tests): reads in that window answer empty without calling it, and the traceback is logged once per window. A transient failure still recovers without invalidate_source, which now also clears the failure. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

chatgpt-codex-connector · 2026-10-19T05:48:01Z

Codex Review Summary

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

Review	Status	Commit	Review trigger
🔒 Security Review	✅ Completed 2026-10-19T05:53:54.287811Z	`33dc3aa`	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.

cloudflare-workers-and-pages · 2026-10-19T05:49:41Z

Deploying simple-module-python with Cloudflare Pages

Latest commit:	`9f2c4d5`
Status:	✅ Deploy successful!
Preview URL:	https://0b3b874f.simple-module-python.pages.dev
Branch Preview URL:	https://fix-bug-batch-517-548.simple-module-python.pages.dev

View logs

…, reuse base_type - background_tasks: fold the #548 env-pin tests into the runner/settings suites - users tests: find_user, seed_password_user, set_users_settings in the shared support module - file_storage: OwnerScope defined once in deps; media_types reuses thumbnails.base_type; owner-scope tests use the shared upload helpers Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

…ne, one Register copy object, typed _culprits Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

/auth/token refuses unverified accounts while require_verification is on; the all_permissions snapshot is cached until a failing source's retry instead of rebuilt per check. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

…gs defaults, inline errors) (#552) * fix(file_storage): show owner-scoped callers their own usage, not the tenant's Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix(file_storage): sandbox CSP on streamed authenticated downloads Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix(settings): show a field's validation error on its own row below the control Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix(settings): saving a field's default removes its override row Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix(settings): a no-op save of a default removes its stale override row Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * docs: changelog for the #551 QA follow-ups Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

…gs defaults, inline errors) (#553) * fix(file_storage): show owner-scoped callers their own usage, not the tenant's Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix(file_storage): sandbox CSP on streamed authenticated downloads Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix(settings): show a field's validation error on its own row below the control Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix(settings): saving a field's default removes its override row Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix(settings): a no-op save of a default removes its stale override row Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * docs: changelog for the #551 QA follow-ups Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

antosubash added 18 commits October 19, 2026 07:04

fix(core): don't cache a failed permission source as empty (#517)

6d1c2c2

Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

fix(background_tasks): don't share the sync engine across a fork (#532)

0037d02

Record the creating PID; rebuild with dispose(close=False) on mismatch and drop inherited engines on worker_process_init. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

fix(background_tasks): require task_soft_time_limit < task_time_limit (…

a0dfbaf

…#548) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

style(background_tasks): ruff format/import order for #532

086bf6c

Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

Merge branch 'worktree-agent-ab2c98c7c47ea61b5' into fix/bug-batch-51…

381ca60

…7-548

Merge branch 'worktree-agent-adb52b41740a2a08c' into fix/bug-batch-51…

b684bf4

…7-548

docs: changelog for #517, #532, #533, #543, #544, #545, #547, #548

005bb40

Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

antosubash added 3 commits October 19, 2026 08:40

refactor(optimize): over-engineering — reset engine pid in _drop_engi…

d190b83

…ne, one Register copy object, typed _culprits Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4

antosubash merged commit 1dd1bc1 into main Oct 19, 2026
14 checks passed

antosubash mentioned this pull request Oct 19, 2026

fix: QA follow-ups for #551 (owner-scoped usage, download CSP, settings defaults, inline errors) #552

Merged

antosubash mentioned this pull request Oct 19, 2026

fix: QA follow-ups for #551 (owner-scoped usage, download CSP, settings defaults, inline errors) #553

Merged

Navigation Menu

Repository navigation

fix: security and settings bug batch (#517, #532, #533, #543, #544, #545, #547, #548) - #551

fix: security and settings bug batch (#517, #532, #533, #543, #544, #545, #547, #548)#551
antosubash merged 21 commits into
mainfrom
fix/bug-batch-517-548

antosubash commented Oct 19, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented Oct 19, 2026 •

edited

Loading

Uh oh!

cloudflare-workers-and-pages Bot commented Oct 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

antosubash commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Behaviour changes

Fixes

Review

Verification

/ship

Uh oh!

chatgpt-codex-connector Bot commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

cloudflare-workers-and-pages Bot commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying simple-module-python with Cloudflare Pages

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

antosubash commented Oct 19, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 19, 2026 •

edited

Loading

cloudflare-workers-and-pages Bot commented Oct 19, 2026 •

edited

Loading