Shadowbroker/backend/tests/test_openclaw_connect_info_reveal.py

"""Issue #302 (tg12): OpenClaw connect-info HMAC secret disclosure.

Before this change, ``GET /api/ai/connect-info?reveal=true`` returned the
full HMAC secret in the response body on every modal open AND the same
GET endpoint auto-bootstrapped (generated + persisted) the secret on a
mere read. Even gated to ``require_local_operator``, that put the full
secret into:

  * browser visit history
  * dev-tools network panel
  * browser disk cache
  * HAR exports
  * screen captures / shoulder-surfing

Every single time the OpenClaw Connect modal opened.

After this change:

  GET  /api/ai/connect-info            — always returns the MASKED
                                          fingerprint. No ?reveal param.
                                          No side effects (auto-bootstrap
                                          gone).
  POST /api/ai/connect-info/bootstrap  — mints+persists the secret if
                                          missing. Idempotent. Never
                                          returns the full secret.
  POST /api/ai/connect-info/reveal     — returns the full secret with
                                          strict Cache-Control: no-store
                                          headers. POST so the body
                                          doesn't land in URL history.
  POST /api/ai/connect-info/regenerate — keeps the one-time-disclosure
                                          for the new secret (regen IS a
                                          deliberate destructive action).
                                          Same no-store headers added.

These tests pin every property.
"""

from __future__ import annotations

import asyncio
from unittest.mock import patch

import pytest
from httpx import ASGITransport, AsyncClient


# ---------------------------------------------------------------------------
# Loopback test client. ``require_local_operator`` resolves true for
# request.client.host == "127.0.0.1"; FastAPI's TestClient sets it to
# "testclient" which isn't on the allowlist. Use raw ASGITransport.
# ---------------------------------------------------------------------------


@pytest.fixture
def loopback():
    from main import app

    class _Client:
        def __init__(self, peer_ip: str = "127.0.0.1"):
            self._loop = asyncio.new_event_loop()
            self._transport = ASGITransport(app=app, client=(peer_ip, 12345))
            self._base = f"http://{peer_ip}:8000"

        def _do(self, method: str, url: str, **kw):
            async def go():
                async with AsyncClient(transport=self._transport, base_url=self._base) as ac:
                    return await ac.request(method, url, **kw)
            return self._loop.run_until_complete(go())

        def get(self, url, **kw):  return self._do("GET", url, **kw)
        def post(self, url, **kw): return self._do("POST", url, **kw)
        def close(self): self._loop.close()

    c = _Client()
    yield c
    c.close()


@pytest.fixture
def remote():
    from main import app

    class _Client:
        def __init__(self):
            self._loop = asyncio.new_event_loop()
            self._transport = ASGITransport(app=app, client=("1.2.3.4", 12345))
            self._base = "http://1.2.3.4:8000"

        def _do(self, method: str, url: str, **kw):
            async def go():
                async with AsyncClient(transport=self._transport, base_url=self._base) as ac:
                    return await ac.request(method, url, **kw)
            return self._loop.run_until_complete(go())

        def get(self, url, **kw):  return self._do("GET", url, **kw)
        def post(self, url, **kw): return self._do("POST", url, **kw)
        def close(self): self._loop.close()

    c = _Client()
    yield c
    c.close()


@pytest.fixture
def stub_env(monkeypatch):
    """Isolate connect-info tests from the dev's real backend .env.

    Pydantic ``Settings()`` reads from ``.env`` file directly on
    instantiation, so monkey-patching ``os.environ`` isn't sufficient
    — the real ``OPENCLAW_HMAC_SECRET`` would leak through. Instead we
    override ``get_settings()`` in the route module to return a fresh
    ``Settings`` instance whose env values are driven entirely by an
    in-test dict, AND we replace ``_write_env_value`` so writes update
    that same dict instead of touching the developer's filesystem.

    Yields the dict so individual tests can pre-seed values or assert
    that writes happened.
    """
    import routers.ai_intel as ai_intel
    import services.config as config

    state: dict[str, str] = {}

    class _FakeSettings:
        @property
        def OPENCLAW_HMAC_SECRET(self) -> str:
            return state.get("OPENCLAW_HMAC_SECRET", "")

        @property
        def OPENCLAW_ACCESS_TIER(self) -> str:
            return state.get("OPENCLAW_ACCESS_TIER", "restricted")

    fake = _FakeSettings()

    def _fake_get_settings():
        return fake

    # Route code calls ``get_settings.cache_clear()`` after writing the
    # env. The production version is wrapped with ``@lru_cache``, so
    # cache_clear exists. Attach a no-op shim here.
    _fake_get_settings.cache_clear = lambda: None  # type: ignore[attr-defined]

    monkeypatch.setattr(config, "get_settings", _fake_get_settings)

    def _fake_write_env_value(key: str, value: str) -> None:
        state[key] = value

    monkeypatch.setattr(ai_intel, "_write_env_value", _fake_write_env_value)

    yield state


