Skip to content

Preserve file destinations when migrations fail - #263

Merged
Mark Brown (markjbrown) merged 5 commits into
mainfrom
markjbrown-failure-safe-file-outputs
Oct 31, 2026
Merged

Mark Brown (markjbrown) merged 5 commits into
mainfrom
markjbrown-failure-safe-file-outputs

Conversation

@markjbrown

@markjbrown Mark Brown (markjbrown) commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Fixes #251.

  • Stage local overwrite output in a unique sibling .dmt-*.tmp file. Publish only after source enumeration, serialization, compression and stream disposal succeed; clean up staging files on handled failures.
  • Preserve existing destination content and access permissions on failure. Report cleanup and secondary finalization errors without hiding the original error; retain nonzero CLI failure results.
  • Reject symbolic-link/reparse-point overwrite destinations (the explicitly selected user policy) and Unix non-regular files, including FIFOs, sockets and devices. Unix type checks inspect metadata without opening the destination.
  • Preserve direct, nontransactional append behavior and explicitly report partial append risk; compressed append remains unsupported.
  • Document temporary-file lifecycle, interruption limits, metadata, same-path input/output behavior and the distinction from cloud/database sinks.

Copilot review fixes

Both findings were valid and are addressed in 51ee288 and 6b8da22:

  • Unix staging is created atomically with owner-only 0600 permissions, subject to umask, before applying any existing destination mode. New Unix overwrite outputs remain owner-only. Windows destination DACLs are supplied at creation rather than applied afterward. Tests inspect permissions immediately when creation returns, before formatting.
  • A regular-file whitelist uses the runtime's normalized libSystem.Native lstat ABI, avoiding platform-specific libc struct layouts and any open/read of special files. Disposable FIFO/socket tests verify rejection before staging and preservation of the original entry.
  • Packaged smoke tests now overwrite existing output, exercising native resolution on all six runtime targets. Container smoke runs as the host UID/GID so owner-only output is readable on its bind mount; this ownership requirement is documented rather than weakening permissions.

Validation

Current head: 6b8da227453b994f0ec35af1aaa87d8666df8fa5.

  • Added 121 deterministic cases across Common and real JSON/CLI-handler regressions, including early/mid-stream failure, cancellation, write/open/disposal/compression-finalization/publication/cleanup failure, compressed success, creation-time permissions, special files, symlinks, same-path/directory sources and append diagnostics.
  • Local Windows full solution: 1,466 passed / 1,483 total, no failures. Skips: 7 unconfigured Cosmos tests, 1 SQL test, and 9 Unix-only or unavailable Windows symlink-privilege cases. Windows creation-time DACL and locked-replacement tests pass.
  • Current-head Linux build and full suite: passed. Downloaded TRX reports 1,522 passed / 1,533 total, including 50 AzureBlob tests; the 11 skips are 7 unconfigured Cosmos tests, 1 SQL test, 1 empty-current-culture CSV case and 2 Windows-only cases. All Unix creation-mode, preserved-mode, FIFO/socket, inspection-error and symlink cases pass.
  • Current-head package builds and native smoke: passed for Windows/Linux/macOS x64 and ARM64 plus the container smoke test. The CI-built Windows package also passed a local smoke check (.NET 10.0.14, 15 sources/sinks, existing-output JSON replacement).
  • Current-head Cosmos integration: passed for current and vNext emulators and failure guards. Existing vNext exclusions and strict assertions remain unchanged.
  • Current-head CodeQL: passed.
  • Before review hardening, the coordinator independently verified 40 published CLI cases across all compression modes and 12 composed Missing named Azure Blob source can be reported as a successful empty migration #250+Failed migrations can leave incomplete output files and truncate existing destinations #251 cases using the real Azure SDK with a synthetic loopback server. Local CSV/Parquet roundtrips and failure-preservation checks also passed.

No cloud accounts/resources or valuable data were used. This change does not add transactions to append, cloud-storage or database sinks. Existing dependency/analyzer warnings and integration-skip policies are unchanged.

Stage overwrite output beside the destination, finalize before replacement, preserve access permissions, and report cleanup and append failures. Reject symbolic-link overwrite destinations and add deterministic file/CLI regression coverage for failures, cancellation, compression, and successful publication.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Distinguish staged local overwrite output from nontransactional append, cloud, and database sinks, and link the common lifecycle documentation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Critical Unix flaws can expose private migration data through initially permissive staging files and replace FIFO or device destinations with regular output files.

2 open issues
What changed in this PR

Addresses #251 by staging local file-sink overwrites and publishing them only after successful completion, protecting existing destinations from failed migrations.

Changes:

  • Adds staged publication, permission handling, cleanup, and append-failure diagnostics.
  • Adds shared-sink and real JSON/CLI regression coverage for failures, cancellation, compression, and destination preservation.
  • Documents temporary-file lifecycle and the limits of overwrite protection.
File Description
Interfaces/​Cosmos.DataTransfer.Common/​README.md Documents overwrite, permissions, cleanup, and append policies.
Interfaces/​Cosmos.DataTransfer.Common/​FileDataSink.cs Implements staging, publication, validation, and failure handling.
Interfaces/​Cosmos.DataTransfer.Common.UnitTests/​FileDataSinkFailureTests.cs Tests publication, failure preservation, permissions, and cleanup.
Extensions/​Json/​README.md Clarifies local overwrite protection versus nontransactional sinks.
Core/​Cosmos.DataTransfer.Core.UnitTests/​RunCommandFileOutputTests.cs Tests JSON pipeline output preservation and CLI exit codes.
Core/​Cosmos.DataTransfer.Core.UnitTests/​Cosmos.DataTransfer.Core.UnitTests.csproj References the JSON extension for pipeline tests.

Review effort: Balanced

Create Unix staging atomically with owner-only permissions and pass existing Windows DACLs during creation. Inspect Unix file types without opening destinations, reject nonregular entries, and cover creation-time permissions, FIFOs, sockets, and packaged overwrite behavior.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Match the runtime's libSystem.Native import name and exercise owner-only container outputs under the host UID. Document bind-mount ownership without broadening output permissions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@markjbrown
Mark Brown (markjbrown) merged commit 9b5db2c into main Oct 31, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Failed migrations can leave incomplete output files and truncate existing destinations

2 participants