diff --git a/.env.example b/.env.example index 0f4dcd449..e96dd151c 100644 --- a/.env.example +++ b/.env.example @@ -169,6 +169,26 @@ SEARXNG_INSTANCE=http://localhost:8080 # ODYSSEUS_STT_MAX_AUDIO_BYTES=26214400 # speech-to-text audio (25 MB) # ODYSSEUS_ICS_MAX_BYTES=10485760 # calendar .ics import (10 MB) +# ============================================================ +# Host Docker access (explicit opt-in) +# ============================================================ +# Default Docker Compose does not mount /var/run/docker.sock. Existing +# Ollama, vLLM, and other OpenAI-compatible endpoints remain usable without it. +# +# Enable this only for intentional Cookbook/local Docker-daemon management. +# Raw socket access is high-trust and can grant broad control over the host +# Docker daemon. Set DOCKER_GID to the host docker group's numeric GID. +# Put these values in .env, or export them before running docker compose. +# COMPOSE_FILE=docker-compose.yml:docker/host-docker.yml +# DOCKER_GID=963 +# docker/host-docker.yml sets this inside the container. Keep it paired +# with the socket overlay; setting it alone is not sufficient. +# ODYSSEUS_ENABLE_HOST_DOCKER=true +# +# Host Docker access can be combined with one GPU overlay: +# COMPOSE_FILE=docker-compose.yml:docker/gpu.nvidia.yml:docker/host-docker.yml +# COMPOSE_FILE=docker-compose.yml:docker/gpu.amd.yml:docker/host-docker.yml + # ============================================================ # GPU support (Docker Compose) # ============================================================ diff --git a/.github/scripts/focused_test_guidance.py b/.github/scripts/focused_test_guidance.py new file mode 100644 index 000000000..1426d35fa --- /dev/null +++ b/.github/scripts/focused_test_guidance.py @@ -0,0 +1,124 @@ +#!/usr/bin/env python3 +"""Report focused pytest guidance for changed paths under tests/.""" + +from __future__ import annotations + +import argparse +import os +import shlex +import subprocess +import sys +from collections.abc import Iterable +from pathlib import PurePosixPath + + +def parse_paths(raw_paths: bytes) -> list[str]: + """Decode the NUL-delimited output of ``git diff --name-only -z``.""" + return [os.fsdecode(path) for path in raw_paths.split(b"\0") if path] + + +def changed_paths_from_merge_base(base_sha: str, head_sha: str) -> list[str]: + """Return changed ``tests/`` paths using GitHub PR three-dot semantics. + + GitHub PR changed files are based on the merge base and the PR head, not a + direct endpoint diff between the current base branch tip and the PR head. + Using the direct endpoint diff can include files changed only on the base + branch when the PR branch is stale. + """ + merge_base = subprocess.check_output( + ["git", "merge-base", base_sha, head_sha], + stderr=subprocess.DEVNULL, + ).strip() + raw_paths = subprocess.check_output( + [ + "git", + "diff", + "--name-only", + "--diff-filter=ACMRT", + "-z", + os.fsdecode(merge_base), + head_sha, + "--", + "tests/", + ], + ) + return parse_paths(raw_paths) + + +def select_test_paths(paths: Iterable[str]) -> list[str]: + """Return unique, repository-relative paths contained by tests/.""" + selected: set[str] = set() + for raw_path in paths: + path = PurePosixPath(raw_path) + if path.is_absolute() or ".." in path.parts: + continue + parts = tuple(part for part in path.parts if part != ".") + if len(parts) >= 2 and parts[0] == "tests": + selected.add(PurePosixPath(*parts).as_posix()) + return sorted(selected) + + +def is_pytest_file(path: str) -> bool: + """Return whether a changed path follows this repository's pytest naming.""" + name = PurePosixPath(path).name + return name.endswith(".py") and ( + name.startswith("test_") or name.endswith("_test.py") + ) + + +def pytest_command(paths: Iterable[str]) -> str: + """Build a copyable pytest command for changed runnable test files.""" + command = ["python3", "-m", "pytest", "-q", *paths] + return shlex.join(command) + + +def format_report(paths: Iterable[str]) -> str: + """Format focused guidance for CI logs and the workflow summary.""" + changed_paths = select_test_paths(paths) + runnable_paths = [path for path in changed_paths if is_pytest_file(path)] + lines = ["## Focused test guidance (report-only)", ""] + if not changed_paths: + lines.append("No changed paths under `tests/`.") + else: + lines.extend(["Changed paths under `tests/`:", ""]) + lines.extend(f"- `{path}`" for path in changed_paths) + lines.extend(["", "Suggested focused validation:", ""]) + if runnable_paths: + lines.append(f"```sh\n{pytest_command(runnable_paths)}\n```") + else: + lines.append("No directly runnable pytest files changed.") + lines.extend( + [ + "", + "This guidance does not infer tests from source changes. " + "Existing blocking CI remains the source of truth.", + ] + ) + return "\n".join(lines) + + +def _parse_args(argv: list[str]) -> argparse.Namespace: + parser = argparse.ArgumentParser( + description="Report focused pytest guidance for changed tests/ paths.", + ) + parser.add_argument("--base-sha", help="Pull request base commit SHA.") + parser.add_argument("--head-sha", help="Pull request head commit SHA.") + return parser.parse_args(argv) + + +def main(argv: list[str] | None = None) -> int: + args = _parse_args(sys.argv[1:] if argv is None else argv) + if bool(args.base_sha) != bool(args.head_sha): + raise SystemExit("--base-sha and --head-sha must be provided together") + + if args.base_sha and args.head_sha: + paths = changed_paths_from_merge_base(args.base_sha, args.head_sha) + else: + paths = parse_paths(sys.stdin.buffer.read()) + + print(format_report(paths)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 787bd9dea..f7d3659e8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,6 +15,60 @@ concurrency: cancel-in-progress: true jobs: + focused-test-guidance: + name: Focused test guidance (report-only) + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + continue-on-error: true + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + fetch-depth: 0 + persist-credentials: false + - name: Report changed test paths + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + report_file="$RUNNER_TEMP/focused-test-guidance.md" + publish_report() { + cat "$report_file" + if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then + cat "$report_file" >> "$GITHUB_STEP_SUMMARY" || true + fi + return 0 + } + + report_unavailable() { + { + printf '%s\n\n' '## Focused test guidance unavailable (report-only)' + printf '%s\n\n' "$1" + printf '%s\n' 'Existing blocking CI remains the source of truth.' + } > "$report_file" + publish_report + exit 0 + } + + if [ -z "$BASE_SHA" ] || [ -z "$HEAD_SHA" ]; then + report_unavailable "Pull request base/head metadata is missing." + fi + + if ! git cat-file -e "${BASE_SHA}^{commit}" 2>/dev/null; then + report_unavailable "The pull request base commit is unavailable locally." + fi + + if ! git cat-file -e "${HEAD_SHA}^{commit}" 2>/dev/null; then + report_unavailable "The pull request head commit is unavailable locally." + fi + + if ! python3 .github/scripts/focused_test_guidance.py \ + --base-sha "$BASE_SHA" \ + --head-sha "$HEAD_SHA" > "$report_file"; then + report_unavailable "The focused test guidance helper could not produce a report." + fi + + publish_report + python-syntax: name: Python syntax (compileall) runs-on: ubuntu-latest diff --git a/ACKNOWLEDGMENTS.md b/ACKNOWLEDGMENTS.md index fdf55c48a..94092c6ca 100644 --- a/ACKNOWLEDGMENTS.md +++ b/ACKNOWLEDGMENTS.md @@ -86,6 +86,7 @@ Bundled in `static/fonts/`: | [Fira Code](https://github.com/tonsky/FiraCode) | SIL Open Font License 1.1 | Nikita Prokopov & contributors | | [Inter](https://github.com/rsms/inter) | SIL Open Font License 1.1 | Rasmus Andersson | | [GohuFont](https://font.gohu.org/) (`fonts/custom/GohuFont.ttf`) | WTFPL | Hugo Chargois | +| [OpenDyslexic](https://opendyslexic.org/) (`fonts/OpenDyslexic-{Regular,Bold}.woff2`) | SIL Open Font License 1.1 ([`licenses/OpenDyslexic-OFL.txt`](licenses/OpenDyslexic-OFL.txt)) | Abbie Gonzalez | ## Python dependencies diff --git a/Dockerfile b/Dockerfile index bed5e2002..9b305569c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,3 +1,14 @@ +# ---- builder: patch + build wheels for Real-ESRGAN's broken-on-3.14 deps ---- +# basicsr/gfpgan/facexlib read their version via exec()+locals()['__version__'], +# which raises KeyError on Python 3.13+ (PEP 667). Build patched wheels here so +# the final image / Cookbook never has to compile the broken sdists. See +# docker/build-realesrgan-wheels.sh for the full rationale. +FROM python:3.14-slim AS realesrgan-wheels +RUN apt-get update && apt-get install -y --no-install-recommends curl \ + && rm -rf /var/lib/apt/lists/* +COPY docker/build-realesrgan-wheels.sh /usr/local/bin/build-realesrgan-wheels.sh +RUN bash /usr/local/bin/build-realesrgan-wheels.sh /wheels + FROM python:3.14-slim # System deps. tmux is required by Cookbook for background downloads/serves. @@ -18,8 +29,27 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ tmux \ openssh-client \ gosu \ + libgl1 \ + libglib2.0-0t64 \ + libxcb1 \ + libmagic1 \ && rm -rf /var/lib/apt/lists/* +# libgl1/libglib2.0-0t64/libxcb1 are runtime shared libs (libGL.so.1, +# libglib-2.0/libgthread, libxcb.so.1) that opencv-python (cv2) loads. The +# slim base omits them, so the Cookbook "install realesrgan" path imports cv2 +# and dies with `libxcb.so.1: cannot open shared object file` despite a clean +# pip install. Using full opencv-python (not -headless) because basicsr/gfpgan/ +# facexlib/realesrgan all depend on the `opencv-python` distribution by name. +# +# libmagic1 is the shared lib (libmagic.so.1) that python-magic dlopens for +# content-based MIME sniffing in src/upload_handler.py. We install both here +# (libmagic1 + the python-magic wrapper, below) rather than in requirements.txt +# because python-magic resolves libmagic at import time: where the lib is +# absent the import can block or raise, so keeping it image-only avoids +# regressing pip/venv installs on hosts without libmagic. Debian always has the +# lib here, so the import is instant and detection actually works. + # Docker CLI (client only — daemon stays on the host via the # /var/run/docker.sock mount). The Debian `docker.io` package ships # dockerd but not the client binary on slim, so grab the static client @@ -46,6 +76,20 @@ COPY requirements.txt requirements-optional.txt ./ RUN pip install --no-cache-dir -r requirements.txt \ && if [ "$INSTALL_OPTIONAL" = "true" ]; then pip install --no-cache-dir -r requirements-optional.txt; fi +# python-magic powers content-based MIME sniffing in src/upload_handler.py. +# Image-only (not in requirements.txt) because it needs the libmagic1 system +# lib installed above; see the apt note near the top of this stage. +RUN pip install --no-cache-dir python-magic==0.4.27 + +# Pre-install the patched basicsr/gfpgan/facexlib wheels built in the +# realesrgan-wheels stage (--no-deps keeps the image lean — torch & friends are +# pulled only when realesrgan is actually installed). With these dists already +# satisfied, the Cookbook's plain `pip install realesrgan` resolves them from +# wheels instead of rebuilding the sdists that fail on Python 3.14. +COPY --from=realesrgan-wheels /wheels/ /tmp/odysseus-wheels/ +RUN pip install --no-cache-dir --no-deps /tmp/odysseus-wheels/*.whl \ + && rm -rf /tmp/odysseus-wheels + # Copy app code COPY . . diff --git a/ROADMAP.md b/ROADMAP.md index 7c59c1f6a..d29ac5c75 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -12,7 +12,6 @@ the codebase, you are probably right to stay away. and WSL all need coverage. - Integration audit: do integrations even work? Confirm what works, what needs setup docs, and what should be removed or hidden. -- Self-host troubleshooting cookbook. Document the weird 30-second fixes that otherwise become 30-minute searches: Dovecot cleartext auth for local stacks, ntfy Android Instant Delivery for non-ntfy.sh servers, clipboard limits on plain-HTTP Tailscale URLs, Radicale collection URLs, and similar traps. - Cookbook reliability on other computers. This is probably the area most likely to need work across different machines, GPUs, drivers, shells, and Python environments. - Cookbook SGLang support across platforms. Make sure SGLang setup/serve works predictably on Linux, Windows/WSL, macOS where possible, Docker, and common diff --git a/app.py b/app.py index 57d091efd..38c848e0b 100644 --- a/app.py +++ b/app.py @@ -2,6 +2,17 @@ import mimetypes import os import sys +import asyncio +import time + +# On Windows, asyncio.create_subprocess_exec/shell require the ProactorEventLoop. +# When started via `python -m uvicorn` from a terminal, uvicorn sets this +# automatically. But the VS Code debugger (and other non-uvicorn entrypoints) +# use the default SelectorEventLoop, which raises NotImplementedError on any +# subprocess call. Force ProactorEventLoop here so the right loop is always +# used, regardless of how the process is launched. +if sys.platform == "win32": + asyncio.set_event_loop_policy(asyncio.WindowsProactorEventLoopPolicy()) def register_static_mime_types() -> None: @@ -44,7 +55,7 @@ from typing import Dict from contextlib import asynccontextmanager from fastapi import FastAPI, Request, HTTPException -from fastapi.responses import JSONResponse, FileResponse, HTMLResponse +from fastapi.responses import JSONResponse, FileResponse from fastapi.middleware.cors import CORSMiddleware from fastapi.staticfiles import StaticFiles from starlette.middleware.base import BaseHTTPMiddleware @@ -65,7 +76,7 @@ from core.exceptions import ( import bcrypt as _bcrypt -from src.app_helpers import abs_join +from src.app_helpers import abs_join, serve_html_with_nonce from src.generated_images import GENERATED_IMAGE_HEADERS, resolve_generated_image_path from starlette.responses import RedirectResponse @@ -187,7 +198,50 @@ class _RequestTimeoutMiddleware(_BaseHTTPMiddleware): ) +class _InteractiveActivityMiddleware(_BaseHTTPMiddleware): + async def dispatch(self, request, call_next): + from src.interactive_gate import should_track_interactive_request, track_interactive_request + + path = request.url.path or "" + if not should_track_interactive_request(path, request.method): + return await call_next(request) + async def _stop_background(): + try: + await task_scheduler.stop_background_tasks_for_foreground(reason=f"foreground request {request.method} {path}") + except Exception: + logging.getLogger("app.foreground_gate").debug("foreground task stop failed", exc_info=True) + asyncio.create_task(_stop_background()) + async with track_interactive_request(path, request.method): + return await call_next(request) + + +class _SlowRequestLogMiddleware(_BaseHTTPMiddleware): + async def dispatch(self, request, call_next): + start = time.perf_counter() + status = 500 + try: + response = await call_next(request) + status = getattr(response, "status_code", 0) or 0 + return response + finally: + elapsed = time.perf_counter() - start + try: + threshold = float(os.getenv("ODYSSEUS_SLOW_REQUEST_LOG_SECONDS", "0.75") or "0.75") + except Exception: + threshold = 0.75 + if elapsed >= threshold: + logging.getLogger("app.slow_request").warning( + "slow_request method=%s path=%s status=%s elapsed=%.3fs", + request.method, + request.url.path, + status, + elapsed, + ) + + app.add_middleware(_RequestTimeoutMiddleware) +app.add_middleware(_InteractiveActivityMiddleware) +app.add_middleware(_SlowRequestLogMiddleware) # ========= AUTH ========= from routes.auth_routes import setup_auth_routes, SESSION_COOKIE @@ -573,6 +627,20 @@ webhook_manager = WebhookManager(api_key_manager=api_key_manager) auth_router = setup_auth_routes(auth_manager) app.include_router(auth_router) + +@app.post("/api/activity/heartbeat") +async def activity_heartbeat(): + from src.interactive_gate import mark_browser_activity + await mark_browser_activity() + async def _stop_background(): + try: + await task_scheduler.stop_background_tasks_for_foreground(reason="browser heartbeat") + except Exception: + logging.getLogger("app.foreground_gate").debug("heartbeat task stop failed", exc_info=True) + asyncio.create_task(_stop_background()) + return {"ok": True} + + # Uploads from routes.upload_routes import setup_upload_routes upload_router, upload_cleanup_func = setup_upload_routes(upload_handler) @@ -587,14 +655,19 @@ app.include_router(setup_emoji_routes()) # Sessions from routes.session_routes import setup_session_routes session_config = {"REQUEST_TIMEOUT": REQUEST_TIMEOUT, "OPENAI_API_KEY": OPENAI_API_KEY, "SESSIONS_FILE": SESSIONS_FILE} -app.include_router(setup_session_routes(session_manager, session_config, webhook_manager=webhook_manager)) +app.include_router(setup_session_routes( + session_manager, + session_config, + webhook_manager=webhook_manager, + upload_handler=upload_handler, +)) # Admin Danger Zone wipes (Settings → System → Danger Zone) from routes.admin_wipe_routes import setup_admin_wipe_routes app.include_router(setup_admin_wipe_routes(session_manager)) # Memory -from routes.memory_routes import setup_memory_routes +from routes.memory.memory_routes import setup_memory_routes memory_router = setup_memory_routes(memory_manager, session_manager, memory_vector=memory_vector) app.include_router(memory_router) from routes.skills_routes import setup_skills_routes @@ -611,12 +684,12 @@ app.include_router(setup_chat_routes( )) # Research (background deep-research tasks) -from routes.research_routes import setup_research_routes +from routes.research.research_routes import setup_research_routes app.include_router(setup_research_routes(research_handler, session_manager=session_manager)) # History -from routes.history_routes import setup_history_routes -app.include_router(setup_history_routes(session_manager)) +from routes.history.history_routes import setup_history_routes +app.include_router(setup_history_routes(session_manager, upload_handler=upload_handler)) # Search from routes.search_routes import setup_search_routes @@ -675,7 +748,7 @@ from routes.signature_routes import setup_signature_routes app.include_router(setup_signature_routes()) # Gallery (image library) -from routes.gallery_routes import setup_gallery_routes +from routes.gallery.gallery_routes import setup_gallery_routes app.include_router(setup_gallery_routes()) # Persisted image-editor drafts (server-backed projects) @@ -695,7 +768,7 @@ app.include_router(setup_assistant_routes(task_scheduler)) # Calendar (CalDAV) from routes.calendar_routes import setup_calendar_routes -calendar_router = setup_calendar_routes() +calendar_router = setup_calendar_routes(upload_handler=upload_handler) app.include_router(calendar_router) # Shell (user-facing command execution) @@ -758,7 +831,7 @@ logger.info("Webhook & API token routes initialized") # Notes (Google Keep-style notes/todos) from routes.note_routes import setup_note_routes -app.include_router(setup_note_routes(task_scheduler)) +app.include_router(setup_note_routes(task_scheduler, upload_handler=upload_handler)) # Email from routes.email_routes import setup_email_routes @@ -783,7 +856,7 @@ from routes.vault_routes import setup_vault_routes app.include_router(setup_vault_routes()) # Contacts (CardDAV) -from routes.contacts_routes import setup_contacts_routes +from routes.contacts.contacts_routes import setup_contacts_routes app.include_router(setup_contacts_routes()) from companion import setup_companion_routes @@ -791,23 +864,17 @@ app.include_router(setup_companion_routes()) # ========= ROUTES (kept in app.py) ========= -def _serve_html_with_nonce(request: Request, file_path: str) -> HTMLResponse: - """Read an HTML file and inject the CSP nonce into inline - - + + @@ -581,6 +587,7 @@ +
@@ -591,6 +598,13 @@
+
+ + +
-
-

Writing Style

-
AI-extracted from your sent emails. Used when AI drafts replies.
-
- -
- - - -
-
-
@@ -2046,6 +2061,16 @@ +
+

Model Defaults

+
+
+
Share defaults with users
+
When on, users without a personal default inherit the global default model (only if those models are allowed for them).
+
+ +
+

Users

Loading...
@@ -2069,7 +2094,7 @@
-

Add Local Models (Endpoint) +

Add Local Models (Endpoint)

