fix(permissions): assistant grants file is the one authority (live desync fix)
ai_grants_unified UNIONED the assistant grants.json with the legacy settings/ai_permissions.json on every read. On archi-dev-box legacy held all-ten and grants.json held four, so the Settings UI and the AIUI frame saw every category ON while the assistant refused six — and no UI toggle could fix it, because both write paths existed but only ai.permissions.set synced both files. Now: an existing grants.json answers alone; the legacy file is consulted only when no grants file exists (pre-unification upgrade), and that read migrates forward and persists the authority. assistant.grants-set now also rewrites the legacy file in step. Regression tests: authority is not widened by legacy; migration folds forward once. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -187,6 +187,21 @@ impl RpcHandler {
|
|||||||
grants.set(category, granted);
|
grants.set(category, granted);
|
||||||
grants.save(self.data_dir()).await?;
|
grants.save(self.data_dir()).await?;
|
||||||
|
|
||||||
|
// Keep the legacy UI-store file in step — same downgrade-safety as
|
||||||
|
// `ai.permissions.set` — so no reader of either file sees a stale
|
||||||
|
// set, and a future `ai_grants_unified` migration (should the
|
||||||
|
// grants file ever be removed) cannot resurrect an old state.
|
||||||
|
let names: Vec<String> = grants
|
||||||
|
.categories()
|
||||||
|
.iter()
|
||||||
|
.filter_map(|c| serde_json::to_value(c).ok()?.as_str().map(str::to_string))
|
||||||
|
.collect();
|
||||||
|
let _ = crate::settings::ai_permissions::save(
|
||||||
|
self.data_dir(),
|
||||||
|
crate::settings::ai_permissions::AiPermissions { granted: names },
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
|
||||||
Ok(serde_json::json!({ "categories": grants_categories_json(&grants) }))
|
Ok(serde_json::json!({ "categories": grants_categories_json(&grants) }))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1076,24 +1076,48 @@ impl RpcHandler {
|
|||||||
|
|
||||||
/// The node's AI grants, as category id strings.
|
/// The node's AI grants, as category id strings.
|
||||||
///
|
///
|
||||||
/// Reads the assistant's grants file — the one that actually gates tools —
|
/// The assistant's grants file is the AUTHORITY — it is what gates the
|
||||||
/// and folds in anything still recorded only in the older
|
/// tools, so it alone answers whenever it exists. The older
|
||||||
/// settings/ai_permissions.json, so grants made before the two were
|
/// settings/ai_permissions.json is consulted only when no assistant
|
||||||
/// unified are not silently revoked on upgrade.
|
/// file exists yet (upgrade from pre-unification versions), and that
|
||||||
|
/// read migrates forward: the legacy set is persisted as assistant
|
||||||
|
/// grants, ending the union. A permanent union resurrected a revoked
|
||||||
|
/// category on every read whenever the two files drifted — observed
|
||||||
|
/// live on archi-dev-box, where legacy said all-ten and grants.json
|
||||||
|
/// said four, so the UI showed every toggle ON while the assistant
|
||||||
|
/// refused six of them.
|
||||||
async fn ai_grants_unified(data_dir: &std::path::Path) -> Vec<String> {
|
async fn ai_grants_unified(data_dir: &std::path::Path) -> Vec<String> {
|
||||||
let grants = crate::assistant::grants::Grants::load(data_dir).await;
|
if crate::assistant::grants::Grants::exists(data_dir).await {
|
||||||
let mut out: Vec<String> = grants
|
let grants = crate::assistant::grants::Grants::load(data_dir).await;
|
||||||
.categories()
|
let mut out: Vec<String> = grants
|
||||||
.iter()
|
.categories()
|
||||||
.filter_map(|c| serde_json::to_value(c).ok()?.as_str().map(str::to_string))
|
.iter()
|
||||||
.collect();
|
.filter_map(|c| serde_json::to_value(c).ok()?.as_str().map(str::to_string))
|
||||||
|
.collect();
|
||||||
|
out.sort();
|
||||||
|
return out;
|
||||||
|
}
|
||||||
|
|
||||||
|
// One-time upgrade fold: no assistant file yet — carry the legacy
|
||||||
|
// UI-store grants forward so pre-unification grants are not silently
|
||||||
|
// revoked, persist them as the authoritative file, and let the
|
||||||
|
// legacy file become inert (every set path rewrites it in step for
|
||||||
|
// downgrade-safety).
|
||||||
let legacy = crate::settings::ai_permissions::load(data_dir).await;
|
let legacy = crate::settings::ai_permissions::load(data_dir).await;
|
||||||
for name in legacy.granted {
|
if legacy.granted.is_empty() {
|
||||||
if !out.contains(&name) {
|
return Vec::new();
|
||||||
out.push(name);
|
}
|
||||||
|
let mut grants = crate::assistant::grants::Grants::default_closed();
|
||||||
|
let mut out: Vec<String> = Vec::new();
|
||||||
|
for name in &legacy.granted {
|
||||||
|
if let Ok(cat) = serde_json::from_value::<crate::assistant::PermissionCategory>(
|
||||||
|
serde_json::Value::String(name.clone()),
|
||||||
|
) {
|
||||||
|
grants.set(cat, true);
|
||||||
|
out.push(name.clone());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
let _ = grants.save(data_dir).await;
|
||||||
out.sort();
|
out.sort();
|
||||||
out.dedup();
|
out.dedup();
|
||||||
out
|
out
|
||||||
@@ -1607,3 +1631,58 @@ mod host_secrets_tests {
|
|||||||
assert_eq!(v["rotated_at"], "2026-08-02T12:04:00Z");
|
assert_eq!(v["rotated_at"], "2026-08-02T12:04:00Z");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod ai_grants_tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
/// The assistant grants file is the authority: when it exists, the
|
||||||
|
/// legacy UI-store file must NOT widen it. Regression test for the
|
||||||
|
/// live archi-dev-box desync — legacy said all-ten, grants.json said
|
||||||
|
/// four, and the union resurrected the six closed categories into the
|
||||||
|
/// UI on every read.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn existing_grants_file_outranks_legacy() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
std::fs::create_dir_all(dir.path().join("assistant")).unwrap();
|
||||||
|
std::fs::create_dir_all(dir.path().join("settings")).unwrap();
|
||||||
|
std::fs::write(
|
||||||
|
dir.path().join("assistant/grants.json"),
|
||||||
|
r#"{"categories":["apps"]}"#,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
std::fs::write(
|
||||||
|
dir.path().join("settings/ai_permissions.json"),
|
||||||
|
r#"{"granted":["apps","wallet"]}"#,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let got = RpcHandler::ai_grants_unified(dir.path()).await;
|
||||||
|
assert_eq!(got, vec!["apps".to_string()], "legacy file must not widen the authority");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// With no assistant grants file yet (pre-unification upgrade), the
|
||||||
|
/// legacy set folds forward and is persisted as the authoritative
|
||||||
|
/// file, so subsequent reads answer from it.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn missing_grants_file_migrates_legacy_forward() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
std::fs::create_dir_all(dir.path().join("settings")).unwrap();
|
||||||
|
std::fs::write(
|
||||||
|
dir.path().join("settings/ai_permissions.json"),
|
||||||
|
r#"{"granted":["apps","wallet"]}"#,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let got = RpcHandler::ai_grants_unified(dir.path()).await;
|
||||||
|
assert_eq!(got, vec!["apps".to_string(), "wallet".to_string()]);
|
||||||
|
assert!(
|
||||||
|
dir.path().join("assistant/grants.json").exists(),
|
||||||
|
"migration must persist the authoritative file"
|
||||||
|
);
|
||||||
|
// Second read answers from the migrated file even if legacy is gone.
|
||||||
|
std::fs::remove_file(dir.path().join("settings/ai_permissions.json")).unwrap();
|
||||||
|
let got2 = RpcHandler::ai_grants_unified(dir.path()).await;
|
||||||
|
assert_eq!(got2, vec!["apps".to_string(), "wallet".to_string()]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -60,6 +60,8 @@ impl Grants {
|
|||||||
/// data layer; `CallerScope::granted_categories` has no other source of
|
/// data layer; `CallerScope::granted_categories` has no other source of
|
||||||
/// authority to fall back to.
|
/// authority to fall back to.
|
||||||
pub async fn load(data_dir: &Path) -> Grants {
|
pub async fn load(data_dir: &Path) -> Grants {
|
||||||
|
|
||||||
|
|
||||||
let path = data_dir.join(GRANTS_FILE);
|
let path = data_dir.join(GRANTS_FILE);
|
||||||
let Ok(content) = tokio::fs::read_to_string(&path).await else {
|
let Ok(content) = tokio::fs::read_to_string(&path).await else {
|
||||||
return Grants::default_closed();
|
return Grants::default_closed();
|
||||||
@@ -67,6 +69,15 @@ impl Grants {
|
|||||||
serde_json::from_str(&content).unwrap_or_else(|_| Grants::default_closed())
|
serde_json::from_str(&content).unwrap_or_else(|_| Grants::default_closed())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Whether a grants file exists on disk at all. `load` cannot say this —
|
||||||
|
/// it maps "absent" and "present but empty" to the same value, and the
|
||||||
|
/// unified `ai.permissions.get` reader needs the distinction: an existing
|
||||||
|
/// file is authoritative, while an absent one triggers the one-time
|
||||||
|
/// legacy migration.
|
||||||
|
pub(crate) async fn exists(data_dir: &Path) -> bool {
|
||||||
|
tokio::fs::metadata(data_dir.join(GRANTS_FILE)).await.is_ok()
|
||||||
|
}
|
||||||
|
|
||||||
/// Persist the grants for this node, 0600 (following
|
/// Persist the grants for this node, 0600 (following
|
||||||
/// `streaming/session.rs`'s `data_dir`-scoped persisted-state
|
/// `streaming/session.rs`'s `data_dir`-scoped persisted-state
|
||||||
/// convention, and this codebase's convention of keeping
|
/// convention, and this codebase's convention of keeping
|
||||||
|
|||||||
Reference in New Issue
Block a user