From 61326cd79fc26168690625cd5a8d8dfc6eeb3deb Mon Sep 17 00:00:00 2001 From: Ignace Mouzannar Date: Fri, 6 Mar 2026 22:40:56 -0500 Subject: [PATCH 1/3] Fix source command: cmd_export and cmd_source to handle quoted values and tilde expansion in environment variables --- lshell/builtincmd.py | 37 ++++++++++++++++++----------------- test/test_parser_jobs_unit.py | 27 +++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 18 deletions(-) diff --git a/lshell/builtincmd.py b/lshell/builtincmd.py index 7732743..25a20ff 100644 --- a/lshell/builtincmd.py +++ b/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 diff --git a/test/test_parser_jobs_unit.py b/test/test_parser_jobs_unit.py index 3a10c44..b824a84 100644 --- a/test/test_parser_jobs_unit.py +++ b/test/test_parser_jobs_unit.py @@ -272,6 +272,33 @@ def test_cmd_source_missing_file_returns_error(self): 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") + def test_cmd_bg_fg_no_jobs(self): """Report failure when attempting fg with no jobs queued.""" stdout = io.StringIO() From 8824e0378ae612ee68172bcc12e2a99f40ba5cff Mon Sep 17 00:00:00 2001 From: Ignace Mouzannar Date: Sat, 7 Mar 2026 17:32:45 -0500 Subject: [PATCH 2/3] Add pylint + flak8 to docker-compose --- docker-compose.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index c1c41d2..def07d6 100644 --- a/docker-compose.yml +++ b/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: From 1d136bfdadbffc567cd75a37d19903551c030cdf Mon Sep 17 00:00:00 2001 From: Ignace Mouzannar Date: Sat, 7 Mar 2026 17:35:30 -0500 Subject: [PATCH 3/3] Add unit tests for source command and create fixture for environment variables --- test/test_builtins.py | 25 +++----- test/test_parser_jobs_unit.py | 58 +------------------ test/test_source_command_unit.py | 69 +++++++++++++++++++++++ test/testfiles/source_command_fixture.lsh | 10 ++++ 4 files changed, 89 insertions(+), 73 deletions(-) create mode 100644 test/test_source_command_unit.py create mode 100644 test/testfiles/source_command_fixture.lsh diff --git a/test/test_builtins.py b/test/test_builtins.py index b65d162..894a447 100644 --- a/test/test_builtins.py +++ b/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 diff --git a/test/test_parser_jobs_unit.py b/test/test_parser_jobs_unit.py index b824a84..56d45c7 100644 --- a/test/test_parser_jobs_unit.py +++ b/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,60 +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()) - - @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") - def test_cmd_bg_fg_no_jobs(self): """Report failure when attempting fg with no jobs queued.""" stdout = io.StringIO() diff --git a/test/test_source_command_unit.py b/test/test_source_command_unit.py new file mode 100644 index 0000000..20effc9 --- /dev/null +++ b/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() diff --git a/test/testfiles/source_command_fixture.lsh b/test/testfiles/source_command_fixture.lsh new file mode 100644 index 0000000..de53b6f --- /dev/null +++ b/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'