Repository navigation
feat(link): connect a client to its hub through an SSH tunnel - #5801
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 (55)
✨ 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. |
|
✅ Deterministic PR hygiene checks passed. |
443b8b1 to
093e1aa
Compare
c61ce2e to
767dcda
Compare
![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: c61ce2ecf1
ℹ️ 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".
| if (!Number.isInteger(localPort) || localPort < 1 || localPort > 65535) { | ||
| throw new Error("link mode requires a valid local config port"); | ||
| } | ||
| return `http://localhost:${localPort}/v1`; |
There was a problem hiding this comment.
On Windows, localhost commonly resolves to ::1 first, but the linked client listener is explicitly bound only to 127.0.0.1 in src/client/machine-listener.ts. The repository's existing providerBaseHost helper already uses 127.0.0.1 to avoid this exact Windows failure; using localhost here makes every Codex request fail with connection refused on affected systems. Generate the link target with 127.0.0.1 as well.
Useful? React with 👍 / 👎.
| if (linkMode && linkRouteAllowed(url, req)) { | ||
| return relayLinkDataRequest(req, { tunnelPort: connection.link!.tunnelPort }, { fetchImpl: deps.fetchImpl }); |
There was a problem hiding this comment.
In link mode, GET /readyz satisfies linkRouteAllowed, so this branch runs before the listener's local readiness response. ocx ready probes /readyz without a data key, while the hub-link ingress requires one, causing every linked runtime to appear unreachable; even a keyed request returns the hub's PID and port and fails the local identity check. Handle the machine listener's own /readyz before relaying link routes, or exclude readiness from this local relay path.
Useful? React with 👍 / 👎.
| if (linkMode && (!Number.isInteger(config.port) || config.port < 1 || config.port > 65535)) { | ||
| throw new Error("link mode requires a valid local config port"); | ||
| } | ||
| const preferred = linkMode ? config.port : options.port ?? config.port ?? 10100; |
There was a problem hiding this comment.
If config.port changes after ocx connect --link—including when an operator follows the new error's advice to “change port”—startup binds the new value here, but the injected Codex configuration still targets the port captured during connect, and the connected-client startup path performs no reinjection before blocking. The runtime therefore starts successfully while all Codex requests continue hitting the old dead port. Reinject or validate the saved routing target at startup, or reject port drift until an explicit sync updates it.
Useful? React with 👍 / 👎.
| ocx connect --link --key-stdin --tunnel-port <port> --link-id <id> | ||
| [--clients codex,claude] [--catalog-timeout <seconds>] [--no-sync] |
There was a problem hiding this comment.
This exposes a new user-facing command whose stdin must be an exact JSON object containing apiKeyId and a patterned key, but only the internal error usage was updated: ocx help connect still reads the old hub-only entry from src/cli/registry.ts, and the public remote-hub documentation contains no link-mode syntax or credential format. Operators cannot construct this command from published help, so update the command registry and docs-site/ alongside the implementation.
AGENTS.md reference: AGENTS.md:L452-L453
Useful? React with 👍 / 👎.
리뷰 · 우선순위 72 / 80이 PR은 원격 링크의 세 번째 층입니다. 자식 컴퓨터가 공개 HTTPS 주소 대신 SSH 터널로 집(허브)에 붙습니다. 기준 브랜치는 Codex 설정 모양은 그대로입니다. Codex는 자식의
라인 - 라인 - 라인 - 라인 - 라인 - 메인테이너의 판단이 필요한 지점
Codex 주소를 체크리스트의 보안 리뷰는 아직 비어 있습니다. 표준 입력 열쇠와, 자격 증명을 더하지 않는 릴레이가 이 층의 경계입니다. 받은 바이트는 0으로 지우지만, JSON으로 읽은 열쇠 문자열은 너의 추천 방향은 맞습니다. 터널 주소만 허용하고, 회전과 폐기는 허브에 맡기며, 포트를 못 잡으면 임시 포트로 넘어가지 않습니다. 닫을 중복 PR은 없습니다. #5798이 이 댓글은 grok-bot이 작성했습니다 |
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Reviewed exact head 767dcda. Blocking credential-boundary issue: forwardLinkRequestHeaders calls filterRelayHeaders without an allowlist and omits only Host, Content-Length, hop-by-hop, and Connection-nominated fields. Requests reaching the child machine listener can therefore forward child-local management secrets (x-opencodex-machine-session, x-opencodex-machine-csrf-token, x-opencodex-machine-gui-origin) and cookies through the tunnel to the hub data listener. A direct probe of the current helper returns all three of machine-session, machine-CSRF, and Cookie as present. The response path likewise forwards unrestricted end-to-end headers, including Set-Cookie, back into the child localhost origin. Please use an explicit data-plane request/response allowlist (or at minimum strip every machine/management credential and Cookie/Set-Cookie), and add regression tests proving local management/session headers cannot cross either direction while required Codex/Anthropic data-plane headers still do. Focused connection, relay, runtime, injection, listener, and boundary suites otherwise passed 143/143 under CPUQuota=200%, MemoryMax=4G, MemorySwapMax=0, TasksMax=128. The stack also remains blocked on #5788.
093e1aa to
6914338
Compare
A client with transport link keeps Codex on its own localhost port, accepts its data key on bounded stdin, and relays exactly the hub-link route table through the tunnel with the hub-relay header and body bounds. It binds its configured port or fails, keeps WebSockets off, and refuses key rotation and revocation, which the hub owns.
Link mode is now an explicit flag on the Codex routing target instead of an inference from localhost plus admission, so hub connections on localhost keep WebSockets. The link credential reader zeroes every source chunk and copy on success, error and overflow. Hub-mode connect again reads config after the catalog download, keeping its stderr order.
767dcda to
e346ece
Compare
2 participants
Summary
Third layer of the remote link stack. A child machine can now connect to its home (hub) through an SSH tunnel instead of a public HTTPS origin. Codex on the child keeps pointing at the child's own
http://localhost:<port>, so its config does not change shape.OcxClientConnectionConfiggainstransport?: "hub" | "link"andlink?: { tunnelPort, linkId }. Existing configs are unaffected because an absent transport means hub. In link mode both origins must behttp://127.0.0.1:<tunnelPort>, with the tunnel port restricted to 1024-65535.ocx connect --link --key-stdin --tunnel-port P --link-id IDreusesconnectClientand its journal. It reads the data key from at most 4 KiB of stdin and zeroes the source and copies on every path.src/client/link-relay.ts): the machine listener forwards exactly the route table insrc/link/routes.ts, which the hub-link listener from L2 now shares.503withRetry-Afterwhile the tunnel is down. The management relay is off in link mode.structure/remote-link.mdhas a "Client link transport" section, andstructure/runtime.mdpoints to it.Stack: #5788 (L1) ← #5798 (L2) ← this PR (L3) ← supervisor/API/CLI (L4) ← GUI (L5) ← child-initiated flow (L6). Base is
codex/remote-link-2-hub-listener.Security review requested (MAINTAINERS.md). This handles a data key on stdin and adds a credential-free relay into the tunnel.
Verification
cli-connect-readinessand test-layout suites: 212 pass, 0 fail.tests/codex-integration/injection-*.test.ts: 21 pass.bun run typecheck,bun run structure:check,bun run privacy:scan,bun scripts/file-size-ratchet.ts: pass.bun run test:changedselected 24,309 tests. I reran the 45 files with failures against the pre-change commit in a separate worktree.Checklist
structure/remote-link.md,structure/runtime.md; user docs land with the GUI layer)