diff --git a/app/api/welcome.py b/app/api/welcome.py index b09c1c0..7f06c7c 100644 --- a/app/api/welcome.py +++ b/app/api/welcome.py @@ -43,6 +43,14 @@ _VALIDATION_STUB_CONTEXT = { "today": "2026-01-01", } +# Same stub with user=None to validate templates against anonymous /setup visitors. +# /setup is publicly accessible — templates that reference user.* without an +# {% if user %} guard will crash with StrictUndefined for anon visitors. +_VALIDATION_STUB_CONTEXT_ANON = { + **{k: v for k, v in _VALIDATION_STUB_CONTEXT.items() if k != "user"}, + "user": None, +} + class BannerResponse(BaseModel): content: str @@ -93,11 +101,29 @@ async def admin_put_template( env = Environment(undefined=StrictUndefined, autoescape=False) try: template = env.from_string(payload.content) - # Render against a stub context so undefined placeholders or runtime - # errors are caught here, not when /setup renders for a real user. + # Pass 1 — render with an authenticated user stub so undefined + # placeholders or runtime errors are caught at save time. template.render(**_VALIDATION_STUB_CONTEXT) except TemplateError as e: raise HTTPException(status_code=400, detail=f"Template invalid: {e}") + + # Pass 2 — render with user=None to catch templates that reference user.* + # fields without an {% if user %} guard. /setup is publicly accessible to + # anonymous visitors, so a guard-less template would crash with + # StrictUndefined at runtime and silently fall back to the default — the + # admin would have no idea their override is broken for anon visitors. + try: + template.render(**_VALIDATION_STUB_CONTEXT_ANON) + except TemplateError as e: + raise HTTPException( + status_code=400, + detail=( + f"Template fails for anonymous /setup visitors: {e}. " + "Wrap user-dependent expressions in {{% if user %}}...{{% endif %}} — " + "/setup is publicly accessible to non-logged-in users." + ), + ) + WelcomeTemplateRepository(conn).set(payload.content, updated_by=user["email"]) return {"status": "ok"} diff --git a/app/web/router.py b/app/web/router.py index 2d0d56b..603bebb 100644 --- a/app/web/router.py +++ b/app/web/router.py @@ -247,52 +247,46 @@ def _build_context( return {k: v for k, v in theme.items() if v} return {} - # Lines + server_url for the "Setup a new Claude Code" preview/clipboard - # partial; single source of truth lives in app/web/setup_instructions.py. - # Resolve the wheel filename server-side so the URL in the setup snippet - # is a PEP 427-compliant path — `uv tool install` rejects bare `agnes.whl`. - from app.web.setup_instructions import resolve_lines - from app.api.cli_artifacts import _find_wheel - _wheel = _find_wheel() - _wheel_filename = _wheel.name if _wheel else "agnes.whl" - - # Inline the user's RBAC-allowed marketplace plugins as `claude plugin - # install` commands so a single paste also bootstraps the marketplace - # and plugin set. Anonymous viewers (no user, or no DB conn) get the - # original 6-step layout. - plugin_install_names: list[str] = [] - if user and conn is not None: - try: - from src import marketplace_filter - plugin_install_names = [ - p["manifest_name"] - for p in marketplace_filter.resolve_allowed_plugins(conn, user) - ] - except Exception: # pragma: no cover — defensive: never block dashboard render - logger.exception("Failed to resolve marketplace plugins for setup prompt") - plugin_install_names = [] - - # `AGNES_DEBUG_AUTH` is the existing dev/staging gate (see - # `app/api/me_debug.py`, `app/web/router.py` template ConfigProxy). - # When on, the setup prompt also disables host-scoped git TLS verify - # so `claude plugin marketplace add` works against self-signed instances. - # Subsumed by the cert trust block when `ca_pem` is loaded below. - self_signed_tls = os.environ.get("AGNES_DEBUG_AUTH", "").strip().lower() in ( - "1", "true", "yes", - ) - server_host = request.url.netloc - - ca_pem = _read_agnes_ca_pem() - - setup_instructions_lines = resolve_lines( - _wheel_filename, - plugin_install_names=plugin_install_names, - self_signed_tls=self_signed_tls, - server_host=server_host, - ca_pem=ca_pem, - ) ctx_server_url = str(request.base_url).rstrip("/") + # Lines for the "Setup a new Claude Code" preview/clipboard partial. + # + # When a DB connection is available, we go through render_agent_prompt_banner + # which checks for an admin override first (stored in welcome_template) and + # falls back to the live default from setup_instructions.resolve_lines(). + # This guarantees that both /setup and /dashboard clipboard CTA always reflect + # the same content — the override is honoured everywhere. + # + # When no conn is supplied (e.g. public pages that don't need a DB round-trip) + # we fall back to resolve_lines() directly with anonymous/no-plugin context. + if conn is not None: + from src.welcome_template import render_agent_prompt_banner + _script_text = render_agent_prompt_banner( + conn, user=user, server_url=ctx_server_url + ) + setup_instructions_lines = _script_text.split("\n") + else: + # No DB connection — use the unauthenticated default (no override possible, + # no marketplace plugins). + from app.web.setup_instructions import resolve_lines + from app.api.cli_artifacts import _find_wheel + _wheel = _find_wheel() + _wheel_filename = _wheel.name if _wheel else "agnes.whl" + + self_signed_tls = os.environ.get("AGNES_DEBUG_AUTH", "").strip().lower() in ( + "1", "true", "yes", + ) + server_host = request.url.netloc + ca_pem = _read_agnes_ca_pem() + + setup_instructions_lines = resolve_lines( + _wheel_filename, + plugin_install_names=[], + self_signed_tls=self_signed_tls, + server_host=server_host, + ca_pem=ca_pem, + ) + ctx = { "request": request, "config": ConfigProxy, diff --git a/tests/test_welcome_template_api.py b/tests/test_welcome_template_api.py index f1ea249..c3f9fda 100644 --- a/tests/test_welcome_template_api.py +++ b/tests/test_welcome_template_api.py @@ -45,10 +45,10 @@ def test_admin_can_set_and_reset_template(seeded_app): c = seeded_app["client"] admin = _auth(seeded_app["admin_token"]) - # PUT override + # PUT override — use an {% if user %} guard so it passes anon validation too r = c.put( "/api/admin/welcome-template", - json={"content": "

Hello {{ user.email }}

"}, + json={"content": "

{% if user %}Hello {{ user.email }}{% endif %}

"}, headers=admin, ) assert r.status_code == 200 @@ -56,7 +56,7 @@ def test_admin_can_set_and_reset_template(seeded_app): # GET reflects override r = c.get("/api/admin/welcome-template", headers=admin) assert r.status_code == 200 - assert r.json()["content"] == "

Hello {{ user.email }}

" + assert r.json()["content"] == "

{% if user %}Hello {{ user.email }}{% endif %}

" # DELETE = reset (no banner) r = c.delete("/api/admin/welcome-template", headers=admin) @@ -212,3 +212,72 @@ def test_get_template_default_field_has_server_url_placeholder(seeded_app): body = r.json() assert "{server_url}" in body["default"] assert "{token}" in body["default"] + + +# --------------------------------------------------------------------------- +# Finding #2: PUT validation rejects templates that fail for anonymous visitors +# --------------------------------------------------------------------------- + +def test_put_rejects_template_that_breaks_for_anonymous(seeded_app): + """A template referencing user.email without an {% if user %} guard fails + for anonymous visitors on /setup (user is None → StrictUndefined raises). + PUT validation must catch this and return 400 with a clear message.""" + c = seeded_app["client"] + admin = _auth(seeded_app["admin_token"]) + r = c.put( + "/api/admin/welcome-template", + json={"content": "Hello {{ user.email }}"}, # no {% if user %} guard + headers=admin, + ) + assert r.status_code == 400 + detail = r.json()["detail"].lower() + assert "anonymous" in detail + + +def test_put_accepts_template_with_user_guard(seeded_app): + """The same template wrapped in {% if user %} passes both validation passes.""" + c = seeded_app["client"] + admin = _auth(seeded_app["admin_token"]) + r = c.put( + "/api/admin/welcome-template", + json={"content": "{% if user %}Hello {{ user.email }}{% endif %}"}, + headers=admin, + ) + assert r.status_code == 200 + + +# --------------------------------------------------------------------------- +# Finding #1: /dashboard clipboard CTA honours admin override +# --------------------------------------------------------------------------- + +def test_dashboard_clipboard_uses_override_when_set(seeded_app): + """When an admin override is saved, the dashboard's SETUP_INSTRUCTIONS_TEMPLATE + JS array must contain the override content instead of the default script. + The dashboard button copies that array — so the override must appear there.""" + c = seeded_app["client"] + admin = _auth(seeded_app["admin_token"]) + + # Save a distinctive override (plain text — no Jinja2 needed). + r = c.put( + "/api/admin/welcome-template", + json={"content": "# CUSTOM_OVERRIDE_MARKER\necho custom-setup"}, + headers=admin, + ) + assert r.status_code == 200 + + # Fetch the dashboard as the admin user (cookie-style auth via access_token). + r = c.get("/dashboard", cookies={"access_token": seeded_app["admin_token"]}) + assert r.status_code == 200 + body = r.text + + # The override lines must appear in the embedded JS template array. + assert "CUSTOM_OVERRIDE_MARKER" in body + + # Reset + r = c.delete("/api/admin/welcome-template", headers=admin) + assert r.status_code == 204 + + # After reset, the override must no longer appear in the dashboard. + r = c.get("/dashboard", cookies={"access_token": seeded_app["admin_token"]}) + assert r.status_code == 200 + assert "CUSTOM_OVERRIDE_MARKER" not in r.text