From 6001110f5fd2e25f7f065997f8e7c0de6907f831 Mon Sep 17 00:00:00 2001 From: Bassel Mbariky Date: Thu, 28 Aug 2025 15:33:52 +0300 Subject: [PATCH 1/4] Add curation client header to all requests --- commands/curation/curationaudit.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/commands/curation/curationaudit.go b/commands/curation/curationaudit.go index 77d3a0fc2..769a06b93 100644 --- a/commands/curation/curationaudit.go +++ b/commands/curation/curationaudit.go @@ -78,6 +78,10 @@ const ( MinArtiNuGetSupport = "7.93.0" MinXrayPassThroughSupport = "3.92.0" MinArtiGradleGemSupport = "7.63.5" + + // Curation request headers + waiverHeader = "X-Artifactory-Curation-Request-Waiver" + waiverHeaderValue = "syn_JFrog-Curation-Client" ) var CurationOutputFormats = []string{string(outFormat.Table), string(outFormat.Json)} @@ -490,6 +494,8 @@ func (ca *CurationAuditCommand) auditTree(tech techutils.Technology, results map parallelRequests: ca.parallelRequests, downloadUrls: depTreeResult.DownloadUrls, } + // Add curation client header to all requests + analyzer.httpClientDetails.Headers[waiverHeader] = waiverHeaderValue rootNodes := map[string]struct{}{} for _, tree := range depTreeResult.FullDepTrees { @@ -571,7 +577,7 @@ func (ca *CurationAuditCommand) sendWaiverRequests(pkgs []*PackageStatus, msg st return nil, err } clientDetails := rtAuth.CreateHttpClientDetails() - clientDetails.Headers["X-Artifactory-Curation-Request-Waiver"] = msg + clientDetails.Headers[waiverHeader] = msg for _, pkg := range pkgs { response, body, _, err := rtManager.Client().SendGet(pkg.BlockedPackageUrl, true, &clientDetails) if err != nil { @@ -857,7 +863,7 @@ func (nc *treeAnalyzer) fetchNodeStatus(node xrayUtils.GraphNode, p *sync.Map) e // We try to collect curation details from GET response after HEAD request got forbidden status code. func (nc *treeAnalyzer) getBlockedPackageDetails(packageUrl string, name string, version string) (*PackageStatus, error) { - nc.httpClientDetails.Headers["X-Artifactory-Curation-Request-Waiver"] = "syn" + nc.httpClientDetails.Headers[waiverHeader] = waiverHeaderValue getResp, respBody, _, err := nc.rtManager.Client().SendGet(packageUrl, true, &nc.httpClientDetails) if err != nil { if getResp == nil { From 79582579c183845446cad5a132debc242dfe40ef Mon Sep 17 00:00:00 2001 From: Bassel Mbariky Date: Thu, 28 Aug 2025 15:42:08 +0300 Subject: [PATCH 2/4] added-unit-test --- commands/curation/curationaudit_test.go | 32 +++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/commands/curation/curationaudit_test.go b/commands/curation/curationaudit_test.go index 86ed6d6d2..221032984 100644 --- a/commands/curation/curationaudit_test.go +++ b/commands/curation/curationaudit_test.go @@ -484,6 +484,38 @@ func createTempHomeDirWithConfig(t *testing.T, basePathToTests string, testCase } } +func TestCurationHeaders(t *testing.T) { + // Test that a request actually includes the header + ca := &CurationAuditCommand{} + + // Mock server to capture the request headers + var capturedHeaders http.Header + testHandler := func(w http.ResponseWriter, r *http.Request) { + capturedHeaders = r.Header + w.WriteHeader(http.StatusForbidden) + _, err := w.Write([]byte(`{"errors":[{"status":403,"message":"waiver-id|forbidden"}]}`)) + assert.NoError(t, err) + } + + mockServer, mockServerDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, testHandler) + defer mockServer.Close() + + // Create test package + pkg := &PackageStatus{ + BlockedPackageUrl: mockServerDetails.GetArtifactoryUrl() + "/test/pkg", + PackageName: "test-pkg", + PackageVersion: "1.0.0", + } + + // Make the request - this should include our header + _, err := ca.sendWaiverRequests([]*PackageStatus{pkg}, "test waiver", mockServerDetails) + assert.NoError(t, err) + + // Verify the header was sent + assert.NotNil(t, capturedHeaders) + assert.Equal(t, "test waiver", capturedHeaders["X-Artifactory-Curation-Request-Waiver"][0]) +} + func setCurationFlagsForTest(t *testing.T) func() { callbackCurationFlag := clienttestutils.SetEnvWithCallbackAndAssert(t, utils.CurationSupportFlag, "true") // Golang option to disable the use of the checksum database From d55c4f6b0cf02992f59f2069c00e39c6d1e2161b Mon Sep 17 00:00:00 2001 From: Bassel Mbariky Date: Fri, 5 Sep 2025 02:32:05 +0300 Subject: [PATCH 3/4] added_new_header --- commands/curation/curationaudit.go | 82 +++++++++++++++++++++++-- commands/curation/curationaudit_test.go | 44 +++++++------ 2 files changed, 103 insertions(+), 23 deletions(-) diff --git a/commands/curation/curationaudit.go b/commands/curation/curationaudit.go index 769a06b93..192bc2bcb 100644 --- a/commands/curation/curationaudit.go +++ b/commands/curation/curationaudit.go @@ -13,6 +13,7 @@ import ( "strings" "sync" + "github.com/google/uuid" "github.com/jfrog/gofrog/datastructures" "github.com/jfrog/gofrog/parallel" rtUtils "github.com/jfrog/jfrog-cli-core/v2/artifactory/utils" @@ -80,8 +81,33 @@ const ( MinArtiGradleGemSupport = "7.63.5" // Curation request headers - waiverHeader = "X-Artifactory-Curation-Request-Waiver" - waiverHeaderValue = "syn_JFrog-Curation-Client" + waiverHeader = "X-Artifactory-Curation-Request-Waiver" +) + +// generateWaiverHeaderValue creates a header value with a unique batch ID +func generateWaiverHeaderValue() string { + batchID := fmt.Sprintf("batch_%s", uuid.New().String()[:8]) + return fmt.Sprintf("syn_JFrog-Curation-Client_%s", batchID) +} + +// createRequestDetails creates a copy of httpClientDetails for thread-safe header modification +func (nc *treeAnalyzer) createRequestDetails() httputils.HttpClientDetails { + requestDetails := nc.httpClientDetails + if requestDetails.Headers == nil { + requestDetails.Headers = make(map[string]string) + } else { + // Create a copy of the headers map + requestDetails.Headers = make(map[string]string) + for k, v := range nc.httpClientDetails.Headers { + requestDetails.Headers[k] = v + } + } + return requestDetails +} + +var ( + waiverHeaderValue string + blockedPackageUrl string ) var CurationOutputFormats = []string{string(outFormat.Table), string(outFormat.Json)} @@ -494,6 +520,10 @@ func (ca *CurationAuditCommand) auditTree(tech techutils.Technology, results map parallelRequests: ca.parallelRequests, downloadUrls: depTreeResult.DownloadUrls, } + // Initialize the waiver header value with batch ID from Xray API + waiverHeaderValue = generateWaiverHeaderValue() + blockedPackageUrl = "" + // Add curation client header to all requests analyzer.httpClientDetails.Headers[waiverHeader] = waiverHeaderValue @@ -515,6 +545,7 @@ func (ca *CurationAuditCommand) auditTree(tech techutils.Technology, results map // We subtract 1 because the root node is not a package. totalNumberOfPackages: len(depTreeResult.FlatTree.Nodes) - 1, } + return err } @@ -794,12 +825,14 @@ func (nc *treeAnalyzer) fetchNodesStatus(graph *xrayUtils.GraphNode, p *sync.Map var multiErrors error consumerProducer := parallel.NewBounedRunner(nc.parallelRequests, false) errorsQueue := clientutils.NewErrorsQueue(1) + go func() { defer consumerProducer.Done() for _, node := range graph.Nodes { if _, ok := rootNodeIds[node.Id]; ok { continue } + getTask := func(node xrayUtils.GraphNode) func(threadId int) error { return func(threadId int) (err error) { return nc.fetchNodeStatus(node, p) @@ -814,6 +847,15 @@ func (nc *treeAnalyzer) fetchNodesStatus(graph *xrayUtils.GraphNode, p *sync.Map if err := errorsQueue.GetError(); err != nil { multiErrors = errors.Join(err, multiErrors) } + + // After all parallel processing is done, send the completed request + // Only send completed request if there are blocked packages + if blockedPackageUrl != "" { + if err := nc.sendCompletedRequest(blockedPackageUrl); err != nil { + multiErrors = errors.Join(err, multiErrors) + } + } + return multiErrors } @@ -825,8 +867,15 @@ func (nc *treeAnalyzer) fetchNodeStatus(node xrayUtils.GraphNode, p *sync.Map) e if scope != "" { name = scope + "/" + name } + for _, packageUrl := range packageUrls { - resp, _, err := nc.rtManager.Client().SendHead(packageUrl, &nc.httpClientDetails) + // Create a copy of httpClientDetails for this request to avoid modifying the shared one + requestDetails := nc.createRequestDetails() + + // Set the header for regular HEAD request + requestDetails.Headers[waiverHeader] = waiverHeaderValue + + resp, _, err := nc.rtManager.Client().SendHead(packageUrl, &requestDetails) if err != nil { if resp != nil && resp.StatusCode >= 400 { return errorutils.CheckErrorf(errorTemplateHeadRequest, packageUrl, name, version, resp.StatusCode, err) @@ -843,6 +892,11 @@ func (nc *treeAnalyzer) fetchNodeStatus(node xrayUtils.GraphNode, p *sync.Map) e return errorutils.CheckErrorf(errorTemplateHeadRequest, packageUrl, name, version, resp.StatusCode, err) } if resp.StatusCode == http.StatusForbidden { + // Track this blocked package URL (only need one for completed request) + if blockedPackageUrl == "" { + blockedPackageUrl = packageUrl + } + pkStatus, err := nc.getBlockedPackageDetails(packageUrl, name, version) if err != nil { return err @@ -863,8 +917,13 @@ func (nc *treeAnalyzer) fetchNodeStatus(node xrayUtils.GraphNode, p *sync.Map) e // We try to collect curation details from GET response after HEAD request got forbidden status code. func (nc *treeAnalyzer) getBlockedPackageDetails(packageUrl string, name string, version string) (*PackageStatus, error) { - nc.httpClientDetails.Headers[waiverHeader] = waiverHeaderValue - getResp, respBody, _, err := nc.rtManager.Client().SendGet(packageUrl, true, &nc.httpClientDetails) + // Create a copy of httpClientDetails for this request to avoid modifying the shared one + requestDetails := nc.createRequestDetails() + + // Set the header for regular GET request + requestDetails.Headers[waiverHeader] = waiverHeaderValue + + getResp, respBody, _, err := nc.rtManager.Client().SendGet(packageUrl, true, &requestDetails) if err != nil { if getResp == nil { return nil, err @@ -904,6 +963,19 @@ func (nc *treeAnalyzer) getBlockedPackageDetails(packageUrl string, name string, return nil, nil } +// sendCompletedRequest sends a final request with the _completed header +func (nc *treeAnalyzer) sendCompletedRequest(packageUrl string) error { + // Create a copy of httpClientDetails for this request + requestDetails := nc.createRequestDetails() + + // Set the header with _completed suffix + requestDetails.Headers[waiverHeader] = waiverHeaderValue + "_completed" + + // Send a HEAD request with the completed header + _, _, err := nc.rtManager.Client().SendHead(packageUrl, &requestDetails) + return err +} + // Return policies and conditions names from the FORBIDDEN HTTP error message. // Message structure: Package %s:%s download was blocked by JFrog Packages Curation service due to the following policies violated {%s, %s, %s, %s},{%s, %s, %s, %s}. func (nc *treeAnalyzer) extractPoliciesFromMsg(respError *ErrorsResp) []Policy { diff --git a/commands/curation/curationaudit_test.go b/commands/curation/curationaudit_test.go index 221032984..0b06ff3f5 100644 --- a/commands/curation/curationaudit_test.go +++ b/commands/curation/curationaudit_test.go @@ -20,6 +20,7 @@ import ( biutils "github.com/jfrog/build-info-go/utils" "github.com/jfrog/gofrog/datastructures" + rtUtils "github.com/jfrog/jfrog-cli-core/v2/artifactory/utils" coreCommonTests "github.com/jfrog/jfrog-cli-core/v2/common/tests" "github.com/jfrog/jfrog-cli-core/v2/utils/config" "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" @@ -485,35 +486,42 @@ func createTempHomeDirWithConfig(t *testing.T, basePathToTests string, testCase } func TestCurationHeaders(t *testing.T) { - // Test that a request actually includes the header - ca := &CurationAuditCommand{} + // Test that batch ID generation works correctly + headerValue := generateWaiverHeaderValue() + assert.True(t, strings.HasPrefix(headerValue, "syn_JFrog-Curation-Client_batch_")) + assert.Len(t, strings.Split(headerValue, "_"), 4) // syn_JFrog-Curation-Client_batch_ - // Mock server to capture the request headers - var capturedHeaders http.Header + // Test that completed request adds "_completed" suffix + var capturedHeader http.Header testHandler := func(w http.ResponseWriter, r *http.Request) { - capturedHeaders = r.Header - w.WriteHeader(http.StatusForbidden) - _, err := w.Write([]byte(`{"errors":[{"status":403,"message":"waiver-id|forbidden"}]}`)) - assert.NoError(t, err) + capturedHeader = r.Header.Clone() + w.WriteHeader(http.StatusOK) } mockServer, mockServerDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, testHandler) defer mockServer.Close() - // Create test package - pkg := &PackageStatus{ - BlockedPackageUrl: mockServerDetails.GetArtifactoryUrl() + "/test/pkg", - PackageName: "test-pkg", - PackageVersion: "1.0.0", + // Create analyzer and send completed request + rtAuth, err := mockServerDetails.CreateArtAuthConfig() + require.NoError(t, err) + rtManager, err := rtUtils.CreateServiceManager(mockServerDetails, 2, 0, false) + require.NoError(t, err) + + analyzer := &treeAnalyzer{ + rtManager: rtManager, + httpClientDetails: rtAuth.CreateHttpClientDetails(), } - // Make the request - this should include our header - _, err := ca.sendWaiverRequests([]*PackageStatus{pkg}, "test waiver", mockServerDetails) + // Use real header format like: syn_JFrog-Curation-Client_batch_a3404ff3 + waiverHeaderValue = "syn_JFrog-Curation-Client_batch_a3404ff3" + testUrl := mockServerDetails.GetArtifactoryUrl() + "/test/pkg" + err = analyzer.sendCompletedRequest(testUrl) assert.NoError(t, err) - // Verify the header was sent - assert.NotNil(t, capturedHeaders) - assert.Equal(t, "test waiver", capturedHeaders["X-Artifactory-Curation-Request-Waiver"][0]) + // Verify the header has "_completed" suffix + assert.NotNil(t, capturedHeader) + waiverHeader := capturedHeader["X-Artifactory-Curation-Request-Waiver"] + assert.Equal(t, "syn_JFrog-Curation-Client_batch_a3404ff3_completed", waiverHeader[0]) } func setCurationFlagsForTest(t *testing.T) func() { From 79c0938a202d7501b20fad49abf3f996397f2f08 Mon Sep 17 00:00:00 2001 From: Bassel Mbariky Date: Thu, 18 Sep 2025 14:54:38 +0300 Subject: [PATCH 4/4] Added-json-header --- commands/curation/curationaudit.go | 42 +++++++++----- commands/curation/curationaudit_test.go | 76 +++++++++++++++++++++---- 2 files changed, 94 insertions(+), 24 deletions(-) diff --git a/commands/curation/curationaudit.go b/commands/curation/curationaudit.go index 192bc2bcb..6a64169dc 100644 --- a/commands/curation/curationaudit.go +++ b/commands/curation/curationaudit.go @@ -84,10 +84,28 @@ const ( waiverHeader = "X-Artifactory-Curation-Request-Waiver" ) -// generateWaiverHeaderValue creates a header value with a unique batch ID func generateWaiverHeaderValue() string { - batchID := fmt.Sprintf("batch_%s", uuid.New().String()[:8]) - return fmt.Sprintf("syn_JFrog-Curation-Client_%s", batchID) + batchID := uuid.New().String() + headerData := map[string]string{ + "batch_id": batchID, + } + jsonData, _ := json.Marshal(headerData) + return string(jsonData) +} + +// addFieldToWaiverHeader adds a field to the existing waiver header JSON and returns the updated JSON string +func addFieldToWaiverHeader(fieldName string, fieldValue interface{}) (string, error) { + // Parse the existing header + var headerData map[string]interface{} + if err := json.Unmarshal([]byte(waiverHeaderValue), &headerData); err != nil { + return "", fmt.Errorf("failed to parse waiver header: %v", err) + } + headerData[fieldName] = fieldValue + updatedHeaderData, err := json.Marshal(headerData) + if err != nil { + return "", fmt.Errorf("failed to marshal updated header: %v", err) + } + return string(updatedHeaderData), nil } // createRequestDetails creates a copy of httpClientDetails for thread-safe header modification @@ -919,9 +937,7 @@ func (nc *treeAnalyzer) fetchNodeStatus(node xrayUtils.GraphNode, p *sync.Map) e func (nc *treeAnalyzer) getBlockedPackageDetails(packageUrl string, name string, version string) (*PackageStatus, error) { // Create a copy of httpClientDetails for this request to avoid modifying the shared one requestDetails := nc.createRequestDetails() - - // Set the header for regular GET request - requestDetails.Headers[waiverHeader] = waiverHeaderValue + requestDetails.Headers[waiverHeader] = "syn" getResp, respBody, _, err := nc.rtManager.Client().SendGet(packageUrl, true, &requestDetails) if err != nil { @@ -963,16 +979,14 @@ func (nc *treeAnalyzer) getBlockedPackageDetails(packageUrl string, name string, return nil, nil } -// sendCompletedRequest sends a final request with the _completed header func (nc *treeAnalyzer) sendCompletedRequest(packageUrl string) error { - // Create a copy of httpClientDetails for this request requestDetails := nc.createRequestDetails() - - // Set the header with _completed suffix - requestDetails.Headers[waiverHeader] = waiverHeaderValue + "_completed" - - // Send a HEAD request with the completed header - _, _, err := nc.rtManager.Client().SendHead(packageUrl, &requestDetails) + completedHeaderData, err := addFieldToWaiverHeader("completed", true) + if err != nil { + return err + } + requestDetails.Headers[waiverHeader] = completedHeaderData + _, _, err = nc.rtManager.Client().SendHead(packageUrl, &requestDetails) return err } diff --git a/commands/curation/curationaudit_test.go b/commands/curation/curationaudit_test.go index 0b06ff3f5..31f15c25e 100644 --- a/commands/curation/curationaudit_test.go +++ b/commands/curation/curationaudit_test.go @@ -488,10 +488,15 @@ func createTempHomeDirWithConfig(t *testing.T, basePathToTests string, testCase func TestCurationHeaders(t *testing.T) { // Test that batch ID generation works correctly headerValue := generateWaiverHeaderValue() - assert.True(t, strings.HasPrefix(headerValue, "syn_JFrog-Curation-Client_batch_")) - assert.Len(t, strings.Split(headerValue, "_"), 4) // syn_JFrog-Curation-Client_batch_ - // Test that completed request adds "_completed" suffix + // Verify it's valid JSON with batch_id + var headerData map[string]interface{} + err := json.Unmarshal([]byte(headerValue), &headerData) + require.NoError(t, err) + assert.Contains(t, headerData, "batch_id") + assert.NotEmpty(t, headerData["batch_id"]) + + // Test that completed request adds "completed" flag var capturedHeader http.Header testHandler := func(w http.ResponseWriter, r *http.Request) { capturedHeader = r.Header.Clone() @@ -506,22 +511,73 @@ func TestCurationHeaders(t *testing.T) { require.NoError(t, err) rtManager, err := rtUtils.CreateServiceManager(mockServerDetails, 2, 0, false) require.NoError(t, err) - analyzer := &treeAnalyzer{ rtManager: rtManager, httpClientDetails: rtAuth.CreateHttpClientDetails(), } - - // Use real header format like: syn_JFrog-Curation-Client_batch_a3404ff3 - waiverHeaderValue = "syn_JFrog-Curation-Client_batch_a3404ff3" + // Use JSON header format like: {"batch_id":"196c55b0-5724-4a94-aa45-9b0bfd658985"} + waiverHeaderValue = `{"batch_id":"196c55b0-5724-4a94-aa45-9b0bfd658985"}` testUrl := mockServerDetails.GetArtifactoryUrl() + "/test/pkg" err = analyzer.sendCompletedRequest(testUrl) assert.NoError(t, err) - - // Verify the header has "_completed" suffix assert.NotNil(t, capturedHeader) waiverHeader := capturedHeader["X-Artifactory-Curation-Request-Waiver"] - assert.Equal(t, "syn_JFrog-Curation-Client_batch_a3404ff3_completed", waiverHeader[0]) + + // Parse the completed header JSON + var completedHeaderData map[string]interface{} + err = json.Unmarshal([]byte(waiverHeader[0]), &completedHeaderData) + require.NoError(t, err) + assert.Equal(t, "196c55b0-5724-4a94-aa45-9b0bfd658985", completedHeaderData["batch_id"]) + assert.Equal(t, true, completedHeaderData["completed"]) +} + +func TestCurationSynHeader(t *testing.T) { + // Test that syn field is added correctly to the header + headerValue := generateWaiverHeaderValue() + + // Verify it's valid JSON with batch_id + var headerData map[string]interface{} + err := json.Unmarshal([]byte(headerValue), &headerData) + require.NoError(t, err) + assert.Contains(t, headerData, "batch_id") + assert.NotEmpty(t, headerData["batch_id"]) + + // Add syn field (simulating what happens in getBlockedPackageDetails) + headerData["syn"] = true + synHeaderData, err := json.Marshal(headerData) + require.NoError(t, err) + + // Verify the final JSON contains both batch_id and syn + var finalHeaderData map[string]interface{} + err = json.Unmarshal(synHeaderData, &finalHeaderData) + require.NoError(t, err) + assert.Equal(t, true, finalHeaderData["syn"]) + assert.Contains(t, finalHeaderData, "batch_id") +} + +func TestAddFieldToWaiverHeader(t *testing.T) { + // Test the helper function directly + waiverHeaderValue = `{"batch_id":"test-batch-123"}` + + // Add syn field + synHeader, err := addFieldToWaiverHeader("syn", true) + require.NoError(t, err) + + var synData map[string]interface{} + err = json.Unmarshal([]byte(synHeader), &synData) + require.NoError(t, err) + assert.Equal(t, "test-batch-123", synData["batch_id"]) + assert.Equal(t, true, synData["syn"]) + + // Add completed field + completedHeader, err := addFieldToWaiverHeader("completed", true) + require.NoError(t, err) + + var completedData map[string]interface{} + err = json.Unmarshal([]byte(completedHeader), &completedData) + require.NoError(t, err) + assert.Equal(t, "test-batch-123", completedData["batch_id"]) + assert.Equal(t, true, completedData["completed"]) } func setCurationFlagsForTest(t *testing.T) func() {