Skip to content

fix(settings): confine settings scopes to the caller's own on multi-tenant hosts (#368) - #379

Closed
antosubash wants to merge 2 commits into
mainfrom
fix/368-settings-tenant-scope
Closed

antosubash wants to merge 2 commits into
mainfrom
fix/368-settings-tenant-scope

Conversation

@antosubash

Copy link
Copy Markdown
Owner

Closes #368.

Why

The settings routes only checked settings.view, settings.edit and settings.delete. On a multi-tenant host, anyone holding them could:

  • write the host-wide system scope, where every module's DB-backed configuration lives;
  • write any tenant's or user's scope by putting its id in the URL.

The issue's repro: acme's admin turned on a records setting for the whole host, and wrote a setting into globex's scope.

What changed

  • New permission settings.system, plus settings/scope_guard.py.
  • With multi_tenant on, settings.* lets a caller manage:
    • its own tenant scope (the request's tenant);
    • its own user scope.
  • Everything else needs a platform settings admin, meaning a caller that holds settings.system. This covers:
    • the system scope;
    • other tenants' and users' scopes;
    • resolve for another tenant or user;
    • the list route, unless it asks for the caller's own scope;
    • id-based create, read, update and delete;
    • /api/settings/modules;
    • every /admin/settings screen and action.
  • Who counts as a platform settings admin. admin holds every permission, including settings.system. Beyond that, it depends on whether a tenant resolver is registered:
    • Without one (no tenants module): a user whose record carries a tenant_id is never a platform admin, even with the admin role. On that setup the global admin role is the only admin role a tenant's operator can have, so the role alone can't tell the two apart.
    • With the tenants resolver: tenant roles arrive as tenant:<role> and admin stays a platform role, even while that user is working inside an organisation.
  • Single-tenant hosts are unchanged. Every check passes when multi_tenant is off.
  • Error responses: a refused request gets a 403 whose message is Permission required: settings.system. A permission check that fails first still gives its usual 401 or 403.

Deliberate limits

  • /resolve falls back to system values. Resolving your own keys still ends in system-scope values, as before. That is the configuration the caller already runs under, and secrets stay masked. This is documented on require_resolvable.
  • The sidebar entry is unchanged. It still needs only settings.view. Settings registration runs before the host's multi_tenant value is available, and requiring settings.system everywhere would hide the entry from custom single-tenant roles that can still use the screen. On a multi-tenant host, a tenant user who has settings.view sees the entry, and the screen answers with a 403 that names settings.system.
  • Not covered here: branding writes system-scope settings through its own API, which checks only branding.manage. That is the same class of bug, and branding: no table to add a mixin to — it's a single process-global settings singleton that must become per-tenant lookup #373 tracks it.

Testing

  • New: modules/settings/tests/test_settings_tenant_scope.py, 26 tests:
    • the issue's two repros;
    • own-tenant and own-user access, resolve, list, id-based routes, module-settings API and screens, including their write actions;
    • the platform admin keeps access to every scope;
    • single-tenant hosts are unchanged;
    • the tenants resolver path, with tenant:owner mapped to the settings permissions.
  • Full Python suite: 3,312 passed, 11 skipped. The 6 write-path tests were added after that run and pass on their own.
  • Lint: ruff, ty and the 300-line check are clean.
  • Review: an independent security review of the diff found no bypass. Its suggestions were the resolve docstring and the extra write-path tests, both added.

…enant hosts (#368)

The settings routes checked settings.view/edit/delete only, so on a
multi-tenant host any holder could write the host-wide system scope and any
tenant's or user's scope by naming it in the URL.

Add a settings.system permission and settings/scope_guard.py. With
multi_tenant on, a caller reaches its own tenant scope and its own user scope;
the system scope, other scopes, resolve for another tenant/user, id-based
CRUD, the module-settings API and the /admin/settings screens need a platform
settings admin. Without a tenant resolver, a user whose record carries a
tenant_id is never a platform admin, even with the admin role. Single-tenant
hosts are unchanged.
@cloudflare-workers-and-pages

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

Copy link
Copy Markdown

Deploying simple-module-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: 69a381f
Status: ✅  Deploy successful!
Preview URL: https://43be1a41.simple-module-python.pages.dev
Branch Preview URL: https://fix-368-settings-tenant-scop.simple-module-python.pages.dev

View logs

@antosubash antosubash added pi-ready Approved for the headless Pi worker pi-working The headless Pi worker is implementing this issue and removed pi-ready Approved for the headless Pi worker labels Oct 2, 2026
@antosubash

Copy link
Copy Markdown
Owner Author

🤖 Claimed existing pull request. I am creating an isolated worktree from origin/fix/368-settings-tenant-scope and starting automatic conflict, feedback, and CI handling.

@antosubash antosubash added pi-pr-open The headless Pi worker opened a draft pull request and removed pi-working The headless Pi worker is implementing this issue labels Oct 2, 2026
@antosubash

Copy link
Copy Markdown
Owner Author

🔀 Base-branch conflicts resolved and pushed. Verification follows.

Resolved all three conflict files without staging or committing. constants.py retains both the platform-only settings.system permission and tenant self-service permission; api.py preserves the PR’s scope guards alongside the base branch’s tenant validation; views.py keeps the platform guard and the base branch’s form handler.

Updated modules/settings/tests/test_settings_tenant_scope.py for membership-based tenancy after the base branch removed User.tenant_id, and corrected the related documentation in modules/settings/settings/scope_guard.py.

Verification: uv sync --all-packages completed; 284 settings tests and 20 related integration tests passed. Focused Ruff and Ty checks, the file-size check, and git diff --check passed. No conflict markers remain in the three files. Risk: the full repository suite was not run.

antosubash added a commit that referenced this pull request Oct 2, 2026
@antosubash

Copy link
Copy Markdown
Owner Author

✅ Post-resolution verification passed.

Independent QA passed. Local report: /home/anto/.local/share/pi-issue-worker/antosubash-simple-module-python-5242ec3307a6/verification/issue-379/1bad67d7-db5a-40d0-b34b-49ae8c7a601d/result.json.

Conflict-resolution QA evidence (independent verifier).

Attached QA evidence

denied-desktop.png

desktop.png

mobile.png

Omitted evidence:

  • workflow GIF was not produced

@antosubash

Copy link
Copy Markdown
Owner Author

✅ CI checks passed for this pull request.

@antosubash

Copy link
Copy Markdown
Owner Author

Closing as superseded. #368 was closed as stale after #370, and #382 (merged in #394) settled the settings model differently: settings.view/edit/delete are platform-operator permissions, and tenant owners/admins edit their own organisation's overridable keys through /api/settings/tenant/current under settings.tenant.edit. #381 also retired users_user.tenant_id, which this PR's legacy-path rule relied on. Merging this would now conflict with #382: its own-tenant allowance on the explicit /tenant/{scope_id} routes would bypass the overridable-keys limit for anyone holding settings.edit. If defence in depth for hosts that map settings.* onto tenant roles is wanted later, it should get a fresh issue.

@antosubash antosubash closed this Oct 4, 2026
@antosubash
antosubash deleted the fix/368-settings-tenant-scope branch October 4, 2026 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pi-pr-open The headless Pi worker opened a draft pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

settings: on a multi-tenant host any tenant's admin can edit system-scope settings and any other tenant's tenant-scope settings

1 participant