diff --git a/helm-wrapper/src/main/java/io/github/inseefrlab/helmwrapper/utils/Command.java b/helm-wrapper/src/main/java/io/github/inseefrlab/helmwrapper/utils/Command.java index d0519caa..e06699e8 100644 --- a/helm-wrapper/src/main/java/io/github/inseefrlab/helmwrapper/utils/Command.java +++ b/helm-wrapper/src/main/java/io/github/inseefrlab/helmwrapper/utils/Command.java @@ -22,7 +22,7 @@ public class Command { "^[ ]*[a-z0-9]([-a-z0-9 ]*[a-z0-9 ])?(\\.[a-z0-9 ]([-a-z0-9 ]*[a-z0-9 ])?)*[ ]*$"); private static final Logger LOGGER = LoggerFactory.getLogger(Command.class); - private static ProcessExecutor getProcessExecutor() { + static ProcessExecutor getProcessExecutor() { // Changed to package-private ProcessExecutor processExecutor = new ProcessExecutor(); processExecutor.redirectError(System.err); processExecutor.readOutput(true); @@ -40,45 +40,52 @@ public void afterStart(Process process, ProcessExecutor executor) { public static ProcessResult executeAndGetResponseAsJson( HelmConfiguration helmConfiguration, String command) throws InvalidExitValueException, IOException, InterruptedException, TimeoutException { + validateCommand(command); return getProcessExecutor() .environment(getEnv(helmConfiguration)) - .commandSplit(addConfigToCommand(command, helmConfiguration) + " --output json") + .commandSplit(buildSecureCommand(command, helmConfiguration) + " --output json") .execute(); } public static ProcessResult executeAndGetResponseAsJson(String command) throws InvalidExitValueException, IOException, InterruptedException, TimeoutException { + validateCommand(command); return executeAndGetResponseAsJson(null, command); } public static ProcessResult executeAndGetResponseAsRaw( HelmConfiguration helmConfiguration, String command) throws InvalidExitValueException, IOException, InterruptedException, TimeoutException { + validateCommand(command); return getProcessExecutor() .environment(getEnv(helmConfiguration)) - .commandSplit(addConfigToCommand(command, helmConfiguration)) + .commandSplit(buildSecureCommand(command, helmConfiguration)) .execute(); } public static ProcessResult executeAndGetResponseAsRaw(String command) throws InvalidExitValueException, IOException, InterruptedException, TimeoutException { + validateCommand(command); return executeAndGetResponseAsRaw(null, command); } public static ProcessResult execute(HelmConfiguration helmConfiguration, String command) throws InvalidExitValueException, IOException, InterruptedException, TimeoutException { + validateCommand(command); return getProcessExecutor() .environment(getEnv(helmConfiguration)) - .commandSplit(addConfigToCommand(command, helmConfiguration)) + .commandSplit(buildSecureCommand(command, helmConfiguration)) .execute(); } public static ProcessResult execute(String command) throws InvalidExitValueException, IOException, InterruptedException, TimeoutException { + validateCommand(command); return execute(null, command); } - private static Map getEnv(HelmConfiguration helmConfiguration) { + static Map getEnv( + HelmConfiguration helmConfiguration) { // Changed to package-private Map env = new HashMap<>(); if (System.getProperty("http.proxyHost") != null) { env.put( @@ -105,45 +112,50 @@ private static Map getEnv(HelmConfiguration helmConfiguration) { return env; } - private static String addConfigToCommand(String command, HelmConfiguration helmConfiguration) { - if (helmConfiguration == null) { - return command; - } - String newCommand = command; - newCommand = newCommand.concat(" "); - if (helmConfiguration.getAsKubeUser() != null) { - newCommand = - newCommand.concat(" --kube-as-user " + helmConfiguration.getAsKubeUser() + " "); - } - String kubeConfig = null; - if (StringUtils.isNotEmpty(helmConfiguration.getApiserverUrl())) { - newCommand = - newCommand - .concat(" --kube-apiserver=" + helmConfiguration.getApiserverUrl()) - .concat(" "); - // Kubeconfig should be set to /dev/null to prevent mixing user provided configuration - // with pre-existing local kubeconfig (most likely re-using a cluster certificate from - // another cluster) - kubeConfig = "/dev/null"; - } - - if (StringUtils.isNotEmpty(helmConfiguration.getKubeToken())) { - newCommand = - newCommand - .concat(" --kube-token=" + helmConfiguration.getKubeToken()) - .concat(" "); - kubeConfig = "/dev/null"; - } - - if (StringUtils.isNotEmpty(helmConfiguration.getKubeConfig())) { - kubeConfig = helmConfiguration.getKubeConfig(); + static String buildSecureCommand( + String command, HelmConfiguration helmConfiguration) { // Changed to package-private + StringBuilder newCommand = new StringBuilder(command).append(" "); + if (helmConfiguration != null) { + if (helmConfiguration.getAsKubeUser() != null) { + newCommand + .append(" --kube-as-user ") + .append(escapeArgument(helmConfiguration.getAsKubeUser())) + .append(" "); + } + String kubeConfig = null; + if (StringUtils.isNotEmpty(helmConfiguration.getApiserverUrl())) { + newCommand + .append(" --kube-apiserver=") + .append(escapeArgument(helmConfiguration.getApiserverUrl())) + .append(" "); + kubeConfig = "/dev/null"; + } + if (StringUtils.isNotEmpty(helmConfiguration.getKubeToken())) { + newCommand + .append(" --kube-token=") + .append(escapeArgument(helmConfiguration.getKubeToken())) + .append(" "); + kubeConfig = "/dev/null"; + } + if (StringUtils.isNotEmpty(helmConfiguration.getKubeConfig())) { + kubeConfig = helmConfiguration.getKubeConfig(); + } + if (kubeConfig != null) { + newCommand.append(" --kubeconfig=").append(escapeArgument(kubeConfig)).append(" "); + } } + return newCommand.toString(); + } - if (kubeConfig != null) { - newCommand = newCommand.concat(" --kubeconfig=" + kubeConfig).concat(" "); + static void validateCommand(String command) + throws IllegalArgumentException { // Changed to package-private + if (!safeToConcatenate.matcher(command).matches()) { + throw new IllegalArgumentException("Illegal characters in command"); } + } - return newCommand; + static String escapeArgument(String argument) { // Changed to package-private + return argument.replaceAll("([\"\\\\])", "\\\\$1"); } public static void safeConcat(StringBuilder currentCommand, String toConcat) diff --git a/helm-wrapper/src/test/java/io/github/inseefrlab/helmwrapper/UtilsCommandTest.java b/helm-wrapper/src/test/java/io/github/inseefrlab/helmwrapper/UtilsCommandTest.java deleted file mode 100644 index 52f15906..00000000 --- a/helm-wrapper/src/test/java/io/github/inseefrlab/helmwrapper/UtilsCommandTest.java +++ /dev/null @@ -1,47 +0,0 @@ -package io.github.inseefrlab.helmwrapper; - -import static org.junit.jupiter.api.Assertions.assertThrows; - -import io.github.inseefrlab.helmwrapper.utils.Command; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; -import org.springframework.boot.test.context.SpringBootTest; - -@SpringBootTest -public class UtilsCommandTest { - - @ParameterizedTest - @ValueSource( - strings = { - "rstudio", - "hello-world", - " vscode-python-33432 ", - "vscode-python-11 ", - " rstudio" - }) - public void shouldAllowConcatenate(String toConcatenate) { - Command.safeConcat(new StringBuilder("helm ls -a"), toConcatenate); - } - - @ParameterizedTest - @ValueSource( - strings = { - "", - "--post-renderer test", - "-o", - "-o", - "--option", - " --option", - "&", - "&&", - "@", - "|" - }) - public void shouldDisallowConcatenate(String toConcatenate) { - assertThrows( - IllegalArgumentException.class, - () -> { - Command.safeConcat(new StringBuilder("helm ls -a"), toConcatenate); - }); - } -} diff --git a/helm-wrapper/src/test/java/io/github/inseefrlab/helmwrapper/utils/CommandTest.java b/helm-wrapper/src/test/java/io/github/inseefrlab/helmwrapper/utils/CommandTest.java new file mode 100644 index 00000000..a45eea31 --- /dev/null +++ b/helm-wrapper/src/test/java/io/github/inseefrlab/helmwrapper/utils/CommandTest.java @@ -0,0 +1,207 @@ +package io.github.inseefrlab.helmwrapper.utils; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +import io.github.inseefrlab.helmwrapper.configuration.HelmConfiguration; +import java.util.Map; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.zeroturnaround.exec.ProcessExecutor; +import org.zeroturnaround.exec.ProcessResult; + +class CommandTest { + + private HelmConfiguration helmConfiguration; + private ProcessExecutor processExecutor; + + @BeforeEach + void setUp() { + helmConfiguration = new HelmConfiguration(); + helmConfiguration.setAsKubeUser("admin-user"); + helmConfiguration.setApiserverUrl("https://k8s-cluster.local"); + helmConfiguration.setKubeToken("eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9..."); + helmConfiguration.setKubeConfig("/etc/kubernetes/admin.conf"); + + processExecutor = mock(ProcessExecutor.class); + } + + @Test + void testExecuteAndGetResponseAsJson_withValidCommand() throws Exception { + try (MockedStatic mockedCommand = + Mockito.mockStatic(Command.class, Mockito.CALLS_REAL_METHODS)) { + mockedCommand.when(Command::getProcessExecutor).thenReturn(processExecutor); + + ProcessResult expectedResult = mock(ProcessResult.class); + when(processExecutor.commandSplit(any())).thenReturn(processExecutor); + when(processExecutor.environment(any(Map.class))).thenReturn(processExecutor); + when(processExecutor.execute()).thenReturn(expectedResult); + + ProcessResult result = + Command.executeAndGetResponseAsJson(helmConfiguration, "helm repo list"); + + assertNotNull(result); + verify(processExecutor, times(1)).execute(); + } + } + + @Test + void testExecuteAndGetResponseAsJson_withInvalidCommand() { + assertThrows( + IllegalArgumentException.class, + () -> { + Command.executeAndGetResponseAsJson(helmConfiguration, "invalid;command"); + }); + } + + @Test + void testExecuteAndGetResponseAsRaw_withValidCommand() throws Exception { + try (MockedStatic mockedCommand = + Mockito.mockStatic(Command.class, Mockito.CALLS_REAL_METHODS)) { + mockedCommand.when(Command::getProcessExecutor).thenReturn(processExecutor); + + ProcessResult expectedResult = mock(ProcessResult.class); + when(processExecutor.commandSplit(any())).thenReturn(processExecutor); + when(processExecutor.environment(any(Map.class))).thenReturn(processExecutor); + when(processExecutor.execute()).thenReturn(expectedResult); + + ProcessResult result = + Command.executeAndGetResponseAsRaw(helmConfiguration, "helm repo list"); + + assertNotNull(result); + verify(processExecutor, times(1)).execute(); + } + } + + @Test + void testExecuteAndGetResponseAsRaw_withInvalidCommand() { + assertThrows( + IllegalArgumentException.class, + () -> { + Command.executeAndGetResponseAsRaw(helmConfiguration, "invalid;command"); + }); + } + + @Test + void testExecute_withValidCommand() throws Exception { + try (MockedStatic mockedCommand = + Mockito.mockStatic(Command.class, Mockito.CALLS_REAL_METHODS)) { + mockedCommand.when(Command::getProcessExecutor).thenReturn(processExecutor); + + ProcessResult expectedResult = mock(ProcessResult.class); + when(processExecutor.commandSplit(any())).thenReturn(processExecutor); + when(processExecutor.environment(any(Map.class))).thenReturn(processExecutor); + when(processExecutor.execute()).thenReturn(expectedResult); + + ProcessResult result = Command.execute(helmConfiguration, "helm repo list"); + + assertNotNull(result); + verify(processExecutor, times(1)).execute(); + } + } + + @Test + void testExecute_withNefariousCommand() { + assertThrows( + IllegalArgumentException.class, + () -> { + Command.execute(helmConfiguration, "helm repo list; rm -rf /"); + }); + } + + @Test + void testValidateCommand_withValidCommand() { + assertDoesNotThrow( + () -> { + Command.validateCommand("helm repo list"); + }); + } + + @Test + void testValidateCommand_withInvalidCommand() { + assertThrows( + IllegalArgumentException.class, + () -> { + Command.validateCommand("invalid;command"); + }); + } + + @Test + void testValidateCommand_withNefariousCommand() { + assertThrows( + IllegalArgumentException.class, + () -> { + Command.validateCommand("helm repo list; rm -rf /"); + }); + } + + @Test + void testEscapeArgument() { + String escapedArgument = Command.escapeArgument("\"test\""); + assertEquals("\\\"test\\\"", escapedArgument); + } + + @Test + void testBuildSecureCommand() { + String secureCommand = Command.buildSecureCommand("helm repo list", helmConfiguration); + assertTrue(secureCommand.contains(" --kube-as-user admin-user ")); + assertTrue(secureCommand.contains(" --kube-apiserver=https://k8s-cluster.local ")); + assertTrue( + secureCommand.contains(" --kube-token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9... ")); + assertTrue(secureCommand.contains(" --kubeconfig=/etc/kubernetes/admin.conf ")); + } + + @Test + void testGetEnv() { + System.setProperty("http.proxyHost", "http://proxy"); + System.setProperty("http.proxyPort", "8080"); + System.setProperty("https.proxyHost", "https://secureproxy"); + System.setProperty("https.proxyPort", "8443"); + System.setProperty("no_proxy", "localhost"); + + Map env = Command.getEnv(helmConfiguration); + + assertEquals("http://proxy:8080", env.get("HTTP_PROXY")); + assertEquals("https://secureproxy:8443", env.get("HTTPS_PROXY")); + assertEquals("localhost", env.get("NO_PROXY")); + } + + @ParameterizedTest + @ValueSource( + strings = { + "rstudio", + "hello-world", + "vscode-python-33432", + "vscode-python-11", + "rstudio" + }) + public void shouldAllowConcatenate(String toConcatenate) { + Command.safeConcat(new StringBuilder("helm ls -a"), toConcatenate); + } + + @ParameterizedTest + @ValueSource( + strings = { + "", + "--post-renderer test", + "-o", + "--option", + " --option", + "&", + "&&", + "@", + "|" + }) + public void shouldDisallowConcatenate(String toConcatenate) { + assertThrows( + IllegalArgumentException.class, + () -> { + Command.safeConcat(new StringBuilder("helm ls -a"), toConcatenate); + }); + } +} diff --git a/onyxia-api/src/main/java/fr/insee/onyxia/api/dao/universe/CatalogRefresher.java b/onyxia-api/src/main/java/fr/insee/onyxia/api/dao/universe/CatalogRefresher.java index 284cee56..d3e8af69 100644 --- a/onyxia-api/src/main/java/fr/insee/onyxia/api/dao/universe/CatalogRefresher.java +++ b/onyxia-api/src/main/java/fr/insee/onyxia/api/dao/universe/CatalogRefresher.java @@ -36,7 +36,7 @@ public CatalogRefresher( this.refreshTime = refreshTime; } - private void refreshCatalogs() { + private void refreshCatalogs() throws InterruptedException { catalogs.getCatalogs() .forEach( c -> { @@ -49,6 +49,9 @@ private void refreshCatalogs() { c.getCaFile()); LOGGER.info("Updating catalog: {}", c.getId()); catalogLoader.updateCatalog(c); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException(e); // or handle it as needed } catch (Exception e) { LOGGER.warn( "Exception occurred while updating catalog: {}",