-
Notifications
You must be signed in to change notification settings - Fork 1.5k
add env var support for config files #1650
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds support for two environment variables read at init to override the default config file locations and documents them in the README: SUBFINDER_CONFIG (config.yaml) and SUBFINDER_PROVIDER_CONFIG (provider-config.yaml). Also updates tests to ignore one additional passive source. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Env as Environment
participant CLI as subfinder CLI
participant Runner as ParseOptions
User->>CLI: Run command
CLI->>Runner: ParseOptions()
Runner->>Env: Get SUBFINDER_CONFIG
Env-->>Runner: Value or empty
Runner->>Env: Get SUBFINDER_PROVIDER_CONFIG
Env-->>Runner: Value or empty
alt Env vars present
Note over Runner: Override default config paths
else Not present
Note over Runner: Use built-in default locations
end
Runner-->>CLI: Options with resolved config paths
CLI-->>User: Proceed with execution
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
README.md (1)
112-118: Clarify precedence and include a quick usage example.Explicitly stating precedence reduces ambiguity and helps Docker/CI users copy-paste.
## Environment Variables Subfinder supports environment variables to specify custom paths for configuration files: - `SUBFINDER_CONFIG` - Path to config.yaml file (overrides default `$CONFIG/subfinder/config.yaml`) - `SUBFINDER_PROVIDER_CONFIG` - Path to provider-config.yaml file (overrides default `$CONFIG/subfinder/provider-config.yaml`) + +Precedence: CLI flags override environment variables, which override defaults. + +Example: + +```sh +export SUBFINDER_CONFIG=/mnt/config/config.yaml +export SUBFINDER_PROVIDER_CONFIG=/mnt/config/provider-config.yaml +subfinder -d example.com +```pkg/runner/options.go (1)
121-128: Normalize env-provided paths to avoid whitespace/relative-path gotchas.Trim and clean the paths; use LookupEnv to distinguish “unset” vs “set to empty”.
- if envConfig := os.Getenv("SUBFINDER_CONFIG"); envConfig != "" { - defaultConfigLocation = envConfig - } + if v, ok := os.LookupEnv("SUBFINDER_CONFIG"); ok { + v = filepath.Clean(strings.TrimSpace(v)) + if v != "" { + defaultConfigLocation = v + } + } - if envProviderConfig := os.Getenv("SUBFINDER_PROVIDER_CONFIG"); envProviderConfig != "" { - defaultProviderConfigLocation = envProviderConfig - } + if v, ok := os.LookupEnv("SUBFINDER_PROVIDER_CONFIG"); ok { + v = filepath.Clean(strings.TrimSpace(v)) + if v != "" { + defaultProviderConfigLocation = v + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)pkg/runner/options.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test Builds (windows-latest)
- GitHub Check: Test Builds (ubuntu-latest)
- GitHub Check: Test Builds (macOS-13)
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
pkg/runner/options.go (2)
121-128: LGTM — env defaults applied before flag registration; CLI flags still win.
Repo search shows SUBFINDER_CONFIG / SUBFINDER_PROVIDER_CONFIG and the default* variables only in pkg/runner/options.go (definition/env-read/flag registration), pkg/runner/runner.go (usage) and README — no other resets found.
121-128: Confirm intended behavior for config.yaml loading with env defaults. Currently SUBFINDER_CONFIG only changes defaultConfigLocation (pkg/runner/options.go:121-128) and the flag default is set from that value (pkg/runner/options.go:130); MergeConfigFile is only called when options.Config != defaultConfigLocation (pkg/runner/options.go:166-172). With SUBFINDER_CONFIG set but no -config flag, MergeConfigFile will not run — change the condition if you want the env var to trigger loading.
|
we can ignore test fails |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/runner/options.go (1)
154-158: Create provider-config at the user-selected path (not always the default)Post-parse, honor -pc/ProviderConfig (which may come from env) when creating the file. Using the default path here ignores user choice.
Apply:
- if exists := fileutil.FileExists(defaultProviderConfigLocation); !exists { - if err := createProviderConfigYAML(defaultProviderConfigLocation); err != nil { + if exists := fileutil.FileExists(options.ProviderConfig); !exists { + if err := createProviderConfigYAML(options.ProviderConfig); err != nil { gologger.Error().Msgf("Could not create provider config file: %s\n", err) } }
🧹 Nitpick comments (2)
pkg/runner/options.go (2)
160-165: Optionally auto-load config.yaml when SUBFINDER_CONFIG is setCurrent logic only merges when --config differs from default. If users set SUBFINDER_CONFIG (without --config), they may expect it to load. Consider merging when the env var is present.
Apply:
- if options.Config != defaultConfigLocation { + if options.Config != defaultConfigLocation || os.Getenv("SUBFINDER_CONFIG") != "" { // An empty source file is not a fatal error if err := flagSet.MergeConfigFile(options.Config); err != nil && !errors.Is(err, io.EOF) { gologger.Fatal().Msgf("Could not read config: %s\n", err) } }If you prefer to keep current semantics, please explicitly document that SUBFINDER_CONFIG only sets the default path and does not trigger auto-loading.
123-125: Nice-to-have: surface env vars in flag helpMakes discoverability better from
-halone.- flagSet.StringVar(&options.Config, "config", defaultConfigLocation, "flag config file"), - flagSet.StringVarP(&options.ProviderConfig, "provider-config", "pc", defaultProviderConfigLocation, "provider config file"), + flagSet.StringVar(&options.Config, "config", defaultConfigLocation, "flag config file (env: SUBFINDER_CONFIG)"), + flagSet.StringVarP(&options.ProviderConfig, "provider-config", "pc", defaultProviderConfigLocation, "provider config file (env: SUBFINDER_PROVIDER_CONFIG)"),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/runner/options.go(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test Builds (windows-latest)
- GitHub Check: Test Builds (ubuntu-latest)
- GitHub Check: Test Builds (macOS-13)
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
pkg/runner/options.go (2)
18-18: Import of env util looks goodAlias and package path are consistent with ProjectDiscovery utils.
27-28: Approve env var documentation
README.md includesSUBFINDER_CONFIGandSUBFINDER_PROVIDER_CONFIGunder the configuration section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/passive/sources_wo_auth_test.go (2)
47-52: Reduce bursty rate-limits to avoid 3rd‑party bans/timeoutsUsing math.MaxInt32 with a 1ms window is hostile to remote services and likely contributes to flaky failures (incl. unmarshal errors on stressed endpoints).
Consider a gentler default for tests:
- multiRateLimiter, _ = addRateLimiter(ctxParent, multiRateLimiter, source.Name(), math.MaxInt32, time.Millisecond) + // Reduce burst to lower risk of bans/timeouts in CI + multiRateLimiter, _ = addRateLimiter(ctxParent, multiRateLimiter, source.Name(), 10, 200*time.Millisecond)If the project has per-source recommended limits, prefer wiring those instead.
Please confirm this still yields at least one result per source consistently in CI.
26-37: Gate “threatcrowd” ignore to CI-onlyApply in pkg/passive/sources_wo_auth_test.go:
- "threatcrowd", // failing with "randomly failing with unmarshal error when hit multiple times" -} +} +// CI-only quarantine for flaky source; keep local coverage +if os.Getenv("CI") != "" { + ignoredSources = append(ignoredSources, "threatcrowd") // intermittently fails with unmarshal errors when invoked multiple times +}Also add to imports:
import ( // … "os" )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.github/workflows/build-test.ymlis excluded by!**/*.yml
📒 Files selected for processing (1)
pkg/passive/sources_wo_auth_test.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test Builds (macOS-latest)
- GitHub Check: Test Builds (windows-latest)
- GitHub Check: Test Builds (ubuntu-latest)
- GitHub Check: Analyze (go)
closes #1648
Summary by CodeRabbit
New Features
Documentation
Tests