Repository navigation
Prevent version names from escaping the versions directory #15
Description
What is happening?
Ampup currently treats a version name as both an identifier and a filesystem path.
Normal version names such as v0.1.0 work as expected. However, ampup also accepts path-like values such as:
/tmp/some-directory../some-directory.or..
Internally, ampup builds the version directory with:
self.versions_dir.join(version)An absolute path replaces the versions_dir prefix, while .. can move outside it. This means a command intended to operate inside AMP_DIR/versions may instead operate somewhere else on the filesystem.
Why does this matter?
The clearest example is uninstall:
ampup uninstall \
--install-dir /tmp/ampup-version-repro/amp-root \
/tmp/ampup-version-repro/outsideIf /tmp/ampup-version-repro/outside exists, the current uninstall flow can recursively remove it because it treats that absolute path as the version directory.
The same underlying behavior affects other commands:
usecan activate binaries from a directory outsideAMP_DIR.installand source builds can write outside the versions directory.- ADBC commands can inspect, install, or remove driver files outside
AMP_DIR. - A malformed value in
.versioncan reach the same ADBC paths.
This is a local path-safety and correctness issue, not a remote exploit by itself. It requires path-like CLI input, a malformed .version file, or similar local state. Still, destructive commands should never be able to escape their configured directory because a version identifier happens to look like a path.
Expected behavior
A version name should be an identifier—not a path. Ampup should accept names such as:
v0.1.0main-deadbeefmy-dev-build
It should reject:
- Empty names
- Absolute paths
- Names containing
/or\ .and..- Names with surrounding whitespace or control characters
Generated names need slightly different treatment: commands such as ampup build --branch feature/new-thing should continue to work, but the generated local version name should safely encode or flatten the branch separator. An explicit --name feature/new-thing should be rejected rather than silently changed.
Suggested approach
- Introduce a validated
VersionNametype representing exactly one normal path component. - Validate version values when they enter the system:
- CLI arguments
.version- GitHub release tags used as local version directories
- Custom and generated source-build names
- Make filesystem-facing APIs accept
VersionNamerather than an unchecked string. - Ensure install, use, uninstall, build, and ADBC paths cannot escape
versions_dir.
Acceptance criteria
- Path-like version values fail before any filesystem mutation or network/build work.
- Invalid values in
.versionfail before any version-scoped file is accessed. - Normal release and custom version names continue to work.
- Branches such as
feature/new-thingstill produce a valid single-directory build name. - Regression tests cover absolute paths and parent traversal for the destructive version and ADBC operations.
- Tests verify that files and directories outside
AMP_DIR/versionsremain untouched.
This was discovered while reviewing #14. The original version uninstall already had the strongest form of the problem; #14 added additional ADBC paths that rely on the same unchecked version-name assumption.
Activity
[-]Validate version names before using them in filesystem paths[/-][+]Prevent version names from escaping the versions directory[/+]