Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
167 changes: 155 additions & 12 deletions Resources/bin/open
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,136 @@ is_http_url() {
return 1
}

is_file_url() {
local value="$1"
case "$value" in
[Ff][Ii][Ll][Ee]://*)
return 0
;;
esac
return 1
}

has_uri_scheme() {
local value="$1"
[[ "$value" =~ ^[A-Za-z][A-Za-z0-9+.-]*: ]]
}

is_html_extension() {
local value
value="$(to_lower_ascii "$(trim "$1")")"
case "$value" in
*.html|*.htm)
return 0
;;
esac
return 1
}

file_url_points_to_html() {
local value="$1"
if [[ -n "$PYTHON3_BIN" ]]; then
"$PYTHON3_BIN" - "$value" <<'PY' >/dev/null 2>&1
import sys
from urllib.parse import unquote, urlsplit

value = sys.argv[1].strip()
if not value:
raise SystemExit(1)

parts = urlsplit(value)
path = unquote(parts.path or "")
lower = path.lower()
if lower.endswith(".html") or lower.endswith(".htm"):
Comment on lines +155 to +158

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject non-local file:// hosts in HTML routing

file_url_points_to_html only checks the parsed path suffix and never validates the file URL host, so inputs like file://example.com/report.html are now classified as local HTML and routed to cmux browser open instead of /usr/bin/open. In this commit, resolveBrowserNavigableURL also accepts file URLs based on path alone, so hosted file URLs can be misinterpreted as local paths (for example /report.html), breaking UNC/hosted file links that previously opened through the system handler. Please require an empty/localhost file host before treating a file:// target as a local HTML file.

Useful? React with 👍 / 👎.

raise SystemExit(0)
raise SystemExit(1)
PY
return $?
fi

local without_fragment="${value%%\#*}"
local without_query="${without_fragment%%\?*}"
is_html_extension "$without_query"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}

path_to_file_url_without_python() {
local raw="$1"
local expanded="$raw"
case "$expanded" in
"~")
expanded="$HOME"
;;
"~/"*)
expanded="$HOME/${expanded#~/}"
;;
esac

local absolute
if [[ "$expanded" == /* ]]; then
absolute="$expanded"
else
absolute="$(pwd)/$expanded"
fi

local directory="$absolute"
local basename=""
if [[ "$absolute" == */* ]]; then
directory="${absolute%/*}"
basename="${absolute##*/}"
fi

local resolved_directory
if resolved_directory="$(cd "$directory" 2>/dev/null && pwd -P)"; then
absolute="$resolved_directory"
if [[ -n "$basename" ]]; then
absolute="$absolute/$basename"
fi
fi

local encoded=""
local length=${#absolute}
local index char hex
local LC_ALL=C
for ((index = 0; index < length; index++)); do
char="${absolute:index:1}"
case "$char" in
[a-zA-Z0-9.~_-]|/)
encoded+="$char"
;;
*)
printf -v hex '%02X' "'$char"
encoded+="%$hex"
;;
esac
done
printf 'file://%s\n' "$encoded"
}

path_to_file_url() {
local raw="$1"
if [[ -n "$PYTHON3_BIN" ]]; then
"$PYTHON3_BIN" - "$raw" <<'PY' 2>/dev/null
import pathlib
import sys

raw = sys.argv[1]
if not raw:
raise SystemExit(1)

path = pathlib.Path(raw).expanduser()
if path.is_absolute():
resolved = path.resolve(strict=False)
else:
resolved = (pathlib.Path.cwd() / path).resolve(strict=False)

sys.stdout.write(resolved.as_uri())
PY
return $?
fi

path_to_file_url_without_python "$raw"
}

normalize_host() {
local value
value="$(trim "$1")"
Expand Down Expand Up @@ -212,9 +342,10 @@ if [[ $# -eq 0 ]]; then
fi

# Scan for flags that indicate explicit user intent → pass through.
# Also collect non-flag arguments (potential URLs/files).
# Also collect non-flag arguments and route eligible browser targets to cmux.
passthrough=false
urls=()
cmux_targets=()
passthrough_args=()
for arg in "$@"; do
case "$arg" in
-a|-b|-R|-e|-t|-f|-W|-g|-n|-h|-s|-j|-u|--env|--stdin|--stdout|--stderr)
Expand All @@ -228,17 +359,29 @@ for arg in "$@"; do
;;
*)
if is_http_url "$arg"; then
urls+=("$arg")
cmux_targets+=("$arg")
elif is_file_url "$arg"; then
if file_url_points_to_html "$arg"; then
cmux_targets+=("$arg")
else
passthrough_args+=("$arg")
fi
elif has_uri_scheme "$arg"; then
passthrough_args+=("$arg")
elif is_html_extension "$arg"; then
if local_file_url="$(path_to_file_url "$arg")"; then
cmux_targets+=("$local_file_url")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restrict .html interception to actual local file paths

The new is_html_extension branch converts any argument ending in .html/.htm into a file:// target, even when the input is not a local path (for example example.com/report.html). That means these no-scheme web links are now routed to cmux browser open file:///... instead of being passed through to /usr/bin/open as before, so users/scripts that call open with host/path-style URLs will open a non-existent local file. Please gate this conversion on local-path conditions (e.g., existing path or explicit path prefix) before rewriting.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Gate .html rewrites to real local paths

The .html fallback currently rewrites any non-schemed token ending in .html to a file:// URL, so host/path-style inputs like example.com/report.html are treated as local files and sent to cmux browser open file:///... instead of being passed through to system open. This is a behavior regression for scripts/users that rely on no-scheme web targets, and I verified the current wrapper produces file:///tmp/.../example.com/report.html for that input.

Useful? React with 👍 / 👎.

else
passthrough_args+=("$arg")
fi
else
# Non-URL, non-flag argument (file path, etc.) → pass through all
passthrough=true
break
passthrough_args+=("$arg")
fi
;;
esac
done

if [[ "$passthrough" == true ]] || [[ ${#urls[@]} -eq 0 ]]; then
if [[ "$passthrough" == true ]] || [[ ${#cmux_targets[@]} -eq 0 ]]; then
system_open "$@"
fi

Expand Down Expand Up @@ -269,15 +412,15 @@ fi

# Open each URL in cmux's in-app browser; track failures individually.
failed_urls=()
for url in "${urls[@]}"; do
if ! host_matches_whitelist "$url"; then
for url in "${cmux_targets[@]}"; do
if is_http_url "$url" && ! host_matches_whitelist "$url"; then
failed_urls+=("$url")
continue
fi
"$CMUX_CLI" browser open "$url" 2>/dev/null || failed_urls+=("$url")
done

# Fall back to system open only for URLs that failed.
if [[ ${#failed_urls[@]} -gt 0 ]]; then
system_open "${failed_urls[@]}"
# Fall back to system open for unmatched args and URLs that failed.
if [[ ${#passthrough_args[@]} -gt 0 ]] || [[ ${#failed_urls[@]} -gt 0 ]]; then
system_open "${passthrough_args[@]}" "${failed_urls[@]}"
fi
121 changes: 121 additions & 0 deletions tests/test_open_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ def run_wrapper(
legacy_open_setting: str | None = None,
whitelist: str | None,
fail_urls: list[str] | None = None,
local_files: list[str] | None = None,
python_bin: str | None = None,
) -> tuple[list[str], list[str], int, str]:
with tempfile.TemporaryDirectory(prefix="cmux-open-wrapper-test-") as td:
tmp = Path(td)
Expand Down Expand Up @@ -113,13 +115,23 @@ def run_wrapper(
""",
)

if local_files:
for relative_path in local_files:
target = tmp / relative_path
target.parent.mkdir(parents=True, exist_ok=True)
target.write_text("<!doctype html><title>fixture</title>", encoding="utf-8")

env = os.environ.copy()
env["CMUX_SOCKET_PATH"] = "/tmp/cmux-open-wrapper-test.sock"
env["CMUX_BUNDLE_ID"] = "com.cmuxterm.app.debug.test"
env["CMUX_OPEN_WRAPPER_SYSTEM_OPEN"] = str(system_open)
env["CMUX_OPEN_WRAPPER_DEFAULTS"] = str(defaults)
env["FAKE_OPEN_LOG"] = str(open_log)
env["FAKE_CMUX_LOG"] = str(cmux_log)
if python_bin is None:
env.pop("CMUX_OPEN_WRAPPER_PYTHON3", None)
else:
env["CMUX_OPEN_WRAPPER_PYTHON3"] = python_bin

if intercept_setting is None:
env.pop("FAKE_DEFAULTS_INTERCEPT_OPEN", None)
Expand All @@ -143,6 +155,7 @@ def run_wrapper(

result = subprocess.run(
["/bin/bash", str(wrapper), *args],
cwd=tmp,
env=env,
capture_output=True,
text=True,
Expand Down Expand Up @@ -282,6 +295,108 @@ def test_uppercase_scheme_routes_to_cmux(failures: list[str]) -> None:
expect(cmux_log == [f"browser open {url}"], f"uppercase scheme: unexpected cmux log {cmux_log}", failures)


def test_local_html_file_routes_to_cmux(failures: list[str]) -> None:
filename = "fixtures/hello page.HTML"
open_log, cmux_log, code, stderr = run_wrapper(
args=[filename],
intercept_setting="1",
whitelist="",
local_files=[filename],
)
expect(code == 0, f"local html file: wrapper exited {code}: {stderr}", failures)
expect(open_log == [], f"local html file: system open should not be called, got {open_log}", failures)
expect(len(cmux_log) == 1, f"local html file: expected exactly one cmux call, got {cmux_log}", failures)
if cmux_log:
expect(
cmux_log[0].startswith("browser open file://"),
f"local html file: expected file:// target, got {cmux_log[0]}",
failures,
)
expect(
"hello%20page.HTML" in cmux_log[0],
f"local html file: expected URL-encoded filename in cmux target, got {cmux_log[0]}",
failures,
)


def test_file_url_html_routes_to_cmux(failures: list[str]) -> None:
url = "file:///tmp/cmux-open-wrapper-fixture.html"
open_log, cmux_log, code, stderr = run_wrapper(
args=[url],
intercept_setting="1",
whitelist="",
)
expect(code == 0, f"file url html: wrapper exited {code}: {stderr}", failures)
expect(open_log == [], f"file url html: system open should not be called, got {open_log}", failures)
expect(cmux_log == [f"browser open {url}"], f"file url html: unexpected cmux log {cmux_log}", failures)


def test_local_html_file_routes_to_cmux_without_python_binary(failures: list[str]) -> None:
filename = "fixtures/no python fallback.html"
open_log, cmux_log, code, stderr = run_wrapper(
args=[filename],
intercept_setting="1",
whitelist="",
local_files=[filename],
python_bin="/definitely/missing/python3",
)
expect(code == 0, f"local html no-python fallback: wrapper exited {code}: {stderr}", failures)
expect(open_log == [], f"local html no-python fallback: system open should not be called, got {open_log}", failures)
expect(
len(cmux_log) == 1,
f"local html no-python fallback: expected exactly one cmux call, got {cmux_log}",
failures,
)
if cmux_log:
expect(
cmux_log[0].startswith("browser open file://"),
f"local html no-python fallback: expected file:// target, got {cmux_log[0]}",
failures,
)
expect(
"no%20python%20fallback.html" in cmux_log[0],
f"local html no-python fallback: expected URL-encoded filename, got {cmux_log[0]}",
failures,
)


def test_non_file_scheme_html_passthrough(failures: list[str]) -> None:
url = "ftp://example.com/report.html"
open_log, cmux_log, code, stderr = run_wrapper(
args=[url],
intercept_setting="1",
whitelist="",
)
expect(code == 0, f"non-file scheme html: wrapper exited {code}: {stderr}", failures)
expect(cmux_log == [], f"non-file scheme html: cmux should not be called, got {cmux_log}", failures)
expect(open_log == [url], f"non-file scheme html: expected system open [{url}], got {open_log}", failures)


def test_mailto_html_passthrough(failures: list[str]) -> None:
url = "mailto:help@example.com?subject=report.html"
open_log, cmux_log, code, stderr = run_wrapper(
args=[url],
intercept_setting="1",
whitelist="",
)
expect(code == 0, f"mailto html: wrapper exited {code}: {stderr}", failures)
expect(cmux_log == [], f"mailto html: cmux should not be called, got {cmux_log}", failures)
expect(open_log == [url], f"mailto html: expected system open [{url}], got {open_log}", failures)


def test_local_non_html_file_passthrough(failures: list[str]) -> None:
filename = "fixtures/readme.md"
open_log, cmux_log, code, stderr = run_wrapper(
args=[filename],
intercept_setting="1",
whitelist="",
local_files=[filename],
)
expect(code == 0, f"local non-html file: wrapper exited {code}: {stderr}", failures)
expect(cmux_log == [], f"local non-html file: cmux should not be called, got {cmux_log}", failures)
expect(open_log == [filename], f"local non-html file: expected system open [{filename}], got {open_log}", failures)


def test_unicode_whitelist_matches_punycode_url(failures: list[str]) -> None:
url = "https://xn--bcher-kva.example/path"
open_log, cmux_log, code, stderr = run_wrapper(
Expand Down Expand Up @@ -316,6 +431,12 @@ def main() -> int:
test_legacy_toggle_fallback_passthrough(failures)
test_legacy_toggle_fallback_case_insensitive_passthrough(failures)
test_uppercase_scheme_routes_to_cmux(failures)
test_local_html_file_routes_to_cmux(failures)
test_file_url_html_routes_to_cmux(failures)
test_local_html_file_routes_to_cmux_without_python_binary(failures)
test_non_file_scheme_html_passthrough(failures)
test_mailto_html_passthrough(failures)
test_local_non_html_file_passthrough(failures)
test_unicode_whitelist_matches_punycode_url(failures)
test_punycode_whitelist_matches_unicode_url(failures)

Expand Down
Loading