feat: add validation to vault address

[nmgaston]

This pull request strengthens the validation logic for the SECRETS_ADDR configuration, ensuring improved security and correctness when specifying the Vault server address. It enforces that non-localhost addresses use HTTPS, adds granular error handling, and introduces comprehensive tests for these scenarios.

Security and Validation Improvements

• Added strict validation for the SECRETS_ADDR config: non-localhost addresses must use HTTPS, and various error cases (missing scheme, invalid URL, missing host) are now handled with specific error messages. (config/config.go)
• Introduced helper functions isLocalhost and stripPort to reliably detect localhost addresses and handle IPv4/IPv6/hostname formats. (config/config.go)

Error Handling

• Defined custom error variables for different misconfiguration scenarios (empty address, insecure address, invalid/missing scheme, missing host). (config/config.go)

Testing

• Added extensive unit tests covering all validation logic, including localhost/non-localhost detection, HTTPS enforcement, error cases, and edge cases for address parsing. (config/config_test.go)
• Improved test isolation by clearing the SECRETS_ADDR environment variable before and after tests. (config/config_test.go)
• Added a test for handling invalid environment variable values in NewConfig. (config/config_test.go)

These changes make it much harder to accidentally misconfigure the Vault address in an insecure way and ensure that configuration errors are caught early with clear error messages.

[codecov]

Codecov Report

❌ Patch coverage is 92.50000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.87%. Comparing base (894b30e) to head (52b42be).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
config/config.go	92.50%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1186      +/-   ##
==========================================
+ Coverage   49.72%   49.87%   +0.14%     
==========================================
  Files         146      146              
  Lines       13455    13495      +40     
==========================================
+ Hits         6691     6730      +39     
- Misses       6184     6185       +1     
  Partials      580      580              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

This PR adds stricter validation for the SECRETS_ADDR (Vault address) configuration, aiming to prevent insecure (non-HTTPS) remote Vault endpoints and to provide clearer error reporting, along with expanded unit test coverage for these cases.

Changes:

• Added Config.Validate() and Vault address validation that enforces HTTPS for non-localhost endpoints.
• Introduced dedicated error values for common Vault address misconfiguration scenarios.
• Added unit tests covering Vault address parsing/validation behavior and a NewConfig invalid-env-var failure case.

Reviewed changes

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

File	Description
config/config.go	Adds Vault address validation (including localhost detection) and new config validation errors.
config/config_test.go	Adds tests for the new Vault address validation behavior and NewConfig env-var error handling.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[sudhir-intc]

@nmgaston : Could you please add some evidence with your testing to this PR like when the vault address is misconfigure how does it get reported when console gets executed. Looking for how this error could be observed in the logs or the execution output for the following scenarios

ErrSecretsAddrEmpty = errors.New("SECRETS_ADDR is empty")
ErrSecretsAddrInsecure = errors.New("SECRETS_ADDR must use HTTPS for non-localhost addresses")
ErrSecretsAddrInvalid = errors.New("invalid SECRETS_ADDR")
ErrSecretsAddrMissingScheme = errors.New("SECRETS_ADDR missing scheme (use http:// or https://)")
ErrSecretsAddrNoHost = errors.New("SECRETS_ADDR contains no host")

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Suppressed comments (9)

config/config.go:385

• More //nolint:staticcheck annotations intended for QF1008 should target the linter that emits QF1008 (gocritic), otherwise they may be ineffective and/or flagged as unused by nolintlint.

	// Check for valid scheme (must be http or https)
	if parsed.Scheme != "http" && parsed.Scheme != "https" {
		if !strings.Contains(c.Secrets.Address, "://") { //nolint:staticcheck // QF1008: explicit field reference is clearer
			return fmt.Errorf("%w: %q", ErrSecretsAddrMissingScheme, c.Secrets.Address) //nolint:staticcheck // QF1008: explicit field reference is clearer
		}

		return fmt.Errorf("%w: unsupported scheme %q in %q", ErrSecretsAddrInvalid, parsed.Scheme, c.Secrets.Address) //nolint:staticcheck // QF1008: explicit field reference is clearer
	}

config/config_test.go:194

• These subtests call t.Parallel() and close over the range variable addr. Capture addr inside the loop (pattern used elsewhere in the repo, e.g. internal/controller/httpapi/v1/server_test.go:50-55) to satisfy paralleltest and avoid accidental variable capture issues.

	for _, addr := range testCases {
		t.Run(addr, func(t *testing.T) {
			t.Parallel()

config/config_test.go:231

• These parallel subtests close over addr from the surrounding range loop. Capture the variable inside the loop (addr := addr) to match the repo’s paralleltest expectations (see internal/controller/httpapi/v1/server_test.go:50-55).

	for _, addr := range testCases {
		t.Run(addr, func(t *testing.T) {
			t.Parallel()

config/config_test.go:256

• These parallel subtests close over the loop variable addr. Capture it inside the loop (addr := addr) to match the repo’s table-test pattern for t.Parallel() subtests.

	for _, addr := range testCases {
		t.Run(addr, func(t *testing.T) {
			t.Parallel()

config/config_test.go:364

• These parallel subtests close over addr from the range loop. Capture addr inside the loop (addr := addr) to match the repo’s parallel table-test pattern and avoid paralleltest violations.

	for _, addr := range testCases {
		t.Run(addr, func(t *testing.T) {
			t.Parallel()

config/config_test.go:415

• These parallel subtests close over tc from a range loop over a map. Capture tc inside the loop (tc := tc) before calling t.Run(..., func(t *testing.T) { t.Parallel(); ... }) to satisfy paralleltest expectations.

	for name, tc := range testCases {
		t.Run(name, func(t *testing.T) {
			t.Parallel()

config/config.go:376

• These //nolint:staticcheck annotations appear to be intended to suppress gocritic QF1008 (per the comment), but they currently target the wrong linter. This can fail CI if nolintlint is enabled (unused nolint) and it also won't suppress QF1008 if emitted by gocritic.

This issue also appears on line 378 of the same file.

func (c *Config) Validate() error {
	// Ensure non-localhost Vault addresses use HTTPS
	if c.Secrets.Address != "" { //nolint:staticcheck // QF1008: explicit field reference is clearer
		if err := c.validateSecretsAddr(); err != nil {
			return err

config/config_test.go:172

• These subtests call t.Parallel() and close over the range variable addr. This repo enforces capturing the loop variable for parallel subtests (see internal/controller/httpapi/v1/server_test.go:50-55 / CLAUDE.md), so add addr := addr inside the loop.

This issue also appears in the following locations of the same file:

• line 192
• line 229
• line 254
• line 362

	for _, addr := range testCases {
		t.Run(addr, func(t *testing.T) {
			t.Parallel()

config/config_test.go:305

• These parallel subtests close over the range variable tc. Capture it inside the loop (tc := tc) to satisfy the repo’s paralleltest expectations for table-driven tests with t.Parallel() subtests.

This issue also appears on line 413 of the same file.

	for _, tc := range testCases {
		t.Run(tc.host, func(t *testing.T) {
			t.Parallel()

[nmgaston]

@nmgaston : Could you please add some evidence with your testing to this PR like when the vault address is misconfigure how does it get reported when console gets executed. Looking for how this error could be observed in the logs or the execution output for the following scenarios

ErrSecretsAddrEmpty = errors.New("SECRETS_ADDR is empty") ErrSecretsAddrInsecure = errors.New("SECRETS_ADDR must use HTTPS for non-localhost addresses") ErrSecretsAddrInvalid = errors.New("invalid SECRETS_ADDR") ErrSecretsAddrMissingScheme = errors.New("SECRETS_ADDR missing scheme (use http:// or https://)") ErrSecretsAddrNoHost = errors.New("SECRETS_ADDR contains no host")

Runtime behavior when console starts with invalid SECRETS_ADDR
Command used:
env SECRETS_ADDR='vault.example.com:8200' go run ./cmd/app

Observed output:
2026/08/10 09:19:52 Config error: SECRETS_ADDR missing scheme (use http:// or https://): "vault.example.com:8200"
exit status 1

Command used:
env SECRETS_ADDR='http://vault.example.com:8200' go run ./cmd/app

Observed output:
2026/08/10 09:20:14 Config error: SECRETS_ADDR must use HTTPS for non-localhost addresses
exit status 1

Command used:
env SECRETS_ADDR='://invalid' go run ./cmd/app

Observed output:
2026/08/10 09:20:26 Config error: invalid SECRETS_ADDR: parse "://invalid": missing protocol scheme
exit status 1

Command used:
env SECRETS_ADDR='ftp://vault.example.com:8200' go run ./cmd/app

Observed output:
2026/08/10 09:19:58 Config error: invalid SECRETS_ADDR: unsupported scheme "ftp" in "ftp://vault.example.com:8200"
exit status 1

Command used:
env SECRETS_ADDR='https://:8200' go run ./cmd/app

Observed output:
2026/08/10 09:20:22 Config error: SECRETS_ADDR contains no host: "https://:8200"
exit status 1

[nmgaston]

ErrSecretsAddrEmpty = errors.New("SECRETS_ADDR is empty")

I removed this one, since it can be empty. It will skip the validation if it is empty.

[Copilot]

Pull request overview

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

Suppressed comments (1)

config/config.go:27

• ErrSecretsAddrMissingScheme currently suggests using either http:// or https://, but the validation only permits HTTP for localhost/loopback. Updating the error text will make the remediation clearer and avoid users switching to insecure HTTP on remote hosts and getting a second error.

	ErrSecretsAddrMissingScheme = errors.New("SECRETS_ADDR missing scheme (use http:// or https://)")