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

Feature request: extending external configs #236

Open
raveclassic opened this issue Oct 29, 2015 · 48 comments
Open

Feature request: extending external configs #236

raveclassic opened this issue Oct 29, 2015 · 48 comments

Comments

@raveclassic
Copy link

raveclassic commented Oct 29, 2015

I suggest adding some kind of rule to allow extending external configurations like it's done in eslint.

I have different project configurations in external npm package (linters, code inspections, webpack configs, etc.) and as editorconfig is related to codestyle like eslint it would be great to reuse configs in every project.

Example:

extend './node_modules/my-configuration-package/.editorconfig'

[*]
indent_size = 2

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@DullReferenceException
Copy link

+1 for this. It'd be very helpful for enforcing organizational coding styles.

@thomasklein
Copy link

+1

@gilmrjc
Copy link

gilmrjc commented Apr 6, 2017

Quoting the specification:

When opening a file, EditorConfig plugins look for a file named .editorconfig in the directory of the opened file and in every parent directory. A search for .editorconfig files will stop if the root filepath is reached or an EditorConfig file with root=true is found.

You can have a global .editorconfig in you home directory and extend it in your project dir, and also have a editorconfig file in every subdir of your project. I guess this is the closes thing to the feature you request.

@raveclassic
Copy link
Author

@gilmrjc This does not solve the problem. I need to extend .editorconfig from an npm-package which is bundled to the project. For now the only way is to copy-paste it from project to project.

@iamdanthedev
Copy link

Can't bundle it together with tslint settings because of that :(

@kirillgroshkov
Copy link

Still needed!

@hax
Copy link

hax commented Oct 17, 2018

I assume editorconfig settings are much simpler than eslint/tslint, and extends make it a little bit hard to see all settings value, so maybe we should always copy the file?

@raveclassic
Copy link
Author

@hax Exstensibility is always better than copy-pasting

@xuhdev
Copy link
Member

xuhdev commented Oct 17, 2018

@editorconfig/board-member Surprisingly we didn't have much discussion on this... I personally think this can be a good extension allowing large projects to modulize their .editorconfig configurations if extend is replaced by include. For example, we/user can define some standard coding styles and they can be included directly. Let's say we define two coding styles: ./editorconfigs/c-gnu.editorconfig and ./editorconfigs/c-kr.editorconfig. Then, in different sub source tree, we can use

[*.c]
include = ../.editorconfig/c-gnu.editorconfig
[*.cc]
include = ../.editorconfig/c-kr.editorconfig

to switch between different coding styles. However, I think this should be motivated for languages other than C/C++ as well, since I haven't seen another language using significantly standard different coding styles. Thoughts?

@ffes
Copy link
Member

ffes commented Oct 18, 2018

In general I like the idea.

But what about security? Are absolute paths allowed, or required? Is .. allowed? If so, how many levels deep? Should be able to get out of the root of the project? Is it even possible to really detect the root of a project?

One might say that we only have some basic settings that go into a .editorconfig file why worry about security. Think again, see sgur/vim-editorconfig#31 for instance.

And since it is possible to add there own tags, like for instance the Roslyn compiler for .Net, basically the sky is the limit. So security should always something to think about. Although you can say that when somebody adds his own tags it is their responsibility to think about security.

Anyway, we should be very specific about what is allowed and what not. Remember #360 / #361

@xuhdev
Copy link
Member

xuhdev commented Oct 18, 2018

@ffes That's a true concern: We will get the system overly complicated. If we are not motivated enough, I would prefer not supporting include. Are you aware of some template language can generate files following these kinds of rules? Perhaps OP can use it to regenerate .editorconfig files: effectively include and extend are supported this way.

@mrmckeb
Copy link

mrmckeb commented May 24, 2019

Hi all, it looks like this hasn't progressed.

I'd be happy to help out. It looks like extends will work for most people.

@sorenhoyer
Copy link

@mrmckeb are you going to look into this?

@mrmckeb
Copy link

mrmckeb commented Nov 2, 2019

Hi @sorenhoyer, I was waiting for a response from the team - I can still see this being useful for many people.

@muehre
Copy link

muehre commented Nov 21, 2019

I do agree. Pretty useful especially for sharing editor configs between multiple projects.
Would be awesome to extend a company preset of editorconfig.

Edit: Look at browserslist, stylelint, eslint or commitlint etc.. They all provide an option to extend configs.

@florianb
Copy link
Member

@xuhdev, @ffes - Since interest in this problem is continuing, I would like to propose that we may open a formal discussion about a include-functionality.

I am very curious about the intended use-cases since i think it will be a bit of work to create a solution which is performant, safe and meeting a users expectation.

In my current opinion there is (at least) one EC-file in the repository which should be configured the way the repo-owners want their code being formatted. Having external dependencies which might change eventually may disrupt the repo's intentional formatting.

@xuhdev
Copy link
Member

xuhdev commented Nov 22, 2019

I agree -- do you think it would solve the concern by limiting the included file to be no higher than the directory level of the including .editorconfig file?

@florianb
Copy link
Member

I don't know (yet) - as @ffes mentioned i'd fear a directory-traversal attack.

Currently my main pain-point is that i don't see a case where a repo-owner would prefer implicit over explicit EC-rules. Because that's probably the reason why most of the repos (i worked with) have a root = true-statement.


This could be a starting-point for further research:

@stevenvachon
Copy link

...

@mrmckeb
Copy link

mrmckeb commented Feb 29, 2020

@florianb I think there are some sensible defaults that I'd like to see enforced. This is particularly valuable if you work in an organisation with many projects.

I don't think this is hard to set up per project, but it would be great to inherit company defaults - for both new projects and updates to existing projects. It also ensures consistency.

There are approaches we could take as an organisation of course, like populating the file with defaults via a custom package, and updating files (trying to take into account user changes).

Does anyone have any other ideas on how this could be solved without having config extension?

@cxw42
Copy link
Member

cxw42 commented Mar 2, 2020

I can think of two options for various use cases. In the following, / is the repo top dir.

  • For the corporate-standards use case, this could be a plug-in option rather than something in a .editorconfig file. E.g., after the normal directory search finishes, the plug-in can also check one or more files the user (or IT Dept.) has configured. I would suggest pulling the list from an environment variable or a dotfile in a known location (%appdata% or ~/.config) rather than a project for ease of corporate administration.

  • For the npm-packaged rules use case, symlink /.editorconfig -> /node_modules/foo/.editorconfig. Put project-specific EC files, and source files, in /src.

Requesters of this feature, what say you? Neither of these ideas is a panacea, but both avoid requiring plugins to implement a system as complex as eslint's.

@mrmckeb
Copy link

mrmckeb commented Mar 2, 2020

I think those are great suggestions, and we'll likely go with the second as we don't configure developer environments to that level.

@vith
Copy link

vith commented Mar 2, 2020

  • For the npm-packaged rules use case, symlink /.editorconfig -> /node_modules/foo/.editorconfig. Put project-specific EC files, and source files, in /src.

Do symlinks committed to git from a unix system work properly when checked out on a windows system and vice versa?

Trying to answer my own question, it seems to depend on configuring windows and git to make it work: https://stackoverflow.com/a/59761201

I wonder what the situation is with version control systems other than git as well.

@cxw42
Copy link
Member

cxw42 commented Mar 2, 2020

@vith Those are good questions --- I unfortunately do not know for sure. I do know that symlinks work fine for me on Cygwin running in Windows, which is my primary npm environment.

Edit In git repos, my experience is that symlinks checked in from Cygwin check out fine on Linux, and vice versa.

@mrmckeb
Copy link

mrmckeb commented Mar 3, 2020

I'd argue that any developer on Windows should try to use WSL if they can, but I don't think we can say that WSL/Cigwin/etc is a solution to that problem sadly.

So perhaps that brings us back to the config question. Could we overcome concerns of a directory-traversal attack? It seems to me that the implementations (editor plugins, etc.) themselves would need to handle this, rather than the config.

@florianb
Copy link
Member

florianb commented Mar 3, 2020

@mrmckeb thank you very much - i see your point. And absolutely valid, the implementations should disallow misuse as a last resort.

But that's a new issue - i think EditorConfig should be way more strict and explicit about it's implementation. And thinking about past discussions this is very hard to achieve since we have a multitude of different platforms with different abilities. Currently it seems we found some kind of sweet spot where most of the tools using EditorConfig are actually able to support its feature set.

And i am sorry i have to say it like this for my perspective: we (or at least some of us) are thinking hard about ways to improve the standard to better support and represent gaining requirements. But it takes its time to do that right.


To come back to the issue - what is actually the issue? I became mislead by "request[ing a feature for] extending external configs" which is already an solution. For a lot of reasons that's not easy to do.

But the further discussion showed the real pain point is, how provide a standardized editorconfig to a group of users? And this should really not be so much of a problem.

To answer @raveclassic first intent: Why not adding postinstall & preuninstall hooks to your script which ensures the symlink is set to your module's folder?

These hooks are available on many platforms and package managers.

If you drive an organization you already have to provide a dev-environment, don't you (i do)? How do you provide settings and tooling at all?

@cxw42
Copy link
Member

cxw42 commented Apr 17, 2020

