Skip to content

Prevent glob expansion from corrupting file discovery#3

Open
pascal-zarrad wants to merge 1 commit into
mainfrom
feature/log-file-endings
Open

Prevent glob expansion from corrupting file discovery#3
pascal-zarrad wants to merge 1 commit into
mainfrom
feature/log-file-endings

Conversation

@pascal-zarrad

Copy link
Copy Markdown
Member

Description

This pull request improves the reliability and correctness of log file discovery in the log rotation system, especially regarding how files are matched by their endings and how the script handles potential shell globbing issues. It also adds new end-to-end tests to verify these behaviors.

Log file discovery improvements:

  • Refactored the logic in logrotate-create-config.sh to build the find command filter as an array, preventing shell glob expansion issues and ensuring only files with the correct endings are matched. Also switched to using -print0 with read -r -d '' for robust filename handling.

Testing enhancements:

  • Added new end-to-end tests in 02_file_discovery.sh to verify:
    • Files without the specified ending are excluded.
    • Rotated files with the ending in the middle of the name (e.g., access.log-20251021) are not matched.
    • File discovery is not affected by .log files present in the container's current working directory, preventing issues caused by glob expansion.

Solves #1

@pascal-zarrad
pascal-zarrad requested a review from Copilot March 13, 2026 23:26
@pascal-zarrad pascal-zarrad self-assigned this Mar 13, 2026
@pascal-zarrad pascal-zarrad added the C1 - bug A bug, if something isn't working label Mar 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Improves log file discovery for the container’s logrotate config generation by preventing shell glob expansion issues and adding E2E coverage to ensure only intended files are matched.

Changes:

  • Refactors logrotate-create-config.sh to build find filters via a Bash array and consume results with -print0/read -d ''.
  • Adds E2E tests validating extension filtering, rotated-file exclusion, and glob-expansion immunity.

Reviewed changes

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

File Description
logrotate.d/logrotate-create-config.sh Builds find predicates as an array and switches discovery to -print0/NUL-safe reads.
e2e/tests/02_file_discovery.sh Adds end-to-end scenarios for file ending filtering, rotated-name exclusion, and glob-expansion regression coverage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +28 to +51
find_filter=()
for ending in $logs_ending; do
if [ ${#find_filter[@]} -gt 0 ]; then
find_filter+=("-o")
fi
let COUNTER=COUNTER+1
find_filter+=("-iname" "*.${ending}")
done
IFS=$SAVEIFS

# Check if regex search is enabled
if [ -n "${LOGS_FILE_REGEX}" ]; then
if [ "$COUNTER" -eq "0" ]; then
LOGS_FILE_ENDINGS_INSTRUCTION="-regex $LOGS_FILE_REGEX"
else
LOGS_FILE_ENDINGS_INSTRUCTION="$LOGS_FILE_ENDINGS_INSTRUCTION -o -regex $LOGS_FILE_REGEX"
if [ ${#find_filter[@]} -gt 0 ]; then
find_filter+=("-o")
fi
find_filter+=("-regex" "${LOGS_FILE_REGEX}")
fi

for d in ${log_dirs}
do
log_files=$(find ${d} -type f $LOGS_FILE_ENDINGS_INSTRUCTION) || continue
for f in ${log_files};
do
if [ -f "${f}" ]; then
echo "Found new file $f, Processing..."
handleSingleFile "$f"
fi
done
for d in ${log_dirs}; do
if [ ! -d "${d}" ]; then
continue
fi

while IFS= read -r -d '' f; do
echo "Found new file $f, Processing..."
handleSingleFile "$f"
done < <(find "${d}" -type f \( "${find_filter[@]}" \) -print0 2>/dev/null)
Base automatically changed from feature/e2e-tests to main March 14, 2026 23:34
Use bash array for find arguments instead of building a string.
The unquoted glob pattern *.log was subject to shell expansion
when .log files existed in the container CWD, causing find to
discover all files regardless of LOG_FILE_ENDINGS setting.
@pascal-zarrad
pascal-zarrad force-pushed the feature/log-file-endings branch from 6a97970 to 599b8f7 Compare March 14, 2026 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C1 - bug A bug, if something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants