Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regarding shell scripts that are only meant to be source (SC2148) #581

Closed
chauncey-garrett opened this issue Jan 22, 2016 · 24 comments
Closed

Comments

@chauncey-garrett
Copy link

Shellcheck looks to the shebang statement at the beginning of a shell script as one means of determining what language is used. This works well for shell scripts that are meant to be called as an executable.

However, there are non-executable shell files where the convention is to not include the shebang statement in the file because they are meant to be sourced either by the shell during startup (e.g., bashrc) or because they are part of a shell library.

These files would benefit from spellcheck but can't be used as they are without including a shebang statement.

I'd like to propose a directive for this specific case that can specify the language. Something like:

# shellcheck lang=bash

There are other related issues that have brought up this issue but the suggestions were to fall back to sh #215 or ignore the issue #237. I think it'd be best to allow your users the ability to specify the language type to take advantage of language specific features.

I recognize that shellcheck -s dialect exists but this doesn't help in the case of vim's syntastic plugin.

Finally, the lang directive should throw a warning if a shebang is used simultaneously.

Alternatively, the shebang statement could simply be added to these files but I hesitate here because this is not the convention for source files.

Thoughts?

@bittorf
Copy link

bittorf commented Jan 23, 2016

why dont you just include the shebang? it also solves the syntax highlightning for your $EDITOR 8-)

@kurahaupo
Copy link

kurahaupo commented Jan 25, 2016

I just write:

#!/module/for/bash

@jhrr
Copy link

jhrr commented Jan 29, 2016

I agree with @chauncey-garrett, having the directive would be useful. The editor highlithing issue is already handled in a similar way with:

# -*- mode: sh -*-
# vi: set ft=sh :

So having a directive like # shellcheck lang=bash would be more consistent for these non-executed files.

@chauncey-garrett
Copy link
Author

@kurahaupo: While highly unlikely to be exploited, using #!/module/for/bash as the shell directive for interpretation of the script is a security vulnerability. Creative hack though :)

@kurahaupo
Copy link

To create even a potential vulnerability, one would also need execute
permission on the file, and write permission on the root (or a preexisting
/module).

Nobody would be silly enough to use 'chmod +x' on a module... ahem

And as security flaws go, it's nothing beside #!/usr/bin/env whatever
which actually is recommended by many authorities, especially for Bash,
Perl and Python. aaaarghh...

@zimbatm
Copy link

zimbatm commented Mar 1, 2016

I would be in favour of having another pragma syntax to indicate the shell flavour. Adding a shebang is a signal that the script is executable even if the file doesn't have the executable bit.

Right now as a workaround I use #!bash which seem to be detected by vim and shellcheck properly but isn't valid for execution.

@chauncey-garrett
Copy link
Author

@kurahaupo Agreed.

@zimbatm That's the best alternative I've come across.

Adds SC2148 as a keyword.

@koalaman
Copy link
Owner

koalaman commented Mar 9, 2016

This is a recurring question, and n 944313c, you can use a shell annotation:

# shellcheck shell=sh
echo foo &> bar

Any directives specified after (or instead of) the shebang will also apply to the entire script.

Using a shell directive anywhere else is undefined.

@koalaman koalaman closed this as completed Mar 9, 2016
@ljharb
Copy link

ljharb commented May 10, 2016

@koalaman shellcheck.net as well as the latest version on brew do not seem to support this yet. Is v0.4.4 forthcoming?

@brother
Copy link
Collaborator

brother commented May 10, 2016

@ljharb do you have an example of what's not working. The above example from @koalaman works at shellcheck.net for instance.

@ljharb
Copy link

ljharb commented May 10, 2016

@brother ah, # shellcheck shell=sh says ^-- SC1073: Couldn't parse this shellcheck annotation. - looks like it requires a non-comment statement to work?

That doesn't cover the homebrew formula, however :-(

@koalaman
Copy link
Owner

0.4.4 was pushed last week and it appears that @dunn has graciously updated the homebrew formula (thanks!).

Please give it another go.

@ljharb
Copy link

ljharb commented May 23, 2016

@koalaman now with a file that has # shellcheck shell=sh as the first line, when I run shellcheck file.sh I get ^-- SC2148: Tips depend on target shell and yours is unknown. Add a shebang.

@szepeviktor
Copy link

szepeviktor commented Sep 25, 2016

Also!

In debian-setup-functions line 1:
# shellcheck shell=bash
^-- SC2148: Tips depend on target shell and yours is unknown. Add a shebang.

