SensorML parsers spread raw JSON into typed results — bypasses type system

[Sam-Bolling]

Finding

physical-system.ts, aggregate-process.ts, and simple-process.ts all construct their typed result objects by spreading raw (unvalidated) server JSON via ...(json as Record), then coercing required fields with as string and using a delete-then-reassign mutation pattern to overwrite managed keys.

Review Source: Senior developer code review of clean-pr
Severity: P2-Important
Category: Type Safety
Ownership: Ours

---

Problem Statement

All three SensorML process-type parsers construct their typed result objects by:

1. Spreading the entire raw server JSON object onto the result via ...(json as Record)
2. Coercing required fields (label, uniqueId) with as string — no typeof check
3. Deleting managed keys from the result and re-assigning parsed values

This means any field the server sends with the wrong type (e.g. "label": 42) lands on the typed result object without any runtime error. TypeScript believes the result is PhysicalSystem (or AggregateProcess/SimpleProcess) but any field could actually be the wrong type. The delete-then-reassign mutation pattern is also fragile and non-obvious.

Affected code:

// src/ogc-api/csapi/formats/sensorml/physical-system.ts, lines 411–438
const result: PhysicalSystem = {
  ...(json as Record),  // ← spreads ALL raw server fields
  type: 'PhysicalSystem' as const,
  label: json.label as string,           // ← coerces without checking type
  uniqueId: json.uniqueId as string,     // ← same
};
// Then deletes raw keys and reassigns parsed values (lines 421–438)
delete (result as unknown as Record)['outputs'];
result.outputs = parsedOutputs;

Same pattern at:

• aggregate-process.ts lines 126–134 (spread + coerce)
• simple-process.ts lines 105–113 (spread + coerce)

Scenario:

// A server returns a PhysicalSystem with a numeric label:
const json = {
  type: 'PhysicalSystem',
  label: 42,           // wrong type — should be string
  uniqueId: null,      // wrong type — should be string
  // ...other fields
};

const result = parsePhysicalSystem(json);
// TypeScript reports result.label as string
// But result.label is actually 42 (number)
// result.uniqueId is actually null
// No error thrown anywhere — the lie propagates

Impact: All consumers of the parsed SensorML types receive objects whose fields may silently have the wrong type. The type guarantee is hollow — if label.toUpperCase() is called downstream, it would throw TypeError: label.toUpperCase is not a function with no indication that the error originates from upchecked deserialization. Additionally, the delete-then-reassign mutation pattern makes the code fragile — if a new managed key is added but the managedKeys array is not updated, the raw value leaks through.

Ownership Verification

All three affected files are entirely within the csapi/ isolation boundary and have zero commits on upstream/main:

$ git log upstream/main --oneline -- src/ogc-api/csapi/formats/sensorml/physical-system.ts
(no output)
$ git log upstream/main --oneline -- src/ogc-api/csapi/formats/sensorml/aggregate-process.ts
(no output)
$ git log upstream/main --oneline -- src/ogc-api/csapi/formats/sensorml/simple-process.ts
(no output)

Git blame of affected lines:

File	Commit	Author	Date
physical-system.ts L411-416	0060356f	Sam-Bolling	2026-02-15
aggregate-process.ts L129-134	814aef64	Sam-Bolling	2026-02-15
simple-process.ts L108-113	242c2bfa	Sam-Bolling	2026-02-15

Conclusion: This code is ours. All three files are entirely within the csapi/ isolation boundary and do not exist in upstream camptocamp/ogc-client.

Files to Modify

File	Action	Est. Lines	Purpose
src/ogc-api/csapi/formats/sensorml/physical-system.ts	Modify	~20	Add type checks for required fields; replace delete-then-reassign with explicit construction
src/ogc-api/csapi/formats/sensorml/aggregate-process.ts	Modify	~20	Same
src/ogc-api/csapi/formats/sensorml/simple-process.ts	Modify	~20	Same
src/ogc-api/csapi/formats/sensorml/physical-system.spec.ts	Modify	~15	Add tests for wrong-type required fields
src/ogc-api/csapi/formats/sensorml/aggregate-process.spec.ts	Modify	~15	Same
src/ogc-api/csapi/formats/sensorml/simple-process.spec.ts	Modify	~15	Same

