[PR #27] [MERGED] V2.2.0 - Security hardening, performance optimizations, and GUI impro… #23

New issue

Closed

opened 2026-03-03 01:17:26 +03:00 by kerem · 0 comments

kerem commented

2026-03-03 01:17:26 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/coelacant1/ProxmoxScripts/pull/27
Author: @coelacant1
Created: 3/2/2026
Status: ✅ Merged
Merged: 3/2/2026
Merged by: @coelacant1

Base: main ← Head: testing

📝 Commits (1)

7fb11ca V2.2.0 - Security hardening, performance optimizations, and GUI improvements

📊 Changes

30 files changed (+457 additions, -119 deletions)

View changed files

📝 .docs/TODO.md (+1 -1)
📝 .github/workflows/checks.yml (+6 -0)
📝 CHANGELOG.md (+71 -1)
📝 GUI.sh (+76 -20)
📝 Host/Hardware/EnableCPUScalingGovernor.sh (+7 -7)
➕ LXC/Operations/BulkAddToPool.sh (+73 -0)
📝 Networking/FindVMIDFromIP.sh (+17 -13)
📝 README.md (+12 -1)
📝 Resources/ChangeAllMACPrefix.sh (+2 -1)
📝 Resources/InteractiveRestore.sh (+1 -1)
📝 Storage/AddStorage.sh (+3 -3)
📝 Storage/RemoveStorage.sh (+6 -2)
📝 ThirdParty/ApacheGuacamole/GetGuacamoleAuthenticationToken.sh (+4 -2)
📝 Utilities/ArgumentParser.sh (+8 -6)
📝 Utilities/BulkOperations.sh (+6 -2)
📝 Utilities/Conversion.sh (+1 -1)
📝 Utilities/Logger.sh (+1 -1)
📝 Utilities/Operations.sh (+4 -4)
📝 Utilities/RemoteExecutor.sh (+50 -5)
📝 Utilities/SSH.sh (+11 -5)

...and 10 more files

📄 Description

…vements

Security

SSH Password Exposure - Switched all sshpass -p calls to sshpass -e (environment variable)
- Passwords no longer visible in ps aux process listing
- Applied to all 4 sites in SSH.sh (__wait_for_ssh__, __ssh_exec__, __scp_send__, __scp_fetch__)
- SSHPASS environment variable is unset immediately after each command
Container Password Exposure - Changed __ct_change_password__ to pipe credentials via stdin
- Previously embedded password in bash -c command string (visible in /proc)
- Now pipes directly to pct exec -- chpasswd
Guacamole Token Security - Token file now created with restricted permissions
- Directory created with mkdir -p -m 700, token file set to chmod 600
- Prevents other system users from reading authentication tokens
Guacamole API Credentials - Switched to --data-urlencode for curl authentication
- Prevents special characters in passwords (e.g., &, =) from breaking API calls
Eval Removal - Replaced eval with safer alternatives across 10 sites in 6 files
- Command execution contexts now use bash -c instead of eval "$cmd"
- ArgumentParser.sh uses declare -g instead of eval for variable assignment
ArgumentParser Blocklist - Extended reserved variable name list
- Added high-risk names (HOSTNAME, RANDOM, SECONDS, GROUPS, etc.) to prevent overwrites

Fixed

Filename Typo - Renamed EnableCPUScalingGoverner.sh to EnableCPUScalingGovernor.sh
- Updated all references in CHANGELOG.md, .docs/TODO.md, and internal SCRIPT_NAME
CreateFromISO Structure - Moved set -euo pipefail after header comment block
- Added shellcheck source directive for sourced utility files
RemoveStorage Race Condition - Cached VM/CT config per iteration
- Added || continue to skip VMs/CTs deleted between list and config check
Locale-Dependent Parsing - Fixed AWK decimal parsing in CreateFromISO.sh
- Added LC_NUMERIC=C and comma-to-dot conversion for European locale compatibility
GUI Unicode Symbols - Replaced all Unicode checkmarks/crosses with plain text

Changed

GUI Breadcrumb Navigation - Path display now shows cc_pve > Storage > Ceph style
GUI Script Descriptions - Menu listings show inline description extracted from script headers
GUI Log Level Hint - "Type 'l' to change log level" only shown in remote execution mode
SSH Error Context - Connection failures now display the SSH error reason at all 7 failure sites
SSH Keepalive - Added ServerAliveInterval=5 and ServerAliveCountMax=3 to SSH and SCP
Multi-Node Recovery - Execution summary now lists per-node results with retry option
- Shows OK: node1 node2 and FAIL: node3 after multi-remote execution
- Prompts to retry only the failed nodes
CreateFromISO ArgumentParser Migration - Replaced getopts with __parse_args__
- Arguments now use --vm-name, --iso-url, --vm-storage style flags
- All 8 arguments optional with interactive fallback preserved

Added

CI Unit Tests - Added unit test stage to .github/workflows/checks.yml
- Runs Utilities/RunAllTests.sh after static analysis checks
BulkOperations Source Guards - Defensive guards on source calls in BulkOperations.sh
GUI Update Safety Guard - Validates BASE_DIR before cleanup in update_scripts()
Documentation - Added Manuals/README.md table of contents and Documentation section in main README

Performance

FindVMIDFromIP Caching - Config fetched once per VMID instead of 3 times (~67% fewer API calls)
Double-Sed Consolidation - Merged 9 paired sed | sed calls into single sed -e ... -e ...
- Applied to BulkConfigureNetworkBandwidth, BulkConfigureDiskIOPS, BulkConfigureDiskBandwidth
Bash Builtins - Replaced echo | tr subprocesses with native ${var^^} case conversion
- Applied to FindVMIDFromIP, BulkCloneSetIP_Proxmox, BulkReconfigureMacAddresses, Conversion.sh, ChangeAllMACPrefix.sh
Carriage Return Removal - Replaced echo | tr -d '\r' with ${var//$'\r'/} in GUI.sh

Technical Details

sshpass -e reads from SSHPASS environment variable; inline assignment (SSHPASS=x cmd) used where possible
declare -g requires Bash 4.2+
eval retained in TestFramework.sh (dynamic function stubs) and RemoteExecutor.sh (SSH parameter expansion) - both legitimate uses
Multi-node retry uses recursive __execute_remote_script__ call with filtered target list
FindVMIDFromIP caches both JSON and plain-text config formats per VMID for reuse
--data-urlencode sends each parameter separately, preventing URL parameter injection

name: Pull Request
about: Security hardening, performance optimizations, and GUI improvements
title: "[PR] Security hardening, performance optimizations, and GUI improvements"
labels: enhancement
assignees: 'coelacant1'

Type of Change

Bug fix (non-breaking change fixing an issue)
New feature (non-breaking change adding functionality)
Breaking change (fix or feature that would break existing functionality)
Documentation update
Other (please describe):

How Has This Been Tested?

Automated checks and remote execution on single node + multi node to virtual test cluster.

Checklist

I have performed a self-review of my own code.
I have commented my code where necessary.
I have made corresponding changes to the documentation (if applicable).
My changes do not generate new warnings or errors.
I have tested this code.

N/A

Additional Context

N/A

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/coelacant1/ProxmoxScripts/pull/27 **Author:** [@coelacant1](https://github.com/coelacant1) **Created:** 3/2/2026 **Status:** ✅ Merged **Merged:** 3/2/2026 **Merged by:** [@coelacant1](https://github.com/coelacant1) **Base:** `main` ← **Head:** `testing` --- ### 📝 Commits (1) - [`7fb11ca`](https://github.com/coelacant1/ProxmoxScripts/commit/7fb11ca8fab9baa9e9e8103894de0733d771d9d3) V2.2.0 - Security hardening, performance optimizations, and GUI improvements ### 📊 Changes **30 files changed** (+457 additions, -119 deletions) <details> <summary>View changed files</summary> 📝 `.docs/TODO.md` (+1 -1) 📝 `.github/workflows/checks.yml` (+6 -0) 📝 `CHANGELOG.md` (+71 -1) 📝 `GUI.sh` (+76 -20) 📝 `Host/Hardware/EnableCPUScalingGovernor.sh` (+7 -7) ➕ `LXC/Operations/BulkAddToPool.sh` (+73 -0) 📝 `Networking/FindVMIDFromIP.sh` (+17 -13) 📝 `README.md` (+12 -1) 📝 `Resources/ChangeAllMACPrefix.sh` (+2 -1) 📝 `Resources/InteractiveRestore.sh` (+1 -1) 📝 `Storage/AddStorage.sh` (+3 -3) 📝 `Storage/RemoveStorage.sh` (+6 -2) 📝 `ThirdParty/ApacheGuacamole/GetGuacamoleAuthenticationToken.sh` (+4 -2) 📝 `Utilities/ArgumentParser.sh` (+8 -6) 📝 `Utilities/BulkOperations.sh` (+6 -2) 📝 `Utilities/Conversion.sh` (+1 -1) 📝 `Utilities/Logger.sh` (+1 -1) 📝 `Utilities/Operations.sh` (+4 -4) 📝 `Utilities/RemoteExecutor.sh` (+50 -5) 📝 `Utilities/SSH.sh` (+11 -5) _...and 10 more files_ </details> ### 📄 Description …vements ### Security - **SSH Password Exposure** - Switched all `sshpass -p` calls to `sshpass -e` (environment variable) - Passwords no longer visible in `ps aux` process listing - Applied to all 4 sites in SSH.sh (`__wait_for_ssh__`, `__ssh_exec__`, `__scp_send__`, `__scp_fetch__`) - SSHPASS environment variable is unset immediately after each command - **Container Password Exposure** - Changed `__ct_change_password__` to pipe credentials via stdin - Previously embedded password in `bash -c` command string (visible in /proc) - Now pipes directly to `pct exec -- chpasswd` - **Guacamole Token Security** - Token file now created with restricted permissions - Directory created with `mkdir -p -m 700`, token file set to `chmod 600` - Prevents other system users from reading authentication tokens - **Guacamole API Credentials** - Switched to `--data-urlencode` for curl authentication - Prevents special characters in passwords (e.g., `&`, `=`) from breaking API calls - **Eval Removal** - Replaced `eval` with safer alternatives across 10 sites in 6 files - Command execution contexts now use `bash -c` instead of `eval "$cmd"` - ArgumentParser.sh uses `declare -g` instead of `eval` for variable assignment - **ArgumentParser Blocklist** - Extended reserved variable name list - Added high-risk names (HOSTNAME, RANDOM, SECONDS, GROUPS, etc.) to prevent overwrites ### Fixed - **Filename Typo** - Renamed `EnableCPUScalingGoverner.sh` to `EnableCPUScalingGovernor.sh` - Updated all references in CHANGELOG.md, .docs/TODO.md, and internal SCRIPT_NAME - **CreateFromISO Structure** - Moved `set -euo pipefail` after header comment block - Added shellcheck source directive for sourced utility files - **RemoveStorage Race Condition** - Cached VM/CT config per iteration - Added `|| continue` to skip VMs/CTs deleted between list and config check - **Locale-Dependent Parsing** - Fixed AWK decimal parsing in CreateFromISO.sh - Added `LC_NUMERIC=C` and comma-to-dot conversion for European locale compatibility - **GUI Unicode Symbols** - Replaced all Unicode checkmarks/crosses with plain text ### Changed - **GUI Breadcrumb Navigation** - Path display now shows `cc_pve > Storage > Ceph` style - **GUI Script Descriptions** - Menu listings show inline description extracted from script headers - **GUI Log Level Hint** - "Type 'l' to change log level" only shown in remote execution mode - **SSH Error Context** - Connection failures now display the SSH error reason at all 7 failure sites - **SSH Keepalive** - Added `ServerAliveInterval=5` and `ServerAliveCountMax=3` to SSH and SCP - **Multi-Node Recovery** - Execution summary now lists per-node results with retry option - Shows `OK: node1 node2` and `FAIL: node3` after multi-remote execution - Prompts to retry only the failed nodes - **CreateFromISO ArgumentParser Migration** - Replaced `getopts` with `__parse_args__` - Arguments now use `--vm-name`, `--iso-url`, `--vm-storage` style flags - All 8 arguments optional with interactive fallback preserved ### Added - **CI Unit Tests** - Added unit test stage to `.github/workflows/checks.yml` - Runs `Utilities/RunAllTests.sh` after static analysis checks - **BulkOperations Source Guards** - Defensive guards on source calls in BulkOperations.sh - **GUI Update Safety Guard** - Validates BASE_DIR before cleanup in `update_scripts()` - **Documentation** - Added `Manuals/README.md` table of contents and Documentation section in main README ### Performance - **FindVMIDFromIP Caching** - Config fetched once per VMID instead of 3 times (~67% fewer API calls) - **Double-Sed Consolidation** - Merged 9 paired `sed | sed` calls into single `sed -e ... -e ...` - Applied to BulkConfigureNetworkBandwidth, BulkConfigureDiskIOPS, BulkConfigureDiskBandwidth - **Bash Builtins** - Replaced `echo | tr` subprocesses with native `${var^^}` case conversion - Applied to FindVMIDFromIP, BulkCloneSetIP_Proxmox, BulkReconfigureMacAddresses, Conversion.sh, ChangeAllMACPrefix.sh - **Carriage Return Removal** - Replaced `echo | tr -d '\r'` with `${var//$'\r'/}` in GUI.sh ### Technical Details - `sshpass -e` reads from `SSHPASS` environment variable; inline assignment (`SSHPASS=x cmd`) used where possible - `declare -g` requires Bash 4.2+ - `eval` retained in TestFramework.sh (dynamic function stubs) and RemoteExecutor.sh (SSH parameter expansion) - both legitimate uses - Multi-node retry uses recursive `__execute_remote_script__` call with filtered target list - FindVMIDFromIP caches both JSON and plain-text config formats per VMID for reuse - `--data-urlencode` sends each parameter separately, preventing URL parameter injection --- name: Pull Request about: Security hardening, performance optimizations, and GUI improvements title: "[PR] Security hardening, performance optimizations, and GUI improvements" labels: enhancement assignees: 'coelacant1' --- ## Type of Change - [x] Bug fix (non-breaking change fixing an issue) - [x] New feature (non-breaking change adding functionality) - [ ] Breaking change (fix or feature that would break existing functionality) - [x] Documentation update - [ ] Other (please describe): ## How Has This Been Tested? Automated checks and remote execution on single node + multi node to virtual test cluster. ## Checklist - [x] I have performed a self-review of my own code. - [x] I have commented my code where necessary. - [x] I have made corresponding changes to the documentation (if applicable). - [x] My changes do not generate new warnings or errors. - [x] I have tested this code. ## Related Issues N/A ## Additional Context N/A --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>