Skip to content

fix(flow): resolve sender path and inflight staging - #138

Open
robjarawan wants to merge 1 commit into
developmentfrom
fix/sender-dry-run-local-dir
Open

fix(flow): resolve sender path and inflight staging#138
robjarawan wants to merge 1 commit into
developmentfrom
fix/sender-dry-run-local-dir

Conversation

@robjarawan

@robjarawan robjarawan commented Sep 5, 2026

Copy link
Copy Markdown
Owner

When running sender with dry_run enabled, Flow.send skipped changing directory to local_dir. Relative path existence checks against local_file failed when the daemon working directory differed from local_dir, preventing test executions from completing successfully. Furthermore, staging directories were prepared during fileOp events, partitioned block transfers triggered invalid renames of non-existent temporary paths, short final blocks were rejected during inplace transfers, and non-string inflight values caused unhandled exceptions.

Fixes: MetPX#1772
Upstream PR: MetPX#1773
Fork Issue: #137

What this adds

Flow.send resolves local_path using os.path.realpath and verifies that os.path.commonpath([base_dir, local_path]) == base_dir. Any attempted path traversal outside base_dir logs an error and returns -1.

Flow.send preserves the original process working directory file descriptor prior to directory modification and restores it in a finally: block across live and dry run modes.

Inflight options enforce string validation. Directory staging paths starting with / or ending in / take precedence over file suffixes. Absolute staging paths disable accelerated transfer and are rejected on object storage backends. Staging directory creation and rename capability checks are deferred to the payload transfer section so fileOp events (remove, rename, directory, link) bypass staging operations.

Partitioned block transfers bypass staging and temporary rename steps. Inplace block transfers extract offsets and lengths from msg['blocks'] manifests, take precedence over inflight is None, and use msg['size'] as the expected written length for short final blocks. In sarracenia/transfer/sftp.py, file truncation during partial block writes is removed so out-of-order block assembly succeeds.

Written byte counts returned by transfer backends are validated against expected_length before executing renames or writing success report codes.

Blast radius and rollback

This change modifies transfer staging, directory switching, and write validation logic in sarracenia/flow/__init__.py and sarracenia/transfer/sftp.py.

Destructive operations:

  • Staging directory preparation creates remote directories using cd_forced when configured with inflight directory paths.
  • Temporary files are renamed to final destination paths upon payload transfer completion.
  • Files and directories are removed during remove fileOp events.

If transfer validation fails, files remain in temporary staging paths without replacing existing target files. Rollback is accomplished by reverting the commit on the branch. Reviewers should focus on the staging directory preparation placement in sarracenia/flow/__init__.py lines 2735-2755 and block transfer precedence in lines 2820-2850.

How to verify

Execute the flow test suite:

python3 -m pytest tests/sarracenia/flow/

Verify formatting and line length:

git diff origin/development..HEAD | pycodestyle --diff --max-line-length=119

Evidence

Platform results
============================= test session starts ==============================
platform linux -- Python 3.10.12, pytest-9.0.2, pluggy-1.6.0
rootdir: /net/local/home/jarawanr/src/sarracenia/tests
configfile: pytest.ini
plugins: mock-3.15.1, depends-1.0.1, anyio-4.13.0
collected 13 items

tests/sarracenia/flow/__flow___test.py ....                              [ 30%]
tests/sarracenia/flow/callback_dispatch_test.py ........                 [ 92%]
tests/sarracenia/flow/metrics_leak_test.py .                             [100%]

======================== 13 passed, 2 warnings in 1.65s ========================

@robjarawan
robjarawan force-pushed the fix/sender-dry-run-local-dir branch from b6211bb to 14014db Compare September 5, 2026 02:35
@robjarawan robjarawan added the bug Something isn't working label Sep 5, 2026
@robjarawan robjarawan self-assigned this Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

Test Results

376 tests   375 ✅  1m 46s ⏱️
  1 suites    1 💤
  1 files      0 ❌

Results for commit 84f3f23.

♻️ This comment has been updated with latest results.

@robjarawan
robjarawan force-pushed the fix/sender-dry-run-local-dir branch from 14014db to 43486ba Compare September 5, 2026 02:53
@robjarawan robjarawan changed the title Fix sender dry_run path resolution and unbound len_written fix(flow): resolve sender local path in dry run Sep 5, 2026
@robjarawan
robjarawan force-pushed the fix/sender-dry-run-local-dir branch 3 times, most recently from 8a65097 to 476da71 Compare September 5, 2026 03:58
@robjarawan robjarawan changed the title fix(flow): resolve sender local path in dry run fix(flow): resolve sender path and inflight staging Sep 5, 2026
@robjarawan
robjarawan force-pushed the fix/sender-dry-run-local-dir branch from 476da71 to 0497976 Compare September 5, 2026 14:52
@robjarawan robjarawan closed this Sep 5, 2026
@robjarawan robjarawan reopened this Sep 5, 2026
robjarawan pushed a commit that referenced this pull request Sep 5, 2026
When running sender with dry_run enabled, Flow.send skipped changing
to local_dir, causing relative path checks against local_file to fail
when the working directory differed from local_dir. Furthermore,
inflight modes allowed directory mutations during fileOp events,
performed renames on partitioned block transfers, mishandled short
final inplace blocks, permitted path traversal, and failed to restore
the caller working directory.

Resolve local_path with realpath and enforce containment under baseDir.
Defer inflight staging and rename checks to payload transfers. Support
absolute and subfolder staging paths while disabling accelerated
transfers for absolute paths. Ensure block transfers bypass temporary
rename, prioritize inplace block offsets over inflight=None, accept
short final blocks, validate write byte counts, and restore working
directory.

Fixes: MetPX#1772
Refs: #138
@robjarawan
robjarawan force-pushed the fix/sender-dry-run-local-dir branch from 0497976 to 3d95cd9 Compare September 5, 2026 19:10
When running sender with dry_run enabled, Flow.send skipped changing
to local_dir, causing relative path checks against local_file to fail
when the working directory differed from local_dir. Furthermore,
inflight modes allowed directory mutations during fileOp events,
performed renames on partitioned block transfers, mishandled short
final inplace blocks, permitted path traversal, and failed to restore
the caller working directory.

Resolve local_path with realpath and enforce containment under baseDir.
Defer inflight staging and rename checks to payload transfers. Support
absolute and subfolder staging paths while disabling accelerated
transfers for absolute paths. Ensure block transfers bypass temporary
rename, prioritize inplace block offsets over inflight=None, accept
short final blocks, validate write byte counts, and restore working
directory.

Fixes: MetPX#1772
Refs: #138
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flow.send dry_run fails on files in local_dir and unbound len_written

1 participant