Repository navigation
refactor(fsroot): delete the unconfined variants - #573
Merged
Merged
Conversation
#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.
… 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#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, both unconfinedModeconstants, and every test that existed only to pin thedeleted variants' behavior — including
TestApplyMode_BothWritableRootsProtect, whose surviving halfduplicates
TestApplyMode_PrivateFileGetsProtectedDACL.Kept, as a documented vestige:
Mode,ModeConfined(now an explicit= 0), andOpen—op.RuntimeEnvironmentSpec.WithRootstill carries theModeparameter, and #568 step 5 deletes all threewith it. Repo-wide grep: zero references to any deleted name.
Star's anchor (#571)
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 on the first
make build: star scripts resolve relative paths against the session root's anchor,so anchoring at
/redefined every relative path in every script. No current extension uses an absolute pathor XDG (verified), so wd-confined drops nothing exercised today. The real ruling — wd, repo root, home, or
declared roots — is #571, open until #563's hermeticity design.
Production defect exposed and fixed
CopyDirhandedMkdirAlla fullFileMode, type bits included.os.Mkdirsilently discards them;os.Root.Mkdirrejects them (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. The unconfined test
structurally could not catch this; the confined one did, immediately. Now
Mode().Perm()(cmd/internal/cli/selfinstall.go).
Test migrations
13 remaining constructor call sites →
OpenConfined(orcli.OpenTreewhere the tree must be createdfirst): index (1), selfinstall (5), file provider (2), signing (1), signing_windows (3), triad (1).
TestChecksumFile_NonExistentstops reaching for/nonexistent/file.txtoutside its root — the same fixturedefect class #570 fixed eight of. fsroot's own tests drop the variant tables and the refusal suites.
Found while gating → #572
Every provider's Makefile
&:gen group listsnode_builder.gen_test.go, which the generator no longeremits — the groups are permanently stale and
GOOS=linux/windows make buildcannot pass on any tree (itexecs target-OS tools on the host). Cross-platform verification for this PR is
make vet×3 GOOS + CI's realWindows job.
Verified
make check— 103 packages ok, 0 failuresmake build(darwin) — full regeneration; every generated file byte-identical to develop HEAD by blobSHA, across two regeneration cycles
make vet— GOOS=linux, darwin, windowsgofmt -l— cleanRefs #568, #571, #572.