Skip to content

feat(proxy): opt-in macOS system proxy discovery for proxy auto - #6111

Closed
lidge-jun wants to merge 6 commits into
devfrom
codex/t4-clients-proxy-macos-proxy
Closed

lidge-jun wants to merge 6 commits into
devfrom
codex/t4-clients-proxy-macos-proxy

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 27, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Adds opt-in macOS system proxy discovery for proxy: "auto", carried from feat(proxy): macOS system proxy auto-discovery #5893 onto current dev. Windows discovery already existed; on macOS, "auto" now reads the global scutil --proxy dictionary once at startup (fixed path /usr/sbin/scutil, 2 s timeout, 64 KiB bound) and applies an enabled static HTTP/HTTPS proxy. Closes Feature: macOS system proxy discovery for proxy: "auto" (slice 2 of #1525) #5853 once this is on dev.
  • Discovery never runs without proxy: "auto", and it never runs when HTTP(S)_PROXY, the lowercase forms, ALL_PROXY, or all_proxy is inherited. The inherited route keeps both its proxy and its bypass variables. Nothing runs on the request path.
  • Exceptions are only applied when Bun and the WebSocket route matcher can represent them the same way. IP literals and * translate directly. *.<domain> becomes .<domain> (Bun matches on label boundaries, so foo.local goes direct and xlocal does not; the bare apex also goes direct). The macOS default link-local ranges 169.254/16, 169.254.0.0/16, and fe80::/10 are dropped with a one-line notice. Any other CIDR, glob shape, simple-host rule, ExcludeSimpleHostnames, PAC, or WPAD refuses discovery before any environment write, as does an unrepresentable configured localhost alongside an inherited lowercase no_proxy. The default macOS list (*.local, 169.254/16) therefore activates.
  • Bypass entries go into the variable each transport actually reads: Bun fetch reads a non-empty lowercase no_proxy before NO_PROXY, while resolveProxyRoute honors an explicit uppercase NO_PROXY. Proxy URLs are logged in redacted form only.
  • This carry drops the source PR's silent exception filtering, its uppercase-only bypass merge, and its partial acceptance of malformed settings. structure/config-proxy.md and the English plus seven translated server-configuration pages describe the rules.

Supersedes #5893. No GUI change.

Verification

  • bun test tests/server/proxy-env.test.ts tests/server/proxy-env-macos.test.ts tests/lab/core-lab-boundary.test.ts with a temporary HOME: 122 pass, 1 Windows-only skip, 0 fail. Coverage includes activation with the default list, refusal cases with a byte-identical 8-variable snapshot, inherited HTTP(S) and SOCKS precedence, split lowercase/uppercase bypass lists with route assertions for both transports, configured noProxy staying direct on both, the localhost refusal, malformed and disabled scutil output, and unchanged Windows behavior. scutil is mocked, so no real macOS Settings session was exercised.
  • The glob and CIDR semantics were measured with a Bun 1.4.0 probe against a local proxy: .local bypasses foo.local and not xlocal, while *.local and CIDR entries have no effect.
  • Layout tests pass (18), the file-size ratchet passes (9), and bun run typecheck, bun run structure:check, bun run privacy:scan, and the docs build (537 pages) all exit 0.
  • bun run test:changed is not passing evidence. The implementer's run overlapped test runs from other lane worktrees and exited 1 (26,718 pass / 110 fail / 17 errors, mostly timeouts in unrelated suites). I omitted the full local suite for the same resource-contention reason, so exact-head CI is the broad gate.
  • Independent security review took four rounds. It found that configured noProxy was missing from lowercase no_proxy and that bare localhost could be widened or dropped. All three were fixed, and the final round returned PASS. One residual predates this change and is outside it: a configured localhost in uppercase NO_PROXY is exact for the WebSocket matcher but a suffix for Bun. dev behaves the same way on every proxy path.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Co-authored-by: codingbo 9621077+codingbooo@users.noreply.github.com

