Skip to content

Commit c6c8c12

Browse files
MarkEWaiteMarkEWaite
committed
Merge branch 'stable-6.6'
2 parents 4b93ef5 + fc5f068 commit c6c8c12

5 files changed

Lines changed: 172 additions & 32 deletions

File tree

‎pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
</scm>
3333

3434
<properties>
35-
<revision>6.6.1</revision>
35+
<revision>6.6.2</revision>
3636
<changelist>-SNAPSHOT</changelist>
3737
<!-- Character set tests fail unless file.encoding is set -->
3838
<argLine>-Dfile.encoding=${project.build.sourceEncoding}</argLine>

‎src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2004,7 +2004,7 @@ Path createTempFile(String prefix, String suffix) throws IOException {
20042004
return Files.createTempFile(tmpPath, prefix, suffix);
20052005
}
20062006
// Unix specific
2007-
if (absolutePath.contains("`")) {
2007+
if (absolutePath.contains("`") || absolutePath.contains("$(")) {
20082008
// Avoid backquote shell expansion
20092009
return createTempFileInSystemDir(prefix, suffix);
20102010
}
@@ -2118,7 +2118,11 @@ private String launchCommandWithCredentials(
21182118
userName = sshUser.getUsername();
21192119
}
21202120
passphrase = createPassphraseFile(sshUser);
2121-
knownHostsTemp = createTempFile("known_hosts", "");
2121+
// The known hosts file lists the public keys of the git ssh servers.
2122+
// Public keys of servers are not sensitive information.
2123+
// They can be stored in the system temporary directory.
2124+
// Avoids issues with workspace names in the known hosts file path.
2125+
knownHostsTemp = Files.createTempFile("known_hosts", "");
21222126
if (launcher.isUnix()) {
21232127
ssh = createUnixGitSSH(key, userName, knownHostsTemp);
21242128
askpass = createUnixSshAskpass(sshUser, passphrase);

‎src/main/java/org/jenkinsci/plugins/gitclient/verifier/ManuallyProvidedKeyVerifier.java‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,26 +30,24 @@ public AbstractCliGitHostKeyVerifier forCliGit(TaskListener listener) {
3030
Level.FINEST,
3131
"Verifying manually-configured host keys entry in {0} with host keys {1}",
3232
new Object[] {tempKnownHosts.toAbsolutePath(), approvedHostKeys});
33-
String userKnownHostsFileFlag;
34-
if (File.pathSeparatorChar
35-
== ';') { // check whether on Windows or not without sending Functions over remoting
36-
// no escaping for windows because created temp file can't contain spaces
37-
userKnownHostsFileFlag = " -o UserKnownHostsFile=%s".formatted(escapePath(tempKnownHosts));
38-
} else {
39-
// escaping needed in case job name contains spaces
40-
userKnownHostsFileFlag =
41-
" -o UserKnownHostsFile=\\\"\"\"%s\\\"\"\"".formatted(escapePath(tempKnownHosts));
42-
}
43-
return "-o StrictHostKeyChecking=yes " + userKnownHostsFileFlag;
33+
String path = escapePath(tempKnownHosts);
34+
LOGGER.log(Level.FINEST, "-o StrictHostKeyChecking=yes -o UserKnownHostsFile={0}", path);
35+
return "-o StrictHostKeyChecking=yes -o UserKnownHostsFile=%s".formatted(path);
4436
};
4537
}
4638

4739
private static String escapePath(Path path) {
48-
if (File.pathSeparatorChar == ';') // check whether on Windows or not without sending Functions over remoting
49-
{
50-
return path.toAbsolutePath().toString();
40+
String p = path.toAbsolutePath().toString();
41+
// check whether on Windows or not without sending Functions over remoting
42+
if (File.pathSeparatorChar == ';') {
43+
// no escaping for windows because created temp file can't contain spaces
44+
return p;
5145
}
52-
return path.toAbsolutePath().toString().replace(" ", "\\ ");
46+
if (p.contains("'")) {
47+
p = p.replace("'", "\\'");
48+
}
49+
// OpenSSH needs arguments that contain a space to be surrounded with double quotes
50+
return p.contains(" ") ? "'\"" + p + "\"'" : "'" + p + "'";
5351
}
5452

5553
@NonNull
@@ -58,6 +56,7 @@ public File getKnownHostsFile() {
5856
try {
5957
Path tempKnownHosts = Files.createTempFile("known_hosts", "");
6058
Files.write(tempKnownHosts, (approvedHostKeys + System.lineSeparator()).getBytes(StandardCharsets.UTF_8));
59+
LOGGER.log(Level.FINEST, "Known hosts written to {0}", tempKnownHosts);
6160
return tempKnownHosts.toFile();
6261
} catch (IOException e) {
6362
throw new RuntimeException(e);

‎src/test/java/org/jenkinsci/plugins/gitclient/CliGitAPILinuxTest.java‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package org.jenkinsci.plugins.gitclient;
2+
3+
import static org.hamcrest.CoreMatchers.containsString;
4+
import static org.hamcrest.CoreMatchers.not;
5+
import static org.hamcrest.MatcherAssert.assertThat;
6+
7+
import java.io.File;
8+
import java.nio.file.Path;
9+
import org.junit.jupiter.api.Test;
10+
import org.junit.jupiter.api.io.TempDir;
11+
12+
class CliGitAPILinuxTest {
13+
14+
@TempDir
15+
private static File tempDir;
16+
17+
@Test
18+
void test_invalid_workspace_dir_name_rce() throws Exception {
19+
String rceInjection = "$(date>>/tmp/rce-proof)";
20+
File workspaceDir = new File(tempDir, rceInjection);
21+
workspaceDir.mkdirs();
22+
CliGitAPIImpl cliGit = new CliGitAPIImpl("git", workspaceDir, null, null);
23+
Path file = cliGit.createTempFile("something", ".suffix");
24+
assertThat(file.toAbsolutePath().toString(), not(containsString(rceInjection)));
25+
}
26+
27+
@Test
28+
void test_valid_workspace_dir_name_single_quote() throws Exception {
29+
String singleQuote = "Bob's-job";
30+
File workspaceDir = new File(tempDir, singleQuote);
31+
workspaceDir.mkdirs();
32+
CliGitAPIImpl cliGit = new CliGitAPIImpl("git", workspaceDir, null, null);
33+
Path file = cliGit.createTempFile("something", ".suffix");
34+
assertThat(file.toAbsolutePath().toString(), containsString(singleQuote));
35+
}
36+
37+
@Test
38+
void test_valid_workspace_dir_name_parentheses() throws Exception {
39+
String parentheses = "My OWASP (special) case";
40+
File workspaceDir = new File(tempDir, parentheses);
41+
workspaceDir.mkdirs();
42+
CliGitAPIImpl cliGit = new CliGitAPIImpl("git", workspaceDir, null, null);
43+
Path file = cliGit.createTempFile("something", ".suffix");
44+
if (isWindows()) {
45+
assertThat(file.toAbsolutePath().toString(), not(containsString(parentheses)));
46+
} else {
47+
assertThat(file.toAbsolutePath().toString(), containsString(parentheses));
48+
}
49+
}
50+
51+
private static boolean isWindows() {
52+
return File.pathSeparatorChar == ';';
53+
}
54+
}

‎src/test/java/org/jenkinsci/plugins/gitclient/verifier/ManuallyProvidedKeyVerifierTest.java‎

Lines changed: 97 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import static org.hamcrest.Matchers.is;
55
import static org.jenkinsci.plugins.gitclient.verifier.KnownHostsTestUtil.nonGitHubHost;
66
import static org.jenkinsci.plugins.gitclient.verifier.KnownHostsTestUtil.runKnownHostsTests;
7+
import static org.junit.jupiter.api.Assumptions.assumeFalse;
78
import static org.junit.jupiter.api.Assumptions.assumeTrue;
89

910
import hudson.model.StreamBuildListener;
@@ -62,7 +63,7 @@ void connectWhenHostKeyProvidedThenShouldNotFail() throws Exception {
6263
ManuallyProvidedKeyVerifier.ManuallyProvidedKeyJGitHostKeyVerifier jGitHostKeyVerifier =
6364
(ManuallyProvidedKeyVerifier.ManuallyProvidedKeyJGitHostKeyVerifier)
6465
verifier.forJGit(StreamBuildListener.fromStdout());
65-
Path tempKnownHosts = Files.createTempFile("known_hosts", "");
66+
Path tempKnownHosts = Files.createTempFile(testFolder.toPath(), "known_hosts", "");
6667
Files.writeString(tempKnownHosts, hostKey + System.lineSeparator());
6768
KnownHostsTestUtil.connectToHost(
6869
"github.com", 22, tempKnownHosts.toFile(), jGitHostKeyVerifier, "ecdsa-sha2-nistp256", s -> {
@@ -84,7 +85,7 @@ void connectWhenWrongHostKeyProvidedThenShouldFail() throws Exception {
8485
ManuallyProvidedKeyVerifier.ManuallyProvidedKeyJGitHostKeyVerifier jGitHostKeyVerifier =
8586
(ManuallyProvidedKeyVerifier.ManuallyProvidedKeyJGitHostKeyVerifier)
8687
verifier.forJGit(StreamBuildListener.fromStdout());
87-
Path tempKnownHosts = Files.createTempFile("known_hosts", "");
88+
Path tempKnownHosts = Files.createTempFile(testFolder.toPath(), "known_hosts", "");
8889
Files.writeString(tempKnownHosts, key + System.lineSeparator());
8990
KnownHostsTestUtil.connectToHost(
9091
"github.com", 22, tempKnownHosts.toFile(), jGitHostKeyVerifier, "ssh-ed25519", s -> {
@@ -106,7 +107,7 @@ void connectWhenHostKeyProvidedWithPortThenShouldNotFail() throws Exception {
106107
ManuallyProvidedKeyVerifier.ManuallyProvidedKeyJGitHostKeyVerifier jGitHostKeyVerifier =
107108
(ManuallyProvidedKeyVerifier.ManuallyProvidedKeyJGitHostKeyVerifier)
108109
verifier.forJGit(StreamBuildListener.fromStdout());
109-
Path tempKnownHosts = Files.createTempFile("known_hosts", "");
110+
Path tempKnownHosts = Files.createTempFile(testFolder.toPath(), "known_hosts", "");
110111
Files.writeString(tempKnownHosts, key + System.lineSeparator());
111112
KnownHostsTestUtil.connectToHost(
112113
"github.com", 22, tempKnownHosts.toFile(), jGitHostKeyVerifier, "ecdsa-sha2-nistp256", s -> {
@@ -120,30 +121,112 @@ void connectWhenHostKeyProvidedWithPortThenShouldNotFail() throws Exception {
120121

121122
@Test
122123
void testGetVerifyHostKeyOption() throws Exception {
123-
if (isWindows()) {
124-
return; // Skip test without generating a Maven surefire warning
124+
Path tempFile = Files.createTempFile(testFolder.toPath(), "manual-host-key", "");
125+
String actual = new ManuallyProvidedKeyVerifier(hostKey)
126+
.forCliGit(TaskListener.NULL)
127+
.getVerifyHostKeyOption(tempFile);
128+
String fileArg = tempFile.toAbsolutePath().toString();
129+
if (!isWindows()) {
130+
fileArg = "'" + fileArg + "'";
125131
}
126-
Path tempFile = File.createTempFile("junit", null, testFolder).toPath();
132+
assertThat(actual, is("-o StrictHostKeyChecking=yes -o UserKnownHostsFile=" + fileArg));
133+
assertThat(Files.readAllLines(tempFile), is(Collections.singletonList(hostKey)));
134+
}
135+
136+
@Test
137+
void testGetVerifyHostKeyOptionSpace() throws Exception {
138+
assumeFalse(isWindows(), "Embedded space not allowed on Windows for git plugin temporary files");
139+
Path tempFile = Files.createTempFile(testFolder.toPath(), "x y", "");
127140
String actual = new ManuallyProvidedKeyVerifier(hostKey)
128141
.forCliGit(TaskListener.NULL)
129142
.getVerifyHostKeyOption(tempFile);
130-
assertThat(
131-
actual,
132-
is("-o StrictHostKeyChecking=yes -o UserKnownHostsFile=\\\"\"\"" + tempFile.toAbsolutePath()
133-
+ "\\\"\"\""));
143+
String fileArg = tempFile.toAbsolutePath().toString();
144+
fileArg = "'\"" + fileArg + "\"'";
145+
assertThat(actual, is("-o StrictHostKeyChecking=yes -o UserKnownHostsFile=" + fileArg));
134146
assertThat(Files.readAllLines(tempFile), is(Collections.singletonList(hostKey)));
135147
}
136148

137149
@Test
138-
void testGetVerifyHostKeyOptionOnWindows() throws Exception {
150+
void testGetVerifyHostKeyOptionParentheses() throws Exception {
151+
Path tempFile = Files.createTempFile(testFolder.toPath(), "paren(1)", "");
152+
String actual = new ManuallyProvidedKeyVerifier(hostKey)
153+
.forCliGit(TaskListener.NULL)
154+
.getVerifyHostKeyOption(tempFile);
155+
String fileArg = tempFile.toAbsolutePath().toString();
139156
if (!isWindows()) {
140-
return; // Skip test without generating a Maven surefire warning
157+
fileArg = "'" + fileArg + "'";
141158
}
142-
Path tempFile = File.createTempFile("junit", null, testFolder).toPath();
159+
assertThat(actual, is("-o StrictHostKeyChecking=yes -o UserKnownHostsFile=" + fileArg));
160+
assertThat(Files.readAllLines(tempFile), is(Collections.singletonList(hostKey)));
161+
}
162+
163+
@Test
164+
void testGetVerifyHostKeyOptionParenthesesAndSpace() throws Exception {
165+
assumeFalse(isWindows(), "Embedded space not allowed on Windows for git plugin temporary files");
166+
Path tempFile = Files.createTempFile(testFolder.toPath(), "paren (2)", "");
167+
String actual = new ManuallyProvidedKeyVerifier(hostKey)
168+
.forCliGit(TaskListener.NULL)
169+
.getVerifyHostKeyOption(tempFile);
170+
String fileArg = tempFile.toAbsolutePath().toString();
171+
fileArg = "'\"" + fileArg + "\"'";
172+
assertThat(actual, is("-o StrictHostKeyChecking=yes -o UserKnownHostsFile=" + fileArg));
173+
assertThat(Files.readAllLines(tempFile), is(Collections.singletonList(hostKey)));
174+
}
175+
176+
@Test
177+
void testGetVerifyHostKeyOptionShellEscape() throws Exception {
178+
Path tempFile = Files.createTempFile(testFolder.toPath(), "shell-$(date)", "");
143179
String actual = new ManuallyProvidedKeyVerifier(hostKey)
144180
.forCliGit(TaskListener.NULL)
145181
.getVerifyHostKeyOption(tempFile);
146-
assertThat(actual, is("-o StrictHostKeyChecking=yes -o UserKnownHostsFile=" + tempFile.toAbsolutePath()));
182+
String fileArg = tempFile.toAbsolutePath().toString();
183+
if (!isWindows()) {
184+
fileArg = "'" + fileArg + "'";
185+
}
186+
assertThat(actual, is("-o StrictHostKeyChecking=yes -o UserKnownHostsFile=" + fileArg));
187+
assertThat(Files.readAllLines(tempFile), is(Collections.singletonList(hostKey)));
188+
}
189+
190+
@Test
191+
void testGetVerifyHostKeyOptionShellEscape1() throws Exception {
192+
Path tempFile = Files.createTempFile(testFolder.toPath(), "shell-`date`", "");
193+
String actual = new ManuallyProvidedKeyVerifier(hostKey)
194+
.forCliGit(TaskListener.NULL)
195+
.getVerifyHostKeyOption(tempFile);
196+
String fileArg = tempFile.toAbsolutePath().toString();
197+
if (!isWindows()) {
198+
fileArg = "'" + fileArg + "'";
199+
}
200+
assertThat(actual, is("-o StrictHostKeyChecking=yes -o UserKnownHostsFile=" + fileArg));
201+
assertThat(Files.readAllLines(tempFile), is(Collections.singletonList(hostKey)));
202+
}
203+
204+
@Test
205+
void testGetVerifyHostKeyOptionSingleQuoteAndSpace() throws Exception {
206+
assumeFalse(isWindows(), "Embedded space not allowed on Windows for git plugin temporary files");
207+
Path tempFile = Files.createTempFile(testFolder.toPath(), "Bob's Job", "");
208+
String actual = new ManuallyProvidedKeyVerifier(hostKey)
209+
.forCliGit(TaskListener.NULL)
210+
.getVerifyHostKeyOption(tempFile);
211+
String fileArg = tempFile.toAbsolutePath().toString();
212+
fileArg = fileArg.replace("'", "\\'"); // Expect escaped single quotes inside the string
213+
fileArg = "'\"" + fileArg + "\"'";
214+
assertThat(actual, is("-o StrictHostKeyChecking=yes -o UserKnownHostsFile=" + fileArg));
215+
assertThat(Files.readAllLines(tempFile), is(Collections.singletonList(hostKey)));
216+
}
217+
218+
@Test
219+
void testGetVerifyHostKeyOptionSingleQuote() throws Exception {
220+
Path tempFile = Files.createTempFile(testFolder.toPath(), "Bob's-Job", "");
221+
String actual = new ManuallyProvidedKeyVerifier(hostKey)
222+
.forCliGit(TaskListener.NULL)
223+
.getVerifyHostKeyOption(tempFile);
224+
String fileArg = tempFile.toAbsolutePath().toString();
225+
if (!isWindows()) {
226+
fileArg = fileArg.replace("'", "\\'"); // Expect escaped single quotes inside the string
227+
fileArg = "'" + fileArg + "'";
228+
}
229+
assertThat(actual, is("-o StrictHostKeyChecking=yes -o UserKnownHostsFile=" + fileArg));
147230
assertThat(Files.readAllLines(tempFile), is(Collections.singletonList(hostKey)));
148231
}
149232

0 commit comments

Comments
 (0)