spring-cloud / spring-cloud-commons

Common classes used in different Spring Cloud implementations
Apache License 2.0
707 stars 702 forks source link

Spring Actuator Env Endpoint does not validate request body for POST requests #1060

Open Walnussbaer opened 2 years ago

Walnussbaer commented 2 years ago

Copied from https://github.com/spring-projects/spring-framework/issues/27962 as suggested by https://github.com/bclozel.

In our project, we are using Spring Boot Actuator (version 2.6.2) as well as Spring Cloud (2021.0.0) on Spring Boot Version 2.6.2.

My current objective is to implement a config editor, which enables the users of our product to manipulate our custom spring properties (properties in classes with @ConfigurationProperties) during runtime.

I'm currently somehow achieving this goal using a combined approach. This approach consists of sending a POST request to the /actuator/env endpoint (that's why Spring Cloud is a dependency of our project) with a valid HTTP request body as well as writing to an external application.yaml file, which stores these config overrides, in case the spring container has to restart (as all calls to /actuator/env only manipulate the Spring environment).

Lets say I have this configuraiton bean:

@Getter
@Setter
@Component
@Scope(proxyMode = ScopedProxyMode.TARGET_CLASS, value = "prototype")
@ConfigurationProperties(prefix="custom")
public class MyConfigurationClass {

    String myProperty = "Hello World";

}

When I make a HTTP POST request to /actuator/env, my request body looks like the following:

{
   "name": "custom.my-property",
   "value": "Hello"
}

This works fine as long the the value in the value property matches the type of the property, which is defined in the configuration bean. But let's say, the type of the property custom.my-property is of type int (or any other type), like this:

@Getter
@Setter
@Component
@Scope(proxyMode = ScopedProxyMode.TARGET_CLASS, value = "prototype")
@ConfigurationProperties(prefix="custom")
public class MyConfigurationClass {

    int myProperty = 12345;

}

When I now send my HTTP POST request with value = "Hello World", the problems (and potential bugs or things to improve) arise.

The /actuator/env endpoint seems to not make any validation of the request body. It looks like it changes the spring environment, but then spring cannot bind the new value to the configuration bean (ovioulsy, because a string cannot be bound to an integer). An exception occurs, and the response of the /actuatur/env endpoint contains the following:

