diff --git a/CHANGELOG.md b/CHANGELOG.md index 313285b0..d4949630 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) - `delete-page` no longer sends an empty deletion reason when the wiki sets `attributeEdits: false` and the call gave no comment. MediaWiki recorded that empty reason verbatim, leaving a blank deletion log entry; with no reason sent at all it can autogenerate its own `content was: …` reason instead. - `update-page` no longer advertises itself as idempotent: in `mode='append'` and `mode='prepend'` it never was, so a client replaying a call whose result never arrived adds the content a second time. A replace resends the same content rather than adding to it. - `upload-file-from-url` and `update-file-from-url` no longer leak a connection when they refuse a source URL whose declared size is over `MCP_UPLOAD_MAX_BYTES`. Each refused call held one connection open for as long as the server ran. +- A `config.json` field written with the wrong type now stops the server at startup with an error naming the field, instead of being ignored. A quoted boolean — `"readOnly": "true"`, `"private": "true"`, `"allowWikiManagement": "false"` — matched neither `true` nor `false`, so the setting fell back to its default and a deployment meant to be locked down stayed open. A `${VAR}` reference in a boolean or numeric field is refused for the same reason: substitution produces a string, so such a field never took effect and now has to be written literally. ## [0.16.0] - 2026-07-30 diff --git a/docs/configuration.md b/docs/configuration.md index acb2f7e9..33cc5c92 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -4,6 +4,9 @@ Covers configuration topics beyond the basic `config.json` shape documented in [ ## Configuration fields +A field written with the wrong type stops the server at startup with an error naming it. Booleans and numbers are +written unquoted: `"readOnly": true`, not `"readOnly": "true"`. + ### Top-level fields | Field | Description | @@ -60,6 +63,9 @@ If a referenced variable is not set: - **Secret fields** (`token`, `username`, `password`): the server exits at startup with an error naming the wiki, the field, and the missing variable. - **Non-secret fields**: the `${VAR_NAME}` text is kept as-is. +Substitution produces a string, so it cannot supply a boolean or numeric field. `"readOnly": "${MCP_READ_ONLY}"` +is refused at startup; write `true` or `false` in the file. + ## Secret sources Secret fields can also run an external command and use its output as the secret. This lets you fetch credentials from a password manager, keyring, or secret store without writing them to disk: diff --git a/src/config/loadConfig.ts b/src/config/loadConfig.ts index 65bd928a..edb53b83 100644 --- a/src/config/loadConfig.ts +++ b/src/config/loadConfig.ts @@ -249,12 +249,12 @@ function parseExecSecret(raw: unknown, fieldPath: string): ExecSecret { if (typeof raw !== 'object' || raw === null || Array.isArray(raw)) { throw new Error(`Config error: ${fieldPath} must be a string, null, or an {exec: …} object`); } - // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- post-JSON.parse boundary; ajv-validated WikiConfig parsing is a separate follow-up + // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- post-JSON.parse boundary const src = raw as { exec?: unknown }; if (typeof src.exec !== 'object' || src.exec === null || Array.isArray(src.exec)) { throw new Error(`Config error: ${fieldPath} must be a string, null, or an {exec: …} object`); } - // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- post-JSON.parse boundary; ajv-validated WikiConfig parsing is a separate follow-up + // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- post-JSON.parse boundary const exec = src.exec as { command?: unknown; args?: unknown }; if (typeof exec.command !== 'string' || exec.command === '') { throw new Error(`Config error: ${fieldPath}.exec.command must be a non-empty string`); @@ -265,7 +265,7 @@ function parseExecSecret(raw: unknown, fieldPath: string): ExecSecret { ) { throw new Error(`Config error: ${fieldPath}.exec.args must be an array of strings`); } - // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- post-JSON.parse boundary; ajv-validated WikiConfig parsing is a separate follow-up + // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- post-JSON.parse boundary return { exec: { command: exec.command, args: (exec.args as string[]) ?? [] } }; } @@ -317,11 +317,122 @@ function resolveUploadDirs(rawFromConfig: unknown): readonly string[] { return canonicalised; } +type FieldKind = 'string' | 'boolean' | 'number' | 'stringOrStringArray'; + +interface FieldType { + kind: FieldKind; + /** True for the fields whose declared type includes `null`. */ + nullable?: boolean; +} + +const KIND_DESCRIPTIONS: Record = { + string: 'a string', + boolean: 'a boolean', + number: 'a number', + stringOrStringArray: 'a string or an array of strings', +}; + +const KIND_PREDICATES: Record boolean> = { + string: (value) => typeof value === 'string', + boolean: (value) => typeof value === 'boolean', + number: (value) => typeof value === 'number', + stringOrStringArray: (value) => + typeof value === 'string' || + (Array.isArray(value) && value.every((entry) => typeof entry === 'string')), +}; + +/** + * The declared type of each config field. Keyed off `Config` and `WikiConfig` + * so that a field added to either without an entry here is a compile error + * rather than a field that quietly stops being validated. The credential + * fields are excluded because `resolveSecretField` parses them, and + * `uploadDirs` and `wikis` because `resolveUploadDirs` and `resolveWiki` do. + */ +const CONFIG_FIELD_TYPES: Record, FieldType> = { + defaultWiki: { kind: 'string' }, + allowWikiManagement: { kind: 'boolean' }, +}; + +const WIKI_FIELD_TYPES: Record, FieldType> = { + sitename: { kind: 'string' }, + server: { kind: 'string' }, + articlepath: { kind: 'string' }, + scriptpath: { kind: 'string' }, + publicServer: { kind: 'string', nullable: true }, + oauth2ClientId: { kind: 'string', nullable: true }, + oauth2ClientSecret: { kind: 'string', nullable: true }, + oauth2CallbackPort: { kind: 'number', nullable: true }, + private: { kind: 'boolean' }, + readOnly: { kind: 'boolean' }, + attributeEdits: { kind: 'boolean' }, + tags: { kind: 'stringOrStringArray', nullable: true }, +}; + +/** + * Refuses a field whose value does not have its declared type. TypeScript + * erases the declarations, so without this a `"true"` written for a boolean + * reaches the strict comparisons that read it, matches neither `true` nor + * `false`, and leaves the field at its default — which for `readOnly` and + * `private` leaves a deployment the operator meant to lock down open. Coercing + * instead of refusing would answer `"false"` with `true`, so refusing is the + * only reading that cannot be wrong. + * + * `pathPrefix` names the object being checked and ends in a dot, or is empty + * for the top level. + */ +function assertFieldTypes( + source: Record, + types: Record, + pathPrefix: string, +): void { + for (const [field, type] of Object.entries(types)) { + const value = source[field]; + if (value === undefined || (value === null && type.nullable === true)) { + continue; + } + if (KIND_PREDICATES[type.kind](value)) { + continue; + } + const quoting = + typeof value === 'string' && (type.kind === 'boolean' || type.kind === 'number') + ? ' Remove the quotes.' + : ''; + // An array reaching a field that accepts one is an array of the wrong + // contents, which "an array" would not tell the reader. + const actual = + type.kind === 'stringOrStringArray' && Array.isArray(value) + ? 'an array with a non-string entry' + : describeValue(value); + throw new Error( + `Config error: ${pathPrefix}${field} must be ${KIND_DESCRIPTIONS[type.kind]}, but is ${actual}.${quoting}`, + ); + } +} + +function describeValue(value: unknown): string { + if (value === null) { + return 'null'; + } + if (Array.isArray(value)) { + return 'an array'; + } + switch (typeof value) { + case 'string': + return 'a string'; + case 'number': + return 'a number'; + case 'boolean': + return 'a boolean'; + default: + return 'an object'; + } +} + function resolveWiki(raw: unknown, wikiKey: string): WikiConfig { if (typeof raw !== 'object' || raw === null || Array.isArray(raw)) { throw new Error(`Config error: wikis.${wikiKey} must be an object`); } - // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- post-JSON.parse boundary; ajv-validated WikiConfig parsing is a separate follow-up + // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- post-JSON.parse boundary const src = raw as Record; const resolved: Record = {}; for (const [fieldKey, fieldValue] of Object.entries(src)) { @@ -331,10 +442,8 @@ function resolveWiki(raw: unknown, wikiKey: string): WikiConfig { resolved[fieldKey] = replaceEnvVarsInObject(fieldValue); } } - if (resolved.readOnly !== undefined && typeof resolved.readOnly !== 'boolean') { - throw new Error(`Config error: wikis.${wikiKey}.readOnly must be a boolean`); - } - // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- post-JSON.parse boundary; ajv-validated WikiConfig parsing is a separate follow-up + assertFieldTypes(resolved, WIKI_FIELD_TYPES, `wikis.${wikiKey}.`); + // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- post-JSON.parse boundary; the type of each field present is checked above, its presence is not return resolved as unknown as WikiConfig; } @@ -342,16 +451,20 @@ function resolveConfig(parsed: unknown): Config { if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) { throw new Error('Config error: config.json must be an object'); } - // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- post-JSON.parse boundary; ajv-validated WikiConfig parsing is a separate follow-up + // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- post-JSON.parse boundary const p = parsed as Record; + assertFieldTypes(p, CONFIG_FIELD_TYPES, ''); const defaultWiki = typeof p.defaultWiki === 'string' ? replaceEnvVars(p.defaultWiki) : ''; const allowWikiManagement = typeof p.allowWikiManagement === 'boolean' ? p.allowWikiManagement : undefined; const uploadDirs = resolveUploadDirs(p.uploadDirs); const rawWikis = p.wikis; - if (typeof rawWikis !== 'object' || rawWikis === null || Array.isArray(rawWikis)) { + if (rawWikis === undefined) { return { defaultWiki, wikis: {}, allowWikiManagement, uploadDirs }; } + if (typeof rawWikis !== 'object' || rawWikis === null || Array.isArray(rawWikis)) { + throw new Error(`Config error: wikis must be an object, but is ${describeValue(rawWikis)}.`); + } const wikis: Record = {}; for (const [key, rawWiki] of Object.entries(rawWikis)) { const problem = wikiKeyProblem(key); diff --git a/tests/config/loadConfig.test.ts b/tests/config/loadConfig.test.ts index 528b0bab..ca058c7f 100644 --- a/tests/config/loadConfig.test.ts +++ b/tests/config/loadConfig.test.ts @@ -163,6 +163,92 @@ describe('loadConfigFromFile', () => { }); }); + describe('field types', () => { + const loadWikiWith = async (field: string, value: unknown) => { + setConfigFile({ + defaultWiki: 'w', + wikis: { w: { ...baseWiki, [field]: value } }, + }); + const { loadConfigFromFile } = await import('../../src/config/loadConfig.ts'); + return loadConfigFromFile; + }; + + it.each(['private', 'readOnly', 'attributeEdits'])( + 'throws when %s is a quoted boolean', + async (field) => { + const load = await loadWikiWith(field, 'true'); + expect(load).toThrow(`Config error: wikis.w.${field} must be a boolean`); + }, + ); + + it('points at the quoting when a boolean is quoted', async () => { + const load = await loadWikiWith('readOnly', 'true'); + expect(load).toThrow('Remove the quotes'); + }); + + it('throws when a string field holds a number', async () => { + const load = await loadWikiWith('server', 42); + expect(load).toThrow('Config error: wikis.w.server must be a string'); + }); + + it('throws when oauth2CallbackPort is a quoted number', async () => { + const load = await loadWikiWith('oauth2CallbackPort', '8080'); + expect(load).toThrow('Config error: wikis.w.oauth2CallbackPort must be a number'); + }); + + it('throws when tags holds a non-string entry', async () => { + const load = await loadWikiWith('tags', ['mcp', 7]); + expect(load).toThrow( + 'Config error: wikis.w.tags must be a string or an array of strings, but is an array with a non-string entry.', + ); + }); + + it('throws when a boolean field is given as an env var reference', async () => { + vi.stubEnv('MCP_READ_ONLY', 'true'); + const load = await loadWikiWith('readOnly', '${MCP_READ_ONLY}'); + expect(load).toThrow('Config error: wikis.w.readOnly must be a boolean'); + }); + + it('accepts tags as an array of strings', async () => { + const load = await loadWikiWith('tags', ['mcp', 'automated']); + expect(load().wikis.w.tags).toEqual(['mcp', 'automated']); + }); + + it('throws when a boolean field is null', async () => { + const load = await loadWikiWith('readOnly', null); + expect(load).toThrow('Config error: wikis.w.readOnly must be a boolean'); + }); + + it('accepts null in a field whose declared type allows it', async () => { + const load = await loadWikiWith('publicServer', null); + const wiki = load().wikis.w; + expect(wiki.sitename).toBe('Test Wiki'); + expect(wiki.publicServer).toBeNull(); + }); + + it('throws when allowWikiManagement is a quoted boolean', async () => { + setConfigFile({ + allowWikiManagement: 'false', + defaultWiki: 'w', + wikis: { w: baseWiki }, + }); + const { loadConfigFromFile } = await import('../../src/config/loadConfig.ts'); + expect(loadConfigFromFile).toThrow('Config error: allowWikiManagement must be a boolean'); + }); + + it('throws when defaultWiki is not a string', async () => { + setConfigFile({ defaultWiki: 7, wikis: { w: baseWiki } }); + const { loadConfigFromFile } = await import('../../src/config/loadConfig.ts'); + expect(loadConfigFromFile).toThrow('Config error: defaultWiki must be a string'); + }); + + it('throws when wikis is not an object', async () => { + setConfigFile({ defaultWiki: 'w', wikis: [baseWiki] }); + const { loadConfigFromFile } = await import('../../src/config/loadConfig.ts'); + expect(loadConfigFromFile).toThrow('Config error: wikis must be an object'); + }); + }); + describe('wiki keys', () => { // Percent-encoding carries these through to a reachable resource URI. it.each(['a/b', 'a?b', 'a#b', 'my wiki'])('accepts a wiki key containing %j', async (key) => {