From f82d05607258cff6ed716585d09024ef294c5fb7 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Tue, 26 Mar 2024 10:36:24 +0100 Subject: [PATCH 1/2] Hide secrets in settings and task queue --- index-scheduler/src/batch.rs | 6 ++- meilisearch-types/src/settings.rs | 53 +++++++++++++++++++++- meilisearch-types/src/task_view.rs | 3 +- meilisearch/src/routes/indexes/settings.rs | 6 +-- meilitool/src/main.rs | 6 ++- 5 files changed, 66 insertions(+), 8 deletions(-) diff --git a/index-scheduler/src/batch.rs b/index-scheduler/src/batch.rs index b7e31c136..3161dc499 100644 --- a/index-scheduler/src/batch.rs +++ b/index-scheduler/src/batch.rs @@ -920,7 +920,11 @@ impl IndexScheduler { } // 3.2. Dump the settings - let settings = meilisearch_types::settings::settings(index, &rtxn)?; + let settings = meilisearch_types::settings::settings( + index, + &rtxn, + meilisearch_types::settings::SecretPolicy::RevealSecrets, + )?; index_dumper.settings(&settings)?; Ok(()) })?; diff --git a/meilisearch-types/src/settings.rs b/meilisearch-types/src/settings.rs index 5480e72c6..ce3a74d69 100644 --- a/meilisearch-types/src/settings.rs +++ b/meilisearch-types/src/settings.rs @@ -211,6 +211,43 @@ pub struct Settings { pub _kind: PhantomData, } +impl Settings { + pub fn hide_secrets(&mut self) { + let Setting::Set(embedders) = &mut self.embedders else { + return; + }; + + for mut embedder in embedders.values_mut() { + let Setting::Set(embedder) = &mut embedder else { + continue; + }; + + let Setting::Set(api_key) = &mut embedder.api_key else { + continue; + }; + + Self::hide_secret(api_key); + } + } + + fn hide_secret(secret: &mut String) { + match secret.len() { + x if x < 10 => { + secret.replace_range(.., "XXX..."); + } + x if x < 20 => { + secret.replace_range(2.., "XXXX..."); + } + x if x < 30 => { + secret.replace_range(3.., "XXXXX..."); + } + _x => { + secret.replace_range(5.., "XXXXXX..."); + } + } + } +} + impl Settings { pub fn cleared() -> Settings { Settings { @@ -555,9 +592,15 @@ pub fn apply_settings_to_builder( } } +pub enum SecretPolicy { + RevealSecrets, + HideSecrets, +} + pub fn settings( index: &Index, rtxn: &crate::heed::RoTxn, + secret_policy: SecretPolicy, ) -> Result, milli::Error> { let displayed_attributes = index.displayed_fields(rtxn)?.map(|fields| fields.into_iter().map(String::from).collect()); @@ -643,7 +686,7 @@ pub fn settings( let search_cutoff_ms = index.search_cutoff(rtxn)?; - Ok(Settings { + let mut settings = Settings { displayed_attributes: match displayed_attributes { Some(attrs) => Setting::Set(attrs), None => Setting::Reset, @@ -674,7 +717,13 @@ pub fn settings( None => Setting::Reset, }, _kind: PhantomData, - }) + }; + + if let SecretPolicy::HideSecrets = secret_policy { + settings.hide_secrets() + } + + Ok(settings) } #[derive(Debug, Clone, PartialEq, Eq, Deserr)] diff --git a/meilisearch-types/src/task_view.rs b/meilisearch-types/src/task_view.rs index 02be91a88..659427c9d 100644 --- a/meilisearch-types/src/task_view.rs +++ b/meilisearch-types/src/task_view.rs @@ -86,7 +86,8 @@ impl From
for DetailsView { ..DetailsView::default() } } - Details::SettingsUpdate { settings } => { + Details::SettingsUpdate { mut settings } => { + settings.hide_secrets(); DetailsView { settings: Some(settings), ..DetailsView::default() } } Details::IndexInfo { primary_key } => { diff --git a/meilisearch/src/routes/indexes/settings.rs b/meilisearch/src/routes/indexes/settings.rs index 99c3d0fbb..0918444ef 100644 --- a/meilisearch/src/routes/indexes/settings.rs +++ b/meilisearch/src/routes/indexes/settings.rs @@ -7,7 +7,7 @@ use meilisearch_types::error::ResponseError; use meilisearch_types::facet_values_sort::FacetValuesSort; use meilisearch_types::index_uid::IndexUid; use meilisearch_types::milli::update::Setting; -use meilisearch_types::settings::{settings, RankingRuleView, Settings, Unchecked}; +use meilisearch_types::settings::{settings, RankingRuleView, SecretPolicy, Settings, Unchecked}; use meilisearch_types::tasks::KindWithContent; use serde_json::json; use tracing::debug; @@ -134,7 +134,7 @@ macro_rules! make_setting_route { let index = index_scheduler.index(&index_uid)?; let rtxn = index.read_txn()?; - let settings = settings(&index, &rtxn)?; + let settings = settings(&index, &rtxn, meilisearch_types::settings::SecretPolicy::HideSecrets)?; debug!(returns = ?settings, "Update settings"); let mut json = serde_json::json!(&settings); @@ -819,7 +819,7 @@ pub async fn get_all( let index = index_scheduler.index(&index_uid)?; let rtxn = index.read_txn()?; - let new_settings = settings(&index, &rtxn)?; + let new_settings = settings(&index, &rtxn, SecretPolicy::HideSecrets)?; debug!(returns = ?new_settings, "Get all settings"); Ok(HttpResponse::Ok().json(new_settings)) } diff --git a/meilitool/src/main.rs b/meilitool/src/main.rs index f199df216..bace7d16b 100644 --- a/meilitool/src/main.rs +++ b/meilitool/src/main.rs @@ -291,7 +291,11 @@ fn export_a_dump( } // 4.2. Dump the settings - let settings = meilisearch_types::settings::settings(&index, &rtxn)?; + let settings = meilisearch_types::settings::settings( + &index, + &rtxn, + meilisearch_types::settings::SecretPolicy::RevealSecrets, + )?; index_dumper.settings(&settings)?; count += 1; } From 9a95ed619dc41f502c11e509646adbe9872acbee Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Tue, 26 Mar 2024 10:36:56 +0100 Subject: [PATCH 2/2] Add tests --- index-scheduler/src/lib.rs | 60 ++++++++ ...x_scheduler__tests__settings_update-2.snap | 13 ++ ...x_scheduler__tests__settings_update-3.snap | 23 ++++ ...dex_scheduler__tests__settings_update.snap | 13 ++ .../after_registering_settings_task.snap | 36 +++++ .../settings_update_processed.snap | 40 ++++++ meilisearch/tests/settings/get_settings.rs | 130 ++++++++++++++++++ 7 files changed, 315 insertions(+) create mode 100644 index-scheduler/src/snapshots/index_scheduler__tests__settings_update-2.snap create mode 100644 index-scheduler/src/snapshots/index_scheduler__tests__settings_update-3.snap create mode 100644 index-scheduler/src/snapshots/index_scheduler__tests__settings_update.snap create mode 100644 index-scheduler/src/snapshots/lib.rs/test_settings_update/after_registering_settings_task.snap create mode 100644 index-scheduler/src/snapshots/lib.rs/test_settings_update/settings_update_processed.snap diff --git a/index-scheduler/src/lib.rs b/index-scheduler/src/lib.rs index 46ed76649..84416e869 100644 --- a/index-scheduler/src/lib.rs +++ b/index-scheduler/src/lib.rs @@ -3028,6 +3028,66 @@ mod tests { snapshot!(serde_json::to_string_pretty(&documents).unwrap(), name: "documents"); } + #[test] + fn test_settings_update() { + use meilisearch_types::settings::{Settings, Unchecked}; + use milli::update::Setting; + + let (index_scheduler, mut handle) = IndexScheduler::test(true, vec![]); + + let mut new_settings: Box> = Box::default(); + let mut embedders = BTreeMap::default(); + let embedding_settings = milli::vector::settings::EmbeddingSettings { + source: Setting::Set(milli::vector::settings::EmbedderSource::Rest), + api_key: Setting::Set(S("My super secret")), + url: Setting::Set(S("http://localhost:7777")), + ..Default::default() + }; + embedders.insert(S("default"), Setting::Set(embedding_settings)); + new_settings.embedders = Setting::Set(embedders); + + index_scheduler + .register( + KindWithContent::SettingsUpdate { + index_uid: S("doggos"), + new_settings, + is_deletion: false, + allow_index_creation: true, + }, + None, + false, + ) + .unwrap(); + index_scheduler.assert_internally_consistent(); + + snapshot!(snapshot_index_scheduler(&index_scheduler), name: "after_registering_settings_task"); + + { + let rtxn = index_scheduler.read_txn().unwrap(); + let task = index_scheduler.get_task(&rtxn, 0).unwrap().unwrap(); + let task = meilisearch_types::task_view::TaskView::from_task(&task); + insta::assert_json_snapshot!(task.details); + } + + handle.advance_n_successful_batches(1); + snapshot!(snapshot_index_scheduler(&index_scheduler), name: "settings_update_processed"); + + { + let rtxn = index_scheduler.read_txn().unwrap(); + let task = index_scheduler.get_task(&rtxn, 0).unwrap().unwrap(); + let task = meilisearch_types::task_view::TaskView::from_task(&task); + insta::assert_json_snapshot!(task.details); + } + + // has everything being pushed successfully in milli? + let index = index_scheduler.index("doggos").unwrap(); + let rtxn = index.read_txn().unwrap(); + + let configs = index.embedding_configs(&rtxn).unwrap(); + let (_, embedding_config) = configs.first().unwrap(); + insta::assert_json_snapshot!(embedding_config.embedder_options); + } + #[test] fn test_document_replace_without_autobatching() { let (index_scheduler, mut handle) = IndexScheduler::test(false, vec![]); diff --git a/index-scheduler/src/snapshots/index_scheduler__tests__settings_update-2.snap b/index-scheduler/src/snapshots/index_scheduler__tests__settings_update-2.snap new file mode 100644 index 000000000..85f0926b9 --- /dev/null +++ b/index-scheduler/src/snapshots/index_scheduler__tests__settings_update-2.snap @@ -0,0 +1,13 @@ +--- +source: index-scheduler/src/lib.rs +expression: task.details +--- +{ + "embedders": { + "default": { + "source": "rest", + "apiKey": "MyXXXX...", + "url": "http://localhost:7777" + } + } +} diff --git a/index-scheduler/src/snapshots/index_scheduler__tests__settings_update-3.snap b/index-scheduler/src/snapshots/index_scheduler__tests__settings_update-3.snap new file mode 100644 index 000000000..50a42d678 --- /dev/null +++ b/index-scheduler/src/snapshots/index_scheduler__tests__settings_update-3.snap @@ -0,0 +1,23 @@ +--- +source: index-scheduler/src/lib.rs +expression: embedding_config.embedder_options +--- +{ + "Rest": { + "api_key": "My super secret", + "distribution": null, + "dimensions": null, + "url": "http://localhost:7777", + "query": null, + "input_field": [ + "input" + ], + "path_to_embeddings": [ + "data" + ], + "embedding_object": [ + "embedding" + ], + "input_type": "text" + } +} diff --git a/index-scheduler/src/snapshots/index_scheduler__tests__settings_update.snap b/index-scheduler/src/snapshots/index_scheduler__tests__settings_update.snap new file mode 100644 index 000000000..85f0926b9 --- /dev/null +++ b/index-scheduler/src/snapshots/index_scheduler__tests__settings_update.snap @@ -0,0 +1,13 @@ +--- +source: index-scheduler/src/lib.rs +expression: task.details +--- +{ + "embedders": { + "default": { + "source": "rest", + "apiKey": "MyXXXX...", + "url": "http://localhost:7777" + } + } +} diff --git a/index-scheduler/src/snapshots/lib.rs/test_settings_update/after_registering_settings_task.snap b/index-scheduler/src/snapshots/lib.rs/test_settings_update/after_registering_settings_task.snap new file mode 100644 index 000000000..01bb73993 --- /dev/null +++ b/index-scheduler/src/snapshots/lib.rs/test_settings_update/after_registering_settings_task.snap @@ -0,0 +1,36 @@ +--- +source: index-scheduler/src/lib.rs +--- +### Autobatching Enabled = true +### Processing Tasks: +[] +---------------------------------------------------------------------- +### All Tasks: +0 {uid: 0, status: enqueued, details: { settings: Settings { displayed_attributes: NotSet, searchable_attributes: NotSet, filterable_attributes: NotSet, sortable_attributes: NotSet, ranking_rules: NotSet, stop_words: NotSet, non_separator_tokens: NotSet, separator_tokens: NotSet, dictionary: NotSet, synonyms: NotSet, distinct_attribute: NotSet, proximity_precision: NotSet, typo_tolerance: NotSet, faceting: NotSet, pagination: NotSet, embedders: Set({"default": Set(EmbeddingSettings { source: Set(Rest), model: NotSet, revision: NotSet, api_key: Set("My super secret"), dimensions: NotSet, document_template: NotSet, url: Set("http://localhost:7777"), query: NotSet, input_field: NotSet, path_to_embeddings: NotSet, embedding_object: NotSet, input_type: NotSet })}), search_cutoff_ms: NotSet, _kind: PhantomData } }, kind: SettingsUpdate { index_uid: "doggos", new_settings: Settings { displayed_attributes: NotSet, searchable_attributes: NotSet, filterable_attributes: NotSet, sortable_attributes: NotSet, ranking_rules: NotSet, stop_words: NotSet, non_separator_tokens: NotSet, separator_tokens: NotSet, dictionary: NotSet, synonyms: NotSet, distinct_attribute: NotSet, proximity_precision: NotSet, typo_tolerance: NotSet, faceting: NotSet, pagination: NotSet, embedders: Set({"default": Set(EmbeddingSettings { source: Set(Rest), model: NotSet, revision: NotSet, api_key: Set("My super secret"), dimensions: NotSet, document_template: NotSet, url: Set("http://localhost:7777"), query: NotSet, input_field: NotSet, path_to_embeddings: NotSet, embedding_object: NotSet, input_type: NotSet })}), search_cutoff_ms: NotSet, _kind: PhantomData }, is_deletion: false, allow_index_creation: true }} +---------------------------------------------------------------------- +### Status: +enqueued [0,] +---------------------------------------------------------------------- +### Kind: +"settingsUpdate" [0,] +---------------------------------------------------------------------- +### Index Tasks: +doggos [0,] +---------------------------------------------------------------------- +### Index Mapper: + +---------------------------------------------------------------------- +### Canceled By: + +---------------------------------------------------------------------- +### Enqueued At: +[timestamp] [0,] +---------------------------------------------------------------------- +### Started At: +---------------------------------------------------------------------- +### Finished At: +---------------------------------------------------------------------- +### File Store: + +---------------------------------------------------------------------- + diff --git a/index-scheduler/src/snapshots/lib.rs/test_settings_update/settings_update_processed.snap b/index-scheduler/src/snapshots/lib.rs/test_settings_update/settings_update_processed.snap new file mode 100644 index 000000000..d1d219da1 --- /dev/null +++ b/index-scheduler/src/snapshots/lib.rs/test_settings_update/settings_update_processed.snap @@ -0,0 +1,40 @@ +--- +source: index-scheduler/src/lib.rs +--- +### Autobatching Enabled = true +### Processing Tasks: +[] +---------------------------------------------------------------------- +### All Tasks: +0 {uid: 0, status: succeeded, details: { settings: Settings { displayed_attributes: NotSet, searchable_attributes: NotSet, filterable_attributes: NotSet, sortable_attributes: NotSet, ranking_rules: NotSet, stop_words: NotSet, non_separator_tokens: NotSet, separator_tokens: NotSet, dictionary: NotSet, synonyms: NotSet, distinct_attribute: NotSet, proximity_precision: NotSet, typo_tolerance: NotSet, faceting: NotSet, pagination: NotSet, embedders: Set({"default": Set(EmbeddingSettings { source: Set(Rest), model: NotSet, revision: NotSet, api_key: Set("My super secret"), dimensions: NotSet, document_template: NotSet, url: Set("http://localhost:7777"), query: NotSet, input_field: NotSet, path_to_embeddings: NotSet, embedding_object: NotSet, input_type: NotSet })}), search_cutoff_ms: NotSet, _kind: PhantomData } }, kind: SettingsUpdate { index_uid: "doggos", new_settings: Settings { displayed_attributes: NotSet, searchable_attributes: NotSet, filterable_attributes: NotSet, sortable_attributes: NotSet, ranking_rules: NotSet, stop_words: NotSet, non_separator_tokens: NotSet, separator_tokens: NotSet, dictionary: NotSet, synonyms: NotSet, distinct_attribute: NotSet, proximity_precision: NotSet, typo_tolerance: NotSet, faceting: NotSet, pagination: NotSet, embedders: Set({"default": Set(EmbeddingSettings { source: Set(Rest), model: NotSet, revision: NotSet, api_key: Set("My super secret"), dimensions: NotSet, document_template: NotSet, url: Set("http://localhost:7777"), query: NotSet, input_field: NotSet, path_to_embeddings: NotSet, embedding_object: NotSet, input_type: NotSet })}), search_cutoff_ms: NotSet, _kind: PhantomData }, is_deletion: false, allow_index_creation: true }} +---------------------------------------------------------------------- +### Status: +enqueued [] +succeeded [0,] +---------------------------------------------------------------------- +### Kind: +"settingsUpdate" [0,] +---------------------------------------------------------------------- +### Index Tasks: +doggos [0,] +---------------------------------------------------------------------- +### Index Mapper: +doggos: { number_of_documents: 0, field_distribution: {} } + +---------------------------------------------------------------------- +### Canceled By: + +---------------------------------------------------------------------- +### Enqueued At: +[timestamp] [0,] +---------------------------------------------------------------------- +### Started At: +[timestamp] [0,] +---------------------------------------------------------------------- +### Finished At: +[timestamp] [0,] +---------------------------------------------------------------------- +### File Store: + +---------------------------------------------------------------------- + diff --git a/meilisearch/tests/settings/get_settings.rs b/meilisearch/tests/settings/get_settings.rs index 09e38e55a..980ef3064 100644 --- a/meilisearch/tests/settings/get_settings.rs +++ b/meilisearch/tests/settings/get_settings.rs @@ -88,6 +88,136 @@ async fn get_settings() { assert_eq!(settings["searchCutoffMs"], json!(null)); } +#[actix_rt::test] +async fn secrets_are_hidden_in_settings() { + let server = Server::new().await; + let (response, code) = server.set_features(json!({"vectorStore": true})).await; + + meili_snap::snapshot!(code, @"200 OK"); + meili_snap::snapshot!(meili_snap::json_string!(response), @r###" + { + "vectorStore": true, + "metrics": false, + "logsRoute": false, + "exportPuffinReports": false + } + "###); + + let index = server.index("test"); + let (response, _code) = index.create(None).await; + index.wait_task(response.uid()).await; + + let (response, code) = index + .update_settings(json!({ + "embedders": { + "default": { + "source": "rest", + "url": "https://localhost:7777", + "apiKey": "My super secret value you will never guess" + } + } + })) + .await; + meili_snap::snapshot!(code, @"202 Accepted"); + + meili_snap::snapshot!(meili_snap::json_string!(response, { ".duration" => "[duration]", ".enqueuedAt" => "[date]", ".startedAt" => "[date]", ".finishedAt" => "[date]" }), + @r###" + { + "taskUid": 1, + "indexUid": "test", + "status": "enqueued", + "type": "settingsUpdate", + "enqueuedAt": "[date]" + } + "###); + + let settings_update_uid = response.uid(); + + index.wait_task(settings_update_uid).await; + + let (response, code) = index.settings().await; + meili_snap::snapshot!(code, @"200 OK"); + meili_snap::snapshot!(meili_snap::json_string!(response), @r###" + { + "displayedAttributes": [ + "*" + ], + "searchableAttributes": [ + "*" + ], + "filterableAttributes": [], + "sortableAttributes": [], + "rankingRules": [ + "words", + "typo", + "proximity", + "attribute", + "sort", + "exactness" + ], + "stopWords": [], + "nonSeparatorTokens": [], + "separatorTokens": [], + "dictionary": [], + "synonyms": {}, + "distinctAttribute": null, + "proximityPrecision": "byWord", + "typoTolerance": { + "enabled": true, + "minWordSizeForTypos": { + "oneTypo": 5, + "twoTypos": 9 + }, + "disableOnWords": [], + "disableOnAttributes": [] + }, + "faceting": { + "maxValuesPerFacet": 100, + "sortFacetValuesBy": { + "*": "alpha" + } + }, + "pagination": { + "maxTotalHits": 1000 + }, + "embedders": { + "default": { + "source": "rest", + "apiKey": "My suXXXXXX...", + "documentTemplate": "{% for field in fields %} {{ field.name }}: {{ field.value }}\n{% endfor %}", + "url": "https://localhost:7777", + "query": null, + "inputField": [ + "input" + ], + "pathToEmbeddings": [ + "data" + ], + "embeddingObject": [ + "embedding" + ], + "inputType": "text" + } + }, + "searchCutoffMs": null + } + "###); + + let (response, code) = server.get_task(settings_update_uid).await; + meili_snap::snapshot!(code, @"200 OK"); + meili_snap::snapshot!(meili_snap::json_string!(response["details"]), @r###" + { + "embedders": { + "default": { + "source": "rest", + "apiKey": "My suXXXXXX...", + "url": "https://localhost:7777" + } + } + } + "###); +} + #[actix_rt::test] async fn error_update_settings_unknown_field() { let server = Server::new().await;