Skip to content

feat(gemini): A bash script to monitor system resource usage (CPU, RAM, Disk) and alert if thresholds are exceeded. - #5827

Open
polsala wants to merge 1 commit into
mainfrom
ai/gemini-20260902-0240
Open

feat(gemini): A bash script to monitor system resource usage (CPU, RAM, Disk) and alert if thresholds are exceeded.#5827
polsala wants to merge 1 commit into
mainfrom
ai/gemini-20260902-0240

Conversation

@polsala

@polsala polsala commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Implementation Summary

  • Utility: nightly-sys-resource-monitor
  • Provider: gemini
  • Location: bash-utils/nightly-nightly-sys-resource-monitor-2
  • Files Created: 3
  • Description: A bash script to monitor system resource usage (CPU, RAM, Disk) and alert if thresholds are exceeded.

Rationale

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

Why safe to merge

  • Utility is isolated to bash-utils/nightly-nightly-sys-resource-monitor-2.
  • 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-sys-resource-monitor-2/README.md
  • Run tests located in bash-utils/nightly-nightly-sys-resource-monitor-2/tests/

Links

  • Generated docs and examples committed alongside this change.

Mock Justification

  • Not applicable; generator did not introduce new mocks.

…M, Disk) and alert if thresholds are exceeded.
@polsala

polsala commented Sep 2, 2026

Copy link
Copy Markdown
Owner Author

🤖 Review by GROQ Agent

✅ What's solid

  • Clear purpose & isolation – The script lives in its own bash-utils/nightly‑nightly‑sys-resource-monitor-2 folder and does not touch any existing code‑base.
  • Configurable thresholds – Using environment variables (CPU_THRESHOLD, RAM_THRESHOLD, DISK_THRESHOLD, DISK_PARTITION) makes the utility flexible without hard‑coding values.
  • Human‑readable output – The script prints a concise header/footer and labels each metric, which is great for ad‑hoc runs or piping into log aggregators.
  • Self‑contained tests – A test harness is provided that mocks the heavy‑weight system commands, allowing CI to run on any runner without needing a stressed machine.

🧪 Tests

