Suggestion
While working on PR #121, Sourcery-AI flagged the use of shell=True in subprocess calls throughout the codebase. This is a common pattern in NetworkMgr, but using shell=False with argument lists is generally considered safer.
Current Pattern
run(f'ifconfig {netcard} inet6 {inet6} prefixlen {prefixlen}', shell=True)
Proposed Pattern
run(['ifconfig', netcard, 'inet6', inet6, 'prefixlen', prefixlen])
Scope
This affects approximately 50 locations in net_api.py and configuration.py.
Benefits
- Eliminates potential command injection vectors
- More explicit argument handling
- Follows Python security best practices
Considerations
- Commands with shell features (pipes, redirects like
2>/dev/null) would need alternative handling
- Requires thorough testing across all network configurations
- Low priority since inputs come from trusted sources (GUI, system)
This is just a suggestion for future consideration - not urgent. Happy to help with a PR if desired.
Suggestion
While working on PR #121, Sourcery-AI flagged the use of
shell=Truein subprocess calls throughout the codebase. This is a common pattern in NetworkMgr, but usingshell=Falsewith argument lists is generally considered safer.Current Pattern
Proposed Pattern
Scope
This affects approximately 50 locations in
net_api.pyandconfiguration.py.Benefits
Considerations
2>/dev/null) would need alternative handlingThis is just a suggestion for future consideration - not urgent. Happy to help with a PR if desired.