fix(gui): reject remove/reorder when settings.json drifted externally

remove/reorder targeted userRepos entries by array position only. An
external write to settings.json (another ALCOM instance, VCC, vrc-get
CLI, manual edit) between the frontend's last fetch and the call
could silently delete or reorder the wrong repo.

The frontend already has the expected id (part of TauriUserRepository),
so send it alongside the index and have the backend verify the repo at
that index still has the expected id before mutating. On mismatch,
reject with an `unrecoverable_str` error; the frontend's existing
onError + onSettled refetch rolls back optimistic state and shows the
on-disk truth.

- environment_remove_repository takes (index, expected_id)
- environment_reorder_repositories takes Vec<TauriUserRepositoryRef>;
  all pairs verified up front, no partial reorder on mismatch
- verify_repo_at_index helper centralises the lookup + comparison
This commit is contained in:
nekochanfood 2026-05-30 20:50:24 +09:00
commit f6e677909c
3 changed files with 63 additions and 14 deletions

View file

@ -291,10 +291,11 @@ function PageBody() {
const queryClient = useQueryClient();
const reorderMutation = useMutation({
mutationFn: (listIds: string[]) => {
const indices = listIds
.map((lid) => userRepoByListId.get(lid)?.index)
.filter((i): i is number => i !== undefined);
return commands.environmentReorderRepositories(indices);
const repos = listIds
.map((lid) => userRepoByListId.get(lid))
.filter((r): r is UserRepoWithListId => r !== undefined)
.map((r) => ({ index: r.index, id: r.id }));
return commands.environmentReorderRepositories(repos);
},
// Pin listIds to the new positions so duplicate-keyed rows don't swap their listIds on refetch.
onMutate: (newListIds: string[]) => {
@ -760,6 +761,7 @@ function RepositoryRow({
void openSingleDialog(RemoveRepositoryDialog, {
displayName,
index: repoIndex ?? 0,
id: repoId,
})
}
dragListeners={listeners}
@ -824,17 +826,19 @@ function RemoveRepositoryDialog({
dialog,
displayName,
index,
id,
}: {
dialog: DialogContext<void>;
displayName: string;
index: number;
id: string;
}) {
const queryClient = useQueryClient();
const removeRepository = useMutation({
mutationFn: async (index: number) =>
await commands.environmentRemoveRepository(index),
onMutate: async (index) => {
mutationFn: async (args: { index: number; id: string }) =>
await commands.environmentRemoveRepository(args.index, args.id),
onMutate: async ({ index }) => {
await queryClient.cancelQueries(environmentRepositoriesInfo);
const data = queryClient.getQueryData(
environmentRepositoriesInfo.queryKey,
@ -849,7 +853,7 @@ function RemoveRepositoryDialog({
}
return data;
},
onError: (e, _index, ctx) => {
onError: (e, _args, ctx) => {
queryClient.setQueryData(environmentRepositoriesInfo.queryKey, ctx);
toastThrownError(e);
},
@ -873,7 +877,7 @@ function RemoveRepositoryDialog({
<Button
onClick={() => {
dialog.close();
removeRepository.mutate(index);
removeRepository.mutate({ index, id });
}}
className={"ml-2"}
>

View file

@ -40,8 +40,8 @@ export const commands = {
environmentSetHideLocalUserPackages: (value: boolean) => __TAURI_INVOKE<null>("environment_set_hide_local_user_packages", { value }),
environmentDownloadRepository: (url: string, headers: { [key in string]: string }) => __TAURI_INVOKE<TauriDownloadRepository>("environment_download_repository", { url, headers }),
environmentAddRepository: (url: string, headers: { [key in string]: string }) => __TAURI_INVOKE<TauriAddRepositoryResult>("environment_add_repository", { url, headers }),
environmentRemoveRepository: (index: number) => __TAURI_INVOKE<null>("environment_remove_repository", { index }),
environmentReorderRepositories: (indices: number[]) => __TAURI_INVOKE<null>("environment_reorder_repositories", { indices }),
environmentRemoveRepository: (index: number, expectedId: string) => __TAURI_INVOKE<null>("environment_remove_repository", { index, expectedId }),
environmentReorderRepositories: (repos: TauriUserRepositoryRef[]) => __TAURI_INVOKE<null>("environment_reorder_repositories", { repos }),
environmentImportRepositoryPick: () => __TAURI_INVOKE<TauriImportRepositoryPickResult>("environment_import_repository_pick"),
environmentImportDownloadRepositories: (channel: string, repositories: TauriRepositoryDescriptor[]) => __TAURI_INVOKE<AsyncCallResult<number, ([TauriRepositoryDescriptor, TauriDownloadRepository])[]>>("environment_import_download_repositories", { channel, repositories }),
environmentImportAddRepositories: (repositories: TauriRepositoryDescriptor[]) => __TAURI_INVOKE<null>("environment_import_add_repositories", { repositories }),
@ -411,6 +411,11 @@ export type TauriUserRepository = {
display_name: string,
};
export type TauriUserRepositoryRef = {
index: number,
id: string,
};
export type TauriVersion = {
major: number,
minor: number,

View file

@ -18,7 +18,7 @@ use vrc_get_vpm::environment::{
use vrc_get_vpm::io::{DefaultEnvironmentIo, IoTrait};
use vrc_get_vpm::repositories_file::RepositoriesFile;
use vrc_get_vpm::repository::RemoteRepository;
use vrc_get_vpm::{HttpClient, VersionSelector};
use vrc_get_vpm::{HttpClient, UserRepoSetting, VersionSelector};
#[tauri::command]
#[specta::specta]
@ -348,6 +348,30 @@ pub async fn environment_add_repository(
Ok(TauriAddRepositoryResult::Success)
}
// Verifies that the repo at `index` in the freshly-loaded settings still has
// the `expected_id` the frontend last saw. Guards against silent corruption
// from external writes to settings.json between fetch and mutation.
fn verify_repo_at_index(
repos: &[UserRepoSetting],
index: usize,
expected_id: &str,
) -> Result<(), RustError> {
let Some(repo) = repos.get(index) else {
return Err(RustError::unrecoverable_str(format!(
"Repository index {index} out of range (expected id {expected_id}). \
settings.json was likely modified externally; please refresh."
)));
};
let actual = repo.id().or(repo.url().map(Url::as_str));
if actual != Some(expected_id) {
return Err(RustError::unrecoverable_str(format!(
"Repository at index {index} changed (expected id {expected_id}, found {actual:?}). \
settings.json was likely modified externally; please refresh."
)));
}
Ok(())
}
#[tauri::command]
#[specta::specta]
pub async fn environment_remove_repository(
@ -355,9 +379,12 @@ pub async fn environment_remove_repository(
packages: State<'_, PackagesState>,
io: State<'_, DefaultEnvironmentIo>,
index: usize,
expected_id: String,
) -> Result<(), RustError> {
let mut settings = settings.load_mut(io.inner()).await?;
verify_repo_at_index(settings.get_user_repos(), index, &expected_id)?;
let removed = settings.remove_repo_at_index(index);
if let Some(repo) = &removed {
@ -390,16 +417,29 @@ pub struct TauriRepositoryDescriptor {
pub headers: Headers,
}
#[derive(Deserialize, specta::Type)]
pub struct TauriUserRepositoryRef {
pub index: usize,
pub id: String,
}
#[tauri::command]
#[specta::specta]
pub async fn environment_reorder_repositories(
settings: State<'_, SettingsState>,
packages: State<'_, PackagesState>,
io: State<'_, DefaultEnvironmentIo>,
indices: Vec<usize>,
repos: Vec<TauriUserRepositoryRef>,
) -> Result<(), RustError> {
let mut settings = settings.load_mut(io.inner()).await?;
log::debug!("reorder user repositories by indices: {:?}", indices);
log::debug!("reorder user repositories: {} entries", repos.len());
let user_repos = settings.get_user_repos();
for r in &repos {
verify_repo_at_index(user_repos, r.index, &r.id)?;
}
let indices: Vec<usize> = repos.into_iter().map(|r| r.index).collect();
settings.reorder_user_repos_by_indices(&indices);
settings.save().await?;
packages.clear_cache();