From 8c3739dfd061e4e8a7a513df5da67619d52cb311 Mon Sep 17 00:00:00 2001 From: CD Cabrera Date: Thu, 16 Jul 2026 10:58:06 -0400 Subject: [PATCH 1/5] refactor(options): pf-4387 centralize url whitelist --- .../options.defaults.test.ts.snap | 11 +++++ src/__tests__/options.test.ts | 32 ++++++++++++ src/options.defaults.ts | 49 ++++++++++++++----- 3 files changed, 81 insertions(+), 11 deletions(-) diff --git a/src/__tests__/__snapshots__/options.defaults.test.ts.snap b/src/__tests__/__snapshots__/options.defaults.test.ts.snap index cd37bf4c..5b15502c 100644 --- a/src/__tests__/__snapshots__/options.defaults.test.ts.snap +++ b/src/__tests__/__snapshots__/options.defaults.test.ts.snap @@ -141,6 +141,17 @@ exports[`options defaults should return specific properties: defaults 1`] = ` "toolModules": [], "urlRegex": /\\^\\(https\\?:\\)\\\\/\\\\//i, "version": "0.0.0", + "whitelist": { + "protocols": [ + "http", + "https", + ], + "urls": [ + "https://patternfly.org", + "https://github.com/patternfly", + "https://raw.githubusercontent.com/patternfly", + ], + }, "xhrFetch": { "timeoutMs": 15000, }, diff --git a/src/__tests__/options.test.ts b/src/__tests__/options.test.ts index 2a8e9aae..ff58d187 100644 --- a/src/__tests__/options.test.ts +++ b/src/__tests__/options.test.ts @@ -45,3 +45,35 @@ describe('SET_OPTIONS', () => { }); }); +describe('SET_OPTIONS exposure contract', () => { + /** + * Consumer-settable keys. + * + * @note + * This suite is a tripwire for "did a new default accidentally get exposed + * to CLI/programmatic consumers?": + * + * - `notForbidden` — explicit allowlist of keys that MAY appear in + * SET_OPTIONS. Anything not on this list MUST NOT be settable by a + * consumer. Updating this list means your work will be audited. + */ + const notForbidden = [ + 'mode', + 'modeOptions', + 'http', + 'isHttp', + 'logging', + 'pluginIsolation', + 'docsPaths', + 'name', + 'toolModules', + 'version', + 'contextManagement' + ] as const; + + it('should expose exactly the notForbidden keys via SET_OPTIONS', () => { + const exposed = Object.keys(SET_OPTIONS).sort(); + + expect(exposed).toEqual([...notForbidden].sort()); + }); +}); diff --git a/src/options.defaults.ts b/src/options.defaults.ts index 2498128a..e947bf52 100644 --- a/src/options.defaults.ts +++ b/src/options.defaults.ts @@ -49,6 +49,7 @@ import { getNodeMajorVersion } from './options.helpers'; * registered with the server. * @property urlRegex - Regular expression pattern for URL matching. * @property version - Version of the package. + * @property whitelist - Central outbound-URL policy options. * @property xhrFetch - XHR and Fetch options. */ interface DefaultOptions { @@ -84,6 +85,7 @@ interface DefaultOptions { toolModules: ToolModule | ToolModule[]; urlRegex: RegExp; version: string; + whitelist: WhitelistOptions; xhrFetch: XhrFetchOptions; } @@ -173,11 +175,6 @@ interface ModeOptions { } | undefined; } -/** - * A string that must start with a valid protocol. - */ -type WhitelistUrl = `${'http' | 'https'}://${string}`; - /** * PatternFly-specific options. * @@ -289,6 +286,26 @@ interface StatsSession extends StatsOptions { channels: StatsChannels } +/** + * A string that must start with a valid protocol. + */ +type WhitelistUrl = `${'http' | 'https'}://${string}`; + +/** + * Central outbound-URL policy. + * + * @note Any code that fetches a remote URL; PatternFly docs, + * `setFetch`, resource loaders; must validate against this + * list via `assertInputUrlWhiteListed`. + * + * @property urls Allowed URL prefixes (scheme + host [+ path]). + * @property protocols Allowed URL protocols. + */ +interface WhitelistOptions { + urls: WhitelistUrl[]; + protocols: string[]; +} + /** * XHR and Fetch options. * @@ -437,6 +454,18 @@ const STATS_OPTIONS: StatsOptions = { } }; +/** + * Central whitelist. Spread into downstream consumers. + */ +const WHITELIST_OPTIONS: WhitelistOptions = { + urls: [ + 'https://patternfly.org', + 'https://github.com/patternfly', + 'https://raw.githubusercontent.com/patternfly' + ], + protocols: ['http', 'https'] +}; + /** * Default XHR and Fetch options. */ @@ -466,12 +495,8 @@ const PATTERNFLY_OPTIONS: PatternFlyOptions = { ], versionStrategy: 'highest' }, - urlWhitelist: [ - 'https://patternfly.org', - 'https://github.com/patternfly', - 'https://raw.githubusercontent.com/patternfly' - ], - urlWhitelistProtocols: ['http', 'https'] + urlWhitelist: [...WHITELIST_OPTIONS.urls], + urlWhitelistProtocols: [...WHITELIST_OPTIONS.protocols] }; /** @@ -534,6 +559,7 @@ const DEFAULT_OPTIONS: DefaultOptions = { separator: DEFAULT_SEPARATOR, urlRegex: URL_REGEX, version: (process.env.NODE_ENV === 'local' && '0.0.0') || packageJson.version, + whitelist: WHITELIST_OPTIONS, xhrFetch: XHR_FETCH_OPTIONS }; @@ -554,6 +580,7 @@ export { type ServerInstanceOptions, type StatsSession, type ToolModule, + type WhitelistOptions, type WhitelistUrl, type XhrFetchOptions }; From 7d90ed4c5652ecd1c06e9feb61ba9a91a6122f0f Mon Sep 17 00:00:00 2001 From: CD Cabrera Date: Thu, 16 Jul 2026 11:05:47 -0400 Subject: [PATCH 2/5] fix: review update --- src/__tests__/options.test.ts | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/src/__tests__/options.test.ts b/src/__tests__/options.test.ts index ff58d187..2918a208 100644 --- a/src/__tests__/options.test.ts +++ b/src/__tests__/options.test.ts @@ -44,36 +44,3 @@ describe('SET_OPTIONS', () => { } }); }); - -describe('SET_OPTIONS exposure contract', () => { - /** - * Consumer-settable keys. - * - * @note - * This suite is a tripwire for "did a new default accidentally get exposed - * to CLI/programmatic consumers?": - * - * - `notForbidden` — explicit allowlist of keys that MAY appear in - * SET_OPTIONS. Anything not on this list MUST NOT be settable by a - * consumer. Updating this list means your work will be audited. - */ - const notForbidden = [ - 'mode', - 'modeOptions', - 'http', - 'isHttp', - 'logging', - 'pluginIsolation', - 'docsPaths', - 'name', - 'toolModules', - 'version', - 'contextManagement' - ] as const; - - it('should expose exactly the notForbidden keys via SET_OPTIONS', () => { - const exposed = Object.keys(SET_OPTIONS).sort(); - - expect(exposed).toEqual([...notForbidden].sort()); - }); -}); From d5433f74c1a8f3db704012b3bc836d15aeb0a243 Mon Sep 17 00:00:00 2001 From: CD Cabrera Date: Thu, 16 Jul 2026 12:03:49 -0400 Subject: [PATCH 3/5] fix: review update --- .../__snapshots__/options.defaults.test.ts.snap | 9 --------- src/options.context.ts | 2 +- src/options.defaults.ts | 14 ++++---------- src/server.assertions.ts | 2 +- src/tool.patternFlyDocs.ts | 2 +- 5 files changed, 7 insertions(+), 22 deletions(-) diff --git a/src/__tests__/__snapshots__/options.defaults.test.ts.snap b/src/__tests__/__snapshots__/options.defaults.test.ts.snap index 5b15502c..f8fe7cef 100644 --- a/src/__tests__/__snapshots__/options.defaults.test.ts.snap +++ b/src/__tests__/__snapshots__/options.defaults.test.ts.snap @@ -72,15 +72,6 @@ exports[`options defaults should return specific properties: defaults 1`] = ` "@patternfly/patternfly", ], }, - "urlWhitelist": [ - "https://patternfly.org", - "https://github.com/patternfly", - "https://raw.githubusercontent.com/patternfly", - ], - "urlWhitelistProtocols": [ - "http", - "https", - ], }, "pluginHost": { "gracePeriodMs": 2000, diff --git a/src/options.context.ts b/src/options.context.ts index 21086668..83ac882f 100644 --- a/src/options.context.ts +++ b/src/options.context.ts @@ -121,7 +121,7 @@ const setOptions = ( ): GlobalOptions => { const base = mergeObjects(DEFAULT_OPTIONS as GlobalOptions, options, { allowNullValues: false, allowUndefinedValues: false }); - assertProtocol(base.patternflyOptions.urlWhitelist, base.patternflyOptions.urlWhitelistProtocols); + assertProtocol(base.whitelist.urls, base.whitelist.protocols); const baseLogging = isPlainObject(base.logging) ? base.logging : DEFAULT_OPTIONS.logging; const basePluginIsolation = PLUGIN_ISOLATION.includes(base.pluginIsolation) ? base.pluginIsolation : DEFAULT_OPTIONS.pluginIsolation; diff --git a/src/options.defaults.ts b/src/options.defaults.ts index e947bf52..0b23a6a0 100644 --- a/src/options.defaults.ts +++ b/src/options.defaults.ts @@ -189,8 +189,6 @@ interface ModeOptions { * @property default.versionStrategy Strategy to use when multiple PatternFly versions are detected. * - 'highest': Use the highest major version found. * - 'lowest': Use the lowest major version found. - * @property {WhitelistUrl[]} urlWhitelist List of allowed URLs to fetch PatternFly resources from. - * @property urlWhitelistProtocols List of allowed URL protocols to validate against when fetching PatternFly resources. */ interface PatternFlyOptions { availableResourceVersions: ('6.0.0')[]; @@ -202,9 +200,7 @@ interface PatternFlyOptions { latestSchemasVersion: 'v6'; versionWhitelist: string[]; versionStrategy: 'highest' | 'lowest'; - }, - urlWhitelist: WhitelistUrl[]; - urlWhitelistProtocols: string[]; + } } /** @@ -303,7 +299,7 @@ type WhitelistUrl = `${'http' | 'https'}://${string}`; */ interface WhitelistOptions { urls: WhitelistUrl[]; - protocols: string[]; + protocols: ('http' | 'https')[]; } /** @@ -455,7 +451,7 @@ const STATS_OPTIONS: StatsOptions = { }; /** - * Central whitelist. Spread into downstream consumers. + * Central outbound-URL policy. Single source of truth on `DefaultOptions.whitelist`. */ const WHITELIST_OPTIONS: WhitelistOptions = { urls: [ @@ -494,9 +490,7 @@ const PATTERNFLY_OPTIONS: PatternFlyOptions = { '@patternfly/patternfly' ], versionStrategy: 'highest' - }, - urlWhitelist: [...WHITELIST_OPTIONS.urls], - urlWhitelistProtocols: [...WHITELIST_OPTIONS.protocols] + } }; /** diff --git a/src/server.assertions.ts b/src/server.assertions.ts index 2edcf62d..3a7c5381 100644 --- a/src/server.assertions.ts +++ b/src/server.assertions.ts @@ -156,7 +156,7 @@ function assertInputStringNumberEnumLike( function assertInputUrlWhiteListed( input: unknown, whitelist: WhitelistUrl[], - { allowedProtocols = DEFAULT_OPTIONS.patternflyOptions.urlWhitelistProtocols, inputDisplayName, message, urlDisplayMaxLength = 50 }: { + { allowedProtocols = DEFAULT_OPTIONS.whitelist.protocols, inputDisplayName, message, urlDisplayMaxLength = 50 }: { allowedProtocols?: string[]; inputDisplayName?: string; message?: string; urlDisplayMaxLength?: number } = {} ): asserts input is string | string[] { diff --git a/src/tool.patternFlyDocs.ts b/src/tool.patternFlyDocs.ts index 1e3d8723..318e2226 100644 --- a/src/tool.patternFlyDocs.ts +++ b/src/tool.patternFlyDocs.ts @@ -55,7 +55,7 @@ const usePatternFlyDocsTool = (options = getOptions()): McpTool => { if (options.mode !== 'test') { assertInputUrlWhiteListed( urlList, - options.patternflyOptions.urlWhitelist, + options.whitelist.urls, { inputDisplayName: 'urlList' } ); } From 2ddc64c174d1bfddc431121d72e76eb9b367eb98 Mon Sep 17 00:00:00 2001 From: CD Cabrera Date: Thu, 16 Jul 2026 13:58:34 -0400 Subject: [PATCH 4/5] fix: review update --- guidelines/skills/add-docs-links/SKILL.md | 2 +- guidelines/skills/add-docs-links/reference.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/guidelines/skills/add-docs-links/SKILL.md b/guidelines/skills/add-docs-links/SKILL.md index 76c9b9d1..399d6e07 100644 --- a/guidelines/skills/add-docs-links/SKILL.md +++ b/guidelines/skills/add-docs-links/SKILL.md @@ -22,7 +22,7 @@ For **edits or removals**: find the row by `path` and/or its `docs` key; update - New ref for an existing repo: resolve SHA (e.g. commits API) or use a stable tag. 2. **Whitelist** - - `path` must match `patternflyOptions.urlWhitelist` in `src/options.defaults.ts`. See [reference.md — URL whitelist](reference.md#url-whitelist-allowed-domains). Use **https** only. Do not widen the whitelist in a catalog-only PR. + - `path` must match `whitelist.urls` in `src/options.defaults.ts`. See [reference.md — URL whitelist](reference.md#url-whitelist-allowed-domains). Use **https** only. Do not widen the whitelist in a catalog-only PR. 3. **Reachability** - Response must be 2xx (e.g. `curl -sI -o /dev/null -w "%{http_code}" ""`). If not, fix ref or path; do not add a dead link. diff --git a/guidelines/skills/add-docs-links/reference.md b/guidelines/skills/add-docs-links/reference.md index 9dbf72f0..5dce9f73 100644 --- a/guidelines/skills/add-docs-links/reference.md +++ b/guidelines/skills/add-docs-links/reference.md @@ -59,7 +59,7 @@ Each object in `docs.[]`: ## URL whitelist (allowed domains) -Defined in `src/options.defaults.ts` → `patternflyOptions.urlWhitelist`. Used when validating/fetching paths. +Defined in `src/options.defaults.ts` → `whitelist.urls`. Used when validating/fetching paths. - **Prefixes:** `https://patternfly.org`, `https://github.com/patternfly`, `https://raw.githubusercontent.com/patternfly` - **`docs.json`:** use **https** only. From 9c923a2c5a5856c09e516ce9f89c4fe12e897d39 Mon Sep 17 00:00:00 2001 From: CD Cabrera Date: Thu, 16 Jul 2026 15:03:48 -0400 Subject: [PATCH 5/5] fix: review update --- src/options.defaults.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/options.defaults.ts b/src/options.defaults.ts index 0b23a6a0..0e0f3ae8 100644 --- a/src/options.defaults.ts +++ b/src/options.defaults.ts @@ -291,8 +291,8 @@ type WhitelistUrl = `${'http' | 'https'}://${string}`; * Central outbound-URL policy. * * @note Any code that fetches a remote URL; PatternFly docs, - * `setFetch`, resource loaders; must validate against this - * list via `assertInputUrlWhiteListed`. + * resource loaders; must validate against this list via + * `assertInputUrlWhiteListed`. * * @property urls Allowed URL prefixes (scheme + host [+ path]). * @property protocols Allowed URL protocols.