Skip to content

fix(runtime): keep private state on a filesystem that cannot express ownership - #7168

Open
gaord wants to merge 7 commits into
codewhale-hq:mainfrom
gaord:feat/hmdfs-ownerless-mount
Open

gaord wants to merge 7 commits into
codewhale-hq:mainfrom
gaord:feat/hmdfs-ownerless-mount

Conversation

@gaord

@gaord gaord commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Why

A GUI start on HarmonyOS hmdfs (the 2in1 user directory, /storage/Users/currentUser) failed one feature at a time:

Engine exited before becoming ready: error: workspace drive lease check failed for
/storage/Users/currentUser/magic/CodeWhale-VSCode: Codewhale credentials directory
must be owned by the current user

then error: failed to build daemon state, then Codewhale credentials directory must be owned by the current user again. Each fix revealed the next, because one mechanism — admit a directory by proving it is current-user-owned and closed to group and other — sits under the workspace lease, the owner control socket, the OAuth credential directory, plugin state, tool report files, pet-watch state and delivery files.

That mount cannot satisfy the proof. Measured on a HarmonyOS PC (HPR-W72, API 26):

operation result
touch ~/.codewhale/x mode=0640, uid=20001006
chmod 600 ~/.codewhale/x Permission denied (EPERM)
mkdir -m 700 ~/.codewhale/d mode=2771, uid=20001006
chmod 777 ~/.codewhale/d Permission denied (EPERM)

Every object is reported as belonging to the logged-in user whatever uid created it, so the calling process (uid 20020225 for CodeArts, 20020084 for HiShell) is never the reported owner — and chmod on someone else's file is refused for that reason, not because the mount lacks mode support.

What this does

The mechanism names the condition once. private_footing creates one 0o700 directory under the target, reads back what the mount made of it, and — when that is not the private owner and mode it asked for — asks the mount to narrow it and looks again. Only a mount that refuses, or keeps its own values after being asked, is synthesising. A filesystem that applies an unexpected mode at creation but honours the correction stores modes and keeps the full guard, so a default ACL or a setgid parent cannot silently relax a check. The probe runs only after a strict check has already failed, and its answer with its one warning is cached per directory, so a write path never probes a healthy filesystem.

Where the mount is synthesising, the private object is admitted with the ownership guard recorded as unverifiable. The no-follow component walk, the single-link check, atomic publication and the retained handle still apply.

Two capabilities are kept apart, because conflating them is what made the original failure look like a missing feature:

  • Exclusivity is the kernel lock, not the ownership proof. The workspace lease is still taken and a second driver is still refused with the holder's pid, client, store and age. Only a filesystem that refuses advisory locks themselves (EPERM, EACCES, ENOTSUP, EINVAL, ENOSYS — never EWOULDBLOCK) is reported as unable to provide exclusivity, once per admitted workspace scope.
  • The owner control socket has no other discovery path, so it degrades to HTTP only and logs what was lost. A daemon-only run and ACP publication still require it and name the reason.

Narrowing after a write goes through one helper that attempts the chmod, keeps the error where the filesystem stores modes, and excuses only a mount that cannot apply them — applied to the handle that was opened where one exists, so a substituted path cannot be the object narrowed. Checks that re-proved ownership on an object already admitted (the retained-directory identity comparison, an owner receipt's mode, a published socket's owner) now follow the guard it was admitted with.

A directory this process may not enter keeps its original error everywhere, so a filesystem that stores mode bits never takes the wider path.

Scope

Two commits: the secret store's own mode/lock gate (self-contained, crates/secrets + crates/memory), then the runtime mechanism and its consumers. Three device/probe scripts are included; documentation and the cross-build PATH fix belong to #7151 and are not duplicated here. No CHANGELOG entry, per the maintainer's request.