@ljharb
Copy link

ljharb commented Sep 25, 2016

@koalaman any update on this?

@koalaman
Copy link
Owner

In 0.4.4 the statement works, but falsely produces that warning.

It was fixed in July in 42f7479, but is not yet part of a stable release. I'm hoping to push 0.4.5 in the next 2-3 weeks.

@szepeviktor
Copy link

Thank you.

@hugovk
Copy link

hugovk commented Oct 21, 2016

@koalaman A 0.4.5 release would be great, so sourced files can be shellchecked as part of our CI. Thank you!

urbanautomaton added a commit to urbanautomaton/dotfiles that referenced this issue Oct 26, 2016
These aren't directly executable and shellcheck will soon be pacifiable with a
custom directive:

koalaman/shellcheck#581
issyl0 added a commit to alphagov/govuk-aws that referenced this issue Jan 28, 2020
- This switches to use `# shellcheck shell=bash`, as per [this
  comment](koalaman/shellcheck#581 (comment)),
  otherwise we get errors in newer versions of shellcheck while using
  the `# shellcheck disable=SC2148` directive:

```
In terraform/userdata/10-db-admin line 1:
^-- SC2148: Tips depend on target shell and yours is unknown. Add a shebang.
```
issyl0 added a commit to alphagov/govuk-aws that referenced this issue Jan 29, 2020
- This switches to use `# shellcheck shell=bash`, as per [this
  comment](koalaman/shellcheck#581 (comment)),
  otherwise we get errors in newer versions of shellcheck while using
  the `# shellcheck disable=SC2148` directive:

```
In terraform/userdata/10-db-admin line 1:
^-- SC2148: Tips depend on target shell and yours is unknown. Add a shebang.
```
@Jackaed
Copy link

Jackaed commented Oct 21, 2024

#!/usr/bin/env -S echo "This should not be run directly - try using the following command: \nsource"
# shellcheck shell=sh

Preamble for anyone looking for a somewhat nice solution to this problem - this wont execute if you run it directly, and prints a helpful message, but will still source if you source it.

@kurahaupo
Copy link

kurahaupo commented Oct 28, 2024

@Jackaed excellent idea!

Small nit pick on English grammar: "run" is an (extremely) irregular verb that has "ran" as its simple past tense but "run" as its past participle; so I (and many other readers) would prefer:

  • should not be run

rather than:

  • should not be ran.

Or avoid passive voice and side-step the issue:

  • Do not run this directly.

@kurahaupo
Copy link

kurahaupo commented Nov 5, 2024

@kurahaupo: While highly unlikely to be exploited, using #!/module/for/bash as the shell directive for interpretation of the script is a security vulnerability.

Then how about this version:

#!/dev/null/module/for/bash

Since /dev/null is required to be an inode of type "char special", that path can never refer to an extant file.

Or substitute some other name that is required to exist but not as a directory:

/dev/tty/module/for/bash
/bin/echo/module/for/bash
/etc/fstab/module/for/bash

@wileyhy
Copy link

wileyhy commented Nov 9, 2024 via email

@Jackaed
Copy link

Jackaed commented Nov 9, 2024

@Jackaed excellent idea!

Small nit pick on English grammar: "run" is an (extremely) irregular verb that has "ran" as its simple past tense but "run" as its past participle; so I (and many other readers) would prefer:

  • should not be run

rather than:

  • should not be ran.

Or avoid passive voice and side-step the issue:

  • Do not run this directly.

Thanks for the quick lesson, I've corrected my original message! Hope it's useful for people.

@kurahaupo
Copy link

Some Posix options, although there might be some very good reasons not to use any of these as well.

#!/bin/true
#!/bin/false

Just a reminder, the point of this exercise is to have bash as the last part of the apparent interpreter name, so that editors will set their modes accordingly.

We also want to avoid inadvertently encouraging people to treat it as an executable script. which is why we don't just write #!/bin/bash.

#!/bin/sh -c 'exit 254'

This won't work because there's no handling of quotes in a #! line.

If you write:

#!/bin/sh -c exit 254

then on Linux (and most places other than MacOS Darwin) that will be used as:

/bin/sh "-c exit 254" /path/to/script

because the rest of the line isn't divided into multiple arguments. And that might or might not work depending on whether sh accepts the -cCOMMAND as a single argument (rather than -c COMMAND as two arguments).

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

No branches or pull requests