Add Dependency Review and Submission #21

aegilops · 2025-12-04T17:54:39Z

Adds branch scanning, dependency submission, and more robustness.

fetching SBOM diffs for non-default branches
automatic dependency snapshot submission using Component Detection
better CLI options and documentation
improved advisory sync reliability
support for org listing with Enterprise Server

…ew and submission

Copilot

Pull request overview

Copilot reviewed 15 out of 18 changed files in this pull request and generated 15 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/sbomCollector.ts

src/componentSubmission.ts

src/sbomCollector.ts

src/componentSubmission.ts

src/componentDetection.ts

src/cli.ts

src/sbomCollector.ts

src/componentSubmission.ts

src/cli.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…ity/github-sbom-toolkit into dependency-review

Copilot

Pull request overview

Copilot reviewed 15 out of 18 changed files in this pull request and generated 12 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/componentDetection.ts

src/sbomCollector.ts

src/componentSubmission.ts

src/types.ts

src/componentSubmission.ts

src/sbomCollector.ts

Copilot · 2025-12-08T18:15:55Z

src/sbomCollector.ts

+                  console.debug(chalk.blue(`Force-submission enabled: submitting component snapshot for ${fullName} branch ${b.name}...`));
+                  if (await submitSnapshotIfPossible({ octokit: this.octokit, owner: org, repo: repo.name, branch: b.name, languages: this.opts.submitLanguages, quiet: this.opts.quiet, componentDetectionBinPath: this.opts.componentDetectionBinPath })) {
+                    // brief delay to allow snapshot ingestion
+                    await new Promise(r => setTimeout(r, 1500));


The magic number 1500 (milliseconds) for the delay after snapshot submission is hardcoded. This delay allows time for snapshot ingestion before retrying the dependency review diff. Consider:

Extracting this as a named constant (e.g., SNAPSHOT_INGESTION_DELAY_MS) to make its purpose clear

Making it configurable through options if different GitHub instances have different ingestion times

Adding a comment explaining why this specific duration was chosen

The same applies to the 3000ms delay on line 539.

@copilot open a new pull request to apply changes based on this feedback

src/sbomCollector.ts

src/componentDetection.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot · 2025-12-08T18:21:14Z

@aegilops I've opened a new pull request, #28, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot

Pull request overview

Copilot reviewed 15 out of 18 changed files in this pull request and generated 12 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/componentDetection.ts

src/serialization.ts

src/malwareAdvisories.ts

src/types.ts

src/sbomCollector.ts

src/componentSubmission.ts

src/sbomCollector.ts

src/componentSubmission.ts

src/componentDetection.ts

Co-authored-by: aegilops <41705651+aegilops@users.noreply.github.com>

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Co-authored-by: aegilops <41705651+aegilops@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 15 out of 18 changed files in this pull request and generated 10 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/sbomCollector.ts

Copilot · 2025-12-08T18:38:22Z

CHANGELOG.md

@@ -0,0 +1,32 @@
+# Changelog
+
+## [2025-12-04] – 0.2.0 - Branch scanning and dependency submission


[nitpick] The CHANGELOG.md file shows a date of [2025-12-04] on line 3, but the current date is December 8, 2025. This appears to be a date in the past (4 days ago). While this could be intentional if the work was completed then, typically changelogs reflect the actual release date. Verify this is the intended date.

Suggested change

## [2025-12-04] – 0.2.0 - Branch scanning and dependency submission

## [2025-12-08] – 0.2.0 - Branch scanning and dependency submission

src/sbomCollector.ts

src/componentDetection.ts

src/cli.ts

Copilot · 2025-12-08T18:38:23Z

src/malwareMatcher.ts

+          // Some ecosystems require a namespace (e.g., Maven, NuGet)
+          const ecosystemsWithNamespace = new Set(['maven', 'nuget', 'composer', 'golang']);
+          if (ecosystemsWithNamespace.has(change.ecosystem) && change.namespace) {
+            p = `pkg:${change.ecosystem}/${change.namespace}/${change.name}${change.version ? '@' + change.version : ''}`;
+          } else {
+            p = `pkg:${change.ecosystem}/${change.name}${change.version ? '@' + change.version : ''}`;
+          }
+


In the malwareMatcher.ts file, lines 162-169 appear to be orphaned code that doesn't belong to any function. These lines seem to be constructing a purl for Maven/NuGet ecosystems with namespaces, but they are placed outside of any function context. This code should either be removed or properly integrated into the enumerateDiffPackages function where it logically belongs.

Copilot · 2025-12-08T18:38:24Z

src/malwareMatcher.ts

+          // Some ecosystems require a namespace (e.g., Maven, NuGet)
+          const ecosystemsWithNamespace = new Set(['maven', 'nuget', 'composer', 'golang']);
+          if (ecosystemsWithNamespace.has(change.ecosystem) && change.namespace) {
+            p = `pkg:${change.ecosystem}/${change.namespace}/${change.name}${change.version ? '@' + change.version : ''}`;
+          } else {
+            p = `pkg:${change.ecosystem}/${change.name}${change.version ? '@' + change.version : ''}`;
+          }


The enumerateBasePackages function is annotated to return packages with __branch property (line 150), but the code at line 161 doesn't actually set this property on any returned packages. The comment at line 161 mentions "Annotate with default branch for reporting (if known)" but no annotation code follows. Either the code to set __branch is missing, or the return type annotation should not include __branch for base packages.

Suggested change

// Some ecosystems require a namespace (e.g., Maven, NuGet)

const ecosystemsWithNamespace = new Set(['maven', 'nuget', 'composer', 'golang']);

if (ecosystemsWithNamespace.has(change.ecosystem) && change.namespace) {

p = `pkg:${change.ecosystem}/${change.namespace}/${change.name}${change.version ? '@' + change.version : ''}`;

} else {

p = `pkg:${change.ecosystem}/${change.name}${change.version ? '@' + change.version : ''}`;

}

const branch = (repo as any).defaultBranch || undefined;

return list.map(pkg => ({

...pkg,

__branch: branch

}));

Copilot · 2025-12-08T18:38:24Z

src/sbomCollector.ts

+// Move applyQueries to module scope
+function applyQueries(
+  candidatePurls: string[],
+  queries: ParsedQuery[],
+  found: Map<string, string>,
+  branchTag?: string,
+  fallbackVersion?: string
+) {
+  const unique = Array.from(new Set(candidatePurls));
+  for (const p of unique) {
+    const pLower = p.toLowerCase();
+    const outKey = branchTag ? `${p}@${branchTag}` : p;
+    for (const q of queries) {
+      if (q.isPrefixWildcard) {
+        const prefix = q.lower.slice(0, -1);
+        if (pLower.startsWith(prefix)) { if (!found.has(outKey)) found.set(outKey, q.raw); }
+        continue;
+      }
+      if (q.versionConstraint && q.type && q.name) {
+        if (!pLower.startsWith("pkg:")) continue;
+        const body = p.slice(4);
+        const atIdx = body.indexOf("@");
+        const main = atIdx >= 0 ? body.slice(0, atIdx) : body;
+        const ver = atIdx >= 0 ? body.slice(atIdx + 1) : fallbackVersion;
+        const slashIdx = main.indexOf("/");
+        if (slashIdx < 0) continue;
+        const pType = main.slice(0, slashIdx).toLowerCase();
+        const pName = main.slice(slashIdx + 1);
+        if (pType === q.type && pName.toLowerCase() === q.name.toLowerCase() && ver) {
+          try {
+            const coerced = semver.coerce(ver)?.version || ver;
+            if (semver.valid(coerced) && semver.satisfies(coerced, q.versionConstraint, { includePrerelease: true })) {
+              if (!found.has(outKey)) found.set(outKey, q.raw);
+            }
+          } catch { /* ignore */ }
+        }
+      } else if (q.exact) {
+        if (pLower === q.exact) { if (!found.has(outKey)) found.set(outKey, q.raw); }
+      }
+    }
+  }
+}


The applyQueries function at line 595-635 in sbomCollector.ts is declared at module scope (outside the class), which breaks the expected encapsulation. This function appears to be extracted from inside the searchByPurlsWithReasons method but is placed incorrectly. The comment at line 594 says "// Move applyQueries to module scope" which suggests this was intentional, but it's placed in the middle of the class definition rather than before or after it, making the code structure confusing and potentially breaking the class context.

src/sbomCollector.ts

src/componentSubmission.ts

Co-authored-by: aegilops <41705651+aegilops@users.noreply.github.com>

…fa4-4def-4364-b631-731c97106a16 Make snapshot ingestion delays configurable

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 15 out of 18 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-09T09:27:40Z

src/sbomCollector.ts

    }

-    const orgs = this.opts.org ? [this.opts.org] : await this.listEnterpriseOrgs(this.opts.enterprise!);
+    const orgs = this.opts.org ? [this.opts.org] : this.opts.enterprise ? await this.listEnterpriseOrgs(this.opts.enterprise, this.opts.ghes) : [this.opts.repo.split("/")[0]];


At line 168, when this.opts.repo is specified, the code attempts to access this.opts.repo.split("/")[0] without first checking if this.opts.repo is defined. While the constructor validates the repo format (lines 54-61), TypeScript may still treat this.opts.repo as potentially undefined here. The code should handle the case where repo is undefined more explicitly, or the ternary expression should be restructured to avoid this potential runtime error.

Suggested change

const orgs = this.opts.org ? [this.opts.org] : this.opts.enterprise ? await this.listEnterpriseOrgs(this.opts.enterprise, this.opts.ghes) : [this.opts.repo.split("/")[0]];

const orgs = this.opts.org

? [this.opts.org]

: this.opts.enterprise

? await this.listEnterpriseOrgs(this.opts.enterprise, this.opts.ghes)

: this.opts.repo

? [this.opts.repo.split("/")[0]]

: (() => { throw new Error("No org, enterprise, or repo specified in options."); })();

Copilot · 2025-12-09T09:27:41Z

src/sbomCollector.ts

+      totalRepos = 1;
+      const [org, repoName] = this.opts.repo.split("/");
+      orgRepoMap[org] = [await this.getRepo(org, repoName)];
+      this.summary.orgs = orgs;


Line 187 sets this.summary.orgs = orgs redundantly since orgs was already assigned to this.summary.orgs at line 169. This duplicate assignment serves no purpose and should be removed.

Suggested change

this.summary.orgs = orgs;

Copilot · 2025-12-09T09:27:41Z

src/sbomCollector.ts

+// Move applyQueries to module scope
+function applyQueries(
+  candidatePurls: string[],
+  queries: ParsedQuery[],
+  found: Map<string, string>,
+  branchTag?: string,
+  fallbackVersion?: string
+) {
+  const unique = Array.from(new Set(candidatePurls));
+  for (const p of unique) {
+    const pLower = p.toLowerCase();
+    const outKey = branchTag ? `${p}@${branchTag}` : p;
+    for (const q of queries) {
+      if (q.isPrefixWildcard) {
+        const prefix = q.lower.slice(0, -1);
+        if (pLower.startsWith(prefix)) { if (!found.has(outKey)) found.set(outKey, q.raw); }
+        continue;
+      }
+      if (q.versionConstraint && q.type && q.name) {
+        if (!pLower.startsWith("pkg:")) continue;
+        const body = p.slice(4);
+        const atIdx = body.indexOf("@");
+        const main = atIdx >= 0 ? body.slice(0, atIdx) : body;
+        const ver = atIdx >= 0 ? body.slice(atIdx + 1) : fallbackVersion;
+        const slashIdx = main.indexOf("/");
+        if (slashIdx < 0) continue;
+        const pType = main.slice(0, slashIdx).toLowerCase();
+        const pName = main.slice(slashIdx + 1);
+        if (pType === q.type && pName.toLowerCase() === q.name.toLowerCase() && ver) {
+          try {
+            const coerced = semver.coerce(ver)?.version || ver;
+            if (semver.valid(coerced) && semver.satisfies(coerced, q.versionConstraint, { includePrerelease: true })) {
+              if (!found.has(outKey)) found.set(outKey, q.raw);
+            }
+          } catch { /* ignore */ }
+        }
+      } else if (q.exact) {
+        if (pLower === q.exact) { if (!found.has(outKey)) found.set(outKey, q.raw); }
+      }
+    }
+  }
+}


The applyQueries function is defined inside the searchByPurlsWithReasons method but the comment on line 601 says "Move applyQueries to module scope". This function is currently nested within the method, making it inaccessible to other methods and created anew on each call. The function should be moved to module scope (outside the class) as the comment indicates, both for reusability and performance.

Copilot · 2025-12-09T09:27:41Z

src/sbomCollector.ts

+            console.debug((e as Error).message);
+          }
+
+          if (sbom.error) this.summary.failedCount++; else this.summary.successCount++;


The success/fail counting logic at line 344 is only executed when this.opts.includeBranches && sbom && sbom.sbom is true (inside the branch scanning block), but it should be executed for all SBOM fetches. When branch scanning is disabled or conditions aren't met, newly fetched SBOMs won't be counted in the summary statistics. This logic should be moved outside the branch scanning conditional block (after line 350) so it runs for all newly fetched SBOMs.

See below for a potential fix:

// Write freshly fetched SBOM immediately if a cache directory is configured if (this.opts.loadFromDir && this.opts.syncSboms && this.opts.loadFromDir.length) { try { writeOne(sbom, { outDir: this.opts.loadFromDir }); } catch { /* ignore write errors */ } } } if (sbom) { if (sbom.error) this.summary.failedCount++; else this.summary.successCount++;

Copilot · 2025-12-09T09:27:42Z

src/cli.ts

+  if (debug) {
+    console.debug(chalk.blue("Debug logging enabled"));
+  } else {
+    console.debug = () => { };
+  }
+


Overriding console.debug globally (line 99) to disable debug logging can cause issues if other libraries or parts of the code rely on console.debug for their own purposes. A better approach would be to use a custom logger function or wrap console.debug in a conditional check throughout the codebase.

Suggested change

if (debug) {

console.debug(chalk.blue("Debug logging enabled"));

} else {

console.debug = () => { };

}

