Skip to content

Add OCI image support#191

Draft
henrybear327 wants to merge 8 commits into
sysprog21:mainfrom
henrybear327:oci/setup
Draft

Add OCI image support#191
henrybear327 wants to merge 8 commits into
sysprog21:mainfrom
henrybear327:oci/setup

Conversation

@henrybear327

@henrybear327 henrybear327 commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Please see the prior work: #34

The idea is that we implement basic OCI operations, such as pull, unpack, run, prune, status, policy (also the the counterparts like rmi, to complete the lifecycle so we can handle the resource clean up, etc.)

Instead of relying on pure C, this PR relying on the existing go tooling (since container ecosystems are mainly built with go) - go-containerregistry, crane, and umoci. It's about 2.8k lines of go code (about 5k lines of test go code). I think it's a lot more maintainable compared to the 38k lines of C code (this is without the full OCI lifecycle).

The main idea is that this go code is exposed as a command-line tool, handling just the OCI related operations.

Tracking issue: #31

Commands to try out

build/elfuse oci list
build/elfuse oci pull alpine:3
build/elfuse oci inspect alpine:3
build/elfuse oci list
build/elfuse oci rmi --force e7a1a92a5bfe 
build/elfuse oci list

build/elfuse-container run --entrypoint /usr/local/bin/python3 \\n    python:3.12 -c 'import json,math; print(json.dumps({"pi":round(math.pi,5),"ok":True}))'
build/elfuse oci run alpine:3 /bin/sh -c 'echo hello from elfuse'

Summary by cubic

Adds OCI image support via a new pure‑Go CLI elfuse-container, with pull/unpack/run over a spec‑compliant OCI image‑layout. macOS runs default to a case‑sensitive APFS sparsebundle with COW clones; CI now covers Linux layout/interop and a macOS HVF run smoke.

  • New Features

    • CLI: pull, unpack, inspect, list, run, rmi, prune (supports --platform, --insecure for loopback registries only, and digest pins for offline use).
    • Store uses an OCI image‑layout via go-containerregistry; interop checked with crane, skopeo, and umoci (scripts/oci-interop.sh).
    • macOS default run path: case‑sensitive sparsebundle + per‑run COW clonefile; --plain-rootfs fallback available.
    • run resolves Entrypoint/Cmd/Env/User/WorkingDir, injects runtime /etc/{resolv.conf,hosts,hostname}, then execs elfuse --sysroot.
  • Hardening & Fixes

    • Refactor: extracted a shared C launch entry (core/launch.c/.h) used by both the positional ELF path and elfuse-container run; wires UID/GID, envp, and cwd flags.
    • Renamed helper to elfuse-container; removed elfuse oci and multicall; docs updated to a two‑binary model.
    • Store updates serialized with <store>/.lock; auto‑pull only when a ref is truly absent.
    • GC and rmi race‑safe; prune never detaches a live bundle; sparsebundle mount‑point symlinks rejected; orphan COW clone reaping.
    • Unpack enforces exact tar modes regardless of umask and cleans up partial trees on failure.
    • Runtime /etc injection and named user/group lookups are rootfs‑bounded; procfs/device emulation for /dev/full (ENOSPC on write) and /dev/console; cached uname exposed for procfs.
    • CI builds elfuse-container, runs cross‑tool interop on Linux, and adds a macOS HVF run smoke; interop script normalizes manifests for skopeo; klauspost/compress bumped to v1.18.7.
    • Fixed memory leaks on option‑parse error paths by freeing launch‑flag allocations; minor env‑build leak fix.

Written for commit 2fd5111. Summary will update on new commits.

Review in cubic

cubic-dev-ai[bot]

This comment was marked as resolved.

@jserv jserv 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.

Rebase latest main branch, resolve conflicts, and refine per review messages.

@jserv

jserv commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Commands to try out

build/elfuse oci list
build/elfuse oci pull alpine:3
build/elfuse oci inspect alpine:3
build/elfuse oci list
build/elfuse oci rmi --force e7a1a92a5bfe 
build/elfuse oci list
build/elfuse-container run --entrypoint /usr/local/bin/python3 \\n    python:3.12 -c 'import json,math; print(json.dumps({"pi":round(math.pi,5),"ok":True}))'
build/elfuse oci run alpine:3 /bin/sh -c 'echo hello from elfuse'

Leveraging existing Go-based tools and packages is a great step toward full OCI image support. I agree with this change.

