fix(security): followup hardening — ILIKE ESCAPE, allowlist logging, shell deny, tests (#251)

- Add explicit ESCAPE '\' clause to all ILIKE queries (knowledge_graph,
  custom_tools, channel_instances, channel_contacts) for correct wildcard
  escaping regardless of PostgreSQL standard_conforming_strings setting
- Log warning when filterAllowedKeys drops unknown fields for debuggability
- Widen base64 decode shell deny pattern to catch -di, -dw0 variants
- Add unit tests for filterAllowedKeys, pqStringArray, scanStringArray,
  pqStringArray↔scanStringArray roundtrip, limitedBuffer, base64 deny

internal/http/validate.go

@@ -1,6 +1,9 @@
 package http
 
-import "regexp"
+import (
+	"log/slog"
+	"regexp"
+)
 
 var slugRe = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]*[a-z0-9])?$`)
 
@@ -17,6 +20,8 @@ func filterAllowedKeys(updates map[string]any, allowed map[string]bool) map[stri
 	for k, v := range updates {
 		if allowed[k] {
 			filtered[k] = v
+		} else {
+			slog.Warn("security.filtered_unknown_field", "field", k)
 		}
 	}
 	return filtered

internal/http/validate_test.go

@@ -0,0 +1,51 @@
+package http
+
+import (
+	"testing"
+)
+
+func TestFilterAllowedKeys(t *testing.T) {
+	allowed := map[string]bool{"name": true, "status": true}
+
+	tests := []struct {
+		name     string
+		updates  map[string]any
+		wantKeys []string
+	}{
+		{
+			name:     "keeps allowed keys",
+			updates:  map[string]any{"name": "foo", "status": "active"},
+			wantKeys: []string{"name", "status"},
+		},
+		{
+			name:     "filters disallowed keys",
+			updates:  map[string]any{"name": "foo", "id": "inject", "owner_id": "hack"},
+			wantKeys: []string{"name"},
+		},
+		{
+			name:     "empty input returns empty",
+			updates:  map[string]any{},
+			wantKeys: nil,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := filterAllowedKeys(tt.updates, allowed)
+			if tt.wantKeys == nil {
+				if len(result) != 0 {
+					t.Errorf("expected empty map, got %v", result)
+				}
+				return
+			}
+			if len(result) != len(tt.wantKeys) {
+				t.Errorf("expected %d keys, got %d: %v", len(tt.wantKeys), len(result), result)
+			}
+			for _, k := range tt.wantKeys {
+				if _, ok := result[k]; !ok {
+					t.Errorf("expected key %q in result", k)
+				}
+			}
+		})
+	}
+}

internal/store/pg/channel_contacts.go

@@ -56,7 +56,7 @@ func contactWhereClause(opts store.ContactListOpts) (string, []any, int) {
 		escaped := strings.NewReplacer("%", "\\%", "_", "\\_").Replace(opts.Search)
 		pattern := "%" + escaped + "%"
 		conditions = append(conditions, fmt.Sprintf(
-			"(display_name ILIKE $%d OR username ILIKE $%d OR sender_id ILIKE $%d)",
+			"(display_name ILIKE $%d ESCAPE '\\' OR username ILIKE $%d ESCAPE '\\' OR sender_id ILIKE $%d ESCAPE '\\')",
 			argIdx, argIdx, argIdx,
 		))
 		args = append(args, pattern)

internal/store/pg/channel_instances.go

@@ -250,7 +250,7 @@ func buildChannelInstanceWhere(opts store.ChannelInstanceListOpts) (string, []an
 	argIdx := 1
 
 	if opts.Search != "" {
-		conditions = append(conditions, fmt.Sprintf("(name ILIKE $%d OR display_name ILIKE $%d OR channel_type ILIKE $%d)", argIdx, argIdx, argIdx))
+		conditions = append(conditions, fmt.Sprintf("(name ILIKE $%d ESCAPE '\\' OR display_name ILIKE $%d ESCAPE '\\' OR channel_type ILIKE $%d ESCAPE '\\')", argIdx, argIdx, argIdx))
 		escaped := strings.NewReplacer("%", "\\%", "_", "\\_").Replace(opts.Search)
 		args = append(args, "%"+escaped+"%")
 	}

internal/store/pg/custom_tools.go

@@ -204,7 +204,7 @@ func buildCustomToolWhere(opts store.CustomToolListOpts) (string, []any) {
 		argIdx++
 	}
 	if opts.Search != "" {
-		conditions = append(conditions, fmt.Sprintf("(name ILIKE $%d OR description ILIKE $%d)", argIdx, argIdx))
+		conditions = append(conditions, fmt.Sprintf("(name ILIKE $%d ESCAPE '\\' OR description ILIKE $%d ESCAPE '\\')", argIdx, argIdx))
 		escaped := strings.NewReplacer("%", "\\%", "_", "\\_").Replace(opts.Search)
 		args = append(args, "%"+escaped+"%")
 	}

internal/store/pg/helpers_test.go

@@ -0,0 +1,102 @@
+package pg
+
+import (
+	"testing"
+)
+
+func TestPqStringArray(t *testing.T) {
+	tests := []struct {
+		name string
+		arr  []string
+		want string
+	}{
+		{"nil returns nil", nil, ""},
+		{"simple elements", []string{"a", "b"}, `{"a","b"}`},
+		{"element with comma", []string{"a,b", "c"}, `{"a,b","c"}`},
+		{"element with quote", []string{`say "hi"`}, `{"say \"hi\""}`},
+		{"element with backslash", []string{`a\b`}, `{"a\\b"}`},
+		{"empty slice", []string{}, `{}`},
+		{"empty string element", []string{""}, `{""}`},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := pqStringArray(tt.arr)
+			if tt.arr == nil {
+				if got != nil {
+					t.Errorf("expected nil, got %v", got)
+				}
+				return
+			}
+			s, ok := got.(string)
+			if !ok {
+				t.Fatalf("expected string, got %T", got)
+			}
+			if s != tt.want {
+				t.Errorf("got %q, want %q", s, tt.want)
+			}
+		})
+	}
+}
+
+func TestScanStringArray(t *testing.T) {
+	tests := []struct {
+		name string
+		data []byte
+		want []string
+	}{
+		{"nil data", nil, nil},
+		{"empty braces", []byte("{}"), nil},
+		{"simple unquoted", []byte("{a,b,c}"), []string{"a", "b", "c"}},
+		{"quoted with comma", []byte(`{"a,b","c"}`), []string{"a,b", "c"}},
+		{"quoted with escaped quote", []byte(`{"say \"hi\"","ok"}`), []string{`say "hi"`, "ok"}},
+		{"quoted with backslash", []byte(`{"a\\b"}`), []string{`a\b`}},
+		{"mixed quoted unquoted", []byte(`{foo,"bar,baz",qux}`), []string{"foo", "bar,baz", "qux"}},
+		{"single element", []byte("{hello}"), []string{"hello"}},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var got []string
+			scanStringArray(tt.data, &got)
+			if tt.want == nil {
+				if got != nil {
+					t.Errorf("expected nil, got %v", got)
+				}
+				return
+			}
+			if len(got) != len(tt.want) {
+				t.Fatalf("got %v (len %d), want %v (len %d)", got, len(got), tt.want, len(tt.want))
+			}
+			for i := range tt.want {
+				if got[i] != tt.want[i] {
+					t.Errorf("index %d: got %q, want %q", i, got[i], tt.want[i])
+				}
+			}
+		})
+	}
+}
+
+func TestPqStringArrayRoundtrip(t *testing.T) {
+	// Verify encode→decode roundtrip for tricky values.
+	inputs := [][]string{
+		{"simple"},
+		{"a,b", "c"},
+		{`say "hello"`, `back\slash`},
+		{"", "empty"},
+	}
+
+	for _, arr := range inputs {
+		encoded := pqStringArray(arr).(string)
+		var decoded []string
+		scanStringArray([]byte(encoded), &decoded)
+		if len(decoded) != len(arr) {
+			t.Fatalf("roundtrip len mismatch: input %v, encoded %q, decoded %v", arr, encoded, decoded)
+		}
+		for i := range arr {
+			if decoded[i] != arr[i] {
+				t.Errorf("roundtrip mismatch at %d: input %q, got %q (encoded: %q)", i, arr[i], decoded[i], encoded)
+			}
+		}
+	}
+}

internal/store/pg/knowledge_graph.go

@@ -128,7 +128,7 @@ func (s *PGKnowledgeGraphStore) SearchEntities(ctx context.Context, agentID, use
 		SELECT id, agent_id, user_id, external_id, name, entity_type, description,
 		       properties, source_id, confidence, created_at, updated_at
 		FROM kg_entities
-		WHERE %s AND (name ILIKE $%d OR description ILIKE $%d)
+		WHERE %s AND (name ILIKE $%d ESCAPE '\' OR description ILIKE $%d ESCAPE '\')
 		ORDER BY confidence DESC, updated_at DESC LIMIT $%d`, where, idx, idx, idx+1)
 
 	rows, err := s.db.QueryContext(ctx, q, args...)

internal/tools/shell_deny_groups.go

@@ -72,7 +72,7 @@ var DenyGroupRegistry = map[string]*DenyGroup{
 		Default:     true,
 		Patterns: []*regexp.Regexp{
 			regexp.MustCompile(`\beval\s*\$`),
-			regexp.MustCompile(`\bbase64\s+(-d|--decode)\b.*\|\s*(ba)?sh\b`),
+			regexp.MustCompile(`\bbase64\s+(-d\w*|--decode)\b.*\|\s*(ba)?sh\b`),
 		},
 	},
 	"privilege_escalation": {

internal/tools/shell_deny_test.go

@@ -0,0 +1,104 @@
+package tools
+
+import (
+	"testing"
+)
+
+func TestBase64DecodeShellDeny(t *testing.T) {
+	patterns := DenyGroupRegistry["code_injection"].Patterns
+
+	denied := []string{
+		"base64 -d payload.txt | sh",
+		"base64 --decode payload.txt | sh",
+		"base64 -di payload.txt | sh",
+		"base64 -dw0 payload.txt | bash",
+		"base64 --decode something | bash",
+	}
+
+	allowed := []string{
+		"base64 -w0 file.txt",       // encode, no pipe to shell
+		"base64 -d file.txt",        // decode without pipe to shell
+		"echo hello | base64",       // encode
+		"base64 --decode file.txt",  // decode without pipe to shell
+	}
+
+	for _, cmd := range denied {
+		matched := false
+		for _, p := range patterns {
+			if p.MatchString(cmd) {
+				matched = true
+				break
+			}
+		}
+		if !matched {
+			t.Errorf("expected deny for %q", cmd)
+		}
+	}
+
+	for _, cmd := range allowed {
+		matched := false
+		for _, p := range patterns {
+			if p.MatchString(cmd) {
+				matched = true
+				break
+			}
+		}
+		if matched {
+			t.Errorf("unexpected deny for %q", cmd)
+		}
+	}
+}
+
+func TestLimitedBuffer(t *testing.T) {
+	t.Run("under limit", func(t *testing.T) {
+		lb := &limitedBuffer{max: 100}
+		lb.Write([]byte("hello"))
+		if lb.String() != "hello" {
+			t.Errorf("got %q", lb.String())
+		}
+		if lb.truncated {
+			t.Error("should not be truncated")
+		}
+	})
+
+	t.Run("at limit", func(t *testing.T) {
+		lb := &limitedBuffer{max: 5}
+		n, err := lb.Write([]byte("hello"))
+		if err != nil || n != 5 {
+			t.Errorf("Write: n=%d err=%v", n, err)
+		}
+		if lb.truncated {
+			t.Error("exactly at limit should not be truncated")
+		}
+	})
+
+	t.Run("over limit truncates", func(t *testing.T) {
+		lb := &limitedBuffer{max: 5}
+		n, err := lb.Write([]byte("hello world"))
+		if err != nil {
+			t.Fatal(err)
+		}
+		if n != 11 {
+			t.Errorf("Write should report full len, got %d", n)
+		}
+		if !lb.truncated {
+			t.Error("should be truncated")
+		}
+		if lb.Len() != 5 {
+			t.Errorf("buffer len should be 5, got %d", lb.Len())
+		}
+		want := "hello\n[output truncated at 1MB]"
+		if lb.String() != want {
+			t.Errorf("got %q, want %q", lb.String(), want)
+		}
+	})
+
+	t.Run("subsequent writes after truncation", func(t *testing.T) {
+		lb := &limitedBuffer{max: 3}
+		lb.Write([]byte("abc"))
+		lb.Write([]byte("def"))
+		if lb.Len() != 3 {
+			t.Errorf("buffer len should be 3, got %d", lb.Len())
+		}
+	})
+}