Handle Unsupported Binary Artifacts check #4
Conversation
This file contains hidden or 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
New functionality added as part of the PR ossf#2039 is not supported for local repositories. When this code path is hit, it will check if it is an unsupported error and not fail the Binary Artifacts check. Fallback to existing behavior for any other type of errors
abhiseksanyal
deleted the
feature/handle-unsupported-binary-artifacts-check
branch
abhiseksanyal
added a commit
that referenced
this pull request
Jul 6, 2023
New functionality added as part of the PR ossf#2039 is not supported for local repositories. When this code path is hit, it will check if it is an unsupported error and not fail the Binary Artifacts check. Fallback to existing behavior for any other type of errors Co-authored-by: Abhisek Sanyal <[email protected]>
abhiseksanyal
added a commit
that referenced
this pull request
Jul 6, 2023
New functionality added as part of the PR ossf#2039 is not supported for local repositories. When this code path is hit, it will check if it is an unsupported error and not fail the Binary Artifacts check. Fallback to existing behavior for any other type of errors
abhiseksanyal
added a commit
that referenced
this pull request
Aug 22, 2023
New functionality added as part of the PR ossf#2039 is not supported for local repositories. When this code path is hit, it will check if it is an unsupported error and not fail the Binary Artifacts check. Fallback to existing behavior for any other type of errors
|
This change is not required anymore, as support for this has been added in the main fork via PR ossf#3415 |
abhiseksanyal
pushed a commit
that referenced
this pull request
Mar 20, 2025
…ssf#4218) * Merge pull request #1 from joycebrum/feature/setup-environment-for-dw-fix create environment for patch on DW script injections Signed-off-by: Diogo Teles Sant'Anna <[email protected]> Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Merge pull request #3 from joycebrum/feat/connect-patch-generator-with-remediation-output Include the generated patch in the output Signed-off-by: Joyce Brum <[email protected]> Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Merge pull request #2 from joycebrum/test/initial-tests-for-dw-fix Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Merge pull request #4 from joycebrum/feat/get-input-needed-to-generate-patch Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * impl.go: slight refactor to loop Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Add envvars to existing or new env, still not replaced in `run` Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Replace unsafe variables in run commands, generate git diff Git diff created using hexops/gotextdiff, WHICH IS ARCHIVED. It is unfortunately the only package I found which could do it. To be discussed with Scorecard maintainers whether it's worth it. Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Rewrite test file - Test patchWorkflow instead of GeneratePatch. This avoids the complication of comparing diff files; we can instead simply compare the output workflow to an expected "fixed" workflow. - Examples with multiple findings must have separate "fixed" workflows for each finding, not a single file which covers all findings - Instead of hard-coding the finding details (snippet, line position), run raw.DangerousWorkflow() to get that data automatically. This does make these tests a bit more "integration-test-like", but makes them substantially easier to maintain. Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Rewrite patch/impl.go - misc refactors - use go-git to generate diff - Most functions now return errors instead of bools. This can be later used for simpler logging - Existing environment variables are now detected by parsing the files as GH workflows. This is WIP to handle existing envvars in our patches. - Remove instances of C-style for-loops, unnecessarily dangerous! - Fixed proper detection of existing env, handling blank lines and comments. Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Update test workflows - Fix inconsistencies between original and "fixed" versions - Store multiple "fixed" workflows for tests with multiple findings. Each "fixed" workflow fixes a single finding. The files are numbered according to the order in which the findings are found by moving down the file. - allKindsOfUserInput removed. Would require too many "fixed" workflows to test. The behavior can be tested more directly. Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Use existing envvars, validate patched workflow - If an envvar with our name and value already existed but simply wasn't used, the patch no longer duplicates it. - After the patched workflow is created, we validate that it is valid. Or, at least did not introduce any syntax errors that were not present in the original workflow. Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Test for same injection in same step, leading to duplicate findings Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Use existing envvars with different name but same meaning Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Avoid conflicts with irrelevant but existing envvars Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Use first job's indent to define envvar indent Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Refactor patch/impl_test - Create helper function `readWorkflow` - Improved error handling in case of failed workflow validation - Allow the declaration of duplicate findings (cases where 2+ findings have the same patch) Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * patch/impl: Simplify unsafePatterns, use errors, docs, lint - Simplify use of unsafePatterns - Replaced boolean returns with errors, for easier log/debugging - Improved documentation - Changes to satisfy linter, adoption of 120-char line limit Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Fix panic in hasScriptInjection test due to missing file Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Avoid duplicate envvars dealing with array variables Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Adopt existing inter-block spacing for new env Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * chore: Tidy up function order, remove unused files Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Define localPath in runScorecard Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Assert valid offset, use TrimSpace, drop unused struct member Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Just use []bytes instead of string Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Use []byte, not string Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * go mod tidy updates Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Ensure valid offset Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Move /patch to /internal/patch Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Document patch behavior and add patch to remediation in def.yml Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Updates from review Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> * Add patch to finding before adding to list of findings Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> --------- Signed-off-by: Diogo Teles Sant'Anna <[email protected]> Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]> Signed-off-by: Joyce Brum <[email protected]> Co-authored-by: Diogo Teles Sant'Anna <[email protected]> Co-authored-by: Joyce Brum <[email protected]>
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.
What kind of change does this PR introduce?
New functionality added as part of the PR ossf#2039 is not supported for local repositories. When this code path is hit, it will check if it is an unsupported error and not fail the Binary Artifacts check. Fallback to existing behavior for any other type of errors
(Is it a bug fix, feature, docs update, something else?)
What is the current behavior?
When unsupported code path is hit in Binary Artifacts check, it generates an internal error and scorecard eventually exits with an error code of 1
What is the new behavior (if this is a feature change)?**
When unsupported code path is hit in Binary Artifacts check, it will not generate an internal error and scorecard will exit with a zero error code. For any other type of errors, it will fall back to existing behavior
Which issue(s) this PR fixes
Special notes for your reviewer
Does this PR introduce a user-facing change?
For user-facing changes, please add a concise, human-readable release note to
the
release-note(In particular, describe what changes users might need to make in their
application as a result of this pull request.)