fix(awooop): use shared redis for approval gates
This commit is contained in:
@@ -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",
|
||||
|
||||
@@ -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})")
|
||||
|
||||
138
apps/api/tests/test_awooop_approval_token.py
Normal file
138
apps/api/tests/test_awooop_approval_token.py
Normal file
@@ -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,
|
||||
)
|
||||
104
apps/api/tests/test_mcp_gateway_gate5.py
Normal file
104
apps/api/tests/test_mcp_gateway_gate5.py
Normal file
@@ -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
|
||||
@@ -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 無法作為真實審批證據。
|
||||
|
||||
@@ -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`
|
||||
|
||||
Reference in New Issue
Block a user