diff --git a/.github/workflows/memory-validation.yml b/.github/workflows/memory-validation.yml index ed42033..f7f2943 100644 --- a/.github/workflows/memory-validation.yml +++ b/.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 \ diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a5e985..fa583ff 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/scripts/generate_release_checksums.sh b/scripts/generate_release_checksums.sh index 4fb27fd..69aad34 100755 --- a/scripts/generate_release_checksums.sh +++ b/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:-}" fi [[ "${commit}" =~ ^[0-9a-f]{40}$ ]] || die "invalid release commit: ${commit:-}" [[ -d "${asset_dir}" ]] || die "asset directory does not exist: ${asset_dir:-}" +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[*]:-}" >&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 diff --git a/scripts/test_release_checksum_output_safety.sh b/scripts/test_release_checksum_output_safety.sh new file mode 100755 index 0000000..faa8626 --- /dev/null +++ b/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' diff --git a/scripts/test_release_guards.sh b/scripts/test_release_guards.sh index 4366a82..dacb16e 100755 --- a/scripts/test_release_guards.sh +++ b/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: ' "${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' diff --git a/scripts/test_release_helpers_compat.sh b/scripts/test_release_helpers_compat.sh index 0757d1f..1e8ba2e 100755 --- a/scripts/test_release_helpers_compat.sh +++ b/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' diff --git a/scripts/validate_release_version.sh b/scripts/validate_release_version.sh index 987a304..e063ac3 100755 --- a/scripts/validate_release_version.sh +++ b/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}"