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

Add options to Binary Artifacts policy to ignore listed files. #176

Merged
merged 1 commit into from
May 5, 2022

Conversation

jeffmendoza
Copy link
Member

I decided it makes sense to ignore files at the org level (ex: "gradle-wrapper.jar"), and to only allow full-path ignore at the repo level (ex: "gradle/wrapper/gradle-wrapper.jar").

// IgnoreFiles is a list of file names to ignore. Any Binary Artifacts found
// with these names are allowed, and the policy may still pass. These are
// just the file name, not a full path. Globs are not allowed.
IgnoreFiles []string `yaml:"ignoreFiles"`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any plans to add list of acceptable hashes? Hard to maintain...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not in allstar. Maybe with raw results or with scorecard validation.

@jeffmendoza jeffmendoza merged commit 011a434 into ossf:main May 5, 2022
// IgnoreFiles is a list of file names to ignore. Any Binary Artifacts found
// with these names are allowed, and the policy may still pass. These are
// just the file name, not a full path. Globs are not allowed.
IgnoreFiles []string `yaml:"ignoreFiles"`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion - make the allowlist more flexible:

AllowList struct {
  // Allow all files with name "gradle-wrapper.jar"
  Filenames []string
  // Allow the file at path "dir1/dir2/gradle-wrapper.jar"
  Filepaths[]string
  // Allow all files under directory "testdata/"
  Dirs []string
  // Stretch idea - support regexes. Not needed for now.
}

Users can set either of these options depending on their need. Wdyt?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to balance flexibility/usability with security and also complexity.
For example, filenames is inherently less secure than paths. However, filepaths is not very usable at the org level.
Another balance is the "repo override". For example, an org admin might leave repo override disabled, but wish to allow full-path ignore at the repo level. In this case the admin wouldn't want to allow globs, because the repo could just ignore "*".
Keeping the options limited encourages secure use, and decreases config complexity.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of having different type (dir, files, etc), it may be simpler for a user to just provide a list of Glob patterns https://pkg.go.dev/path/filepath#Glob.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the above reasons, I don't want to have glob. To encourage secure use, it would be better to add some extra friction to adding new binary artifacts to a repo by having to add it to a list. Also, without a glob, the ignore config can serve as an audit list of binary artifacts.

For both of these, we can launch and iterate, if we get users asking for globs or more complex config, it can be added.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, I had not thought about the org-level problems. I like the idea of an audit list as well

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify, we allow filenames at org-level, but repos can have filepath-level overrides. Is that correct?

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.

3 participants