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

False positive for stapler permission check #43

Open
uhafner opened this issue Jul 2, 2024 · 2 comments
Open

False positive for stapler permission check #43

uhafner opened this issue Jul 2, 2024 · 2 comments

Comments

@uhafner
Copy link

uhafner commented Jul 2, 2024

The check 'Stapler: Missing permission check` creates false positives if the permission check is hidden in a facade.

Example (https://github.com/jenkinsci/prism-api-plugin/security/code-scanning/7):

Bildschirmfoto 2024-07-02 um 09 31 21

It would be helpful if the check could be improved.

Is there a way to disable some of the rules in the configuration file in the meantime? I get a lot of these false positives in all my plugins...

@daniel-beck
Copy link
Contributor

daniel-beck commented Jul 2, 2024

hidden in a facade

That wouldn't be a problem. The problem is that it doesn't look at the code of dependencies, and since the actual permission check is in plugin-util-api, that isn't visible to the scanner.

It would be helpful if the check could be improved.

CodeQL cannot identify the actual permission check, so… suggestions welcome. I'd rather not go with method name matching or similar hacks.

Is there a way to disable some of the rules in the configuration file in the meantime?

I think you could change https://github.com/jenkins-infra/jenkins-security-scan/blob/187851c1b2401848d69015680c62b48160f7bce1/.github/workflows/jenkins-security-scan.yaml#L43-L45 to specify the rules you want to use based on https://docs.github.com/en/code-security/codeql-cli/getting-started-with-the-codeql-cli/customizing-analysis-with-codeql-packs#specifying-which-queries-to-run-in-a-codeql-pack. I don't think you can exclude individual queries though, so this comes with quite a maintenance burden. Applying suppressions seems easier.

@uhafner
Copy link
Author

uhafner commented Jul 2, 2024

hidden in a facade

That wouldn't be a problem. The problem is that it doesn't look at the code of dependencies, and since the actual permission check is in plugin-util-api, that isn't visible to the scanner.

I see, then I will stay with my permission tests that utilize ArchUnit. This framework works on the byte code including all dependencies.

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

No branches or pull requests

2 participants