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

Feat test report #1244

Draft
wants to merge 40 commits into
base: main
Choose a base branch
from
Draft

Feat test report #1244

wants to merge 40 commits into from

Conversation

timhuynh94
Copy link
Contributor

No description provided.

TimHuynh and others added 30 commits December 12, 2024 09:41
* refactor(pipeline): use server API types for pipeline and migrate compiler types

* gci

* feat: add sender rule for pipelines

---------

Co-authored-by: David May <[email protected]>
* chore(lint): address existing linter issues

* remove dupl from exclusion list
* enhance(build): add fork field for OIDC

* fix test

* integration test update
* enhance(yaml): allow for users to parse pipelines using old library

* testing file for internal yaml

* chore(compiler): convert unmarshaled buildkite to go-yaml

* remove tests used in later PRs

* lintfix

* fix schema

* gci
* init commit

* feat(repo): add pending approval timeout

* fix test

* remove dead code

---------

Co-authored-by: David May <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

golangci

🚫 [golangci] reported by reviewdog 🐶
unnecessary trailing newline (whitespace)


🚫 [golangci] reported by reviewdog 🐶
unnecessary trailing newline (whitespace)


🚫 [golangci] reported by reviewdog 🐶
expressions should not be cuddled with blocks (wsl)

l.Debugf("bucket name: %s", input.BucketName)


🚫 [golangci] reported by reviewdog 🐶
return statements should not be cuddled if block has more than two lines (wsl)


🚫 [golangci] reported by reviewdog 🐶
return statements should not be cuddled if block has more than two lines (wsl)


🚫 [golangci] reported by reviewdog 🐶
return statements should not be cuddled if block has more than two lines (wsl)


🚫 [golangci] reported by reviewdog 🐶
assignments should only be cuddled with other assignments (wsl)

err = storage.FromGinContext(c).Download(ctx, input)


🚫 [golangci] reported by reviewdog 🐶
only one cuddle assignment allowed before if statement (wsl)

if err != nil {


🚫 [golangci] reported by reviewdog 🐶
if statements should only be cuddled with assignments (wsl)

if input.FilePath == "" {


🚫 [golangci] reported by reviewdog 🐶
if statements should only be cuddled with assignments (wsl)

if input.Bucket.BucketName == "" || input.ObjectName == "" {


🚫 [golangci] reported by reviewdog 🐶
assignments should only be cuddled with other assignments (wsl)

err = storage.FromGinContext(c).Upload(ctx, input)


🚫 [golangci] reported by reviewdog 🐶
only one cuddle assignment allowed before if statement (wsl)

if err != nil {


🚫 [golangci] reported by reviewdog 🐶
if statements should only be cuddled with assignments (wsl)

if input.Bucket.BucketName == "" || input.ObjectName == "" {


🚫 [golangci] reported by reviewdog 🐶
only one cuddle assignment allowed before if statement (wsl)

if err != nil {


🚫 [golangci] reported by reviewdog 🐶
assignments should only be cuddled with other assignments (wsl)


🚫 [golangci] reported by reviewdog 🐶
expressions should not be cuddled with blocks (wsl)

logrus.Debug("creating storage client from setup")


🚫 [golangci] reported by reviewdog 🐶
declarations should never be cuddled (wsl)

var opts minio.MakeBucketOptions


🚫 [golangci] reported by reviewdog 🐶
append only allowed to cuddle with appended value (wsl)

objects = append(objects, object.Key)


🚫 [golangci] reported by reviewdog 🐶
ranges should only be cuddled with assignments used in the iteration (wsl)

for object := range objectCh {


🚫 [golangci] reported by reviewdog 🐶
block should not end with a whitespace (or comment) (wsl)


🚫 [golangci] reported by reviewdog 🐶
block should not start with a whitespace (wsl)

func NewTest(endpoint, accessKey, secretKey string, secure bool) (*MinioClient, error) {


🚫 [golangci] reported by reviewdog 🐶
directive //nolint:revive // ignore returning unexported client is unused for linter "revive" (nolintlint)

//nolint:revive // ignore returning unexported client

func (t *TestCase) Duration() string {
// check if the test case doesn't have a started timestamp
if t.GetStarted() == 0 {
return "..."

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
string ... has 3 occurrences, make it a constant (goconst)


logrus.Debugf("getting object in bucket %s from path: %s", object.Bucket.BucketName, object.ObjectName)

logrus.Infof("%s to download", humanize.Bytes(uint64(objInfo.Size)))

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
G115: integer overflow conversion int64 -> uint64 (gosec)

return err
}

logrus.Infof("downloaded %s to %s on local filesystem", humanize.Bytes(uint64(stat.Size())), filename)

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
G115: integer overflow conversion int64 -> uint64 (gosec)

}

// MinioClient implements the Storage interface using MinIO.
type MinioClient struct {

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
exported: type name will be used as minio.MinioClient by other packages, and that stutters; consider calling this Client (revive)


// WithContext adds the minio Storage to the context
func WithContext(ctx context.Context, storage Storage) context.Context {
return context.WithValue(ctx, key, storage)

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
context-keys-type: should not use basic type string as key in context.WithValue (revive)

//
// When the provided TestSuite type is nil, it
// will set nothing and immediately return.
func (t *TestSuite) SetStartTs(v int64) {

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
ST1003: method SetStartTs should be SetStartTS (stylecheck)

//
// When the provided TestSuite type is nil, or the field within
// the type is nil, it returns the zero value for the field.
func (t *TestSuite) GetStartTs() int64 {

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
ST1003: method GetStartTs should be GetStartTS (stylecheck)

@@ -48,6 +48,7 @@ type (
Pull string `json:"pull,omitempty" yaml:"pull,omitempty"`
Ruleset Ruleset `json:"ruleset,omitempty" yaml:"ruleset,omitempty"`
Secrets StepSecretSlice `json:"secrets,omitempty" yaml:"secrets,omitempty"`
TestReport TestReport `json:"test_report,omitempty" yaml:"test_report,omitempty"`

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
tag is not aligned, should be: json:"test_report,omitempty" yaml:"test_report,omitempty" (tagalign)

//
// swagger:model PipelineTestReport
TestReport struct {
Results []string `yaml:"results,omitempty" json:"results,omitempty"`

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
tag is not aligned, should be: yaml:"results,omitempty" json:"results,omitempty" (tagalign)


return New(_setup)

}

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
unnecessary trailing newline (whitespace)

return err
}

stat, err := os.Stat(object.FilePath)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.

Copilot Autofix AI 6 days ago

To fix the problem, we need to validate the object.FilePath to ensure it does not contain any path traversal characters or sequences that could lead to accessing unintended files or directories. We can achieve this by checking for the presence of path separators (/ or \) and parent directory references (..). If any of these are found, we should reject the input.

The best way to fix the problem without changing existing functionality is to add a validation step in the DownloadObject function in api/admin/storage.go before calling the Download method. This ensures that the object.FilePath is safe to use.

Suggested changeset 1
api/admin/storage.go
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/api/admin/storage.go b/api/admin/storage.go
--- a/api/admin/storage.go
+++ b/api/admin/storage.go
@@ -394,2 +394,8 @@
 	}
+	// Validate the file path to prevent path traversal
+	if strings.Contains(input.FilePath, "/") || strings.Contains(input.FilePath, "\\") || strings.Contains(input.FilePath, "..") {
+		retErr := fmt.Errorf("invalid file path")
+		util.HandleError(c, http.StatusBadRequest, retErr)
+		return
+	}
 	err = storage.FromGinContext(c).Download(ctx, input)
EOF
@@ -394,2 +394,8 @@
}
// Validate the file path to prevent path traversal
if strings.Contains(input.FilePath, "/") || strings.Contains(input.FilePath, "\\") || strings.Contains(input.FilePath, "..") {
retErr := fmt.Errorf("invalid file path")
util.HandleError(c, http.StatusBadRequest, retErr)
return
}
err = storage.FromGinContext(c).Download(ctx, input)
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

golangci

🚫 [golangci] reported by reviewdog 🐶
directive //nolint:revive // ignore returning unexported client is unused for linter "revive" (nolintlint)

//nolint:revive // ignore returning unexported client

@@ -0,0 +1,29 @@
package pipeline

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
Missed header for check (goheader)

package storage

import (
"context"

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
File is not properly formatted (gci)

package storage

import (
"context"

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
File is not properly formatted (gci)

package storage

import (
"fmt"

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
File is not properly formatted (gci)

return nil, fmt.Errorf("invalid storage driver provided: %s", s.Driver)
}

}

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
unnecessary trailing newline (whitespace)

return
}
err = storage.FromGinContext(c).Upload(ctx, input)
if err != nil {

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
only one cuddle assignment allowed before if statement (wsl)


return
}
if input.Bucket.BucketName == "" || input.ObjectName == "" {

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
if statements should only be cuddled with assignments (wsl)

util.HandleError(c, http.StatusBadRequest, retErr)
return
}
if input.FilePath == "" {

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
if statements should only be cuddled with assignments (wsl)


return
}
if input.Bucket.BucketName == "" || input.ObjectName == "" {

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
if statements should only be cuddled with assignments (wsl)


return
}
l.Debugf("bucket name: %s", input.BucketName)

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
expressions should not be cuddled with blocks (wsl)

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.

4 participants