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
13 changed files with 385 additions and 66 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))
+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"]
+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()