Skip to content

Show failed parameter node lookups #384

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.

Sign up for GitHub

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

Merged
adnanh merged 1 commit into from
Feb 14, 2020
Merged

Show failed parameter node lookups #384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
146 changes: 88 additions & 58 deletions internal/hook/hook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 "<nil>"
}
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
Expand Down Expand Up @@ -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)
Expand All @@ -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])
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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)))
Expand Down Expand Up @@ -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]})
}
}

Expand All @@ -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 {
Expand All @@ -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)
}
}

Expand All @@ -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 {
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)
}
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)
var fileContent []byte
if h.PassFileToCommand[i].Base64Decode {
dec, err := base64.StdEncoding.DecodeString(arg)
if err != nil {
log.Printf("error decoding string [%s]", err)
}

args = append(args, FileParameter{EnvName: h.PassFileToCommand[i].EnvName, Data: fileContent})

fileContent = []byte(dec)
} else {
errors = append(errors, &ArgumentError{h.PassFileToCommand[i]})
fileContent = []byte(arg)
}

args = append(args, FileParameter{EnvName: h.PassFileToCommand[i].EnvName, Data: fileContent})
}

if len(errors) > 0 {
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down
26 changes: 13 additions & 13 deletions internal/hook/hook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
}
}
Expand All @@ -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)
}
}
}
Expand Down Expand Up @@ -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
Expand All @@ -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)
}
}
}
Expand Down Expand Up @@ -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,
},
}

Expand Down Expand Up @@ -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,
},
}

Expand Down
14 changes: 9 additions & 5 deletions webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down