mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(bash-guard): close scheme-less curl bypass
Previous regex used a greedy [^|]* and required at least one / before the host, so 'curl roboco-orchestrator:8000/api' (no scheme, no slash) slipped through. Split into two simpler checks: (a) line starts with curl/wget/http/https/httpie, AND (b) line contains a forbidden host. Probe-verified: scheme-ful, scheme-less, and protocol-relative forms all denied; external URLs still allowed; GitHub-specific deny still fires first.
This commit is contained in:
@@ -74,17 +74,21 @@ fi
|
||||
|
||||
# --- internal API calls -------------------------------------------------------
|
||||
# Agents must reach the orchestrator through their MCP manifest verbs, never
|
||||
# raw HTTP. This blocks `curl http://roboco-orchestrator:8000/...`,
|
||||
# `wget http://localhost:8000/...`, `http http://127.0.0.1/...` (HTTPie),
|
||||
# scheme-less forms like `curl roboco-orchestrator:8000/...`, and the
|
||||
# protocol-relative `//host/path` form.
|
||||
# raw HTTP. Two-step check: (a) is this a curl/wget/http/https/httpie command,
|
||||
# AND (b) does the line reference a forbidden internal host. Both must match.
|
||||
# This catches all forms uniformly:
|
||||
# - scheme-ful: `curl http://roboco-orchestrator:8000/api`
|
||||
# - scheme-less: `curl roboco-orchestrator:8000/api`
|
||||
# - protocol-relative: `curl //roboco-orchestrator:8000/api`
|
||||
# - any flag ordering: `curl -s -X POST http://localhost:8000/x -d ...`
|
||||
# KNOWN GAPS (out of scope here):
|
||||
# - Variable expansion: `URL=http://orchestrator/x; curl $URL` — the guard
|
||||
# sees `curl $URL`, not the expanded URL, so this slips through. The
|
||||
# X-Agent-Role check (task 4) is the second gate.
|
||||
# - Interpreter one-liners: `python -c "import urllib.request; ..."` — too
|
||||
# deeply hidden to regex. Mitigated by the manifest-bound MCP surface.
|
||||
if echo "$low" | grep -qE '(^|[[:space:];&|])(curl|wget|http|https|httpie)[[:space:]][^|]*((http|https):)?//?(roboco-[a-z0-9_-]+|localhost|127\.0\.0\.1|0\.0\.0\.0)[:/]'; then
|
||||
if echo "$low" | grep -qE '(^|[[:space:];&|])(curl|wget|http|https|httpie)[[:space:]]' && \
|
||||
echo "$low" | grep -qE '((http|https)://)?/?(roboco-[a-z0-9_-]+|localhost|127\.0\.0\.1|0\.0\.0\.0)[:/]'; then
|
||||
echo "Denied: internal API calls bypass the gateway. Use the MCP verbs (roboco-flow / roboco-do / roboco-git-readonly / roboco-optimal / roboco-docs) — they route through the orchestrator with the right auth and tracing." >&2
|
||||
exit 2
|
||||
fi
|
||||
|
||||
@@ -48,5 +48,34 @@ def test_blocks_internal_curl_to_127() -> None:
|
||||
assert _run("curl http://127.0.0.1:8000/api/health") == _DENIED
|
||||
|
||||
|
||||
def test_blocks_scheme_less_curl_to_orchestrator() -> None:
|
||||
assert _run("curl roboco-orchestrator:8000/api") != _ALLOWED
|
||||
|
||||
|
||||
def test_blocks_scheme_less_curl_to_localhost() -> None:
|
||||
assert _run("curl localhost:8000/api") != _ALLOWED
|
||||
|
||||
|
||||
def test_blocks_scheme_less_curl_to_127() -> None:
|
||||
assert _run("curl 127.0.0.1:8000/api") != _ALLOWED
|
||||
|
||||
|
||||
def test_allows_external_curl_to_documentation() -> None:
|
||||
assert _run("curl https://docs.python.org/3/") == _ALLOWED
|
||||
|
||||
|
||||
def test_github_url_still_uses_github_specific_deny() -> None:
|
||||
"""Existing GitHub deny rule must fire BEFORE the new gateway deny."""
|
||||
payload = json.dumps(
|
||||
{"tool_name": "Bash", "tool_input": {"command": "curl https://api.github.com/user"}}
|
||||
)
|
||||
result = subprocess.run(
|
||||
[str(GUARD)],
|
||||
input=payload,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
assert result.returncode != _ALLOWED
|
||||
# The GitHub-specific message should appear, not the gateway message.
|
||||
combined = (result.stdout + result.stderr).lower()
|
||||
assert "github" in combined or "pat" in combined
|
||||
|
||||
Reference in New Issue
Block a user