fix(local): fail closed DNS bind names for credential-bearing destinations

[luvs01]

Summary

Follow-up to #491's direction: credential-bearing local destinations must not dial a re-resolvable name.

probeHostname returns a DNS bind name unchanged, and the credential-bearing destination composers (localInferenceDestination, localManagementOrigin, resolveApiAccessBaseUrl) then embed that name in a URL the client dials with credentials attached. A hostname that re-resolves at dial time is a DNS-rebinding exfiltration path: the credential leaves for whatever the name resolves to then, not what it resolved to at compose time.

New localCredentialDestinationHostname keeps literal IPs as-is and fails closed to 127.0.0.1 for DNS names, so credential-bearing destinations only ever target a literal address. Display-only hosts keep the resolved name.

Test plan

• bun test tests/lib/local-destinations.test.ts tests/server/api-access-endpoints.test.ts — 39 pass: DNS bind name fails closed to loopback for inference, management-origin, and API-access base URLs; literal non-loopback IPs still compose; wildcard/request-derived behavior unchanged.
• bun x tsc --noEmit clean.

Declaration

• This PR is ready for review
• I have tested this change locally
• I have linked related issues or context

Validation

• Focused tests pass (39/39 across the two touched suites)
• Typecheck passes (tsc --noEmit)
• Fail-closed default: DNS names degrade to loopback, never to a re-resolved address

Risk

• Only credential-bearing destinations change; display hosts keep the resolved name
• Literal IP binds are untouched
• Based on current dev tip (0 behind)

Notes

• Draft until maintainer review

Summary by CodeRabbit

• Bug Fixes
  • Credential-bearing inference and management destinations using DNS bind names now safely resolve to 127.0.0.1.
  • Literal IP addresses continue to retain their configured addresses.
  • Loopback hostnames, including localhost, preserve their existing behavior.
  • Generated API base URLs no longer expose unreachable public addresses for DNS-based binds.
• Tests
  • Added coverage for DNS bind names, literal IP addresses, loopback hosts, and credential-bearing API URLs.

[coderabbitai]

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: c5a830fb-9eed-4d02-a979-774bf00ebaca

📥 Commits

Reviewing files that changed from the base of the PR and between f3cb520 and e6635e2.

📒 Files selected for processing (4)

• src/lib/local-destinations.ts
• src/server/management/api-access.ts
• tests/lib/local-destinations.test.ts
• tests/server/api-access-endpoints.test.ts

 _________________________________________________________________________________________________________________________________
< For a successful technology, reality must take precedence over public relations, for Nature cannot be fooled. - Richard Feynman >
 ---------------------------------------------------------------------------------------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       / 　 づ

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

✅ Deterministic PR hygiene checks passed.

[github-actions]

⏳ DRAFT

• review readiness checklist open (10/10 boxes ticked).

What to do

• Tick all four boxes in the PR description once you're done (currently 10/10).

Review readiness checklist

• ✅ All CI tests are green on my local testing.
• ✅ I pushed my PR to the latest dev commit.
• ✅ I resolved all correct Codex and CodeRabbit findings.
• ✅ My PR is ready for review.

10/10 boxes ticked.

Automatic draft conversion failed. Please convert this pull request to a draft manually until every box above is ticked.

[lidge-jun]

리뷰 · 우선순위 73 / 80

이 PR은 credential을 실어 나르는 로컬 destination이 DNS로 다시 풀릴 수 있는 bind 이름을 URL에 넣지 못하게 막습니다. 지금 dev(facd2b6ca)에서는 probeHostname이 DNS 이름을 그대로 돌려주고, localInferenceDestination / localManagementOrigin / resolveApiAccessBaseUrl이 그 이름을 origin에 넣습니다. 클라이언트가 나중에 다시 resolve하면 compose 때와 다른 peer로 credential이 나갈 수 있습니다. DNS rebinding exfiltration 경로입니다. #491 방향의 후속이고, 보안 경계가 분명한 좁은 fix입니다.

변경 핵심은 localCredentialDestinationHostname입니다. probeHostname 결과에 대해 node:net의 isIP로 literal인지 보고, literal IP(대괄호 IPv6 포함)면 그대로, DNS 이름이면 127.0.0.1로 fail-closed합니다. admission token 요구(shouldInjectApiAuthHeader)는 그대로라서, “이름은 loopback으로 떨어지지만 credential은 여전히 필요”합니다. 리스너가 loopback에서 안 받으면 연결이 거절될 뿐, 토큰이 다른 peer로 새지 않습니다. literal tailnet/LAN IP는 예전처럼 자기 주소를 유지합니다. display-only host는 이 헬퍼를 안 쓰거나(문서상) 자격 증명 destination만 바꿉니다.

테스트가 의도를 잘 고정합니다. mutable-bind.example → inference/management/API base 모두 http://127.0.0.1:10100(+/v1), literal non-loopback은 127.0.0.1이 아님, wildcard/request-origin 동작은 기존 스위트에 남김. +66/-9, 파일 네 개. types/config 분할과 무관하고 중복 PR로 보이지 않습니다.

라인 (PR) localCredentialDestinationHostname - isIP(literal) !== 0 ? probed : "127.0.0.1". IPv6 bracket strip 후 판정하는 순서가 맞다.
경로 src/lib/local-destinations.ts - inference·management origin이 새 헬퍼를 쓴다. credential-bearing 경계가 한곳으로 모인다.
경로 src/server/management/api-access.ts - base URL·display host의 non-wildcard 분기도 같은 헬퍼. display host까지 fail-closed하면 UI에 127.0.0.1이 보일 수 있다 — 의도인지 확인.
테스트 mutable-bind.example - DNS fail-closed를 이름 하나로 고정. 좋다.
경로 #491 - 자격 증명 local destination 방향의 후속. tip #5040과 무관한 local/security fix.

메인테이너의 판단이 필요한 지점

• DNS bind + credential이 필요한 운영자가 “전용 loopback listener를 켜라”는 문서/에러 메시지가 충분한지.
• resolveApiAccessDisplayHost까지 127.0.0.1로 떨어뜨리는 게 UX상 괜찮은지, display는 원래 이름을 남길지.
• draft 체크리스트가 아직 열려 있으면 ready 표시만 맞추면 되는지.

너의 추천
merge 쪽으로 간다. 범위가 작고 fail-closed 기본값이 맞으며 테스트가 핵심 계약을 잠근다. display host 정책만 한 줄 확인하고, draft/ready만 정리한 뒤 랜딩.

이 댓글은 grok-bot이 작성했습니다

[lidge-jun]

Pushed a follow-up commit (e6635e2c33) and rebased onto current dev.

test 3/4 was a real regression rather than an environment problem: ocx claude management discovery destination > a loopback or wildcard install keeps asking 127.0.0.1 on the public port pins that a localhost install keeps writing http://localhost:<port> into its exported client configuration, and the fail-closed branch rewrote it to 127.0.0.1 because localhost is a name rather than a literal.

The rewrite also bought nothing. RFC 6761 reserves localhost to loopback, so a second lookup cannot select a peer off this machine — which is the only outcome this branch exists to prevent. The carve-out reuses isLoopbackHostname, already imported in this module and already the encoding of "this name is loopback", rather than growing a second list beside it.

One detail worth recording for the next reader: only localManagementOrigin can observe this. localInferenceDestination answers a loopback bind from its own earlier branch and never reaches the name check, so the new case asserts both, with the different answers each one correctly gives.

The rest of the change is right and I am keeping it: a bind name the client re-resolves can point somewhere other than the listener, and a credential-bearing destination must not take that risk.

[lidge-jun]

Merging. The fail-closed reasoning is right and the localhost carve-out is what makes it correct rather than merely strict.

A bind name the client re-resolves can point somewhere other than the listener, and a destination that carries a local credential must not take that risk — so a DNS bind degrading to a socket that refuses is the right failure. localhost is the one name where that reasoning does not apply: RFC 6761 reserves it to loopback, so a second lookup cannot select a peer off this machine. Rewriting it would have bought nothing and changed the origin every existing loopback install writes into its exported client configuration.

The carve-out reuses isLoopbackHostname, already imported in this module and already the encoding of "this name is loopback", rather than growing a second list beside it.