fix: bugs.

Author: PttCodingMan

backend/app/routers/acl.py

@@ -206,9 +206,12 @@ async def put_page_acl(slug: str, body: AclPutBody, user=Depends(get_current_use
                VALUES (?, ?, ?, ?)""",
             (page["id"], r.principal_type, r.principal_id, r.permission),
         )
+    # Metadata intentionally omits principal details: activity_log is readable
+    # by any authenticated user (see pending finding #5), and leaking the
+    # principal/permission list would reveal organizational ACL structure.
     await log_activity(
         db, user["id"], "acl_updated", "page", page["id"],
-        {"slug": slug, "rows": [r.model_dump() for r in body.rows]},
+        {"slug": slug, "row_count": len(body.rows)},
     )
     await db.commit()
     invalidate_readable_cache()

backend/app/routers/export.py

@@ -1,4 +1,5 @@
 import base64
+import html as _html
 import io
 import mimetypes
 import re
@@ -283,9 +284,12 @@ async def export_page(
     if await resolve_page_permission(db, user, page["id"]) == "none":
         raise HTTPException(status_code=404, detail="Page not found")
     html_content = _inline_media_srcs(md_to_simple_html(page["content_md"]))
+    # Escape title/slug: they're rendered into <title>, <h1>, and a meta
+    # breadcrumb as raw text, so a page title containing e.g. `<script>` would
+    # execute when the exported HTML is opened locally (file:// context).
     full_html = HTML_TEMPLATE.format(
-        title=page["title"],
-        slug=page["slug"],
+        title=_html.escape(page["title"]),
+        slug=_html.escape(page["slug"]),
         content=html_content,
     )
 
@@ -331,13 +335,20 @@ async def export_site(
         for p in pages:
             page = dict(p)
             html_content = _inline_media_srcs(md_to_simple_html(page["content_md"]))
+            safe_title = _html.escape(page["title"])
+            safe_slug = _html.escape(page["slug"])
             full_html = HTML_TEMPLATE.format(
-                title=page["title"],
-                slug=page["slug"],
+                title=safe_title,
+                slug=safe_slug,
                 content=html_content,
             )
             zf.writestr(f"{page['slug']}.html", full_html)
-            page_links.append(f'<li><a href="{page["slug"]}.html">{page["title"]}</a></li>')
+            # Both href and link text need escaping — slug may contain chars
+            # that break the attribute, title may contain HTML.
+            page_links.append(
+                f'<li><a href="{urllib.parse.quote(page["slug"], safe="")}.html">'
+                f'{safe_title}</a></li>'
+            )
 
         # Generate index
         index_html = SITE_INDEX_TEMPLATE.format(page_list="\n".join(page_links))

backend/app/routers/templates.py

@@ -1,7 +1,7 @@
 import aiosqlite
 from fastapi import APIRouter, HTTPException, Depends
 from app.schemas import TemplateCreate, TemplateUpdate, TemplateResponse
-from app.auth import get_current_user
+from app.auth import get_current_user, require_admin
 from app.database import get_db
 
 router = APIRouter(prefix="/api/templates", tags=["templates"])
@@ -15,7 +15,7 @@ async def list_templates(user=Depends(get_current_user)):
 
 
 @router.post("", response_model=TemplateResponse, status_code=201)
-async def create_template(body: TemplateCreate, user=Depends(get_current_user)):
+async def create_template(body: TemplateCreate, user=Depends(require_admin)):
     db = await get_db()
     try:
         cursor = await db.execute(
@@ -33,7 +33,7 @@ async def create_template(body: TemplateCreate, user=Depends(get_current_user)):
 
 @router.put("/{template_id}", response_model=TemplateResponse)
 async def update_template(
-    template_id: int, body: TemplateUpdate, user=Depends(get_current_user)
+    template_id: int, body: TemplateUpdate, user=Depends(require_admin)
 ):
     db = await get_db()
     rows = await db.execute_fetchall(
@@ -62,7 +62,7 @@ async def update_template(
 
 
 @router.delete("/{template_id}")
-async def delete_template(template_id: int, user=Depends(get_current_user)):
+async def delete_template(template_id: int, user=Depends(require_admin)):
     db = await get_db()
     rows = await db.execute_fetchall(
         "SELECT id FROM templates WHERE id = ?", (template_id,)

backend/tests/test_acl.py

@@ -837,6 +837,53 @@ async def test_acl_my_permission_endpoint():
     assert b.status_code == 404
 
 
+@pytest.mark.asyncio
+async def test_acl_updated_activity_log_omits_principal_rows():
+    """PUT /acl writes to activity_log, which is readable by any authenticated
+    user. The metadata must NOT contain principal_type/principal_id/permission
+    details — that would leak organizational ACL structure across the wiki.
+    """
+    import json as _json
+    db = await get_db()
+    admin = await _get_or_create_user(db, "acl_log_admin", "admin")
+    target = await _get_or_create_user(db, "acl_log_target", "editor")
+    page = await _make_page(db, "acl-log-page")
+
+    async with _token_client(admin) as client:
+        r = await client.put(
+            f"/api/pages/acl-log-page/acl",
+            json={
+                "rows": [
+                    {
+                        "principal_type": "user",
+                        "principal_id": target["id"],
+                        "permission": "write",
+                    }
+                ]
+            },
+        )
+    assert r.status_code == 200
+
+    rows = await db.execute_fetchall(
+        """SELECT metadata FROM activity_log
+           WHERE action = 'acl_updated' AND target_id = ?
+           ORDER BY id DESC LIMIT 1""",
+        (page,),
+    )
+    assert rows, "acl_updated activity entry missing"
+    meta = _json.loads(rows[0]["metadata"])
+    # Fine to expose slug + row count — both are bounded.
+    assert meta.get("slug") == "acl-log-page"
+    assert meta.get("row_count") == 1
+    # Must NOT contain principal details.
+    assert "rows" not in meta
+    serialized = _json.dumps(meta)
+    assert "principal_type" not in serialized
+    assert "principal_id" not in serialized
+    assert "permission" not in serialized
+    assert str(target["id"]) not in serialized
+
+
 @pytest.mark.asyncio
 async def test_users_search_endpoint():
     db = await get_db()

backend/tests/test_export.py

@@ -149,6 +149,43 @@ async def test_export_double_backtick_doesnt_desync_inline_code(auth_client):
     assert "</code>.env" not in body
 
 
+@pytest.mark.asyncio
+async def test_export_escapes_title_and_slug(auth_client):
+    """Title/slug are injected raw into <title>, <h1>, <a href>, meta — any
+    HTML in them must be escaped so opening the downloaded HTML doesn't run
+    attacker-controlled script.
+    """
+    await auth_client.post("/api/pages", json={
+        "title": "<script>alert('xss-title')</script>",
+        "content_md": "body",
+        "slug": "xss-target",
+    })
+    response = await auth_client.get("/api/export/page/xss-target")
+    assert response.status_code == 200
+    body = response.text
+    # Escaped form should appear; raw <script> tag for the title should not.
+    assert "&lt;script&gt;alert(&#x27;xss-title&#x27;)&lt;/script&gt;" in body
+    assert "<script>alert('xss-title')</script>" not in body
+
+
+@pytest.mark.asyncio
+async def test_export_site_escapes_title_in_index(admin_client):
+    """Site export's index.html lists page titles as link text — must escape
+    both the title (link text) and slug (href)."""
+    await admin_client.post("/api/pages", json={
+        "title": "<img src=x onerror=alert(1)>",
+        "content_md": "body",
+        "slug": "xss-in-index",
+    })
+    import zipfile, io
+    response = await admin_client.get("/api/export/site")
+    assert response.status_code == 200
+    with zipfile.ZipFile(io.BytesIO(response.content)) as zf:
+        index = zf.read("index.html").decode()
+    assert "&lt;img src=x onerror=alert(1)&gt;" in index
+    assert "<img src=x onerror=alert(1)>" not in index
+
+
 @pytest.mark.asyncio
 async def test_export_site_requires_admin(auth_client):
     # Non-admins should be blocked — site exports would otherwise silently

backend/tests/test_pages_advanced.py

@@ -64,15 +64,14 @@ async def test_page_move(auth_client):
     assert response.json()["sort_order"] == 5
 
 @pytest.mark.asyncio
-async def test_create_page_with_template(auth_client):
-    # 1. Create template
-    res_tmpl = await auth_client.post("/api/templates", json={
+async def test_create_page_with_template(auth_client, admin_client):
+    # Templates are admin-managed; any authenticated user can apply one.
+    res_tmpl = await admin_client.post("/api/templates", json={
         "name": "Tmpl",
         "content_md": "Template Content"
     })
     tmpl_id = res_tmpl.json()["id"]
 
-    # 2. Create page using template
     response = await auth_client.post("/api/pages", json={
         "title": "Tmpl Page",
         "template_id": tmpl_id

backend/tests/test_templates.py

@@ -1,9 +1,9 @@
 import pytest
 
 @pytest.mark.asyncio
-async def test_templates(auth_client):
+async def test_templates(admin_client):
     # Create template
-    response = await auth_client.post("/api/templates", json={
+    response = await admin_client.post("/api/templates", json={
         "name": "Test Template",
         "description": "Desc",
         "content_md": "# Tmpl Content"
@@ -12,54 +12,101 @@ async def test_templates(auth_client):
     tmpl_id = response.json()["id"]
 
     # List templates
-    response = await auth_client.get("/api/templates")
+    response = await admin_client.get("/api/templates")
     assert response.status_code == 200
     assert any(t["id"] == tmpl_id for t in response.json())
 
     # Update template
-    response = await auth_client.put(f"/api/templates/{tmpl_id}", json={
+    response = await admin_client.put(f"/api/templates/{tmpl_id}", json={
         "name": "Updated Tmpl"
     })
     assert response.status_code == 200
     assert response.json()["name"] == "Updated Tmpl"
 
     # Delete template
-    response = await auth_client.delete(f"/api/templates/{tmpl_id}")
+    response = await admin_client.delete(f"/api/templates/{tmpl_id}")
     assert response.status_code == 200
     assert response.json() == {"ok": True}
 
 
 @pytest.mark.asyncio
-async def test_create_duplicate_template_name_returns_409(auth_client):
+async def test_create_duplicate_template_name_returns_409(admin_client):
     payload = {"name": "Unique409Name", "description": "d", "content_md": "# A"}
-    r1 = await auth_client.post("/api/templates", json=payload)
+    r1 = await admin_client.post("/api/templates", json=payload)
     assert r1.status_code == 201
 
-    r2 = await auth_client.post("/api/templates", json=payload)
+    r2 = await admin_client.post("/api/templates", json=payload)
     assert r2.status_code == 409
 
     # cleanup
-    await auth_client.delete(f"/api/templates/{r1.json()['id']}")
+    await admin_client.delete(f"/api/templates/{r1.json()['id']}")
 
 
 @pytest.mark.asyncio
-async def test_update_template_to_duplicate_name_returns_409(auth_client):
-    t1 = (await auth_client.post("/api/templates", json={
+async def test_update_template_to_duplicate_name_returns_409(admin_client):
+    t1 = (await admin_client.post("/api/templates", json={
         "name": "Tmpl409A", "content_md": "a"
     })).json()
-    t2 = (await auth_client.post("/api/templates", json={
+    t2 = (await admin_client.post("/api/templates", json={
         "name": "Tmpl409B", "content_md": "b"
     })).json()
 
-    r = await auth_client.put(f"/api/templates/{t2['id']}", json={"name": "Tmpl409A"})
+    r = await admin_client.put(f"/api/templates/{t2['id']}", json={"name": "Tmpl409A"})
     assert r.status_code == 409
 
     # cleanup
-    await auth_client.delete(f"/api/templates/{t1['id']}")
-    await auth_client.delete(f"/api/templates/{t2['id']}")
+    await admin_client.delete(f"/api/templates/{t1['id']}")
+    await admin_client.delete(f"/api/templates/{t2['id']}")
 
 
 @pytest.mark.asyncio
-async def test_delete_nonexistent_template_returns_404(auth_client):
-    r = await auth_client.delete("/api/templates/999999")
+async def test_delete_nonexistent_template_returns_404(admin_client):
+    r = await admin_client.delete("/api/templates/999999")
     assert r.status_code == 404
+
+
+@pytest.mark.asyncio
+async def test_non_admin_cannot_create_template(auth_client):
+    r = await auth_client.post("/api/templates", json={
+        "name": "Forbidden", "content_md": "x"
+    })
+    assert r.status_code == 403
+
+
+@pytest.mark.asyncio
+async def test_non_admin_cannot_update_template(auth_client, admin_client):
+    created = (await admin_client.post("/api/templates", json={
+        "name": "OwnedByAdmin", "content_md": "x"
+    })).json()
+    try:
+        r = await auth_client.put(
+            f"/api/templates/{created['id']}", json={"name": "Hijacked"}
+        )
+        assert r.status_code == 403
+    finally:
+        await admin_client.delete(f"/api/templates/{created['id']}")
+
+
+@pytest.mark.asyncio
+async def test_non_admin_cannot_delete_template(auth_client, admin_client):
+    created = (await admin_client.post("/api/templates", json={
+        "name": "OwnedByAdmin2", "content_md": "x"
+    })).json()
+    try:
+        r = await auth_client.delete(f"/api/templates/{created['id']}")
+        assert r.status_code == 403
+    finally:
+        await admin_client.delete(f"/api/templates/{created['id']}")
+
+
+@pytest.mark.asyncio
+async def test_non_admin_can_still_list_templates(auth_client, admin_client):
+    created = (await admin_client.post("/api/templates", json={
+        "name": "Listable", "content_md": "x"
+    })).json()
+    try:
+        r = await auth_client.get("/api/templates")
+        assert r.status_code == 200
+        assert any(t["id"] == created["id"] for t in r.json())
+    finally:
+        await admin_client.delete(f"/api/templates/{created['id']}")