[CFX-6228][CFX-6229] sec: validate server-controlled paths in sync engine

[wojtekwdr]

RATIONALE

Two linked [SEC] P1 findings from the sync-engine security review (CFX-6228, CFX-6229):

• internal/workload/sync/download.go::downloadOne passes FileAction.Path (server-controlled, sourced from the AllFiles response) directly into filepath.Join(e.projectDir, filepath.FromSlash(fa.Path)) and then os.Create. A compromised server returning ../../../etc/cron.d/evil writes outside the project directory.
• internal/workload/sync/phase5_execute.go::removeLocalDeletedFiles does the same with os.Remove. A path of ../../../home/user/.ssh/id_rsa deletes that file.

filesapi.AllFiles already validates manifest entries at the HTTP boundary, but paths flow through SyncPlan before reaching disk, so a per-call-site guard is required as defense in depth. P2-4 (Rollback.Backup) provides a third layer at the rollback path and is tracked separately.

CHANGES

• internal/workload/sync/download.go - call fileops.SafeRelPath(fa.Path) at the top of downloadOne, before filepath.Join; return a wrapped "server returned unsafe download path %q: %w" error on rejection.
• internal/workload/sync/phase5_execute.go - call fileops.SafeRelPath(fa.Path) inside the removeLocalDeletedFiles loop, after the ActDownloadDelete filter and before filepath.Join; return a wrapped "server returned unsafe delete path %q: %w" error on rejection.
• internal/workload/sync/path_safety_test.go (new) - shared unsafeServerPaths slice covering five bypass classes (../escape, ../../etc/passwd, /etc/passwd, ..\windows, empty), table-driven tests for both call sites, a sentinel-file test that pins the no-escape-from-project-dir contract by writing a sentinel in t.TempDir()'s parent and asserting it survives, and a negative test confirming non-ActDownloadDelete entries are skipped without validation (uploads carry locally-scanned paths that would over-reject).

The wrapping style and identifier choice (fileops.SafeRelPath) match existing guards at cmd/workload/code/checkout/checkout.go:290 and internal/drapi/filesapi/allfiles.go:43,73.

TESTING

• task lint - 0 issues.
• task test - new tests pass under -race, each under 10ms; full sync package passes. (Full-repo task test reports one unrelated pre-existing failure in cmd.TestWorkloadCommandNotPresentByDefault driven by the local .env's DATAROBOT_CLI_FEATURE_WORKLOAD=true; with that env var unset the entire suite passes.)
• Sentinel test verifies that a malicious ../ path does not result in os.Remove reaching outside the project directory.

---

Note

Low Risk
Tightens path handling at two filesystem call sites with existing SafeRelPath helper; behavior change is rejecting malicious paths that should never have been applied.

Overview
Adds defense-in-depth validation so server-controlled FileAction.Path values cannot escape the project directory during sync.

Before writing or removing files, downloadOne and removeLocalDeletedFiles now call fileops.SafeRelPath and fail with clear errors (server returned unsafe download/delete path). Validation runs only for ActDownloadDelete deletes so upload-side delete entries (locally scanned paths) are not over-rejected.

New path_safety_test.go table-tests traversal/absolute/empty/backslash paths at both sites, uses a parent-dir sentinel to prove deletes never run outside the project dir, and asserts non-download-delete plan entries are skipped.

^{Reviewed by Cursor Bugbot for commit b7a07bd. Configure here.}

[Copilot]

Pull request overview

Adds defense-in-depth validation for server-controlled sync paths before local filesystem write/delete operations.

Changes:

• Validates download paths in downloadOne.
• Validates server-side delete paths in removeLocalDeletedFiles.
• Adds path safety regression tests for unsafe path classes and delete behavior.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.

File	Description
internal/workload/sync/download.go	Adds SafeRelPath validation before creating downloaded files.
internal/workload/sync/phase5_execute.go	Adds SafeRelPath validation before removing remote-deleted local files.
internal/workload/sync/path_safety_test.go	Adds tests for unsafe server paths and delete containment behavior.
go.sum	Updates module checksum metadata.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.