Repository navigation
fix(store): Store 目录 0700、macOS 原生命令透传 Rust/Git 路径、失败只回收本次创建的目录(#570 补全) - #663
yifanxuaaa merged 15 commits into
Conversation
…phemeral-AI-Lab#570 port) Ported from Ephemeral-AI-Lab#570 (8a85fa4 and the store part of 6be2f6c), which was closed as ported into Ephemeral-AI-Lab#647 but never reached layerfs_v2_0_0. The daemon's own startup already refuses an Overlay or Store parent this user does not own privately (`mode & 0o077 != 0`), and `store.create` makes `store/<name>` 0700. The SDK's Store openers had no such rule: the kernel guard checked only the filesystem type, `project.init` guarded the parent of the Store's folder, and `install_local` and backup restore guarded the volume above the folder they then created. Doctor accepted a `0701` Store folder and suggested `chmod g-rwx,o-rw`, a mode no daemon opens. - `kernel_guard`: check the Store's own directory as a real `0700` directory of the current user, refusing a missing path, a symlink, another owner's directory and any group/other bit, and name the path, its mode, the expected `0700` and `chmod 700 <path>`. The check stays side-effect-free: it never creates a directory or changes permissions. Linux and macOS, before the filesystem check. - `store.create`: create `store/` privately, let `make_empty` create `store/<name>` as 0700, then guard the Store file path; a refusal removes the folder only when this call made it, never a user's existing one. - `project.init` and the SQLite diagnostic guard the Store file, so the folder checked is the Store's own. - `install_local` and backup `fetch` create the folder (`0700`) first, then guard the Store file, and remove the folder they made when the guard refuses. - daemon `config::private_directory`: name the path and the failing rule (type, owner uid, or mode with a `chmod 700` hint). - doctor: a Store folder is held to `0o077` with `chmod 700` as its fix, because the daemon refuses any group/other bit there; the home's other folders keep the `0701` pass-through allowance. Tests: `kernel_guard` (a missing directory is refused without being created, `open_direct` leaves no empty directory, `0700` passes for a `.sqlite` and a non-`.sqlite` name, `0755` is refused with path, mode, `0700` and `chmod 700`, and left untouched); `doctor` (a `0701` Store folder is one error naming `chmod 700`, and `0700` is ok while `store/` itself stays `0701`); `store_family` (`store.create` refuses a pre-existing `0755` folder without deleting it or changing its mode, and a new Store's folder is `0700`); `project_family` (`project.init` on a `0755` Store folder refuses naming the path and fix). Fixtures that put a Store in a shared temp directory now make its folder private: `diagnostic_structure`, `store_doctor`, `project_export` and `project_init_worker`. Docs: store-calls service (what the guard checks), store/create (the open order and the `Unsupported` row), project/init (`Unsupported`), daemon startup (the message), doctor (the Store folder row) and the versioned 2.0.0 pages for the `native` machine and doctor. Production LOC: 158272 -> 158376 (delta +104) Scope and method: first-party production Rust/SQL/JavaScript in core, engine, control and api (tests, fixtures, tools, docs, manifests and generated artifacts excluded), counted with `tools/production_loc.py --detail` on the exact parent e9b903e (exported tree) and this tree. engine 73950 -> 74027 (+77: layerfs-store-service 14373 -> 14425, layerfs-daemon 8069 -> 8094); control 38975 -> 39002 (+27: layerfs-store 8567 -> 8573, layerfs-bucket 3788 -> 3797, layerfs-control 5443 -> 5455); core 39787 and api 5560 unchanged. Reference absent (retired R9c; last counted 65,417 at tree 498dd19). Deleted tests: none.
…S commands (Ephemeral-AI-Lab#570 port) Ported from Ephemeral-AI-Lab#570 (cc1edad with the macOS gate of 6be2f6c), which was closed as ported into Ephemeral-AI-Lab#647 but never reached layerfs_v2_0_0. A native macOS Sandbox runs its commands with `HOME` set to the Sandbox's own scratch folder, so a command there saw no toolchain and no Git identity. On macOS, a `HostProcess` command now receives `RUSTUP_HOME`, `CARGO_HOME` and `GIT_CONFIG_GLOBAL` from the caller's environment, or from the caller's real `HOME` (`~/.rustup`, `~/.cargo`, `~/.gitconfig`, else `$XDG_CONFIG_HOME/git/config` or `~/.config/git/config`) when that path exists. They are applied before `request.environment`, so an explicit request value or a command's own override wins, and `HOME` itself stays the Sandbox's scratch folder. Off macOS `user_environment()` answers empty, so the Linux and namespaces backends' command environments are unchanged; the header comment and the `Environment` row in the connect guide say so. Test: `local_search_path::a_native_command_receives_existing_user_toolchain_and_git_locations` covers absent paths, the XDG default, `XDG_CONFIG_HOME/git/config`, `~/.rustup` plus `~/.cargo` plus `~/.gitconfig` (chosen ahead of XDG), caller environment overrides and request environment overrides, with `HOME` remaining the Sandbox's scratch folder. `fixture()` names each run's directory so the two tests in that binary no longer collide, and both take one environment lock. Production LOC: 158376 -> 158412 (delta +36) Scope and method: first-party production Rust/SQL/JavaScript in core, engine, control and api (tests, fixtures, tools, docs, manifests and generated artifacts excluded), counted with `tools/production_loc.py --detail` on the exact parent fad03b5 and this tree. control 39002 -> 39038 (+36, `layerfs-sandbox-backend` 5789 -> 5825); engine 74027, core 39787 and api 5560 unchanged. Reference absent (retired R9c; last counted 65,417 at tree 498dd19). Deleted tests: none.
…remove (Ephemeral-AI-Lab#570 port) `fad03b50c` changed the order backup restore uses and made the kernel guard check the Store's own directory, but two guides still described the old behaviour. - Backup and restore, step 1: `fetch` takes the remote object, then requires the daemons' user (`installer_user`), then creates the destination's directory `0700` when it is absent, then runs the kernel guard on the Store file; a refusal removes only a directory this call made. The flow line and row 1 of the check table now say that, instead of "kernel_guard on the directory; installer_user". - `project.remove`'s `Unsupported` row: the guard refuses the Store's own directory, not only its filesystem, so a missing, symlinked, another owner's or group/other-accessible directory (mode `0700` required) refuses the call too, in the words of `store/create.md` and `project/init.md`. `remove_project` opens the Store through `service::with_service` -> `StoreService::open_direct`, where the guard runs. Checked against the source at `fad03b50c`: `bucket/src/backup/restore.rs` `fetch`, `store-service/src/install.rs` `install_local` (the same order), `store-service/src/kernel_guard.rs` `check`, `store-store/src/project/remove.rs`, `service.rs` `open` and `store_service.rs` `open_direct`. Documentation only: no build, test or measurement was run. Production LOC: 158412 -> 158412 (delta 0) Scope and method: first-party production Rust/SQL/JavaScript in core, engine, control and api (tests, fixtures, tools, docs, manifests and generated artifacts excluded), counted with `tools/production_loc.py --detail` on this tree; its only changes are two Markdown files, so the total is the parent a80592c's. Reference absent (retired R9c; last counted 65,417 at tree 498dd19). Deleted tests: none.
…it (Ephemeral-AI-Lab#570 P1) LP570-REVIEW P1-1: `install_local` and backup restore observed the directory absent before `mkdir` and used that observation as ownership, so a call that lost the creation race recursively removed the directory (and the Store) another call had made while it was away. `store.create` did the same from the observation it made before `make_empty`. - `install::prepare` (new; used by `install_local` and `backup::fetch`): the installer check, the `0700` directory when absent, then the kernel guard. The directory belongs to the call only once its own `mkdir` returned success. An installer-user or `mkdir` failure (including `AlreadyExists`, another caller's directory) deletes nothing, and the guard refusal -- which happens before any Store file exists -- reclaims only that own directory with a non-recursive `remove_dir`, so nothing another caller put there can be taken. `AlreadyExists` stays this call's failure: no continue, no retry. Failures after the guard keep their existing custody. - `backup::fetch` drops its copy of that block and calls the same `prepare`. - `identity::make_empty` hands back `MadeDirectory` (`ThisCall`/`AlreadyThere`) beside its result, and `sqlite::create` reclaims `store/<name>` only for `ThisCall`, instead of the `existed` it saw before `make_empty`. Its own temporary file is still cleaned up; the home lock still serializes `store.create`, so its recursive cleanup of its own directory is unchanged. Tests: new `layerfs-store-service` target `install_directory`. (a) This call observes the directory absent, another caller creates it `0700` with its Store, this call's `mkdir` then fails, and nothing of the other caller's is removed; before the fix this test fails (the directory and its Store were removed). (b) A guard refusal after this call's own `mkdir` reclaims the empty directory it made and leaves both the volume and a directory it did not make, with its content. New `layerfs-store` target `create_directory`: a refused `store.create` reclaims only the empty `store/<name>` its own `mkdir` made and leaves a pre-existing directory with its marker. The race is driven through `install::prepare_between`'s step, never a sleep. Docs: backup-and-restore step 1 and store/create -- the "only a directory this call made" rule now names the guard's non-recursive removal of the still-empty own directory and the mkdir-failure case that removes nothing. Production LOC: 158412 -> 158417 (delta +5) Scope and method: first-party production Rust/SQL/JavaScript in core, engine, control and api (tests, fixtures, tools, docs, manifests and generated artifacts excluded), counted with `tools/production_loc.py --detail` on the parent 64149ba (exported tree) and this tree: engine 74027 -> 74036 (+9, layerfs-store-service 14425 -> 14434), control 39038 -> 39034 (-4, layerfs-bucket 3797 -> 3776 (-21), layerfs-store 8573 -> 8590 (+17)); core 39787 and api 5560 unchanged. Reference absent (retired R9c; last counted 65,417 at tree 498dd19). Deleted tests: none.
…l-AI-Lab#570 P1 review) LP570-P1FIX-REVIEW P2-1: `install::prepare_between(destination, &dyn Fn())` was a `pub fn` whose callback only a regression test passed. `#[doc(hidden)]` hides it from rustdoc, not from callers: the integration test in another crate compiled and called it, so a test-only entry point and its injection point shipped in the product library, which `engine/AGENTS.md` forbids. - `install::prepare` keeps the directory and guard steps inline: the installer-user check, the `0700` directory when absent, then the kernel guard. The directory is this call's own only once its own `mkdir` returned success: an installer-user or `mkdir` failure (including `AlreadyExists`) removes nothing, and a guard refusal reclaims only that own, still-empty directory with a non-recursive `remove_dir`. No callback, hidden name, test feature, `cfg(test)` or included private source. - `installer_user` becomes private: `prepare` is its only caller. - `tests/install_directory.rs` drives the public `prepare` only. The lost-mkdir race cannot be built deterministically without the injection point, so that case is deleted; the fix is supported by source review and by the old-logic run recorded in LP570-P1FIX-REVIEW (exit 101, the other caller's directory removed). The two guard cases stay; `prepare` starts with the installer-user check, so away from the daemons' user (uid 0) they assert that this call refuses before touching the directory or the guard and so creates and removes nothing, and the guard branches themselves run as that user. The same guard and ownership rule run unprivileged in `layerfs-store`'s `create_directory` through `store.create`, which has no installer-user check. - `docs/architecture/api/sdk/store/create.md` (P3-1): the folder is reclaimed for a `make_empty` failure only; a `save_record` failure after `make_empty` succeeded keeps the Store file and the folder, as `sqlite/create.rs` does. Production LOC: 158417 -> 158412 (delta -5) Scope and method: first-party production Rust/SQL/JavaScript in core, engine, control and api (tests, fixtures, tools, docs, manifests and generated artifacts excluded), counted with `tools/production_loc.py --detail` on the parent eb7a34f (exported tree) and this tree: engine 74036 -> 74031 (-5, layerfs-store-service 14434 -> 14429); control 39034, core 39787 and api 5560 unchanged. Reference absent (retired R9c; last counted 65,417 at tree 498dd19). Deleted tests: engine/crates/layerfs-store-service/tests/install_directory.rs `a_lost_creation_race_never_removes_the_other_callers_directory` (built only through the removed `prepare_between` seam; see above).
…phemeral-AI-Lab#570 P1) LP570-P2FIX-REVIEW P3-2: after the creation-race case went away with the removed test seam, nothing drove `install::prepare` or `identity::make_empty` to their own `mkdir` failing with `AlreadyExists`, so the "a `mkdir` failure claims nothing" rule had no deterministic regression. Both cases give the directory name to a dangling symbolic link inside the test's own scratch directory: `Path::exists` looks through the link and reports nothing, so the call reaches its own `mkdir`, which fails on the taken name. The call must then remove neither that name nor anything behind it, and must not create the link's target. - `layerfs-store-service`'s `install_directory`: `a_mkdir_failure_claims_nothing` calls the public `install::prepare`. `prepare` starts with the installer-user check, so as the daemons' user it asserts the `mkdir` failure itself; at any other privilege level it asserts the refusal the other cases in this file assert, and that the call still changes nothing. No root, no callback, no other test seam. - `layerfs-store`'s `create_directory`: `a_mkdir_failure_removes_nothing` calls `sqlite::create::create`, which has no installer-user check, so it drives `make_empty`'s `mkdir` failure unprivileged. Evidence: in a throwaway worktree at 16a0ba3 with the reclaim rule put back to "a directory that was absent when observed goes as well" (`install::prepare` and `sqlite::create::create` as before eb7a34f), `a_mkdir_failure_removes_nothing` fails with exit 101 (the link is removed) while the older tmpfs case still passes; the same reversion with the installer-user check simulated as the daemons' user makes `a_mkdir_failure_claims_nothing` fail with exit 101, and that case passes with the current rule. Production LOC: 158412 -> 158412 (delta 0) Scope and method: first-party production Rust/SQL/JavaScript in core, engine, control and api (tests, fixtures, tools, docs, manifests and generated artifacts excluded), counted with `tools/production_loc.py` on the parent 16a0ba3 (fresh throwaway worktree) and this tree; only the two test files changed. Reference absent (retired R9c; last counted 65,417 at tree 498dd19). Deleted tests: none.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
|
@yhcedpn #570 关闭时只把写入准入挪进了 #647,漏了另外两项:Store 目录 0700 与 doctor 检查、macOS 原生命令透传 RUSTUP_HOME/CARGO_HOME/GIT_CONFIG_GLOBAL(感谢 @ImmuneFOMO 在 #534 指出)。这个 PR 补上这两项,并修了移植时发现的回收问题:失败时只删本次 mkdir 出来的空目录。 已合入最新 layerfs_v2_0_0,Linux 全量 CI 通过,macOS 上 store-service 测试通过。有空麻烦看一下,也欢迎 @ImmuneFOMO 复测。 |
|
Retested 2647fc8 on an M3 Max, macOS 26.6.2, against be317c0, the layerfs_v2_0_0 commit it merged. When Commands in a native Sandbox now get 5 agents at once, 6 runs per build. #663 alone still gets Store busy like its base, which is expected without #647. One agent in the first run got a MountFailure, and its cause is cut off in my log. I didn't see it again in the next 5 runs. Merged with #647 (7d2bfbd) it has no conflicts, and the 0700 and env fixes still work. Commit times, 20 commits per agent:
Not from this PR: a Store restored into a fresh home or under a new name can't start a Sandbox ( Of the tests it changes, doctor, store_doctor, create_directory, diagnostic_structure, kernel_guard and local_search_path pass here. I didn't run the layerfs-api ones, and |
…phemeral-AI-Lab#570 P1) `backup.restore` installed through `home::private_dir`, which keeps an existing folder's mode: a `store/<name>` left at 0755 by an earlier restore was accepted and stayed 0755, so the daemon, `branch.fork` and `sandbox.create` refused the Store the restore had just written. The restore now runs `store.create`'s own kernel guard on an existing `store/<name>` before it stages or writes anything, and refuses with the same `Unsupported` code and `chmod 700` remedy; an absent folder is still made 0700 by the install. Tests: the refusal with nothing staged, the same folder at 0700 and an absent one (backup_refusals); a restore into a pre-made 0700 folder against MinIO (backup_family). No test deleted. Production LOC: 159347 -> 159351 (delta +4) Scope: first-party product implementation under core/engine/control/api crates' src plus shipped runtime SQL; tests, docs, tools and manifests are excluded. Counted with `tools/production_loc.py --root <tree>`, once on the parent tree 2647fc8 and once on this commit's tree; control 39452 -> 39456, api 5613 -> 5613. Migration subtotals: reference absent (retired R9c; last counted 65,417 at tree 498dd19).
…phemeral-AI-Lab#570 P1) `backup.restore`'s folder check runs when the call starts, but the download between it and the install is a long window: a `store/<name>` that appeared meanwhile, or was changed to 0755, was still installed into and stayed 0755, so the daemon, `branch.fork` and `sandbox.create` refused the Store the restore had just written (Ephemeral-AI-Lab#663 review P2-1). The restore's install step is now `backup::restore::install`, which runs the same kernel guard on the target folder again under the home lock, before it links the Store into it or renames it over the old file, and refuses with the same `Unsupported` code and `chmod 700` remedy. What was staged is dropped with the staging folder as before. The first check stays: a folder that is already not private is refused before anything is staged. Tests: `layerfs-store/tests/restore_install.rs` covers the install boundary itself, without a bucket or install permission: a folder made 0755 during the download is refused with nothing installed and its mode kept; a replaced Store's folder is refused before the replacement's own checks; an absent folder is made 0700 and the Store and record are written. The two positive cases in `backup_refusals` now assert a `Failed` from inside the download (the installer-user check as a Linux non-root user, the dead endpoint's own request with install permission) and the staging folder made and removed, instead of "not Unsupported" (review P3-1). No test deleted. Production LOC: 159351 -> 159375 (delta +24) Scope: first-party product implementation under core/engine/control/api crates' src plus shipped runtime SQL; tests, docs, tools and manifests are excluded. Counted with `tools/production_loc.py --root <tree>`, once on the parent tree 7c54540 and once on this commit's staged tree; control 39456 -> 39480, api 5613 -> 5613. Migration subtotals: reference absent (retired R9c; last counted 65,417 at tree 498dd19).
|
Review (static, at 2647fc8). Approve after small fixes. The claim logic is right: a pre-existing directory is never removed, a lost mkdir race fails with no removal or retry, and this also fixes the base To fix
Smaller
Merges cleanly on the current |
Brings the current upstream branch (af2ad92, Ephemeral-AI-Lab#658 and what it merged since be317c0) into the Ephemeral-AI-Lab#570 P1 Store-folder branch; no textual conflict. Production LOC: 159977 -> 160145 (delta +168) Scope and method: first-parent comparison (parent af2ad92, this merge's committed tree). First-party production Rust/SQL/JavaScript in core, engine, control and api; tests, fixtures, tools, docs, manifests/lockfiles and generated artifacts excluded. Counted with tools/production_loc.py --detail on each exported tree: engine 74419 -> 74500 (+81), control 39795 -> 39882 (+87); core 39787 and api 5976 unchanged. Reference absent (retired R9c; last counted 65,417 at tree 498dd19).
Ephemeral-AI-Lab#570 P1 review) Review r663.md, points 1-3 and the smaller ones: - Point 1, the merge with Ephemeral-AI-Lab#647: its `installed::Fixture` made the Store's folder with plain `std::fs::create_dir` (0755), so its new `open_direct` tests reached this branch's guard's `Unsupported` before Busy, corrupt or concurrent (`tests/support/installed_store.rs:48,119`). Both constructors now make that folder `0700` through one `private_directory`. Verified by merging `origin/fix/578-sync-volume` into a throwaway worktree: before, `kernel_guard`'s two new tests failed with `Unsupported(... has mode 0775 ...)`; after, all 5 passed. Whichever PR lands second needs this folder to be `0700`. - Point 2: the restore's install-entry check from 82eb09f is unchanged. `install` checks the target's folder before `private_dir`, `hard_link` and `rename`, under the caller's home lock (`control/crates/layerfs-store/src/backup/restore.rs:166`). - Smaller: the claim step (a `0700` `mkdir`, ownership only when this call's own `mkdir` succeeded) is now one product function, `install::make_private_directory`, used by `prepare` and by `store.create`'s `make_empty`. This removes the second copy of the ownership rule and lets an unprivileged test drive both branches and the failed `mkdir`; the `install_directory` guard cases still need the daemons' user. - Smaller: doctor now reports a folder that is a symlink as a link, and its remedy no longer `chmod`s through the link into the target (the 0777 it reported was the link's own mode). A `store/<name>` link is scanned too, so a Store folder the daemon refuses is reported rather than skipped. - Smaller: docs say the macOS `HostProcess` Rust and Git homes are the caller's own writable ones, so a native command can change the host's toolchain. - Not split: the env passthrough (a80592c) stays in this PR; the review calls the split optional. - Point 3 (the breaking change in the PR description) is in the handoff report, which no call or code change can carry. Production LOC: 160145 -> 160194 (delta +49) Scope and method: first-party production Rust/SQL/JavaScript in core, engine, control and api; tests, fixtures, tools, docs, manifests/lockfiles and generated artifacts excluded. Counted with tools/production_loc.py --detail on the exported parent tree (this commit's parent 3f039a7) and this tree: engine 74500 -> 74506 (+6), control 39882 -> 39925 (+43); core 39787 and api 5976 unchanged. Reference absent (retired R9c; last counted 65,417 at tree 498dd19).
…AI-Lab#570 P1 review R2) When `store_names` began enumerating symlinks under `home/store`, `store()` reported the symlink finding in `store_folder()` but still descended into `record.json` and `project/`. A symlink pointing to a regular file caused `paths::names(&projects)` to fail with ENOTDIR, aborting `scan()` and discarding the entire doctor report. `store_folder()` now returns whether the path is a symlink so `store()` stops descending after reporting it and continues scanning remaining Stores; `store_names()` also sorts entries for deterministic output. Added regression tests for a regular-file symlink and a dangling symlink under `home/store` alongside another Store finding. True first-parent Production LOC disclosure for merge commit 3f039a7 (history preserved without rewriting; 3f039a7's own message erroneously compared against second parent af2ad92, 159977 -> 160145, delta +168): 82eb09f -> 3f039a7: 159375 -> 160145 (delta +770); core 39787 -> 39787 (0), engine 74495 -> 74500 (+5), control 39480 -> 39882 (+402), api 5613 -> 5976 (+363); reference absent (retired R9c; last counted 65,417 at tree 498dd19). Production LOC: 160194 -> 160198 (delta +4) Scope and method: first-parent comparison (parent affb55b, this commit's tree). First-party production Rust/SQL/JavaScript/Python in core, engine, control and api; tests, fixtures, tools, docs, manifests/lockfiles and generated artifacts excluded. Counted with tools/production_loc.py --detail on exported parent tree affb55b and committed tree: control 39925 -> 39929 (+4); core 39787, engine 74506 and api 5976 unchanged. Reference absent (retired R9c; last counted 65,417 at tree 498dd19).
Merged upstream/layerfs_v2_0_0 (de1d77e). Accepted upstream deletion (Ephemeral-AI-Lab#671 architecture docs rebuild) for the 10 modify/delete documentation conflicts; substance from this branch's docs edits is ported separately in the follow-up docs commit. Production LOC: 160198 -> 160198 (delta 0) Scope and method: first-parent comparison (parent 7bbe681, this merge commit's tree). First-party production Rust/SQL/JavaScript/Python in core, engine, control and api; tests, fixtures, tools, docs, manifests/lockfiles and generated artifacts excluded. Counted with tools/production_loc.py --detail: core 39787, engine 74506, control 39929 and api 5976 unchanged. Reference absent (retired R9c; last counted 65,417 at tree 498dd19).
…v notes to rebuilt docs Ported the substantive documentation updates from the 10 architecture pages deleted by upstream Ephemeral-AI-Lab#671 into their rebuilt replacements: - api/sdk/backup.md & control/backups/README.md: backup.restore runs the kernel guard before staging and again under the home lock before install, and install::make_private_directory reclaims only this call's own mkdir - api/sdk/project.md & api/sdk/store.md: kernel guard 0700 directory rule and make_empty own-mkdir reclamation semantics - api/sdk/workspace.md & control/sandboxes/daemon-channel-and-exec.md: macOS HostProcess RUSTUP_HOME/CARGO_HOME/GIT_CONFIG_GLOBAL passthrough and host toolchain mutability note - control/doctor/checks.md: Store folder 0700 rule (0701 refused) and symlink reporting without descending or suggesting chmod - control/stores-and-projects/README.md, engine/daemon/README.md & engine/store-calls/README.md: kernel guard 0700 directory checks and daemon private_directory error format Production LOC: 160198 -> 160198 (delta 0) Scope and method: first-parent comparison (parent f1cdbb4, this commit's tree). First-party production Rust/SQL/JavaScript/Python in core, engine, control and api; tests, fixtures, tools, docs, manifests/lockfiles and generated artifacts excluded. Counted with tools/production_loc.py --detail: core 39787, engine 74506, control 39929 and api 5976 unchanged. Reference absent (retired R9c; last counted 65,417 at tree 498dd19).
|
更新到 aef3281:
验证(Linux,
未跑:durable persistence 测试;macOS 上 |
…rivate Review of Ephemeral-AI-Lab#647, items 1 and 2. Bounded waits (item 1). three_processes_open_and_reserve_concurrently (layerfs-persistence) and three_processes_open_direct_and_commit_concurrently (layerfs-store-service) each collected their children with Child::wait_with_output(), which has no deadline, so a lane deadlock would hang the test instead of failing it. Both now call one wait_bounded helper per test crate: it polls try_wait against a 60 s deadline (the per-invocation ceiling is 120 s), kills and reaps the child and panics with the child's label when the deadline expires, and otherwise reads the pipes of a child that has already exited, so no wait can outlive a live child. Private Store fixture (item 2). installed::Fixture created its Store directory with the process umask (0755 under the CI umask of 022). Ephemeral-AI-Lab#663's kernel guard refuses any Store directory that holds a group or other bit, so once both land the fixture's Store answered Unsupported and the three-process children panicked. Both fixtures now create it with mode 0700 through one private_directory helper: layerfs-store-service's tests/support/installed_store (which layerfs-daemon tests also include by path) covers Fixture::built_with and Fixture::cloned, and layerfs-daemon's own fixture covers Fixture::built. The kernel source directory and the overlay directories keep their previous modes; the guard looks only at the Store file's own directory. Tests: layerfs-persistence writer_admission 13 passed, heads 1, idle_checkpoint 4, serverless_store 5, snapshot 1, concurrent_first_installs 1, sqlite_lock_custody 2, disposable_startup_sync 1; layerfs-store-service kernel_guard 4 (both lane platforms on Linux), commit_service_busy 3, commit_pipeline 3, commit_reclaim 2, write_refusals 3, sdk_method_golden 1 under umask 022; layerfs-api store_open_cache 2; cargo +1.85.1 clippy -D warnings --tests for layerfs-persistence, layerfs-store-service and layerfs-daemon; fmt --check clean for every file this commit touches. Deleted tests: none. Production LOC: 160034 -> 160034 (delta 0) Scope: first-party core/engine/control/API (core 39787, engine 74475, control 39796, api 5976, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py at the merge commit 95d0aef (160034) and at this commit's tree (160034); only external tests change, so the production total is unchanged.
Review item 2, corrected. 523f7fd made the whole installed::Fixture directory 0700 so the Store inside it would satisfy Ephemeral-AI-Lab#663's guard. That directory is also where the daemon harness puts its mount points and where daemon tests run `bash` as another uid: in the mount lane, 24 of 45 binaries then failed with EACCES from `bash_in`'s chdir into the mount, because the parent no longer let that uid traverse. The fixture now keeps everything it owns in `directory` at the process umask (0755 under the CI umask) - kernel source, overlay template, mount points - and puts the sealed and installed Store files in `directory/store` at 0700. The store-service fixture exposes it as `Fixture::store_directory()`, and kernel_guard's corrupt copy moves there: Ephemeral-AI-Lab#663's guard checks the Store file's own parent, which is now the private directory, while every path a test reaches as another uid keeps its old mode. The daemon's own fixture gets the same split as a field. Both three-process cases now own their children as one group instead of waiting on each in turn: wait_bounded becomes Children::wait_all, with one deadline for the whole set (60 s, under the per-invocation ceiling), a Drop that stops any child still running, and a failure that names the child which hung together with what the group wrote to stderr. The barrier handshake is bounded too: the parent's accepted streams and the child's socket get read and write timeouts, so a child that connects and never writes `R` fails the case instead of blocking the accept loop. With this, the remaining review items are in the same tree: api/call-table's Busy open also reaches the 2.0 manual (versioned/2.0.0/calls.md), engine AGENTS.md states that macOS holds the parent-directory lane over short writer jobs and writable opens so only a bypassing writer keeps SQLite's Busy, the coherence box is re-aligned, and the mounter test's comment no longer implies the fallback branch is observed by a test of its own. Tests: layerfs-persistence writer_admission 13, heads 1, idle_checkpoint 4, serverless_store 5, snapshot 1, concurrent_first_installs 1, sqlite_lock_custody 2, disposable_startup_sync 1; layerfs-store-service kernel_guard 4, commit_service_busy 3, commit_pipeline 3, commit_reclaim 2, write_refusals 3, sdk_method_golden 1; layerfs-api store_open_cache 2 - 46 passed, 0 failed. cargo +1.85.1 clippy -D warnings --tests for layerfs-persistence, layerfs-store-service and layerfs-daemon; fmt --check clean for every file here. Mount lane, selected: the 26 binaries that failed at 523f7fd pass 26/26 with this fixture layout (/data/work/ci-logs/pr647-r2-r, 05:44). Deleted tests: none. Production LOC: 160034 -> 160034 (delta 0) Scope: first-party core/engine/control/API (core 39787, engine 74475, control 39796, api 5976, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py at the parent 180ef59 (160034) and at this commit's tree (160034); this commit touches external tests, docs and one test-support module only.
…re-perm-env Conflicts resolved: - control/crates/layerfs-store/src/backup/restore.rs: Ephemeral-AI-Lab#678 plans the project records under the home lock before the install; this branch's extracted `install` (with the folder re-check) runs after that plan, unchanged. The base's inline install block is the same code as `install`. - docs/architecture/control/backups/README.md: steps 3 to 6 combine this branch's folder guard and directory ownership with Ephemeral-AI-Lab#678's record planning and lock order. Production LOC (first parent aef3281): 160198 -> 160463 (delta +265, the base's changes brought in); scope core 39787, engine 74506 -> 74608, control 39929 -> 40092, api 5976; reference absent (retired R9c; last counted 65,417 at tree 498dd19); method tools/production_loc.py. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Review record
Reviewer: Claude Opus 5.5 (independent-context review, then spot-checked) |
…#663, Ephemeral-AI-Lab#642) into fix/578-sync-volume Conflict resolved in engine/crates/layerfs-store-service/tests/support/installed_store.rs: this branch's fixture layout is kept (the Store in a private directory/store, the fixture root left traversable for mounts), and the duplicate private_directory helper Ephemeral-AI-Lab#663 added is dropped. Production LOC (first parent 11b30f0): 160034 -> 160621 (delta +587, the base's changes brought in); scope core 39787, engine 74475 -> 74765, control 39796 -> 40093, api 5976; reference absent (retired R9c; last counted 65,417 at tree 498dd19); method tools/production_loc.py. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…al-AI-Lab#663) Upstream Ephemeral-AI-Lab#663 makes the Store's directory owner-only through one shared helper (make_private_directory + prepare), replacing the open-coded directory creation our wedges had split by cfg. Three conflicts take the upstream helper, which gains the Windows arm (no directory mode; the home's private-inherit default is what 0700 secures there); the daemon installer_user keeps its three-way split (windows refuses with the Ephemeral-AI-Lab#454 pointer, macOS passes, Linux checks the daemon uid), and one stale unix import from the refactor goes. Doctor's scan fuses our Windows mode face with upstream's new symlink check. Production LOC: 161390 -> 161378 (delta -12; tools/production_loc.py --json; the shared helper replaces three open-coded splits).
…ite green again Upstream's new calls.mjs test drives logs.list, which the Windows stub refuses by design; the test now skips with that reason on win32 (and runs everywhere the log exists). The Python module is rebuilt after Ephemeral-AI-Lab#663 touched the packages. Node suite: 5 pass, 1 disclosed skip, 0 fail; in_process_d2 passes after the merge. Production LOC: 161378 -> 161378 (delta 0) - test + rebuilt module.
Clean merge: no code or documentation conflict, so no resolution was needed. The upstream side brings Ephemeral-AI-Lab#678, Ephemeral-AI-Lab#679/Ephemeral-AI-Lab#680, Ephemeral-AI-Lab#663, Ephemeral-AI-Lab#642, Ephemeral-AI-Lab#681 and Ephemeral-AI-Lab#647. Production LOC: 160158 -> 160802 (delta +644) Scope and method: first-parent comparison at 67559fd vs this merge tree via python3 -B tools/production_loc.py over first-party product Rust/SQL/JavaScript only (reference absent, retired R9c).
Tier: P1 (engine/crates/layerfs-store-service, layerfs-daemon; control/crates/layerfs-store) ; risks: risk:store, risk:deps (nix "user" feature), risk:security (macOS env passthrough, by hand) ; gate: yes: risk:store, risk:security ; de-escalated: none
Docs: store/create.md, backup-and-restore.md, doctor.md, control/backups/README.md, api/sdk/backup.md, store-calls/README.md
Durable: NOT_RUN — disabled by owner until explicit reauthorization
Behaviour change (breaking): every SDK Store call (
fs,branch,project, the read-only diagnostic) andproject add,store createandbackup restorenow refuse, withUnsupported, a Store whose folder is not a real0700directory owned by this user. Before, only Sandbox start refused it. Fix an existing Store withchmod 700 <home>/store/<name>.Note on
backup::restore::install: it ispubso thattests/restore_install.rscan drive the post-download re-check without a bucket. The claim below of "no new test-only public interface" does not hold for this function.相关:#570、#534
背景
#570 的部分修复未进入 v2:Store 目录权限与 doctor 检查、macOS 原生命令的环境透传。移植时另发现 install/restore 按「事前不存在」递归回收目录,并发下可能删掉别的调用刚建的目录与 Store。
改动
RUSTUP_HOME、CARGO_HOME、GIT_CONFIG_GLOBAL。install::prepare:目录所有权只来自本次 mkdir 成功;AlreadyExists或 installer 校验失败时不删;guard 拒绝时只用非递归remove_dir。restore 复用同一实现;store create 只在make_empty失败时回收,save_record失败保留。store/create.md、backup-and-restore.md、doctor.md等)。layerfs_v2_0_0(be317c045)。验证
AlreadyExists,断言不删任何东西(engineinstall_directory.rs、controlcreate_directory.rs);旧逻辑下失败。install_directory在 uid 0 与普通用户下均通过。layerfs-store-service173 passed、local_search_path2 passed、clippy-D warnings通过。风险与未覆盖
$HOME之外的用户配置。