Skip to content

Conversation

byu343
Copy link
Contributor

@byu343 byu343 commented Aug 29, 2025

What I did

The existing code does not work because the commands with pipe are only able to run in shell

How I did it

How to verify it

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

The exising code does not work because the commands with pipe
are only able to run in shell
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StormLiangMS
Copy link
Contributor

hi @qiluo-msft @saiarcot895 could you help to take a look?

#
# @return The integer value X from USE_KDUMP=X in /etc/default/kdump-tools
def read_use_kdump():
(rc, lines, err_str) = run_command("grep 'USE_KDUMP=.*' %s | cut -d = -f 2" % kdump_cfg, use_shell=False);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use_shell

There is security related alert on this line by semgrep. ref: https://semgrep.dev/docs/cheat-sheets/python-command-injection#1b-shelltrue.

However in this case you may argue that the command is static constructed and no user input is allowed. It may be not future proof if the command become dynamic constructed.

To make static code analysis easier, let's prevent using shell=True. In this case, it is easy to completely python implementing both grep and cut. No need to run command. Could you explore?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternative, you may want to use some options:

  1. getstatusoutput_noshell_pipe
  2. SysInfoProvider.run_command

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.

4 participants