mirror of
https://github.com/lancedb/lancedb.git
synced 2026-09-10 07:12:30 +00:00
fix(secrets): complete the binding and value contract at the Rust boundary
`validate` checked only the binding count, so a caller reaching the low-level entry point past `bind_secrets` could register an environment variable name the runtime cannot deliver, or bind a Secret to a name `runtime.env` already sets -- which would resolve by delivery order, with a value visible in the Function's record and a value that is not. Secret values are bounded here too, at the limit the service enforces, so an oversized credential is refused before a request body is built rather than after it has been serialized and uploaded. Sandbox-reserved names are deliberately still the service's alone: that list belongs to the runtime that owns it, and a copy here would drift from it silently. Both gate reproducers now fail closed with no request reaching the service. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UfmeJ533rQDnPBkMtjerV6
This commit is contained in:
co-authored by
Claude Opus 5
parent
b85f5f141f
commit
e541a3be93
@@ -179,6 +179,35 @@ def test_a_function_binds_at_most_sixteen_secrets():
|
||||
assert state["requests"] == []
|
||||
|
||||
|
||||
def test_binding_names_are_validated_below_the_python_api():
|
||||
"""The low-level entry point reaches the same validator the typed API does.
|
||||
|
||||
Registration envelopes can be hand-rolled past ``bind_secrets``, so the
|
||||
grammar and the disjointness rule live in Rust, above the backend.
|
||||
"""
|
||||
with _mock_remote_function_catalog() as (host, state):
|
||||
db = lancedb.connect(
|
||||
"db://dev",
|
||||
api_key="fake",
|
||||
host_override=host,
|
||||
client_config={"retry_config": {"retries": 0}},
|
||||
)
|
||||
envelope = json.loads(analyze_caption.registration_request.to_canonical_json())
|
||||
envelope["secret_env_bindings"] = {
|
||||
"BAD=NAME": "openai-prod",
|
||||
"TOKEN_0": "secret-0",
|
||||
}
|
||||
envelope["runtime"]["env"]["TOKEN_0"] = "public"
|
||||
|
||||
async def submit_envelope():
|
||||
return await db._conn._inner.create_function_async(json.dumps(envelope))
|
||||
|
||||
with pytest.raises(ValueError, match="portable"):
|
||||
LOOP.run(submit_envelope())
|
||||
|
||||
assert state["requests"] == []
|
||||
|
||||
|
||||
_SECRET_DEBUG_LOG_SOURCE = """
|
||||
import http.server
|
||||
import json
|
||||
|
||||
@@ -657,9 +657,9 @@ impl Connection {
|
||||
/// consumer is a Function that binds the Secret by name. Local databases
|
||||
/// return [`Error::NotSupported`].
|
||||
pub async fn create_secret(&self, name: impl AsRef<str>, value: impl AsRef<str>) -> Result<()> {
|
||||
self.internal
|
||||
.create_secret(name.as_ref(), value.as_ref())
|
||||
.await
|
||||
let value = value.as_ref();
|
||||
crate::function::validate_secret_value(value)?;
|
||||
self.internal.create_secret(name.as_ref(), value).await
|
||||
}
|
||||
|
||||
/// Replace the credential behind an existing Secret.
|
||||
@@ -670,9 +670,9 @@ impl Connection {
|
||||
/// version registered before it. Local databases return
|
||||
/// [`Error::NotSupported`].
|
||||
pub async fn alter_secret(&self, name: impl AsRef<str>, value: impl AsRef<str>) -> Result<()> {
|
||||
self.internal
|
||||
.alter_secret(name.as_ref(), value.as_ref())
|
||||
.await
|
||||
let value = value.as_ref();
|
||||
crate::function::validate_secret_value(value)?;
|
||||
self.internal.alter_secret(name.as_ref(), value).await
|
||||
}
|
||||
|
||||
/// The names of every Secret in this database.
|
||||
|
||||
@@ -499,6 +499,50 @@ pub struct FunctionArtifactRequest {
|
||||
/// the count needs a bound for the same reason a credential needs a size limit.
|
||||
pub const MAX_FUNCTION_SECRET_ENV_BINDINGS: usize = 16;
|
||||
|
||||
/// Largest credential a Secret may hold, matching the limit the service
|
||||
/// enforces. Bounded because the value is destined for a process environment.
|
||||
pub const MAX_SECRET_VALUE_BYTES: usize = 64 * 1024;
|
||||
|
||||
/// Whether `name` is a portable POSIX environment variable name.
|
||||
///
|
||||
/// Leading letter or underscore, then letters, digits, or underscores. Names
|
||||
/// reserved by the execution sandbox are deliberately not checked here: that
|
||||
/// list belongs to the runtime that owns it, and a copy in the client would
|
||||
/// drift from it silently.
|
||||
fn is_portable_env_name(name: &str) -> bool {
|
||||
let mut bytes = name.bytes();
|
||||
bytes
|
||||
.next()
|
||||
.is_some_and(|byte| byte == b'_' || byte.is_ascii_alphabetic())
|
||||
&& bytes.all(|byte| byte == b'_' || byte.is_ascii_alphanumeric())
|
||||
}
|
||||
|
||||
/// Reject a credential the service would refuse on size alone.
|
||||
///
|
||||
/// Checked before the request body is built, so an oversized value is never
|
||||
/// serialized or uploaded.
|
||||
pub(crate) fn validate_secret_value(value: &str) -> Result<()> {
|
||||
if value.is_empty() {
|
||||
return Err(Error::InvalidInput {
|
||||
message: "a Secret value must not be empty".to_string(),
|
||||
});
|
||||
}
|
||||
if value.contains('\0') {
|
||||
return Err(Error::InvalidInput {
|
||||
message: "a Secret value must not contain NUL".to_string(),
|
||||
});
|
||||
}
|
||||
if value.len() > MAX_SECRET_VALUE_BYTES {
|
||||
return Err(Error::InvalidInput {
|
||||
message: format!(
|
||||
"a Secret value is at most {MAX_SECRET_VALUE_BYTES} bytes, not {}",
|
||||
value.len()
|
||||
),
|
||||
});
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Stable request envelope for remote immutable Function registration.
|
||||
///
|
||||
/// Credential values deliberately have no field here. The only secret-shaped
|
||||
@@ -532,6 +576,30 @@ impl FunctionRegistrationRequest {
|
||||
),
|
||||
});
|
||||
}
|
||||
for variable in self.secret_env_bindings.keys() {
|
||||
if !is_portable_env_name(variable) {
|
||||
return Err(Error::InvalidInput {
|
||||
message: format!(
|
||||
"secret_env_bindings key '{variable}' is not a portable \
|
||||
environment variable name"
|
||||
),
|
||||
});
|
||||
}
|
||||
// `env` travels with the Function and is readable wherever its
|
||||
// record is; a bound Secret is not. One name carrying both would
|
||||
// resolve by delivery order, so refuse rather than pick.
|
||||
if self
|
||||
.runtime
|
||||
.env()
|
||||
.is_some_and(|env| env.contains_key(variable))
|
||||
{
|
||||
return Err(Error::InvalidInput {
|
||||
message: format!(
|
||||
"secret_env_bindings key '{variable}' is already set by runtime.env"
|
||||
),
|
||||
});
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,7 +5,9 @@ use std::fs;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use lancedb::Error;
|
||||
use lancedb::function::{FunctionRegistrationRequest, MAX_FUNCTION_SECRET_ENV_BINDINGS};
|
||||
use lancedb::function::{
|
||||
FunctionRegistrationRequest, MAX_FUNCTION_SECRET_ENV_BINDINGS, MAX_SECRET_VALUE_BYTES,
|
||||
};
|
||||
use serde_json::Value;
|
||||
|
||||
fn fixture(name: &str) -> String {
|
||||
@@ -138,3 +140,80 @@ async fn a_function_binds_at_most_sixteen_secrets() {
|
||||
Error::InvalidInput { message } if message.contains("at most 16 secrets")
|
||||
));
|
||||
}
|
||||
|
||||
/// The binding contract is enforced above the backend in full, not just its
|
||||
/// count: a caller that skips a language binding still cannot register a name
|
||||
/// the runtime could not deliver.
|
||||
#[tokio::test]
|
||||
async fn binding_names_are_validated_before_dispatch() {
|
||||
let directory = tempfile::tempdir().unwrap();
|
||||
let connection = lancedb::connect(directory.path().to_str().unwrap())
|
||||
.execute()
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let mut invalid_name = FunctionRegistrationRequest::from_json(&fixture(
|
||||
"remote_function_registration_request.json",
|
||||
))
|
||||
.unwrap();
|
||||
invalid_name.secret_env_bindings = [("BAD=NAME".to_string(), "openai-prod".to_string())].into();
|
||||
let error = connection
|
||||
.create_function_async(invalid_name)
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(matches!(
|
||||
error,
|
||||
Error::InvalidInput { message } if message.contains("portable")
|
||||
));
|
||||
|
||||
// `env` is readable wherever the Function's record is; a bound Secret is
|
||||
// not. The same name cannot mean both.
|
||||
let mut overlapping = FunctionRegistrationRequest::from_json(&fixture(
|
||||
"remote_function_registration_request.json",
|
||||
))
|
||||
.unwrap();
|
||||
let bound = overlapping
|
||||
.runtime
|
||||
.env()
|
||||
.and_then(|env| env.keys().next().cloned())
|
||||
.expect("fixture runtime declares env");
|
||||
overlapping.secret_env_bindings = [(bound.clone(), "openai-prod".to_string())].into();
|
||||
let error = connection
|
||||
.create_function_async(overlapping)
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(matches!(
|
||||
error,
|
||||
Error::InvalidInput { message } if message.contains("already set by runtime.env")
|
||||
));
|
||||
}
|
||||
|
||||
/// An oversized credential is refused before a body is built, so it is never
|
||||
/// serialized or uploaded to be refused by the service instead.
|
||||
#[tokio::test]
|
||||
async fn an_oversized_secret_value_is_refused_before_the_wire() {
|
||||
let directory = tempfile::tempdir().unwrap();
|
||||
let connection = lancedb::connect(directory.path().to_str().unwrap())
|
||||
.execute()
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
for value in ["", &"x".repeat(MAX_SECRET_VALUE_BYTES + 1)] {
|
||||
let error = connection
|
||||
.create_secret("openai-prod", value)
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(
|
||||
matches!(error, Error::InvalidInput { .. }),
|
||||
"expected InvalidInput, got {error:?}"
|
||||
);
|
||||
}
|
||||
|
||||
// A local database refuses the verb outright, which is what proves the
|
||||
// size check ran ahead of the backend rather than instead of it.
|
||||
let error = connection
|
||||
.create_secret("openai-prod", "x".repeat(MAX_SECRET_VALUE_BYTES))
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(matches!(error, Error::NotSupported { .. }));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user