subPath appended to URLs without encoding or type constraint

[Sam-Bolling]

Finding

buildResourceUrl() in url_builder.ts and buildNestedCommandUrl() in command-routing.ts both append a subPath parameter to URLs without encoding or validating it. All 47 current call sites use hardcoded string literals (safe today), but the subPath parameter is typed as string with no enforcement — a future contributor passing user-controlled data here has no guard.

Review Source: Senior developer code review of clean-pr — docs/code-review/004-pending-p2-subpath-no-encoding.md
Severity: P2-Important
Category: Security / Type Safety
Ownership: Ours

---

Problem Statement

buildResourceUrl is a private method on CSAPIQueryBuilder that constructs every CSAPI resource URL. It encodes the id parameter via encodeResourceId() (a thin encodeURIComponent wrapper) but appends subPath raw:

Affected code:

// src/ogc-api/csapi/url_builder.ts, line 270
private buildResourceUrl(
  resourceType: string,
  id?: string,
  subPath?: string,    // ← typed as bare string
  options?: QueryOptions
): string {
  // ...
  if (id) url += `/${encodeResourceId(id)}`;
  if (subPath) url += `/${subPath}`;  // ← no encoding, no validation
  return url + this.buildQueryString(options);
}

The same pattern appears in the exported buildNestedCommandUrl:

// src/ogc-api/csapi/command-routing.ts, line 160
if (commandId) url += `/${encodeResourceId(commandId)}`;
if (subPath) url += `/${subPath}`;  // ← same gap

Scenario:

// A future contributor passes user input as subPath:
builder.buildResourceUrl('systems', systemId, userInput);
// If userInput = '../../../admin', URL becomes:
// https://server.example.com/api/systems/abc/../../../admin
// → path traversal to https://server.example.com/admin

Impact: No runtime bug today — all 47 buildResourceUrl calls and all test-only buildNestedCommandUrl calls pass hardcoded literals from a finite set of 19 known values. The risk is architectural: the string type provides no guardrail, and the asymmetry with encodeResourceId(id) makes the omission look accidental rather than deliberate.

Known sub-paths used today (19 unique values):

Sub-path	Call count	Used for
'history'	8	Systems, deployments, procedures, samplingFeatures, properties, datastreams, observations, controlStreams
'systems'	6	Deployments→systems, procedures→systems, etc.
'datastreams'	4	Systems→datastreams, procedures→datastreams, properties→datastreams
'controlstreams'	3	Systems→controlstreams, properties→controlstreams
'procedures'	3	Systems→procedures, datastreams→procedures, controlStreams→procedures
'observations'	3	Datastreams→observations, samplingFeatures→observations
'commands'	3	ControlStreams→commands
'subsystems'	2	Systems→subsystems
'subdeployments'	2	Deployments→subdeployments
'samplingFeatures'	2	Systems→samplingFeatures
'schema'	2	Datastreams→schema, controlStreams→schema
'status'	2	Commands→status
'deployments'	1	Systems→deployments
'feasibility'	1	ControlStreams→feasibility
'datastream'	1	Observations→datastream (singular)
'samplingFeature'	1	Observations→samplingFeature (singular)
'system'	1	Observations→system (singular)
'result'	1	Commands→result
'cancel'	1	Commands→cancel

Additionally, 40 calls pass undefined or omit subPath entirely.

Ownership Verification

$ git diff upstream/main clean-fork/clean-pr -- src/ogc-api/csapi/url_builder.ts | grep "subPath"
+    subPath?: string,
+    if (subPath) url += `/${subPath}`;

$ git diff upstream/main clean-fork/clean-pr -- src/ogc-api/csapi/command-routing.ts | grep "subPath"  
+  subPath?: string,
+  if (subPath) url += `/${subPath}`;

Conclusion: This code is ours. Neither file exists on upstream/main — the entire csapi/ directory is our contribution.

Files to Modify

File	Action	Est. Lines	Purpose
src/ogc-api/csapi/url_builder.ts	Modify	~5	Change subPath parameter type from string to union type
src/ogc-api/csapi/command-routing.ts	Modify	~10	Change subPath parameter type to union type AND add runtime allowlist validation

No test file changes required — all existing call sites already pass valid literal values that will match the union type.

Proposed Solutions

Option A: Validate subPath against a runtime allowlist

const ALLOWED_SUB_PATHS = new Set([
  'history', 'subsystems', 'subdeployments', 'systems', 'deployments',
  'samplingFeatures', 'procedures', 'datastreams', 'controlstreams',
  'observations', 'commands', 'schema', 'feasibility',
  'datastream', 'samplingFeature', 'system',
  'status', 'result', 'cancel',
]);

if (subPath) {
  if (!ALLOWED_SUB_PATHS.has(subPath)) {
    throw new EndpointError(`Invalid subPath: "${subPath}"`);
  }
  url += `/${subPath}`;
}

Pros: Catches invalid values at runtime regardless of TypeScript compilation. Defense-in-depth for exported functions where callers may not be under compile-time control.
Cons: Runtime check for something TypeScript can also enforce at compile time. Adds a Set allocation and lookup on every URL construction call.
Effort: Small | Risk: None

Option B: encodeURIComponent on subPath

if (subPath) url += `/${encodeURIComponent(subPath)}`;

Pros: Simple one-character-level fix. encodeURIComponent would encode samplingFeatures to samplingFeatures (no change for ASCII-only values), but any accidental special char in a future value would be encoded rather than rejected.
Cons: Encoding doesn't reject invalid sub-paths — it silently produces URLs like .../systems/abc/%2E%2E%2Fadmin that will 404 on any OGC server but won't flag the error at the source. Treats the symptom, not the cause.
Effort: Trivial | Risk: None

Not recommended. Encoding an invalid sub-path doesn't make it valid — it just makes it silently wrong instead of loudly wrong.

Option C: Change subPath to a discriminated union type

// url_builder.ts — near the top with other type definitions
type ResourceSubPath =
  | 'history' | 'subsystems' | 'subdeployments' | 'systems' | 'deployments'
  | 'samplingFeatures' | 'procedures' | 'datastreams' | 'controlstreams'
  | 'observations' | 'commands' | 'schema' | 'feasibility'
  | 'datastream' | 'samplingFeature' | 'system'    // singular (observation navigation)
  | 'status' | 'result' | 'cancel';                // command sub-paths

private buildResourceUrl(
  resourceType: string,
  id?: string,
  subPath?: ResourceSubPath,
  options?: QueryOptions
): string { /* ... unchanged ... */ }

// command-routing.ts
type CommandSubPath = 'status' | 'result' | 'cancel';

export function buildNestedCommandUrl(
  builder: CSAPIQueryBuilder,
  controlStreamId: string,
  commandId?: string,
  subPath?: CommandSubPath,
  options?: CommandQueryOptions
): string { /* ... unchanged ... */ }

Pros: Compile-time enforcement — TypeScript rejects any string not in the union at the call site. Zero runtime cost. The set is finite, spec-derived, and stable.
Cons: Adding a new OGC sub-path in the future requires updating the union type (trivial, and correct — it forces the developer to acknowledge the new path). Private method union adds type verbosity.
Effort: Small | Risk: None

Recommended Action

Option C (union type) on both functions, combined with Option A (runtime allowlist) on buildNestedCommandUrl for defense-in-depth.

Rationale:

• buildResourceUrl is private — compile-time union enforcement is sufficient since all callers are within the same class under our control.
• buildNestedCommandUrl is exported — even though it is not currently re-exported from the barrel file (csapi/index.ts) and is only called from tests, an exported function is one refactor away from being called externally. The runtime allowlist provides a safety net beyond compile-time checking.
• This matches the original code review recommendation: "Option C (union type) for the buildResourceUrl private method — compile-time safety is free and eliminates any future path injection by construction. Combine with Option A's runtime allowlist for defense-in-depth in command-routing.ts where the subPath arrives via a public function parameter."

Scope — What NOT to Touch

• ❌ Do NOT modify any file outside src/ogc-api/csapi/ — per the upstream maintainer's isolation requirement
• ❌ Do NOT modify files outside the "Files to Modify" table above
• ❌ Do NOT change the resourceType parameter (also typed as string — separate concern)
• ❌ Do NOT change encodeResourceId or its usage on the id parameter
• ❌ Do NOT refactor buildResourceUrl's URL construction logic — only the subPath type
• ❌ Do NOT change the existing QueryOptions or CommandQueryOptions types

Acceptance Criteria

• subPath parameter on buildResourceUrl is typed as a union (not string)
• subPath parameter on buildNestedCommandUrl is typed as a union (not string) AND validated against a runtime allowlist
• A caller passing an unexpected subPath value gets a compile-time error on both functions, and additionally a runtime error on buildNestedCommandUrl
• All existing call sites still compile without changes (they already pass valid literals)
• Existing tests pass (npm test)
• No lint errors (npm run lint)
• All modified files pass npx prettier --check

Dependencies

Blocked by: Nothing
Blocks: Nothing
Related: #141 — parseCollectionResponse type safety (same pattern of tightening types at internal boundaries)

---

Operational Constraints

⚠️ MANDATORY: Before starting work on this issue, review docs/governance/AI_OPERATIONAL_CONSTRAINTS.md.

Key constraints:

• Precedence: OGC specifications → AI Collaboration Agreement → This issue description → Existing code → Conversational context
• No scope expansion: Fix the type, nothing more
• Minimal diffs: Prefer the smallest change that satisfies the acceptance criteria
• Ask when unclear: If intent is ambiguous, stop and ask for clarification

Upstream Isolation Constraint

Per the upstream maintainer's comment on PR #136:

• Anything in src/ogc-api/csapi must NOT be included in the root index.ts
• Anything NOT in src/ogc-api/csapi must NOT import from CSAPI code
• All files in this issue's "Files to Modify" table are within src/ogc-api/csapi/ — this fix is purely internal to the isolated CSAPI module

Ownership-Specific Constraints

Ownership: Ours

• Fix on the working branch
• Include in the next commit to clean-pr if the PR is still open
• Add tests that cover the finding

---

References

Original Code Review Finding

#	Document	What It Provides
0	docs/code-review/004-pending-p2-subpath-no-encoding.md	The original finding from the senior developer code review. Authoritative source for the problem statement, all three proposed solutions (A=allowlist, B=encode, C=union), and the combined recommendation (C+A).

Directly Affected Code

#	Document	What It Provides
1	src/ogc-api/csapi/url_builder.ts line 270	buildResourceUrl — private method with subPath?: string parameter
2	src/ogc-api/csapi/command-routing.ts line 160	buildNestedCommandUrl — exported function with subPath?: string parameter
3	src/ogc-api/csapi/helpers.ts line 97	encodeResourceId — the encoding function already used for id but NOT for subPath

OGC Specifications — URL Path Segment Definitions

#	Document	What It Provides
4	OGC API — Connected Systems Part 1 (23-001)	Defines the nested resource URL patterns (e.g., /systems/{id}/subsystems, /systems/{id}/history) — the spec source for all 19 sub-path values
5	OGC API — Connected Systems Part 2 (23-002)	Defines Part 2 URL patterns (e.g., /datastreams/{id}/observations, /controlstreams/{id}/commands/{id}/status)
6	OGC API — Features (17-069r4)	Base URL construction patterns inherited by CSAPI
7	OGC API — Common (19-072)	Foundation specification for all OGC API URL patterns — defines standard pagination, link patterns, and path conventions that CSAPI extends
8	CSAPI Part 1 OpenAPI Spec	Machine-readable endpoint paths for all Part 1 resources — definitive source for valid URL path segments
9	CSAPI Part 2 OpenAPI Spec	Machine-readable endpoint paths for Part 2 resources — schema, observations, commands, status, result, cancel paths
10	GeoJSON (RFC 7946)	FeatureCollection URL patterns — Part 1 resources use Features API paths that CSAPI extends with nested sub-paths
11	IANA Link Relations	Link relation types that semantically map to sub-path navigation — system, deployment, procedure, datastream, observations etc. correspond to the sub-path values

URL Building & Architecture Research

#	Document	What It Provides
12	URL Building Architecture	Critical context — documents URL building patterns in ogc-client including base URL strategy, query parameter encoding, nested resource path building (e.g., /systems/{id}/subsystems), and array parameter handling. Directly relevant to the subPath construction pattern.
13	QueryBuilder Pattern Analysis	Documents the QueryBuilder pattern lifecycle, state management, and URL building — buildResourceUrl is the core URL construction method in the QueryBuilder
14	Architecture Patterns Analysis	Upstream architectural patterns for maintaining consistency — URL construction should follow established patterns
15	PR #114 (EDR Implementation) Analysis	Direct blueprint pattern — how EDR handles URL construction in its QueryBuilder; reference for how upstream accepted URL building patterns
16	Code Reuse vs Duplication Strategy	Isolation requirements and import guidelines — governs how buildResourceUrl relates to upstream URL utilities
17	Integration with Existing Code	Integration requirements and shared utility reuse — relevant to understanding how URL building fits in the overall architecture
18	CSAPI Architecture Decisions	Architectural choices for 9 CSAPI resource types — sub-resource handling strategy that drives the sub-path design
19	camptocamp/ogc-client	Upstream library — source of URL building patterns we follow; EDR QueryBuilder is the direct precedent

Type System & Design Research

#	Document	What It Provides
20	TypeScript Type System Design	Type safety enforcement strategies; union type patterns for constrained string parameters
21	Data Type and Schema Requirements	50+ TypeScript interfaces; union type patterns applicable to ResourceSubPath and CommandSubPath type design
22	CSAPIQueryBuilder Architecture Decisions	Architecture decision series for CSAPIQueryBuilder — includes scope analysis that defines which sub-paths each resource type supports
23	Collections Reader Research	Collection discovery and URL patterns — how CSAPI resources integrate with collection navigation
24	Design Strategy Research	High-level design strategy — format abstraction layer architecture and URL construction approach
25	OGCAPIEndpoint Integration Research	Factory method integration pattern — how the URL builder connects to the endpoint discovery layer

Requirements Research — Sub-Resource Navigation & URL Construction

#	Document	What It Provides
26	Sub-Resource Navigation Requirements	Critical context — documents ALL nested navigation patterns including relationship endpoints, nesting depth, and bidirectional navigation. This is the authoritative source for the 19 sub-path values and why they exist.
27	Query Parameter Requirements	URL encoding rules, parameter validation requirements, and parameter combination logic — establishes encoding expectations for URL segments
28	CSAPI Part 1 Requirements	Complete Part 1 spec analysis — sub-resource navigation patterns for Systems, Deployments, Procedures, Sampling Features, Properties
29	CSAPI Part 2 Requirements	Complete Part 2 spec analysis — URL patterns for DataStreams, Observations, Control Streams, Commands including schema, status, result, cancel sub-paths
30	CRUD Operations Requirements	HTTP method + URL pattern requirements — documents which sub-paths support which HTTP methods
31	Conformance and Capability Requirements	Conformance class detection — which sub-paths are available depends on server conformance
32	Usage Scenarios and Priorities	15 core usage scenarios — documents the real-world workflows that exercise sub-path URL construction
33	Full Implementation Scope Definition	Production-ready implementation goals — format abstraction and URL construction requirements

Client Library Pattern Research

#	Document	What It Provides
34	OSHConnect-Python Analysis (Detailed)	Builder pattern reference — how Python client constructs URLs for sub-resource navigation
35	OWSLib Analysis	Mature client library URL construction patterns — class-per-resource architecture with typed URL building
36	oscar-viewer Analysis	Real-world TypeScript CSAPI URL usage — how a production app constructs sub-resource URLs
37	osh-viewer Analysis	Vue.js client URL patterns — System → Datastreams → Observations navigation URL construction
38	Upstream Library Expectations	What camptocamp/ogc-client expects — URL building pattern alignment with existing EDR/WFS/WMS implementations

Error Handling & Server Compatibility

#	Document	What It Provides
39	Known Server Quirks	Server non-compliant behaviors — validates importance of defensive URL construction (e.g., OSH returns non-standard link patterns)
40	Error Handling Design Analysis	Error handling patterns — relevant to Option A's EndpointError throw strategy
41	52°North Server Analysis	Server capability variations — URL pattern differences between implementations
42	OpenSensorHub Server Analysis	Primary test server — real-world URL patterns for validating sub-path construction
43	Gap Analysis from Previous Iteration	Lessons from v1 failure — URL construction was part of what needed improvement
44	Lessons Learned from Previous Iterations	Mistakes to avoid; under-engineering gaps — relevant to why string was left untyped
45	Cross-Server Interoperability Analysis	URL pattern behavior across multiple servers — validates that sub-path values are server-consistent

Testing References

#	Document	What It Provides
46	Testing Strategy Research	Overall testing approach and quality gates
47	Testing Plan 12 — QueryBuilder Testing Strategy	Directly relevant — testing patterns for QueryBuilder methods including URL construction
48	Testing Plan 13 — Resource Method Testing Patterns	Resource method test patterns — how to verify URL output from sub-resource navigation methods
49	Testing Plan 18 — Error Condition Testing	Error condition coverage — relevant to testing invalid subPath rejection (Option A runtime check)
50	Test Fixtures Guide	Fixture creation and organization conventions

Planning & Implementation Context

#	Document	What It Provides
51	CSAPI Implementation Guide	Architecture reference — URL builder design requirements and format handler integration
52	Contribution Goal and Definition	PR acceptance criteria — quality bar for URL construction
53	Phase 6: Implementation Guide	Architectural decoupling specifications — isolation enforcement relevant to buildResourceUrl being private and buildNestedCommandUrl being internal-only
54	Phase 6: Contribution Goal	Upstream acceptance readiness — quality requirements for the final PR
55	CSAPI Implementation Roadmap	Master roadmap — phase progression context

Implementation Records — Prior Review Context

#	Document	What It Provides
56	Final Project Code Review	End-to-end review that should have caught this — context on what was/wasn't reviewed
57	Outstanding Findings Status Report	Outstanding findings context — where this finding fits among other open items
58	Deferred Findings Final Disposition	Deferred findings disposition — confirms what was/wasn't addressed before this code review
59	Phase 5.3 Code Review	Schema response parsers review — reviewed the format handlers that call buildResourceUrl with sub-paths
60	Phase 5.4 Code Review	Code review finding resolutions + smoke test results

Governance

#	Document	What It Provides
61	AI Operational Constraints	Mandatory operational constraints for implementation
62	AI Collaboration Agreement	Foundational governance for AI-assisted development — quality standards
63	Phase 3 Lessons Learned	Format handler development insights — URL construction is called from format handlers
64	Phase 2 Lessons Learned	Implementation velocity lessons and code review process improvements

Live Infrastructure & Upstream PR

#	Document	What It Provides
65	OpenSensorHub Demo Server	Live server for validating URL construction against real endpoints
66	52°North Demo Server	Secondary server (degraded) for multi-server URL pattern validation
67	Upstream PR #136	The upstream contribution — jahow's isolation requirements that govern buildResourceUrl visibility
68	OS4CSAPI Fork (clean-pr branch)	Source repo for PR #136 — where the fix will be applied

Demo Application

#	Document	What It Provides
69	Demo App Findings	Real-world integration testing findings — especially issue-14 (non-standard links) which exercises sub-resource URL navigation
70	CSAPI Demo Webapp Assessment	Demo app coverage of CSAPI features — validates which sub-resource URL patterns are exercised in real use