Port wolfSSH Zephyr test sample to Zephyr 4.4.0#1065
Conversation
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1065
Scan targets checked: wolfssh-bugs, wolfssh-src
No new issues found in the changed files. ✅
aidangarske
left a comment
There was a problem hiding this comment.
Skoll Multi-Scan Review
Modes: review + review-security + bugsOverall recommendation: REQUEST_CHANGES
Findings: 4 total — 4 posted, 0 skipped
4 finding(s) posted as inline comments (see file-level comments below)
Posted findings
- [High] [bugs] Unsupported job container entrypoint option breaks Zephyr CI —
.github/workflows/zephyr.yml:24-26 - [Low] [review] Version matrix lacks fail-fast: false, so one Zephyr leg failing cancels the other —
.github/workflows/zephyr.yml:13-22 - [Low] [review] RAM-disk selection keys off a substring match of the CONF_FILE filename —
zephyr/samples/tests/CMakeLists.txt:24 - [Low] [review] New config/overlay files open with multi-line description blocks contrary to repo comment convention —
zephyr/samples/tests/posix_legacy.conf:1-10
Review generated by Skoll
Keyboard-interactive was unusable in builds with WOLFSSH_KEYBOARD_INTERACTIVE but no WOLFSSH_TERM, in two ways. - ClientFreeBuffers declared 'entry' under TERM && KBI but used it under KBI alone, so KBI-without-TERM failed to compile; widen the declaration guard to match the use - ClientUserAuth's keyboard-interactive branch is guarded on TERM && KBI, so without TERM a keyboard-interactive request fell past every branch and returned the initial WOLFSSH_USERAUTH_SUCCESS with no responses populated. There is no terminal to prompt on, so answer WOLFSSH_USERAUTH_FAILURE instead of claiming success.
Zephyr 3.5 moved the RAM disk driver from Kconfig to devicetree and 4.1 replaced CONFIG_NET_SOCKETS_POSIX_NAMES with CONFIG_POSIX_API. Port the sample to the new APIs while keeping it buildable on older Zephyr. - add ramdisk.overlay (zephyr,ram-disk node) for the FS scenarios; drop CONFIG_DISK_RAM_VOLUME_SIZE - switch to CONFIG_POSIX_API; include the POSIX time/select/socket/netdb headers in test.h and port.h, gated on ZEPHYR_VERSION_CODE >= 4.1.0 so older Zephyr keeps the BSD names from <zephyr/net/socket.h> - select RAM disk (overlay >= 3.5, else CONFIG_DISK_RAM_VOLUME_SIZE) and POSIX (CONFIG_POSIX_API >= 4.1, else the legacy symbols) in CMakeLists.txt, with ramdisk_legacy.conf / posix_legacy.conf fallbacks - tests, nofs and kbi scenarios pass on qemu_x86 with Zephyr 3.4.0 (SDK 0.16.1) and 4.4.0 (SDK 1.0.0)
- add a v4.4.0 matrix leg alongside v3.4.0 - run each leg in the Zephyr CI image with the matching SDK (ci:v0.26.4/SDK 0.16.1, ci:v0.29.0/SDK 1.0.0); drops the manual deps, west, pip and SDK install steps - scope twister by --testsuite-root and drop --test: the scenario id is path-prefixed on 3.4.0 but bare on 4.x - replace zip with tar caf logs.tar.xz for the failure-log artifact
There was a problem hiding this comment.
Pull request overview
Updates the wolfSSH Zephyr test sample and CI to support Zephyr 4.x (tested against 4.4.0) while retaining compatibility with Zephyr 3.4.0 by version-gating configuration and header usage.
Changes:
- Add Zephyr-version-aware CMake gating to select RAM disk configuration (devicetree overlay vs Kconfig) and POSIX compatibility symbols.
- Update sample prj*.conf files to use
CONFIG_POSIX_API=y(with a legacy override for pre-4.1), and move RAM disk sizing to devicetree for Zephyr ≥ 3.5. - Modernize the Zephyr GitHub Actions workflow to use official Zephyr CI container images and run all sample scenarios via
twister --testsuite-root.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| zephyr/samples/tests/CMakeLists.txt | Detects Zephyr version early and conditionally applies overlay/legacy .conf files for cross-version compatibility. |
| zephyr/samples/tests/prj.conf | Switches to CONFIG_POSIX_API and relies on devicetree RAM disk sizing for newer Zephyr. |
| zephyr/samples/tests/prj_nofs.conf | Switches to CONFIG_POSIX_API for the no-filesystem scenario. |
| zephyr/samples/tests/prj_kbi.conf | Switches to CONFIG_POSIX_API and relies on devicetree RAM disk sizing for the keyboard-interactive scenario. |
| zephyr/samples/tests/posix_legacy.conf | Restores pre-4.1 POSIX-related Kconfig symbols when building against older Zephyr. |
| zephyr/samples/tests/ramdisk.overlay | Defines a Zephyr devicetree RAM disk node for SFTP filesystem scenarios on Zephyr ≥ 3.5. |
| zephyr/samples/tests/ramdisk_legacy.conf | Provides pre-3.5 Kconfig-based RAM disk sizing equivalent to the devicetree overlay. |
| wolfssh/test.h | Adds Zephyr 4.1+ conditional POSIX header includes to replace removed socket-name exports. |
| wolfssh/port.h | Adds Zephyr 4.1+ conditional include for struct timeval and silences unused-parameter warnings in the Zephyr WUTIMES stub. |
| examples/client/common.c | Makes keyboard-interactive auth fail cleanly when built without terminal support; fixes buffer-free code to compile without WOLFSSH_TERM. |
| .github/workflows/zephyr.yml | Runs CI in Zephyr Project CI container images for 3.4.0 and 4.4.0; archives per-matrix failure logs and runs all scenarios via --testsuite-root. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1065
Scan targets checked: wolfssh-bugs, wolfssh-src
No new issues found in the changed files. ✅
Brings the Zephyr test sample and its CI up to date with Zephyr 4.x APIs while keeping it buildable on the older 3.4.0 release.