Repository navigation
Conversation
loadKuboConfigFile looked in .ipfs-bitsocial-cli while kubo runs in .bitsocial-cli.ipfs or $IPFS_PATH, so the read always came back empty and the daemon fell back to env vars and defaults. Both now use resolveKuboRepoPath. That makes the multiaddr parsers reachable for the first time, so they now handle IPv6 hosts, list-valued addresses and unix sockets.
computeKuboConfig(kuboConfig, reachabilityConfig, enabled) is the one place that decides the kubo edits. planKuboConfig also returns a record of what changed, which tells our entries from identical operator ones and lets disable restore the touched fields byte for byte.
An explicit KUBO_RPC_URL or IPFS_GATEWAY_URL can name a kubo other than the one in this repo, so it still wins over the kubo config. Connectivity checks now strip the brackets a URL keeps around an IPv6 host.
reachability/config.json holds the operator's choices and refuses to load a newer version rather than drop its fields. The daemon's check results go in a separate file so they cannot overwrite a setup in progress.
…e status Caddy keeps certificates for old addresses and other CAs until they expire, so status now filters to the configured CA and current names. A damaged runtime status file reads as no checks yet, and the text output prints domain and Caddy checks.
…le tests Caddy is downloaded on first use and checked against a sha256 pinned in source. The generated Caddyfile keeps Caddy off UDP 443 and port 80. The integration test issues a certificate for 127.0.0.1 from Pebble and completes a js-libp2p handshake with kubo through Caddy.
Discovers public addresses from interfaces, kubo and the router, obtains an IP certificate, applies the kubo config and restarts kubo. It checks what kubo really listens on, because kubo stays silent when it cannot bind UDP 443.
--domain gets a normal certificate when 443 is free, or prints a proxy snippet for the detected server when it is busy. --announce is added only after a libp2p handshake through it succeeds. A name that does not resolve here is refused unless --force is passed.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 30 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff035ebff4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!exitedEarly) { | ||
| await stopCaddy(spec.adminPort).catch(() => caddy.kill()); | ||
| await exited; |
There was a problem hiding this comment.
When setup starts its own Caddy—the normal first-run path—this block stops it immediately after certificate issuance, and no daemon code in this commit starts Caddy again (the only startCaddy call is in this function). Setup nevertheless records and announces /tls/ws addresses, so after the command exits TCP 443 is closed and browser/libp2p WSS dials fail; keep Caddy alive or have the daemon take it over before announcing the addresses.
Useful? React with 👍 / 👎.
|
|
||
| export async function discoverNatRouter(timeoutMs = 5000): Promise<NatRouter | undefined> { | ||
| const { upnpNat } = await import("@achingbrain/nat-port-mapper"); | ||
| const client = upnpNat({ description: "bitsocial reachability", autoRefresh: false, ttl: MAPPING_TTL_MS }); |
There was a problem hiding this comment.
For nodes behind a UPnP router, these mappings are created with a one-hour TTL while autoRefresh is disabled, and neither setup nor the daemon renews them. Such a node can initially obtain certificates and report success but loses TCP/UDP reachability when the router expires the leases, so the mappings need automatic refresh or daemon-managed renewal.
Useful? React with 👍 / 👎.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| const message = dnsMismatchMessage(domain, [...records.v4, ...records.v6], publicIps); | ||
| // Refuse what the operator asked for now. A stored name going stale must not block repairing | ||
| // the rest, so it only sits out this run, and is still stored for when its DNS is fixed. | ||
| if (options.domain && !options.force) throw new SetupRefusedError(message); |
There was a problem hiding this comment.
A domain typed at the interactive prompt (line 311) only sets the local domain, so options.domain is still undefined and this refusal is skipped. A mistyped answer is then treated like a stale stored domain: it warns, sits out the run, and is still saved to config.json (line 376). When publicIps.length === 0 the prompt is also shown (line 307), domainPointsHere is always false, so every answer is dropped but saved. The comment above says a domain the operator asked for now should be refused, and a prompt answer counts as that. Track whether the domain came from the flag or the prompt and use that here, and don't prompt when there are no public IPs to compare against.
| } | ||
|
|
||
| deps.log("WebTransport on UDP 443 is configured. No WSS address is announced."); | ||
| if (portBusy && domain) { |
There was a problem hiding this comment.
This branch doesn't check domainUsable. With a stored domain whose DNS has gone stale (so domainUsable = false), TCP 443 busy, and --non-interactive or no TTY, setup prints a proxy snippet for a name it just warned doesn't point here, then throws NeedsAnswerError (exit 2). That contradicts the intent that a stale stored domain sits out the run and doesn't block automated repair.
| if (portBusy && domain) { | |
| if (portBusy && domain && domainUsable) { |
| export function deriveMode(parts: { ip: boolean; domain: boolean; proxy: boolean }): ReachabilityMode { | ||
| if (parts.ip && parts.domain) return "ip+domain"; | ||
| if (parts.ip && parts.proxy) return "ip+reverse-proxy"; | ||
| if (parts.ip) return "ip"; | ||
| if (parts.domain) return "domain"; | ||
| if (parts.proxy) return "reverse-proxy"; | ||
| return "webtransport-only"; | ||
| } |
There was a problem hiding this comment.
deriveMode checks domain before proxy, and ReachabilityMode has no domain+proxy combination. When Caddy issues a domain certificate and a verified --announce proxy address is also present (the command's own example passes both), the stored mode is domain or ip+domain. reachability status prints Mode: domain while listing the proxy address, even though the PR says mode records the combination. Add modes for the combined cases, or derive the mode so the proxy part isn't lost. The deriveMode table in test/reachability/setup.test.ts also has no domain+proxy case.
|
Consolidated into #208. |
Part of #198 (step 6). Based on #205, so earlier commits show here until those merge.
Summary
Everything that succeeded is announced together, and
moderecords the combination.Decisions the issue left open:
/dns6, because/dns4could never connect.--domainwith 443 busy under--non-interactiveexits 2 after printing the snippet, because the operator still has to act.Evidence
After:
npm run test:clipasses.Merge Danger
Door: two-way
Blast Radius: opt-in
Only runs when an operator passes these flags or answers the prompt. Like #205, this should not ship without step 7.