From b87fc4bcde92d4c5125b36bb472941a1b865ab07 Mon Sep 17 00:00:00 2001 From: Nikhil Sinha <131262146+nikhilsinhaparseable@users.noreply.github.com> Date: Thu, 22 Oct 2026 09:45:06 +0530 Subject: [PATCH 1/2] feat: save promql queries in filters (#1830) (cherry picked from commit 44bd8db7475ad258bb62f4afdfd272499a0132d1) --- src/users/filters.rs | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/users/filters.rs b/src/users/filters.rs index 7ae5a7b0a..216ba54c4 100644 --- a/src/users/filters.rs +++ b/src/users/filters.rs @@ -81,6 +81,7 @@ pub enum FilterType { Filter, SQL, Search, + Promql, } impl FilterType { @@ -89,6 +90,7 @@ impl FilterType { FilterType::Filter => "filter", FilterType::SQL => "sql", FilterType::Search => "search", + FilterType::Promql => "promql", } } } @@ -190,23 +192,26 @@ impl Filters { let query: &str = f.query.filter_query.as_deref().unwrap_or(""); let filter_type = &f.query.filter_type; - // if filter type is SQL, check if the user has access to the dataset based on the query string - // if filter type is search or filter, check if the user has access to the dataset based on the dataset name - if *filter_type == FilterType::SQL { - if (user_auth_for_query(key, query).await).is_ok() { - _filters.push(f.clone()) + match filter_type { + // SQL queries can reference multiple datasets, so authorize the query itself. + FilterType::SQL => { + if (user_auth_for_query(key, query).await).is_ok() { + _filters.push(f.clone()) + } } - } else if *filter_type == FilterType::Search || *filter_type == FilterType::Filter { - let dataset_name = &f.stream_name; - if user_auth_for_datasets( - &permissions, - std::slice::from_ref(dataset_name), - &tenant_id, - ) - .await - .is_ok() - { - _filters.push(f.clone()) + // Other filter types are scoped by the saved filter's dataset. + FilterType::Search | FilterType::Filter | FilterType::Promql => { + let dataset_name = &f.stream_name; + if user_auth_for_datasets( + &permissions, + std::slice::from_ref(dataset_name), + &tenant_id, + ) + .await + .is_ok() + { + _filters.push(f.clone()) + } } } } From b1be0a858ca30d439b52458aa12831bb14f713af Mon Sep 17 00:00:00 2001 From: VibeSWE <338779468+VibeSWE@users.noreply.github.com> Date: Fri, 23 Oct 2026 19:15:21 -0500 Subject: [PATCH 2/2] fix: handle saved PromQL filters across backend tests and frontend Follow-up to the cherry-picked upstream 44bd8db7 (#1830), which adds FilterType::Promql and authorizes PromQL saved filters by dataset name. - Frontend: the /filters list guard only accepted filter|sql|search and rejected the whole list on any unknown entry, so one saved PromQL filter would break Saved log views and the SQL sidebar for everyone who can see it. Accept `promql`, and keep PromQL filters out of the Logs saved-view picker (they are not log views). - Tests: PromQL serde round-trip; list_filters authorizes PromQL by dataset (not by parsing the query as SQL) and hides it without access; PromQL filters require a concrete dataset name (not the `sql` sentinel or path-like names); convert-to-sql rejects PromQL. Co-Authored-By: Claude Opus 5.5 --- frontend/src/features/logs/SavedViews.test.ts | 6 ++ frontend/src/features/logs/SavedViews.tsx | 7 ++- .../features/sql/serverSavedQueries.test.ts | 11 ++++ frontend/src/lib/logsContract.test.ts | 11 ++++ frontend/src/lib/logsContract.ts | 2 +- frontend/src/lib/types.ts | 2 +- src/handlers/http/users/filters.rs | 12 +++- src/users/filters.rs | 57 +++++++++++++++++++ 8 files changed, 103 insertions(+), 5 deletions(-) diff --git a/frontend/src/features/logs/SavedViews.test.ts b/frontend/src/features/logs/SavedViews.test.ts index eeec3a63c..b200b0050 100644 --- a/frontend/src/features/logs/SavedViews.test.ts +++ b/frontend/src/features/logs/SavedViews.test.ts @@ -87,6 +87,12 @@ describe('server saved views', () => { ]); expect(available.map(({ decoded }) => decoded?.dataset)).toEqual(['app', 'app', undefined]); expect(savedLogViewsForDataset([canonical], 'sql')).toEqual([]); + const promql = { + ...classic, + filter_name: 'Request rate', + query: { filter_type: 'promql' as const, filter_query: 'rate(requests_total[5m])' }, + }; + expect(savedLogViewsForDataset([promql], 'app')).toEqual([]); }); it('rejects classic formats and mismatched dataset metadata', () => { const { explorer_state: _state, ...classic } = savedLogViewBody(state, 'Classic'); diff --git a/frontend/src/features/logs/SavedViews.tsx b/frontend/src/features/logs/SavedViews.tsx index 59c4f4899..4b6b87802 100644 --- a/frontend/src/features/logs/SavedViews.tsx +++ b/frontend/src/features/logs/SavedViews.tsx @@ -25,8 +25,11 @@ export function savedLogViewsForDataset(views: SavedLogView[], dataset: string) return decoded.dataset === dataset ? [{ view, decoded }] : []; } catch { // Classic dataset-scoped views remain visible, but canonical views need a - // valid explorer state to establish which dataset they belong to. - return view.stream_name !== 'sql' && view.stream_name === dataset + // valid explorer state to establish which dataset they belong to. Saved + // PromQL queries share the endpoint but are not log views. + return view.query.filter_type !== 'promql' && + view.stream_name !== 'sql' && + view.stream_name === dataset ? [{ view, decoded: undefined }] : []; } diff --git a/frontend/src/features/sql/serverSavedQueries.test.ts b/frontend/src/features/sql/serverSavedQueries.test.ts index 9ecd2ff09..11224ea9e 100644 --- a/frontend/src/features/sql/serverSavedQueries.test.ts +++ b/frontend/src/features/sql/serverSavedQueries.test.ts @@ -92,6 +92,17 @@ const classicSearch: SavedLogView = { }; describe('server saved SQL queries', () => { + it('ignores saved PromQL filters', () => { + const promql: SavedLogView = { + ...classicSearch, + filter_id: '01HPROMQL', + stream_name: 'otel_metrics', + query: { filter_type: 'promql', filter_query: 'sum(rate(http_requests_total[5m]))' }, + }; + expect(savedLogView(promql)).toBe(true); + expect(serverSavedQuery(promql)).toBeUndefined(); + expect(serverSavedQueries([promql])).toEqual([]); + }); it('accepts the real classic fixtures through the client contract', () => { for (const view of [classicSql, classicLogsSql, nativeLogsSql, classicBuilder, classicSearch]) expect(savedLogView(view)).toBe(true); diff --git a/frontend/src/lib/logsContract.test.ts b/frontend/src/lib/logsContract.test.ts index 63040a6d7..0acf36ff4 100644 --- a/frontend/src/lib/logsContract.test.ts +++ b/frontend/src/lib/logsContract.test.ts @@ -106,6 +106,17 @@ describe('saved view HTTP contract', () => { : client.convertLogViewToSql('id'), ).rejects.toBeInstanceOf(ApiError); }); + it('accepts saved PromQL filters in the shared list without failing log views', async () => { + const promql: SavedLogView = { + ...body, + filter_id: 'promql-id', + stream_name: 'otel_metrics', + query: { filter_type: 'promql', filter_query: 'sum(rate(http_requests_total[5m]))' }, + }; + const saved = { ...body, filter_id: 'server-id' }; + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(json([saved, promql]))); + expect(await createClient({ mode: 'live' }).listLogViews()).toEqual([saved, promql]); + }); it('validates optional explorer state while retaining classic metadata', () => { for (const explorer_state of [undefined, null, 'encoded']) expect(savedLogView({ ...body, explorer_state, classic_metadata: { anything: true } })).toBe( diff --git a/frontend/src/lib/logsContract.ts b/frontend/src/lib/logsContract.ts index 3a04a96b9..43ce54fa5 100644 --- a/frontend/src/lib/logsContract.ts +++ b/frontend/src/lib/logsContract.ts @@ -40,7 +40,7 @@ export function savedLogView(value: unknown): value is SavedLogView { optionalString(value.explorer_state) && object(value.query) && typeof value.query.filter_type === 'string' && - ['filter', 'sql', 'search'].includes(value.query.filter_type) && + ['filter', 'sql', 'search', 'promql'].includes(value.query.filter_type) && optionalString(value.query.filter_query) && (value.query.filter_builder == null || builder(value.query.filter_builder)) && (value.time_filter == null || diff --git a/frontend/src/lib/types.ts b/frontend/src/lib/types.ts index d540b63df..fb55fb0e6 100644 --- a/frontend/src/lib/types.ts +++ b/frontend/src/lib/types.ts @@ -130,7 +130,7 @@ export type SavedLogView = { filter_name: string; stream_name: string; query: { - filter_type: 'filter' | 'sql' | 'search'; + filter_type: 'filter' | 'sql' | 'search' | 'promql'; filter_query?: string | null; filter_builder?: SavedLogViewBuilder | null; }; diff --git a/src/handlers/http/users/filters.rs b/src/handlers/http/users/filters.rs index 6f87e3ee2..a27438aba 100644 --- a/src/handlers/http/users/filters.rs +++ b/src/handlers/http/users/filters.rs @@ -306,7 +306,7 @@ mod tests { assert_eq!(converted, once); converted.stream_name = legacy.stream_name.clone(); assert_eq!(converted, legacy); - for kind in [FilterType::Filter, FilterType::Search] { + for kind in [FilterType::Filter, FilterType::Search, FilterType::Promql] { converted.query.filter_type = kind; let unchanged = converted.clone(); assert!(super::convert_filter_to_sql(&mut converted).is_err()); @@ -333,6 +333,16 @@ mod tests { assert!(validate_stream_name(SQL_FILTER_STREAM_NAME, &FilterType::Search).is_err()); } + #[test] + fn promql_filters_require_a_concrete_dataset_name() { + // PromQL evaluation is scoped to one dataset, and listing authorizes + // the saved filter by that dataset, so the SQL sentinel is not valid. + assert!(validate_stream_name("otel_metrics", &FilterType::Promql).is_ok()); + for stream_name in [SQL_FILTER_STREAM_NAME, "", "team1/serviceA", "../metrics"] { + assert!(validate_stream_name(stream_name, &FilterType::Promql).is_err()); + } + } + #[test] fn rejects_path_like_stream_names() { let stream_name = "team1/serviceA".to_owned(); diff --git a/src/users/filters.rs b/src/users/filters.rs index 216ba54c4..bdddbe8a8 100644 --- a/src/users/filters.rs +++ b/src/users/filters.rs @@ -252,6 +252,63 @@ mod tests { })).unwrap() } + fn promql_filter(stream_name: &str) -> Filter { + serde_json::from_value(serde_json::json!({ + "version":"v2", "user_id":"owner", "filter_id":format!("promql-{stream_name}"), + "stream_name":stream_name, "filter_name":"Request rate", "tenant_id":null, + "query":{"filter_type":"promql","filter_query":"sum(rate(http_requests_total[5m]))","filter_builder":null}, + "time_filter":null + })).unwrap() + } + + // `rbac::map::init` needs `PARSEABLE`, so tests create the maps directly. + // A user without groups never reads `ROLES`, which other tests seed. + fn session_with(permissions: Vec) -> SessionKey { + use crate::rbac::map::{SESSIONS, Sessions, USER_GROUPS, USERS, UserGroups}; + USERS.get_or_init(|| parking_lot::RwLock::new(crate::rbac::map::Users::default())); + SESSIONS.get_or_init(|| parking_lot::RwLock::new(Sessions::default())); + USER_GROUPS.get_or_init(|| parking_lot::RwLock::new(UserGroups::default())); + let key = SessionKey::SessionId(ulid::Ulid::new()); + crate::rbac::map::mut_sessions().track_new( + format!("user-{key:?}"), + key.clone(), + chrono::DateTime::::MAX_UTC, + permissions, + &None, + ); + key + } + + #[test] + fn promql_filter_type_round_trips_as_lowercase() { + let filter = promql_filter("otel_metrics"); + assert_eq!(filter.query.filter_type, FilterType::Promql); + assert_eq!(filter.query.filter_type.to_str(), "promql"); + let value = serde_json::to_value(&filter).unwrap(); + assert_eq!(value["query"]["filter_type"], "promql"); + assert_eq!(serde_json::from_value::(value).unwrap(), filter); + } + + #[tokio::test] + async fn promql_filters_are_authorized_by_dataset_not_parsed_as_sql() { + use crate::rbac::role::{Action, Permission}; + let cache = Filters::default(); + let filter = promql_filter("otel_metrics"); + cache.update(&filter, &None).await; + + // PromQL is not SQL: routing it through SQL table extraction would + // fail and silently hide the filter from authorized users. + let reader = session_with(vec![Permission::Resource(Action::Query, None)]); + assert_eq!(cache.list_filters(&reader).await, vec![filter]); + + let unauthorized = session_with(vec![]); + assert!(cache.list_filters(&unauthorized).await.is_empty()); + + for key in [reader, unauthorized] { + crate::rbac::map::mut_sessions().remove_session(&key); + } + } + #[tokio::test] async fn saved_filter_cache_prefers_administrators_own_copy_of_shared_id() { let cache = Filters::default();