Skip to content

fix: gate cross-node fetch auth to FlowMesh origin and bound timeouts - #117

Merged
timzsu merged 1 commit into
mainfrom
fix/multi-node-fetch-auth-audit
Jul 28, 2026
Merged

fix: gate cross-node fetch auth to FlowMesh origin and bound timeouts#117
timzsu merged 1 commit into
mainfrom
fix/multi-node-fetch-auth-audit

Conversation

@timzsu

@timzsu timzsu commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Purpose

Follow-up to #115. That PR fixed one cross-worker image fetch; this PR audits the whole worker/shared surface for the same bug class and fixes the rest.

The class: code paths that diverge between single-node co-located deploys and multi-node cross-node deploys. Co-located, an artifact/result/checkpoint resolves to a local filesystem path — the path exercised in every test and demo. Cross-node (producer and consumer on separate nodes, no shared results storage), the same reference falls back to an HTTP fetch of {FLOWMESH_BASE_URL}/api/v1/results/.... That fallback is rarely exercised, so it had rotted in three ways:

  • (a) auth missing to the worker's own FlowMesh origin → 401 under enforced auth.
  • (b) bearer leak — the worker token (FLOWMESH_API_KEY) attached to a non-origin URL a workflow happens to reference, leaking it to an arbitrary host.
  • (c) unbounded/missing timeout — a hung fetch stalls the task (bandit B113).

The invariant this PR enforces everywhere: attach the worker bearer only when the URL is the worker's own FlowMesh origin (is_flowmesh_origin_url); local paths load locally; external URLs fetch unauthenticated with a bounded timeout.

Changes

  • artifacts.py
    • resolve_artifact now sends auth_headers() only to the FlowMesh origin, so an arbitrary image_embedding URL/path can't leak the bearer (fixes b on the VLM embedding hot path).
    • is_flowmesh_origin_url compares scheme + hostname + effective port (default http/https 80/443) instead of raw netloc, so an implicit vs explicit default port is not mistaken for a different origin and auth isn't dropped (fixes a latent a).
  • data.py (image inputs) — the dict {"url": ...} branch now routes only true non-origin http(s) URLs to the unauthenticated loader; local paths and FlowMesh-origin URLs resolve through the auth-aware artifact loader (fixes a local-path regression that would have HTTP-fetched /local/x.png).
  • checkpoints.py — checkpoint download and HTTP destination gate the bearer to the FlowMesh origin (a/b) with explicit timeouts.
  • data_utils.py — prompt-payload JSONL fetch origin-gates the bearer (a/b).
  • ssh_executor.py — the result-bundle wget gets a bounded timeout (-T 300 -t 1) (c).

Test Plan

bash uv sync --all-packages --group ci uv run pytest tests/worker/test_data_mixin_lineage.py tests/worker/test_artifact_utils.py tests/worker/test_data_utils_prompt_payload.py tests/worker/test_checkpoint_utils.py tests/worker/test_checkpoint_utils_destination.py tests/worker/test_ssh_executor_result_mounting.py -q uv run pre-commit run --files src/worker/executors/mixins/data.py src/worker/executors/utils/artifacts.py src/worker/executors/utils/checkpoints.py src/worker/executors/utils/data_utils.py src/worker/executors/ssh_executor.py ​

Test Result

  • Targeted worker suite: 87 passed. ruff / black / isort / mypy / codespell / gitleaks: clean.
  • New anti-leak tests are parametrized so that with FLOWMESH_API_KEY set, an external URL receives no Authorization header while a FlowMesh-origin URL receives Bearer; timeouts asserted on every fetch; is_flowmesh_origin_url default-port equivalence checked both directions.
  • Full tests/worker/ suite not run locally (GPU/model-download heavy) — deferred to CI.

Out of scope (tracked separately)

vllm_executor.py coerces checkpoint.load.type: http to local and, on a node without the checkpoint present, silently initializes from the base-model identifier. That's a missing HTTP-download feature / silent-fallback divergence, not an auth/leak/timeout gap — it needs its own PR (wire up the authenticated checkpoint download, or fail loudly). Not addressed here.


Pre-submission Checklist
  • I have read the contribution guidelines.
  • I have run pre-commit run --all-files and fixed any issues.
  • I have added or updated tests covering my changes (if applicable).
  • I have verified that uv run pytest tests/ passes locally.
  • If I changed shared schemas or proto definitions, I have checked downstream compatibility across Server and Worker.
  • If I changed the SDK or CLI, I have verified the affected packages work (uv sync --all-packages --group ci --frozen).
  • If this is a breaking change, I have prefixed the PR title with [BREAKING] and described migration steps above.
  • I have updated documentation or config examples if user-facing behavior changed.

Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
@timzsu
timzsu marked this pull request as ready for review July 27, 2026 15:50
@timzsu
timzsu requested a review from kaiitunnz as a code owner July 27, 2026 15:50

@kaiitunnz kaiitunnz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM.

@timzsu
timzsu merged commit 1331af5 into main Jul 28, 2026
11 checks passed
@timzsu
timzsu deleted the fix/multi-node-fetch-auth-audit branch July 28, 2026 07:27
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.

2 participants