Skip to content

fix(function): validate custom tool param keys before code interpolation#4474

Merged
waleedlatif1 merged 2 commits intostagingfrom
fix/function-execute-param-injection
May 6, 2026
Merged

fix(function): validate custom tool param keys before code interpolation#4474
waleedlatif1 merged 2 commits intostagingfrom
fix/function-execute-param-injection

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

@waleedlatif1 waleedlatif1 commented May 6, 2026

Summary

  • Add SAFE_IDENTIFIER regex (/^[a-zA-Z_][a-zA-Z0-9_]*$/) to filter param keys before they are interpolated into generated JS code in the custom tool executor
  • Prevents potential code injection via maliciously crafted parameter key names
  • Keys that fail the identifier check are silently skipped (they can't be valid JS variable names anyway)

Credit: @Maxx191

Type of Change

  • Bug fix

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link
Copy Markdown

vercel Bot commented May 6, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 6, 2026 5:47pm

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 6, 2026

PR Summary

Medium Risk
Touches the custom-tool JavaScript execution path by changing how params are interpolated into generated code, which could affect tools that previously used non-identifier param names. The change reduces injection/SyntaxError risk but should be verified against existing custom tool inputs.

Overview
Prevents unsafe parameter keys from being interpolated into generated JavaScript when executing custom tools via isolated-vm.

Adds SAFE_IDENTIFIER + an ES2023 JS_RESERVED_WORDS denylist, filters/skips invalid keys during param destructuring, and logs a warning when a key is skipped to avoid code-injection and wrapper SyntaxErrors.

Reviewed by Cursor Bugbot for commit 507ccba. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 6, 2026

Greptile Summary

This PR hardens the custom tool executor by validating parameter key names before interpolating them into generated JS code in the isolated-vm path. Two complementary guards are added: a SAFE_IDENTIFIER regex that rejects any key not matching a valid JS identifier, and a JS_RESERVED_WORDS set that blocks keywords (return, const, class, etc.) that would produce a SyntaxError even though they match the identifier pattern.

  • Adds SAFE_IDENTIFIER regex and JS_RESERVED_WORDS set at module scope, then composes them into isSafeParamKey inside the handler.
  • Both code-generation sites (the wrapperLines builder and the codeToExecute paramDestructuring) now call isSafeParamKey consistently, and skipped keys are logged via logger.warn rather than dropped silently.
  • The E2B execution paths (JS and Python) serialize executionParams as JSON into a string literal, so they were never vulnerable to this class of injection and require no changes.

Confidence Score: 5/5

Safe to merge — the change is narrowly scoped to adding two input-validation guards that are applied consistently at both code-generation sites.

Both previously flagged issues (reserved-word bypass that caused SyntaxError at runtime, and silent key dropping with no observability) have been resolved. The JS_RESERVED_WORDS set covers the full ES2023 reserved-word list, isSafeParamKey composes the two checks correctly, and the warning log makes skipped keys visible. The E2B paths are unaffected because they serialise params as JSON rather than interpolating key names.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/app/api/function/execute/route.ts Adds SAFE_IDENTIFIER regex and JS_RESERVED_WORDS set to block unsafe param keys from being interpolated into generated JS; both isolated-vm code-generation sites are updated consistently; E2B paths use JSON serialisation and are unaffected.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[POST handler receives executionParams] --> B{isCustomTool?}
    B -- No --> F[Execute code directly]
    B -- Yes --> C[Iterate param keys]
    C --> D{isSafeParamKey check}
    D -- Valid identifier and not reserved --> E[Emit const binding]
    D -- Invalid or reserved word --> G[logger.warn and skip key]
    E --> H[wrapperLines for isolated-vm]
    E --> I[paramDestructuring in codeToExecute]
    H --> J[executeInIsolatedVM]
    I --> J
    J --> K[Return result to caller]
Loading

Reviews (2): Last reviewed commit: "fix(function): exclude JS reserved words..." | Re-trigger Greptile

Copy link
Copy Markdown
Collaborator Author

@waleedlatif1 waleedlatif1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both issues addressed in the follow-up commit (507ccba):

P1 (reserved keywords): Added a JS_RESERVED_WORDS set at module level covering all ES2023 reserved words and strict-mode future-reserved words. The isSafeParamKey helper now checks both the identifier regex and the reserved-word exclusion — so return, const, class, etc. are filtered out before interpolation.

P2 (silent skip): Skipped keys now emit a logger.warn with the key name and request ID, so failures are visible in logs rather than producing a silent ReferenceError inside the sandbox.

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

@waleedlatif1 waleedlatif1 merged commit 5d38222 into staging May 6, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/function-execute-param-injection branch May 6, 2026 17:57
waleedlatif1 added a commit that referenced this pull request May 7, 2026
…ion (#4474)

* fix(function): validate custom tool param keys before code interpolation

* fix(function): exclude JS reserved words from param key injection guard
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant