fix(devin-review): dashboard CTA respects override; PUT validates anon path
Finding #1: _build_context now routes through render_agent_prompt_banner when a DB connection is available, so both /setup and the /dashboard clipboard CTA always reflect the admin override (or the live default when no override is set). Previously _build_context unconditionally used resolve_lines(), ignoring the welcome_template override for the dashboard JS array. Finding #2: PUT /api/admin/welcome-template now performs a second render pass with user=None (anonymous stub) after the authenticated-user pass. Templates that reference user.* fields without an {% if user %} guard are rejected with a clear 400 error explaining the anon-visitor breakage.
This commit is contained in:
ZdenekSrotyr
parent
d18bc4c8f7
commit
9ad7856f72
3 changed files with 138 additions and 49 deletions
|
|
@ -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"}
|
||||
|
||||
|
|
|
|||
|
|
@ -247,51 +247,45 @@ 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`.
|
||||
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"
|
||||
|
||||
# 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,
|
||||
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("/")
|
||||
|
||||
ctx = {
|
||||
"request": request,
|
||||
|
|
|
|||
|
|
@ -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": "<p>Hello {{ user.email }}</p>"},
|
||||
json={"content": "<p>{% if user %}Hello {{ user.email }}{% endif %}</p>"},
|
||||
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"] == "<p>Hello {{ user.email }}</p>"
|
||||
assert r.json()["content"] == "<p>{% if user %}Hello {{ user.email }}{% endif %}</p>"
|
||||
|
||||
# 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
|
||||
|
|
|
|||
Loading…
Reference in a new issue