feat: add n8n serve node and harden remote access
- Add the n8n community node package with credentials, command mapping, direct serve TCP client, and browser-cli protocol crypto helpers. - Cover Ed25519 signing, canonical JSON, PQ transport encryption, request mapping, and security behavior with unit tests. - Harden serve-http with per-address rate limiting, an 8 MB request body cap, and clear warnings when binding plain HTTP beyond loopback. - Stop one-shot --key overrides from being persisted automatically; document explicit remote trust and keep key-management behind the keys policy tier. - Make HTML-to-Markdown conversion safer by bounding tree depth and dropping unsafe link/image URL schemes. - Bump package and extension release metadata to 0.16.3.
This commit is contained in:
@@ -461,8 +461,8 @@ def test_domain_display_name_backward_compat_with_stored_443(monkeypatch, tmp_pa
|
||||
assert len(targets) == 1
|
||||
assert targets[0].display_name == "browsercli.yiprawr.dev:automatisation"
|
||||
|
||||
def test_send_command_auto_saves_and_reuses_key_for_remote(monkeypatch, tmp_path):
|
||||
"""--key agent is saved on first use; omitting --key on subsequent calls reuses it."""
|
||||
def test_send_command_explicit_key_does_not_persist_remote_key(monkeypatch, tmp_path):
|
||||
"""--key is a one-shot override; use `browser-cli remote trust` to remember it."""
|
||||
import json as _json
|
||||
|
||||
remotes_path = tmp_path / "remotes.json"
|
||||
@@ -491,16 +491,12 @@ def test_send_command_auto_saves_and_reuses_key_for_remote(monkeypatch, tmp_path
|
||||
|
||||
monkeypatch.setattr("browser_cli.client.core._send_remote", fake_send_remote)
|
||||
|
||||
# First call with explicit --key agent
|
||||
send_command("tabs.list", remote="host:8765", key=_Path("agent"))
|
||||
assert used_keys[-1] == "agent"
|
||||
assert key_for_remote("host:8765") is None
|
||||
|
||||
# Key must be persisted now
|
||||
assert key_for_remote("host:8765") == "agent"
|
||||
|
||||
# Second call without --key — should reuse saved "agent"
|
||||
send_command("tabs.list", remote="host:8765")
|
||||
assert used_keys[-1] == "agent"
|
||||
assert used_keys[-1] is None
|
||||
|
||||
# ── async command transport ──────────────────────────────────────────────────
|
||||
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
"""Security/robustness tests for the HTML→Markdown converter on hostile page content."""
|
||||
from browser_cli.markdown.html import _MAX_TREE_DEPTH, convert_html_to_markdown
|
||||
from browser_cli.markdown.render import render_markdown
|
||||
|
||||
def _identity(markdown):
|
||||
return markdown
|
||||
|
||||
# ── depth-bounded recursion (Finding 4: HIGH/DoS) ─────────────────────────────────
|
||||
|
||||
def test_deeply_nested_html_does_not_crash():
|
||||
"""Thousands of nested elements must not raise RecursionError."""
|
||||
depth = 5000
|
||||
html = "<div>" * depth + "deep content" + "</div>" * depth
|
||||
out = convert_html_to_markdown(html, _identity)
|
||||
assert "deep content" in out # text preserved despite flattening
|
||||
|
||||
def test_deeply_nested_via_render_markdown_entrypoint():
|
||||
html = "<div>" * 3000 + "x" + "</div>" * 3000
|
||||
out = render_markdown(html) # routes HTML through the converter
|
||||
assert "x" in out
|
||||
|
||||
def test_nesting_within_cap_is_preserved_structurally():
|
||||
# A modest list nesting (well under the cap) still renders as a list.
|
||||
html = "<ul><li>a<ul><li>b</li></ul></li></ul>"
|
||||
out = convert_html_to_markdown(html, _identity)
|
||||
assert "- a" in out
|
||||
assert "b" in out
|
||||
|
||||
def test_max_tree_depth_is_sane():
|
||||
# Cap must be high enough for real documents, low enough to stay under the
|
||||
# interpreter recursion limit with a few frames per level.
|
||||
assert 50 <= _MAX_TREE_DEPTH <= 400
|
||||
|
||||
# ── unsafe URL schemes (Finding 5: LOW) ───────────────────────────────────────────
|
||||
|
||||
def test_javascript_url_in_link_is_neutralised():
|
||||
# Anchors render their href only in inline context (inside a block like <p>).
|
||||
out = convert_html_to_markdown('<p><a href="javascript:alert(1)">click</a></p>', _identity)
|
||||
assert "javascript:" not in out
|
||||
assert "click" in out # link text kept, dangerous href dropped
|
||||
|
||||
def test_data_and_vbscript_urls_dropped():
|
||||
assert "vbscript:" not in convert_html_to_markdown('<p><a href="vbscript:x">y</a></p>', _identity)
|
||||
assert "data:" not in convert_html_to_markdown('<img src="data:text/html,<script>">', _identity)
|
||||
|
||||
def test_normal_urls_pass_through():
|
||||
out = convert_html_to_markdown('<p><a href="https://example.com">site</a></p>', _identity)
|
||||
assert "(https://example.com)" in out
|
||||
img = convert_html_to_markdown('<img src="https://example.com/x.png" alt="pic">', _identity)
|
||||
assert "(https://example.com/x.png)" in img
|
||||
@@ -357,6 +357,48 @@ def test_serve_http_uses_compare_digest():
|
||||
assert "compare_digest" in src
|
||||
assert "== f\"Bearer" not in src
|
||||
|
||||
def test_serve_http_rate_limiter_blocks_when_exhausted():
|
||||
"""A burst-1 limiter lets the first request through, then sends 429."""
|
||||
from browser_cli.commands.serve_http import _Handler
|
||||
from browser_cli.serve.security import RateLimiter
|
||||
|
||||
handler = _Handler.__new__(_Handler)
|
||||
handler.client_address = ("203.0.113.5", 5000)
|
||||
handler.rate_limiter = RateLimiter(rate=0.001, burst=1)
|
||||
sent = []
|
||||
handler._send = lambda status, payload: sent.append((status, payload))
|
||||
|
||||
assert handler._within_rate_limit() is True
|
||||
assert handler._within_rate_limit() is False
|
||||
assert sent and sent[-1][0] == 429
|
||||
|
||||
def test_serve_http_rate_limiter_none_never_limits():
|
||||
from browser_cli.commands.serve_http import _Handler
|
||||
|
||||
handler = _Handler.__new__(_Handler)
|
||||
handler.client_address = ("203.0.113.5", 5000)
|
||||
handler.rate_limiter = None
|
||||
assert all(handler._within_rate_limit() for _ in range(100))
|
||||
|
||||
def test_serve_http_default_rate_limit_active():
|
||||
from browser_cli.commands.serve_http import _Handler
|
||||
|
||||
with patch("browser_cli.commands.serve_http.BrowserCLI"), \
|
||||
patch("browser_cli.commands.serve_http.ThreadingHTTPServer") as server_cls:
|
||||
server_cls.return_value.serve_forever.side_effect = KeyboardInterrupt
|
||||
CliRunner().invoke(main, ["serve-http", "--no-auth"])
|
||||
# The handler class passed to the server carries an active RateLimiter by default.
|
||||
handler_cls = server_cls.call_args.args[1]
|
||||
assert handler_cls.rate_limiter is not None
|
||||
assert handler_cls.rate_limiter.rate == 100.0
|
||||
|
||||
def test_serve_http_non_loopback_warns_about_cleartext():
|
||||
with patch("browser_cli.commands.serve_http.BrowserCLI"), \
|
||||
patch("browser_cli.commands.serve_http.ThreadingHTTPServer") as server_cls:
|
||||
server_cls.return_value.serve_forever.side_effect = KeyboardInterrupt
|
||||
result = CliRunner().invoke(main, ["serve-http", "--host", "0.0.0.0", "--token", "x"])
|
||||
assert "clear text" in result.output
|
||||
|
||||
def test_command_policy_allow_all_grants_everything():
|
||||
policy = command_policy_from_options(
|
||||
allow_read_page=False, allow_control=False, allow_dangerous=False, allow_all=True
|
||||
|
||||
@@ -462,6 +462,48 @@ class TestPerKeyPolicy:
|
||||
client.close()
|
||||
t.join(timeout=2)
|
||||
|
||||
def _trust_and_query_keys(self, tmp_path, monkeypatch, server_policy):
|
||||
"""Authenticate, then send browser-cli.auth.keys; return the response dict."""
|
||||
from browser_cli.serve.security import ServeSecurity
|
||||
|
||||
path = tmp_path / "authorized_keys"
|
||||
pem, pub = generate_keypair()
|
||||
path.write_text(pub + " mykey\n")
|
||||
key_path = tmp_path / "client.key.pem"
|
||||
key_path.write_bytes(pem)
|
||||
priv = load_private_key(key_path)
|
||||
|
||||
client, server = _pair()
|
||||
t = _spawn(server, path, ServeSecurity(policy=server_policy))
|
||||
challenge = _recv_framed(client)
|
||||
nonce = bytes.fromhex(challenge["nonce"])
|
||||
msg = {"id": "x", "command": "browser-cli.auth.keys", "args": {}, "user_agent": FAKE_UA, "pubkey": pub}
|
||||
msg["sig"] = sign(priv, nonce, msg).hex()
|
||||
_send_framed(client, json.dumps(msg).encode())
|
||||
resp = _recv_framed(client)
|
||||
client.close()
|
||||
t.join(timeout=2)
|
||||
return resp
|
||||
|
||||
def test_key_management_blocked_without_keys_grant(self, tmp_path, monkeypatch):
|
||||
"""Even full control+dangerous can't list keys — the control command is gated."""
|
||||
from browser_cli.command_security import CommandPolicy
|
||||
|
||||
resp = self._trust_and_query_keys(
|
||||
tmp_path, monkeypatch,
|
||||
CommandPolicy(allow_read_page=True, allow_control=True, allow_dangerous=True),
|
||||
)
|
||||
assert resp["success"] is False
|
||||
assert "blocked" in resp["error"].lower() and "keys" in resp["error"].lower()
|
||||
|
||||
def test_key_management_allowed_with_keys_grant(self, tmp_path, monkeypatch):
|
||||
"""With allow_keys, the control command runs and returns the trusted-key list."""
|
||||
from browser_cli.command_security import CommandPolicy
|
||||
|
||||
resp = self._trust_and_query_keys(tmp_path, monkeypatch, CommandPolicy(allow_keys=True))
|
||||
assert resp["success"] is True
|
||||
assert resp["data"][0]["name"] == "mykey"
|
||||
|
||||
class TestRateLimit:
|
||||
def test_shared_rate_limiter_blocks_second_command(self, monkeypatch):
|
||||
"""A burst-1 limiter shared across connections allows the first command, denies the next."""
|
||||
|
||||
@@ -6,6 +6,7 @@ from browser_cli.auth.keys import (
|
||||
format_authorized_line,
|
||||
load_authorized_keys_with_names,
|
||||
load_authorized_keys_with_policies,
|
||||
set_authorized_key_policy,
|
||||
)
|
||||
from browser_cli.command_security import CommandPolicy, assert_command_allowed
|
||||
from browser_cli.serve.security import (
|
||||
@@ -58,6 +59,50 @@ def test_full_control_still_cannot_manage_keys():
|
||||
with pytest.raises(PermissionError):
|
||||
assert_command_allowed("browser-cli.auth.trust", policy)
|
||||
|
||||
# ── set_authorized_key_policy ────────────────────────────────────────────────────
|
||||
|
||||
def test_set_policy_updates_by_pubkey(tmp_path):
|
||||
path = tmp_path / "authorized_keys"
|
||||
pub = "a" * 64
|
||||
path.write_text(f"{pub} laptop\n")
|
||||
assert set_authorized_key_policy(path, pub, ["control"]) == (pub, "laptop")
|
||||
assert load_authorized_keys_with_policies(path) == [(pub, "laptop", ["control"])]
|
||||
|
||||
def test_set_policy_by_name_and_remove_with_none(tmp_path):
|
||||
path = tmp_path / "authorized_keys"
|
||||
pub = "b" * 64
|
||||
path.write_text(f"{pub} ci-bot allow:all\n")
|
||||
assert set_authorized_key_policy(path, "ci-bot", None) == (pub, "ci-bot") # remove token
|
||||
assert load_authorized_keys_with_policies(path) == [(pub, "ci-bot", None)]
|
||||
|
||||
def test_set_policy_safe_only_writes_empty_token(tmp_path):
|
||||
path = tmp_path / "authorized_keys"
|
||||
pub = "c" * 64
|
||||
path.write_text(f"{pub} reader\n")
|
||||
set_authorized_key_policy(path, pub, [])
|
||||
assert path.read_text().strip() == f"{pub} reader allow:"
|
||||
|
||||
def test_set_policy_not_found_returns_none(tmp_path):
|
||||
path = tmp_path / "authorized_keys"
|
||||
path.write_text(f"{'a' * 64} laptop\n")
|
||||
assert set_authorized_key_policy(path, "nonexistent", ["control"]) is None
|
||||
|
||||
def test_set_policy_ambiguous_name_raises(tmp_path):
|
||||
path = tmp_path / "authorized_keys"
|
||||
path.write_text(f"{'a' * 64} dup\n{'b' * 64} dup\n")
|
||||
with pytest.raises(ValueError, match="ambiguous"):
|
||||
set_authorized_key_policy(path, "dup", ["control"])
|
||||
|
||||
def test_set_policy_preserves_other_lines(tmp_path):
|
||||
path = tmp_path / "authorized_keys"
|
||||
a, b = "a" * 64, "b" * 64
|
||||
path.write_text(f"{a} first\n{b} second allow:read-page\n")
|
||||
set_authorized_key_policy(path, a, ["control"])
|
||||
assert load_authorized_keys_with_policies(path) == [
|
||||
(a, "first", ["control"]),
|
||||
(b, "second", ["read-page"]), # untouched
|
||||
]
|
||||
|
||||
# ── authorized_keys line parsing ─────────────────────────────────────────────────
|
||||
|
||||
def test_parse_line_pubkey_only():
|
||||
|
||||
Reference in New Issue
Block a user