dachev / node-crontab

A module for creating, reading, updating, and deleting system cron jobs
189 stars 35 forks source link

order the argument for easier sudo management #16

Closed bmeck closed 9 years ago

bmeck commented 9 years ago

this makes using sudoers work well with node-crontab by placing the user argument first always

dachev commented 9 years ago

Hi @bmeck. I'm not very familiar with the sudoers syntax so I'm having a hard time understanding what's the use case. Can you give me an example of what you are trying to achieve.

bmeck commented 9 years ago

a sudoers file manages what commands a user can run using sudo.

It has a syntax of

$USER_RUNNING_SUDO $HOSTNAME_THIS_RULE_APPLIES_TO=$(USER_SUDO_WILL_RUN_AS) $COMMAND

In particular $COMMAND is the command that is allowed to run. It treats $COMMAND as a prefix in order to allow extra argv to be appended onto the end.

With the current syntax we have the following possibly being executed

crontab -l -u $USER
crontab - -u $USER

Knowing this is an implementation detail. Also, you would mean you need 2 separate entries for sudoers if you want to specify crontab for a specific user like the following:

$USER_RUNNING_SUDO $HOSTNAME_THIS_RULE_APPLIES_TO=$(USER_SUDO_WILL_RUN_AS) crontab -e -u $USER, crontab - -u $USER

Which is more error prone than allowing generic access by keeping -u $USER in front of any crontab actions. The current way means you need to know exactly the possible commands node-crontab can spawn.

dachev commented 9 years ago

node-crontab doesn't invoke sudo. Not directly. Are you saying that there is a way sudo will be triggered indirectly through the parent node process?

bmeck commented 9 years ago

we muck with pathing right now, though having a sudo:true option would be nicer. could add it to the PR?

On Tue, Jun 16, 2015 at 4:14 PM, Blagovest Dachev notifications@github.com wrote:

node-crontab doesn't invoke sudo. Not directly. Are you saying that there is a way sudo will be triggered indirectly through the parent node process?

— Reply to this email directly or view it on GitHub https://github.com/dachev/node-crontab/pull/16#issuecomment-112571442.

bmeck commented 9 years ago

Though maybe a generic spawn function as an option would be better than hard coding "sudo"

-----Original Message----- From: "Blagovest Dachev" notifications@github.com Sent: ‎6/‎15/‎2015 9:07 PM To: "dachev/node-crontab" node-crontab@noreply.github.com Cc: "Bradley Meck" bradley.meck@gmail.com Subject: Re: [node-crontab] order the argument for easier sudo management(#16)

Hi @bmeck. I'm not very familiar with the sudoers syntax so I'm having a hard time understanding what's the use case. Can you give me an example of what you are trying to achieve. — Reply to this email directly or view it on GitHub.

dachev commented 9 years ago

@bmeck I still don't understand what is the justification for this feature request. Can you explain what is your use case and why it can't be accomplished without changing the library.

bmeck commented 9 years ago

We want to be able to edit specific users' crontab without running as root. This means we want to use sudo. Right now we cannot have node-crontab use sudo. Our mitigating solution is to put a PATH entry that highjacks crontab and uses sudo with our desires user hardcoded. However this means all attempts are routed though the fake crontab with the hardcoded user. Doing this forces us to change PATH to span and then revert it. Which means we wrap crontab.load and crontab.save with PATH mutation.

To avoid this we can let a custom spawn function invoke sudo based upon node-crontab's spawn arguments.

In order to whitelist behavior with sudo we cannot specify a prefix for crontab of a user because actions come before user in node-crontab's code. If we reorder the arguments we can white list crontab for a user without knowing the specific actions node-crontab will spawn.

If there is a different way to use sudo and not put implementation details in sudoers that would work too.

-----Original Message----- From: "Blagovest Dachev" notifications@github.com Sent: ‎6/‎20/‎2015 5:24 PM To: "dachev/node-crontab" node-crontab@noreply.github.com Cc: "Bradley Meck" bradley.meck@gmail.com Subject: Re: [node-crontab] order the argument for easier sudo management(#16)

@bmeck I still don't understand what is the justification for this feature request. Can you explain what is your use case and why it can't be accomplished without changing the library. — Reply to this email directly or view it on GitHub.

dachev commented 9 years ago

Makes sense. I'll review the pull request and merge during the week. Thanks for your patience.

dachev commented 9 years ago

@bmeck I published a new version that prepends sudo when running for another user and not root. The -u option will always be first. Please try 1.1.0 and let me know if it works for you.

bmeck commented 9 years ago

this should work, still a bit concerned about hard coding sudo into the lib, but works for now