Merge pull request 'Release v26.03.29.1 — security fixes and MCP bug fixes' (#18) from dev into main
Release v26.03.29.1 — security fixes and MCP bug fixes
This commit was merged in pull request #18.
This commit is contained in:
@@ -298,7 +298,7 @@ async def fable_update_task(
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
async def fable_add_task_log(task_id: int, body: str) -> dict:
|
||||
async def fable_add_task_log(task_id: int, content: str) -> dict:
|
||||
"""Append a timestamped progress log entry to a Fable task.
|
||||
|
||||
Use this to record work sessions, decisions, or status updates over time without
|
||||
@@ -306,7 +306,7 @@ async def fable_add_task_log(task_id: int, body: str) -> dict:
|
||||
chronologically in the task view.
|
||||
"""
|
||||
async with FableClient() as client:
|
||||
return await tasks.add_task_log(client, task_id=task_id, body=body)
|
||||
return await tasks.add_task_log(client, task_id=task_id, content=content)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -87,7 +87,7 @@ async def add_task_log(
|
||||
client: FableClient,
|
||||
*,
|
||||
task_id: int,
|
||||
body: str,
|
||||
content: str,
|
||||
) -> dict[str, Any]:
|
||||
"""Append a log entry to a task."""
|
||||
return await client.post(f"/api/tasks/{task_id}/logs", json={"body": body})
|
||||
return await client.post(f"/api/tasks/{task_id}/logs", json={"content": content})
|
||||
|
||||
@@ -46,9 +46,9 @@ async def test_update_task_patches(client):
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_task_log_posts_to_logs_endpoint(client):
|
||||
from fable_mcp.tools.tasks import add_task_log
|
||||
client.post = AsyncMock(return_value={"id": 1, "body": "progress"})
|
||||
await add_task_log(client, task_id=9, body="progress")
|
||||
client.post = AsyncMock(return_value={"id": 1, "content": "progress"})
|
||||
await add_task_log(client, task_id=9, content="progress")
|
||||
client.post.assert_called_once()
|
||||
path = client.post.call_args[0][0]
|
||||
assert path == "/api/tasks/9/logs"
|
||||
assert client.post.call_args[1]["json"]["body"] == "progress"
|
||||
assert client.post.call_args[1]["json"]["content"] == "progress"
|
||||
|
||||
@@ -123,7 +123,6 @@ export interface NotificationEntry {
|
||||
export interface UserSearchResult {
|
||||
id: number
|
||||
username: string
|
||||
email: string | null
|
||||
}
|
||||
|
||||
// --- User search ---
|
||||
|
||||
@@ -138,7 +138,6 @@ onMounted(async () => {
|
||||
<ul v-if="userResults.length" class="user-results">
|
||||
<li v-for="u in userResults" :key="u.id" @click="selectUser(u)" class="user-result-item">
|
||||
<span class="user-result-name">{{ u.username }}</span>
|
||||
<span class="user-result-email">{{ u.email }}</span>
|
||||
</li>
|
||||
</ul>
|
||||
</div>
|
||||
|
||||
@@ -38,7 +38,7 @@ const headingRenderer = {
|
||||
marked.use({ renderer: headingRenderer });
|
||||
|
||||
const PURIFY_OPTS_FULL = {
|
||||
ADD_ATTR: ["data-tag", "data-title", "src", "alt"],
|
||||
ADD_ATTR: ["data-tag", "data-title"],
|
||||
FORCE_BODY: true,
|
||||
};
|
||||
|
||||
|
||||
@@ -2263,7 +2263,6 @@ FABLE_API_KEY=<your-api-key></pre>
|
||||
<ul v-if="groupMemberResults.length" class="member-results">
|
||||
<li v-for="u in groupMemberResults" :key="u.id" class="member-result-item" @click="addMemberToGroup(g.id, u)">
|
||||
<span class="member-result-name">{{ u.username }}</span>
|
||||
<span class="member-result-email">{{ u.email }}</span>
|
||||
</li>
|
||||
</ul>
|
||||
</div>
|
||||
|
||||
@@ -62,6 +62,13 @@ def create_app() -> Quart:
|
||||
"Set SECRET_KEY or SECRET_KEY_FILE for production use."
|
||||
)
|
||||
|
||||
if not Config.TRUST_PROXY_HEADERS:
|
||||
logger.warning(
|
||||
"TRUST_PROXY_HEADERS is not set. If this instance is behind a reverse proxy "
|
||||
"(nginx, Caddy, Traefik) set TRUST_PROXY_HEADERS=true so rate limiting uses "
|
||||
"real client IPs rather than the proxy IP."
|
||||
)
|
||||
|
||||
app.register_blueprint(admin_bp)
|
||||
app.register_blueprint(api)
|
||||
app.register_blueprint(auth_bp)
|
||||
|
||||
@@ -88,5 +88,10 @@ class Config:
|
||||
errors.append(f"SMTP_PORT={cls.SMTP_PORT} must be between 1 and 65535")
|
||||
if cls.oidc_enabled() and not cls.BASE_URL.startswith(("http://", "https://")):
|
||||
errors.append(f"BASE_URL='{cls.BASE_URL}' must start with http:// or https:// when OIDC is enabled")
|
||||
if cls.SECRET_KEY == "dev-secret-change-me" and cls.SECURE_COOKIES:
|
||||
errors.append(
|
||||
"SECRET_KEY is set to the insecure default but SECURE_COOKIES=true indicates "
|
||||
"a production deployment. Set SECRET_KEY or SECRET_KEY_FILE before starting."
|
||||
)
|
||||
if errors:
|
||||
raise ValueError("Configuration errors:\n" + "\n".join(f" - {e}" for e in errors))
|
||||
|
||||
@@ -1,3 +1,12 @@
|
||||
"""In-process sliding-window rate limiter.
|
||||
|
||||
IMPORTANT — deployment note:
|
||||
Rate limit counters are stored in memory and are lost on process restart.
|
||||
When deployed behind a reverse proxy (nginx, Caddy, Traefik) you MUST set
|
||||
TRUST_PROXY_HEADERS=true so that the real client IP is used as the bucket key
|
||||
rather than the proxy's IP (which would cause all users to share one bucket).
|
||||
"""
|
||||
|
||||
import asyncio
|
||||
import time
|
||||
from collections import defaultdict
|
||||
|
||||
@@ -359,7 +359,10 @@ async def oauth_callback():
|
||||
return redirect("/login?error=oauth")
|
||||
|
||||
sub = claims.get("sub", "")
|
||||
email = claims.get("email", "")
|
||||
# Only trust the email claim for account linking if the provider has verified it.
|
||||
# An unverified email could be used to hijack an existing local account.
|
||||
email_verified = claims.get("email_verified", False)
|
||||
email = claims.get("email", "") if email_verified else ""
|
||||
preferred_username = claims.get("preferred_username", "")
|
||||
|
||||
if not sub:
|
||||
|
||||
@@ -71,6 +71,12 @@ async def add_feed():
|
||||
url = (data.get("url") or "").strip()
|
||||
if not url:
|
||||
return jsonify({"error": "url required"}), 400
|
||||
scheme = url.split("://")[0].lower() if "://" in url else ""
|
||||
if scheme not in ("http", "https"):
|
||||
return jsonify({"error": "Feed URL must use http or https"}), 400
|
||||
from fabledassistant.services.llm import _is_private_url
|
||||
if _is_private_url(url):
|
||||
return jsonify({"error": "Feed URL must not point to an internal address"}), 400
|
||||
category = data.get("category") or None
|
||||
|
||||
async with async_session() as session:
|
||||
|
||||
@@ -5,7 +5,7 @@ import logging
|
||||
import httpx
|
||||
from quart import Blueprint, Response, jsonify, request
|
||||
|
||||
from fabledassistant.auth import login_required, get_current_user_id
|
||||
from fabledassistant.auth import admin_required, login_required, get_current_user_id
|
||||
from fabledassistant.routes.utils import not_found, parse_pagination
|
||||
from fabledassistant.config import Config
|
||||
from fabledassistant.services.chat import (
|
||||
@@ -439,7 +439,7 @@ async def list_models_route():
|
||||
|
||||
|
||||
@chat_bp.route("/models/pull", methods=["POST"])
|
||||
@login_required
|
||||
@admin_required
|
||||
async def pull_model_route():
|
||||
"""Pull a model from Ollama, streaming progress via SSE."""
|
||||
data = await request.get_json()
|
||||
@@ -478,7 +478,7 @@ async def pull_model_route():
|
||||
|
||||
|
||||
@chat_bp.route("/models/delete", methods=["POST"])
|
||||
@login_required
|
||||
@admin_required
|
||||
async def delete_model_route():
|
||||
"""Delete a model from Ollama."""
|
||||
data = await request.get_json()
|
||||
|
||||
@@ -1,14 +1,10 @@
|
||||
"""Serve locally-cached images.
|
||||
|
||||
No authentication required — image IDs are opaque and unguessable (based
|
||||
on the SHA-256 of the original URL). Images are served with a 1-day
|
||||
Cache-Control header so the browser doesn't re-request on every page load.
|
||||
"""
|
||||
"""Serve locally-cached images."""
|
||||
|
||||
import logging
|
||||
|
||||
from quart import Blueprint, jsonify, send_file
|
||||
|
||||
from fabledassistant.auth import login_required
|
||||
from fabledassistant.services.images import get_image_path, get_image_record
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
@@ -17,6 +13,7 @@ images_bp = Blueprint("images", __name__, url_prefix="/api/images")
|
||||
|
||||
|
||||
@images_bp.route("/<int:image_id>", methods=["GET"])
|
||||
@login_required
|
||||
async def serve_image(image_id: int):
|
||||
"""Serve a locally-cached image by its DB ID."""
|
||||
record = await get_image_record(image_id)
|
||||
|
||||
@@ -50,12 +50,16 @@ async def create_milestone_route(project_id: int):
|
||||
data = await request.get_json()
|
||||
if not data.get("title"):
|
||||
return jsonify({"error": "title is required"}), 400
|
||||
status = data.get("status", "active")
|
||||
if status not in ("active", "done"):
|
||||
return jsonify({"error": "status must be 'active' or 'done'"}), 400
|
||||
milestone = await create_milestone(
|
||||
uid,
|
||||
project_id,
|
||||
title=data["title"],
|
||||
description=data.get("description"),
|
||||
order_index=data.get("order_index", 0),
|
||||
status=status,
|
||||
)
|
||||
return jsonify(await _milestone_dict(milestone)), 201
|
||||
|
||||
|
||||
@@ -38,12 +38,16 @@ async def create_project_route():
|
||||
data = await request.get_json()
|
||||
if not data.get("title"):
|
||||
return jsonify({"error": "title is required"}), 400
|
||||
status = data.get("status", "active")
|
||||
if status not in ("active", "archived"):
|
||||
return jsonify({"error": "status must be 'active' or 'archived'"}), 400
|
||||
project = await create_project(
|
||||
uid,
|
||||
title=data["title"],
|
||||
description=data.get("description", ""),
|
||||
goal=data.get("goal", ""),
|
||||
color=data.get("color"),
|
||||
status=status,
|
||||
)
|
||||
return jsonify(project.to_dict()), 201
|
||||
|
||||
|
||||
@@ -3,7 +3,7 @@ from quart import Blueprint, jsonify, request
|
||||
from fabledassistant.auth import login_required, get_current_user_id
|
||||
from fabledassistant.config import Config
|
||||
from fabledassistant.services.caldav import CALDAV_SETTING_KEYS, get_caldav_config, test_connection
|
||||
from fabledassistant.services.llm import get_installed_models
|
||||
from fabledassistant.services.llm import get_installed_models, _is_private_url
|
||||
from fabledassistant.services.settings import delete_setting, get_all_settings, set_settings_batch
|
||||
|
||||
settings_bp = Blueprint("settings", __name__, url_prefix="/api/settings")
|
||||
@@ -77,6 +77,16 @@ async def update_caldav():
|
||||
uid = get_current_user_id()
|
||||
data = await request.get_json()
|
||||
|
||||
# Validate CalDAV URL before saving — block internal/private addresses
|
||||
if "caldav_url" in data:
|
||||
url = str(data.get("caldav_url") or "").strip()
|
||||
if url:
|
||||
parsed_scheme = url.split("://")[0].lower() if "://" in url else ""
|
||||
if parsed_scheme not in ("http", "https"):
|
||||
return jsonify({"error": "CalDAV URL must use http or https"}), 400
|
||||
if _is_private_url(url):
|
||||
return jsonify({"error": "CalDAV URL must not point to an internal or private address"}), 400
|
||||
|
||||
settings_to_save = {}
|
||||
for key in CALDAV_SETTING_KEYS:
|
||||
if key in data:
|
||||
|
||||
@@ -24,6 +24,6 @@ async def search_users():
|
||||
).limit(10)
|
||||
)).scalars().all()
|
||||
return jsonify({"users": [
|
||||
{"id": u.id, "username": u.username, "email": u.email}
|
||||
{"id": u.id, "username": u.username}
|
||||
for u in users
|
||||
]})
|
||||
|
||||
@@ -7,7 +7,9 @@ twice shares one file and one DB row.
|
||||
"""
|
||||
|
||||
import hashlib
|
||||
import ipaddress
|
||||
import logging
|
||||
import socket
|
||||
from pathlib import Path
|
||||
from urllib.parse import urlparse
|
||||
|
||||
@@ -21,6 +23,27 @@ from fabledassistant.models.image_cache import ImageCache
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def _is_safe_image_url(url: str) -> bool:
|
||||
"""Return True only if the URL is a public http/https address (SSRF guard)."""
|
||||
try:
|
||||
parsed = urlparse(url)
|
||||
if parsed.scheme not in ("http", "https"):
|
||||
return False
|
||||
host = parsed.hostname
|
||||
if not host:
|
||||
return False
|
||||
if host.lower() in ("localhost", "::1"):
|
||||
return False
|
||||
addr_info = socket.getaddrinfo(host, None, proto=socket.IPPROTO_TCP)
|
||||
for entry in addr_info:
|
||||
ip = ipaddress.ip_address(entry[4][0])
|
||||
if ip.is_private or ip.is_loopback or ip.is_link_local or ip.is_reserved or ip.is_multicast:
|
||||
return False
|
||||
return True
|
||||
except Exception:
|
||||
return False # Block on resolution failure
|
||||
|
||||
_ALLOWED_TYPES = {
|
||||
"image/jpeg",
|
||||
"image/png",
|
||||
@@ -59,6 +82,10 @@ async def fetch_and_store_image(
|
||||
Returns None if the URL is unreachable, not a valid image type, or
|
||||
exceeds IMAGE_MAX_BYTES.
|
||||
"""
|
||||
if not _is_safe_image_url(url):
|
||||
logger.warning("Blocked image fetch of private/internal URL: %s", url[:80])
|
||||
return None
|
||||
|
||||
url_hash = _url_hash(url)
|
||||
|
||||
# Return existing record if already cached
|
||||
|
||||
@@ -17,6 +17,7 @@ async def create_milestone(
|
||||
title: str,
|
||||
description: str | None = None,
|
||||
order_index: int = 0,
|
||||
status: str = "active",
|
||||
) -> Milestone:
|
||||
async with async_session() as session:
|
||||
milestone = Milestone(
|
||||
@@ -24,7 +25,7 @@ async def create_milestone(
|
||||
project_id=project_id,
|
||||
title=title,
|
||||
description=description,
|
||||
status="active",
|
||||
status=status,
|
||||
order_index=order_index,
|
||||
)
|
||||
session.add(milestone)
|
||||
|
||||
@@ -17,6 +17,7 @@ async def create_project(
|
||||
description: str = "",
|
||||
goal: str = "",
|
||||
color: str | None = None,
|
||||
status: str = "active",
|
||||
) -> Project:
|
||||
async with async_session() as session:
|
||||
project = Project(
|
||||
@@ -25,7 +26,7 @@ async def create_project(
|
||||
description=description,
|
||||
goal=goal,
|
||||
color=color,
|
||||
status="active",
|
||||
status=status,
|
||||
)
|
||||
session.add(project)
|
||||
await session.commit()
|
||||
|
||||
@@ -55,6 +55,10 @@ async def fetch_and_cache_feed(feed_id: int, url: str) -> int:
|
||||
Fetch a feed URL, parse it, and upsert new items into rss_items.
|
||||
Returns the number of new items stored.
|
||||
"""
|
||||
scheme = url.split("://")[0].lower() if "://" in url else ""
|
||||
if scheme not in ("http", "https"):
|
||||
logger.warning("Blocked RSS fetch with non-http(s) scheme: %s", url[:80])
|
||||
return 0
|
||||
try:
|
||||
parsed = await _parse_feed(url)
|
||||
except Exception:
|
||||
|
||||
Reference in New Issue
Block a user