Revert "Prevent redirecting internal processes to file descriptors above 2"

FDs are inherited, and redirecting those is harmless, and forbidding
that is worse than allowing all.

Fixes #7769.

This reverts commit 11a373f121.

doc_src/index.rst

@@ -269,8 +269,6 @@ Any arbitrary file descriptor can used in a redirection by prefixing the redirec
 
 For example, ``echo hello 2> output.stderr`` writes the standard error (file descriptor 2) to ``output.stderr``.
 
-It is an error to redirect a builtin, function, or block to a file descriptor above 2. However this is supported for external commands.
-
 .. [#] Previous versions of fish also allowed specifying this as ``^DESTINATION``, but that made another character special so it was deprecated and will be removed in the future. See :ref:`feature flags<featureflags>`.
 
 .. _pipes:

src/parse_execution.cpp

@@ -900,19 +900,14 @@ end_execution_reason_t parse_execution_context_t::populate_plain_process(
             return arg_result;
         }
 
-        // Determine the process type.
-        process_type = process_type_for_command(statement, cmd);
-
-        // Only external commands and exec may redirect arbitrary fds.
-        bool allow_high_fds =
-            (process_type == process_type_t::external || process_type == process_type_t::exec);
-
         // The set of IO redirections that we construct for the process.
-        auto reason =
-            this->determine_redirections(statement.args_or_redirs, allow_high_fds, &redirections);
+        auto reason = this->determine_redirections(statement.args_or_redirs, &redirections);
         if (reason != end_execution_reason_t::ok) {
             return reason;
         }
+
+        // Determine the process type.
+        process_type = process_type_for_command(statement, cmd);
     }
 
     // Populate the process.
