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

fixing lock logic #4

Merged
merged 1 commit into from
Oct 12, 2020
Merged

fixing lock logic #4

merged 1 commit into from
Oct 12, 2020

Conversation

kimsterv
Copy link
Contributor

No description provided.

@dlorenc dlorenc merged commit 812b3f4 into ossf:main Oct 12, 2020
pnacht pushed a commit to pnacht/scorecard that referenced this pull request Mar 8, 2024
…erate-patch

feat: configure input for patch generation
pnacht added a commit to pnacht/scorecard that referenced this pull request Jun 29, 2024
…erate-patch

Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
pnacht pushed a commit to pnacht/scorecard that referenced this pull request Jun 29, 2024
…erate-patch

Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
pnacht pushed a commit to pnacht/scorecard that referenced this pull request Jun 29, 2024
…erate-patch

Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
pnacht pushed a commit to pnacht/scorecard that referenced this pull request Jun 29, 2024
…erate-patch

Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
pnacht pushed a commit to pnacht/scorecard that referenced this pull request Jun 29, 2024
…erate-patch

Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
pnacht pushed a commit to pnacht/scorecard that referenced this pull request Aug 29, 2024
…erate-patch

Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
pnacht pushed a commit to pnacht/scorecard that referenced this pull request Oct 1, 2024
…erate-patch

Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
pnacht pushed a commit to pnacht/scorecard that referenced this pull request Oct 1, 2024
…erate-patch

Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
spencerschrock pushed a commit that referenced this pull request Nov 10, 2024
…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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants