Always pass in the working directory in path_get_cdpath

If the user is in a directory which has been unlinked, it is possible
for the path .. to not exist, relative to the working directory.
Always pass in the working directory (potentially virtual) to
path_get_cdpath; this ensures we check absolute paths and are immune
from issues if the working directory has been unlinked.

Also introduce a new function path_normalize_for_cd which normalizes the
"join point" of a path and a working directory. This allows us to 'cd' out of
a non-existent directory, but not cd into such a directory.

Fixes #5341

src/builtin_cd.cpp

@@ -47,7 +47,7 @@ int builtin_cd(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
         dir_in = maybe_dir_in->as_string();
     }
 
-    if (!path_get_cdpath(dir_in, &dir)) {
+    if (!path_get_cdpath(dir_in, &dir, env_get_pwd_slash())) {
         if (errno == ENOTDIR) {
             streams.err.append_format(_(L"%ls: '%ls' is not a directory\n"), cmd, dir_in.c_str());
         } else if (errno == ENOENT) {
@@ -65,9 +65,7 @@ int builtin_cd(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
         return STATUS_CMD_ERROR;
     }
 
-    // Prepend the PWD if we don't start with a slash, and then normalize the directory.
-    wcstring norm_dir =
-        normalize_path(string_prefixes_string(L"/", dir) ? dir : env_get_pwd_slash() + dir);
+    wcstring norm_dir = normalize_path(dir);
 
     if (wchdir(norm_dir) != 0) {
         struct stat buffer;
@@ -75,10 +73,10 @@ int builtin_cd(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
 
         status = wstat(dir, &buffer);
         if (!status && S_ISDIR(buffer.st_mode)) {
-            streams.err.append_format(_(L"%ls: Permission denied: '%ls'\n"), cmd, dir.c_str());
+            streams.err.append_format(_(L"%ls: Permission denied: '%ls'\n"), cmd, dir_in.c_str());
 
         } else {
-            streams.err.append_format(_(L"%ls: '%ls' is not a directory\n"), cmd, dir.c_str());
+            streams.err.append_format(_(L"%ls: '%ls' is not a directory\n"), cmd, dir_in.c_str());
         }
 
         if (!shell_is_interactive()) {

src/fish_tests.cpp

@@ -4916,6 +4916,19 @@ void test_normalize_path() {
     do_test(normalize_path(L"foo/../foo") == L"foo");
     do_test(normalize_path(L"foo/../foo/") == L"foo");
     do_test(normalize_path(L"foo/././bar/.././baz") == L"foo/baz");
+
+    do_test(path_normalize_for_cd(L"/", L"..") == L"/..");
+    do_test(path_normalize_for_cd(L"/abc/", L"..") == L"/");
+    do_test(path_normalize_for_cd(L"/abc/def/", L"..") == L"/abc");
+    do_test(path_normalize_for_cd(L"/abc/def/", L"../..") == L"/");
+    do_test(path_normalize_for_cd(L"/abc///def/", L"../..") == L"/");
+    do_test(path_normalize_for_cd(L"/abc///def/", L"../..") == L"/");
+    do_test(path_normalize_for_cd(L"/abc///def///", L"../..") == L"/");
+    do_test(path_normalize_for_cd(L"/abc///def///", L"..") == L"/abc");
+    do_test(path_normalize_for_cd(L"/abc///def///", L"..") == L"/abc");
+    do_test(path_normalize_for_cd(L"/abc/def/", L"./././/..") == L"/abc");
+    do_test(path_normalize_for_cd(L"/abc/def/", L"../../../") == L"/../");
+    do_test(path_normalize_for_cd(L"/abc/def/", L"../ghi/..") == L"/abc/ghi/..");
 }
 
 /// Main test.

src/highlight.cpp

@@ -1021,7 +1021,7 @@ static bool command_is_valid(const wcstring &cmd, enum parse_statement_decoratio
 
     // Implicit cd
     if (!is_valid && implicit_cd_ok) {
-        is_valid = path_can_be_implicit_cd(cmd, NULL, working_directory, vars);
+        is_valid = path_can_be_implicit_cd(cmd, working_directory, NULL, vars);
     }
 
     // Return what we got.

src/parse_execution.cpp

@@ -809,7 +809,8 @@ parse_execution_result_t parse_execution_context_t::populate_plain_process(
                 !args.try_get_child<g::redirection, 0>()) {
                 // Ok, no arguments or redirections; check to see if the command is a directory.
                 wcstring implicit_cd_path;
-                use_implicit_cd = path_can_be_implicit_cd(cmd, &implicit_cd_path);
+                use_implicit_cd =
+                    path_can_be_implicit_cd(cmd, env_get_pwd_slash(), &implicit_cd_path);
             }
         }
 

src/path.cpp

@@ -162,7 +162,7 @@ bool path_get_cdpath(const wcstring &dir, wcstring *out, const wcstring &wd,
     int err = ENOENT;
     if (dir.empty()) return false;
 
-    assert(wd.empty() || wd.back() == L'/');
+    assert(!wd.empty() && wd.back() == L'/');
 
     wcstring_list_t paths;
     if (dir.at(0) == L'/') {
@@ -171,10 +171,7 @@ bool path_get_cdpath(const wcstring &dir, wcstring *out, const wcstring &wd,
     } else if (string_prefixes_string(L"./", dir) || string_prefixes_string(L"../", dir) ||
                dir == L"." || dir == L"..") {
         // Path is relative to the working directory.
-        wcstring path;
-        if (!wd.empty()) path.append(wd);
-        path.append(dir);
-        paths.push_back(path);
+        paths.push_back(path_normalize_for_cd(wd, dir));
     } else {
         // Respect CDPATH.
         wcstring_list_t cdpathsv;
@@ -218,7 +215,7 @@ bool path_get_cdpath(const wcstring &dir, wcstring *out, const wcstring &wd,
     return success;
 }
 
-bool path_can_be_implicit_cd(const wcstring &path, wcstring *out_path, const wcstring &wd,
+bool path_can_be_implicit_cd(const wcstring &path, const wcstring &wd, wcstring *out_path,
                              const env_vars_snapshot_t &vars) {
     wcstring exp_path = path;
     expand_tilde(exp_path);

src/path.h

@@ -56,19 +56,17 @@ wcstring_list_t path_get_paths(const wcstring &cmd);
 ///
 /// \param dir The name of the directory.
 /// \param out_or_NULL If non-NULL, return the path to the resolved directory
-/// \param wd The working directory, or NULL to use the default. The working directory should have a
-/// slash appended at the end.
+/// \param wd The working directory, which should have a slash appended at the end.
 /// \param vars The environment variable snapshot to use (for the CDPATH variable)
 /// \return 0 if the command can not be found, the path of the command otherwise. The path should be
 /// free'd with free().
-bool path_get_cdpath(const wcstring &dir, wcstring *out_or_NULL, const wcstring &wd = L"",
+bool path_get_cdpath(const wcstring &dir, wcstring *out_or_NULL, const wcstring &wd,
                      const env_vars_snapshot_t &vars = env_vars_snapshot_t::current());
 
 /// Returns whether the path can be used for an implicit cd command; if so, also returns the path by
 /// reference (if desired). This requires it to start with one of the allowed prefixes (., .., ~)
 /// and resolve to a directory.
-bool path_can_be_implicit_cd(const wcstring &path, wcstring *out_path = NULL,
-                             const wcstring &wd = L"",
+bool path_can_be_implicit_cd(const wcstring &path, const wcstring &wd, wcstring *out_path = NULL,
                              const env_vars_snapshot_t &vars = env_vars_snapshot_t::current());
 
 /// Remove double slashes and trailing slashes from a path, e.g. transform foo//bar/ into foo/bar.

src/wutil.cpp

@@ -466,6 +466,50 @@ wcstring normalize_path(const wcstring &path) {
     return result;
 }
 
+wcstring path_normalize_for_cd(const wcstring &wd, const wcstring &path) {
+    // Fast paths.
+    const wchar_t sep = L'/';
+    assert(!wd.empty() && wd.front() == sep && wd.back() == sep &&
+           "Invalid working directory, it must start and end with /");
+    if (path.empty()) {
+        return wd;
+    } else if (path.front() == sep) {
+        return path;
+    } else if (path.front() != L'.') {
+        return wd + path;
+    }
+
+    // Split our strings by the sep.
+    wcstring_list_t wd_comps = split_string(wd, sep);
+    wcstring_list_t path_comps = split_string(path, sep);
+
+    // Remove empty segments from wd_comps.
+    // In particular this removes the leading and trailing empties.
+    wd_comps.erase(std::remove(wd_comps.begin(), wd_comps.end(), L""), wd_comps.end());
+
+    // Erase leading . and .. components from path_comps, popping from wd_comps as we go.
+    size_t erase_count = 0;
+    for (const wcstring &comp : path_comps) {
+        bool erase_it = false;
+        if (comp.empty() || comp == L".") {
+            erase_it = true;
+        } else if (comp == L".." && !wd_comps.empty()) {
+            erase_it = true;
+            wd_comps.pop_back();
+        }
+        if (erase_it) {
+            erase_count++;
+        } else {
+            break;
+        }
+    }
+    // Append un-erased elements to wd_comps and join them, then prepend the leading /.
+    std::move(path_comps.begin() + erase_count, path_comps.end(), std::back_inserter(wd_comps));
+    wcstring result = join_strings(wd_comps, sep);
+    result.insert(0, 1, L'/');
+    return result;
+}
+
 wcstring wdirname(const wcstring &path) {
     char *tmp = wcs2str(path);
     char *narrow_res = dirname(tmp);

src/wutil.h

@@ -74,6 +74,12 @@ maybe_t<wcstring> wrealpath(const wcstring &pathname);
 /// 3. Remove /./ in the middle.
 wcstring normalize_path(const wcstring &path);
 
+/// Given an input path \p path and a working directory \p wd, do a "normalizing join" in a way
+/// appropriate for cd. That is, return effectively wd + path while resolving leading ../s from
+/// path. The intent here is to allow 'cd' out of a directory which may no longer exist, without
+/// allowing 'cd' into a directory that may not exist; see #5341.
+wcstring path_normalize_for_cd(const wcstring &wd, const wcstring &path);
+
 /// Wide character version of readdir().
 bool wreaddir(DIR *dir, wcstring &out_name);
 bool wreaddir_resolving(DIR *dir, const std::wstring &dir_path, wcstring &out_name,