Repository navigation
fix(cli): #859 -- compute Frontier.pathString in pure C at both root-load paths - #886
Merged
Merged
Conversation
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).
…uence 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.
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.
…ng 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.
…e 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.
Owner
Author
Gate Review: fix-859-pathstringVerdict: PASS (after one consolidated fix round + delta re-review)Scope: 5 commits, final f8a22e8 on develop 8660748 — frontier-cli/main.c (+229), tests/integration/test_cases/frontier_pathstring_init.yaml (new, 6 tests), tests/integration/cli_pathstring_no_eval_test.sh (new security regression test), run_integration_tests.sh wiring (+17). Fixes the live half of #859. The bug and the fixFrontier.pathString (folder of the loaded ROOT database — legacy semantics proven from the original source: Fix: The gate story (this unit is why gates exist)The first implementation evaluated a constant UserTalk snippet at load. Three reviewers found three different real defects in it:
The pure-C rewrite retired all three at once. Delta re-reviews: security CLEARED (closure is structural — no eval entry points exist on the path; new-C memory safety verified line-by-line); bar-raiser APPROVE (caught one landed trap — the deparsing extractor — fixed in the final micro-round: pullstringvalue swap). Implementation traps found and fixed along the way (each would have shipped broken): the Frontier table is NOT at root level (bare-name lookup silently missed); Mac-only Tests
Persistence semantics (documented, measured)The stored pathString cell is DEAD BY CONSTRUCTION post-fix (unconditionally recomputed at every load — proven by a move test). Compare-before-assign keeps no-op sessions byte-clean; a RW session that mutates-and-saves for other reasons persists the local path (remanence only, #854 domain). Ordering comments at both write sites document the dirty-flag-clear asymmetry (concurrency carry-forward). Pollution walkers should treat Frontier.pathstring as dead-by-construction. Follow-ups filed#884 (linker landmine), fldisableyield-not-honored-headless + bn_buf shared static buffer (concurrency), pathString-as-transient/scrub-on-export, relative-root realpath normalization (both folded conceptually into the root-build-from-source plan). Stats
|
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Frontier.pathString (the folder of the loaded ROOT database, legacy semantics) shipped stale in Virgin.root and remained LIVE under --skip-startup across the entire integration-test and protocol-tooling surface. Only startupScript recomputed it, so getSubFolder operations threw errors and created shadow folder hierarchies at dead paths. This PR fixes the live half of #859 by computing pathString in pure C at both root-load entry points.
The Bug
The Fix
Added
cli_init_frontier_pathstring()in pure C, called from BOTH sequential root-load paths:Gate Review
Verdict: PASS (after one consolidated fix round + delta re-review)
The first implementation evaluated a constant UserTalk snippet at load. Three reviewers surfaced three real defects:
Test Results
Files Changed
Related Issues
Fixes: #859 (live half; byte-remanence half stays with #854)
Unblocks: #855 (with #881 in flight for script repairs)
Follow-ups filed: #884 (linker landmine: Mac folderfrompath), fldisableyield-not-honored-headless, pathString-as-transient, relative-root realpath normalization
https://claude.ai/code/session_01SEFFEgkaQJayd48Y2iuNkx