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.81k stars 215 forks source link

Include a linter for TSQL (aka Microsoft SQL Server Trasact-SQL) #594

Closed jberkers42 closed 2 years ago

jberkers42 commented 2 years ago

Is your feature request related to a problem? Please describe.

In a number of my projects I have Microsoft TransactSQL files that include more than just a plain SQL statement. Using the included SQL Linters (sqllint and sqlfluff) generates failures.

Describe the solution you'd like

A linter for TransactSQL exists at TSQLLint. Would it be possible to include this linter in the MegaLinter project as an alternate SQL Linter? I understand that TSQLLint will likely not be suitable for postgresql or mysql/mariadb databases, so that in most cases where SQL linting is required, some configuration choice will need to be made to turn on the 'correct' linters for the user's project.

Describe alternatives you've considered

Alternatively we could manually run the TSQL Linter, or use the extensible nature of MegaLinter to include it into a pipeline as needed.

Additional context

For scenarios where a developer is using SQL Server in tandem with a .NET application it would make sense to include the TSQL Linter in the .NET flavour of MegaLinter.

nvuillam commented 2 years ago

That's totally doable, thanks for the proposition ^^

If you want to ease my job, please can you provide

At runtime, you'll just have to add the following variables in your .mega-linter.yml config file if you do not want to run other MegaLinter SQL linters:

DISABLE_LINTERS:
- SQL_SQL_LINT
- SQL_SQL_FLUFF
jberkers42 commented 2 years ago

If I may borrow from the TSQL Linter itself:

No Errors

SELECT Foo FROM dbo.Bar;

Failed

SELECT;
GO;

Warnings

SELECT * FROM Bar;

Thanks for the consideration.

nvuillam commented 2 years ago

@jberkers42 PR is working fine on windows, but Mega-Linter is based on a linux alpine image, and it seems there is a tsqllint bug there ...

I declared it , when they solve the issue or provide a workaround, I'll validate the PR :)

https://github.com/tsqllint/tsqllint/issues/267

jberkers42 commented 2 years ago

Thanks for the update, I will look forward to it.

Thanks for actioning this so quickly.

jberkers42 commented 2 years ago

I posted a comment on the linked tsqllint issue. Should be possible to make it work with some apk dependencies:

apk add gcompat libunwind libuuid

Hopefully that is enough to make it work.

Regards, JohnB

nvuillam commented 2 years ago

@jberkers42 great investigation Sherlock 🔍

I updated #596 , let's see if it works better :)

jberkers42 commented 2 years ago

Hey there,

I had a look through the build output for the updated PR, and noted that it still failed with similar errors to previous when running TSQL Linter tests, so I tried to build the container off-line to test for myself. I noted that in my case the build failed. When I scrolled back far enough in the output of the Github Action, I note that the build failed with the same errors, prior to attempting to test the build. The Test seems to have re-used a previously built container, hence the errors not changing.

Now onto the build failure.

The build failed because there was a clash between gcompat and glibc installation that is part of the dartalyzer installation.

(1/1) Installing glibc (2.31-r0)
ERROR: glibc-2.31-r0: trying to overwrite lib/ld-linux-x86-64.so.2 owned by gcompat-1.0.0-r1.
ERROR: glibc-2.31-r0: trying to overwrite lib64/ld-linux-x86-64.so.2 owned by gcompat-1.0.0-r1.
1 error; 1685 MiB in 266 packages
The command '/bin/sh -c wget --tries=5 -q -O /etc/apk/keys/sgerrand.rsa.pub https://alpine-pkgs.sgerrand.com/sgerrand.rsa.pub     && wget --tries=5 -q https://github.com/sgerrand/alpine-pkg-glibc/releases/download/${GLIBC_VERSION}/glibc-${GLIBC_VERSION}.apk     && apk add --no-cache glibc-${GLIBC_VERSION}.apk && rm glibc-${GLIBC_VERSION}.apk     && wget --tries=5 https://storage.googleapis.com/dart-archive/channels/stable/release/${DART_VERSION}/sdk/dartsdk-linux-x64-release.zip -O - -q | unzip -q -     && chmod +x dart-sdk/bin/dart*     && mv dart-sdk/bin/* /usr/bin/ && mv dart-sdk/lib/* /usr/lib/ && mv dart-sdk/include/* /usr/include/     && rm -r dart-sdk/' returned a non-zero code: 1
2021-08-08 11:50:44 [FATAL]   failed to [build] Dockerfile!

There is a clash between the /lib(64)/ld-linux-x86-64.so.2 files from the two packages.

As I performed my test on the "python" flavor, where my test was successful, I checked the Dockerfile for this image, and note that it does not contain the glibc installation as dartalyzer is not included.

It might be that a choice needs to be made between linting Dart and TSQL as the three C Library workarounds for musl-libc included with Alpine seem incompatible.

I have performed some limited testing with dart also, which does not seem to like either of the first two.

Of course the alternative is to re-compile tsqllint assemblies against musl.

Is there a possibility of including tsqllint only on flavors of mega-linter that do not include dart?

nvuillam commented 2 years ago

I think I can try to include tsql only in dotnet flavor, it should do the trick :) Do you think tsql would be necessary in other flavors ?

jberkers42 commented 2 years ago

I think it would make the most sense in the dotnet flavour as Windows based .NET applications are often paired with an MS SQL database in environments I have encountered. It also includes all of the other linters I am interested in for projects where I expect that I will have TSQL Linting requirements.

I cannot say I have enough experience with the other combinations of linters, however, the other one that potentially stands out is the javascript flavour, having also encountered a number of products/project developed around nodejs, react, typescript and SQL Server. Whether you included it in that flavour would be up to you.

I will look to add the dotnet flavour to our CI setup once this is working.

A little off-topic, but, I actually came to mega-linter via super-linter through some posts on a GitLab snippet about using super-linter initially in the GitLab CI pipeline, and switched mostly for the better support of output artifacts in the form of TAP, for which there is a conversion to junit. Thanks again for your consideration to include the TSQL linter.

Regards, JohnB

nvuillam commented 2 years ago

I'll also add it to JavaScript flavor, it can't hurt :)

nvuillam commented 2 years ago

@jberkers42 I merged the PR with tsqllint it is:

As it is not in main flavor, the relative test cases have been skipped during CI

Please can you check with latest/insiders javascript or dotnet flavor to confirm that it works ? Then i'll be able to make an official mega-linter release :)

jberkers42 commented 2 years ago

Just checked, and the tsqllint fails.

Loaded the image up on our Gitlab Runner:

~$ docker run -ti --entrypoint "" nvuillam/mega-linter-dotnet:latest /bin/bash
bash-5.1# cd node_modules/tsqllint/
bash-5.1# ls -la
total 112
drwxr-xr-x   3 root root  4096 Aug 11 09:21 .
drwxr-xr-x 685 root root 20480 Aug 11 09:21 ..
-rw-r--r--   1 root root 59888 May 14  2018 CHANGELOG.md
-rw-r--r--   1 root root  1087 May 14  2018 LICENSE
-rw-r--r--   1 root root  9065 May 14  2018 README.md
-rw-r--r--   1 root root  1743 Aug 11 09:21 package.json
drwxr-xr-x   2 root root  4096 Aug 11 09:21 scripts
-rwxr-xr-x   1 root root  1062 May 14  2018 tsqllint.js
bash-5.1#

Looks like the assemblies are not there, indicating that the build/install did not appear to fetch them.

The tsqllint portion of the run returns the following:

### Processed [SQL] files
- Using [tsqllint vERROR] https://github.com/tsqllint/tsqllint
- Mega-Linter key: [SQL_TSQLLINT]
- Rules config: identified by [tsqllint]
[tsqllint] sql/lraie-metrics.sql - ERROR - 1 error(s)
--Error detail:
node:events:355
      throw er; // Unhandled 'error' event
      ^
Error: spawn /node_modules/tsqllint/assemblies/linux-x64/TSQLLint.Console ENOENT
    at Process.ChildProcess._handle.onexit (node:internal/child_process:282:19)
    at onErrorNT (node:internal/child_process:480:16)
    at processTicksAndRejections (node:internal/process/task_queues:81:21)
Emitted 'error' event on ChildProcess instance at:
    at Process.ChildProcess._handle.onexit (node:internal/child_process:288:12)
    at onErrorNT (node:internal/child_process:480:16)
    at processTicksAndRejections (node:internal/process/task_queues:81:21) {
  errno: -2,
  code: 'ENOENT',
  syscall: 'spawn /node_modules/tsqllint/assemblies/linux-x64/TSQLLint.Console',
  path: '/node_modules/tsqllint/assemblies/linux-x64/TSQLLint.Console',
  spawnargs: [ '/builds/vendors/LogRhythm/lraie-metrics/sql/lraie-metrics.sql' ]
}
[tsqllint] sql/lrmetrics-perms.sql - ERROR - 1 error(s)
--Error detail:
node:events:355
      throw er; // Unhandled 'error' event
      ^
