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

Recognize "403 forbidden" error from pip command output, and clasiffy it as "forbidden" error type #1225

Merged
merged 9 commits into from
Aug 20, 2024

Conversation

asafambar
Copy link
Contributor

@asafambar asafambar commented Aug 7, 2024

  • All tests passed. If this feature is not already covered by the tests, I added new tests.
  • All static analysis checks passed.
  • This pull request is on the dev branch.
  • I used gofmt for formatting the code before submitting the pull request.

Description:
After running "pip " and encountering an error, we examine the output from stderr. If the output includes a "403 Forbidden", we wrap the error with a specific error type for "Forbidden 403." This error type will be recognized later by post actions, such as curation.

@asafambar asafambar changed the base branch from master to dev August 7, 2024 14:47
@asafambar asafambar changed the title Add pip install stderr Recognize "403 forbidden" error from pip command output, and clasiffy it as "forbidden" error type Aug 13, 2024
Comment on lines 75 to 92
for k, v := range pc.GetEnv() {
if err := os.Setenv(k, v); err != nil {
return err
}
}

cmd := pc.GetCmd()
errBuffer := bytes.NewBuffer([]byte{})
multiWriter := io.MultiWriter(os.Stderr, errBuffer)
cmd.Stderr = multiWriter
cmd.Stdout = os.Stdout

err = cmd.Run()
if err != nil {
if buildInfoUtils.IsForbiddenOutput("pip", errBuffer.String()) {
err = errors.Join(err, buildInfoUtils.NewForbiddenError())
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The above code should be mioved into in the build-info-go module. If it isn't place there, this functionality will not work for Pythin when collecting build-info.
It is true that when not collecting build-info for python, the code flow ends here and doesn't continue into build-info-go, but having this code there, will allow you to invoke it from here as well.

Copy link
Contributor Author

@asafambar asafambar Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case we collect build info, the whole "install" execution have different logic which apparently adding the stderr to the err returned from the execution (which was missing in case we don't collect build-info), so in this case it will work in the flow of pip install + collecting dependencies for build-info, In security wrapper we recognize either a 403 in the error string OR forbidden type error.

I don't want to use the same logic to run installation command for the build info, as it can break existing behavior, which I avoided in this PRs.

@eyalbe4 eyalbe4 added the improvement Automatically generated release notes label Aug 20, 2024
@eyalbe4 eyalbe4 merged commit f90a225 into jfrog:dev Aug 20, 2024
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Automatically generated release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants