T09 · Insecure Skill Coding Practices
- Location
src/monitor.js:31- Finding
Path Traversal Through Unvalidated Skill Names
- Content
View full analysis
Vulnerability Details
File Location:
src/monitor.js:31-34,src/monitor.js:42-48, andsrc/monitor.js:96-111
Vulnerability Type: Path traversal leading to unauthorized local file access and modification
Risk Level: HighVulnerable Code
javascript record(skillName, metric) { const timestamp = new Date().toISOString(); const entry = { skill: skillName, timestamp, ...metric, }; const filePath = path.join(this.metricsPath, `${skillName}.jsonl`); fs.appendFileSync(filePath, JSON.stringify(entry) + '\n'); return entry; }javascript getMetrics(skillName, days = 7) { const filePath = path.join(this.metricsPath, `${skillName}.jsonl`); if (!fs.existsSync(filePath)) return []; const since = new Date(Date.now() - days * 24 * 60 * 60 * 1000).toISOString(); return fs.readFileSync(filePath, 'utf-8') .split('\n') .filter(line => line.trim()) .map(line => JSON.parse(line)) .filter(m => m.timestamp >= since); }javascript cleanOldMetrics() { const cutoff = new Date(Date.now() - this.retentionDays * 24 * 60 * 60 * 1000); this.getMonitoredSkills().forEach(skill => { const filePath = path.join(this.metricsPath, `${skill}.jsonl`); const lines = fs.readFileSync(filePath, 'utf-8') .split('\n') .filter(line => { if (!line.trim()) return false; try { const metric = JSON.parse(line); return new Date(metric.timestamp) > cutoff; } catch { return false; } }); fs.writeFileSync(filePath, lines.join('\n')); }); }Technical Analysis
The
skillNameparameter is incorporated directly into a filesystem path without validation or canonical path confinement. Node.jspath.join()normalizes traversal components but does not ensure that the resulting path remains beneaththis.metricsPath.A value containing parent-directory components, such as
../../target, can therefore resolve outside the i ...[truncated 2269 chars]- Remediation
View remediation
Remediation Suggestions
-
Restrict skill identifiers to a safe allowlist
Accept only identifiers that cannot contain path syntax:
javascript function validateSkillName(skillName) { if ( typeof skillName !== 'string' || !/^[A-Za-z0-9_-]+$/.test(skillName) ) { throw new TypeError('Invalid skill name'); } } -
Resolve and confine every generated path
Verify that the canonical candidate remains beneath the configured metrics directory:
javascript function getMetricPath(metricsPath, skillName) { validateSkillName(skillName); const base = path.resolve(metricsPath); const candidate = path.resolve(base, `${skillName}.jsonl`); if (!candidate.startsWith(base + path.sep)) { throw new Error('Metric path escapes the configured directory'); } return candidate; }Use this helper consistently in
record(),getMetrics(), andcleanOldMetrics(). -
Reject symlinks and non-regular files
Before reading, appending, or rewriting an existing metric file, inspect it with
fs.lstatSync()and reject symbolic links and non-regular files. Where stronger guarantees are required, use file-opening flags supported by the platform to prevent symlink following and reduce time-of-check/time-of-use races. -
Harden metric parsing and resource limits
Parse each line inside a controlled
try/catch, enforce a maximum file size and line count, and validate record schemas before returning them. This prevents malformed or oversized files from causing avoidable denial of service. -
Add security regression tests
Test and reject values including
../target,..\\target, absolute paths, path separators, empty strings, NUL bytes, Unicode separator variants, and symlink-based escapes.
-