Skip to content

fix(store): Store 目录 0700、macOS 原生命令透传 Rust/Git 路径、失败只回收本次创建的目录(#570 补全) - #663

Merged
yifanxuaaa merged 15 commits into
Ephemeral-AI-Lab:layerfs_v2_0_0from
jiangkoumo:fix/570-store-perm-env
Nov 14, 2026
Merged

yifanxuaaa merged 15 commits into
Ephemeral-AI-Lab:layerfs_v2_0_0from
jiangkoumo:fix/570-store-perm-env

Conversation

@jiangkoumo

@jiangkoumo jiangkoumo commented Nov 12, 2026 •

Copy link
Copy Markdown
Collaborator

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) and project add, store create and backup restore now refuse, with Unsupported, a Store whose folder is not a real 0700 directory owned by this user. Before, only Sandbox start refused it. Fix an existing Store with chmod 700 <home>/store/<name>.

Note on backup::restore::install: it is pub so that tests/restore_install.rs can 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。

改动

  • Store 目录按 daemon 的 0700 规则创建;doctor 对组/其他人有权限位的 Store 目录报告。
  • macOS 原生命令透传调用者的 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 等)。
  • 已 merge 最新 layerfs_v2_0_0(be317c045)。

验证

  • 新增确定性回归:用悬空 symlink 造出 AlreadyExists,断言不删任何东西(engine install_directory.rs、control create_directory.rs);旧逻辑下失败。install_directory 在 uid 0 与普通用户下均通过。
  • Linux 全量(照 ci.yml fast + nightly slow/mount,Rust 1.85.1,merge 上游后 HEAD 2647fc8):fast 1887 passed、slow 146 passed、mount 45/45。
  • macOS 27.2(M2):layerfs-store-service 173 passed、local_search_path 2 passed、clippy -D warnings 通过。

风险与未覆盖

  • 已有 0755/0701 的 Store 目录会被 doctor 报出。
  • 透传的路径可能指向 $HOME 之外的用户配置。
  • doctor 只检查权限位,不检查属主。
  • 真并发竞态无动态测试,靠源码审阅;测试覆盖「目录已存在」路径。
  • macOS 上 install/restore 端到端未跑。

…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.
Copilot AI balanced review requested due to automatic review settings November 12, 2026 08:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@jiangkoumo

Copy link
Copy Markdown
Collaborator Author

@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 复测。

@ImmuneFOMO

Copy link
Copy Markdown
Contributor

Retested 2647fc8 on an M3 Max, macOS 26.6.2, against be317c0, the layerfs_v2_0_0 commit it merged.

When store/<name> is already 0755 before store create, the base creates the Store and fails only later at sandbox create with daemon I/O: private directory ownership/mode/type. On #663 store create refuses right away with the chmod 700 hint, and doctor gives the same fix. If the folder turns 0755 after store create, whichever of project init, branch fork or sandbox create runs next refuses with the same hint (the base again fails only at sandbox create). After chmod 700 and running the refused step again, branch fork and sandbox create work in each of these cases.

Commands in a native Sandbox now get RUSTUP_HOME, CARGO_HOME and GIT_CONFIG_GLOBAL. My own GIT_CONFIG_GLOBAL file came through, and when I don't set them, they default to ~/.rustup, ~/.cargo and ~/.gitconfig. On the base all three are empty, git has no user, and rustup finds no toolchain in the Sandbox HOME.

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:

build            agents failed                       commits   p50         max
be317c045        12 of 30, all Busy                  398/600   49-182 ms   3.2 s
2647fc84e        12 of 30, 11 Busy, 1 MountFailure   360/600   52-151 ms   3.3 s
2647fc84e+#647   0 of 30                             600/600   5-10 ms     39 ms

backup create and backup restore through a local S3 (rclone serve s3) work, and a restored Store gets a 0700 folder, as on the base. When store/<name> already exists as 0755, restore goes through and leaves it 0755 (same on the base), and then branch fork and sandbox create on it refuse with the chmod 700 hint. The guard in install::prepare checks the staging folder store/.restore-<random>/, and store/<name> itself only goes through home::private_dir (layerfs-store/src/backup/restore.rs), which keeps the mode of a folder that is already there. store create refuses this case before it writes anything. Should restore refuse it too?

Not from this PR: a Store restored into a fresh home or under a new name can't start a Sandbox (no complete project manifest), on the base too. I didn't find an issue for it, so I opened #668.

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 install_directory is Linux-only

…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).
@yifanxuaaa

Copy link
Copy Markdown
Contributor

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 store.create, which removed store/<name> unconditionally on failure.

To fix

  1. Semantic conflict with macOS:Commit 只刷 export 卷;可写 open 与 seal 走同一把目录 lane;Busy 结果保真 #647. It merges with no text conflict, but macOS:Commit 只刷 export 卷;可写 open 与 seal 走同一把目录 lane;Busy 结果保真 #647 adds open_direct_reports_busy_when_locked_and_failed_when_corrupt and three_processes_open_direct_and_commit_concurrently in layerfs-store-service/tests/kernel_guard.rs. Their installed::Fixture creates its directory with plain std::fs::create_dir (tests/support/installed_store.rs:48,119), which gives 0755. With this PR's guard, both get Unsupported before they reach Busy, corrupt or concurrent. Whichever PR lands second must create the fixture directory as 0700.
  2. Backup restore never guards the final store/<name> (control/crates/layerfs-store/src/backup/restore.rs, (None, None) branch). prepare guards only the 0700 staging folder. home::private_dir(store_dir) keeps an existing 0755 folder's mode and hard-links into it, so restore reports success and the next open is refused. Suggest kernel_guard::check(&destination) after private_dir, before hard_link.
  3. State the breaking change. Stores in a non-0700 folder now fail every SDK call (fs, branch, project), not only Sandbox start. That is consistent with the daemon's rule, but the description should say so. A bare relative Store path makes the guard check ..

