Skip to content

Commit a01edff

Browse files
jamietannaClaude Sonnet 5
andcommitted
feat(config): allow self-hosted admins to bypass allowedHeaders
In a similar way to how `allowedEnv` works, we should allow a self-hosted admin to specify whichever headers they wish in their `hostRules` - whether in their global config, a `repositories[]` entry, or a preset a `repositories[]` entry extends. Previously, this would require a self-hosted admin to add any header they wished to set to `allowedHeaders`, which would also allow any of their users' repositories to set that same header. We can align this behaviour with `allowedEnv`, which makes sure that the admin's own `headers` are always applied regardless of `allowedHeaders`, while a repository's own `hostRules` - and those of any preset it extends - remain constrained by it as before. We do not currently allow an `extends` (without a `repositories[]` entry) to bypass, and they'll be followed-up with in #45670. Co-Authored-By: Claude Sonnet 5 <jamie.tanna+claude-code@mend.io> Co-Authored-By: Claude Opus 5 <jamie.tanna+claude-code@mend.io>
1 parent 7a6e7b2 commit a01edff

16 files changed

Lines changed: 184 additions & 249 deletions

docs/usage/configuration-options.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2346,9 +2346,11 @@ By default, all headers starting with "X-" are allowed.
23462346

23472347
A self-hosted administrator may configure an override for [`allowedHeaders`](./self-hosted-configuration.md#allowedheaders) to configure more permitted headers.
23482348

2349-
`headers` are checked against `allowedHeaders` wherever they are configured, including in the self-hosted administrator's own `hostRules` (for example in a `config.js` file).
2349+
`headers` you configure in a repository config file, or a preset it extends, are checked against `allowedHeaders`.
23502350
Any header which is not permitted is dropped, and a warning is logged.
23512351

2352+
`allowedHeaders` does not constrain the self-hosted administrator's own `hostRules` (for example in a `config.js` file, or a `repositories[]` entry): their `headers` are always applied, regardless of `allowedHeaders`.
2353+
23522354
When more than one of your host rules matches a request, the `headers` of the most specific matching rule are used, and replace the `headers` of the broader rules it matched alongside.
23532355

23542356
A self-hosted administrator's own host rules are resolved the same way, and whichever `headers` that leaves them sending to the host are then applied on top of yours:

docs/usage/self-hosted-configuration.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,9 +127,15 @@ The `allowedHeaders` config option takes an array of minimatch-compatible globs
127127
For more details on this syntax see Renovate's [string pattern matching documentation](./string-pattern-matching.md).
128128

129129
!!! note
130+
`allowedHeaders` constrains what a repository, and the presets it extends, may set - it does not constrain you, the self-hosted administrator.
131+
<br>
132+
The `headers` you set in your own `hostRules` - whether in your global config or in a `repositories[]` entry - are always applied, regardless of `allowedHeaders`, as is any `headers` set by a preset a `repositories[]` entry chose to `extends`.
133+
<br>
134+
This does not (yet) extend to a preset your global config itself chooses to `extends` when no `repositories[]` entry is involved: those `headers` are still constrained by `allowedHeaders`, unlike the equivalent case for `env`.
135+
<br>
130136
Where more than one of your own rules matches a request, the `headers` of the most specific rule are used, and replace those of the broader rules it matched alongside.
131137
So to keep a header away from a host that a broader rule of yours also matches, give that host a rule of its own which sets `headers`.
132-
Whichever of your `headers` that leaves for a request are then applied over any a repository set, and always with the value you set: a repository's `hostRules` - or those of a preset it extends - can neither stop one of them being sent, nor replace its value.
138+
Whichever of your `headers` that leaves for a request are then applied over any a repository set, and always with the value you set: a repository's `hostRules` - or those of a preset it extends - can neither stop one of them being sent, nor replace its value, and can only set a header name of its own where it is permitted by `allowedHeaders`.
133139

134140
Examples:
135141

lib/config-validator.spec.ts

Lines changed: 10 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -327,48 +327,33 @@ describe.concurrent('config-validator', () => {
327327
});
328328

329329
describe('hostRules', () => {
330-
it("filters the self-hosted admin's own hostRules headers against allowedHeaders", async () => {
330+
it("does not drop the self-hosted admin's own hostRules headers against allowedHeaders when registering them", async () => {
331+
// `allowedHeaders` constrains what a repository or preset may set, not the self-hosted administrator - `hostRules.add()` no longer filters (and warns about) the admin's own headers
332+
//
333+
// this does not (yet) extend to the separate, pre-existing top-level config security validation in `lib/config/validation.ts`, which still reports a global config's own `hostRules[].headers` outside its own `allowedHeaders` as a `Config security error` - the same limitation `allowedEnv`/`env` already have there, so `exitCode` is still 1 here
331334
await withTmpDir(async (dirPath) => {
332335
const configFile = await writeGlobalConfig(dirPath, 'config.json', {
333336
allowedHeaders: ['X-*'],
334337
hostRules: [
335338
{
336339
matchHost: 'registry.example.com',
337-
headers: { 'X-Allowed': 'yes', Authorization: 'denied' },
340+
headers: { 'X-Allowed': 'yes', Authorization: 'from-admin' },
338341
},
339342
],
340343
});
341344

342-
const { all } = await runValidator([], {
345+
const { exitCode, all } = await runValidator([], {
343346
cwd: dirPath,
344347
env: { RENOVATE_CONFIG_FILE: configFile },
345348
});
346349

347-
expect(all).toContain(
348-
"Ignoring hostRules headers not permitted by this Renovate instance's `allowedHeaders`",
349-
);
350-
expect(all).toContain('Authorization');
351-
});
352-
});
353-
354-
it("honors a CLI-arg global config file's own allowedHeaders", async () => {
355-
// a global config file brings its own `allowedHeaders`, and its hostRules should be filtered against those - not the surrounding environment's - as a real run would after parsing it
356-
await withTmpDir(async (dirPath) => {
357-
const configFile = await writeGlobalConfig(dirPath, 'config.json', {
358-
allowedHeaders: ['Authorization'],
359-
hostRules: [
360-
{
361-
matchHost: 'registry.example.com',
362-
headers: { Authorization: 'Bearer token' },
363-
},
364-
],
365-
});
366-
367-
const { all } = await runValidator([configFile], { cwd: dirPath });
368-
369350
expect(all).not.toContain(
370351
"Ignoring hostRules headers not permitted by this Renovate instance's `allowedHeaders`",
371352
);
353+
expect(exitCode).toBe(1);
354+
expect(all).toContain(
355+
"hostRules header `Authorization` is not allowed by this Renovate instance's `allowedHeaders`.",
356+
);
372357
});
373358
});
374359

lib/config-validator.ts

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { getConfigFileNames } from './config/app-strings.ts';
1212
import { GlobalConfig } from './config/global.ts';
1313
import { massageConfig } from './config/massage.ts';
1414
import { migrateConfig } from './config/migration.ts';
15-
import type { AllConfig, RenovateConfig } from './config/types.ts';
15+
import type { RenovateConfig } from './config/types.ts';
1616
import { validateConfig } from './config/validation.ts';
1717
import { pkg } from './expose.ts';
1818
import { init, logger } from './logger/index.ts';
@@ -42,9 +42,9 @@ async function partiallyGlobalInitialize(): Promise<void> {
4242
GlobalConfig.set(globalConfig);
4343

4444
if (globalConfig.hostRules) {
45-
// the self-hosted admin's own headers get no exemption from `allowedHeaders` either, as `applyHostRule` filters the rule it matches by header name alone whoever set it - so `addHostRule` drops them here too, with a WARN, rather than leave them to be silently discarded at request time
45+
// this is the self-hosted admin's own config, so its `headers` are exempt from `allowedHeaders` altogether - see `hostRules.add()`
4646
for (const hostRule of globalConfig.hostRules) {
47-
addHostRule(hostRule);
47+
addHostRule(hostRule, { trusted: true });
4848
}
4949
}
5050
}
@@ -57,15 +57,9 @@ async function validate(
5757
isPreset = false,
5858
): Promise<void> {
5959
if (config.hostRules) {
60-
// `allowedHeaders` enforces the checks regardless of whether it's global self-hosted administrator config, or repo config
61-
// a global config file being validated brings its own `allowedHeaders`, and should be validated against it, like a real run would after parsing it
62-
const allowedHeaders =
63-
configType === 'global'
64-
? ((config as AllConfig).allowedHeaders ??
65-
GlobalConfig.get('allowedHeaders'))
66-
: GlobalConfig.get('allowedHeaders');
60+
// a `global` config is the self-hosted administrator's own, so its `headers` are exempt from `allowedHeaders` altogether - see `hostRules.add()`; a `repo` config's `hostRules` are still constrained by this instance's `allowedHeaders`
6761
for (const hostRule of config.hostRules) {
68-
addHostRule(hostRule, { allowedHeaders });
62+
addHostRule(hostRule, { trusted: configType === 'global' });
6963
}
7064
}
7165
const { isMigrated, migratedConfig } = migrateConfig(config);

lib/config/options/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ const options: Readonly<RenovateOptions>[] = [
1717
{
1818
name: 'allowedHeaders',
1919
description:
20-
'List of allowed patterns for header names in hostRules config.',
20+
'List of allowed patterns for header names in repository hostRules config.',
2121
type: 'array',
2222
default: ['X-*'],
2323
subType: 'string',

lib/util/host-rules.spec.ts

Lines changed: 29 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -202,23 +202,23 @@ describe('util/host-rules', () => {
202202
expect(find({ url: 'https://registry.example.com' })).toEqual({});
203203
});
204204

205-
it('prefers an explicitly-passed allowlist over GlobalConfig', () => {
206-
// used when registering rules for a repository before `GlobalConfig` reflects it, i.e. a `repositories[]` entry's own `allowedHeaders` override
205+
it('does not filter a trusted rule against allowedHeaders', () => {
206+
// `allowedHeaders` constrains what a repository or preset may set, not the self-hosted administrator - mirrors `allowedEnv`'s exemption of the admin's own `env`
207+
GlobalConfig.reset();
208+
207209
add(
208210
{
209211
matchHost: 'registry.example.com',
210-
headers: { Authorization: 'from-admin', 'X-Dropped': 'yes' },
212+
headers: { Authorization: 'from-admin' },
211213
},
212-
{ allowedHeaders: ['Authorization'] },
214+
{ trusted: true },
213215
);
214216

215217
expect(find({ url: 'https://registry.example.com' })).toEqual({
216218
headers: { Authorization: 'from-admin' },
219+
trustedHeaderNames: ['Authorization'],
217220
});
218-
expect(logger.logger.warn).toHaveBeenCalledWith(
219-
{ denied: ['X-Dropped'] },
220-
"Ignoring hostRules headers not permitted by this Renovate instance's `allowedHeaders`",
221-
);
221+
expect(logger.logger.warn).not.toHaveBeenCalled();
222222
});
223223
});
224224

@@ -284,29 +284,6 @@ describe('util/host-rules', () => {
284284
"Ignoring hostRules headers not permitted by this Renovate instance's `allowedHeaders`",
285285
);
286286
});
287-
288-
it('prefers an explicitly-passed allowlist over GlobalConfig', () => {
289-
expect(
290-
filterAllowedHeaders(
291-
[
292-
{
293-
matchHost: 'registry.example.com',
294-
headers: { Authorization: 'from-admin', 'X-Dropped': 'yes' },
295-
},
296-
],
297-
['Authorization'],
298-
),
299-
).toEqual([
300-
{
301-
matchHost: 'registry.example.com',
302-
headers: { Authorization: 'from-admin' },
303-
},
304-
]);
305-
expect(logger.logger.warn).toHaveBeenCalledWith(
306-
{ denied: ['X-Dropped'] },
307-
"Ignoring hostRules headers not permitted by this Renovate instance's `allowedHeaders`",
308-
);
309-
});
310287
});
311288

312289
describe('find()', () => {
@@ -319,6 +296,21 @@ describe('util/host-rules', () => {
319296
expect(find({ abc: 'def' } as any)).toEqual({});
320297
});
321298

299+
it('returns a truly empty object for no match, not one with an undefined trustedHeaderNames', () => {
300+
// `toEqual({})` alone would not catch this: it ignores `undefined`-valued properties, but callers elsewhere use `Object.keys(...).length`/`isNonEmptyObject` to detect an empty result
301+
add(
302+
{
303+
matchHost: 'registry.example.com',
304+
headers: { 'X-From-Admin': 'yes' },
305+
},
306+
{ trusted: true },
307+
);
308+
309+
expect(
310+
Object.keys(find({ url: 'https://unrelated.example.com' })),
311+
).toEqual([]);
312+
});
313+
322314
it('needs exact host matches', () => {
323315
add(
324316
partial<LegacyHostRule & HostRule>({
@@ -562,6 +554,7 @@ describe('util/host-rules', () => {
562554
expect(find({ url: 'https://registry.example.com' })).toEqual({
563555
token: 'from-admin',
564556
headers: { 'X-From-Admin': 'yes', 'X-From-Repo': 'yes' },
557+
trustedHeaderNames: ['X-From-Admin'],
565558
});
566559
});
567560

@@ -583,6 +576,7 @@ describe('util/host-rules', () => {
583576
find({ url: 'https://registry.example.com/some/path/resource' }),
584577
).toEqual({
585578
headers: { 'X-Custom': 'from-admin' },
579+
trustedHeaderNames: ['X-Custom'],
586580
});
587581
});
588582

@@ -623,9 +617,11 @@ describe('util/host-rules', () => {
623617

624618
expect(find({ url: 'https://untrusted.example.com' })).toEqual({
625619
headers: { 'X-Other': 'yes' },
620+
trustedHeaderNames: ['X-Other'],
626621
});
627622
expect(find({ url: 'https://trusted.example.com' })).toEqual({
628623
headers: { 'X-Api-Key': 'secret' },
624+
trustedHeaderNames: ['X-Api-Key'],
629625
});
630626
});
631627

@@ -649,6 +645,7 @@ describe('util/host-rules', () => {
649645
// the repo's narrower rule masks its own broader one, but cannot mask the admin's
650646
expect(find({ url: 'https://untrusted.example.com' })).toEqual({
651647
headers: { 'X-Other-Repo-Header': 'yes', 'X-From-Admin': 'yes' },
648+
trustedHeaderNames: ['X-From-Admin'],
652649
});
653650
});
654651

@@ -669,6 +666,7 @@ describe('util/host-rules', () => {
669666

670667
expect(find({ url: 'https://registry.example.com' })).toEqual({
671668
headers: { 'X-From-Admin': 'from-repo', 'X-Other-Admin-Header': 'yes' },
669+
trustedHeaderNames: ['X-Other-Admin-Header'],
672670
});
673671
});
674672

lib/util/host-rules.ts

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,18 @@ import { isHttpUrl, massageHostUrl, parseUrl } from './url.ts';
1616
interface RegisteredHostRule extends HostRule {
1717
/** whether the rule came from the self-hosted administrator's own global config, rather than from repository or preset config */
1818
trusted?: boolean;
19+
/** set only by {@link find}; see {@link CombinedHostRuleWithTrustedHeaders.trustedHeaderNames} */
20+
trustedHeaderNames?: string[];
21+
}
22+
23+
/**
24+
* A {@link CombinedHostRule} returned by {@link find}, additionally reporting which of its `headers` came from a `trusted` rule.
25+
*
26+
* `trustedHeaderNames` is deliberately not a field of `HostRule`, for the same reason `trusted` is not: it must never be settable through configuration.
27+
*/
28+
export interface CombinedHostRuleWithTrustedHeaders extends CombinedHostRule {
29+
/** header names in `headers` that came from a `trusted` rule (the self-hosted administrator's own config), and so already bypassed `allowedHeaders` at registration - `applyHostRule`'s request-time defence-in-depth must not re-check, and drop, them */
30+
trustedHeaderNames?: string[];
1931
}
2032

2133
let hostRules: RegisteredHostRule[] = [];
@@ -70,22 +82,12 @@ export function migrateRule(rule: LegacyHostRule & HostRule): HostRule {
7082
/**
7183
* Enforce the `allowedHeaders` allowlist on a set of host rules.
7284
*
73-
* Loudly remove anything that's not permitted, logging a WARN.
74-
*
75-
* `add()` applies this to every rule it registers, so callers only need it themselves to pre-filter - e.g. to avoid repeating the WARN when the same rules are registered again and again.
76-
*
77-
* @param [allowedHeaders] the effective allowlist. Defaults to `GlobalConfig`, but must be passed explicitly when filtering before `GlobalConfig` reflects the repository being processed, i.e. for a `repositories[]` entry's own `allowedHeaders` override
78-
* @param [warnOnDenied=true] whether to log the WARN. Pass `false` where the very same rules are filtered again, so that it is logged once rather than repeated
85+
* Loudly remove anything that's not permitted, logging a WARN. Used by `add()` for an untrusted rule's `headers`; a `trusted` rule's are exempt, so never reach this.
7986
*
8087
* Headers that survive this allowlist are still subject to how {@link find} combines them: an admin's headers for a host are applied over those of any repository or preset rule matching the same request, so a repository can neither drop nor substitute them.
8188
*/
82-
export function filterAllowedHeaders(
83-
rules: HostRule[],
84-
allowedHeaders?: string[],
85-
warnOnDenied = true,
86-
): HostRule[] {
87-
// `??`, rather than a parameter default: a default only applies to `undefined`, and a `null` can reach us from user config
88-
const allowlist = allowedHeaders ?? GlobalConfig.get('allowedHeaders');
89+
export function filterAllowedHeaders(rules: HostRule[]): HostRule[] {
90+
const allowlist = GlobalConfig.get('allowedHeaders');
8991
const denied: string[] = [];
9092

9193
const result = rules.map((rule) => {
@@ -116,7 +118,7 @@ export function filterAllowedHeaders(
116118
return filtered;
117119
});
118120

119-
if (denied.length && warnOnDenied) {
121+
if (denied.length) {
120122
logger.warn(
121123
{ denied },
122124
"Ignoring hostRules headers not permitted by this Renovate instance's `allowedHeaders`",
@@ -126,9 +128,6 @@ export function filterAllowedHeaders(
126128
}
127129

128130
export interface AddHostRuleOptions {
129-
/** the effective allowlist. Defaults to `GlobalConfig`; pass it explicitly when `GlobalConfig` does not yet reflect the repository the rule is registered for */
130-
allowedHeaders?: string[];
131-
132131
/**
133132
* Whether this rule comes from the self-hosted administrator's own global config, rather than from repository or preset config.
134133
*
@@ -140,15 +139,17 @@ export interface AddHostRuleOptions {
140139
export function add(params: HostRule, options?: AddHostRuleOptions): void {
141140
let rule: RegisteredHostRule = migrateRule(params);
142141

143-
// set only from `options`, and dropped first so that it cannot be carried over from `params`: `HostRule` has no `trusted` field, but configuration is parsed from JSON, so a repository could otherwise smuggle one in and have its headers treated as the administrator's
142+
// set only from `options`, and dropped first so that it cannot be carried over from `params`: `HostRule` has no `trusted`/`trustedHeaderNames` field, but configuration is parsed from JSON, so a repository could otherwise smuggle one in and have its headers treated as the administrator's
144143
delete rule.trusted;
144+
delete rule.trustedHeaderNames;
145145
if (options?.trusted) {
146146
rule.trusted = true;
147147
}
148148

149-
if (rule.headers) {
149+
if (rule.headers && !rule.trusted) {
150150
// enforced here, at the single registration chokepoint, so that no current or future caller can register a rule whose headers bypass `allowedHeaders`; `applyHostRule` filters by header name again at request time as defence in depth
151-
[rule] = filterAllowedHeaders([rule], options?.allowedHeaders);
151+
// `allowedHeaders` constrains what a repository or preset may set, not the self-hosted administrator - the same exemption `filterAllowedEnv` gives the admin's own `env`, just enforced through `trusted` here rather than a name+value comparison, as `headers` are host-scoped
152+
[rule] = filterAllowedHeaders([rule]);
152153
}
153154

154155
if (rule.matchHost) {
@@ -258,7 +259,9 @@ function headersOfLastRuleToSetThem(
258259
.pop();
259260
}
260261

261-
export function find(search: HostRuleSearch): CombinedHostRule {
262+
export function find(
263+
search: HostRuleSearch,
264+
): CombinedHostRuleWithTrustedHeaders {
262265
if ([search.hostType, search.url].every(isFalsy)) {
263266
logger.warn({ search }, 'Invalid hostRules search');
264267
return {};
@@ -316,6 +319,12 @@ export function find(search: HostRuleSearch): CombinedHostRule {
316319
if (untrustedHeaders ?? trustedHeaders) {
317320
// the admin's own headers are applied last, so a repository cannot override one they set for this host either
318321
res.headers = { ...untrustedHeaders, ...trustedHeaders };
322+
// set as a pair with `headers`, even to `undefined`, rather than only when there are trusted headers: `findMatchingRule`'s hostType fallbacks build on `find()` with `{ ...fallbackResult, ...res }`, so `res`'s own `trustedHeaderNames` must shadow a fallback's own whenever `res.headers` does the same to a fallback's `headers` - otherwise a fallback's `trustedHeaderNames` could survive alongside `res`'s own, unrelated `headers`
323+
// never set when `res.headers` isn't either, so a `find()` with no matching headers still returns `{}` rather than `{ trustedHeaderNames: undefined }`, which callers that check for an empty result (e.g. `isNonEmptyObject`) rely on
324+
// tracked so that `applyHostRule`'s request-time defence-in-depth knows which of `res.headers` already bypassed `allowedHeaders` at registration as the administrator's own, and does not re-drop them
325+
res.trustedHeaderNames = trustedHeaders
326+
? Object.keys(trustedHeaders)
327+
: undefined;
319328
}
320329

321330
delete res.hostType;

0 commit comments

Comments
 (0)