formatting-stack icon indicating copy to clipboard operation
formatting-stack copied to clipboard

Published 20 hours ago • verified publisher icon nedap

Ban symlinks

Open vemv opened this issue 4 years ago • 3 comments

Context

These show how symlinks in linters can go wrong:

  • https://github.com/check-spelling/check-spelling/security/advisories/GHSA-g86g-chm8-7r2p
  • https://github.com/justinsteven/advisories/blob/master/2021_github_actions_checkspelling_token_leak_via_advice_symlink.md

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.

vemv avatar Sep 10 '21 01:09 vemv

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.

thumbnail avatar Sep 10 '21 08:09 thumbnail

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

vemv avatar Sep 10 '21 09:09 vemv

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 👍

thumbnail avatar Sep 10 '21 11:09 thumbnail