Skip to content

Recognize all forms of su's command option in SC2117 - #3613

Open
pSub wants to merge 1 commit into
koalaman:masterfrom
pSub:su-command-option
Open

pSub wants to merge 1 commit into
koalaman:masterfrom
pSub:su-command-option

Conversation

@pSub

@pSub pSub commented Nov 7, 2026

Copy link
Copy Markdown

SC2117 used the argument count to decide whether su runs a command. That allowed 'su -c foo' but flagged the equivalent long form 'su --command=foo'. Instead, look for the command option itself: -c, -cfoo, short option clusters like -lc, --command or --command=foo.

SC2117 used the argument count to decide whether su runs a command.
That allowed 'su -c foo' but flagged the equivalent long form
'su --command=foo'. Instead, look for the command option itself:
-c, -cfoo, short option clusters like -lc, --command or --command=foo.

@e-kwsm e-kwsm left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In BSD implementation -c can be class:

-c class
Use the settings of the specified login class. The login class must be defined in login.conf(5). Only allowed for the super-user.

su -m -c staff operator -c 'shutdown -p now'
Same as above, but the target command is run with the resource limits of the login class "staff". Note: in this example, the first -c option applies to su while the second is an argument to the shell being invoked.

https://man.freebsd.org/cgi/man.cgi?query=su&sektion=1&format=html

@pSub

pSub commented Nov 8, 2026

Copy link
Copy Markdown
Author

This would mean, that we miss a warning for su -c staff operator on BSD, right?. Not sure if it would make sense / is possible to do different checks depending on the OS. I couldn't find an example in shellcheck.
From my perspective this PR is still an improvement over the current state.

@e-kwsm

e-kwsm commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator

I have no BSD machines, but IIUC su -c staff operator starts an interactive session as operator with setting of class staff. su --command foo is not ambiguous, though it is not portable.
sudo and doas are preferable, I think.

@slycordinator

Copy link
Copy Markdown
Contributor

This would mean, that we miss a warning for su -c staff operator on BSD, right?. Not sure if it would make sense / is possible to do different checks depending on the OS. I couldn't find an example in shellcheck. From my perspective this PR is still an improvement over the current state.

If the checks were to depend on OS, this raises questions:

  • Which OS should be assumed when run in Windows (like in the vscode extension)?
  • Should we have checks default to where the recommendations can ensure that the scripts aren't portable?

@e-kwsm

e-kwsm commented Nov 12, 2026

Copy link
Copy Markdown
Collaborator

Unfortunately su implementation varies between Linux and BSD as it is not specified in POSIX, and sudo and doas are not always avaiable.

In BSD, su -c cmd tries to start an interactive session with class of cmd but does not execute cmd; su root -c cmd is portable.

$ uname -sr
OpenBSD 8.0
$ su -c whoami
su: only the superuser may specify a login class
$ su root -c whoami
root
$ su -
# su -c whoami
su: no such login class: whoami
# su -c build  # start an interactive session as root (implied) with class of build
# exit
# su -c build root  # the same as above
# exit
# su root -c whoami
root
# su -c build root -c whoami
root

shellcheck targets BSD and macOS as you can see in README. There are issues expressing concerns about portability between GNU Linux and BSD e.g. #2902.

In that sense the current message does not care portability either:

$ shellcheck -s sh -f gcc - <<< 'su alice'
-:1:1: note: To run commands as another user, use su -c or sudo. [SC2117]
command run command on Linux run command on BSD
su -c cmd Y N
su -c cmd foo Y N
su foo -c cmd Y Y
su -c bar foo -c cmd Y§ Y
su --command cmd Y N

§ silently ignores -c bar

@e-kwsm

e-kwsm commented Nov 12, 2026

Copy link
Copy Markdown
Collaborator
su -c foo        # SC2117 on BSD
su root -c foo   # better
su --command foo # this does not work on BSD, but should SC2117 cover it?

# atm shellcheck does not validate options to su
#su -z foo  # invalid option 

This branch has not been deployed

No deployments
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.

3 participants