fix(security): enforce registered endpoint authority

Require caller-supplied model endpoints to resolve through enabled owner-visible registrations, harden session path encoding, and remove the SVG title HTML parsing sink.
This commit is contained in:
Alexandre Teixeira 2026-10-05 22:24:15 +01:00
parent 481726acf2
commit 9ee614e2ac
9 changed files with 349 additions and 14 deletions

View file

@ -16,6 +16,7 @@ from pydantic import BaseModel
from core.database import SessionLocal, CrewMember, ScheduledTask
from src.auth_helpers import get_current_user
from src.endpoint_resolver import resolve_owner_registered_endpoint_url
from src.owner_identity import REQUEST_SENTINEL_OWNERS
from src.task_scheduler import compute_next_run
@ -178,7 +179,13 @@ def setup_assistant_routes(task_scheduler) -> APIRouter:
if payload.model is not None:
crew_db.model = payload.model or None
if payload.endpoint_url is not None:
crew_db.endpoint_url = payload.endpoint_url or None
try:
crew_db.endpoint_url = (
resolve_owner_registered_endpoint_url(db, payload.endpoint_url, owner)
if payload.endpoint_url else None
)
except ValueError as exc:
raise HTTPException(400, str(exc)) from exc
if payload.timezone is not None:
crew_db.timezone = payload.timezone or None

View file

@ -2003,7 +2003,11 @@ def setup_skills_routes(skills_manager: SkillsManager) -> APIRouter:
it untouched). It never changes the skill's published/draft STATUS."""
import time as _time
import asyncio as _asyncio
from src.endpoint_resolver import resolve_endpoint
from core.database import SessionLocal
from src.endpoint_resolver import (
build_chat_url, build_headers, resolve_endpoint,
resolve_endpoint_runtime, resolve_owner_registered_endpoint,
)
user = _owner(request)
body = await request.json()
@ -2027,10 +2031,18 @@ def setup_skills_routes(skills_manager: SkillsManager) -> APIRouter:
# session's model. Fall back to the caller's session model only if unset.
url, model, headers = resolve_endpoint("utility", owner=user)
if not url or not model:
url = url or ((body.get("endpoint_url") or "").strip() or None)
if not url and body.get("endpoint_url") is not None:
db = SessionLocal()
try:
endpoint = resolve_owner_registered_endpoint(db, body["endpoint_url"], user)
base, api_key = resolve_endpoint_runtime(endpoint, owner=user)
url = build_chat_url(base)
headers = build_headers(api_key, base)
except ValueError as exc:
raise HTTPException(400, str(exc)) from exc
finally:
db.close()
model = model or ((body.get("model") or "").strip() or None)
if headers is None and isinstance(body.get("headers"), dict):
headers = body.get("headers")
if not url or not model:
raise HTTPException(400, "No model configured — set a Default or Utility model in Settings.")

View file

@ -14,6 +14,7 @@ from core.database import SessionLocal, ScheduledTask, TaskRun, NotificationLog
from core.constants import internal_api_base
from src.auth_helpers import get_current_user
from src.constants import DATA_DIR, EMAIL_URGENCY_CACHE_DIR
from src.endpoint_resolver import resolve_owner_registered_endpoint_url
from src.task_action_policy import (
ADMIN_ONLY_TASK_ACTIONS,
is_admin_only_task_action,
@ -519,6 +520,12 @@ def setup_task_routes(task_scheduler) -> APIRouter:
db = SessionLocal()
try:
then_task_id = _validate_then_task_id(db, req.then_task_id, user)
endpoint_url = None
if req.endpoint_url:
try:
endpoint_url = resolve_owner_registered_endpoint_url(db, req.endpoint_url, user)
except ValueError as exc:
raise HTTPException(400, str(exc)) from exc
notifications_enabled = (
False if req.task_type == "action" and req.notifications_enabled is None
else bool(req.notifications_enabled) if req.notifications_enabled is not None
@ -555,7 +562,7 @@ def setup_task_routes(task_scheduler) -> APIRouter:
status="active" if (req.trigger_type in ("event", "webhook") or next_run) else "completed",
output_target=req.output_target,
model=req.model or None,
endpoint_url=req.endpoint_url or None,
endpoint_url=endpoint_url,
then_task_id=then_task_id,
webhook_token=webhook_token,
notifications_enabled=notifications_enabled,
@ -755,7 +762,14 @@ def setup_task_routes(task_scheduler) -> APIRouter:
if req.model is not None:
task.model = req.model or None
if req.endpoint_url is not None:
task.endpoint_url = req.endpoint_url or None
try:
# An empty override restores the existing default-model workflow.
task.endpoint_url = (
resolve_owner_registered_endpoint_url(db, req.endpoint_url, user)
if req.endpoint_url else None
)
except ValueError as exc:
raise HTTPException(400, str(exc)) from exc
if req.trigger_type is not None:
# Generate webhook token when switching to webhook trigger
if req.trigger_type == "webhook" and not task.webhook_token:

View file

@ -281,6 +281,29 @@ def same_endpoint_base(left, right) -> bool:
return False
def resolve_owner_registered_endpoint(db, endpoint_url: str, owner: Optional[str] = None):
"""Authorize a caller URL against enabled, owner-visible endpoint rows.
Request credentials, query strings and fragments are never endpoint identity.
Return the server-owned row so runtime credentials come from registration.
"""
from src.auth_helpers import owner_filter
if not isinstance(endpoint_url, str) or not same_endpoint_base(endpoint_url, endpoint_url):
raise ValueError("Invalid model endpoint URL")
query = db.query(ModelEndpoint).filter(ModelEndpoint.is_enabled.is_(True))
for endpoint in owner_filter(query, ModelEndpoint, owner).all():
if same_endpoint_base(endpoint_url, endpoint.base_url):
return endpoint
raise ValueError("Model endpoint must be enabled and registered for the current owner")
def resolve_owner_registered_endpoint_url(db, endpoint_url: str, owner: Optional[str] = None) -> str:
"""Return only the registered canonical base, never the caller's URL."""
endpoint = resolve_owner_registered_endpoint(db, endpoint_url, owner)
return normalize_base(endpoint.base_url)
def _validated_endpoint_base(url: str) -> str:
"""Return a base URL that is safe for endpoint path appends."""
base = (url or "").strip().rstrip("/")

View file

@ -791,12 +791,13 @@ function renderSvgSandbox(source) {
const height = viewBox ? Number(viewBox[2]) : 9;
const ratio = Number.isFinite(width / height) && width > 0 && height > 0
? Math.max(0.5, Math.min(3, width / height)) : (16 / 9);
// Parse in an inert template: nested/malformed SVG title markup stays text.
// XML parsing extracts text without inserting title markup into an HTML DOM.
let title = 'Visual explanation';
if (typeof document !== 'undefined') {
const template = document.createElement('template');
template.innerHTML = cleaned;
title = template.content.querySelector?.('svg title')?.textContent?.trim() || title;
if (typeof DOMParser !== 'undefined') {
const svg = new DOMParser().parseFromString(cleaned, 'image/svg+xml');
if (!svg.querySelector('parsererror')) {
title = svg.querySelector('svg title')?.textContent?.trim() || title;
}
}
const csp = "default-src 'none'; img-src 'none'; media-src 'none'; font-src 'none'; style-src 'unsafe-inline'";
const srcdoc = `<!doctype html><html><head><meta charset="utf-8"><meta name="viewport" content="width=device-width,initial-scale=1"><meta http-equiv="Content-Security-Policy" content="${csp}"><style>:root{${svgThemeCss()}}html,body{margin:0;min-height:100%;background:var(--bg);color:var(--fg);overflow:hidden}body{display:grid;place-items:center}svg{display:block;width:100%;height:100%;max-width:100%;background:var(--bg);color:var(--fg)}</style></head><body>${cleaned}</body></html>`;

View file

@ -435,7 +435,7 @@ function getFolderNames() {
async function moveToFolder(sessionId, folderName) {
const fd = new FormData();
fd.append('folder', folderName || '');
await fetch(`${API_BASE}/api/session/${sessionId}`, { method: 'PATCH', body: fd });
await fetch(`${API_BASE}/api/session/${encodeURIComponent(sessionId)}`, { method: 'PATCH', body: fd });
// Update local data
const s = sessions.find(x => x.id === sessionId);
if (s) s.folder = folderName || null;

View file

@ -30,6 +30,8 @@ const { extractThemeBootstrap } = require('./helpers/theme_bootstrap.cjs');
// Expose the real internal loader only in this served test copy.
await page.route('**/static/js/admin-codeql-harness.js', route => route.fulfill({ contentType: 'application/javascript',
body: readFileSync('static/js/admin.js', 'utf8') + '\nexport { loadEndpoints };\n' }));
await page.route('**/static/js/sessions-codeql-harness.js', route => route.fulfill({ contentType: 'application/javascript',
body: readFileSync('static/js/sessions.js', 'utf8') + '\nexport { moveToFolder };\n' }));
await page.goto(origin + '/security-harness');
const rendered = await page.evaluate(async () => {
@ -58,14 +60,19 @@ const { extractThemeBootstrap } = require('./helpers/theme_bootstrap.cjs');
}
const valid = addMessage('user', 'In the document, edit this specific text (lines 1–2):\n```\nselected\n```\n\nInstruction: **Keep bold** and `code`');
const titles = [
{ source: '<svg xmlns="http://www.w3.org/2000/svg"><title>Valid &amp; safe</title></svg>', title: 'Valid & safe' },
{ source: '<svg><title>Nested <b>bold</b> &amp; text</title></svg>', title: 'Nested bold & text' },
{ source: '<svg><title>\" onload=\"parent.executed++ &lt;script&gt;</title></svg>', title: '\" onload=\"parent.executed++ <script>' },
{ source: '<svg><title>Malformed <b>nested</title ></svg>', title: 'Malformed nested' },
{ source: '<svg><title>Malformed <b>nested</title ></svg>', title: 'Visual explanation' },
{ source: '<svg><title><script>parent.executed++</script><b onload="parent.executed++">nested</b></title></svg>', title: 'parent.executed++nested' },
{ source: '<svg><title><![CDATA["><img src=x onerror="parent.executed++">]]></title></svg>', title: '"><img src=x onerror="parent.executed++">' },
{ source: '<svg><title></title><img src=x onerror="parent.executed++"></svg>', title: 'Visual explanation' },
{ source: '<svg><title> </title></svg>', title: 'Visual explanation' },
{ source: '<svg><text>No title</text></svg>', title: 'Visual explanation' },
].map(({ source, title }) => {
const host = document.createElement('div');
host.innerHTML = markdown.mdToHtml('```svg\n' + source + '\n```');
document.body.appendChild(host);
const frame = host.querySelector('iframe');
return { expected: title, actual: frame.title, sandbox: frame.getAttribute('sandbox'),
referrer: frame.referrerPolicy, onload: frame.hasAttribute('onload'),
@ -92,6 +99,17 @@ const { extractThemeBootstrap } = require('./helpers/theme_bootstrap.cjs');
assert.equal(title.csp, "default-src 'none'; img-src 'none'; media-src 'none'; font-src 'none'; style-src 'unsafe-inline'");
}
const folderRequest = page.waitForRequest(request => request.method() === 'PATCH');
const sessionId = 'session/other?folder=bad#fragment%value';
await page.evaluate(async id => {
const { moveToFolder } = await import('/static/js/sessions-codeql-harness.js');
await moveToFolder(id, 'Safe folder');
}, sessionId);
const folderUrl = new URL((await folderRequest).url());
assert.equal(folderUrl.pathname, '/api/session/' + encodeURIComponent(sessionId));
assert.equal(folderUrl.search, '');
assert.equal(folderUrl.hash, '');
const menus = await page.evaluate(async () => {
const { _showReaderMoreMenu } = await import('/static/js/emailLibrary/menus.js');
const { _safeRenderEmailBody } = await import('/static/js/emailLibrary/bodyRender.js');

View file

@ -0,0 +1,249 @@
"""Caller model URLs cannot exceed enabled, owner-visible registration."""
from types import SimpleNamespace
from unittest.mock import AsyncMock, MagicMock
import pytest
from fastapi import HTTPException
import core.database as database
import routes.assistant_routes as assistant_routes
import routes.skills_routes as skills_routes
import routes.task.task_routes as task_routes
import src.endpoint_resolver as resolver
from tests.helpers.database import disposable_database
LOCAL = "http://localhost:1234/v1"
LAN = "http://192.168.1.20:8000/v1"
SHARED = "http://127.0.0.1:11434/api"
REJECTED = [
"https://unregistered.example/v1",
"http://169.254.169.254/latest/meta-data",
"http://bob.example/v1",
"http://disabled.example/v1",
"http://caller:secret@localhost:1234/v1",
LOCAL + "?api_key=caller-secret",
LOCAL + "#fragment",
LOCAL + "/other-base",
"not-a-url",
" ",
]
def _request(body=None, owner="alice"):
return SimpleNamespace(
state=SimpleNamespace(current_user=owner),
headers={},
json=AsyncMock(return_value=body or {}),
)
def _route(router, method, path):
return next(r.endpoint for r in router.routes if r.path == path and method in r.methods)
@pytest.fixture
def registered_db(tmp_path, monkeypatch):
with disposable_database(tmp_path) as factory:
for module in (database, task_routes, assistant_routes, resolver):
monkeypatch.setattr(module, "SessionLocal", factory)
with factory() as db:
for endpoint_id, owner, url, enabled in [
("local", "alice", LOCAL + "/", True),
("lan", "alice", LAN, True),
("shared", None, SHARED, True),
("bob", "bob", "http://bob.example/v1", True),
("disabled", "alice", "http://disabled.example/v1", False),
]:
db.add(database.ModelEndpoint(
id=endpoint_id, name=endpoint_id, owner=owner, base_url=url,
is_enabled=enabled, api_key="server-secret",
))
db.add(database.ScheduledTask(
id="task", owner="alice", name="Existing task", prompt="Work",
task_type="llm", trigger_type="webhook", status="active",
endpoint_url=LOCAL,
))
db.add(database.CrewMember(
id="assistant", owner="alice", name="Assistant",
is_default_assistant=True, endpoint_url=LOCAL,
))
db.commit()
yield factory
@pytest.mark.parametrize("url", REJECTED + ["", None, 42])
def test_registered_endpoint_helper_rejects_invalid_or_invisible_url(registered_db, url):
with registered_db() as db, pytest.raises(ValueError):
resolver.resolve_owner_registered_endpoint_url(db, url, "alice")
@pytest.mark.parametrize("url, canonical", [
("HTTP://LOCALHOST:1234/v1/chat/completions/", LOCAL),
(LAN + "/models", LAN),
(SHARED + "/chat", SHARED),
])
def test_registered_endpoint_helper_preserves_local_lan_and_shared(registered_db, url, canonical):
with registered_db() as db:
assert resolver.resolve_owner_registered_endpoint_url(db, url, "alice") == canonical
@pytest.mark.asyncio
@pytest.mark.parametrize("url", REJECTED)
async def test_task_create_rejects_unregistered_url_before_persistence(registered_db, url):
create = _route(task_routes.setup_task_routes(MagicMock()), "POST", "/api/tasks")
with pytest.raises(HTTPException) as exc:
await create(_request(), task_routes.TaskCreate(
name="Rejected", prompt="Work", trigger_type="webhook", endpoint_url=url,
))
assert exc.value.status_code == 400
with registered_db() as db:
assert db.query(database.ScheduledTask).count() == 1
@pytest.mark.asyncio
@pytest.mark.parametrize("url", REJECTED)
async def test_task_update_rejects_unregistered_url_before_persistence(registered_db, url):
update = _route(task_routes.setup_task_routes(MagicMock()), "PUT", "/api/tasks/{task_id}")
with pytest.raises(HTTPException) as exc:
await update(_request(), "task", task_routes.TaskUpdate(endpoint_url=url, name="Rejected"))
assert exc.value.status_code == 400
with registered_db() as db:
task = db.get(database.ScheduledTask, "task")
assert task.endpoint_url == LOCAL
assert task.name == "Existing task"
@pytest.mark.asyncio
async def test_task_create_and_update_store_registered_canonical_url(registered_db):
router = task_routes.setup_task_routes(MagicMock())
create = _route(router, "POST", "/api/tasks")
update = _route(router, "PUT", "/api/tasks/{task_id}")
result = await create(_request(), task_routes.TaskCreate(
name="Accepted", prompt="Work", trigger_type="webhook",
endpoint_url="HTTP://LOCALHOST:1234/v1/chat/completions/",
))
assert result["endpoint_url"] == LOCAL
assert (await update(_request(), "task", task_routes.TaskUpdate(
endpoint_url=LOCAL + "/models",
)))["endpoint_url"] == LOCAL
with registered_db() as db:
created = db.get(database.ScheduledTask, result["id"])
assert created.owner == "alice"
assert created.endpoint_url == LOCAL
assert created.request_authority_json
assert db.get(database.ScheduledTask, "task").endpoint_url == LOCAL
@pytest.mark.asyncio
async def test_task_empty_override_still_restores_default(registered_db):
router = task_routes.setup_task_routes(MagicMock())
create = _route(router, "POST", "/api/tasks")
update = _route(router, "PUT", "/api/tasks/{task_id}")
result = await create(_request(), task_routes.TaskCreate(
name="Default", prompt="Work", trigger_type="webhook", endpoint_url="",
))
assert result["endpoint_url"] is None
assert (await update(_request(), "task", task_routes.TaskUpdate(endpoint_url="")))["endpoint_url"] is None
@pytest.mark.asyncio
async def test_task_update_keeps_task_owner_verification(registered_db):
update = _route(task_routes.setup_task_routes(MagicMock()), "PUT", "/api/tasks/{task_id}")
with pytest.raises(HTTPException) as exc:
await update(_request(owner="bob"), "task", task_routes.TaskUpdate(endpoint_url=LOCAL))
assert exc.value.status_code == 403
@pytest.mark.asyncio
@pytest.mark.parametrize("url", REJECTED)
async def test_assistant_settings_reject_unregistered_url(registered_db, url):
update = _route(assistant_routes.setup_assistant_routes(MagicMock()), "PATCH", "/api/assistant/settings")
with pytest.raises(HTTPException) as exc:
await update(assistant_routes.AssistantSettingsUpdate(endpoint_url=url, name="Rejected"), _request())
assert exc.value.status_code == 400
with registered_db() as db:
crew = db.get(database.CrewMember, "assistant")
assert crew.endpoint_url == LOCAL
assert crew.name == "Assistant"
@pytest.mark.asyncio
async def test_assistant_settings_accept_registered_local_endpoint(registered_db):
update = _route(assistant_routes.setup_assistant_routes(MagicMock()), "PATCH", "/api/assistant/settings")
await update(assistant_routes.AssistantSettingsUpdate(endpoint_url=LOCAL + "/chat/completions"), _request())
with registered_db() as db:
assert db.get(database.CrewMember, "assistant").endpoint_url == LOCAL
@pytest.fixture
def skill_test_route(registered_db, monkeypatch):
manager = SimpleNamespace(
load=lambda owner: [{"name": "skill", "owner": "alice"}],
read_skill_md=lambda name, owner: "# Skill",
)
monkeypatch.setattr(resolver, "resolve_endpoint", lambda *a, **kw: (None, None, None))
probe = MagicMock(return_value=["model"])
monkeypatch.setattr("src.llm_core.list_model_ids", probe)
run = AsyncMock()
monkeypatch.setattr(skills_routes, "_run_skill_test_job", run)
test = _route(skills_routes.setup_skills_routes(manager), "POST", "/api/skills/{skill_id}/test")
try:
yield test, probe, run
finally:
skills_routes._skill_test_jobs.pop(("alice", "skill"), None)
@pytest.mark.asyncio
@pytest.mark.parametrize("url", REJECTED + [""])
async def test_skill_test_rejects_raw_fallback_before_network_or_execution(skill_test_route, url):
test, probe, run = skill_test_route
with pytest.raises(HTTPException) as exc:
await test(_request({"endpoint_url": url, "model": "model", "headers": {"Authorization": "attacker"}}), "skill")
assert exc.value.status_code == 400
probe.assert_not_called()
run.assert_not_called()
assert ("alice", "skill") not in skills_routes._skill_test_jobs
@pytest.mark.asyncio
@pytest.mark.parametrize("server_key", ["server-secret", None])
async def test_skill_test_uses_registered_runtime_credentials(skill_test_route, registered_db, server_key):
import asyncio
with registered_db() as db:
db.get(database.ModelEndpoint, "local").api_key = server_key
db.commit()
test, probe, run = skill_test_route
await test(_request({
"endpoint_url": "HTTP://LOCALHOST:1234/v1/chat/completions/", "model": "model",
"headers": {"Authorization": "Bearer attacker", "x-api-key": "attacker", "Host": "169.254.169.254"},
}), "skill")
await asyncio.sleep(0)
headers = {"Authorization": "Bearer server-secret"} if server_key else {}
probe.assert_called_once_with(LOCAL + "/chat/completions", headers=headers)
assert run.await_args.args[4:8] == (LOCAL + "/chat/completions", "model", headers, "alice")
assert skills_routes._skill_test_jobs[("alice", "skill")]["_run"]["headers"] == headers
@pytest.mark.asyncio
async def test_skill_test_prefers_utility_and_ignores_request_headers(skill_test_route, monkeypatch):
import asyncio
test, probe, run = skill_test_route
monkeypatch.setattr(resolver, "resolve_endpoint", lambda *a, **kw: (LAN + "/chat/completions", "model", None))
await test(_request({"endpoint_url": REJECTED[1], "headers": {"Authorization": "attacker"}}), "skill")
await asyncio.sleep(0)
probe.assert_called_once_with(LAN + "/chat/completions", headers=None)
assert run.await_args.args[6] is None
@pytest.mark.asyncio
async def test_skill_test_keeps_skill_owner_verification(skill_test_route):
test, probe, run = skill_test_route
with pytest.raises(HTTPException) as exc:
await test(_request({"endpoint_url": "http://bob.example/v1", "model": "model"}, owner="bob"), "skill")
assert exc.value.status_code == 404
probe.assert_not_called()
run.assert_not_called()

View file

@ -7,6 +7,17 @@ from tests.helpers.document_source import document_source
_REPO = Path(__file__).resolve().parent.parent
def test_svg_title_extraction_uses_xml_text_without_html_assignment():
src = (_REPO / "static" / "js" / "markdown.js").read_text(encoding="utf-8")
render = src.split("function renderSvgSandbox(source)", 1)[1].split(
"function replaceRawSvgBlocks", 1
)[0]
assert "innerHTML" not in render
assert "parseFromString(cleaned, 'image/svg+xml')" in render
assert "textContent" in render
assert "parsererror" in render
def test_markdown_raw_html_sanitizer_checks_url_attr_edge_cases():
src = (_REPO / "static" / "js" / "markdown.js").read_text(encoding="utf-8")