fix: verify staged vendor payloads before install

[codeforester]

Summary

• Verify every staged vendor payload against the bundle manifest before and after copying.
• Derive install lock metadata from staged bytes and validate staged create, update, and standalone trees before the atomic move.
• Add a regression test that mutates a framework payload during copy and requires a fail-closed install.

Issue

Fixes #526

Validation

• bats tests/vendor.bats (10 tests passed).
• ./tests/validate.sh (683 tests passed; artifact, release, concurrency, and quality contracts passed).
• shellcheck --shell=bash --severity=warning scripts/vendor tests/vendor.bats
• bash -n scripts/vendor
• git diff --check

Security Notes

The install now rejects source mutation, copied-byte mismatch, and manifest mismatch before replacing the destination. Incomplete staging trees are cleaned up and no network or bundle-data evaluation was introduced.

[codeforester]

Multi-angle review of this PR (fix: verify staged vendor payloads before install). This is a security-hardening PR, so I pushed hard on it — five findings, ranked by severity:

1. Path traversal isn't actually rejected in the new manifest-driven copy path. copy_verified_bundle's new while read -r checksum path loop over MANIFEST.sha256 (lines ~44-75) never validates the path value itself — no rejection of .., a leading /, or // — unlike library-bundle's existing verify_bundle, which does perform that validation when checking a manifest. If MANIFEST.sha256 is swapped (mid-race, or via any path that reaches this function with an attacker-influenced bundle) to include a line like <hash-of-evil> ../../../tmp/evil with a matching file placed at that relative location, the hash check alone passes and mkdir -p "$destination/$(dirname -- "../../../tmp/evil")" + cp (lines ~56-57) write outside the intended staging directory — defeating the atomic-staging/mv design the rest of install_bundle/update_bundle relies on for safety.

2. The new checks only lstat the final path component, not intermediate directories — a gap the same PR explicitly defends against elsewhere. The PR adds a test ("standalone refuses leaf and parent symlink swaps during payload copy") proving parent-directory symlink swaps matter for application-payload copying, and validate_payload_file correctly walks every path component to catch it. But copy_verified_bundle/verify_manifest_copy's new checks ([[ -f "$source/$path" && ! -L "$source/$path" ]]) only check the resolved leaf. Swapping an intermediate directory in the framework bundle (e.g. $source/lib) to a symlink mid-copy, then restoring it, is silently accepted here even though the equivalent attack on application payloads is explicitly tested and rejected a few functions away.

3. standalone_bundle's two copies of the framework bundle get different verification strength, and the recorded lock hash comes from the weaker one. The root copy at $temporary is checked only by the new, weaker verify_manifest_copy; the nested copy at $temporary/vendor/base-bash-libs gets the full library-bundle verify (exact BUNDLE.release inventory, no stray files). But framework_lock in BASE_BASH_STANDALONE.release is hashed from the root copy's manifest — the one that never went through full verification — and nothing cross-checks it against the nested copy's manifest_sha256 in base-bash-libs.lock. No test in the PR's new bats coverage catches this divergence.

4. Several of the new tamper-detection branches fail silently, unlike every sibling check in the same function. The new pre-loop guard ([[ -f "$source/MANIFEST.sha256" && ! -L ... ]] || return 1), the manifest-hash read, and all four failure branches inside the new verify_manifest_copy return 1 with no error() call — while every other check added in copy_verified_bundle (lines ~48, 53, 59, 64, 69, 74, 81, 86) does call error() with a specific "Vendor error: ..." message first. In practice this means the one failure mode this PR is specifically about — active tampering caught mid-copy — is the one case an operator sees as a bare, silent exit 1 with no diagnostic, indistinguishable from an unrelated environment failure. This exact path also isn't exercised by the PR's own new bats tests.

5. Root cause tying 1, 2, and 4 together: this is now the third independent reimplementation of "hash every file against MANIFEST.sha256" in this script/repo, alongside library-bundle's existing verify_bundle and copy_verified_bundle's own pre-existing inline loop — each with different rigor (path-safety validation, parent-symlink walking, error reporting). A shared primitive (e.g. splitting verify_bundle into a generic "verify this directory against its own manifest" step usable here, plus the repo-specific inventory check kept separate) would remove findings 1, 2, and 4 at the source instead of requiring three implementations to be kept in sync by hand.