Proposed Solutions

Option A: Construct result explicitly field-by-field

const result: PhysicalSystem = {
  type: 'PhysicalSystem' as const,
  label: typeof json.label === 'string' ? json.label
    : (() => { throw new Error('PhysicalSystem: label must be a string'); })(),
  uniqueId: typeof json.uniqueId === 'string' ? json.uniqueId
    : (() => { throw new Error('PhysicalSystem: uniqueId must be a string'); })(),
  // ...all other fields extracted explicitly with type checks
};

Pros: No raw spread; no delete-then-reassign mutation; type checks are explicit; guaranteed to match the TypeScript interface.
Cons: More verbose; all DescribedObject fields must be listed explicitly. SensorML has a very large optional field surface area.
Effort: Medium | Risk: Low

Option B: Keep spread, add runtime type checks for required fields only (Recommended)

if (typeof json.label !== 'string') {
  throw new Error('PhysicalSystem: label must be a string');
}
if (typeof json.uniqueId !== 'string') {
  throw new Error('PhysicalSystem: uniqueId must be a string');
}
const result: PhysicalSystem = {
  ...(json as Record),
  type: 'PhysicalSystem' as const,
  label: json.label,      // now narrowed to string by guard above
  uniqueId: json.uniqueId, // same
};

And replace the delete-then-reassign pattern with explicit field assignment (either two-stage construction or Object.assign).

Pros: Less verbose; retains spread for optional fields (pragmatic given SensorML's large optional surface area); adds guards for required fields; eliminates as string coercions.
Cons: Still spreads unknown optional fields without validation. Optional fields remain unguarded.
Effort: Small | Risk: Low

Recommended Action

Option B — pragmatic middle ground that eliminates the silent coercions on required fields while acknowledging that SensorML has a very large optional field surface area where exhaustive validation would be impractical. The delete-then-reassign mutation should also be replaced with explicit field assignment.

The original code review recommended Option A for maximum type safety. Option B is recommended here as the pragmatic middle ground given SensorML's 20+ optional DescribedObject fields — exhaustive field-by-field construction would be scope expansion beyond the finding.

Scope — What NOT to Touch

• ❌ Do NOT modify files outside the "Files to Modify" table above
• ❌ Do NOT refactor the SensorML sub-parsers (_helpers.ts, parser.ts) — they are not part of this finding
• ❌ Do NOT change public API signatures — the parsed types (PhysicalSystem, AggregateProcess, SimpleProcess) remain the same
• ❌ Do NOT add exhaustive validation of all optional DescribedObject fields — that would be scope expansion (SensorML has 20+ optional fields)
• ❌ Do NOT change the existing SWE Common or SensorML sub-parser functions that are called within these files — only the result construction pattern

Acceptance Criteria

• Required fields (label, uniqueId, type) are type-checked before assignment — wrong type produces a clear Error, not a silent coercion
• The as string casts on label and uniqueId are replaced with typeof guards
• The delete-then-reassign mutation pattern is removed or replaced with explicit assignment
• All three files (physical-system.ts, aggregate-process.ts, simple-process.ts) are updated consistently
• New tests cover: parser throws when label is not a string; parser throws when uniqueId is not a string
• Existing SensorML parser 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 unchecked generic cast (same category: type-safety at trust boundaries); #143 — extractCSAPIFeature properties null cast (same category: unsafe as cast in format handler)

---

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 finding, 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
• Respect ownership: This is our code — fix on the phase-6 branch

Ownership-Specific Constraints

This is Ours:

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

---

References

#	Document	What It Provides
0	docs/code-review/008-pending-p2-raw-json-spread-into-typed-result.md	Original finding with full context and ownership assessment
1	src/ogc-api/csapi/formats/sensorml/physical-system.ts L411-438	Primary affected code — spread + coerce + delete-reassign in parsePhysicalSystem
2	src/ogc-api/csapi/formats/sensorml/aggregate-process.ts L126-155	Same pattern in parseAggregateProcess
3	src/ogc-api/csapi/formats/sensorml/simple-process.ts L105-130	Same pattern in parseSimpleProcess
4	OGC SensorML 3.0 (23-000r1)	Defines the four concrete process types (SimpleProcess, AggregateProcess, PhysicalComponent, PhysicalSystem) and their JSON structure
5	OGC SWE Common 3.0 (23-011r1)	Defines capabilities, characteristics, and parameters structures nested inside SensorML JSON — part of the raw spread
6	OGC API — Connected Systems Part 1 (23-001)	Establishes SensorML encoding requirements for Part 1 resources
7	docs/research/standards/ogcapi-connectedsystems-1.bundled.oas31.yaml	Machine-readable response schema — authoritative source for expected field types
8	OGC API — Features (17-069r4)	Foundation parser pattern the SensorML parsers should follow
9	GeoJSON (RFC 7946)	Library's GeoJSON parser establishes the expected pattern for typed object construction from parsed JSON
10	JSON Schema	SensorML 3.0 has JSON Schemas for validation — relevant to whether raw JSON should be spread without checking
11	docs/research/references.md	Full project reference catalog
12	camptocamp/ogc-client	Upstream library with format handler patterns and quality bar for PR acceptance
13	OS4CSAPI/ogc-client-CSAPI_2	This repository containing the affected SensorML parser files
14	docs/research/requirements/contribution-definition.md	Format abstraction layer requirements and production-ready implementation goals
15	docs/research/requirements/csapi-52north-analysis.md	Server format variations — different servers could send unexpected field types
16	docs/research/requirements/csapi-datatype-schema-requirements.md	50+ TypeScript interfaces — the type system the as casts bypass
17	docs/research/requirements/csapi-format-requirements-3.1.md	Parsing vs pass-through requirements for format handlers
18	docs/research/requirements/csapi-format-requirements.md	Comprehensive format requirements including SensorML 3.0 parsing needs
19	docs/research/requirements/csapi-gap-analysis.md	Gap analysis — incomplete format parsing led to prior rejection
20	docs/research/requirements/csapi-opensensorhub-analysis.md	Primary server whose SensorML responses the parsers must safely handle
21	docs/research/requirements/csapi-oscarviewer-analysis.md	Real-world TypeScript CSAPI client — type safety reference
22	docs/research/requirements/csapi-oshconnect-python-analysis.md	Pydantic runtime validation — contrasts the finding's lack of runtime checks
23	docs/research/requirements/OSHConnect-Python-Analysis.md	Generic type system and Pydantic validation pattern reference
24	docs/research/requirements/csapi-owslib-analysis.md	Mature client library with class-per-resource typed construction
25	docs/research/requirements/csapi-part1-requirements.md	GeoJSON and SensorML format requirements for Part 1 resources
26	docs/research/requirements/csapi-usage-scenarios.md	Error handling requirements and common error patterns
27	docs/research/requirements/lessons-learned-analysis.md	Under-engineering gaps — unsafe casts are an under-engineering mistake
28	docs/research/requirements/upstream-expectations.md	Quality standards expected by camptocamp — unsafe casts risk PR rejection
29	docs/research/upstream/architecture-patterns-analysis.md	Upstream architectural patterns parsers should follow
30	docs/research/upstream/code-reuse-analysis.md	Code reuse vs duplication strategy for shared utilities
31	docs/research/upstream/csapi-architecture-analysis.md	SWE Common/SensorML integration architecture decisions
32	docs/research/upstream/error-handling-analysis.md	Error handling patterns at parse boundaries
33	docs/research/upstream/format-negotiation-analysis.md	Format detection for application/sml+json — integration with SensorML handlers
34	docs/research/upstream/pr114-analysis.md	EDR implementation blueprint — test patterns and quality reference
35	docs/research/upstream/typescript-types-analysis.md	Type safety strategies and enforcement patterns
36	docs/research/design/csapiquerybuilder/architecture-decision/	Architecture decisions series including multi-class failure analysis
37	docs/research/strategy/design-strategy-research.md	Format abstraction layer architecture design context
38	docs/research/testing/testing-strategy-research.md	Overall testing approach and coverage targets
39	docs/research/testing/research-plans/09-sensorml-testing-requirements.md	SensorML parser testing requirements
40	docs/research/testing/research-plans/10-swe-common-testing-requirements.md	SWE Common testing requirements — nested inside SensorML
41	docs/research/testing/research-plans/03-typescript-testing-standards.md	TypeScript testing standards including type safety verification
42	docs/research/testing/research-plans/18-error-condition-testing.md	Error condition coverage — wrong-type fields are edge conditions
43	docs/research/testing/review/notes-parser-testing-vs-spec-validation.md	Parser testing vs spec validation philosophy — trust boundary context
44	docs/research/testing/review/notes-why-models-default-to-server-validation.md	Why parsers default to server validation — guards against turning fix into validation gate
45	docs/planning/csapi-implementation-guide.md	Architecture reference — format handler design requirements
46	docs/planning/contribution-goal-and-definition.md	Acceptance criteria for upstream PR
47	docs/planning/phase-5/P5-parser-completion-implementation-guide.md	Parser function signatures and field mapping specifications — defines how parsers should construct typed results
48	docs/planning/phase-5/P5-contribution-goal-and-definition.md	Per-parser acceptance criteria and coverage requirements
49	docs/planning/phase-6/P6-implementation-guide.md	Code quality normalization — the raw-spread pattern may need normalization
50	docs/planning/phase-6/P6-contribution-goal-and-definition.md	Upstream acceptance readiness quality bar
51	docs/governance/known-server-quirks.md	Known server non-compliant behaviors — servers could send unexpected types
52	docs/governance/phase-2-lessons-learned.md	Code review and testing strategy lessons
53	docs/governance/phase-3-lessons-learned.md	Format handler development insights and SWE Common complexity management
54	docs/governance/code-review-prompt-template-phase-6.md	Phase 6 code review quality criteria
55	docs/governance/issue-creation-prompt-template-code-review.md	Template C for code review issue creation — the process that produced this issue
56	docs/implementation/final-project-code-review.md	End-to-end code review prior to upstream — context on why this was missed
57	docs/implementation/phase-5.3-code-review.md	Phase 5.3 code review of schema response parsers — adjacent parser quality standards context
58	docs/implementation/phase-5.4-code-review.md	Code review finding resolutions and smoke test results
59	docs/implementation/design-notes-validation-extraction-decoupling.md	Validation/extraction decoupling — the design principle the fix should follow
60	docs/implementation/deferred-findings-final-disposition.md	Deferred findings — may document prior awareness of this pattern
61	docs/implementation/outstanding-findings-status-report.md	Outstanding findings context
62	docs/testing/fixtures-guide.md	Fixture creation for wrong-type test cases
63	docs/testing/demo-app-findings/issue-12-constructor-parameter-narrowing.md	Constructor parameter narrowing — same class of type-safety gap
64	docs/testing/demo-app-findings/issue-13-type-guard-functions-for-union-narrowing.md	Type guard utilities — recommended pattern to replace unsafe as casts
65	Upstream PR #136	Upstream contribution — jahow's review requirements affect PR acceptance
66	OpenSensorHub Demo Server	Primary live server providing SensorML responses the parsers must safely deserialize
67	52°North Demo Server	Secondary server with format variations — responses may differ from OSH
68	docs/planning/ROADMAP.md	Master roadmap — phase progression context