Skip to content

refactor(cli): open CLI trees confined, creating them when absent - #570

Merged
David-Noble-at-work merged 1 commit into
developfrom
confine-cli-trees
Aug 20, 2026
Merged

David-Noble-at-work merged 1 commit into
developfrom
confine-cli-trees

Conversation

@David-Noble-at-work

Copy link
Copy Markdown
Contributor

#568 step 2. Step 1's trace found no caller reading or writing outside its own tree, so the only thing keeping
CLI roots unconfined was bootstrapping — the one site that recorded a reason said so:

"the prefix need not exist yet — a first install creates it — and a confined root requires its anchor to
exist."

cli.OpenTree

Creates the directory when absent, then opens a confined root at it. Named for the creation, so
fsroot.OpenConfined stays a query and a missing root stays an error.

  • Creates through a root, not os.MkdirAll — a mode is worth nothing on Windows without the DACL fsroot
    applies for it. A 0o700 directory from os.MkdirAll carries an inherited DACL and is readable by every
    account with access to its parent.
  • Anchored at the volume, so every component is kernel-resolved and no window opens between creating the
    path and opening it.
  • Refuses a path that isn't absolute on this platform. On Windows that includes a leading separator with no
    volume — drive-relative, and Unix-literal "/" roots panic on Windows: four test fixtures, and writ's System layer TargetRoot #392's territory, not something to guess at here.
  • 0o700, per the XDG specification: "If, when attempting to write a file, the destination directory is
    non-existent an attempt should be made to create it with permission 0700."
    The sites used 0o750, so this
    drops group access on config, state, cache and layers — and owner-only is what makes them enforceable on
    Windows.

14 production sites

config, store, selfinstall (7), man, star's extension hook, and devlore-test's workspace — the last
needing only OpenConfined, since its tree already exists.

Several carried a //nolint reading "an unconfined root holds no handle, so Close cannot fail." Untrue of a
confined root, so those now state what is actually being ignored and why.

8 fixtures were reaching across trees

Fixture Defect
TestIsDir/IsFile_NonExistent "/nonexistent/path" as a convenient absent value — outside the root, so confined it's an escape, not a not-exist
TestCopy_* (4) testFileResource called t.TempDir() itself, putting the source in a second tree
TestExtract_* (2) tarball written one level above the root it extracted into

Production has been confined all along — plan/provider.go:488, deploy/plan.go:387 — so these tests were
exercising a path production never takes. testFileResource now takes its directory, which puts the constraint
at the call site rather than hiding it in the helper.

Verified

  • make check — 103 packages ok, 0 failures
  • make build-all, make vet-all — linux, darwin, windows
  • gofmt -l over the change set — clean

4.5-fsroot-variants.status.md moves step 1's trace and step 2's creation rule from Open to Settled.

Not in this PR

The remaining test call sites still use OpenWritableUnconfined, which still exists. They go with #568 step 3,
which deletes the constructor they call — migrating them first would mean touching them twice.

Refs #568.

Every CLI-side root was writable-unconfined, and the reason recorded at the one site that gave one was not
access but bootstrapping: "the prefix need not exist yet -- a first install creates it -- and a confined root
requires its anchor to exist." Tracing all of them (#568 step 1) found none that reads or writes outside its own
tree, so the reason is the only thing keeping them unconfined.

cli.OpenTree is the compound operation that answers it: create the directory when absent, then open a confined
root at it. It is named for the creation so that opening keeps meaning only opening -- fsroot.OpenConfined stays
a query, and a missing root stays an error.

Creation runs through a root rather than os.MkdirAll, because a mode is worth nothing on windows without the
access-control list fsroot applies for it: a directory made 0o700 by os.MkdirAll carries an inherited DACL and
is readable by every account with access to its parent. The chain is created through a root anchored at the
volume, so every component is kernel-resolved and no window opens between creating the path and opening it. A
path that is not absolute on this platform is refused rather than resolved -- on windows that includes a leading
separator with no volume, which is drive-relative and belongs to #392.

The mode is 0o700 because the XDG Base Directory Specification says so: "If, when attempting to write a file,
the destination directory is non-existent an attempt should be made to create it with permission 0700." The
sites used 0o750, so this drops group access on the config, state, cache and layers trees. Owner-only is also
what makes them enforceable on windows.

Fourteen production sites move over: config, store, selfinstall, man, star's extension hook, and devlore-test's
workspace, which needed only OpenConfined since its tree already exists. Several carried a //nolint whose
reasoning was "an unconfined root holds no handle, so Close cannot fail" -- untrue of a confined root, so those
now say what is actually being ignored and why.

Eight test fixtures failed once their roots were confined, and all eight were reaching across trees because
nothing stopped them. testFileResource called t.TempDir() itself, so a copy's source sat in a second tree; the
archive fixtures wrote their tarball one level above the root they extracted into; and two tests used
"/nonexistent/path" as a convenient absent value, which confined is an escape rather than a not-exist.
Production has been confined all along -- plan/provider.go:488 and deploy/plan.go:387 -- so these tests were
exercising a path production never takes. testFileResource now takes its directory, which puts the constraint
at the call site instead of hiding it in the helper.

The status document moves step 1's trace and step 2's creation rule from Open to Settled.

Verified with make check (103 packages ok, 0 failures), and with build-all and vet-all across linux, darwin
and windows.

Refs #568.
@David-Noble-at-work David-Noble-at-work self-assigned this Aug 19, 2026
@David-Noble-at-work
David-Noble-at-work merged commit 0a96a5e into develop Aug 20, 2026
7 of 8 checks passed
@David-Noble-at-work
David-Noble-at-work deleted the confine-cli-trees branch August 20, 2026 00:06
David-Noble-at-work added a commit that referenced this pull request Aug 20, 2026
* refactor(fsroot): delete the unconfined variants

#568 step 3. Two types remain -- the confined root and its scratch sibling -- and one behavior, the kernel's.
Deleted: unconfinedRootReader, unconfinedRootReaderWriter, rootBase, errReadOnly, OpenUnconfined,
OpenWritableUnconfined, the two unconfined Mode constants, and the tests that existed only to pin the deleted
variants' behavior -- including TestApplyMode_BothWritableRootsProtect, whose surviving half duplicates
TestApplyMode_PrivateFileGetsProtectedDACL. Mode, ModeConfined and Open stay as a documented vestige:
op.RuntimeEnvironmentSpec.WithRoot still carries the Mode parameter, and step 5 deletes all three with it.

star was the last production unconfined root, mode-dispatched, so step 2's constructor migration never saw it.
It now anchors at the working directory, confined. A volume anchor was tried first and broke the generator:
star scripts resolve relative paths against the session root's anchor, so anchoring at the volume redefined
every relative path in every script. #571 holds the anchor question -- working directory, repository root,
home, or declared roots -- open until #563's hermeticity design rules it; no current extension uses an
absolute path or XDG, so wd-confined drops nothing exercised today.

Confining the tests exposed a production defect in CopyDir: it handed MkdirAll a full FileMode, type bits
included. os.Mkdir silently discards them; os.Root.Mkdir rejects them as an unsupported file mode, and
production selfinstall has run confined since #570, so star's extension-copy hook would have failed on its
first directory copy. Now Mode().Perm().

Thirteen remaining test call sites move to OpenConfined -- or cli.OpenTree where the tree must be created
first -- and TestChecksumFile_NonExistent stops reaching for /nonexistent/file.txt outside its root, the same
fixture defect class #570 fixed eight of. fsroot's own tests drop the variant tables and refusal suites that
tested the deleted types.

Found while gating and filed as #572: every provider's Makefile gen group lists node_builder.gen_test.go,
which the generator no longer emits, so the groups are permanently stale and cross-GOOS make build cannot
pass on any tree.

Verified with make check (103 packages ok, 0 failures), make build on darwin (full regeneration, every
generated file byte-identical to develop HEAD by blob SHA), and make vet under GOOS linux, darwin and
windows.

Refs #568, #571, #572.

* fix(star): close the test Applications, and move the knowledge target inside star's root

Two CI failures from confining star's session root, both the confinement collecting debts the unconfined
root had been hiding.

test (windows): seven TestLintCopyright failures, none of them assertions -- t.TempDir cleanup failed with
"being used by another process". A confined root is a real OS handle; the lint tests chdir into the temp
directory, build an Application there, and never close it, so the handle pinned the tree. The unconfined
root held no handle, which is why the leak was free until now. All three test sites that build an
Application now register t.Cleanup(r.Close) -- the two integration sites anchor on the repo directory and
never failed, but it is the same leak.

knowledge-extract: the workflow ran star from devlore-cli with --target at a sibling checkout, which a
wd-confined root refuses (statat ../devlore-registry: path escapes from parent). The registry is now checked
out inside the cli tree, after the star build so the nested repo cannot perturb generation, and the target
and the registry steps' working directories follow it. This constrains the scenario rather than
accommodating it; the Knowledge extension's own argument defaults are cross-root by design
(_find_sibling in extract.star), and the ruling recorded on #571 is that star should accommodate working
across roots -- the declared-roots candidate. Until #571 is answered, the target lives inside the root.

Refs #568, #571.

---------

Co-authored-by: David Noble <[email protected]>
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