mirror of
https://github.com/rookiestar28/ComfyUI-OpenClaw.git
synced 2026-08-14 00:48:07 +00:00
add R123 real-backend model-list SSRF parity lane and enforce no-skip CI/SOP/full-gate checks
This commit is contained in:
@@ -134,6 +134,8 @@ jobs:
|
||||
run: |
|
||||
# CRITICAL: this lane must stay low-mock and exercise real aiohttp request flow.
|
||||
python scripts/run_unittests.py --module tests.test_r122_real_backend_lane --enforce-skip-policy tests/skip_policy.json --max-skipped 0
|
||||
# R123: model-list loopback/private-IP SSRF parity must remain no-skip.
|
||||
python scripts/run_unittests.py --module tests.test_r123_real_backend_model_list_lane --enforce-skip-policy tests/skip_policy.json --max-skipped 0
|
||||
|
||||
contract-tests:
|
||||
name: Contract Tests (R52)
|
||||
|
||||
@@ -280,12 +280,12 @@ if [ "$NODE_MAJOR" -lt 18 ]; then
|
||||
fi
|
||||
|
||||
echo "[pre-push] Node version: $(node -v)"
|
||||
echo "[pre-push] 0/4 R120 dependency preflight"
|
||||
echo "[pre-push] 0/5 R120 dependency preflight"
|
||||
"$VENV_PY" scripts/preflight_check.py --strict
|
||||
echo "[pre-push] 1/4 detect-secrets"
|
||||
echo "[pre-push] 1/5 detect-secrets"
|
||||
run_pre_commit_safe run detect-secrets --all-files
|
||||
|
||||
echo "[pre-push] 2/4 pre-commit all hooks (pass 1)"
|
||||
echo "[pre-push] 2/5 pre-commit all hooks (pass 1)"
|
||||
if run_pre_commit_safe run --all-files --show-diff-on-failure; then
|
||||
:
|
||||
else
|
||||
@@ -303,16 +303,22 @@ else
|
||||
fi
|
||||
fi
|
||||
|
||||
echo "[pre-push] 3/4 backend unit tests"
|
||||
echo "[pre-push] 3/5 backend unit tests"
|
||||
MOLTBOT_STATE_DIR="$ROOT_DIR/moltbot_state/_pre_push_unit" \
|
||||
"$VENV_PY" scripts/run_unittests.py --start-dir tests --pattern "test_*.py" --enforce-skip-policy tests/skip_policy.json
|
||||
|
||||
if [ -n "${OPENCLAW_IMPL_RECORD_PATH:-}" ]; then
|
||||
echo "[pre-push] 3.5/4 implementation record lint (strict)"
|
||||
echo "[pre-push] 3.5/5 implementation record lint (strict)"
|
||||
"$VENV_PY" scripts/lint_implementation_record.py --path "$OPENCLAW_IMPL_RECORD_PATH" --strict
|
||||
fi
|
||||
|
||||
echo "[pre-push] 4/4 npm test (Playwright)"
|
||||
echo "[pre-push] 4/5 backend real E2E lanes (R122/R123)"
|
||||
MOLTBOT_STATE_DIR="$ROOT_DIR/moltbot_state/_pre_push_backend_e2e_real" \
|
||||
"$VENV_PY" scripts/run_unittests.py --module tests.test_r122_real_backend_lane --enforce-skip-policy tests/skip_policy.json --max-skipped 0
|
||||
MOLTBOT_STATE_DIR="$ROOT_DIR/moltbot_state/_pre_push_backend_e2e_real" \
|
||||
"$VENV_PY" scripts/run_unittests.py --module tests.test_r123_real_backend_model_list_lane --enforce-skip-policy tests/skip_policy.json --max-skipped 0
|
||||
|
||||
echo "[pre-push] 5/5 npm test (Playwright)"
|
||||
npm test
|
||||
|
||||
echo "[pre-push] PASS"
|
||||
|
||||
@@ -110,13 +110,13 @@ fi
|
||||
|
||||
echo "[tests] Node version: $(node -v)"
|
||||
|
||||
echo "[tests] 0/4 R120 dependency preflight"
|
||||
echo "[tests] 0/5 R120 dependency preflight"
|
||||
"$VENV_PY" scripts/preflight_check.py --strict
|
||||
|
||||
echo "[tests] 1/4 detect-secrets"
|
||||
echo "[tests] 1/5 detect-secrets"
|
||||
"$VENV_PY" -m pre_commit run detect-secrets --all-files
|
||||
|
||||
echo "[tests] 2/4 pre-commit all hooks (pass 1: autofix)"
|
||||
echo "[tests] 2/5 pre-commit all hooks (pass 1: autofix)"
|
||||
if "$VENV_PY" -m pre_commit run --all-files --show-diff-on-failure; then
|
||||
:
|
||||
else
|
||||
@@ -124,16 +124,22 @@ else
|
||||
"$VENV_PY" -m pre_commit run --all-files --show-diff-on-failure
|
||||
fi
|
||||
|
||||
echo "[tests] 3/4 backend unit tests"
|
||||
echo "[tests] 3/5 backend unit tests"
|
||||
MOLTBOT_STATE_DIR="$ROOT_DIR/moltbot_state/_local_unit" "$VENV_PY" scripts/run_unittests.py --start-dir tests --pattern "test_*.py" --enforce-skip-policy tests/skip_policy.json
|
||||
|
||||
if [ -n "${OPENCLAW_IMPL_RECORD_PATH:-}" ]; then
|
||||
echo "[tests] 3.5/4 implementation record lint (strict)"
|
||||
echo "[tests] 3.5/5 implementation record lint (strict)"
|
||||
# IMPORTANT: strict mode is opt-in via OPENCLAW_IMPL_RECORD_PATH to avoid retroactive legacy record failures.
|
||||
"$VENV_PY" scripts/lint_implementation_record.py --path "$OPENCLAW_IMPL_RECORD_PATH" --strict
|
||||
fi
|
||||
|
||||
echo "[tests] 4/4 frontend E2E"
|
||||
echo "[tests] 4/5 backend real E2E lanes (R122/R123)"
|
||||
MOLTBOT_STATE_DIR="$ROOT_DIR/moltbot_state/_local_backend_e2e_real" \
|
||||
"$VENV_PY" scripts/run_unittests.py --module tests.test_r122_real_backend_lane --enforce-skip-policy tests/skip_policy.json --max-skipped 0
|
||||
MOLTBOT_STATE_DIR="$ROOT_DIR/moltbot_state/_local_backend_e2e_real" \
|
||||
"$VENV_PY" scripts/run_unittests.py --module tests.test_r123_real_backend_model_list_lane --enforce-skip-policy tests/skip_policy.json --max-skipped 0
|
||||
|
||||
echo "[tests] 5/5 frontend E2E"
|
||||
npm test
|
||||
|
||||
echo "[tests] PASS"
|
||||
|
||||
@@ -152,34 +152,43 @@ if ($nodeMajor -lt 18) {
|
||||
|
||||
Write-Host "[tests] Node version: $(node -v)"
|
||||
|
||||
Write-Host "[tests] 0/4 R120 dependency preflight"
|
||||
Write-Host "[tests] 0/5 R120 dependency preflight"
|
||||
Invoke-Checked "preflight_check" { & $venvPython scripts\preflight_check.py --strict }
|
||||
|
||||
Write-Host "[tests] 1/4 detect-secrets"
|
||||
Write-Host "[tests] 1/5 detect-secrets"
|
||||
Invoke-Checked "detect-secrets" { & $venvPython -m pre_commit run detect-secrets --all-files }
|
||||
|
||||
Write-Host "[tests] 2/4 pre-commit all hooks (pass 1: autofix)"
|
||||
Write-Host "[tests] 2/5 pre-commit all hooks (pass 1: autofix)"
|
||||
& $venvPython -m pre_commit run --all-files --show-diff-on-failure
|
||||
if ($LASTEXITCODE -ne 0) {
|
||||
Write-Host "[tests] INFO: pre-commit reported changes/issues; running pass 2 verification..."
|
||||
Invoke-Checked "pre-commit all hooks (pass 2 verify)" { & $venvPython -m pre_commit run --all-files --show-diff-on-failure }
|
||||
}
|
||||
|
||||
Write-Host "[tests] 3/4 backend unit tests"
|
||||
Write-Host "[tests] 3/5 backend unit tests"
|
||||
$env:MOLTBOT_STATE_DIR = "$root\moltbot_state\_local_unit"
|
||||
Invoke-Checked "backend unit tests" {
|
||||
& $venvPython scripts\run_unittests.py --start-dir tests --pattern "test_*.py" --enforce-skip-policy tests\skip_policy.json
|
||||
}
|
||||
|
||||
if ($env:OPENCLAW_IMPL_RECORD_PATH) {
|
||||
Write-Host "[tests] 3.5/4 implementation record lint (strict)"
|
||||
Write-Host "[tests] 3.5/5 implementation record lint (strict)"
|
||||
# IMPORTANT: strict mode is opt-in via OPENCLAW_IMPL_RECORD_PATH to avoid retroactive legacy record failures.
|
||||
Invoke-Checked "implementation record lint" {
|
||||
& $venvPython scripts\lint_implementation_record.py --path $env:OPENCLAW_IMPL_RECORD_PATH --strict
|
||||
}
|
||||
}
|
||||
|
||||
Write-Host "[tests] 4/4 frontend E2E"
|
||||
Write-Host "[tests] 4/5 backend real E2E lanes (R122/R123)"
|
||||
$env:MOLTBOT_STATE_DIR = "$root\moltbot_state\_local_backend_e2e_real"
|
||||
Invoke-Checked "backend real E2E lane R122" {
|
||||
& $venvPython scripts\run_unittests.py --module tests.test_r122_real_backend_lane --enforce-skip-policy tests\skip_policy.json --max-skipped 0
|
||||
}
|
||||
Invoke-Checked "backend real E2E lane R123" {
|
||||
& $venvPython scripts\run_unittests.py --module tests.test_r123_real_backend_model_list_lane --enforce-skip-policy tests\skip_policy.json --max-skipped 0
|
||||
}
|
||||
|
||||
Write-Host "[tests] 5/5 frontend E2E"
|
||||
Invoke-Checked "frontend E2E" { npm test }
|
||||
|
||||
Write-Host "[tests] PASS"
|
||||
|
||||
@@ -1,2 +1,6 @@
|
||||
All E2E tests must be performed using the standard procedures defined in
|
||||
`tests/E2E_TESTING_SOP.md`.
|
||||
|
||||
Scope note:
|
||||
- `tests/E2E_TESTING_SOP.md` is frontend Playwright harness SOP.
|
||||
- Backend real-E2E lanes (`tests.test_r122_real_backend_lane`, `tests.test_r123_real_backend_model_list_lane`) are governed by `tests/TEST_SOP.md`.
|
||||
|
||||
@@ -3,6 +3,10 @@
|
||||
This SOP documents the verified, repeatable steps to run Playwright E2E tests
|
||||
against a local **test harness** (no live ComfyUI backend required).
|
||||
|
||||
Boundary:
|
||||
- This file covers frontend Playwright harness E2E only.
|
||||
- Backend low-mock real lanes (`R122`, `R123`) are specified in `tests/TEST_SOP.md`.
|
||||
|
||||
## 1. Requirements
|
||||
|
||||
- Node.js 18+
|
||||
|
||||
+6
-2
@@ -70,6 +70,7 @@ Failed preflight checks must be resolved before proceeding with full test suites
|
||||
- `tests.security.test_endpoint_drift`
|
||||
- Real-backend low-mock lane must be no-skip in CI:
|
||||
- `tests.test_r122_real_backend_lane`
|
||||
- `tests.test_r123_real_backend_model_list_lane` (model-list loopback SSRF regression lane)
|
||||
|
||||
- **R112 (security triple-assert)**:
|
||||
- For security reject/degrade paths, tests should assert all three signals:
|
||||
@@ -190,16 +191,18 @@ Then every `git push` will run:
|
||||
bash scripts/pre_push_checks.sh
|
||||
```
|
||||
|
||||
`scripts/pre_push_checks.sh` is the CI-parity guard and must include all 4 stages:
|
||||
`scripts/pre_push_checks.sh` is the CI-parity guard and must include all 5 stages:
|
||||
|
||||
1) `detect-secrets`
|
||||
2) all `pre-commit` hooks
|
||||
3) backend unit tests (`scripts/run_unittests.py --pattern "test_*.py" --enforce-skip-policy tests/skip_policy.json`)
|
||||
4) frontend E2E (`npm test`)
|
||||
4) backend real E2E lanes (`tests.test_r122_real_backend_lane` + `tests.test_r123_real_backend_model_list_lane`)
|
||||
5) frontend E2E (`npm test`)
|
||||
|
||||
IMPORTANT:
|
||||
|
||||
- Do not remove stage (3). If pre-push skips backend unit tests, local pushes can pass while GitHub CI fails later.
|
||||
- Do not remove stage (4). If pre-push skips real-backend lanes, model-list/webhook wiring regressions can bypass local checks and fail later in CI.
|
||||
- Keep dependency bootstrap in this script aligned with `.github/workflows/ci.yml` unit-test dependencies.
|
||||
|
||||
1) Detect Secrets (baseline-based)
|
||||
@@ -241,6 +244,7 @@ MOLTBOT_STATE_DIR="$(pwd)/moltbot_state/_local_unit" python scripts/run_unittest
|
||||
|
||||
```bash
|
||||
MOLTBOT_STATE_DIR="$(pwd)/moltbot_state/_local_backend_e2e_real" python scripts/run_unittests.py --module tests.test_r122_real_backend_lane --enforce-skip-policy tests/skip_policy.json --max-skipped 0
|
||||
MOLTBOT_STATE_DIR="$(pwd)/moltbot_state/_local_backend_e2e_real" python scripts/run_unittests.py --module tests.test_r123_real_backend_model_list_lane --enforce-skip-policy tests/skip_policy.json --max-skipped 0
|
||||
```
|
||||
|
||||
1) Frontend E2E (Playwright; CI enforces)
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
"tests.test_s60_routes_startup_gate",
|
||||
"tests.test_s61_registry_signature",
|
||||
"tests.security.test_endpoint_drift",
|
||||
"tests.test_r122_real_backend_lane"
|
||||
"tests.test_r122_real_backend_lane",
|
||||
"tests.test_r123_real_backend_model_list_lane"
|
||||
]
|
||||
}
|
||||
|
||||
@@ -0,0 +1,209 @@
|
||||
"""
|
||||
R123: Real-backend model-list lane (low-mock).
|
||||
|
||||
This suite validates /openclaw/llm/models through a real aiohttp upstream
|
||||
service and real safe_io SSRF checks. It closes the loopback-regression gap
|
||||
left by mock-heavy settings/model-list tests.
|
||||
"""
|
||||
|
||||
import json
|
||||
import os
|
||||
import shutil
|
||||
import tempfile
|
||||
import threading
|
||||
import unittest
|
||||
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
|
||||
from unittest.mock import patch
|
||||
|
||||
from aiohttp import web
|
||||
from aiohttp.test_utils import AioHTTPTestCase, unittest_run_loop
|
||||
|
||||
from api.config import _MODEL_LIST_CACHE, llm_models_handler
|
||||
|
||||
|
||||
class TestR123RealBackendModelListLane(AioHTTPTestCase):
|
||||
"""Low-mock backend lane for llm model-list SSRF parity."""
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
self._patchers = []
|
||||
self._fixtures_dir = tempfile.mkdtemp(prefix="openclaw-r123-")
|
||||
self._upstream_server = None
|
||||
self._upstream_thread = None
|
||||
self._upstream_base_url = ""
|
||||
self._models_payload = {
|
||||
"data": [{"id": "gemma3:4b"}, {"id": "llama3.2:3b"}],
|
||||
}
|
||||
|
||||
env_patch = patch.dict(
|
||||
os.environ,
|
||||
{
|
||||
"OPENCLAW_ADMIN_TOKEN": "r123-admin-token",
|
||||
# IMPORTANT: real-backend lane focuses on model-list SSRF parity,
|
||||
# so keep remote-admin guard out of scope for deterministic results.
|
||||
"OPENCLAW_ALLOW_REMOTE_ADMIN": "1",
|
||||
"OPENCLAW_DEPLOYMENT_PROFILE": "local",
|
||||
"MOLTBOT_STATE_DIR": os.path.join(self._fixtures_dir, "state"),
|
||||
},
|
||||
)
|
||||
env_patch.start()
|
||||
self._patchers.append(env_patch)
|
||||
_MODEL_LIST_CACHE.clear()
|
||||
self._start_upstream_http_server()
|
||||
|
||||
def _start_upstream_http_server(self):
|
||||
payload_bytes = json.dumps(self._models_payload).encode("utf-8")
|
||||
|
||||
class ModelsHandler(BaseHTTPRequestHandler):
|
||||
def do_GET(self):
|
||||
if self.path in ("/models", "/v1/models"):
|
||||
self.send_response(200)
|
||||
self.send_header("Content-Type", "application/json")
|
||||
self.send_header("Content-Length", str(len(payload_bytes)))
|
||||
self.end_headers()
|
||||
self.wfile.write(payload_bytes)
|
||||
return
|
||||
self.send_response(404)
|
||||
self.end_headers()
|
||||
|
||||
def log_message(self, format, *args):
|
||||
return
|
||||
|
||||
bind_ports = (11434, 1234, 5000, 8080, 8443)
|
||||
last_err = None
|
||||
for port in bind_ports:
|
||||
try:
|
||||
# CRITICAL:
|
||||
# Debug finding from R123 bring-up:
|
||||
# - llm_models_handler -> safe_request_json uses synchronous urllib.
|
||||
# - If upstream is served by aiohttp TestServer on the same event loop,
|
||||
# request handling can deadlock/time out (handler waits on itself).
|
||||
# Keep upstream in a separate threaded HTTP server to preserve real HTTP
|
||||
# behavior while avoiding same-loop re-entrancy deadlock.
|
||||
server = ThreadingHTTPServer(("127.0.0.1", port), ModelsHandler)
|
||||
self._upstream_server = server
|
||||
self._upstream_base_url = f"http://127.0.0.1:{port}"
|
||||
self._upstream_thread = threading.Thread(
|
||||
target=server.serve_forever,
|
||||
kwargs={"poll_interval": 0.05},
|
||||
daemon=True,
|
||||
)
|
||||
self._upstream_thread.start()
|
||||
return
|
||||
except OSError as e:
|
||||
last_err = e
|
||||
|
||||
raise RuntimeError(
|
||||
f"Unable to bind upstream HTTP server on allowed ports {bind_ports}: {last_err}"
|
||||
)
|
||||
|
||||
def tearDown(self):
|
||||
for p in reversed(self._patchers):
|
||||
p.stop()
|
||||
if self._upstream_server is not None:
|
||||
self._upstream_server.shutdown()
|
||||
self._upstream_server.server_close()
|
||||
self._upstream_server = None
|
||||
self._upstream_thread = None
|
||||
_MODEL_LIST_CACHE.clear()
|
||||
shutil.rmtree(self._fixtures_dir, ignore_errors=True)
|
||||
super().tearDown()
|
||||
|
||||
async def get_application(self):
|
||||
app = web.Application()
|
||||
app.router.add_get("/openclaw/llm/models", llm_models_handler)
|
||||
app.router.add_get("/moltbot/llm/models", llm_models_handler)
|
||||
return app
|
||||
|
||||
def _admin_headers(self):
|
||||
return {"X-OpenClaw-Admin-Token": "r123-admin-token"}
|
||||
|
||||
@unittest_run_loop
|
||||
async def test_local_ollama_loopback_models_success_openclaw(self):
|
||||
base_url = self._upstream_base_url
|
||||
with patch.dict(
|
||||
os.environ,
|
||||
{
|
||||
"OPENCLAW_LLM_PROVIDER": "ollama",
|
||||
"OPENCLAW_LLM_BASE_URL": base_url,
|
||||
},
|
||||
):
|
||||
_MODEL_LIST_CACHE.clear()
|
||||
resp = await self.client.get(
|
||||
"/openclaw/llm/models",
|
||||
headers=self._admin_headers(),
|
||||
)
|
||||
|
||||
self.assertEqual(resp.status, 200)
|
||||
data = await resp.json()
|
||||
self.assertTrue(data["ok"])
|
||||
self.assertIn("gemma3:4b", data["models"])
|
||||
|
||||
@unittest_run_loop
|
||||
async def test_local_lmstudio_loopback_models_success_legacy_prefix(self):
|
||||
base_url = f"{self._upstream_base_url}/v1"
|
||||
with patch.dict(
|
||||
os.environ,
|
||||
{
|
||||
"OPENCLAW_LLM_PROVIDER": "lmstudio",
|
||||
"OPENCLAW_LLM_BASE_URL": base_url,
|
||||
},
|
||||
):
|
||||
_MODEL_LIST_CACHE.clear()
|
||||
resp = await self.client.get(
|
||||
"/moltbot/llm/models",
|
||||
headers=self._admin_headers(),
|
||||
)
|
||||
|
||||
self.assertEqual(resp.status, 200)
|
||||
data = await resp.json()
|
||||
self.assertTrue(data["ok"])
|
||||
self.assertEqual(data["provider"], "lmstudio")
|
||||
self.assertIn("llama3.2:3b", data["models"])
|
||||
|
||||
@unittest_run_loop
|
||||
async def test_private_non_loopback_ip_is_fail_closed(self):
|
||||
with patch.dict(
|
||||
os.environ,
|
||||
{
|
||||
"OPENCLAW_LLM_PROVIDER": "ollama",
|
||||
"OPENCLAW_LLM_BASE_URL": "http://192.168.1.5:11434",
|
||||
"OPENCLAW_ALLOW_ANY_PUBLIC_LLM_HOST": "1",
|
||||
},
|
||||
):
|
||||
_MODEL_LIST_CACHE.clear()
|
||||
resp = await self.client.get(
|
||||
"/openclaw/llm/models",
|
||||
headers=self._admin_headers(),
|
||||
)
|
||||
|
||||
self.assertEqual(resp.status, 403)
|
||||
data = await resp.json()
|
||||
self.assertIn("SSRF policy blocked", data.get("error", ""))
|
||||
self.assertIn("Private/reserved IP blocked", data.get("error", ""))
|
||||
|
||||
@unittest_run_loop
|
||||
async def test_allow_any_public_host_does_not_break_loopback_parity(self):
|
||||
base_url = self._upstream_base_url
|
||||
with patch.dict(
|
||||
os.environ,
|
||||
{
|
||||
"OPENCLAW_LLM_PROVIDER": "ollama",
|
||||
"OPENCLAW_LLM_BASE_URL": base_url,
|
||||
"OPENCLAW_ALLOW_ANY_PUBLIC_LLM_HOST": "1",
|
||||
},
|
||||
):
|
||||
_MODEL_LIST_CACHE.clear()
|
||||
resp = await self.client.get(
|
||||
"/openclaw/llm/models",
|
||||
headers=self._admin_headers(),
|
||||
)
|
||||
|
||||
self.assertEqual(resp.status, 200)
|
||||
data = await resp.json()
|
||||
self.assertTrue(data["ok"])
|
||||
self.assertIn("gemma3:4b", data["models"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user