Fix 243417: sanitize env command output for bash integration

[ryenus]

In the current bash shell integration implementation, it only considers the lines of simple key/value pairs while processing the output of the env command. However, when there're exported shell functions, which are typically printed over multiple lines, there could be lines with arbitrary shell script that might lead to unexpected result, causing errors like:

bash: vsc_aa_env: bad array subscript
bash: currentEnvMap["$key"]: bad array subscript

Fortunately with env -0, it becomes possible to join the lines of an exported function into one line with a custom line separator, such as '\r', or any other unused ascii control characters. With that we could also exclude the exported functions from code completion suggestions:

Fixes #243417

[Tyriar]

@anthonykim1 could you review+test this one?

[anthonykim1]

Hi @ryenus Thanks much for putting up this PR.

I have to admit, we did suffer through some pain in the bash shell integration script where we had bunch of debris from user's env show up in the terminal.

I did make some changes recently to resolve this and separate the envs with null delimiter here
Relevant PR here

Would you be able to check on the latest vscode-insiders version how this would behave on your setup?
I'm wondering if the check I've merged is smart enough.

I feel like it may still be worth taking the suggestion from BASH_FUNC, but still worth checking first.
Thanks,

[ryenus]

I feel like it may still be worth taking the suggestion from BASH_FUNC, but still worth checking first. Thanks,

@anthonykim1, good that you brought this up, please consider these:

1. The exported bash functions are excluded by bash completion
2. How to use the suggested $BASH_FUNC_<function_name>%% variables? For what?

With that I'd suggest to NOT include the BASH_FUNC entries, and the code would performs better with less entries to process.

Meanwhile please also checkout the fixes in minor shell integration code style fix (cc42b1d). While env -0 might have fixed the unbound variable errors, we'd still need to avoid leaking function local variables that might unexpected overwrite existing shell variables with the same name(s).

[anthonykim1]

Thanks @ryenus
Do you mind revisiting this after resolving merge conflicts? There were bunch of changes in this script over the past months.

[ryenus]

Do you mind revisiting this after resolving merge conflicts? There were bunch of changes in this script over the past months.

@anthonykim1, may I ask what would you expect as of now?

1. I can see that you had already taken the idea of using env -0, as you mentioned earlier:

   I did make some changes recently to resolve this and separate the envs with null delimiter here
   Relevant PR Fix prompt glitch on zsh, bash when environment reporting is set to true #243804

2. meanwhile the original issue no longer exists since you now use a fixed set of env vars as with:
   

   vscode/src/vs/platform/terminal/node/terminalEnvironment.ts

   Line 102 in cb99fc2

   const scopedDownShellEnvs = ['PATH', 'VIRTUAL_ENV', 'HOME', 'SHELL', 'PWD'];

[anthonykim1]

@ryenus Sounds good, just wanted to make sure I didn't miss anything before I close this out.
btw env -0 was really useful.

# Filtered env output without exported functions
 __vsc_env() {
 	command env -0 | tr '\0\n' '\n\r' | sort | grep -Ev '^BASH_FUNC'
 }

May come in handy in the future when we make the env setting configurable as opposed to being the "hardcoded" list right now.

I'd /cc you then :)