…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 <diogoteles@google.com>
Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* Merge pull request ossf#3 from joycebrum/feat/connect-patch-generator-with-remediation-output

Include the generated patch in the output

Signed-off-by: Joyce Brum <joycebrum@google.com>
Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* Merge pull request ossf#2 from joycebrum/test/initial-tests-for-dw-fix

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* Merge pull request ossf#4 from joycebrum/feat/get-input-needed-to-generate-patch

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* impl.go: slight refactor to loop

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* Add envvars to existing or new env, still not replaced in `run`

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* 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 <pnacht@google.com>

* 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 <pnacht@google.com>

* 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 <pnacht@google.com>

* 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 <pnacht@google.com>

* 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 <pnacht@google.com>

* Test for same injection in same step, leading to duplicate findings

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* Use existing envvars with different name but same meaning

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* Avoid conflicts with irrelevant but existing envvars

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* Use first job's indent to define envvar indent

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* 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 <pnacht@google.com>

* 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 <pnacht@google.com>

* Fix panic in hasScriptInjection test due to missing file

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* Avoid duplicate envvars dealing with array variables

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* Adopt existing inter-block spacing for new env

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* chore: Tidy up function order, remove unused files

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* Define localPath in runScorecard

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* Assert valid offset, use TrimSpace, drop unused struct member

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* Just use []bytes instead of string

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* Use []byte, not string

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* go mod tidy updates

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* Ensure valid offset

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* Move /patch to /internal/patch

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* Document patch behavior and add patch to remediation in def.yml

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* Updates from review

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

* Add patch to finding before adding to list of findings

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>

---------

Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>
Signed-off-by: Joyce Brum <joycebrum@google.com>
Co-authored-by: Diogo Teles Sant'Anna <diogoteles@google.com>
Co-authored-by: Joyce Brum <joycebrum@google.com>