Skip to content

Suggestion: Refactor subprocess calls to use shell=False #122

Description

@Kernel-Error

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels
    No fields configured for Security.

    Projects

    Status
    Ready

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions