Repository navigation
feat(link): add /api/link routes, the tunnel supervisor and ocx link - #5803
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (81)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
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. |
github-actions
Bot
added
the
intake: hygiene-blocked
|
✅ READY
Hygiene✅ Deterministic PR hygiene checks passed. |
리뷰 · 우선순위 66 / 80이 PR은 집 컴퓨터가 다른 컴퓨터를 SSH 터널로 붙이는 네 번째 층이다. 터널 프로세스를 살려 두고, 대시보드용
메인테이너의 판단이 필요한 지점 본문의 보안 리뷰 체크는 아직 비어 있다. 키 발급, 관리자 토큰, SSH 실행이 들어 있다. 신뢰하는 루프백은 요청이 도착한 리스너와
자동 테스트는 두 컴퓨터 사이 실제 SSH를 돌리지 않는다. 작성자도 GUI 층 다음에 확인한다고 적었다. 이 PR은 L1부터 L3 위에 쌓인 L4다. 스택에 남겨 두고, 너의 추천 목록을 저장하기 직전에 다시 읽어서, 지우거나 더하는 그 항목만 바꿔라. 원격 끊기가 실패하면 터널을 다시 띄워라. 이 댓글은 grok-bot이 작성했습니다 |
![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: 3a58c71992
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const current = readStoreFor(ctx); | ||
| const next = { ...current, links: current.links.filter(link => link.id !== record.id) }; | ||
| if (next.links.length !== current.links.length) { | ||
| writeStoreFor(ctx, next); |
There was a problem hiding this comment.
After a failed apply reaches compensation, the supervisor has already loaded this record, but stopLink() only kills the current child and sets its state to idle; deleting the record on disk without calling reload() leaves it in the supervisor's in-memory store, so the next timer tick respawns the SSH tunnel. The successful DELETE path at line 375 has the same defect, leaving a supposedly removed link's tunnel running and its remote port occupied until shutdown. Reload the supervisor after each persisted deletion.
Useful? React with 👍 / 👎.
| } catch { return fail("remote_disconnect_failed", "The remote client could not be disconnected.", 502); } | ||
| } | ||
| if (!revokeKeyIdempotent(ctx, record.apiKeyId)) return fail("key_revoke_failed", "The link key could not be revoked.", 502); | ||
| const next = { ...current, links: current.links.filter(link => link.id !== id) }; |
There was a problem hiding this comment.
The current snapshot was captured before stopLink() and a remote disconnect that may take up to 30 seconds. If another issue, apply, or delete request changes the store while either operation is awaited, constructing next from this stale snapshot overwrites that completed change—for example, dropping a newly issued link while leaving its API key live, or resurrecting a concurrently removed record whose key was revoked. Serialize link mutations or re-read the store immediately before filtering.
Useful? React with 👍 / 👎.
| if (!deps.baseUrl && error instanceof RuntimeApiError && error.status === 503) { | ||
| const store = (deps.readStore ?? (() => readLinkStore(linkStorePath())))(); | ||
| console.log(JSON.stringify(localStatus(store))); |
There was a problem hiding this comment.
runtimeBaseUrl() also throws status 503 when it discovers a live client-role listener, not only when no proxy is running. Consequently, normal ocx link status execution on a connected child enters this fallback, reads the hub-side link store (typically empty), and reports role: "standalone" instead of the child's link state. Narrow the fallback to the explicit no-live-proxy case or project status from the local client connection when the detected role is client.
Useful? React with 👍 / 👎.
github-actions
Bot
removed
the
intake: hygiene-blocked
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Reviewed exact head a0a5a24517b1c30d91797ba4593f4224390b90bc. The latest commit fixes the stale store snapshots, persists compensation evidence, restarts after remote-disconnect failure, and coalesces supervisor reloads safely. Three current-head blockers remain:
src/server/management/link-routes.ts:187-196and:398-407remove the record on disk afterstopLink(), but never reload the supervisor. Its in-memory store still contains the deleted hub link, so the timer can spawn the supposedly removed SSH tunnel again and keep the remote port occupied. Reload after every persisted deletion, including successful compensation.src/server/management/link-routes.ts:397returns immediately when key revocation fails after the tunnel was stopped. The store and still-valid key remain, but unlike the remote-disconnect failure branches, the tunnel is not restarted. Restore the prior live state or persist a durable failed state that prevents a silent half-removal.src/cli/link.ts:237-246treats every implicitruntimeBaseUrl()503 as “no proxy” and projects the local hub-side store.runtimeBaseUrl()also uses 503 for a live client-role listener, soocx link statuson a connected child reportsstandaloneinstead of child state. Distinguish the no-listener result from the explicit client-role refusal.
Please add regression coverage for successful delete/compensation reloading the supervisor, revoke failure after stopLink(), and client-role status projection before re-requesting review.
767dcda to
e346ece
Compare
src/link gains the no-shell SSH runner, host-key fingerprint parsing, the per-link tunnel supervisor with verified orphan reaping, the K16 status projection and a first-admission waiter. ocx link port/issue/revoke/status talk to the running proxy on loopback with the admin token and never print the key outside issue's stdout.
Paired dashboard sessions probe, confirm, apply and remove links; status and removal also accept the admin token on a trusted loopback ingress, and key issue accepts only that token. Tailscale-identity sessions are refused on every link route. Apply records the link, reloads the supervisor, runs the client's connect over SSH with the key on stdin and waits for the first key-authenticated catalog read; any failed step removes the new key and record.
… changed tunnels Apply compensation now revokes a newly issued key before removing its record and keeps the record with a compensation_failed status when revocation fails. Issue and apply refuse unless the hub-link listener is listening. A supervisor reload stops and respawns a tunnel whose record changed, and the trusted-loopback derivation is a tested pure function.
A key already absent from the live config counts as revoked, so a DELETE retried after a partially completed compensation finishes instead of failing on the key it removed the first time. The lifecycle-less issue path now reports compensation_failed when it cannot revoke.
… persisted compensation Link routes reread the store right before each write and change only their own record, a failed remote disconnect restarts the tunnel, and compensation_failed persists in a private sidecar that survives restarts. The supervisor coalesces reloads, reports connected only after an authenticated request or a surviving ssh child, and revokes link keys no record references at startup. Issue refuses a duplicate client alias.
a0a5a24 to
1cefb16
Compare
2 participants
Summary
Fourth layer of the remote link stack. It turns the building blocks into a working flow a home machine can drive: it keeps SSH tunnels alive, exposes
/api/link/*for the dashboard, and addsocx linkfor the CLI and for the child-initiated flow in L6.Supervisor (
src/link/supervisor.ts): onessh -R 127.0.0.1:P:127.0.0.1:Lchild per hub-initiated link, driven by the L1 reducer./proc/<pid>/cmdlinematches the recorded argv exactly; elsewhere it is reported, never killed.Runner:
src/link/ssh-runner.tsruns everysshandssh-keygenargv without a shell and caps captured output. Fingerprints come fromssh-keygen -lfon a temporary known_hosts file.Routes (
src/server/management/link-routes.ts, loaded lazily):GET /api/link/candidates,POST /api/link/probe,POST /api/link/confirm-host,POST /api/link/applyGET /api/link/status,DELETE /api/link/{id}POST /api/link/issueTailscale-identity sessions are refused on every link route. "Trusted loopback" comes from the listener the request arrived on, not from
Host, so the Tailscale Serve management ingress and the hub-link ingress never qualify.Apply: probe the host key → operator confirms the fingerprint → issue a data key → record the link → ensure the listener is listening → reload the supervisor → run
ocx connect --link --key-stdinon the client over SSH with the key on stdin. Success needs exit 0 and the first key-authenticated/v1/catalogread within 15 seconds. A failed step revokes the new key before removing its record. If revocation fails, the record stays with acompensation_failedstatus, and revocation is idempotent so a retried removal completes.Remove: stop the tunnel →
ocx disconnecton the client → revoke → delete the record → close the listener after the last link. A failed remote disconnect keeps everything unlessforceis set.CLI:
ocx link port|issue|revoke|status(src/cli/link.ts), registered in capabilities, the usage banner and the generated skill surface. The key is printed only onissue's stdout.Docs:
structure/remote-link.mdandstructure/gui-and-management-api.md.Stack: #5788 (L1) ← #5798 (L2) ← #5801 (L3) ← this PR (L4) ← GUI (L5) ← child-initiated flow (L6). Base is
codex/remote-link-3-client.Security review requested (MAINTAINERS.md). This covers key issuance, admin-token routes, SSH execution and host-key trust.
Verification
/api/keyssuites and test-layout: 431 pass, 0 fail.bun run typecheck,bun run structure:check,bun run privacy:scan,bun run skill:surface:check,bun scripts/file-size-ratchet.ts: pass.src/server/index.tsis 891/893 lines.bun run test:changed(24,403 tests): I compared the failing files against the pre-change commit in a separate worktree.ocx linkmissing from the usage banner) and is fixed here.Checklist
Review follow-up (a0a5a24)
This addresses the review comment on this PR:
issue,applyandremoverereadlinks.jsonright before each write and change only their own record. Links written by another request during the SSH wait survive (race tests for all three).remote_disconnect_failed.compensation_failedpersists in a privatelink/compensation.jsonsidecar, so status shows it after a restart. At startup the supervisor revokes anylink:key that no record references.connectedonly after the hub-link listener sees an authenticated request with its key, or after its ssh child survives a 5-second grace period. An early exit goes to reconnecting or failed.issuerejects a duplicate client-initiated alias with409 link_exists, the same asapply.catch {}in the route tests is gone. The repository hygiene rules run locally against this diff return no findings.Verification at a0a5a24: 433 pass / 0 fail on the link, route, listener, boundary, CLI, registry,
/api/keysand layout suites, plus typecheck, structure, privacy and file-size gates.Still for maintainers: the security review, and the loopback-trust question (the admin token on a public listener bound to a loopback name is accepted for the CLI path). The PR stays in draft until the security review.