Skip to content

Conversation

@Taywee
Copy link
Contributor

@Taywee Taywee commented Sep 10, 2025

This was causing install to fail for rpm-ostree install.

This was causing install to fail for rpm-ostree install.
@github-actions
Copy link

Script Analysis

  • The changes add XDG_CONFIG_HOME directory handling logic to both post-install.sh and pre-remove.sh scripts
  • Follows standard Unix directory conventions for configuration files
  • Maintains backward compatibility by falling back to $HOME/.config and then $PWD/.config
  • Uses proper conditional checks for environment variable existence

Security Review

  • ❗ Directly using $PWD for config directory in fallback case could lead to unintended locations if script is run from unexpected directories
  • ⚠️ No validation of XDG_CONFIG_HOME or HOME paths for special characters or path traversal
  • Missing error handling if mkdir -p fails
  • No cleanup of temporary .config directory in pre-remove.sh

Optimization Suggestions

  • Consider moving this directory setup logic into the Nushell scripts themselves for better consistency
  • Add error handling and exit on failure for critical directory creation
  • Validate environment variables before use to prevent path injection
  • Consider using path expand in Nushell for more robust path handling

Overall Quality: 3

@Taywee
Copy link
Contributor Author

Taywee commented Sep 10, 2025

closes #69

@github-actions
Copy link

Script Analysis

  • The changes add proper configuration directory handling for Nushell execution in package management contexts
  • Added set -e for better error handling in the shell scripts
  • Implemented a fallback mechanism for XDG_CONFIG_HOME when neither XDG_CONFIG_HOME nor HOME are set
  • Maintains cross-platform compatibility by using standard environment variables

Security Review

  • ❗ Potential security issue: The fallback to $PWD/.config could create config directories in unexpected locations if the script is run from a world-writable directory
  • ⚠️ No input validation on the environment variables before using them in path construction
  • The scripts now properly fail-fast with set -e which improves security by preventing partial execution

Optimization Suggestions

  • Consider using Nushell's built-in config path handling instead of shell script logic
  • Could potentially merge the common directory creation logic into a shared function
  • The current implementation is simple and efficient for its purpose

Overall Quality: 4

@hustcer hustcer merged commit 7d7e1af into nushell:main Sep 10, 2025
11 of 12 checks passed
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