Problem/Motivation

Error using drush "skip" option values where id and title are different.

Using any of the skip option values of admin_permissions, fields, input_formats, or trusted_hosts encounters the following error 

Error: Call to a member function getPluginId() on null in <path>/modules/contrib/security_review/src/Commands/SecurityReviewCommands.php on line 158 #0 [internal function]:
...

It appears that SecurityCheckPluginManager method getCheck attempts to match the id with the title.
https://git.drupalcode.org/project/security_review/-/blob/3.1.x/src/Secu...

In SecurityReviewCommands method getCheck the check_name is used as the title argument.
https://git.drupalcode.org/project/security_review/-/blob/3.1.x/src/Comm...

For these checks the title doesn't "match" the id.

Steps to reproduce

drush secrev --skip="trusted_hosts"

Issue fork security_review-3538772

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

joestewart created an issue. See original summary.

joestewart’s picture

joestewart commented
Issue summary: View changes
joestewart’s picture

joestewart commented
Issue summary: View changes

joestewart opened merge request !84

joestewart’s picture

joestewart commented
Category: Task » Bug report
Status: Active » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Good time to maybe add test coverage for this plugin

smustgrave’s picture

smustgrave commented
Status: Needs work » Postponed (maintainer needs more info)

Does https://git.drupalcode.org/project/security_review/-/merge_requests/86/d... solve this?

May be unrelated but wanted to ask.

joestewart’s picture

joestewart commented

It does look similar but did not solve this issue in my quick testing.

joestewart’s picture

joestewart commented
Status: Postponed (maintainer needs more info) » Needs work

smustgrave closed merge request !84

smustgrave opened merge request !94

smustgrave’s picture

smustgrave commented
Status: Needs work » Fixed

I had to start a new MR because of a random conflict but got test coverage added. Not pretty but there

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • smustgrave committed 1382fc4a on 3.1.x 
    feat: #3538772 Error using drush skip option values where id and title...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.