From 0d723a6c3180b2a1bab32c37258d941777a18870 Mon Sep 17 00:00:00 2001 From: prismiwi2015 Date: Tue, 1 Sep 2026 19:29:56 +0200 Subject: [PATCH] A spam/ham correction obeys the form's Akismet switch, not just Akismet's presence The WordPress.org review flagged it: marking an entry as spam or not-spam sent its contents, IP address and user agent to Akismet whenever the Akismet plugin was configured, without consulting the form's own opt-in setting. The check on submission already honoured that switch; the correction did not. `alltfo_akismet_submit_correction()` now looks up the entry's form and returns before touching Akismet unless that form has `spam.akismet` on. The same test, `alltfo_form_uses_akismet()`, is now the single gate both code paths pass through, so no request to Akismet can be made on behalf of a form that never opted in. The function returns whether a correction was sent. Tests exercise both paths against a recording stand-in for Akismet: a form with Akismet off produces zero requests on submission and on either correction, even with an API key configured; a form with it on produces exactly one comment-check, one submit-spam and one submit-ham. readme.txt's External services section now states that corrections obey the per-form switch. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01DVAaHFURDGeBzeuWHZ2dcP --- includes/entries.php | 5 +- includes/spam.php | 40 ++++- readme.txt | 6 +- tests/phpunit/stubs/class-akismet.php | 86 ++++++++++ tests/phpunit/tests/akismet.php | 226 ++++++++++++++++++++++++++ 5 files changed, 355 insertions(+), 8 deletions(-) create mode 100644 tests/phpunit/stubs/class-akismet.php create mode 100644 tests/phpunit/tests/akismet.php diff --git a/includes/entries.php b/includes/entries.php index 182bcaa..aa0c702 100644 --- a/includes/entries.php +++ b/includes/entries.php @@ -221,8 +221,9 @@ function alltfo_prepare_entry( $entry ) { /** * Changes an entry's status. * - * Marking as spam or not-spam also tells Akismet, when it is installed, which is - * what keeps the service learning about this particular site. + * Marking as spam or not-spam also tells Akismet -- but only when the entry's + * form has Akismet switched on. A form that never opted in sends nothing, on + * submission or on correction; see `alltfo_akismet_submit_correction()`. * * @since 0.1.0 * diff --git a/includes/spam.php b/includes/spam.php index d78a751..7605eb3 100644 --- a/includes/spam.php +++ b/includes/spam.php @@ -87,7 +87,7 @@ function alltfo_screen_for_spam( $schema, $values, $request ) { ); } - if ( ! $verdict['spam'] && ! empty( $settings['akismet'] ) && alltfo_akismet_available() ) { + if ( ! $verdict['spam'] && alltfo_form_uses_akismet( $schema ) && alltfo_akismet_available() ) { if ( alltfo_akismet_says_spam( $schema, $values ) ) { $verdict = array( 'spam' => true, @@ -461,6 +461,24 @@ function alltfo_akismet_says_spam( $schema, $values ) { return isset( $response[1] ) && 'true' === trim( $response[1] ); } +/** + * Whether a form has opted in to Akismet. + * + * The single gate every Akismet request passes through. It is the form's own + * `spam.akismet` switch -- off by default -- and nothing else: not whether the + * Akismet plugin happens to be installed, and not a site-wide setting. Data + * leaves the site for this form only because the site owner said so for this + * form. + * + * @since 0.1.0 + * + * @param array $schema The form schema. + * @return bool + */ +function alltfo_form_uses_akismet( $schema ) { + return ! empty( $schema['settings']['spam']['akismet'] ); +} + /** * Tells Akismet it got one wrong. * @@ -468,22 +486,34 @@ function alltfo_akismet_says_spam( $schema, $values ) { * which is what keeps the service learning about this particular site rather * than only about the internet in general. * + * The correction is subject to the same opt-in as the check that preceded it. + * An entry belongs to a form, and unless that form has Akismet switched on the + * entry's contents, IP address and user agent never leave the site -- not on + * submission, and not when somebody clicks "spam" or "not spam" afterwards. The + * form's switch is consulted first, before Akismet is so much as looked for. + * * @since 0.1.0 * * @param int $entry_id The entry. * @param string $verdict `spam` or `ham`. - * @return void + * @return bool Whether a correction was sent. */ function alltfo_akismet_submit_correction( $entry_id, $verdict ) { + $form_id = (int) get_post_meta( $entry_id, ALLTFO_META_FORM, true ); + + if ( ! $form_id || ! alltfo_form_uses_akismet( alltfo_get_form_schema( $form_id ) ) ) { + return false; + } + if ( ! alltfo_akismet_available() ) { - return; + return false; } $context = json_decode( (string) get_post_meta( $entry_id, ALLTFO_META_CONTEXT, true ), true ); $values = json_decode( (string) get_post_meta( $entry_id, ALLTFO_META_VALUES, true ), true ); if ( ! is_array( $context ) || ! is_array( $values ) ) { - return; + return false; } $request = array( @@ -495,4 +525,6 @@ function alltfo_akismet_submit_correction( $entry_id, $verdict ) { ); Akismet::http_post( build_query( $request ), 'spam' === $verdict ? 'submit-spam' : 'submit-ham' ); + + return true; } diff --git a/readme.txt b/readme.txt index 69b1dd8..b7b2935 100644 --- a/readme.txt +++ b/readme.txt @@ -201,8 +201,10 @@ Two optional features send data to a destination **you** choose: configured on your site. Only when you switch it on for a form is each submission to that form (the answers, plus the sender's IP and user agent) sent to Akismet's servers for a spam verdict, with spam/ham corrections you - make in the entries screen reported back. Without the Akismet plugin, or - with the per-form switch off, nothing is sent. See the + make in the entries screen reported back. Corrections obey the same per-form + switch: marking an entry spam or not-spam on a form that has Akismet off + sends nothing. Without the Akismet plugin, or with the per-form switch off, + nothing is ever sent. See the [Akismet Terms of Service](https://akismet.com/tos/) and the [Automattic Privacy Policy](https://automattic.com/privacy/). * **Webhooks** — a form can POST each entry as JSON to a URL you enter, diff --git a/tests/phpunit/stubs/class-akismet.php b/tests/phpunit/stubs/class-akismet.php new file mode 100644 index 0000000..8f832f0 --- /dev/null +++ b/tests/phpunit/stubs/class-akismet.php @@ -0,0 +1,86 @@ + + */ + public static $calls = array(); + + /** + * Back to a blank slate: no key, nothing recorded. + * + * @return void + */ + public static function reset() { + self::$api_key = ''; + self::$body = 'false'; + self::$calls = array(); + } + + /** + * Mirrors `Akismet::get_api_key()`. + * + * @return string + */ + public static function get_api_key() { + return self::$api_key; + } + + /** + * Mirrors `Akismet::http_post()`, recording instead of sending. + * + * @param string $request The URL-encoded request body. + * @param string $path `comment-check`, `submit-spam` or `submit-ham`. + * @param string|null $ip Unused; part of the real signature. + * @return array { 0: array headers, 1: string body } + */ + public static function http_post( $request, $path, $ip = null ) { // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed -- Mirrors the real signature. + $parsed = array(); + parse_str( (string) $request, $parsed ); + + self::$calls[] = array( + 'path' => $path, + 'request' => $parsed, + ); + + return array( array(), self::$body ); + } + } + + // phpcs:enable +} diff --git a/tests/phpunit/tests/akismet.php b/tests/phpunit/tests/akismet.php new file mode 100644 index 0000000..b80f59c --- /dev/null +++ b/tests/phpunit/tests/akismet.php @@ -0,0 +1,226 @@ +user->create( array( 'role' => 'administrator' ) ) ); + } + + /** + * Leaves the stand-in unconfigured for whichever test file runs next. + */ + public function tear_down() { + Akismet::reset(); + + parent::tear_down(); + } + + /** + * A form with one stored entry. + * + * Every other spam check is switched off so that, when Akismet is on, the + * screening reaches it rather than stopping at the honeypot or time trap. + * + * @param bool $akismet Whether the form opts in to Akismet. + * @return array { form_id: int, schema: array, entry_id: int } + */ + private function seed( $akismet ) { + $form_id = alltfo_test_form( + array( + 'fields' => array( + array( + 'id' => 'f1', + 'type' => 'text', + 'label' => 'Name', + ), + array( + 'id' => 'f2', + 'type' => 'email', + 'label' => 'Email', + ), + ), + 'settings' => array( + 'spam' => array( + 'honeypot' => false, + 'timeTrap' => 0, + 'rateLimit' => 0, + 'blocklist' => '', + 'challenge' => false, + 'akismet' => $akismet, + ), + ), + ) + ); + + $schema = alltfo_get_form_schema( $form_id ); + $entry_id = alltfo_store_entry( + $form_id, + $schema, + array( + 'f1' => 'Ada Lovelace', + 'f2' => 'ada@example.com', + ) + ); + + return compact( 'form_id', 'schema', 'entry_id' ); + } + + /** + * The paths of every request made so far. + * + * @return string[] + */ + private function paths() { + return wp_list_pluck( Akismet::$calls, 'path' ); + } + + /** + * The gate reads the form's switch and nothing else. + * + * @covers ::alltfo_form_uses_akismet + */ + public function test_the_gate_is_the_form_switch() { + $this->assertFalse( alltfo_form_uses_akismet( alltfo_default_schema() ), 'A fresh form has Akismet off.' ); + $this->assertFalse( alltfo_form_uses_akismet( array() ), 'A schema with no spam settings at all has Akismet off.' ); + $this->assertTrue( alltfo_form_uses_akismet( $this->seed( true )['schema'] ) ); + $this->assertFalse( alltfo_form_uses_akismet( $this->seed( false )['schema'] ) ); + } + + /** + * Marking an entry spam, then not-spam, on a form that never opted in. + * + * This is the case the directory review flagged: the correction used to + * check only whether Akismet was installed, so a click in the entries + * screen shipped the entry's answers, IP and user agent off-site for a form + * whose owner had never agreed to that. + * + * @covers ::alltfo_akismet_submit_correction + * @covers ::alltfo_set_entry_status + */ + public function test_corrections_send_nothing_when_the_form_has_akismet_off() { + $seeded = $this->seed( false ); + + $this->assertTrue( alltfo_set_entry_status( $seeded['entry_id'], ALLTFO_STATUS_SPAM ) ); + $this->assertSame( ALLTFO_STATUS_SPAM, get_post_status( $seeded['entry_id'] ), 'The status still changes locally.' ); + + $this->assertTrue( alltfo_set_entry_status( $seeded['entry_id'], ALLTFO_STATUS_READ ) ); + $this->assertSame( ALLTFO_STATUS_READ, get_post_status( $seeded['entry_id'] ) ); + + $this->assertSame( array(), Akismet::$calls, 'A form with Akismet off must never contact Akismet, even when the plugin is configured.' ); + $this->assertFalse( alltfo_akismet_submit_correction( $seeded['entry_id'], 'spam' ), 'Called directly, the correction reports that nothing was sent.' ); + $this->assertSame( array(), Akismet::$calls ); + } + + /** + * The same two clicks on a form that did opt in reach Akismet, once each. + * + * @covers ::alltfo_akismet_submit_correction + * @covers ::alltfo_set_entry_status + */ + public function test_corrections_are_sent_when_the_form_has_akismet_on() { + $seeded = $this->seed( true ); + + alltfo_set_entry_status( $seeded['entry_id'], ALLTFO_STATUS_SPAM ); + $this->assertSame( array( 'submit-spam' ), $this->paths() ); + + alltfo_set_entry_status( $seeded['entry_id'], ALLTFO_STATUS_SPAM ); + $this->assertSame( array( 'submit-spam' ), $this->paths(), 'Marking spam twice is not two corrections.' ); + + alltfo_set_entry_status( $seeded['entry_id'], ALLTFO_STATUS_READ ); + $this->assertSame( array( 'submit-spam', 'submit-ham' ), $this->paths() ); + + $this->assertSame( 'contact-form', Akismet::$calls[0]['request']['comment_type'] ); + $this->assertStringContainsString( 'Ada Lovelace', Akismet::$calls[0]['request']['comment_content'] ); + } + + /** + * The correction consults the form before it consults Akismet. + * + * With the form opted in but Akismet unconfigured, nothing is sent either -- + * and a correction for an entry with no form (not an entry at all) is a + * quiet no-op rather than a request. + * + * @covers ::alltfo_akismet_submit_correction + */ + public function test_corrections_need_both_the_form_and_a_configured_akismet() { + $seeded = $this->seed( true ); + + Akismet::$api_key = ''; + + $this->assertFalse( alltfo_akismet_submit_correction( $seeded['entry_id'], 'spam' ) ); + + Akismet::$api_key = 'test-key'; + + $this->assertFalse( alltfo_akismet_submit_correction( self::factory()->post->create(), 'spam' ), 'A post that is not an entry has no form, so no opt-in.' ); + $this->assertSame( array(), Akismet::$calls ); + } + + /** + * Screening a submission respects the same switch. + * + * @covers ::alltfo_screen_for_spam + * @covers ::alltfo_akismet_says_spam + */ + public function test_screening_asks_akismet_only_when_the_form_opts_in() { + $values = array( + 'f1' => 'Ada Lovelace', + 'f2' => 'ada@example.com', + ); + $request = array(); + + $off = $this->seed( false ); + + $verdict = alltfo_screen_for_spam( $off['schema'], $values, $request ); + + $this->assertFalse( $verdict['spam'] ); + $this->assertSame( array(), Akismet::$calls, 'Akismet off: no comment-check request.' ); + + Akismet::$body = 'true'; + + $on = $this->seed( true ); + + $verdict = alltfo_screen_for_spam( $on['schema'], $values, $request ); + + $this->assertSame( array( 'comment-check' ), $this->paths(), 'Akismet on: exactly one comment-check request.' ); + $this->assertTrue( $verdict['spam'] ); + $this->assertSame( 'akismet', $verdict['reason'] ); + $this->assertSame( 'ada@example.com', Akismet::$calls[0]['request']['comment_author_email'] ); + } +}