Skip to content

A mount point's ancestors were unreachable whenever / was mounted - #435

Merged
tobert merged 1 commit into
mainfrom
fix/router-synthesizes-mount-ancestors
Sep 2, 2026
Merged

A mount point's ancestors were unreachable whenever / was mounted#435
tobert merged 1 commit into
mainfrom
fix/router-synthesizes-mount-ancestors

Conversation

@tobert

@tobert tobert commented Sep 1, 2026

Copy link
Copy Markdown
Owner

readlink -f failing on a rooted mount was one caller meeting a router bug. ls, stat, cd, and the file tests fail the same way on the same paths:

ls /…/project           -> not found
[[ -d /…/project ]]     -> false

where /…/project/fixture is a mount and another backend covers /.

The router already knew the answer. stat, lstat, list, and path_access each carry a branch that synthesizes a directory for a path above a mount, because a mount at /a/b/c implies /a/b is a directory the same way a real mount implies its mount point's parents exist.

That branch sat on the Err arm of find_mount. Mounting / makes mount_of match every path, so find_mount always succeeds, the backend covering / is asked for /a, and its NotFound reaches the caller before the ancestor branch can run. The code was correct and unreachable in the one configuration every embedder uses — a root mount plus a deeper one.

The check now runs on the answer rather than on the routing. Only NotFound is recovered: any other error is the backend's answer about a path it owns and reaches the caller unchanged, so this cannot mask a permission failure.

A control pins the boundary — a path that is not an ancestor of any mount is still absent, so synthesizing ancestors did not make the namespace exist.

Gates: clippy -D warnings, cargo test --all (6519 passed, 0 failed), insta --check, rustdoc -D warnings.

…nted

`readlink -f` failing on a rooted mount was one caller meeting a router bug.
`ls`, `stat`, `cd` and the file tests fail the same way, on the same paths:

    ls /…/project    -> not found
    [[ -d /…/project ]]  -> false

where `/…/project/fixture` is a mount and something else covers `/`.

The router already knew the answer. `stat`, `lstat`, `list` and `path_access`
each carry an ancestor branch that synthesizes a directory for a path above a
mount, because a mount at `/a/b/c` implies `/a/b` is a directory the same way
a real mount implies its mount point's parents exist.

That branch sat on the `Err` arm of `find_mount`. Mounting `/` makes
`mount_of` match every path, so `find_mount` always succeeds, the backend
covering `/` is asked for `/a`, and its `NotFound` returns to the caller
before the ancestor branch can run. The code was correct and unreachable in
the one configuration every embedder uses -- a root mount plus a deeper one.

The check now runs on the answer rather than on the routing. Only `NotFound`
is recovered: any other error is the backend's answer about a path it owns
and reaches the caller unchanged, so this cannot mask a permission failure.

This supersedes the ancestor half of readlink's own fix. Against this commit
alone, 7 of that branch's 8 tests pass with none of its `is_structural` /
`owning_mount` code present; the 8th is containment, which is a separate
guarantee and stays there.

A control pins the boundary: a path that is not an ancestor of any mount is
still absent, so synthesizing ancestors did not make the namespace exist.
@tobert
tobert force-pushed the fix/router-synthesizes-mount-ancestors branch from 281220f to 8c1f7a0 Compare September 2, 2026 12:38
@tobert
tobert merged commit d35e0bd into main Sep 2, 2026
3 checks passed
@tobert tobert mentioned this pull request Sep 2, 2026
tobert added a commit that referenced this pull request Sep 2, 2026
Version bump and changelog stamp for v0.17.1, a patch release covering
six PRs merged since v0.17.0: help/kaish-tools nested-subcommand
recursion (#430), the wrapped-command allow_external_commands framing
correction (#431), a changelog correction plus a new zero-padded
date/time migration note (#432), nested verb groups for wrapped commands
(#433), mount-point ancestor navigation when a backend also covers `/`
(#435), and VfsRouter-shared path canonicalization closing a containment
leak in `readlink -f`/`realpath` (#434).

This bump also carries two documentation fixes surfaced by the
release-gate review below rather than opening a separate PR for
text-only changes: `docs/EMBEDDING.md` claimed `realpath` passes
`allow_missing_final: true` and rechecks existence, when it actually
passes `false` directly; and the canonicalize changelog entry overstated
the default implementation as containment-checked, when containment is a
property of `LocalFs`'s and `VfsRouter`'s overrides, not the shared
default.

Reviewed with kaibo (`consult`, cast `deepseek`) against the full
`v0.17.0..HEAD` diff. Verdict: no undocumented semver breaks — the two
new `canonicalize` trait methods are defaulted and every changed public
type is either `#[non_exhaustive]` or privately fielded, so the patch
framing holds. Two smaller findings from that review are real but scoped
as code changes rather than release-blocking text, so they're queued as
follow-up work rather than folded into this bump: a wrapped-command node
can silently accept a no-op `json_output` declaration instead of being
refused, and the new `canonicalize` default's symlink-hop cap has thin
test coverage.

Gates: `cargo test --all` (2231 passed), `cargo clippy --all
--all-targets -- -D warnings` (clean), `cargo insta test --check` (no
pending snapshots).
tobert added a commit to tobert/kaibo that referenced this pull request Sep 2, 2026
0.17.1 carries the fix that has been blocking kaibo v0.4.0: `readlink
-f` and
`realpath` failed on **every** operand on a rooted mount, with a message
naming
neither the operand nor its target (`readlink: o-exists: No such file or
directory:
/tmp`). Both resolve now, and an escape refuses by name. We reported
that shape
upstream during the 0.17.0 bump; it became two structural kaish PRs
rather than a
patch — [#435](tobert/kaish#435) (the router's
mount-ancestor
synthesis was unreachable whenever `/` is mounted) and
[#434](tobert/kaish#434)
(`Filesystem::canonicalize` as a
defaulted trait method, moving containment out of the builtin into the
VFS).

## The compiler found nothing, so the shell was the check

No API break reached kaibo — `Filesystem::canonicalize` is defaulted and
kaibo
implements no `Filesystem` or `KernelBackend`. The composed tool
contract is
byte-identical under both pins (throwaway crate calling
`compose(&Recipe::tool_description(), …)`), and so are all eleven `help`
surfaces
kaibo renders, 43941 bytes each side.

So the check was the shell, per the rule the 0.17.0 bump wrote into
AGENTS.md. Every
battery in `docs/sandbox-probes.md` was run against **both** a 0.17.0
and a 0.17.1
binary and diffed. A, B, D, E, F and G came back identical. Two changes
are the whole
delta a model can see.

**1. The release blocker is fixed.** `readlink -f` / `realpath` resolve
an in-tree
path (exit 0) and refuse an escape by name. Battery G3 re-run on the new
canonicalize
path: existing, missing, and unreadable targets still refuse
byte-identically, so the
new code introduces no existence oracle.

**2. The directories above the mount list again — new, and accepted.**
0.17.0 answered
"not found" for every directory above the project; 0.17.1 restores them
as synthesized
directories. It is synthesis from kaibo's own mount table, not a host
read: each level
names only the next component down to the project, so a model walking up
recovers the
root path string the caller already handed it and nothing else. Counted
rather than
argued — `ls /tmp` returns one entry where the host `/tmp` holds 3575.
Adjacent
secrets, real siblings, and the state db and media CAS all stay
invisible (E2/F2
re-run against a non-empty store: 4 KiB db, 210 CAS objects, both `not
found`).

That observable is pinned by a new containment test with a **recorded
positive
control** — move the mount up and the leak assertion fires — because a
battery where
everything comes back empty proves nothing otherwise. Battery C's claim
that `/home`
is "an inert stub that cannot be walked" is now false and is corrected
in place, the
same way the 0.17.0 bump corrected three other criteria.

## The probe caught itself once

E1 run without `--root` created a state db, because the fixture was then
outside every
allowed tree and the guard correctly did not fire. Re-run with the root
pointing at the
fixture, both E1 and F1 refuse loudly and create nothing. The §0
question — *would this
read differently if the probe were broken, versus if the thing it audits
were broken?*
— is what found it.

## Gates

- `cargo clippy --all-targets`: clean.
- `cargo test`: 1147 passed. The lone failure is the known
`tests/credentials.rs`
ETXTBSY exec race under parallelism (green serially, reproduces on
unmodified code).
- containment: 25 passed, one new.
- `cargo tree -i` empty for `aws-lc-rs`, `mimalloc`, `openssl-sys`.

Cross-family review posted as a comment below.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
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