[codeforester]

Two more concrete issues surfaced by a follow-up pass, additional to the five above:

6. verify_manifest_copy runs before the application's VERSION/README.md get restored over the framework's copies (lines ~404-414 restore consumer-owned files that share a framework filename), so the root tree's MANIFEST.sha256 is stale for any overwritten path by the time the bundle actually ships. Not exploitable, but a correctness gap: any later tooling that re-validates the shipped root against its own MANIFEST.sha256 will report a false hash mismatch for VERSION even though the tree is in its intended, correctly-built state — because the manifest was verified before the intentional overwrite happened.

7. The two copy_verified_bundle calls against the same $framework_bundle (root at ~line 395, vendor/base-bash-libs at ~line 396) never check they observed identical source content. If $framework_bundle is rebuilt or mutated in the narrow window between the two calls, each call independently passes its own internal hash checks against whatever manifest is present at that moment — so root and the nested copy can silently end up staged from two different versions of the framework, with nothing comparing them to catch the divergence. This compounds finding #3 above (the recorded framework_lock already comes from the weaker-verified copy; this finding means it might not even describe the same bytes as what's nested under vendor/base-bash-libs).

[codeforester]

Addressed in 3f2893a. The vendor path now uses a shared manifest validator (safe relative paths and every path component free of symlinks), snapshots and validates manifest entries with diagnostics, and fully verifies both standalone framework copies. Their manifest identities are compared before application payload restoration; framework_lock now binds the canonical nested copy; and the root manifest refreshes its intentional application VERSION override before final verification. Added regression coverage and updated the standalone workflow documentation. Local ./tests/validate.sh passed all 683 tests and contract stages.

[codeforester]

Confirmed fixed — re-verified with live reproductions, not just reading the diff (worktree at 3f2893a2):

• Path traversal (Import reusable Bash libraries #1): crafted a manifest entry pointing outside the source tree with a matching hash; now rejected with Vendor error: unsafe bundle checksum path: ../outside/evil.txt.
• Intermediate-symlink gap (Prepare base-bash-libs v0.1.0 release #2): made an intermediate directory a symlink to an outside tree with a manifest entry whose leaf check alone would pass; now rejected with Vendor error: bundle payload path traverses a symlink while being copied (and equivalently in verify_manifest_copy). Confirmed the new component-walking check is now applied to the framework bundle copy path too, not just application payloads.
• Inconsistent root/nested verification + lock hash (Prepare base-bash-libs v0.1.0 release #3): root now also gets full library-bundle verify, and a new explicit cross-check aborts if the root and nested manifests differ. framework_lock is now recorded from the fully-verified nested copy — confirmed live that it exactly matches sha256(vendor/base-bash-libs/MANIFEST.sha256).
• Silent failures (Document Homebrew tap trust for standalone installs #4): every new failure branch now calls error() first — confirmed via the live tamper repros above, both produced actual stderr diagnostics, not silent exits.
• Triplicated logic (Document Homebrew tap trust #5): the path-safety and symlink-walk checks are now extracted into scripts/bundle-manifest.sh and shared identically across library-bundle, copy_verified_bundle, and verify_manifest_copy — this closes the inconsistency that caused findings 1/2/4 in the first place. (The full per-file hashing loop itself is still implemented three times, just now equally rigorous each time — a nice-to-have follow-up, not a blocker.)
• Stale manifest before payload restore (Fail cleanly when sourced by unsupported Bash versions #6): order is now restore-then-refresh-then-verify; refresh_standalone_manifest rewrites the VERSION entry and hard-fails if it can't find exactly one VERSION line. Confirmed live the shipped VERSION hash matches its manifest entry.
• No cross-check between the two framework copies (Preserve target file metadata when updating managed sections #7): reproduced the race directly (mutated the source between the two copy_verified_bundle calls) — now caught by the same root/nested manifest-hash comparison as Prepare base-bash-libs v0.1.0 release #3, aborting with `standalone framework copies observed different manifest content". Note this specific race isn't covered by a dedicated bats test yet, only proven by this manual repro — worth adding as regression coverage at some point, but not blocking.

bats tests/vendor.bats tests/library-bundle.bats: 16/16 passing, including new regression tests for the intermediate-symlink rejection and the VERSION/framework_lock consistency checks. All CI green. Good to merge from my side.