T09 · Insecure Skill Coding Practices
- Location
scripts/nightly.ts:21- Finding
Failed dependency audits are incorrectly reported as secure
- Content
View full analysis
Vulnerability Details
File Location:
scripts/nightly.ts, lines 21–60
Vulnerability Type: Improper error handling and false security reporting
Risk Level: Mediumts function runCmd(cmd: string, cwd: string = WORKSPACE_DIR): string { try { log(`Running: ${cmd} in ${cwd}`); const output = execSync(cmd, { cwd, encoding: 'utf-8', timeout: 60000 }); // 60s timeout return output.trim(); } catch (err: any) { const errorMsg = `Command failed: ${cmd}\nError: ${err.message}`; log(errorMsg); return `ERROR: ${errorMsg}`; } } // --- TASKS --- // 1. Skill Audit (npm audit) function auditSkills(): string { let report = "### 📦 Skill Audit\n\n"; if (!fs.existsSync(SKILLS_DIR)) { return report + "No skills directory found.\n"; } const skills = fs.readdirSync(SKILLS_DIR); let vulnerableCount = 0; for (const skill of skills) { const skillPath = path.join(SKILLS_DIR, skill); if (fs.existsSync(path.join(skillPath, 'package.json'))) { try { // Just check, don't fix automatically yet runCmd('npm audit --audit-level=high', skillPath); report += `- ✅ ${skill}: Secure\n`; } catch (e) { report += `- ⚠️ ${skill}: High vulnerabilities found!\n`; vulnerableCount++; } } } if (vulnerableCount === 0) report += "\nAll skills are clean.\n"; return report; }Technical Analysis
execSyncthrows whennpm audit --audit-level=highexits with a nonzero status. This can occur when high-severity vulnerabilities are detected, the audit service cannot be reached, package metadata is invalid, or the command otherwise fails.The
runCmdhelper catches every such exception and converts it into an ordinary string beginning withERROR:. Consequently, thetry/catchinauditSkillsnever receives the exception. It unconditionally appends theSecureresult without inspecting the returned value. The vulnerability counter remains zero, causing the ...[truncated 1737 chars]- Remediation
View remediation
Remediation Suggestions
- Return a structured command result containing standard output, standard error, and exit status, or allow
execSyncexceptions to propagate to the caller. - Only report a skill as secure after
npm auditcompletes successfully with a verified zero exit code. - Distinguish among these outcomes:
- Audit completed with no matching vulnerabilities.
- Audit completed and found vulnerabilities.
- Audit could not be completed because of network, registry, timeout, configuration, or execution errors.
- Treat audit execution failures as an unknown or failed state rather than a secure state.
- Include enough sanitized diagnostic information in the report to support investigation without exposing credentials or sensitive environment data.
- Add automated tests covering a zero exit status, a vulnerability-related nonzero status, a timeout, a missing executable, and a registry/network failure.
A suitable design is:
ts interface CommandResult { ok: boolean; output: string; error?: string; } function runCmd(cmd: string, cwd: string = WORKSPACE_DIR): CommandResult { try { const output = execSync(cmd, { cwd, encoding: 'utf-8', timeout: 60000 }); return { ok: true, output: output.trim() }; } catch (err: any) { return { ok: false, output: err.stdout?.toString().trim() || '', error: err.stderr?.toString().trim() || err.message }; } } const result = runCmd('npm audit --audit-level=high', skillPath); if (result.ok) { report += `- ✅ ${skill}: No high-severity vulnerabilities reported\n`; } else { report += `- ⚠️ ${skill}: Audit found vulnerabilities or could not complete\n`; vulnerableCount++; }For more precise reporting, invoke
npm audit --json, parse its structured output, and separately classify confirmed vulnerabilities and operational audit failures.- Return a structured command result containing standard output, standard error, and exit status, or allow