diff --git a/apps/api/src/plugins/mcp/gateway.py b/apps/api/src/plugins/mcp/gateway.py index 45dae66e..b1dd2a56 100644 --- a/apps/api/src/plugins/mcp/gateway.py +++ b/apps/api/src/plugins/mcp/gateway.py @@ -44,7 +44,7 @@ from typing import Any from uuid import UUID import structlog -from sqlalchemy import select, text +from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession from src.db.awooop_models import ( @@ -54,6 +54,7 @@ from src.db.awooop_models import ( AwoooPMcpToolRegistry, AwoooPProject, ) +from src.core.redis_client import get_redis from src.plugins.mcp.interfaces import MCPToolResult from src.plugins.mcp.registry import get_provider_registry @@ -359,14 +360,9 @@ class McpGateway: raise GateApprovalError("write/admin 操作需要 run_id（approval 追蹤用）") try: - import aioredis - - from src.core.config import settings - - redis = aioredis.from_url(settings.REDIS_URL) + redis = get_redis() approval_key = f"mcp_approval:{ctx.project_id}:{ctx.agent_id}:{ctx.tool_name}:{ctx.run_id}" approved = await redis.get(approval_key) - await redis.aclose() except Exception as exc: logger.warning( "mcp_gate5_redis_error", diff --git a/apps/api/src/services/awooop_approval_token.py b/apps/api/src/services/awooop_approval_token.py index 79c1c69b..5e460b69 100644 --- a/apps/api/src/services/awooop_approval_token.py +++ b/apps/api/src/services/awooop_approval_token.py @@ -46,6 +46,8 @@ from typing import Any import structlog +from src.core.redis_client import get_redis + logger = structlog.get_logger(__name__) # ───────────────────────────────────────────────────────────────────────────── @@ -219,29 +221,23 @@ async def record_approval( exp = payload["exp"] try: - import aioredis - from src.core.config import settings - - redis = aioredis.from_url(settings.REDIS_URL) + redis = get_redis() # jti NX jti_key = f"{_JTI_KEY_PREFIX}{jti}" ttl_remaining = max(exp - int(time.time()), 1) ok = await redis.set(jti_key, "1", nx=True, ex=ttl_remaining) if not ok: - await redis.aclose() raise TokenReplayError(f"jti={jti!r} 已使用") # SADD approver sig_key = f"{_SIG_SET_PREFIX}{project_id}:{run_id}:{tool_name}" added = await redis.sadd(sig_key, approver_id) if added == 0: - await redis.aclose() raise DuplicateApproverError(f"approver '{approver_id}' 已簽核") await redis.expire(sig_key, _SIG_TTL_SECONDS) count = int(await redis.scard(sig_key)) - await redis.aclose() logger.info( "awooop_approval_recorded", @@ -271,13 +267,9 @@ async def check_approval_quorum( 檢查 quorum。Raises QuorumNotMetError if 不足。 """ try: - import aioredis - from src.core.config import settings - - redis = aioredis.from_url(settings.REDIS_URL) + redis = get_redis() sig_key = f"{_SIG_SET_PREFIX}{project_id}:{run_id}:{tool_name}" count = int(await redis.scard(sig_key)) - await redis.aclose() if count < required_count: raise QuorumNotMetError(f"簽核數不足（{count}/{required_count}）") diff --git a/apps/api/tests/test_awooop_approval_token.py b/apps/api/tests/test_awooop_approval_token.py new file mode 100644 index 00000000..970090c5 --- /dev/null +++ b/apps/api/tests/test_awooop_approval_token.py @@ -0,0 +1,138 @@ +from __future__ import annotations + +import pytest + +from src.services import awooop_approval_token as approval_token + + +class FakeRedis: + def __init__(self) -> None: + self.values: dict[str, str] = {} + self.sets: dict[str, set[str]] = {} + self.expirations: dict[str, int] = {} + + async def set( + self, + key: str, + value: str, + *, + nx: bool = False, + ex: int | None = None, + ) -> bool: + if nx and key in self.values: + return False + self.values[key] = value + if ex is not None: + self.expirations[key] = ex + return True + + async def sadd(self, key: str, member: str) -> int: + members = self.sets.setdefault(key, set()) + before = len(members) + members.add(member) + return 1 if len(members) > before else 0 + + async def expire(self, key: str, ttl: int) -> bool: + self.expirations[key] = ttl + return True + + async def scard(self, key: str) -> int: + return len(self.sets.get(key, set())) + + +@pytest.fixture(autouse=True) +def stable_hmac(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(approval_token, "_get_hmac_key", lambda: b"test-hmac-key") + + +@pytest.mark.asyncio +async def test_record_approval_uses_shared_redis_pool( + monkeypatch: pytest.MonkeyPatch, +) -> None: + redis = FakeRedis() + monkeypatch.setattr(approval_token, "get_redis", lambda: redis) + + token = approval_token.issue_approval_token( + project_id="awoooi", + run_id="run-1", + tool_name="operator_console_approve", + approver_id="telegram:123456", + ) + + count = await approval_token.record_approval( + project_id="awoooi", + run_id="run-1", + tool_name="operator_console_approve", + approver_id="telegram:123456", + token=token, + ) + + assert count == 1 + assert any(key.startswith("awooop_appr:jti:") for key in redis.values) + assert redis.sets[ + "awooop_appr:sigs:awoooi:run-1:operator_console_approve" + ] == {"telegram:123456"} + + +@pytest.mark.asyncio +async def test_record_approval_rejects_token_replay( + monkeypatch: pytest.MonkeyPatch, +) -> None: + redis = FakeRedis() + monkeypatch.setattr(approval_token, "get_redis", lambda: redis) + + token = approval_token.issue_approval_token( + project_id="awoooi", + run_id="run-1", + tool_name="operator_console_approve", + approver_id="telegram:123456", + ) + + await approval_token.record_approval( + project_id="awoooi", + run_id="run-1", + tool_name="operator_console_approve", + approver_id="telegram:123456", + token=token, + ) + + with pytest.raises(approval_token.TokenReplayError): + await approval_token.record_approval( + project_id="awoooi", + run_id="run-1", + tool_name="operator_console_approve", + approver_id="telegram:123456", + token=token, + ) + + +@pytest.mark.asyncio +async def test_check_approval_quorum_uses_shared_redis_pool( + monkeypatch: pytest.MonkeyPatch, +) -> None: + redis = FakeRedis() + redis.sets["awooop_appr:sigs:awoooi:run-1:tool"] = {"operator-a"} + monkeypatch.setattr(approval_token, "get_redis", lambda: redis) + + assert await approval_token.check_approval_quorum( + project_id="awoooi", + run_id="run-1", + tool_name="tool", + required_count=1, + ) + + +@pytest.mark.asyncio +async def test_check_approval_quorum_rejects_insufficient_count( + monkeypatch: pytest.MonkeyPatch, +) -> None: + redis = FakeRedis() + monkeypatch.setattr(approval_token, "get_redis", lambda: redis) + + with pytest.raises(approval_token.QuorumNotMetError): + await approval_token.check_approval_quorum( + project_id="awoooi", + run_id="run-1", + tool_name="tool", + required_count=1, + ) diff --git a/apps/api/tests/test_mcp_gateway_gate5.py b/apps/api/tests/test_mcp_gateway_gate5.py new file mode 100644 index 00000000..74616b99 --- /dev/null +++ b/apps/api/tests/test_mcp_gateway_gate5.py @@ -0,0 +1,104 @@ +from __future__ import annotations + +import uuid + +import pytest + +from src.plugins.mcp import gateway as gateway_module +from src.plugins.mcp.gateway import ( + GateApprovalError, + GateCheckResult, + GatewayContext, + McpGateway, +) + + +class FakeRedis: + def __init__(self, values: dict[str, str] | None = None) -> None: + self.values = values or {} + self.closed = False + + async def get(self, key: str) -> str | None: + return self.values.get(key) + + async def aclose(self) -> None: + self.closed = True + + +@pytest.mark.asyncio +async def test_gate5_uses_shared_redis_pool_without_closing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + run_id = uuid.uuid4() + approval_key = f"mcp_approval:awoooi:openclaw-sre:k8s_apply:{run_id}" + redis = FakeRedis({approval_key: "approved"}) + monkeypatch.setattr(gateway_module, "get_redis", lambda: redis) + + gate_result = GateCheckResult() + await McpGateway(db=None)._gate5_approval( + GatewayContext( + project_id="awoooi", + agent_id="openclaw-sre", + tool_name="k8s_apply", + run_id=run_id, + is_shadow=False, + required_scope="write", + ), + grant_row=None, + gate_result=gate_result, + ) + + assert gate_result.gate5_approval is True + assert redis.closed is False + + +@pytest.mark.asyncio +async def test_gate5_rejects_missing_approval( + monkeypatch: pytest.MonkeyPatch, +) -> None: + redis = FakeRedis() + monkeypatch.setattr(gateway_module, "get_redis", lambda: redis) + + with pytest.raises(GateApprovalError): + await McpGateway(db=None)._gate5_approval( + GatewayContext( + project_id="awoooi", + agent_id="openclaw-sre", + tool_name="k8s_apply", + run_id=uuid.uuid4(), + is_shadow=False, + required_scope="write", + ), + grant_row=None, + gate_result=GateCheckResult(), + ) + + +@pytest.mark.asyncio +async def test_gate5_read_scope_skips_redis( + monkeypatch: pytest.MonkeyPatch, +) -> None: + called = False + + def fail_if_called() -> FakeRedis: + nonlocal called + called = True + return FakeRedis() + + monkeypatch.setattr(gateway_module, "get_redis", fail_if_called) + gate_result = GateCheckResult() + + await McpGateway(db=None)._gate5_approval( + GatewayContext( + project_id="awoooi", + agent_id="openclaw-sre", + tool_name="k8s_get", + is_shadow=False, + required_scope="read", + ), + grant_row=None, + gate_result=gate_result, + ) + + assert called is False + assert gate_result.gate5_approval is True diff --git a/docs/LOGBOOK.md b/docs/LOGBOOK.md index d07b9eb4..2c02bd32 100644 --- a/docs/LOGBOOK.md +++ b/docs/LOGBOOK.md @@ -1,3 +1,17 @@ +## 2026-05-06 | AwoooP approval and MCP Gate 5 stop importing aioredis + +**背景**：整合計畫 P0-L 指出 AwoooP approval token service 與 MCP Gate 5 還在 runtime import `aioredis`；這會讓 approval / gateway path 在 Python 3.11+ 或套件漂移時直接壞掉，也繞過既有 Redis pool 管理。 + +**本次修補**： +- `awooop_approval_token.py` 的 `record_approval()` / `check_approval_quorum()` 改用 `src.core.redis_client.get_redis()`，不再自行 `aioredis.from_url()` 或關閉共享連線。 +- `plugins/mcp/gateway.py` Gate 5 approval lookup 同步改用共享 Redis pool。 +- 補 `test_awooop_approval_token.py` 與 `test_mcp_gateway_gate5.py`，鎖住 jti replay、quorum、MCP Gate 5 approval 與 read-scope bypass。 + +**驗證**： +- `pytest tests/test_awooop_approval_token.py tests/test_mcp_gateway_gate5.py tests/test_awooop_operator_auth.py -q` → 12 passed。 +- `py_compile` touched backend files OK；ruff fatal checks OK。 +- `rg "import aioredis|aioredis.from_url" approval token + MCP gateway` 無命中。 + ## 2026-05-06 | AwoooP approval decide no longer trusts browser identity **背景**：AwoooP Operator Console 的 `/api/v1/platform/approvals/{run_id}/decide` 仍接受前端 body 內的 `approver_id`，前端甚至硬編 `approver_id: "operator"`；這會讓 audit identity 無法作為真實審批證據。 diff --git a/docs/awooop/AWOOOI-AWOOOP-AI-AUTONOMOUS-FLYWHEEL-INTEGRATION-PLAN.md b/docs/awooop/AWOOOI-AWOOOP-AI-AUTONOMOUS-FLYWHEEL-INTEGRATION-PLAN.md index c7259058..4a49b4ee 100644 --- a/docs/awooop/AWOOOI-AWOOOP-AI-AUTONOMOUS-FLYWHEEL-INTEGRATION-PLAN.md +++ b/docs/awooop/AWOOOI-AWOOOP-AI-AUTONOMOUS-FLYWHEEL-INTEGRATION-PLAN.md @@ -189,6 +189,9 @@ Goal: prevent the automation flywheel from bypassing governance. Progress: +- 2026-05-06: P0-L first enforcement patch landed. AwoooP approval token + storage and MCP Gateway Gate 5 now use the shared Redis pool instead of + runtime `aioredis.from_url()` clients. - 2026-05-06: P0-I first enforcement patch landed. The AwoooP approval decide endpoint now derives `approver_id` from trusted operator headers instead of frontend body data, and production fails closed when `AWOOOP_OPERATOR_API_KEY`