@@ -2446,10 +2471,10 @@ - + - + @@ -2459,7 +2484,7 @@ - + diff --git a/static/js/MODULE_SUMMARY.md b/static/js/MODULE_SUMMARY.md index 0e847423f..df5b0cb33 100644 --- a/static/js/MODULE_SUMMARY.md +++ b/static/js/MODULE_SUMMARY.md @@ -1,124 +1,228 @@ -# Module Organization Summary +# Frontend Module Organization Summary -## Purpose -This document describes what each JavaScript module is responsible for. - -> **Note:** This file is a partial, historical overview — not a complete authoritative -> inventory. The authoritative module set is the current `static/js/` tree plus the -> scripts loaded by `static/index.html`. As of this writing that tree holds **65 `.js` -> files** across **8 subdirectories** (`calendar/`, `color/`, `compare/`, `editor/`, -> `emailLibrary/`, `markdown/`, `research/`, `util/`), and `static/index.html` loads -> **35** `/static…` script tags. The catalog below covers only the original core -> modules and is not kept in sync with every module. +> **Scope:** This document describes the architecture of the Odysseus no-build +> frontend. The app is a collection of native ES6 modules loaded from +> `static/`. The authoritative source is the current `static/js/` tree and the +> top-level orchestrator `static/app.js`. --- -## Core Modules (in static/js/) +## 1. Top-level Application Orchestrator -### 1. **ui.js** -- UI helper functions and utilities -- Toast notifications (`showToast`, `showError`) -- Element getter (`el()`) -- Clipboard operations (`copyToClipboard`) -- Scroll management (`scrollHistory`, `setAutoScroll`) -- Auto-resize textarea -- Debounce utility +### `static/app.js` +*Main application entry point.* -### 2. **markdown.js** -- Markdown processing and rendering -- Convert markdown to HTML (`mdToHtml`) -- Code block handling with syntax highlighting -- Content rendering for message arrays -- Text cleanup (`squashOutsideCode`) +- Imports all feature modules. +- Exposes a few modules on `window` for legacy inter-module reachability + (`themeModule`, `sessionModule`, `uiModule`, `adminModule`, `cookbookModule`). +- Patches `fetch` so any `401` redirects the user to `/login`. +- Fetches the default chat configuration and handles deep-link route openers + (`/notes`, `/calendar`, `/email`, `/memory`, `/gallery`, `/cookbook`, `/library`, `/tasks`). +- Wires global event listeners: chat-history scrolling, popups, Escape handling, + drag-and-drop/paste attachment handling, transcription export, sidebar toggles, + rail/tool buttons, and session sorting. +- Loads auth status and applies per-user privilege restrictions. -### 3. **sessions.js** -- Session/chat management -- Create, load, delete, switch sessions -- Session history loading -- Direct chat creation with models -- Session renaming - -### 4. **memory.js** -- AI memory management -- Load, add, edit, delete memories -- Memory search/filtering -- Memory UI rendering -- Memory count updates - -### 5. **fileHandler.js** -- File attachment handling -- File picker dialog -- File upload to server -- Attachment strip rendering -- Pending files management -- File preview/removal - -### 6. **voiceRecorder.js** -- Voice recording functionality -- Start/stop recording -- Audio file creation -- Microphone permission handling -- Recording UI updates - -### 7. **models.js** -- Model scanning and display -- Local model discovery (ports 8000-8020) -- Provider management (OpenAI) -- Model selection UI - -### 8. **rag.js** -- RAG (Retrieval Augmented Generation) management -- Load personal documents -- Add directories to RAG -- Display included files/directories - -### 9. **presets.js** -- Conversation preset management -- Load, save, activate presets -- Custom preset configuration -- Temperature, tokens, system prompt settings - -### 10. **search.js** -- Web search settings -- Provider selection (DuckDuckGo, Brave, SearXNG) -- API key management -- Save/load search configuration - -### 11. **chat.js** ⭐ (The Big One) -- Main chat functionality -- Message handling (`addMessage`) -- Chat submission (`handleChatSubmit`) -- Streaming response handling -- Performance metrics display -- Abort request management -- Loading states and error handling +### `static/index.html` +*SPA shell.* Loads `app.js` as a module, includes the theme-aware inline script, +and defines the DOM skeleton that the modules populate (chat history, composer, +sidebar, icon rail, modals). --- -## Main Application File +## 2. Core Foundation Modules -### **app.js** -- Application initialization -- Event listener setup -- Drag & drop handlers -- Keyboard shortcuts -- Module initialization -- Global configuration (API_BASE) -- Coordinates all modules together +These are imported first and used across most features. + +| Module | Primary Exports | Responsibility | +|---|---|---| +| **`ui.js`** | `showToast`, `showError`, `el`, `copyToClipboard`, `scrollHistory`, `setAutoScroll`, `autoResize`, `debounce`, `esc` | Shared UI helpers, toast notifications, scroll behavior, element accessor, text escaping. | +| **`storage.js`** | `default` storage wrapper | LocalStorage helpers and toggle state persistence. | +| **`markdown.js`** | `mdToHtml`, `processWithThinking`, `squashOutsideCode`, `normalizeThinkingMarkup`, `extractThinkingBlocks`, `hasUnclosedThinkTag`, `startsWithReasoningPrefix` | Markdown→HTML, thinking/reasoning block parsing, code-block normalization. | +| **`spinner.js`** | `create`, `createWhirlpool` | Loading/spinner factories for streaming and tool cards. | +| **`keyboard-shortcuts.js`** | `initKeyboardShortcuts` | Global keyboard shortcut wiring. | +| **`sidebar-layout.js`** | `initSidebarLayout`, `syncRailSide` | Wide sidebar ↔ icon-rail layout behavior. | +| **`section-management.js`** | `initSectionCollapse`, `initSectionDrag` | Collapsible/draggable sidebar sections. | +| **`modalManager.js`** | side-effect import | Unified minimize/restore behavior for floating tool modals. | +| **`tileManager.js`** | side-effect import | Desktop window tiling and snap-to-edge behavior. | +| **`windowDrag.js`** | `makeWindowDraggable` | Drag support for floating panels. | +| **`modalSnap.js`**, **`toolWindowZOrder.js`**, **`windowResize.js`** | — | Modal snapping, z-index management, resize handles. | --- -## Dependency Order (Load Order in HTML) -```html - - - - - - - - - - - - +## 3. Chat Pipeline + +The largest and most central subsystem. Chat submission → backend SSE → progressive rendering of text, tools, research, documents, and UI events. + +| Module | Responsibility | +|---|---| +| **`chat.js`** | Main chat controller. Handles `handleChatSubmit`, stops/continues, builds `FormData`, posts to `/api/chat_stream`, reads the SSE stream, and dispatches each JSON event to the appropriate renderer. Tracks background streams, stalls, auto-recovery, and multi-round agent state. | +| **`chatStream.js`** | Helpers shared between streaming consumers: browser notifications, background-stream completion toasts, and `ui_control` event handling. | +| **`chatRenderer.js`** | Message DOM construction: `addMessage`, role labels, model route labels, color coding, footers, metrics, code blocks, sources boxes (`web`/`research`/`RAG`), findings box, images, report links, ask-user cards, welcome screen, and transcript utilities. | +| **`streamingRenderer.js`** | Incremental streaming renderer used by `chat.js`. Freezes finalized DOM blocks and only re-renders the growing tail to avoid flicker and O(N²) re-parsing. | +| **`streamingSegmenter.js`** | Splits a token stream into display units (text vs code fences) for `streamingRenderer.js`. | +| **`slashCommands.js`** | Slash-command registry (`/help`, `/setup`, etc.), parsing, and dispatch handlers. Exported functions are consumed by `chat.js` and `slashAutocomplete.js`. | +| **`slashAutocomplete.js`** | Composer autocomplete popup for `/` commands. | +| **`composerArrowUpRecall.js`** | Recall last user message with `↑` on an empty composer. | +| **`assistant.js`** | Assistant/persona behaviors and message styling helpers. | +| **`tts-ai.js`** | AI text-to-speech manager, enqueueing, streaming TTS, and playback button injection. | +| **`voiceRecorder.js`** | Voice recording from the composer microphone. | +| **`fileHandler.js`** | Attachment picker, paste/drop handling, upload, attachment strip rendering, pending-file management. | +| **`codeRunner.js`** | Client-side execution affordances for code blocks returned by the model. | + +--- + +## 4. Model, Endpoint, and Configuration Modules + +| Module | Responsibility | +|---|---| +| **`models.js`** | Model discovery / scanning, local model port probing, provider management, model selection UI state. | +| **`modelPicker.js`** | Composer model-picker dropdown and endpoint selection. | +| **`modelSort.js`** | Sorting helpers for model lists. | +| **`model/matchKey.js`** | Model-to-key matching helper. | +| **`providers.js`** | Provider metadata and account-management helpers. | +| **`providerDeviceFlow.js`** | OAuth device-flow support for providers. | +| **`presets.js`** | Character/preset selection, custom preset saving, inject prefix/suffix handling. | +| **`search.js`** | Web-search settings, provider selection, API key management. | +| **`settings.js`** | Settings panel (models, search, appearance, users, MCP, RAG, embedding, tokens). | +| **`admin.js`** | Admin panel and privileged user/endpoint configuration. | +| **`theme.js`** | Theme presets, custom colors, fonts, backgrounds, live theme switching. | + +--- + +## 5. Session, Sidebar, and Workspace + +| Module | Responsibility | +|---|---| +| **`sessions.js`** | Chat session list loading, creation, switching, renaming, archiving, library modal, and direct-chat creation. Tracks current session, streaming/research indicators in the sidebar. | +| **`workspace.js`** | Workspace folder path management for shell/file tool confinement. | +| **`search-chat.js`** | In-chat history search. | +| **`skills.js`** | Client-side skill library UI (load, edit, delete, test, and audit status display). | + +--- + +## 6. Knowledge, Memory, and RAG + +| Module | Responsibility | +|---|---| +| **`memory.js`** | AI memory CRUD, search/filter UI, memory extraction, count badge. | +| **`rag.js`** | Personal document RAG: load documents, add directories/files, show included paths. | +| **`group.js`** | Group-chat UI and model orchestration. | + +--- + +## 7. Document and Editor Subsystems + +| Module | Responsibility | +|---|---| +| **`document.js`** | Tabbed document editor, AI edit suggestions, Markdown/HTML/CSV editing, document streaming (`streamDocOpen`/`streamDocDelta`), and panel state. | +| **`documentLibrary.js`** | Document library modal. | +| **`editor/`** | Gallery image editor canvas modules: layers, brush, inpaint, crop, filters, state, history panel, top-bar wiring, canvas coordinate helpers, and AI model runners for inpainting/background-removal. | + +--- + +## 8. Research UI + +| Module | Responsibility | +|---|---| +| **`research/panel.js`** | Research panel UI, job list, and controls. | +| **`research/jobs.js`** | Research job polling and status rendering. | +| **`researchSynapse.js`** | Animated research-progress visualization shown inside the chat bubble during a research run. | + +--- + +## 9. Gallery, Email, Calendar, Tasks, and Notes + +| Module | Responsibility | +|---|---| +| **`gallery.js`** / **`galleryEditor.js`** | Gallery/image library and canvas editor entry points. | +| **`emailInbox.js`** / **`emailLibrary.js`** | Email inbox reader and library modal. Sub-modules handle signatures, reply recipients, state, and signature folding. | +| **`calendar.js`** / **`calendar/utils.js`** / **`calendar/reminders.js`** | Calendar views, event forms, reminders. | +| **`tasks.js`** | Scheduled task/recurring LLM job UI. | +| **`notes.js`** | Notes and todo panel, reminders, pinboard. | + +--- + +## 10. Cookbook (Model Serving) + +| Module | Responsibility | +|---|---| +| **`cookbook.js`** | Cookbook main UI: hardware fitting, presets, action panels. | +| **`cookbook-hwfit.js`** / **`cookbook-diagnosis.js`** / **`cookbook-deps-recipes.js`** | Hardware-fit scoring, dependency diagnosis, recipe handling. | +| **`cookbookDownload.js`** / **`cookbookServe.js`** / **`cookbookRunning.js`** / **`cookbookSchedule.js`** / **`cookbookPorts.js`** / **`cookbookProgressSignal.js`** | Model download/serve flow, running job cards, scheduling, port detection, and progress computation. | + +--- + +## 11. Compare and Utility Modules + +| Module | Responsibility | +|---|---| +| **`compare/index.js`** (with `compare/state.js`, `compare/stream.js`, `compare/panes.js`, `compare/selector.js`, `compare/scoreboard.js`, `compare/probe.js`, `compare/vote.js`, `compare/icons.js`) | Model compare mode: parallel streams, panes, scoring, vote UI. | +| **`censor.js`** | Text/image censor overlay toggles. | +| **`a11y.js`** | Accessibility helpers. | +| **`platform.js`** | Platform detection (macOS/Windows/Linux) and keyboard-modifier helpers. | +| **`escMenuStack.js`** | Stack manager for dismissible popups. | +| **`dragSort.js`** | Drag-to-sort shared behavior. | +| **`tourHints.js`** / **`tourAutoplay.js`** | Onboarding tour helpers. | +| **`color/hex.js`**, **`colorPicker.js`**, **`langIcons.js`**, **`util/ordinal.js`** | Small utility modules for color, language icons, and formatting. | + +--- + +## 12. Frontend Event Streaming Flow + +``` +User submits composer + └── chat.js::handleChatSubmit() builds FormData + ├── fileHandler.uploadPending() for attachments + ├── document.js saved (if a document panel is open) + └── POST /api/chat_stream + +Server responds with SSE stream + └── chat.js reads chunks via res.body.getReader() + TextDecoder + ├── Lines starting with "event:" set next-error state + └── Lines starting with "data:" carry JSON payloads + +JSON events are dispatched by "type": + delta → streamingRenderer → markdown → live reply text + agent_prep → update spinner label + tool_start → finalize text bubble; create agent-thread node with wave animation + tool_progress → append/update live stdout/stderr tail + tool_output → mark node done/failed, render output, diffs, screenshots + agent_step → finalize tool thread; create new msg-continuation bubble + doc_stream_open → document.js opens a live document + doc_stream_delta → document.js appends content to that document + research_progress → researchSynapse visualization + spinner timer + research_sources → build sources box for research + research_done → reload session history to show the report + web_sources → build web-search sources box + model_info → update role header with requested/actual model + fallback → show fallback model toast + update role label + metrics → collect/display token/cost metrics + message_saved → store database id on the message element + budget_exceeded → show budget banner + rounds_exhausted → show Continue button for step-limit hits + teacher_takeover → insert escalation banner, reset round state + skill_saved → show skill-learned banner +``` + +Foreground vs background streams: +- If the user switches sessions while a stream is running, `chat.js` pauses DOM + updates and stores the state in `_backgroundStreams`. Completion is signaled + with a sidebar dot/notifications, and the history is reloaded when the user + returns. + +--- + +## 13. What Changed from the Previous Summary + +- The frontend is now exclusively ES6-module based; the old `', encoding="utf-8") + resp = serve_html_with_nonce(_request_with_nonce("nonce-abc"), str(page)) + assert resp.status_code == 200 + body = resp.body.decode("utf-8") + assert "nonce-abc" in body + assert "{{CSP_NONCE}}" not in body diff --git a/tests/test_serve_profiles.py b/tests/test_serve_profiles.py index e612a7a83..cc7e3788e 100644 --- a/tests/test_serve_profiles.py +++ b/tests/test_serve_profiles.py @@ -28,6 +28,12 @@ def _sys(vram, family="rdna"): return {"backend": "rocm", "gpu_vram_gb": vram, "gpu_family": family} +def test_compute_serve_profiles_ignores_invalid_inputs(): + assert compute_serve_profiles(None, _DENSE_8B) == [] + assert compute_serve_profiles(_sys(8), None) == [] + assert compute_serve_profiles(["bad"], _DENSE_8B) == [] + + def test_big_moe_on_small_card_offloads_not_fails(): """A 35B MoE can't hold its weights on 16 GB, so the Quality profile must offload experts to CPU (n_cpu_moe > 0) rather than be dropped.""" diff --git a/tests/test_service_health.py b/tests/test_service_health.py deleted file mode 100644 index 56283cef8..000000000 --- a/tests/test_service_health.py +++ /dev/null @@ -1,472 +0,0 @@ -"""Tests for src.service_health — the consolidated degraded-state report. - -Imports the real module (conftest.py stubs the heavy deps). Network is never -touched: HTTP probes take an injected `http_get`, and the email/provider probes -take an injected `connect` / `probe`. Asserts the ok/degraded/down/disabled -mapping per subsystem, the overall rollup, and that no secrets leak into meta. -""" -import types - -import pytest - -from src import service_health as sh - - -def _resp(status_code): - return types.SimpleNamespace(status_code=status_code) - - -def _raise(*_a, **_k): - raise RuntimeError("connection refused") - - -# ── chromadb_health ── - -class _Store: - def __init__(self, healthy): - self.healthy = healthy - - -def test_chromadb_both_healthy_ok(): - s = sh.chromadb_health(_Store(True), _Store(True)) - assert s["status"] == sh.OK - assert s["meta"] == {"rag": True, "memory": True} - - -def test_chromadb_one_down_degraded(): - s = sh.chromadb_health(_Store(True), _Store(False)) - assert s["status"] == sh.DEGRADED - - -def test_chromadb_both_unhealthy_down(): - s = sh.chromadb_health(_Store(False), _Store(False)) - assert s["status"] == sh.DOWN - - -def test_chromadb_both_absent_disabled(): - s = sh.chromadb_health(None, None) - assert s["status"] == sh.DISABLED - - -def test_chromadb_one_absent_one_healthy_ok(): - # An absent store is not a failure; the present one being healthy is ok. - s = sh.chromadb_health(_Store(True), None) - assert s["status"] == sh.OK - assert s["meta"]["memory"] is None - - -# ── searxng_health ── - -def test_searxng_disabled_when_other_provider(): - s = sh.searxng_health({"search_provider": "brave"}) - assert s["status"] == sh.DISABLED - - -def test_searxng_ok_on_healthz(): - s = sh.searxng_health( - {"search_provider": "searxng", "search_url": "http://sx:8080"}, - http_get=lambda url, timeout: _resp(200), - ) - assert s["status"] == sh.OK - assert s["meta"]["probed"] == "/healthz" - - -def test_searxng_ok_on_root_fallback(): - def getter(url, timeout): - return _resp(404) if url.endswith("/healthz") else _resp(200) - - s = sh.searxng_health( - {"search_provider": "searxng", "search_url": "http://sx:8080"}, - http_get=getter, - ) - assert s["status"] == sh.OK - assert s["meta"]["probed"] == "/" - - -def test_searxng_down_on_exception(): - s = sh.searxng_health( - {"search_provider": "searxng", "search_url": "http://sx:8080"}, - http_get=_raise, - ) - assert s["status"] == sh.DOWN - - -def test_searxng_down_on_5xx(): - s = sh.searxng_health( - {"search_provider": "searxng", "search_url": "http://sx:8080"}, - http_get=lambda url, timeout: _resp(502), - ) - assert s["status"] == sh.DOWN - - -# ── ntfy_health ── - -def _ntfy_intg(): - return [{"preset": "ntfy", "enabled": True, "base_url": "http://ntfy:80"}] - - -def test_ntfy_disabled_without_integration(): - s = sh.ntfy_health([], {"reminder_channel": "ntfy"}) - assert s["status"] == sh.DISABLED - - -def test_ntfy_ok(): - s = sh.ntfy_health(_ntfy_intg(), {"reminder_channel": "ntfy"}, - http_get=lambda url, timeout: _resp(200)) - assert s["status"] == sh.OK - assert s["meta"]["base"] == "http://ntfy:80" - - -def test_ntfy_probes_v1_health_not_a_topic(): - seen = {} - - def getter(url, timeout): - seen["url"] = url - return _resp(200) - - sh.ntfy_health(_ntfy_intg(), {"reminder_channel": "ntfy"}, http_get=getter) - # Non-intrusive: hits /v1/health, never publishes to a topic. - assert seen["url"].endswith("/v1/health") - - -def test_ntfy_down_on_exception(): - s = sh.ntfy_health(_ntfy_intg(), {"reminder_channel": "ntfy"}, - http_get=_raise) - assert s["status"] == sh.DOWN - - -# ── email_health ── - -def _acct(name, host="imap.example.com"): - return {"account_id": name, "account_name": name, "imap_host": host, - "imap_password": "hunter2"} - - -class _Conn: - def logout(self): - pass - - -def test_email_disabled_without_accounts(): - assert sh.email_health([])["status"] == sh.DISABLED - - -def test_email_ok_all_connect(): - s = sh.email_health([_acct("a"), _acct("b")], connect=lambda _id: _Conn()) - assert s["status"] == sh.OK - - -def test_email_degraded_some_fail(): - def connect(account_id): - if account_id == "bad": - raise RuntimeError("auth failed") - return _Conn() - - s = sh.email_health([_acct("good"), _acct("bad")], connect=connect) - assert s["status"] == sh.DEGRADED - - -def test_email_down_all_fail(): - s = sh.email_health([_acct("a")], connect=_raise) - assert s["status"] == sh.DOWN - - -def test_email_account_without_host_marked_failed(): - s = sh.email_health([_acct("a", host="")], connect=lambda _id: _Conn()) - assert s["status"] == sh.DOWN - - -def test_email_meta_never_leaks_password(): - s = sh.email_health([_acct("a")], connect=lambda _id: _Conn()) - assert "hunter2" not in repr(s) - - -# ── providers_health ── - -def _ep(name): - return {"name": name, "base_url": f"http://{name}:8000/v1", "api_key": "sk-secret"} - - -def test_providers_disabled_without_endpoints(): - assert sh.providers_health([])["status"] == sh.DISABLED - - -def test_providers_ok_all_reachable(): - s = sh.providers_health([_ep("a")], - probe=lambda base, key, timeout: ["m1", "m2"]) - assert s["status"] == sh.OK - assert s["meta"]["endpoints"][0]["model_count"] == 2 - - -def test_providers_degraded_some_empty(): - def probe(base, key, timeout): - return ["m1"] if "good" in base else [] - - s = sh.providers_health([_ep("good"), _ep("bad")], probe=probe) - assert s["status"] == sh.DEGRADED - - -def test_providers_down_all_fail(): - s = sh.providers_health([_ep("a")], probe=_raise) - assert s["status"] == sh.DOWN - - -def test_providers_meta_never_leaks_api_key(): - s = sh.providers_health([_ep("a")], - probe=lambda base, key, timeout: ["m1"]) - assert "sk-secret" not in repr(s) - - -# ── rollup ── - -def test_rollup_picks_worst_non_disabled(): - services = [ - {"status": sh.OK}, {"status": sh.DISABLED}, - {"status": sh.DEGRADED}, {"status": sh.OK}, - ] - assert sh._rollup(services) == sh.DEGRADED - - -def test_rollup_down_beats_degraded(): - assert sh._rollup([{"status": sh.DEGRADED}, {"status": sh.DOWN}]) == sh.DOWN - - -def test_rollup_all_disabled_is_ok(): - assert sh._rollup([{"status": sh.DISABLED}, {"status": sh.DISABLED}]) == sh.OK - - -# ── collect_service_health (async aggregate) ── - -def test_collect_service_health_shape(monkeypatch): - import asyncio - - # Avoid touching real data sources / network. - monkeypatch.setattr(sh, "_gather_inputs", lambda: { - "settings": {"search_provider": "disabled"}, - "integrations": [], - "accounts": [], - "endpoints": [], - }) - out = asyncio.run(sh.collect_service_health(_Store(True), _Store(True))) - assert set(out) == {"overall", "services", "timestamp"} - names = {s["name"] for s in out["services"]} - assert names == {"chromadb", "searxng", "ntfy", "email", "providers"} - # Chroma healthy, everything else disabled → overall ok. - assert out["overall"] == sh.OK - - -# ── _safe_url: strip userinfo / query / fragment ── - -@pytest.mark.parametrize("raw,expected", [ - ("http://user:pass@host:8080/path?api_key=secret#frag", "http://host:8080/path"), - ("https://admin:hunter2@searx.example.com/", "https://searx.example.com"), - ("http://ntfy.local:80?token=abc", "http://ntfy.local:80"), - ("host:8080", "host:8080"), - ("", ""), - (None, ""), -]) -def test_safe_url_strips_secrets(raw, expected): - out = sh._safe_url(raw) - assert out == expected - for bad in ("pass", "secret", "hunter2", "abc", "token", "@"): - if raw and bad in raw and bad not in expected: - assert bad not in out - - -# ── _classify_error: controlled categories, never raw text ── - -def test_classify_error_categories(): - import socket - assert sh._classify_error(TimeoutError()) == "timeout" - assert sh._classify_error(socket.timeout()) == "timeout" - assert sh._classify_error(socket.gaierror()) == "dns_error" - assert sh._classify_error(ConnectionRefusedError()) == "connection_refused" - assert sh._classify_error(OSError("boom")) == "network_error" - assert sh._classify_error(ValueError("x")) == "error" - - -# ── Sanitization in subsystem output (blocker #2) ── - -def test_searxng_meta_redacts_instance_url(): - s = sh.searxng_health( - {"search_provider": "searxng", - "search_url": "http://user:s3cr3t@searx.local:8080/?token=zzz"}, - http_get=lambda url, timeout: _resp(200), - ) - blob = repr(s) - assert "s3cr3t" not in blob and "zzz" not in blob and "user:" not in blob - assert s["meta"]["instance"] == "http://searx.local:8080" - - -def test_searxng_down_uses_error_category_not_raw_exception(): - def boom(url, timeout): - raise RuntimeError("failed connecting to http://user:pw@searx.local secret-token") - s = sh.searxng_health( - {"search_provider": "searxng", "search_url": "http://searx.local"}, - http_get=boom, - ) - assert s["status"] == sh.DOWN - assert s["meta"]["error"] == "error" # controlled category token - assert "secret-token" not in repr(s) and "pw@" not in repr(s) - - -def test_ntfy_meta_redacts_userinfo_in_base(): - intg = [{"preset": "ntfy", "enabled": True, - "base_url": "https://user:topsecret@ntfy.example.com"}] - seen = {} - - def getter(url, timeout): - seen["url"] = url # the probe itself may keep credentials - return _resp(200) - - s = sh.ntfy_health(intg, {"reminder_channel": "ntfy"}, http_get=getter) - assert s["meta"]["base"] == "https://ntfy.example.com" - assert "topsecret" not in repr(s) - - -def test_providers_name_fallback_is_sanitized(): - # No display name → falls back to the base_url, which must be sanitized. - ep = {"base_url": "http://user:k3y@prov.local:9000/v1?api_key=zzz", "api_key": "sk-x"} - s = sh.providers_health([ep], probe=lambda b, k, t: ["m1"]) - entry = s["meta"]["endpoints"][0] - assert entry["name"] == "http://prov.local:9000/v1" - assert "k3y" not in repr(s) and "zzz" not in repr(s) and "sk-x" not in repr(s) - - -def test_providers_probe_exception_maps_to_category(): - def boom(base, key, timeout): - raise RuntimeError(f"500 from {base} with key {key}") # would leak base+key - s = sh.providers_health([_ep("a")], probe=boom) - assert s["status"] == sh.DOWN - assert s["meta"]["endpoints"][0]["error"] == "error" - assert "sk-secret" not in repr(s) and "http://a" not in repr(s) - - -def test_email_connect_exception_maps_to_category(): - def boom(account_id): - raise RuntimeError("login failed for user bob with password hunter2") - s = sh.email_health([_acct("a")], connect=boom) - assert s["status"] == sh.DOWN - assert s["meta"]["accounts"][0]["error"] == "error" - assert "hunter2" not in repr(s) - - -# ── Bounded wall-clock (blocker #1) ── - -def test_providers_bounded_marks_slow_as_timeout(monkeypatch): - import time - monkeypatch.setattr(sh, "_FANOUT_BUDGET", 1) - - def probe(base, key, timeout): - if "slow" in base: - time.sleep(10) # would blow the budget if unbounded - return ["m1"] - - eps = [{"name": "fast", "base_url": "http://fast", "api_key": "k"}, - {"name": "slow", "base_url": "http://slow", "api_key": "k"}] - t0 = time.monotonic() - out = sh.providers_health(eps, probe=probe) - elapsed = time.monotonic() - t0 - assert elapsed < 4, f"providers_health not bounded: took {elapsed:.1f}s" - by = {e["name"]: e for e in out["meta"]["endpoints"]} - assert by["fast"]["ok"] is True - assert by["slow"]["ok"] is False and by["slow"]["error"] == "timeout" - assert out["status"] == sh.DEGRADED - - -def test_providers_bounded_with_many_slow_endpoints(monkeypatch): - import time - monkeypatch.setattr(sh, "_FANOUT_BUDGET", 1) - - def probe(base, key, timeout): - time.sleep(10) - return ["m1"] - - eps = [{"name": f"ep{i}", "base_url": f"http://ep{i}", "api_key": "k"} - for i in range(25)] - t0 = time.monotonic() - out = sh.providers_health(eps, probe=probe) - elapsed = time.monotonic() - t0 - # 25 endpoints * sleep would be huge if sequential; bounded keeps it ~budget. - assert elapsed < 4, f"not bounded with many endpoints: {elapsed:.1f}s" - assert out["status"] == sh.DOWN - assert all(e["error"] == "timeout" for e in out["meta"]["endpoints"]) - - -def test_email_bounded_marks_slow_as_timeout(monkeypatch): - import time - monkeypatch.setattr(sh, "_FANOUT_BUDGET", 1) - - def connect(account_id): - if account_id == "slow": - time.sleep(10) - return _Conn() - - accts = [_acct("fast"), _acct("slow")] - accts[1]["account_id"] = "slow" - t0 = time.monotonic() - out = sh.email_health(accts, connect=connect) - elapsed = time.monotonic() - t0 - assert elapsed < 4, f"email_health not bounded: took {elapsed:.1f}s" - by = {a["name"]: a for a in out["meta"]["accounts"]} - assert by["slow"]["error"] == "timeout" - - -def test_collect_runs_subsystems_concurrently(monkeypatch): - # The aggregate is bounded by running the (internally-bounded) subsystems - # concurrently, so total wall-clock ≈ max(subsystem), not the sum. Each of - # the four network subsystems here sleeps ~0.6s; sequential would be ~2.4s. - import asyncio - import time - monkeypatch.setattr(sh, "_gather_inputs", lambda: { - "settings": {}, "integrations": [], "accounts": [], "endpoints": [], - }) - - def slow(name): - def _fn(*_a, **_k): - time.sleep(0.6) - return {"name": name, "status": sh.OK, "detail": "", "meta": {}} - return _fn - - monkeypatch.setattr(sh, "searxng_health", slow("searxng")) - monkeypatch.setattr(sh, "ntfy_health", slow("ntfy")) - monkeypatch.setattr(sh, "email_health", slow("email")) - monkeypatch.setattr(sh, "providers_health", slow("providers")) - - t0 = time.monotonic() - out = asyncio.run(sh.collect_service_health(None, None)) - elapsed = time.monotonic() - t0 - assert elapsed < 1.5, f"subsystems not concurrent: took {elapsed:.1f}s" - assert {s["name"] for s in out["services"]} == { - "chromadb", "searxng", "ntfy", "email", "providers"} - - -def test_collect_aggregate_deadline_yields_controlled_result(monkeypatch): - # If the gather overruns the aggregate ceiling, the response is still a - # controlled {overall, services, timestamp} with each network subsystem - # marked down/timeout — never a hang or a raised exception. - import asyncio - import time - monkeypatch.setattr(sh, "_AGGREGATE_DEADLINE", 0.5) - monkeypatch.setattr(sh, "_SUBSYSTEM_DEADLINE", 0.4) - monkeypatch.setattr(sh, "_gather_inputs", lambda: { - "settings": {}, "integrations": [], "accounts": [], "endpoints": [], - }) - - async def _slow_gather(*coros, **_k): - for c in coros: # close unawaited coros to avoid warnings - close = getattr(c, "close", None) - if close: - close() - await asyncio.sleep(5) - - # Force the outer wait_for to trip by making gather itself slow. - monkeypatch.setattr(sh.asyncio, "gather", _slow_gather) - t0 = time.monotonic() - out = asyncio.run(sh.collect_service_health(None, None)) - elapsed = time.monotonic() - t0 - assert elapsed < 2, f"aggregate deadline did not bound: {elapsed:.1f}s" - assert set(out) == {"overall", "services", "timestamp"} - net = [s for s in out["services"] if s["name"] != "chromadb"] - assert all(s["status"] == sh.DOWN and s["meta"].get("error") == "timeout" - for s in net) diff --git a/tests/test_service_health_chromadb.py b/tests/test_service_health_chromadb.py new file mode 100644 index 000000000..290d1f986 --- /dev/null +++ b/tests/test_service_health_chromadb.py @@ -0,0 +1,37 @@ +"""Tests for chromadb_health — ok/degraded/down/disabled classification.""" +import pytest + +from src import service_health as sh + + +class _Store: + def __init__(self, healthy): + self.healthy = healthy + + +def test_chromadb_both_healthy_ok(): + s = sh.chromadb_health(_Store(True), _Store(True)) + assert s["status"] == sh.OK + assert s["meta"] == {"rag": True, "memory": True} + + +def test_chromadb_one_down_degraded(): + s = sh.chromadb_health(_Store(True), _Store(False)) + assert s["status"] == sh.DEGRADED + + +def test_chromadb_both_unhealthy_down(): + s = sh.chromadb_health(_Store(False), _Store(False)) + assert s["status"] == sh.DOWN + + +def test_chromadb_both_absent_disabled(): + s = sh.chromadb_health(None, None) + assert s["status"] == sh.DISABLED + + +def test_chromadb_one_absent_one_healthy_ok(): + # An absent store is not a failure; the present one being healthy is ok. + s = sh.chromadb_health(_Store(True), None) + assert s["status"] == sh.OK + assert s["meta"]["memory"] is None diff --git a/tests/test_service_health_collect.py b/tests/test_service_health_collect.py new file mode 100644 index 000000000..40e2d6f60 --- /dev/null +++ b/tests/test_service_health_collect.py @@ -0,0 +1,139 @@ +"""Tests for rollup logic, aggregate collection, and shared utility helpers (_safe_url, _classify_error).""" +import pytest + +from src import service_health as sh + + +class _Store: + def __init__(self, healthy): + self.healthy = healthy + + +# ── rollup ── + +def test_rollup_picks_worst_non_disabled(): + services = [ + {"status": sh.OK}, {"status": sh.DISABLED}, + {"status": sh.DEGRADED}, {"status": sh.OK}, + ] + assert sh._rollup(services) == sh.DEGRADED + + +def test_rollup_down_beats_degraded(): + assert sh._rollup([{"status": sh.DEGRADED}, {"status": sh.DOWN}]) == sh.DOWN + + +def test_rollup_all_disabled_is_ok(): + assert sh._rollup([{"status": sh.DISABLED}, {"status": sh.DISABLED}]) == sh.OK + + +# ── collect_service_health (async aggregate) ── + +def test_collect_service_health_shape(monkeypatch): + import asyncio + + # Avoid touching real data sources / network. + monkeypatch.setattr(sh, "_gather_inputs", lambda: { + "settings": {"search_provider": "disabled"}, + "integrations": [], + "accounts": [], + "endpoints": [], + }) + out = asyncio.run(sh.collect_service_health(_Store(True), _Store(True))) + assert set(out) == {"overall", "services", "timestamp"} + names = {s["name"] for s in out["services"]} + assert names == {"chromadb", "searxng", "ntfy", "email", "providers"} + # Chroma healthy, everything else disabled → overall ok. + assert out["overall"] == sh.OK + + +# ── _safe_url: strip userinfo / query / fragment ── + +@pytest.mark.parametrize("raw,expected", [ + ("http://user:pass@host:8080/path?api_key=secret#frag", "http://host:8080/path"), + ("https://admin:hunter2@searx.example.com/", "https://searx.example.com"), + ("http://ntfy.local:80?token=abc", "http://ntfy.local:80"), + ("host:8080", "host:8080"), + ("", ""), + (None, ""), +]) +def test_safe_url_strips_secrets(raw, expected): + out = sh._safe_url(raw) + assert out == expected + for bad in ("pass", "secret", "hunter2", "abc", "token", "@"): + if raw and bad in raw and bad not in expected: + assert bad not in out + + +# ── _classify_error: controlled categories, never raw text ── + +def test_classify_error_categories(): + import socket + assert sh._classify_error(TimeoutError()) == "timeout" + assert sh._classify_error(socket.timeout()) == "timeout" + assert sh._classify_error(socket.gaierror()) == "dns_error" + assert sh._classify_error(ConnectionRefusedError()) == "connection_refused" + assert sh._classify_error(OSError("boom")) == "network_error" + assert sh._classify_error(ValueError("x")) == "error" + + +# ── Concurrent collection and aggregate deadline ── + +def test_collect_runs_subsystems_concurrently(monkeypatch): + # The aggregate is bounded by running the (internally-bounded) subsystems + # concurrently, so total wall-clock ≈ max(subsystem), not the sum. Each of + # the four network subsystems here sleeps ~0.6s; sequential would be ~2.4s. + import asyncio + import time + monkeypatch.setattr(sh, "_gather_inputs", lambda: { + "settings": {}, "integrations": [], "accounts": [], "endpoints": [], + }) + + def slow(name): + def _fn(*_a, **_k): + time.sleep(0.6) + return {"name": name, "status": sh.OK, "detail": "", "meta": {}} + return _fn + + monkeypatch.setattr(sh, "searxng_health", slow("searxng")) + monkeypatch.setattr(sh, "ntfy_health", slow("ntfy")) + monkeypatch.setattr(sh, "email_health", slow("email")) + monkeypatch.setattr(sh, "providers_health", slow("providers")) + + t0 = time.monotonic() + out = asyncio.run(sh.collect_service_health(None, None)) + elapsed = time.monotonic() - t0 + assert elapsed < 1.5, f"subsystems not concurrent: took {elapsed:.1f}s" + assert {s["name"] for s in out["services"]} == { + "chromadb", "searxng", "ntfy", "email", "providers"} + + +def test_collect_aggregate_deadline_yields_controlled_result(monkeypatch): + # If the gather overruns the aggregate ceiling, the response is still a + # controlled {overall, services, timestamp} with each network subsystem + # marked down/timeout — never a hang or a raised exception. + import asyncio + import time + monkeypatch.setattr(sh, "_AGGREGATE_DEADLINE", 0.5) + monkeypatch.setattr(sh, "_SUBSYSTEM_DEADLINE", 0.4) + monkeypatch.setattr(sh, "_gather_inputs", lambda: { + "settings": {}, "integrations": [], "accounts": [], "endpoints": [], + }) + + async def _slow_gather(*coros, **_k): + for c in coros: # close unawaited coros to avoid warnings + close = getattr(c, "close", None) + if close: + close() + await asyncio.sleep(5) + + # Force the outer wait_for to trip by making gather itself slow. + monkeypatch.setattr(sh.asyncio, "gather", _slow_gather) + t0 = time.monotonic() + out = asyncio.run(sh.collect_service_health(None, None)) + elapsed = time.monotonic() - t0 + assert elapsed < 2, f"aggregate deadline did not bound: {elapsed:.1f}s" + assert set(out) == {"overall", "services", "timestamp"} + net = [s for s in out["services"] if s["name"] != "chromadb"] + assert all(s["status"] == sh.DOWN and s["meta"].get("error") == "timeout" + for s in net) diff --git a/tests/test_service_health_email.py b/tests/test_service_health_email.py new file mode 100644 index 000000000..5ae490b1c --- /dev/null +++ b/tests/test_service_health_email.py @@ -0,0 +1,80 @@ +"""Tests for email_health — probe logic, status classification, sanitization, and bounded timeout.""" +import pytest + +from src import service_health as sh + + +def _raise(*_a, **_k): + raise RuntimeError("connection refused") + + +def _acct(name, host="imap.example.com"): + return {"account_id": name, "account_name": name, "imap_host": host, + "imap_password": "hunter2"} + + +class _Conn: + def logout(self): + pass + + +def test_email_disabled_without_accounts(): + assert sh.email_health([])["status"] == sh.DISABLED + + +def test_email_ok_all_connect(): + s = sh.email_health([_acct("a"), _acct("b")], connect=lambda _id: _Conn()) + assert s["status"] == sh.OK + + +def test_email_degraded_some_fail(): + def connect(account_id): + if account_id == "bad": + raise RuntimeError("auth failed") + return _Conn() + + s = sh.email_health([_acct("good"), _acct("bad")], connect=connect) + assert s["status"] == sh.DEGRADED + + +def test_email_down_all_fail(): + s = sh.email_health([_acct("a")], connect=_raise) + assert s["status"] == sh.DOWN + + +def test_email_account_without_host_marked_failed(): + s = sh.email_health([_acct("a", host="")], connect=lambda _id: _Conn()) + assert s["status"] == sh.DOWN + + +def test_email_meta_never_leaks_password(): + s = sh.email_health([_acct("a")], connect=lambda _id: _Conn()) + assert "hunter2" not in repr(s) + + +def test_email_connect_exception_maps_to_category(): + def boom(account_id): + raise RuntimeError("login failed for user bob with password hunter2") + s = sh.email_health([_acct("a")], connect=boom) + assert s["status"] == sh.DOWN + assert s["meta"]["accounts"][0]["error"] == "error" + assert "hunter2" not in repr(s) + + +def test_email_bounded_marks_slow_as_timeout(monkeypatch): + import time + monkeypatch.setattr(sh, "_FANOUT_BUDGET", 1) + + def connect(account_id): + if account_id == "slow": + time.sleep(10) + return _Conn() + + accts = [_acct("fast"), _acct("slow")] + accts[1]["account_id"] = "slow" + t0 = time.monotonic() + out = sh.email_health(accts, connect=connect) + elapsed = time.monotonic() - t0 + assert elapsed < 4, f"email_health not bounded: took {elapsed:.1f}s" + by = {a["name"]: a for a in out["meta"]["accounts"]} + assert by["slow"]["error"] == "timeout" diff --git a/tests/test_service_health_ntfy.py b/tests/test_service_health_ntfy.py new file mode 100644 index 000000000..f8820f2ac --- /dev/null +++ b/tests/test_service_health_ntfy.py @@ -0,0 +1,62 @@ +"""Tests for ntfy_health — probe logic, status classification, and sanitization.""" +import types + +import pytest + +from src import service_health as sh + + +def _resp(status_code): + return types.SimpleNamespace(status_code=status_code) + + +def _raise(*_a, **_k): + raise RuntimeError("connection refused") + + +def _ntfy_intg(): + return [{"preset": "ntfy", "enabled": True, "base_url": "http://ntfy:80"}] + + +def test_ntfy_disabled_without_integration(): + s = sh.ntfy_health([], {"reminder_channel": "ntfy"}) + assert s["status"] == sh.DISABLED + + +def test_ntfy_ok(): + s = sh.ntfy_health(_ntfy_intg(), {"reminder_channel": "ntfy"}, + http_get=lambda url, timeout: _resp(200)) + assert s["status"] == sh.OK + assert s["meta"]["base"] == "http://ntfy:80" + + +def test_ntfy_probes_v1_health_not_a_topic(): + seen = {} + + def getter(url, timeout): + seen["url"] = url + return _resp(200) + + sh.ntfy_health(_ntfy_intg(), {"reminder_channel": "ntfy"}, http_get=getter) + # Non-intrusive: hits /v1/health, never publishes to a topic. + assert seen["url"].endswith("/v1/health") + + +def test_ntfy_down_on_exception(): + s = sh.ntfy_health(_ntfy_intg(), {"reminder_channel": "ntfy"}, + http_get=_raise) + assert s["status"] == sh.DOWN + + +def test_ntfy_meta_redacts_userinfo_in_base(): + intg = [{"preset": "ntfy", "enabled": True, + "base_url": "https://user:topsecret@ntfy.example.com"}] + seen = {} + + def getter(url, timeout): + seen["url"] = url # the probe itself may keep credentials + return _resp(200) + + s = sh.ntfy_health(intg, {"reminder_channel": "ntfy"}, http_get=getter) + assert s["meta"]["base"] == "https://ntfy.example.com" + assert "topsecret" not in repr(s) diff --git a/tests/test_service_health_providers.py b/tests/test_service_health_providers.py new file mode 100644 index 000000000..ad2d72794 --- /dev/null +++ b/tests/test_service_health_providers.py @@ -0,0 +1,100 @@ +"""Tests for providers_health — probe logic, status classification, sanitization, and bounded timeout.""" +import pytest + +from src import service_health as sh + + +def _raise(*_a, **_k): + raise RuntimeError("connection refused") + + +def _ep(name): + return {"name": name, "base_url": f"http://{name}:8000/v1", "api_key": "sk-secret"} + + +def test_providers_disabled_without_endpoints(): + assert sh.providers_health([])["status"] == sh.DISABLED + + +def test_providers_ok_all_reachable(): + s = sh.providers_health([_ep("a")], + probe=lambda base, key, timeout: ["m1", "m2"]) + assert s["status"] == sh.OK + assert s["meta"]["endpoints"][0]["model_count"] == 2 + + +def test_providers_degraded_some_empty(): + def probe(base, key, timeout): + return ["m1"] if "good" in base else [] + + s = sh.providers_health([_ep("good"), _ep("bad")], probe=probe) + assert s["status"] == sh.DEGRADED + + +def test_providers_down_all_fail(): + s = sh.providers_health([_ep("a")], probe=_raise) + assert s["status"] == sh.DOWN + + +def test_providers_meta_never_leaks_api_key(): + s = sh.providers_health([_ep("a")], + probe=lambda base, key, timeout: ["m1"]) + assert "sk-secret" not in repr(s) + + +def test_providers_name_fallback_is_sanitized(): + # No display name → falls back to the base_url, which must be sanitized. + ep = {"base_url": "http://user:k3y@prov.local:9000/v1?api_key=zzz", "api_key": "sk-x"} + s = sh.providers_health([ep], probe=lambda b, k, t: ["m1"]) + entry = s["meta"]["endpoints"][0] + assert entry["name"] == "http://prov.local:9000/v1" + assert "k3y" not in repr(s) and "zzz" not in repr(s) and "sk-x" not in repr(s) + + +def test_providers_probe_exception_maps_to_category(): + def boom(base, key, timeout): + raise RuntimeError(f"500 from {base} with key {key}") # would leak base+key + s = sh.providers_health([_ep("a")], probe=boom) + assert s["status"] == sh.DOWN + assert s["meta"]["endpoints"][0]["error"] == "error" + assert "sk-secret" not in repr(s) and "http://a" not in repr(s) + + +def test_providers_bounded_marks_slow_as_timeout(monkeypatch): + import time + monkeypatch.setattr(sh, "_FANOUT_BUDGET", 1) + + def probe(base, key, timeout): + if "slow" in base: + time.sleep(10) # would blow the budget if unbounded + return ["m1"] + + eps = [{"name": "fast", "base_url": "http://fast", "api_key": "k"}, + {"name": "slow", "base_url": "http://slow", "api_key": "k"}] + t0 = time.monotonic() + out = sh.providers_health(eps, probe=probe) + elapsed = time.monotonic() - t0 + assert elapsed < 4, f"providers_health not bounded: took {elapsed:.1f}s" + by = {e["name"]: e for e in out["meta"]["endpoints"]} + assert by["fast"]["ok"] is True + assert by["slow"]["ok"] is False and by["slow"]["error"] == "timeout" + assert out["status"] == sh.DEGRADED + + +def test_providers_bounded_with_many_slow_endpoints(monkeypatch): + import time + monkeypatch.setattr(sh, "_FANOUT_BUDGET", 1) + + def probe(base, key, timeout): + time.sleep(10) + return ["m1"] + + eps = [{"name": f"ep{i}", "base_url": f"http://ep{i}", "api_key": "k"} + for i in range(25)] + t0 = time.monotonic() + out = sh.providers_health(eps, probe=probe) + elapsed = time.monotonic() - t0 + # 25 endpoints * sleep would be huge if sequential; bounded keeps it ~budget. + assert elapsed < 4, f"not bounded with many endpoints: {elapsed:.1f}s" + assert out["status"] == sh.DOWN + assert all(e["error"] == "timeout" for e in out["meta"]["endpoints"]) diff --git a/tests/test_service_health_search.py b/tests/test_service_health_search.py new file mode 100644 index 000000000..56553808a --- /dev/null +++ b/tests/test_service_health_search.py @@ -0,0 +1,79 @@ +"""Tests for searxng_health — probe logic, status classification, and sanitization.""" +import types + +import pytest + +from src import service_health as sh + + +def _resp(status_code): + return types.SimpleNamespace(status_code=status_code) + + +def _raise(*_a, **_k): + raise RuntimeError("connection refused") + + +def test_searxng_disabled_when_other_provider(): + s = sh.searxng_health({"search_provider": "brave"}) + assert s["status"] == sh.DISABLED + + +def test_searxng_ok_on_healthz(): + s = sh.searxng_health( + {"search_provider": "searxng", "search_url": "http://sx:8080"}, + http_get=lambda url, timeout: _resp(200), + ) + assert s["status"] == sh.OK + assert s["meta"]["probed"] == "/healthz" + + +def test_searxng_ok_on_root_fallback(): + def getter(url, timeout): + return _resp(404) if url.endswith("/healthz") else _resp(200) + + s = sh.searxng_health( + {"search_provider": "searxng", "search_url": "http://sx:8080"}, + http_get=getter, + ) + assert s["status"] == sh.OK + assert s["meta"]["probed"] == "/" + + +def test_searxng_down_on_exception(): + s = sh.searxng_health( + {"search_provider": "searxng", "search_url": "http://sx:8080"}, + http_get=_raise, + ) + assert s["status"] == sh.DOWN + + +def test_searxng_down_on_5xx(): + s = sh.searxng_health( + {"search_provider": "searxng", "search_url": "http://sx:8080"}, + http_get=lambda url, timeout: _resp(502), + ) + assert s["status"] == sh.DOWN + + +def test_searxng_meta_redacts_instance_url(): + s = sh.searxng_health( + {"search_provider": "searxng", + "search_url": "http://user:s3cr3t@searx.local:8080/?token=zzz"}, + http_get=lambda url, timeout: _resp(200), + ) + blob = repr(s) + assert "s3cr3t" not in blob and "zzz" not in blob and "user:" not in blob + assert s["meta"]["instance"] == "http://searx.local:8080" + + +def test_searxng_down_uses_error_category_not_raw_exception(): + def boom(url, timeout): + raise RuntimeError("failed connecting to http://user:pw@searx.local secret-token") + s = sh.searxng_health( + {"search_provider": "searxng", "search_url": "http://searx.local"}, + http_get=boom, + ) + assert s["status"] == sh.DOWN + assert s["meta"]["error"] == "error" # controlled category token + assert "secret-token" not in repr(s) and "pw@" not in repr(s) diff --git a/tests/test_session_routes_utcnow.py b/tests/test_session_routes_utcnow.py new file mode 100644 index 000000000..33b0f18a4 --- /dev/null +++ b/tests/test_session_routes_utcnow.py @@ -0,0 +1,11 @@ +"""Regression: session routes must not call datetime.utcnow() (#1116).""" + +import inspect + +import routes.session_routes as sr + + +def test_session_routes_module_does_not_reference_utcnow(): + source = inspect.getsource(sr) + assert "datetime.utcnow()" not in source + assert "_dt.utcnow()" not in source \ No newline at end of file diff --git a/tests/test_session_tools_registry.py b/tests/test_session_tools_registry.py index 804cfdbdc..4f63f550f 100644 --- a/tests/test_session_tools_registry.py +++ b/tests/test_session_tools_registry.py @@ -137,6 +137,55 @@ def test_no_session_manager_is_handled(monkeypatch): assert "error" in res or "results" in res +class _FakeSession: + def __init__(self, owner, name, history): + self.owner = owner + self.name = name + self.endpoint_url = "http://x" + self.model = "fixture-tool-model" # offline path: returns transcript, no network + self._history = history + self.added = [] + + def get_context_messages(self): + return list(self._history) + + def add_message(self, m): + self.added.append(m) + + +class _FakeMgr: + def __init__(self, sessions): + self._s = sessions + + def get_session(self, sid): + return self._s.get(sid) + + +def test_send_to_session_blocks_null_owner_for_authenticated_caller(monkeypatch): + # An authenticated caller must not reach a null-owner (legacy / auth-was-off) + # session: list_sessions and manage_session already hide those, so this path + # was the inconsistency — it let an agent read/write a session the other + # tools exclude. Mirrors the calendar owner=None hardening. + null_sess = _FakeSession(None, "Secret", [{"role": "user", "content": "PIN 4321"}]) + bob_sess = _FakeSession("bob", "Bob", [{"role": "user", "content": "bob secret"}]) + monkeypatch.setattr(st, "get_session_manager", + lambda: _FakeMgr({"nsid": null_sess, "bsid": bob_sess})) + + # authenticated alice: null-owner session is not-found and its history is not leaked + r = asyncio.run(st.send_to_session("nsid\nhello", owner="alice")) + assert r.get("error", "").endswith("not found") + assert "4321" not in str(r) + assert null_sess.added == [] # nothing written into it either + + # authenticated alice still cannot reach another real user's session + r2 = asyncio.run(st.send_to_session("bsid\nhello", owner="alice")) + assert r2.get("error", "").endswith("not found") + + # auth disabled (no owner): single-user still reaches the null-owner session + r3 = asyncio.run(st.send_to_session("nsid\nhello", owner=None)) + assert r3.get("offline_transcript") is True + + def test_dispatched_via_registry_not_dispatch_ai_tool(): source = (Path(__file__).resolve().parent.parent / "src" / "tool_execution.py").read_text(encoding="utf-8") assert 'elif tool in ("create_session", "list_sessions", "send_to_session", "manage_session"):' in source diff --git a/tests/test_setup_admin_user.py b/tests/test_setup_admin_user.py index 9ecfb416b..b0fde4d75 100644 --- a/tests/test_setup_admin_user.py +++ b/tests/test_setup_admin_user.py @@ -1,5 +1,6 @@ import importlib.util import json +import os from pathlib import Path @@ -23,3 +24,49 @@ def test_create_default_admin_normalizes_env_username(tmp_path, monkeypatch): data = json.loads(auth_path.read_text(encoding="utf-8")) assert "adminuser" in data["users"] assert "AdminUser" not in data["users"] + + +def test_main_loads_admin_password_from_env_file(tmp_path, monkeypatch): + """Regression: setup.py must honor an admin password pre-seeded in .env on + native installs, even when the var is not exported into the shell + (docs/setup.md documents this). Previously setup.py never called + load_dotenv(), so os.getenv() saw nothing and a random password was + generated instead.""" + import bcrypt + + setup_module = _load_setup_module() + + # Credentials live ONLY in a .env beside setup.py (written with a UTF-8 BOM, + # the Notepad-on-Windows case that utf-8-sig must tolerate) — not exported. + monkeypatch.delenv("ODYSSEUS_ADMIN_USER", raising=False) + monkeypatch.delenv("ODYSSEUS_ADMIN_PASSWORD", raising=False) + (tmp_path / ".env").write_text( + "ODYSSEUS_ADMIN_USER=presetuser\nODYSSEUS_ADMIN_PASSWORD=fromenvfile12345\n", + encoding="utf-8-sig", + ) + + # Point setup at the temp dir and neutralize main()'s heavy steps. + monkeypatch.setattr(setup_module, "BASE_DIR", str(tmp_path)) + auth_path = tmp_path / "auth.json" + monkeypatch.setattr(setup_module, "AUTH_FILE", str(auth_path)) + monkeypatch.setattr(setup_module, "check_arch", lambda: None) + monkeypatch.setattr(setup_module, "create_dirs", lambda: None) + monkeypatch.setattr(setup_module, "create_env", lambda: None) + monkeypatch.setattr(setup_module, "check_deps", lambda: None) + monkeypatch.setattr(setup_module, "init_database", lambda: None) + # Force the non-interactive branch so the test never blocks on a prompt. + monkeypatch.setenv("ODYSSEUS_SKIP_ADMIN_PROMPT", "1") + + try: + setup_module.main() + finally: + # load_dotenv writes real os.environ entries; undo so sibling tests + # don't inherit them. + os.environ.pop("ODYSSEUS_ADMIN_USER", None) + os.environ.pop("ODYSSEUS_ADMIN_PASSWORD", None) + + data = json.loads(auth_path.read_text(encoding="utf-8")) + assert "presetuser" in data["users"], data + assert bcrypt.checkpw( + b"fromenvfile12345", data["users"]["presetuser"]["password_hash"].encode() + ), "admin password from .env was ignored; a random one was generated" diff --git a/tests/test_setup_llamacpp_hint_js.py b/tests/test_setup_llamacpp_hint_js.py new file mode 100644 index 000000000..2eef9483c --- /dev/null +++ b/tests/test_setup_llamacpp_hint_js.py @@ -0,0 +1,17 @@ +"""The /setup guide must offer a llama.cpp (llama-server) local example. + +Without it, the port-8080 "llama.cpp" provider label (src/llm_core.py +_provider_label) is never reachable from first-run setup — a user pasting a +local endpoint only saw the Ollama and generic examples. Both the static-HTML +and the streamed-blocks renderings of the setup guide must carry the example. +""" +from pathlib import Path + +_SRC = Path(__file__).resolve().parent.parent / "static" / "js" / "slashCommands.js" + + +def test_setup_guide_offers_llamacpp_local_example(): + src = _SRC.read_text(encoding="utf-8") + # The example URL appears in both the HTML-string and streamed renderings. + assert src.count("http://localhost:8080/v1") >= 2 + assert "llama.cpp (llama-server)" in src diff --git a/tests/test_shell_routes.py b/tests/test_shell_routes.py index 5f9ea59a3..6ee7bbe15 100644 --- a/tests/test_shell_routes.py +++ b/tests/test_shell_routes.py @@ -5,6 +5,7 @@ import importlib import importlib.util import json import os +import socket import sys from pathlib import Path from types import SimpleNamespace @@ -13,6 +14,7 @@ import pytest from routes.shell_routes import ( _find_line_break, + _host_docker_access_enabled, _import_optional_dependency_for_status, _running_in_container, _docker_row_status, @@ -216,13 +218,24 @@ class TestDockerRowStatus: assert status.applicable is False assert status.install_hint == DOCKER_IN_CONTAINER_HINT - def test_in_container_but_present_is_applicable_with_default_hint(self): + def test_in_container_cli_without_opt_in_is_not_applicable(self): status = _docker_row_status( on_remote=False, in_container=True, installed=True, default_hint=self.DEFAULT, ) + assert status.applicable is False + assert status.install_hint == DOCKER_IN_CONTAINER_HINT + + def test_in_container_opt_in_with_socket_is_applicable(self): + status = _docker_row_status( + on_remote=False, + in_container=True, + installed=True, + default_hint=self.DEFAULT, + host_docker_access=True, + ) assert status.applicable is True assert status.install_hint == self.DEFAULT @@ -260,7 +273,51 @@ class TestDockerRowStatus: lowered = DOCKER_IN_CONTAINER_HINT.lower() assert "remote" in lowered assert "socket" in lowered - assert "host-root" in lowered or "host root" in lowered + assert "high-trust" in lowered + assert "docker/host-docker.yml" in lowered + + +class TestHostDockerAccess: + def test_opt_in_without_socket_is_disabled(self, monkeypatch, tmp_path): + monkeypatch.setenv("ODYSSEUS_ENABLE_HOST_DOCKER", "true") + + assert _host_docker_access_enabled(str(tmp_path / "missing.sock")) is False + + def test_regular_file_is_not_accepted(self, monkeypatch, tmp_path): + socket_path = tmp_path / "docker.sock" + socket_path.touch() + monkeypatch.setenv("ODYSSEUS_ENABLE_HOST_DOCKER", "true") + + assert _host_docker_access_enabled(str(socket_path)) is False + + @pytest.mark.parametrize("flag", [None, "false"]) + def test_socket_without_explicit_opt_in_is_disabled( + self, + monkeypatch, + tmp_path, + flag, + ): + socket_path = tmp_path / "docker.sock" + with socket.socket(socket.AF_UNIX) as unix_socket: + unix_socket.bind(str(socket_path)) + if flag is None: + monkeypatch.delenv("ODYSSEUS_ENABLE_HOST_DOCKER", raising=False) + else: + monkeypatch.setenv("ODYSSEUS_ENABLE_HOST_DOCKER", flag) + + assert _host_docker_access_enabled(str(socket_path)) is False + + def test_explicit_opt_in_with_unix_socket_is_enabled( + self, + monkeypatch, + tmp_path, + ): + socket_path = tmp_path / "docker.sock" + with socket.socket(socket.AF_UNIX) as unix_socket: + unix_socket.bind(str(socket_path)) + monkeypatch.setenv("ODYSSEUS_ENABLE_HOST_DOCKER", "true") + + assert _host_docker_access_enabled(str(socket_path)) is True class TestPackageProbeStatus: diff --git a/tests/test_skill_importer.py b/tests/test_skill_importer.py index eecca614f..d7822711b 100644 --- a/tests/test_skill_importer.py +++ b/tests/test_skill_importer.py @@ -71,7 +71,7 @@ def test_fetch_bytes_rejects_cross_host_redirect(monkeypatch): monkeypatch.setattr("services.memory.skill_importer.httpx.Client", _Client) monkeypatch.setattr( "services.memory.skill_importer.check_outbound_url", - lambda url: (True, ""), + lambda url, **kwargs: (True, ""), ) with pytest.raises(SkillImportError, match="redirect target"): _fetch_bytes("https://raw.githubusercontent.com/o/r/main/SKILL.md") @@ -91,7 +91,7 @@ def test_list_github_dir_accepts_api_github_response(monkeypatch): ) monkeypatch.setattr( "services.memory.skill_importer.check_outbound_url", - lambda url: (True, ""), + lambda url, **kwargs: (True, ""), ) class _Resp: @@ -146,7 +146,7 @@ def _mock_httpx_client(monkeypatch, response): monkeypatch.setattr("services.memory.skill_importer.httpx.Client", _Client) monkeypatch.setattr( "services.memory.skill_importer.check_outbound_url", - lambda url: (True, ""), + lambda url, **kwargs: (True, ""), ) diff --git a/tests/test_skill_importer_ssrf_redirect.py b/tests/test_skill_importer_ssrf_redirect.py new file mode 100644 index 000000000..800be633f --- /dev/null +++ b/tests/test_skill_importer_ssrf_redirect.py @@ -0,0 +1,123 @@ +"""Skill importer SSRF hardening: redirects must be re-validated per hop. + +The importer follows redirects manually (`_get_checked`) and re-runs the SSRF +guard on every hop with ``block_private=True``, matching the hardened web-fetch +path in ``services/search/content.py:_get_public_url``. Previously it used +``httpx``'s ``follow_redirects=True`` with the lenient guard on the *initial* +URL only, so a ``3xx`` to an internal/metadata address was still connected to. + +These tests are hermetic: every host is an IP literal, so ``check_outbound_url`` +resolves them locally (``getaddrinfo`` on a numeric address does no DNS) and no +network access is required. The HTTP layer is faked so no real request is made. +""" +import pytest + +from services.memory import skill_importer +from services.memory.skill_importer import ( + SkillImportError, + _check_fetch_url, + _fetch_bytes, + _get_checked, + parse_skill_source, +) + +# Clearly-public, non-reserved IP literals for the initial (allowed) hop. +PUBLIC_A = "https://1.1.1.1/skill" +PUBLIC_B = "https://8.8.8.8/skill" +# Internal redirect targets that must be refused before connection. +LOOPBACK = "http://127.0.0.1/latest" +METADATA = "http://169.254.169.254/latest/meta-data/" + + +def _install_fake_client(monkeypatch, *, redirect_from, redirect_to): + """Replace httpx.Client so `redirect_from` 302s to `redirect_to`, and any + other URL returns 200. No real socket is opened.""" + + class _Resp: + def __init__(self, url, status, location): + self.url = url + self.status_code = status + self.headers = {"location": location} if location else {} + self.content = b"ok" + self.text = "" + + def raise_for_status(self): + return None + + def json(self): + return {} + + class _Client: + def __init__(self, *args, **kwargs): + # Safety invariant: the importer follows redirects by hand and + # re-runs the SSRF guard per hop, so it MUST disable httpx's own + # redirect following. Asserting ``follow_redirects is False`` here + # (not merely accepting the kwarg) makes any regression to + # ``follow_redirects=True`` fail these tests instead of passing + # silently — httpx being faked would otherwise hide the change. + assert kwargs.get("follow_redirects") is False, ( + "skill importer must construct httpx.Client with " + "follow_redirects=False; got " + f"{kwargs.get('follow_redirects')!r}" + ) + + def __enter__(self): + return self + + def __exit__(self, *args): + return False + + def get(self, url, headers=None): + if url == redirect_from: + return _Resp(url, 302, redirect_to) + return _Resp(url, 200, None) + + monkeypatch.setattr(skill_importer.httpx, "Client", _Client) + + +# --- Guard unit: block_private=True refuses internal, allows public ---------- + +@pytest.mark.parametrize("url", [LOOPBACK, METADATA, "http://10.0.0.5/", "http://[::1]/"]) +def test_check_fetch_url_blocks_internal(url): + with pytest.raises(SkillImportError): + _check_fetch_url(url) + + +@pytest.mark.parametrize("url", [PUBLIC_A, PUBLIC_B]) +def test_check_fetch_url_allows_public(url): + # Should not raise for a public IP literal. + _check_fetch_url(url) + + +# --- Redirect revalidation: the core regression ------------------------------ + +@pytest.mark.parametrize("internal", [LOOPBACK, METADATA]) +def test_get_checked_blocks_redirect_to_internal(monkeypatch, internal): + _install_fake_client(monkeypatch, redirect_from=PUBLIC_A, redirect_to=internal) + with pytest.raises(SkillImportError, match="blocked"): + _get_checked(PUBLIC_A) + + +@pytest.mark.parametrize("internal", [LOOPBACK, METADATA]) +def test_fetch_bytes_blocks_redirect_to_internal(monkeypatch, internal): + # Higher-level: the public fetch helpers inherit the per-hop guard. + _install_fake_client(monkeypatch, redirect_from=PUBLIC_A, redirect_to=internal) + with pytest.raises(SkillImportError, match="blocked"): + _fetch_bytes(PUBLIC_A) + + +def test_skills_sh_entry_blocks_redirect_to_metadata(monkeypatch): + # The skills.sh unwrap path (user-supplied host) must also revalidate hops. + raw = "http://1.1.1.1/skills.sh" # contains "skills.sh", not "github.com" + _install_fake_client(monkeypatch, redirect_from=raw, redirect_to=METADATA) + with pytest.raises(SkillImportError, match="blocked"): + parse_skill_source(raw) + + +# --- Positive: a legitimate public->public redirect is still followed -------- + +def test_get_checked_follows_public_redirect(monkeypatch): + _install_fake_client(monkeypatch, redirect_from=PUBLIC_A, redirect_to=PUBLIC_B) + resp = _get_checked(PUBLIC_A) + assert resp.status_code == 200 + assert str(resp.url) == PUBLIC_B diff --git a/tests/test_slash_setup_provider_aliases.py b/tests/test_slash_setup_provider_aliases.py new file mode 100644 index 000000000..0e50c7c48 --- /dev/null +++ b/tests/test_slash_setup_provider_aliases.py @@ -0,0 +1,29 @@ +import re +import subprocess +from pathlib import Path + + +def test_opencode_setup_provider_aliases_resolve(): + source = Path("static/js/slashCommands.js").read_text() + match = re.search( + r"const SETUP_PROVIDER_URLS = \{[\s\S]*?\nfunction _normalizeSetupBaseUrl", + source, + ) + assert match, "setup provider helper block not found" + helper_source = match.group(0).removesuffix("\nfunction _normalizeSetupBaseUrl") + script = helper_source + r""" +function assert(condition, message) { + if (!condition) throw new Error(message); +} +const zenFromCommand = _setupProviderFromInput('opencode zen'); +assert(zenFromCommand && zenFromCommand.url === 'https://opencode.ai/zen/v1', 'opencode zen command alias failed'); +const goFromCommand = _setupProviderFromInput('opencode-go'); +assert(goFromCommand && goFromCommand.url === 'https://opencode.ai/zen/go/v1', 'opencode-go command alias failed'); +const zenCredential = _extractSetupProviderCredential('opencode-zen sk-test'); +assert(zenCredential && zenCredential.provider.name === 'OpenCode Zen', 'opencode-zen credential provider failed'); +assert(zenCredential.credential === 'sk-test', 'opencode-zen credential extraction failed'); +const goCredential = _extractSetupProviderCredential('opencode go sk-test'); +assert(goCredential && goCredential.provider.name === 'OpenCode Go', 'opencode go credential provider failed'); +assert(goCredential.credential === 'sk-test', 'opencode go credential extraction failed'); +""" + subprocess.run(["node", "-e", script], check=True) diff --git a/tests/test_snap_other_layers_nonarray_js.py b/tests/test_snap_other_layers_nonarray_js.py index f99e10163..c2925d330 100644 --- a/tests/test_snap_other_layers_nonarray_js.py +++ b/tests/test_snap_other_layers_nonarray_js.py @@ -36,6 +36,28 @@ def test_compute_snap_tolerates_non_array_other_layers(): assert r["x"] == 10 and r["y"] == 10 and r["guides"] == [] +@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH") +def test_compute_snap_tolerates_missing_layer_or_context(): + js = f""" + import {{ computeSnap }} from '{_HELPER.as_posix()}'; + console.log(JSON.stringify([ + computeSnap(null, 10, 20, {{ zoom: 1, canvasW: 800, canvasH: 600 }}), + computeSnap({{ id: 'L1' }}, 11, 21, {{ zoom: 1, canvasW: 800, canvasH: 600 }}), + computeSnap({{ id: 'L1', canvas: {{ width: 100, height: 50 }} }}, 12, 22, null) + ])); + """ + proc = subprocess.run( + ["node", "--input-type=module"], + input=js, capture_output=True, text=True, cwd=str(_REPO), timeout=30, + ) + assert proc.returncode == 0, proc.stderr + assert json.loads(proc.stdout.strip()) == [ + {"x": 10, "y": 20, "guides": []}, + {"x": 11, "y": 21, "guides": []}, + {"x": 12, "y": 22, "guides": []}, + ] + + @pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH") def test_compute_snap_still_snaps_to_a_layer_edge(): other = [{"id": "L2", "visible": True, "offset": {"x": 12, "y": 300}, diff --git a/tests/test_task_cookbook_admin_gate.py b/tests/test_task_cookbook_admin_gate.py new file mode 100644 index 000000000..d7e72f9ef --- /dev/null +++ b/tests/test_task_cookbook_admin_gate.py @@ -0,0 +1,350 @@ +"""Task CRUD must not let non-admins schedule Cookbook serve actions.""" + +import sys +from datetime import datetime, timedelta, timezone +from types import SimpleNamespace +from unittest.mock import MagicMock + +import pytest +from fastapi import HTTPException +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker +from sqlalchemy.pool import NullPool + +from tests.helpers.import_state import clear_fake_database_modules + +clear_fake_database_modules() + +import core.auth as core_auth +import core.database as cdb +import routes.task_routes as task_routes +from core.database import ScheduledTask +from core.database import TaskRun +from src.task_scheduler import TaskScheduler + +_REAL_DATABASE_ATTRS = { + "Base": cdb.Base, + "SessionLocal": cdb.SessionLocal, + "ScheduledTask": ScheduledTask, + "TaskRun": TaskRun, +} +if hasattr(cdb, "engine"): + _REAL_DATABASE_ATTRS["engine"] = cdb.engine + + +def _restore_module_binding(monkeypatch, name, module): + monkeypatch.setitem(sys.modules, name, module) + parent_name, _, attr = name.rpartition(".") + parent = sys.modules.get(parent_name) + if parent is not None: + monkeypatch.setattr(parent, attr, module, raising=False) + + +@pytest.fixture() +def task_db(monkeypatch, tmp_path): + _restore_module_binding(monkeypatch, "core.database", cdb) + for attr, value in _REAL_DATABASE_ATTRS.items(): + monkeypatch.setattr(cdb, attr, value, raising=False) + engine = create_engine( + f"sqlite:///{tmp_path / 'tasks.db'}", + connect_args={"check_same_thread": False}, + poolclass=NullPool, + ) + cdb.Base.metadata.create_all(engine) + testing_session = sessionmaker(bind=engine, autoflush=False, autocommit=False) + monkeypatch.setattr(task_routes, "SessionLocal", testing_session) + monkeypatch.setattr(cdb, "SessionLocal", testing_session) + return testing_session + + +@pytest.fixture() +def configured_auth(monkeypatch): + _restore_module_binding(monkeypatch, "core.auth", core_auth) + monkeypatch.setenv("AUTH_ENABLED", "true") + + class FakeAuthManager: + is_configured = True + + def is_admin(self, user): + return user == "admin" + + monkeypatch.setattr(core_auth, "AuthManager", FakeAuthManager) + + +@pytest.fixture() +def builtin_action_info(monkeypatch): + mod = sys.modules.get("src.builtin_actions") + if mod is None: + import src.builtin_actions as mod + monkeypatch.setattr( + mod, + "BUILTIN_ACTION_INFO", + { + "summarize_emails": "Summarize emails", + "cookbook_serve": "Serve Cookbook model", + }, + raising=False, + ) + + +def _req(user): + return SimpleNamespace(state=SimpleNamespace(current_user=user)) + + +def _endpoint(method, path): + router = task_routes.setup_task_routes(MagicMock()) + for route in router.routes: + if getattr(route, "path", None) == path and method in getattr(route, "methods", set()): + return route.endpoint + raise RuntimeError(f"{method} {path} not found") + + +def _cookbook_create_req(): + return task_routes.TaskCreate( + name="Serve test model", + prompt="{}", + task_type="action", + action="cookbook_serve", + trigger_type="webhook", + ) + + +def _seed_action_task( + session_factory, + task_id, + owner, + action="summarize_emails", + *, + task_type="action", + webhook_token=None, + next_run=None, +): + db = session_factory() + try: + task = ScheduledTask( + id=task_id, + owner=owner, + name=task_id, + prompt="{}", + task_type=task_type, + action=action, + trigger_type="webhook", + status="active", + output_target="session", + webhook_token=webhook_token, + next_run=next_run, + ) + db.add(task) + db.commit() + finally: + db.close() + + +@pytest.mark.asyncio +async def test_non_admin_cannot_create_cookbook_serve_task(task_db, configured_auth): + create_task = _endpoint("POST", "/api/tasks") + + with pytest.raises(HTTPException) as exc: + await create_task(_req("alice"), _cookbook_create_req()) + + assert exc.value.status_code == 403 + db = task_db() + try: + assert db.query(ScheduledTask).count() == 0 + finally: + db.close() + + +@pytest.mark.asyncio +async def test_non_admin_cannot_update_task_to_cookbook_serve(task_db, configured_auth): + _seed_action_task(task_db, "alice-task", "alice") + update_task = _endpoint("PUT", "/api/tasks/{task_id}") + + with pytest.raises(HTTPException) as exc: + await update_task( + _req("alice"), + "alice-task", + task_routes.TaskUpdate(action="cookbook_serve"), + ) + + assert exc.value.status_code == 403 + db = task_db() + try: + task = db.query(ScheduledTask).filter(ScheduledTask.id == "alice-task").first() + assert task.action == "summarize_emails" + finally: + db.close() + + +@pytest.mark.asyncio +async def test_non_admin_cannot_update_task_type_to_activate_existing_cookbook_serve( + task_db, configured_auth +): + _seed_action_task( + task_db, + "alice-task", + "alice", + action="cookbook_serve", + task_type="llm", + ) + update_task = _endpoint("PUT", "/api/tasks/{task_id}") + + with pytest.raises(HTTPException) as exc: + await update_task( + _req("alice"), + "alice-task", + task_routes.TaskUpdate(task_type="action"), + ) + + assert exc.value.status_code == 403 + db = task_db() + try: + task = db.query(ScheduledTask).filter(ScheduledTask.id == "alice-task").first() + assert task.task_type == "llm" + assert task.action == "cookbook_serve" + finally: + db.close() + + +@pytest.mark.asyncio +async def test_non_admin_cannot_manually_run_existing_cookbook_serve_task( + task_db, configured_auth +): + _seed_action_task(task_db, "alice-task", "alice", action="cookbook_serve") + scheduler = SimpleNamespace(run_task_now=MagicMock()) + router = task_routes.setup_task_routes(scheduler) + for route in router.routes: + if getattr(route, "path", None) == "/api/tasks/{task_id}/run": + run_task = route.endpoint + break + else: + raise RuntimeError("POST /api/tasks/{task_id}/run not found") + + with pytest.raises(HTTPException) as exc: + await run_task(_req("alice"), "alice-task") + + assert exc.value.status_code == 403 + scheduler.run_task_now.assert_not_called() + + +@pytest.mark.asyncio +async def test_webhook_rejects_stale_non_admin_cookbook_serve_task( + task_db, configured_auth +): + _seed_action_task( + task_db, + "alice-task", + "alice", + action="cookbook_serve", + webhook_token="secret", + ) + webhook_trigger = _endpoint("POST", "/api/tasks/{task_id}/webhook/{token}") + + with pytest.raises(HTTPException) as exc: + await webhook_trigger("alice-task", "secret") + + assert exc.value.status_code == 403 + db = task_db() + try: + task = db.query(ScheduledTask).filter(ScheduledTask.id == "alice-task").first() + assert task.status == "paused" + assert task.next_run is None + finally: + db.close() + + +@pytest.mark.asyncio +async def test_scheduler_pauses_stale_non_admin_cookbook_serve_task( + task_db, configured_auth +): + due = datetime.now(timezone.utc).replace(tzinfo=None) - timedelta(minutes=1) + _seed_action_task( + task_db, + "alice-task", + "alice", + action="cookbook_serve", + next_run=due, + ) + db = task_db() + try: + db.add(TaskRun(id="run-1", task_id="alice-task", status="queued")) + db.commit() + finally: + db.close() + + scheduler = TaskScheduler.__new__(TaskScheduler) + scheduler._task_handles = {} + await scheduler._execute_task_locked( + "alice-task", + "run-1", + gate_foreground=False, + release_executing=False, + ) + + db = task_db() + try: + task = db.query(ScheduledTask).filter(ScheduledTask.id == "alice-task").first() + run = db.query(TaskRun).filter(TaskRun.id == "run-1").first() + assert task.status == "paused" + assert task.next_run is None + assert run.status == "error" + assert run.error == "Action 'cookbook_serve' requires admin privileges" + finally: + db.close() + + +@pytest.mark.asyncio +async def test_non_admin_action_metadata_hides_cookbook_serve( + configured_auth, builtin_action_info +): + list_actions = _endpoint("GET", "/api/tasks/meta/actions") + + out = await list_actions(_req("alice")) + + action_names = {action["name"] for action in out["actions"]} + assert "cookbook_serve" not in action_names + + +@pytest.mark.asyncio +async def test_admin_can_create_cookbook_serve_task(task_db, configured_auth): + create_task = _endpoint("POST", "/api/tasks") + + out = await create_task(_req("admin"), _cookbook_create_req()) + + assert out["action"] == "cookbook_serve" + db = task_db() + try: + task = db.query(ScheduledTask).filter(ScheduledTask.id == out["id"]).first() + assert task.owner == "admin" + finally: + db.close() + + +@pytest.mark.asyncio +async def test_admin_action_metadata_includes_cookbook_serve( + configured_auth, builtin_action_info +): + list_actions = _endpoint("GET", "/api/tasks/meta/actions") + + out = await list_actions(_req("admin")) + + action_names = {action["name"] for action in out["actions"]} + assert "cookbook_serve" in action_names + + +@pytest.mark.asyncio +async def test_auth_disabled_single_user_can_create_cookbook_serve_task( + monkeypatch, task_db +): + monkeypatch.setenv("AUTH_ENABLED", "false") + create_task = _endpoint("POST", "/api/tasks") + + out = await create_task(_req(None), _cookbook_create_req()) + + assert out["action"] == "cookbook_serve" + db = task_db() + try: + task = db.query(ScheduledTask).filter(ScheduledTask.id == out["id"]).first() + assert task.owner is None + finally: + db.close() diff --git a/tests/test_task_endpoint_normalization.py b/tests/test_task_endpoint_normalization.py new file mode 100644 index 000000000..3b751d71a --- /dev/null +++ b/tests/test_task_endpoint_normalization.py @@ -0,0 +1,43 @@ +"""Regression test for the task-path endpoint-URL normalization fix. + +Bug: the task executor passed ``task.endpoint_url`` verbatim to the model HTTP +call (unlike the chat path, which normalizes via ``build_chat_url``). A bare +OpenAI-compatible base such as ``http://host:11434/v1`` POSTed to a 404 and the +run silently reported "The model returned an empty response". + +The fix routes every resolved task endpoint through ``_normalize_chat_endpoint``. +""" +from src.task_scheduler import _normalize_chat_endpoint + + +def test_bare_v1_base_gets_chat_completions_suffix(): + # The exact failure case: a bare /v1 base must become a full chat URL. + assert ( + _normalize_chat_endpoint("http://localhost:11434/v1") + == "http://localhost:11434/v1/chat/completions" + ) + + +def test_full_chat_url_is_unchanged_idempotent(): + full = "http://localhost:11434/v1/chat/completions" + assert _normalize_chat_endpoint(full) == full + # Idempotent under repeated application. + assert _normalize_chat_endpoint(_normalize_chat_endpoint(full)) == full + + +def test_native_ollama_url_left_alone(): + # Native Ollama (/api...) has its own downstream normalizer — don't touch it. + assert _normalize_chat_endpoint("http://localhost:11434/api") == "http://localhost:11434/api" + assert _normalize_chat_endpoint("http://localhost:11434/api/chat") == "http://localhost:11434/api/chat" + + +def test_empty_and_none_are_passthrough(): + assert _normalize_chat_endpoint("") == "" + assert _normalize_chat_endpoint(None) is None + + +def test_trailing_slash_base_normalized(): + assert ( + _normalize_chat_endpoint("http://localhost:11434/v1/") + == "http://localhost:11434/v1/chat/completions" + ) diff --git a/tests/test_task_shell_tools.py b/tests/test_task_shell_tools.py index 376ceaa39..8e4440ab4 100644 --- a/tests/test_task_shell_tools.py +++ b/tests/test_task_shell_tools.py @@ -111,7 +111,8 @@ async def test_scheduled_task_honors_global_disabled_tools(monkeypatch): captured = {} async def _capture(endpoint_url, model, task, session_id, *, - system_prompt=None, disabled_tools=None, relevant_tools=None): + system_prompt=None, disabled_tools=None, relevant_tools=None, + datetime_context_msg=None): captured["disabled_tools"] = disabled_tools captured["relevant_tools"] = relevant_tools return "done" diff --git a/tests/test_taxonomy.py b/tests/test_taxonomy.py index 9b00201e4..8869ac568 100644 --- a/tests/test_taxonomy.py +++ b/tests/test_taxonomy.py @@ -50,6 +50,12 @@ def test_classify_examples(filename, expected_area, expected_sub): assert result.sub_area == expected_sub +def test_embedding_lanes_memory_file_keeps_specific_sub_area(): + result = classify_test_path("tests/test_embedding_lanes_memory.py") + assert result.area == "services" + assert result.sub_area == "embedding_memory" + + # --- classify_test_path: fallback -------------------------------------------- def test_unknown_filename_is_uncategorized(): diff --git a/tests/test_teacher_eval_tier2.py b/tests/test_teacher_eval_tier2.py new file mode 100644 index 000000000..c3eb2a643 --- /dev/null +++ b/tests/test_teacher_eval_tier2.py @@ -0,0 +1,239 @@ +import asyncio +from types import SimpleNamespace +import pytest + +import src.teacher_escalation as teacher_escalation + + +@pytest.mark.asyncio +async def test_evaluate_turn_llm_ok(monkeypatch): + seen = {} + + def fake_resolve_endpoint(prefix, fallback_url=None, owner=None): + seen["prefix"] = prefix + seen["owner"] = owner + return "http://endpoint.local/v1", "utility-model", {} + + async def fake_llm_call_async(url, model, messages, **kwargs): + seen["called"] = True + return "ok" + + monkeypatch.setattr("src.endpoint_resolver.resolve_endpoint", fake_resolve_endpoint) + monkeypatch.setattr("src.llm_core.llm_call_async", fake_llm_call_async) + + status, reason = await teacher_escalation.evaluate_turn_llm( + user_request="test request", + tool_results=[], + agent_reply="test reply", + student_endpoint_url="http://student.local/v1", + owner="alice", + ) + + assert status == "ok" + assert reason is None + assert seen["prefix"] == "utility" + assert seen["owner"] == "alice" + assert seen["called"] is True + + +@pytest.mark.asyncio +async def test_evaluate_turn_llm_failure(monkeypatch): + def fake_resolve_endpoint(prefix, fallback_url=None, owner=None): + return "http://endpoint.local/v1", "utility-model", {} + + async def fake_llm_call_async(url, model, messages, **kwargs): + return " \"Failure\" " + + monkeypatch.setattr("src.endpoint_resolver.resolve_endpoint", fake_resolve_endpoint) + monkeypatch.setattr("src.llm_core.llm_call_async", fake_llm_call_async) + + status, reason = await teacher_escalation.evaluate_turn_llm( + user_request="test request", + tool_results=[], + agent_reply="test reply", + student_endpoint_url="http://student.local/v1", + owner="alice", + ) + + assert status == "failure" + assert "LLM evaluation flagged failure" in reason + + +@pytest.mark.asyncio +async def test_evaluate_turn_llm_contains_failure_but_not_exact_match(monkeypatch): + def fake_resolve_endpoint(prefix, fallback_url=None, owner=None): + return "http://endpoint.local/v1", "utility-model", {} + + async def fake_llm_call_async(url, model, messages, **kwargs): + return "this agent execution is not a failure" + + monkeypatch.setattr("src.endpoint_resolver.resolve_endpoint", fake_resolve_endpoint) + monkeypatch.setattr("src.llm_core.llm_call_async", fake_llm_call_async) + + status, reason = await teacher_escalation.evaluate_turn_llm( + user_request="test request", + tool_results=[], + agent_reply="test reply", + student_endpoint_url="http://student.local/v1", + owner="alice", + ) + + assert status == "ok" + assert reason is None + + +@pytest.mark.asyncio +async def test_evaluate_turn_llm_exception_handling(monkeypatch): + def fake_resolve_endpoint(prefix, fallback_url=None, owner=None): + return "http://endpoint.local/v1", "utility-model", {} + + async def fake_llm_call_async(url, model, messages, **kwargs): + raise RuntimeError("model timeout") + + monkeypatch.setattr("src.endpoint_resolver.resolve_endpoint", fake_resolve_endpoint) + monkeypatch.setattr("src.llm_core.llm_call_async", fake_llm_call_async) + + # Should degrade gracefully to "ok" + status, reason = await teacher_escalation.evaluate_turn_llm( + user_request="test request", + tool_results=[], + agent_reply="test reply", + student_endpoint_url="http://student.local/v1", + owner="alice", + ) + + assert status == "ok" + assert reason is None + + +@pytest.mark.asyncio +async def test_maybe_escalate_triggers_tier2_background_task(monkeypatch): + # Enable teacher settings + monkeypatch.setattr("src.settings.get_setting", lambda key, default=None: {"teacher_enabled": True, "teacher_model": "teacher-model", "teacher_tier2_enabled": True}.get(key, default)) + + # Regex check says OK + monkeypatch.setattr("src.teacher_escalation.evaluate_turn_regex", lambda *args: ("ok", None)) + + llm_eval_called = [] + async def fake_evaluate_turn_llm(*args, **kwargs): + llm_eval_called.append(True) + return "failure", "LLM flagged failure" + + monkeypatch.setattr("src.teacher_escalation.evaluate_turn_llm", fake_evaluate_turn_llm) + + escalate_called = [] + async def fake_escalate_and_learn(user_request, tool_results, agent_reply, failure_reason, owner): + escalate_called.append(failure_reason) + return "skill-slug" + + monkeypatch.setattr("src.teacher_escalation.escalate_and_learn", fake_escalate_and_learn) + + # Call maybe_escalate + task = teacher_escalation.maybe_escalate( + student_endpoint_url="http://student.local/v1", + mode="agent", + user_request="test request", + tool_results=[], + agent_reply="test reply", + owner="alice", + ) + + assert task is not None + assert task.get_name() == "teacher_escalation_tier2" + + # Await the background task execution + await task + + assert llm_eval_called == [True] + assert escalate_called == ["LLM flagged failure"] + + +@pytest.mark.asyncio +async def test_maybe_escalate_tier2_disabled_by_default(monkeypatch): + # Enable teacher settings, but keep tier2 disabled + monkeypatch.setattr("src.settings.get_setting", lambda key, default=None: {"teacher_enabled": True, "teacher_model": "teacher-model", "teacher_tier2_enabled": False}.get(key, default)) + + # Regex check says OK + monkeypatch.setattr("src.teacher_escalation.evaluate_turn_regex", lambda *args: ("ok", None)) + + # Call maybe_escalate + task = teacher_escalation.maybe_escalate( + student_endpoint_url="http://student.local/v1", + mode="agent", + user_request="test request", + tool_results=[], + agent_reply="test reply", + owner="alice", + ) + + # Should not start any background task since Tier 2 is disabled + assert task is None + + +@pytest.mark.asyncio +async def test_run_teacher_inline_triggers_tier2_escalation(monkeypatch): + # Settings and gates + monkeypatch.setattr("src.settings.get_setting", lambda key, default=None: {"teacher_enabled": True, "teacher_model": "teacher-model", "teacher_tier2_enabled": True}.get(key, default)) + monkeypatch.setattr("src.ai_interaction._resolve_model", lambda spec, owner=None: ("http://teacher.local/v1", "teacher-model", {})) + + # Regex evaluation says "ok" + monkeypatch.setattr("src.teacher_escalation.evaluate_turn_regex", lambda *args: ("ok", None)) + + # LLM evaluation flags "failure" + async def fake_evaluate_turn_llm(*args, **kwargs): + return "failure", "LLM flagged failure" + monkeypatch.setattr("src.teacher_escalation.evaluate_turn_llm", fake_evaluate_turn_llm) + + # Mock stream_agent_loop recursively called by run_teacher_inline + async def fake_stream_agent_loop(*args, **kwargs): + yield "data: {\"type\": \"tool_output\", \"tool\": \"bash\"}\n\n" + yield "data: {\"type\": \"text\", \"delta\": \"Teacher reply\"}\n\n" + yield "data: [DONE]\n\n" + monkeypatch.setattr("src.agent_loop.stream_agent_loop", fake_stream_agent_loop) + + # Mock _call_teacher returning a skill definition + async def fake_call_teacher(spec, prompt, owner=None): + return '```json\n{"action": "add", "name": "test-skill"}\n```' + monkeypatch.setattr("src.teacher_escalation._call_teacher", fake_call_teacher) + + # Mock do_manage_skills + async def fake_do_manage_skills(skill_json, owner=None): + return {"success": True} + monkeypatch.setattr("src.tool_implementations.do_manage_skills", fake_do_manage_skills) + + events = [] + async for evt in teacher_escalation.run_teacher_inline( + student_endpoint_url="http://student.local/v1", + student_messages=[{"role": "user", "content": "test request"}], + student_tool_events=[], + student_reply="student reply", + owner="alice", + ): + events.append(evt) + + # Make sure teacher takeover was announced and executed + assert any("teacher_takeover" in evt for evt in events) + assert any("tool_output" in evt for evt in events) + assert any("skill_saved" in evt for evt in events) + + +@pytest.mark.asyncio +async def test_run_teacher_inline_tier2_disabled_by_default(monkeypatch): + # Settings and gates (Tier 2 disabled) + monkeypatch.setattr("src.settings.get_setting", lambda key, default=None: {"teacher_enabled": True, "teacher_model": "teacher-model", "teacher_tier2_enabled": False}.get(key, default)) + + # Regex evaluation says "ok" + monkeypatch.setattr("src.teacher_escalation.evaluate_turn_regex", lambda *args: ("ok", None)) + + events = [] + async for evt in teacher_escalation.run_teacher_inline( + student_endpoint_url="http://student.local/v1", + student_messages=[{"role": "user", "content": "test request"}], + student_tool_events=[], + student_reply="student reply", + owner="alice", + ): + events.append(evt) + + # Should exit early without any events (no takeover) + assert len(events) == 0 diff --git a/tests/test_toast_dismiss_pointer_events.py b/tests/test_toast_dismiss_pointer_events.py new file mode 100644 index 000000000..52676ff88 --- /dev/null +++ b/tests/test_toast_dismiss_pointer_events.py @@ -0,0 +1,155 @@ +"""Guard that toast dismissal (via the × close button) correctly resets +pointer-events so the invisible fixed overlay does not block clicks. + +The reviewer flagged that action-toasts set ``pointer-events: auto`` on +``#toast`` for their clickable button, but the close-button dismiss path +was cancelling the auto-hide timer without resetting ``pointer-events``. +This left an invisible element intercepting mouse/touch events. + +These are source-level assertions (no browser, no DOM) that verify the +close-button handler includes the reset. They cover: + • ordinary (plain text) toast – showToast + • error toast – showError + • action toast – showToast with action opts +""" +import re +from pathlib import Path + +_REPO = Path(__file__).resolve().parent.parent +_UI_PATH = _REPO / "static" / "js" / "ui.js" + + +def _read_ui(): + return _UI_PATH.read_text(encoding="utf-8") + + +# --------------------------------------------------------------------------- +# Helpers – extract the close-button event-handler bodies from each function. +# --------------------------------------------------------------------------- + +def _extract_function(src: str, func_name: str) -> str: + """Return the full body of *func_name* (exported or not).""" + # Match export function showToast(… or function showToast(… + pat = re.compile( + rf"(?:export\s+)?function\s+{re.escape(func_name)}\s*\(", re.DOTALL + ) + m = pat.search(src) + assert m, f"could not find function {func_name!r} in ui.js" + start = m.start() + # Walk forward counting braces to find the matching closing brace. + depth = 0 + for i in range(start, len(src)): + if src[i] == "{": + depth += 1 + elif src[i] == "}": + depth -= 1 + if depth == 0: + return src[start : i + 1] + raise AssertionError(f"unbalanced braces for {func_name}") + + +def _extract_close_handler(func_body: str) -> str: + """Return the close-button click-handler body inside *func_body*. + + Looks for the ``toast-close-btn`` class assignment, then finds the + ``addEventListener('click'`` call that follows, and extracts the arrow + function body. + """ + idx = func_body.find("toast-close-btn") + assert idx != -1, "toast-close-btn not found in function body" + # Find the addEventListener('click', … that follows + listen_idx = func_body.find("addEventListener('click'", idx) + if listen_idx == -1: + listen_idx = func_body.find('addEventListener("click"', idx) + assert listen_idx != -1, "addEventListener('click') not found after toast-close-btn" + + # Find the opening brace of the handler + brace = func_body.find("{", listen_idx) + assert brace != -1 + depth = 0 + for i in range(brace, len(func_body)): + if func_body[i] == "{": + depth += 1 + elif func_body[i] == "}": + depth -= 1 + if depth == 0: + return func_body[brace : i + 1] + raise AssertionError("unbalanced braces in close handler") + + +# --------------------------------------------------------------------------- +# Tests +# --------------------------------------------------------------------------- + +def test_showToast_close_handler_resets_pointer_events(): + """showToast's × handler must clear pointer-events so an action-toast + that set them to 'auto' doesn't leave the overlay blocking clicks.""" + src = _read_ui() + body = _extract_function(src, "showToast") + handler = _extract_close_handler(body) + assert "pointerEvents" in handler, ( + "showToast close-button handler does not reset pointerEvents – " + "action toasts will leave an invisible click-blocking overlay" + ) + + +def test_showError_close_handler_resets_pointer_events(): + """showError's × handler must also clear pointer-events defensively, + in case a prior action-toast left them as 'auto'.""" + src = _read_ui() + body = _extract_function(src, "showError") + handler = _extract_close_handler(body) + assert "pointerEvents" in handler, ( + "showError close-button handler does not reset pointerEvents – " + "a prior action toast could leave the overlay blocking clicks" + ) + + +def test_showToast_timer_resets_pointer_events(): + """The auto-hide timer in showToast must also reset pointer-events. + This was already in place before the × button was added; make sure + it stays.""" + src = _read_ui() + body = _extract_function(src, "showToast") + # The _hideTimer setTimeout body should contain the reset + timer_idx = body.find("_hideTimer") + assert timer_idx != -1, "no _hideTimer found in showToast" + # Find the setTimeout callback after the last _hideTimer assignment + last_timer = body.rfind("_hideTimer = setTimeout") + assert last_timer != -1 + # Extract the setTimeout callback body + brace = body.find("{", last_timer) + depth = 0 + timer_body = "" + for i in range(brace, len(body)): + if body[i] == "{": + depth += 1 + elif body[i] == "}": + depth -= 1 + if depth == 0: + timer_body = body[brace : i + 1] + break + assert "pointerEvents" in timer_body, ( + "showToast auto-hide timer no longer resets pointerEvents" + ) + + +def test_action_toast_sets_pointer_events_auto(): + """When an action button is present the toast must set pointer-events + to 'auto' so the button is clickable.""" + src = _read_ui() + body = _extract_function(src, "showToast") + assert "pointerEvents = 'auto'" in body or 'pointerEvents = "auto"' in body, ( + "showToast no longer sets pointer-events:auto for action toasts" + ) + + +def test_plain_toast_clears_pointer_events(): + """When there is NO action button, showToast must clear any leftover + pointer-events from a previous action toast.""" + src = _read_ui() + body = _extract_function(src, "showToast") + # The else-branch of the action check should reset pointerEvents + assert "pointerEvents = ''" in body or 'pointerEvents = ""' in body, ( + "showToast does not clear pointer-events for non-action toasts" + ) diff --git a/tests/test_tool_implementations_shim.py b/tests/test_tool_implementations_shim.py new file mode 100644 index 000000000..8180b7a44 --- /dev/null +++ b/tests/test_tool_implementations_shim.py @@ -0,0 +1,165 @@ +"""Protection test: the tool_implementations compatibility shim must keep +re-exporting every symbol importers depend on. + +Guards the slice-1 split (tool_implementations.py -> src/tools/*) from +accidentally dropping a symbol. The contract is enforced by two +self-verifying tests, not by the hand-maintained list below: + +* ``test_shim_reexports_every_domain_do_function`` discovers every ``do_*`` + from the domain modules and asserts reachability through the shim. +* ``test_every_facade_import_in_repo_resolves`` discovers every + ``from src.tool_implementations import X`` site across first-party Python + dirs (src/, tests/, routes/, ...) and asserts ``X`` resolves through the + shim. + +Both fail automatically if a re-export is forgotten (the do_* discovery +covers the tool surface; the import-site scan covers underscore helpers a +reviewer's P3 finding showed could otherwise slip through the list). The +``_EXPECTED`` list below is the curated historical surface (the original +module's top-level names), kept as a belt-and-suspenders check and as the +async-shape contract for ``do_*``; it is not the ground truth. +""" + +import inspect + +import src.tool_implementations as ti + +# 33 do_* tool functions +_EXPECTED = [ + "do_adopt_served_model", "do_api_call", "do_app_api", "do_cancel_download", + "do_download_model", "do_edit_image", "do_list_cached_models", + "do_list_cookbook_servers", "do_list_downloads", "do_list_served_models", + "do_list_serve_presets", "do_manage_calendar", "do_manage_contact", + "do_manage_endpoints", "do_manage_mcp", "do_manage_notes", + "do_manage_research", "do_manage_settings", "do_manage_skills", + "do_manage_tasks", "do_manage_tokens", "do_manage_webhooks", + "do_resolve_contact", "do_search_chats", "do_search_hf_models", + "do_serve_model", "do_serve_preset", "do_stop_served_model", + "do_tail_serve_output", "do_trigger_research", "do_vault_get", + "do_vault_search", "do_vault_unlock", + # module-private helpers (importable by name too) + "_cookbook_apply_retry_suggestion", "_cookbook_env_for_host", + "_cookbook_kill_session", "_cookbook_register_task", "_cookbook_servers", + "_ensure_served_endpoint", "_infer_serve_host", "_infer_serve_port", + "_internal_headers", "_load_vault_config", "_mcp_allowed_commands", + "_parse_tool_args", "_resolve_cookbook_host", "_run_bw", + "_scan_running_model_processes", "_skill_dump", "_string_arg", + "_validate_cookbook_ssh_target", + # active-email facade helpers (no do_* prefix); consumed by + # routes/chat_routes.py — listed here because get_active_email has no + # in-repo importer, so the import-site scan below can't see it alone. + "set_active_email", "get_active_email", "clear_active_email", +] + + +def test_shim_reexports_all_top_level_symbols(): + """Every original top-level function must remain importable via the module.""" + missing = [name for name in _EXPECTED if not hasattr(ti, name)] + assert not missing, f"shim dropped symbols: {missing}" + + +def test_do_functions_remain_async_through_shim(): + """Every do_* must remain a coroutine function through the shim.""" + for name in _EXPECTED: + if name.startswith("do_"): + obj = getattr(ti, name) + assert inspect.iscoroutinefunction(obj), ( + f"{name} is not async via shim (got {type(obj).__name__})" + ) + + +# Domain modules that own tool implementations after the slice-1 split. +# The shim must re-export every public do_* from each so existing +# `from src.tool_implementations import do_X` imports keep resolving. +_DOMAIN_MODULES = ( + "src.tools.system", + "src.tools.cookbook", + "src.tools.search", + "src.tools.notes", + "src.tools.calendar", + "src.tools.image", + "src.tools.research", + "src.tools.contacts", + "src.tools.vault", + "src.agent_tools.admin_tools", # admin manage_* tools migrated here (#3629) +) + + +def test_shim_reexports_every_domain_do_function(): + """Auto-discovered guard: every do_* defined in a domain module must be + reachable through the shim. + + The hand-maintained ``_EXPECTED`` list above can drift silently when a + new tool is added to a domain module but not re-exported by the facade + (exactly the omission a reviewer found post-split). This test discovers + the ground truth from the domain modules themselves, so a forgotten + re-export fails the build automatically. ``hasattr`` is used (not + ``dir(ti)``) because the admin symbols are re-exported lazily via + module ``__getattr__`` and therefore do not appear in ``dir(ti)``. + """ + import importlib + + dropped = [] + for mod_name in _DOMAIN_MODULES: + mod = importlib.import_module(mod_name) + for name in dir(mod): + if not name.startswith("do_"): + continue + if not inspect.iscoroutinefunction(getattr(mod, name, None)): + continue + if not hasattr(ti, name): + dropped.append(f"{mod_name}.{name}") + assert not dropped, f"shim dropped domain do_* (re-export forgotten): {dropped}" + + +def test_every_facade_import_in_repo_resolves(): + """Every ``from src.tool_implementations import X`` in any first-party + Python dir (src/, tests/, routes/, ...) must resolve through the shim. + + This makes the module-docstring contract ("existing ``from + src.tool_implementations import X`` imports keep working") self-verifying + instead of reliant on the hand-maintained ``_EXPECTED`` list, which + omitted three underscore helpers in a reviewer's P3 finding and can drift + again. The import sites are enumerated with ``ast`` rather than checked + at runtime because the invariant is *which names the rest of the + codebase asks the facade for* — no runtime hook enumerates that set, + only the import statements do (the narrow source-scanning exception to + the behavioral-first rule). The per-name assertion is runtime + (``hasattr``), so any forgotten re-export — helper or ``do_*`` — fails + here automatically. + """ + import ast + import os + from pathlib import Path + + repo = Path(__file__).resolve().parents[1] + # Walk every first-party Python dir so route-level (and any future) + # facade consumers are covered, not just src/ and tests/. Prune + # non-source trees (venvs, caches, data, build artifacts) in-place. + _SKIP_DIRS = { + "__pycache__", "venv", "node_modules", "data", "logs", + "odysseus.egg-info", "static", "specs", "licenses", "docker", + } + names = set() + for root, _dirs, files in os.walk(repo): + _dirs[:] = [d for d in _dirs if not (d.startswith(".") or d in _SKIP_DIRS)] + for fn in files: + if not fn.endswith(".py"): + continue + path = Path(root) / fn + text = path.read_text(encoding="utf-8") + if "src.tool_implementations" not in text: + continue + try: + tree = ast.parse(text, filename=str(path)) + except SyntaxError: + continue # unrelated to the facade contract + for node in ast.walk(tree): + if isinstance(node, ast.ImportFrom) and node.module == "src.tool_implementations": + for alias in node.names: + if alias.name != "*": + names.add(alias.name) + unresolved = sorted(n for n in names if not hasattr(ti, n)) + assert not unresolved, ( + f"facade consumers import names the shim does not re-export: {unresolved}" + ) diff --git a/tests/test_tool_index_keyword_boundaries.py b/tests/test_tool_index_keyword_boundaries.py index be4dc5b58..4231fcf6b 100644 --- a/tests/test_tool_index_keyword_boundaries.py +++ b/tests/test_tool_index_keyword_boundaries.py @@ -55,3 +55,10 @@ def test_genuine_keywords_still_force_include(): assert "reply_to_email" in ti.get_tools_for_query("reply to this email") assert "edit_document" in ti.get_tools_for_query("edit the document") assert "serve_model" in ti.get_tools_for_query("serve the model") + + +def test_find_info_online_forces_web_search_tools(): + ti = _index() + tools = ti.get_tools_for_query("find info online about crow box designs") + assert "web_search" in tools + assert "web_fetch" in tools diff --git a/tests/test_tool_path_confinement.py b/tests/test_tool_path_confinement.py index 6288623c4..f9f1bd03f 100644 --- a/tests/test_tool_path_confinement.py +++ b/tests/test_tool_path_confinement.py @@ -57,6 +57,27 @@ def test_non_sensitive_path(): assert not _is_sensitive_path("/home/user/projects/file.py") +def test_sensitive_case_insensitive(): + """On case-insensitive filesystems (Windows, default macOS) a case-variant + name resolves to the same protected file, so the deny-list must match + regardless of case. Built with os.path.join so the separator is right on + both POSIX and Windows. + """ + from src.tool_execution import _is_sensitive_path + # sensitive directory, varied case + assert _is_sensitive_path(os.path.join("home", "u", ".SSH", "authorized_keys")) + assert _is_sensitive_path(os.path.join("home", "u", ".Gnupg", "pubring.kbx")) + # sensitive filename, varied case + assert _is_sensitive_path(os.path.join("ws", "AUTHORIZED_KEYS")) + assert _is_sensitive_path(os.path.join("ws", "Id_Rsa")) + assert _is_sensitive_path(os.path.join("ws", ".ENV")) + assert _is_sensitive_path(os.path.join("ws", ".Env")) + # both dir and file varied + assert _is_sensitive_path(os.path.join("home", "u", ".SSH", "AUTHORIZED_KEYS")) + # an ordinary file with none of the sensitive names is still allowed + assert not _is_sensitive_path(os.path.join("ws", "Readme.md")) + + # ── Unit tests on _resolve_tool_path ───────────────────────────────── def test_blocks_etc_shadow(): diff --git a/tests/test_tool_policy.py b/tests/test_tool_policy.py index 177a667a4..3664b0625 100644 --- a/tests/test_tool_policy.py +++ b/tests/test_tool_policy.py @@ -6,7 +6,12 @@ from types import SimpleNamespace import src.agent_loop as al from src.agent_tools import ToolBlock from src.tool_execution import execute_tool_block -from src.tool_policy import build_effective_tool_policy, detect_guide_only_turn +from src.tool_policy import ( + WEB_TOOL_NAMES, + build_effective_tool_policy, + detect_guide_only_turn, + web_search_enabled_for_turn, +) def _collect(gen): @@ -76,6 +81,116 @@ def test_normal_policy_preserves_existing_disabled_tools(): assert not policy.blocks("bash") +def test_web_search_enabled_for_turn_requires_explicit_enable(): + assert web_search_enabled_for_turn(None, None) is False + assert web_search_enabled_for_turn("true", None) is True + assert web_search_enabled_for_turn(None, "true") is True + assert web_search_enabled_for_turn(True, None) is True + assert web_search_enabled_for_turn("false", "true") is False + assert web_search_enabled_for_turn(False, "true") is False + + +def _schema_names(tools): + return { + tool.get("function", {}).get("name") or tool.get("name") + for tool in (tools or []) + } + + +def test_agent_loop_web_intent_preserves_disabled_web_tools(monkeypatch): + _patch_loop_basics(monkeypatch) + sent_tools = [] + + async def _fake_stream(_candidates, messages, **kwargs): + sent_tools.append(kwargs.get("tools")) + yield _delta_chunk("ok") + yield "data: [DONE]\n\n" + + monkeypatch.setattr(al, "stream_llm_with_fallback", _fake_stream, raising=False) + + _collect( + al.stream_agent_loop( + "https://api.openai.com/v1", + "gpt-test", + [{"role": "user", "content": "please look up the latest CVEs"}], + max_rounds=1, + relevant_tools=set(), + disabled_tools=set(WEB_TOOL_NAMES), + ) + ) + + assert sent_tools + assert WEB_TOOL_NAMES.isdisjoint(_schema_names(sent_tools[0])) + + +def test_agent_loop_forced_web_tools_filtered_by_disabled_tools(monkeypatch): + _patch_loop_basics(monkeypatch) + sent_tools = [] + + async def _fake_stream(_candidates, messages, **kwargs): + sent_tools.append(kwargs.get("tools")) + yield _delta_chunk("ok") + yield "data: [DONE]\n\n" + + monkeypatch.setattr(al, "stream_llm_with_fallback", _fake_stream, raising=False) + + _collect( + al.stream_agent_loop( + "https://api.openai.com/v1", + "gpt-test", + [{"role": "user", "content": "latest Kubernetes release"}], + max_rounds=1, + relevant_tools=set(), + forced_tools=set(WEB_TOOL_NAMES), + disabled_tools=set(WEB_TOOL_NAMES), + ) + ) + + assert sent_tools + assert WEB_TOOL_NAMES.isdisjoint(_schema_names(sent_tools[0])) + + +def test_agent_loop_policy_blocks_disabled_web_tool_call_before_execution(monkeypatch): + _patch_loop_basics(monkeypatch) + called = False + + async def _fake_exec(*args, **kwargs): + nonlocal called + called = True + return ("web_search", {"output": "ran", "exit_code": 0}) + + async def _fake_stream(_candidates, messages, **kwargs): + yield _delta_chunk('```web_search\n{"query":"current CVEs"}\n```') + yield "data: [DONE]\n\n" + + monkeypatch.setattr(al, "execute_tool_block", _fake_exec, raising=False) + monkeypatch.setattr(al, "stream_llm_with_fallback", _fake_stream, raising=False) + + policy = build_effective_tool_policy( + disabled_tools=WEB_TOOL_NAMES, + last_user_message="please look up the latest CVEs", + ) + chunks = _collect( + al.stream_agent_loop( + "http://local.test/v1", + "local-model", + [{"role": "user", "content": "please look up the latest CVEs"}], + max_rounds=1, + relevant_tools={"web_search"}, + disabled_tools=set(policy.all_disabled_names()), + tool_policy=policy, + ) + ) + events = _events(chunks) + blocked = [event for event in events if event.get("type") == "tool_output"] + + assert called is False + assert not any(event.get("type") == "tool_start" for event in events) + assert blocked + assert blocked[0]["tool"] == "web_search" + assert blocked[0]["exit_code"] == 1 + + def test_executor_policy_backstop_blocks_tools(): policy = build_effective_tool_policy(last_user_message="Do not use tools.") desc, result = asyncio.run( @@ -297,6 +412,36 @@ def test_guide_only_suppresses_active_document_context(monkeypatch): assert "Relevant skills" not in prompt_payloads[0] +def test_document_my_style_does_not_infer_public_persona(monkeypatch): + _patch_loop_basics(monkeypatch) + monkeypatch.setattr(al, "_build_base_prompt", lambda *a, **k: ("BASE", ""), raising=False) + monkeypatch.setattr(al, "_cached_base_prompt", None, raising=False) + monkeypatch.setattr(al, "_cached_base_prompt_key", None, raising=False) + + import src.settings as settings + monkeypatch.setattr(settings, "load_settings", lambda: {"document_writing_style": ""}, raising=False) + + active_doc = SimpleNamespace( + id="doc-style", + current_content="A short poem already exists here.", + title="Morning Poem", + language="markdown", + ) + + messages, _ = al._build_system_prompt( + [{"role": "user", "content": "Write as my style"}], + model="local-model", + active_document=active_doc, + mcp_mgr=None, + relevant_tools={"edit_document", "update_document"}, + suppress_skills=True, + ) + payload = "\n\n".join(str(msg.get("content", "")) for msg in messages) + + assert "There is no saved document writing style" in payload + assert "do NOT infer that style from memories, identity, public persona" in payload + + def test_guide_only_skips_teacher_escalation(monkeypatch): _patch_loop_basics(monkeypatch) diff --git a/tests/test_tool_task_cancelled_on_disconnect.py b/tests/test_tool_task_cancelled_on_disconnect.py new file mode 100644 index 000000000..cb086dd20 --- /dev/null +++ b/tests/test_tool_task_cancelled_on_disconnect.py @@ -0,0 +1,92 @@ +"""Regression: the tool-execution task inside stream_agent_loop must be +cancelled (not orphaned) when the SSE consumer stops draining the generator +early — e.g. a client disconnect mid tool-call. + +The drain loop in stream_agent_loop: + + _tool_task = asyncio.create_task(_run_tool()) + while True: + evt = await _progress_q.get() + if evt is None: + break + yield ... + desc, result = await _tool_task + +used to have no try/finally around it. If the generator is closed while +suspended on `await _progress_q.get()` (which is exactly what Starlette does +via `aclose()` when an SSE client disconnects), GeneratorExit is thrown at +that point and `_tool_task` is abandoned mid-flight — never awaited, never +cancelled. For a long-running `bash`/`python` tool this orphans the +subprocess server-side with nothing left to reap it. + +The fix wraps the drain loop in try/finally and cancels+awaits `_tool_task` +on early exit. This test drives the real stream_agent_loop with a fake tool +handler that sleeps until cancelled, closes the generator mid-tool-call (the +same way a dropped SSE connection would), and asserts the fake handler +actually observed cancellation. +""" +import asyncio +import json + +import src.agent_loop as al + + +def test_tool_task_cancelled_on_generator_close(monkeypatch): + cancelled = {"v": False} + + async def _slow_exec(block, *a, progress_cb=None, **k): + if progress_cb: + await progress_cb({"elapsed_s": 1, "tail": "running"}) + try: + await asyncio.sleep(60) + except asyncio.CancelledError: + cancelled["v"] = True + raise + return ("bash", {"output": "ok", "exit_code": 0}) + + monkeypatch.setattr(al, "get_setting", lambda key, default=None: default, raising=False) + monkeypatch.setattr(al, "get_mcp_manager", lambda: None, raising=False) + monkeypatch.setattr(al, "estimate_tokens", lambda *a, **k: 10, raising=False) + monkeypatch.setattr(al, "execute_tool_block", _slow_exec, raising=False) + + native_calls = [{"name": "bash", "arguments": json.dumps({"command": "sleep 60"})}] + + async def _fake_stream(_candidates, messages, **kwargs): + yield f'data: {json.dumps({"delta": "Running it now."})}\n\n' + yield f'data: {json.dumps({"type": "tool_calls", "calls": native_calls})}\n\n' + yield "data: [DONE]\n\n" + + monkeypatch.setattr(al, "stream_llm_with_fallback", _fake_stream, raising=False) + + async def _run(): + gen = al.stream_agent_loop( + "https://api.openai.com/v1", "gpt-4o", + [{"role": "user", "content": "run sleep 60"}], + max_rounds=2, + relevant_tools={"bash"}, + ) + saw_tool_start = False + saw_tool_progress = False + async for chunk in gen: + if '"type": "tool_start"' in chunk: + saw_tool_start = True + elif '"type": "tool_progress" ' in chunk or '"type": "tool_progress"' in chunk: + saw_tool_progress = True + break + assert saw_tool_start, "expected a tool_start event before the tool ran" + assert saw_tool_progress, "expected a tool_progress event once the fake tool started (task must exist by now)" + # Simulate an SSE client disconnecting mid tool-call: close the + # generator while it is suspended awaiting the next progress event. + await gen.aclose() + # Assert *inside* this coroutine, immediately after aclose() returns. + # asyncio.run()'s own shutdown sequence cancels any tasks still + # pending once _run() itself completes — checking after asyncio.run() + # returns would pass even with the bug, because that unrelated + # cleanup would cancel the orphaned task anyway and mask the fix. + assert cancelled["v"] is True, ( + "tool task must be cancelled by stream_agent_loop's own cleanup " + "on generator close, not left running until asyncio.run() tears " + "down the loop" + ) + + asyncio.run(_run()) diff --git a/tests/test_tts_available_nonstring_provider.py b/tests/test_tts_available_nonstring_provider.py new file mode 100644 index 000000000..632e1cd89 --- /dev/null +++ b/tests/test_tts_available_nonstring_provider.py @@ -0,0 +1,17 @@ +from services.tts.tts_service import TTSService + + +def test_available_tolerates_non_string_provider(tmp_path): + """A hand-edited/corrupt data/settings.json can store a non-string + tts_provider (e.g. null or a number). available reads it and calls + provider.startswith("endpoint:"), which raised AttributeError on a + non-str. It must instead fall through and report unavailable.""" + service = TTSService(cache_dir=str(tmp_path)) + service._load_settings = lambda: { + "tts_enabled": True, + "tts_provider": 123, + "tts_model": "tts-1", + "tts_voice": "alloy", + "tts_speed": "1", + } + assert service.available is False diff --git a/tests/test_upload_content_detection_magic.py b/tests/test_upload_content_detection_magic.py new file mode 100644 index 000000000..d5ae6a350 --- /dev/null +++ b/tests/test_upload_content_detection_magic.py @@ -0,0 +1,46 @@ +"""Regression for #4875: the official Docker image shipped without python-magic +(and without the libmagic system lib), so content-based MIME detection in +src/upload_handler.py was dead and uploads were typed by extension only. + +python-magic resolves libmagic at import time and can block/raise when the lib +is absent, so it's installed in the Docker image (which always has libmagic1) +rather than in the shared requirements.txt. These tests pin: + 1. the Dockerfile installs both libmagic1 (apt) and python-magic (pip); + 2. when libmagic is actually present, detect_content_type sniffs the MIME + from the bytes and overrides a misleading/missing extension. +""" +import io +import os + +import pytest + +from src.upload_handler import UploadHandler + +# 1x1 PNG (header is enough for libmagic to report image/png). +_PNG = ( + b"\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01" + b"\x08\x06\x00\x00\x00\x1f\x15\xc4\x89\x00\x00\x00\nIDATx\x9cc\x00" + b"\x01\x00\x00\x05\x00\x01\r\n-\xb4\x00\x00\x00\x00IEND\xaeB`\x82" +) + +_REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + + +def test_dockerfile_installs_libmagic_and_python_magic(): + with open(os.path.join(_REPO_ROOT, "Dockerfile"), encoding="utf-8") as f: + dockerfile = f.read() + # The C library python-magic dlopens, installed via apt... + assert "libmagic1" in dockerfile + # ...and the wrapper itself, installed via pip in the image. + assert "python-magic" in dockerfile + + +def test_content_detection_overrides_misleading_extension(tmp_path): + handler = UploadHandler(base_dir=str(tmp_path), upload_dir=str(tmp_path)) + if handler.file_detector is None: + pytest.skip("libmagic/python-magic not installed in this environment") + + # PNG bytes behind a .bin name: extension sniffing can't help, so a correct + # image/png result proves content-based detection is doing the work. + detected = handler.detect_content_type(io.BytesIO(_PNG), "payload.bin") + assert detected == "image/png" diff --git a/tests/test_upload_error_surfaced.py b/tests/test_upload_error_surfaced.py index 1eb267999..4e5be7763 100644 --- a/tests/test_upload_error_surfaced.py +++ b/tests/test_upload_error_surfaced.py @@ -17,7 +17,7 @@ SRC = Path(__file__).resolve().parent.parent / "static/js/fileHandler.js" def _upload_pending_body() -> str: text = SRC.read_text(encoding="utf-8") - start = text.index("export async function uploadPending()") + start = text.index("export async function uploadPending(") rest = text[start:] m = re.search(r"\n(export |function )", rest[1:]) return rest[: m.start() + 1] if m else rest diff --git a/tests/test_upload_handler_cleanup.py b/tests/test_upload_handler_cleanup.py new file mode 100644 index 000000000..9810c2b20 --- /dev/null +++ b/tests/test_upload_handler_cleanup.py @@ -0,0 +1,831 @@ +import asyncio +import concurrent.futures +import json +import os +import threading +from datetime import datetime +from pathlib import Path +from types import SimpleNamespace + +import pytest +from fastapi import HTTPException + +from core.database import ( + Base, + ChatMessage as DbChatMessage, + CalendarCal, + CalendarEvent, + Document, + DocumentVersion, + GalleryImage, + Note, + Session as DbSession, +) +from src.upload_handler import ( + UploadCleanupSafetyError, + UploadHandler, + extract_internal_upload_ids, + reserve_message_upload_references, + reserve_upload_references, +) +from tests.helpers.sqlite_db import make_temp_sqlite + + +OLD_TIMESTAMP = "2000-01-01T00:00:00" + + +class _AdminAuth: + is_configured = True + + @staticmethod + def is_admin(user): + return user == "admin" + + +class _AdminRequest: + headers = {} + state = SimpleNamespace(current_user="admin") + app = SimpleNamespace(state=SimpleNamespace(auth_manager=_AdminAuth())) + + +def _make_handler(tmp_path: Path) -> UploadHandler: + base_dir = tmp_path / "base" + upload_dir = tmp_path / "uploads" + base_dir.mkdir() + upload_dir.mkdir() + return UploadHandler(str(base_dir), str(upload_dir)) + + +def _seed_old_uploads(handler: UploadHandler, rows: list[dict]) -> dict[str, Path]: + dated_dir = Path(handler.upload_dir) / "2000" / "01" / "01" + dated_dir.mkdir(parents=True) + index = {} + paths = {} + for row in rows: + upload_id = row["id"] + path = dated_dir / upload_id + path.write_bytes(row.get("bytes", upload_id.encode("ascii"))) + info = { + "id": upload_id, + "path": str(path), + "mime": row.get("mime", "application/octet-stream"), + "size": path.stat().st_size, + "name": row.get("name", upload_id), + "original_name": row.get("name", upload_id), + "hash": row["hash"], + "checksum_sha256": row["hash"], + "uploaded_at": row.get("uploaded_at", OLD_TIMESTAMP), + "created_at": row.get("created_at", OLD_TIMESTAMP), + "last_accessed": row.get("last_accessed", OLD_TIMESTAMP), + "owner": row.get("owner", "alice"), + } + index[f"{info['owner']}:{info['hash']}"] = info + paths[upload_id] = path + + Path(handler.upload_dir, "uploads.json").write_text( + json.dumps(index), + encoding="utf-8", + ) + handler._index_cache = None + return paths + + +def _manual_cleanup_endpoint(handler: UploadHandler, monkeypatch): + import fastapi.dependencies.utils as dependency_utils + from routes.upload_routes import router, setup_upload_routes + + monkeypatch.setattr(dependency_utils, "ensure_multipart_is_installed", lambda: None) + before = len(router.routes) + setup_upload_routes(handler) + return { + route.endpoint.__name__: route.endpoint + for route in router.routes[before:] + }["manual_cleanup"] + + +def _reference_database(monkeypatch, *, upload_id: str, gallery_hash: str = None): + from routes import upload_routes + + SessionLocal, engine, tmpfile = make_temp_sqlite(Base.metadata) + db = SessionLocal() + try: + db.add(DbSession( + id="session-1", + name="Cleanup regression", + endpoint_url="http://localhost", + model="test-model", + owner="alice", + )) + db.add(DbChatMessage( + id="message-1", + session_id="session-1", + role="user", + content=f"[Attachment: retained.png | id={upload_id} | mime=image/png]", + meta_data=json.dumps({ + "attachments": [{ + "id": upload_id, + "name": "retained.png", + "mime": "image/png", + "size": 8, + }] + }), + )) + if gallery_hash: + db.add(GalleryImage( + id="gallery-cleanup-reference", + filename="abcdef123456.png", + prompt="Chat upload", + owner="alice", + file_hash=gallery_hash, + )) + db.commit() + finally: + db.close() + + monkeypatch.setattr(upload_routes, "SessionLocal", SessionLocal) + return engine, tmpfile + + +def test_admin_cleanup_preserves_referenced_upload_and_reconciles_deleted_row( + tmp_path, + monkeypatch, +): + handler = _make_handler(tmp_path) + referenced_id = "a" * 32 + ".png" + unreferenced_id = "b" * 32 + ".txt" + gallery_id = "7" * 32 + ".png" + gallery_hash = "7" * 64 + paths = _seed_old_uploads(handler, [ + { + "id": referenced_id, + "hash": "1" * 64, + "mime": "image/png", + }, + { + "id": unreferenced_id, + "hash": "2" * 64, + "mime": "text/plain", + }, + { + "id": gallery_id, + "hash": gallery_hash, + "mime": "image/png", + }, + ]) + engine, tmpfile = _reference_database( + monkeypatch, + upload_id=referenced_id, + gallery_hash=gallery_hash, + ) + + try: + response = asyncio.run( + _manual_cleanup_endpoint(handler, monkeypatch)(_AdminRequest()) + ) + finally: + engine.dispose() + tmpfile.close() + try: + os.unlink(tmpfile.name) + except OSError: + pass + + assert response == {"status": "success", "files_cleaned": 1} + assert paths[referenced_id].is_file() + referenced_info = handler.get_upload_info(referenced_id) + assert referenced_info is not None + assert handler.resolve_upload(referenced_id, owner="alice") is not None + assert paths[gallery_id].is_file() + assert handler.get_upload_info(gallery_id) is not None + + assert not paths[unreferenced_id].exists() + assert handler.get_upload_info(unreferenced_id) is None + assert handler.resolve_upload(unreferenced_id, owner="alice") is None + + live_index = json.loads( + Path(handler.upload_dir, "uploads.json").read_text(encoding="utf-8") + ) + assert {info["id"] for info in live_index.values()} == { + referenced_id, + gallery_id, + } + backup_index = json.loads( + Path(handler.upload_dir, "uploads.json.bak").read_text(encoding="utf-8") + ) + assert {info["id"] for info in backup_index.values()} == { + referenced_id, + gallery_id, + } + + # Recovery must not resurrect the deliberately deleted row. + Path(handler.upload_dir, "uploads.json").write_text("{broken", encoding="utf-8") + handler._index_cache = None + assert handler.get_upload_info(unreferenced_id) is None + assert paths[referenced_id].parent.is_dir() + + +def test_cleanup_retains_upload_and_all_rows_when_index_rows_disagree(tmp_path): + handler = _make_handler(tmp_path) + upload_id = "c" * 32 + ".txt" + path = _seed_old_uploads(handler, [{ + "id": upload_id, + "hash": "1" * 64, + "mime": "text/plain", + "owner": "alice", + }])[upload_id] + index_path = Path(handler.upload_dir, "uploads.json") + index = json.loads(index_path.read_text(encoding="utf-8")) + alice_row = next(iter(index.values())) + index["bob:" + "2" * 64] = { + **alice_row, + "owner": "bob", + "hash": "2" * 64, + "checksum_sha256": "2" * 64, + } + index_path.write_text(json.dumps(index), encoding="utf-8") + handler._index_cache = None + + assert handler.cleanup_old_uploads(set(), set()) == 0 + assert path.is_file() + assert json.loads(index_path.read_text(encoding="utf-8")) == index + + +def test_cleanup_retains_lone_row_without_authoritative_lifecycle_metadata(tmp_path): + handler = _make_handler(tmp_path) + upload_id = "6" * 32 + ".txt" + path = _seed_old_uploads(handler, [{ + "id": upload_id, + "hash": "6" * 64, + "mime": "text/plain", + }])[upload_id] + index_path = Path(handler.upload_dir, "uploads.json") + index = json.loads(index_path.read_text(encoding="utf-8")) + row = next(iter(index.values())) + for field in ( + "owner", + "hash", + "checksum_sha256", + "uploaded_at", + "created_at", + "last_accessed", + ): + row.pop(field) + index_path.write_text(json.dumps(index), encoding="utf-8") + handler._index_cache = None + + assert handler.cleanup_old_uploads(set(), set()) == 0 + assert path.is_file() + assert json.loads(index_path.read_text(encoding="utf-8")) == index + + +def test_reservation_and_cleanup_are_serialized_without_dangling_references( + tmp_path, + monkeypatch, +): + # Writer wins: reservation holds the shared index lock, refreshes access, + # then cleanup observes the refreshed row and preserves the file. + writer_root = tmp_path / "writer-wins" + writer_root.mkdir() + writer_handler = _make_handler(writer_root) + upload_id = "2" * 32 + ".txt" + writer_path = _seed_old_uploads(writer_handler, [{ + "id": upload_id, + "hash": "2" * 64, + "mime": "text/plain", + }])[upload_id] + write_entered = threading.Event() + release_write = threading.Event() + real_atomic_write = writer_handler._atomic_write_json + + def blocking_reservation_write(path, data, *, sync_backup=False): + refreshed = any( + isinstance(row, dict) and row.get("last_accessed") != OLD_TIMESTAMP + for row in data.values() + ) + if sync_backup and refreshed and not write_entered.is_set(): + write_entered.set() + assert release_write.wait(5) + return real_atomic_write(path, data, sync_backup=sync_backup) + + monkeypatch.setattr(writer_handler, "_atomic_write_json", blocking_reservation_write) + with concurrent.futures.ThreadPoolExecutor(max_workers=2) as pool: + reserve_future = pool.submit( + writer_handler.reserve_upload, + upload_id, + owner="alice", + ) + assert write_entered.wait(5) + cleanup_future = pool.submit(writer_handler.cleanup_old_uploads, set(), set()) + release_write.set() + assert reserve_future.result(timeout=5) is not None + assert cleanup_future.result(timeout=5) == 0 + assert writer_path.is_file() + + # Cleanup wins: reservation cannot pass the same lock until the row and + # bytes are gone, then fails so a caller cannot commit a dangling reference. + cleanup_root = tmp_path / "cleanup-wins" + cleanup_root.mkdir() + cleanup_handler = _make_handler(cleanup_root) + cleanup_path = _seed_old_uploads(cleanup_handler, [{ + "id": upload_id, + "hash": "3" * 64, + "mime": "text/plain", + }])[upload_id] + remove_entered = threading.Event() + release_remove = threading.Event() + real_remove = os.remove + + def blocking_remove(candidate): + if os.path.realpath(candidate) == os.path.realpath(cleanup_path): + remove_entered.set() + assert release_remove.wait(5) + return real_remove(candidate) + + monkeypatch.setattr("src.upload_handler.os.remove", blocking_remove) + with concurrent.futures.ThreadPoolExecutor(max_workers=2) as pool: + cleanup_future = pool.submit(cleanup_handler.cleanup_old_uploads, set(), set()) + assert remove_entered.wait(5) + reserve_future = pool.submit( + cleanup_handler.reserve_upload, + upload_id, + owner="alice", + ) + release_remove.set() + assert cleanup_future.result(timeout=5) == 1 + assert reserve_future.result(timeout=5) is None + assert not cleanup_path.exists() + + +def test_admin_cleanup_reference_discovery_failure_returns_503_without_deleting( + tmp_path, + monkeypatch, +): + from routes import upload_routes + + handler = _make_handler(tmp_path) + upload_id = "d" * 32 + ".png" + path = _seed_old_uploads(handler, [ + {"id": upload_id, "hash": "4" * 64, "mime": "image/png"}, + ])[upload_id] + + def fail_reference_scan(): + raise RuntimeError("database unavailable") + + monkeypatch.setattr( + upload_routes, + "_collect_persisted_upload_references", + fail_reference_scan, + ) + endpoint = _manual_cleanup_endpoint(handler, monkeypatch) + + with pytest.raises(HTTPException) as exc: + asyncio.run(endpoint(_AdminRequest())) + + assert exc.value.status_code == 503 + assert path.is_file() + assert handler.get_upload_info(upload_id) is not None + + +def test_cleanup_restores_index_when_file_removal_fails(tmp_path, monkeypatch): + handler = _make_handler(tmp_path) + upload_id = "e" * 32 + ".txt" + path = _seed_old_uploads(handler, [ + { + "id": upload_id, + "hash": "5" * 64, + "mime": "text/plain", + }, + ])[upload_id] + + real_remove = os.remove + + def fail_target_remove(candidate): + if os.path.realpath(candidate) == os.path.realpath(path): + raise PermissionError("file is in use") + return real_remove(candidate) + + monkeypatch.setattr("src.upload_handler.os.remove", fail_target_remove) + + assert handler.cleanup_old_uploads(set(), set()) == 0 + assert path.is_file() + assert handler.get_upload_info(upload_id) is not None + assert any( + info["id"] == upload_id + for info in json.loads( + Path(handler.upload_dir, "uploads.json").read_text(encoding="utf-8") + ).values() + ) + assert any( + info["id"] == upload_id + for info in json.loads( + Path(handler.upload_dir, "uploads.json.bak").read_text(encoding="utf-8") + ).values() + ) + + +def test_admin_cleanup_with_corrupt_index_returns_503_and_fails_closed( + tmp_path, + monkeypatch, +): + from routes import upload_routes + + handler = _make_handler(tmp_path) + upload_id = "9" * 32 + ".png" + path = _seed_old_uploads(handler, [ + {"id": upload_id, "hash": "9" * 64, "mime": "image/png"}, + ])[upload_id] + Path(handler.upload_dir, "uploads.json").write_text( + '{"alice:broken": {', + encoding="utf-8", + ) + handler._index_cache = None + + monkeypatch.setattr( + upload_routes, + "_collect_persisted_upload_references", + lambda: (set(), set()), + ) + endpoint = _manual_cleanup_endpoint(handler, monkeypatch) + + with pytest.raises(HTTPException) as exc: + asyncio.run(endpoint(_AdminRequest())) + + assert exc.value.status_code == 503 + assert path.is_file() + + +def test_cleanup_with_missing_live_index_fails_closed(tmp_path): + handler = _make_handler(tmp_path) + upload_id = "8" * 32 + ".png" + dated_dir = Path(handler.upload_dir) / "2000" / "01" / "01" + dated_dir.mkdir(parents=True) + path = dated_dir / upload_id + path.write_bytes(b"unindexed bytes") + + with pytest.raises(UploadCleanupSafetyError): + handler.cleanup_old_uploads(set(), set()) + + assert path.is_file() + + +def test_reference_discovery_covers_all_durable_upload_stores( + monkeypatch, +): + from routes import upload_routes + + document_id = "f" * 32 + ".pdf" + version_id = "1" * 32 + ".pdf" + note_upload_id = "3" * 32 + ".png" + note_color_id = "2" * 32 + ".png" + calendar_upload_id = "4" * 32 + ".png" + event_upload_id = "5" * 32 + ".png" + event_description_id = "7" * 32 + ".txt" + event_location_id = "8" * 32 + ".png" + gallery_hash = "6" * 64 + SessionLocal, engine, tmpfile = make_temp_sqlite(Base.metadata) + db = SessionLocal() + try: + db.add(DbSession( + id="session-2", + name="Reference sources", + endpoint_url="http://localhost", + model="test-model", + owner="alice", + )) + db.add(Document( + id="document-1", + session_id="session-2", + title="PDF", + current_content=f'', + owner="alice", + )) + db.add(DocumentVersion( + id="version-1", + document_id="document-1", + version_number=1, + content=f'', + )) + db.add(GalleryImage( + id="gallery-1", + # Gallery filenames are normally generated 12-hex names, so this + # record proves retention comes from its stored content hash. + filename="abcdef123456.png", + prompt="Chat upload", + owner="alice", + file_hash=gallery_hash, + )) + db.add(Note( + id="note-1", + owner="alice", + title="Photo note", + image_url=f"/api/upload/{note_upload_id}", + color=f"odysseus://attachment/{note_color_id}", + )) + db.add(CalendarCal( + id="calendar-1", + owner="alice", + name="Personal", + color=f"/api/upload/{calendar_upload_id}", + )) + db.add(CalendarEvent( + uid="event-1", + calendar_id="calendar-1", + summary="Photo event", + dtstart=datetime(2026, 7, 10, 12, 0), + dtend=datetime(2026, 7, 10, 13, 0), + color=f"/api/upload/{event_upload_id}", + description=f"Notes: odysseus://attachment/{event_description_id}", + location=f"/api/upload/{event_location_id}", + )) + db.commit() + finally: + db.close() + + monkeypatch.setattr(upload_routes, "SessionLocal", SessionLocal) + try: + referenced_ids, referenced_hashes = ( + upload_routes._collect_persisted_upload_references() + ) + finally: + engine.dispose() + tmpfile.close() + try: + os.unlink(tmpfile.name) + except OSError: + pass + + assert { + document_id, + version_id, + note_upload_id, + note_color_id, + calendar_upload_id, + event_upload_id, + event_description_id, + event_location_id, + } <= referenced_ids + assert gallery_hash in referenced_hashes + + +def test_write_reservation_extracts_only_explicit_internal_references(): + upload_id = "a" * 32 + ".png" + checksum_like_text = "b" * 32 + + assert extract_internal_upload_ids(checksum_like_text) == set() + assert extract_internal_upload_ids(f"sha={checksum_like_text}") == set() + assert extract_internal_upload_ids({ + "image": f"/api/upload/{upload_id}", + "nested": [f"odysseus://attachment/{upload_id}"], + }) == {upload_id} + assert extract_internal_upload_ids( + f'' + ) == {upload_id} + assert extract_internal_upload_ids( + f"[Attachment: photo.png | id={upload_id} | mime=image/png]" + ) == {upload_id} + extensionless_id = "c" * 32 + assert extract_internal_upload_ids( + f"See /api/upload/{extensionless_id}. Then continue." + ) == {extensionless_id} + assert extract_internal_upload_ids( + f"Attachment: odysseus://attachment/{extensionless_id}: ready" + ) == {extensionless_id} + assert extract_internal_upload_ids(f"/api/upload/{upload_id}/extra") == set() + + +def test_reservation_never_uses_admin_override(tmp_path): + handler = _make_handler(tmp_path) + upload_id = "c" * 32 + ".txt" + _seed_old_uploads(handler, [{ + "id": upload_id, + "hash": "c" * 64, + "mime": "text/plain", + "owner": "alice", + }]) + + assert reserve_upload_references( + handler, + "alice", + f"odysseus://attachment/{upload_id}", + ) is None + assert reserve_upload_references( + handler, + "admin", + f"odysseus://attachment/{upload_id}", + ) == upload_id + assert handler.reserve_upload( + upload_id, + owner="admin", + auth_manager=_AdminAuth(), + allow_admin=False, + ) is None + assert reserve_message_upload_references( + handler, + "admin", + "legacy attachment metadata", + {"attachments": [{"id": upload_id, "name": "owned.txt"}]}, + ) == upload_id + + +def test_remaining_durable_writers_reserve_before_commit(monkeypatch): + import core.database as database + import core.session_manager as session_manager_module + import src.database as legacy_database + from core.models import ChatMessage + from core.session_manager import SessionManager + from src import tool_utils + from src.agent_tools.document_tools import EditDocumentTool + from src.tools.calendar import do_manage_calendar + from src.tools.notes import do_manage_notes + + upload_id = "6" * 32 + ".png" + + class RejectingHandler: + def reserve_upload(self, _candidate, **_kwargs): + return None + + handler = RejectingHandler() + monkeypatch.setattr(tool_utils, "_upload_handler", handler) + + SessionLocal, engine, tmpfile = make_temp_sqlite(Base.metadata) + monkeypatch.setattr(database, "SessionLocal", SessionLocal) + monkeypatch.setattr(legacy_database, "SessionLocal", SessionLocal) + monkeypatch.setattr(legacy_database, "Document", Document, raising=False) + monkeypatch.setattr( + legacy_database, + "DocumentVersion", + DocumentVersion, + raising=False, + ) + monkeypatch.setattr(session_manager_module, "SessionLocal", SessionLocal) + db = SessionLocal() + try: + db.add(DbSession( + id="writer-session", + name="Writer coverage", + endpoint_url="http://localhost", + model="test-model", + owner="alice", + )) + db.add(Document( + id="email-document", + session_id="writer-session", + title="New Email", + language="email", + current_content="To: team@example.test\nSubject: Status\n---\nOld body", + version_count=1, + owner="alice", + )) + db.commit() + + manager = SessionManager() + manager.upload_handler = handler + manager._persist_message( + "writer-session", + ChatMessage( + "user", + "attachment", + metadata={"attachments": [{"id": upload_id}]}, + ), + ) + + document_result = asyncio.run(EditDocumentTool().execute( + "<<>>\n\n<<>>\n" + f"See /api/upload/{upload_id}\n<<>>", + {"doc_id": "email-document", "owner": "alice"}, + )) + assert document_result["exit_code"] == 1 + assert "no longer available" in document_result["error"] + + calendar_result = asyncio.run(do_manage_calendar( + json.dumps({ + "action": "create_event", + "summary": "Attachment review", + "dtstart": "2026-07-12T12:00:00", + "description": f"See /api/upload/{upload_id}", + }), + owner="alice", + )) + assert calendar_result["exit_code"] == 1 + assert "no longer available" in calendar_result["error"] + + note_result = asyncio.run(do_manage_notes( + json.dumps({ + "action": "add", + "title": "Attachment note", + "content": f"See /api/upload/{upload_id}", + }), + owner="alice", + )) + assert note_result["exit_code"] == 1 + assert "no longer available" in note_result["error"] + + verify = SessionLocal() + try: + assert verify.query(DbChatMessage).count() == 0 + stored_doc = verify.query(Document).filter(Document.id == "email-document").one() + assert stored_doc.current_content.endswith("Old body") + assert verify.query(CalendarEvent).count() == 0 + assert verify.query(Note).count() == 0 + finally: + verify.close() + finally: + db.close() + engine.dispose() + tmpfile.close() + try: + os.unlink(tmpfile.name) + except OSError: + pass + + +def test_note_calendar_and_document_routes_reserve_before_database_writes(monkeypatch): + from routes.calendar_routes import EventCreate, setup_calendar_routes + from routes import document_routes + from routes.document_helpers import DocumentCreate + from routes.note_routes import NoteCreate, setup_note_routes + from src import auth_helpers + + upload_id = "d" * 32 + ".png" + + class RejectingHandler: + def __init__(self): + self.calls = [] + + def reserve_upload(self, candidate, **kwargs): + self.calls.append((candidate, kwargs)) + return None + + request = SimpleNamespace( + state=SimpleNamespace(current_user="alice", api_token=False), + app=SimpleNamespace(state=SimpleNamespace()), + ) + + note_handler = RejectingHandler() + note_router = setup_note_routes(upload_handler=note_handler) + create_note = next( + route.endpoint for route in note_router.routes + if route.endpoint.__name__ == "create_note" + ) + with pytest.raises(HTTPException) as note_error: + create_note( + request, + NoteCreate(image_url=f"/api/upload/{upload_id}"), + ) + assert note_error.value.status_code == 409 + assert note_handler.calls == [ + (upload_id, {"owner": "alice", "allow_admin": False}) + ] + + calendar_handler = RejectingHandler() + calendar_router = setup_calendar_routes(upload_handler=calendar_handler) + create_event = next( + route.endpoint for route in calendar_router.routes + if route.endpoint.__name__ == "create_event" + ) + with pytest.raises(HTTPException) as calendar_error: + asyncio.run(create_event( + request, + EventCreate( + summary="Photo", + dtstart="2026-07-10T12:00:00", + color=f"odysseus://attachment/{upload_id}", + ), + )) + assert calendar_error.value.status_code == 409 + assert calendar_handler.calls == [ + (upload_id, {"owner": "alice", "allow_admin": False}) + ] + + class EmptyDb: + @staticmethod + def close(): + return None + + document_handler = RejectingHandler() + monkeypatch.setattr(document_routes, "SessionLocal", EmptyDb) + monkeypatch.setattr( + auth_helpers, + "require_privilege", + lambda _request, _privilege: "alice", + ) + document_router = document_routes.setup_document_routes( + SimpleNamespace(), + document_handler, + ) + create_document = next( + route.endpoint for route in document_router.routes + if route.endpoint.__name__ == "create_document" + ) + with pytest.raises(HTTPException) as document_error: + asyncio.run(create_document( + request, + DocumentCreate( + language="markdown", + content=f"![image](/api/upload/{upload_id})", + ), + )) + assert document_error.value.status_code == 409 + assert document_handler.calls == [ + (upload_id, {"owner": "alice", "allow_admin": False}) + ] diff --git a/tests/test_upload_limits_centralized.py b/tests/test_upload_limits_centralized.py index a870228fa..ebce4ca03 100644 --- a/tests/test_upload_limits_centralized.py +++ b/tests/test_upload_limits_centralized.py @@ -80,11 +80,11 @@ def test_non_positive_env_rejected(monkeypatch, env): def test_routes_import_from_upload_limits_not_local_defs(): """Routes must import the constant, not redefine it via raw getenv / literal.""" forbidden = { - "routes/gallery_routes.py": [ + "routes/gallery/gallery_routes.py": [ 'int(os.getenv("ODYSSEUS_GALLERY_UPLOAD_MAX_BYTES"', 'int(os.getenv("ODYSSEUS_GALLERY_TRANSFORM_UPLOAD_MAX_BYTES"', ], - "routes/memory_routes.py": ['int(os.getenv("ODYSSEUS_MEMORY_IMPORT_MAX_BYTES"'], + "routes/memory/memory_routes.py": ['int(os.getenv("ODYSSEUS_MEMORY_IMPORT_MAX_BYTES"'], "routes/personal_routes.py": ['os.getenv("ODYSSEUS_PERSONAL_UPLOAD_MAX_BYTES"'], "routes/email_routes.py": ["EMAIL_COMPOSE_UPLOAD_MAX_BYTES = 25 * 1024 * 1024"], "routes/stt_routes.py": ["STT_MAX_AUDIO_BYTES = 25 * 1024 * 1024"], @@ -97,8 +97,8 @@ def test_routes_import_from_upload_limits_not_local_defs(): # And each imports from upload_limits. imports = { - "routes/gallery_routes.py": "GALLERY_UPLOAD_MAX_BYTES", - "routes/memory_routes.py": "MEMORY_IMPORT_MAX_BYTES", + "routes/gallery/gallery_routes.py": "GALLERY_UPLOAD_MAX_BYTES", + "routes/memory/memory_routes.py": "MEMORY_IMPORT_MAX_BYTES", "routes/personal_routes.py": "PERSONAL_UPLOAD_MAX_BYTES", "routes/email_routes.py": "EMAIL_COMPOSE_UPLOAD_MAX_BYTES", "routes/stt_routes.py": "STT_MAX_AUDIO_BYTES", diff --git a/tests/test_upload_routes_owner_scope.py b/tests/test_upload_routes_owner_scope.py index a2647f580..a29b7c795 100644 --- a/tests/test_upload_routes_owner_scope.py +++ b/tests/test_upload_routes_owner_scope.py @@ -313,3 +313,32 @@ def test_put_vision_text_allows_same_owner_to_write_cache(tmp_path, monkeypatch) assert (upload_dir / ".vision" / f"{alice_id}.txt").read_text( encoding="utf-8" ) == "edited alice text" + + +def test_download_file_survives_corrupted_uploads_json(tmp_path, monkeypatch): + # A truncated/corrupt uploads.json must not 500 the download endpoint — + # metadata simply becomes unavailable and the file is still served. + handler, alice_id, _bob_id, upload_dir = _make_upload_store(tmp_path, monkeypatch) + download_file = _upload_endpoints(handler, monkeypatch)["download_file"] + (upload_dir / "uploads.json").write_text('{"alice:h1": {', encoding="utf-8") + + # No auth configured -> owner gate skipped. + response = asyncio.run(download_file(_Request(), alice_id)) + + assert str(response.path).endswith(alice_id) + # Metadata unreadable, so the display filename falls back to the file_id. + assert response.filename == alice_id + + +def test_put_vision_text_returns_400_on_malformed_json(tmp_path, monkeypatch): + # A non-JSON request body must yield 400, not an unhandled JSONDecodeError -> 500. + handler, alice_id, _bob_id, _upload_dir = _make_upload_store(tmp_path, monkeypatch) + put_vision_text = _upload_endpoints(handler, monkeypatch)["put_vision_text"] + + class _BadJsonRequest(_Request): + async def json(self): + raise json.JSONDecodeError("Expecting value", "not json", 0) + + with pytest.raises(HTTPException) as exc: + asyncio.run(put_vision_text(_BadJsonRequest(), alice_id)) + assert exc.value.status_code == 400 diff --git a/tests/test_url_safety.py b/tests/test_url_safety.py index 8d4a18901..faae6b86b 100644 --- a/tests/test_url_safety.py +++ b/tests/test_url_safety.py @@ -68,3 +68,23 @@ def test_unresolvable_host_blocked(): ok, reason = check_outbound_url("http://does-not-resolve.invalid", resolver=PUBLIC) assert ok is False assert "resolve" in reason + + +def test_resolver_values_must_include_a_parseable_ip(): + ok, reason = check_outbound_url( + "https://example.test", + resolver=lambda _host: [None, 123, "not-an-ip"], + ) + + assert ok is False + assert "does not resolve to an IP" in reason + + +def test_resolver_skips_invalid_values_but_accepts_public_ip(): + ok, reason = check_outbound_url( + "https://example.test", + resolver=lambda _host: [None, "not-an-ip", "93.184.216.34"], + ) + + assert ok is True + assert reason == "ok" diff --git a/tests/test_vault_password_not_in_argv.py b/tests/test_vault_password_not_in_argv.py index 32267a925..f23cddcd8 100644 --- a/tests/test_vault_password_not_in_argv.py +++ b/tests/test_vault_password_not_in_argv.py @@ -102,7 +102,7 @@ def test_unlock_handler_feeds_password_on_stdin_not_argv(): def test_tool_vault_unlock_feeds_password_on_stdin_not_argv(): - text = open("src/tool_implementations.py", encoding="utf-8").read() + text = open("src/tools/vault.py", encoding="utf-8").read() assert '["unlock", master_password, "--raw"]' not in text assert '_run_bw(["unlock", master_password' not in text diff --git a/tests/test_vcard_unfolding.py b/tests/test_vcard_unfolding.py new file mode 100644 index 000000000..fda680338 --- /dev/null +++ b/tests/test_vcard_unfolding.py @@ -0,0 +1,45 @@ +"""vCard parsing must unfold RFC 6350 folded lines. + +CardDAV servers fold logical lines longer than 75 octets onto continuation +lines that begin with a space/tab. _parse_vcards split on raw newlines +without unfolding, so a folded EMAIL/FN line lost its continuation (a long +address like ...@exampledomain.com was stored as ...@exampledomain), +silently corrupting the contact. +""" +from routes.contacts_routes import _parse_vcards + + +def test_folded_email_is_reassembled(): + vcard = ( + "BEGIN:VCARD\r\n" + "VERSION:3.0\r\n" + "FN:John Doe\r\n" + "EMAIL;TYPE=INTERNET:john.doe.with.a.very.long.local.part@exampledomain\r\n" + " .com\r\n" + "END:VCARD\r\n" + ) + contacts = _parse_vcards(vcard) + assert len(contacts) == 1 + assert contacts[0]["emails"] == [ + "john.doe.with.a.very.long.local.part@exampledomain.com" + ] + + +def test_folded_display_name_is_reassembled(): + vcard = ( + "BEGIN:VCARD\n" + "FN:A Very Long Display Name That The Server\n" + " Decided To Fold\n" + "EMAIL:x@y.com\n" + "END:VCARD\n" + ) + c = _parse_vcards(vcard)[0] + assert c["name"] == "A Very Long Display Name That The Server Decided To Fold" + + +def test_unfolded_vcard_still_parses(): + vcard = "BEGIN:VCARD\nFN:Jane\nEMAIL:jane@z.com\nTEL:+15550001\nEND:VCARD\n" + c = _parse_vcards(vcard)[0] + assert c["name"] == "Jane" + assert c["emails"] == ["jane@z.com"] + assert c["phones"] == ["+15550001"] diff --git a/tests/test_vision_owner_scope.py b/tests/test_vision_owner_scope.py index 90a17adb3..f0d3a184d 100644 --- a/tests/test_vision_owner_scope.py +++ b/tests/test_vision_owner_scope.py @@ -89,8 +89,8 @@ def test_request_vision_call_sites_pass_owner(): processor_source = (ROOT / "src" / "document_processor.py").read_text() upload_source = (ROOT / "routes" / "upload_routes.py").read_text() document_source = (ROOT / "routes" / "document_routes.py").read_text() - gallery_source = (ROOT / "routes" / "gallery_routes.py").read_text() - memory_source = (ROOT / "routes" / "memory_routes.py").read_text() + gallery_source = (ROOT / "routes" / "gallery" / "gallery_routes.py").read_text() + memory_source = (ROOT / "routes" / "memory" / "memory_routes.py").read_text() assert 'analyze_image_with_vl_result(file_info["path"], owner=owner)' in chat_source assert "analyze_image_with_vl(path, owner=current_user)" in upload_source diff --git a/tests/test_visual_report_slug_unique.py b/tests/test_visual_report_slug_unique.py new file mode 100644 index 000000000..ee3ca8023 --- /dev/null +++ b/tests/test_visual_report_slug_unique.py @@ -0,0 +1,27 @@ +"""Regression: _extract_headings must emit a unique slug per heading. + +_make_slug disambiguates repeats by appending "-N", but it only tracked the +*base* slug, so a generated "intro-1" could collide with a naturally-occurring +"intro-1" (e.g. headings "Intro", "Intro", "Intro 1" all produced +["intro", "intro-1", "intro-1"]). Duplicate slugs become duplicate heading ids, +which makes the second table-of-contents link dead. Slugs are now guaranteed +unique. Plain repeats keep their existing "-1", "-2" sequence. +""" +from src.visual_report import _extract_headings + + +def _slugs(md): + return [h["slug"] for h in _extract_headings(md)] + + +def test_disambiguated_slug_does_not_collide_with_natural_slug(): + slugs = _slugs("## Intro\n\n## Intro\n\n## Intro 1\n") + assert len(slugs) == len(set(slugs)), slugs + + +def test_plain_repeats_keep_sequential_suffixes(): + assert _slugs("## Foo\n\n## Foo\n\n## Foo\n") == ["foo", "foo-1", "foo-2"] + + +def test_distinct_headings_are_unchanged(): + assert _slugs("## Alpha\n\n## Beta\n") == ["alpha", "beta"] diff --git a/tests/test_visual_report_toc_code_fence.py b/tests/test_visual_report_toc_code_fence.py new file mode 100644 index 000000000..617ea4a6a --- /dev/null +++ b/tests/test_visual_report_toc_code_fence.py @@ -0,0 +1,28 @@ +"""TOC heading extraction must ignore headings inside code fences. + +A "## ..." comment inside a ``` or ~~~ block is not rendered as an

, but +_extract_headings counted it, so _apply_heading_ids (which zips TOC headings +against rendered

/

by position) gave later sections the wrong anchor +id and the trailing TOC link went dead. +""" +import pytest + +pytest.importorskip("bs4") + +from src.visual_report import _extract_headings + + +def test_backtick_fenced_heading_is_ignored(): + md = "## Intro\n\n```bash\n## not a heading\n```\n\n## Conclusion" + assert [h["text"] for h in _extract_headings(md)] == ["Intro", "Conclusion"] + + +def test_tilde_fenced_heading_is_ignored(): + md = "## A\n\n~~~\n## fake\n~~~\n\n## B" + assert [h["text"] for h in _extract_headings(md)] == ["A", "B"] + + +def test_normal_headings_unaffected(): + md = "## One\n\nsome text\n\n### Two" + out = [(h["level"], h["text"]) for h in _extract_headings(md)] + assert out == [(2, "One"), (3, "Two")] diff --git a/tests/test_web_fetch_size_caps.py b/tests/test_web_fetch_size_caps.py index 19320c6c2..a3cfa64ed 100644 --- a/tests/test_web_fetch_size_caps.py +++ b/tests/test_web_fetch_size_caps.py @@ -13,6 +13,57 @@ import pytest from src.constants import WEB_FETCH_SOFT_MAX_BYTES, WEB_FETCH_HARD_MAX_BYTES from services.search import content as content_mod +import pytest as _pytest_for_client_stream_compat + + +@_pytest_for_client_stream_compat.fixture(autouse=True) +def _client_stream_compat_for_pinned_fetch(monkeypatch): + """Adapt old size-cap tests to the current pinned Client.stream path. + + These tests monkeypatch httpx.stream(...) to return fake responses. The + production fetcher now uses httpx.Client(...).stream(...) so it can pass a + pinned transport. When a test has replaced httpx.stream, route Client.stream + through that fake. When it has not, fall back to a real Client so unrelated + behavior in this file is not changed. + """ + import httpx + + real_client_cls = httpx.Client + original_stream = httpx.stream + + class _ClientProxy: + def __init__(self, *args, **kwargs): + self._args = args + self._kwargs = kwargs + self._real_cm = None + self._real_client = None + + def __enter__(self): + if httpx.stream is original_stream: + self._real_cm = real_client_cls(*self._args, **self._kwargs) + self._real_client = self._real_cm.__enter__() + return self._real_client + return self + + def __exit__(self, *args): + if self._real_cm is not None: + return self._real_cm.__exit__(*args) + return False + + def stream(self, method, url): + if self._real_client is not None: + return self._real_client.stream(method, url) + + kwargs = { + "headers": self._kwargs.get("headers"), + "timeout": self._kwargs.get("timeout"), + "follow_redirects": self._kwargs.get("follow_redirects"), + } + return httpx.stream(method, url, **kwargs) + + monkeypatch.setattr(httpx, "Client", _ClientProxy) + + class _FakeStream: """Stands in for the httpx.stream(...) context manager.""" diff --git a/tests/test_web_search_query_sanitization.py b/tests/test_web_search_query_sanitization.py new file mode 100644 index 000000000..58e338233 --- /dev/null +++ b/tests/test_web_search_query_sanitization.py @@ -0,0 +1,209 @@ +"""Regression tests for #4547 — chat-mode web search query sanitization. + +Chat-mode web search (``use_web``) selects a search query via the +generated-query flow added in #4557: an LLM extracts a concise query, falling +back to the first non-empty line of the user message when the LLM fails or +returns an empty result. PR #4863 layers a focused, *defensive* cleanup on top +of that flow: whatever query is finally selected (generated or fallback) is +passed through ``_clean_search_query()`` before reaching +``comprehensive_web_search()``, so residual fenced/inline markdown never leaks +into the search call. + +``_clean_search_query()`` renders the query to HTML via ``markdown`` +(``fenced_code`` extension), drops ``
`` blocks entirely, unwraps inline
+```` to its text (so ``git reset`` survives), collapses whitespace, and
+truncates.
+
+The first four tests pin the helper directly; the last three prove it is
+wired into the production path and that the combined generated-query +
+sanitization behaviour holds for all three selection outcomes (generated
+success, LLM exception, empty LLM result).
+
+This is intentionally a narrow interim/defensive fix for #4547; it does not
+replace the generated-query flow from #4557.
+"""
+from src.chat_processor import ChatProcessor, _clean_search_query
+
+
+# ── Unit tests: _clean_search_query ──
+
+
+def test_clean_search_query_removes_fenced_code_blocks():
+    """A fenced code block must be dropped entirely, including the code body
+    and the fences — only the surrounding prose survives."""
+    message = '```python\nprint("hello")\n```\nWhat is the capital of France?'
+
+    result = _clean_search_query(message)
+
+    assert result == "What is the capital of France?"
+    # Guards against the original leak: no fences, no code body.
+    assert "```" not in result
+    assert "print" not in result
+
+
+def test_clean_search_query_preserves_inline_code():
+    """Inline code text is search-relevant and must survive unwrapped; only the
+    backticks are removed. This is the ``git reset`` case the reviewer flagged
+    against the earlier regex approach (which dropped the word entirely)."""
+    message = "Is it a good idea to use `git reset` to undo my changes?"
+
+    result = _clean_search_query(message)
+
+    assert result == "Is it a good idea to use git reset to undo my changes?"
+    assert "git reset" in result
+    assert "`" not in result
+
+
+def test_clean_search_query_collapses_whitespace():
+    """Runs of whitespace (tabs, multiple spaces, newlines) collapse to a single
+    space so the query is a single clean line."""
+    message = "hello\tworld   foo\n\n   bar"
+
+    result = _clean_search_query(message)
+
+    assert result == "hello world foo bar"
+    assert "  " not in result
+    assert "\n" not in result
+    assert "\t" not in result
+
+
+def test_clean_search_query_truncates_long_input():
+    """Long queries are capped at ``max_len`` (default 200) to stay within search
+    API limits; truncation is a strict prefix of the cleaned text."""
+    long_message = "x" * 300
+
+    result_default = _clean_search_query(long_message)
+    result_custom = _clean_search_query(long_message, max_len=10)
+
+    assert len(result_default) == 200
+    assert result_default == "x" * 200
+    assert result_custom == "x" * 10
+
+
+# ── Integration tests: the generated-query + sanitization flow ──
+#
+# These cover the combined behaviour requested in review of #4863 after #4557
+# landed: the LLM-generated query is used on success, the first-line fallback is
+# used when the LLM fails or returns empty, and in every case the *final* query
+# handed to comprehensive_web_search() is sanitized.
+
+# A messy user message whose first non-empty line (the #4557 fallback) is
+# inline-code prose followed by a fenced block. After sanitization the fallback
+# collapses to plain prose.
+_MESSY = 'Is `git reset` safe?\n```python\nprint("leaked body")\n```'
+_SANITIZED_FALLBACK = "Is git reset safe?"
+
+
+class _Session:
+    """Minimal stand-in for the session object read by the generated-query
+    flow (endpoint_url / model / headers)."""
+
+    endpoint_url = "http://example.local/v1"
+    model = "test-model"
+    headers = {"Authorization": "Bearer test"}
+
+
+class _Memory:
+    def load(self, owner=None):
+        return []
+
+
+class _Docs:
+    rag_manager = None
+
+
+def _patch_flow(monkeypatch, llm_behaviour, captured):
+    """Wire both seams of the generated-query flow: the LLM call and the
+    search call. ``llm_behaviour`` is either a string to return or an Exception
+    instance to raise."""
+
+    def _fake_search(query, *args, **kwargs):
+        captured["query"] = query
+        captured["kwargs"] = kwargs
+        return ("web context", [{"title": "src"}])
+
+    def _fake_llm(*args, **kwargs):
+        if isinstance(llm_behaviour, Exception):
+            raise llm_behaviour
+        return llm_behaviour
+
+    monkeypatch.setattr("src.chat_processor.comprehensive_web_search", _fake_search)
+    monkeypatch.setattr("src.llm_core.llm_call", _fake_llm)
+
+
+def test_generated_query_is_used_and_sanitized(monkeypatch):
+    """Requirement: on LLM success the generated query wins, and the *final*
+    query handed to comprehensive_web_search() is sanitized.
+
+    The fake LLM returns a query containing inline-code markdown so we can also
+    prove the sanitizer runs on the generated path (not just the fallback)."""
+    captured = {}
+    _patch_flow(monkeypatch, "capital of `France`", captured)
+
+    processor = ChatProcessor(memory_manager=_Memory(), personal_docs_manager=_Docs())
+    preface, _, _ = processor.build_context_preface(
+        message=_MESSY,
+        session=_Session(),
+        use_web=True,
+        use_memory=False,
+        use_rag=False,
+    )
+
+    assert "query" in captured, "comprehensive_web_search was not called"
+
+    # The generated query won (not the sanitized first-line fallback) ...
+    assert captured["query"] == "capital of France"
+    assert captured["query"] != _SANITIZED_FALLBACK
+    # ... and it was sanitized: no residual markdown fences/backticks.
+    assert "`" not in captured["query"]
+    assert "```" not in captured["query"]
+
+    # The other call-site kwargs (return_sources) are still forwarded.
+    assert captured["kwargs"].get("return_sources") is True
+    # And the retrieved context was still appended to the preface.
+    assert any("web context" in (msg.get("content") or "") for msg in preface)
+
+
+def test_falls_back_to_sanitized_first_line_when_llm_raises(monkeypatch):
+    """Requirement: when the LLM call raises, #4557's fallback (first non-empty
+    line) is used — and that fallback is sanitized before the search call."""
+    captured = {}
+    _patch_flow(monkeypatch, RuntimeError("LLM endpoint down"), captured)
+
+    processor = ChatProcessor(memory_manager=_Memory(), personal_docs_manager=_Docs())
+    processor.build_context_preface(
+        message=_MESSY,
+        session=_Session(),
+        use_web=True,
+        use_memory=False,
+        use_rag=False,
+    )
+
+    assert "query" in captured, "comprehensive_web_search was not called"
+    # Fallback was the first line ("Is `git reset` safe?"), sanitized.
+    assert captured["query"] == _SANITIZED_FALLBACK
+    assert "git reset" in captured["query"]  # inline code preserved
+    assert "`" not in captured["query"]  # backticks stripped
+    # The fenced body from later lines never reached the query.
+    assert "leaked body" not in captured["query"]
+
+
+def test_falls_back_to_sanitized_first_line_when_llm_returns_empty(monkeypatch):
+    """Requirement: when the LLM returns an empty/whitespace-only query, #4557
+    falls back — and that fallback is sanitized before the search call."""
+    captured = {}
+    _patch_flow(monkeypatch, "   ", captured)
+
+    processor = ChatProcessor(memory_manager=_Memory(), personal_docs_manager=_Docs())
+    processor.build_context_preface(
+        message=_MESSY,
+        session=_Session(),
+        use_web=True,
+        use_memory=False,
+        use_rag=False,
+    )
+
+    assert "query" in captured, "comprehensive_web_search was not called"
+    assert captured["query"] == _SANITIZED_FALLBACK
+    assert "git reset" in captured["query"]
+    assert "`" not in captured["query"]
diff --git a/tests/test_webhook_dns_rebinding_pin.py b/tests/test_webhook_dns_rebinding_pin.py
new file mode 100644
index 000000000..144a19e96
--- /dev/null
+++ b/tests/test_webhook_dns_rebinding_pin.py
@@ -0,0 +1,156 @@
+"""Regression: webhook delivery must pin the TCP connect to the SSRF-approved IP.
+
+validate_webhook_url resolves the host to accept/reject, but the delivery
+connect previously re-resolved independently — a DNS record flipping between
+the two lookups (rebinding) could slip an internal IP past the check. _deliver
+now resolves+validates once via _validated_public_ips and pins the connect to
+that IP through _PinnedAsyncTransport. These tests drive the real transport
+against local servers so the pin is exercised end-to-end, not mocked away.
+"""
+import asyncio
+import http.server
+import ipaddress
+import socketserver
+import threading
+
+import pytest
+
+from tests.helpers.import_state import clear_module, preserve_import_state
+
+import os
+import sys
+from unittest.mock import patch
+
+with patch.dict(os.environ, {"DATABASE_URL": "sqlite:///:memory:"}), \
+        preserve_import_state("src.database", "core.database"):
+    clear_module("src.database")
+    _core_database = sys.modules.get("core.database")
+    if _core_database is not None and not getattr(_core_database, "__file__", None):
+        del sys.modules["core.database"]
+    import src.webhook_manager as wm
+
+
+# ---------------------------------------------------------------------------
+# _validated_public_ips
+# ---------------------------------------------------------------------------
+
+def test_validated_public_ips_rejects_metadata_literal():
+    with pytest.raises(ValueError):
+        wm._validated_public_ips("http://169.254.169.254/")
+
+
+def test_validated_public_ips_rejects_loopback_literal():
+    with pytest.raises(ValueError):
+        wm._validated_public_ips("http://127.0.0.1/")
+
+
+def test_validated_public_ips_returns_public_literal():
+    ips = wm._validated_public_ips("http://93.184.216.34/")
+    assert ips == [ipaddress.ip_address("93.184.216.34")]
+
+
+def test_validated_public_ips_rejects_hostname_resolving_private(monkeypatch):
+    # Rebinding shape: a hostname that (now) resolves into loopback space.
+    monkeypatch.setattr(wm, "_resolve_hostname_ips",
+                        lambda h: [ipaddress.ip_address("127.0.0.1")])
+    with pytest.raises(ValueError):
+        wm._validated_public_ips("http://evil.rebind.example/")
+
+
+# ---------------------------------------------------------------------------
+# End-to-end: the pinned transport actually routes to the pinned IP
+# ---------------------------------------------------------------------------
+
+def _serve(handler):
+    srv = socketserver.TCPServer(("127.0.0.1", 0), handler)
+    port = srv.server_address[1]
+    threading.Thread(target=srv.serve_forever, daemon=True).start()
+    return srv, port
+
+
+def test_pinned_transport_connects_to_pinned_ip():
+    """A request whose URL host is a throwaway hostname is still delivered to
+    the pinned loopback IP — proving the socket destination comes from the pin,
+    not from resolving the URL host."""
+    hits = []
+
+    class _Handler(http.server.BaseHTTPRequestHandler):
+        def do_POST(self):  # noqa: N802
+            length = int(self.headers.get("Content-Length", 0))
+            self.rfile.read(length)
+            hits.append(self.path)
+            self.send_response(204)
+            self.end_headers()
+
+        def log_message(self, *a):
+            pass
+
+    srv, port = _serve(_Handler)
+    try:
+        ip = ipaddress.ip_address("127.0.0.1")
+        transport = wm._PinnedAsyncTransport(ip)
+
+        async def go():
+            async with __import__("httpx").AsyncClient(
+                transport=transport, timeout=5, follow_redirects=False,
+            ) as client:
+                # Host "unresolvable.invalid" would never resolve; the pin is
+                # what makes this reach the loopback server on `port`.
+                return await client.post(
+                    f"http://unresolvable.invalid:{port}/hook", content=b"{}",
+                )
+
+        resp = asyncio.run(go())
+        assert resp.status_code == 204
+        assert hits == ["/hook"]
+    finally:
+        srv.shutdown()
+
+
+def test_deliver_pins_to_validated_ip_end_to_end(monkeypatch):
+    """Full _deliver path: a hostname that validation resolves to loopback is
+    pinned to loopback and the local server receives the signed POST."""
+    received = {}
+
+    class _Handler(http.server.BaseHTTPRequestHandler):
+        def do_POST(self):  # noqa: N802
+            length = int(self.headers.get("Content-Length", 0))
+            received["body"] = self.rfile.read(length)
+            received["event"] = self.headers.get("X-Odysseus-Event")
+            self.send_response(200)
+            self.end_headers()
+
+        def log_message(self, *a):
+            pass
+
+    srv, port = _serve(_Handler)
+
+    class _Query:
+        def filter(self, *a, **k): return self
+        def update(self, values): return None
+
+    class _Db:
+        def query(self, _m): return _Query()
+        def commit(self): pass
+        def rollback(self): pass
+        def close(self): pass
+
+    # Make both the validation resolve and the pin target loopback, and treat
+    # loopback as allowed for this test (production blocks it — here we only
+    # want to prove the pin routes to the validated IP).
+    monkeypatch.setattr(wm, "SessionLocal", lambda: _Db())
+    monkeypatch.setattr(wm, "_is_private_url", lambda url: False)
+    monkeypatch.setattr(wm, "_resolve_hostname_ips",
+                        lambda h: [ipaddress.ip_address("127.0.0.1")])
+    monkeypatch.setattr(wm, "_ip_is_private", lambda a: False)
+
+    manager = wm.WebhookManager()
+    try:
+        asyncio.run(manager._deliver(
+            "hook-1", f"http://webhook.test:{port}/cb", "s3cret",
+            "webhook.test", {"ok": True},
+        ))
+        assert received.get("event") == "webhook.test"
+        assert b'"ok": true' in received["body"]
+    finally:
+        srv.shutdown()
diff --git a/tests/test_webhook_ssrf_resilience.py b/tests/test_webhook_ssrf_resilience.py
index e02f17a25..ca82a7595 100644
--- a/tests/test_webhook_ssrf_resilience.py
+++ b/tests/test_webhook_ssrf_resilience.py
@@ -96,26 +96,29 @@ async def test_webhook_delivery_uses_naive_utc_timestamps(monkeypatch):
     class _Response:
         status_code = 204
 
-    class _Client:
-        def __init__(self):
-            self.content = ""
-
-        async def post(self, _url, content, headers):
-            self.content = content
-            assert headers["X-Odysseus-Event"] == "webhook.test"
-            return _Response()
-
     db = _Db()
-    client = _Client()
     monkeypatch.setattr(wm, "SessionLocal", lambda: db)
 
     manager = wm.WebhookManager()
-    await manager._client.aclose()
-    manager._client = client
+
+    # Replace the pinned-transport send seam so no real socket is opened. The
+    # public-IP literal below still exercises _validated_public_ips (which pins
+    # the connect); the captured content proves the body/headers are built.
+    captured = {}
+
+    async def _fake_send(url, body, headers, ip):
+        captured["content"] = body
+        captured["ip"] = str(ip)
+        assert headers["X-Odysseus-Event"] == "webhook.test"
+        return _Response()
+
+    monkeypatch.setattr(manager, "_send_request", _fake_send)
 
     await manager._deliver("hook-1", "http://93.184.216.34/", None, "webhook.test", {"ok": True})
 
-    body = json.loads(client.content)
+    # The delivery must have pinned to the literal public IP from the URL.
+    assert captured["ip"] == "93.184.216.34"
+    body = json.loads(captured["content"])
     payload_timestamp = datetime.fromisoformat(body["timestamp"])
     assert payload_timestamp.tzinfo is None
     assert db.updates[0]["last_triggered_at"].tzinfo is None
diff --git a/tests/test_workspace_confine.py b/tests/test_workspace_confine.py
index 81bc7235c..6d90789c9 100644
--- a/tests/test_workspace_confine.py
+++ b/tests/test_workspace_confine.py
@@ -140,6 +140,65 @@ async def test_grep_and_ls_confined_e2e(ws, admin):
     assert r["exit_code"] == 1 and "outside the workspace" in r["error"]
 
 
+@pytest.mark.asyncio
+async def test_glob_confined_e2e(ws, admin):
+    """glob's literal fast-path must stay inside the workspace. A pattern with
+    ../ or an absolute path outside the root would otherwise leak the existence
+    and full path of arbitrary host files (an oracle), even though read_file
+    blocks reading them."""
+    with open(os.path.join(ws, "found.py"), "w") as f:
+        f.write("x")
+    _, r = await execute_tool_block(_block("glob", json.dumps({"pattern": "found.py"})), owner="a", workspace=ws)
+    assert r["exit_code"] == 0 and "found.py" in r["output"]
+
+    # a secret outside the workspace must not be discoverable via glob
+    outside = tempfile.mkdtemp()
+    secret = os.path.join(outside, "secret.txt")
+    with open(secret, "w") as f:
+        f.write("nope")
+    # An escaping pattern must come back as "No files" (the not-found message),
+    # not as a match that returns the file's path. The not-found message echoes
+    # the pattern the model supplied, so the signal is the absence of a match,
+    # not the absence of the path string.
+    rel = os.path.relpath(secret, os.path.realpath(ws))
+    _, r = await execute_tool_block(_block("glob", json.dumps({"pattern": rel})), owner="a", workspace=ws)
+    assert r["exit_code"] == 0 and "No files" in r["output"] and secret not in r["output"]
+    _, r = await execute_tool_block(_block("glob", json.dumps({"pattern": secret})), owner="a", workspace=ws)
+    assert r["exit_code"] == 0 and "No files" in r["output"]
+
+
+@pytest.mark.asyncio
+async def test_glob_skips_sensitive_files_in_workspace(ws, admin):
+    """glob must not enumerate deny-listed sensitive files that live inside the
+    workspace. read_file/write_file/edit_file refuse them and grep skips them,
+    so glob surfacing their paths is an enumeration oracle for prompt-injection.
+    """
+    with open(os.path.join(ws, "keep.py"), "w") as f:
+        f.write("x")
+    with open(os.path.join(ws, ".env"), "w") as f:
+        f.write("AWS_SECRET=xxx")
+    with open(os.path.join(ws, "id_rsa"), "w") as f:  # non-dotfile key at root
+        f.write("KEY")
+    os.makedirs(os.path.join(ws, ".ssh"), exist_ok=True)
+    with open(os.path.join(ws, ".ssh", "authorized_keys"), "w") as f:
+        f.write("ssh-rsa AAAA")
+
+    # A recursive wildcard returns ordinary files but none of the sensitive
+    # ones. The pattern "**/*" contains no secret names, so a secret basename
+    # appearing in the output is a real leak (not the echoed not-found pattern).
+    _, r = await execute_tool_block(_block("glob", json.dumps({"pattern": "**/*"})), owner="a", workspace=ws)
+    assert r["exit_code"] == 0
+    assert "keep.py" in r["output"]
+    for leak in (".env", "id_rsa", "authorized_keys"):
+        assert leak not in r["output"], f"glob leaked sensitive file: {leak}"
+
+    # Directly targeting a sensitive file (literal fast-path and wildcard) must
+    # come back as the not-found message, never a match with the file's path.
+    for pat in (".env", "**/id_rsa", "**/authorized_keys"):
+        _, r = await execute_tool_block(_block("glob", json.dumps({"pattern": pat})), owner="a", workspace=ws)
+        assert r["exit_code"] == 0 and "No files" in r["output"]
+
+
 @pytest.mark.asyncio
 async def test_subprocess_cwd_is_workspace_e2e(ws, admin):
     """python tool runs with cwd = workspace (OS-agnostic probe)."""