Skip to content

fix(link): gate join credential on tunnel survival - #644

Closed
luvs01 wants to merge 4 commits into
devfrom
codex/fix-vulnerability-in-child-join-process
Closed

luvs01 wants to merge 4 commits into
devfrom
codex/fix-vulnerability-in-child-join-process

Conversation

@luvs01

@luvs01 luvs01 commented Sep 26, 2026 •

Copy link
Copy Markdown
Owner

Motivation

  • Prevent a local port-squatting race where an attacker can bind the selected loopback port and receive the issued x-opencodex-api-key before the SSH tunnel actually owns the forward.
  • Ensure enrollment only proceeds when the client-owned SSH tunnel is observed to survive its initial spawn window so credential delivery and connect/commit steps are bound to the authenticated transport.

Description

  • Add a short spawn-grace (JOIN_TUNNEL_SPAWN_GRACE_MS) and require the spawned tunnel handle to be observed before issuing credential-bearing readiness probes by changing waitForReady to accept a ClientLinkTunnelHandle and racing readiness probes against tunnel.exited.
  • Race every fetch of the loopback /readyz endpoint with tunnel exit so an exited tunnel short-circuits enrollment and triggers the existing rollback flow instead of delivering the key.
  • Pass the tunnel handle through joinHome so the readiness gate can monitor the supervisor handle returned by spawnClientLinkTunnel and fail fast on tunnel termination.
  • Add a regression test in tests/server/link-join-route.test.ts proving that an already-exited tunnel does not receive the issued key, that issuance is revoked, and that rollback behavior remains intact.
  • Document the tunnel-survival contract in structure/runtime.md to record the architectural expectation for client-initiated Remote Link enrollment.

Testing

  • Ran the focused unit tests: ./node_modules/.bin/bun test tests/server/link-join-route.test.ts, which passed (11 tests, 0 failures) under the repository-pinned Bun used in CI emulation.
  • Ran typecheck: ./node_modules/.bin/bun run typecheck, which passed.
  • Ran structure gate: ./node_modules/.bin/bun run structure:check, which passed after the small structure/runtime.md update.
  • Ran privacy scan: ./node_modules/.bin/bun run privacy:scan, which passed.
  • Note: the older system Bun (1.2.14) in this environment lacked an export used by the repo tooling; the focused test suite and checks were executed successfully with the repository-pinned Bun used above.

Codex Task


Devin Review

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Repository: luvs01/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: a8496f4b-8883-41bd-8c14-5981a3f96dad


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the bug Something isn't working label Sep 26, 2026
devin-ai-integration[bot]

This comment was marked as resolved.

@github-actions

Copy link
Copy Markdown

✅ Deterministic PR hygiene checks passed.

…onnect

Co-Authored-By: Epinephrine <luvs01@hanmail.net>
devin-ai-integration[bot]

This comment was marked as resolved.

…readyz

A port squatter could answer the unkeyed /readyz probe with the 401
challenge and receive the following keyed request; readiness now only
runs while the LISTEN owner of the tunnel port is the spawned ssh
process (unverifiable scans stay not-ready), and both probes use
redirect: manual so a redirecting occupant cannot reroute the
challenge or the credential-bearing request.

Co-Authored-By: Epinephrine <luvs01@hanmail.net>
devin-ai-integration[bot]

This comment was marked as resolved.

… ss fallback

Three join-readiness hardening fixes:

- scanListenEntries now keeps each listener's bound address and the
  readiness check only counts sockets that serve the tunnel's 127.0.0.1
  bind — a listener on 127.0.0.2 or another interface no longer stalls
  enrollment until the issued link is revoked.
- The POSIX scanner chain gains ss -Hltnp between lsof and netstat, so
  minimal Linux installs with only iproute2 can still verify ownership
  instead of failing every probe as unavailable.
- Ownership is re-verified in the same iteration immediately before the
  keyed request, narrowing the scan-to-request takeover window that
  could have delivered the issued key to a port flipper.

Co-Authored-By: Epinephrine <luvs01@hanmail.net>
@luvs01

luvs01 commented Sep 27, 2026

Copy link
Copy Markdown
Owner Author

이관됨: lidge-jun#6042

@luvs01

luvs01 commented Sep 27, 2026

Copy link
Copy Markdown
Owner Author

동일 수정이 상류 저장소에 제출되어 이 포크 PR의 목적은 달성됐습니다.

@luvs01 luvs01 closed this Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark bug Something isn't working codex

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant