jfrog-cli icon indicating copy to clipboard operation
jfrog-cli copied to clipboard

JGC-429 - Bump go version

Open ehl-jf opened this issue 2 months ago • 3 comments

  • [ ] All tests have passed. If this feature is not already covered by the tests, new tests have been added.
  • [ ] The pull request is targeting the master branch.
  • [ ] The code has been validated to compile successfully by running go vet ./....
  • [ ] The code has been formatted properly using go fmt ./....

ehl-jf avatar Nov 06 '25 14:11 ehl-jf

I see you updated the GHA from https://github.com/jfrog/.github but when I discussed about this issue with @RemiBou, we said we should probably remove these GHA and let each project define its configuration for Go, Linters, etc.

cyrilc-pro avatar Nov 06 '25 14:11 cyrilc-pro

I see you updated the GHA from https://github.com/jfrog/.github but when I discussed about this issue with @RemiBou, we said we should probably remove these GHA and let each project define its configuration for Go, Linters, etc.

For now I've just follow the current bump go documentation.

I think is not a bad idea that every one use the same go version since are tightly interconnected for now

ehl-jf avatar Nov 06 '25 14:11 ehl-jf

🚨 Frogbot scanned this pull request and found the below:

📗 Scan Summary

  • Frogbot scanned for vulnerabilities and found 1 issues
Scan Category Status Security Issues
Software Composition Analysis ✅ Done Not Found
Contextual Analysis ✅ Done -
Static Application Security Testing (SAST) ✅ Done
1 Issues Found 1 High
Secrets ✅ Done -
Infrastructure as Code (IaC) ✅ Done Not Found

github-actions[bot] avatar Nov 07 '25 09:11 github-actions[bot]

body

at utils/cliutils/utils.go (line 646)

🎯 Static Application Security Testing (SAST) Vulnerability

Severity Finding
high
High
Deserializing untrusted data without validation
Full description

Vulnerability Details

Rule ID: go-unsafe-deserialization

Overview

Unsafe deserialization in Go occurs when a program deserializes untrusted data with a potentially dangerous deserializer. Deserialization is the process of converting serialized data (data that has been converted into a format that can be easily transmitted or stored) back into its original form. In some ("unsafe") serialization protocols, if an attacker is able to manipulate the serialized data, they may be able to execute arbitrary code or perform other malicious actions when the data is deserialized.

Vulnerable example

import (
    "github.com/go-yaml/yaml"
    "net/http"
)

func storeHandler(w http.ResponseWriter, r *http.Request) {
    var data map[string]interface{}
    yaml.Unmarshal([]byte(r.URL.Query().Get("data")), &data) // NOT OK
}

This code uses yaml.Unmarshal to deserialize untrusted data from the user, with a potentially dangerous deserializer.

Remediation

import (
    "github.com/go-yaml/yaml"
    "net/http"
)

func storeHandler(w http.ResponseWriter, r *http.Request) {
    var data map[string]interface{}
    yaml.UnmarshalStrict([]byte(r.URL.Query().Get("data")), &data) // SAFE
}

Using yaml.UnmarshalStrict solves the problem by ensuring a safe serialization protocol.

Code Flows
Vulnerable data flow analysis result

↘️ client.Do(req) (at utils/cliutils/utils.go line 652)

↘️ resp (at utils/cliutils/utils.go line 652)

↘️ resp (at utils/cliutils/utils.go line 661)

↘️ resp.Body (at utils/cliutils/utils.go line 661)

↘️ io.ReadAll(resp.Body) (at utils/cliutils/utils.go line 661)

↘️ body (at utils/cliutils/utils.go line 661)

↘️ func doHttpRequest(client *http.Client, req *http.Request) (resp *http.Response, body []byte, err error) { req.Close = true resp, err = client.Do(req) if errorutils.CheckError(err) != nil { return } defer func() { if resp != nil && resp.Body != nil { err = errors.Join(err, errorutils.CheckError(resp.Body.Close())) } }() body, err = io.ReadAll(resp.Body) return resp, body, errorutils.CheckError(err) } (at utils/cliutils/utils.go line 650)

↘️ (resp *http.Response, body []byte, err error) (at utils/cliutils/utils.go line 650)

↘️ doHttpRequest(client, req) (at utils/cliutils/utils.go line 633)

↘️ resp (at utils/cliutils/utils.go line 633)

↘️ body (at utils/cliutils/utils.go line 633)

↘️ body (at utils/cliutils/utils.go line 646)




github-actions[bot] avatar Nov 07 '25 09:11 github-actions[bot]

I see you updated the GHA from https://github.com/jfrog/.github but when I discussed about this issue with @RemiBou, we said we should probably remove these GHA and let each project define its configuration for Go, Linters, etc.

For now I've just follow the current bump go documentation.

I think is not a bad idea that every one use the same go version since are tightly interconnected for now

I think we should remove this version from here, or make it configurable when using the step : the tooling version should sit next to the code in the repo, or else we can have issue like non reproductible builds for patch or conflicts. We'll do it in another piece of work

RemiBou avatar Nov 07 '25 12:11 RemiBou