Compare commits

...
Author SHA1 Message Date
Robby ManihaniandClaude Opus 4.8 90b7d0cb9b fix(windows): installer encoding + Ollama readiness loop (#523)
Two bugs found during end-to-end testing on a clean Windows 11 24H2
Azure VM (closes #522). Both are dodged by the canonical `irm | iex`
one-liner but hit by the documented `-OutFile` fallback and any
non-interactive run.

1. Encoding. install.ps1 was UTF-8 without a BOM and contained em-dashes
   plus a box-drawing banner. Windows PowerShell 5.1 decodes BOM-less
   files with the legacy ANSI/OEM code page, mis-decoding the multi-byte
   sequences and desyncing the parser into cascading here-string parse
   errors. Converted the file to pure ASCII (em-dashes -> hyphens, banner
   -> ASCII art) so it parses no matter how it's read.

2. Ollama readiness loop. With $ErrorActionPreference='Stop', the probe
   `& $ollamaExe list 2>&1 | Out-Null` turned the daemon-not-up stderr
   into a terminating NativeCommandError, aborting the install on the
   first iteration and making the loop's own Start-Process serve retry +
   Write-Warn2 fallback dead code. Wrapped the probe in try/catch so it
   falls through to the self-start path as intended.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-10 19:15:57 -07:00
d7053c35d5 security: harden network-exposed surface (#509)
* security: harden network-exposed surface

Hardening for the network-reachable attack surface, prioritizing fixes
that are strong but do not change working local/loopback defaults.

- auth_middleware: constant-time API key comparison (secrets.compare_digest)
  for the HTTP path, and gate /metrics behind auth so operational counters
  are not readable unauthenticated. /health stays open.
- webhook_routes: fail closed when a channel's secret/token is unset. Twilio,
  BlueBubbles, WhatsApp (verify + inbound), and SendBlue now reject (403)
  instead of processing unsigned/unauthenticated input. Constant-time
  comparisons for BlueBubbles/SendBlue/WhatsApp verify token.
- http_request: follow redirects manually and re-run the SSRF check on every
  hop (capped at 5) so an allowed public URL cannot 30x-redirect to an
  internal/metadata address.
- api_routes /v1/memory/index: restrict indexing to OPENJARVIS_WORKSPACE roots
  when configured and refuse sensitive files (.env, keys, credentials).
- config.toml: default [server] host to 127.0.0.1 (loopback) with a comment
  on how to safely expose to a LAN (0.0.0.0 + API key).

Tests: new fail-closed webhook tests, /metrics auth tests, and SSRF
redirect block/follow tests; updated SendBlue tests for the new
secret-required behavior. Affected suites pass (95 tests), ruff clean.

* fix(http): keep SSRF redirect-following patchable via httpx.request

The manual redirect-following loop used a private httpx.Client, which
bypassed the `http_request.httpx.request` mock seam that consumers' tests
rely on (e.g. the twitter-bot GitHub-issue tests escaped to the real
network and 401'd). Issue each hop via module-level httpx.request with
follow_redirects=False instead — same per-hop SSRF re-check, restored
testability.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Jon Saad-Falcon <jonsaadfalcon@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-10 15:32:28 -07:00
03c5ec3e40 fix(chat): wire SystemPromptBuilder so persona files load in jarvis chat (fixes #458) (#497)
* fix(chat): wire SystemPromptBuilder so persona files load in jarvis chat (fixes #458)

* fix(chat): make `--persona none` actually disable persona files

This PR exposes `--persona none`, but SystemPromptBuilder._load_file read
empty paths as "." (Path("") -> ".") and raised IsADirectoryError, so the
documented opt-out crashed. Guard empty path_str so the "none" opt-out
(which _resolve_persona maps to empty file paths) cleanly injects no
persona. Adds an end-to-end regression test (building with persona
"none" must not raise). Also merges current main (branch was stale).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Jon Saad-Falcon <jonsaadfalcon@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-10 14:10:13 -07:00
Jon Saad-FalconandClaude Opus 4.8 4218258486 perf(serve): build the system once — drop duplicate SystemBuilder.build() (#263) (#529)
`jarvis serve` constructed every heavy component inline (engine discovery +
instrumentation, telemetry, memory, agent manager, per-agent tools) and then
called `SystemBuilder(config).build()` a second time inside the scheduler
block purely to feed `AgentExecutor.set_system()`. That second build
re-discovered and re-connected the engine, re-instrumented it, re-resolved
tools, re-opened the configured channel and re-created the agent manager —
~30-40s of fully redundant startup work (the headline remaining cost in #263
after engine probes were parallelised and the version check moved off the hot
path in #470).

Fix: assemble the executor's `JarvisSystem` from the components already built
inline instead of rebuilding from scratch. `AgentExecutor` only reads
`engine`, `model`, `config`, `memory_backend`, `tool_executor`,
`session_store` and `channel_backend` off the system; all are wired here. The
memory backend is now constructed just before the scheduler block (it was
built later) so the executor's system can reference it, and the primary
agent's resolved tool list is reused to build the scheduler's `ToolExecutor`
(preserving the MCP-discovered-tool pool the executor reads via
`tool_executor._tools`). `skill_manager` / the learning orchestrator are only
consumed by the orchestrator's `system.ask()` path, which the executor never
invokes, so they are intentionally omitted.

Tests: new `tests/cli/test_serve_single_build.py` patches
`SystemBuilder.build` and asserts it is never called during `jarvis serve`
startup, and that the executor still receives a system exposing
`tool_executor` / `session_store` / `memory_backend` (plus engine/model/config).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-10 14:10:10 -07:00
15 changed files with 668 additions and 85 deletions
+6 -1
View File
@@ -106,6 +106,11 @@ enabled = true # Record traces for analysis
db_path = "~/.openjarvis/traces.db"
[server]
host = "0.0.0.0"
# Bind to loopback by default so the API is not exposed to the local network.
# To serve other devices on your LAN, set host = "0.0.0.0" AND set an API key
# (OPENJARVIS_API_KEY / `jarvis auth generate-key`) — startup refuses a
# non-loopback bind without a key. The "server" security profile also flips
# this to 0.0.0.0 intentionally.
host = "127.0.0.1"
port = 8000
agent = "native_openhands"
+30 -25
View File
@@ -5,12 +5,12 @@
.DESCRIPTION
Phase-1 of the native-Windows-support RFC (#298). Mirrors the
behavior of scripts/install/install.sh (the curl-pipe-bash installer
for Linux/WSL2/macOS) but for native Windows PowerShell no WSL,
for Linux/WSL2/macOS) but for native Windows PowerShell - no WSL,
no Docker, no MSYS2.
Steps:
1. Refuse non-Windows / Windows < 10.
2. Check Python 3.10 3.13 on PATH (3.14 has no numpy wheels yet,
2. Check Python 3.10 - 3.13 on PATH (3.14 has no numpy wheels yet,
see #432).
3. Check git on PATH.
4. Install uv (https://astral.sh/uv) if absent.
@@ -65,7 +65,7 @@ if (-not $Service -and $env:OPENJARVIS_SERVICE) { $Service = $true
if (-not $Force -and $env:OPENJARVIS_FORCE) { $Force = $true }
# ---------------------------------------------------------------------------
# Output helpers coloured but plain enough for Constrained Language Mode.
# Output helpers - coloured but plain enough for Constrained Language Mode.
# ---------------------------------------------------------------------------
function Write-Info ($msg) { Write-Host "[info] $msg" -ForegroundColor Cyan }
@@ -77,13 +77,13 @@ function Write-Fail ($msg) {
}
# ---------------------------------------------------------------------------
# Shared helpers winget bootstrap + PATH refresh
# Shared helpers - winget bootstrap + PATH refresh
# ---------------------------------------------------------------------------
# Pull the latest Machine + User PATH from the registry into the current
# PowerShell session. Tools installed by `winget install` (Python, git,
# Ollama, etc.) update the User PATH, but the running process inherits
# the parent shell's environment so without this refresh the just-
# the parent shell's environment - so without this refresh the just-
# installed tool stays invisible to subsequent `Get-Command` calls.
#
# CRITICAL: registry PATH entries can be REG_EXPAND_SZ (with literal
@@ -157,7 +157,7 @@ function Get-PythonCommand {
Write-Info "Checking Python (3.10 - 3.13)..."
$pythonExe = Get-PythonCommand
if (-not $pythonExe) {
Write-Info "Python not on PATH attempting auto-install via winget..."
Write-Info "Python not on PATH - attempting auto-install via winget..."
$pythonExe = Install-WithWinget -WingetId 'Python.Python.3.13' -CommandName 'python'
if (-not $pythonExe) {
Write-Fail @"
@@ -196,7 +196,7 @@ Write-Ok "Python $pyMajor.$pyMinor ($pythonExe)"
Write-Info "Checking git..."
$gitExe = (Get-Command git -ErrorAction SilentlyContinue).Source
if (-not $gitExe) {
Write-Info "git not on PATH attempting auto-install via winget..."
Write-Info "git not on PATH - attempting auto-install via winget..."
$gitExe = Install-WithWinget -WingetId 'Git.Git' -CommandName 'git'
if (-not $gitExe) {
Write-Fail @"
@@ -227,7 +227,7 @@ if (-not $uvExe) {
}
# The astral installer puts uv at %USERPROFILE%\.local\bin\uv.exe and
# adds that dir to the User PATH. The current process's PATH isn't
# refreshed automatically prepend the install dir so the rest of
# refreshed automatically - prepend the install dir so the rest of
# this script picks it up.
$uvDir = Join-Path $env:USERPROFILE '.local\bin'
if (Test-Path (Join-Path $uvDir 'uv.exe')) {
@@ -295,13 +295,13 @@ try {
Write-Ok "Dependencies installed"
# ---------------------------------------------------------------------------
# 7. Ollama install + start + wait for daemon
# 7. Ollama - install + start + wait for daemon
# ---------------------------------------------------------------------------
Write-Info "Checking Ollama..."
$ollamaExe = (Get-Command ollama -ErrorAction SilentlyContinue).Source
if (-not $ollamaExe) {
Write-Info " Ollama not on PATH downloading the official installer (~150 MB)..."
Write-Info " Ollama not on PATH - downloading the official installer (~150 MB)..."
$ollamaSetup = Join-Path $env:TEMP 'OllamaSetup.exe'
# SilentlyContinue is load-bearing in PS 5.1: the default progress
# bar renderer slows Invoke-WebRequest down 30x on large downloads
@@ -340,13 +340,18 @@ Write-Ok "Ollama ($ollamaExe)"
Write-Info "Waiting for Ollama daemon..."
$ollamaReady = $false
for ($i = 0; $i -lt 60; $i++) {
& $ollamaExe list 2>&1 | Out-Null
# 'ollama list' writes to stderr until the daemon is reachable; under
# $ErrorActionPreference='Stop' the 2>&1 merge surfaces that as a
# terminating NativeCommandError that would abort the whole install on
# the very first probe. Swallow it and rely on $LASTEXITCODE so the
# Start-Process serve fallback below actually runs (issue #522).
try { & $ollamaExe list 2>&1 | Out-Null } catch { }
if ($LASTEXITCODE -eq 0) {
$ollamaReady = $true
break
}
if ($i -eq 5) {
# Daemon clearly isn't auto-running start it ourselves. Ollama
# Daemon clearly isn't auto-running - start it ourselves. Ollama
# for Windows uses the tray app `ollama app.exe`; falling back to
# `ollama serve` works headless.
Start-Process -FilePath $ollamaExe -ArgumentList 'serve' -WindowStyle Hidden -ErrorAction SilentlyContinue
@@ -354,11 +359,11 @@ for ($i = 0; $i -lt 60; $i++) {
Start-Sleep -Seconds 1
}
if (-not $ollamaReady) {
Write-Warn2 "Ollama daemon didn't become ready in 60s. Continuing bg-orchestrator will retry later."
Write-Warn2 "Ollama daemon didn't become ready in 60s. Continuing - bg-orchestrator will retry later."
}
# ---------------------------------------------------------------------------
# 8. Pull a starter model (qwen3.5:2b ~1.5 GB)
# 8. Pull a starter model (qwen3.5:2b - ~1.5 GB)
# ---------------------------------------------------------------------------
$modelPullOk = $false
@@ -372,11 +377,11 @@ if ($ollamaReady) {
Write-Warn2 "ollama pull failed; the bg-orchestrator will retry once Ollama is reachable."
}
} else {
Write-Warn2 "Skipping model pull daemon wasn't ready."
Write-Warn2 "Skipping model pull - daemon wasn't ready."
}
# ---------------------------------------------------------------------------
# 9. jarvis.cmd shim so bare `jarvis` works in any new PowerShell
# 9. jarvis.cmd shim - so bare `jarvis` works in any new PowerShell
# ---------------------------------------------------------------------------
$binDir = Join-Path $installRoot 'bin'
@@ -387,7 +392,7 @@ if (-not (Test-Path $binDir)) {
}
# %~dp0 in a .cmd file resolves to the directory containing the script,
# so the shim is self-locating moving %LOCALAPPDATA%\OpenJarvis won't
# so the shim is self-locating - moving %LOCALAPPDATA%\OpenJarvis won't
# break it as long as the user moves the whole tree. `uv` is resolved
# from PATH at runtime (astral installer adds it to User PATH); avoids
# pinning to the install-time uv.exe path which can shift on uv updates.
@@ -400,7 +405,7 @@ uv run --project "%SRC%" jarvis %*
Set-Content -Path $shimPath -Value $shimContent -Encoding ASCII
# Add %LOCALAPPDATA%\OpenJarvis\bin to User PATH if it isn't already
# there. The current process won't see it until restart handled in the
# there. The current process won't see it until restart - handled in the
# final banner.
#
# Compare against the EXPANDED form: a previous install may have written
@@ -430,7 +435,7 @@ Write-Ok "jarvis shim installed at $shimPath"
$serviceScript = Join-Path $srcDir 'deploy\windows\jarvis-service.ps1'
$shouldInstallService = $false
# Pre-check admin if the user wants the service Register-ScheduledTask
# Pre-check admin if the user wants the service - Register-ScheduledTask
# requires elevation. We do this before the prompt so we don't ask "do
# you want the service?" only to fail with Access Denied after they say
# yes.
@@ -439,7 +444,7 @@ $isAdmin = ([Security.Principal.WindowsPrincipal] `
).IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator)
if ($Service -and -not $isAdmin) {
Write-Fail "-Service was requested, but this PowerShell is not elevated. Register-ScheduledTask needs admin rights re-run from an elevated PowerShell, or drop -Service."
Write-Fail "-Service was requested, but this PowerShell is not elevated. Register-ScheduledTask needs admin rights - re-run from an elevated PowerShell, or drop -Service."
}
if ($Service) {
$shouldInstallService = $true
@@ -448,7 +453,7 @@ if ($Service) {
} elseif (-not $isAdmin) {
# Default to skip-with-explanation when we can't elevate, rather
# than prompting and then failing at Register-ScheduledTask.
Write-Warn2 "Skipping scheduled-task setup this PowerShell is not elevated."
Write-Warn2 "Skipping scheduled-task setup - this PowerShell is not elevated."
Write-Warn2 " Register-ScheduledTask requires admin. To install the service later:"
Write-Warn2 " Right-click PowerShell -> Run as administrator, then run:"
Write-Warn2 " powershell -ExecutionPolicy Bypass -File `"$serviceScript`" install"
@@ -464,7 +469,7 @@ if ($Service) {
$reply = Read-Host "Register OpenJarvis as a Windows scheduled task (auto-start at logon, loopback only)? [y/N]"
$shouldInstallService = ($reply -match '^[yY]')
} else {
Write-Warn2 "Non-interactive install skipping scheduled-task setup."
Write-Warn2 "Non-interactive install - skipping scheduled-task setup."
Write-Warn2 "To register the service later, run (from an elevated PowerShell):"
Write-Warn2 " powershell -ExecutionPolicy Bypass -File `"$serviceScript`" install"
}
@@ -487,9 +492,9 @@ if ($shouldInstallService) {
# ---------------------------------------------------------------------------
Write-Host ""
Write-Host " ┌──────────────────────────────────┐" -ForegroundColor Green
Write-Host " OpenJarvis install complete " -ForegroundColor Green
Write-Host " └──────────────────────────────────┘" -ForegroundColor Green
Write-Host " +----------------------------------+" -ForegroundColor Green
Write-Host " | OpenJarvis install complete |" -ForegroundColor Green
Write-Host " +----------------------------------+" -ForegroundColor Green
Write-Host ""
Write-Host " Repo: $srcDir"
+43
View File
@@ -28,12 +28,22 @@ def _read_input(prompt: str = "You> ") -> Optional[str]:
@click.option("-a", "--agent", "agent_name", default=None, help="Agent type.")
@click.option("--tools", default=None, help="Comma-separated tool names.")
@click.option("--system", "system_prompt", default=None, help="Custom system prompt.")
@click.option(
"--persona",
"persona_name",
default=None,
help=(
"Named persona dir under ~/.openjarvis/personas/<name>/ "
"(overrides config). Pass 'none' to disable all persona files."
),
)
def chat(
engine_key: str | None,
model_name: str | None,
agent_name: str | None,
tools: str | None,
system_prompt: str | None,
persona_name: str | None,
) -> None:
"""Start an interactive multi-turn chat session.
@@ -48,6 +58,14 @@ def chat(
config = load_config()
import dataclasses as _dc
effective_mf = (
_dc.replace(config.memory_files, persona_name=persona_name)
if persona_name is not None
else config.memory_files
)
# Resolve engine
from openjarvis.engine import get_engine
from openjarvis.intelligence import register_builtin_models
@@ -121,6 +139,21 @@ def chat(
kwargs["interactive"] = True
kwargs["confirm_callback"] = _confirm
import inspect as _inspect
if (
"prompt_builder"
in _inspect.signature(agent_cls.__init__).parameters
):
from openjarvis.prompt.builder import SystemPromptBuilder
kwargs["prompt_builder"] = SystemPromptBuilder(
agent_template=config.agent.default_system_prompt or "",
memory_files_config=effective_mf,
system_prompt_config=config.system_prompt,
)
agent = agent_cls(engine, model, **kwargs)
except Exception as exc:
console.print(f"[yellow]Agent '{agent_key}' failed: {exc}[/yellow]")
@@ -147,6 +180,16 @@ def chat(
_notifications = NotificationDispatcher(get_status())
# Conversation state
if not system_prompt:
from openjarvis.prompt.builder import SystemPromptBuilder
builder = SystemPromptBuilder(
agent_template=config.agent.default_system_prompt or "",
memory_files_config=effective_mf,
system_prompt_config=config.system_prompt,
)
system_prompt = builder.build()
history: List[Message] = []
if system_prompt:
history.append(Message(role=Role.SYSTEM, content=system_prompt))
+73 -19
View File
@@ -261,6 +261,11 @@ def serve(
# Resolve agent
agent = None
agent_key = agent_name or config.server.agent
# Tool instances resolved for the primary agent are reused below to build
# the scheduler's ToolExecutor — avoiding a second full SystemBuilder.build()
# (which would re-discover the engine, re-resolve tools, re-open the channel,
# etc.). See the scheduler block near the bottom of this function (#263).
resolved_tools: list = []
if agent_key:
try:
import openjarvis.agents # noqa: F401
@@ -328,6 +333,8 @@ def serve(
if tools:
agent_kwargs["tools"] = tools
# Reuse these for the scheduler's ToolExecutor (#263).
resolved_tools = tools
if getattr(agent_cls, "accepts_tools", False):
agent_kwargs["max_turns"] = config.agent.max_turns
@@ -461,6 +468,24 @@ def serve(
# Create app
from openjarvis.server.app import create_app
# Set up memory backend for context injection. Built before the scheduler
# block so the executor's JarvisSystem can reference it (#263).
memory_backend = None
if config.agent.context_from_memory:
try:
import openjarvis.tools.storage # noqa: F401
from openjarvis.core.registry import MemoryRegistry
mem_key = config.memory.default_backend
if MemoryRegistry.contains(mem_key):
memory_backend = MemoryRegistry.create(
mem_key,
db_path=config.memory.db_path,
)
console.print(" Memory: [cyan]active[/cyan]")
except Exception as exc:
logger.debug("Memory backend init failed: %s", exc)
# Set up agent manager
agent_manager = None
if config.agent_manager.enabled:
@@ -500,9 +525,55 @@ def serve(
event_bus=bus,
trace_store=_trace_store,
)
from openjarvis.system import SystemBuilder
# Reuse the components already built inline above instead of a
# second full SystemBuilder.build() — the original double-build
# re-discovered the engine, re-instrumented it, re-resolved tools,
# re-opened the channel and re-created the agent manager, costing
# ~30-40s on top of an already-paid startup (#263). The executor
# only reads engine/model/config/memory_backend/tool_executor/
# session_store/channel_backend from the system (see
# AgentExecutor), all of which are wired here.
from openjarvis.sessions.session import SessionStore
from openjarvis.system import JarvisSystem
from openjarvis.tools._stubs import ToolExecutor
system = SystemBuilder(config).build()
_sched_session_store = None
if config.sessions.enabled:
try:
from pathlib import Path as _SchedPath
_sched_session_store = SessionStore(
db_path=_SchedPath(config.sessions.db_path).expanduser(),
max_age_hours=config.sessions.max_age_hours,
consolidation_threshold=(
config.sessions.consolidation_threshold
),
)
except Exception as exc:
logger.debug("Scheduler session store init failed: %s", exc)
_sched_tool_executor = (
ToolExecutor(resolved_tools, bus) if resolved_tools else None
)
system = JarvisSystem(
config=config,
bus=bus,
engine=engine,
engine_key=engine_name,
model=model_name,
agent=agent,
agent_name=agent_key or "",
tools=resolved_tools,
tool_executor=_sched_tool_executor,
memory_backend=memory_backend,
telemetry_store=telem_store,
trace_store=_trace_store,
session_store=_sched_session_store,
capability_policy=sec.capability_policy,
agent_manager=agent_manager,
agent_executor=executor,
)
executor.set_system(system)
agent_scheduler = AgentScheduler(
@@ -522,23 +593,6 @@ def serve(
except Exception as exc:
logger.debug("Agent scheduler init failed: %s", exc)
# Set up memory backend for context injection
memory_backend = None
if config.agent.context_from_memory:
try:
import openjarvis.tools.storage # noqa: F401
from openjarvis.core.registry import MemoryRegistry
mem_key = config.memory.default_backend
if MemoryRegistry.contains(mem_key):
memory_backend = MemoryRegistry.create(
mem_key,
db_path=config.memory.db_path,
)
console.print(" Memory: [cyan]active[/cyan]")
except Exception as exc:
logger.debug("Memory backend init failed: %s", exc)
# --- Channel Gateway: API key, sessions, ChannelBridge ---
import os as _os
+5
View File
@@ -229,6 +229,11 @@ class SystemPromptBuilder:
)
def _load_file(self, path_str: str, max_chars: int) -> str:
# An empty path means "no file" (e.g. the persona "none" opt-out, which
# resolves to empty paths). Guard before Path("") — which becomes "." —
# so reading it does not raise IsADirectoryError.
if not path_str:
return ""
path = Path(path_str).expanduser()
if not path.exists():
return ""
+25
View File
@@ -285,14 +285,39 @@ async def memory_config(request: Request):
async def memory_index(req: MemoryIndexRequest, request: Request):
"""Index files from a path into memory."""
try:
import os
from pathlib import Path
from openjarvis.security.file_policy import is_sensitive_file
from openjarvis.tools.storage.ingest import ingest_path
target = Path(req.path).expanduser().resolve()
if not target.exists():
raise HTTPException(status_code=404, detail=f"Path not found: {req.path}")
# Sandbox: when workspace roots are configured via OPENJARVIS_WORKSPACE
# (os.pathsep-separated), only allow indexing inside them. This endpoint
# must not become an arbitrary-filesystem read primitive over the API.
workspace = os.environ.get("OPENJARVIS_WORKSPACE", "").strip()
if workspace:
roots = [
Path(d).expanduser().resolve()
for d in workspace.split(os.pathsep)
if d.strip()
]
if not any(
target == root or root in target.parents for root in roots
):
raise HTTPException(
status_code=403,
detail="Path is outside the allowed workspace directories.",
)
# Never ingest sensitive files (.env, private keys, credentials, ...).
if target.is_file() and is_sensitive_file(target):
raise HTTPException(
status_code=403, detail="Refusing to index a sensitive file."
)
backend = _get_memory_backend(request)
if backend is None:
raise HTTPException(status_code=503, detail="Memory is not configured")
+16 -3
View File
@@ -33,7 +33,10 @@ class AuthMiddleware(BaseHTTPMiddleware):
status_code=401,
)
scheme, _, token = auth.partition(" ")
if scheme.lower() != "bearer" or token != self._api_key:
# Constant-time comparison to avoid leaking the key via timing.
if scheme.lower() != "bearer" or not secrets.compare_digest(
token, self._api_key
):
return JSONResponse(
{"detail": "Invalid API key"},
status_code=401,
@@ -42,8 +45,18 @@ class AuthMiddleware(BaseHTTPMiddleware):
@staticmethod
def _requires_auth(path: str) -> bool:
"""Only protect API routes, not the frontend UI or static assets."""
return path.startswith("/v1/") or path.startswith("/api/")
"""Protect API routes and operational metrics; leave the UI/health open.
``/metrics`` exposes request/token counters that should not be readable
by unauthenticated clients, so it is gated alongside ``/v1`` and
``/api``. ``/health`` stays open for liveness probes.
"""
return (
path.startswith("/v1/")
or path.startswith("/api/")
or path == "/metrics"
or path.startswith("/metrics/")
)
+45 -24
View File
@@ -113,7 +113,14 @@ def create_webhook_router(
signature = request.headers.get("X-Twilio-Signature", "")
url = str(request.url)
if twilio_auth_token and not _validate_twilio_signature(
# Fail closed: an unconfigured token means we cannot verify the sender,
# so reject rather than trust unsigned input.
if not twilio_auth_token:
logger.error(
"Twilio webhook rejected: TWILIO_AUTH_TOKEN not configured."
)
return Response("Webhook signature verification not configured", 403)
if not _validate_twilio_signature(
twilio_auth_token, url, params, signature
):
return Response("Invalid signature", status_code=403)
@@ -257,7 +264,13 @@ def create_webhook_router(
request: Request,
) -> Response:
auth = request.headers.get("Authorization", "")
if bluebubbles_password and auth != bluebubbles_password:
# Fail closed when no password is configured.
if not bluebubbles_password:
logger.error(
"BlueBubbles webhook rejected: password not configured."
)
return Response("Webhook authentication not configured", 403)
if not hmac.compare_digest(auth, bluebubbles_password):
return Response("Invalid password", status_code=403)
payload = await request.json()
@@ -292,7 +305,11 @@ def create_webhook_router(
token = request.query_params.get("hub.verify_token", "")
challenge = request.query_params.get("hub.challenge", "")
if mode == "subscribe" and token == whatsapp_verify_token:
# Fail closed: never echo the challenge if no verify token is set,
# otherwise an empty token would match an empty query value.
if not whatsapp_verify_token:
return Response("Forbidden", status_code=403)
if mode == "subscribe" and hmac.compare_digest(token, whatsapp_verify_token):
return PlainTextResponse(challenge)
return Response("Forbidden", status_code=403)
@@ -302,19 +319,23 @@ def create_webhook_router(
) -> Response:
body_bytes = await request.body()
# Verify signature
if whatsapp_app_secret:
signature = request.headers.get("X-Hub-Signature-256", "")
expected = (
"sha256="
+ hmac.new(
whatsapp_app_secret.encode(),
body_bytes,
hashlib.sha256,
).hexdigest()
# Fail closed: reject when no app secret is configured to verify HMAC.
if not whatsapp_app_secret:
logger.error(
"WhatsApp webhook rejected: app secret not configured."
)
if not hmac.compare_digest(signature, expected):
return Response("Invalid signature", status_code=403)
return Response("Webhook signature verification not configured", 403)
signature = request.headers.get("X-Hub-Signature-256", "")
expected = (
"sha256="
+ hmac.new(
whatsapp_app_secret.encode(),
body_bytes,
hashlib.sha256,
).hexdigest()
)
if not hmac.compare_digest(signature, expected):
return Response("Invalid signature", status_code=403)
payload = json.loads(body_bytes)
for entry in payload.get("entry", []):
@@ -349,16 +370,16 @@ def create_webhook_router(
# Get the SendBlue channel — may be passed at init or set later
sb = sendblue_channel or getattr(request.app.state, "sendblue_channel", None)
# Verify webhook secret if configured
if sb and sb.webhook_secret:
header_secret = request.headers.get("x-sendblue-secret", "")
if header_secret != sb.webhook_secret:
return Response("Invalid secret", status_code=403)
elif sb:
logger.warning(
"SendBlue webhook received without secret verification. "
"Set webhook_secret for HMAC validation."
# Fail closed: require a configured channel + webhook secret to verify
# the sender before processing any inbound message.
if sb is None or not getattr(sb, "webhook_secret", ""):
logger.error(
"SendBlue webhook rejected: webhook_secret not configured."
)
return Response("Webhook secret not configured", status_code=403)
header_secret = request.headers.get("x-sendblue-secret", "")
if not hmac.compare_digest(header_secret, sb.webhook_secret):
return Response("Invalid secret", status_code=403)
# Ignore outbound status callbacks
if payload.get("is_outbound", False):
+66 -7
View File
@@ -5,6 +5,7 @@ from __future__ import annotations
import logging
import os
import time
import urllib.parse
from typing import Any
import httpx
@@ -21,6 +22,13 @@ _MAX_RESPONSE_BYTES = 1_048_576
_ALLOWED_METHODS = frozenset({"GET", "POST", "PUT", "DELETE", "PATCH", "HEAD"})
# Cap redirect chains so a malicious server cannot loop us indefinitely.
_MAX_REDIRECTS = 5
class _SSRFRedirectError(Exception):
"""Raised when a redirect target fails the SSRF check."""
@ToolRegistry.register("http_request")
class HttpRequestTool(BaseTool):
@@ -136,13 +144,11 @@ class HttpRequestTool(BaseTool):
try:
t0 = time.time()
response = httpx.request(
method,
url,
headers=headers,
content=body,
timeout=float(timeout),
follow_redirects=True,
# Follow redirects manually so each hop is re-checked for SSRF — an
# allowed public URL must not be able to 30x-redirect us to an
# internal/metadata address.
response = self._request_following_redirects(
method, url, headers=headers, content=body, timeout=float(timeout)
)
elapsed_ms = (time.time() - t0) * 1000
@@ -178,6 +184,12 @@ class HttpRequestTool(BaseTool):
content=f"Request timed out after {timeout}s: {exc}",
success=False,
)
except _SSRFRedirectError as exc:
return ToolResult(
tool_name="http_request",
content=f"SSRF protection blocked redirect: {exc}",
success=False,
)
except httpx.RequestError as exc:
return ToolResult(
tool_name="http_request",
@@ -191,5 +203,52 @@ class HttpRequestTool(BaseTool):
success=False,
)
@staticmethod
def _request_following_redirects(
method: str,
url: str,
*,
headers: dict,
content: Any,
timeout: float,
) -> httpx.Response:
"""Issue the request, re-checking SSRF on every redirect hop.
httpx's built-in ``follow_redirects`` would chase a 30x ``Location``
without re-validating it, letting a public URL bounce us to an internal
host. We follow manually and run :func:`check_ssrf` on each target.
"""
current_url = url
current_method = method
body = content
# Use module-level ``httpx.request`` (not a private Client) so the SSRF
# re-check seam stays patchable by callers' tests, with redirects
# disabled so we control every hop ourselves.
for _ in range(_MAX_REDIRECTS + 1):
response = httpx.request(
current_method,
current_url,
headers=headers,
content=body,
timeout=timeout,
follow_redirects=False,
)
if response.status_code not in (301, 302, 303, 307, 308):
return response
location = response.headers.get("location", "")
if not location:
return response
# Resolve relative redirects against the URL we just fetched.
current_url = urllib.parse.urljoin(str(response.url), location)
ssrf_error = check_ssrf(current_url)
if ssrf_error:
raise _SSRFRedirectError(ssrf_error)
# Per RFC 7231, 301/302/303 turn the method into GET and drop
# the body (except for HEAD).
if response.status_code in (301, 302, 303) and current_method != "HEAD":
current_method = "GET"
body = None
raise _SSRFRedirectError(f"Exceeded maximum of {_MAX_REDIRECTS} redirects.")
__all__ = ["HttpRequestTool"]
+210
View File
@@ -0,0 +1,210 @@
"""Regression tests for #263 — ``jarvis serve`` must build the system once.
serve.py used to construct all heavy components inline and then call
``SystemBuilder(config).build()`` a second time inside the scheduler block,
re-discovering the engine, re-instrumenting it, re-resolving tools, re-opening
the channel and re-creating the agent manager — ~30-40s of redundant work.
These tests pin the fix:
1. ``SystemBuilder.build`` is never called during ``jarvis serve`` startup
(the duplicate build is gone).
2. The ``AgentExecutor`` still receives a system exposing the attributes it
actually reads: ``tool_executor``, ``session_store``, ``memory_backend``,
plus ``engine`` / ``model`` / ``config``.
"""
from __future__ import annotations
import importlib
from unittest.mock import MagicMock, patch
import pytest
from click.testing import CliRunner
from openjarvis.cli import cli
pytest.importorskip("fastapi")
pytest.importorskip("uvicorn")
# ``openjarvis.cli.serve`` as an attribute resolves to the click *command*
# (re-exported on the package); grab the real module to monkeypatch its globals.
serve_mod = importlib.import_module("openjarvis.cli.serve")
def _fake_engine() -> MagicMock:
engine = MagicMock()
engine.list_models.return_value = ["test-model"]
engine.health.return_value = True
engine.name = "mock"
return engine
def _repopulate_registries() -> None:
"""Re-run the @register decorators wiped by the autouse conftest fixture.
The tool/memory modules are import-cached, so a plain ``import`` inside
serve.py is a no-op after the registries are cleared per-test. Reload the
individual submodules so ToolRegistry/MemoryRegistry are populated exactly
as they would be on a fresh process — otherwise serve would resolve an
empty tool list and no memory backend, masking the very wiring under test.
"""
import importlib
import sys
import openjarvis.agents # noqa: F401
import openjarvis.tools # noqa: F401
import openjarvis.tools.storage # noqa: F401
from openjarvis.core.registry import (
AgentRegistry,
MemoryRegistry,
ToolRegistry,
)
if not AgentRegistry.keys():
for mod_name in list(sys.modules):
if mod_name.startswith("openjarvis.agents.") and not mod_name.endswith(
"_stubs"
):
try:
importlib.reload(sys.modules[mod_name])
except Exception:
pass
if not ToolRegistry.keys():
for mod_name in list(sys.modules):
if (
mod_name.startswith("openjarvis.tools.")
and not mod_name.endswith("_stubs")
and not mod_name.endswith("agent_tools")
):
try:
importlib.reload(sys.modules[mod_name])
except Exception:
pass
if not MemoryRegistry.keys():
for mod_name in list(sys.modules):
if mod_name.startswith(
"openjarvis.tools.storage."
) and not mod_name.endswith("_stubs"):
try:
importlib.reload(sys.modules[mod_name])
except Exception:
pass
def _run_serve(tmp_path, monkeypatch, *, build_spy, set_system_spy):
"""Invoke ``jarvis serve`` with all heavy/blocking pieces stubbed out.
Returns the CliRunner result. The server is never actually started
(``uvicorn.run`` is a no-op) and no real engine is contacted.
"""
from openjarvis.core.config import JarvisConfig
_repopulate_registries()
config = JarvisConfig()
# Keep the scheduler block alive (it owns the executor wiring under test)
# while pointing every store at the temp dir.
config.agent_manager.enabled = True
config.agent_manager.db_path = str(tmp_path / "agents.db")
config.sessions.enabled = True
config.sessions.db_path = str(tmp_path / "sessions.db")
config.memory.db_path = str(tmp_path / "memory.db")
config.telemetry.enabled = False
config.traces.enabled = False
config.channel.enabled = False
config.skills.enabled = False
config.server.host = "127.0.0.1"
config.server.port = 8123
# Resolve a model without contacting a real engine / discovery.
config.intelligence.default_model = "test-model"
engine = _fake_engine()
monkeypatch.setattr(serve_mod, "load_config", lambda *a, **k: config)
monkeypatch.setattr(serve_mod, "get_engine", lambda *a, **k: ("mock", engine))
monkeypatch.setattr(serve_mod, "discover_engines", lambda *a, **k: {})
monkeypatch.setattr(serve_mod, "discover_models", lambda *a, **k: {})
# setup_security returns its own context; pass the engine straight through
# so we don't need real guardrails wired up.
sec = MagicMock()
sec.engine = engine
sec.capability_policy = None
sec.audit_logger = None
monkeypatch.setattr("openjarvis.security.setup_security", lambda *a, **k: sec)
with (
patch(
"openjarvis.system.builder.SystemBuilder.build",
build_spy,
),
patch(
"openjarvis.agents.executor.AgentExecutor.set_system",
set_system_spy,
),
patch("uvicorn.run", lambda *a, **k: None),
):
return CliRunner().invoke(cli, ["serve"], catch_exceptions=False)
def test_serve_does_not_call_systembuilder_build(tmp_path, monkeypatch):
"""The redundant second full build is gone (#263)."""
build_spy = MagicMock(
side_effect=AssertionError(
"SystemBuilder.build() must not run during `jarvis serve` startup "
"— it is the duplicate build #263 removed."
)
)
set_system_spy = MagicMock()
result = _run_serve(
tmp_path,
monkeypatch,
build_spy=build_spy,
set_system_spy=set_system_spy,
)
assert result.exit_code == 0, result.output
build_spy.assert_not_called()
def test_executor_receives_required_system_attrs(tmp_path, monkeypatch):
"""The executor still gets a system exposing the attributes it reads.
AgentExecutor reads engine/model/config/memory_backend/tool_executor/
session_store off ``self._system``; the de-dup must not strip any of them.
"""
build_spy = MagicMock()
captured: dict = {}
def _capture_set_system(self, system): # noqa: ANN001
captured["system"] = system
# Preserve real behaviour so the executor is usable afterwards.
self._system = system
result = _run_serve(
tmp_path,
monkeypatch,
build_spy=build_spy,
set_system_spy=_capture_set_system,
)
assert result.exit_code == 0, result.output
# Built once, from the inline components — not via SystemBuilder.build().
build_spy.assert_not_called()
system = captured.get("system")
assert system is not None, "executor.set_system was never called"
# Correctness constraint from the verifier: these must survive the de-dup.
assert system.tool_executor is not None
assert system.session_store is not None
assert system.memory_backend is not None
# And the basics the executor resolves engine/model from.
assert system.engine is not None
assert system.model == "test-model"
assert system.config is not None
+20
View File
@@ -31,3 +31,23 @@ def test_named_persona_resolves_to_personas_dir():
def test_path_traversal_rejected(bad):
with pytest.raises(ValueError):
SystemPromptBuilder._resolve_persona(MemoryFilesConfig(persona_name=bad))
def test_none_persona_build_does_not_raise():
"""Regression (#497): `--persona none` resolves to empty file paths; building
the prompt must not raise IsADirectoryError when those empty paths are read
(Path("") is "." — reading a directory raised before the empty-path guard).
"""
import dataclasses
from openjarvis.core.config import load_config
cfg = load_config()
mf = dataclasses.replace(cfg.memory_files, persona_name="none")
builder = SystemPromptBuilder(
agent_template=cfg.agent.default_system_prompt or "",
memory_files_config=mf,
system_prompt_config=cfg.system_prompt,
)
out = builder.build()
assert isinstance(out, str)
+15
View File
@@ -28,6 +28,10 @@ def _make_app(api_key: str) -> FastAPI:
async def twilio_webhook():
return {"status": "received"}
@app.get("/metrics")
async def metrics():
return {"requests": 0}
return app
@@ -65,7 +69,18 @@ class TestAuthMiddleware:
resp = client.post("/webhooks/twilio")
assert resp.status_code == 200
def test_metrics_requires_auth(self, client):
resp = client.get("/metrics")
assert resp.status_code == 401
def test_metrics_accepts_valid_key(self, client):
resp = client.get(
"/metrics", headers={"Authorization": "Bearer oj_sk_test123"}
)
assert resp.status_code == 200
def test_no_key_configured_allows_all(self):
client = TestClient(_make_app(""))
resp = client.get("/v1/models")
assert resp.status_code == 200
assert client.get("/metrics").status_code == 200
+28 -2
View File
@@ -41,6 +41,9 @@ def sendblue_channel():
api_key_id="test_key",
api_secret_key="test_secret",
from_number="+15551234567",
# Webhooks now fail closed without a secret, so configure one and have
# the test client send the matching header by default.
webhook_secret="testsecret",
)
ch.connect()
return ch
@@ -61,7 +64,9 @@ def webhook_app(mock_bridge, sendblue_channel):
@pytest.fixture
def client(webhook_app):
return TestClient(webhook_app)
# Send the webhook secret by default so message-handling tests reach the
# bridge; fail-closed behavior is covered separately below.
return TestClient(webhook_app, headers={"x-sendblue-secret": "testsecret"})
# ---------------------------------------------------------------------------
@@ -169,7 +174,7 @@ class TestSendBlueWebhook:
app = FastAPI()
router = create_webhook_router(bridge=None, sendblue_channel=sendblue_channel)
app.include_router(router)
c = TestClient(app)
c = TestClient(app, headers={"x-sendblue-secret": "testsecret"})
resp = c.post(
"/webhooks/sendblue",
@@ -181,6 +186,27 @@ class TestSendBlueWebhook:
)
assert resp.status_code == 200
def test_no_secret_configured_is_rejected(self, mock_bridge):
"""Fail closed: a channel without a webhook_secret rejects all posts."""
from openjarvis.channels.sendblue import SendBlueChannel
from openjarvis.server.webhook_routes import create_webhook_router
ch = SendBlueChannel(
api_key_id="k", api_secret_key="s", from_number="+1555"
)
ch.connect()
app = FastAPI()
router = create_webhook_router(bridge=mock_bridge, sendblue_channel=ch)
app.include_router(router)
c = TestClient(app)
resp = c.post(
"/webhooks/sendblue",
json={"from_number": "+19127130720", "content": "Hi", "is_outbound": False},
)
assert resp.status_code == 403
mock_bridge.handle_incoming.assert_not_called()
# ---------------------------------------------------------------------------
# Health endpoint (requires agent_manager_routes)
+45
View File
@@ -213,3 +213,48 @@ class TestWhatsAppWebhook:
},
)
assert resp.status_code == 200
class TestWebhooksFailClosed:
"""When a channel's secret/token is unset, webhooks must reject (403)."""
def _client(self, mock_bridge, **kwargs):
app = FastAPI()
app.include_router(create_webhook_router(bridge=mock_bridge, **kwargs))
return TestClient(app)
def test_twilio_without_token_rejected(self, mock_bridge):
c = self._client(mock_bridge) # no twilio_auth_token
resp = c.post(
"/webhooks/twilio",
data={"From": "+15551234567", "Body": "hi", "MessageSid": "SM1"},
)
assert resp.status_code == 403
mock_bridge.handle_incoming.assert_not_called()
def test_bluebubbles_without_password_rejected(self, mock_bridge):
c = self._client(mock_bridge) # no bluebubbles_password
resp = c.post(
"/webhooks/bluebubbles",
json={"type": "new-message", "data": {}},
headers={"Authorization": "anything"},
)
assert resp.status_code == 403
def test_whatsapp_without_secret_rejected(self, mock_bridge):
c = self._client(mock_bridge) # no whatsapp_app_secret
resp = c.post(
"/webhooks/whatsapp",
content=b"{}",
headers={"Content-Type": "application/json"},
)
assert resp.status_code == 403
def test_whatsapp_verify_without_token_rejected(self, mock_bridge):
c = self._client(mock_bridge) # no whatsapp_verify_token
resp = c.get(
"/webhooks/whatsapp",
params={"hub.mode": "subscribe", "hub.verify_token": "",
"hub.challenge": "x"},
)
assert resp.status_code == 403
+41 -4
View File
@@ -184,8 +184,9 @@ class TestHttpRequestTool:
"""Timeout should produce a clear error."""
tool = HttpRequestTool()
with patch("openjarvis.tools.http_request.check_ssrf", return_value=None):
with patch(
"openjarvis.tools.http_request.httpx.request",
with patch.object(
HttpRequestTool,
"_request_following_redirects",
side_effect=httpx.TimeoutException("timed out"),
):
result = tool.execute(url="https://slow.example.com", timeout=5)
@@ -196,14 +197,50 @@ class TestHttpRequestTool:
"""Connection error should produce a clear error."""
tool = HttpRequestTool()
with patch("openjarvis.tools.http_request.check_ssrf", return_value=None):
with patch(
"openjarvis.tools.http_request.httpx.request",
with patch.object(
HttpRequestTool,
"_request_following_redirects",
side_effect=httpx.ConnectError("Connection refused"),
):
result = tool.execute(url="https://down.example.com")
assert result.success is False
assert "Request error" in result.content
@respx.mock
def test_redirect_to_private_ip_blocked(self):
"""A redirect to an internal/metadata host must be re-checked + blocked."""
respx.get("https://public.example.com/start").mock(
return_value=httpx.Response(
302, headers={"location": "http://169.254.169.254/latest/"}
)
)
tool = HttpRequestTool()
# First check (initial URL) passes; the redirect target is blocked.
with patch(
"openjarvis.tools.http_request.check_ssrf",
side_effect=[None, "Blocked host: 169.254.169.254"],
):
result = tool.execute(url="https://public.example.com/start")
assert result.success is False
assert "SSRF protection blocked redirect" in result.content
@respx.mock
def test_safe_redirect_is_followed(self):
"""A redirect to another public URL is followed normally."""
respx.get("https://public.example.com/start").mock(
return_value=httpx.Response(
302, headers={"location": "https://public.example.com/final"}
)
)
respx.get("https://public.example.com/final").mock(
return_value=httpx.Response(200, text="done")
)
tool = HttpRequestTool()
with patch("openjarvis.tools.http_request.check_ssrf", return_value=None):
result = tool.execute(url="https://public.example.com/start")
assert result.success is True
assert "done" in result.content
def test_method_validation(self):
"""Invalid HTTP method should be rejected."""
tool = HttpRequestTool()