P2 pre-merge fixes: io_captured column, reconcile copy, harden security test

Fix 1: Add io_captured TINYINT(1) NOT NULL DEFAULT 0 to prv_call_meta CREATE (baked into initial schema — table is new in v0.3.0, no separate migration needed). PRV_Capture_Writer::write_io() sets io_captured=1 via set_io_captured() on success; error rows and legacy rows keep 0. Drawer state now driven by io_captured instead of config_version heuristic: 0 → Legacy/not-captured 1 + io → Normal 1 + no io → Pruned ARCHITECTURE.md schema section updated. Fix 2: Soften reconciliation copy in PRV_Costs_Renderer (~line 75): 'matches the Dashboard MTD total' → 'approximately reconciles to the Dashboard MTD total' — error calls write prv_call_meta (cost 0) but not prv_ai_visibility, so exact equality is not guaranteed. Fix 3: Harden security test (test-capture-writer Test 4): inject sentinel sk-or-TEST-LEAK-CANARY as PRV_OPENROUTER_API_KEY constant; queue a mocked 401 wp_remote_post response; drive PRV_Probe_Runner → PRV_Probe_Run_Executor → probe_one() 401 error path end-to-end; assert the sentinel, Bearer, and Authorization appear in NO stored row (prv_call_meta or prv_call_io). Original direct-writer security test retained as Test 3.
2026-06-15 09:56:11 +00:00
parent 94bef937c7
commit 09dc6db9ce
7 changed files with 116 additions and 25 deletions
--- a/ARCHITECTURE.md
+++ b/ARCHITECTURE.md
@@ -151,6 +151,7 @@ Admin page load ─────────────────────
 | http_status | INT | HTTP status (200=ok, 4xx/5xx=error) |
 | captured_at | DATETIME | UTC |
 | config_version | INT NULL | Config version at run time |
+| io_captured | TINYINT(1) | 1 when write_io() succeeded; 0 for error rows and legacy/pre-feature rows; drives drawer state (0=Legacy, 1+io present=Normal, 1+io absent=Pruned) |

 **Table:** `{prefix}prv_call_io` (pruned after retention window)

--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,9 @@ rendered prompt + raw response. Two new admin pages (Costs, Call Log) with
 a non-modal detail drawer. Raw I/O is pruned on a configurable retention
 window (default 90 days); cost/metadata kept indefinitely.

+### Fixed (pre-launch P2 cleanup)
+- `io_captured TINYINT(1) NOT NULL DEFAULT 0` column added to `prv_call_meta` CREATE (baked into initial schema — table is new in v0.3.0). Set to `1` by `PRV_Capture_Writer::write_io()` on success, remains `0` for error rows and legacy/pre-feature rows. Drawer state now driven by `io_captured` instead of the `config_version < 3` heuristic: `0` = Legacy/not-captured; `1` + io row present = Normal; `1` + io row absent = Pruned (aged out). Removes a fragile inference.
+
 ### Security (P0)
 - `PRV_Gateway_Client::post_to_gateway()` — removed response body from
  RuntimeException message; exception now carries HTTP status code only.
--- a/includes/core/class-prv-call-drawer-renderer.php
+++ b/includes/core/class-prv-call-drawer-renderer.php
@@ -86,21 +86,21 @@ class PRV_Call_Drawer_Renderer {
 		$latency_ms = isset( $meta['latency_ms'] ) ? (int) $meta['latency_ms'] : null;
 		$cost_usd   = (float) ( $meta['cost_usd'] ?? 0.0 );
 		$run_id     = (string) ( $meta['run_id'] ?? '' );
-		$config_ver = isset( $meta['config_version'] ) ? (int) $meta['config_version'] : null;
+		$config_ver  = isset( $meta['config_version'] ) ? (int) $meta['config_version'] : null;
+		$io_captured = isset( $meta['io_captured'] ) ? (int) $meta['io_captured'] : 0;

-		// Determine drawer state.
-		$is_legacy   = ( null === $io && ! $is_error );
-		$is_pruned   = false; // Detection: io null + not legacy = checked by caller.
+		// Determine drawer state using the explicit io_captured flag.
+		// io_captured = 1 AND io row present  → Normal.
+		// io_captured = 1 AND io row absent   → Pruned (aged out by retention cron).
+		// io_captured = 0                     → Legacy / not captured (pre-feature row).
 		$has_io      = ( null !== $io );
 		$vis_row_null = empty( $meta['visibility_row'] );
-		if ( null === $io && ! $is_error ) {
-			// Could be pruned or legacy (pre-v0.3). Distinguish by config_version.
-			if ( null === $config_ver || $config_ver < 3 ) {
-				$is_legacy = true;
-			} else {
-				$is_pruned = true;
-				$is_legacy = false;
-			}
+		if ( ! $is_error ) {
+			$is_legacy = ( 0 === $io_captured );
+			$is_pruned = ( 1 === $io_captured && ! $has_io );
+		} else {
+			$is_legacy = false;
+			$is_pruned = false;
 		}

 		// Header.
--- a/includes/core/class-prv-call-meta-table.php
+++ b/includes/core/class-prv-call-meta-table.php
@@ -68,6 +68,7 @@ class PRV_Call_Meta_Table {
 			http_status    INT(11)             NOT NULL DEFAULT 200,
 			captured_at    DATETIME            NOT NULL,
 			config_version INT(11)             NULL     DEFAULT NULL,
+			io_captured    TINYINT(1)          NOT NULL DEFAULT 0,
 			PRIMARY KEY (id),
 			KEY visibility_row (visibility_row),
 			KEY run_id (run_id),
--- a/includes/core/class-prv-capture-writer.php
+++ b/includes/core/class-prv-capture-writer.php
@@ -15,7 +15,7 @@ declare(strict_types=1);
 *   - NEVER receives $headers, $parsed_args, Authorization, or api keys.
 *   - Allowlist: prompt_text, response_text, model, peptide_slug, intent_label,
 *     tokens_in, tokens_out, cost_usd, latency_ms, cited, http_status,
- *     config_version, visibility_row, run_id, captured_at.
+ *     config_version, visibility_row, run_id, captured_at, io_captured.
 *
 * Capture is BEST-EFFORT: capture_io() is wrapped in a swallowing try/catch
 * by the caller (PRV_Probe_Runner). A write failure must never reach the probe
@@ -38,6 +38,7 @@ class PRV_Capture_Writer {
 	 *
 	 * Called inside the probe loop BEFORE capture_io() so the metadata row
 	 * exists even when the I/O write fails. Returns the inserted row ID.
+	 * io_captured is always 0 here; call set_io_captured() after write_io() succeeds.
 	 *
 	 * Side effects: Database write to prv_call_meta.
 	 *
@@ -72,8 +73,9 @@ class PRV_Capture_Writer {
 				'http_status'    => (int) ( $meta['http_status'] ?? 200 ),
 				'captured_at'    => $now,
 				'config_version' => isset( $meta['config_version'] ) ? (int) $meta['config_version'] : null,
+				'io_captured'    => 0,
 			),
-			array( '%d', '%s', '%s', '%s', '%s', '%d', '%d', '%s', '%d', '%d', '%d', '%s', '%d' )
+			array( '%d', '%s', '%s', '%s', '%s', '%d', '%d', '%s', '%d', '%d', '%d', '%s', '%d', '%d' )
 		);

 		return (int) $wpdb->insert_id;
@@ -85,7 +87,11 @@ class PRV_Capture_Writer {
 	 * Security: only prompt_text and response_text are stored — never
 	 * headers, Authorization, or any raw HTTP request object.
 	 *
-	 * Side effects: Database write to prv_call_io.
+	 * On success, sets io_captured = 1 on the matching prv_call_meta row.
+	 * The meta update is a best-effort follow-up; a failure here is not
+	 * propagated (caller already swallows this entire method).
+	 *
+	 * Side effects: Database writes to prv_call_io and prv_call_meta.
 	 *
 	 * @param int    $call_id       FK to prv_call_meta.id.
 	 * @param string $prompt_text   Rendered plain-text prompt (messages joined).
@@ -111,7 +117,12 @@ class PRV_Capture_Writer {
 			array( '%d', '%s', '%s', '%s' )
 		);

-		return false !== $result;
+		if ( false !== $result ) {
+			$this->set_io_captured( $call_id );
+			return true;
+		}
+
+		return false;
 	}

 	/**
@@ -143,4 +154,31 @@ class PRV_Capture_Writer {

 		return false !== $result;
 	}
+
+	/**
+	 * Mark io_captured = 1 on the prv_call_meta row after a successful I/O write.
+	 *
+	 * Best-effort: called internally from write_io(); failure is non-fatal.
+	 * Legacy/pre-feature rows retain io_captured = 0 (the column default).
+	 *
+	 * Side effects: Database write to prv_call_meta.
+	 *
+	 * @param int $call_id PK of the prv_call_meta row.
+	 *
+	 * @return void
+	 */
+	private function set_io_captured( int $call_id ): void {
+		global $wpdb;
+
+		$table = PRV_Call_Meta_Table::get_table_name();
+
+		// phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching
+		$wpdb->update(
+			$table,
+			array( 'io_captured' => 1 ),
+			array( 'id' => $call_id ),
+			array( '%d' ),
+			array( '%d' )
+		);
+	}
 }
--- a/includes/core/class-prv-costs-renderer.php
+++ b/includes/core/class-prv-costs-renderer.php
@@ -83,7 +83,7 @@ class PRV_Costs_Renderer {
 			echo esc_html(
 				sprintf(
 					/* translators: 1: cost, 2: call count */
-					__( '$%1$s recorded across %2$d calls this month — matches the Dashboard MTD total.', 'pr-vision' ),
+					__( '$%1$s recorded across %2$d calls this month — approximately reconciles to the Dashboard MTD total.', 'pr-vision' ),
 					number_format( $mtd_cost, 4 ),
 					$total_calls
 				)
--- a/tests/unit/test-capture-writer.php
+++ b/tests/unit/test-capture-writer.php
@@ -37,6 +37,7 @@ prv_assert( count( $meta_rows ) === 1, 'write_meta inserts exactly one call_meta
 prv_assert_equals( 'bpc-157', $meta_rows[0]['peptide_slug'], 'write_meta stores peptide_slug' );
 prv_assert_equals( 100, $meta_rows[0]['tokens_in'], 'write_meta stores tokens_in' );
 prv_assert_equals( 250, $meta_rows[0]['tokens_out'], 'write_meta stores tokens_out' );
+prv_assert_equals( 0, $meta_rows[0]['io_captured'], 'write_meta sets io_captured=0 initially' );

 // ── Test 2: write_io writes a row to prv_call_io ─────────────────────────
 $ok = $writer->write_io( $call_id, 'what is BPC-157?', 'BPC-157 is a peptide...' );
@@ -46,9 +47,8 @@ prv_assert( count( $io_rows ) === 1, 'write_io inserts exactly one call_io row'
 prv_assert_equals( 'what is BPC-157?', $io_rows[0]['prompt_text'], 'write_io stores prompt_text' );
 prv_assert_equals( 'BPC-157 is a peptide...', $io_rows[0]['response_text'], 'write_io stores response_text' );

-// ── Test 3 (P0 SECURITY): 401 error record does NOT store key/Bearer ──────
-// Simulate the probe-runner error path: write_meta called with http_status=401
-// and write_io called with the rendered query only (no auth).
+// ── Test 3 (P0 SECURITY — direct writer): 401 error record does NOT store key/Bearer ──
+// Direct-writer path: simulate the probe-runner error path parameters.
 prv_test_reset();
 $writer2 = new PRV_Capture_Writer();
 $writer2->write_meta(
@@ -72,10 +72,8 @@ $writer2->write_io( 1, 'what is TB-500?', '' );
 $err_meta = $GLOBALS['prv_test_state']['wpdb_call_meta_rows'][0];
 $err_io   = $GLOBALS['prv_test_state']['wpdb_call_io_rows'][0];

-// The error metadata must contain http_status=401.
 prv_assert_equals( 401, $err_meta['http_status'], 'Error record stores http_status 401' );

-// The stored data must NOT contain any key-like strings.
 $all_stored = wp_json_encode( $err_meta ) . wp_json_encode( $err_io );
 prv_assert( false === strpos( $all_stored, 'Bearer' ), 'Stored data does not contain "Bearer"' );
 prv_assert( false === strpos( $all_stored, 'sk-or-' ), 'Stored data does not contain "sk-or-"' );
@@ -83,9 +81,59 @@ prv_assert( false === strpos( $all_stored, 'Authorization' ), 'Stored data does
 prv_assert( null === $err_meta['cited'], 'Error record has cited=null' );
 prv_assert_equals( 'what is TB-500?', $err_io['prompt_text'], 'Error record stores rendered query as prompt_text' );

-// ── Test 4: update_meta_cost writes back the cost ────────────────────────
-// No direct state to assert in the stub (update is a no-op returning 1), but
-// confirm the method returns true.
+// ── Test 4 (P0 SECURITY — full executor path): sentinel key must not leak ─
+// Inject a known sentinel as the resolved API key; drive a mocked 401 error
+// path end-to-end through PRV_Probe_Run_Executor (via PRV_Probe_Runner).
+// Assert the sentinel, "Bearer", and "Authorization" appear in NO stored row.
+prv_test_reset();
+
+if ( ! defined( 'PRV_OPENROUTER_API_KEY' ) ) {
+	define( 'PRV_OPENROUTER_API_KEY', 'sk-or-TEST-LEAK-CANARY' );
+}
+
+$GLOBALS['prv_test_state']['options']['prv_monthly_budget_usd']    = 5.0;
+$GLOBALS['prv_test_state']['options']['prv_peptides']               = array( array( 'slug' => 'bpc-157', 'label' => 'BPC-157' ) );
+$GLOBALS['prv_test_state']['options']['prv_prompt_intents']         = array( 'what is {peptide}' );
+$GLOBALS['prv_test_state']['options']['prv_models_v2']              = array(
+	array( 'slug' => 'openai/gpt-4o', 'enabled' => true ),
+);
+$GLOBALS['prv_test_state']['options']['prv_active_config_version']  = 3;
+$GLOBALS['prv_test_state']['wpdb_var']                              = '0.0'; // No prior spend.
+
+// Queue a 401 response for wp_remote_post.
+$GLOBALS['prv_test_state']['remote_posts'][] = array(
+	'response' => array( 'code' => 401 ),
+	'body'     => '{"error":{"message":"Invalid API key","code":401}}',
+);
+
+$sentinel_capture = new PRV_Capture_Writer();
+$sentinel_ledger  = new PRV_Cost_Ledger();
+$sentinel_runner  = new PRV_Probe_Runner( $sentinel_ledger, $sentinel_capture );
+
+try {
+	$sentinel_counts = $sentinel_runner->run();
+} catch ( \Throwable $ex ) {
+	$sentinel_counts = array( 'skipped_error' => -99 );
+}
+
+// The executor must have recorded the 401 error call (skipped_error >= 1).
+prv_assert( isset( $sentinel_counts['skipped_error'] ) && $sentinel_counts['skipped_error'] >= 1, 'Executor recorded a skipped_error for the 401 path' );
+
+// Now verify NO stored row (meta or io) contains the sentinel key or auth material.
+$all_meta = $GLOBALS['prv_test_state']['wpdb_call_meta_rows'];
+$all_io   = $GLOBALS['prv_test_state']['wpdb_call_io_rows'];
+$all_json = wp_json_encode( $all_meta ) . wp_json_encode( $all_io );
+
+prv_assert( false === strpos( $all_json, 'sk-or-TEST-LEAK-CANARY' ), 'Sentinel key does not appear in any stored row (executor path)' );
+prv_assert( false === strpos( $all_json, 'Bearer' ), 'Bearer does not appear in any stored row (executor path)' );
+prv_assert( false === strpos( $all_json, 'Authorization' ), 'Authorization does not appear in any stored row (executor path)' );
+prv_assert( count( $all_meta ) >= 1, 'Executor path wrote at least one call_meta row on 401' );
+
+if ( ! empty( $all_meta ) ) {
+	prv_assert_equals( 401, $all_meta[0]['http_status'], 'Executor 401 path stores http_status=401 in call_meta' );
+}
+
+// ── Test 5: update_meta_cost writes back the cost ────────────────────────
 prv_test_reset();
 $writer3 = new PRV_Capture_Writer();
 $result  = $writer3->update_meta_cost( 5, 0.004567 );