fix(security): validate settings keys at the getSetting boundary #8

Closed

hermes wants to merge 0 commits from cal/settings-key-validation-20260917 into main

hermes commented

2026-09-17 14:40:13 +00:00

Owner

Settings keys are now validated against the known settings schema; requests writing unknown or non-settings keys are rejected with a validation error instead of reaching the store.

Three independent review rounds; final review (read-only, fresh session) returned one LOW, non-blocking finding on test rejection-reason pinning — a test-only tightening for that finding is in flight and will land on this branch before merge.

Gate reproduced independently: 30/30 tests pass, typecheck clean, negative controls confirm the assertions are load-bearing. Release notes included.

Settings keys are now validated against the known settings schema; requests writing unknown or non-settings keys are rejected with a validation error instead of reaching the store. Three independent review rounds; final review (read-only, fresh session) returned one LOW, non-blocking finding on test rejection-reason pinning — a test-only tightening for that finding is in flight and will land on this branch before merge. Gate reproduced independently: 30/30 tests pass, typecheck clean, negative controls confirm the assertions are load-bearing. Release notes included.

hermes added 2 commits

2026-09-17 14:40:13 +00:00

fix(security): validate the settings key at the getSetting boundary b5a968b6a3

getSetting read its key straight from request.qs() and passed it to
KVStore.getValue with an unchecked `as KVStoreKey` cast, so any caller
could name an arbitrary KV store key and read it back through the
settings endpoint.

Route the key through a vine schema built on the same SETTINGS_KEYS
authority that updateSettingSchema already uses, so unknown keys and
non-settings KV store keys are both rejected before the store is
touched, and the cast can be dropped because the key is narrowed by
the validated enum.

Cover it with unit tests that assert the rejection reason, that the
store is never read on an invalid key, and that keys present in
KV_STORE_SCHEMA but deliberately absent from SETTINGS_KEYS are
refused. The rejection set is derived at runtime from
Object.keys(KV_STORE_SCHEMA) minus SETTINGS_KEYS, with a non-empty
guard so the loop cannot pass vacuously.

docs: note settings-key validation in release notes c23eefb51c

hermes added 1 commit

2026-09-17 16:04:14 +00:00

test: tighten settings-key validation assertions 55958ee9ec

Address review findings on the settings-key validation spec:

- Assert ValidationError type identity, not just the error code string,
  at the controller-level rejection site. Every rejection assertion now
  pins both the constructor and the code.
- Guard the accepted-keys loop with a length assertion, mirroring the
  sibling rejection loop, so an empty SETTINGS_KEYS cannot pass vacuously.
- Replace hand-rolled try/finally patching of the KVStore.getValue global
  static with Japa per-test cleanup, which also restores on a thrown body,
  a failed assertion, or a suite timeout.
- Drop redundant undefined initializers and rewrap two lines to the
  configured print width.

Test-only change; both product sources are byte-identical. Verified with
tsc --noEmit (exit 0) and the full unit suite (30 passed). Non-vacuity
proven by three independent negative controls, each failing at the new
assertion sites and each reverted afterwards.

hermes referenced this pull request from a commit

2026-09-17 16:05:23 +00:00

Merge pull request #8 from cal/settings-key-validation-20260917

hermes closed this pull request

2026-10-14 02:55:50 +00:00