From c117a884d7d83395e3ad958dc9ebdb011120a192 Mon Sep 17 00:00:00 2001
From: Ray Walker <ray@27b.io>
Date: Thu, 11 Jun 2026 21:36:24 +1000
Subject: [PATCH] security: send lock_id via X-CacheKit-Lock-Id header, not
 query string (#24)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

release_lock built the unlock URL as {key}/lock?lock_id={token}. Query strings
leak into access/proxy logs and OpenTelemetry http.url spans (CWE-532) — even
URL-encoded, the capability token is still in the URL and replayable.

Extract release_request() (shared by release_lock + a server-free test), drop
?lock_id= from the URL, and send the token in the X-CacheKit-Lock-Id header.
SaaS dual-reads header + legacy query (deployed in saas 0.1.7), so this is
wire-compatible with prod. Ratified in protocol spec/saas-api.md.

Audited crates/cachekit/src/backend/workers.rs — no lock_id query usage there.
Test asserts the token is in the header and never in the DELETE URL.
---
 .../cachekit/src/backend/cachekitio_lock.rs   | 73 ++++++++++++++++---
 1 file changed, 62 insertions(+), 11 deletions(-)

diff --git a/crates/cachekit/src/backend/cachekitio_lock.rs b/crates/cachekit/src/backend/cachekitio_lock.rs
index b0a3ad1..999cd03 100644
--- a/crates/cachekit/src/backend/cachekitio_lock.rs
+++ b/crates/cachekit/src/backend/cachekitio_lock.rs
@@ -7,6 +7,30 @@ use super::cachekitio::{reqwest_err_sanitized, CachekitIO};
 use super::LockableBackend;
 use crate::error::BackendError;
 
+/// Lock capability token travels in this request header, never the query string
+/// (CWE-532): a `?lock_id=` query leaks the token into access/proxy logs and
+/// OpenTelemetry `http.url` spans. SaaS dual-reads header + legacy query during
+/// rollout, preferring the header. See protocol `spec/saas-api.md`.
+const LOCK_ID_HEADER: &str = "X-CacheKit-Lock-Id";
+
+impl CachekitIO {
+    /// Build the unlock request. Extracted so tests can assert the lock_id rides the
+    /// `X-CacheKit-Lock-Id` header and never appears in the URL (CWE-532).
+    fn release_request(&self, key: &str, lock_id: &str) -> reqwest::RequestBuilder {
+        let url = format!(
+            "{}/v1/cache/{}/lock",
+            self.api_url(),
+            urlencoding::encode(key)
+        );
+        self.with_standard_headers(
+            self.client()
+                .delete(&url)
+                .bearer_auth(self.api_key_str())
+                .header(LOCK_ID_HEADER, lock_id),
+        )
+    }
+}
+
 #[derive(Serialize)]
 #[serde(rename_all = "camelCase")]
 struct LockAcquireRequest {
@@ -64,17 +88,10 @@ impl LockableBackend for CachekitIO {
     }
 
     async fn release_lock(&self, key: &str, lock_id: &str) -> Result<bool, BackendError> {
-        let url = format!(
-            "{}/v1/cache/{}/lock?lock_id={}",
-            self.api_url(),
-            urlencoding::encode(key),
-            urlencoding::encode(lock_id),
-        );
-
-        let req =
-            self.with_standard_headers(self.client().delete(&url).bearer_auth(self.api_key_str()));
-
-        let resp = req
+        // lock_id is a capability token → X-CacheKit-Lock-Id header, not the query string
+        // (CWE-532). See `release_request`.
+        let resp = self
+            .release_request(key, lock_id)
             .send()
             .await
             .map_err(|e| reqwest_err_sanitized(e, self.api_key_str()))?;
@@ -100,4 +117,38 @@ mod tests {
             _assert_lockable(backend);
         }
     }
+
+    #[test]
+    #[allow(clippy::expect_used)] // test-only: a failed build/missing header should panic loudly
+    fn release_lock_sends_token_in_header_not_url() {
+        let backend = CachekitIO::builder()
+            .api_url("https://api.cachekit.io")
+            .api_key("ck_test_key")
+            .build()
+            .expect("builder should succeed for the canonical host");
+
+        let req = backend
+            .release_request("my-key", "lock-secret-123")
+            .build()
+            .expect("request should build");
+
+        // CWE-532: the capability token must never appear in the URL.
+        let url = req.url();
+        assert!(url.query().is_none(), "unexpected query string: {url}");
+        assert!(
+            !url.as_str().contains("lock_id"),
+            "lock_id leaked into URL: {url}"
+        );
+        assert!(
+            !url.as_str().contains("lock-secret-123"),
+            "token leaked into URL: {url}"
+        );
+
+        // ...it rides the X-CacheKit-Lock-Id header under the exact wire name.
+        let header = req
+            .headers()
+            .get("X-CacheKit-Lock-Id")
+            .expect("X-CacheKit-Lock-Id header must be set");
+        assert_eq!(header, "lock-secret-123");
+    }
 }