aboutsummaryrefslogtreecommitdiffstats
path: root/includes/shell/Shell.php
Commit message (Collapse)AuthorAgeFilesLines
* Use MainConfigNames instead of string literals, #2Aryeh Gregor2022-04-131-1/+2
| | | | | | | This covers all occurrences of /onfig->.*get( '/ in includes/. Undoubtedly there are still plenty more to go. Change-Id: I33196c4153437778496f40436bcde399638ac361
* Refactor global variables to use MediaWikiServices insteadTChin2022-01-101-2/+2
| | | | | | | | | | | | Automatically refactors wg prefixed globals to use MediaWikiServices config using Rector. Doesn't include files that set globals or files that fail CI. Rector Gist: https://gist.github.com/tchin25/7cc54f6d23aedef010b22e4dfbead228 * This patch uses a modified source code rector library for our specific use case and the rector will have different effects without it. A writeup for future reference is here: https://meta.wikimedia.org/wiki/User:TChin_(WMF)/Using_Rector_On_MediaWiki Change-Id: I1a691f01cd82e60bf41207d32501edb4b9835e37
* Remove $wgShellLocale, always use CTim Starling2021-09-241-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | $wgShellLocale was a flawed solution to the problem of locale dependence. MediaWiki has its own concept of locale (the Language hierarchy) and any kind of dependence on the server's libc locale is incorrect and harmful, leading to bugs. Developers have an expectation that functions like strtolower() will work in a certain way, and respecting the locale set in the environment at install time violates this expectation. The problems with using C as a locale, which led to $wgShellLocale, are: * escapeshellarg() will strip non-ASCII characters. This can be worked around by not using it. The security vulnerability it was trying to fix can be prevented in another way. * Shell commands like rsvg will fail to correctly interpret UTF-8 arguments. This is the reason for the putenv(). On Linux, this can be fixed by using C.UTF-8, which we didn't know at the time. On Windows, the problem is not relevant (there are unrelated issues with UTF-8 arguments). Bug: T291234 Change-Id: Ib5ac0e7bc720dcc094303a358ee1c7bbdcfc6447
* Use Shellbox for Shell::command() etc.Tim Starling2021-02-051-56/+2
| | | | | | | | | | | | | Make Command extend Shellbox's UnboxedCommand. Only a few MediaWiki- specific features remain in the subclass. Also add BoxedCommand abstraction and Shellbox client. The Result alias didn't actually work, I just had to change the return type hint. Bug: T260330 Change-Id: Iff7428e4c5fe3959a5cda8e113f223caa0976fc1
* Remove terminating line breaks from debug messagesTim Starling2020-06-031-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | A terminating line break has not been required in wfDebug() since 2014, however no migration was done. Some of these line breaks found their way into LoggerInterface::debug() calls, where they mess up the formatting of the debug log. So, remove terminating line breaks from wfDebug() and LoggerInterface::debug() calls. Also: * Fix the stripping of leading line breaks from the log header emitted by Setup.php. This feature, accidentally broken in 2014, allows requests to be distinguished in the log file. * Avoid using the global variable $self. * Move the logging of the client IP back to Setup.php. It was moved to WebRequest in the hopes that it would not always be needed, however $wgRequest->getIP() is now called unconditionally a few lines up in Setup.php. This means that it is put in its proper place after the "start request" message. * Wrap the log header code in a closure so that variables like $name do not leak into global scope. * In Linker.php, remove a few instances of an unnecessary second parameter to wfDebug(). Change-Id: I96651d3044a95b9d210b51cb8368edc76bebbb9e
* Hooks::run() call site migrationTim Starling2020-05-301-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Migrate all callers of Hooks::run() to use the new HookContainer/HookRunner system. General principles: * Use DI if it is already used. We're not changing the way state is managed in this patch. * HookContainer is always injected, not HookRunner. HookContainer is a service, it's a more generic interface, it is the only thing that provides isRegistered() which is needed in some cases, and a HookRunner can be efficiently constructed from it (confirmed by benchmark). Because HookContainer is needed for object construction, it is also needed by all factories. * "Ask your friendly local base class". Big hierarchies like SpecialPage and ApiBase have getHookContainer() and getHookRunner() methods in the base class, and classes that extend that base class are not expected to know or care where the base class gets its HookContainer from. * ProtectedHookAccessorTrait provides protected getHookContainer() and getHookRunner() methods, getting them from the global service container. The point of this is to ease migration to DI by ensuring that call sites ask their local friendly base class rather than getting a HookRunner from the service container directly. * Private $this->hookRunner. In some smaller classes where accessor methods did not seem warranted, there is a private HookRunner property which is accessed directly. Very rarely (two cases), there is a protected property, for consistency with code that conventionally assumes protected=private, but in cases where the class might actually be overridden, a protected accessor is preferred over a protected property. * The last resort: Hooks::runner(). Mostly for static, file-scope and global code. In a few cases it was used for objects with broken construction schemes, out of horror or laziness. Constructors with new required arguments: * AuthManager * BadFileLookup * BlockManager * ClassicInterwikiLookup * ContentHandlerFactory * ContentSecurityPolicy * DefaultOptionsManager * DerivedPageDataUpdater * FullSearchResultWidget * HtmlCacheUpdater * LanguageFactory * LanguageNameUtils * LinkRenderer * LinkRendererFactory * LocalisationCache * MagicWordFactory * MessageCache * NamespaceInfo * PageEditStash * PageHandlerFactory * PageUpdater * ParserFactory * PermissionManager * RevisionStore * RevisionStoreFactory * SearchEngineConfig * SearchEngineFactory * SearchFormWidget * SearchNearMatcher * SessionBackend * SpecialPageFactory * UserNameUtils * UserOptionsManager * WatchedItemQueryService * WatchedItemStore Constructors with new optional arguments: * DefaultPreferencesFactory * Language * LinkHolderArray * MovePage * Parser * ParserCache * PasswordReset * Router setHookContainer() now required after construction: * AuthenticationProvider * ResourceLoaderModule * SearchEngine Change-Id: Id442b0dbe43aba84bd5cf801d86dedc768b082c7
* Coding style: Auto-fix MediaWiki.Usage.IsNull.IsNullJames D. Forrester2020-01-101-1/+1
| | | | Change-Id: I90cfe8366c0245c9c67e598d17800684897a4e27
* Shell: Add more typesMax Semenik2019-10-311-3/+5
| | | | Change-Id: I315f0bb2746ccf7249b8d622a153162dd634ff2e
* Shell: Declare constants visibilityMax Semenik2019-10-281-9/+9
| | | | Change-Id: Ic1285b34fe8ef3efd3d5515e917f4fad7494b9a2
* Upgrade phan config to 0.7.1Daimona Eaytoy2019-09-041-1/+0
| | | | | | | | This allows us to remove many suppressions for phan false positives. Bug: T231636 Depends-On: I82a279e1f7b0fdefd3bb712e46c7d0665429d065 Change-Id: I5c251e9584a1ae9fb1577afcafb5001e0dcd41c7
* Unsuppress phan issues part 6Daimona Eaytoy2019-09-011-0/+2
| | | | | | Bug: T231636 Depends-On: I50377746f01749b058c39fd8229f9d566224cc43 Change-Id: I2cd24e73726394e3200a570c45d5e86b6849bfa9
* shell: annotate return typesMax Semenik2019-04-261-2/+2
| | | | Change-Id: I3ab0a6409088c86581d9d50a340e82b0ea354814
* Use PHP 7 '??' operator instead of if-then-elseFomafix2018-10-211-1/+1
| | | | Change-Id: If9d4be5d88c8927f63cbb84dfc8181baf62ea3eb
* shell: Note that ::isDisabled() should be called before ::command()Kunal Mehta2018-07-021-0/+2
| | | | | | | And check it in the FirejailCommandTest (integration) for completeness, even though it will make no practical difference. Change-Id: Ieb130a888ef8a8162cb0a049ab9c20eac3f58217
* Begin introducing PHP 5.6 variadic parameters where appropriateMax Semenik2018-06-041-10/+8
| | | | Change-Id: I5670b8482e8d3bcb0b3a2b4d2ce9834cfc37e171
* Use PHP7 constant expression instead of a magic numberMax Semenik2018-05-301-12/+11
| | | | Change-Id: I84e13dc6019c429359df3395f0731d17859be06c
* Restrict shell commands by defaultMax Semenik2018-04-161-0/+7
| | | | | | | | | | Before it's too late, let's boil the oceans and just do it. This patch assumes that old code calling wfShellExec() doesn't know about restrictions so it doesn't restrict anything. New code, however, needs to specify its restrictions or deal with defaults. Change-Id: I58963901087202d4a405bcdb6bd12758bb6b0ff7
* Deprecate wfShellWikiCmd()Max Semenik2018-04-161-0/+29
| | | | | | Bug: T184339 Change-Id: Ic86a451e0e9d609e06865a4969560d151efa844c
* Allow programmatic input in CommandGergő Tisza2018-01-031-0/+1
| | | | | Bug: T182463 Change-Id: Ib68180c7af12558686f4864c24fd85f01201d6fb
* shell: Add NO_LOCALSETTINGS restrictionKunal Mehta2017-12-081-2/+9
| | | | | | | | | | | | | | | Most secret information like database passwords are kept in LocalSettings.php, so blacklisting that file by default would take away a lot of information an attacker would want. Since most commands shouldn't need to read the PHP configuration, add it to RESTRICT_DEFAULT. People can still use: $cmd->restrict( Shell::RESTRICT_DEFAULT & ~Shell::NO_LOCALSETTINGS ); if they need to still access LocalSettings.php Bug: T182484 Change-Id: I4032e2706e808e9b819e92a06eff536ccf043388
* Shell: skip null parametersMax Semenik2017-11-291-1/+4
| | | | | | | | | | | | | | | | | | Right now they're treated as empty strings, however this doesn't allow skipping parameters in the middle like $params = [ 'foo', $x ? '--bar' : null, '--baz', ]; In some cases this matters, e.g. `ls` works while `ls ''` doesn't. Also, fix spacing problems the new tests uncovered: * Extra space when using params() * Missing space when combining params() and unsafeParams() Change-Id: Icb29d4c48ae7f92fb5635e3865346c98f47abb01
* shell: Optionally restrict commands' access with firejailKunal Mehta2017-11-281-0/+51
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Introduces a FirejailCommand class, which can be used to add additional restrictions to a command, for increased security. For now, firejail containment needs to be enabled on a per-command basis. The following restrictions are implemented: * NO_ROOT - disallows any root access, including via setuid binaries * SECCOMP - block dangerous syscalls with seccomp * PRIVATE_DEV - create a private /dev * NO_NETWORK - deny all network access * NO_EXECVE - block the execve syscall A convenient Shell::RESTRICT_DEFAULT is equivalent to NO_ROOT | SECCOMP | PRIVATE_DEV, with the expectation that more restrictions may be added to it in the future. In addition, specific paths can be whitelisted with Command::whitelistPaths(). Any file/directory that isn't whitelisted in that top level directory (e.g. /srv) won't exist inside the firejail. $wgShellRestrictionMethod can be set to false for no restriction system, 'firejail' to explicitly use it, or 'autodetect' to autodetect whatever system is available. In the future the default should be changed to autodetection once firejail is tested more. Bug: T173370 Change-Id: Id74df0dbba40e1e7c07c4368aacffb6eb06a17c5
* Remove @codingStandardsIgnore from long linesUmherirrender2017-10-221-2/+0
| | | | | | | | | Breaks some line where the ignore is not needed. The sniff was changed upstream to be okay with long unbreakable lines in comments Change-Id: I2bbe2be7cedd4d3c0ce8dc3e62d0e268bc171876
* Introduce Shell\CommandFactoryMax Semenik2017-10-171-13/+3
| | | | | Bug: T177038 Change-Id: Id875e68ea1fa72b44a463f977ab52270fe1e7088
* Return stderr from Shell\CommandMax Semenik2017-10-121-0/+1
| | | | Change-Id: I5551ae4bbe7b539b528a734aa82198b11f103871
* Inject dependencies into Shell\CommandMax Semenik2017-10-031-3/+18
| | | | | | This slightly changes how execution time limits fall back on each other. Change-Id: I7754a9e6be9638eebe90cb953adb8e2a6ee97cef
* Replace wfShellExec() with a classMax Semenik2017-09-081-0/+149
This function has gotten so unwieldy that a helper was introduced. Instead, here's this class that makes shelling out easier and more readable. Example usage: $result = Shell::command( 'shell command' ) ->environment( [ 'ENVIRONMENT_VARIABLE' => 'VALUE' ] ) ->limits( [ 'time' => 300 ] ) ->execute(); $exitCode = $result->getExitCode(); $output = $result->getStdout(); This is a minimal change, so lots of stuff remains unrefactored - I'd rather limit the scope of this commit. A future improvement could be an ability to get stderr separately from stdout. Caveat: execution errors (proc_open is disabled/returned error) now throw errors instead of returning a status code. wfShellExec() still emulates this behavior though. Competing commit: I7dccb2b67a4173a8a89b035e444fbda9102e4d0f <legoktm> MaxSem: so you should continue working on your patch and I'll probably refactor on top of it later after its merged :P Change-Id: I8ac9858b80d7908cf7e7981d7e19d0fc9c2265c0