Skip to content

feat(groq): A bash utility that safely backs up and regenerates an SSH host key with a timestamped backup.#5138

Open
polsala wants to merge 1 commit into
mainfrom
ai/groq-20260708-0632
Open

feat(groq): A bash utility that safely backs up and regenerates an SSH host key with a timestamped backup.#5138
polsala wants to merge 1 commit into
mainfrom
ai/groq-20260708-0632

Conversation

@polsala

@polsala polsala commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Implementation Summary

  • Utility: nightly-ssh-key-rotator
  • Provider: groq
  • Location: bash-utils/nightly-nightly-ssh-key-rotator-5
  • Files Created: 3
  • Description: A bash utility that safely backs up and regenerates an SSH host key with a timestamped backup.

Rationale

  • Automated proposal from the Groq generator delivering a fresh community utility.
  • This utility was generated using the groq AI provider.

Why safe to merge

  • Utility is isolated to bash-utils/nightly-nightly-ssh-key-rotator-5.
  • README + tests ship together (see folder contents).
  • No secrets or credentials touched.
  • All changes are additive and self-contained.

Test Plan

  • Follow the instructions in the generated README at bash-utils/nightly-nightly-ssh-key-rotator-5/README.md
  • Run tests located in bash-utils/nightly-nightly-ssh-key-rotator-5/tests/

Links

  • Generated docs and examples committed alongside this change.

Mock Justification

  • Not applicable; generator did not introduce new mocks.

@polsala

polsala commented Jul 8, 2026

Copy link
Copy Markdown
Owner Author

🤖 Review by GROQ Agent

✅ What's solid

  • Self‑contained utility – All files live under bash-utils/nightly‑nightly‑ssh‑key‑rotator‑5; no external dependencies are introduced.
  • Robust Bash optionsset -euo pipefail protects against unset variables and pipeline failures.
  • Clear usage contract – The script validates the argument count and prints a concise usage message.
  • Deterministic backup naming – Timestamped backup_<YYYYMMDDHHMMSS> directories make it easy to locate previous keys.
  • Read‑only test suite – Tests run entirely offline and do not touch the real host key files.
  • README – Provides a quick‑start example, high‑level description, and a pointer to the test suite.

🧪 Tests

Observation Recommendation
The test suite sources the script and then aliases ssh-keygen to a mock function. Bash does not expand aliases in non‑interactive shells unless shopt -s expand_aliases is set. Add shopt -s expand_aliases at the top of the test file or, more reliably, replace the alias with a function override:

bash\n# Mock implementation\nssh-keygen() { mock_ssh_keygen "$@"; }\n
No cleanup of the temporary directory (mktemp -d). Add a trap to remove the workspace after the test finishes, e.g.

bash\ntrap 'rm -rf \"$TMPDIR\"' EXIT\n
Tests cover the happy‑path (existing private + public key). Add edge‑case tests:
• When no key exists (script should still generate a new pair).
• When only the public key exists.
• When the backup directory already exists (e.g., run twice within the same second).
• When ssh-keygen returns a non‑zero exit code (ensure the script propagates the failure).
Assertions use if [[ "$(cat …)" != "…" ]] which is fine, but the error messages could be more diagnostic. Use printf with the expected vs actual values to aid debugging, e.g.

```bash\nexpected='MOCK PRIVATE KEY'\nactual=$(cat "$KEY_PATH")\n[[ "$actual" == "$expected" ]]
Test file is named test_rotate_ssh_keys.sh but lives under tests/. Consider adding a BATS or shunit2 harness for consistency with other bash utilities in the repo, or at least a comment indicating the test runner (bash test_rotate_ssh_keys.sh).

🔒 Security

  • Permission handling – The script assumes the caller has write permission to the key directory. It would be safer to explicitly check that the directory is writable before proceeding:

    bash\n[[ -w \"$KEY_DIR\" ]] || { echo \"Error: $KEY_DIR is not writable\" >&2; exit 2; }\n |
  • Backup collision – Using a per‑second timestamp could theoretically cause a collision if the script is invoked multiple times within the same second. Consider appending a random suffix or using mktemp -d for the backup directory:

    bash\nBACKUP_DIR=$(mktemp -d \"${KEY_DIR}/backup_${TIMESTAMP}_XXXX\")\n |
  • Overwrite protectioncp will silently overwrite files if a backup directory already contains a file with the same name (unlikely but possible with collisions). Using cp -n (no‑clobber) or checking for existence first adds an extra safety net. |
  • No passphrase – Host keys are typically unencrypted, but documenting that the script intentionally generates a key with an empty passphrase helps auditors understand the design decision. |
  • Shebang & execution bits – The script is marked executable (chmod +x is implied by the repo). Ensure the file mode is 0755 in the repository to avoid accidental non‑executable commits.

🧩 Docs/DX

  • README enhancements:
    • Mention that the script must be run as a user with sufficient privileges (usually root).
    • Clarify the expected exit codes (e.g., 0 on success, 1 for usage error, 2 for permission issues).
    • Provide an example of the backup directory layout after rotation.
    • Add a “Prerequisites” section (requires ssh-keygen in $PATH). |
  • Usage message – Currently prints Usage: $0 <ssh-key-path> and exits with status 1. It could also output a short description, e.g., Rotate an SSH host key and keep a timestamped backup. |
  • Help flag – Supporting -h/--help would improve ergonomics:

    bash\nif [[ \"$1\" == -h || \"$1\" == --help ]]; then usage; fi\n |
  • Cross‑platform note – The date +%Y%m%d%H%M%S format works on GNU and BSD date, but adding a comment that the script has been tested on both Linux and macOS would reassure users. |
  • Contribution guidelines – Since the utility is generated by Groq, a brief note on how to regenerate or update it (e.g., run the Groq prompt again) could be useful for future maintainers.

🧱 Mocks/Fakes

  • Mock implementation – The test’s mock_ssh_keygen parses arguments manually; it works but could be simplified using getopts or by delegating to ssh-keygen with -f only. The current parsing skips unknown flags, which is fine, but documenting the supported subset would aid readability. |
  • Exporting the mockexport -f mock_ssh_keygen is unnecessary when you later alias ssh-keygen to it; a plain function override (as suggested above) is clearer and does not rely on the export mechanism. |
  • Isolation – The test creates its own temporary directory, which is good. Ensure that any environment variables that could affect ssh-keygen (e.g., SSH_KEYGEN) are cleared or set to a known state. |
  • Future extensibility – If additional external commands are added (e.g., chmod, chown), consider building a small test harness that can stub multiple binaries via a temporary PATH pointing to a directory of mock scripts. This pattern scales better than per‑test aliasing. |

Overall, the utility is a solid addition: it follows good Bash practices, provides clear documentation, and includes an offline test suite. Addressing the points above—especially the test‑suite robustness and a few safety checks—will make the contribution even more reliable and maintainable.

@polsala

polsala commented Jul 8, 2026

Copy link
Copy Markdown
Owner Author

🤖 Review by GEMINI Agent

What's solid

  • The script implements a clear and straightforward approach to backing up and regenerating SSH host keys.
  • The use of set -euo pipefail is excellent practice for robust shell scripting.
  • The script correctly identifies and handles the directory and base name of the provided key path.
  • The timestamped backup directory naming convention is clear and effective for organization.
  • The script explicitly checks for the existence of both private and public key files before attempting to copy them, preventing errors if only one exists.
  • The ssh-keygen command is used with appropriate flags (-q for quiet, -N "" for no passphrase) for automated generation.
  • The script provides informative output regarding the location of the new key and the backup directory.
  • The safety rationale is well-articulated, emphasizing that existing keys are moved, not overwritten.

🧪 Tests

  • The test suite effectively mocks ssh-keygen using a function alias, which is a good approach for deterministic testing.
  • The tests correctly simulate the creation of an initial key pair.
  • The verification steps for the new key content and the presence of the backup directory are thorough.
  • The tests confirm that the old keys are correctly copied into the timestamped backup directory.
  • Consider adding a test case for when the input key path does not exist initially. This would verify the script's behavior when creating a new key without any prior backup.
  • It would be beneficial to test the script's behavior with different key types (e.g., ssh_host_ecdsa_key, ssh_host_ed25519_key) to ensure path handling is generic.

🔒 Security

  • The script's core functionality of rotating keys is a security enhancement, reducing the risk associated with long-lived keys.
  • The backup mechanism ensures that no data is lost during the rotation process.
  • The script does not handle or expose any secrets or credentials, which is a positive security posture.
  • The use of ssh-keygen -N "" generates keys without a passphrase. While convenient for automation, this means the private key would be accessible if the file itself were compromised. For highly sensitive environments, consider if a passphrase is a necessary security consideration, though this would complicate automation.

🧩 Docs/DX

  • The README provides a clear explanation of the utility's purpose, usage, and what it does.
  • The usage example is concise and easy to understand.
  • The "Safety" section effectively communicates the script's protective measures.
  • The "Testing" section correctly points to the test file.
  • The documentation could be slightly more explicit about the expected permissions required for the script to run successfully (e.g., write permissions to the directory containing the SSH host key).
  • Consider adding a note about the implications of generating keys without a passphrase, as mentioned in the security section.

🧱 Mocks/Fakes

  • The mock for ssh-keygen is well-implemented and correctly simulates the creation of dummy key files.
  • The mock correctly parses the -f argument to determine the output file path.
  • The mock handles -N and -q flags by simply shifting them, which is sufficient for this test's purpose.
  • The test suite correctly uses export -f and alias to ensure the mock is used when rotate_key is called.

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