diff --git a/lib/config/options/index.ts b/lib/config/options/index.ts index 42d92a34ed..9bef86a34c 100644 --- a/lib/config/options/index.ts +++ b/lib/config/options/index.ts @@ -2842,6 +2842,7 @@ const options: Readonly[] = [ stage: 'repository', cli: true, mergeable: true, + requiresCheckAtTrustBoundary: true, }, { name: 'hostType', diff --git a/lib/config/validation.spec.ts b/lib/config/validation.spec.ts index a11914341d..08c97c520b 100644 --- a/lib/config/validation.spec.ts +++ b/lib/config/validation.spec.ts @@ -3245,6 +3245,58 @@ describe('config/validation', () => { expect(warnings).toBeEmptyArray(); }); + it('allows `env` within `force` inside the global `allowedEnv`', async () => { + const config: AllConfig = { + allowedEnv: ['PATH'], + force: { env: { PATH: '/home/ubuntu/bin' } }, + }; + const { warnings, errors } = await configValidation.validateConfig( + 'global', + config, + ); + expect(errors).toBeEmptyArray(); + expect(warnings).toBeEmptyArray(); + }); + + it('reports `env` within `force` outside the global `allowedEnv`', async () => { + const config: AllConfig = { + allowedEnv: ['PATH'], + force: { env: { NOT_ALLOWED: 'value' } }, + }; + const { warnings, errors } = await configValidation.validateConfig( + 'global', + config, + ); + expect(errors).toBeEmptyArray(); + expect(warnings).toMatchObject([ + { + topic: 'Configuration Error', + message: + "Env variable name `NOT_ALLOWED` is not allowed by this Renovate instance's `allowedEnv`.", + }, + ]); + }); + + it('allows hostRules `headers` within `force` inside the global `allowedHeaders`', async () => { + const config: AllConfig = { + allowedHeaders: ['X-Custom-*'], + force: { + hostRules: [ + { + matchHost: 'https://domain.com/all-versions', + headers: { 'X-Custom-Token': 'token' }, + }, + ], + }, + }; + const { warnings, errors } = await configValidation.validateConfig( + 'global', + config, + ); + expect(errors).toBeEmptyArray(); + expect(warnings).toBeEmptyArray(); + }); + it('allows hostRules `headers` on repositories[] entries within the global `allowedHeaders`', async () => { const config: AllConfig = { allowedHeaders: ['X-Custom-*'], diff --git a/lib/config/validation.ts b/lib/config/validation.ts index 393ad6bfa1..5df4245342 100644 --- a/lib/config/validation.ts +++ b/lib/config/validation.ts @@ -1300,7 +1300,15 @@ async function validateGlobalConfig( warnings.push(warning); } } else if (key === 'force') { - const subValidation = await validateConfig('global', val); + // `force` is validated as a global config of its own, so it does not automatically see the top-level `allowedEnv`/`allowedHeaders`. + // Inherit them (unless `force` sets its own) so that the self-hosted admin's own `force.env`/`force.hostRules[].headers` are validated against the allowlists they set, rather than an empty one. + const subValidation = await validateConfig('global', { + ...(config.allowedEnv ? { allowedEnv: config.allowedEnv } : {}), + ...(config.allowedHeaders + ? { allowedHeaders: config.allowedHeaders } + : {}), + ...val, + }); for (const warning of subValidation.warnings.concat( subValidation.errors, )) { diff --git a/lib/workers/repository/init/filter-allowed-headers.spec.ts b/lib/workers/repository/init/filter-allowed-headers.spec.ts new file mode 100644 index 0000000000..1e172c8ef1 --- /dev/null +++ b/lib/workers/repository/init/filter-allowed-headers.spec.ts @@ -0,0 +1,79 @@ +import { logger } from '~test/util.ts'; +import { GlobalConfig } from '../../../config/global.ts'; +import { filterAllowedHeaders } from './filter-allowed-headers.ts'; + +describe('workers/repository/init/filter-allowed-headers', () => { + beforeEach(() => { + GlobalConfig.reset(); + }); + + it('leaves rules without headers untouched', () => { + const rules = [{ matchHost: 'registry.example.com' }]; + + expect(filterAllowedHeaders(rules)).toEqual(rules); + expect(logger.logger.warn).not.toHaveBeenCalled(); + }); + + it('keeps headers matching allowedHeaders and drops the rest with a warning', () => { + GlobalConfig.set({ allowedHeaders: ['X-*'] }); + + expect( + filterAllowedHeaders([ + { + matchHost: 'registry.example.com', + headers: { 'X-Allowed': 'yes', Authorization: 'Bearer secret' }, + }, + ]), + ).toEqual([ + { + matchHost: 'registry.example.com', + headers: { 'X-Allowed': 'yes' }, + }, + ]); + expect(logger.logger.warn).toHaveBeenCalledWith( + { denied: ['Authorization'] }, + "Ignoring hostRules headers not permitted by this Renovate instance's `allowedHeaders`", + ); + }); + + it('drops all headers when allowedHeaders is unset (deny by default)', () => { + expect( + filterAllowedHeaders([ + { matchHost: 'registry.example.com', headers: { anything: 'x' } }, + ]), + ).toEqual([{ matchHost: 'registry.example.com', headers: {} }]); + }); + + it('leaves everything but the headers untouched', () => { + GlobalConfig.set({ allowedHeaders: ['X-*'] }); + const rules = [ + { + matchHost: 'registry.example.com', + hostType: 'npm', + username: 'user', + password: 'pass', + token: 'token', + headers: { 'X-Allowed': 'yes' }, + }, + ]; + + expect(filterAllowedHeaders(rules)).toEqual(rules); + expect(logger.logger.warn).not.toHaveBeenCalled(); + }); + + it('drops a header the self-hosted admin supplied themselves', () => { + // `allowedHeaders` enforces the checks regardless of whether it's global self-hosted administrator config, or repo config + expect( + filterAllowedHeaders([ + { + matchHost: 'registry.example.com', + headers: { Authorization: 'set-by-admin' }, + }, + ]), + ).toEqual([{ matchHost: 'registry.example.com', headers: {} }]); + expect(logger.logger.warn).toHaveBeenCalledWith( + { denied: ['Authorization'] }, + "Ignoring hostRules headers not permitted by this Renovate instance's `allowedHeaders`", + ); + }); +}); diff --git a/lib/workers/repository/init/filter-allowed-headers.ts b/lib/workers/repository/init/filter-allowed-headers.ts new file mode 100644 index 0000000000..963ea227c6 --- /dev/null +++ b/lib/workers/repository/init/filter-allowed-headers.ts @@ -0,0 +1,46 @@ +import { GlobalConfig } from '../../../config/global.ts'; +import { logger } from '../../../logger/index.ts'; +import type { HostRule } from '../../../types/index.ts'; +import { matchRegexOrGlobList } from '../../../util/string-match.ts'; + +/** + * Enforce the `allowedHeaders` allowlist before the host rules are applied. + * + * Loudly remove anything that's not permitted, logging a WARN. + * + * Unlike {@link filterAllowedEnv}, it is not currently possible for a self-hosted admin's headers to take precedence, as we lean on the "merging" logic (which currently, incorrectly, takes only repo-level `headers` if they're set for a given host). + */ +export function filterAllowedHeaders(rules: HostRule[]): HostRule[] { + const allowedHeaders = GlobalConfig.get('allowedHeaders'); + const denied: string[] = []; + + const result = rules.map((rule) => { + if (!rule.headers) { + return rule; + } + + const allowed: Record = {}; + const ruleDenied: string[] = []; + for (const [name, value] of Object.entries(rule.headers)) { + if (matchRegexOrGlobList(name, allowedHeaders)) { + allowed[name] = value; + } else { + ruleDenied.push(name); + } + } + + if (!ruleDenied.length) { + return rule; + } + denied.push(...ruleDenied); + return { ...rule, headers: allowed }; + }); + + if (denied.length) { + logger.warn( + { denied }, + "Ignoring hostRules headers not permitted by this Renovate instance's `allowedHeaders`", + ); + } + return result; +} diff --git a/lib/workers/repository/init/merge.spec.ts b/lib/workers/repository/init/merge.spec.ts index ec9eacf78e..47bfb5d1ea 100644 --- a/lib/workers/repository/init/merge.spec.ts +++ b/lib/workers/repository/init/merge.spec.ts @@ -790,7 +790,9 @@ describe('workers/repository/init/merge', () => { ); }); - it('incorrectly applies `repositories[]` entry headers, if it is NOT in `allowedHeaders`', async () => { + it('drops `repositories[]` entry headers, if it is not in `allowedHeaders`', async () => { + // previously this would apply due to a gap in re-validating `allowedHeaders` against the resolved config. + // `applyHostRules` filters by header name at request time, so this does not reach the final HTTP call, but we should make sure this also doesn't break GlobalConfig.set({ allowedHeaders: ['X-*'] }); fs.readLocalFile.mockResolvedValue(JSON.stringify({})); @@ -807,13 +809,17 @@ describe('workers/repository/init/merge', () => { }); expect(hostRules.find({ url: 'https://registry.example.com' })).toEqual( - { headers: { Authorization: 'from-admin' } }, + { headers: {} }, + ); + expect(logger.logger.warn).toHaveBeenCalledWith( + { denied: ['Authorization'] }, + "Ignoring hostRules headers not permitted by this Renovate instance's `allowedHeaders`", ); }); - it('incorrectly applies `hostRules` headers injected by a preset, if it is NOT in `allowedHeaders`', async () => { - // `allowedHeaders` is only enforced by `validateConfig`, against the config as written, and nothing re-checks the resolved config before `hostRules.add`. - // `applyHostRules` filters by header name at request time, so this does not reach the final HTTP call - but it is applied here, and nothing reports it + it('drops `hostRules` headers injected by a preset, if it is not in `allowedHeaders`', async () => { + // previously this would apply due to a gap in re-validating `allowedHeaders` against the resolved config. + // `applyHostRules` filters by header name at request time, so this does not reach the final HTTP call, but we should make sure this also doesn't break GlobalConfig.set({ allowedHeaders: ['X-*'] }); memCache.set('preset:local>headerPreset', { hostRules: [ @@ -830,7 +836,11 @@ describe('workers/repository/init/merge', () => { await mergeRenovateConfig(config); expect(hostRules.find({ url: 'https://registry.example.com' })).toEqual( - { headers: { Authorization: 'from-preset' } }, + { headers: {} }, + ); + expect(logger.logger.warn).toHaveBeenCalledWith( + { denied: ['Authorization'] }, + "Ignoring hostRules headers not permitted by this Renovate instance's `allowedHeaders`", ); }); }); @@ -926,6 +936,28 @@ describe('workers/repository/init/merge', () => { expect(config.hostRules).toBeUndefined(); }); + it('filters headers against allowedHeaders before adding', () => { + GlobalConfig.set({ allowedHeaders: ['X-*'] }); + const addSpy = vi + .spyOn(hostRules, 'add') + .mockImplementation(() => undefined); + const config = { + hostRules: [ + { + matchHost: 'registry.example.com', + headers: { 'X-Allowed': 'yes', Authorization: 'Bearer secret' }, + }, + ], + }; + + applyHostRules(config); + + expect(addSpy).toHaveBeenCalledExactlyOnceWith({ + matchHost: 'registry.example.com', + headers: { 'X-Allowed': 'yes' }, + }); + }); + it('warns on invalid hostRule and continues applying others', () => { const addSpy = vi .spyOn(hostRules, 'add') diff --git a/lib/workers/repository/init/merge.ts b/lib/workers/repository/init/merge.ts index 5f814abc2c..cb7580e79f 100644 --- a/lib/workers/repository/init/merge.ts +++ b/lib/workers/repository/init/merge.ts @@ -48,6 +48,7 @@ import { getDefaultConfigFileName, } from '../onboarding/common.ts'; import { filterAllowedEnv } from './filter-allowed-env.ts'; +import { filterAllowedHeaders } from './filter-allowed-headers.ts'; import type { RepoFileConfig, RepositoryWorkerConfig } from './types.ts'; export async function detectConfigFile(): Promise { @@ -371,7 +372,7 @@ export function applyHostRules(config: RenovateConfig): void { } logger.debug('Setting hostRules from config'); - for (const rule of config.hostRules) { + for (const rule of filterAllowedHeaders(config.hostRules)) { try { hostRules.add(rule); } catch (err) {