You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "ying-hua (via GitHub)" <gi...@apache.org> on 2026/09/10 14:02:57 UTC

[PR] fix(bots): apply the configured network proxy to Client-owned bot traffic [maka]

ying-hua opened a new pull request, #5159:
URL: https://github.com/apache/maka/pull/5159

   ## Summary
   
   `setActiveProxy` had no production caller anywhere in the repository, so `resolveActiveProxy()` always returned `null` and the bot bridges' `proxiedFetch` never built a proxy dispatcher. Every bot request went direct whatever the Network settings said. On a blocked network that surfaces as the `Fetch timeout` in the report, and it explains why enabling TUN was the only thing that worked — TUN intercepts below the application, so it needs no cooperation from this seam.
   
   Model execution was never affected: it resolves the proxy inside the Host and injects a transport per connection. The bot bridges cannot do that. `BotRegistry` is constructed in the Client process, the local Host is a separate spawned process, and the resolved proxy — including its credential — only exists Host-side. That is the actual gap; the protocol exposed only `network-proxy.test`.
   
   So this adds `network-proxy.resolve`, served by `HostNetworkProxyCoordinator` from the existing `resolveNetworkProxyExecution()` and `toRuntimePolicyProxy()`, and applies the result in the Client at Host registration and whenever a settings patch writes the proxy policy.
   
   Two decisions worth flagging:
   
   - **A non-local Host's policy is never adopted.** The bot bridges dial out from the Client machine, so a `remote` or `environment` Host describes the wrong network. Those profiles are not even asked — the Client stays direct rather than guessing.
   - **The first resolution can lose a race with Host readiness.** Rather than settling on direct until the user next edits the proxy, a failed resolution keeps the last applied value and retries on a bounded 1s/5s/15s budget.
   
   `RUNTIME_HOST_COMPATIBILITY_EPOCH` goes to 141. Two builds can share an epoch and differ in operations, so without the bump a Client could reach an already-running older local Host that cannot answer the new operation — and a Client cannot tell that refusal apart from a proxy that is genuinely unset.
   
   The fix covers every bot platform, not just Telegram — `telegram`, `discord`, `qq`, `dingtalk` and `wechat` bridges, credential verification in `bot-test.ts`, and the Desktop onboarding and QQ scan-login paths all share `proxiedFetch`.
   
   Fixes #5091
   
   ## Verification
   
   Reproduced and verified on Windows 11 with Clash, TUN **off** and system proxy on at `127.0.0.1:7897`. `HTTP_PROXY`/`HTTPS_PROXY` were stripped from the process to match a normal desktop launch.
   
   Driving the real chain end to end — Host coordinator → protocol codec → applier → `setActiveProxy` → the bots' `proxiedFetch` → a live request to `api.telegram.org`:
   
   ```text
   env HTTPS_PROXY = (unset)
   A. before any refresh (today's shipped behaviour): FAILED in 10010ms -> Fetch timeout
   B. after refresh with a remote Host:               FAILED in 10009ms -> Fetch timeout
   B. after refresh with a local Host:                HTTP 200 in 2567ms
   ```
   
   A is `main` today, and its error string and 10s duration match the report. The middle line confirms the non-local gate holds. Only the local Host applies the proxy.
   
   Checks run locally:
   
   - `npm run lint`, `npm run format:check`, `npm run check:asf-headers` — clean
   - `npm run typecheck` — clean across all workspaces
   - `npx knip` — no new findings
   - Affected suites, 117 passing: the new `network-proxy-coordinator` and `client-network-proxy` suites, plus `runtime-host-settings-ipc-main`, `proxied-fetch` and `protocol`
   
   Not run: the full workspace suite and the Desktop E2E specs. `handshake-compatibility.test.ts` fails 4 of 6 on this machine, but it fails identically on unmodified `upstream/main` (a local Windows transport-teardown issue), so it is not evidence about this change.
   
   ## Security
   
   `network-proxy.resolve` sends the proxy credential from the Host to the Client. That is new, and SECURITY.md calls out `proxiedFetch` specifically, so it deserves explicit review. The reasoning: a Client that runs its own outbound traffic cannot dial an authenticated proxy without the secret, and the alternative — a separate Client-side proxy setting — duplicates configuration the user has already entered. The failure path returns a fixed message rather than the underlying error, which can carry credential material, and there is a test for that.
   
   If you would rather keep the secret Host-side, the alternative is a dedicated Client-owned bot proxy setting (option 1 in the report). Happy to switch.
   
   ## AI use
   
   Select exactly one:
   
   - [ ] No generative tool made a substantive contribution
   - [x] Generative tooling made a substantive contribution
   
   Tool(s) and scope: Claude Code — root-cause investigation, the implementation, and the tests, under my direction and review. Commits carry `Generated-by: Claude Code`.
   
   ## Checklist
   
   - [x] Tests cover the change and fail without it
   - [x] Lint, format, typecheck and the affected suites pass locally
   
   Does this PR entail a change in behavior?
   
   - [x] Yes — described under Summary above
   - [ ] No
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org


