Bug 8745 - disallow rebuild_zebra.pl from executing, when run by root user
Summary: disallow rebuild_zebra.pl from executing, when run by root user
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Galen Charlton
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-10 05:19 UTC by Mason James
Modified: 2014-05-26 21:04 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Addition of "executed by user" check to warn if execute by root user. (1.99 KB, patch)
2012-09-10 14:43 UTC, Barry Cannon
Details | Diff | Splinter Review
Bug 8745 - Disallow rebuild_zebra.pl from executing, when run by root user. (2.87 KB, patch)
2012-11-06 20:53 UTC, Mason James
Details | Diff | Splinter Review
Bug 8745 - Disallow rebuild_zebra.pl from executing, when run by root user. (2.84 KB, patch)
2012-11-16 05:25 UTC, Mason James
Details | Diff | Splinter Review
Bug 8745 - Disallow rebuild_zebra.pl from executing, when run by root user. (2.80 KB, patch)
2013-04-02 09:24 UTC, Mason James
Details | Diff | Splinter Review
Bug 8745 - Disallow rebuild_zebra.pl from executing, when run by root user. (2.78 KB, patch)
2013-04-02 09:35 UTC, Mason James
Details | Diff | Splinter Review
general whitespace and tab tidy (11.21 KB, patch)
2013-04-02 09:37 UTC, Mason James
Details | Diff | Splinter Review
[PASSED QA] Bug 8745 - Disallow rebuild_zebra.pl from executing, when run by root user. (2.80 KB, patch)
2013-04-07 20:31 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 8745: General whitespace and tab tidy (11.46 KB, patch)
2013-04-07 20:31 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mason James 2012-09-10 05:19:37 UTC
many people run rebuild_zebra.pl as root, this often causes zebra reindexing to fail, (sometimes silently)

the Koha mailing-list has many people experiencing problems causes by this

patching rebuild_zebra.pl to quit (with a warning) when run from root user, would stop this

perhaps --force arg could be passed to rebuild_zebra.pl, to allow it to run as root
Comment 1 Barry Cannon 2012-09-10 14:43:26 UTC Comment hidden (obsolete)
Comment 2 Mason James 2012-11-06 20:53:15 UTC Comment hidden (obsolete)
Comment 3 Mason James 2012-11-06 20:54:38 UTC
(In reply to comment #2)
> Created attachment 13274 [details] [review]
> Signed-off-by: Mason James <mtj@kohaaloha.com>

just tested this patch, it works well...

thanks for the patch Barry! :)
Comment 4 Mason James 2012-11-06 21:19:49 UTC
> Added a 'runas-root' switch to allow users to force execution as root user.

just a quickie...

1/ i 'improved' the arg name to --run-as-root

2/ and added a help section for the arg too
Comment 5 Jonathan Druart 2012-11-14 10:30:43 UTC
Hi Mason,

There are debug messages in your SO patch.

Sorry about that, I know it is the second time I bore you with that but I think it is unnecessary to load the Smart::comments module in this patch.
Comment 6 Mason James 2012-11-16 05:25:17 UTC Comment hidden (obsolete)
Comment 7 Mason James 2012-11-16 05:27:51 UTC
(In reply to comment #5)
> Hi Mason,
> 
> Sorry about that, I know it is the second time I bore you with that but I
> think it is unnecessary to load the Smart::comments module in this patch.

i've fixed this, a new patch is attached...
Comment 8 Jonathan Druart 2012-11-16 08:26:29 UTC
This patch cannot work.

the following line is wrong:
    if( undef $run_as_root and $run_user eq 'root')

undef $run_as_root is always false.

You wanted write
    if ( not defined $run_as_root and $run_user eq 'root' )

Marked as Failed QA.
Comment 9 Mason James 2013-04-02 09:24:49 UTC Comment hidden (obsolete)
Comment 10 Mason James 2013-04-02 09:35:38 UTC Comment hidden (obsolete)
Comment 11 Mason James 2013-04-02 09:37:22 UTC Comment hidden (obsolete)
Comment 12 Mason James 2013-04-02 09:39:09 UTC
(In reply to comment #8)
> This patch cannot work.
> 
> the following line is wrong:
>     if( undef $run_as_root and $run_user eq 'root')
> 
> undef $run_as_root is always false.
> 
> You wanted write
>     if ( not defined $run_as_root and $run_user eq 'root' )
> 
> Marked as Failed QA.

Jonathan, thanks for spotting the mistake. i have fixed it now
Comment 13 Mason James 2013-04-02 09:43:19 UTC
(In reply to comment #1)
> Created attachment 12107 [details] [review] [review]
> Addition of "executed by user" check to warn if execute by root user.
> 
> This patch will check the uid of user who executed the script. If it is the
> root user a warning will be issued and die. A user can force "execute as
> root" by using the -runas-root switch.


to test, apply patch...

1/ run script as root user, script should quit

2/ run script as root user with -run-as-root arg, script should *not* quit
Comment 14 Katrin Fischer 2013-04-07 20:31:00 UTC
Created attachment 17266 [details] [review]
[PASSED QA] Bug 8745 - Disallow rebuild_zebra.pl from executing, when run by root user.

Added a check to warn users of execution as root user.
Added a 'runas-root' switch to allow users to force execution as root user.

Signed-off-by: Mason James <mtj@kohaaloha.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 15 Katrin Fischer 2013-04-07 20:31:14 UTC
Created attachment 17267 [details] [review]
[PASSED QA] Bug 8745: General whitespace and tab tidy

http://bugs.koha-community.org/show_bug.cgi?id=8745
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
1) Runs not with root.
2) Runs with root and -run-as-root.
3) Runs using the normal koha user.

Note: Maybe the message should be clear about why
running as root is bad and which user you should
be running the script with?
Comment 16 Jared Camins-Esakov 2013-04-21 13:42:20 UTC
This patch has been pushed to master and 3.12.x.
Comment 17 Chris Cormack 2013-04-27 05:41:18 UTC
Good patch, may break running systems though, so will not push to stable branches.