I suggest repositioning the build/elfuse binary as an efficient Linux syscall-to-macOS/Darwin runtime, while implementing build/elfuse-container in Go using OCI-related packages.

In other words, build/elfuse oci should not be considered a valid command. OCI-specific functionality should reside in elfuse-container, not in the elfuse executable.

@henrybear327 henrybear327 force-pushed the oci/setup branch 2 times, most recently from c18f864 to ed17166 Compare July 10, 2026 21:56
henrybear327 added a commit to henrybear327/elfuse that referenced this pull request Jul 11, 2026
Runtime file injection de-symlinked only the /etc directory itself; the
per-file os.WriteFile still followed an image-shipped symlink at
etc/{hostname,hosts,resolv.conf}, letting a malicious image redirect
the write outside the rootfs. Named --user resolution had the same
flaw on the read side: a symlinked etc/passwd or etc/group made
lookupPasswd/lookupGroup read host account files.

Route both through os.OpenRoot (the same containment the layer
unpacker already uses): injection unlinks the existing entry and
recreates it O_EXCL, and passwd/group opens are rootfs-bounded.
Regression tests pin both behaviors.

Reported by cubic review on PR sysprog21#191.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
henrybear327 added a commit to henrybear327/elfuse that referenced this pull request Jul 11, 2026
Mode finalization ran only when an entry carried setuid/setgid/sticky
bits, but the creation modes passed to os.Root.OpenFile and MkdirAll
are masked by the process umask: under e.g. umask 0077 a layer's
0755/0644 entries unpacked as 0700/0600 and were never corrected.

Chmod every created file and directory entry to its exact tar mode
(applyMode), and split the ensure-parent path out (ensureParent) so
finalizing an entry cannot reset the mode of an already-unpacked
parent directory to the 0755 default. Regression test unpacks under
umask 0077 and checks exact modes, including a 0700 parent that a
later child entry must not widen.

Reported by cubic review on PR sysprog21#191.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
henrybear327 added a commit to henrybear327/elfuse that referenced this pull request Jul 11, 2026
Both run paths infer "already unpacked" from the rootfs path existing
(csrun.go and the plain-directory path in commands.go), but unpackImage
created the destination before applying layers and left it behind on
failure. A run after a failed unpack therefore skipped the unpack and
executed against the truncated tree.

Delete the destination on unpack failure. The cleanup applies only
when unpackImage created the directory itself, so an explicit
pre-existing `unpack --rootfs DIR` target is never removed. Regression
test drives a two-layer image whose second layer fails and checks both
the cleanup and the pre-existing-directory guard.

Reported by cubic review on PR sysprog21#191.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
henrybear327 added a commit to henrybear327/elfuse that referenced this pull request Jul 11, 2026
pin/rmi updated refs.json with an unlocked load-modify-save cycle and
a fixed temp filename, so two concurrent pulls (or a pull racing an
rmi) could clobber each other's temp file and drop a just-recorded pin
-- a later unpack/run then reports the image as not pulled even though
its pull succeeded. index.json has the same read-modify-write shape
inside the layout package.

Add an exclusive flock on <store>/.lock held across pin's cycle,
addImage's check-append-pin, and rmi's whole resolve-modify-GC
sequence, and give savePins a unique temp name. A 16-writer
concurrent-pin regression test asserts no entry is lost.

Reported by cubic review on PR sysprog21#191.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
henrybear327 added a commit to henrybear327/elfuse that referenced this pull request Jul 11, 2026
cmdRun treated every s.image failure as "not pulled" and fell into the
auto-pull path, so a corrupt refs.json or unreadable layout triggered a
surprise network pull instead of reporting the store problem.

Introduce an errNotPulled sentinel wrapped by digestFor and
resolvePinnedTarget (user-facing message text is unchanged) and gate
the auto-pull on errors.Is. A regression test pins the two error
kinds apart.

Reported by cubic review on PR sysprog21#191.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
henrybear327 added a commit to henrybear327/elfuse that referenced this pull request Jul 11, 2026
Two GC robustness fixes:

DirEntry.Info() returning ENOENT (a blob reclaimed by a concurrent
rmi/prune between ReadDir and Info) aborted the whole GC pass; skip
the vanished entry instead, matching the IsNotExist tolerance already
used elsewhere in gc.go.

