feat: harden function code by validating return values#1070
Open
Datron wants to merge 1 commit into
Open
Conversation
|
Review changes with Changed Files
|
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughFunction compilation now requires a ChangesFunction type-aware compile validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Handler as create_handler/update_handler
participant CompileFn as compile_fn
participant Runtime as rustyscript runtime
Client->>Handler: submit function code
Handler->>Handler: determine function_type
Handler->>CompileFn: compile_fn(code, function_type)
CompileFn->>CompileFn: generate_wrapped_code(code)
CompileFn->>Runtime: execute() with typed payload
Runtime-->>CompileFn: JSON result
CompileFn->>CompileFn: validate array/boolean by function_type
CompileFn-->>Handler: Ok or Err
Handler-->>Client: response
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/context_aware_config/src/validation_functions.rs`:
- Around line 187-242: The compile_fn validation currently calls validation
functions through validation_functions::compile_fn using an empty
FunctionEnvironment and blank/default request fields, which can falsely fail
functions that require real key/value/context data. Update the execute smoke
test payloads for FunctionType::ValueCompute, FunctionType::ValueValidation, and
FunctionType::ContextValidation to use contract-appropriate fixture inputs, or
relax this path so runtime exceptions from missing data do not hard-fail
create/update validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 53826c1e-16b9-468b-a64f-0979a723afc4
📒 Files selected for processing (2)
crates/context_aware_config/src/api/functions/handlers.rscrates/context_aware_config/src/validation_functions.rs
Comment on lines
+187
to
+242
| 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(()) | ||
| } | ||
|
|
||
| } |
Contributor
There was a problem hiding this comment.
🎯 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
compile_fn executes execute with an empty FunctionEnvironment and blank fields, so any function that legitimately needs a key, value, or context entry can be rejected at create/update time even if it works during normal execution. Use contract-appropriate fixture data or skip hard-failing on runtime exceptions here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/context_aware_config/src/validation_functions.rs` around lines 187 -
242, The compile_fn validation currently calls validation functions through
validation_functions::compile_fn using an empty FunctionEnvironment and
blank/default request fields, which can falsely fail functions that require real
key/value/context data. Update the execute smoke test payloads for
FunctionType::ValueCompute, FunctionType::ValueValidation, and
FunctionType::ContextValidation to use contract-appropriate fixture inputs, or
relax this path so runtime exceptions from missing data do not hard-fail
create/update validation.
Datron
force-pushed
the
rustyscript-hardening
branch
from
e584879 to
46ce270
Compare
sauraww
requested changes
Jul 1, 2026
Comment on lines
105
to
111
Collaborator
There was a problem hiding this comment.
This change is not required.
Collaborator
There was a problem hiding this comment.
This file changes also looks only formatting change, Can we revert ?
Datron
force-pushed
the
rustyscript-hardening
branch
from
46ce270 to
bc9e7c1
Compare
Signed-off-by: datron <[email protected]>
Datron
force-pushed
the
rustyscript-hardening
branch
from
bc9e7c1 to
2c9d6d4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Superposition does not validate function return types
Solution
During compilation and checks, simplify and strengthen checks by running the
executefunction. This does the following:executeexistsexecuteis a functionexecutereturns the correct type based on the function typePost-deployment activity
Maybe update blog post
Review these two files, the other changes were generated when I ran
make checkSummary by CodeRabbit