feat(config): allow self-hosted admins to bypass allowedHeaders

docs/usage/configuration-options.md

@@ -2346,9 +2346,11 @@ By default, all headers starting with "X-" are allowed.
 
 A self-hosted administrator may configure an override for [`allowedHeaders`](./self-hosted-configuration.md#allowedheaders) to configure more permitted headers.
 
-`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).
+`headers` you configure in a repository config file, or a preset it extends, are checked against `allowedHeaders`.
 Any header which is not permitted is dropped, and a warning is logged.
 
+`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`.
+
 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.
 
 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

@@ -127,9 +127,15 @@ The `allowedHeaders` config option takes an array of minimatch-compatible globs
 For more details on this syntax see Renovate's [string pattern matching documentation](./string-pattern-matching.md).
 
 !!! note
+  `allowedHeaders` constrains what a repository, and the presets it extends, may set - it does not constrain you, the self-hosted administrator.
+  <br>
+  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`.
+  <br>
+  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`.
+  <br>
   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.
   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`.
-  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.
+  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`.
 
 Examples:
 

lib/config-validator.spec.ts

@@ -327,48 +327,33 @@ describe.concurrent('config-validator', () => {
   });
 
   describe('hostRules', () => {
-    it("filters the self-hosted admin's own hostRules headers against allowedHeaders", async () => {
+    it("does not drop the self-hosted admin's own hostRules headers against allowedHeaders when registering them", async () => {
+      // `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
+      //
+      // 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
       await withTmpDir(async (dirPath) => {
         const configFile = await writeGlobalConfig(dirPath, 'config.json', {
           allowedHeaders: ['X-*'],
           hostRules: [
             {
               matchHost: 'registry.example.com',
-              headers: { 'X-Allowed': 'yes', Authorization: 'denied' },
+              headers: { 'X-Allowed': 'yes', Authorization: 'from-admin' },
             },
           ],
         });
 
-        const { all } = await runValidator([], {
+        const { exitCode, all } = await runValidator([], {
           cwd: dirPath,
           env: { RENOVATE_CONFIG_FILE: configFile },
         });
 
-        expect(all).toContain(
-          "Ignoring hostRules headers not permitted by this Renovate instance's `allowedHeaders`",
-        );
-        expect(all).toContain('Authorization');
-      });
-    });
-
-    it("honors a CLI-arg global config file's own allowedHeaders", async () => {
-      // 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
-      await withTmpDir(async (dirPath) => {
-        const configFile = await writeGlobalConfig(dirPath, 'config.json', {
-          allowedHeaders: ['Authorization'],
-          hostRules: [
-            {
-              matchHost: 'registry.example.com',
-              headers: { Authorization: 'Bearer token' },
-            },
-          ],
-        });
-
-        const { all } = await runValidator([configFile], { cwd: dirPath });
-
         expect(all).not.toContain(
           "Ignoring hostRules headers not permitted by this Renovate instance's `allowedHeaders`",
         );
+        expect(exitCode).toBe(1);
+        expect(all).toContain(
+          "hostRules header `Authorization` is not allowed by this Renovate instance's `allowedHeaders`.",
+        );
       });
     });
 

lib/config-validator.ts

@@ -12,7 +12,7 @@ import { getConfigFileNames } from './config/app-strings.ts';
 import { GlobalConfig } from './config/global.ts';
 import { massageConfig } from './config/massage.ts';
 import { migrateConfig } from './config/migration.ts';
-import type { AllConfig, RenovateConfig } from './config/types.ts';
+import type { RenovateConfig } from './config/types.ts';
 import { validateConfig } from './config/validation.ts';
 import { pkg } from './expose.ts';
 import { init, logger } from './logger/index.ts';
@@ -42,9 +42,9 @@ async function partiallyGlobalInitialize(): Promise<void> {
   GlobalConfig.set(globalConfig);
 
   if (globalConfig.hostRules) {
-    // 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
+    // this is the self-hosted admin's own config, so its `headers` are exempt from `allowedHeaders` altogether - see `hostRules.add()`
     for (const hostRule of globalConfig.hostRules) {
-      addHostRule(hostRule);
+      addHostRule(hostRule, { trusted: true });
     }
   }
 }
@@ -57,15 +57,9 @@ async function validate(
   isPreset = false,
 ): Promise<void> {
   if (config.hostRules) {
-    // `allowedHeaders` enforces the checks regardless of whether it's global self-hosted administrator config, or repo config
-    // a global config file being validated brings its own `allowedHeaders`, and should be validated against it, like a real run would after parsing it
-    const allowedHeaders =
-      configType === 'global'
-        ? ((config as AllConfig).allowedHeaders ??
-          GlobalConfig.get('allowedHeaders'))
-        : GlobalConfig.get('allowedHeaders');
+    // 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`
     for (const hostRule of config.hostRules) {
-      addHostRule(hostRule, { allowedHeaders });
+      addHostRule(hostRule, { trusted: configType === 'global' });
     }
   }
   const { isMigrated, migratedConfig } = migrateConfig(config);

lib/config/options/index.ts

@@ -17,7 +17,7 @@ const options: Readonly<RenovateOptions>[] = [
   {
     name: 'allowedHeaders',
     description:
-      'List of allowed patterns for header names in hostRules config.',
+      'List of allowed patterns for header names in repository hostRules config.',
     type: 'array',
     default: ['X-*'],
     subType: 'string',

lib/util/host-rules.spec.ts

@@ -202,23 +202,23 @@ describe('util/host-rules', () => {
       expect(find({ url: 'https://registry.example.com' })).toEqual({});
     });
 
-    it('prefers an explicitly-passed allowlist over GlobalConfig', () => {
-      // used when registering rules for a repository before `GlobalConfig` reflects it, i.e. a `repositories[]` entry's own `allowedHeaders` override
+    it('does not filter a trusted rule against allowedHeaders', () => {
+      // `allowedHeaders` constrains what a repository or preset may set, not the self-hosted administrator - mirrors `allowedEnv`'s exemption of the admin's own `env`
+      GlobalConfig.reset();
+
       add(
         {
           matchHost: 'registry.example.com',
-          headers: { Authorization: 'from-admin', 'X-Dropped': 'yes' },
+          headers: { Authorization: 'from-admin' },
         },
-        { allowedHeaders: ['Authorization'] },
+        { trusted: true },
       );
 
       expect(find({ url: 'https://registry.example.com' })).toEqual({
         headers: { Authorization: 'from-admin' },
+        trustedHeaderNames: ['Authorization'],
       });
-      expect(logger.logger.warn).toHaveBeenCalledWith(
-        { denied: ['X-Dropped'] },
-        "Ignoring hostRules headers not permitted by this Renovate instance's `allowedHeaders`",
-      );
+      expect(logger.logger.warn).not.toHaveBeenCalled();
     });
   });
 
@@ -284,29 +284,6 @@ describe('util/host-rules', () => {
         "Ignoring hostRules headers not permitted by this Renovate instance's `allowedHeaders`",
       );
     });
-
-    it('prefers an explicitly-passed allowlist over GlobalConfig', () => {
-      expect(
-        filterAllowedHeaders(
-          [
-            {
-              matchHost: 'registry.example.com',
-              headers: { Authorization: 'from-admin', 'X-Dropped': 'yes' },
-            },
-          ],
-          ['Authorization'],
-        ),
-      ).toEqual([
-        {
-          matchHost: 'registry.example.com',
-          headers: { Authorization: 'from-admin' },
-        },
-      ]);
-      expect(logger.logger.warn).toHaveBeenCalledWith(
-        { denied: ['X-Dropped'] },
-        "Ignoring hostRules headers not permitted by this Renovate instance's `allowedHeaders`",
-      );
-    });
   });
 
   describe('find()', () => {
@@ -319,6 +296,21 @@ describe('util/host-rules', () => {
       expect(find({ abc: 'def' } as any)).toEqual({});
     });
 
+    it('returns a truly empty object for no match, not one with an undefined trustedHeaderNames', () => {
+      // `toEqual({})` alone would not catch this: it ignores `undefined`-valued properties, but callers elsewhere use `Object.keys(...).length`/`isNonEmptyObject` to detect an empty result
+      add(
+        {
+          matchHost: 'registry.example.com',
+          headers: { 'X-From-Admin': 'yes' },
+        },
+        { trusted: true },
+      );
+
+      expect(
+        Object.keys(find({ url: 'https://unrelated.example.com' })),
+      ).toEqual([]);
+    });
+
     it('needs exact host matches', () => {
       add(
         partial<LegacyHostRule & HostRule>({
@@ -562,6 +554,7 @@ describe('util/host-rules', () => {
       expect(find({ url: 'https://registry.example.com' })).toEqual({
         token: 'from-admin',
         headers: { 'X-From-Admin': 'yes', 'X-From-Repo': 'yes' },
+        trustedHeaderNames: ['X-From-Admin'],
       });
     });
 
@@ -583,6 +576,7 @@ describe('util/host-rules', () => {
         find({ url: 'https://registry.example.com/some/path/resource' }),
       ).toEqual({
         headers: { 'X-Custom': 'from-admin' },
+        trustedHeaderNames: ['X-Custom'],
       });
     });
 
@@ -623,9 +617,11 @@ describe('util/host-rules', () => {
 
       expect(find({ url: 'https://untrusted.example.com' })).toEqual({
         headers: { 'X-Other': 'yes' },
+        trustedHeaderNames: ['X-Other'],
       });
       expect(find({ url: 'https://trusted.example.com' })).toEqual({
         headers: { 'X-Api-Key': 'secret' },
+        trustedHeaderNames: ['X-Api-Key'],
       });
     });
 
@@ -649,6 +645,7 @@ describe('util/host-rules', () => {
       // the repo's narrower rule masks its own broader one, but cannot mask the admin's
       expect(find({ url: 'https://untrusted.example.com' })).toEqual({
         headers: { 'X-Other-Repo-Header': 'yes', 'X-From-Admin': 'yes' },
+        trustedHeaderNames: ['X-From-Admin'],
       });
     });
 
@@ -669,6 +666,7 @@ describe('util/host-rules', () => {
 
       expect(find({ url: 'https://registry.example.com' })).toEqual({
         headers: { 'X-From-Admin': 'from-repo', 'X-Other-Admin-Header': 'yes' },
+        trustedHeaderNames: ['X-Other-Admin-Header'],
       });
     });
 

lib/util/host-rules.ts

@@ -16,6 +16,18 @@ import { isHttpUrl, massageHostUrl, parseUrl } from './url.ts';
 interface RegisteredHostRule extends HostRule {
   /** whether the rule came from the self-hosted administrator's own global config, rather than from repository or preset config */
   trusted?: boolean;
+  /** set only by {@link find}; see {@link CombinedHostRuleWithTrustedHeaders.trustedHeaderNames} */
+  trustedHeaderNames?: string[];
+}
+
+/**
+ * A {@link CombinedHostRule} returned by {@link find}, additionally reporting which of its `headers` came from a `trusted` rule.
+ *
+ * `trustedHeaderNames` is deliberately not a field of `HostRule`, for the same reason `trusted` is not: it must never be settable through configuration.
+ */
+export interface CombinedHostRuleWithTrustedHeaders extends CombinedHostRule {
+  /** 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 */
+  trustedHeaderNames?: string[];
 }
 
 let hostRules: RegisteredHostRule[] = [];
@@ -70,22 +82,12 @@ export function migrateRule(rule: LegacyHostRule & HostRule): HostRule {
 /**
  * Enforce the `allowedHeaders` allowlist on a set of host rules.
  *
- * Loudly remove anything that's not permitted, logging a WARN.
- *
- * `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.
- *
- * @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
- * @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
+ * 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.
  *
  * 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.
  */
-export function filterAllowedHeaders(
-  rules: HostRule[],
-  allowedHeaders?: string[],
-  warnOnDenied = true,
-): HostRule[] {
-  // `??`, rather than a parameter default: a default only applies to `undefined`, and a `null` can reach us from user config
-  const allowlist = allowedHeaders ?? GlobalConfig.get('allowedHeaders');
+export function filterAllowedHeaders(rules: HostRule[]): HostRule[] {
+  const allowlist = GlobalConfig.get('allowedHeaders');
   const denied: string[] = [];
 
   const result = rules.map((rule) => {
@@ -116,7 +118,7 @@ export function filterAllowedHeaders(
     return filtered;
   });
 
-  if (denied.length && warnOnDenied) {
+  if (denied.length) {
     logger.warn(
       { denied },
       "Ignoring hostRules headers not permitted by this Renovate instance's `allowedHeaders`",
@@ -126,9 +128,6 @@ export function filterAllowedHeaders(
 }
 
 export interface AddHostRuleOptions {
-  /** the effective allowlist. Defaults to `GlobalConfig`; pass it explicitly when `GlobalConfig` does not yet reflect the repository the rule is registered for */
-  allowedHeaders?: string[];
-
   /**
    * Whether this rule comes from the self-hosted administrator's own global config, rather than from repository or preset config.
    *
@@ -140,15 +139,17 @@ export interface AddHostRuleOptions {
 export function add(params: HostRule, options?: AddHostRuleOptions): void {
   let rule: RegisteredHostRule = migrateRule(params);
 
-  // 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
+  // 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
   delete rule.trusted;
+  delete rule.trustedHeaderNames;
   if (options?.trusted) {
     rule.trusted = true;
   }
 
-  if (rule.headers) {
+  if (rule.headers && !rule.trusted) {
     // 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
-    [rule] = filterAllowedHeaders([rule], options?.allowedHeaders);
+    // `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
+    [rule] = filterAllowedHeaders([rule]);
   }
 
   if (rule.matchHost) {
@@ -258,7 +259,9 @@ function headersOfLastRuleToSetThem(
     .pop();
 }
 
-export function find(search: HostRuleSearch): CombinedHostRule {
+export function find(
+  search: HostRuleSearch,
+): CombinedHostRuleWithTrustedHeaders {
   if ([search.hostType, search.url].every(isFalsy)) {
     logger.warn({ search }, 'Invalid hostRules search');
     return {};
@@ -316,6 +319,12 @@ export function find(search: HostRuleSearch): CombinedHostRule {
   if (untrustedHeaders ?? trustedHeaders) {
     // the admin's own headers are applied last, so a repository cannot override one they set for this host either
     res.headers = { ...untrustedHeaders, ...trustedHeaders };
+    // 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`
+    // 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
+    // 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
+    res.trustedHeaderNames = trustedHeaders
+      ? Object.keys(trustedHeaders)
+      : undefined;
   }
 
   delete res.hostType;