diff --git a/devlog/_plan/260807_untouched_bug_stack/100_loopback_peer_admission.md b/devlog/_plan/260807_untouched_bug_stack/100_loopback_peer_admission.md new file mode 100644 index 000000000..08c71baf1 --- /dev/null +++ b/devlog/_plan/260807_untouched_bug_stack/100_loopback_peer_admission.md @@ -0,0 +1,378 @@ +# 100 — #1102: `0.0.0.0` 바인드에서 로컬 Codex 가 401 로 막힌다 + +> **개정 이력.** 첫 판은 "opt-in 으로 loopback 소켓 피어를 무인증 admit" 을 +> 제안했다. 독립 감사가 P1 다섯 건으로 되돌렸고, 그중 둘이 설계를 바꿨다: +> (a) 그 스위치는 `resolveApiAuth` 를 타고 #1102 와 무관한 8개 엔드포인트까지 +> 열고, (b) 공용 리스너의 피어 주소는 최종 사용자 신원이 아니다. 아래는 +> 재설계된 판이다. +> +> **2차 개정.** 재설계본도 감사에서 P1 세 건을 받았다. 설계 방향은 유지됐지만 +> 구현 계약이 비어 있었다: ephemeral 포트가 재시작마다 바뀌면 우리가 부정했던 +> "재시작 후 app-server 가 깨진다" 를 우리 손으로 만들고, 로컬 리스너의 +> auth/origin/WS 처리 경계가 미정이며, 두 bind 가 하나의 트랜잭션이 아니었다. +> 아래 §고정 포트 / §리스너 정책 / §바인드 트랜잭션 이 그 답이다. +> +> **3차 개정.** 세 번째 감사가 P1 둘을 더 찾았고 둘 다 검증된 사실이다: +> 카탈로그가 없을 때 app-server 가 `GET /v1/models` 로 폴백하는데 우리 +> allowlist 가 그걸 404 로 막고, `allSettled` 만으로는 stop 실패가 삼켜져 +> 재시작이 아직 포트를 쥔 리스너 위에 바인드를 시도한다. + +## 이슈가 말한 것과 실제 + +리포터는 두 개의 트리거를 보고했다. 하나는 정확했고, 하나는 원인이 다르다. + +**맞음 — direct-spawn 갭.** `app-server` 는 shim 의 `CODEX_INTERNAL_COMMANDS` +(`src/codex/shim.ts:42`) 에 있고 shim 은 디스패치 전에 토큰을 export 한다 +(`:384-389`). 그러니 shim 을 거친 `codex app-server` 는 인증된다. 문제는 서드파티 +호스트가 `require.resolve('@openai/codex/bin/codex.js')` 로 엔트리포인트를 직접 +resolve 해서 spawn 할 때다. 그 경로는 shim 을 통째로 우회하고, 대안이 없다: +`/v1/responses` admission 은 `x-opencodex-api-key` 만 받고 +(`src/server/auth-cors.ts:369-376`), 토큰 파일은 admission 시점에 읽히지 않는다. + +**틀림 — "재시작하면 토큰이 회전된다".** `writeServiceApiTokenFile()` 은 이미 +`process.env.OPENCODEX_API_AUTH_TOKEN` 에 있는 값을 쓰고, 없으면 아무것도 쓰지 +않는다 (`src/service.ts:347`). 호출자는 service install/repair 뿐이고 +(`:1707`, `:1799`, `:1937`, `:2116`), `ocx service start` 는 파일을 다시 쓰지 +않는다 (`:2660`). 토큰은 애초에 사용자가 공급하는 값이고 OpenCodex 가 생성하지 +않는다. 그러니 평범한 재시작이 살아 있는 app-server 를 무효화하지 않는다. + +이 정정은 이미 이슈에 코멘트로 게시되어 있고, 리포터에게 두 가지를 물었다. +답은 아직 없다. + +## 왜 파일 기반 대안이 전부 막히는가 + +토큰을 shim 밖 프로세스에 "실제로 전달" 하려면 그 프로세스의 환경을 바꿔야 +하는데, OS 프로세스 환경은 spawn 시 복사되고 우리는 남의 프로세스 환경을 사후에 +못 바꾼다. 남는 후보를 전부 확인했다: + +| 후보 | 왜 안 되는가 | +|---|---| +| Codex `env_http_headers` 를 파일 기반으로 | 값이 **환경변수 이름**이다. 업스트림 설계이고 우리 쪽 변경 범위 밖 | +| static `http_headers` | 시크릿을 `~/.codex/config.toml` 에 평문으로 직렬화한다. 백업·저널·동기화 경로로 퍼진다 | +| `auth.command` | bearer credential 을 공급하는데, `/v1/responses` 는 전용 헤더만 받는다. Codex Direct 와 충돌 방지를 위한 의도적 거부 (`auth-cors.ts:369-372`) | +| OS 전역 환경 주입 | 무관한 GUI/터미널 자식까지 credential 을 상속한다. 이미 떠 있는 호스트에는 적용도 안 된다 | + +전부 막힌다. 그래서 이건 credential **전달** 문제가 아니라 admission **정책** +문제다. + +## 첫 설계가 왜 틀렸나 + +처음에는 `isApiAuthRequired()` 를 우회하는 opt-in 스위치 +(`trustLoopbackPeersOnRemoteBind`) 를 제안했다. 감사가 두 가지를 지적했고 둘 다 +코드로 확인된다. + +**하나 — 폭발 반경.** `resolveApiAuth` 는 8곳에서 호출된다 +(`src/server/index.ts:692, 882, 903, 937, 1009, 1024, 1087, 1121`): `/v1/models`, +Images generations/edits, artifacts, alpha search, Messages, Live/Realtime, +sideband WebSocket. `resolveResponsesApiAuth` 도 `/v1/responses` 만이 아니라 +compact 와 Chat Completions 경로에서 쓰인다. resolver 안에 피어 예외를 넣으면 +#1102 가 요청하지 않은 표면 전부가 같이 열린다. 수용 기준이 +`/v1/responses` 만 검사했으므로 이 확대를 탐지하지도 못했을 것이다. + +**둘 — 피어 주소가 증명하는 것.** `requestIP()` 는 **마지막 transport hop** 만 +알려준다. Docker Desktop 의 포트 포워딩, `--network host` 컨테이너, WSL2 의 +mirrored networking 과 `netsh portproxy`, Kubernetes sidecar, VPN/터널 종단 — +전부 원격 연결을 로컬 TCP 연결로 다시 연다. 그 배포에서는 원격 호출자가 +loopback 피어로 보인다. 흔한 구성이고, 첫 판은 리버스 프록시와 SSH 터널만 +예시로 들어 이 계열을 과소평가했다. + +"opt-in 이니까 괜찮다" 로는 부족하다. 켜는 사람이 자기 배포가 저 목록에 +해당하는지 모를 수 있다. + +## 재설계 — 인증을 우회하지 않고, 별도 리스너를 연다 + +감사가 제시한 대안이 더 낫다. 공용 리스너의 admission 정책은 **한 줄도** 바꾸지 +않는다. 대신 `127.0.0.1` 에만 바인드된 **두 번째 리스너**를 옵션으로 연다. + +``` +0.0.0.0:10100 ← 기존 리스너. 인증 정책 불변. 모든 원격 호출자는 키가 필요하다. +127.0.0.1:PORT ← 새 리스너. 커널이 원격 연결을 아예 받지 않는다. +``` + +차이가 핵심이다. 첫 설계는 "원격에서 온 연결인데 로컬처럼 보이면 통과" 였다. +이 설계는 **커널이 원격 연결을 애초에 accept 하지 않는다.** 판정할 주소가 없고, +속일 피어 필드도 없다. Docker 포트 포워딩도 `127.0.0.1` 바인드는 기본적으로 +호스트 밖으로 내보내지 못한다. + +주입되는 Codex provider block 은 이미 wildcard 바인드에서 `base_url` 을 +`127.0.0.1` 로 쓴다 (`tests/codex-inject.test.ts:47-54`). 그 URL 의 포트만 로컬 +리스너로 바꾸면 shim 을 우회해 직접 spawn 된 app-server 도 인증 없이 붙는다 — +**공용 리스너의 경계는 한 줄도 건드리지 않고.** (넓히는 것이 없다는 뜻은 +아니다 — 명시적인 로컬 신뢰 표면이 하나 추가된다. 아래 §여전히 opt-in 인 +이유 참조.) + +### 여전히 opt-in 인 이유 + +`127.0.0.1` 바인드라도 그 머신의 **모든 로컬 프로세스**가 접근할 수 있다. +단일 사용자 워크스테이션에서는 받아들일 만하고, 멀티테넌트 호스트에서는 아니다. +그래서 기본값은 꺼짐이고, 이름은 결과가 드러나게 짓는다: +`unauthenticatedLoopbackListener`. + +더 정확히 말하면, 이 설계는 **보안 경계를 넓히지 않는** 것이 아니라 +**공용 리스너의 경계를 그대로 두고 명시적인 로컬 신뢰 표면을 하나 추가하는** +것이다. 그 표면에서 무인증 로컬 프로세스는 active-turn capacity, 계정 풀 쿼터, +유료 provider credential 을 소비할 수 있다 — 즉 인증된 원격 클라이언트를 굶길 +수 있다. 문서 경고는 "모든 로컬 프로세스가 접근 가능" 에서 멈추지 않고 이 +비용·DoS 측면까지 적는다. + +## 고정 포트 — ephemeral 은 우리가 부정한 버그를 우리가 만든다 + +첫 재설계본은 포트 미지정 시 OS 할당을 허용했다. 그건 틀렸다. + +`ocx sync` 와 startup sync 는 공용 `port` 만 `injectCodexConfig()` 에 넘긴다 +(`src/codex/sync.ts:100`, `src/cli/index.ts:353`). 로컬 리스너의 실제 포트를 +발견할 경로가 없다. 그리고 ephemeral 포트는 재시작마다 바뀔 수 있는데, +`config.toml` 이 새 포트로 다시 쓰여도 **이미 실행 중인 app-server 는 시작 시 +읽은 옛 `base_url` 을 계속 쓴다.** + +그 실패 모드를 그대로 읽어보면 — "재시작하면 이미 떠 있는 app-server 가 깨진다" +— 이 이슈가 신고했고 우리가 코드로 부정한 바로 그 증상이다. 원인이 토큰 회전이 +아니었을 뿐이고, ephemeral 포트로는 진짜로 만들어낸다. + +**포트는 설정에 필수로 둔다.** 오프라인 `ocx sync`, 재시작, 이미 실행 중인 +app-server 가 전부 같은 값을 본다. 활성화 시 포트를 안 주면 config 검증이 +거부한다. + +## 리스너 정책 — 무엇을 어떻게 다르게 취급하는가 + +로컬 리스너는 같은 프로세스, 같은 라우팅, 같은 계정 풀을 쓴다. 다른 것은 두 +가지뿐이다. + +**1. auth/origin 판정용 config view.** 같은 `config` 객체를 그대로 넘기면 +`hostname` 이 `"0.0.0.0"` 이라 `resolveResponsesApiAuth()` 가 여전히 인증을 +요구한다. 그렇다고 config 전체를 `{...config, hostname:"127.0.0.1"}` 로 복제해 +오래 들고 있으면 management 로 설정을 바꿨을 때 로컬 리스너가 낡은 값을 쓴다. + +그래서 **비즈니스/라우팅은 canonical config 를 공유하고, auth 와 origin 판정에만 +매 요청 만든 view 를 넘긴다.** + +**resolver 시그니처는 바꾸지 않는다.** `resolveResponsesApiAuth(req, config)` 에 +`allowUnauthenticated` 같은 파라미터를 추가하면 공용 리스너에서도 호출 가능한 +admission 우회 스위치가 생긴다. 정책 선택은 resolver 밖, 리스너 클로저에서 +한다. + +view 를 받는 함수는 이것들 전부다 — 하나라도 빠뜨리면 그 지점만 공용 정책으로 +판정한다: + +- `resolveResponsesApiAuth` +- `isAllowedRequestOrigin` +- `withCors`, `corsHeaders` +- `jsonResponse` — `/v1/models` 의 성공 응답이 이걸 통과하며 내부에서 CORS + 헤더를 만든다 (`src/server/auth-cors.ts:187-191`). 빠뜨리면 그 경로만 공용 + 정책으로 헤더를 붙인다 +- 에러 응답 헬퍼 (CORS 헤더를 붙이는 것들) + +모델 수집과 응답 내용 구성에는 계속 canonical config 를 넘긴다 — view 는 오직 +auth/CORS 판정용이다. + +view 타입은 `Pick` 수준으로 +좁힌다. 완전한 비즈니스 config 로 위장할 수 없어야 실수로 라우팅 경로에 흘러도 +타입에서 걸린다. + +**2. origin 게이트는 반드시 적용한다.** 인증만 우회하고 origin 검사에 공용 +config 를 넘기면 `isAllowedRequestOrigin` 의 remote 분기를 타서 +`isSameOriginAsRequest()` 로 허용될 수 있다 (`src/server/auth-cors.ts:76-82`). +공격자 서버가 피해자 브라우저로 `127.0.0.1` 에 붙는 DNS rebinding 이 정확히 그 +모양이다 — 커널 관점에서는 정상 로컬 연결이다. 로컬 리스너는 loopback 분기를 +타야 하고, 그 분기는 `Host` 헤더까지 검사한다. + +커널 바인드와 Host/Origin 게이트가 **함께** 경계다. 바인드만으로는 브라우저를 +경유한 접근을 막지 못한다. + +**3. WebSocket upgrade 는 그 요청을 받은 서버로.** 현재 Responses WS 는 클로저 +바깥의 primary `server.upgrade()` 를 부른다 (`src/server/index.ts:621`). 그대로 +공유하면 로컬 리스너가 받은 Request 를 primary 서버에서 upgrade 하려 든다. +반드시 해당 fetch 호출의 `requestServer.upgrade()` 를 쓴다. + +### 라우트 allowlist + +"data-plane 만" 은 너무 넓었다. 정확히 고정한다: + +- `POST /v1/responses` +- `/v1/responses` WebSocket upgrade +- `POST /v1/responses/compact` + +- `GET /v1/models` + +`/v1/models` 를 넣는 이유는 증거가 나왔기 때문이다. `syncCodex` 는 카탈로그 +생성이 실패하거나 소스가 없으면 경고만 남기고 `catalogPath: null` 로 +`injectCodexConfig()` 를 부른다 (`src/codex/sync.ts:129-156`). 그러면 Codex 는 +static catalog 매니저 대신 online 매니저를 고르고, app-server 의 `model/list` 가 +`GET {base_url}/models` 로 나간다. 우리가 404 를 주면 모델 목록이 낡은 채로 +남거나 번들 캐시로 떨어진다. 정확히 direct-spawn 호스트를 고치겠다면서 그 +호스트의 모델 목록을 깨뜨리는 셈이다. + +대안은 카탈로그 설치 실패 시 활성화를 fail-closed 로 막는 것인데, 카탈로그 +없음은 이미 경고로 관용되는 상태다. 그걸 이 옵션 때문에 에러로 승격시키는 건 +범위를 넘는다. + +나머지 — Chat Completions, Messages, Images, search, artifacts, Live/Realtime, +`/api/*`, GUI, health/readiness — 는 404. + +## 바인드 트랜잭션 + +config 검증으로 두 포트가 다른지 보는 것만으로는 부족하다. 로컬 포트를 다른 +프로세스가 이미 잡고 있을 수 있다. + +두 bind 를 **하나의 startup 트랜잭션**으로 다룬다. 어느 쪽이 실패하든 이미 열린 +리스너를 `await stop(true)` 로 닫고 원래 오류를 다시 던진다. 그렇지 않으면 +primary 만 살아남고, CLI 의 기존 포트 재시도가 이걸 공용 포트 충돌로 오인해 +다른 포트를 고르면서 리스너를 누적한다 (`src/cli/index.ts:234`). + +로컬 포트 충돌과 공용 포트 충돌은 구분한다. 로컬 충돌 때문에 공용 포트를 바꾸지 +않는다. + +합성 `stop()` 은 두 가지를 **동시에** 만족해야 한다. 한쪽만 하면 다른 쪽이 +깨진다. + +1. **정리는 끝까지 시도한다.** 한쪽 stop 이 실패해도 나머지 stop 과 native + lifecycle release 를 건너뛰지 않는다. +2. **실패는 호출자에게 전파한다.** `allSettled` 로 삼키면 안 된다. + +2번이 중요한 이유: 기존 `stopServerListener` 는 stop 실패를 의도적으로 +전파하고, 모든 호출자가 같은 결과를 본 뒤에야 교체 프로세스가 포트를 잡게 +되어 있다 (`src/server/lifecycle.ts:290-305`). 삼키면 `drainAndShutdown` 이 +종료 완료로 오인하고, 아직 포트를 쥔 리스너 위에 교체가 바인드를 시도한다. +정리는 다 했는데 실패는 보고되는 상태여야 하므로, 결과를 모아 하나라도 +실패했으면 `AggregateError` 로 reject 한다. + +### 이 설계가 P1 다섯 건에 어떻게 답하는가 + +| 감사 P1 | 재설계에서 | +|---|---| +| 피어 주소는 최종 신원이 아니다 | 피어 주소를 아예 판정하지 않는다. 커널 바인드 + Host/Origin 게이트가 경계다 | +| 8개 무관 엔드포인트가 같이 열린다 | 공용 리스너 정책 불변. 로컬 리스너는 4개 라우트만 노출하고 나머지는 404 | +| 새 admission kind 의 로그 파급 | `{ kind: "loopback" }` 재사용 — 이미 존재하는 kind 이고 의미도 정확하다 (인증 없는 로컬 바인드). 새 kind 없음 | +| 문자열 모양 주소 판정 | 판정 함수 자체가 없다 | +| 수용 기준이 실제 경로를 증명 못 함 | 실제 리스너를 띄우고 원격 인터페이스에서 연결 거부를 확인한다 | + +`admissionKind` 를 새로 늘리지 않는 것이 특히 크다. 감사가 지적한 대로 +`RequestLogContext`, `RequestLogEntry`, `PersistedUsageEntry` 가 전부 세 kind 로 +고정돼 있고 (`src/server/request-log.ts:52,119`, `src/usage/log.ts:56`), +`KNOWN_ADMISSION_KINDS` 가 모르는 값을 조용히 버린다 (`src/usage/log.ts:115`). +새 kind 는 타입체크를 깨거나 감사 로그에서 사라진다. + +## 변경 파일 + +- `src/types.ts` — `unauthenticatedLoopbackListener?: { enabled: false } | { enabled: true; port: number }` + (판별 유니온: 꺼져 있을 때 포트를 요구하지 않는다) +- `src/config.ts` — 스키마 + 검증 (포트 필수, 공용 포트와 동일 거부) +- `src/server/index.ts` — 두 번째 `Bun.serve`, 바인드 트랜잭션, 합성 stop, + 라우트 allowlist, 요청별 auth/origin view, `requestServer.upgrade()` +- `src/codex/inject.ts` — 켜져 있으면 `base_url` 이 로컬 리스너 포트를 가리킴 +- `src/codex/sync.ts`, `src/cli/index.ts` — 로컬 포트를 주입 경로로 전달 +- `src/cli/index.ts` — 실효 공용 포트 검증, 폴백 선택에서 로컬 포트 제외 +- `docs-site/` — 설정 문서 + 로컬 접근·비용·DoS 경고 +- `tests/` — 아래 기준 + +## 수용 기준 + +1. 설정 없음 → 리스너가 하나뿐. `0.0.0.0` 동작은 오늘과 동일 (401 유지). +2. 설정 켬 → `127.0.0.1:PORT` 로 키 없이 `POST /v1/responses` 가 admit 되고 + `{ kind: "loopback" }` 로 기록된다. 실제 WS upgrade 와 + `POST /v1/responses/compact` 도 같다. +3. 설정 켬 → 공용 리스너는 **여전히** 키를 요구한다. +4. 설정 켬 → 로컬 리스너가 비-loopback 인터페이스에 바인드되지 않는다. 머신의 + non-loopback 주소로 실제 연결을 시도해 거부를 확인한다. non-loopback + 인터페이스가 없어 skip 되면 기준 14 의 첫 ablation 이 green 이 되므로, 지원 + OS 에서는 skip 없이 돌거나 별도 결정적 보조 검사를 둔다. +5. allowlist 밖 라우트는 로컬 리스너에서 404: Chat Completions, Messages, + Images, search, artifacts, Live/Realtime, `/api/*`, GUI, health/readiness 각 + 대표 하나씩. +6. 적대적 `Host`/`Origin` (DNS rebinding 형태) 은 로컬 리스너에서도 거부된다. + 거부만이 아니라 **반환되는 CORS 헤더도** 로컬 정책 view 로 만들어졌는지 + 확인한다 — 라우팅 전 origin 판정만 보면 응답 헤더 경로의 누락을 놓친다. + 성공 응답도 확인한다: 로컬 `/v1/models` 200 응답의 CORS 헤더가 로컬 view 로 + 만들어졌는지. +7. 주입되는 `base_url` 이 설정된 로컬 포트를 가리키고, 재시작 후에도, 독립 + `ocx sync` 실행 후에도 같은 값이다. +8. 포트 필수: 활성화하면서 포트를 생략하거나 공용 포트와 같게 주면 config + 검증이 거부한다. +9. 로컬 포트를 다른 소켓이 이미 점유한 상태로 기동하면 startup 이 실패하고 + **두 포트 모두** 다시 바인드 가능한 상태로 남는다 (rollback). +10. `server.stop(true)` 와 `drainAndShutdown()` 양쪽에서 두 리스너가 모두 + 닫힌다. 한쪽 stop 이 실패해도 다른 쪽 stop 과 lifecycle release 가 실행된다. + **그리고 호출자는 reject 를 관측한다** — 정리 완주와 실패 전파 둘 다. +11. 실제 direct-spawn 수용 테스트: 격리된 `CODEX_HOME` 으로 app-server 를 + 띄우고 `model/list` 를 부르고 턴을 하나 돌려서, 요청이 로컬 리스너에 + `loopback` 으로 도달하는지 확인한다. **카탈로그 있음과 없음 두 경로 모두.** + 라우트에 POST 를 날려보는 것만으로는 리포터가 신고한 통합이 동작한다는 + 증명이 되지 않는다. + + **오라클이 없으면 이 기준은 공허하다.** Codex 의 models-manager 는 refresh + 실패를 catch 하고 기존 번들/캐시 목록을 반환한다. 그러니 `/v1/models` 를 + allowlist 에서 빼도 `model/list` 는 여전히 성공하고, 번들 모델로 턴을 + 돌리면 그것도 성공한다 — 기준이 green 인 채로 호환 경로가 깨진다. 이 + 저장소가 반복해서 데인 "통과만 하는 테스트" 의 교과서적 형태다. + + 카탈로그 없음 경로는 **오직 우리 라우트를 통해서만 알 수 있는 모델**로 + 판정한다: + + - Codex 의 번들 카탈로그와 캐시에 존재할 수 없는 고유 이름의 routed 모델을 + 구성한다. 이름은 **런타임에 생성**한다 — + `ocx-direct-spawn-${crypto.randomUUID()}` 형태. 하드코딩한 이름은 언젠가 + 누군가의 카탈로그와 충돌할 수 있고, 그 순간 오라클이 조용히 죽는다. + - `model/list` 응답에 **그 이름이 정확히** 들어 있는지 단언한다. + - 턴도 **그 모델로** 돌리고, 의도한 가짜 업스트림에 도달하는지 확인한다. + - 격리된 `CODEX_HOME` 은 `models_cache.json` 없이 시작한다. 기동 **전에** + `models_cache.json` 부재와 활성 `model_catalog_json` 부재를 단언한다 — + 전제가 깨진 채로 도는 테스트는 오라클이 아니다. + + 실행 경로도 고정한다. PATH 의 `codex` 가 아니라 resolve 된 + `@openai/codex/bin/codex.js` 를 직접 띄우고, 자식 환경에서 + `OPENCODEX_API_AUTH_TOKEN` 을 제거한다. 그러지 않으면 shim 인증 경로를 + 실수로 타면서 아무것도 증명하지 못한다. + + 증명은 둘로 나눈다. 이 저장소는 `@openai/codex` 를 테스트 의존성으로 설치하지 + 않으므로, CI 에서 결정적으로 도는 부분과 실기동 증거를 구분한다: + + - **CI 결정적:** 로컬 리스너의 `/v1/models?client_version=...` 라우트가 + 고유 모델을 반환하는지, allowlist 에서 빼면 404 가 되는지. + - **활성화 증거 (skip 금지):** 실제 지원 버전의 Codex app-server 로 위 + 시퀀스를 돌린 기록. 스킵된 채로는 이 기준을 충족한 것으로 치지 않는다. +12. 실효 공용 포트 충돌: `ocx start --port <로컬포트>`, `config.port = 0` 이 + 로컬 포트로 해석되는 경우, 그리고 선호 포트가 막혀 `findAvailablePort()` 의 + ephemeral 폴백이 로컬 포트를 고르는 경우 — 전부 startup 이 실패하는 대신 + 로컬 포트를 후보에서 제외해야 한다 (`src/cli/index.ts:146-180`). +13. 활성화 시 시작 로그에 눈에 띄는 경고가 나온다: `127.0.0.1:PORT`, 무인증 + 로컬 접근, 유료 credential 소비, 로컬 DoS 위험. +14. ablation: + - 로컬 리스너 hostname 을 `0.0.0.0` 으로 바꾸면 기준 4 가 red. + - `inject.ts` 포트 배선을 되돌리면 기준 7 이 red. + - origin 판정에 공용 config 를 넘기면 기준 6 이 red. + - rollback 을 제거하면 기준 9 가 red. + - 합성 stop 이 한쪽 reject 를 삼키게 하면 기준 10 이 red. + - `/v1/models` 를 allowlist 에서 빼면 기준 11 의 카탈로그 없음 경로가 red. + +## 상태 — 완결이 아니라 완화책 + +감사의 마지막 P1 을 그대로 받는다. 기본값이 꺼짐이므로, 리포터가 이 옵션을 +수용하기 전까지 원래 재현은 여전히 401 이다. 그래서 이 유닛은 **#1102 를 close +하지 않는다.** PR 은 `Closes` 대신 이슈를 참조하고, 리포터에게 이 옵션이 +배포에 맞는지 묻는 코멘트를 남긴다. + +--- + +## 부록 — 첫 설계의 원 분석 (기록용) + +`isApiAuthRequired()` 는 오직 바인드 hostname 만 본다: + +```ts +export function isApiAuthRequired(config: OcxConfig): boolean { + return !isLoopbackHostname(config.hostname); +} +``` + +`hostname: "0.0.0.0"` 이면 요청 피어가 `127.0.0.1` 이어도 인증을 요구한다. +그런데 우리가 Codex 에 주입하는 provider block 은 wildcard 바인드에서 +`base_url` 을 `127.0.0.1` 로 쓴다 (`tests/codex-inject.test.ts:47-54`). 즉 +우리가 만들어낸 구성이 정확히 이 상황을 만든다. + +이 진단 자체는 유효하고 재설계도 같은 사실 위에 서 있다. 다만 해법이 +"admission 을 우회" 에서 "별도 리스너" 로 바뀌었다. + +## 범위 밖 + +토큰 grace window 와 `service rotate-api-token` 은 별개 유닛이다. 리포터가 보고한 +회전 트리거는 원인이 다르다고 확인됐고 (자동 회전이 없음), operator 가 직접 값을 +바꾸고 install/repair 한 경우만 남는데 그건 이 이슈가 신고한 것이 아니다. diff --git a/docs-site/src/content/docs/reference/configuration/server.md b/docs-site/src/content/docs/reference/configuration/server.md index bc72f17b3..942af88cc 100644 --- a/docs-site/src/content/docs/reference/configuration/server.md +++ b/docs-site/src/content/docs/reference/configuration/server.md @@ -65,6 +65,44 @@ A `0.0.0.0` bind exposes the proxy and configured provider access to the LAN. Us networks with a strong token. ::: +### Local clients that cannot receive the token + +A remote bind requires a credential from every caller, including local ones. That breaks a specific +case: a `codex app-server` launched by a host process that resolves the Codex entrypoint directly +(`require.resolve('@openai/codex/bin/codex.js')`) never passes through the generated `codex` shim, +so it never inherits `OPENCODEX_API_AUTH_TOKEN` and every model call fails with `401` before a +stream opens. + +`unauthenticatedLoopbackListener` opens a second listener bound to `127.0.0.1` that admits without a +credential. The main listener is untouched — remote callers still need the token. + +```json +{ + "hostname": "0.0.0.0", + "port": 10100, + "unauthenticatedLoopbackListener": { "enabled": true, "port": 10200 } +} +``` + +`ocx sync` then writes `base_url = "http://127.0.0.1:10200/v1"` into the managed Codex provider block +and omits the auth header, so a directly spawned app-server works without any credential plumbing. + +The port is required and must differ from the proxy port. It is never OS-assigned: an ephemeral port +would change across restarts while already-running app-servers kept the previous `base_url`. + +The listener serves only `POST /v1/responses`, its WebSocket upgrade, `POST /v1/responses/compact`, +and `GET /v1/models`. Everything else, including `/api/*` and the dashboard, returns `404`. + +:::danger[This is an unauthenticated surface] +Every process on the machine can use this listener. It spends account quota and paid provider +credentials, and it can exhaust the shared turn capacity that authenticated remote clients depend +on. Do not enable it on a shared or multi-tenant host. + +Binding to `127.0.0.1` means the kernel refuses remote connections, but it does not stop a browser: +a page you visit can make your browser connect to `127.0.0.1`. The listener therefore applies the +same `Host` and `Origin` checks as an ordinary loopback bind. Off by default. +::: + ### SSH port forwarding Remote use does not require a remote bind. Keep loopback and forward it: diff --git a/scripts/verify-loopback-direct-spawn.mjs b/scripts/verify-loopback-direct-spawn.mjs new file mode 100644 index 000000000..0d48e07e1 --- /dev/null +++ b/scripts/verify-loopback-direct-spawn.mjs @@ -0,0 +1,246 @@ +#!/usr/bin/env node +/** + * Activation evidence for the unauthenticated loopback listener (#1102). + * + * The server-level tests prove admission, the route allowlist, CORS, the bind scope and the + * injected port independently. None of them prove the thing the feature exists for: that a real + * `codex app-server`, spawned the way a third-party host spawns it, reaches the proxy without a + * credential. That seam is between two processes, so no in-process test can stand in for it. + * + * This is deliberately not a `bun test` file. The repository does not depend on `@openai/codex`, + * so a test that silently skips when it is absent would be worse than no test — it would report + * green on machines that never ran it. This script fails loudly instead, and its output is the + * evidence attached to the PR. + * + * The oracle is a routed model whose id is generated at run time. Codex caches model lists and + * falls back to a bundled catalog when a refresh fails, so asking "did model/list succeed" proves + * nothing — a broken `/v1/models` looks identical to a working one. A name no bundled catalog can + * contain can only have come through our listener. + * + * Usage: node scripts/verify-loopback-direct-spawn.mjs + */ +import { spawn, spawnSync } from "node:child_process"; +import http from "node:http"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { createServer } from "node:net"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { randomUUID } from "node:crypto"; + +const UNIQUE_MODEL = `ocx-direct-spawn-${randomUUID()}`; +const steps = []; +function record(name, ok, detail) { + steps.push({ name, ok, detail }); + console.log(`${ok ? "PASS" : "FAIL"} ${name}${detail ? ` — ${detail}` : ""}`); +} + +function resolveCodexEntrypoint() { + // The resolved entrypoint, never `codex` from PATH: the whole defect is that PATH may hold the + // generated shim, which exports the token and would make this pass for the wrong reason. + const probe = spawnSync(process.execPath, [ + "-e", + "process.stdout.write(require.resolve('@openai/codex/bin/codex.js'))", + ], { encoding: "utf8" }); + if (probe.status === 0 && probe.stdout.trim()) return probe.stdout.trim(); + const which = spawnSync("readlink", ["-f", spawnSync("which", ["codex"], { encoding: "utf8" }).stdout.trim()], { encoding: "utf8" }); + const path = which.stdout.trim(); + if (!path) throw new Error("cannot resolve @openai/codex/bin/codex.js"); + return path; +} + +async function freePort() { + return await new Promise((resolve, reject) => { + const probe = createServer(); + probe.once("error", reject); + probe.once("listening", () => { + const { port } = probe.address(); + probe.close(() => resolve(port)); + }); + probe.listen({ port: 0, host: "127.0.0.1" }); + }); +} + +/** A stand-in proxy: serves the loopback listener's four routes and records what Codex asked for. */ +function startFakeProxy(port, seen) { + return new Promise(resolve => { + const srv = http.createServer((req, res) => { + seen.push(`${req.method} ${req.url}`); + if (req.url.startsWith("/v1/responses")) { + res.writeHead(500, { "content-type": "application/json" }); + res.end(JSON.stringify({ error: { message: "stub upstream" } })); + return; + } + if (req.url.startsWith("/v1/models")) { + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify({ + object: "list", + data: [{ id: UNIQUE_MODEL, object: "model", created: 0, owned_by: "opencodex" }], + })); + return; + } + res.writeHead(404, { "content-type": "application/json" }); + res.end(JSON.stringify({ error: { message: "not found" } })); + }); + srv.listen(port, "127.0.0.1", () => resolve(srv)); + }); +} + +async function main() { + const entrypoint = resolveCodexEntrypoint(); + record("resolved the real Codex entrypoint, not PATH", true, entrypoint); + + const version = spawnSync(process.execPath, [entrypoint, "--version"], { encoding: "utf8" }); + record("entrypoint runs", version.status === 0, version.stdout.trim() || version.stderr.trim()); + + const home = mkdtempSync(join(tmpdir(), "ocx-direct-spawn-")); + const codexHome = join(home, ".codex"); + const port = await freePort(); + const seen = []; + const proxy = await startFakeProxy(port, seen); + + try { + // The provider block `ocx sync` writes when the loopback listener is enabled: loopback host, + // the listener's port, and NO env_http_headers — the app-server has no token to put in one. + // + // The catalog file matters and is easy to get wrong. `model/list` reads `model_catalog_json`; + // it does not call the provider's `/v1/models`. A first version of this script omitted the + // catalog and watched Codex return its five bundled ids while never touching the listener — + // which is exactly the false-negative shape the unique-id oracle exists to expose, just + // pointed at the harness instead of the feature. + mkdirSync(codexHome, { recursive: true }); + // Build the catalog with OUR OWN serializer rather than a hand-written object. Codex rejects + // the whole file on any schema mismatch and silently falls back to its bundled list, so a + // hand-rolled fixture drifts into a false negative the moment the schema moves. Using + // `buildCatalogEntries` also means this script exercises the same bytes `ocx sync` writes. + const catalogPath = join(codexHome, "opencodex-models.json"); + const build = spawnSync("bun", ["-e", ` + const { buildCatalogEntries } = await import("./src/codex/catalog/sync.ts"); + const entries = buildCatalogEntries(null, [], [{ + provider: "opencodex", + id: ${JSON.stringify(UNIQUE_MODEL)}, + contextWindow: 128000, + }]); + process.stdout.write(JSON.stringify({ models: entries })); + `], { cwd: process.cwd(), encoding: "utf8" }); + if (build.status !== 0 || !build.stdout.trim()) { + record("built the catalog with our own serializer", false, (build.stderr || "").slice(0, 400)); + throw new Error("catalog build failed"); + } + writeFileSync(catalogPath, build.stdout, "utf-8"); + record("built the catalog with our own serializer", true, `${JSON.parse(build.stdout).models.length} entries`); + writeFileSync(join(codexHome, "config.toml"), [ + `model = "${UNIQUE_MODEL}"`, + 'model_provider = "opencodex"', + `model_catalog_json = ${JSON.stringify(catalogPath)}`, + "", + "[model_providers.opencodex]", + 'name = "OpenCodex Proxy"', + `base_url = "http://127.0.0.1:${port}/v1"`, + 'wire_api = "responses"', + "requires_openai_auth = true", + "", + ].join("\n"), "utf-8"); + record("wrote an isolated CODEX_HOME with no models_cache.json", true, codexHome); + + const env = { ...process.env, CODEX_HOME: codexHome }; + // The credential must be absent, or this would prove nothing about the shim-less path. + delete env.OPENCODEX_API_AUTH_TOKEN; + record("stripped OPENCODEX_API_AUTH_TOKEN from the child environment", true); + + const child = spawn(process.execPath, [entrypoint, "app-server"], { + env, + stdio: ["pipe", "pipe", "pipe"], + }); + + let buffered = ""; + const responses = new Map(); + child.stdout.on("data", chunk => { + buffered += chunk.toString(); + let index; + while ((index = buffered.indexOf("\n")) >= 0) { + const line = buffered.slice(0, index).trim(); + buffered = buffered.slice(index + 1); + if (!line) continue; + try { + const message = JSON.parse(line); + if (message.id !== undefined) responses.set(message.id, message); + } catch { /* notifications and logs are not our concern */ } + } + }); + const stderr = []; + child.stderr.on("data", chunk => stderr.push(chunk.toString())); + + const send = payload => child.stdin.write(`${JSON.stringify(payload)}\n`); + const await_ = async (id, timeoutMs = 30_000) => { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (responses.has(id)) return responses.get(id); + await new Promise(r => setTimeout(r, 50)); + } + return null; + }; + + send({ id: 1, method: "initialize", params: { clientInfo: { name: "ocx-verify", version: "1", title: "OpenCodex verification" } } }); + const init = await await_(1); + record("app-server initialized", !!init && !init.error, init?.error ? JSON.stringify(init.error) : "ok"); + + send({ id: 2, method: "model/list", params: {} }); + const list = await await_(2, 45_000); + const models = list?.result?.items ?? list?.result?.models ?? list?.result?.data ?? []; + const ids = models.map(m => m?.id ?? m?.model ?? m?.slug).filter(Boolean); + // Routed models are namespaced `/` in the catalog, so match on the unique + // suffix rather than a bare equality that would fail for a correct result. + const sawUnique = ids.some(id => id === UNIQUE_MODEL || id.endsWith(`/${UNIQUE_MODEL}`)); + record( + "model/list contains the unique routed model that only our listener can supply", + sawUnique, + sawUnique ? UNIQUE_MODEL : `saw ${ids.length} ids, none matching (${ids.slice(0, 6).join(", ")})`, + ); + + const hitModels = seen.some(entry => entry.includes("/v1/models")); + // `model/list` reads the catalog file, so it does NOT prove a network hop. The turn does: + // it opens `/v1/responses` against the injected base_url, and reaching our listener there + // without a credential is the whole claim of #1102. + send({ + id: 3, + method: "thread/start", + params: { cwd: home, model: UNIQUE_MODEL, provider: "opencodex" }, + }); + const started = await await_(3, 30_000); + const threadId = started?.result?.threadId ?? started?.result?.thread?.id; + record("thread/start accepted", !!threadId, threadId ? String(threadId) : JSON.stringify(started?.error ?? started).slice(0, 200)); + + if (threadId) { + send({ + id: 4, + method: "turn/start", + params: { threadId, input: [{ type: "text", text: "ping" }] }, + }); + // The upstream is a stub, so the turn is expected to FAIL. What matters is that the + // request arrived at all: a 401 at admission would never reach the handler. + await await_(4, 25_000); + } + + const hitResponses = seen.some(entry => entry.includes("/v1/responses")); + record( + "the app-server reached the loopback listener without a credential", + hitModels || hitResponses, + seen.slice(0, 8).join(" | ") || "no requests observed", + ); + + child.kill(); + if (stderr.length && !sawUnique) console.log("\nchild stderr:\n" + stderr.join("").slice(0, 2000)); + } finally { + proxy.close(); + rmSync(home, { recursive: true, force: true }); + } + + const failed = steps.filter(step => !step.ok); + console.log(`\n${steps.length - failed.length}/${steps.length} checks passed`); + process.exit(failed.length === 0 ? 0 : 1); +} + +main().catch(error => { + console.error(error); + process.exit(1); +}); diff --git a/src/cli/index.ts b/src/cli/index.ts index e9f3c22c3..ace0ee130 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -147,6 +147,18 @@ async function chooseListenPort(requestedPort?: number): Promise { const config = loadConfig(); const preferred = requestedPort ?? config.port ?? 10100; const hardPin = requestedPort !== undefined && requestedPort > 0; + const reservedLoopbackPort = config.unauthenticatedLoopbackListener?.enabled + ? config.unauthenticatedLoopbackListener.port + : undefined; + // Before the reclaim path, not after (#1102). Asking for the port the loopback listener is + // configured to bind is a configuration mistake, and reclaim would spend up to 60 seconds + // waiting for a socket to free before reporting "port is busy" — the wrong diagnosis for a + // collision the config can state outright. + if (reservedLoopbackPort !== undefined && preferred === reservedLoopbackPort) { + throw new Error( + `Port ${preferred} is reserved for unauthenticatedLoopbackListener; choose a different proxy port.`, + ); + } // Soft start: brief prefer-retry then ephemeral hop. // Explicit `--port` (service wrappers / update restart): wait for the pinned port // to free without killing any listener (healthy ocx / foreign). Never hop. @@ -170,6 +182,11 @@ async function chooseListenPort(requestedPort?: number): Promise { preferRetryMs: hardPin ? 5_000 : 750, preferRetryIntervalMs: 50, allowEphemeralFallback: !hardPin, + // Never hand the public listener the port the loopback listener is configured to + // bind (#1102). Without this, `--port ` binds the public listener + // first and the loopback bind then fails, rolling back a startup that was only + // ever a config collision. + ...(reservedLoopbackPort !== undefined ? { reservedPort: reservedLoopbackPort } : {}), }); if (preferred > 0 && selected !== preferred) { console.log(`⚠️ Port ${preferred} is busy; starting opencodex on ${selected}.`); diff --git a/src/codex/inject.ts b/src/codex/inject.ts index 62ff9f220..20d570870 100644 --- a/src/codex/inject.ts +++ b/src/codex/inject.ts @@ -189,8 +189,13 @@ export function providerBaseHost(hostname: string | undefined): string { } export function shouldInjectApiAuthHeader( - config: Pick | undefined, + config: Pick | undefined, ): boolean { + // The unauthenticated loopback listener is a loopback bind, so it admits without a + // credential (#1102). Emitting the env header anyway would be worse than useless: the + // directly-spawned app-server this exists for has no OPENCODEX_API_AUTH_TOKEN in its + // environment, and Codex would send an empty header value. + if (config?.unauthenticatedLoopbackListener?.enabled) return false; return !isLoopbackHostname(config?.hostname); } @@ -630,6 +635,17 @@ export async function injectCodexConfig( config?: OcxConfig, options: InjectCodexOptions = {}, ): Promise { + // Point Codex at the unauthenticated loopback listener when it is enabled (#1102). + // + // Resolved here rather than at the call sites because every caller already passes the proxy + // port and the config together: startup sync, `ocx sync`, and the ensure path would each + // need the same two-line change, and a caller that missed it would silently emit a base_url + // requiring a credential the directly-spawned app-server does not have. + // + // The listener port is fixed in config, never OS-assigned, so this value survives restarts + // and matches what an already-running app-server read at startup. + const loopback = config?.unauthenticatedLoopbackListener; + if (loopback?.enabled) port = loopback.port; if (!existsSync(CODEX_CONFIG_PATH)) { return { success: false, diff --git a/src/config.ts b/src/config.ts index 717b11aea..f2587d4c8 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1035,6 +1035,14 @@ const configSchema = z.object({ // is safe: startServer() already falls back to 127.0.0.1 for a missing hostname. Write-time // rejection lives in validateConfigCandidate() so bad values still surface to the caller. hostname: z.string().trim().min(1).optional().catch(undefined), + // Discriminated on `enabled` so a disabled entry cannot be forced to carry a port, and an + // enabled one cannot omit it (#1102). A malformed value degrades to undefined rather than + // failing the whole parse: this is an opt-in convenience surface, and a hand-edit typo here + // must never reset providers/apiKeys through the backup-and-defaults repair path. + unauthenticatedLoopbackListener: z.union([ + z.object({ enabled: z.literal(false) }), + z.object({ enabled: z.literal(true), port: z.number().int().min(1).max(65535) }), + ]).optional().catch(undefined), providers: z.record(z.string(), providerConfigSchema), defaultProvider: z.string().min(1).default("openai"), openaiProviderTierVersion: z.union([z.literal(1), z.literal(2)]).optional(), @@ -1974,13 +1982,52 @@ function codexAccountPickerEnabledError(value: unknown): string | null { } /** Validate an in-memory config candidate without touching disk. Used by headless CLI import/set. */ +/** + * Reject a loopback-listener port that collides with the proxy port (#1102). + * + * The schema can only check the shape of each field on its own; the two ports being distinct + * is a relationship between them. Letting the pair through would surface as a startup failure + * after the public listener already bound, which reads like an unrelated port conflict. + * + * This is write-time only, matching `blankHostnameError`: a live caller can be told the value + * is wrong, whereas a hand-edited config on the read path degrades to undefined rather than + * resetting the whole file. + */ +function loopbackListenerPortError(value: unknown): string | null { + if (!value || typeof value !== "object" || Array.isArray(value)) return null; + const listener = (value as Record).unauthenticatedLoopbackListener; + if (listener === undefined) return null; + if (!listener || typeof listener !== "object" || Array.isArray(listener)) { + return "schema_invalid: unauthenticatedLoopbackListener: must be an object or omitted"; + } + const entry = listener as Record; + // `enabled` must be a real boolean. The schema's `.catch(undefined)` would otherwise DELETE + // a `"true"` string entry and report success, leaving an operator convinced they enabled an + // unauthenticated listener that is in fact off. Load-time still degrades quietly — a hand + // edit must not reset the file — but a live caller gets told. + if (typeof entry.enabled !== "boolean") { + return "schema_invalid: unauthenticatedLoopbackListener.enabled: must be a boolean"; + } + if (entry.enabled !== true) return null; + const listenerPort = entry.port; + if (typeof listenerPort !== "number" || !Number.isInteger(listenerPort) || listenerPort < 1 || listenerPort > 65535) { + return "schema_invalid: unauthenticatedLoopbackListener.port: must be an integer port when enabled"; + } + const proxyPort = (value as Record).port; + if (typeof proxyPort === "number" && proxyPort === listenerPort) { + return "schema_invalid: unauthenticatedLoopbackListener.port: must differ from the proxy port"; + } + return null; +} + export function validateConfigCandidate(value: unknown): { ok: true; config: OcxConfig } | { ok: false; error: string } { const boundaryError = blankHostnameError(value) ?? claudeSubagentEffortError(value) ?? appOwnedMemoryBudgetError(value) ?? googleAntigravityStaticCatalogVersionError(value) ?? codexAccountPrioritiesError(value) - ?? codexAccountPickerEnabledError(value); + ?? codexAccountPickerEnabledError(value) + ?? loopbackListenerPortError(value); if (boundaryError) return { ok: false, error: boundaryError }; const result = configSchema.safeParse(value); if (result.success) return { ok: true, config: normalizeApiKeyIds(result.data as OcxConfig) }; diff --git a/src/server/auth-cors.ts b/src/server/auth-cors.ts index e439ceecb..536526f3e 100644 --- a/src/server/auth-cors.ts +++ b/src/server/auth-cors.ts @@ -73,7 +73,7 @@ export function isSameOriginAsRequest(req: Request, origin: string): boolean { } } -export function isAllowedRequestOrigin(req: Request, config: OcxConfig): boolean { +export function isAllowedRequestOrigin(req: Request, config: RequestPolicyView): boolean { const origin = req.headers.get("Origin"); if (!isApiAuthRequired(config)) { if (!isLoopbackRequestHost(req.headers.get("Host"))) return false; @@ -82,7 +82,7 @@ export function isAllowedRequestOrigin(req: Request, config: OcxConfig): boolean return !origin || isLoopbackOriginValue(origin) || isSameOriginAsRequest(req, origin) || isExtraAllowedOrigin(origin, config); } -function isExtraAllowedOrigin(origin: string, cfg: OcxConfig): boolean { +function isExtraAllowedOrigin(origin: string, cfg: RequestPolicyView): boolean { if (!cfg.corsAllowOrigins?.length) return false; const parsedOrigin = comparableOrigin(origin); return cfg.corsAllowOrigins.some(allowed => { @@ -136,7 +136,7 @@ export function browserSecurityHeaders(): Record { }; } -export function corsHeaders(req?: Request, config?: OcxConfig): Record { +export function corsHeaders(req?: Request, config?: RequestPolicyView): Record { const origin = req?.headers.get("Origin"); const allowOrigin = origin && req && config && isAllowedRequestOrigin(req, config) ? origin : _corsOrigin; return { @@ -160,7 +160,7 @@ export function managementCorsHeaders(req?: Request, config?: OcxConfig): Record return headers; } -export function withCors(response: Response, req: Request, config: OcxConfig): Response { +export function withCors(response: Response, req: Request, config: RequestPolicyView): Response { const headers = new Headers(response.headers); for (const [name, value] of Object.entries(corsHeaders(req, config))) { headers.set(name, value); @@ -184,14 +184,18 @@ export function withManagementCors(response: Response, req: Request, config: Ocx }); } -export function jsonResponse(data: unknown, status = 200, req?: Request, config?: OcxConfig): Response { +export function jsonResponse(data: unknown, status = 200, req?: Request, config?: RequestPolicyView): Response { return new Response(JSON.stringify(data), { status, headers: { "Content-Type": "application/json", ...corsHeaders(req, config) }, }); } -export function configuredApiAuthToken(_config: OcxConfig): string | undefined { +// The parameter is vestigial — the token has always come from the environment — but callers +// pass a config, so keep accepting one. Typed as `unknown` rather than `OcxConfig` so a narrow +// policy view can reach it too (#1102); widening to OcxConfig here would force every caller in +// the admission path back to the full config. +export function configuredApiAuthToken(_config?: unknown): string | undefined { const token = process.env.OPENCODEX_API_AUTH_TOKEN?.trim(); return token || undefined; } @@ -208,10 +212,37 @@ export function isLoopbackHostname(hostname: string | undefined): boolean { return normalized === "" || normalized === "localhost" || normalized === "127.0.0.1" || normalized === "::1" || normalized === "[::1]"; } -export function isApiAuthRequired(config: OcxConfig): boolean { +export function isApiAuthRequired(config: Pick): boolean { return !isLoopbackHostname(config.hostname); } +/** + * The slice of config that decides admission and CORS, and nothing else (#1102). + * + * The unauthenticated loopback listener shares this process with the public one: same routing, + * same account pool, same drain. The only thing it must see differently is its own bind + * address, because `isApiAuthRequired` reads `hostname` and the shared config says "0.0.0.0". + * + * Two ways to express that were rejected. Passing the whole config with `hostname` rewritten + * and holding it for the listener's lifetime would go stale the moment the management API + * changes a setting. Adding an `allowUnauthenticated` parameter to the resolvers would create a + * callable admission bypass that the PUBLIC listener could also reach — the switch would exist + * on the wrong side of the boundary. + * + * So this type is deliberately narrow: it cannot masquerade as a business config, and a policy + * view that leaks into a routing path fails to typecheck rather than silently taking effect. + */ +export type RequestPolicyView = Pick; + +/** Derive the per-request policy view for a listener. Cheap enough to build per request. */ +export function requestPolicyView(config: OcxConfig, bindHostname: string): RequestPolicyView { + return { + hostname: bindHostname, + ...(config.corsAllowOrigins ? { corsAllowOrigins: config.corsAllowOrigins } : {}), + ...(config.apiKeys ? { apiKeys: config.apiKeys } : {}), + }; +} + export function assertServerAuthConfig(config: OcxConfig): void { const hasConfiguredDataCredential = !!configuredApiAuthToken(config) || (config.apiKeys ?? []).some(entry => !!entry.key.trim()); @@ -253,7 +284,7 @@ export type DataPlaneAdmission = * discarded, which is what makes per-key attribution possible without touching * the admission decision itself. */ -export function resolveDataPlaneAdmissionSecret(token: string, config: OcxConfig): DataPlaneAdmission | null { +export function resolveDataPlaneAdmissionSecret(token: string, config: Pick): DataPlaneAdmission | null { const actual = token.trim(); if (!actual) return null; if (secretEquals(actual, configuredApiAuthToken(config))) return { kind: "environment" }; @@ -341,7 +372,7 @@ export function validateForwardAdmissionCredential(headers: Headers, config: Ocx * Resolving form of `hasValidApiAuth`: identical header precedence, identical * decision, but it names the admission instead of collapsing it to a boolean. */ -export function resolveApiAuth(req: Request, config: OcxConfig): DataPlaneAdmission | null { +export function resolveApiAuth(req: Request, config: RequestPolicyView): DataPlaneAdmission | null { // A loopback bind never reads a token at all, so there is no key to name. if (!isApiAuthRequired(config)) return { kind: "loopback" }; const actual = req.headers.get("x-opencodex-api-key")?.trim() @@ -352,11 +383,11 @@ export function resolveApiAuth(req: Request, config: OcxConfig): DataPlaneAdmiss return resolveDataPlaneAdmissionSecret(actual, config); } -export function hasValidApiAuth(req: Request, config: OcxConfig): boolean { +export function hasValidApiAuth(req: Request, config: RequestPolicyView): boolean { return resolveApiAuth(req, config) !== null; } -export function requireApiAuth(req: Request, config: OcxConfig, _kind: "data-plane"): Response | null { +export function requireApiAuth(req: Request, config: RequestPolicyView, _kind: "data-plane"): Response | null { if (hasValidApiAuth(req, config)) return null; return formatErrorResponse(401, "authentication_error", "opencodex API key required"); } @@ -366,7 +397,7 @@ export function requireApiAuth(req: Request, config: OcxConfig, _kind: "data-pla * Codex Direct. Remote binds must use the dedicated proxy header so the two bearer * domains can never be confused. */ -export function resolveResponsesApiAuth(req: Request, config: OcxConfig): DataPlaneAdmission | null { +export function resolveResponsesApiAuth(req: Request, config: RequestPolicyView): DataPlaneAdmission | null { if (!isApiAuthRequired(config)) return { kind: "loopback" }; // Dedicated header ONLY. `Authorization` on these transports may belong to // Codex Direct passthrough, and the two bearer domains must stay unconfusable. @@ -375,7 +406,7 @@ export function resolveResponsesApiAuth(req: Request, config: OcxConfig): DataPl return resolveDataPlaneAdmissionSecret(actual, config); } -export function requireResponsesApiAuth(req: Request, config: OcxConfig): Response | null { +export function requireResponsesApiAuth(req: Request, config: RequestPolicyView): Response | null { if (resolveResponsesApiAuth(req, config)) return null; return formatErrorResponse(401, "authentication_error", "opencodex API key required"); } diff --git a/src/server/index.ts b/src/server/index.ts index b25f4ce67..07a900e1b 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -71,6 +71,7 @@ import { getActiveTurnCount, isDraining, registerTurn, + runListenerShutdown, setServerRef, trackStreamLifetime, tryAdmitTurn, @@ -138,6 +139,8 @@ import { admissionFields, resolveApiAuth, resolveResponsesApiAuth, + requestPolicyView, + type RequestPolicyView, safeConfigDTO, setCorsOrigin, withCors, @@ -492,30 +495,82 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server config; + const loopbackPolicy = (): RequestPolicyView => requestPolicyView(config, "127.0.0.1"); + void publicPolicy; + + /** + * Routes the unauthenticated loopback listener will serve. Everything else 404s. + * + * This is an allowlist rather than a filter applied to the public handler, because a filter + * inverts the failure mode: a route added later would be reachable here by default. The four + * entries are exactly what a directly-spawned `codex app-server` needs. + * + * `GET /v1/models` is on the list for a reason that is easy to miss. When catalog + * materialization fails or finds no source, `syncCodex` warns and injects with + * `catalogPath: null`; Codex then builds an ONLINE model manager and `model/list` refreshes + * through `GET {base_url}/models`. Returning 404 there would leave the picker on its bundled + * fallback — fixing the direct-spawn host while breaking its model list. + */ + function loopbackRouteAllowed(url: URL, req: Request): boolean { + const path = url.pathname; + if (path === "/v1/responses") { + return req.method === "POST" || req.headers.get("upgrade")?.toLowerCase() === "websocket"; + } + if (path === "/v1/responses/compact") return req.method === "POST"; + if (path === "/v1/models") return req.method === "GET"; + return false; + } + // Codex treats empty / non-JSON 503 bodies as "Unknown error" (#452). Keep Retry-After and // the server_is_overloaded code so clients can back off, but always return a JSON envelope. - function drainingResponse(req: Request): Response { + // These two run BEFORE the auth/origin checks, so they need the receiving listener's policy + // explicitly (#1102). Reaching for the shared `config` here would attach public-policy CORS + // headers to a 503 on the loopback listener — no model runs and no credential is spent, but + // it is the one error path that would answer a rebinding origin with its own origin echoed + // back. + function drainingResponse(req: Request, policy: RequestPolicyView): Response { const response = formatErrorResponse(503, "server_error", "Service shutting down"); const headers = new Headers(response.headers); - for (const [name, value] of Object.entries(corsHeaders(req, config))) { + for (const [name, value] of Object.entries(corsHeaders(req, policy))) { headers.set(name, value); } headers.set("Retry-After", "5"); return new Response(response.body, { status: 503, headers }); } - function serverBusyResponse(req: Request, resource: string): Response { + function serverBusyResponse(req: Request, resource: string, policy: RequestPolicyView): Response { return withCors(new Response(JSON.stringify({ error: { type: "server_error", code: "server_busy", message: `${resource} capacity reached` }, }), { status: 503, headers: { "Content-Type": "application/json", "Retry-After": "1" }, - }), req, config); + }), req, policy); } - async function runAdmittedHttpTurn(req: Request, work: (lease: ActiveTurnLease) => Promise): Promise { + async function runAdmittedHttpTurn( + req: Request, + policy: RequestPolicyView, + work: (lease: ActiveTurnLease) => Promise, + ): Promise { const lease = tryAdmitTurn(); - if (!lease) return serverBusyResponse(req, "active turns"); + if (!lease) return serverBusyResponse(req, "active turns", policy); let response: Response; try { response = await work(lease); @@ -554,12 +609,27 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server {}, }; let server: Server; + let loopbackServer: Server | null = null; try { - server = Bun.serve({ - port: listenPort, - hostname: bindHost, + const serveOptions = { idleTimeout: 255, - async fetch(req, requestServer): Promise { + async fetch(req: Request, requestServer: Server): Promise { + // The unauthenticated loopback listener (#1102) serves a fixed allowlist and nothing + // else. Rejecting here, before any handler runs, is what keeps the surface from growing + // silently when a route is added below. + if (requestServer === loopbackServer && !loopbackRouteAllowed(new URL(req.url), req)) { + return withCors( + formatErrorResponse(404, "not_found", `Unknown endpoint: ${req.method} ${new URL(req.url).pathname}`), + req, + loopbackPolicy(), + ); + } + // Auth and CORS decisions below read `policy`, not `config`. For the public listener the + // two are the same object, so its behaviour is unchanged; for the loopback listener the + // view substitutes 127.0.0.1 as the bind address, which is what routes it through the + // same code path a plain loopback bind has always taken — Host-header check included. + // Routing, provider selection and response bodies keep using `config`. + const policy: RequestPolicyView = requestServer === loopbackServer ? loopbackPolicy() : config; const url = new URL(req.url); markActivity(`${req.method} ${url.pathname}`); @@ -580,18 +650,18 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server 0, ), - }, 200, req, config); + }, 200, req, policy); } // OpenAI list shape: native gpt bare + routed models namespaced "/" // (pure availability list — disabled natives are omitted entirely). @@ -835,7 +909,7 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server { + return runAdmittedHttpTurn(req, policy, async turnAdmissionLease => { let response: Response; try { response = await handleResponsesCompact(req, config, logCtx, turnAdmissionLease); @@ -867,7 +941,7 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server { + return runAdmittedHttpTurn(req, policy, async turnAdmissionLease => { const response = await handleImages(req, config, endpoint, logCtx, turnAdmissionLease); addFinalRequestLog(requestId, start, logCtx, response.status, response.status === 499 ? { closeReason: "client_cancel" } : undefined); - return withCors(response, req, config); + return withCors(response, req, policy); }); } if (req.method === "GET" && url.pathname.startsWith("/v1/opencodex/artifacts/")) { - const admission = resolveApiAuth(req, config); - if (!admission) return withCors(formatErrorResponse(401, "authentication_error", "opencodex API key required"), req, config); - if (!isAllowedRequestOrigin(req, config)) { - return withCors(formatErrorResponse(403, "origin_rejected", "cross-origin data-plane request blocked"), req, config); + const admission = resolveApiAuth(req, policy); + if (!admission) return withCors(formatErrorResponse(401, "authentication_error", "opencodex API key required"), req, policy); + if (!isAllowedRequestOrigin(req, policy)) { + return withCors(formatErrorResponse(403, "origin_rejected", "cross-origin data-plane request blocked"), req, policy); } const id = decodeURIComponent(url.pathname.slice("/v1/opencodex/artifacts/".length)); const { resolveArtifactPath } = await import("../images/artifacts"); const artifactPath = resolveArtifactPath(id); if (!artifactPath) { - return withCors(formatErrorResponse(404, "not_found", "artifact not found"), req, config); + return withCors(formatErrorResponse(404, "not_found", "artifact not found"), req, policy); } const file = Bun.file(artifactPath); const ext = artifactPath.split(".").pop()?.toLowerCase(); @@ -926,18 +1000,18 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server { + return runAdmittedHttpTurn(req, policy, async turnAdmissionLease => { const response = await handleSearch(req, config, logCtx, turnAdmissionLease); addFinalRequestLog(requestId, start, logCtx, response.status, response.status === 499 ? { closeReason: "client_cancel" } : undefined); - return withCors(response, req, config); + return withCors(response, req, policy); }); } if (url.pathname === "/v1/responses" && req.method === "POST") { disableResponsesRequestTimeout(req, requestServer); if (isDraining()) { - return drainingResponse(req); + return drainingResponse(req, policy); } - const admission = resolveResponsesApiAuth(req, config); - if (!admission) return withCors(formatErrorResponse(401, "authentication_error", "opencodex API key required"), req, config); - if (!isAllowedRequestOrigin(req, config)) { - return withCors(formatErrorResponse(403, "origin_rejected", "cross-origin data-plane request blocked"), req, config); + const admission = resolveResponsesApiAuth(req, policy); + if (!admission) return withCors(formatErrorResponse(401, "authentication_error", "opencodex API key required"), req, policy); + if (!isAllowedRequestOrigin(req, policy)) { + return withCors(formatErrorResponse(403, "origin_rejected", "cross-origin data-plane request blocked"), req, policy); } const start = Date.now(); const requestId = nextRequestLogId(start); @@ -981,7 +1055,7 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server { + return runAdmittedHttpTurn(req, policy, async turnAdmissionLease => { const response = await handleResponses(req, config, logCtx, { turnAdmissionLease, abortSignal: req.signal, @@ -996,7 +1070,7 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server withCors(await handleClaudeCountTokens(req, config), req, config)); + return runAdmittedHttpTurn(req, policy, async () => withCors(await handleClaudeCountTokens(req, config), req, policy)); } if (url.pathname === "/v1/messages" && req.method === "POST") { disableResponsesRequestTimeout(req, requestServer); if (isDraining()) { - return drainingResponse(req); + return drainingResponse(req, policy); } - const admission = resolveApiAuth(req, config); + const admission = resolveApiAuth(req, policy); if (!admission) { - return withCors(anthropicErrorResponse(401, "opencodex API key required", "authentication_error"), req, config); + return withCors(anthropicErrorResponse(401, "opencodex API key required", "authentication_error"), req, policy); } - if (!isAllowedRequestOrigin(req, config)) { - return withCors(anthropicErrorResponse(403, "cross-origin data-plane request blocked", "permission_error"), req, config); + if (!isAllowedRequestOrigin(req, policy)) { + return withCors(anthropicErrorResponse(403, "cross-origin data-plane request blocked", "permission_error"), req, policy); } const start = Date.now(); const requestId = nextRequestLogId(start); @@ -1039,7 +1113,7 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server withCors( + return runAdmittedHttpTurn(req, policy, async turnAdmissionLease => withCors( await handleClaudeMessages(req, config, logCtx, { requestId, start, turnAdmissionLease }), req, config, @@ -1051,12 +1125,12 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server withCors( + return runAdmittedHttpTurn(req, policy, async turnAdmissionLease => withCors( await handleChatCompletions(req, config, logCtx, { requestId, start, turnAdmissionLease }), req, config, @@ -1082,12 +1156,12 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server { + return runAdmittedHttpTurn(req, policy, async turnAdmissionLease => { const response = await handleLive(req, config, logCtx, turnAdmissionLease); addFinalRequestLog( requestId, @@ -1105,7 +1179,7 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server({ ...serveOptions, port: listenPort, hostname: bindHost }); + + // Both binds are one startup transaction (#1102). If the loopback bind fails after the + // public one succeeded, leaving the public listener up would strand it: the CLI's port + // retry would read the failure as a public-port conflict and pick a different port, + // accumulating listeners. Roll back and rethrow the original error instead. + if (loopbackListenerPort !== null) { + try { + loopbackServer = Bun.serve({ + ...serveOptions, + port: loopbackListenerPort, + hostname: "127.0.0.1", + }); + } catch (error) { + try { + // startServer is synchronous, so this rollback cannot await. Bun begins closing the + // listen socket on the call itself; the caller sees the original bind error either + // way, and the alternative — leaving the public listener up — is the failure this + // rollback exists to prevent. + void server.stop(true); + } catch { + /* the original bind error is the one worth reporting */ + } + throw error; + } + } } catch (error) { void nativeMainLifecycle.release(); throw error; @@ -1393,14 +1494,21 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server => { - try { - await nativeStop(closeActiveConnections); - } finally { - await releaseNativeMainStartupLifecycle(server); - } + // The orchestration lives in `runListenerShutdown` so its two competing properties — + // cleanup completes, failure propagates — are testable without a live socket. + await runListenerShutdown( + [ + () => nativeStop(closeActiveConnections), + ...(loopbackListenerRef + ? [() => loopbackListenerRef.stop(closeActiveConnections)] + : []), + ], + () => releaseNativeMainStartupLifecycle(server), + ); }, }); setServerRef(server); @@ -1415,6 +1523,17 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server Promise>, + always: () => Promise, +): Promise { + const failures: unknown[] = []; + for (const step of steps) { + try { + await step(); + } catch (error) { + failures.push(error); + } + } + try { + await always(); + } catch (error) { + failures.push(error); + } + if (failures.length === 1) throw failures[0]; + if (failures.length > 1) throw new AggregateError(failures, "listener shutdown failed"); +} + export function stopServerListener( server: ReturnType | undefined = _serverRef, ): Promise { diff --git a/src/server/ports.ts b/src/server/ports.ts index 88bf97d53..ae8fe16c1 100644 --- a/src/server/ports.ts +++ b/src/server/ports.ts @@ -57,6 +57,16 @@ export type FindAvailablePortOptions = { * update restart cannot hop to a random ephemeral listener (PR #152 gap). */ allowEphemeralFallback?: boolean; + /** + * A port this selection must never return, even when it is free (#1102). + * + * The unauthenticated loopback listener binds a fixed port from config. If the public + * listener took that port first — via an explicit `--port`, a `config.port` of 0, or the + * ephemeral fallback happening to land on it — the loopback bind would then fail with + * EADDRINUSE, and the startup transaction would roll back a public listener that had + * nothing wrong with it. Excluding the port here fails the right thing at the right time. + */ + reservedPort?: number; }; export class PortUnavailableError extends Error { @@ -75,6 +85,12 @@ export async function findAvailablePort( ): Promise { const preferRetryMs = opts.preferRetryMs ?? 0; const allowEphemeral = opts.allowEphemeralFallback !== false; + const reserved = opts.reservedPort; + // An explicit preference for the reserved port is a configuration mistake, not a busy + // socket: retrying or hopping would hide it. Refuse before probing anything. + if (reserved !== undefined && preferredPort === reserved) { + throw new PortUnavailableError(preferredPort, hostname); + } // Port 0 asks the OS to select an ephemeral port. Resolve it to that concrete // port here so callers never persist or advertise an unusable `:0` endpoint. if (preferredPort > 0 && preferRetryMs > 0) { @@ -92,7 +108,31 @@ export async function findAvailablePort( throw new PortUnavailableError(preferredPort, hostname); } - return await new Promise((resolve, reject) => { + // Bounded, not recursive. The OS can hand back the reserved port, and a redraw practically + // always differs — but "practically always" is not a termination argument, and an unbounded + // async recursion has no way to stop if the assumption is ever wrong. + for (let attempt = 0; attempt < EPHEMERAL_REDRAW_LIMIT; attempt += 1) { + const port = await allocateEphemeralPort(hostname); + if (port !== reserved) return port; + } + throw new Error("failed to allocate an available port"); +} + +/** How many times an ephemeral draw may come back reserved before we give up. */ +const EPHEMERAL_REDRAW_LIMIT = 8; + +/** Test seam: replace the OS ephemeral allocator so the redraw path is reachable. */ +let ephemeralAllocator: ((hostname: string) => Promise) | null = null; + +export function setEphemeralPortAllocatorForTests( + allocator: ((hostname: string) => Promise) | null, +): void { + ephemeralAllocator = allocator; +} + +async function allocateEphemeralPort(hostname: string): Promise { + if (ephemeralAllocator) return ephemeralAllocator(hostname); + return await new Promise((resolve, reject) => { const server = createServer(); server.once("error", reject); server.once("listening", () => { diff --git a/src/types.ts b/src/types.ts index 7e1aff442..526056e25 100644 --- a/src/types.ts +++ b/src/types.ts @@ -728,6 +728,30 @@ export interface OcxConfig { contextCapValue?: number; /** Bind hostname. Default "127.0.0.1" (loopback only). Set "0.0.0.0" to expose on all interfaces. */ hostname?: string; + /** + * Optional second listener bound to 127.0.0.1 that admits data-plane requests without a + * credential (issue #1102). + * + * Why a separate listener rather than an exemption on the main one: when `hostname` is a + * wildcard, every caller needs `x-opencodex-api-key`, but a `codex app-server` spawned + * directly from the resolved entrypoint never goes through the generated shim and so never + * inherits the token. Exempting "loopback-looking peers" on the public listener would be + * unsound — `requestIP()` only proves the last transport hop, and Docker Desktop port + * forwarding, host-network containers, WSL mirrored networking and tunnels all terminate + * remote connections locally. Binding a second socket to 127.0.0.1 makes the kernel refuse + * remote connections outright, so there is no address to judge. + * + * The public listener's admission policy is unchanged. This adds an explicit local trust + * surface: every process on the machine can reach it, spend account quota, and consume paid + * provider credentials. Off by default; not for multi-tenant hosts. + * + * The port is required when enabled and must differ from the proxy port. An OS-assigned port + * would change across restarts, which would break already-running app-servers holding the + * previous `base_url` — the exact symptom #1102 reported and we disproved for token rotation. + */ + unauthenticatedLoopbackListener?: + | { enabled: false } + | { enabled: true; port: number }; /** * Outbound HTTP(S) proxy URL for provider requests (e.g. "http://user:pass@proxy:8080", or * "${HTTPS_PROXY}"-style env reference). Mirrored into HTTP_PROXY/HTTPS_PROXY at startup when diff --git a/tests/loopback-listener-admission.test.ts b/tests/loopback-listener-admission.test.ts new file mode 100644 index 000000000..2ecd269d4 --- /dev/null +++ b/tests/loopback-listener-admission.test.ts @@ -0,0 +1,177 @@ +/** + * Tests for the unauthenticated loopback listener (#1102). + * + * The defect: with `hostname: "0.0.0.0"`, every caller needs `x-opencodex-api-key`, but a + * `codex app-server` spawned from the resolved entrypoint never goes through the generated + * shim and so never inherits the token. Every model call 401s at admission. + * + * The fix is deliberately NOT an exemption on the public listener. `requestIP()` only proves + * the last transport hop, and Docker port forwarding, host-network containers, WSL mirrored + * networking and tunnels all terminate remote connections locally — a peer that "looks + * loopback" is not evidence of a local caller. Instead a second socket binds 127.0.0.1, so the + * kernel refuses remote connections and there is no address to judge. + */ +import { describe, expect, test } from "bun:test"; +import { + isAllowedRequestOrigin, + requestPolicyView, + resolveResponsesApiAuth, +} from "../src/server/auth-cors"; +import { buildProviderTableBlock, shouldInjectApiAuthHeader } from "../src/codex/inject"; +import { validateConfigCandidate } from "../src/config"; +import type { OcxConfig } from "../src/types"; + +const wildcardConfig = { + hostname: "0.0.0.0", + apiKeys: [{ id: "k1", key: "ocx_data_realsecret", name: "test" }], +} as unknown as OcxConfig; + +function request(path = "/v1/responses", headers: Record = {}): Request { + return new Request(`http://127.0.0.1:10200${path}`, { headers }); +} + +describe("loopback listener policy view", () => { + test("the public listener still demands a credential on a wildcard bind", () => { + // The whole point of the separate listener is that this does not change. + expect(resolveResponsesApiAuth(request(), wildcardConfig)).toBeNull(); + }); + + test("the loopback view admits without a credential and names it loopback", () => { + const policy = requestPolicyView(wildcardConfig, "127.0.0.1"); + expect(resolveResponsesApiAuth(request(), policy)).toEqual({ kind: "loopback" }); + }); + + test("the view carries no bind address other than the one it was given", () => { + // A view built from a wildcard config must not leak that wildcard back into an auth + // decision — that would silently restore the 401 the listener exists to avoid. + const policy = requestPolicyView(wildcardConfig, "127.0.0.1"); + expect(policy.hostname).toBe("127.0.0.1"); + }); + + test("a valid configured key is still attributed to that key, not collapsed to loopback", () => { + // The loopback view takes the same branch a plain loopback bind always has, which returns + // before reading any header. Assert the public listener keeps per-key attribution so a + // future refactor cannot quietly make every admission anonymous. + expect(resolveResponsesApiAuth( + request("/v1/responses", { "x-opencodex-api-key": "ocx_data_realsecret" }), + wildcardConfig, + )).toEqual({ kind: "configured", keyId: "k1" }); + }); +}); + +describe("loopback listener origin gate", () => { + // The kernel bind stops remote TCP, but not a victim browser: an attacker page can make the + // browser connect to 127.0.0.1, and that connection IS local. The Host/Origin gate is the + // other half of the boundary, and the loopback view must route through it. + test("a hostile Host is rejected under the loopback policy", () => { + const policy = requestPolicyView(wildcardConfig, "127.0.0.1"); + expect(isAllowedRequestOrigin( + request("/v1/responses", { Host: "attacker.example" }), + policy, + )).toBe(false); + }); + + test("a hostile Origin is rejected even when the Host looks local", () => { + const policy = requestPolicyView(wildcardConfig, "127.0.0.1"); + expect(isAllowedRequestOrigin( + request("/v1/responses", { Host: "127.0.0.1:10200", Origin: "http://attacker.example" }), + policy, + )).toBe(false); + }); + + test("the same hostile Origin would pass under the PUBLIC policy via same-origin", () => { + // This is why the view matters. On a remote bind `isAllowedRequestOrigin` accepts a + // same-origin request, so handing the public config to the loopback listener's origin + // check would admit exactly the DNS-rebinding shape the test above rejects. + const sameOrigin = new Request("http://attacker.example/v1/responses", { + headers: { Origin: "http://attacker.example" }, + }); + expect(isAllowedRequestOrigin(sameOrigin, wildcardConfig)).toBe(true); + }); + + test("an ordinary local request is allowed", () => { + const policy = requestPolicyView(wildcardConfig, "127.0.0.1"); + expect(isAllowedRequestOrigin(request("/v1/responses", { Host: "127.0.0.1:10200" }), policy)).toBe(true); + }); +}); + +describe("loopback listener configuration", () => { + test("an enabled listener sharing the proxy port is rejected at write time", () => { + // A collision would otherwise surface as a startup failure after the public listener had + // already bound, which reads like an unrelated port conflict. + const result = validateConfigCandidate({ + port: 10100, + providers: { openai: { adapter: "openai", baseUrl: "https://chatgpt.com/backend-api/codex" } }, + defaultProvider: "openai", + unauthenticatedLoopbackListener: { enabled: true, port: 10100 }, + }); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.error).toContain("must differ from the proxy port"); + }); + + test("an enabled listener without a port is rejected", () => { + // An OS-assigned port would change across restarts and strand app-servers holding the + // previous base_url — the symptom #1102 reported and we disproved for token rotation. + const result = validateConfigCandidate({ + port: 10100, + providers: { openai: { adapter: "openai", baseUrl: "https://chatgpt.com/backend-api/codex" } }, + defaultProvider: "openai", + unauthenticatedLoopbackListener: { enabled: true }, + }); + expect(result.ok).toBe(false); + }); + + test("a disabled listener needs no port", () => { + const result = validateConfigCandidate({ + port: 10100, + providers: { openai: { adapter: "openai", baseUrl: "https://chatgpt.com/backend-api/codex" } }, + defaultProvider: "openai", + unauthenticatedLoopbackListener: { enabled: false }, + }); + expect(result.ok).toBe(true); + }); + + test("a distinct port is accepted and survives the parse", () => { + const result = validateConfigCandidate({ + port: 10100, + providers: { openai: { adapter: "openai", baseUrl: "https://chatgpt.com/backend-api/codex" } }, + defaultProvider: "openai", + unauthenticatedLoopbackListener: { enabled: true, port: 10200 }, + }); + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.config.unauthenticatedLoopbackListener).toEqual({ enabled: true, port: 10200 }); + } + }); +}); + +describe("injected Codex provider block", () => { + test("a wildcard bind alone still emits the env auth header", () => { + expect(shouldInjectApiAuthHeader({ hostname: "0.0.0.0" })).toBe(true); + }); + + test("enabling the loopback listener drops the header", () => { + // The directly-spawned app-server has no OPENCODEX_API_AUTH_TOKEN, so emitting the header + // would make Codex send an empty value rather than authenticate. + expect(shouldInjectApiAuthHeader({ + hostname: "0.0.0.0", + unauthenticatedLoopbackListener: { enabled: true, port: 10200 }, + })).toBe(false); + }); + + test("a disabled listener leaves the wildcard behaviour intact", () => { + expect(shouldInjectApiAuthHeader({ + hostname: "0.0.0.0", + unauthenticatedLoopbackListener: { enabled: false }, + })).toBe(true); + }); + + test("the emitted block points at the loopback port and carries no auth header", () => { + // shouldInjectApiAuthHeader alone does not prove the injected TOML is usable. Assert the + // rendered block, because that is what a directly spawned app-server actually reads: a + // base_url on the public port, or an env header it cannot populate, both reproduce #1102. + const block = buildProviderTableBlock(10200, false, false, "0.0.0.0"); + expect(block).toContain('base_url = "http://127.0.0.1:10200/v1"'); + expect(block).not.toContain("env_http_headers"); + }); +}); diff --git a/tests/loopback-listener-integration.test.ts b/tests/loopback-listener-integration.test.ts new file mode 100644 index 000000000..25419c593 --- /dev/null +++ b/tests/loopback-listener-integration.test.ts @@ -0,0 +1,483 @@ +/** + * Integration coverage for the unauthenticated loopback listener (#1102). + * + * The companion unit file exercises the admission and CORS helpers in isolation. That is not + * enough for a surface that admits without a credential: helper-level tests stay green if the + * second listener never opens, binds the wrong address, is not distinguished from the public + * one, or serves routes outside its allowlist. These tests start real servers and speak HTTP + * to them, so those regressions have somewhere to fail. + */ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import { connect } from "node:net"; +import { networkInterfaces, tmpdir } from "node:os"; +import { join } from "node:path"; +import { saveConfig } from "../src/config"; +import { startServer } from "../src/server"; +import { runListenerShutdown } from "../src/server/lifecycle"; +import { + findAvailablePort, + PortUnavailableError, + setEphemeralPortAllocatorForTests, +} from "../src/server/ports"; +import type { OcxConfig } from "../src/types"; + +const previousApiToken = process.env.OPENCODEX_API_AUTH_TOKEN; +const previousHome = process.env.OPENCODEX_HOME; +let testDir = ""; + +function baseConfig(loopbackPort: number | null): OcxConfig { + return { + port: 0, + hostname: "0.0.0.0", + defaultProvider: "chatgpt", + providers: { + chatgpt: { + adapter: "openai-responses", + baseUrl: "https://chatgpt.com/backend-api/codex", + authMode: "forward", + }, + }, + ...(loopbackPort === null + ? {} + : { unauthenticatedLoopbackListener: { enabled: true, port: loopbackPort } }), + } as unknown as OcxConfig; +} + +/** A free port to hand the loopback listener, chosen the same way production would not reuse. */ +async function freePort(): Promise { + return await findAvailablePort(0, "127.0.0.1"); +} + +function firstNonLoopbackIPv4(): string | null { + for (const entries of Object.values(networkInterfaces())) { + for (const entry of entries ?? []) { + if (entry.family === "IPv4" && !entry.internal) return entry.address; + } + } + return null; +} + +/** One-shot settle with a cleared timer, so a late timeout cannot fire into the next test. */ +function handshake(url: string): Promise { + return new Promise(resolve => { + const ws = new WebSocket(url); + let settled = false; + let timer: ReturnType | undefined; + const settle = (value: boolean) => { + if (settled) return; + settled = true; + if (timer) clearTimeout(timer); + try { ws.close(); } catch { /* already closing */ } + resolve(value); + }; + ws.addEventListener("open", () => settle(true)); + ws.addEventListener("error", () => settle(false)); + timer = setTimeout(() => settle(false), 3_000); + }); +} + +beforeEach(() => { + testDir = mkdtempSync(join(tmpdir(), "ocx-loopback-listener-")); + process.env.OPENCODEX_HOME = testDir; + process.env.OPENCODEX_API_AUTH_TOKEN = "public-secret"; +}); + +afterEach(() => { + if (previousApiToken === undefined) delete process.env.OPENCODEX_API_AUTH_TOKEN; + else process.env.OPENCODEX_API_AUTH_TOKEN = previousApiToken; + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + if (testDir && existsSync(testDir)) rmSync(testDir, { recursive: true, force: true }); + testDir = ""; +}); + +describe("unauthenticated loopback listener", () => { + test("is absent unless configured, and the public listener still demands a key", async () => { + saveConfig(baseConfig(null)); + const server = startServer(0); + try { + const res = await fetch(`http://127.0.0.1:${server.port}/v1/models`); + expect(res.status).toBe(401); + } finally { + await server.stop(true); + } + }); + + test("admits without a credential while the public listener does not", async () => { + const loopbackPort = await freePort(); + saveConfig(baseConfig(loopbackPort)); + const server = startServer(0); + try { + // Same request, two sockets, two answers. This is the whole feature. + const viaPublic = await fetch(`http://127.0.0.1:${server.port}/v1/models`); + expect(viaPublic.status).toBe(401); + + const viaLoopback = await fetch(`http://127.0.0.1:${loopbackPort}/v1/models`); + expect(viaLoopback.status).toBe(200); + } finally { + await server.stop(true); + } + }); + + test("refuses connections on a non-loopback interface", async () => { + const address = firstNonLoopbackIPv4(); + if (!address) { + // A host with no external IPv4 cannot prove this. Say so rather than pass silently: + // a quiet skip here would let the bind address regress unnoticed on that machine. + console.warn("[loopback-listener] no non-loopback IPv4 interface; bind-scope check not run"); + return; + } + const loopbackPort = await freePort(); + saveConfig(baseConfig(loopbackPort)); + const server = startServer(0); + try { + const refused = await new Promise(resolve => { + const socket = connect({ host: address, port: loopbackPort }); + const settle = (value: boolean) => { + socket.destroy(); + resolve(value); + }; + socket.setTimeout(2_000); + socket.once("connect", () => settle(false)); + socket.once("error", () => settle(true)); + socket.once("timeout", () => settle(true)); + }); + expect(refused).toBe(true); + } finally { + await server.stop(true); + } + }); + + test("serves only the four allowlisted routes, using each route's real method", async () => { + const loopbackPort = await freePort(); + saveConfig(baseConfig(loopbackPort)); + const server = startServer(0); + const base = `http://127.0.0.1:${loopbackPort}`; + try { + // Each entry uses the METHOD its handler actually accepts. Probing a POST route with GET + // would 404 on method mismatch inside the handler, so the assertion would hold even if + // the allowlist were widened to admit that route — the test would be watching nothing. + const denied: Array<{ method: string; path: string; body?: string }> = [ + { method: "GET", path: "/api/config" }, + { method: "GET", path: "/" }, + { method: "GET", path: "/healthz" }, + { method: "GET", path: "/readyz" }, + { method: "POST", path: "/v1/chat/completions", body: '{"model":"x","messages":[]}' }, + { method: "POST", path: "/v1/messages", body: '{"model":"x","messages":[]}' }, + { method: "POST", path: "/v1/images/generations", body: '{"prompt":"x"}' }, + { method: "POST", path: "/v1/alpha/search", body: '{"query":"x"}' }, + { method: "GET", path: "/v1/opencodex/artifacts/x" }, + { method: "POST", path: "/v1/live", body: "{}" }, + { method: "POST", path: "/v1/realtime/calls", body: "{}" }, + // Allowlisted paths still reject the methods they do not serve. + { method: "DELETE", path: "/v1/responses" }, + { method: "POST", path: "/v1/models" }, + ]; + for (const { method, path, body } of denied) { + const res = await fetch(`${base}${path}`, { + method, + ...(body ? { body, headers: { "content-type": "application/json" } } : {}), + }); + expect({ method, path, status: res.status }).toEqual({ method, path, status: 404 }); + } + + // And an allowlisted route is genuinely reachable, so the rejections above are not + // passing merely because nothing works on this listener. + expect((await fetch(`${base}/v1/models`)).status).toBe(200); + } finally { + await server.stop(true); + } + }); + + test("admits POST /v1/responses and its compact sibling without a credential", async () => { + const loopbackPort = await freePort(); + saveConfig(baseConfig(loopbackPort)); + const server = startServer(0); + const base = `http://127.0.0.1:${loopbackPort}`; + const publicBase = `http://127.0.0.1:${server.port}`; + try { + // These are the routes the reported defect actually fails on. `/v1/models` passing does + // not prove they admit: they use a different resolver. + // + // The request is deliberately malformed, so it fails INSIDE the handler rather than at + // admission. Any status other than 401 proves admission let it through, which is the + // only thing under test here — no upstream is involved. + for (const path of ["/v1/responses", "/v1/responses/compact"]) { + const viaPublic = await fetch(`${publicBase}${path}`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: "{}", + }); + expect({ path, status: viaPublic.status }).toEqual({ path, status: 401 }); + + const viaLoopback = await fetch(`${base}${path}`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: "{}", + }); + // Not `not.toBe(401)`: a 404 would satisfy that too, so removing the route from the + // allowlist would keep the assertion green. The route must be admitted AND reachable, + // which means neither 401 nor 404. + expect({ path, status: viaLoopback.status }).not.toEqual({ path, status: 401 }); + expect({ path, status: viaLoopback.status }).not.toEqual({ path, status: 404 }); + } + } finally { + await server.stop(true); + } + }); + + test("upgrades a Responses WebSocket on the listener that received it", async () => { + const loopbackPort = await freePort(); + saveConfig({ ...baseConfig(loopbackPort), websockets: true } as unknown as OcxConfig); + const server = startServer(0); + try { + // What this proves: the loopback listener completes a Responses WebSocket handshake + // without a credential, and the public one does not. + // + // What it does NOT prove: that the upgrade goes through `requestServer` rather than the + // captured `server`. That ablation was run and stayed green — this Bun version accepts + // an upgrade issued from a sibling Bun.serve in the same process. `requestServer` is + // still correct (it is the server that received the request, and nothing documents the + // cross-listener behaviour as supported), but the assertion below cannot defend it. + // Saying so beats implying coverage that does not exist. + expect(await handshake(`ws://127.0.0.1:${loopbackPort}/v1/responses`)).toBe(true); + expect(await handshake(`ws://127.0.0.1:${server.port}/v1/responses`)).toBe(false); + } finally { + await server.stop(true); + } + }); + + test("applies the loopback Host and Origin gate, not the public same-origin rule", async () => { + const loopbackPort = await freePort(); + saveConfig(baseConfig(loopbackPort)); + const server = startServer(0); + const url = `http://127.0.0.1:${loopbackPort}/v1/models`; + try { + // The kernel refuses remote TCP, but a victim's browser connects locally on an + // attacker's behalf. Under the PUBLIC policy this same-origin shape is allowed; under + // the loopback policy it must not be. + const rebinding = await fetch(url, { headers: { Host: "attacker.example" } }); + expect(rebinding.status).toBe(403); + + const hostileOrigin = await fetch(url, { headers: { Origin: "http://attacker.example" } }); + expect(hostileOrigin.status).toBe(403); + expect(hostileOrigin.headers.get("access-control-allow-origin")).not.toBe("http://attacker.example"); + + const ok = await fetch(url); + expect(ok.status).toBe(200); + } finally { + await server.stop(true); + } + }); + + test("stopping the server closes both listeners", async () => { + const loopbackPort = await freePort(); + saveConfig(baseConfig(loopbackPort)); + const server = startServer(0); + const publicPort = server.port; + await server.stop(true); + + // Both ports must be rebindable. A surviving loopback listener would keep serving + // unauthenticated traffic after shutdown reported success. + for (const port of [publicPort, loopbackPort]) { + const probe = Bun.serve({ port, hostname: "127.0.0.1", fetch: () => new Response("ok") }); + probe.stop(true); + } + }); + + test("a loopback bind failure rolls back the public listener rather than stranding it", async () => { + const loopbackPort = await freePort(); + // A FIXED public port, not 0. Throwing is not the property under test — a startup that + // throws while leaving the public listener bound is exactly the failure the rollback + // exists to prevent, and only a rebind attempt can tell the two apart. + // Reserve the loopback port during this draw: two back-to-back freePort() calls can hand + // back the same port, which would make the test squat its own public port. + const publicPort = await findAvailablePort(0, "127.0.0.1", { reservedPort: loopbackPort }); + const squatter = Bun.serve({ + port: loopbackPort, + hostname: "127.0.0.1", + fetch: () => new Response("occupied"), + }); + saveConfig(baseConfig(loopbackPort)); + try { + expect(() => startServer(publicPort)).toThrow(); + + const rebound = Bun.serve({ + port: publicPort, + hostname: "127.0.0.1", + fetch: () => new Response("ok"), + }); + expect(rebound.port).toBe(publicPort); + rebound.stop(true); + } finally { + squatter.stop(true); + } + }); + + // Not covered here: composite stop's failure PROPAGATION when one listener's stop rejects. + // The composite captures the underlying stop at construction, so a test cannot inject a + // rejection from outside without a seam that does not exist yet. Its sibling property — + // cleanup completing across both listeners — is covered by the test above. Writing a case + // that asserts something weaker and calls it propagation coverage would be worse than the + // gap, because the next reader would believe the branch was defended. +}); + +describe("composite listener shutdown", () => { + // Both listeners share one `stop`, and the two properties it must hold pull against each + // other: keep cleaning up after a failure, yet still report that failure. A test against a + // live server cannot inject the rejection, so the orchestration was extracted. + test("a failing step does not stop the others, and still reaches the caller", async () => { + const ran: string[] = []; + const failure = new Error("primary stop failed"); + await expect(runListenerShutdown( + [ + async () => { ran.push("primary"); throw failure; }, + async () => { ran.push("loopback"); }, + ], + async () => { ran.push("lifecycle"); }, + )).rejects.toBe(failure); + // The whole point: a rejected primary stop must not strand the loopback socket or skip + // the native lifecycle release. + expect(ran).toEqual(["primary", "loopback", "lifecycle"]); + }); + + test("two failures are reported together rather than one hiding the other", async () => { + const ran: string[] = []; + let caught: unknown; + try { + await runListenerShutdown( + [ + async () => { ran.push("primary"); throw new Error("a"); }, + async () => { ran.push("loopback"); throw new Error("b"); }, + ], + async () => { ran.push("lifecycle"); }, + ); + } catch (error) { + caught = error; + } + expect(caught).toBeInstanceOf(AggregateError); + expect((caught as AggregateError).errors).toHaveLength(2); + expect(ran).toEqual(["primary", "loopback", "lifecycle"]); + }); + + test("a failing lifecycle release is reported too", async () => { + const failure = new Error("release failed"); + await expect(runListenerShutdown( + [async () => {}], + async () => { throw failure; }, + )).rejects.toBe(failure); + }); + + test("an all-clear shutdown resolves", async () => { + await expect(runListenerShutdown([async () => {}, async () => {}], async () => {})) + .resolves.toBeUndefined(); + }); +}); + +describe("seams the runtime cannot defend", () => { + // Two properties have no runtime oracle on this Bun version, and both would regress + // silently. A source assertion is a weak instrument, but a weak instrument aimed at a known + // blind spot beats none — the alternative is a comment nobody runs. + const serverSource = readFileSync(join(process.cwd(), "src", "server", "index.ts"), "utf-8"); + + test("the WebSocket upgrade uses the receiving server, never the captured binding", () => { + // Swapping in `server.upgrade` stays green at runtime here: this Bun accepts an upgrade + // issued from a sibling Bun.serve in the same process. Another version or platform is not + // promised to, and the loopback listener would then fail to upgrade at all. + expect(serverSource).not.toMatch(/\bif \(server\.upgrade\(req,/); + expect(serverSource.match(/requestServer\.upgrade\(req,/g)?.length).toBe(2); + }); + + test("the loopback listener binds 127.0.0.1 explicitly", () => { + // The connection-refused test above is the real oracle, but it degrades to a warning on a + // host with no external IPv4 — and on that host the 0.0.0.0 ablation would pass. This + // holds everywhere. + expect(serverSource).toMatch(/port: loopbackListenerPort,\s*\n\s*hostname: "127\.0\.0\.1",/); + }); +}); + +describe("public port selection avoids the loopback port", () => { + afterEach(() => setEphemeralPortAllocatorForTests(null)); + + test("an explicit preference for the reserved port is refused rather than taken", async () => { + const reserved = await freePort(); + // Free, yet must not be selected: taking it would bind the public listener onto the + // address the loopback listener is configured for, and the loopback bind would then fail. + await expect(findAvailablePort(reserved, "127.0.0.1", { reservedPort: reserved })) + .rejects.toBeInstanceOf(PortUnavailableError); + }); + + test("ephemeral selection redraws when the OS hands back the reserved port", async () => { + // Without a seam this branch is unreachable: the OS practically never returns the one + // reserved port, so a loop over real draws would pass with the redraw code deleted. + const reserved = 45_001; + const draws = [reserved, reserved, 45_002]; + let index = 0; + setEphemeralPortAllocatorForTests(async () => draws[index++] ?? 45_003); + expect(await findAvailablePort(0, "127.0.0.1", { reservedPort: reserved })).toBe(45_002); + expect(index).toBe(3); + }); + + test("redrawing is bounded rather than recursing forever", async () => { + const reserved = 45_001; + let draws = 0; + setEphemeralPortAllocatorForTests(async () => { + draws += 1; + return reserved; + }); + await expect(findAvailablePort(0, "127.0.0.1", { reservedPort: reserved })).rejects.toThrow(); + expect(draws).toBe(8); + }); + + test("an unreserved preference is still honored", async () => { + const reserved = await freePort(); + const wanted = await freePort(); + if (wanted === reserved) return; + expect(await findAvailablePort(wanted, "127.0.0.1", { reservedPort: reserved })).toBe(wanted); + }); +}); + +describe("Codex injection targets the loopback listener", () => { + test("the written config points at the loopback port with no auth header", () => { + // A subprocess, because CODEX_CONFIG_PATH is resolved at module load: setting CODEX_HOME + // in-process would write to whatever path this test file already imported. + // + // The child passes the PUBLIC port to injectCodexConfig, which is what every real caller + // does. The loopback substitution happens inside the injector, so handing the loopback + // port straight to the block builder would keep passing with that wiring deleted. + const root = mkdtempSync(join(tmpdir(), "ocx-loopback-inject-")); + const codexHome = join(root, ".codex"); + const ocxHome = join(root, ".opencodex"); + mkdirSync(codexHome, { recursive: true }); + mkdirSync(ocxHome, { recursive: true }); + writeFileSync(join(codexHome, "config.toml"), 'model = "gpt-5"\n', "utf-8"); + const config = baseConfig(10_200) as Record; + config.port = 10_100; + writeFileSync(join(ocxHome, "config.json"), JSON.stringify(config), "utf-8"); + try { + const child = spawnSync(process.execPath, [ + join(process.cwd(), "tests", "helpers", "codex-inject-race-child.ts"), + ], { + cwd: process.cwd(), + encoding: "utf8", + env: { + ...process.env, + CODEX_HOME: codexHome, + OPENCODEX_HOME: ocxHome, + OCX_INJECT_RACE_PAYLOAD: JSON.stringify({ port: 10_100 }), + }, + }); + const line = (child.stdout ?? "").trim().split("\n").filter(Boolean).pop() ?? "{}"; + expect(JSON.parse(line)).toMatchObject({ success: true }); + + const written = readFileSync(join(codexHome, "config.toml"), "utf-8"); + expect(written).toContain("http://127.0.0.1:10200/v1"); + expect(written).not.toContain("http://127.0.0.1:10100/v1"); + expect(written).not.toContain("env_http_headers"); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); +}); diff --git a/tests/windows-deploy-close-regressions.test.ts b/tests/windows-deploy-close-regressions.test.ts index 05287d2de..41933c22a 100644 --- a/tests/windows-deploy-close-regressions.test.ts +++ b/tests/windows-deploy-close-regressions.test.ts @@ -73,8 +73,16 @@ describe("server bind canonicalizes explicit localhost but preserves wildcards ( test("literal localhost binds to 127.0.0.1; 0.0.0.0/:: exposure is untouched", () => { expect(src).toContain("const configuredHost = config.hostname?.trim();"); expect(src).toContain('!configuredHost || /^localhost$/i.test(configuredHost) ? "127.0.0.1"'); - expect(src).toContain("hostname: bindHost,"); - // Must not blanket-rewrite the bind host (that would break intentional 0.0.0.0 exposure). - expect(src).not.toContain('hostname: "127.0.0.1",'); + // Must not blanket-rewrite the PUBLIC bind host — that would break intentional 0.0.0.0 + // exposure, which is the regression this guards. + // + // A literal "127.0.0.1" now appears once, for the separate unauthenticated loopback + // listener (#1102). That one is a second socket whose entire purpose is to be + // loopback-only, so a bare substring ban would forbid the fix rather than the defect. + // Pin the assertion to the public serve call instead: it must take bindHost and nothing + // else. + expect(src).toContain("server = Bun.serve({ ...serveOptions, port: listenPort, hostname: bindHost });"); + expect(src).not.toMatch(/port: listenPort,\s*\n\s*hostname: "127\.0\.0\.1"/); + expect(src).not.toContain("port: listenPort, hostname: \"127.0.0.1\""); }); });