diff --git a/src/agentkit/core/orchestrator.py b/src/agentkit/core/orchestrator.py index bff418b..2abe8d0 100644 --- a/src/agentkit/core/orchestrator.py +++ b/src/agentkit/core/orchestrator.py @@ -293,6 +293,7 @@ class Orchestrator: for i, defn in enumerate(subtask_defs): depends_on = [ f"task-{i}" for i in defn.get("depends_on", []) + if isinstance(i, int) and 0 <= i < len(subtask_defs) ] subtasks.append(SubTask( task_id=f"task-{i}", @@ -799,7 +800,7 @@ class Orchestrator: aggregated = await self._aggregate_results(retry_plan, merged_results, task) failed_count = sum( - 1 for r in merged_results.values() if r.get("status") != "completed" + 1 for r in merged_results.values() if r.get("status") == "failed" ) if failed_count == len(merged_results): status = TaskStatus.FAILED diff --git a/src/agentkit/core/plan_executor.py b/src/agentkit/core/plan_executor.py index adb7aaa..6d76680 100644 --- a/src/agentkit/core/plan_executor.py +++ b/src/agentkit/core/plan_executor.py @@ -494,7 +494,9 @@ class PlanExecutor: if failed > 0: return TaskStatus.PARTIALLY_COMPLETED # 部分成功 if completed + skipped == total: - # 所有步骤要么完成要么跳过 - return TaskStatus.COMPLETED + # 所有步骤要么完成要么跳过 — 至少需要一个完成才算成功 + if completed > 0: + return TaskStatus.COMPLETED + return TaskStatus.FAILED # 全部跳过 = 没有实际完成 - return TaskStatus.COMPLETED + return TaskStatus.PARTIALLY_COMPLETED diff --git a/src/agentkit/server/app.py b/src/agentkit/server/app.py index 587e8d4..03b82ed 100644 --- a/src/agentkit/server/app.py +++ b/src/agentkit/server/app.py @@ -203,6 +203,11 @@ async def lifespan(app: FastAPI): if mcp_manager is not None: await mcp_manager.stop_all() + # Close Redis client for working memory + working_redis = getattr(app.state, "working_redis_client", None) + if working_redis is not None: + await working_redis.aclose() + if server_config is not None: server_config.stop_watching() @@ -493,6 +498,7 @@ def create_app( redis_url = server_config.memory["working"].get("redis_url", "redis://localhost:6379") redis_client = aioredis.from_url(redis_url, decode_responses=True) working = WorkingMemory(redis=redis_client) + app.state.working_redis_client = redis_client if server_config.memory.get("semantic", {}).get("enabled"): sem_conf = server_config.memory["semantic"] diff --git a/src/agentkit/server/routes/workflows.py b/src/agentkit/server/routes/workflows.py index efdd046..b292233 100644 --- a/src/agentkit/server/routes/workflows.py +++ b/src/agentkit/server/routes/workflows.py @@ -414,7 +414,11 @@ def _evaluate_condition(expression: str, variables: dict[str, Any]) -> bool: if left and not _SAFE_VAR_PATTERN.match(left): raise ValueError(f"Invalid variable name in condition: {left}") - left_val = variables.get(left, left) + if left and left not in variables: + # Missing variable: treat as None/empty — condition evaluates to False + left_val = None + else: + left_val = variables.get(left, left) # Strip quotes from right side if present if right.startswith('"') and right.endswith('"'): right_val = right[1:-1] @@ -469,7 +473,7 @@ async def _broadcast_ws(message: dict[str, Any]) -> None: @router.get("/workflows") -async def list_workflows(request: Request, limit: int = 50): +async def list_workflows(request: Request, limit: int = 50, _auth: None = Depends(_verify_api_key)): """List all workflows.""" store = _get_store(request) summaries = store.list(limit=limit) @@ -477,7 +481,7 @@ async def list_workflows(request: Request, limit: int = 50): @router.post("/workflows", status_code=201) -async def create_workflow(request: Request, body: CreateWorkflowRequest): +async def create_workflow(request: Request, body: CreateWorkflowRequest, _auth: None = Depends(_verify_api_key)): """Create a new workflow.""" store = _get_store(request) _validate_workflow_stages(body.stages) @@ -495,7 +499,7 @@ async def create_workflow(request: Request, body: CreateWorkflowRequest): @router.get("/workflows/{workflow_id}") -async def get_workflow(request: Request, workflow_id: str): +async def get_workflow(request: Request, workflow_id: str, _auth: None = Depends(_verify_api_key)): """Get a workflow by ID.""" store = _get_store(request) workflow = store.get(workflow_id) @@ -506,7 +510,8 @@ async def get_workflow(request: Request, workflow_id: str): @router.put("/workflows/{workflow_id}") async def update_workflow( - request: Request, workflow_id: str, body: CreateWorkflowRequest + request: Request, workflow_id: str, body: CreateWorkflowRequest, + _auth: None = Depends(_verify_api_key), ): """Update an existing workflow.""" store = _get_store(request) @@ -527,7 +532,7 @@ async def update_workflow( @router.delete("/workflows/{workflow_id}") -async def delete_workflow(request: Request, workflow_id: str): +async def delete_workflow(request: Request, workflow_id: str, _auth: None = Depends(_verify_api_key)): """Delete a workflow.""" store = _get_store(request) deleted = store.delete(workflow_id) @@ -638,7 +643,7 @@ async def approve_execution( @router.post("/workflows/executions/{execution_id}/cancel") -async def cancel_execution(request: Request, execution_id: str): +async def cancel_execution(request: Request, execution_id: str, _auth: None = Depends(_verify_api_key)): """Cancel a running execution.""" store = _get_store(request) execution = store.get_execution(execution_id) diff --git a/src/agentkit/tools/computer_use.py b/src/agentkit/tools/computer_use.py index 1afbede..2a08878 100644 --- a/src/agentkit/tools/computer_use.py +++ b/src/agentkit/tools/computer_use.py @@ -8,10 +8,8 @@ from __future__ import annotations import base64 -import ipaddress import logging from typing import Any, Callable, Awaitable -from urllib.parse import urlparse import httpx @@ -43,26 +41,7 @@ _FALLBACK_SHELL_SUGGESTIONS: dict[str, list[str]] = { } -def _is_safe_url(url: str) -> bool: - """Check if URL is safe (not pointing to private/internal networks).""" - try: - parsed = urlparse(url) - if parsed.scheme not in ("http", "https"): - return False - hostname = parsed.hostname - if not hostname: - return False - if hostname in ("localhost", "metadata.google.internal", "metadata.internal"): - return False - try: - ip = ipaddress.ip_address(hostname) - if ip.is_private or ip.is_loopback or ip.is_reserved or ip.is_link_local: - return False - except ValueError: - pass - return True - except Exception: - return False +from agentkit.utils.security import is_safe_url as _is_safe_url class ComputerUseTool(Tool): diff --git a/src/agentkit/tools/shell.py b/src/agentkit/tools/shell.py index 435b77f..0d9a970 100644 --- a/src/agentkit/tools/shell.py +++ b/src/agentkit/tools/shell.py @@ -102,7 +102,7 @@ _DANGEROUS_PATTERNS: tuple[str, ...] = ( ) -_SHELL_OPERATORS = re.compile(r'[|;&]|&&|\|\||\$\(|`') +_SHELL_OPERATORS = re.compile(r'[|;&]|&&|\|\||\$\(|\$\{|`|\$<|>|<|\n') class ShellTool(Tool): diff --git a/src/agentkit/utils/security.py b/src/agentkit/utils/security.py index c81bd0f..c832859 100644 --- a/src/agentkit/utils/security.py +++ b/src/agentkit/utils/security.py @@ -5,7 +5,14 @@ from urllib.parse import urlparse def is_safe_url(url: str) -> bool: - """Check if URL is safe (not pointing to private/internal networks).""" + """Check if URL is safe (not pointing to private/internal networks). + + Validates against: + - Private/loopback/reserved/link-local/unspecified IPs + - IPv6-mapped IPv4 addresses (e.g. ::ffff:127.0.0.1) + - Common metadata/internal hostnames + - Non-HTTP schemes + """ try: parsed = urlparse(url) if parsed.scheme not in ("http", "https"): @@ -13,14 +20,41 @@ def is_safe_url(url: str) -> bool: hostname = parsed.hostname if not hostname: return False - if hostname in ("localhost", "metadata.google.internal", "metadata.internal"): + # Block known internal/metadata hostnames + _blocked_hostnames = { + "localhost", + "metadata.google.internal", + "metadata.internal", + "metadata.azure.com", + } + hostname_lower = hostname.lower() + if hostname_lower in _blocked_hostnames: + return False + if hostname_lower.endswith((".internal", ".local", ".localhost")): return False try: ip = ipaddress.ip_address(hostname) - if ip.is_private or ip.is_loopback or ip.is_reserved or ip.is_link_local: + if _is_unsafe_ip(ip): return False except ValueError: + # hostname is a domain, not a literal IP — DNS rebinding risk remains + # (would need DNS resolution to fully mitigate) pass return True except Exception: return False + + +def _is_unsafe_ip(ip: ipaddress.IPv4Address | ipaddress.IPv6Address) -> bool: + """Check if an IP address is unsafe (private, loopback, etc.). + + Handles IPv6-mapped IPv4 addresses by checking the embedded IPv4. + """ + if ip.is_private or ip.is_loopback or ip.is_reserved or ip.is_link_local or ip.is_unspecified: + return True + # Check IPv6-mapped IPv4 (e.g. ::ffff:127.0.0.1) + if isinstance(ip, ipaddress.IPv6Address) and ip.ipv4_mapped is not None: + mapped = ip.ipv4_mapped + if mapped.is_private or mapped.is_loopback or mapped.is_reserved or mapped.is_link_local: + return True + return False diff --git a/tests/unit/core/test_plan_executor.py b/tests/unit/core/test_plan_executor.py index 1582907..223bd70 100644 --- a/tests/unit/core/test_plan_executor.py +++ b/tests/unit/core/test_plan_executor.py @@ -662,8 +662,8 @@ class TestPlanExecutorOverallStatus: executor = PlanExecutor(agent_pool=pool, max_retries=0) result = await executor.execute(plan, make_task()) - # 默认策略 SKIP → 全部跳过 → COMPLETED - assert result.status == TaskStatus.COMPLETED + # 默认策略 SKIP → 全部跳过 → FAILED(没有实际完成的步骤) + assert result.status == TaskStatus.FAILED @pytest.mark.asyncio async def test_empty_plan(self):