Repository navigation
feat(connections): a declared local homeserver may be plain http or a private address - #6079
Conversation
… private address A self-hosted homeserver (ag2space-backend deploy/selfhost, #3165) on this machine or the owner's own network could not be added: the preview refuses http, single-label names such as localhost, and any loopback, private or shared address, and then never verifies it. Now a core.dotspace.instances entry with "allow_local": true lets a connect code for that exact gateway host (or server) be http, name localhost, or resolve to a non-public address; its server is the declared one, so the usual identity check (the gateway's listing or whoami must name an agent of that server) verifies it and the connection launches. Only that entry turns it on: an http, localhost or private gateway without it is refused exactly as before. An http gateway defaults to port 80 and is fetched over plain TCP (_connect_plain). Verified against a running bundle on localhost: the claim-agent code previews as verified with the agent's mxid; without the entry it is invalid_gateway. Tests: a new DeclaredLocalServer class (http + loopback, LAN private address, no entry / false / "true" / another host refused before any resolution or probe, the listing still only confirms, port 80 and plain TCP); connections 341 and dotspace 52 pass. Schema and docs/dotspace.md document allow_local (and mcp_url, which the schema lacked). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ution Pins the ordering the core asked about: _gateway_port now answers 80 for http, but only the declared-local path can use it; any undeclared http gateway (including localhost and an address) is invalid_gateway before anything resolves or is probed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sonichi
left a comment
There was a problem hiding this comment.
Requesting changes at 35f6932.
The shape is right. Only an owner-written core.dotspace.instances entry with allow_local: true turns this on, and I found no code path that writes that key. Without the entry, a local gateway is refused with the same invalid_gateway, before any resolution or probe. Non-bool values ("true", 1) change nothing. The listing/whoami check still decides verified. Redirects are still not followed, and the connection is still pinned to the one resolved address. I probed http://evil@localhost/, http://localhost@evil.example/, ws://, a fullwidth localhost and a mismatched gateway_host: all refused. With the src/connections.py change reverted, 4 of the 5 new tests fail. At head, tests/connections.test.py passes (341 tests).
Two things go further than the PR's own description, and both are small:
allow_private=Trueaccepts any address, not loopback/private/shared (src/connections.py:516). The docs and schema promise "loopback, private or shared", but the code skips the address check entirely. With a declaredlocalhostentry, a resolver answer of169.254.169.254(cloud metadata),0.0.0.0or8.8.8.8is probed and verified. Please replace the skip with a positive check, e.g. a_local_address(ip)that accepts loopback, RFC1918/ULA and 100.64/10 and still refuses link-local, unspecified, multicast and reserved addresses.- The bearer travels in cleartext to a LAN address. For a declared LAN host,
http://sendsAuthorization: Bearer <secret>unencrypted: on the probe, and then from the bridge on every poll, ack, result and heartbeat, for the life of the connection. That is fine for loopback. Please allow plain http only when the vetted address is loopback, and require https for private/shared addresses (or a second, explicit opt-in that names the risk in the schema).
Smaller:
- The port is not part of the match (
host_only(gateway)). An entry withgateway_host: "localhost:8008"also admitshttp://localhost:22/…(verified, probed at 127.0.0.1:22). Either match the port when the entry gives one, or say in the docs that the entry covers every port on the host. - The doc example drops the built-in DotSpaces. A configured
instanceslist replacesDEFAULT_INSTANCES, so pasting the example alone leaves onlylocalhostand drops ag2.space, dev.ag2.space, s.space and dev.s.space. Please say to restate the built-ins, or show them in the example. - The IPv6 Host header has no brackets. A declared
[::1]sendsHost: ::1. Pre-existing, but newly reachable. - Wording: the schema says "gateway_host (or server)", but the server is consulted only when
gateway_hostis absent. The code is stricter, which is good; the text should match it. - Tests: please add negative cases for 1 to 3 (address class, http toward a private address, port mismatch), plus userinfo and a non-http scheme on the local path.
Worst case: this is opt-in and inert without the entry, so existing installs are unaffected. The real risk is an owner who follows the doc example (4), or who points allow_local at a LAN box over http (2).
(agent-assisted, Blue for Chi's seat)
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 94%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
src/connections.pyLines 493-502 493 unspecified, multicast, reserved or public: a declared local server is on this machine or
494 the owner's own network."""
495 try:
496 addr = ipaddress.ip_address(ip.split("%", 1)[0])
! 497 except ValueError:
! 498 return False
499 mapped = getattr(addr, "ipv4_mapped", None)
500 if mapped is not None:
501 addr = mapped
502 if addr.is_link_local or addr.is_unspecified or addr.is_multicast: |
qingyun-wu
left a comment
There was a problem hiding this comment.
Reviewed at e9bfa41; CI is fully green. It would be an approval, but GitHub refuses one from this PR's author account.
connections.local_server_foris the single decision point. Only an owner-writtencore.dotspace.instancesentry withallow_local: truefor that exact gateway_host or server unlocks http, a single-label host, or private/loopback/shared addresses. A token or a server answer can't turn it on.- Without the entry, the old path is unchanged.
test_probe_requires_httpsnow pins that http on an undeclared host (a public name, localhost, 127.0.0.1) isinvalid_gatewaybefore anything is resolved or probed. registeredbecomes the declared server, so whoami still has to name an agent of that server. Plain TCP is used only for http on a declared host.- It's exercised end to end against the self-hosted bundle in ag2space-backend#3171's selfhost-e2e job.
Needs an approval from another reviewer.
…loopback Review (sonichi, CHANGES_REQUESTED at 35f6932): 1. allow_private skipped the address check entirely, so a declared localhost entry probed and verified 169.254.169.254, 0.0.0.0 or 8.8.8.8. It is now a positive check, _local_address: loopback, RFC 1918 / IPv6 ULA, or 100.64/10 (and their IPv4-mapped forms); never link-local, unspecified, multicast, reserved or public. 2. Plain http sent the bearer in cleartext to a LAN address on every poll, ack and result. http is now allowed only when the vetted address is loopback; a private or shared address needs https. 3. The port is part of the match: an entry naming gateway_host "localhost:8008" covers that port only (no longer :22); one without a port covers every port on the host. 4. docs/dotspace.md: the example restates the built-in DotSpaces, since a configured instances list replaces DEFAULT_INSTANCES. 5. An IPv6 gateway's Host header is bracketed ([::1], [::1]:8008). 6. Schema and docstring: the server is consulted only when the entry names no gateway_host. 7. Tests: address class (8 accepted, 10 refused incl. metadata and mapped forms), http to LAN refused and to loopback accepted, port match, userinfo / ws / ftp / fullwidth on the local path, IPv6 Host. Four of these fail at e9bfa41 and pass here. connections 346, dotspace 52, the literal guard and agent-api connections pass; a live preview against the localhost bundle still verifies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks. All seven are fixed at 8532c0f.
Suites: |
…t matches nothing Core's review of 8532c0f: - The http check ran against the address resolved at preview, but the bridge resolves the name again on every poll, so a declared name that later resolves to a LAN address would carry the bearer in cleartext. http now requires the gateway host itself to be loopback (localhost, a .localhost name, 127/8 or ::1 literals) as well as its resolved address. - urlsplit(...).port raises on a malformed port in an owner's entry; such an entry now matches nothing instead of breaking previews for the others. Tests: a name resolving to 127.0.0.1 over http is refused; localhost, 127.0.0.1 and [::1] over http are accepted; a malformed-port entry beside a good one leaves the good one working. connections 347 OK. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sonichi
left a comment
There was a problem hiding this comment.
Requesting changes at ebef28b. One mechanical item; the substance is done.
All seven points from my earlier review are fixed, and I re-checked each one at this head:
- Address class: for a declared
localhost, 169.254.169.254,::ffff:169.254.169.254, 0.0.0.0,::, fe80::1, 224.0.0.1 and 8.8.8.8 are all refused. A mix of loopback and link-local answers is refused too. - Plain http: the gateway host itself must now name loopback, not just resolve to it. http toward 192.168.x, 10.x, 100.64/10 and fd00::/8 is refused, and so is a LAN name that resolves to loopback, which covers the bridge resolving the name again on every poll. https to those addresses is still probed.
- Port: an entry with a port covers only that port, IPv6 included. A malformed entry port now matches nothing instead of raising.
4–7. The doc example restates the built-ins, the IPv6 Host header is bracketed, the wording matches the code, and the negative tests are in.
tests/connections.test.py passes 347 at head. With src/connections.py reverted to main, the 9 new tests fail; reverted to 8532c0f, the two new tests for the loopback-host rule and the malformed port fail. Behaviour without an entry is unchanged.
Blocking: scripts/review-checks.sh --diff fails prose-cap at src/connections.py:733 (comment block is 3 lines, cap 2), and CI runs this check. Something like # Cleartext bearer only to this machine: the host must name loopback, since the bridge re-resolves it every poll. fits, with the rest in the PR body.
Nit, not blocking: docs/dotspace.md:83 and the schema's allow_local text still say "plain http only when that address is loopback". The rule is now that the host must name loopback (localhost, *.localhost, 127/8, ::1).
I'll approve once the comment is within the cap.
(agent-assisted, Blue for Chi's seat)
…ame the loopback-host rule review-checks prose-cap: the comment at the http check was 3 lines. docs/dotspace.md and the schema's allow_local text now say plain http needs the gateway host itself to name loopback (localhost, *.localhost, a 127/8 or ::1 literal), not just resolve to it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@cla-assistant check |
sonichi
left a comment
There was a problem hiding this comment.
Approving at e8e242c. The comment at src/connections.py:733 is now 2 lines, and scripts/review-checks.sh --diff passes here (prose-cap clean). docs/dotspace.md and the schema now state the rule the code enforces: plain http only when the gateway host itself names loopback. tests/connections.test.py passes at this head. Everything from my earlier rounds holds; this head changes only the comment and the two doc lines.
(agent-assisted, Blue for Chi's seat)
Phase 1a, engine side, of the self-hosted homeserver work (ag2-space/ag2space-backend#3165). A self-hosted bundle on a laptop (
http://localhost) or a LAN couldn't be added as a connection. The preview refuses http, single-label names and non-public addresses, so the server is never verified and launching fails withNoIdentity.What
A
core.dotspace.instancesentry with"allow_local": truedeclares a local homeserver:{"core": {"dotspace": {"instances": [ {"server": "localhost", "gateway_host": "localhost", "allow_local": true} ]}}}For that exact gateway host (or its server),
preview_connectionnow accepts:http://gateway, which defaults to port 80 and is fetched over plain TCP (_connect_plain);localhost;vetted_address(..., allow_private=True)).Its server is the declared one, so the usual identity check still decides: the gateway's listing or
whoamimust name an agent of that server. The connection is then verified and launches.Only the entry turns this on. An http, localhost or private gateway without it is refused exactly as before (
invalid_gateway, before any resolution or probe). An entry withallow_local: falseor"true"(a string), or for another host, changes nothing. The newconnections.local_server_foris the single place that decides.Schema (
docs/sutando-config.schema.json) anddocs/dotspace.mddocumentallow_local. The schema also gainsmcp_url, which it was missing.Verified
http://localhost: the claim-agent tokenhttp://localhost/relay|<bearer>previews asverified: true,server_source: identity,agent_mxid: @alice-laptop2.agent:localhost. Without the entry it isinvalid_gateway.DeclaredLocalServer:false,"true", or another host: all refused before resolving;tests/connections.test.py341 OK,tests/dotspace.test.py52 OK,tests/dotspace-literal-guard.test.pyOK,tests/agent-api-connections.test.pyPASS.Not in this PR:
dotspace.pystill reads only the built-in list;🤖 Generated with Claude Code