oils-for-unix / oils

Oils is our upgrade path from bash to a better language and runtime. It's also for Python and JavaScript users who avoid shell!
http://www.oilshell.org/
Other
2.85k stars 159 forks source link

`command` treats directories found on the $PATH as commands #2126

Open quexxon opened 3 days ago

quexxon commented 3 days ago

This affects OSH/YSH. Discovered due to a .NET package in my path with a tr directory for translations.

osh-0.24.0$ which cat
/usr/bin/cat
osh-0.24.0$ command -v cat
/usr/bin/cat
osh-0.24.0$ = PATH => split(':')[0]
(Str)   '/home/will/.cargo/bin'
osh-0.24.0$ mkdir ~/.cargo/bin/cat
osh-0.24.0$ osh
osh-0.24.0$ which cat
/usr/bin/cat
osh-0.24.0$ command -v cat
/home/will/.cargo/bin/cat
osh-0.24.0$ cat
  cat
  ^~~
[ interactive ]:3: Can't execute '/home/will/.cargo/bin/cat': Permission denied
quexxon commented 3 days ago

I think the issue is here. That should check for the executable mode bit and check that the file is a regular file. I can take a stab at this.

andychu commented 3 days ago

Oh yes! Thanks for noticing this

That code is for command -v I think, which does need to be changed


And then also executor.py LookupExecutable() needs to be changed

I think that needs the same file check, regardless of exec_required ?

I appreciate any help!


Also let me know if you need help with spec tests

For these it should be fairly satisfying. The tests start in a tempdir, so you can do

mkdir -p cat
PATH=. cat

or something like that

And then you assert status and/or stdout

https://github.com/oils-for-unix/oils/wiki/Spec-Tests

andychu commented 3 days ago

I also use grep to navigate the code and tests, like

grep LookupExecutable */*.py

and

grep 'command -v' spec/*.test.sh

and so forth

There is a cheat sheet here - https://github.com/oils-for-unix/oils/wiki/Oil-Dev-Cheat-Sheet