Re: [PR] fix(bots): apply the configured network proxy to Client-owned bot traffic [maka]

Posted by "ying-hua (via GitHub)" <gi...@apache.org>.
ying-hua commented on PR #5159:
URL: https://github.com/apache/maka/pull/5159#issuecomment-6523236837

   Rebased onto current `main` (`85a1031c1`) and addressed the open review.
   
   **Epoch.** 199 has since been taken on `main`, which is now at 237, so this PR takes **238**.
   
   **[P2] `credential_not_configured` going direct (@hqhq1025).** Agreed: when the policy still asks for an authenticated proxy, that state should not be treated as "proxy disabled". Fixed in c1911d077:
   
   - `active-proxy-state` gains a blocked state (`blockActiveProxy(reason)`, cleared by the next `setActiveProxy`). While it is set, `proxiedFetch` throws `ActiveProxyBlockedError` before any dispatcher or socket is created.
   - The Client applier enters it on `credential_not_configured` and reports once on the redacted `[bots:proxy]` channel as `proxy_credential_missing`. Saving the credential is a `network.proxy` patch, so the refresh it triggers unblocks traffic. Removing a saved credential while authentication stays enabled blocks it again.
   - The test that asserted the direct fallback now asserts the block. New tests cover: a repeated block is reported once; the block lifts once the credential is configured and returns if it is removed; and a real local server receives no request while blocked.
   
   Affected suites pass locally (desktop proxy and settings IPC 27, runtime proxy/network 83, coordinator 9, protocol 93). Runtime and runtime-host typechecks, biome and `git diff --check` are clean.
   
   Scope note: the Feishu, WeCom and Slack SDK clients and the bot WebSocket connections never went through `proxiedFetch`, so neither this PR nor the block covers them. That gap predates this PR.
   
   Still open for a maintainer: whether `network-proxy.resolve` may carry the proxy credential across the Host↔Client boundary (see SECURITY.md §2.3.6). If not, I'll switch to a Client-owned bot proxy setting.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org


Re: [PR] fix(bots): apply the configured network proxy to Client-owned bot traffic [maka]

Posted by "hqhq1025 (via GitHub)" <gi...@apache.org>.
hqhq1025 commented on code in PR #5159:
URL: https://github.com/apache/maka/pull/5159#discussion_r4131702587


##########
apps/desktop/src/main/client-network-proxy.ts:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Applies the Runtime Policy network proxy to the Client's own outbound
+ * traffic.
+ *
+ * Model execution resolves the proxy inside the Host and injects a transport
+ * per connection. The bot bridges do not: `BotRegistry` is constructed in this
+ * process, so its `proxiedFetch` reads process-wide state that only this module
+ * writes. Without it the seam stays empty and every bot request goes direct,
+ * whatever the Network settings say (apache/maka#5091).
+ */
+
+import type { ProxySettings } from "@maka/core/settings/network-settings";
+import type { RuntimeHostProfileKind } from "@maka/runtime-host/profile-kind";
+import type { NetworkProxyResolveResult } from "@maka/runtime-host/protocol";
+import { setActiveProxy } from "@maka/runtime/network/active-proxy-state";
+
+/**
+ * The first resolution runs while the Host connection is still settling, so a
+ * `host_not_ready` refusal is expected rather than terminal.
+ *
+ * The ladder backs off but never runs out: while the policy asks for a proxy
+ * and this Client cannot obtain it, bot traffic is going direct, past the
+ * network policy the user configured. Giving up would leave that in place
+ * until the next proxy edit or restart, so the last delay repeats instead.
+ */
+const RETRY_DELAYS_MS = [1_000, 5_000, 15_000, 60_000] as const;
+
+export interface ClientNetworkProxyDeps {
+  readonly profileKind: RuntimeHostProfileKind;
+  readonly resolve: () => Promise<NetworkProxyResolveResult>;
+  readonly apply?: (proxy: ProxySettings | null) => void;
+  readonly onError?: (error: unknown) => void;
+  readonly schedule?: (run: () => void, delayMs: number) => void;
+}
+
+export interface ClientNetworkProxyApplier {
+  /** Re-resolves and applies. Safe to call concurrently; calls are serialized. */
+  refresh(): Promise<void>;
+}
+
+export function createClientNetworkProxyApplier(
+  deps: ClientNetworkProxyDeps,
+): ClientNetworkProxyApplier {
+  const apply = deps.apply ?? setActiveProxy;
+  const schedule =
+    deps.schedule ??
+    ((run, delayMs) => {
+      setTimeout(run, delayMs).unref?.();
+    });
+  let lane: Promise<void> = Promise.resolve();
+  let lastReportedError: string | undefined;
+  let attempt = 0;
+  // One retry in flight at a time: an explicit refresh that also fails must not
+  // fan out into a second ladder.
+  let retryPending = false;
+
+  const refreshWithoutLane = async (): Promise<void> => {
+    // A non-local Host describes a different machine's network. The bot
+    // bridges dial out from this one, so its proxy policy does not apply and
+    // guessing would be worse than staying direct.
+    if (deps.profileKind !== "local") {
+      apply(null);
+      return;
+    }
+    let resolved: NetworkProxyResolveResult;
+    try {
+      resolved = await deps.resolve();
+    } catch (error) {
+      // Keep the last applied proxy. A Host that is briefly unreachable is not
+      // evidence that the user wants direct connections.
+      const message = error instanceof Error ? error.message : String(error);
+      if (message !== lastReportedError) {
+        lastReportedError = message;
+        deps.onError?.(error);
+      }
+      const delayMs =
+        RETRY_DELAYS_MS[Math.min(attempt, RETRY_DELAYS_MS.length - 1)] ?? 60_000;
+      attempt += 1;
+      if (!retryPending) {
+        retryPending = true;
+        schedule(() => {
+          retryPending = false;
+          void enqueue();
+        }, delayMs);
+      }
+      return;
+    }
+    lastReportedError = undefined;
+    attempt = 0;
+    apply(resolved.kind === "ready" ? (resolved.proxy ?? null) : null);

Review Comment:
   [P2] Keep `credential_not_configured` distinct from a disabled proxy. The Host returns this state specifically when policy requires proxy authentication but the secret is missing (`network-proxy-coordinator.ts:87-89`). Mapping it to `null` makes `proxiedFetch` omit the dispatcher and send bot traffic directly, including after a previously saved credential is deleted while authentication remains enabled. The new test currently asserts that fallback. Block Client-owned outbound requests until the credential is configured, rather than silently bypassing the user's enabled proxy policy.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org


Re: [PR] fix(bots): apply the configured network proxy to Client-owned bot traffic [maka]

Posted by "ying-hua (via GitHub)" <gi...@apache.org>.
ying-hua commented on PR #5159:
URL: https://github.com/apache/maka/pull/5159#issuecomment-5886968335

   Thanks — both findings were fair. Rebased onto current `main` (the branch was 365 commits behind and conflicting) and addressed them.
   
   **[P2] draft / no `test` lane.** Marked ready for review, so the required gate can run. The epoch also had to be re-taken: I had claimed 141, which `main` has since used for WorkHub root admissions, so it is now **199**.
   
   **[P3] silent exhaustion.** Agreed, and the diagnosis was sharper than "add a log" — the real defect was that the ladder *ended*. While the policy asks for a proxy this Client cannot obtain, bot traffic is going direct past the user's network policy, and no amount of reporting fixes that; stopping the retries is what leaves it in place until the next proxy edit or restart. So the last delay (60s) now repeats instead of the budget running out, with one retry held in flight so a failing explicit refresh cannot fan out into a second ladder.
   
   On the surface question: `onError` was indeed only wired to `console.error`. It now reports on the same redacted `[bots:*]` channel that `settings-ipc-helpers.ts` already uses for bot connection failures — the channel whose output the reporter pasted into #5091 — so the proxy failure and the `Fetch timeout` lines it causes appear together in an attached log:
   
   ```
   [bots:proxy] proxy_unresolved: bot traffic stays direct until the Runtime Host answers — <redacted>
   ```
   
   I stopped short of a toast or a `BotStatus.readiness` state: `BotRegistry` has no API to accept an external warning, and adding one would pull in the bots package, the status shape, the settings page and its i18n copy. Happy to do it as a follow-up if you would rather the degraded state be visible in-product rather than in the support log.
   
   Verification was also redone against the rebased head. The decisive case points the policy at a port with nothing listening — a request that still succeeds there is one that leaked past the proxy:
   
   ```text
   A. no refresh — unwired seam, request goes direct: HTTP 200 in 3612ms
   B. proxy 127.0.0.1:1 (nothing listening)         : FAILED in 5ms -> fetch failed
   C. same dead proxy, but a remote Host            : HTTP 200 in 986ms
   D. proxy 127.0.0.1:7897 (Clash)                  : HTTP 200 in 4200ms
   ```
   
   A is `main` today, B shows traffic is genuinely routed through the configured proxy, C shows the non-local gate holds, D shows a working proxy is used. Affected suites are at 127 passing; lint, format, ASF headers and the relevant typechecks are clean. Details and the local-environment caveats are in the updated description.
   
   Still unanswered from #5091, and the one thing I would not want to merge without a maintainer view: `network-proxy.resolve` sends the proxy credential across the Host↔Client boundary, and SECURITY.md names `proxiedFetch` specifically. If that is not acceptable, the alternative is a dedicated Client-owned bot proxy setting and I will switch.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org


Re: [PR] fix(bots): apply the configured network proxy to Client-owned bot traffic [maka]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5159:
URL: https://github.com/apache/maka/pull/5159#issuecomment-6514422845

   <!-- maka-lifecycle:stale-warning -->
   This pull request has had no new commits for 30 days and has been marked stale. It will be closed in 7 days unless a new commit is pushed.
   
   Comments do not reset this timer: only a new commit does. If the pull request is intentionally long-lived, a maintainer can apply the `pinned` label.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org