T09 · Insecure Skill Coding Practices
Error
- Location
- memory-router.js:96
- Finding
- Destructive tiering continues after backup or archive write failure<![CDATA[ ## Vulnerability Details **File Location**: `memory-router.js`, lines 96–128 and 467–527 **Vulnerability Type**: Unchecked filesystem operation failure **Risk Level**: High ### Vulnerable Code ```js function writeSafe(filepath, content) { const resolved = path.resolve(filepath); // Path validation — reject paths outside workspace const validation = validatePath(filepath); if (!validation.ok) { console.error(`[memory-router] Refusing to write: ${validation.reason}`); return; } // Symlink check — must be before any file operations let fileExists = false; try { const lstat = fs.lstatSync(resolved); if (lstat.isSymbolicLink()) { console.error(`[memory-router] Refusing to write to symlink: ${filepath}`); return; } fileExists = true; } catch { // File doesn't exist yet — that's fine, we're creating it } // Size check if (content.length > MAX_FILE_SIZE) { console.error(`[memory-router] Content too large (${content.length} bytes) for ${filepath}`); return; } // Atomic write: write to temp file, then rename ensureDir(path.dirname(filepath)); const tmpPath = resolved + '.tmp-' + process.pid; try { fs.writeFileSync(tmpPath, content, 'utf8'); fs.renameSync(tmpPath, resolved); } catch (err) { // Clean up temp file on failure try { fs.unlinkSync(tmpPath); } catch {} console.error(`[memory-router] Write failed for ${filepath}: ${err.message}`); } } ``` ```js writeSafe(backupFile, backupContent); console.log(`[memory-router] ✅ Pre-tier backup: ${backupFile} (${lineCount} lines saved)`); // ─── Actual tiering ─── const archiveDir = path.join(WORKSPACE, config.tiering.archiveDir); ensureDir(archiveDir); const archiveFile = path.join(archiveDir, `MEMORY-archive-${getToday()}.md`); let archiveContent = `# Archived Memory Sections\n\n`; archiveContent += `> Auto-archived on ${getToday()} by memory-router skill\n`; archiveContent += `> Original MEMORY.md had ...[truncated 3081 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Make `writeSafe()` return an explicit boolean or structured result: ```js return { ok: true, path: resolved }; ``` Return `{ ok: false, error }` for every validation or filesystem failure. 2. Abort tiering immediately unless the backup write succeeds: ```js const backupResult = writeSafe(backupFile, backupContent); if (!backupResult.ok) { throw new Error(`Tiering aborted: backup failed: ${backupResult.error}`); } ``` 3. Verify the backup after writing: - Confirm that it is a regular file and not a symlink. - Read it back. - Compare its byte length or a cryptographic hash with the intended backup content. 4. Require successful archive creation and verification before replacing `MEMORY.md`. 5. Perform the operation transactionally: - Write backup, archive, and new core content to temporary files. - Flush and validate all temporary files. - Commit the archive first and replace `MEMORY.md` last. - Roll back or retain the original file if any stage fails. 6. Do not print success messages until the corresponding operation has been verified. 7. Account for generated metadata and formatting when applying size limits, or use separate, appropriately bounded limits for input files and generated backup/archive files. ]]>