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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security: Migrate from colors.js to some other library, fork, or a vendor-ed alternative #57

Open
fbartho opened this issue Jan 10, 2022 · 4 comments

Comments

@fbartho
Copy link

fbartho commented Jan 10, 2022

A short-term fix to lock prettyjson to a non-corrupt version of colors.js was shipped in #54 馃帀 Great work!

Open Question: What's the longterm plan?

I use prettyjson/colors only as transitive dependencies, but I'm happy to contribute to make a long-term fix to prettyjson if you can explain which approach you prefer.

Options

A. keep colors.js pinned at 1.4.0 forever (end-users may have dependabot constantly trying to upgrade the lib)
B. drop colors (but the colors are pretty!)
C. replace colors.js with a vendored/in-house implementation -- My casual look at prettyjson's source is that we just need compatible implementations of colors/safe::(green|red|grey). I'm not sure of the licensing compatibilities, but it feels like this shouldn't be a very complicated API surface area to replace.
D. replace colors.js with a non-vulnerable fork. What fork should we pick?
E. replace colors.js with some other library. What library? -- chalk was suggested in #29
F. Do something else?

@MichaelDeBoey
Copy link
Contributor

MichaelDeBoey commented Jan 11, 2022

I'd go for E and switch to chalk

@rafeca
Copy link
Owner

rafeca commented Jan 11, 2022

Hey! I agree about switching to chalk, and I'm open to PRs that replace colors with chalk.

Something to take into account is that we need to allow passing custom colors by name via options (e.g --dash=yellow), which I'm pretty sure that chalk supports.

fbartho added a commit to fbartho/prettyjson that referenced this issue Jan 12, 2022
@fbartho
Copy link
Author

fbartho commented Jan 12, 2022

@rafeca I have opened a PR switching from colors to chalk #59!

Passing colors by name still works, though there's an open question about rainbow and other "fancy color generators" should work.

@DABH
Copy link
Contributor

DABH commented Feb 17, 2022

@rafeca @fbartho I have opened a simpler PR in #60 . We can just continue using the same interface with no code changes, since colors is being maintained (just under a new name). Please let me know if you have any questions. Thank you!

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

No branches or pull requests

4 participants