fix(sql): stop false-positive forbidden keywords in Call-IDs (11.0.305)

docs/MCP.md

@@ -93,7 +93,7 @@ The generated SQL is returned in the response (`generated_sql`) and is server-si
 - only `SELECT` / `WITH` allowed
 - semicolons forbidden
 - only `homer_lake.main.hep_proto_1_call` allowed
-- forbidden tokens: `insert`, `update`, `delete`, `drop`, `alter`, `truncate`, `copy`, `attach`, `detach`, `call`, `create`, `grant`, `revoke`
+- forbidden tokens (as SQL identifiers, not inside string literals): `insert`, `update`, `delete`, `drop`, `alter`, `truncate`, `copy`, `attach`, `detach`, `call`, `create`, `grant`, `revoke`, plus the shared read-only keyword set (`load`, `pragma`, `merge`, …). Call-IDs that embed words like `call` are allowed.
 
 ### `homer_query`
 

docs/SECURITY.md

@@ -70,7 +70,7 @@ See [AUTH_LDAP_AND_OAUTH.md](./AUTH_LDAP_AND_OAUTH.md#internal-duckdb-authentica
 
 - Allowed statement starts: `SELECT`, `WITH`, `SHOW`, `DESCRIBE`, `EXPLAIN`, `PRAGMA`
 - No semicolons (multi-statement blocked)
-- Blocked DML/DDL keywords and dangerous functions
+- Blocked DML/DDL keywords and dangerous functions (token-aware: keywords inside string literals / Call-IDs are ignored; real `CALL` / `DELETE` identifiers remain blocked)
 
 Invalid SQL returns **400** with `SQL validation failed`. Grafana-style read-only panels using single `SELECT` statements continue to work.
 

src/coordinator/sqlvalidator/validator.go

@@ -598,6 +598,56 @@ func ContainsUnsafeComment(sql string) bool {
 	return false
 }
 
+// ---- Forbidden identifiers (read-only paths) -------------------------------
+
+// ForbiddenReadOnlyKeywords are statement keywords that must not appear as
+// identifiers outside string literals on read-only SELECT paths (node / MCP).
+// CALL remains blocked as a real DuckDB statement; matching is token-aware so
+// Call-IDs / session_ids that embed words like "call" are not rejected.
+var ForbiddenReadOnlyKeywords = map[string]bool{
+	"ATTACH":   true,
+	"DETACH":   true,
+	"COPY":     true,
+	"PRAGMA":   true,
+	"INSTALL":  true,
+	"LOAD":     true,
+	"CALL":     true,
+	"CREATE":   true,
+	"ALTER":    true,
+	"DROP":     true,
+	"TRUNCATE": true,
+	"INSERT":   true,
+	"UPDATE":   true,
+	"DELETE":   true,
+	"MERGE":    true,
+	"REPLACE":  true,
+	"GRANT":    true,
+	"REVOKE":   true,
+	"VACUUM":   true,
+	"ANALYZE":  true,
+	"EXPORT":   true,
+	"IMPORT":   true,
+}
+
+// ContainsForbiddenIdentifier reports whether sql contains any of the given
+// keywords as identifier tokens outside string literals. A naive whole-string
+// regex would false-positive on Call-IDs / session_ids that embed words like
+// "call" or "delete".
+func ContainsForbiddenIdentifier(sql string, forbidden map[string]bool) bool {
+	if len(forbidden) == 0 {
+		return false
+	}
+	for _, tok := range tokenize(sql) {
+		if tok.kind != tkIdent {
+			continue
+		}
+		if forbidden[tok.upper] {
+			return true
+		}
+	}
+	return false
+}
+
 // ---- SafeString ------------------------------------------------------------
 
 const maxSafeStringLen = 1000

src/coordinator/sqlvalidator/validator_test.go

@@ -824,6 +824,58 @@ func TestContainsUnsafeComment(t *testing.T) {
 	}
 }
 
+func TestContainsForbiddenIdentifier(t *testing.T) {
+	tests := []struct {
+		name string
+		sql  string
+		want bool
+	}{
+		{
+			name: "call inside session_id literal",
+			sql:  "SELECT * FROM t WHERE session_id = 'foo-call-bar'",
+			want: false,
+		},
+		{
+			name: "hep_proto_1_call table name",
+			sql:  "SELECT * FROM hep_proto_1_call",
+			want: false,
+		},
+		{
+			name: "real CALL statement",
+			sql:  "SELECT * FROM t CALL some_proc()",
+			want: true,
+		},
+		{
+			name: "delete inside literal allowed",
+			sql:  "SELECT * FROM t WHERE x = 'delete'",
+			want: false,
+		},
+		{
+			name: "DELETE as identifier blocked",
+			sql:  "SELECT * FROM t WHERE DELETE",
+			want: true,
+		},
+		{
+			name: "empty forbidden map",
+			sql:  "SELECT CALL FROM t",
+			want: false, // tested with nil map below
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			forbidden := ForbiddenReadOnlyKeywords
+			if tt.name == "empty forbidden map" {
+				forbidden = nil
+			}
+			got := ContainsForbiddenIdentifier(tt.sql, forbidden)
+			if got != tt.want {
+				t.Errorf("ContainsForbiddenIdentifier(%q) = %v, want %v", tt.sql, got, tt.want)
+			}
+		})
+	}
+}
+
 func TestHasLimitToken(t *testing.T) {
 	tests := []struct {
 		name string

src/mcp/mcp.go

@@ -17,6 +17,7 @@ import (
 	"github.com/mark3labs/mcp-go/mcp"
 	"github.com/mark3labs/mcp-go/server"
 	"github.com/sipcapture/homer-core/src/config"
+	"github.com/sipcapture/homer-core/src/coordinator/sqlvalidator"
 	logger "github.com/sipcapture/homer-core/src/utils/logging"
 )
 
@@ -103,10 +104,6 @@ var (
 		"INVITE", "BYE", "REGISTER", "OPTIONS", "ACK", "CANCEL", "PRACK",
 		"UPDATE", "INFO", "REFER", "SUBSCRIBE", "NOTIFY", "PUBLISH", "MESSAGE",
 	}
-	bannedSQLTokens = []string{
-		"insert", "update", "delete", "drop", "alter", "truncate",
-		"copy", "attach", "detach", "call", "create", "grant", "revoke",
-	}
 )
 
 func New(cfg *config.MCPConfig) (*Module, error) {
@@ -685,11 +682,8 @@ func validateSQL(sql string) error {
 		return fmt.Errorf("only homer_lake.main.hep_proto_1_call is allowed")
 	}
 
-	for _, token := range bannedSQLTokens {
-		re := regexp.MustCompile(`(?i)\b` + token + `\b`)
-		if re.MatchString(trimmed) {
-			return fmt.Errorf("forbidden SQL token: %s", token)
-		}
+	if sqlvalidator.ContainsForbiddenIdentifier(trimmed, sqlvalidator.ForbiddenReadOnlyKeywords) {
+		return fmt.Errorf("forbidden SQL token")
 	}
 	return nil
 }

src/mcp/mcp_test.go

@@ -59,6 +59,13 @@ func TestValidateSQLAllowsCallTableName(t *testing.T) {
 	}
 }
 
+func TestValidateSQLAllowsForbiddenWordsInLiterals(t *testing.T) {
+	sql := "SELECT * FROM homer_lake.main.hep_proto_1_call WHERE session_id = 'foo-call-bar' OR note = 'drop table'"
+	if err := validateSQL(sql); err != nil {
+		t.Fatalf("expected keywords inside string literals to be allowed, got: %v", err)
+	}
+}
+
 func TestValidateSQLRejectsSemicolon(t *testing.T) {
 	sql := "SELECT * FROM homer_lake.main.hep_proto_1_call WHERE method = 'INVITE';"
 	if err := validateSQL(sql); err == nil {
@@ -67,9 +74,18 @@ func TestValidateSQLRejectsSemicolon(t *testing.T) {
 }
 
 func TestValidateSQLRejectsDropToken(t *testing.T) {
-	sql := "SELECT * FROM homer_lake.main.hep_proto_1_call WHERE note = 'drop table'"
+	// Bare DROP identifier must still be rejected; words inside string
+	// literals are allowed (see TestValidateSQLAllowsForbiddenWordsInLiterals).
+	sql := "SELECT * FROM homer_lake.main.hep_proto_1_call WHERE DROP"
+	if err := validateSQL(sql); err == nil {
+		t.Fatalf("expected DROP identifier SQL to be rejected")
+	}
+}
+
+func TestValidateSQLRejectsCallStatement(t *testing.T) {
+	sql := "SELECT * FROM homer_lake.main.hep_proto_1_call WHERE 1=1 CALL some_proc()"
 	if err := validateSQL(sql); err == nil {
-		t.Fatalf("expected DROP token SQL to be rejected")
+		t.Fatalf("expected CALL statement SQL to be rejected")
 	}
 }
 

src/node/node.go

@@ -511,8 +511,6 @@ func mergeSelectResults(a, b []map[string]interface{}, colsA, colsB []string, li
 // UNIONs and the overall statement shape are built dynamically. As defence in
 // depth we still reject anything that is not a single read-only SELECT before
 // it reaches the driver, so a malformed/stacked statement can never run here.
-var dangerousSQLPattern = regexp.MustCompile(`(?i)\b(attach|detach|copy|pragma|install|load|call|create|alter|drop|truncate|insert|update|delete|merge|replace|grant|revoke|vacuum|analyze|export|import)\b`)
-
 func validateUserSQL(query string) error {
 	trimmed := strings.TrimSpace(query)
 	if trimmed == "" {
@@ -534,7 +532,10 @@ func validateUserSQL(query string) error {
 		return fmt.Errorf("SQL contains forbidden comment or statement separator")
 	}
 
-	if dangerousSQLPattern.MatchString(trimmed) {
+	// Token-aware keyword check: ignore string literals so Call-IDs that embed
+	// words like "call" / "delete" do not false-positive (same class of bug as
+	// "--" inside session_id before ContainsUnsafeComment).
+	if sqlvalidator.ContainsForbiddenIdentifier(trimmed, sqlvalidator.ForbiddenReadOnlyKeywords) {
 		return fmt.Errorf("SQL contains forbidden keyword")
 	}
 

src/node/rewrite_query_test.go

@@ -552,6 +552,63 @@ func TestValidateUserSQL_CommentMarkers(t *testing.T) {
 	}
 }
 
+func TestValidateUserSQL_KeywordsInLiterals(t *testing.T) {
+	tests := []struct {
+		name    string
+		query   string
+		wantErr string // empty = pass
+	}{
+		{
+			name:  "session_id with call token passes",
+			query: "SELECT * FROM homer_lake.main.hep_proto_1_call WHERE session_id = 'foo-call-bar'",
+		},
+		{
+			name:  "session_id exactly call passes",
+			query: "SELECT * FROM t WHERE session_id = 'call'",
+		},
+		{
+			name:  "delete/load/update inside literals pass",
+			query: "SELECT * FROM t WHERE session_id = 'delete-me' OR cid = 'load-copy-update'",
+		},
+		{
+			name:  "hep_proto_1_call table name is not CALL keyword",
+			query: "SELECT * FROM homer_lake.main.hep_proto_1_call WHERE method = 'INVITE'",
+		},
+		{
+			name:    "real CALL statement rejected",
+			query:   "SELECT * FROM t WHERE 1=1 CALL some_proc()",
+			wantErr: "forbidden keyword",
+		},
+		{
+			name:    "real DELETE identifier rejected",
+			query:   "SELECT * FROM t WHERE DELETE",
+			wantErr: "forbidden keyword",
+		},
+		{
+			name:    "real INSERT identifier rejected",
+			query:   "SELECT INSERT FROM t",
+			wantErr: "forbidden keyword",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validateUserSQL(tt.query)
+			if tt.wantErr == "" {
+				if err != nil {
+					t.Errorf("expected no error, got: %v", err)
+				}
+			} else {
+				if err == nil {
+					t.Errorf("expected error containing %q, got nil", tt.wantErr)
+				} else if !strings.Contains(err.Error(), tt.wantErr) {
+					t.Errorf("expected error containing %q, got: %v", tt.wantErr, err)
+				}
+			}
+		})
+	}
+}
+
 func TestPrepareFlightSQLDataSQLUsesThresholdDecision(t *testing.T) {
 	n := defaultMemoryUnionNode()
 

src/version.go

@@ -24,7 +24,7 @@ import (
 // Version information for homer-core
 var (
 	// VERSION_APPLICATION is the application version
-	VERSION_APPLICATION = "11.0.304"
+	VERSION_APPLICATION = "11.0.305"
 
 	// BuildDate is the build date
 	BuildDate = ""