Skip to content

Commit 6e688c9

Browse files
committed
Code review changes
1 parent 7be90d9 commit 6e688c9

File tree

4 files changed

+85
-39
lines changed

4 files changed

+85
-39
lines changed

src/cli.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ async function main() {
6969

7070
const token = argv.token as string | undefined || process.env.GITHUB_TOKEN;
7171

72-
if (argv.sbom || argv["sync-malware"] || argv.uploadSarif) {
72+
// Require a token for any network operation (syncing SBOMs, malware advisories, or SARIF upload)
73+
if (argv.syncSboms || argv["sync-malware"] || argv.uploadSarif) {
7374
if (!token) {
7475
console.error(chalk.red("GitHub token must be provided via --token or GITHUB_TOKEN environment variable"));
7576
process.exit(1);

src/malwareMatcher.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,14 +57,17 @@ function parsePurl(p: string): ParsedPurl | null {
5757

5858
function versionInRange(ecosystem: string, version: string | null, range: string | null): boolean {
5959
if (!version || !range) return false;
60-
// try semver logic on all ecosystems
6160
try {
62-
return semver.satisfies(semver.coerce(version) || version, range, { includePrerelease: true });
61+
const coerced = semver.coerce(version)?.version;
62+
if (coerced && semver.valid(coerced) && semver.validRange(range)) {
63+
return semver.satisfies(coerced, range, { includePrerelease: true });
64+
}
6365
} catch {
64-
// Fallback: simple equality if range is exact version (no operators)
65-
if (/^[0-9A-Za-z._-]+$/.test(range)) return version === range;
66-
return false;
66+
// ignore and fall through
6767
}
68+
// Fallback: if range looks like a plain version (no operators) do literal comparison
69+
if (/^[0-9A-Za-z._-]+$/.test(range)) return version === range;
70+
return false;
6871
}
6972

7073
export function matchMalware(advisories: MalwareAdvisoryNode[], sboms: RepositorySbom[]): MalwareMatch[] {

src/sbomCollector.ts

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export interface CollectorOptions {
2626
}
2727

2828
export class SbomCollector {
29-
private octokit; // typed by Octokit instance
29+
private octokit: ReturnType<typeof createOctokit> | undefined; // explicit type
3030
private opts: Required<CollectorOptions>;
3131
private sboms: RepositorySbom[] = [];
3232
private summary: CollectionSummary;
@@ -37,19 +37,24 @@ export class SbomCollector {
3737
if (!options.loadFromDir && !options.enterprise && !options.org) {
3838
throw new Error("Either enterprise/org or loadFromDir must be specified");
3939
}
40+
// Spread user options first then apply defaults via nullish coalescing so that
41+
// passing undefined does not erase defaults
42+
const o = { ...options };
4043
this.opts = {
41-
concurrency: 5,
42-
includePrivate: true,
43-
delayMsBetweenRepos: options.delayMsBetweenRepos ?? 5000,
44-
lightDelayMs: options.lightDelayMs ?? 500,
45-
baseUrl: options.baseUrl ?? undefined,
46-
autoEnableDependencyGraph: true,
47-
loadFromDir: options.loadFromDir ?? undefined,
48-
syncSboms: options.syncSboms ?? false,
49-
showProgressBar: options.showProgressBar ?? false,
50-
suppressSecondaryRateLimitLogs: options.suppressSecondaryRateLimitLogs ?? false,
51-
quiet: options.quiet ?? false,
52-
...options
44+
token: o.token,
45+
enterprise: o.enterprise,
46+
org: o.org,
47+
baseUrl: o.baseUrl,
48+
concurrency: o.concurrency ?? 5,
49+
includePrivate: o.includePrivate ?? true,
50+
delayMsBetweenRepos: o.delayMsBetweenRepos ?? 5000,
51+
lightDelayMs: o.lightDelayMs ?? 500,
52+
loadFromDir: o.loadFromDir,
53+
syncSboms: o.syncSboms ?? false,
54+
autoEnableDependencyGraph: o.autoEnableDependencyGraph ?? true,
55+
showProgressBar: o.showProgressBar ?? false,
56+
suppressSecondaryRateLimitLogs: o.suppressSecondaryRateLimitLogs ?? false,
57+
quiet: o.quiet ?? false
5358
} as Required<CollectorOptions>;
5459

5560
if (this.opts.token) {

src/serialization.ts

Lines changed: 57 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -25,32 +25,69 @@ export function writeOne(sbom: RepositorySbom, { outDir, flatten = false }: Seri
2525
fs.writeFileSync(filePath, JSON.stringify(sbom, null, 2), "utf8");
2626
}
2727

28-
export interface ReadOptions { flatten?: boolean }
28+
export interface ReadOptions {
29+
/** Treat directory as flattened (owner-repo.json). */
30+
flatten?: boolean;
31+
/** Log parse errors (filename + message). */
32+
logParseErrors?: boolean;
33+
/** Stop after reading this many valid SBOMs (safety for huge trees). */
34+
maxFiles?: number;
35+
}
2936

30-
export function readAll(dir: string, _opts: ReadOptions = {}): RepositorySbom[] {
37+
/**
38+
* Read all serialized SBOMs below a directory.
39+
* Modes:
40+
* - Hierarchical (default): recurse and only accept files named exactly `sbom.json` (tightened from previous any *.json behavior).
41+
* - Flattened: do not recurse; accept top-level *.json where filename (without extension) contains at least one dash (owner-repo convention).
42+
*/
43+
export function readAll(dir: string, opts: ReadOptions = {}): RepositorySbom[] {
44+
const { flatten = false, logParseErrors = false, maxFiles } = opts;
3145
const results: RepositorySbom[] = [];
3246
if (!fs.existsSync(dir)) throw new Error(`Directory not found: ${dir}`);
3347

34-
const visit = (current: string) => {
35-
const stat = fs.statSync(current);
36-
if (stat.isDirectory()) {
37-
const entries = fs.readdirSync(current);
38-
for (const e of entries) visit(path.join(current, e));
39-
} else if (stat.isFile()) {
40-
const base = path.basename(current);
41-
if (base === "sbom.json" || base.endsWith(".json")) {
42-
try {
43-
const raw = fs.readFileSync(current, "utf8");
44-
const obj = JSON.parse(raw);
45-
if (obj && obj.repo && Array.isArray(obj.packages)) {
46-
results.push(obj as RepositorySbom);
47-
}
48-
} catch (e) {
49-
// skip malformed file
50-
}
48+
const pushIfValid = (filePath: string) => {
49+
try {
50+
const raw = fs.readFileSync(filePath, "utf8");
51+
const obj = JSON.parse(raw);
52+
if (obj && obj.repo && Array.isArray(obj.packages)) {
53+
results.push(obj as RepositorySbom);
54+
}
55+
} catch (e) {
56+
if (logParseErrors) {
57+
// eslint-disable-next-line no-console
58+
console.warn(`Skipping malformed SBOM JSON ${filePath}: ${(e as Error).message}`);
5159
}
5260
}
5361
};
54-
visit(dir);
62+
63+
if (flatten) {
64+
const entries = fs.readdirSync(dir, { withFileTypes: true });
65+
for (const ent of entries) {
66+
if (!ent.isFile()) continue;
67+
if (!ent.name.endsWith('.json')) continue;
68+
// Require at least one dash to loosely match owner-repo.json (avoid pulling unrelated JSON)
69+
const baseName = ent.name.slice(0, -5); // remove .json
70+
if (!baseName.includes('-')) continue;
71+
pushIfValid(path.join(dir, ent.name));
72+
if (maxFiles && results.length >= maxFiles) break;
73+
}
74+
} else {
75+
const visit = (current: string) => {
76+
const stat = fs.statSync(current);
77+
if (stat.isDirectory()) {
78+
const entries = fs.readdirSync(current);
79+
for (const e of entries) {
80+
visit(path.join(current, e));
81+
if (maxFiles && results.length >= maxFiles) return; // early exit
82+
}
83+
} else if (stat.isFile()) {
84+
const base = path.basename(current);
85+
if (base === "sbom.json") {
86+
pushIfValid(current);
87+
}
88+
}
89+
};
90+
visit(dir);
91+
}
5592
return results;
5693
}

0 commit comments

Comments
 (0)