// Custom debug logger to avoid overriding console.debug globally

const logDebug = (...args: any[]) => {

if (debug) {

console.debug(chalk.blue(...args));

}

};

logDebug("Debug logging enabled");

aegilops added 28 commits November 26, 2025 15:49

Added using Dependency Review on branches

c7899b5

Added Dependency Submission with Component Detection

d2a6b9f

Added language choice to submission

ec32784

Debug of submission, partial progress, but not working yet

5043ccb

Single repo scanning added. More scaffolding added to enable Dep Revi…

a729fb3

…ew and submission

Runs component-detection now, but is killed on MacOS

506b9a2

Allow using component-detection at a provided path

68f5d22

Refactor, make branch submission independent of freshly fetching SBOM

51a111e

Removed unused function

6f43ed0

Works for branch changes

c002406

Formatting

4787c2e

Updated malware matching to do diffs

290c070

Updated README

e55933e

Removed default branch limit

3e94895

Update to better handling rate limiting when querying malware database

5364da0

Allows just malware sync/caching

00116c5

Fix single repo behaviour on first pass

5c92626

Fixed missing diff from skipped branches

9f284c2

Allow forcing submission with Component Detection

f6d37c2

Fixed not waiting for process

33415a9

Now runs Component Detection correctly, on correct path

3e65ebc

Fixed mistake reading wrong path in Component Detection

6314b14

Fixed missing graphs by normalising appropriately

99b20d9

CHANGELOG

3a89bfb

Fixed submission by fixing calling of git to capture STDOUT properly

1887425

Update docs

9ea2fa0

Update GHES URL for listing orgs

7f13c04

CHANGELOG

dab9c61

Copilot AI review requested due to automatic review settings December 4, 2025 17:54

Copilot started reviewing on behalf of aegilops December 4, 2025 17:55 View session

Copilot AI review requested due to automatic review settings December 8, 2025 17:52

Copilot started reviewing on behalf of aegilops December 8, 2025 17:52 View session

Formatted

e3aaf6f

Copilot AI reviewed Dec 8, 2025

View reviewed changes

Update src/sbomCollector.ts

4dcf2b3

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings December 8, 2025 18:07

Copilot started reviewing on behalf of aegilops December 8, 2025 18:08 View session

Merge branch 'dependency-review' of https://github.com/advanced-secur…

5ac59f7

…ity/github-sbom-toolkit into dependency-review

Copilot AI reviewed Dec 8, 2025

View reviewed changes

Update src/componentDetection.ts

ba8ff05

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings December 8, 2025 18:16

Copilot started reviewing on behalf of aegilops December 8, 2025 18:17 View session

aegilops and others added 2 commits December 8, 2025 18:18

Update src/sbomCollector.ts

5431556

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Initial plan

4ad422a

Copilot AI mentioned this pull request Dec 8, 2025

Make snapshot ingestion delays configurable #28

Merged

Merge branch 'main' into dependency-review

a1e931f

Copilot AI reviewed Dec 8, 2025

View reviewed changes

Copilot AI and others added 2 commits December 8, 2025 18:29

Make snapshot ingestion delays configurable

395c925

Co-authored-by: aegilops <41705651+aegilops@users.noreply.github.com>

Update src/malwareMatcher.ts

4b2e75c

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings December 8, 2025 18:30

Copilot started reviewing on behalf of aegilops December 8, 2025 18:30 View session

Remove redundant 'ms' from log message

3d5f9ad

Co-authored-by: aegilops <41705651+aegilops@users.noreply.github.com>

Copilot AI reviewed Dec 8, 2025

View reviewed changes

Copilot AI and others added 3 commits December 8, 2025 18:43

Add CLI arguments for snapshot ingestion delays

e7f34a6

Co-authored-by: aegilops <41705651+aegilops@users.noreply.github.com>

Merge pull request #28 from advanced-security/copilot/sub-pr-21-a1e28…

de9be57

…fa4-4def-4364-b631-731c97106a16 Make snapshot ingestion delays configurable

Update src/componentDetection.ts

bd2e92e

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings December 9, 2025 09:20

Copilot started reviewing on behalf of aegilops December 9, 2025 09:20 View session

Copilot AI reviewed Dec 9, 2025

View reviewed changes

		@@ -0,0 +1,32 @@
		# Changelog

		## [2025-12-04] – 0.2.0 - Branch scanning and dependency submission

	## [2025-12-04] – 0.2.0 - Branch scanning and dependency submission
	## [2025-12-08] – 0.2.0 - Branch scanning and dependency submission

Add Dependency Review and Submission #21

Are you sure you want to change the base?

Add Dependency Review and Submission #21

Conversation

aegilops commented Dec 4, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

aegilops Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Dec 8, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview