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

BUG: docker-compose file with YAML directive breaking #2853

Closed
gabibguti opened this issue Apr 11, 2023 · 3 comments
Closed

BUG: docker-compose file with YAML directive breaking #2853

gabibguti opened this issue Apr 11, 2023 · 3 comments
Labels

Comments

@gabibguti
Copy link
Contributor

Describe the bug
A docker-compose file with YAML directive seems to break the Pinned-Dependencies check.

Reproduction steps
Steps to reproduce the behavior:

  1. Run Scorecard @4.10.2 for github.com/grpc/grpc: scorecard --repo=github.com/grpc/grpc
  2. View that Pinned-Dependencies check ends in:
internal error: error parsing shell code: https://github.com/ossf/scorecard/blob/main/docs/checks.md#pinned-dependencies
templates/examples/php/echo/base.Dockerfile.template:1:48
parameter expansion requires a literal
  1. The problematic file/line is https://github.com/grpc/grpc/blob/master/templates/examples/php/echo/base.Dockerfile.template#L1

Expected behavior
I expected Scorecard to be able to parse docker-compose files with YAML directive.

Additional context
None.

@gabibguti gabibguti added the kind/bug Something isn't working label Apr 11, 2023
@spencerschrock
Copy link
Contributor

I'm guessing it's coming from here:

if err != nil {
// Note: this is caught by internal caller and only printed
// to avoid failing on shell scripts that our parser does not understand.
// Example: https://github.com/openssl/openssl/blob/master/util/shlib_wrap.sh.in
return sce.WithMessage(sce.ErrorShellParsing, err.Error())
}

Based on the comment, I'm guessing we aren't ignoring the error everywhere we call it. But I haven't tracked it down.

@spencerschrock
Copy link
Contributor

So we used to catch it here:

func validateShellFile(pathfn string, content []byte, dl checker.DetailLogger) (bool, error) {
files := make(map[string]bool)
r, err := validateShellFileAndRecord(pathfn, content, files, dl)
if err != nil {
if errors.Is(err, errInternalInvalidShellCode) {
// Discard and print this particular error for now.
dl.Debug(err.Error())
} else {
return r, err
}
}
return r, nil
}

Until it was removed by this large PR #1932

@laurentsimon any idea if this was an intentional removal?

@spencerschrock
Copy link
Contributor

Crashing the whole Pinned-Dependencies check shouldn't be an issue anymore after #3515.
There's still a processing error, which gets logged, but this doesn't seem like a normal dockerfile so happy to close this one out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants