mirror of
https://github.com/renovatebot/renovate.git
synced 2026-10-27 07:50:47 +00:00
fix(config): enforce allowedHeaders when applying headers
As an additional defence-in-depth, we can make sure that when applying the `headers` configuration (from repo config, or presets) we re-validate it. We allow any admin-supplied configuration regardless, and then any user-supplied settings are re-filtered according to `allowedHeaders`. Due to the existing way that header merging incorrectly merges the repo (or preset) `headers` over the admin config, we don't currently handle this explicitly, unlike `filterAllowedEnv`. If a user-supplied setting is filtered, we log a WARN message to inform the user - and the admin - that this is ignored. At this point, we don't try and explain which preset(s) this may have come from, only that there is an issue. Co-authored-by: Claude Opus 4.8 <jamie.tanna+claude-code@mend.io> Co-authored-by: Claude Opus 5 <jamie.tanna+claude-code@mend.io>
This commit is contained in:
@@ -2842,6 +2842,7 @@ const options: Readonly<RenovateOptions>[] = [
|
||||
stage: 'repository',
|
||||
cli: true,
|
||||
mergeable: true,
|
||||
requiresCheckAtTrustBoundary: true,
|
||||
},
|
||||
{
|
||||
name: 'hostType',
|
||||
|
||||
@@ -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-*'],
|
||||
|
||||
@@ -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,
|
||||
)) {
|
||||
|
||||
@@ -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`",
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -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<string, string> = {};
|
||||
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;
|
||||
}
|
||||
@@ -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')
|
||||
|
||||
@@ -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<string | null> {
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user