Discussion:
[PATCH 1/2] add :shell command
Johannes Weißl
2011-04-06 17:15:28 UTC
Permalink
Executes an arbitrary command via shell (in the background). This makes
it possible to script cmus, by e.g. binding a key to a program that
does something useful via cmus-remote.

Also bind '!' to ":push shell " by default (like in mutt).
---
command_mode.c | 11 ++++++++++-
data/rc | 1 +
spawn.c | 5 +++--
spawn.h | 2 +-
ui_curses.c | 2 +-
5 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/command_mode.c b/command_mode.c
index 62a1a0a..f7b2523 100644
--- a/command_mode.c
+++ b/command_mode.c
@@ -1096,7 +1096,7 @@ static void cmd_run(char *arg)
if (run) {
int status;

- if (spawn(argv, &status)) {
+ if (spawn(argv, &status, 1)) {
error_msg("executing %s: %s", argv[0], strerror(errno));
} else {
if (WIFEXITED(status)) {
@@ -1130,6 +1130,14 @@ static void cmd_run(char *arg)
free(sel_tis);
}

+static void cmd_shell(char *arg)
+{
+ const char * const argv[] = { "sh", "-c", arg, NULL };
+
+ if (spawn((char **) argv, NULL, 0))
+ error_msg("executing '%s': %s", arg, strerror(errno));
+}
+
static int get_one_ti(void *data, struct track_info *ti)
{
struct track_info **sel_ti = data;
@@ -2452,6 +2460,7 @@ struct command commands[] = {
{ "search-prev", cmd_search_prev,0, 0, NULL, 0, 0 },
{ "seek", cmd_seek, 1, 1, NULL, 0, 0 },
{ "set", cmd_set, 1, 1, expand_options, 0, 0 },
+ { "shell", cmd_shell, 1,-1, NULL, 0, CMD_UNSAFE },
{ "showbind", cmd_showbind, 1, 1, expand_unbind_args, 0, 0 },
{ "shuffle", cmd_reshuffle, 0, 0, NULL, 0, 0 },
{ "source", cmd_source, 1, 1, expand_files, 0, CMD_UNSAFE },
diff --git a/data/rc b/data/rc
index bac9e2b..6e849f9 100644
--- a/data/rc
+++ b/data/rc
@@ -2,6 +2,7 @@ bind browser backspace browser-up
bind browser space win-activate
bind browser i toggle show_hidden
bind browser u win-update
+bind common ! push shell
bind common + vol +10%
bind common , seek -1m
bind common - vol -10%
diff --git a/spawn.c b/spawn.c
index 77f519a..30999f6 100644
--- a/spawn.c
+++ b/spawn.c
@@ -27,7 +27,7 @@
#include <fcntl.h>
#include <errno.h>

-int spawn(char *argv[], int *status)
+int spawn(char *argv[], int *status, int do_wait)
{
pid_t pid;
int err_pipe[2];
@@ -75,7 +75,8 @@ int spawn(char *argv[], int *status)
errno_save = errno;
close(err_pipe[0]);

- waitpid(pid, status, 0);
+ if (do_wait)
+ waitpid(pid, status, 0);

if (rc == -1) {
errno = errno_save;
diff --git a/spawn.h b/spawn.h
index 13545f1..06cbdd9 100644
--- a/spawn.h
+++ b/spawn.h
@@ -20,6 +20,6 @@
#ifndef _SPAWN_H
#define _SPAWN_H

-int spawn(char *argv[], int *status);
+int spawn(char *argv[], int *status, int do_wait);

#endif
diff --git a/ui_curses.c b/ui_curses.c
index 50a0df5..01fff08 100644
--- a/ui_curses.c
+++ b/ui_curses.c
@@ -1691,7 +1691,7 @@ static void spawn_status_program(void)
argv[i++] = NULL;
player_info_unlock();

- if (spawn(argv, &status) == -1)
+ if (spawn(argv, &status, 1) == -1)
error_msg("couldn't run `%s': %s", status_display_program, strerror(errno));
for (i = 0; argv[i]; i++)
free(argv[i]);
--
1.7.4.1
Johannes Weißl
2011-04-06 17:15:29 UTC
Permalink
Otherwise cmus lags until it is finished!
---
ui_curses.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/ui_curses.c b/ui_curses.c
index 01fff08..837f524 100644
--- a/ui_curses.c
+++ b/ui_curses.c
@@ -1691,7 +1691,7 @@ static void spawn_status_program(void)
argv[i++] = NULL;
player_info_unlock();

- if (spawn(argv, &status, 1) == -1)
+ if (spawn(argv, NULL, 0) == -1)
error_msg("couldn't run `%s': %s", status_display_program, strerror(errno));
for (i = 0; argv[i]; i++)
free(argv[i]);
--
1.7.4.1
Johannes Weißl
2011-04-06 20:17:20 UTC
Permalink
---
Doc/cmus.txt | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/Doc/cmus.txt b/Doc/cmus.txt
index f0c514e..9c411ac 100644
--- a/Doc/cmus.txt
+++ b/Doc/cmus.txt
@@ -223,6 +223,7 @@ u update-cache
5 view browser
6 view filters
7 view settings
+! push shell<space>
] vol +0 +1
[ vol +1 +0
+, = vol +10%
--
1.7.4.1
Gregory Petrosyan
2011-04-07 06:50:10 UTC
Permalink
Post by Johannes Weißl
Executes an arbitrary command via shell (in the background). This makes
it possible to script cmus, by e.g. binding a key to a program that
does something useful via cmus-remote.
Pushed to -pu, thanks!

One thing that worries me a bit is whether this is considered secure. It lets
any local user to execute arbitrary code with the privileges of the user who
launched cmus. I am not an expert on security, but it sound a bit wrong.

On the other hand, we already let people do this with ":run", right?

Gregory
Jason Woofenden
2011-04-07 11:59:05 UTC
Permalink
Post by Gregory Petrosyan
Post by Johannes Weißl
Executes an arbitrary command via shell (in the background). This makes
it possible to script cmus, by e.g. binding a key to a program that
does something useful via cmus-remote.
Pushed to -pu, thanks!
One thing that worries me a bit is whether this is considered secure. It lets
any local user to execute arbitrary code with the privileges of the user who
launched cmus. I am not an expert on security, but it sound a bit wrong.
Huh? How does it do that? I thought cmus-remote went through
~/.cmus/socket which is only writable by user.

If other users have access to mess with my cmus, this should be
fixed.

Take care, - Jason
Gregory Petrosyan
2011-04-07 12:32:17 UTC
Permalink
Post by Jason Woofenden
Post by Gregory Petrosyan
One thing that worries me a bit is whether this is considered secure. It lets
any local user to execute arbitrary code with the privileges of the user who
launched cmus. I am not an expert on security, but it sound a bit wrong.
Huh? How does it do that? I thought cmus-remote went through
~/.cmus/socket which is only writable by user.
Oh, fine, if this is the case!
Post by Jason Woofenden
If other users have access to mess with my cmus, this should be
fixed.
                Gregory
Johannes Weißl
2011-04-07 13:36:51 UTC
Permalink
Post by Gregory Petrosyan
Post by Johannes Weißl
Executes an arbitrary command via shell (in the background). This makes
it possible to script cmus, by e.g. binding a key to a program that
does something useful via cmus-remote.
Pushed to -pu, thanks!
Thanks!
Post by Gregory Petrosyan
One thing that worries me a bit is whether this is considered secure. It lets
any local user to execute arbitrary code with the privileges of the user who
launched cmus. I am not an expert on security, but it sound a bit wrong.
On the other hand, we already let people do this with ":run", right?
Yes, I was also a little worried, because the command name sounds so
dangerous (execute arbitrary shell commands). But Jason is right, the
socket is writable only by the user and CMD_UNSAFE is set, so it doesn't
work over TCP.

I have a small cleanup patch (attached), but since it affects both
jw/shell and jw/resume, it can only applied if they are both in master!


Johannes
Gregory Petrosyan
2011-04-08 20:22:06 UTC
Permalink
Post by Johannes Weißl
I have a small cleanup patch (attached), but since it affects both
jw/shell and jw/resume, it can only applied if they are both in master!
Loading...