]> Piment Noir Git Repositories - e-mobility-charging-stations-simulator.git/commit
fix(config): enforce Zod simulator-config validation at boot (#1874) (#1903)
authorJérôme Benoit <jerome.benoit@piment-noir.org>
Mon, 15 Jun 2026 19:20:06 +0000 (21:20 +0200)
committerGitHub <noreply@github.com>
Mon, 15 Jun 2026 19:20:06 +0000 (21:20 +0200)
commit225159487916788c59a9276f474826ccdcb372e1
tree5959e443a8097ce1fdce22c18250eaeafc8be91b
parent19afcb2852ccff5e18d11b8522d77f7c5ff80976
fix(config): enforce Zod simulator-config validation at boot (#1874) (#1903)

* fix(config): enforce Zod simulator-config validation at boot (#1874)

Audit pass over uiServer.* primitives that PR #1901 left under-constrained.
Each constraint closes a concrete fail-fast gap surfaced by a 3-angle design
review (schema completeness, fail-fast wiring, adversarial risk).

UIServerAuthenticationSchema:
- username: z.string().min(1).optional() — empty username currently passes;
  AbstractUIServer falls back to '' and authenticates against the empty
  configured value, silently bypassing Basic-Auth.
- password: z.string().min(1).optional() — same hole on password.
- username .refine(no ':') (RFC 7617) — lifts the runtime check at
  UIServerFactory.ts:48-54 into the schema layer.
- object-level .refine: enabled === true requires both username and password.
  Closes the silent-bypass when authentication is enabled with no creds.

UIServerAccessPolicySchema:
- object-level .refine: allowLoopbackProxy === true requires
  trustedProxies.length >= 1. With allowLoopbackProxy: true and an empty
  trustedProxies list, the proxy-trust check at runtime always evaluates
  against the empty allowlist, contradicting the operator's intent.

No wiring change: Configuration.ts:160 (boot, process.exit(1)) and
Configuration.ts:386-431 (hot-reload, rollback) already route
ConfigurationValidationError correctly. Schema tightening lands at parse
time without rewiring.

Test fixture update: tests/utils/ConfigurationSchema.test.ts unknown-key
test for uiServer.authentication now includes valid username/password
alongside bogusAuthKey so the unknown-key intent stays unambiguous after
the new object-level superRefine.

9 new test cases (5 reject + 3 accept regression-anchors + 1 cross-field
reject) covering rejection paths and the docker/config.json-shape
acceptance.

Designs that did NOT survive cross-validation are documented as
out-of-scope follow-ups: type×version coupling (coupled to in-place
mutation bug at UIServerFactory.ts:69), type×auth coupling (defense in
depth at runtime is intentional), options.{path,readableAll,writableAll,
ipv6Only} refinements (no shipped config exercises these), and the
TLS-non-loopback wildcard refine (would break docker/config.json:18).

* fix(config): refine auth schema error path and tighten test path checks

Address review feedback on #1903:

src/utils/ConfigurationSchema.ts
- UIServerAuthenticationSchema object-level .refine now carries
  path: ['username'], producing an error path of
  'uiServer.authentication.username' rather than 'uiServer.authentication'.
  Mirrors the path: ['allowLoopbackProxy'] convention already used on
  UIServerAccessPolicySchema and yields a more actionable message.
- JSDoc clarifies that the field-level constraints (min(1), no ':')
  apply unconditionally even when authentication.enabled is false:
  the schema is intentionally stricter than the runtime guard in
  UIServerFactory so a credentials value cannot ship under
  enabled: false and become a bypass the moment enabled flips on
  (e.g. via hot-reload).

tests/utils/ConfigurationSchema.test.ts
- Three .includes('uiServer.authentication') substring checks tightened
  to .startsWith('uiServer.authentication') so they cannot match a
  hypothetical sibling key like 'uiServer.authenticationFoo'. Affects
  the two object-level coupling tests and the unknown-key fixture
  assertion. Exact-element checks at lines 630/651 (specific leaf
  paths) and the leaf-specific .includes at line 671 are left as-is.

No behavior change. 114/114 schema tests pass, typecheck + lint clean.

* docs(config): tighten UIServerAuthenticationSchema JSDoc

Rewrite the JSDoc as a single factual paragraph matching the file
convention (LogConfigurationSchema, WorkerConfigurationSchema,
UIServerListenOptionsObjectSchema, ...) — 12 lines collapsed to 7,
no redundant phrasing, structured as: constraint definitions →
required-when-enabled coupling → unconditional-field-level rationale.
Per AGENTS.md 'Documentation conventions' (concision, structure).

* fix(config): emit per-field error paths on auth-required refine

Address sub-agent review feedback on #1903.

src/utils/ConfigurationSchema.ts
- Replace the .refine() with a fixed path: ['username'] by a
  .superRefine() that emits one issue per actually-missing field with
  the correct path. Configurations supplying only 'username' now
  surface the issue at 'uiServer.authentication.password' rather than
  'uiServer.authentication.username', and vice versa. Configurations
  missing both produce two issues, one per field.

tests/utils/ConfigurationSchema.test.ts
- 'should reject authentication enabled without username and password'
  now asserts both 'uiServer.authentication.username' and
  'uiServer.authentication.password' paths are present, locking the
  per-field issue contract.
- 'should reject authentication enabled with username but no password'
  now asserts 'uiServer.authentication.password' specifically — would
  catch the prior path-misanchoring regression.
- The 'should reject unknown key in uiServer.authentication section'
  regression-anchor now asserts i.code === 'unrecognized_keys' at the
  'uiServer.authentication' path, locking the unknown-key intent so a
  hypothetical future refine cannot satisfy the assertion by accident.

114/114 schema tests pass, typecheck + lint clean. No production-
runtime behavior change beyond per-field error paths.

* fix(ui-common): align authenticationConfigSchema with simulator auth schema

Apply the same defense-in-depth + per-field-error-path pattern shipped
on the simulator-side UIServerAuthenticationSchema in this PR. The Web
UI client schema (consumed by ui/web/src/main.ts and transitively by
ui/cli/src/config/loader.ts) carried the same set of defects:
- password lacked min(1) at the field level
- username regex /^[^:]*$/ accepted the empty string
- a single .refine combined type-check + presence + length, producing
  a generic error at the auth object root with no path
- empty creds shipped under enabled: false would become a Basic-Auth
  bypass the moment enabled is flipped on

ui/common/src/config/schema.ts
- password: z.string().min(1).optional()
- username: z.string().min(1).refine(no ':' RFC 7617).optional()
  (replaces the regex which silently accepted '')
- .refine(...) replaced by .superRefine(...) emitting one issue per
  actually-missing field with the correct path, only when
  enabled === true && type === PROTOCOL_BASIC_AUTH (existing
  type-check short-circuit preserved as future-proofing).
- JSDoc mirrors the simulator-side sibling, documenting the
  intentional stricter-than-runtime semantics.

ui/common/tests/config.test.ts
- 4 new tests: empty password rejected (path 'authentication.password'),
  username with ':' rejected (RFC 7617 message at
  'authentication.username'), password-only and username-only enabled
  configs each rejected at the missing field's path.
- 2 existing tests tightened: 'missing credentials' now asserts both
  username and password paths; 'empty username' now asserts the
  'authentication.username' path.
- 6 redundant 'if (result.success) return' narrowing guards removed —
  assert.strictEqual already narrows the result type.

118/118 ui-common tests pass, typecheck + lint clean. No production
behavior change for the only valid-config consumer ('admin'/'admin'
shape regression-anchored).

* fix(config): point accessPolicy refine error path at the actionable field

Address review feedback on #1903. The .refine() rejecting
'allowLoopbackProxy: true' with empty 'trustedProxies' attached its
issue at path ['allowLoopbackProxy'] — the trigger of the rejection,
but not the field the operator must edit. Move the path to
['trustedProxies'] so structured log scrapers and IDE jump-to-field
tooling steer the operator to the field that needs populating. The
message wording (which already names both fields) is unchanged.

Test 'should reject allowLoopbackProxy=true with empty trustedProxies'
tightened from a permissive substring check
('allowLoopbackProxy or trustedProxies') to an exact-path check on
'uiServer.accessPolicy.trustedProxies' so the path contract is locked.

A second review suggestion (gate the auth credentials-required
superRefine on type === PROTOCOL_BASIC_AUTH) was investigated and
declined: the server-side AuthenticationType enum has BASIC_AUTH AND
PROTOCOL_BASIC_AUTH (unlike the Web UI client's single-member enum),
both runtime paths require username/password
(AbstractUIServer.ts:358-381), and adding the type-gate would re-open
the silent Basic-Auth bypass on the legacy 'basic-auth' path that
this PR is closing on the modern path.

114/114 schema tests pass, typecheck + lint clean.

* docs(config): refine auth-schema JSDoc rationale and tighten RFC 7617 path check

Address sub-agent review feedback on #1903.

src/utils/ConfigurationSchema.ts + ui/common/src/config/schema.ts
- Drop the 'across hot-reload toggles of enabled' phrasing in the
  UIServerAuthenticationSchema and authenticationConfigSchema JSDoc.
  Hot-reload of uiServer.* is documented as a no-op in the PR body
  (R4: Bootstrap.ts:103,161 instantiates the UI server once), so the
  unconditional field-level constraints are an early-detection signal
  rather than a hot-reload mitigation. The accurate rationale is that
  empty placeholders cannot ship under enabled: false and become a
  Basic-Auth bypass on the next boot with enabled: true.
- The asymmetry between the two superRefines (server gates only on
  enabled; Web UI also gates on type === PROTOCOL_BASIC_AUTH) is
  preserved and intentional — server-side AuthenticationType has
  legacy BASIC_AUTH which is also credential-required at runtime
  (AbstractUIServer.ts isBasicAuthEnabled / isValidBasicAuth). The
  rationale is captured in this commit body and the prior
  86a6d4a2 commit body for git blame discoverability.

tests/utils/ConfigurationSchema.test.ts
- RFC 7617 username-rejection test now matches the issue path with
  i.path.join('.') === 'uiServer.authentication.username' (exact)
  instead of String.prototype.includes (substring), aligning with
  every other test in the file and with the ui-common sibling.

114/114 schema tests + 118/118 ui-common tests pass, typecheck +
lint clean. No behavior change.
src/utils/ConfigurationSchema.ts
tests/utils/ConfigurationSchema.test.ts
ui/common/src/config/schema.ts
ui/common/tests/config.test.ts