Add machine-wide (all users) config layer without shared auth#3842
Closed
icanhasmath wants to merge 1 commit into
Closed
Add machine-wide (all users) config layer without shared auth#3842icanhasmath wants to merge 1 commit into
icanhasmath wants to merge 1 commit into
Conversation
Introduce a read-only, machine-wide config file that applies to every user on a machine, while keeping authentication strictly per-user. The file is optional YAML at a system location, not rooted in any user's home dir: - Linux: /etc/activestate/cli-<channel>/config.yaml - macOS: /Library/Application Support/activestate/cli-<channel>/config.yaml - Windows: %PROGRAMDATA%\activestate\cli-<channel>\config.yaml The location can be overridden with ACTIVESTATE_CLI_SYSTEM_CONFIGDIR. Precedence for Get: user value > machine-wide value > built-in default, so machine-wide values act as defaults that users can still override. The machine-wide layer only serves *registered* config options. Because the auth token (apiToken) is not a registered option, credentials can never be read from the shared file, so auth stays per-user by design. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds an optional, read-only machine-wide configuration layer (system config.yaml) that can provide defaults for all users on a machine while ensuring auth remains per-user.
Changes:
- Introduces OS-specific “system app data” base paths and a
SystemAppDataPath()helper for locating machine-wide config. - Loads an optional machine-wide YAML config file at startup and applies it as a default layer in
Instance.Get(user value > system value > built-in default), only for registered options. - Adds unit tests validating precedence behavior, missing-file fallback, and that unregistered keys like
apiTokenare not returned.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/installation/storage/storage.go | Adds SystemAppDataPath() with env override support. |
| internal/installation/storage/storage_xdg.go | Adds Linux /etc system base path for machine-wide config. |
| internal/installation/storage/storage_darwin.go | Adds macOS system base path (/Library/Application Support). |
| internal/installation/storage/storage_windows.go | Adds Windows system base path (%ProgramData%). |
| internal/constants/constants.go | Adds env var + filename constants for machine-wide config. |
| internal/config/instance.go | Loads machine-wide YAML config at startup and consults it in Get for registered options. |
| internal/config/instance_test.go | Adds tests for machine-wide defaults, override precedence, missing file, and auth non-leakage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+87
to
+90
| // loadSystemConfig reads the optional machine-wide (all users) config file into memory. The file | ||
| // is a plain YAML map of registered config keys to values. It is entirely optional: a missing | ||
| // file is not an error. Values here act as defaults for users who have not set the key themselves, | ||
| // and are only ever surfaced for registered config options, so credentials are never read here. |
Comment on lines
+102
to
+110
| parsed := map[string]interface{}{} | ||
| if err := yaml.Unmarshal(data, &parsed); err != nil { | ||
| multilog.Error("config: could not parse system config at %s: %s", path, errs.JoinMessage(err)) | ||
| return | ||
| } | ||
|
|
||
| i.systemConfig = parsed | ||
| logging.Debug("Loaded machine-wide config from %s (%d keys)", path, len(parsed)) | ||
| } |
Comment on lines
+72
to
+76
| // SystemAppDataPath returns the machine-wide (all users) appdata dir. Unlike AppDataPath this | ||
| // is not rooted in a user's home directory, so a single file placed here applies to every user | ||
| // on the machine. It is used exclusively for read-only config defaults; credentials are never | ||
| // stored or read here. The location can be overridden with the SystemConfigDirEnvVarName env var. | ||
| func SystemAppDataPath() string { |
Contributor
Author
|
Superseded by #3843, which takes a different approach: instead of a machine-wide config file, all config options can be set via environment variables (canonical ACTIVESTATE_CONFIG_* names, with the existing bespoke vars kept as aliases). Auth stays out of scope since credentials are not registered config options. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a read-only, machine-wide config layer that applies to every user on a machine, while keeping authentication strictly per-user.
Motivation: administrators want to install config settings that apply to all users on a machine, without sharing auth credentials across those users.
How it works
An optional YAML file at a system location (not rooted in any user's home dir):
/etc/activestate/cli-<channel>/config.yaml/Library/Application Support/activestate/cli-<channel>/config.yaml%PROGRAMDATA%\activestate\cli-<channel>\config.yamlThe location can be overridden with
ACTIVESTATE_CLI_SYSTEM_CONFIGDIR(mainly for testing).Example
config.yaml:Precedence (
Instance.Get): user value > machine-wide value > built-in default. Machine-wide values act as defaults a user can still override locally.Auth is never shared — by design
The machine-wide layer only serves registered config options. Because the auth token (
apiToken) is not a registered option, credentials can never be read from the shared file. This is structural, not a filter — even anapiToken:entry in the shared file is ignored.Notes / decisions
IsSet(key)still reflects only the user's own store; the machine-wide layer behaves like a default, not an explicit set.Testing
internal/config/instance_test.go).🤖 Generated with Claude Code