#200
Pull Requests/monid-ai/monid/#200

fix(engine): reject empty path params before url substitution

Opensuzhiguo7wants to merge intofrom
1 commit2 files changed+70-2

Fixes #119.

What & why

An empty string supplied for a {pathParam} placeholder passed schema validation — the base RunInput path-param shape is z.string() with no length rule, and connector schemas don't consistently add minLength — and was substituted into the url, producing an empty path segment: an endpoint compiled from /v1/items/{id}/sub with pathParams.id = "" issued https://api.example.test/v1/items//sub. An empty segment can address a different vendor route (e.g. a list route), and that request goes out with the endpoint's credentials.

The change

substituteUrl (engine/request.ts) now treats an empty placeholder value like a missing one: INVALID_INPUT before any request is built. The guard sits at the engine boundary, shared by the declarative pipeline (buildRequest) and the lifecycle ctx's data.request, so every caller gets the same rule regardless of each connector's own schema strictness.

Tests (engine/engine.test.ts)

New "path substitution" section, mirroring the query section's shape:

  • a non-empty id substitutes into the url (positive control);
  • an empty id is INVALID_INPUT, asserting the transport is never called — no request leaves the engine (seen.url stays undefined);
  • a missing id stays INVALID_INPUT (covers the pre-existing guard).

The new empty-value test fails on main and passes with this change.

Verification

  • deno test --allow-read --allow-env --allow-write engine/ — 103 passed (100 before + 3 new)
  • deno test on connectors/clay/endpoints/search/query-mode-run/endpoint.test.ts and connectors/orbit/endpoints/profile-read/endpoint.test.ts — the only two tests in the repo that pass an empty path param, both negative controls asserting INVALID_INPUT; both still pass
  • deno check engine/mod.ts engine/request.ts engine/engine.test.ts, deno lint engine/, deno fmt — clean
  • repo-wide grep: no other caller passes an empty path param

Summary by CodeRabbit

  • Bug Fixes
    • URL path parameters with empty values are now rejected, matching the existing handling of missing values.
    • Requests with an empty path parameter are not sent and return an invalid-input error.

Activity

2
S
fix(engine): reject empty path params before url substitution
7fb1d14
coderabbitai[bot]6d ago

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 09d54479142e36eff876383beacfbd37b33b5908 and 7fb1d141991e8bedf88900ef0b32b890693c0597.

Files selected for processing: 2
  • engine/engine.test.ts
  • engine/request.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Walkthrough

substituteUrl now rejects empty path parameter values with INVALID_INPUT, in addition to rejecting missing values. Tests cover non-empty substitution, empty-value rejection without sending a request, and missing-value rejection.

Priority: Normal

Severity of issue fixed: Medium

Merge Risk: Minimal · up to 7fb1d

The requested validation is implemented and covered by tests, with no remaining merge-blocking risk identified.

Pre-merge checks: 4
Check nameStatusExplanation
Description CheckCheck skipped - CodeRabbit’s high-level summary is enabled.
Title checkThe title clearly and concisely describes the main change: rejecting empty path parameters before URL substitution.
Linked Issues checkIssue #119 requires empty path-placeholder values to fail with INVALID_INPUT before URL substitution. engine/request.ts now rejects value === "" together with missing values in substituteUrl. Th…
Out of Scope Changes checkThe changes stay within issue #119. The substituteUrl guard implements the requested behavior. The three engine tests provide the required regression coverage and positive control.
Full details: Linked Issues check

Explanation

Issue #119 requires empty path-placeholder values to fail with INVALID_INPUT before URL substitution. engine/request.ts now rejects value === "" together with missing values in substituteUrl. The new tests cover non-empty substitution, empty-value rejection, no transport call, and missing-value rejection.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.