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

C#: Add experimental queries. #72

Merged
merged 5 commits into from
Nov 7, 2024
Merged

Conversation

michaelnebel
Copy link
Collaborator

@michaelnebel michaelnebel commented Oct 29, 2024

In this PR we add all C# experimental queries to the CodeQL community pack.

At least one open question:

  • The PR changes the query id to align with the query ids already in the pack and removes the experimental tag. Should we keep the experimental tag and keep the id's as is? It is unclear as there is also a mention in the requirement doc that Deprecate experimental queries in CodeQL repo and point to new location.

Review on a commit by commit basis is recommended.

@michaelnebel michaelnebel force-pushed the csharp/addexperimentalqueries branch 3 times, most recently from 094d6c2 to 312aea6 Compare November 1, 2024 08:17
@michaelnebel michaelnebel marked this pull request as ready for review November 1, 2024 10:39
@@ -0,0 +1 @@
experimental/Security Features/CWE-1004/CookieWithoutHttpOnly.ql

Choose a reason for hiding this comment

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

@michaelnebel do these paths need to be updated?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Btw. what do you think about the query IDs?

Choose a reason for hiding this comment

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

Im ok using githubsecuritylab but maybe we want to use something related to community packs, @GeekMasher any thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the use of a namespace like githubsecuritylab, I think githubsecuritylab makes sense as this is the org where the community pack resides.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great. I have just prefixed the query IDs with githubsecuritylab. Then lets go with that.
Should we just go ahead and get the queries merged?

pwntester
pwntester previously approved these changes Nov 4, 2024
@michaelnebel michaelnebel force-pushed the csharp/addexperimentalqueries branch 3 times, most recently from f079d10 to e6d4cc4 Compare November 6, 2024 11:02
@michaelnebel michaelnebel force-pushed the csharp/addexperimentalqueries branch from e6d4cc4 to 94cba02 Compare November 7, 2024 11:49
@pwntester pwntester self-requested a review November 7, 2024 12:17
@michaelnebel michaelnebel merged commit 0740889 into main Nov 7, 2024
13 checks passed
@michaelnebel michaelnebel deleted the csharp/addexperimentalqueries branch November 7, 2024 15:00
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.

3 participants