Repository navigation
wolfsshd: drain stderr before ending the session - #1314
ejohnstown wants to merge 3 commits into
Conversation
The shell loop waits on the child's stderr the way it already waits on its stdout. A child that exits with error output still in the pipe had the rest relayed only by the single read below the loop -- one bufferful, however much was waiting -- so the peer saw a truncated error stream and no error. A pty session has no stderr of its own.
A child that exits with its error output still in the pipe must have all of it relayed. The child grows its stderr pipe and queues the whole payload before exiting, so the result does not depend on the pipe size or on which cat is installed. - forty passes, counted in whole lines - a client failure is reported as one, not as a truncation - skips without perl - the external-host tally counts 13 local-only tests
There was a problem hiding this comment.
🟢 Approval recommended
The implementation correctly handles PTY and non-PTY sessions and includes focused regression coverage.
0 open findings
What changed in this PR
Ensures wolfsshd fully relays child stderr before closing the session.
Changes:
- Waits for stderr EOF alongside stdout.
- Adds and registers a regression test for stderr truncation.
| File | Description |
|---|---|
apps/wolfsshd/wolfsshd.c |
Drains stderr before session shutdown. |
apps/wolfsshd/test/sshd_stderr_drain_test.sh |
Tests complete stderr delivery. |
apps/wolfsshd/test/run_all_sshd_tests.sh |
Registers the new test. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1314
Scan targets checked: wolfssh-src, wolfssh-bugs
Coverage: 1 of 1 in-scope changed file(s) opened by the reviewer
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Lite
A pty session has no stderr pipe, so the shell loop must start with stderrEmpty set. The drain test now runs "exit 3" in a pty shell and expects the session to end with that status rather than wait for a stderr EOF that never comes.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1314
Scan targets checked: none
Unchanged since last review (not re-run): wolfssh-src, wolfssh-bugs
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Lite
A child that exits with error output still in its stderr pipe had only one bufferful of it relayed, by the drain after the shell loop, so the peer saw a truncated error stream and no error. The loop now waits for stderr EOF the way it waits for stdout.
sshd_stderr_drain_test.shhas the child queue its whole payload in a grown pipe and checks every line arrives