Skip to content

Conversation

@timlegge
Copy link
Contributor

@timlegge timlegge commented Jun 14, 2025

I have noticed that having a security_contact option would be useful.

Dist::Zilla::Plugin::SecurityPolicy could then specify:

[SecurityPolicy]
 -policy = Individual
 timeframe = 2 weeks
 security_contact = Security Contact X. Ample <sec_x.example@example.com>

that would then overide the author(s)

@timlegge timlegge requested a review from Leont June 14, 2025 14:51
@timlegge
Copy link
Contributor Author

@robrwo you might want to review this one too

@robrwo
Copy link
Contributor

robrwo commented Jun 14, 2025

Some comments:

  • It's potentially confusing for the reader of a policy to see a maintainer and a separate security contact
  • If there's a security contact and a report_url, which takes priority?
  • Or should there always be a report_url, which if it's not set default to a mailto: for the security contact or maintainer?
  • There should be keys in the distribution metadata that have this information,
    • Should this use defaults from the distribution metadata?
    • Or should there be an option for it to set those keys in the distribution metadata

@timlegge
Copy link
Contributor Author

Some comments:

  • It's potentially confusing for the reader of a policy to see a maintainer and a separate security contacthe

  • If there's a security contact and a report_url, which takes priority?

  • Or should there always be a report_url, which if it's not set default to a mailto: for the security contact or maintainer?

  • There should be keys in the distribution metadata that have this information,

    • Should this use defaults from the distribution metadata?
    • Or should there be an option for it to set those keys in the distribution metadata

@robrwo I have clarified some of the pod. The security_contact is meant to allow you to override the author. When I take over a module I don't like to change the author in the dist.ini (or elsewhere). This allows me to say instead of the author use the provided security contact.

The report_url overrides both so I clarified that.

I should probably make the test for security contact demonstrate that the report_url will override both.

Copy link

@sjn sjn left a comment

Choose a reason for hiding this comment

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

Added a few comment around the interactions between security_contact, report_url and maintainer. I'm thinking some of this could be made a little clearer? :-)

the current maintainer for the distribution; B<Required>
if a security_contact is defined it will override the maintainer.
Copy link

Choose a reason for hiding this comment

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

This text is maybe a bit ambiguous? I'm wondering it may be useful to offer answers to the question "What happens if I don't set this value?".

If the security_contact field is defined, it will be presented as the primary contact point for security issues. The maintainer field will remain for as a contact point for other issues. If unset, the maintainer remains presented as the primary contact point for all issues.

Same would go for report_url.

The B<report_url>, if defined, will override both the B<maintainer>
and B<security_contact> (if defined)
Copy link

Choose a reason for hiding this comment

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

Likewise as above,

If the report_url field is defined, it will be presented as the primary contact point for security issues. The maintainer field will continue to be presented as a contact point for other issues.
If unset, the maintainer remains presented as the primary contact point for all issues.
If the security_contact field is also set, it will be offered as an secondary contact point for security issues.

Does this make sense?

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