From de581fc00a22dc16807ae1907968358b647ce2ca Mon Sep 17 00:00:00 2001 From: Juan Lentino <74268474+juanlentino@users.noreply.github.com> Date: Sat, 10 Oct 2026 16:19:47 -0400 Subject: [PATCH 1/2] Do not keep answers for fields hidden by logic Validation already skips a field that logic hides, but its posted value was still stored, and confirmation, notification and action rules read the stored values. The value is now emptied after spam screening (so alltfo_spam_verdict still sees what was posted) and before the entry is stored and the rules run, which is what the alltfo_visible_fields() docblock already promised. Engineered with Claude as a pair programmer. Co-Authored-By: Claude Opus 5.5 --- includes/submission.php | 45 +++++ tests/phpunit/tests/hiddenValues.php | 247 +++++++++++++++++++++++++++ 2 files changed, 292 insertions(+) create mode 100644 tests/phpunit/tests/hiddenValues.php diff --git a/includes/submission.php b/includes/submission.php index c65bf1c..b6c7514 100644 --- a/includes/submission.php +++ b/includes/submission.php @@ -130,6 +130,11 @@ function alltfo_process_submission( $form_id, $request, $files = array() ) { $spam = alltfo_screen_for_spam( $schema, $values, $request ); + // After screening, so a spam check still sees everything that was posted: + // from here on a field hidden by logic holds no answer, which is what the + // entry, the notifications, the confirmations and the actions all read. + $values = alltfo_drop_hidden_values( $schema, $values ); + if ( $is_preview ) { return array( 'success' => true, @@ -258,6 +263,46 @@ function alltfo_sanitize_request( $request, $depth = 0 ) { return $clean; } +/** + * Empties the answer of every field that logic hides for this submission. + * + * Validation already skips a hidden field (see `alltfo_visible_fields()`), but + * its value was still stored, and notification, confirmation and action rules + * read the stored values. A value for a hidden field comes from a browser that + * did not run the form's script (which disables hidden controls), from a field's + * default or pre-fill that was never shown, or from a client that never rendered + * the form. None of those is an answer the visitor gave, so it is replaced with + * the field's own empty value, exactly as if nothing had been posted for it. A + * hidden file field's uploads are deleted, as a refused submission's are. + * + * @since 1.4.0 + * + * @param array $schema The form schema. + * @param array $values Field id => sanitised value. + * @return array The values, hidden fields emptied. + */ +function alltfo_drop_hidden_values( $schema, $values ) { + $visible = alltfo_visible_fields( $schema, $values ); + + foreach ( alltfo_input_fields( $schema ) as $field ) { + $id = $field['id']; + + if ( ! empty( $visible[ $id ] ) || ! array_key_exists( $id, $values ) ) { + continue; + } + + if ( 'file' === $field['type'] ) { + alltfo_delete_upload_attachments( $values[ $id ] ); + unset( $values[ $id ] ); + continue; + } + + $values[ $id ] = alltfo_sanitize_field_value( '', $field ); + } + + return $values; +} + /** * Sanitises every submitted value through its own field type. * diff --git a/tests/phpunit/tests/hiddenValues.php b/tests/phpunit/tests/hiddenValues.php new file mode 100644 index 0000000..e1bfcde --- /dev/null +++ b/tests/phpunit/tests/hiddenValues.php @@ -0,0 +1,247 @@ +form_id = alltfo_test_form( + array( + 'fields' => array( + array( + 'id' => 'intent', + 'type' => 'select', + 'label' => 'What is this about?', + 'choices' => array( + array( + 'label' => 'Press', + 'value' => 'press', + ), + array( + 'label' => 'Something else', + 'value' => 'other', + ), + ), + ), + array( + 'id' => 'outlet', + 'type' => 'text', + 'label' => 'Outlet', + 'logic' => array( + 'enabled' => true, + 'action' => 'show', + 'match' => 'all', + 'rules' => array( + array( + 'field' => 'intent', + 'operator' => 'is', + 'value' => 'press', + ), + ), + ), + ), + array( + 'id' => 'source', + 'type' => 'hidden', + 'label' => 'Source', + 'default' => 'footer', + ), + ), + 'confirmations' => array( + array( + 'id' => 'c-press', + 'type' => 'message', + 'message' => 'Thanks, press desk.', + 'logic' => array( + 'enabled' => true, + 'action' => 'show', + 'match' => 'all', + 'rules' => array( + array( + 'field' => 'outlet', + 'operator' => 'not_empty', + 'value' => '', + ), + ), + ), + ), + array( + 'id' => 'c-default', + 'type' => 'message', + 'message' => 'Thanks, we will be in touch.', + ), + ), + ) + ); + } + + /** + * A signed request that passes the time trap. + * + * @param array $values Field values. + * @return array A request body. + */ + private function request( $values ) { + $issued = time() - 30; + + return array( + 'alltfo_form_id' => $this->form_id, + 'alltfo_nonce' => wp_create_nonce( 'alltfo_submit_' . $this->form_id ), + 'alltfo_t' => $issued, + 'alltfo_ts' => alltfo_sign_timestamp( $this->form_id, $issued ), + 'atf' => $values, + ); + } + + /** + * The stored values of an entry. + * + * @param int $entry_id The entry. + * @return array Field id => value. + */ + private function stored( $entry_id ) { + return json_decode( get_post_meta( $entry_id, ALLTFO_META_VALUES, true ), true ); + } + + /** + * A value for a hidden field is not stored, and its rule does not fire. + * + * The same request a browser without the form's script sends, or a client + * that never rendered the form: the outlet is posted while intent says it is + * not asked. + * + * @covers ::alltfo_drop_hidden_values + * @covers ::alltfo_process_submission + */ + public function test_a_hidden_fields_value_is_not_kept() { + $result = alltfo_process_submission( + $this->form_id, + $this->request( + array( + 'intent' => 'other', + 'outlet' => 'Daily Planet', + ) + ) + ); + + $this->assertTrue( $result['success'] ); + $this->assertSame( '', $this->stored( $result['entry_id'] )['outlet'] ); + $this->assertStringContainsString( 'we will be in touch', $result['confirmation']['message'], 'A rule on a hidden field must not fire on a value nobody was asked for.' ); + } + + /** + * The same field, shown by its condition, keeps its answer. + * + * @covers ::alltfo_drop_hidden_values + */ + public function test_a_shown_fields_value_is_kept() { + $result = alltfo_process_submission( + $this->form_id, + $this->request( + array( + 'intent' => 'press', + 'outlet' => 'Daily Planet', + ) + ) + ); + + $this->assertSame( 'Daily Planet', $this->stored( $result['entry_id'] )['outlet'] ); + $this->assertStringContainsString( 'press desk', $result['confirmation']['message'] ); + } + + /** + * A `hidden` type field is not hidden by logic, so its value is kept. + * + * @covers ::alltfo_drop_hidden_values + */ + public function test_a_hidden_type_field_is_kept() { + $result = alltfo_process_submission( + $this->form_id, + $this->request( + array( + 'intent' => 'other', + 'source' => 'footer', + ) + ) + ); + + $this->assertSame( 'footer', $this->stored( $result['entry_id'] )['source'] ); + } + + /** + * Spam screening still sees what was posted, hidden fields included. + * + * A plugin judging spam through `alltfo_spam_verdict` can only reason about + * an answer to a hidden field if it still receives one. + * + * @covers ::alltfo_screen_for_spam + */ + public function test_spam_screening_sees_the_posted_value() { + $seen = null; + + $capture = function ( $verdict, $schema, $values ) use ( &$seen ) { + $seen = $values['outlet']; + return $verdict; + }; + add_filter( 'alltfo_spam_verdict', $capture, 10, 3 ); + + alltfo_process_submission( + $this->form_id, + $this->request( + array( + 'intent' => 'other', + 'outlet' => 'Daily Planet', + ) + ) + ); + + remove_filter( 'alltfo_spam_verdict', $capture, 10 ); + + $this->assertSame( 'Daily Planet', $seen ); + } + + /** + * An agent submitting through the ability gets the same result. + * + * @covers ::alltfo_ability_submit_form + */ + public function test_submit_form_ability_drops_hidden_values_too() { + wp_set_current_user( self::factory()->user->create( array( 'role' => 'administrator' ) ) ); + alltfo_add_capabilities(); + + $result = wp_get_ability( 'allterrain-forms/submit-form' )->execute( + array( + 'form_id' => $this->form_id, + 'values' => array( + 'intent' => 'other', + 'outlet' => 'Daily Planet', + ), + ) + ); + + $this->assertTrue( $result['success'] ); + $this->assertSame( '', $this->stored( $result['entry_id'] )['outlet'] ); + } +} From 7bc6c4d097308a315f1cc48b2887899030377caa Mon Sep 17 00:00:00 2001 From: Juan Lentino <74268474+juanlentino@users.noreply.github.com> Date: Sat, 10 Oct 2026 16:24:26 -0400 Subject: [PATCH 2/2] Run the hidden-values tests: tag them with the suite's group Co-Authored-By: Claude Opus 5.5 --- tests/phpunit/tests/hiddenValues.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/phpunit/tests/hiddenValues.php b/tests/phpunit/tests/hiddenValues.php index e1bfcde..6d02d47 100644 --- a/tests/phpunit/tests/hiddenValues.php +++ b/tests/phpunit/tests/hiddenValues.php @@ -2,7 +2,8 @@ /** * Values posted for fields that logic hides are not kept. * - * @package AllTerrainForms + * @package AllTerrain_Forms + * @group allterrain-forms */ /** @@ -11,6 +12,8 @@ * Validation has always skipped a hidden field. These pin that the stored entry * and the rules that read it (confirmations, notifications, actions) skip it * too, and that the spam screening still sees what was posted. + * + * @group allterrain-forms */ class ALLTFO_Test_Hidden_Values extends WP_UnitTestCase {