Skip to content

Comments

fix(execution): scope X-Sim-Via header to internal routes and enforce depth limit#3313

Merged
waleedlatif1 merged 3 commits intostagingfrom
sim-636
Feb 23, 2026
Merged

fix(execution): scope X-Sim-Via header to internal routes and enforce depth limit#3313
waleedlatif1 merged 3 commits intostagingfrom
sim-636

Conversation

@waleedlatif1
Copy link
Collaborator

@waleedlatif1 waleedlatif1 commented Feb 23, 2026

Summary

  • Scoped X-Sim-Via header to internal routes only — previously it was attached to all outgoing HTTP requests, leaking internal workflow IDs to external third-party APIs
  • Removed cycle detection from validateCallChain — depth limit of 10 alone prevents infinite loops while allowing legitimate self-recursion (pagination, tree traversal, batch splitting)
  • Added depth validation in workflow-handler.ts before spawning child executors — previously in-process child workflows skipped validation entirely
  • Removed unsafe (params as any)._context type bypass in request.ts by moving header injection to tools/index.ts where _context is already accessed

Type of Change

  • Bug fix

Testing

  • 23 unit tests for call-chain utilities (parse, serialize, validate, build, round-trip)
  • All 4133 existing tests pass with no regressions
  • TypeScript compiles cleanly

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

vercel bot commented Feb 23, 2026

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Feb 23, 2026 11:05pm

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 23, 2026

Greptile Summary

This PR fixes a security issue where the X-Sim-Via header (containing internal workflow IDs) was being attached to all outgoing HTTP requests, including those to external third-party APIs. The header is now scoped to internal routes only via the isInternalRoute check in tools/index.ts. Additionally, cycle detection was removed from validateCallChain in favor of depth-only validation (max 10), which allows legitimate self-recursion patterns (pagination, tree traversal). Depth validation was also added to the in-process child workflow path in workflow-handler.ts, which previously skipped validation entirely.

  • Security fix: X-Sim-Via header injection moved from tools/http/utils.ts (all requests) to tools/index.ts gated behind isInternalRoute, preventing internal workflow ID leakage to external services
  • Depth validation on child workflows: workflow-handler.ts now validates childCallChain (after appending current workflow ID) before spawning sub-executors, closing a gap where inline child workflows bypassed depth checks
  • Removed unsafe type bypass: Eliminated (params as any)._context in request.ts by moving header injection to tools/index.ts where _context is already accessed as Record<string, any>
  • Call chain threading: callChain is now properly propagated through all three execution paths (SSE, non-SSE, async) via ExecutionMetadata, ContextExtensions, and ExecutionContext
  • Well-tested: 23 unit tests cover all call-chain utility functions with good edge case coverage

Confidence Score: 4/5

  • This PR is safe to merge — it fixes a real security issue (header leakage) and adds proper depth validation to a previously unguarded code path.
  • Score of 4 reflects that the changes are well-structured, address real security and correctness issues, have comprehensive test coverage for the core utilities, and the call chain is properly threaded through all execution paths. The one minor concern is the as string[] | undefined cast in tools/index.ts, but this is consistent with existing patterns in the file and noted by the developer as a broader refactor opportunity.
  • apps/sim/tools/index.ts has a type cast for callChain that could benefit from proper typing in a follow-up, but is consistent with existing _context access patterns in the file.

Important Files Changed

