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

Ban symlinks #189

Open
vemv opened this issue Sep 10, 2021 · 3 comments
Open

Ban symlinks #189

vemv opened this issue Sep 10, 2021 · 3 comments

Comments

@vemv
Copy link
Contributor

vemv commented Sep 10, 2021

Context

These show how symlinks in linters can go wrong:

Proposal

Do one of these two (likely they aren't compatible):

  • filter out symlinks as a strategy
  • create a linter that fails whenever a file is a symlink

Acceptance criteria

lein checkouts keep working

Additional resources

git config --global core.symlinks false might be a good thing to perform in CI prior to the checkout step.

@thumbnail
Copy link
Member

create a linter that fails whenever a file is a symlink

This forbids the usage of any symlinks. That seems over opinionated imo.

I'm working on a plugin system for formatting-stack, this would make a good candidate for a plugin. It could be implemented as either a linter or a filter and be configured by the consumer.

This would require my work to be in before we can ship this though. Once the details become more clear I'll share the work in this area.

@vemv
Copy link
Contributor Author

vemv commented Sep 10, 2021

This forbids the usage of any symlinks. That seems over opinionated imo.

👀 maybe we can ban them if (System/getenv "CI"). The idea being that a symlink shouldn't make it past a local setup.

Personally I don't recall a practical use for symlinks in git. When mixed with github/CI things get dangerous.

Security aside, surfacing non-local uses of symlinks seems a good thing to do. Just like clj-kondo/Eastwood show things that while might work, aren't really recommended.

this would make a good candidate for a plugin

Regardless of implementation it seems good to default to security. Especially if we implement PR suggestions (already on the radar); that scenario would get more similar to GHSA-g86g-chm8-7r2p

@thumbnail
Copy link
Member

I gave this a bit more thought; the initial wording threw me off. The linter would simply report on symlinks. Users are still free to ignore the report.

Keeping that in mind; i'd prefer a linter over filtering the files. We could ship that before the refactor 👍

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

2 participants