-
Notifications
You must be signed in to change notification settings - Fork 94
Support external ESLint configs #86
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
Comments
Probably related to #75 |
Hi @caiges, We don't currently support ESLint plugins or shared configs via NPM due to security concerns. We've made a few exceptions for popular plugins that we now include within the engine. What we recommend for now is to keep a shared config within the repo and periodically update it. Here's a good example: https://github.com/sagiegurari/simple-oracledb/blob/master/.codeclimate.yml Here at Code Climate we have a separate repo which holds our shared configs. Sorry I don't have a better approach to suggest but that's what we can support today. Thanks! |
Thanks for the info @dblandin. Could you elaborate on the security issue if you can? I'd like to help address the issue if it's something ESLint is doing specifically. |
Unlike many popular CI systems, our build infrastructure isn't yet designed to execute customer code. It was designed to run engines statically against source code. Before we allow arbitrary execution of customer code (including NPM packages or RubyGems), we have some work to do to ensure that our systems and customer data remain secure. Unfortunately, this isn't an issue specific to ESLint. It's a shared concern across our platform. Another workaround is to use our CLI, which you can run locally and in your own CI environment. You may be able to run |
@dblandin Wow, great response - thank you. This isn't a showstopper for us and it's nice to know you're thinking about the issue. |
@caiges My pleasure! Thanks for the support! |
Right, so this one is a showstopper for us 😭 Has there been any movement to support third-party shared configs yet? Essentially I want what codeclimate.com is providing i.e. showing improvements on a branch/PR level I have noticed from #75 that you support the airbnb style so a workaround for us would be to be able to configure a custom "extends" in the I hope all this makes sense 😂 |
The same case: we have our own ESLint config in a separate NPM package, it is inherited from one of the common-well-known configs, but we want to use other syntax guidelines.. And now we are unable to use Codeclimate , because custom configs (enclosed in packages) are not supported... |
Hey folks. I just wanted to let you know that I created a sticky issue at qltysh/qlty#480 to document this limitation, some of the reasoning behind it, and provide an avenue for any updates from our side. I'm going to go ahead and close this issue here, because the limitation is broader than any one engine. @mansona the |
I'll split my reply into two comments: To explain my "extends" idea: Essentially I have 3 tiers of rules being applied to my eslint now, the vast majority coming from Airbnb's external config. Next, I have a
The idea of having my own external config was so that I didn't have to maintain this ^^ exception based eslint.json on every repo and change all our repos once we need to change something. The last tier of the solution is to add this to our
This means that the This issue has already had a chilling effect on our development. We want to continue with our own slightly custom coding style, but we have had to remove a lot of the customisations we were using because having Code Climate is more valuable to us than having our style followed. Just to be clear this is not a good thing. If you are not going to allow us to use the standard method of including custom configs, I would recommend that you do something on the backend like allowing us to define our own rules in our accounts that can apply to all our applications. This way we could have our local |
@pbrisbin I am commenting here instead of qltysh/qlty#480 because that issue is locked, are you planning to open that up for further discussion? I know that you have a major security issue to contend with here (and I'm not a security professional) but I don't understand why this isn't a solved problem already? Travis-ci already allows you to install any npm package your want, surely we can re-use some of their solutions for this? |
About your extends idea:
Org-wide config is an initiative we hope to take on soon, potentially next quarter. It's a meaty project but something that would provide great value. This feature is broader than any one engine. It doesn't sound like your suggesting any changes that would make sense within this engine specifically to support your use-case better, is that correct? About the locked issue:
Not at this time. We believe we understand the limitations (we feel the same pain as our users), and that Issue is meant as a notification and way we can provide updates going forward, not a mechanism for wider discussion, or
We may be being overly paranoid here, but we'd still prefer that. The difference is that Travis will let you run commands you wrote via your configuration file. With our system, there is code a 3rd party wrote (open source engines) being run with access to your source code. It's not that we specifically want to prevent npm package installs, we want to prevent a malicious engine from To be clear: we don't suspect anything nefarious to be going on in the community-provided engines, and we do vet them in many ways (for quality and security concerns). But we feel it's important to maintain a last-line-of-defense at the network layer to ensure that any future mistakes on our part don't result in our customers losing valuable IP. |
I realize this is an old thread and perhaps this stuff is no longer really on your radar, but this single issue causes a major headache for our organization (and I imagine many others). The assertion that by refusing to run eslint plugins that you haven't vetted somehow protects an ignorant developer from having their source code stolen is dubious at best as it assumes that the developer in question isn't running eslint checks on their own local machine, where the evil plugin could do all the damage it wanted well before hitting your service. If this is the only reason we can't use popular plugins (ie |
Hi @aaroncraig10e, sorry this is causing you headaches.
No one's made this assertion and what you describe is not at all our motivation.
Our stance is that there is security risk with allowing engines to make network calls. Running a shared plugin that's not pre-installed requires a network call to install it. Therefore, we cannot run plugins that are not pre-installed in the engine image. We do not "vet" the plugins. We just limit what we pre-install to those with reasonable adoption. This is just a trade off between the value of having the plugin there (dependent on how many users would use it) and impact of having a larger image to operate (which can impact run and deploy time for everyone). Now, the
Have you considered ignoring those check(s) via I hope this reply helped clarify some things. |
Thanks for the quick response. Unless I'm misunderstanding, I think we are saying the same thing -- your reasoning holds that by restricting networking, a potentially malicious engine could steal source code. I am simply pointing out that any code installed through npm's package.json would probably have already been run on the developer's local machine, where networking is not restricted, and therefore could also steal code directly from the developer's machine. Perhaps I'm not understanding why executing code in your environment is somehow different from executing the same code in another environment? Regarding the suggestion that I ignore the checks via .codeclimate.yml, I tried that. Configuring to turn off import checks still provokes the error, probably because the eslint engine still tries to parse this bit in .eslintrc.json:
I suppose I could make another eslint config file just for Code Climate, but that would then mean I have to maintain two eslint configs, which is sub-optimal. |
Again, we're not targeting that attack vector here. We are protecting against an engine doing something malicious. As a concrete example, if someone compromised our docker registry and replaced
It's not. When you run a codeclimate engine, its networking is restricted. This is true if run locally via the codeclimate CLI or on our hosted environment. |
I see, that makes more sense. I hadn't understood that you were concerned with a breach in your own security. At the risk of beating a dead horse, I still wonder whether or not that's a useful tactic, seeing as how if someone manages to get enough access to replace the code in one of the engines that you maintain, you probably have a lot more to worry about than just having your client's code stolen. |
We also encountered this problem with our shared projects. I ended up getting around it by writing a simple utility to recursively reduce inheritances. It might be helpful for others encountering this issue - https://www.npmjs.com/package/eslint-reduce We added this to the prepublish hook of our shared config, so that whenever we publish a new version we also share the reduced version without the external dependencies. It basically looks like this: scripts": {
"prepublish": "npm run build",
"build": "eslint-reduce -v -f index.js -o ./dist/.eslintrc.json"
} Because code climate does not run # Dependency directory
node_modules/
# do not ignore eslint prebuilt for code climate
node_modules/eslint-config-loanmarket-react/*
!node_modules/eslint-config-loanmarket-react/dist and our codeclimate.yml also points to our node module eslint:
enabled: true
channel: "eslint-4"
config:
config: ./node_modules/eslint-config-loanmarket-react/dist/.eslintrc.json I hope someone else finds this useful. |
Did you know a way to use |
@Xosmond I think this question is better suited for a separate issue but the engine should already support a |
For my organization's projects and my own I use an external npm package that houses my eslint config. It would be nice to support this workflow as eslint configs change slowly so sharing via npm package or similar means is a fairly common practice. This workflow is described in ESLint's shareable configs documentation.
The text was updated successfully, but these errors were encountered: