mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
* fix(skills): anchor gitnexus-plan safe writer on macOS (#2905) The safe generated-plan writer refused to run on anything but Linux. `requireDescriptorAnchoring()` hard-gated `process.platform !== 'linux'` because every name it resolves went through `/proc/self/fd/<fd>/<child>`, and publication went through `renameat2(RENAME_NOREPLACE)`. macOS has neither, so `write-plan` and `read-plan` failed on every input and `snapshot` failed whenever a materialized path was absent. Node cannot perform openat-style directory-relative resolution on macOS at all: `node:fs` exposes no dir_fd parameter, and `fcntl(F_GETPATH)` is a snapshot string that XNU reconstructs from the name cache, so using it would reintroduce the exact race this helper exists to prevent. Python does expose the *at() family via dir_fd, and macOS has renameatx_np with RENAME_EXCL, so the anchoring borrows the interpreter the writer already spawns for renameat2. Anchoring now goes through a backend with two implementations. The Linux one keeps the original expressions, flags, ordering and error strings. The Darwin one runs each operation in the integrity-checked python3: it re-walks the chain from the repository root with O_DIRECTORY|O_NOFOLLOW, asserting the caller's recorded device, inode and mode at every level before acting. A chain that fails that assertion reports a dedicated anchoring errno and never ENOENT, so a moved parent cannot be read as an absent file. Node holds an open descriptor on every chain element for the anchor's lifetime, which pins the inodes so their numbers cannot be recycled between spawns, and that coupling is re-checked on the way into every request rather than left implicit. A filesystem that answers ENOTSUP to RENAME_EXCL is a refusal, never a fallback to a replacing rename. Every other platform is still refused. The suite had silently skipped on every non-Linux runner, so it is now gated on linux-or-darwin and registered in the cross-platform test list, which puts it on the macos-latest CI matrix. Disclosed rather than papered over: operations that must hand Node a file descriptor are anchored in the helper and then opened lexically with O_NOFOLLOW and identity-compared. A racer can force a mismatch, which aborts, or land on the inode the anchored walk already found, which is harmless. A perfect ABA inside that window is impossible on Linux and detected in all but its narrowest form on macOS. The reference doc says so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NtGZN6YSNU8chALSnKYHBp * test(skills): normalize the anchoring-gate fixture repo on Windows The two capability-gate tests are the only ones in this file that run on Windows, and both failed there: `createBaseRepo` returned the path `os.tmpdir()` gave it, which on Windows is the 8.3 short form (C:\Users\RUNNER~1\...). `assertRepository` compares fs.realpathSync of the caller's path against the realpath of `git rev-parse --show-toplevel`, and plain realpathSync does not expand short names while git always reports the long form, so the helper rejected its own fixture with "--repo must be the Git worktree root" before either platform gate was reached. Resolve the fixture with the native resolver, which returns the canonical long path. No-op on platforms where the two already agree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NtGZN6YSNU8chALSnKYHBp * test(skills): skip the darwin backend gate on Windows Spoofing process.platform does not spoof fs.constants. Windows Node defines no O_DIRECTORY, so a darwin-spoofed run there refuses at the anchoring-flag check and returns that message instead of ever reaching the python3-backend branch the test exists to cover. Skip it on win32 rather than loosening the regex, which would also let a macOS run pass on the wrong message. The sibling test still asserts the Windows refusal on Windows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NtGZN6YSNU8chALSnKYHBp * refactor(skills): tighten the macOS anchoring backend Quality pass over the Darwin backend. No behaviour change was intended on the success paths; the guarantees are the same or stronger. Structural: - openChildRead now proves identity inside the backend instead of by comment. It was returning a raw descriptor from a lexical open, with the "callers always compare against the preceding anchored stat" invariant enforced across four call sites in prose — and since the Linux predicate is a literal `return true`, a fifth caller that forgot would have been an unanchored open on macOS that Linux CI could not see. It routes through darwinAdoptAnchoredFile, which already did open-then-compare-then-close-on-mismatch for createChild. - recordAnchoredAbsence shares one prefix walk per snapshot instead of re-walking from the repository root for every absent cited path. With three absent paths under a three-deep prefix that is 12 helper spawns down to 6 and 12 retained descriptors down to 4. citedPaths is caller-supplied and unbounded, so the descriptor retention was the real problem; the cache is now the sole close owner. This does change Linux descriptor lifetime — prefixes stay open for the snapshot rather than only the tail, deduplicated across paths. - assertRepository and the sibling realpath comparisons use realpathSync.native. Windows hands back 8.3 short names that plain realpathSync preserves while git reports the long form, so `snapshot`, which is not platform-gated, could reject a worktree root by quoting that same directory back at the user. The fixture workaround that papered over this for the new gate tests is gone. Efficiency, all measured at ~13.5ms per helper spawn: - consume the identity mkdir already computed rather than re-stat it - act on renameNoReplace's return value rather than spending two stats re-deriving what it already reported - drop a duplicate anchored stat taken twice in a row in movePathToVault - import ctypes only where it is used; 19 of 20 spawns never touch it Simplification: pins folded into the descriptors the handle already carried, an unreachable refreshAnchorTail branch and the dead darwinHardenedOpen mode parameter removed, the four copies of the spawn options collapsed, the spawn-and-parse shared between the probe and the request path, the unreachable launch-path fallback and a redundant memo deleted, and the helper's dispatch made a real elif chain with leaf name and mode validated at one chokepoint rather than per operation. The two chain encodings were left alone deliberately: merging them would have grown triple fields on Linux for no Linux benefit and changed the Linux validatePlanParent comparison. The double re-stamp that motivated the merge is contained in one named helper with the hazard documented. Rejected candidate interpreters now say which dir_fd operations were missing instead of producing a generic refusal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NtGZN6YSNU8chALSnKYHBp * refactor(skills): publish plans with link(2) and drop the interpreter The macOS backend spawned python3 for two jobs: openat-style resolution, which Node cannot do, and a no-replace rename. Only the first is actually unavoidable, and the second was carrying the whole dependency. link(2) is a no-replace publish. It is atomic, it fails EEXIST when the destination name is taken, and it refuses a symlinked destination without following it — the same guarantee renameat2(RENAME_NOREPLACE) and renameatx_np(RENAME_EXCL) give, reachable from plain fs.linkSync. The published file is the same inode as the verified temporary, so the downstream identity checks hold by construction rather than by argument. That removes the interpreter from Linux entirely, since /proc already did the resolving there, and it removes ctypes, libSystem, RENAME_EXCL and the ENOTSUP handling from macOS. Deleted with them: the trusted-executable validation, the held-descriptor exec and its two-tier probe, the capability probe, the JSON request protocol, and both embedded Python programs. The helper drops from 3047 to 2327 lines. macOS keeps the part that genuinely cannot be done in Node, and now does it without a subprocess: a lexical O_NOFOLLOW walk that holds an open descriptor on every directory in the chain and re-proves the chain either side of every step. Pinning is load-bearing — an open descriptor keeps its inode number from being recycled, which is what makes the recorded identities trustworthy across steps. The guarantees are no longer symmetric and the docs say so plainly. /dev/fd/<fd> is a devfs node, not a magic link: opening it works, resolving through it does not, open("/dev/fd/<fd>/child") returns ENOENT and realpath returns /dev/fd/<fd> — measured on macOS 26 rather than inferred. So Linux makes a parent swap impossible while macOS detects one and aborts. Also fixes the writer on 9p mounts, where renameat2(RENAME_NOREPLACE) returns EINVAL and publication failed every time; link(2) succeeds there. Tests 174 -> 154: dropped 29 fixtures that drove the deleted Python program directly, added coverage for the link publish, for a macOS parent swap caught through the pinned chain, and for a spoofed-darwin round trip that asserts no /proc path reaches the hooks, which the portable backend now makes runnable on Linux CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NtGZN6YSNU8chALSnKYHBp * fix(skills): drop O_NOFOLLOW_ANY, guard trailing slashes, handle link edge cases macOS CI rejected our hardened directory open with EINVAL on 30 tests. The flag O_NOFOLLOW_ANY was ORed into every open on the theory that XNU ignores unrecognized open bits, so it would be inert where unsupported. That theory is wrong, at least combined with O_DIRECTORY. The Python design never hit it because the walk ran inside the interpreter; once Node did the opening, every Darwin directory open went through it. Removed rather than probed. The per-component O_NOFOLLOW walk is what delivers the guarantee, and cap-std — the closest reference implementation of this problem — has not adopted O_NOFOLLOW_ANY either. A fixture now pins the exact flags of every directory open under a spoofed darwin, so the next failure names the flag instead of printing a stack trace. With the flag gone the two backends' directory open became identical, so it is no longer a platform concern at all. Three findings from researching the prior art, all now covered: Trailing slashes. CVE-2026-39822 escaped Go's os.Root because open(fd, path, O_NOFOLLOW) follows symlinks when the path ends in "/". It reproduces here: with docs a symlink, opening "docs" is ENOTDIR but "docs/" succeeds into the attacker's directory, and path.join preserves the slash. We were safe only by construction, and only for repo-derived names — the generated temporary and vault artifact names never passed through the validator. The guard now sits at anchoredChild, the single place a name becomes a path, so it holds for every caller. link() can lie on NFS. Per link(2) BUGS, the return code may be wrong if the server creates the link then dies before replying; open(2) NOTES gives the remedy, which is to stat the source and treat a link count of 2 as success. Implemented, with the man-page reasoning in the comment so it is not later removed as paranoia. Filesystems without hard links now fail loudly. EPERM, ENOTSUP and EMLINK say so and refuse to fall back to a replacing rename. Git falls back and accepts losing collision detection because its objects are content addressed; that reasoning does not transfer to a named plan destination. Durability was already correct — the temporary is fsynced before publication and the parent directory immediately after — but the comment now records why the parent fsync is required for link as it was for rename, and the honest limitation that fsync is not a write barrier on macOS while F_FULLFSYNC, which Node cannot reach, is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NtGZN6YSNU8chALSnKYHBp * refactor(skills): shrink the anchoring seam and fix two CI breaks Four quality reviews over the pure-Node writer. Two real breaks, one drift that had already happened, and a seam that was sized for a design we deleted. The macOS round-trip fixture asserted that every observed path started with join(repo, 'docs/plans'). Reproduced on Linux by handing the helper a repo reached through a symlink, which is the shape macOS gives us via /var to /private/var: assertRepository realpaths the repo, so the handle builds paths from the resolved form while the fixture holds the form it passed in, and the prefix can never match. The assertion now proves the same thing without depending on the prefix — a lexical resolution always contains a docs/plans segment and /proc/self/fd/<fd>/<name> never does. Two publish fixtures sat in the capability-gate describe, the one block deliberately not skipped on unsupported platforms, while this PR added the file to the Windows matrix. They test link(2), not the gate, so they moved to SAFE_WRITE_FIXTURES. validatePlanParent restated verifyLexicalChain's loop without the try/catch that converts ENOENT and ENOTDIR into the parity message, so a raw errno could escape a function with a dozen call sites. It was masked on Darwin only because parentStillResolves catches first. It now calls the helpers, which also removes a second full chain walk per call there. openVerifiedFile adds O_NONBLOCK so a FIFO swapped in at the target name cannot wedge the process on open, and only Darwin was calling it. The operations are now shared, so Linux gets it by construction rather than by a per-backend decision. The backend is five methods rather than ten. The platform difference is two things — how a name becomes a path, and what guard wraps an operation — so the five operations became shared functions over a `verified` hook that is run() on Linux and the pinned-plus-lexical sandwich on Darwin. openChildRead always runs the identity adoption, so that proof is structural rather than a comment about what callers must remember. Selecting the backend is a registry that throws on an unknown platform instead of a ternary defaulting to Linux, which surfaced seven dead bindings that ran before the capability gate and made win32 report the registry error instead of the refusal. Snapshot capture no longer re-walks a prefix per record: 36,018 lstats to 6,384 and 162ms to 130ms on 2,000 dirty files across 100 directories, with a byte-identical global_dirty_digest. Absence anchoring is now bounded at 4096 pinned directories and refuses rather than evicting, because closing a cached descriptor would break the pinned chain of a guard already recorded — the inode-recycling hole the pins exist to close. The test suite no longer cache-busts its imports. That existed for the memoized python3 descriptor, the file's only mutable module binding, which is gone; the suite drops from 10.0s to 8.2s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NtGZN6YSNU8chALSnKYHBp --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|---|---|---|
| .. | ||
| bench | ||
| spikes | ||
| assert-publish-grammar-coverage.cjs | ||
| bench-scope-resolution.ts | ||
| build-tree-sitter-grammars.cjs | ||
| build.js | ||
| cross-platform-shard.ts | ||
| cross-platform-tests.ts | ||
| ensure-fts.ts | ||
| install-duckdb-extension.mjs | ||
| run-cross-platform.ts | ||
| shard-arg.ts | ||
| sync-plugin-manifests.mjs | ||