Skip to content

Commit 6118fd6

Browse files
committed
gh-140006: Harden fish prompt against shadowed builtins
A user function that shadows the `.`/`source` builtin hijacks the activate.fish prompt. fish resolves functions ahead of builtins, so the `echo "exit $status" | .` line that restores the exit status pipes into the user function instead of sourcing. Dot-style directory navigators redefine `.`, which made the prompt list the directory on every command and dropped the exit status handed to the original prompt. Route every builtin the prompt path uses (`source`, `echo`, `printf`, `set_color`, `functions`) through `builtin` so no user function can intercept them. These have all been fish builtins with a stable interface since fish 2.0.0, the version the script already required through `functions --copy`, so the minimum supported fish version does not change.
1 parent e28a2f4 commit 6118fd6

3 files changed

Lines changed: 55 additions & 8 deletions

File tree

Lib/test/test_venv.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,46 @@ def test_special_chars_csh(self):
553553
self.assertTrue(env_name.encode() in lines[0])
554554
self.assertEndsWith(lines[1], env_name.encode())
555555

556+
# gh-140006: the fish prompt override must keep working when a user
557+
# function shadows a builtin it relies on.
558+
@unittest.skipIf(os.name == 'nt', 'fish is not available on Windows')
559+
def test_fish_activate_shadowed_builtins(self):
560+
"""
561+
The fish prompt override restores the exit status through `source` and
562+
prints through `printf`/`echo`/`set_color`. A user function that
563+
shadows one of those builtins (a common pattern for `.`-style directory
564+
navigators) must not hijack the prompt or break status restoration.
565+
"""
566+
fish = shutil.which('fish')
567+
if fish is None:
568+
self.skipTest('fish required for this test')
569+
rmtree(self.env_dir)
570+
builder = venv.EnvBuilder(clear=True)
571+
builder.create(self.env_dir)
572+
activate = os.path.join(self.env_dir, self.bindir, 'activate.fish')
573+
test_script = os.path.join(self.env_dir, 'test_shadowed_builtins.fish')
574+
with open(test_script, "w") as f:
575+
f.write(
576+
# The pre-existing prompt reports the status it receives;
577+
# activation copies it to _old_fish_prompt.
578+
'function fish_prompt; builtin echo "OLDSTATUS=$status"; end\n'
579+
f'source {shlex.quote(activate)}\n'
580+
# Shadow every builtin the override uses. A dot-navigator that
581+
# lists the directory is the reported failure.
582+
'function .; builtin echo DOT_LEAK; end\n'
583+
'function source; builtin echo SOURCE_LEAK; end\n'
584+
'function echo; command echo ECHO_LEAK; end\n'
585+
'function printf; command printf PRINTF_LEAK; end\n'
586+
'function set_color; command true; end\n'
587+
'function _exit7; return 7; end\n'
588+
'_exit7\n'
589+
'fish_prompt\n'
590+
)
591+
out, err = check_output([fish, '--no-config', test_script])
592+
text = out.decode()
593+
self.assertNotIn('LEAK', text)
594+
self.assertIn('OLDSTATUS=7', text)
595+
556596
# gh-124651: test quoted strings on Windows
557597
@unittest.skipUnless(os.name == 'nt', 'only relevant on Windows')
558598
def test_special_chars_windows(self):

Lib/venv/scripts/common/activate.fish

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,18 @@ function deactivate -d "Exit virtual environment and return to normal shell env
1515
if test -n "$_OLD_FISH_PROMPT_OVERRIDE"
1616
set -e _OLD_FISH_PROMPT_OVERRIDE
1717
# prevents error when using nested fish instances (Issue #93858)
18-
if functions -q _old_fish_prompt
19-
functions -e fish_prompt
20-
functions -c _old_fish_prompt fish_prompt
21-
functions -e _old_fish_prompt
18+
if builtin functions -q _old_fish_prompt
19+
builtin functions -e fish_prompt
20+
builtin functions -c _old_fish_prompt fish_prompt
21+
builtin functions -e _old_fish_prompt
2222
end
2323
end
2424

2525
set -e VIRTUAL_ENV
2626
set -e VIRTUAL_ENV_PROMPT
2727
if test "$argv[1]" != "nondestructive"
2828
# Self-destruct!
29-
functions -e deactivate
29+
builtin functions -e deactivate
3030
end
3131
end
3232

@@ -49,18 +49,21 @@ if test -z "$VIRTUAL_ENV_DISABLE_PROMPT"
4949
# fish uses a function instead of an env var to generate the prompt.
5050

5151
# Save the current fish_prompt function as the function _old_fish_prompt.
52-
functions -c fish_prompt _old_fish_prompt
52+
builtin functions -c fish_prompt _old_fish_prompt
5353

5454
# With the original prompt function renamed, we can override with our own.
55+
# Call every builtin through `builtin` so a user function that shadows
56+
# `printf`, `set_color`, `echo`, or `source`/`.` cannot hijack the prompt
57+
# (Issue #140006).
5558
function fish_prompt
5659
# Save the return status of the last command.
5760
set -l old_status $status
5861

5962
# Output the venv prompt; color taken from the blue of the Python logo.
60-
printf "%s(%s)%s " (set_color 4B8BBE) __VENV_PROMPT__ (set_color normal)
63+
builtin printf "%s(%s)%s " (builtin set_color 4B8BBE) __VENV_PROMPT__ (builtin set_color normal)
6164

6265
# Restore the return status of the previous command.
63-
echo "exit $old_status" | .
66+
builtin echo "exit $old_status" | builtin source -
6467
# Output the original/"old" prompt.
6568
_old_fish_prompt
6669
end
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
The :mod:`venv` ``activate.fish`` script now calls fish builtins through
2+
``builtin`` so a user function that shadows ``.``/``source``, ``echo``,
3+
``printf``, ``set_color``, or ``functions`` can no longer hijack the virtual
4+
environment prompt or break exit-status reporting.

0 commit comments

Comments
 (0)