From 7fb2190211b935b366f6b0498bd3d7643d2bac08 Mon Sep 17 00:00:00 2001
From: Josh Knapp <jknapp85@gmail.com>
Date: Sun, 27 Sep 2026 09:27:10 -0700
Subject: [PATCH] Marketplace sync script: review fixes (round 2)

Removal only derives a path from an exact <kind>:<key> state id with a known
kind; any other record is dropped with an error and nothing is deleted
(an id like "skill" used to remove ~/.claude/skills/skill). Invalid plugin
records are dropped too, and a failed chmod 600 on settings.json is reported.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 app/src-tauri/src/marketplace/sync.sh         | 22 ++++++++++---
 .../src/marketplace/sync_script_tests.rs      | 33 +++++++++++++++++++
 2 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/app/src-tauri/src/marketplace/sync.sh b/app/src-tauri/src/marketplace/sync.sh
index 6f2050e..381419d 100644
--- a/app/src-tauri/src/marketplace/sync.sh
+++ b/app/src-tauri/src/marketplace/sync.sh
@@ -271,9 +271,16 @@ while read -r id; do
   if grep -qxF "$id" "$R/new_ids"; then continue; fi
   # Still selected but failed this run: keep the old files and record.
   if in_manifest "$id"; then carry_forward "$id"; continue; fi
-  kind=${id%%:*}
-  key=${id#*:}
-  if ! valid_key "$key" || ! path=$(item_path "$kind" "$key"); then
+  # Only an exact "<kind>:<key>" with a known kind names a path; anything
+  # else in state is dropped without deleting anything.
+  case "$id" in
+    agent:* | skill:* | command:* | hook:*)
+      kind=${id%%:*}
+      key=${id#*:}
+      ;;
+    *) kind="" key="" ;;
+  esac
+  if [ -z "$kind" ] || ! valid_key "$key" || ! path=$(item_path "$kind" "$key"); then
     fail "$id: dropped an unrecognised record from the marketplace state"
     continue
   fi
@@ -330,7 +337,8 @@ if [ "$OLD_HOOKS" != "{}" ] || [ "$NEW_HOOKS" != "{}" ]; then
       ' "$R/current.json" >"$tmp" 2>/dev/null &&
       jq -e 'type == "object"' "$tmp" >/dev/null 2>&1 &&
       mv -f "$tmp" "$target"; then
-      chmod 600 "$target"
+      chmod 600 "$target" ||
+        fail "~/.claude/settings.json was updated but could not be made private (chmod 600)"
     else
       rm -f "$tmp"
       HOOKS_FAILED=1
@@ -389,7 +397,11 @@ while read -r id; do
   if in_manifest "$id"; then carry_forward "$id"; continue; fi
   key=${id#plugin:}
   slug=$(jq -r --arg id "$id" '.items[$id].slug // ""' "$STATE")
-  if valid_key "$key" && valid_slug "$slug" && claude_cmd plugin uninstall "$key@triple-c-$slug"; then
+  if ! valid_key "$key" || ! valid_slug "$slug"; then
+    fail "$id: dropped an unrecognised record from the marketplace state"
+    continue
+  fi
+  if claude_cmd plugin uninstall "$key@triple-c-$slug"; then
     report removed "$id"
   else
     fail "$id: uninstall failed"
diff --git a/app/src-tauri/src/marketplace/sync_script_tests.rs b/app/src-tauri/src/marketplace/sync_script_tests.rs
index 72968ea..a0606af 100644
--- a/app/src-tauri/src/marketplace/sync_script_tests.rs
+++ b/app/src-tauri/src/marketplace/sync_script_tests.rs
@@ -851,3 +851,36 @@ fn sync_settings_move_failure_is_reported_and_not_recorded() {
         "{state}"
     );
 }
+
+#[test]
+fn sync_drops_state_records_without_a_kind_key_id() {
+    let Some(env) = env() else { return };
+    install_all(&env);
+    let claude = env.home.join(".claude");
+    let mine = [
+        claude.join("skills/skill/SKILL.md"),
+        claude.join("agents/agent.md"),
+    ];
+    for p in &mine {
+        fs::create_dir_all(p.parent().unwrap()).unwrap();
+        fs::write(p, "mine\n").unwrap();
+    }
+    let state_path = claude.join("triple-c/marketplace/state.json");
+    let mut state = read_json(&state_path);
+    for bogus in ["skill", "agent", "widget:x", "agent:a:b", "plugin:a:b"] {
+        state["items"][bogus] = json!({ "commit": C1, "path": "/" });
+    }
+    fs::write(&state_path, state.to_string()).unwrap();
+
+    payload(&env, &[], empty_manifest());
+    let r = run(&env);
+
+    assert_eq!(sorted(r.removed.clone()).len(), 4, "{r:?}");
+    assert_eq!(r.errors.len(), 5, "{r:?}");
+    assert!(claude_log(&env).is_empty(), "{:?}", claude_log(&env));
+    for p in &mine {
+        assert_eq!(fs::read_to_string(p).unwrap(), "mine\n", "{}", p.display());
+    }
+    let state = read_json(&state_path);
+    assert_eq!(state["items"], json!({}), "bogus records dropped");
+}