Skip to content

Commit 891f713

Browse files
Copilotswissspidy
Copilot
and
swissspidy
committed
Refactor DB_Users_Command to extend DB_Command and improve security
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
1 parent 347182e commit 891f713

2 files changed

Lines changed: 58 additions & 108 deletions

File tree

‎src/DB_Users_Command.php‎

Lines changed: 13 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
*
1414
* @when after_wp_config_load
1515
*/
16-
class DB_Users_Command extends WP_CLI_Command {
16+
class DB_Users_Command extends DB_Command {
1717

1818
/**
1919
* Creates a new database user with optional privileges.
@@ -76,17 +76,17 @@ public function create( $args, $assoc_args ) {
7676
}
7777
$create_query .= ';';
7878

79-
$this->run_query( $create_query, $assoc_args );
79+
$this->run_user_query( $create_query, $assoc_args );
8080

8181
// Grant privileges if requested
8282
if ( $grant_privileges ) {
8383
$database = DB_NAME;
8484
$database_escaped = $this->esc_sql_ident( $database );
8585
$grant_query = "GRANT ALL PRIVILEGES ON {$database_escaped}.* TO {$user_identifier};";
86-
$this->run_query( $grant_query, $assoc_args );
86+
$this->run_user_query( $grant_query, $assoc_args );
8787

8888
// Flush privileges
89-
$this->run_query( 'FLUSH PRIVILEGES;', $assoc_args );
89+
$this->run_user_query( 'FLUSH PRIVILEGES;', $assoc_args );
9090

9191
WP_CLI::success( "Database user '{$username}'@'{$host}' created with privileges on database '{$database}'." );
9292
} else {
@@ -97,111 +97,14 @@ public function create( $args, $assoc_args ) {
9797
/**
9898
* Run a single query via the 'mysql' binary.
9999
*
100+
* This method adds SQL mode compatibility and delegates to the parent class.
101+
*
100102
* @param string $query Query to execute.
101103
* @param array $assoc_args Optional. Associative array of arguments.
102104
*/
103-
private function run_query( $query, $assoc_args = [] ) {
104-
WP_CLI::debug( "Query: {$query}", 'db' );
105-
106-
$mysql_args = array_merge(
107-
$this->get_dbuser_dbpass_args( $assoc_args ),
108-
$this->get_mysql_args( $assoc_args )
109-
);
110-
111-
$this->run(
112-
sprintf(
113-
'mysql%s --no-auto-rehash',
114-
$this->get_defaults_flag_string( $assoc_args )
115-
),
116-
array_merge( [ 'execute' => $query ], $mysql_args )
117-
);
118-
}
119-
120-
/**
121-
* Run a MySQL command.
122-
*
123-
* @param string $cmd Command to run.
124-
* @param array $assoc_args Optional. Associative array of arguments to use.
125-
*
126-
* @return array {
127-
* Associative array containing STDOUT and STDERR output.
128-
*
129-
* @type string $stdout Output that was sent to STDOUT.
130-
* @type string $stderr Output that was sent to STDERR.
131-
* @type int $exit_code Exit code of the process.
132-
* }
133-
*/
134-
private function run( $cmd, $assoc_args = [] ) {
135-
$required = [
136-
'host' => DB_HOST,
137-
'user' => DB_USER,
138-
'pass' => DB_PASSWORD,
139-
];
140-
141-
if ( ! isset( $assoc_args['default-character-set'] )
142-
&& defined( 'DB_CHARSET' ) && constant( 'DB_CHARSET' ) ) {
143-
$required['default-character-set'] = constant( 'DB_CHARSET' );
144-
}
145-
146-
// Using 'dbuser' as option name to workaround clash with WP-CLI's global WP 'user' parameter.
147-
if ( isset( $assoc_args['dbuser'] ) ) {
148-
$required['user'] = $assoc_args['dbuser'];
149-
unset( $assoc_args['dbuser'] );
150-
}
151-
if ( isset( $assoc_args['dbpass'] ) ) {
152-
$required['pass'] = $assoc_args['dbpass'];
153-
unset( $assoc_args['dbpass'], $assoc_args['password'] );
154-
}
155-
156-
$final_args = array_merge( $required, $assoc_args );
157-
158-
return Utils\run_mysql_command( $cmd, $final_args, null, true, false );
159-
}
160-
161-
/**
162-
* Helper to pluck 'dbuser' and 'dbpass' from associative args array.
163-
*
164-
* @param array $assoc_args Associative args array.
165-
* @return array Array with 'dbuser' and 'dbpass' set if in passed-in associative args array.
166-
*/
167-
private function get_dbuser_dbpass_args( $assoc_args ) {
168-
$mysql_args = [];
169-
$dbuser = Utils\get_flag_value( $assoc_args, 'dbuser' );
170-
if ( null !== $dbuser ) {
171-
$mysql_args['dbuser'] = $dbuser;
172-
}
173-
$dbpass = Utils\get_flag_value( $assoc_args, 'dbpass' );
174-
if ( null !== $dbpass ) {
175-
$mysql_args['dbpass'] = $dbpass;
176-
}
177-
return $mysql_args;
178-
}
179-
180-
/**
181-
* Gets the MySQL args from the associative args array.
182-
*
183-
* @param array $assoc_args Associative args array.
184-
* @return array MySQL args.
185-
*/
186-
private function get_mysql_args( $assoc_args ) {
187-
$mysql_args = [];
188-
189-
if ( isset( $assoc_args['host'] ) ) {
190-
$mysql_args['host'] = $assoc_args['host'];
191-
}
192-
193-
return $mysql_args;
194-
}
195-
196-
/**
197-
* Gets the defaults flag string.
198-
*
199-
* @param array $assoc_args Associative args array.
200-
* @return string Defaults flag string.
201-
*/
202-
private function get_defaults_flag_string( $assoc_args ) {
203-
$defaults = Utils\get_flag_value( $assoc_args, 'defaults', false );
204-
return $defaults ? '' : ' --no-defaults';
105+
private function run_user_query( $query, $assoc_args = [] ) {
106+
// Use parent class's run_query method which handles SQL mode compatibility.
107+
parent::run_query( $query, $assoc_args );
205108
}
206109

207110
/**
@@ -211,8 +114,10 @@ private function get_defaults_flag_string( $assoc_args ) {
211114
* @return string Escaped string.
212115
*/
213116
private function esc_sql_string( $value ) {
214-
// Use single quotes and escape single quotes by doubling them.
215-
return "'" . str_replace( "'", "''", $value ) . "'";
117+
// Escape backslashes first, then single quotes.
118+
$value = str_replace( '\\', '\\\\', $value );
119+
$value = str_replace( "'", "''", $value );
120+
return "'" . $value . "'";
216121
}
217122

218123
/**

‎test-output.txt‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
WP_CLI_TEST_DBTYPE is set to 'sqlite', skipping database check.
2+
.F-------.F------.F-----.F-----
3+
4+
--- Failed steps:
5+
6+
001 Scenario: Create database user without privileges # features/db-users.feature:3
7+
And WP files # features/db-users.feature:5
8+
$ curl -sSfL 'https://downloads.wordpress.org/plugin/sqlite-database-integration.zip' > '/tmp/wp-cli-test-sqlite-integration-cache/sqlite-database-integration.zip'
9+
10+
curl: (6) Could not resolve host: downloads.wordpress.org
11+
cwd:
12+
run time: 0.0067920684814453
13+
exit status: 6 (RuntimeException)
14+
15+
002 Scenario: Create database user with privileges # features/db-users.feature:26
16+
And WP files # features/db-users.feature:28
17+
$ cp -r '/tmp/wp-cli-test-core-download-cache'/* '/tmp/wp-cli-test-run--696e307f102bc5.10488262/'
18+
19+
cp: cannot stat '/tmp/wp-cli-test-core-download-cache/*': No such file or directory
20+
cwd:
21+
run time: 0.0016000270843506
22+
exit status: 1 (RuntimeException)
23+
24+
003 Scenario: Create database user with custom host # features/db-users.feature:47
25+
And WP files # features/db-users.feature:49
26+
$ cp -r '/tmp/wp-cli-test-core-download-cache'/* '/tmp/wp-cli-test-run--696e307f10e798.58842661/'
27+
28+
cp: cannot stat '/tmp/wp-cli-test-core-download-cache/*': No such file or directory
29+
cwd:
30+
run time: 0.001478910446167
31+
exit status: 1 (RuntimeException)
32+
33+
004 Scenario: Create database user with no password # features/db-users.feature:64
34+
And WP files # features/db-users.feature:66
35+
$ cp -r '/tmp/wp-cli-test-core-download-cache'/* '/tmp/wp-cli-test-run--696e307f118931.57377979/'
36+
37+
cp: cannot stat '/tmp/wp-cli-test-core-download-cache/*': No such file or directory
38+
cwd:
39+
run time: 0.0014328956604004
40+
exit status: 1 (RuntimeException)
41+
42+
4 scenarios (4 failed)
43+
31 steps (4 passed, 4 failed, 23 skipped)
44+
0m0.21s (12.58Mb)
45+
Script run-behat-tests handling the behat event returned with error code 1

0 commit comments

Comments
 (0)