Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reject user texts matching a regex #88

Merged
merged 23 commits into from
Feb 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
efc824a
Proof of concept filtering
davidegirardi Jan 30, 2025
84d4d39
Match either an app or the user text in the rejection conditions
davidegirardi Jan 30, 2025
2e8fdfe
Present rejection reason to the client
davidegirardi Jan 30, 2025
afd59fd
Use pointer to string instead of string
davidegirardi Jan 31, 2025
20d4c24
Refactor rejection logic to make all fields optional
davidegirardi Jan 31, 2025
8b4fb16
Clarify structure
davidegirardi Jan 31, 2025
5b07991
Exit early when a condition does not match
davidegirardi Jan 31, 2025
3f3c450
Rename reason
davidegirardi Jan 31, 2025
7227717
Add tests
davidegirardi Jan 31, 2025
187c74f
Fix comments
davidegirardi Feb 4, 2025
a221680
Explain test hack
davidegirardi Feb 4, 2025
a1c8420
Remove outdated sanity checks for rejection conditions
davidegirardi Feb 4, 2025
d99aa70
Refactor rejection to receive a payload and only return *string
davidegirardi Feb 4, 2025
7ae0cfc
Add exception for gocyclo
davidegirardi Feb 4, 2025
1df923d
Add changelog entry
davidegirardi Feb 5, 2025
71922cf
Update docs
davidegirardi Feb 5, 2025
89561f0
Remove gocyclo exclusion
davidegirardi Feb 5, 2025
7a8a3bc
Move rejection checks to per-attribute dedicated functions
davidegirardi Feb 10, 2025
a028696
Switch to boolean logic
davidegirardi Feb 11, 2025
34f0f51
Lint
davidegirardi Feb 11, 2025
af51a8a
Aling comments with current logic
davidegirardi Feb 11, 2025
5cb5334
Remove redundant call
davidegirardi Feb 11, 2025
fe5bdfa
Remove unused named return value
davidegirardi Feb 11, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/88.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Reject user text (problem description) matching a regex and send the reason why to the client-side.
2 changes: 2 additions & 0 deletions docs/blocked_rageshake.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ The rageshake server you attempted to upload a report to is not accepting ragesh
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.

The rageshake server could also be rejecting the text you wrote in your bug report because its content matches a rejection rule. This usually happens to prevent you from disclosing private information in the bug report itself.

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
Expand Down
97 changes: 55 additions & 42 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"net"
"net/http"
"os"
"regexp"
"strings"
"text/template"
"time"
Expand Down Expand Up @@ -97,55 +98,76 @@ type config struct {
GenericWebhookURLs []string `yaml:"generic_webhook_urls"`
}

// RejectionCondition contains the fields that should match a bug report for it to be rejected.
// RejectionCondition contains the fields that can match a bug report for it to be rejected.
// All the (optional) fields must match for the rejection condition to apply
type RejectionCondition struct {
// Required field: if a payload does not match this app name, the condition does not match.
// App name, applies only if not empty
App string `yaml:"app"`
// Optional: version that must also match in addition to the app and label. If empty, does not check version.
// Version, applies only if not empty
Version string `yaml:"version"`
// Optional: label that must also match in addition to the app and version. If empty, does not check label.
// Label, applies only if not empty
Label string `yaml:"label"`
// Message sent by the user, applies only if not empty
UserTextMatch string `yaml:"usertext"`
// Send this text to the client-side to inform the user why the server rejects the rageshake. Uses a default generic value if empty.
Reason string `yaml:"reason"`
}

// shouldReject returns true if the app name AND version AND labels all match the rejection condition.
// If any one of these do not match the condition, it is not rejected.
func (c RejectionCondition) shouldReject(appName, version string, labels []string) bool {
if appName != c.App {
return false
}
// version was a condition and it doesn't match => accept it
if version != c.Version && c.Version != "" {
return false
func (c RejectionCondition) matchesApp(p *payload) bool {
// Empty `RejectionCondition.App` is a wildcard which matches anything
return c.App == "" || c.App == p.AppName
}

func (c RejectionCondition) matchesVersion(p *payload) bool {
version := ""
if p.Data != nil {
version = p.Data["Version"]
}
// Empty `RejectionCondition.Version` is a wildcard which matches anything
return c.Version == "" || c.Version == version
}

// label was a condition and no label matches it => accept it
if c.Label != "" {
labelMatch := false
for _, l := range labels {
if l == c.Label {
labelMatch = true
break
}
}
if !labelMatch {
return false
func (c RejectionCondition) matchesLabel(p *payload) bool {
// Empty `RejectionCondition.Label` is a wildcard which matches anything
if c.Label == "" {
return true
}
// Otherwise return true only if there is a label that matches
labelMatch := false
for _, l := range p.Labels {
if l == c.Label {
labelMatch = true
break
}
}
return labelMatch
}

return true
func (c RejectionCondition) matchesUserText(p *payload) bool {
// Empty `RejectionCondition.UserTextMatch` is a wildcard which matches anything
return c.UserTextMatch == "" || regexp.MustCompile(c.UserTextMatch).MatchString(p.UserText)
}

func (c *config) matchesRejectionCondition(p *payload) bool {
for _, rc := range c.RejectionConditions {
version := ""
if p.Data != nil {
version = p.Data["Version"]
func (c RejectionCondition) shouldReject(p *payload) *string {
if c.matchesApp(p) && c.matchesVersion(p) && c.matchesLabel(p) && c.matchesUserText(p) {
// RejectionCondition matches all of the conditions: we should reject this submission/
defaultReason := "app or user text rejected"
if c.Reason != "" {
return &c.Reason
}
if rc.shouldReject(p.AppName, version, p.Labels) {
return true
return &defaultReason
}
return nil
}

func (c *config) matchesRejectionCondition(p *payload) *string {
for _, rc := range c.RejectionConditions {
reject := rc.shouldReject(p)
if reject != nil {
return reject
}
}
return false
return nil
}

func basicAuth(handler http.Handler, username, password, realm string) http.Handler {
Expand Down Expand Up @@ -319,14 +341,5 @@ func loadConfig(configPath string) (*config, error) {
if err = yaml.Unmarshal(contents, &cfg); err != nil {
return nil, err
}
// sanity check rejection conditions
for _, rc := range cfg.RejectionConditions {
if rc.App == "" {
fmt.Println("rejection_condition missing an app field so will never match anything.")
}
if rc.Label == "" && rc.Version == "" {
fmt.Println("rejection_condition missing both label and version so will always match, specify label and/or version")
}
}
return &cfg, nil
}
51 changes: 44 additions & 7 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,54 +17,86 @@ func TestConfigRejectionCondition(t *testing.T) {
App: "my-app",
Version: "0.1.2",
Label: "nightly",
Reason: "no nightlies",
},
{
App: "block-my-app",
},
{
UserTextMatch: "(\\w{4}\\s){11}\\w{4}",
Reason: "it matches a recovery key and recovery keys are private",
},
},
}
rejectPayloads := []payload{
davidegirardi marked this conversation as resolved.
Show resolved Hide resolved
{
AppName: "my-app",
Data: map[string]string{
"Version": "0.1.0",
// Hack add how we expect the rageshake to be rejected to the test
// The actual data in a rageshake has no ExpectedRejectReason field
"ExpectedRejectReason": "app or user text rejected",
},
},
{
AppName: "my-app",
Data: map[string]string{},
Labels: []string{"0.1.1"},
Data: map[string]string{
"ExpectedRejectReason": "app or user text rejected",
},
Labels: []string{"0.1.1"},
},
{
AppName: "my-app",
Labels: []string{"foo", "nightly"},
Data: map[string]string{
"Version": "0.1.2",
"Version": "0.1.2",
"ExpectedRejectReason": "no nightlies",
},
},
{
AppName: "block-my-app",
Data: map[string]string{
"ExpectedRejectReason": "app or user text rejected",
},
},
{
AppName: "block-my-app",
Labels: []string{"foo"},
Data: map[string]string{
"ExpectedRejectReason": "app or user text rejected",
},
},
{
AppName: "block-my-app",
Data: map[string]string{
"Version": "42",
"Version": "42",
"ExpectedRejectReason": "app or user text rejected",
},
},
{
AppName: "block-my-app",
Labels: []string{"foo"},
Data: map[string]string{
"Version": "42",
"Version": "42",
"ExpectedRejectReason": "app or user text rejected",
},
},
{
AppName: "my-app",
UserText: "Looks like a recover key abcd abcd abcd abcd abcd abcd abcd abcd abcd abcd abcd abcd",
Data: map[string]string{
"ExpectedRejectReason": "it matches a recovery key and recovery keys are private",
},
},
}
for _, p := range rejectPayloads {
if !cfg.matchesRejectionCondition(&p) {
reject := cfg.matchesRejectionCondition(&p)
if reject != nil {
if *reject != p.Data["ExpectedRejectReason"] {
t.Errorf("payload was rejected with the wrong reason:\n payload=%+v\nconfig=%+v", p, cfg)
}
}
if reject == nil {
t.Errorf("payload was accepted when it should be rejected:\n payload=%+v\nconfig=%+v", p, cfg)
}
}
Expand Down Expand Up @@ -112,9 +144,14 @@ func TestConfigRejectionCondition(t *testing.T) {
"Version": "0.1.2",
},
},
{
AppName: "my-app",
UserText: "Some description",
},
}
for _, p := range acceptPayloads {
if cfg.matchesRejectionCondition(&p) {
reject := cfg.matchesRejectionCondition(&p)
if reject != nil {
t.Errorf("payload was rejected when it should be accepted:\n payload=%+v\nconfig=%+v", p, cfg)
}
}
Expand Down
7 changes: 5 additions & 2 deletions rageshake.sample.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ listings_auth_pass: secret
allowed_app_names: []

# If any submission matches one of these rejection conditions, the submission is rejected.
# The 'app' field is required, but 'version' and 'label' are both optional. A condition with just
# an 'app' will reject that app altogether, effectively acting as a blocklist for app in contrast to allowed_app_names.
# A condition is made by an union of optional fields: app, version, labels, user text. They all need to match for rejecting the rageshake
# It can also contain an optional reason to explain why this server is rejecting a user's submission.
rejection_conditions:
- app: my-app
version: "0.4.9" # if the submission has a Version which is exactly this value, reject the submission.
Expand All @@ -23,6 +23,9 @@ rejection_conditions:
- app: my-app
version: "0.4.9"
label: "nightly" # both label and Version must match for this condition to be true
reason: "this server does not accept rageshakes from nightlies"
- usertext: "(\\w{4}\\s){11}\\w{4}" # reject text containing possible recovery keys
reason: "it matches a recovery key and recovery keys are private"

# a GitHub personal access token (https://github.com/settings/tokens), which
# will be used to create a GitHub issue for each report. It requires
Expand Down
8 changes: 5 additions & 3 deletions submit.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,13 +226,15 @@ func (s *submitServer) handleSubmission(w http.ResponseWriter, req *http.Request
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
}
if s.cfg.matchesRejectionCondition(p) {
log.Printf("Blocking rageshake from app %s because it matches a rejection_condition", p.AppName)
rejection := s.cfg.matchesRejectionCondition(p)
if rejection != nil {
log.Printf("Blocking rageshake from app %s because it matches a rejection_condition: %s", p.AppName, *rejection)
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 + version. See https://github.com/matrix-org/rageshake/blob/master/docs/blocked_rageshake.md", 400)
userErrorText := fmt.Sprintf("This server did not accept the rageshake because it matches a rejection condition: %s. See https://github.com/matrix-org/rageshake/blob/master/docs/blocked_rageshake.md", *rejection)
http.Error(w, userErrorText, 400)
return
}

Expand Down
Loading