# ---------------------------------------------------------------------------
# GET /api/ai/connect-info — always masked, no auto-bootstrap
# ---------------------------------------------------------------------------


class TestGetConnectInfoMasking:
    def test_returns_masked_when_secret_set(self, loopback, stub_env):
        secret = "abcdef" + "0" * 38 + "wxyz"
        stub_env["OPENCLAW_HMAC_SECRET"] = secret

        r = loopback.get("/api/ai/connect-info")
        assert r.status_code == 200
        body = r.json()
        # Body must NOT carry the full secret value anywhere.
        assert secret not in r.text, (
            "GET /api/ai/connect-info MUST NOT include the full HMAC "
            "secret. Response body contained the secret value."
        )
        assert body["hmac_secret_set"] is True
        assert body["masked_hmac_secret"].startswith("abcdef")
        assert body["masked_hmac_secret"].endswith("wxyz")
        assert "•" in body["masked_hmac_secret"]
        # Pre-fix field is gone.
        assert "hmac_secret" not in body

    def test_no_auto_bootstrap_when_secret_missing(self, loopback, stub_env):
        """Side-effect-on-GET was the second half of issue #302. A GET
        with no secret configured must NOT mint one — that should
        require an explicit POST /bootstrap."""
        r = loopback.get("/api/ai/connect-info")
        assert r.status_code == 200
        body = r.json()
        assert body["hmac_secret_set"] is False
        assert body["masked_hmac_secret"] == ""
        # The bootstrap_behavior block should advertise the new flow.
        assert body["bootstrap_behavior"]["auto_generates_when_missing"] is False
        # And no _write_env_value call happened.
        assert "OPENCLAW_HMAC_SECRET" not in stub_env

    def test_no_reveal_query_param(self, loopback, stub_env):
        """Pre-fix, ?reveal=true would return the full secret. Post-fix
        the param is silently ignored — the response is the same as
        without it (still masked, no leak)."""
        secret = "abcdef" + "0" * 38 + "wxyz"
        stub_env["OPENCLAW_HMAC_SECRET"] = secret

        r = loopback.get("/api/ai/connect-info?reveal=true")
        assert r.status_code == 200
        assert secret not in r.text, (
            "?reveal=true must be a no-op on GET — the full secret "
            "MUST NOT come back in the response body."
        )


# ---------------------------------------------------------------------------
# POST /api/ai/connect-info/bootstrap
# ---------------------------------------------------------------------------


class TestBootstrap:
    def test_mints_when_missing(self, loopback, stub_env):
        r = loopback.post("/api/ai/connect-info/bootstrap")
        assert r.status_code == 200
        body = r.json()
        assert body["ok"] is True
        assert body["generated"] is True
        assert body["hmac_secret_set"] is True
        # Bootstrap must NOT return the full secret in-line.
        assert "hmac_secret" not in body or not body.get("hmac_secret")
        assert "•" in body["masked_hmac_secret"]
        # _write_env_value was actually called.
        assert stub_env.get("OPENCLAW_HMAC_SECRET")
        # The full value isn't echoed back in the response text either.
        assert stub_env["OPENCLAW_HMAC_SECRET"] not in r.text

    def test_idempotent_when_already_set(self, loopback, stub_env):
        existing = "abcdef" + "0" * 38 + "wxyz"
        stub_env["OPENCLAW_HMAC_SECRET"] = existing

        r = loopback.post("/api/ai/connect-info/bootstrap")
        assert r.status_code == 200
        body = r.json()
        assert body["ok"] is True
        assert body["generated"] is False
        assert body["hmac_secret_set"] is True
        # Existing secret untouched — value is still the seeded one.
        assert stub_env["OPENCLAW_HMAC_SECRET"] == existing
        # No full secret in the response.
        assert existing not in r.text


