diff --git a/core/src/main/java/hudson/cli/CLICommand.java b/core/src/main/java/hudson/cli/CLICommand.java index e779afbc3453..f2dc2f8b5fc3 100644 --- a/core/src/main/java/hudson/cli/CLICommand.java +++ b/core/src/main/java/hudson/cli/CLICommand.java @@ -26,6 +26,7 @@ import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.AbortException; import hudson.Extension; import hudson.ExtensionList; @@ -51,6 +52,7 @@ import java.util.logging.Level; import java.util.logging.Logger; import jenkins.model.Jenkins; +import jenkins.util.SystemProperties; import org.apache.commons.discovery.ResourceClassIterator; import org.apache.commons.discovery.ResourceNameIterator; import org.apache.commons.discovery.resource.ClassLoaders; @@ -62,6 +64,7 @@ import org.kohsuke.accmod.restrictions.NoExternalUse; import org.kohsuke.args4j.CmdLineException; import org.kohsuke.args4j.CmdLineParser; +import org.kohsuke.args4j.ParserProperties; import org.kohsuke.args4j.spi.OptionHandler; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.authentication.BadCredentialsException; @@ -107,6 +110,16 @@ */ @LegacyInstancesAreScopedToHudson public abstract class CLICommand implements ExtensionPoint, Cloneable { + + /** + * Boolean values to either allow or disallow parsing of @-prefixes. + * If a command line value starts with @, it is interpreted as being a file, loaded, + * and interpreted as if the file content would have been passed to the command line + */ + @SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "Accessible via System Groovy Scripts") + @Restricted(NoExternalUse.class) + public static boolean ALLOW_AT_SYNTAX = SystemProperties.getBoolean(CLICommand.class.getName() + ".allowAtSyntax"); + /** * Connected to stdout and stderr of the CLI agent that initiated the session. * IOW, if you write to these streams, the person who launched the CLI command @@ -307,7 +320,8 @@ private void logAndPrintError(Throwable e, String errorMessage, String logMessag * @since 1.538 */ protected CmdLineParser getCmdLineParser() { - return new CmdLineParser(this); + ParserProperties properties = ParserProperties.defaults().withAtSyntax(ALLOW_AT_SYNTAX); + return new CmdLineParser(this, properties); } /** diff --git a/core/src/main/java/hudson/cli/declarative/CLIRegisterer.java b/core/src/main/java/hudson/cli/declarative/CLIRegisterer.java index 8b2f2348c577..f5d050174940 100644 --- a/core/src/main/java/hudson/cli/declarative/CLIRegisterer.java +++ b/core/src/main/java/hudson/cli/declarative/CLIRegisterer.java @@ -59,6 +59,7 @@ import org.jvnet.localizer.ResourceBundleHolder; import org.kohsuke.args4j.CmdLineException; import org.kohsuke.args4j.CmdLineParser; +import org.kohsuke.args4j.ParserProperties; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.authentication.BadCredentialsException; import org.springframework.security.core.Authentication; @@ -131,7 +132,8 @@ protected CmdLineParser getCmdLineParser() { private CmdLineParser bindMethod(List binders) { registerOptionHandlers(); - CmdLineParser parser = new CmdLineParser(null); + ParserProperties properties = ParserProperties.defaults().withAtSyntax(ALLOW_AT_SYNTAX); + CmdLineParser parser = new CmdLineParser(null, properties); // build up the call sequence Stack chains = new Stack<>(); diff --git a/test/src/test/java/jenkins/security/Security3314Test.java b/test/src/test/java/jenkins/security/Security3314Test.java new file mode 100644 index 000000000000..4dbdede0a5b2 --- /dev/null +++ b/test/src/test/java/jenkins/security/Security3314Test.java @@ -0,0 +1,55 @@ +package jenkins.security; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; + +import hudson.cli.CLICommandInvoker; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.List; +import jenkins.model.Jenkins; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.jvnet.hudson.test.JenkinsRule; + +@RunWith(Parameterized.class) +public class Security3314Test { + private String commandName; + + @Rule + public final JenkinsRule j = new JenkinsRule(); + + /** + * connect-node to test the CLICommand behavior + * disable-job to test the CLIRegisterer behavior (@CLIMethod) + */ + @Parameterized.Parameters + public static List commands() { + return Arrays.asList("connect-node", "disable-job"); + } + + public Security3314Test(String commandName) { + this.commandName = commandName; + } + + @Test + public void commandShouldNotParseAt() throws Exception { + CLICommandInvoker command = new CLICommandInvoker(j, commandName); + + Path tempPath = Files.createTempFile("tempFile", ".txt"); + tempPath.toFile().deleteOnExit(); + String content = "AtGotParsed"; + Files.write(tempPath, content.getBytes()); + + final CLICommandInvoker.Result result = command + .authorizedTo(Jenkins.READ) + .invokeWithArgs("@" + tempPath); + + assertThat(result.stderr(), containsString("@" + tempPath)); + assertThat(result.stderr(), not(containsString("AtGotParsed"))); + } +}