Move the supported systemd path to two services: an owner-user bangerd for
orchestration and a narrow root helper for bridge/tap, NAT/resolver, dm/loop,
and Firecracker ownership. This removes repeated sudo from daily vm and image
flows without leaving the general daemon running as root.
Add install metadata, system install/status/restart/uninstall commands, and a
system-owned runtime layout. Keep user SSH/config material in the owner home,
lock file_sync to the owner home, and move daemon known_hosts handling out of
the old root-owned control path.
Route privileged lifecycle steps through typed privilegedOps calls, harden the
two systemd units, and rewrite smoke plus docs around the supported service
model.
Verified with make build, make test, make lint, and make smoke on the
supported systemd host path.
A VM name flows into five places that all have narrower grammars
than "arbitrary string":
- the guest's /etc/hostname (vm_disk.patchRootOverlay)
- the guest's /etc/hosts (same)
- the <name>.vm DNS record (vmdns.RecordName)
- the kernel command line (system.BuildBootArgs*)
- VM-dir file-path fragments (layout.VMsDir/<id>, etc.)
Nothing in the chain was validating the input. A name with
whitespace, newline, dot, slash, colon, or = would produce broken
hostnames, weird DNS labels, smuggled kernel cmdline tokens, or
(in the worst case) surprising traversal through the on-disk
layout. Not host shell injection — we already avoid shelling out
with the raw name — but a real correctness and supportability bug.
New: model.ValidateVMName. Rules:
- 1..63 chars (DNS label max per RFC 1123; also a comfortable
/etc/hostname cap)
- lowercase ASCII letters, digits, '-' only
- no leading or trailing '-'
- no normalization — the name is the user-visible identifier
(store key, `ssh <name>.vm`, `vm show`); silently rewriting
"MyVM" → "myvm" would hand the user back something different
than they typed
Called from two places:
- internal/cli/commands_vm.go vmCreateParamsFromFlags — rejects
bad `--name` values before any RPC. Empty name still passes
through so the daemon can generate one.
- internal/daemon/vm_create.go reserveVM — defense in depth for
any non-CLI RPC caller (SDK, direct JSON over the socket).
Tests:
- internal/model/vm_name_test.go — exhaustive character-class
matrix (space, newline, tab, dot, slash, colon, equals, quote,
control chars, unicode letters, uppercase, leading/trailing
hyphen, over-length, max-length-exact, digits-only).
- internal/cli TestVMCreateParamsFromFlagsRejectsInvalidName —
CLI wire-through + empty-name passthrough.
- internal/daemon TestReserveVMRejectsInvalidName — daemon
defense-in-depth (including `box/../evil` path-traversal).
- scripts/smoke.sh — end-to-end rejection + no-leaked-row
assertion.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
--readonly ran `chmod -R a-w` over the workspace after copying, but
every banger guest boots as root, and root bypasses DAC mode checks.
So a user running `vm workspace prepare ... --readonly` got the
mode bits set to 0444 but `echo x >> file` in the guest still
succeeded. The flag promised enforcement it couldn't deliver.
The feature also doesn't match the product model: workspaces are
prepared precisely so the guest CAN edit them, and `workspace
export` exists to pull those edits back as a patch. A
"read-only workspace" contradicts that loop.
Removed:
- CLI flag `--readonly` on `vm workspace prepare`
- api.VMWorkspacePrepareParams.ReadOnly field
- model.WorkspacePrepareResult.ReadOnly field
- daemon chmod dispatch in prepareVMWorkspaceGuestIO
- smoke scenario pinning the (advisory) mode-bit behavior
- misleading "exportbox-readonly" VM name in an unrelated export
test (the test is about not mutating the real git index;
renamed to exportbox-noindex-mutation)
If real enforcement becomes a user need later, the right primitive
is `chattr +i` (immutable bit — root CAN'T write) or a ro bind-mount.
Reintroducing a new flag is cheaper than debugging what the current
one actually guarantees.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two bugs in the untracked-skipped warning, both surfaced by review.
1. Wrong scope for subdir inputs. The helper accepted any path the
caller had (sourcePath, which may be a user-supplied subdirectory)
and ran `git -C <path> ls-files --others --exclude-standard`. Git
scopes that output to the cwd, so pointing `vm run ./repo/sub` at
a subdir silently underreported untracked files living elsewhere
in the repo — exactly the files the warning exists to surface.
Fix: resolve sourcePath to the repo root inside the helper via
`rev-parse --show-toplevel` before counting.
2. Inconsistent failure handling. The comment said the helper should
be silent when the count can't be determined; the body returned
the error. vm_run.go treated the error as non-fatal (logged a
warning, continued); workspace prepare and --dry-run aborted the
whole operation on the same helper failure. A courtesy notice
shouldn't kill the operation.
Fix: make the helper best-effort in signature and body — no error
return, swallows rev-parse + count failures, emits nothing when
there's nothing to say. All three callers lose their error
branches.
Regression tests:
- subdir input reports the root-level untracked file (the bug case)
- non-repo path produces silence, not a fatal error
- inspector whose runner errors on every call produces silence
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three test seams were still package-level mutable vars, which tests
had to swap before use. That's the classic path to flaky parallel
tests — two goroutines fighting over the same global fake. Push each
down to the struct that owns the behaviour.
internal/daemon/dns_routing.go
lookupExecutableFunc + vmDNSAddrFunc → fields on *HostNetwork,
defaulted at newHostNetwork time. dns_routing_test builds
HostNetwork{..., lookupExecutable: stub, vmDNSAddr: stub} inline,
no more t.Cleanup dance around package-level vars.
internal/daemon/preflight.go + doctor.go
vsockHostDevicePath (mutable string) → vsockHostDevice field on
*VMService, defaulted via defaultVsockHostDevice constant in
newVMService. Preflight reads s.vsockHostDevice; doctor reads
d.vm.vsockHostDevice. Logger test sets d.vm.vsockHostDevice = tmp
after wireServices.
internal/daemon/workspace/workspace.go
HostCommandOutputFunc → *Inspector struct with a Runner field.
Every git-using helper (GitOutput, GitTrimmedOutput,
GitResolvedConfigValue, RunHostCommand, ListSubmodules,
ListOverlayPaths, CountUntrackedPaths, InspectRepo,
ImportRepoToGuest, PrepareRepoCopy) is now a method on *Inspector.
NewInspector() wraps the real host runner for production;
WorkspaceService holds one via repoInspector, CLI deps holds one
too. cli_test.go's submodule-rejection test builds its own
Inspector with a scripted Runner instead of patching a global.
Pure helpers (FinalizeScript, ResolveSourcePath, ParsePrepareMode,
ShellQuote, FormatStepError, GitFileURL, ParseNullSeparatedOutput)
stay free functions since they don't touch the host.
Sentinel: grep for HostCommandOutputFunc, lookupExecutableFunc,
vmDNSAddrFunc, vsockHostDevicePath is now empty across internal/.
make lint test green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Workspace-mode vm run and vm workspace prepare used to copy both
tracked AND untracked non-ignored files into the guest. That silently
catches local .env files, scratch notes, credentials, and any other
working-tree state a developer hasn't explicitly gitignored — a real
data-exposure footgun given the golden image ships Docker and the
usual dev tooling.
Flip the default to tracked-only. Users who actually want the fuller
set opt in with --include-untracked (documented in both commands'
help). Gitignored files are still always excluded regardless of the
flag.
Add --dry-run to both vm run and vm workspace prepare. Dry-run
inspects the repo CLI-side (no VM created, no daemon RPC needed since
the daemon is always local and the inspection is a pure git read),
prints the exact file list + mode, and exits. A byte-level preview of
what would land in the guest.
When running real (non-dry) and untracked files exist in the repo but
are being skipped under the new default, print a one-line notice
pointing to --include-untracked so users aren't surprised when the
guest is missing something they expected.
Signature changes:
- ListOverlayPaths takes an includeUntracked bool (tracked always;
untracked gated by flag).
- InspectRepo takes the same flag and passes it through.
- VMWorkspacePrepareParams gains IncludeUntracked.
- WorkspaceService.workspaceInspectRepo seam signature widened to
match (4 callers in tests updated).
New workspace package tests cover both modes and verify that
gitignored files never leak regardless of the flag.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cuts the daemon-managed guest-session machinery (start/list/show/
logs/stop/kill/attach/send). The feature shipped aimed at agent-
orchestration workflows (programmatic stdin piping into a long-lived
guest process) that aren't driving any concrete user today, and the
~2.3K LOC of daemon surface area — attach bridge, FIFO keepalive,
controller registry, sessionstream framing, SQLite persistence — was
locking in an API we'd have to keep through v0.1.0.
Anything session-flavoured that people actually need today can be
done with `vm ssh + tmux` or `vm run -- cmd`.
Deleted:
- internal/cli/commands_vm_session.go
- internal/daemon/{guest_sessions,session_lifecycle,session_attach,session_stream,session_controller}.go
- internal/daemon/session/ (guest-session helpers package)
- internal/sessionstream/ (framing package)
- internal/daemon/guest_sessions_test.go
- internal/store/guest_session_test.go
- GuestSession* types from internal/{api,model}
- Store UpsertGuestSession/GetGuestSession/ListGuestSessionsByVM/DeleteGuestSession + scanner helpers
- guest.session.* RPC dispatch entries
- 5 CLI session tests, 2 completion tests, 2 printer tests
Extracted:
- ShellQuote + FormatStepError lifted to internal/daemon/workspace/util.go
(only non-session consumer); workspace package now self-contained
- internal/daemon/guest_ssh.go keeps guestSSHClient + dialGuest +
waitForGuestSSH — still used by workspace prepare/export
- internal/daemon/fake_firecracker_test.go preserves the test helper
that used to live in guest_sessions_test.go
Store schema: CREATE TABLE guest_sessions and its column migrations
removed. Existing dev DBs keep an orphan table (harmless, pre-v0.1.0).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CLI: introduce internal/cli.deps which owns every RPC/SSH/host-command
seam the tree used to reach through mutable package vars. Command
builders, orchestrators, and the completion helpers become methods on
*deps. Tests construct their own deps per case, so fakes no longer leak
across cases and tests are free to run in parallel.
Daemon: move workspaceInspectRepoFunc + workspaceImportFunc onto the
Daemon struct (workspaceInspectRepo / workspaceImport), mirroring the
existing guestWaitForSSH / guestDial pattern. Workspace-prepare tests
drop t.Parallel() guards now that they no longer mutate process-wide
state.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pure code motion — banger.go 3508→240 LOC, same-package
decomposition keeps all identifiers visible without export changes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
