fix(captcha): stop the CSV info screen from spending the challenge (#1061)

CHANGELOG.md

@@ -19,7 +19,7 @@ The format follows [Keep a Changelog] (https://keepachangelog.com/en/1.1.0/).
 
 - **Um agendamento era criado mas reportado como falha** (#1058): `$appointment['id']` chega do `$wpdb` como string e `AppointmentReceiptHandler::get_receipt_url()` declara `int` sob `strict_types` — o `TypeError` fatalava no e-mail de confirmação, *depois* do commit da reserva, e o visitante recebia HTTP 500 para um agendamento que existia. O `catch` do handler passou a `\Throwable` e o bloco pós-commit foi isolado por etapa: uma reserva commitada nunca mais é reportada como falha.
 - **Toda linha de activity log de agendamento criado era perdida** (#1058): `on_appointment_created()` passava o id do agendamento no 4º argumento de `ActivityLog::log()`, que é `$user_id` — com `fk_ffc_activity_log_user` ativa o MySQL rejeitava o insert.
-
+- **O download público de CSV recusava o captcha que acabara de aceitar** (#1061): o fluxo tem duas requisições — a tela de detalhes e o download — e ambas validavam o mesmo token; com o uso único do #1053 a primeira o queimava, e a segunda recusava a resposta que o visitante tinha acabado de ver aceita. Conferir e gastar viraram operações distintas (`peek` no contrato de captcha): a tela de detalhes confere, o download consome. Um par capturado continua valendo um download só.
 - **O captcha se atrapalhava com dois formulários na mesma página** (#1056): o refresh do agendamento reescrevia a pergunta em *todos* os formulários mas, casando por id, trocava o token só do primeiro — o segundo passava a exibir uma pergunta que seu token não respondia. Agora é escopado ao formulário e casa por `name`. Os ids do captcha passam a ser únicos por render, corrigindo também a associação `<label for>` para leitores de tela.
 
 ## [6.22.0] (2026-09-04) — `aa5ba5b`

includes/core/captcha/class-ffc-challenge-store.php

@@ -73,6 +73,28 @@ public static function redeem( string $proof, int $ttl ): bool {
 		return true;
 	}
 
+	/**
+	 * Whether a proof has already been redeemed.
+	 *
+	 * The read-only half of {@see redeem()}. It exists because a flow can
+	 * legitimately check a challenge on one request and spend it on a later
+	 * one — the public CSV download validates on its info screen and spends
+	 * on the download itself. Without this, that first check would either
+	 * burn the token (and the second request would reject a challenge the
+	 * visitor had just been told was correct) or accept a spent one (and the
+	 * rejection would surface a step too late, saying the same thing).
+	 *
+	 * @param string $proof Signature identifying the challenge.
+	 * @return bool True when the proof is already in the ledger.
+	 */
+	public static function is_spent( string $proof ): bool {
+		if ( '' === $proof ) {
+			return false;
+		}
+
+		return false !== \get_transient( self::key( $proof ) );
+	}
+
 	/**
 	 * Build the transient key for a proof.
 	 *

includes/core/captcha/class-ffc-math-captcha.php

@@ -98,11 +98,39 @@ public static function reset_instances(): void {
 	 * @return true|string
 	 */
 	public function verify( array $request ) {
+		return $this->check( $request, true );
+	}
+
+	/**
+	 * {@inheritDoc}
+	 *
+	 * @param array<string, mixed> $request Request data.
+	 * @return true|string
+	 */
+	public function peek( array $request ) {
+		return $this->check( $request, false );
+	}
+
+	/**
+	 * Shared body of {@see verify()} and {@see peek()}.
+	 *
+	 * @param array<string, mixed> $request Request data.
+	 * @param bool                 $consume Whether to spend the challenge.
+	 * @return true|string
+	 */
+	private function check( array $request, bool $consume ) {
 		if ( ! isset( $request['ffc_captcha_ans'] ) || ! isset( $request['ffc_captcha_hash'] ) ) {
 			return \__( 'Error: Please answer the security question.', 'ffcertificate' );
 		}
 
-		if ( ! SecurityService::verify_simple_captcha( (string) $request['ffc_captcha_ans'], (string) $request['ffc_captcha_hash'] ) ) {
+		$answer = (string) $request['ffc_captcha_ans'];
+		$token  = (string) $request['ffc_captcha_hash'];
+
+		$ok = $consume
+			? SecurityService::verify_simple_captcha( $answer, $token )
+			: SecurityService::peek_simple_captcha( $answer, $token );
+
+		if ( ! $ok ) {
 			return \__( 'Error: The math answer is incorrect.', 'ffcertificate' );
 		}
 

includes/core/captcha/interface-ffc-captcha-provider-interface.php

@@ -52,6 +52,24 @@ public function render_fields(): string;
 	 */
 	public function verify( array $request );
 
+	/**
+	 * Check the challenge in a request payload without spending it.
+	 *
+	 * Every strategy worth having is single-use — a proof-of-work solution is
+	 * replayable until the server records it, exactly as the math token is —
+	 * so a flow that validates on one request and acts on a later one needs
+	 * this everywhere, not just for one provider. The public CSV download is
+	 * that flow: its info screen checks, its download consumes.
+	 *
+	 * Implementations must reject an already-spent challenge here too;
+	 * otherwise the rejection merely moves to the next request.
+	 *
+	 * @since 6.23.0
+	 * @param array<string, mixed> $request Request data (typically `$_POST`).
+	 * @return true|string True when valid, else a translated error message.
+	 */
+	public function peek( array $request );
+
 	/**
 	 * A freshly issued challenge, shaped for a JSON response.
 	 *

includes/core/class-ffc-security-service.php

@@ -175,36 +175,84 @@ private static function payload( string $answer, int $expires, string $nonce ):
 	 * @return bool True if correct, false otherwise
 	 */
 	public static function verify_simple_captcha( string $answer, string $hash ): bool {
+		$proof = self::authenticate_token( $answer, $hash );
+		if ( null === $proof ) {
+			return false;
+		}
+
+		// Burn the token last: an unauthentic or expired proof must not be
+		// able to evict a legitimate one from the ledger.
+		return Captcha\ChallengeStore::redeem( $proof['signature'], $proof['ttl'] );
+	}
+
+	/**
+	 * Check a captcha answer without spending the challenge.
+	 *
+	 * The read-only sibling of {@see verify_simple_captcha()}, for a flow that
+	 * validates on one request and acts on a later one. The public CSV
+	 * download is the case: its info screen checks the answer, and the
+	 * download that follows carries the same token and is what actually
+	 * consumes it. Consuming on the check instead would reject, one screen
+	 * later, a challenge the visitor had just been told was correct.
+	 *
+	 * A challenge already in the ledger fails here too — reporting a spent
+	 * token as valid would only move the contradiction downstream.
+	 *
+	 * @since 6.23.0
+	 * @param string $answer User's answer.
+	 * @param string $hash   Token issued with the challenge.
+	 * @return bool True when the answer is correct and the token is unspent.
+	 */
+	public static function peek_simple_captcha( string $answer, string $hash ): bool {
+		$proof = self::authenticate_token( $answer, $hash );
+
+		return null !== $proof && ! Captcha\ChallengeStore::is_spent( $proof['signature'] );
+	}
+
+	/**
+	 * Authenticate a token against an answer, without touching the ledger.
+	 *
+	 * Everything {@see verify_simple_captcha()} and {@see peek_simple_captcha()}
+	 * agree on: shape, expiry and signature. Redemption is deliberately left
+	 * to the callers, because that is the only thing they differ on.
+	 *
+	 * @param string $answer User's answer.
+	 * @param string $hash   Token issued with the challenge.
+	 * @return array{signature: string, ttl: int}|null Null when the token
+	 *                                                 does not authenticate.
+	 */
+	private static function authenticate_token( string $answer, string $hash ): ?array {
 		// Note: '' === trim() handles both empty and whitespace-only, and — unlike empty() —
 		// does not reject a valid answer of "0" (which can happen for n - n subtraction).
 		$answer = trim( $answer );
 		if ( '' === $answer || '' === $hash ) {
-			return false;
+			return null;
 		}
 
 		$parts = explode( '.', $hash );
 		if ( 3 !== count( $parts )
 			|| 1 !== preg_match( '/^\d+$/', $parts[0] )
 			|| 1 !== preg_match( '/^[0-9a-f]{16}$/', $parts[1] )
 		) {
-			return false;
+			return null;
 		}
 
 		$expires   = (int) $parts[0];
 		$nonce     = $parts[1];
 		$signature = $parts[2];
 
 		if ( $expires <= time() ) {
-			return false;
+			return null;
 		}
 
 		if ( ! Captcha\ChallengeSigner::matches( self::payload( $answer, $expires, $nonce ), $signature ) ) {
-			return false;
+			return null;
 		}
 
-		// Burn the token last: an unauthentic or expired proof must not be
-		// able to evict a legitimate one from the ledger.
-		return Captcha\ChallengeStore::redeem( $signature, $expires - time() );
+		return array(
+			'signature' => $signature,
+			'ttl'       => $expires - time(),
+		);
 	}
 
 	/**
@@ -215,14 +263,45 @@ public static function verify_simple_captcha( string $answer, string $hash ): bo
 	 * @return bool|string True if valid, error message string if invalid
 	 */
 	public static function validate_security_fields( array $data ) {
+		return self::run_security_gate( $data, true );
+	}
+
+	/**
+	 * Run the security gate without spending the challenge.
+	 *
+	 * For the first leg of a multi-request flow: it answers "would this pass?"
+	 * so the visitor is told about a wrong answer immediately, while leaving
+	 * the challenge for the request that actually performs the action. Use
+	 * {@see validate_security_fields()} everywhere else — a single-request
+	 * surface that only peeks never spends the challenge at all, which is the
+	 * replay hole this gate exists to close.
+	 *
+	 * @since 6.23.0
+	 * @param array<string, mixed> $data Form data containing security fields.
+	 * @return bool|string True if valid, error message string if invalid.
+	 */
+	public static function peek_security_fields( array $data ) {
+		return self::run_security_gate( $data, false );
+	}
+
+	/**
+	 * Shared body of the two security gates.
+	 *
+	 * @param array<string, mixed> $data    Form data containing security fields.
+	 * @param bool                 $consume Whether to spend the challenge.
+	 * @return bool|string True if valid, error message string if invalid.
+	 */
+	private static function run_security_gate( array $data, bool $consume ) {
 		// Check honeypot.
 		if ( ! empty( $data['ffc_honeypot_trap'] ) ) {
 			return \__( 'Security Error: Request blocked (Honeypot).', 'ffcertificate' );
 		}
 
 		// The captcha half belongs to whichever strategy is configured; the
 		// honeypot above is provider-independent, which is why it stays here.
-		return Captcha\CaptchaProvider::resolve()->verify( $data );
+		$provider = Captcha\CaptchaProvider::resolve();
+
+		return $consume ? $provider->verify( $data ) : $provider->peek( $data );
 	}
 
 	/**

includes/frontend/class-ffc-public-csv-download.php

@@ -329,7 +329,14 @@ public function handle_request(): void {
 		$form_id     = isset( $_POST['form_id'] ) ? absint( wp_unslash( $_POST['form_id'] ) ) : 0;
 		$posted_hash = RequestInput::get_post_string( 'hash' );
 
-		// 4. Honeypot + CAPTCHA.
+		/*
+		 * 4. Honeypot + CAPTCHA — spent here, on purpose.
+		 *
+		 * This is the no-JS path: one request that both validates and
+		 * streams, so the challenge is consumed by the very request it
+		 * authorizes. The AJAX path splits those across two requests and
+		 * therefore only peeks on the first — see `ajax_info()`.
+		 */
 		$security_check = \FreeFormCertificate\Core\SecurityService::validate_security_fields( $_POST );
 		if ( true !== $security_check ) {
 			if ( $form_id > 0 ) {
@@ -430,8 +437,17 @@ public function ajax_info(): void {
 		$form_id     = isset( $_POST['form_id'] ) ? absint( wp_unslash( $_POST['form_id'] ) ) : 0;
 		$posted_hash = RequestInput::get_post_string( 'hash' );
 
-		// 4. Honeypot + CAPTCHA.
-		$security_check = \FreeFormCertificate\Core\SecurityService::validate_security_fields( $_POST );
+		/*
+		 * 4. Honeypot + CAPTCHA — checked, deliberately NOT spent.
+		 *
+		 * This screen is the first leg of a two-request flow: the download
+		 * button that follows re-posts this same payload to
+		 * `PublicFormsExportSource::authorize_start()`, which is where the
+		 * challenge is consumed. Spending it here would reject, one screen
+		 * later, the very answer the visitor was just told was correct —
+		 * the regression single-use tokens introduced in 6.23.0.
+		 */
+		$security_check = \FreeFormCertificate\Core\SecurityService::peek_security_fields( $_POST );
 		if ( true !== $security_check ) {
 			if ( $form_id > 0 ) {
 				$this->validator->record_download_log_entry( $form_id, 'captcha', '', 'fail_captcha' );

tests/Unit/CaptchaChallengeTest.php

@@ -163,4 +163,45 @@ public function test_redeem_keeps_a_longer_ttl(): void {
 
 		$this->assertSame( 600, $this->last_ttl );
 	}
+
+	// ------------------------------------------------------------------
+	// is_spent() — the read-only half the two-request flows depend on.
+	// ------------------------------------------------------------------
+
+	public function test_is_spent_is_false_before_redemption(): void {
+		$this->assertFalse( ChallengeStore::is_spent( 'proof-a' ) );
+	}
+
+	public function test_is_spent_is_true_after_redemption(): void {
+		ChallengeStore::redeem( 'proof-a', 600 );
+
+		$this->assertTrue( ChallengeStore::is_spent( 'proof-a' ) );
+	}
+
+	public function test_is_spent_does_not_itself_spend_the_proof(): void {
+		// The whole point: asking must not consume. Two questions, then the
+		// redemption still has to be admitted.
+		$this->assertFalse( ChallengeStore::is_spent( 'proof-a' ) );
+		$this->assertFalse( ChallengeStore::is_spent( 'proof-a' ) );
+		$this->assertSame( array(), $this->transients, 'asking must not write to the ledger' );
+
+		$this->assertTrue( ChallengeStore::redeem( 'proof-a', 600 ) );
+	}
+
+	public function test_is_spent_writes_nothing(): void {
+		ChallengeStore::is_spent( 'proof-a' );
+
+		$this->assertSame( array(), $this->transients );
+	}
+
+	public function test_is_spent_tracks_proofs_independently(): void {
+		ChallengeStore::redeem( 'proof-a', 600 );
+
+		$this->assertTrue( ChallengeStore::is_spent( 'proof-a' ) );
+		$this->assertFalse( ChallengeStore::is_spent( 'proof-b' ) );
+	}
+
+	public function test_is_spent_is_false_for_an_empty_proof(): void {
+		$this->assertFalse( ChallengeStore::is_spent( '' ) );
+	}
 }