A last-pin rmi committed the pin removal to refs.json before removing
the manifest descriptor from index.json. If descriptor removal then
failed, the image was stranded: no ref resolves to it, the descriptor
keeps every blob live, and prune never removes descriptors. Reorder
the writes so the same failure window leaves a stale pin over a
removed descriptor, which a retried rmi resolves and completes
(RemoveDescriptors is a filter; re-removal is a no-op). Regression
test forces the descriptor write to fail and checks the pin survives
and the retry finishes.

Reported by cubic review on PR sysprog21#191.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
henrybear327 added a commit to henrybear327/elfuse that referenced this pull request Jul 11, 2026
pruneCSBundle ran the crash-recovery sweep -- including an
unconditional hdiutil detach -force of any attached volume -- before
checking whether the digest was still pinned, so a non---all
`prune --cache` could rip the rootfs out from under an active run
(the sweep cannot tell a crashed leftover mount from a live one by
mount state alone).

Two guards fix this. The live[key] pin check now runs before the
sweep, so a plain prune never touches a pinned bundle; a crashed
pinned bundle is recovered by the next run's provision or by --all.
And sweepCSBundle now reports a volume busy instead of detaching when
a run-<pid> clone of a live process remains inside it, protecting the
--all and legacy/unpinned paths too.

The gated darwin round-trip now covers the busy path, and folds in two
review fixes of its own: the orphan clone uses a never-assignable pid
(999999999 > kern.maxpid) instead of a reaped pid that the OS could
reuse mid-test, and a t.Cleanup force-detach keeps failed runs from
leaking an attached volume.

Reported by cubic review on PR sysprog21#191.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
henrybear327 added a commit to henrybear327/elfuse that referenced this pull request Jul 11, 2026
clearDir followed a symlink at the mount-point path, so pre-attach
cleanup of a corrupt or tampered store could empty a directory outside
the OCI cache. Reject a symlinked mount dir with an error instead.

Also drop csMount.imagePath: it was set but never read in production
(every consumer derives <bundle>/rootfs.sparsebundle itself), so the
field was state to maintain with no behavior behind it.

Reported by cubic review on PR sysprog21#191.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
henrybear327 added a commit to henrybear327/elfuse that referenced this pull request Jul 11, 2026
runMainSubprocess read the stdout pipe to EOF before touching the
stderr pipe -- the sequential-read pattern the os/exec docs warn can
deadlock once the unread stream fills its ~64KB buffer. Hand both
streams to exec.Cmd as bytes.Buffers instead, which the package drains
concurrently; less code and no deadlock potential as outputs grow.

Reported by cubic review on PR sysprog21#191.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
henrybear327 added a commit to henrybear327/elfuse that referenced this pull request Jul 11, 2026
The jq expression comparing registry truth against the store pin
returned every manifest matching os/arch, so a manifest list with two
matching entries (several variants, or a future extra descriptor)
produced a multi-line string and failed the equality check on a valid
image. Wrap the selection in first(...) and exclude BuildKit
attestation manifests, mirroring what crane.Pull(WithPlatform)
resolves.

Reported by cubic review on PR sysprog21#191.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
henrybear327 added a commit to henrybear327/elfuse that referenced this pull request Jul 11, 2026
The prune summary presented one number as a uniform on-disk-allocation
figure, but blob bytes come from logical file sizes while cache-dir
bytes come from st_blocks allocation. Say so in the pruneReport doc and
mark the user-facing total approximate rather than pretending to a
single metric.

Reported by cubic review on PR sysprog21#191.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
henrybear327 added a commit to henrybear327/elfuse that referenced this pull request Jul 11, 2026
v1.18.7 fixes an out-of-bounds read in s2.NewDict. The dependency is
indirect (via go-containerregistry) and nothing here imports the s2
package, so there is no reachable exposure -- this is dependency
hygiene while the report is fresh.

Reported by cubic review on PR sysprog21#191.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
henrybear327 added a commit to henrybear327/elfuse that referenced this pull request Jul 11, 2026
Every pre-launch failure path unlinks a FUSE-materialized temporary
ELF (elf_host_temp) before returning; the --env/--clear-env OOM branch
was the lone omission and leaked the temp file on disk. Mirror the
sibling paths.

Reported by cubic review on PR sysprog21#191.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot 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.

25 issues found and verified against the latest diff

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="cmd/elfuse-container/list.go">

<violation number="1" location="cmd/elfuse-container/list.go:32">
P2: Concurrent `rmi` can make `list` fail rather than produce a coherent result: it reads pins and manifests without the store lock while `rmi` deletes both under that lock. Hold the same lock across snapshot construction so lifecycle commands cannot interleave.</violation>

