fix(cache): exclude secret-dependent results

Qualify worker-owned dependencies in catalog macro ASTs so macros remain valid under caller-selected catalog aliases. Fail closed on producer and exchange result caching whenever a function declares required secrets, without retaining secret material in keys or diagnostics.

Author: rustyconover

src/include/vgi_exchange_cache_key.hpp

@@ -26,6 +26,7 @@ struct VgiCachedBatch;         // vgi_result_cache.hpp
 struct VgiCacheControl;        // vgi_cache_control.hpp
 struct VgiTableInOutBindData;  // vgi_table_in_out_impl.hpp
 struct VgiAttachParameters;    // vgi_attach_parameters.hpp
+struct VgiSecretRequirement;   // vgi_catalog_metadata.hpp
 
 // ============================================================================
 // Canonical key-component serializers (shared with the producer path)
@@ -105,7 +106,8 @@ void SyncResultCacheSettings(ClientContext &context);
 //! producer-only filter/order/sample components). `key.input_hash` is left EMPTY —
 //! the caller sets it per memoization event. Returns false + sets `*reason` when
 //! ineligible (global switch off, per-catalog opt-out, unresolved identity,
-//! unknown catalog version). On true, fills `key`, `catalog_name`, `catalog_version`.
+//! unknown catalog version, or any secret dependency). On true, fills `key`,
+//! `catalog_name`, `catalog_version`.
 //!
 //! SECURITY: identity_scope folds in the caller's auth principal
 //! (BuildCatalogIdentityScope); "" fails closed so two identities never cross-serve.
@@ -126,7 +128,9 @@ bool BuildExchangeCacheKeyStaticFields(ClientContext &context,
                                        const std::string &function_name, const std::string &schema_name,
                                        const std::string &canonical_arguments,
                                        const std::map<std::string, Value> &settings,
-                                       const std::vector<int32_t> &projection_ids, VgiResultCacheKey &key,
+                                       const std::vector<int32_t> &projection_ids,
+                                       const std::vector<VgiSecretRequirement> &required_secrets,
+                                       VgiResultCacheKey &key,
                                        std::string &catalog_name, int64_t &catalog_version,
                                        const char *&reason, const std::string &operator_kind);
 

src/storage/vgi_macro_set.cpp

@@ -4,13 +4,17 @@
 #include "duckdb/catalog/catalog.hpp"
 #include "duckdb/catalog/catalog_entry/scalar_macro_catalog_entry.hpp"
 #include "duckdb/catalog/catalog_entry/table_macro_catalog_entry.hpp"
+#include "duckdb/common/case_insensitive_map.hpp"
 #include "duckdb/function/scalar_macro_function.hpp"
 #include "duckdb/function/table_macro_function.hpp"
 #include "duckdb/main/attached_database.hpp"
 #include "duckdb/main/client_context.hpp"
 #include "duckdb/parser/expression/columnref_expression.hpp"
 #include "duckdb/parser/expression/constant_expression.hpp"
+#include "duckdb/parser/expression/function_expression.hpp"
+#include "duckdb/parser/expression/subquery_expression.hpp"
 #include "duckdb/parser/parsed_data/create_macro_info.hpp"
+#include "duckdb/parser/parsed_expression_iterator.hpp"
 #include "duckdb/parser/parser.hpp"
 #include "duckdb/parser/statement/select_statement.hpp"
 
@@ -23,6 +27,81 @@
 
 namespace duckdb {
 
+static void QualifyWorkerFunctions(QueryNode &node, const case_insensitive_set_t &worker_functions,
+                                   const string &catalog_name, const string &schema_name);
+
+static void QualifyWorkerFunctions(ParsedExpression &expression,
+                                   const case_insensitive_set_t &worker_functions,
+                                   const string &catalog_name, const string &schema_name) {
+	if (expression.GetExpressionClass() == ExpressionClass::FUNCTION) {
+		auto &function = expression.Cast<FunctionExpression>();
+		if (function.catalog.empty() && function.schema.empty() &&
+		    worker_functions.find(function.function_name) != worker_functions.end()) {
+			function.catalog = catalog_name;
+			function.schema = schema_name;
+		}
+	} else if (expression.GetExpressionClass() == ExpressionClass::SUBQUERY) {
+		auto &subquery = expression.Cast<SubqueryExpression>();
+		QualifyWorkerFunctions(*subquery.subquery->node, worker_functions, catalog_name, schema_name);
+	}
+
+	ParsedExpressionIterator::EnumerateChildren(expression, [&](ParsedExpression &child) {
+		QualifyWorkerFunctions(child, worker_functions, catalog_name, schema_name);
+	});
+}
+
+static void QualifyWorkerFunctions(QueryNode &node, const case_insensitive_set_t &worker_functions,
+                                   const string &catalog_name, const string &schema_name) {
+	ParsedExpressionIterator::EnumerateQueryNodeChildren(
+	    node, [&](unique_ptr<ParsedExpression> &expression) {
+		    QualifyWorkerFunctions(*expression, worker_functions, catalog_name, schema_name);
+	    });
+}
+
+static void AddFunctionNames(const vgi::CatalogRpcContext &rpc_ctx, const string &schema_name,
+                             const char *function_type, ClientContext &context,
+                             case_insensitive_set_t &worker_functions) {
+	for (const auto &function :
+	     vgi::InvokeCatalogSchemaContentsFunctions(rpc_ctx, schema_name, function_type, context)) {
+		worker_functions.insert(function.name);
+	}
+}
+
+static case_insensitive_set_t LoadWorkerFunctionNames(const vgi::CatalogRpcContext &rpc_ctx,
+                                                      const VgiSchemaEntry &schema,
+                                                      const std::vector<vgi::VgiMacroInfo> &macros,
+                                                      CatalogType macro_catalog_type,
+                                                      bool trust_empty_kinds,
+                                                      ClientContext &context) {
+	case_insensitive_set_t worker_functions;
+	for (const auto &macro : macros) {
+		worker_functions.insert(macro.name);
+	}
+
+	const auto &counts = schema.GetEstimatedCounts();
+	if (!trust_empty_kinds || counts.scalar_function != 0) {
+		AddFunctionNames(rpc_ctx, schema.name, "SCALAR_FUNCTION", context, worker_functions);
+	}
+	if (!trust_empty_kinds || counts.aggregate_function != 0) {
+		AddFunctionNames(rpc_ctx, schema.name, "AGGREGATE_FUNCTION", context, worker_functions);
+	}
+	if (!trust_empty_kinds || counts.table_function != 0) {
+		AddFunctionNames(rpc_ctx, schema.name, "TABLE_FUNCTION", context, worker_functions);
+	}
+
+	// Table macros can use scalar macros in their SELECT expressions. Scalar
+	// macros cannot call table macros, and macros of the current kind are already
+	// present in `macros`, so this is the only additional macro inventory needed.
+	if (macro_catalog_type == CatalogType::TABLE_MACRO_ENTRY &&
+	    (!trust_empty_kinds || counts.macro != 0)) {
+		for (const auto &macro :
+		     vgi::InvokeCatalogSchemaContentsMacros(rpc_ctx, schema.name, "SCALAR_MACRO", context)) {
+			worker_functions.insert(macro.name);
+		}
+	}
+	return worker_functions;
+}
+
 VgiMacroSet::VgiMacroSet(Catalog &catalog, VgiSchemaEntry &schema, CatalogType macro_type)
     : VgiCatalogSet(catalog, &schema), schema_(schema), macro_catalog_type_(macro_type) {
 }
@@ -86,6 +165,8 @@ void VgiMacroSet::LoadEntries(ClientContext &context, const std::lock_guard<std:
 	rpc_ctx.entity_qualifier = schema_.name;
 
 	auto macros = vgi::InvokeCatalogSchemaContentsMacros(rpc_ctx, schema_.name, rpc_type, context);
+	auto worker_functions = LoadWorkerFunctionNames(rpc_ctx, schema_, macros, macro_catalog_type_,
+	                                               ReadTrustEmptyKinds(context), context);
 
 	for (auto &macro_info : macros) {
 		try {
@@ -99,6 +180,7 @@ void VgiMacroSet::LoadEntries(ClientContext &context, const std::lock_guard<std:
 					                 macro_info.name.c_str());
 					continue;
 				}
+				QualifyWorkerFunctions(*expressions[0], worker_functions, catalog_.GetName(), schema_.name);
 				macro_func = make_uniq<ScalarMacroFunction>(std::move(expressions[0]));
 			} else {
 				// Table macro: parse query
@@ -111,6 +193,7 @@ void VgiMacroSet::LoadEntries(ClientContext &context, const std::lock_guard<std:
 					continue;
 				}
 				auto &select = parser.statements[0]->Cast<SelectStatement>();
+				QualifyWorkerFunctions(*select.node, worker_functions, catalog_.GetName(), schema_.name);
 				macro_func = make_uniq<TableMacroFunction>(std::move(select.node));
 			}
 

src/vgi_exchange_cache_key.cpp

@@ -272,7 +272,9 @@ bool BuildExchangeCacheKeyStaticFields(ClientContext &context,
                                        const std::string &function_name, const std::string &schema_name,
                                        const std::string &canonical_arguments,
                                        const std::map<std::string, Value> &settings,
-                                       const std::vector<int32_t> &projection_ids, VgiResultCacheKey &key,
+                                       const std::vector<int32_t> &projection_ids,
+                                       const std::vector<VgiSecretRequirement> &required_secrets,
+                                       VgiResultCacheKey &key,
                                        std::string &catalog_name, int64_t &catalog_version,
                                        const char *&reason, const std::string &operator_kind) {
 	reason = nullptr;
@@ -292,6 +294,15 @@ bool BuildExchangeCacheKeyStaticFields(ClientContext &context,
 		reason = "disabled_attach";
 		return false;
 	}
+	// Resolved secret values are deliberately absent from cache identity and
+	// diagnostics. A function that requested any secret can therefore neither
+	// safely probe nor populate a result cache: rotation/removal must be visible
+	// on the very next call, and hashing secret material into a key would retain
+	// credentials in process/disk state. Fail closed for every exchange shape.
+	if (!required_secrets.empty()) {
+		reason = "secret_dependent";
+		return false;
+	}
 
 	// Identity + version. Catalog-attached functions resolve a VgiCatalog by name so
 	// the runtime catalog_version gates freshness (0 = unknown → never cache; a bump
@@ -352,8 +363,8 @@ bool BuildExchangeCacheKeyStatic(ClientContext &context, const VgiTableInOutBind
                                  const std::string &operator_kind) {
 	return BuildExchangeCacheKeyStaticFields(context, bd.attach_params, bd.function_name, bd.schema_name,
 	                                         bd.arguments.array ? bd.arguments.array->ToString() : "",
-	                                         bd.settings, projection_ids, key, catalog_name, catalog_version,
-	                                         reason, operator_kind);
+	                                         bd.settings, projection_ids, bd.required_secrets, key, catalog_name,
+	                                         catalog_version, reason, operator_kind);
 }
 
 // ----------------------------------------------------------------------------

src/vgi_scalar_function_impl.cpp

@@ -573,14 +573,17 @@ void VgiScalarFunctionExecute(DataChunk &args, ExpressionState &state, Vector &r
 		const std::string op_kind = "scalar\x1f" + result.GetType().ToString();
 		if (BuildExchangeCacheKeyStaticFields(context, bind_data->attach_params, bind_data->function_name,
 		                                      bind_data->schema_name, canon_args, bind_data->settings, {},
-		                                      local_state.cache_static_key, local_state.cache_catalog_name, cver,
-		                                      reason, op_kind)) {
+		                                      bind_data->required_secrets, local_state.cache_static_key,
+		                                      local_state.cache_catalog_name, cver, reason, op_kind)) {
 			local_state.cache_eligible = true;
 			local_state.cache_static_fp = local_state.cache_static_key.Fingerprint();
 			Value ttl_v;
 			if (context.TryGetCurrentSetting("vgi_result_cache_default_ttl_seconds", ttl_v) && !ttl_v.IsNull()) {
 				local_state.cache_default_ttl_seconds = static_cast<int64_t>(ttl_v.GetValue<uint64_t>());
 			}
+		} else if (reason) {
+			VGI_LOG(context, "result_cache.ineligible",
+			        {{"function", bind_data->function_name}, {"reason", reason}});
 		}
 	}
 	bool pv_setting = true;

src/vgi_table_function_impl.cpp

@@ -1464,6 +1464,13 @@ CacheEligibility EvaluateCacheEligibility(ClientContext &context,
 		e.ineligible_reason = "disabled_attach";
 		return e;
 	}
+	// Secret values are intentionally neither serialized into the key nor retained
+	// by the cache. Any declared secret dependency makes the result sensitive to
+	// rotation/removal, so probing or storing it would serve stale credentials.
+	if (!bind_data.required_secrets.empty()) {
+		e.ineligible_reason = "secret_dependent";
+		return e;
+	}
 	// Identity + version dimensions. Two paths (M5 adds the direct one):
 	//   - catalog-attached (bind_data.table_entry set): identity = catalog name,
 	//     freshness bounded by the runtime catalog_version (0 = unknown → never

test/sql/integration/cache/secret_ineligible.test

@@ -0,0 +1,80 @@
+# name: test/sql/integration/cache/secret_ineligible.test
+# description: Secret-dependent producer and scalar functions never probe or populate the result cache
+# group: [vgi_integration_cache]
+
+require-env VGI_TEST_WORKER
+
+require vgi
+
+require httpfs
+
+statement ok
+CALL enable_logging(level := 'debug');
+
+statement ok
+ATTACH 'example' AS ex (TYPE vgi, LOCATION '${VGI_TEST_WORKER}');
+
+statement ok
+SELECT * FROM vgi_result_cache_flush();
+
+statement ok
+CREATE SECRET vgi_cache_ineligible_secret (TYPE vgi_example, secret_string 'before', api_key 'key-1', port 1001, use_ssl true, timeout 1.0);
+
+# Scalar results must reflect rotation even when the worker advertises per-value
+# cacheability. The ineligibility event contains only the reason, never secret data.
+query I
+SELECT ex.return_secret_value() LIKE '%before%';
+----
+true
+
+statement ok
+DROP SECRET vgi_cache_ineligible_secret;
+
+statement ok
+CREATE SECRET vgi_cache_ineligible_secret (TYPE vgi_example, secret_string 'after', api_key 'key-2', port 1002, use_ssl false, timeout 2.0);
+
+query I
+SELECT ex.return_secret_value() LIKE '%after%';
+----
+true
+
+# Per-value diagnostics intentionally aggregate by arena rather than exposing the
+# function name. A flushed process with only these scalar calls must have no arena.
+query I
+SELECT COUNT(*) FROM vgi_result_cache() WHERE tier = 'arena';
+----
+0
+
+# Producer results likewise reflect secret removal instead of replaying the prior
+# non-empty result. No producer cache entry is published.
+query I
+SELECT COUNT(*) > 0 FROM ex.secret_demo();
+----
+true
+
+statement ok
+DROP SECRET vgi_cache_ineligible_secret;
+
+query I
+SELECT COUNT(*) FROM ex.secret_demo();
+----
+0
+
+query I
+SELECT COUNT(*) FROM vgi_result_cache() WHERE function = 'secret_demo';
+----
+0
+
+query I
+SELECT COUNT(*) >= 3 FROM duckdb_logs()
+WHERE type = 'VGI'
+  AND message LIKE '%result_cache.ineligible%'
+  AND message LIKE '%reason=secret_dependent%';
+----
+true
+
+statement ok
+DETACH ex;
+
+statement ok
+SELECT * FROM vgi_result_cache_flush();

test/sql/integration/macro/macros.test

@@ -132,3 +132,16 @@ SELECT MAX(range) FROM example.vgi_range_table(5);
 
 statement ok
 DETACH example;
+
+# Macro dependencies are qualified to the catalog alias chosen by the caller,
+# rather than the worker's catalog name or the current search path.
+statement ok
+ATTACH 'example' AS renamed (TYPE vgi, LOCATION '${VGI_TEST_WORKER}');
+
+query I
+SELECT renamed.vgi_multiply(3, 4);
+----
+12
+
+statement ok
+DETACH renamed;