Smaller

  • The install_directory tests exercise the claim branches only as root, so unprivileged CI covers them only through make_empty's separate copy.
  • Doctor on a symlinked Store folder reports 0777 (via symlink_metadata) and suggests chmod 700, which changes the target instead.
  • The env passthrough points CARGO_HOME/RUSTUP_HOME at the user's writable toolchain, so cargo install or rustup update in a native command changes the host toolchain. Worth one doc line.
  • Merge commit 2647fc8 has no first-parent Production LOC line. A squash merge avoids it.
  • The env passthrough (a80592c) is independent of the 0700/claim work. Splitting it would be cleaner, but not required.

Merges cleanly on the current layerfs_v2_0_0.

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).
@jiangkoumo

Copy link
Copy Markdown
Collaborator Author

更新到 aef3281:

  • 复审 P2-1:doctor 遇到 store/<name> 是符号链接时只报一条 error(不建议 chmod),不再读 record.json、不再往下遍历,其余 Store 照常扫描;store_names 排序保证输出稳定。补了两个测试:指向普通文件的链接、悬空链接,都确认不中断其他 findings、不改链接目标。
  • 合入最新 layerfs_v2_0_0(de1d77e13)。冲突只在文档:2.0 docs: delete most docs and reorganize by layer (core, engine, control, api) #671 删掉的 10 个旧页已接受删除,本 PR 的说明(Store 目录 0700/属主/非符号链接守卫、backup.restore 双重 kernel guard、doctor 符号链接规则、macOS workspace.exec 透传 RUSTUP_HOME/CARGO_HOME/GIT_CONFIG_GLOBAL)移植到新结构的对应页面。合并与移植均不改产品代码(Production LOC delta 0)。

验证(Linux,cargo +1.85.1 --locked,head aef3281):

  • fast 1912/1912,slow 146/146,mount 二进制 45 过 0 失败
  • PG:persistence、store-service、project、api、cli、control 六包全过
  • layerfs-control --test doctor 17/17

未跑:durable persistence 测试;macOS 上 workspace.exec 环境变量透传的实机复测。

jiangkoumo added a commit to jiangkoumo/layerfs that referenced this pull request Nov 13, 2026
…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.
jiangkoumo added a commit to jiangkoumo/layerfs that referenced this pull request Nov 13, 2026
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>
@yifanxuaaa

Copy link
Copy Markdown
Contributor

Review record

  • Head reviewed: 76c88545c3. That is the author's aef328188c plus my merge of layerfs_v2_0_0 at e8fa0a7e00 (fix(backup): rebuild project records on backup.restore #678), with conflicts resolved in backup/restore.rs and control/backups/README.md (see the commit message).
  • Tier: P1, risks risk:store, risk:deps and risk:security (the last added by hand). Gate: yes. Owner approved the merge, including the macOS RUSTUP_HOME/CARGO_HOME/GIT_CONFIG_GLOBAL passthrough.
  • Read: the whole diff. Directory ownership in install::prepare, make_private_directory and store.create holds: only this call's own mkdir is reclaimed, a guard refusal removes only with a non-recursive remove_dir, and remove_dir_all in create.rs runs only for this call's directory, before the final rename, under the home lock. Also read: the 0700 guard and its callers, and the doctor checks. Tests: no assertion weakened, no test deleted.
  • Re-run on macOS, Rust 1.85.1, at 76c88545c3:
    • tools/check_product_boundary.py: PASS.
    • cargo clippy -p layerfs-store -p layerfs-bucket -p layerfs-store-service -p layerfs-control -p layerfs-sandbox-backend -p layerfs-daemon --all-targets -- -D warnings: clean.
    • cargo test (--no-run first, 100 s limit): restored_records 6/6, restore_install 3/3, create_directory 1/1 and kernel_guard 2/2 pass. install_directory ran 0 tests (Linux-only).
  • Not checked: Linux runs, including install_directory and the layerfs-api families; real concurrent races; macOS install/restore end to end.
  • Follow-ups, not blocking:
    1. The restore re-check runs before home::private_dir, and only when the folder exists. A dangling link fails later as Failed, not Unsupported.
    2. identity::MadeDirectory mirrors install::Made.

Reviewer: Claude Opus 5.5 (independent-context review, then spot-checked)

@yifanxuaaa
yifanxuaaa merged commit 836036d into Ephemeral-AI-Lab:layerfs_v2_0_0 Nov 14, 2026
yifanxuaaa added a commit to jiangkoumo/layerfs that referenced this pull request Nov 14, 2026
…#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>
NelsonLongxiang added a commit to NelsonLongxiang/layerfs that referenced this pull request Nov 14, 2026
…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).
NelsonLongxiang added a commit to NelsonLongxiang/layerfs that referenced this pull request Nov 14, 2026
…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.
jiangkoumo added a commit to jiangkoumo/layerfs that referenced this pull request Nov 14, 2026
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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants