oxsecurity / megalinter

🦙 MegaLinter analyzes 50 languages, 22 formats, 21 tooling formats, excessive copy-pastes, spelling mistakes and security issues in your repository sources with a GitHub Action, other CI tools or locally.
https://megalinter.io
GNU Affero General Public License v3.0
1.82k stars 214 forks source link

`cli_lint_user_args` not recognised as expected #755

Closed llaville closed 2 years ago

llaville commented 2 years ago

Describe the bug Side effect found when specify addtional user arguments on linters

To Reproduce Steps to reproduce the behavior:

  1. Use a flavor that have at least dotenv-linter
  2. Add additional arguments such as ENV_DOTENV_LINTER_ARGUMENTS: "--skip UnorderedKey" in your .mega-linter.yml config file
  3. Activate debug mode LOG_LEVEL: "DEBUG"
  4. Use at least a source file with unordered key contents. For example https://github.com/devilbox/flames/blob/master/flames/selenium/env-example

Expected behavior dotenv-linter should return No problems found as in bottom of following screenshot

native_dotenv-linter

Actually we got debug_dotenv-linter

Side effect is : Mega-Linter return all success, that is partially TRUE if we considered https://github.com/dotenv-linter/dotenv-linter/issues/384

Reason come from usage of https://github.com/nvuillam/mega-linter/blob/v4.45.0/megalinter/config.py#L132

And compare actual runs

ENV_DOTENV_LINTER_ARGUMENTS: "-s UnorderedKey"

[dotenv-linter] command: ['dotenv-linter', '-s', 'UnorderedKey', '/tmp/lint/flames/selenium/env-example']
[dotenv-linter] CWD: /
[dotenv-linter] result: 0 Nothing to check
ENV_DOTENV_LINTER_ARGUMENTS: "--skip UnorderedKey"

[dotenv-linter] command: ['dotenv-linter', '--skip', 'UnorderedKey', '/tmp/lint/flames/selenium/env-example']
[dotenv-linter] CWD: /
[dotenv-linter] result: 0 Nothing to check

With workaround

ENV_DOTENV_LINTER_ARGUMENTS: "--skip=UnorderedKey"

[dotenv-linter] command: ['dotenv-linter', '--skip=UnorderedKey', '/tmp/lint/flames/selenium/env-example']
[dotenv-linter] CWD: /
[dotenv-linter] result: 0 Checking tmp/lint/flames/selenium/env-example

No problems found

Additional context

I recommand to use https://docs.python.org/3/library/shlex.html#shlex.quote rather than https://docs.python.org/3/library/shlex.html#shlex.split

If @nvuillam you are agree a PR may follows !

See script to compare results

import shlex

executable = 'dot-linter'

command = 'dot-linter {}'
args = '-s UnorderedKey'

remote_args = shlex.split(args)
remote_command = command .format(remote_args)
native = [executable] + remote_args
print('native --', native)
print('>> shlex split  -- ', shlex.split(args))
print('>> shlex split command -- ', remote_command)

remote_args = shlex.quote(args)
remote_command = command .format(remote_args)
cmd = [executable] + [remote_args]
print('native --', native)
print('>> shlex quote -- ', shlex.quote(args))
print('>> shlex quote command -- ', remote_command)
llaville commented 2 years ago

@nvuillam Finally I found the real origin of problem. This is not the usage of shlex.spit as mentioned, but order between arguments and options with dotenv-linter

As we seen with workaround (ENV_DOTENV_LINTER_ARGUMENTS: "--skip=UnorderedKey"), that works fined I didn't realized that position was important.

See tests proceed with native dotenv-linter dotenv_linter_2

First test is without any additional argument that show error we must found

Second test is also OK when we specify arguments after executable and filename

Third test is KO, when we specify executable, arguments and end with filename (syntax built by Mega-Linter)

[dotenv-linter] command: ['dotenv-linter', '--skip UnorderedKey', '/tmp/lint/flames/webgrind/env-example']
[dotenv-linter] CWD: /
[dotenv-linter] result: 1 error: Found argument '--skip UnorderedKey' which wasn't expected, or isn't valid in this context
        Did you mean --skip?

USAGE:
    dotenv-linter <input>... --skip <CHECK_NAME>...

For more information try --help

I'm able to notice it when I see in code that I can specify a list of arguments such as : ENV_DOTENV_LINTER_ARGUMENTS: ["--skip UnorderedKey"]

Last test is OK, because it's same as workaround solution

llaville commented 2 years ago

A real show case with current ci_light:v4 flavor (no code changes) and workaround applied on user arguments is available with commit https://github.com/llaville/flames/commit/68e5b0c4dbc4990bd09b9dc328374f02b3d3637e

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

llaville commented 2 years ago

gh stale, friendly ping that no reply was given since a month now 😣

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

llaville commented 2 years ago

:frowning_face:

nvuillam commented 2 years ago

It's not i don't want to solve it, it's that I'm dying under my daily work , so I have less time for more complicated problems ^^

What if you try ENV_DOTENV_LINTER_ARGUMENTS: ["--skip", "UnorderedKey"] ? :)

llaville commented 2 years ago

I'll close this issue definitivly now, because it does not exists anymore. I've re-tested again will all the following syntaxes without trouble

ENV_DOTENV_LINTER_ARGUMENTS: ["--skip", "UnorderedKey"]
ENV_DOTENV_LINTER_ARGUMENTS: ["--skip=UnorderedKey"]
ENV_DOTENV_LINTER_ARGUMENTS: "--skip=UnorderedKey"
ENV_DOTENV_LINTER_ARGUMENTS: "--skip UnorderedKey"
ENV_DOTENV_LINTER_ARGUMENTS: "-s UnorderedKey"