Skip to content

Commit 5991f12

Browse files
committed
test[faustwp]: relax hash_equals count assertion to avoid false positives
Principal-review nit on the source-level regression guard in RestCallbacksTests::test_secret_comparisons_use_constant_time_hash_equals. The previous assertion hardcoded substr_count(rest/callbacks.php, 'hash_equals') === 2, matching the exact number of hash_equals call sites that this PR adds. Any future contributor legitimately adding a third hash_equals call (e.g. for a new permission callback) would trip a false-positive 'security regression' and waste debugging time chasing a non-bug. Switched to assertGreaterThanOrEqual(1, ...). The real revert guards are the two assertStringNotContainsString(...) calls above, which specifically catch the '=== $header_key' and '!== $_SERVER[...]' patterns this PR removes. The substr_count assertion is an affirmative check that hash_equals() is still present somewhere -- a count of >= 1 is sufficient for that purpose.
1 parent 31e10ba commit 5991f12

1 file changed

Lines changed: 8 additions & 3 deletions

File tree

plugins/faustwp/tests/integration/RestCallbacksTests.php

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,14 @@ public function test_secret_comparisons_use_constant_time_hash_equals(): void {
153153
$this->assertStringNotContainsString( "!== \$_SERVER['HTTP_X_FAUST_SECRET']", $graphql_callbacks,
154154
'filter_introspection must use hash_equals(), not !==.' );
155155

156-
// And the positive: both files must contain hash_equals().
157-
$this->assertSame( 2, substr_count( $rest_callbacks, 'hash_equals' ),
158-
'Both rest_authorize_permission_callback and wpac_authorize_permission_callback must use hash_equals.' );
156+
// And the positive: both files must contain hash_equals(). Use >= 1 rather
157+
// than a hardcoded count so a future contributor adding a legitimate third
158+
// hash_equals call (e.g. for a new permission callback) doesn't trip a
159+
// false-positive "security regression". The not-contains assertions above
160+
// are the real revert guards; these are affirmative checks that the timing-
161+
// safe primitive is still present somewhere.
162+
$this->assertGreaterThanOrEqual( 1, substr_count( $rest_callbacks, 'hash_equals' ),
163+
'rest/callbacks.php must contain at least one hash_equals call.' );
159164
$this->assertStringContainsString( 'hash_equals', $graphql_callbacks,
160165
'filter_introspection must use hash_equals.' );
161166
}

0 commit comments

Comments
 (0)