fix: address PR #262 review feedback

- parseJSONOrString: only parse JSON objects/arrays (starting with { or [),
  not bare primitives like "order", 123, true that could be JQ expressions
- Add HTTP status code range validation (100-599) for --rule-retry-response-status-codes
- Fix UpsertRetryResponseStatusCodesAsArray acceptance test: status codes are
  now integers (float64 after JSON round-trip), not strings
- Upgrade assert.True to require.True for map type assertions to prevent
  nil-pointer panics on failure
- Update filter flag help text to mention JSON object support alongside JQ
- Remove unused strings import from acceptance test

https://claude.ai/code/session_01No6Vv4niFtYoF56at7uL2P

Author: claude

pkg/cmd/connection_common.go

@@ -77,10 +77,10 @@ func addConnectionRuleFlags(cmd *cobra.Command, f *connectionRuleFlags) {
 	cmd.Flags().IntVar(&f.RuleRetryInterval, "rule-retry-interval", 0, "Interval between retries in milliseconds")
 	cmd.Flags().StringVar(&f.RuleRetryResponseStatusCode, "rule-retry-response-status-codes", "", "Comma-separated HTTP status codes to retry on")
 
-	cmd.Flags().StringVar(&f.RuleFilterBody, "rule-filter-body", "", "JQ expression to filter on request body")
-	cmd.Flags().StringVar(&f.RuleFilterHeaders, "rule-filter-headers", "", "JQ expression to filter on request headers")
-	cmd.Flags().StringVar(&f.RuleFilterQuery, "rule-filter-query", "", "JQ expression to filter on request query parameters")
-	cmd.Flags().StringVar(&f.RuleFilterPath, "rule-filter-path", "", "JQ expression to filter on request path")
+	cmd.Flags().StringVar(&f.RuleFilterBody, "rule-filter-body", "", "JSON object or JQ expression to filter on request body")
+	cmd.Flags().StringVar(&f.RuleFilterHeaders, "rule-filter-headers", "", "JSON object or JQ expression to filter on request headers")
+	cmd.Flags().StringVar(&f.RuleFilterQuery, "rule-filter-query", "", "JSON object or JQ expression to filter on request query parameters")
+	cmd.Flags().StringVar(&f.RuleFilterPath, "rule-filter-path", "", "JSON object or JQ expression to filter on request path")
 
 	cmd.Flags().StringVar(&f.RuleTransformName, "rule-transform-name", "", "Name or ID of the transformation to apply")
 	cmd.Flags().StringVar(&f.RuleTransformCode, "rule-transform-code", "", "Transformation code (if creating inline)")
@@ -203,6 +203,9 @@ func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) {
 				if err != nil {
 					return nil, fmt.Errorf("invalid HTTP status code %q in --rule-retry-response-status-codes: must be an integer", part)
 				}
+				if n < 100 || n > 599 {
+					return nil, fmt.Errorf("invalid HTTP status code %d in --rule-retry-response-status-codes: must be between 100 and 599", n)
+				}
 				intCodes = append(intCodes, n)
 			}
 			rule["response_status_codes"] = intCodes
@@ -213,11 +216,18 @@ func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) {
 	return rules, nil
 }
 
-// parseJSONOrString attempts to parse s as JSON. If successful it returns the
-// parsed value (object, array, number, bool, etc.); otherwise it returns s as
-// a plain string. This lets filter flags accept both JSON objects and JQ
-// expressions transparently.
+// parseJSONOrString attempts to parse s as a JSON object or array. Only values
+// starting with '{' or '[' (after trimming whitespace) are candidates for
+// parsing; bare primitives like "order", 123, or true are returned as-is so
+// that JQ expressions and other plain strings are never misinterpreted.
 func parseJSONOrString(s string) interface{} {
+	trimmed := strings.TrimSpace(s)
+	if len(trimmed) == 0 {
+		return s
+	}
+	if trimmed[0] != '{' && trimmed[0] != '[' {
+		return s
+	}
 	var v interface{}
 	if err := json.Unmarshal([]byte(s), &v); err == nil {
 		return v

pkg/cmd/connection_upsert_test.go

@@ -33,7 +33,7 @@ func TestBuildConnectionRulesFilterHeadersJSON(t *testing.T) {
 		assert.False(t, isString, "headers should be a parsed object, not a string")
 
 		headersMap, isMap := headers.(map[string]interface{})
-		assert.True(t, isMap, "headers should be map[string]interface{}, got %T", headers)
+		require.True(t, isMap, "headers should be map[string]interface{}, got %T", headers)
 		assert.Contains(t, headersMap, "x-shopify-topic")
 	})
 
@@ -51,6 +51,24 @@ func TestBuildConnectionRulesFilterHeadersJSON(t *testing.T) {
 		assert.True(t, isString, "non-JSON value should remain a string")
 	})
 
+	t.Run("bare JSON primitives should remain as strings", func(t *testing.T) {
+		// Values like "order", 123, true are valid JSON primitives but should
+		// NOT be parsed — they are likely JQ expressions or literal strings.
+		for _, input := range []string{`"order"`, `123`, `true`} {
+			flags := connectionRuleFlags{
+				RuleFilterHeaders: input,
+			}
+			rules, err := buildConnectionRules(&flags)
+			require.NoError(t, err)
+			require.Len(t, rules, 1)
+
+			headers := rules[0]["headers"]
+			_, isString := headers.(string)
+			assert.True(t, isString, "input %q should remain a string, got %T", input, headers)
+			assert.Equal(t, input, headers, "value should be unchanged")
+		}
+	})
+
 	t.Run("filter body JSON should also be parsed", func(t *testing.T) {
 		flags := connectionRuleFlags{
 			RuleFilterBody: `{"event_type":"payment"}`,
@@ -64,7 +82,7 @@ func TestBuildConnectionRulesFilterHeadersJSON(t *testing.T) {
 		_, isString := body.(string)
 		assert.False(t, isString, "body should be a parsed object, not a string")
 		_, isMap := body.(map[string]interface{})
-		assert.True(t, isMap, "body should be map[string]interface{}, got %T", body)
+		require.True(t, isMap, "body should be map[string]interface{}, got %T", body)
 	})
 }
 

test/acceptance/connection_upsert_test.go

@@ -1,7 +1,6 @@
 package acceptance
 
 import (
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -373,14 +372,17 @@ func TestConnectionUpsertPartialUpdates(t *testing.T) {
 				require.True(t, ok, "response_status_codes should be array, got: %T (%v)", rule["response_status_codes"], rule["response_status_codes"])
 				assert.Len(t, statusCodes, 4, "Should have 4 status codes")
 
-				codes := make([]string, len(statusCodes))
+				// After JSON round-trip, numbers are float64
+				codes := make([]float64, len(statusCodes))
 				for i, c := range statusCodes {
-					codes[i] = strings.TrimSpace(c.(string))
+					n, ok := c.(float64)
+					require.True(t, ok, "status code should be a number, got %T", c)
+					codes[i] = n
 				}
-				assert.Contains(t, codes, "500")
-				assert.Contains(t, codes, "502")
-				assert.Contains(t, codes, "503")
-				assert.Contains(t, codes, "504")
+				assert.Contains(t, codes, float64(500))
+				assert.Contains(t, codes, float64(502))
+				assert.Contains(t, codes, float64(503))
+				assert.Contains(t, codes, float64(504))
 				break
 			}
 		}
@@ -504,7 +506,7 @@ func TestConnectionUpsertPartialUpdates(t *testing.T) {
 				"--rule-filter-headers should store JSON as an object, not an escaped string; got: %v", headers)
 
 			headersMap, isMap := headers.(map[string]interface{})
-			assert.True(t, isMap,
+			require.True(t, isMap,
 				"headers should be a JSON object (map[string]interface{}), got %T", headers)
 			assert.Contains(t, headersMap, "x-shopify-topic",
 				"headers object should contain the expected key")
@@ -564,7 +566,7 @@ func TestConnectionUpsertPartialUpdates(t *testing.T) {
 				"--rule-filter-body should store JSON as an object, not an escaped string; got: %v", body)
 
 			bodyMap, isMap := body.(map[string]interface{})
-			assert.True(t, isMap, "body should be a JSON object, got %T", body)
+			require.True(t, isMap, "body should be a JSON object, got %T", body)
 			assert.Contains(t, bodyMap, "event_type", "body object should contain the expected key")
 			break
 		}