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

Support a fix mode? #28

Open
woodruffw opened this issue Sep 22, 2024 · 5 comments
Open

Support a fix mode? #28

woodruffw opened this issue Sep 22, 2024 · 5 comments

Comments

@woodruffw
Copy link
Owner

Many findings are trivially fixable and insert-only, e.g. inserting persist-credentials: false. It would be nice to expose a --fix flag or similar that could apply these.

Implementation-wise, this should probably happen at tree-sitter level, since tree-sitter has mature support for modifying trees.

@woodruffw
Copy link
Owner Author

Thinking out loud: it wouldn't be too hard to add some kind of .fix(...) to each SymbolicLocation, which yamlpath would then need to properly situate. That would work fine for whole key replacements/insertions, but modifying pre-existing strings/values would be slightly more complicated.

@woodruffw
Copy link
Owner Author

#136 and #129 are continuing to reveal that there are a lot of false positives with template injection, since the github.* context's namespace is massive.

A fix mode could help a little bit here by offloading the work of identifying many of these: GitHub provides default environment variables that cover many common context accesses, and we could suggest/autofix users into those.

@netomi
Copy link

netomi commented Dec 8, 2024

I would love to see a fix mode similar to e.g. ruff.

We developed a simple tool to do action pinning, see: https://github.com/eclipse-csi/octopin, but ultimately, we would like to do other fixes that your tool detects as well. So I could work on a PR to add some action pinning as initial fix. Though I have not used rust before, so that might require some iterations.

@woodruffw
Copy link
Owner Author

So I could work on a PR to add some action pinning as initial fix. Though I have not used rust before, so that might require some iterations.

Please feel free! The main challenge here (I think) will be doing this at the AST level, and not with regular expressions or other string patching techniques.

My plan was to look into doing that with tree-sitter-yaml, but I won't get to it in the near future so I welcome any efforts here 🙂

@netomi
Copy link

netomi commented Dec 8, 2024

So I looked into tree-sitter for another usecase (to modify jsonnet AST), but apparently, tree-sitter does not support editing the AST it generates: tree-sitter/tree-sitter#2667

There are workarounds that I found, but that is not what you wanna do imho as they are very hacky afaict.

go-yaml seems to have some support to preserve whitespace and comments when you do a roundtrip, not sure if something like this exists for rust as well. Having a full blown AST parser that supports source preserving roundtrips would be preferred ofc.

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