From 3c24dbfb2a2b19571b71493f9ae8e6041c0ff20a Mon Sep 17 00:00:00 2001 From: gdh Date: Thu, 12 Apr 2018 15:30:29 +0000 Subject: [PATCH 001/142] Bare-bones hashes-to-hashes database integration Not tested, not usable. Still an idea. --- github-api.php | 67 +++++++++++++++++++++++++++++++++++++++++++ hashes-api.php | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++ vip-go-ci.php | 12 ++++++++ 3 files changed, 156 insertions(+) create mode 100644 hashes-api.php diff --git a/github-api.php b/github-api.php index c8a7f0bce..26954185d 100644 --- a/github-api.php +++ b/github-api.php @@ -1822,3 +1822,70 @@ function vipgoci_github_authenticated_user_get( $github_token ) { return $current_user_info; } + + +/* + * Ask the hashes-to-hashes database API if the + * specified file is approved. + */ + +function vipgoci_hashes_api_file_approved( + $options, + $file_path +) { + + $file_contents = file_get_contents( + $file_path + ); + + $file_contents = php_strip_whitespaces( + $file_contents + ); + + $file_sha1 = sha1( $file_contents ); + + $github_url = + 'https://' . + 'hashes-to-hashes.go-vip.co' . // FIXME: From option + '/wp-json/viphash/v1/hashes/id' . + rawurlencode( $file_sha1 ); + + $file_hashes_info = json_decode( + vipgoci_github_fetch_url( + $github_url, + $github_token + ), + true + ); + + $file_approved = null; + + /* + * Only approve file if all info-items show + * the file to be approved. + */ + + foreach( $file_hashes_info as $file_hash_info ) { + if ( false === $file_hash_info[ 'status' ] ) { + $file_approved = false; + } + + if ( true === $file_hash_info[ 'status' ] ) { + if ( null === $file_approved ) { + $file_approved = true; + } + } + } + + + /* + * If no approval is seen, assume it is not. + */ + + if ( null === $file_approved ) { + $file_approved = false; + } + + return $file_approved; +} + diff --git a/hashes-api.php b/hashes-api.php new file mode 100644 index 000000000..2cbaa2b93 --- /dev/null +++ b/hashes-api.php @@ -0,0 +1,77 @@ + $options['repo-owner'], + 'repo_name' => $options['repo-name'], + 'commit_id' => $options['commit'], + // FIXME: relevant options should follow + ) + ); + + + $prs_implicated = vipgoci_github_prs_implicated( + $options['repo-owner'], + $options['repo-name'], + $options['commit'], + $options['token'], + $options['branches-ignore'] + ); + + + foreach ( $prs_implicated as $pr_item ) { + $pr_diff = vipgoci_github_diffs_fetch( + $options['repo-owner'], + $options['repo-name'], + $options['token'], + $pr_item->base->sha, + $options['commit'] + ); + + + $files_seen_in_pr = array(); + $files_approved_in_pr = array(); + + foreach( $pr_diff as + $pr_diff_file_name => $pr_diff_contents + ) { + $files_seen_in_pr[] = $pr_diff_file_name; + + /* + * Get SHA1 checksum for the file-contents + */ + $approval_status = vipgoci_hashes_api_approved( + $options, + $pr_diff_file_name + ); + + /* + * Check if the hashes-to-hashes database + * recognises this file, and check its + * status. + */ + + /* + * Add the file to a list of approved files + * of these affected by the Pull-Request. + */ + if ( true === $approval_status ) { + $files_approved_in_pr[] = $pr_diff_file_name; + } + } + } + + /* + * Reduce memory-usage as possible + */ + + unset( $files_seen_in_pr ); + unset( $files_approved_in_pr ); + unset( $prs_implicated ); + unset( $pr_diff ); + + gc_collect_cycles(); +} diff --git a/vip-go-ci.php b/vip-go-ci.php index cbac6e0fa..105e57d6b 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -647,6 +647,18 @@ function vipgoci_run() { } + /* + * Do scanning of all altered file, using + * the hashes-to-hashes database API. + */ + + if ( true === $options['hashes-api'] ) { + vipgoci_hashes_api_scan_commit( + $options + ); + } + + /* * Submit any issues to GitHub */ From 73f6ac7918b3d30353b2f4b6db1d2afa52205cbd Mon Sep 17 00:00:00 2001 From: gdh Date: Thu, 12 Apr 2018 15:43:16 +0000 Subject: [PATCH 002/142] Moving comment around --- hashes-api.php | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/hashes-api.php b/hashes-api.php index 2cbaa2b93..bfa8c5444 100644 --- a/hashes-api.php +++ b/hashes-api.php @@ -41,18 +41,15 @@ function vipgoci_hashes_api_scan_commit( $options ) { $files_seen_in_pr[] = $pr_diff_file_name; /* - * Get SHA1 checksum for the file-contents + * Check if the hashes-to-hashes database + * recognises this file, and check its + * status. */ $approval_status = vipgoci_hashes_api_approved( $options, $pr_diff_file_name ); - /* - * Check if the hashes-to-hashes database - * recognises this file, and check its - * status. - */ /* * Add the file to a list of approved files From 98f69d893684b1ecf4c097aeab26add8979e1a42 Mon Sep 17 00:00:00 2001 From: gdh Date: Thu, 12 Apr 2018 15:52:50 +0000 Subject: [PATCH 003/142] Add skeleton-logic and comments --- hashes-api.php | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/hashes-api.php b/hashes-api.php index bfa8c5444..dcde959b5 100644 --- a/hashes-api.php +++ b/hashes-api.php @@ -45,6 +45,9 @@ function vipgoci_hashes_api_scan_commit( $options ) { * recognises this file, and check its * status. */ + + // FIXME: Take into consideration the review-level of both + // the target-repo and of the code $approval_status = vipgoci_hashes_api_approved( $options, $pr_diff_file_name @@ -61,6 +64,27 @@ function vipgoci_hashes_api_scan_commit( $options ) { } } + // FIXME: If all seen files are found in approved, simply + // make a comment to the PR stating that this is approved + // or even auto-approve + + if ( + count( + array_diff( + $files_seen_in_pr, + $files_approved_in_pr + ) + ) === 0 + ) { + // FIXME: Make a comment on that this can be auto-approved + } + + else { + // FIXME: Make separate comment for each file approved, + // noting that it does not need a review + } + + /* * Reduce memory-usage as possible */ From 1755ebf07cdc1881b2ae5e525530da9e140348f6 Mon Sep 17 00:00:00 2001 From: gdh Date: Tue, 17 Apr 2018 13:46:20 +0000 Subject: [PATCH 004/142] Moved function to another file --- github-api.php | 67 -------------------------------------------------- hashes-api.php | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 67 deletions(-) diff --git a/github-api.php b/github-api.php index 26954185d..c8a7f0bce 100644 --- a/github-api.php +++ b/github-api.php @@ -1822,70 +1822,3 @@ function vipgoci_github_authenticated_user_get( $github_token ) { return $current_user_info; } - - -/* - * Ask the hashes-to-hashes database API if the - * specified file is approved. - */ - -function vipgoci_hashes_api_file_approved( - $options, - $file_path -) { - - $file_contents = file_get_contents( - $file_path - ); - - $file_contents = php_strip_whitespaces( - $file_contents - ); - - $file_sha1 = sha1( $file_contents ); - - $github_url = - 'https://' . - 'hashes-to-hashes.go-vip.co' . // FIXME: From option - '/wp-json/viphash/v1/hashes/id' . - rawurlencode( $file_sha1 ); - - $file_hashes_info = json_decode( - vipgoci_github_fetch_url( - $github_url, - $github_token - ), - true - ); - - $file_approved = null; - - /* - * Only approve file if all info-items show - * the file to be approved. - */ - - foreach( $file_hashes_info as $file_hash_info ) { - if ( false === $file_hash_info[ 'status' ] ) { - $file_approved = false; - } - - if ( true === $file_hash_info[ 'status' ] ) { - if ( null === $file_approved ) { - $file_approved = true; - } - } - } - - - /* - * If no approval is seen, assume it is not. - */ - - if ( null === $file_approved ) { - $file_approved = false; - } - - return $file_approved; -} - diff --git a/hashes-api.php b/hashes-api.php index dcde959b5..69455a9a9 100644 --- a/hashes-api.php +++ b/hashes-api.php @@ -1,5 +1,70 @@ Date: Tue, 17 Apr 2018 13:51:41 +0000 Subject: [PATCH 005/142] Measure how long it takes to check approval-status --- hashes-api.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/hashes-api.php b/hashes-api.php index 69455a9a9..942730791 100644 --- a/hashes-api.php +++ b/hashes-api.php @@ -9,6 +9,7 @@ function vipgoci_hashes_api_file_approved( $options, $file_path ) { + vipgoci_runtime_measure( 'start', 'hashes_api_scan_file' ); $file_contents = file_get_contents( $file_path @@ -62,10 +63,14 @@ function vipgoci_hashes_api_file_approved( $file_approved = false; } + vipgoci_runtime_measure( 'stop', 'hashes_api_scan_file' ); + return $file_approved; } function vipgoci_hashes_api_scan_commit( $options ) { + vipgoci_runtime_measure( 'start', 'hashes_api_scan' ); + vipgoci_log( 'Scanning altered or new files affected by Pull-Request(s) ', 'using hashes-to-hashes database via API', @@ -160,6 +165,7 @@ function vipgoci_hashes_api_scan_commit( $options ) { unset( $pr_diff ); gc_collect_cycles(); -} + vipgoci_runtime_measure( 'stop', 'hashes_api_scan' ); +} From 30d4806a757a171ecac87096f3d08cbc8cb853fc Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 2 Jul 2018 11:02:50 +0000 Subject: [PATCH 006/142] Now supports specifying parameters for hashes-api on commandline --- vip-go-ci.php | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/vip-go-ci.php b/vip-go-ci.php index 105e57d6b..d7a9ea180 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -278,10 +278,12 @@ function vipgoci_run() { 'phpcs-path:', 'phpcs-standard:', 'phpcs-severity:', + 'hashes-api-url:', 'php-path:', 'local-git-repo:', 'lint:', 'phpcs:', + 'hashes-api:', 'help', 'debug-level:', ) @@ -310,6 +312,12 @@ function vipgoci_run() { "\t" . '--phpcs-severity=NUMBER Specify severity for PHPCS' . PHP_EOL . "\t" . '--php-path=FILE Full path to PHP, if not specified the' . PHP_EOL . "\t" . ' default in $PATH will be used instead' . PHP_EOL . + "\t" . '--hashes-api=BOOL Wether to do hashes-to-hashes API verfication ' . PHP_EOL . + "\t" . ' with individual PHP files found to be altered ' . PHP_EOL . + "\t" . ' in the branch specified' . PHP_EOL . + "\t" . '--hashes-api-url=STRING URL to hashes-to-hashes HTTP API root URL' . PHP_EOL . + "\t" . ' -- note that it should not include any specific' . PHP_EOL . + "\t" . ' paths to individual parts of the API.' . PHP_EOL . "\t" . '--branches-ignore=STRING,... What branches to ignore -- useful to make sure' . PHP_EOL . "\t" . ' some branches never get scanned. Separate branches' . PHP_EOL . "\t" . ' with commas' . PHP_EOL . @@ -388,6 +396,30 @@ function vipgoci_run() { ); + /* + * Process --hashes-api -- expected to be a boolean. + */ + + vipgoci_option_bool_handle( $options, 'hashes-api', 'false' ); + + + /* + * Process --hashes-api-url -- expected to + * be an URL to a webservice. + */ + + if ( isset( $options['hashes-api-url'] ) ) { + $options['hashes-api-url'] = trim( + $options['hashes-api-url'] + ); + + $options['hashes-api-url'] = rtrim( + $options['hashes-api-url'], + '/' + ); + } + + /* * Handle --local-git-repo parameter */ @@ -652,9 +684,11 @@ function vipgoci_run() { * the hashes-to-hashes database API. */ + if ( true === $options['hashes-api'] ) { vipgoci_hashes_api_scan_commit( - $options + $options, + $results['issues'] ); } From fea9897ee7f992de506e8da054fafede1ea5c7bf Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 2 Jul 2018 11:11:29 +0000 Subject: [PATCH 007/142] Add comments, error-checking and functionality --- hashes-api.php | 130 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 114 insertions(+), 16 deletions(-) diff --git a/hashes-api.php b/hashes-api.php index 942730791..45cf82ebb 100644 --- a/hashes-api.php +++ b/hashes-api.php @@ -11,29 +11,79 @@ function vipgoci_hashes_api_file_approved( ) { vipgoci_runtime_measure( 'start', 'hashes_api_scan_file' ); + /* + * Try to read file from disk, then + * get rid of whitespaces in the file + * and calculate SHA1 hash from the whole. + */ + $file_contents = file_get_contents( $file_path ); + if ( false === $file_contents ) { + vipgoci_log( + 'Unable to read file', + array( + 'file_path' => $file_path, + ) + ); + + return null; + } + $file_contents = php_strip_whitespaces( $file_contents ); + $file_sha1 = sha1( $file_contents ); - $github_url = - 'https://' . - 'hashes-to-hashes.go-vip.co' . // FIXME: From option + unset( $file_contents ); + + /* + * Ask the API for information about + * the specific hash we calculated. + */ + + $hashes_to_hashes_url = + $options['hashes-api-url'] . '/wp-json/viphash/v1/hashes/id' . rawurlencode( $file_sha1 ); - $file_hashes_info = json_decode( + $file_hashes_info = vipgoci_github_fetch_url( - $github_url, - $github_token - ), - true - ); + $hashes_to_hashes_url, + null + ); + + + /* + * Try to parse, and check for errors. + */ + if ( false !== $file_hashes_info ) { + $file_hashes_info = json_decode( + $file_hashes_info + ); + } + + + if ( + ( false === $file_hashes_info ) || + ( null === $file_hashes_info ) + ) { + vipgoci_log( + 'Unable to get information from ' . + 'hashes-to-hashes HTTP API', + array( + 'hashes_to_hashes_url' => $hashes_to_hashes_url, + 'file_path' => $file_path, + ) + ); + + return null; + } + $file_approved = null; @@ -48,6 +98,9 @@ function vipgoci_hashes_api_file_approved( } if ( true === $file_hash_info[ 'status' ] ) { + /* If we hit one non-approval, + * effectively assume it is not approved. + */ if ( null === $file_approved ) { $file_approved = true; } @@ -68,17 +121,28 @@ function vipgoci_hashes_api_file_approved( return $file_approved; } -function vipgoci_hashes_api_scan_commit( $options ) { + +/* + * Scan a particular commit, look for altered + * files in the Pull-Request we are associated with + * and for each of these files, check if they + * are approved in the hashes-to-hashes API. + */ +function vipgoci_hashes_api_scan_commit( + $options, + $commit_issues_submit +) { vipgoci_runtime_measure( 'start', 'hashes_api_scan' ); vipgoci_log( 'Scanning altered or new files affected by Pull-Request(s) ', 'using hashes-to-hashes database via API', array( - 'repo_owner' => $options['repo-owner'], - 'repo_name' => $options['repo-name'], - 'commit_id' => $options['commit'], - // FIXME: relevant options should follow + 'repo_owner' => $options['repo-owner'], + 'repo_name' => $options['repo-name'], + 'commit_id' => $options['commit'], + 'hashes-api' => $options['hashes-api'], + 'hashes-api-url' => $options['hashes-api-url'], ) ); @@ -131,6 +195,26 @@ function vipgoci_hashes_api_scan_commit( $options ) { if ( true === $approval_status ) { $files_approved_in_pr[] = $pr_diff_file_name; } + + else if ( false === $approval_status ) { + vipgoci_log( + 'File is not approved in ' . + 'hashes-to-hashes API', + array( + 'file_name' => $pr_diff_file_name, + ) + ); + } + + else if ( null === $approval_status ) { + vipgoci_log( + 'Could not determine if file is approved ' . + 'in hashes-to-hashes API', + array( + 'file_name' => $pr_diff_file_name, + ) + ); + } } } @@ -150,8 +234,22 @@ function vipgoci_hashes_api_scan_commit( $options ) { } else { - // FIXME: Make separate comment for each file approved, - // noting that it does not need a review + foreach ( $files_approved_in_pr as $file_name ) { + /* + * Make comment for each file, noting + * that it is already approved. + */ + $commit_issues_submit[ + $pr_item->number + ][] = array( + 'type' => 'phpcs', + 'file_name' => $file_name, + 'file_line' => 1, + 'issue' + => 'File is approved in ' . + 'hashes-to-hashes database', + ); + } } From fbfe7afb15cb7de19c129bff4107e0a213239bdd Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 2 Jul 2018 11:18:45 +0000 Subject: [PATCH 008/142] Solving conflict --- vip-go-ci.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/vip-go-ci.php b/vip-go-ci.php index d7a9ea180..8267168ac 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -283,6 +283,9 @@ function vipgoci_run() { 'local-git-repo:', 'lint:', 'phpcs:', + 'autoapprove:', + 'autoapprove-filetypes:', + 'autoapprove-label:', 'hashes-api:', 'help', 'debug-level:', From 11349124229669d085718bdacdd09303cc8584cc Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 2 Jul 2018 11:20:02 +0000 Subject: [PATCH 009/142] Solving conflicts --- vip-go-ci.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vip-go-ci.php b/vip-go-ci.php index 8267168ac..70ebec811 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -286,10 +286,10 @@ function vipgoci_run() { 'autoapprove:', 'autoapprove-filetypes:', 'autoapprove-label:', - 'hashes-api:', 'help', 'debug-level:', - ) + 'hashes-api:', + ) ); // Validate args From 2c276b5e04c1da2945703f1cbdb65506aa971e80 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 2 Jul 2018 11:24:58 +0000 Subject: [PATCH 010/142] access-token can be specified as null, and in these cases, not using it --- github-api.php | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/github-api.php b/github-api.php index c8a7f0bce..dda126ff4 100644 --- a/github-api.php +++ b/github-api.php @@ -383,11 +383,14 @@ function vipgoci_github_fetch_url( 'vipgoci_curl_headers' ); - curl_setopt( - $ch, - CURLOPT_HTTPHEADER, - array( 'Authorization: token ' . $github_token ) - ); + if ( null !== $github_token ) { + curl_setopt( + $ch, + CURLOPT_HTTPHEADER, + array( 'Authorization: token ' . $github_token ) + ); + } + // Make sure to pause between GitHub-requests vipgoci_github_wait(); From 81ee2cf245f663f660e4f2da6790daa556c30253 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 2 Jul 2018 11:35:31 +0000 Subject: [PATCH 011/142] Root path to HTTP API is to be specified by user --- hashes-api.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hashes-api.php b/hashes-api.php index 45cf82ebb..b2b47e2bc 100644 --- a/hashes-api.php +++ b/hashes-api.php @@ -48,7 +48,7 @@ function vipgoci_hashes_api_file_approved( $hashes_to_hashes_url = $options['hashes-api-url'] . - '/wp-json/viphash/v1/hashes/id' . + '/v1/hashes/id' . rawurlencode( $file_sha1 ); $file_hashes_info = From 166d98e7423e890d2cb8c9711db82b89444a94db Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 2 Jul 2018 11:51:39 +0000 Subject: [PATCH 012/142] Requring hashes-api.php --- vip-go-ci.php | 1 + 1 file changed, 1 insertion(+) diff --git a/vip-go-ci.php b/vip-go-ci.php index e6c0f7b09..a0ba661cd 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -7,6 +7,7 @@ require_once( __DIR__ . '/phpcs-scan.php' ); require_once( __DIR__ . '/lint-scan.php' ); require_once( __DIR__ . '/auto-approval.php' ); +require_once( __DIR__ . '/hashes-api.php' ); /* * Handle boolean parameters given on the command-line. From c4cff33a6672eefe347c343b1d7289645447c552 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 2 Jul 2018 13:10:39 +0000 Subject: [PATCH 013/142] Various fixes --- hashes-api.php | 107 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 95 insertions(+), 12 deletions(-) diff --git a/hashes-api.php b/hashes-api.php index b2b47e2bc..57906eb10 100644 --- a/hashes-api.php +++ b/hashes-api.php @@ -8,17 +8,76 @@ function vipgoci_hashes_api_file_approved( $options, $file_path -) { +) { vipgoci_runtime_measure( 'start', 'hashes_api_scan_file' ); + $file_extensions_approvable = array( + 'php', + 'js', + ); + + + /* + * Make sure to process only .php and + * .js files -- others are ignored. + */ + + $file_info_extension = pathinfo( + $file_path, + PATHINFO_EXTENSION + ); + + + if ( in_array( + strtolower( $file_info_extension ), + $file_extensions_approvable + ) === false ) { + vipgoci_log( + 'Not checking file for approval in hashes-to-hashes ' . + 'API, as it is not a file-type that are ' . + ' to be checked with it', + + array( + 'file_path' + => $file_path, + + 'file_extension' + => $file_info_extension, + + 'file_extensions_approvable' + => $file_extensions_approvable, + ) + ); + + return null; + } + + + vipgoci_log( + 'Checking if file is already approved in ' . + 'hashes-to-hashes API', + array( + 'repo_owner' => $options['repo-owner'], + 'repo_name' => $options['repo-name'], + 'token' => $options['token'], + 'commit' => $options['commit'], + 'file_path' => $file_path, + ) + ); + /* * Try to read file from disk, then * get rid of whitespaces in the file * and calculate SHA1 hash from the whole. */ - - $file_contents = file_get_contents( - $file_path + + $file_contents = vipgoci_gitrepo_fetch_committed_file( + $options['repo-owner'], + $options['repo-name'], + $options['token'], + $options['commit'], + $file_path, + $options['local-git-repo'] ); if ( false === $file_contents ) { @@ -32,14 +91,32 @@ function vipgoci_hashes_api_file_approved( return null; } - $file_contents = php_strip_whitespaces( + vipgoci_log( + 'Saving file from git-repository into temporary file ' . + 'in order to strip any whitespacing from it', + array( + 'file_path' => $file_path, + ), + 2 + ); + + + $file_temp_path = vipgoci_save_temp_file( + $file_path, + null, $file_contents ); + $file_contents_stripped = php_strip_whitespace( + $file_temp_path + ); + - $file_sha1 = sha1( $file_contents ); + $file_sha1 = sha1( $file_contents_stripped ); + unlink( $file_temp_path ); unset( $file_contents ); + unset( $file_contents_stripped ); /* * Ask the API for information about @@ -48,7 +125,7 @@ function vipgoci_hashes_api_file_approved( $hashes_to_hashes_url = $options['hashes-api-url'] . - '/v1/hashes/id' . + '/v1/hashes/id/' . rawurlencode( $file_sha1 ); $file_hashes_info = @@ -63,21 +140,27 @@ function vipgoci_hashes_api_file_approved( */ if ( false !== $file_hashes_info ) { $file_hashes_info = json_decode( - $file_hashes_info + $file_hashes_info, + true ); } if ( ( false === $file_hashes_info ) || - ( null === $file_hashes_info ) + ( null === $file_hashes_info ) || + ( + ( isset( $file_hashes_info['data']['status'] ) ) && + ( 404 === $file_hashes_info['data']['status'] ) + ) ) { vipgoci_log( 'Unable to get information from ' . 'hashes-to-hashes HTTP API', array( - 'hashes_to_hashes_url' => $hashes_to_hashes_url, - 'file_path' => $file_path, + 'hashes_to_hashes_url' => $hashes_to_hashes_url, + 'file_path' => $file_path, + 'http_reply' => $file_hashes_info, ) ); @@ -182,7 +265,7 @@ function vipgoci_hashes_api_scan_commit( // FIXME: Take into consideration the review-level of both // the target-repo and of the code - $approval_status = vipgoci_hashes_api_approved( + $approval_status = vipgoci_hashes_api_file_approved( $options, $pr_diff_file_name ); From 2f850251a57b4b96991ed052a1a1d5f4cffdf408 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 2 Jul 2018 16:20:47 +0000 Subject: [PATCH 014/142] Moving things into constants, special file for defines, enabling posting comments on GitHub --- github-api.php | 26 ++++++++++++++------------ hashes-api.php | 49 ++++++++++++++++++++++++++++++++++++++++++------- lint-scan.php | 2 +- misc.php | 23 ++++++++++------------- phpcs-scan.php | 2 +- vip-go-ci.php | 14 +++++++++----- 6 files changed, 77 insertions(+), 39 deletions(-) diff --git a/github-api.php b/github-api.php index dceede633..53b58cac8 100644 --- a/github-api.php +++ b/github-api.php @@ -1,13 +1,5 @@ $file_path, + 'file_sha1' => $file_sha1, + ), + 2 + ); + $hashes_to_hashes_url = $options['hashes-api-url'] . '/v1/hashes/id/' . @@ -176,11 +187,17 @@ function vipgoci_hashes_api_file_approved( */ foreach( $file_hashes_info as $file_hash_info ) { - if ( false === $file_hash_info[ 'status' ] ) { + if ( + ( 'false' === $file_hash_info[ 'status' ] ) || + ( false === $file_hash_info[ 'status' ] ) + ) { $file_approved = false; } - if ( true === $file_hash_info[ 'status' ] ) { + else if ( + ( 'true' === $file_hash_info[ 'status' ] ) || + ( true === $file_hash_info[ 'status' ] ) + ) { /* If we hit one non-approval, * effectively assume it is not approved. */ @@ -213,7 +230,8 @@ function vipgoci_hashes_api_file_approved( */ function vipgoci_hashes_api_scan_commit( $options, - $commit_issues_submit + &$commit_issues_submit, + &$commit_issues_stats ) { vipgoci_runtime_measure( 'start', 'hashes_api_scan' ); @@ -276,6 +294,14 @@ function vipgoci_hashes_api_scan_commit( * of these affected by the Pull-Request. */ if ( true === $approval_status ) { + vipgoci_log( + 'File is approved in ' . + 'hashes-to-hashes API', + array( + 'file_name' => $pr_diff_file_name, + ) + ); + $files_approved_in_pr[] = $pr_diff_file_name; } @@ -325,13 +351,22 @@ function vipgoci_hashes_api_scan_commit( $commit_issues_submit[ $pr_item->number ][] = array( - 'type' => 'phpcs', + 'type' => VIPGOCI_STATS_HASHES_API, 'file_name' => $file_name, 'file_line' => 1, - 'issue' - => 'File is approved in ' . - 'hashes-to-hashes database', + 'issue' + => array( + 'message' => + 'File is approved in ' . + 'hashes-to-hashes ' . + 'database', + 'level' => 'INFO', + ) ); + + $commit_issues_stats[ + $pr_item->number + ]['info']++; } } diff --git a/lint-scan.php b/lint-scan.php index 06e50e6a6..c88157eb1 100644 --- a/lint-scan.php +++ b/lint-scan.php @@ -332,7 +332,7 @@ function vipgoci_lint_scan_commit( $commit_issues_submit[ $pr_item->number ][] = array( - 'type' => 'lint', + 'type' => VIPGOCI_STATS_LINT, 'file_name' => $filename, diff --git a/misc.php b/misc.php index 44d32c933..ccd583e4e 100644 --- a/misc.php +++ b/misc.php @@ -1,16 +1,5 @@ number ] = array( 'error' => 0, - 'warning' => 0 + 'warning' => 0, + 'info' => 0, ); } } diff --git a/phpcs-scan.php b/phpcs-scan.php index bd1ac2816..c09b4f557 100644 --- a/phpcs-scan.php +++ b/phpcs-scan.php @@ -595,7 +595,7 @@ function( $item ) { $commit_issues_submit[ $pr_item->number ][] = array( - 'type' => 'phpcs', + 'type' => VIPGOCI_STATS_PHPCS, 'file_name' => $file_name, diff --git a/vip-go-ci.php b/vip-go-ci.php index a0ba661cd..2602a522b 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -1,6 +1,7 @@ #!/usr/bin/php array(), 'stats' => array( - 'phpcs' => null, - 'lint' => null, + VIPGOCI_STATS_PHPCS => null, + VIPGOCI_STATS_LINT => null, + VIPGOCI_STATS_HASHES_API => null, ), ); @@ -793,7 +796,7 @@ function vipgoci_run() { vipgoci_lint_scan_commit( $options, $results['issues'], - $results['stats']['lint'] + $results['stats'][VIPGOCI_STATS_LINT] ); } @@ -806,7 +809,7 @@ function vipgoci_run() { vipgoci_phpcs_scan_commit( $options, $results['issues'], - $results['stats']['phpcs'] + $results['stats'][VIPGOCI_STATS_PHPCS] ); } @@ -832,7 +835,8 @@ function vipgoci_run() { if ( true === $options['hashes-api'] ) { vipgoci_hashes_api_scan_commit( $options, - $results['issues'] + $results['issues'], + $results['stats'][ VIPGOCI_STATS_HASHES_API ] ); } From 93b03f0a84dc284eaa8c112d0dda9c7359db7709 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 2 Jul 2018 16:42:26 +0000 Subject: [PATCH 015/142] Adding defines file --- defines.php | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 defines.php diff --git a/defines.php b/defines.php new file mode 100644 index 000000000..bd6536953 --- /dev/null +++ b/defines.php @@ -0,0 +1,33 @@ + Date: Mon, 2 Jul 2018 18:56:07 +0000 Subject: [PATCH 016/142] Tidying up comments, spacing, etc. --- hashes-api.php | 25 ++++++++++++++----------- misc.php | 2 +- vip-go-ci.php | 4 ++-- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/hashes-api.php b/hashes-api.php index 24d56d1a4..bcd1aeb28 100644 --- a/hashes-api.php +++ b/hashes-api.php @@ -11,17 +11,17 @@ function vipgoci_hashes_api_file_approved( ) { vipgoci_runtime_measure( 'start', 'hashes_api_scan_file' ); + /* + * Make sure to process only *.php and + * *.js files -- others are ignored. + */ + $file_extensions_approvable = array( 'php', 'js', ); - - /* - * Make sure to process only .php and - * .js files -- others are ignored. - */ - + $file_info_extension = pathinfo( $file_path, PATHINFO_EXTENSION @@ -34,8 +34,8 @@ function vipgoci_hashes_api_file_approved( ) === false ) { vipgoci_log( 'Not checking file for approval in hashes-to-hashes ' . - 'API, as it is not a file-type that are ' . - ' to be checked with it', + 'API, as it is not a file-type that is ' . + 'to be checked using it', array( 'file_path' @@ -198,8 +198,10 @@ function vipgoci_hashes_api_file_approved( ( 'true' === $file_hash_info[ 'status' ] ) || ( true === $file_hash_info[ 'status' ] ) ) { - /* If we hit one non-approval, - * effectively assume it is not approved. + /* + * Only update approval-flag if we have not + * seen any other approvals, and if we have + * not seen any rejections. */ if ( null === $file_approved ) { $file_approved = true; @@ -209,7 +211,8 @@ function vipgoci_hashes_api_file_approved( /* - * If no approval is seen, assume it is not. + * If no approval is seen, assume it is not + * approved at all. */ if ( null === $file_approved ) { diff --git a/misc.php b/misc.php index ccd583e4e..bb485df44 100644 --- a/misc.php +++ b/misc.php @@ -504,7 +504,7 @@ function vipgoci_stats_init( $options, $prs_implicated, &$results ) { VIPGOCI_STATS_LINT, VIPGOCI_STATS_HASHES_API ) - as $stats_type + as $stats_type ) { /* * Initialize stats for the stats-types only when diff --git a/vip-go-ci.php b/vip-go-ci.php index 2602a522b..da1fdebcc 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -796,7 +796,7 @@ function vipgoci_run() { vipgoci_lint_scan_commit( $options, $results['issues'], - $results['stats'][VIPGOCI_STATS_LINT] + $results['stats'][ VIPGOCI_STATS_LINT ] ); } @@ -809,7 +809,7 @@ function vipgoci_run() { vipgoci_phpcs_scan_commit( $options, $results['issues'], - $results['stats'][VIPGOCI_STATS_PHPCS] + $results['stats'][ VIPGOCI_STATS_PHPCS ] ); } From 2e5e90ef82f574b54576096ac44c360799e675b2 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 2 Jul 2018 19:10:51 +0000 Subject: [PATCH 017/142] Adding emoji --- misc.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/misc.php b/misc.php index bb485df44..f15c76aaa 100644 --- a/misc.php +++ b/misc.php @@ -306,6 +306,9 @@ function vipgoci_github_labels( $text_string ) { case 'error': return ':no_entry_sign:'; + + case 'info': + return ':information_source:'; } return ''; From faf5e1a1ae369a14fb6b0c16466f7117f328c2d4 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 16 Jul 2018 14:30:40 +0000 Subject: [PATCH 018/142] Updating to PHPCS 3.2.0 --- tools-init.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools-init.sh b/tools-init.sh index f8454875f..9386e4813 100644 --- a/tools-init.sh +++ b/tools-init.sh @@ -1,6 +1,6 @@ #!/bin/bash -export PHP_CODESNIFFER_VER="3.1.0" +export PHP_CODESNIFFER_VER="3.2.0" export WP_CODING_STANDARDS_VER="0.14.0" export VIP_CODING_STANDARDS_VER="0.2.3" From f9dbe14e26231422b84a7d91c011a677a36091b7 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 16 Jul 2018 14:30:58 +0000 Subject: [PATCH 019/142] Altering comment --- hashes-api.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/hashes-api.php b/hashes-api.php index bcd1aeb28..48574a3d5 100644 --- a/hashes-api.php +++ b/hashes-api.php @@ -330,9 +330,12 @@ function vipgoci_hashes_api_scan_commit( } } - // FIXME: If all seen files are found in approved, simply - // make a comment to the PR stating that this is approved - // or even auto-approve + /* + * If all seen files are found in approved in hashes-to-hashes, + * simply make a comment to the PR stating that this is approved. + * If only some files are approved, make a comment on these + * saying that the files are approved in hashes-to-hashes. + */ if ( count( @@ -343,6 +346,7 @@ function vipgoci_hashes_api_scan_commit( ) === 0 ) { // FIXME: Make a comment on that this can be auto-approved + // -- or just auto-approve? } else { From 21a97a0db8fc9d7e5bcd2665bce1aeb0bea0f31b Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 17 Jul 2018 14:41:39 +0000 Subject: [PATCH 020/142] When fetching user results in error, log something to debug --- vip-go-ci.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/vip-go-ci.php b/vip-go-ci.php index da1fdebcc..3cb87205f 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -598,7 +598,9 @@ function vipgoci_run() { ) { vipgoci_sysexit( 'Unable to get information about token-holder user from GitHub', - array(), + array( + 'current_user_info' => $current_user_info, + ), VIPGOCI_EXIT_GITHUB_PROBLEM ); } From 55aaf42612ee85e49c2ead1e800b40e0021c9083 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 17 Jul 2018 15:19:12 +0000 Subject: [PATCH 021/142] Enhancing error-logging when getting info about user fails --- github-api.php | 33 ++++++++++++++++++++++++++++----- vip-go-ci.php | 2 +- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/github-api.php b/github-api.php index 53b58cac8..5527b12c9 100644 --- a/github-api.php +++ b/github-api.php @@ -1978,13 +1978,36 @@ function vipgoci_github_authenticated_user_get( $github_token ) { 'https://api.github.com/' . 'user'; - $current_user_info = json_decode( - vipgoci_github_fetch_url( - $github_url, - $github_token - ) + $current_user_info_json = vipgoci_github_fetch_url( + $github_url, + $github_token ); + $current_user_info = null; + + if ( false !== $current_user_info_json ) { + $current_user_info = json_decode( + $current_user_info_json + ); + } + + if ( + ( false === $current_user_info_json ) || + ( null === $current_user_info ) + ) { + vipgoci_log( + 'Unable to get information about token-holder from' . + 'GitHub due to error', + array( + 'current_user_info_json' => $current_user_info_json, + 'current_user_info' => $current_user_info, + ) + ); + + return false; + } + + vipgoci_cache( $cached_id, $current_user_info ); return $current_user_info; diff --git a/vip-go-ci.php b/vip-go-ci.php index 3cb87205f..e235bef50 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -593,13 +593,13 @@ function vipgoci_run() { ); if ( + ( false === $current_user_info ) || ( ! isset( $current_user_info->login ) ) || ( empty( $current_user_info->login ) ) ) { vipgoci_sysexit( 'Unable to get information about token-holder user from GitHub', array( - 'current_user_info' => $current_user_info, ), VIPGOCI_EXIT_GITHUB_PROBLEM ); From bf373f031beb26ce5c718db9a7705de87f06a4a3 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 20 Jul 2018 13:55:01 +0000 Subject: [PATCH 022/142] Auto-approval if all files approved in hashes-to-hashes --- auto-approval.php | 1 + defines.php | 7 +++ github-api.php | 33 +++++++++++--- hashes-api.php | 112 +++++++++++++++++++++++++++++++++++++++++----- vip-go-ci.php | 29 ++++++++++-- 5 files changed, 163 insertions(+), 19 deletions(-) diff --git a/auto-approval.php b/auto-approval.php index f48a01484..9cc236433 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -198,6 +198,7 @@ function vipgoci_auto_approval( $options ) { $pr_item->number, $options['commit'], $options['autoapprove-filetypes'], + VIPGOCI_APPROVAL_AUTOAPPROVE, $options['dry-run'] ); diff --git a/defines.php b/defines.php index bd6536953..404dae85c 100644 --- a/defines.php +++ b/defines.php @@ -31,3 +31,10 @@ define( 'VIPGOCI_STATS_PHPCS', 'phpcs' ); define( 'VIPGOCI_STATS_LINT', 'lint' ); define( 'VIPGOCI_STATS_HASHES_API', 'hashes-api' ); + +/* + * Define auto-approval types + */ + +define( 'VIPGOCI_APPROVAL_AUTOAPPROVE', 'auto-approval' ); +define( 'VIPGOCI_APPROVAL_HASHES_API', 'hashes-api' ); diff --git a/github-api.php b/github-api.php index 5527b12c9..524289d9c 100644 --- a/github-api.php +++ b/github-api.php @@ -1563,7 +1563,7 @@ function vipgoci_github_pr_review_submit( * * The race-conditions can occur when a Pull-Request * is approved, but it is approved after a new commit - * was added, but that has not been scanned. + * was added which has not been scanned. */ function vipgoci_github_approve_pr( @@ -1573,6 +1573,7 @@ function vipgoci_github_approve_pr( $pr_number, $latest_commit_id, $filetypes_approve, + $approval_type = null, $dry_run ) { @@ -1588,14 +1589,36 @@ function vipgoci_github_approve_pr( $github_postfields = array( 'commit_id' => $latest_commit_id, - 'body' => 'Auto-approved Pull-Request #' . - (int) $pr_number . ' as it ' . - 'contains only allowable file-types ' . - '(' . implode( ', ', $filetypes_approve ) . ')', 'event' => 'APPROVE', + 'body' => null, 'comments' => array() ); + if ( VIPGOCI_APPROVAL_AUTOAPPROVE === $approval_type ) { + $github_postfields['body'] = + 'Auto-approved Pull-Request #' . + (int) $pr_number . ' as it ' . + 'contains only allowable file-types ' . + '(' . implode( ', ', $filetypes_approve ) . ')'; + } + + else if ( VIPGOCI_APPROVAL_HASHES_API === $approval_type ) { + $github_postfields['body'] = + 'Auto-approved Pull-Request #' . + (int) $pr_number . ' as it contains ' . + 'contains only files approved in hashes-to-hashes' . + 'database'; + } + + else { + vipgoci_sysexit( + 'Illegal usage of function', + array( + 'function_name' => __FUNCTION__, + ) + ); + } + if ( true === $dry_run ) { return; } diff --git a/hashes-api.php b/hashes-api.php index 48574a3d5..80524c1db 100644 --- a/hashes-api.php +++ b/hashes-api.php @@ -8,20 +8,25 @@ function vipgoci_hashes_api_file_approved( $options, $file_path -) { +) { vipgoci_runtime_measure( 'start', 'hashes_api_scan_file' ); /* * Make sure to process only *.php and * *.js files -- others are ignored. + * + * Cross-reference: These files are not + * auto-approved by our own auto-approval + * mechanism, as to avoid any conflicts between + * hashes-api and the auto-approval mechanism. */ - + $file_extensions_approvable = array( 'php', 'js', ); - + $file_info_extension = pathinfo( $file_path, PATHINFO_EXTENSION @@ -36,7 +41,7 @@ function vipgoci_hashes_api_file_approved( 'Not checking file for approval in hashes-to-hashes ' . 'API, as it is not a file-type that is ' . 'to be checked using it', - + array( 'file_path' => $file_path, @@ -70,7 +75,7 @@ function vipgoci_hashes_api_file_approved( * get rid of whitespaces in the file * and calculate SHA1 hash from the whole. */ - + $file_contents = vipgoci_gitrepo_fetch_committed_file( $options['repo-owner'], $options['repo-name'], @@ -198,7 +203,7 @@ function vipgoci_hashes_api_file_approved( ( 'true' === $file_hash_info[ 'status' ] ) || ( true === $file_hash_info[ 'status' ] ) ) { - /* + /* * Only update approval-flag if we have not * seen any other approvals, and if we have * not seen any rejections. @@ -330,9 +335,24 @@ function vipgoci_hashes_api_scan_commit( } } + /* + * Get label associated, but + * only our auto-approved one + */ + + $pr_label = vipgoci_github_labels_get( + $options['repo-owner'], + $options['repo-name'], + $options['token'], + (int) $pr_item->number, + $options['autoapprove-label'] + ); + + /* * If all seen files are found in approved in hashes-to-hashes, - * simply make a comment to the PR stating that this is approved. + * approve the Pull-Request and add a label. + * * If only some files are approved, make a comment on these * saying that the files are approved in hashes-to-hashes. */ @@ -345,12 +365,84 @@ function vipgoci_hashes_api_scan_commit( ) ) === 0 ) { - // FIXME: Make a comment on that this can be auto-approved - // -- or just auto-approve? + /* + * Actually approve, if not in dry-mode. + * Also add a label to the Pull-Request + * if applicable. + */ + + vipgoci_github_approve_pr( + $options['repo-owner'], + $options['repo-name'], + $options['token'], + $pr_item->number, + $options['commit'], + $options['autoapprove-filetypes'], + VIPGOCI_APPROVAL_HASHES_API, + $options['dry-run'] + ); + + /* Add label, if needed */ + if ( false === $pr_label ) { + vipgoci_github_label_add_to_pr( + $options['repo-owner'], + $options['repo-name'], + $options['token'], + $pr_item->number, + $options['autoapprove-label'], + $options['dry-run'] + ); + } + + else { + vipgoci_log( + 'Will not add label to issue, ' . + 'as it already exists', + + array( + 'repo_owner' => + $options['repo-owner'], + + 'repo_name' => + $options['repo-name'], + + 'pr_number' => + $pr_item->number, + + 'label_name' => + $options['autoapprove-label'], + ) + ); + } } else { + /* + * Remove auto-approve label + */ + + if ( false !== $pr_label ) { + vipgoci_github_label_remove_from_pr( + $options['repo-owner'], + $options['repo-name'], + $options['token'], + (int) $pr_item->number, + $pr_label->name, + $options['dry-run'] + ); + } + + + /* + * Go through files that are approved, + * and add comment for them saying that + * they are approved already in the hashes-api + * database. + */ foreach ( $files_approved_in_pr as $file_name ) { + // FIXME: Check if comment has been + // made before and do not re-post if so. + /* * Make comment for each file, noting * that it is already approved. @@ -361,7 +453,7 @@ function vipgoci_hashes_api_scan_commit( 'type' => VIPGOCI_STATS_HASHES_API, 'file_name' => $file_name, 'file_line' => 1, - 'issue' + 'issue' => array( 'message' => 'File is approved in ' . diff --git a/vip-go-ci.php b/vip-go-ci.php index e235bef50..0e843d5d0 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -572,11 +572,33 @@ function vipgoci_run() { if ( ( true === $options['autoapprove'] ) && - ( in_array( 'php', $options['autoapprove-filetypes'], true ) ) + + /* + * Cross-reference: We disallow autoapproving + * PHP and JS files here, because hashes-api + * could autoapprove them and because they can + * contain dangerous code. By doing this, we + * avoid any possible conflicts between autoapproval + * and hases-api. + */ + ( + ( in_array( + 'php', + $options['autoapprove-filetypes'], + true + ) ) + || + ( in_array( + 'js', + $options['autoapprove-filetypes'], + true + ) ) + + ) ) { vipgoci_sysexit( - 'PHP files cannot be auto-approved, as they can' . - 'contain serious problems for execution', + 'PHP and JS files cannot be auto-approved, as they ' . + 'can contain serious problems for execution', array( ), VIPGOCI_EXIT_USAGE_ERROR @@ -833,7 +855,6 @@ function vipgoci_run() { * the hashes-to-hashes database API. */ - if ( true === $options['hashes-api'] ) { vipgoci_hashes_api_scan_commit( $options, From a261ffb1bffc7a482d9b516a61dc67f4aae693cf Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 20 Jul 2018 14:05:11 +0000 Subject: [PATCH 023/142] Whitespacing adjustment --- defines.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/defines.php b/defines.php index 404dae85c..df95c5d54 100644 --- a/defines.php +++ b/defines.php @@ -4,9 +4,9 @@ * Client-ID for curl-requests, etc. */ -define( 'VIPGOCI_CLIENT_ID', 'automattic-vip-go-ci' ); -define( 'VIPGOCI_SYNTAX_ERROR_STR', 'PHP Syntax Errors Found' ); -define( 'VIPGOCI_GITHUB_ERROR_STR', 'GitHub API communication error'); +define( 'VIPGOCI_CLIENT_ID', 'automattic-vip-go-ci' ); +define( 'VIPGOCI_SYNTAX_ERROR_STR', 'PHP Syntax Errors Found' ); +define( 'VIPGOCI_GITHUB_ERROR_STR', 'GitHub API communication error'); /* From e70e1dca69051b86d41344173d8808fb213a19a9 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 20 Jul 2018 14:12:16 +0000 Subject: [PATCH 024/142] Re-arranging fields --- github-api.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github-api.php b/github-api.php index 524289d9c..03eb014a5 100644 --- a/github-api.php +++ b/github-api.php @@ -1589,8 +1589,8 @@ function vipgoci_github_approve_pr( $github_postfields = array( 'commit_id' => $latest_commit_id, - 'event' => 'APPROVE', 'body' => null, + 'event' => 'APPROVE', 'comments' => array() ); From 74166a33dabee8d8eec5ec02c6ff446c40c26680 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 20 Jul 2018 14:15:20 +0000 Subject: [PATCH 025/142] Removed duplicated word --- github-api.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github-api.php b/github-api.php index 03eb014a5..c82772381 100644 --- a/github-api.php +++ b/github-api.php @@ -1606,7 +1606,7 @@ function vipgoci_github_approve_pr( $github_postfields['body'] = 'Auto-approved Pull-Request #' . (int) $pr_number . ' as it contains ' . - 'contains only files approved in hashes-to-hashes' . + 'only files approved in hashes-to-hashes' . 'database'; } From 94ac633d820cf801c2eae3bf46b974e3416e309f Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 20 Jul 2018 14:16:46 +0000 Subject: [PATCH 026/142] Updated comment --- hashes-api.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hashes-api.php b/hashes-api.php index 80524c1db..516ee33a1 100644 --- a/hashes-api.php +++ b/hashes-api.php @@ -15,7 +15,7 @@ function vipgoci_hashes_api_file_approved( * Make sure to process only *.php and * *.js files -- others are ignored. * - * Cross-reference: These files are not + * Cross-reference: These file types are not * auto-approved by our own auto-approval * mechanism, as to avoid any conflicts between * hashes-api and the auto-approval mechanism. From aa3df81725168c955b7e2f147845a10e6af2824e Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 20 Jul 2018 14:19:54 +0000 Subject: [PATCH 027/142] Added comment --- hashes-api.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hashes-api.php b/hashes-api.php index 516ee33a1..47aa1d3b5 100644 --- a/hashes-api.php +++ b/hashes-api.php @@ -144,6 +144,12 @@ function vipgoci_hashes_api_file_approved( '/v1/hashes/id/' . rawurlencode( $file_sha1 ); + /* + * Not really asking GitHub here, + * but we can re-use the function + * for this purpose. + */ + $file_hashes_info = vipgoci_github_fetch_url( $hashes_to_hashes_url, From 548ba9ee138aac95f2db77d585b88d4b92ba5fc4 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 20 Jul 2018 14:27:24 +0000 Subject: [PATCH 028/142] Adding FIXME --- hashes-api.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hashes-api.php b/hashes-api.php index 47aa1d3b5..eb1e12f36 100644 --- a/hashes-api.php +++ b/hashes-api.php @@ -388,6 +388,8 @@ function vipgoci_hashes_api_scan_commit( $options['dry-run'] ); + // FIXME: We should have our own label + /* Add label, if needed */ if ( false === $pr_label ) { vipgoci_github_label_add_to_pr( From be46a8234d776772756f6c3a3073e82197ae426f Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 20 Jul 2018 14:31:35 +0000 Subject: [PATCH 029/142] Update help message --- vip-go-ci.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vip-go-ci.php b/vip-go-ci.php index 0e843d5d0..a9b84f599 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -344,7 +344,7 @@ function vipgoci_run() { "\t" . '--autoapprove-label=STRING String to use for labels when auto-approving' . PHP_EOL . "\t" . '--php-path=FILE Full path to PHP, if not specified the' . PHP_EOL . "\t" . ' default in $PATH will be used instead' . PHP_EOL . - "\t" . '--hashes-api=BOOL Wether to do hashes-to-hashes API verfication ' . PHP_EOL . + "\t" . '--hashes-api=BOOL Whether to do hashes-to-hashes API verfication ' . PHP_EOL . "\t" . ' with individual PHP files found to be altered ' . PHP_EOL . "\t" . ' in the branch specified' . PHP_EOL . "\t" . '--hashes-api-url=STRING URL to hashes-to-hashes HTTP API root URL' . PHP_EOL . From ed860545682f421cd5a850c9e169710f4b773d91 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 20 Jul 2018 14:32:18 +0000 Subject: [PATCH 030/142] Fixing help message --- vip-go-ci.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vip-go-ci.php b/vip-go-ci.php index a9b84f599..ee8753c7e 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -347,7 +347,7 @@ function vipgoci_run() { "\t" . '--hashes-api=BOOL Whether to do hashes-to-hashes API verfication ' . PHP_EOL . "\t" . ' with individual PHP files found to be altered ' . PHP_EOL . "\t" . ' in the branch specified' . PHP_EOL . - "\t" . '--hashes-api-url=STRING URL to hashes-to-hashes HTTP API root URL' . PHP_EOL . + "\t" . '--hashes-api-url=STRING URL to hashes-to-hashes HTTP API root' . PHP_EOL . "\t" . ' -- note that it should not include any specific' . PHP_EOL . "\t" . ' paths to individual parts of the API.' . PHP_EOL . "\t" . '--branches-ignore=STRING,... What branches to ignore -- useful to make sure' . PHP_EOL . From b44cabf72c7988cc04b9e6ff650522bba6c5bc6e Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 20 Jul 2018 14:33:59 +0000 Subject: [PATCH 031/142] Update comment --- vip-go-ci.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vip-go-ci.php b/vip-go-ci.php index ee8753c7e..755f2dd23 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -851,7 +851,7 @@ function vipgoci_run() { /* - * Do scanning of all altered file, using + * Do scanning of all altered files, using * the hashes-to-hashes database API. */ From 1d4ec8fb3b029622809ef8fd3d778834ddb2dabb Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 3 Sep 2018 15:23:19 +0000 Subject: [PATCH 032/142] Updating to match master --- tools-init.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools-init.sh b/tools-init.sh index eab755a36..49582acde 100644 --- a/tools-init.sh +++ b/tools-init.sh @@ -1,8 +1,8 @@ #!/bin/bash -export PHP_CODESNIFFER_VER="3.2.0" -export WP_CODING_STANDARDS_VER="0.14.0" -export VIP_CODING_STANDARDS_VER="0.2.4" +export PHP_CODESNIFFER_VER="3.3.1" +export WP_CODING_STANDARDS_VER="1.0.0" +export VIP_CODING_STANDARDS_VER="0.3.0" export TMP_LOCK_FILE="$HOME/.vip-go-ci-tools-init.lck" From 12921573147224d51616024b25f982742ce47a9e Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 3 Sep 2018 18:01:38 +0000 Subject: [PATCH 033/142] Splitting auto-approval into three modules One module will oversee the actual approval, based up on if all files in the PR have been approved by the other modules. These other modules will look through PR-altered files, and determine if each file can be auto-approved, and if so, adds it to a list that the overseeing module looks at. --- ap-file-types.php | 104 ++++++++++++++++ hashes-api.php => ap-hashes-api.php | 176 ++++------------------------ auto-approval.php | 32 +++-- vip-go-ci.php | 43 +++++-- 4 files changed, 184 insertions(+), 171 deletions(-) create mode 100644 ap-file-types.php rename hashes-api.php => ap-hashes-api.php (67%) diff --git a/ap-file-types.php b/ap-file-types.php new file mode 100644 index 000000000..ce62115b6 --- /dev/null +++ b/ap-file-types.php @@ -0,0 +1,104 @@ + $options['repo-owner'], + 'repo_name' => $options['repo-name'], + 'commit_id' => $options['commit'], + 'autoapprove' => $options['autoapprove'], + + 'autoapprove-filetypes' => + $options['autoapprove-filetypes'], + ) + ); + + + $prs_implicated = vipgoci_github_prs_implicated( + $options['repo-owner'], + $options['repo-name'], + $options['commit'], + $options['token'], + $options['branches-ignore'] + ); + + + foreach ( $prs_implicated as $pr_item ) { + $pr_diff = vipgoci_github_diffs_fetch( + $options['repo-owner'], + $options['repo-name'], + $options['token'], + $pr_item->base->sha, + $options['commit'] + ); + + + foreach ( $pr_diff as + $pr_diff_file_name => $pr_diff_contents + ) { + $pr_diff_file_extension = pathinfo( + $pr_diff_file_name, + PATHINFO_EXTENSION + ); + + /* + * If the file is already in the array + * of approved file, do not do anything. + */ + if ( in_array( + $pr_diff_file_name, + $auto_approved_files_arr, + true + ) ) { + continue; + } + + /* + * Check if the extension of the file + * is in a list of auto-approvable + * file extensions. + */ + if ( in_array( + strtolower( + $pr_diff_file_extension + ), + $options['autoapprove-filetypes'], + true + ) ) { + $auto_approved_files_arr[] = $pr_diff_file_name; + } + } + } + + /* + * Reduce memory-usage as possible + */ + unset( $prs_implicated ); + unset( $pr_diff ); + unset( $pr_item ); + + gc_collect_cycles(); + + vipgoci_runtime_measure( 'stop', 'ap_file_types' ); +} + diff --git a/hashes-api.php b/ap-hashes-api.php similarity index 67% rename from hashes-api.php rename to ap-hashes-api.php index eb1e12f36..67e9c31bc 100644 --- a/hashes-api.php +++ b/ap-hashes-api.php @@ -5,7 +5,7 @@ * specified file is approved. */ -function vipgoci_hashes_api_file_approved( +function vipgoci_ap_hashes_api_file_approved( $options, $file_path ) { @@ -174,6 +174,10 @@ function vipgoci_hashes_api_file_approved( ( ( isset( $file_hashes_info['data']['status'] ) ) && ( 404 === $file_hashes_info['data']['status'] ) + ) || + ( + ( isset( $file_hashes_info['data']['status'] ) ) && + ( 401 === $file_hashes_info['data']['status'] ) ) ) { vipgoci_log( @@ -198,6 +202,11 @@ function vipgoci_hashes_api_file_approved( */ foreach( $file_hashes_info as $file_hash_info ) { + if ( ! isset( $file_hash_info[ 'status' ] ) ) { + + $file_approved = false; + } + if ( ( 'false' === $file_hash_info[ 'status' ] ) || ( false === $file_hash_info[ 'status' ] ) @@ -242,10 +251,11 @@ function vipgoci_hashes_api_file_approved( * and for each of these files, check if they * are approved in the hashes-to-hashes API. */ -function vipgoci_hashes_api_scan_commit( +function vipgoci_ap_hashes_api_scan_commit( $options, &$commit_issues_submit, - &$commit_issues_stats + &$commit_issues_stats, + &$auto_approved_files_arr ) { vipgoci_runtime_measure( 'start', 'hashes_api_scan' ); @@ -281,23 +291,16 @@ function vipgoci_hashes_api_scan_commit( ); - $files_seen_in_pr = array(); - $files_approved_in_pr = array(); - foreach( $pr_diff as $pr_diff_file_name => $pr_diff_contents ) { - $files_seen_in_pr[] = $pr_diff_file_name; - /* * Check if the hashes-to-hashes database * recognises this file, and check its * status. */ - // FIXME: Take into consideration the review-level of both - // the target-repo and of the code - $approval_status = vipgoci_hashes_api_file_approved( + $approval_status = vipgoci_ap_hashes_api_file_approved( $options, $pr_diff_file_name ); @@ -316,7 +319,18 @@ function vipgoci_hashes_api_scan_commit( ) ); - $files_approved_in_pr[] = $pr_diff_file_name; + /* + * If it is already approved, + * do not add again. + */ + + if ( ! in_array( + $pr_diff_file_name, + $auto_approved_files_arr, + true + ) ) { + $auto_approved_files_arr[] = $pr_diff_file_name; + } } else if ( false === $approval_status ) { @@ -341,149 +355,11 @@ function vipgoci_hashes_api_scan_commit( } } - /* - * Get label associated, but - * only our auto-approved one - */ - - $pr_label = vipgoci_github_labels_get( - $options['repo-owner'], - $options['repo-name'], - $options['token'], - (int) $pr_item->number, - $options['autoapprove-label'] - ); - - - /* - * If all seen files are found in approved in hashes-to-hashes, - * approve the Pull-Request and add a label. - * - * If only some files are approved, make a comment on these - * saying that the files are approved in hashes-to-hashes. - */ - - if ( - count( - array_diff( - $files_seen_in_pr, - $files_approved_in_pr - ) - ) === 0 - ) { - /* - * Actually approve, if not in dry-mode. - * Also add a label to the Pull-Request - * if applicable. - */ - - vipgoci_github_approve_pr( - $options['repo-owner'], - $options['repo-name'], - $options['token'], - $pr_item->number, - $options['commit'], - $options['autoapprove-filetypes'], - VIPGOCI_APPROVAL_HASHES_API, - $options['dry-run'] - ); - - // FIXME: We should have our own label - - /* Add label, if needed */ - if ( false === $pr_label ) { - vipgoci_github_label_add_to_pr( - $options['repo-owner'], - $options['repo-name'], - $options['token'], - $pr_item->number, - $options['autoapprove-label'], - $options['dry-run'] - ); - } - - else { - vipgoci_log( - 'Will not add label to issue, ' . - 'as it already exists', - - array( - 'repo_owner' => - $options['repo-owner'], - - 'repo_name' => - $options['repo-name'], - - 'pr_number' => - $pr_item->number, - - 'label_name' => - $options['autoapprove-label'], - ) - ); - } - } - - else { - /* - * Remove auto-approve label - */ - - if ( false !== $pr_label ) { - vipgoci_github_label_remove_from_pr( - $options['repo-owner'], - $options['repo-name'], - $options['token'], - (int) $pr_item->number, - $pr_label->name, - $options['dry-run'] - ); - } - - - /* - * Go through files that are approved, - * and add comment for them saying that - * they are approved already in the hashes-api - * database. - */ - foreach ( $files_approved_in_pr as $file_name ) { - // FIXME: Check if comment has been - // made before and do not re-post if so. - - /* - * Make comment for each file, noting - * that it is already approved. - */ - $commit_issues_submit[ - $pr_item->number - ][] = array( - 'type' => VIPGOCI_STATS_HASHES_API, - 'file_name' => $file_name, - 'file_line' => 1, - 'issue' - => array( - 'message' => - 'File is approved in ' . - 'hashes-to-hashes ' . - 'database', - 'level' => 'INFO', - ) - ); - - $commit_issues_stats[ - $pr_item->number - ]['info']++; - } - } - /* * Reduce memory-usage as possible */ - unset( $files_seen_in_pr ); - unset( $files_approved_in_pr ); unset( $prs_implicated ); unset( $pr_diff ); diff --git a/auto-approval.php b/auto-approval.php index 9cc236433..5dc2f138d 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -13,8 +13,7 @@ * in this function. */ -function vipgoci_auto_approval( $options ) { - +function vipgoci_auto_approval( $options, &$auto_approved_files_arr ) { vipgoci_runtime_measure( 'start', 'auto_approve_commit' ); vipgoci_log( @@ -55,6 +54,11 @@ function vipgoci_auto_approval( $options ) { $files_seen = array(); + /* + * Loop through all files that are + * altered by the Pull-Request, look for + * files that can be auo-approved. + */ foreach( $pr_diff as $pr_diff_file_name => $pr_diff_contents ) { @@ -69,11 +73,14 @@ function vipgoci_auto_approval( $options ) { ); + /* + * Is file in array of files + * that can be auto-approved? + * If not, we cannot auto-approve. + */ if ( ! in_array( - strtolower( - $pr_diff_file_extension - ), - $options['autoapprove-filetypes'], + $pr_diff_file_name, + $auto_approved_files_arr, true ) ) { $can_auto_approve = false; @@ -100,6 +107,9 @@ function vipgoci_auto_approval( $options ) { (int) $pr_item->number . ' ' . 'since no files were found', array( + 'auto_approved_files_arr' => + $auto_approved_files_arr, + 'files_seen' => $files_seen, ) ); @@ -113,12 +123,15 @@ function vipgoci_auto_approval( $options ) { 'Will not auto-approve Pull-Request #' . (int) $pr_item->number . ' ' . 'as it contains ' . "\n\t" . - 'file-types which are not ' . + 'files which are not ' . 'automatically approvable', array( 'autoapprove-filetypes' => $options['autoapprove-filetypes'], + 'auto_approved_files_arr' => + $auto_approved_files_arr, + 'files_seen' => $files_seen, ) ); @@ -163,7 +176,7 @@ function vipgoci_auto_approval( $options ) { 'auto-approve Pull-Request #' . (int) $pr_item->number . ' ' . 'as it alters or creates ' . "\n\t" . - 'only file-types that can be ' . + 'only files that can be ' . 'automatically approved', array( 'repo_owner' @@ -181,6 +194,9 @@ function vipgoci_auto_approval( $options ) { 'autoapprove-filetypes' => $options['autoapprove-filetypes'], + 'auto_approved_files_arr' => + $auto_approved_files_arr, + 'files_seen' => $files_seen, ) ); diff --git a/vip-go-ci.php b/vip-go-ci.php index 755f2dd23..9be61a435 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -8,7 +8,8 @@ require_once( __DIR__ . '/phpcs-scan.php' ); require_once( __DIR__ . '/lint-scan.php' ); require_once( __DIR__ . '/auto-approval.php' ); -require_once( __DIR__ . '/hashes-api.php' ); +require_once( __DIR__ . '/ap-file-types.php' ); +require_once( __DIR__ . '/ap-hashes-api.php' ); /* @@ -838,28 +839,44 @@ function vipgoci_run() { } /* - * If to auto-approve, then do so. + * If to auto-approve based on file-types, + * scan through the files in the PR, and + * register which can be auto-approved. */ + $auto_approved_files_arr = array(); - if ( true === $options['autoapprove'] ) { - // FIXME: Do not auto-approve if there are - // any linting or PHPCS-issues. - vipgoci_auto_approval( - $options - ); - } - + // FIXME: Command line arguments + vipgoci_ap_file_types( + $options, + $auto_approved_files_arr + ); /* * Do scanning of all altered files, using - * the hashes-to-hashes database API. + * the hashes-to-hashes database API, collecting + * which files can be auto-approved. */ if ( true === $options['hashes-api'] ) { - vipgoci_hashes_api_scan_commit( + vipgoci_ap_hashes_api_scan_commit( $options, $results['issues'], - $results['stats'][ VIPGOCI_STATS_HASHES_API ] + $results['stats'][ VIPGOCI_STATS_HASHES_API ], + $auto_approved_files_arr + ); + } + /* + * If to auto-approve, then do so. + */ + + if ( true === $options['autoapprove'] ) { + $auto_approved_files_arr = array(); + + // FIXME: Do not auto-approve if there are + // any linting or PHPCS-issues. + vipgoci_auto_approval( + $options, + $auto_approved_files_arr ); } From 5bfe920ffcc8b593aa1959689ff53c7514bd4248 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Wed, 5 Sep 2018 16:35:21 +0000 Subject: [PATCH 034/142] Unsetting more variables. --- ap-file-types.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ap-file-types.php b/ap-file-types.php index ce62115b6..5879ca061 100644 --- a/ap-file-types.php +++ b/ap-file-types.php @@ -96,6 +96,8 @@ function vipgoci_ap_file_types( unset( $prs_implicated ); unset( $pr_diff ); unset( $pr_item ); + unset( $pr_diff_file_extension ); + unset( $pr_diff_file_name ); gc_collect_cycles(); From a483deb42b33b608b274c01b182e6cc2298d9a21 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Thu, 6 Sep 2018 15:23:14 +0000 Subject: [PATCH 035/142] Introduce support for OAuth 1.0a authorization --- github-api.php | 204 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 202 insertions(+), 2 deletions(-) diff --git a/github-api.php b/github-api.php index c82772381..462e236df 100644 --- a/github-api.php +++ b/github-api.php @@ -159,6 +159,182 @@ function vipgoci_github_wait() { vipgoci_runtime_measure( 'stop', 'github_forced_wait' ); } +/* + * Calculate HMAC-SHA1 signature for OAuth 1.0 HTTP + * request. Follows the standard on this but to a + * limited extent only. For instance, this function + * does not support having two parameters with the + * same name. + * + * See here for background: + * https://oauth.net/core/1.0a/#signing_process + */ +function vipgoci_oauth1_signature_get_hmac_sha1( + $http_method, + $request_url, + $parameters_arr +) { + $parts = array(); + + /* + * Start constructing the 'base string' -- + * a crucial part of the signature. + */ + $base_string = strtoupper( $http_method ) . '&'; + $base_string .= rawurlencode( $request_url ) . '&'; + + + /* + * New array for parameters, temporary + * so we can alter them freely. + */ + $parameters_arr_new = array(); + + /* + * In case this parameter is present, it + * should not be part of the signature according + * to the standard. + */ + if ( isset( $parameters_arr['realm'] ) ) { + unset( $parameters_arr['realm'] ); + } + + /* + * Add parameters to the new array, these + * need to be encoded in a certain way. + */ + foreach( $parameters_arr as $key => $value ) { + $parameters_arr_new[ rawurlencode( $key ) ] = + rawurlencode( $value ); + } + + /* + * Also these two should not be part of the + * signature. + */ + unset( $parameters_arr_new['oauth_token_secret'] ); + unset( $parameters_arr_new['oauth_consumer_secret'] ); + + /* + * Sort the parameters alphabetically. + */ + ksort( $parameters_arr_new ); + + + /* + * Loop through the parameters, and add them + * to a temporary 'base string' according to the standard. + */ + + $delimiter = ''; + $base_string_tmp = ''; + + foreach( $parameters_arr_new as $key => $value ) { + $base_string_tmp .= + $delimiter . + $key . + '=' . + $value; + + $delimiter = '&'; + } + + /* + * Then add the temporary 'base string' to the + * permanent 'base string'. + */ + $base_string .= rawurlencode( + $base_string_tmp + ); + + + /* + * Now calculate hash, using the + * 'base string' as input, and + * secrets as key. + */ + $hash_raw = hash_hmac( + 'sha1', + $base_string, + $parameters_arr['oauth_consumer_secret'] . '&' . + $parameters_arr['oauth_token_secret'], + true + ); + + /* + * Return it base64 encoded. + */ + return base64_encode( $hash_raw ); +} + + +/* + * Create and set HTTP header for OAuth 1.0a requests, + * including timestamp, nonce, signature method + * (all part of the header) and then actually sign + * the request. Returns with a full HTTP header for + * a OAuth 1.0a HTTP request. + */ +function vipgoci_oauth1_headers_get( + $http_method, + $github_url, + $github_token +) { + + /* + * Set signature-method header, static. + */ + $github_token['oauth_signature_method'] = + 'HMAC-SHA1'; + + /* + * Set timestamp and nonce. + */ + $github_token['oauth_timestamp'] = (string) time(); + + $github_token['oauth_nonce'] = (string) md5( + openssl_random_pseudo_bytes( 100 ) + ); + + /* + * Get the signature for the header. + */ + $github_token['oauth_signature'] = + vipgoci_oauth1_signature_get_hmac_sha1( + $http_method, + $github_url, + $github_token + ); + + unset( $github_token['oauth_token_secret' ] ); + unset( $github_token['oauth_consumer_secret' ] ); + + /* + * Actually create the full HTTP header + */ + + $res_header = 'OAuth '; + $sep = ''; + + foreach( + $github_token as + $github_token_key => + $github_token_value + ) { + $res_header .= + $sep . + $github_token_key . '="' . + $github_token_value . + '"'; + $sep = ', '; + } + + /* + * Return the header. + */ + return $res_header; +} + /* * Send a POST/DELETE request to GitHub -- attempt @@ -421,7 +597,7 @@ function vipgoci_github_fetch_url( 'vipgoci_curl_headers' ); - if ( null !== $github_token ) { + if ( is_string( $github_token ) ) { curl_setopt( $ch, CURLOPT_HTTPHEADER, @@ -429,6 +605,30 @@ function vipgoci_github_fetch_url( ); } + else if ( is_array( $github_token ) ) { + if ( + ( isset( $github_token[ 'oauth_consumer_key' ] ) ) && + ( isset( $github_token[ 'oauth_consumer_secret' ] ) ) && + ( isset( $github_token[ 'oauth_token' ] ) ) && + ( isset( $github_token[ 'oauth_token_secret' ] ) ) + ) { + $github_auth_header = vipgoci_oauth1_headers_get( + 'GET', + $github_url, + $github_token + ); + + curl_setopt( + $ch, + CURLOPT_HTTPHEADER, + array( + 'Authorization: ' . + $github_auth_header + ) + ); + } + } + // Make sure to pause between GitHub-requests vipgoci_github_wait(); @@ -2014,7 +2214,7 @@ function vipgoci_github_authenticated_user_get( $github_token ) { ); } - if ( + if ( ( false === $current_user_info_json ) || ( null === $current_user_info ) ) { From f8865c2f86d1a76dc4d2247f3cd51ad05a80c5f3 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Thu, 6 Sep 2018 15:47:57 +0000 Subject: [PATCH 036/142] Whitespacing, adding support for 'modular auto-approvals'. Modular auto-approvals, essentially, means that auto-approvals happen within modules that individually decide which files can be approved. Then, when all modules have finished their work, a process goes through all the files altered by the PR, and figures out to do with the whole PR, based on this. If all the files are approved, the whole PR gets approved, but if only a subset of files is approved, then a message is attached to the file (in case of PHP and JS files). This functionality is not complete, but well underway. --- vip-go-ci.php | 66 ++++++++++++++++++++++++++------------------------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/vip-go-ci.php b/vip-go-ci.php index 9be61a435..7583603a0 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -454,7 +454,7 @@ function vipgoci_run() { $options['hashes-api-url'], '/' ); - } + } /* @@ -583,18 +583,17 @@ function vipgoci_run() { * and hases-api. */ ( - ( in_array( + ( in_array( 'php', $options['autoapprove-filetypes'], true ) ) || - ( in_array( + ( in_array( 'js', $options['autoapprove-filetypes'], true ) ) - ) ) { vipgoci_sysexit( @@ -839,38 +838,41 @@ function vipgoci_run() { } /* - * If to auto-approve based on file-types, - * scan through the files in the PR, and - * register which can be auto-approved. + * If to do auto-approvals, then do so now. + * First ask all 'auto-approval modules' + * to do their scanning, collecting all files that + * can be auto-approved, and then actually do the + * auto-approval if possible. */ - $auto_approved_files_arr = array(); - - // FIXME: Command line arguments - vipgoci_ap_file_types( - $options, - $auto_approved_files_arr - ); + if ( true === $options['autoapprove'] ) { + /* + * If to auto-approve based on file-types, + * scan through the files in the PR, and + * register which can be auto-approved. + */ + $auto_approved_files_arr = array(); - /* - * Do scanning of all altered files, using - * the hashes-to-hashes database API, collecting - * which files can be auto-approved. - */ + if ( ! empty( $options[ 'autoapprove-filetypes' ] ) ) { + vipgoci_ap_file_types( + $options, + $auto_approved_files_arr + ); + } - if ( true === $options['hashes-api'] ) { - vipgoci_ap_hashes_api_scan_commit( - $options, - $results['issues'], - $results['stats'][ VIPGOCI_STATS_HASHES_API ], - $auto_approved_files_arr - ); - } - /* - * If to auto-approve, then do so. - */ + /* + * Do scanning of all altered files, using + * the hashes-to-hashes database API, collecting + * which files can be auto-approved. + */ - if ( true === $options['autoapprove'] ) { - $auto_approved_files_arr = array(); + if ( true === $options['hashes-api'] ) { + vipgoci_ap_hashes_api_scan_commit( + $options, + $results['issues'], + $results['stats'][ VIPGOCI_STATS_HASHES_API ], + $auto_approved_files_arr + ); + } // FIXME: Do not auto-approve if there are // any linting or PHPCS-issues. From d0eb7c47be00a3e03aec609986a41b942cce6251 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Thu, 6 Sep 2018 16:28:02 +0000 Subject: [PATCH 037/142] Implement OAuth 1.0 authorization for hashes-to-hashes API calls --- ap-hashes-api.php | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/ap-hashes-api.php b/ap-hashes-api.php index 67e9c31bc..45bd4c84b 100644 --- a/ap-hashes-api.php +++ b/ap-hashes-api.php @@ -144,7 +144,7 @@ function vipgoci_ap_hashes_api_file_approved( '/v1/hashes/id/' . rawurlencode( $file_sha1 ); - /* + /* * Not really asking GitHub here, * but we can re-use the function * for this purpose. @@ -153,13 +153,26 @@ function vipgoci_ap_hashes_api_file_approved( $file_hashes_info = vipgoci_github_fetch_url( $hashes_to_hashes_url, - null + array( + 'oauth_consumer_key' => + $options['hashes-oauth-consumer-key'], + + 'oauth_consumer_secret' => + $options['hashes-oauth-consumer-secret'], + + 'oauth_token' => + $options['hashes-oauth-token'], + + 'oauth_token_secret' => + $options['hashes-oauth-token-secret'], + ) ); /* * Try to parse, and check for errors. */ + if ( false !== $file_hashes_info ) { $file_hashes_info = json_decode( $file_hashes_info, @@ -171,14 +184,7 @@ function vipgoci_ap_hashes_api_file_approved( if ( ( false === $file_hashes_info ) || ( null === $file_hashes_info ) || - ( - ( isset( $file_hashes_info['data']['status'] ) ) && - ( 404 === $file_hashes_info['data']['status'] ) - ) || - ( - ( isset( $file_hashes_info['data']['status'] ) ) && - ( 401 === $file_hashes_info['data']['status'] ) - ) + ( isset( $file_hashes_info['data']['status'] ) ) ) { vipgoci_log( 'Unable to get information from ' . @@ -193,7 +199,6 @@ function vipgoci_ap_hashes_api_file_approved( return null; } - $file_approved = null; /* @@ -203,7 +208,6 @@ function vipgoci_ap_hashes_api_file_approved( foreach( $file_hashes_info as $file_hash_info ) { if ( ! isset( $file_hash_info[ 'status' ] ) ) { - $file_approved = false; } From bb51bb82313c08f1139e7a6543a8962e33a6e706 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Thu, 6 Sep 2018 16:55:16 +0000 Subject: [PATCH 038/142] Adding support for hashes-oauth CLI arguments --- vip-go-ci.php | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/vip-go-ci.php b/vip-go-ci.php index 7583603a0..369ce350d 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -296,6 +296,10 @@ function vipgoci_run() { 'phpcs-standard:', 'phpcs-severity:', 'hashes-api-url:', + 'hashes-oauth-token:', + 'hashes-oauth-token-secret:', + 'hashes-oauth-consumer-key:', + 'hashes-oauth-consumer-secret:', 'php-path:', 'local-git-repo:', 'skip-folders:', @@ -368,6 +372,7 @@ function vipgoci_run() { "\t" . ' -- higher number indicates more detailed debugging-messages.' . PHP_EOL . "\t" . ' Default is zero' . PHP_EOL; +// FIXME: hashes oauth stuff exit( VIPGOCI_EXIT_USAGE_ERROR ); } @@ -456,6 +461,25 @@ function vipgoci_run() { ); } + /* + * Process hashes-oauth arguments + */ + + foreach( array( + 'hashes-oauth-token', + 'hashes-oauth-token-secret', + 'hashes-oauth-consumer-key', + 'hashes-oauth-consumer-secret' + ) as $tmp_key ) { + if ( ! isset( $options[ $tmp_key ] ) ) { + continue; + } + + $options[ $tmp_key ] = rtrim( trim( + $options[ $tmp_key ] + ) ); + } + /* * Handle --local-git-repo parameter @@ -570,6 +594,10 @@ function vipgoci_run() { ); } + // FIXME: If hashes is used, auto-approvals has to be used as well + // but not nessesary autoapprovals-filetypes + + // FIXME: If hashes-url is used, must use hashes-oauth parameters as well if ( ( true === $options['autoapprove'] ) && @@ -597,8 +625,8 @@ function vipgoci_run() { ) ) { vipgoci_sysexit( - 'PHP and JS files cannot be auto-approved, as they ' . - 'can contain serious problems for execution', + 'PHP and JS files cannot be auto-approved on file-type basis, as they ' . + 'can cause serious problems for execution', array( ), VIPGOCI_EXIT_USAGE_ERROR @@ -875,7 +903,8 @@ function vipgoci_run() { } // FIXME: Do not auto-approve if there are - // any linting or PHPCS-issues. + // any linting or PHPCS-issues -- but only + // for SVG files. vipgoci_auto_approval( $options, $auto_approved_files_arr From 4fb3638a568da247756b1c2148bfe5bf86c2f272 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Thu, 6 Sep 2018 16:55:32 +0000 Subject: [PATCH 039/142] Adding FIXMEs and debug --- auto-approval.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/auto-approval.php b/auto-approval.php index 5dc2f138d..ab27e50e9 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -26,6 +26,9 @@ function vipgoci_auto_approval( $options, &$auto_approved_files_arr ) { 'autoapprove-filetypes' => $options['autoapprove-filetypes'], + + 'autoapproved-files-arr' => + $auto_approved_files_arr, ) ); @@ -164,6 +167,10 @@ function vipgoci_auto_approval( $options, &$auto_approved_files_arr ) { $options['dry-run'] ); } + + // FIXME: Add comment for each hashes-to-hashes approved PHP and JS file, + // indicating that it is approved in the DB so human reviewers do not + // need to look at it again. } else if ( @@ -252,6 +259,8 @@ function vipgoci_auto_approval( $options, &$auto_approved_files_arr ) { ) ); } + + // FIXME: Remove any comments indicating that a file is approved. } unset( $files_seen ); From d755ec549dc11f515a2509a50835a50f1c7240a4 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Thu, 6 Sep 2018 17:47:20 +0000 Subject: [PATCH 040/142] --help message improved, sanity checking put in place for --hashes-oauth-* parameters --- vip-go-ci.php | 61 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 43 insertions(+), 18 deletions(-) diff --git a/vip-go-ci.php b/vip-go-ci.php index 369ce350d..12c41b7e5 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -250,6 +250,14 @@ function vipgoci_run() { global $argv; global $vipgoci_debug_level; + $hashes_oauth_arguments = + array( + 'hashes-oauth-token', + 'hashes-oauth-token-secret', + 'hashes-oauth-consumer-key', + 'hashes-oauth-consumer-secret' + ); + vipgoci_log( 'Initializing...', array() @@ -324,11 +332,11 @@ function vipgoci_run() { isset( $options['help'] ) ) { print 'Usage: ' . $argv[0] . PHP_EOL . - "\t" . 'Options --repo-owner, --repo-name, --commit, --token, --local-git-repo are ' . PHP_EOL . + "\t" . 'Options --repo-owner, --repo-name, --commit, --token, --local-git-repo, --phpcs-path are ' . PHP_EOL . "\t" . 'mandatory, while others are optional.' . PHP_EOL . PHP_EOL . - "\t" . 'Note that if option --autoapprove is specified, --autoapprove-filetypes and ' . PHP_EOL . - "\t" . '--autoapprove-label need to be specified as well.' . PHP_EOL . + "\t" . 'Note that if option --autoapprove is specified, --autoapprove-label needs to' . PHP_EOL . + "\t" . 'be specified as well.' . PHP_EOL . PHP_EOL . "\t" . '--repo-owner=STRING Specify repository owner, can be an organization' . PHP_EOL . "\t" . '--repo-name=STRING Specify name of the repository' . PHP_EOL . @@ -355,6 +363,13 @@ function vipgoci_run() { "\t" . '--hashes-api-url=STRING URL to hashes-to-hashes HTTP API root' . PHP_EOL . "\t" . ' -- note that it should not include any specific' . PHP_EOL . "\t" . ' paths to individual parts of the API.' . PHP_EOL . + PHP_EOL . + "\t" . '--hashes-oauth-token=STRING, --hashes-oauth-token-secret=STRING, ' . PHP_EOL . + "\t" . '--hashes-oauth-consumer-key=STRING, --hashes-oauth-consumer-secret=STRING ' . PHP_EOL . + "\t" . ' OAuth 1.0 token, token secret, consumer key and ' . PHP_EOL . + "\t" . ' consumer secret needed for hashes-to-hashes HTTP requests' . PHP_EOL . + "\t" . ' All required for hashes-to-hashes requests.' . PHP_EOL . + PHP_EOL . "\t" . '--branches-ignore=STRING,... What branches to ignore -- useful to make sure' . PHP_EOL . "\t" . ' some branches never get scanned. Separate branches' . PHP_EOL . "\t" . ' with commas' . PHP_EOL . @@ -372,7 +387,6 @@ function vipgoci_run() { "\t" . ' -- higher number indicates more detailed debugging-messages.' . PHP_EOL . "\t" . ' Default is zero' . PHP_EOL; -// FIXME: hashes oauth stuff exit( VIPGOCI_EXIT_USAGE_ERROR ); } @@ -465,12 +479,7 @@ function vipgoci_run() { * Process hashes-oauth arguments */ - foreach( array( - 'hashes-oauth-token', - 'hashes-oauth-token-secret', - 'hashes-oauth-consumer-key', - 'hashes-oauth-consumer-secret' - ) as $tmp_key ) { + foreach( $hashes_oauth_arguments as $tmp_key ) { if ( ! isset( $options[ $tmp_key ] ) ) { continue; } @@ -580,10 +589,7 @@ function vipgoci_run() { if ( ( true === $options['autoapprove'] ) && - ( - ( empty( $options['autoapprove-filetypes'] ) ) || - ( false === $options['autoapprove-label'] ) - ) + ( false === $options['autoapprove-label'] ) ) { vipgoci_sysexit( 'To be able to auto-approve, file-types to approve ' . @@ -594,10 +600,29 @@ function vipgoci_run() { ); } - // FIXME: If hashes is used, auto-approvals has to be used as well - // but not nessesary autoapprovals-filetypes + /* + * Do sanity-checking with hashes-api-url + * and --hashes-oauth-* parameters + */ + if ( isset( $options['hashes-api-url'] ) ) { + foreach ( $hashes_oauth_arguments as $tmp_key ) { + if ( ! isset( $options[ $tmp_key ] ) ) { + vipgoci_sysexit( + 'Asking to use --hashes-api-url without --hashes-oauth-* parameters, but that is not possible, as authorization is needed for hashes-to-hashes API', + array(), + VIPGOCI_EXIT_USAGE_ERROR + ); + } + } - // FIXME: If hashes-url is used, must use hashes-oauth parameters as well + if ( false === $options['autoapprove'] ) { + vipgoci_sysexit( + 'Asking to use --hashes-api-url without --autoapproval set to true, but for hashes-to-hashes functionality to be useful, --autoapprove must be enabled. Otherwise the functionality will not really be used', + array(), + VIPGOCI_EXIT_USAGE_ERROR + ); + } + } if ( ( true === $options['autoapprove'] ) && @@ -866,7 +891,7 @@ function vipgoci_run() { } /* - * If to do auto-approvals, then do so now. + * If to do auto-approvals, then do so now. * First ask all 'auto-approval modules' * to do their scanning, collecting all files that * can be auto-approved, and then actually do the From b4174243358819896bb19dafb01705d3b540b323 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 10 Sep 2018 14:35:08 +0000 Subject: [PATCH 041/142] Fix a problem with occasional rejection of OAuth signatures --- github-api.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/github-api.php b/github-api.php index 462e236df..84afec614 100644 --- a/github-api.php +++ b/github-api.php @@ -290,7 +290,7 @@ function vipgoci_oauth1_headers_get( /* * Set timestamp and nonce. */ - $github_token['oauth_timestamp'] = (string) time(); + $github_token['oauth_timestamp'] = (string) ( time() - 1); $github_token['oauth_nonce'] = (string) md5( openssl_random_pseudo_bytes( 100 ) @@ -324,7 +324,7 @@ function vipgoci_oauth1_headers_get( $res_header .= $sep . $github_token_key . '="' . - $github_token_value . + rawurlencode( $github_token_value ) . '"'; $sep = ', '; } From bf4697dcec1c7c866d214caff848c1e802181763 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 10 Sep 2018 14:36:08 +0000 Subject: [PATCH 042/142] Using array-keys, updating comment --- ap-file-types.php | 23 +++++++++++++---------- ap-hashes-api.php | 12 +++++++----- auto-approval.php | 8 ++++---- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/ap-file-types.php b/ap-file-types.php index 5879ca061..941736bd7 100644 --- a/ap-file-types.php +++ b/ap-file-types.php @@ -1,13 +1,14 @@ Date: Mon, 10 Sep 2018 21:47:47 +0000 Subject: [PATCH 043/142] Move constant to a new location, remove duplicates --- defines.php | 1 + github-api.php | 9 --------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/defines.php b/defines.php index df95c5d54..0064c23f1 100644 --- a/defines.php +++ b/defines.php @@ -7,6 +7,7 @@ define( 'VIPGOCI_CLIENT_ID', 'automattic-vip-go-ci' ); define( 'VIPGOCI_SYNTAX_ERROR_STR', 'PHP Syntax Errors Found' ); define( 'VIPGOCI_GITHUB_ERROR_STR', 'GitHub API communication error'); +define( 'VIPGOCI_GITHUB_BASE_URL', 'https://api.github.com' ); /* diff --git a/github-api.php b/github-api.php index 5ad307e0c..93d78d236 100644 --- a/github-api.php +++ b/github-api.php @@ -1,14 +1,5 @@ Date: Mon, 10 Sep 2018 21:49:14 +0000 Subject: [PATCH 044/142] Hide secrets when printing out options --- vip-go-ci.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/vip-go-ci.php b/vip-go-ci.php index 12c41b7e5..e82e92a39 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -714,6 +714,12 @@ function vipgoci_run() { $options_clean = $options; $options_clean['token'] = '***'; + foreach( $hashes_oauth_arguments as $hashes_oauth_argument ) { + if ( isset( $options_clean[ $hashes_oauth_argument ] ) ) { + $options_clean[ $hashes_oauth_argument ] = '***'; + } + } + vipgoci_log( 'Starting up...', array( From 56dbd70061470c62e0c20d566482d38b906c9c8f Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 11 Sep 2018 13:25:35 +0000 Subject: [PATCH 045/142] Clarify code a bit --- github-api.php | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/github-api.php b/github-api.php index 93d78d236..9b65b23c8 100644 --- a/github-api.php +++ b/github-api.php @@ -1353,23 +1353,21 @@ function vipgoci_github_pr_comments_cleanup( ( strpos( $pr_comment->body, VIPGOCI_SYNTAX_ERROR_STR - ) === false ) - && + ) !== false ) + || ( strpos( $pr_comment->body, VIPGOCI_GITHUB_ERROR_STR - ) === false ) + ) !== false ) ) { - continue; + // Actually delete the comment + vipgoci_github_pr_generic_comment_delete( + $repo_owner, + $repo_name, + $github_token, + $pr_comment->id + ); } - - // Actually delete the comment - vipgoci_github_pr_generic_comment_delete( - $repo_owner, - $repo_name, - $github_token, - $pr_comment->id - ); } } } From 1c3dfd0e60a8760e2d4e2ed674da391a42693d53 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 11 Sep 2018 15:22:36 +0000 Subject: [PATCH 046/142] Remove token from log, fix logging issue --- ap-hashes-api.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ap-hashes-api.php b/ap-hashes-api.php index 80b97dff0..02bc2744c 100644 --- a/ap-hashes-api.php +++ b/ap-hashes-api.php @@ -64,7 +64,6 @@ function vipgoci_ap_hashes_api_file_approved( array( 'repo_owner' => $options['repo-owner'], 'repo_name' => $options['repo-name'], - 'token' => $options['token'], 'commit' => $options['commit'], 'file_path' => $file_path, ) @@ -264,7 +263,7 @@ function vipgoci_ap_hashes_api_scan_commit( vipgoci_runtime_measure( 'start', 'hashes_api_scan' ); vipgoci_log( - 'Scanning altered or new files affected by Pull-Request(s) ', + 'Scanning altered or new files affected by Pull-Request(s) ' . 'using hashes-to-hashes database via API', array( 'repo_owner' => $options['repo-owner'], From 3ada1e00b5eb9d9251d5b8c8c1ead99b1ac1ca66 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 11 Sep 2018 15:23:05 +0000 Subject: [PATCH 047/142] Adding FIXME --- auto-approval.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/auto-approval.php b/auto-approval.php index bf0342295..471d74b16 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -171,6 +171,8 @@ function vipgoci_auto_approval( $options, &$auto_approved_files_arr ) { // FIXME: Add comment for each hashes-to-hashes approved PHP and JS file, // indicating that it is approved in the DB so human reviewers do not // need to look at it again. + + // FIXME: Dismiss any approving reviews from the PR. } else if ( From 34713d5773a20d1355ca3137a59ae1f77913fe9f Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 11 Sep 2018 15:23:21 +0000 Subject: [PATCH 048/142] Adding SVG autoapprovals and FIXME --- vip-go-ci.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/vip-go-ci.php b/vip-go-ci.php index e82e92a39..cd4e2a583 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -10,7 +10,7 @@ require_once( __DIR__ . '/auto-approval.php' ); require_once( __DIR__ . '/ap-file-types.php' ); require_once( __DIR__ . '/ap-hashes-api.php' ); - +require_once( __DIR__ . '/ap-svg-files.php' ); /* * Handle boolean parameters given on the command-line. @@ -933,9 +933,12 @@ function vipgoci_run() { ); } - // FIXME: Do not auto-approve if there are - // any linting or PHPCS-issues -- but only - // for SVG files. + // FIXME: Use options here + vipgoci_ap_svg_files( + $options, + $auto_approved_files_arr + ); + vipgoci_auto_approval( $options, $auto_approved_files_arr From ba5ef9c21348107fb29ad3adbb646bdd16c9f508 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 11 Sep 2018 15:24:00 +0000 Subject: [PATCH 049/142] Auto-approve SVG files which are PHPCS-issue free --- ap-svg-files.php | 155 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 155 insertions(+) create mode 100644 ap-svg-files.php diff --git a/ap-svg-files.php b/ap-svg-files.php new file mode 100644 index 000000000..7ef257942 --- /dev/null +++ b/ap-svg-files.php @@ -0,0 +1,155 @@ + $options['repo-owner'], + 'repo_name' => $options['repo-name'], + 'commit_id' => $options['commit'], + ) + ); + + + $prs_implicated = vipgoci_github_prs_implicated( + $options['repo-owner'], + $options['repo-name'], + $options['commit'], + $options['token'], + $options['branches-ignore'] + ); + + + foreach ( $prs_implicated as $pr_item ) { + $pr_diff = vipgoci_github_diffs_fetch( + $options['repo-owner'], + $options['repo-name'], + $options['token'], + $pr_item->base->sha, + $options['commit'] + ); + + + foreach ( $pr_diff as + $pr_diff_file_name => $pr_diff_contents + ) { + $pr_diff_file_extension = pathinfo( + $pr_diff_file_name, + PATHINFO_EXTENSION + ); + + /* + * If not a SVG file, do not do anything. + */ + + if ( + strtolower( $pr_diff_file_extension ) !== + 'svg' + ) { + continue; + } + + /* + * If the file is already in the array + * of approved files, do not do anything. + */ + if ( isset( + $auto_approved_files_arr[ + $pr_diff_file_name + ] + ) ) { + continue; + } + + /* + * PHPCS scan the file, get the results. + */ + $tmp_scan_results = vipgoci_phpcs_scan_single_file( + $options, + $pr_diff_file_name + ); + + $file_issues_arr_master = + $tmp_scan_results['file_issues_arr_master']; + + /* + * If no issues were found, we + * can approve this file. + */ + if ( + ( isset( + $file_issues_arr_master['totals'] + ) ) + && + ( 0 === + $file_issues_arr_master['totals']['errors'] + ) + && + ( 0 === + $file_issues_arr_master['totals']['warnings'] + ) + ) { + vipgoci_log( + 'Adding SVG file to list of approved ' . + 'files, as no PHPCS-issues ' . + 'were found', + array( + 'file_name' => + $pr_diff_file_name, + ) + ); + + $auto_approved_files_arr[ + $pr_diff_file_name + ] = 'ap-svg-files'; + } + + else { + vipgoci_log( + 'Not adding SVG file to list of ' . + 'approved files as issues' . + 'were found', + array( + 'file_name' => + $pr_diff_file_name, + ) + ); + } + } + } + + /* + * Reduce memory-usage as possible + */ + unset( $tmp_scan_results ); + unset( $prs_implicated ); + unset( $pr_diff ); + unset( $pr_item ); + unset( $pr_diff_file_extension ); + unset( $pr_diff_file_name ); + + gc_collect_cycles(); + + vipgoci_runtime_measure( 'stop', 'ap_svg_files' ); +} + From 41c2c4fe22d8af6c72f062d0688ffed85e48391d Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 11 Sep 2018 15:24:40 +0000 Subject: [PATCH 050/142] Break out functionality into function, deal with PHPCS files when scanning --- phpcs-scan.php | 149 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 99 insertions(+), 50 deletions(-) diff --git a/phpcs-scan.php b/phpcs-scan.php index f0116c6c2..4471d5a8e 100644 --- a/phpcs-scan.php +++ b/phpcs-scan.php @@ -11,6 +11,19 @@ function vipgoci_phpcs_do_scan( $phpcs_standard, $phpcs_severity ) { + /* + * Determine file-extension of the + * file. + */ + $filename_extension = pathinfo( + $filename_tmp, + PATHINFO_EXTENSION + ); + + $filename_extension = strtolower( + $filename_extension + ); + /* * Run PHPCS from the shell, making sure we escape everything. * @@ -18,10 +31,8 @@ function vipgoci_phpcs_do_scan( * * Make sure to use wide enough output, so we can catch all of it. */ -// FIXME: When dealing with SVG files -// use: --extensions=svg --sniffs=WordPressVIPMinimum.SVG.HTMLCode $cmd = sprintf( - '%s %s --standard=%s --severity=%s --report=%s %s 2>&1', + '%s %s --standard=%s --severity=%s --report=%s %s', escapeshellcmd( 'php' ), escapeshellcmd( $phpcs_path ), escapeshellarg( $phpcs_standard ), @@ -30,6 +41,20 @@ function vipgoci_phpcs_do_scan( escapeshellarg( $filename_tmp ) ); + /* + * If this is a SVG file, we need special arguments + * for PHPCS. + */ + if ( 'svg' === $filename_extension ) { + $cmd .= sprintf( + ' --extensions=%s --sniffs=%s', + escapeshellarg( 'svg' ), + escapeshellarg( 'WordPressVIPMinimum.SVG.HTMLCode' ) + ); + } + + $cmd .= ' 2>&1'; + vipgoci_runtime_measure( 'start', 'phpcs_cli' ); $result = shell_exec( $cmd ); @@ -39,6 +64,68 @@ function vipgoci_phpcs_do_scan( return $result; } +function vipgoci_phpcs_scan_single_file( + $options, + $file_name +) { + $file_contents = vipgoci_gitrepo_fetch_committed_file( + $options['repo-owner'], + $options['repo-name'], + $options['token'], + $options['commit'], + $file_name, + $options['local-git-repo'] + ); + + $file_extension = pathinfo( + $file_name, + PATHINFO_EXTENSION + ); + + if ( empty( $file_extension ) ) { + $file_extension = null; + } + + $temp_file_name = vipgoci_save_temp_file( + 'phpcs-scan-', + $file_extension, + $file_contents + ); + + vipgoci_log( + 'About to PHPCS-scan file', + array( + 'repo_owner' => $options['repo-owner'], + 'repo_name' => $options['repo-name'], + 'commit_id' => $options['commit'], + 'filename' => $file_name, + 'temp_file_name' => $temp_file_name, + ) + ); + + + $file_issues_str = vipgoci_phpcs_do_scan( + $temp_file_name, + $options['phpcs-path'], + $options['phpcs-standard'], + $options['phpcs-severity'] + ); + + /* Get rid of temporary file */ + unlink( $temp_file_name ); + + $file_issues_arr_master = json_decode( + $file_issues_str, + true + ); + + return array( + 'file_issues_arr_master' => $file_issues_arr_master, + 'file_issues_str' => $file_issues_str, + 'temp_file_name' => $temp_file_name, + ); +} + /* * Dump output of scan-analysis to a file, @@ -372,57 +459,19 @@ function vipgoci_phpcs_scan_commit( */ vipgoci_runtime_measure( 'start', 'phpcs_scan_single_file' ); - $file_contents = vipgoci_gitrepo_fetch_committed_file( - $repo_owner, - $repo_name, - $github_token, - $commit_id, - $file_name, - $options['local-git-repo'] + $tmp_scanning_results = vipgoci_phpcs_scan_single_file( + $options, + $file_name ); - $file_extension = pathinfo( - $file_name, - PATHINFO_EXTENSION - ); + $file_issues_arr_master = + $tmp_scanning_results['file_issues_arr_master']; - if ( empty( $file_extension ) ) { - $file_extension = null; - } - - $temp_file_name = vipgoci_save_temp_file( - 'phpcs-scan-', - $file_extension, - $file_contents - ); - - vipgoci_log( - 'About to PHPCS-scan file', - array( - 'repo_owner' => $repo_owner, - 'repo_name' => $repo_name, - 'commit_id' => $commit_id, - 'filename' => $file_name, - 'temp_file_name' => $temp_file_name, - ) - ); - - - $file_issues_str = vipgoci_phpcs_do_scan( - $temp_file_name, - $options['phpcs-path'], - $options['phpcs-standard'], - $options['phpcs-severity'] - ); - - /* Get rid of temporary file */ - unlink( $temp_file_name ); - - $file_issues_arr_master = json_decode( - $file_issues_str, - true - ); + $file_issues_str = + $tmp_scanning_results['file_issues_str']; + $temp_file_name = + $tmp_scanning_results['temp_file_name']; /* * Do sanity-checking From cd321a5fb084e482b48b9ec7842bb82267dde996 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 11 Sep 2018 16:02:46 +0000 Subject: [PATCH 051/142] Move filename to the last argument to PHPCS --- phpcs-scan.php | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/phpcs-scan.php b/phpcs-scan.php index 4471d5a8e..435ad2335 100644 --- a/phpcs-scan.php +++ b/phpcs-scan.php @@ -32,13 +32,12 @@ function vipgoci_phpcs_do_scan( * Make sure to use wide enough output, so we can catch all of it. */ $cmd = sprintf( - '%s %s --standard=%s --severity=%s --report=%s %s', + '%s %s --standard=%s --severity=%s --report=%s', escapeshellcmd( 'php' ), escapeshellcmd( $phpcs_path ), escapeshellarg( $phpcs_standard ), escapeshellarg( $phpcs_severity ), - escapeshellarg( 'json' ), - escapeshellarg( $filename_tmp ) + escapeshellarg( 'json' ) ); /* @@ -53,8 +52,25 @@ function vipgoci_phpcs_do_scan( ); } + /* + * Lastly, append the target filename + * to the command-line string. + */ + $cmd .= sprintf( + ' %s', + escapeshellarg( $filename_tmp ) + ); + $cmd .= ' 2>&1'; + vipgoci_log( + 'Running PHPCS now', + array( + 'cmd' => $cmd, + ), + 2 + ); + vipgoci_runtime_measure( 'start', 'phpcs_cli' ); $result = shell_exec( $cmd ); From d9ba7f4955786950544ec21b7faf9630322f8756 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 11 Sep 2018 16:07:22 +0000 Subject: [PATCH 052/142] Adding whitespace to string --- ap-svg-files.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ap-svg-files.php b/ap-svg-files.php index 7ef257942..addbf7ddd 100644 --- a/ap-svg-files.php +++ b/ap-svg-files.php @@ -127,7 +127,7 @@ function vipgoci_ap_svg_files( else { vipgoci_log( 'Not adding SVG file to list of ' . - 'approved files as issues' . + 'approved files as issues ' . 'were found', array( 'file_name' => From ab9a00b4ec28d3023836348b5a3b78f4b0e1209c Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 17 Sep 2018 15:40:39 +0000 Subject: [PATCH 053/142] Do not rely on PHPCS for scanning SVG files --- phpcs-scan.php | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/phpcs-scan.php b/phpcs-scan.php index 435ad2335..e31c2c722 100644 --- a/phpcs-scan.php +++ b/phpcs-scan.php @@ -11,19 +11,6 @@ function vipgoci_phpcs_do_scan( $phpcs_standard, $phpcs_severity ) { - /* - * Determine file-extension of the - * file. - */ - $filename_extension = pathinfo( - $filename_tmp, - PATHINFO_EXTENSION - ); - - $filename_extension = strtolower( - $filename_extension - ); - /* * Run PHPCS from the shell, making sure we escape everything. * @@ -40,18 +27,6 @@ function vipgoci_phpcs_do_scan( escapeshellarg( 'json' ) ); - /* - * If this is a SVG file, we need special arguments - * for PHPCS. - */ - if ( 'svg' === $filename_extension ) { - $cmd .= sprintf( - ' --extensions=%s --sniffs=%s', - escapeshellarg( 'svg' ), - escapeshellarg( 'WordPressVIPMinimum.SVG.HTMLCode' ) - ); - } - /* * Lastly, append the target filename * to the command-line string. From deeba239d25510dff6a7ce2050b3160ee1fd0d87 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 17 Sep 2018 15:51:17 +0000 Subject: [PATCH 054/142] Using function to determine file-extension --- ap-file-types.php | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/ap-file-types.php b/ap-file-types.php index 941736bd7..6ff92881b 100644 --- a/ap-file-types.php +++ b/ap-file-types.php @@ -57,9 +57,8 @@ function vipgoci_ap_file_types( foreach ( $pr_diff as $pr_diff_file_name => $pr_diff_contents ) { - $pr_diff_file_extension = pathinfo( - $pr_diff_file_name, - PATHINFO_EXTENSION + $pr_diff_file_extension = vipgoci_file_extension( + $pr_diff_file_name ); /* @@ -80,9 +79,7 @@ function vipgoci_ap_file_types( * file extensions. */ if ( in_array( - strtolower( - $pr_diff_file_extension - ), + $pr_diff_file_extension, $options['autoapprove-filetypes'], true ) ) { From 44573bb92ff6dbbb071bb2b92b8942882a4d5014 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 17 Sep 2018 15:51:53 +0000 Subject: [PATCH 055/142] Using function to determine file-extension --- ap-hashes-api.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/ap-hashes-api.php b/ap-hashes-api.php index 02bc2744c..66f82bb94 100644 --- a/ap-hashes-api.php +++ b/ap-hashes-api.php @@ -27,14 +27,13 @@ function vipgoci_ap_hashes_api_file_approved( ); - $file_info_extension = pathinfo( - $file_path, - PATHINFO_EXTENSION + $file_info_extension = vipgoci_file_extension( + $file_path ); if ( in_array( - strtolower( $file_info_extension ), + $file_info_extension, $file_extensions_approvable ) === false ) { vipgoci_log( From 617d714eff72b61aaa4aa2ff04d98fc8d589fb05 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 17 Sep 2018 15:55:55 +0000 Subject: [PATCH 056/142] Introduce function to determine file-extension, lowercased --- misc.php | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/misc.php b/misc.php index f15c76aaa..8be38ea0e 100644 --- a/misc.php +++ b/misc.php @@ -292,6 +292,27 @@ function vipgoci_save_temp_file( return $temp_file_name; } +/* + * Determine file-extension of a particular file, + * and return it in lowercase. If it can not be + * determined, return null. + */ +function vipgoci_file_extension( $file_name ) { + $file_extension = pathinfo( + $file_name, + PATHINFO_EXTENSION + ); + + if ( empty( $file_extension ) ) { + return null; + } + + $file_extension = strtolower( + $file_extension + ); + + return $file_extension; +} /* * Return ASCII-art for GitHub, which will then @@ -324,9 +345,8 @@ function vipgoci_filter_file_path( $filename, $filter ) { - $file_info_extension = pathinfo( - $filename, - PATHINFO_EXTENSION + $file_info_extension = vipgoci_file_extension( + $filename ); $file_dirs = pathinfo( @@ -343,9 +363,9 @@ function vipgoci_filter_file_path( ( null !== $filter ) && ( isset( $filter['file_extensions'] ) ) && ( ! in_array( - strtolower( $file_info_extension ), - $filter['file_extensions'], - true + $file_info_extension, + $filter['file_extensions'], + true ) ); /* From 34e908404be97c53206029a36094d35fd78961e9 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 17 Sep 2018 15:56:28 +0000 Subject: [PATCH 057/142] Introducing function to scan for problems in SVG files --- ap-svg-files.php | 14 ++-- svg-scan.php | 171 +++++++++++++++++++++++++++++++++++++++++++++++ vip-go-ci.php | 1 + 3 files changed, 179 insertions(+), 7 deletions(-) create mode 100644 svg-scan.php diff --git a/ap-svg-files.php b/ap-svg-files.php index addbf7ddd..68726df0b 100644 --- a/ap-svg-files.php +++ b/ap-svg-files.php @@ -53,9 +53,8 @@ function vipgoci_ap_svg_files( foreach ( $pr_diff as $pr_diff_file_name => $pr_diff_contents ) { - $pr_diff_file_extension = pathinfo( - $pr_diff_file_name, - PATHINFO_EXTENSION + $pr_diff_file_extension = vipgoci_file_extension( + $pr_diff_file_name ); /* @@ -63,8 +62,8 @@ function vipgoci_ap_svg_files( */ if ( - strtolower( $pr_diff_file_extension ) !== - 'svg' + 'svg' !== + $pr_diff_file_extension ) { continue; } @@ -82,9 +81,10 @@ function vipgoci_ap_svg_files( } /* - * PHPCS scan the file, get the results. + * Scan the SVG file, get the results. */ - $tmp_scan_results = vipgoci_phpcs_scan_single_file( + + $tmp_scan_results = vipgoci_svg_scan_single_file( $options, $pr_diff_file_name ); diff --git a/svg-scan.php b/svg-scan.php new file mode 100644 index 000000000..22e86965d --- /dev/null +++ b/svg-scan.php @@ -0,0 +1,171 @@ + array(), + 'errors' => 0, + 'warnings' => 0, + 'fixable' => 0, + ); + } + + /* + * Scan for each disallowed token + */ + foreach( $disallowed_tokens as $disallowed_token ) { + /* + * Do a case insensitive search + */ + $token_pos = stripos( + $file_line_item, + $disallowed_token + ); + + if ( false === $token_pos ) { + continue; + } + + + $results_files[ $temp_file_name ]['errors']++; + + $results_files[ $temp_file_name ]['messages'][] = + array( + 'message' => 'Found PHP tag in SVG file: \'' . $disallowed_token . '\'', + 'source' => 'WordPressVIPMinimum.Security.SVG.DisallowedTags', + 'severity' => 5, + 'fixable' => 0, + 'type' => 'ERROR', + 'line' => $line_no, + 'column' => $token_pos, + ); + } + + $line_no++; + } + + $results = array( + 'totals' => array( + 'errors' => $results_files[ + $temp_file_name + ]['errors'], + + 'warnings' => $results_files[ + $temp_file_name + ]['warnings'], + + 'fixable' => $results_files[ + $temp_file_name + ]['fixable'], + ), + + 'files' => array( + $results_files[ + $temp_file_name + ] + ) + ); + + + /* + * Emulate results returned + * by vipgoci_phpcs_scan_single_file() + */ + + return array( + 'file_issues_arr_master' => $results, + 'file_issues_str' => json_encode( $results ), + 'temp_file_name' => $temp_file_name, + ); +} + diff --git a/vip-go-ci.php b/vip-go-ci.php index cd4e2a583..10489282e 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -11,6 +11,7 @@ require_once( __DIR__ . '/ap-file-types.php' ); require_once( __DIR__ . '/ap-hashes-api.php' ); require_once( __DIR__ . '/ap-svg-files.php' ); +require_once( __DIR__ . '/svg-scan.php' ); /* * Handle boolean parameters given on the command-line. From 40cb03fd717d7391305902d3021dd46c1fb1592b Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 17 Sep 2018 16:00:51 +0000 Subject: [PATCH 058/142] Fix conflict --- defines.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/defines.php b/defines.php index 0064c23f1..fbb0c3b1a 100644 --- a/defines.php +++ b/defines.php @@ -9,6 +9,11 @@ define( 'VIPGOCI_GITHUB_ERROR_STR', 'GitHub API communication error'); define( 'VIPGOCI_GITHUB_BASE_URL', 'https://api.github.com' ); +define( 'VIPGOCI_INFORMATIONAL_MESSAGE', + 'This bot provides automated ' . + 'PHP Linting and PHPCS scanning, ' . + 'read more [here](%s).' +); /* * Define exit-codes From 9951d6ac473f5d395f6d502420155c9209e3930d Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 17 Sep 2018 16:15:36 +0000 Subject: [PATCH 059/142] Adding comments, fixing problems --- svg-scan.php | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/svg-scan.php b/svg-scan.php index 22e86965d..146a75714 100644 --- a/svg-scan.php +++ b/svg-scan.php @@ -53,7 +53,7 @@ function vipgoci_svg_scan_single_file( } $temp_file_name = vipgoci_save_temp_file( - 'phpcs-scan-', + 'svg-scan-', $file_extension, $file_contents ); @@ -116,6 +116,9 @@ function vipgoci_svg_scan_single_file( continue; } + /* + * Found a problem, adding to results. + */ $results_files[ $temp_file_name ]['errors']++; @@ -134,6 +137,11 @@ function vipgoci_svg_scan_single_file( $line_no++; } + /* + * Emulate results returned + * by vipgoci_phpcs_scan_single_file(). + */ + $results = array( 'totals' => array( 'errors' => $results_files[ @@ -150,18 +158,13 @@ function vipgoci_svg_scan_single_file( ), 'files' => array( - $results_files[ - $temp_file_name - ] + $temp_file_name => + $results_files[ + $temp_file_name + ] ) ); - - /* - * Emulate results returned - * by vipgoci_phpcs_scan_single_file() - */ - return array( 'file_issues_arr_master' => $results, 'file_issues_str' => json_encode( $results ), From 7eb2c6ae4e6eaf05be7729a0176fcf1369135edd Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 17 Sep 2018 19:44:53 +0000 Subject: [PATCH 060/142] Making compliant with output from PHPCS --- svg-scan.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/svg-scan.php b/svg-scan.php index 146a75714..b218cec65 100644 --- a/svg-scan.php +++ b/svg-scan.php @@ -93,10 +93,10 @@ function vipgoci_svg_scan_single_file( */ if ( ! isset( $results_files[ $temp_file_name ] ) ) { $results_files[ $temp_file_name ] = array( - 'messages' => array(), 'errors' => 0, 'warnings' => 0, 'fixable' => 0, + 'messages' => array(), ); } @@ -127,7 +127,7 @@ function vipgoci_svg_scan_single_file( 'message' => 'Found PHP tag in SVG file: \'' . $disallowed_token . '\'', 'source' => 'WordPressVIPMinimum.Security.SVG.DisallowedTags', 'severity' => 5, - 'fixable' => 0, + 'fixable' => false, 'type' => 'ERROR', 'line' => $line_no, 'column' => $token_pos, From 87e9237a3dfe0db94eb2f4f8aef000550ce0b860 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 17 Sep 2018 19:46:14 +0000 Subject: [PATCH 061/142] Introducing SVG scanning --- phpcs-scan.php | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/phpcs-scan.php b/phpcs-scan.php index e31c2c722..2b148a4a6 100644 --- a/phpcs-scan.php +++ b/phpcs-scan.php @@ -398,7 +398,7 @@ function vipgoci_phpcs_scan_commit( $commit_id, array( 'file_extensions' => - array( 'php', 'js', 'twig' ), + array( 'php', 'js', 'twig', 'svg' ), 'skip_folders' => $options['skip-folders'], ) @@ -450,7 +450,21 @@ function vipgoci_phpcs_scan_commit( */ vipgoci_runtime_measure( 'start', 'phpcs_scan_single_file' ); - $tmp_scanning_results = vipgoci_phpcs_scan_single_file( + $file_extension = vipgoci_file_extension( + $file_name + ); + + /* + * If a SVG file, scan using a + * custom internal function, otherwise + * use PHPCS. + */ + $scanning_func = + ( 'svg' === $file_extension ) ? + 'vipgoci_svg_scan_single_file' : + 'vipgoci_phpcs_scan_single_file'; + + $tmp_scanning_results = $scanning_func( $options, $file_name ); From 94a4814e67b3450344eda4bc098468580fc39e61 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 17 Sep 2018 19:46:53 +0000 Subject: [PATCH 062/142] Bugfixes Moving informational message around so it is appended only once, making printing of 's' work correctly. --- github-api.php | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/github-api.php b/github-api.php index a1e9f7d3a..7ad8f6557 100644 --- a/github-api.php +++ b/github-api.php @@ -1669,25 +1669,24 @@ function vipgoci_github_pr_review_submit( $commit_issue_stat_value . ' ' . $commit_issue_stat_key . - ( $commit_issue_stat_value > 1 ) ? '' :'s' . + ( ( $commit_issue_stat_value > 1 ) ? 's' : '' ) . ' ' . "\n\r"; } + } - - /* - * If we have a informational-URL about - * the bot, append it along with a generic - * message. - */ - if ( null !== $informational_url ) { - $github_postfields['body'] .= - "\n\r***\n\r" . - sprintf( - VIPGOCI_INFORMATIONAL_MESSAGE, - $informational_url - ); - } + /* + * If we have a informational-URL about + * the bot, append it along with a generic + * message. + */ + if ( null !== $informational_url ) { + $github_postfields['body'] .= + "\n\r***\n\r" . + sprintf( + VIPGOCI_INFORMATIONAL_MESSAGE, + $informational_url + ); } From 493ed4bfbe852ca1e8122eceb12d84a796e784ee Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 17 Sep 2018 21:05:26 +0000 Subject: [PATCH 063/142] Do not report a statistic if it is at zero, skip 'hashes-api' statistics --- github-api.php | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/github-api.php b/github-api.php index 7ad8f6557..9ee4399d6 100644 --- a/github-api.php +++ b/github-api.php @@ -1622,6 +1622,16 @@ function vipgoci_github_pr_review_submit( $stats_types_to_process as $stats_type ) { + /* + * No report for 'hashes-api', + * as it can report no issues back + * and statistics for it will always + * be at zero. + */ + if ( 'hashes-api' === $stats_type ) { + continue; + } + /* * Add page-breaking, if needed. */ @@ -1648,7 +1658,6 @@ function vipgoci_github_pr_review_submit( continue; } - $github_postfields['body'] .= '**' . $stats_type . '**' . " scanning turned up:\n\r"; @@ -1662,6 +1671,14 @@ function vipgoci_github_pr_review_submit( $commit_issue_stat_key => $commit_issue_stat_value ) { + /* + * Do not include statistic in the + * the report if there is nothing found. + */ + if ( 0 === $commit_issue_stat_value ) { + continue; + } + $github_postfields['body'] .= vipgoci_github_labels( $commit_issue_stat_key From e416685979a426bf546af5ea881b572dc03186a1 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 17 Sep 2018 21:10:50 +0000 Subject: [PATCH 064/142] Adding comment. --- github-api.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/github-api.php b/github-api.php index 9ee4399d6..54d615e1d 100644 --- a/github-api.php +++ b/github-api.php @@ -1673,7 +1673,12 @@ function vipgoci_github_pr_review_submit( ) { /* * Do not include statistic in the - * the report if there is nothing found. + * the report if nothing is found. + * + * Note that if nothing is found at + * all, we will not get to this point, + * so there is no need to report if + * nothing is found at all. */ if ( 0 === $commit_issue_stat_value ) { continue; From b4025403311167d3dbd2274728e25ea1615cfe0f Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 17 Sep 2018 21:46:52 +0000 Subject: [PATCH 065/142] If needed, HTML encode proposed issue-comment --- misc.php | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/misc.php b/misc.php index 8be38ea0e..25b9fa2ea 100644 --- a/misc.php +++ b/misc.php @@ -760,9 +760,33 @@ function vipgoci_github_comment_match( ': ' ); + /* + * Transform strings to lowercase. + */ + $comment_made_body = strtolower( + $comment_made_body + ); + + $file_issue_comment = strtolower( + $file_issue_comment + ); + + /* + * Check if comments match, including + * if we need to HTML-encode our new comment + * (GitHub encodes their comments when + * returning them. + */ if ( - strtolower( $comment_made_body ) == - strtolower( $file_issue_comment ) + ( + $comment_made_body == + $file_issue_comment + ) + || + ( + $comment_made_body == + htmlentities( $file_issue_comment ) + ) ) { /* Comment found, return true. */ return true; From db80999fb6190b80014a3415092047f7278a7321 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 17 Sep 2018 21:47:15 +0000 Subject: [PATCH 066/142] Indenting fixed --- svg-scan.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/svg-scan.php b/svg-scan.php index b218cec65..e76c6d95a 100644 --- a/svg-scan.php +++ b/svg-scan.php @@ -124,8 +124,16 @@ function vipgoci_svg_scan_single_file( $results_files[ $temp_file_name ]['messages'][] = array( - 'message' => 'Found PHP tag in SVG file: \'' . $disallowed_token . '\'', - 'source' => 'WordPressVIPMinimum.Security.SVG.DisallowedTags', + 'message' => + 'Found forbidden tag in SVG ' . + 'file: \'' . + $disallowed_token . + '\'', + + 'source' => + 'WordPressVIPMinimum.' . + 'Security.SVG.DisallowedTags', + 'severity' => 5, 'fixable' => false, 'type' => 'ERROR', From 1a587f0081fb6997ca4dc71d934eb3dde2ae86d1 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 18 Sep 2018 14:43:45 +0000 Subject: [PATCH 067/142] Stop printing file-types, as that is irrelevant to this function now --- auto-approval.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/auto-approval.php b/auto-approval.php index 471d74b16..537e5b194 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -24,10 +24,7 @@ function vipgoci_auto_approval( $options, &$auto_approved_files_arr ) { 'commit_id' => $options['commit'], 'autoapprove' => $options['autoapprove'], - 'autoapprove-filetypes' => - $options['autoapprove-filetypes'], - - 'autoapproved-files-arr' => + 'autoapproved_files_arr' => $auto_approved_files_arr, ) ); From 95845892d4a6b40314ee2ecf658af18ef5de46eb Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 18 Sep 2018 14:44:15 +0000 Subject: [PATCH 068/142] Removing FIXME --- misc.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc.php b/misc.php index 25b9fa2ea..b6621e3be 100644 --- a/misc.php +++ b/misc.php @@ -94,7 +94,7 @@ function vipgoci_patch_changed_lines( * Get patch for the relevant file * our caller is interested in */ - // FIXME: Detect if file is not part of the patch + $lines_arr = explode( "\n", $patch_arr[ $file_name ] From 1125637d044ca857edcbcae392c71337a6f23de3 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 18 Sep 2018 14:44:48 +0000 Subject: [PATCH 069/142] Make SVG scanning configurable via command-line argument --- phpcs-scan.php | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/phpcs-scan.php b/phpcs-scan.php index 2b148a4a6..5988b65cf 100644 --- a/phpcs-scan.php +++ b/phpcs-scan.php @@ -398,12 +398,23 @@ function vipgoci_phpcs_scan_commit( $commit_id, array( 'file_extensions' => - array( 'php', 'js', 'twig', 'svg' ), + /* + * If SVG-checks are enabled, + * include it in the file-extensions + */ + array_merge( + array( 'php', 'js', 'twig' ), + ( $options['svg-checks'] ? + array( 'svg' ) : + array() + ) + ), 'skip_folders' => $options['skip-folders'], ) ); + foreach ( $pr_item_files_tmp as $pr_item_file_name ) { if ( in_array( $pr_item_file_name, @@ -458,9 +469,15 @@ function vipgoci_phpcs_scan_commit( * If a SVG file, scan using a * custom internal function, otherwise * use PHPCS. + * + * However, only do this if SVG-checks + * is enabled. */ $scanning_func = - ( 'svg' === $file_extension ) ? + ( + ( 'svg' === $file_extension ) && + ( $options['svg-checks'] ) + ) ? 'vipgoci_svg_scan_single_file' : 'vipgoci_phpcs_scan_single_file'; From 4e610dfba8ac5ac09f37c765aa9281c9bc8a9d25 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 18 Sep 2018 14:45:43 +0000 Subject: [PATCH 070/142] Adding logging --- svg-scan.php | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/svg-scan.php b/svg-scan.php index e76c6d95a..5a678e39b 100644 --- a/svg-scan.php +++ b/svg-scan.php @@ -4,11 +4,28 @@ * Scan a SVG-file for disallowed * tokens. Will return results in the * same format as PHPCS does. + * + * Note that this function is designed as + * a substitute for PHPCS in case of + * scanning SVG files. */ function vipgoci_svg_scan_single_file( $options, $file_name ) { + vipgoci_runtime_measure( 'start', 'svg_scan_single_file' ); + + vipgoci_log( + 'Scanning single SVG file', + array( + 'repo_owner' => $options['repo-owner'], + 'repo_name' => $options['repo-name'], + 'commit_id' => $options['commit'], + 'svg_checks' => $options['svg-checks'], + 'file_name' => $file_name, + ) + ); + /* * These tokens are not allowed * in SVG files. Note that we do @@ -49,6 +66,21 @@ function vipgoci_svg_scan_single_file( * We only process SVG files. */ if ( 'svg' !== $file_extension ) { + + vipgoci_runtime_measure( 'stop', 'svg_scan_single_file' ); + + vipgoci_log( + 'Could not scan file, does not seem to be a SVG file', + array( + 'repo_owner' => $options['repo-owner'], + 'repo_name' => $options['repo-name'], + 'commit_id' => $options['commit'], + 'svg_checks' => $options['svg-checks'], + 'file_name' => $file_name, + ) + ); + + return null; } @@ -173,6 +205,15 @@ function vipgoci_svg_scan_single_file( ) ); + vipgoci_runtime_measure( 'stop', 'svg_scan_single_file' ); + + vipgoci_log( + 'SVG scanning of a single file finished', + array( + 'file_issues_arr_master' => $results, + ) + ); + return array( 'file_issues_arr_master' => $results, 'file_issues_str' => json_encode( $results ), From aa0c8a34aed67c9fdbbda1052f06def92cfab66d Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 18 Sep 2018 14:46:32 +0000 Subject: [PATCH 071/142] Introducing --svg-checks, adding FIXMEs --- vip-go-ci.php | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/vip-go-ci.php b/vip-go-ci.php index 517fca82e..fa2758385 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -369,6 +369,7 @@ function vipgoci_run() { 'skip-folders:', 'lint:', 'phpcs:', + 'svg-checks:', 'autoapprove:', 'autoapprove-filetypes:', 'autoapprove-label:', @@ -415,6 +416,9 @@ function vipgoci_run() { "\t" . '--autoapprove-label=STRING String to use for labels when auto-approving' . PHP_EOL . "\t" . '--php-path=FILE Full path to PHP, if not specified the' . PHP_EOL . "\t" . ' default in $PATH will be used instead' . PHP_EOL . + "\t" . '--svg-checks=BOOL Enable or disable SVG checks, both' . PHP_EOL . + "\t" . ' auto-approval of SVG files and problem' . PHP_EOL . + "\t" . ' checking of these files' . PHP_EOL . "\t" . '--hashes-api=BOOL Whether to do hashes-to-hashes API verfication ' . PHP_EOL . "\t" . ' with individual PHP files found to be altered ' . PHP_EOL . "\t" . ' in the branch specified' . PHP_EOL . @@ -511,11 +515,13 @@ function vipgoci_run() { /* - * Process --hashes-api -- expected to be a boolean. + * Process --hashes-api and --svg-checks + * -- expected to be a boolean. */ vipgoci_option_bool_handle( $options, 'hashes-api', 'false' ); + vipgoci_option_bool_handle( $options, 'svg-checks', 'false' ); /* * Process --hashes-api-url -- expected to @@ -925,6 +931,11 @@ function vipgoci_run() { $results ); + /* FIXME: + * Loop through all reviews for each PR, + * dismiss reviews that contain *only* + * inactive comments. + */ /* * Clean up old comments made by us previously @@ -1001,11 +1012,12 @@ function vipgoci_run() { ); } - // FIXME: Use options here - vipgoci_ap_svg_files( - $options, - $auto_approved_files_arr - ); + if ( true === $options['svg-checks'] ) { + vipgoci_ap_svg_files( + $options, + $auto_approved_files_arr + ); + } vipgoci_auto_approval( $options, @@ -1013,6 +1025,15 @@ function vipgoci_run() { ); } + /* + * FIXME: Remove issues from $results + * for files that are approved. + */ + + /* + * FIXME: Limit number of issues in $results + * -- take into account previously posted issues. + */ /* * Submit any issues to GitHub From eab52637858cac07ee0cf0fe9bca61167b2aaa64 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 18 Sep 2018 14:49:02 +0000 Subject: [PATCH 072/142] Using own function to determine file-extension --- phpcs-scan.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/phpcs-scan.php b/phpcs-scan.php index 5988b65cf..16000ff8b 100644 --- a/phpcs-scan.php +++ b/phpcs-scan.php @@ -68,9 +68,8 @@ function vipgoci_phpcs_scan_single_file( $options['local-git-repo'] ); - $file_extension = pathinfo( - $file_name, - PATHINFO_EXTENSION + $file_extension = vipgoci_file_extension( + $file_name ); if ( empty( $file_extension ) ) { From 85f3990d070d7a4c180ea0af50041718899166d2 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 18 Sep 2018 14:49:52 +0000 Subject: [PATCH 073/142] Removing code that was not used --- auto-approval.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/auto-approval.php b/auto-approval.php index 537e5b194..1f8de8cfd 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -67,11 +67,6 @@ function vipgoci_auto_approval( $options, &$auto_approved_files_arr ) { $files_seen[] = $pr_diff_file_name; - $pr_diff_file_extension = pathinfo( - $pr_diff_file_name, - PATHINFO_EXTENSION - ); - /* * Is file in array of files From f7d9ecc14a1293ed03e96df7310a819d7536ba77 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Thu, 20 Sep 2018 15:51:29 +0000 Subject: [PATCH 074/142] Add functionality to ignore issues with approved files --- misc.php | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++ vip-go-ci.php | 14 ++++-- 2 files changed, 141 insertions(+), 3 deletions(-) diff --git a/misc.php b/misc.php index b6621e3be..8cce56c46 100644 --- a/misc.php +++ b/misc.php @@ -796,3 +796,133 @@ function vipgoci_github_comment_match( return false; } +/* + * For each approved file, remove any issues + * to be submitted against them. However, + * do not do this for 'info' type messages, + * as they are informational, and not problems. + * + * We do this, because sometimes Pull-Requests + * will be opened that contain approved code, + * and we do not want to clutter them with + * non-relevant comments. + * + * Make sure to update statistics to + * reflect this. + */ + +function vipgoci_approved_files_comments_remove( + $options, + &$results, + $auto_approved_files_arr +) { + + $issues_removed = array( + ); + + vipgoci_log( + 'Removing any potential issues (errors, warnings) ' . + 'found for approved files', + + array( + 'auto_approved_files_arr' => $auto_approved_files_arr, + ) + ); + + /* + * Loop through each Pull-Request + */ + foreach( $results['issues'] as + $pr_number => $pr_issues + ) { + /* + * Loop through each issue affecting each + * Pull-Request. + */ + foreach( $pr_issues as + $issue_number => $issue_item + ) { + + /* + * If the file affected is + * not found in the auto-approved files, + * do not to anything. + */ + if ( ! isset( + $auto_approved_files_arr[ + $issue_item['file_name'] + ] + ) ) { + continue; + } + + /* + * We do not touch on 'info' type, + * as that does not report any errors. + */ + if ( strtolower( + $issue_item['type'] + ) === 'info' ) { + continue; + } + + /* + * We have found an item that is approved, + * and has non-info issues -- remove it + * from the array of submittable issues. + */ + unset( + $results[ + 'issues' + ][ + $pr_number + ][ + $issue_number + ] + ); + + /* + * Update statistics accordingly. + */ + $results[ + 'stats' + ][ + $issue_item['type'] + ][ + $pr_number + ][ + strtolower( + $issue_item['issue']['type'] + ) + ]--; + + /* + * Update our own information array on + * what we did. + */ + $issues_removed[ + $pr_number + ][] = $issue_item; + } + + /* + * Re-order the array as + * some keys might be missing + */ + ksort( + $results[ + 'issues' + ][ + $pr_number + ] + ); + } + + + vipgoci_log( + 'Completed cleaning out issues for pre-approved files', + array( + 'issues_removed' => $issues_removed, + ) + ); +} diff --git a/vip-go-ci.php b/vip-go-ci.php index fa2758385..c8e48a658 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -1025,18 +1025,26 @@ function vipgoci_run() { ); } + /* - * FIXME: Remove issues from $results - * for files that are approved. + * Remove issues from $results for files + * that are approved in hashes-to-hashes API. */ + vipgoci_approved_files_comments_remove( + $options, + $results, + $auto_approved_files_arr + ); + + /* * FIXME: Limit number of issues in $results * -- take into account previously posted issues. */ /* - * Submit any issues to GitHub + * Submit any remaining issues to GitHub */ vipgoci_github_pr_generic_comment_submit( From 8818c92681bd8292dd5ebb6a319dee34a76a3996 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Thu, 20 Sep 2018 15:59:35 +0000 Subject: [PATCH 075/142] Replacing ksort() with array_values() --- misc.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/misc.php b/misc.php index 8cce56c46..17cd066cb 100644 --- a/misc.php +++ b/misc.php @@ -909,7 +909,11 @@ function vipgoci_approved_files_comments_remove( * Re-order the array as * some keys might be missing */ - ksort( + $results[ + 'issues' + ][ + $pr_number + ] = array_values( $results[ 'issues' ][ From 5838d90b9268ab2417fbefe1f72ac260f233ebf6 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Thu, 20 Sep 2018 17:36:25 +0000 Subject: [PATCH 076/142] For any auto-approved files, report it to the PR. --- auto-approval.php | 66 ++++++++++++++++++++++++++++++++++++++++++++--- github-api.php | 10 ------- misc.php | 3 ++- vip-go-ci.php | 3 ++- 4 files changed, 66 insertions(+), 16 deletions(-) diff --git a/auto-approval.php b/auto-approval.php index 1f8de8cfd..cc3aca1f9 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -13,7 +13,11 @@ * in this function. */ -function vipgoci_auto_approval( $options, &$auto_approved_files_arr ) { +function vipgoci_auto_approval( + $options, + &$auto_approved_files_arr, + &$results +) { vipgoci_runtime_measure( 'start', 'auto_approve_commit' ); vipgoci_log( @@ -160,10 +164,64 @@ function vipgoci_auto_approval( $options, &$auto_approved_files_arr ) { ); } - // FIXME: Add comment for each hashes-to-hashes approved PHP and JS file, - // indicating that it is approved in the DB so human reviewers do not - // need to look at it again. + /* + * Loop through approved PHP and JS files, + * adding comment for each about it + * being approved in the hashes-to-hashes API. + */ + foreach( + $auto_approved_files_arr as + $approved_file => + $approved_file_system + ) { + if ( + $approved_file_system !== + 'autoapprove-hashes-to-hashes' + ) { + /* + * If not autoapproved by hashes-to-hashes, + * do not comment on it. Only PHP and JS files + * are auto-approved by hashes-to-hashes. + */ + continue; + } + + $results[ + 'issues' + ][ + (int) $pr_item->number + ] + [] = array( + 'type' => VIPGOCI_STATS_HASHES_API, + 'file_name' => $approved_file, + 'file_line' => 1, + 'issue' => array( + 'message'=> 'File is ' . + 'approved in hashes-to-hashes database', + + 'source' + => 'WordPressVIPMinimum.' . + 'Info.ApprovedHashesToHashesAPI', + + 'severity' => 1, + 'fixable' => false, + 'type' => 'INFO', + 'line' => 1, + 'column' => 1, + 'level' =>'INFO' + ) + ); + $results[ + 'stats' + ][ + VIPGOCI_STATS_HASHES_API + ][ + (int) $pr_item->number + ][ + 'info' + ]++; + } // FIXME: Dismiss any approving reviews from the PR. } diff --git a/github-api.php b/github-api.php index 54d615e1d..0937261ec 100644 --- a/github-api.php +++ b/github-api.php @@ -1622,16 +1622,6 @@ function vipgoci_github_pr_review_submit( $stats_types_to_process as $stats_type ) { - /* - * No report for 'hashes-api', - * as it can report no issues back - * and statistics for it will always - * be at zero. - */ - if ( 'hashes-api' === $stats_type ) { - continue; - } - /* * Add page-breaking, if needed. */ diff --git a/misc.php b/misc.php index 17cd066cb..8579c97ef 100644 --- a/misc.php +++ b/misc.php @@ -860,8 +860,9 @@ function vipgoci_approved_files_comments_remove( * We do not touch on 'info' type, * as that does not report any errors. */ + if ( strtolower( - $issue_item['type'] + $issue_item['issue']['type'] ) === 'info' ) { continue; } diff --git a/vip-go-ci.php b/vip-go-ci.php index c8e48a658..3b3e529b3 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -1021,7 +1021,8 @@ function vipgoci_run() { vipgoci_auto_approval( $options, - $auto_approved_files_arr + $auto_approved_files_arr, + $results ); } From e3f52d3c3862ac5be847be76d7b4d69399d3db5d Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 21 Sep 2018 14:45:03 +0000 Subject: [PATCH 077/142] Remove already submitted comments here --- vip-go-ci.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/vip-go-ci.php b/vip-go-ci.php index 3b3e529b3..4c6db3353 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -1038,6 +1038,16 @@ function vipgoci_run() { $auto_approved_files_arr ); + /* + * Remove comments from $results that have + * already been submitted. + */ + + vipgoci_remove_existing_github_comments_from_results( + $options, + $prs_implicated, + $results + ); /* * FIXME: Limit number of issues in $results From 635f71569388a59315e82dbde9e361c4199db33b Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 21 Sep 2018 14:45:37 +0000 Subject: [PATCH 078/142] Alter how vipgoci_github_pr_reviews_comments_get() is called --- github-api.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/github-api.php b/github-api.php index 0937261ec..e0de6344e 100644 --- a/github-api.php +++ b/github-api.php @@ -866,13 +866,14 @@ function vipgoci_github_fetch_commit_info( * return false on an error. */ function vipgoci_github_pr_reviews_comments_get( - &$prs_comments, - $repo_owner, - $repo_name, + $options, $commit_id, $commit_made_at, - $github_token + &$prs_comments ) { + $repo_owner = $options['repo-owner']; + $repo_name = $options['repo-name']; + $github_token = $options['token']; /* * Try to get comments from cache From 9897e017489d1f0f3547e072b06a357ec37bb6c4 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 21 Sep 2018 14:45:50 +0000 Subject: [PATCH 079/142] Make sure to remove any info-related stuff as well --- misc.php | 139 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 137 insertions(+), 2 deletions(-) diff --git a/misc.php b/misc.php index 8579c97ef..e360a20df 100644 --- a/misc.php +++ b/misc.php @@ -746,7 +746,7 @@ function vipgoci_github_comment_match( * as "Warning: ..." -- remove all of that. */ $comment_made_body = str_replace( - array("**", "Warning", "Error", ":no_entry_sign:", ":exclamation:"), + array("**", "Warning", "Error", "Info", ":no_entry_sign:", ":exclamation:", ":information_source:"), array("", "", "", "", ""), $comment_made->body ); @@ -796,6 +796,141 @@ function vipgoci_github_comment_match( return false; } +/* + * Remove comments that exist on a GitHub Pull-Request from + * the results array. Will loop through each Pull-Request + * affected by the current commit, and remove any comment + * from the results array if it already exists. + */ +function vipgoci_remove_existing_github_comments_from_results( + $options, + $prs_implicated, + &$results +) { + $comments_removed = array(); + + foreach ( $prs_implicated as $pr_item ) { + $prs_comments = array(); + + if ( ! isset( + $comments_removed[ $pr_item->number ] + ) ) { + $comments_removed[ $pr_item->number ] = array(); + } + + /* + * Get all commits related to the current + * Pull-Request. + */ + + $pr_item_commits = vipgoci_github_prs_commits_list( + $options['repo-owner'], + $options['repo-name'], + $pr_item->number, + $options['token'] + ); + + /* + * Loop through each commit, fetching all comments + * made in relation to that commit + */ + + foreach ( $pr_item_commits as $pr_item_commit_id ) { + vipgoci_github_pr_reviews_comments_get( + $options, + $pr_item_commit_id, + $pr_item->created_at, + $prs_comments + ); + + unset( $pr_item_commit_id ); + } + + foreach( + $results['issues'][ $pr_item->number ] as + $submitted_comment_key => + $submitted_comment + ) { + + /* + * Filter out issues that have already been + * reported got GitHub. + */ + + if ( + // Only do check if everything above is looking good + vipgoci_github_comment_match( + $submitted_comment['file_name'], + $submitted_comment['file_line'], + $submitted_comment['issue']['message'], + $prs_comments + ) + ) { + /* + * Keep a record of what we remove. + */ + $comments_removed[ $pr_item->number ][] = + $submitted_comment; + + /* Remove it */ + unset( + $results[ + 'issues' + ][ + $pr_item->number + ][ + $submitted_comment_key + ] + ); + + /* + * Update statistics + */ + $results[ + 'stats' + ][ + $submitted_comment['type'] + ][ + $pr_item->number + ][ + strtolower( + $submitted_comment['issue']['type'] + ) + ]--; + } + } + + /* + * Re-create the issues + * array, so that no array + * keys are missing. + */ + $results[ + 'issues' + ][ + $pr_item->number + ] = array_values( + $results[ + 'issues' + ][ + $pr_item->number + ] + ); + } + + /* + * Report what we removed. + */ + vipgoci_log( + 'Removed following comments from array of ' . + 'to be submitted comments to PRs, as they ' . + 'have been submitted already', + array( + 'comments_removed' => $comments_removed + ) + ); +} + /* * For each approved file, remove any issues * to be submitted against them. However, @@ -804,7 +939,7 @@ function vipgoci_github_comment_match( * * We do this, because sometimes Pull-Requests * will be opened that contain approved code, - * and we do not want to clutter them with + * and we do not want to clutter them with * non-relevant comments. * * Make sure to update statistics to From 8065defede51bdf51511e4d1eae4d2406dd957d5 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 21 Sep 2018 14:50:24 +0000 Subject: [PATCH 080/142] Removing whitespace. --- github-api.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/github-api.php b/github-api.php index e0de6344e..c2df0b72a 100644 --- a/github-api.php +++ b/github-api.php @@ -272,7 +272,7 @@ function vipgoci_oauth1_signature_get_hmac_sha1( * Create and set HTTP header for OAuth 1.0a requests, * including timestamp, nonce, signature method * (all part of the header) and then actually sign - * the request. Returns with a full HTTP header for + * the request. Returns with a full HTTP header for * a OAuth 1.0a HTTP request. */ function vipgoci_oauth1_headers_get( @@ -288,7 +288,7 @@ function vipgoci_oauth1_headers_get( 'HMAC-SHA1'; /* - * Set timestamp and nonce. + * Set timestamp and nonce. */ $github_token['oauth_timestamp'] = (string) ( time() - 1); @@ -1681,7 +1681,7 @@ function vipgoci_github_pr_review_submit( ) . ' ' . $commit_issue_stat_value . ' ' . - $commit_issue_stat_key . + $commit_issue_stat_key . ( ( $commit_issue_stat_value > 1 ) ? 's' : '' ) . ' ' . "\n\r"; From 5d892acd8512677493f118f3e886d09c804e04ac Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 21 Sep 2018 14:51:22 +0000 Subject: [PATCH 081/142] Removing whitespaces --- auto-approval.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/auto-approval.php b/auto-approval.php index cc3aca1f9..ebfed164b 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -181,9 +181,11 @@ function vipgoci_auto_approval( /* * If not autoapproved by hashes-to-hashes, * do not comment on it. Only PHP and JS files - * are auto-approved by hashes-to-hashes. + * are auto-approved by hashes-to-hashes. */ continue; + + // FIXME: Check if comment has been made already. } $results[ From 2bd21cd0038cf43b7e56186b0c1b26dd4424034a Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 21 Sep 2018 14:51:42 +0000 Subject: [PATCH 082/142] Removing whitespaces --- svg-scan.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/svg-scan.php b/svg-scan.php index 5a678e39b..dd7b43790 100644 --- a/svg-scan.php +++ b/svg-scan.php @@ -156,13 +156,13 @@ function vipgoci_svg_scan_single_file( $results_files[ $temp_file_name ]['messages'][] = array( - 'message' => + 'message' => 'Found forbidden tag in SVG ' . 'file: \'' . $disallowed_token . '\'', - 'source' => + 'source' => 'WordPressVIPMinimum.' . 'Security.SVG.DisallowedTags', From 24ba0a02b6c7b29362bf6dcdcb7aaeee5ef888fe Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 21 Sep 2018 14:52:32 +0000 Subject: [PATCH 083/142] Removing whitespaces and extra line --- vip-go-ci.php | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/vip-go-ci.php b/vip-go-ci.php index 4c6db3353..26eb38e99 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -198,7 +198,7 @@ function vipgoci_option_file_handle( /* * Handle parameter that we expect to be a URL. * - * If the parameter is not empty, and is not really + * If the parameter is not empty, and is not really * a URL (not starting with http:// or https://), * exit with error. If empty, sets a default. */ @@ -221,7 +221,6 @@ function vipgoci_option_url_handle( /* * If not default value, check if it looks like an URL, * and if so, use it, but if not, exit with error. - * */ if ( $default_value !== $options[ $option_name ] ) { $options[ $option_name ] = trim( @@ -240,7 +239,7 @@ function vipgoci_option_url_handle( ) ) ) { vipgoci_sysexit( - 'Option --' . $option_name . ' should ' . + 'Option --' . $option_name . ' should ' . 'be an URL', array( ), @@ -305,7 +304,7 @@ function vipgoci_run() { global $argv; global $vipgoci_debug_level; - $hashes_oauth_arguments = + $hashes_oauth_arguments = array( 'hashes-oauth-token', 'hashes-oauth-token-secret', @@ -392,7 +391,7 @@ function vipgoci_run() { "\t" . 'Options --repo-owner, --repo-name, --commit, --token, --local-git-repo, --phpcs-path are ' . PHP_EOL . "\t" . 'mandatory, while others are optional.' . PHP_EOL . PHP_EOL . - "\t" . 'Note that if option --autoapprove is specified, --autoapprove-label needs to' . PHP_EOL . + "\t" . 'Note that if option --autoapprove is specified, --autoapprove-label needs to' . PHP_EOL . "\t" . 'be specified as well.' . PHP_EOL . PHP_EOL . "\t" . '--repo-owner=STRING Specify repository owner, can be an organization' . PHP_EOL . @@ -418,7 +417,7 @@ function vipgoci_run() { "\t" . ' default in $PATH will be used instead' . PHP_EOL . "\t" . '--svg-checks=BOOL Enable or disable SVG checks, both' . PHP_EOL . "\t" . ' auto-approval of SVG files and problem' . PHP_EOL . - "\t" . ' checking of these files' . PHP_EOL . + "\t" . ' checking of these files' . PHP_EOL . "\t" . '--hashes-api=BOOL Whether to do hashes-to-hashes API verfication ' . PHP_EOL . "\t" . ' with individual PHP files found to be altered ' . PHP_EOL . "\t" . ' in the branch specified' . PHP_EOL . @@ -688,7 +687,7 @@ function vipgoci_run() { } /* - * Do sanity-checking with hashes-api-url + * Do sanity-checking with hashes-api-url * and --hashes-oauth-* parameters */ if ( isset( $options['hashes-api-url'] ) ) { @@ -1047,7 +1046,7 @@ function vipgoci_run() { $options, $prs_implicated, $results - ); + ); /* * FIXME: Limit number of issues in $results From bb26314d22a2ef709b4d8a98ac1ed584a310b340 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 21 Sep 2018 14:54:06 +0000 Subject: [PATCH 084/142] Removing duplicate-comment functionality from vipgoci_issues_filter_irrellevant() --- phpcs-scan.php | 66 +------------------------------------------------- 1 file changed, 1 insertion(+), 65 deletions(-) diff --git a/phpcs-scan.php b/phpcs-scan.php index 16000ff8b..045e31adf 100644 --- a/phpcs-scan.php +++ b/phpcs-scan.php @@ -152,14 +152,10 @@ function vipgoci_phpcs_scan_output_dump( $output_file, $data ) { * that existed prior to the change. */ function vipgoci_issues_filter_irrellevant( - $repo_owner, - $repo_name, - $commit_id, $file_name, $file_issues_arr, $file_blame_log, $pr_item_commits, - $comments_existing, $file_relative_lines ) { /* @@ -218,40 +214,6 @@ function vipgoci_issues_filter_irrellevant( continue; } - /* - * Filter out issues that have already been - * reported got GitHub. - */ - - if ( - // Only do check if everything above is looking good - vipgoci_github_comment_match( - $file_name, - $file_relative_lines[ - $file_issue_val['line'] - ], - $file_issue_val['message'], - $comments_existing - ) - ) { - vipgoci_log( - 'Skipping submission of ' . - 'comment, has already been ' . - 'submitted', - array( - 'repo_owner' => $repo_owner, - 'repo_name' => $repo_name, - 'filename' => $file_name, - 'file_issue_line' => $file_issue_val['line'], - 'file_issue_msg' => $file_issue_val['message'], - 'commit_id' => $commit_id, - ) - ); - - /* Skip */ - continue; - } - // Passed all tests, keep this issue $file_issues_ret[] = $file_issue_val; } @@ -602,13 +564,6 @@ function( $item ) { foreach ( $prs_implicated as $pr_item ) { - /* - * Loop through each commit, fetching all comments - * made in relation to that commit - */ - - $prs_comments = array(); - /* * Get all commits related to the current * Pull-Request. @@ -620,19 +575,6 @@ function( $item ) { $github_token ); - foreach ( $pr_item_commits as $pr_item_commit_id ) { - vipgoci_github_pr_reviews_comments_get( - $prs_comments, - $repo_owner, - $repo_name, - $pr_item_commit_id, - $pr_item->created_at, - $github_token - ); - - unset( $pr_item_commit_id ); - } - /* * Loop through each file, get a @@ -676,19 +618,14 @@ function( $item ) { * the ones that the are not found * in the blame-log (meaning that * they are due to commits outside of - * the Pull-Request), and remove - * those which have already been submitted. + * the Pull-Request). */ $file_issues_arr_filtered = vipgoci_issues_filter_irrellevant( - $repo_owner, - $repo_name, - $commit_id, $file_name, $files_issues_arr, $file_blame_log, $pr_item_commits, - $prs_comments, $file_relative_lines ); @@ -737,7 +674,6 @@ function( $item ) { } } - unset( $prs_comments ); unset( $pr_item_commits ); unset( $pr_item_files_changed ); unset( $file_blame_log ); From c06c51a01f881bb5a948ae76b4814f06fee68629 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 24 Sep 2018 15:20:19 +0000 Subject: [PATCH 085/142] Dismiss reviews that previously approved a PR when they become obsolete --- auto-approval.php | 33 ++++- github-api.php | 345 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 374 insertions(+), 4 deletions(-) diff --git a/auto-approval.php b/auto-approval.php index ebfed164b..9b2a2beb4 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -114,6 +114,7 @@ function vipgoci_auto_approval( ); } +// FIXME: Break into functions else if ( ( true === $did_foreach ) && ( false === $can_auto_approve ) @@ -184,8 +185,6 @@ function vipgoci_auto_approval( * are auto-approved by hashes-to-hashes. */ continue; - - // FIXME: Check if comment has been made already. } $results[ @@ -224,7 +223,35 @@ function vipgoci_auto_approval( 'info' ]++; } - // FIXME: Dismiss any approving reviews from the PR. + + /* + * Get any approving reviews for the Pull-Request + * submitted by us. + */ + $pr_item_reviews = vipgoci_github_pr_reviews_get( + $options['repo-owner'], + $options['repo-name'], + (int) $pr_item->number, + $options['token'], + array( + 'login' => 'myself', + 'state' => 'APPROVED' + ) + ); + + /* + * Dismiss any approving reviews. + */ + foreach( $pr_item_reviews as $pr_item_review ) { + vipgoci_github_pr_review_dismiss( + $options['repo-owner'], + $options['repo-name'], + (int) $pr_item->number, + (int) $pr_item_review->id, + 'Dismissing obsolete review; not approved any longer', + $options['token'] + ); + } } else if ( diff --git a/github-api.php b/github-api.php index c2df0b72a..4289cdb1c 100644 --- a/github-api.php +++ b/github-api.php @@ -559,7 +559,6 @@ function vipgoci_github_post_url( return $ret_val; } - /* * Make a GET request to GitHub, for the URL * provided, using the access-token specified. @@ -707,6 +706,201 @@ function vipgoci_github_fetch_url( return $resp_data; } +/* + * Submit PUT request to the GitHub API. + */ +function vipgoci_github_put_url( + $github_url, + $github_postfields, + $github_token +) { + /* + * Actually send a request to GitHub -- make sure + * to retry if something fails. + */ + do { + /* + * By default, assume request went through okay. + */ + + $ret_val = 0; + + /* + * By default, do not retry the request, + * just assume everything goes well + */ + + $retry_req = false; + + /* + * Initialize and send request. + */ + + $ch = curl_init(); + + curl_setopt( + $ch, CURLOPT_URL, $github_url + ); + + curl_setopt( + $ch, CURLOPT_RETURNTRANSFER, 1 + ); + + curl_setopt( + $ch, CURLOPT_CONNECTTIMEOUT, 20 + ); + + curl_setopt( + $ch, CURLOPT_USERAGENT, VIPGOCI_CLIENT_ID + ); + + curl_setopt( + $ch, CURLOPT_CUSTOMREQUEST, 'PUT' + ); + + curl_setopt( + $ch, + CURLOPT_POSTFIELDS, + json_encode( $github_postfields ) + ); + + curl_setopt( + $ch, + CURLOPT_HEADERFUNCTION, + 'vipgoci_curl_headers' + ); + + curl_setopt( + $ch, + CURLOPT_HTTPHEADER, + array( 'Authorization: token ' . $github_token ) + ); + + // Make sure to pause between GitHub-requests + vipgoci_github_wait(); + + /* + * Execute query to GitHub, keep + * record of how long time it took, + * and keep count of how many requests we do. + */ + + vipgoci_runtime_measure( 'start', 'github_api' ); + + vipgoci_counter_report( 'do', 'github_api_request_put', 1 ); + + $resp_data = curl_exec( $ch ); + + vipgoci_runtime_measure( 'stop', 'github_api' ); + + + $resp_headers = vipgoci_curl_headers( + null, + null + ); + + + /* + * Assume 200 for success, everything else for failure. + */ + if ( intval( $resp_headers['status'][0] ) !== 200 ) { + /* + * Set default wait period between requests + */ + $retry_sleep = 10; + + /* + * Set error-return value + */ + $ret_val = -1; + + /* + * Figure out if to retry... + */ + + // Decode JSON + $resp_data = json_decode( $resp_data ); + + if ( + ( isset( + $resp_headers['retry-after'] + ) ) && + ( intval( + $resp_headers['retry-after'] + ) > 0 ) + ) { + $retry_req = true; + $retry_sleep = intval( + $resp_headers['retry-after'] + ); + } + + else if ( + ( $resp_data->message == + 'Validation Failed' ) && + + ( $resp_data->errors[0] == + 'was submitted too quickly ' . + 'after a previous comment' ) + ) { + /* + * These messages are due to the + * submission being categorized + * as a spam by GitHub -- no good + * reason to retry, really. + */ + $retry_req = false; + $retry_sleep = 20; + } + + else if ( + ( $resp_data->message == + 'Validation Failed' ) + ) { + $retry_req = false; + } + + else if ( + ( $resp_data->message == + 'Server Error' ) + ) { + $retry_req = false; + } + + vipgoci_log( + 'GitHub reported an error' . + ( $retry_req === true ? + ' will retry request in ' . + $retry_sleep . ' seconds' : + '' ), + array( + 'http_url' + => $github_url, + + 'http_response_headers' + => $resp_headers, + + 'http_reponse_body' + => $resp_data, + ) + ); + + sleep( $retry_sleep + 1 ); + } + + vipgoci_github_rate_limits_check( + $github_url, + $resp_headers + ); + + + curl_close( $ch ); + + } while ( $retry_req == true ); + + return $ret_val; +} + /* * Fetch information from GitHub on a particular * commit within a particular repository, using @@ -1436,6 +1630,111 @@ function vipgoci_github_pr_generic_comment_delete( ); } +/* + * Get all reviews for a particular Pull-Request, + * and allow filtering by: + * - User submitted (parameter: login) + * - State of review (parameter: state, values are: CHANGES_REQUESTED, COMMENTED, APPROVED) + * + * Note that parameter login can be assigned a magic + * value, 'myself', in which case the actual username + * will be assumed to be that of the token-holder. + */ +function vipgoci_github_pr_reviews_get( + $repo_owner, + $repo_name, + $pr_number, + $github_token, + $filter = array() +) { + vipgoci_log( + 'Fetching reviews for Pull-Request', + array( + 'repo_owner' => $repo_owner, + 'repo_name' => $repo_name, + 'pr_number' => $pr_number, + ) + ); + + $ret_reviews = array(); + + $page = 1; + $per_page = 100; + + + /* + * Figure out login name. + */ + if ( + ( ! empty( $filter['login'] ) ) && + ( $filter['login'] === 'myself' ) + ) { + $current_user_info = vipgoci_github_authenticated_user_get( + $github_token + ); + + $filter['login'] = $current_user_info->login; + } + + /* + * Fetch reviews, paged, from GitHub. + */ + + do { + $github_url = + VIPGOCI_GITHUB_BASE_URL . '/' . + 'repos/' . + rawurlencode( $repo_owner ) . '/' . + rawurlencode( $repo_name ) . '/' . + 'pulls/' . + rawurlencode( $pr_number ) . '/' . + 'reviews' . + '?per_page=' . rawurlencode( $per_page ) . '&' . + 'page=' . rawurlencode( $page ); + + + /* + * Fetch reviews, decode result. + */ + $pr_reviews = json_decode( + vipgoci_github_fetch_url( + $github_url, + $github_token + ) + ); + + + /* + * Loop through each review-item, + * do filtering and save the ones + * we want to keep. + */ + + foreach( $pr_reviews as $pr_review ) { + if ( ! empty( $filter['login'] ) ) { + if ( + $pr_review->user->login !== + $filter['login'] + ) { + continue; + } + } + + if ( ! empty( $filter['state'] ) ) { + if ( + $pr_review->state !== + $filter['state'] + ) { + continue; + } + } + + $ret_reviews[] = $pr_review; + } + } while( count( $pr_reviews ) >= $per_page ); + + return $ret_reviews; +} /* * Submit a review on GitHub for a particular commit, @@ -1802,6 +2101,50 @@ function vipgoci_github_pr_review_submit( return; } +/* + * Dismiss a particular review + * previously submitted to a Pull-Request. + */ + +function vipgoci_github_pr_review_dismiss( + $repo_owner, + $repo_name, + $pr_number, + $review_id, + $dismiss_message, + $github_token +) { + + vipgoci_log( + 'Dismissing a Pull-Request Review', + array( + 'repo_owner' => $repo_owner, + 'repo_name' => $repo_name, + 'pr_number' => $pr_number, + 'review_id' => $review_id, + 'dismiss_message' => $dismiss_message, + ) + ); + + $github_url = + VIPGOCI_GITHUB_BASE_URL . '/' . + 'repos/' . + rawurlencode( $repo_owner ) . '/' . + rawurlencode( $repo_name ) . '/' . + 'pulls/' . + rawurlencode( $pr_number ) . '/' . + 'reviews/' . + rawurlencode( $review_id ) . '/' . + 'dismissals'; + + vipgoci_github_put_url( + $github_url, + array( + 'message' => $dismiss_message + ), + $github_token + ); +} /* * Approve a Pull-Request, and afterwards From 68c3a9c2335ab4d7feee9e1485b6cec8fd122172 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 25 Sep 2018 15:38:37 +0000 Subject: [PATCH 086/142] Moving state into array --- auto-approval.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auto-approval.php b/auto-approval.php index 9b2a2beb4..3029a0e7a 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -235,7 +235,7 @@ function vipgoci_auto_approval( $options['token'], array( 'login' => 'myself', - 'state' => 'APPROVED' + 'state' => array( 'APPROVED' ), ) ); From 734ab6f3ae2543440c786126cf039ac23bf0f02c Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 25 Sep 2018 15:41:09 +0000 Subject: [PATCH 087/142] Add code to dismiss stale reviews --- vip-go-ci.php | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/vip-go-ci.php b/vip-go-ci.php index 26eb38e99..e44a12904 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -351,6 +351,7 @@ function vipgoci_run() { 'commit:', 'token:', 'review-comments-max:', + 'dismiss-stale-reviews:', 'branches-ignore:', 'output:', 'dry-run:', @@ -402,6 +403,10 @@ function vipgoci_run() { "\t" . ' to GitHub in one review. If the number of ' . PHP_EOL . "\t" . ' comments exceed this number, additional reviews ' . PHP_EOL . "\t" . ' will be submitted.' . PHP_EOL . + "\t" . '--dismiss-stale-reviews=BOOL Dismiss any reviews associated with Pull-Requests ' . PHP_EOL . + "\t" . ' that we process which have no active comments. ' . PHP_EOL . + "\t" . ' The Pull-Requests we process are those associated ' . PHP_EOL . + "\t" . ' with the commit specified.' . PHP_EOL . "\t" . '--informational-url=STRING URL to documentation on what this bot does. Should ' . PHP_EOL . "\t" . ' start with https:// or https:// ' . PHP_EOL . "\t" . '--phpcs=BOOL Whether to run PHPCS (true/false)' . PHP_EOL . @@ -626,6 +631,7 @@ function vipgoci_run() { vipgoci_option_bool_handle( $options, 'lint', 'true' ); + vipgoci_option_bool_handle( $options, 'dismiss-stale-reviews', 'false' ); if ( ( false === $options['lint'] ) && @@ -655,6 +661,9 @@ function vipgoci_run() { /* * Do some sanity-checking on the parameters + * + * Note: Parameters should not be set after + * this point. */ $options['autoapprove-filetypes'] = array_map( @@ -930,11 +939,20 @@ function vipgoci_run() { $results ); - /* FIXME: - * Loop through all reviews for each PR, - * dismiss reviews that contain *only* - * inactive comments. - */ + if ( true === $options['dismiss-stale-reviews'] ) { + /* + * Dismiss any reviews that contain *only* + * inactive comments -- i.e. comments that + * are obsolete as the code has been changed. + */ + + foreach ( $prs_implicated as $pr_item ) { + vipgoci_github_pr_reviews_dismiss_non_active_comments( + $options, + $pr_item->number + ); + } + } /* * Clean up old comments made by us previously From b76ea5438a946f8bc9eaf137294c3ebee6905db4 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 25 Sep 2018 15:58:14 +0000 Subject: [PATCH 088/142] Functionality to dismiss reviews with non-active comments --- github-api.php | 212 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 209 insertions(+), 3 deletions(-) diff --git a/github-api.php b/github-api.php index 4289cdb1c..75dab6459 100644 --- a/github-api.php +++ b/github-api.php @@ -1178,6 +1178,58 @@ function vipgoci_github_pr_reviews_comments_get( } +/* + * Get all review-comments submitted to a + * particular Pull-Request + */ +function vipgoci_github_pr_reviews_comments_get_by_pr( + $options, + $pr_number +) { + vipgoci_log( + 'Fetching all review comments submitted to a Pull-Request', + array( + 'repo_owner' => $options['repo-owner'], + 'repo_name' => $options['repo-name'], + 'commit_id' => $options['commit'], + 'pr_number' => $pr_number, + ) + ); + + $page = 1; + $per_page = 100; + + $all_comments = array(); + + do { + $github_url = + VIPGOCI_GITHUB_BASE_URL . '/' . + 'repos/' . + rawurlencode( $options['repo-owner'] ) . '/' . + rawurlencode( $options['repo-name'] ) . '/' . + 'pulls/' . + rawurlencode( $pr_number ) . '/' . + 'comments?' . + 'page=' . rawurlencode( $page ) . '&' . + 'per_page=' . rawurlencode( $per_page ); + + $comments = json_decode( + vipgoci_github_fetch_url( + $github_url, + $options['token'] + ) + ); + + foreach( $comments as $comment ) { + $all_comments[] = $comment; + } + + $page++; + } while( count( $comments ) >= $per_page ); + + return $all_comments; +} + /* * Get all generic comments made to a Pull-Request from Github. */ @@ -1721,10 +1773,21 @@ function vipgoci_github_pr_reviews_get( } if ( ! empty( $filter['state'] ) ) { - if ( - $pr_review->state !== - $filter['state'] + $match = false; + + foreach( + $filter['state'] as + $allowed_state ) { + if ( + $pr_review->state === + $allowed_state + ) { + $match = true; + } + } + + if ( false === $match ) { continue; } } @@ -2146,6 +2209,149 @@ function vipgoci_github_pr_review_dismiss( ); } + +/* + * Dismiss all Pull-Request Reviews that have no + * active comments attached to them. + */ +function vipgoci_github_pr_reviews_dismiss_non_active_comments( + $options, + $pr_number +) { + vipgoci_log( + 'Dismissing any Pull-Request reviews submitted by ' . + 'us and contain no active inline comments any more', + array( + 'repo_owner' => $options['repo-owner'], + 'repo_name' => $options['repo-name'], + 'pr_number' => $pr_number, + ) + ); + + /* + * Get any Pull-Request reviews with changes + * required status, and submitted by us. + */ + $pr_reviews = vipgoci_github_pr_reviews_get( + $options['repo-owner'], + $options['repo-name'], + $pr_number, + $options['token'], + array( + 'login' => 'myself', + 'state' => array( 'CHANGES_REQUESTED' ) + ) + ); + + /* + * Get all comments to a the current Pull-Request + */ + $all_comments = vipgoci_github_pr_reviews_comments_get_by_pr( + $options, + $pr_number + ); + + if ( count( $all_comments ) === 0 ) { + /* + * In case we receive no comments at all + * from GitHub, do not do anything, as a precaution. + * Receiving no comments might indicate a + * failure (communication error or something else), + * and because we dismiss reviews that seem not to + * contain any comments, we might risk dismissing + * all reviews when there is a failure. By + * doing this, we take much less risk. + */ + vipgoci_log( + 'Not dismissing any reviews, as no inactive ' . + 'comments submitted to the Pull-Request ' . + 'were found', + array( + 'repo_owner' => $options['repo-owner'], + 'repo_name' => $options['repo-name'], + 'pr_number' => $pr_number, + ) + ); + + return; + } + + $reviews_status = array(); + + foreach( $all_comments as $comment_item ) { + /* + * Not associated with a review? Ignore then. + */ + if ( ! isset( $comment_item->pull_request_review_id ) ) { + continue; + } + + /* + * If the review ID is not found in + * the array of reviews, put in 'null'. + */ + if ( ! isset( $reviews_status[ + $comment_item->pull_request_review_id + ] ) ) { + $reviews_status[ + $comment_item->pull_request_review_id + ] = null; + } + + /* + * In case position (relative line number) + * is at null, this means that the comment + * is no longer 'active': It has become obsolete + * as the code has changed. If we have not so far + * found any instance of the review associated + * with the comment having other active comments, + * mark it as 'safe to dismiss'. + */ + if ( null === $comment_item->position ) { + if ( + $reviews_status[ + $comment_item->pull_request_review_id + ] !== false + ) { + $reviews_status[ + $comment_item->pull_request_review_id + ] = true; + } + } + + else { + $reviews_status[ + $comment_item->pull_request_review_id + ] = false; + } + } + + /* + * Loop through each review we + * found matching the specific criteria. + */ + foreach( $pr_reviews as $pr_review ) { + /* + * If no active comments were found, + * it should be safe to dismiss the review. + */ + if ( + ( ! isset( $reviews_status[ $pr_review->id ] ) ) || + ( true === $reviews_status[ $pr_review->id ] ) + ) { + vipgoci_github_pr_review_dismiss( + $options['repo-owner'], + $options['repo-name'], + $pr_number, + $pr_review->id, + 'Dismissing review as all inline comments ' . + 'are obsolete by now', + $options['token'] + ); + } + } +} + /* * Approve a Pull-Request, and afterwards * make sure to verify that the latest commit From 84cb3f4ad8141a2f54a7cbcc9dd0b4adfcf377fc Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 25 Sep 2018 18:49:19 +0000 Subject: [PATCH 089/142] Adding define for approved-message --- defines.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/defines.php b/defines.php index fbb0c3b1a..d1cbb6885 100644 --- a/defines.php +++ b/defines.php @@ -15,6 +15,8 @@ 'read more [here](%s).' ); +define( 'VIPGOCI_FILE_IS_APPROVED_MSG', 'File is approved in ' . + 'hashes-to-hashes database' ); /* * Define exit-codes */ From 705dee91d5601a314776751f55c4132da020c6d6 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 25 Sep 2018 18:49:50 +0000 Subject: [PATCH 090/142] Adding logic to remove obsolete inline comments from reviews --- auto-approval.php | 56 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 4 deletions(-) diff --git a/auto-approval.php b/auto-approval.php index 3029a0e7a..00e338607 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -197,8 +197,7 @@ function vipgoci_auto_approval( 'file_name' => $approved_file, 'file_line' => 1, 'issue' => array( - 'message'=> 'File is ' . - 'approved in hashes-to-hashes database', + 'message'=> VIPGOCI_FILE_IS_APPROVED_MSG, 'source' => 'WordPressVIPMinimum.' . @@ -226,8 +225,18 @@ function vipgoci_auto_approval( /* * Get any approving reviews for the Pull-Request - * submitted by us. + * submitted by us. Then dismiss them. */ + + vipgoci_log( + 'Dismissing any approving reviews for ' . + 'the Pull-Request, as it is not ' . + 'approved anymore', + array( + 'pr_number' => $pr_item->number, + ) + ); + $pr_item_reviews = vipgoci_github_pr_reviews_get( $options['repo-owner'], $options['repo-name'], @@ -242,6 +251,7 @@ function vipgoci_auto_approval( /* * Dismiss any approving reviews. */ + foreach( $pr_item_reviews as $pr_item_review ) { vipgoci_github_pr_review_dismiss( $options['repo-owner'], @@ -341,7 +351,45 @@ function vipgoci_auto_approval( ); } - // FIXME: Remove any comments indicating that a file is approved. + /* + * Remove any comments indicating that a file is + * approved -- we want to get rid of these, as they + * are useless to reviewers at this point. The PR is + * approved anyway. + */ + vipgoci_log( + 'Removing any comments indicating that a ' . + 'particular file is approved, as ' . + 'the whole Pull-Request is approved', + array( + 'pr_number' => $pr_item->number, + ) + ); + + $pr_comments = vipgoci_github_pr_reviews_comments_get_by_pr( + $options, + $pr_item->number, + array( + 'login' => 'myself', + ) + ); + + foreach( $pr_comments as $pr_comment_item ) { + /* + * If we find the 'approved in hashes-to-hashes ...' + * message, we can safely remove the comment. + */ + if ( false !== strpos( + $pr_comment_item->body, + VIPGOCI_FILE_IS_APPROVED_MSG + ) ) { + vipgoci_github_pr_reviews_comments_delete( + $options, + $pr_comment_item->id + ); + } + } + } unset( $files_seen ); From d8a85dc5130754426ceecbafa611f1a1b0a21374 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 25 Sep 2018 18:56:14 +0000 Subject: [PATCH 091/142] Filtering capability for a function, function to delete a comment --- github-api.php | 73 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 67 insertions(+), 6 deletions(-) diff --git a/github-api.php b/github-api.php index 75dab6459..59b45d462 100644 --- a/github-api.php +++ b/github-api.php @@ -1184,18 +1184,33 @@ function vipgoci_github_pr_reviews_comments_get( */ function vipgoci_github_pr_reviews_comments_get_by_pr( $options, - $pr_number + $pr_number, + $filter = array() ) { vipgoci_log( 'Fetching all review comments submitted to a Pull-Request', array( - 'repo_owner' => $options['repo-owner'], - 'repo_name' => $options['repo-name'], - 'commit_id' => $options['commit'], - 'pr_number' => $pr_number, + 'repo_owner' => $options['repo-owner'], + 'repo_name' => $options['repo-name'], + 'commit_id' => $options['commit'], + 'pr_number' => $pr_number, + 'filter' => $filter, ) ); + + if ( + ( isset( $filter['login'] ) ) && + ( 'myself' === $filter['login'] ) + ) { + /* Get info about token-holder */ + $current_user_info = vipgoci_github_authenticated_user_get( + $options['token'] + ); + + $filter['login'] = $current_user_info->login; + } + $page = 1; $per_page = 100; @@ -1221,6 +1236,13 @@ function vipgoci_github_pr_reviews_comments_get_by_pr( ); foreach( $comments as $comment ) { + if ( + ( isset( $filter['login'] ) ) && + ( $comment->user->login !== $filter['login'] ) + ) { + continue; + } + $all_comments[] = $comment; } @@ -1230,6 +1252,42 @@ function vipgoci_github_pr_reviews_comments_get_by_pr( return $all_comments; } + +/* + * Remove a particular comment. + */ + +function vipgoci_github_pr_reviews_comments_delete( + $options, + $comment_id +) { + vipgoci_log( + 'Deleting an inline comment from a Pull-Request ' . + 'Review', + array( + 'repo_owner' => $options['repo-owner'], + 'repo_name' => $options['repo-name'], + 'comment_id' => $comment_id, + ) + ); + + $github_url = + VIPGOCI_GITHUB_BASE_URL . '/' . + 'repos/' . + rawurlencode( $options['repo-owner'] ) . '/' . + rawurlencode( $options['repo-name'] ) . '/' . + 'pulls/' . + 'comments/' . + rawurlencode( $comment_id ); + + vipgoci_github_post_url( + $github_url, + array(), + $options['token'], + true // Indicates a 'DELETE' request + ); +} + /* * Get all generic comments made to a Pull-Request from Github. */ @@ -2248,7 +2306,10 @@ function vipgoci_github_pr_reviews_dismiss_non_active_comments( */ $all_comments = vipgoci_github_pr_reviews_comments_get_by_pr( $options, - $pr_number + $pr_number, + array( + 'login' => 'myself', + ) ); if ( count( $all_comments ) === 0 ) { From dc90d4fa62f6d9418ee23a70fce059438e068a4b Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 25 Sep 2018 19:14:25 +0000 Subject: [PATCH 092/142] Adding caching, bugfix --- github-api.php | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/github-api.php b/github-api.php index 59b45d462..06df5fc20 100644 --- a/github-api.php +++ b/github-api.php @@ -1187,17 +1187,38 @@ function vipgoci_github_pr_reviews_comments_get_by_pr( $pr_number, $filter = array() ) { + + /* + * Calculate caching ID + */ + $cache_id = array( + __FUNCTION__, $options['repo-owner'], $options['repo-name'], + $pr_number, $filter + ); + + /* + * Try to get cached data + */ + $cached_data = vipgoci_cache( $cache_id ); + vipgoci_log( - 'Fetching all review comments submitted to a Pull-Request', + 'Fetching all review comments submitted to a Pull-Request' . + (( $cached_data !== false ) ? ' (cached)' : '' ), array( 'repo_owner' => $options['repo-owner'], 'repo_name' => $options['repo-name'], - 'commit_id' => $options['commit'], 'pr_number' => $pr_number, 'filter' => $filter, ) ); + /* + * If we have the information cached, + * return that. + */ + if ( false !== $cached_data ) { + return $cached_data; + } if ( ( isset( $filter['login'] ) ) && @@ -1249,6 +1270,11 @@ function vipgoci_github_pr_reviews_comments_get_by_pr( $page++; } while( count( $comments ) >= $per_page ); + /* + * Cache the results and return + */ + vipgoci_cache( $cache_id, $all_comments ); + return $all_comments; } @@ -1791,6 +1817,7 @@ function vipgoci_github_pr_reviews_get( */ do { +echo "Foobar" . PHP_EOL; $github_url = VIPGOCI_GITHUB_BASE_URL . '/' . 'repos/' . @@ -1852,6 +1879,8 @@ function vipgoci_github_pr_reviews_get( $ret_reviews[] = $pr_review; } + + $page++; } while( count( $pr_reviews ) >= $per_page ); return $ret_reviews; From 50ab1f40a0dfdfd440565aca320cf0b30932d75f Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 25 Sep 2018 19:26:09 +0000 Subject: [PATCH 093/142] Removing FIXME --- github-api.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/github-api.php b/github-api.php index 06df5fc20..f89dd2eb7 100644 --- a/github-api.php +++ b/github-api.php @@ -1105,15 +1105,6 @@ function vipgoci_github_pr_reviews_comments_get( $per_page = 100; $prs_comments_cache = array(); - /* - * FIXME: - * - * Asking for all the pages from GitHub - * might get expensive as we process more - * commits/hour -- maybe cache this in memcache, - * making it possible to share data between processes. - */ - do { $github_url = VIPGOCI_GITHUB_BASE_URL . '/' . From c3e303c80506c1fca8eb13ee34f39b0b1428b896 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 25 Sep 2018 19:29:51 +0000 Subject: [PATCH 094/142] Updated comment --- auto-approval.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/auto-approval.php b/auto-approval.php index 00e338607..522c25e5f 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -358,9 +358,10 @@ function vipgoci_auto_approval( * approved anyway. */ vipgoci_log( - 'Removing any comments indicating that a ' . - 'particular file is approved, as ' . - 'the whole Pull-Request is approved', + 'Removing any previously submitted comments ' . + 'indicating that a particular file ' . + 'is approved as the whole ' . + 'Pull-Request is approved', array( 'pr_number' => $pr_item->number, ) From ed787f84704482e6ba73546720d0a0009efbf9c5 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Wed, 26 Sep 2018 12:06:12 +0000 Subject: [PATCH 095/142] Removing debug message --- github-api.php | 1 - 1 file changed, 1 deletion(-) diff --git a/github-api.php b/github-api.php index f89dd2eb7..1d00b8fbf 100644 --- a/github-api.php +++ b/github-api.php @@ -1808,7 +1808,6 @@ function vipgoci_github_pr_reviews_get( */ do { -echo "Foobar" . PHP_EOL; $github_url = VIPGOCI_GITHUB_BASE_URL . '/' . 'repos/' . From 7346e7bf555523a17dd9e82bb6b754713763ec03 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 28 Sep 2018 12:24:22 +0000 Subject: [PATCH 096/142] Adding filtering capability to vipgoci_github_pr_reviews_comments_get_by_pr() --- github-api.php | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/github-api.php b/github-api.php index 1d00b8fbf..1be8b45f2 100644 --- a/github-api.php +++ b/github-api.php @@ -1171,7 +1171,15 @@ function vipgoci_github_pr_reviews_comments_get( /* * Get all review-comments submitted to a - * particular Pull-Request + * particular Pull-Request. + * Supports filtering:Will filter the + * - User submitted (parameter: login) + * - Comment state (parameter: comments_active, true/false) + * + * Note that parameter login can be assigned a magic + * value, 'myself', in which case the actual username + * will be assumed to be that of the token-holder. + */ function vipgoci_github_pr_reviews_comments_get_by_pr( $options, @@ -1254,6 +1262,18 @@ function vipgoci_github_pr_reviews_comments_get_by_pr( ) { continue; } + + if ( isset( $filter['comments_active'] ) ) { + if ( + ( ( $comment->position !== null ) && + ( $filter['comments_active'] === false ) ) + || + ( ( $comment->position === null ) && + ( $filter['comments_active'] === true ) ) + ) { + continue; + } + } $all_comments[] = $comment; } From 6f73961c1d2c1f48f0788574679fa2998d1e7ff1 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 28 Sep 2018 14:40:42 +0000 Subject: [PATCH 097/142] Removing comments if there are excessive number of them --- misc.php | 215 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 215 insertions(+) diff --git a/misc.php b/misc.php index e360a20df..67ec4f785 100644 --- a/misc.php +++ b/misc.php @@ -1066,3 +1066,218 @@ function vipgoci_approved_files_comments_remove( ) ); } + +/* + * Limit the number of to-be-submitted comments to + * the Pull-Requests. We take into account the number + * to be submitted for each Pull-Request, the number of + * comments already submitted, and the limit specified + * on start-up. Comments are removed as needed, and + * what comments are removed is reported. + */ +function vipgoci_github_results_filter_comments_to_max( + $options, + &$results +) { + + vipgoci_log( + 'Preparing to remove any excessive comments from array of ' . + 'issues to be submitted to PRs', + array( + 'review_comments_total_max' + => $options['review-comments-total-max'], + ) + ); + + + /* + * We might need to remove comments. + * + * We will begin with lower priority comments + * first, remove them, and then progressively + * continue removing comments as priority increases + * and there is still a need for removal. + */ + + /* + * Keep track of what we remove. + */ + $comments_removed = array(); + + foreach( + $results['issues'] as + $pr_number => $pr_issues_comments + ) { + /* + * Take into account previously submitted comments + * by us for the current Pull-Request. + */ + + $pr_previous_comments_cnt = count( + vipgoci_github_pr_reviews_comments_get_by_pr( + $options, + $pr_number, + array( + 'login' => 'myself', + 'comments_active' => true, + ) + ) + ); + + /* + * How many comments need + * to be removed? Count in + * comments in the PR in addition + * to possible new ones, substract + * from the maximum specified. + */ + + $comments_to_remove = + ( + count( $pr_issues_comments ) + + + $pr_previous_comments_cnt + ) + - + $options['review-comments-total-max']; + + /* + * If there are no comments to remove, + * skip and continue. + */ + if ( $comments_to_remove <= 0 ) { + continue; + } + + /* + * If more are to be removed than are to be + * submitted, limit to the number of available ones. + */ + else if ( + $comments_to_remove > + count( $pr_issues_comments ) + ) { + $comments_to_remove = count( $pr_issues_comments ); + } + + /* + * Figure out severity, minimum and maximum. + */ + + $severity_min = 0; + $severity_max = 0; + + foreach( $pr_issues_comments as $pr_issue ) { + $severity_min = min( + $pr_issue['issue']['severity'], + $severity_min + ); + + $severity_max = max( + $pr_issue['issue']['severity'], + $severity_max + ); + } + + /* + * Loop through severity-levels from low to high + * and remove comments as needed. + */ + for ( + $i = $severity_min; + $i <= $severity_max && $comments_to_remove > 0; + $i++ + ) { + foreach( + $pr_issues_comments as + $pr_issue_key => $pr_issue + ) { + /* + * If we have removed enough, stop here. + */ + if ( $comments_to_remove <= 0 ) { + break; + } + + /* + * Not correct severity level? Ignore. + */ + if ( $pr_issue['issue']['severity'] !== $i ) { + continue; + } + + /* + * Actually remove and + * keep statistics up to date. + */ + + unset( + $results[ + 'issues' + ][ + $pr_number + ][ + $pr_issue_key + ] + ); + + $results[ + 'stats' + ][ + $pr_issue['type'] + ][ + $pr_number + ][ + strtolower( + $pr_issue['issue']['type'] + ) + ]--; + + /* + * Keep track of what we remove + */ + if ( ! isset( + $comments_removed[ + $pr_number + ] + ) ) { + $comments_removed[ + $pr_number + ] = array(); + } + + $comments_removed[ + $pr_number + ] = $pr_issue; + + $comments_to_remove--; + } + } + + /* + * Re-create array so to + * keep continuous ordering + * of index. + */ + $results[ + 'issues' + ][ + $pr_number + ] = array_values( + $results[ + 'issues' + ][ + $pr_number + ] + ); + } + + vipgoci_log( + 'Removed issue comments from array of to be submitted ' . + 'comments to PRs due to limit constraints', + array( + 'review_comments_total_max' => $options['review-comments-total-max'], + 'comments_removed' => $comments_removed, + ) + ); +} From abaca7fbd89d2f8662bbb12f388f7043af641fee Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 28 Sep 2018 14:42:04 +0000 Subject: [PATCH 098/142] Removing excessive number of comments --- vip-go-ci.php | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/vip-go-ci.php b/vip-go-ci.php index e44a12904..1d2b129ff 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -351,6 +351,7 @@ function vipgoci_run() { 'commit:', 'token:', 'review-comments-max:', + 'review-comments-total-max:', 'dismiss-stale-reviews:', 'branches-ignore:', 'output:', @@ -403,6 +404,10 @@ function vipgoci_run() { "\t" . ' to GitHub in one review. If the number of ' . PHP_EOL . "\t" . ' comments exceed this number, additional reviews ' . PHP_EOL . "\t" . ' will be submitted.' . PHP_EOL . + "\t" . '--review-comments-total-max=NUMBER Maximum number of inline comments submitted to' . PHP_EOL . + "\t" . ' a single Pull-Request by the program -- includes' . PHP_EOL . + "\t" . ' comments from previous executions. A value of ' . PHP_EOL . + "\t" . ' \'0\' indicates no limit.' . PHP_EOL . "\t" . '--dismiss-stale-reviews=BOOL Dismiss any reviews associated with Pull-Requests ' . PHP_EOL . "\t" . ' that we process which have no active comments. ' . PHP_EOL . "\t" . ' The Pull-Requests we process are those associated ' . PHP_EOL . @@ -609,6 +614,19 @@ function vipgoci_run() { range( 5, 100, 1 ) ); + /* + * Overall maximum number of inline comments + * posted to GitHub Pull-Request Reviews -- from + * 0 to 500. 0 means unlimited. + */ + + vipgoci_option_integer_handle( + $options, + 'review-comments-total-max', + 200, + range( 0, 500, 1 ) + ); + /* * Handle optional --informational-url -- * URL to information on what this bot does. @@ -1039,7 +1057,7 @@ function vipgoci_run() { vipgoci_auto_approval( $options, $auto_approved_files_arr, - $results + $results // FIXME: dry-run ); } @@ -1067,10 +1085,18 @@ function vipgoci_run() { ); /* - * FIXME: Limit number of issues in $results - * -- take into account previously posted issues. + * Limit number of issues in $results. + * + * If set to zero, skip this part. */ + if ( 0 !== $options['review-comments-total-max'] ) { + vipgoci_github_results_filter_comments_to_max( + $options, + $results + ); + } + /* * Submit any remaining issues to GitHub */ From 0d9f0698e2d5826e717c536e8a1ed7578d2475c2 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 26 Oct 2018 16:37:09 +0000 Subject: [PATCH 099/142] List all files altered/added by the PRs (for debugging). --- phpcs-scan.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/phpcs-scan.php b/phpcs-scan.php index 045e31adf..522653891 100644 --- a/phpcs-scan.php +++ b/phpcs-scan.php @@ -411,6 +411,8 @@ function vipgoci_phpcs_scan_commit( 'repo_owner' => $repo_owner, 'repo_name' => $repo_name, 'commit_id' => $commit_id, + 'all_files_changed_by_prs' => + $pr_item_files_changed['all'], ) ); From 2077705321311c881e1b53c96dcb6fb484f8853d Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 5 Nov 2018 16:57:10 +0000 Subject: [PATCH 100/142] Moving functionality around for more efficiency. --- ap-file-types.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ap-file-types.php b/ap-file-types.php index 6ff92881b..eac8265c1 100644 --- a/ap-file-types.php +++ b/ap-file-types.php @@ -57,13 +57,9 @@ function vipgoci_ap_file_types( foreach ( $pr_diff as $pr_diff_file_name => $pr_diff_contents ) { - $pr_diff_file_extension = vipgoci_file_extension( - $pr_diff_file_name - ); - /* * If the file is already in the array - * of approved file, do not do anything. + * of approved files, do not do anything. */ if ( isset( $auto_approved_files_arr[ @@ -73,6 +69,10 @@ function vipgoci_ap_file_types( continue; } + $pr_diff_file_extension = vipgoci_file_extension( + $pr_diff_file_name + ); + /* * Check if the extension of the file * is in a list of auto-approvable From ee32a38c993d329378f0a8e62714161c46e0b188 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 5 Nov 2018 16:58:54 +0000 Subject: [PATCH 101/142] Adding documentation, cleanup. --- vip-go-ci.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/vip-go-ci.php b/vip-go-ci.php index 1d2b129ff..4a176aaa1 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -427,7 +427,9 @@ function vipgoci_run() { "\t" . ' default in $PATH will be used instead' . PHP_EOL . "\t" . '--svg-checks=BOOL Enable or disable SVG checks, both' . PHP_EOL . "\t" . ' auto-approval of SVG files and problem' . PHP_EOL . - "\t" . ' checking of these files' . PHP_EOL . + "\t" . ' checking of these files. Note that if' . PHP_EOL . + "\t" . ' auto-approvals are turned off globally, no' . PHP_EOL . + "\t" . ' auto-approval is performed for SVG files.' . PHP_EOL . "\t" . '--hashes-api=BOOL Whether to do hashes-to-hashes API verfication ' . PHP_EOL . "\t" . ' with individual PHP files found to be altered ' . PHP_EOL . "\t" . ' in the branch specified' . PHP_EOL . @@ -742,11 +744,8 @@ function vipgoci_run() { /* * Cross-reference: We disallow autoapproving - * PHP and JS files here, because hashes-api - * could autoapprove them and because they can - * contain dangerous code. By doing this, we - * avoid any possible conflicts between autoapproval - * and hases-api. + * PHP and JS files here, because they chould contain + * contain dangerous code. */ ( ( in_array( From 401052b50d2881e767b74acfac5c960ec7b6cc67 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 5 Nov 2018 16:59:18 +0000 Subject: [PATCH 102/142] More details when logging --- phpcs-scan.php | 1 + 1 file changed, 1 insertion(+) diff --git a/phpcs-scan.php b/phpcs-scan.php index 522653891..eb43641f4 100644 --- a/phpcs-scan.php +++ b/phpcs-scan.php @@ -89,6 +89,7 @@ function vipgoci_phpcs_scan_single_file( 'repo_name' => $options['repo-name'], 'commit_id' => $options['commit'], 'filename' => $file_name, + 'file_extension' => $file_extension, 'temp_file_name' => $temp_file_name, ) ); From 764d7efe4d448d3a9803641dced21206b42f44fd Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 6 Nov 2018 13:32:08 +0000 Subject: [PATCH 103/142] New defines for vipgoci_runtime_measure() --- defines.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/defines.php b/defines.php index d1cbb6885..de3c9e6ce 100644 --- a/defines.php +++ b/defines.php @@ -46,3 +46,12 @@ define( 'VIPGOCI_APPROVAL_AUTOAPPROVE', 'auto-approval' ); define( 'VIPGOCI_APPROVAL_HASHES_API', 'hashes-api' ); + + +/* + * Defines for vipgoci_runtime_measure() + */ + +define( 'VIPGOCI_RUNTIME_START', 'start' ); +define( 'VIPGOCI_RUNTIME_STOP', 'stop' ); +define( 'VIPGOCI_RUNTIME_DUMP', 'dump' ); From f77615d9a82fa1bfb048bf121e16e784400a84ca Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 6 Nov 2018 13:37:06 +0000 Subject: [PATCH 104/142] Using constants for vipgoci_runtime_measure() --- ap-file-types.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ap-file-types.php b/ap-file-types.php index eac8265c1..431548b54 100644 --- a/ap-file-types.php +++ b/ap-file-types.php @@ -19,7 +19,7 @@ function vipgoci_ap_file_types( &$auto_approved_files_arr ) { - vipgoci_runtime_measure( 'start', 'ap_file_types' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'ap_file_types' ); vipgoci_log( 'Doing auto-approval scanning based on file-types', @@ -101,6 +101,6 @@ function vipgoci_ap_file_types( gc_collect_cycles(); - vipgoci_runtime_measure( 'stop', 'ap_file_types' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'ap_file_types' ); } From bca66dfb7667c3ea8633b814cf0dbab797e75b8c Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 6 Nov 2018 13:39:20 +0000 Subject: [PATCH 105/142] Using constants for vipgoci_runtime_measure() --- git-repo.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/git-repo.php b/git-repo.php index 545398b5b..75cf85406 100644 --- a/git-repo.php +++ b/git-repo.php @@ -92,11 +92,11 @@ function vipgoci_git_repo_get_head( $local_git_repo ) { ); /* Actually execute */ - vipgoci_runtime_measure( 'start', 'git_cli' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'git_cli' ); $result = shell_exec( $cmd ); - vipgoci_runtime_measure( 'stop', 'git_cli' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'git_cli' ); return $result; } @@ -205,13 +205,13 @@ function vipgoci_gitrepo_fetch_committed_file( * If everything seems fine, return the file. */ - vipgoci_runtime_measure( 'start', 'git_repo_fetch_file' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'git_repo_fetch_file' ); $file_contents_tmp = @file_get_contents( $local_git_repo . '/' . $file_name ); - vipgoci_runtime_measure( 'stop', 'git_repo_fetch_file' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'git_repo_fetch_file' ); return $file_contents_tmp; } @@ -231,7 +231,7 @@ function vipgoci_gitrepo_blame_for_file( $commit_id, $local_git_repo ); - vipgoci_runtime_measure( 'start', 'git_repo_blame_for_file' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'git_repo_blame_for_file' ); vipgoci_log( 'Fetching \'git blame\' log from Git repository for file', @@ -255,11 +255,11 @@ function vipgoci_gitrepo_blame_for_file( /* Actually execute */ - vipgoci_runtime_measure( 'start', 'git_cli' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'git_cli' ); $result = shell_exec( $cmd ); - vipgoci_runtime_measure( 'stop', 'git_cli' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'git_cli' ); /* * Process the output from git, @@ -348,7 +348,7 @@ function vipgoci_gitrepo_blame_for_file( } } - vipgoci_runtime_measure( 'stop', 'git_repo_blame_for_file' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'git_repo_blame_for_file' ); return $blame_log; } From 61a43d2a086787886ce51962c64515db98920fc0 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 6 Nov 2018 13:39:36 +0000 Subject: [PATCH 106/142] Using constants for vipgoci_runtime_measure() --- vip-go-ci.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vip-go-ci.php b/vip-go-ci.php index 4a176aaa1..c14519987 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -1133,7 +1133,7 @@ function vipgoci_run() { 'run_time_seconds' => time() - $startup_time, 'run_time_measurements' => vipgoci_runtime_measure( - 'dump', + VIPGOCI_RUNTIME_DUMP, null ), 'counters_report' => From b8249e29cd7d7f626fc8425d32cd16a8e2f8a602 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 6 Nov 2018 13:41:01 +0000 Subject: [PATCH 107/142] Using constants for vipgoci_runtime_measure() --- svg-scan.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/svg-scan.php b/svg-scan.php index dd7b43790..54d4ce21e 100644 --- a/svg-scan.php +++ b/svg-scan.php @@ -13,7 +13,7 @@ function vipgoci_svg_scan_single_file( $options, $file_name ) { - vipgoci_runtime_measure( 'start', 'svg_scan_single_file' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'svg_scan_single_file' ); vipgoci_log( 'Scanning single SVG file', @@ -67,7 +67,7 @@ function vipgoci_svg_scan_single_file( */ if ( 'svg' !== $file_extension ) { - vipgoci_runtime_measure( 'stop', 'svg_scan_single_file' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'svg_scan_single_file' ); vipgoci_log( 'Could not scan file, does not seem to be a SVG file', @@ -205,7 +205,7 @@ function vipgoci_svg_scan_single_file( ) ); - vipgoci_runtime_measure( 'stop', 'svg_scan_single_file' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'svg_scan_single_file' ); vipgoci_log( 'SVG scanning of a single file finished', From 87263ae9d99fe396b34b72c0eb2a8998e599adcc Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 6 Nov 2018 13:41:25 +0000 Subject: [PATCH 108/142] Using constants for vipgoci_runtime_measure --- phpcs-scan.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/phpcs-scan.php b/phpcs-scan.php index eb43641f4..39c8fd909 100644 --- a/phpcs-scan.php +++ b/phpcs-scan.php @@ -46,11 +46,11 @@ function vipgoci_phpcs_do_scan( 2 ); - vipgoci_runtime_measure( 'start', 'phpcs_cli' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'phpcs_cli' ); $result = shell_exec( $cmd ); - vipgoci_runtime_measure( 'stop', 'phpcs_cli' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'phpcs_cli' ); return $result; } @@ -269,7 +269,7 @@ function vipgoci_phpcs_scan_commit( $commit_id = $options['commit']; $github_token = $options['token']; - vipgoci_runtime_measure( 'start', 'phpcs_scan_commit' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'phpcs_scan_commit' ); vipgoci_log( 'About to PHPCS-scan repository', @@ -423,7 +423,7 @@ function vipgoci_phpcs_scan_commit( * Loop through each file affected by * the commit. */ - vipgoci_runtime_measure( 'start', 'phpcs_scan_single_file' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'phpcs_scan_single_file' ); $file_extension = vipgoci_file_extension( $file_name @@ -543,7 +543,7 @@ function( $item ) { gc_collect_cycles(); - vipgoci_runtime_measure( 'stop', 'phpcs_scan_single_file' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'phpcs_scan_single_file' ); } @@ -700,6 +700,6 @@ function( $item ) { gc_collect_cycles(); - vipgoci_runtime_measure( 'stop', 'phpcs_scan_commit' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'phpcs_scan_commit' ); } From a14a650cfa9ee1cd31356607fdecbe45c5f8f473 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 6 Nov 2018 13:42:02 +0000 Subject: [PATCH 109/142] Using constant for vipgoci_runtime_measure() --- lint-scan.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lint-scan.php b/lint-scan.php index c88157eb1..79f69b39a 100644 --- a/lint-scan.php +++ b/lint-scan.php @@ -36,11 +36,11 @@ function vipgoci_lint_do_scan( * measure how long time it took */ - vipgoci_runtime_measure( 'start', 'php_lint_cli' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'php_lint_cli' ); exec( $cmd, $file_issues_arr ); - vipgoci_runtime_measure( 'stop', 'php_lint_cli' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'php_lint_cli' ); vipgoci_log( @@ -154,7 +154,7 @@ function vipgoci_lint_scan_commit( $commit_id = $options['commit']; $github_token = $options['token']; - vipgoci_runtime_measure( 'start', 'lint_scan_commit' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'lint_scan_commit' ); vipgoci_log( 'About to lint PHP-files', @@ -209,7 +209,7 @@ function vipgoci_lint_scan_commit( */ foreach( $commit_tree as $filename ) { - vipgoci_runtime_measure( 'start', 'lint_scan_single_file' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'lint_scan_single_file' ); $file_contents = vipgoci_gitrepo_fetch_committed_file( $repo_owner, @@ -281,7 +281,7 @@ function vipgoci_lint_scan_commit( // If there are no new issues, just leave it at that if ( empty( $file_issues_arr ) ) { - vipgoci_runtime_measure( 'stop', 'lint_scan_single_file' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'lint_scan_single_file' ); continue; } @@ -352,7 +352,7 @@ function vipgoci_lint_scan_commit( } } - vipgoci_runtime_measure( 'stop', 'lint_scan_single_file' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'lint_scan_single_file' ); } @@ -369,5 +369,5 @@ function vipgoci_lint_scan_commit( gc_collect_cycles(); - vipgoci_runtime_measure( 'stop', 'lint_scan_commit' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'lint_scan_commit' ); } From cf943764be628c2f204f4300abdcdc55a850517d Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 6 Nov 2018 13:45:07 +0000 Subject: [PATCH 110/142] Constants used, moving code around, cleanups --- ap-hashes-api.php | 54 ++++++++++++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/ap-hashes-api.php b/ap-hashes-api.php index 66f82bb94..7567a3754 100644 --- a/ap-hashes-api.php +++ b/ap-hashes-api.php @@ -9,16 +9,15 @@ function vipgoci_ap_hashes_api_file_approved( $options, $file_path ) { - vipgoci_runtime_measure( 'start', 'hashes_api_scan_file' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'hashes_api_scan_file' ); /* * Make sure to process only *.php and * *.js files -- others are ignored. * * Cross-reference: These file types are not - * auto-approved by our own auto-approval - * mechanism, as to avoid any conflicts between - * hashes-api and the auto-approval mechanism. + * auto-approved by the auto-approval mechanism -- + * see vip-go-ci.php. */ $file_extensions_approvable = array( @@ -53,6 +52,8 @@ function vipgoci_ap_hashes_api_file_approved( ) ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'hashes_api_scan_file' ); + return null; } @@ -91,6 +92,8 @@ function vipgoci_ap_hashes_api_file_approved( ) ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'hashes_api_scan_file' ); + return null; } @@ -194,6 +197,8 @@ function vipgoci_ap_hashes_api_file_approved( ) ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'hashes_api_scan_file' ); + return null; } @@ -241,7 +246,7 @@ function vipgoci_ap_hashes_api_file_approved( $file_approved = false; } - vipgoci_runtime_measure( 'stop', 'hashes_api_scan_file' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'hashes_api_scan_file' ); return $file_approved; } @@ -259,11 +264,11 @@ function vipgoci_ap_hashes_api_scan_commit( &$commit_issues_stats, &$auto_approved_files_arr ) { - vipgoci_runtime_measure( 'start', 'hashes_api_scan' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'hashes_api_scan' ); vipgoci_log( 'Scanning altered or new files affected by Pull-Request(s) ' . - 'using hashes-to-hashes database via API', + 'using hashes-to-hashes API', array( 'repo_owner' => $options['repo-owner'], 'repo_name' => $options['repo-name'], @@ -296,6 +301,19 @@ function vipgoci_ap_hashes_api_scan_commit( foreach( $pr_diff as $pr_diff_file_name => $pr_diff_contents ) { + /* + * If it is already approved, + * do not do anything. + */ + + if ( isset( + $auto_approved_files_arr[ + $pr_diff_file_name + ] + ) ) { + continue; + } + /* * Check if the hashes-to-hashes database * recognises this file, and check its @@ -321,20 +339,9 @@ function vipgoci_ap_hashes_api_scan_commit( ) ); - /* - * If it is already approved, - * do not add again. - */ - - if ( ! isset( - $auto_approved_files_arr[ - $pr_diff_file_name - ] - ) ) { - $auto_approved_files_arr[ - $pr_diff_file_name - ] = 'autoapprove-hashes-to-hashes'; - } + $auto_approved_files_arr[ + $pr_diff_file_name + ] = 'autoapprove-hashes-to-hashes'; } else if ( false === $approval_status ) { @@ -365,10 +372,13 @@ function vipgoci_ap_hashes_api_scan_commit( */ unset( $prs_implicated ); + unset( $pr_item ); unset( $pr_diff ); + unset( $pr_diff_contents ); + unset( $approval_status ); gc_collect_cycles(); - vipgoci_runtime_measure( 'stop', 'hashes_api_scan' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'hashes_api_scan' ); } From 07d4d128c64f80c2a4077359e47a22c4a9ebd116 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 6 Nov 2018 13:50:35 +0000 Subject: [PATCH 111/142] Using constant, moving functionality around --- ap-svg-files.php | 41 +++++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/ap-svg-files.php b/ap-svg-files.php index 68726df0b..23d028cc2 100644 --- a/ap-svg-files.php +++ b/ap-svg-files.php @@ -19,7 +19,7 @@ function vipgoci_ap_svg_files( &$auto_approved_files_arr ) { - vipgoci_runtime_measure( 'start', 'ap_svg_files' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'ap_svg_files' ); vipgoci_log( 'Doing auto-approval scanning for SVG files', @@ -93,14 +93,40 @@ function vipgoci_ap_svg_files( $tmp_scan_results['file_issues_arr_master']; /* - * If no issues were found, we - * can approve this file. + * Check for failure */ if ( - ( isset( + ( ! isset( $file_issues_arr_master['totals'] ) ) - && + || + ( ! isset( + $file_issues_arr_master['totals']['errors'] + ) ) + || + ( ! isset( + $file_issues_arr_master['totals']['warnings'] + ) ) + ) { + vipgoci_log( + 'Not adding SVG file to list of ' . + 'approved files as a failure occurred', + + array( + 'file_name' => + $pr_diff_file_name, + 'file_issues_arr_master' => + $file_issues_arr_master, + ) + ); + } + + + /* + * If no issues were found, we + * can approve this file. + */ + else if ( ( 0 === $file_issues_arr_master['totals']['errors'] ) @@ -132,6 +158,8 @@ function vipgoci_ap_svg_files( array( 'file_name' => $pr_diff_file_name, + 'file_issues_arr_master' => + $file_issues_arr_master, ) ); } @@ -147,9 +175,10 @@ function vipgoci_ap_svg_files( unset( $pr_item ); unset( $pr_diff_file_extension ); unset( $pr_diff_file_name ); + unset( $file_issues_arr_master ); gc_collect_cycles(); - vipgoci_runtime_measure( 'stop', 'ap_svg_files' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'ap_svg_files' ); } From c2282594919901d116e07aeffe6f9f1382ecdd67 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 6 Nov 2018 13:52:43 +0000 Subject: [PATCH 112/142] Renaming variables, using constants, fixing printed messages --- misc.php | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/misc.php b/misc.php index 67ec4f785..0dc3a826b 100644 --- a/misc.php +++ b/misc.php @@ -266,14 +266,14 @@ function vipgoci_save_temp_file( } if ( false !== $temp_file_name ) { - vipgoci_runtime_measure( 'start', 'save_temp_file' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'save_temp_file' ); $temp_file_save_status = file_put_contents( $temp_file_name, $file_contents ); - vipgoci_runtime_measure( 'stop', 'save_temp_file' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'save_temp_file' ); } // Detect possible errors when saving the temporary file @@ -446,11 +446,11 @@ function vipgoci_scandir_git_repo( $path, $filter ) { ); - vipgoci_runtime_measure( 'start', 'git_repo_scandir' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'git_repo_scandir' ); $cdir = scandir( $path ); - vipgoci_runtime_measure( 'stop', 'git_repo_scandir' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'git_repo_scandir' ); foreach ( $cdir as $key => $value ) { @@ -571,15 +571,15 @@ function vipgoci_runtime_measure( $action = null, $type = null ) { * Check usage. */ if ( - ( 'start' !== $action ) && - ( 'stop' !== $action ) && - ( 'dump' !== $action ) + ( VIPGOCI_RUNTIME_START !== $action ) && + ( VIPGOCI_RUNTIME_STOP !== $action ) && + ( VIPGOCI_RUNTIME_DUMP !== $action ) ) { return false; } // Dump all runtimes we have - if ( 'dump' === $action ) { + if ( VIPGOCI_RUNTIME_DUMP === $action ) { return $runtime; } @@ -594,13 +594,13 @@ function vipgoci_runtime_measure( $action = null, $type = null ) { } - if ( 'start' === $action ) { + if ( VIPGOCI_RUNTIME_START === $action ) { $timers[ $type ] = microtime( true ); return true; } - else if ( 'stop' === $action ) { + else if ( VIPGOCI_RUNTIME_STOP === $action ) { if ( ! isset( $timers[ $type ] ) ) { return false; } @@ -854,7 +854,7 @@ function vipgoci_remove_existing_github_comments_from_results( /* * Filter out issues that have already been - * reported got GitHub. + * reported to GitHub. */ if ( @@ -957,7 +957,7 @@ function vipgoci_approved_files_comments_remove( vipgoci_log( 'Removing any potential issues (errors, warnings) ' . - 'found for approved files', + 'found for approved files from internal results', array( 'auto_approved_files_arr' => $auto_approved_files_arr, @@ -1081,7 +1081,7 @@ function vipgoci_github_results_filter_comments_to_max( ) { vipgoci_log( - 'Preparing to remove any excessive comments from array of ' . + 'Preparing to remove any excessive number comments from array of ' . 'issues to be submitted to PRs', array( 'review_comments_total_max' @@ -1184,9 +1184,10 @@ function vipgoci_github_results_filter_comments_to_max( * and remove comments as needed. */ for ( - $i = $severity_min; - $i <= $severity_max && $comments_to_remove > 0; - $i++ + $severity_current = $severity_min; + $severity_current <= $severity_max && + $comments_to_remove > 0; + $severity_current++ ) { foreach( $pr_issues_comments as @@ -1202,7 +1203,10 @@ function vipgoci_github_results_filter_comments_to_max( /* * Not correct severity level? Ignore. */ - if ( $pr_issue['issue']['severity'] !== $i ) { + if ( + $pr_issue['issue']['severity'] !== + $severity_current + ) { continue; } From 14ec0054b82fe0e7a4a2f00e96d50cc72a9cd2a6 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 6 Nov 2018 14:00:20 +0000 Subject: [PATCH 113/142] Using constants, cleaning up memory, alterting comments --- auto-approval.php | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/auto-approval.php b/auto-approval.php index 522c25e5f..b5963a0fe 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -18,7 +18,7 @@ function vipgoci_auto_approval( &$auto_approved_files_arr, &$results ) { - vipgoci_runtime_measure( 'start', 'auto_approve_commit' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'auto_approve_commit' ); vipgoci_log( 'Doing auto-approval', @@ -61,7 +61,7 @@ function vipgoci_auto_approval( /* * Loop through all files that are * altered by the Pull-Request, look for - * files that can be auo-approved. + * files that can be auto-approved. */ foreach( $pr_diff as $pr_diff_file_name => $pr_diff_contents @@ -71,7 +71,6 @@ function vipgoci_auto_approval( $files_seen[] = $pr_diff_file_name; - /* * Is file in array of files * that can be auto-approved? @@ -110,11 +109,16 @@ function vipgoci_auto_approval( $auto_approved_files_arr, 'files_seen' => $files_seen, + 'pr_diff' => $pr_diff, ) ); } -// FIXME: Break into functions + + /* + * FIXME: Break into functions + */ + else if ( ( true === $did_foreach ) && ( false === $can_auto_approve ) @@ -312,7 +316,6 @@ function vipgoci_auto_approval( $pr_item->number, $options['commit'], $options['autoapprove-filetypes'], - VIPGOCI_APPROVAL_AUTOAPPROVE, $options['dry-run'] ); @@ -399,11 +402,25 @@ function vipgoci_auto_approval( /* * Reduce memory-usage as possible */ - unset( $prs_implicated ); + unset( $pr_diff ); + unset( $pr_diff_file_name ); + unset( $pr_diff_contents ); + unset( $pr_item ); + unset( $pr_comments ); + unset( $pr_comment_item ); + unset( $pr_label ); + unset( $pr_item_reviews ); + unset( $pr_item_review ); + unset( $prs_implicated ); + unset( $files_seen ); + unset( $did_foreach ); + unset( $can_auto_approve ); + unset( $approved_file ); + unset( $approved_file_system ); gc_collect_cycles(); - vipgoci_runtime_measure( 'stop', 'auto_approve_commit' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'auto_approve_commit' ); } From 0b27aae572e06bcd7216d6f63d9203a402370916 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 6 Nov 2018 14:02:26 +0000 Subject: [PATCH 114/142] Updating comments, using constants, removing obsolete code. --- github-api.php | 75 ++++++++++++++++++++++++-------------------------- 1 file changed, 36 insertions(+), 39 deletions(-) diff --git a/github-api.php b/github-api.php index 1be8b45f2..ec69fe1ef 100644 --- a/github-api.php +++ b/github-api.php @@ -142,7 +142,7 @@ function vipgoci_github_rate_limit_usage( function vipgoci_github_wait() { static $last_request_time = null; - vipgoci_runtime_measure( 'start', 'github_forced_wait' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'github_forced_wait' ); if ( null !== $last_request_time ) { /* @@ -156,7 +156,7 @@ function vipgoci_github_wait() { $last_request_time = time(); - vipgoci_runtime_measure( 'stop', 'github_forced_wait' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'github_forced_wait' ); } /* @@ -174,8 +174,6 @@ function vipgoci_oauth1_signature_get_hmac_sha1( $request_url, $parameters_arr ) { - $parts = array(); - /* * Start constructing the 'base string' -- * a crucial part of the signature. @@ -306,6 +304,11 @@ function vipgoci_oauth1_headers_get( $github_token ); + /* + * Those are not needed after this point, + * so we remove them to limit any risk + * of information leakage. + */ unset( $github_token['oauth_token_secret' ] ); unset( $github_token['oauth_consumer_secret' ] ); @@ -321,6 +324,18 @@ function vipgoci_oauth1_headers_get( $github_token_key => $github_token_value ) { + if ( strpos( + $github_token_key, + 'oauth_' + ) !== 0 ) { + /* + * If the token_key does not + * start with 'oauth_' we skip to + * avoid information-leakage. + */ + continue; + } + $res_header .= $sep . $github_token_key . '="' . @@ -429,13 +444,13 @@ function vipgoci_github_post_url( * and keep count of how many requests we do. */ - vipgoci_runtime_measure( 'start', 'github_api' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'github_api_post' ); vipgoci_counter_report( 'do', 'github_api_request_post', 1 ); $resp_data = curl_exec( $ch ); - vipgoci_runtime_measure( 'stop', 'github_api' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'github_api_post' ); $resp_headers = vipgoci_curl_headers( @@ -638,13 +653,13 @@ function vipgoci_github_fetch_url( * record of how long time it took, + and also keep count of how many we do. */ - vipgoci_runtime_measure( 'start', 'github_api' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'github_api_get' ); - vipgoci_counter_report( 'do', 'github_api_request_fetch', 1 ); + vipgoci_counter_report( 'do', 'github_api_request_get', 1 ); $resp_data = curl_exec( $ch ); - vipgoci_runtime_measure( 'stop', 'github_api' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'github_api_get' ); $resp_headers = vipgoci_curl_headers( @@ -785,13 +800,13 @@ function vipgoci_github_put_url( * and keep count of how many requests we do. */ - vipgoci_runtime_measure( 'start', 'github_api' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'github_api_put' ); vipgoci_counter_report( 'do', 'github_api_request_put', 1 ); $resp_data = curl_exec( $ch ); - vipgoci_runtime_measure( 'stop', 'github_api' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'github_api_put' ); $resp_headers = vipgoci_curl_headers( @@ -1647,6 +1662,9 @@ function vipgoci_github_pr_comments_error_msg( /* * Remove any comments made by us earlier. + * + * FIXME: For future alterations, move comments + * to be removed to arguments of this function. */ function vipgoci_github_pr_comments_cleanup( @@ -2357,7 +2375,7 @@ function vipgoci_github_pr_reviews_dismiss_non_active_comments( * from GitHub, do not do anything, as a precaution. * Receiving no comments might indicate a * failure (communication error or something else), - * and because we dismiss reviews that seem not to + * and if we dismiss reviews that seem not to * contain any comments, we might risk dismissing * all reviews when there is a failure. By * doing this, we take much less risk. @@ -2471,11 +2489,8 @@ function vipgoci_github_approve_pr( $pr_number, $latest_commit_id, $filetypes_approve, - $approval_type = null, $dry_run ) { - - $github_url = VIPGOCI_GITHUB_BASE_URL . '/' . 'repos/' . @@ -2492,30 +2507,12 @@ function vipgoci_github_approve_pr( 'comments' => array() ); - if ( VIPGOCI_APPROVAL_AUTOAPPROVE === $approval_type ) { - $github_postfields['body'] = - 'Auto-approved Pull-Request #' . - (int) $pr_number . ' as it ' . - 'contains only allowable file-types ' . - '(' . implode( ', ', $filetypes_approve ) . ')'; - } - - else if ( VIPGOCI_APPROVAL_HASHES_API === $approval_type ) { - $github_postfields['body'] = - 'Auto-approved Pull-Request #' . - (int) $pr_number . ' as it contains ' . - 'only files approved in hashes-to-hashes' . - 'database'; - } - - else { - vipgoci_sysexit( - 'Illegal usage of function', - array( - 'function_name' => __FUNCTION__, - ) - ); - } + $github_postfields['body'] = + 'Auto-approved Pull-Request #' . + (int) $pr_number . ' as it ' . + 'contains only auto-approvable files' . + '-- either pre-approved or file-types that are auto-approvable (' . + '(' . implode( ', ', $filetypes_approve ) . ').'; if ( true === $dry_run ) { return; From 121969996008f5c1ce01dbab1b4c58918bb8b5b7 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Thu, 8 Nov 2018 13:30:22 +0000 Subject: [PATCH 115/142] Adding API for sending messages to IRC --- other-web-services.php | 119 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 other-web-services.php diff --git a/other-web-services.php b/other-web-services.php new file mode 100644 index 000000000..f02681704 --- /dev/null +++ b/other-web-services.php @@ -0,0 +1,119 @@ + $msg_queue, + ) + ); + + foreach( $msg_queue as $message ) { + $irc_api_postfields = array( + 'message' => $message, + 'botname' => $botname, + 'channel' => $channel, + ); + + $ch = curl_init(); + + curl_setopt( + $ch, CURLOPT_URL, $irc_api_url + ); + + curl_setopt( + $ch, CURLOPT_RETURNTRANSFER, 1 + ); + + curl_setopt( + $ch, CURLOPT_CONNECTTIMEOUT, 20 + ); + + curl_setopt( + $ch, CURLOPT_USERAGENT, VIPGOCI_CLIENT_ID + ); + + curl_setopt( + $ch, CURLOPT_POST, 1 + ); + + curl_setopt( + $ch, + CURLOPT_POSTFIELDS, + json_encode( $irc_api_postfields ) + ); + + curl_setopt( + $ch, + CURLOPT_HEADERFUNCTION, + 'vipgoci_curl_headers' + ); + + curl_setopt( + $ch, + CURLOPT_HTTPHEADER, + array( 'Authorization: Bearer ' . $irc_api_token ) + ); + + /* + * Execute query, keep record of how long time it + * took, and keep count of how many requests we do. + */ + + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'irc_api_post' ); + + vipgoci_counter_report( 'do', 'irc_api_request_post', 1 ); + + $resp_data = curl_exec( $ch ); + + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'irc_api_post' ); + + $resp_headers = vipgoci_curl_headers( + null, + null + ); + + curl_close( $ch ); + + /* + * Enforce a small wait between requests. + */ + + time_nanosleep( 0, 500000000 ); + } +} From ed5a25eaa27ad6dd90a05f00e1ce5df06a94ea17 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Thu, 8 Nov 2018 13:31:04 +0000 Subject: [PATCH 116/142] Implementing IRC API support in vipgoci_log() --- misc.php | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/misc.php b/misc.php index 0dc3a826b..674900930 100644 --- a/misc.php +++ b/misc.php @@ -6,7 +6,12 @@ * our caller might pass us. */ -function vipgoci_log( $str, $debug_data = array(), $debug_level = 0 ) { +function vipgoci_log( + $str, + $debug_data = array(), + $debug_level = 0, + $irc = false +) { global $vipgoci_debug_level; /* @@ -32,6 +37,23 @@ function vipgoci_log( $str, $debug_data = array(), $debug_level = 0 ) { true ) . PHP_EOL; + + /* + * Send to IRC API as well if asked + * to do so. Include debugging information as well. + */ + if ( true === $irc ) { + vipgoci_irc_api_alert_queue( + $str . + '; ' . + print_r( + json_encode( + $debug_data + ), + true + ) + ); + } } /* From 91947f0890ff16ac9f30900acbef3b2fc58d08db Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Thu, 8 Nov 2018 13:53:00 +0000 Subject: [PATCH 117/142] Implement IRC API --- vip-go-ci.php | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 85 insertions(+), 1 deletion(-) diff --git a/vip-go-ci.php b/vip-go-ci.php index c14519987..d70eb107b 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -12,6 +12,7 @@ require_once( __DIR__ . '/ap-hashes-api.php' ); require_once( __DIR__ . '/ap-svg-files.php' ); require_once( __DIR__ . '/svg-scan.php' ); +require_once( __DIR__ . '/other-web-services.php' ); /* * Handle boolean parameters given on the command-line. @@ -365,6 +366,10 @@ function vipgoci_run() { 'hashes-oauth-token-secret:', 'hashes-oauth-consumer-key:', 'hashes-oauth-consumer-secret:', + 'irc-api-url:', + 'irc-api-token:', + 'irc-api-bot:', + 'irc-api-room:', 'php-path:', 'local-git-repo:', 'skip-folders:', @@ -443,6 +448,13 @@ function vipgoci_run() { "\t" . ' consumer secret needed for hashes-to-hashes HTTP requests' . PHP_EOL . "\t" . ' All required for hashes-to-hashes requests.' . PHP_EOL . PHP_EOL . + "\t" . '--irc-api-url=STRING URL to IRC API to send alerts' . PHP_EOL . + "\t" . '--irc-api-token=STRING Access-token to use to communicate with the IRC ' . PHP_EOL . + "\t" . ' API' . PHP_EOL . + "\t" . '--irc-api-bot=STRING Name for the bot which is supposed to send the IRC ' .PHP_EOL . + "\t" . ' messages.' . PHP_EOL . + "\t" . '--irc-api-room=STRING Name for the chatroom to which the IRC messages should ' . PHP_EOL . + "\t" . ' be sent. ' . PHP_EOL . "\t" . '--branches-ignore=STRING,... What branches to ignore -- useful to make sure' . PHP_EOL . "\t" . ' some branches never get scanned. Separate branches' . PHP_EOL . "\t" . ' with commas' . PHP_EOL . @@ -679,6 +691,55 @@ function vipgoci_run() { 'php' ); + /* + * Handle IRC API parameters + */ + + $irc_params_defined = 0; + + foreach( array( + 'irc-api-url', + 'irc-api-token', + 'irc-api-bot', + 'irc-api-room' + ) as $irc_api_param ) { + + if ( isset( $options[ $irc_api_param ] ) ) { + $options[ $irc_api_param ] = trim( + $options[ $irc_api_param ] + ); + + $options[ $irc_api_param ] = rtrim( + $options[ $irc_api_param ] + ); + + $irc_params_defined++; + } + } + + if ( isset( $options['irc-api-url'] ) ) { + vipgoci_option_url_handle( + $options, + 'irc-api-url', + null + ); + } + + if ( + ( $irc_params_defined > 0 ) && + ( $irc_params_defined !== 4 ) + ) { + vipgoci_sysexit( + 'Some IRC API parameters defined but not all; all must be defined to be useful', + array( + ), + VIPGOCI_EXIT_USAGE_ERROR + ); + } + + unset( $irc_params_defined ); + + /* * Do some sanity-checking on the parameters * @@ -813,6 +874,10 @@ function vipgoci_run() { $options_clean = $options; $options_clean['token'] = '***'; + if ( isset( $options_clean['irc-api-token'] ) ) { + $options_clean['irc-api-token'] = '***'; + } + foreach( $hashes_oauth_arguments as $hashes_oauth_argument ) { if ( isset( $options_clean[ $hashes_oauth_argument ] ) ) { $options_clean[ $hashes_oauth_argument ] = '***'; @@ -1085,7 +1150,7 @@ function vipgoci_run() { /* * Limit number of issues in $results. - * + * * If set to zero, skip this part. */ @@ -1122,6 +1187,25 @@ function vipgoci_run() { $options['review-comments-max'] ); + /* + * Send out to IRC API any alerts + * that are queued up. + */ + + if ( + ( ! empty( $options['irc-api-url'] ) ) && + ( ! empty( $options['irc-api-token'] ) ) && + ( ! empty( $options['irc-api-bot'] ) ) && + ( ! empty( $options['irc-api-room'] ) ) + ) { + vipgoci_irc_api_alerts_send( + $options['irc-api-url'], + $options['irc-api-token'], + $options['irc-api-bot'], + $options['irc-api-room'] + ); + } + $github_api_rate_limit_usage = vipgoci_github_rate_limit_usage( $options['token'] From 13705fee5ed7fbacb929bce4c83d0bfcf20b7ded Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Thu, 8 Nov 2018 14:05:52 +0000 Subject: [PATCH 118/142] Log to IRC information when approving and not approving. --- auto-approval.php | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/auto-approval.php b/auto-approval.php index b5963a0fe..2abd1baaa 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -130,6 +130,23 @@ function vipgoci_auto_approval( 'files which are not ' . 'automatically approvable', array( + 'repo_owner' => + $options['repo-owner'], + + 'repo_name' => + $options['repo-name'], + + 'pr_number' => + $pr_item->number, + + 'pr_url' => + 'https://github.com/' . + rawurlencode( $options['repo-owner'] ) . + '/' . + rawurlencode( $options['repo-name'] ) . + '/pull/' . + (int) $pr_item->number, + 'autoapprove-filetypes' => $options['autoapprove-filetypes'], @@ -137,7 +154,9 @@ function vipgoci_auto_approval( $auto_approved_files_arr, 'files_seen' => $files_seen, - ) + ), + 0, + true // Send to IRC ); @@ -287,6 +306,17 @@ function vipgoci_auto_approval( 'repo_name' => $options['repo-name'], + 'pr_number' + => (int) $pr_item->number, + + 'pr_url' => + 'https://github.com/' . + rawurlencode( $options['repo-owner'] ) . + '/' . + rawurlencode( $options['repo-name'] ) . + '/pull/' . + (int) $pr_item->number, + 'commit_id' => $options['commit'], @@ -300,7 +330,9 @@ function vipgoci_auto_approval( $auto_approved_files_arr, 'files_seen' => $files_seen, - ) + ), + 0, + true // Send to IRC ); From c3a3159e38aca80eef087f2af9da70fe1b8c8009 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Thu, 8 Nov 2018 17:32:23 +0000 Subject: [PATCH 119/142] Split auto-approvals into functions. --- auto-approval.php | 674 +++++++++++++++++++++++++--------------------- github-api.php | 9 +- 2 files changed, 369 insertions(+), 314 deletions(-) diff --git a/auto-approval.php b/auto-approval.php index 2abd1baaa..4a0300e93 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -1,5 +1,348 @@ number . ' ' . + 'as it contains ' . + 'files which are not ' . + 'automatically approvable' . + ' -- PR URL: https://github.com/' . + rawurlencode( $options['repo-owner'] ) . + '/' . + rawurlencode( $options['repo-name'] ) . + '/pull/' . + (int) $pr_item->number . ' ', + + array( + 'repo_owner' => + $options['repo-owner'], + + 'repo_name' => + $options['repo-name'], + + 'pr_number' => + $pr_item->number, + + 'autoapprove-filetypes' => + $options['autoapprove-filetypes'], + + 'auto_approved_files_arr' => + $auto_approved_files_arr, + + 'files_seen' => $files_seen, + ), + 0, + true // Send to IRC + ); + + + if ( false === $pr_label ) { + vipgoci_log( + 'Will not attempt to remove label ' . + 'from issue as it does not ' . + 'exist', + array( + 'repo_owner' => $options['repo-owner'], + 'repo_name' => $options['repo-name'], + 'pr_number' => $pr_item->number, + 'label_name' => $options['autoapprove-label'], + ) + ); + } + + else { + /* + * Remove auto-approve label + */ + vipgoci_github_label_remove_from_pr( + $options['repo-owner'], + $options['repo-name'], + $options['token'], + (int) $pr_item->number, + $pr_label->name, + $options['dry-run'] + ); + } + + /* + * Loop through approved PHP and JS files, + * adding comment for each about it + * being approved in the hashes-to-hashes API. + */ + foreach( + $auto_approved_files_arr as + $approved_file => + $approved_file_system + + ) { + + if ( + $approved_file_system !== + 'autoapprove-hashes-to-hashes' + ) { + /* + * If not autoapproved by hashes-to-hashes, + * do not comment on it. Only PHP and JS files + * are auto-approved by hashes-to-hashes. + */ + continue; + } + + $results[ + 'issues' + ][ + (int) $pr_item->number + ] + [] = array( + 'type' => VIPGOCI_STATS_HASHES_API, + 'file_name' => $approved_file, + 'file_line' => 1, + 'issue' => array( + 'message'=> VIPGOCI_FILE_IS_APPROVED_MSG, + + 'source' + => 'WordPressVIPMinimum.' . + 'Info.ApprovedHashesToHashesAPI', + + 'severity' => 1, + 'fixable' => false, + 'type' => 'INFO', + 'line' => 1, + 'column' => 1, + 'level' =>'INFO' + ) + ); + + $results[ + 'stats' + ][ + VIPGOCI_STATS_HASHES_API + ][ + (int) $pr_item->number + ][ + 'info' + ]++; + } + + /* + * Get any approving reviews for the Pull-Request + * submitted by us. Then dismiss them. + */ + + vipgoci_log( + 'Dismissing any approving reviews for ' . + 'the Pull-Request, as it is not ' . + 'approved anymore', + array( + 'pr_number' => $pr_item->number, + ) + ); + + $pr_item_reviews = vipgoci_github_pr_reviews_get( + $options['repo-owner'], + $options['repo-name'], + (int) $pr_item->number, + $options['token'], + array( + 'login' => 'myself', + 'state' => array( 'APPROVED' ), + ) + ); + + /* + * Dismiss any approving reviews. + */ + + foreach( $pr_item_reviews as $pr_item_review ) { + vipgoci_github_pr_review_dismiss( + $options['repo-owner'], + $options['repo-name'], + (int) $pr_item->number, + (int) $pr_item_review->id, + 'Dismissing obsolete review; not approved any longer', + $options['token'] + ); + } + + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'vipgoci_auto_approval_non_approval' ); +} + +/* + * Approve a particular Pull-Request, + * alter label for the PR if needed, + * remove old comments, and log everything + * we do. + */ +function vipgoci_autoapproval_do_approve( + $options, + $pr_item, + $pr_label, + &$auto_approved_files_arr, + $files_seen +) { + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'vipgoci_autoapproval_do_approve' ); + + + vipgoci_log( + ( $options['dry-run'] === true + ? 'Would ' : 'Will ' ) . + 'auto-approve Pull-Request #' . + (int) $pr_item->number . ' ' . + 'as it alters or creates ' . + 'only files that can be ' . + 'automatically approved' . + ' -- PR URL: https://github.com/' . + rawurlencode( $options['repo-owner'] ) . + '/' . + rawurlencode( $options['repo-name'] ) . + '/pull/' . + (int) $pr_item->number . ' ', + + array( + 'repo_owner' + => $options['repo-owner'], + + 'repo_name' + => $options['repo-name'], + + 'pr_number' + => (int) $pr_item->number, + + 'commit_id' + => $options['commit'], + + 'dry_run' + => $options['dry-run'], + + 'autoapprove-filetypes' => + $options['autoapprove-filetypes'], + + 'auto_approved_files_arr' => + $auto_approved_files_arr, + + 'files_seen' => $files_seen, + ), + 0, + true // Send to IRC + ); + + + /* + * Actually approve, if not in dry-mode. + * Also add a label to the Pull-Request + * if applicable. + */ + vipgoci_github_approve_pr( + $options['repo-owner'], + $options['repo-name'], + $options['token'], + $pr_item->number, + $options['commit'], + 'Auto-approved Pull-Request #' . + (int) $pr_item->number . ' as it ' . + 'contains only auto-approvable files' . + '-- either pre-approved or file-types that are ' . + 'auto-approvable (' . + implode( ', ', $options['autoapprove-filetypes'] ) . + ').', + $options['dry-run'] + ); + + + /* + * Add label to Pull-Request, but + * only if it is not associated already. + * If it is already associated, just log + * that fact. + */ + if ( false === $pr_label ) { + vipgoci_github_label_add_to_pr( + $options['repo-owner'], + $options['repo-name'], + $options['token'], + $pr_item->number, + $options['autoapprove-label'], + $options['dry-run'] + ); + } + + else { + vipgoci_log( + 'Will not add label to issue, ' . + 'as it already exists', + array( + 'repo_owner' => + $options['repo-owner'], + 'repo_name' => + $options['repo-name'], + 'pr_number' => + $pr_item->number, + 'label_name' => + $options['autoapprove-label'], + ) + ); + } + + /* + * Remove any comments indicating that a file is + * approved -- we want to get rid of these, as they + * are useless to reviewers at this point. The PR is + * approved anyway. + */ + vipgoci_log( + 'Removing any previously submitted comments ' . + 'indicating that a particular file ' . + 'is approved as the whole ' . + 'Pull-Request is approved', + array( + 'pr_number' => $pr_item->number, + ) + ); + + $pr_comments = vipgoci_github_pr_reviews_comments_get_by_pr( + $options, + $pr_item->number, + array( + 'login' => 'myself', + ) + ); + + foreach( $pr_comments as $pr_comment_item ) { + /* + * If we find the 'approved in hashes-to-hashes ...' + * message, we can safely remove the comment. + */ + if ( false !== strpos( + $pr_comment_item->body, + VIPGOCI_FILE_IS_APPROVED_MSG + ) ) { + vipgoci_github_pr_reviews_comments_delete( + $options, + $pr_comment_item->id + ); + } + } + + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'vipgoci_autoapproval_do_approve' ); +} + /* * Process auto-approval(s) of the Pull-Request(s) * involved with the commit specified. @@ -103,329 +446,52 @@ function vipgoci_auto_approval( vipgoci_log( 'No action taken with Pull-Request #' . (int) $pr_item->number . ' ' . - 'since no files were found', + 'since no files were found' . + ' -- PR URL: https://github.com/' . + rawurlencode( $options['repo-owner'] ) . + '/' . + rawurlencode( $options['repo-name'] ) . + '/pull/' . + (int) $pr_item->number . ' ', + array( 'auto_approved_files_arr' => $auto_approved_files_arr, 'files_seen' => $files_seen, + 'pr_number' => (int) $pr_item->number, 'pr_diff' => $pr_diff, - ) + ), + 0, + true ); } - - /* - * FIXME: Break into functions - */ - else if ( ( true === $did_foreach ) && ( false === $can_auto_approve ) ) { - vipgoci_log( - 'Will not auto-approve Pull-Request #' . - (int) $pr_item->number . ' ' . - 'as it contains ' . "\n\t" . - 'files which are not ' . - 'automatically approvable', - array( - 'repo_owner' => - $options['repo-owner'], - - 'repo_name' => - $options['repo-name'], - - 'pr_number' => - $pr_item->number, - - 'pr_url' => - 'https://github.com/' . - rawurlencode( $options['repo-owner'] ) . - '/' . - rawurlencode( $options['repo-name'] ) . - '/pull/' . - (int) $pr_item->number, - - 'autoapprove-filetypes' => - $options['autoapprove-filetypes'], - - 'auto_approved_files_arr' => - $auto_approved_files_arr, - - 'files_seen' => $files_seen, - ), - 0, - true // Send to IRC - ); - - - if ( false === $pr_label ) { - vipgoci_log( - 'Will not attempt to remove label ' . - 'from issue as it does not ' . - 'exist', - array( - 'repo_owner' => $options['repo-owner'], - 'repo_name' => $options['repo-name'], - 'pr_number' => $pr_item->number, - 'label_name' => $options['autoapprove-label'], - ) - ); - } - - else { - /* - * Remove auto-approve label - */ - vipgoci_github_label_remove_from_pr( - $options['repo-owner'], - $options['repo-name'], - $options['token'], - (int) $pr_item->number, - $pr_label->name, - $options['dry-run'] - ); - } - - /* - * Loop through approved PHP and JS files, - * adding comment for each about it - * being approved in the hashes-to-hashes API. - */ - foreach( - $auto_approved_files_arr as - $approved_file => - $approved_file_system - ) { - if ( - $approved_file_system !== - 'autoapprove-hashes-to-hashes' - ) { - /* - * If not autoapproved by hashes-to-hashes, - * do not comment on it. Only PHP and JS files - * are auto-approved by hashes-to-hashes. - */ - continue; - } - - $results[ - 'issues' - ][ - (int) $pr_item->number - ] - [] = array( - 'type' => VIPGOCI_STATS_HASHES_API, - 'file_name' => $approved_file, - 'file_line' => 1, - 'issue' => array( - 'message'=> VIPGOCI_FILE_IS_APPROVED_MSG, - - 'source' - => 'WordPressVIPMinimum.' . - 'Info.ApprovedHashesToHashesAPI', - - 'severity' => 1, - 'fixable' => false, - 'type' => 'INFO', - 'line' => 1, - 'column' => 1, - 'level' =>'INFO' - ) - ); - - $results[ - 'stats' - ][ - VIPGOCI_STATS_HASHES_API - ][ - (int) $pr_item->number - ][ - 'info' - ]++; - } - - /* - * Get any approving reviews for the Pull-Request - * submitted by us. Then dismiss them. - */ - - vipgoci_log( - 'Dismissing any approving reviews for ' . - 'the Pull-Request, as it is not ' . - 'approved anymore', - array( - 'pr_number' => $pr_item->number, - ) - ); - - $pr_item_reviews = vipgoci_github_pr_reviews_get( - $options['repo-owner'], - $options['repo-name'], - (int) $pr_item->number, - $options['token'], - array( - 'login' => 'myself', - 'state' => array( 'APPROVED' ), - ) + vipgoci_auto_approval_non_approval( + $options, + $results, + $pr_item, + $pr_label, + $auto_approved_files_arr, + $files_seen ); - - /* - * Dismiss any approving reviews. - */ - - foreach( $pr_item_reviews as $pr_item_review ) { - vipgoci_github_pr_review_dismiss( - $options['repo-owner'], - $options['repo-name'], - (int) $pr_item->number, - (int) $pr_item_review->id, - 'Dismissing obsolete review; not approved any longer', - $options['token'] - ); - } } else if ( ( true === $did_foreach ) && ( true === $can_auto_approve ) ) { - vipgoci_log( - ( $options['dry-run'] === true - ? 'Would ' : 'Will ' ) . - 'auto-approve Pull-Request #' . - (int) $pr_item->number . ' ' . - 'as it alters or creates ' . "\n\t" . - 'only files that can be ' . - 'automatically approved', - array( - 'repo_owner' - => $options['repo-owner'], - - 'repo_name' - => $options['repo-name'], - - 'pr_number' - => (int) $pr_item->number, - - 'pr_url' => - 'https://github.com/' . - rawurlencode( $options['repo-owner'] ) . - '/' . - rawurlencode( $options['repo-name'] ) . - '/pull/' . - (int) $pr_item->number, - - 'commit_id' - => $options['commit'], - - 'dry_run' - => $options['dry-run'], - - 'autoapprove-filetypes' => - $options['autoapprove-filetypes'], - - 'auto_approved_files_arr' => - $auto_approved_files_arr, - - 'files_seen' => $files_seen, - ), - 0, - true // Send to IRC - ); - - - /* - * Actually approve, if not in dry-mode. - * Also add a label to the Pull-Request - * if applicable. - */ - vipgoci_github_approve_pr( - $options['repo-owner'], - $options['repo-name'], - $options['token'], - $pr_item->number, - $options['commit'], - $options['autoapprove-filetypes'], - $options['dry-run'] - ); - - - /* - * Add label to Pull-Request, but - * only if it is not associated already. - * If it is already associated, just log - * that fact. - */ - if ( false === $pr_label ) { - vipgoci_github_label_add_to_pr( - $options['repo-owner'], - $options['repo-name'], - $options['token'], - $pr_item->number, - $options['autoapprove-label'], - $options['dry-run'] - ); - } - - else { - vipgoci_log( - 'Will not add label to issue, ' . - 'as it already exists', - array( - 'repo_owner' => - $options['repo-owner'], - 'repo_name' => - $options['repo-name'], - 'pr_number' => - $pr_item->number, - 'label_name' => - $options['autoapprove-label'], - ) - ); - } - - /* - * Remove any comments indicating that a file is - * approved -- we want to get rid of these, as they - * are useless to reviewers at this point. The PR is - * approved anyway. - */ - vipgoci_log( - 'Removing any previously submitted comments ' . - 'indicating that a particular file ' . - 'is approved as the whole ' . - 'Pull-Request is approved', - array( - 'pr_number' => $pr_item->number, - ) - ); - - $pr_comments = vipgoci_github_pr_reviews_comments_get_by_pr( + vipgoci_autoapproval_do_approve( $options, - $pr_item->number, - array( - 'login' => 'myself', - ) + $pr_item, + $pr_label, + $auto_approved_files_arr, + $files_seen ); - - foreach( $pr_comments as $pr_comment_item ) { - /* - * If we find the 'approved in hashes-to-hashes ...' - * message, we can safely remove the comment. - */ - if ( false !== strpos( - $pr_comment_item->body, - VIPGOCI_FILE_IS_APPROVED_MSG - ) ) { - vipgoci_github_pr_reviews_comments_delete( - $options, - $pr_comment_item->id - ); - } - } - } unset( $files_seen ); @@ -439,17 +505,11 @@ function vipgoci_auto_approval( unset( $pr_diff_file_name ); unset( $pr_diff_contents ); unset( $pr_item ); - unset( $pr_comments ); - unset( $pr_comment_item ); unset( $pr_label ); - unset( $pr_item_reviews ); - unset( $pr_item_review ); unset( $prs_implicated ); unset( $files_seen ); unset( $did_foreach ); unset( $can_auto_approve ); - unset( $approved_file ); - unset( $approved_file_system ); gc_collect_cycles(); diff --git a/github-api.php b/github-api.php index ec69fe1ef..294a5abd1 100644 --- a/github-api.php +++ b/github-api.php @@ -2488,7 +2488,7 @@ function vipgoci_github_approve_pr( $github_token, $pr_number, $latest_commit_id, - $filetypes_approve, + $message, $dry_run ) { $github_url = @@ -2507,12 +2507,7 @@ function vipgoci_github_approve_pr( 'comments' => array() ); - $github_postfields['body'] = - 'Auto-approved Pull-Request #' . - (int) $pr_number . ' as it ' . - 'contains only auto-approvable files' . - '-- either pre-approved or file-types that are auto-approvable (' . - '(' . implode( ', ', $filetypes_approve ) . ').'; + $github_postfields['body'] = $message; if ( true === $dry_run ) { return; From 8ba6c0f7c31ae6f419c54e952c1694e2ff76d6bd Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 12 Nov 2018 14:51:04 +0000 Subject: [PATCH 120/142] Logging to IRC when an error occurs in communication with GitHub API. --- github-api.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/github-api.php b/github-api.php index 294a5abd1..ee76f9baf 100644 --- a/github-api.php +++ b/github-api.php @@ -1631,7 +1631,9 @@ function vipgoci_github_pr_comments_error_msg( 'commit_id' => $commit_id, 'pr_number' => $pr_number, 'message' => $message, - ) + ), + 0, + true // Log to IRC as well ); $github_url = From 2d345805fa7023a883f36a9cb8ba1d57ec4dcca4 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 12 Nov 2018 17:02:19 +0000 Subject: [PATCH 121/142] Added a parameter to vipgoci_github_diffs_fetch() to get also files with no patch Previously, files with no patch were ignored. This resulted in them not being taken into account when auto-approving, which could have lead to either false positives or false negatives. This has now been fixed by taking them into account. --- ap-file-types.php | 3 ++- ap-hashes-api.php | 3 ++- ap-svg-files.php | 3 ++- auto-approval.php | 5 +++-- github-api.php | 11 ++++++++--- misc.php | 3 ++- 6 files changed, 19 insertions(+), 9 deletions(-) diff --git a/ap-file-types.php b/ap-file-types.php index 431548b54..286cc2b51 100644 --- a/ap-file-types.php +++ b/ap-file-types.php @@ -50,7 +50,8 @@ function vipgoci_ap_file_types( $options['repo-name'], $options['token'], $pr_item->base->sha, - $options['commit'] + $options['commit'], + true ); diff --git a/ap-hashes-api.php b/ap-hashes-api.php index 7567a3754..3dcd136aa 100644 --- a/ap-hashes-api.php +++ b/ap-hashes-api.php @@ -294,7 +294,8 @@ function vipgoci_ap_hashes_api_scan_commit( $options['repo-name'], $options['token'], $pr_item->base->sha, - $options['commit'] + $options['commit'], + true ); diff --git a/ap-svg-files.php b/ap-svg-files.php index 23d028cc2..c92c4369e 100644 --- a/ap-svg-files.php +++ b/ap-svg-files.php @@ -46,7 +46,8 @@ function vipgoci_ap_svg_files( $options['repo-name'], $options['token'], $pr_item->base->sha, - $options['commit'] + $options['commit'], + true ); diff --git a/auto-approval.php b/auto-approval.php index 4a0300e93..77b620c02 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -258,7 +258,7 @@ function vipgoci_autoapproval_do_approve( 'Auto-approved Pull-Request #' . (int) $pr_item->number . ' as it ' . 'contains only auto-approvable files' . - '-- either pre-approved or file-types that are ' . + '-- either pre-approved files or file-types that are ' . 'auto-approvable (' . implode( ', ', $options['autoapprove-filetypes'] ) . ').', @@ -392,7 +392,8 @@ function vipgoci_auto_approval( $options['repo-name'], $options['token'], $pr_item->base->sha, - $options['commit'] + $options['commit'], + true ); diff --git a/github-api.php b/github-api.php index ee76f9baf..f5fdb5921 100644 --- a/github-api.php +++ b/github-api.php @@ -2762,7 +2762,8 @@ function vipgoci_github_pr_files_changed( $repo_name, $github_token, $pr_base_sha, - $current_commit_id + $current_commit_id, + true ); $files_changed_ret = array(); @@ -2792,7 +2793,8 @@ function vipgoci_github_diffs_fetch( $repo_name, $github_token, $commit_id_a, - $commit_id_b + $commit_id_b, + $empty_patches_also = false ) { /* @@ -2853,7 +2855,10 @@ function vipgoci_github_diffs_fetch( * Loop through all files, save patch in an array */ foreach( $resp_raw->files as $file_item ) { - if ( ! isset( $file_item->patch ) ) { + if ( + ( false === $empty_files_also ) && + ( ! isset( $file_item->patch ) ) + ) { continue; } diff --git a/misc.php b/misc.php index 674900930..2e1011754 100644 --- a/misc.php +++ b/misc.php @@ -109,7 +109,8 @@ function vipgoci_patch_changed_lines( $repo_name, $github_token, $pr_base_sha, - $commit_id + $commit_id, + false ); /* From adb896391fae2926b1abfb8d7b0a88b34d31db8b Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 12 Nov 2018 17:56:57 +0000 Subject: [PATCH 122/142] Fixing comment --- github-api.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github-api.php b/github-api.php index f5fdb5921..91ffb8e2f 100644 --- a/github-api.php +++ b/github-api.php @@ -1187,7 +1187,7 @@ function vipgoci_github_pr_reviews_comments_get( /* * Get all review-comments submitted to a * particular Pull-Request. - * Supports filtering:Will filter the + * Supports filtering by: * - User submitted (parameter: login) * - Comment state (parameter: comments_active, true/false) * From 30526957fe9f547a647fff2a0477428e69bcacb5 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 13 Nov 2018 13:19:03 +0000 Subject: [PATCH 123/142] Clean up stale review at the end of execution --- vip-go-ci.php | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/vip-go-ci.php b/vip-go-ci.php index d70eb107b..461da411e 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -1021,21 +1021,6 @@ function vipgoci_run() { $results ); - if ( true === $options['dismiss-stale-reviews'] ) { - /* - * Dismiss any reviews that contain *only* - * inactive comments -- i.e. comments that - * are obsolete as the code has been changed. - */ - - foreach ( $prs_implicated as $pr_item ) { - vipgoci_github_pr_reviews_dismiss_non_active_comments( - $options, - $pr_item->number - ); - } - } - /* * Clean up old comments made by us previously */ @@ -1187,6 +1172,25 @@ function vipgoci_run() { $options['review-comments-max'] ); + if ( true === $options['dismiss-stale-reviews'] ) { + /* + * Dismiss any reviews that contain *only* + * inactive comments -- i.e. comments that + * are obsolete as the code has been changed. + * + * Note that we do this again here because we might + * just have deleted comments from a Pull-Request which + * would then remain without comments. + */ + + foreach ( $prs_implicated as $pr_item ) { + vipgoci_github_pr_reviews_dismiss_non_active_comments( + $options, + $pr_item->number + ); + } + } + /* * Send out to IRC API any alerts * that are queued up. From 28362dd257c4d1cc65ce58eb0be9f6aeaca2aed2 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 13 Nov 2018 13:19:23 +0000 Subject: [PATCH 124/142] Adding comment, avoiding cache. --- github-api.php | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/github-api.php b/github-api.php index 91ffb8e2f..baa86874f 100644 --- a/github-api.php +++ b/github-api.php @@ -1203,7 +1203,11 @@ function vipgoci_github_pr_reviews_comments_get_by_pr( ) { /* - * Calculate caching ID + * Calculate caching ID. + * + * Note that $filter should be used here and not its + * individual components, to enable bypassing of caching + * by callers. */ $cache_id = array( __FUNCTION__, $options['repo-owner'], $options['repo-name'], @@ -2361,13 +2365,16 @@ function vipgoci_github_pr_reviews_dismiss_non_active_comments( ); /* - * Get all comments to a the current Pull-Request + * Get all comments to a the current Pull-Request. + * + * Note that we must bypass cache here, */ $all_comments = vipgoci_github_pr_reviews_comments_get_by_pr( $options, $pr_number, array( 'login' => 'myself', + 'timestamp' => time() // To bypass caching ) ); @@ -2456,7 +2463,7 @@ function vipgoci_github_pr_reviews_dismiss_non_active_comments( * it should be safe to dismiss the review. */ if ( - ( ! isset( $reviews_status[ $pr_review->id ] ) ) || + ( isset( $reviews_status[ $pr_review->id ] ) ) && ( true === $reviews_status[ $pr_review->id ] ) ) { vipgoci_github_pr_review_dismiss( From 01aa0ef1b3916b991ac9f381c943344515a26061 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 13 Nov 2018 13:20:36 +0000 Subject: [PATCH 125/142] Clarified comment --- github-api.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/github-api.php b/github-api.php index baa86874f..45737c69d 100644 --- a/github-api.php +++ b/github-api.php @@ -1206,8 +1206,8 @@ function vipgoci_github_pr_reviews_comments_get_by_pr( * Calculate caching ID. * * Note that $filter should be used here and not its - * individual components, to enable bypassing of caching - * by callers. + * individual components, to enable new data to be fetched + * (i.e. avoiding of caching by callers). */ $cache_id = array( __FUNCTION__, $options['repo-owner'], $options['repo-name'], From 7546d4847bcb4eda887052e50617f1483b650ac4 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 13 Nov 2018 15:39:47 +0000 Subject: [PATCH 126/142] Fixing undefined property warning --- github-api.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/github-api.php b/github-api.php index 45737c69d..9cb569b52 100644 --- a/github-api.php +++ b/github-api.php @@ -2863,12 +2863,16 @@ function vipgoci_github_diffs_fetch( */ foreach( $resp_raw->files as $file_item ) { if ( - ( false === $empty_files_also ) && + ( false === $empty_patches_also ) && ( ! isset( $file_item->patch ) ) ) { continue; } + if ( ! isset( $file_item->patch ) ) { + $file_item->patch = null; + } + $diffs[ $file_item->filename ] = $file_item->patch; } From 87c3158efeb197e185860787d6949768619974be Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Thu, 15 Nov 2018 12:30:01 +0000 Subject: [PATCH 127/142] Updated comment. --- auto-approval.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/auto-approval.php b/auto-approval.php index 77b620c02..dbcfcdf65 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -353,7 +353,10 @@ function vipgoci_autoapproval_do_approve( * of files, the function will auto-approve them, and else not. * * Note that the --skip-folders argument is ignored - * in this function. + * in this function. This is intentional, because if this + * function did not ignore these folders, it might approve + * Pull-Requests containing files that are actually used + * in production and could contain dangerous code. */ function vipgoci_auto_approval( From 600a1ff4d5a622e1f27e8953b996697d75a33ace Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Thu, 15 Nov 2018 13:54:35 +0000 Subject: [PATCH 128/142] Remove comments indicating approval for currently non-approved files --- auto-approval.php | 54 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/auto-approval.php b/auto-approval.php index dbcfcdf65..ff6bd35d8 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -5,7 +5,7 @@ * remove label if needed, leave messages * on files that are approved, dismiss * any previously approving PRs. - */ + */ function vipgoci_auto_approval_non_approval( $options, @@ -90,7 +90,6 @@ function vipgoci_auto_approval_non_approval( $auto_approved_files_arr as $approved_file => $approved_file_system - ) { if ( @@ -141,6 +140,55 @@ function vipgoci_auto_approval_non_approval( ]++; } + + /* + * Remove any 'file is approved in ...' comments, + * but only for files that are no longer approved. + */ + vipgoci_log( + 'Removing any comments indicating a file is approved' . + 'for files that are not approved anymore', + array( + 'pr_number' => $pr_item->number, + ) + ); + + $pr_comments = vipgoci_github_pr_reviews_comments_get_by_pr( + $options, + $pr_item->number, + array( + 'login' => 'myself', + ) + ); + + foreach( $pr_comments as $pr_comment_item ) { + /* + * Skip approved files. + */ + if ( isset( + $auto_approved_files_arr[ + $pr_comment_item->path + ] + ) ) { + continue; + } + + /* + * If we find the 'approved in hashes-to-hashes ...' + * message, we can safely remove the comment. + */ + if ( false !== strpos( + $pr_comment_item->body, + VIPGOCI_FILE_IS_APPROVED_MSG + ) ) { + vipgoci_github_pr_reviews_comments_delete( + $options, + $pr_comment_item->id + ); + } + } + + /* * Get any approving reviews for the Pull-Request * submitted by us. Then dismiss them. @@ -185,7 +233,7 @@ function vipgoci_auto_approval_non_approval( } /* - * Approve a particular Pull-Request, + * Approve a particular Pull-Request, * alter label for the PR if needed, * remove old comments, and log everything * we do. From 761fef728251645a22fb7ad150ecfac59edfabf2 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Thu, 15 Nov 2018 14:23:25 +0000 Subject: [PATCH 129/142] Renamed variable for more clarity --- misc.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/misc.php b/misc.php index 2e1011754..9da185bc6 100644 --- a/misc.php +++ b/misc.php @@ -871,8 +871,8 @@ function vipgoci_remove_existing_github_comments_from_results( foreach( $results['issues'][ $pr_item->number ] as - $submitted_comment_key => - $submitted_comment + $tobe_submitted_cmt_key => + $tobe_submitted_cmt ) { /* @@ -883,9 +883,9 @@ function vipgoci_remove_existing_github_comments_from_results( if ( // Only do check if everything above is looking good vipgoci_github_comment_match( - $submitted_comment['file_name'], - $submitted_comment['file_line'], - $submitted_comment['issue']['message'], + $tobe_submitted_cmt['file_name'], + $tobe_submitted_cmt['file_line'], + $tobe_submitted_cmt['issue']['message'], $prs_comments ) ) { @@ -893,7 +893,7 @@ function vipgoci_remove_existing_github_comments_from_results( * Keep a record of what we remove. */ $comments_removed[ $pr_item->number ][] = - $submitted_comment; + $tobe_submitted_cmt; /* Remove it */ unset( @@ -902,7 +902,7 @@ function vipgoci_remove_existing_github_comments_from_results( ][ $pr_item->number ][ - $submitted_comment_key + $tobe_submitted_cmt_key ] ); @@ -912,12 +912,12 @@ function vipgoci_remove_existing_github_comments_from_results( $results[ 'stats' ][ - $submitted_comment['type'] + $tobe_submitted_cmt['type'] ][ $pr_item->number ][ strtolower( - $submitted_comment['issue']['type'] + $tobe_submitted_cmt['issue']['type'] ) ]--; } From e5ede4845fbc64a32675d6f7d9575cde76b1e54f Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 16 Nov 2018 14:34:31 +0000 Subject: [PATCH 130/142] Fixing message string. --- auto-approval.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auto-approval.php b/auto-approval.php index ff6bd35d8..3465dc4af 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -146,7 +146,7 @@ function vipgoci_auto_approval_non_approval( * but only for files that are no longer approved. */ vipgoci_log( - 'Removing any comments indicating a file is approved' . + 'Removing any comments indicating a file is approved ' . 'for files that are not approved anymore', array( 'pr_number' => $pr_item->number, From 7a6c48aad3f7d0f72a8805ae42d84b8b6570d370 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 16 Nov 2018 14:36:47 +0000 Subject: [PATCH 131/142] Ignoring 'resurfaced' comments that are part of dismissed reviews --- misc.php | 125 +++++++++++++++++++++++++++++++++++++++++++++++++- vip-go-ci.php | 3 +- 2 files changed, 126 insertions(+), 2 deletions(-) diff --git a/misc.php b/misc.php index 9da185bc6..eefb17fdc 100644 --- a/misc.php +++ b/misc.php @@ -828,8 +828,20 @@ function vipgoci_github_comment_match( function vipgoci_remove_existing_github_comments_from_results( $options, $prs_implicated, - &$results + &$results, + $ignore_dismissed_reviews = false ) { + vipgoci_log( + 'Removing existing GitHub comments from results' . + ' to be posted to GitHub API', + array( + 'repo_owner' => $options['repo-owner'], + 'repo_name' => $options['repo-name'], + 'prs_implicated' => array_keys( $prs_implicated ), + 'ignore_dismissed_reviews' => $ignore_dismissed_reviews, + ) + ); + $comments_removed = array(); foreach ( $prs_implicated as $pr_item ) { @@ -869,6 +881,117 @@ function vipgoci_remove_existing_github_comments_from_results( unset( $pr_item_commit_id ); } + + /* + * Ignore dismissed reviews, if requested. + */ + if ( true === $ignore_dismissed_reviews ) { + /* + * Get dismissed reviews and extract ID of each. + */ + $pr_reviews = vipgoci_github_pr_reviews_get( + $options['repo-owner'], + $options['repo-name'], + $pr_item->number, + $options['token'], + array( + 'login' => 'myself', + 'state' => array( 'DISMISSED' ) + ) + ); + + $dismissed_reviews = array_column( + $pr_reviews, + 'id' + ); + + unset( $pr_reviews ); + + + /* + * Loop through each file to have comments + * submitted against, then look through each + * comment, looking for any comment associated + * with dismissed reviews. + * + * If we find a dismissed review, we will act + * as if the comment was never there by removing + * it from $prs_comments. This will ensure + * that our to-be posted review will contain + * such comments, even though they could be + * considered duplictes. The aim is to make + * them more visible and part of a blocking review. + */ + + $removed_comments = array(); + + foreach( + $prs_comments as + $pr_comment_key => $pr_comments_items + ) { + foreach( + $pr_comments_items as + $pr_review_key => $pr_review_comment + ) { + if ( false === in_array( + $pr_review_comment->pull_request_review_id, + $dismissed_reviews + ) ) { + continue; + } + + $removed_comments[] = array( + 'pr_number' => + $pr_item->number, + + 'pull_request_review_id' => + $pr_review_comment->pull_request_review_id, + + 'comment_id' => + $pr_review_comment->id, + + 'message_body' => + $pr_review_comment->body, + + 'message_created_at' => + $pr_review_comment->created_at, + + 'message_updated_at' => + $pr_review_comment->updated_at, + ); + + + /* + * Comment is a part of a dismissed review, + * get rid of the comment -- act as if was + * never there. + */ + unset( + $prs_comments[ + $pr_comment_key + ][ + $pr_review_key + ] + ); + } + } + + vipgoci_log( + 'Removed following comments from list of previously submitted ' . + 'comments to older PR reviews, as they are ' . + 'part of dismissed reviews', + + array( + 'removed_comments' => + $removed_comments + ) + ); + + unset( $removed_comments ); + unset( $dismissed_reviews ); + } + + foreach( $results['issues'][ $pr_item->number ] as $tobe_submitted_cmt_key => diff --git a/vip-go-ci.php b/vip-go-ci.php index 461da411e..ec7feccaa 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -1130,7 +1130,8 @@ function vipgoci_run() { vipgoci_remove_existing_github_comments_from_results( $options, $prs_implicated, - $results + $results, + true ); /* From d5a802d4b838f0fc2bd721947e604958da8c1a37 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 16 Nov 2018 16:22:12 +0000 Subject: [PATCH 132/142] Whitespacing fixed --- auto-approval.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auto-approval.php b/auto-approval.php index 3465dc4af..ff99ac226 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -303,7 +303,7 @@ function vipgoci_autoapproval_do_approve( $options['token'], $pr_item->number, $options['commit'], - 'Auto-approved Pull-Request #' . + 'Auto-approved Pull-Request #' . (int) $pr_item->number . ' as it ' . 'contains only auto-approvable files' . '-- either pre-approved files or file-types that are ' . From e1174759f993ebe1c4212744586997d992f8fe51 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 16 Nov 2018 16:24:34 +0000 Subject: [PATCH 133/142] Whitespacing fixed. --- github-api.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/github-api.php b/github-api.php index 9cb569b52..8b18bfbdc 100644 --- a/github-api.php +++ b/github-api.php @@ -216,7 +216,7 @@ function vipgoci_oauth1_signature_get_hmac_sha1( /* * Sort the parameters alphabetically. */ - ksort( $parameters_arr_new ); + ksort( $parameters_arr_new ); /* @@ -235,7 +235,7 @@ function vipgoci_oauth1_signature_get_hmac_sha1( $value; $delimiter = '&'; - } + } /* * Then add the temporary 'base string' to the @@ -3017,7 +3017,7 @@ function vipgoci_github_labels_get( */ $cache_id = array( __FUNCTION__, $repo_owner, $repo_name, - $github_token, $pr_number, $label_to_look_for + $github_token, $pr_number, $label_to_look_for ); $cached_data = vipgoci_cache( $cache_id ); From 9ccf5ba5fa9cc7c34a4112c4add93adff0ae1df4 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 16 Nov 2018 16:24:48 +0000 Subject: [PATCH 134/142] Whitespacing fixed. --- latest-release.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/latest-release.php b/latest-release.php index 38573529e..92d81d4c0 100755 --- a/latest-release.php +++ b/latest-release.php @@ -12,10 +12,10 @@ $ch = curl_init(); -curl_setopt( $ch, CURLOPT_URL, $github_url ); -curl_setopt( $ch, CURLOPT_RETURNTRANSFER, 1 ); -curl_setopt( $ch, CURLOPT_CONNECTTIMEOUT, 20 ); -curl_setopt( $ch, CURLOPT_USERAGENT, $client_id ); +curl_setopt( $ch, CURLOPT_URL, $github_url ); +curl_setopt( $ch, CURLOPT_RETURNTRANSFER, 1 ); +curl_setopt( $ch, CURLOPT_CONNECTTIMEOUT, 20 ); +curl_setopt( $ch, CURLOPT_USERAGENT, $client_id ); $resp_data_raw = curl_exec( $ch ); @@ -31,7 +31,7 @@ $resp_data = json_decode( - $resp_data_raw + $resp_data_raw ); unset( $resp_data_raw ); From e69cc16280e6c84e92e0d750ecc31a69f56545cb Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 16 Nov 2018 16:25:12 +0000 Subject: [PATCH 135/142] Whitespacing fixed. --- lint-scan.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lint-scan.php b/lint-scan.php index 79f69b39a..bcbcb255a 100644 --- a/lint-scan.php +++ b/lint-scan.php @@ -218,7 +218,7 @@ function vipgoci_lint_scan_commit( $commit_id, $filename, $options['local-git-repo'] - ); + ); // Save the file-contents in a temporary-file $temp_file_name = vipgoci_save_temp_file( From 32c072296270f9254db9c140548ec1bb03886bb7 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 16 Nov 2018 16:25:21 +0000 Subject: [PATCH 136/142] Whitespacing fixed. --- phpcs-scan.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpcs-scan.php b/phpcs-scan.php index 39c8fd909..d5c424d0f 100644 --- a/phpcs-scan.php +++ b/phpcs-scan.php @@ -269,7 +269,7 @@ function vipgoci_phpcs_scan_commit( $commit_id = $options['commit']; $github_token = $options['token']; - vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'phpcs_scan_commit' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'phpcs_scan_commit' ); vipgoci_log( 'About to PHPCS-scan repository', @@ -700,6 +700,6 @@ function( $item ) { gc_collect_cycles(); - vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'phpcs_scan_commit' ); + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'phpcs_scan_commit' ); } From 45db0fa05f96366a51b21cd334e5016b625b4c91 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 16 Nov 2018 16:26:04 +0000 Subject: [PATCH 137/142] Whitespacing fixed. --- svg-scan.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/svg-scan.php b/svg-scan.php index 54d4ce21e..c9ad10017 100644 --- a/svg-scan.php +++ b/svg-scan.php @@ -18,9 +18,9 @@ function vipgoci_svg_scan_single_file( vipgoci_log( 'Scanning single SVG file', array( - 'repo_owner' => $options['repo-owner'], - 'repo_name' => $options['repo-name'], - 'commit_id' => $options['commit'], + 'repo_owner' => $options['repo-owner'], + 'repo_name' => $options['repo-name'], + 'commit_id' => $options['commit'], 'svg_checks' => $options['svg-checks'], 'file_name' => $file_name, ) @@ -72,9 +72,9 @@ function vipgoci_svg_scan_single_file( vipgoci_log( 'Could not scan file, does not seem to be a SVG file', array( - 'repo_owner' => $options['repo-owner'], - 'repo_name' => $options['repo-name'], - 'commit_id' => $options['commit'], + 'repo_owner' => $options['repo-owner'], + 'repo_name' => $options['repo-name'], + 'commit_id' => $options['commit'], 'svg_checks' => $options['svg-checks'], 'file_name' => $file_name, ) @@ -203,7 +203,7 @@ function vipgoci_svg_scan_single_file( $temp_file_name ] ) - ); + ); vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'svg_scan_single_file' ); From 480c7ce6d6a64b1bb019a0939fe24d286dfe5367 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 16 Nov 2018 16:44:04 +0000 Subject: [PATCH 138/142] Disable auto-approvals while we test the logic. --- auto-approval.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/auto-approval.php b/auto-approval.php index ff99ac226..1bc62ca38 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -52,6 +52,15 @@ function vipgoci_auto_approval_non_approval( true // Send to IRC ); + /* + * Temporary: Just while we test the + * approval-logic. + */ + + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'vipgoci_auto_approval_non_approval' ); + + return; + if ( false === $pr_label ) { vipgoci_log( @@ -248,6 +257,16 @@ function vipgoci_autoapproval_do_approve( vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'vipgoci_autoapproval_do_approve' ); + /* + * Temporary: Just while we test the + * approval-logic. + */ + + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'vipgoci_autoapproval_do_approve' ); + + return; + + vipgoci_log( ( $options['dry-run'] === true ? 'Would ' : 'Will ' ) . From 00915433c7c11f80de6d699d317e3aae5147ec9d Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Fri, 16 Nov 2018 16:46:10 +0000 Subject: [PATCH 139/142] Moving code around. --- auto-approval.php | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/auto-approval.php b/auto-approval.php index 1bc62ca38..f00d80adc 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -256,17 +256,6 @@ function vipgoci_autoapproval_do_approve( ) { vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'vipgoci_autoapproval_do_approve' ); - - /* - * Temporary: Just while we test the - * approval-logic. - */ - - vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'vipgoci_autoapproval_do_approve' ); - - return; - - vipgoci_log( ( $options['dry-run'] === true ? 'Would ' : 'Will ' ) . @@ -310,6 +299,15 @@ function vipgoci_autoapproval_do_approve( true // Send to IRC ); + /* + * Temporary: Just while we test the + * approval-logic. + */ + + vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'vipgoci_autoapproval_do_approve' ); + + return; + /* * Actually approve, if not in dry-mode. From 043d1fe3450edcdf5f884916a815e92d85e04f05 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 19 Nov 2018 18:26:36 +0000 Subject: [PATCH 140/142] Keep statistics of how often we approve --- auto-approval.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/auto-approval.php b/auto-approval.php index f00d80adc..e4fde8014 100644 --- a/auto-approval.php +++ b/auto-approval.php @@ -17,6 +17,12 @@ function vipgoci_auto_approval_non_approval( ) { vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'vipgoci_auto_approval_non_approval' ); + vipgoci_counter_report( + 'do', + 'github_pr_non_approval', + 1 + ); + vipgoci_log( 'Will not auto-approve Pull-Request #' . (int) $pr_item->number . ' ' . @@ -256,6 +262,12 @@ function vipgoci_autoapproval_do_approve( ) { vipgoci_runtime_measure( VIPGOCI_RUNTIME_START, 'vipgoci_autoapproval_do_approve' ); + vipgoci_counter_report( + 'do', + 'github_pr_approval', + 1 + ); + vipgoci_log( ( $options['dry-run'] === true ? 'Would ' : 'Will ' ) . From 79c27cfdce98781c33287e5f20c2e5a80fcf1d61 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 19 Nov 2018 18:36:38 +0000 Subject: [PATCH 141/142] Implement pixel API calls --- other-web-services.php | 75 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 74 insertions(+), 1 deletion(-) diff --git a/other-web-services.php b/other-web-services.php index f02681704..a54a549b9 100644 --- a/other-web-services.php +++ b/other-web-services.php @@ -19,7 +19,7 @@ function vipgoci_irc_api_alert_queue( return $msg_queue_tmp; } - $msg_queue[] = $message; + $msg_queue[] = $message; } /* @@ -117,3 +117,76 @@ function vipgoci_irc_api_alerts_send( time_nanosleep( 0, 500000000 ); } } + +/* + * Send statistics to pixel API so + * we can keep track of actions we + * take during runtime. + */ +function vipgoci_send_stats_to_pixel_api( + $pixel_api_url, + $statistic_group, + $stat_names_to_report, + $statistics +) { + vipgoci_log( + 'Sending statistics to pixel API service', + array( + 'stat_names_to_report' => + $stat_names_to_report + ) + ); + + foreach( + $statistics as + $stat_name => $stat_value + ) { + /* + * We are to report only certain + * values, so skip those who we should + * not report on. + */ + if ( false === in_array( + $stat_name, + $stat_names_to_report + ) ) { + /* + * Not found, so nothing to report, skip. + */ + continue; + } + + /* + * Compose URL. + */ + $url = + $pixel_api_url . + '?' . + 'v=wpcom-no-pv' . + '&' . + 'x_' . rawurlencode( $statistic_group ) . + '/' . + rawurlencode( + $stat_name + ) . '=' . + rawurlencode( + $stat_value + ); + + /* + * Call service, do nothing with output. + */ + file_get_contents( $url ); + + /* + * Sleep a short while between + * requests. + */ + time_nanosleep( + 0, + 500000000 + ); + } +} + + From f8124a1f1db6ea7a4a2efab8f9d731dd16dad24d Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 19 Nov 2018 18:41:02 +0000 Subject: [PATCH 142/142] Add pixel API support. --- vip-go-ci.php | 75 ++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 68 insertions(+), 7 deletions(-) diff --git a/vip-go-ci.php b/vip-go-ci.php index ec7feccaa..311d7cedc 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -370,6 +370,8 @@ function vipgoci_run() { 'irc-api-token:', 'irc-api-bot:', 'irc-api-room:', + 'pixel-api-url:', + 'pixel-api-groupname:', 'php-path:', 'local-git-repo:', 'skip-folders:', @@ -738,7 +740,24 @@ function vipgoci_run() { } unset( $irc_params_defined ); - + + /* + * Handle settings for the pixel API. + */ + if ( isset( $options['pixel-api-url'] ) ) { + vipgoci_option_url_handle( + $options, + 'pixel-api-url', + null + ); + } + + if ( isset( $options['pixel-api-groupname'] ) ) { + $options['pixel-api-groupname'] = trim( + $options['pixel-api-groupname'] + ); + } + /* * Do some sanity-checking on the parameters @@ -1216,6 +1235,49 @@ function vipgoci_run() { $options['token'] ); + /* + * Prepare to send statistics to external service, + * also keep for exit-message. + */ + $counter_report = vipgoci_counter_report( + 'dump', + null, + null + ); + + /* + * Actually send statistics if configured + * to do so. + */ + + if ( + ( ! empty( $options['pixel-api-url'] ) ) && + ( ! empty( $options['pixel-api-groupname' ] ) ) + ) { + vipgoci_send_stats_to_pixel_api( + $options['pixel-api-url'], + $options['pixel-api-groupname'], + + /* + * Which statistics to send. + */ + array( + 'github_pr_approval', + 'github_pr_non_approval', + 'github_api_request_get', + 'github_api_request_post', + 'github_api_request_put', + 'github_api_request_fetch', + 'github_api_request_delete', + ), + $counter_report + ); + } + + + /* + * Final logging before quitting. + */ vipgoci_log( 'Shutting down', array( @@ -1225,12 +1287,8 @@ function vipgoci_run() { VIPGOCI_RUNTIME_DUMP, null ), - 'counters_report' => - vipgoci_counter_report( - 'dump', - null, - null - ), + 'counters_report' => $counter_report, + 'github_api_rate_limit' => $github_api_rate_limit_usage->resources->core, @@ -1239,6 +1297,9 @@ function vipgoci_run() { ); + /* + * Determine exit code. + */ return vipgoci_exit_status( $results );