fix(api): validate template render on PUT; broaden render-time catch
This commit is contained in:
parent
0d1ecd235d
commit
93b713900b
2 changed files with 76 additions and 8 deletions
|
|
@ -6,11 +6,12 @@
|
|||
- DELETE /api/admin/welcome-template : reset to default (admin)
|
||||
"""
|
||||
|
||||
import datetime
|
||||
from typing import Optional
|
||||
|
||||
import duckdb
|
||||
from fastapi import APIRouter, Depends, HTTPException, Query, Response
|
||||
from jinja2 import TemplateSyntaxError
|
||||
from jinja2 import Environment, StrictUndefined, TemplateError, TemplateSyntaxError
|
||||
from pydantic import BaseModel, Field
|
||||
|
||||
from app.auth.access import require_admin
|
||||
|
|
@ -21,6 +22,29 @@ from src.welcome_template import _load_default_template, render_welcome
|
|||
|
||||
router = APIRouter(tags=["welcome"])
|
||||
|
||||
# Stub context used to validate that a saved template renders end-to-end,
|
||||
# not just that it parses. Mirrors the shape of build_context() output.
|
||||
_VALIDATION_STUB_CONTEXT = {
|
||||
"instance": {"name": "Example", "subtitle": "Example Org"},
|
||||
"server": {"url": "https://example.com", "hostname": "example.com"},
|
||||
"sync_interval": "1 hour",
|
||||
"data_source": {"type": "local"},
|
||||
"tables": [{"name": "example", "description": "", "query_mode": "local"}],
|
||||
"metrics": {"count": 0, "categories": []},
|
||||
"marketplaces": [
|
||||
{"slug": "example", "name": "Example Marketplace", "plugins": [{"name": "x"}]}
|
||||
],
|
||||
"user": {
|
||||
"id": "u",
|
||||
"email": "user@example.com",
|
||||
"name": "User",
|
||||
"is_admin": False,
|
||||
"groups": ["Everyone"],
|
||||
},
|
||||
"now": datetime.datetime(2026, 1, 1, tzinfo=datetime.timezone.utc),
|
||||
"today": "2026-01-01",
|
||||
}
|
||||
|
||||
|
||||
class WelcomeResponse(BaseModel):
|
||||
content: str
|
||||
|
|
@ -46,10 +70,10 @@ async def get_welcome(
|
|||
"""Render the welcome prompt for the calling user. Returns rendered markdown."""
|
||||
try:
|
||||
rendered = render_welcome(conn, user=user, server_url=server_url)
|
||||
except TemplateSyntaxError as e:
|
||||
except TemplateError as e:
|
||||
raise HTTPException(
|
||||
status_code=500,
|
||||
detail=f"Welcome template has a syntax error: {e.message}. Reset via /admin/welcome.",
|
||||
detail=f"Welcome template render failed: {e}. An admin can reset it via /admin/welcome.",
|
||||
)
|
||||
return WelcomeResponse(content=rendered)
|
||||
|
||||
|
|
@ -74,11 +98,14 @@ async def admin_put_template(
|
|||
user: dict = Depends(require_admin),
|
||||
conn: duckdb.DuckDBPyConnection = Depends(_get_db),
|
||||
):
|
||||
from jinja2 import Environment, StrictUndefined
|
||||
env = Environment(undefined=StrictUndefined)
|
||||
try:
|
||||
Environment(undefined=StrictUndefined).parse(payload.content)
|
||||
except TemplateSyntaxError as e:
|
||||
raise HTTPException(status_code=400, detail=f"Jinja2 syntax error: {e.message}")
|
||||
template = env.from_string(payload.content)
|
||||
# Render against a stub context so undefined placeholders or runtime
|
||||
# errors are caught here, not when an analyst calls /api/welcome.
|
||||
template.render(**_VALIDATION_STUB_CONTEXT)
|
||||
except TemplateError as e:
|
||||
raise HTTPException(status_code=400, detail=f"Template invalid: {e}")
|
||||
WelcomeTemplateRepository(conn).set(payload.content, updated_by=user["email"])
|
||||
return {"status": "ok"}
|
||||
|
||||
|
|
|
|||
|
|
@ -77,4 +77,45 @@ def test_invalid_jinja2_returns_400(seeded_app):
|
|||
headers=admin,
|
||||
)
|
||||
assert r.status_code == 400
|
||||
assert "syntax" in r.json()["detail"].lower()
|
||||
assert "invalid" in r.json()["detail"].lower()
|
||||
|
||||
|
||||
def test_put_rejects_undefined_placeholder(seeded_app):
|
||||
"""Templates that parse but reference unknown placeholders must be rejected
|
||||
at PUT time so an admin can fix the typo immediately rather than after an
|
||||
analyst's bootstrap blows up."""
|
||||
c = seeded_app["client"]
|
||||
admin = _auth(seeded_app["admin_token"])
|
||||
r = c.put(
|
||||
"/api/admin/welcome-template",
|
||||
json={"content": "Hello {{ user.emial }}"}, # typo, would fail StrictUndefined at render
|
||||
headers=admin,
|
||||
)
|
||||
assert r.status_code == 400
|
||||
assert "emial" in r.json()["detail"] or "undefined" in r.json()["detail"].lower()
|
||||
|
||||
|
||||
def test_get_welcome_500_includes_reset_hint_on_render_failure(seeded_app, monkeypatch):
|
||||
"""If an override slips through validation and fails at render time, the
|
||||
user-visible 500 must point at /admin/welcome rather than leaking a
|
||||
Jinja stack trace."""
|
||||
# Stub render_welcome to raise a TemplateError so we exercise the
|
||||
# exception path without needing a malformed override (PUT validation
|
||||
# blocks those now).
|
||||
from jinja2 import UndefinedError
|
||||
import app.api.welcome as welcome_module
|
||||
|
||||
def fake_render(*args, **kwargs):
|
||||
raise UndefinedError("'foo' is undefined")
|
||||
|
||||
monkeypatch.setattr(welcome_module, "render_welcome", fake_render)
|
||||
|
||||
c = seeded_app["client"]
|
||||
admin = _auth(seeded_app["admin_token"])
|
||||
r = c.get(
|
||||
"/api/welcome",
|
||||
params={"server_url": "https://example.com"},
|
||||
headers=admin,
|
||||
)
|
||||
assert r.status_code == 500
|
||||
assert "/admin/welcome" in r.json()["detail"]
|
||||
|
|
|
|||
Loading…
Reference in a new issue