crates/tui/src/workspace_lease.rs and runtime_log.rs moved to crates/runtime upstream (#7149, #7151-era refactors); the changes are replayed there rather than in tui. Because the module is now public API, LeaseAttempt is widened to pub to match WorkspaceDriveLease::try_acquire.

Verification

Branch is rebased on main at acfd66000 (0 behind).

  • check --workspace --all-targets --locked clean; fmt --all --check clean.
  • Clippy clean with CI's flags (-A clippy::uninlined_format_args -A clippy::too_many_arguments -A clippy::unnecessary_map_or) on config, secrets, memory, util, app-server, plugins, runtime and tui.
  • Suites: config 813, secrets 55, util 51, plugins 201, app-server 177 + 15 integration, memory 4, runtime 928, cli 378.
  • tui lib, full parallel run: 15 failed / 13403 passed, then 17 failed / 13401 passed on a re-run — the set varies run to run.

The tui failures are pre-existing, not from this change

Measured against a clean origin/main (acfd66000) worktree, same suite, same machine:

tree failures
origin/main 13
this branch, run 1 15
this branch, run 2 17

Every name that differs between the sets passes individually on its own tree:

  • core::engine::tests::recursive_rlm_round_waits_on_the_code_execution_gate — passes alone here; also fails the core::engine::tests:: group on clean origin/main (8 failures there, same 8 names).
  • runtime_threads::tests::preturn_control_status_does_not_make_empty_turn_succeed — passes alone here; also fails the runtime_threads::tests:: group on clean origin/main.
  • tools::subagent::tests::worker_compacts_past_its_context_window_and_keeps_working, runtime_api::tests::reload_config_with_malformed_file_returns_error, runtime_threads::tests::profile_constitution_... — appear in one run and not the other, on both trees.
  • tools::subagent::tests::late_launch_permit_still_gets_the_full_work_budget fails on clean origin/main and passes here.

Not covered

  • Windows: bind_captured_owner gained an Option return and the non-unix tighten_mode* stubs are by inspection only — no Windows toolchain on this machine; CI covers it.
  • The synthesising-mount and no-advisory-lock branches cannot be produced on a portable filesystem. They are exercised on a real device by scripts/ohos-secrets-store-smoke.sh and predicted by scripts/ohos-lease-footing-probe.sh; scripts/ohos-sign-binary.sh cross-builds, strips, self-signs and verifies the Engine, which starts on the device above.

Refs #7151

@gaord
gaord requested a review from Hmbown as a code owner October 31, 2026 05:11

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

gaord added 2 commits October 31, 2026 13:12
A file-backed secret store could not be written on HarmonyOS hmdfs, the 2in1
user directory. Every object there is reported as belonging to the logged-in
user whatever uid created it, so a process is never the owner of what it
creates: `chmod` answers EPERM, and the base mode is `0660` for every file.
Three checks assumed otherwise — the `0600` gate on `secrets.json` and its
`.lock`, the narrowing before the atomic rename, and the cross-process write
lock itself.

The mode gate now decides from evidence rather than from the mode alone: it
creates one probe file at `0600` in the store's directory, asks the mount to
narrow it, and reads the mode back. Only a mount that refuses, or that keeps
its own mode after being asked, is treated as one where the mode says nothing
about who may read the file; such a mount is accepted with a warning, because
the alternative is a plaintext key in `config.toml` — a wider exposure. A
filesystem that stores mode bits keeps the strict refusal, so a store left
readable by group or other is still refused rather than silently repaired:
those bytes may no longer be ours, and re-narrowing the file would hide the
question. The `.lock` sibling holds no secret and is repaired in place instead.

When the filesystem reports no advisory locks (EPERM, EACCES, ENOTSUP, EINVAL,
ENOSYS), secret-store writes are serialised within the process and the
fallback is logged. A lock that is genuinely held (EWOULDBLOCK) is unaffected.
Two Codewhale processes whose snapshots overlap can lose one write on such a
mount, which needs both to rewrite the store in the same instant.

The memory database's mode check takes the same evidence-based rule, so a
default ACL on a mode-storing filesystem no longer counts as a mount that
cannot hold a permission.
… ownership

A GUI start on HarmonyOS hmdfs failed one feature at a time: the workspace drive
lease, then the owner control socket as `failed to build daemon state`, then the
OAuth credential directory as `Codewhale credentials directory must be owned by
the current user`. Each of those admitted a directory by proving it was
current-user-owned and closed to group and other, and on that mount no directory
can: it reports the logged-in user's uid and a group-writable mode for every
object, whatever uid created it, so the calling process is never the reported
owner and `chmod` answers EPERM. Each fix revealed the next, because the same
mechanism sits under every private directory Codewhale keeps.

The mechanism now names the condition once. `private_footing` creates one `0o700`
directory under the target, reads back what the mount made of it, and — when that
is not the private owner and mode it asked for — asks the mount to narrow it and
looks again. Only a mount that refuses, or that keeps its own values after being
asked, is synthesising; a filesystem that applies an unexpected mode at creation
but honours the correction stores modes and keeps the full guard, so a default
ACL cannot silently relax a check. The probe runs only after a strict check has
already failed, and its answer (with its one warning) is remembered per
directory, so no write path probes a healthy filesystem.

Where the mount is synthesising, the private object is admitted with the
ownership guard recorded as unverifiable. The no-follow component walk, the
single-link check, atomic publication and the retained handle all still apply,
and the mount's own access control governs the rest.

Two capabilities are kept apart, because conflating them is what made the
original failure look like a missing feature. Exclusivity is the kernel lock, not
the ownership proof: the workspace lease is still taken, and a second driver is
still refused with the holder's pid, client, store and age. Only a filesystem
that refuses advisory locks themselves (`EPERM`, `EACCES`, `ENOTSUP`, `EINVAL`,
`ENOSYS`, and never `EWOULDBLOCK`) is reported as unable to provide exclusivity,
once per admitted workspace scope. The owner control socket has nothing else to
fall back on for discovery, so it degrades to HTTP only and says so; a
daemon-only run and ACP publication still require it and name the reason.

Narrowing a file or directory after writing it goes through one helper that
attempts the `chmod`, keeps the error on a filesystem that stores modes, and
excuses only a mount that cannot apply them — applied to the handle that was
opened where one exists, so a substituted path cannot be the object narrowed.
Checks that repeated the ownership proof on an object already admitted (the
retained-directory identity comparison, an owner receipt's mode, a published
socket's owner) now follow the guard it was admitted with.

`scripts/ohos-sign-binary.sh` cross-builds, strips and self-signs the Engine and
verifies the signature before handing the file back;
`scripts/ohos-lease-footing-probe.sh` reports what a directory on a device does
with ownership, mode and advisory locks; `scripts/ohos-secrets-store-smoke.sh`
exercises the secret store's degraded path against a real mount.
@gaord
gaord force-pushed the feat/hmdfs-ownerless-mount branch from b8943ce to 9578367 Compare October 31, 2026 05:45
@gaord
gaord marked this pull request as draft October 31, 2026 05:51
@gaord

gaord commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor Author

along with pr #7151, codewhale will run smoothly in HarmonyOS, natively. That's really beautiful. Failed code in main takes hours to find out where breaks, rather than in the pr, though. It would be nice to keep main clean, anyway.

with the pr, 'codewhale auth set --provider deepseek' and other scenario stuck with file ownership, now work in HarmonyOS

@gaord
gaord marked this pull request as ready for review October 31, 2026 07:41
CodeWhale Bot added 5 commits October 31, 2026 01:01
Two build breaks in the previous commit, both invisible on macOS.

`StoreFile` and `advisory_locking_unavailable` were `#[cfg(unix)]`, but
`open_private` takes the role on every target and the write path calls the
predicate everywhere, so `codewhale-secrets` did not compile for Windows:

    error[E0433]: cannot find `StoreFile` in `file_lock`
      --> crates/secrets/src/lib.rs:627:73
    error[E0425]: cannot find type `StoreFile` in this scope
      --> crates/secrets/src/file_lock.rs:10:11
    error[E0433]: cannot find type `StoreFile` in this scope
      --> crates/secrets/src/file_lock.rs:242:72
    error[E0425]: cannot find function `advisory_locking_unavailable`
      --> crates/secrets/src/file_lock.rs:252:23

The role is a plain enum and is meaningful wherever it is passed, so it is no
longer gated. Windows takes real `LockFileEx` locks through `fd-lock`, so a
failed write lock there is genuine contention and the predicate answers false
rather than falling back to the in-process guard. `require_private_mode` stays
Unix-only, with a `let _ = role` on the other targets where nothing reads it.

`libc::stat.st_mode` is `mode_t`, which is `u16` on Apple and `u32` on Linux, so
`as u32` was a widening cast on macOS and a no-op on Linux that clippy rejects
under the workspace's `-D warnings`:

    error: casting to the same type is unnecessary (`u32` -> `u32`)
      --> crates/config/src/private_directory.rs

`u32::from` is total for both widths, and for the 32-bit Android target that is
`u16` too.

Verified on refs/pull/7168/head 9578367 with both changes applied:
`cargo check -p codewhale-secrets --lib --target x86_64-pc-windows-msvc` fails
with the four errors above without them and passes with them;
`cargo clippy -p codewhale-secrets -p codewhale-config --lib -- -D warnings`
passes on the host; `cargo test -p codewhale-secrets` 55 passed, 1 ignored, 1
passed; `cargo fmt --all --check` clean. The Linux clippy run itself was not
executed here — cross-compiling this workspace needs a Linux `libdbus-sys`
build script — so the cast fix rests on `mode_t`'s declared width in libc
0.2.191 (`bsd/apple: u16`, `linux_like/linux: u32`) rather than a Linux run.

Signed-off-by: CodeWhale Bot <bot@codewhale.net>
The HarmonyOS change added Windows arms but left the Unix originals
ungated, so `codewhale-config` did not build for Windows, and one call site
still passed the pre-change arity:

    error[E0592]: duplicate definitions with name `open_trusting_mount`
      --> crates/config/src/private_directory.rs:95:5
    error[E0592]: duplicate definitions with name `admit_trusting_mount`
    error[E0592]: duplicate definitions with name `inspect_trusting_mount`
    error[E0599]: no associated function `with_mount_tolerance`
    error[E0061]: this function takes 2 arguments but 3 were supplied
      --> crates/config/src/xai_credentials.rs:257:13

`open_trusting_mount`, `admit_trusting_mount` and `inspect_trusting_mount` now
carry `#[cfg(unix)]` on the bodies that call `with_mount_tolerance`, leaving the
existing `#[cfg(not(unix))]` arms as the only definitions there.
`validate_owned_file_handle` keeps its three-argument shape on every platform:
Windows and the remaining fallback take the guard and ignore it, because the
Windows ownership guard is the security descriptor rather than the mount
footing. The Windows-only callers that never had a guard to pass use a
two-argument internal helper instead.

Also replaces `u32::from(metadata.st_mode)` with a cast behind a scoped
`#[allow(clippy::unnecessary_cast)]`: `mode_t` is `u16` on Apple and `u32` on
Linux, so `as` is the widening there and a no-op here, and no single conversion
is lint-clean on both — `u32::from` failed Linux with
`clippy::useless_conversion` after `as` had failed it with
`clippy::unnecessary_cast`.

Verified on 36c038a plus this change: `cargo clippy -p codewhale-config
--lib -- -D warnings` clean on the host, `cargo check -p codewhale-secrets
--target x86_64-pc-windows-msvc` passing from the previous commit, `cargo test
-p codewhale-secrets` 55 passed / 1 ignored / 1 passed, `cargo fmt --all
--check` clean. Not verified locally: `codewhale-config` for Windows, whose
dependency graph reaches `ring`, and `ring`'s build script needs a Windows
toolchain this machine does not have. The Windows compile is left to CI.

Signed-off-by: CodeWhale Bot <bot@codewhale.net>
Two call sites in the Windows owner-only test still used the old two-argument
`validate_owned_file_handle`, which now takes the private guard on every
platform:

    error[E0061]: this function takes 3 arguments but 2 arguments were supplied
       --> crates\config\src\xai_credentials.rs:930:9
       --> crates\config\src\xai_credentials.rs:941:9

Both pass `store.private.guard()`, the same value the production path at
`xai_credentials.rs:257` already passes, so on Windows the guard is
`PrivateGuard::Required` and the ownership proof is unchanged.

Verified: `cargo clippy -p codewhale-config --lib -- -D warnings` clean on the
host with the CI allow-list; `cargo fmt --all --check` clean. Not verified
locally: the Windows compile itself, which needs a Windows toolchain for
`ring`; CI's `Test (windows-latest)` is the check.

Signed-off-by: CodeWhale Bot <bot@codewhale.net>
`only_a_held_lock_is_contention_and_only_a_lockless_mount_is_unsupported`
asserts Unix errno semantics through `libc`, which this crate takes only under
`[target.'cfg(unix)'.dependencies]`:

    error[E0433]: cannot find module or crate `libc` in this scope
    error[E0425]: cannot find function `advisory_locking_unavailable`
       --> crates\runtime\src\workspace_lease.rs:733:36

The predicate it calls is `#[cfg(unix)]` in `codewhale-util`, and production only
reaches it inside a `#[cfg(unix)] if` because Windows takes real `LockFileEx`
locks through `fd-lock`, so a failed lock there is contention rather than a
mount without locks. The test is Unix-only for the same reason, so it now says
so. Nothing about the Unix behaviour changes.

Verified: `cargo check -p codewhale-runtime -p codewhale-util` clean;
`cargo fmt --all --check` clean. Not verified locally: the Windows test build,
which needs a Windows toolchain for `ring`; CI's `Test (windows-latest)` is the
check.

Signed-off-by: CodeWhale Bot <bot@codewhale.net>
Gating the lease predicate test to Unix left `LeaseUnavailability::new` with no
caller on Windows, and the crate denies dead code:

    error: associated function `new` is never used
       --> crates\runtime\src\workspace_lease.rs:148:8

Its only caller builds `LeaseAttempt::Unsupported` on the "this mount has no
advisory locks" branch, which is `#[cfg(unix)]` because Windows takes real
`LockFileEx` locks. The constructor now carries the same gate. The type and its
`Display` stay available on every target, so callers that only match on an
`Unsupported` lease are unaffected.

Verified: `cargo clippy -p codewhale-runtime --lib -- -D warnings` clean with the
CI allow-list; `cargo fmt --all --check` clean. Not verified locally: the Windows
compile, which needs a Windows toolchain for `ring`; CI's
`Test (windows-latest)` is the check.

Signed-off-by: CodeWhale Bot <bot@codewhale.net>

This branch has not been deployed

No deployments
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.

1 participant