diff --git a/Common/source/shellsysverbs.c b/Common/source/shellsysverbs.c index 04750d7dc..c882609d9 100644 --- a/Common/source/shellsysverbs.c +++ b/Common/source/shellsysverbs.c @@ -838,11 +838,28 @@ static boolean sysfunctionvalue (short token, hdltreenode hparam1, tyvaluerecord * macOS: execlp("open", ...), Linux: execlp("xdg-open", ...). * * 3/23/26 JES: Respect --browser CLI arg via system.environment.args.browser. - * Allowed values: "default" (system browser) or "agent-browser". + * Allowed values: "default", "agent-browser" or "allow-gui". * Note: Scripts can also set system.environment.args.browser directly; * this is intentional — scripts are trusted code within the ODB. * The browser check is guarded by FRONTIER_HEADLESS; GUI builds * always use the system default browser (no --browser flag). + * + * 2026-08-15 JES #891: in headless builds the default no longer execs a + * host GUI browser -- it logs the URL and records it at + * system.temp.Frontier.pendingBrowserUrl. The onboarding intent (legacy + * Frontier opened the owner's browser at setupFrontier to collect config) + * is preserved; only the host-GUI launch from a headless/server context is + * wrong, since no human is at that machine's desktop. --browser allow-gui + * restores the launch. GUI builds are unaffected: the whole block is + * inside FRONTIER_HEADLESS. + * + * TWIN: tests/headless_sys_verbs.c case sysv_openurl is the parallel + * implementation and must change in lockstep. NOTE that despite living + * under Common/source, THIS file is not linked into frontier-cli (it is + * absent from frontier-cli/Makefile); the headless twin is what the CLI + * runs. The two are NOT observably identical: the twin also logs the URL + * and sets system.temp.Frontier.pendingBrowserUrl, neither of which is in + * scope here, so this path only suppresses the exec. Dedup tracked in #893. */ Handle hurl; @@ -878,8 +895,15 @@ static boolean sysfunctionvalue (short token, hdltreenode hparam1, tyvaluerecord * only sets it to true in headless mode. */ boolean use_agent_browser = false; + /* Stays false in a GUI build, so the exec proceeds: the correct + * legacy behavior, since there IS a human at this machine's + * desktop. allow_gui_browser is scoped to the headless block + * below, where it is the only place it can be read. */ + boolean fl_no_exec = false; + #ifdef FRONTIER_HEADLESS { + boolean allow_gui_browser = false; hdlhashnode hnode; tyvaluerecord vargs; bigstring bsargs = BIGSTRING ("\x04" "args"); @@ -903,11 +927,14 @@ static boolean sysfunctionvalue (short token, hdltreenode hparam1, tyvaluerecord if (len == (long)(sizeof ("agent-browser") - 1) && memcmp (*vbrowser.data.stringvalue, "agent-browser", sizeof ("agent-browser") - 1) == 0) { use_agent_browser = true; } + else if (len == (long)(sizeof ("allow-gui") - 1) && memcmp (*vbrowser.data.stringvalue, "allow-gui", sizeof ("allow-gui") - 1) == 0) { + allow_gui_browser = true; + } else if (len != (long)(sizeof ("default") - 1) || memcmp (*vbrowser.data.stringvalue, "default", sizeof ("default") - 1) != 0) { /* Unknown browser value — reject for security */ unlockhandle (hurl); disposehandle (hurl); - langerrormessage (BIGSTRING ("\x45" "Can't open URL: --browser must be \"default\" or \"agent-browser\".")); + langerrormessage (BIGSTRING ("\x4c" "Can't open URL: --browser must be \"default\", \"agent-browser\" or \"allow-gui\".")); return (false); } } @@ -915,9 +942,24 @@ static boolean sysfunctionvalue (short token, hdltreenode hparam1, tyvaluerecord } } } + + /* #891: headless default performs no exec. Kept in lockstep with + * the headless twin; see the TWIN note above. */ + fl_no_exec = !use_agent_browser && !allow_gui_browser; } #endif + if (fl_no_exec) { + unlockhandle (hurl); + disposehandle (hurl); + + /* True: the onboarding step was reached, which is what the caller + * asks about. The headless twin additionally logs the URL and sets + * system.temp.Frontier.pendingBrowserUrl; this classic path has + * neither the logging nor the marker helper in scope. */ + return (setbooleanvalue (true, v)); + } + #if defined(__APPLE__) || defined(__linux__) /* Double-fork to avoid zombie processes: first child forks again @@ -939,7 +981,7 @@ static boolean sysfunctionvalue (short token, hdltreenode hparam1, tyvaluerecord if (use_agent_browser) { execlp ("agent-browser", "agent-browser", "open", url, NULL); } - else { + else { /* allow_gui_browser, or any GUI build (#891) */ #ifdef __APPLE__ execlp ("open", "open", url, NULL); #else diff --git a/frontier-cli/main.c b/frontier-cli/main.c index 4301cd79c..b6bd4840e 100644 --- a/frontier-cli/main.c +++ b/frontier-cli/main.c @@ -1387,8 +1387,12 @@ static void print_usage(const char* program_name) { printf(" --my-flag value -> system.environment.args.myFlag\n"); printf("\n"); printf(" Built-in custom flags:\n"); - printf(" --browser MODE Set browser for sys.openUrl (default: system default)\n"); - printf(" MODE: \"default\" or \"agent-browser\"\n"); + printf(" --browser MODE Set browser for sys.openUrl (default: log the URL only)\n"); + printf(" MODE: \"default\", \"agent-browser\" or \"allow-gui\"\n"); + printf(" default: log the URL, set\n"); + printf(" system.temp.Frontier.pendingBrowserUrl, no exec\n"); + printf(" agent-browser: drive the URL with agent-browser\n"); + printf(" allow-gui: launch this machine's GUI browser\n"); printf("\n"); printf("Environment Variables:\n"); diff --git a/tests/Makefile b/tests/Makefile index 1c169db42..b746a9b0d 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -888,6 +888,13 @@ test-custom-args: @echo "Running custom CLI args tests..." @./custom_cli_args_test.sh +# Run headless sys.openUrl GUI-browser guard tests (shell-based, uses -e mode) +# 2026-08-15 JES #891: the headless default must not exec a host GUI browser. +# Exec assertions use PATH-shadowing stubs, so no real browser is ever launched. +test-openurl-guard: + @echo "Running headless sys.openUrl guard tests..." + @./openurl_headless_guard_test.sh + # Run REPL selection flag tests (shell-based, uses -e mode + --help) # 2026-06-25 JES C.6 #691: covers --plain / --debug-tui / --protocol # mutual exclusion, the --debug-tui deprecation warning, and the new @@ -935,7 +942,7 @@ check-license: @python3 ../tools/relicense_headers.py --check # Run all tests (unit + integration + CLI migration + custom args + debug + hooks + license check) -test-all: test test-integration test-migrate-flag test-migrate-long-path test-system-root-long-path test-system-root-temp-path-boundary test-sys-getenv-long-value test-custom-args test-repl-flags test-debug test-agent-debug test-debug-tui test-cow-idempotent test-ut-sync-lifecycle test-hooks check-license +test-all: test test-integration test-migrate-flag test-migrate-long-path test-system-root-long-path test-system-root-temp-path-boundary test-sys-getenv-long-value test-custom-args test-openurl-guard test-repl-flags test-debug test-agent-debug test-debug-tui test-cow-idempotent test-ut-sync-lifecycle test-hooks check-license @echo "All test suites completed" # Verify Virgin.root <-> .ut corpus sync (issue #675). @@ -1027,7 +1034,7 @@ verify-errors: @echo "Verifying error message coverage..." @../tools/verify_error_coverage.sh -.PHONY: all test test-report test-integration test-integration-verbose test-migrate-flag test-migrate-long-path test-system-root-long-path test-system-root-temp-path-boundary test-sys-getenv-long-value test-custom-args test-repl-flags test-debug test-agent-debug test-debug-tui test-hooks test-all verify-odb-sync test-odb-sync clean install uninstall help results strings_generated kernel_verbs_generated verify-errors validate-bigstrings check-python-deps +.PHONY: all test test-report test-integration test-integration-verbose test-migrate-flag test-migrate-long-path test-system-root-long-path test-system-root-temp-path-boundary test-sys-getenv-long-value test-custom-args test-openurl-guard test-repl-flags test-debug test-agent-debug test-debug-tui test-hooks test-all verify-odb-sync test-odb-sync clean install uninstall help results strings_generated kernel_verbs_generated verify-errors validate-bigstrings check-python-deps check_v6_tables: check_v6_tables.c $(LANG_RUNTIME_SOURCES) ../Common/source/odbengine.c ../frontier-cli/stubs/headless_shell.c $(CC) $(CFLAGS) -DFRONTIER_HEADLESS -DHEADLESS_TEST_PORTABLE_FILE -DHEADLESS_LINKS_REAL_DB -I../Common/headers -I../Common/SystemHeaders -I../portable check_v6_tables.c \ diff --git a/tests/headless_sys_verbs.c b/tests/headless_sys_verbs.c index 6b25682e1..1a43b503d 100644 --- a/tests/headless_sys_verbs.c +++ b/tests/headless_sys_verbs.c @@ -20,6 +20,7 @@ #include "standard.h" #include /* for getenv, setenv */ +#include /* 2026-08-15 JES #891: for strlen in setpendingbrowserurl */ #include /* for fork, execlp, _exit */ #include /* for waitpid */ #include "memory.h" @@ -27,6 +28,7 @@ #include "lang.h" #include "langinternal.h" #include "tablestructure.h" +#include "tableverbs.h" /* 2026-08-15 JES #891: for findnamedtable */ #include "sysshellcall.h" #include "logging.h" #include "langexternal.h" @@ -72,6 +74,81 @@ static void trimtrailingwhitespace(Handle h) { } } +/* + * #891: record the URL that wanted a browser at + * system.temp.Frontier.pendingBrowserUrl, creating the intervening tables if + * needed. Semantics are "last URL that wanted a browser" -- each call + * overwrites, and no URL sniffing is done, so first-run flows assert the + * setup URL by equality rather than a bare boolean. + * + * The value is stored as a heap string (newfilledhandle) rather than via the + * bigstring assign helpers because a URL can exceed the 255-byte Pascal-string + * limit. + */ +static boolean setpendingbrowserurl (const char *url) { + hdlhashtable httemp; + hdlhashtable htfrontier; + bigstring bstemp; + bigstring bsfrontier; + bigstring bsurl; + Handle hvalue; + tyvaluerecord val; + long len; + + if (systemtable == nil) + return (false); + + copystring (PSTRING ("\x04", "temp"), bstemp); + copystring (PSTRING ("\x08", "Frontier"), bsfrontier); + copystring (PSTRING ("\x11", "pendingBrowserUrl"), bsurl); + + if (!findnamedtable (systemtable, bstemp, &httemp)) { + if (!tablenewsubtable (systemtable, bstemp, &httemp)) + return (false); + } + + if (!findnamedtable (httemp, bsfrontier, &htfrontier)) { + if (!tablenewsubtable (httemp, bsfrontier, &htfrontier)) + return (false); + } + + len = (long) strlen (url); + + if (!newfilledhandle ((ptrvoid) url, len, &hvalue)) + return (false); + + /* setheapvalue disposes the handle itself on failure (langvalue.c:594 -- + * "the caller isn't managing the memory any more"), so do NOT dispose here. */ + if (!setheapvalue (hvalue, stringvaluetype, &val)) + return (false); + + if (!hashtableassign (htfrontier, bsurl, val)) { + /* val is still on the tmp stack; disposetmpvalue removes it from the + * stack AND disposes the handle. A bare disposehandle would leave a + * dangling tmp-stack entry. + * + * The headless trigger for this branch is allocation failure inside + * hashassign (newhashnode). The protect-block refusal path + * (non-external value over an external one) does NOT apply here: it is + * gated on fllanghashassignprotect, which is false in this build -- + * tests/headless_threadglobals.c:60 initializes it false and the only + * toggles, tableverbs.c:288/294, are commented out. Verified live: with + * pendingBrowserUrl pre-created as a table, the assign SUCCEEDS and + * overwrites it (covered by an integration case). */ + disposetmpvalue (&val); + return (false); + } + + /* The table now owns the handle; keep it off the tmp stack so it is not + * released out from under the marker. The assign->exempt pair must stay + * yield-free: a yield in between could let the tmp stack unwind the value + * the table now references. */ + exemptfromtmpstack (&val); + + return (true); +} /*setpendingbrowserurl*/ + + /* Helper function to safely escape shell arguments for single-quote wrapping * Converts: foo'bar -> 'foo'\''bar' * This prevents command injection by escaping all single quotes and wrapping in quotes @@ -642,8 +719,25 @@ static boolean sys_valueproc(short token, hdltreenode hparam1, * Uses fork/execlp to avoid shell injection. * Respects --browser CLI arg via system.environment.args.browser: * "agent-browser" → execlp("agent-browser", "agent-browser", "open", url) - * "default" or absent → execlp("open", "open", url) on macOS - * See shellsysverbs.c openurlfunc for the equivalent classic implementation. + * "allow-gui" → execlp("open", "open", url) on macOS / xdg-open on Linux + * "default" or absent → no exec; log the URL and record it at + * system.temp.Frontier.pendingBrowserUrl + * + * 2026-08-15 JES #891: the headless default no longer execs a host GUI + * browser. Legacy Frontier launched the owner's browser at the + * setupFrontier page to collect config from the server owner, and that + * onboarding intent is preserved -- but a headless/server/CI process has + * no human at its desktop, and the daemon host is frequently not the + * operator's workstation. The default logs the URL for a remote operator + * and records it in the marker; --browser allow-gui restores the launch + * for a local desktop-style session. + * + * TWIN: Common/source/shellsysverbs.c openurlfunc is the parallel + * implementation for the classic build and must change in lockstep. + * That file is NOT in the frontier-cli link (grep frontier-cli/Makefile); + * this TU is the one the CLI actually runs. The two are NOT observably + * identical: the classic path suppresses the exec but does not log the + * URL or set the marker, neither being in scope there. Dedup: #893. */ /* No URL scheme validation — any non-empty string is passed through @@ -678,6 +772,7 @@ static boolean sys_valueproc(short token, hdltreenode hparam1, /* Check --browser arg from system.environment.args */ boolean use_agent_browser = false; + boolean allow_gui_browser = false; { hdlhashnode hnode; @@ -704,10 +799,13 @@ static boolean sys_valueproc(short token, hdltreenode hparam1, if (len == (long)(sizeof ("agent-browser") - 1) && memcmp (*vbrowser.data.stringvalue, "agent-browser", sizeof ("agent-browser") - 1) == 0) { use_agent_browser = true; } + else if (len == (long)(sizeof ("allow-gui") - 1) && memcmp (*vbrowser.data.stringvalue, "allow-gui", sizeof ("allow-gui") - 1) == 0) { + allow_gui_browser = true; + } else if (len != (long)(sizeof ("default") - 1) || memcmp (*vbrowser.data.stringvalue, "default", sizeof ("default") - 1) != 0) { unlockhandle (hurl); disposehandle (hurl); - langerrormessage (PSTRING ("\x3f", "Can't open URL: --browser must be \"default\" or \"agent-browser\".")); + langerrormessage (PSTRING ("\x4c", "Can't open URL: --browser must be \"default\", \"agent-browser\" or \"allow-gui\".")); return (false); } } @@ -717,6 +815,30 @@ static boolean sys_valueproc(short token, hdltreenode hparam1, } } + /* #891: headless default -- no host GUI browser. Log the URL so a + * remote operator can open it from their own machine, record it for + * flows and tests, and return true: the onboarding step was reached, + * which is what the caller is asking about. */ + if (!use_agent_browser && !allow_gui_browser) { + boolean flmarked = setpendingbrowserurl (url); + + /* Log only after the marker write succeeded: the log line is + * the operator's instruction to go complete setup, and it must + * not claim the onboarding step was reached when the marker + * flows and tests rely on was never recorded. */ + if (flmarked) + log_info (LOG_COMP_GENERAL, + "Frontier first-run: complete setup at %s", url); + + unlockhandle (hurl); + disposehandle (hurl); + + if (!flmarked) + return (false); + + return (setbooleanvalue (true, vreturned)); + } + #if defined(__APPLE__) || defined(__linux__) /* Double-fork to avoid zombie processes. */ pid_t pid = fork (); @@ -725,11 +847,13 @@ static boolean sys_valueproc(short token, hdltreenode hparam1, pid_t pid2 = fork (); if (pid2 == 0) { - /* All execlp calls rely on a trusted PATH. */ + /* All execlp calls rely on a trusted PATH. + * Only reachable when an opt-in was named: the default + * returned above without forking (#891). */ if (use_agent_browser) { execlp ("agent-browser", "agent-browser", "open", url, NULL); } - else { + else { /* allow_gui_browser */ #ifdef __APPLE__ execlp ("open", "open", url, NULL); #else diff --git a/tests/integration/test_cases/sys_processor_tests.yaml b/tests/integration/test_cases/sys_processor_tests.yaml index f0ad8867d..1dad33ce0 100644 --- a/tests/integration/test_cases/sys_processor_tests.yaml +++ b/tests/integration/test_cases/sys_processor_tests.yaml @@ -385,15 +385,16 @@ tests: # even without a display server). # ============================================================================ - # sys.openUrl - Opens a URL via fork/execlp. In headless CI the fork succeeds - # even if the platform launcher (open/xdg-open) isn't available, so we only - # verify the verb is callable and returns a boolean. - # Note: These tests cannot verify actual browser launch — only that the - # verb compiles, dispatches, and returns the expected type. + # sys.openUrl in the headless default mode performs NO process exec (#891): + # it logs the URL and records it at system.temp.Frontier.pendingBrowserUrl so + # a remote operator (or a test) can pick it up. These tests are therefore safe + # to run anywhere — they cannot launch a browser on the host. + # The exec opt-ins (--browser allow-gui / agent-browser) need CLI flags the + # protocol runner can't pass, so they are covered by + # tests/openurl_headless_guard_test.sh with PATH-stubbed launchers. - name: "sys.openUrl - returns boolean type" - skip: "sys.openUrl not implemented" - description: "openUrl should return a boolean regardless of fork outcome" + description: "openUrl should return a boolean in the headless default mode" script: 'typeof(sys.openUrl("https://example.com"))' expected_success: true expected_result: "bool" @@ -406,12 +407,45 @@ tests: # getexempttextvalue coerces numbers to strings — standard Frontier behavior. # The verb accepts any non-empty string (no URL scheme validation). - name: "sys.openUrl - number coerced to string succeeds" - skip: "sys.openUrl not implemented" - description: "Passing a number coerces to string; fork succeeds even for non-URL" + description: "Passing a number coerces to string; accepted even for non-URL" script: 'typeof(sys.openUrl(12345))' expected_success: true expected_result: "bool" + - name: "sys.openUrl - headless default records the pending browser URL" + description: "The onboarding intent is asserted via the marker, not a browser launch" + script: 'sys.openUrl("http://127.0.0.1:5336/setupFrontier");system.temp.Frontier.pendingBrowserUrl' + expected_success: true + expected_result: "http://127.0.0.1:5336/setupFrontier" + + - name: "sys.openUrl - marker holds the most recent URL" + description: "Each call overwrites the marker (last URL that wanted a browser)" + script: 'sys.openUrl("http://example.com/first");sys.openUrl("http://example.com/second");system.temp.Frontier.pendingBrowserUrl' + expected_success: true + expected_result: "http://example.com/second" + + # A table pre-created at the marker path does NOT force the assign-failure + # branch in this build: hashassign's protect block is gated on + # fllanghashassignprotect, which headless_threadglobals.c:60 initializes to + # false (and tableverbs.c:288/294 leaves its only toggles commented out). The + # assign therefore succeeds and overwrites the table, verified directly. So + # this asserts the observable behavior rather than a failure that cannot occur; + # the cleanup correctness of that branch is covered by inspection, not a test. + - name: "sys.openUrl - marker overwrites a pre-existing table at its path" + description: "hashassign protection is off in headless, so the marker write wins" + script: 'if not defined(system.temp.Frontier) {new(tableType, @system.temp.Frontier)};if defined(system.temp.Frontier.pendingBrowserUrl) {delete(@system.temp.Frontier.pendingBrowserUrl)};new(tableType, @system.temp.Frontier.pendingBrowserUrl);sys.openUrl("http://example.com/overwrite");system.temp.Frontier.pendingBrowserUrl' + expected_success: true + expected_result: "http://example.com/overwrite" + + # Sequential tests share one session (#852), so an earlier openUrl case may have + # left a marker behind. Clear it first: this asserts a rejected URL does not + # WRITE a marker, which is independent of test ordering. + - name: "sys.openUrl - empty URL does not set the marker" + description: "A rejected URL must not leave a pending onboarding marker behind" + script: 'if defined(system.temp.Frontier.pendingBrowserUrl) {delete(@system.temp.Frontier.pendingBrowserUrl)};try {sys.openUrl("")};defined(system.temp.Frontier.pendingBrowserUrl)' + expected_success: true + expected_result: "false" + # ============================================================================ # sys.unixshellcommand Tests (4 parameter variants) # ============================================================================ diff --git a/tests/openurl_headless_guard_test.sh b/tests/openurl_headless_guard_test.sh new file mode 100755 index 000000000..c5878a232 --- /dev/null +++ b/tests/openurl_headless_guard_test.sh @@ -0,0 +1,251 @@ +#!/bin/bash +# +# Integration tests for the headless sys.openUrl GUI-browser guard (#891). +# +# Legacy Frontier launched the owner's browser at the setupFrontier page to +# collect config from the server owner. That onboarding intent is preserved; +# only the host-GUI launch from a headless/server/test context is wrong. +# +# Headless default: no process exec. The URL is logged and recorded at +# system.temp.Frontier.pendingBrowserUrl so flows and tests can assert the +# onboarding intent was reached. +# Opt-in (--browser allow-gui): the real launcher runs again. +# +# HARD RULE: no test here may launch a host GUI browser. The opt-in exec is +# proven with a PATH-shadowing stub named "open" that records its argv; the +# real /usr/bin/open is never reached because execlp resolves through PATH. +# +# These tests use -e mode (not --protocol) because the protocol runner cannot +# pass arbitrary unknown flags to the frontier-cli subprocess. + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PROJECT_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +CLI="$PROJECT_ROOT/frontier-cli/frontier-cli" +SOURCE_DB="$PROJECT_ROOT/databases/Frontier.root" + +if [ ! -x "$CLI" ]; then + echo "Error: frontier-cli not found at $CLI" >&2 + exit 1 +fi +if [ ! -f "$SOURCE_DB" ]; then + echo "Error: database not found at $SOURCE_DB" >&2 + exit 1 +fi + +RED='\033[0;31m' +GREEN='\033[0;32m' +NC='\033[0m' + +PASSED=0 +FAILED=0 + +TESTURL="http://127.0.0.1:5336/setupFrontier" + +# Scratch dir holding the PATH stubs, their recording file, and a private copy +# of the database. The CLI opens the system root read-write, so these runs must +# never point at the shared working DB in databases/. +STUBDIR=$(mktemp -d "${TMPDIR:-/tmp}/openurl_guard.XXXXXX") +RECORD="$STUBDIR/exec_record.txt" +DB="$STUBDIR/Frontier.root" + +cleanup() { + rm -rf "$STUBDIR" +} +trap cleanup EXIT + +cp "$SOURCE_DB" "$DB" || { + echo "Error: could not copy $SOURCE_DB to the scratch dir" >&2 + exit 1 +} + +# Stub launchers. Named exactly as the ones execlp() resolves through PATH, so +# a stub firing proves an exec happened and the real browser is never invoked. +for stub in open xdg-open agent-browser; do + cat > "$STUBDIR/$stub" <> "$RECORD" +exit 0 +STUBEOF + chmod +x "$STUBDIR/$stub" +done + +pass() { + echo -e " ${GREEN}PASS${NC}: $1" + PASSED=$((PASSED + 1)) +} + +fail() { + echo -e " ${RED}FAIL${NC}: $1" + echo " $2" + FAILED=$((FAILED + 1)) +} + +# Run the CLI with the stub dir FIRST on PATH so any exec hits a stub. +# The recording file is truncated per run so each assertion is independent. +run_cli() { + : > "$RECORD" + PATH="$STUBDIR:$PATH" "$CLI" "$@" 2>/dev/null +} + +# The double-fork means the grandchild runs asynchronously; give a real exec a +# moment to land before concluding the record is empty. +settle() { + local i + for i in 1 2 3 4 5 6 7 8 9 10; do + if [ -s "$RECORD" ]; then + return + fi + sleep 0.1 + done +} + +echo "==============================================" +echo "Headless sys.openUrl GUI-browser guard (#891)" +echo "==============================================" +echo + +echo "--- Default (no --browser): log and mark, never exec ---" + +actual=$(run_cli --skip-startup --system-root "$DB" \ + -e "sys.openUrl(\"$TESTURL\")") +if [ "$actual" = "true" ]; then + pass "default returns true (onboarding intent honored)" +else + fail "default returns true (onboarding intent honored)" "Got: '$actual'" +fi + +settle +if [ ! -s "$RECORD" ]; then + pass "default performs NO exec" +else + fail "default performs NO exec" "Exec recorded: $(cat "$RECORD")" +fi + +actual=$(run_cli --skip-startup --system-root "$DB" \ + -e "sys.openUrl(\"$TESTURL\");system.temp.Frontier.pendingBrowserUrl") +if [ "$actual" = "$TESTURL" ]; then + pass "default sets pendingBrowserUrl to the URL" +else + fail "default sets pendingBrowserUrl to the URL" "Expected '$TESTURL', got '$actual'" +fi + +echo +echo "--- Explicit --browser default: same as absent ---" + +actual=$(run_cli --skip-startup --system-root "$DB" --browser default \ + -e "sys.openUrl(\"$TESTURL\")") +if [ "$actual" = "true" ]; then + pass "--browser default returns true" +else + fail "--browser default returns true" "Got: '$actual'" +fi + +settle +if [ ! -s "$RECORD" ]; then + pass "--browser default performs NO exec" +else + fail "--browser default performs NO exec" "Exec recorded: $(cat "$RECORD")" +fi + +echo +echo "--- Marker overwrites on each call (last URL wins) ---" + +actual=$(run_cli --skip-startup --system-root "$DB" \ + -e "sys.openUrl(\"http://example.com/first\");sys.openUrl(\"$TESTURL\");system.temp.Frontier.pendingBrowserUrl") +if [ "$actual" = "$TESTURL" ]; then + pass "marker holds the most recent URL" +else + fail "marker holds the most recent URL" "Expected '$TESTURL', got '$actual'" +fi + +echo +echo "--- Opt-in --browser allow-gui: real launcher runs (stubbed) ---" + +actual=$(run_cli --skip-startup --system-root "$DB" --browser allow-gui \ + -e "sys.openUrl(\"$TESTURL\")") +if [ "$actual" = "true" ]; then + pass "allow-gui returns true" +else + fail "allow-gui returns true" "Got: '$actual'" +fi + +settle +if grep -q "$TESTURL" "$RECORD" 2>/dev/null; then + pass "allow-gui EXECS the launcher with the URL" +else + fail "allow-gui EXECS the launcher with the URL" "Record: '$(cat "$RECORD" 2>/dev/null)'" +fi + +echo +echo "--- Opt-in does NOT set the pending marker ---" + +actual=$(run_cli --skip-startup --system-root "$DB" --browser allow-gui \ + -e "sys.openUrl(\"$TESTURL\");defined(system.temp.Frontier.pendingBrowserUrl)") +if [ "$actual" = "false" ]; then + pass "allow-gui leaves pendingBrowserUrl undefined" +else + fail "allow-gui leaves pendingBrowserUrl undefined" "Got: '$actual'" +fi + +echo +echo "--- agent-browser still execs (automation path unchanged) ---" + +actual=$(run_cli --skip-startup --system-root "$DB" --browser agent-browser \ + -e "sys.openUrl(\"$TESTURL\")") +if [ "$actual" = "true" ]; then + pass "agent-browser returns true" +else + fail "agent-browser returns true" "Got: '$actual'" +fi + +settle +if grep -q "^agent-browser .*$TESTURL" "$RECORD" 2>/dev/null; then + pass "agent-browser EXECS agent-browser with the URL" +else + fail "agent-browser EXECS agent-browser with the URL" "Record: '$(cat "$RECORD" 2>/dev/null)'" +fi + +echo +echo "--- Unknown --browser value is rejected ---" + +actual=$(run_cli --skip-startup --system-root "$DB" --browser firefox \ + -e "sys.openUrl(\"$TESTURL\")") +if [ "$actual" != "true" ]; then + pass "unknown browser value does not report success" +else + fail "unknown browser value does not report success" "Got: '$actual'" +fi + +settle +if [ ! -s "$RECORD" ]; then + pass "unknown browser value performs NO exec" +else + fail "unknown browser value performs NO exec" "Exec recorded: $(cat "$RECORD")" +fi + +echo +echo "--- Empty URL still errors before any fork ---" + +actual=$(run_cli --skip-startup --system-root "$DB" -e 'sys.openUrl("")') +if [ "$actual" != "true" ]; then + pass "empty URL does not report success" +else + fail "empty URL does not report success" "Got: '$actual'" +fi + +settle +if [ ! -s "$RECORD" ]; then + pass "empty URL performs NO exec" +else + fail "empty URL performs NO exec" "Exec recorded: $(cat "$RECORD")" +fi + +echo +echo "==============================================" +echo "Passed: $PASSED Failed: $FAILED" +echo "==============================================" + +if [ "$FAILED" -ne 0 ]; then + exit 1 +fi +exit 0