Merge pull request #12 from decksoftware/feat/provisioning-run-fixes

fix: make --provision-tools actually run Gitleaks/Trivy (Trivy no-DB + transient retry)

Author: decksoftware

csreview/src/securityTools.js

@@ -274,7 +274,10 @@ export const RUN_SPECS = {
   },
   trivy: {
     normalize: normalizeTrivyFindings,
-    argv: (rootDir) => ['fs', '--scanners', 'vuln,misconfig,secret', '--format', 'json', '--quiet', rootDir],
+    // misconfig + secret only: dependency vulnerabilities are already covered by
+    // OSV-Scanner, and dropping `vuln` avoids Trivy's large vulnerability-DB
+    // download (which otherwise blows the run timeout on the first scan).
+    argv: (rootDir) => ['fs', '--scanners', 'misconfig,secret', '--format', 'json', '--quiet', rootDir],
     parsesStdout: true,
   },
 };
@@ -285,25 +288,43 @@ export const RUN_SPECS = {
  * checksum-verified tool path). Fail-open: any error yields available:false.
  *
  * @param {string} toolKey
- * @param {{ rootDir: string, toolPath: string, exec: (path: string, argv: string[]) => Promise<{stdout: string, stderr?: string}> }} opts
+ * @param {{ rootDir: string, toolPath: string, exec: (path: string, argv: string[]) => Promise<{stdout: string, stderr?: string}>, retryDelayMs?: number }} opts
  * @returns {Promise<{tool: string, available: boolean, findings: Array<object>, rawCount: number, error?: string}>}
  */
 export async function runSecurityTool(toolKey, opts) {
   const spec = RUN_SPECS[toolKey];
   if (!spec) return { tool: toolKey, available: false, findings: [], rawCount: 0, error: `unknown tool ${toolKey}` };
-  try {
+
+  const runOnce = async () => {
     const { stdout } = await opts.exec(opts.toolPath, spec.argv(opts.rootDir));
     const parsed = JSON.parse(stdout || (toolKey === 'gitleaks' ? '[]' : '{}'));
     const findings = spec.normalize(parsed);
     return { tool: toolKey, available: true, findings, rawCount: findings.length };
-  } catch (err) {
-    return {
-      tool: toolKey,
-      available: false,
-      findings: [],
-      rawCount: 0,
-      error: err && err.message ? err.message : String(err),
-    };
+  };
+
+  try {
+    return await runOnce();
+  } catch (firstErr) {
+    const message = firstErr && firstErr.message ? firstErr.message : String(firstErr);
+    // Never retry a timeout (that would double a slow scan). DO retry a transient
+    // failure once — a freshly provisioned binary can be briefly locked on Windows
+    // (e.g. by antivirus) the instant after it is downloaded.
+    const isTimeout = Boolean(firstErr && (firstErr.killed || /timed?\s*out|ETIMEDOUT/i.test(message)));
+    if (isTimeout) {
+      return { tool: toolKey, available: false, findings: [], rawCount: 0, error: message };
+    }
+    await new Promise((resolve) => setTimeout(resolve, opts.retryDelayMs == null ? 400 : opts.retryDelayMs));
+    try {
+      return await runOnce();
+    } catch (secondErr) {
+      return {
+        tool: toolKey,
+        available: false,
+        findings: [],
+        rawCount: 0,
+        error: secondErr && secondErr.message ? secondErr.message : String(secondErr),
+      };
+    }
   }
 }
 

csreview/test/security-tools.test.js

@@ -190,14 +190,54 @@ test('runSecurityTool parses injected stdout and is fail-open on errors', async
   const throwExec = async () => {
     throw new Error('ENOENT');
   };
-  const fail = await runSecurityTool('gitleaks', { rootDir: '/p', toolPath: 'gitleaks', exec: throwExec });
+  const fail = await runSecurityTool('gitleaks', {
+    rootDir: '/p',
+    toolPath: 'gitleaks',
+    exec: throwExec,
+    retryDelayMs: 0,
+  });
   assert.equal(fail.available, false);
   assert.match(fail.error, /ENOENT/);
 
   const unknown = await runSecurityTool('nope', { rootDir: '/p', toolPath: 'x', exec: okExec });
   assert.equal(unknown.available, false);
 });
 
+test('runSecurityTool retries once on a transient failure but not on a timeout', async () => {
+  // Transient: first call throws (e.g. just-provisioned binary briefly locked), second succeeds.
+  let calls = 0;
+  const flakyExec = async () => {
+    calls += 1;
+    if (calls === 1) throw new Error('EBUSY: resource busy or locked');
+    return { stdout: JSON.stringify([{ RuleID: 'x', File: 'a.js', StartLine: 1, Secret: 'zzzz' }]) };
+  };
+  const retried = await runSecurityTool('gitleaks', {
+    rootDir: '/p',
+    toolPath: 'gitleaks',
+    exec: flakyExec,
+    retryDelayMs: 0,
+  });
+  assert.equal(retried.available, true);
+  assert.equal(calls, 2);
+
+  // Timeout: must NOT retry (would double a slow scan).
+  let timeoutCalls = 0;
+  const timeoutExec = async () => {
+    timeoutCalls += 1;
+    const e = new Error('Command failed: timed out');
+    /** @type {any} */ (e).killed = true;
+    throw e;
+  };
+  const timedOut = await runSecurityTool('trivy', {
+    rootDir: '/p',
+    toolPath: 'trivy',
+    exec: timeoutExec,
+    retryDelayMs: 0,
+  });
+  assert.equal(timedOut.available, false);
+  assert.equal(timeoutCalls, 1);
+});
+
 test('runSecurityTool passes the rootDir only as argv (no shell interpolation)', async () => {
   /** @type {string[]|null} */
   let capturedArgv = null;