Fix source command: cmd_export and cmd_source to handle quoted values and tilde expansion in environment variables

docker-compose.yml

@@ -215,7 +215,7 @@ services:
     user: "testuser"
     working_dir: /home/testuser/lshell
     container_name: ubuntu_tests
-    command: "sh -c 'pytest && pylint lshell && flake8 lshell'"
+    command: "sh -c 'pylint lshell test setup.py && flake8 lshell && pytest'"
     volumes:
       - .:/home/testuser/lshell
     environment:
@@ -228,7 +228,7 @@ services:
       args:
         DISTRO: "debian:latest"
     container_name: debian_tests
-    command: "sh -c 'pytest; pylint lshell; flake8 lshell'"
+    command: "sh -c 'pylint lshell test setup.py; flake8 lshell; pytest'"
     volumes:
       - .:/app
     environment:
@@ -241,7 +241,7 @@ services:
       args:
         DISTRO: "fedora:latest"
     container_name: fedora_tests
-    command: "pytest; pylint lshell; flake8 lshell"
+    command: "sh -c 'pylint lshell test setup.py; flake8 lshell; pytest'"
     volumes:
       - .:/app
     environment:
@@ -254,7 +254,7 @@ services:
       args:
         DISTRO: "centos:8"
     container_name: centos_tests
-    command: "sh -c 'pytest-3; pylint lshell; pyflake lshell'"
+    command: "sh -c 'pylint lshell test setup.py; pyflake lshell; pytest-3'"
     volumes:
       - .:/app
     environment:

lshell/builtincmd.py

@@ -4,6 +4,7 @@
 import sys
 import os
 import re
+import shlex
 import readline
 import signal
 
@@ -115,32 +116,32 @@ def cmd_history(conf, log):
 
 def cmd_export(args):
     """export environment variables"""
-    # if command contains at least 1 space
-    if args.count(" "):
-        env = args.split(" ", 1)[1]
-        # if it contains the equal sign, consider only the first one
-        if env.count("="):
-            var, value = env.split(" ")[0].split("=")[0:2]
-            # disallow dangerous variable
-            if var in variables.FORBIDDEN_ENVIRON:
-                return 1, var
-            # Strip the quotes from the value if it begins and ends with quotes (single or double)
-            if (value.startswith('"') and value.endswith('"')) or (
-                value.startswith("'") and value.endswith("'")
-            ):
-                value = value[1:-1]
-            os.environ.update({var: value})
+    try:
+        tokens = shlex.split(args, posix=True)
+    except ValueError:
+        return 0, None
+
+    if len(tokens) >= 2 and "=" in tokens[1]:
+        var, value = tokens[1].split("=", 1)
+        # disallow dangerous variable
+        if var in variables.FORBIDDEN_ENVIRON:
+            return 1, var
+        os.environ.update({var: value})
     return 0, None
 
 
 def cmd_source(envfile):
     """Source a file in the current shell context"""
-    envfile = os.path.expandvars(envfile)
+    envfile = envfile.strip().strip("'").strip('"')
+    envfile = os.path.expanduser(os.path.expandvars(envfile))
     try:
         with open(envfile, encoding="utf-8") as env_vars:
             for env_var in env_vars.readlines():
-                if env_var.split(" ", 1)[0] == "export":
-                    cmd_export(env_var.strip())
+                line = env_var.strip()
+                if not line or line.startswith("#"):
+                    continue
+                if line.startswith("export "):
+                    cmd_export(line)
     except (OSError, IOError):
         sys.stderr.write(f"lshell: unable to read environment file: {envfile}\n")
         return 1

test/test_builtins.py

@@ -9,6 +9,7 @@
 TOPDIR = os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))
 CONFIG = f"{TOPDIR}/test/testfiles/test.conf"
 LSHELL = f"{TOPDIR}/bin/lshell"
+SOURCE_FIXTURE = f"{TOPDIR}/test/testfiles/source_command_fixture.lsh"
 USER = getuser()
 PROMPT = f"{USER}:~\\$"
 
@@ -159,23 +160,18 @@ def test_68_source_nonexistent_file(self):
     def test_69_source_valid_file(self):
         """F69 | Test sourcing a valid environment file sets variables"""
 
-        # Write a sample environment file
-        env_file = "random_test_env"
-        with open(env_file, "w") as file:
-            file.write("export TEST_VAR='test_value'\n")
-
         # Start lshell and source the environment file
         child = pexpect.spawn(f"{LSHELL} --config {CONFIG} --allowed \"+['source']\"")
         child.expect(PROMPT)
 
         # Source the file and check if the variable is set
-        child.sendline(f"source {env_file}")
+        child.sendline(f"source {SOURCE_FIXTURE}")
         child.expect(PROMPT)
-        child.sendline("echo $TEST_VAR")
+        child.sendline("echo $SOURCE_DOUBLE_QUOTED")
         child.expect(PROMPT)
 
         output = child.before.decode("utf-8").split("\n")[1].strip()
-        expected_output = "test_value"
+        expected_output = "hello world"
 
         assert (
             output == expected_output
@@ -187,25 +183,20 @@ def test_69_source_valid_file(self):
     def test_70_source_overwrite_variable(self):
         """F70 | Test sourcing a file overwrites existing environment variables"""
 
-        # Write a sample environment file
-        env_file = "test_env_overwrite"
-        with open(env_file, "w") as file:
-            file.write("export TEST_VAR='new_value'\n")
-
         # Start lshell, set initial variable, and source file to overwrite it
         child = pexpect.spawn(f"{LSHELL} --config {CONFIG} --allowed \"+['source']\"")
         child.expect(PROMPT)
 
         # Set initial variable and source the file
-        child.sendline("export TEST_VAR='initial_value'")
+        child.sendline("export SOURCE_SIMPLE='initial_value'")
         child.expect(PROMPT)
-        child.sendline(f"source {env_file}")
+        child.sendline(f"source {SOURCE_FIXTURE}")
         child.expect(PROMPT)
-        child.sendline("echo $TEST_VAR")
+        child.sendline("echo $SOURCE_SIMPLE")
         child.expect(PROMPT)
 
         output = child.before.decode("utf-8").split("\n")[1].strip()
-        expected_output = "new_value"
+        expected_output = "value"
 
         assert (
             output == expected_output

test/test_parser_jobs_unit.py

@@ -4,7 +4,7 @@
 import os
 import tempfile
 import unittest
-from contextlib import redirect_stderr, redirect_stdout
+from contextlib import redirect_stdout
 from unittest.mock import patch
 
 from lshell import builtincmd
@@ -233,7 +233,7 @@ def test_completenames_dot_slash_with_prefixed_text(self):
 
 
 class TestBuiltinsJobsAndSource(unittest.TestCase):
-    """Tests for built-in commands around source and job control."""
+    """Tests for built-in commands around job control."""
 
     def setUp(self):
         """Save and clear global background job state before each test."""
@@ -245,33 +245,6 @@ def tearDown(self):
         builtincmd.BACKGROUND_JOBS.clear()
         builtincmd.BACKGROUND_JOBS.extend(self._previous_jobs)
 
-    @patch.dict(os.environ, {}, clear=True)
-    def test_cmd_source_loads_exported_values(self):
-        """Load exported entries from a source file into the environment."""
-        with tempfile.NamedTemporaryFile("w", delete=False) as env_file:
-            env_file.write("export FIRST=one\n")
-            env_file.write("NOPE=ignore\n")
-            env_file.write("export SECOND='two_words'\n")
-            file_path = env_file.name
-
-        try:
-            self.assertEqual(builtincmd.cmd_source(file_path), 0)
-            self.assertEqual(os.environ.get("FIRST"), "one")
-            self.assertIsNone(os.environ.get("NOPE"))
-            self.assertEqual(os.environ.get("SECOND"), "two_words")
-        finally:
-            os.remove(file_path)
-
-    def test_cmd_source_missing_file_returns_error(self):
-        """Return an error and stderr message when the source file is missing."""
-        missing = "/tmp/lshell_missing_source_file"
-        if os.path.exists(missing):
-            os.remove(missing)
-        stderr = io.StringIO()
-        with redirect_stderr(stderr):
-            self.assertEqual(builtincmd.cmd_source(missing), 1)
-        self.assertIn("lshell: unable to read environment file", stderr.getvalue())
-
     def test_cmd_bg_fg_no_jobs(self):
         """Report failure when attempting fg with no jobs queued."""
         stdout = io.StringIO()

test/test_source_command_unit.py

@@ -0,0 +1,69 @@
+"""Unit tests for the source built-in command."""
+
+import io
+import os
+import tempfile
+import unittest
+from contextlib import redirect_stderr
+from unittest.mock import patch
+
+from lshell import builtincmd
+
+TOPDIR = os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))
+SOURCE_FIXTURE = f"{TOPDIR}/test/testfiles/source_command_fixture.lsh"
+
+
+class TestSourceCommand(unittest.TestCase):
+    """Tests for sourcing environment files into the current shell context."""
+
+    @patch.dict(os.environ, {}, clear=True)
+    def test_cmd_source_loads_fixture_exports(self):
+        """Load exported values from a checked-in source fixture."""
+        self.assertEqual(builtincmd.cmd_source(SOURCE_FIXTURE), 0)
+        self.assertEqual(os.environ.get("SOURCE_SIMPLE"), "value")
+        self.assertEqual(os.environ.get("SOURCE_SINGLE_QUOTED"), "two words")
+        self.assertEqual(os.environ.get("SOURCE_DOUBLE_QUOTED"), "hello world")
+        self.assertEqual(os.environ.get("SOURCE_EMPTY"), "")
+        self.assertEqual(os.environ.get("SOURCE_WITH_EQUALS"), "a=b=c")
+        self.assertIsNone(os.environ.get("IGNORED_ASSIGNMENT"))
+
+    def test_cmd_source_missing_file_returns_error(self):
+        """Return an error and stderr message when the source file is missing."""
+        missing = "/tmp/lshell_missing_source_file"
+        if os.path.exists(missing):
+            os.remove(missing)
+        stderr = io.StringIO()
+        with redirect_stderr(stderr):
+            self.assertEqual(builtincmd.cmd_source(missing), 1)
+        self.assertIn("lshell: unable to read environment file", stderr.getvalue())
+
+    @patch.dict(os.environ, {}, clear=True)
+    def test_cmd_source_preserves_quoted_values_with_spaces(self):
+        """Load quoted export values without truncating them at the first space."""
+        with tempfile.NamedTemporaryFile("w", delete=False) as env_file:
+            env_file.write('export GREETING="hello world"\n')
+            env_file.write("export TARGET='two words here'\n")
+            file_path = env_file.name
+
+        try:
+            self.assertEqual(builtincmd.cmd_source(file_path), 0)
+            self.assertEqual(os.environ.get("GREETING"), "hello world")
+            self.assertEqual(os.environ.get("TARGET"), "two words here")
+        finally:
+            os.remove(file_path)
+
+    @patch.dict(os.environ, {}, clear=True)
+    def test_cmd_source_expands_tilde_paths(self):
+        """Resolve home-relative source paths the same way the shell does."""
+        with tempfile.TemporaryDirectory(dir=".") as home_dir:
+            file_path = os.path.join(home_dir, ".lshell_env")
+            with open(file_path, "w", encoding="utf-8") as env_file:
+                env_file.write("export HOME_SCOPED=value\n")
+
+            with patch.dict(os.environ, {"HOME": home_dir}, clear=True):
+                self.assertEqual(builtincmd.cmd_source("~/.lshell_env"), 0)
+                self.assertEqual(os.environ.get("HOME_SCOPED"), "value")
+
+
+if __name__ == "__main__":
+    unittest.main()

test/testfiles/source_command_fixture.lsh

@@ -0,0 +1,10 @@
+# Source fixture for lshell tests.
+# Only exported assignments should be loaded into the shell environment.
+
+IGNORED_ASSIGNMENT=should_not_be_imported
+
+export SOURCE_SIMPLE=value
+export SOURCE_SINGLE_QUOTED='two words'
+export SOURCE_DOUBLE_QUOTED="hello world"
+export SOURCE_EMPTY=
+export SOURCE_WITH_EQUALS='a=b=c'