fix(companion): honor configured pairing address (#6060)

* fix(companion): honor configured pairing origin

* fix(companion): keep configured pairing on v1 LAN contract

* fix(companion): reject numeric pairing hosts

---------

Co-authored-by: RaresKeY <158580472+RaresKeY@users.noreply.github.com>
This commit is contained in:
Alexandre Teixeira
2026-08-15 21:11:10 +02:00
committed by GitHub
co-authored by RaresKeY
parent 18991d6f67
commit 9c71948376
8 changed files with 281 additions and 4 deletions
+8
View File
@@ -76,6 +76,14 @@ SEARXNG_INSTANCE=http://localhost:8080
# Change this if another local service already uses 7000 (macOS AirPlay often does). # Change this if another local service already uses 7000 (macOS AirPlay often does).
# APP_PORT=7000 # APP_PORT=7000
# Optional HTTP address advertised in companion/mobile pairing codes. Set this
# when Docker would otherwise advertise a container address or loopback. Use a
# LAN or Tailscale IPv4 address, a single-label hostname, or an mDNS *.local
# name that the phone can reach. HTTPS and public hostnames are not supported
# by the current companion client. Do not include credentials, a path, query,
# or fragment.
# COMPANION_BASE_URL=http://192.168.1.50:7000
# Development-only auth bypass for loopback requests. # Development-only auth bypass for loopback requests.
# Keep false for Docker, LAN, reverse proxy, and any shared deployment. # Keep false for Docker, LAN, reverse proxy, and any shared deployment.
# LOCALHOST_BYPASS=false # LOCALHOST_BYPASS=false
+99
View File
@@ -6,11 +6,14 @@ units so the route layer stays thin and the logic is directly testable.
from __future__ import annotations from __future__ import annotations
import ipaddress
import json import json
import os import os
import re
import secrets import secrets
import socket import socket
import uuid import uuid
from urllib.parse import urlsplit
import bcrypt import bcrypt
@@ -20,6 +23,102 @@ PAIRING_VERSION = 1
COMPANION_SCOPE = "chat" COMPANION_SCOPE = "chat"
_COMPANION_IPV4_NETWORKS = tuple(
ipaddress.ip_network(cidr)
for cidr in (
"10.0.0.0/8",
"100.64.0.0/10",
"127.0.0.0/8",
"169.254.0.0/16",
"172.16.0.0/12",
"192.168.0.0/16",
)
)
_DNS_LABEL_RE = re.compile(r"[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?\Z")
def _valid_companion_client_host(host: str) -> bool:
"""Match the host forms supported by the current v1 Expo client."""
if not host or len(host) > 253 or not host.isascii() or "%" in host:
return False
try:
address = ipaddress.ip_address(host)
except ValueError:
labels = host.split(".")
if any(not _DNS_LABEL_RE.fullmatch(label) for label in labels):
return False
if any(label.startswith("xn--") for label in labels):
return False
# WHATWG URL parsers treat a decimal or ``0x`` single-label hostname
# as an IPv4 number even though Python's strict ``ipaddress`` parser
# rejects that spelling. The v1 client interpolates this host back
# into a URL, so accepting e.g. ``134744072`` would make the phone send
# its bearer token to public 8.8.8.8. Keep DNS labels unambiguous.
if len(labels) == 1 and (
labels[0].isdigit()
or re.fullmatch(r"0x[0-9a-f]*", labels[0]) is not None
):
return False
return len(labels) == 1 or (len(labels) >= 2 and labels[-1] == "local")
return isinstance(address, ipaddress.IPv4Address) and any(
address in network for network in _COMPANION_IPV4_NETWORKS
)
def parse_companion_base_url(value: str) -> tuple[str, int]:
"""Validate a v1 companion address and return its legacy (host, port).
The deployed client understands only HTTP plus a LAN-style host and port.
Reject anything outside that exact contract instead of advertising a URL
the client would reject, downgrade, or interpret differently.
"""
if not isinstance(value, str) or not value:
raise ValueError("COMPANION_BASE_URL must be a canonical HTTP LAN origin")
if not value.isascii():
raise ValueError("COMPANION_BASE_URL must contain only ASCII characters")
if any(
ord(char) <= 32 or ord(char) == 127 or char in {"\\", "%"}
for char in value
):
raise ValueError(
"COMPANION_BASE_URL contains a forbidden character"
)
try:
parsed = urlsplit(value)
port = parsed.port
except ValueError as exc:
raise ValueError("COMPANION_BASE_URL must be a valid HTTP LAN origin") from exc
host = parsed.hostname
if parsed.scheme.lower() != "http" or not parsed.netloc or not host:
raise ValueError("COMPANION_BASE_URL must be a canonical HTTP LAN origin")
if parsed.username is not None or parsed.password is not None:
raise ValueError("COMPANION_BASE_URL must not contain credentials")
if parsed.path or parsed.query or parsed.fragment:
raise ValueError("COMPANION_BASE_URL must not contain a path, query, or fragment")
if port is not None and not 1 <= port <= 65535:
raise ValueError("COMPANION_BASE_URL port must be between 1 and 65535")
if not _valid_companion_client_host(host):
raise ValueError("COMPANION_BASE_URL host is not supported by companion v1")
netloc = f"{host}:{port}" if port is not None else host
origin = f"http://{netloc}"
if value != origin:
raise ValueError("COMPANION_BASE_URL must be a canonical HTTP LAN origin")
return host, port or 80
def configured_companion_origin() -> tuple[str, int] | None:
"""Return the validated operator-configured v1 address, if any."""
value = os.environ.get("COMPANION_BASE_URL")
if value is None or value == "":
return None
return parse_companion_base_url(value)
def default_port() -> int: def default_port() -> int:
"""Best guess at the port the server is reachable on. Callers that know the """Best guess at the port the server is reachable on. Callers that know the
real request port should pass it explicitly.""" real request port should pass it explicitly."""
+13 -4
View File
@@ -200,19 +200,27 @@ def setup_companion_routes() -> APIRouter:
the code works immediately, no restart. `?format=json` returns the the code works immediately, no restart. `?format=json` returns the
payload for an in-app pairing screen.""" payload for an in-app pairing screen."""
require_admin(request) require_admin(request)
try:
configured_origin = _pairing.configured_companion_origin()
except ValueError as exc:
raise HTTPException(500, str(exc)) from None
owner = get_current_user(request) owner = get_current_user(request)
invalidate = getattr(request.app.state, "invalidate_token_cache", None) invalidate = getattr(request.app.state, "invalidate_token_cache", None)
token_id, raw_token = mint_pairing_token(owner, invalidate) token_id, raw_token = mint_pairing_token(owner, invalidate)
hosts = _pairing.lan_ip_candidates() if configured_origin:
host = hosts[0] if hosts else "127.0.0.1" host, port = configured_origin
port = request.url.port or _pairing.default_port() hosts = [host]
else:
hosts = _pairing.lan_ip_candidates()
host = hosts[0] if hosts else "127.0.0.1"
port = request.url.port or _pairing.default_port()
payload = _pairing.pairing_payload(host, port, raw_token) payload = _pairing.pairing_payload(host, port, raw_token)
qr = _pairing.pairing_qr_png_data_uri(payload) qr = _pairing.pairing_qr_png_data_uri(payload)
qr_ok = bool(qr and qr.startswith("data:image/png;base64,")) qr_ok = bool(qr and qr.startswith("data:image/png;base64,"))
if (request.query_params.get("format") or "").lower() == "json": if (request.query_params.get("format") or "").lower() == "json":
return { response = {
"host": host, "host": host,
"port": port, "port": port,
"token": raw_token, "token": raw_token,
@@ -221,6 +229,7 @@ def setup_companion_routes() -> APIRouter:
"payload": payload, "payload": payload,
"qr": qr if qr_ok else None, "qr": qr if qr_ok else None,
} }
return response
import json as _json import json as _json
payload_json = _json.dumps(payload, separators=(",", ":")) payload_json = _json.dumps(payload, separators=(",", ":"))
+1
View File
@@ -46,6 +46,7 @@ services:
- DATABASE_URL=${DATABASE_URL:-sqlite:///./data/app.db} - DATABASE_URL=${DATABASE_URL:-sqlite:///./data/app.db}
- AUTH_ENABLED=${AUTH_ENABLED:-true} - AUTH_ENABLED=${AUTH_ENABLED:-true}
- LOCALHOST_BYPASS=${LOCALHOST_BYPASS:-false} - LOCALHOST_BYPASS=${LOCALHOST_BYPASS:-false}
- COMPANION_BASE_URL=${COMPANION_BASE_URL:-}
- ODYSSEUS_ADMIN_USER=${ODYSSEUS_ADMIN_USER:-admin} - ODYSSEUS_ADMIN_USER=${ODYSSEUS_ADMIN_USER:-admin}
- ODYSSEUS_ADMIN_PASSWORD=${ODYSSEUS_ADMIN_PASSWORD:-} - ODYSSEUS_ADMIN_PASSWORD=${ODYSSEUS_ADMIN_PASSWORD:-}
- ALLOWED_ORIGINS=${ALLOWED_ORIGINS:-http://localhost,http://127.0.0.1} - ALLOWED_ORIGINS=${ALLOWED_ORIGINS:-http://localhost,http://127.0.0.1}
+1
View File
@@ -45,6 +45,7 @@ services:
- DATABASE_URL=${DATABASE_URL:-sqlite:///./data/app.db} - DATABASE_URL=${DATABASE_URL:-sqlite:///./data/app.db}
- AUTH_ENABLED=${AUTH_ENABLED:-true} - AUTH_ENABLED=${AUTH_ENABLED:-true}
- LOCALHOST_BYPASS=${LOCALHOST_BYPASS:-false} - LOCALHOST_BYPASS=${LOCALHOST_BYPASS:-false}
- COMPANION_BASE_URL=${COMPANION_BASE_URL:-}
- ODYSSEUS_ADMIN_USER=${ODYSSEUS_ADMIN_USER:-admin} - ODYSSEUS_ADMIN_USER=${ODYSSEUS_ADMIN_USER:-admin}
- ODYSSEUS_ADMIN_PASSWORD=${ODYSSEUS_ADMIN_PASSWORD:-} - ODYSSEUS_ADMIN_PASSWORD=${ODYSSEUS_ADMIN_PASSWORD:-}
- ALLOWED_ORIGINS=${ALLOWED_ORIGINS:-http://localhost,http://127.0.0.1} - ALLOWED_ORIGINS=${ALLOWED_ORIGINS:-http://localhost,http://127.0.0.1}
+1
View File
@@ -34,6 +34,7 @@ services:
- DATABASE_URL=${DATABASE_URL:-sqlite:///./data/app.db} - DATABASE_URL=${DATABASE_URL:-sqlite:///./data/app.db}
- AUTH_ENABLED=${AUTH_ENABLED:-true} - AUTH_ENABLED=${AUTH_ENABLED:-true}
- LOCALHOST_BYPASS=${LOCALHOST_BYPASS:-false} - LOCALHOST_BYPASS=${LOCALHOST_BYPASS:-false}
- COMPANION_BASE_URL=${COMPANION_BASE_URL:-}
- ODYSSEUS_ADMIN_USER=${ODYSSEUS_ADMIN_USER:-admin} - ODYSSEUS_ADMIN_USER=${ODYSSEUS_ADMIN_USER:-admin}
- ODYSSEUS_ADMIN_PASSWORD=${ODYSSEUS_ADMIN_PASSWORD:-} - ODYSSEUS_ADMIN_PASSWORD=${ODYSSEUS_ADMIN_PASSWORD:-}
- ALLOWED_ORIGINS=${ALLOWED_ORIGINS:-http://localhost,http://127.0.0.1} - ALLOWED_ORIGINS=${ALLOWED_ORIGINS:-http://localhost,http://127.0.0.1}
+153
View File
@@ -55,6 +55,7 @@ _db.ApiToken = _ApiToken
@pytest.fixture(autouse=True) @pytest.fixture(autouse=True)
def _companion_pairing_stubs(monkeypatch): def _companion_pairing_stubs(monkeypatch):
monkeypatch.delenv("COMPANION_BASE_URL", raising=False)
monkeypatch.setitem(sys.modules, "core.database", _db) monkeypatch.setitem(sys.modules, "core.database", _db)
for _name, _attrs in { for _name, _attrs in {
"core.auth": {"AuthManager": MagicMock()}, "core.auth": {"AuthManager": MagicMock()},
@@ -116,6 +117,96 @@ def test_pairing_payload_shape():
assert p == {"v": 1, "host": "192.168.1.9", "port": 7000, "token": "ody_x"} assert p == {"v": 1, "host": "192.168.1.9", "port": 7000, "token": "ody_x"}
@pytest.mark.parametrize(
("value", "expected"),
[
("http://odysseus", ("odysseus", 80)),
("http://odysseus:7000", ("odysseus", 7000)),
("http://localhost:7000", ("localhost", 7000)),
("http://odysseus.local", ("odysseus.local", 80)),
("http://api.odysseus.local:7000", ("api.odysseus.local", 7000)),
("http://10.0.0.1:7000", ("10.0.0.1", 7000)),
("http://100.64.0.1:7000", ("100.64.0.1", 7000)),
("http://100.127.255.254:7000", ("100.127.255.254", 7000)),
("http://127.0.0.1:7000", ("127.0.0.1", 7000)),
("http://169.254.1.1:7000", ("169.254.1.1", 7000)),
("http://172.16.0.1:7000", ("172.16.0.1", 7000)),
("http://172.31.255.254:7000", ("172.31.255.254", 7000)),
("http://192.168.1.9:7000", ("192.168.1.9", 7000)),
],
)
def test_parse_companion_base_url_accepts_v1_client_addresses(value, expected):
assert P.parse_companion_base_url(value) == expected
@pytest.mark.parametrize(
"value",
[
"",
"odysseus.example",
"ftp://odysseus.example",
"https://odysseus.local",
"http://user:password@odysseus.local",
"http://odysseus.local/",
"http://odysseus.local/path",
"http://odysseus.local?query=1",
"http://odysseus.local#fragment",
"http://odysseus.local:not-a-port",
"http://odysseus.local:0",
"http://odysseus.local:65536",
"http://odysseus.local:07000",
"HTTP://odysseus.local:7000",
"http://Odysseus.local:7000",
" http://odysseus.local",
"http://odysseus.local ",
"http://odysseus\\local",
"http://odysseus.local\n",
"http://odysseus.local\t",
"http://odysseus.local\x7f",
"http://example.com:7000",
"http://1.1.1.1:7000",
"http://100.63.255.255:7000",
"http://100.128.0.1:7000",
"http://126.255.255.255:7000",
"http://128.0.0.1:7000",
"http://169.253.255.255:7000",
"http://169.255.0.1:7000",
"http://172.15.255.255:7000",
"http://172.32.0.1:7000",
"http://192.167.255.255:7000",
"http://192.169.0.1:7000",
# WHATWG URL parsing normalizes these legacy numeric host spellings to
# IPv4 addresses even though Python's strict ipaddress parser rejects
# them. 134744072 / 0x08080808 both become public 8.8.8.8.
"http://134744072:7000",
"http://0x:7000",
"http://0x08080808:7000",
"http://017700000001:7000",
"http://[fd00::1]:7000",
"http://[fe80::1%25eth0]:7000",
"http://b\N{LATIN SMALL LETTER U WITH DIAERESIS}cher.local:7000",
"http://xn--bcher-kva.local:7000",
"http://xn--bcher-kva:7000",
"http://odysseus%2elocal:7000",
"http://%31%39%32.168.1.9:7000",
"http://odysseus%40local:7000",
"http://.local:7000",
"http://odysseus..local:7000",
"http://odysseus.local.:7000",
"http://-odysseus:7000",
"http://odysseus-:7000",
"http://odysseus_name:7000",
f"http://{'a' * 64}:7000",
f"http://{'a' * 250}.local:7000",
],
)
def test_parse_companion_base_url_rejects_unsupported_or_noncanonical_addresses(
value,
):
with pytest.raises(ValueError):
P.parse_companion_base_url(value)
@pytest.mark.parametrize("payload", ["[]", '{"users": []}']) @pytest.mark.parametrize("payload", ["[]", '{"users": []}'])
def test_find_admin_user_ignores_invalid_auth_shape(tmp_path, monkeypatch, payload): def test_find_admin_user_ignores_invalid_auth_shape(tmp_path, monkeypatch, payload):
auth_file = tmp_path / "auth.json" auth_file = tmp_path / "auth.json"
@@ -244,6 +335,15 @@ def test_pair_post_json_returns_pairing_payload(monkeypatch):
assert response["port"] == 7000 assert response["port"] == 7000
assert response["token"] == "ody_raw" assert response["token"] == "ody_raw"
assert response["token_id"] == "tok123" assert response["token_id"] == "tok123"
assert set(response) == {
"host",
"port",
"token",
"token_id",
"hosts",
"payload",
"qr",
}
assert response["payload"] == { assert response["payload"] == {
"v": 1, "v": 1,
"host": "192.168.1.50", "host": "192.168.1.50",
@@ -255,6 +355,59 @@ def test_pair_post_json_returns_pairing_payload(monkeypatch):
assert secret_key not in response["payload"] assert secret_key not in response["payload"]
def test_pair_post_json_prefers_configured_origin(monkeypatch):
monkeypatch.setenv("COMPANION_BASE_URL", "http://odysseus.local:7000")
mint = MagicMock(return_value=("tok123", "ody_raw"))
discovery = MagicMock(side_effect=AssertionError("configured origin must skip LAN discovery"))
monkeypatch.setattr(R, "require_admin", lambda request: None, raising=False)
monkeypatch.setattr(R, "get_current_user", lambda request: "alice")
monkeypatch.setattr(R, "mint_pairing_token", mint)
monkeypatch.setattr(R._pairing, "lan_ip_candidates", discovery)
monkeypatch.setattr(R._pairing, "pairing_qr_png_data_uri", lambda payload: None)
request = _fake_pair_request(format="json", port=7000)
response = _pair_route("POST")(request)
assert response["host"] == "odysseus.local"
assert response["port"] == 7000
assert response["hosts"] == ["odysseus.local"]
assert set(response) == {
"host",
"port",
"token",
"token_id",
"hosts",
"payload",
"qr",
}
assert response["payload"] == {
"v": 1,
"host": "odysseus.local",
"port": 7000,
"token": "ody_raw",
}
discovery.assert_not_called()
def test_pair_post_rejects_invalid_config_before_mint_without_echoing_it(monkeypatch):
configured_secret = "secret-password"
monkeypatch.setenv(
"COMPANION_BASE_URL",
f"http://admin:{configured_secret}@odysseus.local",
)
mint = MagicMock(side_effect=AssertionError("invalid config must not mint a token"))
monkeypatch.setattr(R, "require_admin", lambda request: None, raising=False)
monkeypatch.setattr(R, "mint_pairing_token", mint)
with pytest.raises(HTTPException) as exc:
_pair_route("POST")(_fake_pair_request(format="json"))
assert exc.value.status_code == 500
assert "COMPANION_BASE_URL" in exc.value.detail
assert configured_secret not in exc.value.detail
mint.assert_not_called()
def test_pair_post_json_qr_failure_returns_null_qr(monkeypatch): def test_pair_post_json_qr_failure_returns_null_qr(monkeypatch):
monkeypatch.setattr(R, "require_admin", lambda request: None, raising=False) monkeypatch.setattr(R, "require_admin", lambda request: None, raising=False)
monkeypatch.setattr(R, "get_current_user", lambda request: "alice") monkeypatch.setattr(R, "get_current_user", lambda request: "alice")
+5
View File
@@ -55,6 +55,11 @@ def test_compose_files_forward_every_upload_limit_env_var():
assert expected <= _compose_env_names(path), path.name assert expected <= _compose_env_names(path), path.name
def test_compose_files_forward_companion_base_url():
for path in COMPOSE_FILES:
assert "COMPANION_BASE_URL" in _compose_env_names(path), path.name
def test_default_compose_files_do_not_mount_host_docker_socket(): def test_default_compose_files_do_not_mount_host_docker_socket():
for path in COMPOSE_FILES: for path in COMPOSE_FILES:
text = path.read_text(encoding="utf-8") text = path.read_text(encoding="utf-8")