-
-
Notifications
You must be signed in to change notification settings - Fork 778
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
Update codeql.yml to exclude YAML front-matter and Liquid code #7081
Update codeql.yml to exclude YAML front-matter and Liquid code #7081
Conversation
Local repository branch is out of sync.
Want to review this pull request? Take a look at this documentation for a step by step guide! From your project repository, check out a new branch and test the changes.
|
Great work!
|
available 7/7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @luisitocanlas Thanks for taking on this issue, and good work so far.
-
I checked out your branch locally, pushed it to my desktop, and triggered "CodeQL Scan" through a
workflow_dispatch
. Unfortunately, when I ran CodeQL on your branch, it generated nearly the same "Security Alerts" as currently on the HfLA main branch. That is, the code edits do not appear to change the "Security alert" results or prevent the "Syntax error(s)" due to YAML front matter or Liquid code. Example alert Let me know if you think I am missing something here. -
Side note: if this route works, I would like to discuss with the team about where the
exclude-patterns.ql
file is located. I believe it should be in a sub-directory withingithub-actions
, possiblygithub-actions/code-ql/
(or similar) rather than as current.
Thank you again for working on this, and again if you think I am missing something let me know!
Hi @t-will-gillis, I rerun the CodeQL workflow on my branch, this time the errors are back under the I'll redo the I also agree that the file should be in a sub-directory in Thank you! |
e704194
to
044aa1e
Compare
Hi @luisitocanlas thanks for slogging through this. To answer the second question, |
Hi @t-will-gillis, I moved the I also tried something different using predicates inside So far, I don't see any errors under For the The screenshot below says that the error appeared on the branch on Jun 13. Thank you again! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @luisitocanlas - I copied your branch and made sure that no "Security" alerts were showing, and then I ran "codeql-scan-job" manually. The result is that CodeQL found 16 alerts, including the same 7 "Could not process some files due to syntax errors".
I could be wrong in interpreting this but I believe that CodeQL is still flagging the Liquid syntax as new errors, that is not just re-stating the errors it found on previous scans.
Comparing your scan job with the one I ran on my repo, I can see that your job did not find errors but that mine did. I don't know why that would be the case. Have you set up "Security" on your repo? Just curious....
@luisitocanlas Also want to reiterate:
|
Hey @luisitocanlas Checking in with you to see your thoughts about this issue- are you still working on it? Last messaging I said that I believe CodeQL is still flagging the Liquid syntax, however if you believe your changes are correct we/you can ask others to review this PR as well. Please let us know how you would like to proceed- |
Hi @luisitocanlas I just noticed that you unassigned from #6548 and left some notes there- thank you. I will discuss this issue with Bonnie and Roslyn to see how we want to proceed, so we may ask for your thoughts too. But again, thanks for taking on this likely frustrating issue. |
Fixes #6548
What changes did you make?
./codeql-queries/exclude-patterns.ql
following the suggestion solution option 1, slightly modifying it.exclude-patterns.ql
to the queries at line 48 in./.github/workflows/codeql-scan-job.yml
Why did you make the changes (we will use this info to test)?
Additional info:
update-codeql-6548
with theexclude-pattern.ql
insidecodeql-scan-job.yml
. The workflow was able to run without throwing any errors.view alerts
on theCode scanning alerts
and see if there are any alerts thrown by the scan.