Skip to content

Comments

Fix unsafe rm in testStrataCommand.sh: remove unnecessary glob and quote variable#455

Open
tautschnig wants to merge 2 commits intomainfrom
tautschnig/script-safety
Open

Fix unsafe rm in testStrataCommand.sh: remove unnecessary glob and quote variable#455
tautschnig wants to merge 2 commits intomainfrom
tautschnig/script-safety

Conversation

@tautschnig
Copy link
Contributor

The cleanup function used 'rm -R ${temp_dir}*' which would expand to 'rm -R *' if temp_dir were ever empty, deleting the current directory's contents. Since all temporary files are created inside the mktemp directory, the glob is unnecessary. Removed it and quoted the variable.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

…ote variable

The cleanup function used 'rm -R ${temp_dir}*' which would expand to
'rm -R *' if temp_dir were ever empty, deleting the current directory's
contents. Since all temporary files are created inside the mktemp
directory, the glob is unnecessary. Removed it and quoted the variable.
Copilot AI review requested due to automatic review settings February 19, 2026 10:55
@tautschnig tautschnig requested a review from a team as a code owner February 19, 2026 10:55
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a critical security vulnerability in the cleanup function of the Strata command test script. The original code used an unquoted variable with a glob pattern that could catastrophically delete the current directory's contents if the temp_dir variable were empty.

Changes:

  • Fixed unsafe rm -R ${temp_dir}* by removing the glob and properly quoting the variable as rm -R "${temp_dir}"

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants