-
Notifications
You must be signed in to change notification settings - Fork 42
[network] pac , granular proxy setting #147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| // Draining resp.Body here can block indefinitely and stall the TLS handshake. | ||
| if resp.Body != nil { | ||
| resp.Body.Close() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CONNECT response body Close blocks TLS tunnel indefinitely
High Severity
In handleTLSViaHTTPProxy, after a successful CONNECT 2xx response, resp.Body.Close() is called. Go's http.ReadResponse for a CONNECT 2xx with no Content-Length header sets the body length to -1 (unknown), so Close() tries to drain the body by reading until EOF from the proxy connection. Since the proxy is waiting for the client's TLS ClientHello through the tunnel, this read blocks indefinitely and tunnel() is never reached. The comment on line 293–294 even acknowledges this risk, but the code still calls Close(). All HTTPS requests routed through an HTTP proxy will hang.
| r.mu.Lock() | ||
| r.cache[key] = decision | ||
| r.mu.Unlock() | ||
| return decision |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PAC cache key ignores URL path causing wrong routing
Medium Severity
The resolve cache key is scheme://host (line 67), but evalPAC is called with the full rawURL (line 76). PAC scripts receive the complete URL in FindProxyForURL(url, host) and can make routing decisions based on path or query parameters. The host-only cache key causes the first request's routing decision to be reused for all subsequent requests to the same host, silently returning wrong results for path-based PAC rules.
| return &value | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused function extractPACFlagValue is dead code
Low Severity
The unexported function extractPACFlagValue is defined but never called anywhere in the codebase. Grep confirms the only match is the definition itself. Since it's unexported, it can only be used within the api package, and no caller exists. This is dead code that adds maintenance burden.


[feature] pac granulary proxy
granular proxy setting via api
implementation
PUT /chromium/proxy/pacGET /chromium/proxy/pacGET /chromium/proxy/pac/scriptiptablesnatOUTPUTredirect tcp/80 and tcp/443 to local transparent proxy (pac-proxy, port15080)pac-proxyevaluates pac per-request and forwards direct/proxy accordinglyusage
demos
tested on local docker container
proxy image replacement
proxy to intercept+override images requests
demo_pac_images.mp4
additional : proxy setup snippets
# demo upstream proxy process docker run -d --name pac-upstream \ -p 18080:8080 \ -v /tmp/pac_mitm_addon.py:/addons/pac.py:ro \ -v /tmp/pac_rep_png.png:/addons/image.png:ro \ mitmproxy/mitmproxy:10.3.0 \ mitmdump -p 8080 -s /addons/pac.pynote : cert
PAC_MITM_CA_CERT_PATH(wrapper.sh) to pre-import CA into chromium NSS before launch.[ @rgarcia ]
Note
High Risk
Adds OS-level traffic interception via
iptablesand a new transparent proxy process, which can impact all outbound networking and is sensitive to container permissions and runtime environment differences.Overview
Adds API-managed PAC (Proxy Auto-Configuration) support for Chromium with new endpoints
GET/PUT /chromium/proxy/pacandGET /chromium/proxy/pac/script, including persisted PAC content/state under/chromiumand optional Chromium restart on updates.Implements OS-level enforcement by introducing a new
pac-proxydaemon andiptablesrules to transparently redirect outbound TCP80/443(and block UDP/443 QUIC) to a local proxy, and wires this into thechromium-headfulimage (new binary build/copy,pacproxyuser, supervisord service, required packages).Hardens Chromium startup by serializing
/chromiumconfig writes, switching launcher to run Chromium underdbus-run-session, cleaning crash-restore artifacts, and optionally importing a custom MITM CA into Chromium’s NSS DB before launch.Written by Cursor Bugbot for commit a209737. This will update automatically on new commits. Configure here.