nix develop: parse multi-line vars in get-env.sh

[impl]

The get-env.sh script parses the output of declare -p line-by-line, but some shell variables can span multiple lines. In this case, if one of the variables is script-like (for example, shellHook), it can potentially pollute the derivation environment or fail outright. A trivial reproduction of the issue looks like:

{
  # ...
  shellHook = ''
    declare -x VAR=FOO
    declare -x "TEST_$VAR"=BAR
  '';
}

To avoid this problem, we teach get-env.sh to identify and ignore the content of multi-line string values when parsing variable declarations.

[edolstra]

Hm, not sure I understand the problem description. nix develop already support multi-line strings, e.g. in Nix's own flake:

$ nix develop
$ echo "$shellHook"
PATH=$prefix/bin:$PATH
unset PYTHONPATH
export MANPATH=$out/share/man:$MANPATH

# Make bash completion work.
XDG_DATA_DIRS+=:$out/share

[impl]

Hi Eelco,

Thanks for taking a look! Let me see if I can provide a bit more detail for you.

You can carefully craft a value for shellHook (or, in my case, incidentally craft, and discover this bug) that pollutes the variables found by the get-env.sh script. The problem is that the script parses each line as if it's at the top-level scope, without regard for quoting. For example, if I set:

{
  shellHook = ''
    echo foo
    declare -x I_SHOULD_NOT_BE_HERE=NOPE
  '';
}

Then we read, line-by-line,

declare -x shellHook="

echo foo

declare -x I_SHOULD_NOT_BE_HERE=NOPE

"

Normally, it isn't a problem because the regex on line 41 will skip over this out-of-scope multi-line content (this happens to the lines echo foo and " of our example), but if a line happens to also contain declare ..., then it gets treated as if it were part of the overall derivation environment! To validate:

$ nix print-dev-env --json | jq '.variables | has("I_SHOULD_NOT_BE_HERE")'
true

The fix I've proposed just makes the script skip over the content of multi-line variables by parsing them as a string unto themselves, rather than allowing the outer loop to read their content line-by-line. Does that help clarify a bit?

Edit: Note that this doesn't prevent us from returning the correct content of shellHook to the derivation environment, as that is evaluated separately using ${!__var_name}.

[impl]

Hi @edolstra,

Anything else I can do to help give clarity on this change? Happy to hop on Matrix or IRC to explain if it'd be more helpful to do it synchronously too. Let me know!

Thanks!