-
Notifications
You must be signed in to change notification settings - Fork 46
feat: harden function code by validating return values #1070
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,8 +7,14 @@ use superposition_macros::{bad_argument, validation_error}; | |
| use service_utils::service::types::{EncryptionKey, WorkspaceContext}; | ||
| use superposition_types::{ | ||
| DBConnection, | ||
| api::functions::{FunctionExecutionRequest, FunctionExecutionResponse}, | ||
| database::models::cac::{FunctionCode, FunctionRuntimeVersion, FunctionType}, | ||
| api::functions::{ | ||
| ContextValidationTrigger, FunctionEnvironment, FunctionExecutionRequest, | ||
| FunctionExecutionResponse, KeyType, | ||
| }, | ||
| database::models::{ | ||
| ChangeReason, | ||
| cac::{FunctionCode, FunctionRuntimeVersion, FunctionType}, | ||
| }, | ||
| result as superposition, | ||
| }; | ||
|
|
||
|
|
@@ -73,11 +79,12 @@ pub fn run_js_function( | |
| args: FunctionExecutionRequest, | ||
| runtime_version: FunctionRuntimeVersion, | ||
| ) -> Result<FunctionExecutionResponse, (String, Option<String>)> { | ||
| let wrapped_code = generate_wrapped_code(&code.0); | ||
| let wrapped_code = generate_wrapped_code(&code); | ||
| let module = Module::new("function.js", &wrapped_code); | ||
|
|
||
| let runtime_options = RuntimeOptions { | ||
| timeout: Duration::from_millis(1500), | ||
| // import_provider: Some(Box::new(DenyFileImportProvider::default())), | ||
| ..Default::default() | ||
| }; | ||
|
|
||
|
|
@@ -147,19 +154,14 @@ pub fn execute_fn( | |
| run_js_function(code, args.clone(), runtime_version) | ||
| } | ||
|
|
||
| pub fn compile_fn(code_str: &FunctionCode) -> superposition::Result<()> { | ||
| let type_check_code = format!( | ||
| r#" | ||
| {} | ||
|
|
||
| export function typeCheck() {{ | ||
| if (typeof execute === "undefined") {{ | ||
| throw new Error("execute function is not defined"); | ||
| }} | ||
| }} | ||
| "#, | ||
| code_str | ||
| ); | ||
| pub fn compile_fn( | ||
| code_str: &FunctionCode, | ||
| fn_type: &FunctionType, | ||
| ) -> superposition::Result<()> { | ||
| // syntax validation, so we do not need the actual secrets and vars | ||
| let stubs = "const SECRETS = {};\nconst VARS = {};\n"; | ||
| let stubbed_code = FunctionCode(format!("{}\n{}", stubs, code_str)); | ||
| let type_check_code = generate_wrapped_code(&stubbed_code); | ||
|
|
||
| rustyscript::validate(&type_check_code).map_err(|err| { | ||
| log::error!("Invalid function syntax: {:?}", err); | ||
|
|
@@ -185,13 +187,62 @@ pub fn compile_fn(code_str: &FunctionCode) -> superposition::Result<()> { | |
| let tokio_runtime = runtime.tokio_runtime(); | ||
| tokio_runtime | ||
| .block_on(async { | ||
| runtime | ||
| .call_function_async::<()>( | ||
| Some(&module_handle), | ||
| "typeCheck", | ||
| json_args!(), | ||
| ) | ||
| .await | ||
| let environment = FunctionEnvironment { | ||
| context: serde_json::Map::new(), | ||
| overrides: serde_json::Map::new(), | ||
| }; | ||
| match fn_type { | ||
| FunctionType::ValueCompute => { | ||
| let payload = FunctionExecutionRequest::ValueComputeFunctionRequest { | ||
| name: String::new(), | ||
| prefix: String::new(), | ||
| r#type: superposition_types::api::functions::KeyType::ConfigKey, | ||
| environment, | ||
| }; | ||
| let serde_json::Value::Array(_) = runtime | ||
| .call_function_async::<serde_json::Value>( | ||
| Some(&module_handle), | ||
| "execute", | ||
| json_args!(payload), | ||
| ) | ||
| .await? else { | ||
| return Err(rustyscript::Error::Runtime("The value compute function did not return an array".to_string())); | ||
| }; | ||
| Ok(()) | ||
| } | ||
| other => { | ||
| let payload = match other { | ||
| FunctionType::ValueValidation => FunctionExecutionRequest::ValueValidationFunctionRequest { | ||
| key: String::new(), | ||
| value: serde_json::Value::String(String::new()), | ||
| r#type: | ||
| KeyType::ConfigKey, | ||
| environment, | ||
| }, | ||
| FunctionType::ContextValidation => FunctionExecutionRequest::ContextValidationFunctionRequest { | ||
| environment, | ||
| trigger_reason: ContextValidationTrigger::Context, | ||
| }, | ||
| FunctionType::ChangeReasonValidation => FunctionExecutionRequest::ChangeReasonValidationFunctionRequest { | ||
| change_reason: ChangeReason::default(), | ||
| }, | ||
| _ => { | ||
| return Err(rustyscript::Error::Runtime("An invalid situation occurred in the runtime".to_string())) | ||
| } | ||
| }; | ||
| let serde_json::Value::Bool(_) = runtime | ||
| .call_function_async::<serde_json::Value>( | ||
| Some(&module_handle), | ||
| "execute", | ||
| json_args!(payload), | ||
| ) | ||
| .await? else { | ||
| return Err(rustyscript::Error::Runtime("The function did not return a boolean".to_string())) | ||
| }; | ||
| Ok(()) | ||
| } | ||
|
|
||
| } | ||
|
Comment on lines
+190
to
+245
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verify rustyscript Error::Runtime variant and call_function_async signature
rg -nP 'enum Error|Runtime\s*\(|fn call_function_async' $(fd -HI -t d 'rustyscript*' ~/.cargo 2>/dev/null) 2>/dev/null
cargo doc -p rustyscript --no-deps 2>/dev/null | head -n 5 || echo "inspect rustyscript version in Cargo.lock"
rg -nP 'name = "rustyscript"' -A2 Cargo.lockRepository: juspay/superposition Length of output: 158 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect the validation code and nearby context
sed -n '1,320p' crates/context_aware_config/src/validation_functions.rs | cat -n
# Find related usages/tests/docs in the repo
rg -n "validation_functions|FunctionType::ValueCompute|ContextValidation|ChangeReasonValidation|validate.*function|execute" crates/context_aware_config -S
# Locate rustyscript version and any cached source/docs in the local cargo registry
rg -n 'name = "rustyscript"' Cargo.lock
fd -HI -t d 'rustyscript-*' ~/.cargo/registry/src ~/.cargo/registry/cache 2>/dev/null | head -n 20Repository: juspay/superposition Length of output: 20912 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect call sites and validation semantics
sed -n '1,260p' crates/context_aware_config/src/api/functions/handlers.rs | cat -n
sed -n '1,260p' crates/context_aware_config/src/api/dimension/validations.rs | cat -n
sed -n '1,260p' crates/context_aware_config/src/api/default_config/handlers.rs | cat -n
# Search for tests or docs that describe what compile_fn accepts
rg -n "compile_fn|validation_error!|ValueCompute|ContextValidation|ChangeReasonValidation|dummy|empty/default|return type" crates/context_aware_config -S
# Locate rustyscript source in the local cargo registry and inspect the API
fd -HI -t d 'rustyscript-*' ~/.cargo/registry/src ~/.cargo/registry/cache 2>/dev/null | head -n 20Repository: juspay/superposition Length of output: 39607 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Get rustyscript version from Cargo.lock and locate the corresponding source
version=$(awk '
$0 == '\"'\"'name = \"rustyscript\"'\"'\"' {found=1}
found && $0 ~ /^version = / {gsub(/"/,""); print $3; exit}
' Cargo.lock)
echo "rustyscript version: ${version:-unknown}"
root=$(fd -HI -t d "rustyscript-${version}" ~/.cargo/registry/src ~/.cargo/registry/cache 2>/dev/null | head -n 1)
echo "rustyscript root: $root"
if [ -n "${root:-}" ]; then
rg -n "enum Error|Runtime\\(|call_function_async" "$root" -S
fiRepository: juspay/superposition Length of output: 242 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect runtime validation semantics for context validation functions
sed -n '280,360p' crates/context_aware_config/src/api/context/helpers.rs | cat -n
# Safely extract the rustyscript version from Cargo.lock
python3 - <<'PY'
import re, pathlib
text = pathlib.Path("Cargo.lock").read_text()
m = re.search(r'name = "rustyscript"\nversion = "([^"]+)"', text)
print(m.group(1) if m else "not found")
PYRepository: juspay/superposition Length of output: 3623 Avoid validating with empty/default inputs 🤖 Prompt for AI Agents |
||
| }) | ||
| .map_err(|e| { | ||
| let err_str = e.to_string(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we remove this comment ?