Error: spawn /node_modules/tsqllint/assemblies/linux-x64/TSQLLint.Console ENOENT
    at Process.ChildProcess._handle.onexit (node:internal/child_process:282:19)
    at onErrorNT (node:internal/child_process:480:16)
    at processTicksAndRejections (node:internal/process/task_queues:81:21)
Emitted 'error' event on ChildProcess instance at:
    at Process.ChildProcess._handle.onexit (node:internal/child_process:288:12)
    at onErrorNT (node:internal/child_process:480:16)
    at processTicksAndRejections (node:internal/process/task_queues:81:21) {
  errno: -2,
  code: 'ENOENT',
  syscall: 'spawn /node_modules/tsqllint/assemblies/linux-x64/TSQLLint.Console',
  path: '/node_modules/tsqllint/assemblies/linux-x64/TSQLLint.Console',
  spawnargs: [ '/builds/vendors/LogRhythm/lraie-metrics/sql/lrmetrics-perms.sql' ]
}
❌ Linted [SQL] files with [tsqllint]: Found 2 error(s) - (0.96s)

Looking at the module itself, when you call npm install tsqllint, it pulls the base elements, then runs the scripts/install.js inside the module folder. This then determines the platform and downloads the required assemblies as a zip file from github.com, before decompressing them. Perhaps something failed during the build phase.

Same appears to have happened for the javascript flavour.

If I manually re-run npm install tsqllint it appears to complete without any breaking errors, just a couple of warnings for optional dependencies.

bash-5.1# npm install tsqllint

> tsqllint@1.11.0 install /node_modules/tsqllint
> node ./scripts/install.js

Downloading TSQLLint linux-x64 Runtime [================] 10939078/bps 100% 0.0
npm WARN saveError ENOENT: no such file or directory, open '/package.json'
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@~2.3.2 (node_modules/chokidar/node_modules/fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@2.3.2: wanted {"os":"darwin","arch":"any"} (current: {"os":"linux","arch":"x64"})
npm WARN enoent ENOENT: no such file or directory, open '/package.json'
npm WARN eslint-config-airbnb@18.2.1 requires a peer of eslint-plugin-jsx-a11y@^6.4.1 but none is installed. You must install peer dependencies yourself.
npm WARN eslint-config-airbnb@18.2.1 requires a peer of eslint-plugin-react-hooks@^4 || ^3 || ^2.3.0 || ^1.7.0 but none is installed. You must install peer dependencies yourself.
npm WARN !invalid#1 No description
npm WARN !invalid#1 No repository field.
npm WARN !invalid#1 No README data
npm WARN !invalid#1 No license field.

+ tsqllint@1.11.0
updated 1 package and audited 1842 packages in 33.032s

234 packages are looking for funding
  run `npm fund` for details

found 35 vulnerabilities (3 moderate, 32 high)
  run `npm audit fix` to fix them, or `npm audit` for details
bash-5.1# tsqllint

tsqllint [options] [file.sql] | [dir] | [file.sql | dir]

  -c, --config          Used to specify a .tsqllintrc file path other than the
                        default
  -f, --force           Used to force generation of default config file when
                        one already exists
  -i, --init            Generate default .tsqllintrc config file
  -p, --print-config    Print path to config file
  -l, --list-plugins    List the loaded plugins
  -v, --version         Display tsqllint version
  -h, --help            Display this help dialog

bash-5.1#

Hope that helps tracking down the issue.

nvuillam commented 2 years ago

Mega-Linter uses npm install --no-cache --ignore-scripts (some scripts used to crash so I disabled them by default)

Do you know a way to call post install script manually for a single npm package ?

nvuillam commented 2 years ago

I tried with using descriptor dockerfile section, so it is installed with a single RUN and not in the grouped npm install, let's see what is does :) ( available in flavors once this action is finished https://github.com/nvuillam/mega-linter/actions/runs/1121108678 )

jberkers42 commented 2 years ago

I double-checked the run to build the dotnet image. The installation appeared to be successful.

I have re-triggered a pipeline in our GitLab environment with TSQL linting enabled, and got the expected Your code is bad result.

After applying the fixes recommended by tsqllint the pipeline passed succesfully.

Thanks for your effort integrating tsqllint into mega-linter!

nvuillam commented 2 years ago

@jberkers42 it seems we are good, thanks for the idea and for the quality support into achieving this task :)