Skip to content

Commit 37ade98

Browse files
joshavantjoshavant
committed
fix: keep firewall inspection off critical flows
1 parent c061710 commit 37ade98

10 files changed

Lines changed: 313 additions & 104 deletions

‎src/cli/daemon-cli/status.gather.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ describe("gatherDaemonStatus", () => {
373373
});
374374

375375
expect(inspectWindowsGatewayFirewall).toHaveBeenCalledWith(
376-
expect.objectContaining({ bind: "lan", port: 19001 }),
376+
expect.objectContaining({ bind: "lan", mode: "quick", port: 19001 }),
377377
);
378378
expect(status.gateway?.windowsFirewall).toMatchObject({
379379
severity: "warning",

‎src/cli/daemon-cli/status.gather.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,7 @@ export async function gatherDaemonStatus(
609609
opts.deep === true && shouldInspectLocalGateway
610610
? await inspectWindowsGatewayFirewall({
611611
bind: gateway.bindMode,
612+
mode: "quick",
612613
port: daemonPort,
613614
platform: process.platform,
614615
})

‎src/commands/configure.wizard.test.ts‎

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -94,16 +94,10 @@ vi.mock("../infra/control-ui-assets.js", () => ({
9494

9595
vi.mock("../infra/windows-gateway-firewall-diagnostics.js", () => ({
9696
inspectWindowsGatewayFirewall: mocks.inspectWindowsGatewayFirewall,
97-
formatWindowsGatewayFirewallDiagnostic: (diagnostic: {
98-
applies: boolean;
99-
severity: string;
100-
message: string;
101-
details: string[];
102-
}) =>
103-
diagnostic.applies && diagnostic.severity === "warning"
97+
formatWindowsGatewayFirewallGuidance: (params: { bind?: string }) =>
98+
params.bind === "lan"
10499
? [
105-
`Windows firewall: ${diagnostic.message}`,
106-
...diagnostic.details.map((line) => ` ${line}`),
100+
"Windows firewall: if another device cannot connect to the LAN URL, run `openclaw gateway status --deep` from this Windows host.",
107101
]
108102
: [],
109103
}));
@@ -433,33 +427,20 @@ describe("runConfigureWizard", () => {
433427
);
434428
});
435429

436-
it("warns when Windows Firewall may block configured LAN Gateway links", async () => {
430+
it("shows static Windows Firewall guidance for LAN Gateway links without inspection", async () => {
437431
setupBaseWizardState({
438432
gateway: {
439433
mode: "local",
440434
bind: "lan",
441435
auth: { token: "token" },
442436
},
443437
});
444-
mocks.inspectWindowsGatewayFirewall.mockResolvedValueOnce({
445-
applies: true,
446-
severity: "warning",
447-
code: "windows_firewall_local_rules_ignored",
448-
message: "Windows Firewall may ignore local Gateway allow rules for this network profile.",
449-
details: ["Windows reports LocalFirewallRules as N/A (GPO-store only)."],
450-
});
451438

452439
await runConfigureWizard({ command: "configure", sections: ["gateway"] }, createRuntime());
453440

454-
expect(mocks.inspectWindowsGatewayFirewall).toHaveBeenCalledWith(
455-
expect.objectContaining({ bind: "lan", port: 18789 }),
456-
);
457-
expect(mocks.note).toHaveBeenCalledWith(
458-
expect.stringContaining("Windows firewall: Windows Firewall may ignore"),
459-
"Control UI",
460-
);
441+
expect(mocks.inspectWindowsGatewayFirewall).not.toHaveBeenCalled();
461442
expect(mocks.note).toHaveBeenCalledWith(
462-
expect.stringContaining("GPO-store only"),
443+
expect.stringContaining("Windows firewall: if another device cannot connect to the LAN URL"),
463444
"Control UI",
464445
);
465446
});

‎src/commands/configure.wizard.ts‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,7 @@ import { logConfigUpdated } from "../config/logging.js";
1818
import { ConfigMutationConflictError } from "../config/mutate.js";
1919
import type { OpenClawConfig } from "../config/types.openclaw.js";
2020
import { ensureControlUiAssetsBuilt } from "../infra/control-ui-assets.js";
21-
import {
22-
formatWindowsGatewayFirewallDiagnostic,
23-
inspectWindowsGatewayFirewall,
24-
} from "../infra/windows-gateway-firewall-diagnostics.js";
21+
import { formatWindowsGatewayFirewallGuidance } from "../infra/windows-gateway-firewall-diagnostics.js";
2522
import { resolvePluginContributionOwners } from "../plugins/plugin-registry.js";
2623
import type { RuntimeEnv } from "../runtime.js";
2724
import { defaultRuntime } from "../runtime.js";
@@ -888,12 +885,7 @@ export async function runConfigureWizard(
888885
const gatewayStatusLine = gatewayProbe.ok
889886
? "Gateway: reachable"
890887
: `Gateway: not detected${gatewayProbe.detail ? ` (${gatewayProbe.detail})` : ""}`;
891-
const windowsFirewallLines = formatWindowsGatewayFirewallDiagnostic(
892-
await inspectWindowsGatewayFirewall({
893-
bind,
894-
port: gatewayPort,
895-
}),
896-
);
888+
const windowsFirewallLines = formatWindowsGatewayFirewallGuidance({ bind });
897889

898890
note(
899891
[

‎src/commands/gateway-status.test.ts‎

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -389,36 +389,24 @@ describe("gateway-status command", () => {
389389
requireRecord(firstTarget.summary, "first target summary");
390390
});
391391

392-
it("includes Windows LAN firewall diagnostics in gateway status warnings", async () => {
392+
it("does not run Windows LAN firewall diagnostics during fast gateway status", async () => {
393393
readBestEffortConfig.mockResolvedValueOnce({
394394
gateway: {
395395
mode: "local",
396396
bind: "lan",
397397
auth: { token: "ltok" },
398398
},
399399
} as never);
400-
inspectWindowsGatewayFirewall.mockResolvedValueOnce({
401-
applies: true,
402-
severity: "warning",
403-
code: "windows_firewall_local_rules_ignored",
404-
message: "Windows Firewall may ignore local Gateway allow rules for this network profile.",
405-
details: ["Windows reports LocalFirewallRules as N/A (GPO-store only)."],
406-
});
407400
const { runtime, runtimeLogs } = createRuntimeCapture();
408401

409402
await runGatewayStatus(runtime, { timeout: "1000", json: true });
410403

411-
expect(inspectWindowsGatewayFirewall).toHaveBeenCalledWith(
412-
expect.objectContaining({ bind: "lan", port: 18789 }),
413-
);
404+
expect(inspectWindowsGatewayFirewall).not.toHaveBeenCalled();
414405
const parsed = JSON.parse(runtimeLogs.join("\n")) as {
415-
warnings: Array<{ code: string; details?: string[] }>;
406+
warnings: Array<{ code?: string }>;
416407
};
417-
expect(parsed.warnings).toContainEqual(
418-
expect.objectContaining({
419-
code: "windows_firewall_local_rules_ignored",
420-
details: ["Windows reports LocalFirewallRules as N/A (GPO-store only)."],
421-
}),
408+
expect(parsed.warnings.some((warning) => warning.code?.startsWith("windows_firewall_"))).toBe(
409+
false,
422410
);
423411
});
424412

‎src/commands/gateway-status.ts‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import { parseGatewayPortOption } from "../cli/gateway-port-option.js";
44
import { withProgress } from "../cli/progress.js";
55
import { readBestEffortConfig, resolveGatewayPort } from "../config/config.js";
66
import { resolveWideAreaDiscoveryDomain } from "../infra/widearea-dns.js";
7-
import { inspectWindowsGatewayFirewall } from "../infra/windows-gateway-firewall-diagnostics.js";
87
import type { RuntimeEnv } from "../runtime.js";
98
import { createLazyImportLoader } from "../shared/lazy-promise.js";
109
import { inferSshTargetFromRemoteUrl, resolveSshTarget } from "./gateway-status/discovery.js";
@@ -140,23 +139,6 @@ export async function gatewayStatusCommand(
140139
? (localTlsRuntime.error ?? "gateway tls is enabled but local TLS runtime could not load")
141140
: null,
142141
});
143-
const shouldInspectLocalGatewayFirewall =
144-
!hasExplicitUrl && (portOverride !== undefined || cfg.gateway?.mode !== "remote");
145-
const windowsFirewall = shouldInspectLocalGatewayFirewall
146-
? await inspectWindowsGatewayFirewall({
147-
bind: cfg.gateway?.bind,
148-
port: remotePort,
149-
platform: process.platform,
150-
})
151-
: undefined;
152-
if (windowsFirewall?.applies && windowsFirewall.severity === "warning") {
153-
warnings.push({
154-
code: windowsFirewall.code,
155-
message: windowsFirewall.message,
156-
details: windowsFirewall.details,
157-
targetIds: ["localLoopback"],
158-
});
159-
}
160142
const primary = pickPrimaryProbedTarget(probePass.probed);
161143

162144
if (opts.json) {

‎src/infra/windows-gateway-firewall-diagnostics.test.ts‎

Lines changed: 82 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import { describe, expect, it, vi } from "vitest";
33
import {
44
DEFAULT_WINDOWS_GATEWAY_FIREWALL_TIMEOUT_MS,
5+
QUICK_WINDOWS_GATEWAY_FIREWALL_TIMEOUT_MS,
56
inspectWindowsGatewayFirewall,
67
parseWindowsGatewayFirewallState,
78
classifyWindowsGatewayFirewallState,
@@ -103,6 +104,18 @@ function ruleJson(params?: {
103104
return JSON.stringify([ruleRow(params)]);
104105
}
105106

107+
function quickPayloadJson(params?: {
108+
state?: string;
109+
activeRules?: Array<Record<string, unknown>>;
110+
localRules?: Array<Record<string, unknown>>;
111+
}) {
112+
return JSON.stringify({
113+
State: JSON.parse(params?.state ?? stateJson({ localAllowRules: "True" })),
114+
ActiveRules: params?.activeRules ?? [],
115+
LocalRules: params?.localRules ?? [ruleRow()],
116+
});
117+
}
118+
106119
function rulesPayloadJson(params: { active?: unknown[]; local?: unknown[] }) {
107120
return JSON.stringify({
108121
ActiveRules: params.active ?? [],
@@ -527,7 +540,75 @@ describe("Windows Gateway firewall diagnostics", () => {
527540
});
528541
});
529542

530-
it("runs bounded read-only Windows probes for LAN binding", async () => {
543+
it("runs a quick bounded Windows probe without netsh or follow-up commands", async () => {
544+
const runner = vi.fn<WindowsGatewayFirewallCommandRunner>(async (argv) => {
545+
const command = argv.join(" ");
546+
expect(command).toContain("Get-NetConnectionProfile");
547+
expect(command).toContain("HNetCfg.FwPolicy2");
548+
expect(command).toContain("Get-NetFirewallRule");
549+
expect(command).toContain("PolicyStore ActiveStore");
550+
expect(command).toContain("foreach ($entry in @($value))");
551+
expect(command).not.toContain("advfirewall");
552+
expect(command).not.toContain("PolicyStore PersistentStore");
553+
return { code: 0, stdout: quickPayloadJson() };
554+
});
555+
556+
await expect(
557+
inspectWindowsGatewayFirewall({
558+
bind: "lan",
559+
mode: "quick",
560+
port: 18789,
561+
platform: "win32",
562+
runCommandWithTimeout: runner,
563+
}),
564+
).resolves.toMatchObject({
565+
code: "windows_firewall_rule_present",
566+
});
567+
expect(runner).toHaveBeenCalledTimes(1);
568+
expect(runner.mock.calls[0]?.[0][0]).toBe(getWindowsPowerShellExePath());
569+
expect(runner.mock.calls[0]?.[1]).toMatchObject({
570+
timeoutMs: QUICK_WINDOWS_GATEWAY_FIREWALL_TIMEOUT_MS,
571+
});
572+
});
573+
574+
it("preserves managed ActiveStore allow rules during quick inspection", async () => {
575+
const runner = vi.fn<WindowsGatewayFirewallCommandRunner>(async (argv) => {
576+
const command = argv.join(" ");
577+
expect(command).toContain("Get-NetFirewallRule");
578+
expect(command).toContain("GroupPolicy");
579+
expect(command).toContain("MDM");
580+
return {
581+
code: 0,
582+
stdout: quickPayloadJson({
583+
state: stateJson({ activeAllowLocalRules: "False" }),
584+
activeRules: [
585+
ruleRow({
586+
displayName: "MDM-managed Gateway allow",
587+
policyStoreSource: "Intune",
588+
policyStoreSourceType: "MDM",
589+
}),
590+
],
591+
localRules: [ruleRow({ displayName: "Ignored local allow" })],
592+
}),
593+
};
594+
});
595+
596+
await expect(
597+
inspectWindowsGatewayFirewall({
598+
bind: "lan",
599+
mode: "quick",
600+
port: 18789,
601+
platform: "win32",
602+
runCommandWithTimeout: runner,
603+
}),
604+
).resolves.toMatchObject({
605+
severity: "info",
606+
code: "windows_firewall_rule_present",
607+
});
608+
expect(runner).toHaveBeenCalledTimes(1);
609+
});
610+
611+
it("runs bounded read-only full Windows probes for LAN binding", async () => {
531612
const runner = vi.fn<WindowsGatewayFirewallCommandRunner>(async (argv) => {
532613
const command = argv.join(" ");
533614
if (command.includes("Get-NetConnectionProfile")) {

0 commit comments

Comments
 (0)