T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/nightly.ts:21
- Finding
- Failed dependency audits are incorrectly reported as secure## Vulnerability Details **File Location**: `scripts/nightly.ts`, lines 21–60 **Vulnerability Type**: Improper error handling and false security reporting **Risk Level**: Medium ```ts 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 `execSync` throws when `npm audit --audit-level=high` exits 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 `runCmd` helper catches every such exception and converts it into an ordinary string beginning with `ERROR:`. Consequently, the `try/catch` in `auditSkills` never receives the exception. It unconditionally appends the `Secure` result without inspecting the returned value. The vulnerability counter remains zero, causing the ...[truncated 1737 chars]
- Remediation
- ## Remediation Suggestions 1. Return a structured command result containing standard output, standard error, and exit status, or allow `execSync` exceptions to propagate to the caller. 2. Only report a skill as secure after `npm audit` completes successfully with a verified zero exit code. 3. 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. 4. Treat audit execution failures as an unknown or failed state rather than a secure state. 5. Include enough sanitized diagnostic information in the report to support investigation without exposing credentials or sensitive environment data. 6. 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.