-
Notifications
You must be signed in to change notification settings - Fork 35
add --ecosystems flag and rename --limit to --pr-limit for socket fix #960
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
add --ecosystems flag and rename --limit to --pr-limit for socket fix #960
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment @cursor review or bugbot run to trigger another review on this PR
| minimumReleaseAge, | ||
| outputFile, | ||
| prCheck, | ||
| prLimit, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Hidden --limit flag alias value is never used
The hidden limit flag is defined as an alias for prLimit, but the code only reads prLimit from cli.flags and never checks cli.flags['limit']. When a user passes --limit 5, meow stores this in a separate limit property, but the code uses the default value of prLimit (10) instead. Unlike the ghsa/id flags which are explicitly combined using cmdFlagValueToArray, there's no logic to merge or prefer the limit value over prLimit, making the --limit alias completely non-functional.
Additional Locations (1)
mtorp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good ✅
I'll let @jdalton review and merge as well.
| outputFile, | ||
| outputKind, | ||
| prCheck, | ||
| prLimit, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we need to check if the old hidden limit is set as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll follow up with a limit alias pr.
Changes: - Add aliases: ['limit'] to prLimit flag definition - Remove separate limit flag from hiddenFlags that was causing conflicts - Add comprehensive tests for --limit alias functionality The --limit flag now properly functions as a hidden alias for --pr-limit, resolving the issue identified in PR #960 discussion.
Ported from v1.x commit 39a114d (#960) - Add --ecosystems flag to limit fix analysis to specific package ecosystems - Rename --limit to --prLimit for clarity (only affects CI/PR mode) - In local mode, process all discovered/provided IDs without limit - Update types, handlers, and tests to use prLimit and ecosystems - Add ecosystem validation with getEcosystemChoicesForMeow() Based on PR #960
Note
Adds ecosystem filtering and renames fix limit to pr-limit (CI-only), processes all vulns in local mode when no --id, and updates Coana CLI.
socket fix):--ecosystemsto limit analysis by ecosystem; supports comma-separated and repeated flags with validation; values passed to coana as--purl-types.--limit→--pr-limit(kept--limitas hidden alias); applied only in CI/PR mode and auto-adjusted by existing open Socket Fix PRs.pr-limithas no effect; when no--idis provided, discover and process all vulnerabilities.cmd-fix.mts,handle-fix.mts, andcoana-fix.mts; updates GHSA discovery and fix invocation.--pr-limit,--ecosystems, ID parsing, and local vs CI behavior.1.1.42; update@coana-tech/clito14.12.113; updateCHANGELOG.md.Written by Cursor Bugbot for commit 6b47dbc. Configure here.