From c3e46b0a60edaff39369a857c0b569b772d46d2a Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Wed, 22 Jul 2026 12:19:09 +0300 Subject: [PATCH] pilotctl: confine appstore uninstall appID under the store root (path traversal) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cmdAppStoreUninstall did filepath.Join(root, appID) + RemoveAll without validating appID — 'appstore uninstall --yes ../../something' would RemoveAll outside the install tree. install/verify already guard this via resolveUnder; uninstall now does too. Local-operator-only (no remote trust boundary), but a cheap correctness fix. resolveUnder's existing traversal test (zz_appstore_helpers_test.go) covers the guard. Co-Authored-By: Claude Opus 4.8 --- cmd/pilotctl/appstore.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/cmd/pilotctl/appstore.go b/cmd/pilotctl/appstore.go index 03f31827..6b07d9ae 100644 --- a/cmd/pilotctl/appstore.go +++ b/cmd/pilotctl/appstore.go @@ -762,8 +762,16 @@ func cmdAppStoreUninstall(args []string) { "refusing to uninstall %q without --yes", appID) } + // appID is user input; confine it under the app-store root so a + // crafted id (e.g. "../../something") can't RemoveAll outside the + // install tree. Same guard the install/verify paths use. root := appStoreRoot() - dir := filepath.Join(root, appID) + dir, err := resolveUnder(root, appID) + if err != nil { + fatalHint("invalid_argument", + "app ids are single directory names; try `pilotctl appstore list`", + "invalid app id %q: %v", appID, err) + } info, err := os.Stat(dir) if err != nil { if errors.Is(err, os.ErrNotExist) {