[PR #5179] [CLOSED] fix(browser): accept explicitly spelled default CDP ports #6803

Closed
opened 2026-11-01 21:43:08 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5179
Author: @Yi-111-a
Created: 10/19/2026
Status: ❌ Closed

Base: main ← Head: fix-5178-default-port


📝 Commits (3)

  • fd1331c fix(browser): accept explicitly spelled default CDP ports
  • a636754 Merge branch 'main' into fix-5178-default-port
  • 0557044 fix(browser): preserve explicit default CDP ports in recorded endpoints

📊 Changes

2 files changed (+105 additions, -11 deletions)

View changed files

📝 src/domain/browserObservation.test.ts (+56 -0)
📝 src/domain/browserObservation.ts (+49 -11)

📄 Description

Summary

Fixes #5178. Explicit loopback CDP endpoints that spell the default HTTP/WS port (:80) were rejected as invalid_request because the URL API clears url.port for default ports. Validation now treats a spelled default port as explicit while still rejecting implicit ports and non-loopback hosts.

Problem and expected behavior

http://127.0.0.1:80, http://[::1]:80, and ws://127.0.0.1:80/devtools/browser/… failed endpoint validation with “must be an explicit-port …” even though the port was written. Implicit endpoints (http://127.0.0.1) and other unsupported shapes must stay rejected. Non-default ports are unchanged.

Change and scope

In src/domain/browserObservation.ts, keep the loopback / path / userinfo gates, and replace the bare url.port !== "" check with a helper that:

  1. accepts a nonempty url.port, or
  2. when the URL API omitted a default port, confirms the raw string contains hostname:defaultPort ending the authority.

Applies to both HTTP discovery (browserEndpointSchema / httpDiscoveryOrigin) and browser WebSocket authority checks. No new providers or target-execution surface. Maintainer scope decision: not applicable.

Contract and boundary impact

  • Semantic owner and earliest changed stage: browserObservation request validation (before discovery).
  • Contribution scope fit and linked maintainer decision: ordinary fix within existing CDP endpoint boundary; not applicable.
  • CLI and MCP/tool-catalog contract: same fields (inspector_endpoint / cdp_endpoint); acceptance widens only for spelled default ports.
  • Provider, bridge, target-format, or platform compatibility: none beyond URL-API default-port normalization.
  • Evidence, artifact, provenance, or reconstruction contract: none.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable.
  • Generated metadata / package impact: none.

Expected effects:

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

(Validation-only change; discovery still happens only after a successful parse.)

Evidence and regression coverage

  • Tests added or updated: src/domain/browserObservation.test.ts — accept :80 HTTP and WS forms; reject implicit http://127.0.0.1.
  • Base reproduction or other evidence: source-derived from #5178 (URL API clears port for :80); unit regression would fail on main without the helper.
  • User-visible CLI/MCP output (if applicable): validation no longer returns invalid_request for spelled :80 loopback endpoints.
  • Remaining proof gaps: live attach on a privileged port-80 listener was not exercised (EACCES on bind), matching the issue.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • npx vitest run src/domain/browserObservation.test.ts — 7 passed
  • pre-push turbo run typecheck lint — success
  • Real port-80 Inspector listener — not run (bind EACCES / not required for the validation fix)

Compatibility, safety, and release

  • Breaking changes or migration steps: none (widens acceptance only).
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: still loopback-only; implicit ports remain rejected.

Author readiness

Human owner: @Yi-111-a

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/5179 **Author:** [@Yi-111-a](https://github.com/Yi-111-a) **Created:** 10/19/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix-5178-default-port` --- ### 📝 Commits (3) - [`fd1331c`](https://github.com/morluto/rea/commit/fd1331cfcce9daf335945cdb78d59ca885790906) fix(browser): accept explicitly spelled default CDP ports - [`a636754`](https://github.com/morluto/rea/commit/a6367546ca7fd18008ccce34bb3fd38434f80795) Merge branch 'main' into fix-5178-default-port - [`0557044`](https://github.com/morluto/rea/commit/0557044791a0299986dd87487090a83c575aebbd) fix(browser): preserve explicit default CDP ports in recorded endpoints ### 📊 Changes **2 files changed** (+105 additions, -11 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/browserObservation.test.ts` (+56 -0) 📝 `src/domain/browserObservation.ts` (+49 -11) </details> ### 📄 Description ## Summary Fixes #5178. Explicit loopback CDP endpoints that spell the default HTTP/WS port (`:80`) were rejected as `invalid_request` because the URL API clears `url.port` for default ports. Validation now treats a spelled default port as explicit while still rejecting implicit ports and non-loopback hosts. ## Problem and expected behavior `http://127.0.0.1:80`, `http://[::1]:80`, and `ws://127.0.0.1:80/devtools/browser/…` failed endpoint validation with “must be an explicit-port …” even though the port was written. Implicit endpoints (`http://127.0.0.1`) and other unsupported shapes must stay rejected. Non-default ports are unchanged. ## Change and scope In `src/domain/browserObservation.ts`, keep the loopback / path / userinfo gates, and replace the bare `url.port !== ""` check with a helper that: 1. accepts a nonempty `url.port`, or 2. when the URL API omitted a default port, confirms the raw string contains `hostname:defaultPort` ending the authority. Applies to both HTTP discovery (`browserEndpointSchema` / `httpDiscoveryOrigin`) and browser WebSocket authority checks. No new providers or target-execution surface. Maintainer scope decision: not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `browserObservation` request validation (before discovery). - Contribution scope fit and linked maintainer decision: ordinary fix within existing CDP endpoint boundary; not applicable. - CLI and MCP/tool-catalog contract: same fields (`inspector_endpoint` / `cdp_endpoint`); acceptance widens only for spelled default ports. - Provider, bridge, target-format, or platform compatibility: none beyond URL-API default-port normalization. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable. - Generated metadata / package impact: none. Expected effects: - [ ] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [x] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. (Validation-only change; discovery still happens only after a successful parse.) ## Evidence and regression coverage - Tests added or updated: `src/domain/browserObservation.test.ts` — accept `:80` HTTP and WS forms; reject implicit `http://127.0.0.1`. - Base reproduction or other evidence: source-derived from #5178 (URL API clears `port` for `:80`); unit regression would fail on main without the helper. - User-visible CLI/MCP output (if applicable): validation no longer returns `invalid_request` for spelled `:80` loopback endpoints. - Remaining proof gaps: live attach on a privileged port-80 listener was not exercised (`EACCES` on bind), matching the issue. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npx vitest run src/domain/browserObservation.test.ts` — 7 passed - pre-push `turbo run typecheck lint` — success - Real port-80 Inspector listener — not run (bind `EACCES` / not required for the validation fix) ## Compatibility, safety, and release - Breaking changes or migration steps: none (widens acceptance only). - Real Hopper/Ghidra, browser, or OS coverage: not applicable. - Package or release metadata impact: none. - Security, privacy, process, or containment review: still loopback-only; implicit ports remain rejected. ## Author readiness Human owner: @Yi-111-a - [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [ ] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:43:08 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#6803
No description provided.