fix: reject invalid URL matches - #117
Conversation
Follow up to: - #115 - Route resolution preserves empty path segments. - An exact match requires a URL that resolves to an OpenAPI route.
There was a problem hiding this comment.
🟡 Changes recommended
One existing test expectation (isUrlMatch('', '') === true) appears inconsistent with the PR’s stated new semantics and is likely to fail unless the OpenAPI spec defines a root / path.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR tightens URL allowlist matching by requiring that an “exact” URL match is only accepted when the URL can be resolved to a known OpenAPI route, and by ensuring route resolution does not collapse empty path segments (e.g., //).
Changes:
- Preserve empty path segments during route segmentation to prevent accidental matches when URLs contain
//. - Require a URL to resolve to an OpenAPI route before allowing an exact-match return path in
isUrlMatch. - Add unit coverage for rejecting exact matches on unknown endpoints and for rejecting URLs with empty path segments.
File summaries
| File | Description |
|---|---|
unit/index.test.ts |
Adds coverage for rejecting unknown endpoints and URLs containing empty path segments. |
src/index.ts |
Updates URL segmentation and matching to preserve empty segments and require successful OpenAPI route resolution for exact matches. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟢 Approval recommended
The changes are small, well-covered by targeted unit tests, and align with the stated matching semantics without introducing observable regressions in the reviewed code paths.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
Follow up to:
Implementation