Skip to content

Refuse a config field written with the wrong type - #564

Closed
alistair3149 wants to merge 2 commits into
masterfrom
fix/validate-config-field-types
Closed

Refuse a config field written with the wrong type#564
alistair3149 wants to merge 2 commits into
masterfrom
fix/validate-config-field-types

Conversation

@alistair3149

@alistair3149 alistair3149 commented Aug 11, 2026

Copy link
Copy Markdown
Member

Fixes #560

config.json was parsed and then trusted. Its shape is declared as TypeScript interfaces, which are erased at runtime, so a value of the wrong type reached the code that reads it unchanged. Every boolean is read with a strict comparison, so a quoted "true" matched neither true nor false and the field fell back to its default: readOnly, private and allowWikiManagement all stayed open on a deployment the operator meant to lock down.

The loader now checks every declared field of Config and WikiConfig against its type and refuses to start on a mismatch, naming the field and the type it got. A string where a boolean or a number belongs also gets Remove the quotes., since that is the mistake. null is accepted only where the declared type allows it, so readOnly: null stays refused as before and oauth2ClientId: null still round-trips. defaultWiki and wikis are covered too; both silently degraded to an empty value before.

The two field tables are keyed off Exclude<keyof WikiConfig, SecretFieldName> rather than Record<string, …>, so a field added to either interface without a table entry — or a table key typo'd as readonly — is a compile error rather than a field that quietly stops being validated. Verified by removing an entry and by misspelling one.

${VAR} substitution produces a string, so a boolean or numeric field written that way never took effect and is now refused. That is the one upgrade break here: a config with "oauth2CallbackPort": "${PORT}" booted before, with the port silently ephemeral, and now will not start. Called out in the changelog and beside the substitution rules it contradicts.

The field table is hand-rolled rather than a zod schema as the issue proposed. Validation here is interleaved with env-var substitution, exec-secret parsing and realpath resolution, which a schema cannot subsume, so zod would have left two validation systems and swapped operator-facing messages for generated ones.

Considered, omitted

  • Rejecting unknown keys. "readonly": true in a wiki entry still leaves that wiki writable, and that is a likelier operator mistake than quoting a boolean — the strongest argument for widening this PR. Held back because refusing an unknown key would break a config that works today, making it a breaking change rather than a fix, and a warning instead of a refusal is a design call worth making separately. Happy to add either here if you would rather it landed together.
  • Requiring sitename, server, articlepath and scriptpath to be present. Presence is a separate contract from type, and enforcing it could refuse a config that works today. A wiki missing server still fails at first use as undefined/w/api.php rather than at startup.

Verified by running the new tests against master: 9 of the 14 fail there, and the other 5 pin behaviour the field table must not relax. Each new test was also mutation-checked against 14 mutations of the validator — removing either validation call, flipping the nullable rule both ways, dropping the quoting hint, weakening the array check, and dropping each table entry — all caught.

AI-authored — Claude Code, Opus 5 (1M context); "Fix 560" from @alistair3149, no redirections; diff not yet human-reviewed; new tests written test-first and run against master to confirm they fail, mutation-checked, full suite, lint, typecheck and fmt clean locally, CI green on the first commit.

alistair3149 and others added 2 commits August 11, 2026 17:56
Fixes #560

config.json was parsed and then trusted. Its shape is declared as TypeScript
interfaces, which are erased at runtime, so a value of the wrong type reached
the code that reads it unchanged. Every boolean is read with a strict
comparison, so a quoted "true" matched neither true nor false and the field fell
back to its default: readOnly, private and allowWikiManagement all stayed open
on a deployment the operator meant to lock down.

The loader now checks every declared field of Config and WikiConfig against its
type and refuses to start on a mismatch, naming the field and the type it got. A
string where a boolean or a number belongs also gets "Remove the quotes.", since
that is how the value comes to be wrong. null is accepted only where the
declared type allows it, so readOnly: null stays refused as before and
oauth2ClientId: null still round-trips. defaultWiki and wikis are covered too;
both silently degraded to an empty value before.

The field table is hand-rolled rather than a zod schema as the issue proposed.
Validation here is interleaved with env-var substitution, exec-secret parsing
and realpath resolution, which a schema cannot subsume, so zod would have left
two validation systems and swapped operator-facing messages for generated ones.

Considered, omitted: rejecting unknown keys, which would catch "readonly"
written for readOnly but also refuse the "_comment" idiom; and requiring
sitename, server, articlepath and scriptpath to be present, which is a separate
contract from type and could refuse a config that works today.

Verified by running the new tests against master: 9 of the 13 fail there, and
the other 4 pin behaviour the field table must not relax.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two tables were typed Record<string, FieldType>, so nothing tied them to
Config and WikiConfig. A per-wiki boolean added later without a table entry
would have reopened the hole this validation closes, silently and with a green
typecheck, and a key typo'd as readonly would do it today. Keying the tables off
Exclude<keyof WikiConfig, SecretFieldName> makes both a compile error, verified
by removing an entry and by misspelling one.

Substitution turns out to be worth naming: it produces a string, so a boolean or
numeric field written as ${VAR} never took effect and is now refused. That is an
upgrade break for a config that used one, so it is called out in the changelog
and beside the substitution rules it contradicts.

An array reaching tags now reports "an array with a non-string entry" rather
than "an array", which read as a contradiction of the expectation beside it. The
comment on the WikiConfig cast no longer claims more than the check delivers:
the type of each field present is checked, its presence is not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Config booleans are not validated, and a quoted value silently disables readOnly or private

1 participant