From db40c0c538501d68d37d9ba7bd9d2fb209d46480 Mon Sep 17 00:00:00 2001 From: rookiestar28 <151893693+rookiestar28@users.noreply.github.com> Date: Thu, 19 Feb 2026 15:22:48 +0800 Subject: [PATCH] add R123 real-backend model-list SSRF parity lane and enforce no-skip CI/SOP/full-gate checks --- .github/workflows/ci.yml | 2 + scripts/pre_push_checks.sh | 18 +- scripts/run_full_tests_linux.sh | 18 +- scripts/run_full_tests_windows.ps1 | 21 +- tests/E2E_TESTING_NOTICE.md | 4 + tests/E2E_TESTING_SOP.md | 4 + tests/TEST_SOP.md | 8 +- tests/skip_policy.json | 3 +- .../test_r123_real_backend_model_list_lane.py | 209 ++++++++++++++++++ 9 files changed, 266 insertions(+), 21 deletions(-) create mode 100644 tests/test_r123_real_backend_model_list_lane.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 29270a8..419c9ec 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -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) diff --git a/scripts/pre_push_checks.sh b/scripts/pre_push_checks.sh index c528f75..2202692 100644 --- a/scripts/pre_push_checks.sh +++ b/scripts/pre_push_checks.sh @@ -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" diff --git a/scripts/run_full_tests_linux.sh b/scripts/run_full_tests_linux.sh index 5c83d75..2a638c6 100644 --- a/scripts/run_full_tests_linux.sh +++ b/scripts/run_full_tests_linux.sh @@ -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" diff --git a/scripts/run_full_tests_windows.ps1 b/scripts/run_full_tests_windows.ps1 index 174f809..066b717 100644 --- a/scripts/run_full_tests_windows.ps1 +++ b/scripts/run_full_tests_windows.ps1 @@ -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" diff --git a/tests/E2E_TESTING_NOTICE.md b/tests/E2E_TESTING_NOTICE.md index ac15fc5..3405a86 100644 --- a/tests/E2E_TESTING_NOTICE.md +++ b/tests/E2E_TESTING_NOTICE.md @@ -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`. diff --git a/tests/E2E_TESTING_SOP.md b/tests/E2E_TESTING_SOP.md index 53aa6c0..65a35b2 100644 --- a/tests/E2E_TESTING_SOP.md +++ b/tests/E2E_TESTING_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+ diff --git a/tests/TEST_SOP.md b/tests/TEST_SOP.md index 6ae0265..560afa0 100644 --- a/tests/TEST_SOP.md +++ b/tests/TEST_SOP.md @@ -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) diff --git a/tests/skip_policy.json b/tests/skip_policy.json index c1e0bb1..c25879e 100644 --- a/tests/skip_policy.json +++ b/tests/skip_policy.json @@ -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" ] } diff --git a/tests/test_r123_real_backend_model_list_lane.py b/tests/test_r123_real_backend_model_list_lane.py new file mode 100644 index 0000000..7b9a5c6 --- /dev/null +++ b/tests/test_r123_real_backend_model_list_lane.py @@ -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()