Fix configuration handling and enhance path ACL checks

README.md

@@ -122,6 +122,14 @@ Make sure lshell is present in `/etc/shells`.
 
 The main template is [`etc/lshell.conf`](etc/lshell.conf). Full reference is available in the man page.
 
+### Best practices
+
+- Prefer an explicit `allowed` allow-list instead of `'all'`.
+- Keep `allowed_shell_escape` short and audit every entry. Never add tools that execute arbitrary commands (for example `find`, `vim`, `xargs`).
+- Use `allowed_file_extensions` when users are expected to work with a known set of file types.
+- Keep `warning_counter` enabled (avoid `-1` unless you intentionally want warning-only behavior).
+- Use `policy-show` during reviews to validate effective policy before assigning it to users.
+
 ### Section model and precedence
 
 Supported section types:
@@ -155,6 +163,7 @@ For local executables, add explicit relative paths (for example `./deploy.sh`).
 
 `warning_counter` is decremented on forbidden command/path/character attempts.
 When `strict = 1`, unknown syntax/commands also decrement `warning_counter`.
+`strict = 1` is typically preferred for higher-assurance restricted environments.
 
 ### `messages`
 
@@ -199,6 +208,12 @@ messages : {
 - `allowed_shell_escape`: explicit list of commands allowed to run child programs. Do not set it to `'all'`.
 - `allowed_file_extensions`: optional allow-list for file extensions passed in command lines.
 
+### Prompt accessibility
+
+- Keep the default prompt text-based and readable in monochrome terminals.
+- If you use ANSI colors in `prompt` or `$LPS1`, avoid color-only meaning (for example, include separators like `user@host:path`).
+- Verify contrast and readability over SSH clients commonly used in your environment.
+
 ### `umask`
 
 Set a persistent session umask in config:

etc/lshell.conf

@@ -3,7 +3,7 @@
 # $Id: lshell.conf,v 1.27 2010-10-18 19:05:17 ghantoos Exp $
 
 [global]
-##  log directory (default /var/log/lshell/ )
+##  log directory (default: /var/log/lshell/)
 logpath         : /var/log/lshell/
 ##  set log level to 0, 1, 2, 3 or 4  (0: no logs, 1: least verbose,
 ##                                                 4: log all commands)
@@ -26,49 +26,49 @@ loglevel        : 2
 ##  using the -exec flag.
 #path_noexec     : '/usr/libexec/sudo/sudo_noexec.so'
 
-## include a directory containing multiple configuration files. These files
-## can only contain default/user/group configuration. The global configuration will
-## only be loaded from the default configuration file.
+## include a directory containing multiple configuration files.
+## these files can only contain default/user/group configuration.
+## global configuration is only loaded from this main configuration file.
 ## e.g. splitting users into separate files
 #include_dir     : /etc/lshell.d/*.conf
 
 [default]
-##  a list of the allowed commands without execution privileges or 'all' to
-##  allow all commands in user's PATH
+##  a list of allowed commands, or 'all' to allow every command in user's PATH
+##  best practice: prefer an explicit allow-list instead of 'all'
 ##  local commands must be explicitly listed with their relative path
 ##  (e.g. './backup.sh')
 ##
-##  if  sudo(8) is installed and sudo_noexec.so is available, it will be loaded
-##  before running every command, preventing it from  running  further  commands
+##  if sudo(8) is installed and sudo_noexec.so is available, it will be loaded
+##  before running every command, preventing it from running further commands
 ##  itself. If not available, beware of commands like vim/find/more/etc. that
 ##  will allow users to execute code (e.g. /bin/sh) from within the application,
 ##  thus easily escaping lshell. See variable 'path_noexec' to use an alternative
-##  path to library.
-allowed         : ['ls','echo','ll','vim','tail','sleep','touch','mkdir','cat','export', 'pwd', './shutdown.sh', './shitdown.sh', 'shutdown.sh', 'shitdown.sh']
+##  path to the library.
+allowed         : ['ls','echo','ll','vim','tail','sleep','touch','mkdir','cat','export', 'pwd']
 #allowed         : ['echo test'] # this will allow only the command 'echo test'
 
-##  A list of the allowed commands that are permitted to execute other
+##  a list of allowed commands that are permitted to execute other
 ##  programs (e.g. shell scripts with exec(3)). Setting this variable to 'all'
-##  is NOT allowed. Warning do not put here any command that can execute
+##  is NOT allowed. warning: do not put here any command that can execute
 ##  arbitrary commands (e.g. find, vim, xargs)
 ##
 ##  Important: commands defined in 'allowed_shell_escape' override their
 ##  definition in the 'allowed' variable
 #allowed_shell_escape        : ['man','zcat']
 
-##  A list of allowed file extensions that can be provided in the command line.
-##  If a list of allowed extensions is provided, all other file extensions will be disallowed.
+##  a list of allowed file extensions that can be provided in the command line.
+##  if set, all other file extensions are denied.
 #allowed_file_extensions : ['.tmp', '.log']
 
-##  a list of forbidden character or commands
+##  a list of forbidden characters or commands
 forbidden       : [';','&', '|','`','>','<', '$(','${']
 
-##  a list of allowed command to use with sudo(8)
-##  if set to ´all', all the 'allowed' commands will be accessible through sudo(8)
+##  a list of allowed commands to use with sudo(8)
+##  if set to 'all', all values from 'allowed' are accessible through sudo(8)
 sudo_commands   : ['ls', 'more']
 
-##  number of warnings when user enters a forbidden value before getting 
-##  exited from lshell, set to -1 to disable.
+##  number of warnings before lshell terminates the session.
+##  set to -1 to disable the counter.
 warning_counter : 2
 
 ##  command aliases list (similar to bash’s alias directive)
@@ -104,10 +104,12 @@ aliases         : {'ll':'ls -l'}
 ##  introduction text to print (when entering lshell)
 #intro           : "== My personal intro ==\nWelcome to lshell\nType '?' or 'help' to get the list of allowed commands"
 
-##  configure your prompt using %u or %h (default: username)
-# prompt          : "%u@%h"
-##  colored prompt using ANSI escape codes colors (light red user, light cyan host)
-prompt          : "\033[91m%u\033[97m@\033[96m%h\033[0m"
+##  configure your prompt using %u (username) and %h (hostname)
+##  accessibility tip: use plain text by default; color-only distinction can be
+##  hard to read for some users and terminals.
+prompt          : "%u@%h"
+##  optional colorized prompt using ANSI escape codes (light red user, light cyan host):
+#prompt          : "\033[91m%u\033[97m@\033[96m%h\033[0m"
 
 
 ##  set prompt path style (0, 1, or 2; default: 0)
@@ -120,18 +122,19 @@ prompt          : "\033[91m%u\033[97m@\033[96m%h\033[0m"
 ##  a value in seconds for the session timer
 #timer           : 5
 
-##  list of path to restrict the user "geographicaly"
-##  warning: many commands like vi and less allow to break this restriction
+##  list of paths to restrict where the user can operate
+##  warning: commands like vi and less can bypass this restriction
 #path            : ['/etc','/var/log','/var/lib']
 
-##  set the home folder of your user. If not specified the home_path is set to 
+##  set the home folder for your user. if not specified, home_path is set to
 ##  the $HOME environment variable
+##  deprecated: prefer setting the home directory with system account tools.
 #home_path       : '/home/bla/'
 
 ##  update the environment variable $PATH of the user
 #env_path        : '/usr/local/bin:/usr/sbin'
 
-##  a list of path; all executable files inside these path will be allowed 
+##  a list of paths; all executable files inside these paths are allowed
 #allowed_cmd_path: ['/home/bla/bin','/home/bla/stuff/libexec']
 
 ##  add environment variables
@@ -150,14 +153,14 @@ prompt          : "\033[91m%u\033[97m@\033[96m%h\033[0m"
 ## forbid scp download
 #scp_download     : 0
 
-##  allow of forbid the use of sftp (set to 1 or 0)
+##  allow or forbid the use of sftp (set to 1 or 0)
 ##  this option will not work if you are using OpenSSH's internal-sftp service
 #sftp            : 1
 
-##  list of command allowed to execute over ssh (e.g. rsync, rdiff-backup, etc.)
+##  list of commands allowed to execute over ssh (e.g. rsync, rdiff-backup)
 #overssh         : ['ls', 'rsync']
 
-##  logging strictness. If set to 1, unknown syntax/commands are considered
+##  strictness mode. if set to 1, unknown syntax/commands are considered
 ##  forbidden and decrement warning_counter (which can kick the user out).
 ##  If set to 0, they are reported as unknown syntax only.
 strict          : 0
@@ -174,7 +177,7 @@ strict          : 0
 ##    - allowed: +['scp', 'env', 'pwd', 'groups', 'unset', 'unalias']
 #winscp: 0
 
-##  history file maximum size 
+##  history file maximum size
 #history_size     : 100
 
 ##  set history file name (default is /home/%u/.lhistory)
@@ -193,8 +196,8 @@ strict          : 0
 ##  changes there (such as umask) do not persist in the lshell parent process
 #login_script     : "/path/to/myscript.sh"
 
-## disable user exit, this could be useful when lshell is spawned from another
-## none-restricted shell (e.g. bash)
+## disable user exit; useful when lshell is spawned from another
+## non-restricted shell (e.g. bash)
 #disable_exit      : 0
 
 ## show/hide policy introspection builtins:

lshell/checkconfig.py

@@ -1,4 +1,4 @@
-""" This module contains the checkconfig class of lshell """
+"""This module contains the checkconfig class of lshell"""
 
 import sys
 import os
@@ -353,12 +353,13 @@ def get_config_sub(self, section):
                                 self.minusplus(self.conf_raw, key, stuff)
                             )
                         elif configschema.is_all_literal(stuff):
-                            if key == "allowed_shell_escape":
+                            if key == "allowed":
+                                self.conf_raw.update({key: self.expand_all()})
+                            else:
                                 self.log.critical(
-                                    "lshell: config: 'allowed_shell_escape' cannot be set to 'all'"
+                                    f"lshell: config: '{key}' cannot be set to 'all'"
                                 )
                                 sys.exit(1)
-                            self.conf_raw.update({key: self.expand_all()})
                         elif stuff and key == "path":
                             liste = ["", ""]
                             for path in self._parse_config_value(stuff, key):
@@ -371,8 +372,8 @@ def get_config_sub(self, section):
                             self._parse_config_value(stuff, key), list
                         ):
                             self.conf_raw.update({key: stuff})
-                # case allowed is set to 'all'
-                elif key == "allowed" and split[0].strip() == "'all'":
+                # case allowed/sudo_commands is set to all
+                elif key == "allowed" and configschema.is_all_literal(split[0]):
                     self.conf_raw.update({key: self.expand_all()})
                 elif key == "allowed_shell_escape" and configschema.is_all_literal(
                     split[0]
@@ -472,7 +473,9 @@ def _parse_config_value(self, value, key=""):
 
     def _parse_history_file(self):
         """Parse and validate history_file from raw config."""
-        history_value = self.conf_raw["history_file"].replace("%u", self.conf["username"])
+        history_value = self.conf_raw["history_file"].replace(
+            "%u", self.conf["username"]
+        )
         if (
             isinstance(history_value, str)
             and len(history_value) >= 2
@@ -555,7 +558,9 @@ def get_config_user(self):
                 if len(self.conf_raw[item]) == 0:
                     self.conf[item] = ""
                 else:
-                    self.conf[item] = self._parse_config_value(self.conf_raw[item], item)
+                    self.conf[item] = self._parse_config_value(
+                        self.conf_raw[item], item
+                    )
             except KeyError:
                 if item in [
                     "allowed",
@@ -718,15 +723,15 @@ def get_config_user(self):
                         self.conf["allowed"].append(item)
 
         # case sudo_commands set to 'all', expand to all 'allowed' commands
-        if (
-            "sudo_commands" in self.conf_raw
-            and configschema.is_all_literal(str(self.conf_raw["sudo_commands"]))
+        if "sudo_commands" in self.conf_raw and configschema.is_all_literal(
+            str(self.conf_raw["sudo_commands"])
         ):
-            # exclude native commands and sudo(8)
-            exclude = builtincmd.builtins_list + ["sudo"]
-            self.conf["sudo_commands"] = [
-                x for x in self.conf["allowed"] if x not in exclude
-            ]
+            # Keep shell-internal builtins out of sudo all-expansion while
+            # preserving `ls`, which is executed via exec_cmd in lshell.
+            exclude = [cmd for cmd in builtincmd.builtins_list if cmd != "ls"] + ["sudo"]
+            self.conf["sudo_commands"] = list(
+                dict.fromkeys(x for x in self.conf["allowed"] if x not in exclude)
+            )
 
         # sort lsudo commands
         self.conf["sudo_commands"].sort()

lshell/completion.py

@@ -83,6 +83,9 @@ def complete_change_dir(conf, text, line, begidx, endidx):
             if instance.startswith(directory) and instance.startswith(tocomplete):
                 # Extract the next unmatched segment of the allowed path
                 remaining_path = instance[len(directory) :].lstrip("/")
+                # Nothing left to suggest for this allowed path.
+                if not remaining_path:
+                    continue
                 if "/" in remaining_path:
                     next_segment = remaining_path.split("/", 1)[0] + "/"
                 else:

lshell/configschema.py

@@ -85,9 +85,12 @@ def parse_config_value(value, key=""):
 
     Raises ValueError with user-friendly field-level errors.
     """
-    if isinstance(value, str) and key in {"allowed", "sudo_commands"}:
-        if value.strip() == "all":
-            return "all"
+    if (
+        isinstance(value, str)
+        and key in {"allowed", "sudo_commands"}
+        and is_all_literal(value)
+    ):
+        return "all"
 
     try:
         evaluated = ast.literal_eval(value)

lshell/policy.py

@@ -185,7 +185,10 @@ def _merge_section(conf_raw, section, section_items, key_sources, trace):
                         raise ValueError(
                             "'allowed_shell_escape' cannot be set to 'all'"
                         )
-                    conf_raw.update({key: _expand_all()})
+                    if key == "allowed":
+                        conf_raw.update({key: _expand_all()})
+                    else:
+                        raise ValueError(f"'{key}' cannot be set to 'all'")
                     trace.append(
                         {
                             "section": section,
@@ -231,7 +234,7 @@ def _merge_section(conf_raw, section, section_items, key_sources, trace):
                         }
                     )
                     previous = conf_raw.get(key)
-        elif key == "allowed" and split[0].strip() == "'all'":
+        elif key == "allowed" and configschema.is_all_literal(split[0]):
             conf_raw.update({key: _expand_all()})
             trace.append(
                 {
@@ -357,8 +360,10 @@ def _build_runtime_policy(conf_raw, username):
     if "sudo_commands" in conf_raw and configschema.is_all_literal(
         str(conf_raw["sudo_commands"])
     ):
-        exclude = builtincmd.builtins_list + ["sudo"]
-        policy["sudo_commands"] = [x for x in policy["allowed"] if x not in exclude]
+        exclude = [cmd for cmd in builtincmd.builtins_list if cmd != "ls"] + ["sudo"]
+        policy["sudo_commands"] = list(
+            dict.fromkeys(x for x in policy["allowed"] if x not in exclude)
+        )
 
     policy["allowed"] += policy["allowed_shell_escape"]