Skip to content

Fix sshd test flakiness and orphan daemon#1024

Open
ejohnstown wants to merge 1 commit into
wolfSSL:masterfrom
ejohnstown:tmux
Open

Fix sshd test flakiness and orphan daemon#1024
ejohnstown wants to merge 1 commit into
wolfSSL:masterfrom
ejohnstown:tmux

Conversation

@ejohnstown

Copy link
Copy Markdown
Contributor
  • term size test: poll tmux output with retries instead of fixed sleeps
  • run_all_sshd_tests.sh: validate --match before starting wolfSSHd

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #1024

No scan targets match the changed files in this PR. Review skipped.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the wolfSSHd test harness to reduce CI flakiness and prevent leaving orphaned wolfSSHd/tmux resources when a test selection is invalid or a test times out.

Changes:

  • sshd_term_size_test.sh: Replace fixed sleeps with polling of tmux pane output; add tmux-session teardown via trap to avoid stale “duplicate session” failures.
  • run_all_sshd_tests.sh: Validate --match before starting a local wolfSSHd to avoid orphan daemons when a test name is invalid.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
apps/wolfsshd/test/sshd_term_size_test.sh Adds polling helpers + EXIT trap to reduce tmux/terminal-size test flakiness.
apps/wolfsshd/test/run_all_sshd_tests.sh Moves --match validation ahead of wolfSSHd startup to prevent orphan processes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/wolfsshd/test/sshd_term_size_test.sh

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #1024

No scan targets match the changed files in this PR. Review skipped.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 9 changed files in this pull request and generated 1 comment.

Comment thread keys/renewcerts.cnf Outdated
- term size test: poll tmux output with retries instead of fixed sleeps,
  re-sending the size query each pass in case the shell was not yet ready
- term close test: scope netstat checks to the test port so unrelated
  host CLOSE_WAIT/TIME_WAIT sockets don't fail the test
- run_all_sshd_tests.sh: validate --match before starting wolfSSHd
- renewcerts.sh: build the per-user cert from a throwaway config copy so
  the tracked renewcerts.cnf is never modified in place

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #1024

No scan targets match the changed files in this PR. Review skipped.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread keys/renewcerts.sh
Comment on lines 11 to +15
USER_NAME=$1
cp fred-key.der $USER_NAME-key.der
cp fred-key.pem $USER_NAME-key.pem
sed -i.bak "s/fred/$USER_NAME/g" renewcerts.cnf
CONFIG="renewcerts-$USER_NAME.cnf"
sed "s/fred/$USER_NAME/g" renewcerts.cnf > "$CONFIG"
Comment thread keys/renewcerts.sh
Comment on lines +23 to 26
openssl req -subj "/C=US/ST=WA/L=Seattle/O=wolfSSL Inc/OU=Development/CN=$USER_NAME/emailAddress=$USER_NAME@example.com" -key $USER_NAME-key.pem -out $USER_NAME-cert.csr -config "$CONFIG" -new -nodes

openssl x509 -req -in $USER_NAME-cert.csr -days 3650 -extfile renewcerts.cnf -extensions v3_$USER_NAME -CA ca-cert-ecc.pem -CAkey ca-key-ecc.pem -text -out $USER_NAME-cert.pem -set_serial 7
openssl x509 -req -in $USER_NAME-cert.csr -days 3650 -extfile "$CONFIG" -extensions v3_$USER_NAME -CA ca-cert-ecc.pem -CAkey ca-key-ecc.pem -text -out $USER_NAME-cert.pem -set_serial 7
openssl x509 -in $USER_NAME-cert.pem -outform DER -out $USER_NAME-cert.der
Comment on lines +64 to +73
# validate the requested test before any setup so a bad name does not leave
# a wolfSSHd running
if [[ -n "$MATCH" ]] && [[ ! " ${test_cases[*]} " =~ " $MATCH " ]]; then
echo "Error: Test '$MATCH' not found."
echo "All test cases:"
for test in "${test_cases[@]}"; do
echo " $test"
done
exit 1
fi
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.

3 participants