diff --git a/scripts/install-skills.sh b/scripts/install-skills.sh index 5ed3f101..820e5880 100755 --- a/scripts/install-skills.sh +++ b/scripts/install-skills.sh @@ -204,6 +204,20 @@ sha256_stdin() { fi } +# sha256_file : content digest used by skill_dir_content_digest. Never +# emits on failure so the identity fails instead of parsing a wrong token. +sha256_file() { + if command -v sha256sum >/dev/null 2>&1; then + sha256sum "$1" 2>/dev/null | awk '{print $1}' + elif command -v shasum >/dev/null 2>&1; then + shasum -a 256 "$1" 2>/dev/null | awk '{print $1}' + elif command -v openssl >/dev/null 2>&1; then + openssl dgst -sha256 -r "$1" 2>/dev/null | awk '{print $1}' + else + return 1 + fi +} + verify_release_asset_checksum() { _checksum_asset="$1" _checksum_path="$2" @@ -450,10 +464,12 @@ skill_link_inode() { printf '%s\n' "$_sli_entry" | awk '{print $1}' } -# Directory publication identity is inode plus a stable child-name list. -# Inode reuse on Linux overlayfs would otherwise make a concurrent directory -# look owned. The token has no colon so dest:identity manifest parsing stays -# dest="${entry%:*}" / identity="${entry##*:}". +# Directory publication identity is inode, a stable child-name list, and a +# recursive content digest. Inode reuse on Linux overlayfs would otherwise +# make a concurrent directory look owned, and an in-place content edit after +# publish (same inode, same names) would otherwise be retracted as this +# transaction's object. The token has no colon so dest:identity manifest +# parsing stays dest="${entry%:*}" / identity="${entry##*:}". skill_dir_identity() { _sdi_inode="$(skill_link_inode "$1")" || return 1 if [ -L "$1" ] || [ ! -d "$1" ]; then @@ -461,7 +477,34 @@ skill_dir_identity() { return 0 fi _sdi_names="$(LC_ALL=C ls -A "$1" 2>/dev/null | tr '\n' ',')" - printf '%s,%s\n' "$_sdi_inode" "$_sdi_names" + _sdi_digest="$(skill_dir_content_digest "$1")" || return 1 + printf '%s,%s,%s\n' "$_sdi_inode" "$_sdi_names" "$_sdi_digest" +} + +# Recursive content digest over a published tree, mirroring the Go +# fingerprint: sorted relative paths emit type+permission bits plus either a +# content sha256 or the symlink target, and the whole line stream feeds +# sha256_stdin. Digest failures surface as a non-zero identity instead of a +# wrong-but-parseable token. +skill_dir_content_digest() { + ( + cd "$1" 2>/dev/null || exit 1 + find . -print 2>/dev/null | + LC_ALL=C sort | + while IFS= read -r _sdcd_path; do + _sdcd_mode="$(ls -ld "$_sdcd_path" 2>/dev/null | cut -c1-10)" + if [ -L "$_sdcd_path" ]; then + printf '%s|%s|link|%s\n' "$_sdcd_path" "$_sdcd_mode" "$(readlink "$_sdcd_path" 2>/dev/null)" + elif [ -d "$_sdcd_path" ]; then + printf '%s|%s|dir\n' "$_sdcd_path" "$_sdcd_mode" + elif [ -f "$_sdcd_path" ]; then + _sdcd_hash="$(sha256_file "$_sdcd_path")" || exit 1 + printf '%s|%s|file|%s\n' "$_sdcd_path" "$_sdcd_mode" "$_sdcd_hash" + else + printf '%s|%s|other\n' "$_sdcd_path" "$_sdcd_mode" + fi + done + ) | sha256_stdin } skill_link_matches() { @@ -505,8 +548,9 @@ cleanup_nested_staged_link() { # staging directory is created under the same umask as the claim, so their # modes match by construction). A failed child move relocates the children # back and removes only the claim. On success the destination's inode is -# recorded as : so rollback only ever deletes this -# transaction's object even when the filesystem recycles the claim inode. +# recorded as : so rollback only ever +# deletes this transaction's object even when the filesystem recycles the +# claim inode or the published files are edited in place afterwards. publish_skill_dir_no_replace() { _psd_stage="$1" _psd_dest="$2" diff --git a/scripts/install.sh b/scripts/install.sh index 2beb6c93..300d5e95 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -222,6 +222,21 @@ sha256_stdin() { fi } +# sha256_file : content digest used by skill_dir_content_digest. Never +# emits on failure (the caller's pipeline fails the whole identity instead of +# producing a wrong-but-parseable token). +sha256_file() { + if need_cmd sha256sum; then + sha256sum "$1" 2>/dev/null | awk '{print $1}' + elif need_cmd shasum; then + shasum -a 256 "$1" 2>/dev/null | awk '{print $1}' + elif need_cmd openssl; then + openssl dgst -sha256 -r "$1" 2>/dev/null | awk '{print $1}' + else + return 1 + fi +} + verify_release_asset_checksum() { _checksum_asset="$1" _checksum_path="$2" @@ -470,10 +485,12 @@ skill_link_inode() { printf '%s\n' "$_sli_entry" | awk '{print $1}' } -# Directory publication identity is inode plus a stable child-name list. -# Inode reuse on Linux overlayfs would otherwise make a concurrent directory -# look owned. The token has no colon so dest:identity manifest parsing stays -# dest="${entry%:*}" / identity="${entry##*:}". +# Directory publication identity is inode, a stable child-name list, and a +# recursive content digest. Inode reuse on Linux overlayfs would otherwise +# make a concurrent directory look owned, and an in-place content edit after +# publish (same inode, same names) would otherwise be retracted as this +# transaction's object. The token has no colon so dest:identity manifest +# parsing stays dest="${entry%:*}" / identity="${entry##*:}". skill_dir_identity() { _sdi_inode="$(skill_link_inode "$1")" || return 1 if [ -L "$1" ] || [ ! -d "$1" ]; then @@ -481,7 +498,36 @@ skill_dir_identity() { return 0 fi _sdi_names="$(LC_ALL=C ls -A "$1" 2>/dev/null | tr '\n' ',')" - printf '%s,%s\n' "$_sdi_inode" "$_sdi_names" + _sdi_digest="$(skill_dir_content_digest "$1")" || return 1 + printf '%s,%s,%s\n' "$_sdi_inode" "$_sdi_names" "$_sdi_digest" +} + +# Recursive content digest over a published tree: for each path (LC_ALL=C +# sorted, relative) emit path, type+permission bits from ls -ld, and either +# the sha256 of the file content or the symlink target. Sorted-path lines are +# piped through sha256_stdin, so one opaque token covers renames, mode flips, +# content edits and link retargets — the same in-place-edit guarantee the Go +# fingerprint provides. Digest failures (missing hash tool, unreadable file) +# surface as a non-zero identity instead of a wrong-but-parseable token. +skill_dir_content_digest() { + ( + cd "$1" 2>/dev/null || exit 1 + find . -print 2>/dev/null | + LC_ALL=C sort | + while IFS= read -r _sdcd_path; do + _sdcd_mode="$(ls -ld "$_sdcd_path" 2>/dev/null | cut -c1-10)" + if [ -L "$_sdcd_path" ]; then + printf '%s|%s|link|%s\n' "$_sdcd_path" "$_sdcd_mode" "$(readlink "$_sdcd_path" 2>/dev/null)" + elif [ -d "$_sdcd_path" ]; then + printf '%s|%s|dir\n' "$_sdcd_path" "$_sdcd_mode" + elif [ -f "$_sdcd_path" ]; then + _sdcd_hash="$(sha256_file "$_sdcd_path")" || exit 1 + printf '%s|%s|file|%s\n' "$_sdcd_path" "$_sdcd_mode" "$_sdcd_hash" + else + printf '%s|%s|other\n' "$_sdcd_path" "$_sdcd_mode" + fi + done + ) | sha256_stdin } skill_link_matches() { @@ -525,8 +571,9 @@ cleanup_nested_staged_link() { # staging directory is created under the same umask as the claim, so their # modes match by construction). A failed child move relocates the children # back and removes only the claim. On success the destination's inode is -# recorded as : so rollback only ever deletes this -# transaction's object even when the filesystem recycles the claim inode. +# recorded as : so rollback only ever +# deletes this transaction's object even when the filesystem recycles the +# claim inode or the published files are edited in place afterwards. publish_skill_dir_no_replace() { _psd_stage="$1" _psd_dest="$2" diff --git a/test/scripts/install_script_test.go b/test/scripts/install_script_test.go index 300d2c0f..04d967b0 100644 --- a/test/scripts/install_script_test.go +++ b/test/scripts/install_script_test.go @@ -2553,6 +2553,94 @@ fi } } +// TestInstallerShellRollbackPreservesInPlaceEditedSkill pins the P1 contract: +// a published Skill directory whose file contents were edited in place (same +// directory inode, same child names) must not be deleted by the rollback of a +// later failed publication. The recorded identity carries a recursive content +// digest, so the edit makes the dest provably not-this-transaction's object. +func TestInstallerShellRollbackPreservesInPlaceEditedSkill(t *testing.T) { + for _, scriptName := range []string{"install.sh", "install-skills.sh"} { + scriptName := scriptName + t.Run(scriptName, func(t *testing.T) { + t.Parallel() + scriptPath, err := filepath.Abs(filepath.Join("..", "..", "scripts", scriptName)) + if err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(scriptPath) + if err != nil { + t.Fatal(err) + } + cut := strings.LastIndex(string(data), "\nmain\n") + if cut < 0 { + t.Fatalf("%s final main invocation not found", scriptName) + } + library := filepath.Join(t.TempDir(), scriptName+"-lib.sh") + mustWriteFile(t, library, data[:cut], 0o755) + + home := t.TempDir() + base := filepath.Join(home, ".zcode", "skills") + first := filepath.Join(base, "dingtalk-first") + second := filepath.Join(base, "dingtalk-second") + mustWriteFile(t, filepath.Join(first, "SKILL.md"), []byte("old first\n"), 0o644) + mustWriteFile(t, filepath.Join(second, "SKILL.md"), []byte("old second\n"), 0o644) + src := filepath.Join(t.TempDir(), "multi") + mustWriteFile(t, filepath.Join(src, "dingtalk-first", "SKILL.md"), []byte("new first\n"), 0o644) + mustWriteFile(t, filepath.Join(src, "dingtalk-first", "references", "guide.md"), []byte("guide\n"), 0o644) + mustWriteFile(t, filepath.Join(src, "dingtalk-second", "SKILL.md"), []byte("new second\n"), 0o644) + + // Inside the second publication's child move — after the first + // skill has been published and its identity recorded — edit a file + // inside the first destination in place (append keeps the inode, + // directory inode and child names are untouched), then fail the + // transaction. Rollback must detect the content drift through the + // recorded digest and leave the user's edit alone. + harness := `. "$DWS_TEST_LIBRARY" +mv() { + case "$1" in + */.dws-multi-set.*) + if [ "$2" = "$DWS_TEST_SECOND/" ]; then + printf 'user-edit\n' >> "$DWS_TEST_FIRST/references/guide.md" + return 1 + fi + ;; + esac + command mv "$@" +} +if _install_multi_to_base "$DWS_TEST_SRC" "$DWS_TEST_BASE" "$HOME" ".zcode/skills"; then + exit 2 +fi +` + cmd := exec.Command("sh", "-c", harness) + cmd.Env = append(os.Environ(), + "HOME="+home, + "DWS_TEST_LIBRARY="+library, + "DWS_TEST_BASE="+base, + "DWS_TEST_SRC="+src, + "DWS_TEST_FIRST="+first, + "DWS_TEST_SECOND="+second, + ) + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("%s in-place edit harness failed: %v\n%s", scriptName, err, output) + } + if !strings.Contains(string(output), "跳过回滚已被并发修改的 Skill 路径: "+first) { + t.Fatalf("%s did not skip in-place-edited rollback:\n%s", scriptName, output) + } + got, err := os.ReadFile(filepath.Join(first, "references", "guide.md")) + if err != nil || string(got) != "guide\nuser-edit\n" { + t.Fatalf("%s in-place user edit was lost: %q, %v", scriptName, string(got), err) + } + if restored, err := os.ReadFile(filepath.Join(second, "SKILL.md")); err != nil || string(restored) != "old second\n" { + t.Fatalf("%s second skill was not restored from backup: %q, %v", scriptName, string(restored), err) + } + if matches, err := filepath.Glob(filepath.Join(base, ".dws-multi-set.*")); err != nil || len(matches) != 0 { + t.Fatalf("%s staging leftovers = %v, err=%v", scriptName, matches, err) + } + }) + } +} + func TestInstallerShellUsesUpstreamXDGAndCustomAgentRoots(t *testing.T) { for _, scriptName := range []string{"install.sh", "install-skills.sh"} { t.Run(scriptName, func(t *testing.T) {