Validate args for system UDFs (#23235)

Validate arguments for system UDFs

GitOrigin-RevId: 1c482648ad68cbf544f1eb256f059c35a8283bcb

crates/isolate/src/environment/action/mod.rs

@@ -81,7 +81,9 @@ use value::{
     heap_size::HeapSize,
     ConvexArray,
     Size,
+    TableMapping,
     TableMappingValue,
+    VirtualTableMapping,
 };
 
 use self::{
@@ -1081,8 +1083,12 @@ impl<RT: Runtime> IsolateEnvironment<RT> for ActionEnvironment<RT> {
         self.phase.get_environment_variable(name)
     }
 
-    fn get_table_mapping(&mut self) -> anyhow::Result<TableMappingValue> {
-        anyhow::bail!("get_table_mapping unsupported in actions")
+    fn get_table_mapping_without_system_tables(&mut self) -> anyhow::Result<TableMappingValue> {
+        anyhow::bail!("get_table_mapping_without_system_tables unsupported in actions")
+    }
+
+    fn get_all_table_mappings(&mut self) -> anyhow::Result<(TableMapping, VirtualTableMapping)> {
+        anyhow::bail!("get_all_table_mappings unsupported in actions")
     }
 
     // We lookup all modules' sources upfront when initializing the action

crates/isolate/src/environment/analyze.rs

@@ -72,7 +72,9 @@ use sync_types::{
 };
 use value::{
     heap_size::WithHeapSize,
+    TableMapping,
     TableMappingValue,
+    VirtualTableMapping,
 };
 
 use crate::{
@@ -151,7 +153,14 @@ impl<RT: Runtime> IsolateEnvironment<RT> for AnalyzeEnvironment {
         Ok(value)
     }
 
-    fn get_table_mapping(&mut self) -> anyhow::Result<TableMappingValue> {
+    fn get_table_mapping_without_system_tables(&mut self) -> anyhow::Result<TableMappingValue> {
+        anyhow::bail!(ErrorMetadata::bad_request(
+            "NoTableMappingFetchDuringImport",
+            "Getting the table mapping unsupported at import time"
+        ))
+    }
+
+    fn get_all_table_mappings(&mut self) -> anyhow::Result<(TableMapping, VirtualTableMapping)> {
         anyhow::bail!(ErrorMetadata::bad_request(
             "NoTableMappingFetchDuringImport",
             "Getting the table mapping unsupported at import time"

crates/isolate/src/environment/auth_config.rs

@@ -33,7 +33,11 @@ use rand_chacha::ChaCha12Rng;
 use regex::Regex;
 use serde::Deserialize;
 use serde_json::Value as JsonValue;
-use value::TableMappingValue;
+use value::{
+    TableMapping,
+    TableMappingValue,
+    VirtualTableMapping,
+};
 
 use crate::{
     concurrency_limiter::ConcurrencyPermit,
@@ -118,7 +122,14 @@ impl<RT: Runtime> IsolateEnvironment<RT> for AuthConfigEnvironment {
             .map(Some)
     }
 
-    fn get_table_mapping(&mut self) -> anyhow::Result<TableMappingValue> {
+    fn get_table_mapping_without_system_tables(&mut self) -> anyhow::Result<TableMappingValue> {
+        anyhow::bail!(ErrorMetadata::bad_request(
+            "NoTableMappingFetchDuringAuthConfig",
+            "Getting the table mapping unsupported when evaluating auth config file"
+        ))
+    }
+
+    fn get_all_table_mappings(&mut self) -> anyhow::Result<(TableMapping, VirtualTableMapping)> {
         anyhow::bail!(ErrorMetadata::bad_request(
             "NoTableMappingFetchDuringAuthConfig",
             "Getting the table mapping unsupported when evaluating auth config file"

crates/isolate/src/environment/mod.rs

@@ -28,7 +28,11 @@ use model::modules::module_versions::{
 };
 use rand::Rng;
 use serde_json::Value as JsonValue;
-use value::TableMappingValue;
+use value::{
+    TableMapping,
+    TableMappingValue,
+    VirtualTableMapping,
+};
 
 pub use self::async_op::AsyncOpRequest;
 use crate::{
@@ -80,7 +84,10 @@ pub trait IsolateEnvironment<RT: Runtime>: 'static {
 
     fn get_environment_variable(&mut self, name: EnvVarName)
         -> anyhow::Result<Option<EnvVarValue>>;
-    fn get_table_mapping(&mut self) -> anyhow::Result<TableMappingValue>;
+
+    /// The table mapping omitting system tables, intended for the dashboard.
+    fn get_table_mapping_without_system_tables(&mut self) -> anyhow::Result<TableMappingValue>;
+    fn get_all_table_mappings(&mut self) -> anyhow::Result<(TableMapping, VirtualTableMapping)>;
 
     fn start_async_op(
         &mut self,

crates/isolate/src/environment/schema.rs

@@ -36,7 +36,11 @@ use model::{
 use rand::SeedableRng;
 use rand_chacha::ChaCha12Rng;
 use serde_json::Value as JsonValue;
-use value::TableMappingValue;
+use value::{
+    TableMapping,
+    TableMappingValue,
+    VirtualTableMapping,
+};
 
 use crate::{
     concurrency_limiter::ConcurrencyPermit,
@@ -101,7 +105,14 @@ impl<RT: Runtime> IsolateEnvironment<RT> for SchemaEnvironment {
         ))
     }
 
-    fn get_table_mapping(&mut self) -> anyhow::Result<TableMappingValue> {
+    fn get_table_mapping_without_system_tables(&mut self) -> anyhow::Result<TableMappingValue> {
+        anyhow::bail!(ErrorMetadata::bad_request(
+            "NoTableMappingFetchInSchema",
+            "Getting the table mapping unsupported when evaluating schema"
+        ))
+    }
+
+    fn get_all_table_mappings(&mut self) -> anyhow::Result<(TableMapping, VirtualTableMapping)> {
         anyhow::bail!(ErrorMetadata::bad_request(
             "NoTableMappingFetchInSchema",
             "Getting the table mapping unsupported when evaluating schema"

crates/isolate/src/environment/udf/mod.rs

@@ -89,7 +89,9 @@ use value::{
         WithHeapSize,
     },
     Size,
+    TableMapping,
     TableMappingValue,
+    VirtualTableMapping,
     MAX_DOCUMENT_NESTING,
     MAX_USER_SIZE,
     VALUE_TOO_LARGE_SHORT_MSG,
@@ -254,10 +256,18 @@ impl<RT: Runtime> IsolateEnvironment<RT> for DatabaseUdfEnvironment<RT> {
         self.phase.get_environment_variable(name)
     }
 
-    fn get_table_mapping(&mut self) -> anyhow::Result<TableMappingValue> {
+    fn get_table_mapping_without_system_tables(&mut self) -> anyhow::Result<TableMappingValue> {
         Ok(self.phase.tx()?.table_mapping().clone().into())
     }
 
+    fn get_all_table_mappings(&mut self) -> anyhow::Result<(TableMapping, VirtualTableMapping)> {
+        let tx = self.phase.tx()?;
+        Ok((
+            tx.table_mapping().clone(),
+            tx.virtual_table_mapping().clone(),
+        ))
+    }
+
     async fn lookup_source(
         &mut self,
         path: &str,

crates/isolate/src/execution_scope.rs

@@ -568,7 +568,10 @@ impl<'a, 'b: 'a, RT: Runtime, E: IsolateEnvironment<RT>> ExecutionScope<'a, 'b,
             "atob" => self.op_atob(args, rv)?,
             "btoa" => self.op_btoa(args, rv)?,
             "environmentVariables/get" => self.op_environmentVariables_get(args, rv)?,
-            "getTableMapping" => self.op_getTableMapping(args, rv)?,
+            "getTableMappingWithoutSystemTables" => {
+                self.op_getTableMappingWithoutSystemTables(args, rv)?
+            },
+            "validateArgs" => self.op_validate_args(args, rv)?,
             "crypto/randomUUID" => self.op_crypto_randomUUID(args, rv)?,
             "crypto/getRandomValues" => self.op_crypto_getRandomValues(args, rv)?,
             "crypto/sign" => self.op_crypto_sign(args, rv)?,

crates/isolate/src/ops/database.rs

@@ -9,11 +9,14 @@ use crate::{
 
 impl<'a, 'b: 'a, RT: Runtime, E: IsolateEnvironment<RT>> ExecutionScope<'a, 'b, RT, E> {
     #[convex_macro::v8_op]
-    pub fn op_getTableMapping(&mut self) -> anyhow::Result<JsonValue> {
+    pub fn op_getTableMappingWithoutSystemTables(&mut self) -> anyhow::Result<JsonValue> {
         let state = self.state_mut()?;
-        state.environment.get_table_mapping().and_then(|mapping| {
-            serde_json::to_value(mapping)
-                .map_err(|_| anyhow!("Couldn’t serialize the table mapping"))
-        })
+        state
+            .environment
+            .get_table_mapping_without_system_tables()
+            .and_then(|mapping| {
+                serde_json::to_value(mapping)
+                    .map_err(|_| anyhow!("Couldn’t serialize the table mapping"))
+            })
     }
 }

crates/isolate/src/ops/mod.rs

@@ -16,5 +16,6 @@ mod storage;
 mod stream;
 mod text;
 mod time;
+mod validate_args;
 
 pub use self::crypto::CryptoOps;

crates/isolate/src/ops/validate_args.rs

@@ -0,0 +1,72 @@
+use std::convert::{
+    TryFrom,
+    TryInto,
+};
+
+use anyhow::Context;
+use common::{
+    self,
+    runtime::Runtime,
+};
+use model::{
+    self,
+    modules::args_validator::ArgsValidator,
+};
+use serde_json::{
+    json,
+    Value as JsonValue,
+};
+use value::ConvexArray;
+
+use crate::{
+    environment::IsolateEnvironment,
+    execution_scope::ExecutionScope,
+    helpers::UdfArgsJson,
+};
+
+impl<'a, 'b: 'a, RT: Runtime, E: IsolateEnvironment<RT>> ExecutionScope<'a, 'b, RT, E> {
+    #[convex_macro::v8_op]
+
+    pub fn op_validate_args(
+        &mut self,
+        validator: JsonValue,
+        args: UdfArgsJson,
+    ) -> anyhow::Result<JsonValue> {
+        let JsonValue::String(validator_string) = validator.clone() else {
+            return Err(anyhow::anyhow!("export_args result not a string"));
+        };
+
+        let args_validator: ArgsValidator =
+            match serde_json::from_str::<JsonValue>(&validator_string) {
+                Ok(args_json) => match ArgsValidator::try_from(args_json) {
+                    Ok(validator) => validator,
+                    Err(err) => return Err(err),
+                },
+                Err(json_error) => {
+                    let message =
+                        format!("Unable to parse JSON returned from `exportArgs`: {json_error}");
+                    return Err(anyhow::anyhow!(message));
+                },
+            };
+
+        let args_array = args
+            .into_arg_vec()
+            .into_iter()
+            .map(|arg| arg.try_into())
+            .collect::<anyhow::Result<Vec<_>>>()
+            .and_then(ConvexArray::try_from)
+            .map_err(|err| anyhow::anyhow!(format!("{}", err)))?;
+
+        let state = self.state_mut()?;
+        let (table_mapping, virtual_table_mapping) = state.environment.get_all_table_mappings()?;
+        match args_validator.check_args(&args_array, &table_mapping, &virtual_table_mapping)? {
+            Some(js_error) => Ok(json!({
+                "valid": false,
+                "message": format!("{}", js_error)
+            })),
+            None => Ok(json!({
+                "valid": true,
+            })),
+        }
+    }
+}

crates/isolate/src/tests/mod.rs

@@ -28,6 +28,7 @@ mod search;
 mod shapes;
 mod size_errors;
 mod source_maps;
+mod system_udfs;
 mod unicode;
 mod user_error;
 mod vector_search;

crates/isolate/src/tests/system_udfs.rs

@@ -0,0 +1,51 @@
+use std::collections::HashSet;
+
+use common::{
+    assert_obj,
+    types::EnvironmentVariable,
+    value::ConvexValue,
+};
+use keybroker::Identity;
+use model::environment_variables::EnvironmentVariablesModel;
+use must_let::must_let;
+use runtime::testing::TestRuntime;
+use value::ConvexObject;
+
+use crate::test_helpers::UdfTest;
+
+#[convex_macro::test_runtime]
+async fn test_system_udf(rt: TestRuntime) -> anyhow::Result<()> {
+    let t = UdfTest::default(rt).await?;
+    let mut tx = t.database.begin(Identity::system()).await?;
+    let environment_variable = EnvironmentVariable::new("A".parse()?, "B".parse()?);
+    EnvironmentVariablesModel::new(&mut tx)
+        .create(environment_variable, &HashSet::new())
+        .await?;
+    t.database.commit(tx).await?;
+
+    // nonexistent returns Null
+    must_let!(let ConvexValue::Null = t.query(
+        "_system/cli/queryEnvironmentVariables:get",
+        assert_obj!("name" => "nonexistent".to_string()),
+    )
+    .await?);
+
+    // return environment variable
+    must_let!(let ConvexValue::Object(obj) = t.query(
+        "_system/cli/queryEnvironmentVariables:get",
+        assert_obj!("name" => "A".to_string()),
+    )
+    .await?);
+    must_let!(let Some(ConvexValue::String(name)) = ConvexObject::get(&obj, "name"));
+    assert_eq!(name.to_string(), "A");
+    must_let!(let Some(ConvexValue::String(value)) = ConvexObject::get(&obj, "value"));
+    assert_eq!(value.to_string(), "B");
+
+    // calling with empty argument fails
+    let error = t
+        .query_js_error("_system/cli/queryEnvironmentVariables:get", assert_obj!())
+        .await?;
+
+    assert!(error.message.contains("missing the required field `name`"));
+    Ok(())
+}

crates/local_backend/src/node_action_callbacks.rs

@@ -508,7 +508,7 @@ mod tests {
         let json_body = json!({
             "path":
                 "_system/frontend/paginatedScheduledJobs.js",
-            "args":json!({"paginationOpts": {"numItems": 10}}),
+            "args":json!({"paginationOpts": {"numItems": 10, "cursor": null}}),
             "format": "json",
         });
         let body = Body::from(serde_json::to_vec(&json_body)?);
@@ -571,7 +571,7 @@ mod tests {
         let json_body = json!({
             "path":
                 "_system/frontend/paginatedScheduledJobs.js",
-            "args":json!({"paginationOpts": {"numItems": 10}}),
+            "args":json!({"paginationOpts": {"numItems": 10, "cursor": null}}),
             "format": "json",
         });
         let body = Body::from(serde_json::to_vec(&json_body)?);

npm-packages/system-udfs/convex/_system/frontend/getTableMapping.ts

@@ -8,6 +8,6 @@ import { queryPrivateSystem } from "../secretSystemTables";
 export default queryPrivateSystem({
   args: {},
   handler: async () => {
-    return performOp("getTableMapping");
+    return performOp("getTableMappingWithoutSystemTables");
   },
 });

npm-packages/system-udfs/convex/_system/secretSystemTables.ts

@@ -9,6 +9,7 @@ import {
 import { Validator } from "convex/values";
 import { query as baseQuery } from "../_generated/server";
 import { Id } from "../_generated/dataModel";
+import { performOp } from "../syscall";
 
 // This set must be kept up-to-date to prevent accidental access to secret
 // system tables in system UDFs.
@@ -95,6 +96,7 @@ function maskPublicSystem<T extends GenericDataModel>(
 type FunctionDefinition = {
   args: Record<string, Validator<any, boolean>>;
   handler: (ctx: any, args: DefaultFunctionArgs) => any;
+  exportArgs(): string;
 };
 
 /// `queryPrivateSystem` is for querying private system tables.
@@ -106,9 +108,18 @@ export const queryPrivateSystem = ((functionDefinition: FunctionDefinition) => {
   if (!("args" in functionDefinition)) {
     throw new Error("args validator required for system udf");
   }
+  const query = baseQuery({
+    args: functionDefinition.args,
+    handler: () => {},
+  });
+  const argsValidatorJson = query.exportArgs();
   return baseQuery({
     args: functionDefinition.args,
-    handler: (ctx: any, args: any) => {
+    handler: async (ctx: any, args: any) => {
+      const result = await performOp("validateArgs", argsValidatorJson, args);
+      if (!result.valid) {
+        throw new Error(result.message);
+      }
       return functionDefinition.handler(
         { ...ctx, db: maskPrivateSystem(ctx.db) },
         args,