Summary by CodeRabbit

  • New Features
    • proxy: "auto" now discovers static HTTP/HTTPS proxy settings on macOS as well as Windows, unless macOS proxy settings are inherited from the environment.
    • macOS proxy exclusions support *, IP addresses, and *.<domain> patterns. Unsupported exclusions prevent discovery without changing proxy environment settings.
    • Link-local IP exclusions are omitted with diagnostics; link-local IP addresses therefore use the proxy.
  • Documentation
    • Clarified proxy discovery behavior and supported settings across configuration guides.

lidge-jun and others added 6 commits September 28, 2026 01:30
Carry the bounded #5893 behavior with fail-closed exception translation and inherited proxy precedence.

Co-authored-by: codingbo <9621077+codingbooo@users.noreply.github.com>
Cover inherited SOCKS activation skips and the distinct lowercase Bun and uppercase WebSocket bypass decisions, including an explicitly empty uppercase value.
Map valid leading domain globs to Bun label-boundary suffixes and report their apex widening. Drop only exact link-local default ranges with a privacy-safe diagnostic; keep other unrepresentable rules fail-closed.
When macOS auto installs a previously absent proxy, copy configured noProxy entries into an inherited non-empty lowercase no_proxy so Bun and the WebSocket route keep configured destinations direct.

Co-authored-by: codingbo <9621077+codingbooo@users.noreply.github.com>
Preserve configured localhost in uppercase NO_PROXY for the exact-host WebSocket route, while leaving it out of Bun lowercase no_proxy so app.localhost remains proxied.

Co-authored-by: codingbo <9621077+codingbooo@users.noreply.github.com>
… bypass

When macOS auto discovers a proxy, a configured exact localhost bypass cannot be preserved by non-empty inherited lowercase no_proxy. Refuse before writing proxy variables and retain the existing uppercase-only path when lowercase is absent.

Co-authored-by: codingbo <9621077+codingbooo@users.noreply.github.com>
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 27, 2026 16:31
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-27T16:33:55.831561Z 001b833 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: b9419462-230e-431a-8519-f753325709e0

📥 Commits

Reviewing files that changed from the base of the PR and between 6d64ea2 and 001b833.

📒 Files selected for processing (16)
  • docs-site/src/content/docs/fr/reference/configuration/server.md
  • docs-site/src/content/docs/ja/reference/configuration/server.md
  • docs-site/src/content/docs/ko/reference/configuration/server.md
  • docs-site/src/content/docs/reference/configuration/server.md
  • docs-site/src/content/docs/ru/reference/configuration/server.md
  • docs-site/src/content/docs/tr/reference/configuration/server.md
  • docs-site/src/content/docs/zh-cn/reference/configuration/server.md
  • docs-site/src/content/docs/zh-tw/reference/configuration/server.md
  • scripts/test-layout/layout.json
  • src/config/macos-system-proxy.ts
  • src/config/proxy-env.ts
  • src/types/config.ts
  • structure/config-proxy.md
  • tests/fixtures/test-layout-expected.json
  • tests/server/proxy-env-macos.test.ts
  • tests/server/proxy-env.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

proxy: "auto" now reads supported macOS system HTTP and HTTPS proxy settings at startup. The implementation validates proxy URLs and bypass exceptions, applies eligible settings to the process environment, and leaves inherited proxy routing unchanged. Documentation and tests cover the discovery and routing rules.

Changes

macOS proxy reader

Layer / File(s) Summary
Read and validate system proxy settings
src/config/macos-system-proxy.ts
Reads bounded scutil --proxy output and validates enabled HTTP(S) proxies. Translates supported exceptions, reports omitted link-local ranges, and rejects unsupported settings or exceptions.

Proxy environment integration

Layer / File(s) Summary
Apply automatic discovery and bypass rules
src/config/proxy-env.ts
On macOS, checks inherited proxy variables before discovery. For eligible discovery results, applies proxy values and merges configured and translated exceptions with loopback bypasses. Shares configured noProxy parsing with the regular proxy path.
Document proxy configuration behavior
src/types/config.ts, structure/config-proxy.md, docs-site/src/content/docs/reference/configuration/server.md, docs-site/src/content/docs/*/reference/configuration/server.md
Updates proxy configuration guidance for macOS discovery, inherited proxy precedence, exception handling, and unsupported settings.

Regression tests

Layer / File(s) Summary
Cover discovery and routing behavior
tests/server/proxy-env-macos.test.ts, tests/server/proxy-env.test.ts, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json
Adds tests for discovery results, bypass routing, unsupported settings, and inherited proxies. Registers the new test and changes an existing test case to use Linux.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant applyProxyEnvWith
  participant readMacOSSystemProxy
  participant scutil
  participant ProcessEnvironment
  applyProxyEnvWith->>readMacOSSystemProxy: Read system settings when inherited proxy variables are absent
  readMacOSSystemProxy->>scutil: Run --proxy
  scutil-->>readMacOSSystemProxy: Return settings
  readMacOSSystemProxy-->>applyProxyEnvWith: Return validated proxy result
  applyProxyEnvWith->>ProcessEnvironment: Apply eligible proxy and bypass variables
Loading

Merge Risk: ⚪ Minimal · up to 001b8

No actionable merge-blocking issue was identified. Native macOS proxy settings remain untested, so normal platform validation is still advisable.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 001b8

Opt-in macOS discovery can change outbound routing for the process. The reviewed code limits when discovery runs and rejects settings it cannot safely translate; no exploitable weakness was established. Native macOS behavior remains to be verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — When opted in, macOS global proxy settings can influence outbound requests made through the process proxy environment, including server and catalog-sync entrypoints. The maximum affected scope is the process and its environment consumers; no tenant-level exposure was established.

Trust Boundaries and Controls

  • observed — Inherited HTTP, HTTPS, or ALL_PROXY routes prevent macOS discovery, preserving the inherited route and bypass authority.
  • observed — Accepted system exceptions are translated into bypass entries; unrepresentable exceptions refuse discovery, while specified link-local ranges are omitted rather than made direct.

Resilience and Maintainability Implications

  • inferred — The synchronous application path prevents ordinary same-thread interleaving during writes. Recovery from an unexpected failure after the first environment write is not established by the reviewed path.

Hardening Proposals

  • proposed — Verify parsing and routing against captured native macOS output, and document that proxy:auto is best-effort discovery rather than an enforced egress boundary when discovery is refused.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 5 files. (11 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely identifies the main change: opt-in macOS system-proxy discovery for proxy auto configuration. It matches the implementation, tests, documentation, and stated PR obje…
Linked Issues check ✅ Passed Issue #5853 coding requirements are met. src/config/macos-system-proxy.ts adds readMacOSSystemProxy() and reads the global /usr/sbin/scutil --proxy dictionary with a 2-second timeout and 64 KiB …
Out of Scope Changes check ✅ Passed The changed files remain connected to Issue #5853. src/config/macos-system-proxy.ts and src/config/proxy-env.ts implement discovery and environment integration. `tests/server/proxy-env-macos.test.…
Full details: Docstring Coverage

Explanation

Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 5 files. (11 skipped: 11 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the enhancement New feature or request label Sep 27, 2026
@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 64 / 80

이 PR은 맥에서 proxy를 "auto"로 적었을 때, 맥 시스템 설정의 HTTP/HTTPS 프록시를 프로그램이 켜질 때 한 번만 읽게 해요. 바탕은 dev예요. 윈도우용 자동 찾기는 원래 있었고, 맥이 이번에 들어와요. #5853을 닫는 변경이고, 아직 열려 있는 #5893을 지금 dev 위로 다시 올린 거예요.

proxy를 비우거나 "auto"가 아니면 맥 설정을 읽지 않아요. 이미 HTTP_PROXY, HTTPS_PROXY, 그 소문자 이름, ALL_PROXY, all_proxy 중 하나라도 있으면 맥 설정을 읽지 않아요. 읽는 명령은 /usr/sbin/scutil --proxy 하나예요. 2초가 넘거나 출력이 64KB를 넘으면 읽기에 실패한 것으로 봐요. 요청마다 다시 읽지 않아요.

예외 목록은 두 전송 길이 같은 뜻으로 옮길 수 있을 때만 써요. IP 주소와 *는 그대로 둬요. *.local 같은 모양은 .local로 바꿔요. foo.local은 프록시를 피하고, xlocal은 프록시를 타요. 이름 local 자체도 프록시를 피해요. 맥 기본값에 있는 169.254/16, 169.254.0.0/16, fe80::/10은 빼요. 그 주소는 프록시를 타요. 그 밖의 IP 대역, 다른 별표 모양, 그냥 호스트 이름, PAC, WPAD가 하나라도 있으면 환경 변수를 건드리지 않고 자동 찾기를 포기해요. 로그에는 프록시 주소를 가려서 찍어요.

라인 - src/config/proxy-env.ts 218행. 맥에서 프록시 환경 변수가 이미 있으면 여기서 함수가 끝나요. 설정 파일의 noProxy를 합치지 않고, 127.0.0.1 같은 루프백도 넣지 않아요. 윈도우는 262행에서 환경 변수가 이겨도 끝나지 않아서, 307행에서 noProxy와 루프백을 합쳐요. 이 분기가 생기기 전, 맥의 "auto"도 환경 변수가 이미 있으면 그 307행까지 내려갔어요. 맥에서 HTTPS_PROXY가 이미 있고 설정에 noProxy가 있으면, 그 예외와 루프백이 빠져요. 이 프로그램이 자기 주소로 보내는 확인 요청이 프록시로 나갈 수 있어요. tests/server/proxy-env-macos.test.ts 209행은 noProxy 없이 환경 변수가 그대로인지만 봐요.

라인 - src/config/macos-system-proxy.ts 230행. HTTP/HTTPS가 없고 SOCKS만 켜져 있으면 결과가 disabled예요. src/config/proxy-env.ts 228행은 그걸 "맥 시스템 프록시가 꺼져 있다"고 찍어요. SOCKS만 있는 경우와 정말 꺼진 경우가 같은 문장이에요.

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

예외를 하나라도 안전하게 못 옮기면 자동 찾기 전체를 포기하는 쪽을 유지할지 정하면 돼요. 회사 맥은 localhost나 10.0.0.0/8을 예외에 넣는 경우가 많아요. 그러면 "auto"는 프록시를 넣지 않고 로그만 남겨요. 테스트는 그 포기를, 환경 변수가 한 글자도 안 바뀌는 것으로 잠가 두었어요.

링크 로컬 범위를 빼서 맥 기본 목록(*.local, 169.254/16)은 켜지게 했어요. 그 주소는 프록시로 나가요. 기본 목록을 살리려면 이 교환이 필요해요.

찾기가 거절되거나 프록시가 꺼져 있을 때, 설정 파일의 noProxy도 아예 안 쓸지 정하면 돼요. 맥은 환경 변수를 안 바꿔요. 윈도우는 찾기가 실패해도 noProxy를 합쳐요.

#5893은 아직 열려 있어요. 이 PR이 그 내용을 현재 dev에 다시 담았고, 예외를 조용히 빼 먹지 않게 고쳤어요.

너의 추천

바탕은 dev로 두세요. types.ts와 config.ts를 나누는 일과는 다른 변경이에요. #5893은 닫으세요.

218행에서는 맥 설정 읽기만 건너뛰게 하세요. 프록시 환경 변수가 이미 있어도, 윈도우와 같이 설정의 noProxy와 루프백은 합치세요. 맥에서 읽어 온 예외 목록은 그 길에는 넣지 마세요. SOCKS만 켜진 경우는 꺼짐과 다른 로그로 나누면 좋아요. 실패하면 환경을 안 바꾸는 규칙과, 기본 *.local을 켜는 번역은 유지해도 돼요.

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

@lidge-jun

Copy link
Copy Markdown
Owner Author

Integration continues in #6124, which carries this PR's reviewed commits unchanged together with the other clients/proxy lane changes, so that only one branch has to chase the moving dev head through CI. This PR will be closed with a link once #6124 is merged.

@lidge-jun

Copy link
Copy Markdown
Owner Author

Landed on dev through #6124 (merge commit 296f0ce), which carries this PR's reviewed commits unchanged. Closing as integrated.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant