From 2f7ec561cd7d81a7aa36b64b96c36d475e948c64 Mon Sep 17 00:00:00 2001 From: Jake Savin Date: Tue, 11 Aug 2026 00:07:31 -0700 Subject: [PATCH 1/5] fix(cli): #859 -- derive Frontier.pathString from the loaded system root Frontier.pathString anchors every Frontier.getSubFolder() call, but the only writer was startupScript.ut:60, which never runs under --skip-startup or --protocol. On those surfaces -- the entire integration-test and protocol tooling surface -- pathString kept whatever absolute path was serialized into the shipped root. In Virgin.root that was a deleted worktree directory, so getSubFolder built paths under a nonexistent parent and file.newFolder threw "Can't create folder" (getSubFolder.ut:10). That is what stranded startupScript above the #849 guards when firstRootRun was true (#855). A shipped absolute path cannot be correct in the dist, repo, and staged-test layouts at once, so compute it at load instead of storing it. Virgin.root is not modified. Root-relative, not binary-relative: "Guest Databases" is a sibling of the ROOT FILE in every layout, while the binary lives in frontier-cli/ with no such sibling. startupScript already encodes this distinction -- line 48 uses getProgramPath() for the app folder, line 60 uses getFilePath() for pathString. Deriving from the root makes the two agree, so a later real startup recomputes the identical value and the with-startup path is unchanged. The helper calls frontier.getFilePath() inside the script rather than interpolating the C-side path into the source text, keeping a filesystem path out of a UserTalk string literal entirely so there is no injection surface. Failures are logged, not fatal. Tests: new frontier_pathstring_init.yaml asserts behaviorally (no hardcoded absolute paths) that pathString exists, tracks the root folder, and that getSubFolder can both resolve and create beneath it. Red before the fix (4/5 failing), green after (5/5). --- frontier-cli/main.c | 82 +++++++++++++ .../test_cases/frontier_pathstring_init.yaml | 116 ++++++++++++++++++ 2 files changed, 198 insertions(+) create mode 100644 tests/integration/test_cases/frontier_pathstring_init.yaml diff --git a/frontier-cli/main.c b/frontier-cli/main.c index d678127d3..cc08d7f42 100644 --- a/frontier-cli/main.c +++ b/frontier-cli/main.c @@ -181,6 +181,79 @@ const char *cli_get_system_root_basename(void) { return bn_buf; } +/* + * langrunstringnoerror: compile and run a UserTalk bigstring expression. + * Declared in lang.h/langinternal.h; also used by window_registry.c. + */ +extern boolean langrunstringnoerror(const bigstring bsprogram, bigstring bsresult); + +/* + * cli_init_frontier_pathstring -- set Frontier.pathString from the loaded root. + * + * Issue #859. Frontier.pathString anchors every Frontier.getSubFolder() call + * (getSubFolder.ut:5 builds pathString + "Guest Databases" + pathChar). Until + * now the only writer was startupScript.ut:60: + * + * Frontier.pathstring = file.folderFromPath (frontier.getFilePath ()) + * + * which never runs under --skip-startup or --protocol. On those surfaces + * pathString kept whatever absolute path was serialized into the shipped root + * -- in Virgin.root, a deleted worktree directory. getSubFolder then built + * paths under a nonexistent parent and file.newFolder threw "Can't create + * folder" (getSubFolder.ut:10), which is what stranded startupScript above the + * #849 guards when firstRootRun was true (#855). + * + * A shipped absolute path cannot be correct: the dist layout, the repo layout, + * and the per-worker staged-test layout put the root in three different places. + * So the value is computed at load instead of stored. + * + * ROOT-relative, not BINARY-relative: "Guest Databases" is a sibling of the + * ROOT FILE in every layout (dist/, databases/, and the staged worker dir), + * while the binary lives in frontier-cli/ with no such sibling. startupScript + * already encodes this distinction deliberately -- line 48 uses + * frontier.getProgramPath() for the app folder, line 60 uses + * frontier.getFilePath() for pathString. Deriving from the root keeps this + * function and startupScript in agreement, so a later real startup recomputes + * the identical value and the with-startup path is unchanged. + * + * The script calls frontier.getFilePath() rather than interpolating the C-side + * path into the source text. That is deliberate: it keeps a filesystem path -- + * which can contain quotes or other metacharacters -- out of a UserTalk string + * literal entirely, so there is no injection surface to escape (contrast the + * langdeparsestring contract fire_window_script must honor in window_registry.c). + * + * Boot-failure-safe: a failure here is logged and ignored rather than aborting + * the load. A minimal headless root may have no Frontier table at all, and a + * runtime that boots with a stale pathString is no worse off than before this + * function existed. + * + * Must be called AFTER g_system_root_path/g_system_root_loaded are set, since + * frontier.getFilePath() reads them. + */ +static void cli_init_frontier_pathstring(void) { + bigstring bs_program; + bigstring bs_result; + + if (!copyctopstring("Frontier.pathString = file.folderFromPath (frontier.getFilePath ())", + bs_program)) { + log_warn(LOG_COMP_STARTUP, + "cli_init_frontier_pathstring: script text exceeded 255 bytes; " + "Frontier.pathString not initialized"); + return; + } + + if (!langrunstringnoerror(bs_program, bs_result)) { + log_warn(LOG_COMP_STARTUP, + "cli_init_frontier_pathstring: failed to set Frontier.pathString from %s -- " + "Frontier.getSubFolder() paths may be wrong", + g_system_root_path); + return; + } + + log_debug(LOG_COMP_STARTUP, "Frontier.pathString initialized from system root: %s", + g_system_root_path); +} + /* * .ut import boot-time re-dirty infrastructure. * @@ -1837,6 +1910,11 @@ static boolean hydrate_system_root_database(const char* path, boolean read_only) snprintf(g_system_root_path, sizeof(g_system_root_path), "%s", path); g_system_root_loaded = true; + /* Issue #859 -- derive Frontier.pathString from the root just loaded, so it is + * correct on boot modes that never run startupScript (--skip-startup, --protocol). + * Must follow the two assignments above; see the function comment. */ + cli_init_frontier_pathstring(); + /* NOTE: Startup scripts are NOT run here during hydration. * They are run in main() AFTER hydration completes, which ensures: * 1. EFP tables are properly linked (linksystemtablestructure) @@ -2124,6 +2202,10 @@ static boolean load_system_root_database_internal(const char* path, boolean allo snprintf(g_system_root_path, sizeof(g_system_root_path), "%s", path); g_system_root_loaded = true; + /* Issue #859 -- see cli_init_frontier_pathstring(). Both load paths need this: + * this one runs when hydration is skipped (allow_hydrate false). */ + cli_init_frontier_pathstring(); + /* NOTE: Startup scripts are NOT run here. They are run in main() AFTER * hydrate_system_root_database() completes, which ensures EFP tables are * properly linked and system.paths is resolved before scripts execute. diff --git a/tests/integration/test_cases/frontier_pathstring_init.yaml b/tests/integration/test_cases/frontier_pathstring_init.yaml new file mode 100644 index 000000000..dc22241bb --- /dev/null +++ b/tests/integration/test_cases/frontier_pathstring_init.yaml @@ -0,0 +1,116 @@ +# Integration tests for Frontier.pathString initialization (issue #859). +# +# BACKGROUND: +# Frontier.pathString is the anchor every Frontier.getSubFolder() call builds on +# (getSubFolder.ut:5 -> pathString + "Guest Databases" + pathChar). Historically +# only startupScript.ut:60 ever set it: +# +# Frontier.pathstring = file.folderFromPath (frontier.getFilePath ()) +# +# The integration runner and all protocol tooling boot with --skip-startup, so +# that line never ran and pathString kept whatever absolute path happened to be +# serialized into the shipped root. In Virgin.root that was a deleted worktree +# directory, which made it LIVE and wrong for the entire test/protocol surface: +# getSubFolder built paths under a nonexistent parent and file.newFolder threw +# "Can't create folder" (getSubFolder.ut:10). That is what stranded startupScript +# above the #849 guards when firstRootRun=true (#855). +# +# A shipped absolute path is wrong by definition -- no stored string can be right +# in the dist layout, the repo layout, and the per-worker staged-test layout at +# once. The fix is therefore compute-at-load, C-side, so it covers every boot mode +# including --skip-startup and --protocol. +# +# ROOT-relative, not BINARY-relative: "Guest Databases" is a sibling of the ROOT +# FILE in every layout (dist/, databases/, and the staged worker dir the runner +# builds), while the binary lives in frontier-cli/ with no such sibling. Legacy +# startupScript already encodes this distinction deliberately -- line 48 uses +# frontier.getProgramPath() for the app folder, line 60 uses frontier.getFilePath() +# for pathString. +# +# These tests assert the invariant behaviorally: pathString must point at a folder +# that EXISTS, and getSubFolder() must be able to resolve/create beneath it. They +# do not hardcode any absolute path, so they hold in every layout. +# +# All folder creation is confined to the staged system-root directory that the +# runner builds per worker (tests/tmp/integration/worker_N), never the canonical +# databases/ tree and never anywhere outside test staging. + +tests: + # ============================================================ + # Test 1: pathString is set at load, without a startup script + # ============================================================ + - name: "pathString: defined and non-empty under --skip-startup" + description: | + The runner boots with --skip-startup, so startupScript.ut:60 never executes. + pathString must still be populated by the load-time initialization. + timeout: 20 + script: | + return defined(Frontier.pathString) and (sizeOf(Frontier.pathString) > 0) + expected_success: true + expected_result: "true" + + # ============================================================ + # Test 2: pathString points at a folder that actually exists + # ============================================================ + - name: "pathString: resolves to an existing folder" + description: | + The regression that motivated #859: pathString held a deleted worktree path, + so file.exists() was false and every getSubFolder() call built paths under a + nonexistent parent. Asserting existence (not a literal path) keeps this test + valid in the dist, repo, and staged-test layouts alike. + timeout: 20 + script: | + return file.exists(Frontier.pathString) + expected_success: true + expected_result: "true" + + # ============================================================ + # Test 3: pathString tracks the loaded ROOT file, not the binary + # ============================================================ + - name: "pathString: equals the folder of the loaded system root" + description: | + Pins the semantics decided in #859. pathString must equal + file.folderFromPath(frontier.getFilePath()) -- the folder containing the + loaded root -- which is what startupScript.ut:60 computes on the + with-startup path. This makes the two paths agree, so the load-time + initialization is idempotent with respect to a later real startup. + timeout: 20 + script: | + return Frontier.pathString == file.folderFromPath(frontier.getFilePath()) + expected_success: true + expected_result: "true" + + # ============================================================ + # Test 4: getSubFolder resolves beneath pathString + # ============================================================ + - name: "pathString: getSubFolder resolves an existing subfolder" + description: | + End-to-end check of the consumer that actually broke. getSubFolder("ops/backups/") + is the exact call firstRootRun's backups.init() makes; under the stale pathString + it threw "Can't create folder" at getSubFolder.ut:10. Asserts the returned path + exists rather than comparing strings. + timeout: 20 + script: | + local (f = Frontier.getSubFolder("ops/backups/")); + return file.exists(f) + expected_success: true + expected_result: "true" + + # ============================================================ + # Test 5: getSubFolder can CREATE a new subfolder + # ============================================================ + - name: "pathString: getSubFolder creates a new subfolder successfully" + description: | + The stale-pathString failure mode was specifically a creation failure, so + exercise creation rather than only lookup of a folder that already ships. + Uses a test-specific name under the staged Guest Databases tree and deletes + it again, leaving no residue for later tests in the shared sequential session. + timeout: 20 + script: | + local (f); + f = Frontier.getSubFolder("ops/pathStringInitTest/"); + local (created = file.exists(f)); + try {file.deleteFolder(f)}; + return created + expected_success: true + expected_result: "true" From cd33a60bb80c8266e5974fdb420e4967118dd360 Mon Sep 17 00:00:00 2001 From: Jake Savin Date: Tue, 11 Aug 2026 00:24:52 -0700 Subject: [PATCH 2/5] fix(cli): #859 -- pin trailing separator, document persistence consequence Folds in review requirements on top of the pathString load-time init. - New test asserts pathString keeps its trailing path separator. About twenty call sites concatenate onto it directly instead of going through getSubFolder (log/startup.ut:11, cleanRoot.ut:178), where a missing separator would silently yield ".../FrontierLogs" instead of ".../Frontier/Logs". - Document the persistence consequence in the helper comment: a read-write session that saves on exit (#127) persists the local absolute path into the stored cell, but the stored value is dead by construction because it is unconditionally overwritten at every load. Verified by saving a root carrying one pathString and loading that same file from a different directory -- the new location wins. Impact is byte-remanence/privacy (#854) and diff noise, not correctness. Future path-pollution walkers should treat Frontier.pathstring as dead-by-construction rather than flagging it. - Verified --lock-opened-roots does not persist it: file byte-identical. --- frontier-cli/main.c | 19 ++++++++++++++++ .../test_cases/frontier_pathstring_init.yaml | 22 +++++++++++++++++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/frontier-cli/main.c b/frontier-cli/main.c index cc08d7f42..fa0186448 100644 --- a/frontier-cli/main.c +++ b/frontier-cli/main.c @@ -222,6 +222,25 @@ extern boolean langrunstringnoerror(const bigstring bsprogram, bigstring bsresul * literal entirely, so there is no injection surface to escape (contrast the * langdeparsestring contract fire_window_script must honor in window_registry.c). * + * The value keeps the trailing path separator that file.folderFromPath returns. + * Roughly twenty call sites concatenate onto pathString directly rather than going + * through getSubFolder (log/startup.ut:11 builds pathString + "Logs" + pathChar, + * cleanRoot.ut:178 builds pathString + "Virgin.root"), so dropping the separator + * would silently yield ".../FrontierLogs" instead of ".../Frontier/Logs". + * + * PERSISTENCE CONSEQUENCE (by design; do not "fix" by skipping the write): + * this sets the in-memory Frontier table, so a read-write session that saves on + * exit (#127) persists the running machine's absolute path into the stored cell. + * That is the mechanism that produced #859's original pollution, but the impact is + * now confined to byte-remanence/privacy (#854's domain) and git-diff noise on + * databases/Virgin.root -- it is NOT a correctness bug, because the stored value is + * dead by construction: unconditionally overwritten at every load before any script + * can read it. Verified by saving a root carrying one pathString and then loading + * that same file from a different directory -- the new location wins. + * Corollary for future path-pollution walkers: treat Frontier.pathstring as + * dead-by-construction rather than flagging it as a live polluted reference. + * Inspection sessions (--lock-opened-roots) do not persist it at all. + * * Boot-failure-safe: a failure here is logged and ignored rather than aborting * the load. A minimal headless root may have no Frontier table at all, and a * runtime that boots with a stale pathString is no worse off than before this diff --git a/tests/integration/test_cases/frontier_pathstring_init.yaml b/tests/integration/test_cases/frontier_pathstring_init.yaml index dc22241bb..9c5fa99d8 100644 --- a/tests/integration/test_cases/frontier_pathstring_init.yaml +++ b/tests/integration/test_cases/frontier_pathstring_init.yaml @@ -81,7 +81,25 @@ tests: expected_result: "true" # ============================================================ - # Test 4: getSubFolder resolves beneath pathString + # Test 4: pathString ends with the platform path separator + # ============================================================ + - name: "pathString: ends with the platform path separator" + description: | + Roughly twenty call sites concatenate onto pathString directly instead of + going through getSubFolder -- log/startup.ut:11 builds + pathString + "Logs" + pathChar, cleanRoot.ut:178 builds + pathString + "Virgin.root". A value missing its trailing separator would + silently produce ".../FrontierLogs" instead of ".../Frontier/Logs", so the + invariant is pinned here rather than left implicit in the behavior of + file.folderFromPath. + timeout: 20 + script: | + return Frontier.pathString endsWith file.getPathChar() + expected_success: true + expected_result: "true" + + # ============================================================ + # Test 5: getSubFolder resolves beneath pathString # ============================================================ - name: "pathString: getSubFolder resolves an existing subfolder" description: | @@ -97,7 +115,7 @@ tests: expected_result: "true" # ============================================================ - # Test 5: getSubFolder can CREATE a new subfolder + # Test 6: getSubFolder can CREATE a new subfolder # ============================================================ - name: "pathString: getSubFolder creates a new subfolder successfully" description: | From 867a8d8a27a28abee4e2e6b8629955d8c1b96c9f Mon Sep 17 00:00:00 2001 From: Jake Savin Date: Tue, 11 Aug 2026 00:46:08 -0700 Subject: [PATCH 3/5] fix(cli): #859 -- compute pathString in pure C, no interpreter at load Gate findings. Replaces the langrunstringnoerror eval with a pure-C computation, adds compare-before-assign, and pins the security property. SECURITY (the driver). Keeping the path out of the source text closed string-literal injection but not VERB RESOLUTION: file.folderFromPath and frontier.getFilePath resolve through the loaded root's tables before the kernel efptable, and verb shadowing is a supported feature. A hostile root storing a script at either path got arbitrary UserTalk executed at load -- including under --skip-startup --lock-opened-roots, an inspection posture that otherwise runs no root code. Demonstrated against the previous build: a root shadowing file.folderFromPath made pathString "/tmp/pwned/" and ran the attacker's side effect. Computing in C removes the interpreter from the load path. Two further hazards retire with it: langrunstringnoerror releases the GIL for up to 1ms per call in headless builds (fldisableyield is Mac-only and headless_backgroundtask never checks it), and it force-clears fllangerror. COMPARE BEFORE ASSIGN. hashtableassign dirties unconditionally, so an unconditional write would dirty the root every load and defeat the clear_post_hydration_dirty_flags save-skip (#127) -- every RW session would rewrite the file and every worktree ODB edit would carry a spurious pathString delta. Now written only when the stored value differs. Verified both branches fire, and that an RW open+exit with no mutation leaves the file byte-identical. Implementation notes: - Resolution is an explicit walk of system.verbs.builtins.Frontier. The bare name "Frontier" is not a root-level table; it only resolves through UserTalk's verb search. Every step is a lookup, never a create. - Uses portable_folderfrompath, the primitive backing the file.folderFromPath verb. The Mac-only folderfrompath in file.h has NO definition in the headless build; it links via -undefined dynamic_lookup and segfaults when called. - Absolutizes a relative --system-root against cwd, matching the frontier.getFilePath verb. Without it, --system-root databases/Virgin.root yielded the relative pathString "databases/". - Assignment goes through langassignstringvalue, which owns its handle and disposes it on failure, so no tmp-stack exemption is required. - Drops the now-unused langrunstringnoerror extern. Tests: new cli_pathstring_no_eval_test.sh installs a shadowing file.folderFromPath in a throwaway root and asserts the script does not run at load and that pathString is the real folder. Red on the eval build (shadow executed) and red on develop (no init at all); green here. Wired into run_integration_tests.sh. Full suite 2483 / 0 failed / 16 baselined. --- frontier-cli/main.c | 156 ++++++++++++++---- .../cli_pathstring_no_eval_test.sh | 125 ++++++++++++++ tools/run_integration_tests.sh | 17 ++ 3 files changed, 268 insertions(+), 30 deletions(-) create mode 100755 tests/integration/cli_pathstring_no_eval_test.sh diff --git a/frontier-cli/main.c b/frontier-cli/main.c index fa0186448..43229616e 100644 --- a/frontier-cli/main.c +++ b/frontier-cli/main.c @@ -181,11 +181,11 @@ const char *cli_get_system_root_basename(void) { return bn_buf; } -/* - * langrunstringnoerror: compile and run a UserTalk bigstring expression. - * Declared in lang.h/langinternal.h; also used by window_registry.c. - */ -extern boolean langrunstringnoerror(const bigstring bsprogram, bigstring bsresult); +/* Forward declaration from file_portable_posix.c -- the primitive that backs the + * file.folderFromPath verb (portable/fileverbs_portable.c:733). The Mac-only + * folderfrompath declared in file.h has NO definition in the headless build; the + * link succeeds via -undefined dynamic_lookup and then segfaults when called. */ +extern boolean portable_folderfrompath(const bigstring bspath, bigstring bsfolder); /* * cli_init_frontier_pathstring -- set Frontier.pathString from the loaded root. @@ -216,11 +216,21 @@ extern boolean langrunstringnoerror(const bigstring bsprogram, bigstring bsresul * function and startupScript in agreement, so a later real startup recomputes * the identical value and the with-startup path is unchanged. * - * The script calls frontier.getFilePath() rather than interpolating the C-side - * path into the source text. That is deliberate: it keeps a filesystem path -- - * which can contain quotes or other metacharacters -- out of a UserTalk string - * literal entirely, so there is no injection surface to escape (contrast the - * langdeparsestring contract fire_window_script must honor in window_registry.c). + * NO INTERPRETER, BY DESIGN (security). An earlier revision evaluated the + * UserTalk one-liner above via langrunstringnoerror. Keeping the path out of the + * source text closed string-literal injection, but not VERB RESOLUTION: the names + * file.folderFromPath and frontier.getFilePath resolve through the loaded root's + * own tables before the kernel efptable, and verb shadowing is a supported feature + * (docs/VERB_RESOLUTION_ARCHITECTURE.md). A hostile root that stores a script at + * either path therefore got arbitrary UserTalk executed at load -- including under + * --skip-startup --lock-opened-roots, an inspection posture that otherwise runs no + * root code at all. Demonstrated before this rewrite: a root with a shadowing + * file.folderFromPath made pathString "/tmp/pwned/" and ran the attacker's side + * effect. Computing in C removes the interpreter from the load path entirely. + * Two further hazards retire with it: langrunstringnoerror releases the GIL for up + * to 1ms per call in headless builds (fldisableyield is Mac-only and + * headless_backgroundtask never checks it), and it force-clears fllangerror. + * Do not reintroduce an eval here. * * The value keeps the trailing path separator that file.folderFromPath returns. * Roughly twenty call sites concatenate onto pathString directly rather than going @@ -228,15 +238,22 @@ extern boolean langrunstringnoerror(const bigstring bsprogram, bigstring bsresul * cleanRoot.ut:178 builds pathString + "Virgin.root"), so dropping the separator * would silently yield ".../FrontierLogs" instead of ".../Frontier/Logs". * - * PERSISTENCE CONSEQUENCE (by design; do not "fix" by skipping the write): - * this sets the in-memory Frontier table, so a read-write session that saves on - * exit (#127) persists the running machine's absolute path into the stored cell. - * That is the mechanism that produced #859's original pollution, but the impact is - * now confined to byte-remanence/privacy (#854's domain) and git-diff noise on - * databases/Virgin.root -- it is NOT a correctness bug, because the stored value is - * dead by construction: unconditionally overwritten at every load before any script - * can read it. Verified by saving a root carrying one pathString and then loading - * that same file from a different directory -- the new location wins. + * COMPARE BEFORE ASSIGN. The cell is written only when the computed value differs + * from what is already stored. hashtableassign dirties the node unconditionally, so + * an unconditional write would dirty the root on every load and defeat the + * clear_post_hydration_dirty_flags save-skip (#127) -- every read-write session + * would rewrite the file, and every worktree ODB edit would carry a spurious + * pathString delta in the committed binary. Skipping the no-op write keeps a + * clean-tree session clean. + * + * PERSISTENCE CONSEQUENCE of the write that does happen (by design): it sets the + * in-memory Frontier table, so a read-write session that saves on exit persists the + * running machine's absolute path into the stored cell. That is the mechanism that + * produced #859's original pollution, but the impact is confined to + * byte-remanence/privacy (#854's domain) and diff noise -- it is NOT a correctness + * bug, because the stored value is dead by construction: recomputed at every load + * before any script can read it. Verified by saving a root carrying one pathString + * and loading that same file from a different directory -- the new location wins. * Corollary for future path-pollution walkers: treat Frontier.pathstring as * dead-by-construction rather than flagging it as a live polluted reference. * Inspection sessions (--lock-opened-roots) do not persist it at all. @@ -246,31 +263,110 @@ extern boolean langrunstringnoerror(const bigstring bsprogram, bigstring bsresul * runtime that boots with a stale pathString is no worse off than before this * function existed. * - * Must be called AFTER g_system_root_path/g_system_root_loaded are set, since - * frontier.getFilePath() reads them. + * Must be called AFTER g_system_root_path is set and roottable is assigned. + * + * The Frontier table is looked up, never created: findnamedtable is a read-only + * resolve. A root without one is left alone rather than gaining a new table. */ static void cli_init_frontier_pathstring(void) { - bigstring bs_program; - bigstring bs_result; + bigstring bsfolder; + bigstring bspath; + bigstring bsexisting; + hdlhashtable frontiertable = nil; + tyvaluerecord val; + hdlhashnode hnode = nil; + char abspath[CLI_MAX_PATH_LENGTH * 2 + 2]; + const char *rootpath = g_system_root_path; + + if (roottable == nil || g_system_root_path[0] == '\0') + return; + + /* Absolutize a relative --system-root against the cwd before deriving the + * folder, matching what the frontier.getFilePath verb does + * (tests/headless_frontier_verbs.c). Without this, `--system-root + * databases/Virgin.root` would yield the relative pathString "databases/", + * and every getSubFolder consumer would resolve against whatever the process + * cwd happened to be. */ + if (g_system_root_path[0] != '/') { + char cwd[CLI_MAX_PATH_LENGTH + 1]; + + if (getcwd(cwd, sizeof(cwd)) == NULL) { + log_warn(LOG_COMP_STARTUP, + "cli_init_frontier_pathstring: getcwd failed; " + "Frontier.pathString not initialized"); + return; + } + + if (snprintf(abspath, sizeof(abspath), "%s/%s", cwd, g_system_root_path) + >= (int) sizeof(abspath)) { + log_warn(LOG_COMP_STARTUP, + "cli_init_frontier_pathstring: absolute path too long; " + "Frontier.pathString not initialized"); + return; + } + rootpath = abspath; + } - if (!copyctopstring("Frontier.pathString = file.folderFromPath (frontier.getFilePath ())", - bs_program)) { + if (!copyctopstring(rootpath, bspath)) { log_warn(LOG_COMP_STARTUP, - "cli_init_frontier_pathstring: script text exceeded 255 bytes; " + "cli_init_frontier_pathstring: system root path exceeds 255 bytes; " "Frontier.pathString not initialized"); return; } - if (!langrunstringnoerror(bs_program, bs_result)) { + /* Same derivation as file.folderFromPath: everything up to and including the + * last separator, so the trailing separator the concatenation sites rely on + * is preserved. */ + if (!portable_folderfrompath(bspath, bsfolder) || stringlength(bsfolder) == 0) { + log_warn(LOG_COMP_STARTUP, + "cli_init_frontier_pathstring: could not derive folder from %s; " + "Frontier.pathString not initialized", + rootpath); + return; + } + + /* The Frontier table is NOT at root level -- it lives at + * system.verbs.builtins.Frontier, and the bare name "Frontier" only resolves + * through UserTalk's verb search. Walk the explicit path so this stays a + * plain table lookup with no interpreter involved. Each step is a lookup, not + * a create: a root missing any segment is left untouched. */ + if (!findnamedtable(roottable, BIGSTRING("\006" "system"), &frontiertable) + || frontiertable == nil + || !findnamedtable(frontiertable, BIGSTRING("\005" "verbs"), &frontiertable) + || frontiertable == nil + || !findnamedtable(frontiertable, BIGSTRING("\010" "builtins"), &frontiertable) + || frontiertable == nil + || !findnamedtable(frontiertable, BIGSTRING("\010" "Frontier"), &frontiertable) + || frontiertable == nil) { + log_debug(LOG_COMP_STARTUP, + "cli_init_frontier_pathstring: system.verbs.builtins.Frontier not " + "resolvable in this root; skipping"); + return; + } + + /* Compare before assign -- see the header comment. hashtablelookup matches + * case-insensitively, so this finds the cell whichever way it is spelled, and + * assigning through the same key preserves the stored node rather than adding + * a sibling. */ + if (hashtablelookup(frontiertable, BIGSTRING("\012" "pathString"), &val, &hnode) + && val.valuetype == stringvaluetype + && hashgetvaluestring(val, bsexisting) + && equalstrings(bsexisting, bsfolder)) { + log_debug(LOG_COMP_STARTUP, + "Frontier.pathString already current; leaving node clean"); + return; + } + + if (!langassignstringvalue(frontiertable, BIGSTRING("\012" "pathString"), bsfolder)) { log_warn(LOG_COMP_STARTUP, - "cli_init_frontier_pathstring: failed to set Frontier.pathString from %s -- " + "cli_init_frontier_pathstring: failed to assign Frontier.pathString from %s -- " "Frontier.getSubFolder() paths may be wrong", - g_system_root_path); + rootpath); return; } log_debug(LOG_COMP_STARTUP, "Frontier.pathString initialized from system root: %s", - g_system_root_path); + rootpath); } /* diff --git a/tests/integration/cli_pathstring_no_eval_test.sh b/tests/integration/cli_pathstring_no_eval_test.sh new file mode 100755 index 000000000..ba1c2fc1d --- /dev/null +++ b/tests/integration/cli_pathstring_no_eval_test.sh @@ -0,0 +1,125 @@ +#!/bin/bash +# +# Security regression test for issue #859: initializing Frontier.pathString at +# load must NOT invoke the UserTalk interpreter. +# +# BACKGROUND +# The first fix for #859 set pathString by evaluating the same one-liner +# startupScript.ut:60 uses: +# +# Frontier.pathString = file.folderFromPath (frontier.getFilePath ()) +# +# Keeping the path out of the source text closed string-literal injection, but +# not VERB RESOLUTION. The names file.folderFromPath and frontier.getFilePath +# resolve through the loaded root's own tables BEFORE the kernel efptable, and +# verb shadowing is a supported feature (docs/VERB_RESOLUTION_ARCHITECTURE.md). +# So a hostile root that stores a script at either path got arbitrary UserTalk +# executed at load -- including under --skip-startup --lock-opened-roots, an +# inspection posture that otherwise runs no root code at all. +# +# Measured against the eval-based build, this exact fixture yielded: +# pathString=[/tmp/pwned/] shadowRan=true +# +# The fix computes the folder in C (portable_folderfrompath) and assigns it via +# the hash-table APIs, so no interpreter participates in the load path. +# +# WHAT THIS TEST DOES +# Builds a throwaway root whose file.folderFromPath is shadowed by a script that +# records a side effect and returns an attacker-chosen path, then loads it in the +# inspection posture and asserts: +# 1. the shadowing script did NOT run (no arbitrary code execution at load) +# 2. pathString is the REAL root folder, not the attacker's value +# +# All work happens in tests/tmp/integration; the canonical databases/ tree is +# never touched (the fixture is a copy). + +set -u + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PROJECT_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" +CLI="$PROJECT_ROOT/frontier-cli/frontier-cli" +SOURCE_ROOT="$PROJECT_ROOT/databases/Virgin.root" + +RED='\033[0;31m' +GREEN='\033[0;32m' +NC='\033[0m' + +echo "================================================" +echo " pathString load-time init runs no root code (issue #859)" +echo "================================================" +echo + +if [ ! -x "$CLI" ]; then + echo "SKIP: frontier-cli not built at $CLI" + exit 0 +fi +if [ ! -f "$SOURCE_ROOT" ]; then + echo "SKIP: source root not found at $SOURCE_ROOT" + exit 0 +fi + +WORK_DIR="$(mktemp -d "$PROJECT_ROOT/tests/tmp/integration/pathstring_no_eval.XXXXXX")" || { + echo "SKIP: could not create work dir" + exit 0 +} +trap 'rm -rf "$WORK_DIR"' EXIT + +HOSTILE_ROOT="$WORK_DIR/hostile.root" +cp "$SOURCE_ROOT" "$HOSTILE_ROOT" + +FAILURES=0 +pass() { printf "${GREEN}PASS${NC}: %s\n" "$1"; } +fail() { printf "${RED}FAIL${NC}: %s\n" "$1"; FAILURES=$((FAILURES + 1)); } + +# Install the shadowing verb. Writing it is expected to succeed -- shadowing is a +# supported feature; the point of the test is that it must not be REACHED at load. +INSTALL_SCRIPT='local (src); +src = "on folderFromPath (p) {" + "\r" + "\tuser.SHADOW_EXECUTED = 1;" + "\r" + "\treturn (\"/tmp/pwned/\")}"; +script.newScriptObject (src, @file.folderFromPath); +return ("compiles=" + defined (file.folderFromPath))' + +INSTALL_OUT="$("$CLI" --skip-startup --system-root "$HOSTILE_ROOT" -e "$INSTALL_SCRIPT" 2>&1 | tail -1)" + +if [[ "$INSTALL_OUT" != *"compiles=true"* ]]; then + echo "SKIP: could not install shadowing fixture (got: $INSTALL_OUT)" + echo " Without a compiled shadow the assertions below would pass vacuously." + exit 0 +fi +echo "Fixture installed: file.folderFromPath is shadowed in $HOSTILE_ROOT" +echo + +# Load the hostile root in the inspection posture and report both facts at once. +PROBE='return "shadowRan=" + defined (user.SHADOW_EXECUTED) + " path=" + Frontier.pathString' +PROBE_OUT="$("$CLI" --skip-startup --lock-opened-roots --system-root "$HOSTILE_ROOT" -e "$PROBE" 2>&1 | tail -1)" + +echo "Probe output: $PROBE_OUT" +echo + +# 1. No arbitrary code execution at load. +if [[ "$PROBE_OUT" == *"shadowRan=false"* ]]; then + pass "shadowing script did not execute during load" +elif [[ "$PROBE_OUT" == *"shadowRan=true"* ]]; then + fail "shadowing script EXECUTED at load -- arbitrary code execution from a hostile root" +else + fail "could not determine whether the shadow ran (output: $PROBE_OUT)" +fi + +# 2. pathString reflects the real root folder, not the attacker's return value. +if [[ "$PROBE_OUT" == *"/tmp/pwned/"* ]]; then + fail "pathString was set from the shadowed verb's return value" +elif [[ "$PROBE_OUT" == *"path=$WORK_DIR/"* ]]; then + pass "pathString derived from the real root location" +else + fail "pathString is not the hostile root's folder (expected $WORK_DIR/, output: $PROBE_OUT)" +fi + +echo +echo "================================================" +if [ $FAILURES -eq 0 ]; then + echo " Results: all checks passed, 0 failed" + echo "================================================" + exit 0 +fi +echo " Results: $FAILURES check(s) failed" +echo "================================================" +exit 1 diff --git a/tools/run_integration_tests.sh b/tools/run_integration_tests.sh index 19eaf95fe..60bc45325 100755 --- a/tools/run_integration_tests.sh +++ b/tools/run_integration_tests.sh @@ -364,6 +364,23 @@ if [ "$ORIG_ARG_COUNT" -eq 0 ] && [ -x "$SYSTEM_ROOT_TEMP_PATH_TEST" ]; then fi fi +# Shell-based security regression for issue #859: the load-time +# Frontier.pathString initialization must not invoke the UserTalk interpreter, +# because verb resolution would let a hostile root shadow file.folderFromPath +# and get arbitrary code executed at load -- even under the +# --skip-startup --lock-opened-roots inspection posture. Needs a throwaway root +# with a shadowing verb installed, so it lives outside the yaml suite. +# Self-skips if the fixture cannot be built. +PATHSTRING_NO_EVAL_TEST="$PROJECT_ROOT/tests/integration/cli_pathstring_no_eval_test.sh" +if [ "$ORIG_ARG_COUNT" -eq 0 ] && [ -x "$PATHSTRING_NO_EVAL_TEST" ]; then + echo + PATHSTRING_NO_EVAL_RC=0 + "$PATHSTRING_NO_EVAL_TEST" || PATHSTRING_NO_EVAL_RC=$? + if [ $PATHSTRING_NO_EVAL_RC -ne 0 ]; then + EXIT_CODE=$PATHSTRING_NO_EVAL_RC + fi +fi + # Verify integrity of every staged database after tests. # # Drift is reported as a warning only and does NOT fail EXIT_CODE. This From c39a450176ad310b5bbc1e7a19785d1d892d4818 Mon Sep 17 00:00:00 2001 From: Jake Savin Date: Tue, 11 Aug 2026 00:49:49 -0700 Subject: [PATCH 4/5] docs(cli): #859 -- document the dirty-clear ordering at both pathString sites Carry-forward from the concurrency verdict. Comment-only; no behavior change. The two load paths are asymmetric. In hydrate_system_root_database the pathString write is followed by clear_post_hydration_dirty_flags(hroot), which wipes the dirty bit it sets -- desirable, but incidental to statement order. load_system_root_database_internal has no equivalent clear, so a dirty bit set there survives toward save_system_root_on_exit and would rewrite the whole root at shutdown. Compare-before-assign already neutralizes the common case on both paths, but a later reorder could quietly reintroduce per-shutdown full-root rewrites (the ~1.2 MB drift #127 removed). Each site now states its own ordering contract and points at the other. Verified both paths leave the file byte-identical on a no-mutation open+exit. Note both call sites fire in a single explicit --system-root boot: initialize_frontier_runtime (main.c:971) runs the internal loader before the hydrate call at main.c:1018, so the no-mutation runs exercise the internal write -- the one with no dirty-clear behind it -- as well as the hydrate write. Repeated boots stay byte-identical to pristine. --- frontier-cli/main.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/frontier-cli/main.c b/frontier-cli/main.c index 43229616e..85f78c514 100644 --- a/frontier-cli/main.c +++ b/frontier-cli/main.c @@ -2027,7 +2027,18 @@ static boolean hydrate_system_root_database(const char* path, boolean read_only) /* Issue #859 -- derive Frontier.pathString from the root just loaded, so it is * correct on boot modes that never run startupScript (--skip-startup, --protocol). - * Must follow the two assignments above; see the function comment. */ + * Must follow the two assignments above; see the function comment. + * + * ORDERING: this write is deliberately placed BEFORE the + * clear_post_hydration_dirty_flags(hroot) call below, which wipes the dirty bit + * it sets. Do not move the write after that clear, and do not move the clear + * above this call: either reorder would leave the root dirty at every boot and + * make save_system_root_on_exit rewrite the whole file on every shutdown (the + * ~1.2 MB drift #127 removed). The compare-before-assign inside the helper keeps + * the common case clean regardless -- an unchanged value is never written -- but + * that is a second line of defence, not a substitute for this ordering. + * The sibling call in load_system_root_database_internal has NO equivalent clear + * after it and relies solely on compare-before-assign; see the note there. */ cli_init_frontier_pathstring(); /* NOTE: Startup scripts are NOT run here during hydration. @@ -2318,7 +2329,15 @@ static boolean load_system_root_database_internal(const char* path, boolean allo g_system_root_loaded = true; /* Issue #859 -- see cli_init_frontier_pathstring(). Both load paths need this: - * this one runs when hydration is skipped (allow_hydrate false). */ + * this one runs when hydration is skipped (allow_hydrate false). + * + * ORDERING, and how it differs from the hydrate path: there is NO + * clear_post_hydration_dirty_flags call after this one, so unlike its sibling + * this write is not incidentally undone -- a dirty bit set here survives toward + * save_system_root_on_exit and would rewrite the whole root at shutdown. The + * compare-before-assign inside the helper is therefore the ONLY thing keeping a + * no-op session clean on this path. Do not weaken or bypass that check, and if a + * dirty-clear is ever added here, keep this call ahead of it. */ cli_init_frontier_pathstring(); /* NOTE: Startup scripts are NOT run here. They are run in main() AFTER From f8a22e85ffa4d59550788b7d85e4fdcfe78162be Mon Sep 17 00:00:00 2001 From: Jake Savin Date: Tue, 11 Aug 2026 00:56:48 -0700 Subject: [PATCH 5/5] fix(cli): #859 -- read the stored pathString raw, include the portable header Bar-raiser delta verdict, two P2s. 1. pullstringvalue instead of hashgetvaluestring in the compare block. hashgetvaluestring is a display-oriented coercion that deparses non-printing characters ("2.1b4 dmb: don't deparse quotes, just non-printing characters", langhash.c). A stored path containing a backslash or control character came back escaped, never compared equal to the raw computed value, and re-dirtied the node on every load -- which on the internal-loader path, where no dirty-clear follows, resurrects per-session full-root rewrites for exactly those paths. pullstringvalue is a raw texthandletostring. It returns void, so the condition is reshaped: the stringvaluetype guard still precedes the read, which is what makes touching data.stringvalue safe. 2. Include Common/headers/file_portable.h instead of declaring portable_folderfrompath locally. The warning about the Mac-only folderfrompath in file.h -- declared but undefined in the headless build, links via -undefined dynamic_lookup, segfaults when called -- moves to the include site and is kept verbatim; it is the reason this file must use the portable_ entry point. langinternal.h added for pullstringvalue. Behavior-preserving. Re-verified after the change: both compare branches still fire (assign when stale, skip when current); no-mutation open+exit byte-identical on BOTH load paths across two boot cycles each, including the explicit --system-root boot that exercises the clear-less internal-loader write; --lock-opened-roots byte-identical; security regression green; 6/6 yaml green. --- frontier-cli/main.c | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/frontier-cli/main.c b/frontier-cli/main.c index 85f78c514..4301cd79c 100644 --- a/frontier-cli/main.c +++ b/frontier-cli/main.c @@ -65,6 +65,13 @@ #include "../Common/headers/shell_api.h" #include "../Common/headers/db.h" #include "../Common/headers/file.h" +/* file_portable.h declares portable_folderfrompath, the primitive that backs the + * file.folderFromPath verb (portable/fileverbs_portable.c:733). Note the Mac-only + * folderfrompath declared in file.h above has NO definition in the headless build: + * the link succeeds via -undefined dynamic_lookup and then segfaults when called, + * so pathString derivation must use the portable_ entry point. */ +#include "../Common/headers/file_portable.h" +#include "../Common/headers/langinternal.h" #include "../Common/headers/db_format.h" #include "../Common/headers/tableverbs.h" #include "../Common/headers/langexternal.h" @@ -181,12 +188,6 @@ const char *cli_get_system_root_basename(void) { return bn_buf; } -/* Forward declaration from file_portable_posix.c -- the primitive that backs the - * file.folderFromPath verb (portable/fileverbs_portable.c:733). The Mac-only - * folderfrompath declared in file.h has NO definition in the headless build; the - * link succeeds via -undefined dynamic_lookup and then segfaults when called. */ -extern boolean portable_folderfrompath(const bigstring bspath, bigstring bsfolder); - /* * cli_init_frontier_pathstring -- set Frontier.pathString from the loaded root. * @@ -347,14 +348,26 @@ static void cli_init_frontier_pathstring(void) { /* Compare before assign -- see the header comment. hashtablelookup matches * case-insensitively, so this finds the cell whichever way it is spelled, and * assigning through the same key preserves the stored node rather than adding - * a sibling. */ + * a sibling. + * + * pullstringvalue, NOT hashgetvaluestring: the latter is a display-oriented + * coercion that deparses non-printing characters ("2.1b4 dmb: don't deparse + * quotes, just non-printing characters", langhash.c). A stored path containing + * a backslash or control character would come back escaped, never compare equal + * to the raw computed value, and re-dirty the node on every load -- which on the + * internal-loader path, where no dirty-clear follows, resurrects per-session + * full-root rewrites for exactly those paths. pullstringvalue is a raw + * texthandletostring. It returns void, so the stringvaluetype guard above is + * what makes reading data.stringvalue safe. */ if (hashtablelookup(frontiertable, BIGSTRING("\012" "pathString"), &val, &hnode) - && val.valuetype == stringvaluetype - && hashgetvaluestring(val, bsexisting) - && equalstrings(bsexisting, bsfolder)) { - log_debug(LOG_COMP_STARTUP, - "Frontier.pathString already current; leaving node clean"); - return; + && val.valuetype == stringvaluetype) { + pullstringvalue(&val, bsexisting); + + if (equalstrings(bsexisting, bsfolder)) { + log_debug(LOG_COMP_STARTUP, + "Frontier.pathString already current; leaving node clean"); + return; + } } if (!langassignstringvalue(frontiertable, BIGSTRING("\012" "pathString"), bsfolder)) {