Shell escape the value of Elastic APM custom properties (#908)

The current behavior is for no escaping to occur. This if you use a value like `"object_name[java.lang:type=Memory] attribute[HeapMemoryUsage:metric_name=test_heap_metric]"` (as we see in #786), the literal value is inserted into a system property and the space (among other characters) breaks the start command.

This change will use `Shellwords.escape(..)` the value for every custom property unless that property value contains what looks like a subshell `$(..)` or environment variable `${..}` reference. If it looks like a subshell or env variable is being referenced, we will not escape just wrap the value in escaped quotes. We wrap it in escaped quotes in case the shell variable or subshell returns something which includes spaces. This is not perfect though, and you need to be careful if using subshell/env variables, you should ensure the output is properly escaped.

For example:

- `object_name[java.lang:type=Memory] attribute[HeapMemoryUsage:metric_name=test_heap_metric]` becomes `object_name\[java.lang:type\=Memory\]\ attribute\[HeapMemoryUsage:metric_name\=test_heap_metric\]`
- `$(echo 'Hello world!') and stuff` becomes `\"$(echo 'Hello world!') and stuff\"`
- `--> ${SOME_VAR} <--` becomes `\"--> ${SOME_VAR} <--\"`

Signed-off-by: Daniel Mikusa <[email protected]>

Author: Daniel Mikusa

docs/framework-elastic_apm_agent.md

@@ -21,7 +21,7 @@ When binding Elastic APM using a user-provided service, it must have name or tag
 | ---- | -----------
 | `server_urls` | The URLs for the Elastic APM Server. They must be fully qualified, including protocol (http or https) and port.
 | `secret_token` (Optional)| This string is used to ensure that only your agents can send data to your APM server. Both the agents and the APM server have to be configured with the same secret token. Use if APM Server requires a token.
-| `***`	(Optional) | Any additional entries will be applied as a system property appended to `-Delastic.apm.` to allow full configuration of the agent. See [Configuration of Elastic Agent][].
+| `***`	(Optional) | Any additional entries will be applied as a system property appended to `-Delastic.apm.` to allow full configuration of the agent. See [Configuration of Elastic Agent][]. Values are shell-escaped by default, but do have limited support, use with caution, for incorporating subshells (i.e. `$(some-cmd)`) and accessing environment variables (i.e. `${SOME_VAR}`).
 
 
 ### Creating an Elastic APM USer Provided Service

lib/java_buildpack/framework/elastic_apm_agent.rb

@@ -15,6 +15,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+require 'shellwords'
 require 'java_buildpack/component/versioned_dependency_component'
 require 'java_buildpack/framework'
 
@@ -89,7 +90,12 @@ def apply_user_configuration(credentials, configuration)
 
       def write_java_opts(java_opts, configuration)
         configuration.each do |key, value|
-          java_opts.add_system_property("elastic.apm.#{key}", value)
+          if /\$[\(\{][^\)\}]+[\)\}]/ =~ value
+            # we need \" because this is a system property which ends up inside `JAVA_OPTS` which is already quoted
+            java_opts.add_system_property("elastic.apm.#{key}", "\\\"#{value}\\\"")
+          else
+            java_opts.add_system_property("elastic.apm.#{key}", Shellwords.escape(value))
+          end
         end
       end
 

spec/java_buildpack/framework/elastic_apm_agent_spec.rb

@@ -60,17 +60,27 @@
     end
 
     it 'updates JAVA_OPTS with additional options' do
+      val = 'object_name[java.lang:type=Memory] attribute[HeapMemoryUsage:metric_name=test_heap_metric]'
+      shell = '$(echo \'Hello world!\') and stuff'
+      var = '--> ${SOME_VAR} <--'
       allow(services).to receive(:find_service).and_return('credentials' => { 'secret_token' => 'test-secret_token',
                                                                               'server_urls' => 'different-serverurl',
                                                                               'service_name' => 'different-name',
-                                                                              'foo' => 'bar' })
+                                                                              'foo' => 'bar',
+                                                                              'capture_jmx_metrics' => val,
+                                                                              'sub' => shell,
+                                                                              'var' => var })
 
       component.release
 
       expect(java_opts).to include('-Delastic.apm.secret_token=test-secret_token')
       expect(java_opts).to include('-Delastic.apm.server_urls=different-serverurl')
       expect(java_opts).to include('-Delastic.apm.service_name=different-name')
       expect(java_opts).to include('-Delastic.apm.foo=bar')
+      escaped = 'object_name\[java.lang:type\=Memory\]\ attribute\[HeapMemoryUsage:metric_name\=test_heap_metric\]'
+      expect(java_opts).to include("-Delastic.apm.capture_jmx_metrics=#{escaped}")
+      expect(java_opts).to include('-Delastic.apm.sub=\"$(echo \'Hello world!\') and stuff\"')
+      expect(java_opts).to include('-Delastic.apm.var=\"--> ${SOME_VAR} <--\"')
     end
 
   end