copilot-cli icon indicating copy to clipboard operation
copilot-cli copied to clipboard

`> /dev/null` and `2>&1` should be special-cased to not require permission

Open andrewneilson opened this issue 3 months ago • 7 comments

Describe the bug

It seems cd is a special case in that you have to approve it every time. This is probably fine, but the issue is that copilot CLI frequently wants to cd into the current directory before running other commands. So even if those commands are already approved, the fact that the cd is there means we still have to approve it.

Here is an example:

 ╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
 │ Check desktop apps files:                                                                                                        │
 │                                                                                                                                  │
 │ ╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ │
 │ │ cd /current/directory && for file in sub/dir/fileA sub/dir/fileB                   │ │
 │ │ sub/dir/fileC; do echo "=== $file ==="; grep -n "NEEDLE" "$file" 2>/dev/null | head -3;  │ │
 │ │ done                                                                                                                         │ │
 │ ╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ │
 │                                                                                                                                  │
 │ Do you want to run this command?                                                                                                 │
 │                                                                                                                                  │
 │ ❯ 1. Yes                                                                                                                         │
 │   2. No, and tell Copilot what to do differently (Esc)                                                                           │
 │                                                                                                                                  │
 │ Confirm with number keys or ↑↓ keys and Enter, Cancel with Esc                                                                   │
 ╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯

And especially note that the approval is one-time.

This happens constantly with many commands so I don't think it has anything to do with the for loop.

Affected version

0.0.334

Steps to reproduce the behavior

No response

Expected behavior

No response

Additional context

No response

andrewneilson avatar Oct 04 '25 23:10 andrewneilson

A better example of something even more innocuous

 ╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
 │ Check if this file exists:                                                                                             │
 │                                                                                                                                  │
 │ ╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ │
 │ │ cd /current/directory && ls -la path/to/file 2>&1                                               │ │
 │ ╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ │
 │                                                                                                                                  │
 │ Do you want to run this command?                                                                                                 │
 │                                                                                                                                  │
 │ ❯ 1. Yes                                                                                                                         │
 │   2. No, and tell Copilot what to do differently (Esc)                                                                           │
 │                                                                                                                                  │
 │ Confirm with number keys or ↑↓ keys and Enter, Cancel with Esc                                                                   │
 ╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯

andrewneilson avatar Oct 04 '25 23:10 andrewneilson

I suspect this isn't to do with cd at all, but really it's the redirection operators that are triggering the prompt. We prompt for confirmation if it looks like the script is trying to write a file using > destination syntax because that could otherwise bypass rules on which directories may be written to.

However we should special-case > /dev/null and 2>&1, as they aren't going to write to a file on disk.

SteveSandersonMS avatar Oct 06 '25 11:10 SteveSandersonMS

Can confirm a simple cd $CWD && do_something_else does not trigger it, it's the output redirection: Image

Going to rename this issue to better reflect the change needed!

RyanHecht avatar Oct 06 '25 15:10 RyanHecht

We have hit this. We have an invocation of Copilot CLI

  copilot --add-dir /tmp/gh-aw/ --log-level all --log-dir /tmp/gh-aw/.copilot/logs/ --allow-tool 'github(download_workflow_run_artifact)' --allow-tool 'github(search_users)' --allow-tool safe_outputs --allow-tool shell --allow-tool web-fetch --allow-tool write --prompt "$COPILOT_CLI_INSTRUCTION" 2>&1 | tee /tmp/gh-aw/agent-stdio.log

During the execution we see this shell command

dotnet tool install --global dotnet-reportgenerator-globaltool 2>&1 || echo "ReportGenerator already installed"

resulting in

Permission denied and could not request permission from user

but then this very similar shell command

export PATH="$PATH:$HOME/.dotnet/tools" && dotnet tool list --global | grep reportgenerator || dotnet tool install --global dotnet-reportgenerator-globaltool

is accepted through the permissions system.

I have access to Copilot CLI source code at GitHub and I set VSCode Agent Mode analyzing this and it said this:

Looking through the onShell handler, I don't see hasWriteApproval being used directly in the shell permission logic! It's only used in the onWrite handler

This might be a bug in the logic. The onShell handler should probably check if write redirections are approved when hasWriteApproval is true.

The issue is that line 428 doesn't consider whether write operations are approved:

This would explain why:

  • First command with 2>&1 fails: hasWriteFileRedirection is true, but the logic doesn't check hasWriteApproval
  • Second command without redirections succeeds: hasWriteFileRedirection is false

The fix would be to modify the condition to also allow approval when write operations are explicitly approved via the --allow-tool write flag.

dsyme avatar Oct 10 '25 20:10 dsyme

We've improved our permissions checking for redirection scenarios in 0.0.351!

RyanHecht avatar Oct 24 '25 22:10 RyanHecht

I believe we're seeing a variant of this too - our code is running Copilot in the background with a -p prompt to accomplish some work.

I'm trying to lock down permissions as responsibly as possible, so we're being quite explicit. The relevant set of flags for this are:

copilot -- additional-mcp-config @<file> --deny-tool write --allow-tool UDB_Server --allow-tool "shell(grep)" --allow-tool "shell(find)" --allow-tool "shell(cat)" --allow-tool "shell(xargs)" --allow-tool "shell(ls)" --allow-tool "shell(head)" -p <prompt>"

What I'm seeing is that Copilot often attempts to run commands such as:

$ find DIRNAME -type f -name "*.c" -path "*/PATTERN/*" 2>/dev/null | head -20

And these seem to consistently fail with permissions issues. I've replicated this by directly asking Copilot to run the command and seen the permissions failure - then removed the 2>/dev/null and seen success.

This use of redirects seems to be common to all the permissions issues we're seen. I note that it's different to the > /dev/null and 2>&1 mentioned in the issue title.

mark-undoio avatar Nov 19 '25 15:11 mark-undoio

@RyanHecht from the change logs on 0.0.351:

Shell redirections like > some_file.txt in paths you've already granted write permissions, > /dev/null, and 2>&1 (Fixes https://github.com/github/copilot-cli/issues/211)

I think the 2>/dev/null pattern should be treated in the same way here.

mark-undoio avatar Nov 19 '25 16:11 mark-undoio