Skip to content

Commit 0d540af

Browse files
authored
gh-140006: Harden fish prompt against shadowed builtins (#150936)
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 b86c305 commit 0d540af

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
@@ -773,6 +773,46 @@ def test_special_chars_csh(self):
773773
self.assertTrue(env_name.encode() in lines[0])
774774
self.assertEndsWith(lines[1], env_name.encode())
775775

776+
# gh-140006: the fish prompt override must keep working when a user
777+
# function shadows a builtin it relies on.
778+
@unittest.skipIf(os.name == 'nt', 'fish is not available on Windows')
779+
def test_fish_activate_shadowed_builtins(self):
780+
"""
781+
The fish prompt override restores the exit status through `source` and
782+
prints through `printf`/`echo`/`set_color`. A user function that
783+
shadows one of those builtins (a common pattern for `.`-style directory
784+
navigators) must not hijack the prompt or break status restoration.
785+
"""
786+
fish = shutil.which('fish')
787+
if fish is None:
788+
self.skipTest('fish required for this test')
789+
rmtree(self.env_dir)
790+
builder = venv.EnvBuilder(clear=True)
791+
builder.create(self.env_dir)
792+
activate = os.path.join(self.env_dir, self.bindir, 'activate.fish')
793+
test_script = os.path.join(self.env_dir, 'test_shadowed_builtins.fish')
794+
with open(test_script, "w") as f:
795+
f.write(
796+
# The pre-existing prompt reports the status it receives;
797+
# activation copies it to _old_fish_prompt.
798+
'function fish_prompt; builtin echo "OLDSTATUS=$status"; end\n'
799+
f'source {shlex.quote(activate)}\n'
800+
# Shadow every builtin the override uses. A dot-navigator that
801+
# lists the directory is the reported failure.
802+
'function .; builtin echo DOT_LEAK; end\n'
803+
'function source; builtin echo SOURCE_LEAK; end\n'
804+
'function echo; command echo ECHO_LEAK; end\n'
805+
'function printf; command printf PRINTF_LEAK; end\n'
806+
'function set_color; command true; end\n'
807+
'function _exit7; return 7; end\n'
808+
'_exit7\n'
809+
'fish_prompt\n'
810+
)
811+
out, err = check_output([fish, '--no-config', test_script])
812+
text = out.decode()
813+
self.assertNotIn('LEAK', text)
814+
self.assertIn('OLDSTATUS=7', text)
815+
776816
# gh-124651: test quoted strings on Windows
777817
@unittest.skipUnless(os.name == 'nt', 'only relevant on Windows')
778818
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

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

5454
# Save the current fish_prompt function as the function _old_fish_prompt.
55-
functions -c fish_prompt _old_fish_prompt
55+
builtin functions -c fish_prompt _old_fish_prompt
5656

5757
# With the original prompt function renamed, we can override with our own.
58+
# Call every builtin through `builtin` so a user function that shadows
59+
# `printf`, `set_color`, `echo`, or `source`/`.` cannot hijack the prompt
60+
# (Issue #140006).
5861
function fish_prompt
5962
# Save the return status of the last command.
6063
set -l old_status $status
6164

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

6568
# Restore the return status of the previous command.
66-
echo "exit $old_status" | .
69+
builtin echo "exit $old_status" | builtin source -
6770
# Output the original/"old" prompt.
6871
_old_fish_prompt
6972
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)