fix(ftp): end the data wait on a refusal the control reader already holds

[axpnet]

What this is

Pre-release audit finding A-04 (lane A, marked PLAUSIBLE, no fixture run by
the auditor): the data-channel watch in read_watching_control peeks the
bare TCP socket UNDERNEATH suppaftp's BufReader. When a server sends the
preliminary 150 and the final 4xx/5xx reply in ONE segment, the read that
collects the 150 pulls the whole segment into the reader's buffer. The
socket then shows nothing, poll_peek returns 0, and the transfer waits out
the full DATA_IDLE_TIMEOUT (1800 s) for an answer that is already inside
the process.

Reproduction first

The first step was not the fix, it was making the defect happen. New test
a_refusal_absorbed_by_the_reader_ends_the_wait:

• the server writes 150 ...\r\n550 ...\r\n in a single write (one segment,
  no flush between, which is precisely the condition of the finding; two
  separate writes would have proved nothing);
• the BufReader is modelled by hand: one read with a large buffer, which is
  what the reader did when it collected the 150;
• two assertions guard the validity of the setup itself: the refusal bytes
  ARE inside the modelled reader buffer, and the socket is empty
  (WouldBlock);
• the data socket is held open and silent, so the loopback shortcut measured
  at the head of read_watching_control (the data socket closing at once
  and ending the wait by another road) cannot hide the stall.

On the pinned base this test FAILED with a 5 s timeout: the wait never
ended, exactly as predicted. Red seen before the fix.

The fix

The observation point was wrong, not the case: looking at the socket under
the BufReader cannot establish that no reply is already queued. suppaftp
10.0.2 does not expose the reader's buffered bytes (the field is private;
only get_ref() is public), so:

• axpdev-lab/suppaftp, branch buffered-reply-bytes off tag v10.0.2,
  adds exactly one method, buffered_reply_bytes() (self.reader.buffer()),
  consuming nothing. git diff v10.0.2 on the fork shows only that method.
  Pinned by rev in [patch.crates-io], same shape as the existing
  n0-mainline patch, with a written exit condition (drop the patch once an
  upstream release exposes the buffered bytes; upstream PR to be filed from
  the same branch).
• read_watching_control now also takes the buffered bytes. In Quiet
  state a buffered reply is classified from its first byte, same rule as the
  socket peek (4/5 ends the wait, anything else shortens the deadline
  and leaves the bytes for the finalizer). In Spoke state an expired
  deadline counts as a refusal when the socket OR the buffer holds bytes.
• Side effect worth having: the reader's buffer holds plaintext even under
  FTPS, so the 4xx/5xx classification now works where the wire bytes would
  be TLS records.

The refusal itself is still consumed by read_queued_refusal, which reads
through the BufReader and therefore sees the buffered reply. The reply is
never eaten by the watch.

Verified

• The new test fails on the pinned base (5 s timeout, reason: the wait did
  not end) and passes with the fix.
• The four pre-existing watch tests still pass, including
  a_completion_reply_is_left_alone_and_the_data_keeps_flowing with its
  "peek did not eat the reply" assertion.
• cargo fmt --all -- --check, clippy and the full pre-push gate: see
  below, updated before merge if anything moved.

NOT verified, and which way each limit leans

• Real-server end-to-end: the repro is an in-memory socket pair, not the
  Docker fixture. On loopback the real flow closes the data socket at once
  and the wait ends by another road in about two seconds, so a Docker
  fixture could not have shown the stall anyway (measured constraint, noted
  in the code at the head of read_watching_control). The limit leans
  toward the defect being rarer in practice on loopback, unchanged on real
  networks.
• The sibling case named in after_timed_out_open (550 then 421 in one
  segment, session kept with the 421 queued) is NOT fixed here: it is
  tracked separately in the code, and the keep/drop session decision it
  feeds is its own change. The new accessor makes it solvable; the comment
  there now says so.
• The suppaftp fork patch is tokio-only, which is the only flavour this
  crate builds.

---

Added during the recovery

The gate was run WITH the patch active, which is the thing the unit test cannot cover: a forked dependency can break builds the test never touches.

