file_sync: skip nested symlinks during recursive copy
A user who sets `[[file_sync]] host = "~/.aws"` (per the README's own example) can unintentionally copy files from outside that directory if .aws contains symlinks. copyHostDir used os.Stat during recursion, which transparently follows: a symlink to a credential dir elsewhere would be recursed into, materialising unrelated secrets inside the guest. For credential trees that's an avoidable sprawl vector. Switched copyHostDir's per-entry probe from os.Stat to os.Lstat and added a default skip-with-warning branch for ModeSymlink. Files and dirs at the SAME level copy as before; symlinks (both file and directory flavours) surface a "file_sync skipped symlink (would escape the requested tree)" warn log and are otherwise omitted. Top-level entry paths still follow — the Stat in runFileSync is unchanged. The user explicitly named that path, so resolving "~/.aws" through a symlink out of $HOME is on them. Tests: - TestRunFileSyncSkipsNestedSymlinks — builds a synced dir with both a file symlink and a directory symlink pointing outside the tree; asserts real files copy, symlinks do not materialise anywhere in the guest mount, and each skipped symlink surfaces a warn log entry. README updated with a one-line note about the skip behaviour so users know to expect it rather than chasing "why didn't my file show up." Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
caa6a2b996
commit
1850904d9c
3 changed files with 127 additions and 8 deletions
|
|
@ -201,7 +201,10 @@ mode = "0755" # optional; default 0600 for files
|
|||
Runs at `vm create` time. Each entry copies `host` → `guest` onto
|
||||
the VM's work disk (mounted at `/root` in the guest). Guest paths
|
||||
must live under `~/` or `/root/...`. Default is no entries — add the
|
||||
ones you want.
|
||||
ones you want. Symlinks encountered while recursing into a synced
|
||||
directory are skipped with a warning — they'd otherwise leak files
|
||||
from outside the named tree (e.g. a symlink inside `~/.aws` pointing
|
||||
to an unrelated credential dir).
|
||||
|
||||
## Advanced
|
||||
|
||||
|
|
|
|||
|
|
@ -220,7 +220,7 @@ func (s *WorkspaceService) runFileSync(ctx context.Context, vm *model.VMRecord)
|
|||
}
|
||||
|
||||
if info.IsDir() {
|
||||
if err := copyHostDir(ctx, runner, hostPath, target); err != nil {
|
||||
if err := s.copyHostDir(ctx, *vm, runner, hostPath, target); err != nil {
|
||||
return fmt.Errorf("file_sync: copy directory %s → %s: %w", hostPath, target, err)
|
||||
}
|
||||
continue
|
||||
|
|
@ -240,10 +240,14 @@ func (s *WorkspaceService) runFileSync(ctx context.Context, vm *model.VMRecord)
|
|||
// copyHostDir recursively copies hostDir into guestTarget using only
|
||||
// `mkdir` (for subdirs) and `install` (for files). Each file's source
|
||||
// permissions are preserved; ownership is forced to root:root via
|
||||
// `install -o 0 -g 0`. Symlinks are followed (target content is
|
||||
// copied as a regular file). Other special types (devices, FIFOs)
|
||||
// are skipped silently.
|
||||
func copyHostDir(ctx context.Context, runner system.CommandRunner, hostDir, guestTarget string) error {
|
||||
// `install -o 0 -g 0`. Symlinks encountered during recursion are
|
||||
// SKIPPED with a warning — `os.Lstat` tells us the entry itself is a
|
||||
// link without resolving it, so a symlink inside ~/.aws that points
|
||||
// at ~/secrets can't leak out of the tree the user named. Other
|
||||
// special types (devices, FIFOs) are skipped silently. Top-level
|
||||
// host paths go through `os.Stat` back in runFileSync and still
|
||||
// follow, since the user explicitly named that path.
|
||||
func (s *WorkspaceService) copyHostDir(ctx context.Context, vm model.VMRecord, runner system.CommandRunner, hostDir, guestTarget string) error {
|
||||
if _, err := runner.RunSudo(ctx, "mkdir", "-p", guestTarget); err != nil {
|
||||
return err
|
||||
}
|
||||
|
|
@ -255,13 +259,15 @@ func copyHostDir(ctx context.Context, runner system.CommandRunner, hostDir, gues
|
|||
hostChild := filepath.Join(hostDir, entry.Name())
|
||||
guestChild := filepath.Join(guestTarget, entry.Name())
|
||||
|
||||
info, err := os.Stat(hostChild)
|
||||
info, err := os.Lstat(hostChild)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
switch {
|
||||
case info.Mode()&os.ModeSymlink != 0:
|
||||
s.warnFileSyncSymlinkSkipped(vm, hostChild)
|
||||
case info.IsDir():
|
||||
if err := copyHostDir(ctx, runner, hostChild, guestChild); err != nil {
|
||||
if err := s.copyHostDir(ctx, vm, runner, hostChild, guestChild); err != nil {
|
||||
return err
|
||||
}
|
||||
case info.Mode().IsRegular():
|
||||
|
|
@ -372,6 +378,17 @@ func (s *WorkspaceService) warnFileSyncSkipped(vm model.VMRecord, hostPath strin
|
|||
s.logger.Warn("file_sync skipped", append(vmLogAttrs(vm), "host_path", hostPath, "error", err.Error())...)
|
||||
}
|
||||
|
||||
// warnFileSyncSymlinkSkipped surfaces a skipped nested symlink to the
|
||||
// user through the daemon log. Skipping is deliberate — see
|
||||
// copyHostDir's docstring — but invisible skips would hide a
|
||||
// "why did my file not show up in the guest?" debugging trail.
|
||||
func (s *WorkspaceService) warnFileSyncSymlinkSkipped(vm model.VMRecord, hostPath string) {
|
||||
if s.logger == nil {
|
||||
return
|
||||
}
|
||||
s.logger.Warn("file_sync skipped symlink (would escape the requested tree)", append(vmLogAttrs(vm), "host_path", hostPath)...)
|
||||
}
|
||||
|
||||
func (s *WorkspaceService) warnGitIdentitySyncSkipped(vm model.VMRecord, source string, err error) {
|
||||
if s.logger == nil || err == nil {
|
||||
return
|
||||
|
|
|
|||
|
|
@ -1177,6 +1177,105 @@ func TestRunFileSyncCopiesDirectoryRecursively(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// TestRunFileSyncSkipsNestedSymlinks pins the anti-sprawl contract:
|
||||
// a symlink INSIDE a synced directory is not followed, even if the
|
||||
// target holds real files. Without this, a user syncing ~/.aws with
|
||||
// a ~/.aws/session -> ~/other-creds symlink would copy the unrelated
|
||||
// creds into the guest. Top-level entries (the path the user
|
||||
// literally named) still follow, because they explicitly asked for
|
||||
// that path.
|
||||
func TestRunFileSyncSkipsNestedSymlinks(t *testing.T) {
|
||||
homeDir := t.TempDir()
|
||||
t.Setenv("HOME", homeDir)
|
||||
|
||||
// Target the user DID NOT name — lives outside the synced tree.
|
||||
outsideDir := filepath.Join(homeDir, "other-creds")
|
||||
if err := os.MkdirAll(outsideDir, 0o700); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(outsideDir, "leaked.txt"), []byte("must-not-escape"), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// The synced directory.
|
||||
srcDir := filepath.Join(homeDir, ".aws")
|
||||
if err := os.MkdirAll(srcDir, 0o700); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(srcDir, "credentials"), []byte("access"), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// File symlink inside .aws pointing OUT of the tree.
|
||||
if err := os.Symlink(filepath.Join(outsideDir, "leaked.txt"), filepath.Join(srcDir, "session")); err != nil {
|
||||
t.Skipf("symlink unsupported on this filesystem: %v", err)
|
||||
}
|
||||
// Directory symlink inside .aws pointing OUT of the tree — must
|
||||
// not be recursed into.
|
||||
if err := os.Symlink(outsideDir, filepath.Join(srcDir, "linked-dir")); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
var buf bytes.Buffer
|
||||
logger, _, err := newDaemonLogger(&buf, "info")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
workDisk := t.TempDir()
|
||||
d := &Daemon{
|
||||
runner: &filesystemRunner{t: t},
|
||||
logger: logger,
|
||||
config: model.DaemonConfig{
|
||||
FileSync: []model.FileSyncEntry{
|
||||
{Host: "~/.aws", Guest: "~/.aws"},
|
||||
},
|
||||
},
|
||||
}
|
||||
wireServices(d)
|
||||
vm := testVM("sync-symlink", "image", "172.16.0.76")
|
||||
vm.Runtime.WorkDiskPath = workDisk
|
||||
if err := d.ws.runFileSync(context.Background(), &vm); err != nil {
|
||||
t.Fatalf("runFileSync: %v", err)
|
||||
}
|
||||
|
||||
// The real file inside the tree must copy.
|
||||
creds, err := os.ReadFile(filepath.Join(workDisk, ".aws", "credentials"))
|
||||
if err != nil {
|
||||
t.Fatalf("credentials not copied: %v", err)
|
||||
}
|
||||
if string(creds) != "access" {
|
||||
t.Fatalf("credentials = %q, want access", creds)
|
||||
}
|
||||
|
||||
// Neither the file symlink nor anything reached through the
|
||||
// directory symlink should have been materialised in the guest
|
||||
// path.
|
||||
for _, shouldNotExist := range []string{
|
||||
filepath.Join(workDisk, ".aws", "session"),
|
||||
filepath.Join(workDisk, ".aws", "linked-dir"),
|
||||
filepath.Join(workDisk, ".aws", "linked-dir", "leaked.txt"),
|
||||
} {
|
||||
if _, err := os.Stat(shouldNotExist); !os.IsNotExist(err) {
|
||||
t.Fatalf("symlinked path %s was materialised in guest tree (stat err = %v); secret leakage path open", shouldNotExist, err)
|
||||
}
|
||||
}
|
||||
|
||||
// Each skipped symlink must be warned.
|
||||
entries := parseLogEntries(t, buf.Bytes())
|
||||
for _, want := range []string{
|
||||
filepath.Join(srcDir, "session"),
|
||||
filepath.Join(srcDir, "linked-dir"),
|
||||
} {
|
||||
if !hasLogEntry(entries, map[string]string{
|
||||
"msg": "file_sync skipped symlink (would escape the requested tree)",
|
||||
"vm_name": vm.Name,
|
||||
"host_path": want,
|
||||
}) {
|
||||
t.Fatalf("expected warn log for skipped symlink %s; got %v", want, entries)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestCreateVMRejectsNonPositiveCPUAndMemory(t *testing.T) {
|
||||
d := &Daemon{}
|
||||
wireServices(d)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue