ENG-2005-Environment-variable overrides for all config options by icanhasmath · Pull Request #3843 · ActiveState/cli

icanhasmath · 2026-07-15T19:50:25Z

Summary

Every registered config option can now be overridden by an environment variable, so configuration can be applied machine-wide (all users) without a shared config file and without sharing auth. Supersedes #3842 (the config-file approach).

Canonical + legacy env vars

Each option gets a canonical override auto-derived from its key (ACTIVESTATE_CONFIG_<KEY>, dots/dashes → underscores, uppercased):

Config key	Canonical env var
`autoupdate`	`ACTIVESTATE_CONFIG_AUTOUPDATE`
`api.host`	`ACTIVESTATE_CONFIG_API_HOST`
`update.info.endpoint`	`ACTIVESTATE_CONFIG_UPDATE_INFO_ENDPOINT`
`privateingredient.mtls_cert`	`ACTIVESTATE_CONFIG_PRIVATEINGREDIENT_MTLS_CERT`
…all 22	`ACTIVESTATE_CONFIG_*`

The five options that already had bespoke env vars keep working as aliases (ACTIVESTATE_API_HOST, ACTIVESTATE_CLI_ANALYTICS_PIXEL, ACTIVESTATE_NOTIFICATIONS_OVERRIDE, ACTIVESTATE_TEST_UPDATE_URL, ACTIVESTATE_TEST_UPDATE_INFO_URL). The canonical variable wins if both are set.

Behavior

Resolved in config.Instance.Get with precedence env > stored > default, so overrides take effect everywhere, not just in display.
Values are coerced to the option's type (bool/int/string/enum).
Only registered options are eligible — credentials such as apiToken are not registered options, so they stay out of scope (no shared auth).

`state config` visibility

Reports the effective value and a Source column showing the overriding env var name, local (user-set), or default.
JSON output (state config --output json) gains source, env (active var), and envVar (canonical var) fields.

Operational note

state-svc must inherit these env vars to honor keys it consumes (e.g. notifications.endpoint) — set them system-wide (/etc/environment, service environment) so the daemon sees them too.

Testing

New unit tests: canonical derivation, alias precedence, type coercion, empty-var handling (registry_test.go); effective-value/source and get behavior (env_test.go).
Verified end-to-end with a real binary across bool/int/string keys, canonical and legacy vars.

🤖 Generated with Claude Code

Every registered config option can now be overridden by an environment variable, so config can be applied machine-wide (all users) without a shared config file and without sharing auth. Each option gets a canonical override derived from its key, e.g. "update.info.endpoint" -> ACTIVESTATE_CONFIG_UPDATE_INFO_ENDPOINT. The five options that already had bespoke env vars (ACTIVESTATE_API_HOST, ACTIVESTATE_CLI_ANALYTICS_PIXEL, ACTIVESTATE_NOTIFICATIONS_OVERRIDE, ACTIVESTATE_TEST_UPDATE_URL, ACTIVESTATE_TEST_UPDATE_INFO_URL) keep working as aliases; the canonical variable takes precedence when both are set. Overrides are resolved in config.Instance.Get (env > stored > default) so they actually take effect everywhere, not just in display. Values are coerced to the option's type. Only registered options are eligible, which keeps credentials (e.g. apiToken, not a registered option) out of scope. `state config` now reports the effective value and a Source column showing the overriding env var name, "local", or "default"; JSON output gains source, env, and envVar fields. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Adds environment-variable overrides for registered configuration options so machine-wide configuration can be applied without shared config files or shared auth, and updates state config to surface the effective value/source.

Changes:

Implement canonical ACTIVESTATE_CONFIG_<KEY> env var derivation plus legacy env-var aliases for select existing options.
Apply env var precedence (env > stored > default) centrally in config.Instance.Get.
Enhance state config output (table + JSON) to show effective value plus source/env var metadata; add unit tests for env behavior and canonical/alias rules.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
pkg/platform/api/settings.go	Registers `api.host` with legacy env var alias support.
internal/updater/updater.go	Registers update endpoint option with legacy env var alias support.
internal/updater/checker.go	Registers update info endpoint option with legacy env var alias support.
internal/runners/config/get.go	Clarifies that `cfg.Get` returns the effective (env-aware) value.
internal/runners/config/env_test.go	Adds tests verifying env override precedence for list/get.
internal/runners/config/config.go	Adds “Source” column + JSON metadata, and resolves effective values for display.
internal/mediators/config/registry.go	Introduces canonical env var derivation + env override resolution/type coercion.
internal/mediators/config/registry_test.go	Adds tests for canonical naming, alias precedence, and coercion.
internal/locale/locales/en-us.yaml	Adds “Source” header translation for the updated config table.
internal/config/instance.go	Applies env overrides in `Instance.Get` with correct precedence and scope (registered-only).
internal/analytics/analytics.go	Registers analytics pixel override option with legacy env var alias support.
cmd/state/main.go	Registers notifications URL config with legacy env var alias support (CLI side).
cmd/state-svc/internal/notifications/notifications.go	Registers notifications URL config with legacy env var alias support (daemon side).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

icanhasmath · 2026-07-15T22:59:48Z

+// EnvOverride returns the effective override value for the option when one of its environment
+// variables is currently set to a non-empty value, coerced to the option's type. The second return
+// value is the name of the variable in effect; the bool reports whether an override applies.
+func EnvOverride(opt Option) (interface{}, string, bool) {
+	for _, name := range EnvVarNames(opt) {
+		if v, ok := os.LookupEnv(name); ok && v != "" {
+			return coerceToType(opt.Type, v), name, true
+		}
+	}
+	return nil, "", false
+}
+
+// coerceToType converts a raw environment-variable string to the option's configured type so that
+// callers receive the same Go type they would get from a stored value.
+func coerceToType(t Type, raw string) interface{} {
+	switch t {
+	case Bool:
+		return cast.ToBool(raw)
+	case Int:
+		return cast.ToInt(raw)
+	default: // String, Enum
+		return raw
+	}
 }


Good catch — fixed in 19678dc. EnvOverride now strictly coerces the value to the option's type via cast.ToBoolE/ToIntE and validates enum values against the allowed set. Anything that doesn't parse (e.g. ACTIVESTATE_CONFIG_AUTOUPDATE=notabool or an out-of-set enum) is ignored, so it falls back to the stored/default value instead of silently becoming false/0. This mirrors the validation state config set already does. Added tests in registry_test.go (TestEnvOverrideInvalidIgnored) and verified end-to-end: an invalid bool leaves autoupdate at its default, an invalid enum leaves security.prompt.level at critical.

Note: I kept the fallback silent rather than logging, because importing multilog/logging into the mediators/config package would create an import cycle (rollbar/logging register config options). The Source column showing local/default instead of the env var name signals that the override didn't take effect.

icanhasmath · 2026-07-15T22:59:49Z

+source:
+  other: Source


Fixed in 19678dc — added config_source_local and config_source_default to en-us.yaml so the Source column no longer relies on the Tl fallback.

- EnvOverride now strictly coerces env values to the option's type and ignores invalid ones (unparseable bool/int, out-of-set enum), so a mis-typed variable falls back to the stored/default value instead of silently becoming a zero value. Mirrors the validation `state config set` already performs. - Add the config_source_local and config_source_default locale strings used by the Source column instead of relying on the fallback path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

icanhasmath · 2026-07-15T22:59:59Z

Thanks for the review. Addressed both Copilot findings in 19678dc:

Invalid env values — EnvOverride now strictly validates/coerces each variable to the option's type (cast.ToBoolE/ToIntE, and enum membership check). Unparseable bools/ints and out-of-set enums are ignored and fall back to the stored/default value instead of silently becoming false/0. Kept the fallback silent to avoid an import cycle (logging packages register config options); the Source column showing local/default rather than the var name makes a non-applied override visible. Added TestEnvOverrideInvalidIgnored.
Missing locale IDs — added config_source_local and config_source_default to en-us.yaml.

All config tests pass and I re-verified end-to-end (invalid bool leaves autoupdate at default; invalid enum leaves security.prompt.level at critical; valid values still apply).

