search

nedap / formatting-stack

An efficient, smart, graceful composition of Clojure formatters, linters and such.
Eclipse Public License 2.0
99 stars 2 forks source link

Ban symlinks #189

Open vemv opened 3 years ago

vemv commented 3 years ago

Context

These show how symlinks in linters can go wrong:

Proposal

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

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 commented 3 years ago

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 commented 3 years ago

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 https://github.com/check-spelling/check-spelling/security/advisories/GHSA-g86g-chm8-7r2p

thumbnail commented 3 years ago

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 👍