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: provide a simple "baseline" functionality #3

Closed
jensschulze opened this issue Nov 11, 2021 · 9 comments
Closed

Feature: provide a simple "baseline" functionality #3

jensschulze opened this issue Nov 11, 2021 · 9 comments
Labels
enhancement New feature or request

Comments

@jensschulze
Copy link
Contributor

Sometimes it can be useful to have a list of files/directories that should not be scanned.

Proposal: with -b FILENAME you can specify a text file that contains one file/directory per line to be excluded from scanning.

(I’m on it)

@ariary
Copy link
Owner

ariary commented Nov 11, 2021

+1

Is there any reason for the -b name of the flag ?

If think -e for exclude is also a good idea

@jensschulze
Copy link
Contributor Author

Originally I thought about perhaps using -e for a regex based exclusion later on … but you are right: -e (--exclude) it shall be 🙂

jensschulze added a commit to jensschulze/TrojanSourceFinder that referenced this issue Nov 11, 2021
Specify an exclude file with -e or --exclude
@jensschulze
Copy link
Contributor Author

I added the feature for the bidirectional check.
This depends on #2

Here is the content of an example exclude file:

# file: excluded.txt

vendor
.git

# comments start with a hash sign
this/does/not/work/atm  # do not mix paths and comments in one line

                              # indented comments are valid
                              indented/paths/are/valid/too.png

./unnecessary_start_but_allowed
trailing_slash/will_be/ignored/
this/looks/ugly/but/will/work.gif/

# only relative paths are allowed!
/do/not/do/this

Example:

tsfinder -c -e excluded.txt .

jensschulze added a commit to jensschulze/TrojanSourceFinder that referenced this issue Nov 11, 2021
Specify an exclude file with -e or --exclude
@ariary ariary added the enhancement New feature or request label Nov 12, 2021
@ariary
Copy link
Owner

ariary commented Nov 12, 2021

Is authorizing the comment line a real need ? I mean, this may lead us to headache to parse correctly the "exclude file" (and also potential breach in the parsing workflow)

For simplicity and effeciency maybe it is better to just accept simple file with 1 file by line
If the file does nto fit => Exit with error

@temp
Copy link

temp commented Nov 12, 2021

+1 for comments, I often find myself quickly commenting out a line for tests.

@temp
Copy link

temp commented Nov 12, 2021

Question: does this functionality really exclude files, or is it ignoring (or supressing the warning) them?
When ignoring, it would be nice to print warning if a file that has NO error is ignored.

@jensschulze
Copy link
Contributor Author

jensschulze commented Nov 12, 2021

@temp it excludes the respective paths from scanning.

When ignoring, it would be nice to print warning if a file that has NO error is ignored.

I think that this kind of check should be an additional feature because I strongly advocate for actually excluding files from scanning with -e.

@jensschulze
Copy link
Contributor Author

jensschulze commented Nov 12, 2021

@ariary my motivations:

  • Comments: more than a nice to have IMO. I use comments often, and they are very helpful for other developers. Especially since excluding files from scanning is a security relevant decision that may need explanation.
  • Processing of the paths: since the exclude file is curated and not generated I as a user want to write the paths knowing there is a certain fault tolerance. The processing is trimming whitespace and normalizing the path string with the standard Go mechanism. The same mechanism applies to the paths in the directory walk so I think this line processing step is reasonable.

What do you think? For the sake of progress we can postpone this matter (implementing a "no processing" approach first) if you are good with the main part of my feature proposal.

jensschulze added a commit to jensschulze/TrojanSourceFinder that referenced this issue Nov 12, 2021
Specify an exclude file with -e or --exclude
@jensschulze
Copy link
Contributor Author

jensschulze commented Nov 12, 2021

I have updated the homoglyph.go file now. The siblings.go part applies to found files only, so no changes here.

Pull Request #5

Open questions:

  • To be decided: the amount of processing of the exclude file.
  • To do: README.md

What do you think?

ariary added a commit that referenced this issue Nov 12, 2021
@ariary ariary closed this as completed Nov 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants