Add environment variable support for config files by kapral18 · Pull Request #547 · sorenlouv/backport

kapral18 · 2025-10-11T12:49:12Z

Summary

Adds automatic fallback to BACKPORT_ACCESS_TOKEN environment variable when accessToken is missing/empty in config files. Also introduces Zod for config validation.

Key Changes

Environment Variable Fallback:

Falls back to BACKPORT_ACCESS_TOKEN when accessToken is missing, empty, or whitespace
Precedence: CLI > config > env variable
accessToken key can be omitted entirely from config files

Zod Validation:

Validates CLI args and config files at entry points (fail fast)
partialConfigSchema: validates structure without applying defaults
getDefaultConfigOptions(): single source of truth for defaults
Manual merge in options.ts: explicit, debuggable precedence

Schema Scope (No Breaking Changes):

✅ Validates all serializable options (CLI args, config files)
✅ Excludes non-serializable fields (e.g., autoFixConflicts function handler)
✅ Includes deprecated options for backward compatibility
✅ All fields remain optional as before

Why This Approach?

BACKPORT_ACCESS_TOKEN vs ${VAR} substitution:

Focused on the specific problem (keeping tokens out of config)
Simpler (no parsing, no edge cases)
Self-documenting

Validate at entry, merge manually:

Clear error messages (know which source failed)
Explicit precedence visible in code
Easy to debug

Exclude non-serializable fields:

CLI/config files can only contain JSON-serializable data
Function handlers (like autoFixConflicts) only available via module API
Realistic validation scope

Usage

Config file:

{
  // Omit accessToken or leave empty
  "accessToken": ""
}

Environment:

export BACKPORT_ACCESS_TOKEN="ghp_..."
backport

CLI override:

backport --accessToken="ghp_different"

Testing

130 tests pass (6 test suites):

BACKPORT_ACCESS_TOKEN fallback: 7 tests
Zod validation (cli-args): 6 tests
Zod validation (config files): 10 tests
Documentation: 1 test

Migration

None needed - existing configs work as before. To use env variable: set BACKPORT_ACCESS_TOKEN and omit/empty accessToken in config.

Commits

Original ${VAR} substitution (kept for reference)
Current approach with BACKPORT_ACCESS_TOKEN + Zod

sorenlouv · 2025-10-30T10:13:24Z

docs/config-file-options.md

+
+```json
+{
+  "accessToken": "${GITHUB_ACCESS_TOKEN}"


I'm not sure this is the right approach. I think selected environment variables should be supported - like GITHUB_ACCESS_TOKEN but it shouldn't require updating the config.

If GITHUB_ACCESS_TOKEN is set it should be used as a fallback if there's no access token in the config.

Btw the config management is currently extremely messy. There are config options, cli options, "module" options, and now also environment options.

I have been considering updating the config management. Moving all config definitions (and defaults) to a single file and using zod for validation instead of relying on yargs for everything. yargs should just be used to parse the cli args.

can you check if this new commit is closer to what you want? Updated PR as well

sorenlouv · 2025-11-07T22:44:33Z

src/options/cli-args.test.ts

    });
  });
+
+  describe('zod validation', () => {


zod should be an implementation detail. See how the other args are tested

✅ Done - removed all zod tests from this file

sorenlouv · 2025-11-07T22:45:25Z

src/options/config-schema.ts

+// Schema for partial config (from files/cli) - without defaults applied
+// This is used for parsing CLI args and config files where we don't want defaults
+// Defaults are applied later via manual merge in options.ts
+export const partialConfigSchema = baseConfigSchema.loose();


Why is this change needed for adding environment variable support?

I was attempting to add zod-based validation for your earlier comment about config management being messy. My schema validated all serializable options but excluded non-serializable fields like autoFixConflicts (function handlers can't be in config files). You're right that this isn't truly comprehensive validation - it's partial validation of the serializable subset. Since that's outside the scope of the env var feature, I've removed all the zod code.

sorenlouv · 2025-11-07T22:46:45Z

src/options/config/global-config.test.ts

      );
    });

+    it('should include BACKPORT_ACCESS_TOKEN documentation in template', async () => {


This test seems really unnecessary. It test the existence of a comment?

My thinking was if it's part of the public interface it can be tested, but either way I'm okay with removing it. Removed.

sorenlouv · 2025-11-07T22:50:05Z

src/options/config/read-config-file.test.ts

+import { BackportError } from '../../lib/backport-error';
+import { parseConfigFile, readConfigFile } from './read-config-file';
+
+describe('parseConfigFile', () => {


Thanks for adding this test.

sorenlouv · 2025-11-07T22:50:25Z

src/options/config/read-config-file.test.ts

+    });
+  });
+
+  describe('zod validation', () => {


As mentioned above, zod should be an implementation detail

✅ Done - removed all zod validation tests

sorenlouv · 2025-11-07T23:04:05Z

src/options/options.test.ts

+      const originalEnv = process.env;
+
+      beforeEach(() => {
+        process.env = { ...originalEnv };
+        mockGithubConfigOptions({
+          viewerLogin: 'testuser',
+        });
+      });
+
+      afterEach(() => {
+        process.env = originalEnv;
+      });


Why are you calling mockGithubConfigOptions?

Suggested change

const originalEnv = process.env;

beforeEach(() => {

process.env = { ...originalEnv };

mockGithubConfigOptions({

viewerLogin: 'testuser',

});

});

afterEach(() => {

process.env = originalEnv;

});

afterEach(() => {

delete process.env.BACKPORT_ACCESS_TOKEN;

});

I was setting up the mock out of habit from other tests, but it's not needed here since we're only testing the env var fallback. Fixed - removed the unnecessary mock call.

sorenlouv · 2025-11-07T23:07:21Z

src/options/options.ts

+  const resolvedAccessToken =
+    // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
+    accessToken?.trim() ||
+    // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
+    process.env.BACKPORT_ACCESS_TOKEN?.trim() ||
+    undefined;


Rename to trimmedAccessToken. "resolved" makes it sounds like an async value

Suggested change

const resolvedAccessToken =

// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing

accessToken?.trim() ||

// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing

process.env.BACKPORT_ACCESS_TOKEN?.trim() ||

undefined;

const trimmedAccessToken =

// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing

accessToken?.trim() || process.env.BACKPORT_ACCESS_TOKEN?.trim();

sorenlouv · 2025-11-07T23:11:58Z

src/options/config-schema.ts

+  help: z.boolean().optional(),
+  version: z.boolean().optional(),
+  v: z.boolean().optional(),
+});


I'd like to introduce zod, but in that case it should replace the the existing validation. I think that's outside the scope of this change

Agreed - removed all zod code. The PR now focuses solely on the BACKPORT_ACCESS_TOKEN fallback feature.

Adds support for environment variable substitution in both global (.backport/config.json) and project (.backportrc.json) configuration files using the syntax ${VARIABLE_NAME}. Features: - Substitutes ${VAR} with process.env.VAR value - Fails fast with clear error if variable is undefined - Fails fast with clear error if variable is empty string - Consistent with codebase's fail-fast validation philosophy - Particularly useful for CI/CD environments (GitHub Actions) Benefits: - Keep secrets out of version control - Share config files across teams safely - Works seamlessly in CI/CD pipelines - Clear error messages for missing/empty variables Example usage: { "accessToken": "${GITHUB_ACCESS_TOKEN}", "repoOwner": "elastic", "repoName": "kibana" } Includes comprehensive test coverage (18 test cases) and documentation updates.

…lback This commit completely replaces the original ${VARIABLE_NAME} substitution approach with a more focused solution: automatic fallback to the BACKPORT_ACCESS_TOKEN environment variable when accessToken is missing, empty, or whitespace-only. Changes: - Remove ${VAR} substitution logic from config file parsing - Add BACKPORT_ACCESS_TOKEN environment variable fallback in options.ts - Add zod for config validation (validates CLI args and config files) - Centralize config schema definitions in config-schema.ts - Use partialConfigSchema for entry point validation - Use getDefaultConfigOptions() for explicit default merging - Update global config template to document BACKPORT_ACCESS_TOKEN - Add comprehensive test coverage for fallback behavior Design decisions: - Validate early at entry points (CLI args, config files) - Apply defaults via explicit manual merge (not zod) - Maintain clear precedence: CLI > remote > config > env > defaults - Use || operator for fallback (treats empty/whitespace as missing) - Allow accessToken key to be omitted entirely from config files The zod implementation uses a clean architecture: - baseConfigSchema (internal): All fields optional, no defaults - partialConfigSchema (exported): Used for validation at entry points - getDefaultConfigOptions(): Single source of truth for defaults - Manual merge in options.ts: Explicit, debuggable precedence Schema validation scope: - Validates all serializable config options (CLI args, config files) - Intentionally excludes non-serializable fields (e.g., autoFixConflicts function handler) - Includes deprecated options for backward compatibility - No breaking changes: all fields remain optional as before Test coverage: - 130 tests pass (6 test suites in src/options/) - BACKPORT_ACCESS_TOKEN fallback: 7 test cases - Zod validation in cli-args: 6 test cases - Zod validation in read-config-file: 10 test cases - Global config documentation: 1 test case

kapral18 force-pushed the add-env-var-support-config branch from cf8528d to 2fc6e14 Compare October 11, 2025 13:03

sorenlouv reviewed Oct 30, 2025

View reviewed changes

kapral18 requested a review from sorenlouv October 30, 2025 23:04

kapral18 force-pushed the add-env-var-support-config branch 6 times, most recently from 7a894a3 to 8037e59 Compare October 30, 2025 23:35

sorenlouv reviewed Nov 7, 2025

View reviewed changes

kapral18 added 3 commits November 17, 2025 21:26

refactor: revert zod, address comments

4d48f0d

kapral18 force-pushed the add-env-var-support-config branch from 8037e59 to 4d48f0d Compare November 18, 2025 12:11

kapral18 requested a review from sorenlouv November 18, 2025 20:23

Conversation

kapral18 commented Oct 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Key Changes

Why This Approach?

Usage

Testing

Migration

Commits

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kapral18 Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kapral18 commented Oct 11, 2025 •

edited

Loading

kapral18 Oct 30, 2025 •

edited

Loading