<violation number="2" location="cmd/elfuse-container/list.go:57">
P3: Variant-specific images are displayed as only `os/arch`, so `list` loses part of the selected OCI platform. Include `cfg.Variant` when present.</violation>
</file>

<file name="src/main.c">

<violation number="1" location="src/main.c:506">
P2: Direct `elfuse --sysroot ... --workdir relative/path` can start in a host-relative directory rather than the guest root. Reject empty or non-`/`-prefixed workdirs when parsing, matching documented and container-wrapper behavior.</violation>
</file>

<file name="cmd/elfuse-container/run.go">

<violation number="1" location="cmd/elfuse-container/run.go:121">
P2: Terminal signals reach this child twice because it remains in the helper's process group and this branch forwards the helper's copy again. Signal-aware guests can run an interrupt/termination handler twice; avoid forwarding group-delivered signals, or put the child in a separate process group and forward exactly once.</violation>
</file>

<file name="cmd/elfuse-container/sparsebundle.go">

<violation number="1" location="cmd/elfuse-container/sparsebundle.go:70">
P3: Attach failures omit hdiutil's diagnostic because `Output()` discards stderr while the error message prints `out`. Use `CombinedOutput()` so invalid-image and mount errors remain actionable.</violation>

<violation number="2" location="cmd/elfuse-container/sparsebundle.go:191">
P2: Mount paths containing XML-escaped characters are returned as literal entities, so marker creation and cleanup use a nonexistent path after attach. Decode the plist string (preferably with an XML/plist parser) before returning it.</violation>
</file>

<file name="cmd/elfuse-container/commands.go">

<violation number="1" location="cmd/elfuse-container/commands.go:130">
P1: `run --platform` is ignored whenever the ref is already present, so a previously pulled different architecture can be launched despite the requested platform. Store pins per platform or verify the pinned image platform and pull/select the requested variant before computing the run spec.</violation>
</file>

<file name="cmd/elfuse-container/inspect.go">

<violation number="1" location="cmd/elfuse-container/inspect.go:31">
P2: `inspect --json` exits successfully when stdout fails (for example a closed pipe or full redirected filesystem), so callers can consume truncated/empty JSON as success. Propagate writer errors; apply the same handling to the human-summary writes.</violation>
</file>

<file name="cmd/elfuse-container/etc.go">

<violation number="1" location="cmd/elfuse-container/etc.go:43">
P2: `injectRuntimeFiles` silently accepts when `/etc` is a regular file, leading to a confusing error later when writing runtime files inside it. If the rootfs has a regular file at `etc` (not a symlink and not a directory), `root.Lstat("etc")` succeeds without the symlink flag, so the removal branch is skipped. Then `root.Mkdir("etc", 0o755)` fails with `EEXIST` which is swallowed by the `errors.Is(err, fs.ErrExist)` check. The code proceeds believing `/etc` is a usable directory, but the subsequent `writeRuntimeFile(root, "etc/hostname", ...)` calls fail with a "not a directory" error — confusing because the user never asked to remove anything.

Consider adding an explicit `li.IsDir()` check after the symlink guard: if `etc` exists and is not a directory and not a symlink, remove it first (or return a clear error) before calling `Mkdir`.</violation>

<violation number="2" location="cmd/elfuse-container/etc.go:76">
P2: When host resolver config is unavailable, guest DNS is silently redirected to Google's public resolver, which can leak private/split-DNS lookups and does not preserve the stated host-network behavior. Surface this as a run error or require an explicit fallback resolver instead of hard-coding 8.8.8.8.</violation>
</file>

<file name="cmd/elfuse-container/unpack_test.go">

<violation number="1" location="cmd/elfuse-container/unpack_test.go:263">
P3: TestUnpackHardlink's comment claims the test validates 'Both must refer to the same inode (hardlink), same content' but the test only checks that os.Stat succeeds for the hardlink entry. It doesn't verify content equality (os.ReadFile + string comparison) or inode sharing (os.SameFile on FileInfo). Consider either adding those checks or trimming the comment to match what's actually verified.</violation>

<violation number="2" location="cmd/elfuse-container/unpack_test.go:320">
P3: TestUnpackReadsExactSize comment claims to verify 'no over-read, no short read' but only validates short reads. The countingReader has exactly hdr.Size bytes, so any over-read attempt just hits EOF and r.n stays at len(content). To detect over-reads, the reader needs MORE data than Size and the test must verify only Size bytes were consumed (e.g., a reader that panics if Read is called after Size bytes). Consider updating the comment to reflect what's actually tested, or making the reader defensive.</violation>
</file>

<file name="scripts/oci-interop.sh">

<violation number="1" location="scripts/oci-interop.sh:93">
P2: Setting `PLAT_OS` or `PLAT_ARCH` makes this check fail against a correctly pulled image: those variables change only `crane manifest` selection, not the preceding `elfuse-container pull`. Keep this comparison fixed to the helper default, or pass the same platform to `pull` before comparing.</violation>
</file>

<file name="cmd/elfuse-container/gc.go">

<violation number="1" location="cmd/elfuse-container/gc.go:33">
P1: A concurrent `pull` can finish with blobs deleted by `prune` before its index descriptor is written. Serialize standalone GC's scan/delete phase with `addImage`; keep an internal locked variant for `rmi`, which already holds the lock.</violation>

<violation number="2" location="cmd/elfuse-container/gc.go:275">
P1: `rmi --force` can tear down an active container's filesystem: it force-detaches the Darwin sparsebundle or removes the plain rootfs while the guest still uses it. Track active runs or reject busy caches instead of treating `force` alone as permission to remove them.</violation>

<violation number="3" location="cmd/elfuse-container/gc.go:290">
P1: An `rmi` can make a concurrently starting cold `run` fail while it is unpacking. Serialize image reads/unpack with removal, or record active setup before descriptor/blob GC.</violation>
</file>

<file name="cmd/elfuse-container/unpack.go">

<violation number="1" location="cmd/elfuse-container/unpack.go:115">
P1: Valid layers with an opaque marker after child entries lose those same-layer children. Apply opaque whiteouts before processing additions for that layer, independent of tar ordering.</violation>

<violation number="2" location="cmd/elfuse-container/unpack.go:118">
P2: An invalid `.wh.` entry deletes its containing directory rather than failing extraction. Validate the whiteout suffix before joining it to the parent path.</violation>

<violation number="3" location="cmd/elfuse-container/unpack.go:228">
P2: A directory entry cannot replace a lower-layer symlink or regular file, so affected images unpack a different filesystem tree. Remove a pre-existing non-directory before creating the directory, while preserving existing real directories and their children.</violation>
</file>

<file name="Makefile">

<violation number="1" location="Makefile:157">
P3: The `oci-vet` darwin cross-vet run doesn't set `GOARCH=arm64`, so on an amd64 Linux host it vets the darwin code for `darwin/amd64` instead of the actual target `darwin/arm64`. CI already pins `GOARCH=arm64` for this vet. It's unlikely to cause a real build issue (the darwin-specific files are `//go:build darwin` without an arch constraint), but adding `GOARCH=arm64` makes the local `make oci-vet` gate match CI more closely and avoids any future arch-specific vet diagnostics from slipping through.</violation>
</file>

<file name="cmd/elfuse-container/cache_darwin.go">

<violation number="1" location="cmd/elfuse-container/cache_darwin.go:188">
P1: `prune --cache --all` can detach and delete a sparsebundle while a live `--no-clone` guest is using its base rootfs. `--no-clone` creates no `run-*` marker, so this check reports the mount idle; track active mount ownership independently (and cover the pre-clone startup window) before detaching.</violation>
</file>

<file name="cmd/elfuse-container/common.go">

<violation number="1" location="cmd/elfuse-container/common.go:49">
P3: Malformed `--platform` values are accepted and passed to `crane` instead of returning the advertised parse error. Validate nonempty components and reject inputs with more than three slash-separated segments.</violation>
</file>

<file name="cmd/elfuse-container/sparsebundle_test.go">

<violation number="1" location="cmd/elfuse-container/sparsebundle_test.go:117">
P3: TestCSMountCloseSuccessRootfsDirAndDetachAfterAttachError tests rootfsDir(), Close() lifecycle, and detachAfterAttachError() as separate behaviors in a single function. Splitting these into targeted subtests or separate functions would improve isolation and make failures easier to diagnose — a rootfsDir failure currently masks all Close and detachAfterAttachError checks.</violation>
</file>

<file name="cmd/elfuse-container/csrun.go">

<violation number="1" location="cmd/elfuse-container/csrun.go:76">
P1: Concurrent `oci run` calls for the same image can tear down each other's sysroot: the later call force-detaches the digest's already-mounted sparsebundle before creating its clone. Serialize ownership of a digest mount or reject/wait when `hasLiveClone` reports an active run, rather than treating every existing mount as stale.</violation>
</file>

<file name="cmd/elfuse-container/clone_sweep.go">

<violation number="1" location="cmd/elfuse-container/clone_sweep.go:54">
P2: hasLiveClone returns false on ReadDir errors, which could cause an unsafe volume detach during prune. If the mount directory is temporarily unreadable (e.g., transient FS issue, permission flake) while a live container run is still using it, sweepCSBundle will force-detach the sparsebundle volume out from under the running guest, causing rootfs I/O failures. `reapOrphanClones` has the same silent-fail pattern but only affects reporting, not safety.

**Suggestion:** Return `true` when `os.ReadDir` fails so the callee conservatively treats the volume as busy, matching the defensive pattern used in `processAlive` (non-ESRCH → alive).</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread cmd/elfuse-container/sparsebundle.go
if err != nil {
return err
}
img, err := s.image(ref)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: run --platform is ignored whenever the ref is already present, so a previously pulled different architecture can be launched despite the requested platform. Store pins per platform or verify the pinned image platform and pull/select the requested variant before computing the run spec.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/elfuse-container/commands.go, line 130:

<comment>`run --platform` is ignored whenever the ref is already present, so a previously pulled different architecture can be launched despite the requested platform. Store pins per platform or verify the pinned image platform and pull/select the requested variant before computing the run spec.</comment>

<file context>
@@ -0,0 +1,225 @@
+	if err != nil {
+		return err
+	}
+	img, err := s.image(ref)
+	if err != nil {
+		// Auto-pull only when the ref is simply absent, so `run` is
</file context>

return nil, false, nil
}
reaped = reapOrphanClonesFn(mnt)
if hasLiveCloneFn(mnt) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: prune --cache --all can detach and delete a sparsebundle while a live --no-clone guest is using its base rootfs. --no-clone creates no run-* marker, so this check reports the mount idle; track active mount ownership independently (and cover the pre-clone startup window) before detaching.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/elfuse-container/cache_darwin.go, line 188:

<comment>`prune --cache --all` can detach and delete a sparsebundle while a live `--no-clone` guest is using its base rootfs. `--no-clone` creates no `run-*` marker, so this check reports the mount idle; track active mount ownership independently (and cover the pre-clone startup window) before detaching.</comment>

<file context>
@@ -0,0 +1,195 @@
+		return nil, false, nil
+	}
+	reaped = reapOrphanClonesFn(mnt)
+	if hasLiveCloneFn(mnt) {
+		return reaped, true, nil
+	}
</file context>

// intra-volume only), so it is instant and free until the guest writes (COW).
// It isolates each run's mutations from the warm base, so re-runs stay clean.
func runCaseSensitive(cf commonFlags, s *store, ref, digest string, cfg *v1.ConfigFile, rf runFlags, tail []string) error {
m, baseRootfs, err := ensureCaseSensitiveRootfsForRun(cf, s, ref, digest, rf.sparseSize)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: Concurrent oci run calls for the same image can tear down each other's sysroot: the later call force-detaches the digest's already-mounted sparsebundle before creating its clone. Serialize ownership of a digest mount or reject/wait when hasLiveClone reports an active run, rather than treating every existing mount as stale.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/elfuse-container/csrun.go, line 76:

<comment>Concurrent `oci run` calls for the same image can tear down each other's sysroot: the later call force-detaches the digest's already-mounted sparsebundle before creating its clone. Serialize ownership of a digest mount or reject/wait when `hasLiveClone` reports an active run, rather than treating every existing mount as stale.</comment>

<file context>
@@ -0,0 +1,161 @@
+// intra-volume only), so it is instant and free until the guest writes (COW).
+// It isolates each run's mutations from the warm base, so re-runs stay clean.
+func runCaseSensitive(cf commonFlags, s *store, ref, digest string, cfg *v1.ConfigFile, rf runFlags, tail []string) error {
+	m, baseRootfs, err := ensureCaseSensitiveRootfsForRun(cf, s, ref, digest, rf.sparseSize)
+	if err != nil {
+		return err
</file context>

// descriptor, which a retried rmi resolves and finishes
// (RemoveDescriptors is a filter, so re-removing is a no-op).
if lastPin {
if err := s.removeManifestDescriptor(digest); err != nil {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: An rmi can make a concurrently starting cold run fail while it is unpacking. Serialize image reads/unpack with removal, or record active setup before descriptor/blob GC.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/elfuse-container/gc.go, line 290:

<comment>An `rmi` can make a concurrently starting cold `run` fail while it is unpacking. Serialize image reads/unpack with removal, or record active setup before descriptor/blob GC.</comment>

<file context>
@@ -0,0 +1,304 @@
+	// descriptor, which a retried rmi resolves and finishes
+	// (RemoveDescriptors is a filter, so re-removing is a no-op).
+	if lastPin {
+		if err := s.removeManifestDescriptor(digest); err != nil {
+			return rmReport{}, fmt.Errorf("rmi: remove manifest descriptor for %q: %w", ref, err)
+		}
</file context>

}
}

func TestCSMountCloseSuccessRootfsDirAndDetachAfterAttachError(t *testing.T) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: TestCSMountCloseSuccessRootfsDirAndDetachAfterAttachError tests rootfsDir(), Close() lifecycle, and detachAfterAttachError() as separate behaviors in a single function. Splitting these into targeted subtests or separate functions would improve isolation and make failures easier to diagnose — a rootfsDir failure currently masks all Close and detachAfterAttachError checks.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/elfuse-container/sparsebundle_test.go, line 117:

<comment>TestCSMountCloseSuccessRootfsDirAndDetachAfterAttachError tests rootfsDir(), Close() lifecycle, and detachAfterAttachError() as separate behaviors in a single function. Splitting these into targeted subtests or separate functions would improve isolation and make failures easier to diagnose — a rootfsDir failure currently masks all Close and detachAfterAttachError checks.</comment>

<file context>
@@ -0,0 +1,349 @@
+	}
+}
+
+func TestCSMountCloseSuccessRootfsDirAndDetachAfterAttachError(t *testing.T) {
+	oldDetach := detachForce
+	var detached []string
</file context>

}
}

func TestUnpackHardlink(t *testing.T) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: TestUnpackHardlink's comment claims the test validates 'Both must refer to the same inode (hardlink), same content' but the test only checks that os.Stat succeeds for the hardlink entry. It doesn't verify content equality (os.ReadFile + string comparison) or inode sharing (os.SameFile on FileInfo). Consider either adding those checks or trimming the comment to match what's actually verified.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/elfuse-container/unpack_test.go, line 263:

<comment>TestUnpackHardlink's comment claims the test validates 'Both must refer to the same inode (hardlink), same content' but the test only checks that os.Stat succeeds for the hardlink entry. It doesn't verify content equality (os.ReadFile + string comparison) or inode sharing (os.SameFile on FileInfo). Consider either adding those checks or trimming the comment to match what's actually verified.</comment>

<file context>
@@ -0,0 +1,348 @@
+	}
+}
+
+func TestUnpackHardlink(t *testing.T) {
+	root, dir := newRoot(t)
+	applyEntryWithContent(t, root, regHeader("etc/passwd", 0o644, 5), "hello")
</file context>


// Ensure applyEntry reads exactly the header's Size bytes from the reader
// (no over-read, no short read).
func TestUnpackReadsExactSize(t *testing.T) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: TestUnpackReadsExactSize comment claims to verify 'no over-read, no short read' but only validates short reads. The countingReader has exactly hdr.Size bytes, so any over-read attempt just hits EOF and r.n stays at len(content). To detect over-reads, the reader needs MORE data than Size and the test must verify only Size bytes were consumed (e.g., a reader that panics if Read is called after Size bytes). Consider updating the comment to reflect what's actually tested, or making the reader defensive.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/elfuse-container/unpack_test.go, line 320:

<comment>TestUnpackReadsExactSize comment claims to verify 'no over-read, no short read' but only validates short reads. The countingReader has exactly hdr.Size bytes, so any over-read attempt just hits EOF and r.n stays at len(content). To detect over-reads, the reader needs MORE data than Size and the test must verify only Size bytes were consumed (e.g., a reader that panics if Read is called after Size bytes). Consider updating the comment to reflect what's actually tested, or making the reader defensive.</comment>

<file context>
@@ -0,0 +1,348 @@
+
+// Ensure applyEntry reads exactly the header's Size bytes from the reader
+// (no over-read, no short read).
+func TestUnpackReadsExactSize(t *testing.T) {
+	root, dir := newRoot(t)
+	content := "abc123"
</file context>

Comment thread scripts/oci-interop.sh Outdated
Comment thread Makefile
# non-darwin stubs are checked from a macOS host. oci-lint bundles both so a
# local run matches the CI gate.
.PHONY: oci-vet oci-fmt-check oci-lint
oci-vet:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The oci-vet darwin cross-vet run doesn't set GOARCH=arm64, so on an amd64 Linux host it vets the darwin code for darwin/amd64 instead of the actual target darwin/arm64. CI already pins GOARCH=arm64 for this vet. It's unlikely to cause a real build issue (the darwin-specific files are //go:build darwin without an arch constraint), but adding GOARCH=arm64 makes the local make oci-vet gate match CI more closely and avoids any future arch-specific vet diagnostics from slipping through.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Makefile, line 157:

<comment>The `oci-vet` darwin cross-vet run doesn't set `GOARCH=arm64`, so on an amd64 Linux host it vets the darwin code for `darwin/amd64` instead of the actual target `darwin/arm64`. CI already pins `GOARCH=arm64` for this vet. It's unlikely to cause a real build issue (the darwin-specific files are `//go:build darwin` without an arch constraint), but adding `GOARCH=arm64` makes the local `make oci-vet` gate match CI more closely and avoids any future arch-specific vet diagnostics from slipping through.</comment>

<file context>
@@ -125,6 +126,55 @@ elfuse: $(ELFUSE_BIN)
+# non-darwin stubs are checked from a macOS host. oci-lint bundles both so a
+# local run matches the CI gate.
+.PHONY: oci-vet oci-fmt-check oci-lint
+oci-vet:
+	$(Q)$(GO) vet ./cmd/elfuse-container/
+	$(Q)GOOS=darwin $(GO) vet ./cmd/elfuse-container/
</file context>

@henrybear327 henrybear327 marked this pull request as draft July 11, 2026 12:21
@henrybear327

henrybear327 commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator Author

Putting this PR as draft for now. Experimenting with Claude code and the setup went wrong, the PoC code polluted the codebase.

elfuse_launch owns guest bring-up (guest_bootstrap_prepare, the FUSE-temp
unlink, the sysroot casefold probe, proc_set_ids, vCPU creation, GDB
init/sync/wait, the run loop, gdb_stub_shutdown, the shim counter + syscall
histogram dumps, and guest_destroy).

main() retains the original CLI argv
(proctitle rewriting), option parsing, sysroot provisioning, the shebang loop,
the --gdb x86_64 guard, host cwd, and the heap resource cleanup. launch_args_t
carries the fields `elfuse-container run` drives (has_creds/uid/gid, cwd_guest,
envp); they are inert until this commit wires the flags.
Add elfuse-container, a standalone Go binary that owns the OCI image
pipeline: pull, unpack, inspect, and run over an OCI image-layout
store, with run resolving Entrypoint/Cmd/Env/User/WorkingDir and
exec'ing the existing elfuse launch path. elfuse itself stays a pure
Linux syscall-to-Darwin runtime with no OCI commands; elfuse-container
locates it as a sibling binary ($ELFUSE_BIN overrides for tests).

The pipeline is hardened at the trust boundaries: refs.json and
index.json updates are serialized by an exclusive store flock, run
auto-pulls only when a ref is genuinely absent (store corruption
surfaces instead of triggering a network pull), runtime /etc injection
and symbolic --user resolution are rootfs-bounded via os.OpenRoot so
image-controlled symlinks cannot escape, unpacked modes are finalized
with an explicit chmod so a restrictive host umask cannot corrupt
layer permissions, and a failed unpack removes the partial rootfs it
created rather than leaving it to be executed later.

@jserv jserv 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.

Refine per review messages from @cubic-dev-ai and squash commits for further review.

Default OCI runs now use a case-sensitive APFS sparsebundle on macOS, with a per-run clonefile COW rootfs for isolation and warm-run speed. The plain-rootfs path remains available with --plain-rootfs.

The sparsebundle cache is keyed by the pinned manifest digest, and the non-Darwin stub keeps elfuse-container buildable for pull/inspect/unpack tests on Linux.
@henrybear327

Copy link
Copy Markdown
Collaborator Author

Refine per review messages from @cubic-dev-ai and squash commits for further review.

I am in the process of resetting the changes.

The original 8 commits would be amended for changes as those were the logically reviewable units.

Add list/images, rmi, and prune on top of the OCI image-layout store. rmi and prune use reachability GC, while cache cleanup handles plain rootfs caches and macOS sparsebundle caches through platform-specific seams.
@jserv

jserv commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

The original 8 commits would be amended for changes as those were the logically reviewable units.

Avoid using Semantic Commit Messages, Instead, stick to How to Write a Git Commit Message,

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.

2 participants