Skip to content

fix: park an onion whose port its service stopped declaring, offer it for reuse; 0.4.9.12:6 → 0.4.9.12:7 - #37

Closed
MattDHill wants to merge 3 commits into
masterfrom
fix/park-superseded-binding
Closed

MattDHill wants to merge 3 commits into
masterfrom
fix/park-superseded-binding

Conversation

@MattDHill

@MattDHill MattDHill commented Sep 21, 2026 •

Copy link
Copy Markdown
Member

Summary

Closes #36. A package converted from 0.3.5 whose 0.4 rewrite renumbered a port leaves the old binding disabled with its interface stripped, still holding a bridge address nothing listens on. getBridgeAddress never reads enabled, so the reconcile pass kept the onion pointed at it, Delete Onion Addresses called it attached, and a relay kept its hidden-service warning with nothing to delete. The analysis, including why the two fixes the issue proposes don't hold as written, is in the issue thread.

  • reconcileOnionTargets parks a port whose binding is disabled and keeps no interface that an enabled binding of the host does not also carry, through a mapped host watch on the host, and resumes it when the binding returns. Disabled alone is not the signal: create_service disables every binding of a package when its container is created, but only the end of the package's own interface pass strips interfaces, so a disabled binding with none is one the package no longer declares. An interface id alone is not either: clear_service_interfaces keeps an excepted id on every binding that ever carried it, and until StartOS 0.4.0.2 (start-technologies#3639) nothing removes the copy an interface leaves on its old binding when it moves port, so an id an enabled binding also carries is that copy. A same-id move to a different host is still invisible on 0.4.0.1, since the host read cannot see the other host; 0.4.0.2 clears the copy at the package's next init. A bridge-only binding such as the SOCKS port never has an interface, but nothing attaches an onion to one — every onion arrives through an interface page.
  • Delete Onion Addresses shows each entry's internal port (nextcloud/main:8080), so a stale row is tellable from a live one on the same host.
  • Add Onion Service offers every parked address of the package, from any of its hosts, beside creating a new one. Before, the reuse variant listed only an address already serving the binding in one SSL mode, so a carried-over address was never offered and deletion was the only way out. Choosing a parked address replaces its parked ports with the new one, so the address moves to the current interface; one from another host has its key directory moved under the current host first, so the hostname survives. An address that already serves the binding keeps its other ports as before. The second commit adds the cross-host half: a fleet audit of the 0.3.5 conversions puts 15 of the 24 packages with a stale binding in the changed-host-id case (Fulcrum's Electrum onion, Bisq, IPFS, LNDg, ThunderHub, …), where a per-host list never sees the address.

README and instructions cover the parking condition, the port on each row, and the move. Nextcloud's own side, retiring the 8080 record, is Start9Labs/nextcloud-startos#157.

Verified with npm run check; not exercised on a box. A box run is straightforward with Tor itself: attach an onion to the OR interface, change the OR port, and the old entry should park, list with its port, and be offered when adding an address on the new OR interface.

… for reuse; 0.4.9.12:6 → 0.4.9.12:7

A package converted from 0.3.5 whose 0.4 rewrite renumbered a port leaves
the old binding disabled with its interface stripped, holding a bridge
address nothing listens on. getBridgeAddress never reads enabled, so the
reconcile pass kept the onion pointed at it, Delete Onion Addresses called
it attached, and a relay kept its hidden-service warning with nothing to
delete (#36).

reconcileOnionTargets parks a port whose binding is disabled with no
interface. Disabled alone flaps: create_service disables every binding of
a package at boot, but only the package's own interface pass strips
interfaces, so that state is final. Delete Onion Addresses shows each
entry's internal port. Add Onion Service offers a parked address of the
same package and host, and choosing it replaces the parked ports, so the
address moves to the current interface instead of being deleted.

Verified with npm run check; not exercised on a box.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A 0.4 rewrite that changed its host id leaves the carried-over onion under
the old id, where a per-host reuse list never sees it. The fleet audit
behind #36 puts 15 of the 24 packages carrying one in that case,
Fulcrum's Electrum onion among them.

Add Onion Service now lists every parked address of the package. Choosing
one from another host moves its key directory under the current host and
rewrites the entry there, so the address keeps its hostname.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@helix-nine helix-nine left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The :5 parking format, its down: IMPOSSIBLE, and the new reuse/move path remain coherent with the recent commits, but the new superseded-binding predicate misses a concrete port-renumber case. I reproduced the PR body's proposed Tor test end to end on StartOS 0.4.0: installed 0.4.9.12:6, enabled the relay on 9001, attached an onion, changed the OR port to 9002, then upgraded to this head. The old binding remained enabled=false with interface or, torrc kept a live HiddenServicePort 9001 10.0.3.1:9001, and Add Onion Service on 9002 offered only Create new address. Details are on the inline comment.

Also remove the Generated with Claude Code footer from the PR body.

Verification: npm ci, npm run check, npm run build, make x86, git diff --check, all three green CI architecture builds, and the VM upgrade/reproduction above.

Comment thread startos/init/reconcileOnionTargets.ts Outdated
return (
!!binding &&
!binding.enabled &&
!Object.keys(binding.interfaces).length

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking — an interface id reused on the new port remains on the disabled old binding, so this never classifies that binding as superseded. clearServiceInterfaces({ except }) retains each excepted id on every binding; it does not track which binding exported the id during this pass. On a real 0.4.0 box, changing Tor's OR port 9001→9002 left both records with interfaces.or, while 9001 was disabled and 9002 enabled. After upgrading to this head, the 9001 onion remained live against the dead bridge port and the 9002 Add Onion Service dialog offered no parked address. This is exactly the box test proposed in the PR body. Please make the final-state signal account for an interface id retained on its former binding (including same-id moves across bindings/hosts), and re-run this port-change case.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a07d326. The retained copy is what clear_service_interfaces leaves behind: it keeps an excepted id on every binding that ever carried it, and nothing removes the old record until start-technologies#3639 (remove_interface_records on re-export), which is on master but not in start-os/v0.4.0.1. getServiceInterface can't disambiguate either — find_service_interface_location returns the first binding carrying the id, which is the stale lower port in your case — so the test reads the host: a disabled binding is superseded when every interface id it keeps is also carried by an enabled binding of that host. Boot leaves the canonical binding disabled with its id on no enabled binding, so the boot window still reads as pending. A same-id move across hosts stays invisible on 0.4.0.1; 0.4.0.2 clears the copy at the package's next init. The port-change re-run on a box is the next step here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed a07d326 closes this blocker. I repeated the exact flow on the StartOS 0.4.0 template: before upgrade, 9001 was disabled and 9002 enabled while both retained interfaces.or; after upgrading to this head, torrc contained #HiddenServicePort 9001, the hidden-service warning disappeared, and the init log named tor/or-multi:9001 as parked. The mapped boolean also stays false during the all-disabled boot window and avoids watching unrelated host changes.

One correction to my earlier evidence: the statement that the Add Onion Service dialog offered only Create new address was not a valid UI test. start-cli package action get-input cannot pass plugin prefill (prefill is #[arg(skip)]), so that CLI invocation always generated the no-metadata spec. The live, unparked torrc was sufficient to establish the original blocker; I am retracting only the dialog observation.

…bled one from parking

clear_service_interfaces retains an excepted id on every binding that ever
carried it, and until StartOS 0.4.0.2 (#3639 in start-technologies) nothing
removes the copy an interface leaves behind when it moves to another port.
A same-id renumber, Tor's own OR port among them, therefore left the old
binding disabled with its interface still listed, and the superseded test
never fired.

A disabled binding now counts as superseded when every interface id it
keeps is also carried by an enabled binding of the host. Boot still leaves
the canonical binding disabled with its id on no enabled binding, so the
boot window stays pending as before.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@helix-nine helix-nine left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified the amended head. The retained-interface predicate now parks the same-host/same-id port move without reacting during the all-disabled boot window; the exact 9001→9002 upgrade test passes on StartOS 0.4.0. The documented pre-0.4.0.2 cross-host limitation matches the OS behavior and self-heals when interface export removes stale records. The PR footer is removed and the README matches the predicate.

Re-ran npm ci, npm run check, npm run build, make x86, and git diff --check; all pass. All three architecture CI builds are green.

@dr-bonez

Copy link
Copy Markdown
Member

Closing in favour of the redesign recorded in #38.

Under it a disabled binding is not treated as a deleted one: its ports stay reserved and nothing forwards to them, so an onion pointed at it is refused rather than misdirected, and Tor does not try to infer from a binding's shape that a service stopped declaring a port. A service that drops a port retires it, and that is what clears the onions attached to it. Start9Labs/nextcloud-startos#157 is that fix for the 8080 case.

The redesign also resolves forward targets when torrc is rendered instead of storing them, so the parking format this builds on goes away.

@dr-bonez dr-bonez closed this Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A superseded binding still counts as reachable, so its onion is neither parked nor marked unattached

3 participants