Page MenuHomePhabricator

InitialiseSettings.php and a few of other files are executable
Closed, ResolvedPublic

Description

wmf-config/InitialiseSettings.php in operations/mediawiki-config.git is executable (has mode 755). This should be changed (and investigated why this happened).

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 436988 had a related patch set uploaded (by Urbanecm; owner: Urbanecm):
[operations/mediawiki-config@master] Change mode of IS.php to 644

https://gerrit.wikimedia.org/r/436988

As rOMWC has a composer.json file, should MinusX be added so this can be checked by the tests? It should probably be run as part of the operations-mw-config-composer-test-docker test.

Urbanecm triaged this task as Medium priority.Jun 2 2018, 1:48 PM
Urbanecm lowered the priority of this task from Medium to Low.Jun 2 2018, 2:53 PM

@Mainframe98 If you know how to do it, feel free to! I think that this is very good idea.

(and investigated why this happened).

git log -p InitialiseSettings.php | grep --before-context 7 "old mode" lists mode changes.
Last one was https://gerrit.wikimedia.org/r/#/c/404911/4/wmf-config/InitialiseSettings.php which changed the mode from 644 to 755.

Running minus-x shows that a total of four files within the repository should not be executable (according to minus-x):

mediawiki-config/wmf-config/InitialiseSettings.php should not be executable
mediawiki-config/rpc/RunJobs.php should not be executable
mediawiki-config/rpc/RunSingleJob.php should not be executable
mediawiki-config/docroot/noc/conf/InitialiseSettings.php.txt should not be executable

I'll prep a patch that will add minus-x.

Running minus-x shows that a total of four files within the repository should not be executable (according to minus-x):

mediawiki-config/wmf-config/InitialiseSettings.php should not be executable
mediawiki-config/rpc/RunJobs.php should not be executable
mediawiki-config/rpc/RunSingleJob.php should not be executable
mediawiki-config/docroot/noc/conf/InitialiseSettings.php.txt should not be executable

I'll prep a patch that will add minus-x.

Are you sure that mediawiki-config/rpc/RunSingleJob.php and mediawiki-config/rpc/RunJobs.php are not to be executable? I don't know, I'm just asking.

(and investigated why this happened).

git log -p InitialiseSettings.php | grep --before-context 7 "old mode" lists mode changes.
Last one was https://gerrit.wikimedia.org/r/#/c/404911/4/wmf-config/InitialiseSettings.php which changed the mode from 644 to 755.

Thank you.

Running minus-x shows that a total of four files within the repository should not be executable (according to minus-x):

Are you sure that mediawiki-config/rpc/RunSingleJob.php and mediawiki-config/rpc/RunJobs.php are not to be executable? I don't know, I'm just asking.

According to Minus-X documentation, if they should have been executable, they should contain a shebang (#!) on the first line. The files also look like they should be executed through a web server.
If I'm wrong, then all that should be done is to add that shebang. All the other executable files do have a shebang, so based on that, I'm assuming that they shouldn't be executable.

Change 436994 had a related patch set uploaded (by Mainframe98; owner: Mainframe98):
[operations/mediawiki-config@master] Add Minus-X to check against files that shouldn't be executable

https://gerrit.wikimedia.org/r/436994

Yeah, that's true, I forgotted about that sequence. Thank you for explanation.

Urbanecm renamed this task from InitialiseSettings.php is executable to InitialiseSettings.php and a few of other files are executable.Jun 2 2018, 3:38 PM

@Mainframe98 I've changed the scope of this task so other files than IS.php that should not be executable but are are included. I also amended my commit and added my commit as an dependency to your commit, so jenkins is satisfied and tests are passing. Thanks!

I'm wondering how to deploy this. If it is just CR+2 ed, will it matter during deployment of other changes? Anybody who can answer this question?

@Urbanecm Excellent, thank you!

I browsed https://wikitech.wikimedia.org/wiki/SWAT_deploys, which states

Allowed types of patches Things not fitting these criteria should instead use the standard deploy window process

  • Fixes of regressions
  • Simple config changes (that don't turn on any new features)
  • Everything should be already committed into master and tested on mw:Beta Cluster and then backported to the relevant release branch

So maybe it should just be scheduled for a regular SWAT?
If that happens to be the case, can you ensure that the patch gets SWAT'ted? You've got more experience than I do, and I won't be available at the SWAT windows given anyways.

I think you are right, it should be SWATed. Sure, I'll include it as soon as I will be looking for a SWAT window to use.

Urbanecm added a subscriber: zeljkofilipin.

The patch was scheduled for SWAT but was vetoed by SWAT team member (@zeljkofilipin). He didn't feel comfortable with deploying and said that he want somebody more experienced to do it (or at least, approve it by CR+1 flag). I will take care about review from such experienced deployers and schedule it again later. Thank you @Mainframe98 for your help!

Change 436988 merged by jenkins-bot:
[operations/mediawiki-config@master] Change mode of IS.php and a few of other files to 644

https://gerrit.wikimedia.org/r/436988

Change 436994 merged by jenkins-bot:
[operations/mediawiki-config@master] Add Minus-X to check against files that shouldn't be executable

https://gerrit.wikimedia.org/r/436994

Vvjjkkii renamed this task from InitialiseSettings.php and a few of other files are executable to msbaaaaaaa.Jul 1 2018, 1:06 AM
Vvjjkkii reopened this task as Open.
Vvjjkkii removed Urbanecm as the assignee of this task.
Vvjjkkii raised the priority of this task from Low to High.
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii removed subscribers: gerritbot, Aklapper.
Mainframe98 renamed this task from msbaaaaaaa to InitialiseSettings.php and a few of other files are executable.Jul 1 2018, 7:02 AM
Mainframe98 closed this task as Resolved.
Mainframe98 assigned this task to Urbanecm.
Mainframe98 lowered the priority of this task from High to Low.
Mainframe98 updated the task description. (Show Details)
Mainframe98 edited subscribers, added: Aklapper, gerritbot; removed: Dereckson.