Make Include regexp stricter to avoid deleting user data

mafredri · mafredri · commit e084a7361ff5 · 2022-06-08T11:06:15.000+03:00
diff --git a/cli/configssh.go b/cli/configssh.go
@@ -54,11 +54,13 @@ var (
 	// Find the semantically correct include statement. Since the user can
 	// modify their configuration as they see fit, there could be:
 	// - Leading indentation (space, tab)
-	// - Trailing indentation (space, tab), followed by e.g. a comment or
-	//   another file to Include (we don't want to support this, but
-	//   explicitly blocking it adds complexity)
-	// - Select newline after Include statement for removal purposes
-	sshCoderIncludedRe = regexp.MustCompile(`(?m)^[\t ]*((?i)Include) coder([\t ].*)?[\r]?[\n]?$`)
+	// - Trailing indentation (space, tab)
+	// - Select newline after Include statement for cleaner removal
+	// In the following cases, we will not recognize the Include statement
+	// and leave as-is (i.e. they're not supported):
+	// - User adds another file to the Include statement
+	// - User adds a comment on the same line as the Include statement
+	sshCoderIncludedRe = regexp.MustCompile(`(?m)^[\t ]*((?i)Include) coder([\t ])?[\r]?[\n]?$`)
 )
 
 // sshCoderConfigOptions represents options that can be stored and read
diff --git a/cli/configssh_test.go b/cli/configssh_test.go
@@ -267,6 +267,86 @@ func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) {
 				{match: "Continue?", write: "yes"},
 			},
 		},
+		{
+			name: "Included file must be named exactly coder, otherwise leave as-is",
+			writeConfig: writeConfig{
+				ssh: strings.Join([]string{
+					"Host test",
+					"  HostName test",
+					"",
+					"Include coders",
+					"",
+				}, "\n"),
+			},
+			wantConfig: wantConfig{
+				ssh: strings.Join([]string{
+					"Include coder",
+					"",
+					"Host test",
+					"  HostName test",
+					"",
+					"Include coders",
+					"",
+				}, "\n"),
+			},
+			matches: []match{
+				{match: "Continue?", write: "yes"},
+			},
+		},
+		{
+			name: "Second file added, Include(s) left as-is, new one on top",
+			writeConfig: writeConfig{
+				ssh: strings.Join([]string{
+					"Host test",
+					"  HostName test",
+					"",
+					"Include coder other",
+					"Include other coder",
+					"",
+				}, "\n"),
+			},
+			wantConfig: wantConfig{
+				ssh: strings.Join([]string{
+					"Include coder",
+					"",
+					"Host test",
+					"  HostName test",
+					"",
+					"Include coder other",
+					"Include other coder",
+					"",
+				}, "\n"),
+			},
+			matches: []match{
+				{match: "Continue?", write: "yes"},
+			},
+		},
+		{
+			name: "Comment added, Include left as-is, new one on top",
+			writeConfig: writeConfig{
+				ssh: strings.Join([]string{
+					"Host test",
+					"  HostName test",
+					"",
+					"Include coder # comment",
+					"",
+				}, "\n"),
+			},
+			wantConfig: wantConfig{
+				ssh: strings.Join([]string{
+					"Include coder",
+					"",
+					"Host test",
+					"  HostName test",
+					"",
+					"Include coder # comment",
+					"",
+				}, "\n"),
+			},
+			matches: []match{
+				{match: "Continue?", write: "yes"},
+			},
+		},
 		{
 			name: "SSH Config does not need modification",
 			writeConfig: writeConfig{
