diff --git a/python/python/lancedb/secrets.py b/python/python/lancedb/secrets.py index b16f32314..e04c0054f 100644 --- a/python/python/lancedb/secrets.py +++ b/python/python/lancedb/secrets.py @@ -14,7 +14,11 @@ from __future__ import annotations import re -_SECRET_NAME = re.compile(r"^[A-Za-z0-9_-]{1,255}$") +# The same characters LanceDB already admits in a namespace or table name, and +# no positional rule on top of them: a segment may begin with `_`, `-` or `.` +# today, so anything narrower would put Secrets out of reach inside namespaces +# that already exist. Matches the service, which admits the same set. +_SECRET_NAME = re.compile(r"^[A-Za-z0-9_.-]{1,255}$") _ENV_VARIABLE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") diff --git a/python/python/tests/test_first_class_function_slice2.py b/python/python/tests/test_first_class_function_slice2.py index d04a2b5f5..24179d867 100644 --- a/python/python/tests/test_first_class_function_slice2.py +++ b/python/python/tests/test_first_class_function_slice2.py @@ -281,6 +281,7 @@ def test_a_credential_value_is_rejected_in_the_binding_position(): ("openai-prod", "not-a-var", "invalid environment variable name"), ("openai-prod", "API-TOKEN", "invalid environment variable name"), ("not a name", "API_TOKEN", "invalid Secret name"), + ("openai$prod", "API_TOKEN", "invalid Secret name"), ], ) def test_a_binding_validates_both_names_locally(secret, variable, message): @@ -288,6 +289,24 @@ def test_a_binding_validates_both_names_locally(secret, variable, message): EnvVarSecret(secret=secret, env_variable=variable) +def test_a_secret_name_admits_what_a_namespace_name_does(): + """A Secret has to be nameable wherever a namespace already is. + + LanceDB namespace and table names are `[A-Za-z0-9_.-]` with no rule about + which character comes first, so a name may lead with `_`, `-` or `.`. + Anything narrower here would leave Secrets unaddressable inside namespaces + that already exist -- the reason periods are admitted is the reason the + edges are too. + """ + for name in ["openai.prod.v1", ".hidden", "_internal", "-lead", "trailing."]: + binding = EnvVarSecret(secret=name, env_variable="OPENAI_API_KEY") + assert binding.secret == name + + for name in ["", "with/slash", "with$delimiter", "a" * 256]: + with pytest.raises(ValueError, match="invalid Secret name"): + EnvVarSecret(secret=name, env_variable="OPENAI_API_KEY") + + def _main_udf_source( *, threshold: int = 20, input_annotation: str = "int", comparison: str = ">=" ) -> str: diff --git a/rust/lancedb/src/remote/client.rs b/rust/lancedb/src/remote/client.rs index 6eaa8fcb1..2b70a9415 100644 --- a/rust/lancedb/src/remote/client.rs +++ b/rust/lancedb/src/remote/client.rs @@ -412,7 +412,7 @@ fn validate_dns_hostname(hostname: &str) -> Result<()> { /// 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 { +fn route_carries_credential(path: &str) -> bool { path.split('/').any(|segment| segment == "secrets") }