From f9ecc01acd8d410a23ac0d3ed826208d677ab769 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 19 Aug 2026 16:26:13 +0200 Subject: [PATCH 01/14] fix(hosting): JSON error bodies for API and fetch callers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 403/404/500 under /api/* — or with an explicit Accept: application/json — now return the JSON detail instead of the Inertia HTML error page, which hid the actual error (permission name, CSRF hint) from API clients. Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc --- .../simple_module_hosting/_error_handlers.py | 22 ++++- .../tests/test_api_error_negotiation.py | 82 +++++++++++++++++++ 2 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 framework/hosting/tests/test_api_error_negotiation.py diff --git a/framework/hosting/simple_module_hosting/_error_handlers.py b/framework/hosting/simple_module_hosting/_error_handlers.py index 4737562b..fe23d208 100644 --- a/framework/hosting/simple_module_hosting/_error_handlers.py +++ b/framework/hosting/simple_module_hosting/_error_handlers.py @@ -23,6 +23,22 @@ _INERTIA_ERROR_STATUSES = frozenset({403, 404, 500}) +def _wants_json(request: Request) -> bool: + """API callers get JSON error bodies; browser-shaped requests get the page. + + ``/api/*`` is the documented prefix for every module's JSON surface + (``ModuleMeta.route_prefix``), so path alone decides there. Elsewhere an + explicit ``Accept: application/json`` (without ``text/html`` — a browser + navigation sends both) opts a fetch caller into JSON. The default + ``*/*`` keeps the rendered Inertia page. + """ + path = request.url.path + if path == "/api" or path.startswith("/api/"): + return True + accept = request.headers.get("accept", "") + return "application/json" in accept and "text/html" not in accept + + async def render_error_page(request: Request, status_code: int, message: str) -> Response: config: InertiaConfig = request.app.state.sm.inertia_config try: @@ -59,13 +75,15 @@ async def render_error_page(request: Request, status_code: int, message: str) -> async def http_exception_handler(request: Request, exc: HTTPException) -> Response: - if exc.status_code in _INERTIA_ERROR_STATUSES: + if exc.status_code in _INERTIA_ERROR_STATUSES and not _wants_json(request): detail = str(exc.detail) if exc.detail else "" return await render_error_page(request, exc.status_code, detail) return JSONResponse(status_code=exc.status_code, content={"detail": exc.detail}) async def not_found_error_handler(request: Request, exc: NotFoundError) -> Response: + if _wants_json(request): + return JSONResponse(status_code=404, content={"detail": str(exc)}) return await render_error_page(request, 404, str(exc)) @@ -83,4 +101,6 @@ async def request_validation_error_handler( async def unhandled_exception_handler(request: Request, exc: Exception) -> Response: logger.exception("Unhandled exception: %s", exc) + if _wants_json(request): + return JSONResponse(status_code=500, content={"detail": "Internal Server Error"}) return await render_error_page(request, 500, "") diff --git a/framework/hosting/tests/test_api_error_negotiation.py b/framework/hosting/tests/test_api_error_negotiation.py new file mode 100644 index 00000000..2d8140bf --- /dev/null +++ b/framework/hosting/tests/test_api_error_negotiation.py @@ -0,0 +1,82 @@ +"""API callers must get JSON error bodies, not the Inertia HTML error page. + +``http_exception_handler`` used to render 403/404/500 as an Inertia page +unconditionally. A ``fetch`` against ``/api/*`` then received a full HTML +document — with the actual error detail (permission name, CSRF hint) +invisible to the caller. Requests under ``/api/`` or that explicitly prefer +``application/json`` must get the JSON ``{"detail": ...}`` body instead; +browser-shaped requests keep the rendered page. +""" + +from __future__ import annotations + +import httpx +from simple_module_hosting._error_handlers import http_exception_handler +from starlette.exceptions import HTTPException +from starlette.requests import Request +from starlette.responses import JSONResponse + + +def _request(path: str, accept: str = "*/*", method: str = "GET") -> Request: + return Request( + { + "type": "http", + "http_version": "1.1", + "method": method, + "path": path, + "raw_path": path.encode(), + "query_string": b"", + "scheme": "http", + "server": ("testserver", 80), + "client": ("testclient", 1234), + "headers": [(b"accept", accept.encode())], + } + ) + + +class TestHandlerNegotiation: + async def test_api_path_403_returns_json_detail(self) -> None: + resp = await http_exception_handler( + _request("/api/pagebuilder/pages", method="POST"), + HTTPException(status_code=403, detail="Permission required: pagebuilder.edit"), + ) + assert isinstance(resp, JSONResponse) + assert resp.status_code == 403 + assert b"pagebuilder.edit" in resp.body + + async def test_api_path_404_returns_json(self) -> None: + resp = await http_exception_handler( + _request("/api/nope"), HTTPException(status_code=404, detail="Not Found") + ) + assert isinstance(resp, JSONResponse) + assert resp.status_code == 404 + + async def test_json_accept_on_view_path_returns_json(self) -> None: + resp = await http_exception_handler( + _request("/pagebuilder/", accept="application/json"), + HTTPException(status_code=403, detail="nope"), + ) + assert isinstance(resp, JSONResponse) + +class TestClientNegotiation: + async def test_api_404_is_json(self, authenticated_client: httpx.AsyncClient) -> None: + resp = await authenticated_client.get("/api/definitely/not/a/route") + assert resp.status_code == 404 + assert resp.headers["content-type"].startswith("application/json") + assert "detail" in resp.json() + + async def test_view_404_with_json_accept_is_json( + self, authenticated_client: httpx.AsyncClient + ) -> None: + resp = await authenticated_client.get( + "/definitely/not/a/route", headers={"Accept": "application/json"} + ) + assert resp.status_code == 404 + assert resp.headers["content-type"].startswith("application/json") + + async def test_view_404_default_accept_still_renders_page( + self, authenticated_client: httpx.AsyncClient + ) -> None: + resp = await authenticated_client.get("/definitely/not/a/route") + assert resp.status_code == 404 + assert "data-page" in resp.text # Inertia error page, unchanged behavior From 3dc4467c96b8301f78cf034a5c6e5a01c59f36f2 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 19 Aug 2026 16:29:36 +0200 Subject: [PATCH 02/14] fix(hosting): cwd-independent .env discovery and SQLite paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settings now find the project .env by walking up from the cwd (bounded, stopping at repo boundaries and $HOME), and relative sqlite URLs resolve against the project root instead of the process cwd — so CLI tools run from host/ or modules// hit the same database as the app. Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc --- .../bootstrap_settings.py | 76 ++++++++++- .../hosting/tests/test_sqlite_path_anchor.py | 125 ++++++++++++++++++ 2 files changed, 200 insertions(+), 1 deletion(-) create mode 100644 framework/hosting/tests/test_sqlite_path_anchor.py diff --git a/framework/hosting/simple_module_hosting/bootstrap_settings.py b/framework/hosting/simple_module_hosting/bootstrap_settings.py index 142103e5..fcc067fa 100644 --- a/framework/hosting/simple_module_hosting/bootstrap_settings.py +++ b/framework/hosting/simple_module_hosting/bootstrap_settings.py @@ -7,6 +7,8 @@ from __future__ import annotations +import os +from pathlib import Path from typing import Literal from pydantic import field_validator, model_validator @@ -16,11 +18,78 @@ _PLACEHOLDER_SECRET_KEY = "change-me-in-production" +# How many parent directories to probe for a `.env` above the cwd. One level +# covers the workspace layout (`host/` → root); a couple more cover running +# from `modules//`. Bounded so an unrelated `.env` far up the tree +# (e.g. in $HOME) is never picked up by accident. +_ENV_WALK_LIMIT = 4 + + +def _discover_env_file() -> Path | str: + """Locate the project ``.env`` regardless of which subdirectory runs us. + + ``SM_PROJECT_ROOT`` wins when set (the convention every out-of-process + tool in this repo already follows — see ``simple_module_core.dotenv``). + Otherwise walk up from the cwd: the web process chdirs to the workspace + root so this finds ``./.env`` immediately, while a CLI invoked from + ``host/`` or ``modules//`` finds the same file its app uses + instead of silently loading nothing. + """ + explicit = os.environ.get("SM_PROJECT_ROOT") + if explicit: + return Path(explicit) / ".env" + current = Path.cwd() + home = Path.home() + for candidate in (current, *current.parents[:_ENV_WALK_LIMIT]): + if candidate == home: + break + env = candidate / ".env" + if env.is_file(): + return env + # A `.git` marks the repository root: never ascend past it, or a + # nested checkout (a git worktree, a repo inside another repo) + # would silently load the *outer* project's `.env`. + if (candidate / ".git").exists(): + break + return ".env" + + +def _project_anchor(env_file: Path | str) -> Path: + """Directory that relative sqlite paths are written against.""" + explicit = os.environ.get("SM_PROJECT_ROOT") + if explicit: + return Path(explicit) + if isinstance(env_file, Path): + return env_file.parent + return Path.cwd() + + +def _absolutize_sqlite_url(url: str, *, anchor: Path) -> str: + """Rewrite a relative sqlite path to an absolute one under ``anchor``. + + ``sqlite+aiosqlite:///./host/app.db`` means "relative to the project + root" by convention, but SQLAlchemy resolves it against the process cwd + — correct in the web process (which chdirs) and silently wrong in every + CLI run from a subdirectory. Absolute paths (``:////...``), ``:memory:``, + and non-sqlite URLs pass through untouched. + """ + if not url.startswith("sqlite"): + return url + scheme, sep, rest = url.partition(":///") + if not sep or not rest or rest.startswith("/") or rest.startswith(":memory:"): + return url + path_part, query_sep, query = rest.partition("?") + resolved = (anchor / path_part).resolve() + return f"{scheme}:///{resolved}{query_sep}{query}" + + +_ENV_FILE = _discover_env_file() + class BootstrapSettings(BaseSettings): """Pre-DB bootstrap environment knobs.""" - model_config = SettingsConfigDict(env_prefix="SM_", env_file=".env", extra="ignore") + model_config = SettingsConfigDict(env_prefix="SM_", env_file=_ENV_FILE, extra="ignore") database_url: str = "sqlite+aiosqlite:///./app.db" db_pool_size: int = 10 @@ -64,6 +133,11 @@ class BootstrapSettings(BaseSettings): ``register_public_routes`` hook, which is method-aware. """ + @field_validator("database_url", mode="after") + @classmethod + def _anchor_relative_sqlite_path(cls, value: str) -> str: + return _absolutize_sqlite_url(value, anchor=_project_anchor(_ENV_FILE)) + @field_validator("auth_provider", mode="after") @classmethod def _normalize_auth_provider(cls, value: str) -> str: diff --git a/framework/hosting/tests/test_sqlite_path_anchor.py b/framework/hosting/tests/test_sqlite_path_anchor.py new file mode 100644 index 00000000..9a38b9f8 --- /dev/null +++ b/framework/hosting/tests/test_sqlite_path_anchor.py @@ -0,0 +1,125 @@ +"""Relative SQLite paths must not depend on the process working directory. + +``SM_DATABASE_URL=sqlite+aiosqlite:///./host/app.db`` is written relative to +the workspace root. The web process chdirs there, but CLI tools +(``smpy users create-admin``, alembic) run from wherever the operator +happens to be — from ``host/`` the same URL silently pointed at +``host/host/app.db`` and failed with "unable to open database file". + +The settings layer now anchors relative sqlite paths on the project root +(``SM_PROJECT_ROOT``, else the directory of the discovered ``.env``), and +discovers the ``.env`` by walking up from the cwd, so subdirectory +invocations see the same database as the app. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +from simple_module_hosting.bootstrap_settings import ( + _absolutize_sqlite_url, + _discover_env_file, +) + + +class TestAbsolutize: + def test_relative_path_resolves_against_anchor(self, tmp_path: Path) -> None: + url = _absolutize_sqlite_url("sqlite+aiosqlite:///./host/app.db", anchor=tmp_path) + assert url == f"sqlite+aiosqlite:///{tmp_path / 'host' / 'app.db'}" + + def test_bare_relative_path_resolves(self, tmp_path: Path) -> None: + url = _absolutize_sqlite_url("sqlite+aiosqlite:///app.db", anchor=tmp_path) + assert url == f"sqlite+aiosqlite:///{tmp_path / 'app.db'}" + + def test_absolute_path_untouched(self, tmp_path: Path) -> None: + url = "sqlite+aiosqlite:////var/data/app.db" + assert _absolutize_sqlite_url(url, anchor=tmp_path) == url + + def test_memory_untouched(self, tmp_path: Path) -> None: + url = "sqlite+aiosqlite:///:memory:" + assert _absolutize_sqlite_url(url, anchor=tmp_path) == url + + def test_bare_scheme_untouched(self, tmp_path: Path) -> None: + url = "sqlite+aiosqlite://" + assert _absolutize_sqlite_url(url, anchor=tmp_path) == url + + def test_postgres_untouched(self, tmp_path: Path) -> None: + url = "postgresql+asyncpg://u:p@localhost/db" + assert _absolutize_sqlite_url(url, anchor=tmp_path) == url + + def test_query_string_survives(self, tmp_path: Path) -> None: + url = _absolutize_sqlite_url("sqlite+aiosqlite:///app.db?mode=ro", anchor=tmp_path) + assert url == f"sqlite+aiosqlite:///{tmp_path / 'app.db'}?mode=ro" + + +class TestDiscoverEnvFile: + def test_walks_up_from_subdirectory( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + (tmp_path / ".env").write_text("SM_X=1\n", encoding="utf-8") + host = tmp_path / "host" + host.mkdir() + monkeypatch.delenv("SM_PROJECT_ROOT", raising=False) + monkeypatch.chdir(host) + assert _discover_env_file() == tmp_path / ".env" + + def test_cwd_env_wins_over_parent( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + (tmp_path / ".env").write_text("SM_X=parent\n", encoding="utf-8") + host = tmp_path / "host" + host.mkdir() + (host / ".env").write_text("SM_X=child\n", encoding="utf-8") + monkeypatch.delenv("SM_PROJECT_ROOT", raising=False) + monkeypatch.chdir(host) + assert _discover_env_file() == host / ".env" + + def test_project_root_env_var_wins( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + root = tmp_path / "elsewhere" + root.mkdir() + monkeypatch.setenv("SM_PROJECT_ROOT", str(root)) + monkeypatch.chdir(tmp_path) + assert _discover_env_file() == root / ".env" + + def test_no_env_anywhere_falls_back_to_plain_name( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + deep = tmp_path / "a" / "b" + deep.mkdir(parents=True) + monkeypatch.delenv("SM_PROJECT_ROOT", raising=False) + monkeypatch.chdir(deep) + assert _discover_env_file() == ".env" + + def test_walk_stops_at_repo_boundary( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A nested checkout must never load the outer project's .env.""" + (tmp_path / ".env").write_text("SM_X=outer\n", encoding="utf-8") + inner = tmp_path / "inner_repo" + inner.mkdir() + (inner / ".git").write_text("gitdir: elsewhere\n", encoding="utf-8") + sub = inner / "host" + sub.mkdir() + monkeypatch.delenv("SM_PROJECT_ROOT", raising=False) + monkeypatch.chdir(sub) + assert _discover_env_file() == ".env" # inner repo has none; outer is off-limits + + +class TestSettingsIntegration: + def test_settings_absolutize_via_project_root( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + from simple_module_hosting.settings import Settings + + monkeypatch.setenv("SM_PROJECT_ROOT", str(tmp_path)) + s = Settings(database_url="sqlite+aiosqlite:///./host/app.db") + assert s.database_url == f"sqlite+aiosqlite:///{tmp_path / 'host' / 'app.db'}" + + def test_settings_leave_memory_url_alone(self) -> None: + from simple_module_hosting.settings import Settings + + s = Settings(database_url="sqlite+aiosqlite:///:memory:") + assert s.database_url == "sqlite+aiosqlite:///:memory:" From ab5094f2241913a60eb173308e1f36ce8490cd2c Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 19 Aug 2026 16:33:57 +0200 Subject: [PATCH 03/14] =?UTF-8?q?feat(core+hosting):=20register=5Fcsp=5Fso?= =?UTF-8?q?urces=20=E2=80=94=20modules=20extend=20the=20CSP?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A module whose frontend loads assets from an external origin (pagebuilder's rsms.me font was the field case) can now declare it: def register_csp_sources(self, registry): registry.add("style-src", "https://rsms.me") Only fetch directives are extendable (never default-src/base-uri/ form-action/frame-ancestors); sources are validated single tokens, so a typo fails at boot instead of weakening the header. Both the dev (Vite- widened) and production CSPs honor the registry. Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc --- framework/core/simple_module_core/__init__.py | 3 + framework/core/simple_module_core/csp.py | 103 ++++++++++++++++++ .../simple_module_core/diagnostics/_module.py | 1 + framework/core/simple_module_core/module.py | 18 +++ framework/core/tests/test_csp_registry.py | 71 ++++++++++++ .../simple_module_hosting/_phase_helpers.py | 26 ++++- .../simple_module_hosting/_registrations.py | 3 + .../simple_module_hosting/app_builder.py | 7 +- .../hosting/tests/test_csp_module_sources.py | 68 ++++++++++++ 9 files changed, 297 insertions(+), 3 deletions(-) create mode 100644 framework/core/simple_module_core/csp.py create mode 100644 framework/core/tests/test_csp_registry.py create mode 100644 framework/hosting/tests/test_csp_module_sources.py diff --git a/framework/core/simple_module_core/__init__.py b/framework/core/simple_module_core/__init__.py index 65ba4730..9f8b70b3 100644 --- a/framework/core/simple_module_core/__init__.py +++ b/framework/core/simple_module_core/__init__.py @@ -1,6 +1,7 @@ """SimpleModule Core - Module system, menu, permissions, events, and diagnostics.""" from simple_module_core.audit_links import AuditLink, AuditLinkRegistry +from simple_module_core.csp import CspSourceError, CspSourceRegistry from simple_module_core.design_packs import DesignPack, DesignPackRegistry from simple_module_core.diagnostics import ( DiagnosticLevel, @@ -72,6 +73,8 @@ "NotFoundError", "PermissionRegistry", "PublicRoute", + "CspSourceError", + "CspSourceRegistry", "PublicRouteRegistry", "Services", "Translator", diff --git a/framework/core/simple_module_core/csp.py b/framework/core/simple_module_core/csp.py new file mode 100644 index 00000000..bb231fc4 --- /dev/null +++ b/framework/core/simple_module_core/csp.py @@ -0,0 +1,103 @@ +"""Registry for module-contributed Content-Security-Policy sources. + +Modules whose frontend loads assets from an external origin (a font CDN, a +tile server, an analytics endpoint) declare those origins through +``ModuleBase.register_csp_sources``. The host folds them into the CSP it +already ships — the module never rewrites the whole policy, and a typo'd +origin fails loudly at boot instead of silently weakening the header. +""" + +from __future__ import annotations + +import re + +__all__ = ["CspSourceError", "CspSourceRegistry"] + +# Fetch directives a module may extend. Deliberately excludes the policy's +# structural directives (default-src, base-uri, form-action, frame-ancestors, +# sandbox): widening those changes the security posture of the whole app and +# belongs to the host operator, not a module. +_EXTENDABLE_DIRECTIVES = frozenset( + { + "script-src", + "script-src-elem", + "style-src", + "style-src-elem", + "img-src", + "font-src", + "connect-src", + "media-src", + "frame-src", + "worker-src", + "child-src", + } +) + +# A source is a scheme, an origin (optionally with scheme/wildcard/port), or +# a data-ish scheme keyword. One token — anything that could smuggle a second +# token or terminate the clause (whitespace, ";", quotes) is rejected. +_SOURCE_RE = re.compile(r"^(?:[A-Za-z][A-Za-z0-9+.-]*:(?://)?)?(?:\*\.)?[^\s;'\"*]*$") + + +class CspSourceError(ValueError): + """Invalid CSP directive or source declared by a module.""" + + +class CspSourceRegistry: + """Collects per-directive extra CSP sources from modules.""" + + def __init__(self) -> None: + self._sources: dict[str, list[str]] = {} + + def add(self, directive: str, source: str) -> None: + """Allow ``source`` in ``directive``, e.g. ``add("style-src", "https://rsms.me")``.""" + if directive not in _EXTENDABLE_DIRECTIVES: + raise CspSourceError( + f"CSP directive {directive!r} is not extendable; " + f"choose one of {sorted(_EXTENDABLE_DIRECTIVES)}" + ) + token = source.strip() + if not token or not _SOURCE_RE.match(token): + raise CspSourceError( + f"invalid CSP source {source!r} for {directive}: must be a single " + "origin or scheme token (no spaces, quotes, wildcards-only, or ';')" + ) + bucket = self._sources.setdefault(directive, []) + if token not in bucket: + bucket.append(token) + + def __bool__(self) -> bool: + return bool(self._sources) + + @property + def sources(self) -> dict[str, tuple[str, ...]]: + return {directive: tuple(items) for directive, items in self._sources.items()} + + def extend_policy(self, policy: str) -> str: + """Fold the registered sources into an existing policy string. + + Existing clauses keep their order and gain only sources they don't + already list. A directive absent from the policy is appended as a new + clause seeded with ``'self'`` — without it the new clause would + *narrow* the policy, since the browser stops falling back to + ``default-src`` once the directive exists at all. + """ + if not self._sources: + return policy + clauses = [c.strip() for c in policy.split(";") if c.strip()] + seen_directives: set[str] = set() + out: list[str] = [] + for clause in clauses: + directive, _, rest = clause.partition(" ") + extras = self._sources.get(directive) + if extras: + seen_directives.add(directive) + existing = rest.split() + merged = existing + [e for e in extras if e not in existing] + out.append(f"{directive} {' '.join(merged)}") + else: + out.append(clause) + for directive, extras in self._sources.items(): + if directive not in seen_directives: + out.append(f"{directive} 'self' {' '.join(extras)}") + return "; ".join(out) diff --git a/framework/core/simple_module_core/diagnostics/_module.py b/framework/core/simple_module_core/diagnostics/_module.py index e5fe1323..481953ec 100644 --- a/framework/core/simple_module_core/diagnostics/_module.py +++ b/framework/core/simple_module_core/diagnostics/_module.py @@ -98,6 +98,7 @@ def _check_empty_modules(self, modules: list[ModuleBase]) -> list[Diagnostic]: "register_middleware", "register_health_checks", "register_public_routes", + "register_csp_sources", "register_exception_handlers", "register_settings", "template_dirs", diff --git a/framework/core/simple_module_core/module.py b/framework/core/simple_module_core/module.py index 04d8d233..b7f81d01 100644 --- a/framework/core/simple_module_core/module.py +++ b/framework/core/simple_module_core/module.py @@ -17,6 +17,7 @@ from simple_module_core.health import HealthRegistry from simple_module_core.menu import MenuRegistry from simple_module_core.permissions import PermissionRegistry + from simple_module_core.csp import CspSourceRegistry from simple_module_core.public_routes import PublicRouteRegistry @@ -148,6 +149,23 @@ def register_public_routes(self, registry): mutations. Called once at boot, in dependency order. """ + def register_csp_sources(self, registry: CspSourceRegistry) -> None: + """Declare external origins this module's frontend loads assets from. + + The host ships a strict Content-Security-Policy; a module whose + pages pull a stylesheet, font, or API from another origin must + declare it here or the browser blocks the request:: + + def register_csp_sources(self, registry): + registry.add("style-src", "https://rsms.me") + registry.add("font-src", "https://rsms.me") + + Only fetch directives can be extended (never default-src, + base-uri, form-action, frame-ancestors), and each source must be a + single origin/scheme token — invalid declarations raise at boot. + Called once, in dependency order. + """ + def register_design_packs(self, registry: DesignPackRegistry) -> None: """Declare design packs this module ships for the public site. diff --git a/framework/core/tests/test_csp_registry.py b/framework/core/tests/test_csp_registry.py new file mode 100644 index 00000000..30004de3 --- /dev/null +++ b/framework/core/tests/test_csp_registry.py @@ -0,0 +1,71 @@ +"""Modules can extend the host Content-Security-Policy with extra origins. + +Found in the field: the pagebuilder module's editor loads a stylesheet from +``rsms.me``, which the framework CSP blocks — and there was no way for the +module (or the host) to declare that origin short of patching the installed +package. ``register_csp_sources`` closes that gap the same way +``register_public_routes`` does for auth exemptions. +""" + +from __future__ import annotations + +import pytest +from simple_module_core import ModuleBase, ModuleMeta +from simple_module_core.csp import CspSourceError, CspSourceRegistry + +_POLICY = ( + "default-src 'self'; " + "style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; " + "font-src 'self' https://fonts.gstatic.com data:" +) + + +class TestRegistry: + def test_extends_existing_directive(self) -> None: + reg = CspSourceRegistry() + reg.add("style-src", "https://rsms.me") + out = reg.extend_policy(_POLICY) + assert "style-src 'self' 'unsafe-inline' https://fonts.googleapis.com https://rsms.me" in out + + def test_duplicate_and_already_present_sources_are_not_repeated(self) -> None: + reg = CspSourceRegistry() + reg.add("style-src", "https://rsms.me") + reg.add("style-src", "https://rsms.me") + reg.add("style-src", "https://fonts.googleapis.com") # already in the policy + out = reg.extend_policy(_POLICY) + assert out.count("https://rsms.me") == 1 + assert out.count("https://fonts.googleapis.com") == 1 + + def test_missing_directive_gets_self_plus_source(self) -> None: + """A brand-new clause must keep 'self', or it would *narrow* the policy: + without the clause the browser falls back to default-src 'self'.""" + reg = CspSourceRegistry() + reg.add("connect-src", "https://api.example.com") + out = reg.extend_policy(_POLICY) + assert "connect-src 'self' https://api.example.com" in out + + def test_empty_registry_returns_policy_unchanged(self) -> None: + reg = CspSourceRegistry() + assert reg.extend_policy(_POLICY) == _POLICY + assert not reg + + def test_unknown_directive_rejected(self) -> None: + reg = CspSourceRegistry() + with pytest.raises(CspSourceError, match="directive"): + reg.add("script-src-attr; evil", "https://x.example") + + def test_injection_shaped_sources_rejected(self) -> None: + reg = CspSourceRegistry() + for bad in ("https://x; script-src *", "https://x 'unsafe-eval'", "", "'*'"): + with pytest.raises(CspSourceError): + reg.add("style-src", bad) + + +class TestHook: + def test_register_csp_sources_is_a_noop_by_default(self) -> None: + class Quiet(ModuleBase): + meta = ModuleMeta(name="Quiet") + + reg = CspSourceRegistry() + Quiet().register_csp_sources(reg) + assert not reg diff --git a/framework/hosting/simple_module_hosting/_phase_helpers.py b/framework/hosting/simple_module_hosting/_phase_helpers.py index 80d37cc2..e2e87839 100644 --- a/framework/hosting/simple_module_hosting/_phase_helpers.py +++ b/framework/hosting/simple_module_hosting/_phase_helpers.py @@ -72,12 +72,31 @@ def register_exception_handlers(app: FastAPI, modules: list) -> None: mod.register_exception_handlers(app) +def build_csp(settings: Settings, csp_registry=None) -> str | None: + """Choose the CSP for this boot and fold in module-declared sources. + + Development gets the Vite-widened policy; production the strict default. + ``csp_registry`` carries origins modules declared via + ``register_csp_sources`` (e.g. an external font host) — merged here so + both variants honor them. + """ + base = ( + SecurityHeadersMiddleware.dev_csp(settings.vite_dev_url) + if settings.is_development + else SecurityHeadersMiddleware._DEFAULT_CSP + ) + if csp_registry: + return csp_registry.extend_policy(base) + return base + + def install_middleware( app: FastAPI, settings: Settings, modules: list, menu_registry: MenuRegistry, perm_registry: PermissionRegistry, + csp_registry=None, ) -> None: """Install the full middleware pipeline. @@ -107,11 +126,14 @@ def install_middleware( if settings.is_development: app.add_middleware( SecurityHeadersMiddleware, - content_security_policy=SecurityHeadersMiddleware.dev_csp(settings.vite_dev_url), + content_security_policy=build_csp(settings, csp_registry), strict_transport_security=None, ) else: - app.add_middleware(SecurityHeadersMiddleware) + app.add_middleware( + SecurityHeadersMiddleware, + content_security_policy=build_csp(settings, csp_registry), + ) # Compress response bodies. Added here so it sits inside CorrelationId and # RequestLogging (which set headers and read request state) but outside # everything that produces a body — including the /static mount, where it diff --git a/framework/hosting/simple_module_hosting/_registrations.py b/framework/hosting/simple_module_hosting/_registrations.py index b1372e1c..75bc068b 100644 --- a/framework/hosting/simple_module_hosting/_registrations.py +++ b/framework/hosting/simple_module_hosting/_registrations.py @@ -26,6 +26,7 @@ def run_module_registrations( public_route_registry, design_pack_registry, audit_link_registry, + csp_registry=None, ) -> None: """Invoke each module's registration hooks, in dependency order. @@ -45,6 +46,8 @@ def run_module_registrations( mod.register_public_routes(public_route_registry) mod.register_design_packs(design_pack_registry) mod.register_audit_links(audit_link_registry) + if csp_registry is not None: + mod.register_csp_sources(csp_registry) health_registry.set_owner("") diff --git a/framework/hosting/simple_module_hosting/app_builder.py b/framework/hosting/simple_module_hosting/app_builder.py index 15ade822..9c7cd92d 100644 --- a/framework/hosting/simple_module_hosting/app_builder.py +++ b/framework/hosting/simple_module_hosting/app_builder.py @@ -34,6 +34,7 @@ register_host_settings, wire_module_routes, ) +from simple_module_core import CspSourceRegistry from simple_module_hosting._registrations import run_module_registrations from simple_module_hosting.health import router as health_router from simple_module_hosting.i18n_manifest import build_i18n_registry, emit_frontend_types @@ -206,6 +207,7 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: print_diagnostics(settings_diagnostics) # ── Phase 5: Module registrations ────────────────────── + csp_registry = CspSourceRegistry() run_module_registrations( modules, app=app, @@ -217,6 +219,7 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: public_route_registry=public_route_registry, design_pack_registry=design_pack_registry, audit_link_registry=audit_link_registry, + csp_registry=csp_registry, ) attach_public_routes(app, settings, public_route_registry) @@ -257,7 +260,9 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: register_exception_handlers(app, modules) # ── Phase 8: Middleware pipeline ─────────────────────── - install_middleware(app, settings, modules, menu_registry, perm_registry) + install_middleware( + app, settings, modules, menu_registry, perm_registry, csp_registry=csp_registry + ) # ── Phase 9: Routes, health, static files ────────────── for mod in modules: diff --git a/framework/hosting/tests/test_csp_module_sources.py b/framework/hosting/tests/test_csp_module_sources.py new file mode 100644 index 00000000..12e8d997 --- /dev/null +++ b/framework/hosting/tests/test_csp_module_sources.py @@ -0,0 +1,68 @@ +"""Module-declared CSP sources must reach the Content-Security-Policy header. + +Wiring half of the ``register_csp_sources`` hook: ``run_module_registrations`` +feeds the registry, and ``build_csp`` folds the collected origins into both +the dev CSP (Vite-widened) and the production default. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock + +from simple_module_core import ModuleBase, ModuleMeta +from simple_module_core.csp import CspSourceRegistry +from simple_module_hosting._phase_helpers import build_csp +from simple_module_hosting._registrations import run_module_registrations +from simple_module_hosting.settings import Settings + + +class NeedsFont(ModuleBase): + meta = ModuleMeta(name="NeedsFont") + + def register_csp_sources(self, registry: CspSourceRegistry) -> None: + registry.add("style-src", "https://rsms.me") + registry.add("font-src", "https://rsms.me") + + +def _filled_registry() -> CspSourceRegistry: + reg = CspSourceRegistry() + run_module_registrations( + [NeedsFont()], + app=MagicMock(), + event_bus=MagicMock(), + menu_registry=MagicMock(), + perm_registry=MagicMock(), + ff_registry=MagicMock(), + health_registry=MagicMock(), + public_route_registry=MagicMock(), + design_pack_registry=MagicMock(), + audit_link_registry=MagicMock(), + csp_registry=reg, + ) + return reg + + +class TestBuildCsp: + def test_dev_csp_carries_module_sources_and_vite(self) -> None: + dev = Settings(database_url="sqlite+aiosqlite:///:memory:", environment="development") + csp = build_csp(dev, _filled_registry()) + assert csp is not None + assert "https://rsms.me" in csp + assert dev.vite_dev_url in csp # dev widening preserved + + def test_prod_csp_carries_module_sources(self) -> None: + prod = Settings( + database_url="sqlite+aiosqlite:///:memory:", + environment="production", + secret_key="not-the-placeholder-secret-key-value", + ) + csp = build_csp(prod, _filled_registry()) + assert csp is not None + assert "https://rsms.me" in csp + assert "localhost:5050" not in csp # no dev widening in prod + + def test_no_registry_keeps_existing_policies(self, settings: Settings) -> None: + dev = build_csp(settings, None) + assert dev is not None + assert "https://fonts.googleapis.com" in dev + assert "https://rsms.me" not in dev From 2d31af154f63ad0dedadfa132a4f43f9231d1b17 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 19 Aug 2026 16:35:25 +0200 Subject: [PATCH 04/14] fix(cli): scaffolded Vite config derives port/origin from SM_VITE_DEV_URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The template hardcoded port 5050 + origin while the backend read SM_VITE_DEV_URL from .env — running on another port meant editing the generated file and the env var in sync. The config now derives both from the single .env value (process env wins; 5050 stays the default). Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc --- .../templates/host/client_app/vite.config.ts | 22 ++++++++- framework/cli/tests/test_cli_vite_port_env.py | 48 +++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 framework/cli/tests/test_cli_vite_port_env.py diff --git a/framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts b/framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts index d3bcb058..dbc21cad 100644 --- a/framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts +++ b/framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts @@ -29,6 +29,24 @@ function findNodeModulesRoot(start: string): string { } const fsRoot = findNodeModulesRoot(__dirname); +// The backend reads SM_VITE_DEV_URL from the project .env; deriving the dev +// server's port and origin from the same value keeps the two sides from +// drifting apart. Process env wins for one-off overrides; the documented +// default stays http://localhost:5050. +function viteDevUrl(): string { + if (process.env.SM_VITE_DEV_URL) return process.env.SM_VITE_DEV_URL; + try { + const env = fs.readFileSync(path.join(fsRoot, '.env'), 'utf8'); + const match = env.match(/^SM_VITE_DEV_URL\s*=\s*(\S+)\s*$/m); + if (match) return match[1].replace(/^['"]|['"]$/g, ''); + } catch { + // no .env yet (fresh checkout) — fall through to the default + } + return 'http://localhost:5050'; +} +const devUrl = viteDevUrl(); +const devPort = Number(new URL(devUrl).port || '5050'); + // Load the module pages manifest written by the Python host at boot. // Each entry points at an absolute pages/ directory — typically inside a // pip-installed module wheel. Vite needs these in server.fs.allow so the @@ -281,9 +299,9 @@ export default defineConfig({ }, }, server: { - port: 5050, + port: devPort, strictPort: true, - origin: 'http://localhost:5050', + origin: devUrl, fs: { allow: [fsRoot, ...moduleFsAllow], }, diff --git a/framework/cli/tests/test_cli_vite_port_env.py b/framework/cli/tests/test_cli_vite_port_env.py new file mode 100644 index 00000000..12bb5b2d --- /dev/null +++ b/framework/cli/tests/test_cli_vite_port_env.py @@ -0,0 +1,48 @@ +"""The scaffolded Vite config must derive its port from SM_VITE_DEV_URL. + +Field finding: `vite.config.ts` hardcoded `port: 5050, strictPort: true` +and a literal origin while the backend read `SM_VITE_DEV_URL` from `.env` — +running on any other port meant editing the generated file *and* the env +var in sync. The scaffold now derives both from the single env value. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +from simple_module_cli.cli import app +from typer.testing import CliRunner + + +def _scaffold(tmp_path: Path) -> Path: + runner = CliRunner() + result = runner.invoke( + app, + [ + "new", + "viteportapp", + "--dest", + str(tmp_path / "viteportapp"), + "--preset", + "minimal", + "--yes", + "--no-install", + ], + ) + assert result.exit_code == 0, result.output + return tmp_path / "viteportapp" / "host" / "client_app" / "vite.config.ts" + + +def test_vite_config_reads_sm_vite_dev_url(tmp_path: Path) -> None: + text = _scaffold(tmp_path).read_text(encoding="utf-8") + assert "SM_VITE_DEV_URL" in text + # port and origin both come from the derived URL — no literal pin left + assert re.search(r"port:\s*5050\b", text) is None + assert re.search(r"origin:\s*'http://localhost:5050'", text) is None + assert "strictPort: true" in text # still fail fast on a taken port + + +def test_vite_config_keeps_5050_as_fallback_only(tmp_path: Path) -> None: + text = _scaffold(tmp_path).read_text(encoding="utf-8") + assert "http://localhost:5050" in text # the documented default remains From e361935c13575b4aeacf5d8f193494660f50e7f6 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 19 Aug 2026 16:40:05 +0200 Subject: [PATCH 05/14] feat(hosting): opt-in session-bound CSRF primitive for modules RequiresCsrf router dependency + get_csrf_token view helper, lifting the design pagebuilder shipped first into the framework so every module shares one header (X-CSRF-Token) and one token-discovery convention. Docs cover the new register_csp_sources hook and the CSRF opt-in. Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc --- CLAUDE.md | 4 +- docs/framework-conventions.md | 35 ++++++++ .../hosting/simple_module_hosting/csrf.py | 77 ++++++++++++++++ framework/hosting/tests/test_hosting_csrf.py | 87 +++++++++++++++++++ 4 files changed, 201 insertions(+), 2 deletions(-) create mode 100644 framework/hosting/simple_module_hosting/csrf.py create mode 100644 framework/hosting/tests/test_hosting_csrf.py diff --git a/CLAUDE.md b/CLAUDE.md index 54db095f..e5155df3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -74,7 +74,7 @@ cascade layer is inert, while unlayered CSS beats every Tailwind utility — hence `SM022`/`SM023`. See `docs/module-authoring.md` § Styling. **Lifecycle hooks** (in `framework/core/simple_module_core/module.py`) — all no-op by default; subclasses override as needed: -`register_settings` → `register_menu_items` / `register_permissions` / `register_feature_flags` / `register_event_handlers` / `register_health_checks` / `register_public_routes` → `register_exception_handlers` → `register_middleware` → `register_routes(api_router, view_router)` → async `on_startup` / `on_shutdown` (reverse order). `register_public_routes(registry)` lets a module exempt anonymous/read-only routes (STAC/OGC, webhooks) from `AuthMiddleware`; rules are method-aware (`registry.add_regex(r"…/tilejson$", methods={"GET"})`), so a GET read route can be public while sibling POST/PATCH mutations under the same prefix stay gated. See [docs/framework/public-routes.md](docs/framework/public-routes.md). +`register_settings` → `register_menu_items` / `register_permissions` / `register_feature_flags` / `register_event_handlers` / `register_health_checks` / `register_public_routes` / `register_csp_sources` → `register_exception_handlers` → `register_middleware` → `register_routes(api_router, view_router)` → async `on_startup` / `on_shutdown` (reverse order). `register_csp_sources(registry)` lets a module whitelist external asset origins (`registry.add("style-src", "https://rsms.me")`) — fetch directives only, validated at boot. `register_public_routes(registry)` lets a module exempt anonymous/read-only routes (STAC/OGC, webhooks) from `AuthMiddleware`; rules are method-aware (`registry.add_regex(r"…/tilejson$", methods={"GET"})`), so a GET read route can be public while sibling POST/PATCH mutations under the same prefix stay gated. See [docs/framework/public-routes.md](docs/framework/public-routes.md). **Middleware pipeline** (Starlette `add_middleware` is LIFO — last added runs first). Execution order on a request: `(ProxyHeaders, if SM_TRUSTED_PROXY) → CorrelationId → RequestLogging → GZip → SecurityHeaders → Session → → Tenant (opt-in) → Locale → InertiaLayoutData → app`. `GZip` compresses any response over 500 bytes, including the `/static` mount — the built CSS is ~139 KB raw versus ~21 KB gzipped, and uncompressed assets dominated cold page load. `ProxyHeaders` (uvicorn's `ProxyHeadersMiddleware`) is installed only when `SM_TRUSTED_PROXY` is set, sitting outermost so the `X-Forwarded-*`-corrected scheme/client IP reach everything downstream (request logs and Inertia's absolute page url). When two modules add middleware at the same dependency tier, the module that sorts **later** wraps outermost. Use `depends_on` to express relative order — don't rely on names. @@ -87,7 +87,7 @@ Standard mixins in `simple_module_db.mixins`: `AuditMixin`, `SoftDeleteMixin` (b **Inertia**. `inertia.render("/", ...)` maps to `modules///pages/.tsx`, where `` is the PascalCase of the module directory (`blog_posts` → `BlogPosts`). Host-level pages under `host/client_app/pages/` use a bare ``. `InertiaLayoutDataMiddleware` populates shared props (`auth`, `menus`, `i18n`); use `InertiaDep` from `simple_module_hosting.inertia_deps`. Mismatched keys fire `SM003` (orphan page) / `SM004` (phantom render). -**CSRF defence**. There is no explicit CSRF token middleware. Protection comes from `SameSite=Lax` on the session cookie (Starlette default): browsers don't attach the cookie to cross-site POST/PUT/DELETE, so a forged form-submit from another origin is unauthenticated. Raw `fetch()` calls in page code don't need a token header. +**CSRF defence**. Baseline protection comes from `SameSite=Lax` on the session cookie (Starlette default): browsers don't attach the cookie to cross-site POST/PUT/DELETE, so a forged form-submit from another origin is unauthenticated. Raw `fetch()` calls in page code don't need a token header by default. Modules wanting defence in depth opt into `simple_module_hosting.csrf` — `RequiresCsrf` as a router dependency plus `get_csrf_token(request)` exposed as a view prop; callers echo it as `X-CSRF-Token` on unsafe methods. ## Conventions to follow diff --git a/docs/framework-conventions.md b/docs/framework-conventions.md index f7ea307c..a29095bb 100644 --- a/docs/framework-conventions.md +++ b/docs/framework-conventions.md @@ -300,6 +300,41 @@ which `AuthMiddleware` consults on every request. See [`docs/framework/public-routes.md`](framework/public-routes.md) for match kinds and resolution order. +### CSP sources (external assets) + +The host ships a strict Content-Security-Policy. A module whose frontend loads +an asset from another origin — a font CDN, a tile server — declares it via +`register_csp_sources`, and the host folds the origins into both the dev +(Vite-widened) and production policies: + +```python +def register_csp_sources(self, registry): + registry.add("style-src", "https://rsms.me") + registry.add("font-src", "https://rsms.me") +``` + +Only fetch directives are extendable (`style-src`, `font-src`, `img-src`, +`connect-src`, …) — never `default-src`, `base-uri`, `form-action`, or +`frame-ancestors`, which belong to the host operator. Sources are validated +single tokens; an invalid declaration raises at boot. + +### CSRF (opt-in token check) + +The framework baseline is `SameSite=Lax` on the session cookie. Modules that +want defence in depth opt into a session-bound token check per router: + +```python +from simple_module_hosting.csrf import RequiresCsrf, get_csrf_token + +router = APIRouter(dependencies=[Depends(RequiresCsrf())]) +# expose the token to the frontend as a view prop: +{"csrf_token": get_csrf_token(request)} +``` + +Callers echo the token back as `X-CSRF-Token` on `POST`/`PUT`/`PATCH`/`DELETE`; +safe methods are never checked, and apps without `SessionMiddleware` (bare +test apps) are exempt. + ### Design packs (site-wide look) A *design pack* is a stylesheet a module ships that restyles the public site by diff --git a/framework/hosting/simple_module_hosting/csrf.py b/framework/hosting/simple_module_hosting/csrf.py new file mode 100644 index 00000000..3231668a --- /dev/null +++ b/framework/hosting/simple_module_hosting/csrf.py @@ -0,0 +1,77 @@ +"""Opt-in session-bound CSRF protection for module mutation endpoints. + +The framework's baseline CSRF defence is ``SameSite=Lax`` on the session +cookie: browsers don't attach it to cross-site POSTs, so a forged form +submit arrives unauthenticated. Modules that want defence in depth (or must +satisfy a stricter audit) opt into a token check per router:: + + from simple_module_hosting.csrf import RequiresCsrf, get_csrf_token + + router = APIRouter(dependencies=[Depends(RequiresCsrf())]) + + # expose the token to the frontend, e.g. as an Inertia prop: + await inertia.render("MyModule/Page", {"csrf_token": get_csrf_token(request)}) + +Callers send the token back as ``X-CSRF-Token`` on POST/PUT/PATCH/DELETE. +Safe methods are never checked. Apps that mount no ``SessionMiddleware`` +(bare unit-test apps) are exempt — without a session there is nothing to +bind a token to. + +This lifts the design the pagebuilder module shipped first +(``pagebuilder/security.py``) into the framework, so every module shares +one header name and one token-discovery convention. +""" + +from __future__ import annotations + +import secrets + +from fastapi import HTTPException, Request + +__all__ = ["CSRF_HEADER", "RequiresCsrf", "get_csrf_token"] + +CSRF_HEADER = "X-CSRF-Token" +_SESSION_KEY = "sm_csrf_token" +_UNSAFE_METHODS = frozenset({"POST", "PUT", "PATCH", "DELETE"}) + + +def get_csrf_token(request: Request) -> str: + """Return the session's CSRF token, generating and persisting it if needed. + + Returns ``""`` when no session is mounted so view code can pass it + straight into page props without a None check. + """ + session = request.scope.get("session") + if session is None: + return "" + token = session.get(_SESSION_KEY) + if not token: + token = secrets.token_urlsafe(32) + session[_SESSION_KEY] = token + return token + + +class RequiresCsrf: + """FastAPI dependency enforcing the CSRF header on unsafe methods. + + Attach at the router level so every mutation under it is covered:: + + router = APIRouter(dependencies=[Depends(RequiresCsrf())]) + """ + + def __call__(self, request: Request) -> None: + if request.method not in _UNSAFE_METHODS: + return + session = request.scope.get("session") + if session is None: + return # no session mounted — nothing to bind a token to + expected = get_csrf_token(request) + provided = request.headers.get(CSRF_HEADER, "") + if not provided or not secrets.compare_digest(provided, expected): + raise HTTPException( + status_code=403, + detail=( + f"CSRF token missing or invalid — read it via get_csrf_token " + f"(exposed in the view's props) and send it as {CSRF_HEADER}." + ), + ) diff --git a/framework/hosting/tests/test_hosting_csrf.py b/framework/hosting/tests/test_hosting_csrf.py new file mode 100644 index 00000000..8b28fb0b --- /dev/null +++ b/framework/hosting/tests/test_hosting_csrf.py @@ -0,0 +1,87 @@ +"""Opt-in CSRF primitive for module mutation endpoints. + +Field finding: with the framework offering only SameSite=Lax, the +pagebuilder module built its own session-bound token middleware — and every +content module after it would reinvent the same wheel with its own header +name and token-discovery convention. ``simple_module_hosting.csrf`` lifts +the proven design into the framework: one header (``X-CSRF-Token``), one +session key, one dependency modules opt into. +""" + +from __future__ import annotations + +import httpx +import pytest +from fastapi import APIRouter, Depends, FastAPI, Request +from simple_module_hosting.csrf import CSRF_HEADER, RequiresCsrf, get_csrf_token +from starlette.middleware.sessions import SessionMiddleware + + +def _app(*, with_session: bool = True) -> FastAPI: + app = FastAPI() + router = APIRouter(dependencies=[Depends(RequiresCsrf())]) + + @app.get("/token") + def token(request: Request) -> dict: + return {"token": get_csrf_token(request)} + + @router.get("/things") + def list_things() -> dict: + return {"ok": True} + + @router.post("/things") + def create_thing() -> dict: + return {"created": True} + + app.include_router(router) + if with_session: + app.add_middleware(SessionMiddleware, secret_key="test-secret") + return app + + +@pytest.fixture +async def client(): + transport = httpx.ASGITransport(app=_app()) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as c: + yield c + + +class TestRequiresCsrf: + async def test_token_endpoint_returns_a_token(self, client: httpx.AsyncClient) -> None: + resp = await client.get("/token") + assert resp.status_code == 200 + assert resp.json()["token"] + + async def test_safe_method_needs_no_token(self, client: httpx.AsyncClient) -> None: + assert (await client.get("/things")).status_code == 200 + + async def test_post_without_token_is_403(self, client: httpx.AsyncClient) -> None: + resp = await client.post("/things") + assert resp.status_code == 403 + assert CSRF_HEADER in resp.json()["detail"] + + async def test_post_with_wrong_token_is_403(self, client: httpx.AsyncClient) -> None: + await client.get("/token") # establish a session token + resp = await client.post("/things", headers={CSRF_HEADER: "forged"}) + assert resp.status_code == 403 + + async def test_post_with_token_succeeds(self, client: httpx.AsyncClient) -> None: + token = (await client.get("/token")).json()["token"] + resp = await client.post("/things", headers={CSRF_HEADER: token}) + assert resp.status_code == 200 + assert resp.json() == {"created": True} + + async def test_token_is_stable_within_a_session(self, client: httpx.AsyncClient) -> None: + first = (await client.get("/token")).json()["token"] + second = (await client.get("/token")).json()["token"] + assert first == second + + +class TestWithoutSessionMiddleware: + """Bare test apps mount no SessionMiddleware — the primitive must not trap them.""" + + async def test_enforcement_is_skipped_and_token_empty(self) -> None: + transport = httpx.ASGITransport(app=_app(with_session=False)) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as c: + assert (await c.get("/token")).json()["token"] == "" + assert (await c.post("/things")).status_code == 200 From 2185737dc3a4600dafb47ce1d1c87776e3db052d Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 19 Aug 2026 16:45:41 +0200 Subject: [PATCH 06/14] style+fix: file-size cap for vite template (extract vite.dev-url.ts), lint fixes Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc --- .../templates/host/client_app/vite.config.ts | 20 +++---------------- .../templates/host/client_app/vite.dev-url.ts | 19 ++++++++++++++++++ framework/cli/tests/test_cli_vite_port_env.py | 14 ++++++------- framework/core/simple_module_core/__init__.py | 4 ++-- framework/core/simple_module_core/module.py | 2 +- framework/core/tests/test_csp_registry.py | 4 +++- .../simple_module_hosting/app_builder.py | 2 +- .../bootstrap_settings.py | 2 +- .../tests/test_api_error_negotiation.py | 1 + 9 files changed, 38 insertions(+), 30 deletions(-) create mode 100644 framework/cli/simple_module_cli/templates/host/client_app/vite.dev-url.ts diff --git a/framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts b/framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts index dbc21cad..747545c3 100644 --- a/framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts +++ b/framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts @@ -3,6 +3,7 @@ import path from 'node:path'; import tailwindcss from '@tailwindcss/vite'; import react from '@vitejs/plugin-react'; import { type Plugin, defineConfig } from 'vite'; +import { viteDevPort, viteDevUrl } from './vite.dev-url'; // Force every importer (host, workspace module, wheel-installed module) // to resolve to one React copy + a single Inertia hook context. Without @@ -29,23 +30,8 @@ function findNodeModulesRoot(start: string): string { } const fsRoot = findNodeModulesRoot(__dirname); -// The backend reads SM_VITE_DEV_URL from the project .env; deriving the dev -// server's port and origin from the same value keeps the two sides from -// drifting apart. Process env wins for one-off overrides; the documented -// default stays http://localhost:5050. -function viteDevUrl(): string { - if (process.env.SM_VITE_DEV_URL) return process.env.SM_VITE_DEV_URL; - try { - const env = fs.readFileSync(path.join(fsRoot, '.env'), 'utf8'); - const match = env.match(/^SM_VITE_DEV_URL\s*=\s*(\S+)\s*$/m); - if (match) return match[1].replace(/^['"]|['"]$/g, ''); - } catch { - // no .env yet (fresh checkout) — fall through to the default - } - return 'http://localhost:5050'; -} -const devUrl = viteDevUrl(); -const devPort = Number(new URL(devUrl).port || '5050'); +const devUrl = viteDevUrl(fsRoot); +const devPort = viteDevPort(devUrl); // Load the module pages manifest written by the Python host at boot. // Each entry points at an absolute pages/ directory — typically inside a diff --git a/framework/cli/simple_module_cli/templates/host/client_app/vite.dev-url.ts b/framework/cli/simple_module_cli/templates/host/client_app/vite.dev-url.ts new file mode 100644 index 00000000..0411f564 --- /dev/null +++ b/framework/cli/simple_module_cli/templates/host/client_app/vite.dev-url.ts @@ -0,0 +1,19 @@ +import fs from 'node:fs'; +import path from 'node:path'; + +// SM_VITE_DEV_URL (process env, then the project .env) drives the dev +// server's port and origin, so Vite and the backend read the same value and +// can never drift apart. The documented default stays http://localhost:5050. +export function viteDevUrl(envDir: string): string { + if (process.env.SM_VITE_DEV_URL) return process.env.SM_VITE_DEV_URL; + let env = ''; + try { + env = fs.readFileSync(path.join(envDir, '.env'), 'utf8'); + } catch {} // no .env yet (fresh checkout) — use the default + const m = env.match(/^SM_VITE_DEV_URL\s*=\s*(\S+)\s*$/m); + return m ? m[1].replace(/^['"]|['"]$/g, '') : 'http://localhost:5050'; +} + +export function viteDevPort(devUrl: string): number { + return Number(new URL(devUrl).port || '5050'); +} diff --git a/framework/cli/tests/test_cli_vite_port_env.py b/framework/cli/tests/test_cli_vite_port_env.py index 12bb5b2d..f24e597d 100644 --- a/framework/cli/tests/test_cli_vite_port_env.py +++ b/framework/cli/tests/test_cli_vite_port_env.py @@ -34,15 +34,15 @@ def _scaffold(tmp_path: Path) -> Path: return tmp_path / "viteportapp" / "host" / "client_app" / "vite.config.ts" -def test_vite_config_reads_sm_vite_dev_url(tmp_path: Path) -> None: - text = _scaffold(tmp_path).read_text(encoding="utf-8") - assert "SM_VITE_DEV_URL" in text +def test_vite_config_derives_port_from_env_url(tmp_path: Path) -> None: + config = _scaffold(tmp_path) + text = config.read_text(encoding="utf-8") # port and origin both come from the derived URL — no literal pin left assert re.search(r"port:\s*5050\b", text) is None assert re.search(r"origin:\s*'http://localhost:5050'", text) is None + assert "viteDevUrl" in text assert "strictPort: true" in text # still fail fast on a taken port - -def test_vite_config_keeps_5050_as_fallback_only(tmp_path: Path) -> None: - text = _scaffold(tmp_path).read_text(encoding="utf-8") - assert "http://localhost:5050" in text # the documented default remains + helper = (config.parent / "vite.dev-url.ts").read_text(encoding="utf-8") + assert "SM_VITE_DEV_URL" in helper + assert "http://localhost:5050" in helper # the documented default remains diff --git a/framework/core/simple_module_core/__init__.py b/framework/core/simple_module_core/__init__.py index 9f8b70b3..a9d116b8 100644 --- a/framework/core/simple_module_core/__init__.py +++ b/framework/core/simple_module_core/__init__.py @@ -49,6 +49,8 @@ "AuditLink", "AuditLinkRegistry", "CircularDependencyError", + "CspSourceError", + "CspSourceRegistry", "DesignPack", "DesignPackRegistry", "DiagnosticLevel", @@ -73,8 +75,6 @@ "NotFoundError", "PermissionRegistry", "PublicRoute", - "CspSourceError", - "CspSourceRegistry", "PublicRouteRegistry", "Services", "Translator", diff --git a/framework/core/simple_module_core/module.py b/framework/core/simple_module_core/module.py index b7f81d01..a9314984 100644 --- a/framework/core/simple_module_core/module.py +++ b/framework/core/simple_module_core/module.py @@ -11,13 +11,13 @@ from fastapi import APIRouter, FastAPI from simple_module_core.audit_links import AuditLinkRegistry + from simple_module_core.csp import CspSourceRegistry from simple_module_core.design_packs import DesignPackRegistry from simple_module_core.events import EventBus from simple_module_core.feature_flags import FeatureFlagRegistry from simple_module_core.health import HealthRegistry from simple_module_core.menu import MenuRegistry from simple_module_core.permissions import PermissionRegistry - from simple_module_core.csp import CspSourceRegistry from simple_module_core.public_routes import PublicRouteRegistry diff --git a/framework/core/tests/test_csp_registry.py b/framework/core/tests/test_csp_registry.py index 30004de3..3216bc06 100644 --- a/framework/core/tests/test_csp_registry.py +++ b/framework/core/tests/test_csp_registry.py @@ -25,7 +25,9 @@ def test_extends_existing_directive(self) -> None: reg = CspSourceRegistry() reg.add("style-src", "https://rsms.me") out = reg.extend_policy(_POLICY) - assert "style-src 'self' 'unsafe-inline' https://fonts.googleapis.com https://rsms.me" in out + assert ( + "style-src 'self' 'unsafe-inline' https://fonts.googleapis.com https://rsms.me" in out + ) def test_duplicate_and_already_present_sources_are_not_repeated(self) -> None: reg = CspSourceRegistry() diff --git a/framework/hosting/simple_module_hosting/app_builder.py b/framework/hosting/simple_module_hosting/app_builder.py index 9c7cd92d..f65a1387 100644 --- a/framework/hosting/simple_module_hosting/app_builder.py +++ b/framework/hosting/simple_module_hosting/app_builder.py @@ -9,6 +9,7 @@ from pathlib import Path from fastapi import FastAPI +from simple_module_core import CspSourceRegistry from simple_module_core.audit_links import AuditLinkRegistry from simple_module_core.design_packs import DesignPackRegistry from simple_module_core.diagnostics import DiagnosticLevel, print_diagnostics, run_diagnostics @@ -34,7 +35,6 @@ register_host_settings, wire_module_routes, ) -from simple_module_core import CspSourceRegistry from simple_module_hosting._registrations import run_module_registrations from simple_module_hosting.health import router as health_router from simple_module_hosting.i18n_manifest import build_i18n_registry, emit_frontend_types diff --git a/framework/hosting/simple_module_hosting/bootstrap_settings.py b/framework/hosting/simple_module_hosting/bootstrap_settings.py index fcc067fa..30083148 100644 --- a/framework/hosting/simple_module_hosting/bootstrap_settings.py +++ b/framework/hosting/simple_module_hosting/bootstrap_settings.py @@ -76,7 +76,7 @@ def _absolutize_sqlite_url(url: str, *, anchor: Path) -> str: if not url.startswith("sqlite"): return url scheme, sep, rest = url.partition(":///") - if not sep or not rest or rest.startswith("/") or rest.startswith(":memory:"): + if not sep or not rest or rest.startswith(("/", ":memory:")): return url path_part, query_sep, query = rest.partition("?") resolved = (anchor / path_part).resolve() diff --git a/framework/hosting/tests/test_api_error_negotiation.py b/framework/hosting/tests/test_api_error_negotiation.py index 2d8140bf..9aaf5938 100644 --- a/framework/hosting/tests/test_api_error_negotiation.py +++ b/framework/hosting/tests/test_api_error_negotiation.py @@ -58,6 +58,7 @@ async def test_json_accept_on_view_path_returns_json(self) -> None: ) assert isinstance(resp, JSONResponse) + class TestClientNegotiation: async def test_api_404_is_json(self, authenticated_client: httpx.AsyncClient) -> None: resp = await authenticated_client.get("/api/definitely/not/a/route") From 5d4c9d6922a48d43b51fc3f1701746209d08399a Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 19 Aug 2026 20:44:49 +0200 Subject: [PATCH 07/14] docs: cover the five framework fixes across the doc tree lifecycle.md gains the register_csp_sources section + hook listings; middleware.md points at the CSP hook and the CSRF opt-in; module-authoring gains External asset origins (CSP) and CSRF on mutation endpoints sections; env-vars/configuration document SM_PROJECT_ROOT, relative-sqlite anchoring, and SM_VITE_DEV_URL as the single dev-port knob; framework-conventions adds the HTML-vs-JSON error contract. Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc --- docs/framework-conventions.md | 15 ++++++++++++++ docs/framework/lifecycle.md | 20 +++++++++++++++++++ docs/framework/middleware.md | 4 ++-- docs/guide/configuration.md | 4 ++-- docs/module-authoring.md | 37 +++++++++++++++++++++++++++++++++++ docs/reference/env-vars.md | 7 ++++--- 6 files changed, 80 insertions(+), 7 deletions(-) diff --git a/docs/framework-conventions.md b/docs/framework-conventions.md index a29095bb..4e901d26 100644 --- a/docs/framework-conventions.md +++ b/docs/framework-conventions.md @@ -335,6 +335,21 @@ Callers echo the token back as `X-CSRF-Token` on `POST`/`PUT`/`PATCH`/`DELETE`; safe methods are never checked, and apps without `SessionMiddleware` (bare test apps) are exempt. +### Error responses (HTML vs JSON) + +403/404/500 are content-negotiated. Requests under `/api/*` — the documented +prefix for every module's JSON surface — or with an explicit +`Accept: application/json` get a JSON body: + +```json +{ "detail": "Permission required: pagebuilder.edit" } +``` + +Browser-shaped requests (navigations, Inertia visits, `Accept: */*`) get the +rendered Inertia error page, which carries the layout, i18n copy, and the +request's correlation id. Module endpoint code doesn't opt in or out — raise +`HTTPException` as usual and the handler picks the right shape. + ### Design packs (site-wide look) A *design pack* is a stylesheet a module ships that restyles the public site by diff --git a/docs/framework/lifecycle.md b/docs/framework/lifecycle.md index cbed6fac..4589fad3 100644 --- a/docs/framework/lifecycle.md +++ b/docs/framework/lifecycle.md @@ -12,6 +12,7 @@ register_feature_flags register_event_handlers register_health_checks register_public_routes +register_csp_sources register_design_packs register_exception_handlers register_middleware @@ -128,6 +129,24 @@ async def _check_db(self) -> HealthCheckResult: ... Each check returns a `HealthCheckResult(status=HealthStatus.HEALTHY | DEGRADED | UNHEALTHY, detail=...)`. The `/health/ready` endpoint runs all checks concurrently and reports the worst status (a raising check counts as `UNHEALTHY`). +## `register_csp_sources(registry)` + +Whitelist external origins your frontend loads assets from — a font CDN, a +tile server, an analytics endpoint. The host ships a strict +Content-Security-Policy; without a declaration the browser blocks the request. + +```python +def register_csp_sources(self, registry) -> None: + registry.add("style-src", "https://rsms.me") + registry.add("font-src", "https://rsms.me") +``` + +Only fetch directives (`style-src`, `font-src`, `img-src`, `connect-src`, …) +can be extended — never `default-src`, `base-uri`, `form-action`, or +`frame-ancestors`, which belong to the host operator. Each source must be a +single origin/scheme token; invalid declarations raise at boot. The origins +land in both the development (Vite-widened) and production policies. + ## `register_exception_handlers(app)` Register FastAPI exception handlers scoped to your module's exceptions: @@ -214,6 +233,7 @@ class OrdersModule(ModuleBase): def register_event_handlers(self, bus, app=None): ... def register_health_checks(self, registry): ... def register_public_routes(self, registry): ... + def register_csp_sources(self, registry): ... def register_exception_handlers(self, app): ... def register_middleware(self, app): ... def register_routes(self, api_router, view_router): ... diff --git a/docs/framework/middleware.md b/docs/framework/middleware.md index 27a102bd..1773b899 100644 --- a/docs/framework/middleware.md +++ b/docs/framework/middleware.md @@ -93,11 +93,11 @@ Emits a structured log line per request with method, path, status, duration, and ### `SecurityHeadersMiddleware` -Sets conservative defaults: `X-Content-Type-Options: nosniff`, `Referrer-Policy: strict-origin-when-cross-origin`, `X-Frame-Options: SAMEORIGIN`, `X-XSS-Protection: 0` (the legacy auditor is disabled in favour of CSP), plus a default CSP and — outside development — HSTS. In development the CSP is widened for the Vite dev origin and HSTS is suppressed. Override on a per-route basis with your own response headers. +Sets conservative defaults: `X-Content-Type-Options: nosniff`, `Referrer-Policy: strict-origin-when-cross-origin`, `X-Frame-Options: SAMEORIGIN`, `X-XSS-Protection: 0` (the legacy auditor is disabled in favour of CSP), plus a default CSP and — outside development — HSTS. In development the CSP is widened for the Vite dev origin and HSTS is suppressed. Modules that load assets from an external origin extend the policy through the [`register_csp_sources`](lifecycle.md#register_csp_sourcesregistry) hook; both the dev and production variants honor those origins. Override on a per-route basis with your own response headers. ### `SessionMiddleware` -Starlette's built-in signed-cookie sessions. Cookie name is `session`; attributes are `HttpOnly`, `SameSite=Lax`. `SameSite=Lax` is the CSRF defence: browsers don't attach the cookie to cross-site POST/PUT/DELETE, so a forged form submission from another origin is unauthenticated. +Starlette's built-in signed-cookie sessions. Cookie name is `session`; attributes are `HttpOnly`, `SameSite=Lax`. `SameSite=Lax` is the baseline CSRF defence: browsers don't attach the cookie to cross-site POST/PUT/DELETE, so a forged form submission from another origin is unauthenticated. Modules wanting defence in depth opt into the session-bound token check in `simple_module_hosting.csrf` — `RequiresCsrf` as a router dependency, `get_csrf_token(request)` exposed as a view prop, and callers echoing it as `X-CSRF-Token` on unsafe methods. ### `TenantMiddleware` *(opt-in)* diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index 9802e1f1..d88db68f 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -13,10 +13,10 @@ Prefix is always `SM_`. These are the pre-DB knobs read by `simple_module_hostin | Variable | Default | Notes | |---|---|---| -| `SM_DATABASE_URL` | `sqlite+aiosqlite:///./app.db` | **Required in production.** Async URL. Postgres: `postgresql+asyncpg://…` | +| `SM_DATABASE_URL` | `sqlite+aiosqlite:///./app.db` | **Required in production.** Async URL. Postgres: `postgresql+asyncpg://…` Relative sqlite paths resolve against the project root (the `.env` location), not the cwd. | | `SM_ENVIRONMENT` | `development` | Any value other than `development`, `test`, `testing` triggers strict discovery + placeholder-secret checks. | | `SM_SECRET_KEY` | `change-me-in-production` | **Must** be overridden in production — session cookie signing key. | -| `SM_VITE_DEV_URL` | `http://localhost:5050` | Dev only — where the Vite HMR client connects. | +| `SM_VITE_DEV_URL` | `http://localhost:5050` | Dev only — where the Vite HMR client connects. Scaffolded apps derive the Vite dev server's port and origin from this same value, so it's the single knob for moving off 5050. | | `SM_DEBUG` | `false` | Enables debug mode (shows tracebacks in HTTP responses). | | `SM_LOG_LEVEL` | `INFO` | `DEBUG`/`INFO`/`WARNING`/`ERROR` | | `SM_LOG_FORMAT` | `json` | `json` (structured) or `text`. | diff --git a/docs/module-authoring.md b/docs/module-authoring.md index 1a427695..9a893524 100644 --- a/docs/module-authoring.md +++ b/docs/module-authoring.md @@ -406,6 +406,43 @@ package dir (a `force-include` maps it in), and `static/dist` is gitignored (an `artifacts` entry ships it when present without failing the build when it isn't). +## External asset origins (CSP) + +The host ships a strict Content-Security-Policy. If your pages load anything +from another origin — a font CDN, a tile server, a third-party API — declare +it, or the browser blocks the request and your feature silently breaks in +every host: + +```python +def register_csp_sources(self, registry): + registry.add("style-src", "https://rsms.me") + registry.add("font-src", "https://rsms.me") +``` + +Only fetch directives can be extended; each source must be a single +origin/scheme token, validated at boot. See +[docs/framework/lifecycle.md](framework/lifecycle.md#register_csp_sourcesregistry). + +## CSRF on mutation endpoints + +The framework's baseline CSRF defence is `SameSite=Lax` on the session +cookie — plain Inertia forms need nothing extra. For defence in depth on a +module's JSON mutation surface, opt into the framework's session-bound token +check instead of rolling your own: + +```python +from simple_module_hosting.csrf import RequiresCsrf, get_csrf_token + +router = APIRouter(dependencies=[Depends(RequiresCsrf())]) + +# expose the token to your pages as a view prop: +await inertia.render("MyModule/Page", {"csrf_token": get_csrf_token(request)}) +``` + +Frontend callers echo the token back as `X-CSRF-Token` on +`POST`/`PUT`/`PATCH`/`DELETE`. Safe methods are never checked, and bare test +apps without `SessionMiddleware` are exempt, so unit tests need no ceremony. + ## Developing out-of-tree A module in its own repo has no host around it — these are the three diff --git a/docs/reference/env-vars.md b/docs/reference/env-vars.md index b1e02e43..7e1ad499 100644 --- a/docs/reference/env-vars.md +++ b/docs/reference/env-vars.md @@ -8,15 +8,16 @@ This is the full reference. See [Configuration](/guide/configuration) for a narr | Variable | Default | Notes | |---|---|---| -| `SM_DATABASE_URL` | `sqlite+aiosqlite:///./app.db` | **Required in production.** Async URL: `postgresql+asyncpg://user:pw@host:5432/db` or `sqlite+aiosqlite:///./app.db`. | +| `SM_DATABASE_URL` | `sqlite+aiosqlite:///./app.db` | **Required in production.** Async URL: `postgresql+asyncpg://user:pw@host:5432/db` or `sqlite+aiosqlite:///./app.db`. Relative sqlite paths resolve against the project root (the `.env` location), not the process cwd — CLI tools run from `host/` hit the same file as the app. | | `SM_ENVIRONMENT` | `development` | `development` and `testing` are the only non-prod values (placeholder-secret check is skipped for both). Any value other than `development` triggers strict module discovery. | | `SM_SECRET_KEY` | `change-me-in-production` | **Must** be overridden in production — session cookie signing key. | | `SM_DEBUG` | `false` | Enables debug mode (tracebacks in HTTP responses). | | `SM_LOG_LEVEL` | `INFO` | `DEBUG`/`INFO`/`WARNING`/`ERROR`. | | `SM_LOG_FORMAT` | `json` | `text` for readable dev logs, `json` for structured logs in prod. | | `SM_MODULES_ENABLED` | unset | Comma-separated allow-list to disable modules without uninstalling them. | -| `SM_VITE_DEV_URL` | `http://localhost:5050` | Dev only — where the Vite HMR client connects. | -| `SM_VITE_PORT` | `5050` | Dev only — port the Vite dev server binds to (read by `vite.config.ts`). If you change it, set `SM_VITE_DEV_URL` to match so the backend points the HMR client at the right origin. | +| `SM_VITE_DEV_URL` | `http://localhost:5050` | Dev only — where the Vite HMR client connects. Freshly scaffolded apps also derive the Vite dev server's port and origin from this one value (via `client_app/vite.dev-url.ts`), so changing it here is the whole move. | +| `SM_VITE_PORT` | `5050` | Dev only — port this repo's own host `vite.config.ts` binds to. If you change it, set `SM_VITE_DEV_URL` to match so the backend points the HMR client at the right origin. New scaffolds don't need it — they read `SM_VITE_DEV_URL` directly. | +| `SM_PROJECT_ROOT` | unset | Overrides project-root discovery: where the `.env` is looked up and what relative sqlite paths resolve against. Normally unnecessary — settings walk up from the cwd (stopping at repo boundaries and `$HOME`) to find the `.env` on their own. | | `SM_AUTH_PUBLIC_PATHS` | `[]` | JSON array of host-level anonymous-access path prefixes. Escape hatch for exposing a route without a session when no module owns it; modules should prefer the method-aware `register_public_routes` hook. | ## DB connection pool From dd8c54e52f7db8f072d35889686908a906452489 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 19 Aug 2026 20:48:05 +0200 Subject: [PATCH 08/14] docs(skills): teach the shipped agent skills the five framework fixes creating: register_csp_sources in the hooks table; conventions: CSRF section rewritten around the opt-in hosting primitive + new CSP-sources convention; registries: phase-5 hook chain extended; inertia-pages: JSON error bodies and X-CSRF-Token notes on the fetch() path; cli: SM_VITE_DEV_URL as the single dev-port knob. The CLI bundle is a symlink to skills/, so smpy skills add/update ships the same content. Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc --- skills/simple-module-cli/SKILL.md | 1 + skills/simple-module-conventions/SKILL.md | 17 +++++++++++++++-- skills/simple-module-creating/SKILL.md | 1 + skills/simple-module-inertia-pages/SKILL.md | 2 ++ skills/simple-module-registries/SKILL.md | 2 +- 5 files changed, 20 insertions(+), 3 deletions(-) diff --git a/skills/simple-module-cli/SKILL.md b/skills/simple-module-cli/SKILL.md index c23701b5..d53de601 100644 --- a/skills/simple-module-cli/SKILL.md +++ b/skills/simple-module-cli/SKILL.md @@ -198,6 +198,7 @@ Don't bake `--password` literals into a script you commit; use a secrets store a - **Used `smpy create-module` to add a module to an existing host.** That command is for **publishable** packages, intended to live in their own repo. To add a module to an existing host: install it (`pip install simple_module_` or add to `pyproject.toml` and `uv sync`), then autogenerate a migration. See **simple-module-creating** + **simple-module-migrations**. - **Calling `smpy create-admin` before migrations have run.** The users tables don't exist yet; the command will error. Run `alembic upgrade head` first (or use `smpy new` which does it for you when `--no-install` isn't set). - **Expecting `--with products` (or `keycloak` / `audit_log`) to work on `smpy new`.** Those modules exist in the monorepo but aren't scaffolder-catalog keys, so `smpy new --with products` errors. Either scaffold without them and `pip install simple_module_products` into the host afterward, or use `smpy create-host --with Products` (which doesn't validate against the catalog). +- **Port 5050 already taken when starting a scaffolded app.** Set `SM_VITE_DEV_URL` in the project `.env` (e.g. `http://localhost:5310`) — the scaffolded `vite.config.ts` derives the dev server's port and origin from that same value, so it's the only knob to turn. - **`smpy package-update` "did nothing" for a workspace dep.** Deps sourced from a `[tool.uv.sources]` workspace/path/git/URL entry are intentionally skipped (their version isn't on PyPI) — they show up in the output as `skipped`, not updated. Also remember to run `uv sync` after; `package-update` only rewrites the constraint strings. ## Related skills diff --git a/skills/simple-module-conventions/SKILL.md b/skills/simple-module-conventions/SKILL.md index 08d43e27..3ede1443 100644 --- a/skills/simple-module-conventions/SKILL.md +++ b/skills/simple-module-conventions/SKILL.md @@ -76,9 +76,18 @@ export function useProductSchema() { **Why:** `t()` resolves once when the schema is constructed. Module-scope construction = first-render-locale-only forever, no matter what the user switches to. -### 6. CSRF: rely on `SameSite=Lax`, don't add a token header +### 6. CSRF: `SameSite=Lax` is the baseline; opt in for defence in depth -There is no CSRF token middleware. Starlette's default `SameSite=Lax` session cookie isn't attached to cross-site POST/PUT/DELETE, so forged form-submits are unauthenticated. Page-side `fetch()` calls don't need a token header. +Starlette's default `SameSite=Lax` session cookie isn't attached to cross-site POST/PUT/DELETE, so forged form-submits are unauthenticated — plain Inertia forms need nothing extra. A module that wants a real token check on its JSON mutations uses the framework primitive instead of rolling its own: + +```python +from simple_module_hosting.csrf import RequiresCsrf, get_csrf_token + +router = APIRouter(dependencies=[Depends(RequiresCsrf())]) +# expose get_csrf_token(request) as a view prop; fetch() callers echo it as X-CSRF-Token +``` + +Safe methods are never checked, and bare test apps without `SessionMiddleware` are exempt. ### 7. Don't call `session.commit()` in service code @@ -94,6 +103,10 @@ Locale-related diagnostics: SM013 (missing file), SM014 (missing keys vs default Projects using the `ty` type checker should globally suppress `unresolved-attribute`, `unsupported-operator`, `unknown-argument`, `no-matching-overload`, `invalid-argument-type`, and `invalid-assignment` in `pyproject.toml`. SQLModel runtime-instruments fields as SQLAlchemy attributes, so the type checker can't see what's there (and `model_config = ConfigDict(...)` clashes with ty's internal `SQLModelConfig` type, which `invalid-assignment` covers). Real bugs surface in tests. +### 10. Declare external asset origins with `register_csp_sources` + +The host CSP blocks any origin you don't declare — a font CDN or tile server your pages load from will silently fail in every host. Declare it in the hook: `registry.add("style-src", "https://rsms.me")`. Fetch directives only (never `default-src` / `base-uri` / `form-action` / `frame-ancestors`); single-token sources, validated at boot. + ## Related skills - **simple-module-database** — per-module Base, mixins, session lifecycle diff --git a/skills/simple-module-creating/SKILL.md b/skills/simple-module-creating/SKILL.md index 628c6a27..779bbccc 100644 --- a/skills/simple-module-creating/SKILL.md +++ b/skills/simple-module-creating/SKILL.md @@ -99,6 +99,7 @@ In execution order — all no-op by default: | `register_event_handlers(bus)` | `bus.subscribe(EventCls, handler)` | | `register_health_checks(registry)` | Module-owned health probes | | `register_public_routes(registry)` | Exempt routes from auth via `add_prefix` / `add_regex` (method-aware) | +| `register_csp_sources(registry)` | Whitelist external asset origins in the CSP (`registry.add("style-src", "https://rsms.me")`) — fetch directives only, validated at boot | | `register_exception_handlers(app)` | Module-specific error mapping | | `register_middleware(app)` | LIFO — module middleware sorted last wraps outermost | | `register_routes(api_router, view_router)` | `include_router(...)` your two routers | diff --git a/skills/simple-module-inertia-pages/SKILL.md b/skills/simple-module-inertia-pages/SKILL.md index bf8c5ac6..b34a694b 100644 --- a/skills/simple-module-inertia-pages/SKILL.md +++ b/skills/simple-module-inertia-pages/SKILL.md @@ -87,6 +87,8 @@ await fetch("/api/orders", { `SM018` warns when it spots Inertia `router.{post,patch,put,delete}()` targeting `/api/*` paths. +Two things to know about that `fetch()` path: errors from `/api/*` come back as JSON (`{"detail": ...}`) rather than the HTML error page, so read `resp.json().detail` for the real reason; and if the module's router opted into `RequiresCsrf`, include the token from the view's props as an `X-CSRF-Token` header or the mutation is rejected with 403. + ## Other gotchas - **Translated strings in props built at module scope.** `const labels = { title: t("orders.title") }` freezes against the first render's locale — build per-request translations inside the handler or via shared props. diff --git a/skills/simple-module-registries/SKILL.md b/skills/simple-module-registries/SKILL.md index 39a0868f..3cf6b719 100644 --- a/skills/simple-module-registries/SKILL.md +++ b/skills/simple-module-registries/SKILL.md @@ -7,7 +7,7 @@ description: Use when a module needs to contribute menu items, permissions, feat Four cross-cutting registries are populated during boot from each module's `register_*` hook. They turn the modular monolith into something more than a bag of routers: navigation aggregates, permission checks expand consistently, features can be toggled per tenant, and modules emit/consume events without importing each other. -All four are populated in **Phase 5** of `app_builder.build_app`, in this per-module order: `register_menu_items` → `register_permissions` → `register_feature_flags` → `register_event_handlers` → `register_health_checks` → `register_public_routes`. (The last two — health checks and anonymous-route exemptions — are also registries but out of scope here; see **simple-module-creating**.) Each is constructed once in Phase 3 and stashed on `app.state.sm` (as `menu_registry`, `permissions`, `feature_flags`, `event_bus`, …) only **after** all module hooks have run. +All four are populated in **Phase 5** of `app_builder.build_app`, in this per-module order: `register_menu_items` → `register_permissions` → `register_feature_flags` → `register_event_handlers` → `register_health_checks` → `register_public_routes` → `register_csp_sources`. (The last three — health checks, anonymous-route exemptions, and CSP sources — are also registries but out of scope here; see **simple-module-creating**.) Each is constructed once in Phase 3 and stashed on `app.state.sm` (as `menu_registry`, `permissions`, `feature_flags`, `event_bus`, …) only **after** all module hooks have run. ## Menu — `register_menu_items(registry: MenuRegistry)` From 3ae1b147673a7fbc045b554b166284a05102cd98 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 19 Aug 2026 21:15:01 +0200 Subject: [PATCH 09/14] fix: address code review findings (round 1, pass 1) Ten confirmed findings from /code-review high: CSP3 fallback chains when seeding new directives, bytes-based CSRF token compare (non-ASCII header 500), shared walk-up .env discovery in core dotenv resolved per instantiation, Accept: text/html wins over the /api prefix (and 422 aligned onto the same rule), scaffold Vite helper rebuilt on loadEnv with env-dir walk-up + origin normalization, register_csp_sources hook order matched to docs, overview.md hook list updated. Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc --- docs/framework-conventions.md | 15 ++-- docs/framework/overview.md | 2 +- .../templates/host/client_app/vite.config.ts | 4 +- .../templates/host/client_app/vite.dev-url.ts | 44 +++++++++--- framework/core/simple_module_core/csp.py | 58 +++++++++++----- framework/core/simple_module_core/dotenv.py | 47 +++++++++++-- framework/core/tests/test_csp_registry.py | 12 ++++ .../simple_module_hosting/_error_handlers.py | 32 +++++---- .../simple_module_hosting/_phase_helpers.py | 28 ++++---- .../simple_module_hosting/_registrations.py | 7 +- .../bootstrap_settings.py | 69 +++++-------------- .../hosting/simple_module_hosting/csrf.py | 13 +++- .../simple_module_hosting/middleware.py | 10 +++ .../tests/test_api_error_negotiation.py | 10 ++- framework/hosting/tests/test_hosting_csrf.py | 11 +++ .../hosting/tests/test_sqlite_path_anchor.py | 55 +++++++++++---- 16 files changed, 279 insertions(+), 138 deletions(-) diff --git a/docs/framework-conventions.md b/docs/framework-conventions.md index 4e901d26..6f132cb0 100644 --- a/docs/framework-conventions.md +++ b/docs/framework-conventions.md @@ -337,18 +337,21 @@ test apps) are exempt. ### Error responses (HTML vs JSON) -403/404/500 are content-negotiated. Requests under `/api/*` — the documented -prefix for every module's JSON surface — or with an explicit +403/404/422/500 are content-negotiated. Requests under `/api/*` — the +documented prefix for every module's JSON surface — or with an explicit `Accept: application/json` get a JSON body: ```json { "detail": "Permission required: pagebuilder.edit" } ``` -Browser-shaped requests (navigations, Inertia visits, `Accept: */*`) get the -rendered Inertia error page, which carries the layout, i18n copy, and the -request's correlation id. Module endpoint code doesn't opt in or out — raise -`HTTPException` as usual and the handler picks the right shape. +Browser-shaped requests get the rendered Inertia error page, which carries +the layout, i18n copy, and the request's correlation id. "Browser-shaped" +means the request accepts `text/html` — navigations and Inertia visits do, +and that wins even under `/api/*` (OAuth login links and file-download +`` hrefs are real navigations to API paths); a bare `fetch()` sends +`Accept: */*` and gets JSON there. Module endpoint code doesn't opt in or +out — raise `HTTPException` as usual and the handler picks the right shape. ### Design packs (site-wide look) diff --git a/docs/framework/overview.md b/docs/framework/overview.md index 62169f11..e2e487b9 100644 --- a/docs/framework/overview.md +++ b/docs/framework/overview.md @@ -20,7 +20,7 @@ What actually happens when you run `uvicorn main:app`: `Settings`, `DatabaseState` (engines per provider), `EventBus`, `MenuRegistry`, `PermissionRegistry`, `FeatureFlagRegistry`, `HealthRegistry`, `I18nRegistry`. They are bundled into a frozen `Services` dataclass and attached to `app.state.sm`. 3. **Discovery** — `discover_modules()` reads Python entry points under the `simple_module` group, imports each one, validates it's a `ModuleBase` subclass with a non-null `meta`, and topologically sorts by `ModuleMeta.depends_on`. 4. **Lifecycle hooks run in sorted order**. For each module, in this order: - `register_settings` → `register_menu_items` → `register_permissions` → `register_feature_flags` → `register_event_handlers` → `register_health_checks` → `register_public_routes` → `register_design_packs` → `register_exception_handlers` → `register_middleware` → `register_routes(api_router, view_router)`. + `register_settings` → `register_menu_items` → `register_permissions` → `register_feature_flags` → `register_event_handlers` → `register_health_checks` → `register_public_routes` → `register_csp_sources` → `register_design_packs` → `register_exception_handlers` → `register_middleware` → `register_routes(api_router, view_router)`. 5. **Middleware is installed** — framework middleware first, then whatever modules registered. See [Middleware pipeline](/framework/middleware). 6. **Routers mount** — `api_router` at `/api`, `view_router` at `/`. Each module's sub-routers were attached via `register_routes`. 7. **Lifespan `on_startup`** — each module's async `on_startup` runs in dependency order. This is where background workers, warm caches, or remote-service health probes start. diff --git a/framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts b/framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts index 747545c3..780a2270 100644 --- a/framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts +++ b/framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts @@ -30,7 +30,9 @@ function findNodeModulesRoot(start: string): string { } const fsRoot = findNodeModulesRoot(__dirname); -const devUrl = viteDevUrl(fsRoot); +// Walks up from client_app to the directory holding the project .env itself — +// fsRoot tracks node_modules, which (in flat mode) is NOT where .env lives. +const devUrl = viteDevUrl(__dirname); const devPort = viteDevPort(devUrl); // Load the module pages manifest written by the Python host at boot. diff --git a/framework/cli/simple_module_cli/templates/host/client_app/vite.dev-url.ts b/framework/cli/simple_module_cli/templates/host/client_app/vite.dev-url.ts index 0411f564..255ac3e8 100644 --- a/framework/cli/simple_module_cli/templates/host/client_app/vite.dev-url.ts +++ b/framework/cli/simple_module_cli/templates/host/client_app/vite.dev-url.ts @@ -1,19 +1,47 @@ import fs from 'node:fs'; import path from 'node:path'; +import { loadEnv } from 'vite'; // SM_VITE_DEV_URL (process env, then the project .env) drives the dev // server's port and origin, so Vite and the backend read the same value and // can never drift apart. The documented default stays http://localhost:5050. -export function viteDevUrl(envDir: string): string { - if (process.env.SM_VITE_DEV_URL) return process.env.SM_VITE_DEV_URL; - let env = ''; + +// The project .env sits at the project root (next to .env.example), which may +// be one or more levels above client_app — mirror the backend's walk-up so +// both sides read the same file, whatever directory node_modules landed in. +export function findEnvDir(start: string): string { + let dir = start; + for (let i = 0; i < 5; i++) { + if (fs.existsSync(path.join(dir, '.env')) || fs.existsSync(path.join(dir, '.env.example'))) { + return dir; + } + const parent = path.dirname(dir); + if (parent === dir) break; + dir = parent; + } + return start; +} + +export function viteDevUrl(startDir: string): string { + // loadEnv parses .env with the same dotenv semantics the backend uses + // (quotes, inline comments, `export` prefixes), and a real process env var + // wins over the file — the same precedence as pydantic-settings. + const env = loadEnv(process.env.NODE_ENV ?? 'development', findEnvDir(startDir), 'SM_'); + const raw = env.SM_VITE_DEV_URL ?? 'http://localhost:5050'; try { - env = fs.readFileSync(path.join(envDir, '.env'), 'utf8'); - } catch {} // no .env yet (fresh checkout) — use the default - const m = env.match(/^SM_VITE_DEV_URL\s*=\s*(\S+)\s*$/m); - return m ? m[1].replace(/^['"]|['"]$/g, '') : 'http://localhost:5050'; + // .origin normalizes away trailing slashes/paths that would otherwise + // produce double-slash asset URLs in server.origin. + return new URL(raw).origin; + } catch { + throw new Error(`SM_VITE_DEV_URL must be a full URL like http://localhost:5050, got: ${raw}`); + } } export function viteDevPort(devUrl: string): number { - return Number(new URL(devUrl).port || '5050'); + const url = new URL(devUrl); + if (url.port) return Number(url.port); + // No explicit port: bind the one the URL implies, so the backend (which + // hands this URL to the browser) and Vite agree instead of silently + // drifting to 5050. + return url.protocol === 'https:' ? 443 : 80; } diff --git a/framework/core/simple_module_core/csp.py b/framework/core/simple_module_core/csp.py index bb231fc4..33fd9aab 100644 --- a/framework/core/simple_module_core/csp.py +++ b/framework/core/simple_module_core/csp.py @@ -38,6 +38,20 @@ # token or terminate the clause (whitespace, ";", quotes) is rejected. _SOURCE_RE = re.compile(r"^(?:[A-Za-z][A-Za-z0-9+.-]*:(?://)?)?(?:\*\.)?[^\s;'\"*]*$") +# CSP3 fallback chains: when a directive is absent from a policy, the browser +# consults these directives in order (ending at ``default-src``). A clause we +# append for a previously-absent directive must be seeded from the nearest +# clause the policy already has along this chain — seeding with a bare +# ``'self'`` would *narrow* the policy (e.g. a fresh ``style-src-elem`` clause +# cuts off the fallback to ``style-src`` and silently drops its +# ``'unsafe-inline'`` and font origins). +_FALLBACK_CHAINS: dict[str, tuple[str, ...]] = { + "script-src-elem": ("script-src", "default-src"), + "style-src-elem": ("style-src", "default-src"), + "frame-src": ("child-src", "default-src"), + "worker-src": ("child-src", "script-src", "default-src"), +} + class CspSourceError(ValueError): """Invalid CSP directive or source declared by a module.""" @@ -78,26 +92,34 @@ def extend_policy(self, policy: str) -> str: Existing clauses keep their order and gain only sources they don't already list. A directive absent from the policy is appended as a new - clause seeded with ``'self'`` — without it the new clause would - *narrow* the policy, since the browser stops falling back to - ``default-src`` once the directive exists at all. + clause seeded from the clause the browser would otherwise have fallen + back to (per ``_FALLBACK_CHAINS``, ending at ``default-src``) — + without that seed the new clause would *narrow* the policy, since the + browser stops consulting the fallback once the directive exists. """ if not self._sources: return policy - clauses = [c.strip() for c in policy.split(";") if c.strip()] - seen_directives: set[str] = set() - out: list[str] = [] - for clause in clauses: + directives: dict[str, list[str]] = {} + order: list[str] = [] + for clause in policy.split(";"): + clause = clause.strip() + if not clause: + continue directive, _, rest = clause.partition(" ") - extras = self._sources.get(directive) - if extras: - seen_directives.add(directive) - existing = rest.split() - merged = existing + [e for e in extras if e not in existing] - out.append(f"{directive} {' '.join(merged)}") - else: - out.append(clause) + directives[directive] = rest.split() + order.append(directive) for directive, extras in self._sources.items(): - if directive not in seen_directives: - out.append(f"{directive} 'self' {' '.join(extras)}") - return "; ".join(out) + if directive not in directives: + directives[directive] = list(self._seed_sources(directive, directives)) + order.append(directive) + bucket = directives[directive] + bucket.extend(e for e in extras if e not in bucket) + return "; ".join(f"{d} {' '.join(directives[d])}".rstrip() for d in order) + + @staticmethod + def _seed_sources(directive: str, directives: dict[str, list[str]]) -> list[str]: + """Sources a brand-new clause inherits from its CSP fallback chain.""" + for fallback in (*_FALLBACK_CHAINS.get(directive, ()), "default-src"): + if fallback in directives: + return directives[fallback] + return ["'self'"] diff --git a/framework/core/simple_module_core/dotenv.py b/framework/core/simple_module_core/dotenv.py index 6842ca80..235115c5 100644 --- a/framework/core/simple_module_core/dotenv.py +++ b/framework/core/simple_module_core/dotenv.py @@ -14,6 +14,46 @@ BOOL_LITERALS_TRUE = frozenset({"1", "true", "t", "yes", "y", "on"}) BOOL_LITERALS_FALSE = frozenset({"0", "false", "f", "no", "n", "off"}) +# How many parent directories to probe for a `.env` above the cwd. One level +# covers the workspace layout (`host/` → root); a couple more cover running +# from `modules//`. Bounded so an unrelated `.env` far up the tree +# (e.g. in $HOME) is never picked up by accident. +_ENV_WALK_LIMIT = 4 + + +def find_env_file() -> Path: + """Locate the project ``.env`` regardless of which subdirectory runs us. + + ``$SM_PROJECT_ROOT/.env`` wins when set. Otherwise walk up from the cwd: + the web process chdirs to the workspace root so this finds ``./.env`` + immediately, while a CLI invoked from ``host/`` or ``modules//`` + finds the same file its app uses instead of silently loading nothing. + Falls back to a cwd-relative ``Path(".env")`` when nothing is found. + + This is the one .env-resolution convention for the whole ecosystem: the + settings layer (``BootstrapSettings``) and every out-of-process tool + (diagnostics CLI, worker entrypoints, users bootstrap) resolve through + here, so they can never disagree about which file is in effect. + """ + explicit = os.environ.get("SM_PROJECT_ROOT") + if explicit: + return Path(explicit) / ".env" + current = Path.cwd() + home = Path.home() + for candidate in (current, *current.parents[:_ENV_WALK_LIMIT]): + if candidate == home: + break + env = candidate / ".env" + if env.is_file(): + return env + # A `.git` or `.env.example` marks a project root: never ascend past + # one, or a nested checkout (a git worktree, a repo inside another + # repo, a fresh scaffold — which ships `.env.example` before any + # `.git` exists) would silently load the *outer* project's `.env`. + if (candidate / ".git").exists() or (candidate / ".env.example").is_file(): + break + return Path(".env") + def parse_dotenv(path: Path | None = None) -> dict[str, str]: """Parse a ``.env`` file into a dict. Empty dict if the file is missing. @@ -23,12 +63,11 @@ def parse_dotenv(path: Path | None = None) -> dict[str, str]: values — keep the file simple. Does *not* mutate ``os.environ``; the caller decides whether to merge. - Without ``path``, looks up ``$SM_PROJECT_ROOT/.env`` (falling back to - ``$CWD/.env``) — the convention used by every tool in this repo. + Without ``path``, resolves via :func:`find_env_file` — the convention + used by every tool in this repo. """ if path is None: - root = Path(os.environ.get("SM_PROJECT_ROOT") or Path.cwd()) - path = root / ".env" + path = find_env_file() if not path.is_file(): return {} parsed: dict[str, str] = {} diff --git a/framework/core/tests/test_csp_registry.py b/framework/core/tests/test_csp_registry.py index 3216bc06..bd2eee3c 100644 --- a/framework/core/tests/test_csp_registry.py +++ b/framework/core/tests/test_csp_registry.py @@ -46,6 +46,18 @@ def test_missing_directive_gets_self_plus_source(self) -> None: out = reg.extend_policy(_POLICY) assert "connect-src 'self' https://api.example.com" in out + def test_missing_elem_directive_inherits_its_fallback_clause(self) -> None: + """style-src-elem falls back to style-src (not default-src): a fresh + clause must inherit style-src's sources, or creating it would silently + drop 'unsafe-inline' and the font origins from element styles.""" + reg = CspSourceRegistry() + reg.add("style-src-elem", "https://rsms.me") + out = reg.extend_policy(_POLICY) + assert ( + "style-src-elem 'self' 'unsafe-inline' https://fonts.googleapis.com https://rsms.me" + in out + ) + def test_empty_registry_returns_policy_unchanged(self) -> None: reg = CspSourceRegistry() assert reg.extend_policy(_POLICY) == _POLICY diff --git a/framework/hosting/simple_module_hosting/_error_handlers.py b/framework/hosting/simple_module_hosting/_error_handlers.py index fe23d208..36f18474 100644 --- a/framework/hosting/simple_module_hosting/_error_handlers.py +++ b/framework/hosting/simple_module_hosting/_error_handlers.py @@ -26,17 +26,20 @@ def _wants_json(request: Request) -> bool: """API callers get JSON error bodies; browser-shaped requests get the page. - ``/api/*`` is the documented prefix for every module's JSON surface - (``ModuleMeta.route_prefix``), so path alone decides there. Elsewhere an - explicit ``Accept: application/json`` (without ``text/html`` — a browser - navigation sends both) opts a fetch caller into JSON. The default - ``*/*`` keeps the rendered Inertia page. + A request that explicitly accepts ``text/html`` is a browser navigation — + and those reach ``/api/*`` too (OAuth login links, file-download hrefs) — + so it always gets the rendered page. Otherwise ``/api/*``, the documented + prefix for every module's JSON surface (``ModuleMeta.route_prefix``), + gets JSON — a bare ``fetch()`` sends ``Accept: */*`` — as does an + explicit ``Accept: application/json`` anywhere else. """ + accept = request.headers.get("accept", "") + if "text/html" in accept: + return False path = request.url.path if path == "/api" or path.startswith("/api/"): return True - accept = request.headers.get("accept", "") - return "application/json" in accept and "text/html" not in accept + return "application/json" in accept async def render_error_page(request: Request, status_code: int, message: str) -> Response: @@ -90,13 +93,14 @@ async def not_found_error_handler(request: Request, exc: NotFoundError) -> Respo async def request_validation_error_handler( request: Request, exc: RequestValidationError ) -> Response: - """Return an Inertia error page for browser requests with invalid params.""" - accept = request.headers.get("accept", "") - if "text/html" in accept: - return await render_error_page( - request, 422, "The requested URL contains invalid parameters." - ) - return JSONResponse(status_code=422, content={"detail": jsonable_encoder(exc.errors())}) + """Return an Inertia error page for browser requests with invalid params. + + Same negotiation rule as 403/404/500 (``_wants_json``), so one request + never sees two different error shapes depending on the error class. + """ + if _wants_json(request): + return JSONResponse(status_code=422, content={"detail": jsonable_encoder(exc.errors())}) + return await render_error_page(request, 422, "The requested URL contains invalid parameters.") async def unhandled_exception_handler(request: Request, exc: Exception) -> Response: diff --git a/framework/hosting/simple_module_hosting/_phase_helpers.py b/framework/hosting/simple_module_hosting/_phase_helpers.py index e2e87839..31396206 100644 --- a/framework/hosting/simple_module_hosting/_phase_helpers.py +++ b/framework/hosting/simple_module_hosting/_phase_helpers.py @@ -46,6 +46,7 @@ from simple_module_hosting.static_files import PrecompressedStaticFiles if TYPE_CHECKING: + from simple_module_core.csp import CspSourceRegistry from simple_module_core.menu import MenuRegistry from simple_module_core.permissions import PermissionRegistry @@ -72,7 +73,7 @@ def register_exception_handlers(app: FastAPI, modules: list) -> None: mod.register_exception_handlers(app) -def build_csp(settings: Settings, csp_registry=None) -> str | None: +def build_csp(settings: Settings, csp_registry: CspSourceRegistry | None = None) -> str: """Choose the CSP for this boot and fold in module-declared sources. Development gets the Vite-widened policy; production the strict default. @@ -83,9 +84,9 @@ def build_csp(settings: Settings, csp_registry=None) -> str | None: base = ( SecurityHeadersMiddleware.dev_csp(settings.vite_dev_url) if settings.is_development - else SecurityHeadersMiddleware._DEFAULT_CSP + else SecurityHeadersMiddleware.default_csp() ) - if csp_registry: + if csp_registry is not None: return csp_registry.extend_policy(base) return base @@ -96,7 +97,7 @@ def install_middleware( modules: list, menu_registry: MenuRegistry, perm_registry: PermissionRegistry, - csp_registry=None, + csp_registry: CspSourceRegistry, ) -> None: """Install the full middleware pipeline. @@ -121,19 +122,14 @@ def install_middleware( mod.register_middleware(app) app.add_middleware(SessionMiddleware, secret_key=settings.secret_key) # In dev, relax CSP so the browser can fetch @vite/client, main.tsx, and - # the HMR WebSocket from the Vite origin. HSTS is also suppressed because - # dev runs over plain HTTP on loopback. + # the HMR WebSocket from the Vite origin (build_csp picks the variant). + # HSTS is suppressed in dev because it runs over plain HTTP on loopback. + security_kwargs: dict[str, str | None] = { + "content_security_policy": build_csp(settings, csp_registry) + } if settings.is_development: - app.add_middleware( - SecurityHeadersMiddleware, - content_security_policy=build_csp(settings, csp_registry), - strict_transport_security=None, - ) - else: - app.add_middleware( - SecurityHeadersMiddleware, - content_security_policy=build_csp(settings, csp_registry), - ) + security_kwargs["strict_transport_security"] = None + app.add_middleware(SecurityHeadersMiddleware, **security_kwargs) # Compress response bodies. Added here so it sits inside CorrelationId and # RequestLogging (which set headers and read request state) but outside # everything that produces a body — including the /static mount, where it diff --git a/framework/hosting/simple_module_hosting/_registrations.py b/framework/hosting/simple_module_hosting/_registrations.py index 75bc068b..b0de0e26 100644 --- a/framework/hosting/simple_module_hosting/_registrations.py +++ b/framework/hosting/simple_module_hosting/_registrations.py @@ -26,7 +26,7 @@ def run_module_registrations( public_route_registry, design_pack_registry, audit_link_registry, - csp_registry=None, + csp_registry, ) -> None: """Invoke each module's registration hooks, in dependency order. @@ -44,10 +44,11 @@ def run_module_registrations( health_registry.set_owner(mod.meta.name) mod.register_health_checks(health_registry) mod.register_public_routes(public_route_registry) + # csp before design packs — the documented lifecycle order + # (docs/framework/lifecycle.md). + mod.register_csp_sources(csp_registry) mod.register_design_packs(design_pack_registry) mod.register_audit_links(audit_link_registry) - if csp_registry is not None: - mod.register_csp_sources(csp_registry) health_registry.set_owner("") diff --git a/framework/hosting/simple_module_hosting/bootstrap_settings.py b/framework/hosting/simple_module_hosting/bootstrap_settings.py index 30083148..a65f15bd 100644 --- a/framework/hosting/simple_module_hosting/bootstrap_settings.py +++ b/framework/hosting/simple_module_hosting/bootstrap_settings.py @@ -7,62 +7,17 @@ from __future__ import annotations -import os from pathlib import Path -from typing import Literal +from typing import Any, Literal from pydantic import field_validator, model_validator from pydantic_settings import BaseSettings, SettingsConfigDict from simple_module_core.discovery import DEFAULT_AUTH_PROVIDER +from simple_module_core.dotenv import find_env_file from simple_module_core.environments import NON_PROD_ENVIRONMENTS _PLACEHOLDER_SECRET_KEY = "change-me-in-production" -# How many parent directories to probe for a `.env` above the cwd. One level -# covers the workspace layout (`host/` → root); a couple more cover running -# from `modules//`. Bounded so an unrelated `.env` far up the tree -# (e.g. in $HOME) is never picked up by accident. -_ENV_WALK_LIMIT = 4 - - -def _discover_env_file() -> Path | str: - """Locate the project ``.env`` regardless of which subdirectory runs us. - - ``SM_PROJECT_ROOT`` wins when set (the convention every out-of-process - tool in this repo already follows — see ``simple_module_core.dotenv``). - Otherwise walk up from the cwd: the web process chdirs to the workspace - root so this finds ``./.env`` immediately, while a CLI invoked from - ``host/`` or ``modules//`` finds the same file its app uses - instead of silently loading nothing. - """ - explicit = os.environ.get("SM_PROJECT_ROOT") - if explicit: - return Path(explicit) / ".env" - current = Path.cwd() - home = Path.home() - for candidate in (current, *current.parents[:_ENV_WALK_LIMIT]): - if candidate == home: - break - env = candidate / ".env" - if env.is_file(): - return env - # A `.git` marks the repository root: never ascend past it, or a - # nested checkout (a git worktree, a repo inside another repo) - # would silently load the *outer* project's `.env`. - if (candidate / ".git").exists(): - break - return ".env" - - -def _project_anchor(env_file: Path | str) -> Path: - """Directory that relative sqlite paths are written against.""" - explicit = os.environ.get("SM_PROJECT_ROOT") - if explicit: - return Path(explicit) - if isinstance(env_file, Path): - return env_file.parent - return Path.cwd() - def _absolutize_sqlite_url(url: str, *, anchor: Path) -> str: """Rewrite a relative sqlite path to an absolute one under ``anchor``. @@ -83,13 +38,19 @@ def _absolutize_sqlite_url(url: str, *, anchor: Path) -> str: return f"{scheme}:///{resolved}{query_sep}{query}" -_ENV_FILE = _discover_env_file() - - class BootstrapSettings(BaseSettings): """Pre-DB bootstrap environment knobs.""" - model_config = SettingsConfigDict(env_prefix="SM_", env_file=_ENV_FILE, extra="ignore") + model_config = SettingsConfigDict(env_prefix="SM_", env_file=".env", extra="ignore") + + def __init__(self, **values: Any) -> None: + # Discover the project `.env` per instantiation (a handful of stat + # calls), never at import time: a process may import this module and + # only later chdir or set SM_PROJECT_ROOT, and the env-file choice + # must always agree with the sqlite anchor in + # `_anchor_relative_sqlite_path`, which also discovers live. + values.setdefault("_env_file", find_env_file()) + super().__init__(**values) database_url: str = "sqlite+aiosqlite:///./app.db" db_pool_size: int = 10 @@ -136,7 +97,11 @@ class BootstrapSettings(BaseSettings): @field_validator("database_url", mode="after") @classmethod def _anchor_relative_sqlite_path(cls, value: str) -> str: - return _absolutize_sqlite_url(value, anchor=_project_anchor(_ENV_FILE)) + # `find_env_file().parent` is the project root: $SM_PROJECT_ROOT when + # set, else the directory of the discovered `.env`, else the cwd + # (Path(".env").parent resolves against it) — always the same file + # `__init__` just loaded, so values and anchor can't split-brain. + return _absolutize_sqlite_url(value, anchor=find_env_file().parent) @field_validator("auth_provider", mode="after") @classmethod diff --git a/framework/hosting/simple_module_hosting/csrf.py b/framework/hosting/simple_module_hosting/csrf.py index 3231668a..b550cf32 100644 --- a/framework/hosting/simple_module_hosting/csrf.py +++ b/framework/hosting/simple_module_hosting/csrf.py @@ -65,9 +65,18 @@ def __call__(self, request: Request) -> None: session = request.scope.get("session") if session is None: return # no session mounted — nothing to bind a token to - expected = get_csrf_token(request) + # Read (never mint) the token here: a rejected request must not mutate + # the session or re-issue the cookie. Compare as bytes — str + # compare_digest raises TypeError on non-ASCII input, and header + # values are attacker-controlled latin-1, which would turn a bad + # token into a 500 instead of a 403. + expected = session.get(_SESSION_KEY, "") provided = request.headers.get(CSRF_HEADER, "") - if not provided or not secrets.compare_digest(provided, expected): + if ( + not provided + or not expected + or not secrets.compare_digest(provided.encode("utf-8"), expected.encode("utf-8")) + ): raise HTTPException( status_code=403, detail=( diff --git a/framework/hosting/simple_module_hosting/middleware.py b/framework/hosting/simple_module_hosting/middleware.py index 97b49b1f..4bf95110 100644 --- a/framework/hosting/simple_module_hosting/middleware.py +++ b/framework/hosting/simple_module_hosting/middleware.py @@ -93,6 +93,16 @@ class SecurityHeadersMiddleware: ) _DEFAULT_HSTS = "max-age=31536000; includeSubDomains" + @classmethod + def default_csp(cls) -> str: + """The strict production Content-Security-Policy. + + Public accessor so boot code (``build_csp``) folds module-declared + sources into the same policy this middleware would send by default, + without reaching into the private constant. + """ + return cls._DEFAULT_CSP + @staticmethod def dev_csp(vite_dev_url: str) -> str: """Build a dev CSP that whitelists the Vite dev server. diff --git a/framework/hosting/tests/test_api_error_negotiation.py b/framework/hosting/tests/test_api_error_negotiation.py index 9aaf5938..20a86418 100644 --- a/framework/hosting/tests/test_api_error_negotiation.py +++ b/framework/hosting/tests/test_api_error_negotiation.py @@ -11,7 +11,7 @@ from __future__ import annotations import httpx -from simple_module_hosting._error_handlers import http_exception_handler +from simple_module_hosting._error_handlers import _wants_json, http_exception_handler from starlette.exceptions import HTTPException from starlette.requests import Request from starlette.responses import JSONResponse @@ -58,6 +58,14 @@ async def test_json_accept_on_view_path_returns_json(self) -> None: ) assert isinstance(resp, JSONResponse) + def test_browser_navigation_to_api_path_keeps_html_page(self) -> None: + """OAuth login links and file-download hrefs are real ```` + navigations under /api/*; a browser Accept (text/html) must keep the + rendered error page rather than dumping raw JSON in the tab.""" + browser_accept = "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8" + assert not _wants_json(_request("/api/users/auth/github/login", accept=browser_accept)) + assert _wants_json(_request("/api/users/auth/github/login")) # bare fetch: */* + class TestClientNegotiation: async def test_api_404_is_json(self, authenticated_client: httpx.AsyncClient) -> None: diff --git a/framework/hosting/tests/test_hosting_csrf.py b/framework/hosting/tests/test_hosting_csrf.py index 8b28fb0b..72c3afec 100644 --- a/framework/hosting/tests/test_hosting_csrf.py +++ b/framework/hosting/tests/test_hosting_csrf.py @@ -65,6 +65,17 @@ async def test_post_with_wrong_token_is_403(self, client: httpx.AsyncClient) -> resp = await client.post("/things", headers={CSRF_HEADER: "forged"}) assert resp.status_code == 403 + async def test_post_with_non_ascii_token_is_403_not_500( + self, client: httpx.AsyncClient + ) -> None: + """Header values are latin-1: a non-ASCII token must be rejected as + 403, not explode in str compare_digest (TypeError → 500).""" + await client.get("/token") + # raw latin-1 bytes: httpx's str path refuses non-ASCII, but a raw + # client on the wire can send it and starlette will decode it + resp = await client.post("/things", headers=[(CSRF_HEADER.encode(), b"caf\xe9-token")]) + assert resp.status_code == 403 + async def test_post_with_token_succeeds(self, client: httpx.AsyncClient) -> None: token = (await client.get("/token")).json()["token"] resp = await client.post("/things", headers={CSRF_HEADER: token}) diff --git a/framework/hosting/tests/test_sqlite_path_anchor.py b/framework/hosting/tests/test_sqlite_path_anchor.py index 9a38b9f8..b0a3bc70 100644 --- a/framework/hosting/tests/test_sqlite_path_anchor.py +++ b/framework/hosting/tests/test_sqlite_path_anchor.py @@ -8,8 +8,10 @@ The settings layer now anchors relative sqlite paths on the project root (``SM_PROJECT_ROOT``, else the directory of the discovered ``.env``), and -discovers the ``.env`` by walking up from the cwd, so subdirectory -invocations see the same database as the app. +discovers the ``.env`` by walking up from the cwd — via the shared +``simple_module_core.dotenv.find_env_file``, so ``parse_dotenv`` consumers +(diagnostics CLI, worker entrypoints, users bootstrap) resolve the same file +— so subdirectory invocations see the same database as the app. """ from __future__ import annotations @@ -17,10 +19,8 @@ from pathlib import Path import pytest -from simple_module_hosting.bootstrap_settings import ( - _absolutize_sqlite_url, - _discover_env_file, -) +from simple_module_core.dotenv import find_env_file +from simple_module_hosting.bootstrap_settings import _absolutize_sqlite_url class TestAbsolutize: @@ -53,7 +53,7 @@ def test_query_string_survives(self, tmp_path: Path) -> None: assert url == f"sqlite+aiosqlite:///{tmp_path / 'app.db'}?mode=ro" -class TestDiscoverEnvFile: +class TestFindEnvFile: def test_walks_up_from_subdirectory( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: @@ -62,7 +62,7 @@ def test_walks_up_from_subdirectory( host.mkdir() monkeypatch.delenv("SM_PROJECT_ROOT", raising=False) monkeypatch.chdir(host) - assert _discover_env_file() == tmp_path / ".env" + assert find_env_file() == tmp_path / ".env" def test_cwd_env_wins_over_parent( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch @@ -73,7 +73,7 @@ def test_cwd_env_wins_over_parent( (host / ".env").write_text("SM_X=child\n", encoding="utf-8") monkeypatch.delenv("SM_PROJECT_ROOT", raising=False) monkeypatch.chdir(host) - assert _discover_env_file() == host / ".env" + assert find_env_file() == host / ".env" def test_project_root_env_var_wins( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch @@ -82,7 +82,7 @@ def test_project_root_env_var_wins( root.mkdir() monkeypatch.setenv("SM_PROJECT_ROOT", str(root)) monkeypatch.chdir(tmp_path) - assert _discover_env_file() == root / ".env" + assert find_env_file() == root / ".env" def test_no_env_anywhere_falls_back_to_plain_name( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch @@ -91,7 +91,7 @@ def test_no_env_anywhere_falls_back_to_plain_name( deep.mkdir(parents=True) monkeypatch.delenv("SM_PROJECT_ROOT", raising=False) monkeypatch.chdir(deep) - assert _discover_env_file() == ".env" + assert find_env_file() == Path(".env") def test_walk_stops_at_repo_boundary( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch @@ -105,7 +105,24 @@ def test_walk_stops_at_repo_boundary( sub.mkdir() monkeypatch.delenv("SM_PROJECT_ROOT", raising=False) monkeypatch.chdir(sub) - assert _discover_env_file() == ".env" # inner repo has none; outer is off-limits + # inner repo has none; outer is off-limits + assert find_env_file() == Path(".env") + + def test_walk_stops_at_scaffold_boundary( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A fresh scaffold has `.env.example` before any `.git` exists — it + must bound the walk the same way, or a scaffold nested inside another + checkout silently boots with the outer project's `.env`.""" + (tmp_path / ".env").write_text("SM_X=outer\n", encoding="utf-8") + scaffold = tmp_path / "demo_app" + scaffold.mkdir() + (scaffold / ".env.example").write_text("SM_X=example\n", encoding="utf-8") + sub = scaffold / "host" + sub.mkdir() + monkeypatch.delenv("SM_PROJECT_ROOT", raising=False) + monkeypatch.chdir(sub) + assert find_env_file() == Path(".env") class TestSettingsIntegration: @@ -123,3 +140,17 @@ def test_settings_leave_memory_url_alone(self) -> None: s = Settings(database_url="sqlite+aiosqlite:///:memory:") assert s.database_url == "sqlite+aiosqlite:///:memory:" + + def test_env_file_discovered_per_instantiation( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """The .env is discovered when Settings is built, not when the module + was first imported — SM_PROJECT_ROOT set after import must load that + project's .env, so env values and the sqlite anchor can't come from + two different projects.""" + from simple_module_hosting.settings import Settings + + (tmp_path / ".env").write_text("SM_SECRET_KEY=from-late-project\n", encoding="utf-8") + monkeypatch.setenv("SM_PROJECT_ROOT", str(tmp_path)) + s = Settings(database_url="sqlite+aiosqlite:///:memory:") + assert s.secret_key == "from-late-project" From 21536d955a226f9b0954da136a7f52a37eca8d97 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 19 Aug 2026 21:32:52 +0200 Subject: [PATCH 10/14] fix: address code review findings (round 1, pass 2) Ten more confirmed findings: -elem shadow clauses now mirror base-directive extras, CSP source validation rejects commas and bare wildcards, Vite dev URL helper handles scheme-less/portless values sanely (single parse, .git boundary walk, SM_PROJECT_ROOT), sqlite file: URI-mode URLs pass through, JSON errors keep exc.headers and respect X-Inertia, doctor CLI anchors on the same walked-up .env, plus async CSRF dep and settings cleanup. Claude-Session: https://claude.ai/code/session_01HMniW4BEhpumUTFnWZVKVc --- .../templates/host/client_app/vite.config.ts | 5 +-- .../templates/host/client_app/vite.dev-url.ts | 41 ++++++++++++------- framework/cli/tests/test_cli_vite_port_env.py | 2 +- framework/core/simple_module_core/__main__.py | 11 +++-- framework/core/simple_module_core/csp.py | 19 ++++++--- framework/core/tests/test_csp_registry.py | 20 ++++++++- .../simple_module_hosting/_error_handlers.py | 21 +++++++--- .../simple_module_hosting/_phase_helpers.py | 10 ++--- .../bootstrap_settings.py | 24 ++++------- .../hosting/simple_module_hosting/csrf.py | 4 +- .../simple_module_hosting/middleware.py | 14 +------ .../tests/test_api_error_negotiation.py | 23 ++++++++++- .../hosting/tests/test_csp_module_sources.py | 4 +- framework/hosting/tests/test_hosting_csrf.py | 38 ++++++++--------- .../hosting/tests/test_sqlite_path_anchor.py | 5 +++ 15 files changed, 150 insertions(+), 91 deletions(-) diff --git a/framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts b/framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts index 780a2270..7a2fabb4 100644 --- a/framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts +++ b/framework/cli/simple_module_cli/templates/host/client_app/vite.config.ts @@ -3,7 +3,7 @@ import path from 'node:path'; import tailwindcss from '@tailwindcss/vite'; import react from '@vitejs/plugin-react'; import { type Plugin, defineConfig } from 'vite'; -import { viteDevPort, viteDevUrl } from './vite.dev-url'; +import { viteDevServer } from './vite.dev-url'; // Force every importer (host, workspace module, wheel-installed module) // to resolve to one React copy + a single Inertia hook context. Without @@ -32,8 +32,7 @@ const fsRoot = findNodeModulesRoot(__dirname); // Walks up from client_app to the directory holding the project .env itself — // fsRoot tracks node_modules, which (in flat mode) is NOT where .env lives. -const devUrl = viteDevUrl(__dirname); -const devPort = viteDevPort(devUrl); +const { origin: devUrl, port: devPort } = viteDevServer(__dirname); // Load the module pages manifest written by the Python host at boot. // Each entry points at an absolute pages/ directory — typically inside a diff --git a/framework/cli/simple_module_cli/templates/host/client_app/vite.dev-url.ts b/framework/cli/simple_module_cli/templates/host/client_app/vite.dev-url.ts index 255ac3e8..00bd0a22 100644 --- a/framework/cli/simple_module_cli/templates/host/client_app/vite.dev-url.ts +++ b/framework/cli/simple_module_cli/templates/host/client_app/vite.dev-url.ts @@ -7,14 +7,21 @@ import { loadEnv } from 'vite'; // can never drift apart. The documented default stays http://localhost:5050. // The project .env sits at the project root (next to .env.example), which may -// be one or more levels above client_app — mirror the backend's walk-up so -// both sides read the same file, whatever directory node_modules landed in. +// be one or more levels above client_app — mirror the backend's resolution +// (SM_PROJECT_ROOT override, then a bounded walk-up stopping at a repo +// boundary) so both sides read the same file, whatever directory +// node_modules landed in. export function findEnvDir(start: string): string { + const explicitRoot = process.env.SM_PROJECT_ROOT; + if (explicitRoot) return explicitRoot; let dir = start; for (let i = 0; i < 5; i++) { if (fs.existsSync(path.join(dir, '.env')) || fs.existsSync(path.join(dir, '.env.example'))) { return dir; } + // A `.git` marks a project root — never ascend past one, or a nested + // checkout would read the outer project's .env (same rule as the backend). + if (fs.existsSync(path.join(dir, '.git'))) break; const parent = path.dirname(dir); if (parent === dir) break; dir = parent; @@ -22,26 +29,30 @@ export function findEnvDir(start: string): string { return start; } -export function viteDevUrl(startDir: string): string { +export function viteDevServer(startDir: string): { origin: string; port: number } { // loadEnv parses .env with the same dotenv semantics the backend uses // (quotes, inline comments, `export` prefixes), and a real process env var // wins over the file — the same precedence as pydantic-settings. const env = loadEnv(process.env.NODE_ENV ?? 'development', findEnvDir(startDir), 'SM_'); const raw = env.SM_VITE_DEV_URL ?? 'http://localhost:5050'; + let url: URL; try { - // .origin normalizes away trailing slashes/paths that would otherwise - // produce double-slash asset URLs in server.origin. - return new URL(raw).origin; + url = new URL(raw); } catch { throw new Error(`SM_VITE_DEV_URL must be a full URL like http://localhost:5050, got: ${raw}`); } -} - -export function viteDevPort(devUrl: string): number { - const url = new URL(devUrl); - if (url.port) return Number(url.port); - // No explicit port: bind the one the URL implies, so the backend (which - // hands this URL to the browser) and Vite agree instead of silently - // drifting to 5050. - return url.protocol === 'https:' ? 443 : 80; + // A scheme-less value ("localhost:5310") parses as protocol "localhost:" + // with origin "null" — catch it here rather than shipping a broken origin. + if (url.protocol !== 'http:' && url.protocol !== 'https:') { + throw new Error(`SM_VITE_DEV_URL must be a full URL like http://localhost:5050, got: ${raw}`); + } + return { + // .origin normalizes away trailing slashes/paths that would otherwise + // produce double-slash asset URLs in server.origin. + origin: url.origin, + // No explicit port means the URL is fronted by a proxy (https://dev.example.com): + // keep binding the documented local default — binding 80/443 directly + // needs privileges and would crash under strictPort. + port: url.port ? Number(url.port) : 5050, + }; } diff --git a/framework/cli/tests/test_cli_vite_port_env.py b/framework/cli/tests/test_cli_vite_port_env.py index f24e597d..5592e035 100644 --- a/framework/cli/tests/test_cli_vite_port_env.py +++ b/framework/cli/tests/test_cli_vite_port_env.py @@ -40,7 +40,7 @@ def test_vite_config_derives_port_from_env_url(tmp_path: Path) -> None: # port and origin both come from the derived URL — no literal pin left assert re.search(r"port:\s*5050\b", text) is None assert re.search(r"origin:\s*'http://localhost:5050'", text) is None - assert "viteDevUrl" in text + assert "viteDevServer" in text assert "strictPort: true" in text # still fail fast on a taken port helper = (config.parent / "vite.dev-url.ts").read_text(encoding="utf-8") diff --git a/framework/core/simple_module_core/__main__.py b/framework/core/simple_module_core/__main__.py index 0f122ab2..020d178e 100644 --- a/framework/core/simple_module_core/__main__.py +++ b/framework/core/simple_module_core/__main__.py @@ -9,7 +9,8 @@ i18n checks are included when ``SM_I18N_SUPPORTED_LOCALES`` is set in env (or ``.env``). Host-level ``host/locales/`` and shared ``packages/ui/locales/`` -are picked up relative to ``SM_PROJECT_ROOT`` (or the current working dir). +are picked up relative to the project root (``SM_PROJECT_ROOT``, else the +directory of the discovered ``.env``, else the current working dir). """ from __future__ import annotations @@ -31,7 +32,7 @@ select_auth_provider, topological_sort, ) -from simple_module_core.dotenv import parse_dotenv +from simple_module_core.dotenv import find_env_file, parse_dotenv from simple_module_core.exceptions import InvalidModuleError @@ -64,7 +65,11 @@ def _load_i18n_settings_from_env() -> tuple[list[str], str] | tuple[None, None]: def _discover_extra_locale_sources() -> list[tuple[str, str, Path]]: """Return ``[(reporter, namespace, path), ...]`` for host + ui locale dirs.""" - root = Path(os.environ.get("SM_PROJECT_ROOT") or Path.cwd()) + # Anchor on the same project root the `.env` was loaded from + # (`parse_dotenv` walks up from the cwd) — resolving against the bare cwd + # here would look for `host/locales` in the wrong directory whenever + # doctor runs from a subdirectory. + root = find_env_file().parent out: list[tuple[str, str, Path]] = [] host_locales = root / "host" / "locales" if host_locales.is_dir(): diff --git a/framework/core/simple_module_core/csp.py b/framework/core/simple_module_core/csp.py index 33fd9aab..b650edf4 100644 --- a/framework/core/simple_module_core/csp.py +++ b/framework/core/simple_module_core/csp.py @@ -35,8 +35,10 @@ # A source is a scheme, an origin (optionally with scheme/wildcard/port), or # a data-ish scheme keyword. One token — anything that could smuggle a second -# token or terminate the clause (whitespace, ";", quotes) is rejected. -_SOURCE_RE = re.compile(r"^(?:[A-Za-z][A-Za-z0-9+.-]*:(?://)?)?(?:\*\.)?[^\s;'\"*]*$") +# token or terminate/extend the clause (whitespace, ";", ",", quotes) is +# rejected, and a wildcard prefix must be followed by a real host (`*.` alone +# is not a source). +_SOURCE_RE = re.compile(r"^(?:[A-Za-z][A-Za-z0-9+.-]*:(?://)?)?(?:(?:\*\.)?[^\s;,'\"*]+)?$") # CSP3 fallback chains: when a directive is absent from a policy, the browser # consults these directives in order (ending at ``default-src``). A clause we @@ -83,10 +85,6 @@ def add(self, directive: str, source: str) -> None: def __bool__(self) -> bool: return bool(self._sources) - @property - def sources(self) -> dict[str, tuple[str, ...]]: - return {directive: tuple(items) for directive, items in self._sources.items()} - def extend_policy(self, policy: str) -> str: """Fold the registered sources into an existing policy string. @@ -114,6 +112,15 @@ def extend_policy(self, policy: str) -> str: order.append(directive) bucket = directives[directive] bucket.extend(e for e in extras if e not in bucket) + # The -elem variants *shadow* their base directive once present: with + # `script-src-elem` already in the policy, a module's `script-src` + # addition would never reach