Skip to content

Adapt to modified LGTM_ env variables behavior #125

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

Merged
merged 14 commits into from
Jun 5, 2024
Merged

Conversation

mbaluda
Copy link
Contributor

@mbaluda mbaluda commented May 29, 2024

  • use LGTM_INDEX_FILETYPES: ".json:JSON" to include model.json and *.cds.json files in the DB
  • replace third party action sergeysova/jq-action with a one-liner script
  • renamed the Code Scanning configuration name so that alerts are recreated correctly
  • update javascript.sarif.expected file

@mbaluda mbaluda requested a review from jeongsoolee09 May 29, 2024 19:58
@mbaluda mbaluda enabled auto-merge (squash) May 29, 2024 19:58
@mbaluda mbaluda changed the title Avoids 3rd party actions and modify code scanning paths Adapt to modified LGTM_ behavior May 31, 2024
@mbaluda mbaluda changed the title Adapt to modified LGTM_ behavior Adapt to modified LGTM_ env variables behavior May 31, 2024
@mbaluda mbaluda self-assigned this May 31, 2024
Copy link
Contributor

@jeongsoolee09 jeongsoolee09 left a comment

Choose a reason for hiding this comment

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

@mbaluda Thanks! It looks good overall. Question: why are files in log-injection-with-service1-protocol-none and log-injection-with-service2-protocol-none renamed?

@mbaluda
Copy link
Contributor Author

mbaluda commented May 31, 2024

@jeongsoolee09 Code Scanning running an updated version of the query will not open a new alert if the location does not change, but instead it will reuse the old one. To see the correct result (only one source in service1) I had to rename the test. An alternative could be to delete and recreate the configuration...

@jeongsoolee09
Copy link
Contributor

@mbaluda I see, could you rename it back to the old name (log-injection-service1-protocol-none, and log-injection-service2-protocol-none, respectively) to keep it consistent with the other tests?

@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@mbaluda mbaluda requested a review from jeongsoolee09 June 4, 2024 09:44
Copy link
Contributor

@jeongsoolee09 jeongsoolee09 left a comment

Choose a reason for hiding this comment

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

Looks good, thank you! Sometime we'll have to migrate from using these undocumented environment variables. 🤔

@mbaluda mbaluda merged commit 4359995 into main Jun 5, 2024
5 checks passed
@mbaluda mbaluda deleted the mbaluda-remove-action branch June 5, 2024 17:19
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.

2 participants