Skip to content

Commit a021786

Browse files
fix(dashboard): embedding Test Connection expands {file:}/{env:} tokens and allows http/loopback/LAN
Two user-reported bugs in the dashboard 'Test Connection' probe, both from the probe applying untrusted-project-config security policy to what is actually the user-config editor: 1. A {file:~/...key} api_key (the pattern we document and recommend) was refused with an {env:}-flavored message ('variable is not set in this process / shell exports the var') that is nonsense for a file token. 2. A self-hosted http://127.0.0.1:1234/v1 endpoint was rejected as 'must start with http:// or https://' (it does) — the validator was https-only, and even with http allowed the SSRF guard then blocked loopback, so localhost servers (LMStudio/Ollama/llama.cpp, the most common local setup) couldn't be tested. The probe's only input is the USER-level ConfigEditor; project-tier editing covers only dreamer schedule/model, never embedding. So the 'malicious project config picks a hostile endpoint/secret-file and the user clicks Test' precondition is not reachable here, and the probe now mirrors doctor (which it is documented to mirror): - expand_config_value() resolves {env:VAR} and {file:~|/abs|rel} exactly like the plugin's load-time variable.ts (file contents trimmed; relative paths resolve against the user config dir). Unresolved tokens still don't reach the network. - {file:} tokens resolving into credential dirs (~/.ssh, ~/.aws, ~/.gnupg, ~/.config/gh) are REFUSED (new BlockedSensitiveFile outcome) — defense in depth. - http:// is now accepted alongside https://. - loopback + private/LAN (incl. IPv6 ULA) are now allowed. Cloud instance-metadata (169.254.169.254, AWS IPv6 fd00:ec2::254, incl. IPv4-mapped), IPv4/IPv6 link-local, and unspecified stay blocked; URL userinfo still rejected. - Frontend message branches on token kind (file vs env) so the guidance is accurate, and renders the new sensitive-file refusal. Documented the reversal of the prior probe-SSRF audit finding as A53 so future audits don't re-flag it. Rust 133/0 (incl. new metadata/loopback/scheme/expand tests), tsc + biome + build clean. Co-authored-by: Alfonso [Magic Context] <288211368+alfonso-magic-context@users.noreply.github.com>
1 parent 1821164 commit a021786

5 files changed

Lines changed: 381 additions & 64 deletions

File tree

docs/AUDIT-KNOWN-ISSUES.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -862,3 +862,25 @@ config" — trusting it (and letting GC run against the model it names) is corre
862862
not a data-safety hole. The destructive-sweep scenario requires a conflicted
863863
shared file to exist, which the migrator structurally never produces. If a user
864864
hand-authors a shared config, that IS their authoritative intent by definition.
865+
866+
### A53. Dashboard embedding "Test Connection" expands {env:}/{file:} tokens and allows http/loopback/LAN (deliberate, user-config-only surface)
867+
A prior audit added a hard SSRF + token-refusal guard to the dashboard
868+
`test_embedding_endpoint` probe (`embedding_probe.rs`): it refused to expand
869+
`{env:}`/`{file:}` tokens, required `https`, and blocked loopback + RFC1918.
870+
That guard was relaxed deliberately because it broke the two most common, fully
871+
legitimate setups (a `{file:~/...key}` api_key, the pattern we document, and a
872+
self-hosted `http://localhost` embedding server) while protecting a threat that
873+
is not reachable on this surface. The probe's only input is the **user-level**
874+
config editor (`ConfigEditor`, rendered once from `App.tsx`); project-tier
875+
editing covers only dreamer schedule/model, never embedding. So the
876+
"malicious-project-config picks a hostile endpoint/secret-file and the user
877+
clicks Test" precondition is **not satisfiable**: the values are always the
878+
user's own, and expanding them to test is exactly what the plugin does at
879+
runtime. The probe now mirrors `doctor` (Node), which it is documented to mirror.
880+
Retained guards (cheap, no legit cost): cloud instance-metadata (IPv4
881+
169.254.169.254 + AWS IPv6 `fd00:ec2::254`, incl. IPv4-mapped) is always blocked;
882+
IPv4 link-local (169.254/16) + IPv6 link-local (fe80::/10) + unspecified stay
883+
blocked; URL userinfo (`user:pass@`) is rejected; and `{file:}` tokens resolving
884+
into credential dirs (`~/.ssh`, `~/.aws`, `~/.gnupg`, `~/.config/gh`) are refused.
885+
Do not re-add the https-only / token-refusal / loopback-block guards without a
886+
new threat model showing an attacker-controlled value can reach this probe.

packages/dashboard/src-tauri/src/commands.rs

Lines changed: 69 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
use crate::embedding_probe::{
2-
find_substitution_token, probe_embedding_endpoint, EmbeddingProbeOptions, EmbeddingProbeOutcome,
2+
expand_config_value, probe_embedding_endpoint, EmbeddingProbeOptions, EmbeddingProbeOutcome,
3+
TokenExpandError,
34
};
45
use crate::process_ext::NoWindowExtTokio;
56
use crate::{config, db, log_parser, AppState};
7+
use std::path::{Path, PathBuf};
68
use tauri::State;
79

810
// ── Memory commands ─────────────────────────────────────────
@@ -903,10 +905,27 @@ pub async fn get_available_pi_models() -> Vec<String> {
903905
///
904906
/// 3. 401/403 → `AuthFailed` (specific "credentials rejected" message).
905907
/// 404/405 → `EndpointUnsupported` (wrong URL / no embeddings API).
906-
fn reject_probe_token(field: &str, value: &str) -> Option<EmbeddingProbeOutcome> {
907-
find_substitution_token(value).map(|token| EmbeddingProbeOutcome::UnresolvedToken {
908-
field: field.to_string(),
909-
token,
908+
/// Expand an `{env:}`/`{file:}` token in one probe field, mapping a failure to
909+
/// the matching probe outcome. The embedding fields come from the USER-level
910+
/// config editor, so expanding the user's own tokens to test their endpoint is
911+
/// exactly what the plugin does at runtime (see `expand_config_value`).
912+
fn expand_probe_field(
913+
field: &str,
914+
value: &str,
915+
config_dir: &Path,
916+
) -> Result<String, EmbeddingProbeOutcome> {
917+
expand_config_value(value, config_dir).map_err(|err| match err {
918+
TokenExpandError::Unresolved(token) => EmbeddingProbeOutcome::UnresolvedToken {
919+
field: field.to_string(),
920+
token,
921+
},
922+
TokenExpandError::SensitiveFile { token, reason } => {
923+
EmbeddingProbeOutcome::BlockedSensitiveFile {
924+
field: field.to_string(),
925+
token,
926+
reason,
927+
}
928+
}
910929
})
911930
}
912931

@@ -917,17 +936,19 @@ fn prepare_embedding_probe_options(
917936
input_type: Option<String>,
918937
truncate: Option<String>,
919938
) -> Result<EmbeddingProbeOptions, EmbeddingProbeOutcome> {
920-
if let Some(outcome) = reject_probe_token("endpoint", &endpoint) {
921-
return Err(outcome);
922-
}
923-
if let Some(outcome) = reject_probe_token("model", &model) {
924-
return Err(outcome);
925-
}
926-
if let Some(key) = api_key.as_deref() {
927-
if let Some(outcome) = reject_probe_token("api_key", key) {
928-
return Err(outcome);
929-
}
930-
}
939+
// Relative `{file:}` references resolve against the user config file's dir,
940+
// matching the plugin's load-time resolution.
941+
let config_dir = config::resolve_user_config_path()
942+
.parent()
943+
.map(Path::to_path_buf)
944+
.unwrap_or_else(|| PathBuf::from("."));
945+
946+
let endpoint = expand_probe_field("endpoint", &endpoint, &config_dir)?;
947+
let model = expand_probe_field("model", &model, &config_dir)?;
948+
let api_key = match api_key {
949+
Some(key) => Some(expand_probe_field("api_key", &key, &config_dir)?),
950+
None => None,
951+
};
931952

932953
let input_type = input_type
933954
.map(|s| s.trim().to_string())
@@ -1130,50 +1151,61 @@ mod tests {
11301151
}
11311152

11321153
#[test]
1133-
fn embedding_probe_refuses_file_api_key_without_reading_secret() {
1154+
fn embedding_probe_expands_file_api_key_for_the_test() {
1155+
// The embedding fields come from the USER config editor, so a
1156+
// {file:...} api_key is the user's own secret; expand it (as the
1157+
// plugin does at runtime) instead of refusing.
11341158
let dir = tempfile::tempdir().expect("tempdir");
11351159
let secret_path = dir.path().join("embedding-secret.txt");
1136-
std::fs::write(&secret_path, "super-secret-token").expect("write secret");
1160+
std::fs::write(&secret_path, " super-secret-token\n").expect("write secret");
11371161
let token = format!("{{file:{}}}", secret_path.display());
11381162

1139-
let outcome = prepare_embedding_probe_options(
1163+
let options = prepare_embedding_probe_options(
11401164
"https://example.com/v1".to_string(),
11411165
"text-embedding-3-small".to_string(),
1142-
Some(token.clone()),
1166+
Some(token),
11431167
None,
11441168
None,
11451169
)
1146-
.expect_err("file token must be refused before any network probe");
1170+
.expect("resolvable file token should expand, not error");
11471171

1148-
match outcome {
1149-
EmbeddingProbeOutcome::UnresolvedToken {
1150-
field,
1151-
token: reported,
1152-
} => {
1153-
assert_eq!(field, "api_key");
1154-
assert_eq!(reported, token);
1155-
}
1156-
other => panic!("expected unresolved token, got {other:?}"),
1157-
}
1172+
// file contents are trimmed by expand_config_value (the file had
1173+
// leading/trailing whitespace).
1174+
assert_eq!(options.api_key.as_deref(), Some("super-secret-token"));
11581175
}
11591176

11601177
#[test]
1161-
fn embedding_probe_refuses_env_model_token_without_expanding() {
1162-
std::env::set_var("MC_DASHBOARD_TEST_MODEL", "secret-model");
1163-
let outcome = prepare_embedding_probe_options(
1178+
fn embedding_probe_expands_env_model_token_for_the_test() {
1179+
std::env::set_var("MC_DASHBOARD_TEST_MODEL", "resolved-model");
1180+
let options = prepare_embedding_probe_options(
11641181
"https://example.com/v1".to_string(),
11651182
"{env:MC_DASHBOARD_TEST_MODEL}".to_string(),
11661183
Some("literal-key".to_string()),
11671184
None,
11681185
None,
11691186
)
1170-
.expect_err("env token must be refused before any network probe");
1187+
.expect("resolvable env token should expand, not error");
11711188
std::env::remove_var("MC_DASHBOARD_TEST_MODEL");
11721189

1190+
assert_eq!(options.model, "resolved-model");
1191+
}
1192+
1193+
#[test]
1194+
fn embedding_probe_reports_unresolved_env_token() {
1195+
std::env::remove_var("MC_DASHBOARD_UNSET_TEST_VAR");
1196+
let outcome = prepare_embedding_probe_options(
1197+
"https://example.com/v1".to_string(),
1198+
"text-embedding-3-small".to_string(),
1199+
Some("{env:MC_DASHBOARD_UNSET_TEST_VAR}".to_string()),
1200+
None,
1201+
None,
1202+
)
1203+
.expect_err("an unset env token must not reach the network");
1204+
11731205
match outcome {
11741206
EmbeddingProbeOutcome::UnresolvedToken { field, token } => {
1175-
assert_eq!(field, "model");
1176-
assert_eq!(token, "{env:MC_DASHBOARD_TEST_MODEL}");
1207+
assert_eq!(field, "api_key");
1208+
assert_eq!(token, "{env:MC_DASHBOARD_UNSET_TEST_VAR}");
11771209
}
11781210
other => panic!("expected unresolved token, got {other:?}"),
11791211
}

packages/dashboard/src-tauri/src/db.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1254,9 +1254,9 @@ fn build_db_cache_events_with_decisions(
12541254
finish: row.finish,
12551255
turn_id: String::new(),
12561256
is_turn_start: false,
1257-
context_limit: 0, // filled in after sessions are known
1257+
context_limit: 0, // filled in after sessions are known
12581258
context_limit_estimated: false, // set with context_limit below
1259-
is_drop: false, // computed in pass 2
1259+
is_drop: false, // computed in pass 2
12601260
});
12611261
}
12621262

@@ -6775,7 +6775,10 @@ mod memory_project_filter_tests {
67756775
let rows = get_memories(&conn, None, None, None, None, None, 100, 0)
67766776
.expect("get_memories must not error when memory_embeddings is absent");
67776777
assert_eq!(rows.len(), 1);
6778-
assert!(!rows[0].has_embedding, "no embeddings table → has_embedding=false");
6778+
assert!(
6779+
!rows[0].has_embedding,
6780+
"no embeddings table → has_embedding=false"
6781+
);
67796782

67806783
let stats = get_memory_stats(&conn, None, None).expect("stats must not error");
67816784
assert_eq!(stats.with_embeddings, 0);

0 commit comments

Comments
 (0)