Observation Recommendation
Tests are written as a single Bash script that redefines top, free and df via function stubs. • Consider using a dedicated Bash testing framework (e.g., BATS or shunit2) to get proper test isolation, setup/teardown hooks, and clearer reporting.
The run_test helper checks for a substring match on the whole script output. • Use exact line matching or regexes anchored to the alert lines to avoid false positives when the script’s formatting changes.
Stubs are installed globally (stub_command top mock_top) and restored only at the very end of the file. • Reset the stubs after each test (or in a teardown block) to guarantee that a failure in an earlier test does not bleed into later ones.
Example:
bash\nfunction restore_commands() {\n unset -f top free df\n}\n
The test harness relies on bc for floating‑point comparison inside the script, but the test file does not verify that bc is present. • Add a sanity check at the start of the test suite:
```bash\ncommand -v bc >/dev/null
Exit codes are not asserted. • Include a check that the script exits with 0 on success and a non‑zero code when any alert fires (you may need to add an explicit exit in the script).
The script is invoked as ./src/monitor.sh from the test directory. • Use $(dirname "$0")/../src/monitor.sh or set PATH to make the test robust regardless of the working directory.

🔒 Security

  • Environment variable injectionDISK_PARTITION is interpolated directly into df -h "${DISK_PARTITION}". If a malicious value like "/; rm -rf /" were exported, it would be passed as a separate argument to df (which is safe) but could still be problematic if later the value is used in a command without quoting. Mitigation: validate the variable before use:
    bash\nif [[ ! "$DISK_PARTITION" =~ ^/[^[:space:]]*$ ]]; then\n echo \"Invalid DISK_PARTITION: $DISK_PARTITION\" >&2\n exit 1\nfi\n |
  • Use of bc and awk – Both are invoked with data derived from system commands; they are not vulnerable to injection in the current flow, but keep the pipeline simple to avoid accidental code execution. |
  • No privileged operations – The script only reads system stats, so there are no immediate privilege‑escalation concerns. |
  • Shebang & set -euo pipefail – Adding strict mode will prevent the script from silently continuing on errors, which is a small but valuable hardening step:
    bash\n#!/usr/bin/env bash\nset -euo pipefail\n |

🧩 Docs / Developer Experience

  • README improvements

    • Add a Prerequisites section listing required tools (bash, top, free, df, bc).
    • Document exit codes (e.g., 0 = no alerts, 1 = one or more alerts).
    • Provide an example of piping the output to a logger or monitoring system.
    • Clarify the directory layout (src/ vs tests/) and how to run tests from the repository root (bash ./bash-utils/.../tests/test_monitor.sh).
  • Usage ergonomics

    • Offer a one‑liner to run the monitor continuously (e.g., via watch or a simple loop) and explain how to schedule it with cron.
    • Suggest a --help flag that prints the configurable variables and defaults. This can be added with a small case statement:
      bash\nif [[ $1 == "--help" ]]; then\n cat <<EOF\nUsage: $0 [options]\n Environment variables:\n CPU_THRESHOLD (default: 80)\n RAM_THRESHOLD (default: 85)\n DISK_THRESHOLD (default: 90)\n DISK_PARTITION (default: /)\nEOF\n exit 0\nfi\n |

🧱 Mocks / Fakes

  • Current approach – Overriding built‑in commands with Bash functions works but can be fragile, especially when other scripts source the same utilities.
  • Alternative: PATH‑based stubs – Create a temporary directory, place mock executables (top, free, df) there, and prepend it to PATH for the duration of each test. This isolates the mocks from the global shell environment:
    bash\nTMPDIR=$(mktemp -d)\ncat > \"$TMPDIR/top\" <<'EOS'\n#!/usr/bin/env bash\n# mock output\n... \nEOS\nchmod +x \"$TMPDIR/top\"\nexport PATH=\"$TMPDIR:$PATH\"\n# run test\n |
  • Cleanup – Ensure the temporary directory is removed in a trap to avoid littering the filesystem:
    bash\ntrap 'rm -rf \"$TMPDIR\"' EXIT\n |
  • Mock reliability – The current mock_top prints a static CPU idle percentage (94.0 id). If the script’s parsing logic changes (e.g., using mpstat), the mock will need updating. Consider parameterising the mock functions so each test can inject the exact numbers it wants to verify.

Overall impression: the utility is a solid addition that fulfills the PR’s promise. Tightening the test harness, adding a few defensive checks, and polishing the documentation will make the script more robust, easier to adopt, and safer to run in varied environments.

@polsala

polsala commented Sep 2, 2026

Copy link
Copy Markdown
Owner Author

🤖 Review by GROQ Agent

✅ What's solid

  • Isolation & Scope – The utility lives in its own bash-utils/nightly‑nightly‑sys‑resource‑monitor‑2 directory and does not touch any existing code, making it easy to reason about and roll back if needed.
  • Configurable thresholds – Using environment variables (CPU_THRESHOLD, RAM_THRESHOLD, DISK_THRESHOLD, DISK_PARTITION) lets users adapt the script without editing source.
  • Self‑contained test suite – A Bash‑only test harness is provided, keeping the dependency footprint low.
  • Clear output format – The script prints a concise header/footer and human‑readable usage numbers, which is helpful for ad‑hoc runs or cron logs.

🧪 Tests

Observation Recommendation
Inconsistent mock data – The mock_top output reports 94.0% id (idle), which the script translates to 6 % CPU usage, yet the first test expects 95.0%. This mismatch will cause the test to fail. Align the mock output with the expected value, e.g. set idle to 5.0% so the calculated usage is 95.0%, or adjust the expected string in the test.
Floating‑point comparison relies on bc – If bc is not installed on the runner, the script (and tests) will abort. Add a guard at the top of monitor.sh that checks for bc (e.g. `command -v bc >/dev/null
Test cleanup is incomplete – The “restore original commands” block simply expands the stored variable (_original_top) instead of redefining the original binaries. This leaves the stub functions in the environment for any subsequent steps. Replace the cleanup with something like:
bash\nunset -f top free df # remove the stub functions\n
or store the original PATH and restore it.
Test harness uses eval for the script callscript_output=$(./src/monitor.sh) is fine, but the run_test helper receives the output via echo "$script_output" which adds an extra newline and may affect string matching. Pass the raw variable to run_test (e.g. run_test "…" "expected" "$script_output"), or trim whitespace inside run_test.
Missing negative test cases – All tests verify that alerts appear when a threshold is crossed, but there is no test for malformed environment variables (e.g., non‑numeric thresholds) or missing commands. Add a test that sets CPU_THRESHOLD=foo and asserts the script exits with an error message. This improves robustness.
Test naming & documentation – The test file is long (≈180 lines) but lacks section headers or comments describing each case. Insert # ==== Test 1: No alerts ====, etc., to make future maintenance easier.

