Block applications not listed in an allow list. (#67)

Optionally block applications not listed in the allow list.
Remove legacy riot-android workaround for missing appName.
Add docs file to give context on why reports are now being rejected.
Give more detail on lint failures through script

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

changelog.d/67.feature

@@ -0,0 +1 @@
+Add config option to block unknown appplication names.

docs/blocked_rageshake.md

@@ -0,0 +1,35 @@
+# Rageshake server not accepting rageshakes
+
+This page contains information useful to someone who has had their rageshake rejected by a rageshake server.
+
+We include it within the error messages to provide a place with context for users reading the error message and wanting
+to know more.
+
+## For matrix client users
+
+Thank you for attempting to report a bug with your matrix client; unfortunately your client application is likely incorrectly configured.
+
+The rageshake server you attempted to upload a report to is not accepting rageshakes from your client at this time.
+
+Generally, the developers who run a rageshake server will only be able to handle reports for applications they are developing,
+and your application is not listed as one of those applications.
+
+Please contact the distributor of your application or the administrator of the web site you visit to report this as a problem.
+
+## For developers of matrix clients
+
+Your application is likely based on one of the matrix SDKs or element applications, if it is submitting rageshakes to a rageshake server.
+
+A change has been made to pre-filter reports that the developers using this rageshake server for applications they do not have control over.
+Typically reports from unknown applications would have to be manually triaged and discarded; there is now automatic filtering in place, which reduces overall effort.
+
+There is generally a configuration file in your application that you can alter to change where these reports are sent, which may require rebuilding and releasing the client.
+
+The easiest solution to this error is to stop sending rageshakes entirely, which may require a code or configuration change in your client.
+
+However, if you wish to accept bug reports from your users applications, you will need to run your own copy of this rageshake server and update the URL appropriately.
+
+## Application specific config locations:
+ * element-web: `bug_report_endpoint_url` in the [sample configuration for element web](https://github.com/vector-im/element-web/blob/develop/config.sample.json).
+ * element-ios: `bugReportEndpointUrlString` in the [BuildSettings.swift](https://github.com/vector-im/element-ios/blob/develop/Config/BuildSettings.swift)
+ * element-android: `bug_report_url` in the [config.xml file for the build](https://github.com/vector-im/element-android/blob/develop/vector-config/src/main/res/values/config.xml)

main.go

@@ -48,6 +48,9 @@ type config struct {
 	// External URI to /api
 	APIPrefix string `yaml:"api_prefix"`
 
+	// Allowed rageshake app names
+	AllowedAppNames []string `yaml:"allowed_app_names"`
+
 	// A GitHub personal access token, to create a GitHub issue for each report.
 	GithubToken string `yaml:"github_token"`
 
@@ -149,10 +152,13 @@ func main() {
 		// remove trailing /
 		apiPrefix = strings.TrimRight(apiPrefix, "/")
 	}
+
+	appNameMap := configureAppNameMap(cfg)
+
 	log.Printf("Using %s/listing as public URI", apiPrefix)
 
 	rand.Seed(time.Now().UnixNano())
-	http.Handle("/api/submit", &submitServer{ghClient, glClient, apiPrefix, slack, genericWebhookClient, cfg})
+	http.Handle("/api/submit", &submitServer{ghClient, glClient, apiPrefix, slack, genericWebhookClient, appNameMap, cfg})
 
 	// Make sure bugs directory exists
 	_ = os.Mkdir("bugs", os.ModePerm)
@@ -180,6 +186,17 @@ func main() {
 	log.Fatal(http.ListenAndServe(*bindAddr, nil))
 }
 
+func configureAppNameMap(cfg *config) map[string]bool {
+	if len(cfg.AllowedAppNames) == 0 {
+		fmt.Println("Warning: allowed_app_names is empty. Accepting requests from all app names")
+	}
+	var allowedAppNameMap = make(map[string]bool)
+	for _, app := range cfg.AllowedAppNames {
+		allowedAppNameMap[app] = true
+	}
+	return allowedAppNameMap
+}
+
 func configureGenericWebhookClient(cfg *config) *http.Client {
 	if len(cfg.GenericWebhookURLs) == 0 {
 		fmt.Println("No generic_webhook_urls configured.")

rageshake.sample.yaml

@@ -8,6 +8,10 @@ listings_auth_pass: secret
 # report to the GitHub issue. If unspecified, based on the listen address.
 # api_prefix: https://riot.im/bugreports
 
+# List of approved AppNames we accept. Names not in the list or missing an application name will be rejected.
+# An empty or missing list will retain legacy behaviour and permit reports from any application name.
+allowed_app_names: []
+
 # a GitHub personal access token (https://github.com/settings/tokens), which
 # will be used to create a GitHub issue for each report. It requires
 # `public_repo` scope. If omitted, no issues will be created.

scripts/lint.sh

@@ -7,6 +7,9 @@
 
 set -eu
 
+echo "golint:"
 golint -set_exit_status
+echo "go vet:"
 go vet -vettool=$(which shadow)
+echo "gocyclo:"
 gocyclo -over 12 .

submit.go

@@ -58,6 +58,7 @@ type submitServer struct {
 	slack *slackClient
 
 	genericWebhookClient *http.Client
+	allowedAppNameMap    map[string]bool
 	cfg                  *config
 }
 
@@ -191,6 +192,17 @@ func (s *submitServer) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		return
 	}
 
+	// Filter out unwanted rageshakes, if a list is defined
+	if len(s.allowedAppNameMap) != 0 && !s.allowedAppNameMap[p.AppName] {
+		log.Printf("Blocking rageshake because app name %s not in list", p.AppName)
+		if err := os.RemoveAll(reportDir); err != nil {
+			log.Printf("Unable to remove report dir %s after rejected upload: %v\n",
+				reportDir, err)
+		}
+		http.Error(w, "This server does not accept rageshakes from your application. See https://github.com/matrix-org/rageshake/blob/master/docs/blocked_rageshake.md", 400)
+		return
+	}
+
 	// We use this prefix (eg, 2022-05-01/125223-abcde) as a unique identifier for this rageshake.
 	// This is going to be used to uniquely identify rageshakes, even if they are not submitted to
 	// an issue tracker for instance with automatic rageshakes that can be plentiful
@@ -243,6 +255,7 @@ func parseRequest(w http.ResponseWriter, req *http.Request, reportDir string) *p
 		http.Error(w, fmt.Sprintf("Could not decode payload: %s", err.Error()), 400)
 		return nil
 	}
+
 	return p
 }
 
@@ -273,35 +286,13 @@ func parseJSONRequest(w http.ResponseWriter, req *http.Request, reportDir string
 		}
 	}
 
-	// backwards-compatibility hack: current versions of riot-android
-	// don't set 'app', so we don't correctly file github issues.
-	if p.AppName == "" && p.UserAgent == "Android" {
-		parsed.AppName = "riot-android"
+	parsed.AppName = p.AppName
 
-		// they also shove lots of stuff into 'Version' which we don't really
-		// want in the github report
-		for _, line := range strings.Split(p.Version, "\n") {
-			line = strings.TrimSpace(line)
-			if line == "" {
-				continue
-			}
-			parts := strings.SplitN(line, ":", 2)
-			key := strings.TrimSpace(parts[0])
-			val := ""
-			if len(parts) > 1 {
-				val = strings.TrimSpace(parts[1])
-			}
-			parsed.Data[key] = val
-		}
-	} else {
-		parsed.AppName = p.AppName
-
-		if p.UserAgent != "" {
-			parsed.Data["User-Agent"] = p.UserAgent
-		}
-		if p.Version != "" {
-			parsed.Data["Version"] = p.Version
-		}
+	if p.UserAgent != "" {
+		parsed.Data["User-Agent"] = p.UserAgent
+	}
+	if p.Version != "" {
+		parsed.Data["Version"] = p.Version
 	}
 
 	return &parsed, nil

submit_test.go

@@ -56,6 +56,55 @@ func testParsePayload(t *testing.T, body, contentType string, tempDir string) (*
 	return p, rr.Result()
 }
 
+
+func submitSimpleRequestToServer(t *testing.T, allowedAppNameMap map[string]bool, body string) int {
+        // Submit a request without files to the server and return statusCode
+        // Could be extended with more complicated config; aimed here just to
+        // test options for allowedAppNameMap
+
+	req, err := http.NewRequest("POST", "/api/submit", strings.NewReader(body))
+	if err != nil {
+		t.Fatal(err)
+	}
+	req.Header.Set("Content-Length", strconv.Itoa(len(body)))
+	w := httptest.NewRecorder()
+
+	var cfg config
+	s := &submitServer{nil, nil, "/", nil, nil, allowedAppNameMap, &cfg}
+
+	s.ServeHTTP(w, req)
+	rsp := w.Result()
+	return rsp.StatusCode
+}
+
+func TestAppNames(t *testing.T) {
+	body := `{
+    "app": "alice",
+    "logs": [ ],
+    "text": "test message",
+    "user_agent": "Mozilla/0.9",
+    "version": "0.9.9"
+}`
+	validAppNameMap := map[string]bool{
+		"alice": true,
+	}
+	if submitSimpleRequestToServer(t, validAppNameMap, body) != 200 {
+		t.Fatal("matching app was not accepted")
+	}
+
+	invalidAppNameMap := map[string]bool{
+		"bob": true,
+	}
+	if submitSimpleRequestToServer(t, invalidAppNameMap, body) != 400 {
+		t.Fatal("nonmatching app was not rejected")
+	}
+
+	emptyAppNameMap := make(map[string]bool)
+	if submitSimpleRequestToServer(t, emptyAppNameMap, body) != 200 {
+		t.Fatal("empty map did not allow all")
+	}
+}
+
 func TestEmptyJson(t *testing.T) {
 	body := "{}"
 
@@ -108,36 +157,6 @@ func TestJsonUpload(t *testing.T) {
 	checkUploadedFile(t, reportDir, "logs-0000.log.gz", true, "line1\nline2")
 }
 
-// check that we can unpick the json submitted by the android clients
-func TestUnpickAndroidMangling(t *testing.T) {
-	body := `{"text": "test ylc 001",
-"version": "User : @ylc8001:matrix.org\nPhone : Lenovo P2a42\nVector version: 0:6:9\n",
-"user_agent": "Android"
-}`
-	p, _ := testParsePayload(t, body, "", "")
-	if p == nil {
-		t.Fatal("parseRequest returned nil")
-	}
-	if p.UserText != "test ylc 001" {
-		t.Errorf("user text: got %s, want %s", p.UserText, "test ylc 001")
-	}
-	if p.AppName != "riot-android" {
-		t.Errorf("appname: got %s, want %s", p.AppName, "riot-android")
-	}
-	if p.Data["Version"] != "" {
-		t.Errorf("version: got %s, want ''", p.Data["Version"])
-	}
-	if p.Data["User"] != "@ylc8001:matrix.org" {
-		t.Errorf("data.user: got %s, want %s", p.Data["User"], "@ylc8001:matrix.org")
-	}
-	if p.Data["Phone"] != "Lenovo P2a42" {
-		t.Errorf("data.phone: got %s, want %s", p.Data["Phone"], "Lenovo P2a42")
-	}
-	if p.Data["Vector version"] != "0:6:9" {
-		t.Errorf("data.version: got %s, want %s", p.Data["Version"], "0:6:9")
-	}
-}
-
 func TestMultipartUpload(t *testing.T) {
 	reportDir := mkTempDir(t)
 	defer os.RemoveAll(reportDir)