{
    "timestamp": 1642671307254,
    "status": 500,
    "error": "Internal Server Error",
    "trace": "org.springframework.boot.context.properties.ConfigurationPropertiesBindException: Error creating bean with name 'scopedTarget.myConfigurationClass': Could not bind properties to 'MyConfigurationClass' : prefix=custom, ignoreInvalidFields=false, ignoreUnknownFields=true; nested exception is org.springframework.boot.context.properties.bind.BindException: Failed to bind properties under 'custom.my-property' to int\r\n\tat org.springframework.boot.context.properties.ConfigurationPropertiesBindingPostProcessor.bind(ConfigurationPropertiesBindingPostProcessor.java:92)\r\n\tat org.springframework.boot.context.properties.ConfigurationPropertiesBindingPostProcessor.postProcessBeforeInitialization(ConfigurationPropertiesBindingPostProcessor.java:78)\r\n\tat org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.applyBeanPostProcessorsBeforeInitialization(AbstractAutowireCapableBeanFactory.java:440)\r\n\tat org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1796)\r\n\tat org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:620)\r\n\tat org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:542)\r\n\tat org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:353)\r\n\tat org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:208)\r\n\tat org.springframework.context.support.AbstractApplicationContext.getBean(AbstractApplicationContext.java:1154)\r\n\tat org.springframework.cloud.context.properties.ConfigurationPropertiesRebinder.rebind(ConfigurationPropertiesRebinder.java:94)\r\n\tat org.springframework.cloud.context.properties.ConfigurationPropertiesRebinder.rebind(ConfigurationPropertiesRebinder.java:83)\r\n\tat org.springframework.cloud.context.properties.ConfigurationPropertiesRebinder.onApplicationEvent(ConfigurationPropertiesRebinder.java:138)\r\n\tat org.springframework.cloud.context.properties.ConfigurationPropertiesRebinder.onApplicationEvent(ConfigurationPropertiesRebinder.java:51)\r\n\tat org.springframework.context.event.SimpleApplicationEventMulticaster.doInvokeListener(SimpleApplicationEventMulticaster.java:176)\r\n\tat org.springframework.context.event.SimpleApplicationEventMulticaster.invokeListener(SimpleApplicationEventMulticaster.java:169)\r\n\tat org.springframework.context.event.SimpleApplicationEventMulticaster.multicastEvent(SimpleApplicationEventMulticaster.java:143)\r\n\tat org.springframework.context.support.AbstractApplicationContext.publishEvent(AbstractApplicationContext.java:421)\r\n\tat org.springframework.context.support.AbstractApplicationContext.publishEvent(AbstractApplicationContext.java:378)\r\n\tat org.springframework.cloud.context.environment.EnvironmentManager.publish(EnvironmentManager.java:104)\r\n\tat org.springframework.cloud.context.environment.EnvironmentManager.setProperty(EnvironmentManager.java:92)\r\n\tat org.springframework.cloud.context.environment.WritableEnvironmentEndpointWebExtension.write(WritableEnvironmentEndpointWebExtension.java:47)\r\n\tat java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)\r\n\tat java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)\r\n\tat java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)\r\n\tat java.base/java.lang.reflect.Method.invoke(Method.java:568)\r\n\tat org.springframework.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:282)\r\n\tat org.springframework.boot.actuate.endpoint.invoke.reflect.ReflectiveOperationInvoker.invoke(ReflectiveOperationInvoker.java:74)\r\n\tat org.springframework.boot.actuate.endpoint.annotation.AbstractDiscoveredOperation.invoke(AbstractDiscoveredOperation.java:60)\r\n\tat org.springframework.boot.actuate.endpoint.web.servlet.AbstractWebMvcEndpointHandlerMapping$ServletWebOperationAdapter.handle(AbstractWebMvcEndpointHandlerMapping.java:336)\r\n\tat org.springframework.boot.actuate.endpoint.web.servlet.AbstractWebMvcEndpointHandlerMapping$OperationHandler.handle(AbstractWebMvcEndpointHandlerMapping.java:421)\r\n\tat java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)\r\n\tat java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)\r\n\tat java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)\r\n\tat java.base/java.lang.reflect.Method.invoke(Method.java:568)\r\n\tat org.springframework.web.method.support.InvocableHandlerMethod.doInvoke(InvocableHandlerMethod.java:205)\r\n\tat org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:150)\r\n\tat org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:117)\r\n\tat org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:895)\r\n\tat org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:808)\r\n\tat org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:87)\r\n\tat org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1067)\r\n\tat org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:963)\r\n\tat org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006)\r\n\tat org.springframework.web.servlet.FrameworkServlet.doPost(FrameworkServlet.java:909)\r\n\tat javax.servlet.http.HttpServlet.service(HttpServlet.java:681)\r\n\tat org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:883)\r\n\tat javax.servlet.http.HttpServlet.service(HttpServlet.java:764)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:227)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)\r\n\tat org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:53)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)\r\n\tat org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:327)\r\n\tat org.springframework.security.web.access.intercept.FilterSecurityInterceptor.invoke(FilterSecurityInterceptor.java:115)\r\n\tat org.springframework.security.web.access.intercept.FilterSecurityInterceptor.doFilter(FilterSecurityInterceptor.java:81)\r\n\tat org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)\r\n\tat org.springframework.security.web.access.ExceptionTranslationFilter.doFilter(ExceptionTranslationFilter.java:122)\r\n\tat org.springframework.security.web.access.ExceptionTranslationFilter.doFilter(ExceptionTranslationFilter.java:116)\r\n\tat org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)\r\n\tat org.springframework.security.web.session.SessionManagementFilter.doFilter(SessionManagementFilter.java:126)\r\n\tat org.springframework.security.web.session.SessionManagementFilter.doFilter(SessionManagementFilter.java:81)\r\n\tat org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)\r\n\tat org.springframework.security.web.authentication.AnonymousAuthenticationFilter.doFilter(AnonymousAuthenticationFilter.java:109)\r\n\tat org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)\r\n\tat org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter.doFilter(SecurityContextHolderAwareRequestFilter.java:149)\r\n\tat org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)\r\n\tat org.springframework.security.web.savedrequest.RequestCacheAwareFilter.doFilter(RequestCacheAwareFilter.java:63)\r\n\tat org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)\r\n\tat org.springframework.security.oauth2.server.resource.web.BearerTokenAuthenticationFilter.doFilterInternal(BearerTokenAuthenticationFilter.java:137)\r\n\tat org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:117)\r\n\tat org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)\r\n\tat org.springframework.security.web.authentication.logout.LogoutFilter.doFilter(LogoutFilter.java:103)\r\n\tat org.springframework.security.web.authentication.logout.LogoutFilter.doFilter(LogoutFilter.java:89)\r\n\tat org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)\r\n\tat org.springframework.web.filter.CorsFilter.doFilterInternal(CorsFilter.java:91)\r\n\tat org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:117)\r\n\tat org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)\r\n\tat org.springframework.security.web.header.HeaderWriterFilter.doHeadersAfter(HeaderWriterFilter.java:90)\r\n\tat org.springframework.security.web.header.HeaderWriterFilter.doFilterInternal(HeaderWriterFilter.java:75)\r\n\tat org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:117)\r\n\tat org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)\r\n\tat org.springframework.security.web.context.SecurityContextPersistenceFilter.doFilter(SecurityContextPersistenceFilter.java:110)\r\n\tat org.springframework.security.web.context.SecurityContextPersistenceFilter.doFilter(SecurityContextPersistenceFilter.java:80)\r\n\tat org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)\r\n\tat org.springframework.security.web.context.request.async.WebAsyncManagerIntegrationFilter.doFilterInternal(WebAsyncManagerIntegrationFilter.java:55)\r\n\tat org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:117)\r\n\tat org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)\r\n\tat org.springframework.security.web.FilterChainProxy.doFilterInternal(FilterChainProxy.java:211)\r\n\tat org.springframework.security.web.FilterChainProxy.doFilter(FilterChainProxy.java:183)\r\n\tat org.springframework.web.filter.DelegatingFilterProxy.invokeDelegate(DelegatingFilterProxy.java:354)\r\n\tat org.springframework.web.filter.DelegatingFilterProxy.doFilter(DelegatingFilterProxy.java:267)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)\r\n\tat org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:100)\r\n\tat org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:117)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)\r\n\tat org.springframework.web.filter.FormContentFilter.doFilterInternal(FormContentFilter.java:93)\r\n\tat org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:117)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)\r\n\tat org.springframework.boot.actuate.metrics.web.servlet.WebMvcMetricsFilter.doFilterInternal(WebMvcMetricsFilter.java:96)\r\n\tat org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:117)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)\r\n\tat org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201)\r\n\tat org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:117)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)\r\n\tat org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:197)\r\n\tat org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:97)\r\n\tat org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:540)\r\n\tat org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:135)\r\n\tat org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:92)\r\n\tat org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:78)\r\n\tat org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:357)\r\n\tat org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:382)\r\n\tat org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)\r\n\tat org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:895)\r\n\tat org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1732)\r\n\tat org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)\r\n\tat org.apache.tomcat.util.threads.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1191)\r\n\tat org.apache.tomcat.util.threads.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:659)\r\n\tat org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)\r\n\tat java.base/java.lang.Thread.run(Thread.java:833)\r\nCaused by: org.springframework.boot.context.properties.bind.BindException: Failed to bind properties under 'custom.my-property' to int\r\n\tat org.springframework.boot.context.properties.bind.Binder.handleBindError(Binder.java:384)\r\n\tat org.springframework.boot.context.properties.bind.Binder.bind(Binder.java:344)\r\n\tat org.springframework.boot.context.properties.bind.Binder.lambda$bindDataObject$4(Binder.java:469)\r\n\tat org.springframework.boot.context.properties.bind.JavaBeanBinder.bind(JavaBeanBinder.java:95)\r\n\tat org.springframework.boot.context.properties.bind.JavaBeanBinder.bind(JavaBeanBinder.java:83)\r\n\tat org.springframework.boot.context.properties.bind.JavaBeanBinder.bind(JavaBeanBinder.java:59)\r\n\tat org.springframework.boot.context.properties.bind.Binder.lambda$bindDataObject$5(Binder.java:473)\r\n\tat org.springframework.boot.context.properties.bind.Binder$Context.withIncreasedDepth(Binder.java:587)\r\n\tat org.springframework.boot.context.properties.bind.Binder$Context.withDataObject(Binder.java:573)\r\n\tat org.springframework.boot.context.properties.bind.Binder$Context.access$300(Binder.java:534)\r\n\tat org.springframework.boot.context.properties.bind.Binder.bindDataObject(Binder.java:471)\r\n\tat org.springframework.boot.context.properties.bind.Binder.bindObject(Binder.java:411)\r\n\tat org.springframework.boot.context.properties.bind.Binder.bind(Binder.java:340)\r\n\tat org.springframework.boot.context.properties.bind.Binder.bind(Binder.java:329)\r\n\tat org.springframework.boot.context.properties.bind.Binder.bind(Binder.java:259)\r\n\tat org.springframework.boot.context.properties.bind.Binder.bind(Binder.java:246)\r\n\tat org.springframework.boot.context.properties.ConfigurationPropertiesBinder.bind(ConfigurationPropertiesBinder.java:95)\r\n\tat org.springframework.boot.context.properties.ConfigurationPropertiesBindingPostProcessor.bind(ConfigurationPropertiesBindingPostProcessor.java:89)\r\n\t... 124 more\r\nCaused by: org.springframework.core.convert.ConversionFailedException: Failed to convert from type [java.lang.String] to type [int] for value 'test'; nested exception is java.lang.NumberFormatException: For input string: \"test\"\r\n\tat org.springframework.core.convert.support.ConversionUtils.invokeConverter(ConversionUtils.java:47)\r\n\tat org.springframework.core.convert.support.GenericConversionService.convert(GenericConversionService.java:192)\r\n\tat org.springframework.boot.context.properties.bind.BindConverter.convert(BindConverter.java:109)\r\n\tat org.springframework.boot.context.properties.bind.BindConverter.convert(BindConverter.java:100)\r\n\tat org.springframework.boot.context.properties.bind.BindConverter.convert(BindConverter.java:92)\r\n\tat org.springframework.boot.context.properties.bind.Binder.bindProperty(Binder.java:456)\r\n\tat org.springframework.boot.context.properties.bind.Binder.bindObject(Binder.java:400)\r\n\tat org.springframework.boot.context.properties.bind.Binder.bind(Binder.java:340)\r\n\t... 140 more\r\nCaused by: java.lang.NumberFormatException: For input string: \"test\"\r\n\tat java.base/java.lang.NumberFormatException.forInputString(NumberFormatException.java:67)\r\n\tat java.base/java.lang.Integer.parseInt(Integer.java:668)\r\n\tat java.base/java.lang.Integer.valueOf(Integer.java:999)\r\n\tat org.springframework.util.NumberUtils.parseNumber(NumberUtils.java:211)\r\n\tat org.springframework.core.convert.support.StringToNumberConverterFactory$StringToNumber.convert(StringToNumberConverterFactory.java:64)\r\n\tat org.springframework.core.convert.support.StringToNumberConverterFactory$StringToNumber.convert(StringToNumberConverterFactory.java:50)\r\n\tat org.springframework.core.convert.support.GenericConversionService$ConverterFactoryAdapter.convert(GenericConversionService.java:437)\r\n\tat org.springframework.core.convert.support.ConversionUtils.invokeConverter(ConversionUtils.java:41)\r\n\t... 147 more\r\n",
    "message": "Error creating bean with name 'scopedTarget.myConfigurationClass': Could not bind properties to 'MyConfigurationClass' : prefix=custom, ignoreInvalidFields=false, ignoreUnknownFields=true; nested exception is org.springframework.boot.context.properties.bind.BindException: Failed to bind properties under 'custom.my-property' to int",
    "path": "/actuator/env"
}

You would think now that the spring environment did not change because of that exception. But that is not the case. When I now request the value of the property custom.my-property (e.g. using springEnvironment.getProperty("custom.my-property"), then the spring property holds the new value ("Hello"). I didn't check yet what happened to the configuration bean, whether it still holds the old (correct) int value or not.

And now it gets even crazier: When I send a second POST request (again with a String and not an integer) to /actuator/env, no exception occurs. It seems like Spring doesn't even try to bind the value anymore to the Configuraiton bean. But when I send a third request to /actuator/env, now with a correct integer value, then the exception occurs again. So to me it looks like Spring changes the data type internally somehow. At least some kind of magic happens that I don't understand.

Now the point ist: Shouldn't there be at least some validaiton at /actuator/env when this endpoint recieves POST requests? Some validation based on the configuration bean? And what about that strange "type-alternating" behaviour I described in the previous paragraph?

spencergibb commented 2 years ago

There's an existing issue about validation during this process. (Ok mobile, will need to find). There are two pieces, the spring environment, which doesn't have any form of validation and configuration priorities, which can have validation. But config props need values in the spring environment before they can be validated. There would have to be a new process to validate config props on a separate environment before it gets applied or be able to roll back, none of which exist.