🔒 Security

  • No external secrets – The script does not read files or network resources, so credential leakage is not a concern.
  • Environment variable surface – Because thresholds are taken from the environment, a malicious actor with write access could set extreme values (e.g., CPU_THRESHOLD=0) to trigger false alerts. This is benign for a monitoring script but worth documenting.
  • Command injection risk – The script directly interpolates $DISK_PARTITION into df -h "${DISK_PARTITION}". If an attacker can control this variable, they could inject additional arguments (e.g., DISK_PARTITION="/; rm -rf /"). Bash will treat the whole string as a single argument, but quoting is already in place. To be extra safe, validate the variable against a whitelist of allowed paths:
    bash\nif [[ ! "$DISK_PARTITION" = /* ]]; then echo "Invalid partition"; exit 1; fi\n |
  • Dependency on bc – If bc is replaced by a malicious binary earlier in $PATH, the script could be subverted. Adding command -v bc >/dev/null as a sanity check mitigates this.

🧩 Docs / Developer Experience

  • README path mismatch – The README refers to bash-utils/nightly-sys-resource-monitor while the actual folder is bash-utils/nightly-nightly-sys-resource-monitor-2. Update the documentation to match the real path to avoid confusion.
  • Usage instructions – The “Clone the repository” step is unnecessary for an internal utility; a simple cd bash-utils/nightly-nightly-sys-resource-monitor-2 && ./src/monitor.sh suffices. Consider adding a one‑liner for cron usage, e.g.:
    bash\n0 * * * * /path/to/monitor.sh >> /var/log/sys-monitor.log 2>&1\n |
  • Threshold defaults – Document the default values in a table for quick reference.
  • Exit codes – The script always exits with status 0, even when alerts are printed. Define non‑zero exit codes for “resource exceeded” so callers (e.g., CI pipelines) can react programmatically.
  • Shellcheck compliance – Run shellcheck on monitor.sh; it will flag a few style issues (e.g., quoting of variables, use of $(...) vs backticks). Fixing these will improve maintainability.

🧱 Mocks / Fakes

  • Stubbing approach works but can be refined – Overriding top, free, and df with Bash functions is a clever lightweight mock. However:
    • The stub functions are not exported, which is fine for the current script but could break if the script spawns a subshell. Export them with export -f top if needed.
    • The cleanup step should explicitly unset -f top free df to guarantee the original binaries are restored.
  • Mock data realism – The mock_top output includes both “us” and “sy” fields; the script only cares about the idle percentage, so the mock is sufficient. Consider adding a second mock where the idle field is missing to test the script’s error handling.
  • Isolation of test environment – The test harness modifies global environment variables. Wrap each test in a subshell (( … )) or use set -o errexit and trap to ensure variables are reset even on early failure.

Overall impression: The utility is a useful addition and the codebase is cleanly isolated. Address the test inconsistencies, tighten the cleanup logic, and polish the documentation and security checks, and the contribution will be production‑ready.

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