fix: refuse an empty or dot path id so a request cannot land on a sibling route - #20
Merged
Conversation
robrigo
force-pushed
the
fix/path-segment-guard
branch
from
August 17, 2026 22:49
8a2653c to
f3366c8
Compare
There was a problem hiding this comment.
Pull request overview
This PR hardens the Explorer API client’s URL construction by rejecting path segment values that can be normalized by the URL parser ('', ., ..) into sibling routes, preventing unintended reads from different endpoints.
Changes:
- Add a guarded
encodeSegment(value, field)that rejectsnull/undefined, empty strings, and dot segments before issuing a request. - Update Explorer API methods that place caller-supplied values into the path to pass a descriptive field name into the guard.
- Add tests that assert no fetch occurs on invalid segments and that dotted (non-dot-segment) names remain unchanged; document the behavior in the README.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
test/explorer-url.test.ts |
Adds regression tests ensuring invalid path segments reject before any fetch and that dotted names still work. |
src/API/Explorer/index.ts |
Introduces and wires in the path-segment guard via encodeSegment(value, field) across all path-building reads. |
README.md |
Documents the new rejection behavior in the 2.1.1 changelog section. |
Suppressed comments (1)
src/API/Explorer/index.ts:207
getTemplateStatsnames its second parameternamebut treats it as a template id (field label is "template id" and it’s placed in the template-id path position). This is confusing for API consumers and for future maintenance; rename the parameter toidto match the route and other template methods.
async getTemplateStats(collection: string, name: string): Promise<ITemplateStats> {
return await this.fetchEndpoint('/v1/templates/' + encodeSegment(collection, 'collection name') + '/' + encodeSegment(name, 'template id') + '/stats', {});
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
robrigo
force-pushed
the
fix/path-segment-guard
branch
from
August 17, 2026 22:53
f3366c8 to
6d40086
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/API/Explorer/index.ts:46
encodeSegmentguards againstnull/undefined, but its parameter is typed asstring, making those checks unreachable in TypeScript and potentially triggeringno-unnecessary-condition/misleading typing. Widen the parameter type so the signature matches the runtime behavior (especially for JS consumers).
function encodeSegment(value: string, field: string): string {
test/explorer-url.test.ts:170
- In the guarded-segment sweep, the label says "getTemplateStats name" but the method’s second parameter is an id. This label is used in assertion messages, so it should match the API surface for clarity.
['getTemplateStats collection', () => api.getTemplateStats('..', '1')],
['getTemplateStats name', () => api.getTemplateStats('mycollection', '..')],
['getTemplateLogs collection', () => api.getTemplateLogs('..', '1')],
README.md:193
- The 2.1.1 summary says "Rejects the path ids…", but the change also applies to collection/schema/template names and accounts. Consider making this line field-agnostic so the release note matches the behavior described below.
Rejects the path ids that would send a read to the wrong route.
…ling route Every path-building read percent-encoded its segments, but encodeURIComponent leaves dots alone, so an id of "." or ".." survived into the path and the URL parser inside fetch collapsed it onto the route above, and an empty id turned a single-row route into its list. The caller then read rows it never asked for instead of seeing a failure. Those values are never an id, a name, or an account, so the path builder now throws before the request, naming the field.
robrigo
force-pushed
the
fix/path-segment-guard
branch
from
August 17, 2026 22:56
6d40086 to
340df14
Compare
robrigo
marked this pull request as ready for review
August 17, 2026 23:11
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Every path-building read percent-encodes its ids, but
encodeURIComponentleaves dots alone, so an id of.or..survives into the path and the URL parser inside fetch collapses it onto the route above; an empty id turns a single-row route into its list. The caller reads rows it never asked for instead of seeing a failure. None of those values is ever an id, a name, or an account, so the path builder now throws before the request, naming the field, and a missing value is refused the same way. Dotted Antelope names such asalien.worldsandmycoll.wampass unchanged. Every other value keeps the existing percent-encoding.Validation
yarn check-types,yarn lint, andyarn testpass. The new tests drive'',.,..,null, andundefinedthrough a representative reader and assert the fetch stub records no call, sweep every path-building method with.., and pin that a dotted name still reaches the recorded URL unchanged.