fix(install): validate APM_LIB_DIR before rm -rf to prevent data loss (closes #1690) by dohwi · Pull Request #1694 · microsoft/apm

dohwi · 2026-06-08T05:28:57Z

Summary

Adds four safety guards that run before install.sh deletes APM_LIB_DIR, fix the broken Quickstart Installation link, and document the new requirements. 32 pytest regression tests included.

Problem

install.sh exposes APM_LIB_DIR as an environment override and unconditionally runs rm -rf "$APM_LIB_DIR". A user who set APM_LIB_DIR=$HOME/.local/share while installing to $HOME/.local/bin lost unrelated application data (Atuin's local history DB). The Quickstart's Installation link is also a 404: ./getting-started/installation/ resolves to /apm/quickstart/getting-started/installation/ instead of /apm/getting-started/installation/.

Closes #1690.

Approach

Four guards, ordered so the cheapest checks fire first:

Guard	Check	rc	Failure example
1. Absolute path	Must start with `/`	11	`./apm`
2. Suffix	Must end with `/apm` or `/lib/apm`	12	`$HOME/.local/share` (the reported incident)
3. Blocklist	Must not be an exact match for a shared system directory (resolved via `readlink -f`/`realpath` to catch symlinks)	13	`$HOME`, `/usr`, `/opt`, `/tmp`, `/`
4. Marker file	If the directory exists and is non-empty, at least one of `apm`/`apm.cmd`/`VERSION`/`.apm-installed` must be present	14	non-empty dir with random files

The blocklist checks for exact path matches only (resolved). A path like $HOME/apm is allowed because the user has explicitly named an APM-specific directory. Suffix guard is the primary defence against accidentally broad paths.

Implementation

install.sh: Extracted guards into apm_lib_dir_validate() with INSTALL_SAFETY_BEGIN/INSTALL_SAFETY_END sentinel markers so tests can source the function without running the full installer.
tests/unit/install/test_install_safety.py: 32 regression tests that source the extracted function in a fresh bash subprocess. No Python build dependencies needed — conftest.py shadows the autouse fixtures from tests/conftest.py.
Docs: Fixed the Quickstart link and documented APM_LIB_DIR requirements in the installation page table and troubleshooting section.

Trade-offs

Blocklist is exact-match: /home/user/apm passes (suffix guard alone). A prefix-blocklist (e.g., "any child of $HOME") would accidentally reject $HOME/.local/lib/apm, which is the documented safe path. Future hardening could add a separate prefix blacklist.
macOS readlink: BSD readlink lacks -f. Falls back to realpath, then raw path. Suffix guard still blocks the reported incident on macOS regardless.

How to test

# Run the regression suite
python3 -m pytest tests/unit/install/test_install_safety.py -v

# Manual: the reported command should now fail safely (in a sandbox, no sudo)
bash -c '
  source <(sed -n "/^# INSTALL_SAFETY_BEGIN/,/^# INSTALL_SAFETY_END/p" install.sh)
  apm_lib_dir_validate "$HOME/.local/share" && echo "PASS" || echo "BLOCKED (rc=$?)"
'

…closes microsoft#1690) Add four safety guards that run before the installer deletes APM_LIB_DIR: 1. Absolute-path guard: reject relative paths 2. Suffix guard: path must end with /apm or /lib/apm 3. Blocklist guard: reject shared system directories ($HOME/.local/share, /usr, /opt, /tmp, /) 4. Marker-file guard: require evidence of a prior APM install before deleting an existing non-empty directory Also fix the broken "Installation" link in the Quickstart page and document the new APM_LIB_DIR requirements in the installation docs. Guards are extracted into apm_lib_dir_validate() with sentinel markers so they can be tested directly from a shell-source harness without running the full installer. 32 regression tests cover the 4 guards, the reported incident, safe default paths, and sentinel invariants.

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a safety validation layer for APM_LIB_DIR in install.sh to prevent destructive deletes on broad/shared directories (regression for issue #1690), plus a pytest-based harness that sources the exact shell function for verification and updates docs to reflect the new constraints.

Changes:

Added apm_lib_dir_validate() in install.sh with 4 guards (absolute path, suffix, blocklist, marker-file) and a refusal flow on unsafe paths.
Added pytest regression tests that extract and execute the sentinel-bounded validator block directly from install.sh, plus a local conftest.py to avoid unrelated autouse fixtures.
Updated installation docs/quickstart to document/point to the safety constraints.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tests/unit/install/test_install_safety.py	New regression tests that source the exact `install.sh` validator block and assert return codes for safety cases.
tests/unit/install/conftest.py	Overrides unrelated autouse fixtures to keep shell-script safety tests independent of Python CLI setup.
install.sh	Introduces `APM_LIB_DIR` validation function (sentinel-bounded for tests) and refusal messaging prior to `rm -rf`.
docs/src/content/docs/quickstart.mdx	Updates the “Installation” link target.
docs/src/content/docs/getting-started/installation.md	Documents `APM_LIB_DIR` safety validation behavior and adds an “Important” callout.

dohwi · 2026-06-08T05:40:22Z

+if ! apm_lib_dir_validate "$APM_LIB_DIR"; then
+    _rc=$?
+    echo -e "${RED}╔══════════════════════════════════════════════════════════════╗${NC}"
+    echo -e "${RED}║  REFUSING: APM_LIB_DIR=\"$APM_LIB_DIR\"${NC}"
+    echo -e "${RED}╠══════════════════════════════════════════════════════════════╣${NC}"
+    case $_rc in
+        11) echo -e "${RED}║  APM_LIB_DIR must be an absolute path.${NC}\n${RED}║  Relative paths are not accepted for safety.${NC}" ;;
+        12) echo -e "${RED}║  APM_LIB_DIR must end with /apm or /lib/apm.${NC}\n${RED}║  This prevents accidental deletion of non-APM data.${NC}\n${RED}║  Example: APM_LIB_DIR=\$HOME/.local/lib/apm${NC}" ;;
+        13) echo -e "${RED}║  This path is a shared system directory. Installing here${NC}\n${RED}║  would delete non-APM data.${NC}\n${RED}║  Use a dedicated APM directory (e.g. /usr/local/lib/apm).${NC}" ;;
+        14) echo -e "${RED}║  This directory exists but does not appear to be a${NC}\n${RED}║  previous APM installation. Refusing to delete it.${NC}\n${RED}║  If you are sure, remove it manually first:${NC}\n${RED}║    rm -rf \"$APM_LIB_DIR\"${NC}" ;;
+    esac
+    echo -e "${RED}╚══════════════════════════════════════════════════════════════╝${NC}"
+    exit 1
+fi
