diff --git a/internal/hook/hook.go b/internal/hook/hook.go index 1131eb4..2ec3525 100644 --- a/internal/hook/hook.go +++ b/internal/hook/hook.go @@ -46,6 +46,28 @@ const ( EnvNamespace string = "HOOK_" ) +// ParameterNodeError describes an error walking a parameter node. +type ParameterNodeError struct { + key string +} + +func (e *ParameterNodeError) Error() string { + if e == nil { + return "" + } + return fmt.Sprintf("parameter node not found: %s", e.key) +} + +// IsParameterNodeError returns whether err is of type ParameterNodeError. +func IsParameterNodeError(err error) bool { + switch err.(type) { + case *ParameterNodeError: + return true + default: + return false + } +} + // SignatureError describes an invalid payload signature passed to Hook. type SignatureError struct { Signature string @@ -274,9 +296,9 @@ func ReplaceParameter(s string, params interface{}, value interface{}) bool { } // GetParameter extracts interface{} value based on the passed string -func GetParameter(s string, params interface{}) (interface{}, bool) { +func GetParameter(s string, params interface{}) (interface{}, error) { if params == nil { - return nil, false + return nil, errors.New("no parameters") } paramsValue := reflect.ValueOf(params) @@ -290,7 +312,7 @@ func GetParameter(s string, params interface{}) (interface{}, bool) { index, err := strconv.ParseUint(p[0], 10, 64) if err != nil || paramsValueSliceLength <= int(index) { - return nil, false + return nil, &ParameterNodeError{s} } return GetParameter(p[1], params.([]interface{})[index]) @@ -299,18 +321,18 @@ func GetParameter(s string, params interface{}) (interface{}, bool) { index, err := strconv.ParseUint(s, 10, 64) if err != nil || paramsValueSliceLength <= int(index) { - return nil, false + return nil, &ParameterNodeError{s} } - return params.([]interface{})[index], true + return params.([]interface{})[index], nil } - return nil, false + return nil, &ParameterNodeError{s} case reflect.Map: // Check for raw key if v, ok := params.(map[string]interface{})[s]; ok { - return v, true + return v, nil } // Checked for dotted references @@ -320,19 +342,21 @@ func GetParameter(s string, params interface{}) (interface{}, bool) { return GetParameter(p[1], pValue) } - return pValue, true + return pValue, nil } } - return nil, false + return nil, &ParameterNodeError{s} } // ExtractParameterAsString extracts value from interface{} as string based on the passed string -func ExtractParameterAsString(s string, params interface{}) (string, bool) { - if pValue, ok := GetParameter(s, params); ok { - return fmt.Sprintf("%v", pValue), true +func ExtractParameterAsString(s string, params interface{}) (string, error) { + pValue, err := GetParameter(s, params) + if err != nil { + return "", err } - return "", false + + return fmt.Sprintf("%v", pValue), nil } // Argument type specifies the parameter key name and the source it should @@ -346,7 +370,7 @@ type Argument struct { // Get Argument method returns the value for the Argument's key name // based on the Argument's source -func (ha *Argument) Get(headers, query, payload *map[string]interface{}) (string, bool) { +func (ha *Argument) Get(headers, query, payload *map[string]interface{}) (string, error) { var source *map[string]interface{} key := ha.Name @@ -359,35 +383,35 @@ func (ha *Argument) Get(headers, query, payload *map[string]interface{}) (string case SourcePayload: source = payload case SourceString: - return ha.Name, true + return ha.Name, nil case SourceEntirePayload: r, err := json.Marshal(payload) if err != nil { - return "", false + return "", err } - return string(r), true + return string(r), nil case SourceEntireHeaders: r, err := json.Marshal(headers) if err != nil { - return "", false + return "", err } - return string(r), true + return string(r), nil case SourceEntireQuery: r, err := json.Marshal(query) if err != nil { - return "", false + return "", err } - return string(r), true + return string(r), nil } if source != nil { return ExtractParameterAsString(key, *source) } - return "", false + return "", errors.New("no source for value retrieval") } // Header is a structure containing header name and it's value @@ -469,7 +493,10 @@ func (h *Hook) ParseJSONParameters(headers, query, payload *map[string]interface errors := make([]error, 0) for i := range h.JSONStringParameters { - if arg, ok := h.JSONStringParameters[i].Get(headers, query, payload); ok { + arg, err := h.JSONStringParameters[i].Get(headers, query, payload) + if err != nil { + errors = append(errors, &ArgumentError{h.JSONStringParameters[i]}) + } else { var newArg map[string]interface{} decoder := json.NewDecoder(strings.NewReader(string(arg))) @@ -503,8 +530,6 @@ func (h *Hook) ParseJSONParameters(headers, query, payload *map[string]interface } else { errors = append(errors, &SourceError{h.JSONStringParameters[i]}) } - } else { - errors = append(errors, &ArgumentError{h.JSONStringParameters[i]}) } } @@ -524,12 +549,14 @@ func (h *Hook) ExtractCommandArguments(headers, query, payload *map[string]inter args = append(args, h.ExecuteCommand) for i := range h.PassArgumentsToCommand { - if arg, ok := h.PassArgumentsToCommand[i].Get(headers, query, payload); ok { - args = append(args, arg) - } else { + arg, err := h.PassArgumentsToCommand[i].Get(headers, query, payload) + if err != nil { args = append(args, "") errors = append(errors, &ArgumentError{h.PassArgumentsToCommand[i]}) + continue } + + args = append(args, arg) } if len(errors) > 0 { @@ -546,16 +573,18 @@ func (h *Hook) ExtractCommandArgumentsForEnv(headers, query, payload *map[string args := make([]string, 0) errors := make([]error, 0) for i := range h.PassEnvironmentToCommand { - if arg, ok := h.PassEnvironmentToCommand[i].Get(headers, query, payload); ok { - if h.PassEnvironmentToCommand[i].EnvName != "" { - // first try to use the EnvName if specified - args = append(args, h.PassEnvironmentToCommand[i].EnvName+"="+arg) - } else { - // then fallback on the name - args = append(args, EnvNamespace+h.PassEnvironmentToCommand[i].Name+"="+arg) - } - } else { + arg, err := h.PassEnvironmentToCommand[i].Get(headers, query, payload) + if err != nil { errors = append(errors, &ArgumentError{h.PassEnvironmentToCommand[i]}) + continue + } + + if h.PassEnvironmentToCommand[i].EnvName != "" { + // first try to use the EnvName if specified + args = append(args, h.PassEnvironmentToCommand[i].EnvName+"="+arg) + } else { + // then fallback on the name + args = append(args, EnvNamespace+h.PassEnvironmentToCommand[i].Name+"="+arg) } } @@ -580,30 +609,30 @@ func (h *Hook) ExtractCommandArgumentsForFile(headers, query, payload *map[strin args := make([]FileParameter, 0) errors := make([]error, 0) for i := range h.PassFileToCommand { - if arg, ok := h.PassFileToCommand[i].Get(headers, query, payload); ok { - - if h.PassFileToCommand[i].EnvName == "" { - // if no environment-variable name is set, fall-back on the name - log.Printf("no ENVVAR name specified, falling back to [%s]", EnvNamespace+strings.ToUpper(h.PassFileToCommand[i].Name)) - h.PassFileToCommand[i].EnvName = EnvNamespace + strings.ToUpper(h.PassFileToCommand[i].Name) - } - - var fileContent []byte - if h.PassFileToCommand[i].Base64Decode { - dec, err := base64.StdEncoding.DecodeString(arg) - if err != nil { - log.Printf("error decoding string [%s]", err) - } - fileContent = []byte(dec) - } else { - fileContent = []byte(arg) - } - - args = append(args, FileParameter{EnvName: h.PassFileToCommand[i].EnvName, Data: fileContent}) - - } else { + arg, err := h.PassFileToCommand[i].Get(headers, query, payload) + if err != nil { errors = append(errors, &ArgumentError{h.PassFileToCommand[i]}) + continue } + + if h.PassFileToCommand[i].EnvName == "" { + // if no environment-variable name is set, fall-back on the name + log.Printf("no ENVVAR name specified, falling back to [%s]", EnvNamespace+strings.ToUpper(h.PassFileToCommand[i].Name)) + h.PassFileToCommand[i].EnvName = EnvNamespace + strings.ToUpper(h.PassFileToCommand[i].Name) + } + + var fileContent []byte + if h.PassFileToCommand[i].Base64Decode { + dec, err := base64.StdEncoding.DecodeString(arg) + if err != nil { + log.Printf("error decoding string [%s]", err) + } + fileContent = []byte(dec) + } else { + fileContent = []byte(arg) + } + + args = append(args, FileParameter{EnvName: h.PassFileToCommand[i].EnvName, Data: fileContent}) } if len(errors) > 0 { @@ -785,7 +814,8 @@ func (r MatchRule) Evaluate(headers, query, payload *map[string]interface{}, bod return CheckScalrSignature(*headers, *body, r.Secret, true) } - if arg, ok := r.Parameter.Get(headers, query, payload); ok { + arg, err := r.Parameter.Get(headers, query, payload) + if err == nil { switch r.Type { case MatchValue: return compare(arg, r.Value), nil @@ -802,7 +832,7 @@ func (r MatchRule) Evaluate(headers, query, payload *map[string]interface{}, bod return err == nil, err } } - return false, nil + return false, err } // compare is a helper function for constant time string comparisons. diff --git a/internal/hook/hook_test.go b/internal/hook/hook_test.go index dfb4874..78319a4 100644 --- a/internal/hook/hook_test.go +++ b/internal/hook/hook_test.go @@ -28,9 +28,9 @@ func TestGetParameter(t *testing.T) { {"z.b", map[string]interface{}{"a": map[string]interface{}{"z": 2}}, nil, false}, {"a.2", map[string]interface{}{"a": []interface{}{"a", "b"}}, nil, false}, } { - res, ok := GetParameter(test.key, test.val) - if ok != test.ok { - t.Errorf("unexpected result given {%q, %q}: %t\n", test.key, test.val, ok) + res, err := GetParameter(test.key, test.val) + if (err == nil) != test.ok { + t.Errorf("unexpected result given {%q, %q}: %s\n", test.key, test.val, err) } if !reflect.DeepEqual(res, test.expect) { @@ -225,9 +225,9 @@ var extractParameterTests = []struct { func TestExtractParameter(t *testing.T) { for _, tt := range extractParameterTests { - value, ok := ExtractParameterAsString(tt.s, tt.params) - if ok != tt.ok || value != tt.value { - t.Errorf("failed to extract parameter %q:\nexpected {value:%#v, ok:%#v},\ngot {value:%#v, ok:%#v}", tt.s, tt.value, tt.ok, value, ok) + value, err := ExtractParameterAsString(tt.s, tt.params) + if (err == nil) != tt.ok || value != tt.value { + t.Errorf("failed to extract parameter %q:\nexpected {value:%#v, ok:%#v},\ngot {value:%#v, err:%s}", tt.s, tt.value, tt.ok, value, err) } } } @@ -252,9 +252,9 @@ var argumentGetTests = []struct { func TestArgumentGet(t *testing.T) { for _, tt := range argumentGetTests { a := Argument{tt.source, tt.name, "", false} - value, ok := a.Get(tt.headers, tt.query, tt.payload) - if ok != tt.ok || value != tt.value { - t.Errorf("failed to get {%q, %q}:\nexpected {value:%#v, ok:%#v},\ngot {value:%#v, ok:%#v}", tt.source, tt.name, tt.value, tt.ok, value, ok) + value, err := a.Get(tt.headers, tt.query, tt.payload) + if (err == nil) != tt.ok || value != tt.value { + t.Errorf("failed to get {%q, %q}:\nexpected {value:%#v, ok:%#v},\ngot {value:%#v, err:%s}", tt.source, tt.name, tt.value, tt.ok, value, err) } } } @@ -452,7 +452,7 @@ var matchRuleTests = []struct { // failures {"value", "", "", "X", "", Argument{"header", "a", "", false}, &map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", false, false}, {"regex", "^X", "", "", "", Argument{"header", "a", "", false}, &map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", false, false}, - {"value", "", "2", "X", "", Argument{"header", "a", "", false}, &map[string]interface{}{"Y": "z"}, nil, nil, []byte{}, "", false, false}, // reference invalid header + {"value", "", "2", "X", "", Argument{"header", "a", "", false}, &map[string]interface{}{"Y": "z"}, nil, nil, []byte{}, "", false, true}, // reference invalid header // errors {"regex", "*", "", "", "", Argument{"header", "a", "", false}, &map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", false, true}, // invalid regex {"payload-hash-sha1", "", "secret", "", "", Argument{"header", "a", "", false}, &map[string]interface{}{"A": ""}, nil, nil, []byte{}, "", false, true}, // invalid hmac @@ -477,7 +477,7 @@ func TestMatchRule(t *testing.T) { r := MatchRule{tt.typ, tt.regex, tt.secret, tt.value, tt.param, tt.ipRange} ok, err := r.Evaluate(tt.headers, tt.query, tt.payload, &tt.body, tt.remoteAddr) if ok != tt.ok || (err != nil) != tt.err { - t.Errorf("%d failed to match %#v:\nexpected ok: %#v, err: %v\ngot ok: %#v, err: %v", i, r, tt.ok, tt.err, ok, (err != nil)) + t.Errorf("%d failed to match %#v:\nexpected ok: %#v, err: %v\ngot ok: %#v, err: %v", i, r, tt.ok, tt.err, ok, err) } } } @@ -540,7 +540,7 @@ var andRuleTests = []struct { "invalid rule", AndRule{{Match: &MatchRule{"value", "", "", "X", Argument{"header", "a", "", false}, ""}}}, &map[string]interface{}{"Y": "z"}, nil, nil, nil, - false, false, + false, true, }, } @@ -595,7 +595,7 @@ var orRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, }, &map[string]interface{}{"Y": "Z"}, nil, nil, []byte{}, - false, false, + false, true, }, } diff --git a/webhook.go b/webhook.go index a502a34..ee9cd20 100644 --- a/webhook.go +++ b/webhook.go @@ -473,11 +473,15 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { } else { ok, err = matchedHook.TriggerRule.Evaluate(&headers, &query, &payload, &body, r.RemoteAddr) if err != nil { - msg := fmt.Sprintf("[%s] error evaluating hook: %s", rid, err) - log.Println(msg) - w.WriteHeader(http.StatusInternalServerError) - fmt.Fprint(w, "Error occurred while evaluating hook rules.") - return + if !hook.IsParameterNodeError(err) { + msg := fmt.Sprintf("[%s] error evaluating hook: %s", rid, err) + log.Println(msg) + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprint(w, "Error occurred while evaluating hook rules.") + return + } + + log.Printf("[%s] %v", rid, err) } }