test(integration): add contention validation tests for scoped cipher

[tobyhede]

Add three integration tests that prove shared mutex contention exists across concurrent proxy connections. Tests measure scaling factor, per-connection latency under concurrency, and cross-connection blocking. Assertions are set to fail under current shared-client architecture and will pass after the per-connection cipher fix.

Updates cipherstash-client 0.33.2 with fixes that make the tests pass.

Summary by CodeRabbit

• Tests
  • Added multitenant performance and contention tests validating concurrent vs. sequential encrypted inserts, per-connection latency under load, and that a slow tenant does not block others. Tests exercise multiple tenant connections and record encrypt+insert timing for diagnostics.
• Chores
  • Bumped client dependency to a newer patch release.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds a new multitenant contention integration test module exercising concurrent encrypted inserts and latency across tenants, registers the module, and bumps the workspace cipherstash-client dependency from 0.33.1 to 0.33.2.

Changes

Cohort / File(s)	Summary
Multitenant Contention Tests packages/cipherstash-proxy-integration/src/multitenant/contention.rs	New integration test module (cfg(test)) with three tests measuring encrypted-insert scaling, per-connection latency under concurrency, and isolation when a slow tenant runs. Uses env tenant keyset IDs, TLS-enabled connections, shared Arc<ZerokmsClient> with internal mutexes, timing helpers, and diagnostic logging.
Module Declaration packages/cipherstash-proxy-integration/src/multitenant/mod.rs	Added mod contention; to include the new test module.
Workspace Dependency Cargo.toml	Bumped workspace dependency cipherstash-client from 0.33.1 to 0.33.2.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• deps: upgrade cipherstash-client to 0.33.1 #371 — Also updates the workspace cipherstash-client version (0.33.1 → 0.33.2), likely related to the same client bump.

Suggested reviewers

• coderdan
• auxesis
• freshtonic

Poem

🐰 I hopped through mutex lanes with glee,
Tenants raced and timed beside me,
One dawdling friend did not delay,
Others hopped on their merry way,
Carrots, tests, and concurrency! 🥕

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and accurately summarizes the main change: adding integration tests for contention validation in a scoped cipher context.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch connection-scoped-cipher

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@packages/cipherstash-proxy-integration/src/contention.rs`:
- Around line 185-186: The 50ms hardcoded delay in the tokio::time::sleep call
is timing-sensitive and can let task A not yet be in pg_sleep when B starts;
replace this fragile sleep with a deterministic synchronization mechanism (e.g.,
a oneshot or Barrier) or, if you prefer a simpler change, increase the delay and
add a clarifying comment explaining why the longer wait is required; locate the
tokio::time::sleep(std::time::Duration::from_millis(50)).await invocation in
contention.rs (the section that sets up connections A and B and expects A to be
in pg_sleep) and update it to wait on an explicit signal from A (or use a larger
duration with a comment) so the test is reliable under CI/cold-TLS conditions.

🧹 Nitpick comments (1)

packages/cipherstash-proxy-integration/src/contention.rs (1)

11-28: Consider using execute() instead of query() for INSERT statements.

Since INSERT doesn't return meaningful rows here, execute() would be more idiomatic and clearer in intent.

♻️ Suggested change

-            client
-                .query(
-                    "INSERT INTO encrypted (id, encrypted_text) VALUES ($1, $2)",
-                    &[&id, &val],
-                )
-                .await
-                .unwrap();
+            client
+                .execute(
+                    "INSERT INTO encrypted (id, encrypted_text) VALUES ($1, $2)",
+                    &[&id, &val],
+                )
+                .await
+                .unwrap();

[freshtonic]

Tests are failing. Given @tobyhede is absent, I will fix up the tests.

[coderabbitai]

🧹 Nitpick comments (1)

packages/cipherstash-proxy-integration/src/multitenant/contention.rs (1)

28-37: Consider using .expect() for clearer test failure messages.

When environment variables are missing, .unwrap() produces a generic panic message. Using .expect() would help developers quickly identify which variable is missing.

💡 Suggested improvement

     fn tenant_keyset_ids(count: usize) -> Vec<String> {
         let keysets = [
-            std::env::var("CS_TENANT_KEYSET_ID_1").unwrap(),
-            std::env::var("CS_TENANT_KEYSET_ID_2").unwrap(),
-            std::env::var("CS_TENANT_KEYSET_ID_3").unwrap(),
+            std::env::var("CS_TENANT_KEYSET_ID_1")
+                .expect("CS_TENANT_KEYSET_ID_1 must be set for multitenant tests"),
+            std::env::var("CS_TENANT_KEYSET_ID_2")
+                .expect("CS_TENANT_KEYSET_ID_2 must be set for multitenant tests"),
+            std::env::var("CS_TENANT_KEYSET_ID_3")
+                .expect("CS_TENANT_KEYSET_ID_3 must be set for multitenant tests"),
         ];

[coderabbitai]

🧹 Nitpick comments (1)

packages/cipherstash-proxy-integration/src/multitenant/contention.rs (1)

39-46: Consider documenting the SET command's non-parameterizable nature.

The SQL string interpolation here is acceptable since keyset_id comes from trusted environment variables, and PostgreSQL's SET command doesn't support parameterized values for custom configuration parameters. A brief comment could clarify this for future readers.

📝 Suggested documentation

 /// Establish a connection and set the keyset for a tenant.
 /// Returns the ready-to-use client (connection setup is excluded from timing).
 async fn connect_as_tenant(keyset_id: &str) -> tokio_postgres::Client {
     let client = connect_with_tls(PROXY).await;
+    // SET commands don't support parameterized values; keyset_id is from trusted env vars
     let sql = format!("SET CIPHERSTASH.KEYSET_ID = '{keyset_id}'");
     client.query(&sql, &[]).await.unwrap();
     client
 }

[coderabbitai]

🧹 Nitpick comments (1)

packages/cipherstash-proxy-integration/src/multitenant/contention.rs (1)

43-44: Consider documenting the SQL string interpolation rationale.

The SET command with string interpolation is acceptable here since keyset_id comes from controlled environment variables. PostgreSQL's SET command doesn't support parameterized queries, so this approach is necessary. A brief inline comment noting this would help future readers understand why parameterization isn't used.