馃摉 Improve rationale for Binary-Artifacts #1016
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I'm fine with prohibiting binary executables, but the *rationale* for doing this was completely unclear. This commit rewrites the rationale to explain, in hopefully a better way, why they can be a problem. I prefer "executable" over "binary". On digital computers, all data (including source code) are binaries :-). In addition, some executables are simultaneously executables and source code, e.g., shell scripts. So I think what is meant here is a "generated binary". I don't really think this merits a "High" level, but that's a different dicussion. Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
docs/checks.md
Outdated
| This check tries to determine if the project has binary artifacts in the source repository. | ||
| Binaries are a threat to auditability and vulnerability management. In addition, a binary could be compromised or malicious. A low score is therefore considered `High` risk. | ||
| This check tries to determine if the project has generated executable (binary) artifacts in the source repository. | ||
| Including generated executables in a source repository increases user risks. Many programming language systems can generate executables from source code (e.g., C/C++ generated machine code, Java `.class` files, Python `.pyc` files, and minified JavaScript). Users will often directly use executables if they are included in the source repository, leading to many dangerous behaviors. One problem is that reviews becomes ineffective and thus enable the use of obsolete or maliciously-subverted executables. Reviews generally review source code, not executables, since it's difficult to audit executables to ensure that they correspond. Since it's hard to audit this, over time the included executables might not correspond to the source code. Another problem is that it allows the executable generation process to atrophy, which can lead to an inability to create working executables. These problems can be countered with verified reproducible builds, but it's easier to implement verified reproducible builds when executables are not included in the source repository (since the executable generation process is less likely to have atrophied). |
There was a problem hiding this comment.
Including generated executables in a source should be Including generated executables in a source,
There was a problem hiding this comment.
One problem is that reviews becomes should be One problem is that reviews become
There was a problem hiding this comment.
Including generated executables in a sourceshould beIncluding generated executables in a source,
I don't think so. The next word is repository. That would produce Including generated executables in a source, repository increases user risks. The first sentence uses the phrase the source repository`, so let's use this:
Including generated executables in the source repository increases user risks.
One problem is that reviews becomesshould beOne problem is that reviews become
Agreed!
docs/checks.md
Outdated
| Binaries are a threat to auditability and vulnerability management. In addition, a binary could be compromised or malicious. A low score is therefore considered `High` risk. | ||
| This check tries to determine if the project has generated executable (binary) artifacts in the source repository. | ||
| Including generated executables in a source repository increases user risks. Many programming language systems can generate executables from source code (e.g., C/C++ generated machine code, Java `.class` files, Python `.pyc` files, and minified JavaScript). Users will often directly use executables if they are included in the source repository, leading to many dangerous behaviors. One problem is that reviews becomes ineffective and thus enable the use of obsolete or maliciously-subverted executables. Reviews generally review source code, not executables, since it's difficult to audit executables to ensure that they correspond. Since it's hard to audit this, over time the included executables might not correspond to the source code. Another problem is that it allows the executable generation process to atrophy, which can lead to an inability to create working executables. These problems can be countered with verified reproducible builds, but it's easier to implement verified reproducible builds when executables are not included in the source repository (since the executable generation process is less likely to have atrophied). | ||
| It's fine to include, in the source repository, files that are simultaneously reviewable source code and executables, since these are reviewable. (Some interpretive systems, such as many operating system shells, don't have a format for separate generated executables.) Similarly, we allow including in the source repository source code generated by other tools (e.g., by bison, yacc, flex, and lex). There are potential downsides to generated source code, but generated source code tends to be much easier to review and thus presents a lower risk. Generated source code is also often difficult for external tools to detect. |
There was a problem hiding this comment.
separate should be separately
There was a problem hiding this comment.
I don't think so. separately generated focuses on generation, but we're not focused on generating source code files through some different process. The focus here is that the source files are separate from the executable files, so I think separate is correct. I'll reword the sentence to emphasize the point: in some cases there is no executable file that lives independently from the source file.
naveensrinivasan
changed the title
Tweak Binary-Artifacts text based on comments from @naveensrinivasan. Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
inferno-chromium
approved these changes
Sep 14, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm fine with prohibiting binary executables, but
the rationale for doing this was completely unclear.
This commit rewrites the rationale to explain, in hopefully
a better way, why they can be a problem.
I prefer "executable" over "binary".
On digital computers, all data (including source code) are binaries :-).
In addition, some executables are simultaneously executables
and source code, e.g., shell scripts.
So I think what is meant here is a "generated binary".
I don't really think this merits a "High" level, but that's
a different dicussion.
Signed-off-by: David A. Wheeler dwheeler@dwheeler.com
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behavior? (You can also link to an open issue here)
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
Other information: