rust/lancedb
1
0
Fork 0
mirror of https://github.com/lancedb/lancedb.git synced 2026-09-12 00:02:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(secrets): give Secrets their own module, leave validation to the service

Browse Source 
Three things from review.

`SecretInfo` moves to `lancedb::secrets`. It was in `database` because that is
where the trait lives, not because it belongs to a database's shape; a new
object should land in its own module and the Secret verbs will follow it.

Binding and value validation come out of the client. The service owns those
rules -- it owns the runtime the names land in and the store the values go to --
and a copy here could disagree with it without either side noticing. What the
client was checking, the service already rejects: the per-Function cap, the
environment-variable grammar, disjointness from `runtime.env`, and the value
size. The cost is a round trip before the error, and the error is the service's
own words rather than a paraphrase that can drift.

The tests follow the rule rather than the check: what the client guarantees is
that the envelope it sends is the envelope the caller wrote, so that is what is
asserted now.

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-11 17:45:20 +00:00
co-authored by Claude Opus 5
parent b9c0446421
commit 762eb0f44f
8 changed files with 50 additions and 275 deletions
Download Patch File Download Diff File Expand all files Collapse all files
python/python/tests/test_first_class_function_slice2.py
+14 -51
View File
@@ -145,45 +145,13 @@ def test_bindings_may_not_collide_with_plain_configuration():
)
def test_a_function_binds_at_most_sixteen_secrets():
"""The cap lives in Rust, so no language surface can be talked past it.
def test_a_binding_envelope_reaches_the_service_for_it_to_judge():
"""Binding rules are the service's: it owns the runtime the names land in.
Registering through the typed API and hand-rolling the request envelope
reach the same boundary, and neither reaches the wire.
"""
bindings = [
EnvVarSecret(secret=f"secret-{index}", env_variable=f"TOKEN_{index}")
for index in range(17)
]
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}},
)
with pytest.raises(ValueError, match="at most 16 secrets"):
db.create_function(normalize_score, secrets=bindings)
envelope = json.loads(normalize_score.registration_request.to_canonical_json())
envelope["secret_env_bindings"] = {
f"TOKEN_{index}": f"secret-{index}" for index in range(17)
}
async def submit_envelope():
return await db._conn._inner.create_function_async(json.dumps(envelope))
with pytest.raises(ValueError, match="at most 16 secrets"):
LOOP.run(submit_envelope())
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.
The client sends what it was given, so a rule it duplicated could disagree
with the service's without either side noticing. What is checked here is
that the envelope arrives intact -- the shape the service judges is the
shape the caller wrote.
"""
with _mock_remote_function_catalog() as (host, state):
db = lancedb.connect(
@@ -192,20 +160,15 @@ def test_binding_names_are_validated_below_the_python_api():
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"
bindings = [
EnvVarSecret(secret=f"secret-{index}", env_variable=f"TOKEN_{index}")
for index in range(17)
]
db.create_function(normalize_score, secrets=bindings)
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"] == []
sent = state["requests"][0][1]
assert len(sent["secret_env_bindings"]) == 17
assert sent["secret_env_bindings"]["TOKEN_0"] == "secret-0"
_SECRET_DEBUG_LOG_SOURCE = """
rust/lancedb/src/connection.rs
+8 -11
View File
@@ -24,7 +24,7 @@ use crate::data::scannable::Scannable;
use crate::database::listing::ListingDatabase;
use crate::database::{
CloneTableRequest, Database, DatabaseOptions, JobInfo, OpenTableRequest, ReadConsistency,
SecretInfo, TableNamesRequest,
TableNamesRequest,
};
use crate::embeddings::{EmbeddingRegistry, MemoryRegistry};
use crate::error::{Error, Result};
@@ -36,6 +36,7 @@ use crate::remote::{
OPT_REMOTE_SQL_HOST_OVERRIDE,
},
};
use crate::secrets::SecretInfo;
use lance::io::ObjectStoreParams;
pub use lance_file::version::LanceFileVersion;
#[cfg(feature = "remote")]
@@ -587,14 +588,10 @@ impl Connection {
/// returned typed job yields the durable [`crate::function::FunctionVersion`].
/// Local databases return [`Error::NotSupported`].
///
/// The request's binding shape is validated here rather than in any one
/// language binding, so every client surface rejects the same envelopes
/// before one reaches the wire.
pub async fn create_function_async(
&self,
request: crate::function::FunctionRegistrationRequest,
) -> Result<crate::job::Job<crate::function::FunctionVersion>> {
request.validate()?;
self.internal.create_function_async(request).await
}
@@ -657,9 +654,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<()> {
let value = value.as_ref();
crate::function::validate_secret_value(value)?;
self.internal.create_secret(name.as_ref(), value).await
self.internal
.create_secret(name.as_ref(), value.as_ref())
.await
}
/// Replace the credential behind an existing Secret.
@@ -670,9 +667,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<()> {
let value = value.as_ref();
crate::function::validate_secret_value(value)?;
self.internal.alter_secret(name.as_ref(), value).await
self.internal
.alter_secret(name.as_ref(), value.as_ref())
.await
}
/// The names of every Secret in this database.
rust/lancedb/src/database.rs
+1 -14
View File
@@ -28,6 +28,7 @@ use lance_namespace::models::{
use crate::data::scannable::Scannable;
use crate::error::Result;
use crate::secrets::SecretInfo;
use crate::table::{BaseTable, WriteOptions};
pub mod listing;
@@ -258,20 +259,6 @@ fn secret_catalog_not_supported<T>() -> Result<T> {
/// The `Database` trait defines the interface for database implementations.
///
/// A database is responsible for managing tables and their metadata.
/// What a database records about a Secret. Never its value.
///
/// Returned by [`crate::connection::Connection::describe_secret`]. There is no
/// field for the credential and no method that could produce one.
#[derive(Debug, Clone, PartialEq, Eq, serde::Deserialize)]
pub struct SecretInfo {
/// The Secret's database-scoped name.
pub name: String,
/// When the Secret was created, as an RFC 3339 timestamp.
pub created_at: String,
/// When the Secret's value was last rotated, as an RFC 3339 timestamp.
pub updated_at: String,
}
#[async_trait::async_trait]
pub trait Database:
Send + Sync + std::any::Any + std::fmt::Debug + std::fmt::Display + 'static
rust/lancedb/src/function.rs
-93
View File
@@ -493,56 +493,6 @@ pub struct FunctionArtifactRequest {
pub adapter: PythonAdapterSpec,
}
/// A Function binds at most this many Secrets to environment variables.
///
/// Each bound Secret is one extra read on the launch path of every fragment, so
/// 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
@@ -561,49 +511,6 @@ pub struct FunctionRegistrationRequest {
pub secret_env_bindings: BTreeMap<String, String>,
}
impl FunctionRegistrationRequest {
/// Reject a registration whose bindings exceed what a launch can deliver.
///
/// Shape only, and deliberately not a check that each bound Secret exists:
/// that is the service's answer, and it is asked for the first time when a
/// column is declared against the registered version.
pub fn validate(&self) -> Result<()> {
if self.secret_env_bindings.len() > MAX_FUNCTION_SECRET_ENV_BINDINGS {
return Err(Error::InvalidInput {
message: format!(
"a Function binds at most {MAX_FUNCTION_SECRET_ENV_BINDINGS} secrets, not {}",
self.secret_env_bindings.len()
),
});
}
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(())
}
}
impl_json!(FunctionRegistrationRequest);
/// Exact FunctionVersion reference embedded in applications and bindings.
rust/lancedb/src/lib.rs
+1
View File
@@ -195,6 +195,7 @@ pub mod query;
#[cfg(feature = "remote")]
pub mod remote;
pub mod rerankers;
pub mod secrets;
pub mod sql;
pub mod table;
#[cfg(test)]
rust/lancedb/src/remote/db.rs
+2 -1
View File
@@ -21,13 +21,14 @@ use lance_namespace::models::{
use crate::Error;
use crate::database::{
CloneTableRequest, CreateTableMode, CreateTableRequest, Database, DatabaseOptions, JobInfo,
OpenTableRequest, ReadConsistency, SecretInfo, TableNamesRequest,
OpenTableRequest, ReadConsistency, TableNamesRequest,
};
use crate::error::Result;
use crate::function::{FunctionRegistrationRequest, FunctionVersion};
use crate::job::Job;
use crate::remote::job::{RemoteJob, job_state_to_client};
use crate::remote::util::stream_as_body;
use crate::secrets::SecretInfo;
use crate::table::BaseTable;
use super::client::{
rust/lancedb/src/secrets.rs
+23
View File
@@ -0,0 +1,23 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: Copyright The LanceDB Authors
//! Named Secrets: database-scoped credentials a Function binds by name.
//!
//! Nothing here holds a credential. The verbs live on
//! [`crate::connection::Connection`], and none of them returns a value -- by
//! construction rather than by policy, so there is no code path that could.
//! What a Function records is a binding, in [`crate::function`].
/// What a database records about a Secret. Never its value.
///
/// Returned by [`crate::connection::Connection::describe_secret`]. There is no
/// field for the credential and no method that could produce one.
#[derive(Debug, Clone, PartialEq, Eq, serde::Deserialize)]
pub struct SecretInfo {
/// The Secret's database-scoped name.
pub name: String,
/// When the Secret was created, as an RFC 3339 timestamp.
pub created_at: String,
/// When the Secret's value was last rotated, as an RFC 3339 timestamp.
pub updated_at: String,
}
rust/lancedb/tests/first_class_function_slice2.rs
+1 -105
View File
@@ -5,9 +5,7 @@ use std::fs;
use std::path::PathBuf;
use lancedb::Error;
use lancedb::function::{
FunctionRegistrationRequest, MAX_FUNCTION_SECRET_ENV_BINDINGS, MAX_SECRET_VALUE_BYTES,
};
use lancedb::function::FunctionRegistrationRequest;
use serde_json::Value;
fn fixture(name: &str) -> String {
@@ -115,105 +113,3 @@ async fn local_function_catalog_operations_return_stable_not_supported() {
));
}
}
/// The cap is enforced above the backend, so every database and every language
/// surface rejects the same envelope. A local connection would otherwise answer
/// `NotSupported` first, which is what makes it the honest probe here.
#[tokio::test]
async fn a_function_binds_at_most_sixteen_secrets() {
let directory = tempfile::tempdir().unwrap();
let connection = lancedb::connect(directory.path().to_str().unwrap())
.execute()
.await
.unwrap();
let mut request = FunctionRegistrationRequest::from_json(&fixture(
"remote_function_registration_request.json",
))
.unwrap();
request.secret_env_bindings = (0..=MAX_FUNCTION_SECRET_ENV_BINDINGS)
.map(|index| (format!("TOKEN_{index}"), format!("secret-{index}")))
.collect();
let error = connection.create_function_async(request).await.unwrap_err();
assert!(matches!(
error,
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 { .. }));
}