Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 1 addition & 1 deletion completion/available/artisan.completion.bash
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ _artisan_completion() {
commands=$(command php artisan --raw --no-ansi list 2> /dev/null | command sed "s/[[:space:]].*//")

# shellcheck disable=SC2034,SC2207
COMPREPLY=($(compgen -W '${commands}' -- "${cur}"))
COMPREPLY=($(compgen -W "\"${commands}\"" -- "${cur}"))
return 0
}

Expand Down
227 changes: 227 additions & 0 deletions plugins/available/safe_autoenv.plugin.bash
Original file line number Diff line number Diff line change
@@ -0,0 +1,227 @@
# shellcheck shell=bash
# BASH_IT_LOAD_PRIORITY: 200

# Autoenv Plugin for Bash-it with automatic cleanup
# Automatically loads .env files when changing directories
# Automatically unsets variables when leaving directories

cite about-plugin
about-plugin 'Automatically loads .env files and cleans up when leaving directories'

# Global tracking for autoenv state
declare -A _AUTOENV_PROCESSED_DIRS # Track processed directories
declare -A _AUTOENV_DIR_VARS # Map directories to their variables
declare -A _AUTOENV_VAR_SOURCES # Map variables to their source directories
declare _AUTOENV_LAST_PWD="" # Track last working directory

_autoenv_init() {
typeset target home _file current_dir
Copy link
Contributor

Choose a reason for hiding this comment

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

typeset is used here, while local is used elsewhere. This should be normalized.

typeset -a _files
target=$1
home="${HOME%/*}"

# shellcheck disable=SC2207
_files=($(
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't work when the directory path contains spaces or glob characters. It is better to use while [[ ... ]]; do ... _files+=("${_file}") ... done than to echo the filename and split it using _files=($(...)).

current_dir="$target"
while [[ "$current_dir" != "/" && "$current_dir" != "$home" ]]; do
_file="$current_dir/.env"
if [[ -e "${_file}" ]]; then
echo "${_file}"
fi
# Move to parent directory
current_dir="$(dirname "$current_dir")"
done
))

# Process files in reverse order (from root to current directory)
local _file_count=${#_files[@]}
local i
for ((i = _file_count - 1; i >= 0; i--)); do
local env_file="${_files[i]}"
local env_dir
env_dir="$(dirname "$env_file")"

# Only process if this directory hasn't been processed yet
# OR if it was processed but its variables were cleaned up
if [[ -z "${_AUTOENV_PROCESSED_DIRS[$env_dir]}" ]] || [[ -z "${_AUTOENV_DIR_VARS[$env_dir]}" ]]; then
echo "Processing $env_file"
_autoenv_process_file "$env_file" "$env_dir"
_AUTOENV_PROCESSED_DIRS[$env_dir]=1
fi
done
}

_autoenv_process_file() {
local env_file="$1"
local env_dir="$2"
local line key value original_line line_number=0
local -a dir_vars=()

# Check if file is readable
if [[ ! -r "$env_file" ]]; then
echo "Warning: Cannot read $env_file" >&2
return 1
fi

# Read the file line by line
while IFS= read -r line || [[ -n "$line" ]]; do
((line_number++))
original_line="$line"

# Skip empty lines and comments
[[ -z "$line" || "$line" =~ ^[[:space:]]*# ]] && continue

# Remove leading whitespace
line="${line#"${line%%[![:space:]]*}"}"

# Check if line matches KEY=value pattern
if [[ "$line" =~ ^([A-Za-z_][A-Za-z0-9_]*)=(.*)$ ]]; then
key="${BASH_REMATCH[1]}"
value="${BASH_REMATCH[2]}"

# Validate key name (additional safety check)
if [[ ! "$key" =~ ^[A-Za-z_][A-Za-z0-9_]*$ ]]; then
echo "Warning: Invalid variable name '$key' at line $line_number in $env_file" >&2
continue
fi
Comment on lines +82 to +86
Copy link
Contributor

Choose a reason for hiding this comment

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

In what situation does this happen? I think key is supposed to contain a valid key name in this context.


# Handle quoted values (remove outer quotes if present)
if [[ "$value" =~ ^\"(.*)\"$ ]]; then
value="${BASH_REMATCH[1]}"
elif [[ "$value" =~ ^\'(.*)\'$ ]]; then
value="${BASH_REMATCH[1]}"
fi

# Export the variable silently
export "$key=$value"
Comment on lines +88 to +96
Copy link
Contributor

Choose a reason for hiding this comment

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

This fails when value contains escape sequences such as key="\"". This fails when value contains multiple '...' pairs, such as key='abc'\''def'. This can simply be

Suggested change
# Handle quoted values (remove outer quotes if present)
if [[ "$value" =~ ^\"(.*)\"$ ]]; then
value="${BASH_REMATCH[1]}"
elif [[ "$value" =~ ^\'(.*)\'$ ]]; then
value="${BASH_REMATCH[1]}"
fi
# Export the variable silently
export "$key=$value"
# Export the variable silently
eval "export $line"

Copy link
Contributor

Choose a reason for hiding this comment

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

I think Oz meant that it unsets vars once you CD out of a directory, however I don't see how it restores a previous value. needs a stack and all.

I now guess that the above part of the code corresponds to the safety.

A typical vulnerability of this type of configuration (such as direnv and autoenv) is that it can load a malicious environment just by cd'ing into the directory. For example, one may download a wild tarball from the internet, extract it, and cd into the directory, assuming that just cd'ing into the directory doesn't execute any code from the tarball. However, if the environment contains key=$(some malicious code), a naive implementation of eval may execute the malicious code.

However, even if the code were not directly executed when the environment is loaded, there are many variables that may be interpreted as commands and executed by applications, such as LESSOPEN, EDITOR, VISUAL, PAGER, etc. Or, LD_PRELOAD in Linux can be used to inject code on an arbitrary exec call. Just setting an environment value in the form export key="$value" isn't actually so safe.

To solve the issue, the framework should require the user to explicitly trust each directory environment (.env) by running something like autoenv trust <dir> and manage the list of sha256 codes of the trusted .env in a safe directory. Or there might be another approach, but I don't have an idea now.

Copy link
Contributor

@hyperupcall hyperupcall Oct 14, 2025

Choose a reason for hiding this comment

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

I don't quite understand the use of repeating the export (or using eval "export ..." if the code is changed to that) here. If the user chooses to write GITHUB_TOKEN=..., I think it would be unexpected for that to be silently converted to an environment variable.

As @akinomyoga mentioned there are security concerns, and autoenv already has a system for allowing or disabling the sourcing of files by their checksum. I think duplicating that here just so some variables are marked as "exported" would be a lot of duplicate work and potentially confusing for the end user.


# Track the variable
dir_vars+=("$key")
_AUTOENV_VAR_SOURCES["$key"]="$env_dir"

else
echo "Warning: Skipping invalid line $line_number in $env_file: $original_line" >&2
fi
done < "$env_file"

# Store variables for this directory
if [[ ${#dir_vars[@]} -gt 0 ]]; then
_AUTOENV_DIR_VARS["$env_dir"]="${dir_vars[*]}"
fi
}

# Check if a directory is an ancestor of the current directory
_autoenv_is_ancestor() {
local ancestor="$1"
local current="$PWD"

# Normalize paths (remove trailing slashes)
ancestor="${ancestor%/}"
current="${current%/}"

# Check if current path starts with ancestor path
[[ "$current" == "$ancestor"* ]]
}

# Clean up variables from directories we've left
_autoenv_cleanup() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer if the function were prefixed with something like _safe_autoenv_ so it's clear that this is separate from the main autoenv project

local dir var_list var

# Check each directory we've processed
for dir in "${!_AUTOENV_DIR_VARS[@]}"; do
# If this directory is no longer an ancestor of current directory
if ! _autoenv_is_ancestor "$dir"; then
var_list="${_AUTOENV_DIR_VARS[$dir]}"

# Unset each variable from this directory
for var in $var_list; do
if [[ "${_AUTOENV_VAR_SOURCES[$var]}" == "$dir" ]]; then
echo "Unsetting $var (from $dir)"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be better UX if print statements were prefixed with "safe_autoenv" so it is clear who is printing to standard output when cding to different directoreis (people may also confuse it with autoenv if it is not prefixed)

unset "$var"
unset "_AUTOENV_VAR_SOURCES[$var]"
fi
done

# Remove directory from tracking AND clear its processed status
unset "_AUTOENV_DIR_VARS[$dir]"
unset "_AUTOENV_PROCESSED_DIRS[$dir]"
fi
done
}

# Main prompt command function
_autoenv_prompt_command() {
local current_dir="$PWD"

# If directory changed, perform cleanup first
if [[ "$current_dir" != "$_AUTOENV_LAST_PWD" ]]; then
_autoenv_cleanup
_AUTOENV_LAST_PWD="$current_dir"
fi

# Always try to initialize the current directory and its ancestors
# The _autoenv_init function will handle checking if processing is needed
_autoenv_init "$current_dir"
}

# Public function for manual use
autoenv() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would like to keep autoenv reserved in case the main autoenv project chooses to use it in the future. Perhaps safe-autoenv or safeenv are good alternatives?

case "$1" in
"reload" | "refresh")
# Clear all tracking and reload
_autoenv_clear_all
_autoenv_init "$PWD"
;;
"status")
echo "Processed directories:"
for dir in "${!_AUTOENV_PROCESSED_DIRS[@]}"; do
Copy link
Contributor

Choose a reason for hiding this comment

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

dir is not localized.

echo " $dir"
done
echo
echo "Active variables by directory:"
for dir in "${!_AUTOENV_DIR_VARS[@]}"; do
echo " $dir: ${_AUTOENV_DIR_VARS[$dir]}"
done
echo
echo "Variable sources:"
for var in "${!_AUTOENV_VAR_SOURCES[@]}"; do
Copy link
Contributor

Choose a reason for hiding this comment

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

var is not localized.

echo " $var -> ${_AUTOENV_VAR_SOURCES[$var]}"
done
;;
"clean" | "cleanup")
_autoenv_cleanup
;;
"clear")
_autoenv_clear_all
;;
*)
_autoenv_init "${1:-$PWD}"
;;
esac
}

# Clear all autoenv state and unset tracked variables
_autoenv_clear_all() {
local var

# Unset all tracked variables
for var in "${!_AUTOENV_VAR_SOURCES[@]}"; do
echo "Clearing $var"
unset "$var"
done

# Clear all tracking arrays
unset _AUTOENV_PROCESSED_DIRS
unset _AUTOENV_DIR_VARS
unset _AUTOENV_VAR_SOURCES

# Reinitialize arrays
declare -A _AUTOENV_PROCESSED_DIRS
declare -A _AUTOENV_DIR_VARS
declare -A _AUTOENV_VAR_SOURCES
Comment on lines +218 to +221
Copy link
Contributor

@akinomyoga akinomyoga Oct 12, 2025

Choose a reason for hiding this comment

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

This doesn't do what is intended. This doesn't initialize global associative arrays. This creates local associative arrays, which are never referenced later. To achieve what is expected, one needs to specify the -g flag, but it requires Bash 4.2+, which is an even stronger requirement than 3.2+.


_AUTOENV_LAST_PWD=""
}

# Hook into bash-it's prompt command system
safe_append_prompt_command '_autoenv_prompt_command'
Loading