fix: cover API auth guard regressions
Maintainer edit: restore CI by updating serve/onboard tests, add auth/config coverage, and keep auth failures on the OpenAI-compatible error shape.
This commit is contained in:
@@ -107,6 +107,7 @@ File operations have path traversal protection, but:
|
|||||||
**API Calls:**
|
**API Calls:**
|
||||||
- All external API calls use HTTPS by default
|
- All external API calls use HTTPS by default
|
||||||
- Timeouts are configured to prevent hanging requests
|
- Timeouts are configured to prevent hanging requests
|
||||||
|
- The OpenAI-compatible API server must set `api.api_key` when binding to `0.0.0.0` or `::`; otherwise startup fails to prevent unauthenticated network access
|
||||||
- Consider using a firewall to restrict outbound connections if needed
|
- Consider using a firewall to restrict outbound connections if needed
|
||||||
|
|
||||||
**WhatsApp:**
|
**WhatsApp:**
|
||||||
|
|||||||
@@ -421,14 +421,9 @@ def create_app(
|
|||||||
return await handler(request)
|
return await handler(request)
|
||||||
auth = request.headers.get("Authorization", "")
|
auth = request.headers.get("Authorization", "")
|
||||||
if not auth.startswith("Bearer "):
|
if not auth.startswith("Bearer "):
|
||||||
return web.json_response(
|
return _error_json(401, "Missing Authorization header. Use: Bearer <api_key>")
|
||||||
{"error": "Missing Authorization header. Use: Bearer <api_key>"},
|
|
||||||
status=401,
|
|
||||||
)
|
|
||||||
if not hmac.compare_digest(auth[len("Bearer "):], api_key):
|
if not hmac.compare_digest(auth[len("Bearer "):], api_key):
|
||||||
return web.json_response(
|
return _error_json(401, "Invalid API key")
|
||||||
{"error": "Invalid API key"}, status=401,
|
|
||||||
)
|
|
||||||
return await handler(request)
|
return await handler(request)
|
||||||
|
|
||||||
app.middlewares.append(auth_middleware)
|
app.middlewares.append(auth_middleware)
|
||||||
|
|||||||
@@ -853,10 +853,11 @@ class TestApiServerRegistration:
|
|||||||
config = Config()
|
config = Config()
|
||||||
from nanobot.config.schema import ApiConfig
|
from nanobot.config.schema import ApiConfig
|
||||||
|
|
||||||
new_api = ApiConfig(host="0.0.0.0", port=9999)
|
new_api = ApiConfig(host="0.0.0.0", port=9999, api_key="secret")
|
||||||
_SETTINGS_SETTER["API Server"](config, new_api)
|
_SETTINGS_SETTER["API Server"](config, new_api)
|
||||||
assert config.api.host == "0.0.0.0"
|
assert config.api.host == "0.0.0.0"
|
||||||
assert config.api.port == 9999
|
assert config.api.port == 9999
|
||||||
|
assert config.api.api_key == "secret"
|
||||||
|
|
||||||
|
|
||||||
class TestMainMenuUpdate:
|
class TestMainMenuUpdate:
|
||||||
|
|||||||
@@ -1561,10 +1561,16 @@ def _patch_serve_runtime(monkeypatch, config: Config, seen: dict[str, object]) -
|
|||||||
async def close_mcp(self) -> None:
|
async def close_mcp(self) -> None:
|
||||||
return None
|
return None
|
||||||
|
|
||||||
def _fake_create_app(agent_loop, model_name: str, request_timeout: float):
|
def _fake_create_app(
|
||||||
|
agent_loop,
|
||||||
|
model_name: str,
|
||||||
|
request_timeout: float,
|
||||||
|
api_key: str = "",
|
||||||
|
):
|
||||||
seen["agent_loop"] = agent_loop
|
seen["agent_loop"] = agent_loop
|
||||||
seen["model_name"] = model_name
|
seen["model_name"] = model_name
|
||||||
seen["request_timeout"] = request_timeout
|
seen["request_timeout"] = request_timeout
|
||||||
|
seen["api_key"] = api_key
|
||||||
return _FakeApiApp()
|
return _FakeApiApp()
|
||||||
|
|
||||||
def _fake_run_app(api_app, host: str, port: int, print):
|
def _fake_run_app(api_app, host: str, port: int, print):
|
||||||
@@ -2507,6 +2513,7 @@ def test_serve_uses_api_config_defaults_and_workspace_override(
|
|||||||
assert seen["host"] == "127.0.0.2"
|
assert seen["host"] == "127.0.0.2"
|
||||||
assert seen["port"] == 18900
|
assert seen["port"] == 18900
|
||||||
assert seen["request_timeout"] == 45.0
|
assert seen["request_timeout"] == 45.0
|
||||||
|
assert seen["api_key"] == ""
|
||||||
|
|
||||||
|
|
||||||
def test_serve_cli_options_override_api_config(monkeypatch, tmp_path: Path) -> None:
|
def test_serve_cli_options_override_api_config(monkeypatch, tmp_path: Path) -> None:
|
||||||
@@ -2538,6 +2545,35 @@ def test_serve_cli_options_override_api_config(monkeypatch, tmp_path: Path) -> N
|
|||||||
assert seen["host"] == "127.0.0.1"
|
assert seen["host"] == "127.0.0.1"
|
||||||
assert seen["port"] == 18901
|
assert seen["port"] == 18901
|
||||||
assert seen["request_timeout"] == 46.0
|
assert seen["request_timeout"] == 46.0
|
||||||
|
assert seen["api_key"] == ""
|
||||||
|
|
||||||
|
|
||||||
|
def test_serve_passes_configured_api_key(monkeypatch, tmp_path: Path) -> None:
|
||||||
|
config_file = _write_instance_config(tmp_path)
|
||||||
|
config = Config()
|
||||||
|
config.api.api_key = " secret "
|
||||||
|
seen: dict[str, object] = {}
|
||||||
|
|
||||||
|
_patch_serve_runtime(monkeypatch, config, seen)
|
||||||
|
|
||||||
|
result = runner.invoke(app, ["serve", "--config", str(config_file)])
|
||||||
|
|
||||||
|
assert result.exit_code == 0
|
||||||
|
assert seen["api_key"] == "secret"
|
||||||
|
|
||||||
|
|
||||||
|
def test_serve_rejects_wildcard_host_without_api_key(monkeypatch, tmp_path: Path) -> None:
|
||||||
|
config_file = _write_instance_config(tmp_path)
|
||||||
|
config = Config()
|
||||||
|
seen: dict[str, object] = {}
|
||||||
|
|
||||||
|
_patch_serve_runtime(monkeypatch, config, seen)
|
||||||
|
|
||||||
|
result = runner.invoke(app, ["serve", "--config", str(config_file), "--host", "0.0.0.0"])
|
||||||
|
|
||||||
|
assert result.exit_code == 1
|
||||||
|
assert "api_key is not set" in result.stdout
|
||||||
|
assert "api_app" not in seen
|
||||||
|
|
||||||
|
|
||||||
def test_channels_login_requires_channel_name() -> None:
|
def test_channels_login_requires_channel_name() -> None:
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ import json
|
|||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from nanobot.config.loader import load_config
|
from nanobot.config.loader import load_config
|
||||||
|
from nanobot.config.schema import ApiConfig
|
||||||
|
|
||||||
|
|
||||||
def test_load_config_missing_file_uses_defaults(tmp_path) -> None:
|
def test_load_config_missing_file_uses_defaults(tmp_path) -> None:
|
||||||
@@ -28,3 +29,16 @@ def test_load_config_invalid_schema_fails_fast(tmp_path) -> None:
|
|||||||
|
|
||||||
with pytest.raises(ValueError, match="Failed to load config"):
|
with pytest.raises(ValueError, match="Failed to load config"):
|
||||||
load_config(config_path)
|
load_config(config_path)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("host", ["0.0.0.0", "::"])
|
||||||
|
def test_api_config_requires_key_for_wildcard_hosts(host: str) -> None:
|
||||||
|
with pytest.raises(ValueError, match="api_key is not set"):
|
||||||
|
ApiConfig(host=host)
|
||||||
|
|
||||||
|
|
||||||
|
def test_api_config_allows_wildcard_host_with_key() -> None:
|
||||||
|
config = ApiConfig(host="0.0.0.0", api_key="secret")
|
||||||
|
|
||||||
|
assert config.host == "0.0.0.0"
|
||||||
|
assert config.api_key == "secret"
|
||||||
|
|||||||
@@ -108,6 +108,25 @@ async def test_missing_messages_returns_400(aiohttp_client, app) -> None:
|
|||||||
assert resp.status == 400
|
assert resp.status == 400
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.skipif(not HAS_AIOHTTP, reason="aiohttp not installed")
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_api_key_protects_api_routes_but_not_health(aiohttp_client, mock_agent) -> None:
|
||||||
|
app = create_app(mock_agent, model_name="test-model", api_key="secret")
|
||||||
|
client = await aiohttp_client(app)
|
||||||
|
|
||||||
|
health = await client.get("/health")
|
||||||
|
missing = await client.get("/v1/models")
|
||||||
|
wrong = await client.get("/v1/models", headers={"Authorization": "Bearer wrong"})
|
||||||
|
ok = await client.get("/v1/models", headers={"Authorization": "Bearer secret"})
|
||||||
|
|
||||||
|
assert health.status == 200
|
||||||
|
assert missing.status == 401
|
||||||
|
assert wrong.status == 401
|
||||||
|
assert ok.status == 200
|
||||||
|
assert (await missing.json())["error"]["message"].startswith("Missing Authorization")
|
||||||
|
assert (await wrong.json())["error"]["message"] == "Invalid API key"
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.skipif(not HAS_AIOHTTP, reason="aiohttp not installed")
|
@pytest.mark.skipif(not HAS_AIOHTTP, reason="aiohttp not installed")
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_no_user_message_returns_400(aiohttp_client, app) -> None:
|
async def test_no_user_message_returns_400(aiohttp_client, app) -> None:
|
||||||
|
|||||||
Reference in New Issue
Block a user