feat(api): add programmatic file copy (VM.CopyTo/CopyFrom) over SSH#14
Conversation
|
Warning Review limit reached
More reviews will be available in 45 minutes and 59 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughIntroduces ChangesProgrammatic file copy via tar-over-SSH
Sequence Diagram(s)sequenceDiagram
participant caller as Caller (VM / CLI cp)
participant orch as orchestrator.VM
participant sshkey as sshkey.Client
participant pipe as io.Pipe
participant guest as Guest (tar -x / tar -c)
rect rgba(34, 139, 34, 0.5)
Note over caller,guest: CopyTo (host → guest)
caller->>orch: CopyTo(ctx, hostPath, guestPath)
orch->>sshkey: Dial SSH (30s timeout)
sshkey->>pipe: open pipe writer/reader
sshkey->>sshkey: buildTar(hostPath) → pipe writer (goroutine)
sshkey->>guest: toGuest: pipe reader → tar -x stdin
guest-->>sshkey: exit code + stderr
sshkey-->>orch: error or nil
orch-->>caller: error or nil
end
rect rgba(70, 130, 180, 0.5)
Note over caller,guest: CopyFrom (guest → host)
caller->>orch: CopyFrom(ctx, guestPath, hostPath)
orch->>sshkey: Dial SSH (30s timeout)
sshkey->>guest: fromGuest: run tar -c (goroutine)
guest-->>pipe: stdout tar stream
sshkey->>sshkey: extractTar(pipe reader, hostPath)
sshkey-->>orch: error or nil
orch-->>caller: error or nil
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/fleetbox/cp.go`:
- Around line 121-123: The code does not guard against the case where guestPath
is `/` (the remote root), which causes path.Base(guestPath) to return `/` and
subsequently resolves localDst to an unsafe target of `/`. Add a guard check
after the guestPath assignment (in the line where guestPath := remotePath(src)
is called) to detect and handle the case where guestPath equals `/` before
proceeding to the resolveLocalDest call. This prevents the unsafe and surprising
behavior of attempting to copy to the root directory.
In `@internal/sshkey/copy.go`:
- Around line 334-351: The writeRegular function and the related directory
creation function lack symlink safety checks, allowing malicious tars to escape
the destination root by following existing symlinks in the path. Before creating
directories with os.MkdirAll and before creating files with os.OpenFile in
writeRegular, add verification that path components are not symlinks by using
os.Lstat to check each path segment and rejecting any symlinks encountered.
Apply the same symlink detection logic to the directory creation function that
handles directory entries (the function referenced in the "Also applies to"
note), ensuring that no symlink traversal can occur when materializing any entry
type (regular files, directories, or symlinks).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 82f22cbc-3bf7-4554-b80a-7adf5d0467d9
📒 Files selected for processing (13)
ARCHITECTURE.mdREADME.mdcluster_cap_test.gocmd/fleetbox/cp.gocmd/fleetbox/cp_test.gocmd/fleetbox/ssh.godocs/adr/0026-programmatic-copy-tar-over-ssh.mdfleetbox.gofleetboxtest/conformance_vm_test.gofleetboxtest/nested_test.gointernal/orchestrator/orchestrator.gointernal/sshkey/copy.gointernal/sshkey/copy_test.go
pilat
force-pushed
the
feat/programmatic-copy
branch
from
d0955f5 to
b4b1a13
Compare
1 participant
fleetbox is library-first — every capability is supposed to live in the Go API, with the CLI just wrapping it. File copy broke that rule: it only existed as
fleetbox cpshelling out to the systemscpbinary, so a library caller had no way to move a file in or out of a running VM, and the project's own dogfood test had to shell out to scp and hand-rebuild the key path just to get its test binary into the guest.WithFixtureonly ever covered read-only host→guest at boot — there was no copy-out, and no copy-in to a running VM.This adds
VM.CopyToandVM.CopyFromto the public API — universal: a file or a whole directory, in either direction. They streamtarover the SSH connection fleetbox already holds, so there are no new module dependencies and the only thing the guest needs istar, which every image in the catalog ships. File modes are preserved (an executable stays executable); ownership is not. The CLIcpand the dogfood now ride the same primitive, so file copy no longer shells out toscpat all — the CLI keeps its scp-like "copy into a directory" convenience, but only as a thin layer above the exact-path library method.It's purely client-side — the control protocol, the helper, and the backends are untouched — so it behaves identically on macOS and Linux. The exact-destination semantics, the rejected alternatives (sftp, keeping the scp shell-out), and the deliberately deferred bits (reader/writer variants, honoring ctx mid-transfer, tolerating GNU tar's "file changed as we read it") are written up in ADR-0026.
Checklist
ARCHITECTURE.mdupdated in this PRdocs/adr/(next sequential number)!in the title) → the description spells out what callers must changeSummary by CodeRabbit
Release Notes
New Features
cpcommand with an improved file transfer mechanismDocumentation