mitchell-as · 2026-07-16T13:52:30Z

There's a regression in state config: (https://github.com/ActiveState/cli/actions/runs/29457176664/job/87492845470?pr=3843#step:27:920)

TestConfigIntegrationTestSuite/TestList (300ms)
      session.go:339: 
          'Expected: "true*": consumer stopped, reason: pty closed':
              'consumer stopped, reason: pty closed': pty closed
          Stack:
          ./internal/testhelpers/e2e/session.go:513:e2e.(*Session).DebugMessage
          ./internal/testhelpers/e2e/session.go:339:e2e.(*Session).newSpawnOpts.func1
          ./vendor/github.com/ActiveState/termtest/expect.go:77:termtest.(*TermTest).ExpectErrorHandler
          ./vendor/github.com/ActiveState/termtest/expect.go:95:termtest.(*TermTest).ExpectCustom
          ./vendor/github.com/ActiveState/termtest/expect.go:104:termtest.(*TermTest).Expect
          ./test/integration/config_int_test.go:113:integration.(*ConfigIntegrationTestSuite).TestList

Snapshot for Cmd '/tmp/530567214/bin/state config set optin.buildscripts true':
          █ Set config value                                                                                                                          
                                                                                                                                                      
        
Snapshot for Cmd '/tmp/530567214/bin/state config':
          █ Listing Configuration Keys and Values                                                                                                     
                                                                                                                                                      
            Key                                    Value         Source     Default                                                                   
            api.host                               ""            default    ""                                                                        
            autostart.svc                          true          default    true                                                                      
            autoupdate                             true          default    true                                                                      
            notifications.endpoint                 ""            default    ""                                                                        
            optin.buildscripts                     true          local      false

The test is expecting that last line to have true* due to the previous state config set optin.buildscripts true command.

icanhasmath · 2026-07-16T14:11:53Z

The test is expecting that last line to have true* due to the previous state config set optin.buildscripts true command.

Diagnosis
Your PR intentionally replaced the old * marker (which appended * to any value differing from default) with a proper Source column — renderConfigValue no longer appends *, and renderConfigSource renders environment (bold env-var name) / local / default.

TestList is stale: after state config set optin.buildscripts true, the row is now optin.buildscripts true local false — value true, Source local — instead of true*. The test still asserts cp.Expect("true*"), which can never match now.

@mitchell-as do you agree with this - and therefor the fix is to update the test?

mitchell-as · 2026-07-16T14:14:22Z

Yes, I would update the text to expect "local" instead of "true*"

The `*` marker that denoted a locally-set value was replaced by the Source column, so `state config` now shows "optin.buildscripts true local" instead of "true*". Update TestList to assert the "local" source (and the new "Source" header) instead of the removed "true*" marker. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

mitchell-as · 2026-07-16T14:34:35Z

@icanhasmath Furthermore, I would add a new integration test that includes a test with environment as the source. This will test your new improvement. You can model it after the test you just fixed.

TestListEnvSource overrides optin.buildscripts via its canonical env var (ACTIVESTATE_CONFIG_OPTIN_BUILDSCRIPTS) without running `state config set`, then asserts the override takes effect and is reported as coming from the environment. The human-readable table confirms the non-default value; the source is asserted via JSON output ("source":"environment" plus the overriding env var name), which is robust against the table wrapping long variable names. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

mitchell-as

This is looking pretty good. There are some changes I'd like to see though. See below.

mitchell-as · 2026-07-16T15:51:24Z

+
+// RegisterOptionWithEnv registers a config option along with one or more legacy/bespoke environment
+// variables that override it, in addition to its canonical ACTIVESTATE_CONFIG_* variable.
+func RegisterOptionWithEnv(key string, t Type, defaultValue interface{}, envAliases ...string) {


Since envAliases is optional, I think we can fold this into the existing RegisterOption(). No need for a separate function.

Make sure we keep the comment that envAliases is only for legacy env vars that do not have the "ACTIVESTATE_CONFIG_*" prefix.

Done in 5fcf95d — folded into RegisterOption via a variadic envAliases ...string and removed RegisterOptionWithEnv. Existing no-alias callers are unchanged. Kept the note that envAliases are only for legacy env vars that don't use the ACTIVESTATE_CONFIG_* prefix (on both RegisterOption and the EnvAliases field).

mitchell-as · 2026-07-16T15:54:05Z

+// registered config option can be overridden this way, e.g. "update.info.endpoint" maps to
+// "ACTIVESTATE_CONFIG_UPDATE_INFO_ENDPOINT".


Can we replace this comment with a config option we'd actually override with an env var? This isn't one of them. Maybe use one of the config option that triggered this PR in the first place.

Good point — changed the example to api.host -> ACTIVESTATE_CONFIG_API_HOST in 5fcf95d.

mitchell-as · 2026-07-16T15:55:55Z

+	// An environment variable override takes precedence over any stored or default value, so that
+	// config can be applied machine-wide (all users) via the environment. Only registered options
+	// can be overridden this way, which keeps unregistered keys (e.g. credentials) out of scope.


Let's just keep this at "An environment variable override takes precedence over any stored or default value."

State Tool is designed for individual use, not a single, machine-wide instance. We don't want to hint the latter in the comments.

Done in 5fcf95d — trimmed to just "An environment variable override takes precedence over any stored or default value." and dropped the machine-wide framing. (The registered-only scoping that keeps credentials out is still enforced by the mediator.KnownOption(opt) guard right below.)

mitchell-as · 2026-07-16T15:59:21Z

+func TestCanonicalEnvVarName(t *testing.T) {
+	assert.Equal(t, "ACTIVESTATE_CONFIG_API_HOST", CanonicalEnvVarName("api.host"))
+	assert.Equal(t, "ACTIVESTATE_CONFIG_AUTOUPDATE", CanonicalEnvVarName("autoupdate"))
+	assert.Equal(t, "ACTIVESTATE_CONFIG_UPDATE_INFO_ENDPOINT", CanonicalEnvVarName("update.info.endpoint"))


nit: we wouldn't actually override this one. Can we pick a different one that has double dots? How about "security.prompt.level"?

Done in 5fcf95d — replaced with security.prompt.level -> ACTIVESTATE_CONFIG_SECURITY_PROMPT_LEVEL (real user-facing option, double dots).

mitchell-as · 2026-07-16T16:01:44Z

+
+	// cfg.Get already applies any environment-variable override for the key, so the value reported
+	// here is the one the State Tool will actually use.


We don't need this comment.

Removed in 5fcf95d.

- Fold RegisterOptionWithEnv into RegisterOption via a variadic envAliases parameter; drop the separate function. Keep the note that envAliases are only for legacy env vars without the ACTIVESTATE_CONFIG_* prefix. - Use real, user-facing config options in doc/test examples (api.host, security.prompt.level) instead of the test-only update endpoints. - Trim the Instance.Get comment to just note env precedence; drop the machine-wide framing. - Remove the unnecessary comment in config get. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

icanhasmath · 2026-07-16T16:18:23Z

@mitchell-as thanks for the review — all five addressed in 5fcf95d:

Folded RegisterOptionWithEnv into a variadic RegisterOption(key, type, default, envAliases ...string) and removed the separate function; kept the legacy-alias note.
Doc example → api.host -> ACTIVESTATE_CONFIG_API_HOST.
Trimmed the Instance.Get comment to just the env-precedence sentence (no machine-wide framing).
Test example → security.prompt.level instead of the test-only update endpoint.
Removed the unnecessary comment in config get.

Build and all config unit tests pass. Ready for another look.

* Add environment-variable overrides for all config options Every registered config option can now be overridden by an environment variable, so config can be applied machine-wide (all users) without a shared config file and without sharing auth. Each option gets a canonical override derived from its key, e.g. "update.info.endpoint" -> ACTIVESTATE_CONFIG_UPDATE_INFO_ENDPOINT. The five options that already had bespoke env vars (ACTIVESTATE_API_HOST, ACTIVESTATE_CLI_ANALYTICS_PIXEL, ACTIVESTATE_NOTIFICATIONS_OVERRIDE, ACTIVESTATE_TEST_UPDATE_URL, ACTIVESTATE_TEST_UPDATE_INFO_URL) keep working as aliases; the canonical variable takes precedence when both are set. Overrides are resolved in config.Instance.Get (env > stored > default) so they actually take effect everywhere, not just in display. Values are coerced to the option's type. Only registered options are eligible, which keeps credentials (e.g. apiToken, not a registered option) out of scope. `state config` now reports the effective value and a Source column showing the overriding env var name, "local", or "default"; JSON output gains source, env, and envVar fields. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address PR review: validate env override values; add locale strings - EnvOverride now strictly coerces env values to the option's type and ignores invalid ones (unparseable bool/int, out-of-set enum), so a mis-typed variable falls back to the stored/default value instead of silently becoming a zero value. Mirrors the validation `state config set` already performs. - Add the config_source_local and config_source_default locale strings used by the Source column instead of relying on the fallback path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix TestList integration test for new Source column The `*` marker that denoted a locally-set value was replaced by the Source column, so `state config` now shows "optin.buildscripts true local" instead of "true*". Update TestList to assert the "local" source (and the new "Source" header) instead of the removed "true*" marker. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add integration test for environment as config source TestListEnvSource overrides optin.buildscripts via its canonical env var (ACTIVESTATE_CONFIG_OPTIN_BUILDSCRIPTS) without running `state config set`, then asserts the override takes effect and is reported as coming from the environment. The human-readable table confirms the non-default value; the source is asserted via JSON output ("source":"environment" plus the overriding env var name), which is robust against the table wrapping long variable names. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address review feedback from mitchell-as - Fold RegisterOptionWithEnv into RegisterOption via a variadic envAliases parameter; drop the separate function. Keep the note that envAliases are only for legacy env vars without the ACTIVESTATE_CONFIG_* prefix. - Use real, user-facing config options in doc/test examples (api.host, security.prompt.level) instead of the test-only update endpoints. - Trim the Instance.Get comment to just note env precedence; drop the machine-wide framing. - Remove the unnecessary comment in config get. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

icanhasmath mentioned this pull request Jul 15, 2026

Add machine-wide (all users) config layer without shared auth #3842

Closed

mitchell-as added Test: critical Test: all and removed Test: critical labels Jul 15, 2026

mitchell-as closed this Jul 15, 2026

mitchell-as reopened this Jul 15, 2026

icanhasmath changed the title ~~Environment-variable overrides for all config options~~ ENG-2005-Environment-variable overrides for all config options Jul 15, 2026

icanhasmath requested a review from Copilot July 15, 2026 22:39

Copilot started reviewing on behalf of icanhasmath July 15, 2026 22:40 View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

mitchell-as added Test: config and removed Test: all labels Jul 16, 2026

mitchell-as requested changes Jul 16, 2026

View reviewed changes

mitchell-as approved these changes Jul 16, 2026

View reviewed changes

icanhasmath merged commit f0ebd00 into master Jul 16, 2026
9 of 12 checks passed

icanhasmath deleted the marcg/env-config-display branch July 16, 2026 17:01

		// registered config option can be overridden this way, e.g. "update.info.endpoint" maps to
		// "ACTIVESTATE_CONFIG_UPDATE_INFO_ENDPOINT".


		// cfg.Get already applies any environment-variable override for the key, so the value reported
		// here is the one the State Tool will actually use.

Uh oh!

Conversation

icanhasmath commented Jul 15, 2026

Summary

Canonical + legacy env vars

Behavior

state config visibility

Operational note

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

icanhasmath commented Jul 15, 2026

Uh oh!

mitchell-as commented Jul 16, 2026

Uh oh!

icanhasmath commented Jul 16, 2026

Uh oh!

mitchell-as commented Jul 16, 2026

Uh oh!

mitchell-as commented Jul 16, 2026

Uh oh!

mitchell-as left a comment

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!

icanhasmath commented Jul 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

`state config` visibility