Skip to content

🐛 Exit cleanly when SSH access is disabled#1051

Open
agners wants to merge 1 commit into
hassio-addons:mainfrom
agners:disable-sshd-service-entirly
Open

🐛 Exit cleanly when SSH access is disabled#1051
agners wants to merge 1 commit into
hassio-addons:mainfrom
agners:disable-sshd-service-entirly

Conversation

@agners
Copy link
Copy Markdown

@agners agners commented May 18, 2026

Proposed Changes

When the SSH port is not exposed, the sshd service ran a sleep 864000 placeholder. On graceful shutdown that placeholder was killed by SIGTERM, causing the finish script to set the container exit code to 143.

Disable the sshd service entirely via an S6_STAGE2_HOOK script that removes it from the user bundle when no SSH port is configured, matching the pattern used by some official add-ons. The container now exits with 0 on a graceful shutdown.

Related Issues

home-assistant/supervisor#6840

Summary by CodeRabbit

  • Bug Fixes

    • SSH daemon now reliably starts on container initialization.
  • New Features

    • Added automatic detection to disable SSH daemon when port 22 is not configured in addon settings.
  • Chores

    • Updated container environment configuration.

Review Change Stack

When the SSH port is not exposed, the sshd service ran a `sleep 864000`
placeholder. On graceful shutdown that placeholder was killed by SIGTERM,
causing the finish script to set the container exit code to 143.

Disable the sshd service entirely via an `S6_STAGE2_HOOK` script that
removes it from the user bundle when no SSH port is configured, matching
the pattern used by some official add-ons. The container now exits with
0 on a graceful shutdown.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a5a80e81-5113-479d-82c6-09f522cc02fe

📥 Commits

Reviewing files that changed from the base of the PR and between e46333e and 0fa3488.

📒 Files selected for processing (3)
  • ssh/Dockerfile
  • ssh/rootfs/etc/s6-overlay/s6-rc.d/sshd/run
  • ssh/rootfs/etc/s6-overlay/scripts/enable-check.sh
💤 Files with no reviewable changes (1)
  • ssh/rootfs/etc/s6-overlay/s6-rc.d/sshd/run

Walkthrough

This pull request refactors SSH daemon conditional startup by moving disable logic from the sshd/run script to a new S6 stage2 hook. A Dockerfile environment variable now points to enable-check.sh, which runs during container init to conditionally prevent SSH daemon startup if port 22 is not configured. The sshd/run script is simplified because the hook handles the disable.

Changes

SSH daemon conditional startup via S6 stage2 hook

Layer / File(s) Summary
S6 stage2 hook script and wiring
ssh/rootfs/etc/s6-overlay/scripts/enable-check.sh, ssh/Dockerfile
New enable-check.sh script uses bashio to detect if port 22 is configured; if absent, it removes the SSH daemon from the s6-rc service list. Dockerfile sets S6_STAGE2_HOOK environment variable to invoke this script during container stage2 initialization.
Simplified SSH daemon startup
ssh/rootfs/etc/s6-overlay/s6-rc.d/sshd/run
The sshd/run script removes the conditional guard that checked port 22 availability. SSH daemon startup is now unconditional because the S6 hook prevents service invocation when port 22 is unconfigured.

🎯 2 (Simple) | ⏱️ ~12 minutes

🐰 A daemon once guarded its port with care,

But now a hook handles the affair,

With S6's stage2 in the container's dawn,

SSH wakes only when port's turned on! 🚀

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: preventing unclean exits when SSH access is disabled by introducing a proper service disabling mechanism instead of using a sleep placeholder.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

1 participant