cloud_controller_ng icon indicating copy to clipboard operation
cloud_controller_ng copied to clipboard

Unquoted environment variables beginning with colon in manifest break /v3/spaces/:guid/actions/apply_manifest

Open dominicrobertsc opened this issue 4 years ago • 2 comments

Issue

Running cf curl v3/spaces/:guid/actions/apply_manifest with a manifest containing an unquoted environment variable beginning with a colon causes a 500 internal server error.

Context

While investigating this cli issue we discovered that that the cloud controller cannot parse unquoted variables beginning with a colon. This seems like a problem, as such variables are valid yaml.

Steps to Reproduce

Using the below manifest (or one containing a environment variable similar to break):

- name: test
  env:
    break: :dd

run cf curl -X POST "v3/spaces/:guid/actions/apply_manifest" -H "Content-type: application/x-yaml" -d @./manifest.yml

Expected result

Manifest for test is applied successfully

Current result

Command fails with a 500 response and the below body:

{
   "errors": [
      {
         "title": "UnknownError",
         "detail": "An unknown error occurred.",
         "code": 10001
      }
   ]
}

Possible Fix

The bug appears to be caused by Ruby's Yaml parsing library, Psych, at this point in the code.

Per a suggestion from a different issue and based on local testing, we might explore changing that call to yaml = YAML.safe_load(request.body.string, [Symbol], [], allow_yaml_aliases).

dominicrobertsc avatar Sep 27 '21 15:09 dominicrobertsc

What is the use case of using symbols in an environment variables? Is there a specific example or is there special code in your app that specifically looks for ":symbols" in the environment variable?

weymanf avatar Oct 11 '21 21:10 weymanf

The intent is not to enable symbols, as such. Rather it is to allow variables to begin with colons; my understanding is that it is a nuance of Ruby that such variables need to be parsed as symbols.

As for the specific use case for such an environment variable, the original issue did not go into specifics. Our thinking around this change is that the key: :value entries are valid yaml and should be parsed as such

dominicrobertsc avatar Oct 12 '21 07:10 dominicrobertsc