-
Notifications
You must be signed in to change notification settings - Fork 0
Add security filtering to environment variables endpoint #9
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -288,11 +288,32 @@ const server = Bun.serve({ | |
| } | ||
|
|
||
| if (url.pathname === "/api/environment") { | ||
| // Return environment variables for system diagnostics | ||
| const envVars = Object.entries(process.env).map(([key, value]) => ({ | ||
| name: key, | ||
| value: value || "", | ||
| })); | ||
| // Return filtered environment variables for system diagnostics | ||
| // Only expose safe, non-sensitive variables | ||
|
|
||
| const safeVariables = [ | ||
| "PATH", "HOME", "USER", "SHELL", "TERM", "LANG", "LC_ALL", | ||
| "EDITOR", "VISUAL", "PAGER", "TZ", "PWD", "OLDPWD", | ||
| "HOSTNAME", "LOGNAME", "XDG_CONFIG_HOME", "XDG_DATA_HOME", | ||
| "NODE_ENV", "RUST_BACKTRACE", "PYTHONDONTWRITEBYTECODE", | ||
| ]; | ||
|
|
||
| const sensitivePatterns = [ | ||
| "KEY", "SECRET", "TOKEN", "PASSWORD", "CREDENTIAL", | ||
| "AUTH", "PRIVATE", "API_KEY", "ACCESS_KEY", | ||
| ]; | ||
|
|
||
| const isSensitive = (name: string): boolean => { | ||
| return sensitivePatterns.some(pattern => name.includes(pattern)); | ||
| }; | ||
|
|
||
| const envVars = Object.entries(process.env) | ||
| .filter(([key]) => safeVariables.includes(key) || key.startsWith("LC_") || key.startsWith("XDG_")) | ||
| .map(([key, value]) => ({ | ||
| name: key, | ||
| value: isSensitive(key) ? "[REDACTED]" : (value || ""), | ||
| })); | ||
|
Comment on lines
+310
to
+315
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Get the actual code context around lines 310-315
head -320 server/index.ts | tail -30Repository: coderabbit-demo/btop Length of output: 1230 🏁 Script executed: # Check for the isSensitive function definition
rg -n "isSensitive" --type=ts --type=jsRepository: coderabbit-demo/btop Length of output: 224 🏁 Script executed: # Search for tests or documentation about environment variable filtering
rg -n -C3 "envVars|REDACTED|environment.*filter|safeVariables" --type=ts --type=jsRepository: coderabbit-demo/btop Length of output: 2813 🏁 Script executed: # Look for security guidelines or comments about this behavior
rg -n -C5 "LC_|XDG_|sensitive.*variable|expose.*variable" --type=ts --type=jsRepository: coderabbit-demo/btop Length of output: 1616 Filter out environment variables with sensitive names, not just redact their values. The code comment states "Only expose safe, non-sensitive variables," but the current implementation exposes variable names that match sensitive patterns (e.g., Either exclude sensitive variables entirely from the response or adjust the intent comment to clarify that only values are redacted. 🤖 Prompt for AI Agents |
||
|
|
||
| return new Response(JSON.stringify({ variables: envVars }), { | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
|
|
||
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.
Critical: Case-sensitive matching misses lowercase sensitive variables.
The
sensitivePatternsarray contains uppercase strings, but the matching inisSensitiveuses case-sensitiveincludes(). Variables likeapi_key,secret,password, orauth_tokenwould bypass detection and have their values exposed.🔎 Proposed fix to use case-insensitive matching
🤖 Prompt for AI Agents