manageiq icon indicating copy to clipboard operation
manageiq copied to clipboard

Upgrade brakeman to v6

Open Fryguy opened this issue 1 year ago • 3 comments

@jrafanie Please review.

Fryguy avatar Aug 02 '24 15:08 Fryguy

Looks like a legit failure with newer brakeman

jrafanie avatar Aug 02 '24 15:08 jrafanie

So, I think the problem is that the file path is part of the brakeman.ignore's fingerprint, and engines could live in different places 😭 - I think I have to open an issue with brakeman.

Fryguy avatar Aug 02 '24 15:08 Fryguy

Message: `protect_from_forgery` should be configured with `with: :exception`
Code:
protect_from_forgery :secret => SecureRandom.hex(64),
                     :except => ([:authenticate, :external_authenticate, :kerberos_authenticate,
                     :saml_login, :initiate_saml_login, :oidc_login, :initiate_oidc_login, :csp_report]),
                     :with => :reset_session

File: application_controller.rb:13

kbrock avatar Sep 03 '24 14:09 kbrock

@kbrock https://github.com/ManageIQ/manageiq/pull/23124/files#diff-62c4b53b7988735188b9b2ac5614a6f7a624451ebdd77f125d35dc6ee013b3d2R23

But also, the problem I'm having isn't that issue. The problem is that the issue exists in a plugin, and the file path is part of the false-positive fingerprint. So, locally that file path is different than when it's in CI, and I can't make it work in both environments. I'm really not sure how to fix it :(

Fryguy avatar Sep 03 '24 14:09 Fryguy

@jrafanie This should be ready for review now. I'm also going to cross-repo with ui-class to show that it works properly when run from within the engines.

@miq-bot cross-repo-test manageiq-ui-classic

Fryguy avatar Sep 12 '24 14:09 Fryguy

Checked commit https://github.com/Fryguy/manageiq/commit/efd6938e3095f04d4d3477a10a285847076fdc7c with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint 3 files checked, 8 offenses detected

lib/extensions/brakeman_fingerprint_patch.rb

miq-bot avatar Sep 12 '24 16:09 miq-bot

@jrafanie All of the rubocops are tripping on original brakeman code, so I am not going to change those.

Fryguy avatar Sep 12 '24 16:09 Fryguy

Yeah I'm going to let it bake a bit - maybe backport in a week or 2 to see how the automated runs go and PRs go.

Fryguy avatar Sep 12 '24 16:09 Fryguy

Merging. Whitesource can't check it out so not really a PR error.

jrafanie avatar Sep 12 '24 17:09 jrafanie

Backported to radjabov via merge of master into radjabov

Fryguy avatar Nov 04 '24 22:11 Fryguy