Add diff logic and parallel logger support for audit #642

eyalk007 · 2026-01-13T10:42:28Z

The pull request is targeting the dev branch.
The code has been validated to compile successfully by running go vet ./....
The code has been formatted properly using go fmt ./....
All static analysis checks passed.
All tests have passed. If this feature is not already covered by the tests, new tests have been added.
Updated the Contributing page / ReadMe page / CI Workflow files if needed.
All changes are detailed at the description. if not already covered at JFrog Documentation, new documentation have been added.

Depends on:

Add goroutine-local logger and BufferedLogger for parallel operations jfrog-client-go#1297

…Results) - based on v1.24.2

- Add branchdiff.go with RunBranchDiffAudit function - Scans target branch first, then source branch (sequential) - SCA diff handled internally by CLI - JAS diff handled by CompareJasResults - Add MergeStatusCodes for partial results filtering - Status codes merged (worst of target + source) for frogbot filtering

- Add logger field to AuditBasicParams with getter/setter - RunAudit swaps global logger if custom logger provided - Enables frogbot to capture logs per-scan for ordered output

- Add LogCollector that captures logs in isolated buffer per audit - Enables parallel audits without log interleaving - Uses goroutine-local logger from jfrog-client-go - Propagate logger to child goroutines in JAS runner and SCA scan - Remove unused diff completion log message

eyalk007 · 2026-01-13T10:59:48Z

utils/results/diff.go

+)
+
+// UnifyScaAndJasResults merges SCA and JAS diff results into a single SecurityCommandResults.
+func UnifyScaAndJasResults(scaResults, jasDiffResults *SecurityCommandResults) *SecurityCommandResults {


myabe needs to be moved to frogbot considering the specific use
and selected parallel audit implementation, your call @attiasas

Mergeing results could stay here if the logic is adjusted to be more general like MergeCommandResults

utils/results/diff.go

…eIncludedBuilds)

utils/results/diff.go

attiasas

Nice work! checkout my comments, also:

Update the PR description with the changes
Adjust tests base on changes

attiasas · 2026-01-18T07:40:01Z

commands/audit/logcollector.go

@@ -0,0 +1,41 @@
+package audit


maybe this should be at jfrog/jfrog-client-go#1297?

these are the options

Move LogCollector to client-go - but it's really just a wrapper
Delete LogCollector entirely - Frogbot can use BufferedLogger directly from client-go

or we Keep it as is

attiasas · 2026-01-18T07:40:36Z

go.mod


 // replace github.com/jfrog/froggit-go => github.com/jfrog/froggit-go master
+
+replace github.com/jfrog/jfrog-client-go => github.com/eyalk007/jfrog-client-go v0.0.0-20260114112951-67b77f49255f


Reminder to remove replace after merging dependend PR

tests/testdata/other/diff-scan/target.sarif

attiasas · 2026-01-18T07:47:12Z

sca/scan/scascan.go

+	currentLogger := log.GetLogger()
+	scaTask := createScaScanTaskWithRunner(params.Runner, strategy, params)
+	wrappedScaTask := func(threadId int) error {
+		log.SetLoggerForGoroutine(currentLogger)
+		defer log.ClearLoggerForGoroutine()
+		return scaTask(threadId)
+	}


why not change in createScaScanTaskWithRunner directly?

I don't get the chnage (looking only here) you call log.GetLogger and setting it for the routine but at the end you clear it? code is not clear why you do it. (looks like you are just setting default log to be default and clear it).

I suggest improving this part or at least adding comment about this

attiasas · 2026-01-18T07:48:23Z

