workspace: drop --readonly flag — advisory only against root guests

--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>
This commit is contained in:
Thales Maciel 2026-04-23 13:04:33 -03:00
parent bafe816fc7
commit 235758e5b2
No known key found for this signature in database
GPG key ID: 33112E6833C34679
6 changed files with 6 additions and 54 deletions

View file

@ -143,7 +143,6 @@ type VMWorkspacePrepareParams struct {
Branch string `json:"branch,omitempty"` Branch string `json:"branch,omitempty"`
From string `json:"from,omitempty"` From string `json:"from,omitempty"`
Mode string `json:"mode,omitempty"` Mode string `json:"mode,omitempty"`
ReadOnly bool `json:"readonly,omitempty"`
IncludeUntracked bool `json:"include_untracked,omitempty"` IncludeUntracked bool `json:"include_untracked,omitempty"`
} }

View file

@ -583,7 +583,6 @@ func (d *deps) newVMWorkspacePrepareCommand() *cobra.Command {
var branchName string var branchName string
var fromRef string var fromRef string
var mode string var mode string
var readOnly bool
var includeUntracked bool var includeUntracked bool
var dryRun bool var dryRun bool
cmd := &cobra.Command{ cmd := &cobra.Command{
@ -594,7 +593,7 @@ func (d *deps) newVMWorkspacePrepareCommand() *cobra.Command {
ValidArgsFunction: d.completeVMNameOnlyAtPos0, ValidArgsFunction: d.completeVMNameOnlyAtPos0,
Example: strings.TrimSpace(` Example: strings.TrimSpace(`
banger vm workspace prepare devbox banger vm workspace prepare devbox
banger vm workspace prepare devbox ../repo --guest-path /root/repo --readonly banger vm workspace prepare devbox ../repo --guest-path /root/repo
banger vm workspace prepare devbox ../repo --mode full_copy banger vm workspace prepare devbox ../repo --mode full_copy
`), `),
RunE: func(cmd *cobra.Command, args []string) error { RunE: func(cmd *cobra.Command, args []string) error {
@ -634,7 +633,6 @@ func (d *deps) newVMWorkspacePrepareCommand() *cobra.Command {
Branch: branchName, Branch: branchName,
From: prepareFrom, From: prepareFrom,
Mode: mode, Mode: mode,
ReadOnly: readOnly,
IncludeUntracked: includeUntracked, IncludeUntracked: includeUntracked,
}) })
if err != nil { if err != nil {
@ -647,7 +645,6 @@ func (d *deps) newVMWorkspacePrepareCommand() *cobra.Command {
cmd.Flags().StringVar(&branchName, "branch", "", "create and switch to a new guest branch") cmd.Flags().StringVar(&branchName, "branch", "", "create and switch to a new guest branch")
cmd.Flags().StringVar(&fromRef, "from", "HEAD", "base ref for --branch") cmd.Flags().StringVar(&fromRef, "from", "HEAD", "base ref for --branch")
cmd.Flags().StringVar(&mode, "mode", string(model.WorkspacePrepareModeShallowOverlay), "workspace mode: shallow_overlay, full_copy, metadata_only") cmd.Flags().StringVar(&mode, "mode", string(model.WorkspacePrepareModeShallowOverlay), "workspace mode: shallow_overlay, full_copy, metadata_only")
cmd.Flags().BoolVar(&readOnly, "readonly", false, "make the prepared workspace read-only")
cmd.Flags().BoolVar(&includeUntracked, "include-untracked", false, "also copy untracked non-ignored files into the guest workspace (default: tracked files only)") cmd.Flags().BoolVar(&includeUntracked, "include-untracked", false, "also copy untracked non-ignored files into the guest workspace (default: tracked files only)")
cmd.Flags().BoolVar(&dryRun, "dry-run", false, "list the files that would be copied and exit without touching the guest") cmd.Flags().BoolVar(&dryRun, "dry-run", false, "list the files that would be copied and exit without touching the guest")
return cmd return cmd

View file

@ -1,7 +1,6 @@
package daemon package daemon
import ( import (
"bytes"
"context" "context"
"errors" "errors"
"fmt" "fmt"
@ -182,13 +181,13 @@ func (s *WorkspaceService) PrepareVMWorkspace(ctx context.Context, params api.VM
unlock := s.workspaceLocks.lock(vm.ID) unlock := s.workspaceLocks.lock(vm.ID)
defer unlock() defer unlock()
return s.prepareVMWorkspaceGuestIO(ctx, vm, strings.TrimSpace(params.SourcePath), guestPath, branchName, fromRef, mode, params.ReadOnly, params.IncludeUntracked) return s.prepareVMWorkspaceGuestIO(ctx, vm, strings.TrimSpace(params.SourcePath), guestPath, branchName, fromRef, mode, params.IncludeUntracked)
} }
// prepareVMWorkspaceGuestIO performs the actual guest-side work: // prepareVMWorkspaceGuestIO performs the actual guest-side work:
// inspect the local repo, dial SSH, stream the tar, optionally chmod // inspect the local repo, dial SSH, stream the tar. Called without
// readonly. It is called without holding the VM mutex. // holding the VM mutex.
func (s *WorkspaceService) prepareVMWorkspaceGuestIO(ctx context.Context, vm model.VMRecord, sourcePath, guestPath, branchName, fromRef string, mode model.WorkspacePrepareMode, readOnly, includeUntracked bool) (model.WorkspacePrepareResult, error) { func (s *WorkspaceService) prepareVMWorkspaceGuestIO(ctx context.Context, vm model.VMRecord, sourcePath, guestPath, branchName, fromRef string, mode model.WorkspacePrepareMode, includeUntracked bool) (model.WorkspacePrepareResult, error) {
spec, err := s.workspaceInspectRepoHook(ctx, sourcePath, branchName, fromRef, includeUntracked) spec, err := s.workspaceInspectRepoHook(ctx, sourcePath, branchName, fromRef, includeUntracked)
if err != nil { if err != nil {
return model.WorkspacePrepareResult{}, err return model.WorkspacePrepareResult{}, err
@ -208,13 +207,6 @@ func (s *WorkspaceService) prepareVMWorkspaceGuestIO(ctx context.Context, vm mod
if err := s.workspaceImportHook(ctx, client, spec, guestPath, mode); err != nil { if err := s.workspaceImportHook(ctx, client, spec, guestPath, mode); err != nil {
return model.WorkspacePrepareResult{}, err return model.WorkspacePrepareResult{}, err
} }
if readOnly {
var chmodLog bytes.Buffer
chmodScript := fmt.Sprintf("set -euo pipefail\nchmod -R a-w %s\n", ws.ShellQuote(guestPath))
if err := client.RunScript(ctx, chmodScript, &chmodLog); err != nil {
return model.WorkspacePrepareResult{}, ws.FormatStepError("set workspace readonly", err, chmodLog.String())
}
}
return model.WorkspacePrepareResult{ return model.WorkspacePrepareResult{
VMID: vm.ID, VMID: vm.ID,
SourcePath: spec.SourcePath, SourcePath: spec.SourcePath,
@ -222,7 +214,6 @@ func (s *WorkspaceService) prepareVMWorkspaceGuestIO(ctx context.Context, vm mod
RepoName: spec.RepoName, RepoName: spec.RepoName,
GuestPath: guestPath, GuestPath: guestPath,
Mode: mode, Mode: mode,
ReadOnly: readOnly,
HeadCommit: spec.HeadCommit, HeadCommit: spec.HeadCommit,
CurrentBranch: spec.CurrentBranch, CurrentBranch: spec.CurrentBranch,
BranchName: spec.BranchName, BranchName: spec.BranchName,

View file

@ -555,7 +555,7 @@ func TestExportVMWorkspace_DoesNotMutateRealIndex(t *testing.T) {
apiSock := filepath.Join(t.TempDir(), "fc.sock") apiSock := filepath.Join(t.TempDir(), "fc.sock")
firecracker := startFakeFirecracker(t, apiSock) firecracker := startFakeFirecracker(t, apiSock)
vm := testVM("exportbox-readonly", "image-export", "172.16.0.107") vm := testVM("exportbox-noindex-mutation", "image-export", "172.16.0.107")
vm.State = model.VMStateRunning vm.State = model.VMStateRunning
vm.Runtime.State = model.VMStateRunning vm.Runtime.State = model.VMStateRunning
vm.Runtime.APISockPath = apiSock vm.Runtime.APISockPath = apiSock

View file

@ -172,7 +172,6 @@ type WorkspacePrepareResult struct {
RepoName string `json:"repo_name"` RepoName string `json:"repo_name"`
GuestPath string `json:"guest_path"` GuestPath string `json:"guest_path"`
Mode WorkspacePrepareMode `json:"mode"` Mode WorkspacePrepareMode `json:"mode"`
ReadOnly bool `json:"readonly"`
HeadCommit string `json:"head_commit,omitempty"` HeadCommit string `json:"head_commit,omitempty"`
CurrentBranch string `json:"current_branch,omitempty"` CurrentBranch string `json:"current_branch,omitempty"`
BranchName string `json:"branch_name,omitempty"` BranchName string `json:"branch_name,omitempty"`

View file

@ -392,40 +392,6 @@ grep -q 'sshd' <<<"$ports_out" \
# shellcheck disable=SC2064 # shellcheck disable=SC2064
trap "rm -rf '$runtime_dir'" EXIT trap "rm -rf '$runtime_dir'" EXIT
# --- workspace prepare --readonly -------------------------------------
# --readonly runs `chmod -R a-w` over the workspace. Root in the
# guest bypasses DAC anyway, so this is advisory rather than
# enforced — the point of the flag is tooling contract: "the
# mode bits SAY readonly". Assert that contract: the write bit
# must be cleared on the guest file after --readonly prepare, and
# set without it. A regression where the chmod silently no-op'd
# would leave the bits unchanged.
log 'workspace prepare --readonly: mode bits reflect the request'
# shellcheck disable=SC2064
trap "\"$BANGER\" vm delete smoke-ro >/dev/null 2>&1 || true; rm -rf '$runtime_dir'" EXIT
"$BANGER" vm create --name smoke-ro >/dev/null || die 'workspace ro: create failed'
"$BANGER" vm workspace prepare smoke-ro "$repodir" --readonly >/dev/null \
|| die 'workspace ro: prepare --readonly failed'
# stat octal mode. a-w clears the 0222 write bits across u/g/o, so
# none of the write bits should be set on the file.
ro_mode="$("$BANGER" vm ssh smoke-ro -- stat -c '%a' /root/repo/smoke-file.txt | tr -d '[:space:]')"
[[ -n "$ro_mode" ]] || die 'workspace ro: could not read mode bits'
case "$ro_mode" in
*[2367])
die "workspace ro: file still has write bit set after --readonly (mode=$ro_mode)"
;;
esac
# Read must still succeed (--readonly means a-w, not a-r).
"$BANGER" vm ssh smoke-ro -- cat /root/repo/smoke-file.txt >/dev/null \
|| die 'workspace ro: read denied — --readonly dropped read perm too'
"$BANGER" vm delete smoke-ro >/dev/null || die 'workspace ro: delete failed'
# shellcheck disable=SC2064
trap "rm -rf '$runtime_dir'" EXIT
# --- workspace prepare --mode full_copy ------------------------------- # --- workspace prepare --mode full_copy -------------------------------
# Default mode is shallow_overlay. full_copy copies the repo via a # Default mode is shallow_overlay. full_copy copies the repo via a
# different transfer path (tar stream into the guest's rootfs with # different transfer path (tar stream into the guest's rootfs with