diff --git a/tests/test_app_js.py b/tests/test_app_js.py index cef8e386..890adc3b 100644 --- a/tests/test_app_js.py +++ b/tests/test_app_js.py @@ -2498,6 +2498,32 @@ def test_console_launcher_paints_project_and_persona_from_cache_synchronously() ) +def test_console_launcher_project_placeholder_tracks_require_project() -> None: + """require_project parity on the console launcher: with the gate on, BOTH + launcher kinds are refused a projectless create server-side (interactive at + the node, coordinator at the console's own mount), so the picker must stop + presenting "No project" as a normal choice. Mirrors the interactive strict + picker's soft treatment — retitle the placeholder ("Select a project…", or + "No projects available" when none exist) and never auto-select a real + project; the server's coded 400 stays the enforcement.""" + body = _CONSOLE_APP_JS.read_text(encoding="utf-8") + fn = _slice_top_level_fn(body, "function _populateHomeProjectDropdown(") + assert "TP.requireProject()" in fn, ( + "the launcher project populate must read the requireProject() advisory" + ) + assert re.search(r'setOptionPlaceholder\(\s*"project"', fn), ( + "strict mode must retitle the seeded placeholder via setOptionPlaceholder" + ) + for label in ("Select a project…", "No projects available", "No project"): + assert label in fn, f"placeholder branch missing the {label!r} title" + # The has-projects check must see REAL projects only: the placeholder is + # computed before the "+ New project…" sentinel is appended, or an empty + # deployment would read as having one project and mis-title the prompt. + assert fn.index("setOptionPlaceholder") < fn.index("_PROJECT_NEW"), ( + "the placeholder must be computed before the + New project… sentinel is appended" + ) + + def test_paint_project_picker_syncs_before_refresh() -> None: """The shared _paintProjectPicker (used by BOTH the modal and dashboard, so the two can't drift and silently re-introduce the FOUC) seeds the required/ diff --git a/tests/test_require_project.py b/tests/test_require_project.py index 5360330c..fcb62c88 100644 --- a/tests/test_require_project.py +++ b/tests/test_require_project.py @@ -1,12 +1,14 @@ """``server.require_project`` — the opt-in, default-off gate refusing projectless -interactive creates. +interactive and coordinator creates. -Three surfaces: +Four surfaces: * the predicate matrix (``require_project_enabled`` / ``require_project_denies_create``); * the fork/resume project inheritance + the cross-tenant 403-vs-400 oracle in the interactive create validator (``_interactive_create_validate_request``); * the console cluster-create proxy's surface-only-require_project / mask-everything - -else policy (``create_workstream``). + -else policy (``create_workstream``); + * the coordinator create mount on the console (gate wired on the real + ``coord_endpoint_config``, operator tokens not exempt). Validator tests drive the coroutine synchronously via ``asyncio.run`` so they need no async-plugin marker. Storage is a MagicMock patched onto the singleton getter that both @@ -92,6 +94,20 @@ class TestRequireProjectPredicate: cs = make_config_store(**{"server.require_project": True}) assert require_project_denies_create(cs, _Auth(), " ") is True + @pytest.mark.parametrize("bad", [123, True, ["p1"], {"id": "p1"}, 1.5]) + def test_truthy_non_string_project_is_projectless( + self, bad: Any, make_config_store: Any + ) -> None: + # A truthy non-string project_id must NOT stringify past the gate: the + # create path coerces non-strings to absent (build_kwargs → None), so + # the gate has to deny them under require_project or a projectless + # session gets minted with the policy on. Regression for the + # str(project_id or "") stringification bypass. + from turnstone.core.auth import require_project_denies_create + + cs = make_config_store(**{"server.require_project": True}) + assert require_project_denies_create(cs, _Auth(), bad) is True + def test_service_scope_exempt(self, make_config_store: Any) -> None: from turnstone.core.auth import require_project_denies_create @@ -501,7 +517,6 @@ def _run_gate(list_kind: Any, flag_on: bool, body: dict[str, Any], make_config_s (kind, flag, body). A gate pass-through raises out of a build_kwargs stub that the handler's own try/except turns into a non-require_project response.""" from turnstone.core.session_routes import SessionEndpointConfig, make_create_handler - from turnstone.core.workstream import WorkstreamKind def _build(*_a: Any, **_k: Any) -> dict[str, Any]: raise RuntimeError("stopped just past the require_project gate") @@ -515,9 +530,10 @@ def _run_gate(list_kind: Any, flag_on: bool, body: dict[str, Any], make_config_s not_found_label="workstream", audit_action_prefix="ws", list_kind=list_kind, - # Derived per-kind HERE the way the real mounts wire it (interactive True, - # coordinator False); production keeps it a declarative field, not a kind check. - create_gate_require_project=(list_kind == WorkstreamKind.INTERACTIVE), + # Both real mounts wire the gate on (interactive on the node, + # coordinator on the console); production keeps it a declarative + # field, not a kind check. + create_gate_require_project=True, create_validate_request=None, create_build_kwargs=_build, create_supports_attachments=False, @@ -529,7 +545,13 @@ def _run_gate(list_kind: Any, flag_on: bool, body: dict[str, Any], make_config_s return asyncio.run(handler(_gate_request(body, cs, auth))) -class TestNodeGateKindScoping: +class TestGateKindParity: + """The gate is kind-INDEPENDENT: both real mounts wire it on + (``create_gate_require_project=True``), so the (kind × flag × project) + matrix must apply uniformly. These synthetic-cfg cases assert that + parity — the exemption lives in ``require_project_denies_create`` + (token_source / service scope), never in the mount kind.""" + def test_interactive_projectless_gated(self, make_config_store: Any, tmp_db: Any) -> None: from turnstone.core.workstream import WorkstreamKind @@ -548,14 +570,131 @@ class TestNodeGateKindScoping: resp = _run_gate(WorkstreamKind.INTERACTIVE, False, {}, make_config_store) assert not _is_require_project_400(resp) - def test_coordinator_projectless_exempt(self, make_config_store: Any, tmp_db: Any) -> None: - # The coordinator mount leaves create_gate_require_project False, so a - # projectless coordinator create is NOT gated even with the flag on. + def test_coordinator_projectless_gated(self, make_config_store: Any, tmp_db: Any) -> None: + # The coordinator mount wires create_gate_require_project=True, so a + # projectless coordinator create is refused the same as interactive. from turnstone.core.workstream import WorkstreamKind resp = _run_gate(WorkstreamKind.COORDINATOR, True, {}, make_config_store) + assert _is_require_project_400(resp) + + def test_coordinator_with_project_passes(self, make_config_store: Any, tmp_db: Any) -> None: + from turnstone.core.workstream import WorkstreamKind + + resp = _run_gate(WorkstreamKind.COORDINATOR, True, {"project_id": "p1"}, make_config_store) assert not _is_require_project_400(resp) + def test_coordinator_flag_off_passes(self, make_config_store: Any, tmp_db: Any) -> None: + from turnstone.core.workstream import WorkstreamKind + + resp = _run_gate(WorkstreamKind.COORDINATOR, False, {}, make_config_store) + assert not _is_require_project_400(resp) + + +# --------------------------------------------------------------------------- +# Coordinator create mount wiring — the REAL console mount wires +# create_gate_require_project=True on coord_endpoint_config. The synthetic-cfg +# tests above can't catch a mis-wire on the actual mount, so drive the mounted +# console endpoint end-to-end (mirrors TestRequireProjectMountWiring in +# test_server_authz.py for the interactive node mount). +# --------------------------------------------------------------------------- + + +def _operator_headers() -> dict[str, str]: + """An ``admin.coordinator`` operator WITHOUT the ``service`` scope. + + Service identities are exempt from the gate; the operator's own token + must not be, so the gate tests would silently pass-through if this + reused ``_console_headers()`` (which carries ``service``). + """ + from turnstone.core.auth import JWT_AUD_CONSOLE, create_jwt + + tok = create_jwt( + user_id="op", + scopes=frozenset({"read", "write", "approve"}), + source="test", + secret=_CONSOLE_JWT_SECRET, + audience=JWT_AUD_CONSOLE, + permissions=frozenset({"admin.coordinator"}), + ) + return {"Authorization": f"Bearer {tok}"} + + +@contextlib.contextmanager +def _console_coord_client(tmp_path: Any, cs: Any) -> Any: + """A console TestClient with the real coord create mount live: a real + ``SessionManager(CoordinatorAdapter)`` over SQLite, a fake model registry + (passes the 503 gates), and the given config store.""" + from starlette.testclient import TestClient + + from tests._coord_test_helpers import _build_mgr, _fake_registry + from turnstone.console.collector import ClusterCollector + from turnstone.console.server import _load_static, create_app + from turnstone.core.storage._sqlite import SQLiteBackend + + _load_static() + storage = SQLiteBackend(str(tmp_path / "coord-gate.db")) + app = create_app(collector=MagicMock(spec=ClusterCollector), jwt_secret=_CONSOLE_JWT_SECRET) + mgr = _build_mgr(storage) + app.state.coord_mgr = mgr + app.state.coord_adapter = mgr._adapter + app.state.coord_registry = _fake_registry() + app.state.coord_registry_error = "" + app.state.config_store = cs + app.state.auth_storage = storage + client = TestClient(app, raise_server_exceptions=False, headers=_operator_headers()) + try: + yield client + finally: + client.close() + + +class TestCoordMountWiring: + def test_projectless_coord_create_gated_when_on( + self, tmp_path: Any, make_config_store: Any + ) -> None: + cs = make_config_store(**{"server.require_project": True}) + with _console_coord_client(tmp_path, cs) as client: + resp = client.post("/v1/api/workstreams/new", json={"name": "no-project"}) + assert resp.status_code == 400, resp.text + assert resp.json().get("code") == "require_project" + + def test_coord_create_with_project_passes_when_on( + self, tmp_path: Any, make_config_store: Any + ) -> None: + # A projected create must clear the gate; it 400s in the coord + # validator instead (unknown project_id on the fresh DB) — asserting + # NOT-the-coded-400 pins the gate without needing a seeded project. + cs = make_config_store(**{"server.require_project": True}) + with _console_coord_client(tmp_path, cs) as client: + resp = client.post( + "/v1/api/workstreams/new", json={"name": "x", "project_id": "p-missing"} + ) + assert resp.json().get("code") != "require_project", resp.text + + def test_projectless_coord_create_allowed_when_off( + self, tmp_path: Any, make_config_store: Any + ) -> None: + with _console_coord_client(tmp_path, make_config_store()) as client: + resp = client.post("/v1/api/workstreams/new", json={"name": "no-project"}) + body = resp.json() + assert resp.status_code == 200, body + assert body.get("ws_id") + + def test_non_string_project_id_cannot_bypass_gate( + self, tmp_path: Any, make_config_store: Any + ) -> None: + # End-to-end regression: the create path coerces a non-string + # project_id to absent (validator skips its attach check, build_kwargs + # → None), so a truthy non-string must be refused by the gate rather + # than minting a projectless coordinator with the policy on. Pins the + # three wired sites (validator / gate / build_kwargs) agreeing. + cs = make_config_store(**{"server.require_project": True}) + with _console_coord_client(tmp_path, cs) as client: + resp = client.post("/v1/api/workstreams/new", json={"name": "x", "project_id": 123}) + assert resp.status_code == 400, resp.text + assert resp.json().get("code") == "require_project" + # --------------------------------------------------------------------------- # list_projects advisory field — the frontend composer reads data.require_project diff --git a/turnstone/console/server.py b/turnstone/console/server.py index 79b560a4..6ec66b26 100644 --- a/turnstone/console/server.py +++ b/turnstone/console/server.py @@ -14573,6 +14573,13 @@ def create_app( events_replay=_coord_events_replay, create_supports_attachments=True, create_supports_user_id_override=False, + # Coordinator creates honour ``server.require_project`` exactly like + # interactive creates — the operator's own token gets no exemption. + # Distinct seam: the sessions a coordinator SPAWNS stay exempt via + # the ``token_source == "coordinator"`` branch inside + # ``require_project_denies_create``; that covers child spawns on + # nodes, not creating the coordinator itself. + create_gate_require_project=True, create_validate_request=_coord_create_validate_request, create_build_kwargs=_coord_create_build_kwargs, create_post_install=_coord_create_post_install, diff --git a/turnstone/console/static/app.js b/turnstone/console/static/app.js index 85bb167e..e781a3ff 100644 --- a/turnstone/console/static/app.js +++ b/turnstone/console/static/app.js @@ -1578,18 +1578,31 @@ function _refreshAndPopulateProjects(callOpts) { // current pick across the rebuild (same reason as _populateLauncherNodes) and // always appending the "+ New project…" sentinel after the live list. // -// Under server.require_project the console launcher intentionally does NOT gate -// this picker on TurnstoneProjects.requireProject() (unlike the node new-ws -// dialog): the node create endpoint is the authoritative gate, and its refusal -// is surfaced to the operator by the console proxy (create_workstream). A -// client-side required-project picker here is a deferred UX nicety, not a -// correctness requirement. +// Under server.require_project BOTH launcher kinds are authoritatively gated +// server-side: interactive at the node create endpoint (refusal surfaced by +// the console proxy, create_workstream), coordinator at the console's own +// create mount (refusal rendered inline by _createCoordinator). The picker +// mirrors the interactive dashboard's soft treatment — retitle the +// placeholder so projectless reads as a prompt, never auto-select a real +// project the operator didn't choose — and leaves enforcement to the +// server's coded 400 (the composer has no per-option disable, so the +// placeholder stays selectable; picking it just surfaces the server's +// message). function _populateHomeProjectDropdown() { if (!_homeCoordComposer) return; const TP = window.TurnstoneProjects; if (!TP) return; const previous = _homeCoordComposer.getOptionValue("project"); const choices = TP.projectChoices(); + const strict = !!TP.requireProject(); + _homeCoordComposer.setOptionPlaceholder( + "project", + strict + ? choices.length + ? "Select a project…" + : "No projects available" + : "No project", + ); choices.push({ value: _PROJECT_NEW, text: "+ New project…" }); _homeCoordComposer.setOptionChoices("project", choices); // Validate before restoring (via _restorePick, like the other pickers): a diff --git a/turnstone/core/auth.py b/turnstone/core/auth.py index 1045a0d1..3ffed226 100644 --- a/turnstone/core/auth.py +++ b/turnstone/core/auth.py @@ -471,8 +471,9 @@ def ensure_project_attachable( # --------------------------------------------------------------------------- # server.require_project — opt-in, default-off gate refusing projectless -# interactive creates. One predicate is the ONLY flag read (gate + advisory -# both call it) so authoritative and advisory logic cannot drift. +# creates on the gated mounts (interactive on nodes, coordinator on the +# console). One predicate is the ONLY flag read (gate + advisory both call +# it) so authoritative and advisory logic cannot drift. # --------------------------------------------------------------------------- # Discriminator on the node's 400 body. The console cluster-create proxy @@ -504,16 +505,28 @@ def require_project_enabled(config_store: Any) -> bool: def require_project_denies_create(config_store: Any, auth: Any, project_id: Any) -> bool: - """Return True to REFUSE an interactive create under ``server.require_project``. + """Return True to REFUSE a gated create under ``server.require_project``. - Refuses only when the gate is on, the caller is not exempt automation, and - no project is attached (whitespace-only counts as none). Exempt identities: - the ``service`` scope (channel gateway, scheduler) and coordinator SESSION - spawns (``token_source == "coordinator"``). NOT exempt on any admin - permission — ``admin.coordinator`` is a human-operator permission and would - leak every operator through the gate. ``console-proxy`` (the normal proxied - human) is deliberately NOT exempt: it carries the human's own scopes, which - never include ``service``. + Serves both gated mounts: interactive creates on nodes and coordinator + creates on the console. Refuses only when the gate is on, the caller is + not exempt automation, and no project is attached. "No project" is read + the way the create path actually persists it: a non-string body value + (int / bool / list / dict) is coerced to absent — exactly as + ``_coord_create_build_kwargs`` and the interactive create do — so a + truthy non-string like ``project_id: 123`` cannot stringify past the + gate and mint a projectless session; whitespace-only counts as none too. + Exempt identities: the ``service`` scope (channel gateway, + scheduler) and the sessions a coordinator spawns + (``token_source == "coordinator"`` — minted per coordinator session for + its child spawns and sends; no current path lets that token CREATE a + coordinator, so the exemption is child-spawn-only in practice. If a + coordinator tool ever gains the ability to create coordinators, decide + then whether that path should stay exempt). NOT exempt on any admin + permission — ``admin.coordinator`` is a human-operator permission and + would leak every operator through the gate, including the console + coordinator launcher this gate now covers. ``console-proxy`` (the normal + proxied human) is deliberately NOT exempt: it carries the human's own + scopes, which never include ``service``. """ if not require_project_enabled(config_store): return False @@ -521,7 +534,7 @@ def require_project_denies_create(config_store: Any, auth: Any, project_id: Any) return False if getattr(auth, "token_source", "") == "coordinator": return False - return not str(project_id or "").strip() + return not (project_id if isinstance(project_id, str) else "").strip() def _permissions_to_scopes(permissions: set[str]) -> frozenset[str]: diff --git a/turnstone/core/session_routes.py b/turnstone/core/session_routes.py index 05f78623..614290b9 100644 --- a/turnstone/core/session_routes.py +++ b/turnstone/core/session_routes.py @@ -429,10 +429,13 @@ class SessionEndpointConfig: # and the operator's auth result is the source of truth. create_supports_user_id_override: bool = False # When ``True`` the shared create handler applies the ``server.require_project`` - # gate (refuse a projectless interactive create). Declarative per-mount - # capability — interactive wires ``True``; coordinator leaves it ``False`` so - # coordinator spawns are never gated. A cfg flag rather than a hardcoded kind - # literal, matching the ``create_supports_*`` idiom. + # gate (refuse a projectless create). Declarative per-mount capability — + # both current kinds wire ``True`` (interactive on the node mount, + # coordinator on the console mount); a future kind opts out by leaving the + # default. Sessions a coordinator spawns stay exempt inside + # ``require_project_denies_create`` (token_source), not via this flag. A + # cfg flag rather than a hardcoded kind literal, matching the + # ``create_supports_*`` idiom. create_gate_require_project: bool = False # (request, body, uid, uploaded_files) -> JSONResponse | None. # Per-kind pre-create gate (ws_id format, parent ownership, kind @@ -2570,15 +2573,19 @@ def make_create_handler( if err_validate is not None: return err_validate - # --- require_project gate (interactive-create mounts only) ------- + # --- require_project gate ---------------------------------------- # Gated by the declarative cfg.create_gate_require_project capability - # (True on the interactive create mount, default-False on coordinator) - # rather than a hardcoded kind literal, matching the create_supports_* - # idiom — coordinator spawns are exempt by design. By this point the - # validator has applied any parent-/resume-inherited project_id into body - # AND (for a fork) discarded any explicit pick to the source's project or - # "", so a private/dangling/projectless fork SOURCE funnels to the SAME - # uniform 400 as a projectless fresh create. + # (True on both the interactive and coordinator create mounts) rather + # than a hardcoded kind literal, matching the create_supports_* idiom. + # Sessions a coordinator SPAWNS remain exempt — that's the + # token_source == "coordinator" branch inside + # require_project_denies_create, not a mount property. On the + # interactive mount, by this point the validator has applied any + # parent-/resume-inherited project_id into body AND (for a fork) + # discarded any explicit pick to the source's project or "", so a + # private/dangling/projectless fork SOURCE funnels to the SAME uniform + # 400 as a projectless fresh create. (Coordinator has no fork/resume — + # its validator only checks attachability of an explicit pick.) if cfg.create_gate_require_project: from turnstone.core.auth import ( REQUIRE_PROJECT_CODE, diff --git a/turnstone/core/settings_registry.py b/turnstone/core/settings_registry.py index 8c34a8d6..9c112677 100644 --- a/turnstone/core/settings_registry.py +++ b/turnstone/core/settings_registry.py @@ -362,18 +362,18 @@ def _build_registry() -> dict[str, SettingDef]: "server.require_project", "bool", False, - "Require new interactive chats to be filed under a project", + "Require new chats and coordinators to be filed under a project", "server", - help="When on, starting a new interactive chat is refused unless it is filed " - "under a project. This is off by default and changes nothing until you turn it " - "on. A person can start a chat only once they belong to at least one project: " - "there is no automatic or personal project, so before turning this on in a " - "strict deployment, give each user membership in a project or mark a project " - "public, or they will not be able to start chats at all. Forking or resuming a " - "chat keeps the source chat's project; forking a chat that has no project is " - "refused just like starting a fresh chat without one. Automation such as " - "channel and scheduler activity, and coordinator-spawned sessions, are not " - "affected.", + help="When on, starting a new chat or a new coordinator is refused unless it " + "is filed under a project. This is off by default and changes nothing until " + "you turn it on. A person can start a chat only once they belong to at least " + "one project: there is no automatic or personal project, so before turning " + "this on in a strict deployment, give each user membership in a project or " + "mark a project public, or they will not be able to start chats at all. " + "Forking or resuming a chat keeps the source chat's project; forking a chat " + "that has no project is refused just like starting a fresh chat without one. " + "Automation such as channel and scheduler activity, and the sessions a " + "coordinator spawns to do its work, are not affected.", ), # -- cluster -------------------------------------------------------- SettingDef(