Skip to content

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

@Kernel-Error

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

    Projects

    Status

    Ready

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions