I've been asked to work on a patch for drush to support doing just security updates. This would be useful for automating security updates. Now as it stands, you have to pick and choose through the available updates manually to apply security updates. With this new command, you could apply only security-related updates.

I see code in pm.drush.inc that leads me to believe this would be straightforward using the release information from drush_get_projects().

I'm not sure if this should be a flag for up/upc or a new command (drush pm-updatesec (ups)). My feeling is a new top-level command since you wouldn't want to bury the option in the help text. Then again, there are already quite a few built in commands and it's getting crowded.

Is anyone working on this already - I didn't see anything obvious in the queue. Feedback on idea and function needed before I proceed, thanks.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

greg.1.anderson’s picture

This should be a flag to pm-updatecode. To make it more obvious, include an example for it in the updatecode help text.

The existing code already includes data indicating which of each release is supported, recommended, development and/or security. See the implementation of pm-releases for more info.

It should be pretty easy to add this filter to pm-updatecode.

ChrisBryant’s picture

I was just thinking that this was needed after the all security advisories today and sure enough there is already and issue here for it. :-) Implementing it as a pm-updatecode flag certainly makes the most sense, and maybe with an alias to something like pm-updatesec or ups.

greg.1.anderson’s picture

At the moment, drush does not support the ability to alias one command to be another command with an option added, but you could use the bash alias command to do this.

danielpacker’s picture

I'm sorry to report that the client changed their mind at the last minute and this feature was never worked on.

webkenny’s picture

This actually is possible assuming you're on a *nix platform or using CygWin on Windows:

Use drush upc to pass the output it produces to grep, cut just the package name from the produced list, and finally pass the list (as one line) back to the drush package manager.

Note: When performing drush actions, it's often helpful to use the -u 1 argument to present yourself as the administrative user.

drush upc -u 1 --pipe | grep 'SECURITY-UPDATE' | cut -d" " -f1 | xargs drush upc -u 1 -y

The -y argument on the end of the command tells Drush to answer "yes" to all prompts. Change this to -n for a Dry Run of the functionality.

You will need to run database updates following this action. Obviously, while it should never be done on a production site, you can easily change upc to up on the end to force it to run DB updates.

pdcarto’s picture

Just tried this in OSX and got "xargs: drush: No such file or directory"
I'm fairly new to drush. Is this probably a fault in my drush setup?

webkenny’s picture

This is assuming you have drush in your path or it's set as an alias.

SeanBannister’s picture

@webkenny Thanks for that, very handy. I'm still keen on seeing this feature in Drush so its easy to apply to multiple sites by using an alias.

webkenny’s picture

I agree. This should be something we can flag with upc. e.g. --security-updates-only - The above is a temp bandage for the barrage of SAs we got the other day. But I'm very interested in this and I'm going to have a look myself as well to see how I can assist.

greg.1.anderson’s picture

If you want to help, inspect #446736: Have drush update be told to ignore a module/theme -- simple solution, which has very similar logic to what would be needed here. Especially see the patch in #53 (already committed).

TravisCarden’s picture

+1 for this feature. +1 for implementing it as a flag rather than a separate command. Good work, all!

webkenny’s picture

Assigned: danielpacker » webkenny

Great, then I'll take this on. Looks like much of the code in #446736 which was committed, applies here with some tweaking. I'll give it a crack this week.

ice5nake’s picture

Thanks for coming up with this but it doesn't behave if there are no security updates.

webkenny’s picture

@iceSnake, PM me and I'll work it out with you. Just give me some more details surrounding what "doesn't behave" means. Thanks.

@all, Still working on this. I will get a patch going shortly.

greg.1.anderson’s picture

Regarding #13, if there are no security updates, your script will reduce to: drush upc -u 1 -y, which will update everything. To avoid this, run drush upc -n first, and only run your script if there are security updates. Better to implement the feature in drush than to fix the workaround, imo.

webkenny’s picture

Greg, agreed. I only posted that as a band-aid. Still working this. Hope to have a patch up shortly. Sorry for the delay. Damn full time jobs. ;)

tim.plunkett’s picture

Component: Code » PM (dl, en, up ...)
Status: Active » Needs review
FileSize
1.24 KB

Using #446736: Have drush update be told to ignore a module/theme -- simple solution as an inspiration, this is what I came up with.
Needs comments, but the flag would be --security-only.

I'll upload with the rest tomorrow.

greg.1.anderson’s picture

Didn't test, but the code looks pretty good to me...

damien_vancouver’s picture

Status: Needs review » Reviewed & tested by the community

I tested the patch against latest drush HEAD and it worked great!

My only suggestion for a possible addition would be some sort of output saying that the --security-only option had been selected.

Other than the list of packages to update being just the ones that have security updates, there is no visual feedback the option is being used. It would be nice if it was displayed somewhere.

However it works great, this is a super useful switch to have, for when you are just trying to keep sites from being insecure, without necessarily having to do feature or functionality updates.

moshe weitzman’s picture

Assigned: webkenny » jonhattan

looks useful. lets document the new option. assigning to jonhattan for eventual review.

tim.plunkett’s picture

Assigned: jonhattan » tim.plunkett
Status: Reviewed & tested by the community » Needs work

Yeah, I only had 30 minutes at the end of the day to work on this, I'll finish it up tomorrow morning.

jonhattan’s picture

I understand this as an option to drive almost automatic updates without 'having to do feature or functionality updates' as expresed in #19. So what if you're using version n of a module, n+1 is available, you don't update for a time, and n+2 is released as a security update? Should we just update to n+2 or warn the user there're some versions in the middle that are not a security related? I'm not tight to this, just realized it.

Code needs:
1. help message for the new option.
2. #19 again: inform the user only security updates are taking place. Perhaps include a note saying some updates not related to security will take place (if there's more than a releases version in between).
3. it doesn't take effect if --pipe. It's better to do the check in pm_project_filter().
4. use drush_get_option() instead of drush_get_option_list().

damien_vancouver’s picture

@jonhattan: Your #2 about updates in between is a very good point, if updating older sites!

My use case is from the point of view of a server administrator faced with a keeping a bunch of sites reaching their golden years secure. By golden years, I mean that the site is older, the developers, maybe even the site's champion at client have mostly moved on, and its budget has likely been exhausted. So using --security-only it's not really an attempt to do it "quicker" or totally automatically, but rather a way of changing only what is required so things are still secure. From this perspective, then being told about multiple versions in the middle would be very helpful..... It would tell me in that case to pay extra attention to testing the identified module's functionality, because other things might have broken or changed. Which is going to help me testing, since I don't know the site as intimately as the original development team.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
2.47 KB

1) Done.
2) Half done. I'm not sure if you want a generic note, or if it's actually supposed to check the difference between release versions?
3) This confused me. What is the desired result for drush update --security-only --pipe?
4) Done.

With --security-only, you'd still want to see the other project information, right? Otherwise I could change the unset() to a continue, which would make more sense for --pipe, but not in general.

greg.1.anderson’s picture

Status: Needs review » Needs work

Looking pretty good.

If there are code updates available, but none of them are security updates, then drush will say "There are no code updates available." It should say there are no security updates available if there are updates available, but they are not being applied due to --security-only..

Similarly, drush also will say "NOTE: A code update for the Drupal core is available." It would be nice if it said that there is a security update for core.

Your messages about security updates vs. code updates are keyed off of whether the --security-only flag is present. It would be nice if the message was accurate even in the default case, when --security-only is not specified (so if you run w/out --security-only and you see the message "There are code updates..." without "security" in it, you can just abort.

To answer your question about #3, here is an example of what it does today:

$ drush @dfdev pm-updatecode --pipe --security-only
drupal 6.16 6.19 SECURITY-UPDATE-available
cck 6.x-2.6 6.x-2.8 SECURITY-UPDATE-available
diff 6.x-2.1-alpha3 6.x-2.1 Update-available
fckeditor 6.x-2.1 6.x-2.1 Up-to-date
features 6.x-1.0-beta6 6.x-1.0 Update-available
etc.

Without --security-only, it shows each module's status in terms of whether it is up to date, has a code update available, or has a security update available. I'm not sure that I agree with jonhattan's advice, which is to make the above equivalent to pm-updatecode --pipe | grep "SECURITY-UPDATE". My fear is that if you do as he advises, then the modules that are up-to-date or that only have non-security updates may also be suppressed in the non-"--pipe" case. Perhaps the filtering should only be done if --security-only AND --pipe are specified? I'm ambivalent.

I don't really think that #2 is necessary. After all, there are likely non-security changes in any release that is marked as a security update.

My opinions only; jonhattan is the maintainer of this section.

tim.plunkett’s picture

Moving the check from drush_pm_updatecode() to pm_project_filter() didn't really change anything, because I wasn't sure how --pipe and --security-only should interact. If it should act as a grep, then I can do that.

I agree with not having extra warnings about non-security changes, I think the warning in the help is enough.

Greg, I'm confused by your comments here:

It would be nice if the message was accurate even in the default case, when --security-only is not specified (so if you run w/out --security-only and you see the message "There are code updates..." without "security" in it, you can just abort.

I'm not sure what you mean. Good point about the "no security updates" part though.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
3.63 KB

With this patch, drush upc --pipe --security-only now behaves like drush upc --pipe | grep "SECURITY-UPDATE", without interfering with the regular functionality.

Unrelated to that, there is one edge case to consider:
If Drupal core is the only security update, but the other projects have regular updates pending, drush upc --security-only -y will never update anything (because updating core requires other modules to be up to date).

greg.1.anderson’s picture

Status: Needs review » Needs work

The restriction that the other modules be up-to-date is artificial; the only actual requirement is that Drupal be updated separately from the modules. It is already possible to update Drupal prior to updating modules via drush upc drupal. You could fix your edge case by modifying the test, and allow Drupal to update in the --security-only case if there are no pending security updates in any module.

My other comment was related to this portion of the patch:

+  if (drush_get_option('security-only')) {
+    drush_print(dt('Security updates will be made to the following projects:'));
+  }
+  else {
+    drush_print(dt('Code updates will be made to the following projects:'));
+  }

It would be nice if that 'if' statement was based on the existence of at least one project where $project['status'] == UPDATE_NOT_SECURE. In this case, the message would either be "Security updates will be made...", "Security and code updates will be made..." or "Code updates will be made...", again, based only on $project['status'] values, not the --security-only setting. Optional.

Beyond that, the patch is looking great.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
4.65 KB

Turns out I managed to fix that edge case without even knowing it. With the --security-only flag, all non-security updates are removed from $updateable, which is the array that's checked before updating core.

Changing the strings for Drupal core was easy enough, but I'm not sure that the Security/Security and code/Code section is done in the best way. Any ideas, or is that good enough?

Also, is --security-only a decent name for the flag? Not to start a meta-discussion, but I'd rather resolve that sooner than later. --security and --security-update were my other choices.

greg.1.anderson’s picture

Status: Needs review » Reviewed & tested by the community

#29 works for me, and I'm satisfied with it the way it is. I think you've taken care of jonhattan's #1-4, but I leave it to him to confirm and commit.

jonhattan’s picture

Status: Reviewed & tested by the community » Fixed

Thanks all. Commited with some changes to make the code a bit more compact. http://drupal.org/cvs?commit=455186

Greg: I don't understand if you prefer --pipe to ignore --security-only.
I think it shouldn't be ignored, as you're explicitly asking drush to do --security-only, but I'm not so tight on this. I don't use --pipe at all..

tim.plunkett’s picture

Assigned: tim.plunkett » Unassigned
Status: Fixed » Active

A thought just occurred to me. Let's say you're running a module verion 6.x-1.1, and 6.x-1.2 is a security update, but there has been a new feature releases 6.x-1.3. Even with the new flag, it will update to 6.x-1.3, when ideally it would only update to 6.x-1.2.

Is that too much hand-holding? And if not, should I open a new issue or continue here?

greg.1.anderson’s picture

Status: Active » Fixed

@tim: I don't think that the situation with #32 is too severe. As I mentioned above, even if there is only one release w/ the 'security update' flag, it is still possible that you might get a non-security-related change in the module. Therefore I do not think it is too bad if you get "extra" releases with your security update. The main benefit of --security-only is that you do not need to spend the time to upgrade and test a release unless there is a security change involved. Most people will not test all changes of every release they upgrade; they will just test to see if any features of their site broke in the upgrade. All the same, your point in #32 is valid in that in some rare circumstances, it might just be possible that that 6.x-1.2 works, but 6.x-1.3 breaks, so your proposed patch would save our hypothetical user -- at least until the 6.x-1.4 security update comes along, and 6.x-1.3 gets pulled in anyway.

But patches are welcome if you still feel inclined to fix it. I'm setting this back to 'fixed' because I feel that this enhancement is optional, and therefore better handled in a separate issue, mostly in case the patch never gets written. I wouldn't mind seeing a patch here if anyone preferred to do it that way, though.

@jonhattan: I think that the current implementation of --security-only --pipe is good. I was just worried that the fix might break the non-pipe case by suppressing updatecode's normal update. Since it does not, I am happy, and the current implementation (works like grep) makes more sense than ignoring the --security-only flag. I would only advocate the later if supporting --security-only --pipe would break something else.

webkenny’s picture

Status: Needs work » Fixed

This is fantastic and works as expected for me as well. Thanks for taking this on, folks. I got totally blown away with work recently and couldn't follow through. Myself AND the entire Client Advisory Team at Acquia (who will use this to death) thank you!

[Edit: Posted way too soon. See below]

webkenny’s picture

Status: Fixed » Needs work

While this does work as expected, I don't think it works as intended if that makes sense. I have to agree with #32. I think it's not hand-holding so much as complete defeat of the intended design.

Pulling in past the security release could be a problem. In proper maintainability practice, a security release should be completely abstracted from feature releases. So if someone releases a 1.2 (a simple security fix) and it's immediately followed by a 1.3 (bug fixes/feature adds) the flag could just as easily read: --pull-in-random-updates-from-modules-which-at-some-point-have-had-a-security-release (Thanks to a colleague for that distinction)

Setting this to needs work. I'll see how I can contribute to tightening it up a little bit. The basic principles are already there. We just need to find a way to only go up to the SA.

greg.1.anderson’s picture

Patches welcome, of course, but note that if it goes the other way, that is, someone releases a bug/feature 1.2 followed later by a 1.3 security release, then you are going to pull in 1.2 when you get 1.3.

I'm marking this as "fixed", not because I reject your idea, but because I want to make sure that this issue is included in the release notes if drush-4 goes out before your patch is ready. Please continue work in a new issue -- unless this fix is deemed required for drush-4, in which case priority=major, status=needs work is appropriate.

greg.1.anderson’s picture

I'm crazy; release notes are generated from cvs commit messages, not issues from the queue. Go ahead and continue work here, or open a new issue; either way, I don't care.

webkenny’s picture

No worries, Greg. Thanks. I plan to dig in this weekend. Considering it might be better to allow users to choose. (e.g. all the way up to the next release? or just the security?) - Still kickin' it around. I'll know more in a couple of days.

greg.1.anderson’s picture

I think that the UI would be too complicated to allow users to choose; with multiple modules potentially being updated at the same time, the confirmation messages required would probably be more annoying than useful. The user can always run pm-updatecode multiple times with different flags and different module names, so I think that the patch should only inform the user if --security-only selected a project where there is also a newer release available. Then the user could run pm-updatecode again w/out --security-only for just that release if they wanted to get it.

Alternatively, there could be one more flag like --lax that allowed --security-only to work as it currently does (grab recommended release iff a security update is available) instead of how it will work (take only the most recent security release).

tim.plunkett’s picture

Assigned: Unassigned » tim.plunkett
Status: Fixed » Active

If anyone want to take a crack at this over the weekend, feel free, but I hope to work on this next week.
Something like

if (-security-only) {
  if (--lax) {
    update to recommended release
  }
  else {
    update to latest security release
  }
}
jonhattan’s picture

I'm with #36 and other previous comments. Later or sooner you will need to address non-security releases previous to a sec release, and you can also force at hand to update to a security release.

So the value of adding two dozen new lines to updatecode to allow auto updating to the latest sec release instead of latest release supposing you're in 1.2; 1.3 is a sec release and 1.4 is a new features release doesn't seem to make worth it. It is only useful to postpone the update to 1.4, until 1.x comes with a new security fix. Well perhaps it is a very good module and it only have a security release per year :)

greg.1.anderson’s picture

@tim.plunkett: n.b. jonhattan is the maintainer of this section, so it's up to him whether this patch is accepted.

webkenny’s picture

IMO, it depends on the definition of useful. I would argue the entire reason behind having a "security updates only" flag is partly that administrators who's job it is to keep sites at least secure don't have to also worry about pulling in new features and bug fixes when running automated scripts to meet those needs or, more commonly, jumping from terminal to terminal, verifying, then testing, etc.

That doesn't only apply to administrators with the job, but also any site builder who's only interested in keeping sites up to date (with the 1.3s) until they have adequate time to test the 1.4s of the world. Security announcements are imminent, especially considering they are released to a broad spectrum so it's critical site owners get on those as quickly as they can and not have to worry about the other stuff. After all, anyone can sign up for those SAs with a d.o account. Even those who might have bad intentions. :(

I agree with the points made that it works backwards (where 1.3 is fixes, 1.4 is security) but the problem is implied and not created. Whereas with jumping the security release and saying, "Hey we're taking all of this with us too" does create it.

Either way, I'm pleased to see we got this far where at least we can get only those modules who have had a security update in upc so none of this argument devalues that. I'm just trying to keep as little breakage introduced as possible but then again, remember folks, I support Drupal full time so I am really biased. ;)

Great work, anyway. I will put forward a patch and if we decide not to go with it, cool. At least I'll get a little drush source code training in the process. Heaven knows I could use it.

tim.plunkett’s picture

In principle, I wholeheartedly agree with #36 and #42. But for maintaining many sites, it could be useful, and is worth consideration. If I can come up with an elegant solution, I'll post it, otherwise I'll remark as fixed.

jonhattan’s picture

any advance?

moshe weitzman’s picture

I actually agree that many folks really only want to update as far as the sec release, and no farther.

tim.plunkett’s picture

Assigned: tim.plunkett » Unassigned
Status: Active » Fixed

I took a couple half-hearted stabs at this, nothing really came of it.
Marking back to fixed as per #31 (and #33 and #36).

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.