mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-31 22:14:25 +01:00
Garry Tanandcapy-ai[bot]
5755601dac
* fix(browse): direct mode launches Chromium with --proxy-server=direct:// so proxy env vars never win silently (#3440 item 8) BROWSE_PROXY_URL=direct only stopped gstack from configuring a proxy; the daemon inherits the environment and the launch passed no proxy switch, so Chromium read HTTPS_PROXY/HTTP_PROXY/ALL_PROXY itself and every goto still went through that proxy while browse reported a direct connection. Every launch path now adds --proxy-server=direct:// when no proxy is configured (chrome-headless-shell ignores --no-proxy-server), the daemon logs the effective proxy mode on launch, and BROWSER.md's proxy section says what Chromium actually gets. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes Name the direct-connection switch once (DIRECT_PROXY_ARG in proxy-config.ts) so the launch argument and the daemon log line cannot drift apart. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(browse): force direct:// only where Chromium would pick its own proxy; review fixes Red team (ship Step 9.2): the first cut passed --proxy-server=direct:// on every platform, but Chromium reads HTTPS_PROXY/HTTP_PROXY/ALL_PROXY itself only on Linux. On macOS and Windows it follows the system proxy settings (PAC/WPAD), which GStack Browser.app users behind a corporate proxy depend on, and BROWSE_PROXY_URL cannot name a PAC. forcesDirect(env, platform) now gates the switch: Linux always, elsewhere only with BROWSE_PROXY_URL=direct. The daemon log says what is really ignored per platform, or that the system settings apply. Review pass 2: the CLI e2e asserts on the daemon-log slice its own invocation wrote (the log is append-mode and shared across cases); connectProxy moves to browse/test/helpers/connect-proxy.ts for both e2e files; the CHANGELOG Tests bullets cover the generated cases. Adds the coverage-audit cases (unparseable HTTPS_PROXY; extension, headed and handoff paths under Xvfb). Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test(browse): assert the direct switch on the launched Chromium command line per platform (#3440 item 8) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes Ship review pass 3 (red team, GStack Minder's decision B, 2026-10-26): on macOS and Windows an explicit BROWSE_PROXY_URL=direct is the only thing that makes the daemon bypass the system proxy settings, so explicitDirect() puts it in the daemon config hash, server.ts records it as proxyDirect in the state file, and buildRestartEnv re-emits it on a crash auto-restart. A reused daemon started on the system proxy is now refused (the refusal names BROWSE_PROXY_URL and offers browse stop or unsetting it) instead of silently keeping the system proxy while the CLI reports direct. Linux hashes as before. BROWSER.md says the daemon log records the mode each time the daemon starts, not on every launch. The Linux desktop (GNOME/KDE) system-proxy question is filed as #3450. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test(browse): daemon-state and refusal e2e for the explicit direct opt-out; headed env and BROWSER.md contract checks (#3440 item 8) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes Ship rerun review pass: proxy-direct-launch-args.test.ts joins the macOS named-regressions job so its omission arm (no switch without the opt-out) runs in CI; serverProxyEnv takes the platform so the state-file predicate is pinned per platform instead of mirroring process.platform; buildHeadedServerEnv's parameter type requires explicitDirect so a narrowed caller cannot drop the opt-out silently; BROWSER.md's precedence item 6 and the reuse sentence say what each platform does (a connect daemon gets the generic flags refusal); CHANGELOG notes the one-time refusal against a pre-release daemon. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes Ship adversarial pass: a crash restart re-emits BROWSE_PROXY_URL=direct from the daemon state only when the invocation has no flags, so the env it hands the new daemon never contradicts the config hash it also hands over; an explicit-direct session is refused against a daemon with no recorded config hash, like --proxy and --headed already were; the unparseable-variable hint says that the opt-out also bypasses the system proxy settings on macOS and Windows; BROWSER.md notes that a managed Chromium proxy policy still overrides the switch; CHANGELOG names both files that joined the macOS named-regressions job. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(browse): read scheme-less proxy variables as http:// so the explicit direct launch keeps those sessions proxied (#3440 item 8) A standard proxy variable without a scheme (curl-style host:port or user:pass@host:port) was rejected by browse's parser with a "connects directly" warning while Chromium on Linux read the variable itself, so the session was proxied anyway. With the launch now explicit (--proxy-server=direct://) that environment went genuinely direct. resolveProxy now reads such values as http://, the way curl and Chromium's own environment reader take them; --proxy and BROWSE_PROXY_URL still require the scheme, and a value that does not parse even as http:// still warns and connects directly. Also drops buildRestartEnv's persisted-state re-emit branch, which production never reached (_globalFlags is set before any restart), names the four BrowserManager launch sites in the CHANGELOG and excludes the cookie-import worker, and carries the docs audit's one-line BROWSER.md project-tree note. Decision: GStack Minder (2026-10-26), option A. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes The lenient scheme-less proxy read is now gated on the plain [user:pass@]host[:port] shape (SCHEMELESS_PROXY_RE, trimmed): a scheme'd value with surrounding whitespace, a one-slash typo and a bare number keep the behaviour they had, and a credential that would spill past the userinfo (?, #, /) is not rewritten, so nothing can land outside the redacted part. Credentials that are not valid percent-encoding are a ProxyConfigError for every source instead of a URIError crash. buildRestartEnv's docblock, BROWSER.md and the CHANGELOG describe the parse (port 80 default, macOS/Windows now treat such a value as a configured proxy). Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes A backslash is excluded from the scheme-less proxy shape and parseProxyConfig refuses any proxy URL with a path, query or fragment, for every source: WHATWG parsing ends an http authority at a backslash, so a DOMAIN\user login or a / in a password landed in the path, which the redactor does not mask, and was printed in the clear on the start line, in the daemon log and in the state file. Such values now warn (standard variables) or error (--proxy, BROWSE_PROXY_URL) without echoing the credential. Docs updated. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(browse): keep a slash-less scheme spelling, refuse a bare ? or #, and tighten the proxy docs A standard proxy variable whose userinfo opens with a known scheme (https:bob:pw@host:3128, which the URL parser reads as https://) is no longer taken as scheme-less credentials, so the protocol it names is kept. parseProxyConfig refuses a literal ? or # anywhere in the value, not only a non-empty query or fragment. The unparseable-variable test now puts the credential in every looped value, so its no-echo assertion is real; new cases pin the slash-less scheme spelling, the bare ?/#, an env-sourced trailing slash, and that mixed credential sources stay a hard error for the scheme-less spelling as for the http:// one (the cred-mixing refusal predates this branch and its test on main pins it, so it is documented rather than softened). CHANGELOG and BROWSER.md: single backslashes inside code spans, "a path other than a bare /", the socks5 bridge log lines in the effective-mode list, and the character list applies anywhere in the value. Decision: GStack Minder (2026-10-26), option B. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(browse): keep a basic-auth URL out of the scheme-guard comment so the pregate secret scan stays clean Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test(browse): pin the mismatch remedy per platform so the Windows subset does not take process.platform proxyMismatchLines defaulted to process.platform in the env-proxy-meets-direct-daemon case, so on the windows-free-tests lane the remedy read 'or unset HTTPS_PROXY' and the assertion failed. The case now passes 'linux' explicitly and pins the darwin/win32 remedy beside it. Test only; source unchanged since1f6c85af. Decision: GStack Minder (2026-10-26), option A. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(browse): the one-slash http:/host:3128 spelling was never a diagnostic; say what the parser does with it The CHANGELOG sentence and the SCHEMELESS_PROXY_RE docblock claimed a one-slash scheme 'keeps its diagnostic'; the URL parser has always read it as http://host:3128/ and browse/test/proxy-env.test.ts:83 pins that. Wording only, found by the Step 14.5 docs audit on1f6c85af. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(browse): percent-encoding advice per scheme, and state the credential-free-path narrowing and the new scheme-less cred-mix refusal BROWSER.md said to percent-encode / \ ? # @ in http(s) credentials; ? and # are refused for every scheme, @ needs no encoding, and only the backslash is http(s)-specific. The CHANGELOG now states the narrowing that a credential-free standard-variable value with a path (http://proxy.corp:3128/pac) used to be read with the path dropped and now warns and goes direct, and that the cred-mix refusal is new for the scheme-less spelling. Wording only. Decision: GStack Minder (2026-10-26), option A. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(browse): only host:port was Chromium's own reading; user:pass@host:port is newly proxied Chromium's proxy spec parser (net/base/proxy_string_util.cc, ProxySchemeHostAndPortToProxyServer) returns an invalid server for any userinfo, so a scheme-less user:pass@host:port in HTTPS_PROXY was never honoured by Chromium on Linux and went direct; reading it as http:// follows curl and is new behaviour. BROWSER.md, the CHANGELOG, the resolveProxy docblock and the Linux e2e test comment now say so, and the scheme-less spill test is named for what it proves (those values never reach the path refusal; the http:// cases in the next test do). Wording only. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>