github/dingtalk-workspace-cli
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(skills): create shell-published links at the destination atomically

Browse Source 
The POSIX shell installers staged shared Skill links and published them
with mv after an existence check; a file or symlink another process
created at the destination between the check and the move was silently
replaced, and the inode confirmation could not detect the loss. Publish
by creating each link directly at its destination instead — symlink(2)
refuses an occupied path with EEXIST, so the creation itself is the
atomic no-replace check. A directory that appears at the destination
turns ln -s into a container; the nested link is removed after an
identity check and the transaction rolls back, leaving the foreign
directory untouched. Applied to install.sh, install-skills.sh,
install-event.sh, and install-devapp.sh. Also covers the remaining
retraction branches of the Go shell-removal fallback so changed-code
coverage is complete. Regression tests inject a concurrent occupant at
the publish instant for regular-file and directory cases and assert the
foreign object and its contents stay completely unchanged.
This commit is contained in:
玉澜
2026-08-18 10:20:15 +08:00
parent e84743615f
commit fcbddb0904
8 changed files with 306 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files
scripts/install-devapp.sh
+19 -3
View File
@@ -244,10 +244,26 @@ link_or_copy_skill() {
return 1
fi
backup="$(backup_skill_dir "$dest")" || { rm -rf "$stage"; return 1; }
if mv "$stage/skill" "$dest"; then rm -rf "$stage"; return 0; fi
# Create the link directly at the destination: symlink(2) refuses an
# occupied path with EEXIST, so publication itself is the atomic no-replace
# check (`mv` after the backup could still replace a file or symlink a
# concurrent writer created in between). A directory that slipped in turns
# `ln -s` into a container: remove exactly our nested link and roll back.
if ln -s "$relative" "$dest" 2>/dev/null && [ -L "$dest" ]; then
rm -rf "$stage"
return 0
fi
nested="$dest/${relative##*/}"
if [ -L "$nested" ] && [ "$(readlink "$nested" 2>/dev/null)" = "$relative" ]; then
rm -f "$nested" || true
fi
rm -rf "$stage"
if [ -n "$backup" ] && ! mv "$backup" "$dest"; then
printf ' ❌ Skill rollback failed; backup retained at %s\n' "$backup" >&2
if [ -n "$backup" ]; then
if [ -e "$dest" ] || [ -L "$dest" ]; then
printf ' ⚠️ %s 在发布期间被并发写入占用，原 Skill 保留于备份: %s\n' "$dest" "$backup" >&2
elif ! mv "$backup" "$dest"; then
printf ' ❌ Skill rollback failed; backup retained at %s\n' "$backup" >&2
fi
fi
return 1
}
scripts/install-event.sh
+19 -3
View File
@@ -313,10 +313,26 @@ link_or_copy_skill() {
return 1
fi
backup="$(backup_skill_dir "$dest")" || { rm -rf "$stage"; return 1; }
if mv "$stage/skill" "$dest"; then rm -rf "$stage"; return 0; fi
# Create the link directly at the destination: symlink(2) refuses an
# occupied path with EEXIST, so publication itself is the atomic no-replace
# check (`mv` after the backup could still replace a file or symlink a
# concurrent writer created in between). A directory that slipped in turns
# `ln -s` into a container: remove exactly our nested link and roll back.
if ln -s "$relative" "$dest" 2>/dev/null && [ -L "$dest" ]; then
rm -rf "$stage"
return 0
fi
nested="$dest/${relative##*/}"
if [ -L "$nested" ] && [ "$(readlink "$nested" 2>/dev/null)" = "$relative" ]; then
rm -f "$nested" || true
fi
rm -rf "$stage"
if [ -n "$backup" ] && ! mv "$backup" "$dest"; then
printf ' ❌ Skill rollback failed; backup retained at %s\n' "$backup" >&2
if [ -n "$backup" ]; then
if [ -e "$dest" ] || [ -L "$dest" ]; then
printf ' ⚠️ %s 在发布期间被并发写入占用，原 Skill 保留于备份: %s\n' "$dest" "$backup" >&2
elif ! mv "$backup" "$dest"; then
printf ' ❌ Skill rollback failed; backup retained at %s\n' "$backup" >&2
fi
fi
return 1
}
scripts/install-skills.sh
+21 -17
View File
@@ -720,25 +720,29 @@ link_canonical_skills_to_base() {
_lcs_link_target="$(readlink "$_lcs_staged" 2>/dev/null)" || {
restore_linked_skill_set "$_lcs_published" "$_lcs_backups" || true; rm -rf "$_lcs_stage"; return 1
}
# The staged symlink keeps its inode across the rename below (the staging
# directory is a sibling inside the same Agent root), so the identity
# recorded in the publication manifest stays exact.
_lcs_inode="$(skill_link_inode "$_lcs_staged")" || {
restore_linked_skill_set "$_lcs_published" "$_lcs_backups" || true; rm -rf "$_lcs_stage"; return 1
}
# Anything still present here appeared after the pre-flight scan, i.e. a
# concurrent writer. Never let `mv` clobber it.
if [ -e "$_lcs_dest" ] || [ -L "$_lcs_dest" ]; then
# Publish by creating the link directly at the destination: symlink(2)
# refuses an occupied path with EEXIST, so the creation itself is the
# atomic no-replace check. `mv` after any pre-check could still replace a
# file or symlink a concurrent writer created in between, and `ln -P`
# stays unusable (absent from BusyBox `ln`, which silently degraded every
# non-universal Agent to the copy layout on Alpine and most containers).
if ! ln -s "$_lcs_link_target" "$_lcs_dest" 2>/dev/null; then
restore_linked_skill_set "$_lcs_published" "$_lcs_backups" || true; rm -rf "$_lcs_stage"; return 1
fi
# Publish by moving the staged symlink. `mv` is POSIX; `ln -P` is not (it is
# absent from BusyBox `ln`, which silently degraded every non-universal
# Agent to the copy layout on Alpine and most containers). install-event.sh
# and install-devapp.sh already publish this way. The staged basename stays
# unique, so a destination a concurrent process turned into a directory can
# only receive our link and post-validation removes exactly that.
if ! mv "$_lcs_staged" "$_lcs_dest" 2>/dev/null || ! skill_link_matches "$_lcs_dest" "$_lcs_link_target" "$_lcs_inode"; then
cleanup_nested_staged_link "$_lcs_dest" "$_lcs_stage_name" "$_lcs_link_target" "$_lcs_inode" || true
# A directory that appeared at the destination turns `ln -s` into a
# container: the link lands inside it under the target basename. Remove
# exactly our link after an identity check and roll back; the foreign
# directory stays untouched.
if [ ! -L "$_lcs_dest" ]; then
_lcs_nested_name="${_lcs_link_target##*/}"
_lcs_nested_inode="$(skill_link_inode "$_lcs_dest/$_lcs_nested_name" 2>/dev/null)" || _lcs_nested_inode=""
cleanup_nested_staged_link "$_lcs_dest" "$_lcs_nested_name" "$_lcs_link_target" "$_lcs_nested_inode" || true
restore_linked_skill_set "$_lcs_published" "$_lcs_backups" || true; rm -rf "$_lcs_stage"; return 1
fi
_lcs_inode="$(skill_link_inode "$_lcs_dest")" || {
restore_linked_skill_set "$_lcs_published" "$_lcs_backups" || true; rm -rf "$_lcs_stage"; return 1
}
if ! skill_link_matches "$_lcs_dest" "$_lcs_link_target" "$_lcs_inode"; then
restore_linked_skill_set "$_lcs_published" "$_lcs_backups" || true; rm -rf "$_lcs_stage"; return 1
fi
if ! printf '%s\n%s\n%s\n' "$_lcs_dest" "$_lcs_link_target" "$_lcs_inode" >> "$_lcs_published"; then
scripts/install.sh
+25 -19
View File
@@ -961,29 +961,35 @@ link_canonical_skills_to_base() {
rm -rf "$_lcs_stage"
return 1
}
# The staged symlink keeps its inode across the rename below (the staging
# directory is a sibling inside the same Agent root), so the identity
# recorded in the publication manifest stays exact.
_lcs_inode="$(skill_link_inode "$_lcs_staged")" || {
restore_linked_skill_set "$_lcs_published" "$_lcs_backups" || true
rm -rf "$_lcs_stage"
return 1
}
# Anything still present here appeared after the pre-flight scan, i.e. a
# concurrent writer. Never let `mv` clobber it.
if [ -e "$_lcs_dest" ] || [ -L "$_lcs_dest" ]; then
# Publish by creating the link directly at the destination: symlink(2)
# refuses an occupied path with EEXIST, so the creation itself is the
# atomic no-replace check. `mv` after any pre-check could still replace a
# file or symlink a concurrent writer created in between, and `ln -P`
# stays unusable (absent from BusyBox `ln`, which silently degraded every
# non-universal Agent to the copy layout on Alpine and most containers).
if ! ln -s "$_lcs_link_target" "$_lcs_dest" 2>/dev/null; then
restore_linked_skill_set "$_lcs_published" "$_lcs_backups" || true
rm -rf "$_lcs_stage"
return 1
fi
# Publish by moving the staged symlink. `mv` is POSIX; `ln -P` is not (it is
# absent from BusyBox `ln`, which silently degraded every non-universal
# Agent to the copy layout on Alpine and most containers). install-event.sh
# and install-devapp.sh already publish this way. The staged basename stays
# unique, so a destination a concurrent process turned into a directory can
# only receive our link and post-validation removes exactly that.
if ! mv "$_lcs_staged" "$_lcs_dest" 2>/dev/null || ! skill_link_matches "$_lcs_dest" "$_lcs_link_target" "$_lcs_inode"; then
cleanup_nested_staged_link "$_lcs_dest" "$_lcs_stage_name" "$_lcs_link_target" "$_lcs_inode" || true
# A directory that appeared at the destination turns `ln -s` into a
# container: the link lands inside it under the target basename. Remove
# exactly our link after an identity check and roll back; the foreign
# directory stays untouched.
if [ ! -L "$_lcs_dest" ]; then
_lcs_nested_name="${_lcs_link_target##*/}"
_lcs_nested_inode="$(skill_link_inode "$_lcs_dest/$_lcs_nested_name" 2>/dev/null)" || _lcs_nested_inode=""
cleanup_nested_staged_link "$_lcs_dest" "$_lcs_nested_name" "$_lcs_link_target" "$_lcs_nested_inode" || true
restore_linked_skill_set "$_lcs_published" "$_lcs_backups" || true
rm -rf "$_lcs_stage"
return 1
fi
_lcs_inode="$(skill_link_inode "$_lcs_dest")" || {
restore_linked_skill_set "$_lcs_published" "$_lcs_backups" || true
rm -rf "$_lcs_stage"
return 1
}
if ! skill_link_matches "$_lcs_dest" "$_lcs_link_target" "$_lcs_inode"; then
restore_linked_skill_set "$_lcs_published" "$_lcs_backups" || true
rm -rf "$_lcs_stage"
return 1