mosaic-sync-skills: unguarded ownership prefix collapses to / and can rm -f foreign symlinks #1087

Open
opened 2026-08-06 21:47:40 +00:00 by Mos · 0 comments
Contributor

Summary

tools/_scripts/mosaic-sync-skills decides whether a symlink is mosaic-owned by prefix-matching against $canonical_real / $local_real. Neither is length-checked. If either resolves empty, the comparison collapses to == "/"*, which matches every absolute path — and at one of the two sites the consequence is rm -f.

Latent, not live: on a normal install both prefixes resolve. Reporting it because the fix is one line on a destructive path, and the failure mode is "deletes symlinks it does not own."

The code (main, unchanged from the deployed copy — both 9183 B)

:163  canonical_real="$(readlink -f "$MOSAIC_SKILLS_DIR")"
:164  local_real="$(readlink -f "$MOSAIC_LOCAL_SKILLS_DIR")"

Site A — overwrite:

:210  if [[ "$resolved_target" == "$canonical_real/"* || "$resolved_target" == "$local_real/"* ]]; then
:211    ln -sfn "$skill_path" "$link_path"

Site B — delete:

:248  if [[ -n "$resolved" && "$resolved" == "$canonical_real/"* ]]; then
:249    rm -f "$link_path"

Note :248 guards $resolved for emptiness but not $canonical_real — the guard is on the wrong operand. :210 guards neither.

Reproduction of the collapse

canonical_real=""; resolved="/home/user/.claude/skills/some-foreign-link"
[[ -n "$resolved" && "$resolved" == "$canonical_real/"* ]] && echo MATCH
# -> MATCH        ⇒ rm -f fires on a link mosaic does not own

canonical_real="/home/user/.config/mosaic/skills"
[[ "$resolved" == "$canonical_real/"* ]] && echo MATCH || echo "correctly rejected"
# -> correctly rejected     (control: with a real prefix the logic is right)

When can the prefix be empty?

readlink -f returns empty only when a parent component is missing — not the leaf:

/home/user/.config/mosaic/skills        -> resolves
/home/user/.config/mosaic/MISSING       -> resolves (leaf may be absent)
/home/user/.config/MISSINGPARENT/skills -> EMPTY      <- the trigger

So it needs $MOSAIC_HOME to point somewhere whose parent chain is incomplete — a custom MOSAIC_HOME, a first run against a not-yet-created tree, or a test harness with a shadow home. Narrow, but it is a "one missing directory away" condition rather than an impossible one, and the blast radius is every symlink in the target dirs.

Suggested fix

Length-check both prefixes before either comparison, and fail closed (preserve, don't touch) if resolution failed:

if [[ -z "$canonical_real" || -z "$local_real" ]]; then
    echo "[mosaic-skills] ownership prefixes unresolved -- preserving all links" >&2
    return
fi

This matches the intent already documented at :245-246: "If resolution fails, ownership is unproven and the link must be preserved." That comment is correct — it is applied to $resolved and not to the prefix it is compared against.

Acceptance

  • Force an empty prefix and assert no link is removed and no link is overwritten.
  • Positive control: with real prefixes, an owned stale link is still removed (the fix must not disable the feature).

Credit

The defect class was identified by fred on sb-it-1-dt, in their own cleanup code, where the same shape would have deleted foreign symlinks across four operator-owned installs. This issue reports that the shipped framework carries the identical shape, unguarded, in main.

No closing keywords intended; none used.

## Summary `tools/_scripts/mosaic-sync-skills` decides whether a symlink is mosaic-owned by prefix-matching against `$canonical_real` / `$local_real`. Neither is length-checked. **If either resolves empty, the comparison collapses to `== "/"*`, which matches every absolute path** — and at one of the two sites the consequence is `rm -f`. Latent, not live: on a normal install both prefixes resolve. Reporting it because the fix is one line on a destructive path, and the failure mode is "deletes symlinks it does not own." ## The code (`main`, unchanged from the deployed copy — both 9183 B) ```bash :163 canonical_real="$(readlink -f "$MOSAIC_SKILLS_DIR")" :164 local_real="$(readlink -f "$MOSAIC_LOCAL_SKILLS_DIR")" ``` Site A — overwrite: ```bash :210 if [[ "$resolved_target" == "$canonical_real/"* || "$resolved_target" == "$local_real/"* ]]; then :211 ln -sfn "$skill_path" "$link_path" ``` Site B — **delete**: ```bash :248 if [[ -n "$resolved" && "$resolved" == "$canonical_real/"* ]]; then :249 rm -f "$link_path" ``` Note `:248` guards `$resolved` for emptiness but **not `$canonical_real`** — the guard is on the wrong operand. `:210` guards neither. ## Reproduction of the collapse ```bash canonical_real=""; resolved="/home/user/.claude/skills/some-foreign-link" [[ -n "$resolved" && "$resolved" == "$canonical_real/"* ]] && echo MATCH # -> MATCH ⇒ rm -f fires on a link mosaic does not own canonical_real="/home/user/.config/mosaic/skills" [[ "$resolved" == "$canonical_real/"* ]] && echo MATCH || echo "correctly rejected" # -> correctly rejected (control: with a real prefix the logic is right) ``` ## When can the prefix be empty? `readlink -f` returns empty only when a **parent** component is missing — not the leaf: ``` /home/user/.config/mosaic/skills -> resolves /home/user/.config/mosaic/MISSING -> resolves (leaf may be absent) /home/user/.config/MISSINGPARENT/skills -> EMPTY <- the trigger ``` So it needs `$MOSAIC_HOME` to point somewhere whose parent chain is incomplete — a custom `MOSAIC_HOME`, a first run against a not-yet-created tree, or a test harness with a shadow home. Narrow, but it is a "one missing directory away" condition rather than an impossible one, and the blast radius is every symlink in the target dirs. ## Suggested fix Length-check both prefixes before either comparison, and fail closed (preserve, don't touch) if resolution failed: ```bash if [[ -z "$canonical_real" || -z "$local_real" ]]; then echo "[mosaic-skills] ownership prefixes unresolved -- preserving all links" >&2 return fi ``` This matches the intent already documented at `:245-246`: *"If resolution fails, ownership is unproven and the link must be preserved."* That comment is correct — it is applied to `$resolved` and not to the prefix it is compared against. ## Acceptance - Force an empty prefix and assert **no** link is removed and **no** link is overwritten. - Positive control: with real prefixes, an owned stale link is still removed (the fix must not disable the feature). ## Credit The defect class was identified by `fred` on sb-it-1-dt, in their own cleanup code, where the same shape would have deleted foreign symlinks across four operator-owned installs. This issue reports that the **shipped** framework carries the identical shape, unguarded, in `main`. No closing keywords intended; none used.
Mos added the bug label 2026-08-06 21:47:40 +00:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#1087