Skip to content

Add warn for helper func #38

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.

Sign up for GitHub

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

Merged
BretRen merged 1 commit into from
Apr 20, 2026
Merged

Add warn for helper func #38

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"log/slog"
"os"
"strconv"
"strings"
Expand All @@ -14,22 +15,32 @@ import (
func setStringIfPresent(key string, dst *string) {
if v, ok := os.LookupEnv(key); ok {
*dst = v
} else {
slog.Warn("missing_env", "env", key)
}
Comment on lines +18 to 20
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The setStringIfPresent function is designed for optional environment variables. Logging a warning when the variable is missing contradicts the "IfPresent" semantics and will result in excessive log noise for optional configuration. If a variable is mandatory, it should be handled by a separate validation step or a different helper function.

	}

}

func setBoolIfPresent(key string, dst *bool) {
if v, ok := os.LookupEnv(key); ok {
if b, err := strconv.ParseBool(v); err == nil {
*dst = b
} else {
slog.Warn("invalid_bool", "env", key, "value", v)
}
} else {
slog.Warn("missing_env", "env", key)
}
Comment on lines +28 to 32
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

This change introduces two issues:

  1. Security: Logging the raw value v from an environment variable can leak sensitive information if a secret was accidentally assigned to the wrong key. It is safer to log the error returned by the parser.
  2. Log Noise: Warning when an optional environment variable is missing is misleading for "IfPresent" helpers.

Consider removing the missing environment warning and logging the error instead of the raw value.

			slog.Warn("invalid_bool", "env", key, "err", err)
		}
	}

}

func setIntIfPresent(key string, dst *int) {
if v, ok := os.LookupEnv(key); ok {
if i, err := strconv.Atoi(v); err == nil {
*dst = i
} else {
slog.Warn("invalid_int", "env", key, "value", v)
}
} else {
slog.Warn("missing_env", "env", key)
}
Comment on lines +40 to 44
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

Similar to setBoolIfPresent, logging the raw value v poses a security risk of leaking secrets into logs. Additionally, warning on missing optional variables creates unnecessary log noise. It is recommended to log the parsing error and remove the warning for missing variables.

			slog.Warn("invalid_int", "env", key, "err", err)
		}
	}

}

Expand Down