Based on the above, my understanding is that we are looking for a way of reading .editorconfig files outside a repo's tree without using symlinks (which may be dicey on vanilla Windows). I note that the discussion above has been about specific files. However, .editorconfig files are currently identified by the directory that contains them. What if we built on that foundation?

I propose a top-level property called search that would add a directory to the plugin's search path.
Then a malicious .editorconfig file would not be able to expressly ask for /etc/passwd. Using search would leave the include and extend keywords available for possible future use. search would be orthogonal to root.

First-draft specification text (edited)

 ## File Processing
 ...
 The search shall stop if an EditorConfig file is found 
 with the root key set to true in the preamble or when 
 reaching the root filesystem directory.
+
+If the preamble includes a 'search' key specifying a path, 
+that path will be canonicalized to an absolute path with no
+symlinks.  Any path including a loop
+or that cannot be canonicalized (for any reason)
+will be discarded.
+The canonicalized path will be added at the end 
+of the list of directories searched for EditorConfig files
+if that path exists and is readable by the current user.

(the basics. You can add to the plugin's search path. Resolving symlinks and making everything absolute right up front makes the rest of the processing simpler. Detecting loops at this stage reduces DoS risk. Finally, both Linux and Windows provide routines to canonicalize.)

+Multiple 'search' keys may be given.  The paths are 
+searched in order the 'search' keys occur in the file.
+However, if a particular canonicalized path has already been
+added to the list of paths, it will not be added again.

(a clear rule in case of unintentional duplicates.)

+While processing EditorConfig files in 'search' directories, 
+ all 'search' keys in those files shall be ignored.

(prevent a DoS attack due to combinatorial explosion in malicious .editorconfig files.)

What say you all?

List of edits

  1. Change "resolve" to "canonicalize"
  2. Reject paths that cannot be canonicalized

@florianb
Copy link
Member

florianb commented Apr 17, 2020

This sounds good!

I'd probably not use the term "resolved" as in many frameworks this is used to describe a functionality to literally resolve an absolute path, containing dots and stuff, to an absolute path.

And i'd like to propose making this more strict like any path must be relative and must consist of real directories (preventing links and linking symbols like single/double-dot).

In order to implement that safely i guess it will be necessary to create a safe resolve function and not to pass the string to any os-method directly.

@cxw42
Copy link
Member

cxw42 commented Apr 17, 2020

@florianb sure - let's use "canonicalize" instead, since that is what the OS/library facilities refer to.
E.g., http://man7.org/linux/man-pages/man3/realpath.3.html, https://docs.microsoft.com/en-us/windows/win32/api/pathcch/nf-pathcch-pathcchcanonicalize. Canonicalizing is a well-studied problem, and existence/readability checks will add additional safety. Nothing is 100% secure, but OS/library functions have much more testing behind them than any individual EC plugin does :) .

@Kasp42
Copy link

Kasp42 commented Aug 26, 2021

+1

@snowinmars
Copy link

snowinmars commented Feb 9, 2022

I don't understand the concern around being able to extend a config outside the current directory. It seems like a desirable capability, i.e. in a monorepos where the .editorconfig might live in a sibling directory next to the code packages.

What could make directory traversal an "attack" in our context?

Agreed: there are a lot of valid cases when a user what to extend a config outside of the current directory.
I came here due to I have a monorepo, microservice per folder. I definitely want to apply .editorconfig from the .git root to a project, and it's possible only if a project folder contains .editorconfig that extends root config:

  • .git
  • .editorconfig # root
  • svc1
    • .editorconfig # extend root
    • ... files
  • svc2 # opening this folder in, f.e., Rider should apply root .editorconfig
    • .editorconfig # extend root
    • ... files

So, yea.

Otherwise, directory traversal risk is a stuff that no one wants to apply. I do expect some startup IDE not knowing how to deal with it.

On the third hand, .browserlistrc could be referenced outside of current directory, and eslintrc either. And HTMLHint and stylelint too.

@itsezc
Copy link

itsezc commented Sep 1, 2022

Has there been any movement on this issue? I think shareable configs are widespread, think IDE or lint configurations. I mainly use TypeScript & Rust, and hate creating an editorconfig for every project and maintaining it, and having a shareable config makes life so much easier, especially when working across multiple projects with the same tooling / stack.

@TheXenocide
Copy link

Just wanted to add that we have a desire to include a base .editorconfig in a NuGet package or custom project SDK (<Project Sdk="Package.Id"> in a .csproj) which gets restored to a non-relative path controlled by tooling, so it might be nice for a formal explanation for how to use tooling-based paths (e.g. extends: nuget:Some.Package.Id or some such), at least enough to provide some degree of consistency for downstream implementations to extend.

More discussion here: dotnet/roslyn#19028

@rendevor
Copy link

rendevor commented Mar 6, 2024

Is there any progress on this?
Is it possible to include HTTP as a source and ping a particular version during implementation?
Should be helpful for huge companies across organizations and repositories.

@TheXenocide
Copy link

TheXenocide commented Mar 19, 2024

We are also still looking for better solutions for this problem.

We have many separate repositories in multiple separate VCS solutions and really don't want to have to copy/paste the same file to some 50+ repositories every time we make a change to it. Automating this is a non-trivial task, provides a sub-par experience, and limits local override flexibility (to a lesser degree).

It seems like Microsoft is reluctant to add support for this functionality to their editors (VS, VSCode) without the underlying spec having some sort of formal agreement on it, so we're dependent on this in order to continue lobbying for the downstream support we need.

@Piedone
Copy link

Piedone commented Mar 19, 2024

If you're working with .NET, then you can do quasi-inheritance/extension with .globalconfig (which is a superset of .editorconfig) files and their global_level property. This is what we also do in our Lombiq .NET Analyzers project to have cascading configuration. E.g., this file with global_level = 0 has the base configuration, and this other one with global_level = 10 overrides some of it.

@ffes
Copy link
Member

ffes commented Mar 20, 2024

It seems like Microsoft is reluctant to add support for this functionality to their editors (VS, VSCode) without the underlying spec having some sort of formal agreement on it, so we're dependent on this in order to continue lobbying for the downstream support we need.

Microsoft don't even follow the official specs in VS, especially trim_trailing_spaces. And the VSCode EditorConfig plugin is not maintained by Microsoft, but by volunteers in this organisation.

@TheXenocide
Copy link

If you're working with .NET, then you can do quasi-inheritance/extension with .globalconfig (which is a superset of .editorconfig) files and their global_level property. This is what we also do in our Lombiq .NET Analyzers project to have cascading configuration. E.g., this file with global_level = 0 has the base configuration, and this other one with global_level = 10 overrides some of it.

That works for defining code analysis rule settings, but does not apply to the text editor settings (tabs vs spaces, indent size, new lines/braces, etc.) which is what we're trying to solve here.

Microsoft don't even follow the official specs in VS, especially trim_trailing_spaces.

Are you sure? It seems like whenever I press enter all the extra whitespace on my current line is trimmed before the editor moves to the next line. Is it possible you're expecting this behavior to apply to all lines in a file on save? I'm not aware of the spec defining the behavior of saving a file, only the behavior of actively editing a file (though admittedly I haven't spent much time with the spec). My curly brace behaviors apply when I create a curly brace, but it doesn't reformat all of the braces in a file if I manually move them to the previous line. Similarly, when I press tab it inserts spaces, but when a file already has tabs it doesn't replace them with spaces unless I manually tell it to. I believe one of my coworkers uses a setting that automatically formats the entire document on save (though this can be somewhat annoying at times).

And the VSCode EditorConfig plugin is not maintained by Microsoft

Interesting, I didn't realize that. Is the VSCode functionality extensible? For example, can the C# Dev Kit for VSCode add .NET-specific functionality to it?

@cxw42
Copy link
Member

cxw42 commented Mar 23, 2024

working across multiple projects with the same tooling / stack.

EditorConfig by design assumes nothing about relationships between projects. (@treyhunner @xuhdev correct me if I'm wrong!) I think that may be part of the communication challenge here.

I am hearing two recurring themes in the comments:

  • "It is easy to add this feature to the spec --- please do so."
  • "It is easy to copy a file when you set up a project --- please do so."

😄 Both of those points are supportable, as witness the good arguments on both sides in this issue.

My own preference is to resolve #415 and then revisit #482 in view of whatever decisions are made in #415. I think part of the strength of EditorConfig is its simplicity. I think it is worth keeping things simple for use cases that don't need the complexity of features such as config inheritance.

@xuhdev
Copy link
Member

xuhdev commented Mar 23, 2024

I agree there's no assumption of relationship between projects here. Allowing referring to an external file would complicate the core libraries quite a bit. (It may not sound so complicated, but a change of requirement to core library would require changes in at least 6 core libraries with totally different implementations.) If we allow this, we would at least have a way to make sure the changes are compatible with the core libraries that do not implement this feature.

@marcospgp
Copy link

Also ran into this while trying to set up an .editorconfig that I can reuse across projects.

Ideally I could include an .editorconfig from a subfolder into my project's root .editorconfig, that way I could have a repo as the source of truth for .editorconfig and include it as a submodule in other repos.

Maybe I can also make a symbolic link for .editorconfig from the submodule folder to the root folder 🤔

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

No branches or pull requests