feat: make SSH user configurable and harden remote actions
This commit is contained in:
@@ -20,7 +20,7 @@ The tool:
|
||||
2. Expects a `NODES` array populated with entries formatted like:
|
||||
`host;display-name;action1;action2;...`
|
||||
3. Displays an interactive multi-select checklist using `whiptail`
|
||||
4. Executes the selected actions on each selected host through `ssh root@host`
|
||||
4. Executes the selected actions on each selected host through SSH, using `root@host` by default or `SSH_USER@host` when configured
|
||||
5. Writes execution logs to `~/netupgrade.log`
|
||||
6. Opens the log with `$EDITOR` when available, otherwise `nano`, `vi`, or `less`; if none is available, it prints the log path, then optionally removes the log file
|
||||
|
||||
@@ -57,18 +57,15 @@ Supported action types currently include:
|
||||
- The configuration format and supported actions are now documented in more detail
|
||||
|
||||
### 2. Shell robustness concerns
|
||||
- Node parsing relies on replacing `;` with spaces and re-splitting:
|
||||
this is fragile if values contain spaces or special characters
|
||||
- Quoting is inconsistent across the script
|
||||
- Config files are sourced directly, which is flexible but implies arbitrary code execution
|
||||
- The script does not use a stricter shell safety baseline
|
||||
- The script still has some quoting-sensitive areas and does not use a stricter shell safety baseline
|
||||
- The `NODES` parsing was hardened to split on `;` with `IFS`/`read -r -a`, which now preserves spaces in action values such as `cmd:...`
|
||||
|
||||
### 3. Remote execution correctness and safety
|
||||
- SSH user is hardcoded to `root`
|
||||
- `cmd:<...>` intentionally allows arbitrary remote command execution, which should be treated as a powerful unsafe feature
|
||||
- Some remote command constructions are brittle
|
||||
- The `pacman` orphan-removal command appears incorrect because command substitution is evaluated locally instead of remotely
|
||||
- The `docker-stacks` remote loop should be reviewed carefully for quoting and path safety
|
||||
- SSH execution now goes through a dedicated `runSSH` helper and the SSH user is configurable via `SSH_USER`, defaulting to `root`
|
||||
- `cmd:<...>` intentionally allows arbitrary remote command execution and is now executed through a remote shell, which improves support for shell operators but remains a powerful unsafe feature
|
||||
- The `pacman` orphan-removal command was corrected so orphan detection happens on the remote host instead of locally
|
||||
- The `docker-stacks` remote loop was rewritten to pass the stack root as an argument to a remote shell script, improving quoting and path handling
|
||||
|
||||
### 4. UX and dependency issues
|
||||
- Required runtime dependencies are now checked at startup (`ssh`, `whiptail`, `sed`, `tee`, `rm`, `touch`)
|
||||
@@ -92,10 +89,9 @@ Supported action types currently include:
|
||||
## Recommended direction
|
||||
|
||||
### Short term
|
||||
- Correct the remote command bugs, especially for `pacman`
|
||||
- Improve parsing of node entries to avoid whitespace-splitting issues
|
||||
- Review `docker-stacks` remote command quoting and behavior
|
||||
- Consider making SSH user, log path, and editor configurable
|
||||
- Review the remaining quoting-sensitive areas, especially around remote shell command construction
|
||||
- Consider making log path and editor configurable
|
||||
- Revisit the log summary insertion method, which still relies on `sed -i` string interpolation
|
||||
|
||||
### Medium term
|
||||
- Improve parsing of node entries to avoid whitespace-splitting issues
|
||||
@@ -116,6 +112,11 @@ Supported action types currently include:
|
||||
- Log viewer selection was made more flexible: `$EDITOR` is preferred, then `nano`, `vi`, or `less`
|
||||
- `nano` is no longer a strict runtime dependency
|
||||
- The unsupported `-b` option remains unimplemented and is no longer shown in help output
|
||||
- `NODES` parsing was hardened to preserve spaces in action values by splitting on `;` with `IFS` and `read -r -a`
|
||||
- SSH calls were centralized through a `runSSH` helper and `SSH_USER` is now configurable, defaulting to `root`
|
||||
- The `pacman` orphan cleanup now runs entirely on the remote host instead of evaluating orphan detection locally
|
||||
- The `docker-stacks` action was rewritten to use a remote shell script with the stack directory passed as an argument
|
||||
- Unknown actions and reboot SSH failures now propagate error status more consistently
|
||||
|
||||
## Change guidance
|
||||
- Preserve backward compatibility for existing config files where possible
|
||||
|
||||
Reference in New Issue
Block a user