ci icon indicating copy to clipboard operation
ci copied to clipboard

Incorrect parsing of environment variable when quoted

Open irarainey opened this issue 3 years ago • 4 comments

I was passing in a URL endpoint as an environment variable to a workflow step to run end-to-end test via Newman, and I was getting a protocol error through by Newman. After some investigation trying to access the same endpoint using wget I noticed that variable being passed in was being parsed incorrectly.

The problem arose when I was calling it like this:

- name: Run Tests
  uses: devcontainers/[email protected]
  with:
    imageName: ghcr.io/microsoft/my-devcontainer
    runCmd: make run-tests
    imageTag: ${{ env.TAG }}
    env: API_ENDPOINT="https://${{ env.APP_NAME }}.azurewebsites.net"

The solution was to remove the quotes around the variable like this:

- name: Run Tests
  uses: devcontainers/[email protected]
  with:
    imageName: ghcr.io/microsoft/my-devcontainer
    runCmd: make run-tests
    imageTag: ${{ env.TAG }}
    env: API_ENDPOINT=https://${{ env.APP_NAME }}.azurewebsites.net

But as a string I would expect it to be able to handle either format.

irarainey avatar Sep 09 '22 11:09 irarainey

I think it would be a relatively straightforward change to make so that ENVVAR=value and ENVVAR="value" result in the same value assigned to ENVVAR.

Technically, this would be a breaking change. I.e. if someone currently has a value with quotes and wants the quotes then making this change would break the existing usage. But, I suspect that most people would expect the same behaviour that @irarainey did.

@chrmarti - what are your thoughts?

stuartleeks avatar Sep 09 '22 11:09 stuartleeks

env: seems to be a list at the moment, maybe it could be changed to an object mapping keys to values. That would align, e.g., with env: of the built-in run: job. Syntax variations would then be given by yaml.

You would then want to support the list for backwards compatibility. I guess that's what you wanted to avoid. :)

chrmarti avatar Sep 09 '22 13:09 chrmarti

Currently the input is a multiline string that needs to be parsed. The ideal way to do this would be to align with the env: from the built-in run syntax as @chrmarti suggests.

I couldn't see a way to do that when initially creating the action, and still can't from looking at the @actions/core library. I was searching through issues and found this issue that seems to have the same conclusion 😞

stuartleeks avatar Dec 18 '22 20:12 stuartleeks

Any news on this? We just encountered a closely related problem where it currently is not possible to forward multiline secrets through environment variables to the devcontainer. Our use-case is forwarding a SSH key from secrets to the devcontainer.

lure8225 avatar Feb 02 '24 11:02 lure8225