fix(release): validate compare links and protect checksum output (successor to #170)

.github/workflows/memory-validation.yml

@@ -53,6 +53,7 @@ jobs:
             scripts/acceptance_agent_memory.sh \
             scripts/generate_release_checksums.sh \
             scripts/render_release_notes.sh \
+            scripts/test_release_checksum_output_safety.sh \
             scripts/test_release_helpers_compat.sh \
             scripts/test_release_guards.sh \
             scripts/test_validate_release_action_pins_compat.sh \

CHANGELOG.md

@@ -112,6 +112,17 @@ The project publishes 0.x prerelease versions; a stable release line is not yet
   why a request failed is still reported, and a DSN still names the parameters it
   sets — every query parameter *value* is replaced by `REDACTED`, including
   `?password=`, which pgx honours as the real password.
+- Checksum manifest generation rejects existing output files, directories and
+  symlinks without modifying their targets, including dangling symlinks, and
+  will not publish a manifest that is missing a row for an expected asset.
+- Release validation requires a release's CHANGELOG link to start at the
+  preceding CHANGELOG release, and accepts a link to that release's own page
+  only when no release precedes it. Checksum generation handles each asset path
+  separately on GNU and BSD tools, including directories with spaces, rejects
+  empty sets, and reports a failed asset listing as a failed listing.
+- The release guard suites count manifest lines without `wc -l`, whose BSD
+  implementation pads the count with blanks, and no longer need GNU
+  `find -printf`.
 - The npm installer no longer aborts a concurrent first run on Windows. The
   per-asset cache lock previously treated only `EEXIST` as contention, but a
   contended `mkdir` on Windows may raise `EPERM` or `EACCES`, so a process

scripts/generate_release_checksums.sh

@@ -11,11 +11,19 @@ die() {
   exit 1
 }
 
+require_absent_output() {
+  # -e alone misses dangling symlinks. In particular, mv follows an output
+  # symlink to a directory and would publish outside this asset directory.
+  [[ ! -e "${output}" && ! -L "${output}" ]] ||
+    die "checksum output path already exists: ${output}"
+}
+
 if [[ ! "${tag}" =~ ^v(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)(-rc\.(0|[1-9][0-9]*))?$ ]]; then
   die "invalid release tag: ${tag:-<empty>}"
 fi
 [[ "${commit}" =~ ^[0-9a-f]{40}$ ]] || die "invalid release commit: ${commit:-<empty>}"
 [[ -d "${asset_dir}" ]] || die "asset directory does not exist: ${asset_dir:-<empty>}"
+require_absent_output
 
 assets=(
   mem-mcp-darwin-amd64
@@ -26,13 +34,19 @@ assets=(
   mem-mcp-windows-arm64.exe
 )
 
+# A process substitution hides find's exit status from the loop below, so a
+# tool failure would surface as the misleading "differ from the exact expected
+# set". Capture the listing first and report a failed listing as what it is.
+asset_listing="$(
+  find "${asset_dir}" -mindepth 1 -maxdepth 1 -type f -exec basename {} \; | LC_ALL=C sort
+)" || die "cannot list release assets in ${asset_dir}"
+
 actual_assets=()
 while IFS= read -r actual_asset; do
+  [[ -n "${actual_asset}" ]] || continue
   actual_assets[${#actual_assets[@]}]="${actual_asset}"
-done < <(
-  find "${asset_dir}" -mindepth 1 -maxdepth 1 -type f -printf '%f\n' | LC_ALL=C sort
-)
-if [[ "${actual_assets[*]}" != "${assets[*]}" ]]; then
+done <<< "${asset_listing}"
+if [[ "${#actual_assets[@]}" -eq 0 ]] || [[ "${actual_assets[*]}" != "${assets[*]}" ]]; then
   printf 'ERROR: release assets differ from the exact expected set\n' >&2
   printf 'expected: %s\n' "${assets[*]}" >&2
   printf 'actual:   %s\n' "${actual_assets[*]:-<none>}" >&2
@@ -57,6 +71,13 @@ trap cleanup EXIT
     sha256sum "${assets[@]}"
   )
 } > "${tmp_output}"
+# The post-publish self-check below uses --ignore-missing, which by definition
+# tolerates a listed file being absent, so completeness is asserted here while
+# the staging file and the expected set are both known.
+manifest_rows="$(grep -c '' "${tmp_output}")"
+[[ "${manifest_rows}" -eq "${#assets[@]}" ]] ||
+  die "checksum manifest must have ${#assets[@]} rows, got ${manifest_rows}"
+require_absent_output
 mv -- "${tmp_output}" "${output}"
 trap - EXIT
 

scripts/test_release_checksum_output_safety.sh

@@ -0,0 +1,102 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+repo_root="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)"
+generator="${1:-${repo_root}/scripts/generate_release_checksums.sh}"
+tmp_dir="$(mktemp -d "${TMPDIR:-/tmp}/mem-checksum-output.XXXXXX")"
+trap 'rm -rf -- "${tmp_dir}"' EXIT
+
+die() {
+  printf 'FAIL: %s\n' "$*" >&2
+  exit 1
+}
+
+assets=(
+  mem-mcp-darwin-amd64
+  mem-mcp-darwin-arm64
+  mem-mcp-linux-amd64
+  mem-mcp-linux-arm64
+  mem-mcp-windows-amd64.exe
+  mem-mcp-windows-arm64.exe
+)
+commit=1111111111111111111111111111111111111111
+
+prepare() {
+  case_root="${tmp_dir}/${1} with spaces"
+  asset_dir="${case_root}/assets with spaces"
+  outside="${case_root}/outside with spaces"
+  mkdir -p -- "${asset_dir}" "${outside}"
+  for asset in "${assets[@]}"; do
+    printf 'test payload for %s\n' "${asset}" > "${asset_dir}/${asset}"
+  done
+  printf 'external data must remain unchanged\n' > "${outside}/keep.txt"
+  output="${asset_dir}/mem-mcp-checksums.txt"
+}
+
+snapshot_files() {
+  find "${case_root}" -type f -exec sha256sum {} \; | LC_ALL=C sort
+}
+
+for kind in symlink-directory symlink-file dangling-symlink directory regular-file; do
+  prepare "${kind}"
+  case "${kind}" in
+    symlink-directory) ln -s -- "${outside}" "${output}" ;;
+    symlink-file) ln -s -- "${outside}/keep.txt" "${output}" ;;
+    dangling-symlink) ln -s -- "${outside}/not-created.txt" "${output}" ;;
+    directory) mkdir -- "${output}" ;;
+    regular-file) printf 'existing manifest must remain unchanged\n' > "${output}" ;;
+  esac
+  before="$(snapshot_files)"
+  status=0
+  bash "${generator}" v0.1.1 "${commit}" "${asset_dir}" > "${tmp_dir}/result.log" 2>&1 || status=$?
+  # Check side effects before status: the original directory-symlink bug may
+  # return failure only after mv has already written outside the asset tree.
+  [[ "$(snapshot_files)" == "${before}" ]] ||
+    die "${kind}: output publication changed existing data or created an unexpected file"
+  [[ "${status}" -ne 0 ]] || die "${kind}: existing output was accepted"
+  grep -Fq 'checksum output path already exists' "${tmp_dir}/result.log" ||
+    die "${kind}: missing output-path diagnostic"
+  case "${kind}" in
+    symlink-directory) [[ -L "${output}" && "$(readlink "${output}")" == "${outside}" ]] ;;
+    symlink-file) [[ -L "${output}" && "$(readlink "${output}")" == "${outside}/keep.txt" ]] ;;
+    dangling-symlink) [[ -L "${output}" && "$(readlink "${output}")" == "${outside}/not-created.txt" ]] ;;
+    directory) [[ -d "${output}" && ! -L "${output}" ]] ;;
+    regular-file) [[ -f "${output}" && ! -L "${output}" ]] ;;
+  esac || die "${kind}: existing output path was replaced"
+  printf 'PASS: %s rejected without changing external data or output path\n' "${kind}"
+done
+
+# Simulate an output symlink appearing while checksum generation is in progress.
+# The second absence check must reject it and cleanup only our own staging file.
+prepare output-created-during-hashing
+fake_bin="${case_root}/hash tools"
+mkdir -p -- "${fake_bin}"
+printf '%s\n' '#!/usr/bin/env bash' 'set -euo pipefail' \
+  "\"\${REAL_SHA256SUM}\" \"\$@\"" \
+  "ln -s -- \"\${OUTPUT_TARGET}\" \"\${OUTPUT_MANIFEST}\"" > "${fake_bin}/sha256sum"
+chmod +x "${fake_bin}/sha256sum"
+before="$(snapshot_files)"
+status=0
+REAL_SHA256SUM="$(command -v sha256sum)" OUTPUT_TARGET="${outside}" OUTPUT_MANIFEST="${output}" \
+  PATH="${fake_bin}:${PATH}" bash "${generator}" v0.1.1 "${commit}" "${asset_dir}" \
+  > "${tmp_dir}/result.log" 2>&1 || status=$?
+[[ "${status}" -ne 0 ]] || die 'late output symlink was accepted'
+[[ "$(snapshot_files)" == "${before}" ]] || die 'late output symlink leaked staging data or changed external files'
+[[ -L "${output}" && "$(readlink "${output}")" == "${outside}" ]] || die 'late output symlink was replaced'
+grep -Fq 'checksum output path already exists' "${tmp_dir}/result.log" || die 'late output symlink lacks diagnostic'
+printf 'PASS: late output symlink rejected and private staging file cleaned up\n'
+
+# The mktemp template is not a predictable staging filename. A pre-existing
+# template-shaped symlink must remain untouched while a fresh manifest works.
+prepare unique-temporary-file
+ln -s -- "${outside}/keep.txt" "${asset_dir}/.mem-mcp-checksums.XXXXXX"
+before="$(sha256sum "${outside}/keep.txt")"
+bash "${generator}" v0.1.1 "${commit}" "${asset_dir}" >/dev/null
+[[ "$(sha256sum "${outside}/keep.txt")" == "${before}" ]] || die 'temporary output overwrote external data'
+[[ -L "${asset_dir}/.mem-mcp-checksums.XXXXXX" ]] || die 'temporary symlink was replaced'
+[[ -f "${output}" && ! -L "${output}" ]] || die 'fresh manifest was not created'
+(
+  cd -- "${asset_dir}"
+  sha256sum --check --strict mem-mcp-checksums.txt >/dev/null
+)
+printf 'PASS: unpredictable temporary output preserves a pre-existing template-shaped symlink\n'

scripts/test_release_guards.sh

@@ -99,6 +99,51 @@ fi
 expect_failure "version mismatch" \
   "${repo_root}/scripts/validate_release_version.sh" 999.999.999
 
+# Keep CHANGELOG mutations in a fixture tree. All other version surfaces remain
+# the real checkout, so failures below must reach the comparison-link guard.
+version_fixture="${tmp_dir}/version fixture"
+mkdir -p -- "${version_fixture}/scripts"
+cp -- "${repo_root}/scripts/validate_release_version.sh" "${version_fixture}/scripts/"
+for surface in npm server worker web deploy docs; do
+  ln -s -- "${repo_root}/${surface}" "${version_fixture}/${surface}"
+done
+fixture_validator="${version_fixture}/scripts/validate_release_version.sh"
+previous_version=0.0.1
+
+write_changelog_fixture() {
+  local link="$1"
+  local include_previous="${2:-yes}"
+  {
+    printf '## [Unreleased]\n\n## [%s] - 2026-01-01\n\n' "${current_version}"
+    if [[ "${include_previous}" == yes ]]; then
+      printf '## [%s] - 2025-01-01\n\n' "${previous_version}"
+    fi
+    printf '[Unreleased]: https://github.com/bytefolk/mem/compare/v%s...HEAD\n' "${current_version}"
+    printf '[%s]: https://github.com/bytefolk/mem/%s\n' "${current_version}" "${link}"
+  } > "${version_fixture}/CHANGELOG.md"
+}
+
+correct_compare="compare/v${previous_version}...${current_tag}"
+write_changelog_fixture "${correct_compare}"
+"${fixture_validator}" "${current_version}" >/dev/null
+for wrong_base in v0.0.0 "${current_tag}" arbitrary; do
+  write_changelog_fixture "compare/${wrong_base}...${current_tag}"
+  expect_failure "wrong compare base ${wrong_base}" "${fixture_validator}" "${current_version}"
+done
+write_changelog_fixture "compare/v${previous_version}...v999.999.999"
+expect_failure "wrong compare endpoint" "${fixture_validator}" "${current_version}"
+write_changelog_fixture "${correct_compare}/extra"
+expect_failure "compare link suffix" "${fixture_validator}" "${current_version}"
+write_changelog_fixture "${correct_compare}" no
+expect_failure "missing compare predecessor" "${fixture_validator}" "${current_version}"
+write_changelog_fixture "releases/tag/${current_tag}" no
+"${fixture_validator}" "${current_version}" >/dev/null
+# A tag link is only legitimate when no release precedes this one. Once a
+# predecessor exists, pointing at the tag page must not satisfy the guard.
+write_changelog_fixture "releases/tag/${current_tag}"
+expect_failure "tag link replaces the predecessor" "${fixture_validator}" "${current_version}"
+printf 'PASS: compare links require the exact predecessor and endpoint; tag links are valid only without a predecessor\n'
+
 notes_file="${tmp_dir}/release-notes.md"
 "${repo_root}/scripts/render_release_notes.sh" "${current_tag}" > "${notes_file}"
 [[ -s "${notes_file}" ]] || die "release notes are empty"
@@ -190,8 +235,17 @@ expect_failure "annotated tag version mismatch" env \
   FAKE_HEAD_COMMIT="${same_commit}" \
   "${repo_root}/scripts/validate_release_source.sh" v999.999.999
 
-asset_dir="${tmp_dir}/assets"
+asset_dir="${tmp_dir}/assets with spaces"
 mkdir -p -- "${asset_dir}"
+# An empty set must fail with the intended diagnostic, including on Bash 3.2.
+if "${repo_root}/scripts/generate_release_checksums.sh" \
+  "${current_tag}" "${same_commit}" "${asset_dir}" > "${tmp_dir}/empty-assets.log" 2>&1; then
+  die "empty asset directory: command unexpectedly succeeded"
+fi
+grep -Fq -- 'actual:   <none>' "${tmp_dir}/empty-assets.log" ||
+  die "empty asset directory must report the missing set"
+[[ ! -e "${asset_dir}/mem-mcp-checksums.txt" ]] ||
+  die "empty asset directory must not produce a manifest"
 assets=(
   mem-mcp-darwin-amd64
   mem-mcp-darwin-arm64
@@ -203,11 +257,14 @@ assets=(
 for asset in "${assets[@]}"; do
   printf 'test payload for %s\n' "${asset}" > "${asset_dir}/${asset}"
 done
+# Use the real find, basename and sha256sum here. Unlike the Bash-only compat
+# suite, this must also catch GNU basename rejecting batched find -exec paths.
 "${repo_root}/scripts/generate_release_checksums.sh" \
   "${current_tag}" "${same_commit}" "${asset_dir}" >/dev/null
 
 manifest="${asset_dir}/mem-mcp-checksums.txt"
-[[ "$(wc -l < "${manifest}")" == 6 ]] || die "checksum manifest must have six rows"
+# BSD wc pads its count with blanks, so a line count must not come from wc -l.
+[[ "$(grep -c '' "${manifest}")" == 6 ]] || die "checksum manifest must have six rows"
 (
   cd -- "${asset_dir}"
   sha256sum --check --strict "$(basename -- "${manifest}")" >/dev/null
@@ -239,4 +296,5 @@ expect_failure "symlink asset" \
   "${repo_root}/scripts/generate_release_checksums.sh" \
   "${current_tag}" "${same_commit}" "${asset_dir}"
 
+bash "${repo_root}/scripts/test_release_checksum_output_safety.sh"
 printf 'PASS: release source, notes, asset-set and checksum guards fail closed\n'

scripts/test_release_helpers_compat.sh

@@ -75,6 +75,11 @@ if ! (
   exit 1
 fi
 
-[[ "$(wc -l < "${asset_dir}/mem-mcp-checksums.txt")" == 6 ]]
+# This suite is the one that claims independence from GNU find and coreutils,
+# so it must not count lines with wc -l: BSD wc pads the count with blanks.
+[[ "$(grep -c '' "${asset_dir}/mem-mcp-checksums.txt")" == 6 ]] || {
+  printf 'ERROR: checksum manifest must have six rows\n' >&2
+  exit 1
+}
 
 printf 'PASS: release version and checksum helpers run without Bash 4-only collection builtins\n'

scripts/validate_release_version.sh

@@ -93,11 +93,33 @@ while IFS= read -r version_link; do
 done < <(grep -F -- "[${version}]: " "${changelog}" || true)
 [[ "${#version_links[@]}" == 1 ]] ||
   die "CHANGELOG.md: expected exactly one [${version}] comparison link"
-if [[ "${version_links[0]}" != \
-    "[${version}]: https://github.com/bytefolk/mem/releases/tag/v${version}" &&
-  "${version_links[0]}" != \
-    "[${version}]: https://github.com/bytefolk/mem/compare/"*"...v${version}" ]]; then
-  die "CHANGELOG.md: [${version}] link must terminate at v${version}"
+compare_base="$(awk '
+  /^## \[[0-9]/ {
+    if (seen++) {
+      gsub(/^## \[/, "", $0)
+      gsub(/\].*/, "", $0)
+      print $0
+      exit
+    }
+  }
+' "${changelog}")"
+
+# Exactly one link form is correct, decided by whether a release precedes this
+# one: with a predecessor the link must start at that release; without one (a
+# first release such as 0.1.0) there is nothing to compare from, so the release
+# keeps its tag link. Accepting the tag form in both cases would let a later
+# release dodge the predecessor requirement.
+if [[ -n "${compare_base}" ]]; then
+  expected_link="[${version}]: https://github.com/bytefolk/mem/compare/v${compare_base}...v${version}"
+else
+  expected_link="[${version}]: https://github.com/bytefolk/mem/releases/tag/v${version}"
+fi
+
+if [[ "${version_links[0]}" != "${expected_link}" ]]; then
+  if [[ -n "${compare_base}" ]]; then
+    die "CHANGELOG.md: [${version}] link must start at the preceding release v${compare_base}:"$'\n'"  ${expected_link}"
+  fi
+  die "CHANGELOG.md: nothing precedes [${version}], so its link must be exactly:"$'\n'"  ${expected_link}"
 fi
 
 printf 'PASS: all release version surfaces match %s\n' "${version}"