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

Remove baseDir from IsSkippableDir #39

Merged

Conversation

j0nnyr0berts
Copy link
Contributor

@j0nnyr0berts j0nnyr0berts commented Mar 15, 2022

  • I would expect that if "{sep}var{sep}lib{sep}docker" is included in
    blacklisted_paths that the contents of /var/lib/docker will not be
    scanned
  • However, when scanning docker images, baseDir =
    /tmp/Deepfence/SecretScanning/df_<image_name><imagetag>/ExtractedFiles
    which means that /var/lib/docker is not skipped
  • Removing baseDir from the IsSkippableDir check also allows for
    matching multiple directories with one pattern
    • If this is seen as undesirable then we could always use strings.HasPrefix

Love the software and am keen to use it in CI pipelines but need to be able to effectively
ignore directories to control false positives 🙂

@ramanan-ravi
Copy link
Contributor

@j0nnyr0berts Thanks for your contribution.

But following use case should also be handled:

  • User wants to ignore /lib but not /var/lib

Current code handles this.

Can you make this new behaviour optional based on a configuration parameter here?

@j0nnyr0berts
Copy link
Contributor Author

@ramanan-ravi good point! How about using if strings.HasPrefix(path, skippablePathIndicator) instead?

@ramanan-ravi
Copy link
Contributor

@ramanan-ravi good point! How about using if strings.HasPrefix(path, skippablePathIndicator) instead?

That works

@j0nnyr0berts j0nnyr0berts force-pushed the remove_basedir_from_isskippabledir branch from 7255bbf to d0e1e82 Compare March 24, 2022 16:32
@j0nnyr0berts
Copy link
Contributor Author

@ramanan-ravi good point! How about using if strings.HasPrefix(path, skippablePathIndicator) instead?

That works

Updated! 🙂

@ramanan-ravi ramanan-ravi merged commit 2df7217 into deepfence:master Mar 24, 2022
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

Successfully merging this pull request may close these issues.

2 participants