Quote multi-line values in @SUMMONENVFILE

[mcanevet]

What does this PR do?

If a secret contains a multi-line value, @SUMMONENVFILE is not sourceable because the value must be quoted.
The PR changes way more thing then I expected, but it was needed due to the fact that the formatting is done the same way for environment variables and environment file.

What ticket does this PR close?

Resolves #31

Checklists

Change log

• The CHANGELOG has been updated, or
• This PR does not include user-facing changes and doesn't require a CHANGELOG update

Test coverage

• This PR includes new unit and integration tests to go with the code changes, or
• The changes in this PR do not require tests

Documentation

• Docs (e.g. READMEs) were updated in this PR, and/or there is a follow-on issue to update docs, or
• This PR does not require updating any documentation

[mcanevet]

Will also fix #31 I guess

[diverdane]

Hi @mcanevet,

Thank you for submitting this pull request! This will be a good change to get in, for sure.

I've done a first pass look through the code, and it looks good.

Would you be able to add a test case to this to the internal/command/action_test.go for this? Also, some of the existing unit tests get broken by this change. (To run the UT, do go test from the internal/command/ directory). Let me know if you need any help with this.

Thank you!

[mcanevet]

@diverdane I fixed the tests and I guess the TestJoinEnv test shows that the new code works fine because it quotes the value.

[diverdane]

@mcanevet, I experimented with this some more today, and I realized that we may have some backwards compatibility issues if we add this feature. The concern is that every value in the @SUMMONENVFILE is going to be double-quoted, and Docker treats everything after the = in env files as a literal string, quotes and all.

So, for example, if I have a very basic secrets.yml file (I'm not using Conjur variables here, but behavior would be the same):

FOO: "The rain in Spain"

With Existing Summon

With the existing summon, I see this string without quotes in the @SUMMONENVFILE and in the environment of a container using this:

$ summon cat @SUMMONENVFILE
FOO=The rain in Spain
$ summon docker run --env-file @SUMMONENVFILE alpine sh -c 'echo $FOO'
The rain in Spain
$ summon docker run --env-file @SUMMONENVFILE bash bash -c 'echo $FOO'
The rain in Spain
$ summon docker run --env-file @SUMMONENVFILE -it bash bash
bash-5.1# echo "$FOO"
The rain in Spain
bash-5.1# exit
exit
$

With Quoted Values in SUMMONENVFILE

With a summon that incorporates changes from this PR, everything shows up with quotes:

$ ./new-summon cat @SUMMONENVFILE
FOO="The rain in Spain"
$ ./new-summon docker run --env-file @SUMMONENVFILE alpine sh -c 'echo $FOO'
"The rain in Spain"
$ ./new-summon docker run --env-file @SUMMONENVFILE bash bash -c 'echo $FOO'
"The rain in Spain"
$ ./new-summon docker run --env-file @SUMMONENVFILE -it bash bash
bash-5.1# echo "$FOO"
"The rain in Spain"
bash-5.1# exit
exit
$

I'm concerned that customers currently using Summon + Docker with SUMMONENVFILE might get thrown off by the introduction of the double quotes.

For your use case, would it be possible to use base64 encoded certificates (and base64 decode them in the container)? Or use the -e environment flags for docker run...?

[doodlesbykumbi]

Hi @mcanevet. Thought I'd chime into the conversation since I've just been dealing with the same issue.

A better way to inject secrets from summon into Docker

I was motivated by the need to use the secrets from summon inside a docker container. @SUMMONENVFILE wasn't particularly useful because

1. it doesn't currently handle multiline secret values at all
2. it doesn’t support the !file tag. @SUMMONENVFILE works perfectly fine for single line values though.

I went with the approach below, where I define a bash function summon_envvars_docker_opts that can

1. Extract all the variable names injected by summon from the secrets.yml file
2. Generate -e VAR_NAME options for docker run for all the variables injected by summon
3. Generate -v FILE_PATH:FILE_PATH options for all the variables that point to files managed by summon as a result of !file.

The benefit of (3) is that summon is still managing these files so when it dies then those files are cleaned up by summon.

summon_envvars_docker_opts then allows me to define a convenient wrapper for docker run, see summon_docker_run below. summon_docker_run takes

1. the secrets.yml file via the environment variable secrets_yml,
2. any other summon command line options via the environment variable summon_opts
3. any arguments passed in, i.e. summon_docker run [args], are propagated to docker run i.e. docker run [args]

I've condensed all this functionality under the definition of summon_docker_run mostly for brevity, but it'd probably be best to split things out.

function summon_docker_run() {
  local secrets_yml="${secrets_yml:-"./secrets.yml"}"
  local summon_opts="${summon_opts:-""}"
  local script=$(<< 'EOL'
function summon_envvars_docker_opts() {
  local secrets_yml="${1:-secrets.yml}";

  if ! cat "${secrets_yml}" | sed '/^$/d' | sed '/^ /d' | { grep '^[^#]' || true; } | sed -E 's/^([^:]*)?.*/\1/' | xargs -n 1 sh -c 'printenv $1 > /dev/null' _; then
    echo "this should never happen! some of the summon values are missing" >&2
    exit 1
  fi

  # create the options for the environment variables listed in secrets.yml
  local envs="$(cat "${secrets_yml}" | sed '/^$/d' | sed '/^ /d' | { grep '^[^#]' || true; } | sed -E 's/^([^:]*)?.*/\1/' | xargs printf -- '-e %s ')"

  # create the options for the volume mounts for secrets that use the !file tag in secrets.yml
  local volumemounts="$(cat "${secrets_yml}" | { grep '^[^#]' || true; } | { grep '![^ ]*file' || true; } | sed '/^$/d' | sed '/^ /d' |  sed -E 's/^([^:]*)?.*/\1/' | xargs -n 1 sh -c '[ "$#" -gt 0 ] && printf "%s" "-v $(printenv $1):$(printenv $1) "' _)"

  echo "${envs}" "${volumemounts}"
}

# summon-envvars-docker-opts allows us to populate the container with environment variables
# from summon
docker_opts="$(summon_envvars_docker_opts ${secrets_yml})"
docker run ${docker_opts} "$@"
EOL
)
  secrets_yml="${secrets_yml}" summon $(printf '%s' "${summon_opts}") -f "${secrets_yml}" sh -e -c "${script}" _ "$@"
}

Example usage of summon_docker_run

Below I provide some example usage of summon_docker_run.

# Generate test secrets.yml file i.e. ./test-secrets.yml 
echo '
A: single line
B: |
  multiples
  lines
# some comment
C: !file |
  multi
  line
  file
' > test-secrets.yml

# Run summon_docker_run with /bin/echo as the provider for summon and use ./test-secrets.yml as the secrets.yml file
secrets_yml=./test-secrets.yml summon_opts="--provider /bin/echo" summon_docker_run -it alpine sh -c '
echo "
A=${A}
B=${B}
C=${C}
C=$(cat ${C})
"
'

This results in the following output from within the container. As we can see below multiline values are supported, and variables using the!file tag can be read from within the container.

A=single line
B=multiples
lines

C=/root/.tmp260279238/.summon040455277
C=multi
line
file

[doodlesbykumbi]

A better way to inject secrets from summon into Docker contniued...

My suggestion above is only useful if your goal is to lift summon secrets into a docker container, which is definitely a usecase I've personally come across time and again. This has prompted me to consider supporting @SUMMONDOCKEROPTS , which would mimic my bash wizardry above and whose usage would look like this

summon docker run @SUMMONDOCKEROPTS ...

Supporting multiline values in envfiles generated from summon secrets

Back to envfiles...

There are definitely instances where an envfile is all that's necessary e.g. using dotenv in Node.js which supports multiline values through double quotes (see https://www.npmjs.com/package/dotenv#rules). dotenv in Node.js has a set of rules for double quotes which I'm not sure is ubiquitous. I think it'd be good to survey the landscape of envfiles and see if there's a standard for multiline values that we can adopt. To maintain backwards compatibility we can leave @SUMMONENVFILE unchanged, and perhaps introduce @SUMMONENVFILEWITHMULTILINESUPPORT or something equivalent which would have multiline support.

[mcanevet]

Actually my main goal is to extract all secrets into a dotenv file in a Gitlab CI job, then pass them as an artifact to the next job that will source the dotenv file. I guess @SUMMONENVFILE is not only broken for Docker usage, but also when using source.
@diverdane do you think that if I quote only values containing \n it would do the trick?

[diverdane]

@mcanevet ,

I REALLY like the @doodlesbykumbi 's idea of implementing a @SUMMONDOCKEROPTS command line option to support passing multiline secrets to Docker (using -e under the hood), rather than adding special behavior to get around what's really a (frustrating) limitation in Docker dotenv files.

For your purposes, would it be possible to script this up (or create a Golang exec) by adding post-processing to the existing Summon, e.g. something that does:

• Retrieve secrets from Summon
• Add quotes for the multiline secrets (or all secrets, if that's easier to handle by the next process in your pipeline)

[doodlesbykumbi]

@mcanevet If you're referring to using artifacts:reports:dotenv, as in https://docs.gitlab.com/ee/ci/variables/#inherit-environment-variables, I'm not sure that supports multiline values. The rules for it are more restricted. For example,

Key values in the env file cannot have spaces or newline characters (\n), including when using single or double quotes.

envfiles are tricky.

---

env_summon suggestion for envfiles

I whipped us env_summon which

1. takes the environment variable secrets_yml which specifies the secrets.yml file to use
2. takes the environment variable secrets_output which chooses the output format

env_summon supports 2 output modes

1. Generate an envfile from summon secrets with quoted values (i.e. secrets_output=dot_env)
2. Generate a shell file that can be sourced and exports the environment variables from summon (i.e. secrets_output=dot_env). The values are escaped so that when they are evaluated in a shell they'll assume their original form.
   See usage below

env_summon() {
  local secrets_yml="${secrets_yml:-"./secrets.yml"}"
  local secrets_output="${secrets_output:-"dot_env"}"
  local script=$(<< "EOL"
common() {
 cat "${secrets_yml:-"./secrets.yml"}" \
 | sed '/^$/d' \
 | sed '/^ /d' \
 | { grep '^[^#]' || true; } \
 | sed -E 's/^([^:]*)?.*/\1/' \
 | awk '!visited[$0]++'
}
dot_env() {
  common | xargs -n 1 sh -c 'echo "{}" | printf "%s=%s\n" "$1" "$(jq --arg v "$(printenv $1)" "\$v")"' _
}
export_env() {
 common | xargs -n 1 sh -c 'printf "%s=%s\n" "export $1" "$(printf "%q" "$(printenv $1)")"' _
}
${secrets_output}
EOL
)
  secrets_yml="${secrets_yml}" secrets_output="${secrets_output}" summon $@ -f "${secrets_yml}" -- sh -e -c "${script}" _
}

env_summon usage

Given ./test-secrets.yml:

A: single line
B: |
  multiples
  lines
# some comment
C: |
  multiple
  line$$ok
  file

You can use summon_env_file to generate an envfile from summon with quoted values (i.e. secrets_output=dot_env)

~ secrets_output=dot_env secrets_yml=./test-secrets.yml summon_env_file --provider /bin/echo
A="single line"
B="multiples\nlines"
C="multiple\nline$ok\nfile"

Or to generate a shell file that can sourced and exports the environment variables (i.e. secrets_output=export_env). The values are escaped so that when they are evaluated in a shell they'll assume their original form.

~ secrets_output=export_env secrets_yml=./test-secrets.yml summon_env_file --provider /bin/echo
export A=single\ line
export B=$'multiples\nlines'
export C=$'multiple\nline$ok\nfile'

[mcanevet]

I'm not sure this problem is limited to docker. I guess @SUMMONENVFILE should be "sourceable" in any shell using the source command.
@doodlesbykumbi I indeed noticed that dotenv are very limited, that's why I'm storing @SUMMONENVFILE in an artifact that in source in the next job with set -a && source .env && set +a.

[doodlesbykumbi]

I'm not sure envfiles are, in general, guaranteed to be "sourceable". For that they'd need to escape the values in a myriad of ways that go beyond doubles quotes, \n, and the parsing rules of envfiles. The best thing I've found to generate "sourceable" files has been thesecrets_output=export_env format in my example above which relies on printf '%q'.

[diverdane]

@mcanevet, are you able to add this support outside of summon (i.e. retrieve secrets from summon, and then create a custom dotenv file with quoting for the multiline secrets)? I'd like to close this PR if so.

[mcanevet]

Don't you think the best solution would be to keep the quoting when the value contains at least one \n? It would make @SUMMONENVFILE source-able without regression, right? SO just a slight modification in my PR.

[diverdane]

Hi @mcanevet,

My preference would be to avoid modifying summon to do something that I feel is a very special case. I'm also concerned that the documentation for this is going to be a bit awkward.

I believe that your particular use case can be handled fairly easily without modifying summon, by using a bash script that takes the SUMMONENVFILE output from summon and does the translation you need, and writing that script's output to a dotenv file.

I've created a bash script to do just that. The invocation looks like this (where create_multiline_dotenv.sh is the translation script):

summon ./create_multiline_dotenv.sh @SUMMONENVFILE > my.env

The create_multiline_dotenv.sh script looks like this:

#!/bin/bash

# Valid environment variable settings are of the form
#   key=value
# Where the 'key' begins with an upper or lower case letter
# followed by any number of letters, numbers, or underscores.
valid_key_value="^([a-zA-Z][a-zA-Z0-9_]*)=(.*)"

function process_file() {
  file="$1"

  prev_key=""
  prev_val=""
  multiline="false"

  while read line; do
    if [[ $line =~ $valid_key_value ]]; then
      # This is the start of a new k/v, so time to output the previous k/v.
      key="${BASH_REMATCH[1]}"
      val="${BASH_REMATCH[2]}"
      if [ -n "$prev_key" ]; then
        if [ "$multiline" = "true" ]; then
          # Append terminating quote to previous value
          prev_val+="\""
          multiline="false"
        fi
        echo "$prev_key=$prev_val"
      fi
      # Start processing new k/v.
      prev_key="$key"
      prev_val="$val"
    else
      # Not a valid key/value pair, so this line is a
      # continuation of the previous (multiline) value.
      if [ "$multiline" = "false" ]; then
        # Prepend starting quote to previous value
        prev_val="\"$prev_val"
        multiline="true"
      fi
      # Append newline and current value to previous value
      prev_val+="\n"
      prev_val+="$line"
    fi
  done < "$file"

  ## Done processing file. Output final k/v.
  if [ -n "$prev_key" ]; then
    if [ "$multiline" = "true" ]; then
      # Append terminating quote to previous value
      prev_val+="\""
    fi
    echo "$prev_key=$prev_val"
  fi
}

process_file "$1"

Here's an example usage, where I have two variables, one of which has a multiline value:

$ summon ./create_multiline_dotenv.sh @SUMMONENVFILE > my.env
$ cat my.env
FOO=foobar
MULTILINE="the rain\nin Spain"
$ source my.env
$ echo $MULTILINE
the rain\nin Spain
$

Please let me know what you think.

[mcanevet]

@diverdane thanks for taking time to find a proper solution that match my needs. Your script would indeed do the trick, I'll try to use it.
However, are your sure what I try to do is a very special case? Currently, @SUMMONENVFILE is neither "sourceable" with source nor usable in Docker when a secret contains multiple lines and I can't think about any tool that could use it properly without being quote.
I agree that I should not quote all values, but only the ones that contains \n. This should be a simple modification to my patch that I can easily do.
Moreover, with the main modification I did, using a map[string]string instead of a []string, I had in mind another PR that would allow to output secrets in json or yaml with two new special string, for example @SUMMONJSONFILE and @SUMMONYAMLFILE.
Would you accept a patch that uses map[string]string instead of a []string (the same one a this one, but without the appendQuote that we are not agree on) that does nothing yet but will allow more flexible things later?

[mcanevet]

I created #188 that contains the most of the diff but does not change the behavior of summon.
I'll rebase this one on to of #188

[mcanevet]

@diverdane I rebased this PR on top of #188 and I only quote value when it contains multiple lines (see the tests).

[mcanevet]

I'm not sure that your shell script would work with a value containing a GPG or SSH key (my use case), as the last line will match the valid_key_value regex because it contains the equals symbol. Dealing with this issue outside of summon could be very tricky I guess...

[diverdane]

Hi @mcanevet,

I'll take a look at your other PR.

In the meantime, have you considered base64 encoding the SSH key when storing it as a Conjur secret (so it's saved as a single line), and then base64 decoding the key after it's retrieved from Conjur via Summon (i.e. either decoding before creating the dotenv file, or having the consumer of the dotenv file doing the decoding)?

[mcanevet]

@diverdane well, having it base64 encoded would not be a solution for me, because this are secrets I want to use also without using @SUMMONENVFILE and a script in the middle.

[mcanevet]

@diverdane I rebased this PR. It's way more simple now.
Now the question is : is this a generic problem or a specific one related to my use case (and the one reported in #31)?
I can't find any use case where you want a multi-line value in an environment file not to be quoted.
I can't find any documentation or RFC that says the a multi-line value in an environment file should be quoted (I'm not even sure that there is a such thing as a environment file standard...)

[mcanevet]

@diverdane I rebased this PR. I'm wondering if you still disagree on the fact that it's a generic use case to be able to source @SUMMONENVFILE when it contains multi-lines values an not a specific one?
If you don't agree then, unfortunately, I guess you should close this PR as wont-do.

[doodlesbykumbi]

@mcanevet Apologies for not responding sooner.

There's been some internal discussion. We've converged on providing primitives that allow users to implement this kind of functionality on their own. We would like to avoid building special cases into Summon. We're still thinking about the right primitives, however we currently have in mind

1. Something like SUMMONVARNAMES (an envvar or substitution value like @SUMMONENVFILE) which would provides a space separated list of the names of environment variables injected by Summon. This could be used for your use case.
2. An interface for custom output processing.

There's an issue for (1) over at #199. We'd love to get your thoughts and feedback on these and the general approach.

[doodlesbykumbi]

@mcanevet For (2) we are considering providing some canned output formats and an envfile with quoted strings could be one of them.

[doodlesbykumbi]

@mcanevet Following the comments above we're going to close this PR for now as we finalise (1), (2) and beyond.

Please allow me to reiterate that we greatly appreciate your contribution and would love your thoughts and feedback moving forward.