Sign up
View stargazersSign up

feat(recapture): add the Egress setting and route app traffic through it#1896

merged3
main
feat/1695-egress-setting
opened12d agobyMMatt Donovan
32 files changed
View on GitHubTwo-way sync enabledPR last imported 9d ago

Closes #1695

Summary

Settings gains an Egress choice: Direct, the default, a SOCKS5 or HTTP proxy that needs no login, or the Operator's own Tor, each with an optional label. While it is not Direct, background captures, the Wayback pane, update checks and downloads, timestamp requests, the Wayback lookup and the cookie-banner filter lists all go through the proxy with no direct fallback, so with the proxy down they fail instead of leaving directly, and if the saved choice cannot be read they send nothing at all. A capture Birdbrain renders through it records the kind of route, the Operator's label and the browser identity string in its signed record, never the proxy's address, and for a secure page the certificate re-fetch after a capture does not run, which the record states for extension captures too. With Direct nothing changes and none of these fields is written. This change builds on the redirect fix in #1887, which has merged, and before it merges a person runs one recapture through Tor Browser and one update check from an installed build with the proxy stopped, as the details below describe.

Changes

Files changed, grouped by purpose
  • Commits: b20f16fd adds the setting, 762d4914 is review fix round 1 and 4d567473 is review fix round 2. #1887 has merged, so the base is main; the fixture tests use its test-only loopback switch.
  • src/shared/types.ts, src/shared/constants.ts, src/shared/schemas.ts: BirdbrainSettings.egress ({ kind: 'direct' }, { kind: 'proxy' | 'tor', proxyUrl, label }, or the load-only { kind: 'unavailable' }), the two Tor presets (socks5://127.0.0.1:9150, socks5://127.0.0.1:9050), and parseProxyUrl, shared by load, edit and the pane: socks5:// or http:// with a port and no path, refusing socks4://, socks4a:// and socks:// (SOCKS4 to Chromium) and any user name or password, with a message for each. The host must be dot-separated labels of letters, digits and inner hyphens, a dotted IPv4 address with each part at most 255, or a bracketed IPv6 address, so text Chromium's proxy rules would read as rule syntax (socks5://http=x;y:9050, which resolves to DIRECT) and out-of-range addresses (999.999.999.999, which Chromium turns into an empty proxy list) are refused; http://host:80, which URL reports with no port, is accepted as port 80. Tor takes socks5:// only. Stored normalised as scheme://host:port.
  • src/main/services/settings.ts: Direct by default, and onEgressChange, the onThemeChange pattern, fired on update, reset and a held write. An Egress the file no longer shows reads as { kind: 'unavailable' }: a read error, a parse error, or an egress key that fails its schema; a legible egress survives another key's failure, and a file with no egress key predates the setting and reads as Direct. This is the restrictive reading of an unknown privacy preference that the timestamping switch already takes (#1169). The marker is accepted on load (StoredEgressSchema), so a save over an unparseable file keeps it, and refused as an edit.
  • src/main/services/egress.ts (new): currentEgress, knownEgress (throws EgressUnavailableError for the unavailable marker), webRtcPolicyFor, createRoutedSession (a long-lived session whose gate re-reads the setting on every request and refuses it unless the session carries that Egress: after a change until the proxy applies, while the setting is unreadable, after the file changes with no write from the app, and after a refused setProxy, which it does not retry until another route has been applied or the setting names another; a session Direct since creation is left alone; onApplied runs after each route it takes), proxyConfigFor (fixed_servers, the proxy alone with no direct://, proxyBypassRules: '<-loopback>' so loopback also goes through it), applyEgressToSession, captureEgressOf (kind and label only), and egressFetch: the global fetch while Direct, otherwise session.fetch on an uncached egress-requests partition whose proxy is re-applied when the setting changes.
  • src/main/services/backgroundRenderer.ts: an unavailable Egress refuses the render before any window is made; the proxy goes on the fresh partition and setWebRTCIPHandlingPolicy('disable_non_proxied_udp') on the window before the first load; a load that fails at the proxy (ERR_PROXY_*, ERR_SOCKS_*, ERR_TUNNEL_*, ERR_NO_SUPPORTED_PROXIES) fails naming the proxy; RenderedPage.egress reports what was applied. Direct leaves the session untouched.
  • src/main/services/recapture.ts, src/main/services/captureLifecycle.ts, src/main/services/manifest.ts: IngestParams.egress writes egressKind, egressLabel and userAgent; the lifecycle sets tlsSkipped: 'egress-not-direct' and does not re-fetch while the Egress is not Direct, for every method, for an https page (the only kind the re-fetch reaches). An extension capture gets the skip and no kind, per the 2026-10-06 constraint on the issue. Duplicate re-anchors the four fields from the source's signed entry, as it does tls. CaptureEntryContext and withCaptureEntry carry them, omitted when absent.
  • src/main/services/timestamp.ts, src/main/services/consentBlocker.ts, src/main/ipcHandlers.ts: the TSA request, the filter-list download and the Wayback lookup go through egressFetch.
  • src/main/services/updater.ts, src/main/index.ts: the proxy goes on autoUpdater.netSession at start and on every Egress change, whether or not the build can check; a session Direct since launch is not touched; switching to Direct resets it to { mode: 'direct' }; a refused apply, or an unavailable Egress, puts the updater in error and no check or download runs. Each check and download re-reads the setting first and applies a changed Egress before it sends, and after waiting on an apply it reads again, so a later change made while it waited is applied first (egressSettled). index.ts gates the Wayback replay partition through createRoutedSession, set up before the window opens: its onBeforeRequest cancels every request the gate refuses, and a refused apply logs app.egress_apply_failed (new code, labelled in src/renderer/lib/notify.ts). Each Wayback guest gets setWebRTCIPHandlingPolicy(webRtcPolicyFor(egress)) when it is created, on every change, and after each route the gate applies.
  • src/renderer/components/settings/OperatorConfig.tsx: an Egress radio group beside the timestamp authority, with the address, Tor preset buttons and the label. Direct saves at once; a proxy or Tor saves only an address that parses. Choosing Proxy with no address says the saved Egress stays in effect; an unavailable Egress shows a notice and checks no choice; the choice follows the saved Egress whenever that changes. The description names the routes it governs, and says PDF export of an older HTML capture is not routed.
  • e2e/fixtures/socks5Proxy.ts (new): a hand-written SOCKS5 server (RFC 1928, no authentication, CONNECT) that connects only to allowed destinations and records its outbound ports. No new dependency.
  • e2e/recapture-egress.spec.ts (new): every request of a capture, page and image, arrives from one of the proxy's outbound connections, and the signed entry carries proxy, the label and the user agent at schema 5 and not the proxy's port; through a stopped proxy the capture fails with an error naming it and the target logs nothing; the electron-updater session and the Wayback partition resolve to the stopped proxy, and a request on the update session fails with nothing reaching the target.
  • Tests: egress.test.ts, timestampEgress.test.ts, and additions to settings.test.ts, updater.test.ts, backgroundRenderer.test.ts, captureLifecycle.test.ts, captureDuplicate.test.ts, recapture.test.ts and SettingsMaintenance.test.tsx. Fixture updates where a test now needs settings or asserts the fetch passed: the ingest and verify setups of captureLifecycle.test.ts, timestamp.test.ts, consentBlocker.test.ts, ipcHandlers.test.ts, StorageConfig.test.tsx.
  • Fix-round 2 tests: egress.test.ts (the gate closes when the file stops reading with no event and reopens on the same route, applies a proxy written from outside the app, stays closed when the setting stops reading during an apply, retries a once-refused proxy, onApplied), updater.test.ts (a later change while a check waits), settings.test.ts (four refused hosts), backgroundRenderer.test.ts (ERR_NO_SUPPORTED_PROXIES names the proxy), SettingsMaintenance.test.tsx (the choice follows the saved setting).
  • docs/specs/2026-07-31-evidence-affecting-paths-assessment.md: a blocking row for src/main/services/egress.ts.
  • Manual checks for a person before merge, which tests cannot make. One: with Tor Browser running, set the Egress to Tor on the Tor Browser preset, recapture any public page, and confirm it stores and its record names tor; then quit Tor Browser and confirm a recapture fails naming socks5://127.0.0.1:9150. Two: build a package (pnpm package:linux or the platform's script), install and launch it, set the Egress to a proxy on a port nothing listens on, and run Check for updates in Settings: it must report an error. The updater checks only in a packaged build (canCheck is app.isPackaged).
  • Not changed, left for the maintainer: the main window's default session is not routed, and on Linux Chromium downloads its spellcheck dictionary through it on the first launch for a language. PDF export renders on the pdf-export partition, which the Egress does not touch, so exporting a legacy .html capture fetches its remote subresources directly; this route predates the PR and is outside the issue's list. Shared Case sync is not routed (ADR-0032 rule 1) and is in no release yet.
  • Not changed: the Wayback wiring in index.ts has no test of its own; the gate and its onApplied follow-up are unit-tested in egress.test.ts, and index.ts is not instrumented.
  • Not changed, follow-up: the persona render ticket (#1499) has not landed, so a note goes on it when this merges, as the issue asks.

Evidence-affecting

Yes

What could change for evidence, and how that is checked

Evidence impact

  • Evidentiary result or interpretation that could change: while the Egress is not Direct, a background capture is fetched through the proxy, and its signed capture entry gains egressKind, egressLabel when one is set, userAgent, and for an https page tlsSkipped: 'egress-not-direct' in place of tls, stamped schema 5. Every capture method skips the TLS Cert Chain re-fetch then, so an extension capture's entry carries tlsSkipped and no kind. A Duplicate copies the four fields. While the Egress is Direct, entries, rows and chain hashes are as before. While the saved Egress cannot be read, no capture is rendered and an extension capture of an https page carries the same skip marker with no kind, so nothing new is stored as if it went Direct. For that entry the verifier prints that the re-fetch "was skipped because the Egress was not Direct", which overstates it when the unreadable file held Direct; a separate marker value would need a verifier release first (ADR-0023), so the existing one is reused and this sentence is the known gap.
  • What verification proves and does not prove after this change: the schema-5 reader that 1.0.1-beta.23 shipped (#1747) verifies these entries and states the Egress the entry records; the tests run the chain verifier and verifyCapture over them. The fields state what Birdbrain asked the browser to do, never which address the site saw, and they never include the proxy's address.
  • Backward verification preserved (existing evidence packages still verify): yes. No verifier changed, entries written while Direct are byte-identical, and a reader older than schema 5 reports a new entry as too new for it rather than as a broken chain (ADR-0023).
  • Known-answer test added or extended, or why none applies: the e2e spec drives real Electron through a hand-written SOCKS5 server and asserts, from the target's own connection log, that every request arrived through the proxy, and the signed fields; unit tests pin the proxy configuration with no direct fallback and each entry shape.

Verification

A pre-merge check failed

pnpm preflight at 4d5674734d07e43335dd2b888b8ff22977cc6f7b on Node v20.20.0; diff scored against origin/main (merge base d0bdbdeec). Result: fail.

  • pnpm lint - pass (exit 0)
  • pnpm format:check - pass (exit 0)
  • pnpm lint:boundaries - pass (exit 0)
  • pnpm lint:agents-md - pass (exit 0) - CLAUDE.md and AGENTS.md match
  • pnpm lint:adr - pass (exit 0)
  • vale-prose --base - pass (exit 0) - 1 changed Markdown file checked
  • pnpm typecheck - pass (exit 0)
  • pnpm build - pass (exit 0)
  • pnpm build:extension - skipped - no extension/ changes
  • pnpm test:coverage - fail (exit 1) - 1 failed, 6557 passed, 28 skipped (6586), failed before thresholds were evaluated
  • pnpm coverage:diff - fail (exit 1) - Missing /home/user/birdbrain/coverage/coverage-final.json. Run "pnpm test:coverage" before scoring diff coverage.

Pull request description generated by Claude Code

Checks

18 successful · 11 skipped
All checks passed
4d56747
CcoderabbitaiBot12d ago
Synced from GitHub

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: Repository: thebristolsound/birdbrain/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 283a92e6-f8ef-4552-b1fd-adf7f46aaa42

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.

Bbirdbrain-agent12d ago
Synced from GitHub

Review verdict: request changes

  1. If the settings file cannot be read or parsed, even for a moment, Birdbrain treats the connection as direct, so captures, timestamp requests and the certificate check leave from the operator's own address, and a failure at startup leaves the archive viewer and update checks direct until the setting next changes.
  2. The archive replay viewer runs page scripts without the browser rule that keeps peer-to-peer traffic inside the proxy, and a test run showed that traffic leaving directly while a proxy was set.
  3. The address check accepts some proxy addresses that the browser reads as "use no proxy", so a capture can leave directly while its signed record says it went through Tor or a proxy.
  4. The new Settings text tells the operator that nothing leaves directly while a proxy or Tor is set, which the three gaps above make untrue.
Full report

Reviewed commit: 1c9d9f836bcf373d8a16eaee208e37364babec6e

Scope reviewed: git diff 32b1033b 1c9d9f83 (this PR's one commit; base is PR #1887 at 32b1033b, still its head). Head re-read at the end of the review: unchanged.

CI at head: every CI job completed success (build, lint, typecheck, test, e2e, Dependency audit, Secret scan, github-advanced-security, adr-numbers, hunks). merge-gate is failure because it reads the pre-pass verdict (ADR-0041), not CI. The e2e job passed with one flaky retry in e2e/dashboard-activity.spec.ts:65 (not touched by this PR). Read from the check-runs API, because checks.sh --wait 540 1896 returned exit 0 at once with every check, including the running ones, under ignored ("not a job of the latest workflow runs for this PR").

Local verify at head (Node v20, Electron 44.7.0):

  • pnpm lint, pnpm typecheck, pnpm build: exit 0.
  • BIRDBRAIN_TEST_SYSTEM_SHARP=1 BIRDBRAIN_REQUIRE_OPENSSL=1 BIRDBRAIN_REQUIRE_JQ=1 pnpm test:coverage: first run 1 failed (tests/dispatchChecks.test.ts:231, a sleep-timing assertion in an untouched file; it passes alone, 19/19), second run exit 0, 6531 passed, 17 skipped (6548), which matches the PR body.
  • pnpm coverage:diff: pass, 200/206 = 97.09%, which matches the PR body. Uncovered: OperatorConfig.tsx:287 (save-failure message), :298-299 (UI refusal of a non-socks5 Tor address), :393 (label blur), schemas.ts:1204 (path/query refusal).
  • e2e not re-run locally; CI ran recapture-egress.spec.ts (3 tests passed) at this head.
#severityfile:linefinding
1blockingsrc/main/services/settings.ts:57, :164-202; src/main/services/egress.ts:11-13The Egress fails open. getSettings() re-reads settings.json on every call, and each of its three failure returns (read error :183, schema-invalid :190, JSON parse error :202) spreads DEFAULT_SETTINGS, whose egress is Direct (:57).
2blockingsrc/main/index.ts:656-666; src/main/services/backgroundRenderer.ts:482The Wayback replay guests get the proxy but no setWebRTCIPHandlingPolicy('disable_non_proxied_udp'); :482 is the only call site in src/. ADR-0032 rule 2 states the WebRTC rule without restricting it to renders, and rule 1 names the replay pane.
3blockingsrc/shared/schemas.ts:1179-1206; src/main/services/egress.ts:39parseProxyUrl lets ; and = through in the host. proxyConfigFor passes the result to Chromium as proxyRules, whose grammar reads scheme=proxy;... as per-scheme rules. socks5://http=x;y:9050 passes parseProxyUrl and the Tor schema, and Chromium resolves both http and https to DIRECT.
4blockingsrc/renderer/components/settings/OperatorConfig.tsx:322-327This new operator-facing string says "While it is not Direct, nothing Birdbrain sends leaves directly, and a request fails when the proxy is down". Findings 1-3 make it false. Fix the gaps or narrow the text.
5advisorysrc/main/index.ts:661-664; src/shared/types.ts:750-752When the Wayback partition refuses setProxy, the error is logged and the pane keeps Direct, which the comment at types.ts:750 documents. Electron rejects setProxy rarely, so this is a fail-open on an unlikely error. It is surfaced as a toast, and nothing blocks the pane.
6advisorysrc/main/index.ts:232-245The default session (main window) is never routed. On Linux it downloads the hunspell dictionary from the Chromium CDN (probe: spellcheck-dictionary-download-begin/-success on a fresh userData). That is a first-launch event while the Egress is Direct by default, and the dictionary is cached after it. Render partitions download through their proxy (the probe saw a download failure through a dead proxy). Relevant to finding 4's claim.
7advisorysrc/shared/schemas.ts:1202http://proxy:80 is refused with "Add the proxy port", because WHATWG URL drops a scheme's default port. The message is wrong for that input.
8advisorysrc/renderer/components/settings/OperatorConfig.tsx:305-316Choosing Proxy with no address selects the radio and saves nothing, so the pane shows Proxy with no message while Direct stays in effect.
9advisorysrc/main/services/updater.ts:304-312applySettingsChange runs void check() before syncEgress(). One partial carrying releaseChannel and a non-Direct egress would start a check before the proxy apply begins. No caller sends that today: reset always goes to Direct, and the pane sends { egress } alone.
10advisoryPR body, Summary"which the record of every capture, extension captures included, states" holds only for https pages (captureLifecycle.ts:1044 wouldRefetchTls). The Changes section is precise.

Failure scenario for 1: an Operator on Tor. Windows antivirus holds settings.json (EBUSY, the case the settings.ts:174-181 comment describes), or a crash truncates it during a plain writeFileSync. For each call made during that window:

  • currentEgress() returns Direct, so renderPageInHiddenWindow (backgroundRenderer.ts:479) renders from the Operator's own IP.
  • egressFetch sends the TSA request, the Wayback lookup and the filter lists directly (egress.ts:61-62).
  • An extension capture runs the raw-socket TLS re-fetch directly (captureLifecycle.ts:1041-1044).

If the failure hits at launch, two sessions stay Direct for the whole run, because nothing re-syncs either one until a write changes the setting:

  • updater.start() → syncEgress() leaves the electron-updater session untouched (updater.ts:200).
  • index.ts:665 skips the Wayback apply.

The PR body names this ("left for the maintainer"), but it describes only "a damaged file". The repository already rules that an unknown privacy preference falls closed (#1169, settings.ts:132-147 for the TSA switch), and ADR-0032 rule 2 says "It fails closed".

Failure scenario for 2: with the Egress on Tor, a replayed page whose archived script creates an RTCPeerConnection with a STUN server gathers candidates over UDP directly. That discloses the Operator's address to the STUN server, and to the page as a server-reflexive candidate. Probe: Electron 44.7.0 under xvfb, a session whose proxy is a dead SOCKS5 port, and a page creating a data-channel offer against a local UDP listener. The listener received 8 STUN packets from 192.168.8.241 and the tailnet address with the policy unset, and 0 packets with disable_non_proxied_udp. onBeforeRequest in hardenWebviewSessions does not see this traffic. The PR body discloses the omission and gives issue scope as the reason; the ADR text does not support that reading.

Failure scenario for 3: an Operator enters, or a settings file holds, socks5://http=x;y:9050.

  • In Electron's Node, parseProxyUrl returns { ok: true, url: 'socks5://http=x;y:9050' }, and EgressSettingSchema accepts it for tor.
  • The renderer's URL parser reads the same host (http=x;y), so the Settings pane saves it.
  • session.resolveProxy under that rule returns DIRECT for https://example.com/ and http://example.com/; socks5://https=q;z:9050 gives the same result.
  • A render then loads directly with no proxy error, and the signed entry records egressKind: 'tor' for a page that went direct, which is an unsound evidence claim.
  • Fix: restrict the host to a hostname or an IP literal.

Evidence gate:

  • Labels: the PR and #1695 both carry evidence-affecting.
  • Blocking-tier hits: index.ts, backgroundRenderer.ts, captureLifecycle.ts, consentBlocker.ts, egress.ts (row added by this PR), manifest.ts, recapture.ts, timestamp.ts, OperatorConfig.tsx.
  • Advisory hits, each with the region touched:
    • ipcHandlers.ts: the Wayback lookup fetch seam, routing only.
    • settings.ts: the egress default and listener. It reaches evidence through findings 1 and 3, and the PR is labelled already.
    • updater.ts: update routing, not evidence.
    • constants.ts: the Tor presets, not evidence.
    • schemas.ts: the settings schema. It reaches the signed egressKind through finding 3.
    • types.ts: a settings type and a log code, not evidence.
  • The Evidence impact section is substantive.
  • Known-answer coverage: the writer tests run verifyManifestChain and verifyCapture over Egress entries (captureLifecycle.test.ts, captureDuplicate.test.ts), and the e2e spec checks the signed fields. The schema-5 verifier has not changed since v1.0.1-beta.23, which carries it: git diff v1.0.1-beta.23 1c9d9f83 -- src/shared/verify is empty, and the schemas.ts changes are settings-only.
  • Backward verification: a Direct entry gains no field (captureLifecycle.ts:349-356, test "leaves a Direct capture exactly as before"), and the full suite's verify tests pass.

Checked and sound:

  • The render partition gets the proxy before the first load and fails naming the proxy.
  • TSA, Wayback lookup and filter lists go through egressFetch.
  • session.fetch sends the TSA's Buffer POST body correctly (probe).
  • Pooled Direct sockets are not reused after setProxy on a live session: probe second fetch ERR_PROXY_CONNECTION_FAILED.
  • Update check and download are blocked after a refused apply.
  • The extension-capture shape (skip marker only, no kind or user agent) matches the 2026-10-06 constraint.
  • Duplicate re-anchors all four fields.
  • No proxy host or port reaches a signed entry, a log line or an export: the failure text goes only to the renderer's in-memory activity list and the Recapture error UI, and no settings serialisation reaches diagnostics or archives.
  • Persona partitions are seeded and cleared only, and never loaded.
  • The main window CSP blocks remote loads.
  • Sequencing: v1.0.1-beta.23 is tagged, #1693 is closed and #1887 is open at 32b1033b.

Completion controls:

  • Attribution line is last authored line: pass.
  • No Claude co-author trailer on 1c9d9f83: pass, a case-insensitive grep of git log origin/main..HEAD --format=%B for the trailer exited 1.
  • auto_merge: null.
  • Head unchanged at the end of the review.

Not checked:

  • The two manual checks, a Tor Browser recapture and a packaged update check against a stopped proxy, need a person or a packaged build.
  • DNS prefetch under SOCKS5.
  • The macOS and Windows updater and spellcheck paths.
  • Local e2e, which CI ran green at this head.

Probe scripts were run under Electron (xvfb-run), not a shell. No .sh or workflow changed, so no bash-semantics check applied.

Bbirdbrain-agent12d ago
Synced from GitHub

Replies to the review verdict above, one per finding.

  1. Applied. A settings file that cannot be read, will not parse, or holds an Egress that no longer parses now reads as an unknown Egress, and an unknown Egress sends nothing: no capture, no timestamp request, lookup or filter list, no certificate re-fetch, no update check, and the archive pane loads nothing until it is set.
  2. Applied. Each archive replay guest now keeps WebRTC traffic inside the proxy, set when it opens and again on every change of the setting.
  3. Applied. The proxy host must now be a host name or an IP address, so text the browser would read as proxy rules is refused, in the app and in Settings.
  4. Applied. The Settings text now names the routes the setting governs and no longer says that nothing at all leaves directly; the sentence about shared-case sync is gone, since no release carries it.
  5. Applied. The archive pane now refuses every request until it carries the current proxy, so a refused proxy leaves it closed rather than direct.
  6. Not applied: the main window's session is outside the routes the issue names; it is listed for the maintainer in the description.
  7. Applied. An http proxy on port 80 is accepted.
  8. Applied. Choosing Proxy with no address now says the saved setting stays in effect.
  9. Applied. Each update check and download re-reads the setting and applies a changed proxy before it sends.
  10. Applied. The summary now says the skipped certificate check applies to secure pages.
Commit and where each change is

Fix round at f7141e8e, on top of 1c9d9f83.

  • 1: src/main/services/settings.ts (legibleEgress, the three failure returns), src/shared/types.ts and src/shared/schemas.ts ({ kind: 'unavailable' }, StoredEgressSchema, refused by SettingsUpdateSchema), src/main/services/egress.ts (knownEgress, EgressUnavailableError, egressFetch), backgroundRenderer.ts (refused before a window), updater.ts, and captureLifecycle.ts through currentEgress (the skip).
  • 2, 5: src/main/index.ts (createRoutedSession gating onBeforeRequest for the Wayback partition; webRtcPolicyFor on guest creation and on onEgressChange), egress.ts (createRoutedSession, webRtcPolicyFor).
  • 3, 7: src/shared/schemas.ts parseProxyUrl.
  • 4, 8: src/renderer/components/settings/OperatorConfig.tsx.
  • 9: src/main/services/updater.ts (egressReady re-syncs before checkForUpdates and downloadUpdate).
  • 10: PR body, Summary.
  • Tests for each are listed in the description's Changes block; e2e/recapture-egress.spec.ts now also checks the Wayback partition resolves to the proxy.
Bbirdbrain-agent12d ago
Synced from GitHub

Review verdict: request changes

  1. Once the archive replay pane is open, it keeps loading on its last route after the settings file becomes unreadable, and stays direct if the file is switched to a proxy from outside the app, while the description, the code notes and a new Settings warning say it would refuse.
  2. Exporting an older plain-HTML capture to PDF still fetches its images and other files over the operator's own connection while a proxy or Tor is set, and the description's list of what stays unrouted does not name it.
  3. While the settings file cannot be read, a browser-extension capture's signed record says the certificate check was skipped because the connection was not direct, which is untrue when the saved choice was direct.
  4. An update check that is waiting for one route change can go out on the old route if the setting changes again before the second change takes effect, though that needs two changes within milliseconds.
Full report

Reviewed commit: f7141e8ea627f904d2a4fcad22bd5c687668ca89

Scope: the fix-round delta git diff 1c9d9f83 f7141e8e (15 files) read in full, plus the parts of git diff 32b1033b f7141e8e its claims reach. Base PR #1887 is still at 32b1033b. Head re-read at the end of the review: unchanged.

Round-1 findings, re-judged at this head

  • 1 (unreadable settings fail open): resolved for every route except the replay pane once it is open (finding 1 below). A read error, a parse error and a schema-invalid egress now read as { kind: 'unavailable' } (settings.ts:169-175, :200, :210, :223). Each route then refuses:
    • renders: knownEgress() before any window, backgroundRenderer.ts:386
    • TSA, Wayback lookup, filter lists: egressFetch, egress.ts:153. Callers: timestamp.ts:54, ipcHandlers.ts:811, consentBlocker.ts:50. lookupSnapshots has no other caller.
    • TLS re-fetch: skipped while not Direct, captureLifecycle.ts:1041-1046
    • updater: re-reads per check and blocks on unavailable, updater.ts:204-242
    • Startup: the pane stays closed until the file reads again.
  • 2 (Wayback WebRTC): resolved. index.ts:369-371 sets the policy at guest creation and :677-686 sets it on every change. Probe under Electron 44.7.0 (xvfb) used a webview guest on a partition with a dead SOCKS5 proxy and a local STUN listener:
    • created with disable_non_proxied_udp: 0 packets; switched live to default: 8
    • created with default: 8; switched live to disable_non_proxied_udp: 0
    • contents.session already matches the partition at web-contents-created.
    • No test covers this wiring (finding 6).
  • 3 (proxy-rule syntax): resolved. schemas.ts:1205 refuses ;, =, , and %. 28 inputs (25 accepted, 3 refused) were run through parseProxyUrl and then setProxy and resolveProxy. None resolved to DIRECT. Out-of-range IPv4 hosts give an empty list, which fails with ERR_NO_SUPPORTED_PROXIES (finding 7).
  • 4 (Settings text): the intro text is now accurate for the routes it names. The new unavailable notice overclaims (finding 1).
  • 5 (refused Wayback apply): resolved. The gate stays closed after a refusal (egress.ts:119-122, :129-133).
  • 6 (main window's default session): not applied. The body discloses it accurately.
  • 7, 8, 10: resolved.
  • 9 (updater re-sync): resolved for the reported partial. A narrower race remains (finding 4).
#severityfile:linefinding
1blockingsrc/main/services/egress.ts:127-134, :98-102; claims at egress.ts:8-9, :73-74, src/renderer/components/settings/OperatorConfig.tsx:343-347, src/shared/types.ts:452-455, PR body Summary and the egress.ts and index.ts bulletsallowsRequests returns true once applied is set, and reads the setting only while the gate is closed. An open pane therefore keeps its last route when settings.json becomes unreadable, and stays Direct when the file is changed to a proxy without an in-app write. The claims say it refuses "while the setting is unreadable" and "until it carries the current Egress". The body says "if the saved choice cannot be read they send nothing at all", and the notice says "sends nothing on these routes until you choose one".
2advisorysrc/main/services/pdfExport.ts:13-15, :258PDF export renders on the pdf-export partition, which the Egress never touches. The code's own comment says legacy .html captures "still fetch remote subresources". Not in the issue's route list. The body's "left for the maintainer" names only the spellcheck session. The Settings intro at OperatorConfig.tsx:338-341 reads as the full list of Birdbrain's own traffic.
3advisorysrc/main/services/captureLifecycle.ts:1041-1046; src/shared/verify/evidencePackage.ts:146-153While the Egress is unavailable, an extension capture of an https page gets tlsSkipped: 'egress-not-direct' with no kind. The verifier prints "the TLS Cert Chain re-fetch was skipped because the Egress was not Direct", which is false when the unreadable file held Direct. Skipping is the fail-closed choice, and a new marker value needs a verifier release first (ADR-0023). The Evidence impact section describes the marker but not the sentence the verifier prints for it.
4advisorysrc/main/services/updater.ts:268-270, :293-295check() and download() await the promise egressReady() returned, and do not re-check after the await. If the setting changes again while they wait, they send once the first apply lands, with the second still pending. The body says each "applies a changed Egress before it sends", which holds only for a change made before the check starts.
5advisorysrc/main/services/egress.ts:121, :131refused is never cleared. Sequence: X refused, Y applied, X applied, an unavailable event, then the file reads X. The gate stays closed for good because the key equals the stale refused. It fails closed, so nothing leaks.
6advisorysrc/main/index.ts:213, :369-371, :677-686The fixes for round-1 findings 2 and 5 have no test. index.ts is not instrumented (CI lists it as not scored), and the e2e addition at e2e/recapture-egress.spec.ts:203-211 checks only resolveProxy on the partition. It does not check the request gate or the guest policy.
7advisorysrc/shared/schemas.ts:1205; src/main/services/backgroundRenderer.ts:374The host check accepts 999.999.999.999, 1.2.3.999, .. and -. The first two give Chromium an empty proxy list, which fails with ERR_NO_SUPPORTED_PROXIES, which proxyFailure does not match, so the render error does not name the proxy. The body calls the accepted set "a host name or an IP literal".
8advisorysrc/shared/types.ts:758-759This comment, added in this PR, still says that after a refused apply "the pane then keeps the proxy it had before the change, which is Direct if there was none". Since the fix round the gate refuses every request in that state.
9advisorysrc/renderer/components/settings/OperatorConfig.tsx:277-289The seeded ref stops later stored changes from reaching the radios. A file that goes unreadable after the first load leaves Tor checked beside the "could not be read" notice. One that reads again leaves nothing checked while Tor is in effect.

Failure scenario for 1. All three paths were reproduced against createRoutedSession extracted verbatim from egress.ts at this head, using a fake session (/tmp/ce1695-probe2/routed-probe.js).

  • (a) The pane applied Tor, then settings.json went EBUSY with no change event. That is the Windows antivirus case settings.ts:189-198 is written for. allowsRequests() returned true while getSettings().egress was unavailable. The renders, egressFetch and the updater refuse in that state. The pane keeps loading through Tor, and Settings tells the Operator nothing is sent.
  • (b) Direct since launch, then the file was changed to Tor without an in-app write. allowsRequests() returned true with the session never touched, so replay requests go direct while renders go through Tor.
  • (c) A Tor-to-Direct change, then an unavailable change event while the Direct apply was in flight. The gate opened Direct while the setting read unavailable, because sync clears applied but not applying (egress.ts:99-102). Only the held-write race at settings.ts:292-301 delivers that event.
  • No in-app write changes the Egress without an event: every writer goes through updateSettings or resetSettings. In (a) no traffic leaves outside the route the Operator last chose. The defect is that the gate artifact, the code contract and an operator-facing string say otherwise.
  • Fix either way: compare keyOf(readEgress()) with applied on each call, as the updater already does per check, or narrow the claims to "while it is closed".

Failure scenario for 2. A probe (/tmp/ce1695-probe2/pdf.js) loaded a file:// page holding one remote <img> into a window on partition pdf-export, with the same sandbox and isolation flags as pdfExport.ts:252-259. resolveProxy reported DIRECT, and the target server logged /subresource.png. With the Egress on Tor, exporting a legacy HTML capture therefore contacts the servers its subresources come from, from the Operator's own address. ADR-0032 rule 1 says nothing Birdbrain sends leaves directly. This route predates the PR and is outside the issue's scope. It is listed for the maintainer, not as a defect of this change.

Failure scenario for 4. The updater was bundled from this head with Electron stubbed (/tmp/ce1695-probe2/upd-probe.js):

  • start on Tor and let the apply finish
  • change to Direct and call check(), which waits on the Direct apply
  • change to Tor, then let only the Direct apply finish
  • checkForUpdates ran while the session carried direct, with the setting on Tor and the Tor apply still pending.

CI at head. Read from the check-runs API at f7141e8e; checks.sh was not used, per the dispatch note.

  • success: lint, typecheck, test, build, e2e, github-advanced-security, Dependency audit, Secret scan, adr-numbers, hunks, Registry publish guard, Font licence check
  • e2e: 86 passed, none flaky, including the three recapture-egress tests
  • skipped: docs-build, coverage-badge, request, triage, stamp, reject, cancel, issue-evidence
  • failure: merge-gate, which reads this verdict (ADR-0041), not CI
  • CI diff coverage against the PR base: 203/211 = 96.21%, pass. Uncovered: captureLifecycle.ts:251, OperatorConfig.tsx:297, :308-309, :328-329, :413, schemas.ts:1214. All are error or rare branches. index.ts and types.ts are not scored.

Local verify at head (Node v20.20.2, Electron 44.7.0):

  • pnpm lint, pnpm typecheck, pnpm build: exit 0.
  • BIRDBRAIN_TEST_SYSTEM_SHARP=1 BIRDBRAIN_REQUIRE_OPENSSL=1 BIRDBRAIN_REQUIRE_JQ=1 pnpm test:coverage: three runs.
    • Run 1 exit 1: tests/components/NotesOverview.test.tsx:133 timed out at 16.9 s against a 15 s limit while Electron probes ran alongside. It passes alone, 13/13.
    • Run 2 exit 1: the tests/dispatchChecks.test.ts:231 sleep-timing assertion, which round 1 also saw. It passes alone, 19/19.
    • Neither file is in the diff, and a failed run writes no coverage report.
    • Run 3, with no other load, exit 0: 6555 passed, 17 skipped (6572), thresholds met. This matches the body.
  • pnpm coverage:diff after run 3: exit 0, 270/278 = 97.12% against origin/main (merge base 58997988d). This matches the body, with the same uncovered lines as CI.
  • e2e was not re-run locally. CI ran it at this head.

PR body.

  • The Verification block names this head.
  • Its test count, 6555 passed and 17 skipped of 6572, matches local run 3.
  • Its diff coverage, 97.12% of 278 lines, is scored against origin/main (merge base 58997988d), as it says, so it includes #1887's lines. It was reproduced locally. CI scores the PR base instead.
  • Every Changes claim was checked against the code. False ones are findings 1, 4 and 7. The rest hold.

Evidence gate.

  • Labels: the PR carries evidence-affecting and agent-authored, and #1695 carries evidence-affecting.
  • Blocking-tier paths in the delta: src/main/index.ts, backgroundRenderer.ts, egress.ts (row at the assessment's line 216), OperatorConfig.tsx.
  • Advisory-tier paths in the delta, each with the region touched:
    • settings.ts: the Egress load fallbacks. These reach evidence, because they decide the extension-capture skip marker (finding 3). The PR is labelled already.
    • updater.ts: update routing only, not evidence.
    • schemas.ts: the proxy host check and the stored or edit Egress schemas. These reach the signed kind through the settings, and are covered by the label.
    • types.ts: the settings type and a comment.
  • The Evidence impact section is substantive.
  • Known-answer coverage: captureLifecycle.test.ts adds "skips the re-fetch while the saved Egress cannot be read", which writes a real unparseable settings.json. The kind-less skip shape is verified at tests/shared/verify/manifestSchema5.test.ts:148.
  • Backward verification: git diff 32b1033b f7141e8e -- src/shared/verify is empty, and Direct entries are unchanged.

Completion controls.

  • The attribution line is the last authored line of the body: pass.
  • No commit on the branch (git log origin/main..HEAD, 4 commits) carries a Claude co-author trailer. A case-insensitive grep exited 1.
  • auto_merge is null.
  • The head was unchanged at the end of the review.

Not checked.

  • The two manual checks: a Tor Browser recapture, and a packaged update check against a stopped proxy.
  • DNS prefetch and preconnect from a replay page while the gate is closed but the session is still Direct.
  • MHTML export: Chromium serves MHTML from the archive, so it was not probed.
  • The updater and spellcheck paths on macOS and Windows.

Every probe ran under Electron through xvfb-run, or under Node. No shell script or workflow changed, so no bash check applied.

Cchatgpt-codex-connector[bot]12d ago
Synced from GitHub

Codex Review Summary

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

ReviewStatusCommitReview trigger
📝 Code Review⚠️ Failed 2026-10-17T15:42:48.019559Zf7a890cDraft marked ready
ℹ️ 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.

Added 3 commits

  • fix(recapture): fail closed on an unknown Egress, gate the Wayback pane762d491
  • feat(recapture): add the Egress setting and route app traffic through itb20f16f
  • fix(recapture): re-read the Egress on each Wayback request4d56747
MMatt Donovan12d ago

Note

Claude responding on behalf of Matt.

Replies to the second review verdict above, one per finding.

  1. Applied. The archive pane now reads the saved setting on every request, so it refuses while the file cannot be read and takes a proxy written from outside the app before it loads again, and WebRTC follows each route it takes.
  2. Not applied: PDF export is outside the routes the issue names; the description and the Settings text now say it is not routed.
  3. Not applied: a new marker needs a verifier release first, so the description now says what the verifier prints while the setting cannot be read.
  4. Applied. An update check that waited on one change now waits for any later change before it sends.
  5. Applied. A refused proxy is retried once another route has been applied.
  6. Not applied: the gate and its WebRTC follow-up are tested in the egress unit tests; the app wiring around them is a few lines in a file the tests do not load.
  7. Applied. Out-of-range IPv4 addresses and hosts made only of dots or hyphens are refused, and a render that finds no usable proxy names it.
  8. Applied. The code note now says the pane refuses every request after a refused proxy.
  9. Applied. The Settings choice follows the saved setting whenever it changes.
Commit and where each change is

Fix round at 4d567473, on top of 762d4914.

  • 1, 5: src/main/services/egress.ts (allowsRequests compares the current key with applied on every call; applied now records what the session carries and is no longer cleared by an unavailable event; refused clears on a successful apply; new onApplied callback), src/main/index.ts (setWaybackWebRtc passed as onApplied).
  • 2, 3: PR body, the "Not changed" bullet and Evidence impact; OperatorConfig.tsx intro text.
  • 4: src/main/services/updater.ts egressSettled, which loops on egressReady after each await.
  • 7: src/shared/schemas.ts isProxyHost; backgroundRenderer.ts proxyFailure matches ERR_NO_SUPPORTED_PROXIES.
  • 8: src/shared/types.ts.
  • 9: OperatorConfig.tsx, the seen ref replaces seeded.
  • Tests: egress.test.ts (five cases: unreadable after open, file change with no event, unavailable during an apply, a refused proxy retried, onApplied), updater.test.ts (a later change while a check waits), settings.test.ts (four refused hosts), backgroundRenderer.test.ts (ERR_NO_SUPPORTED_PROXIES), SettingsMaintenance.test.tsx (the choice follows the saved setting).
  • Local preflight at 4d567473 failed on one test only, tests/main/services/staging.test.ts "keeps the row when the bytes cannot be removed". It fails the same way at 762d4914 without this change, because the container runs as root and root can remove the file the test protects. Coverage and diff coverage therefore did not run locally; CI runs them.

Generated by Claude Code

Bbirdbrain-agent10d ago
Synced from GitHub

Review verdict: approve for human review

Last round's blocking problem is fixed: the archive replay pane now checks the saved setting before every request, so it refuses while the file cannot be read and picks up a proxy written from outside the app.

  1. Three setting changes inside a few thousandths of a second can open the replay pane while a direct route is still being applied, because the check matches route names rather than which change finished; a person cannot click that fast.
  2. A proxy address written with leading zeros, such as 010.0.0.1, is accepted, but the browser reads those parts as octal and connects to 8.0.0.1, which is not the address Settings shows.
  3. The new check does not stop peer-to-peer traffic from a replay page that is already open, so after a change made outside the app that traffic stays on this device's own connection until the page next loads something.
  4. The new Settings line about PDF export names only images, though an older HTML page's other remote files, such as stylesheets and fonts, also load directly.
Full report

Reviewed commit: 4d5674734d07e43335dd2b888b8ff22977cc6f7b

Scope: git diff 762d4914 4d567473 (12 files: 7 source, 5 test) read in full, plus the code its claims reach. git range-diff f7141e8e~2..f7141e8e 762d4914~2..762d4914 shows both earlier commits as =, so the rebase onto main changed nothing that round 2 reviewed. #1887 has merged and the base is now main (merge base d0bdbdeec). Head re-read at the end of the review: unchanged.

CI at head. bash .github/scripts/dispatch/checks.sh --wait 540 1896 exit 0: failing [], pending [], passed: Dependency audit, Font licence check, Registry publish guard, Secret scan, adr-numbers, build, changes, e2e, hunks, lint, seed, test, typecheck. Skipped: coverage-badge, docs-build, triage. merge-gate is ignored because it reads this verdict (ADR-0041). Two CI runs at this sha are both green. The e2e job ran 86 tests, all passed, including the three recapture-egress.spec.ts tests, and the test job's diff coverage was 216/224 = 96.43%.

Local verify at head (Node v20.20.2, run under bash):

  • pnpm lint exit 0 (0 errors, 47 warnings). The one warning in a changed file, OperatorConfig.tsx:287 set-state-in-effect, is on a line the delta did not change: the same setKind(null) call was in the effect at 762d4914 (:283).
  • pnpm typecheck exit 0, pnpm build exit 0.
  • BIRDBRAIN_TEST_SYSTEM_SHARP=1 BIRDBRAIN_REQUIRE_OPENSSL=1 BIRDBRAIN_REQUIRE_JQ=1 flock … pnpm test:coverage exit 0: 6569 passed, 17 skipped (6586), 401 files passed, 1 skipped, thresholds met.
  • pnpm coverage:diff exit 0: 216/224 = 96.43% against origin/main, the same as CI. Uncovered: captureLifecycle.ts:251, OperatorConfig.tsx:302, :313-314, :333-334, :419, schemas.ts:1225. None of these lines is in the round-3 delta. index.ts and types.ts are not scored.
  • e2e not re-run locally. CI ran it at this head, and the delta changes no e2e file.

Round-2 findings, re-judged at this head

  • 1 (open pane keeps its last route): resolved for page requests. allowsRequests re-reads the setting on every call and opens only when applied === key && applying === null (egress.ts:129-136). sync no longer clears applied on an unavailable event (:102). I ran a bounded exhaustive check of createRoutedSession, bundled with esbuild from egress.ts at this head: 1,884,656 sequences up to depth 6 over in-app changes, out-of-app changes, unavailable events, apply resolve and reject, and requests. All three round-2 paths now hold: (a) unreadable after open, (b) an outside change from Direct, (c) unavailable during an apply. The same check on 762d4914's egress.ts reproduces round 2's violations: open while unavailable, open on a stale route, open while untouched on a proxy. Two residuals remain, findings 1 and 3.
  • 2 (PDF export not routed): not applied in code, now disclosed. Body "Not changed" bullet; Settings text at OperatorConfig.tsx:346-347 (finding 4).
  • 3 (verifier sentence while unreadable): not applied, now disclosed. The Evidence impact quote matches evidencePackage.ts:146 word for word.
  • 4 (updater race): resolved. egressSettled (updater.ts:246-253) awaits and then asks egressReady again. Promise reactions run in registration order, so egressApplied is already a boolean when the loop resumes and the loop does not spin. Test at updater.test.ts:580.
  • 5 (refused never cleared): resolved, egress.ts:117. Its test does not pin the line (finding 5).
  • 6 (index.ts wiring untested): not applied. The body discloses it accurately.
  • 7 (out-of-range IPv4, .., -): resolved. isProxyHost (schemas.ts:1182-1188) refuses them, and proxyFailure matches ERR_NO_SUPPORTED_PROXIES (backgroundRenderer.ts:374). One new gap remains (finding 2).
  • 8 (types.ts comment): resolved. types.ts:758-759 is accurate.
  • 9 (seeded ref): resolved. The seen ref compares the stored value as JSON (OperatorConfig.tsx:280-294). The test is at SettingsMaintenance.test.tsx:494.
#severityfile:linefinding
1advisorysrc/main/services/egress.ts:108-126Completions are matched by key, not by call. The sequence is X in flight, then change to Y, then back to X. X's first apply resolving sets applied = X, applying = null while the Y and second X calls are still pending, so the gate is open while Chromium may be on Y. If the second X apply is then refused, the gate stays open while the session carries Y. This predates round 3 (762d4914 has the same callback).
2advisorysrc/shared/schemas.ts:1184-1187Zero-padded IPv4 parts pass, and Chromium reads them as octal. In an Electron 44 probe, socks5://010.0.0.1:9050 resolved to SOCKS5 8.0.0.1:9050 and socks5://0127.0.0.1:9050 to SOCKS5 87.0.0.1:9050. For socks5:// the stored and shown value keeps 010.0.0.1, and the render failure names that address. http:// is normalised by URL itself (to 8.0.0.1).
3advisorysrc/main/index.ts:676-698; src/main/services/egress.ts:8-9; src/renderer/components/settings/OperatorConfig.tsx:351-352The gate is onBeforeRequest, so it does not reach WebRTC from a page that is already open. The WebRTC policy follows events and onApplied only. Two cases follow. After an out-of-app change from Direct to a proxy, it stays default until a later request makes the session apply, or for good if that apply is refused. With the file unreadable after Direct, it stays default. The claims are index.ts:676 "re-set on every change", egress.ts:8-9 "nothing goes out", and the notice "sends nothing on these routes".
4advisorysrc/renderer/components/settings/OperatorConfig.tsx:346-347; src/main/services/pdfExport.ts:252-260The new string says PDF export "can fetch that page's images directly". The pdf-export partition has no request filter and does not turn JavaScript off, so every remote subresource a legacy .html page references goes direct, and so would any script request if the page carries scripts (not confirmed). The body's "remote subresources" is accurate.
5advisorytests/main/services/egress.test.ts:270-290; src/main/services/egress.ts:117The retry test drives the retry through an explicit sync, which never consults refused. With refused = null deleted, the same steps give the same results, as traced by reading. The behaviour the line exists for does change. A sequence probe (X refused, Y applied, then the file says X with no event) opens at head and stays closed for good without the line.
6advisoryPR body, VerificationThe block records status=fail (1 failed, no diff coverage) without naming the test. The cause is in a reply only: staging.test.ts:331 relies on chmod 0o555 refusing an unlink, which root ignores. That file is not in the diff, and the local and CI runs above both pass.
7advisorysrc/shared/schemas.ts:1173-1181parseProxyUrl's doc comment ("The one definition of a usable Egress proxy address…") now runs straight into isProxyHost's comment above isProxyHost, so it no longer sits on the function it describes.
8advisorysrc/main/services/settings.ts:183-185A settings.json deleted while the app runs reads as DEFAULT_SETTINGS, whose Egress is Direct (:61). Every route then goes direct with no notice. The Summary's "if the saved choice cannot be read they send nothing at all" does not cover a missing file. This predates round 3 and I did not confirm a flow that deletes the file.

Failure scenario for 1. The model check found the shortest form: start on X with its apply in flight, change in-app to Direct, change back to X, then X's first apply resolves. allowsRequests() returns true with two calls still pending (D, X). The second form, X ok, D ok, then X refused, leaves the gate open on X while the session carries Direct. Reachability: overlapping applies need two in-app changes within one setProxy latency. An Electron 44 probe measured setProxy at a median of 0.01 ms and a maximum of 8 ms over 40 calls. One gesture produces at most two updates (address blur, then preset click), and two updates starting from an applied route resolve correctly. The fix is a per-call token compared in both callbacks, as egressApplied === applying already does in updater.ts:233.

Failure scenario for 2. An Operator copies a zero-padded address, socks5://010.0.0.1:9050. parseProxyUrl returns it unchanged, and the Settings field and the stored file show 010.0.0.1. Chromium connects to 8.0.0.1:9050, a public address. Usually nothing listens there and the render fails, naming 010.0.0.1. If a SOCKS5 server does answer there, the capture goes through a third party, and the record says proxy. The fix is to refuse a numeric part with a leading zero, or to normalise it before storing.

Failure scenario for 3. A Wayback replay is open on Direct. settings.json is changed to Tor by a hand edit or a config tool, with no in-app write. An archived script that gathers ICE candidates against a STUN server keeps sending UDP from the Operator's address. That lasts until the page issues an HTTP request (which triggers sync, the apply and onApplied) or a new guest is created (index.ts:376 reads the file). Round 1's probe showed this traffic is invisible to onBeforeRequest. Already-open connections are the same family. This is advisory and not blocking for three reasons. It needs an out-of-app change, which round 2 established no app writer makes. The window normally ends at the page's next request. And with the file unreadable after Direct, the traffic still takes the route the Operator last chose.

Other checks, sound:

  • sync's new early return applying === null && applied === key (egress.ts:108) skips only re-applies of the route the session already carries. These are label-only changes and unavailable-then-same events.
  • onApplied fires only for the call that is still current.
  • New guests take the policy from the file (index.ts:376), so an unreadable file gives disable_non_proxied_udp.
  • Per-request settings read: allowsRequests now calls getSettings() on every replay request, which is existsSync, readFileSync, JSON.parse and a zod parse. Measured at 12.8 µs a call on Linux, or about 4 ms for 300 requests. Log volume while unreadable is unchanged, because the gate already re-read on every refused request before this round.
  • OperatorConfig seen ref: the JSON comparison ignores refetches that produce a new object with the same content. A Proxy chosen with no address stays checked until the stored value really changes.
  • isProxyHost: the bracket form, labels, IPv4 range, trailing dot (refused) and punycode (accepted) were run through parseProxyUrl at head.

Evidence gate.

  • Labels: the PR has evidence-affecting and agent-authored, and #1695 has evidence-affecting.
  • Blocking-tier paths in the delta: src/main/index.ts, backgroundRenderer.ts, egress.ts (row at the assessment's line 216), OperatorConfig.tsx.
  • Advisory-tier paths in the delta, each with the region touched:
    • updater.ts: update routing only, not evidence.
    • schemas.ts: the proxy host check. It decides which address the signed egressKind route goes through, and the PR label covers it.
    • types.ts: one log-code comment.
  • The Evidence impact section is substantive and now quotes the verifier sentence correctly.
  • Known-answer coverage: the delta changes no hashing, signing, manifest or verifier code. git diff origin/main...HEAD -- src/shared/verify is empty, and the delta does not touch manifest.ts, captureLifecycle.ts or recapture.ts.
  • Backward verification: preserved. The full suite's verify tests pass.

Completion controls.

  • Attribution line: Pull request description generated by Claude Code is the last authored line of the body. Pass.
  • Co-author trailers: a case-insensitive grep of git log origin/main..HEAD --format=%B (3 commits) for a Claude co-author trailer exited 1 with count 0. Pass.
  • auto_merge: null.
  • Head re-read at the end (gh api …/pulls/1896 --jq .head.sha): 4d5674734d07e43335dd2b888b8ff22977cc6f7b, unchanged.

Convergence note for the dispatcher. createRoutedSession has drawn findings in rounds 2 and 3. This round's (finding 1) predates round 2's fix, is advisory, and is not reachable at human speed. It is in the same construct, so the section 4 convergence question is the maintainer's to weigh.

Not checked.

  • The two manual checks: a Tor Browser recapture, and a packaged update check against a stopped proxy.
  • WebRTC and DNS prefetch from a replay page in the running app. Finding 3 is from reading and the model, not a live probe.
  • The cost of the per-request settings read on Windows with on-access antivirus scanning.
  • The macOS and Windows updater and spellcheck paths.

Probes ran under Node 20 and, for the proxy-rule reads and setProxy timing, under Electron 44 through xvfb-run. No shell script or workflow changed, so no bash-semantics check applied.

Ggithub-actionsBot10d ago
Synced from GitHub

Merge request refused: GitHub would not enable auto-merge; read the pull request's merge box

Remove the cause, then apply the merge label again.