Filename Overview
apps/sim/lib/execution/call-chain.ts New utility module with clean, well-tested functions for parsing, serializing, validating, and building call chains. Simple depth-only validation (no cycle detection) with MAX_CALL_CHAIN_DEPTH=10.
apps/sim/lib/execution/tests/call-chain.test.ts Comprehensive test suite covering all call-chain utility functions: parse, serialize, validate, build, round-trip identity. 23 test cases with good edge case coverage.
apps/sim/tools/index.ts Moved X-Sim-Via header injection from HTTP tool layer to here, scoped behind isInternalRoute check. Properly prevents header leaking to external APIs. Uses params._context?.callChain cast to `string[]
apps/sim/executor/handlers/workflow/workflow-handler.ts Added depth validation before spawning child executors. Correctly validates childCallChain (after adding current workflow) instead of parent chain. Throws ChildWorkflowError on depth exceeded.
apps/sim/app/api/workflows/[id]/execute/route.ts Added ingress validation of call chain from X-Sim-Via header. Parses, validates, and builds next chain before execution. Passes callChain through all three execution paths (SSE, non-SSE, async).
apps/sim/app/api/mcp/serve/[serverId]/route.ts Forwards X-Sim-Via header from incoming MCP request to the execute endpoint. Clean pass-through design — validation happens at the execute route.
apps/sim/background/workflow-execution.ts Added callChain to WorkflowExecutionPayload type and passes it through to ExecutionMetadata for background/async job execution path.
apps/sim/executor/execution/executor.ts Threads callChain from contextExtensions into the ExecutionContext. One-line addition.
apps/sim/executor/execution/types.ts Added callChain property to both ExecutionMetadata and ContextExtensions interfaces with proper JSDoc comments.
apps/sim/executor/handlers/api/api-handler.ts Passes callChain from execution context to tool _context so it's available for header injection in tools/index.ts.
apps/sim/executor/types.ts Added callChain property to ExecutionContext interface with JSDoc documentation.
apps/sim/lib/workflows/executor/execution-core.ts Threads callChain from metadata into contextExtensions so it reaches the Executor.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Incoming Request\n(API or MCP)"] --> B["Parse X-Sim-Via Header\n(parseCallChain)"]
    B --> C{"Validate Depth\n>= 10?"}
    C -->|Yes| D["Return 409\nDepth Exceeded"]
    C -->|No| E["Build Next Chain\n(append workflowId)"]
    E --> F["Execute Workflow"]
    F --> G{"Child Workflow\n(inline)?"}
    G -->|Yes| H["Build Child Chain\n(append childWorkflowId)"]
    H --> I{"Validate Child\nDepth >= 10?"}
    I -->|Yes| J["Throw ChildWorkflowError"]
    I -->|No| K["Execute Child\n(with childCallChain)"]
    G -->|No| L{"Outgoing HTTP\nRequest?"}
    L -->|Internal Route| M["Set X-Sim-Via Header\n(tools/index.ts)"]
    L -->|External Route| N["No X-Sim-Via Header\n(prevents ID leakage)"]
    M --> A
Loading

Last reviewed commit: 27e9560

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

13 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

…ld workflow depth validation

- Move call chain header injection from HTTP tool layer (request.ts/utils.ts)
  to tool execution layer (tools/index.ts) gated on isInternalRoute, preventing
  internal workflow IDs from leaking to external third-party APIs
- Remove cycle detection from validateCallChain — depth limit alone prevents
  infinite loops while allowing legitimate self-recursion (pagination, tree
  processing, batch splitting)
- Add validateCallChain check in workflow-handler.ts before spawning child
  executor, closing the gap where in-process child workflows skipped validation
- Remove unsafe `(params as any)._context` type bypass in request.ts

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@waleedlatif1
Copy link
Collaborator Author

@cursor review

@waleedlatif1
Copy link
Collaborator Author

@greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

12 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Validate childCallChain (after appending current workflow ID) rather
than ctx.callChain (parent). Prevents an off-by-one where a chain at
depth 10 could still spawn an 11th workflow.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@waleedlatif1 waleedlatif1 changed the title feat(execution): workflow cycle detection via X-Sim-Via header fix(execution): scope X-Sim-Via header to internal routes and enforce depth limit Feb 23, 2026
@waleedlatif1
Copy link
Collaborator Author

@greptile

@waleedlatif1
Copy link
Collaborator Author

@cursor review

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

12 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

@waleedlatif1 waleedlatif1 merged commit 9166649 into staging Feb 23, 2026
12 checks passed
@waleedlatif1 waleedlatif1 deleted the sim-636 branch February 23, 2026 23:19
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