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 machine-readable patch to fix script injections in workflows #4218

Open
wants to merge 31 commits into
base: main
Choose a base branch
from

Commits on Oct 1, 2024

  1. 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]>
    diogoteles08 authored and pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    b4ec86d View commit details
    Browse the repository at this point in the history
  2. 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 <[email protected]>
    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    diogoteles08 authored and pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    5ee165c View commit details
    Browse the repository at this point in the history
  3. Merge pull request ossf#2 from joycebrum/test/initial-tests-for-dw-fix

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    diogoteles08 authored and pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    bcb159e View commit details
    Browse the repository at this point in the history
  4. Merge pull request ossf#4 from joycebrum/feat/get-input-needed-to-gen…

    …erate-patch
    
    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    joycebrum authored and pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    5bddd1a View commit details
    Browse the repository at this point in the history
  5. impl.go: slight refactor to loop

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    488d89a View commit details
    Browse the repository at this point in the history
  6. Add envvars to existing or new env, still not replaced in run

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    93c2fba View commit details
    Browse the repository at this point in the history
  7. 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]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    0394b86 View commit details
    Browse the repository at this point in the history
  8. 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]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    3c7f9c6 View commit details
    Browse the repository at this point in the history
  9. 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]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    b299a47 View commit details
    Browse the repository at this point in the history
  10. 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]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    9a5e043 View commit details
    Browse the repository at this point in the history
  11. 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]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    3f8e2af View commit details
    Browse the repository at this point in the history
  12. Test for same injection in same step, leading to duplicate findings

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    8b47fdd View commit details
    Browse the repository at this point in the history
  13. Use existing envvars with different name but same meaning

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    c632590 View commit details
    Browse the repository at this point in the history
  14. Avoid conflicts with irrelevant but existing envvars

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    5c986e8 View commit details
    Browse the repository at this point in the history
  15. Use first job's indent to define envvar indent

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    6534155 View commit details
    Browse the repository at this point in the history
  16. 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]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    bf26120 View commit details
    Browse the repository at this point in the history
  17. 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]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    31ea054 View commit details
    Browse the repository at this point in the history
  18. Fix panic in hasScriptInjection test due to missing file

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    e61d79a View commit details
    Browse the repository at this point in the history
  19. Avoid duplicate envvars dealing with array variables

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    bbe6c85 View commit details
    Browse the repository at this point in the history
  20. Adopt existing inter-block spacing for new env

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    09d4b47 View commit details
    Browse the repository at this point in the history
  21. chore: Tidy up function order, remove unused files

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    89b73a3 View commit details
    Browse the repository at this point in the history
  22. Define localPath in runScorecard

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    71d73a4 View commit details
    Browse the repository at this point in the history
  23. Assert valid offset, use TrimSpace, drop unused struct member

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    938a59c View commit details
    Browse the repository at this point in the history
  24. Just use []bytes instead of string

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    fa8e16b View commit details
    Browse the repository at this point in the history
  25. Use []byte, not string

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    42cf837 View commit details
    Browse the repository at this point in the history
  26. go mod tidy updates

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    10e6589 View commit details
    Browse the repository at this point in the history
  27. Ensure valid offset

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    fb31f93 View commit details
    Browse the repository at this point in the history
  28. Move /patch to /internal/patch

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    d6e4fd1 View commit details
    Browse the repository at this point in the history
  29. Document patch behavior and add patch to remediation in def.yml

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    5a7b390 View commit details
    Browse the repository at this point in the history
  30. Updates from review

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    pnacht committed Oct 1, 2024
    Configuration menu
    Copy the full SHA
    557a1b4 View commit details
    Browse the repository at this point in the history

Commits on Oct 3, 2024

  1. Add patch to finding before adding to list of findings

    Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
    pnacht committed Oct 3, 2024
    Configuration menu
    Copy the full SHA
    892c442 View commit details
    Browse the repository at this point in the history