Skip to content

Prevent version names from escaping the versions directory #15

Description

@mitchhs12

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/outside

If /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:

  • use can activate binaries from a directory outside AMP_DIR.
  • install and source builds can write outside the versions directory.
  • ADBC commands can inspect, install, or remove driver files outside AMP_DIR.
  • A malformed value in .version can 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.0
  • main-deadbeef
  • my-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 VersionName type 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 VersionName rather 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 .version fail before any version-scoped file is accessed.
  • Normal release and custom version names continue to work.
  • Branches such as feature/new-thing still 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/versions remain 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions