fix(provider): enforce durable identity boundaries

This commit is contained in:
SaladDay
2026-08-01 09:09:14 +00:00
parent 37e78166c1
commit 22c010079a
2 changed files with 169 additions and 13 deletions
+9 -3
View File
@@ -133,19 +133,25 @@ impl NewProviderAggregate {
let mut seen = HashSet::with_capacity(endpoints.len());
let mut initial_endpoints = Vec::with_capacity(endpoints.len());
for (key, endpoint) in endpoints {
if key != endpoint.url {
let normalized_key = key.trim().trim_end_matches('/').to_string();
let normalized_url = endpoint.url.trim().trim_end_matches('/').to_string();
if normalized_key != normalized_url {
return Err(AppError::InvalidInput(format!(
"provider endpoint key '{key}' must match endpoint URL '{}'",
endpoint.url
)));
}
if !seen.insert(endpoint.url.clone()) {
if !seen.insert(normalized_url.clone()) {
return Err(AppError::InvalidInput(format!(
"duplicate initial provider endpoint '{}'",
endpoint.url
)));
}
initial_endpoints.push(endpoint.try_into()?);
initial_endpoints.push(NewEndpoint::new(
normalized_url,
endpoint.added_at,
endpoint.last_used,
)?);
}
let key = ProviderKey::new(app_type, input.id.clone())?;
let row = ProviderRowCreate {
+160 -10
View File
@@ -632,6 +632,90 @@ mod tests {
});
}
#[test]
#[serial]
fn provider_service_create_canonicalizes_initial_endpoint_identity() {
with_test_home(|state, _| {
let raw_url = " https://canonical.example/// ";
let mut provider = opencode_provider("canonical-endpoint");
provider.meta = Some(ProviderMeta {
custom_endpoints: HashMap::from([(
raw_url.to_string(),
endpoint(raw_url, None, Some(11)),
)]),
..Default::default()
});
ProviderService::add(
state,
AppType::OpenCode,
provider_to_mutation_input(provider),
false,
)
.expect("create with a non-canonical initial endpoint");
let aggregate = state
.db
.get_provider_aggregate("opencode", "canonical-endpoint")
.expect("read canonical aggregate")
.expect("canonical aggregate");
assert_eq!(aggregate.endpoints.len(), 1);
assert!(aggregate
.endpoints
.contains_key("https://canonical.example"));
ProviderService::update_endpoint_last_used(
state,
AppType::OpenCode,
"canonical-endpoint",
" https://canonical.example/ ".to_string(),
)
.expect("touch must resolve the same canonical endpoint");
ProviderService::remove_custom_endpoint(
state,
AppType::OpenCode,
"canonical-endpoint",
"https://canonical.example///".to_string(),
)
.expect("remove must resolve the same canonical endpoint");
assert!(state
.db
.get_provider_aggregate("opencode", "canonical-endpoint")
.expect("read after remove")
.expect("provider after remove")
.endpoints
.is_empty());
let mut duplicate = opencode_provider("duplicate-canonical-endpoint");
duplicate.meta = Some(ProviderMeta {
custom_endpoints: HashMap::from([
(
"https://duplicate.example".to_string(),
endpoint("https://duplicate.example", None, None),
),
(
" https://duplicate.example/ ".to_string(),
endpoint(" https://duplicate.example/ ", Some(1), None),
),
]),
..Default::default()
});
assert!(matches!(
ProviderService::add(
state,
AppType::OpenCode,
provider_to_mutation_input(duplicate),
false,
),
Err(AppError::InvalidInput(_))
));
assert!(state
.db
.get_provider_aggregate("opencode", "duplicate-canonical-endpoint")
.expect("read duplicate candidate")
.is_none());
});
}
#[test]
#[serial]
fn provider_service_stale_edit_payload_cannot_overwrite_endpoint_operations() {
@@ -868,6 +952,75 @@ mod tests {
});
}
#[test]
#[serial]
fn provider_service_rename_fails_closed_for_malformed_additive_live_config() {
with_test_home(|state, home| {
let cases = [
(
AppType::OpenCode,
opencode_provider("malformed-source"),
opencode_provider("malformed-target"),
home.join(".config").join("opencode").join("opencode.json"),
r#"{"provider":{"malformed-source":{"npm":"@ai-sdk/openai-compatible"}"#,
),
(
AppType::OpenClaw,
openclaw_provider("corrupt-source"),
openclaw_provider("corrupt-target"),
home.join(".openclaw").join("openclaw.json"),
r#"{"models":{"providers":{"corrupt-target":{"baseUrl":"https://example.test"}"#,
),
];
for (app_type, source, target, live_path, malformed_live) in cases {
let app_name = app_type.as_str().to_string();
let source_id = source.id.clone();
let target_id = target.id.clone();
ProviderService::add(
state,
app_type.clone(),
provider_to_mutation_input(source),
false,
)
.expect("create DB-only rename source");
fs::create_dir_all(live_path.parent().expect("live config parent"))
.expect("create live config directory");
fs::write(&live_path, malformed_live).expect("write malformed live config");
let source_before = provider_snapshot(state, &app_name, &source_id);
let target_before = provider_snapshot(state, &app_name, &target_id);
let error = ProviderService::update(
state,
app_type,
Some(&source_id),
provider_to_mutation_input(target),
)
.expect_err("rename must fail closed when live identity cannot be inspected");
assert!(
matches!(error, AppError::Config(_)),
"rename should surface the live parse error, got {error:?}"
);
assert_eq!(
provider_snapshot(state, &app_name, &source_id),
source_before,
"source aggregate must remain unchanged"
);
assert_eq!(
provider_snapshot(state, &app_name, &target_id),
target_before,
"target aggregate must remain unchanged"
);
assert_eq!(
fs::read_to_string(&live_path).expect("reread malformed live config"),
malformed_live,
"failed rename must not rewrite the live config"
);
}
});
}
#[test]
#[serial]
fn automated_row_transform_conflicts_instead_of_reverting_a_newer_edit() {
@@ -3363,11 +3516,12 @@ impl ProviderService {
));
}
let original_in_live = Self::check_live_config_exists(
&app_type,
&original_id,
Self::provider_live_config_managed(&existing_provider),
)?;
// A rename changes durable identity, so "cannot inspect live
// config" must never be treated as "not live". DB-only
// same-ID edits deliberately retain their tolerant path below,
// but rename proves both identities absent from a readable live
// config before committing the SQLite transaction.
let original_in_live = provider_exists_in_live_config(&app_type, &original_id)?;
if original_in_live {
return Err(AppError::Message(
"Provider key cannot be changed after the provider has been added to the app config"
@@ -3375,11 +3529,7 @@ impl ProviderService {
));
}
let next_id_in_live = Self::check_live_config_exists(
&app_type,
&provider.id,
Self::provider_live_config_managed(&existing_provider),
)?;
let next_id_in_live = provider_exists_in_live_config(&app_type, &provider.id)?;
if state
.db
.get_provider_by_id(&provider.id, app_type.as_str())?