ssh: flush any queued output at teardown

[ejohnstown]

wolfSSH_shutdown() flushes whatever a short send left in the output buffer, not just a queued disconnect. A rejected auth's USERAUTH_FAILURE has no channel, and a channel close is retired the moment it is bundled, so neither had anything left to carry the retry.

• The gate still refuses a flush once the peer has disconnected, unless our own disconnect is the thing queued.
• Tests cover the rejected auth with no channel, the close whose channel was already retired, and a flush that short-sends on a live channel.

[Copilot]

Pull request overview

Updates wolfSSH_shutdown() teardown behavior so it will flush any already-queued outbound bytes left behind by a short send (not only a queued local disconnect), while still preventing post-disconnect traffic unless the only queued output is the local disconnect.

Changes:

• Generalize the shutdown-time “flush queued disconnect” gate into a “flush queued output” gate, with special-casing to suppress non-disconnect output after the peer has disconnected.
• Extend regression coverage for queued output flush scenarios: rejected auth with no channel, channel-close queued after channel retirement, and a live-channel short-send during shutdown.
• Document the broader shutdown flush behavior in the public header.

Reviewed changes

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

File	Description
src/ssh.c	Generalizes queued-output flushing at teardown and in the post-disconnect send path.
wolfssh/ssh.h	Expands wolfSSH_shutdown() API comment to describe the broader flush semantics and retry behavior.
tests/regress.c	Adds regression tests covering newly supported queued-output flush scenarios and short-send retry behavior.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1219

Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 1

Required changes (1)

Completed flush on a live session leaves a stale WS_WANT_WRITE latched

File: src/ssh.c:1207
Function: wolfSSH_shutdown
Category: Incorrect error handling

The stale-error clear is gated on ssh->disconnected. On the paths FlushQueuedOutput() newly enables — a live session with no channel, e.g. a CHANNEL_CLOSE whose channel DoChannelClose() already retired — the flush empties the output buffer while ssh->error stays WS_WANT_WRITE, so wolfSSH_get_error() reports a write owed on an empty buffer.

Related known finding #8817 (similar but distinct): Both are in wolfSSH_shutdown and affect channel-close/shutdown handling, but #8817 faults in the ChannelFind ID-type lookup while this candidate faults in the post-flush error reset condition. Their root causes and affected operations differ, and correcting either requires a separate patch.

Recommendation: Clear the latched WS_WANT_WRITE whenever the flush completed, not only when ssh->disconnected is set.

Referenced code: src/ssh.c:1207-1208 (2 lines)

---

This review was generated automatically by Fenrir. Reported findings require changes before merge.

[ejohnstown]

Fixed: a flush that completes on a live session now clears the WS_WANT_WRITE the short send latched, as the disconnected path already did with WS_DISCONNECT. Regression test added.

The cited path does not reproduce, though. On the CHANNEL_CLOSE whose channel DoChannelClose() retired, ssh->error after the flush is WS_CHANNEL_CLOSED; on the rejected auth it is WS_USER_AUTH_E. DoReceive() overwrites the latched want-write before shutdown runs in both. The reachable path is a short send with no channel involved -- wolfSSH_SendIgnore(), wolfSSH_global_request() -- where channelList == NULL skips both the teardown sends and the wolfSSH_worker() call that would otherwise reset the error. That is what the test uses.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1219

Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 1

Required changes (1)

Stale flushRet overrides a teardown that completed the queued write

File: src/ssh.c:1262
Function: wolfSSH_shutdown
Category: Incorrect error handling

flushRet was only reachable as non-success when the session was disconnected, which nulled channel and skipped every later send. The widened flush now fires on live sessions, where SendChannelEof/SendChannelExit/SendChannelClose each flush the whole output buffer and can drain the flush's leftovers; the unconditional override then returns WS_WANT_WRITE with an empty buffer, masking the real result (e.g. WS_SOCKET_ERROR_E from the close send).

Related known finding #8817 (similar but distinct): Both affect wolfSSH_shutdown teardown behavior, but #8817 faults in channel lookup using a peer ID as a self ID, causing teardown sends to be skipped. This candidate faults in the later unconditional flushRet override after teardown sends may drain output. The root causes and operations differ, and each needs a separate patch.

Recommendation: Gate the override on the flush still being owed, e.g. if (flushRet != WS_SUCCESS && wolfSSH_OutputPending(ssh)), mirroring the flushed guard used for ssh->error.

Referenced code: src/ssh.c:1262-1263 (2 lines)

---

This review was generated automatically by Fenrir. Reported findings require changes before merge.

[ejohnstown]

Fixed: the flush's WS_WANT_WRITE now only outranks the teardown result while the output buffer still holds bytes, so a teardown send that carried the leftovers out settles it. Regression test added, with only the data send and the flush short.

The gate is narrower than the suggested wolfSSH_OutputPending() check on its own: a flush that fails outright still reports. WS_CBIO_ERR_GENERAL frees the output buffer, so a bare pending check would turn that failure on the disconnect path into WS_CHANNEL_CLOSED.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1219

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1219

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1219

Scan targets checked: wolfssh-bugs, wolfssh-src

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.

Fenrir's latest completed scan found no issues; clearing the prior automated change request.