fix: restrict config discovery to home directory only#142
Merged
Conversation
4 tasks
JoshMock
approved these changes
Apr 14, 2026
Member
JoshMock
left a comment
JoshMock
left a comment
There was a problem hiding this comment.
LGTM, but waiting to merge in case dropping cosmiconfig is something worth doing first.
| */ | ||
|
|
||
| import { access, constants } from 'node:fs/promises' | ||
| import { homedir } from 'node:os' |
Member
There was a problem hiding this comment.
If we're using homedir from the stdlib and only looking strictly at 4 possible file paths, it effectively removes the need for cosmiconfig at all. Do you see value in keeping it for anything else here?
Contributor
Author
There was a problem hiding this comment.
Probably not, I toyed with the idea of having an env var like ELASTIC_CLI_ADDITIONAL_CONFIG_PATHS as a csv of "trusted directories" where we could allow cosmiconfig to discover from if they were in the parent path. But honestly, I think you're probably right to suggest removing it
Previously, cosmiconfig walked from cwd upward through every parent directory toward $HOME, loading the first config file found. A malicious .elasticrc.yml in a shared directory (e.g. /tmp, a cloned repo) could hijack the CLI to redirect credentials to an attacker-controlled server. Config is now discovered only in $HOME. Explicit overrides via --config-file flag or ELASTIC_CLI_CONFIG_FILE env var are supported. Precedence: --config-file > ELASTIC_CLI_CONFIG_FILE > ~/.elasticrc.yml Ref: #128
Replace cosmiconfig with direct file reading (readFile + yaml.parse / JSON.parse). Since config discovery is now restricted to the home directory and a handful of known file names, cosmiconfig adds no value. This removes 16 transitive packages from the dependency tree.
MattDevy
force-pushed
the
fix/tighten-config-discovery
branch
from
dde6a9a to
89a5f05
Compare
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
2 participants
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
Proposal to tighten config file discovery, based on the security concerns raised in #128 (comment).
Previously, cosmiconfig walked from
cwdupward through every parent directory toward$HOME, loading the first.elasticrc.ymlfound. A malicious config placed in a shared directory (e.g./tmp, a cloned repo, a mounted volume) could hijack the CLI to redirect credentials to an attacker-controlled server or exfiltrate secrets via auth headers.Changes:
~/.elasticrc.ymland variants)--config-file <path>flag continues to work for explicit overridesELASTIC_CLI_CONFIG_FILEenv var as an alternative to the flag--config-file>ELASTIC_CLI_CONFIG_FILE> home-directory discoverysearchStrategy: 'global'and directory-walkingsearch()from cosmiconfig usagepackage.jsonas a config source (only relevant with directory walking)Test plan
npm testpasses (639 tests, 0 failures)$HOMEis discovered when no flag/env var is set--config-file /path/to/config.ymlloads the explicit fileELASTIC_CLI_CONFIG_FILE=/path/to/config.ymlloads the explicit file--config-filetakes precedence overELASTIC_CLI_CONFIG_FILE