Skip HTTP workflow route matching outside base path

[sfmskywalker]

Summary

• Short-circuit HTTP workflow middleware before route matching when the request is outside the configured base path.
• Preserve route matching and base-path stripping for requests inside the base path.
• Add middleware unit coverage for both outside-base-path bypass and inside-base-path route resolution.

Fixes #7709

Tests

• dotnet test test/unit/Elsa.Http.UnitTests/Elsa.Http.UnitTests.csproj

[greptile-apps]

Confidence Score: 5/5

The change is narrowly scoped to HTTP workflow path filtering and is covered by focused unit tests for outside-path bypass and inside-path matching.

What we checked:

• The before-state contract validation was attempted with the exact command, working directory, script content, target commit, middleware excerpt, and the selected test names, using the attempted verbose test command; the run ended with exit code 127.
• The after-state contract validation was attempted with the same setup, and it also ended with exit code 127 due to the same environment limitation.
• Two log artifacts were captured to document the attempted validations and the surrounding command and environment details.

Summary

• Skip HTTP workflow route matching when requests are outside the base path.
• Keep route matching and base-path stripping for requests under the base path.
• Add tests for outside-path bypass, sibling-prefix bypass, and tenant PathBase handling.

_{Reviews (6) · Last reviewed commit: "Use resolved tenant paths for HTTP routi..."}

[sfmskywalker]

@copilot resolve the merge conflicts in this pull request

[sfmskywalker]

@copilot Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

Issue 1 of 1

src/modules/Elsa.Http/Middleware/HttpWorkflowsMiddleware.cs:48-51
Tenant routes are skipped

This early check compares the raw request path to BasePath before route matching. In route-prefix multitenancy, workflow routes are registered as /{tenantPrefix}/workflows/... while BasePath remains /workflows, so a valid request such as /acme/workflows/foo does not start with /workflows. This branch then calls next and returns before GetMatchingRoute can match the tenant-prefixed route, so tenant-prefixed HTTP workflows stop starting or resuming.

[Copilot]

@copilot resolve the merge conflicts in this pull request

Addressed in f7317c8.

[Copilot]

@copilot Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

...

Addressed in 1e255f6.

[Copilot]

🟡 Changes recommended

A critical unresolved issue could allow unrelated prefixed URLs to execute a workflow.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This pull request adds HTTP workflow base-path gating while preserving valid route resolution and adding middleware test coverage.

Changes:

• Bypasses matching outside the configured base path.
• Preserves direct and tenant-prefixed routing.
• Adds unit tests for bypass and route resolution.

File summaries

File	Summary
test/unit/Elsa.Http.UnitTests/Middleware/HttpWorkflowsMiddlewareTests.cs	Adds middleware behavior tests.
src/modules/Elsa.Http/Middleware/HttpWorkflowsMiddleware.cs	Implements base-path gating and route handling; a critical issue remains with validating tenant-prefixed matches.

Review details

Suppressed comments (1)

src/modules/Elsa.Http/Middleware/HttpWorkflowsMiddleware.cs:321

• CouldContainBasePath considers a match at any segment index. As a result, requests such as /api/v1/workflows/status—which are outside the configured base path and cannot use the supported one-segment tenant prefix—still call GetMatchingRoute and scan the route table before TryStripBasePath rejects the match. Restrict this pre-check to a base path at segment 0 or with exactly one leading tenant segment, and add a matcher-invocation assertion to the non-tenant regression test.

    private static bool CouldContainBasePath(string route, string basePath) =>
        FindSegmentSequence(GetRouteSegments(route), GetRouteSegments(basePath)) >= 0;

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[sfmskywalker]

@greptile review

Disposition for the two findings in the latest 4/5 summary:

• Restore token on faults — valid baseline concern, but unchanged by this PR and outside Skip Elsa route matching before base-path gating in HTTP workflow middleware #7709. It was separately addressed by merged PR Restore RequestAborted after timed HTTP workflow failures #7712 (fixing Restore HttpContext.RequestAborted after timed workflow execution failures #7706). The Skip Elsa route matching before base-path gating in HTTP workflow middleware #7709 branch now matches current origin/main in ExecuteWithinTimeoutAsync; reintroducing that unrelated fix here would broaden this PR.
• Handle missing fault state — likewise valid baseline concern, unchanged by this PR and outside Skip Elsa route matching before base-path gating in HTTP workflow middleware #7709. It was separately addressed by merged PR Guard HTTP fault handling when workflow reload returns null #7714 (fixing Guard HTTP workflow fault handling when workflow instance reload returns null #7707). The Skip Elsa route matching before base-path gating in HTTP workflow middleware #7709 branch now matches current origin/main in HandleWorkflowFaultAsync.

This PR intentionally contains only the base-path route-gating change and directly related tests. Please reassess the current-head diff on that scope; the prior sibling-prefix finding is fixed in e118017 and its thread is resolved.

[sfmskywalker]

Fixed Copilot’s current-head finding in dfa7ee3. The precheck now permits the configured base path only directly or after exactly one leading tenant segment. The new /api/v1/workflows/status regression registers a concrete route and asserts that IRouteMatcher.Match is never called. Focused middleware tests pass 6/6 and the full Elsa.Http.UnitTests project passes 36/36 on .NET 10.

@greptile review

Please reassess only the current-head diff. The two exception-path concerns from the preceding Greptile summary are unchanged from current main, absent from this PR diff, and remain outside issue #7709.

[Copilot]

🟡 Changes recommended

Validate tenant prefixes and add an unknown-prefix regression test before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.

[sfmskywalker]

@greptile review

Current head d360307 removes unsafe raw tenant-prefix inference and short-circuits every Request.Path outside the configured BasePath before route matching. Valid tenant routing is preserved in its source-grounded post-resolution PathBase/Path form. Current-source tests pass 7/7 focused and 37/37 for the full Elsa.Http.UnitTests project.

Please assess the current-head diff for issue #7709 only. The two exception-path concerns previously reported are unchanged from current main and absent from this PR diff.

[Copilot]

🟢 Approved

No unresolved review issues remain, and the relevant bypass and routing behaviors are covered.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.