Skip to content

Commit bb017ff

Browse files
committed
test: run bash subprocess tests with --noprofile --norc (#2306)
A `bash -c` whose parent is a top-level shell sources ~/.bashrc when SSH_CLIENT is set or stdin is a socket, so under SSH with SHLVL unset or 0 the bashrc's output and PATH leaked into the steps under test. Every bash argv under tests/ now comes from tests._support.bash_argv, which adds --noprofile --norc as GitHub Actions does for `shell: bash`. tests/project/test_bash_startup_files_2306.py checks the helper keeps a stand-in bashrc out under both triggers, and that no test module spells out a bash argv by hand.
1 parent b12eb1b commit bb017ff

38 files changed

Lines changed: 245 additions & 69 deletions

‎tests/_support.py‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1205,3 +1205,27 @@ def collect_until(done: 'Callable[[], bool]', grace: float = FINALISER_GRACE) ->
12051205
if time.monotonic() >= deadline:
12061206
return False
12071207
time.sleep(0.05)
1208+
1209+
1210+
#: Flags every ``bash`` a test starts is run with, as GitHub Actions runs a
1211+
#: ``shell: bash`` step (#2306). Without them, a ``bash -c`` whose parent is a
1212+
#: top-level shell sources :file:`~/.bashrc` when ``SSH_CLIENT`` is set or its
1213+
#: stdin is a socket, and that file's output and ``PATH`` leak into the step under
1214+
#: test. A stdin of :data:`subprocess.DEVNULL` does not help: ``SSH_CLIENT`` alone
1215+
#: is enough.
1216+
BASH_STARTUP = ('--noprofile', '--norc')
1217+
1218+
1219+
def bash_argv(*args: str, executable: str = 'bash') -> 'list[str]':
1220+
"""The argv running ``bash`` with ``args`` and no startup files.
1221+
1222+
Args:
1223+
*args: What follows :data:`BASH_STARTUP`, such as ``'-e', '-c', script``.
1224+
executable: The ``bash`` to run, such as a path :func:`shutil.which`
1225+
resolved.
1226+
1227+
Returns:
1228+
``executable``, then :data:`BASH_STARTUP`, then ``args``.
1229+
1230+
"""
1231+
return [executable, *BASH_STARTUP, *args]

‎tests/project/test_apt_packages_1967.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
from typing import TYPE_CHECKING
3939
from unittest import mock
4040

41+
from tests._support import bash_argv
4142
from tests._workflows import WORKFLOWS, load, steps
4243
from tests.project.test_ktlo_legs_stay_green_1672 import KTLO_COMMANDS, commands
4344
from tests.project.test_unit_tests_queue import IMAGE_PYTHON
@@ -610,7 +611,7 @@ def both(self, job: 'str', step: 'str', reference: 'str', prepare: 'Callable[[],
610611
for text in (shell, script):
611612
self.reset()
612613
prepare()
613-
result = subprocess.run(['bash', '-e', '-c', text], cwd=ROOT, env=self.env(**env), # nosec: B603
614+
result = subprocess.run(bash_argv('-e', '-c', text), cwd=ROOT, env=self.env(**env), # nosec: B603
614615
stdout=subprocess.PIPE, stderr=subprocess.STDOUT, check=False, timeout=120)
615616
found.append((result.returncode, result.stdout.decode('utf-8', 'replace'), *self.state()))
616617
return found[0], found[1]
@@ -676,7 +677,7 @@ def restored() -> None:
676677
with self.subTest(job=job, restored=prepare is restored):
677678
self.reset()
678679
prepare()
679-
result = subprocess.run(['bash', '-e', '-c', script], cwd=ROOT, env=self.env(), # nosec: B603
680+
result = subprocess.run(bash_argv('-e', '-c', script), cwd=ROOT, env=self.env(), # nosec: B603
680681
stdout=subprocess.PIPE, stderr=subprocess.STDOUT, check=False,
681682
timeout=120)
682683
output = result.stdout.decode('utf-8', 'replace')
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
# -*- coding: utf-8 -*-
2+
"""#2306: every ``bash`` a test starts skips the startup files, as a runner's does.
3+
4+
A ``bash -c`` whose parent is a top-level shell sources :file:`~/.bashrc` when
5+
``SSH_CLIENT`` is set or its stdin is a socket. Run from an SSH session, that
6+
file's output and ``PATH`` leak into the step under test: a stray line in output a
7+
test compares, or the real ``gh`` found ahead of a stub. GitHub Actions runs a
8+
``shell: bash`` step as ``bash --noprofile --norc``, and the tests run theirs
9+
through :func:`tests._support.bash_argv`, which does the same. Pinned here:
10+
11+
* :class:`TestTheHelperSkipsTheStartupFiles`: with a socket on stdin and
12+
``SSH_CLIENT`` set, a plain ``bash -c`` sources a stand-in :file:`~/.bashrc`, and
13+
the helper's argv does not;
14+
* :class:`TestEveryBashGoesThroughTheHelper`: no module under :file:`tests/` spells
15+
out a ``bash`` argv of its own. This module is exempt, since its control run is
16+
the plain ``bash -c`` the others must not be.
17+
18+
"""
19+
20+
from __future__ import annotations
21+
22+
import ast
23+
import os
24+
import pathlib
25+
import shutil
26+
import socket
27+
import subprocess # nosec: B404
28+
import tempfile
29+
import unittest
30+
31+
from tests import _support
32+
33+
#: Test tier (#1958): reads the test sources and runs ``bash``.
34+
__tier__ = 'LATEST'
35+
36+
TESTS = pathlib.Path(__file__).resolve().parents[1]
37+
BASH = shutil.which('bash')
38+
39+
#: What the stand-in :file:`~/.bashrc` prints, so a run that sourced it shows it.
40+
MARKER = 'pcapkit-2306-bashrc-ran'
41+
42+
43+
@unittest.skipIf(os.name == 'nt' or BASH is None, 'needs bash and a POSIX socket on stdin')
44+
class TestTheHelperSkipsTheStartupFiles(unittest.TestCase):
45+
"""The flags :func:`~tests._support.bash_argv` adds are what keeps the bashrc out."""
46+
47+
def setUp(self) -> None:
48+
tmp = tempfile.TemporaryDirectory(prefix='pcapkit-2306-') # pylint: disable=consider-using-with
49+
self.addCleanup(tmp.cleanup)
50+
home = pathlib.Path(tmp.name)
51+
(home / '.bashrc').write_text(f'echo {MARKER}\n', encoding='utf-8')
52+
# SHLVL 0 makes the child a top-level shell, as one sshd starts is.
53+
self.env = {'HOME': str(home), 'PATH': os.environ.get('PATH', ''),
54+
'SSH_CLIENT': '192.0.2.1 50000 22', 'SHLVL': '0'}
55+
56+
def run_bash(self, argv: 'list[str]') -> 'str':
57+
ours, theirs = socket.socketpair()
58+
self.addCleanup(ours.close)
59+
self.addCleanup(theirs.close)
60+
result = subprocess.run(argv, stdin=theirs, env=self.env, capture_output=True, # nosec: B603
61+
text=True, check=False, timeout=_support.scale_timeout(30))
62+
self.assertEqual(result.returncode, 0, result.stderr)
63+
return result.stdout
64+
65+
def test_a_plain_bash_sources_the_bashrc(self) -> None:
66+
self.assertIn(MARKER, self.run_bash([str(BASH), '-c', 'echo step']))
67+
68+
def test_the_helper_does_not(self) -> None:
69+
self.assertEqual(self.run_bash(_support.bash_argv('-e', '-c', 'echo step', executable=str(BASH))), 'step\n')
70+
71+
72+
def names_bash(node: 'ast.expr') -> 'bool':
73+
"""Whether ``node`` is ``'bash'``, a path to it, or ``BASH`` as the tests resolve it."""
74+
if isinstance(node, ast.Constant) and isinstance(node.value, str):
75+
return node.value.replace('\\', '/').rsplit('/', 1)[-1] in ('bash', 'bash.exe')
76+
if isinstance(node, ast.Name):
77+
return node.id == 'BASH'
78+
if isinstance(node, ast.Call) and node.args:
79+
callee = node.func.id if isinstance(node.func, ast.Name) else getattr(node.func, 'attr', None)
80+
return callee in ('str', 'which') and names_bash(node.args[0])
81+
if isinstance(node, ast.BoolOp):
82+
return any(names_bash(value) for value in node.values)
83+
return False
84+
85+
86+
def is_bash_argv(node: 'ast.AST') -> 'bool':
87+
"""A list or tuple opening with a ``bash``, and not a list of tool names."""
88+
if not (isinstance(node, (ast.List, ast.Tuple)) and len(node.elts) > 1 and names_bash(node.elts[0])):
89+
return False
90+
return not all(isinstance(item, ast.Constant)
91+
and (item.value is None or isinstance(item.value, str) and not item.value.startswith('-'))
92+
for item in node.elts[1:])
93+
94+
95+
class TestEveryBashGoesThroughTheHelper(unittest.TestCase):
96+
"""A ``bash`` argv written out by hand is one that can drop the flags."""
97+
98+
def test_no_module_spells_out_a_bash_argv(self) -> None:
99+
found = []
100+
for path in sorted(TESTS.rglob('*.py')):
101+
if path.resolve() == pathlib.Path(__file__).resolve():
102+
continue
103+
tree = ast.parse(path.read_text(encoding='utf-8'), str(path))
104+
found.extend(f'{path.relative_to(TESTS.parent).as_posix()}:{node.lineno}: {ast.unparse(node)}'
105+
for node in ast.walk(tree) if is_bash_argv(node))
106+
self.assertEqual(found, [], 'build these with tests._support.bash_argv')
107+
108+
def test_the_scan_sees_the_shapes_it_replaced(self) -> None:
109+
for source in ("['bash', '-e', '-c', script]", "(str(BASH), '-e')", "[BASH or 'bash', *flags]",
110+
"['/bin/bash', str(path), py]", "[str(shutil.which('bash')), '-c', 'true']"):
111+
with self.subTest(source=source):
112+
self.assertTrue(is_bash_argv(ast.parse(source, mode='eval').body))
113+
for source in ("('bash', 'timeout', 'prlimit')", "('bash', None)", "bash_argv('-e', '-c', script)"):
114+
with self.subTest(source=source):
115+
self.assertFalse(is_bash_argv(ast.parse(source, mode='eval').body))
116+
117+
118+
if __name__ == '__main__':
119+
unittest.main()

‎tests/project/test_benchmark_workflow_2125.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import unittest
3838
from typing import TYPE_CHECKING
3939

40+
from tests._support import bash_argv
4041
from tests._workflows import WORKFLOWS, job, load, steps
4142

4243
try:
@@ -59,7 +60,7 @@
5960
UPDATER = ROOT / 'util' / 'update_python_versions.py'
6061
BASH = shutil.which('bash')
6162
#: How ``shell: bash`` runs a step's script, ahead of the script.
62-
SHELL = [BASH or 'bash', '--noprofile', '--norc', '-eo', 'pipefail', '-c']
63+
SHELL = bash_argv('-eo', 'pipefail', '-c', executable=BASH or 'bash')
6364
TITLE = 'benchmark: weekly engine benchmark report'
6465
INPUTS = {'runner', 'pythons', 'engines', 'rounds', 'repeats'}
6566
MEASURE = 'Run examples/benchmark/native.sh'

‎tests/project/test_codeql_report_2030.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import unittest
3333
from typing import TYPE_CHECKING
3434

35+
from tests._support import bash_argv
3536
from tests._workflows import WORKFLOWS, load, pinned
3637
from tests.project.test_discussion_report_2030 import (BASH, CATEGORY, FAKE_PYTHON, MARKER, RUN, SCRIPT,
3738
_FakeGitHub, evaluate)
@@ -64,7 +65,7 @@ def run_step(name: 'str', cwd: 'pathlib.Path', env: 'dict[str, str]') -> 'subpro
6465
"""Step ``name``'s ``run:`` as a bash step runs it."""
6566
script = cwd.parent / 'step.sh'
6667
script.write_text(named(name)['run'], encoding='utf-8')
67-
return subprocess.run([str(BASH), '--noprofile', '--norc', '-eo', 'pipefail', str(script)], # nosec: B603
68+
return subprocess.run(bash_argv('-eo', 'pipefail', str(script), executable=str(BASH)), # nosec: B603
6869
cwd=cwd, env=env, capture_output=True, text=True, check=False, timeout=120)
6970

7071

‎tests/project/test_compat_util_floor_2206.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
from typing import TYPE_CHECKING
4343
from unittest import mock
4444

45+
from tests._support import bash_argv
4546
from tests._workflows import job
4647
from tests.project.test_python_matrix_tiers_1633 import legs
4748
from tests.project.test_release_gates import evaluate_condition
@@ -127,7 +128,7 @@ def run_step(cwd: 'pathlib.Path', tmp: 'pathlib.Path') -> 'subprocess.CompletedP
127128
(bindir / 'python').symlink_to(sys.executable)
128129
env = dict(os.environ, PATH=f'{bindir}{os.pathsep}{os.environ["PATH"]}', RUNNER_TEMP=str(tmp),
129130
ANNOTATION_LEVEL='warning', LEG=LEG)
130-
return subprocess.run(['bash', '--noprofile', '--norc', '-eo', 'pipefail', '-c', step()['run']],
131+
return subprocess.run(bash_argv('-eo', 'pipefail', '-c', step()['run']),
131132
cwd=cwd, env=env, capture_output=True, text=True, check=False, timeout=60)
132133

133134

‎tests/project/test_conda_publish_dispatch_2060.py‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
from typing import TYPE_CHECKING
4343

4444
from tests import _workflows
45+
from tests._support import bash_argv
4546
from tests._workflows import WORKFLOWS
4647
from tests.project.test_build_only_dispatch import MARKERS
4748
from tests.project.test_cron_reports_2030 import _ReportStep
@@ -159,8 +160,8 @@ def decide(self, event_name: 'str', publish: 'str' = '',
159160
self.output.write_text('', encoding='utf-8')
160161
env = {'PATH': os.environ.get('PATH', ''), 'GITHUB_OUTPUT': str(self.output),
161162
'GITHUB_EVENT_NAME': event_name, 'GITHUB_REF': ref, 'PUBLISH': publish}
162-
result = subprocess.run([str(BASH), '-e', str(self.script)], cwd=self.tmp, env=env, # nosec: B603
163-
capture_output=True, text=True, check=False, timeout=60)
163+
result = subprocess.run(bash_argv('-e', str(self.script), executable=str(BASH)), # nosec: B603
164+
cwd=self.tmp, env=env, capture_output=True, text=True, check=False, timeout=60)
164165
return result.returncode, self.output.read_text(encoding='utf-8'), result.stdout
165166

166167
def test_every_other_event_publishes(self) -> None:
@@ -314,8 +315,8 @@ def test_the_decision(self) -> None:
314315
output.write_text('', encoding='utf-8')
315316
env = {'PATH': os.environ.get('PATH', ''), 'GITHUB_OUTPUT': str(output),
316317
'FILES_CHANGED': files_changed, 'FORCED': forced}
317-
result = subprocess.run([str(BASH), '-e', '-c', script], cwd=tmp, env=env, # nosec: B603
318-
capture_output=True, text=True, check=False, timeout=60)
318+
result = subprocess.run(bash_argv('-e', '-c', script, executable=str(BASH)), # nosec: B603
319+
cwd=tmp, env=env, capture_output=True, text=True, check=False, timeout=60)
319320
with self.subTest(files_changed=files_changed, forced=forced):
320321
self.assertEqual(result.returncode, 0, result.stderr)
321322
self.assertEqual(output.read_text(encoding='utf-8'), f'bump={expected}\nforced={forced}\n')

‎tests/project/test_cron_reports_2030.py‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import unittest
3333
from typing import TYPE_CHECKING
3434

35+
from tests._support import bash_argv
3536
from tests._workflows import WORKFLOWS, load, pinned
3637
from tests.project.test_discussion_report_2030 import (BASH, CATEGORY, FAKE_PYTHON, FIRST_PARTY,
3738
POSTER, RUN, evaluate)
@@ -260,8 +261,8 @@ def run_step(self, code: 'int') -> 'tuple[subprocess.CompletedProcess[str], str]
260261
script = root / 'step.sh'
261262
script.write_text(step(CONDA, 'conda-update', 'Refresh the platform pins')['run'], encoding='utf-8')
262263
env = {'PATH': f'{root}{os.pathsep}{os.environ.get("PATH", "")}', 'RUNNER_TEMP': str(root)}
263-
result = subprocess.run([str(BASH), '-e', str(script)], cwd=root, env=env, # nosec: B603
264-
capture_output=True, text=True, check=False, timeout=60)
264+
result = subprocess.run(bash_argv('-e', str(script), executable=str(BASH)), # nosec: B603
265+
cwd=root, env=env, capture_output=True, text=True, check=False, timeout=60)
265266
return result, (root / 'platform-pins.log').read_text(encoding='utf-8')
266267

267268
def test_the_log_keeps_the_annotations(self) -> None:
@@ -301,8 +302,8 @@ def report(self, **env: 'str') -> 'tuple[str, str]':
301302
self.output.write_text('', encoding='utf-8')
302303
env = dict(env, PATH=f'{self.bin}{os.pathsep}{os.environ.get("PATH", "")}', RUNNER_TEMP=str(self.temp),
303304
GITHUB_OUTPUT=str(self.output))
304-
result = subprocess.run([str(BASH), '-e', str(self.script)], cwd=self.work, env=env, # nosec: B603
305-
capture_output=True, text=True, check=False, timeout=60)
305+
result = subprocess.run(bash_argv('-e', str(self.script), executable=str(BASH)), # nosec: B603
306+
cwd=self.work, env=env, capture_output=True, text=True, check=False, timeout=60)
306307
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
307308
text = (self.temp / self.file).read_text(encoding='utf-8')
308309
self.assertEqual(result.stdout, text)
@@ -487,8 +488,8 @@ def run_step(self, name: 'str', artifact: 'Optional[str]', **env: 'str') -> 'tup
487488
script.write_text(step(name, 'discussion', 'Post the report to Discussions')['run'], encoding='utf-8')
488489
env = dict(env, PATH=f'{self.bin}{os.pathsep}{os.environ.get("PATH", "")}', FAKE_PYTHON_LOG=str(self.log),
489490
REPORT_CATEGORY=CATEGORY)
490-
result = subprocess.run([str(BASH), '-e', str(script)], cwd=work, env=env, # nosec: B603
491-
capture_output=True, text=True, check=False, timeout=60)
491+
result = subprocess.run(bash_argv('-e', str(script), executable=str(BASH)), # nosec: B603
492+
cwd=work, env=env, capture_output=True, text=True, check=False, timeout=60)
492493
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
493494
logged = json.loads(self.log.read_text(encoding='utf-8'))
494495
args = logged['args']

‎tests/project/test_discussion_report_2030.py‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import unittest
3737
from typing import TYPE_CHECKING
3838

39+
from tests._support import bash_argv
3940
from tests._workflows import WORKFLOWS, load, pinned
4041
from tests.project.test_release_gates import STATUS, job_blocks, job_runs
4142

@@ -722,8 +723,8 @@ def run_step(self, head: 'str', vendor: 'str', status: 'str' = '',
722723
env = {'PATH': f'{self.bin}{os.pathsep}{os.environ.get("PATH", "")}', 'FAKE_PYTHON_LOG': str(self.log),
723724
'REPORT_CATEGORY': CATEGORY, 'MODE': 'scheduled', 'HEAD_RESULT': head, 'VENDOR_RESULT': vendor,
724725
'VENDOR_STATUS': status}
725-
result = subprocess.run([str(BASH), '-e', str(self.script)], cwd=self.work, env=env, # nosec: B603
726-
capture_output=True, text=True, check=False, timeout=60)
726+
result = subprocess.run(bash_argv('-e', str(self.script), executable=str(BASH)), # nosec: B603
727+
cwd=self.work, env=env, capture_output=True, text=True, check=False, timeout=60)
727728
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
728729
logged = json.loads(self.log.read_text(encoding='utf-8'))
729730
args = logged['args']
@@ -789,7 +790,7 @@ def report(self, stderr: 'str' = '', **env: 'str') -> 'tuple[str, str]':
789790
"""The report, and the ``status`` the step wrote to ``GITHUB_OUTPUT``."""
790791
(self.temp / 'vendor-stderr.log').write_text(stderr, encoding='utf-8')
791792
self.output.write_text('', encoding='utf-8')
792-
result = subprocess.run([str(BASH), '-e', str(self.script)], cwd=self.repo, # nosec: B603
793+
result = subprocess.run(bash_argv('-e', str(self.script), executable=str(BASH)), cwd=self.repo, # nosec: B603
793794
env=dict(self.env, **env), capture_output=True, text=True, check=False,
794795
timeout=60)
795796
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)

‎tests/project/test_gnu_timeout_2020.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
from typing import TYPE_CHECKING
3434

3535
from tests import _workflows
36+
from tests._support import bash_argv
3637
from tests.project.test_resource_monitor_1740 import DEFAULT, TESTS, sh, step
3738

3839
if TYPE_CHECKING:
@@ -165,7 +166,8 @@ def stub(self, name: 'str', body: 'str', bindir: 'pathlib.Path | None' = None) -
165166

166167
def resolve(self, path: 'str') -> 'str':
167168
# bash by its own path, since ``path`` holds the stubs alone.
168-
result = sh(RESOLVE + 'echo "$gnu_timeout"\n', {'PATH': path}, (str(shutil.which('bash')), '-e'))
169+
result = sh(RESOLVE + 'echo "$gnu_timeout"\n', {'PATH': path},
170+
tuple(bash_argv('-e', executable=str(shutil.which('bash')))))
169171
self.assertEqual(result.returncode, 0, result.stderr)
170172
return result.stdout.strip()
171173

0 commit comments

Comments
 (0)