• cargo test --lib -- providers::ftp::: 29 passed, 0 failed, 1 ignored.
• cargo clippy --all-targets -- -D warnings: clean.
• cargo fmt --all -- --check: clean.
• Cargo.lock already carries the git source and the pinned rev, so the patch resolves in CI and not only on one disk. Verified before it was asked for.
• The fork's rev is reachable from the remote (axpdev-lab/suppaftp, branch buffered-reply-bytes, fork of veeso/suppaftp), so [patch.crates-io] resolves for anybody, not just here.

The exit condition was rewritten, because it was inert. It said "an upstream PR is to be filed from the same branch": a future tense with no subject and no link, which six months from now reads like something that was done. It now states the three things a later reader can check: who has to send it (@axpnet), the status in plain words (NOT OPENED as of 2026-09-01), and where the URL goes when it exists.

A fork does not become permanent because the exit condition is missing. It becomes permanent because the exit condition is written in a form nobody can verify.

The upstream PR is prepared and deliberately NOT sent. Filing a PR on a third-party repository under the account owner's name is an outward-facing action that belongs to the owner, not to whoever is holding the keyboard. Branch, diff and text are ready and the send is one command; it is waiting on a decision, and the Cargo.toml says so out loud.

Was the fork avoidable

Checked against the crate source rather than assumed. In suppaftp 10.0.2, ImplAsyncFtpStream keeps reader: BufReader<DataStream<T>> private, and the only accessor is get_ref(), which returns the TcpStream underneath the buffer. From outside the crate those bytes are not observable at all.

The two alternatives, and why they lose:

• Consume instead of observe, reading the control channel when the data channel stalls. Blocked by a constraint already written in this file at the head of read_watching_control: read_response_in is not cancellation-safe, and a timeout around it drops it mid-line and misaligns the control channel. That replaces a control channel we ignore with one we corrupt.
• Own the control channel ourselves, parsing replies without suppaftp's reader. It closes everything and it is a rewrite of the FTP reply layer on the evening of a tag, against seventeen lines that can be audited with git diff v10.0.2.

So the fork is the honest minimum here, and it has a house precedent in the n0-mainline patch directly above it.

---

Merge record

Merged at zero incomplete check runs, read from the check-runs API on the head commit rather than from this page, where a superseded cancelled run reads as red.

No review bot read this branch: CodeRabbit was rate limited across the whole range 5ea980f1..0345f759, verified across all three surfaces (issue comments, submitted reviews, inline comments on this head) rather than only the first, which is how two Major findings on a sibling pull request were nearly missed tonight.

The work is Kimi K3's, including the part that matters most: a test that fails on the pinned base FOR THE PREDICTED REASON, with the 150 and the 550 leaving in one write, the refusal asserted to be inside the reader's buffer and the socket asserted to be empty, and the data channel held open and silent. The chosen vehicle is a direct test on the watch with an in-memory socket pair, not a Docker fixture: the file itself records that on loopback the data socket closes at once and the wait ends by itself, so a fixture there could have shown nothing for a reason unrelated to the defect and the silence would have read as "does not reproduce".

This ships a forked dependency, and that is a decision rather than a detail. suppaftp keeps its reader private and get_ref() returns the socket UNDER the buffer, so the bytes already absorbed are not observable from outside the crate, verified in the registry source rather than taken on trust. The fork adds one method and nothing else, auditable with git diff v10.0.2. The two alternatives were rejected for reasons already written in this file: consuming instead of observing puts a non-cancellation-safe read inside a select!, which replaces a control channel we ignore with one we corrupt, and owning the response layer ourselves is a rewrite on the eve of a tag.

The exit condition in Cargo.toml names WHO must file the upstream pull request, states that it is not open as of 2026-09-01, and says where the URL goes when it exists. It is prepared and deliberately not sent: opening a pull request against a third-party repository in the owner's name is the owner's call.

[coderabbitai]

Warning

Review limit reached

Next included review available in 9 minutes.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 71339065-6587-4f3a-ab2e-30f77e9f4080

📥 Commits

Reviewing files that changed from the base of the PR and between 5ea980f and 0345f75.

⛔ Files ignored due to path filters (1)

• src-tauri/Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (2)

• src-tauri/Cargo.toml
• src-tauri/src/providers/ftp.rs

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[snyk-io]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.