feat(net): SOCKS5 and PAC proxy support for all outbound connections - #145
Merged
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
pmaxhogan
enabled auto-merge (squash)
July 20, 2026 22:22
Contributor
Coverage
Gate: passed - no coverage regression (epsilon 0.1 pp). |
pmaxhogan
force-pushed
the
feat/proxy-pac-socks
branch
from
July 20, 2026 22:44
9afb058 to
4e58791
Compare
pmaxhogan
added a commit
that referenced
this pull request
Jul 24, 2026
🤖 I have created a release *beep* *boop* --- ## [2.1.0](v2.0.1...v2.1.0) (2026-07-24) ### Features * **core:** adaptive upload parallelism with throughput probe and disk-saturation gate ([#143](#143)) ([8ecced6](8ecced6)) * **core:** filesystem timestamp-granularity probe with ctime fallback and per-directory gitignore cascade ([#141](#141)) ([344262c](344262c)) * **drive:** support Google Shared Drive destinations end-to-end ([#142](#142)) ([d9c3161](d9c3161)) * **net:** native OS reachability backends with automatic fallback ([#138](#138)) ([319e85f](319e85f)) * **net:** SOCKS5 and PAC proxy support for all outbound connections ([#145](#145)) ([2f0b7d1](2f0b7d1)) * **net:** support a custom corporate root CA for all outbound connections ([#134](#134)) ([929e93d](929e93d)) * per-source toggle to back up OneDrive cloud-only placeholder files ([#133](#133)) ([6863ea3](6863ea3)) * **telemetry:** capture latency percentiles and add rollup query endpoint ([#132](#132)) ([4e9fde6](4e9fde6)) * **telemetry:** preview exactly what a telemetry ping sends ([#139](#139)) ([95fbd9a](95fbd9a)) ### Bug Fixes * **core:** commit file_state for a create that skipped post-upload so the next scan updates instead of re-creating ([#146](#146)) ([f5230d1](f5230d1)) * **deps:** bump tauri-winrt-notification to drop vulnerable quick-xml (closes [#89](#89)) ([#129](#129)) ([232fd8f](232fd8f)) * **telemetry:** exclude pre-schema rows from latency rollup ([#137](#137)) ([1ae6220](1ae6220)) * **ui:** add cursor pointer to buttons and link-buttons ([#136](#136)) ([dbd4809](dbd4809)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This was referenced Jul 29, 2026
pmaxhogan
added a commit
that referenced
this pull request
Jul 29, 2026
## The defect The SPEC s18 diagnostic bundle writes `settings_redacted.json` via `redact_settings()`, which built its output with `global: s.global.clone()` - the whole `GlobalSettings` struct verbatim. `GlobalSettings` carries the issue #34 `proxy_url`. In `manual` mode that URL may embed basic-auth userinfo: ``` http://corpuser:hunter2@proxy.corp.example:8080 ``` So the proxy password shipped **in plaintext** inside a bundle whose entire purpose is to be handed to support. Every other secret-bearing surface in the bundle is scrubbed (logs, crashes, and the activity CSV all go through `Redactor`; the telemetry `install_id` is hashed) - `settings_redacted.json` was the hole, and its own doc comment claimed "the only redaction here is the install id". Found while verifying the already-shipped #145 proxy work (issue #34). ## The fix Strip the userinfo before serializing, keeping scheme + `host:port` because that is the diagnostically useful part of a proxy setting: ``` http://<redacted>@proxy.corp.example:8080 ``` `redact_proxy_userinfo` is hand-rolled rather than routed through `url::Url` deliberately: a URL the parser rejects must still get scrubbed, never fall through to emitting the raw string. The authority is everything between `://` and the first `/`, `?` or `#`; the **last** `@` in it splits userinfo from host (a password may itself contain `@`). ## Tests Two new unit tests in `src-tauri/src/commands/settings.rs`: - `redact_settings_strips_proxy_basic_auth_credentials` - asserts the username and password are gone, `host:port` survives, and (the load-bearing one) that the **serialized JSON** carries no secret, since that is the artifact that actually leaves the machine. - `redact_proxy_userinfo_handles_every_url_shape` - no-credential passthrough, socks5, username-only, a password containing `@`, path/query not mistaken for the authority, an `@` inside a path left alone, and unparseable-garbage-with- an-`@` redacted wholesale rather than passed through. Ran locally: `cargo test -p driven-app redact_` (12 passed), `cargo clippy -p driven-app --all-targets -- -D warnings` (clean), `cargo fmt --all -- --check` (clean). ## Scope note Two adjacent fields in the same struct are also cloned verbatim into the bundle and may carry user PII, but are **out of scope here** and reported separately rather than silently swept in: `custom_root_ca_path` (an absolute path that can contain the OS username, and which the rest of the bundle would have hashed to `<path:...>`) and `pre_backup_hook` / `post_backup_hook` (free-text commands). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Qu8GxMwkuxF7JBzwRjtcw7 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
pmaxhogan
added a commit
that referenced
this pull request
Jul 29, 2026
…rocess (#191) ## The defect `load_pac_engine` cached the compiled PAC engine in a process-global slot keyed only on the source string, with **no TTL and no invalidation path**. Once compiled, a PAC script was pinned for the entire process lifetime. Two user-visible consequences: **1. Validation and routing disagreed (the sharp one).** `validate_pac_source` deliberately re-reads the live source - its doc comment said "bypasses the process cache so a re-validate always re-checks the live source" - but nothing ever published that result. So the settings-save flow was: 1. User edits the PAC file (same URL/path). 2. User re-saves. `validate_pac_source` fetches the **new** script, compiles it, confirms it is good, and logs `PAC proxy file validated on save`. 3. Every client keeps using the engine compiled from the **old** script. The user gets a success message and unchanged routing, with no way to tell why short of restarting the app. **2. Server-side PAC changes never landed.** A PAC file is administrator-managed and changes without telling us; a long-running Driven would route by a stale script indefinitely. Found while verifying the already-shipped #145 proxy work (issue #34). ## The fix - **TTL.** Cache entries carry an absolute `expires_at` and are only served before it. The window is `PAC_ENGINE_TTL` = 15 minutes (the usual WPAD refresh ballpark: short enough that a routing change lands in the same session, long enough that the ~9 client-build sites still share one fetch during a backup run). The deadline is stored absolutely rather than as a `fetched_at` age so every computation is an ADDITION to `Instant::now()` - subtracting from an `Instant` can fail on a machine booted moments ago (a fresh CI VM). - **Publish on validate.** `validate_pac_source` now stores the engine it just compiled, so a re-save takes effect immediately. This also removes a redundant refetch, since the save path had already fetched and compiled the script. - **Keyed cache.** The single slot became an `LruCache` keyed by source, so validating a not-yet-saved source cannot evict the live one. Capacity 8; in production only one source is ever configured, so this is effectively 1. No new dependencies (`lru` was already used for the per-host decision cache). ## A failed refresh serves the last-good engine (availability) Adding a TTL on its own would have been an **availability regression**: once the window lapsed, a momentarily unreachable PAC source would make every affected operation (update check, telemetry send, restore) fail - even though the last-good compiled engine was still in the cache, merely filtered out as stale. Pre-PR, the engine was pinned forever, so a PAC-server blip was invisible. So on refresh failure we **serve the last-good cached engine and log a warning**, and only return an error when there is no cached engine at all (the first resolve, where a misconfigured source must never come up unproxied). This does not weaken the fail-closed guarantee that actually matters: a stale PAC script **still routes through its proxy - it never silently degrades to DIRECT**. It is also what browsers do when a WPAD refresh fails. Both halves of "refresh failed" are covered: the source being unreachable, and the source being readable but serving a script that no longer compiles (an administrator pushing a broken redeploy). A 60-second retry backoff re-arms the entry so an outage costs at most one fetch attempt per window rather than one per client build. ## Tests In `crates/driven-tls/src/proxy.rs`: - `revalidating_a_changed_pac_source_republishes_it` - the end-to-end repro: resolve v1, edit the file, assert the cached engine is still served (that reuse is the point of the cache), re-validate, then assert the **new** script is in force. - `a_stale_cache_entry_is_refetched` - the TTL path, expiring the entry directly rather than sleeping, so it is instant and deterministic. - `a_failed_refresh_serves_the_last_good_engine` - TTL lapsed + source gone => the last-good engine is used and no error is returned; the entry stays cached for the retry window; and once the source is readable again the new script takes over. - `a_refresh_that_fetches_but_does_not_compile_also_falls_back` - the broken-redeploy half of the same behaviour. - `a_failed_first_fetch_with_no_cached_engine_still_fails_closed` - the other side of the fallback: with nothing cached, both an unreadable source and an uncompilable one are still hard errors. - `cached_entry_freshness_follows_its_deadline` - the freshness boundary. - `validating_an_unsaved_source_does_not_disturb_the_live_one` - pins the invariant that makes publish-on-validate safe. `validate_pac_source` is also reachable from the `validate_proxy` IPC command (the UI's "test this PAC file" button) for a source the user has **not** saved, so validating an arbitrary URL now writes to the process proxy cache. Because the cache is keyed by source, that write cannot perturb the engine the configured source is using - this test asserts exactly that. **Test isolation.** Every test that depends on PAC-cache state now takes a shared `tokio` mutex and starts from an empty cache, so they cannot evict each other's entries from the bounded LRU. That was a latent flake that would only have shown up on a loaded CI machine; fixed here rather than imported into the suite. **Negative controls** (verified these tests actually catch the bugs): - Removing the `store_pac_engine` call from `validate_pac_source` (pre-fix behaviour) makes `revalidating_a_changed_pac_source_republishes_it` fail with `left: Some("http://old:1")`, `right: Some("http://new:2")` - the stale routing, exactly as described. - Forcing the fallback lookup to `None` (i.e. no last-good fallback) makes both `a_failed_refresh_serves_the_last_good_engine` and `a_refresh_that_fetches_but_does_not_compile_also_falls_back` fail, while `a_failed_first_fetch_with_no_cached_engine_still_fails_closed` keeps passing - so the fail-closed test is not being satisfied by the fallback. Both restored before committing. Gates run locally: `cargo test --workspace` (1222 passed, 0 failed), `cargo clippy --workspace --all-targets -- -D warnings` (clean), `cargo fmt --all -- --check` (clean). Ran the `driven-tls` suite 10x consecutively after the isolation change - no flakes. No UI changes, so the pnpm gates do not apply. No dependency changes, so `cargo deny` is unaffected. ## Scope notes for the release notes (please keep these honest) **A PAC change still does not affect in-flight backup traffic.** The orchestrator and Drive clients resolve the proxy **once**, at account assembly, and hold the built clients. So the TTL makes a PAC change take effect for newly-built clients, not for a backup already running - that still needs a restart or an account re-add. The release notes must not claim PAC changes apply live to in-flight transfers. **The PAC date/time predicates remain stubs.** `weekdayRange`, `dateRange` and `timeRange` are still defined-but-always-`false`, so a time-gated PAC branch is never taken. Implementing them **requires** changing the per-host decision cache key in the same change: that cache keys on host only, with no time component, so a decision computed at 08:59 would be replayed at 09:01 - and unit tests that call the predicates directly would still pass. Flagging it so it is not discovered the hard way. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Qu8GxMwkuxF7JBzwRjtcw7 --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds proxy support to every outbound HTTP client, completing the corporate-network story started by the custom-CA work (#134) through the same
driven-tlsseam.Modes (KV-stored
globalsettings, serde-default - NO migration).no_proxy()everywhere - bypasses env proxies too.http://,https://,socks5://,socks5h://- socks5h resolves DNS proxy-side); reqwestsocksfeature enabled on both workspace reqwest 0.12 and the updater's reqwest 0.13.Proxy::customwith a 256-entry per-HOST LRU. Standard helpers implemented (isPlainHostName, dnsDomainIs, localHostOrDomainIs, dnsDomainLevels, shExpMatch, dnsResolve, isResolvable, myIpAddress, isInNet); date/time predicates (weekdayRange/dateRange/timeRange) are defined-but-false stubs.Semantics + documented caveats
system(the historical default).DRIVEN_PROXY_URL(manual) only; PAC unsupported there.paste(RUSTSEC-2024-0436, unmaintained build-time proc-macro via boa) ignored with justification;lruat 0.16.3+ (clears RUSTSEC-2026-0002).validate_proxy+ Settings UI proxy section (mode select, conditional inputs, inline validation), localized, vitest mount coverage.Refs #34
🤖 Generated with Claude Code
https://claude.ai/code/session_01QZQVP2tUuTLh8oL31D8heC