fix(agent): address review findings on register flow
- Restore agent_id in success output. Pre-PR the user saw the hub's canonical identifier; the merge had reduced this to just `(name)`, which made it impossible to correlate the registered agent with anything on the hub side without inspecting the config file. - Add a 15s timeout to the register POST. urllib's default is none, so a stuck hub would block the CLI indefinitely. - Read User-Agent version from package metadata instead of hardcoding "0.1", so it tracks releases. Also corrected the URL to the canonical Source URL listed in pyproject.toml (was pointing at a likely-404 github.com path). - Add two tests guarding the User-Agent. The whole point of the Cloudflare fix was to set a non-default UA; previously no test asserted this, so a refactor could silently reintroduce the 403 code 1010 bug. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
ba1804a5f9
commit
543517f0ca
|
|
@ -31,6 +31,28 @@ from .namegen import generate_agent_name
|
||||||
_LEGACY_ALIASES = {"--hub", "--key", "--name", "--device", "--insecure", "--log-level", "--config"}
|
_LEGACY_ALIASES = {"--hub", "--key", "--name", "--device", "--insecure", "--log-level", "--config"}
|
||||||
|
|
||||||
|
|
||||||
|
def _user_agent() -> str:
|
||||||
|
"""Build the User-Agent header for hub requests.
|
||||||
|
|
||||||
|
Set explicitly so we don't fall back to Python's default `Python-urllib/<ver>`,
|
||||||
|
which is blocked by Cloudflare's Browser Integrity Check on `riahub.ai`
|
||||||
|
(HTTP 403 edge code 1010). Version is read from package metadata so it
|
||||||
|
tracks releases instead of going stale.
|
||||||
|
"""
|
||||||
|
from importlib.metadata import PackageNotFoundError, version
|
||||||
|
|
||||||
|
try:
|
||||||
|
pkg_version = version("ria-toolkit-oss")
|
||||||
|
except PackageNotFoundError:
|
||||||
|
pkg_version = "unknown"
|
||||||
|
return f"ria-agent/{pkg_version} (+https://riahub.ai/qoherent/ria-toolkit-oss)"
|
||||||
|
|
||||||
|
|
||||||
|
# How long to wait on the hub before giving up. The register endpoint is a
|
||||||
|
# small DB lookup + insert; anything past this is a stuck hub, not a slow one.
|
||||||
|
_REGISTER_TIMEOUT_S = 15
|
||||||
|
|
||||||
|
|
||||||
REGISTRATION_REASON_MESSAGES = {
|
REGISTRATION_REASON_MESSAGES = {
|
||||||
"invalid_key": (
|
"invalid_key": (
|
||||||
"Registration key not recognized. Generate a fresh key from "
|
"Registration key not recognized. Generate a fresh key from "
|
||||||
|
|
@ -100,20 +122,17 @@ def _cmd_register(args: argparse.Namespace) -> int:
|
||||||
url = f"{hub_url}/screens/agents/register"
|
url = f"{hub_url}/screens/agents/register"
|
||||||
name = args.name or generate_agent_name()
|
name = args.name or generate_agent_name()
|
||||||
body = json.dumps({"name": name}).encode()
|
body = json.dumps({"name": name}).encode()
|
||||||
# Explicit User-Agent: Python's default `Python-urllib/<ver>` is blocked
|
|
||||||
# by Cloudflare's Browser Integrity Check on `riahub.ai` (HTTP 403 code
|
|
||||||
# 1010), so register() never reached the hub. Any non-default UA passes.
|
|
||||||
req = urllib.request.Request(
|
req = urllib.request.Request(
|
||||||
url,
|
url,
|
||||||
data=body,
|
data=body,
|
||||||
headers={
|
headers={
|
||||||
"Content-Type": "application/json",
|
"Content-Type": "application/json",
|
||||||
"X-API-Key": args.api_key,
|
"X-API-Key": args.api_key,
|
||||||
"User-Agent": "ria-agent/0.1 (+https://github.com/RIA-Toolkit/ria-toolkit-oss)",
|
"User-Agent": _user_agent(),
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
try:
|
try:
|
||||||
with urllib.request.urlopen(req) as resp:
|
with urllib.request.urlopen(req, timeout=_REGISTER_TIMEOUT_S) as resp:
|
||||||
data = json.loads(resp.read())
|
data = json.loads(resp.read())
|
||||||
except urllib.error.HTTPError as e:
|
except urllib.error.HTTPError as e:
|
||||||
try:
|
try:
|
||||||
|
|
@ -147,7 +166,7 @@ def _cmd_register(args: argparse.Namespace) -> int:
|
||||||
cfg.tx_allowed_freq_ranges = [[float(lo), float(hi)] for lo, hi in freq_ranges]
|
cfg.tx_allowed_freq_ranges = [[float(lo), float(hi)] for lo, hi in freq_ranges]
|
||||||
path = _config.save(cfg)
|
path = _config.save(cfg)
|
||||||
|
|
||||||
print(f"Registered agent: ({name})")
|
print(f"Registered agent: {agent_id} ({name})")
|
||||||
if cfg.tx_enabled:
|
if cfg.tx_enabled:
|
||||||
caps: list[str] = []
|
caps: list[str] = []
|
||||||
if cfg.tx_max_gain_db is not None:
|
if cfg.tx_max_gain_db is not None:
|
||||||
|
|
|
||||||
|
|
@ -71,6 +71,53 @@ def _http_error(status: int, body: bytes) -> urllib.error.HTTPError:
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_user_agent_is_set_and_not_python_default():
|
||||||
|
"""Cloudflare on `riahub.ai` returns 403 code 1010 to `Python-urllib/*`.
|
||||||
|
|
||||||
|
Guarding the UA explicitly is the entire point of the register-flow fix;
|
||||||
|
if this test ever breaks, the production bug is back.
|
||||||
|
"""
|
||||||
|
ua = agent_cli._user_agent()
|
||||||
|
assert ua, "User-Agent must not be empty"
|
||||||
|
assert not ua.lower().startswith("python-urllib"), (
|
||||||
|
f"User-Agent must not be Python's default (got {ua!r}) — Cloudflare blocks it"
|
||||||
|
)
|
||||||
|
assert ua.startswith("ria-agent/")
|
||||||
|
|
||||||
|
|
||||||
|
def test_register_request_carries_explicit_user_agent(tmp_path):
|
||||||
|
"""Capture the outbound urllib Request and verify the UA header is set."""
|
||||||
|
cfg_path = tmp_path / "agent.json"
|
||||||
|
captured: dict = {}
|
||||||
|
|
||||||
|
def _fake_urlopen(req, *args, **kwargs):
|
||||||
|
# urllib normalizes header names; get_header takes the title-cased form.
|
||||||
|
captured["ua"] = req.get_header("User-agent")
|
||||||
|
captured["api_key"] = req.get_header("X-api-key")
|
||||||
|
captured["timeout"] = kwargs.get("timeout")
|
||||||
|
raise urllib.error.HTTPError(
|
||||||
|
url=req.full_url, code=403, msg="", hdrs=None, # type: ignore[arg-type]
|
||||||
|
fp=BytesIO(_structured("invalid_key")),
|
||||||
|
)
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch.dict("os.environ", {"RIA_AGENT_CONFIG": str(cfg_path)}, clear=False),
|
||||||
|
patch("urllib.request.urlopen", side_effect=_fake_urlopen),
|
||||||
|
patch.object(
|
||||||
|
sys,
|
||||||
|
"argv",
|
||||||
|
["ria-agent", "register", "--hub", "http://hub:3005", "--api-key", "ria_reg_x"],
|
||||||
|
),
|
||||||
|
):
|
||||||
|
with pytest.raises(SystemExit):
|
||||||
|
agent_cli.main()
|
||||||
|
|
||||||
|
assert captured["ua"], "User-Agent header was not sent"
|
||||||
|
assert not captured["ua"].lower().startswith("python-urllib")
|
||||||
|
assert captured["api_key"] == "ria_reg_x"
|
||||||
|
assert captured["timeout"] is not None, "register must pass a timeout to urlopen"
|
||||||
|
|
||||||
|
|
||||||
def test_register_surfaces_reason_on_http_error(tmp_path, capsys):
|
def test_register_surfaces_reason_on_http_error(tmp_path, capsys):
|
||||||
cfg_path = tmp_path / "agent.json"
|
cfg_path = tmp_path / "agent.json"
|
||||||
err = _http_error(403, _structured("revoked"))
|
err = _http_error(403, _structured("revoked"))
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user