jas/runner/jasrunner.go

 func addModuleJasScanTask(scanType jasutils.JasScanType, securityParallelRunner *utils.SecurityParallelRunner, task parallel.TaskFunc, scanResults *results.TargetResults, allowSkippingErrors bool) (generalError error) {
 	securityParallelRunner.JasScannersWg.Add(1)
-	if _, addTaskErr := securityParallelRunner.Runner.AddTaskWithError(task, func(err error) {
+	currentLogger := log.GetLogger()


same comment as in scascan.
If this logic is used multiple times (same code), try to create static method to be used in multiple places

utils/results/diff.go

attiasas · 2026-01-18T08:52:33Z

utils/results/diff.go

+)
+
+// UnifyScaAndJasResults merges SCA and JAS diff results into a single SecurityCommandResults.
+func UnifyScaAndJasResults(scaResults, jasDiffResults *SecurityCommandResults) *SecurityCommandResults {


Mergeing results could stay here if the logic is adjusted to be more general like MergeCommandResults

attiasas · 2026-01-18T08:53:51Z

utils/results/diff.go

+
+// CompareJasResults computes the diff between target and source JAS results.
+// Returns only NEW findings in source that don't exist in target.
+func CompareJasResults(targetResults, sourceResults *SecurityCommandResults) *SecurityCommandResults {


IDK about the func name, it does not compare, it filters Source results that exists at target

attiasas · 2026-01-18T08:55:32Z

utils/results/diff.go

+	diffResults := &SecurityCommandResults{
+		ResultsMetaData: ResultsMetaData{
+			EntitledForJas:   sourceResults.EntitledForJas,
+			SecretValidation: sourceResults.SecretValidation,
+			CmdType:          sourceResults.CmdType,
+			XrayVersion:      sourceResults.XrayVersion,
+			XscVersion:       sourceResults.XscVersion,
+			MultiScanId:      sourceResults.MultiScanId,
+			StartTime:        sourceResults.StartTime,
+			ResultContext:    sourceResults.ResultContext,
+		},
+	}


Suggested change

diffResults := &SecurityCommandResults{

ResultsMetaData: ResultsMetaData{

EntitledForJas: sourceResults.EntitledForJas,

SecretValidation: sourceResults.SecretValidation,

CmdType: sourceResults.CmdType,

XrayVersion: sourceResults.XrayVersion,

XscVersion: sourceResults.XscVersion,

MultiScanId: sourceResults.MultiScanId,

StartTime: sourceResults.StartTime,

ResultContext: sourceResults.ResultContext,

},

}

diffResults := &SecurityCommandResults{

ResultsMetaData: sourceResults.ResultsMetaData,

},

}

why not just setting the exact same struct (will help to maintain and be sure all attributes are copied). something. is stopping us from doing that?

attiasas · 2026-01-18T09:00:30Z

utils/results/diff.go

+func extractLocationsOnly(run *sarif.Run, targetKeys map[string]bool) {
+	for _, result := range run.Results {
+		for _, location := range result.Locations {
+			key := getRelativeLocationFileName(location, run.Invocations) + getLocationSnippetText(location)


what about file name changes? it will not catch this...
why are we first filtering only locations and than by fingerprints, can we do it in one go? (reducing go over results multile times)

For SAST: Uses fingerprints (precise_sink_and_sink_function) which are content-based, so renames don't matter

for secrets and iac -
This is a known limitation - AM doesn't handle file renames either. If you rename a file, existing findings in that file will show as "new" in the diff. This is acceptable

We can write a todo and create a ticket for it, but i dont believe we should handle it now

eyalk007 added 6 commits January 5, 2026 15:07

Add parallel PR scanning functions (CompareJasResults, UnifyScaAndJas…

18c22a1

…Results) - based on v1.24.2

Add Logger field to AuditBasicParams for parallel scan log separation

caf59eb

- Add logger field to AuditBasicParams with getter/setter - RunAudit swaps global logger if custom logger provided - Enables frogbot to capture logs per-scan for ordered output

Update go.mod/go.sum

96ae51a

Update LogCollector with ReplayTo for proper log formatting

b22d7c4

eyalk007 self-assigned this Jan 13, 2026

eyalk007 requested a deployment to frogbot January 13, 2026 10:42 — with GitHub Actions Waiting

eyalk007 added the improvement Automatically generated release notes label Jan 13, 2026

Simplify comments to match repo style

16f7f21

eyalk007 requested a deployment to frogbot January 13, 2026 10:44 — with GitHub Actions Waiting

Remove accidentally added test file

c38b6c3

eyalk007 requested a deployment to frogbot January 13, 2026 10:45 — with GitHub Actions Waiting

eyalk007 added 2 commits January 13, 2026 12:52

Remove unused branchdiff.go

5e4a231

Clean up diff functions: remove AM references, simplify comments

86e70fd

eyalk007 requested a deployment to frogbot January 13, 2026 10:54 — with GitHub Actions Waiting

Move diff functions to separate file: utils/results/diff.go

37f3743

eyalk007 requested a deployment to frogbot January 13, 2026 10:58 — with GitHub Actions Waiting

eyalk007 commented Jan 13, 2026

View reviewed changes

utils/results/diff.go Outdated Show resolved Hide resolved

Remove extra comments from logCollector field

b0eb167

eyalk007 requested a deployment to frogbot January 13, 2026 11:03 — with GitHub Actions Waiting

Add diff function tests adapted from analyzer-manager

957b363

eyalk007 requested a deployment to frogbot January 13, 2026 11:14 — with GitHub Actions Waiting

Merge upstream/dev - resolve conflicts (keep both logCollector and us…

809a52f

…eIncludedBuilds)

eyalk007 requested a deployment to frogbot January 13, 2026 11:17 — with GitHub Actions Waiting

eyalk007 added the safe to test Approve running integration tests on a pull request label Jan 13, 2026

github-actions bot removed the safe to test Approve running integration tests on a pull request label Jan 13, 2026

Remove unused MergeStatusCodes function and tests

a5897a9

eyalk007 requested a deployment to frogbot January 14, 2026 08:17 — with GitHub Actions Waiting

Fix UnifyScaAndJasResults to preserve Applicability from SCA scan

22f7713

eyalk007 requested a deployment to frogbot January 14, 2026 08:39 — with GitHub Actions Waiting

Fix: Copy GitContext in UnifyScaAndJasResults for proper upload paths

56609db

eyalk007 requested a deployment to frogbot January 14, 2026 10:30 — with GitHub Actions Waiting

CR fixes - update go.mod replace directive to use git commit

816605d

eyalk007 commented Jan 15, 2026

View reviewed changes

utils/results/diff.go Outdated Show resolved Hide resolved

eyalk007 requested a deployment to frogbot January 15, 2026 15:26 — with GitHub Actions Waiting

attiasas requested changes Jan 18, 2026

View reviewed changes

CR fixes: extract logger helper, use sarifutils, rename functions

1ca4a0a

eyalk007 requested a deployment to frogbot January 18, 2026 10:19 — with GitHub Actions Waiting


		// replace github.com/jfrog/froggit-go => github.com/jfrog/froggit-go master

		replace github.com/jfrog/jfrog-client-go => github.com/eyalk007/jfrog-client-go v0.0.0-20260114112951-67b77f49255f

Add diff logic and parallel logger support for audit #642

Are you sure you want to change the base?

Add diff logic and parallel logger support for audit #642

Conversation

eyalk007 commented Jan 13, 2026 • edited by attiasas Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eyalk007 Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

attiasas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

eyalk007 commented Jan 13, 2026 •

edited by attiasas

Loading

eyalk007 Jan 13, 2026 •

edited

Loading