fix(pi): close final ownership and native UX gaps

This commit is contained in:
SaladDay
2026-08-03 11:49:56 +00:00
parent 469d6e0f59
commit afd58e31d8
15 changed files with 364 additions and 48 deletions
+2 -1
View File
@@ -12,6 +12,7 @@ use crate::database::backup::BackupEntry;
use crate::database::Database;
use crate::error::AppError;
use crate::services::provider::ProviderService;
use crate::services::skill_deployment::PiSkillDeploymentService;
use crate::store::AppState;
// ─── File import/export ──────────────────────────────────────
@@ -57,7 +58,7 @@ pub async fn import_config_from_file(
let import_path = filePath.clone();
let import_result = tauri::async_runtime::spawn_blocking(move || {
db.import_portable_sql(&PathBuf::from(import_path))
PiSkillDeploymentService::import_portable_sql(&db, &PathBuf::from(import_path))
})
.await
.map_err(|error| AppError::Message(format!("SQL import task failed: {error}")))
+40 -13
View File
@@ -300,17 +300,18 @@ pub fn atomic_write(path: &Path, data: &[u8]) -> Result<(), AppError> {
/// Durable same-directory atomic replacement.
///
/// Existing permissions are preserved. `new_file_mode` controls only a newly
/// created Unix file (settings and other local secrets pass `0o600`). The
/// temporary file is created exclusively, synced before replacement, and the
/// containing directory is synced afterwards on Unix.
/// Existing permissions are preserved when `required_file_mode` is `None`.
/// Sensitive callers pass an explicit mode (for example `0o600`), which is
/// enforced for both new and existing Unix files before the replacement
/// becomes visible. The temporary file is created exclusively, synced before
/// replacement, and the containing directory is synced afterwards on Unix.
pub(crate) fn atomic_write_durable(
path: &Path,
data: &[u8],
new_file_mode: Option<u32>,
required_file_mode: Option<u32>,
) -> Result<(), AppError> {
#[cfg(not(unix))]
let _ = new_file_mode;
let _ = required_file_mode;
if let Some(parent) = path.parent() {
fs::create_dir_all(parent).map_err(|e| AppError::io(parent, e))?;
}
@@ -334,7 +335,7 @@ pub(crate) fn atomic_write_durable(
#[cfg(unix)]
{
use std::os::unix::fs::OpenOptionsExt;
options.mode(new_file_mode.unwrap_or(0o666));
options.mode(required_file_mode.unwrap_or(0o666));
}
let mut file = options
.open(&tmp)
@@ -342,18 +343,20 @@ pub(crate) fn atomic_write_durable(
file.write_all(data)
.map_err(|error| AppError::io(&tmp, error))?;
file.flush().map_err(|error| AppError::io(&tmp, error))?;
file.sync_all().map_err(|error| AppError::io(&tmp, error))?;
drop(file);
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
let mode = fs::metadata(path)
.map(|metadata| metadata.permissions().mode())
.unwrap_or_else(|_| new_file_mode.unwrap_or(0o666));
fs::set_permissions(&tmp, fs::Permissions::from_mode(mode))
let mode = required_file_mode.unwrap_or_else(|| {
fs::metadata(path)
.map(|metadata| metadata.permissions().mode())
.unwrap_or(0o666)
});
file.set_permissions(fs::Permissions::from_mode(mode))
.map_err(|error| AppError::io(&tmp, error))?;
}
file.sync_all().map_err(|error| AppError::io(&tmp, error))?;
drop(file);
replace_file_atomically(&tmp, path)?;
#[cfg(unix)]
@@ -560,6 +563,30 @@ mod tests {
);
}
#[cfg(unix)]
#[test]
fn sensitive_atomic_write_tightens_an_existing_file_before_publish() {
use std::os::unix::fs::PermissionsExt;
let temp = tempfile::tempdir().expect("tempdir");
let path = temp.path().join("settings.json");
fs::write(&path, b"old").expect("seed settings");
fs::set_permissions(&path, fs::Permissions::from_mode(0o644))
.expect("make legacy settings permissive");
atomic_write_durable(&path, b"new-secret", Some(0o600)).expect("replace settings");
assert_eq!(fs::read(&path).expect("read settings"), b"new-secret");
assert_eq!(
fs::metadata(&path)
.expect("settings metadata")
.permissions()
.mode()
& 0o777,
0o600
);
}
#[test]
fn sort_json_keys_produces_identical_output_for_different_insertion_orders() {
// 核心保证:同一逻辑配置无论键的插入顺序如何,写出的字节序列必须一致。
+52 -5
View File
@@ -285,21 +285,60 @@ fn validate_direct_instruction_content(content: &str) -> Result<(), AppError> {
}
fn validate_template_slug(slug: &str) -> Result<(), AppError> {
// Pinned Pi's request-capture discovers `release notes.md`, but
// expandPromptTemplate("/release notes", ...) leaves the command
// unchanged because slash-command names are one token. Keep the managed
// namespace both callable and portable across Unix and Windows.
let windows_basename = slug
.split_once('.')
.map_or(slug, |(basename, _extension)| basename);
let windows_basename = windows_basename.to_ascii_lowercase();
let windows_reserved = matches!(
windows_basename.as_str(),
"con"
| "prn"
| "aux"
| "nul"
| "com1"
| "com2"
| "com3"
| "com4"
| "com5"
| "com6"
| "com7"
| "com8"
| "com9"
| "lpt1"
| "lpt2"
| "lpt3"
| "lpt4"
| "lpt5"
| "lpt6"
| "lpt7"
| "lpt8"
| "lpt9"
);
let valid = !slug.is_empty()
&& slug.len() <= MAX_TEMPLATE_SLUG_BYTES
&& slug != "."
&& slug != ".."
&& slug.trim() == slug
&& !slug.starts_with('.')
&& !slug.ends_with('.')
&& !slug
.chars()
.any(|character| character.is_control() || matches!(character, '/' | '\\'));
&& !windows_reserved
&& !slug.chars().any(|character| {
character.is_control()
|| character.is_whitespace()
|| matches!(
character,
'<' | '>' | ':' | '"' | '/' | '\\' | '|' | '?' | '*'
)
});
if valid {
Ok(())
} else {
Err(AppError::InvalidInput(
"Pi prompt-template slug must be one visible filename (1-128 UTF-8 bytes)".to_string(),
"Pi prompt-template slug must be one portable slash-command token (1-128 UTF-8 bytes)"
.to_string(),
))
}
}
@@ -408,8 +447,16 @@ mod tests {
".hidden",
"trailing.",
" padded",
"internal space",
"tab\tname",
"a/b",
r"a\b",
"bad:name",
"bad*name",
"CON",
"con.anything",
"LPT9",
"nul.json",
] {
assert!(validate_template_slug(slug).is_err(), "{slug:?}");
}
@@ -141,6 +141,23 @@ impl PiSkillDeploymentService {
deployment_lock()
}
/// Serialize a portable database/SSOT replacement with every Pi Skill
/// deployment mutation. Callers already hold Pi's switch boundary, so the
/// global lock order remains `Pi switch -> Skill deployment`.
pub(crate) fn coordinate_portable_import<T>(
operation: impl FnOnce() -> Result<T, AppError>,
) -> Result<T, AppError> {
let _guard = Self::operation_guard();
operation()
}
pub(crate) fn import_portable_sql(
db: &Database,
source_path: &Path,
) -> Result<String, AppError> {
Self::coordinate_portable_import(|| db.import_portable_sql(source_path))
}
pub(crate) fn reconcile_skill_under_guard(
_guard: &MutexGuard<'static, ()>,
db: &Arc<Database>,
@@ -1126,6 +1143,44 @@ mod tests {
use super::*;
use crate::app_config::SkillApps;
#[test]
#[serial_test::serial]
fn portable_import_waits_for_the_skill_ownership_boundary() {
use std::sync::mpsc;
use std::time::Duration;
let db = Database::memory().expect("database");
let missing = tempfile::tempdir()
.expect("tempdir")
.path()
.join("missing.sql");
let guard = PiSkillDeploymentService::operation_guard();
let (ready_tx, ready_rx) = mpsc::channel();
let (result_tx, result_rx) = mpsc::channel();
let worker = std::thread::spawn(move || {
ready_tx.send(()).expect("signal worker ready");
let result = PiSkillDeploymentService::import_portable_sql(&db, &missing);
result_tx.send(result).expect("signal import result");
});
ready_rx
.recv_timeout(Duration::from_secs(2))
.expect("worker reaches import entry");
assert!(
result_rx.recv_timeout(Duration::from_millis(100)).is_err(),
"portable import must not pass capture/publish while a Skill mutation owns the boundary"
);
drop(guard);
let result = result_rx
.recv_timeout(Duration::from_secs(2))
.expect("import proceeds after ownership boundary release");
assert!(
result.is_err(),
"the intentionally missing import must fail"
);
worker.join().expect("worker");
}
#[test]
fn digest_includes_hidden_files_and_rejects_symlinks() {
let temp = tempfile::tempdir().expect("tempdir");
+41
View File
@@ -310,6 +310,16 @@ pub(crate) fn apply_snapshot(
db: &crate::database::Database,
db_sql: &[u8],
skills_zip: &[u8],
) -> Result<(), AppError> {
crate::services::skill_deployment::PiSkillDeploymentService::coordinate_portable_import(|| {
apply_snapshot_under_pi_skill_guard(db, db_sql, skills_zip)
})
}
fn apply_snapshot_under_pi_skill_guard(
db: &crate::database::Database,
db_sql: &[u8],
skills_zip: &[u8],
) -> Result<(), AppError> {
let sql_str = std::str::from_utf8(db_sql).map_err(|e| {
localized(
@@ -419,6 +429,37 @@ where
mod tests {
use super::*;
#[test]
#[serial_test::serial]
fn snapshot_application_waits_for_the_pi_skill_ownership_boundary() {
use std::sync::mpsc;
use std::time::Duration;
let db = crate::database::Database::memory().expect("database");
let guard = crate::services::skill_deployment::PiSkillDeploymentService::operation_guard();
let (ready_tx, ready_rx) = mpsc::channel();
let (result_tx, result_rx) = mpsc::channel();
let worker = std::thread::spawn(move || {
ready_tx.send(()).expect("signal worker ready");
let result = apply_snapshot(&db, &[0xff], &[]);
result_tx.send(result).expect("signal snapshot result");
});
ready_rx
.recv_timeout(Duration::from_secs(2))
.expect("worker reaches snapshot entry");
assert!(
result_rx.recv_timeout(Duration::from_millis(100)).is_err(),
"WebDAV/S3 snapshot application must wait for a concurrent Pi Skill mutation"
);
drop(guard);
let result = result_rx
.recv_timeout(Duration::from_secs(2))
.expect("snapshot proceeds after ownership boundary release");
assert!(result.is_err(), "the intentionally invalid SQL must fail");
worker.join().expect("worker");
}
fn artifact(sha256: &str, size: u64) -> ArtifactMeta {
ArtifactMeta {
sha256: sha256.to_string(),
+49
View File
@@ -1373,6 +1373,55 @@ mod tests {
use super::*;
use crate::app_config::AppType;
#[cfg(unix)]
#[test]
#[serial_test::serial]
fn saving_a_gateway_token_tightens_legacy_settings_permissions() {
use std::os::unix::fs::PermissionsExt;
struct EnvGuard(Option<std::ffi::OsString>);
impl Drop for EnvGuard {
fn drop(&mut self) {
match self.0.take() {
Some(value) => std::env::set_var("CC_SWITCH_TEST_HOME", value),
None => std::env::remove_var("CC_SWITCH_TEST_HOME"),
}
}
}
let temp = tempfile::tempdir().expect("tempdir");
let _home = EnvGuard(std::env::var_os("CC_SWITCH_TEST_HOME"));
std::env::set_var("CC_SWITCH_TEST_HOME", temp.path());
let path = AppSettings::settings_path().expect("settings path");
fs::create_dir_all(path.parent().expect("settings parent")).expect("settings directory");
fs::write(&path, b"{}").expect("legacy settings");
fs::set_permissions(&path, fs::Permissions::from_mode(0o644))
.expect("make legacy settings permissive");
let token = GatewayToken::generate();
let expected = token.expose().to_string();
let settings = AppSettings {
pi_gateway_token: Some(token),
..AppSettings::default()
};
save_settings_file(&settings).expect("save sensitive settings");
assert_eq!(
fs::metadata(&path)
.expect("settings metadata")
.permissions()
.mode()
& 0o777,
0o600
);
assert!(
fs::read_to_string(&path)
.expect("settings body")
.contains(&expected),
"the permission assertion must exercise a persisted Pi gateway token"
);
}
#[test]
fn visible_apps_old_settings_default_claude_desktop_visible() {
let visible: VisibleApps = serde_json::from_value(serde_json::json!({