Skip to content

Commit 3e6f749

Browse files
clawsweeper[bot]MarvaeTakhoffman
authored
fix(browser): preserve explicit cdpPort when cdpUrl omits port (#83707)
Summary: - The PR adds raw explicit-port detection for browser CDP URLs, updates profile resolution precedence, adds regression tests, and records the browser fix in the changelog. - Reproducibility: yes. Source inspection shows current main resolves a portless profile `cdpUrl` through `par ... 443, and overwrites the configured `cdpPort`; the source PR also provides live before/after Chrome output. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(browser): encapsulate explicit-port detection in parseBrowserHttpUrl - PR branch already contained follow-up commit before automerge: fix(browser): preserve explicit cdpPort when cdpUrl omits port Validation: - ClawSweeper review passed for head 070c31c. - Required merge gates passed before the squash merge. Prepared head SHA: 070c31c Review: #83707 (comment) Co-authored-by: Hongwei Ma <[email protected]> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <[email protected]>
1 parent 78f3985 commit 3e6f749

4 files changed

Lines changed: 284 additions & 18 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ Docs: https://docs.openclaw.ai
4848

4949
- Browser: enforce current-tab URL allowlist checks for `/act` evaluate/batch actions and `/highlight` routes while leaving tab-management actions unblocked. (#78523)
5050
- CI: require real-behavior-proof verdict markers to come from the ClawSweeper GitHub App before accepting exact-head proof. (#83692)
51+
- Browser: keep a profile `cdpPort` when its `cdpUrl` omits a port, while still letting explicitly written URL ports win. (#82166) Thanks @Marvae.
5152
- Agents/image generation: allow distinct `image_generate` prompts to start separate session-backed background tasks while same-prompt retries still return the active task status. (#83614) Thanks @Elarwei001.
5253
- Control UI: stop the chat reading indicator from sticking after an assistant response finishes. (#83515) Thanks @njuboy11.
5354
- Skills: reject empty or whitespace-only skill names and descriptions during quick validation. (#27061)

‎extensions/browser/src/browser/cdp.helpers.ts‎

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,35 @@ import { withAllowedHostname } from "./ssrf-policy-helpers.js";
1515

1616
export { isLoopbackHost };
1717

18+
/**
19+
* Detects whether a raw URL string contains an explicitly written port.
20+
*
21+
* WHATWG `URL` normalizes default ports (e.g. `:80` for http, `:443` for
22+
* https) to an empty `.port` string, making it impossible to distinguish
23+
* "user wrote :80" from "user omitted the port". This helper inspects the
24+
* raw string to preserve that intent.
25+
*
26+
* Handles IPv6 bracket notation and userinfo (user:pass@host) correctly.
27+
*/
28+
function hasRawExplicitPort(raw: string): boolean {
29+
// Strip scheme (e.g. "http://") and take only the authority portion
30+
// (everything before the first /, ?, or #).
31+
const authority = raw.replace(/^[a-z][a-z0-9+.-]*:\/\//i, "").split(/[/?#]/, 1)[0] ?? "";
32+
33+
// Strip userinfo (user:pass@); the colon there is not a port separator.
34+
const hostPort = authority.includes("@")
35+
? authority.slice(authority.lastIndexOf("@") + 1)
36+
: authority;
37+
38+
// IPv6: [::1]:9222 has a port after the closing bracket.
39+
if (hostPort.startsWith("[")) {
40+
return /^\[[^\]]+\]:\d+$/.test(hostPort);
41+
}
42+
43+
// IPv4 / hostname: host:port
44+
return /:\d+$/.test(hostPort);
45+
}
46+
1847
export function parseBrowserHttpUrl(raw: string, label: string) {
1948
const trimmed = raw.trim();
2049
const parsed = new URL(trimmed);
@@ -40,10 +69,41 @@ export function parseBrowserHttpUrl(raw: string, label: string) {
4069
throw new Error(`${label} has invalid port: ${parsed.port}`);
4170
}
4271

72+
const normalized = parsed.toString().replace(/\/$/, "");
73+
const hasExplicitPort = hasRawExplicitPort(trimmed);
74+
75+
// When the user explicitly wrote a default port (e.g. :80 for http),
76+
// WHATWG normalization drops it from the URL string. Rebuild a
77+
// port-preserving normalized form so callers don't need raw-string hacks.
78+
// Note: the URL .port setter silently discards protocol-default ports,
79+
// so we must inject the port via string surgery on the normalized form.
80+
let normalizedWithPort: string;
81+
if (hasExplicitPort && !parsed.port) {
82+
const proto = parsed.protocol + "//";
83+
const rest = normalized.slice(proto.length);
84+
// Skip userinfo (user:pass@) if present
85+
const atIdx = rest.indexOf("@");
86+
const hostStart = atIdx >= 0 ? atIdx + 1 : 0;
87+
const hostPart = rest.slice(hostStart);
88+
// Find the end of the host: IPv6 brackets, a path slash, or a port colon.
89+
const hostLen = hostPart.startsWith("[")
90+
? hostPart.indexOf("]") + 1
91+
: (() => {
92+
const idx = hostPart.search(/[:/]/);
93+
return idx < 0 ? hostPart.length : idx;
94+
})();
95+
const insertAt = hostStart + hostLen;
96+
normalizedWithPort = proto + rest.slice(0, insertAt) + ":" + port + rest.slice(insertAt);
97+
} else {
98+
normalizedWithPort = normalized;
99+
}
100+
43101
return {
44102
parsed,
45103
port,
46-
normalized: parsed.toString().replace(/\/$/, ""),
104+
hasExplicitPort,
105+
normalized,
106+
normalizedWithPort,
47107
};
48108
}
49109

‎extensions/browser/src/browser/config.test.ts‎

Lines changed: 208 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -556,23 +556,216 @@ describe("browser config", () => {
556556
expect(profile?.cdpIsLoopback).toBe(true);
557557
});
558558

559-
it("prefers cdpPort over stale WebSocket devtools cdpUrl when both are set", () => {
560-
const resolved = resolveBrowserConfig({
561-
profiles: {
562-
"chrome-cdp": {
563-
cdpPort: 9222,
564-
cdpUrl: "ws://127.0.0.1:9222/devtools/browser/old-stale-id",
565-
attachOnly: true,
566-
color: "#F59E0B",
559+
describe("cdpPort vs cdpUrl port precedence", () => {
560+
it("URL with non-default port wins over cdpPort", () => {
561+
const resolved = resolveBrowserConfig({
562+
profiles: {
563+
openclaw: {
564+
cdpPort: 18800,
565+
cdpUrl: "http://127.0.0.1:9222",
566+
color: "#FF4500",
567+
driver: "openclaw",
568+
},
567569
},
568-
},
570+
});
571+
const profile = resolveProfile(resolved, "openclaw");
572+
expect(profile?.cdpPort).toBe(9222);
573+
expect(profile?.cdpUrl).toBe("http://127.0.0.1:9222");
574+
});
575+
576+
it("URL with explicit default port :80 wins over cdpPort", () => {
577+
const resolved = resolveBrowserConfig({
578+
profiles: {
579+
openclaw: {
580+
cdpPort: 18800,
581+
cdpUrl: "http://127.0.0.1:80",
582+
color: "#FF4500",
583+
driver: "openclaw",
584+
},
585+
},
586+
});
587+
const profile = resolveProfile(resolved, "openclaw");
588+
expect(profile?.cdpPort).toBe(80);
589+
expect(profile?.cdpUrl).toBe("http://127.0.0.1:80");
590+
});
591+
592+
it("URL with explicit default port preserves normalized URL details", () => {
593+
const resolved = resolveBrowserConfig({
594+
profiles: {
595+
secure: {
596+
cdpPort: 18800,
597+
cdpUrl: "https://user:[email protected]:443/json/version?token=abc#frag",
598+
color: "#0066CC",
599+
driver: "openclaw",
600+
},
601+
websocket: {
602+
cdpPort: 18800,
603+
cdpUrl: "wss://remote-browser.example.com:443/json/version?token=abc",
604+
color: "#0066CC",
605+
driver: "openclaw",
606+
},
607+
ipv6: {
608+
cdpPort: 18800,
609+
cdpUrl: "http://[::1]:80/json/version?token=abc",
610+
color: "#0066CC",
611+
driver: "openclaw",
612+
},
613+
},
614+
});
615+
616+
const secure = resolveProfile(resolved, "secure");
617+
expect(secure?.cdpPort).toBe(443);
618+
expect(secure?.cdpUrl).toBe(
619+
"https://user:[email protected]:443/json/version?token=abc#frag",
620+
);
621+
622+
const websocket = resolveProfile(resolved, "websocket");
623+
expect(websocket?.cdpPort).toBe(443);
624+
expect(websocket?.cdpUrl).toBe("wss://remote-browser.example.com:443/json/version?token=abc");
625+
626+
const ipv6 = resolveProfile(resolved, "ipv6");
627+
expect(ipv6?.cdpPort).toBe(80);
628+
expect(ipv6?.cdpUrl).toBe("http://[::1]:80/json/version?token=abc");
629+
});
630+
631+
it("userinfo colons without a URL port defer to cdpPort", () => {
632+
const resolved = resolveBrowserConfig({
633+
profiles: {
634+
openclaw: {
635+
cdpPort: 18800,
636+
cdpUrl: "http://user:[email protected]/json/version",
637+
color: "#FF4500",
638+
driver: "openclaw",
639+
},
640+
},
641+
});
642+
const profile = resolveProfile(resolved, "openclaw");
643+
expect(profile?.cdpPort).toBe(18800);
644+
expect(profile?.cdpUrl).toBe("http://user:[email protected]:18800/json/version");
645+
});
646+
647+
it("URL without port defers to cdpPort", () => {
648+
const resolved = resolveBrowserConfig({
649+
profiles: {
650+
openclaw: {
651+
cdpPort: 18800,
652+
cdpUrl: "http://127.0.0.1",
653+
color: "#FF4500",
654+
driver: "openclaw",
655+
},
656+
},
657+
});
658+
const profile = resolveProfile(resolved, "openclaw");
659+
expect(profile?.cdpPort).toBe(18800);
660+
expect(profile?.cdpUrl).toBe("http://127.0.0.1:18800");
661+
});
662+
663+
it("URL with non-default port, no cdpPort configured", () => {
664+
const resolved = resolveBrowserConfig({
665+
profiles: {
666+
openclaw: {
667+
cdpUrl: "http://127.0.0.1:9222",
668+
color: "#FF4500",
669+
driver: "openclaw",
670+
},
671+
},
672+
});
673+
const profile = resolveProfile(resolved, "openclaw");
674+
expect(profile?.cdpPort).toBe(9222);
675+
expect(profile?.cdpUrl).toBe("http://127.0.0.1:9222");
676+
});
677+
678+
it("URL without port and no cdpPort falls back to protocol default", () => {
679+
const resolved = resolveBrowserConfig({
680+
profiles: {
681+
openclaw: {
682+
cdpUrl: "https://remote-browser.example.com",
683+
color: "#FF4500",
684+
driver: "openclaw",
685+
},
686+
},
687+
});
688+
const profile = resolveProfile(resolved, "openclaw");
689+
expect(profile?.cdpPort).toBe(443);
690+
expect(profile?.cdpUrl).toBe("https://remote-browser.example.com");
691+
});
692+
693+
it("no URL + cdpPort constructs URL from defaults", () => {
694+
const resolved = resolveBrowserConfig({
695+
profiles: {
696+
openclaw: {
697+
cdpPort: 9222,
698+
color: "#FF4500",
699+
driver: "openclaw",
700+
},
701+
},
702+
});
703+
const profile = resolveProfile(resolved, "openclaw");
704+
expect(profile?.cdpPort).toBe(9222);
705+
expect(profile?.cdpUrl).toContain(":9222");
706+
});
707+
708+
it("no URL + no cdpPort throws", () => {
709+
const resolved = resolveBrowserConfig({
710+
profiles: {
711+
bad: {
712+
color: "#FF4500",
713+
driver: "openclaw",
714+
},
715+
},
716+
});
717+
expect(() => resolveProfile(resolved, "bad")).toThrow("must define cdpPort or cdpUrl");
718+
});
719+
720+
it("stale WS devtools URL + cdpPort drops path and uses cdpPort", () => {
721+
const resolved = resolveBrowserConfig({
722+
profiles: {
723+
"chrome-cdp": {
724+
cdpPort: 9222,
725+
cdpUrl: "ws://127.0.0.1:12345/devtools/browser/old-stale-id",
726+
attachOnly: true,
727+
color: "#F59E0B",
728+
},
729+
},
730+
});
731+
const profile = resolveProfile(resolved, "chrome-cdp");
732+
expect(profile?.cdpUrl).toBe("http://127.0.0.1:9222");
733+
expect(profile?.cdpPort).toBe(9222);
734+
expect(profile?.cdpIsLoopback).toBe(true);
735+
expect(profile?.attachOnly).toBe(true);
736+
});
737+
738+
it("IPv6 URL without port defers to cdpPort", () => {
739+
const resolved = resolveBrowserConfig({
740+
profiles: {
741+
openclaw: {
742+
cdpPort: 18800,
743+
cdpUrl: "http://[::1]",
744+
color: "#FF4500",
745+
driver: "openclaw",
746+
},
747+
},
748+
});
749+
const profile = resolveProfile(resolved, "openclaw");
750+
expect(profile?.cdpPort).toBe(18800);
751+
expect(profile?.cdpUrl).toBe("http://[::1]:18800");
752+
});
753+
754+
it("IPv6 URL with explicit port wins over cdpPort", () => {
755+
const resolved = resolveBrowserConfig({
756+
profiles: {
757+
openclaw: {
758+
cdpPort: 18800,
759+
cdpUrl: "http://[::1]:9222",
760+
color: "#FF4500",
761+
driver: "openclaw",
762+
},
763+
},
764+
});
765+
const profile = resolveProfile(resolved, "openclaw");
766+
expect(profile?.cdpPort).toBe(9222);
767+
expect(profile?.cdpUrl).toBe("http://[::1]:9222");
569768
});
570-
const profile = resolveProfile(resolved, "chrome-cdp");
571-
// cdpPort produces a stable HTTP endpoint; the stale WS session ID is dropped.
572-
expect(profile?.cdpUrl).toBe("http://127.0.0.1:9222");
573-
expect(profile?.cdpPort).toBe(9222);
574-
expect(profile?.cdpIsLoopback).toBe(true);
575-
expect(profile?.attachOnly).toBe(true);
576769
});
577770

578771
it("preserves profile host when dropping stale devtools WS path", () => {

‎extensions/browser/src/browser/config.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -502,8 +502,20 @@ export function resolveProfile(
502502
} else if (rawProfileUrl) {
503503
const parsed = parseBrowserHttpUrl(rawProfileUrl, `browser.profiles.${profileName}.cdpUrl`);
504504
cdpHost = parsed.parsed.hostname;
505-
cdpPort = parsed.port;
506-
cdpUrl = parsed.normalized;
505+
// Port precedence: explicit URL port > configured cdpPort > protocol default.
506+
if (parsed.hasExplicitPort) {
507+
cdpPort = parsed.port;
508+
cdpUrl = parsed.normalizedWithPort;
509+
} else if (cdpPort) {
510+
// URL omitted the port but we have an explicit cdpPort — inject it while
511+
// preserving the rest of the URL (path, query, credentials, etc.).
512+
const rebuilt = new URL(rawProfileUrl);
513+
rebuilt.port = String(cdpPort);
514+
cdpUrl = rebuilt.toString().replace(/\/$/, "");
515+
} else {
516+
cdpPort = parsed.port;
517+
cdpUrl = parsed.normalized;
518+
}
507519
} else if (cdpPort) {
508520
cdpUrl = `${resolved.cdpProtocol}://${resolved.cdpHost}:${cdpPort}`;
509521
} else {

0 commit comments

Comments
 (0)