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

Audit should (have an option to) fail on abandoned packages #11623

Closed
curry684 opened this issue Sep 7, 2023 · 8 comments · Fixed by #11639
Closed

Audit should (have an option to) fail on abandoned packages #11623

curry684 opened this issue Sep 7, 2023 · 8 comments · Fixed by #11639
Labels
Milestone

Comments

@curry684
Copy link
Contributor

curry684 commented Sep 7, 2023

While implementing some security workflows for a client I noticed that composer audit does not in any way care about abandoned packages. Purists would argue that abandoned packages are by definition insecure.

I would argue that composer audit should by default list abandoned packages in use and fail on their presence, or at the very least support this via a flag.

My actual underlying problem is trying to fail a CI workflow when any security issues are found, and right now I don't see any way to do so for abandoned packages. composer outdated supports the --strict flag, but then doesn't support any kind of filtering on abandoned packages. As argued in this issue, I think audit is a better place for this anyway.

@Seldaek Seldaek added this to the Nice To Have milestone Sep 11, 2023
@Seldaek
Copy link
Member

Seldaek commented Sep 11, 2023

Yeah that'd make sense I guess. I'm not sure if I would do this by default, but now that we have anyway a config.audit as of 2.6 we can also add a new option in there like fail-abandoned: true?

@curry684
Copy link
Contributor Author

Makes sense, to be honest I think it wouldn't be breaking anyone's workflow to default it to true: those actually using composer audit are extremely likely to expect/want this behavior anyway. But yeah we could make an option for now and switch its default later on as well.

Could also be a tristate flag: abandoned: ignore/report/fail, if absent for now print a warning that it defaults to report but will default to fail in the future if not configured. That way everybody actively using it will see the report at some point but not get the fail yet.

@naderman
Copy link
Member

I think this is a good idea. I agree that fail by default should be the right state for the audit command. Can live with making it warn first and output that the warning will convert to failure in next composer release, unless report is explicitly set as the mode. Definitely 👍 on the tristate solution. Think for 2.7 it'd even be fine to just go straight to failing by default, as @curry684 pointed out, anyone running this intentionally likely wants this behavior, and otherwise it's easy to disable again.

@curry684
Copy link
Contributor Author

Semi-related: does Packagist every 'automatically' mark packages as abandoned? Because I've also been having some compliance discussions about library versions that are 2+ years old. Such an audit could either be part of Packagist or on the client side (max-age: 730). Could be easy to implement in one go with above feature.

@Seldaek Seldaek modified the milestones: Nice To Have, 2.6 Sep 13, 2023
@naderman
Copy link
Member

It does not, and there are maintained packages that don't require changes in 2 years and that isn't a problem at all, so that's not that great a signal to use directly like that. In particular a lot of metapackages, or other basic ones which mostly just require additional packages.

@Seldaek
Copy link
Member

Seldaek commented Sep 13, 2023

The only case we mark a package abandoned is if the github repo is marked as archived.

@curry684
Copy link
Contributor Author

Thanks, didn't think of the metapackages, good argument against the client here 😉

@Seldaek
Copy link
Member

Seldaek commented Sep 13, 2023

See #11639

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants