Relax escaping of values to enable limited access to the shell for AppDynamnics config

docs/framework-app_dynamics_agent.md

@@ -26,7 +26,7 @@ When binding AppDynamics using a user-provided service, it must have name or tag
 | `node-name` | (Optional) the application's node name
 | `tier-name` | (Optional) the application's tier name
 
-To provide more complex values such as the `tier-name`, using the interactive mode when creating a user-provided service will manage the character escaping automatically. For example, the default `tier-name` could be set with a value of `Tier-$(expr "$VCAP_APPLICATION" : '.*instance_index[": ]*\([[:digit:]]*\).*')` to calculate a value from the Cloud Foundry instance index.
+To provide more complex values such as the `tier-name`, using the interactive mode when creating a user-provided service will manage the character escaping automatically. For example, the default `tier-name` could be set with a value of `Tier-$(expr "${VCAP_APPLICATION}" : '.*instance_index[": ]*\([[:digit:]]*\).*')` to calculate a value from the Cloud Foundry instance index.
 
 **Note:** Some credentials were previously marked as "(Optional)" as requirements have changed across versions of the AppDynamics agent.  Please see the [AppDynamics Java Agent Configuration Properties][] for the version of the agent used by your application for more details.
 

lib/java_buildpack/framework/app_dynamics_agent.rb

@@ -86,35 +86,35 @@ def supports?
       private_constant :FILTER
 
       def application_name(java_opts, credentials)
-        name = Shellwords.escape(@application.details['application_name'])
+        name = escape(@application.details['application_name'])
         name = @configuration['default_application_name'] if @configuration['default_application_name']
-        name = Shellwords.escape(credentials['application-name']) if credentials['application-name']
+        name = escape(credentials['application-name']) if credentials['application-name']
 
         java_opts.add_system_property('appdynamics.agent.applicationName', name.to_s)
       end
 
       def account_access_key(java_opts, credentials)
         account_access_key = credentials['account-access-key'] || credentials.dig('account-access-secret', 'secret')
-        account_access_key = Shellwords.escape(account_access_key)
+        account_access_key = escape(account_access_key)
 
         java_opts.add_system_property 'appdynamics.agent.accountAccessKey', account_access_key if account_access_key
       end
 
       def account_name(java_opts, credentials)
         account_name = credentials['account-name']
-        java_opts.add_system_property 'appdynamics.agent.accountName', Shellwords.escape(account_name) if account_name
+        java_opts.add_system_property 'appdynamics.agent.accountName', escape(account_name) if account_name
       end
 
       def host_name(java_opts, credentials)
         host_name = credentials['host-name']
         raise "'host-name' credential must be set" unless host_name
 
-        java_opts.add_system_property 'appdynamics.controller.hostName', Shellwords.escape(host_name)
+        java_opts.add_system_property 'appdynamics.controller.hostName', escape(host_name)
       end
 
       def node_name(java_opts, credentials)
         name = @configuration['default_node_name']
-        name = Shellwords.escape(credentials['node-name']) if credentials['node-name']
+        name = escape(credentials['node-name']) if credentials['node-name']
 
         java_opts.add_system_property('appdynamics.agent.nodeName', name.to_s)
       end
@@ -130,15 +130,15 @@ def ssl_enabled(java_opts, credentials)
       end
 
       def tier_name(java_opts, credentials)
-        name = Shellwords.escape(@application.details['application_name'])
+        name = escape(@application.details['application_name'])
         name = @configuration['default_tier_name'] if @configuration['default_tier_name']
-        name = Shellwords.escape(credentials['tier-name']) if credentials['tier-name']
+        name = escape(credentials['tier-name']) if credentials['tier-name']
 
         java_opts.add_system_property('appdynamics.agent.tierName', name.to_s)
       end
 
       def unique_host_name(java_opts)
-        name = Shellwords.escape(@application.details['application_name'])
+        name = escape(@application.details['application_name'])
         name = @configuration['default_unique_host_name'] if @configuration['default_unique_host_name']
 
         java_opts.add_system_property('appdynamics.agent.uniqueHostId', name.to_s)
@@ -180,6 +180,14 @@ def save_cfg_file(file, conf_file)
           FileUtils.cp_r file, target_directory + '/conf/' + conf_file
         end
       end
+
+      def escape(value)
+        if /\$[\(\{][^\)\}]+[\)\}]/ =~ value
+          "\\\"#{value}\\\""
+        else
+          Shellwords.escape(value)
+        end
+      end
     end
   end
 end

spec/java_buildpack/framework/app_dynamics_agent_spec.rb

@@ -92,6 +92,26 @@
         end
       end
 
+      context do
+        let(:credentials) { super().merge 'tier-name' => '--> ${SOME_VAR} <--' }
+
+        it 'adds tier_name from credentials with shell variable in it to JAVA_OPTS if specified' do
+          component.release
+
+          expect(java_opts).to include('-Dappdynamics.agent.tierName=\"--> ${SOME_VAR} <--\"')
+        end
+      end
+
+      context do
+        let(:credentials) { super().merge 'tier-name' => '$(echo \'Hello World!\') and stuff' }
+
+        it 'adds tier_name from credentials with subshell in it to JAVA_OPTS if specified' do
+          component.release
+
+          expect(java_opts).to include('-Dappdynamics.agent.tierName=\"$(echo \'Hello World!\') and stuff\"')
+        end
+      end
+
       context do
         let(:credentials) { super().merge 'application-name' => 'another-test application-name' }
 
@@ -102,6 +122,26 @@
         end
       end
 
+      context do
+        let(:credentials) { super().merge 'application-name' => '$(echo \'Hello World!\') and stuff' }
+
+        it 'adds application_name from credentials with subshell in value to JAVA_OPTS if specified' do
+          component.release
+
+          expect(java_opts).to include('-Dappdynamics.agent.applicationName=\"$(echo \'Hello World!\') and stuff\"')
+        end
+      end
+
+      context do
+        let(:credentials) { super().merge 'application-name' => 'Name ${MY_APP_NAME}' }
+
+        it 'adds application_name from credentials with env variable in value to JAVA_OPTS if specified' do
+          component.release
+
+          expect(java_opts).to include('-Dappdynamics.agent.applicationName=\"Name ${MY_APP_NAME}\"')
+        end
+      end
+
       context do
         let(:configuration) do
           { 'default_tier_name' => nil,