Skip to content

Conversation

@JackPhallen
Copy link
Contributor

Add -- separator to all file path arguments in shell commands to prevent paths starting with - from being interpreted as command-line options.

While unlikely, filepaths that start with a hyphen (e.g., -backup, --config) could be misinterpreted as command-line flags. This change adds the -- separator before all variable file path arguments across the codebase, which explicitly signals the end of option parsing and treats all subsequent arguments as positional parameters.

This is a defensive change, but may prevent bugs in the future as more user-defined paths are supported (ei. whitelist and purge paths config)

Example

# Before
cp "$SOURCE_DIR/mole" "$INSTALL_DIR/mole.new"
chmod +x "$INSTALL_DIR/mole.new"

# After
cp -- "$SOURCE_DIR/mole" "$INSTALL_DIR/mole.new"
chmod +x -- "$INSTALL_DIR/mole.new"

A filepath starting with "-" will be interpreted as an argument.
Using a double -- prevents this.
@JackPhallen
Copy link
Contributor Author

Hmmm I'm not sure why the install test is failing. I will have to take a look tomorrow. This isn't that important or urgent anyways.

@tw93
Copy link
Owner

tw93 commented Jan 5, 2026

Hi @JackPhallen,

Thank you for this thoughtful PR and for continuing to contribute quality improvements to Mole! I really appreciate the defensive programming mindset here. 👍

However, after thorough testing, I've found some compatibility issues with this approach on macOS:

Issues Found

macOS BSD compatibility: macOS uses BSD versions of core utilities, and many of them don't support the -- separator syntax:

# Test results on macOS
$ chmod +x -- "$file"
chmod: --: No such file or directory

# Similar issues with:
- chmod
- mv (with -f flag)
- cp
- test

This is why the install test is failing. The GNU/Linux version supports --, but BSD variants (used by macOS) don't consistently support it.

Risk-Benefit Analysis

While I appreciate the defensive approach, the scenario this solves is extremely unlikely in practice:

  • Users rarely create files/folders starting with - or --
  • All Mole-controlled paths use standard conventions (/usr/local/bin, ~/.config/mole)
  • User-defined paths (whitelist, purge) are typically standard paths

The changes affect 21 files across core modules, which is quite extensive for an edge case that may never occur in real usage.

Alternative Approaches

If we want to protect against this scenario, here are some alternatives:

1. Path validation for user inputs only:

validate_user_path() {
    local path="$1"
    if [[ "$path" =~ ^- ]]; then
        log_error "Path cannot start with hyphen: $path"
        return 1
    fi
}

2. Targeted use of -- for GNU commands only:

# Only use -- with commands that support it (rm, find, grep)
rm -rf -- "$path"
find -- "$base_dir" ...

# Keep BSD commands without -- (chmod, mv, cp, test)
chmod +x "$file"

Conclusion

I think we should hold off on this change for now. The compatibility issues outweigh the benefits, especially since the scenario is so unlikely.

That said, your contributions (#254, #257) have been excellent and much appreciated! Please keep them coming. 🙏

Let me know your thoughts!

@JackPhallen
Copy link
Contributor Author

Yup makes sense!

@JackPhallen JackPhallen closed this Jan 5, 2026
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