@@ -985,8 +980,7 @@ end_execution_reason_t parse_execution_context_t::expand_arguments_from_nodes(
 }
 
 end_execution_reason_t parse_execution_context_t::determine_redirections(
-    const ast::argument_or_redirection_list_t &list, bool allow_high_fds,
-    redirection_spec_list_t *out_redirections) {
+    const ast::argument_or_redirection_list_t &list, redirection_spec_list_t *out_redirections) {
     // Get all redirection nodes underneath the statement.
     for (const ast::argument_or_redirection_t &arg_or_redir : list) {
         if (!arg_or_redir.is_redirection()) continue;
@@ -1014,18 +1008,10 @@ end_execution_reason_t parse_execution_context_t::determine_redirections(
         redirection_spec_t spec{oper->fd, oper->mode, std::move(target)};
 
         // Validate this spec.
-        if (spec.mode == redirection_mode_t::fd && !spec.is_close()) {
-            maybe_t<int> target_fd = spec.get_target_as_fd();
-            if (!target_fd.has_value()) {
-                // Like `cmd >&gibberish`.
-                const wchar_t *fmt =
-                    _(L"Requested redirection to '%ls', which is not a valid file descriptor");
-                return report_error(STATUS_INVALID_ARGS, redir_node, fmt, spec.target.c_str());
-            } else if (*target_fd > STDERR_FILENO && !allow_high_fds) {
-                // Like `echo foo 2>&5`. Don't allow internal procs to write to internal fish fds.
-                const wchar_t *fmt = _(L"Redirection to fd %d is only valid for external commands");
-                return report_error(STATUS_INVALID_ARGS, redir_node, fmt, *target_fd);
-            }
+        if (spec.mode == redirection_mode_t::fd && !spec.is_close() && !spec.get_target_as_fd()) {
+            const wchar_t *fmt =
+                _(L"Requested redirection to '%ls', which is not a valid file descriptor");
+            return report_error(STATUS_INVALID_ARGS, redir_node, fmt, spec.target.c_str());
         }
         out_redirections->push_back(std::move(spec));
 
@@ -1080,8 +1066,7 @@ end_execution_reason_t parse_execution_context_t::populate_block_process(
     assert(args_or_redirs && "Should have args_or_redirs");
 
     redirection_spec_list_t redirections;
-    auto reason =
-        this->determine_redirections(*args_or_redirs, false /* no high fds */, &redirections);
+    auto reason = this->determine_redirections(*args_or_redirs, &redirections);
     if (reason == end_execution_reason_t::ok) {
         proc->type = process_type_t::block_node;
         proc->block_node_source = pstree;

src/parse_execution.h

@@ -130,11 +130,7 @@ class parse_execution_context_t {
                                                        globspec_t glob_behavior);
 
     // Determines the list of redirections for a node.
-    // If \p allow_high_fds is false, then report an error for an fd redirection other than to
-    // in/out/err. This is set when constructing an internal process and prevents writing random
-    // data to internal fish fds.
     end_execution_reason_t determine_redirections(const ast::argument_or_redirection_list_t &list,
-                                                  bool allow_high_fds,
                                                   redirection_spec_list_t *out_redirections);
 
     end_execution_reason_t run_1_job(const ast::job_t &job, const block_t *associated_block);

tests/checks/fds.fish

@@ -23,9 +23,6 @@ $helper print_fds </dev/null
 $helper print_fds 3</dev/null
 # CHECK: 0 1 2 3
 
-$helper print_fds 5>&2
-# CHECK: 0 1 2 5
-
 # This attempts to trip a case where the file opened in fish
 # has the same fd as the redirection. In this case, the dup2
 # does not clear the CLO_EXEC bit.

tests/checks/redirect.fish

@@ -100,35 +100,10 @@ echo $status
 read abc <&-
 #CHECKERR: read: stdin is closed
 
-# This one should output nothing.
-echo derp >&-
 
-# Verify that builtins, blocks, and functions may not write to arbitrary fds.
-echo derp >&12
-#CHECKERR: {{.*}} Redirection to fd 12 is only valid for external commands
-#CHECKERR: echo derp >&12
-#CHECKERR:           ^
-
-begin
-    echo derp
-end <&42
-#CHECKERR: {{.*}} Redirection to fd 42 is only valid for external commands
-#CHECKERR: end <&42
-#CHECKERR:     ^
-
-outnerr 2>&7
-#CHECKERR: {{.*}} Redirection to fd 7 is only valid for external commands
-#CHECKERR: outnerr 2>&7
-#CHECKERR:          ^
-
-# Redirection to 0, 1, 2 is allowed. We don't test 0 since writing to stdin is weird and unpredictable.
-echo hooray1 >&1
-echo hooray2 >&2
-#CHECK: hooray1
-#CHECKERR: hooray2
 
 # "Verify that pipes don't conflict with fd redirections"
-# This code is very similar to eval. We go over a bunch of fds
+# This code is very similar to eval. We go over a bunch of fads
 # to make it likely that we will nominally conflict with a pipe
 # fish is supposed to detect this case and dup the pipe to something else
 echo "/bin/echo pipe 3 <&3 3<&-" | source 3<&0
@@ -138,6 +113,9 @@ echo "/bin/echo pipe 6 <&6 6<&-" | source 6<&0
 echo "/bin/echo pipe 7 <&7 7<&-" | source 7<&0
 echo "/bin/echo pipe 8 <&8 8<&-" | source 8<&0
 echo "/bin/echo pipe 9 <&9 9<&-" | source 9<&0
+echo "/bin/echo pipe 10 <&10 10<&-" | source 10<&0
+echo "/bin/echo pipe 11 <&11 11<&-" | source 11<&0
+echo "/bin/echo pipe 12 <&12 12<&-" | source 12<&0
 #CHECK: pipe 3
 #CHECK: pipe 4
 #CHECK: pipe 5
@@ -145,3 +123,6 @@ echo "/bin/echo pipe 9 <&9 9<&-" | source 9<&0
 #CHECK: pipe 7
 #CHECK: pipe 8
 #CHECK: pipe 9
+#CHECK: pipe 10
+#CHECK: pipe 11
+#CHECK: pipe 12