Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
48 changes: 45 additions & 3 deletions Common/source/shellsysverbs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand All @@ -903,21 +927,39 @@ 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);
}
}
}
}
}
}

/* #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
Expand All @@ -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
Expand Down
8 changes: 6 additions & 2 deletions frontier-cli/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
11 changes: 9 additions & 2 deletions tests/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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 \
Expand Down
134 changes: 129 additions & 5 deletions tests/headless_sys_verbs.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@
#include "standard.h"

#include <stdlib.h> /* for getenv, setenv */
#include <string.h> /* 2026-08-15 JES #891: for strlen in setpendingbrowserurl */
#include <unistd.h> /* for fork, execlp, _exit */
#include <sys/wait.h> /* for waitpid */
#include "memory.h"
#include "strings.h"
#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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
}
Expand All @@ -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 ();
Expand All @@ -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
Expand Down
Loading
Loading