fix(secrets): keep credentials out of the client's own debug log

`log_request` logs any JSON body verbatim at debug, and Python and Node both
wire that logger to `LANCEDB_LOG`. `create_secret` posts the value in its body,
so ordinary SDK debug logging wrote the credential to application logs. The
comment on `write_secret` reasoned correctly about proxy traces and access logs
and missed the logger in this process.

Redaction cannot live in the value model: the logger sees the serialized body,
where the credential is already plaintext bytes. So bodies are suppressed for
the route instead, matched on the `secrets` path segment rather than a
versioned prefix, so a verb added under that namespace later is covered without
an edit here.

The same debug line prints the request's Debug, which prints headers, so the
API key was in every debug line of every request regardless of route. Marking
the header value sensitive is what stops that.

The end-to-end regression fails without this: the log carried
`,"value":"SECRET_VALUE_SENTINEL"}` verbatim.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UfmeJ533rQDnPBkMtjerV6
This commit is contained in:
Jonathan M Hsieh
2026-09-09 16:59:40 +00:00
co-authored by Claude Opus 5
parent 4d09f8ce26
commit b85f5f141f
2 changed files with 138 additions and 7 deletions
@@ -11,6 +11,7 @@ import types
from datetime import date
import http.server
import json
import os
from pathlib import Path
import subprocess
import sys
@@ -178,6 +179,67 @@ def test_a_function_binds_at_most_sixteen_secrets():
assert state["requests"] == []
_SECRET_DEBUG_LOG_SOURCE = """
import http.server
import json
import threading
import lancedb
class Handler(http.server.BaseHTTPRequestHandler):
def log_message(self, *args):
pass
def do_POST(self):
self.rfile.read(int(self.headers.get("Content-Length", "0")))
payload = json.dumps({}).encode()
self.send_response(200)
self.send_header("Content-Type", "application/json")
self.send_header("Content-Length", str(len(payload)))
self.end_headers()
self.wfile.write(payload)
server = http.server.ThreadingHTTPServer(("localhost", 0), Handler)
threading.Thread(target=server.serve_forever, daemon=True).start()
try:
db = lancedb.connect(
"db://dev",
api_key="API_KEY_SENTINEL",
host_override="http://localhost:%d" % server.server_address[1],
client_config={"retry_config": {"retries": 0}},
)
db.create_secret("openai-prod", "SECRET_VALUE_SENTINEL")
finally:
server.shutdown()
"""
def test_a_credential_never_reaches_a_debug_log(tmp_path):
"""The logger sees the serialized body, so no value-side redaction reaches it.
Runs in a subprocess because the Rust logger reads ``LANCEDB_LOG`` once, at
import.
"""
script = tmp_path / "write_secret.py"
script.write_text(_SECRET_DEBUG_LOG_SOURCE)
result = subprocess.run(
[sys.executable, str(script)],
check=True,
capture_output=True,
text=True,
env={**os.environ, "LANCEDB_LOG": "debug"},
)
output = result.stdout + result.stderr
# Without this the test passes when debug logging is simply off.
assert "Sending request_id=" in output, output
assert "SECRET_VALUE_SENTINEL" not in output
assert "API_KEY_SENTINEL" not in output
def test_a_credential_value_is_rejected_in_the_binding_position():
"""The one mistake the typed binding exists to stop."""
with pytest.raises(TypeError, match="EnvVarSecret"):
+76 -7
View File
@@ -404,6 +404,18 @@ fn validate_dns_hostname(hostname: &str) -> Result<()> {
Ok(())
}
/// Whether a route's request body is a credential rather than a description of
/// one.
///
/// Matched on the path segment rather than a versioned prefix, so a `/v2/` bump
/// or a route added under the namespace later is covered without anyone
/// remembering to extend this. Every secrets route is denied, not only the two
/// that carry a value: their bodies hold names and page tokens, which are worth
/// nothing in a debug log next to the risk of a new verb landing here unnoticed.
pub(crate) fn route_carries_credential(path: &str) -> bool {
path.split('/').any(|segment| segment == "secrets")
}
impl RestfulLanceDbClient<Sender> {
fn get_timeout(passed: Option<Duration>, env_var: &str) -> Result<Option<Duration>> {
if let Some(passed) = passed {
@@ -610,12 +622,14 @@ impl<S: HttpSend> RestfulLanceDbClient<S> {
) -> Result<HeaderMap> {
let mut headers = HeaderMap::new();
if !api_key.is_empty() {
headers.insert(
HeaderName::from_static("x-api-key"),
HeaderValue::from_str(api_key).map_err(|_| Error::InvalidInput {
message: "non-ascii api key provided".to_string(),
})?,
);
// `log_request` prints the request's Debug, which prints headers.
// Marking the value sensitive is what makes that print `Sensitive`
// instead of the key itself.
let mut key = HeaderValue::from_str(api_key).map_err(|_| Error::InvalidInput {
message: "non-ascii api key provided".to_string(),
})?;
key.set_sensitive(true);
headers.insert(HeaderName::from_static("x-api-key"), key);
}
if region == "local" {
let host = format!("{}.local.api.lancedb.com", db_name);
@@ -845,7 +859,12 @@ impl<S: HttpSend> RestfulLanceDbClient<S> {
.headers()
.get("content-type")
.map(|v| v.to_str().unwrap());
if content_type == Some("application/json") {
if route_carries_credential(request.url().path()) {
debug!(
"Sending request_id={}: {:?} with body suppressed",
request_id, request
);
} else if content_type == Some("application/json") {
let body = request.body().as_ref().unwrap().as_bytes().unwrap();
let body = String::from_utf8_lossy(body);
debug!(
@@ -1192,6 +1211,56 @@ mod tests {
assert_eq!(headers.get("x-api-key").unwrap(), "api-key");
}
/// `log_request` prints the request's Debug, and Debug for a request prints
/// its headers. Marking the value sensitive is the only thing standing
/// between the API key and every debug line; assert on the header map's own
/// Debug, which is what that printing reduces to.
#[test]
fn test_api_key_is_redacted_in_debug_output() {
let headers = RestfulLanceDbClient::<Sender>::default_headers(
"sk-live-sentinel",
"us-east-1",
"db-name",
false,
&RemoteOptions::default(),
None,
&ClientConfig::default(),
)
.unwrap();
assert_eq!(headers.get("x-api-key").unwrap(), "sk-live-sentinel");
assert!(
!format!("{:?}", headers).contains("sk-live-sentinel"),
"the API key must not survive Debug formatting"
);
}
/// Denial follows the path segment, so a verb that does not exist yet and a
/// future API version are both covered without an edit here.
#[test]
fn test_secrets_routes_never_log_a_body() {
for route in [
"/v1/secrets/create",
"/v1/secrets/alter",
"/v1/secrets/list",
"/v1/secrets/drop",
"/v1/secrets/describe",
"/v2/secrets/rotate",
] {
assert!(
route_carries_credential(route),
"{route} must never log a body"
);
}
for route in [
"/v1/functions/create",
"/v1/table/foo/query",
"/v1/jobs/list",
] {
assert!(!route_carries_credential(route), "{route} is not a secret");
}
}
#[test]
fn test_rejects_invalid_cloud_dns_hostname() {
let invalid_database_names = ["a".repeat(64), "invalid..database".to_string()];