# ---------------------------------------------------------------------------
# POST /api/ai/connect-info/reveal
# ---------------------------------------------------------------------------


class TestReveal:
    def test_returns_full_secret_when_set(self, loopback, stub_env):
        secret = "abcdef" + "0" * 38 + "wxyz"
        stub_env["OPENCLAW_HMAC_SECRET"] = secret

        r = loopback.post("/api/ai/connect-info/reveal")
        assert r.status_code == 200
        body = r.json()
        assert body["ok"] is True
        assert body["hmac_secret"] == secret

    def test_strict_cache_control_headers(self, loopback, stub_env):
        """The whole point of POST /reveal vs GET ?reveal=true is that
        the response carries headers that prevent every cache layer
        from persisting the secret."""
        secret = "abcdef" + "0" * 38 + "wxyz"
        stub_env["OPENCLAW_HMAC_SECRET"] = secret

        r = loopback.post("/api/ai/connect-info/reveal")
        cc = r.headers.get("cache-control", "")
        assert "no-store" in cc, (
            f"reveal MUST set Cache-Control: no-store — got {cc!r}"
        )
        assert "no-cache" in cc
        # Pragma + Expires as well for HTTP/1.0 caches.
        assert r.headers.get("pragma", "").lower() == "no-cache"
        assert r.headers.get("expires") == "0"

    def test_404_when_no_secret_configured(self, loopback, stub_env):
        r = loopback.post("/api/ai/connect-info/reveal")
        assert r.status_code == 404
        # Hint should point at the bootstrap endpoint, not just say "404".
        detail = r.json().get("detail", "")
        assert "/bootstrap" in detail or "bootstrap" in detail.lower()


# ---------------------------------------------------------------------------
# POST /api/ai/connect-info/regenerate — still returns the new secret
# inline (deliberate destructive action), but with no-store headers.
# ---------------------------------------------------------------------------


class TestRegenerate:
    def test_returns_new_secret_with_no_store_headers(self, loopback, stub_env):
        # Seed an existing secret so we can prove it changes.
        old = "oldold" + "0" * 38 + "1234"
        stub_env["OPENCLAW_HMAC_SECRET"] = old

        r = loopback.post("/api/ai/connect-info/regenerate")
        assert r.status_code == 200
        body = r.json()
        assert body["ok"] is True
        assert body["hmac_secret"]
        assert body["hmac_secret"] != old
        # no-store headers MUST be present so the new secret doesn't
        # land in browser disk cache after the regenerate click.
        cc = r.headers.get("cache-control", "")
        assert "no-store" in cc and "no-cache" in cc
        assert r.headers.get("pragma", "").lower() == "no-cache"


# ---------------------------------------------------------------------------
# Auth-gate regression — every endpoint still rejects anonymous remote
# callers. This is the property we already enforce for the rest of the
# operator-only surface; adding the three new endpoints to the audit
# coverage prevents a future refactor from dropping the dependency.
# ---------------------------------------------------------------------------


class TestAnonymousRejection:
    @pytest.mark.parametrize(
        "method,path,body",
        [
            ("get",  "/api/ai/connect-info",            None),
            ("post", "/api/ai/connect-info/bootstrap",  None),
            ("post", "/api/ai/connect-info/reveal",     None),
            ("post", "/api/ai/connect-info/regenerate", None),
        ],
    )
    def test_remote_rejected(self, remote, method, path, body):
        fn = getattr(remote, method)
        r = fn(path, json=body) if body is not None else fn(path)
        assert r.status_code == 403, (
            f"{method.upper()} {path} must reject anonymous remote callers; "
            f"got {r.status_code}"
        )