feat: allow to override default shell#9625
Conversation
With this simple change, it is possible to override the default shell for menu item 8 in `opnsense-shell`. To use bash instead of csh, create a file called `/etc/opnsense-shell` and add the following: ``` CMD_SHELL="/usr/local/bin/bash" ```
| CMD_SETPORTS="/usr/local/opnsense/scripts/shell/setports.php" | ||
| CMD_SHELL="/bin/csh" | ||
|
|
||
| [ -f /etc/opnsense-shell ] && . /etc/opnsense-shell |
There was a problem hiding this comment.
I don't know. I keep looking at this thinking it's certianly simple but also wide open for misuse and future support efforts since you can just take over the whole script here and the end result is not really visible/distinguishable which may backfire in official support cases as well.
A better approach would probably to add .conf file support and import commands as required, but we kind of run the risk of overriding a command with something else entirely here as well.
A shell should ideally only be accepted if it's in /etc/shells. The PR should focus on CMD_SHELL specifically, too, even though a generic configuration file now is better than later even if it's only a single setting.
There was a problem hiding this comment.
certianly simple but also wide open for misuse
I thought about that as well, but a lot of shell scripts source other files. Especially when it comes to overriding variables.
However, one has to be root to create and/or change /etc/opnsense-shell, thus the person who is doing this knows what they are doing. If somebody doesn't know what they are doing, they won't drop into the shell in the first place.
It's like asking someone with a driver's licence to take the driver's licence exam every time before they drive a car.
Someody with root access can do pretty much anything. How do you prevent mistakes or deliberate sabotage? You don't need a sourced script to do harm. You can just add a forkbomb or an rm <we-all-know-what-destroys-a-filesystem> to this script or to .cshrc. You are not preventing that either. Why? Because it is impossible.
A better approach would probably to add .conf file support
For sure, but how exactly? Does OPNsense provide functions to do so? Writing toml/json/ini support in sh is not what I had in mind for this PR.
A shell should ideally only be accepted if it's in /etc/shells. The PR should focus on CMD_SHELL specifically, too, even though a generic configuration file now is better than later even if it's only a single setting.
In that case it won't be a one-liner. I could read (not source) /etc/opnsense-shell and extract the value of CMD_SHELL and verify whether the var is in /etc/shells.
Even though I like the idea, I agree with you that this is overly complex for such a simple task and that this could add a maintenance overhead.
Let me think about it. Maybe I find a succint solution. I might have a few ideas.
P.S.: Otherwise I rather write an rc script or a cronjob that patches this script on every reboot. This solution is easy but local, so it wouldn't be available to others.
|
In the long run we probably want a more flexible/pluggable menu system, all efforts spend here are likely of limited value and only serve a small population of our users in my humble opinion. I wouldn't mind people registering their own menu actions, in the current setup that's just not very feasible, adding hacks in the meantime often accumulates technical debt for the next person to solve. |
|
As mentioned in the forum's post, I don't have a problem closing this PR. I already have a solution that does not require manual intervention to patch this file, when it is reverted due to a firmware update. All my attempts so far to address Franco's concerns are too lengthy and not worth the gain, which admittedly only exists for a low percentage of users. |
|
If you add a shell to /etc/shells without "opnsense-" it will show up in the GUI for the root user so you could make a permanent copy of the shell menu. The only issue is shell restore for recovery purposes which will reset it on each boot (and is not permanently modified): https://github.com/opnsense/core/blob/master/src/etc/rc.subr.d/recover#L121-L130 I don't mind appending a file like I know I know it's a generic override, but it doesn't meddle with the default shell/console menu file or execute anything directly. Cheers, |
This is also a nice "local" solution. Thanks for the tip.
Sorry, but this confuses me a bit. So, Even if it wasn't for my use-case, I still think a IMO any file that is generated should say so in the file (using a comment). e.g. I do that with ansible or (bootstrap) scripts, so that I don't forget and change a file thinking the changes will be retained. |
This dates back to UFS crashes that leave zero byte files and /etc/shells is one of the things that is required for a system to boot properly.
Sure, these are easy changes to contribute. The recovery script for example is here and some hints exist but it's also not that great to add comments to files such as https://github.com/opnsense/core/blob/master/src/etc/rc.subr.d/recover |
Hmm, interesting. Thx for the info. Although isn't there a way to make this conditional: e.g. recover only when file size is 0
True, on BSD this won't work. But yeah, I came across files in Linux, which do not allow comments, as well. In such a case I create a file and add the info there. e.g.: I am not saying this must be done in OPNsense. It's just a practice I've been using for quite some time now and it saved my ass a few times... ;-) |
|
I guess we can close this then. |
3 participants
With this simple change, it is possible to override the default shell for menu item 8 in
opnsense-shell.To use bash instead of csh, create a file called
/etc/opnsense-shelland add the following:As Franco mentioned this has currently no documentation. I see this as a starting point for a discussion.
But it's easy to either add a comment to the file or add it to the official documentation.
On the other side, I suspect that mostly people who are familiar with Unix/Linux and the commandline would want to use a different shell than the default. For those people I don't think any documentation is necessary. They see that the shell for root is
/usr/local/sbin/opnsense-shelland check out that file, which will lead them to/etc/opnsense-shell.(This is how I found out which shell is used when choosing item 8.)
This change also does not have any checks, whether the shell (or its path) is valid. It's easy to add that too, but in that case it won't be a one-liner.
My only goal with this PR is to be able to change the shell w/o having to patch the file every single time there is an update.
It won't break any existing installations, but allows people to replace the shell. All this without any maintenance burden.