Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shellcode for fish #838

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Shellcode for fish #838

wants to merge 3 commits into from

Conversation

mxcl
Copy link
Member

@mxcl mxcl commented Oct 27, 2023

Refs #704

@coveralls
Copy link

Pull Request Test Coverage Report for Build 6667484083

  • 15 of 27 (55.56%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.7%) to 96.222%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/modes/shellcode.ts 15 27 55.56%
Totals Coverage Status
Change from base Build 6653611079: -0.7%
Covered Lines: 1392
Relevant Lines: 1441

💛 - Coveralls

@mxcl mxcl mentioned this pull request Oct 27, 2023
3 tasks
Comment on lines 233 to 236
echo "#!/bin/sh" > "$d/x.$fish_pid"
echo -n "exec " >> "$d/x.$fish_pid"
echo "$argv" >> "$d/x.$fish_pid" #FIXME needs quoting :/
chmod u+x "$d/x.$fish_pid"
Copy link

@Lythenas Lythenas Oct 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use a fish script instead of an sh script, you should be able to use something like this:

echo -n "exec " > "$d/x.$fish_pid"
for arg in $argv
    string escape $arg >> "$d/x.$fish_pid"
    echo -n " " >> "$d/x.$fish_pid"
end

And then in the definition of x call the script with fish "${tmp}/shellcode/x.$fish_pid". Or use a fish shebang. Not sure which is better.

I also tried to implement fish support here: https://github.com/Lythenas/pkgx/tree/fish-support I basically just translated the posix code, and since then I think there were changes of the shellcode in pkgx. But feel free to use that as a reference if it is useful. I didn't open a PR because I wasn't confident that my code is correct and I also didn't really like the implementation (the --fish flag and the way I built the shellcode).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you this is very useful!

@FlatMapIO
Copy link

@mxcl Don't forget this :)

@yoroshikun
Copy link
Contributor

Very eager for this~

@mxcl mxcl force-pushed the main branch 3 times, most recently from 7947ae5 to 533350e Compare January 5, 2024 13:22
@joshmedeski
Copy link

joshmedeski commented Jan 18, 2024

@mxcl let us know if you need any testers for this! Looking forward to seeing this merged.

@mxcl
Copy link
Member Author

mxcl commented Jan 18, 2024

I think I need to refactor the shellcode and other related pieces before this will be maintainable. I don't use fish (only because in general I stick with defaults so that I know what most of my users deal with on a day to day basis) and as a result it would be easy for it to end up breaking unless the underlying infra is solid.

Also not knowing fish it's hard to write. I am mostly using ChatGPT to translate.

@netzhuffle
Copy link

@mxcl Do you have anything in specific about how I might be able to help?

@joshmedeski
Copy link

@mxcl I've got fish scripting experience too, let us know if you want support.

@amok
Copy link

amok commented Jan 18, 2024

I'm at your service too, @mxcl. Looking forward to fish support

@mxcl mxcl force-pushed the main branch 2 times, most recently from 9038067 to 342373c Compare April 23, 2024 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants